Re: [PATCH] drm/i915: fix plane/cursor handling when runtime suspended

Daniel Vetter
2014-08-13T07:59:07Z

On Tue, Aug 12, 2014 at 09:51:13PM +0100, Chris Wilson wrote:
> On Tue, Aug 12, 2014 at 10:37:21PM +0200, Daniel Vetter wrote:
> > On Tue, Aug 12, 2014 at 10:30 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > > On Tue, Aug 12, 2014 at 10:19:20PM +0200, Daniel Vetter wrote:
> > >> On Tue, Aug 12, 2014 at 9:33 PM, Paulo Zanoni <przanoni@gmail.com> wrote:
> > >> > 2014-08-12 16:28 GMT-03:00 Chris Wilson <chris@chris-wilson.co.uk>:
> > >> >> On Tue, Aug 12, 2014 at 04:12:38PM -0300, Paulo Zanoni wrote:
> > >> >>> But we just get/put RPM around this function, not for the whole time
> > >> >>> while the object is pinned.
> > >> >>
> > >> >> Ah misread, saw pin->get, unpin->put and assumed the symmetry. But why
> > >> >> unpin then? It doesn't touch any hardware registers.
> > >> >
> > >> > Only because Daniel asked it on a conversation we had on IRC, and I
> > >> > automatically assumed the patch would be rejected if I didn't include
> > >> > it :)
> > >> >
> > >> > Since both you and VIlle pointed that out, I should probably submit
> > >> > yet another version, without the unpin part, and let Daniel choose
> > >> > which one to merge...
> > >>
> > >> Hm, I've indeed forgotten about the lazy unbinding. But that poses the
> > >> question about the final bo unref. For example:
> > >> 1) create bo, gtt mmap it to force it into existence (and into the global gtt)
> > >> 2) unmap binding
> > >> 3) wait for rpm entry
> > >> 4) unref bo ... causing pte writes for the global gtt unbinding while
> > >> runtime suspended or not?
> > >>
> > >> boom or not boom?
> > >>
> > >> Maybe the bug is simply in a different function ;-)
> > >
> > > Yes. If you get serious about it, you will want to move the lazy stuff
> > > into its own workqueue to be run the next time the device is awake.
> >
> > 4b) shrinker happens and unbinds (potentially purgeable) buffer objects.
> >
> > In that case I don't think the core mm would be happy if we'd
> > indefinitely delay this until someone wiggles the mouse.
>
> You underestimate just how much we can delay it ;-) But for your next
> trick, you could unbind the buffer without touching the ptes since the
> gpu is not using those pages... Diminishing returns I guess.

That's actually something I've considered - on gen6+ we don't use the
global gtt any more for rendering, so it's fully isolated from whatever
userspace can get at. Well ignoring an icky regression from full ppgtt for
the aliasing ppgtt binding rules.

So we /could/ just leave the stale pte hanging in the air forever. But I'm
not sure whether we want to do that for general robustness reasons.
Clearing all ptes on device wake-up isn't an option since it takes too
much time, and delaying the clearing doesn't look like worth it from a
complexity pov.

> > Especially if
> > the compositor wants that memory to render the frame it needs to
> > switch everything on again ...
>
> But's true without rpm anyway. It would need to enable the device to
> render, whether or not the system is thrashing.

Yeah, which is also why I don't think just waking the device in the shrinker/bo_free
callback is harmful - very likely we didn't wake it for nothing anyway. Oh
any my scenario can be fixed with some software rendering into an Xshm or
so, and if you assume something else running overnight has thrashed all
the memory to swap.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch