[PATCH 1/2] winex11.drv: Add 'EDID' registry key to every monitor device.
Zhiyi Zhang
zzhang at codeweavers.com
Fri Sep 3 05:27:22 CDT 2021
On 9/3/21 6:07 PM, Zhiyi Zhang wrote:
>
> 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.
>
I think we can avoid calling XRRListOutputProperties() here. XRRGetOutputProperty should
be able to handle non-existent properties.
>> + 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