[Intel-gfx] [RFC v3 2/3] drm/i915: Update i915 uapi documentation
Niranjana Vishwanathapura
niranjana.vishwanathapura at intel.com
Fri Jun 10 01:43:47 UTC 2022
On Wed, Jun 08, 2022 at 12:24:04PM +0100, Matthew Auld wrote:
>On Tue, 17 May 2022 at 19:32, Niranjana Vishwanathapura
><niranjana.vishwanathapura at intel.com> 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 at intel.com>
>> ---
>> 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." ?
Thanks Matt.
Yah, there doesn't seems to be a good way to add kernel doc for this
kind of declaration. 'struct drm_i915_getparam' also didn't help.
I was able to fix it by first defining the structure and then adding
a typedef for it. Not sure if that has any value, but at least we can
get kernel doc for that.
>
>> + */
>> 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 ;)
>
We no longer need to refer to execbuf2 as vm_bind will have its own
new execbuf3. But will keep the already added execbuf2 documentation.
>> + __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.
>
Ok, will address all your comments above.
Niranjana
>Reviewed-by: Matthew Auld <matthew.auld at intel.com>
>
>> __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
>>
More information about the Intel-gfx
mailing list