> -----Original Message----- > From: Roper, Matthew D <matthew.d.roper@xxxxxxxxx> > Sent: Saturday, September 9, 2023 1:13 AM > To: Bhadane, Dnyaneshwar <dnyaneshwar.bhadane@xxxxxxxxx> > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; De Marchi, Lucas > <lucas.demarchi@xxxxxxxxx>; Garg, Nemesa <nemesa.garg@xxxxxxxxx>; > Atwood, Matthew S <matthew.s.atwood@xxxxxxxxx> > Subject: Re: [PATCH v2] drm/i915: Added Wa_18022495364 > > On Fri, Sep 08, 2023 at 12:39:18PM -0700, Matt Roper wrote: > > On Fri, Sep 08, 2023 at 03:11:42PM +0530, Dnyaneshwar Bhadane wrote: > > > This workaround has two different implementations, one for gen 12 to > > > DG2 and another for DG2 and later. > > > BSpec: 11354, 56551 > > > > Side note: it's generally not worth adding bspec references for > > simple register pages these days. Anyone who has internal bspec > > access can look up the page just as easily using the register offset > > or name as they can using this number, so this doesn't help anyone. > > And in this case it looks like the page numbers you gave are wrong for > > the platforms this workaround is supposed to apply to: 11354 is for > > the pre-gen12 version of the register and 56551 is for the Xe2 version > > of the instruction. > > > > Bspec references are still good when there are narrative pages > > describing general software flows, because those can often be > > difficult to locate in the large table of contents. > > > > > > > > v2: > > > - Removed extra parentheses from the condition (Lucas) > > > - Fixed spacing and new line (Lucas) > > > > > > Signed-off-by: Dnyaneshwar Bhadane > <dnyaneshwar.bhadane@xxxxxxxxx> > > > Cc: Lucas De Marchi <lucas.demarchi@xxxxxxxxx> > > > Reviewed-by: Garg, Nemesa <nemesa.garg@xxxxxxxxx> > > One more minor thing: Always use "First Last" instead of "Last, First" > notation for names in r-b, s-o-b, etc.; otherwise the commas get > misinterpreted when git parses these for email and it causes problems when > sending/replying on the mailing list. > > > Matt > > > > --- > > > drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 5 +++++ > > > drivers/gpu/drm/i915/gt/intel_gt_regs.h | 2 ++ > > > drivers/gpu/drm/i915/gt/intel_workarounds.c | 4 ++++ > > > 3 files changed, 11 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c > > > b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c > > > index 0143445dba83..e260defdfc23 100644 > > > --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c > > > +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c > > > @@ -271,6 +271,11 @@ int gen12_emit_flush_rcs(struct i915_request > *rq, u32 mode) > > > if (GRAPHICS_VER_FULL(rq->i915) >= IP_VER(12, 70)) > > > bit_group_0 |= PIPE_CONTROL_CCS_FLUSH; > > > > > > + /* Wa_18022495364 */ > > > + if (GRAPHICS_VER_FULL(rq->i915) >= IP_VER(12, 70) || > > > + IS_DG2(rq->i915)) > > > > This is going to apply the workaround not only to DG2, but also to > > *all* platforms MTL and later forever. Generally we should never have > > workarounds marked this way...the expectation is that any hardware > > workaround is going to go away eventually, and workaround conditions > > in the code should only match the specific platforms listed as being > > applicable in the workaround database. > > > > Also, when I look in the workaround database, it doesn't appear that > > this workaround is listed as applying to DG2, MTL (Xe_LPG), or LNL > > (Xe2_LPG). Is there some other workaround that requires the same > > programming (but has a different workaround lineage #)? If not, then > > this part of the patch should just go away and only the gen12lp change > > below should remain.. Thank you for correcting me. This is not applicable to DG2 and newer platforms. Therefore, I should remove this part from the patch and only keeping the pre-Gen12 portion for this workaround. Dnyaneshwar > > > > > > Matt > > > > > + bit_group_1 |= > PIPE_CONTROL_CONST_CACHE_INVALIDATE; > > > + > > > bit_group_1 |= PIPE_CONTROL_TILE_CACHE_FLUSH; > > > bit_group_1 |= PIPE_CONTROL_FLUSH_L3; > > > bit_group_1 |= > PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH; > > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h > > > b/drivers/gpu/drm/i915/gt/intel_gt_regs.h > > > index 0e4c638fcbbf..4c0cb3a3d0aa 100644 > > > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h > > > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h > > > @@ -164,6 +164,8 @@ > > > #define GEN9_CSFE_CHICKEN1_RCS > _MMIO(0x20d4) > > > #define GEN9_PREEMPT_GPGPU_SYNC_SWITCH_DISABLE (1 << 2) > > > #define GEN11_ENABLE_32_PLANE_MODE (1 << 7) > > > +#define GEN12_CS_DEBUG_MODE2 > _MMIO(0x20d8) > > > +#define GEN12_GLOBAL_DEBUG_ENABLE BIT(6) > > > > > > #define GEN7_FF_SLICE_CS_CHICKEN1 _MMIO(0x20e0) > > > #define GEN9_FFSC_PERCTX_PREEMPT_CTRL (1 << 14) > > > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c > > > b/drivers/gpu/drm/i915/gt/intel_workarounds.c > > > index 864d41bcf6bb..efdb4bbf8083 100644 > > > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c > > > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c > > > @@ -712,6 +712,10 @@ static void gen12_ctx_workarounds_init(struct > intel_engine_cs *engine, > > > GEN9_PREEMPT_GPGPU_LEVEL_MASK, > > > GEN9_PREEMPT_GPGPU_THREAD_GROUP_LEVEL); > > > > > > + /* Wa_18022495364 */ > > > + wa_masked_en(wal, GEN12_CS_DEBUG_MODE2, > > > + GEN12_GLOBAL_DEBUG_ENABLE); > > > + > > > /* > > > * Wa_16011163337 - GS_TIMER > > > * > > > -- > > > 2.34.1 > > > > > > > -- > > Matt Roper > > Graphics Software Engineer > > Linux GPU Platform Enablement > > Intel Corporation > > -- > Matt Roper > Graphics Software Engineer > Linux GPU Platform Enablement > Intel Corporation
