[Intel-gfx] [PATCH v2] drm/i915: Added Wa_18022495364

[Matt Roper]

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 at intel.com>
> > Cc: Lucas De Marchi <lucas.demarchi at intel.com>
> > Reviewed-by: Garg, Nemesa <nemesa.garg at intel.com>

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