[Intel-gfx] [PATCH 1/4] drm/i915: also disable south interrupts when handling them

[Daniel Vetter]

On Fri, Feb 22, 2013 at 05:05:28PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
> 
> From the docs:
> 
>   "IIR can queue up to two interrupt events. When the IIR is cleared,
>   it will set itself again after one clock if a second event was
>   stored."
> 
>   "Only the rising edge of the PCH Display interrupt will cause the
>   North Display IIR (DEIIR) PCH Display Interrupt even bit to be set,
>   so all PCH Display Interrupts, including back to back interrupts,
>   must be cleared before a new PCH Display interrupt can cause DEIIR
>   to be set".
> 
> The current code works fine because we don't get many interrupts, but
> if we enable the PCH FIFO underrun interrupts we'll start getting so
> many interrupts that at some point new PCH interrupts won't cause
> DEIIR to be set.
> 
> The initial implementation I tried was to turn the code that checks
> SDEIIR into a loop, but we can still get interrupts even after the
> loop is done (and before the irq handler finishes), so we have to
> either disable the interrupts or mask them. In the end I concluded
> that just disabling the PCH interrupts is enough, you don't even need
> the loop, so this is what this patch implements. I've tested it and it
> passes the 2 "PCH FIFO underrun interrupt storms" I can reproduce:
> the "ironlake_crtc_disable" case and the "wrong watermarks" case.
> 
> In other words, here's how to reproduce the problem fixed by this
> patch:
>   1 - Enable PCH FIFO underrun interrupts (SERR_INT on SNB+)
>   2 - Boot the machine
>   3 - While booting we'll get tons of PCH FIFO underrun interrupts
>   4 - Plug a new monitor
>   5 - Run xrandr, notice it won't detect the new monitor
>   6 - Read SDEIIR and notice it's not 0 while DEIIR is 0
> 
> Q: Can't we just clear DEIIR before SDEIIR?
> A: It doesn't work. SDEIIR has to be completely cleared (including the
> interrupts stored on its back queue) before it can flip DEIIR's bit to
> 1 again, and even while you're clearing it you'll be getting more and
> more interrupts.
> 
> Q: Why does it work by just disabling+enabling the south interrupts?
> A: Because when we re-enable them, if there's something on the SDEIIR
> register (maybe an interrupt stored on the queue), the re-enabling
> will make DEIIR's bit flip to 1, and since we'll already have
> interrupts enabled we'll get another interrupt, then run our irq
> handler again to process the "back" interrupts.
> 
> v2: Even bigger commit message, added code comments.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>

Since this seems to fix the dp aux irq timeout regression I've merged this
to -fixes. Also volunteered Imre for a review, I'll add that if it pops up
in the next few days.

Big thanks to Paulo&Imre for tracking this down.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch