[Intel-xe] [PATCH v2 27/30] drm/xe: Allow GT looping and lookup on standalone media

[Lucas De Marchi]

On Fri, May 19, 2023 at 04:18:24PM -0700, 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.  At the
>moment our platforms either have multi-tile (i.e., PVC) or standalone
>media (MTL) but not both.  If a future platform supports both of these
>capabilities at the same time, how will we number the GTs of the
>platform?   primary-primary-media-media?  primary-media-primary-media?
>For that matter should we even still be exposing the concept of 'GT' to
>userspace or should that switch to tile instead (and keep the hardware's
>separation of render and media an internal implementation detail like it
>is on i915)?  If we only expose tiles to userspace and not GTs, then we
>may not even need per-GT ID numbers anymore.

is this something to leave on the commit or something to solve now?
I think userspace may care about what tile the GT is located, but not
much about the order... I think just making it a depth-first on each
tile would be ok and we can have a tile as a gt attribute.

>
>Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
>---
> drivers/gpu/drm/xe/xe_device.h | 27 +++++++++++++++++++++------
> 1 file changed, 21 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h
>index 156c62ac0381..e0085fcd3f47 100644
>--- a/drivers/gpu/drm/xe/xe_device.h
>+++ b/drivers/gpu/drm/xe/xe_device.h
>@@ -55,16 +55,27 @@ static inline struct xe_tile *xe_device_get_root_tile(struct xe_device *xe)
>
> 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);
>+	if (drm_WARN_ON(&xe->drm, gt_id > XE_MAX_TILES_PER_DEVICE))

this condition is odd.

>+		return root_tile->primary_gt;
>
>-	gt = xe->tiles[gt_id].primary_gt;
>-	if (drm_WARN_ON(&xe->drm, !gt))
>+	/*
>+	 * FIXME: This only works for now because multi-tile and standalone
>+	 * media are mutually exclusive on the platforms we have today.
>+	 */
>+	if (MEDIA_VER(xe) >= 13) {
>+		gt = gt_id ? root_tile->media_gt : root_tile->primary_gt;
>+	} else {
>+		gt = xe->tiles[gt_id].primary_gt;
>+	}

I would expect something like:

	for_each_tile(...)
		for_each_gt(...)
			if (gt->id == gt_id)
				return gt_id

which could be optimized by reserving the gt numbers
with XE_MAX_TILES_PER_DEVICE

#define XE_MAX_GTS_PER_TILE 2

	tile = gt_id / XE_MAX_GTS_PER_TILE;
	gt = gt_id % XE_MAX_GTS_PER_TILE;
	return !gt ? xe->tiles[tile].primary_gt : xe->tiles[tile].media_gt;

but hey... why are we not keeping a gt[] to make things simpler instead
of calling them explicitly primary_gt/media_gt?

Lucas De Marchi

>+
>+	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;
> }
>@@ -98,8 +109,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__++)) \
> 		for_each_if ((gt__) = xe_device_get_gt((xe__), (id__)))
>
> static inline struct xe_force_wake * gt_to_fw(struct xe_gt *gt)
>-- 
>2.40.0
>