[Intel-gfx] [PATCH] drm/i915: Fix context/engine cleanup order

[Chris Wilson]

On Mon, Dec 14, 2015 at 05:13:43PM +0000, Chris Wilson wrote:
> On Mon, Dec 14, 2015 at 04:30:04PM +0000, Nick Hoath wrote:
> > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> > index 900ffd0..7df3c7a 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_context.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> > @@ -431,17 +431,22 @@ void i915_gem_context_fini(struct drm_device *dev)
> >  		i915_gem_object_ggtt_unpin(dctx->legacy_hw_ctx.rcs_state);
> >  	}
> >  
> > -	for (i = 0; i < I915_NUM_RINGS; i++) {
> > +	for (i = I915_NUM_RINGS; --i >= 0;) {
> >  		struct intel_engine_cs *ring = &dev_priv->ring[i];
> >  
> >  		if (ring->last_context)
> >  			i915_gem_context_unreference(ring->last_context);
> >  
> > -		ring->default_context = NULL;
> >  		ring->last_context = NULL;
> >  	}
> >  
> >  	i915_gem_context_unreference(dctx);
> > +
> > +	for (i = I915_NUM_RINGS; --i >= 0;) {
> > +		struct intel_engine_cs *ring = &dev_priv->ring[i];
> > +
> > +		ring->default_context = NULL;
> > +	}
> >  }
> 
> Why?

Ok, as you say intel_lrc needs to iterate over all rings to look at the
default context in context_free. Feels odd, and breaks the onion of
context_init / context_fini. The pecularity is in lrc and I would push
the oddness back there.

That engine->default_context is pinned and mapped is only because of the
HWS and for no other reason does it exist (seqno writes are to address
not relative the per-context HWS index). You could simply allocate a
page for each engine and use that as the global HWS irrespective of
contexts.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre