Re: [PATCH 0/2] Optimization on intel HDMI detect and get_modes

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Jan 13, 2014 at 12:21:52PM +0530, Ramalingam C wrote:
> On slow HDMI hotplug out, because of the physical interface design,
> DDC remains active for short duration even when HPD live status is
> indicating the disconnect state. Because of this on VLV and HSW,
> slow hotplug out events are not captured.
> 
> Hence this change uses the HPD pins live status to identify the
> HDMI connector status.
> 
> Multiple forced detect and read modes calls come from
> user space for different connectors, which can be handled
> with connector status and previous detect event's cached EDID.
> With this approach for hot pluggable displays, EDID retrieval
> is required only when there is a real hot-plug events.
> 
> This change also includes:
> 1. A logic to optimize those multiple calls, by re-using
>    cached data from previous detect and get_mode calls.
> 2. Store HDMI EDID, and re-use it in read modes.
> 3. Read live status reg to suppress spurious interrupts
> 
> These two changes will optimize the access to the DDC and also the
> CPU cycles burned by intel_hdmi_detect and intel_hdmi_get_modes.
> 
> Ramalingam C (1):
>   drm/i915: HDMI detection based on HPD pin live status
> 
> Shashank Sharma (1):
>   drm/i915: Optimize EDID retrival on detect and get_modes
> 
>  drivers/gpu/drm/i915/intel_drv.h  |   12 +++
>  drivers/gpu/drm/i915/intel_hdmi.c |  149 ++++++++++++++++++++++++++++++++-----
>  2 files changed, 144 insertions(+), 17 deletions(-)

Nak.

We've had code to use the hpd live status register on all platforms, but
had to take it out again on all platforms due to broken hardware: The live
status simply doesn't work sometimes, resulting in angry users reporting
black screens. So as-is patch 1 can't go into upstream. I know that it'd
be really useful if we could rely on the hpd live status but, but thus far
I couldn't come up with a good idea to make it work. Git history should
have all the details.

Also note that machines with noisy interrupt lines (see the interrupt
storm handling code in i915_irq.c) complicate this further.

This means that patch 2 is also a no-go since we really can't rely upon
the hpd stuff for hdmi detection. But we can save EDID caching if we
automatically invalidate the cached edid after 1-2 seconds or the next hpd
interrupt (whichever is first). 1-2 seconds should be long enough for
userspace to read the EDID a few times and change the output
configuration, but not long enough for users to get pissed. Note that the
caching interval should be shorter than the polling interval, which we use
as a fallback if the hpd lines are noise and atm is at 10 seconds.

Also this EDID caching and invalidation code should imo be a generic
helper with data structures (for the invalidation work and cached edid) in
struct intel_output. That way we can also wire it up for e.g. VGA or any
other output that'll benefit from EDID caching.

Aside: On DP the hpd pins seem to be reliable thus far, but I'm not sure
whether that's just lack of test coverage (since DP screens are less
common) or because it actually works ...

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch