[Spice-devel] [PATCH 02/12] qxl-wddm-dod: Use rendering offload thread
Christophe de Dinechin
cdupontd at redhat.com
Tue Mar 21 13:52:54 UTC 2017
> On 21 Mar 2017, at 13:59, Yuri Benditovich <yuri.benditovich at daynix.com> wrote:
>
>
>
> On Mon, Mar 20, 2017 at 2:08 PM, Frediano Ziglio <fziglio at redhat.com <mailto:fziglio at redhat.com>> wrote:
> >
> > Instead of sending drawable commands down from presentation
> > callback, collect drawables objects and pass them to dedicated
> > thread for processing. This reduce peak load of presentation
> > callback.
> >
> > Signed-off-by: Javier Celaya <javier.celaya at flexvdi.com <mailto:javier.celaya at flexvdi.com>>
> > Signed-off-by: Yuri Benditovich <yuri.benditovich at daynix.com <mailto:yuri.benditovich at daynix.com>>
> > ---
> > qxldod/QxlDod.cpp | 43 +++++++++++++++++++++++++++++++++----------
> > qxldod/QxlDod.h | 4 ++--
> > 2 files changed, 35 insertions(+), 12 deletions(-)
> >
> > diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
> > index b952bf9..01de9b3 100755
> > --- a/qxldod/QxlDod.cpp
> > +++ b/qxldod/QxlDod.cpp
> > @@ -3794,11 +3794,20 @@ QxlDevice::ExecutePresentDisplayOnly(
> > SIZE_T sizeRects = NumDirtyRects*sizeof(RECT);
> > SIZE_T size = sizeof(DoPresentMemory) + sizeMoves + sizeRects;
> >
> > + QXLDrawable **pDrawables = reinterpret_cast<QXLDrawable **>(new
> > (NonPagedPoolNx) BYTE[sizeof(QXLDrawable *)*(NumDirtyRects + NumMoves +
> > 1)]);
>
> here would be
>
> QXLDrawable **pDrawables = new (NonPagedPoolNx) QXLDrawable *[NumDirtyRects + NumMoves + 1];
>
> is non paged memory needed? Both functions (producer and consumer) are in paged areas.
>
> > + UINT nIndex = 0;
> > +
> > + if (!pDrawables)
> > + {
> > + return STATUS_NO_MEMORY;
> > + }
> > +
> > DoPresentMemory* ctx = reinterpret_cast<DoPresentMemory*>
> > (new (NonPagedPoolNx) BYTE[size]);
> >
>
> DoPresentMemory* ctx = new (NonPagedPoolNx) DoPresentMemory;
>
> Current commit intentionally does not change this, to make clear what is changed.
Good.
If we use a non-paged placement new, shouldn’t we use a corresponding placement delete?
> Without changes in the code we can't just allocate less memory, as trailing RECT structures are used for
> unneeded copy from parameters.
> Removal of this completely unneeded 'ctx' is planned for further commits after all the HCK-related work is done.
>
>
> same comments as above
>
> > if (!ctx)
> > {
> > + delete[] reinterpret_cast<BYTE*>(pDrawables);
>
> delete[] pDrawables;
>
> > return STATUS_NO_MEMORY;
> > }
> >
> > @@ -3828,6 +3837,8 @@ QxlDevice::ExecutePresentDisplayOnly(
> > PMDL mdl = IoAllocateMdl((PVOID)SrcAddr, sizeToMap, FALSE, FALSE,
> > NULL);
> > if(!mdl)
> > {
> > + delete[] reinterpret_cast<BYTE*>(ctx);
> > + delete[] reinterpret_cast<BYTE*>(pDrawables);
>
> similar to above, in this case "delete ctx;"
>
> > return STATUS_INSUFFICIENT_RESOURCES;
> > }
> >
> > @@ -3844,6 +3855,8 @@ QxlDevice::ExecutePresentDisplayOnly(
> > {
> > Status = GetExceptionCode();
> > IoFreeMdl(mdl);
> > + delete[] reinterpret_cast<BYTE*>(ctx);
> > + delete[] reinterpret_cast<BYTE*>(pDrawables);
>
> ditto
>
> > return Status;
> > }
> >
> > @@ -3857,6 +3870,8 @@ QxlDevice::ExecutePresentDisplayOnly(
> > Status = STATUS_INSUFFICIENT_RESOURCES;
> > MmUnlockPages(mdl);
> > IoFreeMdl(mdl);
> > + delete[] reinterpret_cast<BYTE*>(ctx);
> > + delete[] reinterpret_cast<BYTE*>(pDrawables);
>
> ditto
>
> > return Status;
> > }
> >
> > @@ -3922,7 +3937,9 @@ QxlDevice::ExecutePresentDisplayOnly(
> > DbgPrint(TRACE_LEVEL_INFORMATION, ("--- %d SourcePoint.x = %ld,
> > SourcePoint.y = %ld, DestRect.bottom = %ld, DestRect.left = %ld,
> > DestRect.right = %ld, DestRect.top = %ld\n",
> > i , pSourcePoint->x, pSourcePoint->y, pDestRect->bottom,
> > pDestRect->left, pDestRect->right, pDestRect->top));
> >
> > - CopyBits(*pDestRect, *pSourcePoint);
> > + pDrawables[nIndex] = CopyBits(*pDestRect, *pSourcePoint);
> > +
> > + if (pDrawables[nIndex]) nIndex++;
> > }
> >
> > // Copy all the dirty rects from source image to video frame buffer.
> > @@ -3936,11 +3953,13 @@ QxlDevice::ExecutePresentDisplayOnly(
> > DbgPrint(TRACE_LEVEL_INFORMATION, ("--- %d pDirtyRect->bottom = %ld,
> > pDirtyRect->left = %ld, pDirtyRect->right = %ld, pDirtyRect->top =
> > %ld\n",
> > i, pDirtyRect->bottom, pDirtyRect->left, pDirtyRect->right,
> > pDirtyRect->top));
> >
> > - BltBits(&DstBltInfo,
> > + pDrawables[nIndex] = BltBits(&DstBltInfo,
> > &SrcBltInfo,
> > 1,
> > pDirtyRect,
> > &sourcePoint);
> > +
> > + if (pDrawables[nIndex]) nIndex++;
> > }
> >
> > // Unmap unmap and unlock the pages.
> > @@ -3951,6 +3970,10 @@ QxlDevice::ExecutePresentDisplayOnly(
> > }
> > delete [] reinterpret_cast<BYTE*>(ctx);
> >
> > + pDrawables[nIndex] = NULL;
> > +
> > + PostToWorkerThread(pDrawables);
> > +
> > return STATUS_SUCCESS;
> > }
> >
> > @@ -4364,7 +4387,7 @@ VOID QxlDevice::SetImageId(InternalImage *internal,
> > }
> > }
> >
> > -void QxlDevice::CopyBits(const RECT& rect, const POINT& sourcePoint)
> > +QXLDrawable *QxlDevice::CopyBits(const RECT& rect, const POINT& sourcePoint)
> > {
>
> This CopyBits and BltBits are not doing anymore the operation, should
> be renamed to something like PrepareCopyBits (better names welcome)
>
> No problem, prefer to do it on later commits.
>
>
> > PAGED_CODE();
> > QXLDrawable *drawable;
> > @@ -4373,18 +4396,18 @@ void QxlDevice::CopyBits(const RECT& rect, const
> > POINT& sourcePoint)
> >
> > if (!(drawable = Drawable(QXL_COPY_BITS, &rect, NULL, 0))) {
> > DbgPrint(TRACE_LEVEL_ERROR, ("Cannot get Drawable.\n"));
> > - return;
> > + return NULL;
> > }
> >
> > drawable->u.copy_bits.src_pos.x = sourcePoint.x;
> > drawable->u.copy_bits.src_pos.y = sourcePoint.y;
> >
> > - PushDrawable(drawable);
> > -
> > DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s\n", __FUNCTION__));
> > +
> > + return drawable;
> > }
> >
> > -VOID QxlDevice::BltBits (
> > +QXLDrawable *QxlDevice::BltBits (
> > BLT_INFO* pDst,
> > CONST BLT_INFO* pSrc,
> > UINT NumRects,
> > @@ -4407,7 +4430,7 @@ VOID QxlDevice::BltBits (
> >
> > if (!(drawable = Drawable(QXL_DRAW_COPY, pRects, NULL, 0))) {
> > DbgPrint(TRACE_LEVEL_ERROR, ("Cannot get Drawable.\n"));
> > - return;
> > + return NULL;
> > }
> >
> > CONST RECT* pRect = &pRects[0];
> > @@ -4480,9 +4503,9 @@ VOID QxlDevice::BltBits (
> > drawable->surfaces_rects[0].top,
> > drawable->surfaces_rects[0].bottom,
> > drawable->u.copy.src_bitmap));
> >
> > - PushDrawable(drawable);
> > -
> > DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s\n", __FUNCTION__));
> > +
> > + return drawable;
> > }
> >
> > VOID QxlDevice::PutBytesAlign(QXLDataChunk **chunk_ptr, UINT8 **now_ptr,
> > diff --git a/qxldod/QxlDod.h b/qxldod/QxlDod.h
> > index 4a62680..f441f4b 100755
> > --- a/qxldod/QxlDod.h
> > +++ b/qxldod/QxlDod.h
> > @@ -495,12 +495,12 @@ public:
> > BOOLEAN IsBIOSCompatible() { return FALSE; }
> > protected:
> > NTSTATUS GetModeList(DXGK_DISPLAY_INFORMATION* pDispInfo);
> > - VOID BltBits (BLT_INFO* pDst,
> > + QXLDrawable *BltBits (BLT_INFO* pDst,
> > CONST BLT_INFO* pSrc,
> > UINT NumRects,
> > _In_reads_(NumRects) CONST RECT *pRects,
> > POINT* pSourcePoint);
> > - void CopyBits(const RECT& rect, const POINT& sourcePoint);
> > + QXLDrawable *CopyBits(const RECT& rect, const POINT& sourcePoint);
> > QXLDrawable *Drawable(UINT8 type,
> > CONST RECT *area,
> > CONST RECT *clip,
>
> Frediano
>
> _______________________________________________
> 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/20170321/b95017f0/attachment-0001.html>
More information about the Spice-devel
mailing list