[PATCH 1/2] dxgi/tests: Add test for enumerating display adapters using SetupAPI.
Signed-off-by: Józef Kucia <jkucia(a)codeweavers.com> --- Shows that display adapter LUIDs are stored in SetupAPI device properties. It might be useful information to fix LUIDs in Wine. We could register display adapters and assign LUIDs to them. DXGI, winevulkan and wined3d could retrieve LUIDs using SetupAPI. --- dlls/dxgi/tests/Makefile.in | 2 +- dlls/dxgi/tests/dxgi.c | 115 ++++++++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+), 1 deletion(-) diff --git a/dlls/dxgi/tests/Makefile.in b/dlls/dxgi/tests/Makefile.in index 1c99d70957d5..76f78cc5fd4d 100644 --- a/dlls/dxgi/tests/Makefile.in +++ b/dlls/dxgi/tests/Makefile.in @@ -1,5 +1,5 @@ TESTDLL = dxgi.dll -IMPORTS = d3d10_1 dxgi user32 +IMPORTS = d3d10_1 dxgi setupapi user32 C_SRCS = \ dxgi.c diff --git a/dlls/dxgi/tests/dxgi.c b/dlls/dxgi/tests/dxgi.c index 02cf744be358..bf57ea2fb1bc 100644 --- a/dlls/dxgi/tests/dxgi.c +++ b/dlls/dxgi/tests/dxgi.c @@ -22,6 +22,9 @@ #include "dxgi1_6.h" #include "d3d11.h" #include "d3d12.h" +#include "devguid.h" +#include "devpkey.h" +#include "setupapi.h" #include "wine/heap.h" #include "wine/test.h" @@ -820,6 +823,117 @@ static void test_adapter_luid(void) ok(!refcount, "Factory has %u references left.\n", refcount); } +DEFINE_DEVPROPKEY(DEVPROPKEY_DISPLAY_ADAPTER_LUID, + 0x60b193cb, 0x5276, 0x4d0f, 0x96, 0xfc, 0xf1, 0x73, 0xab, 0xad, 0x3e, 0xc6, 2); + +static void test_display_adapters(void) +{ + DXGI_ADAPTER_DESC adapter_desc; + BOOL found_display_adapter; + unsigned int adapter_index; + unsigned int device_index; + DEVPROPTYPE property_type; + IDXGIFactory *factory; + IDXGIAdapter *adapter; + SP_DEVINFO_DATA data; + HDEVINFO dev_info; + DWORD last_error; + ULONG refcount; + void *buffer; + WCHAR *name; + DWORD size; + HRESULT hr; + LUID luid; + BOOL ret; + + hr = CreateDXGIFactory(&IID_IDXGIFactory, (void **)&factory); + ok(hr == S_OK, "Failed to create DXGI factory, hr %#x.\n", hr); + + for (adapter_index = 0; (hr = IDXGIFactory_EnumAdapters(factory, adapter_index, &adapter)) == S_OK; ++adapter_index) + { + hr = IDXGIAdapter_GetDesc(adapter, &adapter_desc); + ok(hr == S_OK, "Failed to get adapter desc, hr %#x.\n", hr); + refcount = IDXGIAdapter_Release(adapter); + ok(!refcount, "Adapter has %u references left.\n", refcount); + + /* Skip WARP. */ + if ((!adapter_desc.SubSysId && !adapter_desc.Revision && !adapter_desc.VendorId && !adapter_desc.DeviceId) + || (adapter_desc.VendorId == 0x1414 && adapter_desc.DeviceId == 0x008c)) + continue; + + found_display_adapter = FALSE; + + dev_info = SetupDiGetClassDevsW(&GUID_DEVCLASS_DISPLAY, NULL, NULL, DIGCF_PRESENT); + ok(dev_info != INVALID_HANDLE_VALUE, "Failed to get device information set.\n"); + + data.cbSize = sizeof(data); + for (device_index = 0; SetupDiEnumDeviceInfo(dev_info, device_index, &data); ++device_index) + { + /* LUID */ + size = 0; + property_type = DEVPROP_TYPE_EMPTY; + ret = SetupDiGetDevicePropertyW(dev_info, &data, &DEVPROPKEY_DISPLAY_ADAPTER_LUID, + &property_type, NULL, 0, &size, 0); + last_error = GetLastError(); + todo_wine + ok(!ret && last_error == ERROR_INSUFFICIENT_BUFFER, + "Failed to get device property size, ret %#x, last_error %#x.\n", ret, last_error); + if (!ret && last_error == ERROR_NOT_FOUND) + break; + ok(property_type == DEVPROP_TYPE_UINT64, "Got unexpected property type %#x.\n", property_type); + ok(size == sizeof(LUID), "Got unexpected size %u.\n", size); + + buffer = heap_alloc(size); + ok(!!buffer, "Failed to allocate memory.\n"); + ret = SetupDiGetDevicePropertyW(dev_info, &data, &DEVPROPKEY_DISPLAY_ADAPTER_LUID, + &property_type, buffer, size, &size, 0); + ok(ret, "Failed to get device property, ret %#x, last_error %#x.\n", ret, GetLastError()); + luid = *(LUID *)buffer; + heap_free(buffer); + + if (equal_luid(luid, adapter_desc.AdapterLuid)) + { + found_display_adapter = TRUE; + + /* name */ + size = 0; + property_type = DEVPROP_TYPE_STRING; + ret = SetupDiGetDevicePropertyW(dev_info, &data, &DEVPKEY_NAME, + &property_type, NULL, 0, &size, 0); + last_error = GetLastError(); + ok(!ret && last_error == ERROR_INSUFFICIENT_BUFFER, + "Failed to get device property size, ret %#x, last_error %#x.\n", ret, last_error); + ok(property_type == DEVPROP_TYPE_STRING, "Got unexpected property type %#x.\n", property_type); + + name = heap_alloc(size); + ok(!!name, "Failed to allocate memory.\n"); + ret = SetupDiGetDevicePropertyW(dev_info, &data, &DEVPKEY_NAME, + &property_type, (void *)name, size, &size, 0); + ok(ret, "Failed to get device property, ret %#x, last_error %#x.\n", ret, GetLastError()); + + ok(!lstrcmpW(adapter_desc.Description, name), + "Adapter names do not match: %s, %s.\n", + wine_dbgstr_w(adapter_desc.Description), wine_dbgstr_w(name)); + + heap_free(name); + break; + } + } + + ret = SetupDiDestroyDeviceInfoList(dev_info); + ok(ret, "Failed to destroy device info list.\n"); + + todo_wine + ok(found_display_adapter, "Failed to find display adapter for IDXGIAdapter %u (%s %04x:%04x).\n", + adapter_index, wine_dbgstr_w(adapter_desc.Description), + adapter_desc.VendorId, adapter_desc.DeviceId); + } + ok(hr == DXGI_ERROR_NOT_FOUND, "Got unexpected hr %#x.\n", hr); + + refcount = IDXGIFactory_Release(factory); + ok(!refcount, "Factory has %u references left.\n", refcount); +} + static void test_query_video_memory_info(void) { DXGI_QUERY_VIDEO_MEMORY_INFO memory_info; @@ -5018,6 +5132,7 @@ START_TEST(dxgi) queue_test(test_adapter_desc); queue_test(test_adapter_luid); + queue_test(test_display_adapters); queue_test(test_query_video_memory_info); queue_test(test_check_interface_support); queue_test(test_create_surface); -- 2.18.1
Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=45378 Your paranoid android. === debian9 (build log) === X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig) X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig) X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig) X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig) === debian9 (build log) === X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig) X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig)
On 12/03/2018 12:52 PM, Józef Kucia wrote:
Signed-off-by: Józef Kucia <jkucia(a)codeweavers.com> ---
Shows that display adapter LUIDs are stored in SetupAPI device properties. It might be useful information to fix LUIDs in Wine. We could register display adapters and assign LUIDs to them. DXGI, winevulkan and wined3d could retrieve LUIDs using SetupAPI.
Just to sort of continue the discussion here, what are we imagining to be responsible for creating display devices (and hence allocating LUIDs)? These are, as I understand, kernel devices on Windows, which means that an application could (in theory) open a device handle with SetupDiGetDeviceInstanceId()/SetupDiGetDeviceInterfaceDetail(). I'm not aware of any application that does this, unless one of those attached to bug 35345 [1] does. If there's no demand for these actually being kernel devices, we could instead just create them in the user driver, though this might lead to some complexity if we want DIGCF_PRESENT to actually work. This is related, IIUC, to the perennial problem of EnumDisplayDevices(), which is likely just a thin wrapper around SetupDiEnumDeviceInfo(). Alexandre has stated that this should be handled in the explorer.exe process, despite objections. [2] These objections wouldn't be helped by making it a kernel driver, mind, but I'd also like to push to somehow resolve them as well :-) [1] https://bugs.winehq.org/show_bug.cgi?id=35345 [2] https://www.winehq.org/pipermail/wine-devel/2014-March/103685.html
Thanks for starting the discussion. On Mon, Dec 3, 2018 at 8:43 PM Zebediah Figura <z.figura12(a)gmail.com> wrote:
Just to sort of continue the discussion here, what are we imagining to be responsible for creating display devices (and hence allocating LUIDs)?
I don't really know at this point. We talked about it a bit on IRC, but there is no consensus yet on what should be responsible for creating display devices and allocating LUIDs. I imagine Wine graphics driver should be involved since we need to enumerate available Vulkan physical devices. The patch was submitted mainly to show a potential storage for display adapter LUIDs. We would like to expose stable LUIDs in OpenGL [1] and Vulkan [2]. When we have stable LUIDs in OpenGL and Vulkan we will be able to match OpenGL and Vulkan devices, and expose stable LUIDs in d3d9 and dxgi. [1] - See DEVICE_LUID_EXT in https://www.khronos.org/registry/OpenGL/extensions/EXT/EXT_external_objects_.... [2] - https://www.khronos.org/registry/vulkan/specs/1.1-extensions/man/html/VkPhys...
On Tue, 4 Dec 2018 at 19:13, Józef Kucia <joseph.kucia(a)gmail.com> wrote:
Thanks for starting the discussion.
On Mon, Dec 3, 2018 at 8:43 PM Zebediah Figura <z.figura12(a)gmail.com> wrote:
Just to sort of continue the discussion here, what are we imagining to be responsible for creating display devices (and hence allocating LUIDs)?
I don't really know at this point. We talked about it a bit on IRC, but there is no consensus yet on what should be responsible for creating display devices and allocating LUIDs. I imagine Wine graphics driver should be involved since we need to enumerate available Vulkan physical devices.
I think the way it's supposed to work is that the graphics driver registers itself with the plugplay service. We do have that service in Wine, but it doesn't do a lot yet. I think the question is mostly whether we should try to follow that model, or just have e.g. explorer create the appropriate registry keys.
On Mon, 3 Dec 2018 at 22:22, Józef Kucia <jkucia(a)codeweavers.com> wrote:
+ /* LUID */ + size = 0; + property_type = DEVPROP_TYPE_EMPTY; + ret = SetupDiGetDevicePropertyW(dev_info, &data, &DEVPROPKEY_DISPLAY_ADAPTER_LUID, + &property_type, NULL, 0, &size, 0); + last_error = GetLastError(); + todo_wine + ok(!ret && last_error == ERROR_INSUFFICIENT_BUFFER, + "Failed to get device property size, ret %#x, last_error %#x.\n", ret, last_error); This fails here on Windows:
dxgi.c:879: Test failed: Failed to get device property size, ret 0, last_error 0x490. dxgi.c:927: Test failed: Failed to find display adapter for IDXGIAdapter 0 (L"AMD Radeon HD 6310 Graphics" 1002:9802).
On Tue, Dec 4, 2018 at 4:14 PM Henri Verbeet <hverbeet(a)gmail.com> wrote:
On Mon, 3 Dec 2018 at 22:22, Józef Kucia <jkucia(a)codeweavers.com> wrote:
+ /* LUID */ + size = 0; + property_type = DEVPROP_TYPE_EMPTY; + ret = SetupDiGetDevicePropertyW(dev_info, &data, &DEVPROPKEY_DISPLAY_ADAPTER_LUID, + &property_type, NULL, 0, &size, 0); + last_error = GetLastError(); + todo_wine + ok(!ret && last_error == ERROR_INSUFFICIENT_BUFFER, + "Failed to get device property size, ret %#x, last_error %#x.\n", ret, last_error); This fails here on Windows:
dxgi.c:879: Test failed: Failed to get device property size, ret 0, last_error 0x490. dxgi.c:927: Test failed: Failed to find display adapter for IDXGIAdapter 0 (L"AMD Radeon HD 6310 Graphics" 1002:9802).
It's possible this properly is available only on Windows 10.
participants (5)
-
Henri Verbeet -
Józef Kucia -
Józef Kucia -
Marvin -
Zebediah Figura