Re: Linux 3.10-rc7

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Tue, 25 Jun 2013 19:59:28 +0000
Shuah Khan <shuah.kh@samsung.com> wrote:

> On 06/25/2013 01:52 PM, Jesse Barnes wrote:
> > On Tue, 25 Jun 2013 21:37:37 +0200
> > Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> >
> 
> >>
> >> Adding more lists to cc + Jesse since he's the guilty one for the
> >> vt-switchless state restore stuff.
> >
> > Yeah, looks like we don't fetch the PLL state on resume from hibernate,
> > leading to this warning.  The refcount is nonzero, indicating the pll
> > is in use, but the active field is clear, which means we're missing an
> > update somewhere.
> >
> > Shuah, just to confirm, does your resume actually work ok aside from
> > the warning?  I *think* it's harmless in this case, but does indicate a
> > real bug in our state tracking... trying to come up with a patch now.
> >
> > Thanks,
> >
> 
> Resume works just fine. I see it take longer for it to suspend compared 
> to 3.9.7 and then resumes just fine. Suspend taking longer very well 
> could be because of this warn_on. Other than this warn_on I haven't 
> noticed any other problems.

Here's the patch I'm testing now, can you give it a try?

-- 
Jesse Barnes, Intel Open Source Technology Center

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 56746dc..df0caf0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9289,6 +9289,49 @@ void i915_redisable_vga(struct drm_device *dev)
 	}
 }
 
+static void ironlake_crtc_pll_get(struct intel_crtc *crtc)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 dpll_sel;
+
+	if (HAS_PCH_IBX(dev_priv->dev))
+		crtc->pch_pll = &dev_priv->pch_plls[crtc->pipe];
+
+	if (HAS_PCH_CPT(dev_priv->dev)) {
+		dpll_sel = I915_READ(PCH_DPLL_SEL);
+
+		switch (crtc->pipe) {
+		case PIPE_A:
+			if ((dpll_sel & TRANSA_DPLL_ENABLE) &&
+			    (dpll_sel & TRANSA_DPLLB_SEL))
+				crtc->pch_pll = &dev_priv->pch_plls[1];
+			else if (dpll_sel & TRANSA_DPLL_ENABLE)
+				crtc->pch_pll = &dev_priv->pch_plls[0];
+			break;
+		case PIPE_B:
+			if ((dpll_sel & TRANSB_DPLL_ENABLE) &&
+			    (dpll_sel & TRANSB_DPLLB_SEL))
+				crtc->pch_pll = &dev_priv->pch_plls[1];
+			else if (dpll_sel & TRANSB_DPLL_ENABLE)
+				crtc->pch_pll = &dev_priv->pch_plls[0];
+			break;
+		case PIPE_C:
+			if ((dpll_sel & TRANSC_DPLL_ENABLE) &&
+			    (dpll_sel & TRANSC_DPLLB_SEL))
+				crtc->pch_pll = &dev_priv->pch_plls[1];
+			else if (dpll_sel & TRANSC_DPLL_ENABLE)
+				crtc->pch_pll = &dev_priv->pch_plls[0];
+			break;
+		default:
+			BUG();
+		}
+	}
+
+	crtc->pch_pll->refcount++;
+	crtc->pch_pll->active = 1;
+}
+
 /* Scan out the current hw modeset state, sanitizes it and maps it into the drm
  * and i915 state tracking structures. */
 void intel_modeset_setup_hw_state(struct drm_device *dev,
@@ -9346,6 +9389,9 @@ setup_pipes:
 
 		crtc->base.enabled = crtc->active;
 
+		if (crtc->active && HAS_PCH_SPLIT(dev))
+			ironlake_crtc_pll_get(crtc);
+
 		DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
 			      crtc->base.base.id,
 			      crtc->active ? "enabled" : "disabled");