Re: [PATCH v3 32/38] media: ti-vpe: cal: use CSI-2 frame number

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

Hi Tomi,

Thank you for the patch.

On Mon, May 24, 2021 at 02:09:03PM +0300, Tomi Valkeinen wrote:
> The driver fills buf->vb.sequence with an increasing number which is
> incremented by the driver. This feels a bit pointless, as the userspace
> could as well track that kind of number itself. Instead, lets use the

s/lets/let's/

> frame number provided in the CSI-2 data from the sensor.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  drivers/media/platform/ti-vpe/cal.c | 7 +++++--
>  drivers/media/platform/ti-vpe/cal.h | 1 -
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
> index 888706187fd1..62c45add4efe 100644
> --- a/drivers/media/platform/ti-vpe/cal.c
> +++ b/drivers/media/platform/ti-vpe/cal.c
> @@ -493,7 +493,6 @@ void cal_ctx_unprepare(struct cal_ctx *ctx)
>  
>  void cal_ctx_start(struct cal_ctx *ctx)
>  {
> -	ctx->sequence = 0;
>  	ctx->dma.state = CAL_DMA_RUNNING;
>  
>  	/* Configure the CSI-2, pixel processing and write DMA contexts. */
> @@ -586,6 +585,10 @@ static inline void cal_irq_wdma_start(struct cal_ctx *ctx)
>  static inline void cal_irq_wdma_end(struct cal_ctx *ctx)
>  {
>  	struct cal_buffer *buf = NULL;
> +	u32 frame_num;
> +
> +	frame_num = cal_read(ctx->cal, CAL_CSI2_STATUS(ctx->phy->instance,
> +						       ctx->csi2_ctx)) & 0xffff;
>  
>  	spin_lock(&ctx->dma.lock);
>  
> @@ -607,7 +610,7 @@ static inline void cal_irq_wdma_end(struct cal_ctx *ctx)
>  	if (buf) {
>  		buf->vb.vb2_buf.timestamp = ktime_get_ns();
>  		buf->vb.field = ctx->v_fmt.fmt.pix.field;
> -		buf->vb.sequence = ctx->sequence++;
> +		buf->vb.sequence = frame_num;

We'll need something a bit more complicated. The CSI-2 frame number is
not mandatory, and when used, it is a 16-bit number starting at 1 and
counting to an unspecified value larger than one, resetting to 1 at the
end of the cycle. The V4L2 sequence number, on the other hand, is a
monotonic counter starting at 0 and wrapping only at 2^32-1. We should
thus keep a software sequence counter and

- increase it by 1 if the frame number is zero
- increase it by frame_num - last_frame_num (with wrap-around of
  frame_num handled) otherwise

>  		vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_DONE);
>  	}
>  }
> diff --git a/drivers/media/platform/ti-vpe/cal.h b/drivers/media/platform/ti-vpe/cal.h
> index 400f95485d7c..ad08c189ad3b 100644
> --- a/drivers/media/platform/ti-vpe/cal.h
> +++ b/drivers/media/platform/ti-vpe/cal.h
> @@ -217,7 +217,6 @@ struct cal_ctx {
>  	const struct cal_format_info	**active_fmt;
>  	unsigned int		num_active_fmt;
>  
> -	unsigned int		sequence;
>  	struct vb2_queue	vb_vidq;
>  	u8			dma_ctx;
>  	u8			cport;

-- 
Regards,

Laurent Pinchart