[Intel-gfx] [v2] drm/i915/gen11: Moving WAs to icl_gt_workarounds_init()

[Matt Roper]

On Thu, Dec 09, 2021 at 09:21:39AM -0800, Lucas De Marchi wrote:
> On Fri, Dec 03, 2021 at 08:26:03PM +0530, ravitejax.goud.talla at intel.com wrote:
> > From: Raviteja Goud Talla <ravitejax.goud.talla at intel.com>
> > 
> > Bspec page says "Reset: BUS", Accordingly moving w/a's:
> > Wa_1407352427,Wa_1406680159 to proper function icl_gt_workarounds_init()
> > Which will resolve guc enabling error
> > 
> > v2:
> >  - Previous patch rev2 was created by email client which caused the
> >    Build failure, This v2 is to resolve the previous broken series
> > 
> > Reviewed-by: John Harrison <John.C.Harrison at Intel.com>
> > Signed-off-by: Raviteja Goud Talla <ravitejax.goud.talla at intel.com>
> 
> It seems like this patch is needed to be able to load GuC in ICL:
> https://gitlab.freedesktop.org/drm/intel/-/issues/4067#note_1184951
> 
> And that is failing on Linus' master branch as of
> 2a987e65025e ("Merge tag 'perf-tools-fixes-for-v5.16-2021-12-07' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux")
> and (at least) 5.15.*. Looking at the appropriate "Fixes" tag I found these commits:
> 
>   1) 1cd21a7c5679 ("drm/i915: Add Wa_1407352427:icl,ehl") - original
>      commit adding the WA
>   2) 3551ff928744 ("drm/i915/gen11: Moving WAs to rcs_engine_wa_init()")
>      moving the WA to rcs_engine_wa_init()
> 
> However (2) is on v5.7-rc1 and (1) is on v5.6-rc1 and according to the
> bug report GuC loading was working on 5.13. So I suspect the move
> to GuC 62.0.0 made the checks more strict, or there is another patch

This is correct.  Having "GT workarounds" on the engine list by accident
used to be harmless (because i915 [with execlist submission] and the guc
[with guc submission]) would simply re-apply the register setting more
often than it really needed to.  However the more recent GuC versions
have become more picky about the set of registers they're willing to
save/restore for workarounds and will fail to load if they see a
register on the save/restore list that isn't something they think is
appropriate for an engine reset.

GuC submission isn't officially supported on ICL; you can force it via
module parameter, which taints the kernel and takes you through untested
codepaths, so you do so at your own risk.  Given that, I don't think
there's any real need to worry about getting this backported to specific
stable kernels; having the workaround in the wrong place doesn't
actually harm anything on the official and default non-GuC mode.


Matt

> that paired with that one makes load fail to load.
> 
> Anyway, it seems that commit is the one to fix and we probably need to
> bring both moves to stable (5.15.y and 5.10.y ?), besides propagating
> it to drm-intel-fixes so it applies to 5.16
> 
> Cc'ing some more people.
> 
> 
> Lucas De Marchi
> 
> 
> 
> 
> > ---
> > drivers/gpu/drm/i915/gt/intel_workarounds.c | 18 +++++++++---------
> > 1 file changed, 9 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > index c3211325c2d3..3113266c286e 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > @@ -1224,6 +1224,15 @@ icl_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
> > 		    GAMT_CHKN_BIT_REG,
> > 		    GAMT_CHKN_DISABLE_L3_COH_PIPE);
> > 
> > +	/* Wa_1407352427:icl,ehl */
> > +	wa_write_or(wal, UNSLICE_UNIT_LEVEL_CLKGATE2,
> > +		    PSDUNIT_CLKGATE_DIS);
> > +
> > +	/* Wa_1406680159:icl,ehl */
> > +	wa_write_or(wal,
> > +		    SUBSLICE_UNIT_LEVEL_CLKGATE,
> > +		    GWUNIT_CLKGATE_DIS);
> > +
> > 	/* Wa_1607087056:icl,ehl,jsl */
> > 	if (IS_ICELAKE(i915) ||
> > 	    IS_JSL_EHL_GRAPHICS_STEP(i915, STEP_A0, STEP_B0))
> > @@ -2269,15 +2278,6 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
> > 		wa_write_or(wal, UNSLICE_UNIT_LEVEL_CLKGATE,
> > 			    VSUNIT_CLKGATE_DIS | HSUNIT_CLKGATE_DIS);
> > 
> > -		/* Wa_1407352427:icl,ehl */
> > -		wa_write_or(wal, UNSLICE_UNIT_LEVEL_CLKGATE2,
> > -			    PSDUNIT_CLKGATE_DIS);
> > -
> > -		/* Wa_1406680159:icl,ehl */
> > -		wa_write_or(wal,
> > -			    SUBSLICE_UNIT_LEVEL_CLKGATE,
> > -			    GWUNIT_CLKGATE_DIS);
> > -
> > 		/*
> > 		 * Wa_1408767742:icl[a2..forever],ehl[all]
> > 		 * Wa_1605460711:icl[a0..c0]
> > -- 
> > 2.34.1
> > 

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795