Commit 67b807a8 authored by Chris Wilson's avatar Chris Wilson

drm/i915: Delay disabling the user interrupt for breadcrumbs

A significant cost in setting up a wait is the overhead of enabling the
interrupt. As we disable the interrupt whenever the queue of waiters is
empty, if we are frequently waiting on alternating batches, we end up
re-enabling the interrupt on a frequent basis. We do want to disable the
interrupt during normal operations as under high load it may add several
thousand interrupts/s - we have been known in the past to occupy whole
cores with our interrupt handler after accidentally leaving user
interrupts enabled. As a compromise, leave the interrupt enabled until
the next IRQ, or the system is idle. This gives a small window for a
waiter to keep the interrupt active and not be delayed by having to
re-enable the interrupt.

v2: Restore hangcheck/missed-irq detection for continuations
v3: Be more careful restoring the hangcheck timer after reset
v4: Be more careful restoring the fake irq after reset (if required!)
v5: Redo changes to intel_engine_wakeup()
v6: Factor out __intel_engine_wakeup()
v7: Improve commentary for declaring a missed wakeup
Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20170227205850.2828-4-chris@chris-wilson.co.uk
parent 19d0a572
...@@ -2988,8 +2988,10 @@ i915_gem_idle_work_handler(struct work_struct *work) ...@@ -2988,8 +2988,10 @@ i915_gem_idle_work_handler(struct work_struct *work)
if (wait_for(intel_execlists_idle(dev_priv), 10)) if (wait_for(intel_execlists_idle(dev_priv), 10))
DRM_ERROR("Timeout waiting for engines to idle\n"); DRM_ERROR("Timeout waiting for engines to idle\n");
for_each_engine(engine, dev_priv, id) for_each_engine(engine, dev_priv, id) {
intel_engine_disarm_breadcrumbs(engine);
i915_gem_batch_pool_fini(&engine->batch_pool); i915_gem_batch_pool_fini(&engine->batch_pool);
}
GEM_BUG_ON(!dev_priv->gt.awake); GEM_BUG_ON(!dev_priv->gt.awake);
dev_priv->gt.awake = false; dev_priv->gt.awake = false;
......
...@@ -1060,6 +1060,8 @@ static void notify_ring(struct intel_engine_cs *engine) ...@@ -1060,6 +1060,8 @@ static void notify_ring(struct intel_engine_cs *engine)
rq = wait->request; rq = wait->request;
wake_up_process(wait->tsk); wake_up_process(wait->tsk);
} else {
__intel_engine_disarm_breadcrumbs(engine);
} }
spin_unlock(&engine->breadcrumbs.lock); spin_unlock(&engine->breadcrumbs.lock);
......
...@@ -26,20 +26,30 @@ ...@@ -26,20 +26,30 @@
#include "i915_drv.h" #include "i915_drv.h"
unsigned int intel_engine_wakeup(struct intel_engine_cs *engine) static unsigned int __intel_breadcrumbs_wakeup(struct intel_breadcrumbs *b)
{ {
struct intel_wait *wait; struct intel_wait *wait;
unsigned long flags;
unsigned int result = 0; unsigned int result = 0;
spin_lock_irqsave(&engine->breadcrumbs.lock, flags); wait = b->first_wait;
wait = engine->breadcrumbs.first_wait;
if (wait) { if (wait) {
result = ENGINE_WAKEUP_WAITER; result = ENGINE_WAKEUP_WAITER;
if (!wake_up_process(wait->tsk)) if (wake_up_process(wait->tsk))
result |= ENGINE_WAKEUP_ACTIVE; result |= ENGINE_WAKEUP_ASLEEP;
} }
spin_unlock_irqrestore(&engine->breadcrumbs.lock, flags);
return result;
}
unsigned int intel_engine_wakeup(struct intel_engine_cs *engine)
{
struct intel_breadcrumbs *b = &engine->breadcrumbs;
unsigned long flags;
unsigned int result;
spin_lock_irqsave(&b->lock, flags);
result = __intel_breadcrumbs_wakeup(b);
spin_unlock_irqrestore(&b->lock, flags);
return result; return result;
} }
...@@ -54,7 +64,7 @@ static void intel_breadcrumbs_hangcheck(unsigned long data) ...@@ -54,7 +64,7 @@ static void intel_breadcrumbs_hangcheck(unsigned long data)
struct intel_engine_cs *engine = (struct intel_engine_cs *)data; struct intel_engine_cs *engine = (struct intel_engine_cs *)data;
struct intel_breadcrumbs *b = &engine->breadcrumbs; struct intel_breadcrumbs *b = &engine->breadcrumbs;
if (!b->irq_enabled) if (!b->irq_armed)
return; return;
if (b->hangcheck_interrupts != atomic_read(&engine->irq_count)) { if (b->hangcheck_interrupts != atomic_read(&engine->irq_count)) {
...@@ -63,23 +73,32 @@ static void intel_breadcrumbs_hangcheck(unsigned long data) ...@@ -63,23 +73,32 @@ static void intel_breadcrumbs_hangcheck(unsigned long data)
return; return;
} }
/* If the waiter was currently running, assume it hasn't had a chance /* We keep the hangcheck time alive until we disarm the irq, even
* if there are no waiters at present.
*
* If the waiter was currently running, assume it hasn't had a chance
* to process the pending interrupt (e.g, low priority task on a loaded * to process the pending interrupt (e.g, low priority task on a loaded
* system) and wait until it sleeps before declaring a missed interrupt. * system) and wait until it sleeps before declaring a missed interrupt.
*
* If the waiter was asleep (and not even pending a wakeup), then we
* must have missed an interrupt as the GPU has stopped advancing
* but we still have a waiter. Assuming all batches complete within
* DRM_I915_HANGCHECK_JIFFIES [1.5s]!
*/ */
if (intel_engine_wakeup(engine) & ENGINE_WAKEUP_ACTIVE) { if (intel_engine_wakeup(engine) & ENGINE_WAKEUP_ASLEEP) {
mod_timer(&b->hangcheck, wait_timeout());
return;
}
DRM_DEBUG("Hangcheck timer elapsed... %s idle\n", engine->name); DRM_DEBUG("Hangcheck timer elapsed... %s idle\n", engine->name);
set_bit(engine->id, &engine->i915->gpu_error.missed_irq_rings); set_bit(engine->id, &engine->i915->gpu_error.missed_irq_rings);
mod_timer(&engine->breadcrumbs.fake_irq, jiffies + 1); mod_timer(&engine->breadcrumbs.fake_irq, jiffies + 1);
} else {
mod_timer(&b->hangcheck, wait_timeout());
}
} }
static void intel_breadcrumbs_fake_irq(unsigned long data) static void intel_breadcrumbs_fake_irq(unsigned long data)
{ {
struct intel_engine_cs *engine = (struct intel_engine_cs *)data; struct intel_engine_cs *engine = (struct intel_engine_cs *)data;
struct intel_breadcrumbs *b = &engine->breadcrumbs;
unsigned long flags;
/* /*
* The timer persists in case we cannot enable interrupts, * The timer persists in case we cannot enable interrupts,
...@@ -88,10 +107,15 @@ static void intel_breadcrumbs_fake_irq(unsigned long data) ...@@ -88,10 +107,15 @@ static void intel_breadcrumbs_fake_irq(unsigned long data)
* every jiffie in order to kick the oldest waiter to do the * every jiffie in order to kick the oldest waiter to do the
* coherent seqno check. * coherent seqno check.
*/ */
if (!intel_engine_wakeup(engine))
spin_lock_irqsave(&b->lock, flags);
if (!__intel_breadcrumbs_wakeup(b))
__intel_engine_disarm_breadcrumbs(engine);
spin_unlock_irqrestore(&b->lock, flags);
if (!b->irq_armed)
return; return;
mod_timer(&engine->breadcrumbs.fake_irq, jiffies + 1); mod_timer(&b->fake_irq, jiffies + 1);
/* Ensure that even if the GPU hangs, we get woken up. /* Ensure that even if the GPU hangs, we get woken up.
* *
...@@ -127,6 +151,42 @@ static void irq_disable(struct intel_engine_cs *engine) ...@@ -127,6 +151,42 @@ static void irq_disable(struct intel_engine_cs *engine)
spin_unlock(&engine->i915->irq_lock); spin_unlock(&engine->i915->irq_lock);
} }
void __intel_engine_disarm_breadcrumbs(struct intel_engine_cs *engine)
{
struct intel_breadcrumbs *b = &engine->breadcrumbs;
assert_spin_locked(&b->lock);
if (b->irq_enabled) {
irq_disable(engine);
b->irq_enabled = false;
}
b->irq_armed = false;
}
void intel_engine_disarm_breadcrumbs(struct intel_engine_cs *engine)
{
struct intel_breadcrumbs *b = &engine->breadcrumbs;
unsigned long flags;
if (!b->irq_armed)
return;
spin_lock_irqsave(&b->lock, flags);
/* We only disarm the irq when we are idle (all requests completed),
* so if there remains a sleeping waiter, it missed the request
* completion.
*/
if (__intel_breadcrumbs_wakeup(b) & ENGINE_WAKEUP_ASLEEP)
set_bit(engine->id, &engine->i915->gpu_error.missed_irq_rings);
__intel_engine_disarm_breadcrumbs(engine);
spin_unlock_irqrestore(&b->lock, flags);
}
static bool use_fake_irq(const struct intel_breadcrumbs *b) static bool use_fake_irq(const struct intel_breadcrumbs *b)
{ {
const struct intel_engine_cs *engine = const struct intel_engine_cs *engine =
...@@ -144,6 +204,15 @@ static bool use_fake_irq(const struct intel_breadcrumbs *b) ...@@ -144,6 +204,15 @@ static bool use_fake_irq(const struct intel_breadcrumbs *b)
return atomic_read(&engine->irq_count) == b->hangcheck_interrupts; return atomic_read(&engine->irq_count) == b->hangcheck_interrupts;
} }
static void enable_fake_irq(struct intel_breadcrumbs *b)
{
/* Ensure we never sleep indefinitely */
if (!b->irq_enabled || use_fake_irq(b))
mod_timer(&b->fake_irq, jiffies + 1);
else
mod_timer(&b->hangcheck, wait_timeout());
}
static void __intel_breadcrumbs_enable_irq(struct intel_breadcrumbs *b) static void __intel_breadcrumbs_enable_irq(struct intel_breadcrumbs *b)
{ {
struct intel_engine_cs *engine = struct intel_engine_cs *engine =
...@@ -151,9 +220,17 @@ static void __intel_breadcrumbs_enable_irq(struct intel_breadcrumbs *b) ...@@ -151,9 +220,17 @@ static void __intel_breadcrumbs_enable_irq(struct intel_breadcrumbs *b)
struct drm_i915_private *i915 = engine->i915; struct drm_i915_private *i915 = engine->i915;
assert_spin_locked(&b->lock); assert_spin_locked(&b->lock);
if (b->rpm_wakelock) if (b->irq_armed)
return; return;
/* The breadcrumb irq will be disarmed on the interrupt after the
* waiters are signaled. This gives us a single interrupt window in
* which we can add a new waiter and avoid the cost of re-enabling
* the irq.
*/
b->irq_armed = true;
GEM_BUG_ON(b->irq_enabled);
if (I915_SELFTEST_ONLY(b->mock)) { if (I915_SELFTEST_ONLY(b->mock)) {
/* For our mock objects we want to avoid interaction /* For our mock objects we want to avoid interaction
* with the real hardware (which is not set up). So * with the real hardware (which is not set up). So
...@@ -162,17 +239,15 @@ static void __intel_breadcrumbs_enable_irq(struct intel_breadcrumbs *b) ...@@ -162,17 +239,15 @@ static void __intel_breadcrumbs_enable_irq(struct intel_breadcrumbs *b)
* implementation to call intel_engine_wakeup() * implementation to call intel_engine_wakeup()
* itself when it wants to simulate a user interrupt, * itself when it wants to simulate a user interrupt,
*/ */
b->rpm_wakelock = true;
return; return;
} }
/* Since we are waiting on a request, the GPU should be busy /* Since we are waiting on a request, the GPU should be busy
* and should have its own rpm reference. For completeness, * and should have its own rpm reference. This is tracked
* record an rpm reference for ourselves to cover the * by i915->gt.awake, we can forgo holding our own wakref
* interrupt we unmask. * for the interrupt as before i915->gt.awake is released (when
* the driver is idle) we disarm the breadcrumbs.
*/ */
intel_runtime_pm_get_noresume(i915);
b->rpm_wakelock = true;
/* No interrupts? Kick the waiter every jiffie! */ /* No interrupts? Kick the waiter every jiffie! */
if (intel_irqs_enabled(i915)) { if (intel_irqs_enabled(i915)) {
...@@ -181,34 +256,7 @@ static void __intel_breadcrumbs_enable_irq(struct intel_breadcrumbs *b) ...@@ -181,34 +256,7 @@ static void __intel_breadcrumbs_enable_irq(struct intel_breadcrumbs *b)
b->irq_enabled = true; b->irq_enabled = true;
} }
/* Ensure we never sleep indefinitely */ enable_fake_irq(b);
if (!b->irq_enabled || use_fake_irq(b))
mod_timer(&b->fake_irq, jiffies + 1);
else
mod_timer(&b->hangcheck, wait_timeout());
}
static void __intel_breadcrumbs_disable_irq(struct intel_breadcrumbs *b)
{
struct intel_engine_cs *engine =
container_of(b, struct intel_engine_cs, breadcrumbs);
assert_spin_locked(&b->lock);
if (!b->rpm_wakelock)
return;
if (I915_SELFTEST_ONLY(b->mock)) {
b->rpm_wakelock = false;
return;
}
if (b->irq_enabled) {
irq_disable(engine);
b->irq_enabled = false;
}
intel_runtime_pm_put(engine->i915);
b->rpm_wakelock = false;
} }
static inline struct intel_wait *to_wait(struct rb_node *node) static inline struct intel_wait *to_wait(struct rb_node *node)
...@@ -430,7 +478,6 @@ static void __intel_engine_remove_wait(struct intel_engine_cs *engine, ...@@ -430,7 +478,6 @@ static void __intel_engine_remove_wait(struct intel_engine_cs *engine,
wake_up_process(b->first_wait->tsk); wake_up_process(b->first_wait->tsk);
} else { } else {
b->first_wait = NULL; b->first_wait = NULL;
__intel_breadcrumbs_disable_irq(b);
} }
} else { } else {
GEM_BUG_ON(rb_first(&b->waiters) == &wait->node); GEM_BUG_ON(rb_first(&b->waiters) == &wait->node);
...@@ -722,15 +769,22 @@ void intel_engine_reset_breadcrumbs(struct intel_engine_cs *engine) ...@@ -722,15 +769,22 @@ void intel_engine_reset_breadcrumbs(struct intel_engine_cs *engine)
cancel_fake_irq(engine); cancel_fake_irq(engine);
spin_lock_irq(&b->lock); spin_lock_irq(&b->lock);
__intel_breadcrumbs_disable_irq(b); if (b->irq_enabled)
if (intel_engine_has_waiter(engine)) { irq_enable(engine);
__intel_breadcrumbs_enable_irq(b); else
if (test_bit(ENGINE_IRQ_BREADCRUMB, &engine->irq_posted))
wake_up_process(b->first_wait->tsk);
} else {
/* sanitize the IMR and unmask any auxiliary interrupts */
irq_disable(engine); irq_disable(engine);
}
/* We set the IRQ_BREADCRUMB bit when we enable the irq presuming the
* GPU is active and may have already executed the MI_USER_INTERRUPT
* before the CPU is ready to receive. However, the engine is currently
* idle (we haven't started it yet), there is no possibility for a
* missed interrupt as we enabled the irq and so we can clear the
* immediate wakeup (until a real interrupt arrives for the waiter).
*/
clear_bit(ENGINE_IRQ_BREADCRUMB, &engine->irq_posted);
if (b->irq_armed)
enable_fake_irq(b);
spin_unlock_irq(&b->lock); spin_unlock_irq(&b->lock);
} }
......
...@@ -246,8 +246,8 @@ struct intel_engine_cs { ...@@ -246,8 +246,8 @@ struct intel_engine_cs {
unsigned int hangcheck_interrupts; unsigned int hangcheck_interrupts;
bool irq_armed : 1;
bool irq_enabled : 1; bool irq_enabled : 1;
bool rpm_wakelock : 1;
I915_SELFTEST_DECLARE(bool mock : 1); I915_SELFTEST_DECLARE(bool mock : 1);
} breadcrumbs; } breadcrumbs;
...@@ -644,7 +644,10 @@ static inline bool intel_engine_has_waiter(const struct intel_engine_cs *engine) ...@@ -644,7 +644,10 @@ static inline bool intel_engine_has_waiter(const struct intel_engine_cs *engine)
unsigned int intel_engine_wakeup(struct intel_engine_cs *engine); unsigned int intel_engine_wakeup(struct intel_engine_cs *engine);
#define ENGINE_WAKEUP_WAITER BIT(0) #define ENGINE_WAKEUP_WAITER BIT(0)
#define ENGINE_WAKEUP_ACTIVE BIT(1) #define ENGINE_WAKEUP_ASLEEP BIT(1)
void __intel_engine_disarm_breadcrumbs(struct intel_engine_cs *engine);
void intel_engine_disarm_breadcrumbs(struct intel_engine_cs *engine);
void intel_engine_reset_breadcrumbs(struct intel_engine_cs *engine); void intel_engine_reset_breadcrumbs(struct intel_engine_cs *engine);
void intel_engine_fini_breadcrumbs(struct intel_engine_cs *engine); void intel_engine_fini_breadcrumbs(struct intel_engine_cs *engine);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment