[Intel-gfx] [PATCH] drm/i915: Add Wa_14015150844

[Matt Roper]

On Wed, Aug 30, 2023 at 09:30:01PM +0530, Shekhar Chauhan wrote:
> Disables Atomic-chaining of Typed Writes.
> 
> BSpec: 54040
> Signed-off-by: Shekhar Chauhan <shekhar.chauhan at intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_gt_regs.h     |  2 ++
>  drivers/gpu/drm/i915/gt/intel_workarounds.c | 10 ++++++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index 0e4c638fcbbf..82b533aa0c03 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -1218,6 +1218,8 @@
>  
>  #define XEHP_HDC_CHICKEN0			MCR_REG(0xe5f0)
>  #define   LSC_L1_FLUSH_CTL_3D_DATAPORT_FLUSH_EVENTS_MASK	REG_GENMASK(13, 11)
> +#define   ATOMIC_CHAINING_TYPED_WRITES		REG_BIT(3)
> +
>  #define ICL_HDC_MODE				MCR_REG(0xe5f4)
>  
>  #define EU_PERF_CNTL2				PERF_REG(0xe658)
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 864d41bcf6bb..d54120009334 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -2327,6 +2327,16 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
>  				  LSC_L1_FLUSH_CTL_3D_DATAPORT_FLUSH_EVENTS_MASK);
>  	}
>  
> +	if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_C0, STEP_FOREVER) ||
> +	    IS_DG2_GRAPHICS_STEP(i915, G11, STEP_B0, STEP_FOREVER) ||
> +	    IS_DG2_G12(i915) ||

Since DG2 is at the point where we only support production steppings, we
don't need to worry about avoiding this workaround on old pre-production
steppings.  So these three conditions can be simplified down to just
"IS_DG2(i915)."

See
https://lore.kernel.org/intel-gfx/20230816214201.534095-7-matthew.d.roper@intel.com/
for details.

> +	    IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_FOREVER) ||
> +	    IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_FOREVER)) {

This macro doesn't exist anymore; I think you need to rebase your patch
on a newer snapshot of drm-tip.  BTW, "A0..forever" covers every single
stepping, so there's no need to use a stepping-based check in such
cases.  Once you rebase, this will just turn into

    IS_GFX_GT_IP_RANGE(engine->gt, IP_VER(12, 70), IP_VER(12, 71))

Also note that we generally put the newer platform/IP first in our
conditions, so the overall check will become:

    if (IS_GFX_GT_IP_RANGE(gt, IP_VER(12, 70), IP_VER(12, 71)) ||
        IS_DG2(i915))


> +		/* Wa_14015150844 */
> +		wa_mcr_masked_dis(wal, XEHP_HDC_CHICKEN0,
> +				  ATOMIC_CHAINING_TYPED_WRITES);

It's easy to miss, but up near the top of bspec 54040, there's a note
about this register being a "write only" register.  That means that if
we implement this workaround the normal way we'll get driver warnings on
debug builds when it tries to read back the register later and make sure
the workaround was applied properly.  We need to use the more explicit
wa_mcr_add() function to provide an empty readmask to avoid this.  The
next workaround just below this in the code is an example of how to do
that.


Matt

> +	}
> +
>  	if (IS_DG2_G11(i915) || IS_DG2_G10(i915)) {
>  		/* Wa_22014600077:dg2 */
>  		wa_mcr_add(wal, GEN10_CACHE_MODE_SS, 0,
> -- 
> 2.34.1
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation