Move the register slow register write and readback from out of the
critical path for execlists submission and delay it until the following
worker.

Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/intel_breadcrumbs.c | 48 ++++++++++++++++++---
 1 file changed, 41 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c 
b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
index d8b206e53660..1359314f0fd5 100644
--- a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
@@ -83,6 +83,9 @@ static void __intel_breadcrumbs_arm_irq(struct 
intel_breadcrumbs *b)
 
        if (!b->irq_enabled++)
                irq_enable(b->irq_engine);
+
+       /* Requests may have completed before we could enable the interrupt. */
+       irq_work_queue(&b->irq_work);
 }
 
 static void __intel_breadcrumbs_disarm_irq(struct intel_breadcrumbs *b)
@@ -105,8 +108,6 @@ static void add_signaling_context(struct intel_breadcrumbs 
*b,
 {
        intel_context_get(ce);
        list_add_tail(&ce->signal_link, &b->signalers);
-       if (list_is_first(&ce->signal_link, &b->signalers))
-               __intel_breadcrumbs_arm_irq(b);
 }
 
 static void remove_signaling_context(struct intel_breadcrumbs *b,
@@ -197,6 +198,29 @@ static void signal_irq_work(struct irq_work *work)
 
        spin_lock(&b->irq_lock);
 
+       /*
+        * Keep the irq armed until the interrupt after all listeners are gone.
+        *
+        * Enabling/disabling the interrupt is rather costly, roughly a couple
+        * of hundred microseconds. If we are proactive and enable/disable
+        * the interrupt around every request that wants a breadcrumb, we
+        * quickly drown in the extra orders of magnitude of latency imposed
+        * on request submission.
+        *
+        * So we try to be lazy, and keep the interrupts enabled until no
+        * more listeners appear within a breadcrumb interrupt interval (that
+        * is until a request completes that no one cares about). The
+        * observation is that listeners come in batches, and will often
+        * listen to a bunch of requests in succession.
+        *
+        * We also try to avoid raising too many interrupts, as they may
+        * be generated by userspace batches and it is unfortunately rather
+        * too easy to drown the CPU under a flood of GPU interrupts. Thus
+        * whenever no one appears to be listening, we turn off the interrupts.
+        * Fewer interrupts should conserve power -- at the very least, fewer
+        * interrupt draw less ire from other users of the system and tools
+        * like powertop.
+        */
        if (list_empty(&b->signalers))
                __intel_breadcrumbs_disarm_irq(b);
 
@@ -251,6 +275,13 @@ static void signal_irq_work(struct irq_work *work)
 
                i915_request_put(rq);
        }
+
+       if (!READ_ONCE(b->irq_armed) && !list_empty(&b->signalers)) {
+               spin_lock(&b->irq_lock);
+               if (!list_empty(&b->signalers))
+                       __intel_breadcrumbs_arm_irq(b);
+               spin_unlock(&b->irq_lock);
+       }
 }
 
 struct intel_breadcrumbs *
@@ -301,8 +332,8 @@ void intel_breadcrumbs_park(struct intel_breadcrumbs *b)
        __intel_breadcrumbs_disarm_irq(b);
        spin_unlock_irqrestore(&b->irq_lock, flags);
 
-       if (!list_empty(&b->signalers))
-               irq_work_queue(&b->irq_work);
+       /* Kick the work once more to drain the signalers */
+       irq_work_queue(&b->irq_work);
 }
 
 void intel_breadcrumbs_free(struct intel_breadcrumbs *b)
@@ -362,9 +393,12 @@ static void insert_breadcrumb(struct i915_request *rq,
        GEM_BUG_ON(!check_signal_order(ce, rq));
        set_bit(I915_FENCE_FLAG_SIGNAL, &rq->fence.flags);
 
-       /* Check after attaching to irq, interrupt may have already fired. */
-       if (__request_completed(rq))
-               irq_work_queue(&b->irq_work);
+       /*
+        * Defer enabling the interrupt to after HW submission and recheck
+        * the request as it may have completed and raised the interrupt as
+        * we were attaching it into the lists.
+        */
+       irq_work_queue(&b->irq_work);
 }
 
 bool i915_request_enable_breadcrumb(struct i915_request *rq)
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to