[Intel-xe] [PATCH v3 28/31] drm/xe: Allow GT looping and lookup on standalone media

[Welty, Brian]

On 5/30/2023 2:15 PM, Matt Roper wrote:
> Allow xe_device_get_gt() and for_each_gt() to operate as expected on
> platforms with standalone media.
> 
> FIXME: We need to figure out a consistent ID scheme for GTs.  This patch
> keeps the pre-existing behavior of 0/1 being the GT IDs for both PVC
> (multi-tile) and MTL (multi-GT), but depending on the direction we
> decide to go with uapi, we may change this in the future (e.g., to
> return 0/1 on PVC and 0/2 on MTL).  Or if we decide we only need to
> expose tiles to userspace and not GTs, we may not even need ID numbers
> for the GTs anymore.
> 
> v2:
>   - Restructure a bit to make the assertions more clear.
>   - Clarify in commit message that the goal here is to preserve existing
>     behavior; UAPI-visible changes may be introduced in the future once
>     we settle on what we really want.
> 
> Cc: Lucas De Marchi <lucas.demarchi at intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
> ---
>   drivers/gpu/drm/xe/xe_device.h | 40 +++++++++++++++++++++++++++++-----
>   1 file changed, 34 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h
> index 0a3a9134dd18..ea3270ea0d46 100644
> --- a/drivers/gpu/drm/xe/xe_device.h
> +++ b/drivers/gpu/drm/xe/xe_device.h
> @@ -53,18 +53,42 @@ static inline struct xe_tile *xe_device_get_root_tile(struct xe_device *xe)
>   	return &xe->tiles[0];
>   }
>   
> +#define XE_MAX_GT_PER_TILE 2
> +
> +static inline struct xe_gt *xe_tile_get_gt(struct xe_tile *tile, u8 gt_id)
> +{
> +	if (drm_WARN_ON(&tile_to_xe(tile)->drm, gt_id > XE_MAX_GT_PER_TILE))
> +		gt_id = 0;
> +
> +	return gt_id ? tile->media_gt : tile->primary_gt;
> +}
> +
>   static inline struct xe_gt *xe_device_get_gt(struct xe_device *xe, u8 gt_id)
>   {
> +	struct xe_tile *root_tile = xe_device_get_root_tile(xe);
>   	struct xe_gt *gt;
>   
> -	XE_BUG_ON(gt_id > XE_MAX_TILES_PER_DEVICE);
> +	/*
> +	 * FIXME: This only works for now because multi-tile and standalone
> +	 * media are mutually exclusive on the platforms we have today.
> +	 *
> +	 * id => GT mapping may change once we settle on how we want to handle
> +	 * our UAPI.
> +	 */
> +	if (MEDIA_VER(xe) >= 13) {
> +		gt = xe_tile_get_gt(root_tile, gt_id);
> +	} else {
> +		if (drm_WARN_ON(&xe->drm, gt_id > XE_MAX_TILES_PER_DEVICE))
> +			gt_id = 0;
>   
> -	gt = xe->tiles[gt_id].primary_gt;
> -	if (drm_WARN_ON(&xe->drm, !gt))
> +		gt = xe->tiles[gt_id].primary_gt;
> +	}
> +
> +	if (!gt)
>   		return NULL;
>   
> -	XE_BUG_ON(gt->info.id != gt_id);
> -	XE_BUG_ON(gt->info.type == XE_GT_TYPE_UNINITIALIZED);
> +	drm_WARN_ON(&xe->drm, gt->info.id != gt_id);
> +	drm_WARN_ON(&xe->drm, gt->info.type == XE_GT_TYPE_UNINITIALIZED);
>   
>   	return gt;
>   }
> @@ -100,8 +124,12 @@ static inline void xe_device_guc_submission_disable(struct xe_device *xe)
>   	for ((id__) = 0; (id__) < (xe__)->info.tile_count; (id__++)) \
>   		for_each_if ((tile__) = &(xe__)->tiles[(id__)])
>   
> +/*
> + * FIXME: This only works for now since multi-tile and standalone media
> + * happen to be mutually exclusive.  Future platforms may change this...
> + */
>   #define for_each_gt(gt__, xe__, id__) \
> -	for ((id__) = 0; (id__) < (xe__)->info.tile_count; (id__++)) \
> +	for ((id__) = 0; (id__) < (xe__)->info.tile_count + (MEDIA_VER(xe__) >= 13 ? 1 : 0); (id__++)) \


Did you consider to maintain a xe->info.gt_count instead of computing it
here inside the loop condition?   Seems a little cleaner, and could then
drop the num_gt() function you introduce in your next xe_query patch.
Or maybe is best as you have it until your questions about exposing
gt numbering to user-space are answered.

-Brian



>   		for_each_if ((gt__) = xe_device_get_gt((xe__), (id__)))
>   
>   static inline struct xe_force_wake * gt_to_fw(struct xe_gt *gt)