[PATCH v3 0/5] virtio-gpu: Add a default synchronization mechanism for blobs

[PATCH v3 0/5] virtio-gpu: Add a default synchronization mechanism for blobs

[Vivek Kasireddy]

When the Guest and Host are using Blob resources, there is a chance
that they may use the underlying storage associated with a Blob at
the same time leading to glitches such as flickering or tearing.
To prevent these from happening, the Host needs to ensure that it
waits until its Blit is completed by the Host GPU before letting
the Guest reuse the Blob.

This should be the default behavior regardless of the type of Guest
that is using Blob resources but would be particularly useful for 
Guests that are using frontbuffer rendering such as Linux with X
or Windows 10, etc.

The way it works is the Guest includes a fence as part of 
resource_flush and waits for it to be signalled. The Host will
queue a repaint request and signal the fence after it completes
waiting on the sync object associated with the Blit.

v2:
- Added more description in the cover letter
- Removed the wait from resource_flush and included it in
  a gl_flushed() callback

v3:
- Instead of explicitly waiting on the sync object and stalling the
  thread, add the relevant fence fd to Qemu's main loop and wait
  for it to be signalled. (suggested by Gerd Hoffmann)

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Dongwon Kim <dongwon.kim@intel.com>
Cc: Tina Zhang <tina.zhang@intel.com>

Vivek Kasireddy (5):
  ui/gtk: Create a common release_dmabuf helper
  ui/egl: Add egl helpers to help with synchronization
  ui: Create sync objects and fences only for blobs
  ui/gtk-egl: Wait for the draw signal for dmabuf blobs
  virtio-gpu: Add gl_flushed callback

 hw/display/virtio-gpu-udmabuf.c |  1 +
 hw/display/virtio-gpu.c         | 32 ++++++++++++++++++++++--
 include/ui/console.h            |  3 +++
 include/ui/egl-helpers.h        |  3 +++
 include/ui/gtk.h                |  5 ++--
 ui/egl-helpers.c                | 26 ++++++++++++++++++++
 ui/gtk-egl.c                    | 43 +++++++++++++++++++++++++++------
 ui/gtk-gl-area.c                | 20 +++++++++++++++
 ui/gtk.c                        | 26 ++++++++++++++++++--
 9 files changed, 145 insertions(+), 14 deletions(-)

-- 
2.30.2

[PATCH v3 1/5] ui/gtk: Create a common release_dmabuf helper

[Vivek Kasireddy]

Since the texture release mechanism is same for both gtk-egl
and gtk-glarea, move the helper from gtk-egl to common gtk
code so that it can be shared by both gtk backends.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 include/ui/gtk.h |  2 --
 ui/gtk-egl.c     |  8 --------
 ui/gtk.c         | 11 ++++++++++-
 3 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/include/ui/gtk.h b/include/ui/gtk.h
index 9516670ebc..e6cbf0507c 100644
--- a/include/ui/gtk.h
+++ b/include/ui/gtk.h
@@ -178,8 +178,6 @@ void gd_egl_cursor_dmabuf(DisplayChangeListener *dcl,
                           uint32_t hot_x, uint32_t hot_y);
 void gd_egl_cursor_position(DisplayChangeListener *dcl,
                             uint32_t pos_x, uint32_t pos_y);
-void gd_egl_release_dmabuf(DisplayChangeListener *dcl,
-                           QemuDmaBuf *dmabuf);
 void gd_egl_scanout_flush(DisplayChangeListener *dcl,
                           uint32_t x, uint32_t y, uint32_t w, uint32_t h);
 void gtk_egl_init(DisplayGLMode mode);
diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
index 2a2e6d3a17..b671181272 100644
--- a/ui/gtk-egl.c
+++ b/ui/gtk-egl.c
@@ -249,14 +249,6 @@ void gd_egl_cursor_position(DisplayChangeListener *dcl,
     vc->gfx.cursor_y = pos_y * vc->gfx.scale_y;
 }
 
-void gd_egl_release_dmabuf(DisplayChangeListener *dcl,
-                           QemuDmaBuf *dmabuf)
-{
-#ifdef CONFIG_GBM
-    egl_dmabuf_release_texture(dmabuf);
-#endif
-}
-
 void gd_egl_scanout_flush(DisplayChangeListener *dcl,
                           uint32_t x, uint32_t y, uint32_t w, uint32_t h)
 {
diff --git a/ui/gtk.c b/ui/gtk.c
index 98046f577b..6132bab52f 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -575,6 +575,14 @@ static bool gd_has_dmabuf(DisplayChangeListener *dcl)
     return vc->gfx.has_dmabuf;
 }
 
+static void gd_gl_release_dmabuf(DisplayChangeListener *dcl,
+                                 QemuDmaBuf *dmabuf)
+{
+#ifdef CONFIG_GBM
+    egl_dmabuf_release_texture(dmabuf);
+#endif
+}
+
 /** DisplayState Callbacks (opengl version) **/
 
 static const DisplayChangeListenerOps dcl_gl_area_ops = {
@@ -593,6 +601,7 @@ static const DisplayChangeListenerOps dcl_gl_area_ops = {
     .dpy_gl_scanout_disable  = gd_gl_area_scanout_disable,
     .dpy_gl_update           = gd_gl_area_scanout_flush,
     .dpy_gl_scanout_dmabuf   = gd_gl_area_scanout_dmabuf,
+    .dpy_gl_release_dmabuf   = gd_gl_release_dmabuf,
     .dpy_has_dmabuf          = gd_has_dmabuf,
 };
 
@@ -615,8 +624,8 @@ static const DisplayChangeListenerOps dcl_egl_ops = {
     .dpy_gl_scanout_dmabuf   = gd_egl_scanout_dmabuf,
     .dpy_gl_cursor_dmabuf    = gd_egl_cursor_dmabuf,
     .dpy_gl_cursor_position  = gd_egl_cursor_position,
-    .dpy_gl_release_dmabuf   = gd_egl_release_dmabuf,
     .dpy_gl_update           = gd_egl_scanout_flush,
+    .dpy_gl_release_dmabuf   = gd_gl_release_dmabuf,
     .dpy_has_dmabuf          = gd_has_dmabuf,
 };
 
-- 
2.30.2

[PATCH v3 2/5] ui/egl: Add egl helpers to help with synchronization

[Vivek Kasireddy]

These egl helpers would be used for creating and waiting on
a sync object.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 include/ui/console.h     |  2 ++
 include/ui/egl-helpers.h |  2 ++
 ui/egl-helpers.c         | 26 ++++++++++++++++++++++++++
 3 files changed, 30 insertions(+)

diff --git a/include/ui/console.h b/include/ui/console.h
index b30b63976a..49978fdae3 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -168,6 +168,8 @@ typedef struct QemuDmaBuf {
     uint64_t  modifier;
     uint32_t  texture;
     bool      y0_top;
+    void      *sync;
+    int       fence_fd;
 } QemuDmaBuf;
 
 typedef struct DisplayState DisplayState;
diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h
index f1bf8f97fc..2c3ba92b53 100644
--- a/include/ui/egl-helpers.h
+++ b/include/ui/egl-helpers.h
@@ -45,6 +45,8 @@ int egl_get_fd_for_texture(uint32_t tex_id, EGLint *stride, EGLint *fourcc,
 
 void egl_dmabuf_import_texture(QemuDmaBuf *dmabuf);
 void egl_dmabuf_release_texture(QemuDmaBuf *dmabuf);
+void egl_dmabuf_create_sync(QemuDmaBuf *dmabuf);
+void egl_dmabuf_create_fence(QemuDmaBuf *dmabuf);
 
 #endif
 
diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
index 6d0cb2b5cb..6fa3f2ec45 100644
--- a/ui/egl-helpers.c
+++ b/ui/egl-helpers.c
@@ -76,6 +76,32 @@ void egl_fb_setup_for_tex(egl_fb *fb, int width, int height,
                               GL_TEXTURE_2D, fb->texture, 0);
 }
 
+void egl_dmabuf_create_sync(QemuDmaBuf *dmabuf)
+{
+    EGLSyncKHR sync;
+
+    if (epoxy_has_egl_extension(qemu_egl_display,
+                                "EGL_KHR_fence_sync") &&
+        epoxy_has_egl_extension(qemu_egl_display,
+                                "EGL_ANDROID_native_fence_sync")) {
+        sync = eglCreateSyncKHR(qemu_egl_display,
+				EGL_SYNC_NATIVE_FENCE_ANDROID, NULL);
+        if (sync != EGL_NO_SYNC_KHR) {
+            dmabuf->sync = sync;
+        }
+    }
+}
+
+void egl_dmabuf_create_fence(QemuDmaBuf *dmabuf)
+{
+    if (dmabuf->sync) {
+        dmabuf->fence_fd = eglDupNativeFenceFDANDROID(qemu_egl_display,
+                                                      dmabuf->sync);
+        eglDestroySyncKHR(qemu_egl_display, dmabuf->sync);
+        dmabuf->sync = NULL;
+    }
+}
+
 void egl_fb_setup_new_tex(egl_fb *fb, int width, int height)
 {
     GLuint texture;
-- 
2.30.2

[PATCH v3 3/5] ui: Create sync objects and fences only for blobs

[Vivek Kasireddy]

Create sync objects and fences only for dmabufs that are blobs. Once a
fence is created (after glFlush) and is signalled,
graphic_hw_gl_flushed() will be called and virtio-gpu cmd processing
will be resumed.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 hw/display/virtio-gpu-udmabuf.c |  1 +
 include/ui/console.h            |  1 +
 include/ui/egl-helpers.h        |  1 +
 include/ui/gtk.h                |  1 +
 ui/gtk-egl.c                    | 20 ++++++++++++++++++++
 ui/gtk-gl-area.c                | 20 ++++++++++++++++++++
 ui/gtk.c                        | 13 +++++++++++++
 7 files changed, 57 insertions(+)

diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
index 3c01a415e7..632ba06cbc 100644
--- a/hw/display/virtio-gpu-udmabuf.c
+++ b/hw/display/virtio-gpu-udmabuf.c
@@ -185,6 +185,7 @@ static VGPUDMABuf
     dmabuf->buf.stride = fb->stride;
     dmabuf->buf.fourcc = qemu_pixman_to_drm_format(fb->format);
     dmabuf->buf.fd = res->dmabuf_fd;
+    dmabuf->buf.blob = true;
 
     dmabuf->scanout_id = scanout_id;
     QTAILQ_INSERT_HEAD(&g->dmabuf.bufs, dmabuf, next);
diff --git a/include/ui/console.h b/include/ui/console.h
index 49978fdae3..570d827644 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -170,6 +170,7 @@ typedef struct QemuDmaBuf {
     bool      y0_top;
     void      *sync;
     int       fence_fd;
+    bool      blob;
 } QemuDmaBuf;
 
 typedef struct DisplayState DisplayState;
diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h
index 2c3ba92b53..2fb6e0dd6b 100644
--- a/include/ui/egl-helpers.h
+++ b/include/ui/egl-helpers.h
@@ -19,6 +19,7 @@ typedef struct egl_fb {
     GLuint texture;
     GLuint framebuffer;
     bool delete_texture;
+    QemuDmaBuf *dmabuf;
 } egl_fb;
 
 void egl_fb_destroy(egl_fb *fb);
diff --git a/include/ui/gtk.h b/include/ui/gtk.h
index e6cbf0507c..3e6a48b978 100644
--- a/include/ui/gtk.h
+++ b/include/ui/gtk.h
@@ -152,6 +152,7 @@ extern bool gtk_use_gl_area;
 /* ui/gtk.c */
 void gd_update_windowsize(VirtualConsole *vc);
 int gd_monitor_update_interval(GtkWidget *widget);
+void gd_hw_gl_flushed(void *vc);
 
 /* ui/gtk-egl.c */
 void gd_egl_init(VirtualConsole *vc);
diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
index b671181272..f530bcd940 100644
--- a/ui/gtk-egl.c
+++ b/ui/gtk-egl.c
@@ -12,6 +12,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/main-loop.h"
 
 #include "trace.h"
 
@@ -63,6 +64,7 @@ void gd_egl_draw(VirtualConsole *vc)
 {
     GdkWindow *window;
     int ww, wh;
+    QemuDmaBuf *dmabuf = vc->gfx.guest_fb.dmabuf;
 
     if (!vc->gfx.gls) {
         return;
@@ -94,6 +96,14 @@ void gd_egl_draw(VirtualConsole *vc)
     }
 
     glFlush();
+    if (dmabuf) {
+        egl_dmabuf_create_fence(dmabuf);
+        if (dmabuf->fence_fd > 0) {
+            qemu_set_fd_handler(dmabuf->fence_fd, gd_hw_gl_flushed, NULL, vc);
+            return;
+        }
+        graphic_hw_gl_block(vc->gfx.dcl.con, false);
+    }
     graphic_hw_gl_flushed(vc->gfx.dcl.con);
 }
 
@@ -209,6 +219,8 @@ void gd_egl_scanout_dmabuf(DisplayChangeListener *dcl,
                            QemuDmaBuf *dmabuf)
 {
 #ifdef CONFIG_GBM
+    VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
+
     egl_dmabuf_import_texture(dmabuf);
     if (!dmabuf->texture) {
         return;
@@ -217,6 +229,10 @@ void gd_egl_scanout_dmabuf(DisplayChangeListener *dcl,
     gd_egl_scanout_texture(dcl, dmabuf->texture,
                            false, dmabuf->width, dmabuf->height,
                            0, 0, dmabuf->width, dmabuf->height);
+
+    if (dmabuf->blob) {
+        vc->gfx.guest_fb.dmabuf = dmabuf;
+    }
 #endif
 }
 
@@ -281,6 +297,10 @@ void gd_egl_scanout_flush(DisplayChangeListener *dcl,
         egl_fb_blit(&vc->gfx.win_fb, &vc->gfx.guest_fb, !vc->gfx.y0_top);
     }
 
+    if (vc->gfx.guest_fb.dmabuf) {
+        egl_dmabuf_create_sync(vc->gfx.guest_fb.dmabuf);
+    }
+
     eglSwapBuffers(qemu_egl_display, vc->gfx.esurface);
 }
 
diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
index dd5783fec7..091194789e 100644
--- a/ui/gtk-gl-area.c
+++ b/ui/gtk-gl-area.c
@@ -8,6 +8,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/main-loop.h"
 
 #include "trace.h"
 
@@ -38,6 +39,7 @@ static void gtk_gl_area_set_scanout_mode(VirtualConsole *vc, bool scanout)
 void gd_gl_area_draw(VirtualConsole *vc)
 {
     int ww, wh, y1, y2;
+    QemuDmaBuf *dmabuf = vc->gfx.guest_fb.dmabuf;
 
     if (!vc->gfx.gls) {
         return;
@@ -71,7 +73,18 @@ void gd_gl_area_draw(VirtualConsole *vc)
         surface_gl_render_texture(vc->gfx.gls, vc->gfx.ds);
     }
 
+    if (dmabuf) {
+        egl_dmabuf_create_sync(dmabuf);
+    }
     glFlush();
+    if (dmabuf) {
+        egl_dmabuf_create_fence(dmabuf);
+        if (dmabuf->fence_fd > 0) {
+            qemu_set_fd_handler(dmabuf->fence_fd, gd_hw_gl_flushed, NULL, vc);
+            return;
+        }
+        graphic_hw_gl_block(vc->gfx.dcl.con, false);
+    }
     graphic_hw_gl_flushed(vc->gfx.dcl.con);
 }
 
@@ -213,6 +226,9 @@ void gd_gl_area_scanout_flush(DisplayChangeListener *dcl,
 {
     VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
 
+    if (vc->gfx.guest_fb.dmabuf) {
+        graphic_hw_gl_block(vc->gfx.dcl.con, true);
+    }
     gtk_gl_area_queue_render(GTK_GL_AREA(vc->gfx.drawing_area));
 }
 
@@ -231,6 +247,10 @@ void gd_gl_area_scanout_dmabuf(DisplayChangeListener *dcl,
     gd_gl_area_scanout_texture(dcl, dmabuf->texture,
                                false, dmabuf->width, dmabuf->height,
                                0, 0, dmabuf->width, dmabuf->height);
+
+    if (dmabuf->blob) {
+        vc->gfx.guest_fb.dmabuf = dmabuf;
+    }
 #endif
 }
 
diff --git a/ui/gtk.c b/ui/gtk.c
index 6132bab52f..ee3a084c21 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -36,6 +36,7 @@
 #include "qapi/qapi-commands-machine.h"
 #include "qapi/qapi-commands-misc.h"
 #include "qemu/cutils.h"
+#include "qemu/main-loop.h"
 
 #include "ui/console.h"
 #include "ui/gtk.h"
@@ -583,6 +584,18 @@ static void gd_gl_release_dmabuf(DisplayChangeListener *dcl,
 #endif
 }
 
+void gd_hw_gl_flushed(void *vcon)
+{
+    VirtualConsole *vc = vcon;
+    QemuDmaBuf *dmabuf = vc->gfx.guest_fb.dmabuf;
+
+    graphic_hw_gl_block(vc->gfx.dcl.con, false);
+    graphic_hw_gl_flushed(vc->gfx.dcl.con);
+    qemu_set_fd_handler(dmabuf->fence_fd, NULL, NULL, NULL);
+    close(dmabuf->fence_fd);
+    dmabuf->fence_fd = -1;
+}
+
 /** DisplayState Callbacks (opengl version) **/
 
 static const DisplayChangeListenerOps dcl_gl_area_ops = {
-- 
2.30.2

[PATCH v3 4/5] ui/gtk-egl: Wait for the draw signal for dmabuf blobs

[Vivek Kasireddy]

Instead of immediately drawing and submitting, queue and wait
for the draw signal if the dmabuf submitted is a blob.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 include/ui/gtk.h |  2 ++
 ui/gtk-egl.c     | 15 +++++++++++++++
 ui/gtk.c         |  2 +-
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/include/ui/gtk.h b/include/ui/gtk.h
index 3e6a48b978..a25f5bfecc 100644
--- a/include/ui/gtk.h
+++ b/include/ui/gtk.h
@@ -179,6 +179,8 @@ void gd_egl_cursor_dmabuf(DisplayChangeListener *dcl,
                           uint32_t hot_x, uint32_t hot_y);
 void gd_egl_cursor_position(DisplayChangeListener *dcl,
                             uint32_t pos_x, uint32_t pos_y);
+void gd_egl_flush(DisplayChangeListener *dcl,
+                  uint32_t x, uint32_t y, uint32_t w, uint32_t h);
 void gd_egl_scanout_flush(DisplayChangeListener *dcl,
                           uint32_t x, uint32_t y, uint32_t w, uint32_t h);
 void gtk_egl_init(DisplayGLMode mode);
diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
index f530bcd940..3c7eb244b4 100644
--- a/ui/gtk-egl.c
+++ b/ui/gtk-egl.c
@@ -304,6 +304,21 @@ void gd_egl_scanout_flush(DisplayChangeListener *dcl,
     eglSwapBuffers(qemu_egl_display, vc->gfx.esurface);
 }
 
+void gd_egl_flush(DisplayChangeListener *dcl,
+                  uint32_t x, uint32_t y, uint32_t w, uint32_t h)
+{
+    VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
+    GtkWidget *area = vc->gfx.drawing_area;
+
+    if (vc->gfx.guest_fb.dmabuf) {
+        graphic_hw_gl_block(vc->gfx.dcl.con, true);
+        gtk_widget_queue_draw_area(area, x, y, w, h);
+        return;
+    }
+
+    gd_egl_scanout_flush(&vc->gfx.dcl, x, y, w, h);
+}
+
 void gtk_egl_init(DisplayGLMode mode)
 {
     GdkDisplay *gdk_display = gdk_display_get_default();
diff --git a/ui/gtk.c b/ui/gtk.c
index ee3a084c21..ce885d2ca3 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -637,7 +637,7 @@ static const DisplayChangeListenerOps dcl_egl_ops = {
     .dpy_gl_scanout_dmabuf   = gd_egl_scanout_dmabuf,
     .dpy_gl_cursor_dmabuf    = gd_egl_cursor_dmabuf,
     .dpy_gl_cursor_position  = gd_egl_cursor_position,
-    .dpy_gl_update           = gd_egl_scanout_flush,
+    .dpy_gl_update           = gd_egl_flush,
     .dpy_gl_release_dmabuf   = gd_gl_release_dmabuf,
     .dpy_has_dmabuf          = gd_has_dmabuf,
 };
-- 
2.30.2

[PATCH v3 5/5] virtio-gpu: Add gl_flushed callback

[Vivek Kasireddy]

Adding this callback provides a way to resume the processing of
cmds in fenceq and cmdq that were not processed because the UI
was waiting on a fence and blocked cmd processing.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 hw/display/virtio-gpu.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 4d549377cb..ec9d621e64 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -983,8 +983,10 @@ void virtio_gpu_simple_process_cmd(VirtIOGPU *g,
         break;
     }
     if (!cmd->finished) {
-        virtio_gpu_ctrl_response_nodata(g, cmd, cmd->error ? cmd->error :
-                                        VIRTIO_GPU_RESP_OK_NODATA);
+        if (!g->parent_obj.renderer_blocked) {
+            virtio_gpu_ctrl_response_nodata(g, cmd, cmd->error ? cmd->error :
+                                            VIRTIO_GPU_RESP_OK_NODATA);
+        }
     }
 }
 
@@ -1040,6 +1042,30 @@ void virtio_gpu_process_cmdq(VirtIOGPU *g)
     g->processing_cmdq = false;
 }
 
+static void virtio_gpu_process_fenceq(VirtIOGPU *g)
+{
+    struct virtio_gpu_ctrl_command *cmd, *tmp;
+
+    QTAILQ_FOREACH_SAFE(cmd, &g->fenceq, next, tmp) {
+        trace_virtio_gpu_fence_resp(cmd->cmd_hdr.fence_id);
+        virtio_gpu_ctrl_response_nodata(g, cmd, VIRTIO_GPU_RESP_OK_NODATA);
+        QTAILQ_REMOVE(&g->fenceq, cmd, next);
+        g_free(cmd);
+        g->inflight--;
+        if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
+            fprintf(stderr, "inflight: %3d (-)\r", g->inflight);
+        }
+    }
+}
+
+static void virtio_gpu_handle_gl_flushed(VirtIOGPUBase *b)
+{
+    VirtIOGPU *g = container_of(b, VirtIOGPU, parent_obj);
+
+    virtio_gpu_process_fenceq(g);
+    virtio_gpu_process_cmdq(g);
+}
+
 static void virtio_gpu_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
 {
     VirtIOGPU *g = VIRTIO_GPU(vdev);
@@ -1398,10 +1424,12 @@ static void virtio_gpu_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
     VirtIOGPUClass *vgc = VIRTIO_GPU_CLASS(klass);
+    VirtIOGPUBaseClass *vgbc = &vgc->parent;
 
     vgc->handle_ctrl = virtio_gpu_handle_ctrl;
     vgc->process_cmd = virtio_gpu_simple_process_cmd;
     vgc->update_cursor_data = virtio_gpu_update_cursor_data;
+    vgbc->gl_flushed = virtio_gpu_handle_gl_flushed;
 
     vdc->realize = virtio_gpu_device_realize;
     vdc->reset = virtio_gpu_reset;
-- 
2.30.2

Re: [PATCH v3 0/5] virtio-gpu: Add a default synchronization mechanism for blobs

[no-reply]

Patchew URL: https://patchew.org/QEMU/20210621192425.1188442-1-vivek.kasireddy@intel.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20210621192425.1188442-1-vivek.kasireddy@intel.com
Subject: [PATCH v3 0/5] virtio-gpu: Add a default synchronization mechanism for blobs

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
   53f306f..0add99e  master     -> master
 - [tag update]      patchew/20210621171021.1819881-1-richard.henderson@linaro.org -> patchew/20210621171021.1819881-1-richard.henderson@linaro.org
 * [new tag]         patchew/20210621192425.1188442-1-vivek.kasireddy@intel.com -> patchew/20210621192425.1188442-1-vivek.kasireddy@intel.com
Switched to a new branch 'test'
33105de virtio-gpu: Add gl_flushed callback
c253f9f ui/gtk-egl: Wait for the draw signal for dmabuf blobs
5909df4 ui: Create sync objects and fences only for blobs
4f7d4b6 ui/egl: Add egl helpers to help with synchronization
a153fed ui/gtk: Create a common release_dmabuf helper

=== OUTPUT BEGIN ===
1/5 Checking commit a153fed2c4e2 (ui/gtk: Create a common release_dmabuf helper)
2/5 Checking commit 4f7d4b6596ae (ui/egl: Add egl helpers to help with synchronization)
ERROR: code indent should never use tabs
#62: FILE: ui/egl-helpers.c:88:
+^I^I^I^IEGL_SYNC_NATIVE_FENCE_ANDROID, NULL);$

total: 1 errors, 0 warnings, 48 lines checked

Patch 2/5 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

3/5 Checking commit 5909df40fbc8 (ui: Create sync objects and fences only for blobs)
4/5 Checking commit c253f9f281ae (ui/gtk-egl: Wait for the draw signal for dmabuf blobs)
5/5 Checking commit 33105de28a52 (virtio-gpu: Add gl_flushed callback)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210621192425.1188442-1-vivek.kasireddy@intel.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH v3 3/5] ui: Create sync objects and fences only for blobs

[Gerd Hoffmann]

  Hi,

>      dmabuf->buf.fd = res->dmabuf_fd;
> +    dmabuf->buf.blob = true;

Do you actually need the 'blob' field?
I think checking 'fd' instead should work too.

take care,
  Gerd

Re: [PATCH v3 2/5] ui/egl: Add egl helpers to help with synchronization

[Gerd Hoffmann]

On Mon, Jun 21, 2021 at 12:24:22PM -0700, Vivek Kasireddy wrote:
> These egl helpers would be used for creating and waiting on
> a sync object.
> 
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>

Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>

Re: [PATCH v3 4/5] ui/gtk-egl: Wait for the draw signal for dmabuf blobs

[Gerd Hoffmann]

On Mon, Jun 21, 2021 at 12:24:24PM -0700, Vivek Kasireddy wrote:
> Instead of immediately drawing and submitting, queue and wait
> for the draw signal if the dmabuf submitted is a blob.
> 
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>

Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>

Re: [PATCH v3 5/5] virtio-gpu: Add gl_flushed callback

[Gerd Hoffmann]

On Mon, Jun 21, 2021 at 12:24:25PM -0700, Vivek Kasireddy wrote:
> Adding this callback provides a way to resume the processing of
> cmds in fenceq and cmdq that were not processed because the UI
> was waiting on a fence and blocked cmd processing.
> 
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>

Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>

RE: [PATCH v3 3/5] ui: Create sync objects and fences only for blobs

[Kasireddy, Vivek]

Hi Gerd,

>   Hi,
> 
> >      dmabuf->buf.fd = res->dmabuf_fd;
> > +    dmabuf->buf.blob = true;
> 
> Do you actually need the 'blob' field?
> I think checking 'fd' instead should work too.
[Kasireddy, Vivek] I want these changes to be limited to blob resources only as I do not
know how they might affect other use-cases or whether they are needed there or not. I
don't think I can rely on fd as vfio/display.c also populates the fd field:
    dmabuf = g_new0(VFIODMABuf, 1);
    dmabuf->dmabuf_id  = plane.dmabuf_id;
    dmabuf->buf.width  = plane.width;
    dmabuf->buf.height = plane.height;
    dmabuf->buf.stride = plane.stride;
    dmabuf->buf.fourcc = plane.drm_format;
    dmabuf->buf.modifier = plane.drm_format_mod;
    dmabuf->buf.fd     = fd;

Therefore, I need a way to identify a dmabuf that is associated with blobs vs others.

Thanks,
Vivek

Re: [PATCH v3 3/5] ui: Create sync objects and fences only for blobs

[Gerd Hoffmann]

> >   Hi,
> > 
> > >      dmabuf->buf.fd = res->dmabuf_fd;
> > > +    dmabuf->buf.blob = true;
> > 
> > Do you actually need the 'blob' field?
> > I think checking 'fd' instead should work too.
> [Kasireddy, Vivek] I want these changes to be limited to blob resources only as I do not
> know how they might affect other use-cases or whether they are needed there or not. I
> don't think I can rely on fd as vfio/display.c also populates the fd field:
>     dmabuf = g_new0(VFIODMABuf, 1);
>     dmabuf->dmabuf_id  = plane.dmabuf_id;
>     dmabuf->buf.width  = plane.width;
>     dmabuf->buf.height = plane.height;
>     dmabuf->buf.stride = plane.stride;
>     dmabuf->buf.fourcc = plane.drm_format;
>     dmabuf->buf.modifier = plane.drm_format_mod;
>     dmabuf->buf.fd     = fd;
> 
> Therefore, I need a way to identify a dmabuf that is associated with blobs vs others.

And it actually is a dma-buf too (the guest display provided by i915 gvt
mdev driver).  So fencing that should work, right?

Even if we have to restrict it to some kinds of dma-bufs the field
should have a more descriptive name like "allow_fences".

take care,
  Gerd

RE: [PATCH v3 3/5] ui: Create sync objects and fences only for blobs

[Kasireddy, Vivek]

Hi Gerd,

> > >
> > > >      dmabuf->buf.fd = res->dmabuf_fd;
> > > > +    dmabuf->buf.blob = true;
> > >
> > > Do you actually need the 'blob' field?
> > > I think checking 'fd' instead should work too.
> > [Kasireddy, Vivek] I want these changes to be limited to blob resources only as I do not
> > know how they might affect other use-cases or whether they are needed there or not. I
> > don't think I can rely on fd as vfio/display.c also populates the fd field:
> >     dmabuf = g_new0(VFIODMABuf, 1);
> >     dmabuf->dmabuf_id  = plane.dmabuf_id;
> >     dmabuf->buf.width  = plane.width;
> >     dmabuf->buf.height = plane.height;
> >     dmabuf->buf.stride = plane.stride;
> >     dmabuf->buf.fourcc = plane.drm_format;
> >     dmabuf->buf.modifier = plane.drm_format_mod;
> >     dmabuf->buf.fd     = fd;
> >
> > Therefore, I need a way to identify a dmabuf that is associated with blobs vs others.
> 
> And it actually is a dma-buf too (the guest display provided by i915 gvt
> mdev driver).  So fencing that should work, right?
[Kasireddy, Vivek] Well, for virtio-gpu, as you know we are adding a dma fence to
resource_flush to make it wait until it gets signalled by Qemu. We might have to do
to something similar on i915 GVT side but I do not have the hardware to write a patch
and test it out -- as i915 GVT is not supported for > Gen 9 platforms.

> 
> Even if we have to restrict it to some kinds of dma-bufs the field
> should have a more descriptive name like "allow_fences".
[Kasireddy, Vivek] I think limiting this to blobs makes sense at the moment. If need be,
we can extend it to include dma-bufs generated by i915 GVT later. Let me send a
v4 with your suggestion to change the name.

Thanks,
Vivek
> 
> take care,
>   Gerd