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

[PATCH v2 0/8] 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

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

Vivek Kasireddy (8):
  ui/gtk: Create a common release_dmabuf helper
  ui/egl: Add egl helpers to help with synchronization
  ui: Add a helper to wait on a dmabuf sync object
  ui/gtk: Implement wait_dmabuf function
  ui: Create sync objects only for blobs
  ui/gtk-egl: Wait for the draw signal for dmabuf blobs
  virtio-gpu: Add dmabuf helpers for synchronization
  virtio-gpu: Add gl_flushed callback

 hw/display/virtio-gpu-udmabuf.c | 30 ++++++++++++++++++++++
 hw/display/virtio-gpu.c         | 44 ++++++++++++++++++++++++++++++++-
 include/hw/virtio/virtio-gpu.h  |  2 ++
 include/ui/console.h            |  8 ++++++
 include/ui/egl-helpers.h        |  4 +++
 include/ui/gtk.h                |  4 +--
 stubs/virtio-gpu-udmabuf.c      |  6 +++++
 ui/console.c                    | 10 ++++++++
 ui/egl-helpers.c                | 44 +++++++++++++++++++++++++++++++++
 ui/gtk-egl.c                    | 32 ++++++++++++++++++------
 ui/gtk-gl-area.c                |  8 ++++++
 ui/gtk.c                        | 28 +++++++++++++++++++--
 12 files changed, 207 insertions(+), 13 deletions(-)

-- 
2.30.2

[PATCH v2 1/8] 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>
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 v2 2/8] 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 |  3 +++
 ui/egl-helpers.c         | 44 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 49 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..5a7575dc13 100644
--- a/include/ui/egl-helpers.h
+++ b/include/ui/egl-helpers.h
@@ -45,6 +45,9 @@ 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);
+void egl_dmabuf_wait_sync(QemuDmaBuf *dmabuf);
 
 #endif
 
diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
index 6d0cb2b5cb..47220b66e0 100644
--- a/ui/egl-helpers.c
+++ b/ui/egl-helpers.c
@@ -76,6 +76,50 @@ 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_dmabuf_wait_sync(QemuDmaBuf *dmabuf)
+{
+    EGLSyncKHR sync;
+    EGLint attrib_list[] = {
+        EGL_SYNC_NATIVE_FENCE_FD_ANDROID, dmabuf->fence_fd,
+        EGL_NONE,
+    };
+
+    sync = eglCreateSyncKHR(qemu_egl_display,
+                            EGL_SYNC_NATIVE_FENCE_ANDROID, attrib_list);
+    if (sync != EGL_NO_SYNC_KHR) {
+        eglClientWaitSyncKHR(qemu_egl_display, sync,
+                             0, EGL_FOREVER_KHR);
+        eglDestroySyncKHR(qemu_egl_display, sync);
+        dmabuf->fence_fd = -1;
+    }
+}
+
 void egl_fb_setup_new_tex(egl_fb *fb, int width, int height)
 {
     GLuint texture;
-- 
2.30.2

Re: [PATCH v2 2/8] ui/egl: Add egl helpers to help with synchronization

[Gerd Hoffmann]

  Hi,

> +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_dmabuf_wait_sync(QemuDmaBuf *dmabuf)
> +{

Hmm, still the blocking wait.  Can't you do something like
"qemu_set_fd_handler(dmabuf->fence_fd, ...)" to avoid the
eglClientWaitSyncKHR() completely?

take care,
  Gerd

RE: [PATCH v2 2/8] ui/egl: Add egl helpers to help with synchronization

[Kasireddy, Vivek]

Hi Gerd,
 
> > +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_dmabuf_wait_sync(QemuDmaBuf *dmabuf)
> > +{
> 
> Hmm, still the blocking wait.  Can't you do something like
[Kasireddy, Vivek] Right, it is a blocking wait; but this gets called from a new GTK thread
that does the actual drawing. 

> "qemu_set_fd_handler(dmabuf->fence_fd, ...)" to avoid the
> eglClientWaitSyncKHR() completely?
[Kasireddy, Vivek] Yeah, I think this is also doable; let me look into it.

Thanks,
Vivek
> 
> take care,
>   Gerd

[PATCH v2 3/8] ui: Add a helper to wait on a dmabuf sync object

[Vivek Kasireddy]

This will be called by virtio-gpu in the subsequent patches.

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

diff --git a/include/ui/console.h b/include/ui/console.h
index 49978fdae3..a89f739f10 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -242,6 +242,9 @@ typedef struct DisplayChangeListenerOps {
     /* optional */
     void (*dpy_gl_release_dmabuf)(DisplayChangeListener *dcl,
                                   QemuDmaBuf *dmabuf);
+    /* optional */
+    void (*dpy_gl_wait_dmabuf)(DisplayChangeListener *dcl,
+                               QemuDmaBuf *dmabuf);
     /* required if GL */
     void (*dpy_gl_update)(DisplayChangeListener *dcl,
                           uint32_t x, uint32_t y, uint32_t w, uint32_t h);
@@ -314,6 +317,8 @@ void dpy_gl_cursor_position(QemuConsole *con,
                             uint32_t pos_x, uint32_t pos_y);
 void dpy_gl_release_dmabuf(QemuConsole *con,
                            QemuDmaBuf *dmabuf);
+void dpy_gl_wait_dmabuf(QemuConsole *con,
+                        QemuDmaBuf *dmabuf);
 void dpy_gl_update(QemuConsole *con,
                    uint32_t x, uint32_t y, uint32_t w, uint32_t h);
 
diff --git a/ui/console.c b/ui/console.c
index 2de5f4105b..b0abfd2246 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1917,6 +1917,16 @@ void dpy_gl_release_dmabuf(QemuConsole *con,
     }
 }
 
+void dpy_gl_wait_dmabuf(QemuConsole *con,
+                        QemuDmaBuf *dmabuf)
+{
+    assert(con->gl);
+
+    if (con->gl->ops->dpy_gl_wait_dmabuf) {
+        con->gl->ops->dpy_gl_wait_dmabuf(con->gl, dmabuf);
+    }
+}
+
 void dpy_gl_update(QemuConsole *con,
                    uint32_t x, uint32_t y, uint32_t w, uint32_t h)
 {
-- 
2.30.2

[PATCH v2 4/8] ui/gtk: Implement wait_dmabuf function

[Vivek Kasireddy]

Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 ui/gtk.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/ui/gtk.c b/ui/gtk.c
index 6132bab52f..cd884ca26c 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -583,6 +583,19 @@ static void gd_gl_release_dmabuf(DisplayChangeListener *dcl,
 #endif
 }
 
+static void gd_gl_wait_dmabuf(DisplayChangeListener *dcl,
+                              QemuDmaBuf *dmabuf)
+{
+#ifdef CONFIG_GBM
+    egl_dmabuf_create_fence(dmabuf);
+    if (dmabuf->fence_fd <= 0) {
+        return;
+    }
+
+    egl_dmabuf_wait_sync(dmabuf);
+#endif
+}
+
 /** DisplayState Callbacks (opengl version) **/
 
 static const DisplayChangeListenerOps dcl_gl_area_ops = {
@@ -602,6 +615,7 @@ static const DisplayChangeListenerOps dcl_gl_area_ops = {
     .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_gl_wait_dmabuf      = gd_gl_wait_dmabuf,
     .dpy_has_dmabuf          = gd_has_dmabuf,
 };
 
@@ -626,6 +640,7 @@ static const DisplayChangeListenerOps dcl_egl_ops = {
     .dpy_gl_cursor_position  = gd_egl_cursor_position,
     .dpy_gl_update           = gd_egl_scanout_flush,
     .dpy_gl_release_dmabuf   = gd_gl_release_dmabuf,
+    .dpy_gl_wait_dmabuf      = gd_gl_wait_dmabuf,
     .dpy_has_dmabuf          = gd_has_dmabuf,
 };
 
-- 
2.30.2

[PATCH v2 5/8] ui: Create sync objects only for blobs

[Vivek Kasireddy]

For now, create sync objects only for dmabufs that are blobs.

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

diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
index 3c01a415e7..33e329e8aa 100644
--- a/hw/display/virtio-gpu-udmabuf.c
+++ b/hw/display/virtio-gpu-udmabuf.c
@@ -185,6 +185,8 @@ 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->buf.sync = NULL;
 
     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 a89f739f10..310d34c67a 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 5a7575dc13..1bc0e31b03 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/ui/gtk-egl.c b/ui/gtk-egl.c
index b671181272..b748f51b0b 100644
--- a/ui/gtk-egl.c
+++ b/ui/gtk-egl.c
@@ -209,6 +209,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 +219,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 +287,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..94f3b87c42 100644
--- a/ui/gtk-gl-area.c
+++ b/ui/gtk-gl-area.c
@@ -71,6 +71,10 @@ void gd_gl_area_draw(VirtualConsole *vc)
         surface_gl_render_texture(vc->gfx.gls, vc->gfx.ds);
     }
 
+    if (vc->gfx.guest_fb.dmabuf) {
+        egl_dmabuf_create_sync(vc->gfx.guest_fb.dmabuf);
+    }
+
     glFlush();
     graphic_hw_gl_flushed(vc->gfx.dcl.con);
 }
@@ -231,6 +235,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
 }
 
-- 
2.30.2

[PATCH v2 6/8] 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     | 14 ++++++++++++++
 ui/gtk.c         |  2 +-
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/include/ui/gtk.h b/include/ui/gtk.h
index e6cbf0507c..34e767a1da 100644
--- a/include/ui/gtk.h
+++ b/include/ui/gtk.h
@@ -178,6 +178,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 b748f51b0b..a5655b6bbc 100644
--- a/ui/gtk-egl.c
+++ b/ui/gtk-egl.c
@@ -294,6 +294,20 @@ 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) {
+        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 cd884ca26c..af94f12a98 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -638,7 +638,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_gl_wait_dmabuf      = gd_gl_wait_dmabuf,
     .dpy_has_dmabuf          = gd_has_dmabuf,
-- 
2.30.2

[PATCH v2 7/8] virtio-gpu: Add dmabuf helpers for synchronization

[Vivek Kasireddy]

These helpers will be used in the next subsequent patches to
wait until a dmabuf object (via a texture) has been used
by the UI to render and submit its buffer.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 hw/display/virtio-gpu-udmabuf.c | 28 ++++++++++++++++++++++++++++
 include/hw/virtio/virtio-gpu.h  |  2 ++
 stubs/virtio-gpu-udmabuf.c      |  6 ++++++
 3 files changed, 36 insertions(+)

diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
index 33e329e8aa..8c1b6f8763 100644
--- a/hw/display/virtio-gpu-udmabuf.c
+++ b/hw/display/virtio-gpu-udmabuf.c
@@ -167,6 +167,34 @@ static void virtio_gpu_free_dmabuf(VirtIOGPU *g, VGPUDMABuf *dmabuf)
     g_free(dmabuf);
 }
 
+static VGPUDMABuf
+*virtio_gpu_find_dmabuf(VirtIOGPU *g,
+                        struct virtio_gpu_simple_resource *res)
+{
+    VGPUDMABuf *dmabuf, *tmp;
+
+    QTAILQ_FOREACH_SAFE(dmabuf, &g->dmabuf.bufs, next, tmp) {
+        if (dmabuf->buf.fd == res->dmabuf_fd) {
+            return dmabuf;
+        }
+    }
+
+    return NULL;
+}
+
+void virtio_gpu_resource_wait_sync(VirtIOGPU *g,
+                                   struct virtio_gpu_simple_resource *res)
+{
+    struct virtio_gpu_scanout *scanout;
+    VGPUDMABuf *dmabuf;
+
+    dmabuf = virtio_gpu_find_dmabuf(g, res);
+    if (dmabuf && dmabuf->buf.sync) {
+        scanout = &g->parent_obj.scanout[dmabuf->scanout_id];
+        dpy_gl_wait_dmabuf(scanout->con, &dmabuf->buf);
+    }
+}
+
 static VGPUDMABuf
 *virtio_gpu_create_dmabuf(VirtIOGPU *g,
                           uint32_t scanout_id,
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index bcf54d970f..9b9b499d06 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -274,6 +274,8 @@ int virtio_gpu_update_dmabuf(VirtIOGPU *g,
                              uint32_t scanout_id,
                              struct virtio_gpu_simple_resource *res,
                              struct virtio_gpu_framebuffer *fb);
+void virtio_gpu_resource_wait_sync(VirtIOGPU *g,
+                                   struct virtio_gpu_simple_resource *res);
 
 /* virtio-gpu-3d.c */
 void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
diff --git a/stubs/virtio-gpu-udmabuf.c b/stubs/virtio-gpu-udmabuf.c
index 81f661441a..59dab1a66c 100644
--- a/stubs/virtio-gpu-udmabuf.c
+++ b/stubs/virtio-gpu-udmabuf.c
@@ -25,3 +25,9 @@ int virtio_gpu_update_dmabuf(VirtIOGPU *g,
     /* nothing (stub) */
     return 0;
 }
+
+void virtio_gpu_resource_wait_sync(VirtIOGPU *g,
+                                   struct virtio_gpu_simple_resource *res)
+{
+    /* nothing (stub) */
+}
-- 
2.30.2

[PATCH v2 8/8] virtio-gpu: Add gl_flushed callback

[Vivek Kasireddy]

Adding this callback provides a way to determine when the UI
has submitted the buffer to the Host windowing system. Making
the guest wait for this event will ensure that the dmabuf/buffer
updates are synchronized.

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

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 4d549377cb..bd96332973 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -982,7 +982,7 @@ void virtio_gpu_simple_process_cmd(VirtIOGPU *g,
         cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
         break;
     }
-    if (!cmd->finished) {
+    if (!cmd->finished && !(cmd->cmd_hdr.flags & VIRTIO_GPU_FLAG_FENCE)) {
         virtio_gpu_ctrl_response_nodata(g, cmd, cmd->error ? cmd->error :
                                         VIRTIO_GPU_RESP_OK_NODATA);
     }
@@ -1040,6 +1040,46 @@ void virtio_gpu_process_cmdq(VirtIOGPU *g)
     g->processing_cmdq = false;
 }
 
+static void virtio_gpu_signal_fence(VirtIOGPU *g,
+                                    struct virtio_gpu_ctrl_command *cmd,
+                                    enum virtio_gpu_ctrl_type type)
+{
+    struct virtio_gpu_simple_resource *res;
+    struct virtio_gpu_resource_flush rf;
+
+    VIRTIO_GPU_FILL_CMD(rf);
+    virtio_gpu_bswap_32(&rf, sizeof(rf));
+    res = virtio_gpu_find_check_resource(g, rf.resource_id, true,
+                                         __func__, &cmd->error);
+    if (res) {
+        virtio_gpu_resource_wait_sync(g, res);
+    }
+    virtio_gpu_ctrl_response_nodata(g, cmd, VIRTIO_GPU_RESP_OK_NODATA);
+}
+
+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_signal_fence(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);
+}
+
 static void virtio_gpu_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
 {
     VirtIOGPU *g = VIRTIO_GPU(vdev);
@@ -1398,10 +1438,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 v2 8/8] virtio-gpu: Add gl_flushed callback

[Gerd Hoffmann]

  Hi,

> -    if (!cmd->finished) {
> +    if (!cmd->finished && !(cmd->cmd_hdr.flags & VIRTIO_GPU_FLAG_FENCE)) {
>          virtio_gpu_ctrl_response_nodata(g, cmd, cmd->error ? cmd->error :
>                                          VIRTIO_GPU_RESP_OK_NODATA);
>      }

My idea would be more along the lines of ...

if (!cmd->finished) {
    if (renderer_blocked) {
       g->pending_completion = cmd;
    } else {
       virtio_gpu_ctrl_response_nodata(...)
    }
}

Then, when resuming processing after unblock check pending_completion
and call virtio_gpu_ctrl_response_nodata if needed.

Workflow:

virtio_gpu_simple_process_cmd()
 -> virtio_gpu_resource_flush()
   -> dpy_gfx_update()
     -> gd_gl_area_update()
        call graphic_hw_gl_block(true), create fence.
virtio_gpu_simple_process_cmd()
  -> will see renderer_blocked and delays RESOURCE_FLUSH completion.

Then, when the fence is ready, gtk will:
 - call graphic_hw_gl_block(false)
 - call graphic_hw_gl_flush()
   -> virtio-gpu resumes processing the cmd queue.

When you use the existing block/unblock functionality the fence can be a
gtk internal detail, virtio-gpu doesn't need to know that gtk uses a
fence to wait for the moment when it can unblock virtio queue processing
(the egl fence helpers still make sense).

take care,
  Gerd

RE: [PATCH v2 8/8] virtio-gpu: Add gl_flushed callback

[Kasireddy, Vivek]

Hi Gerd,

> 
> > -    if (!cmd->finished) {
> > +    if (!cmd->finished && !(cmd->cmd_hdr.flags & VIRTIO_GPU_FLAG_FENCE)) {
> >          virtio_gpu_ctrl_response_nodata(g, cmd, cmd->error ? cmd->error :
> >                                          VIRTIO_GPU_RESP_OK_NODATA);
> >      }
> 
> My idea would be more along the lines of ...
> 
> if (!cmd->finished) {
>     if (renderer_blocked) {
>        g->pending_completion = cmd;
>     } else {
>        virtio_gpu_ctrl_response_nodata(...)
>     }
> }
> 
> Then, when resuming processing after unblock check pending_completion
> and call virtio_gpu_ctrl_response_nodata if needed.
> 
> Workflow:
> 
> virtio_gpu_simple_process_cmd()
>  -> virtio_gpu_resource_flush()
>    -> dpy_gfx_update()
>      -> gd_gl_area_update()
>         call graphic_hw_gl_block(true), create fence.
[Kasireddy, Vivek] So, with blobs, as you know we call dpy_gl_update() and this call
just "queues" the render/redraw. And, GTK then later calls the render signal callback 
which in this case would be gd_gl_area_draw() which is where the actual Blit happens
and also glFlush; only after which we can create a fence.

> virtio_gpu_simple_process_cmd()
>   -> will see renderer_blocked and delays RESOURCE_FLUSH completion.
> 
> Then, when the fence is ready, gtk will:
>  - call graphic_hw_gl_block(false)
>  - call graphic_hw_gl_flush()
>    -> virtio-gpu resumes processing the cmd queue.
[Kasireddy, Vivek] Yeah, I think this can be done.

> 
> When you use the existing block/unblock functionality the fence can be a
> gtk internal detail, virtio-gpu doesn't need to know that gtk uses a
> fence to wait for the moment when it can unblock virtio queue processing
> (the egl fence helpers still make sense).
[Kasireddy, Vivek] Ok, I'll try to include your suggestions in v3.

Thanks,
Vivek
> 
> take care,
>   Gerd

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

[no-reply]

Patchew URL: https://patchew.org/QEMU/20210610224837.670192-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: 20210610224837.670192-1-vivek.kasireddy@intel.com
Subject: [PATCH v2 0/8] 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
Switched to a new branch 'test'
d7ca345 virtio-gpu: Add gl_flushed callback
2a73d47 virtio-gpu: Add dmabuf helpers for synchronization
0d1be46 ui/gtk-egl: Wait for the draw signal for dmabuf blobs
3fd1b59 ui: Create sync objects only for blobs
13095b9 ui/gtk: Implement wait_dmabuf function
e2c7ec7 ui: Add a helper to wait on a dmabuf sync object
ffbfd82 ui/egl: Add egl helpers to help with synchronization
4612c6a ui/gtk: Create a common release_dmabuf helper

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

total: 1 errors, 0 warnings, 67 lines checked

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

3/8 Checking commit e2c7ec781b0c (ui: Add a helper to wait on a dmabuf sync object)
4/8 Checking commit 13095b996f1a (ui/gtk: Implement wait_dmabuf function)
5/8 Checking commit 3fd1b5907cbd (ui: Create sync objects only for blobs)
6/8 Checking commit 0d1be4679e6f (ui/gtk-egl: Wait for the draw signal for dmabuf blobs)
7/8 Checking commit 2a73d47f2458 (virtio-gpu: Add dmabuf helpers for synchronization)
8/8 Checking commit d7ca345c6ce8 (virtio-gpu: Add gl_flushed callback)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210610224837.670192-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