| Message ID | 20231213202419.15459-2-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 Thu, Dec 14, 2023 at 8:26 AM Dongwon Kim <dongwon.kim@intel.com> wrote: > > It is needed to unblock the pipeline only if there is an active dmabuf > to be rendered and the fence for it is not yet signaled. > > 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 | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/ui/gtk.c b/ui/gtk.c > index ea8d07833e..073c9eadb8 100644 > --- a/ui/gtk.c > +++ b/ui/gtk.c > @@ -597,10 +597,16 @@ void gd_hw_gl_flushed(void *vcon) > VirtualConsole *vc = vcon; > QemuDmaBuf *dmabuf = vc->gfx.guest_fb.dmabuf; > > - qemu_set_fd_handler(dmabuf->fence_fd, NULL, NULL, NULL); > - close(dmabuf->fence_fd); > - dmabuf->fence_fd = -1; > - graphic_hw_gl_block(vc->gfx.dcl.con, false); > + if (!dmabuf) { > + return; > + } When is this function called with dmabuf == NULL or fence_fd < 0? > + > + if (dmabuf->fence_fd > 0) { this should be >= 0 > + qemu_set_fd_handler(dmabuf->fence_fd, NULL, NULL, NULL); > + close(dmabuf->fence_fd); > + dmabuf->fence_fd = -1; > + graphic_hw_gl_block(vc->gfx.dcl.con, false); > + } > } > > /** DisplayState Callbacks (opengl version) **/ > -- > 2.34.1 > >
On Fri, 15 Dec 2023, Marc-André Lureau wrote: > Hi > > On Thu, Dec 14, 2023 at 8:26 AM Dongwon Kim <dongwon.kim@intel.com> wrote: >> >> It is needed to unblock the pipeline only if there is an active dmabuf >> to be rendered and the fence for it is not yet signaled. >> >> 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 | 14 ++++++++++---- >> 1 file changed, 10 insertions(+), 4 deletions(-) >> >> diff --git a/ui/gtk.c b/ui/gtk.c >> index ea8d07833e..073c9eadb8 100644 >> --- a/ui/gtk.c >> +++ b/ui/gtk.c >> @@ -597,10 +597,16 @@ void gd_hw_gl_flushed(void *vcon) >> VirtualConsole *vc = vcon; >> QemuDmaBuf *dmabuf = vc->gfx.guest_fb.dmabuf; >> >> - qemu_set_fd_handler(dmabuf->fence_fd, NULL, NULL, NULL); >> - close(dmabuf->fence_fd); >> - dmabuf->fence_fd = -1; >> - graphic_hw_gl_block(vc->gfx.dcl.con, false); >> + if (!dmabuf) { >> + return; >> + } > > When is this function called with dmabuf == NULL or fence_fd < 0? > >> + >> + if (dmabuf->fence_fd > 0) { > > this should be >= 0 There's another in line 102 already and one in gtk-gl-area.c too. Regards, BALATON Zoltan >> + qemu_set_fd_handler(dmabuf->fence_fd, NULL, NULL, NULL); >> + close(dmabuf->fence_fd); >> + dmabuf->fence_fd = -1; >> + graphic_hw_gl_block(vc->gfx.dcl.con, false); >> + } >> } >> >> /** DisplayState Callbacks (opengl version) **/ >> -- >> 2.34.1 >> >> > > >
Hi Marc-André, I reviewed and realized these conditions won't be met in normal situations in given upstream code. But we've initially added those conditions in our internal code base for dev because we often had to call gd_hw_gl_flushed to forcefully unblock from HPD code (i.e. 'connectors' param. Not Upstreamed yet) when VM display is disconnected. In such cases, it is needed to make sure there is a frame in the pipeline already. Anyway, I think we can check dmabuf==NULL and fence_fd < 0 before calling gd_hw_flushed in HPD code just as in gd_change_runstate ([PATCH 1/3] ui/gtk: flush display pipeline before saving vmstate when blob=true). > Subject: Re: [PATCH 2/3] ui/gtk: unblock pipeline only if fence hasn't been > signaled yet > > Hi > > On Thu, Dec 14, 2023 at 8:26 AM Dongwon Kim <dongwon.kim@intel.com> > wrote: > > > > It is needed to unblock the pipeline only if there is an active dmabuf > > to be rendered and the fence for it is not yet signaled. > > > > 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 | 14 ++++++++++---- > > 1 file changed, 10 insertions(+), 4 deletions(-) > > > > diff --git a/ui/gtk.c b/ui/gtk.c > > index ea8d07833e..073c9eadb8 100644 > > --- a/ui/gtk.c > > +++ b/ui/gtk.c > > @@ -597,10 +597,16 @@ void gd_hw_gl_flushed(void *vcon) > > VirtualConsole *vc = vcon; > > QemuDmaBuf *dmabuf = vc->gfx.guest_fb.dmabuf; > > > > - qemu_set_fd_handler(dmabuf->fence_fd, NULL, NULL, NULL); > > - close(dmabuf->fence_fd); > > - dmabuf->fence_fd = -1; > > - graphic_hw_gl_block(vc->gfx.dcl.con, false); > > + if (!dmabuf) { > > + return; > > + } > > When is this function called with dmabuf == NULL or fence_fd < 0? > > > + > > + if (dmabuf->fence_fd > 0) { > > this should be >= 0 > > > + qemu_set_fd_handler(dmabuf->fence_fd, NULL, NULL, NULL); > > + close(dmabuf->fence_fd); > > + dmabuf->fence_fd = -1; > > + graphic_hw_gl_block(vc->gfx.dcl.con, false); > > + } > > } > > > > /** DisplayState Callbacks (opengl version) **/ > > -- > > 2.34.1 > > > > > > > -- > Marc-André Lureau
diff --git a/ui/gtk.c b/ui/gtk.c index ea8d07833e..073c9eadb8 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -597,10 +597,16 @@ void gd_hw_gl_flushed(void *vcon) VirtualConsole *vc = vcon; QemuDmaBuf *dmabuf = vc->gfx.guest_fb.dmabuf; - qemu_set_fd_handler(dmabuf->fence_fd, NULL, NULL, NULL); - close(dmabuf->fence_fd); - dmabuf->fence_fd = -1; - graphic_hw_gl_block(vc->gfx.dcl.con, false); + if (!dmabuf) { + return; + } + + if (dmabuf->fence_fd > 0) { + qemu_set_fd_handler(dmabuf->fence_fd, NULL, NULL, NULL); + close(dmabuf->fence_fd); + dmabuf->fence_fd = -1; + graphic_hw_gl_block(vc->gfx.dcl.con, false); + } } /** DisplayState Callbacks (opengl version) **/
It is needed to unblock the pipeline only if there is an active dmabuf to be rendered and the fence for it is not yet signaled. 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 | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)