[Intel-xe] [RFC 2/5] drm/xe/uapi: Document drm_xe_query_gt.

[Matt Roper]

On Fri, Sep 08, 2023 at 04:32:59PM -0400, Rodrigo Vivi wrote:
> Split drm_xe_query_gt out of the gt list one in order to better
> document it. No functional change at this point.
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> ---
>  include/uapi/drm/xe_drm.h | 65 ++++++++++++++++++++++++++-------------
>  1 file changed, 43 insertions(+), 22 deletions(-)
> 
> diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
> index 51c4ef5dee6d..4bcee709bef9 100644
> --- a/include/uapi/drm/xe_drm.h
> +++ b/include/uapi/drm/xe_drm.h
> @@ -262,6 +262,47 @@ struct drm_xe_query_config {
>  	__u64 info[];
>  };
>  
> +/**
> + * struct drm_xe_query_gt - describe an individual GT.
> + *
> + * To be used with drm_xe_query_gts, which will return a list with all the
> + * existing GT individual descriptions.
> + * Graphics Technology (GT) is a subset of a GPU/tile that is responsible for
> + * implementing graphics and/or media operations.
> + */
> +struct drm_xe_query_gt {
> +#define XE_QUERY_GT_TYPE_MAIN		0
> +#define XE_QUERY_GT_TYPE_REMOTE		1

I know the goal of this patch is just to add kerneldoc, but while we're
looking at this, one other thing we'll need to decide before finalizing
the uapi is whether there's any value in keeping these two separate.
I.e., does any userspace actually care about the difference between
"render GT on first tile" vs "render GT on other tiles?"  Given that GT
at the uapi level is pretty much only concerned with power management, I
can't really think of anywhere that the distinction would matter (but we
should still check with the userspace teams).

> +#define XE_QUERY_GT_TYPE_MEDIA		2
> +	/** @type: GT type: Main, Remote, or Media */
> +	__u16 type;
> +	/** @instance: Instance of this GT in the GT list */
> +	__u16 instance;
> +	/** @clock_freq: A clock frequency for timestamp */
> +	__u32 clock_freq;

We may want to be more clear about what frequency this is.  I.e., is it
the current frequency when the query is issued?  Is it the maximum
frequency the GT supports?  Does this always give a meaningful value or
does it report 0 when nothing is executing (and we're in RC6)?

BTW, is this redundant with what we're exposing through sysfs?  Does
userspace get any benefit from grabbing frequency here rather than
through the sysfs interface?

> +	/** @features: Reserved for future information about GT features */
> +	__u64 features;
> +	/**
> +	 * @native_mem_regions: Bit maks of instances from
> +	 * drm_xe_query_mem_usage that lives on the same GPU/Tile and have
> +	 * direct access.
> +	 */
> +	__u64 native_mem_regions;

Was the plan to eventually move these memory region masks to a
tile-based uapi?  Leaving them in a GT query seems awkward since the GT
isn't directly related to memory regions.  It seems like what userspace
cares about would either be tile<->mem_region location or
hwe<->mem_region distance.  How is userspace utilizing this information
today (and are they needing to jump through extra hoops due to the
current placement?)?

Also s/maks/mask/ in the kerneldoc you added here (and in the ones
below).


Matt

> +	/**
> +	 * @slow_mem_regions: Bit maks of instances from
> +	 * drm_xe_query_mem_usage that this GT can indirectly access, although
> +	 * they live on a different GPU/Tile.
> +	 */
> +	__u64 slow_mem_regions;
> +	/**
> +	 * @inaccessible_mem_regions: Bit maks of instances from
> +	 * drm_xe_query_mem_usage that is not accessible by this GT at all.
> +	 */
> +	__u64 inaccessible_mem_regions;
> +	/** @reserved: Reserved */
> +	__u64 reserved[8];
> +};
> +
>  /**
>   * struct drm_xe_query_gts - describe GTs
>   *
> @@ -272,30 +313,10 @@ struct drm_xe_query_config {
>  struct drm_xe_query_gts {
>  	/** @num_gt: number of GTs returned in gts */
>  	__u32 num_gt;
> -
>  	/** @pad: MBZ */
>  	__u32 pad;
> -
> -	/**
> -	 * @gts: The GTs returned for this device
> -	 *
> -	 * TODO: convert drm_xe_query_gt to proper kernel-doc.
> -	 * TODO: Perhaps info about every mem region relative to this GT? e.g.
> -	 * bandwidth between this GT and remote region?
> -	 */
> -	struct drm_xe_query_gt {
> -#define XE_QUERY_GT_TYPE_MAIN		0
> -#define XE_QUERY_GT_TYPE_REMOTE		1
> -#define XE_QUERY_GT_TYPE_MEDIA		2
> -		__u16 type;
> -		__u16 instance;
> -		__u32 clock_freq;
> -		__u64 features;
> -		__u64 native_mem_regions;	/* bit mask of instances from drm_xe_query_mem_usage */
> -		__u64 slow_mem_regions;		/* bit mask of instances from drm_xe_query_mem_usage */
> -		__u64 inaccessible_mem_regions;	/* bit mask of instances from drm_xe_query_mem_usage */
> -		__u64 reserved[8];
> -	} gts[];
> +	/** @gts: The GT list returned for this device */
> +	struct drm_xe_query_gt gts[];
>  };
>  
>  /**
> -- 
> 2.41.0
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation