| Message ID | 20230627221139.11250-1-dongwon.kim@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | virtio-gpu: do not replace surface when scanout is disabled | expand |
Hi On Wed, Jun 28, 2023 at 12:32 AM Dongwon Kim <dongwon.kim@intel.com> wrote: > Surface is replaced with a place holder whenever the surface res > is unreferenced by the guest message. With this logic, there is > very frequent switching between guest display and the place holder > image, which is looking like a flickering display if the guest driver > is designed to unref the current scanout resource before sending out > a new scanout resource. So it is better to leave the current scanout > image until there is a new one flushed by the guest. > > Cc: Gerd Hoffmann <kraxel@redhat.com> > Cc: Marc-André Lureau <marcandre.lureau@redhat.com> > Cc: Vivek Kasireddy <vivek.kasireddy@intel.com> > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com> > Why is the driver not setting a different scanout before destroying the resource? I think it's wrong to not replace the surface, as the associated scanout resource may be destroyed or explicitly disabled for various purposes, and we don't want to display garbage either. --- > hw/display/virtio-gpu.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c > index 66cddd94d9..9d3e922c8f 100644 > --- a/hw/display/virtio-gpu.c > +++ b/hw/display/virtio-gpu.c > @@ -387,7 +387,6 @@ static void virtio_gpu_disable_scanout(VirtIOGPU *g, > int scanout_id) > res->scanout_bitmask &= ~(1 << scanout_id); > } > > - dpy_gfx_replace_surface(scanout->con, NULL); > scanout->resource_id = 0; > scanout->ds = NULL; > scanout->width = 0; > -- > 2.34.1 > > >
On 7/4/2023 8:12 AM, Marc-André Lureau wrote: > Hi > > On Wed, Jun 28, 2023 at 12:32 AM Dongwon Kim <dongwon.kim@intel.com> > wrote: > > Surface is replaced with a place holder whenever the surface res > is unreferenced by the guest message. With this logic, there is > very frequent switching between guest display and the place holder > image, which is looking like a flickering display if the guest driver > is designed to unref the current scanout resource before sending out > a new scanout resource. So it is better to leave the current scanout > image until there is a new one flushed by the guest. > > Cc: Gerd Hoffmann <kraxel@redhat.com> > Cc: Marc-André Lureau <marcandre.lureau@redhat.com> > Cc: Vivek Kasireddy <vivek.kasireddy@intel.com> > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com> > > > Why is the driver not setting a different scanout before destroying > the resource? > > I think it's wrong to not replace the surface, as the associated > scanout resource may be destroyed or explicitly disabled for various > purposes, and we don't want to display garbage either. Yeah..I got your point. This is to address very specific to our use-case with windows guest that runs virtio-gpu like driver that does unref before the next framebuffer is set. And I agree that this sequence doesn't look right. Let me check if we could change the sequence in the guest driver. > > --- > hw/display/virtio-gpu.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c > index 66cddd94d9..9d3e922c8f 100644 > --- a/hw/display/virtio-gpu.c > +++ b/hw/display/virtio-gpu.c > @@ -387,7 +387,6 @@ static void > virtio_gpu_disable_scanout(VirtIOGPU *g, int scanout_id) > res->scanout_bitmask &= ~(1 << scanout_id); > } > > - dpy_gfx_replace_surface(scanout->con, NULL); > scanout->resource_id = 0; > scanout->ds = NULL; > scanout->width = 0; > -- > 2.34.1 > > > > > -- > Marc-André Lureau
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c index 66cddd94d9..9d3e922c8f 100644 --- a/hw/display/virtio-gpu.c +++ b/hw/display/virtio-gpu.c @@ -387,7 +387,6 @@ static void virtio_gpu_disable_scanout(VirtIOGPU *g, int scanout_id) res->scanout_bitmask &= ~(1 << scanout_id); } - dpy_gfx_replace_surface(scanout->con, NULL); scanout->resource_id = 0; scanout->ds = NULL; scanout->width = 0;
Surface is replaced with a place holder whenever the surface res is unreferenced by the guest message. With this logic, there is very frequent switching between guest display and the place holder image, which is looking like a flickering display if the guest driver is designed to unref the current scanout resource before sending out a new scanout resource. So it is better to leave the current scanout image until there is a new one flushed by the guest. Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Marc-André Lureau <marcandre.lureau@redhat.com> Cc: Vivek Kasireddy <vivek.kasireddy@intel.com> Signed-off-by: Dongwon Kim <dongwon.kim@intel.com> --- hw/display/virtio-gpu.c | 1 - 1 file changed, 1 deletion(-)