| Message ID | 20230725001312.1907319-15-matthew.d.roper@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Reduce MTL-specific platform checks | expand |
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
On 25/07/2023 01:13, Matt Roper wrote: > Several workarounds are guarded by IS_MTL_GRAPHICS_STEP. However none > of these workarounds are actually tied to MTL as a platform; they only > relate to the Xe_LPG graphics IP, regardless of what platform it appears > in. At the moment MTL is the only platform that uses Xe_LPG with IP > versions 12.70 and 12.71, but we can't count on this being true in the > future. Switch these to use a new IS_GT_IP_STEP() macro instead that is > purely based on IP version. IS_GT_IP_STEP() is also GT-based rather > than device-based, which will help prevent mistakes where we > accidentally try to apply Xe_LPG graphics workarounds to the Xe_LPM+ > media GT and vice-versa. > > v2: > - Switch to a more generic and shorter IS_GT_IP_STEP macro that can be > used for both graphics and media IP (and any other kind of GTs that > show up in the future). > > Cc: Gustavo Sousa <gustavo.sousa@intel.com> > Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > --- > .../drm/i915/display/skl_universal_plane.c | 4 +- > drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 9 ++-- > drivers/gpu/drm/i915/gt/intel_gt_mcr.c | 4 +- > drivers/gpu/drm/i915/gt/intel_lrc.c | 4 +- > drivers/gpu/drm/i915/gt/intel_reset.c | 2 +- > drivers/gpu/drm/i915/gt/intel_workarounds.c | 52 ++++++++++--------- > drivers/gpu/drm/i915/gt/uc/intel_guc.c | 2 +- > .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 2 +- > drivers/gpu/drm/i915/i915_drv.h | 30 +++++++++-- > 9 files changed, 67 insertions(+), 42 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c > index 6b01a0b68b97..5fd8a42b3ec1 100644 > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c > @@ -2169,8 +2169,8 @@ static bool skl_plane_has_rc_ccs(struct drm_i915_private *i915, > enum pipe pipe, enum plane_id plane_id) > { > /* Wa_14017240301 */ > - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || > - IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0)) > + if (IS_GT_IP_STEP(GFX, to_gt(i915), IP_VER(12, 70), A0, B0) || > + IS_GT_IP_STEP(GFX, to_gt(i915), IP_VER(12, 71), A0, B0)) > return false; > > /* Wa_22011186057 */ > diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c > index 3ba20ea030e8..9d04591f3b4a 100644 > --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c > +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c > @@ -180,8 +180,8 @@ u32 *gen12_emit_aux_table_inv(struct intel_gt *gt, u32 *cs, const i915_reg_t inv > static int mtl_dummy_pipe_control(struct i915_request *rq) > { > /* Wa_14016712196 */ > - if (IS_MTL_GRAPHICS_STEP(rq->i915, M, STEP_A0, STEP_B0) || > - IS_MTL_GRAPHICS_STEP(rq->i915, P, STEP_A0, STEP_B0)) { > + if (IS_GT_IP_STEP(GFX, rq->engine->gt, IP_VER(12, 70), A0, B0) || > + IS_GT_IP_STEP(GFX, rq->engine->gt, IP_VER(12, 71), A0, B0)) { > u32 *cs; > > /* dummy PIPE_CONTROL + depth flush */ > @@ -755,6 +755,7 @@ u32 *gen12_emit_fini_breadcrumb_xcs(struct i915_request *rq, u32 *cs) > u32 *gen12_emit_fini_breadcrumb_rcs(struct i915_request *rq, u32 *cs) > { > struct drm_i915_private *i915 = rq->i915; > + struct intel_gt *gt = rq->engine->gt; > u32 flags = (PIPE_CONTROL_CS_STALL | > PIPE_CONTROL_TLB_INVALIDATE | > PIPE_CONTROL_TILE_CACHE_FLUSH | > @@ -765,8 +766,8 @@ u32 *gen12_emit_fini_breadcrumb_rcs(struct i915_request *rq, u32 *cs) > PIPE_CONTROL_FLUSH_ENABLE); > > /* Wa_14016712196 */ > - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || > - IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0)) > + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0) || > + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), A0, B0)) > /* dummy PIPE_CONTROL + depth flush */ > cs = gen12_emit_pipe_control(cs, 0, > PIPE_CONTROL_DEPTH_CACHE_FLUSH, 0); > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_mcr.c b/drivers/gpu/drm/i915/gt/intel_gt_mcr.c > index 0b414eae1683..c66373176f6c 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt_mcr.c > +++ b/drivers/gpu/drm/i915/gt/intel_gt_mcr.c > @@ -166,8 +166,8 @@ void intel_gt_mcr_init(struct intel_gt *gt) > gt->steering_table[OADDRM] = xelpmp_oaddrm_steering_table; > } else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70)) { > /* Wa_14016747170 */ > - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || > - IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0)) > + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0) || > + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), A0, B0)) > fuse = REG_FIELD_GET(MTL_GT_L3_EXC_MASK, > intel_uncore_read(gt->uncore, > MTL_GT_ACTIVITY_FACTOR)); > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c > index 1b710102390b..8057cc5800b3 100644 > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c > @@ -1377,8 +1377,8 @@ gen12_emit_indirect_ctx_rcs(const struct intel_context *ce, u32 *cs) > cs, GEN12_GFX_CCS_AUX_NV); > > /* Wa_16014892111 */ > - if (IS_MTL_GRAPHICS_STEP(ce->engine->i915, M, STEP_A0, STEP_B0) || > - IS_MTL_GRAPHICS_STEP(ce->engine->i915, P, STEP_A0, STEP_B0) || > + if (IS_GT_IP_STEP(GFX, ce->engine->gt, IP_VER(12, 70), A0, B0) || > + IS_GT_IP_STEP(GFX, ce->engine->gt, IP_VER(12, 71), A0, B0) || > IS_DG2(ce->engine->i915)) > cs = dg2_emit_draw_watermark_setting(cs); > > diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c > index 1ff7b42521c9..efe83157ef78 100644 > --- a/drivers/gpu/drm/i915/gt/intel_reset.c > +++ b/drivers/gpu/drm/i915/gt/intel_reset.c > @@ -1641,7 +1641,7 @@ bool intel_engine_reset_needs_wa_22011802037(struct intel_gt *gt) > if (GRAPHICS_VER(gt->i915) < 11) > return false; > > - if (IS_MTL_GRAPHICS_STEP(gt->i915, M, STEP_A0, STEP_B0)) > + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0)) > return true; > > if (GRAPHICS_VER_FULL(gt->i915) >= IP_VER(12, 70)) > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c > index c4663541c5dd..b7cf1790c827 100644 > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c > @@ -808,24 +808,24 @@ static void dg2_ctx_workarounds_init(struct intel_engine_cs *engine, > static void xelpg_ctx_gt_tuning_init(struct intel_engine_cs *engine, > struct i915_wa_list *wal) > { > - struct drm_i915_private *i915 = engine->i915; > + struct intel_gt *gt = engine->gt; > > dg2_ctx_gt_tuning_init(engine, wal); > > - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_B0, STEP_FOREVER) || > - IS_MTL_GRAPHICS_STEP(i915, P, STEP_B0, STEP_FOREVER)) > + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), B0, FOREVER) || > + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), B0, FOREVER)) > wa_add(wal, DRAW_WATERMARK, VERT_WM_VAL, 0x3FF, 0, false); > } > > static void xelpg_ctx_workarounds_init(struct intel_engine_cs *engine, > struct i915_wa_list *wal) > { > - struct drm_i915_private *i915 = engine->i915; > + struct intel_gt *gt = engine->gt; > > xelpg_ctx_gt_tuning_init(engine, wal); > > - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || > - IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0)) { > + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0) || > + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), A0, B0)) { > /* Wa_14014947963 */ > wa_masked_field_set(wal, VF_PREEMPTION, > PREEMPTION_VERTEX_COUNT, 0x4000); > @@ -1746,8 +1746,8 @@ xelpg_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal) > /* Wa_22016670082 */ > wa_write_or(wal, GEN12_SQCNT1, GEN12_STRICT_RAR_ENABLE); > > - if (IS_MTL_GRAPHICS_STEP(gt->i915, M, STEP_A0, STEP_B0) || > - IS_MTL_GRAPHICS_STEP(gt->i915, P, STEP_A0, STEP_B0)) { > + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0) || > + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), A0, B0)) { > /* Wa_14014830051 */ > wa_mcr_write_clr(wal, SARB_CHICKEN1, COMP_CKN_IN); > > @@ -2424,16 +2424,17 @@ static void > rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal) > { > struct drm_i915_private *i915 = engine->i915; > + struct intel_gt *gt = engine->gt; > > - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || > - IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0)) { > + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0) || > + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), A0, B0)) { > /* Wa_22014600077 */ > wa_mcr_masked_en(wal, GEN10_CACHE_MODE_SS, > ENABLE_EU_COUNT_FOR_TDL_FLUSH); > } > > - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || > - IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0) || > + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0) || > + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), A0, B0) || > IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_FOREVER) || > IS_DG2_G11(i915) || IS_DG2_G12(i915)) { > /* Wa_1509727124 */ > @@ -2443,7 +2444,7 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal) > > if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_FOREVER) || > IS_DG2_G11(i915) || IS_DG2_G12(i915) || > - IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0)) { > + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0)) { > /* Wa_22012856258 */ > wa_mcr_masked_en(wal, GEN8_ROW_CHICKEN2, > GEN12_DISABLE_READ_SUPPRESSION); > @@ -3008,8 +3009,9 @@ static void > general_render_compute_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal) > { > struct drm_i915_private *i915 = engine->i915; > + struct intel_gt *gt = engine->gt; > > - add_render_compute_tuning_settings(engine->gt, wal); > + add_render_compute_tuning_settings(gt, wal); > > if (GRAPHICS_VER(i915) >= 11) { > /* This is not a Wa (although referred to as > @@ -3030,13 +3032,13 @@ general_render_compute_wa_init(struct intel_engine_cs *engine, struct i915_wa_li > GEN11_INDIRECT_STATE_BASE_ADDR_OVERRIDE); > } > > - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_B0, STEP_FOREVER) || > - IS_MTL_GRAPHICS_STEP(i915, P, STEP_B0, STEP_FOREVER)) > + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), B0, FOREVER) || > + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), B0, FOREVER)) > /* Wa_14017856879 */ > wa_mcr_masked_en(wal, GEN9_ROW_CHICKEN3, MTL_DISABLE_FIX_FOR_EOT_FLUSH); > > - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || > - IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0)) > + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0) || > + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), A0, B0)) > /* > * Wa_14017066071 > * Wa_14017654203 > @@ -3044,13 +3046,13 @@ general_render_compute_wa_init(struct intel_engine_cs *engine, struct i915_wa_li > wa_mcr_masked_en(wal, GEN10_SAMPLER_MODE, > MTL_DISABLE_SAMPLER_SC_OOO); > > - if (IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0)) > + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), A0, B0)) > /* Wa_22015279794 */ > wa_mcr_masked_en(wal, GEN10_CACHE_MODE_SS, > DISABLE_PREFETCH_INTO_IC); > > - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || > - IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0) || > + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0) || > + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), A0, B0) || > IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_FOREVER) || > IS_DG2_G11(i915) || IS_DG2_G12(i915)) { > /* Wa_22013037850 */ > @@ -3058,16 +3060,16 @@ general_render_compute_wa_init(struct intel_engine_cs *engine, struct i915_wa_li > DISABLE_128B_EVICTION_COMMAND_UDW); > } > > - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || > - IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0) || > + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0) || > + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), A0, B0) || > IS_PONTEVECCHIO(i915) || > IS_DG2(i915)) { > /* Wa_22014226127 */ > wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0, DISABLE_D8_D16_COASLESCE); > } > > - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || > - IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0) || > + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0) || > + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), A0, B0) || > IS_DG2(i915)) { > /* Wa_18017747507 */ > wa_masked_en(wal, VFG_PREEMPTION_CHICKEN, POLYGON_TRIFAN_LINELOOP_DISABLE); > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c > index 1e532981f74e..c733d43816f4 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c > @@ -277,7 +277,7 @@ static u32 guc_ctl_wa_flags(struct intel_guc *guc) > flags |= GUC_WA_GAM_CREDITS; > > /* Wa_14014475959 */ > - if (IS_MTL_GRAPHICS_STEP(gt->i915, M, STEP_A0, STEP_B0) || > + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0) || > IS_DG2(gt->i915)) > flags |= GUC_WA_HOLD_CCS_SWITCHOUT; > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > index 1bd5d8f7c40b..01d6534dc7e8 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > @@ -4265,7 +4265,7 @@ static void guc_default_vfuncs(struct intel_engine_cs *engine) > > /* Wa_14014475959:dg2 */ > if (engine->class == COMPUTE_CLASS) > - if (IS_MTL_GRAPHICS_STEP(engine->i915, M, STEP_A0, STEP_B0) || > + if (IS_GT_IP_STEP(GFX, engine->gt, IP_VER(12, 70), A0, B0) || > IS_DG2(engine->i915)) > engine->flags |= I915_ENGINE_USES_WA_HOLD_CCS_SWITCHOUT; > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 6bb825d40451..687745550355 100644 > --- 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) Same as previous patch, I'd really like that we have the GT type split out from the macro. Btw I found a way to implement optional macro arguments on Stack Overflow and we could have that too, with default stepping [FOREVER, FOREVER] if we wanted. That way we could only have IS_GT_IP and call it as either IS_GT_IP(gt, IP_VER()) or IS_GT_IP(gt, IP_VER(), A0, B0). But I think *only* if the compiler ends up smart enough not to pollute the code with impossible default forever-forever checks. Possibly a step too far. Regards, Tvrtko > + * > + * "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)) > + > #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))
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
On Tue, Jul 25, 2023 at 01:10:24PM +0100, Tvrtko Ursulin wrote: > > On 25/07/2023 01:13, Matt Roper wrote: > > Several workarounds are guarded by IS_MTL_GRAPHICS_STEP. However none > > of these workarounds are actually tied to MTL as a platform; they only > > relate to the Xe_LPG graphics IP, regardless of what platform it appears > > in. At the moment MTL is the only platform that uses Xe_LPG with IP > > versions 12.70 and 12.71, but we can't count on this being true in the > > future. Switch these to use a new IS_GT_IP_STEP() macro instead that is > > purely based on IP version. IS_GT_IP_STEP() is also GT-based rather > > than device-based, which will help prevent mistakes where we > > accidentally try to apply Xe_LPG graphics workarounds to the Xe_LPM+ > > media GT and vice-versa. > > > > v2: > > - Switch to a more generic and shorter IS_GT_IP_STEP macro that can be > > used for both graphics and media IP (and any other kind of GTs that > > show up in the future). > > > > Cc: Gustavo Sousa <gustavo.sousa@intel.com> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > > --- > > .../drm/i915/display/skl_universal_plane.c | 4 +- > > drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 9 ++-- > > drivers/gpu/drm/i915/gt/intel_gt_mcr.c | 4 +- > > drivers/gpu/drm/i915/gt/intel_lrc.c | 4 +- > > drivers/gpu/drm/i915/gt/intel_reset.c | 2 +- > > drivers/gpu/drm/i915/gt/intel_workarounds.c | 52 ++++++++++--------- > > drivers/gpu/drm/i915/gt/uc/intel_guc.c | 2 +- > > .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 2 +- > > drivers/gpu/drm/i915/i915_drv.h | 30 +++++++++-- > > 9 files changed, 67 insertions(+), 42 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c > > index 6b01a0b68b97..5fd8a42b3ec1 100644 > > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c > > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c > > @@ -2169,8 +2169,8 @@ static bool skl_plane_has_rc_ccs(struct drm_i915_private *i915, > > enum pipe pipe, enum plane_id plane_id) > > { > > /* Wa_14017240301 */ > > - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || > > - IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0)) > > + if (IS_GT_IP_STEP(GFX, to_gt(i915), IP_VER(12, 70), A0, B0) || > > + IS_GT_IP_STEP(GFX, to_gt(i915), IP_VER(12, 71), A0, B0)) > > return false; > > /* Wa_22011186057 */ > > diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c > > index 3ba20ea030e8..9d04591f3b4a 100644 > > --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c > > +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c > > @@ -180,8 +180,8 @@ u32 *gen12_emit_aux_table_inv(struct intel_gt *gt, u32 *cs, const i915_reg_t inv > > static int mtl_dummy_pipe_control(struct i915_request *rq) > > { > > /* Wa_14016712196 */ > > - if (IS_MTL_GRAPHICS_STEP(rq->i915, M, STEP_A0, STEP_B0) || > > - IS_MTL_GRAPHICS_STEP(rq->i915, P, STEP_A0, STEP_B0)) { > > + if (IS_GT_IP_STEP(GFX, rq->engine->gt, IP_VER(12, 70), A0, B0) || > > + IS_GT_IP_STEP(GFX, rq->engine->gt, IP_VER(12, 71), A0, B0)) { > > u32 *cs; > > /* dummy PIPE_CONTROL + depth flush */ > > @@ -755,6 +755,7 @@ u32 *gen12_emit_fini_breadcrumb_xcs(struct i915_request *rq, u32 *cs) > > u32 *gen12_emit_fini_breadcrumb_rcs(struct i915_request *rq, u32 *cs) > > { > > struct drm_i915_private *i915 = rq->i915; > > + struct intel_gt *gt = rq->engine->gt; > > u32 flags = (PIPE_CONTROL_CS_STALL | > > PIPE_CONTROL_TLB_INVALIDATE | > > PIPE_CONTROL_TILE_CACHE_FLUSH | > > @@ -765,8 +766,8 @@ u32 *gen12_emit_fini_breadcrumb_rcs(struct i915_request *rq, u32 *cs) > > PIPE_CONTROL_FLUSH_ENABLE); > > /* Wa_14016712196 */ > > - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || > > - IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0)) > > + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0) || > > + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), A0, B0)) > > /* dummy PIPE_CONTROL + depth flush */ > > cs = gen12_emit_pipe_control(cs, 0, > > PIPE_CONTROL_DEPTH_CACHE_FLUSH, 0); > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_mcr.c b/drivers/gpu/drm/i915/gt/intel_gt_mcr.c > > index 0b414eae1683..c66373176f6c 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_gt_mcr.c > > +++ b/drivers/gpu/drm/i915/gt/intel_gt_mcr.c > > @@ -166,8 +166,8 @@ void intel_gt_mcr_init(struct intel_gt *gt) > > gt->steering_table[OADDRM] = xelpmp_oaddrm_steering_table; > > } else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70)) { > > /* Wa_14016747170 */ > > - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || > > - IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0)) > > + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0) || > > + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), A0, B0)) > > fuse = REG_FIELD_GET(MTL_GT_L3_EXC_MASK, > > intel_uncore_read(gt->uncore, > > MTL_GT_ACTIVITY_FACTOR)); > > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c > > index 1b710102390b..8057cc5800b3 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c > > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c > > @@ -1377,8 +1377,8 @@ gen12_emit_indirect_ctx_rcs(const struct intel_context *ce, u32 *cs) > > cs, GEN12_GFX_CCS_AUX_NV); > > /* Wa_16014892111 */ > > - if (IS_MTL_GRAPHICS_STEP(ce->engine->i915, M, STEP_A0, STEP_B0) || > > - IS_MTL_GRAPHICS_STEP(ce->engine->i915, P, STEP_A0, STEP_B0) || > > + if (IS_GT_IP_STEP(GFX, ce->engine->gt, IP_VER(12, 70), A0, B0) || > > + IS_GT_IP_STEP(GFX, ce->engine->gt, IP_VER(12, 71), A0, B0) || > > IS_DG2(ce->engine->i915)) > > cs = dg2_emit_draw_watermark_setting(cs); > > diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c > > index 1ff7b42521c9..efe83157ef78 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_reset.c > > +++ b/drivers/gpu/drm/i915/gt/intel_reset.c > > @@ -1641,7 +1641,7 @@ bool intel_engine_reset_needs_wa_22011802037(struct intel_gt *gt) > > if (GRAPHICS_VER(gt->i915) < 11) > > return false; > > - if (IS_MTL_GRAPHICS_STEP(gt->i915, M, STEP_A0, STEP_B0)) > > + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0)) > > return true; > > if (GRAPHICS_VER_FULL(gt->i915) >= IP_VER(12, 70)) > > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c > > index c4663541c5dd..b7cf1790c827 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c > > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c > > @@ -808,24 +808,24 @@ static void dg2_ctx_workarounds_init(struct intel_engine_cs *engine, > > static void xelpg_ctx_gt_tuning_init(struct intel_engine_cs *engine, > > struct i915_wa_list *wal) > > { > > - struct drm_i915_private *i915 = engine->i915; > > + struct intel_gt *gt = engine->gt; > > dg2_ctx_gt_tuning_init(engine, wal); > > - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_B0, STEP_FOREVER) || > > - IS_MTL_GRAPHICS_STEP(i915, P, STEP_B0, STEP_FOREVER)) > > + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), B0, FOREVER) || > > + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), B0, FOREVER)) > > wa_add(wal, DRAW_WATERMARK, VERT_WM_VAL, 0x3FF, 0, false); > > } > > static void xelpg_ctx_workarounds_init(struct intel_engine_cs *engine, > > struct i915_wa_list *wal) > > { > > - struct drm_i915_private *i915 = engine->i915; > > + struct intel_gt *gt = engine->gt; > > xelpg_ctx_gt_tuning_init(engine, wal); > > - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || > > - IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0)) { > > + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0) || > > + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), A0, B0)) { > > /* Wa_14014947963 */ > > wa_masked_field_set(wal, VF_PREEMPTION, > > PREEMPTION_VERTEX_COUNT, 0x4000); > > @@ -1746,8 +1746,8 @@ xelpg_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal) > > /* Wa_22016670082 */ > > wa_write_or(wal, GEN12_SQCNT1, GEN12_STRICT_RAR_ENABLE); > > - if (IS_MTL_GRAPHICS_STEP(gt->i915, M, STEP_A0, STEP_B0) || > > - IS_MTL_GRAPHICS_STEP(gt->i915, P, STEP_A0, STEP_B0)) { > > + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0) || > > + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), A0, B0)) { > > /* Wa_14014830051 */ > > wa_mcr_write_clr(wal, SARB_CHICKEN1, COMP_CKN_IN); > > @@ -2424,16 +2424,17 @@ static void > > rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal) > > { > > struct drm_i915_private *i915 = engine->i915; > > + struct intel_gt *gt = engine->gt; > > - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || > > - IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0)) { > > + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0) || > > + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), A0, B0)) { > > /* Wa_22014600077 */ > > wa_mcr_masked_en(wal, GEN10_CACHE_MODE_SS, > > ENABLE_EU_COUNT_FOR_TDL_FLUSH); > > } > > - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || > > - IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0) || > > + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0) || > > + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), A0, B0) || > > IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_FOREVER) || > > IS_DG2_G11(i915) || IS_DG2_G12(i915)) { > > /* Wa_1509727124 */ > > @@ -2443,7 +2444,7 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal) > > if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_FOREVER) || > > IS_DG2_G11(i915) || IS_DG2_G12(i915) || > > - IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0)) { > > + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0)) { > > /* Wa_22012856258 */ > > wa_mcr_masked_en(wal, GEN8_ROW_CHICKEN2, > > GEN12_DISABLE_READ_SUPPRESSION); > > @@ -3008,8 +3009,9 @@ static void > > general_render_compute_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal) > > { > > struct drm_i915_private *i915 = engine->i915; > > + struct intel_gt *gt = engine->gt; > > - add_render_compute_tuning_settings(engine->gt, wal); > > + add_render_compute_tuning_settings(gt, wal); > > if (GRAPHICS_VER(i915) >= 11) { > > /* This is not a Wa (although referred to as > > @@ -3030,13 +3032,13 @@ general_render_compute_wa_init(struct intel_engine_cs *engine, struct i915_wa_li > > GEN11_INDIRECT_STATE_BASE_ADDR_OVERRIDE); > > } > > - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_B0, STEP_FOREVER) || > > - IS_MTL_GRAPHICS_STEP(i915, P, STEP_B0, STEP_FOREVER)) > > + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), B0, FOREVER) || > > + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), B0, FOREVER)) > > /* Wa_14017856879 */ > > wa_mcr_masked_en(wal, GEN9_ROW_CHICKEN3, MTL_DISABLE_FIX_FOR_EOT_FLUSH); > > - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || > > - IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0)) > > + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0) || > > + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), A0, B0)) > > /* > > * Wa_14017066071 > > * Wa_14017654203 > > @@ -3044,13 +3046,13 @@ general_render_compute_wa_init(struct intel_engine_cs *engine, struct i915_wa_li > > wa_mcr_masked_en(wal, GEN10_SAMPLER_MODE, > > MTL_DISABLE_SAMPLER_SC_OOO); > > - if (IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0)) > > + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), A0, B0)) > > /* Wa_22015279794 */ > > wa_mcr_masked_en(wal, GEN10_CACHE_MODE_SS, > > DISABLE_PREFETCH_INTO_IC); > > - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || > > - IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0) || > > + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0) || > > + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), A0, B0) || > > IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_FOREVER) || > > IS_DG2_G11(i915) || IS_DG2_G12(i915)) { > > /* Wa_22013037850 */ > > @@ -3058,16 +3060,16 @@ general_render_compute_wa_init(struct intel_engine_cs *engine, struct i915_wa_li > > DISABLE_128B_EVICTION_COMMAND_UDW); > > } > > - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || > > - IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0) || > > + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0) || > > + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), A0, B0) || > > IS_PONTEVECCHIO(i915) || > > IS_DG2(i915)) { > > /* Wa_22014226127 */ > > wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0, DISABLE_D8_D16_COASLESCE); > > } > > - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || > > - IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0) || > > + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0) || > > + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), A0, B0) || > > IS_DG2(i915)) { > > /* Wa_18017747507 */ > > wa_masked_en(wal, VFG_PREEMPTION_CHICKEN, POLYGON_TRIFAN_LINELOOP_DISABLE); > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c > > index 1e532981f74e..c733d43816f4 100644 > > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c > > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c > > @@ -277,7 +277,7 @@ static u32 guc_ctl_wa_flags(struct intel_guc *guc) > > flags |= GUC_WA_GAM_CREDITS; > > /* Wa_14014475959 */ > > - if (IS_MTL_GRAPHICS_STEP(gt->i915, M, STEP_A0, STEP_B0) || > > + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0) || > > IS_DG2(gt->i915)) > > flags |= GUC_WA_HOLD_CCS_SWITCHOUT; > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > > index 1bd5d8f7c40b..01d6534dc7e8 100644 > > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > > @@ -4265,7 +4265,7 @@ static void guc_default_vfuncs(struct intel_engine_cs *engine) > > /* Wa_14014475959:dg2 */ > > if (engine->class == COMPUTE_CLASS) > > - if (IS_MTL_GRAPHICS_STEP(engine->i915, M, STEP_A0, STEP_B0) || > > + if (IS_GT_IP_STEP(GFX, engine->gt, IP_VER(12, 70), A0, B0) || > > IS_DG2(engine->i915)) > > engine->flags |= I915_ENGINE_USES_WA_HOLD_CCS_SWITCHOUT; > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > index 6bb825d40451..687745550355 100644 > > --- 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) > > Same as previous patch, I'd really like that we have the GT type split out > from the macro. > > Btw I found a way to implement optional macro arguments on Stack Overflow > and we could have that too, with default stepping [FOREVER, FOREVER] if we > wanted. Presumably we'd want [0, FOREVER) rather than [FOREVER, FOREVER], otherwise we'd never match anything. But I'm still questioning whether that would really help clarify anything. If we've lost the GT type check, then I don't see what benefit a somewhat ambiguous IS_GT_IP() would provide over a more direct "GRAPHICS_VER_FULL() == IP_VER()" that makes it clear what kind of version we're checking for. And we'd still need to handle version ranges independently from stepping ranges since those need different semantics. Matt > > That way we could only have IS_GT_IP and call it as either IS_GT_IP(gt, > IP_VER()) or IS_GT_IP(gt, IP_VER(), A0, B0). > > But I think *only* if the compiler ends up smart enough not to pollute the > code with impossible default forever-forever checks. > > Possibly a step too far. > > Regards, > > Tvrtko > > > + * > > + * "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)) > > + > > #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))
On Mon, 24 Jul 2023, Matt Roper <matthew.d.roper@intel.com> 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))
diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c index 6b01a0b68b97..5fd8a42b3ec1 100644 --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c @@ -2169,8 +2169,8 @@ static bool skl_plane_has_rc_ccs(struct drm_i915_private *i915, enum pipe pipe, enum plane_id plane_id) { /* Wa_14017240301 */ - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || - IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0)) + if (IS_GT_IP_STEP(GFX, to_gt(i915), IP_VER(12, 70), A0, B0) || + IS_GT_IP_STEP(GFX, to_gt(i915), IP_VER(12, 71), A0, B0)) return false; /* Wa_22011186057 */ diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c index 3ba20ea030e8..9d04591f3b4a 100644 --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c @@ -180,8 +180,8 @@ u32 *gen12_emit_aux_table_inv(struct intel_gt *gt, u32 *cs, const i915_reg_t inv static int mtl_dummy_pipe_control(struct i915_request *rq) { /* Wa_14016712196 */ - if (IS_MTL_GRAPHICS_STEP(rq->i915, M, STEP_A0, STEP_B0) || - IS_MTL_GRAPHICS_STEP(rq->i915, P, STEP_A0, STEP_B0)) { + if (IS_GT_IP_STEP(GFX, rq->engine->gt, IP_VER(12, 70), A0, B0) || + IS_GT_IP_STEP(GFX, rq->engine->gt, IP_VER(12, 71), A0, B0)) { u32 *cs; /* dummy PIPE_CONTROL + depth flush */ @@ -755,6 +755,7 @@ u32 *gen12_emit_fini_breadcrumb_xcs(struct i915_request *rq, u32 *cs) u32 *gen12_emit_fini_breadcrumb_rcs(struct i915_request *rq, u32 *cs) { struct drm_i915_private *i915 = rq->i915; + struct intel_gt *gt = rq->engine->gt; u32 flags = (PIPE_CONTROL_CS_STALL | PIPE_CONTROL_TLB_INVALIDATE | PIPE_CONTROL_TILE_CACHE_FLUSH | @@ -765,8 +766,8 @@ u32 *gen12_emit_fini_breadcrumb_rcs(struct i915_request *rq, u32 *cs) PIPE_CONTROL_FLUSH_ENABLE); /* Wa_14016712196 */ - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || - IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0)) + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0) || + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), A0, B0)) /* dummy PIPE_CONTROL + depth flush */ cs = gen12_emit_pipe_control(cs, 0, PIPE_CONTROL_DEPTH_CACHE_FLUSH, 0); diff --git a/drivers/gpu/drm/i915/gt/intel_gt_mcr.c b/drivers/gpu/drm/i915/gt/intel_gt_mcr.c index 0b414eae1683..c66373176f6c 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_mcr.c +++ b/drivers/gpu/drm/i915/gt/intel_gt_mcr.c @@ -166,8 +166,8 @@ void intel_gt_mcr_init(struct intel_gt *gt) gt->steering_table[OADDRM] = xelpmp_oaddrm_steering_table; } else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70)) { /* Wa_14016747170 */ - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || - IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0)) + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0) || + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), A0, B0)) fuse = REG_FIELD_GET(MTL_GT_L3_EXC_MASK, intel_uncore_read(gt->uncore, MTL_GT_ACTIVITY_FACTOR)); diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c index 1b710102390b..8057cc5800b3 100644 --- a/drivers/gpu/drm/i915/gt/intel_lrc.c +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c @@ -1377,8 +1377,8 @@ gen12_emit_indirect_ctx_rcs(const struct intel_context *ce, u32 *cs) cs, GEN12_GFX_CCS_AUX_NV); /* Wa_16014892111 */ - if (IS_MTL_GRAPHICS_STEP(ce->engine->i915, M, STEP_A0, STEP_B0) || - IS_MTL_GRAPHICS_STEP(ce->engine->i915, P, STEP_A0, STEP_B0) || + if (IS_GT_IP_STEP(GFX, ce->engine->gt, IP_VER(12, 70), A0, B0) || + IS_GT_IP_STEP(GFX, ce->engine->gt, IP_VER(12, 71), A0, B0) || IS_DG2(ce->engine->i915)) cs = dg2_emit_draw_watermark_setting(cs); diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c index 1ff7b42521c9..efe83157ef78 100644 --- a/drivers/gpu/drm/i915/gt/intel_reset.c +++ b/drivers/gpu/drm/i915/gt/intel_reset.c @@ -1641,7 +1641,7 @@ bool intel_engine_reset_needs_wa_22011802037(struct intel_gt *gt) if (GRAPHICS_VER(gt->i915) < 11) return false; - if (IS_MTL_GRAPHICS_STEP(gt->i915, M, STEP_A0, STEP_B0)) + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0)) return true; if (GRAPHICS_VER_FULL(gt->i915) >= IP_VER(12, 70)) diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c index c4663541c5dd..b7cf1790c827 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c @@ -808,24 +808,24 @@ static void dg2_ctx_workarounds_init(struct intel_engine_cs *engine, static void xelpg_ctx_gt_tuning_init(struct intel_engine_cs *engine, struct i915_wa_list *wal) { - struct drm_i915_private *i915 = engine->i915; + struct intel_gt *gt = engine->gt; dg2_ctx_gt_tuning_init(engine, wal); - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_B0, STEP_FOREVER) || - IS_MTL_GRAPHICS_STEP(i915, P, STEP_B0, STEP_FOREVER)) + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), B0, FOREVER) || + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), B0, FOREVER)) wa_add(wal, DRAW_WATERMARK, VERT_WM_VAL, 0x3FF, 0, false); } static void xelpg_ctx_workarounds_init(struct intel_engine_cs *engine, struct i915_wa_list *wal) { - struct drm_i915_private *i915 = engine->i915; + struct intel_gt *gt = engine->gt; xelpg_ctx_gt_tuning_init(engine, wal); - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || - IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0)) { + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0) || + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), A0, B0)) { /* Wa_14014947963 */ wa_masked_field_set(wal, VF_PREEMPTION, PREEMPTION_VERTEX_COUNT, 0x4000); @@ -1746,8 +1746,8 @@ xelpg_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal) /* Wa_22016670082 */ wa_write_or(wal, GEN12_SQCNT1, GEN12_STRICT_RAR_ENABLE); - if (IS_MTL_GRAPHICS_STEP(gt->i915, M, STEP_A0, STEP_B0) || - IS_MTL_GRAPHICS_STEP(gt->i915, P, STEP_A0, STEP_B0)) { + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0) || + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), A0, B0)) { /* Wa_14014830051 */ wa_mcr_write_clr(wal, SARB_CHICKEN1, COMP_CKN_IN); @@ -2424,16 +2424,17 @@ static void rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal) { struct drm_i915_private *i915 = engine->i915; + struct intel_gt *gt = engine->gt; - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || - IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0)) { + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0) || + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), A0, B0)) { /* Wa_22014600077 */ wa_mcr_masked_en(wal, GEN10_CACHE_MODE_SS, ENABLE_EU_COUNT_FOR_TDL_FLUSH); } - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || - IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0) || + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0) || + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), A0, B0) || IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_FOREVER) || IS_DG2_G11(i915) || IS_DG2_G12(i915)) { /* Wa_1509727124 */ @@ -2443,7 +2444,7 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal) if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_FOREVER) || IS_DG2_G11(i915) || IS_DG2_G12(i915) || - IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0)) { + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0)) { /* Wa_22012856258 */ wa_mcr_masked_en(wal, GEN8_ROW_CHICKEN2, GEN12_DISABLE_READ_SUPPRESSION); @@ -3008,8 +3009,9 @@ static void general_render_compute_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal) { struct drm_i915_private *i915 = engine->i915; + struct intel_gt *gt = engine->gt; - add_render_compute_tuning_settings(engine->gt, wal); + add_render_compute_tuning_settings(gt, wal); if (GRAPHICS_VER(i915) >= 11) { /* This is not a Wa (although referred to as @@ -3030,13 +3032,13 @@ general_render_compute_wa_init(struct intel_engine_cs *engine, struct i915_wa_li GEN11_INDIRECT_STATE_BASE_ADDR_OVERRIDE); } - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_B0, STEP_FOREVER) || - IS_MTL_GRAPHICS_STEP(i915, P, STEP_B0, STEP_FOREVER)) + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), B0, FOREVER) || + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), B0, FOREVER)) /* Wa_14017856879 */ wa_mcr_masked_en(wal, GEN9_ROW_CHICKEN3, MTL_DISABLE_FIX_FOR_EOT_FLUSH); - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || - IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0)) + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0) || + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), A0, B0)) /* * Wa_14017066071 * Wa_14017654203 @@ -3044,13 +3046,13 @@ general_render_compute_wa_init(struct intel_engine_cs *engine, struct i915_wa_li wa_mcr_masked_en(wal, GEN10_SAMPLER_MODE, MTL_DISABLE_SAMPLER_SC_OOO); - if (IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0)) + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), A0, B0)) /* Wa_22015279794 */ wa_mcr_masked_en(wal, GEN10_CACHE_MODE_SS, DISABLE_PREFETCH_INTO_IC); - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || - IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0) || + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0) || + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), A0, B0) || IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_FOREVER) || IS_DG2_G11(i915) || IS_DG2_G12(i915)) { /* Wa_22013037850 */ @@ -3058,16 +3060,16 @@ general_render_compute_wa_init(struct intel_engine_cs *engine, struct i915_wa_li DISABLE_128B_EVICTION_COMMAND_UDW); } - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || - IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0) || + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0) || + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), A0, B0) || IS_PONTEVECCHIO(i915) || IS_DG2(i915)) { /* Wa_22014226127 */ wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0, DISABLE_D8_D16_COASLESCE); } - if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || - IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0) || + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0) || + IS_GT_IP_STEP(GFX, gt, IP_VER(12, 71), A0, B0) || IS_DG2(i915)) { /* Wa_18017747507 */ wa_masked_en(wal, VFG_PREEMPTION_CHICKEN, POLYGON_TRIFAN_LINELOOP_DISABLE); diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c index 1e532981f74e..c733d43816f4 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c @@ -277,7 +277,7 @@ static u32 guc_ctl_wa_flags(struct intel_guc *guc) flags |= GUC_WA_GAM_CREDITS; /* Wa_14014475959 */ - if (IS_MTL_GRAPHICS_STEP(gt->i915, M, STEP_A0, STEP_B0) || + if (IS_GT_IP_STEP(GFX, gt, IP_VER(12, 70), A0, B0) || IS_DG2(gt->i915)) flags |= GUC_WA_HOLD_CCS_SWITCHOUT; diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index 1bd5d8f7c40b..01d6534dc7e8 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -4265,7 +4265,7 @@ static void guc_default_vfuncs(struct intel_engine_cs *engine) /* Wa_14014475959:dg2 */ if (engine->class == COMPUTE_CLASS) - if (IS_MTL_GRAPHICS_STEP(engine->i915, M, STEP_A0, STEP_B0) || + if (IS_GT_IP_STEP(GFX, engine->gt, IP_VER(12, 70), A0, B0) || IS_DG2(engine->i915)) engine->flags |= I915_ENGINE_USES_WA_HOLD_CCS_SWITCHOUT; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 6bb825d40451..687745550355 100644 --- 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)) + #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))
Several workarounds are guarded by IS_MTL_GRAPHICS_STEP. However none of these workarounds are actually tied to MTL as a platform; they only relate to the Xe_LPG graphics IP, regardless of what platform it appears in. At the moment MTL is the only platform that uses Xe_LPG with IP versions 12.70 and 12.71, but we can't count on this being true in the future. Switch these to use a new IS_GT_IP_STEP() macro instead that is purely based on IP version. IS_GT_IP_STEP() is also GT-based rather than device-based, which will help prevent mistakes where we accidentally try to apply Xe_LPG graphics workarounds to the Xe_LPM+ media GT and vice-versa. v2: - Switch to a more generic and shorter IS_GT_IP_STEP macro that can be used for both graphics and media IP (and any other kind of GTs that show up in the future). Cc: Gustavo Sousa <gustavo.sousa@intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- .../drm/i915/display/skl_universal_plane.c | 4 +- drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 9 ++-- drivers/gpu/drm/i915/gt/intel_gt_mcr.c | 4 +- drivers/gpu/drm/i915/gt/intel_lrc.c | 4 +- drivers/gpu/drm/i915/gt/intel_reset.c | 2 +- drivers/gpu/drm/i915/gt/intel_workarounds.c | 52 ++++++++++--------- drivers/gpu/drm/i915/gt/uc/intel_guc.c | 2 +- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 2 +- drivers/gpu/drm/i915/i915_drv.h | 30 +++++++++-- 9 files changed, 67 insertions(+), 42 deletions(-)