Revert "drm/vkms: Fix race-condition between the hrtimer and the atomic commit"
authorMaíra Canal <mcanal@igalia.com>
Thu, 14 Sep 2023 10:19:02 +0000 (07:19 -0300)
committerMaíra Canal <mairacanal@riseup.net>
Thu, 14 Sep 2023 10:48:19 +0000 (07:48 -0300)
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.

Link: https://lore.kernel.org/dri-devel/ZQLAc%2FFwkv%2FGiVoK@phenom.ffwll.local/T/#t
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
Signed-off-by: Maíra Canal <mairacanal@riseup.net>
Link: https://patchwork.freedesktop.org/patch/msgid/20230914102024.1789154-1-mcanal@igalia.com
drivers/gpu/drm/vkms/vkms_composer.c
drivers/gpu/drm/vkms/vkms_crtc.c
drivers/gpu/drm/vkms/vkms_drv.h

index d5d4f64..3c99fb8 100644 (file)
@@ -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);
index 3c5ebf1..61e500b 100644 (file)
@@ -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)
index c7ae6c2..8f5710d 100644 (file)
@@ -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;