[Intel-xe] [PATCH v1 4/8] drm/xe/uapi: Rename *_mem_regions masks

[Matt Roper]

On Tue, Nov 14, 2023 at 01:34:30PM +0000, Francois Dugast wrote:
> From: Rodrigo Vivi <rodrigo.vivi at intel.com>
> 
> - 'native' doesn't make much sense on integrated devices.
> - 'slow' is not necessarily true and doesn't go well with opposition
>   to 'native'.
> 
> Instead, let's use 'near' vs 'far'. It makes sense with all the current
> Intel GPUs and it is future proof. Right now, there's absolutely no need
> to define among the 'far' memory, which ones are slower, either in terms
> of latency, nunmber of hops or bandwidth.
> 
> In case of this might become a requirement in the future, a new query
> could be added to indicate the certain 'distance' between a given engine
> and a memory_region. But for now, this fulfill all of the current
> requirements in the most straightforward way for the userspace drivers.
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Signed-off-by: Francois Dugast <francois.dugast at intel.com>
> ---
>  drivers/gpu/drm/xe/xe_query.c |  8 ++++----
>  include/uapi/drm/xe_drm.h     | 17 +++++++++--------
>  2 files changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_query.c b/drivers/gpu/drm/xe/xe_query.c
> index 48befd9f0812..8b5136460ea6 100644
> --- a/drivers/gpu/drm/xe/xe_query.c
> +++ b/drivers/gpu/drm/xe/xe_query.c
> @@ -377,12 +377,12 @@ static int query_gt_list(struct xe_device *xe, struct drm_xe_device_query *query
>  		gt_list->gt_list[id].gt_id = gt->info.id;
>  		gt_list->gt_list[id].clock_freq = gt->info.clock_freq;
>  		if (!IS_DGFX(xe))
> -			gt_list->gt_list[id].native_mem_regions = 0x1;
> +			gt_list->gt_list[id].near_mem_regions = 0x1;
>  		else
> -			gt_list->gt_list[id].native_mem_regions =
> +			gt_list->gt_list[id].near_mem_regions =
>  				BIT(gt_to_tile(gt)->id) << 1;
> -		gt_list->gt_list[id].slow_mem_regions = xe->info.mem_region_mask ^
> -			gt_list->gt_list[id].native_mem_regions;
> +		gt_list->gt_list[id].far_mem_regions = xe->info.mem_region_mask ^
> +			gt_list->gt_list[id].near_mem_regions;
>  	}
>  
>  	if (copy_to_user(query_ptr, gt_list, size)) {
> diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
> index a8d351c9fa7c..fd51d4444119 100644
> --- a/include/uapi/drm/xe_drm.h
> +++ b/include/uapi/drm/xe_drm.h
> @@ -349,17 +349,18 @@ struct drm_xe_query_gt {
>  	/** @clock_freq: A clock frequency for timestamp */
>  	__u32 clock_freq;
>  	/**
> -	 * @native_mem_regions: Bit mask of instances from
> -	 * drm_xe_query_mem_usage that lives on the same GPU/Tile and have
> -	 * direct access.
> +	 * @near_mem_regions: Bit mask of instances from
> +	 * drm_xe_query_mem_usage that is near the current engines of this GT.

s/is/are/.  And although "near" is good in the field name, I wonder if
the kerneldoc's explanation text should say "nearest" rather than "near"
to help clarify that this is a relative rather than based on some
absolute metric.

>  	 */
> -	__u64 native_mem_regions;
> +	__u64 near_mem_regions;
>  	/**
> -	 * @slow_mem_regions: Bit mask of instances from
> -	 * drm_xe_query_mem_usage that this GT can indirectly access, although
> -	 * they live on a different GPU/Tile.
> +	 * @far_mem_regions: Bit mask of instances from
> +	 * drm_xe_query_mem_usage that is far from the engines of this GT.

s/is/are/ again.

> +	 * In general, it has extra indirections when compared to the

And since instances was plural, also s/it has/they have/.

> +	 * @near_mem_regions. For a discrete device this could mean system
> +	 * memory and memory living in a different Tile.

No need to capitalize "tile."


Aside from those minor nitpicks,

Reviewed-by: Matt Roper <matthew.d.roper at intel.com>

>  	 */
> -	__u64 slow_mem_regions;
> +	__u64 far_mem_regions;
>  	/** @reserved: Reserved */
>  	__u64 reserved[8];
>  };
> -- 
> 2.34.1
> 

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