Re: [PATCH 00/14] drm/i915: IRQ work for chv mostly

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: intel-gfx <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 00/14] drm/i915: IRQ work for chv mostly
Date: Tue, 4 Nov 2014 18:42:16 +0200	[thread overview]
Message-ID: <20141104164216.GL10649@intel.com> (raw)
In-Reply-To: <CAKMK7uFEvG7BUxOY6uuFdQ+m0NpUzOQ+0qTyKcVbPeH=Pde7MA@mail.gmail.com>

On Tue, Nov 04, 2014 at 01:40:05PM +0100, Daniel Vetter wrote:
> On Tue, Nov 4, 2014 at 1:21 PM, Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> >> >   drm/i915: Refactor vlv_display_irq_uninstall()
> >>
> >> Except this, I think consens is that we don't need it?
> >
> > No, it's needed by the later patches. Trying to replace the vlv/chv
> > uninstall() hooks with reset() is best left for another series IMO.
> > And doing that involves more than just reviewing the the display irq
> > install/uninstall paths. Eg. currently VLV_MASTER_IER handling is
> > very inconsistent.
> 
> Well I think we should just open-code it for chv and not do this
> extraction.

I don't see what the extraction hurts. If someone manages to unify it
more that's fine, but in the meantime it could at least prevent random
bugs from cropping up in one of the relevant platforms. Wouldn't be the
first time someone forgets about chv.

> All the complexity here is due to vlv runtime pm, and
> somehow I still think that this should work more magically. At least
> it should be possible if we start to handle power domains in the
> system s/r code instead of just force-enabling them all.

Well the same complexity is already there during driver init. So it just
comes from the power wells. Trying to avoid the force enable all thing
is a bit difficult because we can't really reconstruct the required
refcounts until we've read out the entire modeset state.

I guess one option would be to read out the current power well states
and convert that into temporary refcounts that get dropped after the
init is done. That would at least guarantee we don't prematurely turn
off anything that the BIOS already lit up. And as Imre pointed out to me,
we should then definitely add the unclaimed register check for vlv/chv
so that we might catch more easily any missing power well references
in the init path.

> So my idea was to drop this one and rebase the remaining few patches
> on top. That way we have a nice reminder that there's still work to
> do.

Such rebasing sounds like work without benefit to me.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx