On Mon, 24 Jul 2023, Matt Roper <matthew.d.roper@xxxxxxxxx> wrote: > +/* > + * 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)) > + I really dislike the type, since and until arguments here, passing in something that becomes part of the name of another macro. That's something we do in some of our macro spaghetti internally, but not as part of an interface that gets called all over the place. For function-like macros, I'd generally like to make them such that they could be converted to regular (static inline) functions without changing the call sites. BR, Jani. > #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)) > - > #define IS_MTL_DISPLAY_STEP(__i915, since, until) \ > (IS_METEORLAKE(__i915) && \ > IS_DISPLAY_STEP(__i915, since, until)) -- Jani Nikula, Intel Open Source Graphics Center
