[Intel-gfx] i915: WARN_ON(val > dev_priv->rps.max_freq_softlimit)

[Chris Wilson]

On Wed, Jan 28, 2015 at 12:43:21AM -0500, Michael Auchter wrote:
> Testing out 3.19-rc6 on my 2014 Thinkpad X1 Carbon (Haswell) resulted in
> this WARN at boot (and pretty frequently afterwards):
> 
> WARNING: CPU: 0 PID: 989 at drivers/gpu/drm/i915/intel_pm.c:4377 gen6_set_rps+0x371/0x3c0()
> WARN_ON(val > dev_priv->rps.max_freq_softlimit)

[snip]
 
> I'm not at all familiar with this hardware, but I took a quick look into
> what changed with this commit for my laptop. Before the commit,
> rps.min_freq_softlimit is 4 (from rps.min_freq) and
> rps.max_freq_softlimit is 22.
> 
> After the commit, rps.min_freq_softlimit is set to the
> rps.efficient_freq value read from pcode, which is 34 on my laptop.
> So later when gen6_set_rps() is called with rps.min_freq_softlimit that
> warning is hit.
> 
> Any thoughts? It certainly seems fishy that this commit causes
> rps.min_freq_softlimit to be greater than rps.max_freq_softlimit.

Very fishy indeed. Moral of this story, never trust hw.

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3e630feb18e4..bbedd2901c54 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4007,7 +4007,10 @@ static void gen6_init_rps_frequencies(struct drm_device *dev)
                                        &ddcc_status);
                if (0 == ret)
                        dev_priv->rps.efficient_freq =
-                               (ddcc_status >> 8) & 0xff;
+                               clamp_t(u8,
+                                       (ddcc_status >> 8) & 0xff,
+                                       dev_priv->rps.min_freq,
+                                       dev_priv->rps.max_freq);
        }
 
        /* Preserve min/max settings in case of re-init */

But really it is probably just best to disable the query for hsw:

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3e630feb18e4..01bd508e81f6 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4001,7 +4001,7 @@ static void gen6_init_rps_frequencies(struct drm_device *dev)
        dev_priv->rps.max_freq          = dev_priv->rps.rp0_freq;
 
        dev_priv->rps.efficient_freq = dev_priv->rps.rp1_freq;
-       if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
+       if (IS_BROADWELL(dev)) {
                ret = sandybridge_pcode_read(dev_priv,
                                        HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL,
                                        &ddcc_status);

Paranoia says we do both.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre