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

Allow DRM format modifier to be set during vkBindImageMemory #2397

Open
yshui opened this issue Jul 24, 2024 · 19 comments
Open

Allow DRM format modifier to be set during vkBindImageMemory #2397

yshui opened this issue Jul 24, 2024 · 19 comments
Assignees

Comments

@yshui
Copy link

yshui commented Jul 24, 2024

Context

I was working on wine, trying to (partially) implement VK_EXT_external_memory_win32 on top of native Vulkan. The basic idea is to encapsulate dma-buf file descriptors as win32 handles, and vice versa when crossing the win32/unix boundary in wine.

Problem is, to bind a VkImage to dma-buf memory, the image must be created with a VkImageDrmFormatModifierExplicitCreateInfoEXT. Which means I must know what the modifier is at image creation time.

Obviously win32 doesn't have the concept of drm format modifiers. So I must smuggle the information over somehow. And the only reasonable place to hide this information is when I encapsulate dma-buf descriptors into win32 handles. But these handles are not part of the information used to create VkImages. Which is to say, there is no way to make the drm format modifier information available at image creation time.

Proposal

Since the win32 handles carry the format modifier, and VkDeviceMemory is created out of these handles, vkBindImageMemory is going to be the point where the modifier and the VkImage first come into contact.

Add a structure extending VkBindImageMemoryInfo to allow specifying the drm format modifier at bind time.

Alternative?

Seems to me this is the only way to do it. Unless there is some other way to smuggle the modifier information across the win32/unix boundary.


Related to
VK_EXT_image_drm_format_modifier and VK_EXT_external_memory_dma_buf @versalinyaa

@oddhack
Copy link
Contributor

oddhack commented Aug 7, 2024

Tagging for SI TSG.

@cubanismo
Copy link

cubanismo commented Aug 7, 2024

Why not use opaque FD handles instead of format modifiers + dma-buf? Opaque FD handles are the direct analogy of opaque win32 handles on Linux. dma-buf has different mechanics and is only needed for interoperation with native platform stuff like presentation and display.

@yshui
Copy link
Author

yshui commented Aug 7, 2024

@cubanismo we need dma-buf because not all relevant APIs support opaque fd export. for example, vaapi. Also dma-buf seems like the future-proof option to use.

@cubanismo
Copy link

I'm not clear what vaapi has to do with implementing win32 handle support in Vulkan. opaque FD APIs aren't going anywhere, so I'm not sure what you mean by future proofing either.

@cubanismo
Copy link

To be clear, specifying a format modifier at image bind time just doesn't work. You'd have to effectively defer all image creation work in the driver until bind time, lie about the required size, etc. You couldn't even query whether the image creation parameters were supported without the format modifier, which has to happen before creating the image to ensure valid usage. It breaks the API in so many ways.

@yshui
Copy link
Author

yshui commented Aug 7, 2024

I can see some solutions here.

First, the image is created with a special create info to indicate its modifier will be determined at bind time. For such images, we declare that certain queries (e.g. memory requirements) will not return meaningful results. This should be fine because the memory will be allocated by an external API anyway.

Secondly, while it is true that modifiers and change the sets of valid creation parameters, some drivers can accommodate many different modifiers for a given set of creation parameters. Besides that, the external API allocating the memory often has the ability to choose the right modifier for provided creation parameters too. Although creation parameters used by external APIs don't necessarily map 1-to-1 to Vulkan image create info.

So, I think it would be good enough for my use case if vkBindImageMemory would just fail when the device memory has a modifier incompatible with the image.

@cubanismo
Copy link

For such images, we declare that certain queries (e.g. memory requirements) will not return meaningful results.

These queries aren't necessarily just for the application's benefit. Our driver, at least, needs to know these values at image creation time.

Secondly, while it is true that modifiers and change the sets of valid creation parameters, some drivers can accommodate many different modifiers for a given set of creation parameters.

I don't think "some drivers [and hardware]" qualifies as a good solution. This might all get as far as a demo, but not a robust API.

@yshui
Copy link
Author

yshui commented Aug 7, 2024

I don't think "some drivers [and hardware]" qualifies as a good solution.

But isn't that what the extension system is for? If some driver/hardware cannot support this, they don't have to.

@cubanismo
Copy link

That's one potential motivation for an extension, yes, but in general extensions are just how the Vulkan featureset is advanced. It's not a hard rule, but usually unless some vendor is purposely trying to differentiate themselves or some bit of functionality only applies to a subset of the Vulkan ecosystem (A single OS, a class of hardware), the goal of Vulkan is as uniform an interface across vendors as possible, since disparate featuresets add complexity and are the antithesis of usability. It's a "standard," afterall.

@yshui
Copy link
Author

yshui commented Aug 9, 2024

Fair point. On the other hand I don't see other solutions that will cover as many use cases. :-\ Advises welcome.

BTW, I am curious how this is done on windows? Surely the concept similar to DRM format modifier must exist, just not exposed to the API users, right? Yet you don't need to provide the HANDLE you need to import when creating the VkImage.

@cubanismo
Copy link

The intention behind the design of the OPAQUE handle sharing (both win32 handle and POSIX FD variants) was it could operate in one of two modes:

-Dedicated allocation required: The kernel driver may encapsulate the layout in the kernel somehow, similar to what you're proposing. Proprietary APIs would be used to discover the layout from each vendor's kernel driver. This was to support "legacy" driver stacks until they could be adapted to use the method below. However, we messed up the API and using it that way suffers from the same issues I identified above. The Android native buffer version takes pains to fix this mistake. To avoid all this, our implementation doesn't go that route most of the time and tries to always use the method below regardless of dedicated allocation use.
-Non-dedicated allocations/dedicated allocation optional: The proprietary aspects of the image's internal layout are derived consistently based only on the API-visible image creation parameters, and hence match regardless of which process creates the image. The only valid way to share images using the opaque handle types is to create two identical images bound to the same device memory payload, and since the Vulkan API image creation parameters must be the same on both sides, and the calculations used to derive internal proprietary layouts from those parameters are the same, the internal proprietary layout calculations always result in the same layout. This is the implementation preferred in our driver, both on Windows and Linux.

@yshui
Copy link
Author

yshui commented Aug 9, 2024

Thanks, that's very informative.

I guess the analogue to option 2 in Linux would be: create a vkImage with a list of all possible modifiers, and let the driver choose one; then at memory bind time, we check if the incoming modifier matches the one the image is created with, reject if not.

I wonder if Linux drivers would choose modifiers consistently as well... at least mesa provides both vaapi and vulkan 🤔

@yshui
Copy link
Author

yshui commented Aug 9, 2024

Also it sounds like it wouldn't be a bad idea to add a DRM modifier equivalent to the win32 API as well

@cubanismo
Copy link

As far as I'm aware, nothing technical prevents drivers from exposing the existing DRM format modifier extension for use on windows with win32 handles. I just don't know of any drivers doing that at the moment. After all, the conformance test uses DRM format modifiers with OPAQUE_FD handle types if the driver supports it.

@yshui
Copy link
Author

yshui commented Aug 9, 2024

OK, I think I have an idea.

I think by default we have to rely on the drivers choosing modifiers consistently. I don't know what's the likelihood of it being chosen consistently, might need to do some research on that. If it's really bad, I think it is even possible to have wine take over modifier selection. Though I feel that could get messy, so probably not.

On top of that, wine will also expose the DRM modifier extension. It won't be useful to any existing applications, but it will create a backdoor which wine can take advantage of when using Vulkan API internally.

This way wine can expose a semi-legitimate external_memory_win32 extension, and also have something extra if that is not enough.

@cubanismo
Copy link

Also it sounds like it wouldn't be a bad idea to add a DRM modifier equivalent to the win32 API as well

I think the question here would be: Is anything additional needed from the Vulkan specification/extension side? The existing DRM format modifier extension and DRM format modifiers themselves should work fine on Windows with the existing win32 external objects extensions if drivers choose to support them both simultaneously.

@linyaa-kiwi
Copy link
Contributor

I agree with nearly everything that James said here. (I'm the author of VK_EXT_image_drm_format_modifier).

In EGL, a "slot machine" mechanic dominated much of the api. That is, for many operations, there exists no query api that tells the app if the operation is legal. The only way for the app to determine if the operation is legal is to try it. Pull the lever on the slot machine and pray that it succeeds. If the op fails, then try again with different parameters, or give up completely. This slot machine mechanic existed in many places, and in particular the EGL apis for importing dma-buf images with DRM format modifiers and then importing the EGLImage as a GL texture.

Vulkan purposely avoids the slot machine mechanic nearly everywhere. One reason is that, compared to EGL/GL drivers, a Vulkan driver has less freedom to "fix things" for the app. For example, in EGL/GL, if an image's creation params were not truly supported by the hardware, but almost supported, then the driver could still allow the image creation to succeed as long as the driver "fixed" the hardware-incompatibility with behind-the-scenes magic each time the image was used. Because Vulkan is much much much more explicit than EGL/GL, Vulkan drivers have much less freedom to do such behind-the-scenes fixes.

There are two principles here: (a) no slot machines in the api and (b) drivers have few opportunities for behind-the-scenes fixups. Those principles mandate a lot of the complexity and constraints in VK_EXT_image_drm_format_modifier.

Due to "no slot machines", Vulkan requires the app to query (with vkPhysicalDeviceImageFormatProperties2) if a set of image creation params (including the modifier) is supported by the driver before the app attempts to create an image with those params. The modifier must be included in the query's params because the modifier can significantly impact the allowable ranges for the other image creation params. For example, the hardware may not support VkFormat foo with modifier bar; or the hardware may not support VkImageUsageFlags chow with modifier bar; or maybe the driver could still make it work, but it would require a mountain of code that no one wants to write. The queries that are required by VK_EXT_image_drm_format_modifier, they are necessary to prevent vkCreateImage from becoming a slot machine function.

Next, as drivers work today, vkCreateImage immediately creates a gpu-specific data structure that represents everything that the gpu needs to know about the image, and the cpu data structures needed to describe the image. Of course, both the gpu and cpu structures are highly dependent on the image's memory layout, memory tiling, pixel compression format, ancillary gpu-metadata storage, etc. VK_EXT_image_drm_format_modifier requires, at vkCreateImage time, that either the app chooses the memory layout (VK_IMAGE_TILING_LINEAR or VkImageDrmFormatModifierExplicitCreateInfoEXT) or that the driver chooses the memory layout (VK_IMAGE_TILING_OPTIMAL or VkImageDrmFormatModifierListCreateInfoEXT), because without a memory layout it's impossible to create the cpu and gpu structs. In other words, if no memory layout is chosen at vkCreateImage time, then it's impossible to really create the VkImage. Deferring the selection of modifier to vkBindImageMemory2 hits this problem. Even if we designed an extension that permits the VkImage, if the image creation params were incomplete, to internally remain a stub non-functional VkImage until the remaining params are provided elsewhere, the existence of a stub VkImage introduces a cascade of undesirable consequences in other parts of the Vulkan api.

In summary, if we add the extension you're requesting, and defer specification of modifier to vkBindImageMemory2, then there will be two side-effects, both undesirable:

  • Creation of modifier-backed VkImages now has an ill-defined slot machine mechanic in the interaction between vkBindImageMemory2 and vkCreateImage.
  • vkCreateImage doesn't create an image at all. The best the driver can do in this extension is to (a) merely cache the incomplete params given to vkCreateImage and (b) defer the real creation of the VkImage until the app provides the remaining params (the modifier at vkBindImageMemory2). This causes a cascade of problems with other parts of the Vulkan api.

I guess the analogue to option 2 in Linux would be: create a vkImage with a list of all possible modifiers, and let the driver choose one; then at memory bind time, we check if the incoming modifier matches the one the image is created with, reject if not.

I don't recommend this method if you can avoid it. But if you must do it, then the app must not only query the VkImage's modifier with vkGetImageDrmFormatModifierPropertiesEXT. It must also query the row pitch and offset for each of the image's "memory planes" with vkGetImageSubresourceLayout2(aspectMask=VK_IMAGE_ASPECT_MEMORY_PLANE_{0,1,2,3}_BIT_EXT), for each memory plane that the modifier requires, and confirm that the plane layouts in the VkImage matches the image inside the imported dma-buf/win32 handle. Note that "memory plane" here is distinct than "format plane" (such as YUV planes). The memory plane count must be queried with vkGetPhysicalDeviceFormatProperties2(pNext=VkDrmFormatModifierPropertiesListEXT).

@linyaa-kiwi
Copy link
Contributor

I hope that you're able to find a good implementation of VK_KHR_external_memory_win32 for Wine. If you describe in detail the import-ordering constraints that Wine must support, then maybe me and James can provide more advice.

@linyaa-kiwi linyaa-kiwi self-assigned this Aug 21, 2024
@yshui
Copy link
Author

yshui commented Aug 21, 2024

Thanks for the answer!

If you describe in detail the import-ordering constraints that Wine must support.

Hmm, I am unsure what information I need to provide? The problem is just when a win32 program creates a VkImage, wine as the implementor of VK_KHR_external_memory_win32 has no way to know the drm modifier of the memory that will be later imported.

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

4 participants