On 29/08/2022 19:34, Laurent Pinchart wrote:
Hi Tomi,
Thank you for the patch.
On Wed, Aug 10, 2022 at 03:10:57PM +0300, Tomi Valkeinen wrote:
With the upcoming stream related improvements to the pipelines, the
pipelines are moved from media entities to media pads. As the drivers
currently use the pipelines with the entity based model, moving the
pipelines to pads will cause changes to the drivers.
However, most of the uses of media pipelines are related to a video
device (a DMA engine) with a single pad, and thus there's never a need
to support multiple pads in these use cases. We can avoid pushing the
complexities of the pad based model to the drivers by adding video
device wrappers for the pipeline related functions.
This patch adds a number of wrappers to media_pipeline functions, all of
which take a video_device as a parameter (instead of a media_entity),
and verify that there's just one pad.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxxxxxxxxxxxx>
---
drivers/media/v4l2-core/v4l2-dev.c | 61 ++++++++++++++++++++
include/media/v4l2-dev.h | 90 ++++++++++++++++++++++++++++++
2 files changed, 151 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index d00237ee4cae..7f933ff89fd4 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -1095,6 +1095,67 @@ void video_unregister_device(struct video_device *vdev)
}
EXPORT_SYMBOL(video_unregister_device);
+#if defined(CONFIG_MEDIA_CONTROLLER)
+
+__must_check int video_device_pipeline_start(struct video_device *vdev,
+ struct media_pipeline *pipe)
+{
+ struct media_entity *entity = &vdev->entity;
+
+ if (entity->num_pads != 1)
+ return -ENODEV;
+
+ return media_pipeline_start(entity, pipe);
+}
+EXPORT_SYMBOL_GPL(video_device_pipeline_start);
+
+__must_check int __video_device_pipeline_start(struct video_device *vdev,
+ struct media_pipeline *pipe)
+{
+ struct media_entity *entity = &vdev->entity;
+
+ if (entity->num_pads != 1)
+ return -ENODEV;
+
+ return __media_pipeline_start(entity, pipe);
+}
+EXPORT_SYMBOL_GPL(__video_device_pipeline_start);
+
+void video_device_pipeline_stop(struct video_device *vdev)
+{
+ struct media_entity *entity = &vdev->entity;
+
+ if (WARN_ON(entity->num_pads != 1))
+ return;
+
+ return media_pipeline_stop(entity);
+}
+EXPORT_SYMBOL_GPL(video_device_pipeline_stop);
+
+void __video_device_pipeline_stop(struct video_device *vdev)
+{
+ struct media_entity *entity = &vdev->entity;
+
+ if (WARN_ON(entity->num_pads != 1))
+ return;
+
+ return __media_pipeline_stop(entity);
+}
+EXPORT_SYMBOL_GPL(__video_device_pipeline_stop);
+
+struct media_pipeline *video_device_pipeline(struct video_device *vdev)
+{
+ struct media_entity *entity = &vdev->entity;
+
+ if (WARN_ON(entity->num_pads != 1))
+ return NULL;
+
+ return media_entity_pipeline(entity);
+}
+EXPORT_SYMBOL_GPL(video_device_pipeline);
+
+#endif /* CONFIG_MEDIA_CONTROLLER */
+
/*
* Initialise video for linux
*/
diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
index 5cf1edefb822..4ccc24f5918a 100644
--- a/include/media/v4l2-dev.h
+++ b/include/media/v4l2-dev.h
@@ -539,4 +539,94 @@ static inline int video_is_registered(struct video_device *vdev)
return test_bit(V4L2_FL_REGISTERED, &vdev->flags);
}
+#if defined(CONFIG_MEDIA_CONTROLLER)
+
+/**
+ * video_device_pipeline_start - Mark a pipeline as streaming
+ * @vdev: Starting video device
+ * @pipe: Media pipeline to be assigned to all entities in the pipeline.
+ *
+ * Mark all entities connected to a given video device through enabled links,
+ * either directly or indirectly, as streaming. The given pipeline object is
+ * assigned to every entity in the pipeline and stored in the media_entity pipe
+ * field.
+ *
+ * Calls to this function can be nested, in which case the same number of
+ * video_device_pipeline_stop() calls will be required to stop streaming. The
+ * pipeline pointer must be identical for all nested calls to
+ * video_device_pipeline_start().
+ *
+ * The video device must contain a single pad.
+ *
+ * This is a convenience wrapper to media_pipeline_start().
s/wrapper to/wrapper around/ maybe. Same below.
Sounds good to me.
+ */
+__must_check int video_device_pipeline_start(struct video_device *vdev,
+ struct media_pipeline *pipe);
+
+/**
+ * __video_device_pipeline_start - Mark a pipeline as streaming
+ *
I'd drop the blank line as you don't have one for the previous function.
Same for stop.
Yep.
Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
Thanks!
