On Sun, Aug 24, 2014 at 04:54:22PM +0100, Chris Wilson wrote:
> +static void execlists_submit(struct intel_engine_cs *engine)
> {
> - struct drm_i915_gem_object *ctx_obj0;
> - struct drm_i915_gem_object *ctx_obj1 = NULL;
> -
> - ctx_obj0 = to0->ring[engine->id].state;
> - BUG_ON(!ctx_obj0);
> - WARN_ON(!i915_gem_obj_is_pinned(ctx_obj0));
> -
> - execlists_ctx_write_tail(ctx_obj0, tail0);
> -
> - if (to1) {
> - ctx_obj1 = to1->ring[engine->id].state;
> - BUG_ON(!ctx_obj1);
> - WARN_ON(!i915_gem_obj_is_pinned(ctx_obj1));
> -
> - execlists_ctx_write_tail(ctx_obj1, tail1);
> - }
> -
> - execlists_elsp_write(engine, ctx_obj0, ctx_obj1);
> -
> - return 0;
> -}
> -
> -static void execlists_context_unqueue(struct intel_engine_cs *engine)
> -{
> - struct intel_ctx_submit_request *req0 = NULL, *req1 = NULL;
> - struct intel_ctx_submit_request *cursor = NULL, *tmp = NULL;
> - struct drm_i915_private *dev_priv = engine->dev->dev_private;
> + struct i915_gem_request *rq[2] = {};
> + int i = 0;
>
> assert_spin_locked(&engine->execlist_lock);
>
> - if (list_empty(&engine->execlist_queue))
> - return;
> -
> /* Try to read in pairs */
> - list_for_each_entry_safe(cursor, tmp, &engine->execlist_queue,
> - execlist_link) {
> - if (!req0) {
> - req0 = cursor;
> - } else if (req0->ctx == cursor->ctx) {
> + while (!list_empty(&engine->pending)) {
> + struct i915_gem_request *next;
> +
> + next = list_first_entry(&engine->pending,
> + typeof(*next),
> + engine_list);
> +
> + if (rq[i] == NULL) {
> +new_slot:
> + rq[i] = next;
> + } else if (rq[i]->ctx == next->ctx) {
> /* Same ctx: ignore first request, as second request
> * will update tail past first request's workload */
> - cursor->elsp_submitted = req0->elsp_submitted;
> - list_del(&req0->execlist_link);
> - queue_work(dev_priv->wq, &req0->work);
> - req0 = cursor;
> + rq[i] = next;
> } else {
> - req1 = cursor;
> - break;
> - }
> - }
> + if (++i == ARRAY_SIZE(rq))
> + break;
>
> - WARN_ON(req1 && req1->elsp_submitted);
> -
> - WARN_ON(execlists_submit_context(engine, req0->ctx, req0->tail,
> - req1 ? req1->ctx : NULL,
> - req1 ? req1->tail : 0));
> -
> - req0->elsp_submitted++;
> - if (req1)
> - req1->elsp_submitted++;
> -}
> -
> -static bool execlists_check_remove_request(struct intel_engine_cs *engine,
> - u32 request_id)
> -{
> - struct drm_i915_private *dev_priv = engine->dev->dev_private;
> - struct intel_ctx_submit_request *head_req;
> -
> - assert_spin_locked(&engine->execlist_lock);
> -
> - head_req = list_first_entry_or_null(&engine->execlist_queue,
> - struct intel_ctx_submit_request,
> - execlist_link);
> -
> - if (head_req != NULL) {
> - struct drm_i915_gem_object *ctx_obj =
> - head_req->ctx->ring[engine->id].state;
> - if (intel_execlists_ctx_id(ctx_obj) == request_id) {
> - WARN(head_req->elsp_submitted == 0,
> - "Never submitted head request\n");
> -
> - if (--head_req->elsp_submitted <= 0) {
> - list_del(&head_req->execlist_link);
> - queue_work(dev_priv->wq, &head_req->work);
> - return true;
> - }
> + goto new_slot;
> }
> +
> + list_move_tail(&next->engine_list, &engine->requests);
> }
Drat, this touches engine->requests in the wrong locking context. Will
need to stage the update to requests through an intermediate list to get
the locking correct.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
