On Wed, Aug 30, 2023 at 09:30:01PM +0530, Shekhar Chauhan wrote: > Disables Atomic-chaining of Typed Writes. > > BSpec: 54040 > Signed-off-by: Shekhar Chauhan <shekhar.chauhan@xxxxxxxxx> > --- > drivers/gpu/drm/i915/gt/intel_gt_regs.h | 2 ++ > drivers/gpu/drm/i915/gt/intel_workarounds.c | 10 ++++++++++ > 2 files changed, 12 insertions(+) > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h > index 0e4c638fcbbf..82b533aa0c03 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h > @@ -1218,6 +1218,8 @@ > > #define XEHP_HDC_CHICKEN0 MCR_REG(0xe5f0) > #define LSC_L1_FLUSH_CTL_3D_DATAPORT_FLUSH_EVENTS_MASK REG_GENMASK(13, 11) > +#define ATOMIC_CHAINING_TYPED_WRITES REG_BIT(3) > + > #define ICL_HDC_MODE MCR_REG(0xe5f4) > > #define EU_PERF_CNTL2 PERF_REG(0xe658) > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c > index 864d41bcf6bb..d54120009334 100644 > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c > @@ -2327,6 +2327,16 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal) > LSC_L1_FLUSH_CTL_3D_DATAPORT_FLUSH_EVENTS_MASK); > } > > + if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_C0, STEP_FOREVER) || > + IS_DG2_GRAPHICS_STEP(i915, G11, STEP_B0, STEP_FOREVER) || > + IS_DG2_G12(i915) || Since DG2 is at the point where we only support production steppings, we don't need to worry about avoiding this workaround on old pre-production steppings. So these three conditions can be simplified down to just "IS_DG2(i915)." See https://lore.kernel.org/intel-gfx/20230816214201.534095-7-matthew.d.roper@xxxxxxxxx/ for details. > + IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_FOREVER) || > + IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_FOREVER)) { This macro doesn't exist anymore; I think you need to rebase your patch on a newer snapshot of drm-tip. BTW, "A0..forever" covers every single stepping, so there's no need to use a stepping-based check in such cases. Once you rebase, this will just turn into IS_GFX_GT_IP_RANGE(engine->gt, IP_VER(12, 70), IP_VER(12, 71)) Also note that we generally put the newer platform/IP first in our conditions, so the overall check will become: if (IS_GFX_GT_IP_RANGE(gt, IP_VER(12, 70), IP_VER(12, 71)) || IS_DG2(i915)) > + /* Wa_14015150844 */ > + wa_mcr_masked_dis(wal, XEHP_HDC_CHICKEN0, > + ATOMIC_CHAINING_TYPED_WRITES); It's easy to miss, but up near the top of bspec 54040, there's a note about this register being a "write only" register. That means that if we implement this workaround the normal way we'll get driver warnings on debug builds when it tries to read back the register later and make sure the workaround was applied properly. We need to use the more explicit wa_mcr_add() function to provide an empty readmask to avoid this. The next workaround just below this in the code is an example of how to do that. Matt > + } > + > if (IS_DG2_G11(i915) || IS_DG2_G10(i915)) { > /* Wa_22014600077:dg2 */ > wa_mcr_add(wal, GEN10_CACHE_MODE_SS, 0, > -- > 2.34.1 > -- Matt Roper Graphics Software Engineer Linux GPU Platform Enablement Intel Corporation