drm/virtio: add new virtio gpu capset definitions

[PATCH v3] drm/virtio: add new virtio gpu capset definitions

Posted by Huang Rui 2 years, 2 months ago

These definitions are used fro qemu, and qemu imports this marco in the
headers to enable gfxstream, venus, cross domain, and drm (native
context) for virtio gpu. So it should add them even kernel doesn't use
this.

Signed-off-by: Huang Rui <ray.huang@amd.com>
Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---

Changes V1 -> V2:
- Add all capsets including gfxstream and venus in kernel header (Dmitry Osipenko)

Changes V2 -> V3:
- Add missed capsets including cross domain and drm (native context)
  (Dmitry Osipenko)

v1: https://lore.kernel.org/lkml/20230915105918.3763061-1-ray.huang@amd.com/
v2: https://lore.kernel.org/lkml/20231010032553.1138036-1-ray.huang@amd.com/

 include/uapi/linux/virtio_gpu.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/uapi/linux/virtio_gpu.h b/include/uapi/linux/virtio_gpu.h
index f556fde07b76..240911c8da31 100644
--- a/include/uapi/linux/virtio_gpu.h
+++ b/include/uapi/linux/virtio_gpu.h
@@ -309,6 +309,10 @@ struct virtio_gpu_cmd_submit {
 
 #define VIRTIO_GPU_CAPSET_VIRGL 1
 #define VIRTIO_GPU_CAPSET_VIRGL2 2
+#define VIRTIO_GPU_CAPSET_GFXSTREAM 3
+#define VIRTIO_GPU_CAPSET_VENUS 4
+#define VIRTIO_GPU_CAPSET_CROSS_DOMAIN 5
+#define VIRTIO_GPU_CAPSET_DRM 6
 
 /* VIRTIO_GPU_CMD_GET_CAPSET_INFO */
 struct virtio_gpu_get_capset_info {
-- 
2.25.1

Re: [PATCH v3] drm/virtio: add new virtio gpu capset definitions

Posted by Dmitry Osipenko 2 years, 2 months ago

On 10/10/23 16:57, Huang Rui wrote:
> These definitions are used fro qemu, and qemu imports this marco in the
> headers to enable gfxstream, venus, cross domain, and drm (native
> context) for virtio gpu. So it should add them even kernel doesn't use
> this.
> 
> Signed-off-by: Huang Rui <ray.huang@amd.com>
> Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
> 
> Changes V1 -> V2:
> - Add all capsets including gfxstream and venus in kernel header (Dmitry Osipenko)
> 
> Changes V2 -> V3:
> - Add missed capsets including cross domain and drm (native context)
>   (Dmitry Osipenko)
> 
> v1: https://lore.kernel.org/lkml/20230915105918.3763061-1-ray.huang@amd.com/
> v2: https://lore.kernel.org/lkml/20231010032553.1138036-1-ray.huang@amd.com/
> 
>  include/uapi/linux/virtio_gpu.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/uapi/linux/virtio_gpu.h b/include/uapi/linux/virtio_gpu.h
> index f556fde07b76..240911c8da31 100644
> --- a/include/uapi/linux/virtio_gpu.h
> +++ b/include/uapi/linux/virtio_gpu.h
> @@ -309,6 +309,10 @@ struct virtio_gpu_cmd_submit {
>  
>  #define VIRTIO_GPU_CAPSET_VIRGL 1
>  #define VIRTIO_GPU_CAPSET_VIRGL2 2
> +#define VIRTIO_GPU_CAPSET_GFXSTREAM 3

The GFXSTREAM capset isn't correct, it should be GFXSTREAM_VULKAN in
accordance to [1] and [2]. There are more capsets for GFXSTREAM.

[1]
https://github.com/google/crosvm/blob/main/rutabaga_gfx/src/rutabaga_utils.rs#L172

[2]
https://patchwork.kernel.org/project/qemu-devel/patch/20231006010835.444-7-gurchetansingh@chromium.org/

-- 
Best regards,
Dmitry

Re: [PATCH v3] drm/virtio: add new virtio gpu capset definitions

Posted by Dmitry Osipenko 2 years, 2 months ago

On 10/10/23 18:40, Dmitry Osipenko wrote:
> On 10/10/23 16:57, Huang Rui wrote:
>> These definitions are used fro qemu, and qemu imports this marco in the
>> headers to enable gfxstream, venus, cross domain, and drm (native
>> context) for virtio gpu. So it should add them even kernel doesn't use
>> this.
>>
>> Signed-off-by: Huang Rui <ray.huang@amd.com>
>> Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>> ---
>>
>> Changes V1 -> V2:
>> - Add all capsets including gfxstream and venus in kernel header (Dmitry Osipenko)
>>
>> Changes V2 -> V3:
>> - Add missed capsets including cross domain and drm (native context)
>>   (Dmitry Osipenko)
>>
>> v1: https://lore.kernel.org/lkml/20230915105918.3763061-1-ray.huang@amd.com/
>> v2: https://lore.kernel.org/lkml/20231010032553.1138036-1-ray.huang@amd.com/
>>
>>  include/uapi/linux/virtio_gpu.h | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/include/uapi/linux/virtio_gpu.h b/include/uapi/linux/virtio_gpu.h
>> index f556fde07b76..240911c8da31 100644
>> --- a/include/uapi/linux/virtio_gpu.h
>> +++ b/include/uapi/linux/virtio_gpu.h
>> @@ -309,6 +309,10 @@ struct virtio_gpu_cmd_submit {
>>  
>>  #define VIRTIO_GPU_CAPSET_VIRGL 1
>>  #define VIRTIO_GPU_CAPSET_VIRGL2 2
>> +#define VIRTIO_GPU_CAPSET_GFXSTREAM 3
> 
> The GFXSTREAM capset isn't correct, it should be GFXSTREAM_VULKAN in
> accordance to [1] and [2]. There are more capsets for GFXSTREAM.
> 
> [1]
> https://github.com/google/crosvm/blob/main/rutabaga_gfx/src/rutabaga_utils.rs#L172
> 
> [2]
> https://patchwork.kernel.org/project/qemu-devel/patch/20231006010835.444-7-gurchetansingh@chromium.org/

Though, maybe those are "rutabaga" capsets that not related to
virtio-gpu because crosvm has another defs for virtio-gpu capsets [3].
The DRM capset is oddly missing in [3] and code uses "rutabaga" capset
for DRM and virtio-gpu.

[3]
https://github.com/google/crosvm/blob/main/devices/src/virtio/gpu/protocol.rs#L416

Gurchetan, could you please clarify which capsets definitions are
related to virtio-gpu and gfxstream. The
GFXSTREAM_VULKAN/GLES/MAGMA/COMPOSER or just the single GFXSTREAM?

-- 
Best regards,
Dmitry

Re: [PATCH v3] drm/virtio: add new virtio gpu capset definitions

Posted by Huang Rui 2 years, 2 months ago

On Tue, Oct 10, 2023 at 11:52:14PM +0800, Dmitry Osipenko wrote:
> On 10/10/23 18:40, Dmitry Osipenko wrote:
> > On 10/10/23 16:57, Huang Rui wrote:
> >> These definitions are used fro qemu, and qemu imports this marco in the
> >> headers to enable gfxstream, venus, cross domain, and drm (native
> >> context) for virtio gpu. So it should add them even kernel doesn't use
> >> this.
> >>
> >> Signed-off-by: Huang Rui <ray.huang@amd.com>
> >> Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> >> ---
> >>
> >> Changes V1 -> V2:
> >> - Add all capsets including gfxstream and venus in kernel header (Dmitry Osipenko)
> >>
> >> Changes V2 -> V3:
> >> - Add missed capsets including cross domain and drm (native context)
> >>   (Dmitry Osipenko)
> >>
> >> v1: https://lore.kernel.org/lkml/20230915105918.3763061-1-ray.huang@amd.com/
> >> v2: https://lore.kernel.org/lkml/20231010032553.1138036-1-ray.huang@amd.com/
> >>
> >>  include/uapi/linux/virtio_gpu.h | 4 ++++
> >>  1 file changed, 4 insertions(+)
> >>
> >> diff --git a/include/uapi/linux/virtio_gpu.h b/include/uapi/linux/virtio_gpu.h
> >> index f556fde07b76..240911c8da31 100644
> >> --- a/include/uapi/linux/virtio_gpu.h
> >> +++ b/include/uapi/linux/virtio_gpu.h
> >> @@ -309,6 +309,10 @@ struct virtio_gpu_cmd_submit {
> >>  
> >>  #define VIRTIO_GPU_CAPSET_VIRGL 1
> >>  #define VIRTIO_GPU_CAPSET_VIRGL2 2
> >> +#define VIRTIO_GPU_CAPSET_GFXSTREAM 3
> > 
> > The GFXSTREAM capset isn't correct, it should be GFXSTREAM_VULKAN in
> > accordance to [1] and [2]. There are more capsets for GFXSTREAM.
> > 
> > [1]
> > https://github.com/google/crosvm/blob/main/rutabaga_gfx/src/rutabaga_utils.rs#L172
> > 
> > [2]
> > https://patchwork.kernel.org/project/qemu-devel/patch/20231006010835.444-7-gurchetansingh@chromium.org/
> 
> Though, maybe those are "rutabaga" capsets that not related to
> virtio-gpu because crosvm has another defs for virtio-gpu capsets [3].
> The DRM capset is oddly missing in [3] and code uses "rutabaga" capset
> for DRM and virtio-gpu.
> 
> [3]
> https://github.com/google/crosvm/blob/main/devices/src/virtio/gpu/protocol.rs#L416

Yes, [3] is the file that I referred to add these capsets definitions. And
it's defined as gfxstream not gfxstream_vulkan.

> 
> Gurchetan, could you please clarify which capsets definitions are
> related to virtio-gpu and gfxstream. The
> GFXSTREAM_VULKAN/GLES/MAGMA/COMPOSER or just the single GFXSTREAM?
> 

Gurchetan, may we have your insight?

Thanks,
Ray

> -- 
> Best regards,
> Dmitry
>