Re: [PATCH v6 01/18] media: mc-entity: Record number of video devices in a pipeline

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

Hi Dan,

Thank you for the patch.

On Tue, Jul 09, 2024 at 02:28:49PM +0100, Daniel Scally wrote:
> Record the number of video devices in a pipeline so that we can track
> in a central location how many need to be started before drivers must
> actually begin streaming.
> 
> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> ---
> Changes in v6:
> 
> 	- New patch. This is intended to support Sakari's requirement for the
> 	  driver not to start streaming before all of the video devices have
> 	  called streamon(). This was the cleanest way I could think to acheive
> 	  the goal, and lets us just check for start_count == required_count
> 	  before streaming.

This violates the abstraction layers a bit, the MC code isn't supposed
to handle video devices like that.

What you can instead do is to use media_pipeline_for_each_entity() in
your driver when starting the pipeline to iterate over all entities, and
count the video devices there.

> 
>  drivers/media/mc/mc-entity.c | 5 +++++
>  include/media/media-entity.h | 2 ++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
> index 96dd0f6ccd0d..1e8186b13b55 100644
> --- a/drivers/media/mc/mc-entity.c
> +++ b/drivers/media/mc/mc-entity.c
> @@ -596,6 +596,9 @@ static int media_pipeline_add_pad(struct media_pipeline *pipe,
>  
>  	list_add_tail(&ppad->list, &pipe->pads);
>  
> +	if (pad->entity->obj_type == MEDIA_ENTITY_TYPE_VIDEO_DEVICE)
> +		pipe->required_count++;
> +
>  	dev_dbg(pad->graph_obj.mdev->dev,
>  		"media pipeline: added pad '%s':%u\n",
>  		pad->entity->name, pad->index);
> @@ -713,6 +716,8 @@ static void media_pipeline_cleanup(struct media_pipeline *pipe)
>  		list_del(&ppad->list);
>  		kfree(ppad);
>  	}
> +
> +	pipe->required_count = 0;
>  }
>  
>  static int media_pipeline_populate(struct media_pipeline *pipe,
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index 0393b23129eb..ab84458b40dc 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -104,12 +104,14 @@ struct media_graph {
>   * @mdev:		The media device the pipeline is part of
>   * @pads:		List of media_pipeline_pad
>   * @start_count:	Media pipeline start - stop count
> + * @required_count:	Number of starts required to be "running"
>   */
>  struct media_pipeline {
>  	bool allocated;
>  	struct media_device *mdev;
>  	struct list_head pads;
>  	int start_count;
> +	int required_count;
>  };
>  
>  /**

-- 
Regards,

Laurent Pinchart