[Intel-gfx] [PATCH 3/4] drm/i915/uapi: convert i915_user_extension to kernel doc

[Jason Ekstrand]

On April 16, 2021 05:37:52 Matthew Auld <matthew.auld at intel.com> wrote:

> Add some example usage for the extension chaining also, which is quite
> nifty.
>
> v2: (Daniel)
>  - clarify that the name is just some integer, also document that the
>    name space is not global
>
> Suggested-by: Daniel Vetter <daniel at ffwll.ch>
> Signed-off-by: Matthew Auld <matthew.auld at intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Jordan Justen <jordan.l.justen at intel.com>
> Cc: Daniel Vetter <daniel.vetter at intel.com>
> Cc: Kenneth Graunke <kenneth at whitecape.org>
> Cc: Jason Ekstrand <jason at jlekstrand.net>
> Cc: Dave Airlie <airlied at gmail.com>
> Cc: dri-devel at lists.freedesktop.org
> Cc: mesa-dev at lists.freedesktop.org
> Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
> include/uapi/drm/i915_drm.h | 54 ++++++++++++++++++++++++++++++++++---
> 1 file changed, 50 insertions(+), 4 deletions(-)
>
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 92da48e935d1..d79b51c12ff2 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -62,8 +62,8 @@ extern "C" {
> #define I915_ERROR_UEVENT "ERROR"
> #define I915_RESET_UEVENT "RESET"
>
> -/*
> - * i915_user_extension: Base class for defining a chain of extensions
> +/**
> + * struct i915_user_extension - Base class for defining a chain of extensions
>  *
>  * Many interfaces need to grow over time. In most cases we can simply
>  * extend the struct and have userspace pass in more data. Another option,
> @@ -76,12 +76,58 @@ extern "C" {
>  * increasing complexity, and for large parts of that interface to be
>  * entirely optional. The downside is more pointer chasing; chasing across
>  * the __user boundary with pointers encapsulated inside u64.
> + *
> + * Example chaining:
> + *
> + * .. code-block:: C
> + *
> + * struct i915_user_extension ext3 {
> + * .next_extension = 0, // end
> + * .name = ...,
> + * };
> + * struct i915_user_extension ext2 {
> + * .next_extension = (uintptr_t)&ext3,
> + * .name = ...,
> + * };
> + * struct i915_user_extension ext1 {
> + * .next_extension = (uintptr_t)&ext2,
> + * .name = ...,
> + * };
> + *
> + * Typically the i915_user_extension would be embedded in some uAPI 
> struct, and
> + * in this case we would feed it the head of the chain(i.e ext1), which would
> + * then apply all of the above extensions.
> + *
>  */
> struct i915_user_extension {
> + /**
> + * @next_extension:
> + *
> + * Pointer to the next struct i915_user_extension, or zero if the end.
> + */
>  __u64 next_extension;
> + /**
> + * @name: Name of the extension.
> + *
> + * Note that the name here is just some integer.
> + *
> + * Also note that the name space for this is not global for the whole
> + * driver, but rather its scope/meaning is limited to the specific piece
> + * of uAPI which has embedded the struct i915_user_extension.

We may want to rethink this decision. In Vulkan, we have several cases 
where we use the same extension multiple places.  Given that the only 
extensible thing currently landed is context creation, we still could make 
this global.  Then again, forcing us to go through the exercise of 
redefining every time has its advantages too.

In any case, this is a correct description of the current state of affairs, so

Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>


> + */
>  __u32 name;
> - __u32 flags; /* All undefined bits must be zero. */
> - __u32 rsvd[4]; /* Reserved for future use; must be zero. */
> + /**
> + * @flags: MBZ
> + *
> + * All undefined bits must be zero.
> + */
> + __u32 flags;
> + /**
> + * @rsvd: MBZ
> + *
> + * Reserved for future use; must be zero.
> + */
> + __u32 rsvd[4];
> };
>
> /*
> --
> 2.26.3

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20210416/6af98ad3/attachment.htm>