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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 
> 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@xxxxxxxxxx>
> ---
>  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
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]