[Intel-gfx] [PATCH 3/3] drm/i915: s/seqno/request/ tracking inside objects

[Chris Wilson]

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