From: Paul Gofman pgofman@codeweavers.com
--- dlls/winex11.drv/xrandr.c | 61 ++++++++++++++++++++++++++++----------- 1 file changed, 44 insertions(+), 17 deletions(-)
diff --git a/dlls/winex11.drv/xrandr.c b/dlls/winex11.drv/xrandr.c index 48b579e87e3..d4c9e87be3b 100644 --- a/dlls/winex11.drv/xrandr.c +++ b/dlls/winex11.drv/xrandr.c @@ -714,13 +714,32 @@ static BOOL is_crtc_primary( RECT primary, const XRRCrtcInfo *crtc ) crtc->y + crtc->height == primary.bottom; }
+struct vk_physdev_info +{ + VkPhysicalDevice physdev; + VkPhysicalDeviceProperties2 properties2; + VkPhysicalDeviceIDProperties id; +}; + +static int compare_vulkan_physical_devices( const void *v1, const void *v2 ) +{ + static const int device_type_rank[6] = { 100, 1, 0, 2, 3, 200 }; + const struct vk_physdev_info *d1 = v1, *d2 = v2; + int rank1, rank2; + + rank1 = device_type_rank[ min( d1->properties2.properties.deviceType, ARRAY_SIZE(device_type_rank) - 1) ]; + rank2 = device_type_rank[ min( d2->properties2.properties.deviceType, ARRAY_SIZE(device_type_rank) - 1) ]; + if (rank1 != rank2) return rank1 - rank2; + + return memcmp( &d1->id.deviceUUID, &d2->id.deviceUUID, sizeof(d1->id.deviceUUID) ); +} + static BOOL get_gpu_properties_from_vulkan( struct x11drv_gpu *gpu, const XRRProviderInfo *provider_info, struct x11drv_gpu *prev_gpus, int prev_gpu_count ) { uint32_t device_count, device_idx, output_idx, i; VkPhysicalDevice *vk_physical_devices = NULL; - VkPhysicalDeviceProperties2 properties2; - VkPhysicalDeviceIDProperties id; + struct vk_physdev_info *devs = NULL; VkDisplayKHR vk_display; BOOL ret = FALSE; VkResult vr; @@ -744,43 +763,50 @@ static BOOL get_gpu_properties_from_vulkan( struct x11drv_gpu *gpu, const XRRPro goto done; }
+ if (!(devs = calloc( device_count, sizeof(*devs) ))) + goto done; + + for (device_idx = 0; device_idx < device_count; ++device_idx) + { + devs[device_idx].physdev = vk_physical_devices[device_idx]; + devs[device_idx].id.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_ID_PROPERTIES; + devs[device_idx].properties2.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_PROPERTIES_2; + devs[device_idx].properties2.pNext = &devs[device_idx].id; + p_vkGetPhysicalDeviceProperties2KHR( vk_physical_devices[device_idx], &devs[device_idx].properties2 ); + } + qsort( devs, device_count, sizeof(*devs), compare_vulkan_physical_devices ); + 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) { - vr = p_vkGetRandROutputDisplayEXT( vk_physical_devices[device_idx], gdi_display, + vr = p_vkGetRandROutputDisplayEXT( devs[device_idx].physdev, gdi_display, provider_info->outputs[output_idx], &vk_display ); if (vr != VK_SUCCESS || vk_display == VK_NULL_HANDLE) continue;
- memset( &id, 0, sizeof(id) ); - id.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_ID_PROPERTIES; - properties2.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_PROPERTIES_2; - properties2.pNext = &id; - - p_vkGetPhysicalDeviceProperties2KHR( 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) )) + if (!memcmp( &prev_gpus[i].vulkan_uuid, &devs[device_idx].id.deviceUUID, sizeof(devs[device_idx].id.deviceUUID) )) { - WARN( "device UUID %#x:%#x already assigned to GPU %u.\n", *((uint32_t *)id.deviceUUID + 1), - *(uint32_t *)id.deviceUUID, i ); + WARN( "device UUID %#x:%#x already assigned to GPU %u.\n", *((uint32_t *)devs[device_idx].id.deviceUUID + 1), + *(uint32_t *)devs[device_idx].id.deviceUUID, i ); break; } } if (i < prev_gpu_count) continue;
- memcpy( &gpu->vulkan_uuid, id.deviceUUID, sizeof(id.deviceUUID) ); + memcpy( &gpu->vulkan_uuid, devs[device_idx].id.deviceUUID, sizeof(devs[device_idx].id.deviceUUID) );
/* Ignore Khronos vendor IDs */ - if (properties2.properties.vendorID < 0x10000) + if (devs[device_idx].properties2.properties.vendorID < 0x10000) { - gpu->pci_id.vendor = properties2.properties.vendorID; - gpu->pci_id.device = properties2.properties.deviceID; + gpu->pci_id.vendor = devs[device_idx].properties2.properties.vendorID; + gpu->pci_id.device = devs[device_idx].properties2.properties.deviceID; } - gpu->name = strdup( properties2.properties.deviceName ); + gpu->name = strdup( devs[device_idx].properties2.properties.deviceName );
ret = TRUE; goto done; @@ -788,6 +814,7 @@ static BOOL get_gpu_properties_from_vulkan( struct x11drv_gpu *gpu, const XRRPro }
done: + free( devs ); free( vk_physical_devices ); return ret; }
From: Paul Gofman pgofman@codeweavers.com
--- dlls/win32u/d3dkmt.c | 56 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 44 insertions(+), 12 deletions(-)
diff --git a/dlls/win32u/d3dkmt.c b/dlls/win32u/d3dkmt.c index 564b10cbe98..a6ec70d5ac4 100644 --- a/dlls/win32u/d3dkmt.c +++ b/dlls/win32u/d3dkmt.c @@ -586,38 +586,70 @@ NTSTATUS WINAPI NtGdiDdDDICheckVidPnExclusiveOwnership( const D3DKMT_CHECKVIDPNE return STATUS_SUCCESS; }
+struct vk_physdev_info +{ + VkPhysicalDeviceProperties2 properties2; + VkPhysicalDeviceIDProperties id; + VkPhysicalDeviceMemoryProperties mem_properties; +}; + +static int compare_vulkan_physical_devices( const void *v1, const void *v2 ) +{ + static const int device_type_rank[6] = { 100, 1, 0, 2, 3, 200 }; + const struct vk_physdev_info *d1 = v1, *d2 = v2; + int rank1, rank2; + + rank1 = device_type_rank[ min( d1->properties2.properties.deviceType, ARRAY_SIZE(device_type_rank) - 1) ]; + rank2 = device_type_rank[ min( d2->properties2.properties.deviceType, ARRAY_SIZE(device_type_rank) - 1) ]; + if (rank1 != rank2) return rank1 - rank2; + + return memcmp( &d1->id.deviceUUID, &d2->id.deviceUUID, sizeof(d1->id.deviceUUID) ); +} + BOOL get_vulkan_gpus( struct list *gpus ) { + struct vk_physdev_info *devinfo; VkPhysicalDevice *devices; UINT i, j, count;
if (!d3dkmt_use_vulkan()) return FALSE; if (!(count = get_vulkan_physical_devices( &devices ))) return FALSE;
+ if (!(devinfo = calloc( count, sizeof(*devinfo) ))) + { + free( devices ); + return FALSE; + } + for (i = 0; i < count; ++i) + { + devinfo[i].id.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_ID_PROPERTIES; + devinfo[i].properties2.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_PROPERTIES_2; + devinfo[i].properties2.pNext = &devinfo[i].id; + pvkGetPhysicalDeviceProperties2KHR( devices[i], &devinfo[i].properties2 ); + pvkGetPhysicalDeviceMemoryProperties( devices[i], &devinfo[i].mem_properties ); + } + qsort( devinfo, count, sizeof(*devinfo), compare_vulkan_physical_devices ); + for (i = 0; i < count; ++i) { - VkPhysicalDeviceIDProperties id = {.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_ID_PROPERTIES}; - VkPhysicalDeviceProperties2 properties2 = {.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_PROPERTIES_2, .pNext = &id}; - VkPhysicalDeviceMemoryProperties mem_properties; struct vulkan_gpu *gpu;
if (!(gpu = calloc( 1, sizeof(*gpu) ))) break; - pvkGetPhysicalDeviceProperties2KHR( devices[i], &properties2 ); - memcpy( &gpu->uuid, id.deviceUUID, sizeof(gpu->uuid) ); - gpu->name = strdup( properties2.properties.deviceName ); - gpu->pci_id.vendor = properties2.properties.vendorID; - gpu->pci_id.device = properties2.properties.deviceID; + memcpy( &gpu->uuid, devinfo[i].id.deviceUUID, sizeof(gpu->uuid) ); + gpu->name = strdup( devinfo[i].properties2.properties.deviceName ); + gpu->pci_id.vendor = devinfo[i].properties2.properties.vendorID; + gpu->pci_id.device = devinfo[i].properties2.properties.deviceID;
- pvkGetPhysicalDeviceMemoryProperties( devices[i], &mem_properties ); - for (j = 0; j < mem_properties.memoryHeapCount; j++) + for (j = 0; j < devinfo[i].mem_properties.memoryHeapCount; j++) { - if (mem_properties.memoryHeaps[j].flags & VK_MEMORY_HEAP_DEVICE_LOCAL_BIT) - gpu->memory += mem_properties.memoryHeaps[j].size; + if (devinfo[i].mem_properties.memoryHeaps[j].flags & VK_MEMORY_HEAP_DEVICE_LOCAL_BIT) + gpu->memory += devinfo[i].mem_properties.memoryHeaps[j].size; }
list_add_tail( gpus, &gpu->entry ); }
+ free( devinfo ); free( devices ); return TRUE; }
The sort order of physical devices returned from vkEnumeratePhysicalDevices() is undefined (without explicitly added instance layers like vk_layer_mesa_device_select). It is inconsistent in practice and differs between different Wine processes (e. g., based on some random factors I get different order on a dual AMD GPU laptop).
For add_gpu() (in win32u/sysparams.c) the order is important. The incoming GPU order defines gpu->index which is a part of GPU registry path. So when identical GPUs are enumerated in driver in a different order that is effectively results as deleting both GPUs and creating new ones, leading to various inconsistence, e. g. (but not limited to), just setting display mode may be lost and reset to default during enumeration with forced GPU requery from driver. This is addressed by the first patch. In theory this part can maybe be addressed another way, by explicitly setting device index in some other way, but that looks much more complicated with unclear benefits.
Then, when Vulkan devices are not associated with Vulkan GPUs (happens whenever xrandr is essentially broken, always the case on Xwayland), add_gpu() depends on the consistent order in its own Vulkan GPU enumeration (addressed by the second patch). Since this path is essentially a workaround which relies solely on Vulkan devices sort order to be reproducible between enumerations and consistent between processes it looks like sorting is the only way.
WRT this particular problem sorting could be done without introducing ordering by GPU type (discrete / integrated / software) only using UUID but it looks better to me to have that as an explicit part of the sort. In particular, software devices happen to go to the end of the list on Linux and Windows, making them accidentally go first may confuse apps.
Also WRT patch 1, while vkGetRandROutputDisplayEXT call is supposed to associate Vulkan GPU to xrandr output, even when xrandr is not broken the association may be not unique on dual GPU laptops: the same output can relate to both discrete and integrated GPU (reflecting the reality in this case). We are going to associate the output with one GPU, so the order of physical devices matters in this case too so we always end up with the same association (also preferring discrete GPU).
This merge request was approved by Rémi Bernon.