On 05/04/16 15:09, Chris Wilson wrote:
On Tue, Apr 05, 2016 at 02:42:16PM +0100, Tvrtko Ursulin wrote:
@@ -587,9 +587,6 @@ static int execlists_context_queue(struct
drm_i915_gem_request *request)
      struct drm_i915_gem_request *cursor;
      int num_elements = 0;

-    if (request->ctx != ring->default_context)
-        intel_lr_context_pin(request);
-

Since you remove LRC pin from queue, the lifetime is now either:

1. From request create to cancel.
2. From request create to execlist retirement.

Would it be more logical to leave the LRC pin in queue, but remove it
>from request creation instead? That would make the LRC pin lifetime only
a single possibility, from queue to execlist retire.

Well what we actually need in request allocation is pinning the
ringbuffer. At the moment we do that by pinning the request. We also
need to pin the VM in order to manipulate it. We could leave pinning the
logical context object til actual submission.

Hmmm, yes. Have to think how complicated or not would that be.

I felt was so close in getting rid of execlist_retired_req_queue,
using this patch as a starting point, when I realised this patch
does not play nicely with the GuC. Back to the drawing board. :(

If you mean what happens if the GuC executes requests out-of-order - it
can't in the current model since we only have a single timeline and that
would break it badly - then nothing changes. We are only moving the
release in time, not decoupling it from any serialisation.

No, there is no LRC unpin, it is unbalanced in the GuC mode with this patch. Unless I am missing something... ?

Regards,

Tvrtko

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

Reply via email to