[PULL 04/11] Support per-head resolutions with virtio-gpu

Alex Bennée posted 11 patches 3 weeks, 4 days ago
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Thomas Huth <thuth@redhat.com>, Alexandre Iooss <erdnaxe@crans.org>, Mahmoud Mandour <ma.mandourr@gmail.com>, Pierrick Bouvier <pierrick.bouvier@linaro.org>, John Levon <john.levon@nutanix.com>, Thanos Makatos <thanos.makatos@nutanix.com>, "Cédric Le Goater" <clg@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>, Dmitry Osipenko <dmitry.osipenko@collabora.com>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>
There is a newer version of this series
[PULL 04/11] Support per-head resolutions with virtio-gpu
Posted by Alex Bennée 3 weeks, 4 days ago
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>
[AJB: dropped pointless output_idx range check, tweak commit]
Message-ID: <20251016150357.876415-5-alex.bennee@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

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..14058f6bffb 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -233,6 +233,15 @@ 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++, 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


Re: [PULL 04/11] Support per-head resolutions with virtio-gpu
Posted by Markus Armbruster 3 weeks, 4 days ago
I missed this one somehow.  My apologies!

Alex Bennée <alex.bennee@linaro.org> writes:

> 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>
> [AJB: dropped pointless output_idx range check, tweak commit]
> Message-ID: <20251016150357.876415-5-alex.bennee@linaro.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> 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

Please use @NAME to refer to a local member or argument NAME for proper
rendering.

Elsewhere, we use @width and @height.  Consistency is desirable.

What happens when only one of @xres and @yres are provided?

> +# 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:

[...]
Re: [PULL 04/11] Support per-head resolutions with virtio-gpu
Posted by Alex Bennée 3 weeks, 4 days ago
Markus Armbruster <armbru@redhat.com> writes:

> I missed this one somehow.  My apologies!
>
> Alex Bennée <alex.bennee@linaro.org> writes:
>
>> 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>
>> [AJB: dropped pointless output_idx range check, tweak commit]
>> Message-ID: <20251016150357.876415-5-alex.bennee@linaro.org>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>
>> 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
>
> Please use @NAME to refer to a local member or argument NAME for proper
> rendering.
>
> Elsewhere, we use @width and @height.  Consistency is desirable.
>
> What happens when only one of @xres and @yres are provided?

*sigh* OK I'll drop this from the PR.

>
>> +# 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:
>
> [...]

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro