[Spice-devel] [PATCH 1/7] qxl-wddm-dod: Return EDID data to the OS

[Frediano Ziglio]

> 
> Solves failure of HLK "Test for EDID requirements"
> EDID contains capabilities and manufacturer data of
> the emulated display device. Main parameters are:
> Manufacturer code: QXL
> Product ID: 0001
> Working frequency: 75 Hz
> 
> Signed-off-by: Yuri Benditovich <yuri.benditovich at daynix.com>
> ---
>  qxldod/QxlDod.cpp | 50 ++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 48 insertions(+), 2 deletions(-)
> 
> diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
> index cb64209..35cfb68 100755
> --- a/qxldod/QxlDod.cpp
> +++ b/qxldod/QxlDod.cpp
> @@ -371,6 +371,42 @@ NTSTATUS QxlDod::QueryChildStatus(_Inout_
> DXGK_CHILD_STATUS* pChildStatus,
>      }
>  }
>  
> +static UCHAR edid[256] =

This should be also const

> +{
> +    0x00, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x00, \

Why do you concatenate the lines ?

> +    0x47, 0x0C, 0x01, 0x00, 0x41, 0xFA, 0x38, 0x78, \
> +    0xFF, 0x1B, 0x01, 0x04, 0x6A, 0x22, 0x1B, 0x78, \
> +    0xEA, 0x32, 0x31, 0xA3, 0x57, 0x4C, 0x9D, 0x25, \
> +    0x11, 0x50, 0x54, 0x04, 0x43, 0x00, 0x31, 0x4F, \
> +    0x45, 0x4F, 0x61, 0x4F, 0x81, 0x4F, 0x01, 0x01, \
> +    0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0xBA, 0x2C, \
> +    0x00, 0xA0, 0x50, 0x00, 0x25, 0x40, 0x30, 0x20, \
> +    0x37, 0x00, 0x80, 0x00, 0x10, 0x00, 0x00, 0x1E, \
> +    0x00, 0x00, 0x00, 0xFD, 0x00, 0x38, 0x50, 0x1E, \
> +    0x53, 0x0F, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
> +    0x00, 0x00, 0x00, 0x00, 0x00, 0xFC, 0x00, 0x51, \
> +    0x58, 0x4C, 0x30, 0x30, 0x30, 0x31, 0x0A, 0x20, \
> +    0x20, 0x20, 0x20, 0x20, 0x00, 0x00, 0x00, 0xFC, \
> +    0x00, 0x51, 0x58, 0x4C, 0x30, 0x30, 0x30, 0x31, \
> +    0x0A, 0x20, 0x20, 0x20, 0x20, 0x20, 0x01, 0x6E, \
> +    0x02, 0x03, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, \
> +    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
> +    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
> +    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
> +    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
> +    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
> +    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
> +    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
> +    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
> +    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
> +    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
> +    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
> +    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
> +    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
> +    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
> +    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xF7, \
> +};
> +
>  // EDID retrieval
>  NTSTATUS QxlDod::QueryDeviceDescriptor(_In_    ULONG
>  ChildUid,
>                                         _Inout_ DXGK_DEVICE_DESCRIPTOR*
>                                         pDeviceDescriptor)
> @@ -380,8 +416,18 @@ NTSTATUS QxlDod::QueryDeviceDescriptor(_In_    ULONG
> ChildUid,
>      QXL_ASSERT(pDeviceDescriptor != NULL);
>      QXL_ASSERT(ChildUid < MAX_CHILDREN);
>  
> -    DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s\n", __FUNCTION__));
> -    return STATUS_MONITOR_NO_MORE_DESCRIPTOR_DATA;
> +    if ((pDeviceDescriptor->DescriptorOffset +
> pDeviceDescriptor->DescriptorLength) > sizeof(edid))
> +    {
> +        DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s out of area\n",
> __FUNCTION__));
> +        return STATUS_MONITOR_NO_MORE_DESCRIPTOR_DATA;
> +    }
> +    else
> +    {
> +        PVOID src = ((PUCHAR)&edid[0]) +
> pDeviceDescriptor->DescriptorOffset;

No needs to cast edid, it's already the required type, I would go
with a

   auto src = edid + pDeviceDescriptor->DescriptorOffset;

> +        RtlMoveMemory(pDeviceDescriptor->DescriptorBuffer, src,
> pDeviceDescriptor->DescriptorLength);
> +        DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s\n", __FUNCTION__));
> +        return STATUS_SUCCESS;
> +    }
>  }
>  

About offset and length is not clear but I assume code is right.
What is not clear is what happens if the display port driver passed for
instance an offset == 0 and length == 1024. Your code just return error while
on https://msdn.microsoft.com/en-us/library/windows/hardware/ff561050(v=vs.85).aspx
it states "The display miniport driver must not write more than
DescriptorLength bytes to this buffer" making me think that in this case
our driver should write 256 bytes. Not clear however what our
miniport driver should do in this case with the remaining part of the buffer:
- nothing;
- fill with zeroes;
- set length (DescriptorLength) for the display port driver.

>  NTSTATUS QxlDod::QueryAdapterInfo(_In_ CONST DXGKARG_QUERYADAPTERINFO*
>  pQueryAdapterInfo)

Frediano