[Intel-gfx] [PATCH 05/10] drm/i915: Trim the retired request queue after submitting

[Tvrtko Ursulin]

On 15/01/2018 21:24, Chris Wilson wrote:
> If we submit a request and see that the previous request on this
> timeline was already signaled, we first do not need to add the
> dependency tracker for that completed request and secondly we know that
> we there is then a large backlog in retiring requests affecting this
> timeline. Given that we just submitted more work to the HW, now would be
> a good time to catch up on those retirements.

How can we be sure there is a large backlog? It may just be that the 
submission frequency combined with request duration is just right to 
always see even a solitary previous completed request, no?

> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> ---
>   drivers/gpu/drm/i915/i915_gem_request.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
> index e6d4857b1f78..6a143099cea1 100644
> --- a/drivers/gpu/drm/i915/i915_gem_request.c
> +++ b/drivers/gpu/drm/i915/i915_gem_request.c
> @@ -1019,7 +1019,7 @@ void __i915_add_request(struct drm_i915_gem_request *request, bool flush_caches)
>   
>   	prev = i915_gem_active_raw(&timeline->last_request,
>   				   &request->i915->drm.struct_mutex);
> -	if (prev) {
> +	if (prev && !i915_gem_request_completed(prev)) {
>   		i915_sw_fence_await_sw_fence(&request->submit, &prev->submit,
>   					     &request->submitq);

This makes sense.

>   		if (engine->schedule)
> @@ -1055,6 +1055,9 @@ void __i915_add_request(struct drm_i915_gem_request *request, bool flush_caches)
>   	local_bh_disable();
>   	i915_sw_fence_commit(&request->submit);
>   	local_bh_enable(); /* Kick the execlists tasklet if just scheduled */
> +
> +	if (prev && i915_gem_request_completed(prev))
> +		i915_gem_request_retire_upto(prev);

And here I'm a bit surprised that you want to penalize the submission 
path with house-keeping - assuming cases when there really is a big 
backlog of completed requests. But since it is after the tasklet 
kicking, I suppose the effect on submission latency is somewhat 
mediated. Unless the caller wants to submit many requests rapidly. Hm.. 
retire at execbuf time seems to be coming in and out, albeit in a more 
controlled fashion with this.

Regards,

Tvrtko

>   }
>   
>   static unsigned long local_clock_us(unsigned int *cpu)
>