[PATCH v2 1/5] virtio-gpu: Support the configurable EDID fields

[PATCH v2 1/5] virtio-gpu: Support the configurable EDID fields

[Roland Clobus]

Adds support for the fields that are currently configurable
in the function qemu_edid_generate.

* The serial number does not need to be numerical.
See https://en.wikipedia.org/wiki/Extended_Display_Identification_Data

* The name can be optional, there are now many more fields to set

Signed-off-by: Roland Clobus <rclobus@rclobus.nl>
---

Note: I've not added the struct with 2 elements for bundle the x and y
versions, I think it is easier to keep the flat structure.

Note: I've kept the xres and yres as 32bit, because a lot of the code
works with uint32_t, even though at the end only 16 bits are used.

 hw/display/edid-generate.c   |  1 +
 hw/display/virtio-gpu-base.c | 49 +++++++++++++++++++++++++++++++--
 qapi/virtio.json             | 52 +++++++++++++++++++++++++++++++-----
 3 files changed, 93 insertions(+), 9 deletions(-)

diff --git a/hw/display/edid-generate.c b/hw/display/edid-generate.c
index 2cb819675e..3d9b730c0f 100644
--- a/hw/display/edid-generate.c
+++ b/hw/display/edid-generate.c
@@ -392,6 +392,7 @@ void qemu_edid_generate(uint8_t *edid, size_t size,
     uint32_t large_screen = 0;
 
     /* =============== set defaults  =============== */
+    /* Note: keep the defaults in sync with qapi/virtio.json */
 
     if (!info->vendor || strlen(info->vendor) != 3) {
         info->vendor = "RHT";
diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index cb76302e2d..8389ecc20a 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -69,9 +69,47 @@ virtio_gpu_base_generate_edid(VirtIOGPUBase *g, int scanout,
 
     for (output_idx = 0, node = g->conf.outputs;
          output_idx <= scanout && node; output_idx++, node = node->next) {
-        if (output_idx == scanout && node->value && node->value->name) {
+        if (output_idx != scanout || !node->value) {
+            continue;
+        }
+
+        if (node->value->vendor) {
+            info.vendor = node->value->vendor;
+        }
+        if (node->value->name) {
             info.name = node->value->name;
-            break;
+        }
+        if (node->value->serial) {
+            info.serial = node->value->serial;
+        }
+        if (node->value->has_widthmm) {
+            info.width_mm = node->value->widthmm;
+        }
+        if (node->value->has_heightmm) {
+            info.height_mm = node->value->heightmm;
+        }
+        // Resolution change is special:
+        // The provided resolution will be used only once after it has been set.
+        // After that, the viewer has precedence over these settings
+        // and decides about the resolution.
+        if (node->value->has_xres) {
+            info.prefx = node->value->xres;
+            g->req_state[scanout].width = node->value->xres;
+            node->value->has_xres = 0;
+        }
+        if (node->value->has_yres) {
+            info.prefy = node->value->yres;
+            g->req_state[scanout].height = node->value->yres;
+            node->value->has_yres = 0;
+        }
+        if (node->value->has_xmax) {
+            info.maxx = node->value->xmax;
+        }
+        if (node->value->has_ymax) {
+            info.maxy = node->value->ymax;
+        }
+        if (node->value->has_refreshrate) {
+            info.refresh_rate = node->value->refreshrate;
         }
     }
 
@@ -236,6 +274,10 @@ virtio_gpu_base_device_realize(DeviceState *qdev,
     for (output_idx = 0, node = g->conf.outputs;
          node && output_idx < g->conf.max_outputs;
          output_idx++, node = node->next) {
+        if (node->value->has_widthmm && node->value->has_heightmm) {
+            g->req_state[output_idx].width_mm = node->value->widthmm;
+            g->req_state[output_idx].height_mm = node->value->heightmm;
+        }
         if (node->value->has_xres != node->value->has_yres) {
             error_setg(errp,
                        "must set both outputs[%zd].xres and outputs[%zd].yres",
@@ -247,6 +289,9 @@ virtio_gpu_base_device_realize(DeviceState *qdev,
             g->req_state[output_idx].width = node->value->xres;
             g->req_state[output_idx].height = node->value->yres;
         }
+        if (node->value->has_refreshrate) {
+            g->req_state[output_idx].refresh_rate = node->value->refreshrate;
+        }
     }
 
     g->hw_ops = &virtio_gpu_ops;
diff --git a/qapi/virtio.json b/qapi/virtio.json
index 09dd0e6d05..791d2f30f1 100644
--- a/qapi/virtio.json
+++ b/qapi/virtio.json
@@ -970,24 +970,62 @@
 ##
 # @VirtIOGPUOutput:
 #
-# Describes configuration of a VirtIO GPU output.  If both @xres and
-# @yres are set, they take precedence over root virtio-gpu resolution
-# configuration and enable the corresponding output.  If none of @xres
-# and @yres are set, root virtio-gpu resolution configuration takes
-# precedence and only the first output is enabled.  Only setting one
-# of @xres or @yres is an error.
+# Describes configuration of a VirtIO GPU output.  Matches the fields
+# of struct @qemu_edid_info.
+# If values are set, they take precedence over the root virtio-gpu
+# configuration.
+# If none of @xres and @yres are set, the root virtio-gpu resolution
+# configuration is used and only the first output is enabled.
+#
+# @vendor: a 3 letter code for the vendor (since xx.x TODO)
+#          Default: "RHT" (Red Hat, Inc.)
 #
 # @name: the name of the output
+#        Default: "QEMU Monitor"
+#
+# @serial: the serial number (since xx.x TODO)
+#
+# @widthmm: width of the display in mm (since xx.x TODO)
+#           If @widthmm is set, @heightmm must be set too
+#           If both @widthmm and @heightmm are unset,
+#             100 DPI is assumed
+#
+# @heightmm: height of the display in mm (since xx.x TODO)
+#           If @heightmm is set, @widthmm must be set too
 #
 # @xres: horizontal resolution of the output in pixels (since 11.0)
+#        If @xres is set, @yres must be set too
 #
 # @yres: vertical resolution of the output in pixels (since 11.0)
+#        If @yres is set, @xres must be set too
+#
+# @xmax: maximum horizontal resolution of the output in pixels
+#        If @xmax is set, @ymax must be set too
+#        (since xx.x TODO)
+#
+# @ymax: maximum vertical resolution of the output in pixels
+#        If @ymax is set, @xmax must be set too
+#        (since xx.x TODO)
+#
+# @refreshrate: refreshrate in mHz (since xx.x TODO)
+#               Default: 75000
+#
+# .. note:: keep the defaults in sync with hw/display/edid-generate.c
 #
 # Since: 10.1
 ##
 
 { 'struct': 'VirtIOGPUOutput',
-  'data': { 'name': 'str', '*xres': 'uint16', '*yres': 'uint16' } }
+  'data': { '*vendor': 'str',
+            '*name': 'str',
+            '*serial': 'str',
+            '*widthmm': 'uint16',
+            '*heightmm': 'uint16',
+            '*xres': 'uint32',
+            '*yres': 'uint32',
+            '*xmax': 'uint32',
+            '*ymax': 'uint32',
+            '*refreshrate': 'uint32' } }
 
 ##
 # @DummyVirtioForceArrays:
-- 
2.53.0

[PATCH v2 2/5] hw/core: validate VirtIOGPUOutputList properties

[Roland Clobus]

Validates the values for VirtIOGPUOutputList:
* string length and content
* when a pair of values need to be provided

Signed-off-by: Roland Clobus <rclobus@rclobus.nl>
---
 hw/core/qdev-properties-system.c | 63 ++++++++++++++++++++++++++++++++
 hw/display/virtio-gpu-base.c     | 23 ++----------
 2 files changed, 67 insertions(+), 19 deletions(-)

diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index a805ee2e1f..04e5acac08 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -33,6 +33,7 @@
 #include "system/block-backend.h"
 #include "system/blockdev.h"
 #include "net/net.h"
+#include "hw/display/edid.h"
 #include "hw/pci/pci.h"
 #include "hw/pci/pcie.h"
 #include "hw/i386/x86.h"
@@ -1348,6 +1349,68 @@ static void set_virtio_gpu_output_list(Object *obj, Visitor *v,
         return;
     }
 
+    if (*prop_ptr) {
+        size_t length_current = QAPI_LIST_LENGTH(*prop_ptr);
+        size_t length_new = QAPI_LIST_LENGTH(list);
+        if (length_current != length_new) {
+            error_setg(errp, "new list contains different number of outputs: %zd, old: %zd", length_new, length_current);
+            return;
+        }
+    }
+
+    size_t output_idx;
+    VirtIOGPUOutputList *node;
+    for (output_idx = 0, node = list;
+         node; output_idx++, node = node->next) {
+        if (!node->value) {
+            continue;
+        }
+        if (node->value->vendor) {
+            if (strlen(node->value->vendor) != 3) {
+                error_setg(errp, "invalid vendor length '%s' in outputs[%zd].vendor",
+                           node->value->vendor, output_idx);
+                return;
+            }
+            for (size_t i = 0; i < 3; ++i) {
+                if (node->value->vendor[i] < 'A' || node->value->vendor[i] > 'Z') {
+                    error_setg(errp, "invalid character '%c' in outputs[%zd].vendor '%s'",
+                               node->value->vendor[i], output_idx, node->value->vendor);
+                    return;
+                }
+            }
+        }
+        if (node->value->name &&
+            strlen(node->value->name) > EDID_NAME_MAX_LENGTH) {
+            error_setg(errp, "invalid output name '%s' > %d in outputs[%zd].name",
+                       node->value->name, EDID_NAME_MAX_LENGTH, output_idx);
+            return;
+        }
+        if (node->value->serial &&
+            strlen(node->value->serial) > EDID_NAME_MAX_LENGTH) {
+            error_setg(errp, "too long serial number '%s' > %d in outputs[%zd].serial",
+                       node->value->serial, EDID_NAME_MAX_LENGTH, output_idx);
+            return;
+        }
+        if (node->value->has_widthmm != node->value->has_heightmm) {
+            error_setg(errp,
+                       "must set both outputs[%zd].widthmm and outputs[%zd].heightmm",
+                       output_idx, output_idx);
+            return;
+        }
+        if (node->value->has_xres != node->value->has_yres) {
+            error_setg(errp,
+                       "must set both outputs[%zd].xres and outputs[%zd].yres",
+                       output_idx, output_idx);
+            return;
+        }
+        if (node->value->has_xmax != node->value->has_ymax) {
+            error_setg(errp,
+                       "must set both outputs[%zd].xmax and outputs[%zd].ymax",
+                       output_idx, output_idx);
+            return;
+        }
+    }
+
     qapi_free_VirtIOGPUOutputList(*prop_ptr);
     *prop_ptr = list;
 }
diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index 8389ecc20a..41f69a6ca7 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -225,25 +225,16 @@ virtio_gpu_base_device_realize(DeviceState *qdev,
     VirtIOGPUOutputList *node;
     VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
     VirtIOGPUBase *g = VIRTIO_GPU_BASE(qdev);
+    size_t outputlist_length = QAPI_LIST_LENGTH(g->conf.outputs);
     int i;
 
     if (g->conf.max_outputs > VIRTIO_GPU_MAX_SCANOUTS) {
         error_setg(errp, "invalid max_outputs > %d", VIRTIO_GPU_MAX_SCANOUTS);
         return false;
     }
-
-    for (output_idx = 0, node = g->conf.outputs;
-         node; output_idx++, node = node->next) {
-        if (output_idx == g->conf.max_outputs) {
-            error_setg(errp, "invalid outputs > %d", g->conf.max_outputs);
-            return false;
-        }
-        if (node->value && node->value->name &&
-            strlen(node->value->name) > EDID_NAME_MAX_LENGTH) {
-            error_setg(errp, "invalid output name '%s' > %d",
-                       node->value->name, EDID_NAME_MAX_LENGTH);
-            return false;
-        }
+    if (outputlist_length != 0 && outputlist_length != g->conf.max_outputs) {
+        error_setg(errp, "invalid number of outputs are defined, expected: %d, given: %zd", g->conf.max_outputs, outputlist_length);
+        return false;
     }
 
     if (virtio_gpu_virgl_enabled(g->conf)) {
@@ -278,12 +269,6 @@ virtio_gpu_base_device_realize(DeviceState *qdev,
             g->req_state[output_idx].width_mm = node->value->widthmm;
             g->req_state[output_idx].height_mm = node->value->heightmm;
         }
-        if (node->value->has_xres != node->value->has_yres) {
-            error_setg(errp,
-                       "must set both outputs[%zd].xres and outputs[%zd].yres",
-                       output_idx, output_idx);
-            return false;
-        }
         if (node->value->has_xres && node->value->has_yres) {
             g->enabled_output_bitmask |= (1 << output_idx);
             g->req_state[output_idx].width = node->value->xres;
-- 
2.53.0

[PATCH v2 3/5] virtio-gpu: Allow modifications to the outputs

[Roland Clobus]

Some systems need a stub for the event sender

Signed-off-by: Roland Clobus <rclobus@rclobus.nl>
---
 hw/core/meson.build              | 1 +
 hw/core/qdev-properties-system.c | 7 +++++++
 hw/core/virtio-gpu-stubs.c       | 6 ++++++
 3 files changed, 14 insertions(+)
 create mode 100644 hw/core/virtio-gpu-stubs.c

diff --git a/hw/core/meson.build b/hw/core/meson.build
index 8a96567de8..3e0d917d67 100644
--- a/hw/core/meson.build
+++ b/hw/core/meson.build
@@ -26,6 +26,7 @@ system_ss.add(when: 'CONFIG_SPLIT_IRQ', if_true: files('split-irq.c'))
 system_ss.add(when: 'CONFIG_XILINX_AXI', if_true: files('stream.c'))
 system_ss.add(when: 'CONFIG_PLATFORM_BUS', if_true: files('sysbus-fdt.c'))
 system_ss.add(when: 'CONFIG_EIF', if_true: [files('eif.c'), zlib, libcbor, gnutls])
+system_ss.add(when: 'CONFIG_VIRTIO_GPU', if_false: files('virtio-gpu-stubs.c'))
 
 system_ss.add(files(
   'cpu-system.c',
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 04e5acac08..79b6c27986 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -37,6 +37,8 @@
 #include "hw/pci/pci.h"
 #include "hw/pci/pcie.h"
 #include "hw/i386/x86.h"
+#include "hw/virtio/virtio-gpu.h"
+#include "standard-headers/linux/virtio_gpu.h"
 #include "util/block-helpers.h"
 
 static bool check_prop_still_unset(Object *obj, const char *name,
@@ -1413,6 +1415,10 @@ static void set_virtio_gpu_output_list(Object *obj, Visitor *v,
 
     qapi_free_VirtIOGPUOutputList(*prop_ptr);
     *prop_ptr = list;
+
+    VirtIOGPUBase *g = VIRTIO_GPU_BASE(obj);
+    g->virtio_config.events_read |= VIRTIO_GPU_EVENT_DISPLAY;
+    virtio_notify_config(&g->parent_obj);
 }
 
 static void release_virtio_gpu_output_list(Object *obj,
@@ -1428,6 +1434,7 @@ static void release_virtio_gpu_output_list(Object *obj,
 const PropertyInfo qdev_prop_virtio_gpu_output_list = {
     .type = "VirtIOGPUOutputList",
     .description = "VirtIO GPU output list [{\"name\":\"<name>\"},...]",
+    .realized_set_allowed = true,
     .get = get_virtio_gpu_output_list,
     .set = set_virtio_gpu_output_list,
     .release = release_virtio_gpu_output_list,
diff --git a/hw/core/virtio-gpu-stubs.c b/hw/core/virtio-gpu-stubs.c
new file mode 100644
index 0000000000..a3048a8d45
--- /dev/null
+++ b/hw/core/virtio-gpu-stubs.c
@@ -0,0 +1,6 @@
+typedef struct VirtIODevice VirtIODevice;
+
+void virtio_notify_config(VirtIODevice *);
+
+void virtio_notify_config(VirtIODevice *) {
+}
-- 
2.53.0

[PATCH v2 4/5] virtio-gpu: Allow the first display to be disabled

[Roland Clobus]

Signed-off-by: Roland Clobus <rclobus@rclobus.nl>
---

Tested with
-device '{"driver":"virtio-vga-gl", "max_outputs":2, "id":"vga001", "outputs":[ { "vendor": "IVM", "name":"AAA", "serial": "Wow42", "widthmm": 40, "heightmm": 30, "xres": 0, "yres": 0, "xmax": 2048, "ymax": 1536, "refreshrate": 50000 }, { "vendor": "RHT", "name":"BBB", "serial": "43", "widthmm": 1000, "heightmm": 750, "xres": 1024, "yres": 768, "xmax": 800, "ymax": 600, "refreshrate": 60000 } ]}'
---

 hw/display/virtio-gpu-base.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index 41f69a6ca7..a55481ec3a 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -257,8 +257,6 @@ virtio_gpu_base_device_realize(DeviceState *qdev,
         virtio_add_queue(vdev, 16, cursor_cb);
     }
 
-    g->enabled_output_bitmask = 1;
-
     g->req_state[0].width = g->conf.xres;
     g->req_state[0].height = g->conf.yres;
 
@@ -270,7 +268,9 @@ virtio_gpu_base_device_realize(DeviceState *qdev,
             g->req_state[output_idx].height_mm = node->value->heightmm;
         }
         if (node->value->has_xres && node->value->has_yres) {
-            g->enabled_output_bitmask |= (1 << output_idx);
+            if (node->value->xres != 0 && node->value->yres != 0) {
+                g->enabled_output_bitmask |= (1 << output_idx);
+            }
             g->req_state[output_idx].width = node->value->xres;
             g->req_state[output_idx].height = node->value->yres;
         }
@@ -279,6 +279,10 @@ virtio_gpu_base_device_realize(DeviceState *qdev,
         }
     }
 
+    if (!g->enabled_output_bitmask) {
+        g->enabled_output_bitmask = 1;
+    }
+
     g->hw_ops = &virtio_gpu_ops;
     for (i = 0; i < g->conf.max_outputs; i++) {
         g->scanout[i].con =
-- 
2.53.0

[PATCH v2 5/5] DRAFT virtio-gpu: Enable/disable displays

[Roland Clobus]

Need help: the viewer only shows 'Display output is not active.' after
either a) the qom-set is issued again or b) the viewer is resized

I think that the communication with viewer and qemu is not correct, the
event 'virtio_notify_config' is perhaps not the correct event, or it is
sent at the wrong moment.

Signed-off-by: Roland Clobus <rclobus@rclobus.nl>
---

Tested with:
-device '{"driver":"virtio-vga-gl", "max_outputs":2, "id":"vga001", "outputs":[ { "vendor": "IVM", "name":"AAA", "serial": "Wow42", "widthmm": 40, "heightmm": 30, "xres": 0, "yres": 0, "xmax": 2048, "ymax": 1536, "refreshrate": 50000 }, { "vendor": "RHT", "name":"BBB", "serial": "43", "widthmm": 1000, "heightmm": 750, "xres": 1024, "yres": 768, "xmax": 800, "ymax": 600, "refreshrate": 60000 } ]}'

And then:
Activate 2 displays:
qom-set -j /machine/peripheral/vga001 outputs [ { "yres": 768, "name": "AAA", "widthmm": 40, "serial": "42", "heightmm": 30, "vendor": "IVM", "xmax": 2048, "ymax": 1536, "refreshrate": 50000, "xres": 1024 }, { "yres": 600, "name": "EEE", "widthmm": 1000, "serial": "43", "heightmm": 750, "vendor": "RHT", "xmax": 1024, "ymax": 768, "refreshrate": 60000, "xres": 800 } ]

Disable display 2:
qom-set -j /machine/peripheral/vga001 outputs [ { "yres": 768, "name": "AAA", "widthmm": 40, "serial": "42", "heightmm": 30, "vendor": "IVM", "xmax": 2048, "ymax": 1536, "refreshrate": 50000, "xres": 1024 }, { "yres": 0, "name": "DDD", "widthmm": 1000, "serial": "43", "heightmm": 750, "vendor": "RHT", "xmax": 1024, "ymax": 768, "refreshrate": 60000, "xres": 0 } ]

Disable display 1:
qom-set -j /machine/peripheral/vga001 outputs [ { "yres": 0, "name": "AAA", "widthmm": 40, "serial": "42", "heightmm": 30, "vendor": "IVM", "xmax": 2048, "ymax": 1536, "refreshrate": 50000, "xres": 0 }, { "yres": 600, "name": "DDD", "widthmm": 1000, "serial": "43", "heightmm": 750, "vendor": "RHT", "xmax": 1024, "ymax": 768, "refreshrate": 60000, "xres": 800 } ]

The displays are correctly enabled/disabled, as can be seen in the
'Displays' settings of GNOME, but the viewer (virt-viewer) only shows
'Displays is not active' after the qom-set is issued for the second
time, or when the display is resized.
---
 hw/display/virtio-gpu-base.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index a55481ec3a..007a40bc42 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -92,15 +92,28 @@ virtio_gpu_base_generate_edid(VirtIOGPUBase *g, int scanout,
         // The provided resolution will be used only once after it has been set.
         // After that, the viewer has precedence over these settings
         // and decides about the resolution.
+        bool resolution_set = false;
         if (node->value->has_xres) {
             info.prefx = node->value->xres;
             g->req_state[scanout].width = node->value->xres;
             node->value->has_xres = 0;
+            resolution_set = true;
         }
         if (node->value->has_yres) {
             info.prefy = node->value->yres;
             g->req_state[scanout].height = node->value->yres;
             node->value->has_yres = 0;
+            resolution_set = true;
+        }
+        if (resolution_set) {
+            // Enable/disable the display if needed
+            if (info.prefx && info.prefy) {
+                g->enabled_output_bitmask |= (1 << scanout);
+            } else {
+                g->enabled_output_bitmask &= ~(1 << scanout);
+                dpy_gfx_replace_surface(g->scanout[scanout].con, NULL);
+                /* NEED HELP: dpy_gfx_replace_surface(g->scanout[scanout].con, NULL); only works after the qom-set is issued again */
+            }
         }
         if (node->value->has_xmax) {
             info.maxx = node->value->xmax;
@@ -151,12 +164,10 @@ static void virtio_gpu_ui_info(void *opaque, uint32_t idx, QemuUIInfo *info)
     g->req_state[idx].width_mm = info->width_mm;
     g->req_state[idx].height_mm = info->height_mm;
 
-    if (info->width && info->height) {
-        g->enabled_output_bitmask |= (1 << idx);
-    } else {
-        g->enabled_output_bitmask &= ~(1 << idx);
+    if (!(g->enabled_output_bitmask & (1 << idx))) {
+        /* NEED HELP: This only shows that the display is not active, when the viewer is resized */
+        dpy_gfx_replace_surface(g->scanout[idx].con, NULL);
     }
-
     /* send event to guest */
     virtio_gpu_notify_event(g, VIRTIO_GPU_EVENT_DISPLAY);
 }
-- 
2.53.0

Re: [PATCH v2 1/5] virtio-gpu: Support the configurable EDID fields

[Markus Armbruster]

Roland Clobus <rclobus@rclobus.nl> writes:

> Adds support for the fields that are currently configurable
> in the function qemu_edid_generate.
>
> * The serial number does not need to be numerical.
> See https://en.wikipedia.org/wiki/Extended_Display_Identification_Data
>
> * The name can be optional, there are now many more fields to set
>
> Signed-off-by: Roland Clobus <rclobus@rclobus.nl>

[...]

> diff --git a/qapi/virtio.json b/qapi/virtio.json
> index 09dd0e6d05..791d2f30f1 100644
> --- a/qapi/virtio.json
> +++ b/qapi/virtio.json
> @@ -970,24 +970,62 @@
>  ##
>  # @VirtIOGPUOutput:
>  #
> -# Describes configuration of a VirtIO GPU output.  If both @xres and
> -# @yres are set, they take precedence over root virtio-gpu resolution
> -# configuration and enable the corresponding output.  If none of @xres
> -# and @yres are set, root virtio-gpu resolution configuration takes
> -# precedence and only the first output is enabled.  Only setting one
> -# of @xres or @yres is an error.
> +# Describes configuration of a VirtIO GPU output.  Matches the fields
> +# of struct @qemu_edid_info.

Blank lines between paragraphs, so they get actually rendered as
separate paragraphs.

If VirtIOGPUOutput matches qemu_edid_info, could it *replace*
qemu_edid_info?

> +# If values are set, they take precedence over the root virtio-gpu
> +# configuration.
> +# If none of @xres and @yres are set, the root virtio-gpu resolution
> +# configuration is used and only the first output is enabled.
> +#
> +# @vendor: a 3 letter code for the vendor (since xx.x TODO)
> +#          Default: "RHT" (Red Hat, Inc.)

You need to replace the TODO placeholders before we can merge the patch.

Posting patches that are not ready for merge is okay, but please tag
them with RFC, and explain why in the cover letter.

>  #
>  # @name: the name of the output
> +#        Default: "QEMU Monitor"
> +#
> +# @serial: the serial number (since xx.x TODO)

What's the default?

> +#
> +# @widthmm: width of the display in mm (since xx.x TODO)

Contradiction: "in mm" vs.

> +#           If @widthmm is set, @heightmm must be set too
> +#           If both @widthmm and @heightmm are unset,
> +#             100 DPI is assumed

"DPI".  What's the unit?

Does the default apply to both widthmm and heightmm?

Also, fails to build:

    [77/78] Generating docs/QEMU manual with a custom command
    FAILED: [code=1] docs/docs.stamp 
    /usr/bin/env CONFDIR=etc/qemu /work/armbru/qemu/bld-docs/pyvenv/bin/sphinx-build -q -j auto -W -Dkerneldoc_werror=1 -Dversion=11.0.90 -Drelease= -Ddepfile=docs/docs.d -Ddepfile_stamp=docs/docs.stamp -b html -d /work/armbru/qemu/bld-docs/docs/manual.p /work/armbru/qemu/docs /work/armbru/qemu/bld-docs/docs/manual
    /work/armbru/qemu/docs/../qapi/virtio.json:973: ERROR: Unexpected indentation. [docutils]

With the indentation fixed, this gets rendered like

    • widthmm (int, optional) – width of the display in mm (since xx.x
      TODO) If widthmm is set, heightmm must be set too If both widthmm
      and heightmm are unset, 100 DPI is assumed

End your sentences with punctuation, and separate your paragraphs with
blank lines, please.

> +#
> +# @heightmm: height of the display in mm (since xx.x TODO)
> +#           If @heightmm is set, @widthmm must be set too

Please separate words with dashes in member names: width-mm, height-mm.

We don't normally put the unit in the member name.  E.g.

    ##
    # @ChardevVC:
    #
    # Configuration info for virtual console chardevs.
    #
    # @width: console width, in pixels
    #
    # @height: console height, in pixels
    #
    # @cols: console width, in chars
    #
    # @rows: console height, in chars

Perhaps an argument could be made that it's helpful here, because
millimeters is unusual.

>  #
>  # @xres: horizontal resolution of the output in pixels (since 11.0)
> +#        If @xres is set, @yres must be set too
>  #
>  # @yres: vertical resolution of the output in pixels (since 11.0)
> +#        If @yres is set, @xres must be set too

Defaults?

> +#
> +# @xmax: maximum horizontal resolution of the output in pixels
> +#        If @xmax is set, @ymax must be set too
> +#        (since xx.x TODO)
> +#
> +# @ymax: maximum vertical resolution of the output in pixels
> +#        If @ymax is set, @xmax must be set too
> +#        (since xx.x TODO)

Defaults?

> +#
> +# @refreshrate: refreshrate in mHz (since xx.x TODO)
> +#               Default: 75000
> +#
> +# .. note:: keep the defaults in sync with hw/display/edid-generate.c
>  #
>  # Since: 10.1
>  ##
>  
>  { 'struct': 'VirtIOGPUOutput',
> -  'data': { 'name': 'str', '*xres': 'uint16', '*yres': 'uint16' } }
> +  'data': { '*vendor': 'str',
> +            '*name': 'str',
> +            '*serial': 'str',
> +            '*widthmm': 'uint16',
> +            '*heightmm': 'uint16',
> +            '*xres': 'uint32',
> +            '*yres': 'uint32',
> +            '*xmax': 'uint32',
> +            '*ymax': 'uint32',
> +            '*refreshrate': 'uint32' } }
>  
>  ##
>  # @DummyVirtioForceArrays: