[Spice-devel] [PATCH v2 12/12] qxl-wddm-dod: Non-forced memory allocations with VSync

Christophe de Dinechin dinechin at redhat.com
Tue Apr 4 09:41:13 UTC 2017


> On 1 Apr 2017, at 18:40, Yuri Benditovich <yuri.benditovich at daynix.com> wrote:
> 
> From: "yuri.benditovich at daynix.com" <yuri.benditovich at daynix.com>
> 
> In case of VSync is active allocate bitmaps for drawable objects
> using non-forced requests.

The relation with VSync being active is not obvious from this patch alone. From your earlier patch, I’d explain using something derived from the following tex, i.e explaining when it happens rather than “VSync is active"t:

when forced memory allocation used, but the driver already received
stop command and waits for thread termination. Note that in case
the VSync control is enabled stop command may happen even after the video
subsystem executes switch to VGA mode. In such case QEMU will not return
previously allocated objects (assuming the QXL driver already disabled
and ignoring callbacks from Spice server in VGA mode).

Christophe

> If immediate allocation is not possible,
> place entire bitmap into memory chunk allocated from the OS.
> If bitmap is allocated from device memory, but one of later
> chunks can't be allocated, allocate this and further chunks from
> OS memory. All these 'delayed' allocations placed into linked list
> which root entry is part of QXLOutput structure.
> From separate thread, before sending drawable objects down, review
> the list of delayed chunks and allocate device memory (forced) to
> all of them.
> The cost of solution is 2 pointers added to each drawable or cursor object.
> Cursor commands currently do not use them; in future we have an option
> to offload also cursor commands.
> 
> Signed-off-by: Yuri Benditovich <yuri.benditovich at daynix.com>
> ---
> qxldod/QxlDod.cpp | 107 ++++++++++++++++++++++++++++++++++++++++++++++++++----
> qxldod/QxlDod.h   |   3 ++
> 2 files changed, 102 insertions(+), 8 deletions(-)
> 
> diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
> index a807396..5a4e17d 100755
> --- a/qxldod/QxlDod.cpp
> +++ b/qxldod/QxlDod.cpp
> @@ -4213,6 +4213,13 @@ void QxlDevice::DrawableAddRes(QXLDrawable *drawable, Resource *res)
>    AddRes(output, res);
> }
> 
> +static FORCEINLINE PLIST_ENTRY DelayedList(QXLDrawable *pd)
> +{
> +    QXLOutput *output;
> +    output = (QXLOutput *)((UINT8 *)pd - sizeof(QXLOutput));
> +    return &output->list;
> +}
> +
> void QxlDevice::CursorCmdAddRes(QXLCursorCmd *cmd, Resource *res)
> {
>    PAGED_CODE();
> @@ -4320,6 +4327,7 @@ QXLDrawable *QxlDevice::Drawable(UINT8 type, CONST RECT *area, CONST RECT *clip,
>    drawable->surfaces_dest[1] = - 1;
>    drawable->surfaces_dest[2] = -1;
>    CopyRect(&drawable->bbox, area);
> +    InitializeListHead(DelayedList(drawable));
> 
>    if (!SetClip(clip, drawable)) {
>        DbgPrint(TRACE_LEVEL_VERBOSE, ("%s: set clip failed\n", __FUNCTION__));
> @@ -4414,7 +4422,7 @@ BOOLEAN QxlDevice::AttachNewBitmap(QXLDrawable *drawable, UINT8 *src, UINT8 *src
>    Resource *image_res;
>    InternalImage *internal;
>    QXLDataChunk *chunk;
> -    PLIST_ENTRY pDelayedList = NULL;
> +    PLIST_ENTRY pDelayedList = bForce ? NULL : DelayedList(drawable);
>    UINT8* dest, *dest_end;
> 
>    height = drawable->u.copy.src_area.bottom;
> @@ -4483,9 +4491,12 @@ BOOLEAN QxlDevice::AttachNewBitmap(QXLDrawable *drawable, UINT8 *src, UINT8 *src
>    }
> 
>    for (; src != src_end; src -= pitch, alloc_size -= line_size) {
> -        BOOLEAN b = PutBytesAlign(&chunk, &dest, &dest_end, src, line_size, alloc_size, pDelayedList);
> -        if (!b) {
> -            DbgPrint(TRACE_LEVEL_WARNING, ("%s: aborting copy of lines\n", __FUNCTION__));
> +        if (!PutBytesAlign(&chunk, &dest, &dest_end, src, line_size, alloc_size, pDelayedList)) {
> +            if (pitch < 0 && bForce) {
> +                DbgPrint(TRACE_LEVEL_WARNING, ("%s: aborting copy of lines (forced)\n", __FUNCTION__));
> +            } else {
> +                DbgPrint(TRACE_LEVEL_WARNING, ("%s: unexpected aborting copy of lines (force %d, pitch %d)\n", __FUNCTION__, bForce, pitch));
> +            }
>            return FALSE;
>        }
>    }
> @@ -4540,7 +4551,13 @@ QXLDrawable *QxlDevice::PrepareBltBits (
>    UINT8* src_end = src - pSrc->Pitch;
>    src += pSrc->Pitch * (height - 1);
> 
> -    if (!AttachNewBitmap(drawable, src, src_end, (INT)pSrc->Pitch, TRUE)) {
> +    if (!AttachNewBitmap(drawable, src, src_end, (INT)pSrc->Pitch, !g_bSupportVSync)) {
> +        PLIST_ENTRY pDelayedList = DelayedList(drawable);
> +        // if some delayed chunks were allocated, free them
> +        while (!IsListEmpty(pDelayedList)) {
> +            QXL_DELAYED_CHUNK *pdc = (QXL_DELAYED_CHUNK *)RemoveHeadList(pDelayedList);
> +            delete[] reinterpret_cast<BYTE*>(pdc);
> +        }
>        ReleaseOutput(drawable->release_info.id);
>        drawable = NULL;
>    }
> @@ -5175,11 +5192,75 @@ void QxlDevice::StopPresentThread()
>    }
> }
> 
> +QXLDataChunk *QxlDevice::MakeChunk(QXL_DELAYED_CHUNK *pdc)
> +{
> +    PAGED_CODE();
> +    QXLDataChunk *chunk = (QXLDataChunk *)AllocMem(MSPACE_TYPE_VRAM, pdc->chunk.data_size + sizeof(QXLDataChunk), TRUE);
> +    if (chunk)
> +    {
> +        chunk->data_size = pdc->chunk.data_size;
> +        chunk->next_chunk = 0;
> +        RtlCopyMemory(chunk->data, pdc->chunk.data, chunk->data_size);
> +    }
> +    return chunk;
> +}
> +
> +ULONG QxlDevice::PrepareDrawable(QXLDrawable*& drawable)
> +{
> +    PAGED_CODE();
> +    ULONG n = 0;
> +    BOOLEAN bFail = FALSE;
> +    PLIST_ENTRY pe = DelayedList(drawable);
> +    QXLDataChunk *chunk, *lastchunk = NULL;
> +    if (IsListEmpty(pe)) {
> +        return 0;
> +    }
> +
> +    while (!IsListEmpty(pe)) {
> +        QXL_DELAYED_CHUNK *pdc = (QXL_DELAYED_CHUNK *)RemoveHeadList(pe);
> +        if (!lastchunk) {
> +            lastchunk = (QXLDataChunk *)pdc->chunk.prev_chunk;
> +        }
> +        if (!lastchunk) {
> +            // bitmap was not allocated, this is single delayed chunk
> +            if (AttachNewBitmap(
> +                drawable,
> +                pdc->chunk.data,
> +                pdc->chunk.data + pdc->chunk.data_size,
> +                -(drawable->u.copy.src_area.right * 4),
> +                TRUE)) {
> +                ++n;
> +            } else {
> +                bFail = TRUE;
> +            }
> +        }
> +        if (lastchunk) {
> +            // some chunks were not allocated
> +            chunk = MakeChunk(pdc);
> +            if (chunk) {
> +                chunk->prev_chunk = PA(lastchunk, m_SurfaceMemSlot);
> +                lastchunk->next_chunk = PA(chunk, m_SurfaceMemSlot);
> +                lastchunk = chunk;
> +                ++n;
> +            } else {
> +                bFail = TRUE;
> +            }
> +        }
> +        delete[] reinterpret_cast<BYTE*>(pdc);
> +    }
> +    if (bFail) {
> +        ReleaseOutput(drawable->release_info.id);
> +        drawable = NULL;
> +    }
> +    return n;
> +}
> +
> void QxlDevice::PresentThreadRoutine()
> {
>    PAGED_CODE();
>    int wait;
>    int notify;
> +    ULONG delayed = 0;
>    QXLDrawable** drawables;
> 
>    DbgPrint(TRACE_LEVEL_INFORMATION, ("--->%s\n", __FUNCTION__));
> @@ -5202,13 +5283,23 @@ void QxlDevice::PresentThreadRoutine()
> 
>        if (drawables) {
>            for (UINT i = 0; drawables[i]; ++i)
> -                PushDrawable(drawables[i]);
> +            {
> +                ULONG n = PrepareDrawable(drawables[i]);
> +                // only reason why drawables[i] is zeroed is stop flow
> +                if (drawables[i]) {
> +                    delayed += n;
> +                    PushDrawable(drawables[i]);
> +                }
> +            }
>            delete[] drawables;
> -        }
> -        else {
> +        } else {
>            DbgPrint(TRACE_LEVEL_WARNING, ("%s is being terminated\n", __FUNCTION__));
>            break;
>        }
> +        if (delayed) {
> +            DbgPrint(TRACE_LEVEL_WARNING, ("%s: %d delayed chunks\n", __FUNCTION__, delayed));
> +            delayed = 0;
> +        }
>    }
> }
> 
> diff --git a/qxldod/QxlDod.h b/qxldod/QxlDod.h
> index 831d2b0..2cfb637 100755
> --- a/qxldod/QxlDod.h
> +++ b/qxldod/QxlDod.h
> @@ -474,6 +474,7 @@ ReleaseMutex(
> #define MAX_OUTPUT_RES 6
> 
> typedef struct QXLOutput {
> +    LIST_ENTRY list;
>    UINT32 num_res;
> #ifdef DBG
>    UINT32 type;
> @@ -612,6 +613,8 @@ private:
>    BOOLEAN PutBytesAlign(QXLDataChunk **chunk_ptr, UINT8 **now_ptr,
>                            UINT8 **end_ptr, UINT8 *src, int size,
>                            size_t alloc_size, PLIST_ENTRY pDelayed);
> +    QXLDataChunk *MakeChunk(QXL_DELAYED_CHUNK *pdc);
> +    ULONG PrepareDrawable(QXLDrawable*& drawable);
>    void AsyncIo(UCHAR  Port, UCHAR Value);
>    void SyncIo(UCHAR  Port, UCHAR Value);
>    NTSTATUS UpdateChildStatus(BOOLEAN connect);
> -- 
> 2.7.0.windows.1
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel



More information about the Spice-devel mailing list