[PATCH v5 1/3] winex11.drv: Add 'EDID' registry key to every monitor device.
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 'EDID' registry 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. This patch fixes, for example, monitor detection in the UE-based game 'Industries of Titan'. Signed-off-by: Eduard Permyakov <epermyakov(a)codeweavers.com> --- dlls/winex11.drv/desktop.c | 4 +++- dlls/winex11.drv/display.c | 15 ++++++++++++--- dlls/winex11.drv/x11drv.h | 6 +++++- dlls/winex11.drv/x11drv_main.c | 3 ++- dlls/winex11.drv/xinerama.c | 4 +++- dlls/winex11.drv/xrandr.c | 30 +++++++++++++++++++++++++++++- 6 files changed, 54 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..ca81f169f9f 100644 --- a/dlls/winex11.drv/display.c +++ b/dlls/winex11.drv/display.c @@ -109,6 +109,8 @@ 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 const WCHAR bad_edidW[] = {'B','A','D','_','E','D','I','D',0}; static struct x11drv_display_device_handler host_handler; struct x11drv_display_device_handler desktop_handler; @@ -257,7 +259,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 +613,13 @@ static BOOL X11DRV_InitMonitor(HDEVINFO devinfo, const struct x11drv_monitor *mo DEVPROP_TYPE_UINT32, (const BYTE *)&output_id, sizeof(output_id), 0)) goto done; + hkey = SetupDiCreateDevRegKeyW(devinfo, &device_data, DICS_FLAG_GLOBAL, 0, DIREG_DEV, NULL, NULL); + if (monitor->edid) + RegSetValueExW(hkey, edidW, 0, REG_BINARY, monitor->edid, monitor->edid_len); + else + RegSetValueExW(hkey, bad_edidW, 0, REG_BINARY, NULL, 0); + RegCloseKey(hkey); + /* Create driver key */ hkey = SetupDiCreateDevRegKeyW(devinfo, &device_data, DICS_FLAG_GLOBAL, 0, DIREG_DRV, NULL, NULL); RegCloseKey(hkey); @@ -768,7 +777,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 +797,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; 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..0b22d3e39a3 100644 --- a/dlls/winex11.drv/xrandr.c +++ b/dlls/winex11.drv/xrandr.c @@ -68,6 +68,7 @@ MAKE_FUNCPTR(XRRFreeOutputInfo) MAKE_FUNCPTR(XRRFreeScreenResources) MAKE_FUNCPTR(XRRGetCrtcInfo) MAKE_FUNCPTR(XRRGetOutputInfo) +MAKE_FUNCPTR(XRRGetOutputProperty) MAKE_FUNCPTR(XRRGetScreenResources) MAKE_FUNCPTR(XRRGetScreenResourcesCurrent) MAKE_FUNCPTR(XRRGetScreenSizeRange) @@ -112,6 +113,7 @@ static int load_xrandr(void) LOAD_FUNCPTR(XRRFreeScreenResources); LOAD_FUNCPTR(XRRGetCrtcInfo); LOAD_FUNCPTR(XRRGetOutputInfo); + LOAD_FUNCPTR(XRRGetOutputProperty); LOAD_FUNCPTR(XRRGetScreenResources); LOAD_FUNCPTR(XRRGetScreenResourcesCurrent); LOAD_FUNCPTR(XRRGetScreenSizeRange); @@ -466,6 +468,25 @@ static void get_screen_size( XRRScreenResources *resources, unsigned int *width, } } +static void get_edid( RROutput output, unsigned char **prop, unsigned long *len ) +{ + int result; + Atom actual_type; + int actual_format; + unsigned long bytes_after; + + result = pXRRGetOutputProperty( gdi_display, output, x11drv_atom(EDID), 0, 128, FALSE, FALSE, + AnyPropertyType, &actual_type, &actual_format, len, + &bytes_after, prop ); + + if (result != Success) + { + WARN("Could not retrieve EDID property.\n"); + *prop = NULL; + *len = 0; + } +} + static void set_screen_size( int width, int height ) { int screen = default_visual.screen; @@ -1036,6 +1057,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( adapter_id, &monitors[monitor_count].edid, &monitors[monitor_count].edid_len ); monitor_count = 1; } /* Active monitors, need to find other monitors with the same coordinates as mirrored */ @@ -1091,6 +1113,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( screen_resources->outputs[i], &monitors[monitor_count].edid, + &monitors[monitor_count].edid_len ); monitor_count++; } @@ -1137,8 +1162,11 @@ 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 ); } -- 2.25.1
Some applications use the EDID property of the monitor device to detect supported resolutions, and set the viewport size. Add a computed EDID to the virtual desktop monitor device to allow the virtual desktop driver to be better compatible with such applications. Most fields of the EDID contain details that are of no relevance to applications, so populate only the native resolution and physical monitor dimensions. This should cover the vast majority of use cases. Signed-off-by: Eduard Permyakov <epermyakov(a)codeweavers.com> --- dlls/winex11.drv/desktop.c | 75 +++++++++++++++++++++++++++++++++++++- 1 file changed, 73 insertions(+), 2 deletions(-) diff --git a/dlls/winex11.drv/desktop.c b/dlls/winex11.drv/desktop.c index 71b3a0a5a27..37e5624967e 100644 --- a/dlls/winex11.drv/desktop.c +++ b/dlls/winex11.drv/desktop.c @@ -20,6 +20,7 @@ */ #include "config.h" +#include <stdint.h> #include <X11/cursorfont.h> #include <X11/Xlib.h> @@ -111,6 +112,74 @@ static void add_desktop_mode( DEVMODEW *mode, DWORD depth, DWORD width, DWORD he mode->dmDisplayFrequency = 60; } +static void get_virtual_edid( unsigned char **edid, unsigned long *len ) +{ + static const size_t desc_size = 128; + int i; + uint8_t accum = 0; + /*Sample EDID for an LCD Desktop IT Display, from VESA spec */ + static const unsigned char default_edid[128] = { + /* Header */ + 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, + /* Manufacturer's code name */ + 0x04, 0x43, + /* Product code */ + 0x06, 0xf2, + /* Serial Number */ + 0x01, 0x00, 0x00, 0x00, + /* Date of manufacture */ + 0x01, 0x11, + /* EDID version */ + 0x01, 0x04, + /* Standard characteristics */ + 0x0f, 0x2b, 0x20, 0x78, 0x2b, 0x9c, 0x68, 0xa0, + 0x57, 0x4a, 0x9b, 0x26, 0x12, 0x48, 0x4c, 0xff, + 0xff, 0x80, 0xa9, 0x59, 0xa9, 0x4f, 0xa9, 0x4a, + 0xa9, 0x45, 0x81, 0x99, 0x81, 0x80, 0x61, 0x59, + 0x45, 0x59, + /* First 18-byte data block (Preferred Timing Mode) */ + 0x48, 0x3f, 0x40, 0x30, 0x62, 0xb0, 0x32, 0x40, + 0x40, 0xc0, 0x13, 0x00, 0xab, 0x40, 0x11, 0x00, + 0x00, 0x1e, + /* Second 18-byte data block */ + 0x00, 0x00, 0x00, 0xfd, 0x00, 0x32, 0x5a, 0x1e, + 0x6e, 0x17, 0x04, 0x11, 0x00, 0xc8, 0x90, 0x00, + 0x50, 0x3c, + /* Third 18-byte data block */ + 0x00, 0x00, 0x00, 0xf7, 0x00, 0x0a, 0xf7, 0x0f, + 0x03, 0x87, 0xc0, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, + /* Fourth 18-byte data block */ + 0x00, 0x00, 0x00, 0xfc, 0x00, 0x41, 0x42, 0x43, + 0x20, 0x4c, 0x43, 0x44, 0x32, 0x31, 0x0a, 0x20, + 0x20, 0x20, + /* Extension flag */ + 0x00, + }; + + *edid = heap_calloc( desc_size, 1 ); + if (!*edid) + { + *len = 0; + return; + } + + memcpy( *edid, default_edid, sizeof(default_edid) ); + *len = desc_size; + + /* Set the native resolution in the Preferred Timing Mode descriptor */ + (*edid)[0x38] = max_width & 0xff; + (*edid)[0x3a] = ((max_width >> 8) & 0xf) << 4; + + (*edid)[0x3b] = max_height & 0xff; + (*edid)[0x3d] = ((max_height >> 8) & 0xf) << 4; + + /* Set checksum */ + for (i = 0; i < desc_size-1; i++) + accum += (*edid)[i]; + (*edid)[desc_size - 1] = 256 - accum; +} + static BOOL X11DRV_desktop_get_modes( ULONG_PTR id, DWORD flags, DEVMODEW **new_modes, UINT *mode_count ) { UINT depth_idx, size_idx, mode_idx = 0; @@ -265,8 +334,7 @@ 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; + get_virtual_edid( &monitor->edid, &monitor->edid_len ); if (desktop_width && desktop_height) monitor->state_flags |= DISPLAY_DEVICE_ACTIVE; @@ -277,6 +345,9 @@ static BOOL X11DRV_desktop_get_monitors( ULONG_PTR adapter_id, struct x11drv_mon static void X11DRV_desktop_free_monitors( struct x11drv_monitor *monitors, int count ) { + int i; + for (i = 0; i < count; i++) + heap_free( monitors[i].edid ); heap_free( monitors ); } -- 2.25.1
On 9/8/21 1:43 AM, Eduard Permyakov wrote:
Some applications use the EDID property of the monitor device to detect supported resolutions, and set the viewport size. Add a computed EDID to the virtual desktop monitor device to allow the virtual desktop driver to be better compatible with such applications.
Most fields of the EDID contain details that are of no relevance to applications, so populate only the native resolution and physical monitor dimensions. This should cover the vast majority of use cases.
Signed-off-by: Eduard Permyakov <epermyakov(a)codeweavers.com> --- dlls/winex11.drv/desktop.c | 75 +++++++++++++++++++++++++++++++++++++- 1 file changed, 73 insertions(+), 2 deletions(-)
diff --git a/dlls/winex11.drv/desktop.c b/dlls/winex11.drv/desktop.c index 71b3a0a5a27..37e5624967e 100644 --- a/dlls/winex11.drv/desktop.c +++ b/dlls/winex11.drv/desktop.c @@ -20,6 +20,7 @@ */
#include "config.h" +#include <stdint.h> #include <X11/cursorfont.h> #include <X11/Xlib.h>
@@ -111,6 +112,74 @@ static void add_desktop_mode( DEVMODEW *mode, DWORD depth, DWORD width, DWORD he mode->dmDisplayFrequency = 60; }
+static void get_virtual_edid( unsigned char **edid, unsigned long *len ) +{ + static const size_t desc_size = 128;
You can remove desc_size entirely and use sizeof(default_edid) instead.
+ int i; + uint8_t accum = 0; + /*Sample EDID for an LCD Desktop IT Display, from VESA spec */ + static const unsigned char default_edid[128] = { + /* Header */ + 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, + /* Manufacturer's code name */ + 0x04, 0x43, + /* Product code */ + 0x06, 0xf2, + /* Serial Number */ + 0x01, 0x00, 0x00, 0x00, + /* Date of manufacture */ + 0x01, 0x11, + /* EDID version */ + 0x01, 0x04, + /* Standard characteristics */ + 0x0f, 0x2b, 0x20, 0x78, 0x2b, 0x9c, 0x68, 0xa0, + 0x57, 0x4a, 0x9b, 0x26, 0x12, 0x48, 0x4c, 0xff, + 0xff, 0x80, 0xa9, 0x59, 0xa9, 0x4f, 0xa9, 0x4a, + 0xa9, 0x45, 0x81, 0x99, 0x81, 0x80, 0x61, 0x59, + 0x45, 0x59, + /* First 18-byte data block (Preferred Timing Mode) */ + 0x48, 0x3f, 0x40, 0x30, 0x62, 0xb0, 0x32, 0x40, + 0x40, 0xc0, 0x13, 0x00, 0xab, 0x40, 0x11, 0x00, + 0x00, 0x1e, + /* Second 18-byte data block */ + 0x00, 0x00, 0x00, 0xfd, 0x00, 0x32, 0x5a, 0x1e, + 0x6e, 0x17, 0x04, 0x11, 0x00, 0xc8, 0x90, 0x00, + 0x50, 0x3c, + /* Third 18-byte data block */ + 0x00, 0x00, 0x00, 0xf7, 0x00, 0x0a, 0xf7, 0x0f, + 0x03, 0x87, 0xc0, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, + /* Fourth 18-byte data block */ + 0x00, 0x00, 0x00, 0xfc, 0x00, 0x41, 0x42, 0x43, + 0x20, 0x4c, 0x43, 0x44, 0x32, 0x31, 0x0a, 0x20, + 0x20, 0x20, + /* Extension flag */ + 0x00, + }; + + *edid = heap_calloc( desc_size, 1 ); + if (!*edid) + { + *len = 0; + return; + } + + memcpy( *edid, default_edid, sizeof(default_edid) );
You can use sizeof(default_edid) - 1 since you don't need to copy the last byte. Anyway, I am not feeling comfortable with the current approach for generating fake EDID data. It's not real enough. For example, physical size should reflect the recommended DPI values and the resolution list should match what's reported from EnumDisplaySettings(). In my opinion, you can either generate an EDID by carefully following the VESA EDID standard, or, the other way, which I think it's easier, is to pass the host primary monitor EDID to the virtual desktop by introducing a get_host_primary_monitor_edid(), similar to what get_host_primary_gpu() has done. This way has the benefit of not bothering with the EDID generation and has the same EDID as the host, which can reduce the difference between using virtual desktop and XRandR display device handlers. As for Xinerama, you can leave the EDID empty because Xinerama is rarely used nowadays.
+ *len = desc_size; + + /* Set the native resolution in the Preferred Timing Mode descriptor */ + (*edid)[0x38] = max_width & 0xff; + (*edid)[0x3a] = ((max_width >> 8) & 0xf) << 4; + + (*edid)[0x3b] = max_height & 0xff; + (*edid)[0x3d] = ((max_height >> 8) & 0xf) << 4; + + /* Set checksum */ + for (i = 0; i < desc_size-1; i++) + accum += (*edid)[i]; + (*edid)[desc_size - 1] = 256 - accum; +} + static BOOL X11DRV_desktop_get_modes( ULONG_PTR id, DWORD flags, DEVMODEW **new_modes, UINT *mode_count ) { UINT depth_idx, size_idx, mode_idx = 0; @@ -265,8 +334,7 @@ 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; + get_virtual_edid( &monitor->edid, &monitor->edid_len ); if (desktop_width && desktop_height) monitor->state_flags |= DISPLAY_DEVICE_ACTIVE;
@@ -277,6 +345,9 @@ static BOOL X11DRV_desktop_get_monitors( ULONG_PTR adapter_id, struct x11drv_mon
static void X11DRV_desktop_free_monitors( struct x11drv_monitor *monitors, int count ) { + int i; + for (i = 0; i < count; i++) + heap_free( monitors[i].edid ); heap_free( monitors ); }
On 2021-09-08 6:42 p.m., Zhiyi Zhang wrote:
On 9/8/21 1:43 AM, Eduard Permyakov wrote:
Some applications use the EDID property of the monitor device to detect supported resolutions, and set the viewport size. Add a computed EDID to the virtual desktop monitor device to allow the virtual desktop driver to be better compatible with such applications.
Most fields of the EDID contain details that are of no relevance to applications, so populate only the native resolution and physical monitor dimensions. This should cover the vast majority of use cases.
Signed-off-by: Eduard Permyakov <epermyakov(a)codeweavers.com> --- dlls/winex11.drv/desktop.c | 75 +++++++++++++++++++++++++++++++++++++- 1 file changed, 73 insertions(+), 2 deletions(-)
diff --git a/dlls/winex11.drv/desktop.c b/dlls/winex11.drv/desktop.c index 71b3a0a5a27..37e5624967e 100644 --- a/dlls/winex11.drv/desktop.c +++ b/dlls/winex11.drv/desktop.c @@ -20,6 +20,7 @@ */
#include "config.h" +#include <stdint.h> #include <X11/cursorfont.h> #include <X11/Xlib.h>
@@ -111,6 +112,74 @@ static void add_desktop_mode( DEVMODEW *mode, DWORD depth, DWORD width, DWORD he mode->dmDisplayFrequency = 60; }
+static void get_virtual_edid( unsigned char **edid, unsigned long *len ) +{ + static const size_t desc_size = 128; You can remove desc_size entirely and use sizeof(default_edid) instead.
+ int i; + uint8_t accum = 0; + /*Sample EDID for an LCD Desktop IT Display, from VESA spec */ + static const unsigned char default_edid[128] = { + /* Header */ + 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, + /* Manufacturer's code name */ + 0x04, 0x43, + /* Product code */ + 0x06, 0xf2, + /* Serial Number */ + 0x01, 0x00, 0x00, 0x00, + /* Date of manufacture */ + 0x01, 0x11, + /* EDID version */ + 0x01, 0x04, + /* Standard characteristics */ + 0x0f, 0x2b, 0x20, 0x78, 0x2b, 0x9c, 0x68, 0xa0, + 0x57, 0x4a, 0x9b, 0x26, 0x12, 0x48, 0x4c, 0xff, + 0xff, 0x80, 0xa9, 0x59, 0xa9, 0x4f, 0xa9, 0x4a, + 0xa9, 0x45, 0x81, 0x99, 0x81, 0x80, 0x61, 0x59, + 0x45, 0x59, + /* First 18-byte data block (Preferred Timing Mode) */ + 0x48, 0x3f, 0x40, 0x30, 0x62, 0xb0, 0x32, 0x40, + 0x40, 0xc0, 0x13, 0x00, 0xab, 0x40, 0x11, 0x00, + 0x00, 0x1e, + /* Second 18-byte data block */ + 0x00, 0x00, 0x00, 0xfd, 0x00, 0x32, 0x5a, 0x1e, + 0x6e, 0x17, 0x04, 0x11, 0x00, 0xc8, 0x90, 0x00, + 0x50, 0x3c, + /* Third 18-byte data block */ + 0x00, 0x00, 0x00, 0xf7, 0x00, 0x0a, 0xf7, 0x0f, + 0x03, 0x87, 0xc0, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, + /* Fourth 18-byte data block */ + 0x00, 0x00, 0x00, 0xfc, 0x00, 0x41, 0x42, 0x43, + 0x20, 0x4c, 0x43, 0x44, 0x32, 0x31, 0x0a, 0x20, + 0x20, 0x20, + /* Extension flag */ + 0x00, + }; + + *edid = heap_calloc( desc_size, 1 ); + if (!*edid) + { + *len = 0; + return; + } + + memcpy( *edid, default_edid, sizeof(default_edid) ); You can use sizeof(default_edid) - 1 since you don't need to copy the last byte.
Anyway, I am not feeling comfortable with the current approach for generating fake EDID data. It's not real enough. For example, physical size should reflect the recommended DPI values and the resolution list should match what's reported from EnumDisplaySettings().
In my opinion, you can either generate an EDID by carefully following the VESA EDID standard, or, the other way, which I think it's easier, is to pass the host primary monitor EDID to the virtual desktop by introducing a get_host_primary_monitor_edid(), similar to what get_host_primary_gpu() has done. This way has the benefit of not bothering with the EDID generation and has the same EDID as the host, which can reduce the difference between using virtual desktop and XRandR display device handlers. As for Xinerama, you can leave the EDID empty because Xinerama is rarely used nowadays.
Hi, Zhiyi. If we are passing the host monitor EDID, then there is little point in using the virtual desktop driver, since the app will not respect the constraints of the user-defined virtual resolution. As well, there could be some inconsistencies between the data derived from the EDID and data queried via APIs which do not rely on the EDID. Also, there can be some confusion with regards to which EDID to use when there are multiple physical monitors present. Generating EDID bit-by-bit from the spec will require quite a bit of tedious code, and it will not completely eliminate all instances of bogus data. Things like the resolution list have an associated refresh rate associated with the resolution, and it is not obvious what it should be. As well, there is a lot of low-level timing data that will simply have to be made up. Since the app that was the motivation for making this patch ignores most data in the EDID besides a couple of obvious fields, I also have no easy way to test such a change. And I expect that there will pretty much be 0 apps targeting Wine that will actually want to parse the EDID and do something meaningful with the data. Personally, I think the use case of using the virtual monitor driver with an app that wants to read an EDID is not that important. How about we merge patch 1 and drop patches 2 and 3?
+ *len = desc_size; + + /* Set the native resolution in the Preferred Timing Mode descriptor */ + (*edid)[0x38] = max_width & 0xff; + (*edid)[0x3a] = ((max_width >> 8) & 0xf) << 4; + + (*edid)[0x3b] = max_height & 0xff; + (*edid)[0x3d] = ((max_height >> 8) & 0xf) << 4; + + /* Set checksum */ + for (i = 0; i < desc_size-1; i++) + accum += (*edid)[i]; + (*edid)[desc_size - 1] = 256 - accum; +} + static BOOL X11DRV_desktop_get_modes( ULONG_PTR id, DWORD flags, DEVMODEW **new_modes, UINT *mode_count ) { UINT depth_idx, size_idx, mode_idx = 0; @@ -265,8 +334,7 @@ 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; + get_virtual_edid( &monitor->edid, &monitor->edid_len ); if (desktop_width && desktop_height) monitor->state_flags |= DISPLAY_DEVICE_ACTIVE;
@@ -277,6 +345,9 @@ static BOOL X11DRV_desktop_get_monitors( ULONG_PTR adapter_id, struct x11drv_mon
static void X11DRV_desktop_free_monitors( struct x11drv_monitor *monitors, int count ) { + int i; + for (i = 0; i < count; i++) + heap_free( monitors[i].edid ); heap_free( monitors ); }
On 9/11/21 12:28 AM, Eduard Permyakov wrote:
On 2021-09-08 6:42 p.m., Zhiyi Zhang wrote:
On 9/8/21 1:43 AM, Eduard Permyakov wrote:
Some applications use the EDID property of the monitor device to detect supported resolutions, and set the viewport size. Add a computed EDID to the virtual desktop monitor device to allow the virtual desktop driver to be better compatible with such applications.
Most fields of the EDID contain details that are of no relevance to applications, so populate only the native resolution and physical monitor dimensions. This should cover the vast majority of use cases.
Signed-off-by: Eduard Permyakov <epermyakov(a)codeweavers.com> --- dlls/winex11.drv/desktop.c | 75 +++++++++++++++++++++++++++++++++++++- 1 file changed, 73 insertions(+), 2 deletions(-)
diff --git a/dlls/winex11.drv/desktop.c b/dlls/winex11.drv/desktop.c index 71b3a0a5a27..37e5624967e 100644 --- a/dlls/winex11.drv/desktop.c +++ b/dlls/winex11.drv/desktop.c @@ -20,6 +20,7 @@ */ #include "config.h" +#include <stdint.h> #include <X11/cursorfont.h> #include <X11/Xlib.h> @@ -111,6 +112,74 @@ static void add_desktop_mode( DEVMODEW *mode, DWORD depth, DWORD width, DWORD he mode->dmDisplayFrequency = 60; } +static void get_virtual_edid( unsigned char **edid, unsigned long *len ) +{ + static const size_t desc_size = 128; You can remove desc_size entirely and use sizeof(default_edid) instead.
+ int i; + uint8_t accum = 0; + /*Sample EDID for an LCD Desktop IT Display, from VESA spec */ + static const unsigned char default_edid[128] = { + /* Header */ + 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, + /* Manufacturer's code name */ + 0x04, 0x43, + /* Product code */ + 0x06, 0xf2, + /* Serial Number */ + 0x01, 0x00, 0x00, 0x00, + /* Date of manufacture */ + 0x01, 0x11, + /* EDID version */ + 0x01, 0x04, + /* Standard characteristics */ + 0x0f, 0x2b, 0x20, 0x78, 0x2b, 0x9c, 0x68, 0xa0, + 0x57, 0x4a, 0x9b, 0x26, 0x12, 0x48, 0x4c, 0xff, + 0xff, 0x80, 0xa9, 0x59, 0xa9, 0x4f, 0xa9, 0x4a, + 0xa9, 0x45, 0x81, 0x99, 0x81, 0x80, 0x61, 0x59, + 0x45, 0x59, + /* First 18-byte data block (Preferred Timing Mode) */ + 0x48, 0x3f, 0x40, 0x30, 0x62, 0xb0, 0x32, 0x40, + 0x40, 0xc0, 0x13, 0x00, 0xab, 0x40, 0x11, 0x00, + 0x00, 0x1e, + /* Second 18-byte data block */ + 0x00, 0x00, 0x00, 0xfd, 0x00, 0x32, 0x5a, 0x1e, + 0x6e, 0x17, 0x04, 0x11, 0x00, 0xc8, 0x90, 0x00, + 0x50, 0x3c, + /* Third 18-byte data block */ + 0x00, 0x00, 0x00, 0xf7, 0x00, 0x0a, 0xf7, 0x0f, + 0x03, 0x87, 0xc0, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, + /* Fourth 18-byte data block */ + 0x00, 0x00, 0x00, 0xfc, 0x00, 0x41, 0x42, 0x43, + 0x20, 0x4c, 0x43, 0x44, 0x32, 0x31, 0x0a, 0x20, + 0x20, 0x20, + /* Extension flag */ + 0x00, + }; + + *edid = heap_calloc( desc_size, 1 ); + if (!*edid) + { + *len = 0; + return; + } + + memcpy( *edid, default_edid, sizeof(default_edid) ); You can use sizeof(default_edid) - 1 since you don't need to copy the last byte.
Anyway, I am not feeling comfortable with the current approach for generating fake EDID data. It's not real enough. For example, physical size should reflect the recommended DPI values and the resolution list should match what's reported from EnumDisplaySettings().
In my opinion, you can either generate an EDID by carefully following the VESA EDID standard, or, the other way, which I think it's easier, is to pass the host primary monitor EDID to the virtual desktop by introducing a get_host_primary_monitor_edid(), similar to what get_host_primary_gpu() has done. This way has the benefit of not bothering with the EDID generation and has the same EDID as the host, which can reduce the difference between using virtual desktop and XRandR display device handlers. As for Xinerama, you can leave the EDID empty because Xinerama is rarely used nowadays.
Hi, Zhiyi.
If we are passing the host monitor EDID, then there is little point in using the virtual desktop driver, since the app will not respect the constraints of the user-defined virtual resolution. As well, there could be some inconsistencies between the data derived from the EDID and data queried via APIs which do not rely on the EDID. Also, there can be some confusion with regards to which EDID to use when there are multiple physical monitors present.
Generating EDID bit-by-bit from the spec will require quite a bit of tedious code, and it will not completely eliminate all instances of bogus data. Things like the resolution list have an associated refresh rate associated with the resolution, and it is not obvious what it should be. As well, there is a lot of low-level timing data that will simply have to be made up. Since the app that was the motivation for making this patch ignores most data in the EDID besides a couple of obvious fields, I also have no easy way to test such a change. And I expect that there will pretty much be 0 apps targeting Wine that will actually want to parse the EDID and do something meaningful with the data.
Personally, I think the use case of using the virtual monitor driver with an app that wants to read an EDID is not that important. How about we merge patch 1 and drop patches 2 and 3?
Sure, we can work on it later. I sent a v6 of patch 1 to fix some minor style issue, check NULL before calling XFree() and fix some possible memory leaks. Thanks, Zhiyi
+ *len = desc_size; + + /* Set the native resolution in the Preferred Timing Mode descriptor */ + (*edid)[0x38] = max_width & 0xff; + (*edid)[0x3a] = ((max_width >> 8) & 0xf) << 4; + + (*edid)[0x3b] = max_height & 0xff; + (*edid)[0x3d] = ((max_height >> 8) & 0xf) << 4; + + /* Set checksum */ + for (i = 0; i < desc_size-1; i++) + accum += (*edid)[i]; + (*edid)[desc_size - 1] = 256 - accum; +} + static BOOL X11DRV_desktop_get_modes( ULONG_PTR id, DWORD flags, DEVMODEW **new_modes, UINT *mode_count ) { UINT depth_idx, size_idx, mode_idx = 0; @@ -265,8 +334,7 @@ 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; + get_virtual_edid( &monitor->edid, &monitor->edid_len ); if (desktop_width && desktop_height) monitor->state_flags |= DISPLAY_DEVICE_ACTIVE; @@ -277,6 +345,9 @@ static BOOL X11DRV_desktop_get_monitors( ULONG_PTR adapter_id, struct x11drv_mon static void X11DRV_desktop_free_monitors( struct x11drv_monitor *monitors, int count ) { + int i; + for (i = 0; i < count; i++) + heap_free( monitors[i].edid ); heap_free( monitors ); }
Signed-off-by: Eduard Permyakov <epermyakov(a)codeweavers.com> --- dlls/winex11.drv/xinerama.c | 77 ++++++++++++++++++++++++++++++++++++- 1 file changed, 75 insertions(+), 2 deletions(-) diff --git a/dlls/winex11.drv/xinerama.c b/dlls/winex11.drv/xinerama.c index 0e1e07669c2..03e56f250d8 100644 --- a/dlls/winex11.drv/xinerama.c +++ b/dlls/winex11.drv/xinerama.c @@ -23,6 +23,7 @@ #include <stdarg.h> #include <stdlib.h> +#include <stdint.h> #include <X11/Xlib.h> #ifdef HAVE_X11_EXTENSIONS_XINERAMA_H #include <X11/extensions/Xinerama.h> @@ -111,6 +112,76 @@ static int query_screens(void) return count; } +static void get_virtual_edid( RECT monitor, unsigned char **edid, unsigned long *len ) +{ + int i; + uint8_t accum = 0; + static const size_t desc_size = 128; + unsigned int width = monitor.right - monitor.left; + unsigned int height = monitor.bottom - monitor.top; + /*Sample EDID for an LCD Desktop IT Display, from VESA spec */ + static const unsigned char default_edid[128] = { + /* Header */ + 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, + /* Manufacturer's code name */ + 0x04, 0x43, + /* Product code */ + 0x06, 0xf2, + /* Serial Number */ + 0x01, 0x00, 0x00, 0x00, + /* Date of manufacture */ + 0x01, 0x11, + /* EDID version */ + 0x01, 0x04, + /* Standard characteristics */ + 0x0f, 0x2b, 0x20, 0x78, 0x2b, 0x9c, 0x68, 0xa0, + 0x57, 0x4a, 0x9b, 0x26, 0x12, 0x48, 0x4c, 0xff, + 0xff, 0x80, 0xa9, 0x59, 0xa9, 0x4f, 0xa9, 0x4a, + 0xa9, 0x45, 0x81, 0x99, 0x81, 0x80, 0x61, 0x59, + 0x45, 0x59, + /* First 18-byte data block (Preferred Timing Mode) */ + 0x48, 0x3f, 0x40, 0x30, 0x62, 0xb0, 0x32, 0x40, + 0x40, 0xc0, 0x13, 0x00, 0xab, 0x40, 0x11, 0x00, + 0x00, 0x1e, + /* Second 18-byte data block */ + 0x00, 0x00, 0x00, 0xfd, 0x00, 0x32, 0x5a, 0x1e, + 0x6e, 0x17, 0x04, 0x11, 0x00, 0xc8, 0x90, 0x00, + 0x50, 0x3c, + /* Third 18-byte data block */ + 0x00, 0x00, 0x00, 0xf7, 0x00, 0x0a, 0xf7, 0x0f, + 0x03, 0x87, 0xc0, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, + /* Fourth 18-byte data block */ + 0x00, 0x00, 0x00, 0xfc, 0x00, 0x41, 0x42, 0x43, + 0x20, 0x4c, 0x43, 0x44, 0x32, 0x31, 0x0a, 0x20, + 0x20, 0x20, + /* Extension flag */ + 0x00, + }; + + *edid = heap_calloc( desc_size, 1 ); + if (!*edid) + { + *len = 0; + return; + } + + memcpy( *edid, default_edid, sizeof(default_edid) ); + *len = desc_size; + + /* Set the native resolution in the Preferred Timing Mode descriptor */ + (*edid)[0x38] = width & 0xff; + (*edid)[0x3a] = ((width >> 8) & 0xf) << 4; + + (*edid)[0x3b] = height & 0xff; + (*edid)[0x3d] = ((height >> 8) & 0xf) << 4; + + /* Set checksum */ + for (i = 0; i < desc_size-1; i++) + accum += (*edid)[i]; + (*edid)[desc_size - 1] = 256 - accum; +} + #else /* SONAME_LIBXINERAMA */ static inline int query_screens(void) @@ -244,8 +315,7 @@ 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; + get_virtual_edid( monitors[i].rcMonitor, &monitor[index].edid, &monitor[index].edid_len ); if (!IsRectEmpty( &monitors[i].rcMonitor )) monitor[index].state_flags |= DISPLAY_DEVICE_ACTIVE; @@ -260,6 +330,9 @@ static BOOL xinerama_get_monitors( ULONG_PTR adapter_id, struct x11drv_monitor * static void xinerama_free_monitors( struct x11drv_monitor *monitors, int count ) { + int i; + for (i = 0; i < count; i++) + heap_free( monitors[i].edid ); heap_free( monitors ); } -- 2.25.1
participants (2)
-
Eduard Permyakov -
Zhiyi Zhang