[Intel-gfx] [RFC][PATCH] drm: Insane but more fine grained locking for planes

[ville.syrjala at linux.intel.com]

From: Ville Syrjälä <ville.syrjala at linux.intel.com>

Instead of locking all modeset locks during plane updates, use just
a single CRTC mutex. To make that work, track the CRTC that "owns"
the plane currently. During enable/update that means the new
CRTC, and during disable it means the old CRTC.

Since the plane state is no longer protected by a single lock, we
need to sprinkle some additional memory barriers when relinquishing
ownership. Otherwise the next CRTC might observe some stale state
even though the crtc_mutex already got updated.
drm_framebuffer_remove() doesn't need extra barriers since it
already holds all CRTC locks, and thus no-one can be poking around
at the same time. On the read side cmpxchg() already should have
the necessary memory barriers.

This design implies that before a plane can be moved to another CRTC,
it must be disabled first, even if the hardware would offer some kind
of mechanism to move an active plane over directly. I believe everyone
has agreed that this an acceptable compromise.

Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
---
 drivers/gpu/drm/drm_crtc.c | 43 +++++++++++++++++++++++++++++++++++++++----
 include/drm/drm_crtc.h     |  3 +++
 2 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 957fb70..6f7385e 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -576,6 +576,7 @@ void drm_framebuffer_remove(struct drm_framebuffer *fb)
 				__drm_framebuffer_unreference(plane->fb);
 				plane->fb = NULL;
 				plane->crtc = NULL;
+				plane->crtc_mutex = NULL;
 			}
 		}
 		drm_modeset_unlock_all(dev);
@@ -1785,6 +1786,7 @@ int drm_mode_setplane(struct drm_device *dev, void *data,
 	int ret = 0;
 	unsigned int fb_width, fb_height;
 	int i;
+	struct mutex *old_crtc_mutex;
 
 	if (!drm_core_check_feature(dev, DRIVER_MODESET))
 		return -EINVAL;
@@ -1804,12 +1806,33 @@ int drm_mode_setplane(struct drm_device *dev, void *data,
 
 	/* No fb means shut it down */
 	if (!plane_req->fb_id) {
-		drm_modeset_lock_all(dev);
+		struct mutex *crtc_mutex;
+
+	retry:
+		crtc_mutex = ACCESS_ONCE(plane->crtc_mutex);
+
+		/* plane was already disabled? */
+		if (!crtc_mutex)
+			return 0;
+
+		mutex_lock(crtc_mutex);
+
+		/* re-check that plane is still on the same crtc... */
+		if (crtc_mutex != plane->crtc_mutex) {
+			mutex_unlock(crtc_mutex);
+			goto retry;
+		}
+
 		old_fb = plane->fb;
 		plane->funcs->disable_plane(plane);
 		plane->crtc = NULL;
 		plane->fb = NULL;
-		drm_modeset_unlock_all(dev);
+
+		smp_wmb();
+		plane->crtc_mutex = NULL;
+
+		mutex_unlock(crtc_mutex);
+
 		goto out;
 	}
 
@@ -1875,7 +1898,15 @@ int drm_mode_setplane(struct drm_device *dev, void *data,
 		goto out;
 	}
 
-	drm_modeset_lock_all(dev);
+	mutex_lock(&crtc->mutex);
+
+	old_crtc_mutex = cmpxchg(&plane->crtc_mutex, NULL, &crtc->mutex);
+	if (old_crtc_mutex != NULL && old_crtc_mutex != &crtc->mutex) {
+		mutex_unlock(&crtc->mutex);
+		ret = -EBUSY;
+		goto out;
+	}
+
 	ret = plane->funcs->update_plane(plane, crtc, fb,
 					 plane_req->crtc_x, plane_req->crtc_y,
 					 plane_req->crtc_w, plane_req->crtc_h,
@@ -1886,8 +1917,12 @@ int drm_mode_setplane(struct drm_device *dev, void *data,
 		plane->crtc = crtc;
 		plane->fb = fb;
 		fb = NULL;
+	} else {
+		smp_wmb();
+		plane->crtc_mutex = old_crtc_mutex;
 	}
-	drm_modeset_unlock_all(dev);
+
+	mutex_unlock(&crtc->mutex);
 
 out:
 	if (fb)
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 8c7846b..cc3779f 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -651,6 +651,7 @@ struct drm_plane_funcs {
  * @dev: DRM device this plane belongs to
  * @head: for list management
  * @base: base mode object
+ * @crtc_mutex: points to the mutex of the current "owner" CRTC
  * @possible_crtcs: pipes this plane can be bound to
  * @format_types: array of formats supported by this plane
  * @format_count: number of formats supported
@@ -669,6 +670,8 @@ struct drm_plane {
 
 	struct drm_mode_object base;
 
+	struct mutex *crtc_mutex;
+
 	uint32_t possible_crtcs;
 	uint32_t *format_types;
 	uint32_t format_count;
-- 
1.8.1.5