[Intel-gfx] [PATCH v2 01/14] drm/i915/tgl: Split GT and display workarounds

Jani Nikula jani.nikula at linux.intel.com
Thu Feb 27 06:23:47 UTC 2020


On Tue, 25 Feb 2020, José Roberto de Souza <jose.souza at intel.com> wrote:
> Spliting GT and display revisions id to correctly apply workarounds
> because we have some issues that were fixed in display B0 but no
> hardware was made with B0 stepping, so to keep consistent with BSpec
> splitting it from GT and adding this adittional handling.
>
> BSpec: 52890
> BSpec: 44455
> Cc: Matt Roper <matthew.d.roper at intel.com>
> Cc: James Ausmus <james.ausmus at intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> ---
>  .../drm/i915/display/intel_display_power.c    |  2 +-
>  drivers/gpu/drm/i915/gt/intel_lrc.c           |  2 +-
>  drivers/gpu/drm/i915/gt/intel_workarounds.c   | 10 +++---
>  drivers/gpu/drm/i915/i915_drv.h               | 36 +++++++++++++++++--
>  drivers/gpu/drm/i915/intel_pm.c               |  2 +-
>  5 files changed, 42 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 6e25a1317161..82af963106ab 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -5010,7 +5010,7 @@ static void tgl_bw_buddy_init(struct drm_i915_private *dev_priv)
>  	const struct buddy_page_mask *table;
>  	int i;
>  
> -	if (IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_A0))
> +	if (IS_TGL_DISP_REVID(dev_priv, TGL_DISP_REVID_A0, TGL_DISP_REVID_A0))
>  		/* Wa_1409767108: tgl */
>  		table = wa_1409767108_buddy_page_masks;
>  	else
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 39b0125b7143..852981d533a8 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -4124,7 +4124,7 @@ static int gen12_emit_flush_render(struct i915_request *request,
>  		/*
>  		 * Wa_1604544889:tgl
>  		 */
> -		if (IS_TGL_REVID(request->i915, TGL_REVID_A0, TGL_REVID_A0)) {
> +		if (IS_TGL_GT_REVID(request->i915, TGL_GT_REVID_A0, TGL_GT_REVID_A0)) {
>  			flags = 0;
>  			flags |= PIPE_CONTROL_CS_STALL;
>  			flags |= PIPE_CONTROL_HDC_PIPELINE_FLUSH;
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 887e0dc701f7..bc6114b6dc8f 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -596,8 +596,8 @@ static void tgl_ctx_workarounds_init(struct intel_engine_cs *engine,
>  	 * the read of FF_MODE2.
>  	 */
>  	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);
> +	       IS_TGL_GT_REVID(engine->i915, TGL_GT_REVID_A0,
> +			       TGL_GT_REVID_A0) ? 0 : FF_MODE2_TDS_TIMER_MASK);
>  }
>  
>  static void
> @@ -931,13 +931,13 @@ static void
>  tgl_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal)
>  {
>  	/* Wa_1409420604:tgl */
> -	if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
> +	if (IS_TGL_GT_REVID(i915, TGL_GT_REVID_A0, TGL_GT_REVID_A0))
>  		wa_write_or(wal,
>  			    SUBSLICE_UNIT_LEVEL_CLKGATE2,
>  			    CPSSUNIT_CLKGATE_DIS);
>  
>  	/* Wa_1409180338:tgl */
> -	if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
> +	if (IS_TGL_GT_REVID(i915, TGL_GT_REVID_A0, TGL_GT_REVID_A0))
>  		wa_write_or(wal,
>  			    SLICE_UNIT_LEVEL_CLKGATE,
>  			    L3_CLKGATE_DIS | L3_CR2X_CLKGATE_DIS);
> @@ -1329,7 +1329,7 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
>  {
>  	struct drm_i915_private *i915 = engine->i915;
>  
> -	if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0)) {
> +	if (IS_TGL_GT_REVID(i915, TGL_GT_REVID_A0, TGL_GT_REVID_A0)) {
>  		/* Wa_1606700617:tgl */
>  		wa_masked_en(wal,
>  			     GEN9_CS_DEBUG_MODE1,
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index ea13fc0b409b..4fa01f8d3f33 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1574,11 +1574,43 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
>  #define IS_ICL_REVID(p, since, until) \
>  	(IS_ICELAKE(p) && IS_REVID(p, since, until))
>  
> -#define TGL_REVID_A0		0x0
> +#define TGL_GT_REVID_A0		0x0
>  
> -#define IS_TGL_REVID(p, since, until) \
> +#define IS_TGL_GT_REVID(p, since, until) \
>  	(IS_TIGERLAKE(p) && IS_REVID(p, since, until))
>  
> +/*
> + * revid=0x0 = DISP_REVID_A0
> + * revid=0x1 = DISP_REVID_C0
> + * revid=0x2 = DISP_REVID_D0
> + *
> + * So ids bellow will not match PCI revid and the function bellow is used.
> + */
> +#define TGL_DISP_REVID_A0 0x0
> +#define TGL_DISP_REVID_B0 0x1
> +#define TGL_DISP_REVID_C0 0x2
> +#define TGL_DISP_REVID_D0 0x3
> +
> +static inline bool
> +_tgl_disp_revid(struct drm_i915_private *p, u8 since, u8 until)
> +{
> +	const u8 gt2_disp_revid[] = {
> +		TGL_DISP_REVID_A0,
> +		TGL_DISP_REVID_C0,
> +		TGL_DISP_REVID_D0
> +	};
> +	u8 disp_revid;
> +
> +	if (INTEL_REVID(p) >= ARRAY_SIZE(gt2_disp_revid))
> +		disp_revid = TGL_DISP_REVID_D0;
> +	else
> +		disp_revid = gt2_disp_revid[INTEL_REVID(p)];
> +
> +	return IS_TIGERLAKE(p) && disp_revid >= since && disp_revid <= until;
> +}
> +
> +#define IS_TGL_DISP_REVID(p, since, until) _tgl_disp_revid(p, since, until)
> +

IOW, you could just define display steppings in terms of the revids, and
would not have to introduce two separate revid test macros?

#define TGL_DISPLAY_REVID_A0	TGL_REVID_A0
#define TGL_DISPLAY_REVID_B0	TGL_REVID_A0
#define TGL_DISPLAY_REVID_C0	TGL_REVID_B0
#define TGL_DISPLAY_REVID_D0	TGL_REVID_C0

And then you could simply use:

	if (IS_TGL_REVID(i915, TGL_DISPLAY_REVID_A0, TGL_DISPLAY_REVID_B0))

At this point, I don't think there's any reason at all to add the extra
indirection and function, and two separate revid check macros. Because
ultimately it all depends on a single revid, not two independent revids
for GT and display.

BR,
Jani.


>  #define IS_LP(dev_priv)	(INTEL_INFO(dev_priv)->is_lp)
>  #define IS_GEN9_LP(dev_priv)	(IS_GEN(dev_priv, 9) && IS_LP(dev_priv))
>  #define IS_GEN9_BC(dev_priv)	(IS_GEN(dev_priv, 9) && !IS_LP(dev_priv))
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 22aa205793e5..49484d5f5f84 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6852,7 +6852,7 @@ static void tgl_init_clock_gating(struct drm_i915_private *dev_priv)
>  		   I915_READ(POWERGATE_ENABLE) | vd_pg_enable);
>  
>  	/* Wa_1409825376:tgl (pre-prod)*/
> -	if (IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_A0))
> +	if (IS_TGL_DISP_REVID(dev_priv, TGL_DISP_REVID_A0, TGL_DISP_REVID_A0))
>  		I915_WRITE(GEN9_CLKGATE_DIS_3, I915_READ(GEN9_CLKGATE_DIS_3) |
>  			   TGL_VRH_GATING_DIS);
>  }

-- 
Jani Nikula, Intel Open Source Graphics Center


More information about the Intel-gfx mailing list