| Message ID | 1485347468-7059-1-git-send-email-mika.kuoppala@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jan 25, 2017 at 02:31:08PM +0200, Mika Kuoppala wrote: > Certain Baytrails, namely the 4 cpu core variants, have been > plaqued by spurious system hangs, mostly occurring with light loads. > > Multiple bisects by various people point to a commit which changes the > reclocking strategy for Baytrail to follow its bigger brethen: > commit 8fb55197e64d ("drm/i915: Agressive downclocking on Baytrail") > > There is also a review comment attached to this commit from Deepak S > on avoiding punit access on Cherryview and thus it is excluded on > common reclocking path. By taking the same approach and omitting > the punit access by not tweaking the thresholds when the hardware > has been asked to move into different frequency, considerable gains > in stability have been observed. > > With J1900 box, light render/video load would end up in system hang > in usually less than 12 hours. With this patch applied, the cumulative > uptime has now been 34 days without issues. To provoke system hang, > light loads on both render and bsd engines in parallel have been used: > glxgears >/dev/null 2>/dev/null & > mpv --vo=vaapi --hwdec=vaapi --loop=inf vid.mp4 > > So far, author has not witnessed system hang with above load > and this patch applied. Reports from the tenacious people at > kernel bugzilla are also promising. > > Considering that the punit access frequency with this patch is > considerably less, there is a possibility that this will push > the, still unknown, root cause past the triggering point on most loads. > Further work on investigating the punit accesses on byt is welcomed. Please find the underlying problem and not disabling rps for all vlv for a GT specific problem. -Chris
Chris Wilson <chris@chris-wilson.co.uk> writes: > On Wed, Jan 25, 2017 at 02:31:08PM +0200, Mika Kuoppala wrote: >> Certain Baytrails, namely the 4 cpu core variants, have been >> plaqued by spurious system hangs, mostly occurring with light loads. >> >> Multiple bisects by various people point to a commit which changes the >> reclocking strategy for Baytrail to follow its bigger brethen: >> commit 8fb55197e64d ("drm/i915: Agressive downclocking on Baytrail") >> >> There is also a review comment attached to this commit from Deepak S >> on avoiding punit access on Cherryview and thus it is excluded on >> common reclocking path. By taking the same approach and omitting >> the punit access by not tweaking the thresholds when the hardware >> has been asked to move into different frequency, considerable gains >> in stability have been observed. >> >> With J1900 box, light render/video load would end up in system hang >> in usually less than 12 hours. With this patch applied, the cumulative >> uptime has now been 34 days without issues. To provoke system hang, >> light loads on both render and bsd engines in parallel have been used: >> glxgears >/dev/null 2>/dev/null & >> mpv --vo=vaapi --hwdec=vaapi --loop=inf vid.mp4 >> >> So far, author has not witnessed system hang with above load >> and this patch applied. Reports from the tenacious people at >> kernel bugzilla are also promising. >> >> Considering that the punit access frequency with this patch is >> considerably less, there is a possibility that this will push >> the, still unknown, root cause past the triggering point on most loads. >> Further work on investigating the punit accesses on byt is welcomed. > > Please find the underlying problem and not disabling rps for all vlv > for a GT specific problem. This is not disabling rps. -Mika > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre
On Wed, Jan 25, 2017 at 03:09:04PM +0200, Mika Kuoppala wrote: > Chris Wilson <chris@chris-wilson.co.uk> writes: > > > On Wed, Jan 25, 2017 at 02:31:08PM +0200, Mika Kuoppala wrote: > >> Certain Baytrails, namely the 4 cpu core variants, have been > >> plaqued by spurious system hangs, mostly occurring with light loads. > >> > >> Multiple bisects by various people point to a commit which changes the > >> reclocking strategy for Baytrail to follow its bigger brethen: > >> commit 8fb55197e64d ("drm/i915: Agressive downclocking on Baytrail") > >> > >> There is also a review comment attached to this commit from Deepak S > >> on avoiding punit access on Cherryview and thus it is excluded on > >> common reclocking path. By taking the same approach and omitting > >> the punit access by not tweaking the thresholds when the hardware > >> has been asked to move into different frequency, considerable gains > >> in stability have been observed. > >> > >> With J1900 box, light render/video load would end up in system hang > >> in usually less than 12 hours. With this patch applied, the cumulative > >> uptime has now been 34 days without issues. To provoke system hang, > >> light loads on both render and bsd engines in parallel have been used: > >> glxgears >/dev/null 2>/dev/null & > >> mpv --vo=vaapi --hwdec=vaapi --loop=inf vid.mp4 > >> > >> So far, author has not witnessed system hang with above load > >> and this patch applied. Reports from the tenacious people at > >> kernel bugzilla are also promising. > >> > >> Considering that the punit access frequency with this patch is > >> considerably less, there is a possibility that this will push > >> the, still unknown, root cause past the triggering point on most loads. > >> Further work on investigating the punit accesses on byt is welcomed. > > > > Please find the underlying problem and not disabling rps for all vlv > > for a GT specific problem. > > This is not disabling rps. Your are disabling the key ingredients of the algorithm, making it less generic in order to workaround a problem elsewhere. You are tackling the symptoms and not the cause. -Chris
Chris Wilson <chris@chris-wilson.co.uk> writes: > On Wed, Jan 25, 2017 at 03:09:04PM +0200, Mika Kuoppala wrote: >> Chris Wilson <chris@chris-wilson.co.uk> writes: >> >> > On Wed, Jan 25, 2017 at 02:31:08PM +0200, Mika Kuoppala wrote: >> >> Certain Baytrails, namely the 4 cpu core variants, have been >> >> plaqued by spurious system hangs, mostly occurring with light loads. >> >> >> >> Multiple bisects by various people point to a commit which changes the >> >> reclocking strategy for Baytrail to follow its bigger brethen: >> >> commit 8fb55197e64d ("drm/i915: Agressive downclocking on Baytrail") >> >> >> >> There is also a review comment attached to this commit from Deepak S >> >> on avoiding punit access on Cherryview and thus it is excluded on >> >> common reclocking path. By taking the same approach and omitting >> >> the punit access by not tweaking the thresholds when the hardware >> >> has been asked to move into different frequency, considerable gains >> >> in stability have been observed. >> >> >> >> With J1900 box, light render/video load would end up in system hang >> >> in usually less than 12 hours. With this patch applied, the cumulative >> >> uptime has now been 34 days without issues. To provoke system hang, >> >> light loads on both render and bsd engines in parallel have been used: >> >> glxgears >/dev/null 2>/dev/null & >> >> mpv --vo=vaapi --hwdec=vaapi --loop=inf vid.mp4 >> >> >> >> So far, author has not witnessed system hang with above load >> >> and this patch applied. Reports from the tenacious people at >> >> kernel bugzilla are also promising. >> >> >> >> Considering that the punit access frequency with this patch is >> >> considerably less, there is a possibility that this will push >> >> the, still unknown, root cause past the triggering point on most loads. >> >> Further work on investigating the punit accesses on byt is welcomed. >> > >> > Please find the underlying problem and not disabling rps for all vlv >> > for a GT specific problem. >> >> This is not disabling rps. > > Your are disabling the key ingredients of the algorithm, making it less > generic in order to workaround a problem elsewhere. You are tackling the > symptoms and not the cause. Yes, definitely we are tackling the symptoms. We have been trying to find the root cause for 2 years. Admittely hindered by the multiple other causes for system hangs on baytrail platform. One could argue that why was the deviation for Cherryview accepted, as this just mimics the same way, omitting the sw adjustments. It allows baytrail users to run their rigs without intel_idle.max_cstate=1 which kind of ruins their power budget by far bigger margin than this patch does. -Mika > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre
On Wed, Jan 25, 2017 at 02:31:08PM +0200, Mika Kuoppala wrote: > Certain Baytrails, namely the 4 cpu core variants, have been > plaqued by spurious system hangs, mostly occurring with light loads. > > Multiple bisects by various people point to a commit which changes the > reclocking strategy for Baytrail to follow its bigger brethen: > commit 8fb55197e64d ("drm/i915: Agressive downclocking on Baytrail") > > There is also a review comment attached to this commit from Deepak S > on avoiding punit access on Cherryview and thus it is excluded on > common reclocking path. By taking the same approach and omitting > the punit access by not tweaking the thresholds when the hardware > has been asked to move into different frequency, considerable gains > in stability have been observed. > > With J1900 box, light render/video load would end up in system hang > in usually less than 12 hours. With this patch applied, the cumulative > uptime has now been 34 days without issues. To provoke system hang, > light loads on both render and bsd engines in parallel have been used: > glxgears >/dev/null 2>/dev/null & > mpv --vo=vaapi --hwdec=vaapi --loop=inf vid.mp4 > > So far, author has not witnessed system hang with above load > and this patch applied. Reports from the tenacious people at > kernel bugzilla are also promising. > > Considering that the punit access frequency with this patch is > considerably less, there is a possibility that this will push > the, still unknown, root cause past the triggering point on most loads. > Further work on investigating the punit accesses on byt is welcomed. > > References: https://bugzilla.kernel.org/show_bug.cgi?id=109051 > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Len Brown <len.brown@intel.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: Jani Nikula <jani.nikula@intel.com> > Cc: fritsch@xbmc.org > Cc: miku@iki.fi > Cc: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> > CC: Michal Feix <michal@feix.cz> > Cc: Hans de Goede <hdegoede@redhat.com> > Cc: Deepak S <deepak.s@linux.intel.com> > Cc: Jarkko Nikula <jarkko.nikula@linux.intel.com> > Cc: <stable@vger.kernel.org> # v4.2+ > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> It sucks, but I guess this is better than dead machines. I'd say let's wait another 1-2 weeks for tested-bys to trickle in, and if it does fix the problem then let's apply it. rps keeps on sucking, that's unfortunately not news at all. Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/i915/i915_irq.c | 4 ++-- > drivers/gpu/drm/i915/i915_reg.h | 2 ++ > drivers/gpu/drm/i915/intel_pm.c | 2 +- > 3 files changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 3fc286c..4b9635f 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -1039,7 +1039,7 @@ static u32 vlv_wa_c0_ei(struct drm_i915_private *dev_priv, u32 pm_iir) > if (pm_iir & GEN6_PM_RP_DOWN_EI_EXPIRED) { > if (!vlv_c0_above(dev_priv, > &dev_priv->rps.down_ei, &now, > - dev_priv->rps.down_threshold)) > + VLV_RP_DOWN_EI_THRESHOLD)) > events |= GEN6_PM_RP_DOWN_THRESHOLD; > dev_priv->rps.down_ei = now; > } > @@ -1047,7 +1047,7 @@ static u32 vlv_wa_c0_ei(struct drm_i915_private *dev_priv, u32 pm_iir) > if (pm_iir & GEN6_PM_RP_UP_EI_EXPIRED) { > if (vlv_c0_above(dev_priv, > &dev_priv->rps.up_ei, &now, > - dev_priv->rps.up_threshold)) > + VLV_RP_UP_EI_THRESHOLD)) > events |= GEN6_PM_RP_UP_THRESHOLD; > dev_priv->rps.up_ei = now; > } > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 70d9616..09f6aea 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -787,6 +787,8 @@ enum skl_disp_power_wells { > #define CHV_BIAS_CPU_50_SOC_50 (3 << 2) > > #define VLV_CZ_CLOCK_TO_MILLI_SEC 100000 > +#define VLV_RP_UP_EI_THRESHOLD 90 > +#define VLV_RP_DOWN_EI_THRESHOLD 70 > > /* vlv2 north clock has */ > #define CCK_FUSE_REG 0x8 > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index db24f89..1923b6b 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -4983,7 +4983,7 @@ static void valleyview_set_rps(struct drm_i915_private *dev_priv, u8 val) > > if (val != dev_priv->rps.cur_freq) { > vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val); > - if (!IS_CHERRYVIEW(dev_priv)) > + if (!(IS_CHERRYVIEW(dev_priv) || IS_VALLEYVIEW(dev_priv))) > gen6_set_rps_thresholds(dev_priv, val); > } > > -- > 2.7.4 >
> It sucks, but I guess this is better than dead machines. I'd say let's > wait another 1-2 weeks for tested-bys to trickle in, and if it does fix > the problem then let's apply it. rps keeps on sucking, that's > unfortunately not news at all. > > Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> I have 3 machines that I can hang in under 10 minutes. They are different types of baytrail: n3540, j1900, z3775. With this revert applied, all three machines have the same stress test for over 7 days without failure. I disagree with an action plan that includes the word "wait". Tested-by: Len Brown <len.brown@intel.com> Indeed, my question is if we can turn off GFX p-states entirely on this hardware. Is there a command line parameter I can use to do that? If we have one, it will certainly make troubleshooting orders of magnitude easier. Note that the bisected patch commit 8fb55197e64d5988ec57b54e973daeea72c3f2ff Author: Chris Wilson <chris@chris-wilson.co.uk> Date: Tue Apr 7 16:20:28 2015 +0100 drm/i915: Agressive downclocking on Baytrail was applied to Linux 3.17-rc1. Thus, this revert should be applied to every stable release back to 3.17. thanks, -Len
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 3fc286c..4b9635f 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1039,7 +1039,7 @@ static u32 vlv_wa_c0_ei(struct drm_i915_private *dev_priv, u32 pm_iir) if (pm_iir & GEN6_PM_RP_DOWN_EI_EXPIRED) { if (!vlv_c0_above(dev_priv, &dev_priv->rps.down_ei, &now, - dev_priv->rps.down_threshold)) + VLV_RP_DOWN_EI_THRESHOLD)) events |= GEN6_PM_RP_DOWN_THRESHOLD; dev_priv->rps.down_ei = now; } @@ -1047,7 +1047,7 @@ static u32 vlv_wa_c0_ei(struct drm_i915_private *dev_priv, u32 pm_iir) if (pm_iir & GEN6_PM_RP_UP_EI_EXPIRED) { if (vlv_c0_above(dev_priv, &dev_priv->rps.up_ei, &now, - dev_priv->rps.up_threshold)) + VLV_RP_UP_EI_THRESHOLD)) events |= GEN6_PM_RP_UP_THRESHOLD; dev_priv->rps.up_ei = now; } diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 70d9616..09f6aea 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -787,6 +787,8 @@ enum skl_disp_power_wells { #define CHV_BIAS_CPU_50_SOC_50 (3 << 2) #define VLV_CZ_CLOCK_TO_MILLI_SEC 100000 +#define VLV_RP_UP_EI_THRESHOLD 90 +#define VLV_RP_DOWN_EI_THRESHOLD 70 /* vlv2 north clock has */ #define CCK_FUSE_REG 0x8 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index db24f89..1923b6b 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4983,7 +4983,7 @@ static void valleyview_set_rps(struct drm_i915_private *dev_priv, u8 val) if (val != dev_priv->rps.cur_freq) { vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val); - if (!IS_CHERRYVIEW(dev_priv)) + if (!(IS_CHERRYVIEW(dev_priv) || IS_VALLEYVIEW(dev_priv))) gen6_set_rps_thresholds(dev_priv, val); }
Certain Baytrails, namely the 4 cpu core variants, have been plaqued by spurious system hangs, mostly occurring with light loads. Multiple bisects by various people point to a commit which changes the reclocking strategy for Baytrail to follow its bigger brethen: commit 8fb55197e64d ("drm/i915: Agressive downclocking on Baytrail") There is also a review comment attached to this commit from Deepak S on avoiding punit access on Cherryview and thus it is excluded on common reclocking path. By taking the same approach and omitting the punit access by not tweaking the thresholds when the hardware has been asked to move into different frequency, considerable gains in stability have been observed. With J1900 box, light render/video load would end up in system hang in usually less than 12 hours. With this patch applied, the cumulative uptime has now been 34 days without issues. To provoke system hang, light loads on both render and bsd engines in parallel have been used: glxgears >/dev/null 2>/dev/null & mpv --vo=vaapi --hwdec=vaapi --loop=inf vid.mp4 So far, author has not witnessed system hang with above load and this patch applied. Reports from the tenacious people at kernel bugzilla are also promising. Considering that the punit access frequency with this patch is considerably less, there is a possibility that this will push the, still unknown, root cause past the triggering point on most loads. Further work on investigating the punit accesses on byt is welcomed. References: https://bugzilla.kernel.org/show_bug.cgi?id=109051 Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Len Brown <len.brown@intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Jani Nikula <jani.nikula@intel.com> Cc: fritsch@xbmc.org Cc: miku@iki.fi Cc: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> CC: Michal Feix <michal@feix.cz> Cc: Hans de Goede <hdegoede@redhat.com> Cc: Deepak S <deepak.s@linux.intel.com> Cc: Jarkko Nikula <jarkko.nikula@linux.intel.com> Cc: <stable@vger.kernel.org> # v4.2+ Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> --- drivers/gpu/drm/i915/i915_irq.c | 4 ++-- drivers/gpu/drm/i915/i915_reg.h | 2 ++ drivers/gpu/drm/i915/intel_pm.c | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-)