drm: vc4: WARNING: CPU: 2 PID: 172 at kernel/irq/chip.c:244 __irq_startup+0x9c/0xa8

[Marc Zyngier]

On Sat, 09 Dec 2017 14:17:58 +0000,
Stefan Schake wrote:
> 
> On Sat, Dec 9, 2017 at 1:34 PM, Marc Zyngier <marc.zyngier at arm.com> wrote:
> > On Fri, 08 Dec 2017 22:44:27 +0000,
> > Stefan Wahren wrote:
> >>
> >> Hi,
> >>
> >> the commit 253696ccd613 ("drm/vc4: Account for interrupts in
> >> flight") triggers a warning during boot of Raspberry Pi 2 with
> >> multi_v7_defconfig:
> >>
> >> [    7.962699] vc4_hdmi 3f902000.hdmi: vc4-hdmi-hifi <-> 3f902000.hdmi mapping ok
> >> [    7.962732] vc4_hdmi 3f902000.hdmi: ASoC: no DMI vendor name!
> >> [    7.973355] vc4-drm soc:gpu: bound 3f902000.hdmi (ops vc4_hdmi_ops [vc4])
> >> [    7.973651] vc4-drm soc:gpu: bound 3f806000.vec (ops vc4_vec_ops [vc4])
> >> [    7.973788] vc4-drm soc:gpu: bound 3f400000.hvs (ops vc4_hvs_ops [vc4])
> >> [    7.974157] vc4-drm soc:gpu: bound 3f206000.pixelvalve (ops vc4_crtc_ops [vc4])
> >> [    7.974464] vc4-drm soc:gpu: bound 3f207000.pixelvalve (ops vc4_crtc_ops [vc4])
> >> [    7.974746] vc4-drm soc:gpu: bound 3f807000.pixelvalve (ops vc4_crtc_ops [vc4])
> >> [    8.018820] ------------[ cut here ]------------
> >> [    8.018861] WARNING: CPU: 2 PID: 172 at kernel/irq/chip.c:244 __irq_startup+0x9c/0xa8
> >> [    8.018868] Modules linked in: vc4(+) snd_soc_core ac97_bus snd_pcm_dmaengine snd_pcm snd_timer snd soundcore cec
> >> [    8.018911] CPU: 2 PID: 172 Comm: systemd-udevd Not tainted 4.15.0-rc1-00291-g75f64f6 #10
> >> [    8.018914] Hardware name: BCM2835
> >> [    8.018950] [<c03115a4>] (unwind_backtrace) from [<c030c6b4>] (show_stack+0x10/0x14)
> >> [    8.018970] [<c030c6b4>] (show_stack) from [<c0cfc3b0>] (dump_stack+0x88/0xa4)
> >> [    8.018993] [<c0cfc3b0>] (dump_stack) from [<c03430cc>] (__warn+0xe4/0x110)
> >> [    8.019012] [<c03430cc>] (__warn) from [<c03431c4>] (warn_slowpath_null+0x3c/0x48)
> >> [    8.019029] [<c03431c4>] (warn_slowpath_null) from [<c0391370>] (__irq_startup+0x9c/0xa8)
> >> [    8.019045] [<c0391370>] (__irq_startup) from [<c03913c0>] (irq_startup+0x44/0x134)
> >> [    8.019061] [<c03913c0>] (irq_startup) from [<c038f360>] (enable_irq+0x34/0x6c)
> >> [    8.019210] [<c038f360>] (enable_irq) from [<bf0a8380>] (vc4_irq_postinstall+0x14/0x34 [vc4])
> >> [    8.019338] [<bf0a8380>] (vc4_irq_postinstall [vc4]) from [<c0816a98>] (drm_irq_install+0xc0/0x134)
> >> [    8.019456] [<c0816a98>] (drm_irq_install) from [<bf0ab0c8>] (vc4_v3d_bind+0x12c/0x238 [vc4])
> >
> > The sequence "request_irq/enable" feels pretty odd, given that the
> > interrupt wasn't requested with NOAUTOEN. What are the expected
> > semantics of that patch?
> >
> > Thanks,
> >
> >         M.
> >
> > --
> > Jazz is not dead, it just smell funny.
> 
> There is a second path to vc4_irq_postinstall when we return from
> power management disable. Maybe the enable would be better situated there?

I've no idea. But enabling an already enabled interrupts feels a bit
wrong, to say the least

> That said, some more study of the irq code is giving me more questions than
> answers. If disable/enable are depth-counted, why isn't it also warning
> about that (there is a check)? Depth must be 1 after request_irq.
> Similarly, the irq is activated in request_irq, but the warning reported
> here is complaining that it is not.

I'm afraid you'll have to trace why the irq is not flagged as
activated. From what I can see, the irqchip driver of this HW doesn't
even implement an activate callback, so the interrupt should be
trivially flagged as activated. If it is not, it means something
rather bad has happened. Do you see this splat on each enable_irq()?

	M.

-- 
Jazz is not dead, it just smell funny.