Re: [Intel-gfx] [PATCH] drm/i915/dg2: add Wa_14015023722

[Matt Roper <matthew.d.roper@intel.com>]

On Fri, Jan 14, 2022 at 11:41:31AM -0800, clinton.a.taylor@intel.com wrote:
> From: Clint Taylor <clinton.a.taylor@intel.com>
> 
> BSPEC: 46123
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_workarounds.c | 5 +++++
>  drivers/gpu/drm/i915/i915_reg.h             | 4 ++++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 6a4372c3a3c5..5cdacfa8aefc 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -688,6 +688,11 @@ static void dg2_ctx_workarounds_init(struct intel_engine_cs *engine,
>  	    IS_DG2_GRAPHICS_STEP(engine->i915, G11, STEP_A0, STEP_B0))
>  		wa_masked_en(wal, GEN9_HALF_SLICE_CHICKEN7,
>  			     DG2_DISABLE_ROUND_ENABLE_ALLOW_FOR_SSLA);
> +
> +	/* wa_14015023722: DG2 G11 [B0..NONE] */

We generally want to refer to workarounds by their lineage number rather
than by their per-platform bug IDs.  In this case the lineage number of
the workaround is Wa_14014947963.

> +	if (IS_DG2_GRAPHICS_STEP(engine->i915, G11, STEP_B0, STEP_FOREVER))

It looks like this workaround applies to DG2-G10 for B0 and beyond, so
the condition above should say "G10" rather than G11.  It also applies
to G11 and G12 for _all_ steppings.  I haven't sent the patch that adds
the G12 subplatform yet, but I'll do that soon.  In the meantime, you'll
want an "|| IS_DG2_G11(engine->i915)" added to the condition since it
applies to all steppings of that subplatform.

> +		wa_masked_en(wal, VF_PREEMPTION, PREEMPTION_VERTEX_4000);

wa_masked_en() will turn on any bits in PREEMPTION_VERTEX_4000 (i.e.,
bit 14 in this case), but won't clear any of the other bits that might
already be set in hardware.  I think you want to use

    wa_masked_field_set(wal, VF_PREEMPTION, PREEMPTION_VERTEX_COUNT, 0x4000)

so that the bits 15:0 become exactly 0x4000 rather than just having bit
14 turned on.  I also think it's fine to just pass a literal 0x4000 for
the final parameter rather than PREEMPTION_VERTEX_4000 in this case
since it is just a count and not representing a special enum value or
anything.


Matt

> +
>  }
>  
>  static void fakewa_disable_nestedbb_mode(struct intel_engine_cs *engine,
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index ef6bc8180073..5805a45920b3 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -11934,4 +11934,8 @@ enum skl_power_gate {
>  #define SLICE_COMMON_ECO_CHICKEN1		_MMIO(0x731C)
>  #define   MSC_MSAA_REODER_BUF_BYPASS_DISABLE	REG_BIT(14)
>  
> +#define VF_PREEMPTION			_MMIO(0x83a4)
> +#define  PREEMPTION_VERTEX_COUNT	REG_GENMASK(15, 0)
> +#define  PREEMPTION_VERTEX_4000		REG_FIELD_PREP(PREEMPTION_VERTEX_COUNT, 0x4000)
> +
>  #endif /* _I915_REG_H_ */
> -- 
> 2.34.1
> 

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