Hi,
On 10-11-17 15:56, Chris Wilson wrote:
Quoting Hans de Goede (2017-11-09 18:51:01)
Bay Trail devices are known to hang when changing the frequency often,
this is discussed in great length in:
https://bugzilla.kernel.org/show_bug.cgi?id=109051
Commit 6067a27d1f01 ("drm/i915: Avoid tweaking evaluation thresholds
on Baytrail v3") is an attempt to workaround this. Several users in
bko109051 report that an earlier version of this patch, v1:
https://bugzilla.kernel.org/attachment.cgi?id=251471
Works better for them and they still see hangs with the merged v3.
Comparing the 2 versions shows that they are indeed not equivalent,
v1 not only skips writing the GEN6_RP* registers from valleyview_set_rps,
as v3 does. It also contained these modifications to i915_irq.c:
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;
}
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;
}
Which use less aggressive up/down thresholds, which results in less
GEN6_PM_RP_*_THRESHOLD events and thus in less calls to intel_set_rps() ->
valleyview_set_rps() -> vlv_punit_write(PUNIT_REG_GPU_FREQ_REQ).
With the last call being the likely cause of the hang.
This commit hardcodes the threshold_up and _down values for Bay Trail to
less aggressive values, reducing the amount of clock frequency changes,
thus avoiding the hangs some people are still seeing with the merged fix.
Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=109051
Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
---
drivers/gpu/drm/i915/i915_reg.h | 3 +++
drivers/gpu/drm/i915/intel_pm.c | 5 ++++-
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 68a58cce6ab1..2561af075ebb 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1355,6 +1355,9 @@ enum i915_power_well_id {
#define VLV_BIAS_CPU_125_SOC_875 (6 << 2)
#define CHV_BIAS_CPU_50_SOC_50 (3 << 2)
+#define VLV_RP_UP_EI_THRESHOLD 90
+#define VLV_RP_DOWN_EI_THRESHOLD 70
+
/* vlv2 north clock has */
#define CCK_FUSE_REG 0x8
#define CCK_FUSE_HPLL_FREQ_MASK 0x3
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 01966b89be14..177b6caa0a38 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6096,8 +6096,11 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val)
/* When byt can survive without system hang with dynamic
* sw freq adjustments, this restriction can be lifted.
*/
- if (IS_VALLEYVIEW(dev_priv))
+ if (IS_VALLEYVIEW(dev_priv)) {
+ threshold_up = VLV_RP_UP_EI_THRESHOLD;
+ threshold_down = VLV_RP_DOWN_EI_THRESHOLD;
Basically you want a limit on the frequency of ... pcode access?
As presented, you ultimately do not trust any write and the only
solution is to disable all writes. No RPS whatsoever, run at max and
hope rc6 works (maybe even decrease the rc6 threshold).
One of the ideas we floated was moving the pcode access to a worker and
ratelimiting the updates.
Yes ratelimiting the amount of vlv_punit_write(PUNIT_REG_GPU_FREQ_REQ)
calls is also something which came to my mind as an alternative solution.
As I tried to explain in the commit message my main inspiration for this
patch was that people were reporting different success rates with v1
and v3 of the Commit 6067a27d1f01 ("drm/i915: Avoid tweaking evaluation thresholds
on Baytrail v3") patch.
Regards,
Hans
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
