[Intel-gfx] [PATCH 3/4] drm/i915/bdw: Pin the context backing objects to GGTT on-demand

[Daniel Vetter]

On Mon, Nov 03, 2014 at 05:00:35PM +0000, Daniel, Thomas wrote:
> > -----Original Message-----
> > From: Daniel Vetter [mailto:daniel.vetter at ffwll.ch] On Behalf Of Daniel
> > Vetter
> > Sent: Monday, November 03, 2014 4:54 PM
> > To: Daniel, Thomas
> > Cc: intel-gfx at lists.freedesktop.org; shuang.he at linux.intel.com
> > Subject: Re: [Intel-gfx] [PATCH 3/4] drm/i915/bdw: Pin the context backing
> > objects to GGTT on-demand
> > 
> > On Wed, Oct 29, 2014 at 09:52:52AM +0000, Thomas Daniel wrote:
> > > From: Oscar Mateo <oscar.mateo at intel.com>
> > >
> > > Up until now, we have pinned every logical ring context backing object
> > > during creation, and left it pinned until destruction. This made my
> > > life easier, but it's a harmful thing to do, because we cause
> > > fragmentation of the GGTT (and, eventually, we would run out of space).
> > >
> > > This patch makes the pinning on-demand: the backing objects of the two
> > > contexts that are written to the ELSP are pinned right before
> > > submission and unpinned once the hardware is done with them. The only
> > > context that is still pinned regardless is the global default one, so
> > > that the HWS can still be accessed in the same way (ring->status_page).
> > >
> > > v2: In the early version of this patch, we were pinning the context as
> > > we put it into the ELSP: on the one hand, this is very efficient
> > > because only a maximum two contexts are pinned at any given time, but
> > > on the other hand, we cannot really pin in interrupt time :(
> > >
> > > v3: Use a mutex rather than atomic_t to protect pin count to avoid races.
> > > Do not unpin default context in free_request.
> > >
> > > v4: Break out pin and unpin into functions.  Fix style problems
> > > reported by checkpatch
> > >
> > > Issue: VIZ-4277
> > 
> > This doesn't really do the full task since the integration with the shrinker and
> > related igt testcases are missing. What's your plane here?
> This is a rebase and bug fix of the original patch to unblock execlists
> enabling.  Plan is to address the rest of the issues after the big
> seqno->request rearchitecting change goes in.

Hm, ok makes sense. Please find a review victim for the remaining 3
patches, preferrably someon who digs around in gem too and is not from the
vpg london team (to spread the knowledge of all this a bit).

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch