Re: [Intel-gfx] [PATCH] drm/i915/tgl: Fix REVID macros for TGL to fetch correct stepping

[Jani Nikula <jani.nikula@intel.com>]

On Tue, 24 Nov 2020, Aditya Swarup <aditya.swarup@intel.com> wrote:
> Fix TGL REVID macros to fetch correct display/gt stepping based
> on SOC rev id from INTEL_REVID() macro. Previously, we were just
> returning the first element of the revid array instead of using
> the correct index based on SOC rev id.
>
> Also, add array bound checks for TGL REV ID array. Since, there
> might be a possibility of using older kernels on latest platform
> revision, resulting in out of bounds access for rev ID array.
> In this scenario, print message for unsupported rev ID and apply
> settings for latest rev ID available.
>
> Fixes: ("drm/i915/tgl: Fix stepping WA matching")
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Aditya Swarup <aditya.swarup@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h | 35 +++++++++++++++++++++++++++------
>  1 file changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 15be8debae54..29d55b7017be 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1572,16 +1572,37 @@ enum {
>  	TGL_REVID_D0,
>  };
>  
> -extern const struct i915_rev_steppings tgl_uy_revids[];
> -extern const struct i915_rev_steppings tgl_revids[];
> +extern const struct i915_rev_steppings tgl_uy_revids[4];
> +extern const struct i915_rev_steppings tgl_revids[2];

Just a quick note, the compiler does not check that the size in the
extern declaration matches the size in the array definition. So you
might end up with a mismatch without noticing.

BR,
Jani.

> +
> +#define TGL_UY_REVID_RANGE(revid) \
> +	((revid) < ARRAY_SIZE(tgl_uy_revids))
> +
> +#define TGL_REVID_RANGE(revid) \
> +	((revid) < ARRAY_SIZE(tgl_revids))
>  
>  static inline const struct i915_rev_steppings *
>  tgl_revids_get(struct drm_i915_private *dev_priv)
>  {
> -	if (IS_TGL_U(dev_priv) || IS_TGL_Y(dev_priv))
> -		return tgl_uy_revids;
> -	else
> -		return tgl_revids;
> +	const u8 revid = INTEL_REVID(dev_priv);
> +
> +	if (IS_TGL_U(dev_priv) || IS_TGL_Y(dev_priv)) {
> +		if (TGL_UY_REVID_RANGE(revid)) {
> +			return tgl_uy_revids + revid;
> +		} else {
> +			drm_dbg_kms(&dev_priv->drm,
> +				    "Unsupported SOC stepping found %u, using %lu instead\n",
> +				    revid, ARRAY_SIZE(tgl_uy_revids) - 1);
> +			return tgl_uy_revids + (ARRAY_SIZE(tgl_uy_revids) - 1);
> +		}
> +	} else if (TGL_REVID_RANGE(revid)) {
> +		return tgl_revids + revid;
> +	} else	{
> +		drm_dbg_kms(&dev_priv->drm,
> +			    "Unsupported SOC stepping found %u, using %lu instead\n",
> +			    revid, ARRAY_SIZE(tgl_revids) - 1);
> +		return tgl_uy_revids + (ARRAY_SIZE(tgl_revids) - 1);
> +	}
>  }
>  
>  #define IS_TGL_DISP_REVID(p, since, until) \
> @@ -1591,12 +1612,14 @@ tgl_revids_get(struct drm_i915_private *dev_priv)
>  
>  #define IS_TGL_UY_GT_REVID(p, since, until) \
>  	((IS_TGL_U(p) || IS_TGL_Y(p)) && \
> +	 TGL_UY_REVID_RANGE(INTEL_REVID(p)) && \
>  	 tgl_uy_revids->gt_stepping >= (since) && \
>  	 tgl_uy_revids->gt_stepping <= (until))
>  
>  #define IS_TGL_GT_REVID(p, since, until) \
>  	(IS_TIGERLAKE(p) && \
>  	 !(IS_TGL_U(p) || IS_TGL_Y(p)) && \
> +	 TGL_REVID_RANGE(INTEL_REVID(p)) && \
>  	 tgl_revids->gt_stepping >= (since) && \
>  	 tgl_revids->gt_stepping <= (until))

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx