[Intel-gfx] [PATCH] drm/i915/gen9: implement WaConextSwitchWithConcurrentTLBInvalidate

[Gore, Tim]

Tim Gore 
Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ


> -----Original Message-----
> From: Arun Siluvery [mailto:arun.siluvery at linux.intel.com]
> Sent: Thursday, June 09, 2016 2:05 PM
> To: Gore, Tim; intel-gfx at lists.freedesktop.org
> Subject: Re: [PATCH] drm/i915/gen9: implement
> WaConextSwitchWithConcurrentTLBInvalidate
> 
> On 09/06/2016 13:48, tim.gore at intel.com wrote:
> > From: Tim Gore <tim.gore at intel.com>
> >
> > This patch enables a workaround for a mid thread preemption issue
> > where a hardware timing problem can prevent the context restore from
> > happening, leading to a hang.
> >
> > Signed-off-by: Tim Gore <tim.gore at intel.com>
> > ---
> >   drivers/gpu/drm/i915/i915_gem_gtt.c | 6 ++++++
> >   drivers/gpu/drm/i915/i915_reg.h     | 4 ++++
> >   2 files changed, 10 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index 4668477..e9046f6 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -2156,6 +2156,12 @@ static void gtt_write_workarounds(struct
> drm_device *dev)
> >   		I915_WRITE(GEN8_L3_LRA_1_GPGPU,
> GEN9_L3_LRA_1_GPGPU_DEFAULT_VALUE_SKL);
> >   	else if (IS_BROXTON(dev))
> >   		I915_WRITE(GEN8_L3_LRA_1_GPGPU,
> > GEN9_L3_LRA_1_GPGPU_DEFAULT_VALUE_BXT);
> > +
> > +	/* WaConextSwitchWithConcurrentTLBInvalidate:gen9 */
> > +	if (IS_GEN9(dev))
> > +	{
> > +		I915_WRITE(GEN9_CSFE_CHICKEN1_RCS,
> _MASKED_BIT_ENABLE(GEN9_PREEMPT_GPGPU_SYNC_SWITCH_DISABLE));
> > +	}
> This is not the correct place, it should be added in
> gen9_init_workarounds() in intel_ringbuffer.c as it applies to both skl and
> bxt.
> 
> Please also correct the spelling and we usually mention both skl, bxt instead
> of gen9.
> WaContextSwitchWithConcurrentTLBInvalidate:skl,bxt
> 
> regards
> Arun
> 
The spelling is as per documentation and other code. Changing it will just add confusion.
The w/a is for all gen 9 chips, kbl, bxt, skl etc., not sure why we need to enumerate them,
anyway there is unlikely to be another gen9 chip so maybe a moot point, so I'll list them.
Will move to gen9_init_workarounds, this seems to be called during reset and resume so
Should also work. V2 patch to follow

  Tim

> >   }
> >
> >   static int i915_ppgtt_init(struct drm_device *dev, struct
> > i915_hw_ppgtt *ppgtt) diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h index 81d1896..2a6fc62 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -1810,6 +1810,10 @@ enum skl_disp_power_wells {
> >   #define   GEN9_IZ_HASHING_MASK(slice)			(0x3 <<
> ((slice) * 2))
> >   #define   GEN9_IZ_HASHING(slice, val)			((val) <<
> ((slice) * 2))
> >
> > +/* chicken reg for WaConextSwitchWithConcurrentTLBInvalidate */
> > +#define GEN9_CSFE_CHICKEN1_RCS _MMIO(0x20D4)
> > +#define   GEN9_PREEMPT_GPGPU_SYNC_SWITCH_DISABLE (1 << 2)
> > +
> >   /* WaClearTdlStateAckDirtyBits */
> >   #define GEN8_STATE_ACK		_MMIO(0x20F0)
> >   #define GEN9_STATE_ACK_SLICE1	_MMIO(0x20F8)
> >