[Spice-devel] qxl-wddm-dod: Fix for screen not shown after driver disabled

Submitted by Yuri Benditovich on Jan. 25, 2017, 3:55 p.m.

Details

Message ID 1485359736-153144-1-git-send-email-yuri.benditovich@daynix.com
State New
Headers show
Series "qxl-wddm-dod: Fix for screen not shown after driver disabled" ( rev: 1 ) in Spice

Not browsing as part of any series.

Commit Message

Yuri Benditovich Jan. 25, 2017, 3:55 p.m.
https://bugzilla.redhat.com/show_bug.cgi?id=1411340
Do not set SupportNonVGA field for device rev.4 and higher.
Then the class driver will not call miniport's procedure
StopDeviceAndReleasePostDisplayOwnership upon PnP stop.
QXL device modes are not compatible with VGA ones; the
driver in this procedure returns display information related
to QXL mode which later used by BasicDisplay driver and causes
it to show the screen incorrectly or fail to show it at all.

Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
---
 qxldod/QxlDod.cpp | 2 +-
 qxldod/QxlDod.h   | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
index 7adbec4..001cd67 100755
--- a/qxldod/QxlDod.cpp
+++ b/qxldod/QxlDod.cpp
@@ -411,7 +411,7 @@  NTSTATUS QxlDod::QueryAdapterInfo(_In_ CONST DXGKARG_QUERYADAPTERINFO* pQueryAda
             pDriverCaps->PointerCaps.Monochrome = 1;
             pDriverCaps->PointerCaps.Color = 1;
 
-            pDriverCaps->SupportNonVGA = TRUE;
+            pDriverCaps->SupportNonVGA = m_pHWDevice->IsBIOSCompatible();
 
             DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s 1\n", __FUNCTION__));
             return STATUS_SUCCESS;
diff --git a/qxldod/QxlDod.h b/qxldod/QxlDod.h
index 4df4c61..8df1fcf 100755
--- a/qxldod/QxlDod.h
+++ b/qxldod/QxlDod.h
@@ -263,6 +263,7 @@  public:
     virtual NTSTATUS Escape(_In_ CONST DXGKARG_ESCAPE* pEscap) = 0;
     NTSTATUS AcquireDisplayInfo(DXGK_DISPLAY_INFORMATION& DispInfo);
     ULONG GetId(void) { return m_Id; }
+    virtual BOOLEAN IsBIOSCompatible() { return TRUE; }
 protected:
     virtual NTSTATUS GetModeList(DXGK_DISPLAY_INFORMATION* pDispInfo) = 0;
 protected:
@@ -480,6 +481,7 @@  public:
     NTSTATUS SetPointerShape(_In_ CONST DXGKARG_SETPOINTERSHAPE* pSetPointerShape);
     NTSTATUS SetPointerPosition(_In_ CONST DXGKARG_SETPOINTERPOSITION* pSetPointerPosition);
     NTSTATUS Escape(_In_ CONST DXGKARG_ESCAPE* pEscap);
+    BOOLEAN IsBIOSCompatible() { return FALSE; }
 protected:
     NTSTATUS GetModeList(DXGK_DISPLAY_INFORMATION* pDispInfo);
     VOID BltBits (BLT_INFO* pDst,

Comments

> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1411340
> Do not set SupportNonVGA field for device rev.4 and higher.
> Then the class driver will not call miniport's procedure
> StopDeviceAndReleasePostDisplayOwnership upon PnP stop.
> QXL device modes are not compatible with VGA ones; the
> driver in this procedure returns display information related
> to QXL mode which later used by BasicDisplay driver and causes
> it to show the screen incorrectly or fail to show it at all.
> 
> Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
> ---
>  qxldod/QxlDod.cpp | 2 +-
>  qxldod/QxlDod.h   | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
> index 7adbec4..001cd67 100755
> --- a/qxldod/QxlDod.cpp
> +++ b/qxldod/QxlDod.cpp
> @@ -411,7 +411,7 @@ NTSTATUS QxlDod::QueryAdapterInfo(_In_ CONST
> DXGKARG_QUERYADAPTERINFO* pQueryAda
>              pDriverCaps->PointerCaps.Monochrome = 1;
>              pDriverCaps->PointerCaps.Color = 1;
>  
> -            pDriverCaps->SupportNonVGA = TRUE;
> +            pDriverCaps->SupportNonVGA = m_pHWDevice->IsBIOSCompatible();
>  
>              DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s 1\n", __FUNCTION__));
>              return STATUS_SUCCESS;
> diff --git a/qxldod/QxlDod.h b/qxldod/QxlDod.h
> index 4df4c61..8df1fcf 100755
> --- a/qxldod/QxlDod.h
> +++ b/qxldod/QxlDod.h
> @@ -263,6 +263,7 @@ public:
>      virtual NTSTATUS Escape(_In_ CONST DXGKARG_ESCAPE* pEscap) = 0;
>      NTSTATUS AcquireDisplayInfo(DXGK_DISPLAY_INFORMATION& DispInfo);
>      ULONG GetId(void) { return m_Id; }
> +    virtual BOOLEAN IsBIOSCompatible() { return TRUE; }
>  protected:
>      virtual NTSTATUS GetModeList(DXGK_DISPLAY_INFORMATION* pDispInfo) = 0;
>  protected:
> @@ -480,6 +481,7 @@ public:
>      NTSTATUS SetPointerShape(_In_ CONST DXGKARG_SETPOINTERSHAPE*
>      pSetPointerShape);
>      NTSTATUS SetPointerPosition(_In_ CONST DXGKARG_SETPOINTERPOSITION*
>      pSetPointerPosition);
>      NTSTATUS Escape(_In_ CONST DXGKARG_ESCAPE* pEscap);
> +    BOOLEAN IsBIOSCompatible() { return FALSE; }
>  protected:
>      NTSTATUS GetModeList(DXGK_DISPLAY_INFORMATION* pDispInfo);
>      VOID BltBits (BLT_INFO* pDst,

Why you used IsBIOSCompatible name instead of IsVGACompatible ?

Frediano
We use this name as video mode that the driver uses and passes to the
BasicVideo is expected to be supported by BIOS or UEFI. "VGA Compatible"
sounds like conformance to some specification, which is not a case. Name of
the field "SupportNonVGA" related to this feature is also inclear and seems
at least inverted. The difference between VGA mode and QXL mode is that
first uses video modes declared by the BIOS and changes them via BIOS
calls, when the seconds does not.




On Wed, Jan 25, 2017 at 6:26 PM, Frediano Ziglio <fziglio@redhat.com> wrote:

> >
> > https://bugzilla.redhat.com/show_bug.cgi?id=1411340
> > Do not set SupportNonVGA field for device rev.4 and higher.
> > Then the class driver will not call miniport's procedure
> > StopDeviceAndReleasePostDisplayOwnership upon PnP stop.
> > QXL device modes are not compatible with VGA ones; the
> > driver in this procedure returns display information related
> > to QXL mode which later used by BasicDisplay driver and causes
> > it to show the screen incorrectly or fail to show it at all.
> >
> > Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
> > ---
> >  qxldod/QxlDod.cpp | 2 +-
> >  qxldod/QxlDod.h   | 2 ++
> >  2 files changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
> > index 7adbec4..001cd67 100755
> > --- a/qxldod/QxlDod.cpp
> > +++ b/qxldod/QxlDod.cpp
> > @@ -411,7 +411,7 @@ NTSTATUS QxlDod::QueryAdapterInfo(_In_ CONST
> > DXGKARG_QUERYADAPTERINFO* pQueryAda
> >              pDriverCaps->PointerCaps.Monochrome = 1;
> >              pDriverCaps->PointerCaps.Color = 1;
> >
> > -            pDriverCaps->SupportNonVGA = TRUE;
> > +            pDriverCaps->SupportNonVGA = m_pHWDevice->IsBIOSCompatible(
> );
> >
> >              DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s 1\n",
> __FUNCTION__));
> >              return STATUS_SUCCESS;
> > diff --git a/qxldod/QxlDod.h b/qxldod/QxlDod.h
> > index 4df4c61..8df1fcf 100755
> > --- a/qxldod/QxlDod.h
> > +++ b/qxldod/QxlDod.h
> > @@ -263,6 +263,7 @@ public:
> >      virtual NTSTATUS Escape(_In_ CONST DXGKARG_ESCAPE* pEscap) = 0;
> >      NTSTATUS AcquireDisplayInfo(DXGK_DISPLAY_INFORMATION& DispInfo);
> >      ULONG GetId(void) { return m_Id; }
> > +    virtual BOOLEAN IsBIOSCompatible() { return TRUE; }
> >  protected:
> >      virtual NTSTATUS GetModeList(DXGK_DISPLAY_INFORMATION* pDispInfo)
> = 0;
> >  protected:
> > @@ -480,6 +481,7 @@ public:
> >      NTSTATUS SetPointerShape(_In_ CONST DXGKARG_SETPOINTERSHAPE*
> >      pSetPointerShape);
> >      NTSTATUS SetPointerPosition(_In_ CONST DXGKARG_SETPOINTERPOSITION*
> >      pSetPointerPosition);
> >      NTSTATUS Escape(_In_ CONST DXGKARG_ESCAPE* pEscap);
> > +    BOOLEAN IsBIOSCompatible() { return FALSE; }
> >  protected:
> >      NTSTATUS GetModeList(DXGK_DISPLAY_INFORMATION* pDispInfo);
> >      VOID BltBits (BLT_INFO* pDst,
>
> Why you used IsBIOSCompatible name instead of IsVGACompatible ?
>
> Frediano
>
Acked-by: Frediano Ziglio <fziglio@redhat.com> 

Frediano 

----- Original Message -----

> From: "Yuri Benditovich" <yuri.benditovich@daynix.com>
> To: "Frediano Ziglio" <fziglio@redhat.com>
> Cc: spice-devel@lists.freedesktop.org, "Yan Vugenfirer" <yan@daynix.com>
> Sent: Wednesday, January 25, 2017 5:45:23 PM
> Subject: Re: [Spice-devel] [PATCH] qxl-wddm-dod: Fix for screen not shown
> after driver disabled

> We use this name as video mode that the driver uses and passes to the
> BasicVideo is expected to be supported by BIOS or UEFI. "VGA Compatible"
> sounds like conformance to some specification, which is not a case. Name of
> the field " SupportNonVGA" related to this feature is also inclear and seems
> at least inverted. The difference between VGA mode and QXL mode is that
> first uses video modes declared by the BIOS and changes them via BIOS calls,
> when the seconds does not.

> On Wed, Jan 25, 2017 at 6:26 PM, Frediano Ziglio < fziglio@redhat.com >
> wrote:

> > >
> 
> > > https://bugzilla.redhat.com/show_bug.cgi?id=1411340
> 
> > > Do not set SupportNonVGA field for device rev.4 and higher.
> 
> > > Then the class driver will not call miniport's procedure
> 
> > > StopDeviceAndReleasePostDisplayOwnership upon PnP stop.
> 
> > > QXL device modes are not compatible with VGA ones; the
> 
> > > driver in this procedure returns display information related
> 
> > > to QXL mode which later used by BasicDisplay driver and causes
> 
> > > it to show the screen incorrectly or fail to show it at all.
> 
> > >
> 
> > > Signed-off-by: Yuri Benditovich < yuri.benditovich@daynix.com >
> 
> > > ---
> 
> > > qxldod/QxlDod.cpp | 2 +-
> 
> > > qxldod/QxlDod.h | 2 ++
> 
> > > 2 files changed, 3 insertions(+), 1 deletion(-)
> 
> > >
> 
> > > diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
> 
> > > index 7adbec4..001cd67 100755
> 
> > > --- a/qxldod/QxlDod.cpp
> 
> > > +++ b/qxldod/QxlDod.cpp
> 
> > > @@ -411,7 +411,7 @@ NTSTATUS QxlDod::QueryAdapterInfo(_In_ CONST
> 
> > > DXGKARG_QUERYADAPTERINFO* pQueryAda
> 
> > > pDriverCaps->PointerCaps.Monochrome = 1;
> 
> > > pDriverCaps->PointerCaps.Color = 1;
> 
> > >
> 
> > > - pDriverCaps->SupportNonVGA = TRUE;
> 
> > > + pDriverCaps->SupportNonVGA = m_pHWDevice->IsBIOSCompatible();
> 
> > >
> 
> > > DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s 1\n", __FUNCTION__));
> 
> > > return STATUS_SUCCESS;
> 
> > > diff --git a/qxldod/QxlDod.h b/qxldod/QxlDod.h
> 
> > > index 4df4c61..8df1fcf 100755
> 
> > > --- a/qxldod/QxlDod.h
> 
> > > +++ b/qxldod/QxlDod.h
> 
> > > @@ -263,6 +263,7 @@ public:
> 
> > > virtual NTSTATUS Escape(_In_ CONST DXGKARG_ESCAPE* pEscap) = 0;
> 
> > > NTSTATUS AcquireDisplayInfo(DXGK_DISPLAY_INFORMATION& DispInfo);
> 
> > > ULONG GetId(void) { return m_Id; }
> 
> > > + virtual BOOLEAN IsBIOSCompatible() { return TRUE; }
> 
> > > protected:
> 
> > > virtual NTSTATUS GetModeList(DXGK_DISPLAY_INFORMATION* pDispInfo) = 0;
> 
> > > protected:
> 
> > > @@ -480,6 +481,7 @@ public:
> 
> > > NTSTATUS SetPointerShape(_In_ CONST DXGKARG_SETPOINTERSHAPE*
> 
> > > pSetPointerShape);
> 
> > > NTSTATUS SetPointerPosition(_In_ CONST DXGKARG_SETPOINTERPOSITION*
> 
> > > pSetPointerPosition);
> 
> > > NTSTATUS Escape(_In_ CONST DXGKARG_ESCAPE* pEscap);
> 
> > > + BOOLEAN IsBIOSCompatible() { return FALSE; }
> 
> > > protected:
> 
> > > NTSTATUS GetModeList(DXGK_DISPLAY_INFORMATION* pDispInfo);
> 
> > > VOID BltBits (BLT_INFO* pDst,
> 

> > Why you used IsBIOSCompatible name instead of IsVGACompatible ?
> 

> > Frediano
>