[Intel-gfx] [PATCH 15/21] drm/i915: Reorder i915_add_request to separate the phases better

[Mika Kuoppala]

Chris Wilson <chris at chris-wilson.co.uk> writes:

> Let's avoid mixing sealing the hardware commands for the request and
> adding the request to the software tracking.
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

Reviewed-by: Mika Kuoppala <mika.kuoppala at intel.com>

> ---
>  drivers/gpu/drm/i915/i915_gem_request.c | 28 ++++++++++++++--------------
>  1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
> index c60a7b774cc6..34571e1eb328 100644
> --- a/drivers/gpu/drm/i915/i915_gem_request.c
> +++ b/drivers/gpu/drm/i915/i915_gem_request.c
> @@ -492,6 +492,8 @@ void __i915_add_request(struct drm_i915_gem_request *request, bool flush_caches)
>  	u32 reserved_tail;
>  	int ret;
>  
> +	trace_i915_gem_request_add(request);
> +
>  	/*
>  	 * To ensure that this call will not fail, space for its emissions
>  	 * should already have been reserved in the ring buffer. Let the ring
> @@ -515,20 +517,6 @@ void __i915_add_request(struct drm_i915_gem_request *request, bool flush_caches)
>  		WARN(ret, "engine->emit_flush() failed: %d!\n", ret);
>  	}
>  
> -	trace_i915_gem_request_add(request);
> -
> -	/* Seal the request and mark it as pending execution. Note that
> -	 * we may inspect this state, without holding any locks, during
> -	 * hangcheck. Hence we apply the barrier to ensure that we do not
> -	 * see a more recent value in the hws than we are tracking.
> -	 */
> -	request->emitted_jiffies = jiffies;
> -	request->previous_seqno = engine->last_submitted_seqno;
> -	engine->last_submitted_seqno = request->fence.seqno;
> -	i915_gem_active_set(&engine->last_request, request);
> -	list_add_tail(&request->link, &engine->request_list);
> -	list_add_tail(&request->ring_link, &ring->request_list);
> -
>  	/* Record the position of the start of the breadcrumb so that
>  	 * should we detect the updated seqno part-way through the
>  	 * GPU processing the request, we never over-estimate the
> @@ -549,6 +537,18 @@ void __i915_add_request(struct drm_i915_gem_request *request, bool flush_caches)
>  		  "for adding the request (%d bytes)\n",
>  		  reserved_tail, ret);
>  
> +	/* Seal the request and mark it as pending execution. Note that
> +	 * we may inspect this state, without holding any locks, during
> +	 * hangcheck. Hence we apply the barrier to ensure that we do not
> +	 * see a more recent value in the hws than we are tracking.
> +	 */
> +	request->emitted_jiffies = jiffies;
> +	request->previous_seqno = engine->last_submitted_seqno;
> +	engine->last_submitted_seqno = request->fence.seqno;
> +	i915_gem_active_set(&engine->last_request, request);
> +	list_add_tail(&request->link, &engine->request_list);
> +	list_add_tail(&request->ring_link, &ring->request_list);
> +
>  	i915_gem_mark_busy(engine);
>  
>  	local_bh_disable();
> -- 
> 2.9.3