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
