[Spice-devel] [PATCH qxl-wddm-dod 11/26] Add arbitrary resolution and monitors_config Escape
Frediano Ziglio
fziglio at redhat.com
Tue Aug 30 15:04:18 UTC 2016
>
> From: Sandy Stutsman <sstutsma at redhat.com>
>
> Implement the escapes for the qxl driver and add some debug output for
> multiple monitor support
> ---
> qxldod/QxlDod.cpp | 162
> ++++++++++++++++++++++++++++++-------------
> qxldod/QxlDod.h | 11 ++-
> qxldod/include/qxl_windows.h | 1 +
> 3 files changed, 124 insertions(+), 50 deletions(-)
>
> diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
> index 0ee45a9..a1718a4 100755
> --- a/qxldod/QxlDod.cpp
> +++ b/qxldod/QxlDod.cpp
> @@ -47,6 +47,14 @@ BYTE PixelMask[BITS_PER_BYTE] = {0x80, 0x40, 0x20, 0x10,
> 0x08, 0x04, 0x02, 0x01
> ((ULONG)LOWER_5_BITS((Pixel)) <<
> SHIFT_LOWER_5_IN_565_BACK))
>
>
> +typedef struct _QXL_ESCAPE {
> + int ioctl;
> + union {
> + QXLEscapeSetCustomDisplay custom_display;
> + QXLHead monitor_config;
> + };
> +}QXL_ESCAPE;
> +
> #pragma code_seg(pop)
>
> #pragma code_seg("PAGE")
> @@ -474,19 +482,7 @@ NTSTATUS QxlDod::Escape(_In_ CONST DXGKARG_ESCAPE*
> pEscape)
> DbgPrint(TRACE_LEVEL_VERBOSE, ("<---> %s Flags = %d\n", __FUNCTION__,
> pEscape->Flags));
>
> Status = m_pHWDevice->Escape(pEscape);
> - if (Status == STATUS_SUCCESS)
> - {
> - DXGK_CHILD_STATUS ChildStatus;
> - ChildStatus.Type = StatusConnection;
> - ChildStatus.ChildUid = 0;
> - ChildStatus.HotPlug.Connected = FALSE;
> - Status =
> m_DxgkInterface.DxgkCbIndicateChildStatus(m_DxgkInterface.DeviceHandle,
> &ChildStatus);
> - if (Status == STATUS_SUCCESS)
> - {
> - ChildStatus.HotPlug.Connected = TRUE;
> - Status =
> m_DxgkInterface.DxgkCbIndicateChildStatus(m_DxgkInterface.DeviceHandle,
> &ChildStatus);
> - }
> - }
> +
Agreed to remove this code from here. Not every Escape require to update
child status. Note however that the behavior is changing.
This code set connected to FALSE and then TRUE while the new code just set
to TRUE.
According to https://msdn.microsoft.com/en-us/library/windows/hardware/ff559522(v=vs.85).aspx
the TRUE/FALSE should depend in the status of the monitor so there
should be some logic unless when we set a custom display we always assume
the monitor is connected, in this case the new behavior is correct.
> DbgPrint(TRACE_LEVEL_VERBOSE, ("<---> %s Status = %x\n", __FUNCTION__,
> Status));
> return Status;
> }
> @@ -613,7 +609,7 @@ NTSTATUS QxlDod::QueryVidPnHWCapability(_Inout_
> DXGKARG_QUERYVIDPNHWCAPABILITY*
> NTSTATUS QxlDod::IsSupportedVidPn(_Inout_ DXGKARG_ISSUPPORTEDVIDPN*
> pIsSupportedVidPn)
> {
> PAGED_CODE();
> - DbgPrint(TRACE_LEVEL_VERBOSE, ("---> %s\n", __FUNCTION__));
> + DbgPrint(TRACE_LEVEL_VERBOSE, ("---> %s %d\n", __FUNCTION__,
> m_pHWDevice->Id()));
>
> QXL_ASSERT(pIsSupportedVidPn != NULL);
>
> @@ -935,7 +931,7 @@ NTSTATUS QxlDod::EnumVidPnCofuncModality(_In_ CONST
> DXGKARG_ENUMVIDPNCOFUNCMODAL
> PAGED_CODE();
>
> QXL_ASSERT(pEnumCofuncModality != NULL);
> - DbgPrint(TRACE_LEVEL_VERBOSE, ("---> %s\n", __FUNCTION__));
> + DbgPrint(TRACE_LEVEL_VERBOSE, ("---> %s device %d\n", __FUNCTION__,
> m_pHWDevice->Id()));
>
> D3DKMDT_HVIDPNTOPOLOGY hVidPnTopology = 0;
> D3DKMDT_HVIDPNSOURCEMODESET hVidPnSourceModeSet = 0;
> @@ -1291,7 +1287,7 @@ NTSTATUS QxlDod::EnumVidPnCofuncModality(_In_ CONST
> DXGKARG_ENUMVIDPNCOFUNCMODAL
> NTSTATUS QxlDod::SetVidPnSourceVisibility(_In_ CONST
> DXGKARG_SETVIDPNSOURCEVISIBILITY* pSetVidPnSourceVisibility)
> {
> PAGED_CODE();
> - DbgPrint(TRACE_LEVEL_VERBOSE, ("---> %s\n", __FUNCTION__));
> + DbgPrint(TRACE_LEVEL_VERBOSE, ("---> %s %d\n", __FUNCTION__,
> m_pHWDevice->Id()));
> QXL_ASSERT(pSetVidPnSourceVisibility != NULL);
> QXL_ASSERT((pSetVidPnSourceVisibility->VidPnSourceId < MAX_VIEWS) ||
> (pSetVidPnSourceVisibility->VidPnSourceId == D3DDDI_ID_ALL));
> @@ -3120,17 +3116,20 @@ NTSTATUS QxlDevice::QueryCurrentMode(PVIDEO_MODE
> RequestedMode)
>
> NTSTATUS QxlDevice::SetCurrentMode(ULONG Mode)
> {
> - DbgPrint(TRACE_LEVEL_VERBOSE, ("---> %s Mode = %x\n", __FUNCTION__,
> Mode));
> + DbgPrint(TRACE_LEVEL_INFORMATION, ("---> %s - %d: Mode = %d\n",
> __FUNCTION__, m_Id, Mode));
> for (ULONG idx = 0; idx < GetModeCount(); idx++)
> {
> if (Mode == m_ModeNumbers[idx])
> {
> DestroyPrimarySurface();
> CreatePrimarySurface(&m_ModeInfo[idx]);
> + DbgPrint(TRACE_LEVEL_INFORMATION, ("%s device %d: setting
> current mode %d (%d x %d)\n",
> + __FUNCTION__, m_Id, Mode, m_ModeInfo[idx].VisScreenWidth,
> + m_ModeInfo[idx].VisScreenHeight));
> return STATUS_SUCCESS;
> }
> }
> - DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s\n", __FUNCTION__));
> + DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s failed\n", __FUNCTION__));
> return STATUS_UNSUCCESSFUL;
> }
>
> @@ -3328,6 +3327,7 @@ NTSTATUS QxlDevice::QxlInit(DXGK_DISPLAY_INFORMATION*
> pDispInfo)
> m_RamHdr->int_mask = WIN_QXL_INT_MASK;
> CreateMemSlots();
> InitDeviceMemoryResources();
> + InitMonitorConfig();
> return Status;
> }
>
> @@ -3393,7 +3393,8 @@ void QxlDevice::DestroyMemSlots(void)
> void QxlDevice::CreatePrimarySurface(PVIDEO_MODE_INFORMATION pModeInfo)
> {
> QXLSurfaceCreate *primary_surface_create;
> - DbgPrint(TRACE_LEVEL_VERBOSE, ("---> %s\n", __FUNCTION__));
> + DbgPrint(TRACE_LEVEL_INFORMATION, ("---> %s - %d: (%d x %d)\n",
> __FUNCTION__, m_Id,
> + pModeInfo->VisScreenWidth, pModeInfo->VisScreenHeight));
> primary_surface_create = &m_RamHdr->create_surface;
> primary_surface_create->format = pModeInfo->BitsPerPlane;
> primary_surface_create->width = pModeInfo->VisScreenWidth;
> @@ -3549,6 +3550,17 @@ void QxlDevice::InitDeviceMemoryResources(void)
> DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s\n", __FUNCTION__));
> }
>
> +void QxlDevice::InitMonitorConfig(void)
> +{
> + size_t config_size = sizeof(QXLMonitorsConfig) + sizeof(QXLHead);
> + m_monitor_config = (QXLMonitorsConfig*) AllocMem(MSPACE_TYPE_DEVRAM,
> config_size, TRUE);
> + RtlZeroMemory(m_monitor_config, config_size);
> +
> + m_monitor_config_pa = &m_RamHdr->monitors_config;
> + *m_monitor_config_pa = PA(m_monitor_config, m_MainMemSlot);
> +}
> +
> +
I assume this m_monitor_config_pa will be used in some following patches.
> void QxlDevice::InitMspace(UINT32 mspace_type, UINT8 *start, size_t
> capacity)
> {
> DbgPrint(TRACE_LEVEL_VERBOSE, ("---> %s type = %d, start = %p, capacity
> = %d\n", __FUNCTION__, mspace_type, start, capacity));
> @@ -4142,7 +4154,7 @@ VOID QxlDevice::BltBits (
> LONG width;
> LONG height;
>
> - DbgPrint(TRACE_LEVEL_VERBOSE, ("---> %s\n", __FUNCTION__));
> + DbgPrint(TRACE_LEVEL_VERBOSE, ("---> %s device %d\n",
> __FUNCTION__,m_Id));
> UNREFERENCED_PARAMETER(NumRects);
> UNREFERENCED_PARAMETER(pDst);
>
> @@ -4403,45 +4415,97 @@ NTSTATUS QxlDevice::SetPointerPosition(_In_ CONST
> DXGKARG_SETPOINTERPOSITION* pS
> return STATUS_SUCCESS;
> }
>
> -NTSTATUS QxlDevice::Escape(_In_ CONST DXGKARG_ESCAPE* pEscap)
> +NTSTATUS QxlDevice::UpdateChildStatus(BOOLEAN connect)
> {
> - DbgPrint(TRACE_LEVEL_VERBOSE, ("---> %s\n", __FUNCTION__));
> - QXLEscapeSetCustomDisplay *custom_display;
> - UINT xres;
> - UINT yres;
> - UINT bpp;
> + NTSTATUS Status(STATUS_SUCCESS);
> + DXGK_CHILD_STATUS ChildStatus;
> + PDXGKRNL_INTERFACE pDXGKInterface(m_pQxlDod->GetDxgkInterface());
>
> - if (pEscap->PrivateDriverDataSize != sizeof(QXLEscapeSetCustomDisplay))
> {
> - DbgPrint(TRACE_LEVEL_ERROR, ("<--> %s Incorrect buffer size %d
> instead of %d\n", __FUNCTION__, pEscap->PrivateDriverDataSize,
> sizeof(QXLEscapeSetCustomDisplay)));
> - return STATUS_INVALID_BUFFER_SIZE;
> - }
> - custom_display = (QXLEscapeSetCustomDisplay*)pEscap->pPrivateDriverData;
> - xres = custom_display->xres & ~0x3;
> - yres = custom_display->yres & ~0x3;
New code does not round xres/yres, agreed the change.
> - bpp = custom_display->bpp;
> - if (bpp != QXL_BPP)
> - {
> - bpp = QXL_BPP;
> - }
> - if (xres < MIN_WIDTH_SIZE || yres < MIN_HEIGHT_SIZE)
> - {
> - DbgPrint(TRACE_LEVEL_ERROR, ("%s: xres = %d, yres = %d\n",
> __FUNCTION__, xres, yres));
> + ChildStatus.Type = StatusConnection;
> + ChildStatus.ChildUid = 0;
> + ChildStatus.HotPlug.Connected = connect;
> + Status =
> pDXGKInterface->DxgkCbIndicateChildStatus(pDXGKInterface->DeviceHandle,
> &ChildStatus);
> + return Status;
> +}
> +
> +NTSTATUS QxlDevice::SetCustomDisplay(QXLEscapeSetCustomDisplay*
> custom_display)
> +{
> + NTSTATUS status;
> + UINT xres = custom_display->xres;
> + UINT yres = custom_display->yres;
> + UINT bpp = QXL_BPP;
> + DbgPrint(TRACE_LEVEL_ERROR, ("%s - %d (%dx%d#%d)\n", __FUNCTION__, m_Id,
> xres, yres, bpp));
> + if (xres < MIN_WIDTH_SIZE || yres < MIN_HEIGHT_SIZE) {
> + DbgPrint(TRACE_LEVEL_VERBOSE, ("%s: (%dx%d#%d) must be at least
> (%dxd)\n", __FUNCTION__,
> + xres, yres, bpp, MIN_WIDTH_SIZE, MIN_HEIGHT_SIZE));
> return ERROR_INVALID_DATA;
> }
> -
> - if (m_CustomMode == (m_ModeCount - 1))
> - m_CustomMode = (USHORT)(m_ModeCount - 2);
> - else
> - m_CustomMode = (USHORT)(m_ModeCount - 1);
> + m_CustomMode =(USHORT) ((m_CustomMode == m_ModeCount-1)? m_ModeCount -
> 2 : m_ModeCount - 1);
>
> if ((xres * yres * bpp / 8) > m_RomHdr->surface0_area_size) {
> DbgPrint(TRACE_LEVEL_ERROR, ("%s: Mode (%dx%d#%d) doesn't fit in
> memory (%d)\n",
> __FUNCTION__, xres, yres, bpp,
> m_RomHdr->surface0_area_size));
> - return STATUS_INVALID_PARAMETER;
> + return ERROR_NOT_ENOUGH_MEMORY;
Also agreed this change.
> }
> UpdateVideoModeInfo(m_CustomMode, xres, yres, bpp);
> - DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s\n", __FUNCTION__));
> - return STATUS_SUCCESS;
> + status = UpdateChildStatus(TRUE);
> + return status;
> +}
> +
> +void QxlDevice::SetMonitorConfig(QXLHead * monitor_config)
> +{
> + m_monitor_config->count = 1;
> + m_monitor_config->max_allowed = 1;
> +
> + memcpy(&m_monitor_config->heads[0], monitor_config, sizeof(QXLHead));
> + m_monitor_config->heads[0].id = 0;
> + m_monitor_config->heads[0].surface_id = 0;
> +
> + DbgPrint(TRACE_LEVEL_VERBOSE, ("%s:%d configuring monitor at (%d, %d)
> (%dx%d)\n", __FUNCTION__, m_Id,
> + m_monitor_config->heads[0].x, m_monitor_config->heads[0].y,
> + m_monitor_config->heads[0].width,
> m_monitor_config->heads[0].height));
> + AsyncIo(QXL_IO_MONITORS_CONFIG_ASYNC, 0);
> +}
> +
> +NTSTATUS QxlDevice::Escape(_In_ CONST DXGKARG_ESCAPE* pEscape)
> +{
> + size_t data_size(sizeof(int));
> + QXL_ESCAPE* pQXLEscape((QXL_ESCAPE*) pEscape->pPrivateDriverData);
> + NTSTATUS status(STATUS_SUCCESS);
> +
> + DbgPrint(TRACE_LEVEL_VERBOSE, ("---> %s\n", __FUNCTION__));
> +
> + switch (pQXLEscape->ioctl) {
> + case QXL_ESCAPE_SET_CUSTOM_DISPLAY: {
> + data_size += sizeof(QXLEscapeSetCustomDisplay);
> + if (pEscape->PrivateDriverDataSize != data_size) {
> + status = STATUS_INVALID_BUFFER_SIZE;
> + break;
> + }
> + status = SetCustomDisplay(&pQXLEscape->custom_display);
> + break;
> + }
> + case QXL_ESCAPE_MONITOR_CONFIG: {
> + data_size += sizeof(QXLHead);
> + if (pEscape->PrivateDriverDataSize != data_size) {
> + status = STATUS_INVALID_BUFFER_SIZE;
> + break;
> + }
> + SetMonitorConfig(&pQXLEscape->monitor_config);
> + status = STATUS_SUCCESS;
> + break;
> + }
> + default:
> + DbgPrint(TRACE_LEVEL_ERROR, ("%s: invalid Escape 0x%x\n",
> __FUNCTION__, pQXLEscape->ioctl));
> + status = STATUS_INVALID_PARAMETER;
> + }
> +
> + if (status == STATUS_INVALID_BUFFER_SIZE) {
> + DbgPrint(TRACE_LEVEL_ERROR, ("%s invalid buffer size of %d, should
> be %d\n", __FUNCTION__,
> + pEscape->PrivateDriverDataSize, data_size));
> + }
> +
> + return status;
> }
>
> VOID QxlDevice::WaitForCmdRing()
> diff --git a/qxldod/QxlDod.h b/qxldod/QxlDod.h
> index db343e9..8ecae19 100755
> --- a/qxldod/QxlDod.h
> +++ b/qxldod/QxlDod.h
> @@ -1,6 +1,7 @@
> #pragma once
> #include "baseobject.h"
> #include "qxl_dev.h"
> +#include "qxl_windows.h"
> #include "mspace.h"
>
> #define MAX_CHILDREN 1
> @@ -239,7 +240,7 @@ public:
> BOOLEAN InterruptRoutine(_In_ PDXGKRNL_INTERFACE pDxgkInterface, _In_
> ULONG MessageNumber);;
> VOID DpcRoutine(_In_ PDXGKRNL_INTERFACE pDxgkInterface);
> VOID ResetDevice(void);;
> -
> + ULONG Id() { return m_Id; };
This should be
ULONG Id() const { return m_Id; };
however considering that there is also a
ULONG GetId(void) { return m_Id; }
This change should be removed and Id call should be replaced by GetId.
> PVIDEO_MODE_INFORMATION GetModeInfo(UINT idx) {return &m_ModeInfo[idx];}
> USHORT GetModeNumber(USHORT idx) {return m_ModeNumbers[idx];}
> USHORT GetCurrentModeIndex(void) {return m_CurrentMode;}
> @@ -519,6 +520,7 @@ private:
> UINT64 VA(QXLPHYSICAL paddr, UINT8 slot_id);
> QXLPHYSICAL PA(PVOID virt, UINT8 slot_id);
> void InitDeviceMemoryResources(void);
> + void InitMonitorConfig();
> void InitMspace(UINT32 mspace_type, UINT8 *start, size_t capacity);
> void FlushReleaseRing();
> void FreeMem(UINT32 mspace_type, void *ptr);
> @@ -546,6 +548,10 @@ private:
> void DpcCallback(PDPC_CB_CONTEXT);
> void AsyncIo(UCHAR Port, UCHAR Value);
> void SyncIo(UCHAR Port, UCHAR Value);
> + NTSTATUS UpdateChildStatus(BOOLEAN connect);
> + NTSTATUS SetCustomDisplay(QXLEscapeSetCustomDisplay* custom_display);
> + void SetMonitorConfig(QXLHead* monitor_config);
> +
> private:
> PUCHAR m_IoBase;
> BOOLEAN m_IoMapped;
> @@ -590,6 +596,9 @@ private:
>
> UINT64 m_FreeOutputs;
> UINT32 m_Pending;
> +
> + QXLMonitorsConfig* m_monitor_config;
> + QXLPHYSICAL* m_monitor_config_pa;
> };
>
> class QxlDod {
> diff --git a/qxldod/include/qxl_windows.h b/qxldod/include/qxl_windows.h
> index 237ffd8..1f97fb7 100755
> --- a/qxldod/include/qxl_windows.h
> +++ b/qxldod/include/qxl_windows.h
> @@ -3,6 +3,7 @@
>
> enum {
> QXL_ESCAPE_SET_CUSTOM_DISPLAY = 0x10001,
> + QXL_ESCAPE_MONITOR_CONFIG,
> };
>
> typedef struct QXLEscapeSetCustomDisplay {
Consider this patch reviewed
I have some doubt about disabling monitor, not clear what
should happen.
Frediano
More information about the Spice-devel
mailing list