[PATCH 1/2] winex11.drv: Add 'EDID' registry key to every monitor device.
Zhiyi Zhang
zzhang at codeweavers.com
Fri Sep 3 05:07:11 CDT 2021
On 9/3/21 4:34 PM, Eduard Permyakov wrote:
> Extended display identification data (EDID) is a set of data that
> is provided by a display to describe its capabilities to a graphics
> adapter. EDID data allows a computer to detect the type of monitor
> that is connected to it. EDID data includes the manufacturer name,
> the product type, the timings that are supported by the display, the
> display size, as well as other display characteristics. EDID is
> defined by a standard published by the Video Electronics Standards
> Association (VESA).
>
> In Windows, every display device has an associated 'EEID' registry
Typo, EDID. It would be nice to mention what application this patch fixes.
> key containing this data. Applications can read and parse it to get
> the display capabilities. In Linux, we can get each monitor's EDID
> using the RANDR X extension.
>
> Signed-off-by: Eduard Permyakov <epermyakov at codeweavers.com>
> ---
> dlls/winex11.drv/desktop.c | 4 ++-
> dlls/winex11.drv/display.c | 13 ++++++--
> dlls/winex11.drv/x11drv.h | 6 +++-
> dlls/winex11.drv/x11drv_main.c | 3 +-
> dlls/winex11.drv/xinerama.c | 4 ++-
> dlls/winex11.drv/xrandr.c | 58 +++++++++++++++++++++++++++++++++-
> 6 files changed, 80 insertions(+), 8 deletions(-)
>
> diff --git a/dlls/winex11.drv/desktop.c b/dlls/winex11.drv/desktop.c
> index b517e44e150..71b3a0a5a27 100644
> --- a/dlls/winex11.drv/desktop.c
> +++ b/dlls/winex11.drv/desktop.c
> @@ -265,6 +265,8 @@ static BOOL X11DRV_desktop_get_monitors( ULONG_PTR adapter_id, struct x11drv_mon
> SetRect( &monitor->rc_work, 0, 0, desktop_width, desktop_height );
> query_desktop_work_area( &monitor->rc_work );
> monitor->state_flags = DISPLAY_DEVICE_ATTACHED;
> + monitor->edid_len = 0;
> + monitor->edid = NULL;
> if (desktop_width && desktop_height)
> monitor->state_flags |= DISPLAY_DEVICE_ACTIVE;
>
> @@ -273,7 +275,7 @@ static BOOL X11DRV_desktop_get_monitors( ULONG_PTR adapter_id, struct x11drv_mon
> return TRUE;
> }
>
> -static void X11DRV_desktop_free_monitors( struct x11drv_monitor *monitors )
> +static void X11DRV_desktop_free_monitors( struct x11drv_monitor *monitors, int count )
> {
> heap_free( monitors );
> }
> diff --git a/dlls/winex11.drv/display.c b/dlls/winex11.drv/display.c
> index b3ff213ae89..16769021aa8 100644
> --- a/dlls/winex11.drv/display.c
> +++ b/dlls/winex11.drv/display.c
> @@ -109,6 +109,7 @@ static const WCHAR monitor_hardware_idW[] = {
> 'M','O','N','I','T','O','R','\\',
> 'D','e','f','a','u','l','t','_','M','o','n','i','t','o','r',0,0};
> static const WCHAR driver_date_fmtW[] = {'%','u','-','%','u','-','%','u',0};
> +static const WCHAR edidW[] = {'E','D','I','D',0};
>
> static struct x11drv_display_device_handler host_handler;
> struct x11drv_display_device_handler desktop_handler;
> @@ -257,7 +258,7 @@ RECT get_host_primary_monitor_rect(void)
>
> if (gpus) host_handler.free_gpus(gpus);
> if (adapters) host_handler.free_adapters(adapters);
> - if (monitors) host_handler.free_monitors(monitors);
> + if (monitors) host_handler.free_monitors(monitors, monitor_count);
> return rect;
> }
>
> @@ -611,6 +612,12 @@ static BOOL X11DRV_InitMonitor(HDEVINFO devinfo, const struct x11drv_monitor *mo
> DEVPROP_TYPE_UINT32, (const BYTE *)&output_id, sizeof(output_id), 0))
> goto done;
>
> + if(monitor->edid) {
Formatting. Put a space after 'if' and '{' on the next line.
> + hkey = SetupDiCreateDevRegKeyW(devinfo, &device_data, DICS_FLAG_GLOBAL, 0, DIREG_DEV, NULL, NULL);
> + RegSetValueExW(hkey, edidW, 0, REG_BINARY, monitor->edid, monitor->edid_len);
> + RegCloseKey(hkey);
> + }
When an EDID is not present, Windows puts a BAD_EDID key with empty content there, we should do that as well.
> +
> /* Create driver key */
> hkey = SetupDiCreateDevRegKeyW(devinfo, &device_data, DICS_FLAG_GLOBAL, 0, DIREG_DRV, NULL, NULL);
> RegCloseKey(hkey);
> @@ -768,7 +775,7 @@ void X11DRV_DisplayDevices_Init(BOOL force)
> goto done;
> }
>
> - handler->free_monitors(monitors);
> + handler->free_monitors(monitors, monitor_count);
> monitors = NULL;
> video_index++;
> }
> @@ -788,5 +795,5 @@ done:
> if (adapters)
> handler->free_adapters(adapters);
> if (monitors)
> - handler->free_monitors(monitors);
> + handler->free_monitors(monitors, monitor_count);
> }
> diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h
> index e14e131d819..2b0f7be589f 100644
> --- a/dlls/winex11.drv/x11drv.h
> +++ b/dlls/winex11.drv/x11drv.h
> @@ -497,6 +497,7 @@ enum x11drv_atoms
> XATOM_text_rtf,
> XATOM_text_richtext,
> XATOM_text_uri_list,
> + XATOM_EDID,
> NB_XATOMS
> };
>
> @@ -752,6 +753,9 @@ struct x11drv_monitor
> RECT rc_work;
> /* StateFlags in DISPLAY_DEVICE struct */
> DWORD state_flags;
> + /* Extended Device Identification Data */
> + unsigned long edid_len;
> + unsigned char *edid;
> };
>
> /* Required functions for display device registry initialization */
> @@ -787,7 +791,7 @@ struct x11drv_display_device_handler
> void (*free_adapters)(struct x11drv_adapter *adapters);
>
> /* free_monitors will be called to free a monitor list from get_monitors */
> - void (*free_monitors)(struct x11drv_monitor *monitors);
> + void (*free_monitors)(struct x11drv_monitor *monitors, int count);
>
> /* register_event_handlers will be called to register event handlers.
> * This function pointer is optional and can be NULL when driver doesn't support it */
> diff --git a/dlls/winex11.drv/x11drv_main.c b/dlls/winex11.drv/x11drv_main.c
> index 2fac560556b..6e7cb91c5c7 100644
> --- a/dlls/winex11.drv/x11drv_main.c
> +++ b/dlls/winex11.drv/x11drv_main.c
> @@ -220,7 +220,8 @@ static const char * const atom_names[NB_XATOMS - FIRST_XATOM] =
> "text/plain",
> "text/rtf",
> "text/richtext",
> - "text/uri-list"
> + "text/uri-list",
> + "EDID"
> };
>
> /***********************************************************************
> diff --git a/dlls/winex11.drv/xinerama.c b/dlls/winex11.drv/xinerama.c
> index c9863482534..0e1e07669c2 100644
> --- a/dlls/winex11.drv/xinerama.c
> +++ b/dlls/winex11.drv/xinerama.c
> @@ -244,6 +244,8 @@ static BOOL xinerama_get_monitors( ULONG_PTR adapter_id, struct x11drv_monitor *
> monitor[index].rc_work = monitors[i].rcWork;
> /* Xinerama only reports monitors already attached */
> monitor[index].state_flags = DISPLAY_DEVICE_ATTACHED;
> + monitor[index].edid_len = 0;
> + monitor[index].edid = NULL;
Let's add a fake EDID data for Xinerama as well, same as what you did for virtual desktop monitors.
> if (!IsRectEmpty( &monitors[i].rcMonitor ))
> monitor[index].state_flags |= DISPLAY_DEVICE_ACTIVE;
>
> @@ -256,7 +258,7 @@ static BOOL xinerama_get_monitors( ULONG_PTR adapter_id, struct x11drv_monitor *
> return TRUE;
> }
>
> -static void xinerama_free_monitors( struct x11drv_monitor *monitors )
> +static void xinerama_free_monitors( struct x11drv_monitor *monitors, int count )
> {
> heap_free( monitors );
> }
> diff --git a/dlls/winex11.drv/xrandr.c b/dlls/winex11.drv/xrandr.c
> index e7adcec0f2e..78aaf4f5a01 100644
> --- a/dlls/winex11.drv/xrandr.c
> +++ b/dlls/winex11.drv/xrandr.c
> @@ -68,6 +68,8 @@ MAKE_FUNCPTR(XRRFreeOutputInfo)
> MAKE_FUNCPTR(XRRFreeScreenResources)
> MAKE_FUNCPTR(XRRGetCrtcInfo)
> MAKE_FUNCPTR(XRRGetOutputInfo)
> +MAKE_FUNCPTR(XRRListOutputProperties)
> +MAKE_FUNCPTR(XRRGetOutputProperty)
> MAKE_FUNCPTR(XRRGetScreenResources)
> MAKE_FUNCPTR(XRRGetScreenResourcesCurrent)
> MAKE_FUNCPTR(XRRGetScreenSizeRange)
> @@ -112,6 +114,8 @@ static int load_xrandr(void)
> LOAD_FUNCPTR(XRRFreeScreenResources);
> LOAD_FUNCPTR(XRRGetCrtcInfo);
> LOAD_FUNCPTR(XRRGetOutputInfo);
> + LOAD_FUNCPTR(XRRListOutputProperties);
> + LOAD_FUNCPTR(XRRGetOutputProperty);
> LOAD_FUNCPTR(XRRGetScreenResources);
> LOAD_FUNCPTR(XRRGetScreenResourcesCurrent);
> LOAD_FUNCPTR(XRRGetScreenSizeRange);
> @@ -466,6 +470,50 @@ static void get_screen_size( XRRScreenResources *resources, unsigned int *width,
> }
> }
>
> +static BOOL get_edid(XRROutputInfo *info, RROutput output, unsigned char **prop, unsigned long *len)
> +{
info is not used.
> + BOOL prop_found = FALSE;
> + Atom *properties;
> + int i, nprops;
I prefer property_count;
> + Atom actual_type;
> + int actual_format;
> + unsigned long bytes_after;
> +
> + *prop = NULL;
> + *len = 0;
> +
> + properties = pXRRListOutputProperties(gdi_display, output, &nprops);
> + for(i = 0; i < nprops; i++) {
> +
> + if(properties[i] == x11drv_atom(EDID)) {
> + prop_found = TRUE;
> + break;
> + }
> + }
Styling issue and an empty line.
> + XFree(properties);
> +
> + if(!prop_found) {
> + WARN("EDID property not found.\n");
> + return FALSE;
> + }
> +
> + pXRRGetOutputProperty(gdi_display,
> + output,
> + x11drv_atom(EDID),
> + 0,
> + 128,
> + FALSE,
> + FALSE,
> + AnyPropertyType,
> + &actual_type,
> + &actual_format,
> + len,
> + &bytes_after,
> + prop
> + );
Styling.
> + return TRUE;
Returned value is not used anywhere.
> +}
> +
> static void set_screen_size( int width, int height )
> {
> int screen = default_visual.screen;
> @@ -1036,6 +1084,7 @@ static BOOL xrandr14_get_monitors( ULONG_PTR adapter_id, struct x11drv_monitor *
> {
> lstrcpyW( monitors[monitor_count].name, generic_nonpnp_monitorW );
> monitors[monitor_count].state_flags = DISPLAY_DEVICE_ATTACHED;
> + get_edid(output_info, adapter_id, &monitors[monitor_count].edid, &monitors[monitor_count].edid_len);
Now that we have EDID, it would be nice to have the correct monitor name from EDID. It could be a separate patch.
> monitor_count = 1;
> }
> /* Active monitors, need to find other monitors with the same coordinates as mirrored */
> @@ -1091,6 +1140,9 @@ static BOOL xrandr14_get_monitors( ULONG_PTR adapter_id, struct x11drv_monitor *
>
> if (is_crtc_primary( primary_rect, crtc_info ))
> primary_index = monitor_count;
> +
> + get_edid(enum_output_info, screen_resources->outputs[i],
> + &monitors[monitor_count].edid, &monitors[monitor_count].edid_len);
Formatting.
> monitor_count++;
> }
>
> @@ -1137,8 +1189,12 @@ done:
> return ret;
> }
>
> -static void xrandr14_free_monitors( struct x11drv_monitor *monitors )
> +static void xrandr14_free_monitors( struct x11drv_monitor *monitors, int count )
> {
> + int i;
> + for(i = 0; i < count; i++) {
> + XFree(monitors[i].edid);
> + }
> heap_free( monitors );
> }
>
More information about the wine-devel
mailing list