| Message ID | 1465460290-4561-1-git-send-email-tim.gore@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 09 Jun 2016, tim.gore@intel.com wrote: > From: Tim Gore <tim.gore@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@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 */ ^^^^^^ Typo or sic? BR, Jani. > + if (IS_GEN9(dev)) > + { > + I915_WRITE(GEN9_CSFE_CHICKEN1_RCS, _MASKED_BIT_ENABLE(GEN9_PREEMPT_GPGPU_SYNC_SWITCH_DISABLE)); > + } > } > > 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)
Tim Gore Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ > -----Original Message----- > From: Jani Nikula [mailto:jani.nikula@linux.intel.com] > Sent: Thursday, June 09, 2016 9:34 AM > To: Gore, Tim; intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH] drm/i915/gen9: implement > WaConextSwitchWithConcurrentTLBInvalidate > > On Thu, 09 Jun 2016, tim.gore@intel.com wrote: > > From: Tim Gore <tim.gore@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@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 */ > ^^^^^^ > > Typo or sic? > > BR, > Jani. > Sic !! Tim > > > + if (IS_GEN9(dev)) > > + { > > + I915_WRITE(GEN9_CSFE_CHICKEN1_RCS, > _MASKED_BIT_ENABLE(GEN9_PREEMPT_GPGPU_SYNC_SWITCH_DISABLE)); > > + } > > } > > > > 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) > > -- > Jani Nikula, Intel Open Source Technology Center
On 09/06/2016 13:48, tim.gore@intel.com wrote: > From: Tim Gore <tim.gore@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@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 > } > > 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) >
Tim Gore Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ > -----Original Message----- > From: Arun Siluvery [mailto:arun.siluvery@linux.intel.com] > Sent: Thursday, June 09, 2016 2:05 PM > To: Gore, Tim; intel-gfx@lists.freedesktop.org > Subject: Re: [PATCH] drm/i915/gen9: implement > WaConextSwitchWithConcurrentTLBInvalidate > > On 09/06/2016 13:48, tim.gore@intel.com wrote: > > From: Tim Gore <tim.gore@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@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) > >
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)); + } } 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)