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
