On Mon, Apr 11, 2016 at 10:10:56AM +0100, Tvrtko Ursulin wrote:
>
> On 08/04/16 15:57, Chris Wilson wrote:
> >On Fri, Apr 08, 2016 at 02:54:58PM +0100, Tvrtko Ursulin wrote:
> >>@@ -615,11 +613,6 @@ static void execlists_context_queue(struct
> >>drm_i915_gem_request *request)
> >> struct drm_i915_gem_request *cursor;
> >> int num_elements = 0;
> >>
> >>- if (request->ctx != request->i915->kernel_context)
> >>- intel_lr_context_pin(request->ctx, engine);
> >>-
> >>- i915_gem_request_reference(request);
> >>-
> >> spin_lock_bh(&engine->execlist_lock);
> >>
> >> list_for_each_entry(cursor, &engine->execlist_queue, execlist_link)
> >>@@ -636,11 +629,12 @@ static void execlists_context_queue(struct
> >>drm_i915_gem_request *request)
> >> if (request->ctx == tail_req->ctx) {
> >> WARN(tail_req->elsp_submitted != 0,
> >> "More than 2 already-submitted reqs queued\n");
> >>- list_move_tail(&tail_req->execlist_link,
> >>- &engine->execlist_retired_req_list);
> >>+ list_del(&tail_req->execlist_link);
> >>+ i915_gem_request_unreference(tail_req);
> >> }
> >> }
> >>
> >>+ i915_gem_request_reference(request);
> >> list_add_tail(&request->execlist_link, &engine->execlist_queue);
> >
> >If you want to get truly radical, we do not need the ref on the request
> >until it is submitted to hardware. (As the request cannot be retired
> >until it has done so, it can leave the execlist_queue until we commit it
> >to hw, or perform the cancel).
>
> Don't know. It is simple and nice that reference is tied to presence
> on execlist_queue.
>
> More importantly, the patch as presented has a flaw that it
> dereferences req->ctx from execlists_check_remove_request where the
> context pin may have disappeared already due context complete
> interrupts getting behind when coallescing.
>
> I will need to cache ctx_id in the request I think. It is fine do to
> that since ctx id must be unique and stable for a request.
For comparison, tracking by requests rather than contexts:
https://cgit.freedesktop.org/~ickle/linux-2.6/commit/?h=tasklet&id=e23791ef88636aa6e3f850b61ab2c4e027af0e52
-Chris
>
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
