[Qemu-devel] [PATCH v2 0/3] spice: add support for dmabufs

[Qemu-devel] [PATCH v2 0/3] spice: add support for dmabufs

[Gerd Hoffmann]

v2: rebase, fix codestyle issues.

Gerd Hoffmann (3):
  spice: drop dprint() debug logging
  spice: add scanout_dmabuf support
  spice: add cursor_dmabuf support

 include/ui/spice-display.h |   9 +++
 ui/spice-display.c         | 194 ++++++++++++++++++++++++++++++++-------------
 ui/trace-events            |  12 +++
 3 files changed, 162 insertions(+), 53 deletions(-)

-- 
2.9.3

[Qemu-devel] [PATCH v2 1/3] spice: drop dprint() debug logging

[Gerd Hoffmann]

Some calls are deleted, some are converted into tracepoints.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/spice-display.c | 75 ++++++++++++++++--------------------------------------
 ui/trace-events    |  9 +++++++
 2 files changed, 31 insertions(+), 53 deletions(-)

diff --git a/ui/spice-display.c b/ui/spice-display.c
index 98ccdfb687..f3ae6beb3d 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -26,20 +26,8 @@
 
 #include "ui/spice-display.h"
 
-static int debug = 0;
 bool spice_opengl;
 
-static void GCC_FMT_ATTR(2, 3) dprint(int level, const char *fmt, ...)
-{
-    va_list args;
-
-    if (level <= debug) {
-        va_start(args, fmt);
-        vfprintf(stderr, fmt, args);
-        va_end(args);
-    }
-}
-
 int qemu_spice_rect_is_empty(const QXLRect* r)
 {
     return r->top == r->bottom || r->left == r->right;
@@ -322,8 +310,6 @@ void qemu_spice_create_host_memslot(SimpleSpiceDisplay *ssd)
 {
     QXLDevMemSlot memslot;
 
-    dprint(1, "%s/%d:\n", __func__, ssd->qxl.id);
-
     memset(&memslot, 0, sizeof(memslot));
     memslot.slot_group_id = MEMSLOT_GROUP_HOST;
     memslot.virt_end = ~0;
@@ -347,10 +333,6 @@ void qemu_spice_create_host_primary(SimpleSpiceDisplay *ssd)
         ssd->buf = g_malloc(ssd->bufsize);
     }
 
-    dprint(1, "%s/%d: %ux%u (size %" PRIu64 "/%d)\n", __func__, ssd->qxl.id,
-           surface_width(ssd->ds), surface_height(ssd->ds),
-           surface_size, ssd->bufsize);
-
     surface.format     = SPICE_SURFACE_FMT_32_xRGB;
     surface.width      = surface_width(ssd->ds);
     surface.height     = surface_height(ssd->ds);
@@ -366,8 +348,6 @@ void qemu_spice_create_host_primary(SimpleSpiceDisplay *ssd)
 
 void qemu_spice_destroy_host_primary(SimpleSpiceDisplay *ssd)
 {
-    dprint(1, "%s/%d:\n", __func__, ssd->qxl.id);
-
     qemu_spice_destroy_primary_surface(ssd, 0, QXL_SYNC);
 }
 
@@ -389,8 +369,7 @@ void qemu_spice_display_update(SimpleSpiceDisplay *ssd,
 {
     QXLRect update_area;
 
-    dprint(2, "%s/%d: x %d y %d w %d h %d\n", __func__,
-           ssd->qxl.id, x, y, w, h);
+    trace_qemu_spice_display_update(ssd->qxl.id, x, y, w, h);
     update_area.left = x,
     update_area.right = x + w;
     update_area.top = y;
@@ -413,8 +392,10 @@ void qemu_spice_display_switch(SimpleSpiceDisplay *ssd,
         surface_height(surface) == pixman_image_get_height(ssd->surface) &&
         surface_format(surface) == pixman_image_get_format(ssd->surface)) {
         /* no-resize fast path: just swap backing store */
-        dprint(1, "%s/%d: fast (%dx%d)\n", __func__, ssd->qxl.id,
-               surface_width(surface), surface_height(surface));
+        trace_qemu_spice_display_surface(ssd->qxl.id,
+                                         surface_width(surface),
+                                         surface_height(surface),
+                                         true);
         qemu_mutex_lock(&ssd->lock);
         ssd->ds = surface;
         pixman_image_unref(ssd->surface);
@@ -427,11 +408,10 @@ void qemu_spice_display_switch(SimpleSpiceDisplay *ssd,
     }
 
     /* full mode switch */
-    dprint(1, "%s/%d: full (%dx%d -> %dx%d)\n", __func__, ssd->qxl.id,
-           ssd->surface ? pixman_image_get_width(ssd->surface)  : 0,
-           ssd->surface ? pixman_image_get_height(ssd->surface) : 0,
-           surface ? surface_width(surface)  : 0,
-           surface ? surface_height(surface) : 0);
+    trace_qemu_spice_display_surface(ssd->qxl.id,
+                                     surface_width(surface),
+                                     surface_height(surface),
+                                     false);
 
     memset(&ssd->dirty, 0, sizeof(ssd->dirty));
     if (ssd->surface) {
@@ -495,7 +475,6 @@ void qemu_spice_cursor_refresh_bh(void *opaque)
 
 void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd)
 {
-    dprint(3, "%s/%d:\n", __func__, ssd->qxl.id);
     graphic_hw_update(ssd->dcl.con);
 
     qemu_mutex_lock(&ssd->lock);
@@ -505,10 +484,10 @@ void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd)
     }
     qemu_mutex_unlock(&ssd->lock);
 
+    trace_qemu_spice_display_refresh(ssd->qxl.id, ssd->notify);
     if (ssd->notify) {
         ssd->notify = 0;
         qemu_spice_wakeup(ssd);
-        dprint(2, "%s/%d: notify\n", __func__, ssd->qxl.id);
     }
 }
 
@@ -516,21 +495,17 @@ void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd)
 
 static void interface_attach_worker(QXLInstance *sin, QXLWorker *qxl_worker)
 {
-    SimpleSpiceDisplay *ssd = container_of(sin, SimpleSpiceDisplay, qxl);
-
-    dprint(1, "%s/%d:\n", __func__, ssd->qxl.id);
+    /* nothing to do */
 }
 
 static void interface_set_compression_level(QXLInstance *sin, int level)
 {
-    dprint(1, "%s/%d:\n", __func__, sin->id);
     /* nothing to do */
 }
 
 #if SPICE_NEEDS_SET_MM_TIME
 static void interface_set_mm_time(QXLInstance *sin, uint32_t mm_time)
 {
-    dprint(3, "%s/%d:\n", __func__, sin->id);
     /* nothing to do */
 }
 #endif
@@ -554,8 +529,6 @@ static int interface_get_command(QXLInstance *sin, QXLCommandExt *ext)
     SimpleSpiceUpdate *update;
     int ret = false;
 
-    dprint(3, "%s/%d:\n", __func__, ssd->qxl.id);
-
     qemu_mutex_lock(&ssd->lock);
     update = QTAILQ_FIRST(&ssd->updates);
     if (update != NULL) {
@@ -570,7 +543,6 @@ static int interface_get_command(QXLInstance *sin, QXLCommandExt *ext)
 
 static int interface_req_cmd_notification(QXLInstance *sin)
 {
-    dprint(2, "%s/%d:\n", __func__, sin->id);
     return 1;
 }
 
@@ -582,7 +554,6 @@ static void interface_release_resource(QXLInstance *sin,
     SimpleSpiceCursor *cursor;
     QXLCommandExt *ext;
 
-    dprint(2, "%s/%d:\n", __func__, ssd->qxl.id);
     ext = (void *)(intptr_t)(rext.info->id);
     switch (ext->cmd.type) {
     case QXL_CMD_DRAW:
@@ -603,8 +574,6 @@ static int interface_get_cursor_command(QXLInstance *sin, QXLCommandExt *ext)
     SimpleSpiceDisplay *ssd = container_of(sin, SimpleSpiceDisplay, qxl);
     int ret;
 
-    dprint(3, "%s/%d:\n", __func__, ssd->qxl.id);
-
     qemu_mutex_lock(&ssd->lock);
     if (ssd->ptr_define) {
         *ext = ssd->ptr_define->ext;
@@ -623,7 +592,6 @@ static int interface_get_cursor_command(QXLInstance *sin, QXLCommandExt *ext)
 
 static int interface_req_cursor_notification(QXLInstance *sin)
 {
-    dprint(2, "%s:\n", __func__);
     return 1;
 }
 
@@ -680,7 +648,7 @@ static void interface_set_client_capabilities(QXLInstance *sin,
                                               uint8_t client_present,
                                               uint8_t caps[58])
 {
-    dprint(3, "%s:\n", __func__);
+    /* nothing to do */
 }
 
 static int interface_client_monitors_config(QXLInstance *sin,
@@ -705,9 +673,9 @@ static int interface_client_monitors_config(QXLInstance *sin,
         info.width  = mc->monitors[head].width;
         info.height = mc->monitors[head].height;
     }
+
+    trace_qemu_spice_ui_info(ssd->qxl.id, info.width, info.height);
     dpy_set_ui_info(ssd->dcl.con, &info);
-    dprint(1, "%s/%d: size %dx%d\n", __func__, ssd->qxl.id,
-           info.width, info.height);
     return 1;
 }
 
@@ -902,9 +870,10 @@ static void spice_gl_switch(DisplayChangeListener *dcl,
             return;
         }
 
-        dprint(1, "%s: %dx%d (stride %d/%d, fourcc 0x%x)\n", __func__,
-               surface_width(ssd->ds), surface_height(ssd->ds),
-               surface_stride(ssd->ds), stride, fourcc);
+        trace_qemu_spice_gl_surface(ssd->qxl.id,
+                                    surface_width(ssd->ds),
+                                    surface_height(ssd->ds),
+                                    fourcc);
 
         /* note: spice server will close the fd */
         spice_qxl_gl_scanout(&ssd->qxl, fd,
@@ -932,7 +901,7 @@ static void qemu_spice_gl_scanout_disable(DisplayChangeListener *dcl)
 {
     SimpleSpiceDisplay *ssd = container_of(dcl, SimpleSpiceDisplay, dcl);
 
-    dprint(1, "%s: no framebuffer\n", __func__);
+    trace_qemu_spice_gl_scanout_disable(ssd->qxl.id);
     spice_qxl_gl_scanout(&ssd->qxl, -1, 0, 0, 0, 0, false);
     qemu_spice_gl_monitor_config(ssd, 0, 0, 0, 0);
     ssd->have_surface = false;
@@ -957,8 +926,7 @@ static void qemu_spice_gl_scanout_texture(DisplayChangeListener *dcl,
         fprintf(stderr, "%s: failed to get fd for texture\n", __func__);
         return;
     }
-    dprint(1, "%s: %dx%d (stride %d, fourcc 0x%x)\n", __func__,
-           w, h, stride, fourcc);
+    trace_qemu_spice_gl_scanout_texture(ssd->qxl.id, w, h, fourcc);
 
     /* note: spice server will close the fd */
     spice_qxl_gl_scanout(&ssd->qxl, fd, backing_width, backing_height,
@@ -978,7 +946,8 @@ static void qemu_spice_gl_update(DisplayChangeListener *dcl,
         return;
     }
 
-    dprint(2, "%s: %dx%d+%d+%d\n", __func__, w, h, x, y);
+
+    trace_qemu_spice_gl_update(ssd->qxl.id, w, h, x, y);
     qemu_spice_gl_block(ssd, true);
     cookie = (uintptr_t)qxl_cookie_new(QXL_COOKIE_TYPE_GL_DRAW_DONE, 0);
     spice_qxl_gl_draw_async(&ssd->qxl, x, y, w, h, cookie);
diff --git a/ui/trace-events b/ui/trace-events
index 861b68a305..518e950a01 100644
--- a/ui/trace-events
+++ b/ui/trace-events
@@ -75,6 +75,15 @@ qemu_spice_create_primary_surface(int qid, uint32_t sid, void *surface, int asyn
 qemu_spice_destroy_primary_surface(int qid, uint32_t sid, int async) "%d sid=%u async=%d"
 qemu_spice_wakeup(uint32_t qid) "%d"
 qemu_spice_create_update(uint32_t left, uint32_t right, uint32_t top, uint32_t bottom) "lr %d -> %d,  tb -> %d -> %d"
+qemu_spice_display_update(int qid, uint32_t x, uint32_t y, uint32_t w, uint32_t h) "%d +%d+%d %dx%d"
+qemu_spice_display_surface(int qid, uint32_t w, uint32_t h, int fast) "%d %dx%d, fast %d"
+qemu_spice_display_refresh(int qid, int notify) "%d notify %d"
+qemu_spice_ui_info(int qid, uint32_t width, uint32_t height) "%d %dx%d"
+
+qemu_spice_gl_surface(int qid, uint32_t w, uint32_t h, uint32_t fourcc) "%d %dx%d, fourcc 0x%x"
+qemu_spice_gl_scanout_disable(int qid) "%d"
+qemu_spice_gl_scanout_texture(int qid, uint32_t w, uint32_t h, uint32_t fourcc) "%d %dx%d, fourcc 0x%x"
+qemu_spice_gl_update(int qid, uint32_t x, uint32_t y, uint32_t w, uint32_t h) "%d +%d+%d %dx%d"
 
 # ui/keymaps.c
 keymap_parse(const char *file) "file %s"
-- 
2.9.3

Re: [Qemu-devel] [PATCH v2 1/3] spice: drop dprint() debug logging

[Marc-André Lureau]

Hi

On Tue, Mar 6, 2018 at 9:38 AM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Some calls are deleted, some are converted into tracepoints.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  ui/spice-display.c | 75 ++++++++++++++++--------------------------------------
>  ui/trace-events    |  9 +++++++
>  2 files changed, 31 insertions(+), 53 deletions(-)
>
> diff --git a/ui/spice-display.c b/ui/spice-display.c
> index 98ccdfb687..f3ae6beb3d 100644
> --- a/ui/spice-display.c
> +++ b/ui/spice-display.c
> @@ -26,20 +26,8 @@
>
>  #include "ui/spice-display.h"
>
> -static int debug = 0;
>  bool spice_opengl;
>
> -static void GCC_FMT_ATTR(2, 3) dprint(int level, const char *fmt, ...)
> -{
> -    va_list args;
> -
> -    if (level <= debug) {
> -        va_start(args, fmt);
> -        vfprintf(stderr, fmt, args);
> -        va_end(args);
> -    }
> -}
> -
>  int qemu_spice_rect_is_empty(const QXLRect* r)
>  {
>      return r->top == r->bottom || r->left == r->right;
> @@ -322,8 +310,6 @@ void qemu_spice_create_host_memslot(SimpleSpiceDisplay *ssd)
>  {
>      QXLDevMemSlot memslot;
>
> -    dprint(1, "%s/%d:\n", __func__, ssd->qxl.id);
> -
>      memset(&memslot, 0, sizeof(memslot));
>      memslot.slot_group_id = MEMSLOT_GROUP_HOST;
>      memslot.virt_end = ~0;
> @@ -347,10 +333,6 @@ void qemu_spice_create_host_primary(SimpleSpiceDisplay *ssd)
>          ssd->buf = g_malloc(ssd->bufsize);
>      }
>
> -    dprint(1, "%s/%d: %ux%u (size %" PRIu64 "/%d)\n", __func__, ssd->qxl.id,
> -           surface_width(ssd->ds), surface_height(ssd->ds),
> -           surface_size, ssd->bufsize);
> -
>      surface.format     = SPICE_SURFACE_FMT_32_xRGB;
>      surface.width      = surface_width(ssd->ds);
>      surface.height     = surface_height(ssd->ds);
> @@ -366,8 +348,6 @@ void qemu_spice_create_host_primary(SimpleSpiceDisplay *ssd)
>
>  void qemu_spice_destroy_host_primary(SimpleSpiceDisplay *ssd)
>  {
> -    dprint(1, "%s/%d:\n", __func__, ssd->qxl.id);
> -
>      qemu_spice_destroy_primary_surface(ssd, 0, QXL_SYNC);
>  }
>
> @@ -389,8 +369,7 @@ void qemu_spice_display_update(SimpleSpiceDisplay *ssd,
>  {
>      QXLRect update_area;
>
> -    dprint(2, "%s/%d: x %d y %d w %d h %d\n", __func__,
> -           ssd->qxl.id, x, y, w, h);
> +    trace_qemu_spice_display_update(ssd->qxl.id, x, y, w, h);
>      update_area.left = x,
>      update_area.right = x + w;
>      update_area.top = y;
> @@ -413,8 +392,10 @@ void qemu_spice_display_switch(SimpleSpiceDisplay *ssd,
>          surface_height(surface) == pixman_image_get_height(ssd->surface) &&
>          surface_format(surface) == pixman_image_get_format(ssd->surface)) {
>          /* no-resize fast path: just swap backing store */
> -        dprint(1, "%s/%d: fast (%dx%d)\n", __func__, ssd->qxl.id,
> -               surface_width(surface), surface_height(surface));
> +        trace_qemu_spice_display_surface(ssd->qxl.id,
> +                                         surface_width(surface),
> +                                         surface_height(surface),
> +                                         true);
>          qemu_mutex_lock(&ssd->lock);
>          ssd->ds = surface;
>          pixman_image_unref(ssd->surface);
> @@ -427,11 +408,10 @@ void qemu_spice_display_switch(SimpleSpiceDisplay *ssd,
>      }
>
>      /* full mode switch */
> -    dprint(1, "%s/%d: full (%dx%d -> %dx%d)\n", __func__, ssd->qxl.id,
> -           ssd->surface ? pixman_image_get_width(ssd->surface)  : 0,
> -           ssd->surface ? pixman_image_get_height(ssd->surface) : 0,
> -           surface ? surface_width(surface)  : 0,
> -           surface ? surface_height(surface) : 0);
> +    trace_qemu_spice_display_surface(ssd->qxl.id,
> +                                     surface_width(surface),
> +                                     surface_height(surface),
> +                                     false);

You are now assuming surface is always != NULL, but
dpy_gfx_replace_surface() for example, has assert( || surface == NULL)
before calling dpy_gfx_switch(). If such invariants is added, it would
be nice to document it in DisplayChangeListenerOps declaration.

>
>      memset(&ssd->dirty, 0, sizeof(ssd->dirty));
>      if (ssd->surface) {
> @@ -495,7 +475,6 @@ void qemu_spice_cursor_refresh_bh(void *opaque)
>
>  void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd)
>  {
> -    dprint(3, "%s/%d:\n", __func__, ssd->qxl.id);
>      graphic_hw_update(ssd->dcl.con);
>
>      qemu_mutex_lock(&ssd->lock);
> @@ -505,10 +484,10 @@ void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd)
>      }
>      qemu_mutex_unlock(&ssd->lock);
>
> +    trace_qemu_spice_display_refresh(ssd->qxl.id, ssd->notify);
>      if (ssd->notify) {
>          ssd->notify = 0;
>          qemu_spice_wakeup(ssd);
> -        dprint(2, "%s/%d: notify\n", __func__, ssd->qxl.id);
>      }
>  }
>
> @@ -516,21 +495,17 @@ void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd)
>
>  static void interface_attach_worker(QXLInstance *sin, QXLWorker *qxl_worker)
>  {
> -    SimpleSpiceDisplay *ssd = container_of(sin, SimpleSpiceDisplay, qxl);
> -
> -    dprint(1, "%s/%d:\n", __func__, ssd->qxl.id);
> +    /* nothing to do */
>  }
>
>  static void interface_set_compression_level(QXLInstance *sin, int level)
>  {
> -    dprint(1, "%s/%d:\n", __func__, sin->id);
>      /* nothing to do */
>  }
>
>  #if SPICE_NEEDS_SET_MM_TIME
>  static void interface_set_mm_time(QXLInstance *sin, uint32_t mm_time)
>  {
> -    dprint(3, "%s/%d:\n", __func__, sin->id);
>      /* nothing to do */
>  }
>  #endif
> @@ -554,8 +529,6 @@ static int interface_get_command(QXLInstance *sin, QXLCommandExt *ext)
>      SimpleSpiceUpdate *update;
>      int ret = false;
>
> -    dprint(3, "%s/%d:\n", __func__, ssd->qxl.id);
> -
>      qemu_mutex_lock(&ssd->lock);
>      update = QTAILQ_FIRST(&ssd->updates);
>      if (update != NULL) {
> @@ -570,7 +543,6 @@ static int interface_get_command(QXLInstance *sin, QXLCommandExt *ext)
>
>  static int interface_req_cmd_notification(QXLInstance *sin)
>  {
> -    dprint(2, "%s/%d:\n", __func__, sin->id);
>      return 1;
>  }
>
> @@ -582,7 +554,6 @@ static void interface_release_resource(QXLInstance *sin,
>      SimpleSpiceCursor *cursor;
>      QXLCommandExt *ext;
>
> -    dprint(2, "%s/%d:\n", __func__, ssd->qxl.id);
>      ext = (void *)(intptr_t)(rext.info->id);
>      switch (ext->cmd.type) {
>      case QXL_CMD_DRAW:
> @@ -603,8 +574,6 @@ static int interface_get_cursor_command(QXLInstance *sin, QXLCommandExt *ext)
>      SimpleSpiceDisplay *ssd = container_of(sin, SimpleSpiceDisplay, qxl);
>      int ret;
>
> -    dprint(3, "%s/%d:\n", __func__, ssd->qxl.id);
> -
>      qemu_mutex_lock(&ssd->lock);
>      if (ssd->ptr_define) {
>          *ext = ssd->ptr_define->ext;
> @@ -623,7 +592,6 @@ static int interface_get_cursor_command(QXLInstance *sin, QXLCommandExt *ext)
>
>  static int interface_req_cursor_notification(QXLInstance *sin)
>  {
> -    dprint(2, "%s:\n", __func__);
>      return 1;
>  }
>
> @@ -680,7 +648,7 @@ static void interface_set_client_capabilities(QXLInstance *sin,
>                                                uint8_t client_present,
>                                                uint8_t caps[58])
>  {
> -    dprint(3, "%s:\n", __func__);
> +    /* nothing to do */
>  }
>
>  static int interface_client_monitors_config(QXLInstance *sin,
> @@ -705,9 +673,9 @@ static int interface_client_monitors_config(QXLInstance *sin,
>          info.width  = mc->monitors[head].width;
>          info.height = mc->monitors[head].height;
>      }
> +
> +    trace_qemu_spice_ui_info(ssd->qxl.id, info.width, info.height);
>      dpy_set_ui_info(ssd->dcl.con, &info);
> -    dprint(1, "%s/%d: size %dx%d\n", __func__, ssd->qxl.id,
> -           info.width, info.height);
>      return 1;
>  }
>
> @@ -902,9 +870,10 @@ static void spice_gl_switch(DisplayChangeListener *dcl,
>              return;
>          }
>
> -        dprint(1, "%s: %dx%d (stride %d/%d, fourcc 0x%x)\n", __func__,
> -               surface_width(ssd->ds), surface_height(ssd->ds),
> -               surface_stride(ssd->ds), stride, fourcc);
> +        trace_qemu_spice_gl_surface(ssd->qxl.id,
> +                                    surface_width(ssd->ds),
> +                                    surface_height(ssd->ds),
> +                                    fourcc);
>
>          /* note: spice server will close the fd */
>          spice_qxl_gl_scanout(&ssd->qxl, fd,
> @@ -932,7 +901,7 @@ static void qemu_spice_gl_scanout_disable(DisplayChangeListener *dcl)
>  {
>      SimpleSpiceDisplay *ssd = container_of(dcl, SimpleSpiceDisplay, dcl);
>
> -    dprint(1, "%s: no framebuffer\n", __func__);
> +    trace_qemu_spice_gl_scanout_disable(ssd->qxl.id);
>      spice_qxl_gl_scanout(&ssd->qxl, -1, 0, 0, 0, 0, false);
>      qemu_spice_gl_monitor_config(ssd, 0, 0, 0, 0);
>      ssd->have_surface = false;
> @@ -957,8 +926,7 @@ static void qemu_spice_gl_scanout_texture(DisplayChangeListener *dcl,
>          fprintf(stderr, "%s: failed to get fd for texture\n", __func__);
>          return;
>      }
> -    dprint(1, "%s: %dx%d (stride %d, fourcc 0x%x)\n", __func__,
> -           w, h, stride, fourcc);
> +    trace_qemu_spice_gl_scanout_texture(ssd->qxl.id, w, h, fourcc);
>
>      /* note: spice server will close the fd */
>      spice_qxl_gl_scanout(&ssd->qxl, fd, backing_width, backing_height,
> @@ -978,7 +946,8 @@ static void qemu_spice_gl_update(DisplayChangeListener *dcl,
>          return;
>      }
>
> -    dprint(2, "%s: %dx%d+%d+%d\n", __func__, w, h, x, y);
> +
> +    trace_qemu_spice_gl_update(ssd->qxl.id, w, h, x, y);
>      qemu_spice_gl_block(ssd, true);
>      cookie = (uintptr_t)qxl_cookie_new(QXL_COOKIE_TYPE_GL_DRAW_DONE, 0);
>      spice_qxl_gl_draw_async(&ssd->qxl, x, y, w, h, cookie);
> diff --git a/ui/trace-events b/ui/trace-events
> index 861b68a305..518e950a01 100644
> --- a/ui/trace-events
> +++ b/ui/trace-events
> @@ -75,6 +75,15 @@ qemu_spice_create_primary_surface(int qid, uint32_t sid, void *surface, int asyn
>  qemu_spice_destroy_primary_surface(int qid, uint32_t sid, int async) "%d sid=%u async=%d"
>  qemu_spice_wakeup(uint32_t qid) "%d"
>  qemu_spice_create_update(uint32_t left, uint32_t right, uint32_t top, uint32_t bottom) "lr %d -> %d,  tb -> %d -> %d"
> +qemu_spice_display_update(int qid, uint32_t x, uint32_t y, uint32_t w, uint32_t h) "%d +%d+%d %dx%d"
> +qemu_spice_display_surface(int qid, uint32_t w, uint32_t h, int fast) "%d %dx%d, fast %d"
> +qemu_spice_display_refresh(int qid, int notify) "%d notify %d"
> +qemu_spice_ui_info(int qid, uint32_t width, uint32_t height) "%d %dx%d"
> +
> +qemu_spice_gl_surface(int qid, uint32_t w, uint32_t h, uint32_t fourcc) "%d %dx%d, fourcc 0x%x"
> +qemu_spice_gl_scanout_disable(int qid) "%d"
> +qemu_spice_gl_scanout_texture(int qid, uint32_t w, uint32_t h, uint32_t fourcc) "%d %dx%d, fourcc 0x%x"
> +qemu_spice_gl_update(int qid, uint32_t x, uint32_t y, uint32_t w, uint32_t h) "%d +%d+%d %dx%d"
>
>  # ui/keymaps.c
>  keymap_parse(const char *file) "file %s"
> --
> 2.9.3
>
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v2 1/3] spice: drop dprint() debug logging

[Gerd Hoffmann]

> > @@ -413,8 +392,10 @@ void qemu_spice_display_switch(SimpleSpiceDisplay *ssd,
> >          surface_height(surface) == pixman_image_get_height(ssd->surface) &&
> >          surface_format(surface) == pixman_image_get_format(ssd->surface)) {
> >          /* no-resize fast path: just swap backing store */
> > -        dprint(1, "%s/%d: fast (%dx%d)\n", __func__, ssd->qxl.id,
> > -               surface_width(surface), surface_height(surface));
> > +        trace_qemu_spice_display_surface(ssd->qxl.id,
> > +                                         surface_width(surface),
> > +                                         surface_height(surface),
> > +                                         true);
> >          qemu_mutex_lock(&ssd->lock);
> >          ssd->ds = surface;
> >          pixman_image_unref(ssd->surface);
> > @@ -427,11 +408,10 @@ void qemu_spice_display_switch(SimpleSpiceDisplay *ssd,
> >      }
> >
> >      /* full mode switch */
> > -    dprint(1, "%s/%d: full (%dx%d -> %dx%d)\n", __func__, ssd->qxl.id,
> > -           ssd->surface ? pixman_image_get_width(ssd->surface)  : 0,
> > -           ssd->surface ? pixman_image_get_height(ssd->surface) : 0,
> > -           surface ? surface_width(surface)  : 0,
> > -           surface ? surface_height(surface) : 0);
> > +    trace_qemu_spice_display_surface(ssd->qxl.id,
> > +                                     surface_width(surface),
> > +                                     surface_height(surface),
> > +                                     false);
> 
> You are now assuming surface is always != NULL, but
> dpy_gfx_replace_surface() for example, has assert( || surface == NULL)
> before calling dpy_gfx_switch().

Oops.  Wasn't intentional (cut & paste from above).  Will fix.

cheers,
  Gerd

[Qemu-devel] [PATCH v2 2/3] spice: add scanout_dmabuf support

[Gerd Hoffmann]

Add support for scanout dmabufs.  Just
pass them through to spice-server.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/spice-display.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/ui/spice-display.c b/ui/spice-display.c
index f3ae6beb3d..a494db1196 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -936,6 +936,20 @@ static void qemu_spice_gl_scanout_texture(DisplayChangeListener *dcl,
     ssd->have_scanout = true;
 }
 
+static void qemu_spice_gl_scanout_dmabuf(DisplayChangeListener *dcl,
+                                         QemuDmaBuf *dmabuf)
+{
+    SimpleSpiceDisplay *ssd = container_of(dcl, SimpleSpiceDisplay, dcl);
+
+    /* note: spice server will close the fd, so hand over a dup */
+    spice_qxl_gl_scanout(&ssd->qxl, dup(dmabuf->fd),
+                         dmabuf->width, dmabuf->height,
+                         dmabuf->stride, dmabuf->fourcc, false);
+    qemu_spice_gl_monitor_config(ssd, 0, 0, dmabuf->width, dmabuf->height);
+    ssd->have_surface = false;
+    ssd->have_scanout = true;
+}
+
 static void qemu_spice_gl_update(DisplayChangeListener *dcl,
                                  uint32_t x, uint32_t y, uint32_t w, uint32_t h)
 {
@@ -969,6 +983,7 @@ static const DisplayChangeListenerOps display_listener_gl_ops = {
 
     .dpy_gl_scanout_disable  = qemu_spice_gl_scanout_disable,
     .dpy_gl_scanout_texture  = qemu_spice_gl_scanout_texture,
+    .dpy_gl_scanout_dmabuf   = qemu_spice_gl_scanout_dmabuf,
     .dpy_gl_update           = qemu_spice_gl_update,
 };
 
-- 
2.9.3

Re: [Qemu-devel] [PATCH v2 2/3] spice: add scanout_dmabuf support

[Marc-André Lureau]

Hi

On Tue, Mar 6, 2018 at 9:38 AM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Add support for scanout dmabufs.  Just
> pass them through to spice-server.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  ui/spice-display.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/ui/spice-display.c b/ui/spice-display.c
> index f3ae6beb3d..a494db1196 100644
> --- a/ui/spice-display.c
> +++ b/ui/spice-display.c
> @@ -936,6 +936,20 @@ static void qemu_spice_gl_scanout_texture(DisplayChangeListener *dcl,
>      ssd->have_scanout = true;
>  }
>
> +static void qemu_spice_gl_scanout_dmabuf(DisplayChangeListener *dcl,
> +                                         QemuDmaBuf *dmabuf)
> +{
> +    SimpleSpiceDisplay *ssd = container_of(dcl, SimpleSpiceDisplay, dcl);
> +
> +    /* note: spice server will close the fd, so hand over a dup */
> +    spice_qxl_gl_scanout(&ssd->qxl, dup(dmabuf->fd),
> +                         dmabuf->width, dmabuf->height,
> +                         dmabuf->stride, dmabuf->fourcc, false);
> +    qemu_spice_gl_monitor_config(ssd, 0, 0, dmabuf->width, dmabuf->height);
> +    ssd->have_surface = false;
> +    ssd->have_scanout = true;
> +}
> +
>  static void qemu_spice_gl_update(DisplayChangeListener *dcl,
>                                   uint32_t x, uint32_t y, uint32_t w, uint32_t h)
>  {
> @@ -969,6 +983,7 @@ static const DisplayChangeListenerOps display_listener_gl_ops = {
>
>      .dpy_gl_scanout_disable  = qemu_spice_gl_scanout_disable,
>      .dpy_gl_scanout_texture  = qemu_spice_gl_scanout_texture,
> +    .dpy_gl_scanout_dmabuf   = qemu_spice_gl_scanout_dmabuf,
>      .dpy_gl_update           = qemu_spice_gl_update,
>  };
>
> --
> 2.9.3
>
>

Looks good. Btw, how is multi-monitor designed? Is there going to be a
scanout/dmabuf per monitor when using vfio/mdev?

-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v2 2/3] spice: add scanout_dmabuf support

[Gerd Hoffmann]

> > @@ -969,6 +983,7 @@ static const DisplayChangeListenerOps display_listener_gl_ops = {
> >
> >      .dpy_gl_scanout_disable  = qemu_spice_gl_scanout_disable,
> >      .dpy_gl_scanout_texture  = qemu_spice_gl_scanout_texture,
> > +    .dpy_gl_scanout_dmabuf   = qemu_spice_gl_scanout_dmabuf,
> >      .dpy_gl_update           = qemu_spice_gl_update,
> >  };
> >
> > --
> > 2.9.3
> >
> >
> 
> Looks good. Btw, how is multi-monitor designed? Is there going to be a
> scanout/dmabuf per monitor when using vfio/mdev?

Yes, that would be one QemuConsole per head (simliar to virtio-gpu).
There is no multihead support right now though.  Not in the drivers, and
also not in the vfio API.  Adding api for it without an actual test case
didn't look like a good plan to me.  Extending the API later is easier
than dealing with a broken api in case we miss some detail and only
figure later when the first driver tries to use the untested API ...

cheers,
  Gerd

[Qemu-devel] [PATCH v2 3/3] spice: add cursor_dmabuf support

[Gerd Hoffmann]

Add support for cursor dmabufs.  qemu has to render the cursor for
that, so in case a cursor is present qemu allocates a new dmabuf, blits
the scanout, blends in the pointer and passes on the new dmabuf to
spice-server.  Without cursor qemu continues to simply pass on the
scanout dmabuf as-is.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/ui/spice-display.h |   9 ++++
 ui/spice-display.c         | 114 +++++++++++++++++++++++++++++++++++++++++++--
 ui/trace-events            |   3 ++
 3 files changed, 121 insertions(+), 5 deletions(-)

diff --git a/include/ui/spice-display.h b/include/ui/spice-display.h
index 6b5c73b21c..87a84a59d4 100644
--- a/include/ui/spice-display.h
+++ b/include/ui/spice-display.h
@@ -122,6 +122,15 @@ struct SimpleSpiceDisplay {
     int gl_updates;
     bool have_scanout;
     bool have_surface;
+
+    QemuDmaBuf *guest_dmabuf;
+    bool guest_dmabuf_refresh;
+    bool render_cursor;
+
+    egl_fb guest_fb;
+    egl_fb blit_fb;
+    egl_fb cursor_fb;
+    bool have_hot;
 #endif
 };
 
diff --git a/ui/spice-display.c b/ui/spice-display.c
index a494db1196..61edc9d1ea 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -941,25 +941,126 @@ static void qemu_spice_gl_scanout_dmabuf(DisplayChangeListener *dcl,
 {
     SimpleSpiceDisplay *ssd = container_of(dcl, SimpleSpiceDisplay, dcl);
 
-    /* note: spice server will close the fd, so hand over a dup */
-    spice_qxl_gl_scanout(&ssd->qxl, dup(dmabuf->fd),
-                         dmabuf->width, dmabuf->height,
-                         dmabuf->stride, dmabuf->fourcc, false);
-    qemu_spice_gl_monitor_config(ssd, 0, 0, dmabuf->width, dmabuf->height);
+    ssd->guest_dmabuf = dmabuf;
+    ssd->guest_dmabuf_refresh = true;
+
     ssd->have_surface = false;
     ssd->have_scanout = true;
 }
 
+static void qemu_spice_gl_cursor_dmabuf(DisplayChangeListener *dcl,
+                                        QemuDmaBuf *dmabuf, bool have_hot,
+                                        uint32_t hot_x, uint32_t hot_y)
+{
+    SimpleSpiceDisplay *ssd = container_of(dcl, SimpleSpiceDisplay, dcl);
+
+    ssd->have_hot = have_hot;
+    ssd->hot_x = hot_x;
+    ssd->hot_y = hot_y;
+
+    trace_qemu_spice_gl_cursor(ssd->qxl.id, dmabuf != NULL, have_hot);
+    if (dmabuf) {
+        egl_dmabuf_import_texture(dmabuf);
+        if (!dmabuf->texture) {
+            return;
+        }
+        egl_fb_setup_for_tex(&ssd->cursor_fb, dmabuf->width, dmabuf->height,
+                             dmabuf->texture, false);
+    } else {
+        egl_fb_destroy(&ssd->cursor_fb);
+    }
+}
+
+static void qemu_spice_gl_cursor_position(DisplayChangeListener *dcl,
+                                          uint32_t pos_x, uint32_t pos_y)
+{
+    SimpleSpiceDisplay *ssd = container_of(dcl, SimpleSpiceDisplay, dcl);
+
+    ssd->ptr_x = pos_x;
+    ssd->ptr_y = pos_y;
+}
+
+static void qemu_spice_gl_release_dmabuf(DisplayChangeListener *dcl,
+                                         QemuDmaBuf *dmabuf)
+{
+    SimpleSpiceDisplay *ssd = container_of(dcl, SimpleSpiceDisplay, dcl);
+
+    if (ssd->guest_dmabuf == dmabuf) {
+        ssd->guest_dmabuf = NULL;
+        ssd->guest_dmabuf_refresh = false;
+    }
+    egl_dmabuf_release_texture(dmabuf);
+}
+
 static void qemu_spice_gl_update(DisplayChangeListener *dcl,
                                  uint32_t x, uint32_t y, uint32_t w, uint32_t h)
 {
     SimpleSpiceDisplay *ssd = container_of(dcl, SimpleSpiceDisplay, dcl);
+    EGLint stride = 0, fourcc = 0;
+    bool render_cursor = false;
+    bool y_0_top = false; /* FIXME */
     uint64_t cookie;
+    int fd;
 
     if (!ssd->have_scanout) {
         return;
     }
 
+    if (ssd->cursor_fb.texture) {
+        render_cursor = true;
+    }
+    if (ssd->render_cursor != render_cursor) {
+        ssd->render_cursor = render_cursor;
+        ssd->guest_dmabuf_refresh = true;
+        egl_fb_destroy(&ssd->blit_fb);
+    }
+
+    if (ssd->guest_dmabuf_refresh) {
+        QemuDmaBuf *dmabuf = ssd->guest_dmabuf;
+        if (render_cursor) {
+            egl_dmabuf_import_texture(dmabuf);
+            if (!dmabuf->texture) {
+                return;
+            }
+
+            /* source framebuffer */
+            egl_fb_setup_for_tex(&ssd->guest_fb,
+                                 dmabuf->width, dmabuf->height,
+                                 dmabuf->texture, false);
+
+            /* dest framebuffer */
+            if (ssd->blit_fb.width  != dmabuf->width ||
+                ssd->blit_fb.height != dmabuf->height) {
+                trace_qemu_spice_gl_render_dmabuf(ssd->qxl.id, dmabuf->width,
+                                                  dmabuf->height);
+                egl_fb_destroy(&ssd->blit_fb);
+                egl_fb_setup_new_tex(&ssd->blit_fb,
+                                     dmabuf->width, dmabuf->height);
+                fd = egl_get_fd_for_texture(ssd->blit_fb.texture,
+                                            &stride, &fourcc);
+                spice_qxl_gl_scanout(&ssd->qxl, fd,
+                                     dmabuf->width, dmabuf->height,
+                                     stride, fourcc, false);
+            }
+        } else {
+            trace_qemu_spice_gl_forward_dmabuf(ssd->qxl.id,
+                                               dmabuf->width, dmabuf->height);
+            /* note: spice server will close the fd, so hand over a dup */
+            spice_qxl_gl_scanout(&ssd->qxl, dup(dmabuf->fd),
+                                 dmabuf->width, dmabuf->height,
+                                 dmabuf->stride, dmabuf->fourcc, false);
+        }
+        qemu_spice_gl_monitor_config(ssd, 0, 0, dmabuf->width, dmabuf->height);
+        ssd->guest_dmabuf_refresh = false;
+    }
+
+    if (render_cursor) {
+        egl_texture_blit(ssd->gls, &ssd->blit_fb, &ssd->guest_fb,
+                         !y_0_top);
+        egl_texture_blend(ssd->gls, &ssd->blit_fb, &ssd->cursor_fb,
+                          !y_0_top, ssd->ptr_x, ssd->ptr_y);
+        glFlush();
+    }
 
     trace_qemu_spice_gl_update(ssd->qxl.id, w, h, x, y);
     qemu_spice_gl_block(ssd, true);
@@ -984,6 +1085,9 @@ static const DisplayChangeListenerOps display_listener_gl_ops = {
     .dpy_gl_scanout_disable  = qemu_spice_gl_scanout_disable,
     .dpy_gl_scanout_texture  = qemu_spice_gl_scanout_texture,
     .dpy_gl_scanout_dmabuf   = qemu_spice_gl_scanout_dmabuf,
+    .dpy_gl_cursor_dmabuf    = qemu_spice_gl_cursor_dmabuf,
+    .dpy_gl_cursor_position  = qemu_spice_gl_cursor_position,
+    .dpy_gl_release_dmabuf   = qemu_spice_gl_release_dmabuf,
     .dpy_gl_update           = qemu_spice_gl_update,
 };
 
diff --git a/ui/trace-events b/ui/trace-events
index 518e950a01..a957f363f1 100644
--- a/ui/trace-events
+++ b/ui/trace-events
@@ -83,6 +83,9 @@ qemu_spice_ui_info(int qid, uint32_t width, uint32_t height) "%d %dx%d"
 qemu_spice_gl_surface(int qid, uint32_t w, uint32_t h, uint32_t fourcc) "%d %dx%d, fourcc 0x%x"
 qemu_spice_gl_scanout_disable(int qid) "%d"
 qemu_spice_gl_scanout_texture(int qid, uint32_t w, uint32_t h, uint32_t fourcc) "%d %dx%d, fourcc 0x%x"
+qemu_spice_gl_cursor(int qid, bool enabled, bool hotspot) "%d enabled %d, hotspot %d"
+qemu_spice_gl_forward_dmabuf(int qid, uint32_t width, uint32_t height) "%d %dx%d"
+qemu_spice_gl_render_dmabuf(int qid, uint32_t width, uint32_t height) "%d %dx%d"
 qemu_spice_gl_update(int qid, uint32_t x, uint32_t y, uint32_t w, uint32_t h) "%d +%d+%d %dx%d"
 
 # ui/keymaps.c
-- 
2.9.3

Re: [Qemu-devel] [PATCH v2 3/3] spice: add cursor_dmabuf support

[Marc-André Lureau]

Hi

On Tue, Mar 6, 2018 at 9:38 AM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Add support for cursor dmabufs.  qemu has to render the cursor for
> that, so in case a cursor is present qemu allocates a new dmabuf, blits
> the scanout, blends in the pointer and passes on the new dmabuf to
> spice-server.  Without cursor qemu continues to simply pass on the
> scanout dmabuf as-is.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

What about client side rendering mode? (for responsiveness etc)

Why not read the cursor data and pass it as regular cursor update? I
don't think such a small and infrequent CPU read will impact
performance much. Perhaps less than drawing the cursor ourself in qemu
on each frame (rather than using the client side
rendering/composition, it's hard to tell I guess).

> ---
>  include/ui/spice-display.h |   9 ++++
>  ui/spice-display.c         | 114 +++++++++++++++++++++++++++++++++++++++++++--
>  ui/trace-events            |   3 ++
>  3 files changed, 121 insertions(+), 5 deletions(-)
>
> diff --git a/include/ui/spice-display.h b/include/ui/spice-display.h
> index 6b5c73b21c..87a84a59d4 100644
> --- a/include/ui/spice-display.h
> +++ b/include/ui/spice-display.h
> @@ -122,6 +122,15 @@ struct SimpleSpiceDisplay {
>      int gl_updates;
>      bool have_scanout;
>      bool have_surface;
> +
> +    QemuDmaBuf *guest_dmabuf;
> +    bool guest_dmabuf_refresh;
> +    bool render_cursor;
> +
> +    egl_fb guest_fb;
> +    egl_fb blit_fb;
> +    egl_fb cursor_fb;
> +    bool have_hot;
>  #endif
>  };
>
> diff --git a/ui/spice-display.c b/ui/spice-display.c
> index a494db1196..61edc9d1ea 100644
> --- a/ui/spice-display.c
> +++ b/ui/spice-display.c
> @@ -941,25 +941,126 @@ static void qemu_spice_gl_scanout_dmabuf(DisplayChangeListener *dcl,
>  {
>      SimpleSpiceDisplay *ssd = container_of(dcl, SimpleSpiceDisplay, dcl);
>
> -    /* note: spice server will close the fd, so hand over a dup */
> -    spice_qxl_gl_scanout(&ssd->qxl, dup(dmabuf->fd),
> -                         dmabuf->width, dmabuf->height,
> -                         dmabuf->stride, dmabuf->fourcc, false);
> -    qemu_spice_gl_monitor_config(ssd, 0, 0, dmabuf->width, dmabuf->height);
> +    ssd->guest_dmabuf = dmabuf;
> +    ssd->guest_dmabuf_refresh = true;
> +
>      ssd->have_surface = false;
>      ssd->have_scanout = true;
>  }
>
> +static void qemu_spice_gl_cursor_dmabuf(DisplayChangeListener *dcl,
> +                                        QemuDmaBuf *dmabuf, bool have_hot,
> +                                        uint32_t hot_x, uint32_t hot_y)
> +{
> +    SimpleSpiceDisplay *ssd = container_of(dcl, SimpleSpiceDisplay, dcl);
> +
> +    ssd->have_hot = have_hot;
> +    ssd->hot_x = hot_x;
> +    ssd->hot_y = hot_y;
> +
> +    trace_qemu_spice_gl_cursor(ssd->qxl.id, dmabuf != NULL, have_hot);
> +    if (dmabuf) {
> +        egl_dmabuf_import_texture(dmabuf);
> +        if (!dmabuf->texture) {
> +            return;
> +        }
> +        egl_fb_setup_for_tex(&ssd->cursor_fb, dmabuf->width, dmabuf->height,
> +                             dmabuf->texture, false);
> +    } else {
> +        egl_fb_destroy(&ssd->cursor_fb);
> +    }
> +}
> +
> +static void qemu_spice_gl_cursor_position(DisplayChangeListener *dcl,
> +                                          uint32_t pos_x, uint32_t pos_y)
> +{
> +    SimpleSpiceDisplay *ssd = container_of(dcl, SimpleSpiceDisplay, dcl);
> +
> +    ssd->ptr_x = pos_x;
> +    ssd->ptr_y = pos_y;
> +}
> +
> +static void qemu_spice_gl_release_dmabuf(DisplayChangeListener *dcl,
> +                                         QemuDmaBuf *dmabuf)
> +{
> +    SimpleSpiceDisplay *ssd = container_of(dcl, SimpleSpiceDisplay, dcl);
> +
> +    if (ssd->guest_dmabuf == dmabuf) {
> +        ssd->guest_dmabuf = NULL;
> +        ssd->guest_dmabuf_refresh = false;
> +    }
> +    egl_dmabuf_release_texture(dmabuf);
> +}
> +
>  static void qemu_spice_gl_update(DisplayChangeListener *dcl,
>                                   uint32_t x, uint32_t y, uint32_t w, uint32_t h)
>  {
>      SimpleSpiceDisplay *ssd = container_of(dcl, SimpleSpiceDisplay, dcl);
> +    EGLint stride = 0, fourcc = 0;
> +    bool render_cursor = false;
> +    bool y_0_top = false; /* FIXME */
>      uint64_t cookie;
> +    int fd;
>
>      if (!ssd->have_scanout) {
>          return;
>      }
>
> +    if (ssd->cursor_fb.texture) {
> +        render_cursor = true;
> +    }
> +    if (ssd->render_cursor != render_cursor) {
> +        ssd->render_cursor = render_cursor;
> +        ssd->guest_dmabuf_refresh = true;
> +        egl_fb_destroy(&ssd->blit_fb);
> +    }
> +
> +    if (ssd->guest_dmabuf_refresh) {
> +        QemuDmaBuf *dmabuf = ssd->guest_dmabuf;
> +        if (render_cursor) {
> +            egl_dmabuf_import_texture(dmabuf);
> +            if (!dmabuf->texture) {
> +                return;
> +            }
> +
> +            /* source framebuffer */
> +            egl_fb_setup_for_tex(&ssd->guest_fb,
> +                                 dmabuf->width, dmabuf->height,
> +                                 dmabuf->texture, false);
> +
> +            /* dest framebuffer */
> +            if (ssd->blit_fb.width  != dmabuf->width ||
> +                ssd->blit_fb.height != dmabuf->height) {
> +                trace_qemu_spice_gl_render_dmabuf(ssd->qxl.id, dmabuf->width,
> +                                                  dmabuf->height);
> +                egl_fb_destroy(&ssd->blit_fb);
> +                egl_fb_setup_new_tex(&ssd->blit_fb,
> +                                     dmabuf->width, dmabuf->height);
> +                fd = egl_get_fd_for_texture(ssd->blit_fb.texture,
> +                                            &stride, &fourcc);
> +                spice_qxl_gl_scanout(&ssd->qxl, fd,
> +                                     dmabuf->width, dmabuf->height,
> +                                     stride, fourcc, false);
> +            }
> +        } else {
> +            trace_qemu_spice_gl_forward_dmabuf(ssd->qxl.id,
> +                                               dmabuf->width, dmabuf->height);
> +            /* note: spice server will close the fd, so hand over a dup */
> +            spice_qxl_gl_scanout(&ssd->qxl, dup(dmabuf->fd),
> +                                 dmabuf->width, dmabuf->height,
> +                                 dmabuf->stride, dmabuf->fourcc, false);
> +        }
> +        qemu_spice_gl_monitor_config(ssd, 0, 0, dmabuf->width, dmabuf->height);
> +        ssd->guest_dmabuf_refresh = false;
> +    }
> +
> +    if (render_cursor) {
> +        egl_texture_blit(ssd->gls, &ssd->blit_fb, &ssd->guest_fb,
> +                         !y_0_top);
> +        egl_texture_blend(ssd->gls, &ssd->blit_fb, &ssd->cursor_fb,
> +                          !y_0_top, ssd->ptr_x, ssd->ptr_y);
> +        glFlush();
> +    }
>
>      trace_qemu_spice_gl_update(ssd->qxl.id, w, h, x, y);
>      qemu_spice_gl_block(ssd, true);
> @@ -984,6 +1085,9 @@ static const DisplayChangeListenerOps display_listener_gl_ops = {
>      .dpy_gl_scanout_disable  = qemu_spice_gl_scanout_disable,
>      .dpy_gl_scanout_texture  = qemu_spice_gl_scanout_texture,
>      .dpy_gl_scanout_dmabuf   = qemu_spice_gl_scanout_dmabuf,
> +    .dpy_gl_cursor_dmabuf    = qemu_spice_gl_cursor_dmabuf,
> +    .dpy_gl_cursor_position  = qemu_spice_gl_cursor_position,
> +    .dpy_gl_release_dmabuf   = qemu_spice_gl_release_dmabuf,
>      .dpy_gl_update           = qemu_spice_gl_update,
>  };
>
> diff --git a/ui/trace-events b/ui/trace-events
> index 518e950a01..a957f363f1 100644
> --- a/ui/trace-events
> +++ b/ui/trace-events
> @@ -83,6 +83,9 @@ qemu_spice_ui_info(int qid, uint32_t width, uint32_t height) "%d %dx%d"
>  qemu_spice_gl_surface(int qid, uint32_t w, uint32_t h, uint32_t fourcc) "%d %dx%d, fourcc 0x%x"
>  qemu_spice_gl_scanout_disable(int qid) "%d"
>  qemu_spice_gl_scanout_texture(int qid, uint32_t w, uint32_t h, uint32_t fourcc) "%d %dx%d, fourcc 0x%x"
> +qemu_spice_gl_cursor(int qid, bool enabled, bool hotspot) "%d enabled %d, hotspot %d"
> +qemu_spice_gl_forward_dmabuf(int qid, uint32_t width, uint32_t height) "%d %dx%d"
> +qemu_spice_gl_render_dmabuf(int qid, uint32_t width, uint32_t height) "%d %dx%d"
>  qemu_spice_gl_update(int qid, uint32_t x, uint32_t y, uint32_t w, uint32_t h) "%d +%d+%d %dx%d"
>
>  # ui/keymaps.c
> --
> 2.9.3
>
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v2 3/3] spice: add cursor_dmabuf support

[Gerd Hoffmann]

On Tue, Mar 06, 2018 at 12:56:55PM +0100, Marc-André Lureau wrote:
> Hi
> 
> On Tue, Mar 6, 2018 at 9:38 AM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> > Add support for cursor dmabufs.  qemu has to render the cursor for
> > that, so in case a cursor is present qemu allocates a new dmabuf, blits
> > the scanout, blends in the pointer and passes on the new dmabuf to
> > spice-server.  Without cursor qemu continues to simply pass on the
> > scanout dmabuf as-is.
> >
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> 
> What about client side rendering mode? (for responsiveness etc)

Note that client side cursor rending needs guest support.  Intel added
some extra paravirtual registers for the hotspot location (on physical
hardware this isn't needed).  So client-side cursor rendering will only
work for guests which have support for those extra registers, and we
will need the current rendering code *anyway* in case we don't get the
hotspot location from the guest.  So I figured I go with this code base
for now, put it into shape and run for 2.12 merge.

> Why not read the cursor data and pass it as regular cursor update? I
> don't think such a small and infrequent CPU read will impact
> performance much.

Well, it's not *that* infrequent.  Cursor shape might have changed even
if the dmabuf is still the same (i.e. guest didn't place the cursor
elsewhere in memory).  Also note that Intel uses a 256x256 cursor sprite
(with most of it being unused), so it isn't that small either.

But, yes, I plan to check that out and try to send the cursor as regular
spice cursor update.  Will probably not make 2.12 though, too much
higher priority stuff in the pipeline ...

cheers,
  Gerd