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"); }
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; }
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);
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.
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
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
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;
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.
dri-devel@lists.freedesktop.org
-
Philipp Zabel -
Russell King - ARM Linux -
Thierry Reding