[PATCH v11 02/19] media: v4l2-dev: Add helpers to run media_pipeline_[started|stopped]()

[Jacopo Mondi]

Hi Dan

On Mon, Jul 14, 2025 at 04:06:28PM +0100, Daniel Scally wrote:
> Add helpers to run the new media_pipeline_started() and
> media_pipeline_stopped() functions. The helpers iterate over the
> entities in the pipeline and count the number of video devices and
> compare that to the pipeline's start_count() before acting. This
> allows us to only run the media pipeline callbacks in the event that
> the pipeline has had video_pipeline_start() called for each video
> device.
>
> Signed-off-by: Daniel Scally <dan.scally at ideasonboard.com>
>
> ---
> Changes in v3:
> 	- Revised the documentation a bit, detailing the specific error
> 	  codes and rewording the descriptions
> 	- Renamed __video_device_pipeline_started()
> 	- List the possible return values explicitly
>
> Changes in v2:
>
> 	- Adapted now media_pipeline_for_each_entity() takes an iter
> 	  variable
> 	- Fixed the Return: section of the kerneldoc comments
> ---
>  drivers/media/v4l2-core/v4l2-dev.c | 57 ++++++++++++++++++++++++++++++++++++++
>  include/media/v4l2-dev.h           | 52 ++++++++++++++++++++++++++++++++++
>  2 files changed, 109 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
> index c369235113d98ae26c30a1aa386e7d60d541a66e..a25038f3c9cc9213b840e2b96d1ef49d17d22fb8 100644
> --- a/drivers/media/v4l2-core/v4l2-dev.c
> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> @@ -1200,6 +1200,63 @@ struct media_pipeline *video_device_pipeline(struct video_device *vdev)
>  }
>  EXPORT_SYMBOL_GPL(video_device_pipeline);
>
> +static int video_device_pipeline_unstarted_vdevs(struct media_pipeline *pipe)
> +{
> +	struct media_pipeline_entity_iter iter;
> +	unsigned int n_video_devices = 0;
> +	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->obj_type == MEDIA_ENTITY_TYPE_VIDEO_DEVICE)
> +			n_video_devices++;
> +	}
> +
> +	media_pipeline_entity_iter_cleanup(&iter);
> +
> +	return n_video_devices - pipe->start_count;
> +}
> +
> +int video_device_pipeline_started(struct video_device *vdev)
> +{
> +	struct media_pipeline *pipe;
> +	int ret;
> +
> +	pipe = video_device_pipeline(vdev);
> +	if (!pipe)
> +		return -ENODEV;
> +
> +	ret = video_device_pipeline_unstarted_vdevs(pipe);
> +	if (ret)
> +		return ret;
> +
> +	return media_pipeline_started(pipe);
> +}
> +EXPORT_SYMBOL_GPL(video_device_pipeline_started);
> +
> +int video_device_pipeline_stopped(struct video_device *vdev)
> +{
> +	struct media_pipeline *pipe;
> +	int ret;
> +
> +	pipe = video_device_pipeline(vdev);
> +	if (!pipe)
> +		return -ENODEV;
> +
> +	ret = video_device_pipeline_unstarted_vdevs(pipe);
> +	if (ret)
> +		return ret;
> +
> +	media_pipeline_stopped(pipe);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(video_device_pipeline_stopped);
> +
>  #endif /* CONFIG_MEDIA_CONTROLLER */
>
>  /*
> diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
> index 1b6222fab24eda96cbe459b435431c01f7259366..45a87a5a76e8782c79da9d4890b063b8f0cbb3d8 100644
> --- a/include/media/v4l2-dev.h
> +++ b/include/media/v4l2-dev.h
> @@ -654,6 +654,58 @@ __must_check int video_device_pipeline_alloc_start(struct video_device *vdev);
>   */
>  struct media_pipeline *video_device_pipeline(struct video_device *vdev);
>
> +/**
> + * video_device_pipeline_started - Run the pipeline_started() entity operation
> + *				   for a fully-started media pipeline
> + * @vdev: A video device that's part of the pipeline
> + *
> + * This function checks whether all MEDIA_ENTITY_TYPE_VIDEO_DEVICE entities
> + * connected to a given video device through enabled links, either directly or
> + * indirectly, have been marked as streaming through the use of
> + * video_device_pipeline_start() or one of its equivalent functions. If so,
> + * media_pipeline_started() is called to inform entities in the pipeline of that
> + * fact. The intention is to provide drivers with a mechanism for checking
> + * whether their pipeline is fully ready to start processing data and call the
> + * .pipeline_started() media entity operation on all the entities in the
> + * pipeline if so.
> + *
> + * Return: The number of video devices in the pipeline remaining to be started,
> + * or a negative error number on failure:
> + *
> + * * 0			- Success
> + * * n > 0		- n video devices in the pipeline remain unstarted
> + * * -ENODEV		- The video device has no pipeline
> + * * -ENOMEM		- failed to allocate pipeline iterator
> + * * n < 0		- Error passed through from a media entity's
> + *			  .pipeline_started() operation
> + */
> +int video_device_pipeline_started(struct video_device *vdev);
> +
> +/**
> + * video_device_pipeline_stopped - Run the pipeline_stopped() entity operation
> + *				   for a fully-started media pipeline
> + * @vdev: A video device that's part of the pipeline
> + *
> + * This function checks whether the MEDIA_ENTITY_TYPE_VIDEO_DEVICE entities
> + * connected to a given video device through enabled links, either directly or
> + * indirectly, have been marked as not streaming through the use of
> + * video_device_pipeline_stop() or one of its equivalent functions. If none of
> + * the video devices in the pipeline have been stopped, then the function
> + * media_pipeline_stopped() is called. The intention is to provide drivers with
> + * a mechanism for checking whether this video device is the first device in the
> + * pipeline to be stopped and call the .pipeline_stopped() media entity
> + * operation on all the entities in the pipeline if so.
> + *
> + * Return: The number of video devices in the pipeline remaining to be started,
> + * or a negative error number on failure:
> + *
> + * * 0			- Success
> + * * n > 0		- n video devices in the pipeline already stopped
> + * * -ENODEV		- The video device has no pipeline
> + * * -ENOMEM		- failed to allocate pipeline iterator
> + */
> +int video_device_pipeline_stopped(struct video_device *vdev);

I still have troubles here,

media_pipeline_stopped() is documented as

* media_pipeline_stopped - Inform entities in a pipeline that it has stopped

and the entity operation pipeline_stopped() is documented as

* @pipeline_stopped:  Optional: Notify this entity that the pipeline it is a
*                     part of has been stopped.

However video_device_pipeline_stopped() calls media_pipeline_stopped()
(and consequentially the entities' .pipeline_stopped() op) on the first call
to streamoff on any video devices in the pipeline.

At this point what confuses me is:

- If we're on the first call to streamoff, has the pipeline actually
  "stopped" ?
- if the fist call to streamoff triggers a pipeline halt, what happens
  to the non-yet-stopped video devices ? Will they stop sending
  buffers back ?
- I understand a post-pipeline-start hook is desirable, but I fail to
  clearly find a use case for this one, that to me looks like a
  pre-pipeline-stop hook.

> +
>  #endif /* CONFIG_MEDIA_CONTROLLER */
>
>  #endif /* _V4L2_DEV_H */
>
> --
> 2.34.1
>
>