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

[Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>]

On 06.03.2023 14:36, 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@intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_hw_engine.c | 35 +++++++++++++++++++------------
>  1 file changed, 22 insertions(+), 13 deletions(-)

Reviewed-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>

> 
> 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;
>  
> @@ -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
>