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")

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 403bbd5..a12cc7e 100644 (file)
--- 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));