Revert "drm/vkms: Fix race-condition between the hrtimer and the atomic commit"

Commit Message

Maíra Canal Sept. 14, 2023, 10:19 a.m. UTC

This reverts commit a0e6a017ab56936c0405fe914a793b241ed25ee0.

Unlocking a mutex in the context of a hrtimer callback is violating mutex
locking rules, as mutex_unlock() from interrupt context is not permitted.

Links: https://lore.kernel.org/dri-devel/[e-mail]/T/#t
Acked-by: Daniel Vetter <[e-mail]>
Signed-off-by: Maíra Canal <[e-mail]>
---
 drivers/gpu/drm/vkms/vkms_composer.c | 9 ++-------
 drivers/gpu/drm/vkms/vkms_crtc.c     | 9 ++++-----
 drivers/gpu/drm/vkms/vkms_drv.h      | 4 +---
 3 files changed, 7 insertions(+), 15 deletions(-)

--
2.41.0

Patch

diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
index d5d4f642d367..3c99fb8b54e2 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -408,15 +408,10 @@  void vkms_set_composer(struct vkms_output *out, bool enabled)
 	if (enabled)
 		drm_crtc_vblank_get(&out->crtc);

-	mutex_lock(&out->enabled_lock);
+	spin_lock_irq(&out->lock);
 	old_enabled = out->composer_enabled;
 	out->composer_enabled = enabled;
-
-	/* the composition wasn't enabled, so unlock the lock to make sure the lock
-	 * will be balanced even if we have a failed commit
-	 */
-	if (!out->composer_enabled)
-		mutex_unlock(&out->enabled_lock);
+	spin_unlock_irq(&out->lock);

 	if (old_enabled)
 		drm_crtc_vblank_put(&out->crtc);
diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index 3c5ebf106b66..61e500b8c9da 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -16,7 +16,7 @@  static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer)
 	struct drm_crtc *crtc = &output->crtc;
 	struct vkms_crtc_state *state;
 	u64 ret_overrun;
-	bool ret, fence_cookie, composer_enabled;
+	bool ret, fence_cookie;

 	fence_cookie = dma_fence_begin_signalling();

@@ -25,15 +25,15 @@  static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer)
 	if (ret_overrun != 1)
 		pr_warn("%s: vblank timer overrun\n", __func__);

+	spin_lock(&output->lock);
 	ret = drm_crtc_handle_vblank(crtc);
 	if (!ret)
 		DRM_ERROR("vkms failure on handling vblank");

 	state = output->composer_state;
-	composer_enabled = output->composer_enabled;
-	mutex_unlock(&output->enabled_lock);
+	spin_unlock(&output->lock);

-	if (state && composer_enabled) {
+	if (state && output->composer_enabled) {
 		u64 frame = drm_crtc_accurate_vblank_count(crtc);

 		/* update frame_start only if a queued vkms_composer_worker()
@@ -295,7 +295,6 @@  int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,

 	spin_lock_init(&vkms_out->lock);
 	spin_lock_init(&vkms_out->composer_lock);
-	mutex_init(&vkms_out->enabled_lock);

 	vkms_out->composer_workq = alloc_ordered_workqueue("vkms_composer", 0);
 	if (!vkms_out->composer_workq)
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index c7ae6c2ba1df..8f5710debb1e 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -108,10 +108,8 @@  struct vkms_output {
 	struct workqueue_struct *composer_workq;
 	/* protects concurrent access to composer */
 	spinlock_t lock;
-	/* guarantees that if the composer is enabled, a job will be queued */
-	struct mutex enabled_lock;

-	/* protected by @enabled_lock */
+	/* protected by @lock */
 	bool composer_enabled;
 	struct vkms_crtc_state *composer_state;