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 some resources, thus this needs to be done in sn65dsi83_remove() after drm_bridge_unplug(). Signed-off-by: Luca Ceresoli <luca.ceres...@bootlin.com> --- drivers/gpu/drm/bridge/ti-sn65dsi83.c | 53 +++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c index 033c44326552ab167d4e8d9b74957c585e4c6fb7..9e4cecf4f7cb056f0c34e87007fcebf50780e49c 100644 --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c @@ -157,6 +157,7 @@ struct sn65dsi83 { struct drm_bridge *panel_bridge; struct gpio_desc *enable_gpio; struct regulator *vcc; + bool disable_resources_needed; bool lvds_dual_link; bool lvds_dual_link_even_odd_swap; int lvds_vod_swing_conf[2]; @@ -406,6 +407,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 +420,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 +452,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) @@ -478,10 +491,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); + drm_bridge_exit(idx); return; } @@ -625,6 +643,8 @@ 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); + ctx->disable_resources_needed = true; + drm_bridge_exit(idx); return; } @@ -633,6 +653,9 @@ static void sn65dsi83_atomic_pre_enable(struct drm_bridge *bridge, /* Wait for 10ms after soft reset as specified in datasheet */ usleep_range(10000, 12000); + + ctx->disable_resources_needed = true; + drm_bridge_exit(idx); } static void sn65dsi83_atomic_enable(struct drm_bridge *bridge, @@ -640,6 +663,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,6 +686,8 @@ 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, @@ -666,6 +695,10 @@ static void sn65dsi83_atomic_disable(struct drm_bridge *bridge, { struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge); int ret; + int idx; + + if (!drm_bridge_enter(bridge, &idx)) + return; if (ctx->irq) { /* Disable irq */ @@ -685,6 +718,9 @@ static void sn65dsi83_atomic_disable(struct drm_bridge *bridge, dev_err(ctx->dev, "Failed to disable vcc: %d\n", ret); regcache_mark_dirty(ctx->regmap); + + ctx->disable_resources_needed = false; + drm_bridge_exit(idx); } static enum drm_mode_status @@ -1005,7 +1041,24 @@ static void sn65dsi83_remove(struct i2c_client *client) { struct sn65dsi83 *ctx = i2c_get_clientdata(client); + drm_bridge_unplug(&ctx->bridge); drm_bridge_remove(&ctx->bridge); + + /* + * sn65dsi83_atomic_disable() should release some resources, but it + * cannot if we call drm_bridge_unplug() before it can + * drm_bridge_enter(). If that happens, let's release those + * resources now. + */ + if (ctx->disable_resources_needed) { + if (!ctx->irq) + sn65dsi83_monitor_stop(ctx); + + gpiod_set_value_cansleep(ctx->enable_gpio, 0); + usleep_range(10000, 11000); + + regulator_disable(ctx->vcc); + } } static const struct i2c_device_id sn65dsi83_id[] = { -- 2.50.1