[PATCH 04/13] v4l: vsp1: Repair race between frame end and qbuf handler

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Kieran Bingham <kieran+renesas@xxxxxxxxxxx>

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: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
---
 drivers/media/platform/vsp1/vsp1_video.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_video.c b/drivers/media/platform/vsp1/vsp1_video.c
index ed9759e8a6fc..cd7d215ed455 100644
--- 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;
 
-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux