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 <mripard@kernel.org>
Link: https://patch.msgid.link/20251112-drm-bridge-atomic-vs-remove-v3-2-85db717ce094@bootlin.com
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
This commit is contained in:
Luca Ceresoli 2025-11-12 17:34:35 +01:00
parent d36137085a
commit d2e8d1bc84

View File

@ -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);
}
if (!drm_bridge_enter(bridge, &idx))
return;
/* 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);
devm_release_action(ctx->dev, sn65dsi83_release_resources, ctx);
ret = regulator_disable(ctx->vcc);
if (ret)
dev_err(ctx->dev, "Failed to disable vcc: %d\n", ret);
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[] = {