drm: rcar-du: Repair vblank for DRM page flips using the VSP

The driver recently switched from handling page flip completion in the
DU vertical blanking handler to the VSP frame end handler to fix a race
condition. This unfortunately resulted in incorrect timestamps in the
vertical blanking events sent to userspace as vertical blanking is now
handled after sending the event.

To fix this we must reverse the order of the two operations. The easiest
way is to handle vertical blanking in the VSP frame end handler before
sending the event. The VSP frame end interrupt occurs approximately 50µs
earlier than the DU frame end interrupt, but this should not cause any
undue harm.

As we need to handle vertical blanking even when page flip completion is
delayed, the VSP driver now needs to call the frame end completion
callback unconditionally, with a new argument to report whether page
flip has completed.

With this new scheme the DU vertical blanking interrupt isn't needed
anymore, so we can stop enabling it.

Fixes: d503a43ac06a ("drm: rcar-du: Register a completion callback with VSP1")
Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Acked-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index 6e5bd0b92dfa882730cd4ada48177e1bf759f743..301ea1a8018eb786e7d210c476f041df2327e9e4 100644 (file)
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -690,6 +690,7 @@ static int rcar_du_crtc_enable_vblank(struct drm_crtc *crtc)
 
        rcar_du_crtc_write(rcrtc, DSRCR, DSRCR_VBCL);
        rcar_du_crtc_set(rcrtc, DIER, DIER_VBE);
+       rcrtc->vblank_enable = true;
 
        return 0;
 }
@@ -699,6 +700,7 @@ static void rcar_du_crtc_disable_vblank(struct drm_crtc *crtc)
        struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
 
        rcar_du_crtc_clr(rcrtc, DIER, DIER_VBE);
+       rcrtc->vblank_enable = false;
 }
 
 static const struct drm_crtc_funcs crtc_funcs = {
@@ -743,10 +745,10 @@ static irqreturn_t rcar_du_crtc_irq(int irq, void *arg)
        spin_unlock(&rcrtc->vblank_lock);
 
        if (status & DSSR_VBK) {
-               drm_crtc_handle_vblank(&rcrtc->crtc);
-
-               if (rcdu->info->gen < 3)
+               if (rcdu->info->gen < 3) {
+                       drm_crtc_handle_vblank(&rcrtc->crtc);
                        rcar_du_crtc_finish_page_flip(rcrtc);
+               }
 
                ret = IRQ_HANDLED;
        }

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
index 065b91f5b1d9b1c6510a432c0f7fe943c0d2d161..fdc2bf99bda1420c3fb10e0557675e79dbe237c3 100644 (file)
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
@@ -32,6 +32,7 @@ struct rcar_du_vsp;
  * @mmio_offset: offset of the CRTC registers in the DU MMIO block
  * @index: CRTC software and hardware index
  * @initialized: whether the CRTC has been initialized and clocks enabled
+ * @vblank_enable: whether vblank events are enabled on this CRTC
  * @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
@@ -51,6 +52,7 @@ struct rcar_du_crtc {
        unsigned int index;
        bool initialized;
 
+       bool vblank_enable;
        struct drm_pending_vblank_event *event;
        wait_queue_head_t flip_wait;
 

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
index e43b065e141a21f3c55466031e0aedff9c7ed23b..6de6be3d90900df6699dc7cbf94445e1db9b97aa 100644 (file)
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
@@ -31,11 +31,15 @@
 #include "rcar_du_kms.h"
 #include "rcar_du_vsp.h"
 
-static void rcar_du_vsp_complete(void *private)
+static void rcar_du_vsp_complete(void *private, bool completed)
 {
        struct rcar_du_crtc *crtc = private;
 
-       rcar_du_crtc_finish_page_flip(crtc);
+       if (crtc->vblank_enable)
+               drm_crtc_handle_vblank(&crtc->crtc);
+
+       if (completed)
+               rcar_du_crtc_finish_page_flip(crtc);
 }
 
 void rcar_du_vsp_enable(struct rcar_du_crtc *crtc)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c
index 7791d7b5a7436bcb4d4f9e08cbde3639c671a9ad..4dfbeac8f42c90c97cb63308efbe84e352c035ee 100644 (file)
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -32,12 +32,13 @@
  * Interrupt Handling
  */
 
-static void vsp1_du_pipeline_frame_end(struct vsp1_pipeline *pipe)
+static void vsp1_du_pipeline_frame_end(struct vsp1_pipeline *pipe,
+                                      bool completed)
 {
        struct vsp1_drm_pipeline *drm_pipe = to_vsp1_drm_pipeline(pipe);
 
        if (drm_pipe->du_complete)
-               drm_pipe->du_complete(drm_pipe->du_private);
+               drm_pipe->du_complete(drm_pipe->du_private, completed);
 }
 
 /* -----------------------------------------------------------------------------

diff --git a/drivers/media/platform/vsp1/vsp1_drm.h b/drivers/media/platform/vsp1/vsp1_drm.h
index fca553ddd1846daedaf74bd5fe7ade34c8fe76f2..1cd9db785bf7fea319962c1bb033e305e1853e9c 100644 (file)
--- a/drivers/media/platform/vsp1/vsp1_drm.h
+++ b/drivers/media/platform/vsp1/vsp1_drm.h
@@ -29,7 +29,7 @@ struct vsp1_drm_pipeline {
        bool enabled;
 
        /* Frame synchronisation */
-       void (*du_complete)(void *);
+       void (*du_complete)(void *, bool);
        void *du_private;
 };
 

diff --git a/drivers/media/platform/vsp1/vsp1_pipe.c b/drivers/media/platform/vsp1/vsp1_pipe.c
index 9bb961298af2462a495fc1f129337d8b270492a1..4f4b732df84b0fa185046789161e7c6efb117fe1 100644 (file)
--- a/drivers/media/platform/vsp1/vsp1_pipe.c
+++ b/drivers/media/platform/vsp1/vsp1_pipe.c
@@ -335,16 +335,12 @@ void vsp1_pipeline_frame_end(struct vsp1_pipeline *pipe)
        if (pipe == NULL)
                return;
 
+       /*
+        * If the DL commit raced with the frame end interrupt, the commit ends
+        * up being postponed by one frame. @completed represents whether the
+        * active frame was finished or postponed.
+        */
        completed = vsp1_dlm_irq_frame_end(pipe->output->dlm);
-       if (!completed) {
-               /*
-                * If the DL commit raced with the frame end interrupt, the
-                * commit ends up being postponed by one frame. Return
-                * immediately without calling the pipeline's frame end handler
-                * or incrementing the sequence number.
-                */
-               return;
-       }
 
        if (pipe->hgo)
                vsp1_hgo_frame_end(pipe->hgo);
@@ -352,8 +348,12 @@ void vsp1_pipeline_frame_end(struct vsp1_pipeline *pipe)
        if (pipe->hgt)
                vsp1_hgt_frame_end(pipe->hgt);
 
+       /*
+        * Regardless of frame completion we still need to notify the pipe
+        * frame_end to account for vblank events.
+        */
        if (pipe->frame_end)
-               pipe->frame_end(pipe);
+               pipe->frame_end(pipe, completed);
 
        pipe->sequence++;
 }

diff --git a/drivers/media/platform/vsp1/vsp1_pipe.h b/drivers/media/platform/vsp1/vsp1_pipe.h
index 91a784a134221d4e0a54e18bd256ae156b76043f..c5d01a36537076c84c07583fe47c6ead60230c36 100644 (file)
--- a/drivers/media/platform/vsp1/vsp1_pipe.h
+++ b/drivers/media/platform/vsp1/vsp1_pipe.h
@@ -91,7 +91,7 @@ struct vsp1_pipeline {
        enum vsp1_pipeline_state state;
        wait_queue_head_t wq;
 
-       void (*frame_end)(struct vsp1_pipeline *pipe);
+       void (*frame_end)(struct vsp1_pipeline *pipe, bool completed);
 
        struct mutex lock;
        struct kref kref;

diff --git a/drivers/media/platform/vsp1/vsp1_video.c b/drivers/media/platform/vsp1/vsp1_video.c
index 84139affb8711dd0814196d36c53fc2b2865fbae..e9f5dcb8fae5fa8c639c713472650efde14408a7 100644 (file)
--- a/drivers/media/platform/vsp1/vsp1_video.c
+++ b/drivers/media/platform/vsp1/vsp1_video.c
@@ -440,13 +440,17 @@ static void vsp1_video_pipeline_run(struct vsp1_pipeline *pipe)
        vsp1_pipeline_run(pipe);
 }
 
-static void vsp1_video_pipeline_frame_end(struct vsp1_pipeline *pipe)
+static void vsp1_video_pipeline_frame_end(struct vsp1_pipeline *pipe,
+                                         bool completed)
 {
        struct vsp1_device *vsp1 = pipe->output->entity.vsp1;
        enum vsp1_pipeline_state state;
        unsigned long flags;
        unsigned int i;
 
+       /* M2M Pipelines should never call here with an incomplete frame. */
+       WARN_ON_ONCE(!completed);
+
        spin_lock_irqsave(&pipe->irqlock, flags);
 
        /* Complete buffers on all video nodes. */

diff --git a/include/media/vsp1.h b/include/media/vsp1.h
index c8fc868fb0f26dbeed6704fb978aa076c539f8bd..68a8abe4fac5a35620dc2fa0a2f5bd6ce02e01ed 100644 (file)
--- a/include/media/vsp1.h
+++ b/include/media/vsp1.h
@@ -34,7 +34,7 @@ struct vsp1_du_lif_config {
        unsigned int width;
        unsigned int height;
 
-       void (*callback)(void *);
+       void (*callback)(void *, bool);
        void *callback_data;
 };