| Message ID | 1444645259-16840-1-git-send-email-mika.kuoppala@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Oct 12, 2015 at 01:20:59PM +0300, Mika Kuoppala wrote: > Some registers are, naturally, lost in gpu reset/suspend cycle. > And some registers, for example in display domain, are not subject > to gpu reset so they retain their contents. > > As hang recovery triggers a reset, recoverable gpu hang can currently > flush out essential workarounds and cause havoc later on. > > When register GEN8_GARBNTL is missing the WaEnableGapsTsvCreditFix:skl, > it can cause random system hangs [1]. This workaround was added in: > commit 245d96670d26 ("drm/i915:skl: Add WaEnableGapsTsvCreditFix") > But another set of system hangs were observed and the failure pattern > indicated that there was random gpu hang preceding the system hang [2]. > This lead to the realization that we lose this workaround and BDW_SCRATCH1 > on reset. > > Add these workarounds setup in display init to skl/bxt ring init > where LRI workarounds are also setup. This way their setup is not > dependent on display side init. > > References: [1] https://bugs.freedesktop.org/show_bug.cgi?id=90854 > References: [2] https://bugs.freedesktop.org/show_bug.cgi?id=92315 > Reported-by: Tomi Sarvela <tomix.p.sarvela@intel.com> > Cc: Tomi Sarvela <tomix.p.sarvela@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> I had forgotten we had begun splitting out the GT registers from the display registers for init_hw. These movements make sense so, Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
On Mon, 12 Oct 2015, Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote: > Some registers are, naturally, lost in gpu reset/suspend cycle. > And some registers, for example in display domain, are not subject > to gpu reset so they retain their contents. > > As hang recovery triggers a reset, recoverable gpu hang can currently > flush out essential workarounds and cause havoc later on. > > When register GEN8_GARBNTL is missing the WaEnableGapsTsvCreditFix:skl, > it can cause random system hangs [1]. This workaround was added in: > commit 245d96670d26 ("drm/i915:skl: Add WaEnableGapsTsvCreditFix") > But another set of system hangs were observed and the failure pattern > indicated that there was random gpu hang preceding the system hang [2]. > This lead to the realization that we lose this workaround and BDW_SCRATCH1 > on reset. > > Add these workarounds setup in display init to skl/bxt ring init > where LRI workarounds are also setup. This way their setup is not > dependent on display side init. > > References: [1] https://bugs.freedesktop.org/show_bug.cgi?id=90854 > References: [2] https://bugs.freedesktop.org/show_bug.cgi?id=92315 > Reported-by: Tomi Sarvela <tomix.p.sarvela@intel.com> > Cc: Tomi Sarvela <tomix.p.sarvela@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> Tested-by: Tomi Sarvela <tomix.p.sarvela@intel.com> Mika, the patch doesn't apply to v4.3-rc5 - is it needed there or in dinq? BR, Jani. > --- > drivers/gpu/drm/i915/intel_pm.c | 60 --------------------------------- > drivers/gpu/drm/i915/intel_ringbuffer.c | 44 +++++++++++++++++++++++- > 2 files changed, 43 insertions(+), 61 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 3f9b3c0..2482cfd 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -52,56 +52,10 @@ > #define INTEL_RC6p_ENABLE (1<<1) > #define INTEL_RC6pp_ENABLE (1<<2) > > -static void gen9_init_clock_gating(struct drm_device *dev) > -{ > - struct drm_i915_private *dev_priv = dev->dev_private; > - > - /* WaEnableLbsSlaRetryTimerDecrement:skl */ > - I915_WRITE(BDW_SCRATCH1, I915_READ(BDW_SCRATCH1) | > - GEN9_LBS_SLA_RETRY_TIMER_DECREMENT_ENABLE); > - > - /* WaDisableKillLogic:bxt,skl */ > - I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | > - ECOCHK_DIS_TLB); > -} > - > -static void skl_init_clock_gating(struct drm_device *dev) > -{ > - struct drm_i915_private *dev_priv = dev->dev_private; > - > - gen9_init_clock_gating(dev); > - > - if (INTEL_REVID(dev) <= SKL_REVID_D0) { > - /* WaDisableHDCInvalidation:skl */ > - I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | > - BDW_DISABLE_HDC_INVALIDATION); > - > - /* WaDisableChickenBitTSGBarrierAckForFFSliceCS:skl */ > - I915_WRITE(FF_SLICE_CS_CHICKEN2, > - _MASKED_BIT_ENABLE(GEN9_TSG_BARRIER_ACK_DISABLE)); > - } > - > - /* GEN8_L3SQCREG4 has a dependency with WA batch so any new changes > - * involving this register should also be added to WA batch as required. > - */ > - if (INTEL_REVID(dev) <= SKL_REVID_E0) > - /* WaDisableLSQCROPERFforOCL:skl */ > - I915_WRITE(GEN8_L3SQCREG4, I915_READ(GEN8_L3SQCREG4) | > - GEN8_LQSC_RO_PERF_DIS); > - > - /* WaEnableGapsTsvCreditFix:skl */ > - if (IS_SKYLAKE(dev) && (INTEL_REVID(dev) >= SKL_REVID_C0)) { > - I915_WRITE(GEN8_GARBCNTL, (I915_READ(GEN8_GARBCNTL) | > - GEN9_GAPS_TSV_CREDIT_DISABLE)); > - } > -} > - > static void bxt_init_clock_gating(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > > - gen9_init_clock_gating(dev); > - > /* WaDisableSDEUnitClockGating:bxt */ > I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) | > GEN8_SDEUNIT_CLOCK_GATE_DISABLE); > @@ -112,17 +66,6 @@ static void bxt_init_clock_gating(struct drm_device *dev) > */ > I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) | > GEN8_HDCUNIT_CLOCK_GATE_DISABLE_HDCREQ); > - > - /* WaStoreMultiplePTEenable:bxt */ > - /* This is a requirement according to Hardware specification */ > - if (INTEL_REVID(dev) == BXT_REVID_A0) > - I915_WRITE(TILECTL, I915_READ(TILECTL) | TILECTL_TLBPF); > - > - /* WaSetClckGatingDisableMedia:bxt */ > - if (INTEL_REVID(dev) == BXT_REVID_A0) { > - I915_WRITE(GEN7_MISCCPCTL, (I915_READ(GEN7_MISCCPCTL) & > - ~GEN8_DOP_CLOCK_GATE_MEDIA_ENABLE)); > - } > } > > static void i915_pineview_get_mem_freq(struct drm_device *dev) > @@ -6991,9 +6934,6 @@ void intel_init_pm(struct drm_device *dev) > if (IS_BROXTON(dev)) > dev_priv->display.init_clock_gating = > bxt_init_clock_gating; > - else if (IS_SKYLAKE(dev)) > - dev_priv->display.init_clock_gating = > - skl_init_clock_gating; > dev_priv->display.update_wm = skl_update_wm; > } else if (HAS_PCH_SPLIT(dev)) { > ilk_setup_wm_latency(dev); > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > index 654ae99..0359736 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -906,6 +906,14 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring) > struct drm_i915_private *dev_priv = dev->dev_private; > uint32_t tmp; > > + /* WaEnableLbsSlaRetryTimerDecrement:skl */ > + I915_WRITE(BDW_SCRATCH1, I915_READ(BDW_SCRATCH1) | > + GEN9_LBS_SLA_RETRY_TIMER_DECREMENT_ENABLE); > + > + /* WaDisableKillLogic:bxt,skl */ > + I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | > + ECOCHK_DIS_TLB); > + > /* WaDisablePartialInstShootdown:skl,bxt */ > WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN, > PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE); > @@ -1018,7 +1026,6 @@ static int skl_tune_iz_hashing(struct intel_engine_cs *ring) > return 0; > } > > - > static int skl_init_workarounds(struct intel_engine_cs *ring) > { > int ret; > @@ -1029,6 +1036,30 @@ static int skl_init_workarounds(struct intel_engine_cs *ring) > if (ret) > return ret; > > + if (INTEL_REVID(dev) <= SKL_REVID_D0) { > + /* WaDisableHDCInvalidation:skl */ > + I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | > + BDW_DISABLE_HDC_INVALIDATION); > + > + /* WaDisableChickenBitTSGBarrierAckForFFSliceCS:skl */ > + I915_WRITE(FF_SLICE_CS_CHICKEN2, > + _MASKED_BIT_ENABLE(GEN9_TSG_BARRIER_ACK_DISABLE)); > + } > + > + /* GEN8_L3SQCREG4 has a dependency with WA batch so any new changes > + * involving this register should also be added to WA batch as required. > + */ > + if (INTEL_REVID(dev) <= SKL_REVID_E0) > + /* WaDisableLSQCROPERFforOCL:skl */ > + I915_WRITE(GEN8_L3SQCREG4, I915_READ(GEN8_L3SQCREG4) | > + GEN8_LQSC_RO_PERF_DIS); > + > + /* WaEnableGapsTsvCreditFix:skl */ > + if (IS_SKYLAKE(dev) && (INTEL_REVID(dev) >= SKL_REVID_C0)) { > + I915_WRITE(GEN8_GARBCNTL, (I915_READ(GEN8_GARBCNTL) | > + GEN9_GAPS_TSV_CREDIT_DISABLE)); > + } > + > /* WaDisablePowerCompilerClockGating:skl */ > if (INTEL_REVID(dev) == SKL_REVID_B0) > WA_SET_BIT_MASKED(HIZ_CHICKEN, > @@ -1072,6 +1103,17 @@ static int bxt_init_workarounds(struct intel_engine_cs *ring) > if (ret) > return ret; > > + /* WaStoreMultiplePTEenable:bxt */ > + /* This is a requirement according to Hardware specification */ > + if (INTEL_REVID(dev) == BXT_REVID_A0) > + I915_WRITE(TILECTL, I915_READ(TILECTL) | TILECTL_TLBPF); > + > + /* WaSetClckGatingDisableMedia:bxt */ > + if (INTEL_REVID(dev) == BXT_REVID_A0) { > + I915_WRITE(GEN7_MISCCPCTL, (I915_READ(GEN7_MISCCPCTL) & > + ~GEN8_DOP_CLOCK_GATE_MEDIA_ENABLE)); > + } > + > /* WaDisableThreadStallDopClockGating:bxt */ > WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN, > STALL_DOP_GATING_DISABLE); > -- > 2.1.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, Oct 13, 2015 at 04:08:10PM +0300, Jani Nikula wrote: > On Mon, 12 Oct 2015, Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote: > > Some registers are, naturally, lost in gpu reset/suspend cycle. > > And some registers, for example in display domain, are not subject > > to gpu reset so they retain their contents. > > > > As hang recovery triggers a reset, recoverable gpu hang can currently > > flush out essential workarounds and cause havoc later on. > > > > When register GEN8_GARBNTL is missing the WaEnableGapsTsvCreditFix:skl, > > it can cause random system hangs [1]. This workaround was added in: > > commit 245d96670d26 ("drm/i915:skl: Add WaEnableGapsTsvCreditFix") > > But another set of system hangs were observed and the failure pattern > > indicated that there was random gpu hang preceding the system hang [2]. > > This lead to the realization that we lose this workaround and BDW_SCRATCH1 > > on reset. > > > > Add these workarounds setup in display init to skl/bxt ring init > > where LRI workarounds are also setup. This way their setup is not > > dependent on display side init. > > > > References: [1] https://bugs.freedesktop.org/show_bug.cgi?id=90854 > > References: [2] https://bugs.freedesktop.org/show_bug.cgi?id=92315 > > Reported-by: Tomi Sarvela <tomix.p.sarvela@intel.com> > > Cc: Tomi Sarvela <tomix.p.sarvela@intel.com> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> > > Tested-by: Tomi Sarvela <tomix.p.sarvela@intel.com> > > Mika, the patch doesn't apply to v4.3-rc5 - is it needed there or in > dinq? We definitely need it in dinq to unblock future wa work, but it might be needed for 4.3 just for skl. Applied it to dinq meanwhile, I guess we can cherry-pick just the skl part if needed. -Daniel
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 3f9b3c0..2482cfd 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -52,56 +52,10 @@ #define INTEL_RC6p_ENABLE (1<<1) #define INTEL_RC6pp_ENABLE (1<<2) -static void gen9_init_clock_gating(struct drm_device *dev) -{ - struct drm_i915_private *dev_priv = dev->dev_private; - - /* WaEnableLbsSlaRetryTimerDecrement:skl */ - I915_WRITE(BDW_SCRATCH1, I915_READ(BDW_SCRATCH1) | - GEN9_LBS_SLA_RETRY_TIMER_DECREMENT_ENABLE); - - /* WaDisableKillLogic:bxt,skl */ - I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | - ECOCHK_DIS_TLB); -} - -static void skl_init_clock_gating(struct drm_device *dev) -{ - struct drm_i915_private *dev_priv = dev->dev_private; - - gen9_init_clock_gating(dev); - - if (INTEL_REVID(dev) <= SKL_REVID_D0) { - /* WaDisableHDCInvalidation:skl */ - I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | - BDW_DISABLE_HDC_INVALIDATION); - - /* WaDisableChickenBitTSGBarrierAckForFFSliceCS:skl */ - I915_WRITE(FF_SLICE_CS_CHICKEN2, - _MASKED_BIT_ENABLE(GEN9_TSG_BARRIER_ACK_DISABLE)); - } - - /* GEN8_L3SQCREG4 has a dependency with WA batch so any new changes - * involving this register should also be added to WA batch as required. - */ - if (INTEL_REVID(dev) <= SKL_REVID_E0) - /* WaDisableLSQCROPERFforOCL:skl */ - I915_WRITE(GEN8_L3SQCREG4, I915_READ(GEN8_L3SQCREG4) | - GEN8_LQSC_RO_PERF_DIS); - - /* WaEnableGapsTsvCreditFix:skl */ - if (IS_SKYLAKE(dev) && (INTEL_REVID(dev) >= SKL_REVID_C0)) { - I915_WRITE(GEN8_GARBCNTL, (I915_READ(GEN8_GARBCNTL) | - GEN9_GAPS_TSV_CREDIT_DISABLE)); - } -} - static void bxt_init_clock_gating(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; - gen9_init_clock_gating(dev); - /* WaDisableSDEUnitClockGating:bxt */ I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) | GEN8_SDEUNIT_CLOCK_GATE_DISABLE); @@ -112,17 +66,6 @@ static void bxt_init_clock_gating(struct drm_device *dev) */ I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) | GEN8_HDCUNIT_CLOCK_GATE_DISABLE_HDCREQ); - - /* WaStoreMultiplePTEenable:bxt */ - /* This is a requirement according to Hardware specification */ - if (INTEL_REVID(dev) == BXT_REVID_A0) - I915_WRITE(TILECTL, I915_READ(TILECTL) | TILECTL_TLBPF); - - /* WaSetClckGatingDisableMedia:bxt */ - if (INTEL_REVID(dev) == BXT_REVID_A0) { - I915_WRITE(GEN7_MISCCPCTL, (I915_READ(GEN7_MISCCPCTL) & - ~GEN8_DOP_CLOCK_GATE_MEDIA_ENABLE)); - } } static void i915_pineview_get_mem_freq(struct drm_device *dev) @@ -6991,9 +6934,6 @@ void intel_init_pm(struct drm_device *dev) if (IS_BROXTON(dev)) dev_priv->display.init_clock_gating = bxt_init_clock_gating; - else if (IS_SKYLAKE(dev)) - dev_priv->display.init_clock_gating = - skl_init_clock_gating; dev_priv->display.update_wm = skl_update_wm; } else if (HAS_PCH_SPLIT(dev)) { ilk_setup_wm_latency(dev); diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 654ae99..0359736 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -906,6 +906,14 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring) struct drm_i915_private *dev_priv = dev->dev_private; uint32_t tmp; + /* WaEnableLbsSlaRetryTimerDecrement:skl */ + I915_WRITE(BDW_SCRATCH1, I915_READ(BDW_SCRATCH1) | + GEN9_LBS_SLA_RETRY_TIMER_DECREMENT_ENABLE); + + /* WaDisableKillLogic:bxt,skl */ + I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | + ECOCHK_DIS_TLB); + /* WaDisablePartialInstShootdown:skl,bxt */ WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN, PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE); @@ -1018,7 +1026,6 @@ static int skl_tune_iz_hashing(struct intel_engine_cs *ring) return 0; } - static int skl_init_workarounds(struct intel_engine_cs *ring) { int ret; @@ -1029,6 +1036,30 @@ static int skl_init_workarounds(struct intel_engine_cs *ring) if (ret) return ret; + if (INTEL_REVID(dev) <= SKL_REVID_D0) { + /* WaDisableHDCInvalidation:skl */ + I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | + BDW_DISABLE_HDC_INVALIDATION); + + /* WaDisableChickenBitTSGBarrierAckForFFSliceCS:skl */ + I915_WRITE(FF_SLICE_CS_CHICKEN2, + _MASKED_BIT_ENABLE(GEN9_TSG_BARRIER_ACK_DISABLE)); + } + + /* GEN8_L3SQCREG4 has a dependency with WA batch so any new changes + * involving this register should also be added to WA batch as required. + */ + if (INTEL_REVID(dev) <= SKL_REVID_E0) + /* WaDisableLSQCROPERFforOCL:skl */ + I915_WRITE(GEN8_L3SQCREG4, I915_READ(GEN8_L3SQCREG4) | + GEN8_LQSC_RO_PERF_DIS); + + /* WaEnableGapsTsvCreditFix:skl */ + if (IS_SKYLAKE(dev) && (INTEL_REVID(dev) >= SKL_REVID_C0)) { + I915_WRITE(GEN8_GARBCNTL, (I915_READ(GEN8_GARBCNTL) | + GEN9_GAPS_TSV_CREDIT_DISABLE)); + } + /* WaDisablePowerCompilerClockGating:skl */ if (INTEL_REVID(dev) == SKL_REVID_B0) WA_SET_BIT_MASKED(HIZ_CHICKEN, @@ -1072,6 +1103,17 @@ static int bxt_init_workarounds(struct intel_engine_cs *ring) if (ret) return ret; + /* WaStoreMultiplePTEenable:bxt */ + /* This is a requirement according to Hardware specification */ + if (INTEL_REVID(dev) == BXT_REVID_A0) + I915_WRITE(TILECTL, I915_READ(TILECTL) | TILECTL_TLBPF); + + /* WaSetClckGatingDisableMedia:bxt */ + if (INTEL_REVID(dev) == BXT_REVID_A0) { + I915_WRITE(GEN7_MISCCPCTL, (I915_READ(GEN7_MISCCPCTL) & + ~GEN8_DOP_CLOCK_GATE_MEDIA_ENABLE)); + } + /* WaDisableThreadStallDopClockGating:bxt */ WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN, STALL_DOP_GATING_DISABLE);
Some registers are, naturally, lost in gpu reset/suspend cycle. And some registers, for example in display domain, are not subject to gpu reset so they retain their contents. As hang recovery triggers a reset, recoverable gpu hang can currently flush out essential workarounds and cause havoc later on. When register GEN8_GARBNTL is missing the WaEnableGapsTsvCreditFix:skl, it can cause random system hangs [1]. This workaround was added in: commit 245d96670d26 ("drm/i915:skl: Add WaEnableGapsTsvCreditFix") But another set of system hangs were observed and the failure pattern indicated that there was random gpu hang preceding the system hang [2]. This lead to the realization that we lose this workaround and BDW_SCRATCH1 on reset. Add these workarounds setup in display init to skl/bxt ring init where LRI workarounds are also setup. This way their setup is not dependent on display side init. References: [1] https://bugs.freedesktop.org/show_bug.cgi?id=90854 References: [2] https://bugs.freedesktop.org/show_bug.cgi?id=92315 Reported-by: Tomi Sarvela <tomix.p.sarvela@intel.com> Cc: Tomi Sarvela <tomix.p.sarvela@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> --- drivers/gpu/drm/i915/intel_pm.c | 60 --------------------------------- drivers/gpu/drm/i915/intel_ringbuffer.c | 44 +++++++++++++++++++++++- 2 files changed, 43 insertions(+), 61 deletions(-)