[PATCH v1 0/5] virtio-gpu: Add implicit (and default) sync mechanism for blobs

[PATCH v1 0/5] virtio-gpu: Add implicit (and default) sync 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 returning
control back to the Guest from resource_flush().

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.

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: Add a helper to wait on a dmabuf sync object
  ui/egl: Add egl helpers to help with synchronization
  ui: Create sync objects only for blobs
  virtio-gpu: Make resource_flush wait on the sync object for blobs

 hw/display/virtio-gpu-udmabuf.c | 30 ++++++++++++++++++++++
 hw/display/virtio-gpu.c         |  1 +
 include/hw/virtio/virtio-gpu.h  |  2 ++
 include/ui/console.h            |  8 ++++++
 include/ui/egl-helpers.h        |  4 +++
 include/ui/gtk.h                |  2 --
 ui/console.c                    | 10 ++++++++
 ui/egl-helpers.c                | 44 +++++++++++++++++++++++++++++++++
 ui/gtk-egl.c                    | 18 ++++++++------
 ui/gtk-gl-area.c                |  8 ++++++
 ui/gtk.c                        | 26 ++++++++++++++++++-
 11 files changed, 142 insertions(+), 11 deletions(-)

-- 
2.30.2

[PATCH v1 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>
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 v1 2/5] 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 b30b63976a..c3dca61c31 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -240,6 +240,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);
@@ -312,6 +315,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 v1 3/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 |  3 +++
 ui/egl-helpers.c         | 44 ++++++++++++++++++++++++++++++++++++++++
 ui/gtk.c                 | 15 ++++++++++++++
 4 files changed, 64 insertions(+)

diff --git a/include/ui/console.h b/include/ui/console.h
index c3dca61c31..a89f739f10 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;
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 v1 4/5] 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 v1 5/5] virtio-gpu: Make resource_flush wait on the sync object for blobs

[Vivek Kasireddy]

To make sure that the Guest would not use the backing storage
associated with a blob resource before or at the same time when
the Host does a blit with it, resource_flush needs to wait on
the sync object associated with the blob. Doing this would prevent
tearing/flickering or other issues when using blob resources.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
---
 hw/display/virtio-gpu-udmabuf.c | 28 ++++++++++++++++++++++++++++
 hw/display/virtio-gpu.c         |  1 +
 include/hw/virtio/virtio-gpu.h  |  2 ++
 3 files changed, 31 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/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 4d549377cb..dd037137e9 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -523,6 +523,7 @@ static void virtio_gpu_resource_flush(VirtIOGPU *g,
                 console_has_gl(scanout->con)) {
                 dpy_gl_update(scanout->con, 0, 0, scanout->width,
                               scanout->height);
+                virtio_gpu_resource_wait_sync(g, res);
                 return;
             }
         }
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,
-- 
2.30.2

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

[Gerd Hoffmann]

On Mon, Jun 07, 2021 at 04:25:26PM -0700, Vivek Kasireddy wrote:
> 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>

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

Re: [PATCH v1 2/5] ui: Add a helper to wait on a dmabuf sync object

[Gerd Hoffmann]

  Hi,

> +    /* optional */
> +    void (*dpy_gl_wait_dmabuf)(DisplayChangeListener *dcl,
> +                               QemuDmaBuf *dmabuf);

Hmm, a blocking wait isn't the best plan here,
it'll stall the iothread.

We already have a way to stop virtio-gpu command processing:
graphic_hw_gl_block() + graphic_hw_gl_flush()

graphic_hw_gl_block(true) will block virtio-gpu command processing.
graphic_hw_gl_block(false) will unblock virtio-gpu command processing.
graphic_hw_gl_flush() will kick virtio-gpu so it resumes command processing.

I saw in patch #5 that you just do a blocking wait for the fence, which
isn't fundamentally different from what graphic_hw_gl_block does, so it
should be possible to make the gtk ui use that instead.

You'll need an async notification for the fence then.  I think you can
simply poll the fence fd.  Failing that you can either use a timer to
check the fence or do the blocking wait in a new thread.

One little detail is different with graphic_hw_gl_block:  It'll send the
completion no matter what, then stop fetching the next command from the
queue.  So that logic needs an update to also allow delaying the command
completion.

take care,
  Gerd

Re: [PATCH v1 3/5] ui/egl: Add egl helpers to help with synchronization

[Gerd Hoffmann]

  Hi,

> +        epoxy_has_egl_extension(qemu_egl_display,
> +                                "EGL_ANDROID_native_fence_sync")) {

What about non-android?  Is the name there just for historical reasons?
Or do we actually need something else for desktop systems?

> +void egl_dmabuf_wait_sync(QemuDmaBuf *dmabuf)

See other mail on blocking wait.  Otherwise looks sane.

> +static void gd_gl_wait_dmabuf(DisplayChangeListener *dcl,
> +                              QemuDmaBuf *dmabuf)

separate patch for the gtk bits please.

thanks,
  Gerd

RE: [PATCH v1 3/5] ui/egl: Add egl helpers to help with synchronization

[Kasireddy, Vivek]

Hi Gerd,
 
> > +        epoxy_has_egl_extension(qemu_egl_display,
> > +                                "EGL_ANDROID_native_fence_sync")) {
> 
> What about non-android?  Is the name there just for historical reasons?
> Or do we actually need something else for desktop systems?
[Kasireddy, Vivek] It is not specific to Android:
https://www.khronos.org/registry/EGL/extensions/ANDROID/EGL_ANDROID_native_fence_sync.txt

I have been using Linux (Fedora 33 for both Guest and Host) as my
test platform.

> 
> > +void egl_dmabuf_wait_sync(QemuDmaBuf *dmabuf)
> 
> See other mail on blocking wait.  Otherwise looks sane.
> 
> > +static void gd_gl_wait_dmabuf(DisplayChangeListener *dcl,
> > +                              QemuDmaBuf *dmabuf)
> 
> separate patch for the gtk bits please.
[Kasireddy, Vivek] Ok, will do.

Thanks,
Vivek

> 
> thanks,
>   Gerd

Re: [PATCH v1 0/5] virtio-gpu: Add implicit (and default) sync mechanism for blobs

[no-reply]

Patchew URL: https://patchew.org/QEMU/20210607232530.454435-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: 20210607232530.454435-1-vivek.kasireddy@intel.com
Subject: [PATCH v1 0/5] virtio-gpu: Add implicit (and default) sync 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'
255bbee virtio-gpu: Make resource_flush wait on the sync object for blobs
60d80bd ui: Create sync objects only for blobs
fd4c503 ui/egl: Add egl helpers to help with synchronization
faed37e ui: Add a helper to wait on a dmabuf sync object
9f9a6c6 ui/gtk: Create a common release_dmabuf helper

=== OUTPUT BEGIN ===
1/5 Checking commit 9f9a6c6c4593 (ui/gtk: Create a common release_dmabuf helper)
2/5 Checking commit faed37ec89dc (ui: Add a helper to wait on a dmabuf sync object)
3/5 Checking commit fd4c50392fa3 (ui/egl: Add egl helpers to help with synchronization)
ERROR: code indent should never use tabs
#64: FILE: ui/egl-helpers.c:88:
+^I^I^I^IEGL_SYNC_NATIVE_FENCE_ANDROID, NULL);$

total: 1 errors, 0 warnings, 100 lines checked

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

4/5 Checking commit 60d80bd36f23 (ui: Create sync objects only for blobs)
5/5 Checking commit 255bbee81de3 (virtio-gpu: Make resource_flush wait on the sync object for blobs)
=== OUTPUT END ===

Test command exited with code: 1


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