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. _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
