[PATCH v2 0/1] MR2678: win32u: Partially implement NtUserDisplayConfigGetDeviceInfo(DISPLAYCONFIG_DEVICE_INFO_GET_TARGET_PREFERRED_MODE).
win32u: Partially implement NtUserDisplayConfigGetDeviceInfo(DISPLAYCONFIG_DEVICE_INFO_GET_TARGET_PREFERRED_MODE).
Required for Ghost Recon Breakpoint.
-- v2: win32u: Partially implement NtUserDisplayConfigGetDeviceInfo(DISPLAYCONFIG_DEVICE_INFO_GET_TARGET_PREFERRED_MODE).
From: Paul Gofman pgofman@codeweavers.com
--- dlls/user32/tests/monitor.c | 49 ++++++++++++++++++++- dlls/win32u/sysparams.c | 87 ++++++++++++++++++++++++++++++++++++- 2 files changed, 132 insertions(+), 4 deletions(-)
diff --git a/dlls/user32/tests/monitor.c b/dlls/user32/tests/monitor.c index 1dff85b9621..6b7f7b44b15 100644 --- a/dlls/user32/tests/monitor.c +++ b/dlls/user32/tests/monitor.c @@ -1683,6 +1683,52 @@ static void check_device_path(const WCHAR *device_path, const LUID *adapter_id, SetupDiDestroyDeviceInfoList(set); }
+static void check_preferred_mode(const DISPLAYCONFIG_TARGET_PREFERRED_MODE *mode, const WCHAR *gdi_device_name) +{ + DISPLAYCONFIG_TARGET_PREFERRED_MODE mode2; + DEVMODEW dm, dm2; + LONG lret; + BOOL bret; + + dm.dmSize = sizeof(dm); + bret = EnumDisplaySettingsW(gdi_device_name, ENUM_CURRENT_SETTINGS, &dm); + ok(bret, "got error %lu.\n", GetLastError()); + + if (dm.dmPelsWidth == 1024 && dm.dmPelsHeight == 768) + { + skip("Current display mode is already 1024x768, skipping test.\n"); + return; + } + if (mode->width == 1024 && mode->height == 768) + { + skip("Preferred display mode is 1024x768, skipping test.\n"); + return; + } + + memset(&dm2, 0, sizeof(dm2)); + dm2.dmSize = sizeof(dm2); + dm2.dmFields = DM_PELSWIDTH | DM_PELSHEIGHT; + dm2.dmPelsWidth = 1024; + dm2.dmPelsHeight = 768; + lret = ChangeDisplaySettingsW(&dm2, 0); + if (lret != DISP_CHANGE_SUCCESSFUL) + { + skip("Can't change display settings, skipping test.\n"); + return; + } + + memset(&mode2, 0, sizeof(mode2)); + mode2.header = mode->header; + + lret = pDisplayConfigGetDeviceInfo(&mode2.header); + ok(!lret, "got %ld\n", lret); + ok(mode2.width == mode->width, "got %u, expected %u.\n", mode2.width, mode->width); + ok(mode2.height == mode->height, "got %u, expected %u.\n", mode2.height, mode->height); + + lret = ChangeDisplaySettingsW(&dm, 0); + ok(lret == DISP_CHANGE_SUCCESSFUL, "got %ld.\n", lret); +} + static void test_QueryDisplayConfig_result(UINT32 flags, UINT32 paths, const DISPLAYCONFIG_PATH_INFO *pi, UINT32 modes, const DISPLAYCONFIG_MODE_INFO *mi) { @@ -1722,7 +1768,6 @@ static void test_QueryDisplayConfig_result(UINT32 flags, ok(!ret, "Expected 0, got %ld\n", ret); check_device_path(target_name.monitorDevicePath, &target_name.header.adapterId, target_name.header.id);
- todo_wine { preferred_mode.header.type = DISPLAYCONFIG_DEVICE_INFO_GET_TARGET_PREFERRED_MODE; preferred_mode.header.size = sizeof(preferred_mode); preferred_mode.header.adapterId = pi[i].targetInfo.adapterId; @@ -1732,7 +1777,7 @@ static void test_QueryDisplayConfig_result(UINT32 flags, ok(!ret, "Expected 0, got %ld\n", ret); ok(preferred_mode.width > 0 && preferred_mode.height > 0, "Expected non-zero height/width, got %ux%u\n", preferred_mode.width, preferred_mode.height); - } + check_preferred_mode(&preferred_mode, source_name.viewGdiDeviceName);
todo_wine { adapter_name.header.type = DISPLAYCONFIG_DEVICE_INFO_GET_ADAPTER_NAME; diff --git a/dlls/win32u/sysparams.c b/dlls/win32u/sysparams.c index 374fcf9c11b..832b56eb380 100644 --- a/dlls/win32u/sysparams.c +++ b/dlls/win32u/sysparams.c @@ -33,6 +33,7 @@ #include "ntuser_private.h" #include "devpropdef.h" #include "cfgmgr32.h" +#include "d3dkmdt.h" #include "wine/wingdi16.h" #include "wine/server.h"
@@ -246,6 +247,7 @@ struct adapter
#define MONITOR_INFO_HAS_MONITOR_ID 0x00000001 #define MONITOR_INFO_HAS_MONITOR_NAME 0x00000002 +#define MONITOR_INFO_HAS_PREFERRED_MODE 0x00000004 struct edid_monitor_info { unsigned int flags; @@ -254,6 +256,8 @@ struct edid_monitor_info char monitor_id_string[8]; /* MONITOR_INFO_HAS_MONITOR_NAME */ WCHAR monitor_name[14]; + /* MONITOR_INFO_HAS_PREFERRED_MODE */ + unsigned int preferred_width, preferred_height; };
struct monitor @@ -505,6 +509,16 @@ static void get_monitor_info_from_edid( struct edid_monitor_info *info, const un
for (i = 0; i < 4; ++i) { + if (edid[54 + i * 18] || edid[54 + i * 18 + 1]) + { + /* Detailed timing descriptor. */ + if (info->flags & MONITOR_INFO_HAS_PREFERRED_MODE) continue; + info->preferred_width = edid[54 + i * 18 + 2] | ((UINT32)(edid[54 + i * 18 + 4] & 0xf0) << 4); + info->preferred_height = edid[54 + i * 18 + 5] | ((UINT32)(edid[54 + i * 18 + 7] & 0xf0) << 4); + if (info->preferred_width && info->preferred_height) + info->flags |= MONITOR_INFO_HAS_PREFERRED_MODE; + continue; + } if (edid[54 + i * 18 + 3] != 0xfc) continue; /* "Display name" ASCII descriptor. */ s = (const char *)&edid[54 + i * 18 + 5]; @@ -5862,13 +5876,82 @@ NTSTATUS WINAPI NtUserDisplayConfigGetDeviceInfo( DISPLAYCONFIG_DEVICE_INFO_HEAD case DISPLAYCONFIG_DEVICE_INFO_GET_TARGET_PREFERRED_MODE: { DISPLAYCONFIG_TARGET_PREFERRED_MODE *preferred_mode = (DISPLAYCONFIG_TARGET_PREFERRED_MODE *)packet; + DISPLAYCONFIG_VIDEO_SIGNAL_INFO *signal_info = &preferred_mode->targetMode.targetVideoSignalInfo; + unsigned int i, display_freq; + DEVMODEW *found_mode = NULL; + BOOL have_edid_mode = FALSE; + struct monitor *monitor;
- FIXME( "DISPLAYCONFIG_DEVICE_INFO_GET_TARGET_PREFERRED_MODE stub.\n" ); + FIXME( "DISPLAYCONFIG_DEVICE_INFO_GET_TARGET_PREFERRED_MODE semi-stub.\n" );
if (packet->size < sizeof(*preferred_mode)) return STATUS_INVALID_PARAMETER;
- return STATUS_NOT_SUPPORTED; + if (!lock_display_devices()) return STATUS_UNSUCCESSFUL; + + memset( &preferred_mode->width, 0, sizeof(*preferred_mode) - offsetof(DISPLAYCONFIG_TARGET_PREFERRED_MODE, width) ); + + LIST_FOR_EACH_ENTRY(monitor, &monitors, struct monitor, entry) + { + if (preferred_mode->header.id != monitor->output_id) continue; + if (memcmp( &preferred_mode->header.adapterId, &monitor->adapter->gpu_luid, + sizeof(monitor->adapter->gpu_luid) )) + continue; + + for (i = 0; i < monitor->adapter->mode_count; ++i) + { + DEVMODEW *mode = &monitor->adapter->modes[i]; + + if (!have_edid_mode && monitor->edid_info.flags & MONITOR_INFO_HAS_PREFERRED_MODE + && mode->dmPelsWidth == monitor->edid_info.preferred_width + && mode->dmPelsHeight == monitor->edid_info.preferred_height) + { + found_mode = mode; + have_edid_mode = TRUE; + } + + if (!have_edid_mode && (!found_mode + || (mode->dmPelsWidth > found_mode->dmPelsWidth && mode->dmPelsHeight >= found_mode->dmPelsHeight) + || (mode->dmPelsHeight > found_mode->dmPelsHeight && mode->dmPelsWidth >= found_mode->dmPelsWidth))) + found_mode = mode; + + if (mode->dmPelsWidth == found_mode->dmPelsWidth + && mode->dmPelsHeight == found_mode->dmPelsHeight + && mode->dmDisplayFrequency > found_mode->dmDisplayFrequency) + found_mode = mode; + } + + if (!found_mode) + { + ERR( "No mode found.\n" ); + break; + } + preferred_mode->width = found_mode->dmPelsWidth; + preferred_mode->height = found_mode->dmPelsHeight; + display_freq = found_mode->dmDisplayFrequency; + + signal_info->pixelRate = display_freq * preferred_mode->width * preferred_mode->height; + signal_info->hSyncFreq.Numerator = display_freq * preferred_mode->width; + signal_info->hSyncFreq.Denominator = 1; + signal_info->vSyncFreq.Numerator = display_freq; + signal_info->vSyncFreq.Denominator = 1; + signal_info->activeSize.cx = preferred_mode->width; + signal_info->activeSize.cy = preferred_mode->height; + signal_info->totalSize.cx = preferred_mode->width; + signal_info->totalSize.cy = preferred_mode->height; + signal_info->videoStandard = D3DKMDT_VSS_OTHER; + if (!(found_mode->dmFields & DM_DISPLAYFLAGS)) + signal_info->scanLineOrdering = DISPLAYCONFIG_SCANLINE_ORDERING_UNSPECIFIED; + else if (found_mode->dmDisplayFlags & DM_INTERLACED) + signal_info->scanLineOrdering = DISPLAYCONFIG_SCANLINE_ORDERING_INTERLACED; + else + signal_info->scanLineOrdering = DISPLAYCONFIG_SCANLINE_ORDERING_PROGRESSIVE; + ret = STATUS_SUCCESS; + break; + } + + unlock_display_devices(); + return ret; } case DISPLAYCONFIG_DEVICE_INFO_GET_ADAPTER_NAME: {
v2: - add a test showing that preferred mode does not depend on current mode; - select preferred mode from the mode list; - use EDID info to select preferred mode when available; - fill signal info with some reasonable values (like in user32/sysparams.c:set_mode_target_info()).
Zhiyi Zhang (@zhiyi) commented about dlls/win32u/sysparams.c:
for (i = 0; i < 4; ++i) {
if (edid[54 + i * 18] || edid[54 + i * 18 + 1]){/* Detailed timing descriptor. */if (info->flags & MONITOR_INFO_HAS_PREFERRED_MODE) continue;info->preferred_width = edid[54 + i * 18 + 2] | ((UINT32)(edid[54 + i * 18 + 4] & 0xf0) << 4);info->preferred_height = edid[54 + i * 18 + 5] | ((UINT32)(edid[54 + i * 18 + 7] & 0xf0) << 4);if (info->preferred_width && info->preferred_height)info->flags |= MONITOR_INFO_HAS_PREFERRED_MODE;continue;}
Actually, the EDID Detailed Timing Descriptor should contain a recommended display frequency and we should use that as well.
Zhiyi Zhang (@zhiyi) commented about dlls/win32u/sysparams.c:
break;}preferred_mode->width = found_mode->dmPelsWidth;preferred_mode->height = found_mode->dmPelsHeight;display_freq = found_mode->dmDisplayFrequency;signal_info->pixelRate = display_freq * preferred_mode->width * preferred_mode->height;signal_info->hSyncFreq.Numerator = display_freq * preferred_mode->width;signal_info->hSyncFreq.Denominator = 1;signal_info->vSyncFreq.Numerator = display_freq;signal_info->vSyncFreq.Denominator = 1;signal_info->activeSize.cx = preferred_mode->width;signal_info->activeSize.cy = preferred_mode->height;signal_info->totalSize.cx = preferred_mode->width;signal_info->totalSize.cy = preferred_mode->height;signal_info->videoStandard = D3DKMDT_VSS_OTHER;
I don't know what's the value on Windows. But D3DKMDT_VSS_VESA_CVT seems more common than D3DKMDT_VSS_OTHER.
On Wed Apr 26 14:23:50 2023 +0000, Zhiyi Zhang wrote:
Actually, the EDID Detailed Timing Descriptor should contain a recommended display frequency and we should use that as well.
I looked into doing this, but things are getting complicated in two orthogonal ways:
1. On one of my laptops Rog Strix (AMD with TMX5472 monitor) capable of 1920x1080 60/300Hz Detailed Timing Descriptor advertises 60Hz. Windows reports 300Hz as preferred mode. That 300Hz mode is advertised in EDID in "Display Range Limits Descriptor" (descriptor type 0xfd). It is possible to parse that one too as well but I am not sure if we cover all the possible cases this way. Or do you know if looking at this additionally is sufficient?
2. On my another Nvidia laptop we get some weird refresh rates from xrandr 1.0 (there is fallback to xrandr 1.0 with Nvidia proprietary driver). XRRRates returns those strange values (51 max for max resolution mode here, while in reality that is 120Hz or 60Hz). I didn't dig much into that why is it so (and with fallback disabled the refresh rate is OK), but I think in practice that will mean that we often won't match EDID's frequency on Nvidia machines and will be always falling back to not using EDID data at all (as apparently we can't report preferred mode which does not match anything in display mode list). Not sure, maybe that's ok and we can let it just fallback on Nvidia until xrandr issues on Nvidia are resolved somehow.
As a bottom line, I see p. 1. as more of an issue here, do you still think we should pursue timing data from EDID and use Display Range Limits Descriptor if present, or rather leave it as it is now?
On my another Nvidia laptop we get some weird refresh rates from xrandr 1.0 (there is fallback to xrandr 1.0 with Nvidia proprietary driver). XRRRates returns those strange values (51 max for max resolution mode here, while in reality that is 120Hz or 60Hz). I didn't dig much into that why is it so
That's from NVIDIA DynamicTwinView. See WineHQ bug 34348 for example.
As a bottom line, I see p. 1. as more of an issue here, do you still think we should pursue timing data from EDID and use Display Range Limits Descriptor if present, or rather leave it as it is now?
It seems better to leave it as it is. Thanks for the clarification.
On Wed Apr 26 14:26:07 2023 +0000, Zhiyi Zhang wrote:
I don't know what's the value on Windows. But D3DKMDT_VSS_VESA_CVT seems more common than D3DKMDT_VSS_OTHER.
On my laptop Windows reports D3DKMDT_VSS_OTHER for builtin monitor and D3DKMDT_VSS_EIA_861 for plugged Sony TV. We also report D3DKMDT_VSS_OTHER currently in QueryDisplayConfig(), so maybe it makes more sense to just leave `D3DKMDT_VSS_OTHER? If yes, do you think I should change anything else in the current patch?
This merge request was approved by Zhiyi Zhang.
-
Paul Gofman -
Paul Gofman (＠gofman)
-
Zhiyi Zhang (＠zhiyi)