[Spice-devel,v3,2/6] qxl-wddm-dod: PutBytesAlign supports non-forced allocation

Submitted by Yuri Benditovich on April 8, 2017, 10:46 a.m.

Details

Message ID 1491648415-4684-3-git-send-email-yuri.benditovich@daynix.com
State New
Headers show
Series "Set of patches for further support of VSync" ( rev: 3 ) in Spice

Not browsing as part of any series.

Commit Message

Yuri Benditovich April 8, 2017, 10:46 a.m.
From: "yuri.benditovich@daynix.com" <yuri.benditovich@daynix.com>

Preparation for offload of allocations from device's memory
to separate thread. Procedure PutBytesAlign is called to
allocate memory chunk from device memory and copy data to it.
With current commit the procedure (if called with non-NULL
linked list root entry parameter) can use non-forced allocation.
If such allocation fails, it allocates 'delayed' chunk from
OS memory and copies data to it. Later before sending drawable command
this chunk shall be processed, storage allocated from device memory
(forced) and data copied to it.

Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
---
 qxldod/QxlDod.cpp | 68 +++++++++++++++++++++++++++++++++++--------------------
 qxldod/QxlDod.h   |  9 +++++++-
 2 files changed, 51 insertions(+), 26 deletions(-)

Patch hide | download patch | download mbox

diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
index 5993016..3dce65b 100755
--- a/qxldod/QxlDod.cpp
+++ b/qxldod/QxlDod.cpp
@@ -4501,7 +4501,8 @@  QXLDrawable *QxlDevice::PrepareBltBits (
 
     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
             ReleaseOutput(drawable->release_info.id);
             DbgPrint(TRACE_LEVEL_ERROR, ("Cannot allocate additional bitmap for drawable\n"));
             return NULL;
@@ -4526,36 +4527,54 @@  QXLDrawable *QxlDevice::PrepareBltBits (
     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->next_chunk = 0;
+            }
+            if (!ptr && 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;
+                } 
+            }
+            if (ptr) {
+                chunk->data_size = 0;
+                now = chunk->data;
+                end = now + alloc_size;
+                cp_size = (int)MIN(end - now, size);
+            } 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;
@@ -4567,7 +4586,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)
@@ -4685,12 +4704,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__));
-            ReleaseOutput(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 059e1bd..e18afac 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 {
@@ -603,7 +610,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);

Comments

> 
> From: "yuri.benditovich@daynix.com" <yuri.benditovich@daynix.com>
> 
> Preparation for offload of allocations from device's memory
> to separate thread. Procedure PutBytesAlign is called to
> allocate memory chunk from device memory and copy data to it.
> With current commit the procedure (if called with non-NULL
> linked list root entry parameter) can use non-forced allocation.
> If such allocation fails, it allocates 'delayed' chunk from
> OS memory and copies data to it. Later before sending drawable command
> this chunk shall be processed, storage allocated from device memory
> (forced) and data copied to it.
> 
> Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
> ---
>  qxldod/QxlDod.cpp | 68
>  +++++++++++++++++++++++++++++++++++--------------------
>  qxldod/QxlDod.h   |  9 +++++++-
>  2 files changed, 51 insertions(+), 26 deletions(-)
> 
> diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
> index 5993016..3dce65b 100755
> --- a/qxldod/QxlDod.cpp
> +++ b/qxldod/QxlDod.cpp
> @@ -4501,7 +4501,8 @@ QXLDrawable *QxlDevice::PrepareBltBits (
>  
>      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
>              ReleaseOutput(drawable->release_info.id);
>              DbgPrint(TRACE_LEVEL_ERROR, ("Cannot allocate additional bitmap
>              for drawable\n"));
>              return NULL;
> @@ -4526,36 +4527,54 @@ QXLDrawable *QxlDevice::PrepareBltBits (
>      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->next_chunk = 0;
> +            }
> +            if (!ptr && 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;
> +                }
> +            }
> +            if (ptr) {
> +                chunk->data_size = 0;
> +                now = chunk->data;
> +                end = now + alloc_size;
> +                cp_size = (int)MIN(end - now, size);
> +            } 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;
> @@ -4567,7 +4586,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)
> @@ -4685,12 +4704,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__));
> -            ReleaseOutput(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 059e1bd..e18afac 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;
> +

The name of this type is quite odd.
Is suggests some Windows internal C structure.
This header is only C++ compatible, so the typedef make no sense.
The capital case is used for QXL protocol specific stuff, but
this is not even allocate in QXL memory but in system one.
I would suggest a simple QXLDelayedChunk or even DelayedChunk.

The "type" field is not used in the code.

>  class QxlDod;
>  
>  class HwDeviceInterface {
> @@ -603,7 +610,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);

Frediano
On Mon, Apr 10, 2017 at 3:33 PM, Frediano Ziglio <fziglio@redhat.com> wrote:

> >
> > From: "yuri.benditovich@daynix.com" <yuri.benditovich@daynix.com>
> >
> > Preparation for offload of allocations from device's memory
> > to separate thread. Procedure PutBytesAlign is called to
> > allocate memory chunk from device memory and copy data to it.
> > With current commit the procedure (if called with non-NULL
> > linked list root entry parameter) can use non-forced allocation.
> > If such allocation fails, it allocates 'delayed' chunk from
> > OS memory and copies data to it. Later before sending drawable command
> > this chunk shall be processed, storage allocated from device memory
> > (forced) and data copied to it.
> >
> > Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
> > ---
> >  qxldod/QxlDod.cpp | 68
> >  +++++++++++++++++++++++++++++++++++--------------------
> >  qxldod/QxlDod.h   |  9 +++++++-
> >  2 files changed, 51 insertions(+), 26 deletions(-)
> >
> > diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
> > index 5993016..3dce65b 100755
> > --- a/qxldod/QxlDod.cpp
> > +++ b/qxldod/QxlDod.cpp
> > @@ -4501,7 +4501,8 @@ QXLDrawable *QxlDevice::PrepareBltBits (
> >
> >      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
> >              ReleaseOutput(drawable->release_info.id);
> >              DbgPrint(TRACE_LEVEL_ERROR, ("Cannot allocate additional
> bitmap
> >              for drawable\n"));
> >              return NULL;
> > @@ -4526,36 +4527,54 @@ QXLDrawable *QxlDevice::PrepareBltBits (
> >      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->next_chunk = 0;
> > +            }
> > +            if (!ptr && 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;
> > +                }
> > +            }
> > +            if (ptr) {
> > +                chunk->data_size = 0;
> > +                now = chunk->data;
> > +                end = now + alloc_size;
> > +                cp_size = (int)MIN(end - now, size);
> > +            } 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;
> > @@ -4567,7 +4586,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)
> > @@ -4685,12 +4704,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__));
> > -            ReleaseOutput(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 059e1bd..e18afac 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;
> > +
>
> The name of this type is quite odd.
> Is suggests some Windows internal C structure.
> This header is only C++ compatible, so the typedef make no sense.
> The capital case is used for QXL protocol specific stuff, but
> this is not even allocate in QXL memory but in system one.
> I would suggest a simple QXLDelayedChunk or even DelayedChunk.
>
> The "type" field is not used in the code.
>

The type field is preparation for further extension - we'll need it if we
decide to make also allocations of empty Drawable objects and cursor
objects non-forced.


>
> >  class QxlDod;
> >
> >  class HwDeviceInterface {
> > @@ -603,7 +610,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);
>
> Frediano
>
> On Mon, Apr 10, 2017 at 3:33 PM, Frediano Ziglio < fziglio@redhat.com >
> wrote:

> > >
> 
> > > From: " yuri.benditovich@daynix.com " < yuri.benditovich@daynix.com >
> 
> > >
> 
> > > Preparation for offload of allocations from device's memory
> 
> > > to separate thread. Procedure PutBytesAlign is called to
> 
> > > allocate memory chunk from device memory and copy data to it.
> 
> > > With current commit the procedure (if called with non-NULL
> 
> > > linked list root entry parameter) can use non-forced allocation.
> 
> > > If such allocation fails, it allocates 'delayed' chunk from
> 
> > > OS memory and copies data to it. Later before sending drawable command
> 
> > > this chunk shall be processed, storage allocated from device memory
> 
> > > (forced) and data copied to it.
> 
> > >
> 
> > > Signed-off-by: Yuri Benditovich < yuri.benditovich@daynix.com >
> 
> > > ---
> 
> > > qxldod/QxlDod.cpp | 68
> 
> > > +++++++++++++++++++++++++++++++++++--------------------
> 
> > > qxldod/QxlDod.h | 9 +++++++-
> 
> > > 2 files changed, 51 insertions(+), 26 deletions(-)
> 
> > >
> 
> > > diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
> 
> > > index 5993016..3dce65b 100755
> 
> > > --- a/qxldod/QxlDod.cpp
> 
> > > +++ b/qxldod/QxlDod.cpp
> 
> > > @@ -4501,7 +4501,8 @@ QXLDrawable *QxlDevice::PrepareBltBits (
> 
> > >
> 
> > > 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
> 
> > > ReleaseOutput(drawable-> release_info.id );
> 
> > > DbgPrint(TRACE_LEVEL_ERROR, ("Cannot allocate additional bitmap
> 
> > > for drawable\n"));
> 
> > > return NULL;
> 
> > > @@ -4526,36 +4527,54 @@ QXLDrawable *QxlDevice::PrepareBltBits (
> 
> > > 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->next_chunk = 0;
> 
> > > + }
> 
> > > + if (!ptr && 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;
> 
> > > + }
> 
> > > + }
> 
> > > + if (ptr) {
> 
> > > + chunk->data_size = 0;
> 
> > > + now = chunk->data;
> 
> > > + end = now + alloc_size;
> 
> > > + cp_size = (int)MIN(end - now, size);
> 
> > > + } 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;
> 
> > > @@ -4567,7 +4586,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)
> 
> > > @@ -4685,12 +4704,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__));
> 
> > > - ReleaseOutput(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 059e1bd..e18afac 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;
> 
> > > +
> 

> > The name of this type is quite odd.
> 
> > Is suggests some Windows internal C structure.
> 
> > This header is only C++ compatible, so the typedef make no sense.
> 
> > The capital case is used for QXL protocol specific stuff, but
> 
> > this is not even allocate in QXL memory but in system one.
> 
> > I would suggest a simple QXLDelayedChunk or even DelayedChunk.
> 

> > The "type" field is not used in the code.
> 

> The type field is preparation for further extension - we'll need it if we
> decide to make also allocations of empty Drawable objects and cursor objects
> non-forced.

I'd prefer to avoid it or put as a comment. 
Is not clear for instance which type should be, we could use an enumeration. 
I don't see to meaning to add a possible field without knowing the usage. 

Are you fine with the type name change? 
I can easily change it (on this and following). 

> > > class QxlDod;
> 
> > >
> 
> > > class HwDeviceInterface {
> 
> > > @@ -603,7 +610,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);
> 

> > Frediano
>