On 2021-09-06 1:20 p.m., Zhiyi Zhang wrote:
You need to put this patch in the same series for it to apply.
On 9/3/21 10:32 PM, Eduard Permyakov wrote:
Signed-off-by: Eduard Permyakov epermyakov@codeweavers.com
dlls/winex11.drv/xrandr.c | 43 +++++++++++++++++++++++++++++++++++---- 1 file changed, 39 insertions(+), 4 deletions(-)
diff --git a/dlls/winex11.drv/xrandr.c b/dlls/winex11.drv/xrandr.c index eb2f7021ba6..3ae1e845ed3 100644 --- a/dlls/winex11.drv/xrandr.c +++ b/dlls/winex11.drv/xrandr.c @@ -499,6 +499,43 @@ static void get_edid(RROutput output, unsigned char **prop, unsigned long *len) } }
+static BOOL edid_parse_name(const unsigned char *edid, unsigned long edid_len, WCHAR name[14]) +{
- BOOL found = FALSE;
- enum { DESC_OFFSET = 0x48, DESC_SIZE = 18, NDESCS = 3, MAXLEN = 13 };
You can just use normal variable names here instead of an enum;
- static unsigned char disp_product_name_hdr[5] = {0x00, 0x00, 0x00, 0xFC, 0x00};
static const
- const unsigned char *cursor = edid + DESC_OFFSET;
- int i, name_idx = 0;
- if (edid_len < 128)
return FALSE;- for (i = 0; i < NDESCS; i++, cursor += DESC_SIZE)
- {
if (memcmp( cursor, disp_product_name_hdr, sizeof(disp_product_name_hdr) ) != 0)continue;cursor += sizeof(disp_product_name_hdr);while (*cursor != 0x0A && name_idx < MAXLEN)name[name_idx++] = *cursor++;name[name_idx++] = '\0';found = TRUE;break;You can return directly.
- }
- return found;
+}
+static void monitor_set_name(struct x11drv_monitor *monitor, int index, const WCHAR *default_name) +{
You don't need default_name. generic_nonpnp_monitorW is the default name, you can move generic_nonpnp_monitorW to this function and remove default_name. get_monitor_name() seems better.
- WCHAR name[14];
You can pass monitor->name directly to edid_parse_name to avoid copying.
- if (monitor->edid && edid_parse_name(monitor->edid, monitor->edid_len, name))
lstrcpyW( monitor->name, name );- else
lstrcpyW( monitor->name, default_name );- TRACE("set name of monitor %d: %s\n", index, debugstr_w(monitor->name));
+}
- static void set_screen_size( int width, int height ) { int screen = default_visual.screen;
@@ -1067,9 +1104,9 @@ static BOOL xrandr14_get_monitors( ULONG_PTR adapter_id, struct x11drv_monitor * /* Inactive but attached monitor, no need to check for mirrored/replica monitors */ if (!output_info->crtc || !crtc_info->mode) {
lstrcpyW( monitors[monitor_count].name, generic_nonpnp_monitorW ); monitors[monitor_count].state_flags = DISPLAY_DEVICE_ATTACHED; get_edid( adapter_id, &monitors[monitor_count].edid, &monitors[monitor_count].edid_len );
monitor_set_name( monitors + monitor_count, monitor_count, generic_nonpnp_monitorW ); monitor_count = 1; } /* Active monitors, need to find other monitors with the same coordinates as mirrored */@@ -1112,9 +1149,6 @@ static BOOL xrandr14_get_monitors( ULONG_PTR adapter_id, struct x11drv_monitor * enum_crtc_info->width == crtc_info->width && enum_crtc_info->height == crtc_info->height) {
/* FIXME: Read output EDID property and parse the data to get the correct name */lstrcpyW( monitors[monitor_count].name, generic_nonpnp_monitorW );SetRect( &monitors[monitor_count].rc_monitor, crtc_info->x, crtc_info->y, crtc_info->x + crtc_info->width, crtc_info->y + crtc_info->height ); monitors[monitor_count].rc_work = get_work_area( &monitors[monitor_count].rc_monitor );@@ -1128,6 +1162,7 @@ static BOOL xrandr14_get_monitors( ULONG_PTR adapter_id, struct x11drv_monitor *
get_edid( screen_resources->outputs[i], &monitors[monitor_count].edid, &monitors[monitor_count].edid_len );
monitor_set_name( monitors + monitor_count, monitor_count, generic_nonpnp_monitorW ); monitor_count++; }I just checked on Windows and there seems to be some complications here. EnumDisplayDevice() still report a "Generic Non-PnP Monitor" for me; On my machine, the monitor EDID name "LG Ultra HD" is used for DISPLAYCONFIG_DEVICE_INFO_GET_TARGET_NAME. So either this patch needs more work to see whether DisplayConfigGetDeviceInfo() parses EDID directly, or get stored the monitor EDID name from somewhere, or we can leave this patch for now.
Hi,
Thanks for the feedback on my patches.
The parsing logic here is correct. It reports:
007c:trace:xrandr:monitor_set_name set name of monitor 0: L"Generic Non-PnP Monitor" 007c:trace:xrandr:monitor_set_name set name of monitor 1: L"E2250"
This is consistent with my monitors' EDIDs (The first monitor does not have a ModelName property, hence the default name is used).
However, this patch does not expose the data via DisplayConfigGetDeviceInfo(). Looking at the code, the handling of the DISPLAYCONFIG_DEVICE_INFO_GET_TARGET_NAME type in this function is the following:
case DISPLAYCONFIG_DEVICE_INFO_GET_TARGET_NAME: { DISPLAYCONFIG_TARGET_DEVICE_NAME *target_name = (DISPLAYCONFIG_TARGET_DEVICE_NAME *)packet;
FIXME("DISPLAYCONFIG_DEVICE_INFO_GET_TARGET_NAME: stub\n");
if (packet->size < sizeof(*target_name)) return ERROR_INVALID_PARAMETER;
return ERROR_NOT_SUPPORTED; }
This makes sense since we did not have EDID data before. If you think it is worthwhile, I can also make a patch where I parse the EDID data and populate the DISPLAYCONFIG_TARGET_DEVICE_NAME structure. The data will come from opening and reading the 'EDID' registry key that I added in my earlier patch. Then I can send this as a separate series since this is getting outside the scope of what I originally wanted to fix.