[PATCH] drm/sun4i: hdmi: Improve compatibility with non-hotplug capable connectors

[Russell King - ARM Linux]

On Mon, Nov 19, 2018 at 09:19:34AM +0100, Maxime Ripard wrote:
> Hi,
> 
> On Fri, Nov 16, 2018 at 07:18:29PM +0200, Priit Laes wrote:
> > From: Priit Laes <priit.laes at paf.com>
> > 
> > Even though HDMI connector features hotplug detect pin (HPD), there are
> > devices that which do not support it.
> 
> Which devices?
> 
> > For these devices fall back to additional check on I2C bus. Of
> > course, there might be also devices that do not wire DDC pins too,
> > so we don't really know whether cable has been connected.
> 
> Again, which devices?
> 
> > 
> > Signed-off-by: Priit Laes <plaes at plaes.org>
> > Signed-off-by: Priit Laes <priit.laes at paf.com>
> 
> You only need one :)
> 
> > ---
> >  drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 12 ++++++++----
> >  1 file changed, 8 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
> > index 061d2e0d9011..bded09af1340 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
> > +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
> > @@ -238,14 +238,18 @@ sun4i_hdmi_connector_detect(struct drm_connector *connector, bool force)
> >  	struct sun4i_hdmi *hdmi = drm_connector_to_sun4i_hdmi(connector);
> >  	unsigned long reg;
> >  
> > -	if (readl_poll_timeout(hdmi->base + SUN4I_HDMI_HPD_REG, reg,
> > +	if (!readl_poll_timeout(hdmi->base + SUN4I_HDMI_HPD_REG, reg,
> >  			       reg & SUN4I_HDMI_HPD_HIGH,
> >  			       0, 500000)) {
> > -		cec_phys_addr_invalidate(hdmi->cec_adap);
> > -		return connector_status_disconnected;
> > +		return connector_status_connected;
> >  	}
> >  
> > -	return connector_status_connected;
> > +	if (!IS_ERR(hdmi->i2c) && drm_probe_ddc(hdmi->i2c))
> > +		return connector_status_connected;
> > +
> > +	cec_phys_addr_invalidate(hdmi->cec_adap);
> > +
> > +	return connector_status_unknown;
> 
> You're doing basically two things in that patch, first adding the
> fallback to the DDC probe if the hotplug mechanism couldn't detect the
> display, and then returning a status unknown if both fail.
> 
> While I don't really have an opinion on the first one, it's mandatory
> for every HDMI device to be able to retrieve the EDID through the
> DDC. If a device was to disallow that, it would violate the HDMI, and
> I'm not sure we want to start supporting those devices.

There is also the problem that HDMI uses the HPD signal to indicate
that the source should re-read the EDID due to the EDID changing.
In HDMI, you don't necessarily have a fixed-for-all-time EDID, but
one which can change depending on what devices are in the HDMI path.

Consider, for example, an AV amplifier which needs to subsitute the
audio capabilities when it is turned on, but when in standby needs
to pass through the TVs audio capabilities.  It informs the source
by momentarily deasserting the HDMI HPD signal, which is the HDMI
way to inform the source that the EDID should be re-read.

If you're going to use "read EDID" as the hotplug method, I think
you need to keep track of when it changes so that EDID updates are
correctly handled.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up