diff mbox series

[v3,2/4] media: v4l2-common: add helper functions to call s_stream() callbacks

Message ID 20200415013044.1778572-3-helen.koike@collabora.com (mailing list archive)
State New, archived
Headers show
Series media: add v4l2_pipeline_stream_{enable, disable} helpers | expand

Commit Message

Helen Koike April 15, 2020, 1:30 a.m. UTC
Add v4l2_pipeline_stream_{enable,disable} helper functions to iterate
through the subdevices in a given stream (i.e following links from sink
to source) and call .s_stream() callback.

Add stream_count on the subdevice object for simultaneous streaming from
different video devices which shares subdevices.

Prevent calling .s_stream(true) if it was already called previously by
another stream.

Prevent calling .s_stream(false) from one stream when there are still
others active.

If .s_stream(true) fails, call .s_stream(false) in the reverse order.

Signed-off-by: Helen Koike <helen.koike@collabora.com>
---

Changes in v3:
- re-write helpers to use media walkers as in v1, but with
v4l2_pipeline_subdevs_get() to reverse the order we call s_stream(true)
in subdevices.
- rename size to max_size (and update docs) in v4l2_pipeline_subdevs_get() to
reflect the meaning of the argument.
- add if defined(CONFIG_MEDIA_CONTROLLER) around helpers

Changes in v2:
- re-write helpers to not use media walkers

 drivers/media/v4l2-core/v4l2-common.c | 125 ++++++++++++++++++++++++++
 include/media/v4l2-common.h           |  43 +++++++++
 include/media/v4l2-subdev.h           |   2 +
 3 files changed, 170 insertions(+)

Comments

Niklas Söderlund April 24, 2020, 1:41 p.m. UTC | #1
Hi Helen,

Thanks for your work.

On 2020-04-14 22:30:42 -0300, Helen Koike wrote:
> Add v4l2_pipeline_stream_{enable,disable} helper functions to iterate
> through the subdevices in a given stream (i.e following links from sink
> to source) and call .s_stream() callback.
> 
> Add stream_count on the subdevice object for simultaneous streaming from
> different video devices which shares subdevices.
> 
> Prevent calling .s_stream(true) if it was already called previously by
> another stream.
> 
> Prevent calling .s_stream(false) from one stream when there are still
> others active.
> 
> If .s_stream(true) fails, call .s_stream(false) in the reverse order.
> 
> Signed-off-by: Helen Koike <helen.koike@collabora.com>
> ---
> 
> Changes in v3:
> - re-write helpers to use media walkers as in v1, but with
> v4l2_pipeline_subdevs_get() to reverse the order we call s_stream(true)
> in subdevices.
> - rename size to max_size (and update docs) in v4l2_pipeline_subdevs_get() to
> reflect the meaning of the argument.
> - add if defined(CONFIG_MEDIA_CONTROLLER) around helpers
> 
> Changes in v2:
> - re-write helpers to not use media walkers
> 
>  drivers/media/v4l2-core/v4l2-common.c | 125 ++++++++++++++++++++++++++
>  include/media/v4l2-common.h           |  43 +++++++++
>  include/media/v4l2-subdev.h           |   2 +
>  3 files changed, 170 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index 9e8eb45a5b03c..2f991a1a57d7c 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -442,3 +442,128 @@ int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, u32 pixelformat,
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt);
> +
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +
> +/*
> + * v4l2_pipeline_subdevs_get - Assemble a list of subdevices in a pipeline
> + * @subdevs: the array to be filled.
> + * @max_size: max number of elements that can fit in subdevs array.
> + *
> + * Walk from a video node, following links from sink to source and fill the
> + * array with subdevices in the path.
> + * The walker performs a depth-first traversal, which means that, in a topology
> + * sd1->sd2->sd3->vdev, sd1 will be the first element placed in the array.
> + *
> + * Return the number of subdevices filled in the array.
> + */
> +static int v4l2_pipeline_subdevs_get(struct video_device *vdev,
> +				     struct media_pipeline *pipe,
> +				     struct v4l2_subdev **subdevs,
> +				     unsigned int max_size)
> +{
> +	struct media_entity *entity = &vdev->entity;
> +	struct media_device *mdev = entity->graph_obj.mdev;
> +	int idx = 0;
> +	int ret;
> +
> +	mutex_lock(&mdev->graph_mutex);
> +
> +	if (!pipe->streaming_count) {
> +		ret = media_graph_walk_init(&pipe->graph, mdev);
> +		if (ret) {
> +			mutex_unlock(&mdev->graph_mutex);
> +			return ret;
> +		}
> +	}

This confuses me a bit. Why is this conditional on streaming_count ?  
Looking how it's used in mc-entity.c the count is also 
increased/decreased with this pattern.

Would it make sens to create a local 'struct media_graph graph' here and 
always init and clean it up ?

> +
> +	media_graph_walk_start(&pipe->graph, entity);
> +
> +	while ((entity = media_graph_walk_next_stream(&pipe->graph))) {
> +		if (!is_media_entity_v4l2_subdev(entity))
> +			continue;
> +		if (WARN_ON(idx >= max_size)) {
> +			mutex_unlock(&mdev->graph_mutex);
> +			return -EINVAL;
> +		}
> +		subdevs[idx++] = media_entity_to_v4l2_subdev(entity);
> +	}
> +
> +	if (!pipe->streaming_count)
> +		media_graph_walk_cleanup(&pipe->graph);
> +
> +	mutex_unlock(&mdev->graph_mutex);
> +
> +	return idx;
> +}
> +
> +__must_check int v4l2_pipeline_stream_enable(struct video_device *vdev,
> +					     struct media_pipeline *pipe)
> +{
> +	struct media_device *mdev = vdev->entity.graph_obj.mdev;
> +	struct v4l2_subdev *subdevs[MEDIA_ENTITY_ENUM_MAX_DEPTH];
> +	struct v4l2_subdev *sd;
> +	int i, size, ret;
> +
> +	size = v4l2_pipeline_subdevs_get(vdev, pipe,
> +					 subdevs, ARRAY_SIZE(subdevs));
> +	if (size <= 0)
> +		return size;
> +
> +	/* Call s_stream() in reverse order to enable sensors last */
> +	for (i = size - 1; i >= 0; i--) {
> +		sd = subdevs[i];
 +		if (sd->stream_count++)
> +			continue;
> +		dev_dbg(mdev->dev,
> +			"enabling stream for '%s'\n", sd->entity.name);
> +		ret = v4l2_subdev_call(sd, video, s_stream, true);
> +		if (ret && ret != -ENOIOCTLCMD) {
> +			sd->stream_count = 0;
> +			goto err_stream_disable;
> +		}
> +	}
> +
> +	return 0;
> +
> +err_stream_disable:
> +	for (i++; i < size; i++) {
> +		sd = subdevs[i];
> +		if (--sd->stream_count)
> +			continue;
> +		dev_dbg(mdev->dev,
> +			"disabling stream for '%s'\n", sd->entity.name);
> +		v4l2_subdev_call(sd, video, s_stream, false);
> +	};
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_pipeline_stream_enable);
> +
> +void v4l2_pipeline_stream_disable(struct video_device *vdev,
> +				  struct media_pipeline *pipe)
> +{
> +	struct media_device *mdev = vdev->entity.graph_obj.mdev;
> +	struct v4l2_subdev *subdevs[MEDIA_ENTITY_ENUM_MAX_DEPTH];
> +	unsigned int i;
> +	int size;
> +
> +	size = v4l2_pipeline_subdevs_get(vdev, pipe,
> +					 subdevs, ARRAY_SIZE(subdevs));
> +	if (WARN_ON(size < 0))
> +		return;
> +
> +	/* Call s_stream() in order to disable sensors first */
> +	for (i = 0; i < size; i++) {
> +		struct v4l2_subdev *sd = subdevs[i];
> +
> +		if (--sd->stream_count)
> +			continue;
> +		dev_dbg(mdev->dev,
> +			"disabling stream for '%s'\n", sd->entity.name);
> +		v4l2_subdev_call(sd, video, s_stream, false);

small nit, maybe this can be extracted to a helper as the code is 
duplicated here and in the error path v4l2_pipeline_stream_enable ?

> +	}
> +}
> +EXPORT_SYMBOL_GPL(v4l2_pipeline_stream_disable);
> +
> +#endif /* CONFIG_MEDIA_CONTROLLER */
> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> index 150ee16ebd811..dc46812120cdc 100644
> --- a/include/media/v4l2-common.h
> +++ b/include/media/v4l2-common.h
> @@ -539,4 +539,47 @@ static inline void v4l2_buffer_set_timestamp(struct v4l2_buffer *buf,
>  	buf->timestamp.tv_usec = ts.tv_nsec / NSEC_PER_USEC;
>  }
>  
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +
> +/**
> + * v4l2_pipeline_stream_enable - Call .s_stream(true) callbacks in the stream
> + * @vdev: Starting video device.
> + * @pipe: Pipeline this entity belongs to.
> + *
> + * Call .s_stream(true) callback in all the subdevices participating in the
> + * stream, i.e. following links from sink to source.
> + *
> + * .s_stream(true) is also called from sink to source, i.e. in a topology
> + * sd1->sd2->sd3->vdev, .s_stream(true) of sd3 is called first.
> + *
> + * Calls to this function can be nested, in which case the same number of
> + * v4l2_pipeline_stream_disable() calls will be required to disable streaming
> + * through subdevices in the pipeline.
> + * The  pipeline pointer must be identical for all nested calls to
> + * v4l2_pipeline_stream_enable().
> + */
> +__must_check int v4l2_pipeline_stream_enable(struct video_device *vdev,
> +					     struct media_pipeline *pipe);
> +
> +/**
> + * v4l2_pipeline_stream_disable - Call .s_stream(false) callbacks in the stream
> + * @vdev: Starting video device.
> + * @pipe: Pipeline this entity belongs to.
> + *
> + * Call .s_stream(false) callback in all the subdevices participating in the
> + * stream, i.e. following links from sink to source.
> + *
> + * s_stream(false) is called in a reverse order from
> + * v4l2_pipeline_stream_enable(), i.e. in a topology sd1->sd2->sd3->vdev,
> + * .s_stream(false) of sd1 is called first.
> + *
> + * If multiple calls to v4l2_pipeline_stream_enable() have been made, the same
> + * number of calls to this function are required to disable streaming through
> + * subdevices in the pipeline.
> + */
> +void v4l2_pipeline_stream_disable(struct video_device *vdev,
> +				  struct media_pipeline *pipe);
> +
> +#endif /* CONFIG_MEDIA_CONTROLLER */
> +
>  #endif /* V4L2_COMMON_H_ */
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index a4848de598521..20f913a9f70c5 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -838,6 +838,7 @@ struct v4l2_subdev_platform_data {
>   * @subdev_notifier: A sub-device notifier implicitly registered for the sub-
>   *		     device using v4l2_device_register_sensor_subdev().
>   * @pdata: common part of subdevice platform data
> + * @stream_count: Stream count for the subdevice.
>   *
>   * Each instance of a subdev driver should create this struct, either
>   * stand-alone or embedded in a larger struct.
> @@ -869,6 +870,7 @@ struct v4l2_subdev {
>  	struct v4l2_async_notifier *notifier;
>  	struct v4l2_async_notifier *subdev_notifier;
>  	struct v4l2_subdev_platform_data *pdata;
> +	unsigned int stream_count;
>  };
>  
>  
> -- 
> 2.26.0
>
Helen Koike April 27, 2020, 1:49 p.m. UTC | #2
On 4/24/20 10:41 AM, Niklas Söderlund wrote:
> Hi Helen,
> 
> Thanks for your work.
> 
> On 2020-04-14 22:30:42 -0300, Helen Koike wrote:
>> Add v4l2_pipeline_stream_{enable,disable} helper functions to iterate
>> through the subdevices in a given stream (i.e following links from sink
>> to source) and call .s_stream() callback.
>>
>> Add stream_count on the subdevice object for simultaneous streaming from
>> different video devices which shares subdevices.
>>
>> Prevent calling .s_stream(true) if it was already called previously by
>> another stream.
>>
>> Prevent calling .s_stream(false) from one stream when there are still
>> others active.
>>
>> If .s_stream(true) fails, call .s_stream(false) in the reverse order.
>>
>> Signed-off-by: Helen Koike <helen.koike@collabora.com>
>> ---
>>
>> Changes in v3:
>> - re-write helpers to use media walkers as in v1, but with
>> v4l2_pipeline_subdevs_get() to reverse the order we call s_stream(true)
>> in subdevices.
>> - rename size to max_size (and update docs) in v4l2_pipeline_subdevs_get() to
>> reflect the meaning of the argument.
>> - add if defined(CONFIG_MEDIA_CONTROLLER) around helpers
>>
>> Changes in v2:
>> - re-write helpers to not use media walkers
>>
>>  drivers/media/v4l2-core/v4l2-common.c | 125 ++++++++++++++++++++++++++
>>  include/media/v4l2-common.h           |  43 +++++++++
>>  include/media/v4l2-subdev.h           |   2 +
>>  3 files changed, 170 insertions(+)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
>> index 9e8eb45a5b03c..2f991a1a57d7c 100644
>> --- a/drivers/media/v4l2-core/v4l2-common.c
>> +++ b/drivers/media/v4l2-core/v4l2-common.c
>> @@ -442,3 +442,128 @@ int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, u32 pixelformat,
>>  	return 0;
>>  }
>>  EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt);
>> +
>> +#if defined(CONFIG_MEDIA_CONTROLLER)
>> +
>> +/*
>> + * v4l2_pipeline_subdevs_get - Assemble a list of subdevices in a pipeline
>> + * @subdevs: the array to be filled.
>> + * @max_size: max number of elements that can fit in subdevs array.
>> + *
>> + * Walk from a video node, following links from sink to source and fill the
>> + * array with subdevices in the path.
>> + * The walker performs a depth-first traversal, which means that, in a topology
>> + * sd1->sd2->sd3->vdev, sd1 will be the first element placed in the array.
>> + *
>> + * Return the number of subdevices filled in the array.
>> + */
>> +static int v4l2_pipeline_subdevs_get(struct video_device *vdev,
>> +				     struct media_pipeline *pipe,
>> +				     struct v4l2_subdev **subdevs,
>> +				     unsigned int max_size)
>> +{
>> +	struct media_entity *entity = &vdev->entity;
>> +	struct media_device *mdev = entity->graph_obj.mdev;
>> +	int idx = 0;
>> +	int ret;
>> +
>> +	mutex_lock(&mdev->graph_mutex);
>> +
>> +	if (!pipe->streaming_count) {
>> +		ret = media_graph_walk_init(&pipe->graph, mdev);
>> +		if (ret) {
>> +			mutex_unlock(&mdev->graph_mutex);
>> +			return ret;
>> +		}
>> +	}
> 
> This confuses me a bit. Why is this conditional on streaming_count ?  
> Looking how it's used in mc-entity.c the count is also 
> increased/decreased with this pattern.
> 
> Would it make sens to create a local 'struct media_graph graph' here and 
> always init and clean it up ?

media_pipeline_start() initialize the walker by calling media_graph_walk_init() when
pipe->stream_count increases to 1, and it leaves to media_pipeline_stop() to call
media_graph_walk_cleanup() when pipe->streaming_count reaches 0.

So, if this function is called before any call to media_pipeline_start(),  i.e pipe->streaming_count == 0,
it needs to init and clean the walker up, otherwise it shouldn't do it.
If it is called after the last media_pipeline_stop() is called, i.e pipe->streaming_count == 0 again,
it should also init and clean up.

This is what the conditional checks, if the walker is already initialized or not.

But I think I could use a local struct media_graph to have an independent walker from the media_pipeline_{start,stop}(),
instead or reusing the struct media_pipeline that is being given (and I could remove the pipe parameter of these functions).
I was wondering if there was any issue of having an independent walker, but I don't think so, let me do this and re-send the series.

> 
>> +
>> +	media_graph_walk_start(&pipe->graph, entity);
>> +
>> +	while ((entity = media_graph_walk_next_stream(&pipe->graph))) {
>> +		if (!is_media_entity_v4l2_subdev(entity))
>> +			continue;
>> +		if (WARN_ON(idx >= max_size)) {
>> +			mutex_unlock(&mdev->graph_mutex);
>> +			return -EINVAL;
>> +		}
>> +		subdevs[idx++] = media_entity_to_v4l2_subdev(entity);
>> +	}
>> +
>> +	if (!pipe->streaming_count)
>> +		media_graph_walk_cleanup(&pipe->graph);
>> +
>> +	mutex_unlock(&mdev->graph_mutex);
>> +
>> +	return idx;
>> +}
>> +
>> +__must_check int v4l2_pipeline_stream_enable(struct video_device *vdev,
>> +					     struct media_pipeline *pipe)
>> +{
>> +	struct media_device *mdev = vdev->entity.graph_obj.mdev;
>> +	struct v4l2_subdev *subdevs[MEDIA_ENTITY_ENUM_MAX_DEPTH];
>> +	struct v4l2_subdev *sd;
>> +	int i, size, ret;
>> +
>> +	size = v4l2_pipeline_subdevs_get(vdev, pipe,
>> +					 subdevs, ARRAY_SIZE(subdevs));
>> +	if (size <= 0)
>> +		return size;
>> +
>> +	/* Call s_stream() in reverse order to enable sensors last */
>> +	for (i = size - 1; i >= 0; i--) {
>> +		sd = subdevs[i];
>  +		if (sd->stream_count++)
>> +			continue;
>> +		dev_dbg(mdev->dev,
>> +			"enabling stream for '%s'\n", sd->entity.name);
>> +		ret = v4l2_subdev_call(sd, video, s_stream, true);
>> +		if (ret && ret != -ENOIOCTLCMD) {
>> +			sd->stream_count = 0;
>> +			goto err_stream_disable;
>> +		}
>> +	}
>> +
>> +	return 0;
>> +
>> +err_stream_disable:
>> +	for (i++; i < size; i++) {
>> +		sd = subdevs[i];
>> +		if (--sd->stream_count)
>> +			continue;
>> +		dev_dbg(mdev->dev,
>> +			"disabling stream for '%s'\n", sd->entity.name);
>> +		v4l2_subdev_call(sd, video, s_stream, false);
>> +	};
>> +
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(v4l2_pipeline_stream_enable);
>> +
>> +void v4l2_pipeline_stream_disable(struct video_device *vdev,
>> +				  struct media_pipeline *pipe)
>> +{
>> +	struct media_device *mdev = vdev->entity.graph_obj.mdev;
>> +	struct v4l2_subdev *subdevs[MEDIA_ENTITY_ENUM_MAX_DEPTH];
>> +	unsigned int i;
>> +	int size;
>> +
>> +	size = v4l2_pipeline_subdevs_get(vdev, pipe,
>> +					 subdevs, ARRAY_SIZE(subdevs));
>> +	if (WARN_ON(size < 0))
>> +		return;
>> +
>> +	/* Call s_stream() in order to disable sensors first */
>> +	for (i = 0; i < size; i++) {
>> +		struct v4l2_subdev *sd = subdevs[i];
>> +
>> +		if (--sd->stream_count)
>> +			continue;
>> +		dev_dbg(mdev->dev,
>> +			"disabling stream for '%s'\n", sd->entity.name);
>> +		v4l2_subdev_call(sd, video, s_stream, false);
> 
> small nit, maybe this can be extracted to a helper as the code is 
> duplicated here and in the error path v4l2_pipeline_stream_enable ?

ack, I'll see how I can improve this.


Thanks for reviewing!
Helen

> 
>> +	}
>> +}
>> +EXPORT_SYMBOL_GPL(v4l2_pipeline_stream_disable);
>> +
>> +#endif /* CONFIG_MEDIA_CONTROLLER */
>> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
>> index 150ee16ebd811..dc46812120cdc 100644
>> --- a/include/media/v4l2-common.h
>> +++ b/include/media/v4l2-common.h
>> @@ -539,4 +539,47 @@ static inline void v4l2_buffer_set_timestamp(struct v4l2_buffer *buf,
>>  	buf->timestamp.tv_usec = ts.tv_nsec / NSEC_PER_USEC;
>>  }
>>  
>> +#if defined(CONFIG_MEDIA_CONTROLLER)
>> +
>> +/**
>> + * v4l2_pipeline_stream_enable - Call .s_stream(true) callbacks in the stream
>> + * @vdev: Starting video device.
>> + * @pipe: Pipeline this entity belongs to.
>> + *
>> + * Call .s_stream(true) callback in all the subdevices participating in the
>> + * stream, i.e. following links from sink to source.
>> + *
>> + * .s_stream(true) is also called from sink to source, i.e. in a topology
>> + * sd1->sd2->sd3->vdev, .s_stream(true) of sd3 is called first.
>> + *
>> + * Calls to this function can be nested, in which case the same number of
>> + * v4l2_pipeline_stream_disable() calls will be required to disable streaming
>> + * through subdevices in the pipeline.
>> + * The  pipeline pointer must be identical for all nested calls to
>> + * v4l2_pipeline_stream_enable().
>> + */
>> +__must_check int v4l2_pipeline_stream_enable(struct video_device *vdev,
>> +					     struct media_pipeline *pipe);
>> +
>> +/**
>> + * v4l2_pipeline_stream_disable - Call .s_stream(false) callbacks in the stream
>> + * @vdev: Starting video device.
>> + * @pipe: Pipeline this entity belongs to.
>> + *
>> + * Call .s_stream(false) callback in all the subdevices participating in the
>> + * stream, i.e. following links from sink to source.
>> + *
>> + * s_stream(false) is called in a reverse order from
>> + * v4l2_pipeline_stream_enable(), i.e. in a topology sd1->sd2->sd3->vdev,
>> + * .s_stream(false) of sd1 is called first.
>> + *
>> + * If multiple calls to v4l2_pipeline_stream_enable() have been made, the same
>> + * number of calls to this function are required to disable streaming through
>> + * subdevices in the pipeline.
>> + */
>> +void v4l2_pipeline_stream_disable(struct video_device *vdev,
>> +				  struct media_pipeline *pipe);
>> +
>> +#endif /* CONFIG_MEDIA_CONTROLLER */
>> +
>>  #endif /* V4L2_COMMON_H_ */
>> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
>> index a4848de598521..20f913a9f70c5 100644
>> --- a/include/media/v4l2-subdev.h
>> +++ b/include/media/v4l2-subdev.h
>> @@ -838,6 +838,7 @@ struct v4l2_subdev_platform_data {
>>   * @subdev_notifier: A sub-device notifier implicitly registered for the sub-
>>   *		     device using v4l2_device_register_sensor_subdev().
>>   * @pdata: common part of subdevice platform data
>> + * @stream_count: Stream count for the subdevice.
>>   *
>>   * Each instance of a subdev driver should create this struct, either
>>   * stand-alone or embedded in a larger struct.
>> @@ -869,6 +870,7 @@ struct v4l2_subdev {
>>  	struct v4l2_async_notifier *notifier;
>>  	struct v4l2_async_notifier *subdev_notifier;
>>  	struct v4l2_subdev_platform_data *pdata;
>> +	unsigned int stream_count;
>>  };
>>  
>>  
>> -- 
>> 2.26.0
>>
>
Dafna Hirschfeld May 21, 2020, 12:03 p.m. UTC | #3
Hi

On 15.04.20 03:30, Helen Koike wrote:
> Add v4l2_pipeline_stream_{enable,disable} helper functions to iterate
> through the subdevices in a given stream (i.e following links from sink
> to source) and call .s_stream() callback.
> 
> Add stream_count on the subdevice object for simultaneous streaming from
> different video devices which shares subdevices.
> 
> Prevent calling .s_stream(true) if it was already called previously by
> another stream.
> 
> Prevent calling .s_stream(false) from one stream when there are still
> others active.
> 
> If .s_stream(true) fails, call .s_stream(false) in the reverse order.
> 
> Signed-off-by: Helen Koike <helen.koike@collabora.com>
> ---
> 
> Changes in v3:
> - re-write helpers to use media walkers as in v1, but with
> v4l2_pipeline_subdevs_get() to reverse the order we call s_stream(true)
> in subdevices.
> - rename size to max_size (and update docs) in v4l2_pipeline_subdevs_get() to
> reflect the meaning of the argument.
> - add if defined(CONFIG_MEDIA_CONTROLLER) around helpers
> 
> Changes in v2:
> - re-write helpers to not use media walkers
> 
>   drivers/media/v4l2-core/v4l2-common.c | 125 ++++++++++++++++++++++++++
>   include/media/v4l2-common.h           |  43 +++++++++
>   include/media/v4l2-subdev.h           |   2 +
>   3 files changed, 170 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index 9e8eb45a5b03c..2f991a1a57d7c 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -442,3 +442,128 @@ int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, u32 pixelformat,
>   	return 0;
>   }
>   EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt);
> +
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +
> +/*
> + * v4l2_pipeline_subdevs_get - Assemble a list of subdevices in a pipeline
> + * @subdevs: the array to be filled.
> + * @max_size: max number of elements that can fit in subdevs array.
> + *
> + * Walk from a video node, following links from sink to source and fill the
> + * array with subdevices in the path.
> + * The walker performs a depth-first traversal, which means that, in a topology
> + * sd1->sd2->sd3->vdev, sd1 will be the first element placed in the array.
> + *
> + * Return the number of subdevices filled in the array.
> + */
> +static int v4l2_pipeline_subdevs_get(struct video_device *vdev,
> +				     struct media_pipeline *pipe,
> +				     struct v4l2_subdev **subdevs,
> +				     unsigned int max_size)
> +{
> +	struct media_entity *entity = &vdev->entity;
> +	struct media_device *mdev = entity->graph_obj.mdev;
> +	int idx = 0;
> +	int ret;
> +
> +	mutex_lock(&mdev->graph_mutex);
> +
> +	if (!pipe->streaming_count) {
> +		ret = media_graph_walk_init(&pipe->graph, mdev);
> +		if (ret) {
> +			mutex_unlock(&mdev->graph_mutex);
> +			return ret;
> +		}
> +	}
> +
> +	media_graph_walk_start(&pipe->graph, entity);
> +
> +	while ((entity = media_graph_walk_next_stream(&pipe->graph))) {
> +		if (!is_media_entity_v4l2_subdev(entity))
> +			continue;
> +		if (WARN_ON(idx >= max_size)) {
> +			mutex_unlock(&mdev->graph_mutex);
> +			return -EINVAL;
> +		}
> +		subdevs[idx++] = media_entity_to_v4l2_subdev(entity);
> +	}
> +
> +	if (!pipe->streaming_count)
> +		media_graph_walk_cleanup(&pipe->graph);
> +
> +	mutex_unlock(&mdev->graph_mutex);
> +
> +	return idx;
> +}
> +
> +__must_check int v4l2_pipeline_stream_enable(struct video_device *vdev,
> +					     struct media_pipeline *pipe)
> +{
> +	struct media_device *mdev = vdev->entity.graph_obj.mdev;
> +	struct v4l2_subdev *subdevs[MEDIA_ENTITY_ENUM_MAX_DEPTH];
> +	struct v4l2_subdev *sd;
> +	int i, size, ret;
> +
> +	size = v4l2_pipeline_subdevs_get(vdev, pipe,
> +					 subdevs, ARRAY_SIZE(subdevs));
> +	if (size <= 0)
> +		return size;
> +
> +	/* Call s_stream() in reverse order to enable sensors last */
> +	for (i = size - 1; i >= 0; i--) {
> +		sd = subdevs[i];
> +		if (sd->stream_count++)
> +			continue;
> +		dev_dbg(mdev->dev,
> +			"enabling stream for '%s'\n", sd->entity.name);
> +		ret = v4l2_subdev_call(sd, video, s_stream, true);
> +		if (ret && ret != -ENOIOCTLCMD) {
> +			sd->stream_count = 0;
> +			goto err_stream_disable;
> +		}
> +	}
> +
> +	return 0;
> +
> +err_stream_disable:
> +	for (i++; i < size; i++) {
> +		sd = subdevs[i];
> +		if (--sd->stream_count)
> +			continue;
> +		dev_dbg(mdev->dev,
> +			"disabling stream for '%s'\n", sd->entity.name);
> +		v4l2_subdev_call(sd, video, s_stream, false);
> +	};
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_pipeline_stream_enable);
> +
> +void v4l2_pipeline_stream_disable(struct video_device *vdev,
> +				  struct media_pipeline *pipe)
> +{
> +	struct media_device *mdev = vdev->entity.graph_obj.mdev;
> +	struct v4l2_subdev *subdevs[MEDIA_ENTITY_ENUM_MAX_DEPTH];
> +	unsigned int i;
> +	int size;
> +
> +	size = v4l2_pipeline_subdevs_get(vdev, pipe,
> +					 subdevs, ARRAY_SIZE(subdevs));
> +	if (WARN_ON(size < 0))
> +		return;
> +
> +	/* Call s_stream() in order to disable sensors first */
> +	for (i = 0; i < size; i++) {
> +		struct v4l2_subdev *sd = subdevs[i];
> +
> +		if (--sd->stream_count)
> +			continue;
> +		dev_dbg(mdev->dev,
> +			"disabling stream for '%s'\n", sd->entity.name);
> +		v4l2_subdev_call(sd, video, s_stream, false);
> +	}
> +}
> +EXPORT_SYMBOL_GPL(v4l2_pipeline_stream_disable);
> +
> +#endif /* CONFIG_MEDIA_CONTROLLER */
> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> index 150ee16ebd811..dc46812120cdc 100644
> --- a/include/media/v4l2-common.h
> +++ b/include/media/v4l2-common.h
> @@ -539,4 +539,47 @@ static inline void v4l2_buffer_set_timestamp(struct v4l2_buffer *buf,
>   	buf->timestamp.tv_usec = ts.tv_nsec / NSEC_PER_USEC;
>   }
>   
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +
> +/**
> + * v4l2_pipeline_stream_enable - Call .s_stream(true) callbacks in the stream
> + * @vdev: Starting video device.
> + * @pipe: Pipeline this entity belongs to.
> + *
> + * Call .s_stream(true) callback in all the subdevices participating in the
> + * stream, i.e. following links from sink to source.
> + *
> + * .s_stream(true) is also called from sink to source, i.e. in a topology
> + * sd1->sd2->sd3->vdev, .s_stream(true) of sd3 is called first.
> + *
> + * Calls to this function can be nested, in which case the same number of
> + * v4l2_pipeline_stream_disable() calls will be required to disable streaming
> + * through subdevices in the pipeline.
> + * The  pipeline pointer must be identical for all nested calls to
> + * v4l2_pipeline_stream_enable().
> + */
> +__must_check int v4l2_pipeline_stream_enable(struct video_device *vdev,
> +					     struct media_pipeline *pipe);
> +
> +/**
> + * v4l2_pipeline_stream_disable - Call .s_stream(false) callbacks in the stream
> + * @vdev: Starting video device.
> + * @pipe: Pipeline this entity belongs to.
> + *
> + * Call .s_stream(false) callback in all the subdevices participating in the
> + * stream, i.e. following links from sink to source.
> + *
> + * s_stream(false) is called in a reverse order from
> + * v4l2_pipeline_stream_enable(), i.e. in a topology sd1->sd2->sd3->vdev,
> + * .s_stream(false) of sd1 is called first.
> + *
> + * If multiple calls to v4l2_pipeline_stream_enable() have been made, the same
> + * number of calls to this function are required to disable streaming through
> + * subdevices in the pipeline.
> + */
> +void v4l2_pipeline_stream_disable(struct video_device *vdev,
> +				  struct media_pipeline *pipe);
> +
> +#endif /* CONFIG_MEDIA_CONTROLLER */
> +
>   #endif /* V4L2_COMMON_H_ */
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index a4848de598521..20f913a9f70c5 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -838,6 +838,7 @@ struct v4l2_subdev_platform_data {
>    * @subdev_notifier: A sub-device notifier implicitly registered for the sub-
>    *		     device using v4l2_device_register_sensor_subdev().
>    * @pdata: common part of subdevice platform data
> + * @stream_count: Stream count for the subdevice.
>    *
>    * Each instance of a subdev driver should create this struct, either
>    * stand-alone or embedded in a larger struct.
> @@ -869,6 +870,7 @@ struct v4l2_subdev {
>   	struct v4l2_async_notifier *notifier;
>   	struct v4l2_async_notifier *subdev_notifier;
>   	struct v4l2_subdev_platform_data *pdata;
> +	unsigned int stream_count;
I wonder if it is worth it to add a new field to v4l2_subdev that is used only
for the specific case, it seems that except of rkisp1 and vimc, the other drivers you pointed to that
could use the helpers don't need this counter and they call s_stream without checking if it was already called.
This counter is updated only by the new introduced functions and not when s_stream is called from other places.
Maybe we can add a helper that just return the next subdevice connected to the current entity as a source, and drivers can
iterate it and can decide themselves if s_stream should be called or not

Thanks,
Dafna

>   };
>   
>   
>
Helen Koike May 21, 2020, 12:53 p.m. UTC | #4
Hi Dafna,

On 5/21/20 9:03 AM, Dafna Hirschfeld wrote:
> Hi
> 
> On 15.04.20 03:30, Helen Koike wrote:
>> Add v4l2_pipeline_stream_{enable,disable} helper functions to iterate
>> through the subdevices in a given stream (i.e following links from sink
>> to source) and call .s_stream() callback.
>>
>> Add stream_count on the subdevice object for simultaneous streaming from
>> different video devices which shares subdevices.
>>
>> Prevent calling .s_stream(true) if it was already called previously by
>> another stream.
>>
>> Prevent calling .s_stream(false) from one stream when there are still
>> others active.
>>
>> If .s_stream(true) fails, call .s_stream(false) in the reverse order.
>>
>> Signed-off-by: Helen Koike <helen.koike@collabora.com>
>> ---
>>
>> Changes in v3:
>> - re-write helpers to use media walkers as in v1, but with
>> v4l2_pipeline_subdevs_get() to reverse the order we call s_stream(true)
>> in subdevices.
>> - rename size to max_size (and update docs) in v4l2_pipeline_subdevs_get() to
>> reflect the meaning of the argument.
>> - add if defined(CONFIG_MEDIA_CONTROLLER) around helpers
>>
>> Changes in v2:
>> - re-write helpers to not use media walkers
>>
>>   drivers/media/v4l2-core/v4l2-common.c | 125 ++++++++++++++++++++++++++
>>   include/media/v4l2-common.h           |  43 +++++++++
>>   include/media/v4l2-subdev.h           |   2 +
>>   3 files changed, 170 insertions(+)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
>> index 9e8eb45a5b03c..2f991a1a57d7c 100644
>> --- a/drivers/media/v4l2-core/v4l2-common.c
>> +++ b/drivers/media/v4l2-core/v4l2-common.c
>> @@ -442,3 +442,128 @@ int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, u32 pixelformat,
>>       return 0;
>>   }
>>   EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt);
>> +
>> +#if defined(CONFIG_MEDIA_CONTROLLER)
>> +
>> +/*
>> + * v4l2_pipeline_subdevs_get - Assemble a list of subdevices in a pipeline
>> + * @subdevs: the array to be filled.
>> + * @max_size: max number of elements that can fit in subdevs array.
>> + *
>> + * Walk from a video node, following links from sink to source and fill the
>> + * array with subdevices in the path.
>> + * The walker performs a depth-first traversal, which means that, in a topology
>> + * sd1->sd2->sd3->vdev, sd1 will be the first element placed in the array.
>> + *
>> + * Return the number of subdevices filled in the array.
>> + */
>> +static int v4l2_pipeline_subdevs_get(struct video_device *vdev,
>> +                     struct media_pipeline *pipe,
>> +                     struct v4l2_subdev **subdevs,
>> +                     unsigned int max_size)
>> +{
>> +    struct media_entity *entity = &vdev->entity;
>> +    struct media_device *mdev = entity->graph_obj.mdev;
>> +    int idx = 0;
>> +    int ret;
>> +
>> +    mutex_lock(&mdev->graph_mutex);
>> +
>> +    if (!pipe->streaming_count) {
>> +        ret = media_graph_walk_init(&pipe->graph, mdev);
>> +        if (ret) {
>> +            mutex_unlock(&mdev->graph_mutex);
>> +            return ret;
>> +        }
>> +    }
>> +
>> +    media_graph_walk_start(&pipe->graph, entity);
>> +
>> +    while ((entity = media_graph_walk_next_stream(&pipe->graph))) {
>> +        if (!is_media_entity_v4l2_subdev(entity))
>> +            continue;
>> +        if (WARN_ON(idx >= max_size)) {
>> +            mutex_unlock(&mdev->graph_mutex);
>> +            return -EINVAL;
>> +        }
>> +        subdevs[idx++] = media_entity_to_v4l2_subdev(entity);
>> +    }
>> +
>> +    if (!pipe->streaming_count)
>> +        media_graph_walk_cleanup(&pipe->graph);
>> +
>> +    mutex_unlock(&mdev->graph_mutex);
>> +
>> +    return idx;
>> +}
>> +
>> +__must_check int v4l2_pipeline_stream_enable(struct video_device *vdev,
>> +                         struct media_pipeline *pipe)
>> +{
>> +    struct media_device *mdev = vdev->entity.graph_obj.mdev;
>> +    struct v4l2_subdev *subdevs[MEDIA_ENTITY_ENUM_MAX_DEPTH];
>> +    struct v4l2_subdev *sd;
>> +    int i, size, ret;
>> +
>> +    size = v4l2_pipeline_subdevs_get(vdev, pipe,
>> +                     subdevs, ARRAY_SIZE(subdevs));
>> +    if (size <= 0)
>> +        return size;
>> +
>> +    /* Call s_stream() in reverse order to enable sensors last */
>> +    for (i = size - 1; i >= 0; i--) {
>> +        sd = subdevs[i];
>> +        if (sd->stream_count++)
>> +            continue;
>> +        dev_dbg(mdev->dev,
>> +            "enabling stream for '%s'\n", sd->entity.name);
>> +        ret = v4l2_subdev_call(sd, video, s_stream, true);
>> +        if (ret && ret != -ENOIOCTLCMD) {
>> +            sd->stream_count = 0;
>> +            goto err_stream_disable;
>> +        }
>> +    }
>> +
>> +    return 0;
>> +
>> +err_stream_disable:
>> +    for (i++; i < size; i++) {
>> +        sd = subdevs[i];
>> +        if (--sd->stream_count)
>> +            continue;
>> +        dev_dbg(mdev->dev,
>> +            "disabling stream for '%s'\n", sd->entity.name);
>> +        v4l2_subdev_call(sd, video, s_stream, false);
>> +    };
>> +
>> +    return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(v4l2_pipeline_stream_enable);
>> +
>> +void v4l2_pipeline_stream_disable(struct video_device *vdev,
>> +                  struct media_pipeline *pipe)
>> +{
>> +    struct media_device *mdev = vdev->entity.graph_obj.mdev;
>> +    struct v4l2_subdev *subdevs[MEDIA_ENTITY_ENUM_MAX_DEPTH];
>> +    unsigned int i;
>> +    int size;
>> +
>> +    size = v4l2_pipeline_subdevs_get(vdev, pipe,
>> +                     subdevs, ARRAY_SIZE(subdevs));
>> +    if (WARN_ON(size < 0))
>> +        return;
>> +
>> +    /* Call s_stream() in order to disable sensors first */
>> +    for (i = 0; i < size; i++) {
>> +        struct v4l2_subdev *sd = subdevs[i];
>> +
>> +        if (--sd->stream_count)
>> +            continue;
>> +        dev_dbg(mdev->dev,
>> +            "disabling stream for '%s'\n", sd->entity.name);
>> +        v4l2_subdev_call(sd, video, s_stream, false);
>> +    }
>> +}
>> +EXPORT_SYMBOL_GPL(v4l2_pipeline_stream_disable);
>> +
>> +#endif /* CONFIG_MEDIA_CONTROLLER */
>> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
>> index 150ee16ebd811..dc46812120cdc 100644
>> --- a/include/media/v4l2-common.h
>> +++ b/include/media/v4l2-common.h
>> @@ -539,4 +539,47 @@ static inline void v4l2_buffer_set_timestamp(struct v4l2_buffer *buf,
>>       buf->timestamp.tv_usec = ts.tv_nsec / NSEC_PER_USEC;
>>   }
>>   +#if defined(CONFIG_MEDIA_CONTROLLER)
>> +
>> +/**
>> + * v4l2_pipeline_stream_enable - Call .s_stream(true) callbacks in the stream
>> + * @vdev: Starting video device.
>> + * @pipe: Pipeline this entity belongs to.
>> + *
>> + * Call .s_stream(true) callback in all the subdevices participating in the
>> + * stream, i.e. following links from sink to source.
>> + *
>> + * .s_stream(true) is also called from sink to source, i.e. in a topology
>> + * sd1->sd2->sd3->vdev, .s_stream(true) of sd3 is called first.
>> + *
>> + * Calls to this function can be nested, in which case the same number of
>> + * v4l2_pipeline_stream_disable() calls will be required to disable streaming
>> + * through subdevices in the pipeline.
>> + * The  pipeline pointer must be identical for all nested calls to
>> + * v4l2_pipeline_stream_enable().
>> + */
>> +__must_check int v4l2_pipeline_stream_enable(struct video_device *vdev,
>> +                         struct media_pipeline *pipe);
>> +
>> +/**
>> + * v4l2_pipeline_stream_disable - Call .s_stream(false) callbacks in the stream
>> + * @vdev: Starting video device.
>> + * @pipe: Pipeline this entity belongs to.
>> + *
>> + * Call .s_stream(false) callback in all the subdevices participating in the
>> + * stream, i.e. following links from sink to source.
>> + *
>> + * s_stream(false) is called in a reverse order from
>> + * v4l2_pipeline_stream_enable(), i.e. in a topology sd1->sd2->sd3->vdev,
>> + * .s_stream(false) of sd1 is called first.
>> + *
>> + * If multiple calls to v4l2_pipeline_stream_enable() have been made, the same
>> + * number of calls to this function are required to disable streaming through
>> + * subdevices in the pipeline.
>> + */
>> +void v4l2_pipeline_stream_disable(struct video_device *vdev,
>> +                  struct media_pipeline *pipe);
>> +
>> +#endif /* CONFIG_MEDIA_CONTROLLER */
>> +
>>   #endif /* V4L2_COMMON_H_ */
>> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
>> index a4848de598521..20f913a9f70c5 100644
>> --- a/include/media/v4l2-subdev.h
>> +++ b/include/media/v4l2-subdev.h
>> @@ -838,6 +838,7 @@ struct v4l2_subdev_platform_data {
>>    * @subdev_notifier: A sub-device notifier implicitly registered for the sub-
>>    *             device using v4l2_device_register_sensor_subdev().
>>    * @pdata: common part of subdevice platform data
>> + * @stream_count: Stream count for the subdevice.
>>    *
>>    * Each instance of a subdev driver should create this struct, either
>>    * stand-alone or embedded in a larger struct.
>> @@ -869,6 +870,7 @@ struct v4l2_subdev {
>>       struct v4l2_async_notifier *notifier;
>>       struct v4l2_async_notifier *subdev_notifier;
>>       struct v4l2_subdev_platform_data *pdata;
>> +    unsigned int stream_count;
> I wonder if it is worth it to add a new field to v4l2_subdev that is used only
> for the specific case, it seems that except of rkisp1 and vimc, the other drivers you pointed to that
> could use the helpers don't need this counter and they call s_stream without checking if it was already called.
> This counter is updated only by the new introduced functions and not when s_stream is called from other places.

Other drivers implement their own loop that navigates through the pipeline calling .s_stream().
They are very similar. The only difference from rkisp1/vimc is that they don't support simultaneous streaming from different
capture nodes.

Also, drivers usually don't handle errors very well, in cases of failures where .s_stream() should be called in reversed order.

Adding this counter + helpers allows a common generic implementation that can be used for both cases.
So we re-use a code that should be well tested in the core and avoid re-implementing it.

> Maybe we can add a helper that just return the next subdevice connected to the current entity as a source, and drivers can
> iterate it and can decide themselves if s_stream should be called or not

Patch 1/4 in the series provides this iterator :)
Unless if you think we could add helpers to make things easier.

Regards,
Helen

> 
> Thanks,
> Dafna
> 
>>   };
>>
diff mbox series

Patch

diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
index 9e8eb45a5b03c..2f991a1a57d7c 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -442,3 +442,128 @@  int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, u32 pixelformat,
 	return 0;
 }
 EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt);
+
+#if defined(CONFIG_MEDIA_CONTROLLER)
+
+/*
+ * v4l2_pipeline_subdevs_get - Assemble a list of subdevices in a pipeline
+ * @subdevs: the array to be filled.
+ * @max_size: max number of elements that can fit in subdevs array.
+ *
+ * Walk from a video node, following links from sink to source and fill the
+ * array with subdevices in the path.
+ * The walker performs a depth-first traversal, which means that, in a topology
+ * sd1->sd2->sd3->vdev, sd1 will be the first element placed in the array.
+ *
+ * Return the number of subdevices filled in the array.
+ */
+static int v4l2_pipeline_subdevs_get(struct video_device *vdev,
+				     struct media_pipeline *pipe,
+				     struct v4l2_subdev **subdevs,
+				     unsigned int max_size)
+{
+	struct media_entity *entity = &vdev->entity;
+	struct media_device *mdev = entity->graph_obj.mdev;
+	int idx = 0;
+	int ret;
+
+	mutex_lock(&mdev->graph_mutex);
+
+	if (!pipe->streaming_count) {
+		ret = media_graph_walk_init(&pipe->graph, mdev);
+		if (ret) {
+			mutex_unlock(&mdev->graph_mutex);
+			return ret;
+		}
+	}
+
+	media_graph_walk_start(&pipe->graph, entity);
+
+	while ((entity = media_graph_walk_next_stream(&pipe->graph))) {
+		if (!is_media_entity_v4l2_subdev(entity))
+			continue;
+		if (WARN_ON(idx >= max_size)) {
+			mutex_unlock(&mdev->graph_mutex);
+			return -EINVAL;
+		}
+		subdevs[idx++] = media_entity_to_v4l2_subdev(entity);
+	}
+
+	if (!pipe->streaming_count)
+		media_graph_walk_cleanup(&pipe->graph);
+
+	mutex_unlock(&mdev->graph_mutex);
+
+	return idx;
+}
+
+__must_check int v4l2_pipeline_stream_enable(struct video_device *vdev,
+					     struct media_pipeline *pipe)
+{
+	struct media_device *mdev = vdev->entity.graph_obj.mdev;
+	struct v4l2_subdev *subdevs[MEDIA_ENTITY_ENUM_MAX_DEPTH];
+	struct v4l2_subdev *sd;
+	int i, size, ret;
+
+	size = v4l2_pipeline_subdevs_get(vdev, pipe,
+					 subdevs, ARRAY_SIZE(subdevs));
+	if (size <= 0)
+		return size;
+
+	/* Call s_stream() in reverse order to enable sensors last */
+	for (i = size - 1; i >= 0; i--) {
+		sd = subdevs[i];
+		if (sd->stream_count++)
+			continue;
+		dev_dbg(mdev->dev,
+			"enabling stream for '%s'\n", sd->entity.name);
+		ret = v4l2_subdev_call(sd, video, s_stream, true);
+		if (ret && ret != -ENOIOCTLCMD) {
+			sd->stream_count = 0;
+			goto err_stream_disable;
+		}
+	}
+
+	return 0;
+
+err_stream_disable:
+	for (i++; i < size; i++) {
+		sd = subdevs[i];
+		if (--sd->stream_count)
+			continue;
+		dev_dbg(mdev->dev,
+			"disabling stream for '%s'\n", sd->entity.name);
+		v4l2_subdev_call(sd, video, s_stream, false);
+	};
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(v4l2_pipeline_stream_enable);
+
+void v4l2_pipeline_stream_disable(struct video_device *vdev,
+				  struct media_pipeline *pipe)
+{
+	struct media_device *mdev = vdev->entity.graph_obj.mdev;
+	struct v4l2_subdev *subdevs[MEDIA_ENTITY_ENUM_MAX_DEPTH];
+	unsigned int i;
+	int size;
+
+	size = v4l2_pipeline_subdevs_get(vdev, pipe,
+					 subdevs, ARRAY_SIZE(subdevs));
+	if (WARN_ON(size < 0))
+		return;
+
+	/* Call s_stream() in order to disable sensors first */
+	for (i = 0; i < size; i++) {
+		struct v4l2_subdev *sd = subdevs[i];
+
+		if (--sd->stream_count)
+			continue;
+		dev_dbg(mdev->dev,
+			"disabling stream for '%s'\n", sd->entity.name);
+		v4l2_subdev_call(sd, video, s_stream, false);
+	}
+}
+EXPORT_SYMBOL_GPL(v4l2_pipeline_stream_disable);
+
+#endif /* CONFIG_MEDIA_CONTROLLER */
diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
index 150ee16ebd811..dc46812120cdc 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -539,4 +539,47 @@  static inline void v4l2_buffer_set_timestamp(struct v4l2_buffer *buf,
 	buf->timestamp.tv_usec = ts.tv_nsec / NSEC_PER_USEC;
 }
 
+#if defined(CONFIG_MEDIA_CONTROLLER)
+
+/**
+ * v4l2_pipeline_stream_enable - Call .s_stream(true) callbacks in the stream
+ * @vdev: Starting video device.
+ * @pipe: Pipeline this entity belongs to.
+ *
+ * Call .s_stream(true) callback in all the subdevices participating in the
+ * stream, i.e. following links from sink to source.
+ *
+ * .s_stream(true) is also called from sink to source, i.e. in a topology
+ * sd1->sd2->sd3->vdev, .s_stream(true) of sd3 is called first.
+ *
+ * Calls to this function can be nested, in which case the same number of
+ * v4l2_pipeline_stream_disable() calls will be required to disable streaming
+ * through subdevices in the pipeline.
+ * The  pipeline pointer must be identical for all nested calls to
+ * v4l2_pipeline_stream_enable().
+ */
+__must_check int v4l2_pipeline_stream_enable(struct video_device *vdev,
+					     struct media_pipeline *pipe);
+
+/**
+ * v4l2_pipeline_stream_disable - Call .s_stream(false) callbacks in the stream
+ * @vdev: Starting video device.
+ * @pipe: Pipeline this entity belongs to.
+ *
+ * Call .s_stream(false) callback in all the subdevices participating in the
+ * stream, i.e. following links from sink to source.
+ *
+ * s_stream(false) is called in a reverse order from
+ * v4l2_pipeline_stream_enable(), i.e. in a topology sd1->sd2->sd3->vdev,
+ * .s_stream(false) of sd1 is called first.
+ *
+ * If multiple calls to v4l2_pipeline_stream_enable() have been made, the same
+ * number of calls to this function are required to disable streaming through
+ * subdevices in the pipeline.
+ */
+void v4l2_pipeline_stream_disable(struct video_device *vdev,
+				  struct media_pipeline *pipe);
+
+#endif /* CONFIG_MEDIA_CONTROLLER */
+
 #endif /* V4L2_COMMON_H_ */
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index a4848de598521..20f913a9f70c5 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -838,6 +838,7 @@  struct v4l2_subdev_platform_data {
  * @subdev_notifier: A sub-device notifier implicitly registered for the sub-
  *		     device using v4l2_device_register_sensor_subdev().
  * @pdata: common part of subdevice platform data
+ * @stream_count: Stream count for the subdevice.
  *
  * Each instance of a subdev driver should create this struct, either
  * stand-alone or embedded in a larger struct.
@@ -869,6 +870,7 @@  struct v4l2_subdev {
 	struct v4l2_async_notifier *notifier;
 	struct v4l2_async_notifier *subdev_notifier;
 	struct v4l2_subdev_platform_data *pdata;
+	unsigned int stream_count;
 };