[PATCH 0/2] Recover DP output after a cable replug

[PATCH 0/2] Recover DP output after a cable replug

[Yashas D]

Applications that don't run a hotplug event loop (e.g. kmstest, or any
client that just holds a CRTC open without listening for uevents) never
see a display come back after a DP cable is unplugged and replugged.
The bridge's own link state goes down on removal, but nothing tells it
to retrain when the cable comes back unless something above it issues
a fresh atomic commit -- which these applications never do.

Patch 1 (improve HPD interrupt handling) fixes the interrupt handler
to clear all three IRQ status registers instead of just one, so the
IRQ pin is fully de-asserted and doesn't stay stuck; adds detection of
the bridge's separate replug event (a short unplug/replug that doesn't
always cross the debounce threshold used for insertion/removal); and
switches from polling every connector on the device to notifying only
the DP connector that actually had the event.

Patch 2 (retrain DP link directly on cable replug) adds a work handler
that retrains the DP link and re-enables the video stream directly
from the HPD interrupt path when the pipeline is still active, without
waiting for a new atomic commit. This is the piece that actually fixes
the recovery problem described above -- applications with no hotplug
handling get their display back without doing anything.

Yashas D (2):
  drm/bridge: ti-sn65dsi86: improve HPD interrupt handling
  drm/bridge: ti-sn65dsi86: retrain DP link directly on cable replug

 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 227 ++++++++++++++++++++++----
 1 file changed, 193 insertions(+), 34 deletions(-)

-- 
2.34.1

[PATCH 1/2] drm/bridge: ti-sn65dsi86: improve HPD interrupt handling

[Yashas D]

Fix the interrupt handler to clear all three IRQ status registers
to fully de-assert the IRQ pin, enable replug event detection, and
use per-connector hotplug notification instead of polling all
connectors on every DP HPD event.

Signed-off-by: Yashas D <y-d@ti.com>
---
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 34 ++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 48b83df9aed6..d9bd4ef8f0e2 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -113,14 +113,19 @@
 #define SN_IRQ_EVENTS_EN_REG			0xE6
 #define  HPD_INSERTION_EN			BIT(1)
 #define  HPD_REMOVAL_EN				BIT(2)
+#define  HPD_REPLUG_EN				BIT(3)
 
 #define SN_AUX_CMD_STATUS_REG			0xF4
 #define  AUX_IRQ_STATUS_AUX_RPLY_TOUT		BIT(3)
 #define  AUX_IRQ_STATUS_AUX_SHORT		BIT(5)
 #define  AUX_IRQ_STATUS_NAT_I2C_FAIL		BIT(6)
 #define SN_IRQ_STATUS_REG			0xF5
+#define  HPD_REPLUG_STATUS			BIT(3)
 #define  HPD_REMOVAL_STATUS			BIT(2)
 #define  HPD_INSERTION_STATUS			BIT(1)
+/* General IRQ status registers, write-1-to-clear */
+#define SN_IRQ_STATUS2_REG			0xF0
+#define SN_IRQ_STATUS3_REG			0xF2
 
 #define MIN_DSI_CLK_FREQ_MHZ	40
 
@@ -1266,7 +1271,7 @@ static void ti_sn_bridge_hpd_enable(struct drm_bridge *bridge)
 
 	if (client->irq) {
 		ret = regmap_set_bits(pdata->regmap, SN_IRQ_EVENTS_EN_REG,
-				      HPD_REMOVAL_EN | HPD_INSERTION_EN);
+				      HPD_REMOVAL_EN | HPD_INSERTION_EN | HPD_REPLUG_EN);
 		if (ret)
 			dev_err(pdata->dev, "Failed to enable HPD events: %d\n", ret);
 	}
@@ -1280,7 +1285,7 @@ static void ti_sn_bridge_hpd_disable(struct drm_bridge *bridge)
 
 	if (client->irq) {
 		ret = regmap_clear_bits(pdata->regmap, SN_IRQ_EVENTS_EN_REG,
-					HPD_REMOVAL_EN | HPD_INSERTION_EN);
+					HPD_REMOVAL_EN | HPD_INSERTION_EN | HPD_REPLUG_EN);
 		if (ret)
 			dev_err(pdata->dev, "Failed to disable HPD events: %d\n", ret);
 	}
@@ -1376,7 +1381,6 @@ static int ti_sn_bridge_parse_dsi_host(struct ti_sn65dsi86 *pdata)
 static irqreturn_t ti_sn_bridge_interrupt(int irq, void *private)
 {
 	struct ti_sn65dsi86 *pdata = private;
-	struct drm_device *dev = pdata->bridge.dev;
 	u8 status;
 	int ret;
 	bool hpd_event;
@@ -1387,23 +1391,35 @@ static irqreturn_t ti_sn_bridge_interrupt(int irq, void *private)
 		return IRQ_NONE;
 	}
 
-	hpd_event = status & (HPD_REMOVAL_STATUS | HPD_INSERTION_STATUS);
+	hpd_event = status & (HPD_REMOVAL_STATUS | HPD_INSERTION_STATUS |
+			      HPD_REPLUG_STATUS);
 
 	dev_dbg(pdata->dev, "(SN_IRQ_STATUS_REG = %#x)\n", status);
 	if (!status)
 		return IRQ_NONE;
 
-	ret = regmap_write(pdata->regmap, SN_IRQ_STATUS_REG, status);
+	/*
+	 * Clear all three IRQ status registers to fully de-assert
+	 * the IRQ pin
+	 */
+	ret  = regmap_write(pdata->regmap, SN_IRQ_STATUS2_REG, 0xFF);
+	ret |= regmap_write(pdata->regmap, SN_IRQ_STATUS3_REG, 0xFF);
+	ret |= regmap_write(pdata->regmap, SN_IRQ_STATUS_REG, status);
 	if (ret) {
 		dev_err(pdata->dev, "Failed to clear IRQ status: %d\n", ret);
 		return IRQ_NONE;
 	}
 
-	/* Only send the HPD event if we are bound with a device. */
+	/* Notify only the DP connector, not all connectors on the device. */
 	mutex_lock(&pdata->hpd_mutex);
-	if (pdata->hpd_enabled && hpd_event)
-		drm_kms_helper_hotplug_event(dev);
-	mutex_unlock(&pdata->hpd_mutex);
+	if (pdata->hpd_enabled && hpd_event && pdata->bridge.hpd_data) {
+		struct drm_connector *connector =
+			(struct drm_connector *)pdata->bridge.hpd_data;
+		mutex_unlock(&pdata->hpd_mutex);
+		drm_connector_helper_hpd_irq_event(connector);
+	} else {
+		mutex_unlock(&pdata->hpd_mutex);
+	}
 
 	return IRQ_HANDLED;
 }
-- 
2.34.1

Re: [PATCH 1/2] drm/bridge: ti-sn65dsi86: improve HPD interrupt handling

[Doug Anderson]

Hi,

On Fri, Aug 14, 2026 at 5:03 AM Yashas D <y-d@ti.com> wrote:
>
> Fix the interrupt handler to clear all three IRQ status registers
> to fully de-assert the IRQ pin, enable replug event detection, and
> use per-connector hotplug notification instead of polling all
> connectors on every DP HPD event.
>
> Signed-off-by: Yashas D <y-d@ti.com>
> ---
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 34 ++++++++++++++++++++-------
>  1 file changed, 25 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index 48b83df9aed6..d9bd4ef8f0e2 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -113,14 +113,19 @@
>  #define SN_IRQ_EVENTS_EN_REG                   0xE6
>  #define  HPD_INSERTION_EN                      BIT(1)
>  #define  HPD_REMOVAL_EN                                BIT(2)
> +#define  HPD_REPLUG_EN                         BIT(3)
>
>  #define SN_AUX_CMD_STATUS_REG                  0xF4
>  #define  AUX_IRQ_STATUS_AUX_RPLY_TOUT          BIT(3)
>  #define  AUX_IRQ_STATUS_AUX_SHORT              BIT(5)
>  #define  AUX_IRQ_STATUS_NAT_I2C_FAIL           BIT(6)
>  #define SN_IRQ_STATUS_REG                      0xF5
> +#define  HPD_REPLUG_STATUS                     BIT(3)
>  #define  HPD_REMOVAL_STATUS                    BIT(2)
>  #define  HPD_INSERTION_STATUS                  BIT(1)
> +/* General IRQ status registers, write-1-to-clear */
> +#define SN_IRQ_STATUS2_REG                     0xF0
> +#define SN_IRQ_STATUS3_REG                     0xF2

nit: blank line before this section?


> @@ -1387,23 +1391,35 @@ static irqreturn_t ti_sn_bridge_interrupt(int irq, void *private)
>                 return IRQ_NONE;
>         }
>
> -       hpd_event = status & (HPD_REMOVAL_STATUS | HPD_INSERTION_STATUS);
> +       hpd_event = status & (HPD_REMOVAL_STATUS | HPD_INSERTION_STATUS |
> +                             HPD_REPLUG_STATUS);
>
>         dev_dbg(pdata->dev, "(SN_IRQ_STATUS_REG = %#x)\n", status);
>         if (!status)
>                 return IRQ_NONE;
>
> -       ret = regmap_write(pdata->regmap, SN_IRQ_STATUS_REG, status);
> +       /*
> +        * Clear all three IRQ status registers to fully de-assert
> +        * the IRQ pin
> +        */
> +       ret  = regmap_write(pdata->regmap, SN_IRQ_STATUS2_REG, 0xFF);
> +       ret |= regmap_write(pdata->regmap, SN_IRQ_STATUS3_REG, 0xFF);
> +       ret |= regmap_write(pdata->regmap, SN_IRQ_STATUS_REG, status);

Can you explain clearly why you need to clear STATUS2 and STATUS3? Is
this actually a thing you saw causing problems?

As far as I understand, all these extra interrupts should be masked
anyway. That means they shouldn't be affecting the IRQ line and they
shouldn't need to be cleared, right? Taking one example bit
(CHA_SOT_BIT_ERR), we can see that in register 0xE1 they default state
is to mask it. That means there should be no need to clear it in
register 0xF2, right?

If you want to be defensive, you could explictly zero out some of the
0xE1 - 0xE5 at probe time just in some bootloader made things enabled.
...but I don't see any reason to be clearing the status registers at
IRQ time unless there's some interrupt that we can't mask.



>         if (ret) {
>                 dev_err(pdata->dev, "Failed to clear IRQ status: %d\n", ret);
>                 return IRQ_NONE;
>         }
>
> -       /* Only send the HPD event if we are bound with a device. */
> +       /* Notify only the DP connector, not all connectors on the device. */
>         mutex_lock(&pdata->hpd_mutex);
> -       if (pdata->hpd_enabled && hpd_event)
> -               drm_kms_helper_hotplug_event(dev);
> -       mutex_unlock(&pdata->hpd_mutex);
> +       if (pdata->hpd_enabled && hpd_event && pdata->bridge.hpd_data) {
> +               struct drm_connector *connector =
> +                       (struct drm_connector *)pdata->bridge.hpd_data;
> +               mutex_unlock(&pdata->hpd_mutex);
> +               drm_connector_helper_hpd_irq_event(connector);
> +       } else {
> +               mutex_unlock(&pdata->hpd_mutex);
> +       }

Do you really need the above change? I haven't studied the DRM
framework all that much, but...

1. It feels like an abstraction violation to reach into
"bridge.hpd_data" and assume it's the connector. I guess Sashiko
agress with me...

2. The documentation for the old function were were calling
(drm_kms_helper_hotplug_event) says that, if you know the connector,
you should call drm_kms_helper_connector_hotplug_event(). You're not
doing that and are instead calling
drm_connector_helper_hpd_irq_event().

Unless this is actually fixing problems, I'm inclined not to take this
patch. If you _really_ feel like it's an improvement, maybe someone
else can comment on it or I can schedule some time in the future to
dig more.


Maybe split the "replug" part of this patch into its own patch, if
that's actually something that you've confirmed is useful / does
something?


-Doug

[PATCH 2/2] drm/bridge: ti-sn65dsi86: retrain DP link directly on cable replug

[Yashas D]

When a cable is replugged while the upstream display pipeline is still
active (e.g. a compositor holds the CRTC), the bridge can retrain the
DP link and re-enable the video stream directly from the HPD interrupt
work handler without requiring a full DRM atomic commit. This allows
applications to recover display output after a cable replug.

Signed-off-by: Yashas D <y-d@ti.com>
---
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 215 +++++++++++++++++++++-----
 1 file changed, 179 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index d9bd4ef8f0e2..f6f930ca1519 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -212,6 +212,24 @@ struct ti_sn65dsi86 {
 	struct mutex			comms_mutex;
 	struct mutex			hpd_mutex;
 
+	/*
+	 * bridge_enabled, cached_bpp and cached_mode are written by
+	 * atomic_enable()/atomic_disable() and read by hpd_work(); all
+	 * three are only ever accessed while holding hpd_mutex.
+	 *
+	 * Set true by atomic_enable(), false by atomic_disable().  When the
+	 * cable is replugged while true, hpd_work can retrain the link
+	 * directly without a DRM atomic commit.
+	 */
+	bool				bridge_enabled;
+	unsigned int			cached_bpp;
+	/*
+	 * Copy of the last adjusted mode programmed by atomic_enable().
+	 */
+	struct drm_display_mode		cached_mode;
+	struct drm_display_mode		hpd_mode;
+	struct work_struct		hpd_work;
+
 #if defined(CONFIG_OF_GPIO)
 	struct gpio_chip		gchip;
 	DECLARE_BITMAP(gchip_output, SN_NUM_GPIOS);
@@ -285,13 +303,32 @@ static struct drm_display_mode *
 get_new_adjusted_display_mode(struct drm_bridge *bridge,
 			      struct drm_atomic_commit *state)
 {
-	struct drm_connector *connector =
+	struct ti_sn65dsi86 *pdata = container_of(bridge, struct ti_sn65dsi86,
+						  bridge);
+	struct drm_connector *connector;
+	struct drm_connector_state *conn_state;
+	struct drm_crtc_state *crtc_state;
+
+	/*
+	 * hpd_work calls this with state == NULL since it runs outside any
+	 * DRM commit and holds no modeset lock.  It has already taken its
+	 * own private snapshot (hpd_mode) under hpd_mutex at the start of
+	 * its run, so just return that instead of touching live CRTC state
+	 */
+	if (!state)
+		return &pdata->hpd_mode;
+
+	connector =
 		drm_atomic_get_new_connector_for_encoder(state, bridge->encoder);
-	struct drm_connector_state *conn_state =
+	conn_state =
 		drm_atomic_get_new_connector_state(state, connector);
-	struct drm_crtc_state *crtc_state =
+	crtc_state =
 		drm_atomic_get_new_crtc_state(state, conn_state->crtc);
 
+	mutex_lock(&pdata->hpd_mutex);
+	drm_mode_copy(&pdata->cached_mode, &crtc_state->adjusted_mode);
+	mutex_unlock(&pdata->hpd_mutex);
+
 	return &crtc_state->adjusted_mode;
 }
 
@@ -833,8 +870,16 @@ static void ti_sn_bridge_atomic_disable(struct drm_bridge *bridge,
 {
 	struct ti_sn65dsi86 *pdata = bridge_to_ti_sn65dsi86(bridge);
 
-	/* disable video stream */
+	/*
+	 * Clear bridge_enabled and disable the video stream under hpd_mutex.
+	 * hpd_work takes the same lock before its own VSTREAM_ENABLE write
+	 * and rechecks bridge_enabled at that point, so whichever of the two
+	 * runs last under the lock decides the final hardware state.
+	 */
+	mutex_lock(&pdata->hpd_mutex);
+	pdata->bridge_enabled = false;
 	regmap_update_bits(pdata->regmap, SN_ENH_FRAME_REG, VSTREAM_ENABLE, 0);
+	mutex_unlock(&pdata->hpd_mutex);
 }
 
 static void ti_sn_bridge_set_dsi_rate(struct ti_sn65dsi86 *pdata,
@@ -1092,34 +1137,27 @@ static int ti_sn_link_training(struct ti_sn65dsi86 *pdata, int dp_rate_idx,
 	return ret;
 }
 
-static void ti_sn_bridge_atomic_enable(struct drm_bridge *bridge,
-				       struct drm_atomic_commit *state)
+/*
+ * ti_sn_bridge_link_train - configure lanes, scrambler, data format and
+ *                           run DP link training.
+ *
+ * Shared by atomic_enable() (state from DRM commit) and hpd_work()
+ * (state == NULL, falls back to current CRTC state).
+ */
+static int ti_sn_bridge_link_train(struct ti_sn65dsi86 *pdata,
+				   unsigned int bpp,
+				   struct drm_atomic_commit *state)
 {
-	struct ti_sn65dsi86 *pdata = bridge_to_ti_sn65dsi86(bridge);
-	struct drm_connector *connector;
 	const char *last_err_str = "No supported DP rate";
 	unsigned int valid_rates;
 	int dp_rate_idx;
 	unsigned int val;
 	int ret = -EINVAL;
-	int max_dp_lanes;
-	unsigned int bpp;
-
-	connector = drm_atomic_get_new_connector_for_encoder(state,
-							     bridge->encoder);
-	if (!connector) {
-		dev_err_ratelimited(pdata->dev, "Could not get the connector\n");
-		return;
-	}
-
-	max_dp_lanes = ti_sn_get_max_lanes(pdata);
-	pdata->dp_lanes = min(pdata->dp_lanes, max_dp_lanes);
 
 	/* DSI_A lane config */
 	val = CHA_DSI_LANES(SN_MAX_DP_LANES - pdata->dsi->lanes);
 	regmap_update_bits(pdata->regmap, SN_DSI_LANES_REG,
 			   CHA_DSI_LANES_MASK, val);
-
 	regmap_write(pdata->regmap, SN_LN_ASSIGN_REG, pdata->ln_assign);
 	regmap_update_bits(pdata->regmap, SN_ENH_FRAME_REG, LN_POLRS_MASK,
 			   pdata->ln_polrs << LN_POLRS_OFFSET);
@@ -1139,7 +1177,6 @@ static void ti_sn_bridge_atomic_enable(struct drm_bridge *bridge,
 	if (pdata->bridge.type == DRM_MODE_CONNECTOR_eDP) {
 		drm_dp_dpcd_writeb(&pdata->aux, DP_EDP_CONFIGURATION_SET,
 				   DP_ALTERNATE_SCRAMBLER_RESET_ENABLE);
-
 		regmap_update_bits(pdata->regmap, SN_TRAINING_SETTING_REG,
 				   SCRAMBLE_DISABLE, 0);
 	} else {
@@ -1147,7 +1184,6 @@ static void ti_sn_bridge_atomic_enable(struct drm_bridge *bridge,
 				   SCRAMBLE_DISABLE, SCRAMBLE_DISABLE);
 	}
 
-	bpp = ti_sn_bridge_get_bpp(connector);
 	/* Set the DP output format (18 bpp or 24 bpp) */
 	val = bpp == 18 ? BPP_18_RGB : 0;
 	regmap_update_bits(pdata->regmap, SN_DATA_FORMAT_REG, BPP_18_RGB, val);
@@ -1159,28 +1195,130 @@ static void ti_sn_bridge_atomic_enable(struct drm_bridge *bridge,
 
 	valid_rates = ti_sn_bridge_read_valid_rates(pdata);
 
-	/* Train until we run out of rates */
 	for (dp_rate_idx = ti_sn_bridge_calc_min_dp_rate_idx(pdata, state, bpp);
 	     dp_rate_idx < ARRAY_SIZE(ti_sn_bridge_dp_rate_lut);
 	     dp_rate_idx++) {
 		if (!(valid_rates & BIT(dp_rate_idx)))
 			continue;
-
 		ret = ti_sn_link_training(pdata, dp_rate_idx, &last_err_str);
 		if (!ret)
 			break;
 	}
-	if (ret) {
-		DRM_DEV_ERROR(pdata->dev, "%s (%d)\n", last_err_str, ret);
+
+	if (ret)
+		DRM_DEV_ERROR(pdata->dev, "link training failed: %s\n",
+			      last_err_str);
+
+	return ret;
+}
+
+/*
+ * ti_sn_bridge_hpd_work - retrain the DP link on cable replug
+ *
+ * If bridge_enabled is true the upstream pipeline is still active so
+ * the link can be retrained directly without a DRM atomic commit,
+ * allowing applications to recover after a cable replug.
+ */
+static void ti_sn_bridge_hpd_work(struct work_struct *work)
+{
+	struct ti_sn65dsi86 *pdata =
+		container_of(work, struct ti_sn65dsi86, hpd_work);
+	struct drm_connector *connector;
+	unsigned int hpd_status;
+	int max_dp_lanes;
+	unsigned int bpp;
+	bool enabled;
+	int ret;
+
+	pm_runtime_get_sync(pdata->dev);
+
+	ret = regmap_read(pdata->regmap, SN_HPD_DISABLE_REG, &hpd_status);
+	if (ret || !(hpd_status & HPD_DEBOUNCED_STATE))
+		goto notify;
+
+	/*
+	 * Snapshot what atomic_enable() published under hpd_mutex.
+	 * hpd_mode is only ever written/read by hpd_work, which never runs
+	 * concurrently with itself, so it is safe to use lock-free for the
+	 * rest of this function.
+	 */
+	mutex_lock(&pdata->hpd_mutex);
+	enabled = pdata->bridge_enabled;
+	bpp = pdata->cached_bpp;
+	drm_mode_copy(&pdata->hpd_mode, &pdata->cached_mode);
+	mutex_unlock(&pdata->hpd_mutex);
+
+	if (!enabled)
+		goto notify;
+
+	max_dp_lanes = ti_sn_get_max_lanes(pdata);
+	mutex_lock(&pdata->hpd_mutex);
+	pdata->dp_lanes = min(pdata->dp_lanes, max_dp_lanes);
+	mutex_unlock(&pdata->hpd_mutex);
+
+	ret = ti_sn_bridge_link_train(pdata, bpp, NULL);
+	if (ret)
+		goto notify;
+
+	ti_sn_bridge_set_video_timings(pdata, NULL);
+	mutex_lock(&pdata->hpd_mutex);
+	if (pdata->bridge_enabled)
+		regmap_update_bits(pdata->regmap, SN_ENH_FRAME_REG,
+				   VSTREAM_ENABLE, VSTREAM_ENABLE);
+	mutex_unlock(&pdata->hpd_mutex);
+
+notify:
+	pm_runtime_put_autosuspend(pdata->dev);
+
+	if (pdata->bridge.hpd_data) {
+		connector = (struct drm_connector *)pdata->bridge.hpd_data;
+		drm_connector_helper_hpd_irq_event(connector);
+	}
+}
+
+static void ti_sn_bridge_atomic_enable(struct drm_bridge *bridge,
+				       struct drm_atomic_commit *state)
+{
+	struct ti_sn65dsi86 *pdata = bridge_to_ti_sn65dsi86(bridge);
+	struct drm_connector *connector;
+	int max_dp_lanes;
+	unsigned int bpp;
+	int ret;
+
+	connector = drm_atomic_get_new_connector_for_encoder(state,
+							     bridge->encoder);
+	if (!connector) {
+		dev_err_ratelimited(pdata->dev, "Could not get the connector\n");
 		return;
 	}
 
+	max_dp_lanes = ti_sn_get_max_lanes(pdata);
+	mutex_lock(&pdata->hpd_mutex);
+	pdata->dp_lanes = min(pdata->dp_lanes, max_dp_lanes);
+	mutex_unlock(&pdata->hpd_mutex);
+	bpp = ti_sn_bridge_get_bpp(connector);
+
+	ret = ti_sn_bridge_link_train(pdata, bpp, state);
+	if (ret)
+		return;
+
 	/* config video parameters */
 	ti_sn_bridge_set_video_timings(pdata, state);
 
 	/* enable video stream */
 	regmap_update_bits(pdata->regmap, SN_ENH_FRAME_REG, VSTREAM_ENABLE,
 			   VSTREAM_ENABLE);
+
+	/*
+	 * Publish cached_bpp and bridge_enabled under hpd_mutex.  hpd_work
+	 * reads both under the same lock, which also makes every write this
+	 * function made above (dp_lanes, cached_mode via
+	 * get_new_adjusted_display_mode()) visible to it.
+	 */
+	mutex_lock(&pdata->hpd_mutex);
+	pdata->cached_bpp = bpp;
+	pdata->bridge_enabled = true;
+	mutex_unlock(&pdata->hpd_mutex);
 }
 
 static void ti_sn_bridge_atomic_pre_enable(struct drm_bridge *bridge,
@@ -1270,6 +1408,14 @@ static void ti_sn_bridge_hpd_enable(struct drm_bridge *bridge)
 	mutex_unlock(&pdata->hpd_mutex);
 
 	if (client->irq) {
+		/*
+		 * Clear stale status on all three IRQ registers before
+		 * enabling, to avoid a spurious event.
+		 */
+		regmap_write(pdata->regmap, SN_IRQ_STATUS_REG, 0xFF);
+		regmap_write(pdata->regmap, SN_IRQ_STATUS2_REG, 0xFF);
+		regmap_write(pdata->regmap, SN_IRQ_STATUS3_REG, 0xFF);
+
 		ret = regmap_set_bits(pdata->regmap, SN_IRQ_EVENTS_EN_REG,
 				      HPD_REMOVAL_EN | HPD_INSERTION_EN | HPD_REPLUG_EN);
 		if (ret)
@@ -1294,6 +1440,8 @@ static void ti_sn_bridge_hpd_disable(struct drm_bridge *bridge)
 	pdata->hpd_enabled = false;
 	mutex_unlock(&pdata->hpd_mutex);
 
+	cancel_work_sync(&pdata->hpd_work);
+
 	pm_runtime_put_autosuspend(pdata->dev);
 }
 
@@ -1410,16 +1558,10 @@ static irqreturn_t ti_sn_bridge_interrupt(int irq, void *private)
 		return IRQ_NONE;
 	}
 
-	/* Notify only the DP connector, not all connectors on the device. */
 	mutex_lock(&pdata->hpd_mutex);
-	if (pdata->hpd_enabled && hpd_event && pdata->bridge.hpd_data) {
-		struct drm_connector *connector =
-			(struct drm_connector *)pdata->bridge.hpd_data;
-		mutex_unlock(&pdata->hpd_mutex);
-		drm_connector_helper_hpd_irq_event(connector);
-	} else {
-		mutex_unlock(&pdata->hpd_mutex);
-	}
+	if (pdata->hpd_enabled && hpd_event)
+		schedule_work(&pdata->hpd_work);
+	mutex_unlock(&pdata->hpd_mutex);
 
 	return IRQ_HANDLED;
 }
@@ -2050,6 +2192,7 @@ static int ti_sn65dsi86_probe(struct i2c_client *client)
 
 	mutex_init(&pdata->hpd_mutex);
 	mutex_init(&pdata->comms_mutex);
+	INIT_WORK(&pdata->hpd_work, ti_sn_bridge_hpd_work);
 
 	pdata->regmap = devm_regmap_init_i2c(client,
 					     &ti_sn65dsi86_regmap_config);
-- 
2.34.1

Re: [PATCH 2/2] drm/bridge: ti-sn65dsi86: retrain DP link directly on cable replug

[Doug Anderson]

Hi,

On Fri, Aug 14, 2026 at 5:04 AM Yashas D <y-d@ti.com> wrote:
>
> When a cable is replugged while the upstream display pipeline is still
> active (e.g. a compositor holds the CRTC), the bridge can retrain the
> DP link and re-enable the video stream directly from the HPD interrupt
> work handler without requiring a full DRM atomic commit. This allows
> applications to recover display output after a cable replug.
>
> Signed-off-by: Yashas D <y-d@ti.com>
> ---
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 215 +++++++++++++++++++++-----
>  1 file changed, 179 insertions(+), 36 deletions(-)

There's a lot of code here and Sashiko has a bunch of feedback [1].
I'm not personally very familiar with the HPD flows. I'll probably
skip reviewing this until Sashiko is reasonably happy or until you
explain why Sashiko is confused.

FWIW, just from the patch description, I'd also question if this is
really a good idea. If we get an unplug / replug, we could get an
entirely new display plugged in. This is really something that's OK
without needing to handle things?


[1] https://lore.kernel.org/all/20260814121431.72C701F000E9@smtp.kernel.org/

Re: [PATCH 2/2] drm/bridge: ti-sn65dsi86: retrain DP link directly on cable replug

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1833 bytes --]

On Fri, Aug 14, 2026 at 10:14:42AM -0700, Doug Anderson wrote:
> Hi,
> 
> On Fri, Aug 14, 2026 at 5:04 AM Yashas D <y-d@ti.com> wrote:
> >
> > When a cable is replugged while the upstream display pipeline is still
> > active (e.g. a compositor holds the CRTC), the bridge can retrain the
> > DP link and re-enable the video stream directly from the HPD interrupt
> > work handler without requiring a full DRM atomic commit. This allows
> > applications to recover display output after a cable replug.
> >
> > Signed-off-by: Yashas D <y-d@ti.com>
> > ---
> >  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 215 +++++++++++++++++++++-----
> >  1 file changed, 179 insertions(+), 36 deletions(-)
> 
> There's a lot of code here and Sashiko has a bunch of feedback [1].
> I'm not personally very familiar with the HPD flows. I'll probably
> skip reviewing this until Sashiko is reasonably happy or until you
> explain why Sashiko is confused.
> 
> FWIW, just from the patch description, I'd also question if this is
> really a good idea. If we get an unplug / replug, we could get an
> entirely new display plugged in. This is really something that's OK
> without needing to handle things?

The expectation is that the output should remain the same unless
userspace decides on a modeset. So, we should send the hotplug event,
and reenable the output as is.

I'm not sure if there's only link-training in DP's case, but for HDMI we
have a similar issue with the scrambling setup that gets reset by the
unplug/replug cycle.

The easier thing to do in that case is to create a new empty commit and
disable/enable the whole display chain starting with the CRTC, since it
doesn't create a new path to test. We have
drm_atomic_helper_reset_crtc() and drm_bridge_helper_reset_crtc() to do
just that.

Maxime

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