Re: [PATCH v5 3/4] virtio-gpu: decouple Venus from CONFIG_OPENGL

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

Lucas Amaral <lucaaamaral@gmail.com> writes:

> Alex Bennée <alex.bennee@linaro.org> writes:
>
>> Isn't this entirely in QEMU's control? The Venus flag is checked
>> when needed and set by:
>>
>>     DEFINE_PROP_BIT("venus", VirtIOGPU, parent_obj.conf.flags,
>>                     VIRTIO_GPU_FLAG_VENUS_ENABLED, false),
>>
>> So do we just need to define:
>>
>>     DEFINE_PROP_BIT("virgl", VirtIOGPU, parent_obj.conf.flags,
>>                     VIRTIO_GPU_FLAG_VIRGL_ENABLED, true),
>>
>> and allow the user to squash it and drop the:
>>
>>     g->parent_obj.conf.flags |= (1 << VIRTIO_GPU_FLAG_VIRGL_ENABLED);
>>
>> I guess we need to drop the prop bit definition on platforms that
>> can't support OpenGL?
>
> Agreed -- making "virgl=on" a real OpenGL-only property bit, distinct
> from "venus=on" and no longer implicitly forced on whenever
> virglrenderer is initialized, is the right shape. Both TODOs in
> virtio_gpu_get_flags() resolve cleanly: the GL scanout test becomes
> a pure property check (no display_opengl runtime crutch), and the
> GRAPHIC_FLAGS_VK slot for Venus direct Vulkan scanout becomes a
> one-liner once a Vulkan-scanout path exists.
>
> My hesitation on folding it into this series is scope:
> VIRTIO_GPU_FLAG_VIRGL_ENABLED is currently forced on whenever
> virglrenderer is initialized (including Venus-only configs)

Venus-only configs being MacOS right? On Linux the default would for
virgl to be on unless the user explicitly turns it off (we can expand
the GPU functional tests to make sure this works).

> , so the
> cleanup touches property defaults and existing user expectations
> (virgl=on,venus=on vs venus=on only) -- probably wants its own
> changelog and review, somewhat outside the no-GL plumbing this
> series is about. I'd prefer to land this series as-is and follow up
> with a dedicated "split virgl/venus property bits" series
> afterwards.
>
> That said, if you'd rather see the split as a prep patch at the
> front of v6, happy to do it that way -- your call.

Yes - have it as a prep patch in the series. 

>
> Thanks,
> Lucas

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro