Re: [PATCH v2 3/8] hantro: Use v4l2_m2m_buf_done_and_job_finish

[Hans Verkuil]

On 3/18/20 2:21 PM, Ezequiel Garcia wrote:
> Let the core sort out the nuances of returning buffers
> to userspace, by using the v4l2_m2m_buf_done_and_job_finish
> helper.
> 
> This change also removes usage of buffer sequence fields,
> which shouldn't have any meaning for stateless decoders.

Uh, why remove this? For one, doesn't this cause fails in v4l2-compliance?
Also, while I agree that it is not terribly useful, it doesn't hurt, does it?

And the V4L2 spec makes no exception for stateless codecs with respect to the
sequence field.

Nacked-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>

Regards,

	Hans

> 
> Signed-off-by: Ezequiel Garcia <ezequiel@xxxxxxxxxxxxx>
> ---
>  drivers/staging/media/hantro/hantro_drv.c | 27 ++++++++---------------
>  1 file changed, 9 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
> index 0b1200fc0e1a..ec889d755cd6 100644
> --- a/drivers/staging/media/hantro/hantro_drv.c
> +++ b/drivers/staging/media/hantro/hantro_drv.c
> @@ -94,32 +94,23 @@ static void hantro_job_finish(struct hantro_dev *vpu,
>  			      unsigned int bytesused,
>  			      enum vb2_buffer_state result)
>  {
> -	struct vb2_v4l2_buffer *src, *dst;
>  	int ret;
>  
>  	pm_runtime_mark_last_busy(vpu->dev);
>  	pm_runtime_put_autosuspend(vpu->dev);
>  	clk_bulk_disable(vpu->variant->num_clocks, vpu->clocks);
>  
> -	src = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> -	dst = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> -
> -	if (WARN_ON(!src))
> -		return;
> -	if (WARN_ON(!dst))
> -		return;
> -
> -	src->sequence = ctx->sequence_out++;
> -	dst->sequence = ctx->sequence_cap++;
> -
> -	ret = ctx->buf_finish(ctx, &dst->vb2_buf, bytesused);
> -	if (ret)
> -		result = VB2_BUF_STATE_ERROR;
> +	if (ctx->buf_finish) {
> +		struct vb2_v4l2_buffer *dst;
>  
> -	v4l2_m2m_buf_done(src, result);
> -	v4l2_m2m_buf_done(dst, result);
> +		dst = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
> +		ret = ctx->buf_finish(ctx, &dst->vb2_buf, bytesused);
> +		if (ret)
> +			result = VB2_BUF_STATE_ERROR;
> +	}
>  
> -	v4l2_m2m_job_finish(vpu->m2m_dev, ctx->fh.m2m_ctx);
> +	v4l2_m2m_buf_done_and_job_finish(ctx->dev->m2m_dev, ctx->fh.m2m_ctx,
> +					 result);
>  }
>  
>  void hantro_irq_done(struct hantro_dev *vpu, unsigned int bytesused,
>