* [PATCH] drm/sun4i: hdmi: Improve compatibility with non-hotplug capable connectors @ 2018-11-16 17:18 Priit Laes 2018-11-19 8:19 ` Maxime Ripard 0 siblings, 1 reply; 6+ messages in thread From: Priit Laes @ 2018-11-16 17:18 UTC (permalink / raw) To: linux-arm-kernel From: Priit Laes <priit.laes@paf.com> Even though HDMI connector features hotplug detect pin (HPD), there are devices that which do not support it. 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. Signed-off-by: Priit Laes <plaes@plaes.org> Signed-off-by: Priit Laes <priit.laes@paf.com> --- 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; } static const struct drm_connector_funcs sun4i_hdmi_connector_funcs = { -- 2.19.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] drm/sun4i: hdmi: Improve compatibility with non-hotplug capable connectors 2018-11-16 17:18 [PATCH] drm/sun4i: hdmi: Improve compatibility with non-hotplug capable connectors Priit Laes @ 2018-11-19 8:19 ` Maxime Ripard 2018-11-19 8:50 ` Priit Laes 2018-11-19 10:26 ` Russell King - ARM Linux 0 siblings, 2 replies; 6+ messages in thread From: Maxime Ripard @ 2018-11-19 8:19 UTC (permalink / raw) To: linux-arm-kernel Hi, On Fri, Nov 16, 2018 at 07:18:29PM +0200, Priit Laes wrote: > From: Priit Laes <priit.laes@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@plaes.org> > Signed-off-by: Priit Laes <priit.laes@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. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 228 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20181119/83ff0540/attachment.sig> ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] drm/sun4i: hdmi: Improve compatibility with non-hotplug capable connectors 2018-11-19 8:19 ` Maxime Ripard @ 2018-11-19 8:50 ` Priit Laes 2018-11-19 10:26 ` Russell King - ARM Linux 1 sibling, 0 replies; 6+ messages in thread From: Priit Laes @ 2018-11-19 8:50 UTC (permalink / raw) To: linux-arm-kernel 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@paf.com> > > > > Even though HDMI connector features hotplug detect pin (HPD), there are > > devices that which do not support it. > > Which devices? Device I have here is labelled "AMATIC INDUSTRIES PT-MULTI-1" and based on the TFP401APZP chip. > > > 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? OK, let's skip the part without DDC. I was probably thinking about VGA cables when I was writing that.. > > > > Signed-off-by: Priit Laes <plaes@plaes.org> > > Signed-off-by: Priit Laes <priit.laes@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. Agreed. 'connector_status_disconnected' is the way to go. > 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. Yes, Even if someone runs into those non-spec devices, then there's also possibility to use the force argument. Thanks for review! > > Maxime > > -- > Maxime Ripard, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] drm/sun4i: hdmi: Improve compatibility with non-hotplug capable connectors 2018-11-19 8:19 ` Maxime Ripard 2018-11-19 8:50 ` Priit Laes @ 2018-11-19 10:26 ` Russell King - ARM Linux 2018-11-20 8:58 ` Maxime Ripard 1 sibling, 1 reply; 6+ messages in thread From: Russell King - ARM Linux @ 2018-11-19 10:26 UTC (permalink / raw) To: linux-arm-kernel 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@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@plaes.org> > > Signed-off-by: Priit Laes <priit.laes@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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] drm/sun4i: hdmi: Improve compatibility with non-hotplug capable connectors 2018-11-19 10:26 ` Russell King - ARM Linux @ 2018-11-20 8:58 ` Maxime Ripard 2018-12-05 13:18 ` Priit Laes 0 siblings, 1 reply; 6+ messages in thread From: Maxime Ripard @ 2018-11-20 8:58 UTC (permalink / raw) To: linux-arm-kernel On Mon, Nov 19, 2018 at 10:26:38AM +0000, Russell King - ARM Linux wrote: > 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@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@plaes.org> > > > Signed-off-by: Priit Laes <priit.laes@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. I didn't think about that, thanks for bringing it up! Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 228 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20181120/d0e8a592/attachment-0001.sig> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/sun4i: hdmi: Improve compatibility with non-hotplug capable connectors 2018-11-20 8:58 ` Maxime Ripard @ 2018-12-05 13:18 ` Priit Laes 0 siblings, 0 replies; 6+ messages in thread From: Priit Laes @ 2018-12-05 13:18 UTC (permalink / raw) To: Maxime Ripard Cc: Russell King - ARM Linux, dri-devel, Chen-Yu Tsai, Mats Lundberg, Priit Laes, linux-arm-kernel On Tue, Nov 20, 2018 at 09:58:41AM +0100, Maxime Ripard wrote: > On Mon, Nov 19, 2018 at 10:26:38AM +0000, Russell King - ARM Linux wrote: > > 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@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@plaes.org> > > > > Signed-off-by: Priit Laes <priit.laes@paf.com> > > > > > > You only need one :) > > > > > > > --- > > > > > > 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. > > I didn't think about that, thanks for bringing it up! Well, currently this is broken anyway, becuse we are doing the polling due to missing HPD interrupt. You can test by quickly switching between two different monitors that both have proper HPD pin. > > Maxime > > -- > Maxime Ripard, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-12-05 13:18 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-11-16 17:18 [PATCH] drm/sun4i: hdmi: Improve compatibility with non-hotplug capable connectors Priit Laes 2018-11-19 8:19 ` Maxime Ripard 2018-11-19 8:50 ` Priit Laes 2018-11-19 10:26 ` Russell King - ARM Linux 2018-11-20 8:58 ` Maxime Ripard 2018-12-05 13:18 ` Priit Laes
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).