Re: [PATCH v2 13/14] media: staging: rkisp1: call media_pipeline_start/stop from stats and params

[Helen Koike <helen.koike@collabora.com>]

Hi Dafna,

On 8/15/20 7:37 AM, Dafna Hirschfeld wrote:
> The stats and params video nodes register a media entity
> and therefore they need to call media_pipeline_start/stop.

I'm not sure this is really required, iirc, these functions
basically locks the topology, which would cause EBUSY errors
when trying to modify the topology.
I can be wrong, but I don't think params
and stats requires locking the topology configuration.

Regards,
Helen

> 
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> ---
>  drivers/staging/media/rkisp1/rkisp1-params.c | 9 +++++++++
>  drivers/staging/media/rkisp1/rkisp1-stats.c  | 9 +++++++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/drivers/staging/media/rkisp1/rkisp1-params.c b/drivers/staging/media/rkisp1/rkisp1-params.c
> index cc242ad5106e..7d5c21fa630e 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-params.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-params.c
> @@ -1488,14 +1488,23 @@ static void rkisp1_params_vb2_stop_streaming(struct vb2_queue *vq)
>  
>  	list_for_each_entry(buf, &tmp_list, queue)
>  		vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
> +
> +	media_pipeline_stop(&params->vnode.vdev.entity);
>  }
>  
>  static int
>  rkisp1_params_vb2_start_streaming(struct vb2_queue *queue, unsigned int count)
>  {
>  	struct rkisp1_params *params = queue->drv_priv;
> +	struct media_entity *entity = &params->vnode.vdev.entity;
>  	unsigned long flags;
> +	int ret;
>  
> +	ret = media_pipeline_start(entity, &params->rkisp1->pipe);
> +	if (ret) {
> +		dev_err(params->rkisp1->dev, "params: start pipeline failed %d\n", ret);
> +		return ret;
> +	}
>  	spin_lock_irqsave(&params->config_lock, flags);
>  	params->is_streaming = true;
>  	spin_unlock_irqrestore(&params->config_lock, flags);
> diff --git a/drivers/staging/media/rkisp1/rkisp1-stats.c b/drivers/staging/media/rkisp1/rkisp1-stats.c
> index c86beba709db..b314fb6a4562 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-stats.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-stats.c
> @@ -150,12 +150,21 @@ static void rkisp1_stats_vb2_stop_streaming(struct vb2_queue *vq)
>  		vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
>  	}
>  	spin_unlock_irq(&stats->lock);
> +	media_pipeline_stop(&stats->vnode.vdev.entity);
>  }
>  
>  static int
>  rkisp1_stats_vb2_start_streaming(struct vb2_queue *queue, unsigned int count)
>  {
>  	struct rkisp1_stats *stats = queue->drv_priv;
> +	struct media_entity *entity = &stats->vnode.vdev.entity;
> +	int ret;
> +
> +	ret = media_pipeline_start(entity, &stats->rkisp1->pipe);
> +	if (ret) {
> +		dev_err(stats->rkisp1->dev, "stats: start pipeline failed %d\n", ret);
> +		return ret;
> +	}
>  
>  	spin_lock_irq(&stats->lock);
>  	stats->is_streaming = true;
>