drm/vc4: Fix VBLANK handling in crtc->enable() path

When we are enabling a CRTC, drm_crtc_vblank_get() is called before
drm_crtc_vblank_on(), which is not supposed to happen (hence the
WARN_ON() in the code). To solve the problem, we delay the 'update
display list' operation after the CRTC is actually enabled.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Reviewed-by: Eric Anholt <eric@anholt.net>
Link: http://patchwork.freedesktop.org/patch/msgid/1498163126-26678-1-git-send-email-boris.brezillon@free-electrons.com
Fixes: 34c8ea400ff6 ("drm/vc4: Mimic drm_atomic_helper_commit() behavior")

1 files changed, 43 insertions, 23 deletions

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 403bbd5f99a9..a12cc7ea99b6 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -520,6 +520,34 @@ static void vc4_crtc_disable(struct drm_crtc *crtc)
                      SCALER_DISPSTATX_EMPTY);
 }
 
+static void vc4_crtc_update_dlist(struct drm_crtc *crtc)
+{
+        struct drm_device *dev = crtc->dev;
+        struct vc4_dev *vc4 = to_vc4_dev(dev);
+        struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
+        struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(crtc->state);
+
+        if (crtc->state->event) {
+                unsigned long flags;
+
+                crtc->state->event->pipe = drm_crtc_index(crtc);
+
+                WARN_ON(drm_crtc_vblank_get(crtc) != 0);
+
+                spin_lock_irqsave(&dev->event_lock, flags);
+                vc4_crtc->event = crtc->state->event;
+                crtc->state->event = NULL;
+
+                HVS_WRITE(SCALER_DISPLISTX(vc4_crtc->channel),
+                          vc4_state->mm.start);
+
+                spin_unlock_irqrestore(&dev->event_lock, flags);
+        } else {
+                HVS_WRITE(SCALER_DISPLISTX(vc4_crtc->channel),
+                          vc4_state->mm.start);
+        }
+}
+
 static void vc4_crtc_enable(struct drm_crtc *crtc)
 {
         struct drm_device *dev = crtc->dev;
@@ -530,6 +558,12 @@ static void vc4_crtc_enable(struct drm_crtc *crtc)
 
         require_hvs_enabled(dev);
 
+        /* Enable vblank irq handling before crtc is started otherwise
+         * drm_crtc_get_vblank() fails in vc4_crtc_update_dlist().
+         */
+        drm_crtc_vblank_on(crtc);
+        vc4_crtc_update_dlist(crtc);
+
         /* Turn on the scaler, which will wait for vstart to start
          * compositing.
          */
@@ -541,9 +575,6 @@ static void vc4_crtc_enable(struct drm_crtc *crtc)
         /* Turn on the pixel valve, which will emit the vstart signal. */
         CRTC_WRITE(PV_V_CONTROL,
                    CRTC_READ(PV_V_CONTROL) | PV_VCONTROL_VIDEN);
-
-        /* Enable vblank irq handling after crtc is started. */
-        drm_crtc_vblank_on(crtc);
 }
 
 static bool vc4_crtc_mode_fixup(struct drm_crtc *crtc,
@@ -598,7 +629,6 @@ static void vc4_crtc_atomic_flush(struct drm_crtc *crtc,
 {
         struct drm_device *dev = crtc->dev;
         struct vc4_dev *vc4 = to_vc4_dev(dev);
-        struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
         struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(crtc->state);
         struct drm_plane *plane;
         bool debug_dump_regs = false;
@@ -620,25 +650,15 @@ static void vc4_crtc_atomic_flush(struct drm_crtc *crtc,
 
         WARN_ON_ONCE(dlist_next - dlist_start != vc4_state->mm.size);
 
-        if (crtc->state->event) {
-                unsigned long flags;
-
-                crtc->state->event->pipe = drm_crtc_index(crtc);
-
-                WARN_ON(drm_crtc_vblank_get(crtc) != 0);
-
-                spin_lock_irqsave(&dev->event_lock, flags);
-                vc4_crtc->event = crtc->state->event;
-                crtc->state->event = NULL;
-
-                HVS_WRITE(SCALER_DISPLISTX(vc4_crtc->channel),
-                          vc4_state->mm.start);
-
-                spin_unlock_irqrestore(&dev->event_lock, flags);
-        } else {
-                HVS_WRITE(SCALER_DISPLISTX(vc4_crtc->channel),
-                          vc4_state->mm.start);
-        }
+        /* Only update DISPLIST if the CRTC was already running and is not
+         * being disabled.
+         * vc4_crtc_enable() takes care of updating the dlist just after
+         * re-enabling VBLANK interrupts and before enabling the engine.
+         * If the CRTC is being disabled, there's no point in updating this
+         * information.
+         */
+        if (crtc->state->active && old_state->active)
+                vc4_crtc_update_dlist(crtc);
 
         if (debug_dump_regs) {
                 DRM_INFO("CRTC %d HVS after:\n", drm_crtc_index(crtc));