Re: [Intel-gfx] [PATCH v2 4/9] drm/i915: Eliminate IS_MTL_GRAPHICS_STEP

[Andi Shyti <andi.shyti@linux.intel.com>]

Hi Matt,

> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -436,6 +436,9 @@ static inline struct intel_gt *to_gt(struct drm_i915_private *i915)
>  #define __GT_VER_FULL(gt) (__IS_MEDIA_GT(gt) ? \
>  			   MEDIA_VER_FULL((gt)->i915) : \
>  			   GRAPHICS_VER_FULL((gt)->i915))
> +#define __GT_STEP(gt) (__IS_MEDIA_GT(gt) ? \
> +		       INTEL_MEDIA_STEP((gt)->i915) : \
> +		       INTEL_GRAPHICS_STEP((gt)->i915))
>  
>  /*
>   * Check that a GT contains IP of the specified type and within the specified
> @@ -454,6 +457,29 @@ static inline struct intel_gt *to_gt(struct drm_i915_private *i915)
>  	 __GT_VER_FULL(gt) >= (from) && \
>  	 __GT_VER_FULL(gt) <= (until)))
>  
> +/*
> + * Check whether a GT contains the specific IP version and a stepping within
> + * the specified range [from, until).  The lower stepping bound is inclusive,
> + * the upper bound is exclusive (corresponding to the first hardware stepping
> + * at when the workaround is no longer needed).  E.g.,
> + *
> + *    IS_GT_IP_STEP(GFX, IP_VER(12, 70), A0, B0)
> + *    IS_GT_IP_STEP(MEDIA, IP_VER(13, 00), B1, D0)
> + *    IS_GT_IP_STEP(GFX, IP_VER(12, 71), B1, FOREVER)
> + *
> + * "FOREVER" can be passed as the upper stepping bound for workarounds that
> + * have no upper bound on steppings of the specified IP version.
> + *
> + * Note that media version checks with this macro will only work on platforms
> + * with standalone media design (i.e., media version 13 and higher).
> + */
> +#define IS_GT_IP_STEP(type, gt, ipver, since, until) \
> +	(BUILD_BUG_ON_ZERO(ipver < IP_VER(2, 0)) + \
> +	 (__IS_##type##_GT(gt) && \
> +	  __GT_VER_FULL(gt) == ipver && \
> +	  __GT_STEP(gt) >= STEP_##since && \
> +	  __GT_STEP(gt) <= STEP_##until))
> +

Should this go in intel_gt.h?

>  #define MEDIA_VER(i915)			(RUNTIME_INFO(i915)->media.ip.ver)
>  #define MEDIA_VER_FULL(i915)		IP_VER(RUNTIME_INFO(i915)->media.ip.ver, \
>  					       RUNTIME_INFO(i915)->media.ip.rel)
> @@ -710,10 +736,6 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
>  #define IS_XEHPSDV_GRAPHICS_STEP(__i915, since, until) \
>  	(IS_XEHPSDV(__i915) && IS_GRAPHICS_STEP(__i915, since, until))
>  
> -#define IS_MTL_GRAPHICS_STEP(__i915, variant, since, until) \
> -	(IS_SUBPLATFORM(__i915, INTEL_METEORLAKE, INTEL_SUBPLATFORM_##variant) && \
> -	 IS_GRAPHICS_STEP(__i915, since, until))
> -

For completeness I would either leave this or remove all the
above. Or I would make this a wrapper around IS_GT_IP_STEP() with
a compile error if we are outside the MTL range.

Andi

>  #define IS_MTL_DISPLAY_STEP(__i915, since, until) \
>  	(IS_METEORLAKE(__i915) && \
>  	 IS_DISPLAY_STEP(__i915, since, until))
> -- 
> 2.41.0