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

[Chris Wilson]

On Wed, Apr 27, 2016 at 02:15:29PM +0100, Tvrtko Ursulin wrote:
> 
> 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 at chris-wilson.co.uk>
> >Cc: Tvrtko Ursulin <tvrtko.ursulin at 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?

The game being played here is to first free up enough space inside the
ringbuffer that we shouldn't have to pause again, and then ensure that
we always have enough space to submit the request. So step 1 is flush,
then step 2 is reserve, step 3 is to use the reserve.

MIN_SPACE_FOR_ADD_REQUEST is being used as a conservative estimate. What
we actually use is approx. 1 flush and 1 bb for lrc. 1 flush, several
LRI, 1 switch context, more LRI, another flush and then 1 bb for
ringbuffer. The amount of space we flush and reserve could be fine
tuned, and will likely need to be adjusted in future for various
features. But the flush is just an optimisation, the reserve is
mandatory.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre