* [REVIEWv3 PATCH 01/13] vb2: stop_streaming should return void
2014-04-11 8:11 [REVIEWv3 PATCH 00/13] vb2: various small fixes/improvements Hans Verkuil
@ 2014-04-11 8:11 ` Hans Verkuil
2014-04-16 21:38 ` Mauro Carvalho Chehab
2014-04-11 8:11 ` [REVIEWv3 PATCH 02/13] vb2: fix handling of data_offset and v4l2_plane.reserved[] Hans Verkuil
` (12 subsequent siblings)
13 siblings, 1 reply; 22+ messages in thread
From: Hans Verkuil @ 2014-04-11 8:11 UTC (permalink / raw)
To: linux-media; +Cc: pawel, sakari.ailus, m.szyprowski, s.nawrocki, 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>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
Documentation/video4linux/v4l2-pci-skeleton.c | 3 +--
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 +-
39 files changed, 61 insertions(+), 118 deletions(-)
diff --git a/Documentation/video4linux/v4l2-pci-skeleton.c b/Documentation/video4linux/v4l2-pci-skeleton.c
index 3a1c0d2..54eca2b 100644
--- a/Documentation/video4linux/v4l2-pci-skeleton.c
+++ b/Documentation/video4linux/v4l2-pci-skeleton.c
@@ -254,7 +254,7 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)
* Stop the DMA engine. Any remaining buffers in the DMA queue are dequeued
* and passed on to the vb2 framework marked as STATE_ERROR.
*/
-static int stop_streaming(struct vb2_queue *vq)
+static void stop_streaming(struct vb2_queue *vq)
{
struct skeleton *skel = vb2_get_drv_priv(vq);
@@ -262,7 +262,6 @@ static int stop_streaming(struct vb2_queue *vq)
/* Release all active buffers */
return_all_buffers(skel, VB2_BUF_STATE_ERROR);
- return 0;
}
/*
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
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [REVIEWv3 PATCH 01/13] vb2: stop_streaming should return void
2014-04-11 8:11 ` [REVIEWv3 PATCH 01/13] vb2: stop_streaming should return void Hans Verkuil
@ 2014-04-16 21:38 ` Mauro Carvalho Chehab
2014-04-17 2:26 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 22+ messages in thread
From: Mauro Carvalho Chehab @ 2014-04-16 21:38 UTC (permalink / raw)
To: Hans Verkuil
Cc: linux-media, pawel, sakari.ailus, m.szyprowski, s.nawrocki,
Hans Verkuil
Em Fri, 11 Apr 2014 10:11:07 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> 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.
This patch doesn't apply fine:
patching file drivers/media/platform/davinci/vpbe_display.c
Hunk #1 merged at 371,380.
Hunk #2 NOT MERGED at 402-406.
patching file drivers/staging/media/davinci_vpfe/vpfe_video.c
Hunk #1 merged at 1252, NOT MERGED at 1258-1262.
patching file drivers/staging/media/msi3101/sdr-msi3101.c
Hunk #1 merged at 1077,1084.
Hunk #2 NOT MERGED at 1109-1113.
Could you please rebase and double check? Please notice that a new driver
was added at gspca. Not sure if this is affected by this changeset.
Also, please see below:
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Acked-by: Pawel Osciak <pawel@osciak.com>
> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
> Documentation/video4linux/v4l2-pci-skeleton.c | 3 +--
> 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 +-
> 39 files changed, 61 insertions(+), 118 deletions(-)
>
> diff --git a/Documentation/video4linux/v4l2-pci-skeleton.c b/Documentation/video4linux/v4l2-pci-skeleton.c
> index 3a1c0d2..54eca2b 100644
> --- a/Documentation/video4linux/v4l2-pci-skeleton.c
> +++ b/Documentation/video4linux/v4l2-pci-skeleton.c
> @@ -254,7 +254,7 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)
> * Stop the DMA engine. Any remaining buffers in the DMA queue are dequeued
> * and passed on to the vb2 framework marked as STATE_ERROR.
> */
> -static int stop_streaming(struct vb2_queue *vq)
> +static void stop_streaming(struct vb2_queue *vq)
> {
> struct skeleton *skel = vb2_get_drv_priv(vq);
>
> @@ -262,7 +262,6 @@ static int stop_streaming(struct vb2_queue *vq)
>
> /* Release all active buffers */
> return_all_buffers(skel, VB2_BUF_STATE_ERROR);
> - return 0;
> }
>
> /*
> 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;
> -
Why are you dropping this? IMHO, you should be doing, instead:
if (!vb2_is_streaming(vq))
return;
Except if you're 100% sure that checking it here can be removed. On
this case, please put this on a separate patch, clearly explaining
why we can safely remove this.
Please notice that on other similar parts of this patch, you didn't remove
the test, just removed the returned parameter.
> 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);
> };
--
Regards,
Mauro
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [REVIEWv3 PATCH 01/13] vb2: stop_streaming should return void
2014-04-16 21:38 ` Mauro Carvalho Chehab
@ 2014-04-17 2:26 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 22+ messages in thread
From: Mauro Carvalho Chehab @ 2014-04-17 2:26 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Hans Verkuil, linux-media, pawel, sakari.ailus, m.szyprowski,
s.nawrocki, Hans Verkuil
Hi Hans,
Em Wed, 16 Apr 2014 18:38:25 -0300
Mauro Carvalho Chehab <m.chehab@samsung.com> escreveu:
> Em Fri, 11 Apr 2014 10:11:07 +0200
> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
>
> > From: Hans Verkuil <hans.verkuil@cisco.com>
> >
> > --- 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;
> > -
>
> Why are you dropping this? IMHO, you should be doing, instead:
> if (!vb2_is_streaming(vq))
> return;
>
> Except if you're 100% sure that checking it here can be removed. On
> this case, please put this on a separate patch, clearly explaining
> why we can safely remove this.
>
> Please notice that on other similar parts of this patch, you didn't remove
> the test, just removed the returned parameter.
As I said before, if you need to remove the vb2_is_streaming(vq) check
above, please do it on a separate patch, properly justifying why you're
doing that.
This hunk is still present on your new pull request.
--
Regards,
Mauro
^ permalink raw reply [flat|nested] 22+ messages in thread
* [REVIEWv3 PATCH 02/13] vb2: fix handling of data_offset and v4l2_plane.reserved[]
2014-04-11 8:11 [REVIEWv3 PATCH 00/13] vb2: various small fixes/improvements Hans Verkuil
2014-04-11 8:11 ` [REVIEWv3 PATCH 01/13] vb2: stop_streaming should return void Hans Verkuil
@ 2014-04-11 8:11 ` Hans Verkuil
2014-04-11 8:11 ` [REVIEWv3 PATCH 03/13] vb2: if bytesused is 0, then fill with output buffer length Hans Verkuil
` (11 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Hans Verkuil @ 2014-04-11 8:11 UTC (permalink / raw)
To: linux-media; +Cc: pawel, sakari.ailus, m.szyprowski, s.nawrocki, 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>
Acked-by: Pawel Osciak <pawel@osciak.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.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
^ permalink raw reply related [flat|nested] 22+ messages in thread* [REVIEWv3 PATCH 03/13] vb2: if bytesused is 0, then fill with output buffer length
2014-04-11 8:11 [REVIEWv3 PATCH 00/13] vb2: various small fixes/improvements Hans Verkuil
2014-04-11 8:11 ` [REVIEWv3 PATCH 01/13] vb2: stop_streaming should return void Hans Verkuil
2014-04-11 8:11 ` [REVIEWv3 PATCH 02/13] vb2: fix handling of data_offset and v4l2_plane.reserved[] Hans Verkuil
@ 2014-04-11 8:11 ` Hans Verkuil
2014-04-11 8:11 ` [REVIEWv3 PATCH 04/13] vb2: use correct prefix Hans Verkuil
` (10 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Hans Verkuil @ 2014-04-11 8:11 UTC (permalink / raw)
To: linux-media; +Cc: pawel, sakari.ailus, m.szyprowski, s.nawrocki, 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>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.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
^ permalink raw reply related [flat|nested] 22+ messages in thread* [REVIEWv3 PATCH 04/13] vb2: use correct prefix
2014-04-11 8:11 [REVIEWv3 PATCH 00/13] vb2: various small fixes/improvements Hans Verkuil
` (2 preceding siblings ...)
2014-04-11 8:11 ` [REVIEWv3 PATCH 03/13] vb2: if bytesused is 0, then fill with output buffer length Hans Verkuil
@ 2014-04-11 8:11 ` Hans Verkuil
2014-04-11 8:11 ` [REVIEWv3 PATCH 05/13] vb2: move __qbuf_mmap before __qbuf_userptr Hans Verkuil
` (9 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Hans Verkuil @ 2014-04-11 8:11 UTC (permalink / raw)
To: linux-media; +Cc: pawel, sakari.ailus, m.szyprowski, s.nawrocki, 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>
Acked-by: Pawel Osciak <pawel@osciak.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.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
^ permalink raw reply related [flat|nested] 22+ messages in thread* [REVIEWv3 PATCH 05/13] vb2: move __qbuf_mmap before __qbuf_userptr
2014-04-11 8:11 [REVIEWv3 PATCH 00/13] vb2: various small fixes/improvements Hans Verkuil
` (3 preceding siblings ...)
2014-04-11 8:11 ` [REVIEWv3 PATCH 04/13] vb2: use correct prefix Hans Verkuil
@ 2014-04-11 8:11 ` Hans Verkuil
2014-04-11 8:11 ` [REVIEWv3 PATCH 06/13] vb2: set timestamp when using write() Hans Verkuil
` (8 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Hans Verkuil @ 2014-04-11 8:11 UTC (permalink / raw)
To: linux-media; +Cc: pawel, sakari.ailus, m.szyprowski, s.nawrocki, 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>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.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
^ permalink raw reply related [flat|nested] 22+ messages in thread* [REVIEWv3 PATCH 06/13] vb2: set timestamp when using write()
2014-04-11 8:11 [REVIEWv3 PATCH 00/13] vb2: various small fixes/improvements Hans Verkuil
` (4 preceding siblings ...)
2014-04-11 8:11 ` [REVIEWv3 PATCH 05/13] vb2: move __qbuf_mmap before __qbuf_userptr Hans Verkuil
@ 2014-04-11 8:11 ` Hans Verkuil
2014-04-11 8:11 ` [REVIEWv3 PATCH 07/13] vb2: reject output buffers with V4L2_FIELD_ALTERNATE Hans Verkuil
` (7 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Hans Verkuil @ 2014-04-11 8:11 UTC (permalink / raw)
To: linux-media; +Cc: pawel, sakari.ailus, m.szyprowski, s.nawrocki, 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>
Acked-by: Pawel Osciak <pawel@osciak.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
drivers/media/v4l2-core/videobuf2-core.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index 2e448a7..6e05495 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,14 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
{
struct vb2_fileio_data *fileio;
struct vb2_fileio_buf *buf;
+ /*
+ * 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.
+ */
+ 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 +2861,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
^ permalink raw reply related [flat|nested] 22+ messages in thread* [REVIEWv3 PATCH 07/13] vb2: reject output buffers with V4L2_FIELD_ALTERNATE
2014-04-11 8:11 [REVIEWv3 PATCH 00/13] vb2: various small fixes/improvements Hans Verkuil
` (5 preceding siblings ...)
2014-04-11 8:11 ` [REVIEWv3 PATCH 06/13] vb2: set timestamp when using write() Hans Verkuil
@ 2014-04-11 8:11 ` Hans Verkuil
2014-04-11 8:42 ` Archit Taneja
2014-04-11 8:11 ` [REVIEWv3 PATCH 08/13] vb2: simplify a confusing condition Hans Verkuil
` (6 subsequent siblings)
13 siblings, 1 reply; 22+ messages in thread
From: Hans Verkuil @ 2014-04-11 8:11 UTC (permalink / raw)
To: linux-media; +Cc: pawel, sakari.ailus, m.szyprowski, s.nawrocki, 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>
Acked-by: Pawel Osciak <pawel@osciak.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.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 6e05495..f8c0247 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
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [REVIEWv3 PATCH 07/13] vb2: reject output buffers with V4L2_FIELD_ALTERNATE
2014-04-11 8:11 ` [REVIEWv3 PATCH 07/13] vb2: reject output buffers with V4L2_FIELD_ALTERNATE Hans Verkuil
@ 2014-04-11 8:42 ` Archit Taneja
2014-04-11 8:58 ` Hans Verkuil
0 siblings, 1 reply; 22+ messages in thread
From: Archit Taneja @ 2014-04-11 8:42 UTC (permalink / raw)
To: Hans Verkuil, linux-media
Cc: pawel, sakari.ailus, m.szyprowski, s.nawrocki, Hans Verkuil
On Friday 11 April 2014 01:41 PM, Hans Verkuil 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>
> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.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 6e05495..f8c0247 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;
> + }
If vb2_queue had a field parameter, which tells the format's field type.
We could have returned an error if the field format was ALTERNATE, and
the buffer field was not TOP or BOTTOM.
I don't know whether having a field parameter in vb2_queue is a good
idea or not.
Thanks,
Archit
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [REVIEWv3 PATCH 07/13] vb2: reject output buffers with V4L2_FIELD_ALTERNATE
2014-04-11 8:42 ` Archit Taneja
@ 2014-04-11 8:58 ` Hans Verkuil
2014-04-11 9:02 ` Archit Taneja
0 siblings, 1 reply; 22+ messages in thread
From: Hans Verkuil @ 2014-04-11 8:58 UTC (permalink / raw)
To: Archit Taneja, linux-media
Cc: pawel, sakari.ailus, m.szyprowski, s.nawrocki, Hans Verkuil
On 04/11/2014 10:42 AM, Archit Taneja wrote:
> On Friday 11 April 2014 01:41 PM, Hans Verkuil 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>
>> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.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 6e05495..f8c0247 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;
>> + }
>
> If vb2_queue had a field parameter, which tells the format's field type.
> We could have returned an error if the field format was ALTERNATE, and
> the buffer field was not TOP or BOTTOM.
>
> I don't know whether having a field parameter in vb2_queue is a good
> idea or not.
The predecessor of vb2, videobuf, had that actually.
I am not sure myself if it is a good idea or not to do the same for vb2.
For now I think we should leave it as is. There are very few drivers that
support FIELD_ALTERNATE although this should become more common for
drivers supporting interlaced HDTV formats. When we see more drivers that
support this, then we can see if it makes sense to move part of the handling
to vb2.
Regards,
Hans
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [REVIEWv3 PATCH 07/13] vb2: reject output buffers with V4L2_FIELD_ALTERNATE
2014-04-11 8:58 ` Hans Verkuil
@ 2014-04-11 9:02 ` Archit Taneja
0 siblings, 0 replies; 22+ messages in thread
From: Archit Taneja @ 2014-04-11 9:02 UTC (permalink / raw)
To: Hans Verkuil, linux-media
Cc: pawel, sakari.ailus, m.szyprowski, s.nawrocki, Hans Verkuil
On Friday 11 April 2014 02:28 PM, Hans Verkuil wrote:
> On 04/11/2014 10:42 AM, Archit Taneja wrote:
>> On Friday 11 April 2014 01:41 PM, Hans Verkuil 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>
>>> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.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 6e05495..f8c0247 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;
>>> + }
>>
>> If vb2_queue had a field parameter, which tells the format's field type.
>> We could have returned an error if the field format was ALTERNATE, and
>> the buffer field was not TOP or BOTTOM.
>>
>> I don't know whether having a field parameter in vb2_queue is a good
>> idea or not.
>
> The predecessor of vb2, videobuf, had that actually.
>
> I am not sure myself if it is a good idea or not to do the same for vb2.
> For now I think we should leave it as is. There are very few drivers that
> support FIELD_ALTERNATE although this should become more common for
> drivers supporting interlaced HDTV formats. When we see more drivers that
> support this, then we can see if it makes sense to move part of the handling
> to vb2.
>
Sure, queue_setup vb op might be a good place to populate it. But as you
said, there aren't many drivers that use FIELD_ALTERNATE, and there
doesn't seem to be any other advantage for having 'field' in vb2_queue.
So, it can be left for later.
Archit
^ permalink raw reply [flat|nested] 22+ messages in thread
* [REVIEWv3 PATCH 08/13] vb2: simplify a confusing condition.
2014-04-11 8:11 [REVIEWv3 PATCH 00/13] vb2: various small fixes/improvements Hans Verkuil
` (6 preceding siblings ...)
2014-04-11 8:11 ` [REVIEWv3 PATCH 07/13] vb2: reject output buffers with V4L2_FIELD_ALTERNATE Hans Verkuil
@ 2014-04-11 8:11 ` Hans Verkuil
2014-04-11 8:11 ` [REVIEWv3 PATCH 09/13] vb2: add vb2_fileio_is_active and check it more often Hans Verkuil
` (5 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Hans Verkuil @ 2014-04-11 8:11 UTC (permalink / raw)
To: linux-media; +Cc: pawel, sakari.ailus, m.szyprowski, s.nawrocki, 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>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.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 f8c0247..3d2a003 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
^ permalink raw reply related [flat|nested] 22+ messages in thread* [REVIEWv3 PATCH 09/13] vb2: add vb2_fileio_is_active and check it more often
2014-04-11 8:11 [REVIEWv3 PATCH 00/13] vb2: various small fixes/improvements Hans Verkuil
` (7 preceding siblings ...)
2014-04-11 8:11 ` [REVIEWv3 PATCH 08/13] vb2: simplify a confusing condition Hans Verkuil
@ 2014-04-11 8:11 ` Hans Verkuil
2014-04-11 13:05 ` Tomasz Stanislawski
2014-04-11 8:11 ` [REVIEWv3 PATCH 10/13] vb2: allow read/write as long as the format is single planar Hans Verkuil
` (4 subsequent siblings)
13 siblings, 1 reply; 22+ messages in thread
From: Hans Verkuil @ 2014-04-11 8:11 UTC (permalink / raw)
To: linux-media; +Cc: pawel, sakari.ailus, m.szyprowski, s.nawrocki, 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>
Acked-by: Pawel Osciak <pawel@osciak.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.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 3d2a003..acabf1f 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;
}
@@ -2784,7 +2793,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)
@@ -3152,7 +3161,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
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [REVIEWv3 PATCH 09/13] vb2: add vb2_fileio_is_active and check it more often
2014-04-11 8:11 ` [REVIEWv3 PATCH 09/13] vb2: add vb2_fileio_is_active and check it more often Hans Verkuil
@ 2014-04-11 13:05 ` Tomasz Stanislawski
2014-04-11 13:22 ` Hans Verkuil
0 siblings, 1 reply; 22+ messages in thread
From: Tomasz Stanislawski @ 2014-04-11 13:05 UTC (permalink / raw)
To: Hans Verkuil, linux-media
Cc: pawel, sakari.ailus, m.szyprowski, s.nawrocki, Hans Verkuil
Hi Hans,
On 04/11/2014 10:11 AM, Hans Verkuil 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).
Why? I expect that there is no sane use case for using
mmap() and expbuf in read/write mode but why forbidding this.
Could you provide a reason?
Regard,
Tomasz Stanislawski
>
> 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>
> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [REVIEWv3 PATCH 09/13] vb2: add vb2_fileio_is_active and check it more often
2014-04-11 13:05 ` Tomasz Stanislawski
@ 2014-04-11 13:22 ` Hans Verkuil
0 siblings, 0 replies; 22+ messages in thread
From: Hans Verkuil @ 2014-04-11 13:22 UTC (permalink / raw)
To: Tomasz Stanislawski, Hans Verkuil, linux-media
Cc: pawel, sakari.ailus, m.szyprowski, s.nawrocki, Hans Verkuil
On 04/11/2014 03:05 PM, Tomasz Stanislawski wrote:
> Hi Hans,
>
> On 04/11/2014 10:11 AM, Hans Verkuil 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).
>
> Why? I expect that there is no sane use case for using
> mmap() and expbuf in read/write mode but why forbidding this.
>
> Could you provide a reason?
The buffer management is completely internal to vb2 for read()/write().
I think that allowing expbuf/mmap is just plain weird. I don't think
it would do any harm other than increasing the memory refcount, but
I very much prefer to block this.
The only ioctl allowed is querybuf, and that primarily for debugging.
Frankly, I wouldn't mind if that is blocked off as well but since it
is guaranteed to have no side-effects and it actually has a use-case
(debugging) I've left that in.
Personally I think the question is not: "why block this?", it is:
"why would you allow it?".
Regards,
Hans
^ permalink raw reply [flat|nested] 22+ messages in thread
* [REVIEWv3 PATCH 10/13] vb2: allow read/write as long as the format is single planar
2014-04-11 8:11 [REVIEWv3 PATCH 00/13] vb2: various small fixes/improvements Hans Verkuil
` (8 preceding siblings ...)
2014-04-11 8:11 ` [REVIEWv3 PATCH 09/13] vb2: add vb2_fileio_is_active and check it more often Hans Verkuil
@ 2014-04-11 8:11 ` Hans Verkuil
2014-04-11 8:11 ` [REVIEWv3 PATCH 11/13] vb2: start messages with a lower-case for consistency Hans Verkuil
` (3 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Hans Verkuil @ 2014-04-11 8:11 UTC (permalink / raw)
To: linux-media; +Cc: pawel, sakari.ailus, m.szyprowski, s.nawrocki, 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 acabf1f..6d28f49 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -2609,6 +2609,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;
@@ -2699,13 +2700,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);
@@ -2773,6 +2782,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);
/*
* 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
@@ -2812,6 +2822,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)
@@ -2882,6 +2897,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
^ permalink raw reply related [flat|nested] 22+ messages in thread* [REVIEWv3 PATCH 11/13] vb2: start messages with a lower-case for consistency.
2014-04-11 8:11 [REVIEWv3 PATCH 00/13] vb2: various small fixes/improvements Hans Verkuil
` (9 preceding siblings ...)
2014-04-11 8:11 ` [REVIEWv3 PATCH 10/13] vb2: allow read/write as long as the format is single planar Hans Verkuil
@ 2014-04-11 8:11 ` Hans Verkuil
2014-04-11 8:11 ` [REVIEWv3 PATCH 12/13] DocBook media: update bytesused field description Hans Verkuil
` (2 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Hans Verkuil @ 2014-04-11 8:11 UTC (permalink / raw)
To: linux-media; +Cc: pawel, sakari.ailus, m.szyprowski, s.nawrocki, 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>
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 6d28f49..aa96997 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;
}
@@ -846,7 +846,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;
}
@@ -959,7 +959,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;
}
@@ -1106,7 +1106,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 */
@@ -1816,7 +1816,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;
}
@@ -1828,7 +1828,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;
}
@@ -1843,7 +1843,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);
@@ -1853,7 +1853,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;
}
}
@@ -1908,7 +1908,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;
}
@@ -1957,13 +1957,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;
}
@@ -2237,17 +2237,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;
}
@@ -2277,7 +2277,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;
@@ -2327,7 +2327,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;
}
@@ -2335,17 +2335,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;
}
}
@@ -2381,7 +2381,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);
@@ -2399,7 +2399,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
^ permalink raw reply related [flat|nested] 22+ messages in thread* [REVIEWv3 PATCH 12/13] DocBook media: update bytesused field description
2014-04-11 8:11 [REVIEWv3 PATCH 00/13] vb2: various small fixes/improvements Hans Verkuil
` (10 preceding siblings ...)
2014-04-11 8:11 ` [REVIEWv3 PATCH 11/13] vb2: start messages with a lower-case for consistency Hans Verkuil
@ 2014-04-11 8:11 ` Hans Verkuil
2014-04-11 8:11 ` [REVIEWv3 PATCH 13/13] v4l2-pci-skeleton.c: fix alternate field handling Hans Verkuil
2014-04-11 9:39 ` [REVIEWv3 PATCH 00/13] vb2: various small fixes/improvements Sakari Ailus
13 siblings, 0 replies; 22+ messages in thread
From: Hans Verkuil @ 2014-04-11 8:11 UTC (permalink / raw)
To: linux-media; +Cc: pawel, sakari.ailus, m.szyprowski, s.nawrocki, 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
^ permalink raw reply related [flat|nested] 22+ messages in thread* [REVIEWv3 PATCH 13/13] v4l2-pci-skeleton.c: fix alternate field handling.
2014-04-11 8:11 [REVIEWv3 PATCH 00/13] vb2: various small fixes/improvements Hans Verkuil
` (11 preceding siblings ...)
2014-04-11 8:11 ` [REVIEWv3 PATCH 12/13] DocBook media: update bytesused field description Hans Verkuil
@ 2014-04-11 8:11 ` Hans Verkuil
2014-04-11 9:39 ` [REVIEWv3 PATCH 00/13] vb2: various small fixes/improvements Sakari Ailus
13 siblings, 0 replies; 22+ messages in thread
From: Hans Verkuil @ 2014-04-11 8:11 UTC (permalink / raw)
To: linux-media; +Cc: pawel, sakari.ailus, m.szyprowski, s.nawrocki, Hans Verkuil
From: Hans Verkuil <hans.verkuil@cisco.com>
For interlaced HDTV timings the correct field setting is FIELD_ALTERNATE,
not INTERLACED. Update this template driver accordingly:
- add check for the invalid combination of read() and FIELD_ALTERNATE.
- in the interrupt handler set v4l2_buffer field to alternating TOP and
BOTTOM.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
Documentation/video4linux/v4l2-pci-skeleton.c | 39 +++++++++++++++++++--------
1 file changed, 28 insertions(+), 11 deletions(-)
diff --git a/Documentation/video4linux/v4l2-pci-skeleton.c b/Documentation/video4linux/v4l2-pci-skeleton.c
index 54eca2b..53dd346 100644
--- a/Documentation/video4linux/v4l2-pci-skeleton.c
+++ b/Documentation/video4linux/v4l2-pci-skeleton.c
@@ -77,7 +77,8 @@ struct skeleton {
spinlock_t qlock;
struct list_head buf_list;
- unsigned int sequence;
+ unsigned field;
+ unsigned sequence;
};
struct skel_buffer {
@@ -124,7 +125,7 @@ static const struct v4l2_dv_timings_cap skel_timings_cap = {
* Interrupt handler: typically interrupts happen after a new frame has been
* captured. It is the job of the handler to remove the new frame from the
* internal list and give it back to the vb2 framework, updating the sequence
- * counter and timestamp at the same time.
+ * counter, field and timestamp at the same time.
*/
static irqreturn_t skeleton_irq(int irq, void *dev_id)
{
@@ -139,8 +140,15 @@ static irqreturn_t skeleton_irq(int irq, void *dev_id)
spin_lock(&skel->qlock);
list_del(&new_buf->list);
spin_unlock(&skel->qlock);
- new_buf->vb.v4l2_buf.sequence = skel->sequence++;
v4l2_get_timestamp(&new_buf->vb.v4l2_buf.timestamp);
+ new_buf->vb.v4l2_buf.sequence = skel->sequence++;
+ new_buf->vb.v4l2_buf.field = skel->field;
+ if (skel->format.field == V4L2_FIELD_ALTERNATE) {
+ if (skel->field == V4L2_FIELD_BOTTOM)
+ skel->field = V4L2_FIELD_TOP;
+ else if (skel->field == V4L2_FIELD_TOP)
+ skel->field = V4L2_FIELD_BOTTOM;
+ }
vb2_buffer_done(&new_buf->vb, VB2_BUF_STATE_DONE);
}
#endif
@@ -160,6 +168,17 @@ static int queue_setup(struct vb2_queue *vq, const struct v4l2_format *fmt,
{
struct skeleton *skel = vb2_get_drv_priv(vq);
+ skel->field = skel->format.field;
+ if (skel->field == V4L2_FIELD_ALTERNATE) {
+ /*
+ * You cannot use read() with FIELD_ALTERNATE since the field
+ * information (TOP/BOTTOM) cannot be passed back to the user.
+ */
+ if (vb2_fileio_is_active(q))
+ return -EINVAL;
+ skel->field = V4L2_FIELD_TOP;
+ }
+
if (vq->num_buffers + *nbuffers < 3)
*nbuffers = 3 - vq->num_buffers;
@@ -173,10 +192,7 @@ static int queue_setup(struct vb2_queue *vq, const struct v4l2_format *fmt,
/*
* Prepare the buffer for queueing to the DMA engine: check and set the
- * payload size and fill in the field. Note: if the format's field is
- * V4L2_FIELD_ALTERNATE, then vb->v4l2_buf.field should be set in the
- * interrupt handler since that's usually where you know if the TOP or
- * BOTTOM field has been captured.
+ * payload size.
*/
static int buffer_prepare(struct vb2_buffer *vb)
{
@@ -190,7 +206,6 @@ static int buffer_prepare(struct vb2_buffer *vb)
}
vb2_set_plane_payload(vb, 0, size);
- vb->v4l2_buf.field = skel->format.field;
return 0;
}
@@ -318,10 +333,12 @@ static void skeleton_fill_pix_format(struct skeleton *skel,
/* HDMI input */
pix->width = skel->timings.bt.width;
pix->height = skel->timings.bt.height;
- if (skel->timings.bt.interlaced)
- pix->field = V4L2_FIELD_INTERLACED;
- else
+ if (skel->timings.bt.interlaced) {
+ pix->field = V4L2_FIELD_ALTERNATE;
+ pix->height /= 2;
+ } else {
pix->field = V4L2_FIELD_NONE;
+ }
pix->colorspace = V4L2_COLORSPACE_REC709;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [REVIEWv3 PATCH 00/13] vb2: various small fixes/improvements
2014-04-11 8:11 [REVIEWv3 PATCH 00/13] vb2: various small fixes/improvements Hans Verkuil
` (12 preceding siblings ...)
2014-04-11 8:11 ` [REVIEWv3 PATCH 13/13] v4l2-pci-skeleton.c: fix alternate field handling Hans Verkuil
@ 2014-04-11 9:39 ` Sakari Ailus
13 siblings, 0 replies; 22+ messages in thread
From: Sakari Ailus @ 2014-04-11 9:39 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, pawel, m.szyprowski, s.nawrocki
On Fri, Apr 11, 2014 at 10:11:06AM +0200, Hans Verkuil wrote:
> This is the third version of this review patch series.
> The previous can be found here:
>
> http://www.spinics.net/lists/linux-media/msg75428.html
>
> Changes since v2:
>
> - Updated v4l2-pci-skeleton.c as well in patch 01/13
> - Dropped patch 10/13 as it is not needed
> - Added comment to patch 06/13 as suggested by Pawel
> - Added patch 13/13: fix HDTV interlaced handling in v4l2-pci-skeleton.c
For patches 10--13,
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
--
Sakari Ailus
e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk
^ permalink raw reply [flat|nested] 22+ messages in thread