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.. > > > 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
