Re: [PATCH v6 15/25] drm/i915: Preallocate enough space for the average request

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 26/04/16 21:06, Chris Wilson wrote:
> Rather than being interrupted when we run out of space halfway through
> the request, and having to restart from the beginning (and returning to
> userspace), flush a little more free space when we prepare the request.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_lrc.c        |  7 +++++++
>   drivers/gpu/drm/i915/intel_ringbuffer.c | 16 +++++++++++++++-
>   2 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 01517dd7069b..b5c2c1931a5f 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -700,6 +700,12 @@ int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request
>   {
>   	int ret;
>
> +	/* Flush enough space to reduce the likelihood of waiting after
> +	 * we start building the request - in which case we will just
> +	 * have to repeat work.
> +	 */
> +	request->reserved_space += MIN_SPACE_FOR_ADD_REQUEST;
> +
>   	request->ringbuf = request->ctx->engine[request->engine->id].ringbuf;
>
>   	if (i915.enable_guc_submission) {
> @@ -725,6 +731,7 @@ int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request
>   	if (ret)
>   		goto err_unpin;
>
> +	request->reserved_space -= MIN_SPACE_FOR_ADD_REQUEST;

Without any previous experience with ringbuf space reservation and 
related, this does make sense to me. :)

So why add and subtract and not just increase MIN_SPACE_FOR_ADD_REQUEST? 
Or if MIN_SPACE_FOR_ADD_REQUEST is completely unrelated to the 
subsequent stuff to go in, and perhaps only represent the typical driver 
prologue & epilogue, why increase the reserved size temporarily by that 
amount and not something else?

Regards,

Tvrtko

>   	return 0;
>
>   err_unpin:
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 1193372f74fd..1285605f25c7 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -2332,8 +2332,22 @@ int intel_engine_idle(struct intel_engine_cs *engine)
>
>   int intel_ring_alloc_request_extras(struct drm_i915_gem_request *request)
>   {
> +	int ret;
> +
> +	/* Flush enough space to reduce the likelihood of waiting after
> +	 * we start building the request - in which case we will just
> +	 * have to repeat work.
> +	 */
> +	request->reserved_space += MIN_SPACE_FOR_ADD_REQUEST;
> +
>   	request->ringbuf = request->engine->buffer;
> -	return intel_ring_begin(request, 0);
> +
> +	ret = intel_ring_begin(request, 0);
> +	if (ret)
> +		return ret;
> +
> +	request->reserved_space -= MIN_SPACE_FOR_ADD_REQUEST;
> +	return 0;
>   }
>
>   static int wait_for_space(struct drm_i915_gem_request *req, int bytes)
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx