From d2e8d1bc840b849fc23d8812995645cc79990e7b Mon Sep 17 00:00:00 2001 From: Luca Ceresoli Date: Wed, 12 Nov 2025 17:34:35 +0100 Subject: [PATCH] drm/bridge: ti-sn65dsi83: protect device resources on unplug To support hot-unplug of this bridge we need to protect access to device resources in case sn65dsi83_remove() happens concurrently to other code. Some care is needed for the case when the unplug happens before sn65dsi83_atomic_disable() has a chance to enter the critical section (i.e. a successful drm_bridge_enter() call), which occurs whenever the hardware is removed while the display is active. When that happens, sn65dsi83_atomic_disable() in unable to release the resources taken by sn65dsi83_atomic_pre_enable(). To ensure those resources are released exactly once on device removal: * move the code to release them to a dedicated function * register that function when the resources are taken in sn65dsi83_atomic_pre_enable() * if sn65dsi83_atomic_disable() happens before sn65dsi83_remove() (typical non-hot-unplug case): * sn65dsi83_atomic_disable() can enter the critical section (drm_bridge_enter() returns 0) -> it releases and executes the devres action * if sn65dsi83_atomic_disable() happens after sn65dsi83_remove() (typical hot-unplug case): * sn65dsi83_remove() -> drm_bridge_unplug() prevents sn65dsi83_atomic_disable() from entering the critical section (drm_bridge_enter() returns nonzero), so sn65dsi83_atomic_disable() cannot release and execute the devres action * the devres action is executed at the end of sn65dsi83_remove() Reviewed-by: Maxime Ripard Link: https://patch.msgid.link/20251112-drm-bridge-atomic-vs-remove-v3-2-85db717ce094@bootlin.com Signed-off-by: Luca Ceresoli --- drivers/gpu/drm/bridge/ti-sn65dsi83.c | 86 ++++++++++++++++++++------- 1 file changed, 66 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c index 033c44326552a..ac74b9e85b976 100644 --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c @@ -406,6 +406,10 @@ static void sn65dsi83_reset_work(struct work_struct *ws) { struct sn65dsi83 *ctx = container_of(ws, struct sn65dsi83, reset_work); int ret; + int idx; + + if (!drm_bridge_enter(&ctx->bridge, &idx)) + return; /* Reset the pipe */ ret = sn65dsi83_reset_pipe(ctx); @@ -415,12 +419,18 @@ static void sn65dsi83_reset_work(struct work_struct *ws) } if (ctx->irq) enable_irq(ctx->irq); + + drm_bridge_exit(idx); } static void sn65dsi83_handle_errors(struct sn65dsi83 *ctx) { unsigned int irq_stat; int ret; + int idx; + + if (!drm_bridge_enter(&ctx->bridge, &idx)) + return; /* * Schedule a reset in case of: @@ -441,6 +451,8 @@ static void sn65dsi83_handle_errors(struct sn65dsi83 *ctx) schedule_work(&ctx->reset_work); } + + drm_bridge_exit(idx); } static void sn65dsi83_monitor_work(struct work_struct *work) @@ -463,6 +475,37 @@ static void sn65dsi83_monitor_stop(struct sn65dsi83 *ctx) cancel_delayed_work_sync(&ctx->monitor_work); } +/* + * Release resources taken by sn65dsi83_atomic_pre_enable(). + * + * Invoked by sn65dsi83_atomic_disable() normally, or by devres after + * sn65dsi83_remove() in case this happens befora atomic_disable. + */ +static void sn65dsi83_release_resources(void *data) +{ + struct sn65dsi83 *ctx = (struct sn65dsi83 *)data; + int ret; + + if (ctx->irq) { + /* Disable irq */ + regmap_write(ctx->regmap, REG_IRQ_EN, 0x0); + regmap_write(ctx->regmap, REG_IRQ_GLOBAL, 0x0); + } else { + /* Stop the polling task */ + sn65dsi83_monitor_stop(ctx); + } + + /* Put the chip in reset, pull EN line low, and assure 10ms reset low timing. */ + gpiod_set_value_cansleep(ctx->enable_gpio, 0); + usleep_range(10000, 11000); + + ret = regulator_disable(ctx->vcc); + if (ret) + dev_err(ctx->dev, "Failed to disable vcc: %d\n", ret); + + regcache_mark_dirty(ctx->regmap); +} + static void sn65dsi83_atomic_pre_enable(struct drm_bridge *bridge, struct drm_atomic_state *state) { @@ -478,11 +521,15 @@ static void sn65dsi83_atomic_pre_enable(struct drm_bridge *bridge, __le16 le16val; u16 val; int ret; + int idx; + + if (!drm_bridge_enter(bridge, &idx)) + return; ret = regulator_enable(ctx->vcc); if (ret) { dev_err(ctx->dev, "Failed to enable vcc: %d\n", ret); - return; + goto err_exit; } /* Deassert reset */ @@ -625,7 +672,7 @@ static void sn65dsi83_atomic_pre_enable(struct drm_bridge *bridge, 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; + goto err_add_action; } /* Trigger reset after CSR register update. */ @@ -633,6 +680,11 @@ static void sn65dsi83_atomic_pre_enable(struct drm_bridge *bridge, /* 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); } static void sn65dsi83_atomic_enable(struct drm_bridge *bridge, @@ -640,6 +692,10 @@ static void sn65dsi83_atomic_enable(struct drm_bridge *bridge, { struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge); unsigned int pval; + int idx; + + if (!drm_bridge_enter(bridge, &idx)) + return; /* Clear all errors that got asserted during initialization. */ regmap_read(ctx->regmap, REG_IRQ_STAT, &pval); @@ -659,32 +715,22 @@ static void sn65dsi83_atomic_enable(struct drm_bridge *bridge, /* Use the polling task */ sn65dsi83_monitor_start(ctx); } + + drm_bridge_exit(idx); } static void sn65dsi83_atomic_disable(struct drm_bridge *bridge, struct drm_atomic_state *state) { struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge); - int ret; + int idx; - if (ctx->irq) { - /* Disable irq */ - regmap_write(ctx->regmap, REG_IRQ_EN, 0x0); - regmap_write(ctx->regmap, REG_IRQ_GLOBAL, 0x0); - } else { - /* Stop the polling task */ - sn65dsi83_monitor_stop(ctx); - } - - /* Put the chip in reset, pull EN line low, and assure 10ms reset low timing. */ - gpiod_set_value_cansleep(ctx->enable_gpio, 0); - usleep_range(10000, 11000); + if (!drm_bridge_enter(bridge, &idx)) + return; - ret = regulator_disable(ctx->vcc); - if (ret) - dev_err(ctx->dev, "Failed to disable vcc: %d\n", ret); + devm_release_action(ctx->dev, sn65dsi83_release_resources, ctx); - regcache_mark_dirty(ctx->regmap); + drm_bridge_exit(idx); } static enum drm_mode_status @@ -1005,7 +1051,7 @@ static void sn65dsi83_remove(struct i2c_client *client) { struct sn65dsi83 *ctx = i2c_get_clientdata(client); - drm_bridge_remove(&ctx->bridge); + drm_bridge_unplug(&ctx->bridge); } static const struct i2c_device_id sn65dsi83_id[] = { -- 2.47.3