Hi
On Fri, Jun 21, 2024 at 3:19 AM <dongwon....@intel.com> wrote:
> From: Dongwon Kim <dongwon....@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 <kra...@redhat.com>
> Cc: Marc-André Lureau <marcandre.lur...@redhat.com>
> Cc: Vivek Kasireddy <vivek.kasire...@intel.com>
> Signed-off-by: Dongwon Kim <dongwon....@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
