Re: [PATCH 1/8] Support per-head resolutions with virtio-gpu

["Alex Bennée" <alex.bennee@linaro.org>]

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