Re: [PATCH v5 07/10] media: Improve enable_streams and disable_streams documentation

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Sakari,

Thank you for the patch.

On Mon, Jun 08, 2026 at 12:53:53AM +0300, Sakari Ailus wrote:
> Document that enable_streams may start additional streams and
> disable_streams may not disable requested streams if other related streams
> are still enabled.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> Reviewed-by: Mirela Rabulea <mirela.rabulea@nxp.com>
> ---
>  include/media/v4l2-subdev.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index 0361c8bbee38..a25234befbe9 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -828,6 +828,10 @@ struct v4l2_subdev_state {
>   *	V4L2_SUBDEV_CAP_STREAMS sub-device capability flag can ignore the mask
>   *	argument.
>   *
> + *	Starting the requested streams may require starting additional
> + *	streams. Streams that are started and stopped together due to hardware
> + *	are called a stream group.

This is too vague. "may require starting additional streams" can be
understood as the driver starting more streams than the requested ones,
or as the caller having to start more streams before the requests
streams are effectively started.

Here's a proposal to clarify the documentation:

 * Due to device constraints, starting the requested streams may result in
 * additional streams also being started by the driver. Streams that are started
 * and stopped together due to the nature of the hardware are called a stream
 * group.

"hardware" may be a bit restrictive, I wonder if those constraints could
also sometimes come from the software implementation instead of being
pure hardware constraints. I think it's fine for now though, we can
clarify this later if needed.

> + *
>   * @disable_streams: Disable the streams defined in streams_mask on the given
>   *	source pad. Subdevs that implement this operation must use the active
>   *	state management provided by the subdev core (enabled through a call to
> @@ -837,6 +841,10 @@ struct v4l2_subdev_state {
>   *	Drivers that support only a single stream without setting the
>   *	V4L2_SUBDEV_CAP_STREAMS sub-device capability flag can ignore the mask
>   *	argument.
> + *
> + *	Stopping the requested streams may be delayed due to other enabled
> + *	streams in the stream group, where all streams are started and stopped
> + *	together.

This is better. I would have written

 * When the requested stream are part of a stream group, they will be stopped
 * once all streams in the group are stopped.

to make it clearer that the streams won't be stopped immediately ("may"
in the original text implies they may or may not be stopped
immediately), but I'm also OK with your text.

>   */
>  struct v4l2_subdev_pad_ops {
>  	int (*enum_mbus_code)(struct v4l2_subdev *sd,

-- 
Regards,

Laurent Pinchart