From: Andrew Keesler <ankeesler@google.com>

In 454f4b0f, we started down the path of supporting separate
configurations per display head (e.g., you have 2 heads - one with
EDID name "AAA" and the other with EDID name "BBB").

In this change, we add resolution to this configuration surface (e.g.,
you have 2 heads - one with resolution 111x222 and the other with
resolution 333x444).

  -display vnc=localhost:0,id=aaa,display=vga,head=0 \
  -display vnc=localhost:1,id=bbb,display=vga,head=1 \
  -device '{"driver":"virtio-vga",
            "max_outputs":2,
            "id":"vga",
            "outputs":[
              {
                 "name":"AAA",
                 "xres":111,
                 "yres":222
              },
              {
                 "name":"BBB",
                 "xres":333,
                 "yres":444
              }
            ]}'

Here is the behavior matrix of the current resolution configuration
surface (xres/yres) with the new resolution configuration surface
(outputs[i].xres/yres).

Case: !(xres || yres) && !(outputs[i].has_xres && outputs[i].has_yres)
Behavior: current behavior - outputs[0] enabled with default xres/yres

Case: (xres || yres) && !(outputs[i].has_xres && outputs[i].has_yres)
Behavior: current behavior - outputs[0] enabled with xres/yres

Case: !(xres || yres) && (outputs[i].has_xres && outputs[i].has_yres)
Behavior: new behavior - outputs[i] enabled with outputs[i].xres/yres

Case: (xres || yres) && (outputs[i].has_xres && outputs[i].has_yres)
Behavior: new behavior - outputs[i] enabled with outputs[i].xres/yres
Signed-off-by: Andrew Keesler <ankeesler@google.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-ID: <20250902141312.750525-2-ankeesler@google.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 qapi/virtio.json             | 10 ++++++++--
 hw/display/virtio-gpu-base.c | 10 ++++++++++
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/qapi/virtio.json b/qapi/virtio.json
index 05295ab6655..0ce789bb22f 100644
--- a/qapi/virtio.json
+++ b/qapi/virtio.json
@@ -971,15 +971,21 @@
 ##
 # @VirtIOGPUOutput:
 #
-# Describes configuration of a VirtIO GPU output.
+# 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.
 #
 # @name: the name of the output
 #
+# @xres: horizontal resolution of the output in pixels (since 10.2)
+#
+# @yres: vertical resolution of the output in pixels (since 10.2)
+#
 # Since: 10.1
 ##
 
 { 'struct': 'VirtIOGPUOutput',
-  'data': { 'name': 'str' } }
+  'data': { 'name': 'str', '*xres': 'uint16', '*yres': 'uint16' } }
 
 ##
 # @DummyVirtioForceArrays:
diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index 7269477a1c8..6adb5312a40 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -233,6 +233,16 @@ virtio_gpu_base_device_realize(DeviceState *qdev,
     g->req_state[0].width = g->conf.xres;
     g->req_state[0].height = g->conf.yres;
 
+    for (output_idx = 0, node = g->conf.outputs;
+         node && output_idx < g->conf.max_outputs;
+         output_idx++, node = node->next) {
+        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;
+            g->req_state[output_idx].height = node->value->yres;
+        }
+    }
+
     g->hw_ops = &virtio_gpu_ops;
     for (i = 0; i < g->conf.max_outputs; i++) {
         g->scanout[i].con =
-- 
2.47.3

On 2025/10/14 20:12, Alex Bennée wrote:
> From: Andrew Keesler <ankeesler@google.com>
> 
> In 454f4b0f, we started down the path of supporting separate
> configurations per display head (e.g., you have 2 heads - one with
> EDID name "AAA" and the other with EDID name "BBB").
> 
> In this change, we add resolution to this configuration surface (e.g.,
> you have 2 heads - one with resolution 111x222 and the other with
> resolution 333x444).
> 
>    -display vnc=localhost:0,id=aaa,display=vga,head=0 \
>    -display vnc=localhost:1,id=bbb,display=vga,head=1 \
>    -device '{"driver":"virtio-vga",
>              "max_outputs":2,
>              "id":"vga",
>              "outputs":[
>                {
>                   "name":"AAA",
>                   "xres":111,
>                   "yres":222
>                },
>                {
>                   "name":"BBB",
>                   "xres":333,
>                   "yres":444
>                }
>              ]}'
> 
> Here is the behavior matrix of the current resolution configuration
> surface (xres/yres) with the new resolution configuration surface
> (outputs[i].xres/yres).
> 
> Case: !(xres || yres) && !(outputs[i].has_xres && outputs[i].has_yres)
> Behavior: current behavior - outputs[0] enabled with default xres/yres
> 
> Case: (xres || yres) && !(outputs[i].has_xres && outputs[i].has_yres)
> Behavior: current behavior - outputs[0] enabled with xres/yres
> 
> Case: !(xres || yres) && (outputs[i].has_xres && outputs[i].has_yres)
> Behavior: new behavior - outputs[i] enabled with outputs[i].xres/yres
> 
> Case: (xres || yres) && (outputs[i].has_xres && outputs[i].has_yres)
> Behavior: new behavior - outputs[i] enabled with outputs[i].xres/yres
> Signed-off-by: Andrew Keesler <ankeesler@google.com>

Nitpick: it is better to have a blank line between Behavior: and 
Signed-off-by: to clarify the beginning of tags.

> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Message-ID: <20250902141312.750525-2-ankeesler@google.com>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   qapi/virtio.json             | 10 ++++++++--
>   hw/display/virtio-gpu-base.c | 10 ++++++++++
>   2 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/qapi/virtio.json b/qapi/virtio.json
> index 05295ab6655..0ce789bb22f 100644
> --- a/qapi/virtio.json
> +++ b/qapi/virtio.json
> @@ -971,15 +971,21 @@
>   ##
>   # @VirtIOGPUOutput:
>   #
> -# Describes configuration of a VirtIO GPU output.
> +# 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.
>   #
>   # @name: the name of the output
>   #
> +# @xres: horizontal resolution of the output in pixels (since 10.2)
> +#
> +# @yres: vertical resolution of the output in pixels (since 10.2)
> +#
>   # Since: 10.1
>   ##
>   
>   { 'struct': 'VirtIOGPUOutput',
> -  'data': { 'name': 'str' } }
> +  'data': { 'name': 'str', '*xres': 'uint16', '*yres': 'uint16' } }
>   
>   ##
>   # @DummyVirtioForceArrays:
> diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
> index 7269477a1c8..6adb5312a40 100644
> --- a/hw/display/virtio-gpu-base.c
> +++ b/hw/display/virtio-gpu-base.c
> @@ -233,6 +233,16 @@ virtio_gpu_base_device_realize(DeviceState *qdev,
>       g->req_state[0].width = g->conf.xres;
>       g->req_state[0].height = g->conf.yres;
>   
> +    for (output_idx = 0, node = g->conf.outputs;
> +         node && output_idx < g->conf.max_outputs;

output_idx < g->conf.max_outputs is redundant as it is already enforced 
with the first for-loop that enumerates outputs.

The condition can be simply removed, but I think merging this loop to 
the earlier loop will make the code a bit more concise.

Aside this redundancy of the code, the logic this patch implements looks 
good to me.

> +         output_idx++, node = node->next) {
> +        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;
> +            g->req_state[output_idx].height = node->value->yres;
> +        }
> +    }
> +
>       g->hw_ops = &virtio_gpu_ops;
>       for (i = 0; i < g->conf.max_outputs; i++) {
>           g->scanout[i].con =

Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp> writes:

> On 2025/10/14 20:12, Alex Bennée wrote:
>> From: Andrew Keesler <ankeesler@google.com>
>> In 454f4b0f, we started down the path of supporting separate
>> configurations per display head (e.g., you have 2 heads - one with
>> EDID name "AAA" and the other with EDID name "BBB").
<snip>
>> diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
>> index 7269477a1c8..6adb5312a40 100644
>> --- a/hw/display/virtio-gpu-base.c
>> +++ b/hw/display/virtio-gpu-base.c
>> @@ -233,6 +233,16 @@ virtio_gpu_base_device_realize(DeviceState *qdev,
>>       g->req_state[0].width = g->conf.xres;
>>       g->req_state[0].height = g->conf.yres;
>>   +    for (output_idx = 0, node = g->conf.outputs;
>> +         node && output_idx < g->conf.max_outputs;
>
> output_idx < g->conf.max_outputs is redundant as it is already
> enforced with the first for-loop that enumerates outputs.
>
> The condition can be simply removed, but I think merging this loop to
> the earlier loop will make the code a bit more concise.

I've dropped the extra condition check but I've left the loop where it
is as I don't want to change the effect of:

    g->req_state[0].width = g->conf.xres;
    g->req_state[0].height = g->conf.yres;

above. 

>
> Aside this redundancy of the code, the logic this patch implements
> looks good to me.
>
>> +         output_idx++, node = node->next) {
>> +        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;
>> +            g->req_state[output_idx].height = node->value->yres;
>> +        }
>> +    }
>> +
>>       g->hw_ops = &virtio_gpu_ops;
>>       for (i = 0; i < g->conf.max_outputs; i++) {
>>           g->scanout[i].con =

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro