Re: [PATCH 01/22] drm/i915/icl: Introduce initial Icelake Workarounds

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Oscar Mateo <oscar.mateo@xxxxxxxxx> writes:

> Inherit workarounds from previous platforms that are still valid for
> Icelake.
>
> v2: GEN7_ROW_CHICKEN2 is masked
> v3:
>   - Since it has been fixed already in upstream, removed the TODO
>     comment about WA_SET_BIT for WaInPlaceDecompressionHang.
>   - Squashed with this patch:
>       drm/i915/icl: add icelake_init_clock_gating()
>     from Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
>   - Squashed with this patch:
>       drm/i915/icl: WaForceEnableNonCoherent
>     from Oscar Mateo <oscar.mateo@xxxxxxxxx>
>   - WaPushConstantDereferenceHoldDisable is now Wa_1604370585 and
>     applies to B0 as well.
>   - WaPipeControlBefore3DStateSamplePattern WABB was being applied
>     to ICL incorrectly.
> v4:
>   - Wrap the commit message
>   - s/dev_priv/p to please checkpatch
> v5: Rebased on top of the WA refactoring
> v6: Rebased on top of further whitelist registers refactoring (Michel)
> v7: Added WaRsForcewakeAddDelayForAck
> v8: s/ICL_HDC_CHICKEN0/ICL_HDC_MODE (Mika)
> v9:
>   - C, not lisp (Chris)
>   - WaIncreaseDefaultTLBEntries is the same for GEN > 9_LP (Tvrtko)
>
> Signed-off-by: Oscar Mateo <oscar.mateo@xxxxxxxxx>
> Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> Cc: Tomasz Lis <tomasz.lis@xxxxxxxxx>
> Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/i915_drv.h          |  9 +++++++
>  drivers/gpu/drm/i915/i915_gem_gtt.c      |  6 ++---
>  drivers/gpu/drm/i915/i915_reg.h          |  1 +
>  drivers/gpu/drm/i915/intel_lrc.c         |  2 ++
>  drivers/gpu/drm/i915/intel_pm.c          |  4 ++-
>  drivers/gpu/drm/i915/intel_uncore.c      |  7 +++--
>  drivers/gpu/drm/i915/intel_workarounds.c | 46 ++++++++++++++++++++++++++++++++
>  7 files changed, 69 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 04e2780..ad79d5a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2469,6 +2469,15 @@ static inline unsigned int i915_sg_segment_size(void)
>  #define IS_CNL_REVID(p, since, until) \
>  	(IS_CANNONLAKE(p) && IS_REVID(p, since, until))
>  
> +#define ICL_REVID_A0		0x0
> +#define ICL_REVID_A2		0x1
> +#define ICL_REVID_B0		0x3
> +#define ICL_REVID_B2		0x4
> +#define ICL_REVID_C0		0x5
> +
> +#define IS_ICL_REVID(p, since, until) \
> +	(IS_ICELAKE(p) && IS_REVID(p, since, until))
> +
>  /*
>   * The genX designation typically refers to the render engine, so render
>   * capability related checks should use IS_GEN, while display and other checks
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index c879bfd..ea30e84 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2137,15 +2137,15 @@ static void gtt_write_workarounds(struct drm_i915_private *dev_priv)
>  	 * called on driver load and after a GPU reset, so you can place
>  	 * workarounds here even if they get overwritten by GPU reset.
>  	 */
> -	/* WaIncreaseDefaultTLBEntries:chv,bdw,skl,bxt,kbl,glk,cfl,cnl */
> +	/* WaIncreaseDefaultTLBEntries:chv,bdw,skl,bxt,kbl,glk,cfl,cnl,icl */
>  	if (IS_BROADWELL(dev_priv))
>  		I915_WRITE(GEN8_L3_LRA_1_GPGPU, GEN8_L3_LRA_1_GPGPU_DEFAULT_VALUE_BDW);
>  	else if (IS_CHERRYVIEW(dev_priv))
>  		I915_WRITE(GEN8_L3_LRA_1_GPGPU, GEN8_L3_LRA_1_GPGPU_DEFAULT_VALUE_CHV);
> -	else if (IS_GEN9_BC(dev_priv) || IS_GEN10(dev_priv))
> -		I915_WRITE(GEN8_L3_LRA_1_GPGPU, GEN9_L3_LRA_1_GPGPU_DEFAULT_VALUE_SKL);
>  	else if (IS_GEN9_LP(dev_priv))
>  		I915_WRITE(GEN8_L3_LRA_1_GPGPU, GEN9_L3_LRA_1_GPGPU_DEFAULT_VALUE_BXT);
> +	else if (INTEL_GEN(dev_priv) >= 9)
> +		I915_WRITE(GEN8_L3_LRA_1_GPGPU, GEN9_L3_LRA_1_GPGPU_DEFAULT_VALUE_SKL);
>  
>  	/*
>  	 * To support 64K PTEs we need to first enable the use of the
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 085928c..2b22d4d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7238,6 +7238,7 @@ enum {
>  /* GEN8 chicken */
>  #define HDC_CHICKEN0				_MMIO(0x7300)
>  #define CNL_HDC_CHICKEN0			_MMIO(0xE5F0)
> +#define ICL_HDC_MODE				_MMIO(0xE5F4)
>  #define  HDC_FORCE_CSR_NON_COHERENT_OVR_DISABLE	(1<<15)
>  #define  HDC_FENCE_DEST_SLM_DISABLE		(1<<14)
>  #define  HDC_DONOT_FETCH_MEM_WHEN_MASKED	(1<<11)
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 911f288..920752a 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1665,6 +1665,8 @@ static int intel_init_workaround_bb(struct intel_engine_cs *engine)
>  		return -EINVAL;
>  
>  	switch (INTEL_GEN(engine->i915)) {
> +	case 11:
> +		return 0;
>  	case 10:
>  		wa_bb_fn[0] = gen10_init_indirectctx_bb;
>  		wa_bb_fn[1] = NULL;
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 4126132..9c6e48c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -9190,7 +9190,9 @@ static void nop_init_clock_gating(struct drm_i915_private *dev_priv)
>   */
>  void intel_init_clock_gating_hooks(struct drm_i915_private *dev_priv)
>  {
> -	if (IS_CANNONLAKE(dev_priv))
> +	if (IS_ICELAKE(dev_priv))
> +		dev_priv->display.init_clock_gating = nop_init_clock_gating;
> +	else if (IS_CANNONLAKE(dev_priv))
>  		dev_priv->display.init_clock_gating = cnl_init_clock_gating;
>  	else if (IS_COFFEELAKE(dev_priv))
>  		dev_priv->display.init_clock_gating = cfl_init_clock_gating;
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index d6e20f0..448293e 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -139,7 +139,9 @@ enum ack_type {
>  	 * in the hope that the original ack will be delivered along with
>  	 * the fallback ack.
>  	 *
> -	 * This workaround is described in HSDES #1604254524
> +	 * This workaround is described in HSDES #1604254524 and it's known as:
> +	 * WaRsForcewakeAddDelayForAck:skl,bxt,kbl,glk,cfl,cnl,icl
> +	 * although the name is a bit misleading.
>  	 */
>  
>  	pass = 1;
> @@ -1394,7 +1396,8 @@ static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
>  	if (INTEL_GEN(dev_priv) >= 11) {
>  		int i;
>  
> -		dev_priv->uncore.funcs.force_wake_get = fw_domains_get;
> +		dev_priv->uncore.funcs.force_wake_get =
> +			fw_domains_get_with_fallback;
>  		dev_priv->uncore.funcs.force_wake_put = fw_domains_put;
>  		fw_domain_init(dev_priv, FW_DOMAIN_ID_RENDER,
>  			       FORCEWAKE_RENDER_GEN9,
> diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
> index ec9d340..73d02d3 100644
> --- a/drivers/gpu/drm/i915/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/intel_workarounds.c
> @@ -441,6 +441,27 @@ static int cnl_ctx_workarounds_init(struct drm_i915_private *dev_priv)
>  	return 0;
>  }
>  
> +static int icl_ctx_workarounds_init(struct drm_i915_private *dev_priv)
> +{
> +	/* Wa_1604370585:icl (pre-prod)
> +	 * Formerly known as WaPushConstantDereferenceHoldDisable
> +	 */
> +	if (IS_ICL_REVID(dev_priv, ICL_REVID_A0, ICL_REVID_B0))
> +		WA_SET_BIT_MASKED(GEN7_ROW_CHICKEN2,
> +				  PUSH_CONSTANT_DEREF_DISABLE);
> +
> +	/* WaForceEnableNonCoherent:icl
> +	 * This is not the same workaround as in early Gen9 platforms, where
> +	 * lacking this could cause system hangs, but coherency performance
> +	 * overhead is high and only a few compute workloads really need it
> +	 * (the register is whitelisted in hardware now, so UMDs can opt in
> +	 * for coherency if they have a good reason).
> +	 */
> +	WA_SET_BIT_MASKED(ICL_HDC_MODE, HDC_FORCE_NON_COHERENT);
> +
> +	return 0;
> +}
> +
>  int intel_ctx_workarounds_init(struct drm_i915_private *dev_priv)
>  {
>  	int err = 0;
> @@ -465,6 +486,8 @@ int intel_ctx_workarounds_init(struct drm_i915_private *dev_priv)
>  		err = cfl_ctx_workarounds_init(dev_priv);
>  	else if (IS_CANNONLAKE(dev_priv))
>  		err = cnl_ctx_workarounds_init(dev_priv);
> +	else if (IS_ICELAKE(dev_priv))
> +		err = icl_ctx_workarounds_init(dev_priv);
>  	else
>  		MISSING_CASE(INTEL_GEN(dev_priv));
>  	if (err)
> @@ -663,6 +686,21 @@ static void cnl_gt_workarounds_apply(struct drm_i915_private *dev_priv)
>  		   _MASKED_BIT_ENABLE(GEN9_FFSC_PERCTX_PREEMPT_CTRL));
>  }
>  
> +static void icl_gt_workarounds_apply(struct drm_i915_private *dev_priv)
> +{
> +	/* This is not an Wa. Enable for better image quality */
> +	I915_WRITE(_3D_CHICKEN3,
> +		   _MASKED_BIT_ENABLE(_3D_CHICKEN3_AA_LINE_QUALITY_FIX_ENABLE));
> +
> +	/* WaInPlaceDecompressionHang:icl */
> +	I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA, I915_READ(GEN9_GAMT_ECO_REG_RW_IA) |
> +					    GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
> +
> +	/* WaPipelineFlushCoherentLines:icl */
> +	I915_WRITE(GEN8_L3SQCREG4, I915_READ(GEN8_L3SQCREG4) |
> +				   GEN8_LQSC_FLUSH_COHERENT_LINES);

Ok, for icl, this is needed. And like you said, it is different
than what the WaFlushCoherentL3CacheLinesAtContextSwitch does.

There is more to this saga, as WaPipelineFlushCoherentLines
is needed also for other platforms, and we don't have it.
And it will collide with how we do per bb workarounds around this reg.

A thing we much recheck also on icl, if we add indirect context bb's.

Reviewed-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>

> +}
> +
>  void intel_gt_workarounds_apply(struct drm_i915_private *dev_priv)
>  {
>  	if (INTEL_GEN(dev_priv) < 8)
> @@ -683,6 +721,8 @@ void intel_gt_workarounds_apply(struct drm_i915_private *dev_priv)
>  		cfl_gt_workarounds_apply(dev_priv);
>  	else if (IS_CANNONLAKE(dev_priv))
>  		cnl_gt_workarounds_apply(dev_priv);
> +	else if (IS_ICELAKE(dev_priv))
> +		icl_gt_workarounds_apply(dev_priv);
>  	else
>  		MISSING_CASE(INTEL_GEN(dev_priv));
>  }
> @@ -761,6 +801,10 @@ static void cnl_whitelist_build(struct whitelist *w)
>  	whitelist_reg(w, GEN8_CS_CHICKEN1);
>  }
>  
> +static void icl_whitelist_build(struct whitelist *w)
> +{
> +}
> +
>  static struct whitelist *whitelist_build(struct intel_engine_cs *engine,
>  					 struct whitelist *w)
>  {
> @@ -789,6 +833,8 @@ static struct whitelist *whitelist_build(struct intel_engine_cs *engine,
>  		cfl_whitelist_build(w);
>  	else if (IS_CANNONLAKE(i915))
>  		cnl_whitelist_build(w);
> +	else if (IS_ICELAKE(i915))
> +		icl_whitelist_build(w);
>  	else
>  		MISSING_CASE(INTEL_GEN(i915));
>  
> -- 
> 1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux