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

[Yuri Benditovich]

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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20190128/efb32a18/attachment.html>