[Intel-gfx] [PATCH 6/6] drm/i915: Only attempt to signal the request once from the interrupt handler

[Chris Wilson]

On Wed, Mar 15, 2017 at 07:47:20PM +0000, Tvrtko Ursulin wrote:
> 
> On 15/03/2017 14:01, Chris Wilson wrote:
> >Check that signaled bit on the request->fence before acquiring a
> 
>             "is not set" ^
> 
> Or "Check that the request has not been signalled before.." ?
> 
> >reference to the request for signaling later in the interrupt handler.
> >
> >Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> >Cc: Mika Kuoppala <mika.kuoppala at intel.com>
> >Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> >---
> > drivers/gpu/drm/i915/i915_irq.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> >index 31f0d7c8992f..e646c4eba65d 100644
> >--- a/drivers/gpu/drm/i915/i915_irq.c
> >+++ b/drivers/gpu/drm/i915/i915_irq.c
> >@@ -1056,7 +1056,9 @@ static void notify_ring(struct intel_engine_cs *engine)
> > 		 * and many waiters.
> > 		 */
> > 		if (i915_seqno_passed(intel_engine_get_seqno(engine),
> >-				      wait->seqno))
> >+				      wait->seqno) &&
> >+		    !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
> >+			      &wait->request->fence.flags))
> > 			rq = i915_gem_request_get(wait->request);
> >
> > 		wake_up_process(wait->tsk);
> >
> 
> Is this just to optimize things slightly? Does it happen
> sufficiently often for it to be worth it?

Even once would be enough. An unlocked read of fence.flags is much
cheaper than the locked get/put we go through otherwise. Even if the
check is a dud, the load of the cacheline prior to us making it
exclusive is "free".

I'm mostly thinking about the situations where we have pile of bare
breadcrumbs where we may very well get interrupts faster than we can get
rid of the intel_wait. Or perhaps a mythical slow device.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre