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

Daniel Vetter daniel at ffwll.ch
Fri Jun 7 22:03:20 CEST 2013


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?

> 
> >  
> > @@ -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



More information about the Intel-gfx mailing list