* [PATCH v2 0/3] drm/bridge: ti-sn65dsi83: Various fixes
@ 2026-07-15 13:10 Esben Haabendal
2026-07-15 13:10 ` [PATCH v2 1/3] drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking Esben Haabendal
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Esben Haabendal @ 2026-07-15 13:10 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,
Linus Walleij, Frieder Schrempf, Marek Vasut
Cc: Esben Haabendal, dri-devel, linux-kernel, stable
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>
---
Changes in v2:
- Fix error handling in sn65dsi83_atomic_enable() to ensure
drm_brige_exit() is always called on exit.
- Change logging level from warn to dbg for two messages in
sn65dsi83_parse_dt().
- Removed trailing dot in comment lines.
- Added patch with fix so that DRM bridge critical section is exited on
error in sn65dsi83_reset_work().
- Added Fixes and Cc: stable tags to the premature PLL patch.
- Re-ordered patch so fixes comes first.
- Link to v1: https://patch.msgid.link/20260711-ti-sn65dsi83-fixes-v1-0-d85eb5342b98@geanix.com
To: Andrzej Hajda <andrzej.hajda@intel.com>
To: Neil Armstrong <neil.armstrong@linaro.org>
To: Robert Foss <rfoss@kernel.org>
To: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
To: Jonas Karlman <jonas@kwiboo.se>
To: Jernej Skrabec <jernej.skrabec@gmail.com>
To: Luca Ceresoli <luca.ceresoli@bootlin.com>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
To: Maxime Ripard <mripard@kernel.org>
To: Thomas Zimmermann <tzimmermann@suse.de>
To: David Airlie <airlied@gmail.com>
To: Simona Vetter <simona@ffwll.ch>
To: Linus Walleij <linusw@kernel.org>
To: Frieder Schrempf <frieder.schrempf@kontron.de>
To: Marek Vasut <marex@denx.de>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org
---
Esben Haabendal (3):
drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking
drm/bridge: ti-sn65dsi83: Fix error handling in sn65dsi83_reset_work()
drm/bridge: ti-sn65dsi83: Support LVDS Channel B on SN65DSI84
drivers/gpu/drm/bridge/ti-sn65dsi83.c | 68 ++++++++++++++++++++---------------
1 file changed, 40 insertions(+), 28 deletions(-)
---
base-commit: 8cdeaa50eae8dad34885515f62559ee83e7e8dda
change-id: 20260710-ti-sn65dsi83-fixes-1d08e0ac67b3
Best regards,
--
Esben Haabendal <esben@geanix.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/3] drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking
2026-07-15 13:10 [PATCH v2 0/3] drm/bridge: ti-sn65dsi83: Various fixes Esben Haabendal
@ 2026-07-15 13:10 ` Esben Haabendal
2026-07-15 13:32 ` sashiko-bot
2026-07-15 13:10 ` [PATCH v2 2/3] drm/bridge: ti-sn65dsi83: Fix error handling in sn65dsi83_reset_work() Esben Haabendal
2026-07-15 13:10 ` [PATCH v2 3/3] drm/bridge: ti-sn65dsi83: Support LVDS Channel B on SN65DSI84 Esben Haabendal
2 siblings, 1 reply; 7+ messages in thread
From: Esben Haabendal @ 2026-07-15 13:10 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,
Linus Walleij, Frieder Schrempf, Marek Vasut
Cc: Esben Haabendal, dri-devel, linux-kernel, stable
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/
Fixes: ceb515ba29ba ("drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver")
Cc: stable@vger.kernel.org
Signed-off-by: Esben Haabendal <esben@geanix.com>
---
drivers/gpu/drm/bridge/ti-sn65dsi83.c | 31 +++++++++++++++----------------
1 file changed, 15 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index 42b451432bbb..b4b220eee790 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -530,7 +530,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;
@@ -680,26 +679,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);
@@ -710,11 +695,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);
+ goto err_exit;
+ }
+
/* Clear all errors that got asserted during initialization. */
regmap_read(ctx->regmap, REG_IRQ_STAT, &pval);
regmap_write(ctx->regmap, REG_IRQ_STAT, pval);
@@ -734,6 +732,7 @@ static void sn65dsi83_atomic_enable(struct drm_bridge *bridge,
sn65dsi83_monitor_start(ctx);
}
+err_exit:
drm_bridge_exit(idx);
}
--
2.55.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/3] drm/bridge: ti-sn65dsi83: Fix error handling in sn65dsi83_reset_work()
2026-07-15 13:10 [PATCH v2 0/3] drm/bridge: ti-sn65dsi83: Various fixes Esben Haabendal
2026-07-15 13:10 ` [PATCH v2 1/3] drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking Esben Haabendal
@ 2026-07-15 13:10 ` Esben Haabendal
2026-07-15 13:21 ` sashiko-bot
2026-07-15 13:10 ` [PATCH v2 3/3] drm/bridge: ti-sn65dsi83: Support LVDS Channel B on SN65DSI84 Esben Haabendal
2 siblings, 1 reply; 7+ messages in thread
From: Esben Haabendal @ 2026-07-15 13:10 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,
Linus Walleij, Frieder Schrempf, Marek Vasut
Cc: Esben Haabendal, dri-devel, linux-kernel
We obviously should not leave the DRM bridge critical section activated
when exiting on error.
Signed-off-by: Esben Haabendal <esben@geanix.com>
---
drivers/gpu/drm/bridge/ti-sn65dsi83.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index b4b220eee790..7e73035d7798 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -419,11 +419,12 @@ static void sn65dsi83_reset_work(struct work_struct *ws)
ret = sn65dsi83_reset_pipe(ctx);
if (ret) {
dev_err(ctx->dev, "reset pipe failed %pe\n", ERR_PTR(ret));
- return;
+ goto err_exit;
}
if (ctx->irq)
enable_irq(ctx->irq);
+err_exit:
drm_bridge_exit(idx);
}
--
2.55.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 3/3] drm/bridge: ti-sn65dsi83: Support LVDS Channel B on SN65DSI84
2026-07-15 13:10 [PATCH v2 0/3] drm/bridge: ti-sn65dsi83: Various fixes Esben Haabendal
2026-07-15 13:10 ` [PATCH v2 1/3] drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking Esben Haabendal
2026-07-15 13:10 ` [PATCH v2 2/3] drm/bridge: ti-sn65dsi83: Fix error handling in sn65dsi83_reset_work() Esben Haabendal
@ 2026-07-15 13:10 ` Esben Haabendal
2026-07-15 13:18 ` sashiko-bot
2 siblings, 1 reply; 7+ messages in thread
From: Esben Haabendal @ 2026-07-15 13:10 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,
Linus Walleij, Frieder Schrempf, Marek Vasut
Cc: Esben Haabendal, dri-devel, linux-kernel
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 | 34 +++++++++++++++++++++++-----------
1 file changed, 23 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index 7e73035d7798..02ae2b8179b0 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,38 @@ 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 */
+ } else if (port0 && !port1 && !port2 && port3) {
+ /* DSI Channel A to LVDS Channel B */
+ 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] 7+ messages in thread
* Re: [PATCH v2 3/3] drm/bridge: ti-sn65dsi83: Support LVDS Channel B on SN65DSI84
2026-07-15 13:10 ` [PATCH v2 3/3] drm/bridge: ti-sn65dsi83: Support LVDS Channel B on SN65DSI84 Esben Haabendal
@ 2026-07-15 13:18 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-07-15 13:18 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] The logic to detect single-link LVDS Channel B requires `port@2` to be absent from the device tree, but the DT binding strictly requires `port@2` to be present.
--
--- Patch [3]: [PATCH v2 3/3] drm/bridge: ti-sn65dsi83: Support LVDS Channel B on SN65DSI84 ---
commit e7655ab4d66cbec701adf0102160008944fa2a88
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 7e73035d77986..02ae2b8179b07 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
[ ... ]
> @@ -904,29 +907,38 @@ 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);
>
> if (dual_link == DRM_LVDS_DUAL_LINK_ODD_EVEN_PIXELS) {
> /* Odd pixels to LVDS Channel A, even pixels to B */
> 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 = true;
> + ctx->lvds_channel_swap = true;
> + } else if (port0 && !port1 && port2 && !port3) {
> + /* DSI Channel A to LVDS Channel A */
> + } else if (port0 && !port1 && !port2 && port3) {
^^^^^^
[Severity: Medium]
Will this !port2 condition prevent the single-link LVDS Channel B feature
from working with a standard-compliant device tree?
If a user provides a compliant device tree by including an empty or
unconnected port@2 to satisfy the DT schema required list for this binding,
of_graph_get_port_by_id() will return a valid node.
The driver would then fail this check, fall through leaving panel_port = 2,
and subsequently fail to probe when it cannot find a panel connected to the
empty port@2.
> + /* DSI Channel A to LVDS Channel B */
> + 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/20260715-ti-sn65dsi83-fixes-v2-0-ebc4c3fe29b6@geanix.com?part=3
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/3] drm/bridge: ti-sn65dsi83: Fix error handling in sn65dsi83_reset_work()
2026-07-15 13:10 ` [PATCH v2 2/3] drm/bridge: ti-sn65dsi83: Fix error handling in sn65dsi83_reset_work() Esben Haabendal
@ 2026-07-15 13:21 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-07-15 13:21 UTC (permalink / raw)
To: Esben Haabendal; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] The error handling branch patched by this commit is dead code because sn65dsi83_reset_pipe() unconditionally returns 0.
- [High] Missing cancellation of reset_work on driver removal leads to a potential Use-After-Free.
--
--- Patch [2]: [PATCH v2 2/3] drm/bridge: ti-sn65dsi83: Fix error handling in sn65dsi83_reset_work() ---
commit da59dc684f40189c9b3344ad029c137b580d644a
Author: Esben Haabendal <esben@geanix.com>
drm/bridge: ti-sn65dsi83: Fix error handling in sn65dsi83_reset_work()
This commit updates the error handling path in sn65dsi83_reset_work() to
ensure the DRM bridge critical section is cleanly exited when resetting the
pipe fails.
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> index b4b220eee7909..7e73035d77986 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> @@ -419,11 +419,12 @@ static void sn65dsi83_reset_work(struct work_struct *ws)
> ret = sn65dsi83_reset_pipe(ctx);
> if (ret) {
> dev_err(ctx->dev, "reset pipe failed %pe\n", ERR_PTR(ret));
> - return;
> + goto err_exit;
> }
[Severity: High]
This isn't a bug introduced by this patch, but is this error handling block
actually dead code?
Looking at sn65dsi83_reset_pipe(), it appears to unconditionally return 0,
even if drm_bridge_helper_reset_crtc() returns an error like -EINVAL or
-ENOMEM:
static int sn65dsi83_reset_pipe(struct sn65dsi83 *sn65dsi83)
{
...
retry:
err = drm_bridge_helper_reset_crtc(&sn65dsi83->bridge, &ctx);
if (err == -EDEADLK) {
drm_modeset_backoff(&ctx);
goto retry;
}
drm_modeset_drop_locks(&ctx);
drm_modeset_acquire_fini(&ctx);
return 0;
}
Since ret is always 0, this error branch will never execute. If an actual
hardware error persists (e.g., PLL unlock), the driver will unconditionally
proceed to enable_irq(ctx->irq) below.
Could this result in an immediate subsequent interrupt and cause an interrupt
storm that locks up the CPU?
> if (ctx->irq)
> enable_irq(ctx->irq);
>
> +err_exit:
> drm_bridge_exit(idx);
> }
[Severity: High]
This is also a pre-existing issue, but does this workqueue function have a
use-after-free risk during driver removal?
When the driver is unbound (e.g., via sysfs), sn65dsi83_remove() unregisters
the bridge but does not appear to call cancel_work_sync(&ctx->reset_work).
If an interrupt fires right before cleanup and schedules this work,
devm_drm_bridge_alloc cleanup will free the ctx structure. The workqueue
could then execute sn65dsi83_reset_work() asynchronously, causing a UAF
when dereferencing ctx->bridge.
Should sn65dsi83_remove() be updated to cancel the pending reset work?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260715-ti-sn65dsi83-fixes-v2-0-ebc4c3fe29b6@geanix.com?part=2
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/3] drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking
2026-07-15 13:10 ` [PATCH v2 1/3] drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking Esben Haabendal
@ 2026-07-15 13:32 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-07-15 13:32 UTC (permalink / raw)
To: Esben Haabendal; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The reset_work work item is never cancelled during device removal, leading to a Use-After-Free if executed after devres cleanup.
--
--- Patch [1]: [PATCH v2 1/3] drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking ---
commit 6d16fdef43813e12caf14d60e8f68b785f57a520
Author: Esben Haabendal <esben@geanix.com>
Subject: drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking
This commit moves the PLL locking sequence from atomic_pre_enable() to
atomic_enable(), ensuring the DSI HS clock is running before the PLL is
locked. This resolves PLL lock failures seen with certain configurations.
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 42b451432bbb6..b4b220eee7909 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
[ ... ]
> @@ -734,6 +732,7 @@ static void sn65dsi83_atomic_enable(struct drm_bridge *bridge,
> sn65dsi83_monitor_start(ctx);
> }
>
> +err_exit:
> drm_bridge_exit(idx);
> }
[Severity: High]
This isn't a bug introduced by this patch, but I noticed a pre-existing
issue in this driver regarding device removal.
Could the reset_work item execute after the device has been unbound?
Looking at sn65dsi83_remove(), it unplugs the bridge but doesn't cancel
the pending work:
static void sn65dsi83_remove(struct i2c_client *client)
{
struct sn65dsi83 *ctx = i2c_get_clientdata(client);
drm_bridge_unplug(&ctx->bridge);
}
Because ctx is allocated via devres, the memory is freed immediately after
removal completes. If sn65dsi83_reset_work() is scheduled (e.g., from
hardware errors) and runs after this cleanup, it dereferences the freed
pointer:
static void sn65dsi83_reset_work(struct work_struct *ws)
{
struct sn65dsi83 *ctx = container_of(ws, struct sn65dsi83, reset_work);
...
if (!drm_bridge_enter(&ctx->bridge, &idx))
return;
Does this race condition lead to a use-after-free if the device is removed
while reset_work is pending?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260715-ti-sn65dsi83-fixes-v2-0-ebc4c3fe29b6@geanix.com?part=1
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-07-15 13:32 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-15 13:10 [PATCH v2 0/3] drm/bridge: ti-sn65dsi83: Various fixes Esben Haabendal
2026-07-15 13:10 ` [PATCH v2 1/3] drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking Esben Haabendal
2026-07-15 13:32 ` sashiko-bot
2026-07-15 13:10 ` [PATCH v2 2/3] drm/bridge: ti-sn65dsi83: Fix error handling in sn65dsi83_reset_work() Esben Haabendal
2026-07-15 13:21 ` sashiko-bot
2026-07-15 13:10 ` [PATCH v2 3/3] drm/bridge: ti-sn65dsi83: Support LVDS Channel B on SN65DSI84 Esben Haabendal
2026-07-15 13:18 ` sashiko-bot
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.