> On Mon, Sep 26, 2016 at 7:15 PM, Frediano Ziglio < fzig...@redhat.com >
> wrote:
> > >
>
> > > 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 < dmi...@daynix.com >
>
> > > Signed-off-by: Sameeh Jubran < sam...@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.
>
> Yes, go ahead.
> > Acked-by: Frediano Ziglio < fzig...@redhat.com >
>
Merged
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
