[Intel-gfx] [PATCH] drm/i915: Do a quick check on whether the fence is already signaled first
Chris Wilson
chris at chris-wilson.co.uk
Wed Apr 26 10:48:09 UTC 2017
On Wed, Apr 26, 2017 at 11:43:03AM +0100, Tvrtko Ursulin wrote:
>
> On 26/04/2017 11:15, Chris Wilson wrote:
> >Now that we try to signal the fence from inside the interrupt handler,
> >when we reach the signaler thread, the fence is most likely already
> >signaled. Skip manipulating the bottom-half locks if this is so.
> >
> >Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> >Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> >---
> > drivers/gpu/drm/i915/intel_breadcrumbs.c | 9 ++++++---
> > 1 file changed, 6 insertions(+), 3 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
> >index 8f52fd5f6102..5f79c8135b3f 100644
> >--- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
> >+++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
> >@@ -616,9 +616,12 @@ static int intel_breadcrumbs_signaler(void *arg)
> > request = i915_gem_request_get_rcu(request);
> > rcu_read_unlock();
> > if (signal_complete(request)) {
> >- local_bh_disable();
> >- dma_fence_signal(&request->fence);
> >- local_bh_enable(); /* kick start the tasklets */
> >+ if (!test_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
> >+ &request->fence.flags)) {
> >+ local_bh_disable();
> >+ dma_fence_signal(&request->fence);
> >+ local_bh_enable(); /* kick start the tasklet */
> >+ }
> >
> > spin_lock_irq(&b->rb_lock);
> >
> >
>
> What are you referring to by "bottom-half locks" in the commit msg?
> AFAICS it would only skip kicking the tasklets with this change.
> That may be worth it, I haven't measured, but I don't see a
> difference wrt any locks.
It's just the preempt enable/disable pair plus the function call
> In fact we could change this to:
>
> if (!dma_fence_signal(...)) {
> local_bh_disable();
> local_bh_enable();
> }
>
> If we wanted to avoid touching the flags directly, but then would
> have a function call..
Yeah, plus that looks ridiculous ;)
The biggest cost in the signaler thread is going to sleep.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
More information about the Intel-gfx
mailing list