[PATCH v7 01/34] drm/bridge-helper: Don't unlock the connection mutex after a backoff

[Maxime Ripard]

drm_bridge_helper_reset_crtc() takes the connection mutex using the
acquire context provided by the caller, and then relies on
drm_atomic_get_connector_for_encoder() and
drm_atomic_helper_reset_crtc() that both use the same context.

If either of those functions detects a deadlock, it backs off and drops
every lock held by the context, including the connection mutex taken by
drm_bridge_helper_reset_crtc() itself. The function however still
unlocked the connection mutex unconditionally on its way out, unlocking
a mutex it wasn't holding anymore.

The callers already have to release the context locks through
drm_modeset_drop_locks() and to handle the -EDEADLK error, so we can
leave the connection mutex release to them. We still need to take it
back after drm_atomic_get_connector_for_encoder() returns, since it may
have backed off, before looking at the connector state.

Fixes: 56ae62124177 ("drm/bridge: Add helper to reset bridge pipeline")
Signed-off-by: Maxime Ripard <mrip...@kernel.org>
---
 drivers/gpu/drm/drm_bridge_helper.c | 35 +++++++++++++++++++++++------------
 1 file changed, 23 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/drm_bridge_helper.c 
b/drivers/gpu/drm/drm_bridge_helper.c
index 0a3c8fee66b3..6998e7c41d30 100644
--- a/drivers/gpu/drm/drm_bridge_helper.c
+++ b/drivers/gpu/drm/drm_bridge_helper.c
@@ -20,4 +20,8 @@
  * apply.
  *
+ * All the locks taken by this function use @ctx: the caller is
+ * responsible for releasing them with drm_modeset_drop_locks(), and
+ * for handling an -EDEADLK error with drm_modeset_backoff().
+ *
  * Returns:
  *
@@ -34,4 +38,9 @@ int drm_bridge_helper_reset_crtc(struct drm_bridge *bridge,
        int ret;
 
+       /*
+        * Every lock taken with @ctx is released either by a backoff or
+        * by the caller through drm_modeset_drop_locks(), so we must not
+        * unlock the connection mutex ourselves.
+        */
        ret = drm_modeset_lock(&dev->mode_config.connection_mutex, ctx);
        if (ret)
@@ -39,20 +48,22 @@ int drm_bridge_helper_reset_crtc(struct drm_bridge *bridge,
 
        connector = drm_atomic_get_connector_for_encoder(encoder, ctx);
-       if (IS_ERR(connector)) {
-               ret = PTR_ERR(connector);
-               goto out;
-       }
+       if (IS_ERR(connector))
+               return PTR_ERR(connector);
 
-       if (!connector->state) {
-               ret = -EINVAL;
-               goto out;
-       }
+       /*
+        * drm_atomic_get_connector_for_encoder() may have backed off,
+        * releasing the connection mutex in the process. Take it back
+        * before looking at the connector state.
+        */
+       ret = drm_modeset_lock(&dev->mode_config.connection_mutex, ctx);
+       if (ret)
+               return ret;
+
+       if (!connector->state)
+               return -EINVAL;
 
        crtc = connector->state->crtc;
-       ret = drm_atomic_helper_reset_crtc(crtc, ctx);
 
-out:
-       drm_modeset_unlock(&dev->mode_config.connection_mutex);
-       return ret;
+       return drm_atomic_helper_reset_crtc(crtc, ctx);
 }
 EXPORT_SYMBOL(drm_bridge_helper_reset_crtc);

-- 
2.55.0