Re: [PATCH v5 8/8] docs/system: Expand the virtio-gpu documentation

[Akihiko Odaki <akihiko.odaki@daynix.com>]

On 2025/02/18 15:27, Dmitry Osipenko wrote:
> On 2/13/25 07:32, Akihiko Odaki wrote:
>> On 2025/02/10 6:03, Dmitry Osipenko wrote:
>>> On 2/6/25 08:41, Akihiko Odaki wrote:
>>>> On 2025/02/06 2:40, Dmitry Osipenko wrote:
>>>>> On 2/3/25 08:31, Akihiko Odaki wrote:
>>>>> ...
>>>>>>> Requirements don't vary much. For example virglrenderer minigbm
>>>>>>> support
>>>>>>> is mandatory for crosvm, while for QEMU it's not.
>>>>>>
>>>>>> Is that true? It seems that virglrenderer uses builds without minigbm
>>>>>> support to run tests on GitLab CI.
>>>>>
>>>>> CI is running in a headless mode using software renderer. For a
>>>>> full-featured crosvm support running on a baremetal, minigbm should be
>>>>> needed, along with other downstream features.
>>>>
>>>> That makes sense.
>>>>
>>>> Based on your input, for QEMU, I don't think we need a separate
>>>> documentation to describe libvirglrenderer's build flags though crosvm
>>>> should have some documentation saying it requires minigbm.
>>>>
>>>>>
>>>>>> Anyway, if there is any variance in the build procedure, that may
>>>>>> justify having a separate build instruction in QEMU tree to avoid
>>>>>> confusion. Otherwise, it's better to have a documentation shared with
>>>>>> other VMMs.
>>>>>>
>>>>>>>
>>>>>>>> I'm not entirely sure the documentation will stay as is for that
>>>>>>>> long.
>>>>>>>> The requirements of Intel native context refer to merge requests
>>>>>>>> that
>>>>>>>> can be merged sooner or later. Asahi may need more updates if you
>>>>>>>> document it too because its DRM ABI is still unstable.
>>>>>>>
>>>>>>> The unstable parts of course will need to be updated sooner, but the
>>>>>>> stable should be solid for years. I expect that about a year later
>>>>>>> requirements will need to be revisited.
>>>>>>>
>>>>>>
>>>>>> It will be some burden in the future. Now you are adding this
>>>>>> documentation just for QEMU, but crosvm and libkrun may gain similar
>>>>>> documentation. The DRM native context support for Intel and Asahi
>>>>>> is in
>>>>>> development, and I guess nvk will support it someday.
>>>>>>
>>>>>> So, a very rough estimation of future documentation updates will be:
>>>>>> (number of VMMs) * (number of DRM native contexts in development)
>>>>>> = 3 * 3
>>>>>> = 9
>>>>>>
>>>>>> That's manageable but suboptimal.
>>>>>
>>>>> I don't mind deferring the doc addition if that's preferred. Either way
>>>>> is fine with me. Yet it's better to have doc than not.
>>>>
>>>> My suggestion is not to defer the addition, but to add it to Mesa, which
>>>> does not require deferring.
>>>>
>>>>>
>>>>> In my view crosvm and libkrun exist separately from QEMU, they serve a
>>>>> different purpose. Majority of QEMU users likely never heard about
>>>>> those
>>>>> other VMMs. A unified doc won't be a worthwhile effort, IMO.
>>>>>
>>>>
>>>> When evaluating the utility of a unified documentation, Whether the
>>>> majority of Mesa/Virgl users care VMMs other than QEMU matters more. And
>>>> I think it is true; libkrun and crosvm are excellent options for
>>>> graphics-accelerated VMs.
>>>>
>>>> If we have a unified documentation, any VM can point to it for the build
>>>> instruction of Mesa and virglrenderer. Once that's done, QEMU users who
>>>> want graphics acceleration can take the following steps:
>>>> 1. See docs/system/devices/virtio-gpu.rst
>>>> 2. Figure out that they need Mesa and virglrenderer
>>>> 3. Click the link to the unified documentation
>>>> 4. Build Mesa and virglrenderer accordingly
>>>>
>>>> No other VMMs will bother them in this procedure.
>>>
>>> Will see. For the starter, adding example build flags to QEMU doesn't
>>> hurt, it's a very minimal information. Later on, if and when all
>>> relevant Mesa/virglrenderer doc pages will appear, it won't be a problem
>>> replace QEMU flags with the links. Please let's do it step-by-step, one
>>> step at a time :)
>>>
>>
>> To be honest, I'm concerned that you may be using QEMU as a staging tree
>> for Mesa/virglrenderer. Submitting a documentation to QEMU as a
>> preparation to submit one to Mesa is not OK.
>>
>> You shouldn't submit a documentation to QEMU if upstream
>> Mesa developers rejects it because it contains too little information.
>> It may not hurt QEMU, but still lacks a valid reasoning.
> 
> Don't understand what you're talking about here. I may remind that this
> is not my QEMU doc patch to begin with, hence it has nothing to do with
> Mesa nor with virglrenderer. Alex wants to help QEMU users by adding
> more QEMU documentation.

I understand Alex and you intend to help QEMU users, but sometimes a 
change to help QEMU users is better to be done in the upstream of its 
dependency.

For example, when I submitted a patch to suppress a sanitizer warning in 
the past, I was asked if it may be better to propose a change of the 
sanitizer behavior instead of changing QEMU code. We eventually 
concluded we should change our code at that time, but it could have 
turned out otherwise. It is no different from the normal review process; 
a submitter proposes a solution, a reviewer suggests alternatives, and 
both figure out the optimal one in the discussion.

> 
> Maybe you're also not very familiar with the Mesa development process.
> This is okay, no problems.
> 
>> Mesa should have more people who care virtio-gpu as there are people
>> using other VMMs and perhaps it may be difficult to convince them to add
>> a documentation like this. It is still not a good idea to workaround
>> that by adding one to QEMU. The documentation submitted to QEMU is
>> mostly reviewed only by me, who barely used Venus and DRM native
>> contexts, which is not a good sign. Getting reviewed by more people is
>> one of the advantage of open-source contribution after all so let's keep
>> it.
>>
>> I can help you add a documentation to Mesa by reviewing and supporting
>> one if you want, but I cannot support adding one to QEMU if it's done to
>> avoid a potential challenge to add it to Mesa.
> 
> Thanks a lot! More reviewers always good to have, will definitely ping
> you. Right now writing more Mesa documentation isn't a priority, hence
> will take time to get to it.
> 

I appreciate your contribution to the documentation too. Once there was 
no documentation, but Gurchetan Singh added one along with their 
Rutabaga patches. With your change, the documentation will be 
comprehensive also for virglrenderer.

Regards,
Akihiko Odaki