Avoid taking the display_device_init mutex outside of win32u.
Similar to https://gitlab.winehq.org/wine/wine/-/merge_requests/5786, although this one doesn't cause any problems as far as I know. Still makes things simpler.
From: Rémi Bernon rbernon@codeweavers.com
Avoid taking the display_device_init mutex outside of win32u. --- dlls/win32u/sysparams.c | 23 +++++++++++++ dlls/winevulkan/Makefile.in | 2 +- dlls/winevulkan/loader.c | 68 +++++++------------------------------ include/ntuser.h | 6 ++++ 4 files changed, 43 insertions(+), 56 deletions(-)
diff --git a/dlls/win32u/sysparams.c b/dlls/win32u/sysparams.c index 49bf26214e7..9e51784f8e9 100644 --- a/dlls/win32u/sysparams.c +++ b/dlls/win32u/sysparams.c @@ -2370,6 +2370,26 @@ RECT get_primary_monitor_rect( UINT dpi ) return map_dpi_rect( rect, system_dpi, dpi ); }
+static BOOL get_luid_from_vulkan_uuid( const GUID *uuid, LUID *luid ) +{ + BOOL found = FALSE; + struct gpu *gpu; + + if (!lock_display_devices()) return FALSE; + + LIST_FOR_EACH_ENTRY( gpu, &gpus, struct gpu, entry ) + { + if ((found = !memcmp( &gpu->vulkan_uuid, uuid, sizeof(*uuid) ))) + { + *luid = gpu->luid; + break; + } + } + + unlock_display_devices(); + return found; +} + /********************************************************************** * NtUserGetDisplayConfigBufferSizes (win32u.@) */ @@ -6448,6 +6468,9 @@ ULONG_PTR WINAPI NtUserCallTwoParam( ULONG_PTR arg1, ULONG_PTR arg2, ULONG code case NtUserCallTwoParam_UnhookWindowsHook: return unhook_windows_hook( arg1, (HOOKPROC)arg2 );
+ case NtUserCallTwoParam_GetLUIDFromVulkanUUID: + return get_luid_from_vulkan_uuid( (GUID *)arg1, (LUID *)arg2 ); + /* temporary exports */ case NtUserAllocWinProc: return (UINT_PTR)alloc_winproc( (WNDPROC)arg1, arg2 ); diff --git a/dlls/winevulkan/Makefile.in b/dlls/winevulkan/Makefile.in index 52bffca2843..55344192373 100644 --- a/dlls/winevulkan/Makefile.in +++ b/dlls/winevulkan/Makefile.in @@ -1,7 +1,7 @@ MODULE = winevulkan.dll UNIXLIB = winevulkan.so IMPORTLIB = winevulkan -IMPORTS = user32 gdi32 advapi32 setupapi win32u +IMPORTS = user32 gdi32 advapi32 win32u UNIX_LIBS = -lwin32u $(PTHREAD_LIBS)
SOURCES = \ diff --git a/dlls/winevulkan/loader.c b/dlls/winevulkan/loader.c index bd49b08057d..088710224aa 100644 --- a/dlls/winevulkan/loader.c +++ b/dlls/winevulkan/loader.c @@ -339,20 +339,6 @@ VkResult WINAPI vkEnumerateInstanceVersion(uint32_t *version) return params.result; }
-static HANDLE get_display_device_init_mutex(void) -{ - HANDLE mutex = CreateMutexW(NULL, FALSE, L"display_device_init"); - - WaitForSingleObject(mutex, INFINITE); - return mutex; -} - -static void release_display_device_init_mutex(HANDLE mutex) -{ - ReleaseMutex(mutex); - CloseHandle(mutex); -} - /* Wait until graphics driver is loaded by explorer */ static void wait_graphics_driver_ready(void) { @@ -368,15 +354,7 @@ static void wait_graphics_driver_ready(void) static void fill_luid_property(VkPhysicalDeviceProperties2 *properties2) { VkPhysicalDeviceVulkan11Properties *vk11; - VkBool32 device_luid_valid = VK_FALSE; VkPhysicalDeviceIDProperties *id; - uint32_t device_node_mask = 0; - SP_DEVINFO_DATA device_data; - const uint8_t* device_uuid; - DWORD type, device_idx = 0; - HDEVINFO devinfo; - HANDLE mutex; - GUID uuid; LUID luid;
vk11 = wine_vk_find_struct(properties2, PHYSICAL_DEVICE_VULKAN_1_1_PROPERTIES); @@ -386,48 +364,28 @@ static void fill_luid_property(VkPhysicalDeviceProperties2 *properties2) return;
wait_graphics_driver_ready(); - mutex = get_display_device_init_mutex(); - devinfo = SetupDiGetClassDevsW(&GUID_DEVCLASS_DISPLAY, L"PCI", NULL, 0); - device_data.cbSize = sizeof(device_data); - while (SetupDiEnumDeviceInfo(devinfo, device_idx++, &device_data)) - { - if (!SetupDiGetDevicePropertyW(devinfo, &device_data, &WINE_DEVPROPKEY_GPU_VULKAN_UUID, - &type, (BYTE *)&uuid, sizeof(uuid), NULL, 0)) - continue; - - device_uuid = id ? id->deviceUUID : vk11->deviceUUID;
- if (!IsEqualGUID(&uuid, device_uuid)) - continue; - - if (SetupDiGetDevicePropertyW(devinfo, &device_data, &DEVPROPKEY_GPU_LUID, &type, - (BYTE *)&luid, sizeof(luid), NULL, 0)) - { - device_luid_valid = VK_TRUE; - device_node_mask = 1; - break; - } - } - SetupDiDestroyDeviceInfoList(devinfo); - release_display_device_init_mutex(mutex); + TRACE("deviceName %s\n", properties2->properties.deviceName);
if (id) { - if (device_luid_valid) memcpy(&id->deviceLUID, &luid, sizeof(id->deviceLUID)); - id->deviceLUIDValid = device_luid_valid; - id->deviceNodeMask = device_node_mask; + id->deviceLUIDValid = NtUserGetLUIDFromVulkanUUID((GUID *)id->deviceUUID, &luid); + memcpy(id->deviceLUID, &luid, sizeof(luid)); + id->deviceNodeMask = !!id->deviceLUIDValid; + + TRACE("id %p deviceLUIDValid %d GUID %s LUID %08lx:%08lx\n", id, id->deviceLUIDValid, + debugstr_guid((GUID *)id->deviceUUID), luid.HighPart, luid.LowPart); }
if (vk11) { - if (device_luid_valid) memcpy(&vk11->deviceLUID, &luid, sizeof(vk11->deviceLUID)); - vk11->deviceLUIDValid = device_luid_valid; - vk11->deviceNodeMask = device_node_mask; - } + vk11->deviceLUIDValid = NtUserGetLUIDFromVulkanUUID((GUID *)vk11->deviceUUID, &luid); + memcpy(vk11->deviceLUID, &luid, sizeof(luid)); + vk11->deviceNodeMask = !!vk11->deviceLUIDValid;
- TRACE("deviceName:%s deviceLUIDValid:%d LUID:%08lx:%08lx deviceNodeMask:%#x.\n", - properties2->properties.deviceName, device_luid_valid, luid.HighPart, luid.LowPart, - device_node_mask); + TRACE("vk11 %p deviceLUIDValid %d GUID %s LUID %08lx:%08lx\n", vk11, vk11->deviceLUIDValid, + debugstr_guid((GUID *)vk11->deviceUUID), luid.HighPart, luid.LowPart); + } }
void WINAPI vkGetPhysicalDeviceProperties2(VkPhysicalDevice phys_dev, diff --git a/include/ntuser.h b/include/ntuser.h index ff45ffa2bc8..0fa2e5f6510 100644 --- a/include/ntuser.h +++ b/include/ntuser.h @@ -1045,6 +1045,7 @@ enum NtUserCallTwoParam_SetCaretPos, NtUserCallTwoParam_SetIconParam, NtUserCallTwoParam_UnhookWindowsHook, + NtUserCallTwoParam_GetLUIDFromVulkanUUID, /* temporary exports */ NtUserAllocWinProc, }; @@ -1092,6 +1093,11 @@ static inline BOOL NtUserUnhookWindowsHook( INT id, HOOKPROC proc ) return NtUserCallTwoParam( id, (UINT_PTR)proc, NtUserCallTwoParam_UnhookWindowsHook ); }
+static inline BOOL NtUserGetLUIDFromVulkanUUID( const GUID *uuid, LUID *luid ) +{ + return NtUserCallTwoParam( (UINT_PTR)uuid, (UINT_PTR)luid, NtUserCallTwoParam_GetLUIDFromVulkanUUID ); +} + /* NtUserCallHwnd codes, not compatible with Windows */ enum {
I think we should go one step further, use generated PE thunk and implement the whole `vkGetPhysicalDeviceProperties2` on Unix side. Maybe we could just forward it with `vulkan_funcs` and implement in win32u?
Ah yeah good idea, I'll have a try.
There's actually several issues with it:
1) First is related to how win32u vulkan driver is used a bit elsewhere, from winex11 XRandR and from d3dkmt. They use `vkGetPhysicalDeviceProperties2` and hooking it in win32u creates a cycle which requires a bit of refactor to break up. It's not too bad and it could actually be a good thing to do... however:
2) `vkGetPhysicalDeviceProperties2` is an instance function and needs to be queried with an instance. We don't have a way to hook such functions and dispatch it to the right instance in our win32u interface. This is a bigger problem and I don't see an easy way to solve it.
I see, that would require more hooking. Then maybe we could have it in winevulkan.so and use driver_funcs only for the mapping (like `NtUserGetLUIDFromVulkanUUID` from your patch).
This merge request was closed by Rémi Bernon.
-
Jacek Caban (＠jacek) -
Rémi Bernon
-
Rémi Bernon (＠rbernon)