[RFC PATCH 0/2] Enable additional display heads on guest launch

[RFC PATCH 0/2] Enable additional display heads on guest launch

[Damian Hobson-Garcia]

Hello all,

I would like to propose an option to enable multiple heads (outputs) on
startup when the "max_outputs" option is used on a vga device.
Currently, only 1 head is enabled by default, and all others are
enabled in response to commands from the host side UI interface.
There is also an issue on gitlab that describes this situation
(https://gitlab.com/qemu-project/qemu/-/issues/1107)

The main application that I am thinking of is for running headless
for CI testing of Yocto images.  In this configuration, it can be
sufficient to grab a screenshot of the display from within the guest
for testing purposes.  To do this, however, the DRM connectors must
appear to be connected in the guest.

For now, I've just implemented a simple option that will enable all
of the heads on a virtio-vga/gpu device as an illustration, and was
hoping to get some feedback on the concept.  If there is agreement
I can extend this out to qxl as well. I have tested this with
the following -display settings: none, gtk, sdl, vnc.  

Thank you,
Damian

Damian Hobson-Garcia (2):
  gtk: Make sure widget is realized before updating
  virtio-gpu: Add an option to connect all outputs on startup

 hw/display/virtio-gpu-base.c   | 12 +++++++++---
 include/hw/virtio/virtio-gpu.h |  7 ++++++-
 ui/gtk.c                       |  4 ++++
 3 files changed, 19 insertions(+), 4 deletions(-)

-- 
2.25.1

[RFC PATCH 1/2] gtk: Make sure widget is realized before updating

[Damian Hobson-Garcia]

Check that a widget has a window before trying
to update its contents.
---
 ui/gtk.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/ui/gtk.c b/ui/gtk.c
index fd82e9b1ca..e4e0980323 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -340,6 +340,10 @@ static void gd_update_full_redraw(VirtualConsole *vc)
 {
     GtkWidget *area = vc->gfx.drawing_area;
     int ww, wh;
+
+    if (!gtk_widget_get_realized(area)) {
+        return;
+    }
     ww = gdk_window_get_width(gtk_widget_get_window(area));
     wh = gdk_window_get_height(gtk_widget_get_window(area));
 #if defined(CONFIG_OPENGL)
-- 
2.25.1

Re: [RFC PATCH 1/2] gtk: Make sure widget is realized before updating

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 832 bytes --]

On Wed, Mar 8, 2023 at 8:26 PM Damian Hobson-Garcia <dhobsong@igel.co.jp>
wrote:

> Check that a widget has a window before trying
> to update its contents.
> ---
>  ui/gtk.c | 4 ++++
>  1 file changed, 4 insertions(+)
>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>



>
> diff --git a/ui/gtk.c b/ui/gtk.c
> index fd82e9b1ca..e4e0980323 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -340,6 +340,10 @@ static void gd_update_full_redraw(VirtualConsole *vc)
>  {
>      GtkWidget *area = vc->gfx.drawing_area;
>      int ww, wh;
> +
> +    if (!gtk_widget_get_realized(area)) {
> +        return;
> +    }
>      ww = gdk_window_get_width(gtk_widget_get_window(area));
>      wh = gdk_window_get_height(gtk_widget_get_window(area));
>  #if defined(CONFIG_OPENGL)
> --
> 2.25.1
>
>

[-- Attachment #2: Type: text/html, Size: 1422 bytes --]

[RFC PATCH 2/2] virtio-gpu: Add an option to connect all outputs on startup

[Damian Hobson-Garcia]

When multiple outputs are enabled using the "max_outputs" attribute,
only the first connector appears as "Connected" in the guest DRM
device.  Additional connectors must be enabled from the host side
UI frontend before they are usable by the guest.  However, multiple
outputs can still be of use on a headless configuration, if for example,
the display will only be used for taking periodic screenshots in the
guest or integration into a CI pipeline, etc.

Add an option to start all of the outputs in the "Connected" state,
so that they are immediately available to the guest.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1107
Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>
---
 hw/display/virtio-gpu-base.c   | 12 +++++++++---
 include/hw/virtio/virtio-gpu.h |  7 ++++++-
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index a29f191aa8..885184e302 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -158,6 +158,7 @@ virtio_gpu_base_device_realize(DeviceState *qdev,
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
     VirtIOGPUBase *g = VIRTIO_GPU_BASE(qdev);
+    int connected = 1;
     int i;
 
     if (g->conf.max_outputs > VIRTIO_GPU_MAX_SCANOUTS) {
@@ -186,10 +187,15 @@ virtio_gpu_base_device_realize(DeviceState *qdev,
         virtio_add_queue(vdev, 16, cursor_cb);
     }
 
-    g->enabled_output_bitmask = 1;
+    if (virtio_gpu_connect_all_outputs(g->conf))
+        connected = g->conf.max_outputs;
 
-    g->req_state[0].width = g->conf.xres;
-    g->req_state[0].height = g->conf.yres;
+    g->enabled_output_bitmask = (1 << connected) - 1;
+
+    for (i = 0; i < connected; i++) {
+        g->req_state[i].width = g->conf.xres;
+        g->req_state[i].height = g->conf.yres;
+    }
 
     g->hw_ops = &virtio_gpu_ops;
     for (i = 0; i < g->conf.max_outputs; i++) {
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 2e28507efe..54b3eba632 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -90,6 +90,7 @@ enum virtio_gpu_base_conf_flags {
     VIRTIO_GPU_FLAG_EDID_ENABLED,
     VIRTIO_GPU_FLAG_DMABUF_ENABLED,
     VIRTIO_GPU_FLAG_BLOB_ENABLED,
+    VIRTIO_GPU_FLAG_CONNECT_ALL_OUTPUTS,
 };
 
 #define virtio_gpu_virgl_enabled(_cfg) \
@@ -102,6 +103,8 @@ enum virtio_gpu_base_conf_flags {
     (_cfg.flags & (1 << VIRTIO_GPU_FLAG_DMABUF_ENABLED))
 #define virtio_gpu_blob_enabled(_cfg) \
     (_cfg.flags & (1 << VIRTIO_GPU_FLAG_BLOB_ENABLED))
+#define virtio_gpu_connect_all_outputs(_cfg) \
+    (_cfg.flags & (1 << VIRTIO_GPU_FLAG_CONNECT_ALL_OUTPUTS))
 
 struct virtio_gpu_base_conf {
     uint32_t max_outputs;
@@ -148,7 +151,9 @@ struct VirtIOGPUBaseClass {
     DEFINE_PROP_BIT("edid", _state, _conf.flags, \
                     VIRTIO_GPU_FLAG_EDID_ENABLED, true), \
     DEFINE_PROP_UINT32("xres", _state, _conf.xres, 1280), \
-    DEFINE_PROP_UINT32("yres", _state, _conf.yres, 800)
+    DEFINE_PROP_UINT32("yres", _state, _conf.yres, 800), \
+    DEFINE_PROP_BIT("connect_outputs", _state, _conf.flags, \
+                    VIRTIO_GPU_FLAG_CONNECT_ALL_OUTPUTS, false)
 
 typedef struct VGPUDMABuf {
     QemuDmaBuf buf;
-- 
2.25.1

Re: [RFC PATCH 2/2] virtio-gpu: Add an option to connect all outputs on startup

[Damian Hobson-Garcia]

On 2023/03/08 11:25, Damian Hobson-Garcia wrote:
> When multiple outputs are enabled using the "max_outputs" attribute,
> only the first connector appears as "Connected" in the guest DRM
> device.  Additional connectors must be enabled from the host side
> UI frontend before they are usable by the guest.  However, multiple
> outputs can still be of use on a headless configuration, if for example,
> the display will only be used for taking periodic screenshots in the
> guest or integration into a CI pipeline, etc.
> 
> Add an option to start all of the outputs in the "Connected" state,
> so that they are immediately available to the guest.

I was also thinking that since the main application for this is headless 
configurations, it could be possible to implement this in
ui/egl-headless.c.

Calling dpy_set_ui_info() for each console during initialization is 
enough to enable the outputs, but then there is the question of what 
resolution to pass in the ui_info. Hardcoded? Add a new option to the 
`egl-headless` display property?

Are there any thoughts on this approach vs. addressing this at the 
display driver (virtio-gpu) level?

Thank you,
Damian