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 ed991ba..ed22a71 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -244,7 +244,7 @@
* 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 fad2b93..c0a07cb 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1006,6 +1006,8 @@
if (!crtc)
continue;
+ WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
+
funcs = crtc->helper_private;
if (!funcs || !funcs->atomic_begin)
@@ -1021,6 +1023,8 @@
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 e6c1691..3652ed8 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1152,12 +1152,12 @@
{
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 @@
plane->type);
out:
- drm_modeset_unlock_all(dev);
return ret;
}
@@ -2809,7 +2808,7 @@
* 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 @@
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 474e4d1..51cc47d 100644
--- a/drivers/gpu/drm/drm_modeset_lock.c
+++ b/drivers/gpu/drm/drm_modeset_lock.c
@@ -157,14 +157,20 @@
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 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 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.
+ *
+ * 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 @@
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 @@
}
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 @@
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 31bb1d7..5fae12c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8730,6 +8730,9 @@
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;
old->dpms_mode = connector->dpms;
old->load_detect_temp = false;
@@ -8767,6 +8770,9 @@
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);
to_intel_connector(connector)->new_encoder = intel_encoder;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 941a7bc..3725b52 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -252,7 +252,7 @@
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 @@
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;
}