[Intel-gfx] [PATCH] drm/i915: Acquire RPM wakeref for KMS atomic commit

[Chris Wilson]

On Mon, Dec 21, 2015 at 05:28:16PM +0100, Daniel Vetter wrote:
> On Mon, Dec 21, 2015 at 04:14:53PM +0000, Chris Wilson wrote:
> > On Mon, Dec 21, 2015 at 05:02:08PM +0100, Daniel Vetter wrote:
> > > On Sat, Dec 19, 2015 at 09:58:43AM +0000, Chris Wilson wrote:
> > > > Once all the preparations are complete, we are ready to write the
> > > > modesetting to the hardware. During this phase, we will be making lots
> > > > of HW register access, so take a top level wakeref to prevent an
> > > > unwarranted rpm suspend cycle mid-commit. Lower level functions should
> > > > be waking the individual power wells as required.
> > > > 
> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93439
> > > > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > > > Cc: Imre Deak <imre.deak at intel.com>
> > > 
> > > The original idea here was that doing this will paper over bugs in our rpm
> > > refcounting. There's also the problem that for modeset stuff we have all
> > > the power wells still to take care of.
> > > 
> > > For the referenced bug we should add a power domain check in the get hw
> > > state function instead, which is what we've been doing with all the other
> > > similar hw state readout functions too.
> > 
> > Agreed that there is another bug, but in the long term, we do want a
> > "prolonged" wakeref here. In the next evolution of the wakeref assertions,
> > we should be able to differentiate between the two (i.e. when we have
> > fine grained wakerefs around the hw access, we need to assert we hold one
> > of that type in the mmio accessor, rather than the prolonged version).
> 
> Why? If we enforce that I fear we lose implicit coverage. Currently if you
> touch any piece of modeset hw and don't have the corresponding long-time
> rpm/power well ref there's a good chance something will spot this. If we
> have a short-term rpm reference for everything we won't noticed these
> problems around the long-term rpm references any more.

The theory being that when we get autosuspend on the order of say a
hundred microseconds, we start to run into the real possibility of an
rpm cycle mid update.
 
> Imo the only thing short-term references are useful for is lockdep
> annotations to detect deadlocks, since lockdep requires that we drop a
> lock in the same process again. Long-term ones would simply do a
> might_lock in the get function to annotate the deadlock with rpm resume
> functions.

I'm thinking of a world where suspend-resume time are on the order of
microseconds and the rpm suspend interval not much greater.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre