| Message ID | 20231213202419.15459-1-dongwon.kim@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] ui/gtk: flush display pipeline before saving vmstate when blob=true | expand |
Hi On Thu, Dec 14, 2023 at 8:26 AM Dongwon Kim <dongwon.kim@intel.com> wrote: > > If the guest state is paused before it gets a response for the current > scanout frame submission (resource-flush), it won't flush new frames > after being restored as it still waits for the old response, which is > accepted as a scanout render done signal. So it's needed to unblock > the current scanout render pipeline before the run state is changed > to make sure the guest receives the response for the current frame > submission. > > v2: Giving some time for the fence to be signaled before flushing > the pipeline > > Cc: Marc-André Lureau <marcandre.lureau@redhat.com> > Cc: Vivek Kasireddy <vivek.kasireddy@intel.com> > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com> > --- > ui/gtk.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/ui/gtk.c b/ui/gtk.c > index 810d7fc796..ea8d07833e 100644 > --- a/ui/gtk.c > +++ b/ui/gtk.c > @@ -678,6 +678,25 @@ static const DisplayGLCtxOps egl_ctx_ops = { > static void gd_change_runstate(void *opaque, bool running, RunState state) > { > GtkDisplayState *s = opaque; > + int i; > + > + if (state == RUN_STATE_SAVE_VM) { > + for (i = 0; i < s->nb_vcs; i++) { > + VirtualConsole *vc = &s->vc[i]; > + > + if (vc->gfx.guest_fb.dmabuf && > + vc->gfx.guest_fb.dmabuf->fence_fd >= 0) { > + eglClientWaitSync(qemu_egl_display, > + vc->gfx.guest_fb.dmabuf->sync, > + EGL_SYNC_FLUSH_COMMANDS_BIT_KHR, > + 100000000); This won't work. dmabuf->sync is NULL after egl_dmabuf_create_sync. I will let Vivek, who wrote the sync code, comment. thanks
Hi, > > On Thu, Dec 14, 2023 at 8:26 AM Dongwon Kim <dongwon.kim@intel.com> > wrote: > > > > If the guest state is paused before it gets a response for the current > > scanout frame submission (resource-flush), it won't flush new frames > > after being restored as it still waits for the old response, which is > > accepted as a scanout render done signal. So it's needed to unblock > > the current scanout render pipeline before the run state is changed > > to make sure the guest receives the response for the current frame > > submission. > > > > v2: Giving some time for the fence to be signaled before flushing > > the pipeline > > > > Cc: Marc-André Lureau <marcandre.lureau@redhat.com> > > Cc: Vivek Kasireddy <vivek.kasireddy@intel.com> > > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com> > > --- > > ui/gtk.c | 19 +++++++++++++++++++ > > 1 file changed, 19 insertions(+) > > > > diff --git a/ui/gtk.c b/ui/gtk.c > > index 810d7fc796..ea8d07833e 100644 > > --- a/ui/gtk.c > > +++ b/ui/gtk.c > > @@ -678,6 +678,25 @@ static const DisplayGLCtxOps egl_ctx_ops = { > > static void gd_change_runstate(void *opaque, bool running, RunState > state) > > { > > GtkDisplayState *s = opaque; > > + int i; > > + > > + if (state == RUN_STATE_SAVE_VM) { > > + for (i = 0; i < s->nb_vcs; i++) { > > + VirtualConsole *vc = &s->vc[i]; > > + > > + if (vc->gfx.guest_fb.dmabuf && > > + vc->gfx.guest_fb.dmabuf->fence_fd >= 0) { > > + eglClientWaitSync(qemu_egl_display, > > + vc->gfx.guest_fb.dmabuf->sync, > > + EGL_SYNC_FLUSH_COMMANDS_BIT_KHR, > > + 100000000); > > This won't work. dmabuf->sync is NULL after egl_dmabuf_create_sync. Right, we destroy the sync object after we create the fence from it. If you want to use eglClientWaitSync() here, you either need to recreate the sync object using fence_fd or don't destroy it in egl_dmabuf_create_fence(). Either way should be ok but make sure you destroy it when the fence_fd is closed. Thanks, Vivek > > I will let Vivek, who wrote the sync code, comment. > > thanks > > > > -- > Marc-André Lureau
Hi, > Subject: RE: [PATCH 1/3] ui/gtk: flush display pipeline before saving vmstate > when blob=true > > Hi, > > > > > On Thu, Dec 14, 2023 at 8:26 AM Dongwon Kim <dongwon.kim@intel.com> > > wrote: > > > > > > If the guest state is paused before it gets a response for the > > > current scanout frame submission (resource-flush), it won't flush > > > new frames after being restored as it still waits for the old > > > response, which is accepted as a scanout render done signal. So it's > > > needed to unblock the current scanout render pipeline before the run > > > state is changed to make sure the guest receives the response for > > > the current frame submission. > > > > > > v2: Giving some time for the fence to be signaled before flushing > > > the pipeline > > > > > > Cc: Marc-André Lureau <marcandre.lureau@redhat.com> > > > Cc: Vivek Kasireddy <vivek.kasireddy@intel.com> > > > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com> > > > --- > > > ui/gtk.c | 19 +++++++++++++++++++ > > > 1 file changed, 19 insertions(+) > > > > > > diff --git a/ui/gtk.c b/ui/gtk.c > > > index 810d7fc796..ea8d07833e 100644 > > > --- a/ui/gtk.c > > > +++ b/ui/gtk.c > > > @@ -678,6 +678,25 @@ static const DisplayGLCtxOps egl_ctx_ops = { > > > static void gd_change_runstate(void *opaque, bool running, RunState > > state) > > > { > > > GtkDisplayState *s = opaque; > > > + int i; > > > + > > > + if (state == RUN_STATE_SAVE_VM) { > > > + for (i = 0; i < s->nb_vcs; i++) { > > > + VirtualConsole *vc = &s->vc[i]; > > > + > > > + if (vc->gfx.guest_fb.dmabuf && > > > + vc->gfx.guest_fb.dmabuf->fence_fd >= 0) { > > > + eglClientWaitSync(qemu_egl_display, > > > + vc->gfx.guest_fb.dmabuf->sync, > > > + EGL_SYNC_FLUSH_COMMANDS_BIT_KHR, > > > + 100000000); > > > > This won't work. dmabuf->sync is NULL after egl_dmabuf_create_sync. > Right, we destroy the sync object after we create the fence from it. If you want > to use eglClientWaitSync() here, you either need to recreate the sync object > using fence_fd or don't destroy it in egl_dmabuf_create_fence(). > Either way should be ok but make sure you destroy it when the fence_fd is > closed. I guess it makes sense to destroy sync object inside gd_hw_gl_flushed if that is the case. Another thing is I mentioned that "dmabuf == NULL or fence_fd < 0" doesn't seem to be checked in gd_hw_flushed in my previous reply but now I am thinking it is needed because gd_hw_gl_flushed can be called twice with given code change - once when rendering is done and the fence is signaled during eglClientWaitSync and second after eglClientWaitSync. I will come up with a new version of patches. > > Thanks, > Vivek > > > > > I will let Vivek, who wrote the sync code, comment. > > > > thanks > > > > > > > > -- > > Marc-André Lureau
diff --git a/ui/gtk.c b/ui/gtk.c index 810d7fc796..ea8d07833e 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -678,6 +678,25 @@ static const DisplayGLCtxOps egl_ctx_ops = { static void gd_change_runstate(void *opaque, bool running, RunState state) { GtkDisplayState *s = opaque; + int i; + + if (state == RUN_STATE_SAVE_VM) { + for (i = 0; i < s->nb_vcs; i++) { + VirtualConsole *vc = &s->vc[i]; + + if (vc->gfx.guest_fb.dmabuf && + vc->gfx.guest_fb.dmabuf->fence_fd >= 0) { + eglClientWaitSync(qemu_egl_display, + vc->gfx.guest_fb.dmabuf->sync, + EGL_SYNC_FLUSH_COMMANDS_BIT_KHR, + 100000000); + + /* force flushing current scanout blob rendering process + * just in case the fence is still not signaled */ + gd_hw_gl_flushed(vc); + } + } + } gd_update_caption(s); }
If the guest state is paused before it gets a response for the current scanout frame submission (resource-flush), it won't flush new frames after being restored as it still waits for the old response, which is accepted as a scanout render done signal. So it's needed to unblock the current scanout render pipeline before the run state is changed to make sure the guest receives the response for the current frame submission. v2: Giving some time for the fence to be signaled before flushing the pipeline Cc: Marc-André Lureau <marcandre.lureau@redhat.com> Cc: Vivek Kasireddy <vivek.kasireddy@intel.com> Signed-off-by: Dongwon Kim <dongwon.kim@intel.com> --- ui/gtk.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)