[Spice-devel] [PATCH qxl-wddm-dod v5 4/7] Fixing framebuffer usage logic
Frediano Ziglio
fziglio at redhat.com
Mon Sep 26 16:15:17 UTC 2016
>
> This patch fixes 2 issues:
>
> 1. Framebuffer should only be used in vga mode,
> therefore when QxlDevice is active
> FrameBufferIsActive flag shouldn't be checked;
> 2. FrameBufferIsActive flag should be set true
> on successfull frame buffer allocation only.
>
> Signed-off-by: Dmitry Fleytman <dmitry at daynix.com>
> Signed-off-by: Sameeh Jubran <sameeh at daynix.com>
> ---
> qxldod/QxlDod.cpp | 116
> +++++++++++++++++++++++++++---------------------------
> 1 file changed, 58 insertions(+), 58 deletions(-)
>
> diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
> index 85d6d94..ca3f8a3 100755
> --- a/qxldod/QxlDod.cpp
> +++ b/qxldod/QxlDod.cpp
> @@ -523,40 +523,37 @@ NTSTATUS QxlDod::PresentDisplayOnly(_In_ CONST
> DXGKARG_PRESENT_DISPLAYONLY* pPre
> return STATUS_SUCCESS;
> }
>
> - if
> (m_CurrentModes[pPresentDisplayOnly->VidPnSourceId].Flags.FrameBufferIsActive)
> - {
> -
> - // If actual pixels are coming through, will need to completely zero
> out physical address next time in BlackOutScreen
> -
> m_CurrentModes[pPresentDisplayOnly->VidPnSourceId].ZeroedOutStart.QuadPart
> = 0;
> -
> m_CurrentModes[pPresentDisplayOnly->VidPnSourceId].ZeroedOutEnd.QuadPart
> = 0;
>
> -
> - D3DKMDT_VIDPN_PRESENT_PATH_ROTATION RotationNeededByFb =
> pPresentDisplayOnly->Flags.Rotate ?
> -
> m_CurrentModes[pPresentDisplayOnly->VidPnSourceId].Rotation
> :
> -
> D3DKMDT_VPPR_IDENTITY;
> - BYTE* pDst =
> (BYTE*)m_CurrentModes[pPresentDisplayOnly->VidPnSourceId].FrameBuffer.Ptr;
> - UINT DstBitPerPixel =
> BPPFromPixelFormat(m_CurrentModes[pPresentDisplayOnly->VidPnSourceId].DispInfo.ColorFormat);
> - if (m_CurrentModes[pPresentDisplayOnly->VidPnSourceId].Scaling ==
> D3DKMDT_VPPS_CENTERED)
> - {
> - UINT CenterShift =
> (m_CurrentModes[pPresentDisplayOnly->VidPnSourceId].DispInfo.Height -
> -
> m_CurrentModes[pPresentDisplayOnly->VidPnSourceId].SrcModeHeight)*m_CurrentModes[pPresentDisplayOnly->VidPnSourceId].DispInfo.Pitch;
> - CenterShift +=
> (m_CurrentModes[pPresentDisplayOnly->VidPnSourceId].DispInfo.Width -
> -
> m_CurrentModes[pPresentDisplayOnly->VidPnSourceId].SrcModeWidth)*DstBitPerPixel/8;
> - pDst += (int)CenterShift/2;
> - }
> - Status = m_pHWDevice->ExecutePresentDisplayOnly(
> - pDst,
> - DstBitPerPixel,
> - (BYTE*)pPresentDisplayOnly->pSource,
> - pPresentDisplayOnly->BytesPerPixel,
> - pPresentDisplayOnly->Pitch,
> - pPresentDisplayOnly->NumMoves,
> - pPresentDisplayOnly->pMoves,
> - pPresentDisplayOnly->NumDirtyRects,
> - pPresentDisplayOnly->pDirtyRect,
> - RotationNeededByFb,
> - &m_CurrentModes[0]);
> - }
> + // If actual pixels are coming through, will need to completely zero out
> physical address next time in BlackOutScreen
> +
> m_CurrentModes[pPresentDisplayOnly->VidPnSourceId].ZeroedOutStart.QuadPart
> = 0;
> + m_CurrentModes[pPresentDisplayOnly->VidPnSourceId].ZeroedOutEnd.QuadPart
> = 0;
> +
> +
> + D3DKMDT_VIDPN_PRESENT_PATH_ROTATION RotationNeededByFb =
> pPresentDisplayOnly->Flags.Rotate ?
> +
> m_CurrentModes[pPresentDisplayOnly->VidPnSourceId].Rotation
> :
> +
> D3DKMDT_VPPR_IDENTITY;
> + BYTE* pDst =
> (BYTE*)m_CurrentModes[pPresentDisplayOnly->VidPnSourceId].FrameBuffer.Ptr;
> + UINT DstBitPerPixel =
> BPPFromPixelFormat(m_CurrentModes[pPresentDisplayOnly->VidPnSourceId].DispInfo.ColorFormat);
> + if (m_CurrentModes[pPresentDisplayOnly->VidPnSourceId].Scaling ==
> D3DKMDT_VPPS_CENTERED)
> + {
> + UINT CenterShift =
> (m_CurrentModes[pPresentDisplayOnly->VidPnSourceId].DispInfo.Height -
> +
> m_CurrentModes[pPresentDisplayOnly->VidPnSourceId].SrcModeHeight)*m_CurrentModes[pPresentDisplayOnly->VidPnSourceId].DispInfo.Pitch;
> + CenterShift +=
> (m_CurrentModes[pPresentDisplayOnly->VidPnSourceId].DispInfo.Width -
> +
> m_CurrentModes[pPresentDisplayOnly->VidPnSourceId].SrcModeWidth)*DstBitPerPixel/8;
> + pDst += (int)CenterShift/2;
> + }
> + Status = m_pHWDevice->ExecutePresentDisplayOnly(
> + pDst,
> + DstBitPerPixel,
> + (BYTE*)pPresentDisplayOnly->pSource,
> + pPresentDisplayOnly->BytesPerPixel,
> + pPresentDisplayOnly->Pitch,
> + pPresentDisplayOnly->NumMoves,
> + pPresentDisplayOnly->pMoves,
> + pPresentDisplayOnly->NumDirtyRects,
> + pPresentDisplayOnly->pDirtyRect,
> + RotationNeededByFb,
> + &m_CurrentModes[0]);
> DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s\n", __FUNCTION__));
> return Status;
> }
> @@ -1542,12 +1539,11 @@ 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;
>
> - Status = m_pHWDevice->AcquireFrameBuffer(pCurrentBddMode);
> + Status = m_pHWDevice->AcquireFrameBuffer(pCurrentBddMode);
>
Here I think it's just a small mistake, no reason to indent more.
> if (NT_SUCCESS(Status))
> {
>
> - pCurrentBddMode->Flags.FrameBufferIsActive = TRUE;
I would remove the empty line above
> m_pHWDevice->BlackOutScreen(&m_CurrentModes[pPath->VidPnSourceId]);
>
> // Mark that the next present should be fullscreen so the screen
> doesn't go from black to actual pixels one dirty rect at a time.
> @@ -2979,11 +2975,18 @@ 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));
> + NTSTATUS status = STATUS_UNSUCCESSFUL;
> + 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));
> + if (NT_SUCCESS(status))
> + {
> + pCurrentBddMode->Flags.FrameBufferIsActive = TRUE;
> + }
> + }
> return status;
> }
>
Here I would just add a
if (!pCurrentBddMode->Flags.DoNotMapOrUnmap) {
return STATUS_UNSUCCESSFUL;
}
just after the function starts.
> @@ -4501,26 +4504,23 @@ VOID QxlDevice::BlackOutScreen(CURRENT_BDD_MODE*
> pCurrentBddMod)
> QXLDrawable *drawable;
> RECT Rect;
> PAGED_CODE();
> - if (pCurrentBddMod->Flags.FrameBufferIsActive)
> + Rect.bottom = pCurrentBddMod->SrcModeHeight;
> + Rect.top = 0;
> + Rect.left = 0;
> + Rect.right = pCurrentBddMod->SrcModeWidth;
> + if (!(drawable = Drawable(QXL_DRAW_FILL, &Rect, NULL, 0)))
> {
> - Rect.bottom = pCurrentBddMod->SrcModeHeight;
> - Rect.top = 0;
> - Rect.left = 0;
> - Rect.right = pCurrentBddMod->SrcModeWidth;
> - if (!(drawable = Drawable(QXL_DRAW_FILL, &Rect, NULL, 0)))
> - {
> - DbgPrint(TRACE_LEVEL_ERROR, ("Cannot get Drawable.\n"));
> - return;
> - }
> - drawable->u.fill.brush.type = SPICE_BRUSH_TYPE_SOLID;
> - drawable->u.fill.brush.u.color = 0;
> - drawable->u.fill.rop_descriptor = SPICE_ROPD_OP_PUT;
> - drawable->u.fill.mask.flags = 0;
> - drawable->u.fill.mask.pos.x = 0;
> - drawable->u.fill.mask.pos.y = 0;
> - drawable->u.fill.mask.bitmap = 0;
> - PushDrawable(drawable);
> + DbgPrint(TRACE_LEVEL_ERROR, ("Cannot get Drawable.\n"));
> + return;
> }
> + drawable->u.fill.brush.type = SPICE_BRUSH_TYPE_SOLID;
> + drawable->u.fill.brush.u.color = 0;
> + drawable->u.fill.rop_descriptor = SPICE_ROPD_OP_PUT;
> + drawable->u.fill.mask.flags = 0;
> + drawable->u.fill.mask.pos.x = 0;
> + drawable->u.fill.mask.pos.y = 0;
> + drawable->u.fill.mask.bitmap = 0;
> + PushDrawable(drawable);
> }
>
> __declspec(code_seg("PAGE"))
All changes I commented are mainly spaces and small style, if you
agree I can change them, if not reply.
Acked-by: Frediano Ziglio <fziglio at redhat.com>
Frediano
More information about the Spice-devel
mailing list