[PATCH 0/3 v2] Add proper timestamp types handling in videobuf2

[PATCH 0/3 v2] Add proper timestamp types handling in videobuf2

[Kamil Debski]

Hi,

This is the second version of the patch posted earlier this month.
After the discussion a WARN_ON was added to inform if the driver is not setting
timestamp type when initialising the videobuf2 queue. Also the
davinci/vpbe_display.c driver was modified to correctly report the use of
MONOTONIC timestamp type.

Best wishes,
Kamil Debski

PS. Below please find the original cover letter.

Hi,

The recent addition of timestamp types (and monotonic timestamp) left some room
for improvement. First of all not all drivers use monotonic timestamp. There are
for example mem2mem drivers that copy the timestamp from the OUTPUT buffer to
the corresponding CAPTURE buffer. Some videobuf2 drivers do not fill the
timestamp field altogether (yeah, I can agree that a constant is monotonic, but
still...).

Hence, I propose the following change to videobuf2. After applying this patch
the default timestamp type is UNKNOWN. It is up to the driver to set the
timestamp type to either MONOTONIC or COPY in vb2_queue_init.

This patch also adds setting proper timestamp type value in case of drivers
where I determined that type. This list might be missing some drivers, but
in these cases it will leave the UNKNOWN type which is a safe assumption.

Best wishes,
Kamil Debski

Kamil Debski (3):
  v4l: Define video buffer flag for the COPY timestamp type
  vb2: Add support for non monotonic timestamps
  v4l: Set proper timestamp type in selected drivers which use
    videobuf2

 Documentation/DocBook/media/v4l/io.xml             |    6 ++++++
 drivers/media/platform/blackfin/bfin_capture.c     |    1 +
 drivers/media/platform/davinci/vpbe_display.c      |    1 +
 drivers/media/platform/davinci/vpif_capture.c      |    1 +
 drivers/media/platform/davinci/vpif_display.c      |    1 +
 drivers/media/platform/s3c-camif/camif-capture.c   |    1 +
 drivers/media/platform/s5p-fimc/fimc-capture.c     |    1 +
 drivers/media/platform/s5p-fimc/fimc-lite.c        |    1 +
 drivers/media/platform/s5p-mfc/s5p_mfc.c           |    2 ++
 drivers/media/platform/soc_camera/atmel-isi.c      |    1 +
 drivers/media/platform/soc_camera/mx2_camera.c     |    1 +
 drivers/media/platform/soc_camera/mx3_camera.c     |    1 +
 .../platform/soc_camera/sh_mobile_ceu_camera.c     |    1 +
 drivers/media/platform/vivi.c                      |    1 +
 drivers/media/usb/pwc/pwc-if.c                     |    1 +
 drivers/media/usb/stk1160/stk1160-v4l.c            |    1 +
 drivers/media/usb/uvc/uvc_queue.c                  |    1 +
 drivers/media/v4l2-core/videobuf2-core.c           |    8 ++++++--
 include/media/videobuf2-core.h                     |    1 +
 include/uapi/linux/videodev2.h                     |    1 +
 20 files changed, 31 insertions(+), 2 deletions(-)

-- 
1.7.9.5

[PATCH 1/3 v2] v4l: Define video buffer flag for the COPY timestamp type

[Kamil Debski]

Define video buffer flag for the COPY timestamp. In this case the timestamp
value is copied from the OUTPUT to the corresponding CAPTURE buffer.

Signed-off-by: Kamil Debski <k.debski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 Documentation/DocBook/media/v4l/io.xml |    6 ++++++
 include/uapi/linux/videodev2.h         |    1 +
 2 files changed, 7 insertions(+)

diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
index 73f202f..fdd1822 100644
--- a/Documentation/DocBook/media/v4l/io.xml
+++ b/Documentation/DocBook/media/v4l/io.xml
@@ -1145,6 +1145,12 @@ in which case caches have not been used.</entry>
 	    same clock outside V4L2, use
 	    <function>clock_gettime(2)</function> .</entry>
 	  </row>
+	  <row>
+	    <entry><constant>V4L2_BUF_FLAG_TIMESTAMP_COPY</constant></entry>
+	    <entry>0x4000</entry>
+	    <entry>The CAPTURE buffer timestamp has been taken from the
+	    corresponding OUTPUT buffer.</entry>
+	  </row>
 	</tbody>
       </tgroup>
     </table>
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 72e9921..d5a59af 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -697,6 +697,7 @@ struct v4l2_buffer {
 #define V4L2_BUF_FLAG_TIMESTAMP_MASK		0xe000
 #define V4L2_BUF_FLAG_TIMESTAMP_UNKNOWN		0x0000
 #define V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC	0x2000
+#define V4L2_BUF_FLAG_TIMESTAMP_COPY		0x4000
 
 /**
  * struct v4l2_exportbuffer - export of video buffer as DMABUF file descriptor
-- 
1.7.9.5

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

[Kamil Debski]

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);
+
 	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;
-- 
1.7.9.5

[PATCH 3/3 v2] v4l: Set proper timestamp type in selected drivers which use videobuf2

[Kamil Debski]

Set proper timestamp type in drivers that I am sure that use either
MONOTONIC or COPY timestamps. Other drivers will correctly report
UNKNOWN timestamp type instead of assuming that all drivers use monotonic
timestamps.

Signed-off-by: Kamil Debski <k.debski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/blackfin/bfin_capture.c     |    1 +
 drivers/media/platform/davinci/vpbe_display.c      |    1 +
 drivers/media/platform/davinci/vpif_capture.c      |    1 +
 drivers/media/platform/davinci/vpif_display.c      |    1 +
 drivers/media/platform/s3c-camif/camif-capture.c   |    1 +
 drivers/media/platform/s5p-fimc/fimc-capture.c     |    1 +
 drivers/media/platform/s5p-fimc/fimc-lite.c        |    1 +
 drivers/media/platform/s5p-mfc/s5p_mfc.c           |    2 ++
 drivers/media/platform/soc_camera/atmel-isi.c      |    1 +
 drivers/media/platform/soc_camera/mx2_camera.c     |    1 +
 drivers/media/platform/soc_camera/mx3_camera.c     |    1 +
 .../platform/soc_camera/sh_mobile_ceu_camera.c     |    1 +
 drivers/media/platform/vivi.c                      |    1 +
 drivers/media/usb/pwc/pwc-if.c                     |    1 +
 drivers/media/usb/stk1160/stk1160-v4l.c            |    1 +
 drivers/media/usb/uvc/uvc_queue.c                  |    1 +
 16 files changed, 17 insertions(+)

diff --git a/drivers/media/platform/blackfin/bfin_capture.c b/drivers/media/platform/blackfin/bfin_capture.c
index d422d3c..365d6ef 100644
--- a/drivers/media/platform/blackfin/bfin_capture.c
+++ b/drivers/media/platform/blackfin/bfin_capture.c
@@ -939,6 +939,7 @@ static int __devinit bcap_probe(struct platform_device *pdev)
 	q->buf_struct_size = sizeof(struct bcap_buffer);
 	q->ops = &bcap_video_qops;
 	q->mem_ops = &vb2_dma_contig_memops;
+	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
 
 	vb2_queue_init(q);
 
diff --git a/drivers/media/platform/davinci/vpbe_display.c b/drivers/media/platform/davinci/vpbe_display.c
index 2bfde79..fa03482 100644
--- a/drivers/media/platform/davinci/vpbe_display.c
+++ b/drivers/media/platform/davinci/vpbe_display.c
@@ -1405,6 +1405,7 @@ static int vpbe_display_reqbufs(struct file *file, void *priv,
 	q->ops = &video_qops;
 	q->mem_ops = &vb2_dma_contig_memops;
 	q->buf_struct_size = sizeof(struct vpbe_disp_buffer);
+	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
 
 	ret = vb2_queue_init(q);
 	if (ret) {
diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index 5892d2b..1943e41 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -1035,6 +1035,7 @@ static int vpif_reqbufs(struct file *file, void *priv,
 	q->ops = &video_qops;
 	q->mem_ops = &vb2_dma_contig_memops;
 	q->buf_struct_size = sizeof(struct vpif_cap_buffer);
+	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
 
 	ret = vb2_queue_init(q);
 	if (ret) {
diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c
index dd249c9..5477c2c 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -1001,6 +1001,7 @@ static int vpif_reqbufs(struct file *file, void *priv,
 	q->ops = &video_qops;
 	q->mem_ops = &vb2_dma_contig_memops;
 	q->buf_struct_size = sizeof(struct vpif_disp_buffer);
+	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
 
 	ret = vb2_queue_init(q);
 	if (ret) {
diff --git a/drivers/media/platform/s3c-camif/camif-capture.c b/drivers/media/platform/s3c-camif/camif-capture.c
index a55793c..e91f350 100644
--- a/drivers/media/platform/s3c-camif/camif-capture.c
+++ b/drivers/media/platform/s3c-camif/camif-capture.c
@@ -1153,6 +1153,7 @@ int s3c_camif_register_video_node(struct camif_dev *camif, int idx)
 	q->mem_ops = &vb2_dma_contig_memops;
 	q->buf_struct_size = sizeof(struct camif_buffer);
 	q->drv_priv = vp;
+	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
 
 	ret = vb2_queue_init(q);
 	if (ret)
diff --git a/drivers/media/platform/s5p-fimc/fimc-capture.c b/drivers/media/platform/s5p-fimc/fimc-capture.c
index ddd689b..02cfb2b 100644
--- a/drivers/media/platform/s5p-fimc/fimc-capture.c
+++ b/drivers/media/platform/s5p-fimc/fimc-capture.c
@@ -1747,6 +1747,7 @@ static int fimc_register_capture_device(struct fimc_dev *fimc,
 	q->ops = &fimc_capture_qops;
 	q->mem_ops = &vb2_dma_contig_memops;
 	q->buf_struct_size = sizeof(struct fimc_vid_buffer);
+	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
 
 	ret = vb2_queue_init(q);
 	if (ret)
diff --git a/drivers/media/platform/s5p-fimc/fimc-lite.c b/drivers/media/platform/s5p-fimc/fimc-lite.c
index 1b309a7..39ea893 100644
--- a/drivers/media/platform/s5p-fimc/fimc-lite.c
+++ b/drivers/media/platform/s5p-fimc/fimc-lite.c
@@ -1251,6 +1251,7 @@ static int fimc_lite_subdev_registered(struct v4l2_subdev *sd)
 	q->mem_ops = &vb2_dma_contig_memops;
 	q->buf_struct_size = sizeof(struct flite_buffer);
 	q->drv_priv = fimc;
+	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
 
 	ret = vb2_queue_init(q);
 	if (ret < 0)
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index a527f85..30b4d15 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -807,6 +807,7 @@ static int s5p_mfc_open(struct file *file)
 		goto err_queue_init;
 	}
 	q->mem_ops = (struct vb2_mem_ops *)&vb2_dma_contig_memops;
+	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
 	ret = vb2_queue_init(q);
 	if (ret) {
 		mfc_err("Failed to initialize videobuf2 queue(capture)\n");
@@ -828,6 +829,7 @@ static int s5p_mfc_open(struct file *file)
 		goto err_queue_init;
 	}
 	q->mem_ops = (struct vb2_mem_ops *)&vb2_dma_contig_memops;
+	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
 	ret = vb2_queue_init(q);
 	if (ret) {
 		mfc_err("Failed to initialize videobuf2 queue(output)\n");
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
index c8d748a..e531b82 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -514,6 +514,7 @@ static int isi_camera_init_videobuf(struct vb2_queue *q,
 	q->buf_struct_size = sizeof(struct frame_buffer);
 	q->ops = &isi_video_qops;
 	q->mem_ops = &vb2_dma_contig_memops;
+	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
 
 	return vb2_queue_init(q);
 }
diff --git a/drivers/media/platform/soc_camera/mx2_camera.c b/drivers/media/platform/soc_camera/mx2_camera.c
index 5fbac4f..5ff6a5d 100644
--- a/drivers/media/platform/soc_camera/mx2_camera.c
+++ b/drivers/media/platform/soc_camera/mx2_camera.c
@@ -1022,6 +1022,7 @@ static int mx2_camera_init_videobuf(struct vb2_queue *q,
 	q->ops = &mx2_videobuf_ops;
 	q->mem_ops = &vb2_dma_contig_memops;
 	q->buf_struct_size = sizeof(struct mx2_buffer);
+	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
 
 	return vb2_queue_init(q);
 }
diff --git a/drivers/media/platform/soc_camera/mx3_camera.c b/drivers/media/platform/soc_camera/mx3_camera.c
index 574d125..abe9db6 100644
--- a/drivers/media/platform/soc_camera/mx3_camera.c
+++ b/drivers/media/platform/soc_camera/mx3_camera.c
@@ -455,6 +455,7 @@ static int mx3_camera_init_videobuf(struct vb2_queue *q,
 	q->ops = &mx3_videobuf_ops;
 	q->mem_ops = &vb2_dma_contig_memops;
 	q->buf_struct_size = sizeof(struct mx3_camera_buffer);
+	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
 
 	return vb2_queue_init(q);
 }
diff --git a/drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c b/drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c
index 8a6d58d..b6c1c97 100644
--- a/drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c
+++ b/drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c
@@ -2026,6 +2026,7 @@ static int sh_mobile_ceu_init_videobuf(struct vb2_queue *q,
 	q->ops = &sh_mobile_ceu_videobuf_ops;
 	q->mem_ops = &vb2_dma_contig_memops;
 	q->buf_struct_size = sizeof(struct sh_mobile_ceu_buffer);
+	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
 
 	return vb2_queue_init(q);
 }
diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c
index c2f424f..86a5432 100644
--- a/drivers/media/platform/vivi.c
+++ b/drivers/media/platform/vivi.c
@@ -1305,6 +1305,7 @@ static int __init vivi_create_instance(int inst)
 	q->buf_struct_size = sizeof(struct vivi_buffer);
 	q->ops = &vivi_video_qops;
 	q->mem_ops = &vb2_vmalloc_memops;
+	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
 
 	ret = vb2_queue_init(q);
 	if (ret)
diff --git a/drivers/media/usb/pwc/pwc-if.c b/drivers/media/usb/pwc/pwc-if.c
index 21c1523..1b65e0c 100644
--- a/drivers/media/usb/pwc/pwc-if.c
+++ b/drivers/media/usb/pwc/pwc-if.c
@@ -1001,6 +1001,7 @@ static int usb_pwc_probe(struct usb_interface *intf, const struct usb_device_id
 	pdev->vb_queue.buf_struct_size = sizeof(struct pwc_frame_buf);
 	pdev->vb_queue.ops = &pwc_vb_queue_ops;
 	pdev->vb_queue.mem_ops = &vb2_vmalloc_memops;
+	pdev->vb_queue.timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
 	rc = vb2_queue_init(&pdev->vb_queue);
 	if (rc < 0) {
 		PWC_ERROR("Oops, could not initialize vb2 queue.\n");
diff --git a/drivers/media/usb/stk1160/stk1160-v4l.c b/drivers/media/usb/stk1160/stk1160-v4l.c
index 6694f9e..5307a63 100644
--- a/drivers/media/usb/stk1160/stk1160-v4l.c
+++ b/drivers/media/usb/stk1160/stk1160-v4l.c
@@ -687,6 +687,7 @@ int stk1160_vb2_setup(struct stk1160 *dev)
 	q->buf_struct_size = sizeof(struct stk1160_buffer);
 	q->ops = &stk1160_video_qops;
 	q->mem_ops = &vb2_vmalloc_memops;
+	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
 
 	rc = vb2_queue_init(q);
 	if (rc < 0)
diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c
index 778addc..82d01d8 100644
--- a/drivers/media/usb/uvc/uvc_queue.c
+++ b/drivers/media/usb/uvc/uvc_queue.c
@@ -133,6 +133,7 @@ int uvc_queue_init(struct uvc_video_queue *queue, enum v4l2_buf_type type,
 	queue->queue.buf_struct_size = sizeof(struct uvc_buffer);
 	queue->queue.ops = &uvc_queue_qops;
 	queue->queue.mem_ops = &vb2_vmalloc_memops;
+	queue->queue.timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
 	ret = vb2_queue_init(&queue->queue);
 	if (ret)
 		return ret;
-- 
1.7.9.5

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

[Laurent Pinchart]

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

Re: [PATCH 1/3 v2] v4l: Define video buffer flag for the COPY timestamp type

[Hans Verkuil]

On Thu 24 January 2013 13:35:05 Kamil Debski wrote:
> Define video buffer flag for the COPY timestamp. In this case the timestamp
> value is copied from the OUTPUT to the corresponding CAPTURE buffer.
> 
> Signed-off-by: Kamil Debski <k.debski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  Documentation/DocBook/media/v4l/io.xml |    6 ++++++
>  include/uapi/linux/videodev2.h         |    1 +
>  2 files changed, 7 insertions(+)
> 
> diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
> index 73f202f..fdd1822 100644
> --- a/Documentation/DocBook/media/v4l/io.xml
> +++ b/Documentation/DocBook/media/v4l/io.xml
> @@ -1145,6 +1145,12 @@ in which case caches have not been used.</entry>
>  	    same clock outside V4L2, use
>  	    <function>clock_gettime(2)</function> .</entry>
>  	  </row>
> +	  <row>
> +	    <entry><constant>V4L2_BUF_FLAG_TIMESTAMP_COPY</constant></entry>
> +	    <entry>0x4000</entry>
> +	    <entry>The CAPTURE buffer timestamp has been taken from the
> +	    corresponding OUTPUT buffer.</entry>
> +	  </row>

I think I mentioned it before somewhere, but it is helpful if it is mentioned
here that this only applies to mem2mem devices.

Regards,

	Hans

>  	</tbody>
>        </tgroup>
>      </table>
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 72e9921..d5a59af 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -697,6 +697,7 @@ struct v4l2_buffer {
>  #define V4L2_BUF_FLAG_TIMESTAMP_MASK		0xe000
>  #define V4L2_BUF_FLAG_TIMESTAMP_UNKNOWN		0x0000
>  #define V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC	0x2000
> +#define V4L2_BUF_FLAG_TIMESTAMP_COPY		0x4000
>  
>  /**
>   * struct v4l2_exportbuffer - export of video buffer as DMABUF file descriptor
> 

Re: [PATCH 3/3 v2] v4l: Set proper timestamp type in selected drivers which use videobuf2

[Laurent Pinchart]

Hi Kamil,

Thanks for the patch.

On Thursday 24 January 2013 13:35:07 Kamil Debski wrote:
> Set proper timestamp type in drivers that I am sure that use either
> MONOTONIC or COPY timestamps. Other drivers will correctly report
> UNKNOWN timestamp type instead of assuming that all drivers use monotonic
> timestamps.

I've replied to 2/3 before seeing this patch, sorry (although the reply is 
still valid from a bisection point of view).

Do you have a list of those other drivers using vb2 that will report an 
unknown timestamp type ?

> Signed-off-by: Kamil Debski <k.debski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/media/platform/blackfin/bfin_capture.c     |    1 +
>  drivers/media/platform/davinci/vpbe_display.c      |    1 +
>  drivers/media/platform/davinci/vpif_capture.c      |    1 +
>  drivers/media/platform/davinci/vpif_display.c      |    1 +
>  drivers/media/platform/s3c-camif/camif-capture.c   |    1 +
>  drivers/media/platform/s5p-fimc/fimc-capture.c     |    1 +
>  drivers/media/platform/s5p-fimc/fimc-lite.c        |    1 +
>  drivers/media/platform/s5p-mfc/s5p_mfc.c           |    2 ++
>  drivers/media/platform/soc_camera/atmel-isi.c      |    1 +
>  drivers/media/platform/soc_camera/mx2_camera.c     |    1 +
>  drivers/media/platform/soc_camera/mx3_camera.c     |    1 +
>  .../platform/soc_camera/sh_mobile_ceu_camera.c     |    1 +
>  drivers/media/platform/vivi.c                      |    1 +
>  drivers/media/usb/pwc/pwc-if.c                     |    1 +
>  drivers/media/usb/stk1160/stk1160-v4l.c            |    1 +
>  drivers/media/usb/uvc/uvc_queue.c                  |    1 +
>  16 files changed, 17 insertions(+)
> 
> diff --git a/drivers/media/platform/blackfin/bfin_capture.c
> b/drivers/media/platform/blackfin/bfin_capture.c index d422d3c..365d6ef
> 100644
> --- a/drivers/media/platform/blackfin/bfin_capture.c
> +++ b/drivers/media/platform/blackfin/bfin_capture.c
> @@ -939,6 +939,7 @@ static int __devinit bcap_probe(struct platform_device
> *pdev) q->buf_struct_size = sizeof(struct bcap_buffer);
>  	q->ops = &bcap_video_qops;
>  	q->mem_ops = &vb2_dma_contig_memops;
> +	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> 
>  	vb2_queue_init(q);
> 
> diff --git a/drivers/media/platform/davinci/vpbe_display.c
> b/drivers/media/platform/davinci/vpbe_display.c index 2bfde79..fa03482
> 100644
> --- a/drivers/media/platform/davinci/vpbe_display.c
> +++ b/drivers/media/platform/davinci/vpbe_display.c
> @@ -1405,6 +1405,7 @@ static int vpbe_display_reqbufs(struct file *file,
> void *priv, q->ops = &video_qops;
>  	q->mem_ops = &vb2_dma_contig_memops;
>  	q->buf_struct_size = sizeof(struct vpbe_disp_buffer);
> +	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> 
>  	ret = vb2_queue_init(q);
>  	if (ret) {
> diff --git a/drivers/media/platform/davinci/vpif_capture.c
> b/drivers/media/platform/davinci/vpif_capture.c index 5892d2b..1943e41
> 100644
> --- a/drivers/media/platform/davinci/vpif_capture.c
> +++ b/drivers/media/platform/davinci/vpif_capture.c
> @@ -1035,6 +1035,7 @@ static int vpif_reqbufs(struct file *file, void *priv,
> q->ops = &video_qops;
>  	q->mem_ops = &vb2_dma_contig_memops;
>  	q->buf_struct_size = sizeof(struct vpif_cap_buffer);
> +	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> 
>  	ret = vb2_queue_init(q);
>  	if (ret) {
> diff --git a/drivers/media/platform/davinci/vpif_display.c
> b/drivers/media/platform/davinci/vpif_display.c index dd249c9..5477c2c
> 100644
> --- a/drivers/media/platform/davinci/vpif_display.c
> +++ b/drivers/media/platform/davinci/vpif_display.c
> @@ -1001,6 +1001,7 @@ static int vpif_reqbufs(struct file *file, void *priv,
> q->ops = &video_qops;
>  	q->mem_ops = &vb2_dma_contig_memops;
>  	q->buf_struct_size = sizeof(struct vpif_disp_buffer);
> +	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> 
>  	ret = vb2_queue_init(q);
>  	if (ret) {
> diff --git a/drivers/media/platform/s3c-camif/camif-capture.c
> b/drivers/media/platform/s3c-camif/camif-capture.c index a55793c..e91f350
> 100644
> --- a/drivers/media/platform/s3c-camif/camif-capture.c
> +++ b/drivers/media/platform/s3c-camif/camif-capture.c
> @@ -1153,6 +1153,7 @@ int s3c_camif_register_video_node(struct camif_dev
> *camif, int idx) q->mem_ops = &vb2_dma_contig_memops;
>  	q->buf_struct_size = sizeof(struct camif_buffer);
>  	q->drv_priv = vp;
> +	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> 
>  	ret = vb2_queue_init(q);
>  	if (ret)
> diff --git a/drivers/media/platform/s5p-fimc/fimc-capture.c
> b/drivers/media/platform/s5p-fimc/fimc-capture.c index ddd689b..02cfb2b
> 100644
> --- a/drivers/media/platform/s5p-fimc/fimc-capture.c
> +++ b/drivers/media/platform/s5p-fimc/fimc-capture.c
> @@ -1747,6 +1747,7 @@ static int fimc_register_capture_device(struct
> fimc_dev *fimc, q->ops = &fimc_capture_qops;
>  	q->mem_ops = &vb2_dma_contig_memops;
>  	q->buf_struct_size = sizeof(struct fimc_vid_buffer);
> +	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> 
>  	ret = vb2_queue_init(q);
>  	if (ret)
> diff --git a/drivers/media/platform/s5p-fimc/fimc-lite.c
> b/drivers/media/platform/s5p-fimc/fimc-lite.c index 1b309a7..39ea893 100644
> --- a/drivers/media/platform/s5p-fimc/fimc-lite.c
> +++ b/drivers/media/platform/s5p-fimc/fimc-lite.c
> @@ -1251,6 +1251,7 @@ static int fimc_lite_subdev_registered(struct
> v4l2_subdev *sd) q->mem_ops = &vb2_dma_contig_memops;
>  	q->buf_struct_size = sizeof(struct flite_buffer);
>  	q->drv_priv = fimc;
> +	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> 
>  	ret = vb2_queue_init(q);
>  	if (ret < 0)
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c
> b/drivers/media/platform/s5p-mfc/s5p_mfc.c index a527f85..30b4d15 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> @@ -807,6 +807,7 @@ static int s5p_mfc_open(struct file *file)
>  		goto err_queue_init;
>  	}
>  	q->mem_ops = (struct vb2_mem_ops *)&vb2_dma_contig_memops;
> +	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
>  	ret = vb2_queue_init(q);
>  	if (ret) {
>  		mfc_err("Failed to initialize videobuf2 queue(capture)\n");
> @@ -828,6 +829,7 @@ static int s5p_mfc_open(struct file *file)
>  		goto err_queue_init;
>  	}
>  	q->mem_ops = (struct vb2_mem_ops *)&vb2_dma_contig_memops;
> +	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
>  	ret = vb2_queue_init(q);
>  	if (ret) {
>  		mfc_err("Failed to initialize videobuf2 queue(output)\n");
> diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
> b/drivers/media/platform/soc_camera/atmel-isi.c index c8d748a..e531b82
> 100644
> --- a/drivers/media/platform/soc_camera/atmel-isi.c
> +++ b/drivers/media/platform/soc_camera/atmel-isi.c
> @@ -514,6 +514,7 @@ static int isi_camera_init_videobuf(struct vb2_queue *q,
> q->buf_struct_size = sizeof(struct frame_buffer);
>  	q->ops = &isi_video_qops;
>  	q->mem_ops = &vb2_dma_contig_memops;
> +	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> 
>  	return vb2_queue_init(q);
>  }
> diff --git a/drivers/media/platform/soc_camera/mx2_camera.c
> b/drivers/media/platform/soc_camera/mx2_camera.c index 5fbac4f..5ff6a5d
> 100644
> --- a/drivers/media/platform/soc_camera/mx2_camera.c
> +++ b/drivers/media/platform/soc_camera/mx2_camera.c
> @@ -1022,6 +1022,7 @@ static int mx2_camera_init_videobuf(struct vb2_queue
> *q, q->ops = &mx2_videobuf_ops;
>  	q->mem_ops = &vb2_dma_contig_memops;
>  	q->buf_struct_size = sizeof(struct mx2_buffer);
> +	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> 
>  	return vb2_queue_init(q);
>  }
> diff --git a/drivers/media/platform/soc_camera/mx3_camera.c
> b/drivers/media/platform/soc_camera/mx3_camera.c index 574d125..abe9db6
> 100644
> --- a/drivers/media/platform/soc_camera/mx3_camera.c
> +++ b/drivers/media/platform/soc_camera/mx3_camera.c
> @@ -455,6 +455,7 @@ static int mx3_camera_init_videobuf(struct vb2_queue *q,
> q->ops = &mx3_videobuf_ops;
>  	q->mem_ops = &vb2_dma_contig_memops;
>  	q->buf_struct_size = sizeof(struct mx3_camera_buffer);
> +	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> 
>  	return vb2_queue_init(q);
>  }
> diff --git a/drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c
> b/drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c index
> 8a6d58d..b6c1c97 100644
> --- a/drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c
> +++ b/drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c
> @@ -2026,6 +2026,7 @@ static int sh_mobile_ceu_init_videobuf(struct
> vb2_queue *q, q->ops = &sh_mobile_ceu_videobuf_ops;
>  	q->mem_ops = &vb2_dma_contig_memops;
>  	q->buf_struct_size = sizeof(struct sh_mobile_ceu_buffer);
> +	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> 
>  	return vb2_queue_init(q);
>  }
> diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c
> index c2f424f..86a5432 100644
> --- a/drivers/media/platform/vivi.c
> +++ b/drivers/media/platform/vivi.c
> @@ -1305,6 +1305,7 @@ static int __init vivi_create_instance(int inst)
>  	q->buf_struct_size = sizeof(struct vivi_buffer);
>  	q->ops = &vivi_video_qops;
>  	q->mem_ops = &vb2_vmalloc_memops;
> +	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> 
>  	ret = vb2_queue_init(q);
>  	if (ret)
> diff --git a/drivers/media/usb/pwc/pwc-if.c b/drivers/media/usb/pwc/pwc-if.c
> index 21c1523..1b65e0c 100644
> --- a/drivers/media/usb/pwc/pwc-if.c
> +++ b/drivers/media/usb/pwc/pwc-if.c
> @@ -1001,6 +1001,7 @@ static int usb_pwc_probe(struct usb_interface *intf,
> const struct usb_device_id pdev->vb_queue.buf_struct_size = sizeof(struct
> pwc_frame_buf);
>  	pdev->vb_queue.ops = &pwc_vb_queue_ops;
>  	pdev->vb_queue.mem_ops = &vb2_vmalloc_memops;
> +	pdev->vb_queue.timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
>  	rc = vb2_queue_init(&pdev->vb_queue);
>  	if (rc < 0) {
>  		PWC_ERROR("Oops, could not initialize vb2 queue.\n");
> diff --git a/drivers/media/usb/stk1160/stk1160-v4l.c
> b/drivers/media/usb/stk1160/stk1160-v4l.c index 6694f9e..5307a63 100644
> --- a/drivers/media/usb/stk1160/stk1160-v4l.c
> +++ b/drivers/media/usb/stk1160/stk1160-v4l.c
> @@ -687,6 +687,7 @@ int stk1160_vb2_setup(struct stk1160 *dev)
>  	q->buf_struct_size = sizeof(struct stk1160_buffer);
>  	q->ops = &stk1160_video_qops;
>  	q->mem_ops = &vb2_vmalloc_memops;
> +	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> 
>  	rc = vb2_queue_init(q);
>  	if (rc < 0)
> diff --git a/drivers/media/usb/uvc/uvc_queue.c
> b/drivers/media/usb/uvc/uvc_queue.c index 778addc..82d01d8 100644
> --- a/drivers/media/usb/uvc/uvc_queue.c
> +++ b/drivers/media/usb/uvc/uvc_queue.c
> @@ -133,6 +133,7 @@ int uvc_queue_init(struct uvc_video_queue *queue, enum
> v4l2_buf_type type, queue->queue.buf_struct_size = sizeof(struct
> uvc_buffer);
>  	queue->queue.ops = &uvc_queue_qops;
>  	queue->queue.mem_ops = &vb2_vmalloc_memops;
> +	queue->queue.timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
>  	ret = vb2_queue_init(&queue->queue);
>  	if (ret)
>  		return ret;
-- 
Regards,

Laurent Pinchart

RE: [PATCH 1/3 v2] v4l: Define video buffer flag for the COPY timestamp type

[Kamil Debski]

Hi,

> From: Hans Verkuil [mailto:hverkuil@xs4all.nl]
> Sent: Thursday, January 24, 2013 1:51 PM
> 
> On Thu 24 January 2013 13:35:05 Kamil Debski wrote:
> > Define video buffer flag for the COPY timestamp. In this case the
> > timestamp value is copied from the OUTPUT to the corresponding
> CAPTURE buffer.
> >
> > Signed-off-by: Kamil Debski <k.debski@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > ---
> >  Documentation/DocBook/media/v4l/io.xml |    6 ++++++
> >  include/uapi/linux/videodev2.h         |    1 +
> >  2 files changed, 7 insertions(+)
> >
> > diff --git a/Documentation/DocBook/media/v4l/io.xml
> > b/Documentation/DocBook/media/v4l/io.xml
> > index 73f202f..fdd1822 100644
> > --- a/Documentation/DocBook/media/v4l/io.xml
> > +++ b/Documentation/DocBook/media/v4l/io.xml
> > @@ -1145,6 +1145,12 @@ in which case caches have not been
> used.</entry>
> >  	    same clock outside V4L2, use
> >  	    <function>clock_gettime(2)</function> .</entry>
> >  	  </row>
> > +	  <row>
> > +
> <entry><constant>V4L2_BUF_FLAG_TIMESTAMP_COPY</constant></entry>
> > +	    <entry>0x4000</entry>
> > +	    <entry>The CAPTURE buffer timestamp has been taken from the
> > +	    corresponding OUTPUT buffer.</entry>
> > +	  </row>
> 
> I think I mentioned it before somewhere, but it is helpful if it is
> mentioned here that this only applies to mem2mem devices.

Ok, will do.

Best wishes,
Kamil
 
> Regards,
> 
> 	Hans
> 
> >  	</tbody>
> >        </tgroup>
> >      </table>
> > diff --git a/include/uapi/linux/videodev2.h
> > b/include/uapi/linux/videodev2.h index 72e9921..d5a59af 100644
> > --- a/include/uapi/linux/videodev2.h
> > +++ b/include/uapi/linux/videodev2.h
> > @@ -697,6 +697,7 @@ struct v4l2_buffer {
> >  #define V4L2_BUF_FLAG_TIMESTAMP_MASK		0xe000
> >  #define V4L2_BUF_FLAG_TIMESTAMP_UNKNOWN		0x0000
> >  #define V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC	0x2000
> > +#define V4L2_BUF_FLAG_TIMESTAMP_COPY		0x4000
> >
> >  /**
> >   * struct v4l2_exportbuffer - export of video buffer as DMABUF file
> > descriptor
> >

RE: [PATCH 3/3 v2] v4l: Set proper timestamp type in selected drivers which use videobuf2

[Kamil Debski]

Hi,

> From: Laurent Pinchart [mailto:laurent.pinchart@ideasonboard.com]
> Sent: Thursday, January 24, 2013 1:51 PM
> 
> Hi Kamil,
> 
> Thanks for the patch.
> 
> On Thursday 24 January 2013 13:35:07 Kamil Debski wrote:
> > Set proper timestamp type in drivers that I am sure that use either
> > MONOTONIC or COPY timestamps. Other drivers will correctly report
> > UNKNOWN timestamp type instead of assuming that all drivers use
> > monotonic timestamps.
> 
> I've replied to 2/3 before seeing this patch, sorry (although the reply
> is still valid from a bisection point of view).

Ok, it might be a good idea to squash these two patches.

> 
> Do you have a list of those other drivers using vb2 that will report an
> unknown timestamp type ?

Here are the drivers:

drivers/media/platform/coda.c
drivers/media/platform/exynos-gsc/gsc-m2m.c
drivers/media/platform/m2m-deinterlace.c
drivers/media/platform/marvell-ccic/mcam-core.c
drivers/media/platform/mem2mem_testdev.c
drivers/media/platform/mx2_emmaprp.c
drivers/media/platform/s5p-fimc/fimc-m2m.c
drivers/media/platform/s5p-g2d/g2d.c
drivers/media/platform/s5p-jpeg/jpeg-core.c
drivers/media/platform/s5p-tv/mixer_video.c

These drivers do not fill the timestamp field at all.
 
> > Signed-off-by: Kamil Debski <k.debski@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > ---
> >  drivers/media/platform/blackfin/bfin_capture.c     |    1 +
> >  drivers/media/platform/davinci/vpbe_display.c      |    1 +
> >  drivers/media/platform/davinci/vpif_capture.c      |    1 +
> >  drivers/media/platform/davinci/vpif_display.c      |    1 +
> >  drivers/media/platform/s3c-camif/camif-capture.c   |    1 +
> >  drivers/media/platform/s5p-fimc/fimc-capture.c     |    1 +
> >  drivers/media/platform/s5p-fimc/fimc-lite.c        |    1 +
> >  drivers/media/platform/s5p-mfc/s5p_mfc.c           |    2 ++
> >  drivers/media/platform/soc_camera/atmel-isi.c      |    1 +
> >  drivers/media/platform/soc_camera/mx2_camera.c     |    1 +
> >  drivers/media/platform/soc_camera/mx3_camera.c     |    1 +
> >  .../platform/soc_camera/sh_mobile_ceu_camera.c     |    1 +
> >  drivers/media/platform/vivi.c                      |    1 +
> >  drivers/media/usb/pwc/pwc-if.c                     |    1 +
> >  drivers/media/usb/stk1160/stk1160-v4l.c            |    1 +
> >  drivers/media/usb/uvc/uvc_queue.c                  |    1 +
> >  16 files changed, 17 insertions(+)
> >
> > diff --git a/drivers/media/platform/blackfin/bfin_capture.c
> > b/drivers/media/platform/blackfin/bfin_capture.c index
> > d422d3c..365d6ef
> > 100644
> > --- a/drivers/media/platform/blackfin/bfin_capture.c
> > +++ b/drivers/media/platform/blackfin/bfin_capture.c
> > @@ -939,6 +939,7 @@ static int __devinit bcap_probe(struct
> > platform_device
> > *pdev) q->buf_struct_size = sizeof(struct bcap_buffer);
> >  	q->ops = &bcap_video_qops;
> >  	q->mem_ops = &vb2_dma_contig_memops;
> > +	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> >
> >  	vb2_queue_init(q);
> >
> > diff --git a/drivers/media/platform/davinci/vpbe_display.c
> > b/drivers/media/platform/davinci/vpbe_display.c index
> 2bfde79..fa03482
> > 100644
> > --- a/drivers/media/platform/davinci/vpbe_display.c
> > +++ b/drivers/media/platform/davinci/vpbe_display.c
> > @@ -1405,6 +1405,7 @@ static int vpbe_display_reqbufs(struct file
> > *file, void *priv, q->ops = &video_qops;
> >  	q->mem_ops = &vb2_dma_contig_memops;
> >  	q->buf_struct_size = sizeof(struct vpbe_disp_buffer);
> > +	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> >
> >  	ret = vb2_queue_init(q);
> >  	if (ret) {
> > diff --git a/drivers/media/platform/davinci/vpif_capture.c
> > b/drivers/media/platform/davinci/vpif_capture.c index
> 5892d2b..1943e41
> > 100644
> > --- a/drivers/media/platform/davinci/vpif_capture.c
> > +++ b/drivers/media/platform/davinci/vpif_capture.c
> > @@ -1035,6 +1035,7 @@ static int vpif_reqbufs(struct file *file, void
> > *priv,
> > q->ops = &video_qops;
> >  	q->mem_ops = &vb2_dma_contig_memops;
> >  	q->buf_struct_size = sizeof(struct vpif_cap_buffer);
> > +	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> >
> >  	ret = vb2_queue_init(q);
> >  	if (ret) {
> > diff --git a/drivers/media/platform/davinci/vpif_display.c
> > b/drivers/media/platform/davinci/vpif_display.c index
> dd249c9..5477c2c
> > 100644
> > --- a/drivers/media/platform/davinci/vpif_display.c
> > +++ b/drivers/media/platform/davinci/vpif_display.c
> > @@ -1001,6 +1001,7 @@ static int vpif_reqbufs(struct file *file, void
> > *priv,
> > q->ops = &video_qops;
> >  	q->mem_ops = &vb2_dma_contig_memops;
> >  	q->buf_struct_size = sizeof(struct vpif_disp_buffer);
> > +	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> >
> >  	ret = vb2_queue_init(q);
> >  	if (ret) {
> > diff --git a/drivers/media/platform/s3c-camif/camif-capture.c
> > b/drivers/media/platform/s3c-camif/camif-capture.c index
> > a55793c..e91f350
> > 100644
> > --- a/drivers/media/platform/s3c-camif/camif-capture.c
> > +++ b/drivers/media/platform/s3c-camif/camif-capture.c
> > @@ -1153,6 +1153,7 @@ int s3c_camif_register_video_node(struct
> > camif_dev *camif, int idx) q->mem_ops = &vb2_dma_contig_memops;
> >  	q->buf_struct_size = sizeof(struct camif_buffer);
> >  	q->drv_priv = vp;
> > +	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> >
> >  	ret = vb2_queue_init(q);
> >  	if (ret)
> > diff --git a/drivers/media/platform/s5p-fimc/fimc-capture.c
> > b/drivers/media/platform/s5p-fimc/fimc-capture.c index
> > ddd689b..02cfb2b
> > 100644
> > --- a/drivers/media/platform/s5p-fimc/fimc-capture.c
> > +++ b/drivers/media/platform/s5p-fimc/fimc-capture.c
> > @@ -1747,6 +1747,7 @@ static int fimc_register_capture_device(struct
> > fimc_dev *fimc, q->ops = &fimc_capture_qops;
> >  	q->mem_ops = &vb2_dma_contig_memops;
> >  	q->buf_struct_size = sizeof(struct fimc_vid_buffer);
> > +	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> >
> >  	ret = vb2_queue_init(q);
> >  	if (ret)
> > diff --git a/drivers/media/platform/s5p-fimc/fimc-lite.c
> > b/drivers/media/platform/s5p-fimc/fimc-lite.c index 1b309a7..39ea893
> > 100644
> > --- a/drivers/media/platform/s5p-fimc/fimc-lite.c
> > +++ b/drivers/media/platform/s5p-fimc/fimc-lite.c
> > @@ -1251,6 +1251,7 @@ static int fimc_lite_subdev_registered(struct
> > v4l2_subdev *sd) q->mem_ops = &vb2_dma_contig_memops;
> >  	q->buf_struct_size = sizeof(struct flite_buffer);
> >  	q->drv_priv = fimc;
> > +	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> >
> >  	ret = vb2_queue_init(q);
> >  	if (ret < 0)
> > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c
> > b/drivers/media/platform/s5p-mfc/s5p_mfc.c index a527f85..30b4d15
> > 100644
> > --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
> > +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> > @@ -807,6 +807,7 @@ static int s5p_mfc_open(struct file *file)
> >  		goto err_queue_init;
> >  	}
> >  	q->mem_ops = (struct vb2_mem_ops *)&vb2_dma_contig_memops;
> > +	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> >  	ret = vb2_queue_init(q);
> >  	if (ret) {
> >  		mfc_err("Failed to initialize videobuf2 queue(capture)\n");
> @@
> > -828,6 +829,7 @@ static int s5p_mfc_open(struct file *file)
> >  		goto err_queue_init;
> >  	}
> >  	q->mem_ops = (struct vb2_mem_ops *)&vb2_dma_contig_memops;
> > +	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> >  	ret = vb2_queue_init(q);
> >  	if (ret) {
> >  		mfc_err("Failed to initialize videobuf2 queue(output)\n");
> diff
> > --git a/drivers/media/platform/soc_camera/atmel-isi.c
> > b/drivers/media/platform/soc_camera/atmel-isi.c index
> c8d748a..e531b82
> > 100644
> > --- a/drivers/media/platform/soc_camera/atmel-isi.c
> > +++ b/drivers/media/platform/soc_camera/atmel-isi.c
> > @@ -514,6 +514,7 @@ static int isi_camera_init_videobuf(struct
> > vb2_queue *q,
> > q->buf_struct_size = sizeof(struct frame_buffer);
> >  	q->ops = &isi_video_qops;
> >  	q->mem_ops = &vb2_dma_contig_memops;
> > +	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> >
> >  	return vb2_queue_init(q);
> >  }
> > diff --git a/drivers/media/platform/soc_camera/mx2_camera.c
> > b/drivers/media/platform/soc_camera/mx2_camera.c index
> > 5fbac4f..5ff6a5d
> > 100644
> > --- a/drivers/media/platform/soc_camera/mx2_camera.c
> > +++ b/drivers/media/platform/soc_camera/mx2_camera.c
> > @@ -1022,6 +1022,7 @@ static int mx2_camera_init_videobuf(struct
> > vb2_queue *q, q->ops = &mx2_videobuf_ops;
> >  	q->mem_ops = &vb2_dma_contig_memops;
> >  	q->buf_struct_size = sizeof(struct mx2_buffer);
> > +	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> >
> >  	return vb2_queue_init(q);
> >  }
> > diff --git a/drivers/media/platform/soc_camera/mx3_camera.c
> > b/drivers/media/platform/soc_camera/mx3_camera.c index
> > 574d125..abe9db6
> > 100644
> > --- a/drivers/media/platform/soc_camera/mx3_camera.c
> > +++ b/drivers/media/platform/soc_camera/mx3_camera.c
> > @@ -455,6 +455,7 @@ static int mx3_camera_init_videobuf(struct
> > vb2_queue *q,
> > q->ops = &mx3_videobuf_ops;
> >  	q->mem_ops = &vb2_dma_contig_memops;
> >  	q->buf_struct_size = sizeof(struct mx3_camera_buffer);
> > +	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> >
> >  	return vb2_queue_init(q);
> >  }
> > diff --git a/drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c
> > b/drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c index
> > 8a6d58d..b6c1c97 100644
> > --- a/drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c
> > +++ b/drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c
> > @@ -2026,6 +2026,7 @@ static int sh_mobile_ceu_init_videobuf(struct
> > vb2_queue *q, q->ops = &sh_mobile_ceu_videobuf_ops;
> >  	q->mem_ops = &vb2_dma_contig_memops;
> >  	q->buf_struct_size = sizeof(struct sh_mobile_ceu_buffer);
> > +	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> >
> >  	return vb2_queue_init(q);
> >  }
> > diff --git a/drivers/media/platform/vivi.c
> > b/drivers/media/platform/vivi.c index c2f424f..86a5432 100644
> > --- a/drivers/media/platform/vivi.c
> > +++ b/drivers/media/platform/vivi.c
> > @@ -1305,6 +1305,7 @@ static int __init vivi_create_instance(int inst)
> >  	q->buf_struct_size = sizeof(struct vivi_buffer);
> >  	q->ops = &vivi_video_qops;
> >  	q->mem_ops = &vb2_vmalloc_memops;
> > +	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> >
> >  	ret = vb2_queue_init(q);
> >  	if (ret)
> > diff --git a/drivers/media/usb/pwc/pwc-if.c
> > b/drivers/media/usb/pwc/pwc-if.c index 21c1523..1b65e0c 100644
> > --- a/drivers/media/usb/pwc/pwc-if.c
> > +++ b/drivers/media/usb/pwc/pwc-if.c
> > @@ -1001,6 +1001,7 @@ static int usb_pwc_probe(struct usb_interface
> > *intf, const struct usb_device_id pdev->vb_queue.buf_struct_size =
> > sizeof(struct pwc_frame_buf);
> >  	pdev->vb_queue.ops = &pwc_vb_queue_ops;
> >  	pdev->vb_queue.mem_ops = &vb2_vmalloc_memops;
> > +	pdev->vb_queue.timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> >  	rc = vb2_queue_init(&pdev->vb_queue);
> >  	if (rc < 0) {
> >  		PWC_ERROR("Oops, could not initialize vb2 queue.\n"); diff
> --git
> > a/drivers/media/usb/stk1160/stk1160-v4l.c
> > b/drivers/media/usb/stk1160/stk1160-v4l.c index 6694f9e..5307a63
> > 100644
> > --- a/drivers/media/usb/stk1160/stk1160-v4l.c
> > +++ b/drivers/media/usb/stk1160/stk1160-v4l.c
> > @@ -687,6 +687,7 @@ int stk1160_vb2_setup(struct stk1160 *dev)
> >  	q->buf_struct_size = sizeof(struct stk1160_buffer);
> >  	q->ops = &stk1160_video_qops;
> >  	q->mem_ops = &vb2_vmalloc_memops;
> > +	q->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> >
> >  	rc = vb2_queue_init(q);
> >  	if (rc < 0)
> > diff --git a/drivers/media/usb/uvc/uvc_queue.c
> > b/drivers/media/usb/uvc/uvc_queue.c index 778addc..82d01d8 100644
> > --- a/drivers/media/usb/uvc/uvc_queue.c
> > +++ b/drivers/media/usb/uvc/uvc_queue.c
> > @@ -133,6 +133,7 @@ int uvc_queue_init(struct uvc_video_queue *queue,
> > enum v4l2_buf_type type, queue->queue.buf_struct_size = sizeof(struct
> > uvc_buffer);
> >  	queue->queue.ops = &uvc_queue_qops;
> >  	queue->queue.mem_ops = &vb2_vmalloc_memops;
> > +	queue->queue.timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> >  	ret = vb2_queue_init(&queue->queue);
> >  	if (ret)
> >  		return ret;

Best wishes,
Kamil

Re: [PATCH 3/3 v2] v4l: Set proper timestamp type in selected drivers which use videobuf2

[Sakari Ailus]

Hi Kamil,

On Thu, Jan 24, 2013 at 04:31:26PM +0100, Kamil Debski wrote:
> Hi,
> 
> > From: Laurent Pinchart [mailto:laurent.pinchart@ideasonboard.com]
> > Sent: Thursday, January 24, 2013 1:51 PM
> > 
> > Hi Kamil,
> > 
> > Thanks for the patch.
> > 
> > On Thursday 24 January 2013 13:35:07 Kamil Debski wrote:
> > > Set proper timestamp type in drivers that I am sure that use either
> > > MONOTONIC or COPY timestamps. Other drivers will correctly report
> > > UNKNOWN timestamp type instead of assuming that all drivers use
> > > monotonic timestamps.
> > 
> > I've replied to 2/3 before seeing this patch, sorry (although the reply
> > is still valid from a bisection point of view).
> 
> Ok, it might be a good idea to squash these two patches.
> 
> > 
> > Do you have a list of those other drivers using vb2 that will report an
> > unknown timestamp type ?
> 
> Here are the drivers:
> 
> drivers/media/platform/coda.c
> drivers/media/platform/exynos-gsc/gsc-m2m.c
> drivers/media/platform/m2m-deinterlace.c
> drivers/media/platform/marvell-ccic/mcam-core.c
> drivers/media/platform/mem2mem_testdev.c
> drivers/media/platform/mx2_emmaprp.c
> drivers/media/platform/s5p-fimc/fimc-m2m.c
> drivers/media/platform/s5p-g2d/g2d.c
> drivers/media/platform/s5p-jpeg/jpeg-core.c
> drivers/media/platform/s5p-tv/mixer_video.c
> 
> These drivers do not fill the timestamp field at all.

I wonder what should we do to those. Based on a quick look, only mcam-core.c
and s5p-tv/mixer_video.c seem not to be mem-to-mem devices. So the rest
should be COPY, I presume. At least the one I checked seem to have 1:1 ratio
between output and capture buffers.

I know you didn't break them; they were already broken... But I don't think
it'd be that big task to fix them either. Now that your patchset introduces
the COPY timestamp it'd be nice to see it being properly used, rather than
letting applications see lots of UNKNOWN timestamps again. Do you think you
could have time for that?

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

RE: [PATCH 3/3 v2] v4l: Set proper timestamp type in selected drivers which use videobuf2

[Kamil Debski]

Hi Sakari,

> From: Sakari Ailus [mailto:sakari.ailus@iki.fi]
> Sent: Thursday, January 24, 2013 8:12 PM
> 

[snip]

> > >
> > > Do you have a list of those other drivers using vb2 that will
> report
> > > an unknown timestamp type ?
> >
> > Here are the drivers:
> >
> > drivers/media/platform/coda.c
> > drivers/media/platform/exynos-gsc/gsc-m2m.c
> > drivers/media/platform/m2m-deinterlace.c
> > drivers/media/platform/marvell-ccic/mcam-core.c
> > drivers/media/platform/mem2mem_testdev.c
> > drivers/media/platform/mx2_emmaprp.c
> > drivers/media/platform/s5p-fimc/fimc-m2m.c
> > drivers/media/platform/s5p-g2d/g2d.c
> > drivers/media/platform/s5p-jpeg/jpeg-core.c
> > drivers/media/platform/s5p-tv/mixer_video.c
> >
> > These drivers do not fill the timestamp field at all.
> 
> I wonder what should we do to those. Based on a quick look, only mcam-
> core.c and s5p-tv/mixer_video.c seem not to be mem-to-mem devices. So
> the rest should be COPY, I presume. At least the one I checked seem to
> have 1:1 ratio between output and capture buffers.
> 
> I know you didn't break them; they were already broken... But I don't
> think it'd be that big task to fix them either. Now that your patchset
> introduces the COPY timestamp it'd be nice to see it being properly
> used, rather than letting applications see lots of UNKNOWN timestamps
> again. Do you think you could have time for that?

I agree that they should fixed by adding proper timestamp handling (copy).
Currently I have other tasks to do, I might find some time next week to
write the patch. I think that the fix for these drivers can be separate to
this patch set.

Best wishes,
-- 
Kamil Debski
Linux Platform Group
Samsung Poland R&D Center