Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

QueryDmaBufModifiersEXT does not guarantee obtained modifiers are actually usable #201

Open
yshui opened this issue May 27, 2024 · 16 comments

Comments

@yshui
Copy link
Contributor

yshui commented May 27, 2024

QueryDmaBufModifiersEXT lets me query which modifiers are supported for a given format, but the information I provide to this API (i.e. format) is not sufficient to determine whether a modifier will actually work. For example, at allocation time, mesa AMDGPU will also check if the size of the buffer is within supported range (see: https://gitlab.freedesktop.org/mesa/mesa/-/blob/2487a875527f636565a7b39036690fbf7c5d46db/src/gallium/drivers/radeonsi/si_texture.c#L1564), and will fail if I try to allocate a texture that's too large.

Can QueryDmaBufModifiersEXT be extended to incorporate size info into the query as well? This will make it "complete" w.r.t, e.g. gbm_bo_create_with_modifiers.

@stonesthrow
Copy link
Contributor

Reference: https://registry.khronos.org/EGL/extensions/EXT/EGL_EXT_image_dma_buf_import_modifiers.txt

We need original authors or from the dmabuf community to comment and provide feedback.

If I recall documentation, its possible the size of the buffer is too small and a fault will happen (MESA or Weston). So I guess the original authors realized the "size" could be a lie. Either its the right size or there is a fault. Although size checks are nice check to add.

As for max texture size, you can query GL_MAX_TEXTURE_SIZE

@yshui
Copy link
Contributor Author

yshui commented Aug 21, 2024

As for max texture size, you can query GL_MAX_TEXTURE_SIZE

GL_MAX_TEXTURE_SIZE is the global maximum texture size, right? But there are specific texture size limits for a given modifier. Vulkan has VkImageFormatProperties::maxExtent, which can be queried by chaining a VkPhysicalDeviceImageDrmFormatModifierInfoEXT when calling vkGetPhysicalDeviceImageFormatProperties2

@stonesthrow
Copy link
Contributor

That's Vulkan API. And if you have Vulkan and EGL/GL these are likely the same - maximum the GPU can handle.

@yshui
Copy link
Contributor Author

yshui commented Aug 21, 2024

This is a useful piece of information, can we add this to EGL as well?

@stonesthrow
Copy link
Contributor

You can query max rendering surface from the EGL Configs, that will match up with Max renderer able image of Vulkan.
eglGetConfigAttrib:
EGL_MAX_PBUFFER_WIDTH
Returns the maximum width of a pixel buffer surface in pixels.

EGL_MAX_PBUFFER_HEIGHT
Returns the maximum height of a pixel buffer surface in pixels.

EGL_MAX_PBUFFER_PIXELS
Returns the maximum size of a pixel buffer surface in pixels.

@yshui
Copy link
Contributor Author

yshui commented Aug 21, 2024

That is not what I want though. What I want to query is the maximum possible image size for a given DRM format modifier, or whether a given image dimension is supported by a DRM format modifier.

@stonesthrow
Copy link
Contributor

EGL Images created from textures, and textures from EGL Images would be limited to that GL_MAX_TEXTURE_SIZE.
Since you are talking DRM formats, then you are talking
https://registry.khronos.org/EGL/extensions/EXT/EGL_EXT_image_dma_buf_import.txt and
https://registry.khronos.org/EGL/extensions/EXT/EGL_EXT_image_dma_buf_import_modifiers.txt
There is no mention of size limits with importing dmabufs into EGLImages or importing any native buffer.
However, once that EGLImage is glEGLTargetTexture2DOES to import into a texture - then the texture limit will apply.
Maybe EGLImage needs a limit on creation - but EGL just identifies the type and properties of the buffer for import/sharing with other khronos APIs.

@yshui
Copy link
Contributor Author

yshui commented Aug 21, 2024

There is no mention of size limits with importing dmabufs into EGLImages or importing any native buffer.

Yes, that's what the spec says. But in practice drivers will impose size limits, and that information is exposed nowhere in the API, and that is the problem.

Using Vulkan you will be able to query this information, and this is missing from EGL.

@stonesthrow
Copy link
Contributor

I understand what you are saying.
With Vulkan, you will use the VkImage for rendering or sampling, such that the GPU may have limits on what it can handle.

EGL could have an EGLImage that is not used with OpenGL/ES. So there may not be a GPU limit - if say used with OpenCL or such. So the limit is only imposed when the importing to a texture - that has a GPU limit.

@yshui
Copy link
Contributor Author

yshui commented Aug 21, 2024

EGL could have an EGLImage that is not used with OpenGL/ES. So there may not be a GPU limit - if say used with OpenCL or such. So the limit is only imposed when the importing to a texture - that has a GPU limit.

I don't understand what you are saying. There are limits when creating an EGLImage with DRM format modifier.

@stonesthrow
Copy link
Contributor

You'll have to point that out, I don't see what modifiers you are referring to,
https://registry.khronos.org/EGL/extensions/EXT/EGL_EXT_image_dma_buf_import.txt
There is EGL_WIDTH and EGL_HEIGHT as attributes for creating new image from dmabuf.
So other than the MAX_TEXTURE_SIZE there is nothing in spec. Unless as you say there are some modifiers that indicate a limit for this type of client buffer.

@yshui
Copy link
Contributor Author

yshui commented Aug 22, 2024

You'll have to point that out, I don't see what modifiers you are referring to,

What do you mean? Now I am confused. We are talking about DRM format modifiers, right?

So other than the MAX_TEXTURE_SIZE there is nothing in spec.

Yes! And that is the problem I've been trying to raise this whole time. Drivers in practice impose size limit based on modifiers used, but EGL API does not expose that.

@stonesthrow
Copy link
Contributor

My apologies, I have several concurrent tasks, some related.
So yes, need to get original authors or people working in this space to create a new version of this extension and add the queries you need.
The EGL work group is not active other than a few people checking in on new issues and proposals. So to get changes requires people to do the proposals.

@stonesthrow
Copy link
Contributor

stonesthrow commented Aug 22, 2024

So the thing to do would be copy https://registry.khronos.org/EGL/extensions/EXT/EGL_EXT_image_dma_buf_import_modifiers.txt
into a new spec proposal EGL_EXT_image_dma_buf_import_modifiers2. And in this new spec propose a new way to query the max size. New query function or modifiers to use.

Put that in a pull request and we get as many of the linux participants to review as possible for feedback - needed to justify making this a EXT extension.

https://github.com/KhronosGroup/EGL-Registry/wiki

@stonesthrow
Copy link
Contributor

@linyaa-kiwi add for participant/monitor

@yshui
Copy link
Contributor Author

yshui commented Aug 23, 2024

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants