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

[Matt Roper]

On Tue, Jul 25, 2023 at 12:04:43PM +0200, Andi Shyti wrote:
> 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.

If we leave this, then someone might try to use it in future patches.
Every single place this macro gets used will always be a driver bug,
which is the motivation for killing it off.

In contast, the ones for older platforms are correct and should remain.
Before the hardware switched to the disaggretated IP design, steppings
for each IP were directly tied to the base platform rather than the IP
version, and we inferred from the PCI revid.


Matt

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

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