Re: [PATCH 2/3 v2] vb2: Add support for non monotonic timestamps

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

Hi Kamil,

Thank you for the patch.

On Thursday 24 January 2013 13:35:06 Kamil Debski wrote:
> Not all drivers use monotonic timestamps. This patch adds a way to set the
> timestamp type per every queue.
> 
> Signed-off-by: Kamil Debski <k.debski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/media/v4l2-core/videobuf2-core.c |    8 ++++++--
>  include/media/videobuf2-core.h           |    1 +
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
> b/drivers/media/v4l2-core/videobuf2-core.c index 85e3c22..b816689 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -403,7 +403,7 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb,
> struct v4l2_buffer *b) * Clear any buffer state related flags.
>  	 */
>  	b->flags &= ~V4L2_BUFFER_MASK_FLAGS;
> -	b->flags |= V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> +	b->flags |= q->timestamp_type;
> 
>  	switch (vb->state) {
>  	case VB2_BUF_STATE_QUEUED:
> @@ -2032,9 +2032,13 @@ int vb2_queue_init(struct vb2_queue *q)
>  	    WARN_ON(!q->type)		  ||
>  	    WARN_ON(!q->io_modes)	  ||
>  	    WARN_ON(!q->ops->queue_setup) ||
> -	    WARN_ON(!q->ops->buf_queue))
> +	    WARN_ON(!q->ops->buf_queue)   ||
> +	    WARN_ON(q->timestamp_type & ~V4L2_BUF_FLAG_TIMESTAMP_MASK))
>  		return -EINVAL;
> 
> +	/* Warn that the driver should choose an appropriate timestamp type */
> +	WARN_ON(q->timestamp_type == V4L2_BUF_FLAG_TIMESTAMP_UNKNOWN);
> +

This will cause all the drivers that use vb2 to issue a WARN_ON, and 
timestamps reported as monotonic in v3.7 would then be reported as unknown 
again.

I can see two options to fix this, one is to default to monotonic if the 
timestamp type is unknown, the other is to patch all drivers that use vb2. The 
former is probably easier.

>  	INIT_LIST_HEAD(&q->queued_list);
>  	INIT_LIST_HEAD(&q->done_list);
>  	spin_lock_init(&q->done_lock);
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index 9cfd4ee..7ce4656 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -326,6 +326,7 @@ struct vb2_queue {
>  	const struct vb2_mem_ops	*mem_ops;
>  	void				*drv_priv;
>  	unsigned int			buf_struct_size;
> +	u32	                   	timestamp_type;
> 
>  /* private: internal use only */
>  	enum v4l2_memory		memory;

-- 
Regards,

Laurent Pinchart