On 10/03/16 11:44, Chris Wilson wrote:
This effectively reverts
commit 8e5fd599eb219f1054e39b40d18b217af669eea9
Author: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Date: Wed Apr 9 13:28:50 2014 +0300
drm/i915/chv: Make CHV irq handler loop until all interrupts are consumed
as under continuous execlists load we can saturate the IRQ handler,
destablising the tsc clock and triggering the NMI watchdog to declare a hung
CPU.
[ 552.756051] clocksource: timekeeping watchdog on CPU0: Marking clocksource 'tsc' as unstable because the skew is too large:
[ 552.756080] clocksource: 'refined-jiffies' wd_now: 10003b480 wd_last: 10003b28c mask: ffffffff
[ 552.756091] clocksource: 'tsc' cs_now: d55d31aa50 cs_last: d17446166c mask: ffffffffffffffff
[ 552.756210] clocksource: Switched to clocksource refined-jiffies
[ 575.217870] NMI watchdog: Watchdog detected hard LOCKUP on cpu 1
[ 575.217893] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.5.0-rc7+ #18
[ 575.217905] Hardware name: /NUC5CPYB, BIOS PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015
[ 575.217915] 0000000000000000 ffff88027fd05bc0 ffffffff81288c6d 0000000000000000
[ 575.217935] 0000000000000001 ffff88027fd05be0 ffffffff810e72d1 0000000000000000
[ 575.217951] ffff88027fd05c80 ffff88027fd05c20 ffffffff81114b60 0000000181015f1e
[ 575.217967] Call Trace:
[ 575.217973] <NMI> [<ffffffff81288c6d>] dump_stack+0x4f/0x72
[ 575.217994] [<ffffffff810e72d1>] watchdog_overflow_callback+0x151/0x160
[ 575.218003] [<ffffffff81114b60>] __perf_event_overflow+0xa0/0x1e0
[ 575.218016] [<ffffffff811154c4>] perf_event_overflow+0x14/0x20
[ 575.218028] [<ffffffff8101d2ca>] intel_pmu_handle_irq+0x1da/0x460
[ 575.218042] [<ffffffff814a8aae>] ? poll_idle+0x3e/0x70
[ 575.218052] [<ffffffff814a8aae>] ? poll_idle+0x3e/0x70
[ 575.218064] [<ffffffff81014ae8>] perf_event_nmi_handler+0x28/0x50
[ 575.218075] [<ffffffff81007540>] nmi_handle+0x60/0x130
[ 575.218086] [<ffffffff814a8aae>] ? poll_idle+0x3e/0x70
[ 575.218096] [<ffffffff810079c0>] do_nmi+0x140/0x470
[ 575.218108] [<ffffffff81559ec7>] end_repeat_nmi+0x1a/0x1e
[ 575.218119] [<ffffffff814a8aae>] ? poll_idle+0x3e/0x70
[ 575.218129] [<ffffffff814a8aae>] ? poll_idle+0x3e/0x70
[ 575.218139] [<ffffffff814a8aae>] ? poll_idle+0x3e/0x70
[ 575.218148] <<EOE>> [<ffffffff814a8353>] cpuidle_enter_state+0xf3/0x2f0
[ 575.218164] [<ffffffff814a8587>] cpuidle_enter+0x17/0x20
[ 575.218175] [<ffffffff810aaa3a>] call_cpuidle+0x2a/0x40
[ 575.218185] [<ffffffff810aade3>] cpu_startup_entry+0x273/0x330
[ 575.218196] [<ffffffff81033a1e>] start_secondary+0x10e/0x130
However, not servicing all available IIR within the handler does hurt the
throughput of pathological nop execbuf by about 20%, with a similar effect
upon the dispatch latency of a series of execbuf.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93467
Testcase: igt/gem_exec_nop/basic # requires NMI watchdog
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Cc: Antti Koskipää <antti.koskipaa@xxxxxxxxxxxxxxx
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx
---
drivers/gpu/drm/i915/i915_irq.c | 40 +++++++++++++++++++---------------------
1 file changed, 19 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 53e5104964b3..8a3230427884 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1829,35 +1829,33 @@ static irqreturn_t cherryview_irq_handler(int irq, void *arg)
/* IRQs are synced during runtime_suspend, we don't require a wakeref */
disable_rpm_wakeref_asserts(dev_priv);
- for (;;) {
- master_ctl = I915_READ(GEN8_MASTER_IRQ) & ~GEN8_MASTER_IRQ_CONTROL;
- iir = I915_READ(VLV_IIR);
+ master_ctl = I915_READ(GEN8_MASTER_IRQ) & ~GEN8_MASTER_IRQ_CONTROL;
+ iir = I915_READ(VLV_IIR);
- if (master_ctl == 0 && iir == 0)
- break;
+ if (master_ctl == 0 && iir == 0)
+ break;
return ret; ?
- ret = IRQ_HANDLED;
+ ret = IRQ_HANDLED;
- I915_WRITE(GEN8_MASTER_IRQ, 0);
+ I915_WRITE(GEN8_MASTER_IRQ, 0);
- /* Find, clear, then process each source of interrupt */
+ /* Find, clear, then process each source of interrupt */
- if (iir) {
- /* Consume port before clearing IIR or we'll miss events */
- if (iir & I915_DISPLAY_PORT_INTERRUPT)
- i9xx_hpd_irq_handler(dev);
- I915_WRITE(VLV_IIR, iir);
- }
+ if (iir) {
+ /* Consume port before clearing IIR or we'll miss events */
+ if (iir & I915_DISPLAY_PORT_INTERRUPT)
+ i9xx_hpd_irq_handler(dev);
+ I915_WRITE(VLV_IIR, iir);
+ }
- gen8_gt_irq_handler(dev_priv, master_ctl);
+ gen8_gt_irq_handler(dev_priv, master_ctl);
- /* Call regardless, as some status bits might not be
- * signalled in iir */
- valleyview_pipestat_irq_handler(dev, iir);
+ /* Call regardless, as some status bits might not be
+ * signalled in iir */
+ valleyview_pipestat_irq_handler(dev, iir);
- I915_WRITE(GEN8_MASTER_IRQ, DE_MASTER_IRQ_CONTROL);
- POSTING_READ(GEN8_MASTER_IRQ);
- }
+ I915_WRITE(GEN8_MASTER_IRQ, DE_MASTER_IRQ_CONTROL);
+ POSTING_READ(GEN8_MASTER_IRQ);
enable_rpm_wakeref_asserts(dev_priv);
Ack on this from me since it looks obviously immensely dangerous to loop
like that. I can't test it unfortunately.
Regards,
Tvrtko
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html