Re: [PATCH 1/3] drm/i915: Refactor execlists default context pinning

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 15/04/16 13:16, Chris Wilson wrote:
> On Fri, Apr 15, 2016 at 12:54:33PM +0100, Tvrtko Ursulin wrote:
>> From: Chris Wilson <chris@chris-wilson.co.uk>
>>
>> Refactor pinning and unpinning of contexts, such that the default
>> context for an engine is pinned during initialisation and unpinned
>> during teardown (pinning of the context handles the reference counting).
>> Thus we can eliminate the special case handling of the default context
>> that was required to mask that it was not being pinned normally.
>>
>> v2:
>>    * Rebase on nightly;
>>    * put back pin in execlists_context_queue. (Tvrtko Ursulin)
>
> No, we don't want to pin in execlists_context_queue!
>
> We pin to acquire the request and then want to keep that pin for as long
> as possible. For execlists, this means only dropping it when the
> hardware is finished (which will be in the following request's
> retirement).
>
> Since we can handle an error during queue, the pin has to be upfront.

As longs as we unpin in both request free _and_ retire worker 
(intel_execlists_retire_requests) I don't see how pinning only in 
alloc_extras work.

So it looked to me your patch was unbalanced in that respect.

I have patches under the "eliminate the execlists retired queue" 
umbrella which will do what you are saying but that has some more 
prerequisites and we agreed to handle the two series separately.

Regards,

Tvrtko

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