Re: [PATCH 7/8] media: vimc: stream: init/terminate the first entity

[Andrà Almeida]

On 3/6/19 7:42 PM, Helen Koike wrote:
> The s_stream callback was not being called for the first entity in the
> stream pipeline array.
> Instead of verifying the type of the node (video or subdevice) and
> calling s_stream from the second entity in the pipeline, do this process
> for all the entities in the pipeline for consistency.
>
> The previous code was not a problem because the first entity is a video
> device and not a subdevice, but this patch prepares vimc to allow
> setting some configuration in the entity before calling s_stream.
>
> Signed-off-by: Helen Koike <helen.koike@xxxxxxxxxxxxx>
Hello,

I've tested the stream (with the default media bus format and using an 
alternative one) using a custom user space application and tested all 
capture entities using qv4l2 -d /dev/videoX (where X is each of the 
capture entities). Also, I have checked v4l2-compliance -d /dev/videoX 
and with v4l2-compliance --streaming=5 -d /dev/videoX and no regressions 
where found.

Tested-by: Andrà Almeida <andre.almeida@xxxxxxxxxxxxx>
> ---
>
>   drivers/media/platform/vimc/vimc-streamer.c | 25 ++++++++++++---------
>   1 file changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/media/platform/vimc/vimc-streamer.c b/drivers/media/platform/vimc/vimc-streamer.c
> index b7c1fdef5f0d..5a3bda62fbc8 100644
> --- a/drivers/media/platform/vimc/vimc-streamer.c
> +++ b/drivers/media/platform/vimc/vimc-streamer.c
> @@ -46,19 +46,18 @@ static struct media_entity *vimc_get_source_entity(struct media_entity *ent)
>    */
>   static void vimc_streamer_pipeline_terminate(struct vimc_stream *stream)
>   {
> -	struct media_entity *entity;
> +	struct vimc_ent_device *ved;
>   	struct v4l2_subdev *sd;
>   
>   	while (stream->pipe_size) {
>   		stream->pipe_size--;
> -		entity = stream->ved_pipeline[stream->pipe_size]->ent;
> -		entity = vimc_get_source_entity(entity);
> +		ved = stream->ved_pipeline[stream->pipe_size];
>   		stream->ved_pipeline[stream->pipe_size] = NULL;
>   
> -		if (!is_media_entity_v4l2_subdev(entity))
> +		if (!is_media_entity_v4l2_subdev(ved->ent))
>   			continue;
>   
> -		sd = media_entity_to_v4l2_subdev(entity);
> +		sd = media_entity_to_v4l2_subdev(ved->ent);
>   		v4l2_subdev_call(sd, video, s_stream, 0);
>   	}
>   }
> @@ -89,18 +88,24 @@ static int vimc_streamer_pipeline_init(struct vimc_stream *stream,
>   		}
>   		stream->ved_pipeline[stream->pipe_size++] = ved;
>   
> +		if (is_media_entity_v4l2_subdev(ved->ent)) {
> +			sd = media_entity_to_v4l2_subdev(ved->ent);
> +			ret = v4l2_subdev_call(sd, video, s_stream, 1);
> +			if (ret && ret != -ENOIOCTLCMD) {
> +				pr_err("subdev_call error %s\n", ved->ent->name);
> +				vimc_streamer_pipeline_terminate(stream);
> +				return ret;
> +			}
> +		}
> +
>   		entity = vimc_get_source_entity(ved->ent);
>   		/* Check if the end of the pipeline was reached*/
>   		if (!entity)
>   			return 0;
>   
> +		/* Get the next device in the pipeline */
>   		if (is_media_entity_v4l2_subdev(entity)) {
>   			sd = media_entity_to_v4l2_subdev(entity);
> -			ret = v4l2_subdev_call(sd, video, s_stream, 1);
> -			if (ret && ret != -ENOIOCTLCMD) {
> -				vimc_streamer_pipeline_terminate(stream);
> -				return ret;
> -			}
>   			ved = v4l2_get_subdevdata(sd);
>   		} else {
>   			vdev = container_of(entity,