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
