Re: [PATCH 3.16 012/114] drm/i915: Exit cherryview_irq_handler() after one pass

[Ben Hutchings]

On Tue, 2016-06-14 at 13:47 +0300, Ville SyrjÃlà wrote:
> On Mon, Jun 13, 2016 at 07:36:37PM +0100, Ben Hutchings wrote:
> > 3.16.36-rc1 review patch.ÂÂIf anyone has any objections, please let me know.
> 
> Do not backport this one. It'll break things.

But this has not been re-reverted in mainline, has it? ÂIs it that
3.16-stable would need more changes backported to make this work, or is
mainline currently broken on Cherryview hardware?

Ben.

> > 
> > ------------------
> > 
> > From: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > 
> > commit 9dbaab56ac09f07a73fe83bf69bec3e31060080a upstream.
> > 
> > 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]ÂÂÂÂ[] dump_stack+0x4f/0x72
> > [ÂÂ575.217994]ÂÂ[] watchdog_overflow_callback+0x151/0x160
> > [ÂÂ575.218003]ÂÂ[] __perf_event_overflow+0xa0/0x1e0
> > [ÂÂ575.218016]ÂÂ[] perf_event_overflow+0x14/0x20
> > [ÂÂ575.218028]ÂÂ[] intel_pmu_handle_irq+0x1da/0x460
> > [ÂÂ575.218042]ÂÂ[] ? poll_idle+0x3e/0x70
> > [ÂÂ575.218052]ÂÂ[] ? poll_idle+0x3e/0x70
> > [ÂÂ575.218064]ÂÂ[] perf_event_nmi_handler+0x28/0x50
> > [ÂÂ575.218075]ÂÂ[] nmi_handle+0x60/0x130
> > [ÂÂ575.218086]ÂÂ[] ? poll_idle+0x3e/0x70
> > [ÂÂ575.218096]ÂÂ[] do_nmi+0x140/0x470
> > [ÂÂ575.218108]ÂÂ[] end_repeat_nmi+0x1a/0x1e
> > [ÂÂ575.218119]ÂÂ[] ? poll_idle+0x3e/0x70
> > [ÂÂ575.218129]ÂÂ[] ? poll_idle+0x3e/0x70
> > [ÂÂ575.218139]ÂÂ[] ? poll_idle+0x3e/0x70
> > [ÂÂ575.218148]ÂÂ<>ÂÂ[] cpuidle_enter_state+0xf3/0x2f0
> > [ÂÂ575.218164]ÂÂ[] cpuidle_enter+0x17/0x20
> > [ÂÂ575.218175]ÂÂ[] call_cpuidle+0x2a/0x40
> > [ÂÂ575.218185]ÂÂ[] cpu_startup_entry+0x273/0x330
> > [ÂÂ575.218196]ÂÂ[] 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.
> > 
> > v2: use do {} while(0) for a smaller patch, and easier to revert again
> > 
> > I have reasonable confidence that we do not miss GT interrupts (as
> > execlists provides a stress case with a failure mechanism easily
> > detected by igt), however I have less confidence about all the other
> > sources of interrupts and worry that may lose a display hotplug
> > interrupt, for example.
> > 
> > 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>
> > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> > Reviewed-by: Ville SyrjÃlà <ville.syrjala@xxxxxxxxxxxxxxx>
> > Link: http://patchwork.freedesktop.org/patch/msgid/1457946117-6714-1-git-send-email-chris@xxxxxxxxxxxxxxxxxx
> > (cherry picked from commit 579de73b048a0a4c66c25a033ac76a2836e0cf73)
> > Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> > Signed-off-by: Ben Hutchings <ben@xxxxxxxxxxxxxxx>
> > ---
> > Âdrivers/gpu/drm/i915/i915_irq.c | 4 ++--
> > Â1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -1875,7 +1875,7 @@ static irqreturn_t cherryview_irq_handle
> > Â	u32 master_ctl, iir;
> > Â	irqreturn_t ret = IRQ_NONE;
> > Â
> > -	for (;;) {
> > +	do {
> > Â		master_ctl = I915_READ(GEN8_MASTER_IRQ) & ~GEN8_MASTER_IRQ_CONTROL;
> > Â		iir = I915_READ(VLV_IIR);
> > Â
> > @@ -1897,7 +1897,7 @@ static irqreturn_t cherryview_irq_handle
> > Â		POSTING_READ(GEN8_MASTER_IRQ);
> > Â
> > Â		ret = IRQ_HANDLED;
> > -	}
> > +	} while (0);
> > Â
> > Â	return ret;
> > Â}
> 
-- 
Ben Hutchings
We get into the habit of living before acquiring the habit of thinking.
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ- Albert
Camus

Attachment: signature.asc
Description: This is a digitally signed message part