* Re: [PATCH] drm/panthor: Convert IOCTL defines to an enum
2025-02-04 23:28 [PATCH] drm/panthor: Convert IOCTL defines to an enum Rob Herring (Arm)
@ 2025-02-05 10:39 ` Alice Ryhl
2025-02-05 10:53 ` Erik Faye-Lund
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Alice Ryhl @ 2025-02-05 10:39 UTC (permalink / raw)
To: Rob Herring (Arm)
Cc: Boris Brezillon, Steven Price, Liviu Dudau, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Beata Michalska, dri-devel, linux-kernel,
rust-for-linux
On Wed, Feb 5, 2025 at 12:30 AM Rob Herring (Arm) <robh@kernel.org> wrote:
>
> Use an enum instead of #defines for panthor IOCTLs. This allows the
> header to be used with Rust code as bindgen can't handle complex
> defines.
>
> Cc: Beata Michalska <beata.michalska@arm.com>
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] drm/panthor: Convert IOCTL defines to an enum
2025-02-04 23:28 [PATCH] drm/panthor: Convert IOCTL defines to an enum Rob Herring (Arm)
2025-02-05 10:39 ` Alice Ryhl
@ 2025-02-05 10:53 ` Erik Faye-Lund
2025-02-05 14:16 ` Erik Faye-Lund
2025-02-05 12:51 ` Boris Brezillon
` (2 subsequent siblings)
4 siblings, 1 reply; 8+ messages in thread
From: Erik Faye-Lund @ 2025-02-05 10:53 UTC (permalink / raw)
To: Rob Herring (Arm), Boris Brezillon, Steven Price, Liviu Dudau,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross
Cc: Beata Michalska, dri-devel, linux-kernel, rust-for-linux
On Tue, 2025-02-04 at 17:28 -0600, Rob Herring (Arm) wrote:
> Use an enum instead of #defines for panthor IOCTLs. This allows the
> header to be used with Rust code as bindgen can't handle complex
> defines.
>
Unfortunately, this goes in the opposite direction than what I was
asked to do here:
https://lore.kernel.org/dri-devel/20241029140125.0607c26f@collabora.com/
...I still intend to get around to doing that, because we have problems
with C enum and large values. I don't know if we can solve that while
making Rust happy without requiring C23 (which allows to specify the
underlying type of an enum), unfortunately...
> Cc: Beata Michalska <beata.michalska@arm.com>
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> ---
> include/uapi/drm/panthor_drm.h | 86 +++++++++++++++++---------------
> --
> 1 file changed, 44 insertions(+), 42 deletions(-)
>
> diff --git a/include/uapi/drm/panthor_drm.h
> b/include/uapi/drm/panthor_drm.h
> index b99763cbae48..97e2c4510e69 100644
> --- a/include/uapi/drm/panthor_drm.h
> +++ b/include/uapi/drm/panthor_drm.h
> @@ -129,48 +129,6 @@ enum drm_panthor_ioctl_id {
> DRM_PANTHOR_TILER_HEAP_DESTROY,
> };
>
> -/**
> - * DRM_IOCTL_PANTHOR() - Build a Panthor IOCTL number
> - * @__access: Access type. Must be R, W or RW.
> - * @__id: One of the DRM_PANTHOR_xxx id.
> - * @__type: Suffix of the type being passed to the IOCTL.
> - *
> - * Don't use this macro directly, use the DRM_IOCTL_PANTHOR_xxx
> - * values instead.
> - *
> - * Return: An IOCTL number to be passed to ioctl() from userspace.
> - */
> -#define DRM_IOCTL_PANTHOR(__access, __id, __type) \
> - DRM_IO ## __access(DRM_COMMAND_BASE + DRM_PANTHOR_ ## __id,
> \
> - struct drm_panthor_ ## __type)
> -
> -#define DRM_IOCTL_PANTHOR_DEV_QUERY \
> - DRM_IOCTL_PANTHOR(WR, DEV_QUERY, dev_query)
> -#define DRM_IOCTL_PANTHOR_VM_CREATE \
> - DRM_IOCTL_PANTHOR(WR, VM_CREATE, vm_create)
> -#define DRM_IOCTL_PANTHOR_VM_DESTROY \
> - DRM_IOCTL_PANTHOR(WR, VM_DESTROY, vm_destroy)
> -#define DRM_IOCTL_PANTHOR_VM_BIND \
> - DRM_IOCTL_PANTHOR(WR, VM_BIND, vm_bind)
> -#define DRM_IOCTL_PANTHOR_VM_GET_STATE \
> - DRM_IOCTL_PANTHOR(WR, VM_GET_STATE, vm_get_state)
> -#define DRM_IOCTL_PANTHOR_BO_CREATE \
> - DRM_IOCTL_PANTHOR(WR, BO_CREATE, bo_create)
> -#define DRM_IOCTL_PANTHOR_BO_MMAP_OFFSET \
> - DRM_IOCTL_PANTHOR(WR, BO_MMAP_OFFSET, bo_mmap_offset)
> -#define DRM_IOCTL_PANTHOR_GROUP_CREATE \
> - DRM_IOCTL_PANTHOR(WR, GROUP_CREATE, group_create)
> -#define DRM_IOCTL_PANTHOR_GROUP_DESTROY \
> - DRM_IOCTL_PANTHOR(WR, GROUP_DESTROY, group_destroy)
> -#define DRM_IOCTL_PANTHOR_GROUP_SUBMIT \
> - DRM_IOCTL_PANTHOR(WR, GROUP_SUBMIT, group_submit)
> -#define DRM_IOCTL_PANTHOR_GROUP_GET_STATE \
> - DRM_IOCTL_PANTHOR(WR, GROUP_GET_STATE, group_get_state)
> -#define DRM_IOCTL_PANTHOR_TILER_HEAP_CREATE \
> - DRM_IOCTL_PANTHOR(WR, TILER_HEAP_CREATE, tiler_heap_create)
> -#define DRM_IOCTL_PANTHOR_TILER_HEAP_DESTROY \
> - DRM_IOCTL_PANTHOR(WR, TILER_HEAP_DESTROY,
> tiler_heap_destroy)
> -
> /**
> * DOC: IOCTL arguments
> */
> @@ -1019,6 +977,50 @@ struct drm_panthor_tiler_heap_destroy {
> __u32 pad;
> };
>
> +/**
> + * DRM_IOCTL_PANTHOR() - Build a Panthor IOCTL number
> + * @__access: Access type. Must be R, W or RW.
> + * @__id: One of the DRM_PANTHOR_xxx id.
> + * @__type: Suffix of the type being passed to the IOCTL.
> + *
> + * Don't use this macro directly, use the DRM_IOCTL_PANTHOR_xxx
> + * values instead.
> + *
> + * Return: An IOCTL number to be passed to ioctl() from userspace.
> + */
> +#define DRM_IOCTL_PANTHOR(__access, __id, __type) \
> + DRM_IO ## __access(DRM_COMMAND_BASE + DRM_PANTHOR_ ## __id,
> \
> + struct drm_panthor_ ## __type)
> +
> +enum {
> + DRM_IOCTL_PANTHOR_DEV_QUERY =
> + DRM_IOCTL_PANTHOR(WR, DEV_QUERY, dev_query),
> + DRM_IOCTL_PANTHOR_VM_CREATE =
> + DRM_IOCTL_PANTHOR(WR, VM_CREATE, vm_create),
> + DRM_IOCTL_PANTHOR_VM_DESTROY =
> + DRM_IOCTL_PANTHOR(WR, VM_DESTROY, vm_destroy),
> + DRM_IOCTL_PANTHOR_VM_BIND =
> + DRM_IOCTL_PANTHOR(WR, VM_BIND, vm_bind),
> + DRM_IOCTL_PANTHOR_VM_GET_STATE =
> + DRM_IOCTL_PANTHOR(WR, VM_GET_STATE, vm_get_state),
> + DRM_IOCTL_PANTHOR_BO_CREATE =
> + DRM_IOCTL_PANTHOR(WR, BO_CREATE, bo_create),
> + DRM_IOCTL_PANTHOR_BO_MMAP_OFFSET =
> + DRM_IOCTL_PANTHOR(WR, BO_MMAP_OFFSET,
> bo_mmap_offset),
> + DRM_IOCTL_PANTHOR_GROUP_CREATE =
> + DRM_IOCTL_PANTHOR(WR, GROUP_CREATE, group_create),
> + DRM_IOCTL_PANTHOR_GROUP_DESTROY =
> + DRM_IOCTL_PANTHOR(WR, GROUP_DESTROY, group_destroy),
> + DRM_IOCTL_PANTHOR_GROUP_SUBMIT =
> + DRM_IOCTL_PANTHOR(WR, GROUP_SUBMIT, group_submit),
> + DRM_IOCTL_PANTHOR_GROUP_GET_STATE =
> + DRM_IOCTL_PANTHOR(WR, GROUP_GET_STATE,
> group_get_state),
> + DRM_IOCTL_PANTHOR_TILER_HEAP_CREATE =
> + DRM_IOCTL_PANTHOR(WR, TILER_HEAP_CREATE,
> tiler_heap_create),
> + DRM_IOCTL_PANTHOR_TILER_HEAP_DESTROY =
> + DRM_IOCTL_PANTHOR(WR, TILER_HEAP_DESTROY,
> tiler_heap_destroy),
> +};
> +
> #if defined(__cplusplus)
> }
> #endif
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] drm/panthor: Convert IOCTL defines to an enum
2025-02-05 10:53 ` Erik Faye-Lund
@ 2025-02-05 14:16 ` Erik Faye-Lund
2025-02-05 16:43 ` Rob Herring
0 siblings, 1 reply; 8+ messages in thread
From: Erik Faye-Lund @ 2025-02-05 14:16 UTC (permalink / raw)
To: Rob Herring (Arm), Boris Brezillon, Steven Price, Liviu Dudau,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross
Cc: Beata Michalska, dri-devel, linux-kernel, rust-for-linux
On Wed, 2025-02-05 at 11:53 +0100, Erik Faye-Lund wrote:
> On Tue, 2025-02-04 at 17:28 -0600, Rob Herring (Arm) wrote:
> > Use an enum instead of #defines for panthor IOCTLs. This allows the
> > header to be used with Rust code as bindgen can't handle complex
> > defines.
> >
>
>
> Unfortunately, this goes in the opposite direction than what I was
> asked to do here:
>
> https://lore.kernel.org/dri-devel/20241029140125.0607c26f@collabora.com/
>
> ...I still intend to get around to doing that, because we have
> problems
> with C enum and large values. I don't know if we can solve that while
> making Rust happy without requiring C23 (which allows to specify the
> underlying type of an enum), unfortunately...
>
Seems I misunderstood Boris, and he was referring to flags. The enums
are of course fine to make into real enums.
> > Cc: Beata Michalska <beata.michalska@arm.com>
> > Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> > ---
> > include/uapi/drm/panthor_drm.h | 86 +++++++++++++++++-------------
> > --
> > --
> > 1 file changed, 44 insertions(+), 42 deletions(-)
> >
> > diff --git a/include/uapi/drm/panthor_drm.h
> > b/include/uapi/drm/panthor_drm.h
> > index b99763cbae48..97e2c4510e69 100644
> > --- a/include/uapi/drm/panthor_drm.h
> > +++ b/include/uapi/drm/panthor_drm.h
> > @@ -129,48 +129,6 @@ enum drm_panthor_ioctl_id {
> > DRM_PANTHOR_TILER_HEAP_DESTROY,
> > };
> >
> > -/**
> > - * DRM_IOCTL_PANTHOR() - Build a Panthor IOCTL number
> > - * @__access: Access type. Must be R, W or RW.
> > - * @__id: One of the DRM_PANTHOR_xxx id.
> > - * @__type: Suffix of the type being passed to the IOCTL.
> > - *
> > - * Don't use this macro directly, use the DRM_IOCTL_PANTHOR_xxx
> > - * values instead.
> > - *
> > - * Return: An IOCTL number to be passed to ioctl() from userspace.
> > - */
> > -#define DRM_IOCTL_PANTHOR(__access, __id, __type) \
> > - DRM_IO ## __access(DRM_COMMAND_BASE + DRM_PANTHOR_ ##
> > __id,
> > \
> > - struct drm_panthor_ ## __type)
> > -
> > -#define DRM_IOCTL_PANTHOR_DEV_QUERY \
> > - DRM_IOCTL_PANTHOR(WR, DEV_QUERY, dev_query)
> > -#define DRM_IOCTL_PANTHOR_VM_CREATE \
> > - DRM_IOCTL_PANTHOR(WR, VM_CREATE, vm_create)
> > -#define DRM_IOCTL_PANTHOR_VM_DESTROY \
> > - DRM_IOCTL_PANTHOR(WR, VM_DESTROY, vm_destroy)
> > -#define DRM_IOCTL_PANTHOR_VM_BIND \
> > - DRM_IOCTL_PANTHOR(WR, VM_BIND, vm_bind)
> > -#define DRM_IOCTL_PANTHOR_VM_GET_STATE \
> > - DRM_IOCTL_PANTHOR(WR, VM_GET_STATE, vm_get_state)
> > -#define DRM_IOCTL_PANTHOR_BO_CREATE \
> > - DRM_IOCTL_PANTHOR(WR, BO_CREATE, bo_create)
> > -#define DRM_IOCTL_PANTHOR_BO_MMAP_OFFSET \
> > - DRM_IOCTL_PANTHOR(WR, BO_MMAP_OFFSET, bo_mmap_offset)
> > -#define DRM_IOCTL_PANTHOR_GROUP_CREATE \
> > - DRM_IOCTL_PANTHOR(WR, GROUP_CREATE, group_create)
> > -#define DRM_IOCTL_PANTHOR_GROUP_DESTROY \
> > - DRM_IOCTL_PANTHOR(WR, GROUP_DESTROY, group_destroy)
> > -#define DRM_IOCTL_PANTHOR_GROUP_SUBMIT \
> > - DRM_IOCTL_PANTHOR(WR, GROUP_SUBMIT, group_submit)
> > -#define DRM_IOCTL_PANTHOR_GROUP_GET_STATE \
> > - DRM_IOCTL_PANTHOR(WR, GROUP_GET_STATE, group_get_state)
> > -#define DRM_IOCTL_PANTHOR_TILER_HEAP_CREATE \
> > - DRM_IOCTL_PANTHOR(WR, TILER_HEAP_CREATE,
> > tiler_heap_create)
> > -#define DRM_IOCTL_PANTHOR_TILER_HEAP_DESTROY \
> > - DRM_IOCTL_PANTHOR(WR, TILER_HEAP_DESTROY,
> > tiler_heap_destroy)
> > -
> > /**
> > * DOC: IOCTL arguments
> > */
> > @@ -1019,6 +977,50 @@ struct drm_panthor_tiler_heap_destroy {
> > __u32 pad;
> > };
> >
> > +/**
> > + * DRM_IOCTL_PANTHOR() - Build a Panthor IOCTL number
> > + * @__access: Access type. Must be R, W or RW.
> > + * @__id: One of the DRM_PANTHOR_xxx id.
> > + * @__type: Suffix of the type being passed to the IOCTL.
> > + *
> > + * Don't use this macro directly, use the DRM_IOCTL_PANTHOR_xxx
> > + * values instead.
> > + *
> > + * Return: An IOCTL number to be passed to ioctl() from userspace.
> > + */
> > +#define DRM_IOCTL_PANTHOR(__access, __id, __type) \
> > + DRM_IO ## __access(DRM_COMMAND_BASE + DRM_PANTHOR_ ##
> > __id,
> > \
> > + struct drm_panthor_ ## __type)
> > +
> > +enum {
> > + DRM_IOCTL_PANTHOR_DEV_QUERY =
> > + DRM_IOCTL_PANTHOR(WR, DEV_QUERY, dev_query),
> > + DRM_IOCTL_PANTHOR_VM_CREATE =
> > + DRM_IOCTL_PANTHOR(WR, VM_CREATE, vm_create),
> > + DRM_IOCTL_PANTHOR_VM_DESTROY =
> > + DRM_IOCTL_PANTHOR(WR, VM_DESTROY, vm_destroy),
> > + DRM_IOCTL_PANTHOR_VM_BIND =
> > + DRM_IOCTL_PANTHOR(WR, VM_BIND, vm_bind),
> > + DRM_IOCTL_PANTHOR_VM_GET_STATE =
> > + DRM_IOCTL_PANTHOR(WR, VM_GET_STATE, vm_get_state),
> > + DRM_IOCTL_PANTHOR_BO_CREATE =
> > + DRM_IOCTL_PANTHOR(WR, BO_CREATE, bo_create),
> > + DRM_IOCTL_PANTHOR_BO_MMAP_OFFSET =
> > + DRM_IOCTL_PANTHOR(WR, BO_MMAP_OFFSET,
> > bo_mmap_offset),
> > + DRM_IOCTL_PANTHOR_GROUP_CREATE =
> > + DRM_IOCTL_PANTHOR(WR, GROUP_CREATE, group_create),
> > + DRM_IOCTL_PANTHOR_GROUP_DESTROY =
> > + DRM_IOCTL_PANTHOR(WR, GROUP_DESTROY,
> > group_destroy),
> > + DRM_IOCTL_PANTHOR_GROUP_SUBMIT =
> > + DRM_IOCTL_PANTHOR(WR, GROUP_SUBMIT, group_submit),
> > + DRM_IOCTL_PANTHOR_GROUP_GET_STATE =
> > + DRM_IOCTL_PANTHOR(WR, GROUP_GET_STATE,
> > group_get_state),
> > + DRM_IOCTL_PANTHOR_TILER_HEAP_CREATE =
> > + DRM_IOCTL_PANTHOR(WR, TILER_HEAP_CREATE,
> > tiler_heap_create),
> > + DRM_IOCTL_PANTHOR_TILER_HEAP_DESTROY =
> > + DRM_IOCTL_PANTHOR(WR, TILER_HEAP_DESTROY,
> > tiler_heap_destroy),
> > +};
> > +
> > #if defined(__cplusplus)
> > }
> > #endif
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] drm/panthor: Convert IOCTL defines to an enum
2025-02-05 14:16 ` Erik Faye-Lund
@ 2025-02-05 16:43 ` Rob Herring
0 siblings, 0 replies; 8+ messages in thread
From: Rob Herring @ 2025-02-05 16:43 UTC (permalink / raw)
To: Erik Faye-Lund
Cc: Boris Brezillon, Steven Price, Liviu Dudau, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Beata Michalska, dri-devel, linux-kernel,
rust-for-linux
On Wed, Feb 5, 2025 at 8:17 AM Erik Faye-Lund
<erik.faye-lund@collabora.com> wrote:
>
> On Wed, 2025-02-05 at 11:53 +0100, Erik Faye-Lund wrote:
> > On Tue, 2025-02-04 at 17:28 -0600, Rob Herring (Arm) wrote:
> > > Use an enum instead of #defines for panthor IOCTLs. This allows the
> > > header to be used with Rust code as bindgen can't handle complex
> > > defines.
> > >
> >
> >
> > Unfortunately, this goes in the opposite direction than what I was
> > asked to do here:
> >
> > https://lore.kernel.org/dri-devel/20241029140125.0607c26f@collabora.com/
> >
> > ...I still intend to get around to doing that, because we have
> > problems
> > with C enum and large values. I don't know if we can solve that while
> > making Rust happy without requiring C23 (which allows to specify the
> > underlying type of an enum), unfortunately...
> >
>
> Seems I misunderstood Boris, and he was referring to flags. The enums
> are of course fine to make into real enums.
Maybe we're relying on compiler specific behavior, but that's nothing
new in the kernel. This worked fine for me with both gcc and clang:
#include <stdio.h>
enum foo {
BAR = (1UL << 63),
};
int main(void)
{
printf("%lx\n", BAR);
return 0;
}
Rob
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/panthor: Convert IOCTL defines to an enum
2025-02-04 23:28 [PATCH] drm/panthor: Convert IOCTL defines to an enum Rob Herring (Arm)
2025-02-05 10:39 ` Alice Ryhl
2025-02-05 10:53 ` Erik Faye-Lund
@ 2025-02-05 12:51 ` Boris Brezillon
2025-02-06 14:56 ` Steven Price
2025-02-07 17:36 ` Boris Brezillon
4 siblings, 0 replies; 8+ messages in thread
From: Boris Brezillon @ 2025-02-05 12:51 UTC (permalink / raw)
To: Rob Herring (Arm)
Cc: Steven Price, Liviu Dudau, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Beata Michalska, dri-devel, linux-kernel, rust-for-linux
On Tue, 4 Feb 2025 17:28:24 -0600
"Rob Herring (Arm)" <robh@kernel.org> wrote:
> Use an enum instead of #defines for panthor IOCTLs. This allows the
> header to be used with Rust code as bindgen can't handle complex
> defines.
>
> Cc: Beata Michalska <beata.michalska@arm.com>
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
Acked-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> include/uapi/drm/panthor_drm.h | 86 +++++++++++++++++-----------------
> 1 file changed, 44 insertions(+), 42 deletions(-)
>
> diff --git a/include/uapi/drm/panthor_drm.h b/include/uapi/drm/panthor_drm.h
> index b99763cbae48..97e2c4510e69 100644
> --- a/include/uapi/drm/panthor_drm.h
> +++ b/include/uapi/drm/panthor_drm.h
> @@ -129,48 +129,6 @@ enum drm_panthor_ioctl_id {
> DRM_PANTHOR_TILER_HEAP_DESTROY,
> };
>
> -/**
> - * DRM_IOCTL_PANTHOR() - Build a Panthor IOCTL number
> - * @__access: Access type. Must be R, W or RW.
> - * @__id: One of the DRM_PANTHOR_xxx id.
> - * @__type: Suffix of the type being passed to the IOCTL.
> - *
> - * Don't use this macro directly, use the DRM_IOCTL_PANTHOR_xxx
> - * values instead.
> - *
> - * Return: An IOCTL number to be passed to ioctl() from userspace.
> - */
> -#define DRM_IOCTL_PANTHOR(__access, __id, __type) \
> - DRM_IO ## __access(DRM_COMMAND_BASE + DRM_PANTHOR_ ## __id, \
> - struct drm_panthor_ ## __type)
> -
> -#define DRM_IOCTL_PANTHOR_DEV_QUERY \
> - DRM_IOCTL_PANTHOR(WR, DEV_QUERY, dev_query)
> -#define DRM_IOCTL_PANTHOR_VM_CREATE \
> - DRM_IOCTL_PANTHOR(WR, VM_CREATE, vm_create)
> -#define DRM_IOCTL_PANTHOR_VM_DESTROY \
> - DRM_IOCTL_PANTHOR(WR, VM_DESTROY, vm_destroy)
> -#define DRM_IOCTL_PANTHOR_VM_BIND \
> - DRM_IOCTL_PANTHOR(WR, VM_BIND, vm_bind)
> -#define DRM_IOCTL_PANTHOR_VM_GET_STATE \
> - DRM_IOCTL_PANTHOR(WR, VM_GET_STATE, vm_get_state)
> -#define DRM_IOCTL_PANTHOR_BO_CREATE \
> - DRM_IOCTL_PANTHOR(WR, BO_CREATE, bo_create)
> -#define DRM_IOCTL_PANTHOR_BO_MMAP_OFFSET \
> - DRM_IOCTL_PANTHOR(WR, BO_MMAP_OFFSET, bo_mmap_offset)
> -#define DRM_IOCTL_PANTHOR_GROUP_CREATE \
> - DRM_IOCTL_PANTHOR(WR, GROUP_CREATE, group_create)
> -#define DRM_IOCTL_PANTHOR_GROUP_DESTROY \
> - DRM_IOCTL_PANTHOR(WR, GROUP_DESTROY, group_destroy)
> -#define DRM_IOCTL_PANTHOR_GROUP_SUBMIT \
> - DRM_IOCTL_PANTHOR(WR, GROUP_SUBMIT, group_submit)
> -#define DRM_IOCTL_PANTHOR_GROUP_GET_STATE \
> - DRM_IOCTL_PANTHOR(WR, GROUP_GET_STATE, group_get_state)
> -#define DRM_IOCTL_PANTHOR_TILER_HEAP_CREATE \
> - DRM_IOCTL_PANTHOR(WR, TILER_HEAP_CREATE, tiler_heap_create)
> -#define DRM_IOCTL_PANTHOR_TILER_HEAP_DESTROY \
> - DRM_IOCTL_PANTHOR(WR, TILER_HEAP_DESTROY, tiler_heap_destroy)
> -
> /**
> * DOC: IOCTL arguments
> */
> @@ -1019,6 +977,50 @@ struct drm_panthor_tiler_heap_destroy {
> __u32 pad;
> };
>
> +/**
> + * DRM_IOCTL_PANTHOR() - Build a Panthor IOCTL number
> + * @__access: Access type. Must be R, W or RW.
> + * @__id: One of the DRM_PANTHOR_xxx id.
> + * @__type: Suffix of the type being passed to the IOCTL.
> + *
> + * Don't use this macro directly, use the DRM_IOCTL_PANTHOR_xxx
> + * values instead.
> + *
> + * Return: An IOCTL number to be passed to ioctl() from userspace.
> + */
> +#define DRM_IOCTL_PANTHOR(__access, __id, __type) \
> + DRM_IO ## __access(DRM_COMMAND_BASE + DRM_PANTHOR_ ## __id, \
> + struct drm_panthor_ ## __type)
> +
> +enum {
> + DRM_IOCTL_PANTHOR_DEV_QUERY =
> + DRM_IOCTL_PANTHOR(WR, DEV_QUERY, dev_query),
> + DRM_IOCTL_PANTHOR_VM_CREATE =
> + DRM_IOCTL_PANTHOR(WR, VM_CREATE, vm_create),
> + DRM_IOCTL_PANTHOR_VM_DESTROY =
> + DRM_IOCTL_PANTHOR(WR, VM_DESTROY, vm_destroy),
> + DRM_IOCTL_PANTHOR_VM_BIND =
> + DRM_IOCTL_PANTHOR(WR, VM_BIND, vm_bind),
> + DRM_IOCTL_PANTHOR_VM_GET_STATE =
> + DRM_IOCTL_PANTHOR(WR, VM_GET_STATE, vm_get_state),
> + DRM_IOCTL_PANTHOR_BO_CREATE =
> + DRM_IOCTL_PANTHOR(WR, BO_CREATE, bo_create),
> + DRM_IOCTL_PANTHOR_BO_MMAP_OFFSET =
> + DRM_IOCTL_PANTHOR(WR, BO_MMAP_OFFSET, bo_mmap_offset),
> + DRM_IOCTL_PANTHOR_GROUP_CREATE =
> + DRM_IOCTL_PANTHOR(WR, GROUP_CREATE, group_create),
> + DRM_IOCTL_PANTHOR_GROUP_DESTROY =
> + DRM_IOCTL_PANTHOR(WR, GROUP_DESTROY, group_destroy),
> + DRM_IOCTL_PANTHOR_GROUP_SUBMIT =
> + DRM_IOCTL_PANTHOR(WR, GROUP_SUBMIT, group_submit),
> + DRM_IOCTL_PANTHOR_GROUP_GET_STATE =
> + DRM_IOCTL_PANTHOR(WR, GROUP_GET_STATE, group_get_state),
> + DRM_IOCTL_PANTHOR_TILER_HEAP_CREATE =
> + DRM_IOCTL_PANTHOR(WR, TILER_HEAP_CREATE, tiler_heap_create),
> + DRM_IOCTL_PANTHOR_TILER_HEAP_DESTROY =
> + DRM_IOCTL_PANTHOR(WR, TILER_HEAP_DESTROY, tiler_heap_destroy),
> +};
> +
> #if defined(__cplusplus)
> }
> #endif
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] drm/panthor: Convert IOCTL defines to an enum
2025-02-04 23:28 [PATCH] drm/panthor: Convert IOCTL defines to an enum Rob Herring (Arm)
` (2 preceding siblings ...)
2025-02-05 12:51 ` Boris Brezillon
@ 2025-02-06 14:56 ` Steven Price
2025-02-07 17:36 ` Boris Brezillon
4 siblings, 0 replies; 8+ messages in thread
From: Steven Price @ 2025-02-06 14:56 UTC (permalink / raw)
To: Rob Herring (Arm), Boris Brezillon, Liviu Dudau,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross
Cc: Beata Michalska, dri-devel, linux-kernel, rust-for-linux
On 04/02/2025 23:28, Rob Herring (Arm) wrote:
> Use an enum instead of #defines for panthor IOCTLs. This allows the
> header to be used with Rust code as bindgen can't handle complex
> defines.
Sounds like something that needs to be improved in bindgen...
Never-the-less for this particular case I think it's fine.
Reviewed-by: Steven Price <steven.price@arm.com>
> Cc: Beata Michalska <beata.michalska@arm.com>
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> ---
> include/uapi/drm/panthor_drm.h | 86 +++++++++++++++++-----------------
> 1 file changed, 44 insertions(+), 42 deletions(-)
>
> diff --git a/include/uapi/drm/panthor_drm.h b/include/uapi/drm/panthor_drm.h
> index b99763cbae48..97e2c4510e69 100644
> --- a/include/uapi/drm/panthor_drm.h
> +++ b/include/uapi/drm/panthor_drm.h
> @@ -129,48 +129,6 @@ enum drm_panthor_ioctl_id {
> DRM_PANTHOR_TILER_HEAP_DESTROY,
> };
>
> -/**
> - * DRM_IOCTL_PANTHOR() - Build a Panthor IOCTL number
> - * @__access: Access type. Must be R, W or RW.
> - * @__id: One of the DRM_PANTHOR_xxx id.
> - * @__type: Suffix of the type being passed to the IOCTL.
> - *
> - * Don't use this macro directly, use the DRM_IOCTL_PANTHOR_xxx
> - * values instead.
> - *
> - * Return: An IOCTL number to be passed to ioctl() from userspace.
> - */
> -#define DRM_IOCTL_PANTHOR(__access, __id, __type) \
> - DRM_IO ## __access(DRM_COMMAND_BASE + DRM_PANTHOR_ ## __id, \
> - struct drm_panthor_ ## __type)
> -
> -#define DRM_IOCTL_PANTHOR_DEV_QUERY \
> - DRM_IOCTL_PANTHOR(WR, DEV_QUERY, dev_query)
> -#define DRM_IOCTL_PANTHOR_VM_CREATE \
> - DRM_IOCTL_PANTHOR(WR, VM_CREATE, vm_create)
> -#define DRM_IOCTL_PANTHOR_VM_DESTROY \
> - DRM_IOCTL_PANTHOR(WR, VM_DESTROY, vm_destroy)
> -#define DRM_IOCTL_PANTHOR_VM_BIND \
> - DRM_IOCTL_PANTHOR(WR, VM_BIND, vm_bind)
> -#define DRM_IOCTL_PANTHOR_VM_GET_STATE \
> - DRM_IOCTL_PANTHOR(WR, VM_GET_STATE, vm_get_state)
> -#define DRM_IOCTL_PANTHOR_BO_CREATE \
> - DRM_IOCTL_PANTHOR(WR, BO_CREATE, bo_create)
> -#define DRM_IOCTL_PANTHOR_BO_MMAP_OFFSET \
> - DRM_IOCTL_PANTHOR(WR, BO_MMAP_OFFSET, bo_mmap_offset)
> -#define DRM_IOCTL_PANTHOR_GROUP_CREATE \
> - DRM_IOCTL_PANTHOR(WR, GROUP_CREATE, group_create)
> -#define DRM_IOCTL_PANTHOR_GROUP_DESTROY \
> - DRM_IOCTL_PANTHOR(WR, GROUP_DESTROY, group_destroy)
> -#define DRM_IOCTL_PANTHOR_GROUP_SUBMIT \
> - DRM_IOCTL_PANTHOR(WR, GROUP_SUBMIT, group_submit)
> -#define DRM_IOCTL_PANTHOR_GROUP_GET_STATE \
> - DRM_IOCTL_PANTHOR(WR, GROUP_GET_STATE, group_get_state)
> -#define DRM_IOCTL_PANTHOR_TILER_HEAP_CREATE \
> - DRM_IOCTL_PANTHOR(WR, TILER_HEAP_CREATE, tiler_heap_create)
> -#define DRM_IOCTL_PANTHOR_TILER_HEAP_DESTROY \
> - DRM_IOCTL_PANTHOR(WR, TILER_HEAP_DESTROY, tiler_heap_destroy)
> -
> /**
> * DOC: IOCTL arguments
> */
> @@ -1019,6 +977,50 @@ struct drm_panthor_tiler_heap_destroy {
> __u32 pad;
> };
>
> +/**
> + * DRM_IOCTL_PANTHOR() - Build a Panthor IOCTL number
> + * @__access: Access type. Must be R, W or RW.
> + * @__id: One of the DRM_PANTHOR_xxx id.
> + * @__type: Suffix of the type being passed to the IOCTL.
> + *
> + * Don't use this macro directly, use the DRM_IOCTL_PANTHOR_xxx
> + * values instead.
> + *
> + * Return: An IOCTL number to be passed to ioctl() from userspace.
> + */
> +#define DRM_IOCTL_PANTHOR(__access, __id, __type) \
> + DRM_IO ## __access(DRM_COMMAND_BASE + DRM_PANTHOR_ ## __id, \
> + struct drm_panthor_ ## __type)
> +
> +enum {
> + DRM_IOCTL_PANTHOR_DEV_QUERY =
> + DRM_IOCTL_PANTHOR(WR, DEV_QUERY, dev_query),
> + DRM_IOCTL_PANTHOR_VM_CREATE =
> + DRM_IOCTL_PANTHOR(WR, VM_CREATE, vm_create),
> + DRM_IOCTL_PANTHOR_VM_DESTROY =
> + DRM_IOCTL_PANTHOR(WR, VM_DESTROY, vm_destroy),
> + DRM_IOCTL_PANTHOR_VM_BIND =
> + DRM_IOCTL_PANTHOR(WR, VM_BIND, vm_bind),
> + DRM_IOCTL_PANTHOR_VM_GET_STATE =
> + DRM_IOCTL_PANTHOR(WR, VM_GET_STATE, vm_get_state),
> + DRM_IOCTL_PANTHOR_BO_CREATE =
> + DRM_IOCTL_PANTHOR(WR, BO_CREATE, bo_create),
> + DRM_IOCTL_PANTHOR_BO_MMAP_OFFSET =
> + DRM_IOCTL_PANTHOR(WR, BO_MMAP_OFFSET, bo_mmap_offset),
> + DRM_IOCTL_PANTHOR_GROUP_CREATE =
> + DRM_IOCTL_PANTHOR(WR, GROUP_CREATE, group_create),
> + DRM_IOCTL_PANTHOR_GROUP_DESTROY =
> + DRM_IOCTL_PANTHOR(WR, GROUP_DESTROY, group_destroy),
> + DRM_IOCTL_PANTHOR_GROUP_SUBMIT =
> + DRM_IOCTL_PANTHOR(WR, GROUP_SUBMIT, group_submit),
> + DRM_IOCTL_PANTHOR_GROUP_GET_STATE =
> + DRM_IOCTL_PANTHOR(WR, GROUP_GET_STATE, group_get_state),
> + DRM_IOCTL_PANTHOR_TILER_HEAP_CREATE =
> + DRM_IOCTL_PANTHOR(WR, TILER_HEAP_CREATE, tiler_heap_create),
> + DRM_IOCTL_PANTHOR_TILER_HEAP_DESTROY =
> + DRM_IOCTL_PANTHOR(WR, TILER_HEAP_DESTROY, tiler_heap_destroy),
> +};
> +
> #if defined(__cplusplus)
> }
> #endif
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] drm/panthor: Convert IOCTL defines to an enum
2025-02-04 23:28 [PATCH] drm/panthor: Convert IOCTL defines to an enum Rob Herring (Arm)
` (3 preceding siblings ...)
2025-02-06 14:56 ` Steven Price
@ 2025-02-07 17:36 ` Boris Brezillon
4 siblings, 0 replies; 8+ messages in thread
From: Boris Brezillon @ 2025-02-07 17:36 UTC (permalink / raw)
To: Rob Herring (Arm)
Cc: Steven Price, Liviu Dudau, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Beata Michalska, dri-devel, linux-kernel, rust-for-linux
On Tue, 4 Feb 2025 17:28:24 -0600
"Rob Herring (Arm)" <robh@kernel.org> wrote:
> Use an enum instead of #defines for panthor IOCTLs. This allows the
> header to be used with Rust code as bindgen can't handle complex
> defines.
>
> Cc: Beata Michalska <beata.michalska@arm.com>
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
Queued to drm-misc-next.
> ---
> include/uapi/drm/panthor_drm.h | 86 +++++++++++++++++-----------------
> 1 file changed, 44 insertions(+), 42 deletions(-)
>
> diff --git a/include/uapi/drm/panthor_drm.h b/include/uapi/drm/panthor_drm.h
> index b99763cbae48..97e2c4510e69 100644
> --- a/include/uapi/drm/panthor_drm.h
> +++ b/include/uapi/drm/panthor_drm.h
> @@ -129,48 +129,6 @@ enum drm_panthor_ioctl_id {
> DRM_PANTHOR_TILER_HEAP_DESTROY,
> };
>
> -/**
> - * DRM_IOCTL_PANTHOR() - Build a Panthor IOCTL number
> - * @__access: Access type. Must be R, W or RW.
> - * @__id: One of the DRM_PANTHOR_xxx id.
> - * @__type: Suffix of the type being passed to the IOCTL.
> - *
> - * Don't use this macro directly, use the DRM_IOCTL_PANTHOR_xxx
> - * values instead.
> - *
> - * Return: An IOCTL number to be passed to ioctl() from userspace.
> - */
> -#define DRM_IOCTL_PANTHOR(__access, __id, __type) \
> - DRM_IO ## __access(DRM_COMMAND_BASE + DRM_PANTHOR_ ## __id, \
> - struct drm_panthor_ ## __type)
> -
> -#define DRM_IOCTL_PANTHOR_DEV_QUERY \
> - DRM_IOCTL_PANTHOR(WR, DEV_QUERY, dev_query)
> -#define DRM_IOCTL_PANTHOR_VM_CREATE \
> - DRM_IOCTL_PANTHOR(WR, VM_CREATE, vm_create)
> -#define DRM_IOCTL_PANTHOR_VM_DESTROY \
> - DRM_IOCTL_PANTHOR(WR, VM_DESTROY, vm_destroy)
> -#define DRM_IOCTL_PANTHOR_VM_BIND \
> - DRM_IOCTL_PANTHOR(WR, VM_BIND, vm_bind)
> -#define DRM_IOCTL_PANTHOR_VM_GET_STATE \
> - DRM_IOCTL_PANTHOR(WR, VM_GET_STATE, vm_get_state)
> -#define DRM_IOCTL_PANTHOR_BO_CREATE \
> - DRM_IOCTL_PANTHOR(WR, BO_CREATE, bo_create)
> -#define DRM_IOCTL_PANTHOR_BO_MMAP_OFFSET \
> - DRM_IOCTL_PANTHOR(WR, BO_MMAP_OFFSET, bo_mmap_offset)
> -#define DRM_IOCTL_PANTHOR_GROUP_CREATE \
> - DRM_IOCTL_PANTHOR(WR, GROUP_CREATE, group_create)
> -#define DRM_IOCTL_PANTHOR_GROUP_DESTROY \
> - DRM_IOCTL_PANTHOR(WR, GROUP_DESTROY, group_destroy)
> -#define DRM_IOCTL_PANTHOR_GROUP_SUBMIT \
> - DRM_IOCTL_PANTHOR(WR, GROUP_SUBMIT, group_submit)
> -#define DRM_IOCTL_PANTHOR_GROUP_GET_STATE \
> - DRM_IOCTL_PANTHOR(WR, GROUP_GET_STATE, group_get_state)
> -#define DRM_IOCTL_PANTHOR_TILER_HEAP_CREATE \
> - DRM_IOCTL_PANTHOR(WR, TILER_HEAP_CREATE, tiler_heap_create)
> -#define DRM_IOCTL_PANTHOR_TILER_HEAP_DESTROY \
> - DRM_IOCTL_PANTHOR(WR, TILER_HEAP_DESTROY, tiler_heap_destroy)
> -
> /**
> * DOC: IOCTL arguments
> */
> @@ -1019,6 +977,50 @@ struct drm_panthor_tiler_heap_destroy {
> __u32 pad;
> };
>
> +/**
> + * DRM_IOCTL_PANTHOR() - Build a Panthor IOCTL number
> + * @__access: Access type. Must be R, W or RW.
> + * @__id: One of the DRM_PANTHOR_xxx id.
> + * @__type: Suffix of the type being passed to the IOCTL.
> + *
> + * Don't use this macro directly, use the DRM_IOCTL_PANTHOR_xxx
> + * values instead.
> + *
> + * Return: An IOCTL number to be passed to ioctl() from userspace.
> + */
> +#define DRM_IOCTL_PANTHOR(__access, __id, __type) \
> + DRM_IO ## __access(DRM_COMMAND_BASE + DRM_PANTHOR_ ## __id, \
> + struct drm_panthor_ ## __type)
> +
> +enum {
> + DRM_IOCTL_PANTHOR_DEV_QUERY =
> + DRM_IOCTL_PANTHOR(WR, DEV_QUERY, dev_query),
> + DRM_IOCTL_PANTHOR_VM_CREATE =
> + DRM_IOCTL_PANTHOR(WR, VM_CREATE, vm_create),
> + DRM_IOCTL_PANTHOR_VM_DESTROY =
> + DRM_IOCTL_PANTHOR(WR, VM_DESTROY, vm_destroy),
> + DRM_IOCTL_PANTHOR_VM_BIND =
> + DRM_IOCTL_PANTHOR(WR, VM_BIND, vm_bind),
> + DRM_IOCTL_PANTHOR_VM_GET_STATE =
> + DRM_IOCTL_PANTHOR(WR, VM_GET_STATE, vm_get_state),
> + DRM_IOCTL_PANTHOR_BO_CREATE =
> + DRM_IOCTL_PANTHOR(WR, BO_CREATE, bo_create),
> + DRM_IOCTL_PANTHOR_BO_MMAP_OFFSET =
> + DRM_IOCTL_PANTHOR(WR, BO_MMAP_OFFSET, bo_mmap_offset),
> + DRM_IOCTL_PANTHOR_GROUP_CREATE =
> + DRM_IOCTL_PANTHOR(WR, GROUP_CREATE, group_create),
> + DRM_IOCTL_PANTHOR_GROUP_DESTROY =
> + DRM_IOCTL_PANTHOR(WR, GROUP_DESTROY, group_destroy),
> + DRM_IOCTL_PANTHOR_GROUP_SUBMIT =
> + DRM_IOCTL_PANTHOR(WR, GROUP_SUBMIT, group_submit),
> + DRM_IOCTL_PANTHOR_GROUP_GET_STATE =
> + DRM_IOCTL_PANTHOR(WR, GROUP_GET_STATE, group_get_state),
> + DRM_IOCTL_PANTHOR_TILER_HEAP_CREATE =
> + DRM_IOCTL_PANTHOR(WR, TILER_HEAP_CREATE, tiler_heap_create),
> + DRM_IOCTL_PANTHOR_TILER_HEAP_DESTROY =
> + DRM_IOCTL_PANTHOR(WR, TILER_HEAP_DESTROY, tiler_heap_destroy),
> +};
> +
> #if defined(__cplusplus)
> }
> #endif
^ permalink raw reply [flat|nested] 8+ messages in thread