Re: [PATCH v3 1/7] drm/i915: Add HPD IRQ storm detection (v4)

[Egbert Eich <eich@freedesktop.org>]

Hi Jani,

On Thu, Apr 11, 2013 at 12:32:29PM +0300, Jani Nikula wrote:
> 
> Hi Egbert -
> 
> Up front, I haven't been following this series or read any of the
> previous review comments. Please bear with me, and feel free to direct
> me to earlier comments if I'm in contradiction.

Sorry for the late reply - last week I was quite busy with other stuff,
this week i'm a bit preoccupied.

> 
> On Tue, 09 Apr 2013, Egbert Eich <eich@freedesktop.org> wrote:
> > From: Egbert Eich <eich@suse.de>
> >
> > Add a hotplug IRQ storm detection (triggered when a hotplug interrupt
> > fires more than 5 times / sec).
> 
> Okay, this is theoretical, but a display port sink could do more than
> that many hpd irq requests when connected. Which leads me to wonder in
> general if the storm detection should be different for hot plug
> vs. unplug and hpd irq events.

Agreed. 
During my tests I did not see any issues with the statistics I've 
implemented: 5/sec was an ad-hoc choice and I wanted to start with 
something simple.
I've tested it and it seemed to work, so I didn't bother to look into
this more deeply, however if you feel 5 events  / sec are too few to
really do a good distinction, we could easily increase this number.

There have been two situations where I have seen 'interrupt storms':

1. On G35: some boxes of the affected systems do not show this issue 
   at all while others see a very high load but are still usable.
   In my recollection there were in the order of some 100 interrupts/sec
   on these machines. Then there were systems which would 'stall' in
   the worker during boot due to high load. At one point the NMI 
   watchdog kicked in and stopped this mess. There the interrupts
   happened at an order of magnitude if 10k!

2. A laptop with a Sandybridge chipset where the system load went
   high at certain stages of charging levels - when the power supply
   was connected. I would assume the frequency there also was around
   some 100 / sec.

Thus if we increase the threshold frequency to some 10 / sec we
would still cover all those cases.

Some other issue I've seen is 'bouncing' during manual plugging
I've been contemplating how to address this. There are two things 
to look at:
1. multiple hotplug events due to not getting a perfect connection at first
2. EDID readout happening to early when the EDID lines are not yet fully
   and 'permanently' connected.
It might well be, that a fix for these issues might actually also adress 
the issues you are pointing out.

I have not seen them on Intel hardware - but this may be due to the 
fact that the hardware I saw it on was a separate gfx card which did 
not have the usual mounting bracket and thus the entire setup was a 
bit fragile and not really suitable for hot-plugging.
However I believe that these things might happen everywere for people
not quite used to plugging monitors too much.

> 
> Have you observed difference between hot plug/unplug?

There seems to be a difference between monitor connected/not connected:

On DVI (G35) one doesn't distinguish between plug/unplug: when the hotplug
line on the connector changes state an interrupt is sent. On this system
storms only happened when a monitor was conneted - since the state of the
HPD pin is signalled thru different frequencies on a line across SDVO (in 
my recollection it was 10 vs. 20 MHz) I believe that due to cross talk
the higher(?) frequency could not always reliably be measured.

I did not have access to the laptop system and the customer was not 
patient enough to help me to debug this further with me.

Generally I think we would still adress the 'strom condition' if we
raised the threshold to 20 or 30 /sec.

What do you think?

> 
> Has this been a problem on PCH split platforms, i.e. since ilk/gen5?

I've also observed this on Sandybridge - which would be past GEN5, 
wouldn't it?

I will address some of the other issues mentioned in a new patch.

Thanks a lot for looking at it!

Cheers,
	Egbert.