[Intel-gfx] [PATCH 1/2] drm/i915: use effective_size for ringbuffer calculations

[Chris Wilson]

On Fri, Jun 12, 2015 at 06:09:07PM +0100, Dave Gordon wrote:
> When calculating the available space in a ringbuffer, we should
> use the effective_size rather than the true size of the ring.
> 
> v2: rebase to latest drm-intel-nightly
> v3: rebase to latest drm-intel-nightly
> 
> Signed-off-by: Dave Gordon <david.s.gordon at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_lrc.c        |    5 +++--
>  drivers/gpu/drm/i915/intel_ringbuffer.c |    9 ++++++---
>  2 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 9b74ffa..454e836 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -699,7 +699,7 @@ static int logical_ring_wait_for_space(struct intel_ringbuffer *ringbuf,
>  
>  		/* Would completion of this request free enough space? */
>  		space = __intel_ring_space(request->postfix, ringbuf->tail,
> -					   ringbuf->size);
> +					   ringbuf->effective_size);
>  		if (space >= bytes)
>  			break;
>  	}
> @@ -711,7 +711,8 @@ static int logical_ring_wait_for_space(struct intel_ringbuffer *ringbuf,
>  	if (ret)
>  		return ret;
>  
> -	ringbuf->space = space;
> +	/* Update ring space after wait+retire */
> +	intel_ring_update_space(ringbuf);

Does the function not do what it says on the tin? At least make it seem
like you are explaining your reasoning, not documenting the following
function.

/*
 * Having waited for the request, query the HEAD of most recent retired
 * request and use that for our space calcuations.
 */

However, that makes an incorrect assumption about the waiter. Given that
the current code is written such that ringbuf->last_retired_head =
request->postfix and that space is identical to the repeated
calculation, what is your intention exactly?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre