[PATCH 4/4] media: staging: rkisp1: cap: in stream start, replace calls to rkisp1_handle_buffer with rkisp1_set_next_buf

[Helen Koike]

Hi Dafna,

Thanks for the patch, just a small thing below.

On 7/14/20 9:38 AM, Dafna Hirschfeld wrote:
> The function 'rkisp1_stream_start' calls 'rkisp1_handle_buffer'
> in order to update the 'buf.curr' and 'buf.next' fields and
> configure the device before streaming starts. This cause a wrong
> increment of the debugs field 'frame_drop'. This patch replaces
> the call to 'rkisp1_handle_buffer' with a call to
> 'rkisp1_set_next_buf'.
> 
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld at collabora.com>
> ---
>  drivers/staging/media/rkisp1/rkisp1-capture.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c
> index 7f400aefe550..c05280950ea0 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-capture.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-capture.c
> @@ -916,7 +916,7 @@ static void rkisp1_stream_start(struct rkisp1_capture *cap)
>  	cap->ops->config(cap);
>  
>  	/* Setup a buffer for the next frame */
> -	rkisp1_handle_buffer(cap);
> +	rkisp1_set_next_buf(cap);

You should also protect with the cap->buf.lock, or move the lock protection to rkisp1_set_next_buf() and update patch 2/4.

Regards,
Helen

>  	cap->ops->enable(cap);
>  	/* It's safe to config ACTIVE and SHADOW regs for the
>  	 * first stream. While when the second is starting, do NOT
> @@ -931,7 +931,7 @@ static void rkisp1_stream_start(struct rkisp1_capture *cap)
>  		/* force cfg update */
>  		rkisp1_write(rkisp1,
>  			     RKISP1_CIF_MI_INIT_SOFT_UPD, RKISP1_CIF_MI_INIT);
> -		rkisp1_handle_buffer(cap);
> +		rkisp1_set_next_buf(cap);
>  	}
>  	cap->is_streaming = true;
>  }
>