drm: rcar-du: Fix race condition when disabling planes at CRTC stop

When stopping the CRTC the driver must disable all planes and wait for
the change to take effect at the next vblank. Merely calling
drm_crtc_wait_one_vblank() is not enough, as the function doesn't
include any mechanism to handle the race with vblank interrupts.

Replace the drm_crtc_wait_one_vblank() call with a manual mechanism that
handles the vblank interrupt race.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index 17fd1cd..6e5bd0b 100644 (file)
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -490,23 +490,51 @@ static void rcar_du_crtc_start(struct rcar_du_crtc *rcrtc)
        rcar_du_group_start_stop(rcrtc->group, true);
 }
 
+static void rcar_du_crtc_disable_planes(struct rcar_du_crtc *rcrtc)
+{
+       struct rcar_du_device *rcdu = rcrtc->group->dev;
+       struct drm_crtc *crtc = &rcrtc->crtc;
+       u32 status;
+
+       /* Make sure vblank interrupts are enabled. */
+       drm_crtc_vblank_get(crtc);
+
+       /*
+        * Disable planes and calculate how many vertical blanking interrupts we
+        * have to wait for. If a vertical blanking interrupt has been triggered
+        * but not processed yet, we don't know whether it occurred before or
+        * after the planes got disabled. We thus have to wait for two vblank
+        * interrupts in that case.
+        */
+       spin_lock_irq(&rcrtc->vblank_lock);
+       rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? DS2PR : DS1PR, 0);
+       status = rcar_du_crtc_read(rcrtc, DSSR);
+       rcrtc->vblank_count = status & DSSR_VBK ? 2 : 1;
+       spin_unlock_irq(&rcrtc->vblank_lock);
+
+       if (!wait_event_timeout(rcrtc->vblank_wait, rcrtc->vblank_count == 0,
+                               msecs_to_jiffies(100)))
+               dev_warn(rcdu->dev, "vertical blanking timeout\n");
+
+       drm_crtc_vblank_put(crtc);
+}
+
 static void rcar_du_crtc_stop(struct rcar_du_crtc *rcrtc)
 {
        struct drm_crtc *crtc = &rcrtc->crtc;
 
        /*
         * Disable all planes and wait for the change to take effect. This is
-        * required as the DSnPR registers are updated on vblank, and no vblank
-        * will occur once the CRTC is stopped. Disabling planes when starting
-        * the CRTC thus wouldn't be enough as it would start scanning out
-        * immediately from old frame buffers until the next vblank.
+        * required as the plane enable registers are updated on vblank, and no
+        * vblank will occur once the CRTC is stopped. Disabling planes when
+        * starting the CRTC thus wouldn't be enough as it would start scanning
+        * out immediately from old frame buffers until the next vblank.
         *
         * This increases the CRTC stop delay, especially when multiple CRTCs
         * are stopped in one operation as we now wait for one vblank per CRTC.
         * Whether this can be improved needs to be researched.
         */
-       rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? DS2PR : DS1PR, 0);
-       drm_crtc_wait_one_vblank(crtc);
+       rcar_du_crtc_disable_planes(rcrtc);
 
        /*
         * Disable vertical blanking interrupt reporting. We first need to wait
@@ -695,10 +723,26 @@ static irqreturn_t rcar_du_crtc_irq(int irq, void *arg)
        irqreturn_t ret = IRQ_NONE;
        u32 status;
 
+       spin_lock(&rcrtc->vblank_lock);
+
        status = rcar_du_crtc_read(rcrtc, DSSR);
        rcar_du_crtc_write(rcrtc, DSRCR, status & DSRCR_MASK);
 
        if (status & DSSR_VBK) {
+               /*
+                * Wake up the vblank wait if the counter reaches 0. This must
+                * be protected by the vblank_lock to avoid races in
+                * rcar_du_crtc_disable_planes().
+                */
+               if (rcrtc->vblank_count) {
+                       if (--rcrtc->vblank_count == 0)
+                               wake_up(&rcrtc->vblank_wait);
+               }
+       }
+
+       spin_unlock(&rcrtc->vblank_lock);
+
+       if (status & DSSR_VBK) {
                drm_crtc_handle_vblank(&rcrtc->crtc);
 
                if (rcdu->info->gen < 3)
@@ -756,6 +800,8 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int index)
        }
 
        init_waitqueue_head(&rcrtc->flip_wait);
+       init_waitqueue_head(&rcrtc->vblank_wait);
+       spin_lock_init(&rcrtc->vblank_lock);
 
        rcrtc->group = rgrp;
        rcrtc->mmio_offset = mmio_offsets[index];

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
index 3cc3768..065b91f 100644 (file)
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
@@ -15,6 +15,7 @@
 #define __RCAR_DU_CRTC_H__
 
 #include <linux/mutex.h>
+#include <linux/spinlock.h>
 #include <linux/wait.h>
 
 #include <drm/drmP.h>
@@ -33,6 +34,9 @@ struct rcar_du_vsp;
  * @initialized: whether the CRTC has been initialized and clocks enabled
  * @event: event to post when the pending page flip completes
  * @flip_wait: wait queue used to signal page flip completion
+ * @vblank_lock: protects vblank_wait and vblank_count
+ * @vblank_wait: wait queue used to signal vertical blanking
+ * @vblank_count: number of vertical blanking interrupts to wait for
  * @outputs: bitmask of the outputs (enum rcar_du_output) driven by this CRTC
  * @group: CRTC group this CRTC belongs to
  * @vsp: VSP feeding video to this CRTC
@@ -50,6 +54,10 @@ struct rcar_du_crtc {
        struct drm_pending_vblank_event *event;
        wait_queue_head_t flip_wait;
 
+       spinlock_t vblank_lock;
+       wait_queue_head_t vblank_wait;
+       unsigned int vblank_count;
+
        unsigned int outputs;
 
        struct rcar_du_group *group;