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 <[email protected]>
Reviewed-by: Akihiko Odaki <[email protected]>
---
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/[email protected]/
v2: https://lore.kernel.org/lkml/[email protected]/
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
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 <[email protected]>
> Reviewed-by: Akihiko Odaki <[email protected]>
> ---
>
> 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/[email protected]/
> v2: https://lore.kernel.org/lkml/[email protected]/
>
> 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/[email protected]/
--
Best regards,
Dmitry
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 <[email protected]>
>> Reviewed-by: Akihiko Odaki <[email protected]>
>> ---
>>
>> 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/[email protected]/
>> v2: https://lore.kernel.org/lkml/[email protected]/
>>
>> 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/[email protected]/
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
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 <[email protected]>
> >> Reviewed-by: Akihiko Odaki <[email protected]>
> >> ---
> >>
> >> 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/[email protected]/
> >> v2: https://lore.kernel.org/lkml/[email protected]/
> >>
> >> 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/[email protected]/
>
> 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
>
On 10/19/23 02:25, Gurchetan Singh wrote:
> On Tue, Oct 10, 2023 at 9:41 PM Huang Rui <[email protected]> wrote:
>
>> 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 <[email protected]>
>>>>> Reviewed-by: Akihiko Odaki <[email protected]>
>>>>> ---
>>>>>
>>>>> 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/[email protected]/
>>>>> v2:
>> https://lore.kernel.org/lkml/[email protected]/
>>>>>
>>>>> 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/[email protected]/
>>>
>>> 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?
>
>
> It should be GFXSTREAM_VULKAN. The rest are more experimental and easy to
> modify in terms of the enum value, should the need arise.
>
> I imagine the virtio-spec update to reflect the GFXSTREAM to
> GFXSTREAM_VULKAN change will happen eventually.
Thanks for the clarification. Good point about the spec updating, we
should document DRM context too,
--
Best regards,
Dmitry
On 2023/10/19 8:25, Gurchetan Singh wrote:
>
>
> On Tue, Oct 10, 2023 at 9:41 PM Huang Rui <[email protected]
> <mailto:[email protected]>> wrote:
>
> 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 <[email protected]
> <mailto:[email protected]>>
> > >> Reviewed-by: Akihiko Odaki <[email protected]
> <mailto:[email protected]>>
> > >> ---
> > >>
> > >> 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/[email protected]/ <https://lore.kernel.org/lkml/[email protected]/>
> > >> v2:
> https://lore.kernel.org/lkml/[email protected]/ <https://lore.kernel.org/lkml/[email protected]/>
> > >>
> > >> 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 <https://github.com/google/crosvm/blob/main/rutabaga_gfx/src/rutabaga_utils.rs#L172>
> > >
> > > [2]
> > >
> https://patchwork.kernel.org/project/qemu-devel/patch/[email protected]/ <https://patchwork.kernel.org/project/qemu-devel/patch/[email protected]/>
> >
> > 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 <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?
>
>
> It should be GFXSTREAM_VULKAN. The rest are more experimental and easy
> to modify in terms of the enum value, should the need arise.
>
> I imagine the virtio-spec update to reflect the GFXSTREAM to
> GFXSTREAM_VULKAN change will happen eventually.
I think this is a matter what the committee should determine, but in
general I don't think it is OK to change the existing identifier.
I also think even experimental values should be added to virtio spec at
an early stage unless such an "experiment" is done only on one laptop.
We can obsolete a capset anytime so it's more important to avoid
conflicts of capsets.