| Message ID | 20230718222753.1075713-13-matthew.d.roper@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Reduce MTL-specific platform checks | expand |
On 18/07/2023 23:27, 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_GFX_IPVER_STEP() macro instead > that is purely based on IP version. IS_GFX_IPVER_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. > > 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 | 8 +-- > 9 files changed, 46 insertions(+), 41 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..c13e64faaaad 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_GFX_IPVER_STEP(to_gt(i915), IP_VER(12, 70), STEP_A0, STEP_B0) || > + IS_GFX_IPVER_STEP(to_gt(i915), IP_VER(12, 71), STEP_A0, STEP_B0)) Do we really need/want a combined/long macro like this and not split it out to something like a more readable IS_GFX_IPVER && IS_<...>_STEP? Adding Jani since we were discussing this direction over the past few weeks. Hm.. macro trickery for stepping ranges to make it more compact? Like making the STEP_ prefix be prepended by the macro. Don't know.. Regards, Tvrtko > 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 23857cc08eca..c1af91d908e5 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->engine->i915, M, STEP_A0, STEP_B0) || > - IS_MTL_GRAPHICS_STEP(rq->engine->i915, P, STEP_A0, STEP_B0)) { > + if (IS_GFX_IPVER_STEP(rq->engine->gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > + IS_GFX_IPVER_STEP(rq->engine->gt, IP_VER(12, 71), STEP_A0, STEP_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->engine->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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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..41140eb86051 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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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..4fefa67d285f 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_GFX_IPVER_STEP(ce->engine->gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > + IS_GFX_IPVER_STEP(ce->engine->gt, IP_VER(12, 71), STEP_A0, STEP_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..403f0d9caadf 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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_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 2a5bf50962ad..785453b2e95e 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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_B0, STEP_FOREVER) || > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_B0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_B0, STEP_FOREVER) || > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_B0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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..300b968e6de8 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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_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..3b159069dbc4 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_GFX_IPVER_STEP(engine->gt, IP_VER(12, 70), STEP_A0, STEP_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 f38c93d82c56..d03710c923c8 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -692,9 +692,11 @@ 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_GFX_IPVER_STEP(__gt, ipver, since, until) \ > + (BUILD_BUG_ON_ZERO(ipver < IP_VER(2, 0)) + \ > + (__gt->type != GT_MEDIA && \ > + GRAPHICS_VER_FULL(__gt->i915) == ipver && \ > + IS_GRAPHICS_STEP(__gt->i915, since, until))) > > #define IS_MTL_DISPLAY_STEP(__i915, since, until) \ > (IS_METEORLAKE(__i915) && \
On Wed, Jul 19, 2023 at 09:01:58AM +0100, Tvrtko Ursulin wrote: > > On 18/07/2023 23:27, 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_GFX_IPVER_STEP() macro instead > > that is purely based on IP version. IS_GFX_IPVER_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. > > > > 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 | 8 +-- > > 9 files changed, 46 insertions(+), 41 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..c13e64faaaad 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_GFX_IPVER_STEP(to_gt(i915), IP_VER(12, 70), STEP_A0, STEP_B0) || > > + IS_GFX_IPVER_STEP(to_gt(i915), IP_VER(12, 71), STEP_A0, STEP_B0)) > > Do we really need/want a combined/long macro like this and not split it out > to something like a more readable IS_GFX_IPVER && IS_<...>_STEP? > > Adding Jani since we were discussing this direction over the past few weeks. > > Hm.. macro trickery for stepping ranges to make it more compact? Like making > the STEP_ prefix be prepended by the macro. Don't know.. One other thing that we could consider if some of these things are getting too long is eliminating IP_VER() and using the "versionx100" scheme that Xe and Mesa use. On one hand that makes me a bit nervous since GMD_ID's release number is 8 bits and could go past 100 on future platforms, which would break the scheme (and we're already up to .71 on MTL). But on the other hand it does make the code simpler and shorter and we can always wait and see if we ever actually get a platform with version X.100 or something before we really worry about what to do there. The other point in favor of switching to versionx100 is that we wouldn't have an inconsistent mix of both IP_VER() and verx100 in the Xe driver where the display code comes from i915 but the non-display code is separate. Matt > > Regards, > > Tvrtko > > > 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 23857cc08eca..c1af91d908e5 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->engine->i915, M, STEP_A0, STEP_B0) || > > - IS_MTL_GRAPHICS_STEP(rq->engine->i915, P, STEP_A0, STEP_B0)) { > > + if (IS_GFX_IPVER_STEP(rq->engine->gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > > + IS_GFX_IPVER_STEP(rq->engine->gt, IP_VER(12, 71), STEP_A0, STEP_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->engine->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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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..41140eb86051 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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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..4fefa67d285f 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_GFX_IPVER_STEP(ce->engine->gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > > + IS_GFX_IPVER_STEP(ce->engine->gt, IP_VER(12, 71), STEP_A0, STEP_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..403f0d9caadf 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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_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 2a5bf50962ad..785453b2e95e 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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_B0, STEP_FOREVER) || > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_B0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_B0, STEP_FOREVER) || > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_B0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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..300b968e6de8 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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_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..3b159069dbc4 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_GFX_IPVER_STEP(engine->gt, IP_VER(12, 70), STEP_A0, STEP_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 f38c93d82c56..d03710c923c8 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -692,9 +692,11 @@ 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_GFX_IPVER_STEP(__gt, ipver, since, until) \ > > + (BUILD_BUG_ON_ZERO(ipver < IP_VER(2, 0)) + \ > > + (__gt->type != GT_MEDIA && \ > > + GRAPHICS_VER_FULL(__gt->i915) == ipver && \ > > + IS_GRAPHICS_STEP(__gt->i915, since, until))) > > #define IS_MTL_DISPLAY_STEP(__i915, since, until) \ > > (IS_METEORLAKE(__i915) && \
On Wed, Jul 19, 2023 at 08:28:12AM -0700, Matt Roper wrote: > On Wed, Jul 19, 2023 at 09:01:58AM +0100, Tvrtko Ursulin wrote: > > > > On 18/07/2023 23:27, 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_GFX_IPVER_STEP() macro instead > > > that is purely based on IP version. IS_GFX_IPVER_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. > > > > > > 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 | 8 +-- > > > 9 files changed, 46 insertions(+), 41 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..c13e64faaaad 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_GFX_IPVER_STEP(to_gt(i915), IP_VER(12, 70), STEP_A0, STEP_B0) || > > > + IS_GFX_IPVER_STEP(to_gt(i915), IP_VER(12, 71), STEP_A0, STEP_B0)) > > > > Do we really need/want a combined/long macro like this and not split it out > > to something like a more readable IS_GFX_IPVER && IS_<...>_STEP? On this part, wouldn't that make it less readable? Right now these conditions are an ||'d list of cases for which the workaround should apply (relating to various entries in the workaround database). If we split the version, stepping, and GT checks out into separate macros we'll get more complex conditions that are a mix of && and ||. We can try that, but my gut feeling is that it's going to be more error-prone since we'll have to be really careful to always get the parenentheses in the right places. Also many people (myself included) are already having some trouble adjusting to the new idea of GT (primary vs media) being an important factor that needs to be considered in the workaround condition; one of the goals with these new macros was to make sure that got included in the macro so that it couldn't be omitted by accident. Matt > > > > Adding Jani since we were discussing this direction over the past few weeks. > > > > Hm.. macro trickery for stepping ranges to make it more compact? Like making > > the STEP_ prefix be prepended by the macro. Don't know.. > > One other thing that we could consider if some of these things are > getting too long is eliminating IP_VER() and using the "versionx100" > scheme that Xe and Mesa use. On one hand that makes me a bit nervous > since GMD_ID's release number is 8 bits and could go past 100 on future > platforms, which would break the scheme (and we're already up to .71 on > MTL). But on the other hand it does make the code simpler and shorter > and we can always wait and see if we ever actually get a platform with > version X.100 or something before we really worry about what to do > there. > > The other point in favor of switching to versionx100 is that we wouldn't > have an inconsistent mix of both IP_VER() and verx100 in the Xe driver > where the display code comes from i915 but the non-display code is > separate. > > > Matt > > > > > Regards, > > > > Tvrtko > > > > > 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 23857cc08eca..c1af91d908e5 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->engine->i915, M, STEP_A0, STEP_B0) || > > > - IS_MTL_GRAPHICS_STEP(rq->engine->i915, P, STEP_A0, STEP_B0)) { > > > + if (IS_GFX_IPVER_STEP(rq->engine->gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > > > + IS_GFX_IPVER_STEP(rq->engine->gt, IP_VER(12, 71), STEP_A0, STEP_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->engine->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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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..41140eb86051 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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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..4fefa67d285f 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_GFX_IPVER_STEP(ce->engine->gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > > > + IS_GFX_IPVER_STEP(ce->engine->gt, IP_VER(12, 71), STEP_A0, STEP_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..403f0d9caadf 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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_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 2a5bf50962ad..785453b2e95e 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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_B0, STEP_FOREVER) || > > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_B0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_B0, STEP_FOREVER) || > > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_B0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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..300b968e6de8 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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_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..3b159069dbc4 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_GFX_IPVER_STEP(engine->gt, IP_VER(12, 70), STEP_A0, STEP_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 f38c93d82c56..d03710c923c8 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.h > > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > > @@ -692,9 +692,11 @@ 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_GFX_IPVER_STEP(__gt, ipver, since, until) \ > > > + (BUILD_BUG_ON_ZERO(ipver < IP_VER(2, 0)) + \ > > > + (__gt->type != GT_MEDIA && \ > > > + GRAPHICS_VER_FULL(__gt->i915) == ipver && \ > > > + IS_GRAPHICS_STEP(__gt->i915, since, until))) > > > #define IS_MTL_DISPLAY_STEP(__i915, since, until) \ > > > (IS_METEORLAKE(__i915) && \ > > -- > Matt Roper > Graphics Software Engineer > Linux GPU Platform Enablement > Intel Corporation
On 19/07/2023 16:28, Matt Roper wrote: > On Wed, Jul 19, 2023 at 09:01:58AM +0100, Tvrtko Ursulin wrote: >> >> On 18/07/2023 23:27, 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_GFX_IPVER_STEP() macro instead >>> that is purely based on IP version. IS_GFX_IPVER_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. >>> >>> 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 | 8 +-- >>> 9 files changed, 46 insertions(+), 41 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..c13e64faaaad 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_GFX_IPVER_STEP(to_gt(i915), IP_VER(12, 70), STEP_A0, STEP_B0) || >>> + IS_GFX_IPVER_STEP(to_gt(i915), IP_VER(12, 71), STEP_A0, STEP_B0)) >> >> Do we really need/want a combined/long macro like this and not split it out >> to something like a more readable IS_GFX_IPVER && IS_<...>_STEP? >> >> Adding Jani since we were discussing this direction over the past few weeks. >> >> Hm.. macro trickery for stepping ranges to make it more compact? Like making >> the STEP_ prefix be prepended by the macro. Don't know.. > > One other thing that we could consider if some of these things are > getting too long is eliminating IP_VER() and using the "versionx100" > scheme that Xe and Mesa use. On one hand that makes me a bit nervous > since GMD_ID's release number is 8 bits and could go past 100 on future > platforms, which would break the scheme (and we're already up to .71 on > MTL). But on the other hand it does make the code simpler and shorter > and we can always wait and see if we ever actually get a platform with > version X.100 or something before we really worry about what to do > there. > > The other point in favor of switching to versionx100 is that we wouldn't > have an inconsistent mix of both IP_VER() and verx100 in the Xe driver > where the display code comes from i915 but the non-display code is > separate. I don't think IP_VER is a problem for readability, the opposite. And it nicely enables do to stuff under the hood without churn. So maybe go IP_VER in Xe? Best to settle on these small details as much as possible before first release. Regards, Tvrtko > > > Matt > >> >> Regards, >> >> Tvrtko >> >>> 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 23857cc08eca..c1af91d908e5 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->engine->i915, M, STEP_A0, STEP_B0) || >>> - IS_MTL_GRAPHICS_STEP(rq->engine->i915, P, STEP_A0, STEP_B0)) { >>> + if (IS_GFX_IPVER_STEP(rq->engine->gt, IP_VER(12, 70), STEP_A0, STEP_B0) || >>> + IS_GFX_IPVER_STEP(rq->engine->gt, IP_VER(12, 71), STEP_A0, STEP_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->engine->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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || >>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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..41140eb86051 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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || >>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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..4fefa67d285f 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_GFX_IPVER_STEP(ce->engine->gt, IP_VER(12, 70), STEP_A0, STEP_B0) || >>> + IS_GFX_IPVER_STEP(ce->engine->gt, IP_VER(12, 71), STEP_A0, STEP_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..403f0d9caadf 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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_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 2a5bf50962ad..785453b2e95e 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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_B0, STEP_FOREVER) || >>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_B0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || >>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || >>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || >>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || >>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_B0, STEP_FOREVER) || >>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_B0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || >>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || >>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || >>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || >>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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..300b968e6de8 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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_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..3b159069dbc4 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_GFX_IPVER_STEP(engine->gt, IP_VER(12, 70), STEP_A0, STEP_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 f38c93d82c56..d03710c923c8 100644 >>> --- a/drivers/gpu/drm/i915/i915_drv.h >>> +++ b/drivers/gpu/drm/i915/i915_drv.h >>> @@ -692,9 +692,11 @@ 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_GFX_IPVER_STEP(__gt, ipver, since, until) \ >>> + (BUILD_BUG_ON_ZERO(ipver < IP_VER(2, 0)) + \ >>> + (__gt->type != GT_MEDIA && \ >>> + GRAPHICS_VER_FULL(__gt->i915) == ipver && \ >>> + IS_GRAPHICS_STEP(__gt->i915, since, until))) >>> #define IS_MTL_DISPLAY_STEP(__i915, since, until) \ >>> (IS_METEORLAKE(__i915) && \ >
On 19/07/2023 23:54, Matt Roper wrote: > On Wed, Jul 19, 2023 at 08:28:12AM -0700, Matt Roper wrote: >> On Wed, Jul 19, 2023 at 09:01:58AM +0100, Tvrtko Ursulin wrote: >>> >>> On 18/07/2023 23:27, 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_GFX_IPVER_STEP() macro instead >>>> that is purely based on IP version. IS_GFX_IPVER_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. >>>> >>>> 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 | 8 +-- >>>> 9 files changed, 46 insertions(+), 41 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..c13e64faaaad 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_GFX_IPVER_STEP(to_gt(i915), IP_VER(12, 70), STEP_A0, STEP_B0) || >>>> + IS_GFX_IPVER_STEP(to_gt(i915), IP_VER(12, 71), STEP_A0, STEP_B0)) >>> >>> Do we really need/want a combined/long macro like this and not split it out >>> to something like a more readable IS_GFX_IPVER && IS_<...>_STEP? > > On this part, wouldn't that make it less readable? Right now these > conditions are an ||'d list of cases for which the workaround should > apply (relating to various entries in the workaround database). If we > split the version, stepping, and GT checks out into separate macros > we'll get more complex conditions that are a mix of && and ||. We can > try that, but my gut feeling is that it's going to be more error-prone > since we'll have to be really careful to always get the parenentheses in > the right places. Also many people (myself included) are already having > some trouble adjusting to the new idea of GT (primary vs media) being an > important factor that needs to be considered in the workaround > condition; one of the goals with these new macros was to make sure that > got included in the macro so that it couldn't be omitted by accident. Oh well I don't know.. maybe. It's just that I find that as amount of all uppercase blocks grows over a certain limit things become quite an eyesore and hard to read. One line of: if (IS_GFX_IPVER_STEP(i915, IP_VER(12,70), STEP_A0, STEP_B0)) Is already borderline. A few of those on the same line is just hard to parse. But your argument also has merit so again, don't know. What I was suggesting is to maybe have: IS_GFX_IPVER_STEP(i915, IP_VER(12,70), IP_STEP(A0, B0)) Marginally shorter. Maybe find a shorted name for IS_GFX_IPVER_STEP? IS_GFX_IP(i915, IP_VER(12,70), IP_STEP(A0, B0)) ? Too bad we can't have that macro take the stepping argument as optional. Or maybe it is possible, I don't know C macro magic that well. Also, if we added GT based macros we could get away with encoding the GT type in the macro name. Asking the GT/tile directrly about their IP_VER. Not sure how widely applicable tha would be so just a thought. IS_IP(gt, IP_VER(12, 70)) GT_VER_FULL(gt) == IP_VER(13, 0) Unless that falls apart with that GT is not a tile story. Regards, Tvrtko > > > Matt > >>> >>> Adding Jani since we were discussing this direction over the past few weeks. >>> >>> Hm.. macro trickery for stepping ranges to make it more compact? Like making >>> the STEP_ prefix be prepended by the macro. Don't know.. >> >> One other thing that we could consider if some of these things are >> getting too long is eliminating IP_VER() and using the "versionx100" >> scheme that Xe and Mesa use. On one hand that makes me a bit nervous >> since GMD_ID's release number is 8 bits and could go past 100 on future >> platforms, which would break the scheme (and we're already up to .71 on >> MTL). But on the other hand it does make the code simpler and shorter >> and we can always wait and see if we ever actually get a platform with >> version X.100 or something before we really worry about what to do >> there. >> >> The other point in favor of switching to versionx100 is that we wouldn't >> have an inconsistent mix of both IP_VER() and verx100 in the Xe driver >> where the display code comes from i915 but the non-display code is >> separate. >> >> >> Matt >> >>> >>> Regards, >>> >>> Tvrtko >>> >>>> 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 23857cc08eca..c1af91d908e5 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->engine->i915, M, STEP_A0, STEP_B0) || >>>> - IS_MTL_GRAPHICS_STEP(rq->engine->i915, P, STEP_A0, STEP_B0)) { >>>> + if (IS_GFX_IPVER_STEP(rq->engine->gt, IP_VER(12, 70), STEP_A0, STEP_B0) || >>>> + IS_GFX_IPVER_STEP(rq->engine->gt, IP_VER(12, 71), STEP_A0, STEP_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->engine->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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || >>>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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..41140eb86051 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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || >>>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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..4fefa67d285f 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_GFX_IPVER_STEP(ce->engine->gt, IP_VER(12, 70), STEP_A0, STEP_B0) || >>>> + IS_GFX_IPVER_STEP(ce->engine->gt, IP_VER(12, 71), STEP_A0, STEP_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..403f0d9caadf 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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_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 2a5bf50962ad..785453b2e95e 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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_B0, STEP_FOREVER) || >>>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_B0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || >>>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || >>>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || >>>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || >>>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_B0, STEP_FOREVER) || >>>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_B0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || >>>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || >>>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || >>>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || >>>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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..300b968e6de8 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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_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..3b159069dbc4 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_GFX_IPVER_STEP(engine->gt, IP_VER(12, 70), STEP_A0, STEP_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 f38c93d82c56..d03710c923c8 100644 >>>> --- a/drivers/gpu/drm/i915/i915_drv.h >>>> +++ b/drivers/gpu/drm/i915/i915_drv.h >>>> @@ -692,9 +692,11 @@ 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_GFX_IPVER_STEP(__gt, ipver, since, until) \ >>>> + (BUILD_BUG_ON_ZERO(ipver < IP_VER(2, 0)) + \ >>>> + (__gt->type != GT_MEDIA && \ >>>> + GRAPHICS_VER_FULL(__gt->i915) == ipver && \ >>>> + IS_GRAPHICS_STEP(__gt->i915, since, until))) >>>> #define IS_MTL_DISPLAY_STEP(__i915, since, until) \ >>>> (IS_METEORLAKE(__i915) && \ >> >> -- >> Matt Roper >> Graphics Software Engineer >> Linux GPU Platform Enablement >> Intel Corporation >
On 20.07.2023 11:36, Tvrtko Ursulin wrote: > > On 19/07/2023 23:54, Matt Roper wrote: >> On Wed, Jul 19, 2023 at 08:28:12AM -0700, Matt Roper wrote: >>> On Wed, Jul 19, 2023 at 09:01:58AM +0100, Tvrtko Ursulin wrote: >>>> >>>> On 18/07/2023 23:27, 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_GFX_IPVER_STEP() macro instead >>>>> that is purely based on IP version. IS_GFX_IPVER_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. >>>>> >>>>> 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 | 8 +-- >>>>> 9 files changed, 46 insertions(+), 41 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..c13e64faaaad 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_GFX_IPVER_STEP(to_gt(i915), IP_VER(12, 70), STEP_A0, >>>>> STEP_B0) || >>>>> + IS_GFX_IPVER_STEP(to_gt(i915), IP_VER(12, 71), STEP_A0, >>>>> STEP_B0)) >>>> >>>> Do we really need/want a combined/long macro like this and not split >>>> it out >>>> to something like a more readable IS_GFX_IPVER && IS_<...>_STEP? >> >> On this part, wouldn't that make it less readable? Right now these >> conditions are an ||'d list of cases for which the workaround should >> apply (relating to various entries in the workaround database). If we >> split the version, stepping, and GT checks out into separate macros >> we'll get more complex conditions that are a mix of && and ||. We can >> try that, but my gut feeling is that it's going to be more error-prone >> since we'll have to be really careful to always get the parenentheses in >> the right places. Also many people (myself included) are already having >> some trouble adjusting to the new idea of GT (primary vs media) being an >> important factor that needs to be considered in the workaround >> condition; one of the goals with these new macros was to make sure that >> got included in the macro so that it couldn't be omitted by accident. > > Oh well I don't know.. maybe. It's just that I find that as amount of > all uppercase blocks grows over a certain limit things become quite an > eyesore and hard to read. One line of: > > if (IS_GFX_IPVER_STEP(i915, IP_VER(12,70), STEP_A0, STEP_B0)) > > Is already borderline. A few of those on the same line is just hard to > parse. But your argument also has merit so again, don't know. > > What I was suggesting is to maybe have: > > IS_GFX_IPVER_STEP(i915, IP_VER(12,70), IP_STEP(A0, B0)) > > Marginally shorter. > > Maybe find a shorted name for IS_GFX_IPVER_STEP? > > IS_GFX_IP(i915, IP_VER(12,70), IP_STEP(A0, B0)) > > ? > > Too bad we can't have that macro take the stepping argument as optional. > Or maybe it is possible, I don't know C macro magic that well. It is possible: #define IS_GFX_IP(i915, ver, args...) IS_GFX_IP_STEP(i915, ver, args, default_step) #define IS_GFX_IP_STEP(i915, ver, step, ...) definition_ignoring_elipsis Assuming IP_STEP is just single value. I am not sure about meaning of steppings in i915, it looks somehow special. If it is just another dot in version MAJOR.MINOR.STEP, why not extend IP_VER with stepping (ver << 16 & rel << 8 & step): IP_VER(12) IP_VER(12,70) IP_VER(12, 70, A0) then for example: IS_IP_VER(gt, IP_VER(12)) IS_IP_VER(gt, IP_VER(12), IP_VER(13)) IS_IP_REL(gt, IP_VER(12, 70)) IS_IP_REL(gt, IP_VER(12, 70), IP_VER(12, 71)) IS_IP_STEP(gt, IP_VER(12, 70, A0)) IS_IP_STEP(gt, IP_VER(12, 70, A0), IP_VER(12, 70, B0)) Regards Andrzej > > Also, if we added GT based macros we could get away with encoding the GT > type in the macro name. Asking the GT/tile directrly about their IP_VER. > Not sure how widely applicable tha would be so just a thought. > > IS_IP(gt, IP_VER(12, 70)) > > GT_VER_FULL(gt) == IP_VER(13, 0) > > Unless that falls apart with that GT is not a tile story. > > Regards, > > Tvrtko > >> >> >> Matt >> >>>> >>>> Adding Jani since we were discussing this direction over the past >>>> few weeks. >>>> >>>> Hm.. macro trickery for stepping ranges to make it more compact? >>>> Like making >>>> the STEP_ prefix be prepended by the macro. Don't know.. >>> >>> One other thing that we could consider if some of these things are >>> getting too long is eliminating IP_VER() and using the "versionx100" >>> scheme that Xe and Mesa use. On one hand that makes me a bit nervous >>> since GMD_ID's release number is 8 bits and could go past 100 on future >>> platforms, which would break the scheme (and we're already up to .71 on >>> MTL). But on the other hand it does make the code simpler and shorter >>> and we can always wait and see if we ever actually get a platform with >>> version X.100 or something before we really worry about what to do >>> there. >>> >>> The other point in favor of switching to versionx100 is that we wouldn't >>> have an inconsistent mix of both IP_VER() and verx100 in the Xe driver >>> where the display code comes from i915 but the non-display code is >>> separate. >>> >>> >>> Matt >>> >>>> >>>> Regards, >>>> >>>> Tvrtko >>>> >>>>> 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 23857cc08eca..c1af91d908e5 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->engine->i915, M, STEP_A0, >>>>> STEP_B0) || >>>>> - IS_MTL_GRAPHICS_STEP(rq->engine->i915, P, STEP_A0, >>>>> STEP_B0)) { >>>>> + if (IS_GFX_IPVER_STEP(rq->engine->gt, IP_VER(12, 70), STEP_A0, >>>>> STEP_B0) || >>>>> + IS_GFX_IPVER_STEP(rq->engine->gt, IP_VER(12, 71), STEP_A0, >>>>> STEP_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->engine->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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || >>>>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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..41140eb86051 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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, >>>>> STEP_B0) || >>>>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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..4fefa67d285f 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_GFX_IPVER_STEP(ce->engine->gt, IP_VER(12, 70), STEP_A0, >>>>> STEP_B0) || >>>>> + IS_GFX_IPVER_STEP(ce->engine->gt, IP_VER(12, 71), STEP_A0, >>>>> STEP_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..403f0d9caadf 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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_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 2a5bf50962ad..785453b2e95e 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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_B0, >>>>> STEP_FOREVER) || >>>>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_B0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || >>>>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || >>>>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || >>>>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || >>>>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_B0, >>>>> STEP_FOREVER) || >>>>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_B0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || >>>>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || >>>>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || >>>>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || >>>>> + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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..300b968e6de8 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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_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..3b159069dbc4 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_GFX_IPVER_STEP(engine->gt, IP_VER(12, 70), STEP_A0, >>>>> STEP_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 f38c93d82c56..d03710c923c8 100644 >>>>> --- a/drivers/gpu/drm/i915/i915_drv.h >>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h >>>>> @@ -692,9 +692,11 @@ 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_GFX_IPVER_STEP(__gt, ipver, since, until) \ >>>>> + (BUILD_BUG_ON_ZERO(ipver < IP_VER(2, 0)) + \ >>>>> + (__gt->type != GT_MEDIA && \ >>>>> + GRAPHICS_VER_FULL(__gt->i915) == ipver && \ >>>>> + IS_GRAPHICS_STEP(__gt->i915, since, until))) >>>>> #define IS_MTL_DISPLAY_STEP(__i915, since, until) \ >>>>> (IS_METEORLAKE(__i915) && \ >>> >>> -- >>> Matt Roper >>> Graphics Software Engineer >>> Linux GPU Platform Enablement >>> Intel Corporation >>
On Thu, Jul 20, 2023 at 03:10:21PM +0200, Andrzej Hajda wrote: > On 20.07.2023 11:36, Tvrtko Ursulin wrote: > > > > On 19/07/2023 23:54, Matt Roper wrote: > > > On Wed, Jul 19, 2023 at 08:28:12AM -0700, Matt Roper wrote: > > > > On Wed, Jul 19, 2023 at 09:01:58AM +0100, Tvrtko Ursulin wrote: > > > > > > > > > > On 18/07/2023 23:27, 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_GFX_IPVER_STEP() macro instead > > > > > > that is purely based on IP version. IS_GFX_IPVER_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. > > > > > > > > > > > > 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 | 8 +-- > > > > > > 9 files changed, 46 insertions(+), 41 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..c13e64faaaad 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_GFX_IPVER_STEP(to_gt(i915), IP_VER(12, 70), > > > > > > STEP_A0, STEP_B0) || > > > > > > + IS_GFX_IPVER_STEP(to_gt(i915), IP_VER(12, 71), > > > > > > STEP_A0, STEP_B0)) > > > > > > > > > > Do we really need/want a combined/long macro like this and > > > > > not split it out > > > > > to something like a more readable IS_GFX_IPVER && IS_<...>_STEP? > > > > > > On this part, wouldn't that make it less readable? Right now these > > > conditions are an ||'d list of cases for which the workaround should > > > apply (relating to various entries in the workaround database). If we > > > split the version, stepping, and GT checks out into separate macros > > > we'll get more complex conditions that are a mix of && and ||. We can > > > try that, but my gut feeling is that it's going to be more error-prone > > > since we'll have to be really careful to always get the parenentheses in > > > the right places. Also many people (myself included) are already having > > > some trouble adjusting to the new idea of GT (primary vs media) being an > > > important factor that needs to be considered in the workaround > > > condition; one of the goals with these new macros was to make sure that > > > got included in the macro so that it couldn't be omitted by accident. > > > > Oh well I don't know.. maybe. It's just that I find that as amount of > > all uppercase blocks grows over a certain limit things become quite an > > eyesore and hard to read. One line of: > > > > if (IS_GFX_IPVER_STEP(i915, IP_VER(12,70), STEP_A0, STEP_B0)) > > > > Is already borderline. A few of those on the same line is just hard to > > parse. But your argument also has merit so again, don't know. > > > > What I was suggesting is to maybe have: > > > > IS_GFX_IPVER_STEP(i915, IP_VER(12,70), IP_STEP(A0, B0)) > > > > Marginally shorter. > > > > Maybe find a shorted name for IS_GFX_IPVER_STEP? > > > > IS_GFX_IP(i915, IP_VER(12,70), IP_STEP(A0, B0)) > > > > ? > > > > Too bad we can't have that macro take the stepping argument as optional. > > Or maybe it is possible, I don't know C macro magic that well. > > It is possible: > > #define IS_GFX_IP(i915, ver, args...) IS_GFX_IP_STEP(i915, ver, args, > default_step) > #define IS_GFX_IP_STEP(i915, ver, step, ...) definition_ignoring_elipsis > > Assuming IP_STEP is just single value. > > I am not sure about meaning of steppings in i915, it looks somehow special. > If it is just another dot in version MAJOR.MINOR.STEP, why not extend IP_VER > with stepping (ver << 16 & rel << 8 & step): > IP_VER(12) > IP_VER(12,70) > IP_VER(12, 70, A0) > > then for example: > IS_IP_VER(gt, IP_VER(12)) > IS_IP_VER(gt, IP_VER(12), IP_VER(13)) > IS_IP_REL(gt, IP_VER(12, 70)) > IS_IP_REL(gt, IP_VER(12, 70), IP_VER(12, 71)) > IS_IP_STEP(gt, IP_VER(12, 70, A0)) > IS_IP_STEP(gt, IP_VER(12, 70, A0), IP_VER(12, 70, B0)) I'm not sure it's a good idea to include stepping in the general IP_VER value because the version and stepping information are used in very different ways in the driver. - If we're checking a range of versions, it's an inclusive range of IP versions that the behavior applies to. - If we're checking a range of steppings, it's always within one specific IP version (it's not possible to compare steppings in a sensible way cross-version) and the comparison is expected to match an entry in the workaround database. Because the workaround database is based on "hardware fix" version numbers, the range always has an exclusive upper bound (since that's the "fix version" at which the workaround should no longer be applied. Even if we use CPP trickery to create macros that take default values, I don't see how we'd use that effectively since the appropriate default value would change depending on which type of comparison we were doing (and also whether it was an upper or lower bound for a range). Matt > > Regards > Andrzej > > > > > > Also, if we added GT based macros we could get away with encoding the GT > > type in the macro name. Asking the GT/tile directrly about their IP_VER. > > Not sure how widely applicable tha would be so just a thought. > > > > IS_IP(gt, IP_VER(12, 70)) > > > > GT_VER_FULL(gt) == IP_VER(13, 0) > > > > Unless that falls apart with that GT is not a tile story. > > > > Regards, > > > > Tvrtko > > > > > > > > > > > Matt > > > > > > > > > > > > > Adding Jani since we were discussing this direction over the > > > > > past few weeks. > > > > > > > > > > Hm.. macro trickery for stepping ranges to make it more > > > > > compact? Like making > > > > > the STEP_ prefix be prepended by the macro. Don't know.. > > > > > > > > One other thing that we could consider if some of these things are > > > > getting too long is eliminating IP_VER() and using the "versionx100" > > > > scheme that Xe and Mesa use. On one hand that makes me a bit nervous > > > > since GMD_ID's release number is 8 bits and could go past 100 on future > > > > platforms, which would break the scheme (and we're already up to .71 on > > > > MTL). But on the other hand it does make the code simpler and shorter > > > > and we can always wait and see if we ever actually get a platform with > > > > version X.100 or something before we really worry about what to do > > > > there. > > > > > > > > The other point in favor of switching to versionx100 is that we wouldn't > > > > have an inconsistent mix of both IP_VER() and verx100 in the Xe driver > > > > where the display code comes from i915 but the non-display code is > > > > separate. > > > > > > > > > > > > Matt > > > > > > > > > > > > > > Regards, > > > > > > > > > > Tvrtko > > > > > > > > > > > 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 23857cc08eca..c1af91d908e5 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->engine->i915, M, > > > > > > STEP_A0, STEP_B0) || > > > > > > - IS_MTL_GRAPHICS_STEP(rq->engine->i915, P, > > > > > > STEP_A0, STEP_B0)) { > > > > > > + if (IS_GFX_IPVER_STEP(rq->engine->gt, IP_VER(12, > > > > > > 70), STEP_A0, STEP_B0) || > > > > > > + IS_GFX_IPVER_STEP(rq->engine->gt, IP_VER(12, > > > > > > 71), STEP_A0, STEP_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->engine->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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > > > > > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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..41140eb86051 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_GFX_IPVER_STEP(gt, IP_VER(12, 70), > > > > > > STEP_A0, STEP_B0) || > > > > > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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..4fefa67d285f 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_GFX_IPVER_STEP(ce->engine->gt, IP_VER(12, > > > > > > 70), STEP_A0, STEP_B0) || > > > > > > + IS_GFX_IPVER_STEP(ce->engine->gt, IP_VER(12, > > > > > > 71), STEP_A0, STEP_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..403f0d9caadf 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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_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 2a5bf50962ad..785453b2e95e 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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_B0, > > > > > > STEP_FOREVER) || > > > > > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_B0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > > > > > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > > > > > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > > > > > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > > > > > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_B0, > > > > > > STEP_FOREVER) || > > > > > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_B0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > > > > > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > > > > > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > > > > > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || > > > > > > + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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..300b968e6de8 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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_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..3b159069dbc4 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_GFX_IPVER_STEP(engine->gt, IP_VER(12, > > > > > > 70), STEP_A0, STEP_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 f38c93d82c56..d03710c923c8 100644 > > > > > > --- a/drivers/gpu/drm/i915/i915_drv.h > > > > > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > > > > > @@ -692,9 +692,11 @@ 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_GFX_IPVER_STEP(__gt, ipver, since, until) \ > > > > > > + (BUILD_BUG_ON_ZERO(ipver < IP_VER(2, 0)) + \ > > > > > > + (__gt->type != GT_MEDIA && \ > > > > > > + GRAPHICS_VER_FULL(__gt->i915) == ipver && \ > > > > > > + IS_GRAPHICS_STEP(__gt->i915, since, until))) > > > > > > #define IS_MTL_DISPLAY_STEP(__i915, since, until) \ > > > > > > (IS_METEORLAKE(__i915) && \ > > > > > > > > -- > > > > Matt Roper > > > > Graphics Software Engineer > > > > Linux GPU Platform Enablement > > > > Intel Corporation > > > >
diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c index 6b01a0b68b97..c13e64faaaad 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_GFX_IPVER_STEP(to_gt(i915), IP_VER(12, 70), STEP_A0, STEP_B0) || + IS_GFX_IPVER_STEP(to_gt(i915), IP_VER(12, 71), STEP_A0, STEP_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 23857cc08eca..c1af91d908e5 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->engine->i915, M, STEP_A0, STEP_B0) || - IS_MTL_GRAPHICS_STEP(rq->engine->i915, P, STEP_A0, STEP_B0)) { + if (IS_GFX_IPVER_STEP(rq->engine->gt, IP_VER(12, 70), STEP_A0, STEP_B0) || + IS_GFX_IPVER_STEP(rq->engine->gt, IP_VER(12, 71), STEP_A0, STEP_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->engine->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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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..41140eb86051 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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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..4fefa67d285f 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_GFX_IPVER_STEP(ce->engine->gt, IP_VER(12, 70), STEP_A0, STEP_B0) || + IS_GFX_IPVER_STEP(ce->engine->gt, IP_VER(12, 71), STEP_A0, STEP_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..403f0d9caadf 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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_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 2a5bf50962ad..785453b2e95e 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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_B0, STEP_FOREVER) || + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_B0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_B0, STEP_FOREVER) || + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_B0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_B0) || + IS_GFX_IPVER_STEP(gt, IP_VER(12, 71), STEP_A0, STEP_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..300b968e6de8 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_GFX_IPVER_STEP(gt, IP_VER(12, 70), STEP_A0, STEP_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..3b159069dbc4 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_GFX_IPVER_STEP(engine->gt, IP_VER(12, 70), STEP_A0, STEP_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 f38c93d82c56..d03710c923c8 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -692,9 +692,11 @@ 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_GFX_IPVER_STEP(__gt, ipver, since, until) \ + (BUILD_BUG_ON_ZERO(ipver < IP_VER(2, 0)) + \ + (__gt->type != GT_MEDIA && \ + GRAPHICS_VER_FULL(__gt->i915) == ipver && \ + IS_GRAPHICS_STEP(__gt->i915, since, until))) #define IS_MTL_DISPLAY_STEP(__i915, since, until) \ (IS_METEORLAKE(__i915) && \
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_GFX_IPVER_STEP() macro instead that is purely based on IP version. IS_GFX_IPVER_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. 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 | 8 +-- 9 files changed, 46 insertions(+), 41 deletions(-)