[PATCH 0/8] drm: bridge: dw_hdmi: Misc enable/disable, CEC and EDID cleanup

[PATCH 0/8] drm: bridge: dw_hdmi: Misc enable/disable, CEC and EDID cleanup

[Jonas Karlman]

This series ensure poweron/poweroff and CEC phys addr invalidation is
happening under drm mode_config mutex lock, and also ensure EDID is
updated (when the dw-hdmi connector is used) after a hotplug pulse.

These changes has mainly been tested on Rockchip devices together with a
forthcoming series that add HDMI 2.0 4K@60Hz support to RK3228, RK3328,
RK3399 and RK3568.

Rockchip use the dw-hdmi connector so this should also be validated with
a driver that use the bridge connector.

Jonas Karlman (8):
  drm: bridge: dw_hdmi: Call poweron/poweroff from atomic enable/disable
  drm: bridge: dw_hdmi: Use passed mode instead of stored previous_mode
  drm: bridge: dw_hdmi: Fold poweron and setup functions
  drm: bridge: dw_hdmi: Remove previous_mode and mode_set
  drm: bridge: dw_hdmi: Invalidate CEC phys addr from connector detect
  drm: bridge: dw_hdmi: Remove cec_notifier_mutex
  drm: bridge: dw_hdmi: Update EDID during hotplug processing
  drm: bridge: dw_hdmi: Use display_info is_hdmi and has_audio

 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 145 ++++++----------------
 1 file changed, 39 insertions(+), 106 deletions(-)

-- 
2.45.2

[PATCH 1/8] drm: bridge: dw_hdmi: Call poweron/poweroff from atomic enable/disable

[Jonas Karlman]

Change to only call poweron/poweroff from atomic_enable/atomic_disable
ops instead of trying to keep a bridge_is_on state and poweron/off in
the hotplug irq handler.

A benefit of this is that drm mode_config mutex is always held at
poweron/off, something that may reduce the need for our own mutex.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 33 ++---------------------
 1 file changed, 2 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 9f2bc932c371..34bc6f4754b8 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -172,7 +172,6 @@ struct dw_hdmi {
 	enum drm_connector_force force;	/* mutex-protected force state */
 	struct drm_connector *curr_conn;/* current connector (only valid when !disabled) */
 	bool disabled;			/* DRM has disabled our bridge */
-	bool bridge_is_on;		/* indicates the bridge is on */
 	bool rxsense;			/* rxsense state */
 	u8 phy_mask;			/* desired phy int mask settings */
 	u8 mc_clkdis;			/* clock disable register */
@@ -2383,8 +2382,6 @@ static void initialize_hdmi_ih_mutes(struct dw_hdmi *hdmi)
 
 static void dw_hdmi_poweron(struct dw_hdmi *hdmi)
 {
-	hdmi->bridge_is_on = true;
-
 	/*
 	 * The curr_conn field is guaranteed to be valid here, as this function
 	 * is only be called when !hdmi->disabled.
@@ -2398,30 +2395,6 @@ static void dw_hdmi_poweroff(struct dw_hdmi *hdmi)
 		hdmi->phy.ops->disable(hdmi, hdmi->phy.data);
 		hdmi->phy.enabled = false;
 	}
-
-	hdmi->bridge_is_on = false;
-}
-
-static void dw_hdmi_update_power(struct dw_hdmi *hdmi)
-{
-	int force = hdmi->force;
-
-	if (hdmi->disabled) {
-		force = DRM_FORCE_OFF;
-	} else if (force == DRM_FORCE_UNSPECIFIED) {
-		if (hdmi->rxsense)
-			force = DRM_FORCE_ON;
-		else
-			force = DRM_FORCE_OFF;
-	}
-
-	if (force == DRM_FORCE_OFF) {
-		if (hdmi->bridge_is_on)
-			dw_hdmi_poweroff(hdmi);
-	} else {
-		if (!hdmi->bridge_is_on)
-			dw_hdmi_poweron(hdmi);
-	}
 }
 
 /*
@@ -2546,7 +2519,6 @@ static void dw_hdmi_connector_force(struct drm_connector *connector)
 
 	mutex_lock(&hdmi->mutex);
 	hdmi->force = connector->force;
-	dw_hdmi_update_power(hdmi);
 	dw_hdmi_update_phy_mask(hdmi);
 	mutex_unlock(&hdmi->mutex);
 }
@@ -2955,7 +2927,7 @@ static void dw_hdmi_bridge_atomic_disable(struct drm_bridge *bridge,
 	mutex_lock(&hdmi->mutex);
 	hdmi->disabled = true;
 	hdmi->curr_conn = NULL;
-	dw_hdmi_update_power(hdmi);
+	dw_hdmi_poweroff(hdmi);
 	dw_hdmi_update_phy_mask(hdmi);
 	handle_plugged_change(hdmi, false);
 	mutex_unlock(&hdmi->mutex);
@@ -2974,7 +2946,7 @@ static void dw_hdmi_bridge_atomic_enable(struct drm_bridge *bridge,
 	mutex_lock(&hdmi->mutex);
 	hdmi->disabled = false;
 	hdmi->curr_conn = connector;
-	dw_hdmi_update_power(hdmi);
+	dw_hdmi_poweron(hdmi);
 	dw_hdmi_update_phy_mask(hdmi);
 	handle_plugged_change(hdmi, true);
 	mutex_unlock(&hdmi->mutex);
@@ -3073,7 +3045,6 @@ void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
 		if (hpd)
 			hdmi->rxsense = true;
 
-		dw_hdmi_update_power(hdmi);
 		dw_hdmi_update_phy_mask(hdmi);
 	}
 	mutex_unlock(&hdmi->mutex);
-- 
2.45.2

[PATCH 2/8] drm: bridge: dw_hdmi: Use passed mode instead of stored previous_mode

[Jonas Karlman]

Use the passed mode instead of mixing use of passed mode and the stored
previous_mode. The passed mode is currenly always the previous_mode.

Also fix a small type and add a variable to help shorten a code line.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 34bc6f4754b8..ce4d4d06f758 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2241,6 +2241,7 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi,
 			 const struct drm_connector *connector,
 			 const struct drm_display_mode *mode)
 {
+	const struct drm_display_info *display = &connector->display_info;
 	int ret;
 
 	hdmi_disable_overflow_interrupts(hdmi);
@@ -2286,12 +2287,10 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi,
 	hdmi->hdmi_data.video_mode.mdataenablepolarity = true;
 
 	/* HDMI Initialization Step B.1 */
-	hdmi_av_composer(hdmi, &connector->display_info, mode);
+	hdmi_av_composer(hdmi, display, mode);
 
-	/* HDMI Initializateion Step B.2 */
-	ret = hdmi->phy.ops->init(hdmi, hdmi->phy.data,
-				  &connector->display_info,
-				  &hdmi->previous_mode);
+	/* HDMI Initialization Step B.2 */
+	ret = hdmi->phy.ops->init(hdmi, hdmi->phy.data, display, mode);
 	if (ret)
 		return ret;
 	hdmi->phy.enabled = true;
-- 
2.45.2

[PATCH 3/8] drm: bridge: dw_hdmi: Fold poweron and setup functions

[Jonas Karlman]

Fold the poweron and setup functions into one function and use the
adjusted_mode directly from the new crtc_state to remove the need of
storing previous_mode.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index ce4d4d06f758..76048fc9fd14 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2237,9 +2237,9 @@ static void hdmi_disable_overflow_interrupts(struct dw_hdmi *hdmi)
 		    HDMI_IH_MUTE_FC_STAT2);
 }
 
-static int dw_hdmi_setup(struct dw_hdmi *hdmi,
-			 const struct drm_connector *connector,
-			 const struct drm_display_mode *mode)
+static int dw_hdmi_poweron(struct dw_hdmi *hdmi,
+			   const struct drm_connector *connector,
+			   const struct drm_display_mode *mode)
 {
 	const struct drm_display_info *display = &connector->display_info;
 	int ret;
@@ -2379,15 +2379,6 @@ static void initialize_hdmi_ih_mutes(struct dw_hdmi *hdmi)
 	hdmi_writeb(hdmi, ih_mute, HDMI_IH_MUTE);
 }
 
-static void dw_hdmi_poweron(struct dw_hdmi *hdmi)
-{
-	/*
-	 * The curr_conn field is guaranteed to be valid here, as this function
-	 * is only be called when !hdmi->disabled.
-	 */
-	dw_hdmi_setup(hdmi, hdmi->curr_conn, &hdmi->previous_mode);
-}
-
 static void dw_hdmi_poweroff(struct dw_hdmi *hdmi)
 {
 	if (hdmi->phy.enabled) {
@@ -2937,15 +2928,19 @@ static void dw_hdmi_bridge_atomic_enable(struct drm_bridge *bridge,
 {
 	struct dw_hdmi *hdmi = bridge->driver_private;
 	struct drm_atomic_state *state = old_state->base.state;
+	const struct drm_display_mode *mode;
 	struct drm_connector *connector;
+	struct drm_crtc *crtc;
 
 	connector = drm_atomic_get_new_connector_for_encoder(state,
 							     bridge->encoder);
+	crtc = drm_atomic_get_new_connector_state(state, connector)->crtc;
+	mode = &drm_atomic_get_new_crtc_state(state, crtc)->adjusted_mode;
 
 	mutex_lock(&hdmi->mutex);
 	hdmi->disabled = false;
 	hdmi->curr_conn = connector;
-	dw_hdmi_poweron(hdmi);
+	dw_hdmi_poweron(hdmi, connector, mode);
 	dw_hdmi_update_phy_mask(hdmi);
 	handle_plugged_change(hdmi, true);
 	mutex_unlock(&hdmi->mutex);
-- 
2.45.2

[PATCH 4/8] drm: bridge: dw_hdmi: Remove previous_mode and mode_set

[Jonas Karlman]

With the use of adjusted_mode directly from the crtc_state there is no
longer a need to store a copy in previous_mode, remove it and the now
unneeded mode_set ops.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 19 +------------------
 1 file changed, 1 insertion(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 76048fc9fd14..9ecf038f551e 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -157,8 +157,6 @@ struct dw_hdmi {
 		bool enabled;
 	} phy;
 
-	struct drm_display_mode previous_mode;
-
 	struct i2c_adapter *ddc;
 	void __iomem *regs;
 	bool sink_is_hdmi;
@@ -168,7 +166,7 @@ struct dw_hdmi {
 	struct pinctrl_state *default_state;
 	struct pinctrl_state *unwedge_state;
 
-	struct mutex mutex;		/* for state below and previous_mode */
+	struct mutex mutex;		/* for state below */
 	enum drm_connector_force force;	/* mutex-protected force state */
 	struct drm_connector *curr_conn;/* current connector (only valid when !disabled) */
 	bool disabled;			/* DRM has disabled our bridge */
@@ -2895,20 +2893,6 @@ dw_hdmi_bridge_mode_valid(struct drm_bridge *bridge,
 	return mode_status;
 }
 
-static void dw_hdmi_bridge_mode_set(struct drm_bridge *bridge,
-				    const struct drm_display_mode *orig_mode,
-				    const struct drm_display_mode *mode)
-{
-	struct dw_hdmi *hdmi = bridge->driver_private;
-
-	mutex_lock(&hdmi->mutex);
-
-	/* Store the display mode for plugin/DKMS poweron events */
-	drm_mode_copy(&hdmi->previous_mode, mode);
-
-	mutex_unlock(&hdmi->mutex);
-}
-
 static void dw_hdmi_bridge_atomic_disable(struct drm_bridge *bridge,
 					  struct drm_bridge_state *old_state)
 {
@@ -2972,7 +2956,6 @@ static const struct drm_bridge_funcs dw_hdmi_bridge_funcs = {
 	.atomic_get_input_bus_fmts = dw_hdmi_bridge_atomic_get_input_bus_fmts,
 	.atomic_enable = dw_hdmi_bridge_atomic_enable,
 	.atomic_disable = dw_hdmi_bridge_atomic_disable,
-	.mode_set = dw_hdmi_bridge_mode_set,
 	.mode_valid = dw_hdmi_bridge_mode_valid,
 	.detect = dw_hdmi_bridge_detect,
 	.edid_read = dw_hdmi_bridge_edid_read,
-- 
2.45.2

[PATCH 5/8] drm: bridge: dw_hdmi: Invalidate CEC phys addr from connector detect

[Jonas Karlman]

Wait until the connector detect ops is called to invalidate CEC phys
addr instead of doing it directly from the irq handler.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 9ecf038f551e..0814ca181f04 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2455,7 +2455,17 @@ dw_hdmi_connector_detect(struct drm_connector *connector, bool force)
 {
 	struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi,
 					     connector);
-	return dw_hdmi_detect(hdmi);
+	enum drm_connector_status status;
+
+	status = dw_hdmi_detect(hdmi);
+
+	if (status == connector_status_disconnected) {
+		mutex_lock(&hdmi->cec_notifier_mutex);
+		cec_notifier_phys_addr_invalidate(hdmi->cec_notifier);
+		mutex_unlock(&hdmi->cec_notifier_mutex);
+	}
+
+	return status;
 }
 
 static int dw_hdmi_connector_get_modes(struct drm_connector *connector)
@@ -3066,12 +3076,6 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
 				       phy_stat & HDMI_PHY_HPD,
 				       phy_stat & HDMI_PHY_RX_SENSE);
 
-		if ((phy_stat & (HDMI_PHY_RX_SENSE | HDMI_PHY_HPD)) == 0) {
-			mutex_lock(&hdmi->cec_notifier_mutex);
-			cec_notifier_phys_addr_invalidate(hdmi->cec_notifier);
-			mutex_unlock(&hdmi->cec_notifier_mutex);
-		}
-
 		if (phy_stat & HDMI_PHY_HPD)
 			status = connector_status_connected;
 
-- 
2.45.2

[PATCH 6/8] drm: bridge: dw_hdmi: Remove cec_notifier_mutex

[Jonas Karlman]

With CEC phys addr invalidation moved away from the irq handler there is
no longer a need for cec_notifier_mutex, remove it.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 0814ca181f04..256e00a97a9a 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -189,7 +189,6 @@ struct dw_hdmi {
 	void (*enable_audio)(struct dw_hdmi *hdmi);
 	void (*disable_audio)(struct dw_hdmi *hdmi);
 
-	struct mutex cec_notifier_mutex;
 	struct cec_notifier *cec_notifier;
 
 	hdmi_codec_plugged_cb plugged_cb;
@@ -2459,11 +2458,8 @@ dw_hdmi_connector_detect(struct drm_connector *connector, bool force)
 
 	status = dw_hdmi_detect(hdmi);
 
-	if (status == connector_status_disconnected) {
-		mutex_lock(&hdmi->cec_notifier_mutex);
+	if (status == connector_status_disconnected)
 		cec_notifier_phys_addr_invalidate(hdmi->cec_notifier);
-		mutex_unlock(&hdmi->cec_notifier_mutex);
-	}
 
 	return status;
 }
@@ -2577,9 +2573,7 @@ static int dw_hdmi_connector_create(struct dw_hdmi *hdmi)
 	if (!notifier)
 		return -ENOMEM;
 
-	mutex_lock(&hdmi->cec_notifier_mutex);
 	hdmi->cec_notifier = notifier;
-	mutex_unlock(&hdmi->cec_notifier_mutex);
 
 	return 0;
 }
@@ -2877,10 +2871,8 @@ static void dw_hdmi_bridge_detach(struct drm_bridge *bridge)
 {
 	struct dw_hdmi *hdmi = bridge->driver_private;
 
-	mutex_lock(&hdmi->cec_notifier_mutex);
 	cec_notifier_conn_unregister(hdmi->cec_notifier);
 	hdmi->cec_notifier = NULL;
-	mutex_unlock(&hdmi->cec_notifier_mutex);
 }
 
 static enum drm_mode_status
@@ -3303,7 +3295,6 @@ struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
 
 	mutex_init(&hdmi->mutex);
 	mutex_init(&hdmi->audio_mutex);
-	mutex_init(&hdmi->cec_notifier_mutex);
 	spin_lock_init(&hdmi->audio_lock);
 
 	ret = dw_hdmi_parse_dt(hdmi);
-- 
2.45.2

[PATCH 7/8] drm: bridge: dw_hdmi: Update EDID during hotplug processing

[Jonas Karlman]

Update successfully read EDID during hotplug processing to ensure the
connector diplay_info is always up-to-date.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 256e00a97a9a..a9c39584d31b 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2458,6 +2458,18 @@ dw_hdmi_connector_detect(struct drm_connector *connector, bool force)
 
 	status = dw_hdmi_detect(hdmi);
 
+	/* Update EDID during hotplug processing (force=false) */
+	if (status == connector_status_connected && !force) {
+		const struct drm_edid *drm_edid;
+
+		drm_edid = dw_hdmi_edid_read(hdmi, connector);
+		if (drm_edid)
+			drm_edid_connector_update(connector, drm_edid);
+		cec_notifier_set_phys_addr(hdmi->cec_notifier,
+			connector->display_info.source_physical_address);
+		drm_edid_free(drm_edid);
+	}
+
 	if (status == connector_status_disconnected)
 		cec_notifier_phys_addr_invalidate(hdmi->cec_notifier);
 
-- 
2.45.2

[PATCH 8/8] drm: bridge: dw_hdmi: Use display_info is_hdmi and has_audio

[Jonas Karlman]

drm_edid_connector_update() is being called from bridge connector ops
and from detect and get_modes ops for dw-hdmi connector.

Change to use is_hdmi and has_audio from display_info directly instead
of keeping our own state in sink_is_hdmi and sink_has_audio.

Also remove the old and unused edid struct member and related define.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 32 ++++-------------------
 1 file changed, 5 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index a9c39584d31b..e162c2786178 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -43,8 +43,6 @@
 #define DDC_CI_ADDR		0x37
 #define DDC_SEGMENT_ADDR	0x30
 
-#define HDMI_EDID_LEN		512
-
 /* DW-HDMI Controller >= 0x200a are at least compliant with SCDC version 1 */
 #define SCDC_MIN_SOURCE_VERSION	0x1
 
@@ -148,8 +146,6 @@ struct dw_hdmi {
 
 	int vic;
 
-	u8 edid[HDMI_EDID_LEN];
-
 	struct {
 		const struct dw_hdmi_phy_ops *ops;
 		const char *name;
@@ -159,8 +155,6 @@ struct dw_hdmi {
 
 	struct i2c_adapter *ddc;
 	void __iomem *regs;
-	bool sink_is_hdmi;
-	bool sink_has_audio;
 
 	struct pinctrl *pinctrl;
 	struct pinctrl_state *default_state;
@@ -2041,7 +2035,7 @@ static void hdmi_av_composer(struct dw_hdmi *hdmi,
 		HDMI_FC_INVIDCONF_IN_I_P_INTERLACED :
 		HDMI_FC_INVIDCONF_IN_I_P_PROGRESSIVE;
 
-	inv_val |= hdmi->sink_is_hdmi ?
+	inv_val |= display->is_hdmi ?
 		HDMI_FC_INVIDCONF_DVI_MODEZ_HDMI_MODE :
 		HDMI_FC_INVIDCONF_DVI_MODEZ_DVI_MODE;
 
@@ -2275,7 +2269,7 @@ static int dw_hdmi_poweron(struct dw_hdmi *hdmi,
 	if (hdmi->hdmi_data.enc_out_bus_format == MEDIA_BUS_FMT_FIXED)
 		hdmi->hdmi_data.enc_out_bus_format = MEDIA_BUS_FMT_RGB888_1X24;
 
-	hdmi->hdmi_data.rgb_limited_range = hdmi->sink_is_hdmi &&
+	hdmi->hdmi_data.rgb_limited_range = display->is_hdmi &&
 		drm_default_rgb_quant_range(mode) ==
 		HDMI_QUANTIZATION_RANGE_LIMITED;
 
@@ -2295,7 +2289,7 @@ static int dw_hdmi_poweron(struct dw_hdmi *hdmi,
 	/* HDMI Initialization Step B.3 */
 	dw_hdmi_enable_video_path(hdmi);
 
-	if (hdmi->sink_has_audio) {
+	if (display->has_audio) {
 		dev_dbg(hdmi->dev, "sink has audio support\n");
 
 		/* HDMI Initialization Step E - Configure audio */
@@ -2304,7 +2298,7 @@ static int dw_hdmi_poweron(struct dw_hdmi *hdmi,
 	}
 
 	/* not for DVI mode */
-	if (hdmi->sink_is_hdmi) {
+	if (display->is_hdmi) {
 		dev_dbg(hdmi->dev, "%s HDMI mode\n", __func__);
 
 		/* HDMI Initialization Step F - Configure AVI InfoFrame */
@@ -2418,29 +2412,13 @@ static const struct drm_edid *dw_hdmi_edid_read(struct dw_hdmi *hdmi,
 						struct drm_connector *connector)
 {
 	const struct drm_edid *drm_edid;
-	const struct edid *edid;
 
 	if (!hdmi->ddc)
 		return NULL;
 
 	drm_edid = drm_edid_read_ddc(connector, hdmi->ddc);
-	if (!drm_edid) {
+	if (!drm_edid)
 		dev_dbg(hdmi->dev, "failed to get edid\n");
-		return NULL;
-	}
-
-	/*
-	 * FIXME: This should use connector->display_info.is_hdmi and
-	 * connector->display_info.has_audio from a path that has read the EDID
-	 * and called drm_edid_connector_update().
-	 */
-	edid = drm_edid_raw(drm_edid);
-
-	dev_dbg(hdmi->dev, "got edid: width[%d] x height[%d]\n",
-		edid->width_cm, edid->height_cm);
-
-	hdmi->sink_is_hdmi = drm_detect_hdmi_monitor(edid);
-	hdmi->sink_has_audio = drm_detect_monitor_audio(edid);
 
 	return drm_edid;
 }
-- 
2.45.2

Re: [PATCH 0/8] drm: bridge: dw_hdmi: Misc enable/disable, CEC and EDID cleanup

[Christian Hewitt]

> On 11 Jun 2024, at 7:50 PM, Jonas Karlman <jonas@kwiboo.se> wrote:
> 
> This series ensure poweron/poweroff and CEC phys addr invalidation is
> happening under drm mode_config mutex lock, and also ensure EDID is
> updated (when the dw-hdmi connector is used) after a hotplug pulse.
> 
> These changes has mainly been tested on Rockchip devices together with a
> forthcoming series that add HDMI 2.0 4K@60Hz support to RK3228, RK3328,
> RK3399 and RK3568.
> 
> Rockchip use the dw-hdmi connector so this should also be validated with
> a driver that use the bridge connector.

I’m not sure my level of kernel fiddling counts as proper validation
(Neil would be a more authoritative tester) but FWIW, I picked the
series to my 6.10-rc4 dev branch (for Amlogic devices supported by the
LibreELEC distro) and AFAICT everything is still working. Tested with
Amlogic GXBB, GXL and G12B boards.

Tested-by: Christian Hewitt <christianshewitt@gmail.com>

> Jonas Karlman (8):
>  drm: bridge: dw_hdmi: Call poweron/poweroff from atomic enable/disable
>  drm: bridge: dw_hdmi: Use passed mode instead of stored previous_mode
>  drm: bridge: dw_hdmi: Fold poweron and setup functions
>  drm: bridge: dw_hdmi: Remove previous_mode and mode_set
>  drm: bridge: dw_hdmi: Invalidate CEC phys addr from connector detect
>  drm: bridge: dw_hdmi: Remove cec_notifier_mutex
>  drm: bridge: dw_hdmi: Update EDID during hotplug processing
>  drm: bridge: dw_hdmi: Use display_info is_hdmi and has_audio
> 
> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 145 ++++++----------------
> 1 file changed, 39 insertions(+), 106 deletions(-)

Re: [PATCH 8/8] drm: bridge: dw_hdmi: Use display_info is_hdmi and has_audio

[Neil Armstrong]

On 11/06/2024 17:51, Jonas Karlman wrote:
> drm_edid_connector_update() is being called from bridge connector ops
> and from detect and get_modes ops for dw-hdmi connector.
> 
> Change to use is_hdmi and has_audio from display_info directly instead
> of keeping our own state in sink_is_hdmi and sink_has_audio.
> 
> Also remove the old and unused edid struct member and related define.
> 
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
>   drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 32 ++++-------------------
>   1 file changed, 5 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index a9c39584d31b..e162c2786178 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -43,8 +43,6 @@
>   #define DDC_CI_ADDR		0x37
>   #define DDC_SEGMENT_ADDR	0x30
>   
> -#define HDMI_EDID_LEN		512
> -
>   /* DW-HDMI Controller >= 0x200a are at least compliant with SCDC version 1 */
>   #define SCDC_MIN_SOURCE_VERSION	0x1
>   
> @@ -148,8 +146,6 @@ struct dw_hdmi {
>   
>   	int vic;
>   
> -	u8 edid[HDMI_EDID_LEN];
> -
>   	struct {
>   		const struct dw_hdmi_phy_ops *ops;
>   		const char *name;
> @@ -159,8 +155,6 @@ struct dw_hdmi {
>   
>   	struct i2c_adapter *ddc;
>   	void __iomem *regs;
> -	bool sink_is_hdmi;
> -	bool sink_has_audio;
>   
>   	struct pinctrl *pinctrl;
>   	struct pinctrl_state *default_state;
> @@ -2041,7 +2035,7 @@ static void hdmi_av_composer(struct dw_hdmi *hdmi,
>   		HDMI_FC_INVIDCONF_IN_I_P_INTERLACED :
>   		HDMI_FC_INVIDCONF_IN_I_P_PROGRESSIVE;
>   
> -	inv_val |= hdmi->sink_is_hdmi ?
> +	inv_val |= display->is_hdmi ?
>   		HDMI_FC_INVIDCONF_DVI_MODEZ_HDMI_MODE :
>   		HDMI_FC_INVIDCONF_DVI_MODEZ_DVI_MODE;
>   
> @@ -2275,7 +2269,7 @@ static int dw_hdmi_poweron(struct dw_hdmi *hdmi,
>   	if (hdmi->hdmi_data.enc_out_bus_format == MEDIA_BUS_FMT_FIXED)
>   		hdmi->hdmi_data.enc_out_bus_format = MEDIA_BUS_FMT_RGB888_1X24;
>   
> -	hdmi->hdmi_data.rgb_limited_range = hdmi->sink_is_hdmi &&
> +	hdmi->hdmi_data.rgb_limited_range = display->is_hdmi &&
>   		drm_default_rgb_quant_range(mode) ==
>   		HDMI_QUANTIZATION_RANGE_LIMITED;
>   
> @@ -2295,7 +2289,7 @@ static int dw_hdmi_poweron(struct dw_hdmi *hdmi,
>   	/* HDMI Initialization Step B.3 */
>   	dw_hdmi_enable_video_path(hdmi);
>   
> -	if (hdmi->sink_has_audio) {
> +	if (display->has_audio) {
>   		dev_dbg(hdmi->dev, "sink has audio support\n");
>   
>   		/* HDMI Initialization Step E - Configure audio */
> @@ -2304,7 +2298,7 @@ static int dw_hdmi_poweron(struct dw_hdmi *hdmi,
>   	}
>   
>   	/* not for DVI mode */
> -	if (hdmi->sink_is_hdmi) {
> +	if (display->is_hdmi) {
>   		dev_dbg(hdmi->dev, "%s HDMI mode\n", __func__);
>   
>   		/* HDMI Initialization Step F - Configure AVI InfoFrame */
> @@ -2418,29 +2412,13 @@ static const struct drm_edid *dw_hdmi_edid_read(struct dw_hdmi *hdmi,
>   						struct drm_connector *connector)
>   {
>   	const struct drm_edid *drm_edid;
> -	const struct edid *edid;
>   
>   	if (!hdmi->ddc)
>   		return NULL;
>   
>   	drm_edid = drm_edid_read_ddc(connector, hdmi->ddc);
> -	if (!drm_edid) {
> +	if (!drm_edid)
>   		dev_dbg(hdmi->dev, "failed to get edid\n");
> -		return NULL;
> -	}
> -
> -	/*
> -	 * FIXME: This should use connector->display_info.is_hdmi and
> -	 * connector->display_info.has_audio from a path that has read the EDID
> -	 * and called drm_edid_connector_update().
> -	 */
> -	edid = drm_edid_raw(drm_edid);
> -
> -	dev_dbg(hdmi->dev, "got edid: width[%d] x height[%d]\n",
> -		edid->width_cm, edid->height_cm);
> -
> -	hdmi->sink_is_hdmi = drm_detect_hdmi_monitor(edid);
> -	hdmi->sink_has_audio = drm_detect_monitor_audio(edid);
>   
>   	return drm_edid;
>   }

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Re: [PATCH 5/8] drm: bridge: dw_hdmi: Invalidate CEC phys addr from connector detect

[Neil Armstrong]

On 11/06/2024 17:50, Jonas Karlman wrote:
> Wait until the connector detect ops is called to invalidate CEC phys
> addr instead of doing it directly from the irq handler.
> 
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
>   drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 18 +++++++++++-------
>   1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 9ecf038f551e..0814ca181f04 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -2455,7 +2455,17 @@ dw_hdmi_connector_detect(struct drm_connector *connector, bool force)
>   {
>   	struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi,
>   					     connector);
> -	return dw_hdmi_detect(hdmi);
> +	enum drm_connector_status status;
> +
> +	status = dw_hdmi_detect(hdmi);
> +
> +	if (status == connector_status_disconnected) {
> +		mutex_lock(&hdmi->cec_notifier_mutex);
> +		cec_notifier_phys_addr_invalidate(hdmi->cec_notifier);
> +		mutex_unlock(&hdmi->cec_notifier_mutex);
> +	}
> +
> +	return status;
>   }
>   
>   static int dw_hdmi_connector_get_modes(struct drm_connector *connector)
> @@ -3066,12 +3076,6 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
>   				       phy_stat & HDMI_PHY_HPD,
>   				       phy_stat & HDMI_PHY_RX_SENSE);
>   
> -		if ((phy_stat & (HDMI_PHY_RX_SENSE | HDMI_PHY_HPD)) == 0) {
> -			mutex_lock(&hdmi->cec_notifier_mutex);
> -			cec_notifier_phys_addr_invalidate(hdmi->cec_notifier);
> -			mutex_unlock(&hdmi->cec_notifier_mutex);
> -		}
> -
>   		if (phy_stat & HDMI_PHY_HPD)
>   			status = connector_status_connected;
>   

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Re: [PATCH 6/8] drm: bridge: dw_hdmi: Remove cec_notifier_mutex

[Neil Armstrong]

On 11/06/2024 17:50, Jonas Karlman wrote:
> With CEC phys addr invalidation moved away from the irq handler there is
> no longer a need for cec_notifier_mutex, remove it.
> 
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
>   drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 11 +----------
>   1 file changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 0814ca181f04..256e00a97a9a 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -189,7 +189,6 @@ struct dw_hdmi {
>   	void (*enable_audio)(struct dw_hdmi *hdmi);
>   	void (*disable_audio)(struct dw_hdmi *hdmi);
>   
> -	struct mutex cec_notifier_mutex;
>   	struct cec_notifier *cec_notifier;
>   
>   	hdmi_codec_plugged_cb plugged_cb;
> @@ -2459,11 +2458,8 @@ dw_hdmi_connector_detect(struct drm_connector *connector, bool force)
>   
>   	status = dw_hdmi_detect(hdmi);
>   
> -	if (status == connector_status_disconnected) {
> -		mutex_lock(&hdmi->cec_notifier_mutex);
> +	if (status == connector_status_disconnected)
>   		cec_notifier_phys_addr_invalidate(hdmi->cec_notifier);
> -		mutex_unlock(&hdmi->cec_notifier_mutex);
> -	}
>   
>   	return status;
>   }
> @@ -2577,9 +2573,7 @@ static int dw_hdmi_connector_create(struct dw_hdmi *hdmi)
>   	if (!notifier)
>   		return -ENOMEM;
>   
> -	mutex_lock(&hdmi->cec_notifier_mutex);
>   	hdmi->cec_notifier = notifier;
> -	mutex_unlock(&hdmi->cec_notifier_mutex);
>   
>   	return 0;
>   }
> @@ -2877,10 +2871,8 @@ static void dw_hdmi_bridge_detach(struct drm_bridge *bridge)
>   {
>   	struct dw_hdmi *hdmi = bridge->driver_private;
>   
> -	mutex_lock(&hdmi->cec_notifier_mutex);
>   	cec_notifier_conn_unregister(hdmi->cec_notifier);
>   	hdmi->cec_notifier = NULL;
> -	mutex_unlock(&hdmi->cec_notifier_mutex);
>   }
>   
>   static enum drm_mode_status
> @@ -3303,7 +3295,6 @@ struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
>   
>   	mutex_init(&hdmi->mutex);
>   	mutex_init(&hdmi->audio_mutex);
> -	mutex_init(&hdmi->cec_notifier_mutex);
>   	spin_lock_init(&hdmi->audio_lock);
>   
>   	ret = dw_hdmi_parse_dt(hdmi);

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Re: [PATCH 1/8] drm: bridge: dw_hdmi: Call poweron/poweroff from atomic enable/disable

[Neil Armstrong]

On 11/06/2024 17:50, Jonas Karlman wrote:
> Change to only call poweron/poweroff from atomic_enable/atomic_disable
> ops instead of trying to keep a bridge_is_on state and poweron/off in
> the hotplug irq handler.
> 
> A benefit of this is that drm mode_config mutex is always held at
> poweron/off, something that may reduce the need for our own mutex.
> 
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
>   drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 33 ++---------------------
>   1 file changed, 2 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 9f2bc932c371..34bc6f4754b8 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -172,7 +172,6 @@ struct dw_hdmi {
>   	enum drm_connector_force force;	/* mutex-protected force state */
>   	struct drm_connector *curr_conn;/* current connector (only valid when !disabled) */
>   	bool disabled;			/* DRM has disabled our bridge */
> -	bool bridge_is_on;		/* indicates the bridge is on */
>   	bool rxsense;			/* rxsense state */
>   	u8 phy_mask;			/* desired phy int mask settings */
>   	u8 mc_clkdis;			/* clock disable register */
> @@ -2383,8 +2382,6 @@ static void initialize_hdmi_ih_mutes(struct dw_hdmi *hdmi)
>   
>   static void dw_hdmi_poweron(struct dw_hdmi *hdmi)
>   {
> -	hdmi->bridge_is_on = true;
> -
>   	/*
>   	 * The curr_conn field is guaranteed to be valid here, as this function
>   	 * is only be called when !hdmi->disabled.
> @@ -2398,30 +2395,6 @@ static void dw_hdmi_poweroff(struct dw_hdmi *hdmi)
>   		hdmi->phy.ops->disable(hdmi, hdmi->phy.data);
>   		hdmi->phy.enabled = false;
>   	}
> -
> -	hdmi->bridge_is_on = false;
> -}
> -
> -static void dw_hdmi_update_power(struct dw_hdmi *hdmi)
> -{
> -	int force = hdmi->force;
> -
> -	if (hdmi->disabled) {
> -		force = DRM_FORCE_OFF;
> -	} else if (force == DRM_FORCE_UNSPECIFIED) {
> -		if (hdmi->rxsense)
> -			force = DRM_FORCE_ON;
> -		else
> -			force = DRM_FORCE_OFF;
> -	}

This means we always poweron the bridge even if rxsense is false ?

Neil

> -
> -	if (force == DRM_FORCE_OFF) {
> -		if (hdmi->bridge_is_on)
> -			dw_hdmi_poweroff(hdmi);
> -	} else {
> -		if (!hdmi->bridge_is_on)
> -			dw_hdmi_poweron(hdmi);
> -	}
>   }
>   
>   /*
> @@ -2546,7 +2519,6 @@ static void dw_hdmi_connector_force(struct drm_connector *connector)
>   
>   	mutex_lock(&hdmi->mutex);
>   	hdmi->force = connector->force;
> -	dw_hdmi_update_power(hdmi);
>   	dw_hdmi_update_phy_mask(hdmi);
>   	mutex_unlock(&hdmi->mutex);
>   }
> @@ -2955,7 +2927,7 @@ static void dw_hdmi_bridge_atomic_disable(struct drm_bridge *bridge,
>   	mutex_lock(&hdmi->mutex);
>   	hdmi->disabled = true;
>   	hdmi->curr_conn = NULL;
> -	dw_hdmi_update_power(hdmi);
> +	dw_hdmi_poweroff(hdmi);
>   	dw_hdmi_update_phy_mask(hdmi);
>   	handle_plugged_change(hdmi, false);
>   	mutex_unlock(&hdmi->mutex);
> @@ -2974,7 +2946,7 @@ static void dw_hdmi_bridge_atomic_enable(struct drm_bridge *bridge,
>   	mutex_lock(&hdmi->mutex);
>   	hdmi->disabled = false;
>   	hdmi->curr_conn = connector;
> -	dw_hdmi_update_power(hdmi);
> +	dw_hdmi_poweron(hdmi);
>   	dw_hdmi_update_phy_mask(hdmi);
>   	handle_plugged_change(hdmi, true);
>   	mutex_unlock(&hdmi->mutex);
> @@ -3073,7 +3045,6 @@ void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
>   		if (hpd)
>   			hdmi->rxsense = true;
>   
> -		dw_hdmi_update_power(hdmi);
>   		dw_hdmi_update_phy_mask(hdmi);
>   	}
>   	mutex_unlock(&hdmi->mutex);

Re: [PATCH 1/8] drm: bridge: dw_hdmi: Call poweron/poweroff from atomic enable/disable

[Jonas Karlman]

Hi Neil,

On 2024-06-24 11:23, Neil Armstrong wrote:
> On 11/06/2024 17:50, Jonas Karlman wrote:
>> Change to only call poweron/poweroff from atomic_enable/atomic_disable
>> ops instead of trying to keep a bridge_is_on state and poweron/off in
>> the hotplug irq handler.
>>
>> A benefit of this is that drm mode_config mutex is always held at
>> poweron/off, something that may reduce the need for our own mutex.
>>
>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>> ---
>>   drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 33 ++---------------------
>>   1 file changed, 2 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> index 9f2bc932c371..34bc6f4754b8 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> @@ -172,7 +172,6 @@ struct dw_hdmi {
>>   	enum drm_connector_force force;	/* mutex-protected force state */
>>   	struct drm_connector *curr_conn;/* current connector (only valid when !disabled) */
>>   	bool disabled;			/* DRM has disabled our bridge */
>> -	bool bridge_is_on;		/* indicates the bridge is on */
>>   	bool rxsense;			/* rxsense state */
>>   	u8 phy_mask;			/* desired phy int mask settings */
>>   	u8 mc_clkdis;			/* clock disable register */
>> @@ -2383,8 +2382,6 @@ static void initialize_hdmi_ih_mutes(struct dw_hdmi *hdmi)
>>   
>>   static void dw_hdmi_poweron(struct dw_hdmi *hdmi)
>>   {
>> -	hdmi->bridge_is_on = true;
>> -
>>   	/*
>>   	 * The curr_conn field is guaranteed to be valid here, as this function
>>   	 * is only be called when !hdmi->disabled.
>> @@ -2398,30 +2395,6 @@ static void dw_hdmi_poweroff(struct dw_hdmi *hdmi)
>>   		hdmi->phy.ops->disable(hdmi, hdmi->phy.data);
>>   		hdmi->phy.enabled = false;
>>   	}
>> -
>> -	hdmi->bridge_is_on = false;
>> -}
>> -
>> -static void dw_hdmi_update_power(struct dw_hdmi *hdmi)
>> -{
>> -	int force = hdmi->force;
>> -
>> -	if (hdmi->disabled) {
>> -		force = DRM_FORCE_OFF;
>> -	} else if (force == DRM_FORCE_UNSPECIFIED) {
>> -		if (hdmi->rxsense)
>> -			force = DRM_FORCE_ON;
>> -		else
>> -			force = DRM_FORCE_OFF;
>> -	}
> 
> This means we always poweron the bridge even if rxsense is false ?

If I follow the logic there should not be any difference, beside that
power on now only happen in atomic_enable instead of sometime in irq
handler.

hdmi->rxsense is set to true based on hpd in dw_hdmi_setup_rx_sense().
For both meson and dw-hdmi this means HPD=1 and not rxsense=1.

drm core will call the force/detect ops and enable/disable based on
forced/HPD state, so I am not expecting any difference in how this
currently works.

This change to only poweron/setup in atomic_enable should also fix
issues/situations where dw-hdmi was poweron too early during bootup in
irq handler, before the drm driver was fully probed.

Regards,
Jonas

> 
> Neil
> 
>> -
>> -	if (force == DRM_FORCE_OFF) {
>> -		if (hdmi->bridge_is_on)
>> -			dw_hdmi_poweroff(hdmi);
>> -	} else {
>> -		if (!hdmi->bridge_is_on)
>> -			dw_hdmi_poweron(hdmi);
>> -	}
>>   }
>>   
>>   /*
>> @@ -2546,7 +2519,6 @@ static void dw_hdmi_connector_force(struct drm_connector *connector)
>>   
>>   	mutex_lock(&hdmi->mutex);
>>   	hdmi->force = connector->force;
>> -	dw_hdmi_update_power(hdmi);
>>   	dw_hdmi_update_phy_mask(hdmi);
>>   	mutex_unlock(&hdmi->mutex);
>>   }
>> @@ -2955,7 +2927,7 @@ static void dw_hdmi_bridge_atomic_disable(struct drm_bridge *bridge,
>>   	mutex_lock(&hdmi->mutex);
>>   	hdmi->disabled = true;
>>   	hdmi->curr_conn = NULL;
>> -	dw_hdmi_update_power(hdmi);
>> +	dw_hdmi_poweroff(hdmi);
>>   	dw_hdmi_update_phy_mask(hdmi);
>>   	handle_plugged_change(hdmi, false);
>>   	mutex_unlock(&hdmi->mutex);
>> @@ -2974,7 +2946,7 @@ static void dw_hdmi_bridge_atomic_enable(struct drm_bridge *bridge,
>>   	mutex_lock(&hdmi->mutex);
>>   	hdmi->disabled = false;
>>   	hdmi->curr_conn = connector;
>> -	dw_hdmi_update_power(hdmi);
>> +	dw_hdmi_poweron(hdmi);
>>   	dw_hdmi_update_phy_mask(hdmi);
>>   	handle_plugged_change(hdmi, true);
>>   	mutex_unlock(&hdmi->mutex);
>> @@ -3073,7 +3045,6 @@ void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
>>   		if (hpd)
>>   			hdmi->rxsense = true;
>>   
>> -		dw_hdmi_update_power(hdmi);
>>   		dw_hdmi_update_phy_mask(hdmi);
>>   	}
>>   	mutex_unlock(&hdmi->mutex);
> 

Re: [PATCH 1/8] drm: bridge: dw_hdmi: Call poweron/poweroff from atomic enable/disable

[neil.armstrong]

On 24/06/2024 11:41, Jonas Karlman wrote:
> Hi Neil,
> 
> On 2024-06-24 11:23, Neil Armstrong wrote:
>> On 11/06/2024 17:50, Jonas Karlman wrote:
>>> Change to only call poweron/poweroff from atomic_enable/atomic_disable
>>> ops instead of trying to keep a bridge_is_on state and poweron/off in
>>> the hotplug irq handler.
>>>
>>> A benefit of this is that drm mode_config mutex is always held at
>>> poweron/off, something that may reduce the need for our own mutex.
>>>
>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>>> ---
>>>    drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 33 ++---------------------
>>>    1 file changed, 2 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>> index 9f2bc932c371..34bc6f4754b8 100644
>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>> @@ -172,7 +172,6 @@ struct dw_hdmi {
>>>    	enum drm_connector_force force;	/* mutex-protected force state */
>>>    	struct drm_connector *curr_conn;/* current connector (only valid when !disabled) */
>>>    	bool disabled;			/* DRM has disabled our bridge */
>>> -	bool bridge_is_on;		/* indicates the bridge is on */
>>>    	bool rxsense;			/* rxsense state */
>>>    	u8 phy_mask;			/* desired phy int mask settings */
>>>    	u8 mc_clkdis;			/* clock disable register */
>>> @@ -2383,8 +2382,6 @@ static void initialize_hdmi_ih_mutes(struct dw_hdmi *hdmi)
>>>    
>>>    static void dw_hdmi_poweron(struct dw_hdmi *hdmi)
>>>    {
>>> -	hdmi->bridge_is_on = true;
>>> -
>>>    	/*
>>>    	 * The curr_conn field is guaranteed to be valid here, as this function
>>>    	 * is only be called when !hdmi->disabled.
>>> @@ -2398,30 +2395,6 @@ static void dw_hdmi_poweroff(struct dw_hdmi *hdmi)
>>>    		hdmi->phy.ops->disable(hdmi, hdmi->phy.data);
>>>    		hdmi->phy.enabled = false;
>>>    	}
>>> -
>>> -	hdmi->bridge_is_on = false;
>>> -}
>>> -
>>> -static void dw_hdmi_update_power(struct dw_hdmi *hdmi)
>>> -{
>>> -	int force = hdmi->force;
>>> -
>>> -	if (hdmi->disabled) {
>>> -		force = DRM_FORCE_OFF;
>>> -	} else if (force == DRM_FORCE_UNSPECIFIED) {
>>> -		if (hdmi->rxsense)
>>> -			force = DRM_FORCE_ON;
>>> -		else
>>> -			force = DRM_FORCE_OFF;
>>> -	}
>>
>> This means we always poweron the bridge even if rxsense is false ?
> 
> If I follow the logic there should not be any difference, beside that
> power on now only happen in atomic_enable instead of sometime in irq
> handler.
> 
> hdmi->rxsense is set to true based on hpd in dw_hdmi_setup_rx_sense().
> For both meson and dw-hdmi this means HPD=1 and not rxsense=1.

Yeah I know, I was worried for other platforms using rxsense

> 
> drm core will call the force/detect ops and enable/disable based on
> forced/HPD state, so I am not expecting any difference in how this
> currently works.
> 
> This change to only poweron/setup in atomic_enable should also fix
> issues/situations where dw-hdmi was poweron too early during bootup in
> irq handler, before the drm driver was fully probed.

Hmm, but I thought the code wouldn't poweron the bridge is rxsense was 0
even if a mode was set, this won't work like this anymore right ?

Neil

> 
> Regards,
> Jonas
> 
>>
>> Neil
>>
>>> -
>>> -	if (force == DRM_FORCE_OFF) {
>>> -		if (hdmi->bridge_is_on)
>>> -			dw_hdmi_poweroff(hdmi);
>>> -	} else {
>>> -		if (!hdmi->bridge_is_on)
>>> -			dw_hdmi_poweron(hdmi);
>>> -	}
>>>    }
>>>    
>>>    /*
>>> @@ -2546,7 +2519,6 @@ static void dw_hdmi_connector_force(struct drm_connector *connector)
>>>    
>>>    	mutex_lock(&hdmi->mutex);
>>>    	hdmi->force = connector->force;
>>> -	dw_hdmi_update_power(hdmi);
>>>    	dw_hdmi_update_phy_mask(hdmi);
>>>    	mutex_unlock(&hdmi->mutex);
>>>    }
>>> @@ -2955,7 +2927,7 @@ static void dw_hdmi_bridge_atomic_disable(struct drm_bridge *bridge,
>>>    	mutex_lock(&hdmi->mutex);
>>>    	hdmi->disabled = true;
>>>    	hdmi->curr_conn = NULL;
>>> -	dw_hdmi_update_power(hdmi);
>>> +	dw_hdmi_poweroff(hdmi);
>>>    	dw_hdmi_update_phy_mask(hdmi);
>>>    	handle_plugged_change(hdmi, false);
>>>    	mutex_unlock(&hdmi->mutex);
>>> @@ -2974,7 +2946,7 @@ static void dw_hdmi_bridge_atomic_enable(struct drm_bridge *bridge,
>>>    	mutex_lock(&hdmi->mutex);
>>>    	hdmi->disabled = false;
>>>    	hdmi->curr_conn = connector;
>>> -	dw_hdmi_update_power(hdmi);
>>> +	dw_hdmi_poweron(hdmi);
>>>    	dw_hdmi_update_phy_mask(hdmi);
>>>    	handle_plugged_change(hdmi, true);
>>>    	mutex_unlock(&hdmi->mutex);
>>> @@ -3073,7 +3045,6 @@ void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
>>>    		if (hpd)
>>>    			hdmi->rxsense = true;
>>>    
>>> -		dw_hdmi_update_power(hdmi);
>>>    		dw_hdmi_update_phy_mask(hdmi);
>>>    	}
>>>    	mutex_unlock(&hdmi->mutex);
>>
> 

Re: [PATCH 1/8] drm: bridge: dw_hdmi: Call poweron/poweroff from atomic enable/disable

[Jonas Karlman]

Hi Neil,

On 2024-06-24 11:56, neil.armstrong@linaro.org wrote:
> On 24/06/2024 11:41, Jonas Karlman wrote:
>> Hi Neil,
>>
>> On 2024-06-24 11:23, Neil Armstrong wrote:
>>> On 11/06/2024 17:50, Jonas Karlman wrote:
>>>> Change to only call poweron/poweroff from atomic_enable/atomic_disable
>>>> ops instead of trying to keep a bridge_is_on state and poweron/off in
>>>> the hotplug irq handler.
>>>>
>>>> A benefit of this is that drm mode_config mutex is always held at
>>>> poweron/off, something that may reduce the need for our own mutex.
>>>>
>>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>>>> ---
>>>>    drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 33 ++---------------------
>>>>    1 file changed, 2 insertions(+), 31 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>> index 9f2bc932c371..34bc6f4754b8 100644
>>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>> @@ -172,7 +172,6 @@ struct dw_hdmi {
>>>>    	enum drm_connector_force force;	/* mutex-protected force state */
>>>>    	struct drm_connector *curr_conn;/* current connector (only valid when !disabled) */
>>>>    	bool disabled;			/* DRM has disabled our bridge */
>>>> -	bool bridge_is_on;		/* indicates the bridge is on */
>>>>    	bool rxsense;			/* rxsense state */
>>>>    	u8 phy_mask;			/* desired phy int mask settings */
>>>>    	u8 mc_clkdis;			/* clock disable register */
>>>> @@ -2383,8 +2382,6 @@ static void initialize_hdmi_ih_mutes(struct dw_hdmi *hdmi)
>>>>    
>>>>    static void dw_hdmi_poweron(struct dw_hdmi *hdmi)
>>>>    {
>>>> -	hdmi->bridge_is_on = true;
>>>> -
>>>>    	/*
>>>>    	 * The curr_conn field is guaranteed to be valid here, as this function
>>>>    	 * is only be called when !hdmi->disabled.
>>>> @@ -2398,30 +2395,6 @@ static void dw_hdmi_poweroff(struct dw_hdmi *hdmi)
>>>>    		hdmi->phy.ops->disable(hdmi, hdmi->phy.data);
>>>>    		hdmi->phy.enabled = false;
>>>>    	}
>>>> -
>>>> -	hdmi->bridge_is_on = false;
>>>> -}
>>>> -
>>>> -static void dw_hdmi_update_power(struct dw_hdmi *hdmi)
>>>> -{
>>>> -	int force = hdmi->force;
>>>> -
>>>> -	if (hdmi->disabled) {
>>>> -		force = DRM_FORCE_OFF;
>>>> -	} else if (force == DRM_FORCE_UNSPECIFIED) {
>>>> -		if (hdmi->rxsense)
>>>> -			force = DRM_FORCE_ON;
>>>> -		else
>>>> -			force = DRM_FORCE_OFF;
>>>> -	}
>>>
>>> This means we always poweron the bridge even if rxsense is false ?
>>
>> If I follow the logic there should not be any difference, beside that
>> power on now only happen in atomic_enable instead of sometime in irq
>> handler.
>>
>> hdmi->rxsense is set to true based on hpd in dw_hdmi_setup_rx_sense().
>> For both meson and dw-hdmi this means HPD=1 and not rxsense=1.
> 
> Yeah I know, I was worried for other platforms using rxsense

From what I can find only dw-hdmi.c and dw_hdmi_meson.c are caller of
dw_hdmi_setup_rx_sense(). For meson the same value is passed for both
hpd and rx_sense, everyone else hpd = HPD and rx_sense = RX_SENSE status.

My understanding and testing based on Rockchip has shown that rx_sense
has no significant impact before and after this change.

> 
>>
>> drm core will call the force/detect ops and enable/disable based on
>> forced/HPD state, so I am not expecting any difference in how this
>> currently works.
>>
>> This change to only poweron/setup in atomic_enable should also fix
>> issues/situations where dw-hdmi was poweron too early during bootup in
>> irq handler, before the drm driver was fully probed.

I may have been wrong here, there is a check for disabled here so it
should not have setup() before atomic_enable().

Still we should ensure configure happen in atomic_enable(). The
hpd_irq_event/hpd_notify calls will trigger a detect() and signal core
if the bridge should be enabled/disabled when connector status changes.

> 
> Hmm, but I thought the code wouldn't poweron the bridge is rxsense was 0
> even if a mode was set, this won't work like this anymore right ?

This is what I thought was happening, and the comment in code seem to
indicate that. However, the code only seem to care about HPD=1 status to
poweron() and possible both HPD=0 + RXSENSE=0 to poweroff().

To make matter more complex core also mainly care about detect() status
and most/all related drivers seem to only use the the HPD status.

So I would say that there should not be any changes in impact of the
rxsense signal.

Will also send a new patch in a v2 for dw_hdmi_irq() that may be related:

-		if (phy_stat & HDMI_PHY_HPD)
+		if ((intr_stat & HDMI_IH_PHY_STAT0_HPD) &&
+		    (phy_stat & HDMI_PHY_HPD))
			status = connector_status_connected;

or there may be a status_connected event triggered when rxsense goes
from 1 to 0 and a second one with status_disconnected shortly after when
hpd goes from 1 to 0 when the cable is removed.

Regards,
Jonas

> 
> Neil
> 
>>
>> Regards,
>> Jonas
>>
>>>
>>> Neil
>>>
>>>> -
>>>> -	if (force == DRM_FORCE_OFF) {
>>>> -		if (hdmi->bridge_is_on)
>>>> -			dw_hdmi_poweroff(hdmi);
>>>> -	} else {
>>>> -		if (!hdmi->bridge_is_on)
>>>> -			dw_hdmi_poweron(hdmi);
>>>> -	}
>>>>    }
>>>>    
>>>>    /*
>>>> @@ -2546,7 +2519,6 @@ static void dw_hdmi_connector_force(struct drm_connector *connector)
>>>>    
>>>>    	mutex_lock(&hdmi->mutex);
>>>>    	hdmi->force = connector->force;
>>>> -	dw_hdmi_update_power(hdmi);
>>>>    	dw_hdmi_update_phy_mask(hdmi);
>>>>    	mutex_unlock(&hdmi->mutex);
>>>>    }
>>>> @@ -2955,7 +2927,7 @@ static void dw_hdmi_bridge_atomic_disable(struct drm_bridge *bridge,
>>>>    	mutex_lock(&hdmi->mutex);
>>>>    	hdmi->disabled = true;
>>>>    	hdmi->curr_conn = NULL;
>>>> -	dw_hdmi_update_power(hdmi);
>>>> +	dw_hdmi_poweroff(hdmi);
>>>>    	dw_hdmi_update_phy_mask(hdmi);
>>>>    	handle_plugged_change(hdmi, false);
>>>>    	mutex_unlock(&hdmi->mutex);
>>>> @@ -2974,7 +2946,7 @@ static void dw_hdmi_bridge_atomic_enable(struct drm_bridge *bridge,
>>>>    	mutex_lock(&hdmi->mutex);
>>>>    	hdmi->disabled = false;
>>>>    	hdmi->curr_conn = connector;
>>>> -	dw_hdmi_update_power(hdmi);
>>>> +	dw_hdmi_poweron(hdmi);
>>>>    	dw_hdmi_update_phy_mask(hdmi);
>>>>    	handle_plugged_change(hdmi, true);
>>>>    	mutex_unlock(&hdmi->mutex);
>>>> @@ -3073,7 +3045,6 @@ void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
>>>>    		if (hpd)
>>>>    			hdmi->rxsense = true;
>>>>    
>>>> -		dw_hdmi_update_power(hdmi);
>>>>    		dw_hdmi_update_phy_mask(hdmi);
>>>>    	}
>>>>    	mutex_unlock(&hdmi->mutex);
>>>
>>
> 

Re: [PATCH 1/8] drm: bridge: dw_hdmi: Call poweron/poweroff from atomic enable/disable

[Daniel Vetter]

On Mon, Jun 24, 2024 at 02:19:36PM +0200, Jonas Karlman wrote:
> Hi Neil,
> 
> On 2024-06-24 11:56, neil.armstrong@linaro.org wrote:
> > On 24/06/2024 11:41, Jonas Karlman wrote:
> >> Hi Neil,
> >>
> >> On 2024-06-24 11:23, Neil Armstrong wrote:
> >>> On 11/06/2024 17:50, Jonas Karlman wrote:
> >>>> Change to only call poweron/poweroff from atomic_enable/atomic_disable
> >>>> ops instead of trying to keep a bridge_is_on state and poweron/off in
> >>>> the hotplug irq handler.
> >>>>
> >>>> A benefit of this is that drm mode_config mutex is always held at
> >>>> poweron/off, something that may reduce the need for our own mutex.
> >>>>
> >>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> >>>> ---
> >>>>    drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 33 ++---------------------
> >>>>    1 file changed, 2 insertions(+), 31 deletions(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >>>> index 9f2bc932c371..34bc6f4754b8 100644
> >>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >>>> @@ -172,7 +172,6 @@ struct dw_hdmi {
> >>>>    	enum drm_connector_force force;	/* mutex-protected force state */
> >>>>    	struct drm_connector *curr_conn;/* current connector (only valid when !disabled) */
> >>>>    	bool disabled;			/* DRM has disabled our bridge */
> >>>> -	bool bridge_is_on;		/* indicates the bridge is on */
> >>>>    	bool rxsense;			/* rxsense state */
> >>>>    	u8 phy_mask;			/* desired phy int mask settings */
> >>>>    	u8 mc_clkdis;			/* clock disable register */
> >>>> @@ -2383,8 +2382,6 @@ static void initialize_hdmi_ih_mutes(struct dw_hdmi *hdmi)
> >>>>    
> >>>>    static void dw_hdmi_poweron(struct dw_hdmi *hdmi)
> >>>>    {
> >>>> -	hdmi->bridge_is_on = true;
> >>>> -
> >>>>    	/*
> >>>>    	 * The curr_conn field is guaranteed to be valid here, as this function
> >>>>    	 * is only be called when !hdmi->disabled.
> >>>> @@ -2398,30 +2395,6 @@ static void dw_hdmi_poweroff(struct dw_hdmi *hdmi)
> >>>>    		hdmi->phy.ops->disable(hdmi, hdmi->phy.data);
> >>>>    		hdmi->phy.enabled = false;
> >>>>    	}
> >>>> -
> >>>> -	hdmi->bridge_is_on = false;
> >>>> -}
> >>>> -
> >>>> -static void dw_hdmi_update_power(struct dw_hdmi *hdmi)
> >>>> -{
> >>>> -	int force = hdmi->force;
> >>>> -
> >>>> -	if (hdmi->disabled) {
> >>>> -		force = DRM_FORCE_OFF;
> >>>> -	} else if (force == DRM_FORCE_UNSPECIFIED) {
> >>>> -		if (hdmi->rxsense)
> >>>> -			force = DRM_FORCE_ON;
> >>>> -		else
> >>>> -			force = DRM_FORCE_OFF;
> >>>> -	}
> >>>
> >>> This means we always poweron the bridge even if rxsense is false ?
> >>
> >> If I follow the logic there should not be any difference, beside that
> >> power on now only happen in atomic_enable instead of sometime in irq
> >> handler.
> >>
> >> hdmi->rxsense is set to true based on hpd in dw_hdmi_setup_rx_sense().
> >> For both meson and dw-hdmi this means HPD=1 and not rxsense=1.
> > 
> > Yeah I know, I was worried for other platforms using rxsense
> 
> From what I can find only dw-hdmi.c and dw_hdmi_meson.c are caller of
> dw_hdmi_setup_rx_sense(). For meson the same value is passed for both
> hpd and rx_sense, everyone else hpd = HPD and rx_sense = RX_SENSE status.
> 
> My understanding and testing based on Rockchip has shown that rx_sense
> has no significant impact before and after this change.
> 
> > 
> >>
> >> drm core will call the force/detect ops and enable/disable based on
> >> forced/HPD state, so I am not expecting any difference in how this
> >> currently works.
> >>
> >> This change to only poweron/setup in atomic_enable should also fix
> >> issues/situations where dw-hdmi was poweron too early during bootup in
> >> irq handler, before the drm driver was fully probed.
> 
> I may have been wrong here, there is a check for disabled here so it
> should not have setup() before atomic_enable().
> 
> Still we should ensure configure happen in atomic_enable(). The
> hpd_irq_event/hpd_notify calls will trigger a detect() and signal core
> if the bridge should be enabled/disabled when connector status changes.
> 
> > 
> > Hmm, but I thought the code wouldn't poweron the bridge is rxsense was 0
> > even if a mode was set, this won't work like this anymore right ?
> 
> This is what I thought was happening, and the comment in code seem to
> indicate that. However, the code only seem to care about HPD=1 status to
> poweron() and possible both HPD=0 + RXSENSE=0 to poweroff().

More fundamental question: Why do we even care what happens in this case?
Userspace lit up an output that's not connected either

- it's broken, it gets to keep to pieces

- it knows what it's doing, there is a display, it's just broken, and it
  _really_ wants the output to be lit up.

Drivers trying to be clever isn't a good idea here imo ...
-Sima

> To make matter more complex core also mainly care about detect() status
> and most/all related drivers seem to only use the the HPD status.
> 
> So I would say that there should not be any changes in impact of the
> rxsense signal.
> 
> Will also send a new patch in a v2 for dw_hdmi_irq() that may be related:
> 
> -		if (phy_stat & HDMI_PHY_HPD)
> +		if ((intr_stat & HDMI_IH_PHY_STAT0_HPD) &&
> +		    (phy_stat & HDMI_PHY_HPD))
> 			status = connector_status_connected;
> 
> or there may be a status_connected event triggered when rxsense goes
> from 1 to 0 and a second one with status_disconnected shortly after when
> hpd goes from 1 to 0 when the cable is removed.
> 
> Regards,
> Jonas
> 
> > 
> > Neil
> > 
> >>
> >> Regards,
> >> Jonas
> >>
> >>>
> >>> Neil
> >>>
> >>>> -
> >>>> -	if (force == DRM_FORCE_OFF) {
> >>>> -		if (hdmi->bridge_is_on)
> >>>> -			dw_hdmi_poweroff(hdmi);
> >>>> -	} else {
> >>>> -		if (!hdmi->bridge_is_on)
> >>>> -			dw_hdmi_poweron(hdmi);
> >>>> -	}
> >>>>    }
> >>>>    
> >>>>    /*
> >>>> @@ -2546,7 +2519,6 @@ static void dw_hdmi_connector_force(struct drm_connector *connector)
> >>>>    
> >>>>    	mutex_lock(&hdmi->mutex);
> >>>>    	hdmi->force = connector->force;
> >>>> -	dw_hdmi_update_power(hdmi);
> >>>>    	dw_hdmi_update_phy_mask(hdmi);
> >>>>    	mutex_unlock(&hdmi->mutex);
> >>>>    }
> >>>> @@ -2955,7 +2927,7 @@ static void dw_hdmi_bridge_atomic_disable(struct drm_bridge *bridge,
> >>>>    	mutex_lock(&hdmi->mutex);
> >>>>    	hdmi->disabled = true;
> >>>>    	hdmi->curr_conn = NULL;
> >>>> -	dw_hdmi_update_power(hdmi);
> >>>> +	dw_hdmi_poweroff(hdmi);
> >>>>    	dw_hdmi_update_phy_mask(hdmi);
> >>>>    	handle_plugged_change(hdmi, false);
> >>>>    	mutex_unlock(&hdmi->mutex);
> >>>> @@ -2974,7 +2946,7 @@ static void dw_hdmi_bridge_atomic_enable(struct drm_bridge *bridge,
> >>>>    	mutex_lock(&hdmi->mutex);
> >>>>    	hdmi->disabled = false;
> >>>>    	hdmi->curr_conn = connector;
> >>>> -	dw_hdmi_update_power(hdmi);
> >>>> +	dw_hdmi_poweron(hdmi);
> >>>>    	dw_hdmi_update_phy_mask(hdmi);
> >>>>    	handle_plugged_change(hdmi, true);
> >>>>    	mutex_unlock(&hdmi->mutex);
> >>>> @@ -3073,7 +3045,6 @@ void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
> >>>>    		if (hpd)
> >>>>    			hdmi->rxsense = true;
> >>>>    
> >>>> -		dw_hdmi_update_power(hdmi);
> >>>>    		dw_hdmi_update_phy_mask(hdmi);
> >>>>    	}
> >>>>    	mutex_unlock(&hdmi->mutex);
> >>>
> >>
> > 
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch