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?
- 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?
- do we have any clean documentation on how to split things
around and when or where to place things here or there?

> 
> Signed-off-by: Gustavo Sousa <[email protected]>
> ---
>  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
> 

Reply via email to