[Intel-gfx] [PATCH] drm/i915/gt: Move LSC_CHICKEN_BIT* workarounds to correct function

[Gustavo Sousa]

On Tue, Jan 17, 2023 at 01:06:17PM -0500, Rodrigo Vivi wrote:
> On Thu, Jan 12, 2023 at 10:19:47PM -0300, Gustavo Sousa wrote:
> > That register doesn't belong to a specific engine, so the proper
> > placement for workarounds programming it should be
> > general_render_compute_wa_init().
> 
> 
> Looking to the surrounds it seems like we have more registers
> that are not per engine specific being touched there.
> 
> So, a few questions came to my mind:
> - do we need to a bigger clean up and move other cases as well?

I think so. I just happens that I was already working on a workaround
for that register and realized the inconsistency.

> - do we need to review one by one and see if the bug is really
> for all the engines or affect one specific engine hence the
> function was initially placed here?

As far as I know, the general_render_compute_wa_init() is actually
applied for a single engine and is for registers that are not tied to a
specific engine but share the common render/compute reset domain. I'm
including Matt Roper here, in case I'm missing something.

> - do we have any clean documentation on how to split things
> around and when or where to place things here or there?

I have sent a v2 with an additional patch extending our existing
documentation to make it clear where workarounds should be implemented
according to their classification:

https://patchwork.freedesktop.org/series/113036/

--
Gustavo Sousa

> 
> > 
> > Signed-off-by: Gustavo Sousa <gustavo.sousa at intel.com>
> > ---
> >  drivers/gpu/drm/i915/gt/intel_workarounds.c | 65 ++++++++++++---------
> >  1 file changed, 36 insertions(+), 29 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > index 6dacd0dc5c2c..bd40b8c93d24 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > @@ -2325,10 +2325,6 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
> >  		/* Wa_1509727124 */
> >  		wa_mcr_masked_en(wal, GEN10_SAMPLER_MODE,
> >  				 SC_DISABLE_POWER_OPTIMIZATION_EBB);
> > -
> > -		/* Wa_22013037850 */
> > -		wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0_UDW,
> > -				DISABLE_128B_EVICTION_COMMAND_UDW);
> >  	}
> >  
> >  	if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_FOREVER) ||
> > @@ -2357,21 +2353,6 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
> >  				 GEN12_DISABLE_HDR_PAST_PAYLOAD_HOLD_FIX);
> >  	}
> >  
> > -	if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_C0) ||
> > -	    IS_DG2_G11(i915)) {
> > -		/*
> > -		 * Wa_22012826095:dg2
> > -		 * Wa_22013059131:dg2
> > -		 */
> > -		wa_mcr_write_clr_set(wal, LSC_CHICKEN_BIT_0_UDW,
> > -				     MAXREQS_PER_BANK,
> > -				     REG_FIELD_PREP(MAXREQS_PER_BANK, 2));
> > -
> > -		/* Wa_22013059131:dg2 */
> > -		wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0,
> > -				FORCE_1_SUB_MESSAGE_PER_FRAGMENT);
> > -	}
> > -
> >  	/* Wa_1308578152:dg2_g10 when first gslice is fused off */
> >  	if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_C0) &&
> >  	    needs_wa_1308578152(engine)) {
> > @@ -2396,16 +2377,6 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
> >  		 */
> >  		wa_mcr_masked_en(wal, GEN8_ROW_CHICKEN,
> >  				 MDQ_ARBITRATION_MODE | UGM_BACKUP_MODE);
> > -
> > -		/*
> > -		 * Wa_14010918519:dg2_g10
> > -		 *
> > -		 * LSC_CHICKEN_BIT_0 always reads back as 0 is this stepping,
> > -		 * so ignoring verification.
> > -		 */
> > -		wa_mcr_add(wal, LSC_CHICKEN_BIT_0_UDW, 0,
> > -			   FORCE_SLM_FENCE_SCOPE_TO_TILE | FORCE_UGM_FENCE_SCOPE_TO_TILE,
> > -			   0, false);
> >  	}
> >  
> >  	if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_A0, STEP_B0)) {
> > @@ -2990,6 +2961,15 @@ general_render_compute_wa_init(struct intel_engine_cs *engine, struct i915_wa_li
> >  
> >  	add_render_compute_tuning_settings(i915, wal);
> >  
> > +	if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) ||
> > +	    IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0) ||
> > +	    IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_FOREVER) ||
> > +	    IS_DG2_G11(i915) || IS_DG2_G12(i915)) {
> > +		/* Wa_22013037850 */
> > +		wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0_UDW,
> > +				DISABLE_128B_EVICTION_COMMAND_UDW);
> > +	}
> > +
> >  	if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) ||
> >  	    IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0) ||
> >  	    IS_PONTEVECCHIO(i915) ||
> > @@ -3011,6 +2991,33 @@ general_render_compute_wa_init(struct intel_engine_cs *engine, struct i915_wa_li
> >  		wa_masked_en(wal, VFG_PREEMPTION_CHICKEN, POLYGON_TRIFAN_LINELOOP_DISABLE);
> >  	}
> >  
> > +	if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_C0) ||
> > +	    IS_DG2_G11(i915)) {
> > +		/*
> > +		 * Wa_22012826095:dg2
> > +		 * Wa_22013059131:dg2
> > +		 */
> > +		wa_mcr_write_clr_set(wal, LSC_CHICKEN_BIT_0_UDW,
> > +				     MAXREQS_PER_BANK,
> > +				     REG_FIELD_PREP(MAXREQS_PER_BANK, 2));
> > +
> > +		/* Wa_22013059131:dg2 */
> > +		wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0,
> > +				FORCE_1_SUB_MESSAGE_PER_FRAGMENT);
> > +	}
> > +
> > +	if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_A0, STEP_B0)) {
> > +		/*
> > +		 * Wa_14010918519:dg2_g10
> > +		 *
> > +		 * LSC_CHICKEN_BIT_0 always reads back as 0 is this stepping,
> > +		 * so ignoring verification.
> > +		 */
> > +		wa_mcr_add(wal, LSC_CHICKEN_BIT_0_UDW, 0,
> > +			   FORCE_SLM_FENCE_SCOPE_TO_TILE | FORCE_UGM_FENCE_SCOPE_TO_TILE,
> > +			   0, false);
> > +	}
> > +
> >  	if (IS_PONTEVECCHIO(i915)) {
> >  		/* Wa_16016694945 */
> >  		wa_masked_en(wal, XEHPC_LNCFMISCCFGREG0, XEHPC_OVRLSCCC);
> > -- 
> > 2.39.0
> >