| Message ID | 20240702205503.2774745-2-dongwon.kim@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Consolidate create-sync and create-fence | expand |
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 > > >
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
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;