Hi Jacopo
On 08/07/2025 14:29, Jacopo Mondi wrote:
Hi Dan
On Fri, Jul 04, 2025 at 12:20:18PM +0100, Daniel Scally wrote:
Add two new members to struct media_entity_operations, along with new
functions in media-entity.c to traverse a media pipeline and call the
new operations. The new functions are intended to be used to signal
to a media pipeline that it has fully started, with the entity ops
allowing drivers to define some action to be taken when those
conditions are met.
The combination of the new functions and operations allows drivers
which are part of a multi-driver pipeline to delay actually starting
streaming until all of the conditions for streaming succcessfully are
met across all drivers.
Signed-off-by: Daniel Scally <dan.scally@xxxxxxxxxxxxxxxx>
---
Changes in v4:
- Reverted to having the iter variable
Changes in v3:
- Dropped the iter variable now that the pipeline entity
iterator functions don't need it.
- Updated documentation to specify Optional and return
values
Changes in v2:
- Refactored media_pipeline_started() such that the cleanup
function for media_pipeline_entity_iter is unconditionally
called
- Avoided using media_entity_call() helper for operation that
has return type void to avoid compiler warnings
---
drivers/media/mc/mc-entity.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
include/media/media-entity.h | 29 ++++++++++++++++++++++++++++
2 files changed, 75 insertions(+)
diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
index 045590905582054c46656e20463271b1f93fa6b4..d3443537d4304e12cb015630101efba22375c011 100644
--- a/drivers/media/mc/mc-entity.c
+++ b/drivers/media/mc/mc-entity.c
@@ -1053,6 +1053,52 @@ __media_pipeline_entity_iter_next(struct media_pipeline *pipe,
}
EXPORT_SYMBOL_GPL(__media_pipeline_entity_iter_next);
+int media_pipeline_started(struct media_pipeline *pipe)
+{
+ struct media_pipeline_entity_iter iter;
+ struct media_entity *entity;
+ int ret;
+
+ ret = media_pipeline_entity_iter_init(pipe, &iter);
+ if (ret)
+ return ret;
+
+ media_pipeline_for_each_entity(pipe, &iter, entity) {
+ ret = media_entity_call(entity, pipeline_started);
+ if (ret && ret != -ENOIOCTLCMD)
+ break;
+ }
+
+ media_pipeline_entity_iter_cleanup(&iter);
+
+ ret = ret == -ENOIOCTLCMD ? 0 : ret;
+ if (ret)
+ media_pipeline_stopped(pipe);
If you take my suggestion to limit the return value of
video_device_pipeline_started() to three possible error codes, you
could return -EINVAL here
+
+ return ret;
and 0 here
+}
+EXPORT_SYMBOL_GPL(media_pipeline_started);
+
+int media_pipeline_stopped(struct media_pipeline *pipe)
+{
+ struct media_pipeline_entity_iter iter;
+ struct media_entity *entity;
+ int ret;
+
+ ret = media_pipeline_entity_iter_init(pipe, &iter);
+ if (ret)
+ return ret;
+
+ media_pipeline_for_each_entity(pipe, &iter, entity)
+ if (entity->ops && entity->ops->pipeline_stopped)
+ entity->ops->pipeline_stopped(entity);
+
+ media_pipeline_entity_iter_cleanup(&iter);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(media_pipeline_stopped);
+
/* -----------------------------------------------------------------------------
* Links management
*/
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index 64cf590b11343f68a456c5870ca2f32917c122f9..ad658f42357ec505c84d9479bbbf18494da7f939 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -269,6 +269,10 @@ struct media_pad {
* media_entity_has_pad_interdep().
* Optional: If the operation isn't implemented all pads
* will be considered as interdependent.
+ * @pipeline_started: Notify this entity that the pipeline it is a part of has
+ * been started
+ * @pipeline_stopped: Notify this entity that the pipeline it is a part of has
+ * been stopped
The documentation of the other functions end with a full stop.
If the operation is optional, I would specify it here like it's done
for other operations
*
* .. note::
*
@@ -284,6 +288,8 @@ struct media_entity_operations {
int (*link_validate)(struct media_link *link);
bool (*has_pad_interdep)(struct media_entity *entity, unsigned int pad0,
unsigned int pad1);
+ int (*pipeline_started)(struct media_entity *entity);
+ void (*pipeline_stopped)(struct media_entity *entity);
};
/**
@@ -1261,6 +1267,29 @@ __media_pipeline_entity_iter_next(struct media_pipeline *pipe,
entity != NULL; \
entity = __media_pipeline_entity_iter_next((pipe), iter, entity))
+/**
+ * media_pipeline_started - Inform entities in a pipeline that it has started
+ * @pipe: The pipeline
+ *
+ * Iterate on all entities in a media pipeline and call their pipeline_started
+ * member of media_entity_operations. Optional.
I would move "Optional" to the documentation of the media entity
I don't know what I was thinking putting it here...
Thanks!
Dan
+ *
+ * Return: zero on success, or a negative error code passed through from an
+ * entity's .pipeline_started() operation.
+ */
+int media_pipeline_started(struct media_pipeline *pipe);
+
+/**
+ * media_pipeline_stopped - Inform entities in a pipeline that it has stopped
+ * @pipe: The pipeline
+ *
+ * Iterate on all entities in a media pipeline and call their pipeline_stopped
+ * member of media_entity_operations. Optional.
+ *
+ * Return: zero on success, or -ENOMEM if the iterator initialisation failed.
+ */
+int media_pipeline_stopped(struct media_pipeline *pipe);
+
/**
* media_pipeline_alloc_start - Mark a pipeline as streaming
* @pad: Starting pad
--
2.34.1
