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

[Daniel, Thomas]

> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter at ffwll.ch] On Behalf Of Daniel
> Vetter
> Sent: Monday, November 24, 2014 2:25 PM
> To: Daniel, Thomas
> Cc: intel-gfx at lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH v5 3/4] drm/i915/bdw: Pin the context
> backing objects to GGTT on-demand
> 
> On Thu, Nov 13, 2014 at 10:28:10AM +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
> >
> > v5: Remove unpin_lock as all pinning and unpinning is done with the
> > struct mutex already locked.  Add WARN_ONs to make sure this is the case
> in future.
> >
> > Issue: VIZ-4277
> > Signed-off-by: Oscar Mateo <oscar.mateo at intel.com>
> > Signed-off-by: Thomas Daniel <thomas.daniel at intel.com>
> 
> This patch here scored a regression (leak in the module unload path), please
> address it asap. Deadline for regressions should be 1 week, then I'll just drop
> the patch or apply the revert. That includes review and everything.
> 
> https://bugs.freedesktop.org/show_bug.cgi?id=86507 

Leak identified.  The fix is simple.
Do you want a v6 or a follow-up patch?

Cheers,
Thomas.