drm: Per-plane locking

Turned out to be much simpler on top of my latest atomic stuff than
what I've feared. Some details:

- Drop the modeset_lock_all snakeoil in drm_plane_init. Same
  justification as for the equivalent change in drm_crtc_init done in

	commit d0fa1af40e784aaf7ebb7ba8a17b229bb3fa4c21
	Author: Daniel Vetter <daniel.vetter@ffwll.ch>
	Date:   Mon Sep 8 09:02:49 2014 +0200

	    drm: Drop modeset locking from crtc init function

  Without these the drm_modeset_lock_init would fall over the exact
  same way.

- Since the atomic core code wraps the locking switching it to
  per-plane locks was a one-line change.

- For the legacy ioctls add a plane argument to the locking helper so
  that we can grab the right plane lock (cursor or primary). Since the
  universal cursor plane might not be there, or someone really crazy
  might forgoe the primary plane even accept NULL.

- Add some locking WARN_ON to the atomic helpers for good paranoid
  measure and to check that it all works out.

Tested on my exynos atomic hackfest with full lockdep checks and ww
backoff injection.

v2: I've forgotten about the load-detect code in i915.

v3: Thierry reported that in latest 3.18-rc vmwgfx doesn't compile any
more due to

commit 21e88620aa21b48d4f62d29275e3e2944a5ea2b5
Author: Rob Clark <robdclark@gmail.com>
Date:   Thu Oct 30 13:39:04 2014 -0400

    drm/vmwgfx: fix lock breakage

Rebased and fix this up.

Cc: Thierry Reding <thierry.reding@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Reviewed-by: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Dave Airlie <airlied@redhat.com>

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index ed991ba66e21932c66ff828334dfba1bb343865a..ed22a719440f78d5a082287e955bb0782dee8cf8 100644 (file)
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -244,7 +244,7 @@ drm_atomic_get_plane_state(struct drm_atomic_state *state,
         * grab all crtc locks. Once we have per-plane locks we must update this
         * to only take the plane mutex.
         */
-       ret = drm_modeset_lock_all_crtcs(state->dev, state->acquire_ctx);
+       ret = drm_modeset_lock(&plane->mutex, state->acquire_ctx);
        if (ret)
                return ERR_PTR(ret);
 

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index fad2b932cf7243158fa257deb25bc0e63b26c18a..c0a07cba3aaaa24c0247d0433920a69d83e2d7b5 100644 (file)
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1006,6 +1006,8 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev,
                if (!crtc)
                        continue;
 
+               WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
+
                funcs = crtc->helper_private;
 
                if (!funcs || !funcs->atomic_begin)
@@ -1021,6 +1023,8 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev,
                if (!plane)
                        continue;
 
+               WARN_ON(!drm_modeset_is_locked(&plane->mutex));
+
                funcs = plane->helper_private;
 
                if (!funcs || !funcs->atomic_update)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index e6c169152bf1f57e38d75a716dac3816950f72eb..3652ed8dda80928ca9331c0ae03e85206b42e54f 100644 (file)
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1152,12 +1152,12 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
 {
        int ret;
 
-       drm_modeset_lock_all(dev);
-
        ret = drm_mode_object_get(dev, &plane->base, DRM_MODE_OBJECT_PLANE);
        if (ret)
                goto out;
 
+       drm_modeset_lock_init(&plane->mutex);
+
        plane->base.properties = &plane->properties;
        plane->dev = dev;
        plane->funcs = funcs;
@@ -1185,7 +1185,6 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
                                   plane->type);
 
  out:
-       drm_modeset_unlock_all(dev);
 
        return ret;
 }
@@ -2809,7 +2808,7 @@ static int drm_mode_cursor_common(struct drm_device *dev,
         * If this crtc has a universal cursor plane, call that plane's update
         * handler rather than using legacy cursor handlers.
         */
-       drm_modeset_lock_crtc(crtc);
+       drm_modeset_lock_crtc(crtc, crtc->cursor);
        if (crtc->cursor) {
                ret = drm_mode_cursor_universal(crtc, req, file_priv);
                goto out;
@@ -4598,7 +4597,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
        if (!crtc)
                return -ENOENT;
 
-       drm_modeset_lock_crtc(crtc);
+       drm_modeset_lock_crtc(crtc, crtc->primary);
        if (crtc->primary->fb == NULL) {
                /* The framebuffer is currently unbound, presumably
                 * due to a hotplug event, that userspace has not

diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c
index 474e4d12a2d8a73109a35fe3eb3b9948635fedaa..51cc47d827d82f09f1ed9d0e4200e299b4fa628c 100644 (file)
--- a/drivers/gpu/drm/drm_modeset_lock.c
+++ b/drivers/gpu/drm/drm_modeset_lock.c
@@ -157,14 +157,20 @@ void drm_modeset_unlock_all(struct drm_device *dev)
 EXPORT_SYMBOL(drm_modeset_unlock_all);
 
 /**
- * drm_modeset_lock_crtc - lock crtc with hidden acquire ctx
- * @crtc: drm crtc
+ * drm_modeset_lock_crtc - lock crtc with hidden acquire ctx for a plane update
+ * @crtc: DRM CRTC
+ * @plane: DRM plane to be updated on @crtc
+ *
+ * This function locks the given crtc and plane (which should be either the
+ * primary or cursor plane) using a hidden acquire context. This is necessary so
+ * that drivers internally using the atomic interfaces can grab further locks
+ * with the lock acquire context.
  *
- * This function locks the given crtc using a hidden acquire context. This is
- * necessary so that drivers internally using the atomic interfaces can grab
- * further locks with the lock acquire context.
+ * Note that @plane can be NULL, e.g. when the cursor support hasn't yet been
+ * converted to universal planes yet.
  */
-void drm_modeset_lock_crtc(struct drm_crtc *crtc)
+void drm_modeset_lock_crtc(struct drm_crtc *crtc,
+                          struct drm_plane *plane)
 {
        struct drm_modeset_acquire_ctx *ctx;
        int ret;
@@ -180,6 +186,18 @@ retry:
        if (ret)
                goto fail;
 
+       if (plane) {
+               ret = drm_modeset_lock(&plane->mutex, ctx);
+               if (ret)
+                       goto fail;
+
+               if (plane->crtc) {
+                       ret = drm_modeset_lock(&plane->crtc->mutex, ctx);
+                       if (ret)
+                               goto fail;
+               }
+       }
+
        WARN_ON(crtc->acquire_ctx);
 
        /* now we hold the locks, so now that it is safe, stash the
@@ -437,15 +455,14 @@ void drm_modeset_unlock(struct drm_modeset_lock *lock)
 }
 EXPORT_SYMBOL(drm_modeset_unlock);
 
-/* Temporary.. until we have sufficiently fine grained locking, there
- * are a couple scenarios where it is convenient to grab all crtc locks.
- * It is planned to remove this:
- */
+/* 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)
 {
        struct drm_mode_config *config = &dev->mode_config;
        struct drm_crtc *crtc;
+       struct drm_plane *plane;
        int ret = 0;
 
        list_for_each_entry(crtc, &config->crtc_list, head) {
@@ -454,6 +471,12 @@ int drm_modeset_lock_all_crtcs(struct drm_device *dev,
                        return ret;
        }
 
+       list_for_each_entry(plane, &config->plane_list, head) {
+               ret = drm_modeset_lock(&plane->mutex, ctx);
+               if (ret)
+                       return ret;
+       }
+
        return 0;
 }
 EXPORT_SYMBOL(drm_modeset_lock_all_crtcs);

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 31bb1d7acf9ba53ba279e024f97761962fe4d37b..5fae12cf10728f70bee5f8b2319e0b4f05472859 100644 (file)
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8728,6 +8728,9 @@ retry:
                crtc = encoder->crtc;
 
                ret = drm_modeset_lock(&crtc->mutex, ctx);
+               if (ret)
+                       goto fail_unlock;
+               ret = drm_modeset_lock(&crtc->primary->mutex, ctx);
                if (ret)
                        goto fail_unlock;
 
@@ -8765,6 +8768,9 @@ retry:
        }
 
        ret = drm_modeset_lock(&crtc->mutex, ctx);
+       if (ret)
+               goto fail_unlock;
+       ret = drm_modeset_lock(&crtc->primary->mutex, ctx);
        if (ret)
                goto fail_unlock;
        intel_encoder->new_crtc = to_intel_crtc(crtc);

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 941a7bc0b79190b7bcafe751cd7cac26b371911e..3725b521d9319c9b952bf3920bfcd3a27c61dac3 100644 (file)
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -252,7 +252,7 @@ int vmw_du_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv,
        ret = 0;
 out:
        drm_modeset_unlock_all(dev_priv->dev);
-       drm_modeset_lock_crtc(crtc);
+       drm_modeset_lock_crtc(crtc, crtc->cursor);
 
        return ret;
 }
@@ -281,7 +281,7 @@ int vmw_du_crtc_cursor_move(struct drm_crtc *crtc, int x, int y)
                                   du->cursor_y + du->hotspot_y);
 
        drm_modeset_unlock_all(dev_priv->dev);
-       drm_modeset_lock_crtc(crtc);
+       drm_modeset_lock_crtc(crtc, crtc->cursor);
 
        return 0;
 }

diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index bc1cc3ce05c44fef38c6bfb0802dd08c6c592fa3..cbb475654b1c0d62b90dee87ff357489661660d8 100644 (file)
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -751,6 +751,8 @@ struct drm_plane {
        struct drm_device *dev;
        struct list_head head;
 
+       struct drm_modeset_lock mutex;
+
        struct drm_mode_object base;
 
        uint32_t possible_crtcs;

diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h
index 28931a23d96cd9ea1947f9b881e50cdb5e29a86b..70595ff565baa45190faaae5c42630fe04a1beff 100644 (file)
--- a/include/drm/drm_modeset_lock.h
+++ b/include/drm/drm_modeset_lock.h
@@ -127,11 +127,13 @@ void drm_modeset_unlock(struct drm_modeset_lock *lock);
 
 struct drm_device;
 struct drm_crtc;
+struct drm_plane;
 
 void drm_modeset_lock_all(struct drm_device *dev);
 int __drm_modeset_lock_all(struct drm_device *dev, bool trylock);
 void drm_modeset_unlock_all(struct drm_device *dev);
-void drm_modeset_lock_crtc(struct drm_crtc *crtc);
+void drm_modeset_lock_crtc(struct drm_crtc *crtc,
+                          struct drm_plane *plane);
 void drm_modeset_unlock_crtc(struct drm_crtc *crtc);
 void drm_warn_on_modeset_not_all_locked(struct drm_device *dev);
 struct drm_modeset_acquire_ctx *