[Intel-gfx] [PATCH] drm/i915/irq: wait a little before queuing the hotplug work function

[Daniel Vetter]

On Mon, Jun 15, 2015 at 04:32:24PM +0300, Jani Nikula wrote:
> On Thu, 11 Jun 2015, Jani Nikula <jani.nikula at intel.com> wrote:
> > Currently it's possible this happens when a (non-DP) cable is unplugged:
> >
> > - user starts unplugging cable
> > - hotplug irq fires
> > - hotplug work function runs
> > - connector detect function runs
> > - ddc pin is still connected, edid read succeeds
> >   -> we decide nothing changed, no uevent
> > - cable completely unplugged
> >   -> our state is inconsistent with reality, no power save
> >
> > The user plugs cable back in:
> >
> > - most of the same at first
> > - ddc pin connected, edid read succeeds
> >   -> we decide nothing changed because we thought the cable was plugged
> >      in all along, no uevent
> > - cable completely plugged
> >   -> our state is somewhat consistent, however monitor might be
> >      different, the link might not recover?
> >
> > With our current implementation we rely on the hotplug pin being *both*
> > the last to be connected on plug *and* last to be disconnected on
> > unplug. The educated guess is that this is not the case.
> >
> > Per the logs in the case of the referenced bug, the hdmi detect code
> > runs within a few *microseconds* after the hotplug irq, and the EDID has
> > been successfully read within 25 ms of the irq. If the DDC lines are
> > still connected when the hotplug irq fires, the user has to be blazingly
> > fast to complete the unplug before the detect code runs.
> >
> > We can afford to wait a little before queuing the work function for a
> > bit more reliability. Obviously it's still possible for the user to
> > unplug the cable really slowly, but let's at least give the user a
> > fighting chance to unplug fast enough.
> >
> > I'd love to claim the proposed delay of 400 ms is based on real life
> > measured data and rigorous analysis, but in truth it is just a gut
> > feeling based compromise between solving the issue and meeting some
> > vague real time human interaction deadline for having a picture on
> > screen. (However I expect any criticism to be more substantiated than
> > "my gut feeling is better than yours".)
> >
> > An alternative would be to check the hotplug state in the irq handler,
> > but there have been reliability problems with it in the past, and we've
> > opted not to use it. [citation needed]
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82593
> > Tested-by: Hugh Greenberg <hugegreenbug at gmail.com>
> > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > Signed-off-by: Jani Nikula <jani.nikula at intel.com>
> 
> Daniel wants a fancier solution: queing a one-shot re-check after a
> while in the cases that the connector state appears to not have
> changed. It'll take a while until I get to it. But this patch is not
> going anywhere.

For a bit of context with an abitrary delay we have a bad tuning problem
of people complaining that the latency after a hotplug is too noticeable
while other folks complain that it's still to fast. And we have all the
infrastructure and tracking already to know which hpd pins have fired - we
already only re-probe connectors where we expect a change.

Hence adding an additional reprobe when there's no change when we expect
one isn't invasive, doesn't result in needless reprobing and the delay can
be picked such that even the slowest user won't be able to botch things up
any more ;-)

Also adding a reprobe would allow us to resurrect an opportunistic check
for the hpd pins (instead of the hard one we've had earlier and had to
back out again from hdmi due to crappy hw). Which is something that's been
requested piles of times (together with caching edids and stuff) and hence
fits into the overall plan better.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch