[Intel-gfx] [PATCH 03/31] drm/i915: lock down pch pll accouting some more

[Ville Syrjälä]

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.

> 
> > 
> > >  
> > > @@ -3031,12 +3032,18 @@ static void intel_put_pch_pll(struct intel_crtc *intel_crtc)
> > >  	if (pll == NULL)
> > >  		return;
> > >  
> > > +	WARN_ON(!intel_crtc->config.has_pch_encoder);
> > 
> > Doesn't that trigger if we switch directly from PCH to CPU eDP?
> 
> I've missed this case in testing somehow, and it's indeed broken. I don't
> hit the WARN here, but that's just because I've broken the refcounting
> somewhere.
> 
> At least I've got plenty of backtraces, so the level of paranoia seems to
> be correct ;-)
> 
> I'll fix this up and resend.
> -Daniel
> 
> > 
> > > +
> > >  	if (pll->refcount == 0) {
> > >  		WARN(1, "bad PCH PLL refcount\n");
> > >  		return;
> > >  	}
> > >  
> > > -	--pll->refcount;
> > > +	if (--pll->refcount == 0) {
> > > +		WARN_ON(pll->on);
> > > +		WARN_ON(pll->active);
> > > +	}
> > > +
> > >  	intel_crtc->pch_pll = NULL;
> > >  }
> > >  
> > > -- 
> > > 1.7.11.7
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx at lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > -- 
> > Ville Syrjälä
> > Intel OTC
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel OTC