Hi
On Thu, May 30, 2024 at 2:44 AM <dongwon....@intel.com> wrote:
> From: Dongwon <dongwon....@intel.com>
>
> Make sure rendering of the current frame is finished before switching
> the run state to RUN_STATE_SAVE_VM by waiting for egl-sync object to be
> signaled.
>
Can you expand on what this solves?
>
> Cc: Marc-André Lureau <marcandre.lur...@redhat.com>
> Cc: Vivek Kasireddy <vivek.kasire...@intel.com>
> Signed-off-by: Dongwon Kim <dongwon....@intel.com>
> ---
> ui/egl-helpers.c | 2 --
> ui/gtk.c | 19 +++++++++++++++++++
> 2 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
> index 99b2ebbe23..dafeb36074 100644
> --- a/ui/egl-helpers.c
> +++ b/ui/egl-helpers.c
> @@ -396,8 +396,6 @@ void egl_dmabuf_create_fence(QemuDmaBuf *dmabuf)
> 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);
>
If this function is called multiple times, it will now set a new fence_fd
each time, and potentially leak older fd. Maybe it could first check if a
fence_fd exists instead.
}
> }
>
> diff --git a/ui/gtk.c b/ui/gtk.c
> index 93b13b7a30..cf0dd6abed 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -600,9 +600,12 @@ void gd_hw_gl_flushed(void *vcon)
>
> fence_fd = qemu_dmabuf_get_fence_fd(dmabuf);
> if (fence_fd >= 0) {
> + void *sync = qemu_dmabuf_get_sync(dmabuf);
> qemu_set_fd_handler(fence_fd, NULL, NULL, NULL);
> close(fence_fd);
> qemu_dmabuf_set_fence_fd(dmabuf, -1);
> + eglDestroySyncKHR(qemu_egl_display, sync);
> + qemu_dmabuf_set_sync(dmabuf, NULL);
> graphic_hw_gl_block(vc->gfx.dcl.con, false);
> }
> }
> @@ -682,6 +685,22 @@ static const DisplayGLCtxOps egl_ctx_ops = {
> static void gd_change_runstate(void *opaque, bool running, RunState state)
> {
> GtkDisplayState *s = opaque;
> + QemuDmaBuf *dmabuf;
> + int i;
> +
> + if (state == RUN_STATE_SAVE_VM) {
> + for (i = 0; i < s->nb_vcs; i++) {
> + VirtualConsole *vc = &s->vc[i];
> + dmabuf = vc->gfx.guest_fb.dmabuf;
> + if (dmabuf && qemu_dmabuf_get_fence_fd(dmabuf) >= 0) {
> + /* wait for the rendering to be completed */
> + eglClientWaitSync(qemu_egl_display,
> + qemu_dmabuf_get_sync(dmabuf),
> + EGL_SYNC_FLUSH_COMMANDS_BIT_KHR,
> + 1000000000);
>
I don't think adding waiting points in the migration path is appropriate.
Perhaps once you explain the actual issue, it will be easier to help.
+ }
> + }
> + }
>
> gd_update_caption(s);
> }
> --
> 2.34.1
>
>
>
--
Marc-André Lureau
