| Message ID | 20240620231727.235841-3-dongwon.kim@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/display/virtio-gpu: Introducing asynchronous guest display render | expand |
Hi On Fri, Jun 21, 2024 at 3:19 AM <dongwon.kim@intel.com> wrote: > From: Dongwon Kim <dongwon.kim@intel.com> > > There is no reason to split those two operations so combining > two functions - egl_dmabuf_create_sync and egl_dmabuf_create_fence. > > Cc: Gerd Hoffmann <kraxel@redhat.com> > Cc: Marc-André Lureau <marcandre.lureau@redhat.com> > Cc: Vivek Kasireddy <vivek.kasireddy@intel.com> > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com> > Can you send this change as a different patch series, or earlier in the series? You should also remove qemu_dmabuf_{set,get}_sync() and associated fields --- > include/ui/egl-helpers.h | 3 +-- > ui/egl-helpers.c | 27 +++++++++++---------------- > ui/gtk-egl.c | 19 +++++++------------ > ui/gtk-gl-area.c | 10 ++-------- > 4 files changed, 21 insertions(+), 38 deletions(-) > > diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h > index 4b8c0d2281..606d6c8288 100644 > --- a/include/ui/egl-helpers.h > +++ b/include/ui/egl-helpers.h > @@ -51,8 +51,7 @@ 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); > +int egl_dmabuf_create_fence(QemuDmaBuf *dmabuf); > > #endif > > diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c > index 99b2ebbe23..e16f2cb23d 100644 > --- a/ui/egl-helpers.c > +++ b/ui/egl-helpers.c > @@ -371,34 +371,29 @@ void egl_dmabuf_release_texture(QemuDmaBuf *dmabuf) > qemu_dmabuf_set_texture(dmabuf, 0); > } > > -void egl_dmabuf_create_sync(QemuDmaBuf *dmabuf) > +int egl_dmabuf_create_fence(QemuDmaBuf *dmabuf) > { > EGLSyncKHR sync; > + int fence_fd = -1; > > if (epoxy_has_egl_extension(qemu_egl_display, > "EGL_KHR_fence_sync") && > epoxy_has_egl_extension(qemu_egl_display, > - "EGL_ANDROID_native_fence_sync")) { > + "EGL_ANDROID_native_fence_sync") && > + qemu_dmabuf_get_fence_fd(dmabuf) == -1) { > sync = eglCreateSyncKHR(qemu_egl_display, > EGL_SYNC_NATIVE_FENCE_ANDROID, NULL); > if (sync != EGL_NO_SYNC_KHR) { > - qemu_dmabuf_set_sync(dmabuf, sync); > + fence_fd = eglDupNativeFenceFDANDROID(qemu_egl_display, > + sync); > + if (fence_fd >= 0) { > + qemu_dmabuf_set_fence_fd(dmabuf, fence_fd); > + } > + eglDestroySyncKHR(qemu_egl_display, sync); > } > } > -} > - > -void egl_dmabuf_create_fence(QemuDmaBuf *dmabuf) > -{ > - void *sync = qemu_dmabuf_get_sync(dmabuf); > - int fence_fd; > > - if (sync) { > - fence_fd = eglDupNativeFenceFDANDROID(qemu_egl_display, > - sync); > - qemu_dmabuf_set_fence_fd(dmabuf, fence_fd); > - eglDestroySyncKHR(qemu_egl_display, sync); > - qemu_dmabuf_set_sync(dmabuf, NULL); > - } > + return fence_fd; > It should probably return qemu_dmabuf_get_fence_fd() instead. The function should be renamed with _fd postfix since you make it return the fd. } > > #endif /* CONFIG_GBM */ > diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c > index 9831c10e1b..55199f8659 100644 > --- a/ui/gtk-egl.c > +++ b/ui/gtk-egl.c > @@ -68,7 +68,6 @@ void gd_egl_draw(VirtualConsole *vc) > GdkWindow *window; > #ifdef CONFIG_GBM > QemuDmaBuf *dmabuf = vc->gfx.guest_fb.dmabuf; > - int fence_fd; > #endif > int ww, wh, ws; > > @@ -99,13 +98,12 @@ void gd_egl_draw(VirtualConsole *vc) > glFlush(); > #ifdef CONFIG_GBM > if (dmabuf) { > - egl_dmabuf_create_fence(dmabuf); > - fence_fd = qemu_dmabuf_get_fence_fd(dmabuf); > + fence_fd = egl_dmabuf_create_fence(dmabuf); > if (fence_fd >= 0) { > qemu_set_fd_handler(fence_fd, gd_hw_gl_flushed, NULL, vc); > - return; > + } else { > + graphic_hw_gl_block(vc->gfx.dcl.con, false); > } > - graphic_hw_gl_block(vc->gfx.dcl.con, false); > } > #endif > } else { > @@ -336,7 +334,11 @@ void gd_egl_scanout_flush(DisplayChangeListener *dcl, > { > VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl); > GdkWindow *window; > +#ifdef CONFIG_GBM > + QemuDmaBuf *dmabuf = vc->gfx.guest_fb.dmabuf; > +#endif > int ww, wh, ws; > + int fence_fd; > > if (!vc->gfx.scanout_mode) { > return; > @@ -364,12 +366,6 @@ void gd_egl_scanout_flush(DisplayChangeListener *dcl, > egl_fb_blit(&vc->gfx.win_fb, &vc->gfx.guest_fb, !vc->gfx.y0_top); > } > > -#ifdef CONFIG_GBM > - if (vc->gfx.guest_fb.dmabuf) { > - egl_dmabuf_create_sync(vc->gfx.guest_fb.dmabuf); > - } > -#endif > - > eglSwapBuffers(qemu_egl_display, vc->gfx.esurface); > } > > @@ -387,7 +383,6 @@ void gd_egl_flush(DisplayChangeListener *dcl, > gtk_widget_queue_draw_area(area, x, y, w, h); > return; > } > - > gd_egl_scanout_flush(&vc->gfx.dcl, x, y, w, h); > } > > diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c > index b628b35451..0b11423824 100644 > --- a/ui/gtk-gl-area.c > +++ b/ui/gtk-gl-area.c > @@ -77,17 +77,10 @@ void gd_gl_area_draw(VirtualConsole *vc) > glBlitFramebuffer(0, y1, vc->gfx.w, y2, > 0, 0, ww, wh, > GL_COLOR_BUFFER_BIT, GL_NEAREST); > -#ifdef CONFIG_GBM > - if (dmabuf) { > - egl_dmabuf_create_sync(dmabuf); > - } > -#endif > - glFlush(); > #ifdef CONFIG_GBM > if (dmabuf) { > int fence_fd; > - egl_dmabuf_create_fence(dmabuf); > - fence_fd = qemu_dmabuf_get_fence_fd(dmabuf); > + fence_fd = egl_dmabuf_create_fence(dmabuf); > if (fence_fd >= 0) { > qemu_set_fd_handler(fence_fd, gd_hw_gl_flushed, NULL, vc); > return; > @@ -95,6 +88,7 @@ void gd_gl_area_draw(VirtualConsole *vc) > graphic_hw_gl_block(vc->gfx.dcl.con, false); > } > #endif > + glFlush(); > } else { > if (!vc->gfx.ds) { > return; > -- > 2.34.1 > > >
Hi Marc-André, I will come up with a separate patch for this. On 7/2/2024 1:32 AM, Marc-André Lureau wrote: > Hi > > On Fri, Jun 21, 2024 at 3:19 AM <dongwon.kim@intel.com > <mailto:dongwon.kim@intel.com>> wrote: > > From: Dongwon Kim <dongwon.kim@intel.com <mailto:dongwon.kim@intel.com>> > > There is no reason to split those two operations so combining > two functions - egl_dmabuf_create_sync and egl_dmabuf_create_fence. > > Cc: Gerd Hoffmann <kraxel@redhat.com <mailto:kraxel@redhat.com>> > Cc: Marc-André Lureau <marcandre.lureau@redhat.com > <mailto:marcandre.lureau@redhat.com>> > Cc: Vivek Kasireddy <vivek.kasireddy@intel.com > <mailto:vivek.kasireddy@intel.com>> > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com > <mailto:dongwon.kim@intel.com>> > > > Can you send this change as a different patch series, or earlier in the > series? > > You should also remove qemu_dmabuf_{set,get}_sync() and associated fields > > --- > include/ui/egl-helpers.h | 3 +-- > ui/egl-helpers.c | 27 +++++++++++---------------- > ui/gtk-egl.c | 19 +++++++------------ > ui/gtk-gl-area.c | 10 ++-------- > 4 files changed, 21 insertions(+), 38 deletions(-) > > diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h > index 4b8c0d2281..606d6c8288 100644 > --- a/include/ui/egl-helpers.h > +++ b/include/ui/egl-helpers.h > @@ -51,8 +51,7 @@ 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); > +int egl_dmabuf_create_fence(QemuDmaBuf *dmabuf); > > #endif > > diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c > index 99b2ebbe23..e16f2cb23d 100644 > --- a/ui/egl-helpers.c > +++ b/ui/egl-helpers.c > @@ -371,34 +371,29 @@ void egl_dmabuf_release_texture(QemuDmaBuf > *dmabuf) > qemu_dmabuf_set_texture(dmabuf, 0); > } > > -void egl_dmabuf_create_sync(QemuDmaBuf *dmabuf) > +int egl_dmabuf_create_fence(QemuDmaBuf *dmabuf) > { > EGLSyncKHR sync; > + int fence_fd = -1; > > if (epoxy_has_egl_extension(qemu_egl_display, > "EGL_KHR_fence_sync") && > epoxy_has_egl_extension(qemu_egl_display, > - "EGL_ANDROID_native_fence_sync")) { > + "EGL_ANDROID_native_fence_sync") && > + qemu_dmabuf_get_fence_fd(dmabuf) == -1) { > sync = eglCreateSyncKHR(qemu_egl_display, > EGL_SYNC_NATIVE_FENCE_ANDROID, NULL); > if (sync != EGL_NO_SYNC_KHR) { > - qemu_dmabuf_set_sync(dmabuf, sync); > + fence_fd = eglDupNativeFenceFDANDROID(qemu_egl_display, > + sync); > + if (fence_fd >= 0) { > + qemu_dmabuf_set_fence_fd(dmabuf, fence_fd); > + } > + eglDestroySyncKHR(qemu_egl_display, sync); > } > } > -} > - > -void egl_dmabuf_create_fence(QemuDmaBuf *dmabuf) > -{ > - void *sync = qemu_dmabuf_get_sync(dmabuf); > - int fence_fd; > > - if (sync) { > - fence_fd = eglDupNativeFenceFDANDROID(qemu_egl_display, > - sync); > - qemu_dmabuf_set_fence_fd(dmabuf, fence_fd); > - eglDestroySyncKHR(qemu_egl_display, sync); > - qemu_dmabuf_set_sync(dmabuf, NULL); > - } > + return fence_fd; > > > It should probably return qemu_dmabuf_get_fence_fd() instead. The > function should be renamed with _fd postfix since you make it return the fd. > > } > > #endif /* CONFIG_GBM */ > diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c > index 9831c10e1b..55199f8659 100644 > --- a/ui/gtk-egl.c > +++ b/ui/gtk-egl.c > @@ -68,7 +68,6 @@ void gd_egl_draw(VirtualConsole *vc) > GdkWindow *window; > #ifdef CONFIG_GBM > QemuDmaBuf *dmabuf = vc->gfx.guest_fb.dmabuf; > - int fence_fd; > #endif > int ww, wh, ws; > > @@ -99,13 +98,12 @@ void gd_egl_draw(VirtualConsole *vc) > glFlush(); > #ifdef CONFIG_GBM > if (dmabuf) { > - egl_dmabuf_create_fence(dmabuf); > - fence_fd = qemu_dmabuf_get_fence_fd(dmabuf); > + fence_fd = egl_dmabuf_create_fence(dmabuf); > if (fence_fd >= 0) { > qemu_set_fd_handler(fence_fd, gd_hw_gl_flushed, > NULL, vc); > - return; > + } else { > + graphic_hw_gl_block(vc->gfx.dcl.con, false); > } > - graphic_hw_gl_block(vc->gfx.dcl.con, false); > } > #endif > } else { > @@ -336,7 +334,11 @@ void gd_egl_scanout_flush(DisplayChangeListener > *dcl, > { > VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl); > GdkWindow *window; > +#ifdef CONFIG_GBM > + QemuDmaBuf *dmabuf = vc->gfx.guest_fb.dmabuf; > +#endif > int ww, wh, ws; > + int fence_fd; > > if (!vc->gfx.scanout_mode) { > return; > @@ -364,12 +366,6 @@ void gd_egl_scanout_flush(DisplayChangeListener > *dcl, > egl_fb_blit(&vc->gfx.win_fb, &vc->gfx.guest_fb, > !vc->gfx.y0_top); > } > > -#ifdef CONFIG_GBM > - if (vc->gfx.guest_fb.dmabuf) { > - egl_dmabuf_create_sync(vc->gfx.guest_fb.dmabuf); > - } > -#endif > - > eglSwapBuffers(qemu_egl_display, vc->gfx.esurface); > } > > @@ -387,7 +383,6 @@ void gd_egl_flush(DisplayChangeListener *dcl, > gtk_widget_queue_draw_area(area, x, y, w, h); > return; > } > - > gd_egl_scanout_flush(&vc->gfx.dcl, x, y, w, h); > } > > diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c > index b628b35451..0b11423824 100644 > --- a/ui/gtk-gl-area.c > +++ b/ui/gtk-gl-area.c > @@ -77,17 +77,10 @@ void gd_gl_area_draw(VirtualConsole *vc) > glBlitFramebuffer(0, y1, vc->gfx.w, y2, > 0, 0, ww, wh, > GL_COLOR_BUFFER_BIT, GL_NEAREST); > -#ifdef CONFIG_GBM > - if (dmabuf) { > - egl_dmabuf_create_sync(dmabuf); > - } > -#endif > - glFlush(); > #ifdef CONFIG_GBM > if (dmabuf) { > int fence_fd; > - egl_dmabuf_create_fence(dmabuf); > - fence_fd = qemu_dmabuf_get_fence_fd(dmabuf); > + fence_fd = egl_dmabuf_create_fence(dmabuf); > if (fence_fd >= 0) { > qemu_set_fd_handler(fence_fd, gd_hw_gl_flushed, > NULL, vc); > return; > @@ -95,6 +88,7 @@ void gd_gl_area_draw(VirtualConsole *vc) > graphic_hw_gl_block(vc->gfx.dcl.con, false); > } > #endif > + glFlush(); > } else { > if (!vc->gfx.ds) { > return; > -- > 2.34.1 > > > > > -- > Marc-André Lureau
diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h index 4b8c0d2281..606d6c8288 100644 --- a/include/ui/egl-helpers.h +++ b/include/ui/egl-helpers.h @@ -51,8 +51,7 @@ 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); +int egl_dmabuf_create_fence(QemuDmaBuf *dmabuf); #endif diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c index 99b2ebbe23..e16f2cb23d 100644 --- a/ui/egl-helpers.c +++ b/ui/egl-helpers.c @@ -371,34 +371,29 @@ void egl_dmabuf_release_texture(QemuDmaBuf *dmabuf) qemu_dmabuf_set_texture(dmabuf, 0); } -void egl_dmabuf_create_sync(QemuDmaBuf *dmabuf) +int egl_dmabuf_create_fence(QemuDmaBuf *dmabuf) { EGLSyncKHR sync; + int fence_fd = -1; if (epoxy_has_egl_extension(qemu_egl_display, "EGL_KHR_fence_sync") && epoxy_has_egl_extension(qemu_egl_display, - "EGL_ANDROID_native_fence_sync")) { + "EGL_ANDROID_native_fence_sync") && + qemu_dmabuf_get_fence_fd(dmabuf) == -1) { sync = eglCreateSyncKHR(qemu_egl_display, EGL_SYNC_NATIVE_FENCE_ANDROID, NULL); if (sync != EGL_NO_SYNC_KHR) { - qemu_dmabuf_set_sync(dmabuf, sync); + fence_fd = eglDupNativeFenceFDANDROID(qemu_egl_display, + sync); + if (fence_fd >= 0) { + qemu_dmabuf_set_fence_fd(dmabuf, fence_fd); + } + eglDestroySyncKHR(qemu_egl_display, sync); } } -} - -void egl_dmabuf_create_fence(QemuDmaBuf *dmabuf) -{ - void *sync = qemu_dmabuf_get_sync(dmabuf); - int fence_fd; - if (sync) { - fence_fd = eglDupNativeFenceFDANDROID(qemu_egl_display, - sync); - qemu_dmabuf_set_fence_fd(dmabuf, fence_fd); - eglDestroySyncKHR(qemu_egl_display, sync); - qemu_dmabuf_set_sync(dmabuf, NULL); - } + return fence_fd; } #endif /* CONFIG_GBM */ diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c index 9831c10e1b..55199f8659 100644 --- a/ui/gtk-egl.c +++ b/ui/gtk-egl.c @@ -68,7 +68,6 @@ void gd_egl_draw(VirtualConsole *vc) GdkWindow *window; #ifdef CONFIG_GBM QemuDmaBuf *dmabuf = vc->gfx.guest_fb.dmabuf; - int fence_fd; #endif int ww, wh, ws; @@ -99,13 +98,12 @@ void gd_egl_draw(VirtualConsole *vc) glFlush(); #ifdef CONFIG_GBM if (dmabuf) { - egl_dmabuf_create_fence(dmabuf); - fence_fd = qemu_dmabuf_get_fence_fd(dmabuf); + fence_fd = egl_dmabuf_create_fence(dmabuf); if (fence_fd >= 0) { qemu_set_fd_handler(fence_fd, gd_hw_gl_flushed, NULL, vc); - return; + } else { + graphic_hw_gl_block(vc->gfx.dcl.con, false); } - graphic_hw_gl_block(vc->gfx.dcl.con, false); } #endif } else { @@ -336,7 +334,11 @@ void gd_egl_scanout_flush(DisplayChangeListener *dcl, { VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl); GdkWindow *window; +#ifdef CONFIG_GBM + QemuDmaBuf *dmabuf = vc->gfx.guest_fb.dmabuf; +#endif int ww, wh, ws; + int fence_fd; if (!vc->gfx.scanout_mode) { return; @@ -364,12 +366,6 @@ void gd_egl_scanout_flush(DisplayChangeListener *dcl, egl_fb_blit(&vc->gfx.win_fb, &vc->gfx.guest_fb, !vc->gfx.y0_top); } -#ifdef CONFIG_GBM - if (vc->gfx.guest_fb.dmabuf) { - egl_dmabuf_create_sync(vc->gfx.guest_fb.dmabuf); - } -#endif - eglSwapBuffers(qemu_egl_display, vc->gfx.esurface); } @@ -387,7 +383,6 @@ void gd_egl_flush(DisplayChangeListener *dcl, gtk_widget_queue_draw_area(area, x, y, w, h); return; } - gd_egl_scanout_flush(&vc->gfx.dcl, x, y, w, h); } diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c index b628b35451..0b11423824 100644 --- a/ui/gtk-gl-area.c +++ b/ui/gtk-gl-area.c @@ -77,17 +77,10 @@ void gd_gl_area_draw(VirtualConsole *vc) glBlitFramebuffer(0, y1, vc->gfx.w, y2, 0, 0, ww, wh, GL_COLOR_BUFFER_BIT, GL_NEAREST); -#ifdef CONFIG_GBM - if (dmabuf) { - egl_dmabuf_create_sync(dmabuf); - } -#endif - glFlush(); #ifdef CONFIG_GBM if (dmabuf) { int fence_fd; - egl_dmabuf_create_fence(dmabuf); - fence_fd = qemu_dmabuf_get_fence_fd(dmabuf); + fence_fd = egl_dmabuf_create_fence(dmabuf); if (fence_fd >= 0) { qemu_set_fd_handler(fence_fd, gd_hw_gl_flushed, NULL, vc); return; @@ -95,6 +88,7 @@ void gd_gl_area_draw(VirtualConsole *vc) graphic_hw_gl_block(vc->gfx.dcl.con, false); } #endif + glFlush(); } else { if (!vc->gfx.ds) { return;