* [PATCH 0/2] drm/bridge: ti-sn65dsi83: Various fixes
@ 2026-07-11 11:51 Esben Haabendal
2026-07-11 11:51 ` [PATCH 1/2] drm/bridge: ti-sn65dsi83: Support LVDS Channel B on SN65DSI84 Esben Haabendal
2026-07-11 11:51 ` [PATCH 2/2] drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking Esben Haabendal
0 siblings, 2 replies; 10+ messages in thread
From: Esben Haabendal @ 2026-07-11 11:51 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: dri-devel, linux-kernel, 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>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] drm/bridge: ti-sn65dsi83: Support LVDS Channel B on SN65DSI84
2026-07-11 11:51 [PATCH 0/2] drm/bridge: ti-sn65dsi83: Various fixes Esben Haabendal
@ 2026-07-11 11:51 ` Esben Haabendal
2026-07-11 11:57 ` sashiko-bot
2026-07-15 8:52 ` Luca Ceresoli
2026-07-11 11:51 ` [PATCH 2/2] drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking Esben Haabendal
1 sibling, 2 replies; 10+ messages in thread
From: Esben Haabendal @ 2026-07-11 11:51 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: dri-devel, linux-kernel, 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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking
2026-07-11 11:51 [PATCH 0/2] drm/bridge: ti-sn65dsi83: Various fixes Esben Haabendal
2026-07-11 11:51 ` [PATCH 1/2] drm/bridge: ti-sn65dsi83: Support LVDS Channel B on SN65DSI84 Esben Haabendal
@ 2026-07-11 11:51 ` Esben Haabendal
2026-07-11 11:59 ` sashiko-bot
2026-07-15 8:52 ` Luca Ceresoli
1 sibling, 2 replies; 10+ messages in thread
From: Esben Haabendal @ 2026-07-11 11:51 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: dri-devel, linux-kernel, 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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] drm/bridge: ti-sn65dsi83: Support LVDS Channel B on SN65DSI84
2026-07-11 11:51 ` [PATCH 1/2] drm/bridge: ti-sn65dsi83: Support LVDS Channel B on SN65DSI84 Esben Haabendal
@ 2026-07-11 11:57 ` sashiko-bot
2026-07-15 8:52 ` Luca Ceresoli
1 sibling, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-07-11 11:57 UTC (permalink / raw)
To: Esben Haabendal; +Cc: dri-devel
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking
2026-07-11 11:51 ` [PATCH 2/2] drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking Esben Haabendal
@ 2026-07-11 11:59 ` sashiko-bot
2026-07-11 12:26 ` Esben Haabendal
2026-07-15 8:52 ` Luca Ceresoli
1 sibling, 1 reply; 10+ messages in thread
From: sashiko-bot @ 2026-07-11 11:59 UTC (permalink / raw)
To: Esben Haabendal; +Cc: dri-devel
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking
2026-07-11 11:59 ` sashiko-bot
@ 2026-07-11 12:26 ` Esben Haabendal
0 siblings, 0 replies; 10+ messages in thread
From: Esben Haabendal @ 2026-07-11 12:26 UTC (permalink / raw)
To: sashiko-bot; +Cc: sashiko-reviews, dri-devel
<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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] drm/bridge: ti-sn65dsi83: Support LVDS Channel B on SN65DSI84
2026-07-11 11:51 ` [PATCH 1/2] drm/bridge: ti-sn65dsi83: Support LVDS Channel B on SN65DSI84 Esben Haabendal
2026-07-11 11:57 ` sashiko-bot
@ 2026-07-15 8:52 ` Luca Ceresoli
1 sibling, 0 replies; 10+ messages in thread
From: Luca Ceresoli @ 2026-07-15 8:52 UTC (permalink / raw)
To: Esben Haabendal
Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
dri-devel, linux-kernel
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking
2026-07-11 11:51 ` [PATCH 2/2] drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking Esben Haabendal
2026-07-11 11:59 ` sashiko-bot
@ 2026-07-15 8:52 ` Luca Ceresoli
2026-07-15 13:52 ` Gary Bisson
1 sibling, 1 reply; 10+ messages in thread
From: Luca Ceresoli @ 2026-07-15 8:52 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: dri-devel, linux-kernel, Esben Haabendal, Gary Bisson
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking
2026-07-15 8:52 ` Luca Ceresoli
@ 2026-07-15 13:52 ` Gary Bisson
2026-07-15 14:14 ` Frieder Schrempf
0 siblings, 1 reply; 10+ messages in thread
From: Gary Bisson @ 2026-07-15 13:52 UTC (permalink / raw)
To: Luca Ceresoli
Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, dri-devel,
linux-kernel, Esben Haabendal, frieder.schrempf
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking
2026-07-15 13:52 ` Gary Bisson
@ 2026-07-15 14:14 ` Frieder Schrempf
0 siblings, 0 replies; 10+ messages in thread
From: Frieder Schrempf @ 2026-07-15 14:14 UTC (permalink / raw)
To: Gary Bisson, Luca Ceresoli
Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, dri-devel,
linux-kernel, Esben Haabendal
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
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-07-15 14:14 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-11 11:51 [PATCH 0/2] drm/bridge: ti-sn65dsi83: Various fixes Esben Haabendal
2026-07-11 11:51 ` [PATCH 1/2] drm/bridge: ti-sn65dsi83: Support LVDS Channel B on SN65DSI84 Esben Haabendal
2026-07-11 11:57 ` sashiko-bot
2026-07-15 8:52 ` Luca Ceresoli
2026-07-11 11:51 ` [PATCH 2/2] drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking Esben Haabendal
2026-07-11 11:59 ` sashiko-bot
2026-07-11 12:26 ` Esben Haabendal
2026-07-15 8:52 ` Luca Ceresoli
2026-07-15 13:52 ` Gary Bisson
2026-07-15 14:14 ` Frieder Schrempf
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.