Re: [PATCH v2, 2/4] drm/i915: simplify testing for the global default context

From: Dave Gordon <david.s.gordon@intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	intel-gfx@lists.freedesktop.org, daniel.vetter@ffwll.ch
Subject: Re: [PATCH v2, 2/4] drm/i915: simplify testing for the global default context
Date: Mon, 4 Jan 2016 17:43:10 +0000	[thread overview]
Message-ID: <568AAF2E.5010209@intel.com> (raw)
In-Reply-To: <20151223210237.GA864@nuc-i3427.alporthouse.com>

On 23/12/15 21:02, Chris Wilson wrote:
> On Wed, Dec 23, 2015 at 07:33:53PM +0000, Dave Gordon wrote:
>> There are quite a number of places where the driver tests whether
>> a given context is or is not the global default context, usually by
>> checking whether an engine's default_pointer points to the context.
>> Now that we have a 'is_global_default' flag in the context itself,
>> all these tests these can be rewritten to use it. This makes the
>> logic more obvious, and usually saves at least one memory reference.
>> In addition, with these uses eliminated, a future patch will be able
>> to get rid of engine::default_context entirely.
>
> All the execlists use of ctx != ring->default_context stems from a
> misstep in execlists - if you stop treating that default_context as
> special during request processing and just take the pin/unpin at
> init/fini of the ring, they all disappear.

We do already pin/unpin the default context at creation/deletion; AFAICS 
the extra tests are probably an attempt not to do an extra pin/unpin on 
an object which is by definition already pinned. And I'd be quite happy 
to get rid of those tests, and just issue a pin for *every* request 
issues on a context -- indeed, I think Nick may have just such a patch. 
But his changes are blocked on getting the elimination of 
ring->default_context (patch 4 of THIS set) merged first, since having 
those backpointers dictates the order of creation and destruction.

> And please stop conflating is_global_context when we have already a very
> good expression for when the context is owned by no file.
> -Chris

No existing code uses that as a test. And patch 1 of this set dropped 
file_priv anyway, since there was no actual use for it anywhere at all!
This way we don't risk having dangling pointers to deallocated file objects.

.Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx