[Spice-devel] [PATCH qxl-wddm-dod v5 3/7] Use frame buffer in VGA mode only

[Frediano Ziglio]

> 
> Framebuffer should only be used in VGA mode, as WDDM DOD
> driver doesn't use the frame buffer (bar0), so no
> reason to map in into memory. However the mode is only known
> at runtime therefore framebuffer logic should be active when the
> driver is operating in vga mode only.
> 
> There were rare BSOD failures when the mapping failed.
> 
> Signed-off-by: Sameeh Jubran <sameeh at daynix.com>
> Signed-off-by: Dmitry Fleytman <dmitry at daynix.com>

Acked-by: Frediano Ziglio <fziglio at redhat.com>

Frediano

> ---
>  qxldod/QxlDod.cpp | 38 +++++++++++++++++++++-----------------
>  qxldod/QxlDod.h   |  4 ++++
>  2 files changed, 25 insertions(+), 17 deletions(-)
> 
> diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
> index 136daf7..85d6d94 100755
> --- a/qxldod/QxlDod.cpp
> +++ b/qxldod/QxlDod.cpp
> @@ -224,9 +224,7 @@ VOID QxlDod::CleanUp(VOID)
>      {
>          if (m_CurrentModes[Source].FrameBuffer.Ptr)
>          {
> -            UnmapFrameBuffer(m_CurrentModes[Source].FrameBuffer.Ptr,
> m_CurrentModes[Source].DispInfo.Height *
> m_CurrentModes[Source].DispInfo.Pitch);
> -            m_CurrentModes[Source].FrameBuffer.Ptr = NULL;
> -            m_CurrentModes[Source].Flags.FrameBufferIsActive = FALSE;
> +            m_pHWDevice->ReleaseFrameBuffer(&m_CurrentModes[Source]);
>          }
>      }
>  }
> @@ -1419,11 +1417,7 @@ NTSTATUS QxlDod::CommitVidPn(_In_ CONST
> DXGKARG_COMMITVIDPN* CONST pCommitVidPn)
>      if (m_CurrentModes[pCommitVidPn->AffectedVidPnSourceId].FrameBuffer.Ptr
>      &&
>          !m_CurrentModes[pCommitVidPn->AffectedVidPnSourceId].Flags.DoNotMapOrUnmap)
>      {
> -        Status =
> UnmapFrameBuffer(m_CurrentModes[pCommitVidPn->AffectedVidPnSourceId].FrameBuffer.Ptr,
> -
> m_CurrentModes[pCommitVidPn->AffectedVidPnSourceId].DispInfo.Pitch
> * m_CurrentModes[pCommitVidPn->AffectedVidPnSourceId].DispInfo.Height);
> -        m_CurrentModes[pCommitVidPn->AffectedVidPnSourceId].FrameBuffer.Ptr
> = NULL;
> -
> m_CurrentModes[pCommitVidPn->AffectedVidPnSourceId].Flags.FrameBufferIsActive
> = FALSE;
> -
> +        Status =
> m_pHWDevice->ReleaseFrameBuffer(&m_CurrentModes[pCommitVidPn->AffectedVidPnSourceId]);
>          if (!NT_SUCCESS(Status))
>          {
>              goto CommitVidPnExit;
> @@ -1548,15 +1542,7 @@ NTSTATUS QxlDod::SetSourceModeAndPath(CONST
> D3DKMDT_VIDPN_SOURCE_MODE* pSourceMo
>      pCurrentBddMode->DispInfo.Height =
>      pSourceMode->Format.Graphics.PrimSurfSize.cy;
>      pCurrentBddMode->DispInfo.Pitch =
>      pSourceMode->Format.Graphics.PrimSurfSize.cx *
>      BPPFromPixelFormat(pCurrentBddMode->DispInfo.ColorFormat) /
>      BITS_PER_BYTE;
>  
> -
> -    if (!pCurrentBddMode->Flags.DoNotMapOrUnmap)
> -    {
> -        // Map the new frame buffer
> -        QXL_ASSERT(pCurrentBddMode->FrameBuffer.Ptr == NULL);
> -        Status = MapFrameBuffer(pCurrentBddMode->DispInfo.PhysicAddress,
> -                                pCurrentBddMode->DispInfo.Pitch *
> pCurrentBddMode->DispInfo.Height,
> -                                &(pCurrentBddMode->FrameBuffer.Ptr));
> -    }
> +    Status = m_pHWDevice->AcquireFrameBuffer(pCurrentBddMode);
>  
>      if (NT_SUCCESS(Status))
>      {
> @@ -2991,6 +2977,24 @@ VOID VgaDevice::ResetDevice(VOID)
>  {
>  }
>  
> +NTSTATUS VgaDevice::AcquireFrameBuffer(CURRENT_BDD_MODE* pCurrentBddMode)
> +{
> +    // Map the new frame buffer
> +    QXL_ASSERT(pCurrentBddMode->FrameBuffer.Ptr == NULL);
> +    NTSTATUS status =
> MapFrameBuffer(pCurrentBddMode->DispInfo.PhysicAddress,
> +        pCurrentBddMode->DispInfo.Pitch * pCurrentBddMode->DispInfo.Height,
> +        &(pCurrentBddMode->FrameBuffer.Ptr));
> +    return status;
> +}
> +
> +NTSTATUS VgaDevice::ReleaseFrameBuffer(CURRENT_BDD_MODE* pCurrentBddMode)
> +{
> +    NTSTATUS status = UnmapFrameBuffer(pCurrentBddMode->FrameBuffer.Ptr,
> pCurrentBddMode->DispInfo.Height * pCurrentBddMode->DispInfo.Pitch);
> +    pCurrentBddMode->FrameBuffer.Ptr = NULL;
> +    pCurrentBddMode->Flags.FrameBufferIsActive = FALSE;
> +    return status;
> +}
> +
>  __declspec(code_seg("PAGE"))
>  NTSTATUS  VgaDevice::SetPointerShape(_In_ CONST DXGKARG_SETPOINTERSHAPE*
>  pSetPointerShape)
>  {
> diff --git a/qxldod/QxlDod.h b/qxldod/QxlDod.h
> index 8b90616..585b235 100755
> --- a/qxldod/QxlDod.h
> +++ b/qxldod/QxlDod.h
> @@ -232,6 +232,8 @@ public:
>      virtual BOOLEAN InterruptRoutine(_In_ PDXGKRNL_INTERFACE pDxgkInterface,
>      _In_  ULONG MessageNumber) = 0;
>      virtual VOID DpcRoutine(PVOID) = 0;
>      virtual VOID ResetDevice(void) = 0;
> +    virtual NTSTATUS AcquireFrameBuffer(CURRENT_BDD_MODE* pCurrentBddMode) {
> return STATUS_SUCCESS; }
> +    virtual NTSTATUS ReleaseFrameBuffer(CURRENT_BDD_MODE* pCurrentBddMode) {
> return STATUS_SUCCESS; }
>  
>      __declspec(code_seg("PAGE"))
>      virtual ULONG GetModeCount(void) = 0;
> @@ -320,6 +322,8 @@ public:
>      BOOLEAN InterruptRoutine(_In_ PDXGKRNL_INTERFACE pDxgkInterface, _In_
>      ULONG MessageNumber);
>      VOID DpcRoutine(PVOID);
>      VOID ResetDevice(VOID);
> +    NTSTATUS AcquireFrameBuffer(CURRENT_BDD_MODE* pCurrentBddMode);
> +    NTSTATUS ReleaseFrameBuffer(CURRENT_BDD_MODE* pCurrentBddMode);
>      __declspec(code_seg("PAGE"))
>      NTSTATUS SetPointerShape(_In_ CONST DXGKARG_SETPOINTERSHAPE*
>      pSetPointerShape);
>      __declspec(code_seg("PAGE"))