[PATCH 0/2] drm/bridge: ti-sn65dsi83: Various fixes

[PATCH 0/2] drm/bridge: ti-sn65dsi83: Various fixes

[Esben Haabendal]

This small series adds support for using SN65DSI84 in single-link mode with
output to LVDS channel B, and provides a fix for a PLL locking issue.

Signed-off-by: Esben Haabendal <esben@geanix.com>

---
Esben Haabendal (2):
      drm/bridge: ti-sn65dsi83: Support LVDS Channel B on SN65DSI84
      drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking

 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 66 +++++++++++++++++++++--------------
 1 file changed, 39 insertions(+), 27 deletions(-)
---
base-commit: 8cdeaa50eae8dad34885515f62559ee83e7e8dda
change-id: 20260710-ti-sn65dsi83-fixes-1d08e0ac67b3

Best regards,
--  
Esben Haabendal <esben@geanix.com>

[PATCH 1/2] drm/bridge: ti-sn65dsi83: Support LVDS Channel B on SN65DSI84

[Esben Haabendal]

This adds support for using SN65DSI84 in single-link mode with output to
LVDS Channel B.

Signed-off-by: Esben Haabendal <esben@geanix.com>
---
 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 36 ++++++++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index 42b451432bbb..32e9eadd7452 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -11,6 +11,8 @@
  *   = 1x Single-link DSI ~ 2x Single-link or 1x Dual-link LVDS
  *   - Supported
  *   - Dual-link LVDS mode tested
+ *   - Single-link to LVDS Channel A tested.
+ *   - Single-link to LVDS Channel B tested.
  *   - 2x Single-link LVDS mode unsupported
  *     (should be easy to add by someone who has the HW)
  * - SN65DSI85
@@ -162,7 +164,7 @@ struct sn65dsi83 {
 	struct gpio_desc		*enable_gpio;
 	struct regulator		*vcc;
 	bool				lvds_dual_link;
-	bool				lvds_dual_link_even_odd_swap;
+	bool				lvds_channel_swap;
 	int				lvds_vod_swing_conf[2];
 	int				lvds_term_conf[2];
 	int				irq;
@@ -642,7 +644,7 @@ static void sn65dsi83_atomic_pre_enable(struct drm_bridge *bridge,
 			REG_LVDS_VCOM_CHA_LVDS_VOD_SWING(ctx->lvds_vod_swing_conf[CHANNEL_A]) |
 			REG_LVDS_VCOM_CHB_LVDS_VOD_SWING(ctx->lvds_vod_swing_conf[CHANNEL_B]));
 	regmap_write(ctx->regmap, REG_LVDS_LANE,
-		     (ctx->lvds_dual_link_even_odd_swap ?
+		     (ctx->lvds_channel_swap ?
 		      REG_LVDS_LANE_EVEN_ODD_SWAP : 0) |
 		     (ctx->lvds_term_conf[CHANNEL_A] ?
 			  REG_LVDS_LANE_CHA_LVDS_TERM : 0) |
@@ -893,6 +895,7 @@ static int sn65dsi83_parse_dt(struct sn65dsi83 *ctx, enum sn65dsi83_model model)
 {
 	struct drm_bridge *panel_bridge;
 	struct device *dev = ctx->dev;
+	u32 panel_port = 2;
 	int ret;
 
 	ret = sn65dsi83_parse_lvds_endpoint(ctx, CHANNEL_A);
@@ -904,29 +907,40 @@ static int sn65dsi83_parse_dt(struct sn65dsi83 *ctx, enum sn65dsi83_model model)
 		return ret;
 
 	ctx->lvds_dual_link = false;
-	ctx->lvds_dual_link_even_odd_swap = false;
+	ctx->lvds_channel_swap = false;
 	if (model != MODEL_SN65DSI83) {
-		struct device_node *port2, *port3;
+		struct device_node *port0, *port1, *port2, *port3;
 		int dual_link;
 
+		port0 = of_graph_get_port_by_id(dev->of_node, 0);
+		port1 = of_graph_get_port_by_id(dev->of_node, 1);
 		port2 = of_graph_get_port_by_id(dev->of_node, 2);
 		port3 = of_graph_get_port_by_id(dev->of_node, 3);
 		dual_link = drm_of_lvds_get_dual_link_pixel_order(port2, port3);
-		of_node_put(port2);
-		of_node_put(port3);
 
 		if (dual_link == DRM_LVDS_DUAL_LINK_ODD_EVEN_PIXELS) {
-			ctx->lvds_dual_link = true;
 			/* Odd pixels to LVDS Channel A, even pixels to B */
-			ctx->lvds_dual_link_even_odd_swap = false;
-		} else if (dual_link == DRM_LVDS_DUAL_LINK_EVEN_ODD_PIXELS) {
 			ctx->lvds_dual_link = true;
+		} else if (dual_link == DRM_LVDS_DUAL_LINK_EVEN_ODD_PIXELS) {
 			/* Even pixels to LVDS Channel A, odd pixels to B */
-			ctx->lvds_dual_link_even_odd_swap = true;
+			ctx->lvds_dual_link = true;
+			ctx->lvds_channel_swap = true;
+		} else if (port0 && !port1 && port2 && !port3) {
+			/* DSI Channel A to LVDS Channel A */
+			pr_warn("%s: DSI Channel A to LVDS Channel A\n", __func__);
+		} else if (port0 && !port1 && !port2 && port3) {
+			/* DSI Channel A to LVDS Channel B */
+			pr_warn("%s: DSI Channel A to LVDS Channel B\n", __func__);
+			ctx->lvds_channel_swap = true;
+			panel_port = 3;
 		}
+		of_node_put(port0);
+		of_node_put(port1);
+		of_node_put(port2);
+		of_node_put(port3);
 	}
 
-	panel_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 2, 0);
+	panel_bridge = devm_drm_of_get_bridge(dev, dev->of_node, panel_port, 0);
 	if (IS_ERR(panel_bridge))
 		return dev_err_probe(dev, PTR_ERR(panel_bridge), "Failed to get panel bridge\n");
 

-- 
2.55.0

Re: [PATCH 1/2] drm/bridge: ti-sn65dsi83: Support LVDS Channel B on SN65DSI84

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Leftover debug `pr_warn` statements unconditionally print warnings on standard, valid hardware configurations during device probe.
--

--- Patch [1]: [PATCH 1/2] drm/bridge: ti-sn65dsi83: Support LVDS Channel B on SN65DSI84 ---
commit f232cbf82d686f0a82f41ef8eedda5654e9deea0
Author: Esben Haabendal <esben@geanix.com>

drm/bridge: ti-sn65dsi83: Support LVDS Channel B on SN65DSI84

This adds support for using SN65DSI84 in single-link mode with output to
LVDS Channel B.

> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> index 42b451432bbb6..32e9eadd7452f 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c

[ ... ]

> @@ -904,29 +907,40 @@ static int sn65dsi83_parse_dt(struct sn65dsi83 *ctx, enum sn65dsi83_model model)
>  		return ret;
>  
>  	ctx->lvds_dual_link = false;
> -	ctx->lvds_dual_link_even_odd_swap = false;
> +	ctx->lvds_channel_swap = false;
>  	if (model != MODEL_SN65DSI83) {
> -		struct device_node *port2, *port3;
> +		struct device_node *port0, *port1, *port2, *port3;
>  		int dual_link;
>  
> +		port0 = of_graph_get_port_by_id(dev->of_node, 0);
> +		port1 = of_graph_get_port_by_id(dev->of_node, 1);
>  		port2 = of_graph_get_port_by_id(dev->of_node, 2);
>  		port3 = of_graph_get_port_by_id(dev->of_node, 3);
>  		dual_link = drm_of_lvds_get_dual_link_pixel_order(port2, port3);
> -		of_node_put(port2);
> -		of_node_put(port3);
>  
>  		if (dual_link == DRM_LVDS_DUAL_LINK_ODD_EVEN_PIXELS) {
> -			ctx->lvds_dual_link = true;
>  			/* Odd pixels to LVDS Channel A, even pixels to B */
> -			ctx->lvds_dual_link_even_odd_swap = false;
> -		} else if (dual_link == DRM_LVDS_DUAL_LINK_EVEN_ODD_PIXELS) {
>  			ctx->lvds_dual_link = true;
> +		} else if (dual_link == DRM_LVDS_DUAL_LINK_EVEN_ODD_PIXELS) {
>  			/* Even pixels to LVDS Channel A, odd pixels to B */
> -			ctx->lvds_dual_link_even_odd_swap = true;
> +			ctx->lvds_dual_link = true;
> +			ctx->lvds_channel_swap = true;
> +		} else if (port0 && !port1 && port2 && !port3) {
> +			/* DSI Channel A to LVDS Channel A */
> +			pr_warn("%s: DSI Channel A to LVDS Channel A\n", __func__);
> +		} else if (port0 && !port1 && !port2 && port3) {
> +			/* DSI Channel A to LVDS Channel B */
> +			pr_warn("%s: DSI Channel A to LVDS Channel B\n", __func__);

[Severity: Medium]
Are these pr_warn() statements leftover debugging code?

Since this patch introduces normal support for single-link modes, printing
unconditional warnings in sn65dsi83_parse_dt() on perfectly valid hardware 
configurations (single-link DSI to LVDS channel A or B) will pollute the 
dmesg log during device probe. This might cause false failures in CI systems 
that check for kernel warnings, and confuse users into thinking there is a 
hardware or driver issue.

> +			ctx->lvds_channel_swap = true;
> +			panel_port = 3;
>  		}
> +		of_node_put(port0);
> +		of_node_put(port1);
> +		of_node_put(port2);
> +		of_node_put(port3);
>  	}
>  
> -	panel_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 2, 0);
> +	panel_bridge = devm_drm_of_get_bridge(dev, dev->of_node, panel_port, 0);
>  	if (IS_ERR(panel_bridge))
>  		return dev_err_probe(dev, PTR_ERR(panel_bridge), "Failed to get panel bridge\n");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260711-ti-sn65dsi83-fixes-v1-0-d85eb5342b98@geanix.com?part=1

Re: [PATCH 1/2] drm/bridge: ti-sn65dsi83: Support LVDS Channel B on SN65DSI84

[Luca Ceresoli]

On Sat, 11 Jul 2026 13:51:14 +0200, Esben Haabendal <esben@geanix.com> wrote:

Hi Esben,

>
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> index e6dbe51d0dba..b11fbeb71229 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> @@ -11,6 +11,8 @@
>   *   = 1x Single-link DSI ~ 2x Single-link or 1x Dual-link LVDS
>   *   - Supported
>   *   - Dual-link LVDS mode tested
> + *   - Single-link to LVDS Channel A tested.
> + *   - Single-link to LVDS Channel B tested.

Small nit: no '.' at the end of the line, as for other lines in this list.

> @@ -904,29 +907,40 @@ static int sn65dsi83_parse_dt(struct sn65dsi83 *ctx, enum sn65dsi83_model model)
> [ ... skip 26 lines ... ]
> -			ctx->lvds_dual_link_even_odd_swap = true;
> +			ctx->lvds_dual_link = true;
> +			ctx->lvds_channel_swap = true;
> +		} else if (port0 && !port1 && port2 && !port3) {
> +			/* DSI Channel A to LVDS Channel A */
> +			pr_warn("%s: DSI Channel A to LVDS Channel A\n", __func__);

No readon to warn here. At most a dev_dbg() can fit here, but I'd rather
just print nothing.

Otherwise looks good.

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH 2/2] drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking

[Esben Haabendal]

Locking PLL requires the DSI HS clock to be running, which it might not be
in probe(), but should be in atomic_enable().

This resolves issues like this:

sn65dsi83 1-002c: failed to lock PLL, ret=-110
sn65dsi83 1-002c: Unexpected link status 0x01
sn65dsi83 1-002c: Unexpected link status 0x01
sn65dsi83 1-002c: reset the pipe

as seen with nwl-dsi bridge and others.

This is the same issue as addressed in the patch by Gary Bisson [1],
but changing the ti-sn65dsi83 driver instead, so we don't have to change
all other drivers that could potentially be used with this chip.

[1] https://lore.kernel.org/all/20260120-mtkdsi-v1-1-b0f4094f3ac3@gmail.com/

Signed-off-by: Esben Haabendal <esben@geanix.com>
---
 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index 32e9eadd7452..e87bed3a6640 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -532,7 +532,6 @@ static void sn65dsi83_atomic_pre_enable(struct drm_bridge *bridge,
 	bool test_pattern = sn65dsi83_test_pattern;
 	bool lvds_format_24bpp;
 	bool lvds_format_jeida;
-	unsigned int pval;
 	__le16 le16val;
 	u16 val;
 	int ret;
@@ -682,26 +681,12 @@ static void sn65dsi83_atomic_pre_enable(struct drm_bridge *bridge,
 	regmap_write(ctx->regmap, REG_VID_CHA_TEST_PATTERN,
 		     test_pattern ? REG_VID_CHA_TEST_PATTERN_EN : 0);
 
-	/* Enable PLL */
-	regmap_write(ctx->regmap, REG_RC_PLL_EN, REG_RC_PLL_EN_PLL_EN);
-	usleep_range(3000, 4000);
-	ret = regmap_read_poll_timeout(ctx->regmap, REG_RC_LVDS_PLL, pval,
-				       pval & REG_RC_LVDS_PLL_PLL_EN_STAT,
-				       1000, 100000);
-	if (ret) {
-		dev_err(ctx->dev, "failed to lock PLL, ret=%i\n", ret);
-		/* On failure, disable PLL again and exit. */
-		regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
-		goto err_add_action;
-	}
-
 	/* Trigger reset after CSR register update. */
 	regmap_write(ctx->regmap, REG_RC_RESET, REG_RC_RESET_SOFT_RESET);
 
 	/* Wait for 10ms after soft reset as specified in datasheet */
 	usleep_range(10000, 12000);
 
-err_add_action:
 	devm_add_action(ctx->dev, sn65dsi83_release_resources, ctx);
 err_exit:
 	drm_bridge_exit(idx);
@@ -712,11 +697,24 @@ static void sn65dsi83_atomic_enable(struct drm_bridge *bridge,
 {
 	struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
 	unsigned int pval;
-	int idx;
+	int idx, ret;
 
 	if (!drm_bridge_enter(bridge, &idx))
 		return;
 
+	/* Enable PLL */
+	regmap_write(ctx->regmap, REG_RC_PLL_EN, REG_RC_PLL_EN_PLL_EN);
+	usleep_range(3000, 4000);
+	ret = regmap_read_poll_timeout(ctx->regmap, REG_RC_LVDS_PLL, pval,
+				       pval & REG_RC_LVDS_PLL_PLL_EN_STAT,
+				       1000, 100000);
+	if (ret) {
+		dev_err(ctx->dev, "failed to lock PLL, ret=%i\n", ret);
+		/* On failure, disable PLL again and exit. */
+		regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
+		return;
+	}
+
 	/* Clear all errors that got asserted during initialization. */
 	regmap_read(ctx->regmap, REG_IRQ_STAT, &pval);
 	regmap_write(ctx->regmap, REG_IRQ_STAT, pval);

-- 
2.55.0

Re: [PATCH 2/2] drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Missing SRCU read lock release in error path of sn65dsi83_atomic_enable().

Pre-existing issues:
- [High] Missing SRCU read lock release in error path of sn65dsi83_reset_work().
--

--- Patch [2]: [PATCH 2/2] drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking ---
commit 0c69248e520fff24e51d45bfab961991fbdb4267
Author: Esben Haabendal <esben@geanix.com>

drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking

This patch moves the PLL locking sequence from atomic_pre_enable to
atomic_enable. This ensures the DSI HS clock is running before attempting to
lock the PLL.

Link: https://lore.kernel.org/all/20260120-mtkdsi-v1-1-b0f4094f3ac3@gmail.com/

> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> index 32e9eadd7452f..e87bed3a6640b 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c

[ ... ]

> @@ -712,11 +697,24 @@ static void sn65dsi83_atomic_enable(struct drm_bridge *bridge,
>  {
>  	struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
>  	unsigned int pval;
> -	int idx;
> +	int idx, ret;
>  
>  	if (!drm_bridge_enter(bridge, &idx))
>  		return;
>  
> +	/* Enable PLL */
> +	regmap_write(ctx->regmap, REG_RC_PLL_EN, REG_RC_PLL_EN_PLL_EN);
> +	usleep_range(3000, 4000);
> +	ret = regmap_read_poll_timeout(ctx->regmap, REG_RC_LVDS_PLL, pval,
> +				       pval & REG_RC_LVDS_PLL_PLL_EN_STAT,
> +				       1000, 100000);
> +	if (ret) {
> +		dev_err(ctx->dev, "failed to lock PLL, ret=%i\n", ret);
> +		/* On failure, disable PLL again and exit. */
> +		regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
> +		return;

[Severity: High]
Does this error path leak the SRCU read lock acquired by drm_bridge_enter()?

If the PLL lock fails and regmap_read_poll_timeout() returns an error, the
function returns immediately without calling drm_bridge_exit(idx). 

This unreleased lock could cause subsequent calls to synchronize_srcu() to
hang during module unload or bridge unplug.

> +	}
> +
>  	/* Clear all errors that got asserted during initialization. */
>  	regmap_read(ctx->regmap, REG_IRQ_STAT, &pval);

[Severity: High]
This is a pre-existing issue, but I noticed a similar pattern in another
function in this file. Does sn65dsi83_reset_work() also leak the SRCU read
lock on failure?

drivers/gpu/drm/bridge/ti-sn65dsi83.c:sn65dsi83_reset_work() {
    if (!drm_bridge_enter(&ctx->bridge, &idx))
        return;
    ...
    ret = sn65dsi83_reset_pipe(ctx);
    if (ret) {
        dev_err(ctx->dev, "reset pipe failed %pe\n", ERR_PTR(ret));
        return;
    }
    ...
    drm_bridge_exit(idx);
}

Like the atomic_enable path, if sn65dsi83_reset_pipe() fails, it appears to
return early without calling drm_bridge_exit(idx).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260711-ti-sn65dsi83-fixes-v1-0-d85eb5342b98@geanix.com?part=2

Re: [PATCH 2/2] drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking

[Esben Haabendal]

<sashiko-bot@kernel.org> writes:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> New issues:
> - [High] Missing SRCU read lock release in error path of sn65dsi83_atomic_enable().
>
> Pre-existing issues:
> - [High] Missing SRCU read lock release in error path of sn65dsi83_reset_work().
> --
>
> --- Patch [2]: [PATCH 2/2] drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking ---
> commit 0c69248e520fff24e51d45bfab961991fbdb4267
> Author: Esben Haabendal <esben@geanix.com>
>
> drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking
>
> This patch moves the PLL locking sequence from atomic_pre_enable to
> atomic_enable. This ensures the DSI HS clock is running before attempting to
> lock the PLL.
>
> Link: https://lore.kernel.org/all/20260120-mtkdsi-v1-1-b0f4094f3ac3@gmail.com/
>
>> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
>> index 32e9eadd7452f..e87bed3a6640b 100644
>> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
>> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
>
> [ ... ]
>
>> @@ -712,11 +697,24 @@ static void sn65dsi83_atomic_enable(struct drm_bridge *bridge,
>>  {
>>  	struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
>>  	unsigned int pval;
>> -	int idx;
>> +	int idx, ret;
>>
>>  	if (!drm_bridge_enter(bridge, &idx))
>>  		return;
>>
>> +	/* Enable PLL */
>> +	regmap_write(ctx->regmap, REG_RC_PLL_EN, REG_RC_PLL_EN_PLL_EN);
>> +	usleep_range(3000, 4000);
>> +	ret = regmap_read_poll_timeout(ctx->regmap, REG_RC_LVDS_PLL, pval,
>> +				       pval & REG_RC_LVDS_PLL_PLL_EN_STAT,
>> +				       1000, 100000);
>> +	if (ret) {
>> +		dev_err(ctx->dev, "failed to lock PLL, ret=%i\n", ret);
>> +		/* On failure, disable PLL again and exit. */
>> +		regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
>> +		return;
>
> [Severity: High]
> Does this error path leak the SRCU read lock acquired by drm_bridge_enter()?
>
> If the PLL lock fails and regmap_read_poll_timeout() returns an error, the
> function returns immediately without calling drm_bridge_exit(idx).
>
> This unreleased lock could cause subsequent calls to synchronize_srcu() to
> hang during module unload or bridge unplug.

Yes, that does look like a bug. I will fix it for v2.

>> +	}
>> +
>>  	/* Clear all errors that got asserted during initialization. */
>>  	regmap_read(ctx->regmap, REG_IRQ_STAT, &pval);
>
> [Severity: High]
> This is a pre-existing issue, but I noticed a similar pattern in another
> function in this file. Does sn65dsi83_reset_work() also leak the SRCU read
> lock on failure?
>
> drivers/gpu/drm/bridge/ti-sn65dsi83.c:sn65dsi83_reset_work() {
>     if (!drm_bridge_enter(&ctx->bridge, &idx))
>         return;
>     ...
>     ret = sn65dsi83_reset_pipe(ctx);
>     if (ret) {
>         dev_err(ctx->dev, "reset pipe failed %pe\n", ERR_PTR(ret));
>         return;
>     }
>     ...
>     drm_bridge_exit(idx);
> }
>
> Like the atomic_enable path, if sn65dsi83_reset_pipe() fails, it appears to
> return early without calling drm_bridge_exit(idx).

Yes.

But is it enough to call drm_bridge_exit(idx), or do we want to
enable_irq() also? We are calling disable_irq_nosync() before scheduling
reset_ork, so if don't enable it again in case of error, it will be
stuck disabled. But what is supposed to happen if/when
sn65dsi83_reset_pipe() fails?

If we enable_irq() again, are we just going to endure endless pain?

/Esben

Re: [PATCH 2/2] drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking

[Luca Ceresoli]

On Sat, 11 Jul 2026 13:51:15 +0200, Esben Haabendal <esben@geanix.com> wrote:

Hi Esben,

+Cc Gary

> [...]
> 
> This is the same issue as addressed in the patch by Gary Bisson [1],
> but changing the ti-sn65dsi83 driver instead, so we don't have to change
> all other drivers that could potentially be used with this chip.
> 
> [1] https://lore.kernel.org/all/20260120-mtkdsi-v1-1-b0f4094f3ac3@gmail.com/

AFAICU your patch would replace Gary's one. Also Gary's patch has been
reported to introduce regressions but it hasn't been reverted yet. Can you
reply to that thread mentioning your patch, so everybody in the discussion
is aware of your alternative proposal?

> 
> Signed-off-by: Esben Haabendal <esben@geanix.com>

If it's a fix it should have a 'Fixes:' tag and a 'Cc: stable' line
(https://docs.kernel.org/process/submitting-patches.html) and it should be
the first patch in the series, to facilitate cherry-picking on stable
branches.

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 2/2] drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking

[Gary Bisson]

Hi Esben and Luca,

On Wed, Jul 15, 2026 at 10:52:18AM +0200, Luca Ceresoli wrote:
> On Sat, 11 Jul 2026 13:51:15 +0200, Esben Haabendal <esben@geanix.com> wrote:
> 
> Hi Esben,
> 
> +Cc Gary
> 
> > [...]
> > 
> > This is the same issue as addressed in the patch by Gary Bisson [1],
> > but changing the ti-sn65dsi83 driver instead, so we don't have to change
> > all other drivers that could potentially be used with this chip.
> > 
> > [1] https://lore.kernel.org/all/20260120-mtkdsi-v1-1-b0f4094f3ac3@gmail.com/
> 
> AFAICU your patch would replace Gary's one. Also Gary's patch has been
> reported to introduce regressions but it hasn't been reverted yet. Can you
> reply to that thread mentioning your patch, so everybody in the discussion
> is aware of your alternative proposal?

Thanks for including me. I just tested this change and reverted my other
one (mtk_dsi) and confirm that it works on my Tungsten510 + SN65DSI83 +
tm070jdhg30 panel.

Tested-by: Gary Bisson <bisson.gary@gmail.com>

Note that the sn65dsi83 driver wasn't changed as I thought the PLL lock
in pre-enable was on purpose. It was introduced by Frieder with this
commit. Adding him to the thread to weigh in.
dd9e329af723 drm/bridge: ti-sn65dsi83: Fix enable/disable flow to meet spec

Regards,
Gary

Re: [PATCH 2/2] drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking

[Frieder Schrempf]

On 15.07.26 15:52, Gary Bisson wrote:
> Hi Esben and Luca,
> 
> On Wed, Jul 15, 2026 at 10:52:18AM +0200, Luca Ceresoli wrote:
>> On Sat, 11 Jul 2026 13:51:15 +0200, Esben Haabendal <esben@geanix.com> wrote:
>>
>> Hi Esben,
>>
>> +Cc Gary
>>
>>> [...]
>>>
>>> This is the same issue as addressed in the patch by Gary Bisson [1],
>>> but changing the ti-sn65dsi83 driver instead, so we don't have to change
>>> all other drivers that could potentially be used with this chip.
>>>
>>> [1] https://lore.kernel.org/all/20260120-mtkdsi-v1-1-b0f4094f3ac3@gmail.com/
>>
>> AFAICU your patch would replace Gary's one. Also Gary's patch has been
>> reported to introduce regressions but it hasn't been reverted yet. Can you
>> reply to that thread mentioning your patch, so everybody in the discussion
>> is aware of your alternative proposal?
> 
> Thanks for including me. I just tested this change and reverted my other
> one (mtk_dsi) and confirm that it works on my Tungsten510 + SN65DSI83 +
> tm070jdhg30 panel.
> 
> Tested-by: Gary Bisson <bisson.gary@gmail.com>
> 
> Note that the sn65dsi83 driver wasn't changed as I thought the PLL lock
> in pre-enable was on purpose. It was introduced by Frieder with this
> commit. Adding him to the thread to weigh in.
> dd9e329af723 drm/bridge: ti-sn65dsi83: Fix enable/disable flow to meet spec

Thanks for the mention. This was introduced to keep the init order
according to the datasheet. The DSI host first needs to put the DSI
lanes into the correct state in its pre_enable(). Then the bridge needs
to be enabled (including the PLL) in the pre_enable() of the bridge
driver. Only after that the DSI host is allowed to stream data.

Moving the PLL init from pre_enable() to enable() probably violates this
order. In the past this lead to sporadic issues with some hardware
setups (depending on the display and the DSI host). Some of this is also
described in the docs: [1]

So from the first glance, I would assume this issue needs to be fixed in
the DSI host driver.

[1]
https://docs.kernel.org/gpu/drm-kms-helpers.html#mipi-dsi-bridge-operation

Re: [PATCH 2/2] drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking

[Alexander Stein]

Hi,

Am Mittwoch, 15. Juli 2026, 16:14:42 CEST schrieb Frieder Schrempf:
> On 15.07.26 15:52, Gary Bisson wrote:
> > Hi Esben and Luca,
> > 
> > On Wed, Jul 15, 2026 at 10:52:18AM +0200, Luca Ceresoli wrote:
> >> On Sat, 11 Jul 2026 13:51:15 +0200, Esben Haabendal <esben@geanix.com> wrote:
> >>
> >> Hi Esben,
> >>
> >> +Cc Gary
> >>
> >>> [...]
> >>>
> >>> This is the same issue as addressed in the patch by Gary Bisson [1],
> >>> but changing the ti-sn65dsi83 driver instead, so we don't have to change
> >>> all other drivers that could potentially be used with this chip.
> >>>
> >>> [1] https://lore.kernel.org/all/20260120-mtkdsi-v1-1-b0f4094f3ac3@gmail.com/
> >>
> >> AFAICU your patch would replace Gary's one. Also Gary's patch has been
> >> reported to introduce regressions but it hasn't been reverted yet. Can you
> >> reply to that thread mentioning your patch, so everybody in the discussion
> >> is aware of your alternative proposal?
> > 
> > Thanks for including me. I just tested this change and reverted my other
> > one (mtk_dsi) and confirm that it works on my Tungsten510 + SN65DSI83 +
> > tm070jdhg30 panel.
> > 
> > Tested-by: Gary Bisson <bisson.gary@gmail.com>
> > 
> > Note that the sn65dsi83 driver wasn't changed as I thought the PLL lock
> > in pre-enable was on purpose. It was introduced by Frieder with this
> > commit. Adding him to the thread to weigh in.
> > dd9e329af723 drm/bridge: ti-sn65dsi83: Fix enable/disable flow to meet spec
> 
> Thanks for the mention. This was introduced to keep the init order
> according to the datasheet. The DSI host first needs to put the DSI
> lanes into the correct state in its pre_enable(). Then the bridge needs
> to be enabled (including the PLL) in the pre_enable() of the bridge
> driver. Only after that the DSI host is allowed to stream data.
> 
> Moving the PLL init from pre_enable() to enable() probably violates this
> order. In the past this lead to sporadic issues with some hardware
> setups (depending on the display and the DSI host). Some of this is also
> described in the docs: [1]
> 
> So from the first glance, I would assume this issue needs to be fixed in
> the DSI host driver.
> 
> [1]
> https://docs.kernel.org/gpu/drm-kms-helpers.html#mipi-dsi-bridge-operation

This link says
> Ordinarily the downstream bridge DSI peripheral pre_enable will have been
> called before the DSI host. If the DSI peripheral requires LP-11 and/or the
> clock lane to be in HS mode prior to pre_enable, then it can set the
> pre_enable_prev_first flag to request the pre_enable (and post_disable)
> order to be altered to enable the DSI host first.

So IIRC if pre_enable() already requires LP-11 on data and HS on clock,
pre_enable_prev_first should be true then.

Best regards,
Alexander
-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

Re: [PATCH 2/2] drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking

[Frieder Schrempf]

On 16.07.26 11:32, Alexander Stein wrote:
> Hi,
> 
> Am Mittwoch, 15. Juli 2026, 16:14:42 CEST schrieb Frieder Schrempf:
>> On 15.07.26 15:52, Gary Bisson wrote:
>>> Hi Esben and Luca,
>>>
>>> On Wed, Jul 15, 2026 at 10:52:18AM +0200, Luca Ceresoli wrote:
>>>> On Sat, 11 Jul 2026 13:51:15 +0200, Esben Haabendal <esben@geanix.com> wrote:
>>>>
>>>> Hi Esben,
>>>>
>>>> +Cc Gary
>>>>
>>>>> [...]
>>>>>
>>>>> This is the same issue as addressed in the patch by Gary Bisson [1],
>>>>> but changing the ti-sn65dsi83 driver instead, so we don't have to change
>>>>> all other drivers that could potentially be used with this chip.
>>>>>
>>>>> [1] https://lore.kernel.org/all/20260120-mtkdsi-v1-1-b0f4094f3ac3@gmail.com/
>>>>
>>>> AFAICU your patch would replace Gary's one. Also Gary's patch has been
>>>> reported to introduce regressions but it hasn't been reverted yet. Can you
>>>> reply to that thread mentioning your patch, so everybody in the discussion
>>>> is aware of your alternative proposal?
>>>
>>> Thanks for including me. I just tested this change and reverted my other
>>> one (mtk_dsi) and confirm that it works on my Tungsten510 + SN65DSI83 +
>>> tm070jdhg30 panel.
>>>
>>> Tested-by: Gary Bisson <bisson.gary@gmail.com>
>>>
>>> Note that the sn65dsi83 driver wasn't changed as I thought the PLL lock
>>> in pre-enable was on purpose. It was introduced by Frieder with this
>>> commit. Adding him to the thread to weigh in.
>>> dd9e329af723 drm/bridge: ti-sn65dsi83: Fix enable/disable flow to meet spec
>>
>> Thanks for the mention. This was introduced to keep the init order
>> according to the datasheet. The DSI host first needs to put the DSI
>> lanes into the correct state in its pre_enable(). Then the bridge needs
>> to be enabled (including the PLL) in the pre_enable() of the bridge
>> driver. Only after that the DSI host is allowed to stream data.
>>
>> Moving the PLL init from pre_enable() to enable() probably violates this
>> order. In the past this lead to sporadic issues with some hardware
>> setups (depending on the display and the DSI host). Some of this is also
>> described in the docs: [1]
>>
>> So from the first glance, I would assume this issue needs to be fixed in
>> the DSI host driver.
>>
>> [1]
>> https://docs.kernel.org/gpu/drm-kms-helpers.html#mipi-dsi-bridge-operation
> 
> This link says
>> Ordinarily the downstream bridge DSI peripheral pre_enable will have been
>> called before the DSI host. If the DSI peripheral requires LP-11 and/or the
>> clock lane to be in HS mode prior to pre_enable, then it can set the
>> pre_enable_prev_first flag to request the pre_enable (and post_disable)
>> order to be altered to enable the DSI host first.
> 
> So IIRC if pre_enable() already requires LP-11 on data and HS on clock,
> pre_enable_prev_first should be true then.
> 
Yes, and it is true:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/bridge/ti-sn65dsi83.c?h=v7.2-rc3#n1051