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

Initial Support for DRM interface in vulkan backend #5908

Draft
wants to merge 6 commits into
base: trunk
Choose a base branch
from

Conversation

morr0ne
Copy link

@morr0ne morr0ne commented Jul 4, 2024

This pull request introduces initial support for the Linux DRM interface in wgpu. This enables wgpu to be utilized in environments where a windowing system is unavailable and, more importantly, allows wgpu to function as a renderer for Wayland compositors.

For additional context, please refer to issue #1674.

The implementation is straightforward, though there are known limitations due to my limited familiarity with wgpu internals:

  • Physical device selection is mostly hardcoded, as the method to dynamically select the device used by wgpu is unclear to me.
  • Validation and error handling are lacking, making the code prone to crashes.
  • There is no mechanism to guide wgpu in selecting the appropriate display mode, defaulting to the first available mode, which may not be the correct one.

Despite these issues, the code functions, and I demonstrated this by porting the hello_triangle example, available here: https://github.com/verdiwm/diretto/blob/main/examples/wgpu.rs

I also created a fork of raw-window-handle, adding the necessary connector_id for display initialization.

While this pull request is not ready for merging, I am submitting it to illustrate the feasibility of DRM support and to receive feedback from the wgpu development team.

@teoxoy
Copy link
Member

teoxoy commented Jul 5, 2024

Looking at those Vulkan functions, it seems important to know what VkPhysicalDevice the surface belongs to. I think we need a new surface creation function that users of this API can pass a wgpu Adapter (VkPhysicalDevice) to + any other hints to select the display mode.

@cwfitzgerald
Copy link
Member

I don't think we need a new function, just another variant in the unsafe enum, in which we can have an adapter passed.

@notgull
Copy link

notgull commented Jul 5, 2024

I also created a fork of raw-window-handle, adding the necessary connector_id for display initialization.

Please open a PR against the raw-window-handle repo so we can discuss this in greater depth.

@morr0ne
Copy link
Author

morr0ne commented Jul 6, 2024

I don't think we need a new function, just another variant in the unsafe enum, in which we can have an adapter passed.

I think it would be more appropriate to pass a device rather than an adapter since the actual device isn't created until request_device is called. Would it also be logical to pass mode information here? I’m still figuring out what’s needed to choose the correct mode, but it’s clear that some sort of information must be provided.

I the case of the device, it is also unclear to me how to obtain the raw vkPhysicalDevice from the generic Device struct

Please open a PR against the raw-window-handle repo so we can discuss this in greater depth.

I've opened a PR here: rust-windowing/raw-window-handle#170.

@MarijnS95
Copy link
Contributor

Physical device selection is mostly hardcoded, as the method to dynamically select the device used by wgpu is unclear to me.

The passed File Descriptor should be the opened /dev/dri/cardX character device, to which a /dev/char/{major}:{minor} symlink exists. These correspond to the major and minor in VK_EXT_physical_device_drm's VkPhysicalDeviceDrmPropertiesEXT struct, to help you associate the right physical device.
(Do keep in mind that multiple Vulkan drivers might exist for the same DRM device, such as AMDVLK and RADV)


Note that you can already enumerate the available VkDisplayKHRs directly via the VK_KHR_display extension, for a given physical device. Furthermore, the EXT extensions required here are described to mostly be for HMDs (VR headsets) to get exclusive access (while other connectors are still owned by a running compositor afaik). I think this implementation should get away with only using VK_KHR_display (at least by default, but do read on!), which also doesn't require/allow you to use the connnector ID (and gets rid of the raw-window-handle patch and dependency), as you anyway already need to build out API to select a pipe/plane and mode.

While VK_KHR_display in design looks very similar to DRM/KMS, it doesn't provide any interop by itself so that this extension can be supported regardless of the host OS supporting DRM. However, it seems the VK_KHR_get_display_properties2 extension was built specifically to query extra information when enumerating VkDisplayKHR, such as an extension structure containing the DRM connector ID for that display... but in the current vk.xml no structure exists that extends VkDisplayProperties2KHR just yet.


Regarding EXT extensions, on my Linux machine with an AMD card two drivers are already behaving differently. On the AMDVLK driver vkGetDrmDisplayEXT() always returns ERROR_EXTENSION_NOT_PRESENT, but vkGetPhysicalDeviceDisplayPropertiesKHR() returns an array with a VkDisplayKHR handle.
On the Mesa RADV driver the opposite is true: vkGetDrmDisplayEXT() returns a VkDisplayKHR handle (for every connector ID, even unconnected ones), but vkGetPhysicalDeviceDisplayPropertiesKHR() always returns an empty list. So a fallback to the EXT might be desired to at least support RADV (but shouldn't always be required)?

Comment on lines +586 to +589
if drm_props.primary_major == major.into() && drm_props.primary_minor == minor.into() {
physical_device = Some(device);

break;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned before there can be >1 physical device for which this is true.

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

Successfully merging this pull request may close these issues.

5 participants