Re: [Intel-gfx] [PATCH v2 12/13] drm/i915/xehp: Add compute workarounds

["Ceraolo Spurio, Daniele" <daniele.ceraolospurio@intel.com>]

On 2/28/2022 9:42 AM, Matt Roper wrote:
> Additional workarounds are required once we start exposing CCS engines.
>
> Note that we have a number of workarounds that update registers in the
> shared render/compute reset domain.  Historically we've just added such
> registers to the RCS engine's workaround list.  But going forward we
> should be more careful to place such workarounds on a wa_list for an
> engine that definitely exists and is not fused off (e.g., a platform
> with no RCS would never apply the RCS wa_list).  We'll keep
> rcs_engine_wa_init() focused on RCS-specific workarounds that only need
> to be applied if the RCS engine is present.  A separate
> general_render_compute_wa_init() function will be used to define
> workarounds that touch registers in the shared render/compute reset
> domain and that we need to apply regardless of what render and/or
> compute engines actually exist.  Any workarounds defined in this new
> function will internally be added to the first present RCS or CCS
> engine's workaround list to ensure they get applied (and only get
> applied once rather than being needlessly re-applied several times).
>
> Co-author: Srinivasan Shanmugam
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_gt_regs.h     |  1 +
>   drivers/gpu/drm/i915/gt/intel_lrc.c         |  6 +++
>   drivers/gpu/drm/i915/gt/intel_workarounds.c | 47 +++++++++++++++++++++
>   3 files changed, 54 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index e629443e07ae..19cd34f24263 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -1060,6 +1060,7 @@
>   #define   FLOW_CONTROL_ENABLE			REG_BIT(15)
>   #define   UGM_BACKUP_MODE			REG_BIT(13)
>   #define   MDQ_ARBITRATION_MODE			REG_BIT(12)
> +#define   SYSTOLIC_DOP_CLOCK_GATING_DIS		REG_BIT(10)
>   #define   PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE	REG_BIT(8)
>   #define   STALL_DOP_GATING_DISABLE		REG_BIT(5)
>   #define   THROTTLE_12_5				REG_GENMASK(4, 2)
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index d333400d29fe..e9ea576b96a4 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1217,6 +1217,12 @@ gen12_emit_indirect_ctx_xcs(const struct intel_context *ce, u32 *cs)
>   	cs = gen12_emit_timestamp_wa(ce, cs);
>   	cs = gen12_emit_restore_scratch(ce, cs);
>   
> +	/* Wa_16013000631:dg2 */
> +	if (IS_DG2_GRAPHICS_STEP(ce->engine->i915, G10, STEP_B0, STEP_C0) ||
> +	    IS_DG2_G11(ce->engine->i915))
> +		if (ce->engine->class == COMPUTE_CLASS)
> +			cs = gen8_emit_pipe_control(cs, PIPE_CONTROL_INSTRUCTION_CACHE_INVALIDATE, 0);
> +
>   	return cs;
>   }
>   
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 0471d475e680..0b9435d62808 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -1921,6 +1921,11 @@ static void dg2_whitelist_build(struct intel_engine_cs *engine)
>   					  RING_FORCE_TO_NONPRIV_RANGE_4);
>   
>   		break;
> +	case COMPUTE_CLASS:
> +		/* Wa_16011157294:dg2_g10 */
> +		if (IS_DG2_GRAPHICS_STEP(engine->i915, G10, STEP_A0, STEP_B0))
> +			whitelist_reg(w, GEN9_CTX_PREEMPT_REG);
> +		break;
>   	default:
>   		break;
>   	}
> @@ -2581,6 +2586,40 @@ xcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
>   	}
>   }
>   
> +/*
> + * The workarounds in this function apply to shared registers in
> + * the general render reset domain that aren't tied to a
> + * specific engine.  Since all render+compute engines get reset
> + * together, and the contents of these registers are lost during
> + * the shared render domain reset, we'll define such workarounds
> + * here and then add them to just a single RCS or CCS engine's
> + * workaround list (whichever engine has the XXXX flag).
> + */
> +static void
> +general_render_compute_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
> +{
> +	struct drm_i915_private *i915 = engine->i915;
> +
> +	if (IS_XEHPSDV(i915)) {
> +		/* Wa_1409954639 */
> +		wa_masked_en(wal,
> +			     GEN8_ROW_CHICKEN,
> +			     SYSTOLIC_DOP_CLOCK_GATING_DIS);
> +
> +		/* Wa_1607196519 */
> +		wa_masked_en(wal,
> +			     GEN9_ROW_CHICKEN4,
> +			     GEN12_DISABLE_GRF_CLEAR);
> +
> +		/* Wa_14010670810:xehpsdv */
> +		wa_write_or(wal, XEHP_L3NODEARBCFG, XEHP_LNESPARE);

These 2 WAs are also implemented for DG2 (with different IDs). Are you 
planning to move them over to this function as a follow up?
All the WA implementations match the specs, so as long as there is a 
plan for DG2 this is:

Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

Daniele


> +
> +		/* Wa_14010449647:xehpsdv */
> +		wa_masked_en(wal, GEN7_HALF_SLICE_CHICKEN1,
> +			     GEN7_PSD_SINGLE_PORT_DISPATCH_ENABLE);
> +	}
> +}
> +
>   static void
>   engine_init_workarounds(struct intel_engine_cs *engine, struct i915_wa_list *wal)
>   {
> @@ -2589,6 +2628,14 @@ engine_init_workarounds(struct intel_engine_cs *engine, struct i915_wa_list *wal
>   
>   	engine_fake_wa_init(engine, wal);
>   
> +	/*
> +	 * These are common workarounds that just need to applied
> +	 * to a single RCS/CCS engine's workaround list since
> +	 * they're reset as part of the general render domain reset.
> +	 */
> +	if (engine->class == RENDER_CLASS)
> +		general_render_compute_wa_init(engine, wal);
> +
>   	if (engine->class == RENDER_CLASS)
>   		rcs_engine_wa_init(engine, wal);
>   	else