Re: [PATCH v9 09/11] virtio-gpu: Register capsets dynamically

From: Akihiko Odaki <akihiko.odaki@daynix.com>
To: "Dmitry Osipenko" <dmitry.osipenko@collabora.com>,
	"Huang Rui" <ray.huang@amd.com>,
	"Marc-André Lureau" <marcandre.lureau@gmail.com>,
	"Philippe Mathieu-Daudé" <philmd@linaro.org>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Michael S . Tsirkin" <mst@redhat.com>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"Anthony PERARD" <anthony.perard@citrix.com>,
	"Antonio Caggiano" <quic_acaggian@quicinc.com>,
	"Dr . David Alan Gilbert" <dgilbert@redhat.com>,
	"Robert Beckett" <bob.beckett@collabora.com>,
	"Gert Wollny" <gert.wollny@collabora.com>,
	"Alex Bennée" <alex.bennee@linaro.org>
Cc: qemu-devel@nongnu.org,
	"Gurchetan Singh" <gurchetansingh@chromium.org>,
	ernunes@redhat.com, "Alyssa Ross" <hi@alyssa.is>,
	"Roger Pau Monné" <roger.pau@citrix.com>,
	"Alex Deucher" <alexander.deucher@amd.com>,
	"Stefano Stabellini" <stefano.stabellini@amd.com>,
	"Christian König" <christian.koenig@amd.com>,
	"Xenia Ragiadakou" <xenia.ragiadakou@amd.com>,
	"Pierre-Eric Pelloux-Prayer" <pierre-eric.pelloux-prayer@amd.com>,
	"Honglei Huang" <honglei1.huang@amd.com>,
	"Julia Zhang" <julia.zhang@amd.com>,
	"Chen Jiqian" <Jiqian.Chen@amd.com>,
	"Yiwei Zhang" <zzyiwei@chromium.org>
Subject: Re: [PATCH v9 09/11] virtio-gpu: Register capsets dynamically
Date: Sat, 27 Apr 2024 16:12:31 +0900	[thread overview]
Message-ID: <557f5ddb-4326-48bc-b34e-bda9d51cadcc@daynix.com> (raw)
In-Reply-To: <20240425154539.2680550-10-dmitry.osipenko@collabora.com>

On 2024/04/26 0:45, Dmitry Osipenko wrote:
> From: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
> 
> virtio_gpu_virgl_get_num_capsets will return "num_capsets", but we can't
> assume that capset_index 1 is always VIRGL2 once we'll support more capsets,
> like Venus and DRM capsets. Register capsets dynamically to avoid that problem.
> 
> Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> ---
>   hw/display/virtio-gpu-virgl.c  | 34 +++++++++++++++++++++++-----------
>   include/hw/virtio/virtio-gpu.h |  2 ++
>   2 files changed, 25 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> index de788df155bf..9aa1fd78f1e1 100644
> --- a/hw/display/virtio-gpu-virgl.c
> +++ b/hw/display/virtio-gpu-virgl.c
> @@ -597,19 +597,13 @@ static void virgl_cmd_get_capset_info(VirtIOGPU *g,
>       VIRTIO_GPU_FILL_CMD(info);
>   
>       memset(&resp, 0, sizeof(resp));
> -    if (info.capset_index == 0) {
> -        resp.capset_id = VIRTIO_GPU_CAPSET_VIRGL;
> -        virgl_renderer_get_cap_set(resp.capset_id,
> -                                   &resp.capset_max_version,
> -                                   &resp.capset_max_size);
> -    } else if (info.capset_index == 1) {
> -        resp.capset_id = VIRTIO_GPU_CAPSET_VIRGL2;
> +
> +    if (info.capset_index < g->capset_ids->len) {
> +        resp.capset_id = g_array_index(g->capset_ids, uint32_t,
> +                                       info.capset_index);
>           virgl_renderer_get_cap_set(resp.capset_id,
>                                      &resp.capset_max_version,
>                                      &resp.capset_max_size);
> -    } else {
> -        resp.capset_max_version = 0;
> -        resp.capset_max_size = 0;
>       }
>       resp.hdr.type = VIRTIO_GPU_RESP_OK_CAPSET_INFO;
>       virtio_gpu_ctrl_response(g, cmd, &resp.hdr, sizeof(resp));
> @@ -1159,12 +1153,30 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
>       return 0;
>   }
>   
> +static void virtio_gpu_virgl_add_capset(VirtIOGPU *g, uint32_t capset_id)
> +{
> +    g_array_append_val(g->capset_ids, capset_id);
> +}
> +
>   int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g)
>   {
>       uint32_t capset2_max_ver, capset2_max_size;
> +
> +    if (g->capset_ids) {

Move capset_ids initialization to virtio_gpu_virgl_init() to save this 
conditional. capset_ids also needs to be freed when the device gets 
unrealized.