| Message ID | 20231204184051.16873-1-dongwon.kim@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ui/gtk: flush display pipeline before saving vmstate when blob=true | expand |
Hi On Tue, Dec 5, 2023 at 6:40 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 start submitting > 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. > > 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 | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/ui/gtk.c b/ui/gtk.c > index 810d7fc796..0f6237dd2f 100644 > --- a/ui/gtk.c > +++ b/ui/gtk.c > @@ -678,6 +678,18 @@ 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) { && ..dmabuf->fence_fd >= 0 > + /* force flushing current scanout blob rendering process */ > + gd_hw_gl_flushed(vc); This defeats the purpose of the fence, maybe we should wait for it to be signaled first. At worse, I suppose the client can have some glitches. Although since the guest is stopped, this is unlikely.
Hi Marc-André, > -----Original Message----- > From: Marc-André Lureau <marcandre.lureau@redhat.com> > Sent: Monday, December 4, 2023 11:15 PM > To: Kim, Dongwon <dongwon.kim@intel.com> > Cc: qemu-devel@nongnu.org; Kasireddy, Vivek <vivek.kasireddy@intel.com> > Subject: Re: [PATCH] ui/gtk: flush display pipeline before saving vmstate when > blob=true > > Hi > > On Tue, Dec 5, 2023 at 6:40 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 start submitting > > 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. > > > > 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 | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/ui/gtk.c b/ui/gtk.c > > index 810d7fc796..0f6237dd2f 100644 > > --- a/ui/gtk.c > > +++ b/ui/gtk.c > > @@ -678,6 +678,18 @@ 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) { > > && ..dmabuf->fence_fd >= 0 > > > + /* force flushing current scanout blob rendering process */ > > + gd_hw_gl_flushed(vc); > > This defeats the purpose of the fence, maybe we should wait for it to be > signaled first. At worse, I suppose the client can have some glitches. Although > since the guest is stopped, this is unlikely. [Kim, Dongwon] So this is the flow you are referring to? if (vc->gfx.guest_fb.dmabuf && vc->gfx.guest_fb.dmabuf->fence_fd >= 0) { EGLint ret = eglClientWaitSync(qemu_egl_display, vc->gfx.guest_fb.dmabuf->sync, EGL_SYNC_FLUSH_COMMANDS_BIT_KHR, 100000000); /* timeout of 100ms */ if (ret != EGL_CONDITION_SATISFIED_KHR) { /* force flushing current scanout blob rendering process */ gd_hw_gl_flushed(vc); } } If yes, I will test this then create v2 based on this flow. Thanks
Hi On Wed, Dec 6, 2023 at 2:05 AM Kim, Dongwon <dongwon.kim@intel.com> wrote: > > Hi Marc-André, > > > -----Original Message----- > > From: Marc-André Lureau <marcandre.lureau@redhat.com> > > Sent: Monday, December 4, 2023 11:15 PM > > To: Kim, Dongwon <dongwon.kim@intel.com> > > Cc: qemu-devel@nongnu.org; Kasireddy, Vivek <vivek.kasireddy@intel.com> > > Subject: Re: [PATCH] ui/gtk: flush display pipeline before saving vmstate when > > blob=true > > > > Hi > > > > On Tue, Dec 5, 2023 at 6:40 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 start submitting > > > 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. > > > > > > 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 | 12 ++++++++++++ > > > 1 file changed, 12 insertions(+) > > > > > > diff --git a/ui/gtk.c b/ui/gtk.c > > > index 810d7fc796..0f6237dd2f 100644 > > > --- a/ui/gtk.c > > > +++ b/ui/gtk.c > > > @@ -678,6 +678,18 @@ 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) { > > > > && ..dmabuf->fence_fd >= 0 > > > > > + /* force flushing current scanout blob rendering process */ > > > + gd_hw_gl_flushed(vc); > > > > This defeats the purpose of the fence, maybe we should wait for it to be > > signaled first. At worse, I suppose the client can have some glitches. Although > > since the guest is stopped, this is unlikely. > [Kim, Dongwon] > So this is the flow you are referring to? > > if (vc->gfx.guest_fb.dmabuf && > vc->gfx.guest_fb.dmabuf->fence_fd >= 0) { > EGLint ret = eglClientWaitSync(qemu_egl_display, > vc->gfx.guest_fb.dmabuf->sync, > EGL_SYNC_FLUSH_COMMANDS_BIT_KHR, > 100000000); /* timeout of 100ms */ > > if (ret != EGL_CONDITION_SATISFIED_KHR) { > /* force flushing current scanout blob rendering process */ > gd_hw_gl_flushed(vc); > } > } > > If yes, I will test this then create v2 based on this flow. Yes, you may want to call gd_hw_gl_flushed() even on success, to avoid waiting for the main loop dispatch.
diff --git a/ui/gtk.c b/ui/gtk.c index 810d7fc796..0f6237dd2f 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -678,6 +678,18 @@ 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) { + /* force flushing current scanout blob rendering process */ + 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 start submitting 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. 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 | 12 ++++++++++++ 1 file changed, 12 insertions(+)