RE: [PATCH 1/3] ui/gtk: flush display pipeline before saving vmstate when blob=true

[Kasireddy, Vivek]

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