[PATCH 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.
From: Paul Gofman pgofman@codeweavers.com
--- dlls/user32/tests/monitor.c | 2 -- dlls/win32u/sysparams.c | 40 +++++++++++++++++++++++++++++++++++-- 2 files changed, 38 insertions(+), 4 deletions(-)
diff --git a/dlls/user32/tests/monitor.c b/dlls/user32/tests/monitor.c index 1dff85b9621..adc70b5575a 100644 --- a/dlls/user32/tests/monitor.c +++ b/dlls/user32/tests/monitor.c @@ -1722,7 +1722,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 +1731,6 @@ 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); - }
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..13b9fd7205d 100644 --- a/dlls/win32u/sysparams.c +++ b/dlls/win32u/sysparams.c @@ -5862,13 +5862,49 @@ 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; + unsigned int i, display_frequency = 0; + 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; + + preferred_mode->width = monitor->rc_monitor.right - monitor->rc_monitor.left; + preferred_mode->height = monitor->rc_monitor.bottom - monitor->rc_monitor.top; + + for (i = 0; i < monitor->adapter->mode_count; ++i) + { + DEVMODEW *mode = &monitor->adapter->modes[i]; + + if (mode->dmPelsWidth == preferred_mode->width && mode->dmPelsHeight == preferred_mode->height + && mode->dmDisplayFrequency > display_frequency) + display_frequency = mode->dmDisplayFrequency; + } + if (!display_frequency) display_frequency = 60; + preferred_mode->targetMode.targetVideoSignalInfo.vSyncFreq.Numerator = display_frequency; + preferred_mode->targetMode.targetVideoSignalInfo.vSyncFreq.Denominator = 1; + preferred_mode->targetMode.targetVideoSignalInfo.activeSize.cx = preferred_mode->width; + preferred_mode->targetMode.targetVideoSignalInfo.activeSize.cy = preferred_mode->height; + preferred_mode->targetMode.targetVideoSignalInfo.totalSize = preferred_mode->targetMode.targetVideoSignalInfo.activeSize; + + ret = STATUS_SUCCESS; + break; + } + + unlock_display_devices(); + return ret; } case DISPLAYCONFIG_DEVICE_INFO_GET_ADAPTER_NAME: {
Zhiyi Zhang (@zhiyi) commented about dlls/win32u/sysparams.c:
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;preferred_mode->width = monitor->rc_monitor.right - monitor->rc_monitor.left;preferred_mode->height = monitor->rc_monitor.bottom - monitor->rc_monitor.top;
I don't suppose this is what Windows does. But I guess that's why it's a semi-stub. You should be able to read the preferred timing mode from EDID. If that fails, you can fall back to the maximum resolution with the highest frequency. Also, it would be nice to have a test.
Zhiyi Zhang (@zhiyi) commented about dlls/win32u/sysparams.c:
preferred_mode->height = monitor->rc_monitor.bottom - monitor->rc_monitor.top;for (i = 0; i < monitor->adapter->mode_count; ++i){DEVMODEW *mode = &monitor->adapter->modes[i];if (mode->dmPelsWidth == preferred_mode->width && mode->dmPelsHeight == preferred_mode->height&& mode->dmDisplayFrequency > display_frequency)display_frequency = mode->dmDisplayFrequency;}if (!display_frequency) display_frequency = 60;preferred_mode->targetMode.targetVideoSignalInfo.vSyncFreq.Numerator = display_frequency;preferred_mode->targetMode.targetVideoSignalInfo.vSyncFreq.Denominator = 1;preferred_mode->targetMode.targetVideoSignalInfo.activeSize.cx = preferred_mode->width;preferred_mode->targetMode.targetVideoSignalInfo.activeSize.cy = preferred_mode->height;preferred_mode->targetMode.targetVideoSignalInfo.totalSize = preferred_mode->targetMode.targetVideoSignalInfo.activeSize;
You should also fill in other fields in DISPLAYCONFIG_VIDEO_SIGNAL_INFO with reasonable values instead of zeros.
On Thu Apr 20 02:35:06 2023 +0000, Zhiyi Zhang wrote:
I don't suppose this is what Windows does. But I guess that's why it's a semi-stub. You should be able to read the preferred timing mode from EDID. If that fails, you can fall back to the maximum resolution with the highest frequency. Also, it would be nice to have a test.
Oh yes, rc_monitor reflects current resolution and that's not what we want here, I should be better selecting that from the mode list.
Regarding EDID, are you sure reading that from EDID here worth it now? I think the mode we return here should ideally match what we can be returning elsewhere. We already fill output mode in user32/sysparams.c:set_mode_target_info() based on DEVMODEW. I guess once the difference in timing matters this should ideally match. And normally we should have DEVMODEW modes matches those from EDID. The only part which we don't have correct here is targetVideoSignalInfo.totalSize (which is bigger on Windows even on LCD monitor) and correct pixel clock and horizontal timing. We could fill that from EDID for preferred mode but I doubt it currently affects anything in practice, and whenever it does it is not apparent if reporting the correct timing but not matching QueryDisplayConfig() mode is better. And then if we want to make those timings correct, the modes returned by QueryDisplayConfig() might be not in EDID and to set that fully consistently we might need to relay that info from driver in some way besides EDID probably (e. g., xrandr has the necessary timing information per returned mode).
On Thu Apr 20 16:26:13 2023 +0000, Paul Gofman wrote:
Oh yes, rc_monitor reflects current resolution and that's not what we want here, I should be better selecting that from the mode list. Regarding EDID, are you sure reading that from EDID here worth it now? I think the mode we return here should ideally match what we can be returning elsewhere. We already fill output mode in user32/sysparams.c:set_mode_target_info() based on DEVMODEW. I guess once the difference in timing matters this should ideally match. And normally we should have DEVMODEW modes matches those from EDID. The only part which we don't have correct here is targetVideoSignalInfo.totalSize (which is bigger on Windows even on LCD monitor) and correct pixel clock and horizontal timing. We could fill that from EDID for preferred mode but I doubt it currently affects anything in practice, and whenever it does it is not apparent if reporting the correct timing but not matching QueryDisplayConfig() mode is better. And then if we want to make those timings correct, the modes returned by QueryDisplayConfig() might be not in EDID and to set that fully consistently we might need to relay that info from driver in some way besides EDID probably (e. g., xrandr has the necessary timing information per returned mode).
I was mainly referring to the preferred resolution, not timing. So I meant to get the preferred resolution from EDID, then select from the mode list with the same resolution and highest frequency. Timing can be constructed for the moment as we did for other places. The reason I mentioned this is that the preferred resolution is not necessarily the maximum resolution.
On Fri Apr 21 02:21:46 2023 +0000, Zhiyi Zhang wrote:
I was mainly referring to the preferred resolution, not timing. So I meant to get the preferred resolution from EDID, then select from the mode list with the same resolution and highest frequency. Timing can be constructed for the moment as we did for other places. The reason I mentioned this is that the preferred resolution is not necessarily the maximum resolution.
I guess I wasn't clear about the "preferred timing mode from EDID". I meant the preferred resolution read from the EDID preferred timing mode section.
-
Paul Gofman -
Paul Gofman (＠gofman)
-
Zhiyi Zhang (＠zhiyi)