On Fri, Jun 07, 2013 at 11:46:08PM +0300, Ville Syrj?l? wrote: > On Fri, Jun 07, 2013 at 10:03:20PM +0200, Daniel Vetter wrote: > > On Fri, Jun 07, 2013 at 07:32:56PM +0300, Ville Syrj?l? wrote: > > > On Wed, Jun 05, 2013 at 01:34:05PM +0200, Daniel Vetter wrote: > > > > Before I start to make a complete mess out of this, crank up > > > > the paranoia level a bit. > > > > > > > > Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch> > > > > --- > > > > drivers/gpu/drm/i915/intel_display.c | 9 ++++++++- > > > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > > > index 56fb6ed..39e977f 100644 > > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > > @@ -1440,6 +1440,7 @@ static void intel_disable_pch_pll(struct intel_crtc *intel_crtc) > > > > } > > > > > > > > assert_pch_pll_enabled(dev_priv, pll, NULL); > > > > + WARN_ON(!pll->on); > > > > if (--pll->active) > > > > return; > > > > > > Maybe a WARN_ON(pll->on) near the end of ironlake_enable_pch_pll() too? > > > > At the very end we set on = true, and the only non-error early return > > (when the active refcount is > 0 to begin with) has alreay a > > WARN_ON(!pll->on). Shouldn't that be good enough? > > Well I was just thinking that since we have this dual bookeeping w/ > active and on, maybe we want to warn if things go out of sync. Now I'm confused. I've tried to explain why I think we already have full checking of pll->on in enable_shared_dpll ... Can you maybe show in a diff where you'd want to add more? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch