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

[Chris Wilson]

On Thu, Jun 11, 2015 at 11:35:59AM +0300, Jani Nikula 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".)

400ms is a tag too long. 250ms would be my preference, but 400ms is what
was tested with, let's go with it until we find resume time is now >400ms
I wonder if we would want to flush the hotplug_work on init/resume.

> 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]

Easiest example is https://bugs.freedesktop.org/show_bug.cgi?id=76464.
But yes we had lots of feedback when we used the live state.

> 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>

Looks familiar, https://bugs.freedesktop.org/show_bug.cgi?id=82551
Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre