Re: [PATCH] drm/i915: WARN if interrupts aren't on in en/disable_pipestat

Chris Wilson
2014-08-27T11:28:48Z

On Wed, Aug 27, 2014 at 12:23:47PM +0200, Daniel Vetter wrote:
> On Wed, Aug 27, 2014 at 09:51:58AM +0100, Chris Wilson wrote:
> > On Wed, Aug 27, 2014 at 10:43:37AM +0200, Daniel Vetter wrote:
> > > Now that vlv has runtime pm we kinda should check for that like on the
> > > pch split platforms. Looks like this was simply lost in the vlv rpm
> > > enabling.
> >
> > Is there a reason why setting up the pipestat prior to enabling
> > interrupts is verboten, or is it just not done today?
>
> The similar warning on pch platforms was added to catch runtime pm bugs
> where someone enabled some interrupt source, but didn't hold a runtime pm
> reference. That's just not going to work. So motivation for this patch is
> vlv/chv runtime pm really.
>
> But I also think that this check provides value for driver
> load/resume/suspend since if we change interrupt settings after the
> interrupts have been disabled or before enabling something is at least
> fishy. At least insofar as that our irq install hooks nowadays overwrite
> all of them with default values.

That's a better blurp. But doesn't it sound a little overly generic just
to keep whether or not pipestat_enable is called after uninstall or
before install? Since we track the expected register value, could we
just semi-randomly check that the register matches that value?
-Chris

--
Chris Wilson, Intel Open Source Technology Centre