diff mbox series

[v3,3/4] drm/i915/uapi: convert i915_query and friend to kernel doc

Message ID 20210415155958.391624-3-matthew.auld@intel.com (mailing list archive)
State New, archived
Headers show
Series [v3,1/4] drm/i915/uapi: hide kernel doc warnings | expand

Commit Message

Matthew Auld April 15, 2021, 3:59 p.m. UTC
Add a note about the two-step process.

Suggested-by: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Kenneth Graunke <kenneth@whitecape.org>
Cc: Jason Ekstrand <jason@jlekstrand.net>
Cc: Dave Airlie <airlied@gmail.com>
Cc: dri-devel@lists.freedesktop.org
Cc: mesa-dev@lists.freedesktop.org
---
 include/uapi/drm/i915_drm.h | 57 ++++++++++++++++++++++++++++++-------
 1 file changed, 46 insertions(+), 11 deletions(-)

Comments

Ian Romanick April 15, 2021, 10:25 p.m. UTC | #1
On 4/15/21 8:59 AM, Matthew Auld wrote:
> Add a note about the two-step process.
> 
> Suggested-by: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Kenneth Graunke <kenneth@whitecape.org>
> Cc: Jason Ekstrand <jason@jlekstrand.net>
> Cc: Dave Airlie <airlied@gmail.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: mesa-dev@lists.freedesktop.org
> ---
>  include/uapi/drm/i915_drm.h | 57 ++++++++++++++++++++++++++++++-------
>  1 file changed, 46 insertions(+), 11 deletions(-)
> 
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index d9c954a5a456..ef36f1a0adde 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -2210,14 +2210,23 @@ struct drm_i915_perf_oa_config {
>  	__u64 flex_regs_ptr;
>  };
>  
> +/**
> + * struct drm_i915_query_item - An individual query for the kernel to process.
> + *
> + * The behaviour is determined by the @query_id. Note that exactly what

Since we just had a big discussion about this on mesa-dev w.r.t. Mesa
code and documentation... does the kernel have a policy about which
flavor (pun intended) of English should be used?

> + * @data_ptr is also depends on the specific @query_id.
> + */
>  struct drm_i915_query_item {
> +	/** @query_id: The id for this query */
>  	__u64 query_id;
>  #define DRM_I915_QUERY_TOPOLOGY_INFO    1
>  #define DRM_I915_QUERY_ENGINE_INFO	2
>  #define DRM_I915_QUERY_PERF_CONFIG      3
>  /* Must be kept compact -- no holes and well documented */
>  
> -	/*
> +	/**
> +	 * @length:
> +	 *
>  	 * When set to zero by userspace, this is filled with the size of the
>  	 * data to be written at the data_ptr pointer. The kernel sets this
>  	 * value to a negative value to signal an error on a particular query
> @@ -2225,21 +2234,26 @@ struct drm_i915_query_item {
>  	 */
>  	__s32 length;
>  
> -	/*
> +	/**
> +	 * @flags:
> +	 *
>  	 * When query_id == DRM_I915_QUERY_TOPOLOGY_INFO, must be 0.
>  	 *
>  	 * When query_id == DRM_I915_QUERY_PERF_CONFIG, must be one of the
> -	 * following :
> -	 *         - DRM_I915_QUERY_PERF_CONFIG_LIST
> -	 *         - DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID
> -	 *         - DRM_I915_QUERY_PERF_CONFIG_FOR_UUID
> +	 * following:
> +	 *
> +	 *	- DRM_I915_QUERY_PERF_CONFIG_LIST
> +	 *      - DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID
> +	 *      - DRM_I915_QUERY_PERF_CONFIG_FOR_UUID
>  	 */
>  	__u32 flags;
>  #define DRM_I915_QUERY_PERF_CONFIG_LIST          1
>  #define DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID 2
>  #define DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_ID   3
>  
> -	/*
> +	/**
> +	 * @data_ptr:
> +	 *
>  	 * Data will be written at the location pointed by data_ptr when the
>  	 * value of length matches the length of the data to be written by the
>  	 * kernel.
> @@ -2247,16 +2261,37 @@ struct drm_i915_query_item {
>  	__u64 data_ptr;
>  };
>  
> +/**
> + * struct drm_i915_query - Supply an array of drm_i915_query_item for the kernel
> + * to fill out.
> + *
> + * Note that this is generally a two step process for each drm_i915_query_item
> + * in the array:
> + *
> + *	1.) Call the DRM_IOCTL_I915_QUERY, giving it our array of
> + *	drm_i915_query_item, with drm_i915_query_item.size set to zero. The
> + *	kernel will then fill in the size, in bytes, which tells userspace how
> + *	memory it needs to allocate for the blob(say for an array of
> + *	properties).
> + *
> + *	2.) Next we call DRM_IOCTL_I915_QUERY again, this time with the
> + *	drm_i915_query_item.data_ptr equal to our newly allocated blob. Note
> + *	that the i915_query_item.size should still be the same as what the
> + *	kernel previously set. At this point the kernel can fill in the blob.
> + *
> + */
>  struct drm_i915_query {
> +	/** @num_items: The number of elements in the @items_ptr array */
>  	__u32 num_items;
>  
> -	/*
> -	 * Unused for now. Must be cleared to zero.
> +	/**
> +	 * @flags: Unused for now. Must be cleared to zero.
>  	 */
>  	__u32 flags;
>  
> -	/*
> -	 * This points to an array of num_items drm_i915_query_item structures.
> +	/**
> +	 * @items_ptr: This points to an array of num_items drm_i915_query_item
> +	 * structures.
>  	 */
>  	__u64 items_ptr;
>  };
>
Tvrtko Ursulin April 16, 2021, 7:57 a.m. UTC | #2
On 15/04/2021 16:59, Matthew Auld wrote:
> Add a note about the two-step process.
> 
> Suggested-by: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Kenneth Graunke <kenneth@whitecape.org>
> Cc: Jason Ekstrand <jason@jlekstrand.net>
> Cc: Dave Airlie <airlied@gmail.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: mesa-dev@lists.freedesktop.org
> ---
>   include/uapi/drm/i915_drm.h | 57 ++++++++++++++++++++++++++++++-------
>   1 file changed, 46 insertions(+), 11 deletions(-)
> 
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index d9c954a5a456..ef36f1a0adde 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -2210,14 +2210,23 @@ struct drm_i915_perf_oa_config {
>   	__u64 flex_regs_ptr;
>   };
>   
> +/**
> + * struct drm_i915_query_item - An individual query for the kernel to process.
> + *
> + * The behaviour is determined by the @query_id. Note that exactly what
> + * @data_ptr is also depends on the specific @query_id.
> + */
>   struct drm_i915_query_item {
> +	/** @query_id: The id for this query */
>   	__u64 query_id;
>   #define DRM_I915_QUERY_TOPOLOGY_INFO    1
>   #define DRM_I915_QUERY_ENGINE_INFO	2
>   #define DRM_I915_QUERY_PERF_CONFIG      3
>   /* Must be kept compact -- no holes and well documented */
>   
> -	/*
> +	/**
> +	 * @length:
> +	 *
>   	 * When set to zero by userspace, this is filled with the size of the
>   	 * data to be written at the data_ptr pointer. The kernel sets this
>   	 * value to a negative value to signal an error on a particular query
> @@ -2225,21 +2234,26 @@ struct drm_i915_query_item {
>   	 */
>   	__s32 length;
>   
> -	/*
> +	/**
> +	 * @flags:
> +	 *
>   	 * When query_id == DRM_I915_QUERY_TOPOLOGY_INFO, must be 0.
>   	 *
>   	 * When query_id == DRM_I915_QUERY_PERF_CONFIG, must be one of the
> -	 * following :
> -	 *         - DRM_I915_QUERY_PERF_CONFIG_LIST
> -	 *         - DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID
> -	 *         - DRM_I915_QUERY_PERF_CONFIG_FOR_UUID
> +	 * following:
> +	 *
> +	 *	- DRM_I915_QUERY_PERF_CONFIG_LIST
> +	 *      - DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID
> +	 *      - DRM_I915_QUERY_PERF_CONFIG_FOR_UUID
>   	 */
>   	__u32 flags;
>   #define DRM_I915_QUERY_PERF_CONFIG_LIST          1
>   #define DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID 2
>   #define DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_ID   3
>   
> -	/*
> +	/**
> +	 * @data_ptr:
> +	 *
>   	 * Data will be written at the location pointed by data_ptr when the
>   	 * value of length matches the length of the data to be written by the
>   	 * kernel.
> @@ -2247,16 +2261,37 @@ struct drm_i915_query_item {
>   	__u64 data_ptr;
>   };
>   
> +/**
> + * struct drm_i915_query - Supply an array of drm_i915_query_item for the kernel
> + * to fill out.
> + *
> + * Note that this is generally a two step process for each drm_i915_query_item
> + * in the array:
> + *
> + *	1.) Call the DRM_IOCTL_I915_QUERY, giving it our array of
> + *	drm_i915_query_item, with drm_i915_query_item.size set to zero. The
> + *	kernel will then fill in the size, in bytes, which tells userspace how
> + *	memory it needs to allocate for the blob(say for an array of
> + *	properties).
> + *
> + *	2.) Next we call DRM_IOCTL_I915_QUERY again, this time with the
> + *	drm_i915_query_item.data_ptr equal to our newly allocated blob. Note
> + *	that the i915_query_item.size should still be the same as what the
> + *	kernel previously set. At this point the kernel can fill in the blob.

I'd also document the one step alternative where userspace passes in a 
buffer equal or larger than the required size. For many small queries 
(most?) this actually works just as well and with one ioctl only.

Regards,

Tvrtko

> + *
> + */
>   struct drm_i915_query {
> +	/** @num_items: The number of elements in the @items_ptr array */
>   	__u32 num_items;
>   
> -	/*
> -	 * Unused for now. Must be cleared to zero.
> +	/**
> +	 * @flags: Unused for now. Must be cleared to zero.
>   	 */
>   	__u32 flags;
>   
> -	/*
> -	 * This points to an array of num_items drm_i915_query_item structures.
> +	/**
> +	 * @items_ptr: This points to an array of num_items drm_i915_query_item
> +	 * structures.
>   	 */
>   	__u64 items_ptr;
>   };
>
Daniel Vetter April 16, 2021, 8:49 a.m. UTC | #3
On Thu, Apr 15, 2021 at 04:59:57PM +0100, Matthew Auld wrote:
> Add a note about the two-step process.
> 
> Suggested-by: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Kenneth Graunke <kenneth@whitecape.org>
> Cc: Jason Ekstrand <jason@jlekstrand.net>
> Cc: Dave Airlie <airlied@gmail.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: mesa-dev@lists.freedesktop.org
> ---
>  include/uapi/drm/i915_drm.h | 57 ++++++++++++++++++++++++++++++-------
>  1 file changed, 46 insertions(+), 11 deletions(-)
> 
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index d9c954a5a456..ef36f1a0adde 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -2210,14 +2210,23 @@ struct drm_i915_perf_oa_config {
>  	__u64 flex_regs_ptr;
>  };
>  
> +/**
> + * struct drm_i915_query_item - An individual query for the kernel to process.
> + *
> + * The behaviour is determined by the @query_id. Note that exactly what
> + * @data_ptr is also depends on the specific @query_id.
> + */
>  struct drm_i915_query_item {
> +	/** @query_id: The id for this query */
>  	__u64 query_id;
>  #define DRM_I915_QUERY_TOPOLOGY_INFO    1
>  #define DRM_I915_QUERY_ENGINE_INFO	2
>  #define DRM_I915_QUERY_PERF_CONFIG      3
>  /* Must be kept compact -- no holes and well documented */
>  
> -	/*
> +	/**
> +	 * @length:
> +	 *
>  	 * When set to zero by userspace, this is filled with the size of the
>  	 * data to be written at the data_ptr pointer. The kernel sets this
>  	 * value to a negative value to signal an error on a particular query
> @@ -2225,21 +2234,26 @@ struct drm_i915_query_item {
>  	 */
>  	__s32 length;
>  
> -	/*
> +	/**
> +	 * @flags:
> +	 *
>  	 * When query_id == DRM_I915_QUERY_TOPOLOGY_INFO, must be 0.
>  	 *
>  	 * When query_id == DRM_I915_QUERY_PERF_CONFIG, must be one of the
> -	 * following :
> -	 *         - DRM_I915_QUERY_PERF_CONFIG_LIST
> -	 *         - DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID
> -	 *         - DRM_I915_QUERY_PERF_CONFIG_FOR_UUID
> +	 * following:
> +	 *
> +	 *	- DRM_I915_QUERY_PERF_CONFIG_LIST
> +	 *      - DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID
> +	 *      - DRM_I915_QUERY_PERF_CONFIG_FOR_UUID
>  	 */
>  	__u32 flags;
>  #define DRM_I915_QUERY_PERF_CONFIG_LIST          1
>  #define DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID 2
>  #define DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_ID   3
>  
> -	/*
> +	/**
> +	 * @data_ptr:
> +	 *
>  	 * Data will be written at the location pointed by data_ptr when the
>  	 * value of length matches the length of the data to be written by the
>  	 * kernel.
> @@ -2247,16 +2261,37 @@ struct drm_i915_query_item {
>  	__u64 data_ptr;
>  };
>  
> +/**
> + * struct drm_i915_query - Supply an array of drm_i915_query_item for the kernel
> + * to fill out.
> + *
> + * Note that this is generally a two step process for each drm_i915_query_item
> + * in the array:
> + *
> + *	1.) Call the DRM_IOCTL_I915_QUERY, giving it our array of

I'm not sure this results in pretty rendering in htmldocs output. Please
check this.

This also made me realize that we're not pulling any of this into the drm
documents at all. I'll revise my review on patch 1.

Docs here look good:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>


> + *	drm_i915_query_item, with drm_i915_query_item.size set to zero. The
> + *	kernel will then fill in the size, in bytes, which tells userspace how
> + *	memory it needs to allocate for the blob(say for an array of
> + *	properties).
> + *
> + *	2.) Next we call DRM_IOCTL_I915_QUERY again, this time with the
> + *	drm_i915_query_item.data_ptr equal to our newly allocated blob. Note
> + *	that the i915_query_item.size should still be the same as what the
> + *	kernel previously set. At this point the kernel can fill in the blob.
> + *
> + */
>  struct drm_i915_query {
> +	/** @num_items: The number of elements in the @items_ptr array */
>  	__u32 num_items;
>  
> -	/*
> -	 * Unused for now. Must be cleared to zero.
> +	/**
> +	 * @flags: Unused for now. Must be cleared to zero.
>  	 */
>  	__u32 flags;
>  
> -	/*
> -	 * This points to an array of num_items drm_i915_query_item structures.
> +	/**
> +	 * @items_ptr: This points to an array of num_items drm_i915_query_item
> +	 * structures.
>  	 */
>  	__u64 items_ptr;
>  };
> -- 
> 2.26.3
>
Daniel Vetter April 16, 2021, 8:59 a.m. UTC | #4
On Fri, Apr 16, 2021 at 12:25 AM Ian Romanick <idr@freedesktop.org> wrote:
> On 4/15/21 8:59 AM, Matthew Auld wrote:
> > Add a note about the two-step process.
> >
> > Suggested-by: Daniel Vetter <daniel@ffwll.ch>
> > Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Kenneth Graunke <kenneth@whitecape.org>
> > Cc: Jason Ekstrand <jason@jlekstrand.net>
> > Cc: Dave Airlie <airlied@gmail.com>
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: mesa-dev@lists.freedesktop.org
> > ---
> >  include/uapi/drm/i915_drm.h | 57 ++++++++++++++++++++++++++++++-------
> >  1 file changed, 46 insertions(+), 11 deletions(-)
> >
> > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> > index d9c954a5a456..ef36f1a0adde 100644
> > --- a/include/uapi/drm/i915_drm.h
> > +++ b/include/uapi/drm/i915_drm.h
> > @@ -2210,14 +2210,23 @@ struct drm_i915_perf_oa_config {
> >       __u64 flex_regs_ptr;
> >  };
> >
> > +/**
> > + * struct drm_i915_query_item - An individual query for the kernel to process.
> > + *
> > + * The behaviour is determined by the @query_id. Note that exactly what
>
> Since we just had a big discussion about this on mesa-dev w.r.t. Mesa
> code and documentation... does the kernel have a policy about which
> flavor (pun intended) of English should be used?

I'm not finding it documented in
https://dri.freedesktop.org/docs/drm/doc-guide/sphinx.html but I
thought we've discussed it. Adding linux-doc and Jon Corbet.
-Daniel

>
> > + * @data_ptr is also depends on the specific @query_id.
> > + */
> >  struct drm_i915_query_item {
> > +     /** @query_id: The id for this query */
> >       __u64 query_id;
> >  #define DRM_I915_QUERY_TOPOLOGY_INFO    1
> >  #define DRM_I915_QUERY_ENGINE_INFO   2
> >  #define DRM_I915_QUERY_PERF_CONFIG      3
> >  /* Must be kept compact -- no holes and well documented */
> >
> > -     /*
> > +     /**
> > +      * @length:
> > +      *
> >        * When set to zero by userspace, this is filled with the size of the
> >        * data to be written at the data_ptr pointer. The kernel sets this
> >        * value to a negative value to signal an error on a particular query
> > @@ -2225,21 +2234,26 @@ struct drm_i915_query_item {
> >        */
> >       __s32 length;
> >
> > -     /*
> > +     /**
> > +      * @flags:
> > +      *
> >        * When query_id == DRM_I915_QUERY_TOPOLOGY_INFO, must be 0.
> >        *
> >        * When query_id == DRM_I915_QUERY_PERF_CONFIG, must be one of the
> > -      * following :
> > -      *         - DRM_I915_QUERY_PERF_CONFIG_LIST
> > -      *         - DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID
> > -      *         - DRM_I915_QUERY_PERF_CONFIG_FOR_UUID
> > +      * following:
> > +      *
> > +      *      - DRM_I915_QUERY_PERF_CONFIG_LIST
> > +      *      - DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID
> > +      *      - DRM_I915_QUERY_PERF_CONFIG_FOR_UUID
> >        */
> >       __u32 flags;
> >  #define DRM_I915_QUERY_PERF_CONFIG_LIST          1
> >  #define DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID 2
> >  #define DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_ID   3
> >
> > -     /*
> > +     /**
> > +      * @data_ptr:
> > +      *
> >        * Data will be written at the location pointed by data_ptr when the
> >        * value of length matches the length of the data to be written by the
> >        * kernel.
> > @@ -2247,16 +2261,37 @@ struct drm_i915_query_item {
> >       __u64 data_ptr;
> >  };
> >
> > +/**
> > + * struct drm_i915_query - Supply an array of drm_i915_query_item for the kernel
> > + * to fill out.
> > + *
> > + * Note that this is generally a two step process for each drm_i915_query_item
> > + * in the array:
> > + *
> > + *   1.) Call the DRM_IOCTL_I915_QUERY, giving it our array of
> > + *   drm_i915_query_item, with drm_i915_query_item.size set to zero. The
> > + *   kernel will then fill in the size, in bytes, which tells userspace how
> > + *   memory it needs to allocate for the blob(say for an array of
> > + *   properties).
> > + *
> > + *   2.) Next we call DRM_IOCTL_I915_QUERY again, this time with the
> > + *   drm_i915_query_item.data_ptr equal to our newly allocated blob. Note
> > + *   that the i915_query_item.size should still be the same as what the
> > + *   kernel previously set. At this point the kernel can fill in the blob.
> > + *
> > + */
> >  struct drm_i915_query {
> > +     /** @num_items: The number of elements in the @items_ptr array */
> >       __u32 num_items;
> >
> > -     /*
> > -      * Unused for now. Must be cleared to zero.
> > +     /**
> > +      * @flags: Unused for now. Must be cleared to zero.
> >        */
> >       __u32 flags;
> >
> > -     /*
> > -      * This points to an array of num_items drm_i915_query_item structures.
> > +     /**
> > +      * @items_ptr: This points to an array of num_items drm_i915_query_item
> > +      * structures.
> >        */
> >       __u64 items_ptr;
> >  };
> >
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Jonathan Corbet April 16, 2021, 7:04 p.m. UTC | #5
Daniel Vetter <daniel@ffwll.ch> writes:

> On Fri, Apr 16, 2021 at 12:25 AM Ian Romanick <idr@freedesktop.org> wrote:
>> Since we just had a big discussion about this on mesa-dev w.r.t. Mesa
>> code and documentation... does the kernel have a policy about which
>> flavor (pun intended) of English should be used?
>
> I'm not finding it documented in
> https://dri.freedesktop.org/docs/drm/doc-guide/sphinx.html but I
> thought we've discussed it. Adding linux-doc and Jon Corbet.

It's in Documentation/doc-guide/contributing.rst:

> Please note that some things are *not* typos and should not be "fixed":
> 
>  - Both American and British English spellings are allowed within the
>    kernel documentation.  There is no need to fix one by replacing it with
>    the other.

Thanks,

jon
diff mbox series

Patch

diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index d9c954a5a456..ef36f1a0adde 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -2210,14 +2210,23 @@  struct drm_i915_perf_oa_config {
 	__u64 flex_regs_ptr;
 };
 
+/**
+ * struct drm_i915_query_item - An individual query for the kernel to process.
+ *
+ * The behaviour is determined by the @query_id. Note that exactly what
+ * @data_ptr is also depends on the specific @query_id.
+ */
 struct drm_i915_query_item {
+	/** @query_id: The id for this query */
 	__u64 query_id;
 #define DRM_I915_QUERY_TOPOLOGY_INFO    1
 #define DRM_I915_QUERY_ENGINE_INFO	2
 #define DRM_I915_QUERY_PERF_CONFIG      3
 /* Must be kept compact -- no holes and well documented */
 
-	/*
+	/**
+	 * @length:
+	 *
 	 * When set to zero by userspace, this is filled with the size of the
 	 * data to be written at the data_ptr pointer. The kernel sets this
 	 * value to a negative value to signal an error on a particular query
@@ -2225,21 +2234,26 @@  struct drm_i915_query_item {
 	 */
 	__s32 length;
 
-	/*
+	/**
+	 * @flags:
+	 *
 	 * When query_id == DRM_I915_QUERY_TOPOLOGY_INFO, must be 0.
 	 *
 	 * When query_id == DRM_I915_QUERY_PERF_CONFIG, must be one of the
-	 * following :
-	 *         - DRM_I915_QUERY_PERF_CONFIG_LIST
-	 *         - DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID
-	 *         - DRM_I915_QUERY_PERF_CONFIG_FOR_UUID
+	 * following:
+	 *
+	 *	- DRM_I915_QUERY_PERF_CONFIG_LIST
+	 *      - DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID
+	 *      - DRM_I915_QUERY_PERF_CONFIG_FOR_UUID
 	 */
 	__u32 flags;
 #define DRM_I915_QUERY_PERF_CONFIG_LIST          1
 #define DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID 2
 #define DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_ID   3
 
-	/*
+	/**
+	 * @data_ptr:
+	 *
 	 * Data will be written at the location pointed by data_ptr when the
 	 * value of length matches the length of the data to be written by the
 	 * kernel.
@@ -2247,16 +2261,37 @@  struct drm_i915_query_item {
 	__u64 data_ptr;
 };
 
+/**
+ * struct drm_i915_query - Supply an array of drm_i915_query_item for the kernel
+ * to fill out.
+ *
+ * Note that this is generally a two step process for each drm_i915_query_item
+ * in the array:
+ *
+ *	1.) Call the DRM_IOCTL_I915_QUERY, giving it our array of
+ *	drm_i915_query_item, with drm_i915_query_item.size set to zero. The
+ *	kernel will then fill in the size, in bytes, which tells userspace how
+ *	memory it needs to allocate for the blob(say for an array of
+ *	properties).
+ *
+ *	2.) Next we call DRM_IOCTL_I915_QUERY again, this time with the
+ *	drm_i915_query_item.data_ptr equal to our newly allocated blob. Note
+ *	that the i915_query_item.size should still be the same as what the
+ *	kernel previously set. At this point the kernel can fill in the blob.
+ *
+ */
 struct drm_i915_query {
+	/** @num_items: The number of elements in the @items_ptr array */
 	__u32 num_items;
 
-	/*
-	 * Unused for now. Must be cleared to zero.
+	/**
+	 * @flags: Unused for now. Must be cleared to zero.
 	 */
 	__u32 flags;
 
-	/*
-	 * This points to an array of num_items drm_i915_query_item structures.
+	/**
+	 * @items_ptr: This points to an array of num_items drm_i915_query_item
+	 * structures.
 	 */
 	__u64 items_ptr;
 };