Instead of embedding pipelines in the vsp1_video objects allocate them
on demand when they are needed. This prepares for support of the request
API where the pipeline can differ from request to request. As a side
effects it fixes the streamon race condition where pipelines objects
from different video nodes could be used for the same pipeline.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
---
drivers/media/platform/vsp1/vsp1_bru.c | 1 +
drivers/media/platform/vsp1/vsp1_drv.c | 1 +
drivers/media/platform/vsp1/vsp1_pipe.c | 1 +
drivers/media/platform/vsp1/vsp1_pipe.h | 5 +-
drivers/media/platform/vsp1/vsp1_rpf.c | 1 +
drivers/media/platform/vsp1/vsp1_video.c | 109 +++++++++++++++++--------------
drivers/media/platform/vsp1/vsp1_video.h | 2 -
drivers/media/platform/vsp1/vsp1_wpf.c | 1 +
8 files changed, 68 insertions(+), 53 deletions(-)
diff --git a/drivers/media/platform/vsp1/vsp1_bru.c b/drivers/media/platform/vsp1/vsp1_bru.c
index f46fd79fc3be..41c970bf5f74 100644
--- a/drivers/media/platform/vsp1/vsp1_bru.c
+++ b/drivers/media/platform/vsp1/vsp1_bru.c
@@ -19,6 +19,7 @@
#include "vsp1.h"
#include "vsp1_bru.h"
#include "vsp1_dl.h"
+#include "vsp1_pipe.h"
#include "vsp1_rwpf.h"
#include "vsp1_video.h"
diff --git a/drivers/media/platform/vsp1/vsp1_drv.c b/drivers/media/platform/vsp1/vsp1_drv.c
index 0232057504d6..6b0e3e3382c3 100644
--- a/drivers/media/platform/vsp1/vsp1_drv.c
+++ b/drivers/media/platform/vsp1/vsp1_drv.c
@@ -30,6 +30,7 @@
#include "vsp1_hsit.h"
#include "vsp1_lif.h"
#include "vsp1_lut.h"
+#include "vsp1_pipe.h"
#include "vsp1_rwpf.h"
#include "vsp1_sru.h"
#include "vsp1_uds.h"
diff --git a/drivers/media/platform/vsp1/vsp1_pipe.c b/drivers/media/platform/vsp1/vsp1_pipe.c
index 6c590b857812..b661d8e76a3a 100644
--- a/drivers/media/platform/vsp1/vsp1_pipe.c
+++ b/drivers/media/platform/vsp1/vsp1_pipe.c
@@ -189,6 +189,7 @@ void vsp1_pipeline_init(struct vsp1_pipeline *pipe)
mutex_init(&pipe->lock);
spin_lock_init(&pipe->irqlock);
init_waitqueue_head(&pipe->wq);
+ kref_init(&pipe->kref);
INIT_LIST_HEAD(&pipe->entities);
pipe->state = VSP1_PIPELINE_STOPPED;
diff --git a/drivers/media/platform/vsp1/vsp1_pipe.h b/drivers/media/platform/vsp1/vsp1_pipe.h
index 5ef1a7f374d6..953c473dca8f 100644
--- a/drivers/media/platform/vsp1/vsp1_pipe.h
+++ b/drivers/media/platform/vsp1/vsp1_pipe.h
@@ -13,6 +13,7 @@
#ifndef __VSP1_PIPE_H__
#define __VSP1_PIPE_H__
+#include <linux/kref.h>
#include <linux/list.h>
#include <linux/spinlock.h>
#include <linux/wait.h>
@@ -63,7 +64,7 @@ enum vsp1_pipeline_state {
* @wq: work queue to wait for state change completion
* @frame_end: frame end interrupt handler
* @lock: protects the pipeline use count and stream count
- * @use_count: number of video nodes using the pipeline
+ * @kref: pipeline reference count
* @stream_count: number of streaming video nodes
* @buffers_ready: bitmask of RPFs and WPFs with at least one buffer available
* @num_inputs: number of RPFs
@@ -88,7 +89,7 @@ struct vsp1_pipeline {
void (*frame_end)(struct vsp1_pipeline *pipe);
struct mutex lock;
- unsigned int use_count;
+ struct kref kref;
unsigned int stream_count;
unsigned int buffers_ready;
diff --git a/drivers/media/platform/vsp1/vsp1_rpf.c b/drivers/media/platform/vsp1/vsp1_rpf.c
index 13a853664853..0d6e6f89990f 100644
--- a/drivers/media/platform/vsp1/vsp1_rpf.c
+++ b/drivers/media/platform/vsp1/vsp1_rpf.c
@@ -17,6 +17,7 @@
#include "vsp1.h"
#include "vsp1_dl.h"
+#include "vsp1_pipe.h"
#include "vsp1_rwpf.h"
#include "vsp1_video.h"
diff --git a/drivers/media/platform/vsp1/vsp1_video.c b/drivers/media/platform/vsp1/vsp1_video.c
index 7757ad6e9a61..3a4de7d5dbd5 100644
--- a/drivers/media/platform/vsp1/vsp1_video.c
+++ b/drivers/media/platform/vsp1/vsp1_video.c
@@ -377,12 +377,9 @@ static int vsp1_video_pipeline_build(struct vsp1_pipeline *pipe,
{
struct media_entity_graph graph;
struct media_entity *entity = &video->video.entity;
- struct media_device *mdev = entity->parent;
unsigned int i;
int ret;
- mutex_lock(&mdev->graph_mutex);
-
/* Walk the graph to locate the entities and video nodes. */
media_entity_graph_walk_start(&graph, entity);
@@ -415,13 +412,9 @@ static int vsp1_video_pipeline_build(struct vsp1_pipeline *pipe,
}
}
- mutex_unlock(&mdev->graph_mutex);
-
/* We need one output and at least one input. */
- if (pipe->num_inputs == 0 || !pipe->output) {
- ret = -EPIPE;
- goto error;
- }
+ if (pipe->num_inputs == 0 || !pipe->output)
+ return -EPIPE;
/* Follow links downstream for each input and make sure the graph
* contains no loop and that all branches end at the output WPF.
@@ -434,47 +427,76 @@ static int vsp1_video_pipeline_build(struct vsp1_pipeline *pipe,
pipe->inputs[i].rpf,
pipe->output);
if (ret < 0)
- goto error;
+ return ret;
}
return 0;
-
-error:
- vsp1_pipeline_reset(pipe);
- return ret;
}
static int vsp1_video_pipeline_init(struct vsp1_pipeline *pipe,
struct vsp1_video *video)
{
+ vsp1_pipeline_init(pipe);
+
+ pipe->frame_end = vsp1_video_pipeline_frame_end;
+
+ return vsp1_video_pipeline_build(pipe, video);
+}
+
+static struct vsp1_pipeline *vsp1_video_pipeline_get(struct vsp1_video *video)
+{
+ struct media_device *mdev = &video->vsp1->media_dev;
+ struct vsp1_pipeline *pipe;
int ret;
- mutex_lock(&pipe->lock);
+ /* Get a pipeline object for the video node. If a pipeline has already
+ * been allocated just increment its reference count and return it.
+ * Otherwise allocate a new pipeline and initialize it, it will be freed
+ * when the last reference is released.
+ */
+ mutex_lock(&mdev->graph_mutex);
- /* If we're the first user build and validate the pipeline. */
- if (pipe->use_count == 0) {
- ret = vsp1_video_pipeline_build(pipe, video);
- if (ret < 0)
+ if (!video->rwpf->pipe) {
+ pipe = kzalloc(sizeof(*pipe), GFP_KERNEL);
+ if (!pipe) {
+ pipe = ERR_PTR(-ENOMEM);
goto done;
+ }
+
+ ret = vsp1_video_pipeline_init(pipe, video);
+ if (ret < 0) {
+ vsp1_pipeline_reset(pipe);
+ kfree(pipe);
+ pipe = ERR_PTR(ret);
+ goto done;
+ }
+ } else {
+ pipe = video->rwpf->pipe;
+ kref_get(&pipe->kref);
}
- pipe->use_count++;
- ret = 0;
+ /* TODO: Make sure links can't be touched anymore. */
done:
- mutex_unlock(&pipe->lock);
- return ret;
+ mutex_unlock(&mdev->graph_mutex);
+ return pipe;
}
-static void vsp1_video_pipeline_cleanup(struct vsp1_pipeline *pipe)
+static void vsp1_video_pipeline_release(struct kref *kref)
{
- mutex_lock(&pipe->lock);
+ struct vsp1_pipeline *pipe = container_of(kref, typeof(*pipe), kref);
- /* If we're the last user clean up the pipeline. */
- if (--pipe->use_count == 0)
- vsp1_pipeline_reset(pipe);
+ vsp1_pipeline_reset(pipe);
+ kfree(pipe);
+}
- mutex_unlock(&pipe->lock);
+static void vsp1_video_pipeline_put(struct vsp1_pipeline *pipe)
+{
+ struct media_device *mdev = &pipe->output->entity.vsp1->media_dev;
+
+ mutex_lock(&mdev->graph_mutex);
+ kref_put(&pipe->kref, vsp1_video_pipeline_release);
+ mutex_unlock(&mdev->graph_mutex);
}
/* -----------------------------------------------------------------------------
@@ -651,8 +673,8 @@ static void vsp1_video_stop_streaming(struct vb2_queue *vq)
}
mutex_unlock(&pipe->lock);
- vsp1_video_pipeline_cleanup(pipe);
media_entity_pipeline_stop(&video->video.entity);
+ vsp1_video_pipeline_put(pipe);
/* Remove all buffers from the IRQ queue. */
spin_lock_irqsave(&video->irqlock, flags);
@@ -770,20 +792,16 @@ vsp1_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
video->sequence = 0;
+ pipe = vsp1_video_pipeline_get(video);
+ if (IS_ERR(pipe))
+ return PTR_ERR(pipe);
+
/* Start streaming on the pipeline. No link touching an entity in the
* pipeline can be activated or deactivated once streaming is started.
- *
- * Use the VSP1 pipeline object embedded in the first video object that
- * starts streaming.
- *
- * FIXME: This is racy, the ioctl is only protected by the video node
- * lock.
*/
- pipe = video->rwpf->pipe ? video->rwpf->pipe : &video->pipe;
-
ret = media_entity_pipeline_start(&video->video.entity, &pipe->pipe);
if (ret < 0)
- return ret;
+ goto err_pipe;
/* Verify that the configured format matches the output of the connected
* subdev.
@@ -792,21 +810,17 @@ vsp1_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
if (ret < 0)
goto err_stop;
- ret = vsp1_video_pipeline_init(pipe, video);
- if (ret < 0)
- goto err_stop;
-
/* Start the queue. */
ret = vb2_streamon(&video->queue, type);
if (ret < 0)
- goto err_cleanup;
+ goto err_stop;
return 0;
-err_cleanup:
- vsp1_video_pipeline_cleanup(pipe);
err_stop:
media_entity_pipeline_stop(&video->video.entity);
+err_pipe:
+ vsp1_video_pipeline_put(pipe);
return ret;
}
@@ -922,9 +936,6 @@ struct vsp1_video *vsp1_video_create(struct vsp1_device *vsp1,
spin_lock_init(&video->irqlock);
INIT_LIST_HEAD(&video->irqqueue);
- vsp1_pipeline_init(&video->pipe);
- video->pipe.frame_end = vsp1_video_pipeline_frame_end;
-
/* Initialize the media entity... */
ret = media_entity_init(&video->video.entity, 1, &video->pad, 0);
if (ret < 0)
diff --git a/drivers/media/platform/vsp1/vsp1_video.h b/drivers/media/platform/vsp1/vsp1_video.h
index 64abd39ee1e7..867b00807c46 100644
--- a/drivers/media/platform/vsp1/vsp1_video.h
+++ b/drivers/media/platform/vsp1/vsp1_video.h
@@ -18,7 +18,6 @@
#include <media/videobuf2-v4l2.h>
-#include "vsp1_pipe.h"
#include "vsp1_rwpf.h"
struct vsp1_vb2_buffer {
@@ -44,7 +43,6 @@ struct vsp1_video {
struct mutex lock;
- struct vsp1_pipeline pipe;
unsigned int pipe_index;
struct vb2_queue queue;
diff --git a/drivers/media/platform/vsp1/vsp1_wpf.c b/drivers/media/platform/vsp1/vsp1_wpf.c
index a7d94a76032c..179f21035765 100644
--- a/drivers/media/platform/vsp1/vsp1_wpf.c
+++ b/drivers/media/platform/vsp1/vsp1_wpf.c
@@ -17,6 +17,7 @@
#include "vsp1.h"
#include "vsp1_dl.h"
+#include "vsp1_pipe.h"
#include "vsp1_rwpf.h"
#include "vsp1_video.h"
--
2.4.10
