[media] v4l: vsp1: Repair race between frame end and qbuf handler

The frame-end function releases and completes the buffers on the input
and output entities of the pipe before marking the pipe->state as
'STOPPED'. This introduces a race whereby with the pipe->state still
'RUNNING', a QBUF handler can commence processing a frame before the
frame_end function has completed.

In the event that this happens, a frame queued by QBUF hangs due to the
incorrect pipe->state setting which prevents vsp1_pipeline_run from
issuing a CMD_STRCMD.

By locking the entire function we prevent this from occurring, but we
also change the locking state of the buffer release code. This has been
analysed visually as acceptable, but it must be considered that this now
causes the video->irqlock to be taken under the pipe->irqlock context.

Signed-off-by: Kieran Bingham <kieran+renesas@bingham.xyz>
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>

diff --git a/drivers/media/platform/vsp1/vsp1_video.c b/drivers/media/platform/vsp1/vsp1_video.c
index ed9759e8a6fcee447abde17c3af3a768e3228a3e..cd7d215ed4555390ec506c1a27d317407161b7e2 100644 (file)
--- a/drivers/media/platform/vsp1/vsp1_video.c
+++ b/drivers/media/platform/vsp1/vsp1_video.c
@@ -234,18 +234,13 @@ static void vsp1_video_frame_end(struct vsp1_pipeline *pipe,
 {
        struct vsp1_video *video = rwpf->video;
        struct vsp1_vb2_buffer *buf;
-       unsigned long flags;
 
        buf = vsp1_video_complete_buffer(video);
        if (buf == NULL)
                return;
 
-       spin_lock_irqsave(&pipe->irqlock, flags);
-
        video->rwpf->mem = buf->mem;
        pipe->buffers_ready |= 1 << video->pipe_index;
-
-       spin_unlock_irqrestore(&pipe->irqlock, flags);
 }
 
 static void vsp1_video_pipeline_run(struct vsp1_pipeline *pipe)
@@ -285,6 +280,8 @@ static void vsp1_video_pipeline_frame_end(struct vsp1_pipeline *pipe)
        unsigned long flags;
        unsigned int i;
 
+       spin_lock_irqsave(&pipe->irqlock, flags);
+
        /* Complete buffers on all video nodes. */
        for (i = 0; i < vsp1->info->rpf_count; ++i) {
                if (!pipe->inputs[i])
@@ -295,8 +292,6 @@ static void vsp1_video_pipeline_frame_end(struct vsp1_pipeline *pipe)
 
        vsp1_video_frame_end(pipe, pipe->output);
 
-       spin_lock_irqsave(&pipe->irqlock, flags);
-
        state = pipe->state;
        pipe->state = VSP1_PIPELINE_STOPPED;