[Intel-gfx] [PATCH 1/3] drm/i915: Only spin whilst waiting on the current request

[Chris Wilson]

On Thu, Nov 19, 2015 at 10:05:39AM +0000, Tvrtko Ursulin wrote:
> 
> Hi,
> 
> On 18/11/15 09:56, Chris Wilson wrote:
> >Limit busywaiting only to the request currently being processed by the
> >GPU. If the request is not currently being processed by the GPU, there
> >is a very low likelihood of it being completed within the 2 microsecond
> >spin timeout and so we will just be wasting CPU cycles.
> >
> >Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> >---
> >  drivers/gpu/drm/i915/i915_drv.h | 2 +-
> >  drivers/gpu/drm/i915/i915_gem.c | 8 +++++++-
> >  2 files changed, 8 insertions(+), 2 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >index 8afda459a26e..16095b95d2df 100644
> >--- a/drivers/gpu/drm/i915/i915_drv.h
> >+++ b/drivers/gpu/drm/i915/i915_drv.h
> >@@ -2190,7 +2190,7 @@ struct drm_i915_gem_request {
> >  	struct intel_engine_cs *ring;
> >
> >  	/** GEM sequence number associated with this request. */
> >-	uint32_t seqno;
> >+	uint32_t seqno, spin_seqno;
> 
> Comment needs splitting out.

Is it not the sequence associated with the request? The start of request
activity, end of active marker?
 
> And spin_seqno is not the best name, I think previous_ring_seqno
> would be better. So it would immediately tell you what it is, and
> then at the place which uses it it would also be clearer what is the
> criteria for spinning.

I agree, calling it spin_seqno was a mistake. I didn't like last_seqno
either. Though now I think,

u32 seqno_active, seqno_complete;

and a

i915_gem_request_active() helper to match i915_gem_request_completed().

> Commit message says it will spin only on the request currently being
> processed by the GPU but from here it looks like it will spin for
> any request _queued up_ before the last?

Nope, my mistake. Thanks,
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre