On Mon, Dec 21, 2015 at 07:21:33PM +0100, Daniel Vetter wrote: > On Mon, Dec 21, 2015 at 04:37:41PM +0000, Chris Wilson wrote: > > 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@xxxxxxxxxxxxxxxxxx> > > > > > > Cc: Imre Deak <imre.deak@xxxxxxxxx> > > > > > > > > > > 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. > > That's why we need a bit of hystersis to avoid that. Statistics rule of > thumb is to only suspend once you've spent about as much time idle as it > would take you to suspend/resume. And since we do a few global updates we > actually acquire new power wells before old ones, so as long as you don't > do 2 ioctls to change configurations it's impossible to accidentally > suspend in between. The rest is just appropriately tuning defaults (and > still setting it to 0 in igt for maximum nastiness). I still feel we are talking about complementary techiniques for achieving the same thing - and that there will always be places in the code where we want to say "do not thing about suspending until the end of this big function". -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
