| Message ID | 20230915111130.24064-6-ray.huang@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Support blob memory and venus on qemu | expand |
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 > >
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
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
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
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.
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
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(+)