| Message ID | 20220826100812.410773-2-antonio.caggiano@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | virtio-gpu: CONTEXT_INIT feature | expand |
Hi On Fri, Aug 26, 2022 at 2:12 PM Antonio Caggiano < antonio.caggiano@collabora.com> wrote: > Create virgl renderer context with flags using context_id when valid. > > v2: > - The feature can be enabled via the context_init config option. > - A warning message will be emitted and the feature will not be used > when linking with virglrenderer versions without context_init support. > > v3: Define HAVE_VIRGL_CONTEXT_INIT in config_host_data. > > Signed-off-by: Antonio Caggiano <antonio.caggiano@collabora.com> > --- > hw/display/virtio-gpu-base.c | 3 +++ > hw/display/virtio-gpu-virgl.c | 16 ++++++++++++++-- > hw/display/virtio-gpu.c | 2 ++ > include/hw/virtio/virtio-gpu.h | 3 +++ > meson.build | 5 +++++ > 5 files changed, 27 insertions(+), 2 deletions(-) > > diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c > index a29f191aa8..6c5f1f327f 100644 > --- a/hw/display/virtio-gpu-base.c > +++ b/hw/display/virtio-gpu-base.c > @@ -215,6 +215,9 @@ virtio_gpu_base_get_features(VirtIODevice *vdev, > uint64_t features, > if (virtio_gpu_blob_enabled(g->conf)) { > features |= (1 << VIRTIO_GPU_F_RESOURCE_BLOB); > } > + if (virtio_gpu_context_init_enabled(g->conf)) { > + features |= (1 << VIRTIO_GPU_F_CONTEXT_INIT); > + } > > return features; > } > diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c > index 73cb92c8d5..274cbc44de 100644 > --- a/hw/display/virtio-gpu-virgl.c > +++ b/hw/display/virtio-gpu-virgl.c > @@ -97,8 +97,20 @@ static void virgl_cmd_context_create(VirtIOGPU *g, > trace_virtio_gpu_cmd_ctx_create(cc.hdr.ctx_id, > cc.debug_name); > > - virgl_renderer_context_create(cc.hdr.ctx_id, cc.nlen, > - cc.debug_name); > + if (cc.context_init) { > +#ifdef HAVE_VIRGL_CONTEXT_INIT > + virgl_renderer_context_create_with_flags(cc.hdr.ctx_id, > + cc.context_init, > + cc.nlen, > + cc.debug_name); > + return; > +#else > + qemu_log_mask(LOG_UNIMP, > + "Linked virglrenderer does not support > context-init\n"); > What is the outcome in that case? > +#endif > + } > + > + virgl_renderer_context_create(cc.hdr.ctx_id, cc.nlen, cc.debug_name); > } > > static void virgl_cmd_context_destroy(VirtIOGPU *g, > diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c > index 20cc703dcc..fa667ec234 100644 > --- a/hw/display/virtio-gpu.c > +++ b/hw/display/virtio-gpu.c > @@ -1424,6 +1424,8 @@ static Property virtio_gpu_properties[] = { > 256 * MiB), > DEFINE_PROP_BIT("blob", VirtIOGPU, parent_obj.conf.flags, > VIRTIO_GPU_FLAG_BLOB_ENABLED, false), > + DEFINE_PROP_BIT("context_init", VirtIOGPU, parent_obj.conf.flags, > + VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED, false), > DEFINE_PROP_END_OF_LIST(), > }; > > diff --git a/include/hw/virtio/virtio-gpu.h > b/include/hw/virtio/virtio-gpu.h > index 2e28507efe..c6f5cfde47 100644 > --- a/include/hw/virtio/virtio-gpu.h > +++ b/include/hw/virtio/virtio-gpu.h > @@ -90,6 +90,7 @@ enum virtio_gpu_base_conf_flags { > VIRTIO_GPU_FLAG_EDID_ENABLED, > VIRTIO_GPU_FLAG_DMABUF_ENABLED, > VIRTIO_GPU_FLAG_BLOB_ENABLED, > + VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED, > }; > > #define virtio_gpu_virgl_enabled(_cfg) \ > @@ -102,6 +103,8 @@ enum virtio_gpu_base_conf_flags { > (_cfg.flags & (1 << VIRTIO_GPU_FLAG_DMABUF_ENABLED)) > #define virtio_gpu_blob_enabled(_cfg) \ > (_cfg.flags & (1 << VIRTIO_GPU_FLAG_BLOB_ENABLED)) > +#define virtio_gpu_context_init_enabled(_cfg) \ > + (_cfg.flags & (1 << VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED)) > > struct virtio_gpu_base_conf { > uint32_t max_outputs; > diff --git a/meson.build b/meson.build > index 20fddbd707..e1071b3563 100644 > --- a/meson.build > +++ b/meson.build > @@ -718,6 +718,11 @@ if not get_option('virglrenderer').auto() or > have_system or have_vhost_user_gpu > method: 'pkg-config', > required: get_option('virglrenderer'), > kwargs: static_kwargs) > + > + config_host_data.set('HAVE_VIRGL_CONTEXT_INIT', > + > cc.has_function('virgl_renderer_context_create_with_flags', > + prefix: '#include > <virglrenderer.h>', > + dependencies: virgl)) > endif > curl = not_found > if not get_option('curl').auto() or have_block > -- > 2.34.1 > > > lgtm Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Hi Marc-André, On 26/08/2022 12:16, Marc-André Lureau wrote: > Hi > > On Fri, Aug 26, 2022 at 2:12 PM Antonio Caggiano > <antonio.caggiano@collabora.com <mailto:antonio.caggiano@collabora.com>> > wrote: > > Create virgl renderer context with flags using context_id when valid. > > v2: > - The feature can be enabled via the context_init config option. > - A warning message will be emitted and the feature will not be used > when linking with virglrenderer versions without context_init > support. > > v3: Define HAVE_VIRGL_CONTEXT_INIT in config_host_data. > > Signed-off-by: Antonio Caggiano <antonio.caggiano@collabora.com > <mailto:antonio.caggiano@collabora.com>> > --- > hw/display/virtio-gpu-base.c | 3 +++ > hw/display/virtio-gpu-virgl.c | 16 ++++++++++++++-- > hw/display/virtio-gpu.c | 2 ++ > include/hw/virtio/virtio-gpu.h | 3 +++ > meson.build | 5 +++++ > 5 files changed, 27 insertions(+), 2 deletions(-) > > diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c > index a29f191aa8..6c5f1f327f 100644 > --- a/hw/display/virtio-gpu-base.c > +++ b/hw/display/virtio-gpu-base.c > @@ -215,6 +215,9 @@ virtio_gpu_base_get_features(VirtIODevice *vdev, > uint64_t features, > if (virtio_gpu_blob_enabled(g->conf)) { > features |= (1 << VIRTIO_GPU_F_RESOURCE_BLOB); > } > + if (virtio_gpu_context_init_enabled(g->conf)) { > + features |= (1 << VIRTIO_GPU_F_CONTEXT_INIT); > + } > > return features; > } > diff --git a/hw/display/virtio-gpu-virgl.c > b/hw/display/virtio-gpu-virgl.c > index 73cb92c8d5..274cbc44de 100644 > --- a/hw/display/virtio-gpu-virgl.c > +++ b/hw/display/virtio-gpu-virgl.c > @@ -97,8 +97,20 @@ static void virgl_cmd_context_create(VirtIOGPU *g, > trace_virtio_gpu_cmd_ctx_create(cc.hdr.ctx_id, > cc.debug_name); > > - virgl_renderer_context_create(cc.hdr.ctx_id, cc.nlen, > - cc.debug_name); > + if (cc.context_init) { > +#ifdef HAVE_VIRGL_CONTEXT_INIT > + virgl_renderer_context_create_with_flags(cc.hdr.ctx_id, > + cc.context_init, > + cc.nlen, > + cc.debug_name); > + return; > +#else > + qemu_log_mask(LOG_UNIMP, > + "Linked virglrenderer does not support > context-init\n"); > > > What is the outcome in that case? It's in the commit message: "A warning message will be emitted and the feature will not be used when linking with virglrenderer versions without context_init" > > +#endif > + } > + > + virgl_renderer_context_create(cc.hdr.ctx_id, cc.nlen, > cc.debug_name); > } > > static void virgl_cmd_context_destroy(VirtIOGPU *g, > diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c > index 20cc703dcc..fa667ec234 100644 > --- a/hw/display/virtio-gpu.c > +++ b/hw/display/virtio-gpu.c > @@ -1424,6 +1424,8 @@ static Property virtio_gpu_properties[] = { > 256 * MiB), > DEFINE_PROP_BIT("blob", VirtIOGPU, parent_obj.conf.flags, > VIRTIO_GPU_FLAG_BLOB_ENABLED, false), > + DEFINE_PROP_BIT("context_init", VirtIOGPU, parent_obj.conf.flags, > + VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED, false), > DEFINE_PROP_END_OF_LIST(), > }; > > diff --git a/include/hw/virtio/virtio-gpu.h > b/include/hw/virtio/virtio-gpu.h > index 2e28507efe..c6f5cfde47 100644 > --- a/include/hw/virtio/virtio-gpu.h > +++ b/include/hw/virtio/virtio-gpu.h > @@ -90,6 +90,7 @@ enum virtio_gpu_base_conf_flags { > VIRTIO_GPU_FLAG_EDID_ENABLED, > VIRTIO_GPU_FLAG_DMABUF_ENABLED, > VIRTIO_GPU_FLAG_BLOB_ENABLED, > + VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED, > }; > > #define virtio_gpu_virgl_enabled(_cfg) \ > @@ -102,6 +103,8 @@ enum virtio_gpu_base_conf_flags { > (_cfg.flags & (1 << VIRTIO_GPU_FLAG_DMABUF_ENABLED)) > #define virtio_gpu_blob_enabled(_cfg) \ > (_cfg.flags & (1 << VIRTIO_GPU_FLAG_BLOB_ENABLED)) > +#define virtio_gpu_context_init_enabled(_cfg) \ > + (_cfg.flags & (1 << VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED)) > > struct virtio_gpu_base_conf { > uint32_t max_outputs; > diff --git a/meson.build b/meson.build > index 20fddbd707..e1071b3563 100644 > --- a/meson.build > +++ b/meson.build > @@ -718,6 +718,11 @@ if not get_option('virglrenderer').auto() or > have_system or have_vhost_user_gpu > method: 'pkg-config', > required: get_option('virglrenderer'), > kwargs: static_kwargs) > + > + config_host_data.set('HAVE_VIRGL_CONTEXT_INIT', > + > cc.has_function('virgl_renderer_context_create_with_flags', > + prefix: '#include > <virglrenderer.h>', > + dependencies: virgl)) > endif > curl = not_found > if not get_option('curl').auto() or have_block > -- > 2.34.1 > > > > lgtm > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com > <mailto:marcandre.lureau@redhat.com>> > > -- > Marc-André Lureau
Hi On Fri, Aug 26, 2022 at 2:45 PM Antonio Caggiano < antonio.caggiano@collabora.com> wrote: > Hi Marc-André, > > On 26/08/2022 12:16, Marc-André Lureau wrote: > > Hi > > > > On Fri, Aug 26, 2022 at 2:12 PM Antonio Caggiano > > <antonio.caggiano@collabora.com <mailto:antonio.caggiano@collabora.com>> > > > wrote: > > > > Create virgl renderer context with flags using context_id when valid. > > > > v2: > > - The feature can be enabled via the context_init config option. > > - A warning message will be emitted and the feature will not be used > > when linking with virglrenderer versions without context_init > > support. > > > > v3: Define HAVE_VIRGL_CONTEXT_INIT in config_host_data. > > > > Signed-off-by: Antonio Caggiano <antonio.caggiano@collabora.com > > <mailto:antonio.caggiano@collabora.com>> > > --- > > hw/display/virtio-gpu-base.c | 3 +++ > > hw/display/virtio-gpu-virgl.c | 16 ++++++++++++++-- > > hw/display/virtio-gpu.c | 2 ++ > > include/hw/virtio/virtio-gpu.h | 3 +++ > > meson.build | 5 +++++ > > 5 files changed, 27 insertions(+), 2 deletions(-) > > > > diff --git a/hw/display/virtio-gpu-base.c > b/hw/display/virtio-gpu-base.c > > index a29f191aa8..6c5f1f327f 100644 > > --- a/hw/display/virtio-gpu-base.c > > +++ b/hw/display/virtio-gpu-base.c > > @@ -215,6 +215,9 @@ virtio_gpu_base_get_features(VirtIODevice *vdev, > > uint64_t features, > > if (virtio_gpu_blob_enabled(g->conf)) { > > features |= (1 << VIRTIO_GPU_F_RESOURCE_BLOB); > > } > > + if (virtio_gpu_context_init_enabled(g->conf)) { > > + features |= (1 << VIRTIO_GPU_F_CONTEXT_INIT); > > + } > > > > return features; > > } > > diff --git a/hw/display/virtio-gpu-virgl.c > > b/hw/display/virtio-gpu-virgl.c > > index 73cb92c8d5..274cbc44de 100644 > > --- a/hw/display/virtio-gpu-virgl.c > > +++ b/hw/display/virtio-gpu-virgl.c > > @@ -97,8 +97,20 @@ static void virgl_cmd_context_create(VirtIOGPU *g, > > trace_virtio_gpu_cmd_ctx_create(cc.hdr.ctx_id, > > cc.debug_name); > > > > - virgl_renderer_context_create(cc.hdr.ctx_id, cc.nlen, > > - cc.debug_name); > > + if (cc.context_init) { > > +#ifdef HAVE_VIRGL_CONTEXT_INIT > > + virgl_renderer_context_create_with_flags(cc.hdr.ctx_id, > > + cc.context_init, > > + cc.nlen, > > + cc.debug_name); > > + return; > > +#else > > + qemu_log_mask(LOG_UNIMP, > > + "Linked virglrenderer does not support > > context-init\n"); > > > > > > What is the outcome in that case? > > It's in the commit message: "A warning message will be emitted and the > feature will not be used when linking with virglrenderer versions > without context_init" > > Ah ok, I didn't expect this to be under the changelog. We generally don't put it in the commit message, but rather after the three-dash line. > > > +#endif > > + } > > + > > + virgl_renderer_context_create(cc.hdr.ctx_id, cc.nlen, > > cc.debug_name); > > } > > > > static void virgl_cmd_context_destroy(VirtIOGPU *g, > > diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c > > index 20cc703dcc..fa667ec234 100644 > > --- a/hw/display/virtio-gpu.c > > +++ b/hw/display/virtio-gpu.c > > @@ -1424,6 +1424,8 @@ static Property virtio_gpu_properties[] = { > > 256 * MiB), > > DEFINE_PROP_BIT("blob", VirtIOGPU, parent_obj.conf.flags, > > VIRTIO_GPU_FLAG_BLOB_ENABLED, false), > > + DEFINE_PROP_BIT("context_init", VirtIOGPU, > parent_obj.conf.flags, > > + VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED, false), > > DEFINE_PROP_END_OF_LIST(), > > }; > > > > diff --git a/include/hw/virtio/virtio-gpu.h > > b/include/hw/virtio/virtio-gpu.h > > index 2e28507efe..c6f5cfde47 100644 > > --- a/include/hw/virtio/virtio-gpu.h > > +++ b/include/hw/virtio/virtio-gpu.h > > @@ -90,6 +90,7 @@ enum virtio_gpu_base_conf_flags { > > VIRTIO_GPU_FLAG_EDID_ENABLED, > > VIRTIO_GPU_FLAG_DMABUF_ENABLED, > > VIRTIO_GPU_FLAG_BLOB_ENABLED, > > + VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED, > > }; > > > > #define virtio_gpu_virgl_enabled(_cfg) \ > > @@ -102,6 +103,8 @@ enum virtio_gpu_base_conf_flags { > > (_cfg.flags & (1 << VIRTIO_GPU_FLAG_DMABUF_ENABLED)) > > #define virtio_gpu_blob_enabled(_cfg) \ > > (_cfg.flags & (1 << VIRTIO_GPU_FLAG_BLOB_ENABLED)) > > +#define virtio_gpu_context_init_enabled(_cfg) \ > > + (_cfg.flags & (1 << VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED)) > > > > struct virtio_gpu_base_conf { > > uint32_t max_outputs; > > diff --git a/meson.build b/meson.build > > index 20fddbd707..e1071b3563 100644 > > --- a/meson.build > > +++ b/meson.build > > @@ -718,6 +718,11 @@ if not get_option('virglrenderer').auto() or > > have_system or have_vhost_user_gpu > > method: 'pkg-config', > > required: get_option('virglrenderer'), > > kwargs: static_kwargs) > > + > > + config_host_data.set('HAVE_VIRGL_CONTEXT_INIT', > > + > > cc.has_function('virgl_renderer_context_create_with_flags', > > + prefix: '#include > > <virglrenderer.h>', > > + dependencies: virgl)) > > endif > > curl = not_found > > if not get_option('curl').auto() or have_block > > -- > > 2.34.1 > > > > > > > > lgtm > > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com > > <mailto:marcandre.lureau@redhat.com>> > > > > -- > > Marc-André Lureau > >
diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c index a29f191aa8..6c5f1f327f 100644 --- a/hw/display/virtio-gpu-base.c +++ b/hw/display/virtio-gpu-base.c @@ -215,6 +215,9 @@ virtio_gpu_base_get_features(VirtIODevice *vdev, uint64_t features, if (virtio_gpu_blob_enabled(g->conf)) { features |= (1 << VIRTIO_GPU_F_RESOURCE_BLOB); } + if (virtio_gpu_context_init_enabled(g->conf)) { + features |= (1 << VIRTIO_GPU_F_CONTEXT_INIT); + } return features; } diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c index 73cb92c8d5..274cbc44de 100644 --- a/hw/display/virtio-gpu-virgl.c +++ b/hw/display/virtio-gpu-virgl.c @@ -97,8 +97,20 @@ static void virgl_cmd_context_create(VirtIOGPU *g, trace_virtio_gpu_cmd_ctx_create(cc.hdr.ctx_id, cc.debug_name); - virgl_renderer_context_create(cc.hdr.ctx_id, cc.nlen, - cc.debug_name); + if (cc.context_init) { +#ifdef HAVE_VIRGL_CONTEXT_INIT + virgl_renderer_context_create_with_flags(cc.hdr.ctx_id, + cc.context_init, + cc.nlen, + cc.debug_name); + return; +#else + qemu_log_mask(LOG_UNIMP, + "Linked virglrenderer does not support context-init\n"); +#endif + } + + virgl_renderer_context_create(cc.hdr.ctx_id, cc.nlen, cc.debug_name); } static void virgl_cmd_context_destroy(VirtIOGPU *g, diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c index 20cc703dcc..fa667ec234 100644 --- a/hw/display/virtio-gpu.c +++ b/hw/display/virtio-gpu.c @@ -1424,6 +1424,8 @@ static Property virtio_gpu_properties[] = { 256 * MiB), DEFINE_PROP_BIT("blob", VirtIOGPU, parent_obj.conf.flags, VIRTIO_GPU_FLAG_BLOB_ENABLED, false), + DEFINE_PROP_BIT("context_init", VirtIOGPU, parent_obj.conf.flags, + VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED, false), DEFINE_PROP_END_OF_LIST(), }; diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h index 2e28507efe..c6f5cfde47 100644 --- a/include/hw/virtio/virtio-gpu.h +++ b/include/hw/virtio/virtio-gpu.h @@ -90,6 +90,7 @@ enum virtio_gpu_base_conf_flags { VIRTIO_GPU_FLAG_EDID_ENABLED, VIRTIO_GPU_FLAG_DMABUF_ENABLED, VIRTIO_GPU_FLAG_BLOB_ENABLED, + VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED, }; #define virtio_gpu_virgl_enabled(_cfg) \ @@ -102,6 +103,8 @@ enum virtio_gpu_base_conf_flags { (_cfg.flags & (1 << VIRTIO_GPU_FLAG_DMABUF_ENABLED)) #define virtio_gpu_blob_enabled(_cfg) \ (_cfg.flags & (1 << VIRTIO_GPU_FLAG_BLOB_ENABLED)) +#define virtio_gpu_context_init_enabled(_cfg) \ + (_cfg.flags & (1 << VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED)) struct virtio_gpu_base_conf { uint32_t max_outputs; diff --git a/meson.build b/meson.build index 20fddbd707..e1071b3563 100644 --- a/meson.build +++ b/meson.build @@ -718,6 +718,11 @@ if not get_option('virglrenderer').auto() or have_system or have_vhost_user_gpu method: 'pkg-config', required: get_option('virglrenderer'), kwargs: static_kwargs) + + config_host_data.set('HAVE_VIRGL_CONTEXT_INIT', + cc.has_function('virgl_renderer_context_create_with_flags', + prefix: '#include <virglrenderer.h>', + dependencies: virgl)) endif curl = not_found if not get_option('curl').auto() or have_block
Create virgl renderer context with flags using context_id when valid. v2: - The feature can be enabled via the context_init config option. - A warning message will be emitted and the feature will not be used when linking with virglrenderer versions without context_init support. v3: Define HAVE_VIRGL_CONTEXT_INIT in config_host_data. Signed-off-by: Antonio Caggiano <antonio.caggiano@collabora.com> --- hw/display/virtio-gpu-base.c | 3 +++ hw/display/virtio-gpu-virgl.c | 16 ++++++++++++++-- hw/display/virtio-gpu.c | 2 ++ include/hw/virtio/virtio-gpu.h | 3 +++ meson.build | 5 +++++ 5 files changed, 27 insertions(+), 2 deletions(-)