drm/i915/gt: Drain the breadcrumbs just once
authorChris Wilson <chris@chris-wilson.co.uk>
Thu, 17 Dec 2020 09:15:24 +0000 (09:15 +0000)
committerChris Wilson <chris@chris-wilson.co.uk>
Thu, 17 Dec 2020 19:56:17 +0000 (19:56 +0000)
Matthew Brost pointed out that the while-loop on a shared breadcrumb was
inherently fraught with danger as it competed with the other users of
the breadcrumbs. However, in order to completely drain the re-arming irq
worker, the while-loop is a necessity, despite my optimism that we could
force cancellation with a couple of irq_work invocations.

Given that we can't merely drop the while-loop, use an activity counter on
the breadcrumbs to detect when we are parking the breadcrumbs for the
last time.

Based on a patch by Matthew Brost.

Reported-by: Matthew Brost <matthew.brost@intel.com>
Suggested-by: Matthew Brost <matthew.brost@intel.com>
Fixes: 9d5612ca165a ("drm/i915/gt: Defer enabling the breadcrumb interrupt to after submission")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20201217091524.10258-1-chris@chris-wilson.co.uk
drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
drivers/gpu/drm/i915/gt/intel_breadcrumbs.h
drivers/gpu/drm/i915/gt/intel_breadcrumbs_types.h
drivers/gpu/drm/i915/gt/intel_engine_pm.c

index 0091830..3c62fd6 100644 (file)
@@ -332,17 +332,19 @@ void intel_breadcrumbs_reset(struct intel_breadcrumbs *b)
        spin_unlock_irqrestore(&b->irq_lock, flags);
 }
 
-void intel_breadcrumbs_park(struct intel_breadcrumbs *b)
+void __intel_breadcrumbs_park(struct intel_breadcrumbs *b)
 {
-       /* Kick the work once more to drain the signalers */
+       if (!READ_ONCE(b->irq_armed))
+               return;
+
+       /* Kick the work once more to drain the signalers, and disarm the irq */
        irq_work_sync(&b->irq_work);
-       while (unlikely(READ_ONCE(b->irq_armed))) {
+       while (READ_ONCE(b->irq_armed) && !atomic_read(&b->active)) {
                local_irq_disable();
                signal_irq_work(&b->irq_work);
                local_irq_enable();
                cond_resched();
        }
-       GEM_BUG_ON(!list_empty(&b->signalers));
 }
 
 void intel_breadcrumbs_free(struct intel_breadcrumbs *b)
index ed3d1de..75cc9cf 100644 (file)
@@ -19,7 +19,18 @@ intel_breadcrumbs_create(struct intel_engine_cs *irq_engine);
 void intel_breadcrumbs_free(struct intel_breadcrumbs *b);
 
 void intel_breadcrumbs_reset(struct intel_breadcrumbs *b);
-void intel_breadcrumbs_park(struct intel_breadcrumbs *b);
+void __intel_breadcrumbs_park(struct intel_breadcrumbs *b);
+
+static inline void intel_breadcrumbs_unpark(struct intel_breadcrumbs *b)
+{
+       atomic_inc(&b->active);
+}
+
+static inline void intel_breadcrumbs_park(struct intel_breadcrumbs *b)
+{
+       if (atomic_dec_and_test(&b->active))
+               __intel_breadcrumbs_park(b);
+}
 
 static inline void
 intel_engine_signal_breadcrumbs(struct intel_engine_cs *engine)
index a74bb30..d85a6f7 100644 (file)
@@ -29,8 +29,7 @@
  * the overhead of waking that client is much preferred.
  */
 struct intel_breadcrumbs {
-       /* Not all breadcrumbs are attached to physical HW */
-       struct intel_engine_cs *irq_engine;
+       atomic_t active;
 
        spinlock_t signalers_lock; /* protects the list of signalers */
        struct list_head signalers;
@@ -40,6 +39,9 @@ struct intel_breadcrumbs {
        struct irq_work irq_work; /* for use from inside irq_lock */
        unsigned int irq_enabled;
        bool irq_armed;
+
+       /* Not all breadcrumbs are attached to physical HW */
+       struct intel_engine_cs *irq_engine;
 };
 
 #endif /* __INTEL_BREADCRUMBS_TYPES__ */
index 499b09c..d74e748 100644 (file)
@@ -65,6 +65,7 @@ static int __engine_unpark(struct intel_wakeref *wf)
        if (engine->unpark)
                engine->unpark(engine);
 
+       intel_breadcrumbs_unpark(engine->breadcrumbs);
        intel_engine_unpark_heartbeat(engine);
        return 0;
 }