On Tue, 25 Jun 2013 19:59:28 +0000 Shuah Khan <shuah.kh at 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 at 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");