[PATCH v3 0/3] MR2449: win32u: Add a few device GPU properties.
Diablo IV (closed beta) depends on DEVPKEY_Device_MatchingDeviceId being present for GPUs (once it is able to match GPU luid to the one obtained from dxgi). It also depends on the MatchingDeviceId conating "ven_" and "dev_" substrings. Without those it doesn't consider the gpu as valid, which currently results in the game complaining about no GPUs found. I guess that maybe it is trying to filter out software GPUs and such this way. As far as my testing goes, DEVPKEY_Device_MatchingDeviceId has this form with hardware GPUs only and maybe with the recent enough Win10 or Win11. The other two device properties being added are also queried by the game although they are not strictly needed for it. -- v3: win32u: Set DEVPKEY_Device_RemovalPolicy for GPUs. win32u: Set DEVPKEY_Device_BusNumber for GPUs. win32u: Set DEVPKEY_Device_MatchingDeviceId for GPUs. https://gitlab.winehq.org/wine/wine/-/merge_requests/2449
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/gdi32/tests/driver.c | 46 +++++++++++++++++++++++++++++++++++++++ dlls/win32u/sysparams.c | 19 ++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/dlls/gdi32/tests/driver.c b/dlls/gdi32/tests/driver.c index 94f93d61207..6771c7c8247 100644 --- a/dlls/gdi32/tests/driver.c +++ b/dlls/gdi32/tests/driver.c @@ -32,6 +32,7 @@ #include "initguid.h" #include "setupapi.h" #include "ntddvdeo.h" +#include "devpkey.h" #include "wine/test.h" @@ -996,6 +997,45 @@ static void test_D3DKMTQueryVideoMemoryInfo(void) ok(status == STATUS_SUCCESS, "Got unexpected return code %#lx.\n", status); } +static void test_gpu_device_properties(const GUID *devinterface_guid) +{ + BYTE iface_detail_buffer[sizeof(SP_DEVICE_INTERFACE_DETAIL_DATA_W) + 256 * sizeof(WCHAR)]; + SP_DEVINFO_DATA device_data = {sizeof(device_data)}; + SP_DEVICE_INTERFACE_DATA iface = {sizeof(iface)}; + SP_DEVICE_INTERFACE_DETAIL_DATA_W *iface_data; + WCHAR device_id[256]; + DEVPROPTYPE type; + unsigned int i; + HDEVINFO set; + BOOL ret; + + /* Make sure display devices are initialized. */ + SendMessageW(GetDesktopWindow(), WM_NULL, 0, 0); + + set = SetupDiGetClassDevsW(devinterface_guid, NULL, NULL, DIGCF_DEVICEINTERFACE | DIGCF_PRESENT); + ok(set != INVALID_HANDLE_VALUE, "SetupDiGetClassDevs failed, error %lu.\n", GetLastError()); + + iface_data = (SP_DEVICE_INTERFACE_DETAIL_DATA_W *)iface_detail_buffer; + iface_data->cbSize = sizeof(*iface_data); + + i = 0; + while (SetupDiEnumDeviceInterfaces(set, NULL, devinterface_guid, i, &iface)) + { + ret = SetupDiGetDeviceInterfaceDetailW(set, &iface, iface_data, + sizeof(iface_detail_buffer), NULL, &device_data ); + ok(ret, "Got unexpected ret %d, GetLastError() %lu.\n", ret, GetLastError()); + + ret = SetupDiGetDevicePropertyW(set, &device_data, &DEVPKEY_Device_MatchingDeviceId, &type, + (BYTE *)device_id, sizeof(device_id), NULL, 0); + ok(ret, "Got unexpected ret %d, GetLastError() %lu.\n", ret, GetLastError()); + ok(type == DEVPROP_TYPE_STRING, "Got type %ld.\n", type); + + ++i; + } + SetupDiDestroyDeviceInfoList( set ); +} + + START_TEST(driver) { HMODULE gdi32 = GetModuleHandleA("gdi32.dll"); @@ -1025,6 +1065,12 @@ START_TEST(driver) test_D3DKMTCheckOcclusion(); test_D3DKMTOpenAdapterFromDeviceName(); test_D3DKMTQueryVideoMemoryInfo(); + winetest_push_context("GUID_DEVINTERFACE_DISPLAY_ADAPTER"); + test_gpu_device_properties(&GUID_DEVINTERFACE_DISPLAY_ADAPTER); + winetest_pop_context(); + winetest_push_context("GUID_DISPLAY_DEVICE_ARRIVAL"); + test_gpu_device_properties(&GUID_DISPLAY_DEVICE_ARRIVAL); + winetest_pop_context(); FreeLibrary(dwmapi); } diff --git a/dlls/win32u/sysparams.c b/dlls/win32u/sysparams.c index 60b12321953..a02158a17a0 100644 --- a/dlls/win32u/sysparams.c +++ b/dlls/win32u/sysparams.c @@ -93,6 +93,14 @@ static const WCHAR devpropkey_gpu_luidW[] = '\\','0','0','0','2' }; +static const WCHAR devpkey_device_matching_device_id[] = +{ + 'P','r','o','p','e','r','t','i','e','s', + '\\','{','A','8','B','8','6','5','D','D','-','2','E','3','D','-','4','0','9','4', + '-','A','D','9','7','-','E','5','9','3','A','7','0','C','7','5','D','6','}', + '\\','0','0','0','8' +}; + static const WCHAR devpropkey_device_ispresentW[] = { 'P','r','o','p','e','r','t','i','e','s', @@ -1218,6 +1226,17 @@ static void add_gpu( const struct gdi_gpu *gpu, void *param ) bufferW[size / sizeof(WCHAR)] = 0; /* for REG_MULTI_SZ */ set_reg_value( hkey, hardware_idW, REG_MULTI_SZ, bufferW, size + sizeof(WCHAR) ); + if ((subkey = reg_create_key( hkey, devpkey_device_matching_device_id, + sizeof(devpkey_device_matching_device_id), 0, NULL ))) + { + if (gpu->vendor_id && gpu->device_id) + set_reg_value( subkey, NULL, 0xffff0000 | DEVPROP_TYPE_STRING, bufferW, size ); + else + set_reg_value( subkey, NULL, 0xffff0000 | DEVPROP_TYPE_STRING, bufferW, + asciiz_to_unicode( bufferW, "ROOT\\BasicRender" )); + NtClose( subkey ); + } + desc = gpu->name; if (!desc[0]) desc = wine_adapterW; set_reg_value( hkey, device_descW, REG_SZ, desc, (lstrlenW( desc ) + 1) * sizeof(WCHAR) ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2449
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/gdi32/tests/driver.c | 12 ++++++++++++ dlls/win32u/sysparams.c | 16 ++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/dlls/gdi32/tests/driver.c b/dlls/gdi32/tests/driver.c index 6771c7c8247..ea4c87455cb 100644 --- a/dlls/gdi32/tests/driver.c +++ b/dlls/gdi32/tests/driver.c @@ -1006,6 +1006,7 @@ static void test_gpu_device_properties(const GUID *devinterface_guid) WCHAR device_id[256]; DEVPROPTYPE type; unsigned int i; + UINT32 value; HDEVINFO set; BOOL ret; @@ -1030,6 +1031,17 @@ static void test_gpu_device_properties(const GUID *devinterface_guid) ok(ret, "Got unexpected ret %d, GetLastError() %lu.\n", ret, GetLastError()); ok(type == DEVPROP_TYPE_STRING, "Got type %ld.\n", type); + ret = SetupDiGetDevicePropertyW(set, &device_data, &DEVPKEY_Device_BusNumber, &type, + (BYTE *)&value, sizeof(value), NULL, 0); + if (!wcsicmp(device_id, L"root\\basicrender") || !wcsicmp(device_id, L"root\\basicdisplay")) + { + ok(!ret, "Found Bus Id.\n"); + } + else + { + ok(ret, "Got unexpected ret %d, GetLastError() %lu, %s.\n", ret, GetLastError(), debugstr_w(device_id)); + ok(type == DEVPROP_TYPE_UINT32, "Got type %ld.\n", type); + } ++i; } SetupDiDestroyDeviceInfoList( set ); diff --git a/dlls/win32u/sysparams.c b/dlls/win32u/sysparams.c index a02158a17a0..b9ca5fce439 100644 --- a/dlls/win32u/sysparams.c +++ b/dlls/win32u/sysparams.c @@ -101,6 +101,14 @@ static const WCHAR devpkey_device_matching_device_id[] = '\\','0','0','0','8' }; +static const WCHAR devpkey_device_bus_number[] = +{ + 'P','r','o','p','e','r','t','i','e','s', + '\\','{','A','4','5','C','2','5','4','E','-','D','F','1','C','-','4','E','F','D', + '-','8','0','2','0','-','6','7','D','1','4','6','A','8','5','0','E','0','}', + '\\','0','0','1','7' +}; + static const WCHAR devpropkey_device_ispresentW[] = { 'P','r','o','p','e','r','t','i','e','s', @@ -1237,6 +1245,14 @@ static void add_gpu( const struct gdi_gpu *gpu, void *param ) NtClose( subkey ); } + if (gpu->vendor_id && gpu->device_id) + { + if ((subkey = reg_create_key( hkey, devpkey_device_bus_number, + sizeof(devpkey_device_bus_number), 0, NULL ))) + set_reg_value( subkey, NULL, 0xffff0000 | DEVPROP_TYPE_UINT32, + &gpu_index, sizeof(gpu_index) ); + } + desc = gpu->name; if (!desc[0]) desc = wine_adapterW; set_reg_value( hkey, device_descW, REG_SZ, desc, (lstrlenW( desc ) + 1) * sizeof(WCHAR) ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2449
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/gdi32/tests/driver.c | 7 +++++++ dlls/win32u/sysparams.c | 18 ++++++++++++++++++ include/cfgmgr32.h | 4 ++++ 3 files changed, 29 insertions(+) diff --git a/dlls/gdi32/tests/driver.c b/dlls/gdi32/tests/driver.c index ea4c87455cb..ba6ceea04f1 100644 --- a/dlls/gdi32/tests/driver.c +++ b/dlls/gdi32/tests/driver.c @@ -33,6 +33,7 @@ #include "setupapi.h" #include "ntddvdeo.h" #include "devpkey.h" +#include "cfgmgr32.h" #include "wine/test.h" @@ -1042,6 +1043,12 @@ static void test_gpu_device_properties(const GUID *devinterface_guid) ok(ret, "Got unexpected ret %d, GetLastError() %lu, %s.\n", ret, GetLastError(), debugstr_w(device_id)); ok(type == DEVPROP_TYPE_UINT32, "Got type %ld.\n", type); } + + ret = SetupDiGetDevicePropertyW(set, &device_data, &DEVPKEY_Device_RemovalPolicy, &type, + (BYTE *)&value, sizeof(value), NULL, 0); + ok(ret, "Got unexpected ret %d, GetLastError() %lu, %s.\n", ret, GetLastError(), debugstr_w(device_id)); + ok(value == CM_REMOVAL_POLICY_EXPECT_NO_REMOVAL || value == CM_REMOVAL_POLICY_EXPECT_ORDERLY_REMOVAL + || value == CM_REMOVAL_POLICY_EXPECT_SURPRISE_REMOVAL, "Got value %d.\n", value); ++i; } SetupDiDestroyDeviceInfoList( set ); diff --git a/dlls/win32u/sysparams.c b/dlls/win32u/sysparams.c index b9ca5fce439..20defa6c3bb 100644 --- a/dlls/win32u/sysparams.c +++ b/dlls/win32u/sysparams.c @@ -32,6 +32,7 @@ #include "ntgdi_private.h" #include "ntuser_private.h" #include "devpropdef.h" +#include "cfgmgr32.h" #include "wine/wingdi16.h" #include "wine/server.h" @@ -109,6 +110,14 @@ static const WCHAR devpkey_device_bus_number[] = '\\','0','0','1','7' }; +static const WCHAR devpkey_device_removal_policy[] = +{ + 'P','r','o','p','e','r','t','i','e','s', + '\\','{','A','4','5','C','2','5','4','E','-','D','F','1','C','-','4','E','F','D', + '-','8','0','2','0','-','6','7','D','1','4','6','A','8','5','0','E','0','}', + '\\','0','0','2','1' +}; + static const WCHAR devpropkey_device_ispresentW[] = { 'P','r','o','p','e','r','t','i','e','s', @@ -1253,6 +1262,15 @@ static void add_gpu( const struct gdi_gpu *gpu, void *param ) &gpu_index, sizeof(gpu_index) ); } + if ((subkey = reg_create_key( hkey, devpkey_device_removal_policy, + sizeof(devpkey_device_removal_policy), 0, NULL ))) + { + unsigned int removal_policy = CM_REMOVAL_POLICY_EXPECT_NO_REMOVAL; + + set_reg_value( subkey, NULL, 0xffff0000 | DEVPROP_TYPE_UINT32, + &removal_policy, sizeof(removal_policy) ); + } + desc = gpu->name; if (!desc[0]) desc = wine_adapterW; set_reg_value( hkey, device_descW, REG_SZ, desc, (lstrlenW( desc ) + 1) * sizeof(WCHAR) ); diff --git a/include/cfgmgr32.h b/include/cfgmgr32.h index d300c4babaa..bff32fe1c08 100644 --- a/include/cfgmgr32.h +++ b/include/cfgmgr32.h @@ -180,6 +180,10 @@ typedef DWORD CONFIGRET; #define CM_REGISTRY_USER 0x0100 #define CM_REGISTRY_CONFIG 0x0200 +#define CM_REMOVAL_POLICY_EXPECT_NO_REMOVAL 1 +#define CM_REMOVAL_POLICY_EXPECT_ORDERLY_REMOVAL 2 +#define CM_REMOVAL_POLICY_EXPECT_SURPRISE_REMOVAL 3 + typedef DWORD DEVINST, *PDEVINST; typedef DWORD DEVNODE, *PDEVNODE; typedef HANDLE HMACHINE, *PHMACHINE; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2449
v3: - Fix test failure (and extend the test for GUID_DISPLAY_DEVICE_ARRIVAL device interface). The culprit here is that GUID_DEVINTERFACE_DISPLAY_ADAPTER and GUID_DISPLAY_DEVICE_ARRIVAL ends up being different entities on modern Windows, while Wine links both interfaces to the same device. So on Windows for display arrival the software driver's device id is ROOT\\BasicDisplay while for adapter it is ROOT\\BasicRender. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2449#note_27377
Jacek Caban (@jacek) commented about dlls/win32u/sysparams.c:
NtClose( subkey ); }
+ if (gpu->vendor_id && gpu->device_id) + { + if ((subkey = reg_create_key( hkey, devpkey_device_bus_number, + sizeof(devpkey_device_bus_number), 0, NULL ))) + set_reg_value( subkey, NULL, 0xffff0000 | DEVPROP_TYPE_UINT32, + &gpu_index, sizeof(gpu_index) );
This leaks subkey. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2449#note_27412
Jacek Caban (@jacek) commented about dlls/win32u/sysparams.c:
&gpu_index, sizeof(gpu_index) ); }
+ if ((subkey = reg_create_key( hkey, devpkey_device_removal_policy, + sizeof(devpkey_device_removal_policy), 0, NULL ))) + { + unsigned int removal_policy = CM_REMOVAL_POLICY_EXPECT_NO_REMOVAL; + + set_reg_value( subkey, NULL, 0xffff0000 | DEVPROP_TYPE_UINT32, + &removal_policy, sizeof(removal_policy) ); + }
Same here. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2449#note_27413
On Tue Mar 21 14:58:42 2023 +0000, Jacek Caban wrote:
Same here. Oh yeah, thanks. I will wait a bit for other comments before resending.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2449#note_27414
Zhiyi Zhang (@zhiyi) commented about dlls/gdi32/tests/driver.c:
+ iface_data = (SP_DEVICE_INTERFACE_DETAIL_DATA_W *)iface_detail_buffer; + iface_data->cbSize = sizeof(*iface_data); + + i = 0; + while (SetupDiEnumDeviceInterfaces(set, NULL, devinterface_guid, i, &iface)) + { + ret = SetupDiGetDeviceInterfaceDetailW(set, &iface, iface_data, + sizeof(iface_detail_buffer), NULL, &device_data ); + ok(ret, "Got unexpected ret %d, GetLastError() %lu.\n", ret, GetLastError()); + + ret = SetupDiGetDevicePropertyW(set, &device_data, &DEVPKEY_Device_MatchingDeviceId, &type, + (BYTE *)device_id, sizeof(device_id), NULL, 0); + ok(ret, "Got unexpected ret %d, GetLastError() %lu.\n", ret, GetLastError()); + ok(type == DEVPROP_TYPE_STRING, "Got type %ld.\n", type); + + ++i; I think you should check that there are indeed devices with these properties found.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2449#note_27479
Zhiyi Zhang (@zhiyi) commented about dlls/gdi32/tests/driver.c:
+ { + ret = SetupDiGetDeviceInterfaceDetailW(set, &iface, iface_data, + sizeof(iface_detail_buffer), NULL, &device_data ); + ok(ret, "Got unexpected ret %d, GetLastError() %lu.\n", ret, GetLastError()); + + ret = SetupDiGetDevicePropertyW(set, &device_data, &DEVPKEY_Device_MatchingDeviceId, &type, + (BYTE *)device_id, sizeof(device_id), NULL, 0); + ok(ret, "Got unexpected ret %d, GetLastError() %lu.\n", ret, GetLastError()); + ok(type == DEVPROP_TYPE_STRING, "Got type %ld.\n", type); + + ++i; + } + SetupDiDestroyDeviceInfoList( set ); +} + + Remove this empty line.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2449#note_27480
Zhiyi Zhang (@zhiyi) commented about dlls/gdi32/tests/driver.c:
+ + i = 0; + while (SetupDiEnumDeviceInterfaces(set, NULL, devinterface_guid, i, &iface)) + { + ret = SetupDiGetDeviceInterfaceDetailW(set, &iface, iface_data, + sizeof(iface_detail_buffer), NULL, &device_data ); + ok(ret, "Got unexpected ret %d, GetLastError() %lu.\n", ret, GetLastError()); + + ret = SetupDiGetDevicePropertyW(set, &device_data, &DEVPKEY_Device_MatchingDeviceId, &type, + (BYTE *)device_id, sizeof(device_id), NULL, 0); + ok(ret, "Got unexpected ret %d, GetLastError() %lu.\n", ret, GetLastError()); + ok(type == DEVPROP_TYPE_STRING, "Got type %ld.\n", type); + + ++i; + } + SetupDiDestroyDeviceInfoList( set ); Inconsistent style. Please remove the space inside the parentheses.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2449#note_27478
Zhiyi Zhang (@zhiyi) commented about dlls/gdi32/tests/driver.c:
test_D3DKMTCheckOcclusion(); test_D3DKMTOpenAdapterFromDeviceName(); test_D3DKMTQueryVideoMemoryInfo(); + winetest_push_context("GUID_DEVINTERFACE_DISPLAY_ADAPTER"); + test_gpu_device_properties(&GUID_DEVINTERFACE_DISPLAY_ADAPTER); + winetest_pop_context(); + winetest_push_context("GUID_DISPLAY_DEVICE_ARRIVAL"); + test_gpu_device_properties(&GUID_DISPLAY_DEVICE_ARRIVAL); + winetest_pop_context();
Personally, I prefer you use only one function here because it's cleaner. For example, test_gpu_device_properties(), and then in test_gpu_device_properties() you call something like test_gpu_device_properties_guid() for subtests. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2449#note_27481
On Wed Mar 22 03:29:20 2023 +0000, Zhiyi Zhang wrote:
I think you should check that there are indeed devices with these properties found. This is not the case on many testbot machines, probably due to special GPU drivers not corresponding to real hardware GPUs, and maybe there is also some specifics on old Windows versions. That's why I didn't add that as it would have to accept anything else as broken result anyway. Or do you see some sensible way to add such a test?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2449#note_27482
On Wed Mar 22 03:33:42 2023 +0000, Paul Gofman wrote:
This is not the case on many testbot machines, probably due to special GPU drivers not corresponding to real hardware GPUs, and maybe there is also some specifics on old Windows versions. That's why I didn't add that as it would have to accept anything else as broken result anyway. Or do you see some sensible way to add such a test? It's fine as it is if TestBots doesn't have these properties as long as you have checked the result on a machine with a physical GPU.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2449#note_27483
participants (4)
-
Jacek Caban (＠jacek) -
Paul Gofman
-
Paul Gofman (＠gofman)
-
Zhiyi Zhang (＠zhiyi)