[Spice-devel] [PATCH 01/12] qxl-wddm-dod: Prepare system thread for rendering
Yuri Benditovich
yuri.benditovich at daynix.com
Tue Mar 21 12:52:55 UTC 2017
On Mon, Mar 20, 2017 at 2:01 PM, Frediano Ziglio <fziglio at redhat.com> wrote:
> >
> > Create rendering thread upon device start and terminate it upon
> > stop. The dedicated thread is normally pending for display commands
> > to be sent to the host. Currently only single NULL (termination)
> > command is placed to the ring where the thread consumes events from.
> >
> > Signed-off-by: Javier Celaya <javier.celaya at flexvdi.com>
> > Signed-off-by: Yuri Benditovich <yuri.benditovich at daynix.com>
> > ---
> > qxldod/QxlDod.cpp | 114
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > qxldod/QxlDod.h | 16 ++++++++
> > 2 files changed, 130 insertions(+)
> >
> > diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
> > index 9175300..b952bf9 100755
> > --- a/qxldod/QxlDod.cpp
> > +++ b/qxldod/QxlDod.cpp
> > @@ -3062,6 +3062,7 @@ QxlDevice::QxlDevice(_In_ QxlDod* pQxlDod)
> > m_CustomMode = 0;
> > m_FreeOutputs = 0;
> > m_Pending = 0;
> > + m_PresentThread = NULL;
> > }
> >
> > QxlDevice::~QxlDevice(void)
> > @@ -3441,6 +3442,29 @@ NTSTATUS QxlDevice::HWInit(PCM_RESOURCE_LIST
> pResList,
> > DXGK_DISPLAY_INFORMATION*
> > return QxlInit(pDispInfo);
> > }
> >
> > +NTSTATUS QxlDevice::StartPresentThread()
> > +{
> > + PAGED_CODE();
> > + OBJECT_ATTRIBUTES ObjectAttributes;
> > + NTSTATUS Status;
> > +
> > + KeClearEvent(&m_PresentThreadReadyEvent);
> > +
> > + InitializeObjectAttributes(&ObjectAttributes, NULL,
> OBJ_KERNEL_HANDLE,
> > NULL, NULL);
> > + Status = PsCreateSystemThread(
> > + &m_PresentThread,
> > + THREAD_ALL_ACCESS,
> > + &ObjectAttributes,
> > + NULL,
> > + NULL,
> > + PresentThreadRoutineWrapper,
> > + this);
> > +
> > + WaitForObject(&m_PresentThreadReadyEvent, NULL);
> > +
>
> Why we need to wait here? Same above, no much need to clear the event.
>
> Wait is not mandatory - it is here to have predictable timing and ensure
the thread is ready before first 'Present' command.
> > + return Status;
> > +}
> > +
> > NTSTATUS QxlDevice::QxlInit(DXGK_DISPLAY_INFORMATION* pDispInfo)
> > {
> > PAGED_CODE();
> > @@ -3467,12 +3491,17 @@ NTSTATUS QxlDevice::QxlInit(DXGK_
> DISPLAY_INFORMATION*
> > pDispInfo)
> > InitDeviceMemoryResources();
> > InitMonitorConfig();
> > Status = AcquireDisplayInfo(*(pDispInfo));
> > + if (NT_SUCCESS(Status))
> > + {
> > + Status = StartPresentThread();
> > + }
> > return Status;
> > }
> >
> > void QxlDevice::QxlClose()
> > {
> > PAGED_CODE();
> > + StopPresentThread();
> > DestroyMemSlots();
> > }
> >
> > @@ -3609,6 +3638,12 @@ BOOL QxlDevice::CreateEvents()
> > KeInitializeEvent(&m_IoCmdEvent,
> > SynchronizationEvent,
> > FALSE);
> > + KeInitializeEvent(&m_PresentEvent,
> > + SynchronizationEvent,
> > + FALSE);
> > + KeInitializeEvent(&m_PresentThreadReadyEvent,
> > + SynchronizationEvent,
> > + FALSE);
> > KeInitializeMutex(&m_MemLock,1);
> > KeInitializeMutex(&m_CmdLock,1);
> > KeInitializeMutex(&m_IoLock,1);
> > @@ -3625,6 +3660,7 @@ BOOL QxlDevice::CreateRings()
> > m_CommandRing = &(m_RamHdr->cmd_ring);
> > m_CursorRing = &(m_RamHdr->cursor_ring);
> > m_ReleaseRing = &(m_RamHdr->release_ring);
> > + SPICE_RING_INIT(m_PresentRing);
> > DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s\n", __FUNCTION__));
> > return TRUE;
> > }
> > @@ -4907,6 +4943,7 @@ UINT SpiceFromPixelFormat(D3DDDIFORMAT Format)
> >
> > NTSTATUS HwDeviceInterface::AcquireDisplayInfo(DXGK_
> DISPLAY_INFORMATION&
> > DispInfo)
> > {
> > + PAGED_CODE();
> > NTSTATUS Status = STATUS_SUCCESS;
> > if (GetId() == 0)
> > {
> > @@ -4996,3 +5033,80 @@ QXL_NON_PAGED VOID QxlDod::VsyncTimerProcGate(_
> In_
> > _KDPC *dpc, _In_ PVOID contex
> > QxlDod* pQxl = reinterpret_cast<QxlDod*>(context);
> > pQxl->VsyncTimerProc();
> > }
> > +
> > +void QxlDevice::StopPresentThread()
> > +{
> > + PAGED_CODE();
> > + PVOID pDispatcherObject;
> > + if (m_PresentThread)
> > + {
> > + DbgPrint(TRACE_LEVEL_INFORMATION, ("---> %s\n", __FUNCTION__));
> > + NTSTATUS Status = ObReferenceObjectByHandle(
> > + m_PresentThread, 0, NULL, KernelMode, &pDispatcherObject,
> NULL);
> > + if (NT_SUCCESS(Status))
> > + {
> > + PostToWorkerThread(NULL);
>
> In the unluckily event that ObReferenceObjectByHandle failed would
> be better to do this call anyway, so we have a chance the thread
> will be destroyed before system potentially free from memory
> the thread code and data.
>
> > + WaitForObject(pDispatcherObject, NULL);
> > + ObDereferenceObject(pDispatcherObject);
> > + }
> > + ZwClose(m_PresentThread);
> > + m_PresentThread = NULL;
> > + DbgPrint(TRACE_LEVEL_INFORMATION, ("<--- %s\n", __FUNCTION__));
> > + }
> > +}
> > +
> > +void QxlDevice::PresentThreadRoutine()
> > +{
> > + PAGED_CODE();
> > + int wait;
> > + int notify;
> > + QXLDrawable** drawables;
> > +
> > + DbgPrint(TRACE_LEVEL_INFORMATION, ("--->%s\n", __FUNCTION__));
> > +
> > + KeSetEvent(&m_PresentThreadReadyEvent, 0, FALSE);
> > +
> > + while (1)
> > + {
> > + // Pop a drawables list from the ring
> > + // No need for a mutex, only one consumer thread
> > + SPICE_RING_CONS_WAIT(m_PresentRing, wait);
> > + while (wait) {
> > + WaitForObject(&m_PresentEvent, NULL);
> > + SPICE_RING_CONS_WAIT(m_PresentRing, wait);
> > + }
> > + drawables = *SPICE_RING_CONS_ITEM(m_PresentRing);
> > + SPICE_RING_POP(m_PresentRing, notify);
> > + if (notify) {
> > + KeSetEvent(&m_PresentThreadReadyEvent, 0, FALSE);
> > + }
> > +
> > + if (drawables) {
> > + for (UINT i = 0; drawables[i]; ++i)
> > + PushDrawable(drawables[i]);
> > + delete[] reinterpret_cast<BYTE*>(drawables);
>
> why all these reinterpret_cast ?
> Just
>
> delete[] drawables;
>
> is enough (obviously allocation should the changed too).
>
> > + }
> > + else {
> > + DbgPrint(TRACE_LEVEL_WARNING, ("%s is being terminated\n",
> > __FUNCTION__));
> > + break;
> > + }
> > + }
> > +}
> > +
> > +void QxlDevice::PostToWorkerThread(QXLDrawable** drawables)
> > +{
> > + PAGED_CODE();
> > + // Push drawables into PresentRing and notify worker thread
> > + int notify, wait;
> > + SPICE_RING_PROD_WAIT(m_PresentRing, wait);
> > + while (wait) {
> > + WaitForObject(&m_PresentThreadReadyEvent, NULL);
> > + SPICE_RING_PROD_WAIT(m_PresentRing, wait);
> > + }
> > + *SPICE_RING_PROD_ITEM(m_PresentRing) = drawables;
> > + SPICE_RING_PUSH(m_PresentRing, notify);
> > + if (notify) {
> > + KeSetEvent(&m_PresentEvent, 0, FALSE);
> > + }
> > + DbgPrint(TRACE_LEVEL_INFORMATION, ("<--- %s\n", __FUNCTION__));
> > +}
> > diff --git a/qxldod/QxlDod.h b/qxldod/QxlDod.h
> > index 9cfb6de..4a62680 100755
> > --- a/qxldod/QxlDod.h
> > +++ b/qxldod/QxlDod.h
> > @@ -456,6 +456,10 @@ typedef struct DpcCbContext {
> > #define MAX(x, y) (((x) >= (y)) ? (x) : (y))
> > #define ALIGN(a, b) (((a) + ((b) - 1)) & ~((b) - 1))
> >
> > +#include "start-packed.h"
> > +SPICE_RING_DECLARE(QXLPresentOnlyRing, QXLDrawable**, 1024);
> > +#include "end-packed.h"
> > +
>
> Not a regression, however this structure cannot be byte-packed.
> This would cause potentially problems like 255+1 == 0.
>
> I do not see how this can cause problem.
Packing at byte boundary is not needed here, but the alternative is (ugly)
to use #define SPICE_ATTR_PACKED before declaration and #undef
SPICE_ATTR_PACKED after.
> > class QxlDevice :
> > public HwDeviceInterface
> > {
> > @@ -559,6 +563,13 @@ private:
> > NTSTATUS UpdateChildStatus(BOOLEAN connect);
> > NTSTATUS SetCustomDisplay(QXLEscapeSetCustomDisplay*
> custom_display);
> > void SetMonitorConfig(QXLHead* monitor_config);
> > + NTSTATUS StartPresentThread();
> > + void StopPresentThread();
> > + void PresentThreadRoutine();
> > + static void PresentThreadRoutineWrapper(HANDLE dev) {
> > + ((QxlDevice *)dev)->PresentThreadRoutine();
> > + }
> > + void PostToWorkerThread(QXLDrawable** drawables);
> >
> > static LONG GetMaxSourceMappingHeight(RECT* DirtyRects, ULONG
> > NumDirtyRects);
> >
> > @@ -594,6 +605,8 @@ private:
> > KEVENT m_DisplayEvent;
> > KEVENT m_CursorEvent;
> > KEVENT m_IoCmdEvent;
> > + KEVENT m_PresentEvent;
> > + KEVENT m_PresentThreadReadyEvent;
> >
> > PUCHAR m_LogPort;
> > PUCHAR m_LogBuf;
> > @@ -609,6 +622,9 @@ private:
> >
> > QXLMonitorsConfig* m_monitor_config;
> > QXLPHYSICAL* m_monitor_config_pa;
> > +
> > + QXLPresentOnlyRing m_PresentRing[1];
> > + HANDLE m_PresentThread;
> > };
> >
> > class QxlDod {
>
> Frediano
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170321/3e0a5e16/attachment.html>
More information about the Spice-devel
mailing list