CL/GL sharing support more texture formats

[smunaut]

The support implemented in #166 has some limitations on the supported texture format. I have been discussing in that (now closed) issue the possibility to support more formats but I thought it'd be better to open a new issue rather than keep talking in a closed one.

I've so far removed the quick check that limit the "create memory" to the currently supported texture format and what I noticed is that the call then hangs if you use unsupported textures.

I dug a bit into that and noticed two things :

• The hang I notice is probably some bug in the error path. _eglDestroyImageCommon is only called if eglExportDMABUFImageMESA failed. So it should definitely not hang and it should return the error code instead of hanging but it's also not the root of my problem, ideally that call should work.
• eglExportDMABUFImageMESA fails because dri2_can_export_dma_buf_image returns FALSE. And that's the root of the issue.
• And the above eventually falls to missing formats in dri2_format_table ... but I have no idea if I can just add format to that table or what it implies or whatever ...

[smunaut]

Ok, so digging even further :

• The hang is a bug in Mesa (missing mutex release in an error path)
• The limitation of image format seems to mostly stem from DRI image format and so should be dealt with in mesa first ... and then maybe some support would also needed here as a second step but mesa is definitely the first step.

[kallaballa]

Wow. great work! Today I'll jump in and follow your path. In general (afaik) Mesa/EGL is currently in heavy development. Maybe reaching out to Mesa might be a good idea.

[kallaballa]

I'll use GL_RGBA32F for starters with my code just to see if it breaks the same way. But we need a common example to have a chance. Do you have one?

[smunaut]

Proposed fix for the locking issue btw : https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/24487/diffs?commit_id=fe844bae481b4f2369ab78537d2a800817973389

As for example, I use the following code. It doesn't really do anything except just trying to create the texture / cl mem object and nothing else ATM.

main.c.txt

[kallaballa]

Thx!

[kallaballa]

I think i found the problem. __DRI_IMAGE_FORMAT_NONE collides with an actual format. commenting those 2 lines fixes that: https://github.com/Mesa3D/mesa/blob/957009978ef6d7121fc0d710d03bc20097d4d46b/src/gallium/frontends/dri/dri_helpers.c#L679-L680

Now the cl_mem is succesfully mapped.

[kallaballa]

Anyway that doesn't make a complete patch. I'll create an issue at mesa to maybe get an opinion from someone with the bigger picture.

[smunaut]

Not sure what you mean with __DRI_IMAGE_FORMAT_NONE colliding. I don't see any other image format with the same code.

Here's the flow I traced :

• dri2_can_export_dma_buf_image reports the image can't be exported because
  it doesn't have an attribute __DRI_IMAGE_ATTRIB_FOURCC

• That attribute comes either from image->dri_fourcc directly or from the result
  of dri2_get_mapping_by_format(image->dri_format)

• The image is created by dri2_create_image_khr_texture
  This in turn will cann dri2_create_from_texture to initialize the image properties
  AFAICT this will not set image->dri_fourcc but it will set image->dri_format
  And it's set from mapping the gl texture format as such :
  img->dri_format = driGLFormatToImageFormat(obj->Image[face][level]->TexFormat);

• That method driGLFormatToImageFormat uses a format_mapping table to match.

• And that table just doesn't have any float32 types in it defined at all. And looking
  at the list of available __DRI_IMAGE_FORMAT_xxxx there are none that would match.

So I guess we'd need to introduce new DRI image formats as the first step.

[smunaut]

Apparently going through DRI images and the export dma buf api is the wrong way to do things for interop according to DRI devs I talked to ...

MesaGLInteropEGLExportObject seems to be the way to implement it AFAICT.
It also seems to return a dmabuf but directly from a GL object handle without any intermediate EGLImage step.

[kallaballa]

Hmmm. interesting! Gonna be back at it in a couple of days

[smunaut]

Implemented a quick PoC replacing eglExportDMABUFImageMESA with MesaGLInteropEGLExportObject and it's doing something 😅

And it almost looks right : https://i.imgur.com/SmtjQm2.jpg vs https://i.imgur.com/KPFMzAs.jpg

[kallaballa]

very excellent! sorry i can't help right now. sick in bed

[smunaut]

No worries, take care !

FWIW I pushed my current PoC https://github.com/smunaut/compute-runtime/tree/clgl

Note that it depends on some mesa patches https://gitlab.freedesktop.org/246tnt/mesa/-/commits/clgl-export

Those symbols are normally used internally. They can be accessed just using dlsym() looking for them at the global scope but not in all cases. If you have GLVND (vendor neutral dispatcher) and EGL, then that combination doesn't work. So after talking in the #dri-devel channel, the best option seemed to be to expose those function through the classic GetProcAddress mechanism and that's what those mesa patches do ...

[smunaut]

Found the bug above btw and it's unrelated to the change.

The extension is calling glGetTexLevelParameteriv to collect data about the texture. but that assumes that the texture is currently the one being bound in the GL state which is not necessarily the case ...