[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] ui/gtk: flush display pipeline before saving vmstate when bl
From: |
Marc-André Lureau |
Subject: |
Re: [PATCH] ui/gtk: flush display pipeline before saving vmstate when blob=true |
Date: |
Wed, 6 Dec 2023 11:02:03 +0400 |
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.