[PATCH 2/4] staging: imx-hdmi: correct connector detect and hotplug

[Russell King - ARM Linux]

On Mon, Apr 14, 2014 at 11:38:43AM +0200, Lucas Stach wrote:
> Am Montag, den 14.04.2014, 10:10 +0100 schrieb Russell King - ARM Linux:
> > On Mon, Apr 14, 2014 at 10:42:32AM +0200, Lucas Stach wrote:
> > > Am Sonntag, den 13.04.2014, 15:58 +0100 schrieb Russell King - ARM
> > > Linux:
> > > > On Fri, Apr 11, 2014 at 04:13:33PM +0200, Lucas Stach wrote:
> > > > > Make sure that we probe for a display on detect regardless
> > > > > of previous hotplug events. Don't handle connector
> > > > > hotplug state ourselves, but let DRM do the right thing
> > > > > for us. This brings our hotplug handling in line with
> > > > > what other DRM drivers do.
> > > > 
> > > > Why should working setups have to pay the price for faulty setups when we
> > > > can adequately detect the hotplug signal on iMX SoCs when it's correctly
> > > > wired?
> > > > 
> > > > By "price" I mean - if we end up having to poll the connector, we end up
> > > > calling the i2c functions, and the i2c functions on iMX use a fixed
> > > > timeout of 100ms.  That means the context which runs the
> > > > imx_hdmi_connector_detect() function is forced to sleep for 100ms.  If
> > > > that's being run as part of a softirq (eg, via a work struct), that's
> > > > bad news because that could be any thread in the system.
> > > > 
> > > > The "price" should only be paid by those implementations where the hotplug
> > > > signal is not correctly wired.
> > > > 
> > > This change is not related to broken systems. It just uses the DRM
> > > framework as intended. The detect() callback, which triggers the EDID
> > > fetch will only be called by DRM when a hotplug event was received, or
> > > if someone (e.g. kms_fb_helper, or userspace) explicitly requests to
> > > poll the connector.
> > > 
> > > Not doing so is working around the DRM framework, not using it. So as
> > > mentioned this change just brings us in line with what other DRM drivers
> > > do to handle hotplug and connector detect.
> > 
> > I totally disagree with that.  What we're doing today using HPD to
> > detect connection is entirely in keeping with DRM and the HDMI spec,
> > and is more correct than your solution using EDID to detect the
> > presence of a connection.
> > 
> > HPD in HDMI indicates that the EDID is available for reading.  There
> > is no need what so ever to try reading the EDID to detect whether
> > a device is present.
> > 
> > Moreover, the HDMI spec does not say what state the DDC signals will
> > be when the sink is powered off - it seems to me that it is entirely
> > reasonable when HPD is lowered due to the sink being powered off that
> > the DDC signals may be clamped to logic zero by ESD diodes in the sink,
> > which would cause problems when trying to detect by reading the EDID.
> > 
> > Moreover, it is quite legal for a sink to modify the contents of its
> > EEPROM - and it can do this by manipulating the DDC signals itself.
> > Polling the EDID would open the possibilities of races, reading the
> > EDID before the sink had finished updating it.
> > 
> 
> And that's exactly what happens now. We do not poll the EDID in any way,
> until we are explicitly asked to do so, which happens only very few
> occasions.
> 
> Please go back and read the code after this patch. What we do now in the
> regular case (nobody is calling detect() explicitly) is the following:

Now *you* please go back and read what you said about kms/userspace being
able to poll the connector, thereby causing an EDID read attempt while
HPD may not be active.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.