1. Patchew
  2. Xen
  3. [QEMU PATCH v5 00/13] Support blob memory and venus on qemu
  4. View patch

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

Huang Rui posted 13 patches 2 years, 4 months ago
There is a newer version of this series
[QEMU PATCH v5 05/13] virtio-gpu: Configure context init for virglrenderer
Posted by Huang Rui 2 years, 4 months ago 
Configure context init feature flag for virglrenderer.

Originally-by: Antonio Caggiano <antonio.caggiano@collabora.com>
Signed-off-by: Huang Rui <ray.huang@amd.com>
---

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))
 endif
 blkio = not_found
 if not get_option('blkio').auto() or have_block
-- 
2.34.1
Re: [QEMU PATCH v5 05/13] virtio-gpu: Configure context init for virglrenderer
Posted by Dmitry Osipenko 2 years, 4 months ago 
On 9/15/23 14:11, Huang Rui wrote:
> Configure context init feature flag for virglrenderer.
> 
> Originally-by: Antonio Caggiano <antonio.caggiano@collabora.com>
> Signed-off-by: Huang Rui <ray.huang@amd.com>
> ---
> 
> 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.

@@ -1060,6 +1060,7 @@ virgl = not_found
 have_vhost_user_gpu = have_tools and targetos == 'linux' and pixman.found()
 if not get_option('virglrenderer').auto() or have_system or have_vhost_user_gpu
   virgl = dependency('virglrenderer',
+                     version: '>=1.0.0',
                      method: 'pkg-config',
                      required: get_option('virglrenderer'))
   if virgl.found()


Best regards,
Dmitry
Re: [QEMU PATCH v5 05/13] virtio-gpu: Configure context init for virglrenderer
Posted by Daniel P. Berrangé 2 years, 4 months ago 
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@collabora.com>
> > Signed-off-by: Huang Rui <ray.huang@amd.com>
> > ---
> > 
> > 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.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [QEMU PATCH v5 05/13] virtio-gpu: Configure context init for virglrenderer
Posted by Dmitry Osipenko 2 years, 3 months ago 
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@collabora.com>
>>> Signed-off-by: Huang Rui <ray.huang@amd.com>
>>> ---
>>>
>>> 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
Re: [QEMU PATCH v5 05/13] virtio-gpu: Configure context init for virglrenderer
Posted by Marc-André Lureau 2 years, 4 months ago 
Hi

On Fri, Sep 15, 2023 at 6:16 PM Huang Rui <ray.huang@amd.com> wrote:
>
> Configure context init feature flag for virglrenderer.
>
> Originally-by: Antonio Caggiano <antonio.caggiano@collabora.com>
> Signed-off-by: Huang Rui <ray.huang@amd.com>
> ---
>
> 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))

Move it under the "if virgl.found()" block above.

I suggest to name it after what is actually checked:
HAVE_VIRGL_CONTEXT_CREATE_WITH_FLAGS for ex

>  endif
>  blkio = not_found
>  if not get_option('blkio').auto() or have_block
> --
> 2.34.1
>
>


-- 
Marc-André Lureau
Re: [QEMU PATCH v5 05/13] virtio-gpu: Configure context init for virglrenderer
Posted by Huang Rui 2 years, 4 months ago 
On Tue, Sep 19, 2023 at 04:17:43PM +0800, Marc-André Lureau wrote:
> Hi
> 
> On Fri, Sep 15, 2023 at 6:16 PM Huang Rui <ray.huang@amd.com> wrote:
> >
> > Configure context init feature flag for virglrenderer.
> >
> > Originally-by: Antonio Caggiano <antonio.caggiano@collabora.com>
> > Signed-off-by: Huang Rui <ray.huang@amd.com>
> > ---
> >
> > 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))
> 
> Move it under the "if virgl.found()" block above.
> 
> I suggest to name it after what is actually checked:
> HAVE_VIRGL_CONTEXT_CREATE_WITH_FLAGS for ex
> 

OK, will update it in V6.

Thanks,
Ray

> >  endif
> >  blkio = not_found
> >  if not get_option('blkio').auto() or have_block
> > --
> > 2.34.1
> >
> >
> 
> 
> -- 
> Marc-André Lureau

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.