[Spice-devel] [PATCH win-vdagent v6 3/3] Implementing WDDM interface to support multiple monitors and arbitrary resolution
Sameeh Jubran
sameeh at daynix.com
Mon Aug 15 14:17:24 UTC 2016
On Mon, Aug 15, 2016 at 10:52 AM, Frediano Ziglio <fziglio at redhat.com>
wrote:
> >
> > The Direct3D 9 API operates on either the Windows XP display driver
> > model (XPDM) or the Windows Vista display driver model (WDDM), depending
> > on the operating system installed.
> >
> > This patch implements the WDDM interface while using the CCD API to do
> > so. Moreover it introduces multiple monitors support and arbitrary
> > resolution for Windows 10 while preserving backward compatiblity with
> > previous
> > versions of Windows.
> >
> > Based on a patch by Sandy Stutsman <sstutsma at redhat.com>
> >
> > Signed-off-by: Dmitry Fleytman <dfleytma at redhat.com>
> > Signed-off-by: Sameeh Jubran <sameeh at daynix.com>
> > ---
> > vdagent/display_configuration.cpp | 355
> > +++++++++++++++++++++++++++++++++++++-
> > vdagent/display_configuration.h | 47 +++++
> > 2 files changed, 401 insertions(+), 1 deletion(-)
> >
> > diff --git a/vdagent/display_configuration.cpp
> > b/vdagent/display_configuration.cpp
> > index 5e40d05..4db093f 100755
> > --- a/vdagent/display_configuration.cpp
> > +++ b/vdagent/display_configuration.cpp
> > @@ -152,6 +152,54 @@ struct DISPLAYCONFIG_PATH_INFO {
> > UINT32 flags;
> > };
> >
> > +
> > +enum D3DKMT_ESCAPETYPE {
> > + D3DKMT_ESCAPE_DRIVERPRIVATE = 0
> > +};
> > +
> > +struct D3DDDI_ESCAPEFLAGS {
> > + union {
> > + struct {
> > + UINT Reserved : 31;
> > + };
> > + UINT Value;
> > + };
> > +};
> > +
> > +struct D3DKMT_ESCAPE {
> > + D3D_HANDLE hAdapter;
> > + D3D_HANDLE hDevice;
> > + D3DKMT_ESCAPETYPE Type;
> > + D3DDDI_ESCAPEFLAGS Flags;
> > + VOID* pPrivateDriverData;
> > + UINT PrivateDriverDataSize;
> > + D3D_HANDLE hContext;
> > +};
> > +
> > +struct D3DKMT_OPENADAPTERFROMHDC {
> > + HDC hDc;
> > + D3D_HANDLE hAdapter;
> > + LUID AdapterLuid;
> > + UINT VidPnSourceId;
> > +};
> > +
> > +struct D3DKMT_CLOSEADAPTER {
> > + D3D_HANDLE hAdapter;
> > +};
> > +
> > +struct D3DKMT_OPENADAPTERFROMDEVICENAME {
> > + const WCHAR *pDeviceName;
> > + D3D_HANDLE hAdapter;
> > + LUID AdapterLuid;
> > +};
> > +
> > +struct D3DKMT_OPENADAPTERFROMGDIDISPLAYNAME {
> > + WCHAR DeviceName[32];
> > + D3D_HANDLE hAdapter;
> > + LUID AdapterLuid;
> > + UINT VidPnSourceId;
> > +};
> > +
>
> Where these definition came from ?
>
We have already discussed this in previous versions of the patches:
https://mail.google.com/mail/u/0/#search/in%3Asent+from+scratch/15650fbeba9cfd30
We have written from scratch the ones that we didn't find here:
https://github.com/notr1ch/DWMCapture/blob/master/DWMCaptureSource.cpp
> > struct QXLMonitorEscape {
> > QXLMonitorEscape(DEVMODE* dev_mode)
> > {
> > @@ -174,10 +222,43 @@ struct QxlCustomEscapeObj : public
> > QXLEscapeSetCustomDisplay {
> > QxlCustomEscapeObj() {};
> > };
> >
> > +struct WDDMCustomDisplayEscape {
> > + WDDMCustomDisplayEscape(DEVMODE* dev_mode)
> > + {
> > + _ioctl = QXL_ESCAPE_SET_CUSTOM_DISPLAY;
> > + _custom.bpp = dev_mode->dmBitsPerPel;
> > + _custom.xres = dev_mode->dmPelsWidth;
> > + _custom.yres = dev_mode->dmPelsHeight;
> > + }
> > + int _ioctl;
> > + QXLEscapeSetCustomDisplay _custom;
> > +};
> > +
> > +struct WDDMMonitorConfigEscape {
> > + WDDMMonitorConfigEscape(DisplayMode* mode)
> > + {
> > + _ioctl = QXL_ESCAPE_MONITOR_CONFIG;
> > + _head.id = _head.surface_id = 0;
> > + _head.x = mode->get_pos_x();
> > + _head.y = mode->get_pos_y();
> > + _head.width = mode->get_width();
> > + _head.height = mode->get_height();
> > + }
> > + int _ioctl;
> > + QXLHead _head;
> > +};
> > +
> > DisplayConfig* DisplayConfig::create_config()
> > {
> > DisplayConfig* new_interface;
> > - new_interface = new XPDMInterface();
> > + /* Try to open a WDDM adapter.
> > + If that failed, assume we have an XPDM driver */
> > + try {
> > + new_interface = new WDDMInterface();
> > + }
> > + catch (std::exception& e) {
> > + new_interface = new XPDMInterface();
> > + }
> > return new_interface;
> > }
> >
> > @@ -326,6 +407,278 @@ bool XPDMInterface::find_best_mode(LPCTSTR Device,
> > DEVMODE* dev_mode)
> > return NT_SUCCESS(status);
> > }
> >
> > +WDDMInterface::WDDMInterface()
> > + : _pfnOpen_adapter_hdc(NULL)
> > + , _pfnClose_adapter(NULL)
> > + , _pfnEscape(NULL)
> > + , _pfnOpen_adapter_device_name(NULL)
> > + , _pfnOpen_adapter_gdi_name(NULL)
> > +{
> > + LONG error(0);
> > + //Can we find the D3D calls we need?
> > + if (!init_d3d_api()) {
> > + throw std::exception();
> > + }
> > +
> > + //Initialize CCD path stuff
> > + if (!_ccd.query_display_config()) {
> > + throw std::exception();
> > + }
> > +
> > + if (!_ccd.set_display_config(error)) {
> > + throw std::exception();
> > + }
> > +}
> > +
> > +bool WDDMInterface::is_attached(DISPLAY_DEVICE* dev_info)
> > +{
> > + return _ccd.is_attached(dev_info->DeviceName);
> > +}
> > +
> > +bool WDDMInterface::set_monitor_state(LPCTSTR device_name, DEVMODE*
> > dev_mode, MONITOR_STATE state)
> > +{
> > + return _ccd.set_path_state(device_name, state);
> > +}
> > +
> > +bool WDDMInterface::custom_display_escape(LPCTSTR device_name, DEVMODE*
> > dev_mode)
> > +{
> > + DISPLAYCONFIG_MODE_INFO* mode = _ccd.get_active_mode(device_name,
> > false);
> > + if (!mode) {
> > + return false;
> > + }
> > +
> > + //Don't bother if we are already set to the new resolution
> > + if (mode->sourceMode.width == dev_mode->dmPelsWidth &&
> > + mode->sourceMode.height == dev_mode->dmPelsHeight) {
> > + return true;
> > + }
> > +
> > + vd_printf("%s: updating %S resolution\n", __FUNCTION__,
> device_name);
> > +
> > + WDDMCustomDisplayEscape wddm_escape(dev_mode);
> > + if (escape(device_name, &wddm_escape, sizeof(wddm_escape))) {
>
> This is not portable, not even between 32 and 64 bit.
>
Can you please further explain why?
>
> > + return _ccd.update_mode_size(device_name, dev_mode);
> > + }
> > +
> > + vd_printf("%s: (%dx%d)", __FUNCTION__, mode->sourceMode.width,
> > mode->sourceMode.height);
> > + return false;
> > +}
> > +
> > +bool WDDMInterface::update_monitor_config(LPCTSTR device_name,
> DisplayMode*
> > display_mode,
> > + DEVMODE* dev_mode)
> > +{
> > + if (!display_mode || !display_mode->get_attached()) {
> > + return false;
> > + }
> > + DISPLAYCONFIG_MODE_INFO* mode = _ccd.get_active_mode(device_name,
> > false);
> > + if (!mode || !_send_monitors_config)
> > + return false;
> > +
> > + WDDMMonitorConfigEscape wddm_escape(display_mode);
> > + if (escape(device_name, &wddm_escape, sizeof(wddm_escape))) {
>
> Same as above. It's not clear who is receiving these escape commands...
> the driver?
>
Yes the driver is receiving those escape commands.
> > + //Update the path position
> > + return _ccd.update_mode_position(device_name, dev_mode);
> > + }
> > +
> > + vd_printf("%s: %S failed", __FUNCTION__, device_name);
> > + return false;
> > +
> > +}
> > +
> > +LONG WDDMInterface::update_display_settings()
> > +{
> > + LONG error(0);
> > + //If we removed the primary monitor since the last call, we need to
> > + //reorder the other monitors, making the leftmost one the primary
> > + _ccd.verify_primary_position();
> > + _ccd.set_display_config(error);
> > + return error;
> > +}
> > +
> > +void WDDMInterface::update_config_path()
> > +{
> > + _ccd.query_display_config();
> > +}
> > +
> > +bool WDDMInterface::update_dev_mode_position(LPCTSTR device_name,
> DEVMODE*
> > dev_mode,
> > + LONG x, LONG y)
> > +{
> > + //Convert the position so that the primary is always at (0,0)
>
> This comment does not make any sense here.
>
> > + dev_mode->dmPosition.x = x;
> > + dev_mode->dmPosition.y = y;
> > + return _ccd.update_mode_position(device_name, dev_mode);
> > +}
> > +
> > +bool WDDMInterface::init_d3d_api()
> > +{
> > + HMODULE hModule = LoadLibrary(L"gdi32.dll");
> > +
> > + //Look for the gdi32 functions we need to perform driver escapes
> > + if (!hModule) {
> > + vd_printf("%s something wildly wrong as we can't open
> gdi32.dll",
> > __FUNCTION__);
> > + return false;
> > + }
> > +
> > + do {
> > + _pfnClose_adapter = (PFND3DKMT_CLOSEADAPTER)
> > + GetProcAddress(hModule, "D3DKMTCloseAdapter");
> > + if (!_pfnClose_adapter) {
> > + break;
> > + }
> > +
> > + _pfnEscape = (PFND3DKMT_ESCAPE) GetProcAddress(hModule,
> > "D3DKMTEscape");
> > + if (!_pfnEscape) {
> > + break;
> > + }
> > +
> > + _pfnOpen_adapter_hdc = (PFND3DKMT_OPENADAPTERFROMHDC)
> > + GetProcAddress(hModule, "D3DKMTOpenAdapterFromHdc");
> > + if (!_pfnOpen_adapter_hdc) {
> > + break;
> > + }
> > +
> > + _pfnOpen_adapter_device_name = (PFND3DKMT_
> OPENADAPTERFROMDEVICENAME)
> > + GetProcAddress(hModule, "D3DKMTOpenAdapterFromDeviceName");
> > + if (!_pfnOpen_adapter_device_name) {
> > + break;
> > + }
> > +
> > + _pfnOpen_adapter_gdi_name =
> > (PFND3DKMT_OPENADAPTERFROMGDIDISPLAYNAME)
> > + GetProcAddress(hModule, "D3DKMTOpenAdapterFromGdiDispla
> yName");
> > + if (!_pfnOpen_adapter_gdi_name) {
> > + break;
> > + }
> > +
> > + }
> > + while(0);
> > +
> > + FreeLibrary(hModule);
> > +
>
> If the library was loaded the program is going to crash as the function
> will disappear from memory.
>
I'll use GetModuleHandle instead.
> > + //Did we get them ?
> > + if (!_pfnClose_adapter || !_pfnOpen_adapter_hdc || !_pfnEscape) {
> > + return false;
> > + }
> > + return true;
> > +}
> > +
> > +D3D_HANDLE WDDMInterface::adapter_handle(LPCTSTR device_name)
> > +{
> > + D3D_HANDLE hAdapter(0);
> > +
> > + //For some reason, unknown to me, this call will occasionally fail.
>
> Who is the "me" here? Would be better to make it explicit.
>
> > + if ((hAdapter = handle_from_DC(device_name))) {
> > + return hAdapter;
> > + }
> > + //So try other available methods.
> > + if (_pfnOpen_adapter_device_name && (hAdapter =
> > handle_from_device_name(device_name))) {
> > + return hAdapter;
> > + }
> > + //One last chance to open this guy
> > + if (_pfnOpen_adapter_gdi_name) {
> > + hAdapter = handle_from_GDI_name(device_name);
> > + }
> > +
> > + if (!hAdapter) {
> > + vd_printf("%s: failed to open adapter %S", __FUNCTION__,
> > device_name);
> > + }
> > +
> > + return hAdapter;
> > +}
> > +
> > +D3D_HANDLE WDDMInterface::handle_from_DC(LPCTSTR adapter_name)
> > +{
> > + NTSTATUS status;
> > + D3DKMT_OPENADAPTERFROMHDC open_data;
> > + HDC hDc(CreateDC(adapter_name, NULL, NULL, NULL));
> > +
> > + if (!hDc) {
> > + vd_printf("%s: %S CreateDC failed with %lu", __FUNCTION__,
> > adapter_name, GetLastError());
> > + return 0;
> > + }
> > +
> > + ZeroMemory(&open_data, sizeof(D3DKMT_OPENADAPTERFROMHDC));
> > + open_data.hDc = hDc;
> > +
> > + if (!NT_SUCCESS(status = _pfnOpen_adapter_hdc(&open_data))) {
> > + vd_printf("%s: %S open adapter from hdc failed with %lu",
> > __FUNCTION__, adapter_name,
> > + status);
> > + open_data.hAdapter = 0;
> > + }
> > +
> > + DeleteDC(hDc);
> > + return open_data.hAdapter;
> > +}
> > +
> > +D3D_HANDLE WDDMInterface::handle_from_device_name(LPCTSTR adapter_name)
> > +{
> > + D3DKMT_OPENADAPTERFROMDEVICENAME display_name_data;
> > + NTSTATUS status;
> > +
> > + ZeroMemory(&display_name_data, sizeof(display_name_data));
> > + display_name_data.pDeviceName = adapter_name;
> > +
> > + if (NT_SUCCESS(status =
> > _pfnOpen_adapter_device_name(&display_name_data))) {
> > + return display_name_data.hAdapter;
> > + }
> > +
> > + vd_printf("%s %S failed with 0x%lx", __FUNCTION__, adapter_name,
> > status);
> > + return 0;
> > +}
> > +
> > +D3D_HANDLE WDDMInterface::handle_from_GDI_name(LPCTSTR adapter_name)
> > +{
> > + D3DKMT_OPENADAPTERFROMGDIDISPLAYNAME gdi_display_name;
> > + NTSTATUS status;
> > +
> > + ZeroMemory(&gdi_display_name, sizeof(gdi_display_name));
> > + memcpy((void *) gdi_display_name.DeviceName, adapter_name,
> > sizeof(TCHAR)* CCHDEVICENAME);
> > +
>
> This assume adapter_name is into a buffer of CCHDEVICENAME characters so
> can in theory overflow. As the destination buffer is already 0 filled
> would be better a strncpy style function. The void * conversion is useless
> and misleading.
> > + if (NT_SUCCESS(status = _pfnOpen_adapter_gdi_name(&gdi_display_name)))
> {
> > + return gdi_display_name.hAdapter;
> > + }
> > +
> > + vd_printf("%s: %S aurrrgghh nothing works..error is 0x%lx",
> > __FUNCTION__, adapter_name,
> > + status);
> > + return 0;
> > +}
> > +
> > +void WDDMInterface::close_adapter(D3D_HANDLE handle)
> > +{
> > + D3DKMT_CLOSEADAPTER closeData;
> > + if (handle) {
> > + closeData.hAdapter = handle;
> > + _pfnClose_adapter(&closeData);
> > + }
> > +}
> > +
> > +bool WDDMInterface::escape(LPCTSTR device_name, void* data, UINT
> size_data)
> > +{
> > + D3DKMT_ESCAPE escapeData;
> > + NTSTATUS status;
> > + D3D_HANDLE hAdapter(0);
> > +
> > + if (!(hAdapter = adapter_handle(device_name)))
> > + return false;
> > +
> > + escapeData.hAdapter = hAdapter;
> > + escapeData.hDevice = 0;
> > + escapeData.hContext = 0;
> > + escapeData.Type = D3DKMT_ESCAPE_DRIVERPRIVATE;
> > + escapeData.Flags.Value = 0;
> > + escapeData.pPrivateDriverData = data;
> > + escapeData.PrivateDriverDataSize = size_data;
> > +
> > + status = _pfnEscape(&escapeData);
> > +
> > + if (!NT_SUCCESS(status)) {
> > + vd_printf("%s: this should never happen. Status is 0x%lx",
> > __FUNCTION__, status);
> > + }
> > +
> > + //Close the handle to this device
> > + close_adapter(hAdapter);
> > + return NT_SUCCESS(status);
> > +}
> > +
> > CCD::CCD()
> > :_numPathElements(0)
> > ,_numModeElements(0)
> > diff --git a/vdagent/display_configuration.h
> > b/vdagent/display_configuration.h
> > index 05b35f4..e5ee90d 100755
> > --- a/vdagent/display_configuration.h
> > +++ b/vdagent/display_configuration.h
> > @@ -125,4 +125,51 @@ private:
> > bool find_best_mode(LPCTSTR Device, DEVMODE* dev_mode);
> > };
> >
> > +//DisplayConfig implementation for guest with WDDM graphics drivers
> > +typedef UINT D3D_HANDLE;
> > +
> > +struct D3DKMT_ESCAPE;
> > +struct D3DKMT_OPENADAPTERFROMGDIDISPLAYNAME;
> > +struct D3DKMT_OPENADAPTERFROMDEVICENAME;
> > +struct D3DKMT_CLOSEADAPTER;
> > +struct D3DKMT_OPENADAPTERFROMHDC;
> > +
> > +typedef NTSTATUS(APIENTRY* PFND3DKMT_ESCAPE)(CONST D3DKMT_ESCAPE*);
> > +typedef NTSTATUS(APIENTRY*
> > PFND3DKMT_OPENADAPTERFROMGDIDISPLAYNAME)(D3DKMT_
> OPENADAPTERFROMGDIDISPLAYNAME*);
> > +typedef NTSTATUS(APIENTRY*
> > PFND3DKMT_OPENADAPTERFROMDEVICENAME)(D3DKMT_OPENADAPTERFROMDEVICENAME*);
> > +typedef NTSTATUS(APIENTRY* PFND3DKMT_CLOSEADAPTER)(
> D3DKMT_CLOSEADAPTER*);
> > +typedef NTSTATUS(APIENTRY*
> > PFND3DKMT_OPENADAPTERFROMHDC)(D3DKMT_OPENADAPTERFROMHDC*);
> > +
> > +class WDDMInterface : public DisplayConfig {
> > +public:
> > + WDDMInterface();
> > + bool is_attached(DISPLAY_DEVICE* dev_info);
> > + bool set_monitor_state(LPCTSTR device_name, DEVMODE* dev_mode,
> > MONITOR_STATE state);
> > + LONG update_display_settings();
> > + bool custom_display_escape(LPCTSTR device_name, DEVMODE* dev_mode);
> > + bool update_monitor_config(LPCTSTR device_name, DisplayMode* mode,
> > DEVMODE* dev_mode);
> > + bool update_dev_mode_position(LPCTSTR device_name, DEVMODE *
> dev_mode,
> > LONG x, LONG y);
> > + void update_config_path();
> > +
> > +private:
> > + bool init_d3d_api();
> > + D3D_HANDLE adapter_handle(LPCTSTR device_name);
> > + D3D_HANDLE handle_from_DC(LPCTSTR adapter_name);
> > + D3D_HANDLE handle_from_device_name(LPCTSTR adapter_name);
> > + D3D_HANDLE handle_from_GDI_name(LPCTSTR adapter_name);
> > +
> > + void close_adapter(D3D_HANDLE handle);
> > + bool escape(LPCTSTR device_name, void* data, UINT sizeData);
> > +
> > + //GDI Function pointers
> > + PFND3DKMT_OPENADAPTERFROMHDC _pfnOpen_adapter_hdc;
> > + PFND3DKMT_CLOSEADAPTER _pfnClose_adapter;
> > + PFND3DKMT_ESCAPE _pfnEscape;
> > + PFND3DKMT_OPENADAPTERFROMDEVICENAME _pfnOpen_adapter_device_name;
> > + PFND3DKMT_OPENADAPTERFROMGDIDISPLAYNAME _pfnOpen_adapter_gdi_name;
> > +
> > + //object handles the CCD API
> > + CCD _ccd;
> > +};
> > +
> > #endif
> > \ No newline at end of file
>
> The update_ prefix is confusing. I would personally use
> set/get/query/read/write.
> The reason is that is not clear if they are updating a class internal
> state (so
> reading from the device) or updating the device state or a mix of the two.
>
> Frediano
>
--
Respectfully,
*Sameeh Jubran*
*Linkedin <https://il.linkedin.com/pub/sameeh-jubran/87/747/a8a>*
*Junior Software Engineer @ Daynix <http://www.daynix.com>.*
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160815/fbb92c20/attachment-0001.html>
More information about the Spice-devel
mailing list