| Message ID | 20240511182251.1442078-10-dmitry.osipenko@collabora.com |
|---|---|
| State | New |
| Headers | show |
| Series | Support blob memory and venus on qemu | expand |
On 2024/05/12 3:22, 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-gl.c | 6 ++++-- > hw/display/virtio-gpu-virgl.c | 33 +++++++++++++++++++++------------ > include/hw/virtio/virtio-gpu.h | 4 +++- > 3 files changed, 28 insertions(+), 15 deletions(-) > > diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c > index 95806999189e..431fc2881a00 100644 > --- a/hw/display/virtio-gpu-gl.c > +++ b/hw/display/virtio-gpu-gl.c > @@ -124,8 +124,8 @@ static void virtio_gpu_gl_device_realize(DeviceState *qdev, Error **errp) > } > > g->parent_obj.conf.flags |= (1 << VIRTIO_GPU_FLAG_VIRGL_ENABLED); > - VIRTIO_GPU_BASE(g)->virtio_config.num_capsets = > - virtio_gpu_virgl_get_num_capsets(g); > + g->capset_ids = virtio_gpu_virgl_get_capsets(g); > + VIRTIO_GPU_BASE(g)->virtio_config.num_capsets = g->capset_ids->len; > > #ifdef HAVE_VIRGL_CONTEXT_CREATE_WITH_FLAGS > g->parent_obj.conf.flags |= 1 << VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED; > @@ -148,6 +148,8 @@ static void virtio_gpu_gl_device_unrealize(DeviceState *qdev) > if (gl->renderer_inited) { > virtio_gpu_virgl_deinit(g); > } > + > + g_array_unref(g->capset_ids); > } > > static void virtio_gpu_gl_class_init(ObjectClass *klass, void *data) > diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c > index 3f2e406be3a4..135974431492 100644 > --- a/hw/display/virtio-gpu-virgl.c > +++ b/hw/display/virtio-gpu-virgl.c > @@ -590,19 +590,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)); > @@ -1123,14 +1117,29 @@ int virtio_gpu_virgl_init(VirtIOGPU *g) > return 0; > } > > -int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g) > +static void virtio_gpu_virgl_add_capset(GArray *capset_ids, uint32_t capset_id) > +{ > + g_array_append_val(capset_ids, capset_id); > +} Is it worthwhile to have a function for this?
On 5/13/24 12:20, Akihiko Odaki wrote: ... >> -int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g) >> +static void virtio_gpu_virgl_add_capset(GArray *capset_ids, uint32_t >> capset_id) >> +{ >> + g_array_append_val(capset_ids, capset_id); >> +} > > Is it worthwhile to have a function for this? It's necessary to have it because g_array_append_val() is actually a macro that takes &capset_id ptr internally. I.e. g_array_append_val(capset_ids, VIRTIO_GPU_CAPSET_VIRGL2) will fail to compile with: /usr/include/glib-2.0/glib/garray.h:66:59: error: lvalue required as unary '&' operand 66 | #define g_array_append_val(a,v) g_array_append_vals (a, &(v), 1)
diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c index 95806999189e..431fc2881a00 100644 --- a/hw/display/virtio-gpu-gl.c +++ b/hw/display/virtio-gpu-gl.c @@ -124,8 +124,8 @@ static void virtio_gpu_gl_device_realize(DeviceState *qdev, Error **errp) } g->parent_obj.conf.flags |= (1 << VIRTIO_GPU_FLAG_VIRGL_ENABLED); - VIRTIO_GPU_BASE(g)->virtio_config.num_capsets = - virtio_gpu_virgl_get_num_capsets(g); + g->capset_ids = virtio_gpu_virgl_get_capsets(g); + VIRTIO_GPU_BASE(g)->virtio_config.num_capsets = g->capset_ids->len; #ifdef HAVE_VIRGL_CONTEXT_CREATE_WITH_FLAGS g->parent_obj.conf.flags |= 1 << VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED; @@ -148,6 +148,8 @@ static void virtio_gpu_gl_device_unrealize(DeviceState *qdev) if (gl->renderer_inited) { virtio_gpu_virgl_deinit(g); } + + g_array_unref(g->capset_ids); } static void virtio_gpu_gl_class_init(ObjectClass *klass, void *data) diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c index 3f2e406be3a4..135974431492 100644 --- a/hw/display/virtio-gpu-virgl.c +++ b/hw/display/virtio-gpu-virgl.c @@ -590,19 +590,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)); @@ -1123,14 +1117,29 @@ int virtio_gpu_virgl_init(VirtIOGPU *g) return 0; } -int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g) +static void virtio_gpu_virgl_add_capset(GArray *capset_ids, uint32_t capset_id) +{ + g_array_append_val(capset_ids, capset_id); +} + +GArray *virtio_gpu_virgl_get_capsets(VirtIOGPU *g) { uint32_t capset2_max_ver, capset2_max_size; + GArray *capset_ids; + + capset_ids = g_array_new(false, false, sizeof(uint32_t)); + + /* VIRGL is always supported. */ + virtio_gpu_virgl_add_capset(capset_ids, VIRTIO_GPU_CAPSET_VIRGL); + virgl_renderer_get_cap_set(VIRTIO_GPU_CAPSET_VIRGL2, &capset2_max_ver, &capset2_max_size); + if (capset2_max_ver) { + virtio_gpu_virgl_add_capset(capset_ids, VIRTIO_GPU_CAPSET_VIRGL2); + } - return capset2_max_ver ? 2 : 1; + return capset_ids; } void virtio_gpu_virgl_deinit(VirtIOGPU *g) diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h index c0b0b0eac08b..7bbc6ef268eb 100644 --- a/include/hw/virtio/virtio-gpu.h +++ b/include/hw/virtio/virtio-gpu.h @@ -212,6 +212,8 @@ struct VirtIOGPU { } dmabuf; QEMUBH *cmdq_resume_bh; + + GArray *capset_ids; }; struct VirtIOGPUClass { @@ -346,6 +348,6 @@ void virtio_gpu_virgl_reset_scanout(VirtIOGPU *g); void virtio_gpu_virgl_reset(VirtIOGPU *g); int virtio_gpu_virgl_init(VirtIOGPU *g); void virtio_gpu_virgl_deinit(VirtIOGPU *g); -int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g); +GArray *virtio_gpu_virgl_get_capsets(VirtIOGPU *g); #endif