[Spice-devel] [PATCH] qxl-wddm-dod: Load best know defaults for video mode at driver's start

[Marek Kedzierski]

Hi Yuri,

Did you check how BasicDisplay.sys driver is getting default (initial) display mode information on UEFI systems?

I did a check on both configurations: bare metal UEFI machine and VM (Windows 10 1803 x86 with OVMF).
BasicDisplay!BASIC_DISPLAY_DRIVER::StartDevice is the method that is preparing this information.
I can observe (with windbg) that it is filling PDXGK_DISPLAY_INFORMATION structure with correct data (width 800, height 600, pitch 3200, etc.)
Anyway the mentioned method uses undocumented dxgkrn call (dxgkrn!DpGetPostDisplayInfoPlusEdid).

So in any case, it is required to use undocumented methods.

Marek 


----- Original Message -----
From: "Yuri Benditovich" <yuri.benditovich at daynix.com>
To: "Christophe Fergeau" <cfergeau at redhat.com>
Cc: "Yan Vugenfirer" <yan at daynix.com>, "Spice List" <spice-devel at lists.freedesktop.org>
Sent: Monday, January 28, 2019 1:47:19 PM
Subject: Re: [Spice-devel] [PATCH] qxl-wddm-dod: Load best know defaults for video mode at driver's start



On Mon, Jan 28, 2019 at 2:27 PM Christophe Fergeau < cfergeau at redhat.com > wrote: 


Hey, 

On Fri, Jan 25, 2019 at 09:48:52AM +0200, Yuri Benditovich wrote: 
> Even if initial display resolution is not available at driver start, try 
> to find it in the registry. 

> Then the driver can prevent black screen 
> on uninstall/disable also when it was installed on UEFI machine after 
> the production driver 0.18 which did not report valid video mode. 

I'm not sure I understand the problem you are trying to describe here. 
"Then the driver can prevent black screen on uninstall/disable" -> is 
this saying on non-UEFI system, we still have a black screen issue with 
the driver in git master? 

"also when it was installed on UEFI machine after the production driver 
0.18 which did not report valid video mode." -> I assume this part is 
related to the commit "Fix for black screen on driver uninstall on ovmf 
platform"? If yes, isn't the issue that you describe already fixed by 
that commit? 


All the scenarious below are related to ovmf platform 

Flow 1: the driver from current master branch installed on the machine where no qxl-wddm-dod driver installed 
(or previous one uninstalled and reboot done) 
The driver can be uninstalled without problem (this what was fixed by previous commit). 

Flow 2: there is current production driver (0.18) installed. Driver update 0.18->current master works OK. 
(the driver does not know video mode on startup but it can live without it) 
Uninstalling the driver still causes black screen (or broken screen), as the driver does not know which mode to report on exit. 
If it even reports some default, it might be wrong due to various reasons (will cause broken screen, which is not better than black). 
This is what current commit fixes. 



> 
> Signed-off-by: Yuri Benditovich < yuri.benditovich at daynix.com > 
> --- 
> qxldod/QxlDod.cpp | 37 ++++++++++++++++++++++++++++++++----- 
> 1 file changed, 32 insertions(+), 5 deletions(-) 
> 
> diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp 
> index dea78e2..525cdc3 100755 
> --- a/qxldod/QxlDod.cpp 
> +++ b/qxldod/QxlDod.cpp 
> @@ -5169,6 +5169,37 @@ UINT SpiceFromPixelFormat(D3DDDIFORMAT Format) 
> } 
> } 
> 
> +// Width and Height values for initial video mode are populated by 
> +// display class driver upon switch from boot display to operational one. 
> +// This is not documented and can be changed in future, but 
> +// present under the same key in Win8.1/Win10/2016/2019 
> +static void RetrieveDisplayDefaults(DXGK_DISPLAY_INFORMATION& DispInfo) 
> +{ 
> + PAGED_CODE(); 
> + RTL_QUERY_REGISTRY_TABLE QueryTable[3] = {}; 
> + QueryTable[0].Flags = QueryTable[1].Flags = RTL_QUERY_REGISTRY_DIRECT | RTL_QUERY_REGISTRY_TYPECHECK | RTL_QUERY_REGISTRY_REQUIRED; 
> + QueryTable[0].DefaultType = QueryTable[1].DefaultType = REG_DWORD << 24; 
> + QueryTable[0].Name = L"Height"; 
> + QueryTable[0].EntryContext = &DispInfo.Height; 
> + QueryTable[1].Name = L"Width"; 
> + QueryTable[1].EntryContext = &DispInfo.Width; 
> + 
> + NTSTATUS status = RtlQueryRegistryValues(RTL_REGISTRY_CONTROL, L"BGFX", QueryTable, NULL, NULL); 
> + if (NT_SUCCESS(status)) 
> + { 
> + DbgPrint(TRACE_LEVEL_INFORMATION, ("%s: %dx%d\n", __FUNCTION__, DispInfo.Width, DispInfo.Height)); 
> + } 
> + else 
> + { 
> + DbgPrint(TRACE_LEVEL_INFORMATION, ("%s: status = %X\n", __FUNCTION__, status)); 
> + DispInfo.Width = INITIAL_WIDTH; 
> + DispInfo.Height = INITIAL_HEIGHT; 
> + } 
> + DispInfo.ColorFormat = D3DDDIFMT_X8R8G8B8; 

By any chance, is this stored in the registry as well? 

Proper video mode is stored in the registry by the OS. Undocumented, however. 
This does not help to store it again, the point is to query _proper_ video mode. 




Apart from this, looks good to me. 

Christophe 

> + DispInfo.Pitch = DispInfo.Width * BPPFromPixelFormat(DispInfo.ColorFormat) / BITS_PER_BYTE; 
> + DispInfo.TargetId = DispInfo.AcpiId = 0; 
> +} 
> + 
> NTSTATUS HwDeviceInterface::AcquireDisplayInfo(DXGK_DISPLAY_INFORMATION& DispInfo) 
> { 
> PAGED_CODE(); 
> @@ -5188,11 +5219,7 @@ NTSTATUS HwDeviceInterface::AcquireDisplayInfo(DXGK_DISPLAY_INFORMATION& DispInf 
> if (DispInfo.Width == 0) 
> { 
> DbgPrint(TRACE_LEVEL_WARNING, ("QxlDod::AcquireDisplayInfo has zero width!\n")); 
> - DispInfo.ColorFormat = D3DDDIFMT_A8R8G8B8; 
> - DispInfo.Width = INITIAL_WIDTH; 
> - DispInfo.Height = INITIAL_HEIGHT; 
> - DispInfo.Pitch = DispInfo.Width * BPPFromPixelFormat(DispInfo.ColorFormat) / BITS_PER_BYTE; 
> - DispInfo.TargetId = 0; 
> + RetrieveDisplayDefaults(DispInfo); 
> } 
> return Status; 
> } 
> -- 
> 2.16.1.windows.4 
> 
> _______________________________________________ 
> Spice-devel mailing list 
> Spice-devel at lists.freedesktop.org 
> https://lists.freedesktop.org/mailman/listinfo/spice-devel 

_______________________________________________
Spice-devel mailing list
Spice-devel at lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel