[PATCH 1/4] drm: bridge/dw-hdmi: cache edid data

[PATCH 1/4] drm: bridge/dw-hdmi: cache edid data

[Philipp Zabel]

From: Sascha Hauer <s.hauer@pengutronix.de>

Instead of rereading the edid data each time userspace asks for them,
read them once and cache them in the previously unused edid field in
struct dw_hdmi. When the connector is disconnected, drop the cached
edid data.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/gpu/drm/bridge/dw-hdmi.c | 43 ++++++++++++++++++++++++----------------
 1 file changed, 26 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index 6fbec99..b0ebf92 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -117,7 +117,7 @@ struct dw_hdmi {
 
 	int vic;
 
-	u8 edid[HDMI_EDID_LEN];
+	struct edid *edid;
 	bool cable_plugin;
 
 	bool phy_enabled;
@@ -1437,32 +1437,41 @@ dw_hdmi_connector_detect(struct drm_connector *connector, bool force)
 	dw_hdmi_update_phy_mask(hdmi);
 	mutex_unlock(&hdmi->mutex);
 
-	return hdmi_readb(hdmi, HDMI_PHY_STAT0) & HDMI_PHY_HPD ?
-		connector_status_connected : connector_status_disconnected;
+	if (hdmi_readb(hdmi, HDMI_PHY_STAT0) & HDMI_PHY_HPD)
+		return connector_status_connected;
+
+	/* free previous EDID block */
+	if (hdmi->edid) {
+		drm_mode_connector_update_edid_property(connector, NULL);
+		kfree(hdmi->edid);
+		hdmi->edid = NULL;
+	}
+
+	return connector_status_disconnected;
 }
 
 static int dw_hdmi_connector_get_modes(struct drm_connector *connector)
 {
 	struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi,
 					     connector);
-	struct edid *edid;
 	int ret = 0;
 
-	if (!hdmi->ddc)
-		return 0;
-
-	edid = drm_get_edid(connector, hdmi->ddc);
-	if (edid) {
-		dev_dbg(hdmi->dev, "got edid: width[%d] x height[%d]\n",
-			edid->width_cm, edid->height_cm);
+	if (hdmi->ddc && !hdmi->edid) {
+		hdmi->edid = drm_get_edid(connector, hdmi->ddc);
+		if (hdmi->edid) {
+			hdmi->sink_is_hdmi = drm_detect_hdmi_monitor(hdmi->edid);
+			hdmi->sink_has_audio = drm_detect_monitor_audio(hdmi->edid);
+			drm_mode_connector_update_edid_property(connector,
+								hdmi->edid);
+			dev_dbg(hdmi->dev, "got edid: width[%d] x height[%d]\n",
+				hdmi->edid->width_cm, hdmi->edid->height_cm);
+		}
+	}
 
-		hdmi->sink_is_hdmi = drm_detect_hdmi_monitor(edid);
-		hdmi->sink_has_audio = drm_detect_monitor_audio(edid);
-		drm_mode_connector_update_edid_property(connector, edid);
-		ret = drm_add_edid_modes(connector, edid);
+	if (hdmi->edid) {
+		ret = drm_add_edid_modes(connector, hdmi->edid);
 		/* Store the ELD */
-		drm_edid_to_eld(connector, edid);
-		kfree(edid);
+		drm_edid_to_eld(connector, hdmi->edid);
 	} else {
 		dev_dbg(hdmi->dev, "failed to get edid\n");
 	}
-- 
2.6.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/4] drm: bridge/dw-hdmi: remove unused code

[Philipp Zabel]

From: Sascha Hauer <s.hauer@pengutronix.de>

The cable_plugin field in struct dw_hdmi is never set. Remove it and with
it all code that is only executed when the variable is true.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/gpu/drm/bridge/dw-hdmi.c | 17 -----------------
 1 file changed, 17 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index b0ebf92..e41ff4b 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -118,7 +118,6 @@ struct dw_hdmi {
 	int vic;
 
 	struct edid *edid;
-	bool cable_plugin;
 
 	bool phy_enabled;
 	struct drm_display_mode previous_mode;
@@ -1158,24 +1157,10 @@ static void dw_hdmi_clear_overflow(struct dw_hdmi *hdmi)
 		hdmi_writeb(hdmi, val, HDMI_FC_INVIDCONF);
 }
 
-static void hdmi_enable_overflow_interrupts(struct dw_hdmi *hdmi)
-{
-	hdmi_writeb(hdmi, 0, HDMI_FC_MASK2);
-	hdmi_writeb(hdmi, 0, HDMI_IH_MUTE_FC_STAT2);
-}
-
-static void hdmi_disable_overflow_interrupts(struct dw_hdmi *hdmi)
-{
-	hdmi_writeb(hdmi, HDMI_IH_MUTE_FC_STAT2_OVERFLOW_MASK,
-		    HDMI_IH_MUTE_FC_STAT2);
-}
-
 static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
 {
 	int ret;
 
-	hdmi_disable_overflow_interrupts(hdmi);
-
 	hdmi->vic = drm_match_cea_mode(mode);
 
 	if (!hdmi->vic) {
@@ -1240,8 +1225,6 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
 	hdmi_tx_hdcp_config(hdmi);
 
 	dw_hdmi_clear_overflow(hdmi);
-	if (hdmi->cable_plugin && hdmi->sink_is_hdmi)
-		hdmi_enable_overflow_interrupts(hdmi);
 
 	return 0;
 }
-- 
2.6.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/4] drm: bridge/dw-hdmi: detect initial connector state

[Philipp Zabel]

Allow userspace to read the initial connector state via sysfs without
having to issue a detect manually. There is no reason to keep the state
unknown during initialization.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/gpu/drm/bridge/dw-hdmi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index e41ff4b..2388a55 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -1660,6 +1660,8 @@ static int dw_hdmi_register(struct drm_device *drm, struct dw_hdmi *hdmi)
 				   DRM_MODE_CONNECTOR_HDMIA);
 
 	hdmi->connector.encoder = encoder;
+	hdmi->connector.status = dw_hdmi_connector_detect(&hdmi->connector,
+							  false);
 
 	drm_mode_connector_attach_encoder(&hdmi->connector, encoder);
 
-- 
2.6.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/4] drm: bridge/dw-hdmi: use rx sense for plug detection if hpd is unreliable

[Philipp Zabel]

Due to the voltage divider on the HPD line, the HDMI connector on
imx6q-sabrelite doesn't reliably detect connected DVI monitors.
This patch allows to use the RX_SENSE signals as a workaround when
enabled by a boolean device tree property 'hpd-unreliable'.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/gpu/drm/bridge/dw-hdmi.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index 2388a55..7ffaa44 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -111,6 +111,7 @@ struct dw_hdmi {
 	struct device *dev;
 	struct clk *isfr_clk;
 	struct clk *iahb_clk;
+	bool hpd_unreliable;
 
 	struct hdmi_data_info hdmi_data;
 	const struct dw_hdmi_plat_data *plat_data;
@@ -1413,6 +1414,8 @@ dw_hdmi_connector_detect(struct drm_connector *connector, bool force)
 {
 	struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi,
 					     connector);
+	/* If HPD is not reliable, use RX_SENSE as fallback */
+	u8 stat_mask = hdmi->hpd_unreliable ? HDMI_PHY_RX_SENSE : HDMI_PHY_HPD;
 
 	mutex_lock(&hdmi->mutex);
 	hdmi->force = DRM_FORCE_UNSPECIFIED;
@@ -1420,7 +1423,7 @@ dw_hdmi_connector_detect(struct drm_connector *connector, bool force)
 	dw_hdmi_update_phy_mask(hdmi);
 	mutex_unlock(&hdmi->mutex);
 
-	if (hdmi_readb(hdmi, HDMI_PHY_STAT0) & HDMI_PHY_HPD)
+	if ((hdmi_readb(hdmi, HDMI_PHY_STAT0) & stat_mask) == stat_mask)
 		return connector_status_connected;
 
 	/* free previous EDID block */
@@ -1556,7 +1559,7 @@ static irqreturn_t dw_hdmi_hardirq(int irq, void *dev_id)
 static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
 {
 	struct dw_hdmi *hdmi = dev_id;
-	u8 intr_stat, phy_int_pol, phy_pol_mask, phy_stat;
+	u8 intr_stat, intr_mask, phy_int_pol, phy_pol_mask, phy_stat;
 
 	intr_stat = hdmi_readb(hdmi, HDMI_IH_PHY_STAT0);
 	phy_int_pol = hdmi_readb(hdmi, HDMI_PHY_POL0);
@@ -1583,9 +1586,12 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
 	 * other end of the link.  Use this to decide whether we should
 	 * power on the phy as HPD may be toggled by the sink to merely
 	 * ask the source to re-read the EDID.
+	 * If HPD is known to be unreliable, ignore it completely.
 	 */
-	if (intr_stat &
-	    (HDMI_IH_PHY_STAT0_RX_SENSE | HDMI_IH_PHY_STAT0_HPD)) {
+	intr_mask = hdmi->hpd_unreliable ?
+		    HDMI_IH_PHY_STAT0_RX_SENSE :
+		    HDMI_IH_PHY_STAT0_RX_SENSE | HDMI_IH_PHY_STAT0_HPD;
+	if (intr_stat & intr_mask) {
 		mutex_lock(&hdmi->mutex);
 		if (!hdmi->disabled && !hdmi->force) {
 			/*
@@ -1594,14 +1600,14 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
 			 */
 			if (!(phy_stat & HDMI_PHY_RX_SENSE))
 				hdmi->rxsense = false;
-
 			/*
 			 * Only set the software rxsense status when both
 			 * rxsense and hpd indicates we're connected.
 			 * This avoids what seems to be bad behaviour in
 			 * at least iMX6S versions of the phy.
 			 */
-			if (phy_stat & HDMI_PHY_HPD)
+			else if ((phy_stat & HDMI_PHY_HPD) ||
+				 hdmi->hpd_unreliable)
 				hdmi->rxsense = true;
 
 			dw_hdmi_update_power(hdmi);
@@ -1617,8 +1623,7 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
 	}
 
 	hdmi_writeb(hdmi, intr_stat, HDMI_IH_PHY_STAT0);
-	hdmi_writeb(hdmi, ~(HDMI_IH_PHY_STAT0_HPD | HDMI_IH_PHY_STAT0_RX_SENSE),
-		    HDMI_IH_MUTE_PHY_STAT0);
+	hdmi_writeb(hdmi, ~intr_mask, HDMI_IH_MUTE_PHY_STAT0);
 
 	return IRQ_HANDLED;
 }
@@ -1679,6 +1684,7 @@ int dw_hdmi_bind(struct device *dev, struct device *master,
 	struct device_node *ddc_node;
 	struct dw_hdmi_audio_data audio;
 	struct dw_hdmi *hdmi;
+	u8 intr_mask;
 	int ret;
 	u32 val = 1;
 
@@ -1770,6 +1776,8 @@ int dw_hdmi_bind(struct device *dev, struct device *master,
 
 	initialize_hdmi_ih_mutes(hdmi);
 
+	hdmi->hpd_unreliable = of_property_read_bool(np, "hpd-unreliable");
+
 	ret = devm_request_threaded_irq(dev, irq, dw_hdmi_hardirq,
 					dw_hdmi_irq, IRQF_SHARED,
 					dev_name(dev), hdmi);
@@ -1801,8 +1809,10 @@ int dw_hdmi_bind(struct device *dev, struct device *master,
 		goto err_iahb;
 
 	/* Unmute interrupts */
-	hdmi_writeb(hdmi, ~(HDMI_IH_PHY_STAT0_HPD | HDMI_IH_PHY_STAT0_RX_SENSE),
-		    HDMI_IH_MUTE_PHY_STAT0);
+	intr_mask = hdmi->hpd_unreliable ?
+		    HDMI_IH_PHY_STAT0_RX_SENSE :
+		    HDMI_IH_PHY_STAT0_RX_SENSE | HDMI_IH_PHY_STAT0_HPD;
+	hdmi_writeb(hdmi, ~intr_mask, HDMI_IH_MUTE_PHY_STAT0);
 
 	memset(&pdevinfo, 0, sizeof(pdevinfo));
 	pdevinfo.parent = dev;
-- 
2.6.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/4] drm: bridge/dw-hdmi: detect initial connector state

[Russell King - ARM Linux]

On Fri, Jan 08, 2016 at 10:02:06AM +0100, Philipp Zabel wrote:
> Allow userspace to read the initial connector state via sysfs without
> having to issue a detect manually. There is no reason to keep the state
> unknown during initialization.

Can you describe how it can be unknown?  I've always seen the state to be
correctly initialised on iMX6 irrespective of whether a sink is connected
or not.  However, I always have the FB helper and FB console enabled.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/4] drm: bridge/dw-hdmi: use rx sense for plug detection if hpd is unreliable

[Russell King - ARM Linux]

On Fri, Jan 08, 2016 at 10:02:07AM +0100, Philipp Zabel wrote:
> Due to the voltage divider on the HPD line, the HDMI connector on
> imx6q-sabrelite doesn't reliably detect connected DVI monitors.
> This patch allows to use the RX_SENSE signals as a workaround when
> enabled by a boolean device tree property 'hpd-unreliable'.

There's a got-cha here.  On iMX6S, the RXSENSE interrupts bounce
around madly if the HDMI interface is not fully configured.  I've
seen this many times at boot time (I've been carrying a patch which
reports the HPD/RXSENSE state to the kernel log for a long time now,
it can be rather noisy.)

It's also out-of-spec for reading the EDID: the EDID is only valid
when HPD is asserted.  When HPD is deasserted, the EDID may not be
accessible.  Using RXSENSE opens a window where the EDID may be
unavailable, or may be mid-way through being updated depending on
how the sink hardware works.

With a Yamaha RX-V677 AV receiver and a Panasonic TV, I've observed
this:

initial(-rxsense,-hpd), AV standby, TV standby.
Connected to AV: +rxsense, +hpd
        EDID reads from AV
TV standby->on: -hpd, 2s, +hpd
        EDID reads from TV
AV standby->on: -hpd, 1.2s, +hpd
	EDID reads combined TV/AV
AV on->standby: -hpd, 1.2s, +hpd
        EDID reads from TV

Note that RXSENSE did not drop, but the EDID changed, and that change
was properly signalled via HPD according to the HDMI standard.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/4] drm: bridge/dw-hdmi: detect initial connector state

[Philipp Zabel]

Am Freitag, den 08.01.2016, 11:24 +0000 schrieb Russell King - ARM
Linux:
> On Fri, Jan 08, 2016 at 10:02:06AM +0100, Philipp Zabel wrote:
> > Allow userspace to read the initial connector state via sysfs without
> > having to issue a detect manually. There is no reason to keep the state
> > unknown during initialization.
> 
> Can you describe how it can be unknown?  I've always seen the state to be
> correctly initialised on iMX6 irrespective of whether a sink is connected
> or not.  However, I always have the FB helper and FB console enabled.

I have FB helper and console disabled. Since the kernel doesn't set a
mode automatically, in this case the modes are never requested, and
detect is not called by anyone.

Until userspace issues the detect, either via the DRM API or by writing
"detect" into /sys/class/drm/card0-HDMI-A-1/status, this sysfs file
reads "unknown".

regards
Philipp

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/4] drm: bridge/dw-hdmi: detect initial connector state

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1391 bytes --]

On Fri, Jan 08, 2016 at 10:02:06AM +0100, Philipp Zabel wrote:
> Allow userspace to read the initial connector state via sysfs without
> having to issue a detect manually. There is no reason to keep the state
> unknown during initialization.
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>  drivers/gpu/drm/bridge/dw-hdmi.c | 2 ++
>  1 file changed, 2 insertions(+)

It would seem to me that this should be the default, rather than having
to duplicate this into every driver.

Daniel, can you think of a reason why we wouldn't want to do handle this
in the core so that all drivers can benefit?

Thierry

> 
> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
> index e41ff4b..2388a55 100644
> --- a/drivers/gpu/drm/bridge/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
> @@ -1660,6 +1660,8 @@ static int dw_hdmi_register(struct drm_device *drm, struct dw_hdmi *hdmi)
>  				   DRM_MODE_CONNECTOR_HDMIA);
>  
>  	hdmi->connector.encoder = encoder;
> +	hdmi->connector.status = dw_hdmi_connector_detect(&hdmi->connector,
> +							  false);
>  
>  	drm_mode_connector_attach_encoder(&hdmi->connector, encoder);
>  
> -- 
> 2.6.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel