[Intel-gfx] [PATCH v2 2/2] drm/i915/gt: Move LSC_CHICKEN_BIT* workarounds to correct function
Lucas De Marchi
lucas.demarchi at intel.com
Fri Jan 20 22:46:05 UTC 2023
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 at intel.com>
>> > >
>> > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi at 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
More information about the Intel-gfx
mailing list