[PATCH 0/1] MR164: winex11.drv: Avoid adding duplicate GPUs.

[Paul Gofman (＠gofman)]

On certain dual GPU configurations (AMD integrated / discrete GPU laptop in my case) vkGetRandROutputDisplayEXT() doesn't work the way we currently expect. That is, it returns success and a valid VkDisplay handler for any randr provider and GPU combination. The configuration ends up broken in various aspects, the same GPU gets added twice with identical parameters while another GPU is not represented at all.

I think it is not a Vulkan or xrandr bug, this behaviour probably reflects the fact that one can present from any of Vulkan devices to the displays plugged to any of those outputs.

The patch avoids adding the device twice and also another device gets represented as well. The relation between the added GPUs and monitors ends up being somewhat arbitrary but then it doesn't affect much in the case when displays are accessible though any of adapters.

The alternative to consider would be to have a completely separate path to be taken if Vulkan is available which instead of attaching Vulkan device info to xrandr-driven GPUs list would have GPU device spawned by Vulkan device and use xrandr to link the adapters and displays to GPUs. This way it would also help Wayland where we don't have real xrandr providers (only one WAYLAND provider with all the displays) and end up without creating proper GPU devices.

[PATCH 1/1] winex11.drv: Avoid adding duplicate GPUs.

[Paul Gofman]

From: Paul Gofman

Signed-off-by: Paul Gofman 
---
 dlls/winex11.drv/xrandr.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/dlls/winex11.drv/xrandr.c b/dlls/winex11.drv/xrandr.c
index 3fa6bad56c1..d3b96e3170b 100644
--- a/dlls/winex11.drv/xrandr.c
+++ b/dlls/winex11.drv/xrandr.c
@@ -633,7 +633,8 @@ static BOOL is_crtc_primary( RECT primary, const XRRCrtcInfo *crtc )

VK_DEFINE_NON_DISPATCHABLE_HANDLE(VkDisplayKHR)

-static BOOL get_gpu_properties_from_vulkan( struct gdi_gpu *gpu, const XRRProviderInfo *provider_info )
+static BOOL get_gpu_properties_from_vulkan( struct gdi_gpu *gpu, const XRRProviderInfo *provider_info,
+                                            struct gdi_gpu *prev_gpus, int prev_gpu_count )
 {
     static const char *extensions[] =
     {
@@ -648,7 +649,7 @@ static BOOL get_gpu_properties_from_vulkan( struct gdi_gpu *gpu, const XRRProvid
     VkResult (*pvkGetRandROutputDisplayEXT)( VkPhysicalDevice, Display *, RROutput, VkDisplayKHR * );
     PFN_vkGetPhysicalDeviceProperties2KHR pvkGetPhysicalDeviceProperties2KHR;
     PFN_vkEnumeratePhysicalDevices pvkEnumeratePhysicalDevices;
-    uint32_t device_count, device_idx, output_idx;
+    uint32_t device_count, device_idx, output_idx, i;
     VkPhysicalDevice *vk_physical_devices = NULL;
     VkPhysicalDeviceProperties2 properties2;
     VkInstanceCreateInfo create_info;
@@ -703,6 +704,8 @@ static BOOL get_gpu_properties_from_vulkan( struct gdi_gpu *gpu, const XRRProvid
         goto done;
     }

+    TRACE("provider name %s.\n", debugstr_a(provider_info->name));
+
     for (device_idx = 0; device_idx < device_count; ++device_idx)
     {
         for (output_idx = 0; output_idx < provider_info->noutputs; ++output_idx)
@@ -720,7 +723,19 @@ static BOOL get_gpu_properties_from_vulkan( struct gdi_gpu *gpu, const XRRProvid
             properties2.pNext = &id;

pvkGetPhysicalDeviceProperties2KHR( vk_physical_devices[device_idx], &properties2 );
+            for (i = 0; i < prev_gpu_count; ++i)
+            {
+                if (!memcmp( &prev_gpus[i].vulkan_uuid, &id.deviceUUID, sizeof(id.deviceUUID) ))
+                {
+                    WARN( "device UUID %#x:%#x already assigned to GPU %u.\n", *((uint32_t *)id.deviceUUID + 1),
+                          *(uint32_t *)id.deviceUUID, i );
+                    break;
+                }
+            }
+            if (i < prev_gpu_count) continue;
+
             memcpy( &gpu->vulkan_uuid, id.deviceUUID, sizeof(id.deviceUUID) );
+
             /* Ignore Khronos vendor IDs */
             if (properties2.properties.vendorID < 0x10000)
             {
@@ -808,7 +823,7 @@ static BOOL xrandr14_get_gpus2( struct gdi_gpu **new_gpus, int *count, BOOL get_
         gpus[i].id = provider_resources->providers[i];
         if (get_properties)
         {
-            if (!get_gpu_properties_from_vulkan( &gpus[i], provider_info ))
+            if (!get_gpu_properties_from_vulkan( &gpus[i], provider_info, gpus, i ))
                 RtlUTF8ToUnicodeN( gpus[i].name, sizeof(gpus[i].name), &len, provider_info->name,
                                    strlen( provider_info->name ) + 1 );
             /* FIXME: Add an alternate method of getting PCI IDs, for systems that don't support Vulkan */

[Zhiyi Zhang (＠zhiyi)]

> The alternative to consider would be to have a completely separate path to be taken if Vulkan is available which instead of attaching Vulkan device info to xrandr-driven GPUs list would have GPU device spawned by Vulkan device and use xrandr to link the adapters and displays to GPUs. This way it would also help Wayland where we don't have real xrandr providers (only one WAYLAND provider with all the displays) and end up without creating proper GPU devices.

It won't work if there are multiple GPUs and one of them doesn't support Vulkan.

[PATCH 0/1] MR164: winex11.drv: Avoid adding duplicate GPUs. - approved

[Zhiyi Zhang (＠zhiyi)]

This merge request was approved by Zhiyi Zhang.