| Message ID | 20230118155249.41551-3-gustavo.sousa@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/gt: Move LSC_CHICKEN_BIT* workarounds to correct function (Series) | expand |
On Wed, Jan 18, 2023 at 12:52:49PM -0300, Gustavo Sousa wrote: > That register doesn't belong to a specific engine, so the proper > placement for workarounds programming it should be > general_render_compute_wa_init(). > > Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > --- > drivers/gpu/drm/i915/gt/intel_workarounds.c | 65 ++++++++++++--------- > 1 file changed, 36 insertions(+), 29 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c > index ef6065ce8267..918a271447e2 100644 > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c > @@ -2341,10 +2341,6 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal) > /* Wa_1509727124 */ > wa_mcr_masked_en(wal, GEN10_SAMPLER_MODE, > SC_DISABLE_POWER_OPTIMIZATION_EBB); > - > - /* Wa_22013037850 */ > - wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0_UDW, > - DISABLE_128B_EVICTION_COMMAND_UDW); > } > > if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_FOREVER) || > @@ -2373,21 +2369,6 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal) > GEN12_DISABLE_HDR_PAST_PAYLOAD_HOLD_FIX); > } > > - if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_C0) || > - IS_DG2_G11(i915)) { > - /* > - * Wa_22012826095:dg2 > - * Wa_22013059131:dg2 > - */ > - wa_mcr_write_clr_set(wal, LSC_CHICKEN_BIT_0_UDW, > - MAXREQS_PER_BANK, > - REG_FIELD_PREP(MAXREQS_PER_BANK, 2)); > - > - /* Wa_22013059131:dg2 */ > - wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0, > - FORCE_1_SUB_MESSAGE_PER_FRAGMENT); > - } > - > /* Wa_1308578152:dg2_g10 when first gslice is fused off */ > if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_C0) && > needs_wa_1308578152(engine)) { > @@ -2412,16 +2393,6 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal) > */ > wa_mcr_masked_en(wal, GEN8_ROW_CHICKEN, > MDQ_ARBITRATION_MODE | UGM_BACKUP_MODE); > - > - /* > - * Wa_14010918519:dg2_g10 > - * > - * LSC_CHICKEN_BIT_0 always reads back as 0 is this stepping, > - * so ignoring verification. > - */ > - wa_mcr_add(wal, LSC_CHICKEN_BIT_0_UDW, 0, > - FORCE_SLM_FENCE_SCOPE_TO_TILE | FORCE_UGM_FENCE_SCOPE_TO_TILE, > - 0, false); > } > > if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_A0, STEP_B0)) { > @@ -3006,6 +2977,15 @@ general_render_compute_wa_init(struct intel_engine_cs *engine, struct i915_wa_li > > add_render_compute_tuning_settings(i915, wal); > > + if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || > + IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0) || > + IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_FOREVER) || > + IS_DG2_G11(i915) || IS_DG2_G12(i915)) { > + /* Wa_22013037850 */ > + wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0_UDW, > + 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) || > IS_PONTEVECCHIO(i915) || > @@ -3027,6 +3007,33 @@ general_render_compute_wa_init(struct intel_engine_cs *engine, struct i915_wa_li > wa_masked_en(wal, VFG_PREEMPTION_CHICKEN, POLYGON_TRIFAN_LINELOOP_DISABLE); > } > > + if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_C0) || > + IS_DG2_G11(i915)) { > + /* > + * Wa_22012826095:dg2 > + * Wa_22013059131:dg2 > + */ > + wa_mcr_write_clr_set(wal, LSC_CHICKEN_BIT_0_UDW, > + MAXREQS_PER_BANK, > + REG_FIELD_PREP(MAXREQS_PER_BANK, 2)); > + > + /* Wa_22013059131:dg2 */ > + wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0, > + FORCE_1_SUB_MESSAGE_PER_FRAGMENT); > + } > + > + if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_A0, STEP_B0)) { > + /* > + * Wa_14010918519:dg2_g10 > + * > + * LSC_CHICKEN_BIT_0 always reads back as 0 is this stepping, > + * so ignoring verification. > + */ > + wa_mcr_add(wal, LSC_CHICKEN_BIT_0_UDW, 0, > + FORCE_SLM_FENCE_SCOPE_TO_TILE | FORCE_UGM_FENCE_SCOPE_TO_TILE, > + 0, false); > + } > + > if (IS_PONTEVECCHIO(i915)) { > /* Wa_16016694945 */ > wa_masked_en(wal, XEHPC_LNCFMISCCFGREG0, XEHPC_OVRLSCCC); > -- > 2.39.0 >
On Thu, Jan 19, 2023 at 04:57:09PM -0500, Rodrigo Vivi wrote: > On Wed, Jan 18, 2023 at 12:52:49PM -0300, Gustavo Sousa wrote: > > That register doesn't belong to a specific engine, so the proper > > placement for workarounds programming it should be > > general_render_compute_wa_init(). > > > > Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com> > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> Thanks for the review, Rodrigo! Last time I talked with the team, Lucas suspected there could be a reason why the workarounds were in their current places. I'll include him and Matt Roper here, since they had assigned themselves to check this. I think we should wait for their input before applying this patch. > > > --- > > drivers/gpu/drm/i915/gt/intel_workarounds.c | 65 ++++++++++++--------- > > 1 file changed, 36 insertions(+), 29 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c > > index ef6065ce8267..918a271447e2 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c > > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c > > @@ -2341,10 +2341,6 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal) > > /* Wa_1509727124 */ > > wa_mcr_masked_en(wal, GEN10_SAMPLER_MODE, > > SC_DISABLE_POWER_OPTIMIZATION_EBB); > > - > > - /* Wa_22013037850 */ > > - wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0_UDW, > > - DISABLE_128B_EVICTION_COMMAND_UDW); > > } > > > > if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_FOREVER) || > > @@ -2373,21 +2369,6 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal) > > GEN12_DISABLE_HDR_PAST_PAYLOAD_HOLD_FIX); > > } > > > > - if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_C0) || > > - IS_DG2_G11(i915)) { > > - /* > > - * Wa_22012826095:dg2 > > - * Wa_22013059131:dg2 > > - */ > > - wa_mcr_write_clr_set(wal, LSC_CHICKEN_BIT_0_UDW, > > - MAXREQS_PER_BANK, > > - REG_FIELD_PREP(MAXREQS_PER_BANK, 2)); > > - > > - /* Wa_22013059131:dg2 */ > > - wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0, > > - FORCE_1_SUB_MESSAGE_PER_FRAGMENT); > > - } > > - > > /* Wa_1308578152:dg2_g10 when first gslice is fused off */ > > if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_C0) && > > needs_wa_1308578152(engine)) { > > @@ -2412,16 +2393,6 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal) > > */ > > wa_mcr_masked_en(wal, GEN8_ROW_CHICKEN, > > MDQ_ARBITRATION_MODE | UGM_BACKUP_MODE); > > - > > - /* > > - * Wa_14010918519:dg2_g10 > > - * > > - * LSC_CHICKEN_BIT_0 always reads back as 0 is this stepping, > > - * so ignoring verification. > > - */ > > - wa_mcr_add(wal, LSC_CHICKEN_BIT_0_UDW, 0, > > - FORCE_SLM_FENCE_SCOPE_TO_TILE | FORCE_UGM_FENCE_SCOPE_TO_TILE, > > - 0, false); > > } > > > > if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_A0, STEP_B0)) { > > @@ -3006,6 +2977,15 @@ general_render_compute_wa_init(struct intel_engine_cs *engine, struct i915_wa_li > > > > add_render_compute_tuning_settings(i915, wal); > > > > + if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || > > + IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0) || > > + IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_FOREVER) || > > + IS_DG2_G11(i915) || IS_DG2_G12(i915)) { > > + /* Wa_22013037850 */ > > + wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0_UDW, > > + 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) || > > IS_PONTEVECCHIO(i915) || > > @@ -3027,6 +3007,33 @@ general_render_compute_wa_init(struct intel_engine_cs *engine, struct i915_wa_li > > wa_masked_en(wal, VFG_PREEMPTION_CHICKEN, POLYGON_TRIFAN_LINELOOP_DISABLE); > > } > > > > + if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_C0) || > > + IS_DG2_G11(i915)) { > > + /* > > + * Wa_22012826095:dg2 > > + * Wa_22013059131:dg2 > > + */ > > + wa_mcr_write_clr_set(wal, LSC_CHICKEN_BIT_0_UDW, > > + MAXREQS_PER_BANK, > > + REG_FIELD_PREP(MAXREQS_PER_BANK, 2)); > > + > > + /* Wa_22013059131:dg2 */ > > + wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0, > > + FORCE_1_SUB_MESSAGE_PER_FRAGMENT); > > + } > > + > > + if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_A0, STEP_B0)) { > > + /* > > + * Wa_14010918519:dg2_g10 > > + * > > + * LSC_CHICKEN_BIT_0 always reads back as 0 is this stepping, > > + * so ignoring verification. > > + */ > > + wa_mcr_add(wal, LSC_CHICKEN_BIT_0_UDW, 0, > > + FORCE_SLM_FENCE_SCOPE_TO_TILE | FORCE_UGM_FENCE_SCOPE_TO_TILE, > > + 0, false); > > + } > > + > > if (IS_PONTEVECCHIO(i915)) { > > /* Wa_16016694945 */ > > wa_masked_en(wal, XEHPC_LNCFMISCCFGREG0, XEHPC_OVRLSCCC); > > -- > > 2.39.0 > >
On Thu, 2023-01-19 at 19:24 -0300, Gustavo Sousa wrote: > On Thu, Jan 19, 2023 at 04:57:09PM -0500, Rodrigo Vivi wrote: > > On Wed, Jan 18, 2023 at 12:52:49PM -0300, Gustavo Sousa wrote: > > > That register doesn't belong to a specific engine, so the proper > > > placement for workarounds programming it should be > > > general_render_compute_wa_init(). > > > > > > Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com> > > > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > Thanks for the review, Rodrigo! > > Last time I talked with the team, Lucas suspected there could be a > reason why > the workarounds were in their current places. I'll include him and > Matt Roper > here, since they had assigned themselves to check this. > > I think we should wait for their input before applying this patch. ops, I have just pushed them... > > > > > > --- > > > drivers/gpu/drm/i915/gt/intel_workarounds.c | 65 ++++++++++++--- > > > ------ > > > 1 file changed, 36 insertions(+), 29 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c > > > b/drivers/gpu/drm/i915/gt/intel_workarounds.c > > > index ef6065ce8267..918a271447e2 100644 > > > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c > > > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c > > > @@ -2341,10 +2341,6 @@ rcs_engine_wa_init(struct intel_engine_cs > > > *engine, struct i915_wa_list *wal) > > > /* Wa_1509727124 */ > > > wa_mcr_masked_en(wal, GEN10_SAMPLER_MODE, > > > > > > SC_DISABLE_POWER_OPTIMIZATION_EBB); > > > - > > > - /* Wa_22013037850 */ > > > - wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0_UDW, > > > - > > > DISABLE_128B_EVICTION_COMMAND_UDW); > > > } > > > > > > if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, > > > STEP_FOREVER) || > > > @@ -2373,21 +2369,6 @@ rcs_engine_wa_init(struct intel_engine_cs > > > *engine, struct i915_wa_list *wal) > > > > > > GEN12_DISABLE_HDR_PAST_PAYLOAD_HOLD_FIX); > > > } > > > > > > - if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_C0) || > > > - IS_DG2_G11(i915)) { > > > - /* > > > - * Wa_22012826095:dg2 > > > - * Wa_22013059131:dg2 > > > - */ > > > - wa_mcr_write_clr_set(wal, LSC_CHICKEN_BIT_0_UDW, > > > - MAXREQS_PER_BANK, > > > - > > > REG_FIELD_PREP(MAXREQS_PER_BANK, 2)); > > > - > > > - /* Wa_22013059131:dg2 */ > > > - wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0, > > > - > > > FORCE_1_SUB_MESSAGE_PER_FRAGMENT); > > > - } > > > - > > > /* Wa_1308578152:dg2_g10 when first gslice is fused off > > > */ > > > if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_C0) && > > > needs_wa_1308578152(engine)) { > > > @@ -2412,16 +2393,6 @@ rcs_engine_wa_init(struct intel_engine_cs > > > *engine, struct i915_wa_list *wal) > > > */ > > > wa_mcr_masked_en(wal, GEN8_ROW_CHICKEN, > > > MDQ_ARBITRATION_MODE | > > > UGM_BACKUP_MODE); > > > - > > > - /* > > > - * Wa_14010918519:dg2_g10 > > > - * > > > - * LSC_CHICKEN_BIT_0 always reads back as 0 is > > > this stepping, > > > - * so ignoring verification. > > > - */ > > > - wa_mcr_add(wal, LSC_CHICKEN_BIT_0_UDW, 0, > > > - FORCE_SLM_FENCE_SCOPE_TO_TILE | > > > FORCE_UGM_FENCE_SCOPE_TO_TILE, > > > - 0, false); > > > } > > > > > > if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_A0, STEP_B0)) { > > > @@ -3006,6 +2977,15 @@ general_render_compute_wa_init(struct > > > intel_engine_cs *engine, struct i915_wa_li > > > > > > add_render_compute_tuning_settings(i915, wal); > > > > > > + if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || > > > + IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0) || > > > + IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, > > > STEP_FOREVER) || > > > + IS_DG2_G11(i915) || IS_DG2_G12(i915)) { > > > + /* Wa_22013037850 */ > > > + wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0_UDW, > > > + 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) || > > > IS_PONTEVECCHIO(i915) || > > > @@ -3027,6 +3007,33 @@ general_render_compute_wa_init(struct > > > intel_engine_cs *engine, struct i915_wa_li > > > wa_masked_en(wal, VFG_PREEMPTION_CHICKEN, > > > POLYGON_TRIFAN_LINELOOP_DISABLE); > > > } > > > > > > + if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_C0) || > > > + IS_DG2_G11(i915)) { > > > + /* > > > + * Wa_22012826095:dg2 > > > + * Wa_22013059131:dg2 > > > + */ > > > + wa_mcr_write_clr_set(wal, LSC_CHICKEN_BIT_0_UDW, > > > + MAXREQS_PER_BANK, > > > + > > > REG_FIELD_PREP(MAXREQS_PER_BANK, 2)); > > > + > > > + /* Wa_22013059131:dg2 */ > > > + wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0, > > > + FORCE_1_SUB_MESSAGE_PER_FRAGMENT) > > > ; > > > + } > > > + > > > + if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_A0, STEP_B0)) { > > > + /* > > > + * Wa_14010918519:dg2_g10 > > > + * > > > + * LSC_CHICKEN_BIT_0 always reads back as 0 is > > > this stepping, > > > + * so ignoring verification. > > > + */ > > > + wa_mcr_add(wal, LSC_CHICKEN_BIT_0_UDW, 0, > > > + FORCE_SLM_FENCE_SCOPE_TO_TILE | > > > FORCE_UGM_FENCE_SCOPE_TO_TILE, > > > + 0, false); > > > + } > > > + > > > if (IS_PONTEVECCHIO(i915)) { > > > /* Wa_16016694945 */ > > > wa_masked_en(wal, XEHPC_LNCFMISCCFGREG0, > > > XEHPC_OVRLSCCC); > > > -- > > > 2.39.0 > > >
On Thu, Jan 19, 2023 at 02:28:13PM -0800, Vivi, Rodrigo wrote: > On Thu, 2023-01-19 at 19:24 -0300, Gustavo Sousa wrote: > > On Thu, Jan 19, 2023 at 04:57:09PM -0500, Rodrigo Vivi wrote: > > > On Wed, Jan 18, 2023 at 12:52:49PM -0300, Gustavo Sousa wrote: > > > > That register doesn't belong to a specific engine, so the proper > > > > placement for workarounds programming it should be > > > > general_render_compute_wa_init(). > > > > > > > > Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com> > > > > > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > > Thanks for the review, Rodrigo! > > > > Last time I talked with the team, Lucas suspected there could be a > > reason why > > the workarounds were in their current places. I'll include him and > > Matt Roper > > here, since they had assigned themselves to check this. > > > > I think we should wait for their input before applying this patch. > > ops, I have just pushed them... I didn't review each workaround in detail, but at a high level the patch below looks correct to me. The intention was always to go back and find all the workarounds touching non-RCS, non-CCS registers that are part of the shared render/compute domain and move them to this general_render_compute_wa_init() function. The effort just stalled out because the developer working on it left and nobody else has had time to pick it up yet; it's been on our todo list for a while. Moving these workarounds from rcs_engine_wa_init() to general_render_compute_wa_init() would become very important if a SKU of these platforms ever shows up with a fused-off render engine. In that case the workarounds would get missed (due to lack of RCS), which would negatively impact the behavior of the remaining CCS engines. However that's just a theoretical problem today; in practice all DG2 and MTL platforms have an RCS engine, so the code movement below will not cause any functional change. But it's still good to have Gustavo finally cleaning this up because we never know what the future holds. Matt > > > > > > > > > > --- > > > > drivers/gpu/drm/i915/gt/intel_workarounds.c | 65 ++++++++++++--- > > > > ------ > > > > 1 file changed, 36 insertions(+), 29 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c > > > > b/drivers/gpu/drm/i915/gt/intel_workarounds.c > > > > index ef6065ce8267..918a271447e2 100644 > > > > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c > > > > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c > > > > @@ -2341,10 +2341,6 @@ rcs_engine_wa_init(struct intel_engine_cs > > > > *engine, struct i915_wa_list *wal) > > > > /* Wa_1509727124 */ > > > > wa_mcr_masked_en(wal, GEN10_SAMPLER_MODE, > > > > > > > > SC_DISABLE_POWER_OPTIMIZATION_EBB); > > > > - > > > > - /* Wa_22013037850 */ > > > > - wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0_UDW, > > > > - > > > > DISABLE_128B_EVICTION_COMMAND_UDW); > > > > } > > > > > > > > if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, > > > > STEP_FOREVER) || > > > > @@ -2373,21 +2369,6 @@ rcs_engine_wa_init(struct intel_engine_cs > > > > *engine, struct i915_wa_list *wal) > > > > > > > > GEN12_DISABLE_HDR_PAST_PAYLOAD_HOLD_FIX); > > > > } > > > > > > > > - if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_C0) || > > > > - IS_DG2_G11(i915)) { > > > > - /* > > > > - * Wa_22012826095:dg2 > > > > - * Wa_22013059131:dg2 > > > > - */ > > > > - wa_mcr_write_clr_set(wal, LSC_CHICKEN_BIT_0_UDW, > > > > - MAXREQS_PER_BANK, > > > > - > > > > REG_FIELD_PREP(MAXREQS_PER_BANK, 2)); > > > > - > > > > - /* Wa_22013059131:dg2 */ > > > > - wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0, > > > > - > > > > FORCE_1_SUB_MESSAGE_PER_FRAGMENT); > > > > - } > > > > - > > > > /* Wa_1308578152:dg2_g10 when first gslice is fused off > > > > */ > > > > if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_C0) && > > > > needs_wa_1308578152(engine)) { > > > > @@ -2412,16 +2393,6 @@ rcs_engine_wa_init(struct intel_engine_cs > > > > *engine, struct i915_wa_list *wal) > > > > */ > > > > wa_mcr_masked_en(wal, GEN8_ROW_CHICKEN, > > > > MDQ_ARBITRATION_MODE | > > > > UGM_BACKUP_MODE); > > > > - > > > > - /* > > > > - * Wa_14010918519:dg2_g10 > > > > - * > > > > - * LSC_CHICKEN_BIT_0 always reads back as 0 is > > > > this stepping, > > > > - * so ignoring verification. > > > > - */ > > > > - wa_mcr_add(wal, LSC_CHICKEN_BIT_0_UDW, 0, > > > > - FORCE_SLM_FENCE_SCOPE_TO_TILE | > > > > FORCE_UGM_FENCE_SCOPE_TO_TILE, > > > > - 0, false); > > > > } > > > > > > > > if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_A0, STEP_B0)) { > > > > @@ -3006,6 +2977,15 @@ general_render_compute_wa_init(struct > > > > intel_engine_cs *engine, struct i915_wa_li > > > > > > > > add_render_compute_tuning_settings(i915, wal); > > > > > > > > + if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || > > > > + IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0) || > > > > + IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, > > > > STEP_FOREVER) || > > > > + IS_DG2_G11(i915) || IS_DG2_G12(i915)) { > > > > + /* Wa_22013037850 */ > > > > + wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0_UDW, > > > > + 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) || > > > > IS_PONTEVECCHIO(i915) || > > > > @@ -3027,6 +3007,33 @@ general_render_compute_wa_init(struct > > > > intel_engine_cs *engine, struct i915_wa_li > > > > wa_masked_en(wal, VFG_PREEMPTION_CHICKEN, > > > > POLYGON_TRIFAN_LINELOOP_DISABLE); > > > > } > > > > > > > > + if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_C0) || > > > > + IS_DG2_G11(i915)) { > > > > + /* > > > > + * Wa_22012826095:dg2 > > > > + * Wa_22013059131:dg2 > > > > + */ > > > > + wa_mcr_write_clr_set(wal, LSC_CHICKEN_BIT_0_UDW, > > > > + MAXREQS_PER_BANK, > > > > + > > > > REG_FIELD_PREP(MAXREQS_PER_BANK, 2)); > > > > + > > > > + /* Wa_22013059131:dg2 */ > > > > + wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0, > > > > + FORCE_1_SUB_MESSAGE_PER_FRAGMENT) > > > > ; > > > > + } > > > > + > > > > + if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_A0, STEP_B0)) { > > > > + /* > > > > + * Wa_14010918519:dg2_g10 > > > > + * > > > > + * LSC_CHICKEN_BIT_0 always reads back as 0 is > > > > this stepping, > > > > + * so ignoring verification. > > > > + */ > > > > + wa_mcr_add(wal, LSC_CHICKEN_BIT_0_UDW, 0, > > > > + FORCE_SLM_FENCE_SCOPE_TO_TILE | > > > > FORCE_UGM_FENCE_SCOPE_TO_TILE, > > > > + 0, false); > > > > + } > > > > + > > > > if (IS_PONTEVECCHIO(i915)) { > > > > /* Wa_16016694945 */ > > > > wa_masked_en(wal, XEHPC_LNCFMISCCFGREG0, > > > > XEHPC_OVRLSCCC); > > > > -- > > > > 2.39.0 > > > > >
On Thu, Jan 19, 2023 at 03:03:30PM -0800, Matt Roper wrote: >On Thu, Jan 19, 2023 at 02:28:13PM -0800, Vivi, Rodrigo wrote: >> On Thu, 2023-01-19 at 19:24 -0300, Gustavo Sousa wrote: >> > On Thu, Jan 19, 2023 at 04:57:09PM -0500, Rodrigo Vivi wrote: >> > > On Wed, Jan 18, 2023 at 12:52:49PM -0300, Gustavo Sousa wrote: >> > > > That register doesn't belong to a specific engine, so the proper >> > > > placement for workarounds programming it should be >> > > > general_render_compute_wa_init(). >> > > > >> > > > Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com> >> > > >> > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> >> > >> > Thanks for the review, Rodrigo! >> > >> > Last time I talked with the team, Lucas suspected there could be a >> > reason why >> > the workarounds were in their current places. I'll include him and >> > Matt Roper >> > here, since they had assigned themselves to check this. >> > >> > I think we should wait for their input before applying this patch. >> >> ops, I have just pushed them... > >I didn't review each workaround in detail, but at a high level the patch >below looks correct to me. The intention was always to go back and find >all the workarounds touching non-RCS, non-CCS registers that are part of >the shared render/compute domain and move them to this >general_render_compute_wa_init() function. The effort just stalled out >because the developer working on it left and nobody else has had time to >pick it up yet; it's been on our todo list for a while. > >Moving these workarounds from rcs_engine_wa_init() to >general_render_compute_wa_init() would become very important if a SKU of >these platforms ever shows up with a fused-off render engine. In that >case the workarounds would get missed (due to lack of RCS), which would >negatively impact the behavior of the remaining CCS engines. However >that's just a theoretical problem today; in practice all DG2 and MTL >platforms have an RCS engine, so the code movement below will not cause >any functional change. But it's still good to have Gustavo finally >cleaning this up because we never know what the future holds. it may be that my fuzzy memory about this is that we didn't have yet the general_render_compute_* function to add this kind of WA to, so some of the workarounds got added to the engine because these registers reset when either compute or render reset. So, yes... I agree this should be fine. Lucas De Marchi > > >Matt > >> >> > >> > > >> > > > --- >> > > > drivers/gpu/drm/i915/gt/intel_workarounds.c | 65 ++++++++++++--- >> > > > ------ >> > > > 1 file changed, 36 insertions(+), 29 deletions(-) >> > > > >> > > > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c >> > > > b/drivers/gpu/drm/i915/gt/intel_workarounds.c >> > > > index ef6065ce8267..918a271447e2 100644 >> > > > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c >> > > > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c >> > > > @@ -2341,10 +2341,6 @@ rcs_engine_wa_init(struct intel_engine_cs >> > > > *engine, struct i915_wa_list *wal) >> > > > /* Wa_1509727124 */ >> > > > wa_mcr_masked_en(wal, GEN10_SAMPLER_MODE, >> > > > >> > > > SC_DISABLE_POWER_OPTIMIZATION_EBB); >> > > > - >> > > > - /* Wa_22013037850 */ >> > > > - wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0_UDW, >> > > > - >> > > > DISABLE_128B_EVICTION_COMMAND_UDW); >> > > > } >> > > > >> > > > if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, >> > > > STEP_FOREVER) || >> > > > @@ -2373,21 +2369,6 @@ rcs_engine_wa_init(struct intel_engine_cs >> > > > *engine, struct i915_wa_list *wal) >> > > > >> > > > GEN12_DISABLE_HDR_PAST_PAYLOAD_HOLD_FIX); >> > > > } >> > > > >> > > > - if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_C0) || >> > > > - IS_DG2_G11(i915)) { >> > > > - /* >> > > > - * Wa_22012826095:dg2 >> > > > - * Wa_22013059131:dg2 >> > > > - */ >> > > > - wa_mcr_write_clr_set(wal, LSC_CHICKEN_BIT_0_UDW, >> > > > - MAXREQS_PER_BANK, >> > > > - >> > > > REG_FIELD_PREP(MAXREQS_PER_BANK, 2)); >> > > > - >> > > > - /* Wa_22013059131:dg2 */ >> > > > - wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0, >> > > > - >> > > > FORCE_1_SUB_MESSAGE_PER_FRAGMENT); >> > > > - } >> > > > - >> > > > /* Wa_1308578152:dg2_g10 when first gslice is fused off >> > > > */ >> > > > if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_C0) && >> > > > needs_wa_1308578152(engine)) { >> > > > @@ -2412,16 +2393,6 @@ rcs_engine_wa_init(struct intel_engine_cs >> > > > *engine, struct i915_wa_list *wal) >> > > > */ >> > > > wa_mcr_masked_en(wal, GEN8_ROW_CHICKEN, >> > > > MDQ_ARBITRATION_MODE | >> > > > UGM_BACKUP_MODE); >> > > > - >> > > > - /* >> > > > - * Wa_14010918519:dg2_g10 >> > > > - * >> > > > - * LSC_CHICKEN_BIT_0 always reads back as 0 is >> > > > this stepping, >> > > > - * so ignoring verification. >> > > > - */ >> > > > - wa_mcr_add(wal, LSC_CHICKEN_BIT_0_UDW, 0, >> > > > - FORCE_SLM_FENCE_SCOPE_TO_TILE | >> > > > FORCE_UGM_FENCE_SCOPE_TO_TILE, >> > > > - 0, false); >> > > > } >> > > > >> > > > if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_A0, STEP_B0)) { >> > > > @@ -3006,6 +2977,15 @@ general_render_compute_wa_init(struct >> > > > intel_engine_cs *engine, struct i915_wa_li >> > > > >> > > > add_render_compute_tuning_settings(i915, wal); >> > > > >> > > > + if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || >> > > > + IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0) || >> > > > + IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, >> > > > STEP_FOREVER) || >> > > > + IS_DG2_G11(i915) || IS_DG2_G12(i915)) { >> > > > + /* Wa_22013037850 */ >> > > > + wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0_UDW, >> > > > + 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) || >> > > > IS_PONTEVECCHIO(i915) || >> > > > @@ -3027,6 +3007,33 @@ general_render_compute_wa_init(struct >> > > > intel_engine_cs *engine, struct i915_wa_li >> > > > wa_masked_en(wal, VFG_PREEMPTION_CHICKEN, >> > > > POLYGON_TRIFAN_LINELOOP_DISABLE); >> > > > } >> > > > >> > > > + if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_C0) || >> > > > + IS_DG2_G11(i915)) { >> > > > + /* >> > > > + * Wa_22012826095:dg2 >> > > > + * Wa_22013059131:dg2 >> > > > + */ >> > > > + wa_mcr_write_clr_set(wal, LSC_CHICKEN_BIT_0_UDW, >> > > > + MAXREQS_PER_BANK, >> > > > + >> > > > REG_FIELD_PREP(MAXREQS_PER_BANK, 2)); >> > > > + >> > > > + /* Wa_22013059131:dg2 */ >> > > > + wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0, >> > > > + FORCE_1_SUB_MESSAGE_PER_FRAGMENT) >> > > > ; >> > > > + } >> > > > + >> > > > + if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_A0, STEP_B0)) { >> > > > + /* >> > > > + * Wa_14010918519:dg2_g10 >> > > > + * >> > > > + * LSC_CHICKEN_BIT_0 always reads back as 0 is >> > > > this stepping, >> > > > + * so ignoring verification. >> > > > + */ >> > > > + wa_mcr_add(wal, LSC_CHICKEN_BIT_0_UDW, 0, >> > > > + FORCE_SLM_FENCE_SCOPE_TO_TILE | >> > > > FORCE_UGM_FENCE_SCOPE_TO_TILE, >> > > > + 0, false); >> > > > + } >> > > > + >> > > > if (IS_PONTEVECCHIO(i915)) { >> > > > /* Wa_16016694945 */ >> > > > wa_masked_en(wal, XEHPC_LNCFMISCCFGREG0, >> > > > XEHPC_OVRLSCCC); >> > > > -- >> > > > 2.39.0 >> > > > >> > >-- >Matt Roper >Graphics Software Engineer >Linux GPU Platform Enablement >Intel Corporation
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c index ef6065ce8267..918a271447e2 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c @@ -2341,10 +2341,6 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal) /* Wa_1509727124 */ wa_mcr_masked_en(wal, GEN10_SAMPLER_MODE, SC_DISABLE_POWER_OPTIMIZATION_EBB); - - /* Wa_22013037850 */ - wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0_UDW, - DISABLE_128B_EVICTION_COMMAND_UDW); } if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_FOREVER) || @@ -2373,21 +2369,6 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal) GEN12_DISABLE_HDR_PAST_PAYLOAD_HOLD_FIX); } - if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_C0) || - IS_DG2_G11(i915)) { - /* - * Wa_22012826095:dg2 - * Wa_22013059131:dg2 - */ - wa_mcr_write_clr_set(wal, LSC_CHICKEN_BIT_0_UDW, - MAXREQS_PER_BANK, - REG_FIELD_PREP(MAXREQS_PER_BANK, 2)); - - /* Wa_22013059131:dg2 */ - wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0, - FORCE_1_SUB_MESSAGE_PER_FRAGMENT); - } - /* Wa_1308578152:dg2_g10 when first gslice is fused off */ if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_C0) && needs_wa_1308578152(engine)) { @@ -2412,16 +2393,6 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal) */ wa_mcr_masked_en(wal, GEN8_ROW_CHICKEN, MDQ_ARBITRATION_MODE | UGM_BACKUP_MODE); - - /* - * Wa_14010918519:dg2_g10 - * - * LSC_CHICKEN_BIT_0 always reads back as 0 is this stepping, - * so ignoring verification. - */ - wa_mcr_add(wal, LSC_CHICKEN_BIT_0_UDW, 0, - FORCE_SLM_FENCE_SCOPE_TO_TILE | FORCE_UGM_FENCE_SCOPE_TO_TILE, - 0, false); } if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_A0, STEP_B0)) { @@ -3006,6 +2977,15 @@ general_render_compute_wa_init(struct intel_engine_cs *engine, struct i915_wa_li add_render_compute_tuning_settings(i915, wal); + if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || + IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0) || + IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_FOREVER) || + IS_DG2_G11(i915) || IS_DG2_G12(i915)) { + /* Wa_22013037850 */ + wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0_UDW, + 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) || IS_PONTEVECCHIO(i915) || @@ -3027,6 +3007,33 @@ general_render_compute_wa_init(struct intel_engine_cs *engine, struct i915_wa_li wa_masked_en(wal, VFG_PREEMPTION_CHICKEN, POLYGON_TRIFAN_LINELOOP_DISABLE); } + if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_C0) || + IS_DG2_G11(i915)) { + /* + * Wa_22012826095:dg2 + * Wa_22013059131:dg2 + */ + wa_mcr_write_clr_set(wal, LSC_CHICKEN_BIT_0_UDW, + MAXREQS_PER_BANK, + REG_FIELD_PREP(MAXREQS_PER_BANK, 2)); + + /* Wa_22013059131:dg2 */ + wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0, + FORCE_1_SUB_MESSAGE_PER_FRAGMENT); + } + + if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_A0, STEP_B0)) { + /* + * Wa_14010918519:dg2_g10 + * + * LSC_CHICKEN_BIT_0 always reads back as 0 is this stepping, + * so ignoring verification. + */ + wa_mcr_add(wal, LSC_CHICKEN_BIT_0_UDW, 0, + FORCE_SLM_FENCE_SCOPE_TO_TILE | FORCE_UGM_FENCE_SCOPE_TO_TILE, + 0, false); + } + if (IS_PONTEVECCHIO(i915)) { /* Wa_16016694945 */ wa_masked_en(wal, XEHPC_LNCFMISCCFGREG0, XEHPC_OVRLSCCC);
That register doesn't belong to a specific engine, so the proper placement for workarounds programming it should be general_render_compute_wa_init(). Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com> --- drivers/gpu/drm/i915/gt/intel_workarounds.c | 65 ++++++++++++--------- 1 file changed, 36 insertions(+), 29 deletions(-)