drm: rcar-du: Fix race condition when disabling planes at CRTC stop
authorLaurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Fri, 28 Jul 2017 23:31:33 +0000 (02:31 +0300)
committerLaurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Thu, 3 Aug 2017 13:17:27 +0000 (16:17 +0300)
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>
drivers/gpu/drm/rcar-du/rcar_du_crtc.c
drivers/gpu/drm/rcar-du/rcar_du_crtc.h

index 17fd1cd5212ce129b166f2a3f24847618cd10780..6e5bd0b92dfa882730cd4ada48177e1bf759f743 100644 (file)
@@ -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,9 +723,25 @@ 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);
 
@@ -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];
index 3cc37682659218b3bb6de8db1583150d77faff84..065b91f5b1d9b1c6510a432c0f7fe943c0d2d161 100644 (file)
@@ -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;