Re: [QEMU PATCH v5 05/13] virtio-gpu: Configure context init for virglrenderer

[Dmitry Osipenko <dmitry.osipenko@xxxxxxxxxxxxx>]

On 10/10/23 14:51, Daniel P. Berrangé wrote:
> On Tue, Oct 10, 2023 at 02:41:22PM +0300, Dmitry Osipenko wrote:
>> On 9/15/23 14:11, Huang Rui wrote:
>>> Configure context init feature flag for virglrenderer.
>>>
>>> Originally-by: Antonio Caggiano <antonio.caggiano@xxxxxxxxxxxxx>
>>> Signed-off-by: Huang Rui <ray.huang@xxxxxxx>
>>> ---
>>>
>>> V4 -> V5:
>>>     - Inverted patch 5 and 6 because we should configure
>>>       HAVE_VIRGL_CONTEXT_INIT firstly. (Philippe)
>>>
>>>  meson.build | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/meson.build b/meson.build
>>> index 98e68ef0b1..ff20d3c249 100644
>>> --- a/meson.build
>>> +++ b/meson.build
>>> @@ -1068,6 +1068,10 @@ if not get_option('virglrenderer').auto() or 
>>> have_system or have_vhost_user_gpu
>>>                                         prefix: '#include 
>>> <virglrenderer.h>',
>>>                                         dependencies: virgl))
>>>    endif
>>> +  config_host_data.set('HAVE_VIRGL_CONTEXT_INIT',
>>> +                       
>>> cc.has_function('virgl_renderer_context_create_with_flags',
>>> +                                       prefix: '#include 
>>> <virglrenderer.h>',
>>> +                                       dependencies: virgl))
>> The "cc.has_function" doesn't work properly with PKG_CONFIG_PATH. It ignores 
>> the the given pkg and uses system includes. Antonio was aware about that 
>> problem [1].
>>
>> [1] 
>> https://gitlab.freedesktop.org/Fahien/qemu/-/commit/ea1c252a707940983ccce71e92a292b49496bfcd
>>
>> Given that virglrenderer 1.0 has been released couple weeks ago,
>> can we make the v1.0 a mandatory requirement for qemu and remove
>> all the ifdefs? I doubt that anyone is going to test newer qemu
>> using older libviglrenderer, all that ifdef code will be bitrotting.
> 
> We cannot do that. If is is only weeks old, no distro will
> have virglrenderer 1.0 present. QEMU has a defined set of
> platforms that it targets compatibility with:
> 
>   https://www.qemu.org/docs/master/about/build-platforms.html
> 
> For newly added functionality we can set the min version to
> something newer than the oldest QEMU target platform.
> 
> For existing functionality though, we must not regress wrt
> the currently supported set of target platforms. So we can
> only bump the min version to that present in the oldest
> platform we target.

Well, then alternatively we could specify supported libvirglrender
features based on the lib version to avoid those pkgconfig+meson troubles.

-- 
Best regards,
Dmitry