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