On Wed, Jan 25, 2023 at 03:43:26PM -0800, Matt Roper wrote:
> 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")
>
Reviewed-by: Gustavo Sousa <gustavo.sousa@xxxxxxxxx>
>
> 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
