[Spice-devel] [PATCH 02/12] qxl-wddm-dod: Use rendering offload thread

Yuri Benditovich yuri.benditovich at daynix.com
Tue Mar 21 12:59:24 UTC 2017


On Mon, Mar 20, 2017 at 2:08 PM, Frediano Ziglio <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>
> > Signed-off-by: Yuri Benditovich <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.
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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170321/2f498661/attachment-0001.html>


More information about the Spice-devel mailing list