On Sun, 2017-03-12 at 10:45 +0200, Yuri Benditovich wrote:
> Preparation for allocation offload.
Can you clarify what is meant by "allocation offload"?
> PutBytesAlign called to
> allocate memory chunk from device memory and copy data to it.
The previous sentence is missing a verb. This requires the reader to
make an assumption about was was intended. I think it should be
"PutBytesAlign *is* called to ..." (but considering that the next
sentence starts with "Now, ...", perhaps the previous sentence was
intended to say "PutBytesAlign *was* called to..."?). A minor issue,
perhaps, but it helps with understanability.
> Now, if called with linked list root entry, able to use non-
> forced allocation and if it fails, allocate delayed chunk from
Again missing some verbs here. Perhaps better as (additions marked like
*this*):
"Now, if *PutBytesAlign() is* called with *a* linked list root entry,
*it is* able to use..."
> OS memory and copy data to it. Later before send drawable command,
send -> sending
> this chunk shall be processed, storage allocated from device memory
> (forced) and data copied to it.
>
> Signed-off-by: Yuri Benditovich <yuri.benditovich@xxxxxxxxxx>
> ---
> qxldod/QxlDod.cpp | 78 ++++++++++++++++++++++++++++++++++++---------
> ----------
> qxldod/QxlDod.h | 9 ++++++-
> 2 files changed, 60 insertions(+), 27 deletions(-)
>
> diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
> index 9a3803a..56a4cf2 100755
> --- a/qxldod/QxlDod.cpp
> +++ b/qxldod/QxlDod.cpp
> @@ -4542,8 +4542,9 @@ QXLDrawable *QxlDevice::BltBits (
>
> for (; src != src_end; src -= pSrc->Pitch, alloc_size -=
> line_size) {
> if (!PutBytesAlign(&chunk, &dest, &dest_end, src,
> - line_size, alloc_size, line_size))
> + line_size, alloc_size, NULL))
> {
> + // not reachable when forced allocation used
> ReleaseOutputUnderLock(drawable->release_info.id);
> DbgPrint(TRACE_LEVEL_ERROR, ("Cannot allocate additional
> bitmap for drawable\n"));
> return NULL;
> @@ -4568,37 +4569,63 @@ QXLDrawable *QxlDevice::BltBits (
> return drawable;
> }
>
> +// can work in 2 modes:
> +// forced - as before, when pDelayed not provided or if VSync is not
> in use
> +// non-forced, if VSync is active and pDelayed provided. In this
> case, if memory
> +// can't be allocated immediately, allocates 'delayed chunk' and
> copies data
> +// to it. Further, before send to the device, this 'delayed chunk'
> should be processed,
> +// regular chunk allocated from device memory and the data copied to
> it
> BOOLEAN QxlDevice::PutBytesAlign(QXLDataChunk **chunk_ptr, UINT8
> **now_ptr,
> UINT8 **end_ptr, UINT8 *src, int size,
> - size_t alloc_size, uint32_t alignment)
> + size_t alloc_size, PLIST_ENTRY pDelayed)
> {
> PAGED_CODE();
> + BOOLEAN bResult = TRUE;
> + BOOLEAN bForced = !g_bSupportVSync || !pDelayed;
> QXLDataChunk *chunk = *chunk_ptr;
> UINT8 *now = *now_ptr;
> UINT8 *end = *end_ptr;
> + size_t maxAllocSize = BITS_BUF_MAX - BITS_BUF_MAX % size;
> + alloc_size = MIN(size, maxAllocSize);
> DbgPrint(TRACE_LEVEL_VERBOSE, ("---> %s\n", __FUNCTION__));
>
> while (size) {
> int cp_size = (int)MIN(end - now, size);
> if (!cp_size) {
> - size_t aligned_size;
> - aligned_size = (int)MIN(alloc_size + alignment - 1,
> BITS_BUF_MAX);
> - aligned_size -= aligned_size % alignment;
> -
> - void *ptr = AllocMem(MSPACE_TYPE_VRAM, size +
> sizeof(QXLDataChunk), TRUE);
> - if (!ptr)
> - {
> - return FALSE;
> + void *ptr = (bForced || IsListEmpty(pDelayed)) ?
> AllocMem(MSPACE_TYPE_VRAM, alloc_size + sizeof(QXLDataChunk),
> bForced) : NULL;
> + if (ptr) {
> + chunk->next_chunk = PA(ptr, m_SurfaceMemSlot);
> + ((QXLDataChunk *)ptr)->prev_chunk = PA(chunk,
> m_SurfaceMemSlot);
> + chunk = (QXLDataChunk *)ptr;
> + chunk->data_size = 0;
> + chunk->next_chunk = 0;
> + now = chunk->data;
> + end = now + alloc_size;
> + cp_size = (int)MIN(end - now, size);
> + }
> + else if (pDelayed) {
> + ptr = new (PagedPool)BYTE[alloc_size +
> sizeof(QXL_DELAYED_CHUNK)];
> + if (ptr)
> + {
> + QXL_DELAYED_CHUNK *pChunk = (QXL_DELAYED_CHUNK
> *)ptr;
> + InsertTailList(pDelayed, &pChunk->list);
> + pChunk->chunk.prev_chunk = (QXLPHYSICAL)chunk;
> + chunk = &pChunk->chunk;
> + chunk->data_size = 0;
> + now = chunk->data;
> + end = now + alloc_size;
> + cp_size = (int)MIN(end - now, size);
> + }
> + else
> + {
> + bResult = FALSE;
> + break;
> + }
> + }
> + else {
> + bResult = FALSE;
> + break;
> }
> - chunk->next_chunk = PA(ptr, m_SurfaceMemSlot);
> - ((QXLDataChunk *)ptr)->prev_chunk = PA(chunk,
> m_SurfaceMemSlot);
> - chunk = (QXLDataChunk *)ptr;
> - chunk->data_size = 0;
> - chunk->next_chunk = 0;
> - now = chunk->data;
> - end = now + size;
> -
> - cp_size = (int)MIN(end - now, size);
> }
> RtlCopyMemory(now, src, cp_size);
> src += cp_size;
> @@ -4610,7 +4637,7 @@ BOOLEAN QxlDevice::PutBytesAlign(QXLDataChunk
> **chunk_ptr, UINT8 **now_ptr,
> *now_ptr = now;
> *end_ptr = end;
> DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s\n", __FUNCTION__));
> - return TRUE;
> + return bResult;
> }
>
> VOID QxlDevice::BlackOutScreen(CURRENT_BDD_MODE* pCurrentBddMod)
> @@ -4728,12 +4755,11 @@ NTSTATUS QxlDevice::SetPointerShape(_In_
> CONST DXGKARG_SETPOINTERSHAPE* pSetPoi
> end = (UINT8 *)res + CURSOR_ALLOC_SIZE;
> src_end = src + (pSetPointerShape->Pitch * pSetPointerShape-
> >Height * num_images);
> for (; src != src_end; src += pSetPointerShape->Pitch) {
> - if (!PutBytesAlign(&chunk, &now, &end, src, line_size,
> - PAGE_SIZE, 1))
> - {
> - DbgPrint(TRACE_LEVEL_ERROR, ("%s: Failed to allocate
> cursor bitmap\n", __FUNCTION__));
> - ReleaseOutputUnderLock(cursor_cmd->release_info.id);
> - return STATUS_INSUFFICIENT_RESOURCES;
> + if (!PutBytesAlign(&chunk, &now, &end, src, line_size,
> PAGE_SIZE - PAGE_SIZE % line_size, NULL)) {
> + // we have a chance to get here only with color cursor
> bigger than 45*45
> + // and only if we modify this procedure to use non-
> forced allocation
> + DbgPrint(TRACE_LEVEL_ERROR, ("%s: failed to push part of
> shape\n", __FUNCTION__));
> + break;
> }
> }
> CursorCmdAddRes(cursor_cmd, res);
> diff --git a/qxldod/QxlDod.h b/qxldod/QxlDod.h
> index 3ee16ae..983bc29 100755
> --- a/qxldod/QxlDod.h
> +++ b/qxldod/QxlDod.h
> @@ -260,6 +260,13 @@ public:
> };
> #endif
>
> +typedef struct _QXL_DELAYED_CHUNK
> +{
> + LIST_ENTRY list;
> + UINT32 type;
> + QXLDataChunk chunk;
> +} QXL_DELAYED_CHUNK;
> +
> class QxlDod;
>
> class HwDeviceInterface {
> @@ -604,7 +611,7 @@ private:
> void PushCursor(void);
> BOOLEAN PutBytesAlign(QXLDataChunk **chunk_ptr, UINT8 **now_ptr,
> UINT8 **end_ptr, UINT8 *src, int size,
> - size_t alloc_size, uint32_t alignment);
> + size_t alloc_size, PLIST_ENTRY
> pDelayed);
> void AsyncIo(UCHAR Port, UCHAR Value);
> void SyncIo(UCHAR Port, UCHAR Value);
> NTSTATUS UpdateChildStatus(BOOLEAN connect);
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel