[Intel-gfx] [PATCH v4 1/2] drm/i915/dg2: Introduce Wa_18018764978

[Matt Roper]

On Wed, Jan 18, 2023 at 12:01:20PM -0500, Rodrigo Vivi wrote:
> On Wed, Jan 18, 2023 at 09:54:56AM +0000, Tvrtko Ursulin wrote:
> > 
> > On 28/11/2022 18:26, Matt Roper wrote:
> > > On Wed, Nov 23, 2022 at 04:45:25PM -0300, Gustavo Sousa wrote:
> > > > On Wed, Nov 23, 2022 at 10:36:47AM -0800, Matt Atwood wrote:
> > > > > Wa_18018764978 applies to specific steppings of DG2 (G10 C0+,
> > > > > G11 and G12 A0+). Clean up style in function at the same time.
> > > > > 
> > > > > Bspec: 66622
> > > > > 
> > > > > Signed-off-by: Matt Atwood <matthew.s.atwood at intel.com>
> > > > 
> > > > Reviewed-by: Gustavo Sousa <gustavo.sousa at intel.com>
> > > 
> > > Both patches applied to drm-intel-gt-next.  Thanks for the patches and
> > > review.
> > 
> > Do these need to be sent to 6.2 fixes, given DG2 is out of force probe
> > there?
> 
> Yeap, good point. I'd say in general we are not very good in cherry-picking
> the hw w/a to the fixes. But this one was a good catch. Let's ensure the
> best stability for DG2 on 6.2.
> 
> pushed to drm-intel-fixes now.

I don't have any concerns about putting this in drm-intel-fixes, and the
patch will functionally behave as expected, but I did just notice one
minor problem with the original patch that we should probably fix up on
drm-intel-gt-next:  the PSS_MODE2 register is in a range that has
multicast behavior (GSLICE replication) on Xe_HP platforms.  So the
register should have been defined as MCR_REG() instead of _MMIO, and the
workaround should use wa_mcr_masked_en().

As I mentioned, this oversight shouldn't cause any real problems, so no
concerns about keeping this in -fixes.  On DG2 we don't have to worry
about steering races with external agents, so even with the 'wrong'
register type definition, the steering should already be implicitly set
to multicast mode at the point we're applying workarounds.  The need to
follow up with a correction on drm-intel-gt-next is really only
important for consistency and in case this register gets used in other
manners (not just workaround lists) at some point in the future.

MattA/Gustavo will one of you guys send an update for that, or should I
take care of it?


Matt

> 
> Thanks,
> Rodrigo.
> 
> > 
> > Regards,
> > 
> > Tvrtko
> > 
> > > > > ---
> > > > >   drivers/gpu/drm/i915/gt/intel_gt_regs.h     | 3 +++
> > > > >   drivers/gpu/drm/i915/gt/intel_workarounds.c | 7 ++++++-
> > > > >   2 files changed, 9 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > > > > index 80a979e6f6be..74379d3c5a4d 100644
> > > > > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > > > > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > > > > @@ -457,6 +457,9 @@
> > > > >   #define GEN8_L3CNTLREG				_MMIO(0x7034)
> > > > >   #define   GEN8_ERRDETBCTRL			(1 << 9)
> > > > > +#define PSS_MODE2				_MMIO(0x703c)
> > > > > +#define   SCOREBOARD_STALL_FLUSH_CONTROL	REG_BIT(5)
> > > > > +
> > > > >   #define GEN7_SC_INSTDONE			_MMIO(0x7100)
> > > > >   #define GEN12_SC_INSTDONE_EXTRA			_MMIO(0x7104)
> > > > >   #define GEN12_SC_INSTDONE_EXTRA2		_MMIO(0x7108)
> > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > > > > index 2afb4f80a954..870db5a202dd 100644
> > > > > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > > > > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > > > > @@ -771,9 +771,14 @@ static void dg2_ctx_workarounds_init(struct intel_engine_cs *engine,
> > > > >   	/* Wa_14014947963:dg2 */
> > > > >   	if (IS_DG2_GRAPHICS_STEP(engine->i915, G10, STEP_B0, STEP_FOREVER) ||
> > > > > -		IS_DG2_G11(engine->i915) || IS_DG2_G12(engine->i915))
> > > > > +	    IS_DG2_G11(engine->i915) || IS_DG2_G12(engine->i915))
> > > > >   		wa_masked_field_set(wal, VF_PREEMPTION, PREEMPTION_VERTEX_COUNT, 0x4000);
> > > > > +	/* Wa_18018764978:dg2 */
> > > > > +	if (IS_DG2_GRAPHICS_STEP(engine->i915, G10, STEP_C0, STEP_FOREVER) ||
> > > > > +	    IS_DG2_G11(engine->i915) || IS_DG2_G12(engine->i915))
> > > > > +		wa_masked_en(wal, PSS_MODE2, SCOREBOARD_STALL_FLUSH_CONTROL);
> > > > > +
> > > > >   	/* Wa_15010599737:dg2 */
> > > > >   	wa_masked_en(wal, CHICKEN_RASTER_1, DIS_SF_ROUND_NEAREST_EVEN);
> > > > >   }
> > > > > -- 
> > > > > 2.38.1
> > > > > 
> > > 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation