drm: Implement drm_modeset_lock_all_ctx()

This function is like drm_modeset_lock_all(), but it takes the lock
acquisition context as a parameter rather than storing it in the DRM
device's mode_config structure.

Implement drm_modeset_{,un}lock_all() in terms of the new function for
better code reuse, and add a note to the kerneldoc that new code should
use the new functions.

v2: improve kerneldoc
v4: rename drm_modeset_lock_all_crtcs() to drm_modeset_lock_all_ctx()
    and take mode_config's .connection_mutex instead of .mutex lock to
    avoid lock inversion (Daniel Vetter), use drm_modeset_drop_locks()
    which is now the equivalent of drm_modeset_unlock_all_ctx()
v5: do not take the dev->mode_config.connection_mutex in
    drm_atomic_legacy_backoff() since drm_modeset_lock_all_ctx()
    already keeps it, enhance kerneldoc for drm_modeset_lock_all_ctx()
    (Daniel Vetter)

Signed-off-by: Thierry Reding <treding@nvidia.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1449075005-13937-1-git-send-email-thierry.reding@gmail.com
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 55b4debad79b74766d9228635109fdc50fa7230f..ef5f7663a718260769d6c8c98c6c644b5f1bc03d 100644 (file)
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1188,12 +1188,7 @@ void drm_atomic_legacy_backoff(struct drm_atomic_state *state)
 retry:
        drm_modeset_backoff(state->acquire_ctx);
 
-       ret = drm_modeset_lock(&state->dev->mode_config.connection_mutex,
-                              state->acquire_ctx);
-       if (ret)
-               goto retry;
-       ret = drm_modeset_lock_all_crtcs(state->dev,
-                                        state->acquire_ctx);
+       ret = drm_modeset_lock_all_ctx(state->dev, state->acquire_ctx);
        if (ret)
                goto retry;
 }

diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c
index 6675b14284105cd639fb7fbb7ee9967cfcc38aa9..c2f5971146ba7134d687f80563deaefb46200606 100644 (file)
--- a/drivers/gpu/drm/drm_modeset_lock.c
+++ b/drivers/gpu/drm/drm_modeset_lock.c
@@ -57,11 +57,18 @@
 
 /**
  * drm_modeset_lock_all - take all modeset locks
- * @dev: drm device
+ * @dev: DRM device
  *
  * This function takes all modeset locks, suitable where a more fine-grained
- * scheme isn't (yet) implemented. Locks must be dropped with
- * drm_modeset_unlock_all.
+ * scheme isn't (yet) implemented. Locks must be dropped by calling the
+ * drm_modeset_unlock_all() function.
+ *
+ * This function is deprecated. It allocates a lock acquisition context and
+ * stores it in the DRM device's ->mode_config. This facilitate conversion of
+ * existing code because it removes the need to manually deal with the
+ * acquisition context, but it is also brittle because the context is global
+ * and care must be taken not to nest calls. New code should use the
+ * drm_modeset_lock_all_ctx() function and pass in the context explicitly.
  */
 void drm_modeset_lock_all(struct drm_device *dev)
 {
@@ -78,39 +85,43 @@ void drm_modeset_lock_all(struct drm_device *dev)
        drm_modeset_acquire_init(ctx, 0);
 
 retry:
-       ret = drm_modeset_lock(&config->connection_mutex, ctx);
-       if (ret)
-               goto fail;
-       ret = drm_modeset_lock_all_crtcs(dev, ctx);
-       if (ret)
-               goto fail;
+       ret = drm_modeset_lock_all_ctx(dev, ctx);
+       if (ret < 0) {
+               if (ret == -EDEADLK) {
+                       drm_modeset_backoff(ctx);
+                       goto retry;
+               }
+
+               drm_modeset_acquire_fini(ctx);
+               kfree(ctx);
+               return;
+       }
 
        WARN_ON(config->acquire_ctx);
 
-       /* now we hold the locks, so now that it is safe, stash the
-        * ctx for drm_modeset_unlock_all():
+       /*
+        * We hold the locks now, so it is safe to stash the acquisition
+        * context for drm_modeset_unlock_all().
         */
        config->acquire_ctx = ctx;
 
        drm_warn_on_modeset_not_all_locked(dev);
-
-       return;
-
-fail:
-       if (ret == -EDEADLK) {
-               drm_modeset_backoff(ctx);
-               goto retry;
-       }
-
-       kfree(ctx);
 }
 EXPORT_SYMBOL(drm_modeset_lock_all);
 
 /**
  * drm_modeset_unlock_all - drop all modeset locks
- * @dev: device
+ * @dev: DRM device
  *
- * This function drop all modeset locks taken by drm_modeset_lock_all.
+ * This function drops all modeset locks taken by a previous call to the
+ * drm_modeset_lock_all() function.
+ *
+ * This function is deprecated. It uses the lock acquisition context stored
+ * in the DRM device's ->mode_config. This facilitates conversion of existing
+ * code because it removes the need to manually deal with the acquisition
+ * context, but it is also brittle because the context is global and care must
+ * be taken not to nest calls. New code should pass the acquisition context
+ * directly to the drm_modeset_drop_locks() function.
  */
 void drm_modeset_unlock_all(struct drm_device *dev)
 {
@@ -431,14 +442,34 @@ void drm_modeset_unlock(struct drm_modeset_lock *lock)
 }
 EXPORT_SYMBOL(drm_modeset_unlock);
 
-/* In some legacy codepaths it's convenient to just grab all the crtc and plane
- * related locks. */
-int drm_modeset_lock_all_crtcs(struct drm_device *dev,
-               struct drm_modeset_acquire_ctx *ctx)
+/**
+ * drm_modeset_lock_all_ctx - take all modeset locks
+ * @dev: DRM device
+ * @ctx: lock acquisition context
+ *
+ * This function takes all modeset locks, suitable where a more fine-grained
+ * scheme isn't (yet) implemented.
+ *
+ * Unlike drm_modeset_lock_all(), it doesn't take the dev->mode_config.mutex
+ * since that lock isn't required for modeset state changes. Callers which
+ * need to grab that lock too need to do so outside of the acquire context
+ * @ctx.
+ *
+ * Locks acquired with this function should be released by calling the
+ * drm_modeset_drop_locks() function on @ctx.
+ *
+ * Returns: 0 on success or a negative error-code on failure.
+ */
+int drm_modeset_lock_all_ctx(struct drm_device *dev,
+                            struct drm_modeset_acquire_ctx *ctx)
 {
        struct drm_crtc *crtc;
        struct drm_plane *plane;
-       int ret = 0;
+       int ret;
+
+       ret = drm_modeset_lock(&dev->mode_config.connection_mutex, ctx);
+       if (ret)
+               return ret;
 
        drm_for_each_crtc(crtc, dev) {
                ret = drm_modeset_lock(&crtc->mutex, ctx);
@@ -454,4 +485,4 @@ int drm_modeset_lock_all_crtcs(struct drm_device *dev,
 
        return 0;
 }
-EXPORT_SYMBOL(drm_modeset_lock_all_crtcs);
+EXPORT_SYMBOL(drm_modeset_lock_all_ctx);

diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h
index 94938d89347cf2f8860f617f28ab68d5134bacce..c5576fbcb909f925a241a4f1ff9736bc8c8ad119 100644 (file)
--- a/include/drm/drm_modeset_lock.h
+++ b/include/drm/drm_modeset_lock.h
@@ -138,7 +138,7 @@ void drm_warn_on_modeset_not_all_locked(struct drm_device *dev);
 struct drm_modeset_acquire_ctx *
 drm_modeset_legacy_acquire_ctx(struct drm_crtc *crtc);
 
-int drm_modeset_lock_all_crtcs(struct drm_device *dev,
-               struct drm_modeset_acquire_ctx *ctx);
+int drm_modeset_lock_all_ctx(struct drm_device *dev,
+                            struct drm_modeset_acquire_ctx *ctx);
 
 #endif /* DRM_MODESET_LOCK_H_ */