[Intel-gfx] [PATCH] drm/i915/tgl: add Wa_1409085225, Wa_14010229206

[Matt Roper]

On Wed, Feb 19, 2020 at 02:05:29PM -0500, Matt Atwood wrote:
> Disable Push Constant buffer addition, which can cause FIFO
> underruns.
> 
> Fix a minor white space issue while we're here.
> 
> v2: typos, add additional Wa reference
> 
> Bspec: 52890
> Cc: Rafael Antognolli <rafael.antognolli at intel.com>
> Signed-off-by: Matt Atwood <matthew.s.atwood at intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_workarounds.c | 9 +++++++++
>  drivers/gpu/drm/i915/i915_reg.h             | 3 +++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 887e0dc701f7..0681e64be741 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -580,6 +580,7 @@ static void icl_ctx_workarounds_init(struct intel_engine_cs *engine,
>  static void tgl_ctx_workarounds_init(struct intel_engine_cs *engine,
>  				     struct i915_wa_list *wal)

According to bspec page 46255, this register isn't part of the TGL
context image, so ctx_workarounds_init isn't the right place to handle
this.  The next question is whether this is a general GT workaround or a
specific engine workaround...in this case, 0xE48C falls in one of the
render engine forcewake ranges (0xE000-0xE8FF -- see bspec page 52078)
so rcs_engine_wa_init() is where we'd actually want to take care of
this.  That will ensure that the workaround gets re-applied when we
reset the render engine without doing a full GPU reset.


>  {
> +	struct drm_i915_private *i915 = engine->i915;
>  	u32 val;
>  
>  	/* Wa_1409142259:tgl */
> @@ -590,6 +591,7 @@ static void tgl_ctx_workarounds_init(struct intel_engine_cs *engine,
>  	val = intel_uncore_read(engine->uncore, FF_MODE2);
>  	val &= ~FF_MODE2_TDS_TIMER_MASK;
>  	val |= FF_MODE2_TDS_TIMER_128;
> +
>  	/*
>  	 * FIXME: FF_MODE2 register is not readable till TGL B0. We can
>  	 * enable verification of WA from the later steppings, which enables
> @@ -598,6 +600,13 @@ static void tgl_ctx_workarounds_init(struct intel_engine_cs *engine,
>  	wa_add(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK, val,
>  	       IS_TGL_REVID(engine->i915, TGL_REVID_A0, TGL_REVID_A0) ? 0 :
>  			    FF_MODE2_TDS_TIMER_MASK);
> +
> +	/*
> +	 * Wa_1409085225:tgl
> +	 * Wa_14010229206:tgl
> +	 * Push Constant Buffer can cause FIFO underruns on tgl

The description in the bspec says that it can cause an overflow rather
than an underrun.

Actually, I'm not sure whether it's worth even mentioning this here
since the things that's overflowing isn't something that our kernel
driver has any direct interaction with.  I.e., this FIFO is completely
different than the display FIFO that underruns when we have watermark
issues.


> +	 */
> +	WA_SET_BIT_MASKED(GEN9_ROW_CHICKEN4, GEN12_DISABLE_TDL_PUSH);
>  }
>  
>  static void
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index b09c1d6dc0aa..70cbe576fdfa 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -9153,6 +9153,9 @@ enum {
>  #define   PUSH_CONSTANT_DEREF_DISABLE	(1 << 8)
>  #define   GEN11_TDL_CLOCK_GATING_FIX_DISABLE	(1 << 1)
>  
> +#define GEN9_ROW_CHICKEN4		_MMIO(0xe48c)
> +#define  GEN12_DISABLE_TDL_PUSH		(1 << 9)

Even though many of the registers in this area of the file use the old
form, the preferred style for new registers is to use REG_BIT(9).  See
the comments at the top of the file for details.


Matt

> +
>  #define HSW_ROW_CHICKEN3		_MMIO(0xe49c)
>  #define  HSW_ROW_CHICKEN3_L3_GLOBAL_ATOMICS_DISABLE    (1 << 6)
>  
> -- 
> 2.21.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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