Our current code now mixes some resources whose lifetime are tied to the device (clocks, IO mappings, etc.) and some that are tied to the DRM device (encoder, bridge).
The device one will be freed at unbind time, but the DRM one will only be freed when the last user of the DRM device closes its file handle.
So we end up with a time window during which we can call the encoder hooks, but we don't have access to the underlying resources and device.
Let's protect all those sections with drm_dev_enter() and drm_dev_exit() so that we bail out if we are during that window.
Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_txp.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c index 3f214b702c47..fee00b7003ab 100644 --- a/drivers/gpu/drm/vc4/vc4_txp.c +++ b/drivers/gpu/drm/vc4/vc4_txp.c @@ -15,6 +15,7 @@
#include <drm/drm_atomic.h> #include <drm/drm_atomic_helper.h> +#include <drm/drm_drv.h> #include <drm/drm_edid.h> #include <drm/drm_fb_cma_helper.h> #include <drm/drm_fourcc.h> @@ -274,6 +275,7 @@ static int vc4_txp_connector_atomic_check(struct drm_connector *conn, static void vc4_txp_connector_atomic_commit(struct drm_connector *conn, struct drm_atomic_state *state) { + struct drm_device *drm = conn->dev; struct drm_connector_state *conn_state = drm_atomic_get_new_connector_state(state, conn); struct vc4_txp *txp = connector_to_vc4_txp(conn); @@ -281,6 +283,7 @@ static void vc4_txp_connector_atomic_commit(struct drm_connector *conn, struct drm_display_mode *mode; struct drm_framebuffer *fb; u32 ctrl; + int idx; int i;
if (WARN_ON(!conn_state->writeback_job)) @@ -310,6 +313,9 @@ static void vc4_txp_connector_atomic_commit(struct drm_connector *conn, */ ctrl |= TXP_ALPHA_INVERT;
+ if (!drm_dev_enter(drm, &idx)) + return; + gem = drm_fb_cma_get_gem_obj(fb, 0); TXP_WRITE(TXP_DST_PTR, gem->paddr + fb->offsets[0]); TXP_WRITE(TXP_DST_PITCH, fb->pitches[0]); @@ -320,6 +326,8 @@ static void vc4_txp_connector_atomic_commit(struct drm_connector *conn, TXP_WRITE(TXP_DST_CTRL, ctrl);
drm_writeback_queue_job(&txp->connector, conn_state); + + drm_dev_exit(idx); }
static const struct drm_connector_helper_funcs vc4_txp_connector_helper_funcs = { @@ -345,7 +353,12 @@ static const struct drm_connector_funcs vc4_txp_connector_funcs = {
static void vc4_txp_encoder_disable(struct drm_encoder *encoder) { + struct drm_device *drm = encoder->dev; struct vc4_txp *txp = encoder_to_vc4_txp(encoder); + int idx; + + if (!drm_dev_enter(drm, &idx)) + return;
if (TXP_READ(TXP_DST_CTRL) & TXP_BUSY) { unsigned long timeout = jiffies + msecs_to_jiffies(1000); @@ -360,6 +373,8 @@ static void vc4_txp_encoder_disable(struct drm_encoder *encoder) }
TXP_WRITE(TXP_DST_CTRL, TXP_POWERDOWN); + + drm_dev_exit(idx); }
static const struct drm_encoder_helper_funcs vc4_txp_encoder_helper_funcs = { @@ -443,6 +458,16 @@ static irqreturn_t vc4_txp_interrupt(int irq, void *data) struct vc4_txp *txp = data; struct vc4_crtc *vc4_crtc = &txp->base;
+ /* + * We don't need to protect the register access using + * drm_dev_enter() there because the interrupt handler lifetime + * is tied to the device itself, and not to the DRM device. + * + * So when the device will be gone, one of the first thing we + * will be doing will be to unregister the interrupt handler, + * and then unregister the DRM device. drm_dev_enter() would + * thus always succeed if we are here. + */ TXP_WRITE(TXP_DST_CTRL, TXP_READ(TXP_DST_CTRL) & ~TXP_EI); vc4_crtc_handle_vblank(vc4_crtc); drm_writeback_signal_completion(&txp->connector, 0);