Re: [RFC v3 2/3] drm/i915: Update i915 uapi documentation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 17 May 2022 at 19:32, Niranjana Vishwanathapura
<niranjana.vishwanathapura@xxxxxxxxx> wrote:
>
> Add some missing i915 upai documentation which the new
> i915 VM_BIND feature documentation will be refer to.
>
> Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@xxxxxxxxx>
> ---
>  include/uapi/drm/i915_drm.h | 153 +++++++++++++++++++++++++++---------
>  1 file changed, 116 insertions(+), 37 deletions(-)
>
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index a2def7b27009..8c834a31b56f 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -751,9 +751,16 @@ typedef struct drm_i915_irq_wait {
>
>  /* Must be kept compact -- no holes and well documented */
>
> +/**
> + * typedef drm_i915_getparam_t - Driver parameter query structure.

This one looks funny in the rendered html for some reason, since it
doesn't seem to emit the @param and @value, I guess it doesn't really
understand typedef <struct> ?

Maybe make this "struct drm_i915_getparam - Driver parameter query structure." ?

> + */
>  typedef struct drm_i915_getparam {
> +       /** @param: Driver parameter to query. */
>         __s32 param;
> -       /*
> +
> +       /**
> +        * @value: Address of memory where queried value should be put.
> +        *
>          * WARNING: Using pointers instead of fixed-size u64 means we need to write
>          * compat32 code. Don't repeat this mistake.
>          */
> @@ -1239,76 +1246,114 @@ struct drm_i915_gem_exec_object2 {
>         __u64 rsvd2;
>  };
>
> +/**
> + * struct drm_i915_gem_exec_fence - An input or output fence for the execbuff

s/execbuff/execbuf/, at least that seems to be what we use elsewhere, AFAICT.

> + * ioctl.
> + *
> + * The request will wait for input fence to signal before submission.
> + *
> + * The returned output fence will be signaled after the completion of the
> + * request.
> + */
>  struct drm_i915_gem_exec_fence {
> -       /**
> -        * User's handle for a drm_syncobj to wait on or signal.
> -        */
> +       /** @handle: User's handle for a drm_syncobj to wait on or signal. */
>         __u32 handle;
>
> +       /**
> +        * @flags: Supported flags are,

are:

> +        *
> +        * I915_EXEC_FENCE_WAIT:
> +        * Wait for the input fence before request submission.
> +        *
> +        * I915_EXEC_FENCE_SIGNAL:
> +        * Return request completion fence as output
> +        */
> +       __u32 flags;
>  #define I915_EXEC_FENCE_WAIT            (1<<0)
>  #define I915_EXEC_FENCE_SIGNAL          (1<<1)
>  #define __I915_EXEC_FENCE_UNKNOWN_FLAGS (-(I915_EXEC_FENCE_SIGNAL << 1))
> -       __u32 flags;
>  };
>
> -/*
> - * See drm_i915_gem_execbuffer_ext_timeline_fences.
> - */
> -#define DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES 0
> -
> -/*
> +/**
> + * struct drm_i915_gem_execbuffer_ext_timeline_fences - Timeline fences
> + * for execbuff.
> + *
>   * This structure describes an array of drm_syncobj and associated points for
>   * timeline variants of drm_syncobj. It is invalid to append this structure to
>   * the execbuf if I915_EXEC_FENCE_ARRAY is set.
>   */
>  struct drm_i915_gem_execbuffer_ext_timeline_fences {
> +#define DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES 0
> +       /** @base: Extension link. See struct i915_user_extension. */
>         struct i915_user_extension base;
>
>         /**
> -        * Number of element in the handles_ptr & value_ptr arrays.
> +        * @fence_count: Number of element in the @handles_ptr & @value_ptr

s/element/elements/

> +        * arrays.
>          */
>         __u64 fence_count;
>
>         /**
> -        * Pointer to an array of struct drm_i915_gem_exec_fence of length
> -        * fence_count.
> +        * @handles_ptr: Pointer to an array of struct drm_i915_gem_exec_fence
> +        * of length @fence_count.
>          */
>         __u64 handles_ptr;
>
>         /**
> -        * Pointer to an array of u64 values of length fence_count. Values
> -        * must be 0 for a binary drm_syncobj. A Value of 0 for a timeline
> -        * drm_syncobj is invalid as it turns a drm_syncobj into a binary one.
> +        * @values_ptr: Pointer to an array of u64 values of length
> +        * @fence_count.
> +        * Values must be 0 for a binary drm_syncobj. A Value of 0 for a
> +        * timeline drm_syncobj is invalid as it turns a drm_syncobj into a
> +        * binary one.
>          */
>         __u64 values_ptr;
>  };
>
> +/**
> + * struct drm_i915_gem_execbuffer2 - Structure for execbuff submission
> + */
>  struct drm_i915_gem_execbuffer2 {
> -       /**
> -        * List of gem_exec_object2 structs
> -        */
> +       /** @buffers_ptr: Pointer to a list of gem_exec_object2 structs */
>         __u64 buffers_ptr;
> +
> +       /** @buffer_count: Number of elements in @buffers_ptr array */
>         __u32 buffer_count;
>
> -       /** Offset in the batchbuffer to start execution from. */
> +       /**
> +        * @batch_start_offset: Offset in the batchbuffer to start execution
> +        * from.
> +        */
>         __u32 batch_start_offset;
> -       /** Bytes used in batchbuffer from batch_start_offset */
> +
> +       /** @batch_len: Bytes used in batchbuffer from batch_start_offset */

"Length in bytes of the batchbuffer, otherwise assumed to be the
object size if zero, starting from the @batch_start_offset."

>         __u32 batch_len;
> +
> +       /** @DR1: deprecated */
>         __u32 DR1;
> +
> +       /** @DR4: deprecated */
>         __u32 DR4;
> +
> +       /** @num_cliprects: See @cliprects_ptr */
>         __u32 num_cliprects;
> +
>         /**
> -        * This is a struct drm_clip_rect *cliprects if I915_EXEC_FENCE_ARRAY
> -        * & I915_EXEC_USE_EXTENSIONS are not set.
> +        * @cliprects_ptr: Kernel clipping was a DRI1 misfeature.
> +        *
> +        * It is invalid to use this field if I915_EXEC_FENCE_ARRAY or
> +        * I915_EXEC_USE_EXTENSIONS flags are not set.
>          *
>          * If I915_EXEC_FENCE_ARRAY is set, then this is a pointer to an array
> -        * of struct drm_i915_gem_exec_fence and num_cliprects is the length
> -        * of the array.
> +        * of &drm_i915_gem_exec_fence and @num_cliprects is the length of the
> +        * array.
>          *
>          * If I915_EXEC_USE_EXTENSIONS is set, then this is a pointer to a
> -        * single struct i915_user_extension and num_cliprects is 0.
> +        * single &i915_user_extension and num_cliprects is 0.
>          */
>         __u64 cliprects_ptr;
> +
> +       /** @flags: Execbuff flags */

s/Execbuff/Execbuf/

Could maybe document the I915_EXEC_* also, or maybe not ;)

> +       __u64 flags;
>  #define I915_EXEC_RING_MASK              (0x3f)
>  #define I915_EXEC_DEFAULT                (0<<0)
>  #define I915_EXEC_RENDER                 (1<<0)
> @@ -1326,10 +1371,6 @@ struct drm_i915_gem_execbuffer2 {
>  #define I915_EXEC_CONSTANTS_REL_GENERAL (0<<6) /* default */
>  #define I915_EXEC_CONSTANTS_ABSOLUTE   (1<<6)
>  #define I915_EXEC_CONSTANTS_REL_SURFACE (2<<6) /* gen4/5 only */
> -       __u64 flags;
> -       __u64 rsvd1; /* now used for context info */
> -       __u64 rsvd2;
> -};
>
>  /** Resets the SO write offset registers for transform feedback on gen7. */
>  #define I915_EXEC_GEN7_SOL_RESET       (1<<8)
> @@ -1432,9 +1473,23 @@ struct drm_i915_gem_execbuffer2 {
>   * drm_i915_gem_execbuffer_ext enum.
>   */
>  #define I915_EXEC_USE_EXTENSIONS       (1 << 21)
> -
>  #define __I915_EXEC_UNKNOWN_FLAGS (-(I915_EXEC_USE_EXTENSIONS << 1))
>
> +       /** @rsvd1: Context id */
> +       __u64 rsvd1;
> +
> +       /**
> +        * @rsvd2: in and out sync_file file descriptors.
> +        *
> +        * When I915_EXEC_FENCE_IN or I915_EXEC_FENCE_SUBMIT flag is set, the
> +        * lower 32 bits of this field will have the in sync_file fd (input).
> +        *
> +        * When I915_EXEC_FENCE_OUT flag is set, the upper 32 bits of this
> +        * field will have the out sync_file fd (output).
> +        */
> +       __u64 rsvd2;
> +};
> +
>  #define I915_EXEC_CONTEXT_ID_MASK      (0xffffffff)
>  #define i915_execbuffer2_set_context_id(eb2, context) \
>         (eb2).rsvd1 = context & I915_EXEC_CONTEXT_ID_MASK
> @@ -1814,13 +1869,32 @@ struct drm_i915_gem_context_create {
>         __u32 pad;
>  };
>
> +/**
> + * struct drm_i915_gem_context_create_ext - Structure for creating contexts.
> + */
>  struct drm_i915_gem_context_create_ext {
> -       __u32 ctx_id; /* output: id of new context*/
> +       /** @ctx_id: Id of the created context (output) */
> +       __u32 ctx_id;
> +
> +       /**
> +        * @flags: Supported flags are,

are:

> +        *
> +        * I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS:
> +        *
> +        * Extensions may be appended to this structure and driver must check
> +        * for those.

Maybe add "See @extensions.", and then....

> +        *
> +        * I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE
> +        *
> +        * Created context will have single timeline.
> +        */
>         __u32 flags;
>  #define I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS       (1u << 0)
>  #define I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE      (1u << 1)
>  #define I915_CONTEXT_CREATE_FLAGS_UNKNOWN \
>         (-(I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE << 1))
> +
> +       /** @extensions: Zero-terminated chain of extensions. */

...here perhaps list the extensions, and maybe also move the #define
for each here? See for example @extensions in drm_i915_gem_create_ext.

Reviewed-by: Matthew Auld <matthew.auld@xxxxxxxxx>

>         __u64 extensions;
>  };
>
> @@ -2387,7 +2461,9 @@ struct drm_i915_gem_context_destroy {
>         __u32 pad;
>  };
>
> -/*
> +/**
> + * struct drm_i915_gem_vm_control - Structure to create or destroy VM.
> + *
>   * DRM_I915_GEM_VM_CREATE -
>   *
>   * Create a new virtual memory address space (ppGTT) for use within a context
> @@ -2397,20 +2473,23 @@ struct drm_i915_gem_context_destroy {
>   * The id of new VM (bound to the fd) for use with I915_CONTEXT_PARAM_VM is
>   * returned in the outparam @id.
>   *
> - * No flags are defined, with all bits reserved and must be zero.
> - *
>   * An extension chain maybe provided, starting with @extensions, and terminated
>   * by the @next_extension being 0. Currently, no extensions are defined.
>   *
>   * DRM_I915_GEM_VM_DESTROY -
>   *
> - * Destroys a previously created VM id, specified in @id.
> + * Destroys a previously created VM id, specified in @vm_id.
>   *
>   * No extensions or flags are allowed currently, and so must be zero.
>   */
>  struct drm_i915_gem_vm_control {
> +       /** @extensions: Zero-terminated chain of extensions. */
>         __u64 extensions;
> +
> +       /** @flags: reserved for future usage, currently MBZ */
>         __u32 flags;
> +
> +       /** @vm_id: Id of the VM created or to be destroyed */
>         __u32 vm_id;
>  };
>
> --
> 2.21.0.rc0.32.g243a4c7e27
>



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux