[REVIEWv2 PATCH 00/13] vb2: various small fixes/improvements

[REVIEWv2 PATCH 00/13] vb2: various small fixes/improvements

[Hans Verkuil]

This is the second version of this review patch series.
The first can be found here:

http://www.spinics.net/lists/linux-media/msg74220.html

Changes since v2:

- Rebased to latest master branch
- Incorporated all suggestions/remarks from Pawel (patches 2, 4 and 7)
- Added patch 12 (follow-up of patch 4) and 13 (suggested by Pawel)

Regards,

	Hans

[REVIEWv2 PATCH 01/13] vb2: stop_streaming should return void

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

The vb2 core ignores any return code from the stop_streaming op.
And there really isn't anything it can do anyway in case of an error.
So change the return type to void and update any drivers that implement it.

The int return gave drivers the idea that this operation could actually
fail, but that's really not the case.

The pwc amd sdr-msi3101 drivers both had this construction:

        if (mutex_lock_interruptible(&s->v4l2_lock))
                return -ERESTARTSYS;

This has been updated to just call mutex_lock(). The stop_streaming op
expects this to really stop streaming and I very much doubt this will
work reliably if stop_streaming just returns without really stopping the
DMA.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Pawel Osciak <pawel@osciak.com>
---
 drivers/media/pci/sta2x11/sta2x11_vip.c                  | 3 +--
 drivers/media/platform/blackfin/bfin_capture.c           | 6 +-----
 drivers/media/platform/coda.c                            | 4 +---
 drivers/media/platform/davinci/vpbe_display.c            | 6 ++----
 drivers/media/platform/davinci/vpif_capture.c            | 6 ++----
 drivers/media/platform/davinci/vpif_display.c            | 6 ++----
 drivers/media/platform/exynos-gsc/gsc-m2m.c              | 4 +---
 drivers/media/platform/exynos4-is/fimc-capture.c         | 6 +++---
 drivers/media/platform/exynos4-is/fimc-lite.c            | 6 +++---
 drivers/media/platform/exynos4-is/fimc-m2m.c             | 3 +--
 drivers/media/platform/marvell-ccic/mcam-core.c          | 7 +++----
 drivers/media/platform/mem2mem_testdev.c                 | 5 ++---
 drivers/media/platform/s3c-camif/camif-capture.c         | 4 ++--
 drivers/media/platform/s5p-jpeg/jpeg-core.c              | 4 +---
 drivers/media/platform/s5p-mfc/s5p_mfc_dec.c             | 3 +--
 drivers/media/platform/s5p-mfc/s5p_mfc_enc.c             | 3 +--
 drivers/media/platform/s5p-tv/mixer_video.c              | 3 +--
 drivers/media/platform/soc_camera/atmel-isi.c            | 6 ++----
 drivers/media/platform/soc_camera/mx2_camera.c           | 4 +---
 drivers/media/platform/soc_camera/mx3_camera.c           | 4 +---
 drivers/media/platform/soc_camera/rcar_vin.c             | 4 +---
 drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c | 4 ++--
 drivers/media/platform/vivi.c                            | 3 +--
 drivers/media/platform/vsp1/vsp1_video.c                 | 4 +---
 drivers/media/usb/em28xx/em28xx-v4l.h                    | 2 +-
 drivers/media/usb/em28xx/em28xx-video.c                  | 8 ++------
 drivers/media/usb/pwc/pwc-if.c                           | 7 ++-----
 drivers/media/usb/s2255/s2255drv.c                       | 5 ++---
 drivers/media/usb/stk1160/stk1160-v4l.c                  | 4 ++--
 drivers/media/usb/usbtv/usbtv-video.c                    | 9 +++------
 drivers/staging/media/davinci_vpfe/vpfe_video.c          | 5 ++---
 drivers/staging/media/dt3155v4l/dt3155v4l.c              | 3 +--
 drivers/staging/media/go7007/go7007-v4l2.c               | 3 +--
 drivers/staging/media/msi3101/sdr-msi3101.c              | 7 ++-----
 drivers/staging/media/rtl2832u_sdr/rtl2832_sdr.c         | 7 ++-----
 drivers/staging/media/solo6x10/solo6x10-v4l2-enc.c       | 3 +--
 drivers/staging/media/solo6x10/solo6x10-v4l2.c           | 3 +--
 include/media/videobuf2-core.h                           | 2 +-
 38 files changed, 60 insertions(+), 116 deletions(-)

diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c b/drivers/media/pci/sta2x11/sta2x11_vip.c
index bb11443..7559951 100644
--- a/drivers/media/pci/sta2x11/sta2x11_vip.c
+++ b/drivers/media/pci/sta2x11/sta2x11_vip.c
@@ -357,7 +357,7 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)
 }
 
 /* abort streaming and wait for last buffer */
-static int stop_streaming(struct vb2_queue *vq)
+static void stop_streaming(struct vb2_queue *vq)
 {
 	struct sta2x11_vip *vip = vb2_get_drv_priv(vq);
 	struct vip_buffer *vip_buf, *node;
@@ -374,7 +374,6 @@ static int stop_streaming(struct vb2_queue *vq)
 		list_del(&vip_buf->list);
 	}
 	spin_unlock(&vip->lock);
-	return 0;
 }
 
 static struct vb2_ops vip_video_qops = {
diff --git a/drivers/media/platform/blackfin/bfin_capture.c b/drivers/media/platform/blackfin/bfin_capture.c
index 200bec9..16f643c 100644
--- a/drivers/media/platform/blackfin/bfin_capture.c
+++ b/drivers/media/platform/blackfin/bfin_capture.c
@@ -427,15 +427,12 @@ static int bcap_start_streaming(struct vb2_queue *vq, unsigned int count)
 	return 0;
 }
 
-static int bcap_stop_streaming(struct vb2_queue *vq)
+static void bcap_stop_streaming(struct vb2_queue *vq)
 {
 	struct bcap_device *bcap_dev = vb2_get_drv_priv(vq);
 	struct ppi_if *ppi = bcap_dev->ppi;
 	int ret;
 
-	if (!vb2_is_streaming(vq))
-		return 0;
-
 	bcap_dev->stop = true;
 	wait_for_completion(&bcap_dev->comp);
 	ppi->ops->stop(ppi);
@@ -452,7 +449,6 @@ static int bcap_stop_streaming(struct vb2_queue *vq)
 		list_del(&bcap_dev->cur_frm->list);
 		vb2_buffer_done(&bcap_dev->cur_frm->vb, VB2_BUF_STATE_ERROR);
 	}
-	return 0;
 }
 
 static struct vb2_ops bcap_video_qops = {
diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
index 3e5199e..d9b1a04 100644
--- a/drivers/media/platform/coda.c
+++ b/drivers/media/platform/coda.c
@@ -2269,7 +2269,7 @@ out:
 	return ret;
 }
 
-static int coda_stop_streaming(struct vb2_queue *q)
+static void coda_stop_streaming(struct vb2_queue *q)
 {
 	struct coda_ctx *ctx = vb2_get_drv_priv(q);
 	struct coda_dev *dev = ctx->dev;
@@ -2295,8 +2295,6 @@ static int coda_stop_streaming(struct vb2_queue *q)
 			ctx->bitstream.vaddr, ctx->bitstream.size);
 		ctx->runcounter = 0;
 	}
-
-	return 0;
 }
 
 static struct vb2_ops coda_qops = {
diff --git a/drivers/media/platform/davinci/vpbe_display.c b/drivers/media/platform/davinci/vpbe_display.c
index b4f12d0..d128fda 100644
--- a/drivers/media/platform/davinci/vpbe_display.c
+++ b/drivers/media/platform/davinci/vpbe_display.c
@@ -368,13 +368,13 @@ static int vpbe_start_streaming(struct vb2_queue *vq, unsigned int count)
 	return ret;
 }
 
-static int vpbe_stop_streaming(struct vb2_queue *vq)
+static void vpbe_stop_streaming(struct vb2_queue *vq)
 {
 	struct vpbe_fh *fh = vb2_get_drv_priv(vq);
 	struct vpbe_layer *layer = fh->layer;
 
 	if (!vb2_is_streaming(vq))
-		return 0;
+		return;
 
 	/* release all active buffers */
 	while (!list_empty(&layer->dma_queue)) {
@@ -383,8 +383,6 @@ static int vpbe_stop_streaming(struct vb2_queue *vq)
 		list_del(&layer->next_frm->list);
 		vb2_buffer_done(&layer->next_frm->vb, VB2_BUF_STATE_ERROR);
 	}
-
-	return 0;
 }
 
 static struct vb2_ops video_qops = {
diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index 756da78..3f0ee2c 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -346,7 +346,7 @@ static int vpif_start_streaming(struct vb2_queue *vq, unsigned int count)
 }
 
 /* abort streaming and wait for last buffer */
-static int vpif_stop_streaming(struct vb2_queue *vq)
+static void vpif_stop_streaming(struct vb2_queue *vq)
 {
 	struct vpif_fh *fh = vb2_get_drv_priv(vq);
 	struct channel_obj *ch = fh->channel;
@@ -354,7 +354,7 @@ static int vpif_stop_streaming(struct vb2_queue *vq)
 	unsigned long flags;
 
 	if (!vb2_is_streaming(vq))
-		return 0;
+		return;
 
 	common = &ch->common[VPIF_VIDEO_INDEX];
 
@@ -367,8 +367,6 @@ static int vpif_stop_streaming(struct vb2_queue *vq)
 		vb2_buffer_done(&common->next_frm->vb, VB2_BUF_STATE_ERROR);
 	}
 	spin_unlock_irqrestore(&common->irqlock, flags);
-
-	return 0;
 }
 
 static struct vb2_ops video_qops = {
diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c
index 0ac841e..c663314 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -308,7 +308,7 @@ static int vpif_start_streaming(struct vb2_queue *vq, unsigned int count)
 }
 
 /* abort streaming and wait for last buffer */
-static int vpif_stop_streaming(struct vb2_queue *vq)
+static void vpif_stop_streaming(struct vb2_queue *vq)
 {
 	struct vpif_fh *fh = vb2_get_drv_priv(vq);
 	struct channel_obj *ch = fh->channel;
@@ -316,7 +316,7 @@ static int vpif_stop_streaming(struct vb2_queue *vq)
 	unsigned long flags;
 
 	if (!vb2_is_streaming(vq))
-		return 0;
+		return;
 
 	common = &ch->common[VPIF_VIDEO_INDEX];
 
@@ -329,8 +329,6 @@ static int vpif_stop_streaming(struct vb2_queue *vq)
 		vb2_buffer_done(&common->next_frm->vb, VB2_BUF_STATE_ERROR);
 	}
 	spin_unlock_irqrestore(&common->irqlock, flags);
-
-	return 0;
 }
 
 static struct vb2_ops video_qops = {
diff --git a/drivers/media/platform/exynos-gsc/gsc-m2m.c b/drivers/media/platform/exynos-gsc/gsc-m2m.c
index d0ea94f..e434f1f0 100644
--- a/drivers/media/platform/exynos-gsc/gsc-m2m.c
+++ b/drivers/media/platform/exynos-gsc/gsc-m2m.c
@@ -66,15 +66,13 @@ static int gsc_m2m_start_streaming(struct vb2_queue *q, unsigned int count)
 	return ret > 0 ? 0 : ret;
 }
 
-static int gsc_m2m_stop_streaming(struct vb2_queue *q)
+static void gsc_m2m_stop_streaming(struct vb2_queue *q)
 {
 	struct gsc_ctx *ctx = q->drv_priv;
 
 	__gsc_m2m_job_abort(ctx);
 
 	pm_runtime_put(&ctx->gsc_dev->pdev->dev);
-
-	return 0;
 }
 
 void gsc_m2m_job_finish(struct gsc_ctx *ctx, int vb_state)
diff --git a/drivers/media/platform/exynos4-is/fimc-capture.c b/drivers/media/platform/exynos4-is/fimc-capture.c
index 92ae812..3d2babd 100644
--- a/drivers/media/platform/exynos4-is/fimc-capture.c
+++ b/drivers/media/platform/exynos4-is/fimc-capture.c
@@ -294,15 +294,15 @@ static int start_streaming(struct vb2_queue *q, unsigned int count)
 	return 0;
 }
 
-static int stop_streaming(struct vb2_queue *q)
+static void stop_streaming(struct vb2_queue *q)
 {
 	struct fimc_ctx *ctx = q->drv_priv;
 	struct fimc_dev *fimc = ctx->fimc_dev;
 
 	if (!fimc_capture_active(fimc))
-		return -EINVAL;
+		return;
 
-	return fimc_stop_capture(fimc, false);
+	fimc_stop_capture(fimc, false);
 }
 
 int fimc_capture_suspend(struct fimc_dev *fimc)
diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c b/drivers/media/platform/exynos4-is/fimc-lite.c
index 3ad660b..630aef5 100644
--- a/drivers/media/platform/exynos4-is/fimc-lite.c
+++ b/drivers/media/platform/exynos4-is/fimc-lite.c
@@ -350,14 +350,14 @@ static int start_streaming(struct vb2_queue *q, unsigned int count)
 	return 0;
 }
 
-static int stop_streaming(struct vb2_queue *q)
+static void stop_streaming(struct vb2_queue *q)
 {
 	struct fimc_lite *fimc = q->drv_priv;
 
 	if (!fimc_lite_active(fimc))
-		return -EINVAL;
+		return;
 
-	return fimc_lite_stop_capture(fimc, false);
+	fimc_lite_stop_capture(fimc, false);
 }
 
 static int queue_setup(struct vb2_queue *vq, const struct v4l2_format *pfmt,
diff --git a/drivers/media/platform/exynos4-is/fimc-m2m.c b/drivers/media/platform/exynos4-is/fimc-m2m.c
index 36971d9..d314155 100644
--- a/drivers/media/platform/exynos4-is/fimc-m2m.c
+++ b/drivers/media/platform/exynos4-is/fimc-m2m.c
@@ -85,7 +85,7 @@ static int start_streaming(struct vb2_queue *q, unsigned int count)
 	return ret > 0 ? 0 : ret;
 }
 
-static int stop_streaming(struct vb2_queue *q)
+static void stop_streaming(struct vb2_queue *q)
 {
 	struct fimc_ctx *ctx = q->drv_priv;
 	int ret;
@@ -95,7 +95,6 @@ static int stop_streaming(struct vb2_queue *q)
 		fimc_m2m_job_finish(ctx, VB2_BUF_STATE_ERROR);
 
 	pm_runtime_put(&ctx->fimc_dev->pdev->dev);
-	return 0;
 }
 
 static void fimc_device_run(void *priv)
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c
index 8b34c48..be4b512 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -1156,7 +1156,7 @@ static int mcam_vb_start_streaming(struct vb2_queue *vq, unsigned int count)
 	return mcam_read_setup(cam);
 }
 
-static int mcam_vb_stop_streaming(struct vb2_queue *vq)
+static void mcam_vb_stop_streaming(struct vb2_queue *vq)
 {
 	struct mcam_camera *cam = vb2_get_drv_priv(vq);
 	unsigned long flags;
@@ -1164,10 +1164,10 @@ static int mcam_vb_stop_streaming(struct vb2_queue *vq)
 	if (cam->state == S_BUFWAIT) {
 		/* They never gave us buffers */
 		cam->state = S_IDLE;
-		return 0;
+		return;
 	}
 	if (cam->state != S_STREAMING)
-		return -EINVAL;
+		return;
 	mcam_ctlr_stop_dma(cam);
 	/*
 	 * Reset the CCIC PHY after stopping streaming,
@@ -1182,7 +1182,6 @@ static int mcam_vb_stop_streaming(struct vb2_queue *vq)
 	spin_lock_irqsave(&cam->dev_lock, flags);
 	INIT_LIST_HEAD(&cam->buffers);
 	spin_unlock_irqrestore(&cam->dev_lock, flags);
-	return 0;
 }
 
 
diff --git a/drivers/media/platform/mem2mem_testdev.c b/drivers/media/platform/mem2mem_testdev.c
index 4f3096b..0714070 100644
--- a/drivers/media/platform/mem2mem_testdev.c
+++ b/drivers/media/platform/mem2mem_testdev.c
@@ -787,7 +787,7 @@ static int m2mtest_start_streaming(struct vb2_queue *q, unsigned count)
 	return 0;
 }
 
-static int m2mtest_stop_streaming(struct vb2_queue *q)
+static void m2mtest_stop_streaming(struct vb2_queue *q)
 {
 	struct m2mtest_ctx *ctx = vb2_get_drv_priv(q);
 	struct vb2_buffer *vb;
@@ -799,12 +799,11 @@ static int m2mtest_stop_streaming(struct vb2_queue *q)
 		else
 			vb = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
 		if (vb == NULL)
-			return 0;
+			return;
 		spin_lock_irqsave(&ctx->dev->irqlock, flags);
 		v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR);
 		spin_unlock_irqrestore(&ctx->dev->irqlock, flags);
 	}
-	return 0;
 }
 
 static struct vb2_ops m2mtest_qops = {
diff --git a/drivers/media/platform/s3c-camif/camif-capture.c b/drivers/media/platform/s3c-camif/camif-capture.c
index 4e4d163..deba425 100644
--- a/drivers/media/platform/s3c-camif/camif-capture.c
+++ b/drivers/media/platform/s3c-camif/camif-capture.c
@@ -435,10 +435,10 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)
 	return 0;
 }
 
-static int stop_streaming(struct vb2_queue *vq)
+static void stop_streaming(struct vb2_queue *vq)
 {
 	struct camif_vp *vp = vb2_get_drv_priv(vq);
-	return camif_stop_capture(vp);
+	camif_stop_capture(vp);
 }
 
 static int queue_setup(struct vb2_queue *vq, const struct v4l2_format *pfmt,
diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 8a18972..368b3f6 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -1670,13 +1670,11 @@ static int s5p_jpeg_start_streaming(struct vb2_queue *q, unsigned int count)
 	return ret > 0 ? 0 : ret;
 }
 
-static int s5p_jpeg_stop_streaming(struct vb2_queue *q)
+static void s5p_jpeg_stop_streaming(struct vb2_queue *q)
 {
 	struct s5p_jpeg_ctx *ctx = vb2_get_drv_priv(q);
 
 	pm_runtime_put(ctx->jpeg->dev);
-
-	return 0;
 }
 
 static struct vb2_ops s5p_jpeg_qops = {
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
index 8faf969..58b7bba 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
@@ -1027,7 +1027,7 @@ static int s5p_mfc_start_streaming(struct vb2_queue *q, unsigned int count)
 	return 0;
 }
 
-static int s5p_mfc_stop_streaming(struct vb2_queue *q)
+static void s5p_mfc_stop_streaming(struct vb2_queue *q)
 {
 	unsigned long flags;
 	struct s5p_mfc_ctx *ctx = fh_to_ctx(q->drv_priv);
@@ -1071,7 +1071,6 @@ static int s5p_mfc_stop_streaming(struct vb2_queue *q)
 	}
 	if (aborted)
 		ctx->state = MFCINST_RUNNING;
-	return 0;
 }
 
 
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
index df83cd1..458279e 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
@@ -1954,7 +1954,7 @@ static int s5p_mfc_start_streaming(struct vb2_queue *q, unsigned int count)
 	return 0;
 }
 
-static int s5p_mfc_stop_streaming(struct vb2_queue *q)
+static void s5p_mfc_stop_streaming(struct vb2_queue *q)
 {
 	unsigned long flags;
 	struct s5p_mfc_ctx *ctx = fh_to_ctx(q->drv_priv);
@@ -1983,7 +1983,6 @@ static int s5p_mfc_stop_streaming(struct vb2_queue *q)
 		ctx->src_queue_cnt = 0;
 	}
 	spin_unlock_irqrestore(&dev->irqlock, flags);
-	return 0;
 }
 
 static void s5p_mfc_buf_queue(struct vb2_buffer *vb)
diff --git a/drivers/media/platform/s5p-tv/mixer_video.c b/drivers/media/platform/s5p-tv/mixer_video.c
index a1ce55f..9f1e52f 100644
--- a/drivers/media/platform/s5p-tv/mixer_video.c
+++ b/drivers/media/platform/s5p-tv/mixer_video.c
@@ -985,7 +985,7 @@ static void mxr_watchdog(unsigned long arg)
 	spin_unlock_irqrestore(&layer->enq_slock, flags);
 }
 
-static int stop_streaming(struct vb2_queue *vq)
+static void stop_streaming(struct vb2_queue *vq)
 {
 	struct mxr_layer *layer = vb2_get_drv_priv(vq);
 	struct mxr_device *mdev = layer->mdev;
@@ -1031,7 +1031,6 @@ static int stop_streaming(struct vb2_queue *vq)
 	mxr_streamer_put(mdev);
 	/* allow changes in output configuration */
 	mxr_output_put(mdev);
-	return 0;
 }
 
 static struct vb2_ops mxr_video_qops = {
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
index f0b6c90..38c723a 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -406,7 +406,7 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)
 }
 
 /* abort streaming and wait for last buffer */
-static int stop_streaming(struct vb2_queue *vq)
+static void stop_streaming(struct vb2_queue *vq)
 {
 	struct soc_camera_device *icd = soc_camera_from_vb2q(vq);
 	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
@@ -433,7 +433,7 @@ static int stop_streaming(struct vb2_queue *vq)
 	if (time_after(jiffies, timeout)) {
 		dev_err(icd->parent,
 			"Timeout waiting for finishing codec request\n");
-		return -ETIMEDOUT;
+		return;
 	}
 
 	/* Disable interrupts */
@@ -444,8 +444,6 @@ static int stop_streaming(struct vb2_queue *vq)
 	ret = atmel_isi_wait_status(isi, WAIT_ISI_DISABLE);
 	if (ret < 0)
 		dev_err(icd->parent, "Disable ISI timed out\n");
-
-	return ret;
 }
 
 static struct vb2_ops isi_video_qops = {
diff --git a/drivers/media/platform/soc_camera/mx2_camera.c b/drivers/media/platform/soc_camera/mx2_camera.c
index 3e84480..b40bc2e 100644
--- a/drivers/media/platform/soc_camera/mx2_camera.c
+++ b/drivers/media/platform/soc_camera/mx2_camera.c
@@ -741,7 +741,7 @@ static int mx2_start_streaming(struct vb2_queue *q, unsigned int count)
 	return 0;
 }
 
-static int mx2_stop_streaming(struct vb2_queue *q)
+static void mx2_stop_streaming(struct vb2_queue *q)
 {
 	struct soc_camera_device *icd = soc_camera_from_vb2q(q);
 	struct soc_camera_host *ici =
@@ -773,8 +773,6 @@ static int mx2_stop_streaming(struct vb2_queue *q)
 
 	dma_free_coherent(ici->v4l2_dev.dev,
 			  pcdev->discard_size, b, pcdev->discard_buffer_dma);
-
-	return 0;
 }
 
 static struct vb2_ops mx2_videobuf_ops = {
diff --git a/drivers/media/platform/soc_camera/mx3_camera.c b/drivers/media/platform/soc_camera/mx3_camera.c
index 9ed81ac..83315df 100644
--- a/drivers/media/platform/soc_camera/mx3_camera.c
+++ b/drivers/media/platform/soc_camera/mx3_camera.c
@@ -406,7 +406,7 @@ static int mx3_videobuf_init(struct vb2_buffer *vb)
 	return 0;
 }
 
-static int mx3_stop_streaming(struct vb2_queue *q)
+static void mx3_stop_streaming(struct vb2_queue *q)
 {
 	struct soc_camera_device *icd = soc_camera_from_vb2q(q);
 	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
@@ -430,8 +430,6 @@ static int mx3_stop_streaming(struct vb2_queue *q)
 	}
 
 	spin_unlock_irqrestore(&mx3_cam->lock, flags);
-
-	return 0;
 }
 
 static struct vb2_ops mx3_videobuf_ops = {
diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c
index 704eee7..e594230 100644
--- a/drivers/media/platform/soc_camera/rcar_vin.c
+++ b/drivers/media/platform/soc_camera/rcar_vin.c
@@ -513,7 +513,7 @@ static int rcar_vin_videobuf_init(struct vb2_buffer *vb)
 	return 0;
 }
 
-static int rcar_vin_stop_streaming(struct vb2_queue *vq)
+static void rcar_vin_stop_streaming(struct vb2_queue *vq)
 {
 	struct soc_camera_device *icd = soc_camera_from_vb2q(vq);
 	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
@@ -524,8 +524,6 @@ static int rcar_vin_stop_streaming(struct vb2_queue *vq)
 	list_for_each_safe(buf_head, tmp, &priv->capture)
 		list_del_init(buf_head);
 	spin_unlock_irq(&priv->lock);
-
-	return 0;
 }
 
 static struct vb2_ops rcar_vin_vb2_ops = {
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 3e75a46..20ad4a5 100644
--- a/drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c
+++ b/drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c
@@ -471,7 +471,7 @@ static int sh_mobile_ceu_videobuf_init(struct vb2_buffer *vb)
 	return 0;
 }
 
-static int sh_mobile_ceu_stop_streaming(struct vb2_queue *q)
+static void sh_mobile_ceu_stop_streaming(struct vb2_queue *q)
 {
 	struct soc_camera_device *icd = container_of(q, struct soc_camera_device, vb2_vidq);
 	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
@@ -487,7 +487,7 @@ static int sh_mobile_ceu_stop_streaming(struct vb2_queue *q)
 
 	spin_unlock_irq(&pcdev->lock);
 
-	return sh_mobile_ceu_soft_reset(pcdev);
+	sh_mobile_ceu_soft_reset(pcdev);
 }
 
 static struct vb2_ops sh_mobile_ceu_videobuf_ops = {
diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c
index 3890f4f..d00bf3d 100644
--- a/drivers/media/platform/vivi.c
+++ b/drivers/media/platform/vivi.c
@@ -906,12 +906,11 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)
 }
 
 /* abort streaming and wait for last buffer */
-static int stop_streaming(struct vb2_queue *vq)
+static void stop_streaming(struct vb2_queue *vq)
 {
 	struct vivi_dev *dev = vb2_get_drv_priv(vq);
 	dprintk(dev, 1, "%s\n", __func__);
 	vivi_stop_generating(dev);
-	return 0;
 }
 
 static void vivi_lock(struct vb2_queue *vq)
diff --git a/drivers/media/platform/vsp1/vsp1_video.c b/drivers/media/platform/vsp1/vsp1_video.c
index b48f135..a0595c1 100644
--- a/drivers/media/platform/vsp1/vsp1_video.c
+++ b/drivers/media/platform/vsp1/vsp1_video.c
@@ -720,7 +720,7 @@ static int vsp1_video_start_streaming(struct vb2_queue *vq, unsigned int count)
 	return 0;
 }
 
-static int vsp1_video_stop_streaming(struct vb2_queue *vq)
+static void vsp1_video_stop_streaming(struct vb2_queue *vq)
 {
 	struct vsp1_video *video = vb2_get_drv_priv(vq);
 	struct vsp1_pipeline *pipe = to_vsp1_pipeline(&video->video.entity);
@@ -743,8 +743,6 @@ static int vsp1_video_stop_streaming(struct vb2_queue *vq)
 	spin_lock_irqsave(&video->irqlock, flags);
 	INIT_LIST_HEAD(&video->irqqueue);
 	spin_unlock_irqrestore(&video->irqlock, flags);
-
-	return 0;
 }
 
 static struct vb2_ops vsp1_video_queue_qops = {
diff --git a/drivers/media/usb/em28xx/em28xx-v4l.h b/drivers/media/usb/em28xx/em28xx-v4l.h
index bce4386..432862c 100644
--- a/drivers/media/usb/em28xx/em28xx-v4l.h
+++ b/drivers/media/usb/em28xx/em28xx-v4l.h
@@ -16,5 +16,5 @@
 
 
 int em28xx_start_analog_streaming(struct vb2_queue *vq, unsigned int count);
-int em28xx_stop_vbi_streaming(struct vb2_queue *vq);
+void em28xx_stop_vbi_streaming(struct vb2_queue *vq);
 extern struct vb2_ops em28xx_vbi_qops;
diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 0856e5d..cdcd751 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -937,7 +937,7 @@ int em28xx_start_analog_streaming(struct vb2_queue *vq, unsigned int count)
 	return rc;
 }
 
-static int em28xx_stop_streaming(struct vb2_queue *vq)
+static void em28xx_stop_streaming(struct vb2_queue *vq)
 {
 	struct em28xx *dev = vb2_get_drv_priv(vq);
 	struct em28xx_dmaqueue *vidq = &dev->vidq;
@@ -961,11 +961,9 @@ static int em28xx_stop_streaming(struct vb2_queue *vq)
 	}
 	dev->usb_ctl.vid_buf = NULL;
 	spin_unlock_irqrestore(&dev->slock, flags);
-
-	return 0;
 }
 
-int em28xx_stop_vbi_streaming(struct vb2_queue *vq)
+void em28xx_stop_vbi_streaming(struct vb2_queue *vq)
 {
 	struct em28xx *dev = vb2_get_drv_priv(vq);
 	struct em28xx_dmaqueue *vbiq = &dev->vbiq;
@@ -989,8 +987,6 @@ int em28xx_stop_vbi_streaming(struct vb2_queue *vq)
 	}
 	dev->usb_ctl.vbi_buf = NULL;
 	spin_unlock_irqrestore(&dev->slock, flags);
-
-	return 0;
 }
 
 static void
diff --git a/drivers/media/usb/pwc/pwc-if.c b/drivers/media/usb/pwc/pwc-if.c
index 84a6720..a73b0bc 100644
--- a/drivers/media/usb/pwc/pwc-if.c
+++ b/drivers/media/usb/pwc/pwc-if.c
@@ -681,12 +681,11 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)
 	return r;
 }
 
-static int stop_streaming(struct vb2_queue *vq)
+static void stop_streaming(struct vb2_queue *vq)
 {
 	struct pwc_device *pdev = vb2_get_drv_priv(vq);
 
-	if (mutex_lock_interruptible(&pdev->v4l2_lock))
-		return -ERESTARTSYS;
+	mutex_lock(&pdev->v4l2_lock);
 	if (pdev->udev) {
 		pwc_set_leds(pdev, 0, 0);
 		pwc_camera_power(pdev, 0);
@@ -695,8 +694,6 @@ static int stop_streaming(struct vb2_queue *vq)
 
 	pwc_cleanup_queued_bufs(pdev);
 	mutex_unlock(&pdev->v4l2_lock);
-
-	return 0;
 }
 
 static struct vb2_ops pwc_vb_queue_ops = {
diff --git a/drivers/media/usb/s2255/s2255drv.c b/drivers/media/usb/s2255/s2255drv.c
index 1d4ba2b..e019dd6 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -714,7 +714,7 @@ static void buffer_queue(struct vb2_buffer *vb)
 }
 
 static int start_streaming(struct vb2_queue *vq, unsigned int count);
-static int stop_streaming(struct vb2_queue *vq);
+static void stop_streaming(struct vb2_queue *vq);
 
 static struct vb2_ops s2255_video_qops = {
 	.queue_setup = queue_setup,
@@ -1109,7 +1109,7 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)
 }
 
 /* abort streaming and wait for last buffer */
-static int stop_streaming(struct vb2_queue *vq)
+static void stop_streaming(struct vb2_queue *vq)
 {
 	struct s2255_vc *vc = vb2_get_drv_priv(vq);
 	struct s2255_buffer *buf, *node;
@@ -1123,7 +1123,6 @@ static int stop_streaming(struct vb2_queue *vq)
 			buf, buf->vb.v4l2_buf.index);
 	}
 	spin_unlock_irqrestore(&vc->qlock, flags);
-	return 0;
 }
 
 static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id i)
diff --git a/drivers/media/usb/stk1160/stk1160-v4l.c b/drivers/media/usb/stk1160/stk1160-v4l.c
index 37bc00f..46e8a50 100644
--- a/drivers/media/usb/stk1160/stk1160-v4l.c
+++ b/drivers/media/usb/stk1160/stk1160-v4l.c
@@ -583,10 +583,10 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)
 }
 
 /* abort streaming and wait for last buffer */
-static int stop_streaming(struct vb2_queue *vq)
+static void stop_streaming(struct vb2_queue *vq)
 {
 	struct stk1160 *dev = vb2_get_drv_priv(vq);
-	return stk1160_stop_streaming(dev);
+	stk1160_stop_streaming(dev);
 }
 
 static struct vb2_ops stk1160_video_qops = {
diff --git a/drivers/media/usb/usbtv/usbtv-video.c b/drivers/media/usb/usbtv/usbtv-video.c
index 20365bd..2967e80 100644
--- a/drivers/media/usb/usbtv/usbtv-video.c
+++ b/drivers/media/usb/usbtv/usbtv-video.c
@@ -634,15 +634,12 @@ static int usbtv_start_streaming(struct vb2_queue *vq, unsigned int count)
 	return usbtv_start(usbtv);
 }
 
-static int usbtv_stop_streaming(struct vb2_queue *vq)
+static void usbtv_stop_streaming(struct vb2_queue *vq)
 {
 	struct usbtv *usbtv = vb2_get_drv_priv(vq);
 
-	if (usbtv->udev == NULL)
-		return -ENODEV;
-
-	usbtv_stop(usbtv);
-	return 0;
+	if (usbtv->udev)
+		usbtv_stop(usbtv);
 }
 
 static struct vb2_ops usbtv_vb2_ops = {
diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.c b/drivers/staging/media/davinci_vpfe/vpfe_video.c
index 8c101cb..c91e10f 100644
--- a/drivers/staging/media/davinci_vpfe/vpfe_video.c
+++ b/drivers/staging/media/davinci_vpfe/vpfe_video.c
@@ -1242,13 +1242,13 @@ static int vpfe_buffer_init(struct vb2_buffer *vb)
 }
 
 /* abort streaming and wait for last buffer */
-static int vpfe_stop_streaming(struct vb2_queue *vq)
+static void vpfe_stop_streaming(struct vb2_queue *vq)
 {
 	struct vpfe_fh *fh = vb2_get_drv_priv(vq);
 	struct vpfe_video_device *video = fh->video;
 
 	if (!vb2_is_streaming(vq))
-		return 0;
+		return;
 	/* release all active buffers */
 	while (!list_empty(&video->dma_queue)) {
 		video->next_frm = list_entry(video->dma_queue.next,
@@ -1256,7 +1256,6 @@ static int vpfe_stop_streaming(struct vb2_queue *vq)
 		list_del(&video->next_frm->list);
 		vb2_buffer_done(&video->next_frm->vb, VB2_BUF_STATE_ERROR);
 	}
-	return 0;
 }
 
 static void vpfe_buf_cleanup(struct vb2_buffer *vb)
diff --git a/drivers/staging/media/dt3155v4l/dt3155v4l.c b/drivers/staging/media/dt3155v4l/dt3155v4l.c
index 68d41e2..c9ddff3 100644
--- a/drivers/staging/media/dt3155v4l/dt3155v4l.c
+++ b/drivers/staging/media/dt3155v4l/dt3155v4l.c
@@ -262,7 +262,7 @@ dt3155_buf_prepare(struct vb2_buffer *vb)
 	return 0;
 }
 
-static int
+static void
 dt3155_stop_streaming(struct vb2_queue *q)
 {
 	struct dt3155_priv *pd = vb2_get_drv_priv(q);
@@ -276,7 +276,6 @@ dt3155_stop_streaming(struct vb2_queue *q)
 	}
 	spin_unlock_irq(&pd->lock);
 	msleep(45); /* irq hendler will stop the hardware */
-	return 0;
 }
 
 static void
diff --git a/drivers/staging/media/go7007/go7007-v4l2.c b/drivers/staging/media/go7007/go7007-v4l2.c
index a349878..818acdde 100644
--- a/drivers/staging/media/go7007/go7007-v4l2.c
+++ b/drivers/staging/media/go7007/go7007-v4l2.c
@@ -515,7 +515,7 @@ static int go7007_start_streaming(struct vb2_queue *q, unsigned int count)
 	return ret;
 }
 
-static int go7007_stop_streaming(struct vb2_queue *q)
+static void go7007_stop_streaming(struct vb2_queue *q)
 {
 	struct go7007 *go = vb2_get_drv_priv(q);
 	unsigned long flags;
@@ -537,7 +537,6 @@ static int go7007_stop_streaming(struct vb2_queue *q)
 	/* Turn on Capture LED */
 	if (go->board_id == GO7007_BOARDID_ADS_USBAV_709)
 		go7007_write_addr(go, 0x3c82, 0x000d);
-	return 0;
 }
 
 static struct vb2_ops go7007_video_qops = {
diff --git a/drivers/staging/media/msi3101/sdr-msi3101.c b/drivers/staging/media/msi3101/sdr-msi3101.c
index 011db2c..52132ee 100644
--- a/drivers/staging/media/msi3101/sdr-msi3101.c
+++ b/drivers/staging/media/msi3101/sdr-msi3101.c
@@ -1075,13 +1075,12 @@ static int msi3101_start_streaming(struct vb2_queue *vq, unsigned int count)
 	return ret;
 }
 
-static int msi3101_stop_streaming(struct vb2_queue *vq)
+static void msi3101_stop_streaming(struct vb2_queue *vq)
 {
 	struct msi3101_state *s = vb2_get_drv_priv(vq);
 	dev_dbg(&s->udev->dev, "%s:\n", __func__);
 
-	if (mutex_lock_interruptible(&s->v4l2_lock))
-		return -ERESTARTSYS;
+	mutex_lock(&s->v4l2_lock);
 
 	if (s->udev)
 		msi3101_isoc_cleanup(s);
@@ -1099,8 +1098,6 @@ static int msi3101_stop_streaming(struct vb2_queue *vq)
 	v4l2_subdev_call(s->v4l2_subdev, core, s_power, 0);
 
 	mutex_unlock(&s->v4l2_lock);
-
-	return 0;
 }
 
 static struct vb2_ops msi3101_vb2_ops = {
diff --git a/drivers/staging/media/rtl2832u_sdr/rtl2832_sdr.c b/drivers/staging/media/rtl2832u_sdr/rtl2832_sdr.c
index 104ee8a..093df6b 100644
--- a/drivers/staging/media/rtl2832u_sdr/rtl2832_sdr.c
+++ b/drivers/staging/media/rtl2832u_sdr/rtl2832_sdr.c
@@ -1032,13 +1032,12 @@ err:
 	return ret;
 }
 
-static int rtl2832_sdr_stop_streaming(struct vb2_queue *vq)
+static void rtl2832_sdr_stop_streaming(struct vb2_queue *vq)
 {
 	struct rtl2832_sdr_state *s = vb2_get_drv_priv(vq);
 	dev_dbg(&s->udev->dev, "%s:\n", __func__);
 
-	if (mutex_lock_interruptible(&s->v4l2_lock))
-		return -ERESTARTSYS;
+	mutex_lock(&s->v4l2_lock);
 
 	rtl2832_sdr_kill_urbs(s);
 	rtl2832_sdr_free_urbs(s);
@@ -1053,8 +1052,6 @@ static int rtl2832_sdr_stop_streaming(struct vb2_queue *vq)
 		s->d->props->power_ctrl(s->d, 0);
 
 	mutex_unlock(&s->v4l2_lock);
-
-	return 0;
 }
 
 static struct vb2_ops rtl2832_sdr_vb2_ops = {
diff --git a/drivers/staging/media/solo6x10/solo6x10-v4l2-enc.c b/drivers/staging/media/solo6x10/solo6x10-v4l2-enc.c
index edcabcd..e056476 100644
--- a/drivers/staging/media/solo6x10/solo6x10-v4l2-enc.c
+++ b/drivers/staging/media/solo6x10/solo6x10-v4l2-enc.c
@@ -741,14 +741,13 @@ static int solo_enc_start_streaming(struct vb2_queue *q, unsigned int count)
 	return solo_ring_start(solo_enc->solo_dev);
 }
 
-static int solo_enc_stop_streaming(struct vb2_queue *q)
+static void solo_enc_stop_streaming(struct vb2_queue *q)
 {
 	struct solo_enc_dev *solo_enc = vb2_get_drv_priv(q);
 
 	solo_enc_off(solo_enc);
 	INIT_LIST_HEAD(&solo_enc->vidq_active);
 	solo_ring_stop(solo_enc->solo_dev);
-	return 0;
 }
 
 static struct vb2_ops solo_enc_video_qops = {
diff --git a/drivers/staging/media/solo6x10/solo6x10-v4l2.c b/drivers/staging/media/solo6x10/solo6x10-v4l2.c
index 1815f76..5d0100e 100644
--- a/drivers/staging/media/solo6x10/solo6x10-v4l2.c
+++ b/drivers/staging/media/solo6x10/solo6x10-v4l2.c
@@ -336,13 +336,12 @@ static int solo_start_streaming(struct vb2_queue *q, unsigned int count)
 	return solo_start_thread(solo_dev);
 }
 
-static int solo_stop_streaming(struct vb2_queue *q)
+static void solo_stop_streaming(struct vb2_queue *q)
 {
 	struct solo_dev *solo_dev = vb2_get_drv_priv(q);
 
 	solo_stop_thread(solo_dev);
 	INIT_LIST_HEAD(&solo_dev->vidq_active);
-	return 0;
 }
 
 static void solo_buf_queue(struct vb2_buffer *vb)
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index af46211..3b57851 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -323,7 +323,7 @@ struct vb2_ops {
 	void (*buf_cleanup)(struct vb2_buffer *vb);
 
 	int (*start_streaming)(struct vb2_queue *q, unsigned int count);
-	int (*stop_streaming)(struct vb2_queue *q);
+	void (*stop_streaming)(struct vb2_queue *q);
 
 	void (*buf_queue)(struct vb2_buffer *vb);
 };
-- 
1.9.1

[REVIEWv2 PATCH 02/13] vb2: fix handling of data_offset and v4l2_plane.reserved[]

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

The videobuf2-core did not zero the 'planes' array in __qbuf_userptr()
and __qbuf_dmabuf(). That's now memset to 0. Without this the reserved
array in struct v4l2_plane would be non-zero, causing v4l2-compliance
errors.

More serious is the fact that data_offset was not handled correctly:

- for capture devices it was never zeroed, which meant that it was
  uninitialized. Unless the driver sets it it was a completely random
  number. With the memset above this is now fixed.

- __qbuf_dmabuf had a completely incorrect length check that included
  data_offset.

- in __fill_vb2_buffer in the DMABUF case the data_offset field was
  unconditionally copied from v4l2_buffer to v4l2_plane when this
  should only happen in the output case.

- in the single-planar case data_offset was never correctly set to 0.
  The single-planar API doesn't support data_offset, so setting it
  to 0 is the right thing to do. This too is now solved by the memset.

All these issues were found with v4l2-compliance.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/v4l2-core/videobuf2-core.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index f9059bb..596998e 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1169,8 +1169,6 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b
 					b->m.planes[plane].m.fd;
 				v4l2_planes[plane].length =
 					b->m.planes[plane].length;
-				v4l2_planes[plane].data_offset =
-					b->m.planes[plane].data_offset;
 			}
 		}
 	} else {
@@ -1180,10 +1178,8 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b
 		 * In videobuf we use our internal V4l2_planes struct for
 		 * single-planar buffers as well, for simplicity.
 		 */
-		if (V4L2_TYPE_IS_OUTPUT(b->type)) {
+		if (V4L2_TYPE_IS_OUTPUT(b->type))
 			v4l2_planes[0].bytesused = b->bytesused;
-			v4l2_planes[0].data_offset = 0;
-		}
 
 		if (b->memory == V4L2_MEMORY_USERPTR) {
 			v4l2_planes[0].m.userptr = b->m.userptr;
@@ -1193,9 +1189,7 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b
 		if (b->memory == V4L2_MEMORY_DMABUF) {
 			v4l2_planes[0].m.fd = b->m.fd;
 			v4l2_planes[0].length = b->length;
-			v4l2_planes[0].data_offset = 0;
 		}
-
 	}
 
 	/* Zero flags that the vb2 core handles */
@@ -1238,6 +1232,7 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 	int write = !V4L2_TYPE_IS_OUTPUT(q->type);
 	bool reacquired = vb->planes[0].mem_priv == NULL;
 
+	memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
 	/* Copy relevant information provided by the userspace */
 	__fill_vb2_buffer(vb, b, planes);
 
@@ -1357,6 +1352,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 	int write = !V4L2_TYPE_IS_OUTPUT(q->type);
 	bool reacquired = vb->planes[0].mem_priv == NULL;
 
+	memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
 	/* Copy relevant information provided by the userspace */
 	__fill_vb2_buffer(vb, b, planes);
 
@@ -1374,8 +1370,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 		if (planes[plane].length == 0)
 			planes[plane].length = dbuf->size;
 
-		if (planes[plane].length < planes[plane].data_offset +
-		    q->plane_sizes[plane]) {
+		if (planes[plane].length < q->plane_sizes[plane]) {
 			dprintk(1, "qbuf: invalid dmabuf length for plane %d\n",
 				plane);
 			ret = -EINVAL;
-- 
1.9.1

[REVIEWv2 PATCH 03/13] vb2: if bytesused is 0, then fill with output buffer length

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

The application should really always fill in bytesused for output
buffers, unfortunately the vb2 framework never checked for that.

So for single planar formats replace a bytesused of 0 by the length
of the buffer, and for multiplanar format do the same if bytesused is
0 for ALL planes.

This seems to be what the user really intended if v4l2_buffer was
just memset to 0.

I'm afraid that just checking for this and returning an error would
break too many applications. Quite a few drivers never check for bytesused
at all and just use the buffer length instead.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Pawel Osciak <pawel@osciak.com>
---
 drivers/media/v4l2-core/videobuf2-core.c | 31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index 596998e..b2582cb 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1143,15 +1143,30 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b
 	if (V4L2_TYPE_IS_MULTIPLANAR(b->type)) {
 		/* Fill in driver-provided information for OUTPUT types */
 		if (V4L2_TYPE_IS_OUTPUT(b->type)) {
+			bool bytesused_is_used;
+
+			/* Check if bytesused == 0 for all planes */
+			for (plane = 0; plane < vb->num_planes; ++plane)
+				if (b->m.planes[plane].bytesused)
+					break;
+			bytesused_is_used = plane < vb->num_planes;
+
 			/*
 			 * Will have to go up to b->length when API starts
 			 * accepting variable number of planes.
+			 *
+			 * If bytesused_is_used is false, then fall back to the
+			 * full buffer size. In that case userspace clearly
+			 * never bothered to set it and it's a safe assumption
+			 * that they really meant to use the full plane sizes.
 			 */
 			for (plane = 0; plane < vb->num_planes; ++plane) {
-				v4l2_planes[plane].bytesused =
-					b->m.planes[plane].bytesused;
-				v4l2_planes[plane].data_offset =
-					b->m.planes[plane].data_offset;
+				struct v4l2_plane *pdst = &v4l2_planes[plane];
+				struct v4l2_plane *psrc = &b->m.planes[plane];
+
+				pdst->bytesused = bytesused_is_used ?
+					psrc->bytesused : psrc->length;
+				pdst->data_offset = psrc->data_offset;
 			}
 		}
 
@@ -1177,9 +1192,15 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b
 		 * so fill in relevant v4l2_buffer struct fields instead.
 		 * In videobuf we use our internal V4l2_planes struct for
 		 * single-planar buffers as well, for simplicity.
+		 *
+		 * If bytesused == 0, then fall back to the full buffer size
+		 * as that's a sensible default.
 		 */
 		if (V4L2_TYPE_IS_OUTPUT(b->type))
-			v4l2_planes[0].bytesused = b->bytesused;
+			v4l2_planes[0].bytesused =
+				b->bytesused ? b->bytesused : b->length;
+		else
+			v4l2_planes[0].bytesused = 0;
 
 		if (b->memory == V4L2_MEMORY_USERPTR) {
 			v4l2_planes[0].m.userptr = b->m.userptr;
-- 
1.9.1

[REVIEWv2 PATCH 04/13] vb2: use correct prefix

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Many dprintk's in vb2 use a hardcoded prefix with the function name. In
many cases that is now outdated. To keep things consistent the dprintk
macro has been changed to print the function name in addition to the "vb2:"
prefix. Superfluous prefixes elsewhere in the code have been removed.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/v4l2-core/videobuf2-core.c | 133 +++++++++++++++----------------
 1 file changed, 65 insertions(+), 68 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index b2582cb..1421075 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -27,10 +27,10 @@
 static int debug;
 module_param(debug, int, 0644);
 
-#define dprintk(level, fmt, arg...)					\
-	do {								\
-		if (debug >= level)					\
-			printk(KERN_DEBUG "vb2: " fmt, ## arg);		\
+#define dprintk(level, fmt, arg...)					      \
+	do {								      \
+		if (debug >= level)					      \
+			pr_debug("vb2: %s: " fmt, __func__, ## arg); \
 	} while (0)
 
 #ifdef CONFIG_VIDEO_ADV_DEBUG
@@ -371,7 +371,7 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned int buffers)
 		if (q->bufs[buffer] == NULL)
 			continue;
 		if (q->bufs[buffer]->state == VB2_BUF_STATE_PREPARING) {
-			dprintk(1, "reqbufs: preparing buffers, cannot free\n");
+			dprintk(1, "preparing buffers, cannot free\n");
 			return -EAGAIN;
 		}
 	}
@@ -656,12 +656,12 @@ int vb2_querybuf(struct vb2_queue *q, struct v4l2_buffer *b)
 	int ret;
 
 	if (b->type != q->type) {
-		dprintk(1, "querybuf: wrong buffer type\n");
+		dprintk(1, "wrong buffer type\n");
 		return -EINVAL;
 	}
 
 	if (b->index >= q->num_buffers) {
-		dprintk(1, "querybuf: buffer index out of range\n");
+		dprintk(1, "buffer index out of range\n");
 		return -EINVAL;
 	}
 	vb = q->bufs[b->index];
@@ -721,12 +721,12 @@ static int __verify_memory_type(struct vb2_queue *q,
 {
 	if (memory != V4L2_MEMORY_MMAP && memory != V4L2_MEMORY_USERPTR &&
 	    memory != V4L2_MEMORY_DMABUF) {
-		dprintk(1, "reqbufs: unsupported memory type\n");
+		dprintk(1, "unsupported memory type\n");
 		return -EINVAL;
 	}
 
 	if (type != q->type) {
-		dprintk(1, "reqbufs: requested type is incorrect\n");
+		dprintk(1, "requested type is incorrect\n");
 		return -EINVAL;
 	}
 
@@ -735,17 +735,17 @@ static int __verify_memory_type(struct vb2_queue *q,
 	 * are available.
 	 */
 	if (memory == V4L2_MEMORY_MMAP && __verify_mmap_ops(q)) {
-		dprintk(1, "reqbufs: MMAP for current setup unsupported\n");
+		dprintk(1, "MMAP for current setup unsupported\n");
 		return -EINVAL;
 	}
 
 	if (memory == V4L2_MEMORY_USERPTR && __verify_userptr_ops(q)) {
-		dprintk(1, "reqbufs: USERPTR for current setup unsupported\n");
+		dprintk(1, "USERPTR for current setup unsupported\n");
 		return -EINVAL;
 	}
 
 	if (memory == V4L2_MEMORY_DMABUF && __verify_dmabuf_ops(q)) {
-		dprintk(1, "reqbufs: DMABUF for current setup unsupported\n");
+		dprintk(1, "DMABUF for current setup unsupported\n");
 		return -EINVAL;
 	}
 
@@ -755,7 +755,7 @@ static int __verify_memory_type(struct vb2_queue *q,
 	 * do the memory and type validation.
 	 */
 	if (q->fileio) {
-		dprintk(1, "reqbufs: file io in progress\n");
+		dprintk(1, "file io in progress\n");
 		return -EBUSY;
 	}
 	return 0;
@@ -790,7 +790,7 @@ static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
 	int ret;
 
 	if (q->streaming) {
-		dprintk(1, "reqbufs: streaming active\n");
+		dprintk(1, "streaming active\n");
 		return -EBUSY;
 	}
 
@@ -800,7 +800,7 @@ static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
 		 * are not in use and can be freed.
 		 */
 		if (q->memory == V4L2_MEMORY_MMAP && __buffers_in_use(q)) {
-			dprintk(1, "reqbufs: memory in use, cannot free\n");
+			dprintk(1, "memory in use, cannot free\n");
 			return -EBUSY;
 		}
 
@@ -931,8 +931,7 @@ static int __create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create
 	int ret;
 
 	if (q->num_buffers == VIDEO_MAX_FRAME) {
-		dprintk(1, "%s(): maximum number of buffers already allocated\n",
-			__func__);
+		dprintk(1, "maximum number of buffers already allocated\n");
 		return -ENOBUFS;
 	}
 
@@ -1264,12 +1263,12 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 		    && vb->v4l2_planes[plane].length == planes[plane].length)
 			continue;
 
-		dprintk(3, "qbuf: userspace address for plane %d changed, "
+		dprintk(3, "userspace address for plane %d changed, "
 				"reacquiring memory\n", plane);
 
 		/* Check if the provided plane buffer is large enough */
 		if (planes[plane].length < q->plane_sizes[plane]) {
-			dprintk(1, "qbuf: provided buffer size %u is less than "
+			dprintk(1, "provided buffer size %u is less than "
 						"setup size %u for plane %d\n",
 						planes[plane].length,
 						q->plane_sizes[plane], plane);
@@ -1294,7 +1293,7 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 				      planes[plane].m.userptr,
 				      planes[plane].length, write);
 		if (IS_ERR_OR_NULL(mem_priv)) {
-			dprintk(1, "qbuf: failed acquiring userspace "
+			dprintk(1, "failed acquiring userspace "
 						"memory for plane %d\n", plane);
 			fail_memop(vb, get_userptr);
 			ret = mem_priv ? PTR_ERR(mem_priv) : -EINVAL;
@@ -1318,7 +1317,7 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 		 */
 		ret = call_vb_qop(vb, buf_init, vb);
 		if (ret) {
-			dprintk(1, "qbuf: buffer initialization failed\n");
+			dprintk(1, "buffer initialization failed\n");
 			fail_vb_qop(vb, buf_init);
 			goto err;
 		}
@@ -1326,7 +1325,7 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 
 	ret = call_vb_qop(vb, buf_prepare, vb);
 	if (ret) {
-		dprintk(1, "qbuf: buffer preparation failed\n");
+		dprintk(1, "buffer preparation failed\n");
 		fail_vb_qop(vb, buf_prepare);
 		call_vb_qop(vb, buf_cleanup, vb);
 		goto err;
@@ -1381,7 +1380,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 		struct dma_buf *dbuf = dma_buf_get(planes[plane].m.fd);
 
 		if (IS_ERR_OR_NULL(dbuf)) {
-			dprintk(1, "qbuf: invalid dmabuf fd for plane %d\n",
+			dprintk(1, "invalid dmabuf fd for plane %d\n",
 				plane);
 			ret = -EINVAL;
 			goto err;
@@ -1392,7 +1391,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 			planes[plane].length = dbuf->size;
 
 		if (planes[plane].length < q->plane_sizes[plane]) {
-			dprintk(1, "qbuf: invalid dmabuf length for plane %d\n",
+			dprintk(1, "invalid dmabuf length for plane %d\n",
 				plane);
 			ret = -EINVAL;
 			goto err;
@@ -1405,7 +1404,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 			continue;
 		}
 
-		dprintk(1, "qbuf: buffer for plane %d changed\n", plane);
+		dprintk(1, "buffer for plane %d changed\n", plane);
 
 		if (!reacquired) {
 			reacquired = true;
@@ -1420,7 +1419,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 		mem_priv = call_memop(vb, attach_dmabuf, q->alloc_ctx[plane],
 			dbuf, planes[plane].length, write);
 		if (IS_ERR(mem_priv)) {
-			dprintk(1, "qbuf: failed to attach dmabuf\n");
+			dprintk(1, "failed to attach dmabuf\n");
 			fail_memop(vb, attach_dmabuf);
 			ret = PTR_ERR(mem_priv);
 			dma_buf_put(dbuf);
@@ -1438,7 +1437,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 	for (plane = 0; plane < vb->num_planes; ++plane) {
 		ret = call_memop(vb, map_dmabuf, vb->planes[plane].mem_priv);
 		if (ret) {
-			dprintk(1, "qbuf: failed to map dmabuf for plane %d\n",
+			dprintk(1, "failed to map dmabuf for plane %d\n",
 				plane);
 			fail_memop(vb, map_dmabuf);
 			goto err;
@@ -1460,7 +1459,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 		 */
 		ret = call_vb_qop(vb, buf_init, vb);
 		if (ret) {
-			dprintk(1, "qbuf: buffer initialization failed\n");
+			dprintk(1, "buffer initialization failed\n");
 			fail_vb_qop(vb, buf_init);
 			goto err;
 		}
@@ -1468,7 +1467,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 
 	ret = call_vb_qop(vb, buf_prepare, vb);
 	if (ret) {
-		dprintk(1, "qbuf: buffer preparation failed\n");
+		dprintk(1, "buffer preparation failed\n");
 		fail_vb_qop(vb, buf_prepare);
 		call_vb_qop(vb, buf_cleanup, vb);
 		goto err;
@@ -1508,8 +1507,7 @@ static int __buf_prepare(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 
 	ret = __verify_length(vb, b);
 	if (ret < 0) {
-		dprintk(1, "%s(): plane parameters verification failed: %d\n",
-			__func__, ret);
+		dprintk(1, "plane parameters verification failed: %d\n", ret);
 		return ret;
 	}
 
@@ -1553,7 +1551,7 @@ static int __buf_prepare(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 	}
 
 	if (ret)
-		dprintk(1, "qbuf: buffer preparation failed: %d\n", ret);
+		dprintk(1, "buffer preparation failed: %d\n", ret);
 	vb->state = ret ? VB2_BUF_STATE_DEQUEUED : VB2_BUF_STATE_PREPARED;
 
 	return ret;
@@ -1563,23 +1561,23 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct v4l2_buffer *b,
 				    const char *opname)
 {
 	if (b->type != q->type) {
-		dprintk(1, "%s(): invalid buffer type\n", opname);
+		dprintk(1, "%s: invalid buffer type\n", opname);
 		return -EINVAL;
 	}
 
 	if (b->index >= q->num_buffers) {
-		dprintk(1, "%s(): buffer index out of range\n", opname);
+		dprintk(1, "%s: buffer index out of range\n", opname);
 		return -EINVAL;
 	}
 
 	if (q->bufs[b->index] == NULL) {
 		/* Should never happen */
-		dprintk(1, "%s(): buffer is NULL\n", opname);
+		dprintk(1, "%s: buffer is NULL\n", opname);
 		return -EINVAL;
 	}
 
 	if (b->memory != q->memory) {
-		dprintk(1, "%s(): invalid memory type\n", opname);
+		dprintk(1, "%s: invalid memory type\n", opname);
 		return -EINVAL;
 	}
 
@@ -1607,7 +1605,7 @@ int vb2_prepare_buf(struct vb2_queue *q, struct v4l2_buffer *b)
 	int ret;
 
 	if (q->fileio) {
-		dprintk(1, "%s(): file io in progress\n", __func__);
+		dprintk(1, "file io in progress\n");
 		return -EBUSY;
 	}
 
@@ -1617,7 +1615,7 @@ int vb2_prepare_buf(struct vb2_queue *q, struct v4l2_buffer *b)
 
 	vb = q->bufs[b->index];
 	if (vb->state != VB2_BUF_STATE_DEQUEUED) {
-		dprintk(1, "%s(): invalid buffer state %d\n", __func__,
+		dprintk(1, "invalid buffer state %d\n",
 			vb->state);
 		return -EINVAL;
 	}
@@ -1627,7 +1625,7 @@ int vb2_prepare_buf(struct vb2_queue *q, struct v4l2_buffer *b)
 		/* Fill buffer information for the userspace */
 		__fill_v4l2_buffer(vb, b);
 
-		dprintk(1, "%s() of buffer %d succeeded\n", __func__, vb->v4l2_buf.index);
+		dprintk(1, "prepare of buffer %d succeeded\n", vb->v4l2_buf.index);
 	}
 	return ret;
 }
@@ -1664,7 +1662,7 @@ static int vb2_start_streaming(struct vb2_queue *q)
 		return 0;
 
 	fail_qop(q, start_streaming);
-	dprintk(1, "qbuf: driver refused to start streaming\n");
+	dprintk(1, "driver refused to start streaming\n");
 	if (WARN_ON(atomic_read(&q->owned_by_drv_count))) {
 		unsigned i;
 
@@ -1702,11 +1700,10 @@ static int vb2_internal_qbuf(struct vb2_queue *q, struct v4l2_buffer *b)
 	case VB2_BUF_STATE_PREPARED:
 		break;
 	case VB2_BUF_STATE_PREPARING:
-		dprintk(1, "qbuf: buffer still being prepared\n");
+		dprintk(1, "buffer still being prepared\n");
 		return -EINVAL;
 	default:
-		dprintk(1, "%s(): invalid buffer state %d\n", __func__,
-			vb->state);
+		dprintk(1, "invalid buffer state %d\n", vb->state);
 		return -EINVAL;
 	}
 
@@ -1753,7 +1750,7 @@ static int vb2_internal_qbuf(struct vb2_queue *q, struct v4l2_buffer *b)
 			return ret;
 	}
 
-	dprintk(1, "%s() of buffer %d succeeded\n", __func__, vb->v4l2_buf.index);
+	dprintk(1, "qbuf of buffer %d succeeded\n", vb->v4l2_buf.index);
 	return 0;
 }
 
@@ -1777,7 +1774,7 @@ static int vb2_internal_qbuf(struct vb2_queue *q, struct v4l2_buffer *b)
 int vb2_qbuf(struct vb2_queue *q, struct v4l2_buffer *b)
 {
 	if (q->fileio) {
-		dprintk(1, "%s(): file io in progress\n", __func__);
+		dprintk(1, "file io in progress\n");
 		return -EBUSY;
 	}
 
@@ -1938,7 +1935,7 @@ static int vb2_internal_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool n
 	int ret;
 
 	if (b->type != q->type) {
-		dprintk(1, "dqbuf: invalid buffer type\n");
+		dprintk(1, "invalid buffer type\n");
 		return -EINVAL;
 	}
 	ret = __vb2_get_done_vb(q, &vb, b, nonblocking);
@@ -1947,13 +1944,13 @@ static int vb2_internal_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool n
 
 	switch (vb->state) {
 	case VB2_BUF_STATE_DONE:
-		dprintk(3, "dqbuf: Returning done buffer\n");
+		dprintk(3, "Returning done buffer\n");
 		break;
 	case VB2_BUF_STATE_ERROR:
-		dprintk(3, "dqbuf: Returning done buffer with errors\n");
+		dprintk(3, "Returning done buffer with errors\n");
 		break;
 	default:
-		dprintk(1, "dqbuf: Invalid buffer state\n");
+		dprintk(1, "Invalid buffer state\n");
 		return -EINVAL;
 	}
 
@@ -1997,7 +1994,7 @@ static int vb2_internal_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool n
 int vb2_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool nonblocking)
 {
 	if (q->fileio) {
-		dprintk(1, "dqbuf: file io in progress\n");
+		dprintk(1, "file io in progress\n");
 		return -EBUSY;
 	}
 	return vb2_internal_dqbuf(q, b, nonblocking);
@@ -2069,26 +2066,26 @@ static int vb2_internal_streamon(struct vb2_queue *q, enum v4l2_buf_type type)
 	int ret;
 
 	if (type != q->type) {
-		dprintk(1, "streamon: invalid stream type\n");
+		dprintk(1, "invalid stream type\n");
 		return -EINVAL;
 	}
 
 	if (q->streaming) {
-		dprintk(3, "streamon successful: already streaming\n");
+		dprintk(3, "already streaming\n");
 		return 0;
 	}
 
 	if (!q->num_buffers) {
-		dprintk(1, "streamon: no buffers have been allocated\n");
+		dprintk(1, "no buffers have been allocated\n");
 		return -EINVAL;
 	}
 
 	if (!q->num_buffers) {
-		dprintk(1, "streamon: no buffers have been allocated\n");
+		dprintk(1, "no buffers have been allocated\n");
 		return -EINVAL;
 	}
 	if (q->num_buffers < q->min_buffers_needed) {
-		dprintk(1, "streamon: need at least %u allocated buffers\n",
+		dprintk(1, "need at least %u allocated buffers\n",
 				q->min_buffers_needed);
 		return -EINVAL;
 	}
@@ -2107,7 +2104,7 @@ static int vb2_internal_streamon(struct vb2_queue *q, enum v4l2_buf_type type)
 
 	q->streaming = 1;
 
-	dprintk(3, "Streamon successful\n");
+	dprintk(3, "successful\n");
 	return 0;
 }
 
@@ -2127,7 +2124,7 @@ static int vb2_internal_streamon(struct vb2_queue *q, enum v4l2_buf_type type)
 int vb2_streamon(struct vb2_queue *q, enum v4l2_buf_type type)
 {
 	if (q->fileio) {
-		dprintk(1, "streamon: file io in progress\n");
+		dprintk(1, "file io in progress\n");
 		return -EBUSY;
 	}
 	return vb2_internal_streamon(q, type);
@@ -2137,7 +2134,7 @@ EXPORT_SYMBOL_GPL(vb2_streamon);
 static int vb2_internal_streamoff(struct vb2_queue *q, enum v4l2_buf_type type)
 {
 	if (type != q->type) {
-		dprintk(1, "streamoff: invalid stream type\n");
+		dprintk(1, "invalid stream type\n");
 		return -EINVAL;
 	}
 
@@ -2152,7 +2149,7 @@ static int vb2_internal_streamoff(struct vb2_queue *q, enum v4l2_buf_type type)
 	 */
 	__vb2_queue_cancel(q);
 
-	dprintk(3, "Streamoff successful\n");
+	dprintk(3, "successful\n");
 	return 0;
 }
 
@@ -2174,7 +2171,7 @@ static int vb2_internal_streamoff(struct vb2_queue *q, enum v4l2_buf_type type)
 int vb2_streamoff(struct vb2_queue *q, enum v4l2_buf_type type)
 {
 	if (q->fileio) {
-		dprintk(1, "streamoff: file io in progress\n");
+		dprintk(1, "file io in progress\n");
 		return -EBUSY;
 	}
 	return vb2_internal_streamoff(q, type);
@@ -2242,7 +2239,7 @@ int vb2_expbuf(struct vb2_queue *q, struct v4l2_exportbuffer *eb)
 	}
 
 	if (eb->type != q->type) {
-		dprintk(1, "qbuf: invalid buffer type\n");
+		dprintk(1, "invalid buffer type\n");
 		return -EINVAL;
 	}
 
@@ -2756,7 +2753,7 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
 	struct vb2_fileio_buf *buf;
 	int ret, index;
 
-	dprintk(3, "file io: mode %s, offset %ld, count %zd, %sblocking\n",
+	dprintk(3, "mode %s, offset %ld, count %zd, %sblocking\n",
 		read ? "read" : "write", (long)*ppos, count,
 		nonblock ? "non" : "");
 
@@ -2768,7 +2765,7 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
 	 */
 	if (!q->fileio) {
 		ret = __vb2_init_fileio(q, read);
-		dprintk(3, "file io: vb2_init_fileio result: %d\n", ret);
+		dprintk(3, "vb2_init_fileio result: %d\n", ret);
 		if (ret)
 			return ret;
 	}
@@ -2786,7 +2783,7 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
 		fileio->b.type = q->type;
 		fileio->b.memory = q->memory;
 		ret = vb2_internal_dqbuf(q, &fileio->b, nonblock);
-		dprintk(5, "file io: vb2_dqbuf result: %d\n", ret);
+		dprintk(5, "vb2_dqbuf result: %d\n", ret);
 		if (ret)
 			return ret;
 		fileio->dq_count += 1;
@@ -2816,14 +2813,14 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
 	/*
 	 * Transfer data to userspace.
 	 */
-	dprintk(3, "file io: copying %zd bytes - buffer %d, offset %u\n",
+	dprintk(3, "copying %zd bytes - buffer %d, offset %u\n",
 		count, index, buf->pos);
 	if (read)
 		ret = copy_to_user(data, buf->vaddr + buf->pos, count);
 	else
 		ret = copy_from_user(buf->vaddr + buf->pos, data, count);
 	if (ret) {
-		dprintk(3, "file io: error copying data\n");
+		dprintk(3, "error copying data\n");
 		return -EFAULT;
 	}
 
@@ -2843,7 +2840,7 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
 		 */
 		if (read && (fileio->flags & VB2_FILEIO_READ_ONCE) &&
 		    fileio->dq_count == 1) {
-			dprintk(3, "file io: read limit reached\n");
+			dprintk(3, "read limit reached\n");
 			return __vb2_cleanup_fileio(q);
 		}
 
@@ -2856,7 +2853,7 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
 		fileio->b.index = index;
 		fileio->b.bytesused = buf->pos;
 		ret = vb2_internal_qbuf(q, &fileio->b);
-		dprintk(5, "file io: vb2_dbuf result: %d\n", ret);
+		dprintk(5, "vb2_dbuf result: %d\n", ret);
 		if (ret)
 			return ret;
 
-- 
1.9.1

[REVIEWv2 PATCH 05/13] vb2: move __qbuf_mmap before __qbuf_userptr

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

__qbuf_mmap was sort of hidden in between the much larger __qbuf_userptr
and __qbuf_dmabuf functions. Move it before __qbuf_userptr which is
also conform the usual order these memory models are implemented: first
mmap, then userptr, then dmabuf.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Pawel Osciak <pawel@osciak.com>
---
 drivers/media/v4l2-core/videobuf2-core.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index 1421075..2e448a7 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1240,6 +1240,20 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b
 }
 
 /**
+ * __qbuf_mmap() - handle qbuf of an MMAP buffer
+ */
+static int __qbuf_mmap(struct vb2_buffer *vb, const struct v4l2_buffer *b)
+{
+	int ret;
+
+	__fill_vb2_buffer(vb, b, vb->v4l2_planes);
+	ret = call_vb_qop(vb, buf_prepare, vb);
+	if (ret)
+		fail_vb_qop(vb, buf_prepare);
+	return ret;
+}
+
+/**
  * __qbuf_userptr() - handle qbuf of a USERPTR buffer
  */
 static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
@@ -1346,20 +1360,6 @@ err:
 }
 
 /**
- * __qbuf_mmap() - handle qbuf of an MMAP buffer
- */
-static int __qbuf_mmap(struct vb2_buffer *vb, const struct v4l2_buffer *b)
-{
-	int ret;
-
-	__fill_vb2_buffer(vb, b, vb->v4l2_planes);
-	ret = call_vb_qop(vb, buf_prepare, vb);
-	if (ret)
-		fail_vb_qop(vb, buf_prepare);
-	return ret;
-}
-
-/**
  * __qbuf_dmabuf() - handle qbuf of a DMABUF buffer
  */
 static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
-- 
1.9.1

[REVIEWv2 PATCH 06/13] vb2: set timestamp when using write()

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

When using write() to write data to an output video node the vb2 core
should set timestamps if V4L2_BUF_FLAG_TIMESTAMP_COPY is set. Nobody
else is able to provide this information with the write() operation.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/v4l2-core/videobuf2-core.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index 2e448a7..b7de6be 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -22,6 +22,7 @@
 #include <media/v4l2-dev.h>
 #include <media/v4l2-fh.h>
 #include <media/v4l2-event.h>
+#include <media/v4l2-common.h>
 #include <media/videobuf2-core.h>
 
 static int debug;
@@ -2751,6 +2752,9 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
 {
 	struct vb2_fileio_data *fileio;
 	struct vb2_fileio_buf *buf;
+	bool set_timestamp = !read &&
+		(q->timestamp_flags & V4L2_BUF_FLAG_TIMESTAMP_MASK) ==
+		V4L2_BUF_FLAG_TIMESTAMP_COPY;
 	int ret, index;
 
 	dprintk(3, "mode %s, offset %ld, count %zd, %sblocking\n",
@@ -2852,6 +2856,8 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
 		fileio->b.memory = q->memory;
 		fileio->b.index = index;
 		fileio->b.bytesused = buf->pos;
+		if (set_timestamp)
+			v4l2_get_timestamp(&fileio->b.timestamp);
 		ret = vb2_internal_qbuf(q, &fileio->b);
 		dprintk(5, "vb2_dbuf result: %d\n", ret);
 		if (ret)
-- 
1.9.1

[REVIEWv2 PATCH 07/13] vb2: reject output buffers with V4L2_FIELD_ALTERNATE

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

This is not allowed by the spec and does in fact not make any sense.
Return -EINVAL if this is the case.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/v4l2-core/videobuf2-core.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index b7de6be..c662ad9 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1511,6 +1511,19 @@ static int __buf_prepare(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 		dprintk(1, "plane parameters verification failed: %d\n", ret);
 		return ret;
 	}
+	if (b->field == V4L2_FIELD_ALTERNATE && V4L2_TYPE_IS_OUTPUT(q->type)) {
+		/*
+		 * If the format's field is ALTERNATE, then the buffer's field
+		 * should be either TOP or BOTTOM, not ALTERNATE since that
+		 * makes no sense. The driver has to know whether the
+		 * buffer represents a top or a bottom field in order to
+		 * program any DMA correctly. Using ALTERNATE is wrong, since
+		 * that just says that it is either a top or a bottom field,
+		 * but not which of the two it is.
+		 */
+		dprintk(1, "the field is incorrectly set to ALTERNATE for an output buffer\n");
+		return -EINVAL;
+	}
 
 	vb->state = VB2_BUF_STATE_PREPARING;
 	vb->v4l2_buf.timestamp.tv_sec = 0;
-- 
1.9.1

[REVIEWv2 PATCH 08/13] vb2: simplify a confusing condition.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

q->start_streaming_called is always true, so the WARN_ON check against
it being false can be dropped.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Pawel Osciak <pawel@osciak.com>
---
 drivers/media/v4l2-core/videobuf2-core.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index c662ad9..89147d2 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1094,9 +1094,8 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
 	if (!q->start_streaming_called) {
 		if (WARN_ON(state != VB2_BUF_STATE_QUEUED))
 			state = VB2_BUF_STATE_QUEUED;
-	} else if (!WARN_ON(!q->start_streaming_called)) {
-		if (WARN_ON(state != VB2_BUF_STATE_DONE &&
-			    state != VB2_BUF_STATE_ERROR))
+	} else if (WARN_ON(state != VB2_BUF_STATE_DONE &&
+			   state != VB2_BUF_STATE_ERROR)) {
 			state = VB2_BUF_STATE_ERROR;
 	}
 
-- 
1.9.1

[REVIEWv2 PATCH 09/13] vb2: add vb2_fileio_is_active and check it more often

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Added a vb2_fileio_is_active inline function that returns true if fileio
is in progress. Check for this too in mmap() (you don't want apps mmap()ing
buffers used by fileio) and expbuf() (same reason).

In addition drivers should be able to check for this in queue_setup() to
return an error if an attempt is made to read() or write() with
V4L2_FIELD_ALTERNATE being configured. This is illegal (there is no way
to pass the TOP/BOTTOM information around using file I/O).

However, in order to be able to check for this the init_fileio function
needs to set q->fileio early on, before the buffers are allocated. So switch
to using internal functions (__reqbufs, vb2_internal_qbuf and
vb2_internal_streamon) to skip the fileio check. Well, that's why the internal
functions were created...

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/v4l2-core/videobuf2-core.c | 39 ++++++++++++++++++++------------
 include/media/videobuf2-core.h           | 17 ++++++++++++++
 2 files changed, 41 insertions(+), 15 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index 89147d2..08152dd 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -755,7 +755,7 @@ static int __verify_memory_type(struct vb2_queue *q,
 	 * create_bufs is called with count == 0, but count == 0 should still
 	 * do the memory and type validation.
 	 */
-	if (q->fileio) {
+	if (vb2_fileio_is_active(q)) {
 		dprintk(1, "file io in progress\n");
 		return -EBUSY;
 	}
@@ -1617,7 +1617,7 @@ int vb2_prepare_buf(struct vb2_queue *q, struct v4l2_buffer *b)
 	struct vb2_buffer *vb;
 	int ret;
 
-	if (q->fileio) {
+	if (vb2_fileio_is_active(q)) {
 		dprintk(1, "file io in progress\n");
 		return -EBUSY;
 	}
@@ -1786,7 +1786,7 @@ static int vb2_internal_qbuf(struct vb2_queue *q, struct v4l2_buffer *b)
  */
 int vb2_qbuf(struct vb2_queue *q, struct v4l2_buffer *b)
 {
-	if (q->fileio) {
+	if (vb2_fileio_is_active(q)) {
 		dprintk(1, "file io in progress\n");
 		return -EBUSY;
 	}
@@ -2006,7 +2006,7 @@ static int vb2_internal_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool n
  */
 int vb2_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool nonblocking)
 {
-	if (q->fileio) {
+	if (vb2_fileio_is_active(q)) {
 		dprintk(1, "file io in progress\n");
 		return -EBUSY;
 	}
@@ -2136,7 +2136,7 @@ static int vb2_internal_streamon(struct vb2_queue *q, enum v4l2_buf_type type)
  */
 int vb2_streamon(struct vb2_queue *q, enum v4l2_buf_type type)
 {
-	if (q->fileio) {
+	if (vb2_fileio_is_active(q)) {
 		dprintk(1, "file io in progress\n");
 		return -EBUSY;
 	}
@@ -2183,7 +2183,7 @@ static int vb2_internal_streamoff(struct vb2_queue *q, enum v4l2_buf_type type)
  */
 int vb2_streamoff(struct vb2_queue *q, enum v4l2_buf_type type)
 {
-	if (q->fileio) {
+	if (vb2_fileio_is_active(q)) {
 		dprintk(1, "file io in progress\n");
 		return -EBUSY;
 	}
@@ -2268,6 +2268,11 @@ int vb2_expbuf(struct vb2_queue *q, struct v4l2_exportbuffer *eb)
 		return -EINVAL;
 	}
 
+	if (vb2_fileio_is_active(q)) {
+		dprintk(1, "expbuf: file io in progress\n");
+		return -EBUSY;
+	}
+
 	vb_plane = &vb->planes[eb->plane];
 
 	dbuf = call_memop(vb, get_dmabuf, vb_plane->mem_priv, eb->flags & O_ACCMODE);
@@ -2344,6 +2349,10 @@ int vb2_mmap(struct vb2_queue *q, struct vm_area_struct *vma)
 			return -EINVAL;
 		}
 	}
+	if (vb2_fileio_is_active(q)) {
+		dprintk(1, "mmap: file io in progress\n");
+		return -EBUSY;
+	}
 
 	/*
 	 * Find the plane corresponding to the offset passed by userspace.
@@ -2455,7 +2464,7 @@ unsigned int vb2_poll(struct vb2_queue *q, struct file *file, poll_table *wait)
 	/*
 	 * Start file I/O emulator only if streaming API has not been used yet.
 	 */
-	if (q->num_buffers == 0 && q->fileio == NULL) {
+	if (q->num_buffers == 0 && !vb2_fileio_is_active(q)) {
 		if (!V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_READ) &&
 				(req_events & (POLLIN | POLLRDNORM))) {
 			if (__vb2_init_fileio(q, 1))
@@ -2660,7 +2669,8 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read)
 	fileio->req.count = count;
 	fileio->req.memory = V4L2_MEMORY_MMAP;
 	fileio->req.type = q->type;
-	ret = vb2_reqbufs(q, &fileio->req);
+	q->fileio = fileio;
+	ret = __reqbufs(q, &fileio->req);
 	if (ret)
 		goto err_kfree;
 
@@ -2698,7 +2708,7 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read)
 			b->type = q->type;
 			b->memory = q->memory;
 			b->index = i;
-			ret = vb2_qbuf(q, b);
+			ret = vb2_internal_qbuf(q, b);
 			if (ret)
 				goto err_reqbufs;
 			fileio->bufs[i].queued = 1;
@@ -2714,19 +2724,18 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read)
 	/*
 	 * Start streaming.
 	 */
-	ret = vb2_streamon(q, q->type);
+	ret = vb2_internal_streamon(q, q->type);
 	if (ret)
 		goto err_reqbufs;
 
-	q->fileio = fileio;
-
 	return ret;
 
 err_reqbufs:
 	fileio->req.count = 0;
-	vb2_reqbufs(q, &fileio->req);
+	__reqbufs(q, &fileio->req);
 
 err_kfree:
+	q->fileio = NULL;
 	kfree(fileio);
 	return ret;
 }
@@ -2779,7 +2788,7 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
 	/*
 	 * Initialize emulator on first call.
 	 */
-	if (!q->fileio) {
+	if (!vb2_fileio_is_active(q)) {
 		ret = __vb2_init_fileio(q, read);
 		dprintk(3, "vb2_init_fileio result: %d\n", ret);
 		if (ret)
@@ -3147,7 +3156,7 @@ unsigned int vb2_fop_poll(struct file *file, poll_table *wait)
 
 	/* Try to be smart: only lock if polling might start fileio,
 	   otherwise locking will only introduce unwanted delays. */
-	if (q->num_buffers == 0 && q->fileio == NULL) {
+	if (q->num_buffers == 0 && !vb2_fileio_is_active(q)) {
 		if (!V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_READ) &&
 				(req_events & (POLLIN | POLLRDNORM)))
 			must_lock = true;
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 3b57851..af34ae0 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -472,6 +472,23 @@ static inline bool vb2_is_streaming(struct vb2_queue *q)
 }
 
 /**
+ * vb2_fileio_is_active() - return true if fileio is active.
+ * @q:		videobuf queue
+ *
+ * This returns true if read() or write() is used to stream the data
+ * as opposed to stream I/O. This is almost never an important distinction,
+ * except in rare cases. One such case is that using read() or write() to
+ * stream a format using V4L2_FIELD_ALTERNATE is not allowed since there
+ * is no way you can pass the field information of each buffer to/from
+ * userspace. A driver that supports this field format should check for
+ * this in the queue_setup op and reject it if this function returns true.
+ */
+static inline bool vb2_fileio_is_active(struct vb2_queue *q)
+{
+	return q->fileio;
+}
+
+/**
  * vb2_is_busy() - return busy status of the queue
  * @q:		videobuf queue
  *
-- 
1.9.1

[REVIEWv2 PATCH 10/13] vb2: set v4l2_buffer.bytesused to 0 for mp buffers

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

The bytesused field of struct v4l2_buffer is not used for multiplanar
formats, so just zero it to prevent it from having some random value.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/v4l2-core/videobuf2-core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index 08152dd..ef7ef82 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -582,6 +582,7 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b)
 		 * for it. The caller has already verified memory and size.
 		 */
 		b->length = vb->num_planes;
+		b->bytesused = 0;
 		memcpy(b->m.planes, vb->v4l2_planes,
 			b->length * sizeof(struct v4l2_plane));
 	} else {
-- 
1.9.1

[REVIEWv2 PATCH 11/13] vb2: allow read/write as long as the format is single planar

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

It was impossible to read() or write() a frame if the queue type was multiplanar.
Even if the current format is single planar. Change this to just check whether
the number of planes is 1 or more.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/v4l2-core/videobuf2-core.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index ef7ef82..d33c69b 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -2610,6 +2610,7 @@ struct vb2_fileio_buf {
  */
 struct vb2_fileio_data {
 	struct v4l2_requestbuffers req;
+	struct v4l2_plane p;
 	struct v4l2_buffer b;
 	struct vb2_fileio_buf bufs[VIDEO_MAX_FRAME];
 	unsigned int cur_index;
@@ -2700,13 +2701,21 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read)
 	 * Read mode requires pre queuing of all buffers.
 	 */
 	if (read) {
+		bool is_multiplanar = V4L2_TYPE_IS_MULTIPLANAR(q->type);
+
 		/*
 		 * Queue all buffers.
 		 */
 		for (i = 0; i < q->num_buffers; i++) {
 			struct v4l2_buffer *b = &fileio->b;
+
 			memset(b, 0, sizeof(*b));
 			b->type = q->type;
+			if (is_multiplanar) {
+				memset(&fileio->p, 0, sizeof(fileio->p));
+				b->m.planes = &fileio->p;
+				b->length = 1;
+			}
 			b->memory = q->memory;
 			b->index = i;
 			ret = vb2_internal_qbuf(q, b);
@@ -2774,6 +2783,7 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
 {
 	struct vb2_fileio_data *fileio;
 	struct vb2_fileio_buf *buf;
+	bool is_multiplanar = V4L2_TYPE_IS_MULTIPLANAR(q->type);
 	bool set_timestamp = !read &&
 		(q->timestamp_flags & V4L2_BUF_FLAG_TIMESTAMP_MASK) ==
 		V4L2_BUF_FLAG_TIMESTAMP_COPY;
@@ -2808,6 +2818,11 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
 		memset(&fileio->b, 0, sizeof(fileio->b));
 		fileio->b.type = q->type;
 		fileio->b.memory = q->memory;
+		if (is_multiplanar) {
+			memset(&fileio->p, 0, sizeof(fileio->p));
+			fileio->b.m.planes = &fileio->p;
+			fileio->b.length = 1;
+		}
 		ret = vb2_internal_dqbuf(q, &fileio->b, nonblock);
 		dprintk(5, "vb2_dqbuf result: %d\n", ret);
 		if (ret)
@@ -2878,6 +2893,12 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
 		fileio->b.memory = q->memory;
 		fileio->b.index = index;
 		fileio->b.bytesused = buf->pos;
+		if (is_multiplanar) {
+			memset(&fileio->p, 0, sizeof(fileio->p));
+			fileio->p.bytesused = buf->pos;
+			fileio->b.m.planes = &fileio->p;
+			fileio->b.length = 1;
+		}
 		if (set_timestamp)
 			v4l2_get_timestamp(&fileio->b.timestamp);
 		ret = vb2_internal_qbuf(q, &fileio->b);
-- 
1.9.1

[REVIEWv2 PATCH 12/13] vb2: start messages with a lower-case for consistency.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

The kernel debug messages produced by vb2 started either with a
lower or an upper case character. Switched all to use lower-case
which seemed to be what was used in the majority of the messages.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/v4l2-core/videobuf2-core.c | 58 ++++++++++++++++----------------
 1 file changed, 29 insertions(+), 29 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index d33c69b..e79d70c 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -151,7 +151,7 @@ static void __vb2_buf_mem_free(struct vb2_buffer *vb)
 	for (plane = 0; plane < vb->num_planes; ++plane) {
 		call_memop(vb, put, vb->planes[plane].mem_priv);
 		vb->planes[plane].mem_priv = NULL;
-		dprintk(3, "Freed plane %d of buffer %d\n", plane,
+		dprintk(3, "freed plane %d of buffer %d\n", plane,
 			vb->v4l2_buf.index);
 	}
 }
@@ -246,7 +246,7 @@ static void __setup_offsets(struct vb2_queue *q, unsigned int n)
 		for (plane = 0; plane < vb->num_planes; ++plane) {
 			vb->v4l2_planes[plane].m.mem_offset = off;
 
-			dprintk(3, "Buffer %d, plane %d offset 0x%08lx\n",
+			dprintk(3, "buffer %d, plane %d offset 0x%08lx\n",
 					buffer, plane, off);
 
 			off += vb->v4l2_planes[plane].length;
@@ -273,7 +273,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum v4l2_memory memory,
 		/* Allocate videobuf buffer structures */
 		vb = kzalloc(q->buf_struct_size, GFP_KERNEL);
 		if (!vb) {
-			dprintk(1, "Memory alloc for buffer struct failed\n");
+			dprintk(1, "memory alloc for buffer struct failed\n");
 			break;
 		}
 
@@ -292,7 +292,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum v4l2_memory memory,
 		if (memory == V4L2_MEMORY_MMAP) {
 			ret = __vb2_buf_mem_alloc(vb);
 			if (ret) {
-				dprintk(1, "Failed allocating memory for "
+				dprintk(1, "failed allocating memory for "
 						"buffer %d\n", buffer);
 				kfree(vb);
 				break;
@@ -304,7 +304,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum v4l2_memory memory,
 			 */
 			ret = call_vb_qop(vb, buf_init, vb);
 			if (ret) {
-				dprintk(1, "Buffer %d %p initialization"
+				dprintk(1, "buffer %d %p initialization"
 					" failed\n", buffer, vb);
 				fail_vb_qop(vb, buf_init);
 				__vb2_buf_mem_free(vb);
@@ -320,7 +320,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum v4l2_memory memory,
 	if (memory == V4L2_MEMORY_MMAP)
 		__setup_offsets(q, buffer);
 
-	dprintk(1, "Allocated %d buffers, %d plane(s) each\n",
+	dprintk(1, "allocated %d buffers, %d plane(s) each\n",
 			buffer, num_planes);
 
 	return buffer;
@@ -477,13 +477,13 @@ static int __verify_planes_array(struct vb2_buffer *vb, const struct v4l2_buffer
 
 	/* Is memory for copying plane information present? */
 	if (NULL == b->m.planes) {
-		dprintk(1, "Multi-planar buffer passed but "
+		dprintk(1, "multi-planar buffer passed but "
 			   "planes array not provided\n");
 		return -EINVAL;
 	}
 
 	if (b->length < vb->num_planes || b->length > VIDEO_MAX_PLANES) {
-		dprintk(1, "Incorrect planes array length, "
+		dprintk(1, "incorrect planes array length, "
 			   "expected %d, got %d\n", vb->num_planes, b->length);
 		return -EINVAL;
 	}
@@ -847,7 +847,7 @@ static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
 	/* Finally, allocate buffers and video memory */
 	allocated_buffers = __vb2_queue_alloc(q, req->memory, num_buffers, num_planes);
 	if (allocated_buffers == 0) {
-		dprintk(1, "Memory allocation failed\n");
+		dprintk(1, "memory allocation failed\n");
 		return -ENOMEM;
 	}
 
@@ -960,7 +960,7 @@ static int __create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create
 	allocated_buffers = __vb2_queue_alloc(q, create->memory, num_buffers,
 				num_planes);
 	if (allocated_buffers == 0) {
-		dprintk(1, "Memory allocation failed\n");
+		dprintk(1, "memory allocation failed\n");
 		return -ENOMEM;
 	}
 
@@ -1107,7 +1107,7 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
 	 */
 	vb->cnt_buf_done++;
 #endif
-	dprintk(4, "Done processing on buffer %d, state: %d\n",
+	dprintk(4, "done processing on buffer %d, state: %d\n",
 			vb->v4l2_buf.index, state);
 
 	/* sync buffers */
@@ -1817,7 +1817,7 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, int nonblocking)
 		int ret;
 
 		if (!q->streaming) {
-			dprintk(1, "Streaming off, will not wait for buffers\n");
+			dprintk(1, "streaming off, will not wait for buffers\n");
 			return -EINVAL;
 		}
 
@@ -1829,7 +1829,7 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, int nonblocking)
 		}
 
 		if (nonblocking) {
-			dprintk(1, "Nonblocking and no buffers to dequeue, "
+			dprintk(1, "nonblocking and no buffers to dequeue, "
 								"will not wait\n");
 			return -EAGAIN;
 		}
@@ -1844,7 +1844,7 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, int nonblocking)
 		/*
 		 * All locks have been released, it is safe to sleep now.
 		 */
-		dprintk(3, "Will sleep waiting for buffers\n");
+		dprintk(3, "will sleep waiting for buffers\n");
 		ret = wait_event_interruptible(q->done_wq,
 				!list_empty(&q->done_list) || !q->streaming);
 
@@ -1854,7 +1854,7 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, int nonblocking)
 		 */
 		call_qop(q, wait_finish, q);
 		if (ret) {
-			dprintk(1, "Sleep was interrupted\n");
+			dprintk(1, "sleep was interrupted\n");
 			return ret;
 		}
 	}
@@ -1909,7 +1909,7 @@ static int __vb2_get_done_vb(struct vb2_queue *q, struct vb2_buffer **vb,
 int vb2_wait_for_all_buffers(struct vb2_queue *q)
 {
 	if (!q->streaming) {
-		dprintk(1, "Streaming off, will not wait for buffers\n");
+		dprintk(1, "streaming off, will not wait for buffers\n");
 		return -EINVAL;
 	}
 
@@ -1958,13 +1958,13 @@ static int vb2_internal_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool n
 
 	switch (vb->state) {
 	case VB2_BUF_STATE_DONE:
-		dprintk(3, "Returning done buffer\n");
+		dprintk(3, "returning done buffer\n");
 		break;
 	case VB2_BUF_STATE_ERROR:
-		dprintk(3, "Returning done buffer with errors\n");
+		dprintk(3, "returning done buffer with errors\n");
 		break;
 	default:
-		dprintk(1, "Invalid buffer state\n");
+		dprintk(1, "invalid buffer state\n");
 		return -EINVAL;
 	}
 
@@ -2238,17 +2238,17 @@ int vb2_expbuf(struct vb2_queue *q, struct v4l2_exportbuffer *eb)
 	struct dma_buf *dbuf;
 
 	if (q->memory != V4L2_MEMORY_MMAP) {
-		dprintk(1, "Queue is not currently set up for mmap\n");
+		dprintk(1, "queue is not currently set up for mmap\n");
 		return -EINVAL;
 	}
 
 	if (!q->mem_ops->get_dmabuf) {
-		dprintk(1, "Queue does not support DMA buffer exporting\n");
+		dprintk(1, "queue does not support DMA buffer exporting\n");
 		return -EINVAL;
 	}
 
 	if (eb->flags & ~(O_CLOEXEC | O_ACCMODE)) {
-		dprintk(1, "Queue does support only O_CLOEXEC and access mode flags\n");
+		dprintk(1, "queue does support only O_CLOEXEC and access mode flags\n");
 		return -EINVAL;
 	}
 
@@ -2278,7 +2278,7 @@ int vb2_expbuf(struct vb2_queue *q, struct v4l2_exportbuffer *eb)
 
 	dbuf = call_memop(vb, get_dmabuf, vb_plane->mem_priv, eb->flags & O_ACCMODE);
 	if (IS_ERR_OR_NULL(dbuf)) {
-		dprintk(1, "Failed to export buffer %d, plane %d\n",
+		dprintk(1, "failed to export buffer %d, plane %d\n",
 			eb->index, eb->plane);
 		fail_memop(vb, get_dmabuf);
 		return -EINVAL;
@@ -2328,7 +2328,7 @@ int vb2_mmap(struct vb2_queue *q, struct vm_area_struct *vma)
 	unsigned long length;
 
 	if (q->memory != V4L2_MEMORY_MMAP) {
-		dprintk(1, "Queue is not currently set up for mmap\n");
+		dprintk(1, "queue is not currently set up for mmap\n");
 		return -EINVAL;
 	}
 
@@ -2336,17 +2336,17 @@ int vb2_mmap(struct vb2_queue *q, struct vm_area_struct *vma)
 	 * Check memory area access mode.
 	 */
 	if (!(vma->vm_flags & VM_SHARED)) {
-		dprintk(1, "Invalid vma flags, VM_SHARED needed\n");
+		dprintk(1, "invalid vma flags, VM_SHARED needed\n");
 		return -EINVAL;
 	}
 	if (V4L2_TYPE_IS_OUTPUT(q->type)) {
 		if (!(vma->vm_flags & VM_WRITE)) {
-			dprintk(1, "Invalid vma flags, VM_WRITE needed\n");
+			dprintk(1, "invalid vma flags, VM_WRITE needed\n");
 			return -EINVAL;
 		}
 	} else {
 		if (!(vma->vm_flags & VM_READ)) {
-			dprintk(1, "Invalid vma flags, VM_READ needed\n");
+			dprintk(1, "invalid vma flags, VM_READ needed\n");
 			return -EINVAL;
 		}
 	}
@@ -2382,7 +2382,7 @@ int vb2_mmap(struct vb2_queue *q, struct vm_area_struct *vma)
 		return ret;
 	}
 
-	dprintk(3, "Buffer %d, plane %d successfully mapped\n", buffer, plane);
+	dprintk(3, "buffer %d, plane %d successfully mapped\n", buffer, plane);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(vb2_mmap);
@@ -2400,7 +2400,7 @@ unsigned long vb2_get_unmapped_area(struct vb2_queue *q,
 	int ret;
 
 	if (q->memory != V4L2_MEMORY_MMAP) {
-		dprintk(1, "Queue is not currently set up for mmap\n");
+		dprintk(1, "queue is not currently set up for mmap\n");
 		return -EINVAL;
 	}
 
-- 
1.9.1

[REVIEWv2 PATCH 13/13] DocBook media: update bytesused field description

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

For output buffers the application has to set the bytesused field.
In reality applications often do not set this since drivers that
deal with fix image sizes just override it anyway.

The vb2 framework will replace this field with the length field if
bytesused was set to 0 by the application, which is what happens
in practice. Document this behavior.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 Documentation/DocBook/media/v4l/io.xml | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
index 97a69bf..188e621 100644
--- a/Documentation/DocBook/media/v4l/io.xml
+++ b/Documentation/DocBook/media/v4l/io.xml
@@ -699,7 +699,12 @@ linkend="v4l2-buf-type" /></entry>
 buffer. It depends on the negotiated data format and may change with
 each buffer for compressed variable size data like JPEG images.
 Drivers must set this field when <structfield>type</structfield>
-refers to an input stream, applications when it refers to an output stream.</entry>
+refers to an input stream, applications when it refers to an output stream.
+If the application sets this to 0 for an output stream, then
+<structfield>bytesused</structfield> will be set to the size of the
+buffer (see the <structfield>length</structfield> field of this struct) by
+the driver. For multiplanar formats this field is ignored and the
+<structfield>planes</structfield> pointer is used instead.</entry>
 	  </row>
 	  <row>
 	    <entry>__u32</entry>
@@ -861,7 +866,11 @@ should set this to 0.</entry>
 	    <entry></entry>
 	    <entry>The number of bytes occupied by data in the plane
 	      (its payload). Drivers must set this field when <structfield>type</structfield>
-	      refers to an input stream, applications when it refers to an output stream.</entry>
+	      refers to an input stream, applications when it refers to an output stream.
+	      If the application sets this to 0 for an output stream, then
+	      <structfield>bytesused</structfield> will be set to the size of the
+	      plane (see the <structfield>length</structfield> field of this struct)
+	      by the driver.</entry>
 	  </row>
 	  <row>
 	    <entry>__u32</entry>
-- 
1.9.1

Re: [REVIEWv2 PATCH 02/13] vb2: fix handling of data_offset and v4l2_plane.reserved[]

[Pawel Osciak]

Looks good to me, just a small nit below.


On Mon, Apr 7, 2014 at 10:11 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> The videobuf2-core did not zero the 'planes' array in __qbuf_userptr()
> and __qbuf_dmabuf(). That's now memset to 0. Without this the reserved
> array in struct v4l2_plane would be non-zero, causing v4l2-compliance
> errors.
>
> More serious is the fact that data_offset was not handled correctly:
>
> - for capture devices it was never zeroed, which meant that it was
>   uninitialized. Unless the driver sets it it was a completely random
>   number. With the memset above this is now fixed.
>
> - __qbuf_dmabuf had a completely incorrect length check that included
>   data_offset.
>
> - in __fill_vb2_buffer in the DMABUF case the data_offset field was
>   unconditionally copied from v4l2_buffer to v4l2_plane when this
>   should only happen in the output case.
>
> - in the single-planar case data_offset was never correctly set to 0.
>   The single-planar API doesn't support data_offset, so setting it
>   to 0 is the right thing to do. This too is now solved by the memset.
>
> All these issues were found with v4l2-compliance.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Acked-by: Pawel Osciak <pawel@osciak.com>

> ---
>  drivers/media/v4l2-core/videobuf2-core.c | 13 ++++---------
>  1 file changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
> index f9059bb..596998e 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -1169,8 +1169,6 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b
>                                         b->m.planes[plane].m.fd;
>                                 v4l2_planes[plane].length =
>                                         b->m.planes[plane].length;
> -                               v4l2_planes[plane].data_offset =
> -                                       b->m.planes[plane].data_offset;
>                         }
>                 }
>         } else {
> @@ -1180,10 +1178,8 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b
>                  * In videobuf we use our internal V4l2_planes struct for
>                  * single-planar buffers as well, for simplicity.
>                  */
> -               if (V4L2_TYPE_IS_OUTPUT(b->type)) {
> +               if (V4L2_TYPE_IS_OUTPUT(b->type))
>                         v4l2_planes[0].bytesused = b->bytesused;
> -                       v4l2_planes[0].data_offset = 0;
> -               }
>
>                 if (b->memory == V4L2_MEMORY_USERPTR) {
>                         v4l2_planes[0].m.userptr = b->m.userptr;
> @@ -1193,9 +1189,7 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b
>                 if (b->memory == V4L2_MEMORY_DMABUF) {
>                         v4l2_planes[0].m.fd = b->m.fd;
>                         v4l2_planes[0].length = b->length;
> -                       v4l2_planes[0].data_offset = 0;
>                 }
> -
>         }
>
>         /* Zero flags that the vb2 core handles */
> @@ -1238,6 +1232,7 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>         int write = !V4L2_TYPE_IS_OUTPUT(q->type);
>         bool reacquired = vb->planes[0].mem_priv == NULL;
>
> +       memset(planes, 0, sizeof(planes[0]) * vb->num_planes);

memset(planes, 0, sizeof(planes));

>         /* Copy relevant information provided by the userspace */
>         __fill_vb2_buffer(vb, b, planes);
>
> @@ -1357,6 +1352,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>         int write = !V4L2_TYPE_IS_OUTPUT(q->type);
>         bool reacquired = vb->planes[0].mem_priv == NULL;
>
> +       memset(planes, 0, sizeof(planes[0]) * vb->num_planes);

memset(planes, 0, sizeof(planes));

>         /* Copy relevant information provided by the userspace */
>         __fill_vb2_buffer(vb, b, planes);
>
> @@ -1374,8 +1370,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>                 if (planes[plane].length == 0)
>                         planes[plane].length = dbuf->size;
>
> -               if (planes[plane].length < planes[plane].data_offset +
> -                   q->plane_sizes[plane]) {
> +               if (planes[plane].length < q->plane_sizes[plane]) {
>                         dprintk(1, "qbuf: invalid dmabuf length for plane %d\n",
>                                 plane);
>                         ret = -EINVAL;
> --
> 1.9.1
>



-- 
Best regards,
Pawel Osciak

Re: [REVIEWv2 PATCH 04/13] vb2: use correct prefix

[Pawel Osciak]

On Mon, Apr 7, 2014 at 10:11 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> Many dprintk's in vb2 use a hardcoded prefix with the function name. In
> many cases that is now outdated. To keep things consistent the dprintk
> macro has been changed to print the function name in addition to the "vb2:"
> prefix. Superfluous prefixes elsewhere in the code have been removed.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  drivers/media/v4l2-core/videobuf2-core.c | 133 +++++++++++++++----------------
>  1 file changed, 65 insertions(+), 68 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
> index b2582cb..1421075 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -27,10 +27,10 @@
>  static int debug;
>  module_param(debug, int, 0644);
>
> -#define dprintk(level, fmt, arg...)                                    \
> -       do {                                                            \
> -               if (debug >= level)                                     \
> -                       printk(KERN_DEBUG "vb2: " fmt, ## arg);         \
> +#define dprintk(level, fmt, arg...)                                          \
> +       do {                                                                  \
> +               if (debug >= level)                                           \
> +                       pr_debug("vb2: %s: " fmt, __func__, ## arg); \
>         } while (0)
>
>  #ifdef CONFIG_VIDEO_ADV_DEBUG
> @@ -371,7 +371,7 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned int buffers)
>                 if (q->bufs[buffer] == NULL)
>                         continue;
>                 if (q->bufs[buffer]->state == VB2_BUF_STATE_PREPARING) {
> -                       dprintk(1, "reqbufs: preparing buffers, cannot free\n");
> +                       dprintk(1, "preparing buffers, cannot free\n");
>                         return -EAGAIN;
>                 }
>         }
> @@ -656,12 +656,12 @@ int vb2_querybuf(struct vb2_queue *q, struct v4l2_buffer *b)
>         int ret;
>
>         if (b->type != q->type) {
> -               dprintk(1, "querybuf: wrong buffer type\n");
> +               dprintk(1, "wrong buffer type\n");
>                 return -EINVAL;
>         }
>
>         if (b->index >= q->num_buffers) {
> -               dprintk(1, "querybuf: buffer index out of range\n");
> +               dprintk(1, "buffer index out of range\n");
>                 return -EINVAL;
>         }
>         vb = q->bufs[b->index];
> @@ -721,12 +721,12 @@ static int __verify_memory_type(struct vb2_queue *q,
>  {
>         if (memory != V4L2_MEMORY_MMAP && memory != V4L2_MEMORY_USERPTR &&
>             memory != V4L2_MEMORY_DMABUF) {
> -               dprintk(1, "reqbufs: unsupported memory type\n");
> +               dprintk(1, "unsupported memory type\n");
>                 return -EINVAL;
>         }
>
>         if (type != q->type) {
> -               dprintk(1, "reqbufs: requested type is incorrect\n");
> +               dprintk(1, "requested type is incorrect\n");
>                 return -EINVAL;
>         }
>
> @@ -735,17 +735,17 @@ static int __verify_memory_type(struct vb2_queue *q,
>          * are available.
>          */
>         if (memory == V4L2_MEMORY_MMAP && __verify_mmap_ops(q)) {
> -               dprintk(1, "reqbufs: MMAP for current setup unsupported\n");
> +               dprintk(1, "MMAP for current setup unsupported\n");
>                 return -EINVAL;
>         }
>
>         if (memory == V4L2_MEMORY_USERPTR && __verify_userptr_ops(q)) {
> -               dprintk(1, "reqbufs: USERPTR for current setup unsupported\n");
> +               dprintk(1, "USERPTR for current setup unsupported\n");
>                 return -EINVAL;
>         }
>
>         if (memory == V4L2_MEMORY_DMABUF && __verify_dmabuf_ops(q)) {
> -               dprintk(1, "reqbufs: DMABUF for current setup unsupported\n");
> +               dprintk(1, "DMABUF for current setup unsupported\n");
>                 return -EINVAL;
>         }
>
> @@ -755,7 +755,7 @@ static int __verify_memory_type(struct vb2_queue *q,
>          * do the memory and type validation.
>          */
>         if (q->fileio) {
> -               dprintk(1, "reqbufs: file io in progress\n");
> +               dprintk(1, "file io in progress\n");
>                 return -EBUSY;
>         }
>         return 0;
> @@ -790,7 +790,7 @@ static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
>         int ret;
>
>         if (q->streaming) {
> -               dprintk(1, "reqbufs: streaming active\n");
> +               dprintk(1, "streaming active\n");
>                 return -EBUSY;
>         }
>
> @@ -800,7 +800,7 @@ static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
>                  * are not in use and can be freed.
>                  */
>                 if (q->memory == V4L2_MEMORY_MMAP && __buffers_in_use(q)) {
> -                       dprintk(1, "reqbufs: memory in use, cannot free\n");
> +                       dprintk(1, "memory in use, cannot free\n");
>                         return -EBUSY;
>                 }
>
> @@ -931,8 +931,7 @@ static int __create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create
>         int ret;
>
>         if (q->num_buffers == VIDEO_MAX_FRAME) {
> -               dprintk(1, "%s(): maximum number of buffers already allocated\n",
> -                       __func__);
> +               dprintk(1, "maximum number of buffers already allocated\n");
>                 return -ENOBUFS;
>         }
>
> @@ -1264,12 +1263,12 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>                     && vb->v4l2_planes[plane].length == planes[plane].length)
>                         continue;
>
> -               dprintk(3, "qbuf: userspace address for plane %d changed, "
> +               dprintk(3, "userspace address for plane %d changed, "
>                                 "reacquiring memory\n", plane);
>
>                 /* Check if the provided plane buffer is large enough */
>                 if (planes[plane].length < q->plane_sizes[plane]) {
> -                       dprintk(1, "qbuf: provided buffer size %u is less than "
> +                       dprintk(1, "provided buffer size %u is less than "
>                                                 "setup size %u for plane %d\n",
>                                                 planes[plane].length,
>                                                 q->plane_sizes[plane], plane);
> @@ -1294,7 +1293,7 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>                                       planes[plane].m.userptr,
>                                       planes[plane].length, write);
>                 if (IS_ERR_OR_NULL(mem_priv)) {
> -                       dprintk(1, "qbuf: failed acquiring userspace "
> +                       dprintk(1, "failed acquiring userspace "
>                                                 "memory for plane %d\n", plane);
>                         fail_memop(vb, get_userptr);
>                         ret = mem_priv ? PTR_ERR(mem_priv) : -EINVAL;
> @@ -1318,7 +1317,7 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>                  */
>                 ret = call_vb_qop(vb, buf_init, vb);
>                 if (ret) {
> -                       dprintk(1, "qbuf: buffer initialization failed\n");
> +                       dprintk(1, "buffer initialization failed\n");
>                         fail_vb_qop(vb, buf_init);
>                         goto err;
>                 }
> @@ -1326,7 +1325,7 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>
>         ret = call_vb_qop(vb, buf_prepare, vb);
>         if (ret) {
> -               dprintk(1, "qbuf: buffer preparation failed\n");
> +               dprintk(1, "buffer preparation failed\n");
>                 fail_vb_qop(vb, buf_prepare);
>                 call_vb_qop(vb, buf_cleanup, vb);
>                 goto err;
> @@ -1381,7 +1380,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>                 struct dma_buf *dbuf = dma_buf_get(planes[plane].m.fd);
>
>                 if (IS_ERR_OR_NULL(dbuf)) {
> -                       dprintk(1, "qbuf: invalid dmabuf fd for plane %d\n",
> +                       dprintk(1, "invalid dmabuf fd for plane %d\n",
>                                 plane);
>                         ret = -EINVAL;
>                         goto err;
> @@ -1392,7 +1391,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>                         planes[plane].length = dbuf->size;
>
>                 if (planes[plane].length < q->plane_sizes[plane]) {
> -                       dprintk(1, "qbuf: invalid dmabuf length for plane %d\n",
> +                       dprintk(1, "invalid dmabuf length for plane %d\n",
>                                 plane);
>                         ret = -EINVAL;
>                         goto err;
> @@ -1405,7 +1404,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>                         continue;
>                 }
>
> -               dprintk(1, "qbuf: buffer for plane %d changed\n", plane);
> +               dprintk(1, "buffer for plane %d changed\n", plane);
>
>                 if (!reacquired) {
>                         reacquired = true;
> @@ -1420,7 +1419,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>                 mem_priv = call_memop(vb, attach_dmabuf, q->alloc_ctx[plane],
>                         dbuf, planes[plane].length, write);
>                 if (IS_ERR(mem_priv)) {
> -                       dprintk(1, "qbuf: failed to attach dmabuf\n");
> +                       dprintk(1, "failed to attach dmabuf\n");
>                         fail_memop(vb, attach_dmabuf);
>                         ret = PTR_ERR(mem_priv);
>                         dma_buf_put(dbuf);
> @@ -1438,7 +1437,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>         for (plane = 0; plane < vb->num_planes; ++plane) {
>                 ret = call_memop(vb, map_dmabuf, vb->planes[plane].mem_priv);
>                 if (ret) {
> -                       dprintk(1, "qbuf: failed to map dmabuf for plane %d\n",
> +                       dprintk(1, "failed to map dmabuf for plane %d\n",
>                                 plane);
>                         fail_memop(vb, map_dmabuf);
>                         goto err;
> @@ -1460,7 +1459,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>                  */
>                 ret = call_vb_qop(vb, buf_init, vb);
>                 if (ret) {
> -                       dprintk(1, "qbuf: buffer initialization failed\n");
> +                       dprintk(1, "buffer initialization failed\n");
>                         fail_vb_qop(vb, buf_init);
>                         goto err;
>                 }
> @@ -1468,7 +1467,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>
>         ret = call_vb_qop(vb, buf_prepare, vb);
>         if (ret) {
> -               dprintk(1, "qbuf: buffer preparation failed\n");
> +               dprintk(1, "buffer preparation failed\n");
>                 fail_vb_qop(vb, buf_prepare);
>                 call_vb_qop(vb, buf_cleanup, vb);
>                 goto err;
> @@ -1508,8 +1507,7 @@ static int __buf_prepare(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>
>         ret = __verify_length(vb, b);
>         if (ret < 0) {
> -               dprintk(1, "%s(): plane parameters verification failed: %d\n",
> -                       __func__, ret);
> +               dprintk(1, "plane parameters verification failed: %d\n", ret);
>                 return ret;
>         }
>
> @@ -1553,7 +1551,7 @@ static int __buf_prepare(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>         }
>
>         if (ret)
> -               dprintk(1, "qbuf: buffer preparation failed: %d\n", ret);
> +               dprintk(1, "buffer preparation failed: %d\n", ret);
>         vb->state = ret ? VB2_BUF_STATE_DEQUEUED : VB2_BUF_STATE_PREPARED;
>
>         return ret;
> @@ -1563,23 +1561,23 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct v4l2_buffer *b,
>                                     const char *opname)
>  {
>         if (b->type != q->type) {
> -               dprintk(1, "%s(): invalid buffer type\n", opname);
> +               dprintk(1, "%s: invalid buffer type\n", opname);
>                 return -EINVAL;
>         }
>
>         if (b->index >= q->num_buffers) {
> -               dprintk(1, "%s(): buffer index out of range\n", opname);
> +               dprintk(1, "%s: buffer index out of range\n", opname);
>                 return -EINVAL;
>         }
>
>         if (q->bufs[b->index] == NULL) {
>                 /* Should never happen */
> -               dprintk(1, "%s(): buffer is NULL\n", opname);
> +               dprintk(1, "%s: buffer is NULL\n", opname);
>                 return -EINVAL;
>         }
>
>         if (b->memory != q->memory) {
> -               dprintk(1, "%s(): invalid memory type\n", opname);
> +               dprintk(1, "%s: invalid memory type\n", opname);
>                 return -EINVAL;
>         }
>
> @@ -1607,7 +1605,7 @@ int vb2_prepare_buf(struct vb2_queue *q, struct v4l2_buffer *b)
>         int ret;
>
>         if (q->fileio) {
> -               dprintk(1, "%s(): file io in progress\n", __func__);
> +               dprintk(1, "file io in progress\n");
>                 return -EBUSY;
>         }
>
> @@ -1617,7 +1615,7 @@ int vb2_prepare_buf(struct vb2_queue *q, struct v4l2_buffer *b)
>
>         vb = q->bufs[b->index];
>         if (vb->state != VB2_BUF_STATE_DEQUEUED) {
> -               dprintk(1, "%s(): invalid buffer state %d\n", __func__,
> +               dprintk(1, "invalid buffer state %d\n",
>                         vb->state);
>                 return -EINVAL;
>         }
> @@ -1627,7 +1625,7 @@ int vb2_prepare_buf(struct vb2_queue *q, struct v4l2_buffer *b)
>                 /* Fill buffer information for the userspace */
>                 __fill_v4l2_buffer(vb, b);
>
> -               dprintk(1, "%s() of buffer %d succeeded\n", __func__, vb->v4l2_buf.index);
> +               dprintk(1, "prepare of buffer %d succeeded\n", vb->v4l2_buf.index);
>         }
>         return ret;
>  }
> @@ -1664,7 +1662,7 @@ static int vb2_start_streaming(struct vb2_queue *q)
>                 return 0;
>
>         fail_qop(q, start_streaming);
> -       dprintk(1, "qbuf: driver refused to start streaming\n");
> +       dprintk(1, "driver refused to start streaming\n");
>         if (WARN_ON(atomic_read(&q->owned_by_drv_count))) {
>                 unsigned i;
>
> @@ -1702,11 +1700,10 @@ static int vb2_internal_qbuf(struct vb2_queue *q, struct v4l2_buffer *b)
>         case VB2_BUF_STATE_PREPARED:
>                 break;
>         case VB2_BUF_STATE_PREPARING:
> -               dprintk(1, "qbuf: buffer still being prepared\n");
> +               dprintk(1, "buffer still being prepared\n");
>                 return -EINVAL;
>         default:
> -               dprintk(1, "%s(): invalid buffer state %d\n", __func__,
> -                       vb->state);
> +               dprintk(1, "invalid buffer state %d\n", vb->state);
>                 return -EINVAL;
>         }
>
> @@ -1753,7 +1750,7 @@ static int vb2_internal_qbuf(struct vb2_queue *q, struct v4l2_buffer *b)
>                         return ret;
>         }
>
> -       dprintk(1, "%s() of buffer %d succeeded\n", __func__, vb->v4l2_buf.index);
> +       dprintk(1, "qbuf of buffer %d succeeded\n", vb->v4l2_buf.index);
>         return 0;
>  }
>
> @@ -1777,7 +1774,7 @@ static int vb2_internal_qbuf(struct vb2_queue *q, struct v4l2_buffer *b)
>  int vb2_qbuf(struct vb2_queue *q, struct v4l2_buffer *b)
>  {
>         if (q->fileio) {
> -               dprintk(1, "%s(): file io in progress\n", __func__);
> +               dprintk(1, "file io in progress\n");
>                 return -EBUSY;
>         }
>
> @@ -1938,7 +1935,7 @@ static int vb2_internal_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool n
>         int ret;
>
>         if (b->type != q->type) {
> -               dprintk(1, "dqbuf: invalid buffer type\n");
> +               dprintk(1, "invalid buffer type\n");
>                 return -EINVAL;
>         }
>         ret = __vb2_get_done_vb(q, &vb, b, nonblocking);
> @@ -1947,13 +1944,13 @@ static int vb2_internal_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool n
>
>         switch (vb->state) {
>         case VB2_BUF_STATE_DONE:
> -               dprintk(3, "dqbuf: Returning done buffer\n");
> +               dprintk(3, "Returning done buffer\n");
>                 break;
>         case VB2_BUF_STATE_ERROR:
> -               dprintk(3, "dqbuf: Returning done buffer with errors\n");
> +               dprintk(3, "Returning done buffer with errors\n");
>                 break;
>         default:
> -               dprintk(1, "dqbuf: Invalid buffer state\n");
> +               dprintk(1, "Invalid buffer state\n");
>                 return -EINVAL;
>         }
>
> @@ -1997,7 +1994,7 @@ static int vb2_internal_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool n
>  int vb2_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool nonblocking)
>  {
>         if (q->fileio) {
> -               dprintk(1, "dqbuf: file io in progress\n");
> +               dprintk(1, "file io in progress\n");
>                 return -EBUSY;
>         }
>         return vb2_internal_dqbuf(q, b, nonblocking);
> @@ -2069,26 +2066,26 @@ static int vb2_internal_streamon(struct vb2_queue *q, enum v4l2_buf_type type)
>         int ret;
>
>         if (type != q->type) {
> -               dprintk(1, "streamon: invalid stream type\n");
> +               dprintk(1, "invalid stream type\n");
>                 return -EINVAL;
>         }
>
>         if (q->streaming) {
> -               dprintk(3, "streamon successful: already streaming\n");
> +               dprintk(3, "already streaming\n");
>                 return 0;
>         }
>
>         if (!q->num_buffers) {
> -               dprintk(1, "streamon: no buffers have been allocated\n");
> +               dprintk(1, "no buffers have been allocated\n");
>                 return -EINVAL;
>         }
>
>         if (!q->num_buffers) {
> -               dprintk(1, "streamon: no buffers have been allocated\n");
> +               dprintk(1, "no buffers have been allocated\n");
>                 return -EINVAL;
>         }
>         if (q->num_buffers < q->min_buffers_needed) {
> -               dprintk(1, "streamon: need at least %u allocated buffers\n",
> +               dprintk(1, "need at least %u allocated buffers\n",
>                                 q->min_buffers_needed);
>                 return -EINVAL;
>         }
> @@ -2107,7 +2104,7 @@ static int vb2_internal_streamon(struct vb2_queue *q, enum v4l2_buf_type type)
>
>         q->streaming = 1;
>
> -       dprintk(3, "Streamon successful\n");
> +       dprintk(3, "successful\n");
>         return 0;
>  }
>
> @@ -2127,7 +2124,7 @@ static int vb2_internal_streamon(struct vb2_queue *q, enum v4l2_buf_type type)
>  int vb2_streamon(struct vb2_queue *q, enum v4l2_buf_type type)
>  {
>         if (q->fileio) {
> -               dprintk(1, "streamon: file io in progress\n");
> +               dprintk(1, "file io in progress\n");
>                 return -EBUSY;
>         }
>         return vb2_internal_streamon(q, type);
> @@ -2137,7 +2134,7 @@ EXPORT_SYMBOL_GPL(vb2_streamon);
>  static int vb2_internal_streamoff(struct vb2_queue *q, enum v4l2_buf_type type)
>  {
>         if (type != q->type) {
> -               dprintk(1, "streamoff: invalid stream type\n");
> +               dprintk(1, "invalid stream type\n");
>                 return -EINVAL;
>         }
>
> @@ -2152,7 +2149,7 @@ static int vb2_internal_streamoff(struct vb2_queue *q, enum v4l2_buf_type type)
>          */
>         __vb2_queue_cancel(q);
>
> -       dprintk(3, "Streamoff successful\n");
> +       dprintk(3, "successful\n");
>         return 0;
>  }
>
> @@ -2174,7 +2171,7 @@ static int vb2_internal_streamoff(struct vb2_queue *q, enum v4l2_buf_type type)
>  int vb2_streamoff(struct vb2_queue *q, enum v4l2_buf_type type)
>  {
>         if (q->fileio) {
> -               dprintk(1, "streamoff: file io in progress\n");
> +               dprintk(1, "file io in progress\n");
>                 return -EBUSY;
>         }
>         return vb2_internal_streamoff(q, type);
> @@ -2242,7 +2239,7 @@ int vb2_expbuf(struct vb2_queue *q, struct v4l2_exportbuffer *eb)
>         }
>
>         if (eb->type != q->type) {
> -               dprintk(1, "qbuf: invalid buffer type\n");
> +               dprintk(1, "invalid buffer type\n");
>                 return -EINVAL;
>         }
>
> @@ -2756,7 +2753,7 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
>         struct vb2_fileio_buf *buf;
>         int ret, index;
>
> -       dprintk(3, "file io: mode %s, offset %ld, count %zd, %sblocking\n",
> +       dprintk(3, "mode %s, offset %ld, count %zd, %sblocking\n",
>                 read ? "read" : "write", (long)*ppos, count,
>                 nonblock ? "non" : "");
>
> @@ -2768,7 +2765,7 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
>          */
>         if (!q->fileio) {
>                 ret = __vb2_init_fileio(q, read);
> -               dprintk(3, "file io: vb2_init_fileio result: %d\n", ret);
> +               dprintk(3, "vb2_init_fileio result: %d\n", ret);
>                 if (ret)
>                         return ret;
>         }
> @@ -2786,7 +2783,7 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
>                 fileio->b.type = q->type;
>                 fileio->b.memory = q->memory;
>                 ret = vb2_internal_dqbuf(q, &fileio->b, nonblock);
> -               dprintk(5, "file io: vb2_dqbuf result: %d\n", ret);
> +               dprintk(5, "vb2_dqbuf result: %d\n", ret);
>                 if (ret)
>                         return ret;
>                 fileio->dq_count += 1;
> @@ -2816,14 +2813,14 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
>         /*
>          * Transfer data to userspace.
>          */
> -       dprintk(3, "file io: copying %zd bytes - buffer %d, offset %u\n",
> +       dprintk(3, "copying %zd bytes - buffer %d, offset %u\n",
>                 count, index, buf->pos);
>         if (read)
>                 ret = copy_to_user(data, buf->vaddr + buf->pos, count);
>         else
>                 ret = copy_from_user(buf->vaddr + buf->pos, data, count);
>         if (ret) {
> -               dprintk(3, "file io: error copying data\n");
> +               dprintk(3, "error copying data\n");
>                 return -EFAULT;
>         }
>
> @@ -2843,7 +2840,7 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
>                  */
>                 if (read && (fileio->flags & VB2_FILEIO_READ_ONCE) &&
>                     fileio->dq_count == 1) {
> -                       dprintk(3, "file io: read limit reached\n");
> +                       dprintk(3, "read limit reached\n");
>                         return __vb2_cleanup_fileio(q);
>                 }
>
> @@ -2856,7 +2853,7 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
>                 fileio->b.index = index;
>                 fileio->b.bytesused = buf->pos;
>                 ret = vb2_internal_qbuf(q, &fileio->b);
> -               dprintk(5, "file io: vb2_dbuf result: %d\n", ret);
> +               dprintk(5, "vb2_dbuf result: %d\n", ret);
>                 if (ret)
>                         return ret;
>
> --
> 1.9.1
>



-- 
Best regards,
Pawel Osciak

Re: [REVIEWv2 PATCH 04/13] vb2: use correct prefix

[Pawel Osciak]

On Mon, Apr 7, 2014 at 10:11 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> Many dprintk's in vb2 use a hardcoded prefix with the function name. In
> many cases that is now outdated. To keep things consistent the dprintk
> macro has been changed to print the function name in addition to the "vb2:"
> prefix. Superfluous prefixes elsewhere in the code have been removed.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Acked-by: Pawel Osciak <pawel@osciak.com>

> ---
>  drivers/media/v4l2-core/videobuf2-core.c | 133 +++++++++++++++----------------
>  1 file changed, 65 insertions(+), 68 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
> index b2582cb..1421075 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -27,10 +27,10 @@
>  static int debug;
>  module_param(debug, int, 0644);
>
> -#define dprintk(level, fmt, arg...)                                    \
> -       do {                                                            \
> -               if (debug >= level)                                     \
> -                       printk(KERN_DEBUG "vb2: " fmt, ## arg);         \
> +#define dprintk(level, fmt, arg...)                                          \
> +       do {                                                                  \
> +               if (debug >= level)                                           \
> +                       pr_debug("vb2: %s: " fmt, __func__, ## arg); \
>         } while (0)
>
>  #ifdef CONFIG_VIDEO_ADV_DEBUG
> @@ -371,7 +371,7 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned int buffers)
>                 if (q->bufs[buffer] == NULL)
>                         continue;
>                 if (q->bufs[buffer]->state == VB2_BUF_STATE_PREPARING) {
> -                       dprintk(1, "reqbufs: preparing buffers, cannot free\n");
> +                       dprintk(1, "preparing buffers, cannot free\n");
>                         return -EAGAIN;
>                 }
>         }
> @@ -656,12 +656,12 @@ int vb2_querybuf(struct vb2_queue *q, struct v4l2_buffer *b)
>         int ret;
>
>         if (b->type != q->type) {
> -               dprintk(1, "querybuf: wrong buffer type\n");
> +               dprintk(1, "wrong buffer type\n");
>                 return -EINVAL;
>         }
>
>         if (b->index >= q->num_buffers) {
> -               dprintk(1, "querybuf: buffer index out of range\n");
> +               dprintk(1, "buffer index out of range\n");
>                 return -EINVAL;
>         }
>         vb = q->bufs[b->index];
> @@ -721,12 +721,12 @@ static int __verify_memory_type(struct vb2_queue *q,
>  {
>         if (memory != V4L2_MEMORY_MMAP && memory != V4L2_MEMORY_USERPTR &&
>             memory != V4L2_MEMORY_DMABUF) {
> -               dprintk(1, "reqbufs: unsupported memory type\n");
> +               dprintk(1, "unsupported memory type\n");
>                 return -EINVAL;
>         }
>
>         if (type != q->type) {
> -               dprintk(1, "reqbufs: requested type is incorrect\n");
> +               dprintk(1, "requested type is incorrect\n");
>                 return -EINVAL;
>         }
>
> @@ -735,17 +735,17 @@ static int __verify_memory_type(struct vb2_queue *q,
>          * are available.
>          */
>         if (memory == V4L2_MEMORY_MMAP && __verify_mmap_ops(q)) {
> -               dprintk(1, "reqbufs: MMAP for current setup unsupported\n");
> +               dprintk(1, "MMAP for current setup unsupported\n");
>                 return -EINVAL;
>         }
>
>         if (memory == V4L2_MEMORY_USERPTR && __verify_userptr_ops(q)) {
> -               dprintk(1, "reqbufs: USERPTR for current setup unsupported\n");
> +               dprintk(1, "USERPTR for current setup unsupported\n");
>                 return -EINVAL;
>         }
>
>         if (memory == V4L2_MEMORY_DMABUF && __verify_dmabuf_ops(q)) {
> -               dprintk(1, "reqbufs: DMABUF for current setup unsupported\n");
> +               dprintk(1, "DMABUF for current setup unsupported\n");
>                 return -EINVAL;
>         }
>
> @@ -755,7 +755,7 @@ static int __verify_memory_type(struct vb2_queue *q,
>          * do the memory and type validation.
>          */
>         if (q->fileio) {
> -               dprintk(1, "reqbufs: file io in progress\n");
> +               dprintk(1, "file io in progress\n");
>                 return -EBUSY;
>         }
>         return 0;
> @@ -790,7 +790,7 @@ static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
>         int ret;
>
>         if (q->streaming) {
> -               dprintk(1, "reqbufs: streaming active\n");
> +               dprintk(1, "streaming active\n");
>                 return -EBUSY;
>         }
>
> @@ -800,7 +800,7 @@ static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
>                  * are not in use and can be freed.
>                  */
>                 if (q->memory == V4L2_MEMORY_MMAP && __buffers_in_use(q)) {
> -                       dprintk(1, "reqbufs: memory in use, cannot free\n");
> +                       dprintk(1, "memory in use, cannot free\n");
>                         return -EBUSY;
>                 }
>
> @@ -931,8 +931,7 @@ static int __create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create
>         int ret;
>
>         if (q->num_buffers == VIDEO_MAX_FRAME) {
> -               dprintk(1, "%s(): maximum number of buffers already allocated\n",
> -                       __func__);
> +               dprintk(1, "maximum number of buffers already allocated\n");
>                 return -ENOBUFS;
>         }
>
> @@ -1264,12 +1263,12 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>                     && vb->v4l2_planes[plane].length == planes[plane].length)
>                         continue;
>
> -               dprintk(3, "qbuf: userspace address for plane %d changed, "
> +               dprintk(3, "userspace address for plane %d changed, "
>                                 "reacquiring memory\n", plane);
>
>                 /* Check if the provided plane buffer is large enough */
>                 if (planes[plane].length < q->plane_sizes[plane]) {
> -                       dprintk(1, "qbuf: provided buffer size %u is less than "
> +                       dprintk(1, "provided buffer size %u is less than "
>                                                 "setup size %u for plane %d\n",
>                                                 planes[plane].length,
>                                                 q->plane_sizes[plane], plane);
> @@ -1294,7 +1293,7 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>                                       planes[plane].m.userptr,
>                                       planes[plane].length, write);
>                 if (IS_ERR_OR_NULL(mem_priv)) {
> -                       dprintk(1, "qbuf: failed acquiring userspace "
> +                       dprintk(1, "failed acquiring userspace "
>                                                 "memory for plane %d\n", plane);
>                         fail_memop(vb, get_userptr);
>                         ret = mem_priv ? PTR_ERR(mem_priv) : -EINVAL;
> @@ -1318,7 +1317,7 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>                  */
>                 ret = call_vb_qop(vb, buf_init, vb);
>                 if (ret) {
> -                       dprintk(1, "qbuf: buffer initialization failed\n");
> +                       dprintk(1, "buffer initialization failed\n");
>                         fail_vb_qop(vb, buf_init);
>                         goto err;
>                 }
> @@ -1326,7 +1325,7 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>
>         ret = call_vb_qop(vb, buf_prepare, vb);
>         if (ret) {
> -               dprintk(1, "qbuf: buffer preparation failed\n");
> +               dprintk(1, "buffer preparation failed\n");
>                 fail_vb_qop(vb, buf_prepare);
>                 call_vb_qop(vb, buf_cleanup, vb);
>                 goto err;
> @@ -1381,7 +1380,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>                 struct dma_buf *dbuf = dma_buf_get(planes[plane].m.fd);
>
>                 if (IS_ERR_OR_NULL(dbuf)) {
> -                       dprintk(1, "qbuf: invalid dmabuf fd for plane %d\n",
> +                       dprintk(1, "invalid dmabuf fd for plane %d\n",
>                                 plane);
>                         ret = -EINVAL;
>                         goto err;
> @@ -1392,7 +1391,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>                         planes[plane].length = dbuf->size;
>
>                 if (planes[plane].length < q->plane_sizes[plane]) {
> -                       dprintk(1, "qbuf: invalid dmabuf length for plane %d\n",
> +                       dprintk(1, "invalid dmabuf length for plane %d\n",
>                                 plane);
>                         ret = -EINVAL;
>                         goto err;
> @@ -1405,7 +1404,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>                         continue;
>                 }
>
> -               dprintk(1, "qbuf: buffer for plane %d changed\n", plane);
> +               dprintk(1, "buffer for plane %d changed\n", plane);
>
>                 if (!reacquired) {
>                         reacquired = true;
> @@ -1420,7 +1419,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>                 mem_priv = call_memop(vb, attach_dmabuf, q->alloc_ctx[plane],
>                         dbuf, planes[plane].length, write);
>                 if (IS_ERR(mem_priv)) {
> -                       dprintk(1, "qbuf: failed to attach dmabuf\n");
> +                       dprintk(1, "failed to attach dmabuf\n");
>                         fail_memop(vb, attach_dmabuf);
>                         ret = PTR_ERR(mem_priv);
>                         dma_buf_put(dbuf);
> @@ -1438,7 +1437,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>         for (plane = 0; plane < vb->num_planes; ++plane) {
>                 ret = call_memop(vb, map_dmabuf, vb->planes[plane].mem_priv);
>                 if (ret) {
> -                       dprintk(1, "qbuf: failed to map dmabuf for plane %d\n",
> +                       dprintk(1, "failed to map dmabuf for plane %d\n",
>                                 plane);
>                         fail_memop(vb, map_dmabuf);
>                         goto err;
> @@ -1460,7 +1459,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>                  */
>                 ret = call_vb_qop(vb, buf_init, vb);
>                 if (ret) {
> -                       dprintk(1, "qbuf: buffer initialization failed\n");
> +                       dprintk(1, "buffer initialization failed\n");
>                         fail_vb_qop(vb, buf_init);
>                         goto err;
>                 }
> @@ -1468,7 +1467,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>
>         ret = call_vb_qop(vb, buf_prepare, vb);
>         if (ret) {
> -               dprintk(1, "qbuf: buffer preparation failed\n");
> +               dprintk(1, "buffer preparation failed\n");
>                 fail_vb_qop(vb, buf_prepare);
>                 call_vb_qop(vb, buf_cleanup, vb);
>                 goto err;
> @@ -1508,8 +1507,7 @@ static int __buf_prepare(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>
>         ret = __verify_length(vb, b);
>         if (ret < 0) {
> -               dprintk(1, "%s(): plane parameters verification failed: %d\n",
> -                       __func__, ret);
> +               dprintk(1, "plane parameters verification failed: %d\n", ret);
>                 return ret;
>         }
>
> @@ -1553,7 +1551,7 @@ static int __buf_prepare(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>         }
>
>         if (ret)
> -               dprintk(1, "qbuf: buffer preparation failed: %d\n", ret);
> +               dprintk(1, "buffer preparation failed: %d\n", ret);
>         vb->state = ret ? VB2_BUF_STATE_DEQUEUED : VB2_BUF_STATE_PREPARED;
>
>         return ret;
> @@ -1563,23 +1561,23 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct v4l2_buffer *b,
>                                     const char *opname)
>  {
>         if (b->type != q->type) {
> -               dprintk(1, "%s(): invalid buffer type\n", opname);
> +               dprintk(1, "%s: invalid buffer type\n", opname);
>                 return -EINVAL;
>         }
>
>         if (b->index >= q->num_buffers) {
> -               dprintk(1, "%s(): buffer index out of range\n", opname);
> +               dprintk(1, "%s: buffer index out of range\n", opname);
>                 return -EINVAL;
>         }
>
>         if (q->bufs[b->index] == NULL) {
>                 /* Should never happen */
> -               dprintk(1, "%s(): buffer is NULL\n", opname);
> +               dprintk(1, "%s: buffer is NULL\n", opname);
>                 return -EINVAL;
>         }
>
>         if (b->memory != q->memory) {
> -               dprintk(1, "%s(): invalid memory type\n", opname);
> +               dprintk(1, "%s: invalid memory type\n", opname);
>                 return -EINVAL;
>         }
>
> @@ -1607,7 +1605,7 @@ int vb2_prepare_buf(struct vb2_queue *q, struct v4l2_buffer *b)
>         int ret;
>
>         if (q->fileio) {
> -               dprintk(1, "%s(): file io in progress\n", __func__);
> +               dprintk(1, "file io in progress\n");
>                 return -EBUSY;
>         }
>
> @@ -1617,7 +1615,7 @@ int vb2_prepare_buf(struct vb2_queue *q, struct v4l2_buffer *b)
>
>         vb = q->bufs[b->index];
>         if (vb->state != VB2_BUF_STATE_DEQUEUED) {
> -               dprintk(1, "%s(): invalid buffer state %d\n", __func__,
> +               dprintk(1, "invalid buffer state %d\n",
>                         vb->state);
>                 return -EINVAL;
>         }
> @@ -1627,7 +1625,7 @@ int vb2_prepare_buf(struct vb2_queue *q, struct v4l2_buffer *b)
>                 /* Fill buffer information for the userspace */
>                 __fill_v4l2_buffer(vb, b);
>
> -               dprintk(1, "%s() of buffer %d succeeded\n", __func__, vb->v4l2_buf.index);
> +               dprintk(1, "prepare of buffer %d succeeded\n", vb->v4l2_buf.index);
>         }
>         return ret;
>  }
> @@ -1664,7 +1662,7 @@ static int vb2_start_streaming(struct vb2_queue *q)
>                 return 0;
>
>         fail_qop(q, start_streaming);
> -       dprintk(1, "qbuf: driver refused to start streaming\n");
> +       dprintk(1, "driver refused to start streaming\n");
>         if (WARN_ON(atomic_read(&q->owned_by_drv_count))) {
>                 unsigned i;
>
> @@ -1702,11 +1700,10 @@ static int vb2_internal_qbuf(struct vb2_queue *q, struct v4l2_buffer *b)
>         case VB2_BUF_STATE_PREPARED:
>                 break;
>         case VB2_BUF_STATE_PREPARING:
> -               dprintk(1, "qbuf: buffer still being prepared\n");
> +               dprintk(1, "buffer still being prepared\n");
>                 return -EINVAL;
>         default:
> -               dprintk(1, "%s(): invalid buffer state %d\n", __func__,
> -                       vb->state);
> +               dprintk(1, "invalid buffer state %d\n", vb->state);
>                 return -EINVAL;
>         }
>
> @@ -1753,7 +1750,7 @@ static int vb2_internal_qbuf(struct vb2_queue *q, struct v4l2_buffer *b)
>                         return ret;
>         }
>
> -       dprintk(1, "%s() of buffer %d succeeded\n", __func__, vb->v4l2_buf.index);
> +       dprintk(1, "qbuf of buffer %d succeeded\n", vb->v4l2_buf.index);
>         return 0;
>  }
>
> @@ -1777,7 +1774,7 @@ static int vb2_internal_qbuf(struct vb2_queue *q, struct v4l2_buffer *b)
>  int vb2_qbuf(struct vb2_queue *q, struct v4l2_buffer *b)
>  {
>         if (q->fileio) {
> -               dprintk(1, "%s(): file io in progress\n", __func__);
> +               dprintk(1, "file io in progress\n");
>                 return -EBUSY;
>         }
>
> @@ -1938,7 +1935,7 @@ static int vb2_internal_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool n
>         int ret;
>
>         if (b->type != q->type) {
> -               dprintk(1, "dqbuf: invalid buffer type\n");
> +               dprintk(1, "invalid buffer type\n");
>                 return -EINVAL;
>         }
>         ret = __vb2_get_done_vb(q, &vb, b, nonblocking);
> @@ -1947,13 +1944,13 @@ static int vb2_internal_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool n
>
>         switch (vb->state) {
>         case VB2_BUF_STATE_DONE:
> -               dprintk(3, "dqbuf: Returning done buffer\n");
> +               dprintk(3, "Returning done buffer\n");
>                 break;
>         case VB2_BUF_STATE_ERROR:
> -               dprintk(3, "dqbuf: Returning done buffer with errors\n");
> +               dprintk(3, "Returning done buffer with errors\n");
>                 break;
>         default:
> -               dprintk(1, "dqbuf: Invalid buffer state\n");
> +               dprintk(1, "Invalid buffer state\n");
>                 return -EINVAL;
>         }
>
> @@ -1997,7 +1994,7 @@ static int vb2_internal_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool n
>  int vb2_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool nonblocking)
>  {
>         if (q->fileio) {
> -               dprintk(1, "dqbuf: file io in progress\n");
> +               dprintk(1, "file io in progress\n");
>                 return -EBUSY;
>         }
>         return vb2_internal_dqbuf(q, b, nonblocking);
> @@ -2069,26 +2066,26 @@ static int vb2_internal_streamon(struct vb2_queue *q, enum v4l2_buf_type type)
>         int ret;
>
>         if (type != q->type) {
> -               dprintk(1, "streamon: invalid stream type\n");
> +               dprintk(1, "invalid stream type\n");
>                 return -EINVAL;
>         }
>
>         if (q->streaming) {
> -               dprintk(3, "streamon successful: already streaming\n");
> +               dprintk(3, "already streaming\n");
>                 return 0;
>         }
>
>         if (!q->num_buffers) {
> -               dprintk(1, "streamon: no buffers have been allocated\n");
> +               dprintk(1, "no buffers have been allocated\n");
>                 return -EINVAL;
>         }
>
>         if (!q->num_buffers) {
> -               dprintk(1, "streamon: no buffers have been allocated\n");
> +               dprintk(1, "no buffers have been allocated\n");
>                 return -EINVAL;
>         }
>         if (q->num_buffers < q->min_buffers_needed) {
> -               dprintk(1, "streamon: need at least %u allocated buffers\n",
> +               dprintk(1, "need at least %u allocated buffers\n",
>                                 q->min_buffers_needed);
>                 return -EINVAL;
>         }
> @@ -2107,7 +2104,7 @@ static int vb2_internal_streamon(struct vb2_queue *q, enum v4l2_buf_type type)
>
>         q->streaming = 1;
>
> -       dprintk(3, "Streamon successful\n");
> +       dprintk(3, "successful\n");
>         return 0;
>  }
>
> @@ -2127,7 +2124,7 @@ static int vb2_internal_streamon(struct vb2_queue *q, enum v4l2_buf_type type)
>  int vb2_streamon(struct vb2_queue *q, enum v4l2_buf_type type)
>  {
>         if (q->fileio) {
> -               dprintk(1, "streamon: file io in progress\n");
> +               dprintk(1, "file io in progress\n");
>                 return -EBUSY;
>         }
>         return vb2_internal_streamon(q, type);
> @@ -2137,7 +2134,7 @@ EXPORT_SYMBOL_GPL(vb2_streamon);
>  static int vb2_internal_streamoff(struct vb2_queue *q, enum v4l2_buf_type type)
>  {
>         if (type != q->type) {
> -               dprintk(1, "streamoff: invalid stream type\n");
> +               dprintk(1, "invalid stream type\n");
>                 return -EINVAL;
>         }
>
> @@ -2152,7 +2149,7 @@ static int vb2_internal_streamoff(struct vb2_queue *q, enum v4l2_buf_type type)
>          */
>         __vb2_queue_cancel(q);
>
> -       dprintk(3, "Streamoff successful\n");
> +       dprintk(3, "successful\n");
>         return 0;
>  }
>
> @@ -2174,7 +2171,7 @@ static int vb2_internal_streamoff(struct vb2_queue *q, enum v4l2_buf_type type)
>  int vb2_streamoff(struct vb2_queue *q, enum v4l2_buf_type type)
>  {
>         if (q->fileio) {
> -               dprintk(1, "streamoff: file io in progress\n");
> +               dprintk(1, "file io in progress\n");
>                 return -EBUSY;
>         }
>         return vb2_internal_streamoff(q, type);
> @@ -2242,7 +2239,7 @@ int vb2_expbuf(struct vb2_queue *q, struct v4l2_exportbuffer *eb)
>         }
>
>         if (eb->type != q->type) {
> -               dprintk(1, "qbuf: invalid buffer type\n");
> +               dprintk(1, "invalid buffer type\n");
>                 return -EINVAL;
>         }
>
> @@ -2756,7 +2753,7 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
>         struct vb2_fileio_buf *buf;
>         int ret, index;
>
> -       dprintk(3, "file io: mode %s, offset %ld, count %zd, %sblocking\n",
> +       dprintk(3, "mode %s, offset %ld, count %zd, %sblocking\n",
>                 read ? "read" : "write", (long)*ppos, count,
>                 nonblock ? "non" : "");
>
> @@ -2768,7 +2765,7 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
>          */
>         if (!q->fileio) {
>                 ret = __vb2_init_fileio(q, read);
> -               dprintk(3, "file io: vb2_init_fileio result: %d\n", ret);
> +               dprintk(3, "vb2_init_fileio result: %d\n", ret);
>                 if (ret)
>                         return ret;
>         }
> @@ -2786,7 +2783,7 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
>                 fileio->b.type = q->type;
>                 fileio->b.memory = q->memory;
>                 ret = vb2_internal_dqbuf(q, &fileio->b, nonblock);
> -               dprintk(5, "file io: vb2_dqbuf result: %d\n", ret);
> +               dprintk(5, "vb2_dqbuf result: %d\n", ret);
>                 if (ret)
>                         return ret;
>                 fileio->dq_count += 1;
> @@ -2816,14 +2813,14 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
>         /*
>          * Transfer data to userspace.
>          */
> -       dprintk(3, "file io: copying %zd bytes - buffer %d, offset %u\n",
> +       dprintk(3, "copying %zd bytes - buffer %d, offset %u\n",
>                 count, index, buf->pos);
>         if (read)
>                 ret = copy_to_user(data, buf->vaddr + buf->pos, count);
>         else
>                 ret = copy_from_user(buf->vaddr + buf->pos, data, count);
>         if (ret) {
> -               dprintk(3, "file io: error copying data\n");
> +               dprintk(3, "error copying data\n");
>                 return -EFAULT;
>         }
>
> @@ -2843,7 +2840,7 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
>                  */
>                 if (read && (fileio->flags & VB2_FILEIO_READ_ONCE) &&
>                     fileio->dq_count == 1) {
> -                       dprintk(3, "file io: read limit reached\n");
> +                       dprintk(3, "read limit reached\n");
>                         return __vb2_cleanup_fileio(q);
>                 }
>
> @@ -2856,7 +2853,7 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
>                 fileio->b.index = index;
>                 fileio->b.bytesused = buf->pos;
>                 ret = vb2_internal_qbuf(q, &fileio->b);
> -               dprintk(5, "file io: vb2_dbuf result: %d\n", ret);
> +               dprintk(5, "vb2_dbuf result: %d\n", ret);
>                 if (ret)
>                         return ret;
>
> --
> 1.9.1
>



-- 
Best regards,
Pawel Osciak

Re: [REVIEWv2 PATCH 06/13] vb2: set timestamp when using write()

[Pawel Osciak]

I see. Ack, but please add a comment about this in the code.

On Mon, Apr 7, 2014 at 10:11 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> When using write() to write data to an output video node the vb2 core
> should set timestamps if V4L2_BUF_FLAG_TIMESTAMP_COPY is set. Nobody
> else is able to provide this information with the write() operation.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Acked-by: Pawel Osciak <pawel@osciak.com>

> ---
>  drivers/media/v4l2-core/videobuf2-core.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
> index 2e448a7..b7de6be 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -22,6 +22,7 @@
>  #include <media/v4l2-dev.h>
>  #include <media/v4l2-fh.h>
>  #include <media/v4l2-event.h>
> +#include <media/v4l2-common.h>
>  #include <media/videobuf2-core.h>
>
>  static int debug;
> @@ -2751,6 +2752,9 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
>  {
>         struct vb2_fileio_data *fileio;
>         struct vb2_fileio_buf *buf;
> +       bool set_timestamp = !read &&
> +               (q->timestamp_flags & V4L2_BUF_FLAG_TIMESTAMP_MASK) ==
> +               V4L2_BUF_FLAG_TIMESTAMP_COPY;

Please add an explicit comment why we are doing this here in the code.

>         int ret, index;
>
>         dprintk(3, "mode %s, offset %ld, count %zd, %sblocking\n",
> @@ -2852,6 +2856,8 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
>                 fileio->b.memory = q->memory;
>                 fileio->b.index = index;
>                 fileio->b.bytesused = buf->pos;
> +               if (set_timestamp)
> +                       v4l2_get_timestamp(&fileio->b.timestamp);
>                 ret = vb2_internal_qbuf(q, &fileio->b);
>                 dprintk(5, "vb2_dbuf result: %d\n", ret);
>                 if (ret)
> --
> 1.9.1
>



-- 
Best regards,
Pawel Osciak

Re: [REVIEWv2 PATCH 07/13] vb2: reject output buffers with V4L2_FIELD_ALTERNATE

[Pawel Osciak]

On Mon, Apr 7, 2014 at 10:11 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> This is not allowed by the spec and does in fact not make any sense.
> Return -EINVAL if this is the case.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Acked-by: Pawel Osciak <pawel@osciak.com>

> ---
>  drivers/media/v4l2-core/videobuf2-core.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
> index b7de6be..c662ad9 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -1511,6 +1511,19 @@ static int __buf_prepare(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>                 dprintk(1, "plane parameters verification failed: %d\n", ret);
>                 return ret;
>         }
> +       if (b->field == V4L2_FIELD_ALTERNATE && V4L2_TYPE_IS_OUTPUT(q->type)) {
> +               /*
> +                * If the format's field is ALTERNATE, then the buffer's field
> +                * should be either TOP or BOTTOM, not ALTERNATE since that
> +                * makes no sense. The driver has to know whether the
> +                * buffer represents a top or a bottom field in order to
> +                * program any DMA correctly. Using ALTERNATE is wrong, since
> +                * that just says that it is either a top or a bottom field,
> +                * but not which of the two it is.
> +                */
> +               dprintk(1, "the field is incorrectly set to ALTERNATE for an output buffer\n");
> +               return -EINVAL;
> +       }
>
>         vb->state = VB2_BUF_STATE_PREPARING;
>         vb->v4l2_buf.timestamp.tv_sec = 0;
> --
> 1.9.1
>



-- 
Best regards,
Pawel Osciak

Re: [REVIEWv2 PATCH 09/13] vb2: add vb2_fileio_is_active and check it more often

[Pawel Osciak]

On Mon, Apr 7, 2014 at 10:11 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> Added a vb2_fileio_is_active inline function that returns true if fileio
> is in progress. Check for this too in mmap() (you don't want apps mmap()ing
> buffers used by fileio) and expbuf() (same reason).
>
> In addition drivers should be able to check for this in queue_setup() to
> return an error if an attempt is made to read() or write() with
> V4L2_FIELD_ALTERNATE being configured. This is illegal (there is no way
> to pass the TOP/BOTTOM information around using file I/O).
>
> However, in order to be able to check for this the init_fileio function
> needs to set q->fileio early on, before the buffers are allocated. So switch
> to using internal functions (__reqbufs, vb2_internal_qbuf and
> vb2_internal_streamon) to skip the fileio check. Well, that's why the internal
> functions were created...
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Acked-by: Pawel Osciak <pawel@osciak.com>

> ---
>  drivers/media/v4l2-core/videobuf2-core.c | 39 ++++++++++++++++++++------------
>  include/media/videobuf2-core.h           | 17 ++++++++++++++
>  2 files changed, 41 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
> index 89147d2..08152dd 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -755,7 +755,7 @@ static int __verify_memory_type(struct vb2_queue *q,
>          * create_bufs is called with count == 0, but count == 0 should still
>          * do the memory and type validation.
>          */
> -       if (q->fileio) {
> +       if (vb2_fileio_is_active(q)) {
>                 dprintk(1, "file io in progress\n");
>                 return -EBUSY;
>         }
> @@ -1617,7 +1617,7 @@ int vb2_prepare_buf(struct vb2_queue *q, struct v4l2_buffer *b)
>         struct vb2_buffer *vb;
>         int ret;
>
> -       if (q->fileio) {
> +       if (vb2_fileio_is_active(q)) {
>                 dprintk(1, "file io in progress\n");
>                 return -EBUSY;
>         }
> @@ -1786,7 +1786,7 @@ static int vb2_internal_qbuf(struct vb2_queue *q, struct v4l2_buffer *b)
>   */
>  int vb2_qbuf(struct vb2_queue *q, struct v4l2_buffer *b)
>  {
> -       if (q->fileio) {
> +       if (vb2_fileio_is_active(q)) {
>                 dprintk(1, "file io in progress\n");
>                 return -EBUSY;
>         }
> @@ -2006,7 +2006,7 @@ static int vb2_internal_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool n
>   */
>  int vb2_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool nonblocking)
>  {
> -       if (q->fileio) {
> +       if (vb2_fileio_is_active(q)) {
>                 dprintk(1, "file io in progress\n");
>                 return -EBUSY;
>         }
> @@ -2136,7 +2136,7 @@ static int vb2_internal_streamon(struct vb2_queue *q, enum v4l2_buf_type type)
>   */
>  int vb2_streamon(struct vb2_queue *q, enum v4l2_buf_type type)
>  {
> -       if (q->fileio) {
> +       if (vb2_fileio_is_active(q)) {
>                 dprintk(1, "file io in progress\n");
>                 return -EBUSY;
>         }
> @@ -2183,7 +2183,7 @@ static int vb2_internal_streamoff(struct vb2_queue *q, enum v4l2_buf_type type)
>   */
>  int vb2_streamoff(struct vb2_queue *q, enum v4l2_buf_type type)
>  {
> -       if (q->fileio) {
> +       if (vb2_fileio_is_active(q)) {
>                 dprintk(1, "file io in progress\n");
>                 return -EBUSY;
>         }
> @@ -2268,6 +2268,11 @@ int vb2_expbuf(struct vb2_queue *q, struct v4l2_exportbuffer *eb)
>                 return -EINVAL;
>         }
>
> +       if (vb2_fileio_is_active(q)) {
> +               dprintk(1, "expbuf: file io in progress\n");
> +               return -EBUSY;
> +       }
> +
>         vb_plane = &vb->planes[eb->plane];
>
>         dbuf = call_memop(vb, get_dmabuf, vb_plane->mem_priv, eb->flags & O_ACCMODE);
> @@ -2344,6 +2349,10 @@ int vb2_mmap(struct vb2_queue *q, struct vm_area_struct *vma)
>                         return -EINVAL;
>                 }
>         }
> +       if (vb2_fileio_is_active(q)) {
> +               dprintk(1, "mmap: file io in progress\n");
> +               return -EBUSY;
> +       }
>
>         /*
>          * Find the plane corresponding to the offset passed by userspace.
> @@ -2455,7 +2464,7 @@ unsigned int vb2_poll(struct vb2_queue *q, struct file *file, poll_table *wait)
>         /*
>          * Start file I/O emulator only if streaming API has not been used yet.
>          */
> -       if (q->num_buffers == 0 && q->fileio == NULL) {
> +       if (q->num_buffers == 0 && !vb2_fileio_is_active(q)) {
>                 if (!V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_READ) &&
>                                 (req_events & (POLLIN | POLLRDNORM))) {
>                         if (__vb2_init_fileio(q, 1))
> @@ -2660,7 +2669,8 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read)
>         fileio->req.count = count;
>         fileio->req.memory = V4L2_MEMORY_MMAP;
>         fileio->req.type = q->type;
> -       ret = vb2_reqbufs(q, &fileio->req);
> +       q->fileio = fileio;
> +       ret = __reqbufs(q, &fileio->req);
>         if (ret)
>                 goto err_kfree;
>
> @@ -2698,7 +2708,7 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read)
>                         b->type = q->type;
>                         b->memory = q->memory;
>                         b->index = i;
> -                       ret = vb2_qbuf(q, b);
> +                       ret = vb2_internal_qbuf(q, b);
>                         if (ret)
>                                 goto err_reqbufs;
>                         fileio->bufs[i].queued = 1;
> @@ -2714,19 +2724,18 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read)
>         /*
>          * Start streaming.
>          */
> -       ret = vb2_streamon(q, q->type);
> +       ret = vb2_internal_streamon(q, q->type);
>         if (ret)
>                 goto err_reqbufs;
>
> -       q->fileio = fileio;
> -
>         return ret;
>
>  err_reqbufs:
>         fileio->req.count = 0;
> -       vb2_reqbufs(q, &fileio->req);
> +       __reqbufs(q, &fileio->req);
>
>  err_kfree:
> +       q->fileio = NULL;
>         kfree(fileio);
>         return ret;
>  }
> @@ -2779,7 +2788,7 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
>         /*
>          * Initialize emulator on first call.
>          */
> -       if (!q->fileio) {
> +       if (!vb2_fileio_is_active(q)) {
>                 ret = __vb2_init_fileio(q, read);
>                 dprintk(3, "vb2_init_fileio result: %d\n", ret);
>                 if (ret)
> @@ -3147,7 +3156,7 @@ unsigned int vb2_fop_poll(struct file *file, poll_table *wait)
>
>         /* Try to be smart: only lock if polling might start fileio,
>            otherwise locking will only introduce unwanted delays. */
> -       if (q->num_buffers == 0 && q->fileio == NULL) {
> +       if (q->num_buffers == 0 && !vb2_fileio_is_active(q)) {
>                 if (!V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_READ) &&
>                                 (req_events & (POLLIN | POLLRDNORM)))
>                         must_lock = true;
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index 3b57851..af34ae0 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -472,6 +472,23 @@ static inline bool vb2_is_streaming(struct vb2_queue *q)
>  }
>
>  /**
> + * vb2_fileio_is_active() - return true if fileio is active.
> + * @q:         videobuf queue
> + *
> + * This returns true if read() or write() is used to stream the data
> + * as opposed to stream I/O. This is almost never an important distinction,
> + * except in rare cases. One such case is that using read() or write() to
> + * stream a format using V4L2_FIELD_ALTERNATE is not allowed since there
> + * is no way you can pass the field information of each buffer to/from
> + * userspace. A driver that supports this field format should check for
> + * this in the queue_setup op and reject it if this function returns true.
> + */
> +static inline bool vb2_fileio_is_active(struct vb2_queue *q)
> +{
> +       return q->fileio;
> +}
> +
> +/**
>   * vb2_is_busy() - return busy status of the queue
>   * @q:         videobuf queue
>   *
> --
> 1.9.1
>



-- 
Best regards,
Pawel Osciak

Re: [REVIEWv2 PATCH 10/13] vb2: set v4l2_buffer.bytesused to 0 for mp buffers

[Pawel Osciak]

On Mon, Apr 7, 2014 at 10:11 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> The bytesused field of struct v4l2_buffer is not used for multiplanar
> formats, so just zero it to prevent it from having some random value.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Acked-by: Pawel Osciak <pawel@osciak.com>

> ---
>  drivers/media/v4l2-core/videobuf2-core.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
> index 08152dd..ef7ef82 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -582,6 +582,7 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b)
>                  * for it. The caller has already verified memory and size.
>                  */
>                 b->length = vb->num_planes;
> +               b->bytesused = 0;
>                 memcpy(b->m.planes, vb->v4l2_planes,
>                         b->length * sizeof(struct v4l2_plane));
>         } else {
> --
> 1.9.1
>



-- 
Best regards,
Pawel Osciak

Re: [REVIEWv2 PATCH 10/13] vb2: set v4l2_buffer.bytesused to 0 for mp buffers

[Pawel Osciak]

Ah, alas, Sakari is right. This should not be needed, since we memcpy
vb->v4l2_buf to this, also overwriting bytesused.

On Thu, Apr 10, 2014 at 10:08 AM, Pawel Osciak <pawel@osciak.com> wrote:
> On Mon, Apr 7, 2014 at 10:11 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> The bytesused field of struct v4l2_buffer is not used for multiplanar
>> formats, so just zero it to prevent it from having some random value.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>
> Acked-by: Pawel Osciak <pawel@osciak.com>
>
>> ---
>>  drivers/media/v4l2-core/videobuf2-core.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
>> index 08152dd..ef7ef82 100644
>> --- a/drivers/media/v4l2-core/videobuf2-core.c
>> +++ b/drivers/media/v4l2-core/videobuf2-core.c
>> @@ -582,6 +582,7 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b)
>>                  * for it. The caller has already verified memory and size.
>>                  */
>>                 b->length = vb->num_planes;
>> +               b->bytesused = 0;
>>                 memcpy(b->m.planes, vb->v4l2_planes,
>>                         b->length * sizeof(struct v4l2_plane));
>>         } else {
>> --
>> 1.9.1
>>
>
>
>
> --
> Best regards,
> Pawel Osciak



-- 
Best regards,
Pawel Osciak

Re: [REVIEWv2 PATCH 12/13] vb2: start messages with a lower-case for consistency.

[Pawel Osciak]

On Mon, Apr 7, 2014 at 10:11 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> The kernel debug messages produced by vb2 started either with a
> lower or an upper case character. Switched all to use lower-case
> which seemed to be what was used in the majority of the messages.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Acked-by: Pawel Osciak <pawel@osciak.com>

> ---
>  drivers/media/v4l2-core/videobuf2-core.c | 58 ++++++++++++++++----------------
>  1 file changed, 29 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
> index d33c69b..e79d70c 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -151,7 +151,7 @@ static void __vb2_buf_mem_free(struct vb2_buffer *vb)
>         for (plane = 0; plane < vb->num_planes; ++plane) {
>                 call_memop(vb, put, vb->planes[plane].mem_priv);
>                 vb->planes[plane].mem_priv = NULL;
> -               dprintk(3, "Freed plane %d of buffer %d\n", plane,
> +               dprintk(3, "freed plane %d of buffer %d\n", plane,
>                         vb->v4l2_buf.index);
>         }
>  }
> @@ -246,7 +246,7 @@ static void __setup_offsets(struct vb2_queue *q, unsigned int n)
>                 for (plane = 0; plane < vb->num_planes; ++plane) {
>                         vb->v4l2_planes[plane].m.mem_offset = off;
>
> -                       dprintk(3, "Buffer %d, plane %d offset 0x%08lx\n",
> +                       dprintk(3, "buffer %d, plane %d offset 0x%08lx\n",
>                                         buffer, plane, off);
>
>                         off += vb->v4l2_planes[plane].length;
> @@ -273,7 +273,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum v4l2_memory memory,
>                 /* Allocate videobuf buffer structures */
>                 vb = kzalloc(q->buf_struct_size, GFP_KERNEL);
>                 if (!vb) {
> -                       dprintk(1, "Memory alloc for buffer struct failed\n");
> +                       dprintk(1, "memory alloc for buffer struct failed\n");
>                         break;
>                 }
>
> @@ -292,7 +292,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum v4l2_memory memory,
>                 if (memory == V4L2_MEMORY_MMAP) {
>                         ret = __vb2_buf_mem_alloc(vb);
>                         if (ret) {
> -                               dprintk(1, "Failed allocating memory for "
> +                               dprintk(1, "failed allocating memory for "
>                                                 "buffer %d\n", buffer);
>                                 kfree(vb);
>                                 break;
> @@ -304,7 +304,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum v4l2_memory memory,
>                          */
>                         ret = call_vb_qop(vb, buf_init, vb);
>                         if (ret) {
> -                               dprintk(1, "Buffer %d %p initialization"
> +                               dprintk(1, "buffer %d %p initialization"
>                                         " failed\n", buffer, vb);
>                                 fail_vb_qop(vb, buf_init);
>                                 __vb2_buf_mem_free(vb);
> @@ -320,7 +320,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum v4l2_memory memory,
>         if (memory == V4L2_MEMORY_MMAP)
>                 __setup_offsets(q, buffer);
>
> -       dprintk(1, "Allocated %d buffers, %d plane(s) each\n",
> +       dprintk(1, "allocated %d buffers, %d plane(s) each\n",
>                         buffer, num_planes);
>
>         return buffer;
> @@ -477,13 +477,13 @@ static int __verify_planes_array(struct vb2_buffer *vb, const struct v4l2_buffer
>
>         /* Is memory for copying plane information present? */
>         if (NULL == b->m.planes) {
> -               dprintk(1, "Multi-planar buffer passed but "
> +               dprintk(1, "multi-planar buffer passed but "
>                            "planes array not provided\n");
>                 return -EINVAL;
>         }
>
>         if (b->length < vb->num_planes || b->length > VIDEO_MAX_PLANES) {
> -               dprintk(1, "Incorrect planes array length, "
> +               dprintk(1, "incorrect planes array length, "
>                            "expected %d, got %d\n", vb->num_planes, b->length);
>                 return -EINVAL;
>         }
> @@ -847,7 +847,7 @@ static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
>         /* Finally, allocate buffers and video memory */
>         allocated_buffers = __vb2_queue_alloc(q, req->memory, num_buffers, num_planes);
>         if (allocated_buffers == 0) {
> -               dprintk(1, "Memory allocation failed\n");
> +               dprintk(1, "memory allocation failed\n");
>                 return -ENOMEM;
>         }
>
> @@ -960,7 +960,7 @@ static int __create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create
>         allocated_buffers = __vb2_queue_alloc(q, create->memory, num_buffers,
>                                 num_planes);
>         if (allocated_buffers == 0) {
> -               dprintk(1, "Memory allocation failed\n");
> +               dprintk(1, "memory allocation failed\n");
>                 return -ENOMEM;
>         }
>
> @@ -1107,7 +1107,7 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
>          */
>         vb->cnt_buf_done++;
>  #endif
> -       dprintk(4, "Done processing on buffer %d, state: %d\n",
> +       dprintk(4, "done processing on buffer %d, state: %d\n",
>                         vb->v4l2_buf.index, state);
>
>         /* sync buffers */
> @@ -1817,7 +1817,7 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, int nonblocking)
>                 int ret;
>
>                 if (!q->streaming) {
> -                       dprintk(1, "Streaming off, will not wait for buffers\n");
> +                       dprintk(1, "streaming off, will not wait for buffers\n");
>                         return -EINVAL;
>                 }
>
> @@ -1829,7 +1829,7 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, int nonblocking)
>                 }
>
>                 if (nonblocking) {
> -                       dprintk(1, "Nonblocking and no buffers to dequeue, "
> +                       dprintk(1, "nonblocking and no buffers to dequeue, "
>                                                                 "will not wait\n");
>                         return -EAGAIN;
>                 }
> @@ -1844,7 +1844,7 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, int nonblocking)
>                 /*
>                  * All locks have been released, it is safe to sleep now.
>                  */
> -               dprintk(3, "Will sleep waiting for buffers\n");
> +               dprintk(3, "will sleep waiting for buffers\n");
>                 ret = wait_event_interruptible(q->done_wq,
>                                 !list_empty(&q->done_list) || !q->streaming);
>
> @@ -1854,7 +1854,7 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, int nonblocking)
>                  */
>                 call_qop(q, wait_finish, q);
>                 if (ret) {
> -                       dprintk(1, "Sleep was interrupted\n");
> +                       dprintk(1, "sleep was interrupted\n");
>                         return ret;
>                 }
>         }
> @@ -1909,7 +1909,7 @@ static int __vb2_get_done_vb(struct vb2_queue *q, struct vb2_buffer **vb,
>  int vb2_wait_for_all_buffers(struct vb2_queue *q)
>  {
>         if (!q->streaming) {
> -               dprintk(1, "Streaming off, will not wait for buffers\n");
> +               dprintk(1, "streaming off, will not wait for buffers\n");
>                 return -EINVAL;
>         }
>
> @@ -1958,13 +1958,13 @@ static int vb2_internal_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool n
>
>         switch (vb->state) {
>         case VB2_BUF_STATE_DONE:
> -               dprintk(3, "Returning done buffer\n");
> +               dprintk(3, "returning done buffer\n");
>                 break;
>         case VB2_BUF_STATE_ERROR:
> -               dprintk(3, "Returning done buffer with errors\n");
> +               dprintk(3, "returning done buffer with errors\n");
>                 break;
>         default:
> -               dprintk(1, "Invalid buffer state\n");
> +               dprintk(1, "invalid buffer state\n");
>                 return -EINVAL;
>         }
>
> @@ -2238,17 +2238,17 @@ int vb2_expbuf(struct vb2_queue *q, struct v4l2_exportbuffer *eb)
>         struct dma_buf *dbuf;
>
>         if (q->memory != V4L2_MEMORY_MMAP) {
> -               dprintk(1, "Queue is not currently set up for mmap\n");
> +               dprintk(1, "queue is not currently set up for mmap\n");
>                 return -EINVAL;
>         }
>
>         if (!q->mem_ops->get_dmabuf) {
> -               dprintk(1, "Queue does not support DMA buffer exporting\n");
> +               dprintk(1, "queue does not support DMA buffer exporting\n");
>                 return -EINVAL;
>         }
>
>         if (eb->flags & ~(O_CLOEXEC | O_ACCMODE)) {
> -               dprintk(1, "Queue does support only O_CLOEXEC and access mode flags\n");
> +               dprintk(1, "queue does support only O_CLOEXEC and access mode flags\n");
>                 return -EINVAL;
>         }
>
> @@ -2278,7 +2278,7 @@ int vb2_expbuf(struct vb2_queue *q, struct v4l2_exportbuffer *eb)
>
>         dbuf = call_memop(vb, get_dmabuf, vb_plane->mem_priv, eb->flags & O_ACCMODE);
>         if (IS_ERR_OR_NULL(dbuf)) {
> -               dprintk(1, "Failed to export buffer %d, plane %d\n",
> +               dprintk(1, "failed to export buffer %d, plane %d\n",
>                         eb->index, eb->plane);
>                 fail_memop(vb, get_dmabuf);
>                 return -EINVAL;
> @@ -2328,7 +2328,7 @@ int vb2_mmap(struct vb2_queue *q, struct vm_area_struct *vma)
>         unsigned long length;
>
>         if (q->memory != V4L2_MEMORY_MMAP) {
> -               dprintk(1, "Queue is not currently set up for mmap\n");
> +               dprintk(1, "queue is not currently set up for mmap\n");
>                 return -EINVAL;
>         }
>
> @@ -2336,17 +2336,17 @@ int vb2_mmap(struct vb2_queue *q, struct vm_area_struct *vma)
>          * Check memory area access mode.
>          */
>         if (!(vma->vm_flags & VM_SHARED)) {
> -               dprintk(1, "Invalid vma flags, VM_SHARED needed\n");
> +               dprintk(1, "invalid vma flags, VM_SHARED needed\n");
>                 return -EINVAL;
>         }
>         if (V4L2_TYPE_IS_OUTPUT(q->type)) {
>                 if (!(vma->vm_flags & VM_WRITE)) {
> -                       dprintk(1, "Invalid vma flags, VM_WRITE needed\n");
> +                       dprintk(1, "invalid vma flags, VM_WRITE needed\n");
>                         return -EINVAL;
>                 }
>         } else {
>                 if (!(vma->vm_flags & VM_READ)) {
> -                       dprintk(1, "Invalid vma flags, VM_READ needed\n");
> +                       dprintk(1, "invalid vma flags, VM_READ needed\n");
>                         return -EINVAL;
>                 }
>         }
> @@ -2382,7 +2382,7 @@ int vb2_mmap(struct vb2_queue *q, struct vm_area_struct *vma)
>                 return ret;
>         }
>
> -       dprintk(3, "Buffer %d, plane %d successfully mapped\n", buffer, plane);
> +       dprintk(3, "buffer %d, plane %d successfully mapped\n", buffer, plane);
>         return 0;
>  }
>  EXPORT_SYMBOL_GPL(vb2_mmap);
> @@ -2400,7 +2400,7 @@ unsigned long vb2_get_unmapped_area(struct vb2_queue *q,
>         int ret;
>
>         if (q->memory != V4L2_MEMORY_MMAP) {
> -               dprintk(1, "Queue is not currently set up for mmap\n");
> +               dprintk(1, "queue is not currently set up for mmap\n");
>                 return -EINVAL;
>         }
>
> --
> 1.9.1
>



-- 
Best regards,
Pawel Osciak

Re: [REVIEWv2 PATCH 02/13] vb2: fix handling of data_offset and v4l2_plane.reserved[]

[Hans Verkuil]

On 04/10/2014 02:46 AM, Pawel Osciak wrote:
> Looks good to me, just a small nit below.
> 
> 
> On Mon, Apr 7, 2014 at 10:11 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> The videobuf2-core did not zero the 'planes' array in __qbuf_userptr()
>> and __qbuf_dmabuf(). That's now memset to 0. Without this the reserved
>> array in struct v4l2_plane would be non-zero, causing v4l2-compliance
>> errors.
>>
>> More serious is the fact that data_offset was not handled correctly:
>>
>> - for capture devices it was never zeroed, which meant that it was
>>   uninitialized. Unless the driver sets it it was a completely random
>>   number. With the memset above this is now fixed.
>>
>> - __qbuf_dmabuf had a completely incorrect length check that included
>>   data_offset.
>>
>> - in __fill_vb2_buffer in the DMABUF case the data_offset field was
>>   unconditionally copied from v4l2_buffer to v4l2_plane when this
>>   should only happen in the output case.
>>
>> - in the single-planar case data_offset was never correctly set to 0.
>>   The single-planar API doesn't support data_offset, so setting it
>>   to 0 is the right thing to do. This too is now solved by the memset.
>>
>> All these issues were found with v4l2-compliance.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Acked-by: Pawel Osciak <pawel@osciak.com>
> 
>> ---
>>  drivers/media/v4l2-core/videobuf2-core.c | 13 ++++---------
>>  1 file changed, 4 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
>> index f9059bb..596998e 100644
>> --- a/drivers/media/v4l2-core/videobuf2-core.c
>> +++ b/drivers/media/v4l2-core/videobuf2-core.c
>> @@ -1169,8 +1169,6 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b
>>                                         b->m.planes[plane].m.fd;
>>                                 v4l2_planes[plane].length =
>>                                         b->m.planes[plane].length;
>> -                               v4l2_planes[plane].data_offset =
>> -                                       b->m.planes[plane].data_offset;
>>                         }
>>                 }
>>         } else {
>> @@ -1180,10 +1178,8 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b
>>                  * In videobuf we use our internal V4l2_planes struct for
>>                  * single-planar buffers as well, for simplicity.
>>                  */
>> -               if (V4L2_TYPE_IS_OUTPUT(b->type)) {
>> +               if (V4L2_TYPE_IS_OUTPUT(b->type))
>>                         v4l2_planes[0].bytesused = b->bytesused;
>> -                       v4l2_planes[0].data_offset = 0;
>> -               }
>>
>>                 if (b->memory == V4L2_MEMORY_USERPTR) {
>>                         v4l2_planes[0].m.userptr = b->m.userptr;
>> @@ -1193,9 +1189,7 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b
>>                 if (b->memory == V4L2_MEMORY_DMABUF) {
>>                         v4l2_planes[0].m.fd = b->m.fd;
>>                         v4l2_planes[0].length = b->length;
>> -                       v4l2_planes[0].data_offset = 0;
>>                 }
>> -
>>         }
>>
>>         /* Zero flags that the vb2 core handles */
>> @@ -1238,6 +1232,7 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>>         int write = !V4L2_TYPE_IS_OUTPUT(q->type);
>>         bool reacquired = vb->planes[0].mem_priv == NULL;
>>
>> +       memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
> 
> memset(planes, 0, sizeof(planes));

Should we really do this? This array is for 8 planes, whereas today we do not
have more than 2 planes worst case. So zeroing all planes for every qbuf seems
excessive to me.

I fact, looking at the code only the actual planes are copied back anyway:

        /*
         * Now that everything is in order, copy relevant information
         * provided by userspace.
         */
        for (plane = 0; plane < vb->num_planes; ++plane)
                vb->v4l2_planes[plane] = planes[plane];

so memsetting more than the actual number of planes is pointless.

Unless I am missing something?

Regards,

	Hans

> 
>>         /* Copy relevant information provided by the userspace */
>>         __fill_vb2_buffer(vb, b, planes);
>>
>> @@ -1357,6 +1352,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>>         int write = !V4L2_TYPE_IS_OUTPUT(q->type);
>>         bool reacquired = vb->planes[0].mem_priv == NULL;
>>
>> +       memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
> 
> memset(planes, 0, sizeof(planes));
> 
>>         /* Copy relevant information provided by the userspace */
>>         __fill_vb2_buffer(vb, b, planes);
>>
>> @@ -1374,8 +1370,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>>                 if (planes[plane].length == 0)
>>                         planes[plane].length = dbuf->size;
>>
>> -               if (planes[plane].length < planes[plane].data_offset +
>> -                   q->plane_sizes[plane]) {
>> +               if (planes[plane].length < q->plane_sizes[plane]) {
>>                         dprintk(1, "qbuf: invalid dmabuf length for plane %d\n",
>>                                 plane);
>>                         ret = -EINVAL;
>> --
>> 1.9.1
>>
> 
> 
> 

Re: [REVIEWv2 PATCH 02/13] vb2: fix handling of data_offset and v4l2_plane.reserved[]

[Tomasz Stanislawski]

On 04/07/2014 03:11 PM, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> The videobuf2-core did not zero the 'planes' array in __qbuf_userptr()
> and __qbuf_dmabuf(). That's now memset to 0. Without this the reserved
> array in struct v4l2_plane would be non-zero, causing v4l2-compliance
> errors.
> 
> More serious is the fact that data_offset was not handled correctly:
> 
> - for capture devices it was never zeroed, which meant that it was
>   uninitialized. Unless the driver sets it it was a completely random
>   number. With the memset above this is now fixed.
> 
> - __qbuf_dmabuf had a completely incorrect length check that included
>   data_offset.

Hi Hans,

I may understand it wrongly but IMO allowing non-zero data offset
simplifies buffer sharing using dmabuf.
I remember a problem that occurred when someone wanted to use
a single dmabuf with multiplanar API.

For example, MFC shares a buffer with DRM. Assume that DRM device
forces the whole image to be located in one dmabuf.

The MFC uses multiplanar API therefore application must use
the same dmabuf to describe luma and chroma planes.

It is intuitive to use the same dmabuf for both planes and
data_offset=0 for luma plane and data_offset = luma_size
for chroma offset.

The check:

> -		if (planes[plane].length < planes[plane].data_offset +
> -		    q->plane_sizes[plane]) {

assured that the logical plane does not overflow the dmabuf.

Am I wrong?

Regards,
Tomasz Stanislawski

> 
> - in __fill_vb2_buffer in the DMABUF case the data_offset field was
>   unconditionally copied from v4l2_buffer to v4l2_plane when this
>   should only happen in the output case.
> 
> - in the single-planar case data_offset was never correctly set to 0.
>   The single-planar API doesn't support data_offset, so setting it
>   to 0 is the right thing to do. This too is now solved by the memset.
> 
> All these issues were found with v4l2-compliance.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Re: [REVIEWv2 PATCH 02/13] vb2: fix handling of data_offset and v4l2_plane.reserved[]

[Hans Verkuil]

On 04/11/2014 02:48 PM, Tomasz Stanislawski wrote:
> On 04/07/2014 03:11 PM, Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> The videobuf2-core did not zero the 'planes' array in __qbuf_userptr()
>> and __qbuf_dmabuf(). That's now memset to 0. Without this the reserved
>> array in struct v4l2_plane would be non-zero, causing v4l2-compliance
>> errors.
>>
>> More serious is the fact that data_offset was not handled correctly:
>>
>> - for capture devices it was never zeroed, which meant that it was
>>   uninitialized. Unless the driver sets it it was a completely random
>>   number. With the memset above this is now fixed.
>>
>> - __qbuf_dmabuf had a completely incorrect length check that included
>>   data_offset.
> 
> Hi Hans,
> 
> I may understand it wrongly but IMO allowing non-zero data offset
> simplifies buffer sharing using dmabuf.
> I remember a problem that occurred when someone wanted to use
> a single dmabuf with multiplanar API.
> 
> For example, MFC shares a buffer with DRM. Assume that DRM device
> forces the whole image to be located in one dmabuf.
> 
> The MFC uses multiplanar API therefore application must use
> the same dmabuf to describe luma and chroma planes.
> 
> It is intuitive to use the same dmabuf for both planes and
> data_offset=0 for luma plane and data_offset = luma_size
> for chroma offset.
> 
> The check:
> 
>> -		if (planes[plane].length < planes[plane].data_offset +
>> -		    q->plane_sizes[plane]) {
> 
> assured that the logical plane does not overflow the dmabuf.
> 
> Am I wrong?

Yes :-)

For video capture the data_offset field is set by the *driver*, not the
application. In practice data_offset is the size of a header that is in
front of the actual image.

You cannot use data_offset for the purpose you describe. To do that a new
offset field would have to be added (user_offset?). I'm not opposed to
that, I think it is a valid use-case for both dmabuf and userptr and
even mmap in combination with CREATE_BUFS.

Regards,

	Hans

Re: [REVIEWv2 PATCH 02/13] vb2: fix handling of data_offset and v4l2_plane.reserved[]

[Tomasz Stanislawski]

On 04/11/2014 03:03 PM, Hans Verkuil wrote:
> On 04/11/2014 02:48 PM, Tomasz Stanislawski wrote:
>> On 04/07/2014 03:11 PM, Hans Verkuil wrote:
>>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>>
>>> The videobuf2-core did not zero the 'planes' array in __qbuf_userptr()
>>> and __qbuf_dmabuf(). That's now memset to 0. Without this the reserved
>>> array in struct v4l2_plane would be non-zero, causing v4l2-compliance
>>> errors.
>>>
>>> More serious is the fact that data_offset was not handled correctly:
>>>
>>> - for capture devices it was never zeroed, which meant that it was
>>>   uninitialized. Unless the driver sets it it was a completely random
>>>   number. With the memset above this is now fixed.
>>>
>>> - __qbuf_dmabuf had a completely incorrect length check that included
>>>   data_offset.
>>
>> Hi Hans,
>>
>> I may understand it wrongly but IMO allowing non-zero data offset
>> simplifies buffer sharing using dmabuf.
>> I remember a problem that occurred when someone wanted to use
>> a single dmabuf with multiplanar API.
>>
>> For example, MFC shares a buffer with DRM. Assume that DRM device
>> forces the whole image to be located in one dmabuf.
>>
>> The MFC uses multiplanar API therefore application must use
>> the same dmabuf to describe luma and chroma planes.
>>
>> It is intuitive to use the same dmabuf for both planes and
>> data_offset=0 for luma plane and data_offset = luma_size
>> for chroma offset.
>>
>> The check:
>>
>>> -		if (planes[plane].length < planes[plane].data_offset +
>>> -		    q->plane_sizes[plane]) {
>>
>> assured that the logical plane does not overflow the dmabuf.
>>
>> Am I wrong?
> 
> Yes :-)
> 
> For video capture the data_offset field is set by the *driver*, not the
> application. In practice data_offset is the size of a header that is in
> front of the actual image.
> 
> You cannot use data_offset for the purpose you describe. To do that a new
> offset field would have to be added (user_offset?). I'm not opposed to
> that, I think it is a valid use-case for both dmabuf and userptr and
> even mmap in combination with CREATE_BUFS.

Ok. What do you think about allowing the user to set this field?
The driver would be allowed to adjust it.
This is a slight change of semantics. As long as application
was forced to treat data_offset as a reserved field (I mean zeroing)
then this change would be backward compatible.

Otherwise, a new field could be introduces as you mentioned.
The name buffer_offset or simply offset might be more
appropriate than user_offset.

I can prepare some RFC about this extension.

Regards,
Tomasz Stanislawski

> 
> Regards,
> 
> 	Hans
> 

Re: [REVIEWv2 PATCH 02/13] vb2: fix handling of data_offset and v4l2_plane.reserved[]

[Hans Verkuil]

On 04/11/2014 03:48 PM, Tomasz Stanislawski wrote:
> On 04/11/2014 03:03 PM, Hans Verkuil wrote:
>> On 04/11/2014 02:48 PM, Tomasz Stanislawski wrote:
>>> On 04/07/2014 03:11 PM, Hans Verkuil wrote:
>>>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>>>
>>>> The videobuf2-core did not zero the 'planes' array in __qbuf_userptr()
>>>> and __qbuf_dmabuf(). That's now memset to 0. Without this the reserved
>>>> array in struct v4l2_plane would be non-zero, causing v4l2-compliance
>>>> errors.
>>>>
>>>> More serious is the fact that data_offset was not handled correctly:
>>>>
>>>> - for capture devices it was never zeroed, which meant that it was
>>>>   uninitialized. Unless the driver sets it it was a completely random
>>>>   number. With the memset above this is now fixed.
>>>>
>>>> - __qbuf_dmabuf had a completely incorrect length check that included
>>>>   data_offset.
>>>
>>> Hi Hans,
>>>
>>> I may understand it wrongly but IMO allowing non-zero data offset
>>> simplifies buffer sharing using dmabuf.
>>> I remember a problem that occurred when someone wanted to use
>>> a single dmabuf with multiplanar API.
>>>
>>> For example, MFC shares a buffer with DRM. Assume that DRM device
>>> forces the whole image to be located in one dmabuf.
>>>
>>> The MFC uses multiplanar API therefore application must use
>>> the same dmabuf to describe luma and chroma planes.
>>>
>>> It is intuitive to use the same dmabuf for both planes and
>>> data_offset=0 for luma plane and data_offset = luma_size
>>> for chroma offset.
>>>
>>> The check:
>>>
>>>> -		if (planes[plane].length < planes[plane].data_offset +
>>>> -		    q->plane_sizes[plane]) {
>>>
>>> assured that the logical plane does not overflow the dmabuf.
>>>
>>> Am I wrong?
>>
>> Yes :-)
>>
>> For video capture the data_offset field is set by the *driver*, not the
>> application. In practice data_offset is the size of a header that is in
>> front of the actual image.
>>
>> You cannot use data_offset for the purpose you describe. To do that a new
>> offset field would have to be added (user_offset?). I'm not opposed to
>> that, I think it is a valid use-case for both dmabuf and userptr and
>> even mmap in combination with CREATE_BUFS.
> 
> Ok. What do you think about allowing the user to set this field?
> The driver would be allowed to adjust it.
> This is a slight change of semantics. As long as application
> was forced to treat data_offset as a reserved field (I mean zeroing)
> then this change would be backward compatible.

It's not quite that easy. Suppose the driver adds a 1024 bytes header
before the image (so data_offset is 1024) and the application sets
data_offset to 512. What does that mean? That the driver starts capturing
at start_of_buffer + 512 and then returns 1024+512 as data_offset? Or that
the driver replaces it by 1024 anyway?

Besides, there has never been a requirement that apps set it to 0 for
video capture. They only have to set it for video output. So this doesn't
work anyway.

> 
> Otherwise, a new field could be introduces as you mentioned.
> The name buffer_offset or simply offset might be more
> appropriate than user_offset.
> 
> I can prepare some RFC about this extension.

I would go with a new field.

Regards,

	Hans