diff mbox series

[PULL,04/19] virtio-gpu: replace the surface with null surface when resetting

Message ID 20230717124545.177236-5-marcandre.lureau@redhat.com
State New
Headers show
Series [PULL,01/19] virtio-gpu: fix potential divide-by-zero regression | expand

Commit Message

Marc-André Lureau July 17, 2023, 12:45 p.m. UTC
From: Dongwon Kim <dongwon.kim@intel.com>

The primary guest scanout shows the booting screen right after reboot
but additional guest displays (i.e. max_ouptuts > 1) will keep displaying
the old frames until the guest virtio gpu driver gets initialized, which
could cause some confusion. A better way is to to replace the surface with
a place holder that tells the display is not active during the reset of
virtio-gpu device.

And to immediately update the surface with the place holder image after
the switch, displaychangelistener_gfx_switch needs to be called with
'update == TRUE' in dpy_gfx_replace_surface when the new surface is NULL.

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>
Acked-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-ID: <20230627224451.11739-1-dongwon.kim@intel.com>
---
 hw/display/virtio-gpu.c |  5 +++++
 ui/console.c            | 11 ++++++-----
 2 files changed, 11 insertions(+), 5 deletions(-)

Comments

Marc-André Lureau July 26, 2023, 2:19 p.m. UTC | #1
Hi Dongwon

On Mon, Jul 17, 2023 at 4:46 PM <marcandre.lureau@redhat.com> wrote:
>
> From: Dongwon Kim <dongwon.kim@intel.com>
>
> The primary guest scanout shows the booting screen right after reboot
> but additional guest displays (i.e. max_ouptuts > 1) will keep displaying
> the old frames until the guest virtio gpu driver gets initialized, which
> could cause some confusion. A better way is to to replace the surface with
> a place holder that tells the display is not active during the reset of
> virtio-gpu device.
>
> And to immediately update the surface with the place holder image after
> the switch, displaychangelistener_gfx_switch needs to be called with
> 'update == TRUE' in dpy_gfx_replace_surface when the new surface is NULL.
>
> 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>
> Acked-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Message-ID: <20230627224451.11739-1-dongwon.kim@intel.com>
> ---
>  hw/display/virtio-gpu.c |  5 +++++
>  ui/console.c            | 11 ++++++-----
>  2 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index e937c4e348..e8603d78ca 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -1397,6 +1397,7 @@ void virtio_gpu_reset(VirtIODevice *vdev)
>      VirtIOGPU *g = VIRTIO_GPU(vdev);
>      struct virtio_gpu_simple_resource *res, *tmp;
>      struct virtio_gpu_ctrl_command *cmd;
> +    int i = 0;
>
>      QTAILQ_FOREACH_SAFE(res, &g->reslist, next, tmp) {
>          virtio_gpu_resource_destroy(g, res);
> @@ -1415,6 +1416,10 @@ void virtio_gpu_reset(VirtIODevice *vdev)
>          g_free(cmd);
>      }
>
> +    for (i = 0; i < g->parent_obj.conf.max_outputs; i++) {
> +        dpy_gfx_replace_surface(g->parent_obj.scanout[i].con, NULL);

There is an issue with this change, it may end up calling GL functions
on non-main thread. This is not safe, and crashes at least on Windows,
because the GL contexts are differents (ANGLE with gl=es, OPENGL32 for
other threads), and in general I think we shouldn't use GL from
different threads. I am looking at fixing this somehow...

> +    }
> +
>      virtio_gpu_base_reset(VIRTIO_GPU_BASE(vdev));
>  }
>
> diff --git a/ui/console.c b/ui/console.c
> index c1544e0fb8..8da2170a7e 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -1898,6 +1898,7 @@ void dpy_gfx_replace_surface(QemuConsole *con,
>      static const char placeholder_msg[] = "Display output is not active.";
>      DisplayState *s = con->ds;
>      DisplaySurface *old_surface = con->surface;
> +    DisplaySurface *new_surface = surface;
>      DisplayChangeListener *dcl;
>      int width;
>      int height;
> @@ -1911,19 +1912,19 @@ void dpy_gfx_replace_surface(QemuConsole *con,
>              height = 480;
>          }
>
> -        surface = qemu_create_placeholder_surface(width, height, placeholder_msg);
> +        new_surface = qemu_create_placeholder_surface(width, height, placeholder_msg);
>      }
>
> -    assert(old_surface != surface);
> +    assert(old_surface != new_surface);
>
>      con->scanout.kind = SCANOUT_SURFACE;
> -    con->surface = surface;
> -    dpy_gfx_create_texture(con, surface);
> +    con->surface = new_surface;
> +    dpy_gfx_create_texture(con, new_surface);
>      QLIST_FOREACH(dcl, &s->listeners, next) {
>          if (con != (dcl->con ? dcl->con : active_console)) {
>              continue;
>          }
> -        displaychangelistener_gfx_switch(dcl, surface, FALSE);
> +        displaychangelistener_gfx_switch(dcl, new_surface, surface ? FALSE : TRUE);
>      }
>      dpy_gfx_destroy_texture(con, old_surface);
>      qemu_free_displaysurface(old_surface);
> --
> 2.41.0
>
>
diff mbox series

Patch

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index e937c4e348..e8603d78ca 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1397,6 +1397,7 @@  void virtio_gpu_reset(VirtIODevice *vdev)
     VirtIOGPU *g = VIRTIO_GPU(vdev);
     struct virtio_gpu_simple_resource *res, *tmp;
     struct virtio_gpu_ctrl_command *cmd;
+    int i = 0;
 
     QTAILQ_FOREACH_SAFE(res, &g->reslist, next, tmp) {
         virtio_gpu_resource_destroy(g, res);
@@ -1415,6 +1416,10 @@  void virtio_gpu_reset(VirtIODevice *vdev)
         g_free(cmd);
     }
 
+    for (i = 0; i < g->parent_obj.conf.max_outputs; i++) {
+        dpy_gfx_replace_surface(g->parent_obj.scanout[i].con, NULL);
+    }
+
     virtio_gpu_base_reset(VIRTIO_GPU_BASE(vdev));
 }
 
diff --git a/ui/console.c b/ui/console.c
index c1544e0fb8..8da2170a7e 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1898,6 +1898,7 @@  void dpy_gfx_replace_surface(QemuConsole *con,
     static const char placeholder_msg[] = "Display output is not active.";
     DisplayState *s = con->ds;
     DisplaySurface *old_surface = con->surface;
+    DisplaySurface *new_surface = surface;
     DisplayChangeListener *dcl;
     int width;
     int height;
@@ -1911,19 +1912,19 @@  void dpy_gfx_replace_surface(QemuConsole *con,
             height = 480;
         }
 
-        surface = qemu_create_placeholder_surface(width, height, placeholder_msg);
+        new_surface = qemu_create_placeholder_surface(width, height, placeholder_msg);
     }
 
-    assert(old_surface != surface);
+    assert(old_surface != new_surface);
 
     con->scanout.kind = SCANOUT_SURFACE;
-    con->surface = surface;
-    dpy_gfx_create_texture(con, surface);
+    con->surface = new_surface;
+    dpy_gfx_create_texture(con, new_surface);
     QLIST_FOREACH(dcl, &s->listeners, next) {
         if (con != (dcl->con ? dcl->con : active_console)) {
             continue;
         }
-        displaychangelistener_gfx_switch(dcl, surface, FALSE);
+        displaychangelistener_gfx_switch(dcl, new_surface, surface ? FALSE : TRUE);
     }
     dpy_gfx_destroy_texture(con, old_surface);
     qemu_free_displaysurface(old_surface);