Re: [PATCH v2 2/3] drm/i915/mtl: Correct implementation of Wa_18018781329

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jan 25, 2023 at 03:41:58PM -0800, Matt Roper wrote:
> Workaround Wa_18018781329 has applied to several recent Xe_HP-based
> platforms.  However there are some extra gotchas to implementing this
> properly for MTL that we need to take into account:
> 
>  * Due to the separation of media and render/compute into separate GTs,
>    this workaround needs to be implemented on each GT, not just the
>    primary GT.  Since each class of register only exists on one of the
>    two GTs, we should program the appropriate registers on each GT.
> 
>  * As with past Xe_HP platforms, the registers on the primary GT (Xe_LPG
>    IP) are multicast/replicated registers and should be handled with the
>    MCR-aware functions.  However the registers on the media GT (Xe_LPM+
>    IP) are regular singleton registers and should _not_ use MCR
>    handling.  We need to create separate register definitions for the
>    Xe_HP multicast form and the Xe_LPM+ singleton form and use each in
>    the appropriate place.
> 
>  * Starting with MTL, workarounds documented by the hardware teams are
>    technically associated with IP versions/steppings rather than
>    top-level platforms.  That means we should take care to check the
>    media IP version rather than the graphics IP version when deciding
>    whether the workaround is needed on the Xe_LPM+ media GT (in this
>    case the workaround applies to both IPs and the stepping bounds are
>    identical, but we should still write the code appropriately to set a
>    proper precedent for future workaround implementations).
> 
>  * It's worth noting that the GSC register and the CCS register are
>    defined with the same MMIO offset (0xCF30).  Since the CCS is only
>    relevant to the primary GT and the GSC is only relevant to the media
>    GT there isn't actually a clash here (the media GT automatically adds
>    the additional 0x380000 GSI offset).  However there's currently a
>    glitch in the bspec where the CCS register doesn't show up at all and
>    the GSC register is listed as existing on both GTs.  That's a known
>    documentation problem for several registers with shared GSC/CCS
>    offsets; rest assured that the CCS register really does still exist.
> 
> Cc: Gustavo Sousa <gustavo.sousa@xxxxxxxxx>
> Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>

Forgot to add:

Fixes: 41bb543f5598 ("drm/i915/mtl: Add initial gt workarounds")


Matt

> ---
>  drivers/gpu/drm/i915/gt/intel_gt_regs.h     |  7 +++++--
>  drivers/gpu/drm/i915/gt/intel_workarounds.c | 22 ++++++++++++++-------
>  drivers/gpu/drm/i915/i915_drv.h             |  4 ++++
>  3 files changed, 24 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index 2727645864db..310bdde049ab 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -1100,8 +1100,11 @@
>  #define XEHP_MERT_MOD_CTRL			MCR_REG(0xcf28)
>  #define RENDER_MOD_CTRL				MCR_REG(0xcf2c)
>  #define COMP_MOD_CTRL				MCR_REG(0xcf30)
> -#define VDBX_MOD_CTRL				MCR_REG(0xcf34)
> -#define VEBX_MOD_CTRL				MCR_REG(0xcf38)
> +#define XELPMP_GSC_MOD_CTRL			_MMIO(0xcf30)	/* media GT only */
> +#define XEHP_VDBX_MOD_CTRL			MCR_REG(0xcf34)
> +#define XELPMP_VDBX_MOD_CTRL			_MMIO(0xcf34)
> +#define XEHP_VEBX_MOD_CTRL			MCR_REG(0xcf38)
> +#define XELPMP_VEBX_MOD_CTRL			_MMIO(0xcf38)
>  #define   FORCE_MISS_FTLB			REG_BIT(3)
>  
>  #define GEN12_GAMSTLB_CTRL			_MMIO(0xcf4c)
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 9db60078460a..4c978abf3e2a 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -1681,8 +1681,8 @@ dg2_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
>  	/* Wa_18018781329 */
>  	wa_mcr_write_or(wal, RENDER_MOD_CTRL, FORCE_MISS_FTLB);
>  	wa_mcr_write_or(wal, COMP_MOD_CTRL, FORCE_MISS_FTLB);
> -	wa_mcr_write_or(wal, VDBX_MOD_CTRL, FORCE_MISS_FTLB);
> -	wa_mcr_write_or(wal, VEBX_MOD_CTRL, FORCE_MISS_FTLB);
> +	wa_mcr_write_or(wal, XEHP_VDBX_MOD_CTRL, FORCE_MISS_FTLB);
> +	wa_mcr_write_or(wal, XEHP_VEBX_MOD_CTRL, FORCE_MISS_FTLB);
>  
>  	/* Wa_1509235366:dg2 */
>  	wa_write_or(wal, GEN12_GAMCNTRL_CTRL,
> @@ -1700,8 +1700,8 @@ pvc_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
>  	/* Wa_18018781329 */
>  	wa_mcr_write_or(wal, RENDER_MOD_CTRL, FORCE_MISS_FTLB);
>  	wa_mcr_write_or(wal, COMP_MOD_CTRL, FORCE_MISS_FTLB);
> -	wa_mcr_write_or(wal, VDBX_MOD_CTRL, FORCE_MISS_FTLB);
> -	wa_mcr_write_or(wal, VEBX_MOD_CTRL, FORCE_MISS_FTLB);
> +	wa_mcr_write_or(wal, XEHP_VDBX_MOD_CTRL, FORCE_MISS_FTLB);
> +	wa_mcr_write_or(wal, XEHP_VEBX_MOD_CTRL, FORCE_MISS_FTLB);
>  }
>  
>  static void
> @@ -1715,8 +1715,6 @@ xelpg_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
>  		/* Wa_18018781329 */
>  		wa_mcr_write_or(wal, RENDER_MOD_CTRL, FORCE_MISS_FTLB);
>  		wa_mcr_write_or(wal, COMP_MOD_CTRL, FORCE_MISS_FTLB);
> -		wa_mcr_write_or(wal, VDBX_MOD_CTRL, FORCE_MISS_FTLB);
> -		wa_mcr_write_or(wal, VEBX_MOD_CTRL, FORCE_MISS_FTLB);
>  	}
>  
>  	/*
> @@ -1729,7 +1727,17 @@ xelpg_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
>  static void
>  xelpmp_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
>  {
> -	/* FIXME: Actual workarounds will be added in future patch(es) */
> +	if (IS_MTL_MEDIA_STEP(gt->i915, STEP_A0, STEP_B0)) {
> +		/*
> +		 * Wa_18018781329
> +		 *
> +		 * Note that although these registers are MCR on the primary
> +		 * GT, the media GT's versions are regular singleton registers.
> +		 */
> +		wa_write_or(wal, XELPMP_GSC_MOD_CTRL, FORCE_MISS_FTLB);
> +		wa_write_or(wal, XELPMP_VDBX_MOD_CTRL, FORCE_MISS_FTLB);
> +		wa_write_or(wal, XELPMP_VEBX_MOD_CTRL, FORCE_MISS_FTLB);
> +	}
>  
>  	debug_dump_steering(gt);
>  }
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 48c838b4ea62..4295306487c7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -696,6 +696,10 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
>  	(IS_METEORLAKE(__i915) && \
>  	 IS_DISPLAY_STEP(__i915, since, until))
>  
> +#define IS_MTL_MEDIA_STEP(__i915, since, until) \
> +	(IS_METEORLAKE(__i915) && \
> +	 IS_MEDIA_STEP(__i915, since, until))
> +
>  /*
>   * DG2 hardware steppings are a bit unusual.  The hardware design was forked to
>   * create three variants (G10, G11, and G12) which each have distinct
> -- 
> 2.39.1
> 

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



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux