[Intel-xe] [PATCH 1/2] drm/xe: Separate engine fuse handling into dedicated functions

[Lucas De Marchi]

On Mon, Mar 06, 2023 at 02:36:50PM -0800, Matt Roper wrote:
>The single function to handle fuse registers for all types of engines is
>becoming a bit long and hard to follow (and we haven't even added the
>compute engines yet).  Let's split it into dedicated functions for each
>engine class.
>
>v2:
> - Add note about BCS0 always being present.  (Bala)
> - Add forcewake assertion to read_copy_fuses.  (Bala)
>
>Cc: Balasubramani Vivekanandan <balasubramani.vivekanandan at intel.com>
>Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
>---
> drivers/gpu/drm/xe/xe_hw_engine.c | 35 +++++++++++++++++++------------
> 1 file changed, 22 insertions(+), 13 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_hw_engine.c b/drivers/gpu/drm/xe/xe_hw_engine.c
>index b035e2fa6744..ac00693ba0b2 100644
>--- a/drivers/gpu/drm/xe/xe_hw_engine.c
>+++ b/drivers/gpu/drm/xe/xe_hw_engine.c
>@@ -369,29 +369,29 @@ static void hw_engine_setup_logical_mapping(struct xe_gt *gt)
> 	}
> }
>
>-static void read_fuses(struct xe_gt *gt)
>+static void read_media_fuses(struct xe_gt *gt)
> {
> 	struct xe_device *xe = gt_to_xe(gt);
> 	u32 media_fuse;
> 	u16 vdbox_mask;
> 	u16 vebox_mask;
>-	u32 bcs_mask;
> 	int i, j;
>
> 	xe_force_wake_assert_held(gt_to_fw(gt), XE_FW_GT);
>
>+	media_fuse = xe_mmio_read32(gt, GEN11_GT_VEBOX_VDBOX_DISABLE.reg);
>+
> 	/*
>-	 * FIXME: Hack job, thinking we should have table of vfuncs for each
>-	 * class which picks the correct vfunc based on IP version.
>+	 * Pre-Xe_HP platforms had register bits representing absent engines,
>+	 * whereas Xe_HP and beyond have bits representing present engines.
>+	 * Invert the polarity on old platforms so that we can use common
>+	 * handling below.
> 	 */
>-
>-	media_fuse = xe_mmio_read32(gt, GEN11_GT_VEBOX_VDBOX_DISABLE.reg);
> 	if (GRAPHICS_VERx100(xe) < 1250)
> 		media_fuse = ~media_fuse;
>
>-	vdbox_mask = media_fuse & GEN11_GT_VDBOX_DISABLE_MASK;
>-	vebox_mask = (media_fuse & GEN11_GT_VEBOX_DISABLE_MASK) >>
>-		      GEN11_GT_VEBOX_DISABLE_SHIFT;
>+	vdbox_mask = REG_FIELD_GET(GEN11_GT_VDBOX_DISABLE_MASK, media_fuse);
>+	vebox_mask = REG_FIELD_GET(GEN11_GT_VEBOX_DISABLE_MASK, media_fuse);
>
> 	for (i = XE_HW_ENGINE_VCS0, j = 0; i <= XE_HW_ENGINE_VCS7; ++i, ++j) {
> 		if (!(gt->info.engine_mask & BIT(i)))
>@@ -412,11 +412,20 @@ static void read_fuses(struct xe_gt *gt)
> 			drm_info(&xe->drm, "vecs%u fused off\n", j);
> 		}
> 	}
>+}
>+
>+static void read_copy_fuses(struct xe_gt *gt)
>+{
>+	struct xe_device *xe = gt_to_xe(gt);
>+	u32 bcs_mask;
>+
>+	xe_force_wake_assert_held(gt_to_fw(gt), XE_FW_GT);
>
> 	bcs_mask = xe_mmio_read32(gt, GEN10_MIRROR_FUSE3.reg);
> 	bcs_mask = REG_FIELD_GET(GEN12_MEML3_EN_MASK, bcs_mask);
>
>-	for (i = XE_HW_ENGINE_BCS1, j = 0; i <= XE_HW_ENGINE_BCS8; ++i, ++j) {
>+	/* BCS0 is always present; only BCS1-BCS8 may be fused off */
>+	for (int i = XE_HW_ENGINE_BCS1, j = 0; i <= XE_HW_ENGINE_BCS8; ++i, ++j) {
> 		if (!(gt->info.engine_mask & BIT(i)))
> 			continue;

while at it may be worth fixing the coding style one line below.
In any case,


Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>

thanks
Lucas De Marchi

>
>@@ -425,15 +434,15 @@ static void read_fuses(struct xe_gt *gt)
> 			drm_info(&xe->drm, "bcs%u fused off\n", j);
> 		}
> 	}
>-
>-	/* TODO: compute engines */
> }
>
> int xe_hw_engines_init_early(struct xe_gt *gt)
> {
> 	int i;
>
>-	read_fuses(gt);
>+	read_media_fuses(gt);
>+	read_copy_fuses(gt);
>+	/* TODO: compute engines */
>
> 	for (i = 0; i < ARRAY_SIZE(gt->hw_engines); i++)
> 		hw_engine_init_early(gt, &gt->hw_engines[i], i);
>-- 
>2.39.2
>