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.
v2: egl_dmabuf_create_fence -> egl_dmabuf_create_fence_fd
(Marc-André Lureau <marcandre.lureau@redhat.com>)
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>
---
include/ui/egl-helpers.h | 3 +--
ui/egl-helpers.c | 27 +++++++++++----------------
ui/gtk-egl.c | 15 +++------------
ui/gtk-gl-area.c | 10 ++--------
4 files changed, 17 insertions(+), 38 deletions(-)
diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h
index 4b8c0d2281..221548e3c9 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_fd(QemuDmaBuf *dmabuf);
#endif
diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
index 99b2ebbe23..ddbf52bf5f 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_fd(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..8869cdee4f 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);
+ int fence_fd = egl_dmabuf_create_fence_fd(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 {
@@ -364,12 +362,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 +379,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..a3c21c3c59 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_fd(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
On Wed, Jul 3, 2024 at 12:57 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.
>
> v2: egl_dmabuf_create_fence -> egl_dmabuf_create_fence_fd
> (Marc-André Lureau <marcandre.lureau@redhat.com>)
>
> 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>
> ---
> include/ui/egl-helpers.h | 3 +--
> ui/egl-helpers.c | 27 +++++++++++----------------
> ui/gtk-egl.c | 15 +++------------
> ui/gtk-gl-area.c | 10 ++--------
> 4 files changed, 17 insertions(+), 38 deletions(-)
>
> diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h
> index 4b8c0d2281..221548e3c9 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_fd(QemuDmaBuf *dmabuf);
>
> #endif
>
> diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
> index 99b2ebbe23..ddbf52bf5f 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_fd(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..8869cdee4f 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);
> + int fence_fd = egl_dmabuf_create_fence_fd(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);
> }
>
If gd_egl_draw()/gd_egl_refresh() is called multiple times before the
flushed callback, create_fence_fd() is going to return -1 and unblock the
hw. This is probably not desired. I think you need to comment on the code
to explain the interaction between dmabuf_get_draw_submitted,
gd_egl_flush(), fences and hw_block, I can't make sense of it.
thanks
> - graphic_hw_gl_block(vc->gfx.dcl.con, false);
> }
> #endif
> } else {
> @@ -364,12 +362,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 +379,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..a3c21c3c59 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_fd(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
Hi Marc-André,
On 7/3/2024 4:23 AM, Marc-André Lureau wrote:
> Hi
>
> On Wed, Jul 3, 2024 at 12:57 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.
>
> v2: egl_dmabuf_create_fence -> egl_dmabuf_create_fence_fd
> (Marc-André Lureau <marcandre.lureau@redhat.com
> <mailto:marcandre.lureau@redhat.com>>)
>
> 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>>
> ---
> include/ui/egl-helpers.h | 3 +--
> ui/egl-helpers.c | 27 +++++++++++----------------
> ui/gtk-egl.c | 15 +++------------
> ui/gtk-gl-area.c | 10 ++--------
> 4 files changed, 17 insertions(+), 38 deletions(-)
>
> diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h
> index 4b8c0d2281..221548e3c9 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_fd(QemuDmaBuf *dmabuf);
>
> #endif
>
> diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
> index 99b2ebbe23..ddbf52bf5f 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_fd(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..8869cdee4f 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);
> + int fence_fd = egl_dmabuf_create_fence_fd(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);
> }
>
>
> If gd_egl_draw()/gd_egl_refresh() is called multiple times before the
> flushed callback, create_fence_fd() is going to return -1 and unblock
> the hw. This is probably not desired. I think you need to comment on the
> code to explain the interaction between dmabuf_get_draw_submitted,
> gd_egl_flush(), fences and hw_block, I can't make sense of it.
>
> thanks
Yes, indeed it is not desired flow. Let me have some time to take a look
at this case and get back with a better patch.
>
> - graphic_hw_gl_block(vc->gfx.dcl.con, false);
> }
> #endif
> } else {
> @@ -364,12 +362,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 +379,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..a3c21c3c59 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_fd(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
© 2016 - 2024 Red Hat, Inc.