On Thu, Aug 31, 2023 at 09:29:25PM +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 | 8 ++++++++ > 2 files changed, 10 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) Since a value of "1" for this bit disables something in the hardware, we'd often put "DIS" or "DISABLE" in the name somewhere. E.g., the name we used for this bit in the Xe driver is "DIS_ATOMIC_CHAINING_TYPED_WRITES" which helps clarify the semantics a bit. > + > #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..d853f228fabd 100644 > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c > @@ -2327,6 +2327,14 @@ 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_GFX_GT_IP_RANGE(gt, IP_VER(12, 70), IP_VER(12, 71)) || > + IS_DG2(i915)) { > + /* Wa_14015150844 */ > + wa_mcr_add(wal, XEHP_HDC_CHICKEN0, 0, > + _MASKED_BIT_DISABLE(ATOMIC_CHAINING_TYPED_WRITES), _MASKED_BIT_DISABLE will disable the bit (i.e., set it to 0), which will enable the behavior in hardware (and a value of 0 is also the hardware default in this case). Since the workaround description is asking us to set the chicken bit (thus disabling the hardware behavior), I think we want _MASKED_BIT_ENABLE here. It's always a bit confusing when enabling a bit disables a behavior and vice versa, but it's a pretty common pattern for hardware chicken bits like these. Matt > + 0, true); > + } > + > 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