All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 00/17] v4l2 core: push ioctl lock down to ioctl handler
@ 2018-06-15 19:07 Ezequiel Garcia
  2018-06-15 19:07 ` [PATCH v4 01/17] v4l2-ioctl.c: use correct vb2_queue lock for m2m devices Ezequiel Garcia
                   ` (17 more replies)
  0 siblings, 18 replies; 28+ messages in thread
From: Ezequiel Garcia @ 2018-06-15 19:07 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Ezequiel Garcia

Fourth spin of the series posted by Hans:

https://www.mail-archive.com/linux-media@vger.kernel.org/msg131363.html

There aren't any changes from v3, aside from rebasing
and re-ordering the patches as requested by Hans.

See v3 cover letter for more details.

Series was tested with tw686x, gspca sonixj and UVC devices.
Build tested with the 0-day kbuild test robot.

Changelog
---------

v4:
Rebased on top of today's next.

v3:
Reduce changes in patches 6 and 7 for omap3isp and omap4iss
drivers, as suggested by Hans.

v2:
Add the required driver modifications, fixing all
drivers so they define a proper vb2_queue lock.

Ezequiel Garcia (13):
  sta2x11: Add video_device and vb2_queue locks
  omap4iss: Add vb2_queue lock
  omap3isp: Add vb2_queue lock
  mtk-mdp: Add locks for capture and output vb2_queues
  s5p-g2d: Implement wait_prepare and wait_finish
  staging: bcm2835-camera: Provide lock for vb2_queue
  venus: Add video_device and vb2_queue locks
  davinci_vpfe: Add video_device and vb2_queue locks
  mx_emmaprp: Implement wait_prepare and wait_finish
  m2m-deinterlace: Implement wait_prepare and wait_finish
  stk1160: Set the vb2_queue lock before calling vb2_queue_init
  dvb-core: Provide lock for vb2_queue
  media: Remove wait_{prepare, finish}

Hans Verkuil (4):
  v4l2-ioctl.c: use correct vb2_queue lock for m2m devices
  videobuf2-core: require q->lock
  videobuf2: assume q->lock is always set
  v4l2-ioctl.c: assume queue->lock is always set

 Documentation/media/kapi/v4l2-dev.rst         |  7 +--
 drivers/input/rmi4/rmi_f54.c                  |  2 -
 drivers/input/touchscreen/atmel_mxt_ts.c      |  2 -
 drivers/input/touchscreen/sur40.c             |  2 -
 .../media/common/videobuf2/videobuf2-core.c   | 22 +++----
 .../media/common/videobuf2/videobuf2-v4l2.c   | 41 +++---------
 drivers/media/dvb-core/dvb_vb2.c              | 20 +-----
 drivers/media/dvb-frontends/rtl2832_sdr.c     |  2 -
 drivers/media/i2c/video-i2c.c                 |  2 -
 drivers/media/pci/cobalt/cobalt-v4l2.c        |  2 -
 drivers/media/pci/cx23885/cx23885-417.c       |  2 -
 drivers/media/pci/cx23885/cx23885-dvb.c       |  2 -
 drivers/media/pci/cx23885/cx23885-vbi.c       |  2 -
 drivers/media/pci/cx23885/cx23885-video.c     |  2 -
 drivers/media/pci/cx25821/cx25821-video.c     |  2 -
 drivers/media/pci/cx88/cx88-blackbird.c       |  2 -
 drivers/media/pci/cx88/cx88-dvb.c             |  2 -
 drivers/media/pci/cx88/cx88-vbi.c             |  2 -
 drivers/media/pci/cx88/cx88-video.c           |  2 -
 drivers/media/pci/dt3155/dt3155.c             |  2 -
 drivers/media/pci/intel/ipu3/ipu3-cio2.c      |  2 -
 drivers/media/pci/saa7134/saa7134-empress.c   |  2 -
 drivers/media/pci/saa7134/saa7134-ts.c        |  2 -
 drivers/media/pci/saa7134/saa7134-vbi.c       |  2 -
 drivers/media/pci/saa7134/saa7134-video.c     |  2 -
 .../media/pci/solo6x10/solo6x10-v4l2-enc.c    |  2 -
 drivers/media/pci/solo6x10/solo6x10-v4l2.c    |  2 -
 drivers/media/pci/sta2x11/sta2x11_vip.c       |  4 ++
 drivers/media/pci/tw5864/tw5864-video.c       |  2 -
 drivers/media/pci/tw68/tw68-video.c           |  2 -
 drivers/media/pci/tw686x/tw686x-video.c       |  2 -
 drivers/media/platform/am437x/am437x-vpfe.c   |  2 -
 drivers/media/platform/atmel/atmel-isc.c      |  2 -
 drivers/media/platform/atmel/atmel-isi.c      |  2 -
 drivers/media/platform/coda/coda-common.c     |  2 -
 drivers/media/platform/davinci/vpbe_display.c |  2 -
 drivers/media/platform/davinci/vpif_capture.c |  2 -
 drivers/media/platform/davinci/vpif_display.c |  2 -
 drivers/media/platform/exynos-gsc/gsc-m2m.c   |  2 -
 .../media/platform/exynos4-is/fimc-capture.c  |  2 -
 .../platform/exynos4-is/fimc-isp-video.c      |  2 -
 drivers/media/platform/exynos4-is/fimc-lite.c |  2 -
 drivers/media/platform/exynos4-is/fimc-m2m.c  |  2 -
 drivers/media/platform/m2m-deinterlace.c      |  2 +
 .../media/platform/marvell-ccic/mcam-core.c   |  4 --
 .../media/platform/mtk-jpeg/mtk_jpeg_core.c   |  2 -
 drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c  | 18 +-----
 .../platform/mtk-vcodec/mtk_vcodec_dec.c      |  2 -
 .../platform/mtk-vcodec/mtk_vcodec_enc.c      |  2 -
 drivers/media/platform/mx2_emmaprp.c          |  2 +
 drivers/media/platform/omap3isp/ispvideo.c    | 63 +++----------------
 drivers/media/platform/omap3isp/ispvideo.h    |  1 -
 drivers/media/platform/pxa_camera.c           |  2 -
 .../platform/qcom/camss-8x16/camss-video.c    |  2 -
 drivers/media/platform/qcom/venus/core.h      |  4 +-
 drivers/media/platform/qcom/venus/helpers.c   | 16 ++---
 drivers/media/platform/qcom/venus/vdec.c      | 23 +++----
 drivers/media/platform/qcom/venus/venc.c      | 17 +++--
 drivers/media/platform/rcar-vin/rcar-dma.c    |  2 -
 drivers/media/platform/rcar_drif.c            |  2 -
 drivers/media/platform/rcar_fdp1.c            |  2 -
 drivers/media/platform/rcar_jpu.c             |  2 -
 drivers/media/platform/renesas-ceu.c          |  2 -
 drivers/media/platform/rockchip/rga/rga-buf.c |  2 -
 .../media/platform/s3c-camif/camif-capture.c  |  2 -
 drivers/media/platform/s5p-jpeg/jpeg-core.c   |  2 -
 drivers/media/platform/s5p-mfc/s5p_mfc_dec.c  |  2 -
 drivers/media/platform/s5p-mfc/s5p_mfc_enc.c  |  2 -
 drivers/media/platform/sh_veu.c               |  2 -
 drivers/media/platform/sh_vou.c               |  2 -
 .../soc_camera/sh_mobile_ceu_camera.c         |  2 -
 drivers/media/platform/sti/bdisp/bdisp-v4l2.c |  2 -
 drivers/media/platform/sti/delta/delta-v4l2.c |  4 --
 drivers/media/platform/sti/hva/hva-v4l2.c     |  2 -
 drivers/media/platform/stm32/stm32-dcmi.c     |  2 -
 drivers/media/platform/ti-vpe/cal.c           |  2 -
 drivers/media/platform/ti-vpe/vpe.c           |  2 -
 drivers/media/platform/vim2m.c                |  2 -
 drivers/media/platform/vimc/vimc-capture.c    |  6 --
 drivers/media/platform/vivid/vivid-sdr-cap.c  |  2 -
 drivers/media/platform/vivid/vivid-vbi-cap.c  |  2 -
 drivers/media/platform/vivid/vivid-vbi-out.c  |  2 -
 drivers/media/platform/vivid/vivid-vid-cap.c  |  2 -
 drivers/media/platform/vivid/vivid-vid-out.c  |  2 -
 drivers/media/platform/vsp1/vsp1_histo.c      |  2 -
 drivers/media/platform/vsp1/vsp1_video.c      |  2 -
 drivers/media/platform/xilinx/xilinx-dma.c    |  2 -
 drivers/media/usb/airspy/airspy.c             |  2 -
 drivers/media/usb/au0828/au0828-vbi.c         |  2 -
 drivers/media/usb/au0828/au0828-video.c       |  2 -
 drivers/media/usb/em28xx/em28xx-vbi.c         |  2 -
 drivers/media/usb/em28xx/em28xx-video.c       |  2 -
 drivers/media/usb/go7007/go7007-v4l2.c        |  2 -
 drivers/media/usb/gspca/gspca.c               |  2 -
 drivers/media/usb/hackrf/hackrf.c             |  2 -
 drivers/media/usb/msi2500/msi2500.c           |  2 -
 drivers/media/usb/pwc/pwc-if.c                |  2 -
 drivers/media/usb/s2255/s2255drv.c            |  2 -
 drivers/media/usb/stk1160/stk1160-v4l.c       |  4 +-
 drivers/media/usb/usbtv/usbtv-video.c         |  2 -
 drivers/media/usb/uvc/uvc_queue.c             |  4 --
 drivers/media/v4l2-core/v4l2-ioctl.c          | 62 ++++++++++++++++--
 .../staging/media/davinci_vpfe/vpfe_video.c   |  4 +-
 .../staging/media/davinci_vpfe/vpfe_video.h   |  2 +-
 drivers/staging/media/imx/imx-media-capture.c |  2 -
 drivers/staging/media/omap4iss/iss_video.c    | 13 +---
 .../bcm2835-camera/bcm2835-camera.c           | 22 +------
 drivers/usb/gadget/function/uvc_queue.c       |  2 -
 include/media/videobuf2-core.h                |  2 -
 include/media/videobuf2-v4l2.h                | 18 ------
 samples/v4l/v4l2-pci-skeleton.c               |  7 ---
 111 files changed, 131 insertions(+), 429 deletions(-)

-- 
2.17.1

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v4 01/17] v4l2-ioctl.c: use correct vb2_queue lock for m2m devices
  2018-06-15 19:07 [PATCH v4 00/17] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
@ 2018-06-15 19:07 ` Ezequiel Garcia
  2018-06-15 19:07 ` [PATCH v4 02/17] sta2x11: Add video_device and vb2_queue locks Ezequiel Garcia
                   ` (16 subsequent siblings)
  17 siblings, 0 replies; 28+ messages in thread
From: Ezequiel Garcia @ 2018-06-15 19:07 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Hans Verkuil

From: Hans Verkuil <hansverk@cisco.com>

For m2m devices the vdev->queue lock was always taken instead of the
lock for the specific capture or output queue. Now that we pushed
the locking down into __video_do_ioctl() we can pick the correct
lock and improve the performance of m2m devices.

Signed-off-by: Hans Verkuil <hansverk@cisco.com>
---
 drivers/media/v4l2-core/v4l2-ioctl.c | 59 ++++++++++++++++++++++++++--
 1 file changed, 55 insertions(+), 4 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index dd210067151f..db835578e21f 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -29,6 +29,7 @@
 #include <media/v4l2-device.h>
 #include <media/videobuf2-v4l2.h>
 #include <media/v4l2-mc.h>
+#include <media/v4l2-mem2mem.h>
 
 #include <trace/events/v4l2.h>
 
@@ -2662,11 +2663,62 @@ static bool v4l2_is_known_ioctl(unsigned int cmd)
 	return v4l2_ioctls[_IOC_NR(cmd)].ioctl == cmd;
 }
 
+#if IS_ENABLED(CONFIG_V4L2_MEM2MEM_DEV)
+static bool v4l2_ioctl_m2m_queue_is_output(unsigned int cmd, void *arg)
+{
+	switch (cmd) {
+	case VIDIOC_CREATE_BUFS: {
+		struct v4l2_create_buffers *cbufs = arg;
+
+		return V4L2_TYPE_IS_OUTPUT(cbufs->format.type);
+	}
+	case VIDIOC_REQBUFS: {
+		struct v4l2_requestbuffers *rbufs = arg;
+
+		return V4L2_TYPE_IS_OUTPUT(rbufs->type);
+	}
+	case VIDIOC_QBUF:
+	case VIDIOC_DQBUF:
+	case VIDIOC_QUERYBUF:
+	case VIDIOC_PREPARE_BUF: {
+		struct v4l2_buffer *buf = arg;
+
+		return V4L2_TYPE_IS_OUTPUT(buf->type);
+	}
+	case VIDIOC_EXPBUF: {
+		struct v4l2_exportbuffer *expbuf = arg;
+
+		return V4L2_TYPE_IS_OUTPUT(expbuf->type);
+	}
+	case VIDIOC_STREAMON:
+	case VIDIOC_STREAMOFF: {
+		int *type = arg;
+
+		return V4L2_TYPE_IS_OUTPUT(*type);
+	}
+	default:
+		return false;
+	}
+}
+#endif
+
 static struct mutex *v4l2_ioctl_get_lock(struct video_device *vdev,
-					 unsigned int cmd)
+					 struct v4l2_fh *vfh, unsigned cmd,
+					 void *arg)
 {
 	if (_IOC_NR(cmd) >= V4L2_IOCTLS)
 		return vdev->lock;
+#if IS_ENABLED(CONFIG_V4L2_MEM2MEM_DEV)
+	if (vfh && vfh->m2m_ctx &&
+	    (v4l2_ioctls[_IOC_NR(cmd)].flags & INFO_FL_QUEUE)) {
+		bool is_output = v4l2_ioctl_m2m_queue_is_output(cmd, arg);
+		struct v4l2_m2m_queue_ctx *ctx = is_output ?
+			&vfh->m2m_ctx->out_q_ctx : &vfh->m2m_ctx->cap_q_ctx;
+
+		if (ctx->q.lock)
+			return ctx->q.lock;
+	}
+#endif
 	if (vdev->queue && vdev->queue->lock &&
 			(v4l2_ioctls[_IOC_NR(cmd)].flags & INFO_FL_QUEUE))
 		return vdev->queue->lock;
@@ -2714,7 +2766,7 @@ static long __video_do_ioctl(struct file *file,
 		unsigned int cmd, void *arg)
 {
 	struct video_device *vfd = video_devdata(file);
-	struct mutex *lock; /* ioctl serialization mutex */
+	struct mutex *lock;
 	const struct v4l2_ioctl_ops *ops = vfd->ioctl_ops;
 	bool write_only = false;
 	struct v4l2_ioctl_info default_info;
@@ -2733,8 +2785,7 @@ static long __video_do_ioctl(struct file *file,
 	if (test_bit(V4L2_FL_USES_V4L2_FH, &vfd->flags))
 		vfh = file->private_data;
 
-	lock = v4l2_ioctl_get_lock(vfd, cmd);
-
+	lock = v4l2_ioctl_get_lock(vfd, vfh, cmd, arg);
 	if (lock && mutex_lock_interruptible(lock))
 		return -ERESTARTSYS;
 
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v4 02/17] sta2x11: Add video_device and vb2_queue locks
  2018-06-15 19:07 [PATCH v4 00/17] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
  2018-06-15 19:07 ` [PATCH v4 01/17] v4l2-ioctl.c: use correct vb2_queue lock for m2m devices Ezequiel Garcia
@ 2018-06-15 19:07 ` Ezequiel Garcia
  2018-06-15 19:07 ` [PATCH v4 03/17] omap4iss: Add vb2_queue lock Ezequiel Garcia
                   ` (15 subsequent siblings)
  17 siblings, 0 replies; 28+ messages in thread
From: Ezequiel Garcia @ 2018-06-15 19:07 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Ezequiel Garcia

Currently, this driver does not serialize its video4linux
ioctls, which is a bug, as race conditions might appear.

In addition, video_device and vb2_queue locks are now both
mandatory. Add them, and implement wait_prepare and
wait_finish.

To stay on the safe side, this commit uses a single mutex
for both locks. Better latency can be obtained by separating
these if needed.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/media/pci/sta2x11/sta2x11_vip.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c b/drivers/media/pci/sta2x11/sta2x11_vip.c
index 069c4a853346..da2b2a2292d0 100644
--- a/drivers/media/pci/sta2x11/sta2x11_vip.c
+++ b/drivers/media/pci/sta2x11/sta2x11_vip.c
@@ -145,6 +145,7 @@ struct sta2x11_vip {
 	unsigned int sequence;
 	struct vip_buffer *active; /* current active buffer */
 	spinlock_t lock; /* Used in videobuf2 callback */
+	struct mutex v4l_lock;
 
 	/* Interrupt counters */
 	int tcount, bcount;
@@ -385,6 +386,8 @@ static const struct vb2_ops vip_video_qops = {
 	.buf_queue		= buffer_queue,
 	.start_streaming	= start_streaming,
 	.stop_streaming		= stop_streaming,
+	.wait_prepare		= vb2_ops_wait_prepare,
+	.wait_finish		= vb2_ops_wait_finish,
 };
 
 
@@ -870,6 +873,7 @@ static int sta2x11_vip_init_buffer(struct sta2x11_vip *vip)
 	vip->vb_vidq.mem_ops = &vb2_dma_contig_memops;
 	vip->vb_vidq.timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
 	vip->vb_vidq.dev = &vip->pdev->dev;
+	vip->vb_vidq.lock = &vip->v4l_lock;
 	err = vb2_queue_init(&vip->vb_vidq);
 	if (err)
 		return err;
@@ -1034,6 +1038,7 @@ static int sta2x11_vip_init_one(struct pci_dev *pdev,
 	vip->std = V4L2_STD_PAL;
 	vip->format = formats_50[0];
 	vip->config = config;
+	mutex_init(&vip->v4l_lock);
 
 	ret = sta2x11_vip_init_controls(vip);
 	if (ret)
@@ -1080,6 +1085,7 @@ static int sta2x11_vip_init_one(struct pci_dev *pdev,
 	vip->video_dev = video_dev_template;
 	vip->video_dev.v4l2_dev = &vip->v4l2_dev;
 	vip->video_dev.queue = &vip->vb_vidq;
+	vip->video_dev.lock = &vip->v4l_lock;
 	video_set_drvdata(&vip->video_dev, vip);
 
 	ret = video_register_device(&vip->video_dev, VFL_TYPE_GRABBER, -1);
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v4 03/17] omap4iss: Add vb2_queue lock
  2018-06-15 19:07 [PATCH v4 00/17] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
  2018-06-15 19:07 ` [PATCH v4 01/17] v4l2-ioctl.c: use correct vb2_queue lock for m2m devices Ezequiel Garcia
  2018-06-15 19:07 ` [PATCH v4 02/17] sta2x11: Add video_device and vb2_queue locks Ezequiel Garcia
@ 2018-06-15 19:07 ` Ezequiel Garcia
  2018-07-02  8:28   ` Hans Verkuil
  2018-06-15 19:07 ` [PATCH v4 04/17] omap3isp: " Ezequiel Garcia
                   ` (14 subsequent siblings)
  17 siblings, 1 reply; 28+ messages in thread
From: Ezequiel Garcia @ 2018-06-15 19:07 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Ezequiel Garcia

vb2_queue lock is now mandatory. Add it, remove driver ad-hoc
locks, and implement wait_{prepare, finish}.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/staging/media/omap4iss/iss_video.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/media/omap4iss/iss_video.c b/drivers/staging/media/omap4iss/iss_video.c
index a3a83424a926..d919bae83828 100644
--- a/drivers/staging/media/omap4iss/iss_video.c
+++ b/drivers/staging/media/omap4iss/iss_video.c
@@ -873,8 +873,6 @@ iss_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
 	if (type != video->type)
 		return -EINVAL;
 
-	mutex_lock(&video->stream_lock);
-
 	/*
 	 * Start streaming on the pipeline. No link touching an entity in the
 	 * pipeline can be activated or deactivated once streaming is started.
@@ -978,8 +976,6 @@ iss_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
 
 	media_graph_walk_cleanup(&graph);
 
-	mutex_unlock(&video->stream_lock);
-
 	return 0;
 
 err_omap4iss_set_stream:
@@ -996,8 +992,6 @@ iss_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
 err_graph_walk_init:
 	media_entity_enum_cleanup(&pipe->ent_enum);
 
-	mutex_unlock(&video->stream_lock);
-
 	return ret;
 }
 
@@ -1013,10 +1007,8 @@ iss_video_streamoff(struct file *file, void *fh, enum v4l2_buf_type type)
 	if (type != video->type)
 		return -EINVAL;
 
-	mutex_lock(&video->stream_lock);
-
 	if (!vb2_is_streaming(&vfh->queue))
-		goto done;
+		return 0;
 
 	/* Update the pipeline state. */
 	if (video->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
@@ -1041,8 +1033,6 @@ iss_video_streamoff(struct file *file, void *fh, enum v4l2_buf_type type)
 		video->iss->pdata->set_constraints(video->iss, false);
 	media_pipeline_stop(&video->video.entity);
 
-done:
-	mutex_unlock(&video->stream_lock);
 	return 0;
 }
 
@@ -1137,6 +1127,7 @@ static int iss_video_open(struct file *file)
 	q->buf_struct_size = sizeof(struct iss_buffer);
 	q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
 	q->dev = video->iss->dev;
+	q->lock = &video->stream_lock;
 
 	ret = vb2_queue_init(q);
 	if (ret) {
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v4 04/17] omap3isp: Add vb2_queue lock
  2018-06-15 19:07 [PATCH v4 00/17] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
                   ` (2 preceding siblings ...)
  2018-06-15 19:07 ` [PATCH v4 03/17] omap4iss: Add vb2_queue lock Ezequiel Garcia
@ 2018-06-15 19:07 ` Ezequiel Garcia
  2018-06-18  7:31   ` Hans Verkuil
  2018-06-15 19:07 ` [PATCH v4 05/17] mtk-mdp: Add locks for capture and output vb2_queues Ezequiel Garcia
                   ` (13 subsequent siblings)
  17 siblings, 1 reply; 28+ messages in thread
From: Ezequiel Garcia @ 2018-06-15 19:07 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Ezequiel Garcia

vb2_queue locks is now mandatory. Add it, remove driver ad-hoc locks,
and implement wait_{prepare, finish}.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/media/platform/omap3isp/ispvideo.c | 65 ++++------------------
 drivers/media/platform/omap3isp/ispvideo.h |  1 -
 2 files changed, 11 insertions(+), 55 deletions(-)

diff --git a/drivers/media/platform/omap3isp/ispvideo.c b/drivers/media/platform/omap3isp/ispvideo.c
index 9d228eac24ea..f835aeb9ddc5 100644
--- a/drivers/media/platform/omap3isp/ispvideo.c
+++ b/drivers/media/platform/omap3isp/ispvideo.c
@@ -496,6 +496,8 @@ static const struct vb2_ops isp_video_queue_ops = {
 	.buf_prepare = isp_video_buffer_prepare,
 	.buf_queue = isp_video_buffer_queue,
 	.start_streaming = isp_video_start_streaming,
+	.wait_prepare = vb2_ops_wait_prepare,
+	.wait_finish = vb2_ops_wait_finish,
 };
 
 /*
@@ -628,11 +630,8 @@ void omap3isp_video_resume(struct isp_video *video, int continuous)
 {
 	struct isp_buffer *buf = NULL;
 
-	if (continuous && video->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
-		mutex_lock(&video->queue_lock);
+	if (continuous && video->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		vb2_discard_done(video->queue);
-		mutex_unlock(&video->queue_lock);
-	}
 
 	if (!list_empty(&video->dmaqueue)) {
 		buf = list_first_entry(&video->dmaqueue,
@@ -909,13 +908,8 @@ isp_video_reqbufs(struct file *file, void *fh, struct v4l2_requestbuffers *rb)
 {
 	struct isp_video_fh *vfh = to_isp_video_fh(fh);
 	struct isp_video *video = video_drvdata(file);
-	int ret;
-
-	mutex_lock(&video->queue_lock);
-	ret = vb2_reqbufs(&vfh->queue, rb);
-	mutex_unlock(&video->queue_lock);
 
-	return ret;
+	return vb2_reqbufs(&vfh->queue, rb);
 }
 
 static int
@@ -923,13 +917,8 @@ isp_video_querybuf(struct file *file, void *fh, struct v4l2_buffer *b)
 {
 	struct isp_video_fh *vfh = to_isp_video_fh(fh);
 	struct isp_video *video = video_drvdata(file);
-	int ret;
 
-	mutex_lock(&video->queue_lock);
-	ret = vb2_querybuf(&vfh->queue, b);
-	mutex_unlock(&video->queue_lock);
-
-	return ret;
+	return vb2_querybuf(&vfh->queue, b);
 }
 
 static int
@@ -937,13 +926,8 @@ isp_video_qbuf(struct file *file, void *fh, struct v4l2_buffer *b)
 {
 	struct isp_video_fh *vfh = to_isp_video_fh(fh);
 	struct isp_video *video = video_drvdata(file);
-	int ret;
 
-	mutex_lock(&video->queue_lock);
-	ret = vb2_qbuf(&vfh->queue, b);
-	mutex_unlock(&video->queue_lock);
-
-	return ret;
+	return vb2_qbuf(&vfh->queue, b);
 }
 
 static int
@@ -951,13 +935,8 @@ isp_video_dqbuf(struct file *file, void *fh, struct v4l2_buffer *b)
 {
 	struct isp_video_fh *vfh = to_isp_video_fh(fh);
 	struct isp_video *video = video_drvdata(file);
-	int ret;
 
-	mutex_lock(&video->queue_lock);
-	ret = vb2_dqbuf(&vfh->queue, b, file->f_flags & O_NONBLOCK);
-	mutex_unlock(&video->queue_lock);
-
-	return ret;
+	return vb2_dqbuf(&vfh->queue, b, file->f_flags & O_NONBLOCK);
 }
 
 static int isp_video_check_external_subdevs(struct isp_video *video,
@@ -1096,8 +1075,6 @@ isp_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
 	if (type != video->type)
 		return -EINVAL;
 
-	mutex_lock(&video->stream_lock);
-
 	/* Start streaming on the pipeline. No link touching an entity in the
 	 * pipeline can be activated or deactivated once streaming is started.
 	 */
@@ -1106,7 +1083,7 @@ isp_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
 
 	ret = media_entity_enum_init(&pipe->ent_enum, &video->isp->media_dev);
 	if (ret)
-		goto err_enum_init;
+		return ret;
 
 	/* TODO: Implement PM QoS */
 	pipe->l3_ick = clk_get_rate(video->isp->clock[ISP_CLK_L3_ICK]);
@@ -1158,14 +1135,10 @@ isp_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
 	atomic_set(&pipe->frame_number, -1);
 	pipe->field = vfh->format.fmt.pix.field;
 
-	mutex_lock(&video->queue_lock);
 	ret = vb2_streamon(&vfh->queue, type);
-	mutex_unlock(&video->queue_lock);
 	if (ret < 0)
 		goto err_check_format;
 
-	mutex_unlock(&video->stream_lock);
-
 	return 0;
 
 err_check_format:
@@ -1184,9 +1157,6 @@ isp_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
 
 	media_entity_enum_cleanup(&pipe->ent_enum);
 
-err_enum_init:
-	mutex_unlock(&video->stream_lock);
-
 	return ret;
 }
 
@@ -1203,15 +1173,10 @@ isp_video_streamoff(struct file *file, void *fh, enum v4l2_buf_type type)
 	if (type != video->type)
 		return -EINVAL;
 
-	mutex_lock(&video->stream_lock);
-
 	/* Make sure we're not streaming yet. */
-	mutex_lock(&video->queue_lock);
 	streaming = vb2_is_streaming(&vfh->queue);
-	mutex_unlock(&video->queue_lock);
-
 	if (!streaming)
-		goto done;
+		return 0;
 
 	/* Update the pipeline state. */
 	if (video->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
@@ -1229,19 +1194,13 @@ isp_video_streamoff(struct file *file, void *fh, enum v4l2_buf_type type)
 	omap3isp_pipeline_set_stream(pipe, ISP_PIPELINE_STREAM_STOPPED);
 	omap3isp_video_cancel_stream(video);
 
-	mutex_lock(&video->queue_lock);
 	vb2_streamoff(&vfh->queue, type);
-	mutex_unlock(&video->queue_lock);
 	video->queue = NULL;
 	video->error = false;
 
 	/* TODO: Implement PM QoS */
 	media_pipeline_stop(&video->video.entity);
-
 	media_entity_enum_cleanup(&pipe->ent_enum);
-
-done:
-	mutex_unlock(&video->stream_lock);
 	return 0;
 }
 
@@ -1333,6 +1292,7 @@ static int isp_video_open(struct file *file)
 	queue->buf_struct_size = sizeof(struct isp_buffer);
 	queue->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
 	queue->dev = video->isp->dev;
+	queue->lock = &video->queue_lock;
 
 	ret = vb2_queue_init(&handle->queue);
 	if (ret < 0) {
@@ -1363,10 +1323,9 @@ static int isp_video_release(struct file *file)
 	struct v4l2_fh *vfh = file->private_data;
 	struct isp_video_fh *handle = to_isp_video_fh(vfh);
 
+	mutex_lock(&video->queue_lock);
 	/* Disable streaming and free the buffers queue resources. */
 	isp_video_streamoff(file, vfh, video->type);
-
-	mutex_lock(&video->queue_lock);
 	vb2_queue_release(&handle->queue);
 	mutex_unlock(&video->queue_lock);
 
@@ -1449,7 +1408,6 @@ int omap3isp_video_init(struct isp_video *video, const char *name)
 	atomic_set(&video->active, 0);
 
 	spin_lock_init(&video->pipe.lock);
-	mutex_init(&video->stream_lock);
 	mutex_init(&video->queue_lock);
 	spin_lock_init(&video->irqlock);
 
@@ -1474,7 +1432,6 @@ void omap3isp_video_cleanup(struct isp_video *video)
 {
 	media_entity_cleanup(&video->video.entity);
 	mutex_destroy(&video->queue_lock);
-	mutex_destroy(&video->stream_lock);
 	mutex_destroy(&video->mutex);
 }
 
diff --git a/drivers/media/platform/omap3isp/ispvideo.h b/drivers/media/platform/omap3isp/ispvideo.h
index f6a2082b4a0a..5a8fba85e0eb 100644
--- a/drivers/media/platform/omap3isp/ispvideo.h
+++ b/drivers/media/platform/omap3isp/ispvideo.h
@@ -167,7 +167,6 @@ struct isp_video {
 
 	/* Pipeline state */
 	struct isp_pipeline pipe;
-	struct mutex stream_lock;	/* pipeline and stream states */
 	bool error;
 
 	/* Video buffers queue */
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v4 05/17] mtk-mdp: Add locks for capture and output vb2_queues
  2018-06-15 19:07 [PATCH v4 00/17] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
                   ` (3 preceding siblings ...)
  2018-06-15 19:07 ` [PATCH v4 04/17] omap3isp: " Ezequiel Garcia
@ 2018-06-15 19:07 ` Ezequiel Garcia
  2018-06-15 19:07 ` [PATCH v4 06/17] s5p-g2d: Implement wait_prepare and wait_finish Ezequiel Garcia
                   ` (12 subsequent siblings)
  17 siblings, 0 replies; 28+ messages in thread
From: Ezequiel Garcia @ 2018-06-15 19:07 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Ezequiel Garcia

Use the mutex in struct mtk_mdp_ctx to protect the
capture and output  vb2_queues. This allows to replace
the ad-hoc wait_{prepare, finish} with
vb2_ops_wait_{prepare, finish}.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
index 583d47724ee8..c2e2f5f1ebf1 100644
--- a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
+++ b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
@@ -394,20 +394,6 @@ static bool mtk_mdp_ctx_state_is_set(struct mtk_mdp_ctx *ctx, u32 mask)
 	return ret;
 }
 
-static void mtk_mdp_ctx_lock(struct vb2_queue *vq)
-{
-	struct mtk_mdp_ctx *ctx = vb2_get_drv_priv(vq);
-
-	mutex_lock(&ctx->mdp_dev->lock);
-}
-
-static void mtk_mdp_ctx_unlock(struct vb2_queue *vq)
-{
-	struct mtk_mdp_ctx *ctx = vb2_get_drv_priv(vq);
-
-	mutex_unlock(&ctx->mdp_dev->lock);
-}
-
 static void mtk_mdp_set_frame_size(struct mtk_mdp_frame *frame, int width,
 				   int height)
 {
@@ -625,10 +611,10 @@ static const struct vb2_ops mtk_mdp_m2m_qops = {
 	.queue_setup	 = mtk_mdp_m2m_queue_setup,
 	.buf_prepare	 = mtk_mdp_m2m_buf_prepare,
 	.buf_queue	 = mtk_mdp_m2m_buf_queue,
-	.wait_prepare	 = mtk_mdp_ctx_unlock,
-	.wait_finish	 = mtk_mdp_ctx_lock,
 	.stop_streaming	 = mtk_mdp_m2m_stop_streaming,
 	.start_streaming = mtk_mdp_m2m_start_streaming,
+	.wait_prepare	 = vb2_ops_wait_prepare,
+	.wait_finish	 = vb2_ops_wait_finish,
 };
 
 static int mtk_mdp_m2m_querycap(struct file *file, void *fh,
@@ -996,6 +982,7 @@ static int mtk_mdp_m2m_queue_init(void *priv, struct vb2_queue *src_vq,
 	src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
 	src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
 	src_vq->dev = &ctx->mdp_dev->pdev->dev;
+	src_vq->lock = &ctx->mdp_dev->lock;
 
 	ret = vb2_queue_init(src_vq);
 	if (ret)
@@ -1010,6 +997,7 @@ static int mtk_mdp_m2m_queue_init(void *priv, struct vb2_queue *src_vq,
 	dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
 	dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
 	dst_vq->dev = &ctx->mdp_dev->pdev->dev;
+	dst_vq->lock = &ctx->mdp_dev->lock;
 
 	return vb2_queue_init(dst_vq);
 }
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v4 06/17] s5p-g2d: Implement wait_prepare and wait_finish
  2018-06-15 19:07 [PATCH v4 00/17] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
                   ` (4 preceding siblings ...)
  2018-06-15 19:07 ` [PATCH v4 05/17] mtk-mdp: Add locks for capture and output vb2_queues Ezequiel Garcia
@ 2018-06-15 19:07 ` Ezequiel Garcia
  2018-06-15 19:07 ` [PATCH v4 07/17] staging: bcm2835-camera: Provide lock for vb2_queue Ezequiel Garcia
                   ` (11 subsequent siblings)
  17 siblings, 0 replies; 28+ messages in thread
From: Ezequiel Garcia @ 2018-06-15 19:07 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Ezequiel Garcia

This driver is currently specifying a vb2_queue lock,
which means it straightforward to implement wait_prepare
and wait_finish.

Having these callbacks releases the queue lock while blocking,
which improves latency by allowing for example streamoff
or qbuf operations while waiting in dqbuf.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/media/platform/s5p-g2d/g2d.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/platform/s5p-g2d/g2d.c b/drivers/media/platform/s5p-g2d/g2d.c
index 66aa8cf1d048..ce4280730835 100644
--- a/drivers/media/platform/s5p-g2d/g2d.c
+++ b/drivers/media/platform/s5p-g2d/g2d.c
@@ -142,6 +142,8 @@ static const struct vb2_ops g2d_qops = {
 	.queue_setup	= g2d_queue_setup,
 	.buf_prepare	= g2d_buf_prepare,
 	.buf_queue	= g2d_buf_queue,
+	.wait_prepare	= vb2_ops_wait_prepare,
+	.wait_finish	= vb2_ops_wait_finish,
 };
 
 static int queue_init(void *priv, struct vb2_queue *src_vq,
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v4 07/17] staging: bcm2835-camera: Provide lock for vb2_queue
  2018-06-15 19:07 [PATCH v4 00/17] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
                   ` (5 preceding siblings ...)
  2018-06-15 19:07 ` [PATCH v4 06/17] s5p-g2d: Implement wait_prepare and wait_finish Ezequiel Garcia
@ 2018-06-15 19:07 ` Ezequiel Garcia
  2018-06-15 19:07 ` [PATCH v4 08/17] venus: Add video_device and vb2_queue locks Ezequiel Garcia
                   ` (10 subsequent siblings)
  17 siblings, 0 replies; 28+ messages in thread
From: Ezequiel Garcia @ 2018-06-15 19:07 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Ezequiel Garcia

Use the device mutex to protect the vb2_queue.
This allows to replace the ad-hoc wait_{prepare, finish}
with vb2_ops_wait_{prepare, finish}.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 .../bcm2835-camera/bcm2835-camera.c           | 24 ++++---------------
 1 file changed, 5 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index ce26741ae9d9..6dd0c838db05 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -628,20 +628,6 @@ static void stop_streaming(struct vb2_queue *vq)
 		v4l2_err(&dev->v4l2_dev, "Failed to disable camera\n");
 }
 
-static void bm2835_mmal_lock(struct vb2_queue *vq)
-{
-	struct bm2835_mmal_dev *dev = vb2_get_drv_priv(vq);
-
-	mutex_lock(&dev->mutex);
-}
-
-static void bm2835_mmal_unlock(struct vb2_queue *vq)
-{
-	struct bm2835_mmal_dev *dev = vb2_get_drv_priv(vq);
-
-	mutex_unlock(&dev->mutex);
-}
-
 static const struct vb2_ops bm2835_mmal_video_qops = {
 	.queue_setup = queue_setup,
 	.buf_init = buffer_init,
@@ -650,8 +636,8 @@ static const struct vb2_ops bm2835_mmal_video_qops = {
 	.buf_queue = buffer_queue,
 	.start_streaming = start_streaming,
 	.stop_streaming = stop_streaming,
-	.wait_prepare = bm2835_mmal_unlock,
-	.wait_finish = bm2835_mmal_lock,
+	.wait_prepare = vb2_ops_wait_prepare,
+	.wait_finish = vb2_ops_wait_finish,
 };
 
 /* ------------------------------------------------------------------
@@ -1864,6 +1850,8 @@ static int bcm2835_mmal_probe(struct platform_device *pdev)
 			goto cleanup_gdev;
 		}
 
+		/* v4l2 core mutex used to protect all fops and v4l2 ioctls. */
+		mutex_init(&dev->mutex);
 		dev->camera_num = camera;
 		dev->max_width = resolutions[camera][0];
 		dev->max_height = resolutions[camera][1];
@@ -1908,13 +1896,11 @@ static int bcm2835_mmal_probe(struct platform_device *pdev)
 		q->ops = &bm2835_mmal_video_qops;
 		q->mem_ops = &vb2_vmalloc_memops;
 		q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
+		q->lock = &dev->mutex;
 		ret = vb2_queue_init(q);
 		if (ret < 0)
 			goto unreg_dev;
 
-		/* v4l2 core mutex used to protect all fops and v4l2 ioctls. */
-		mutex_init(&dev->mutex);
-
 		/* initialise video devices */
 		ret = bm2835_mmal_init_device(dev, &dev->vdev);
 		if (ret < 0)
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v4 08/17] venus: Add video_device and vb2_queue locks
  2018-06-15 19:07 [PATCH v4 00/17] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
                   ` (6 preceding siblings ...)
  2018-06-15 19:07 ` [PATCH v4 07/17] staging: bcm2835-camera: Provide lock for vb2_queue Ezequiel Garcia
@ 2018-06-15 19:07 ` Ezequiel Garcia
  2018-06-15 19:07 ` [PATCH v4 09/17] davinci_vpfe: " Ezequiel Garcia
                   ` (9 subsequent siblings)
  17 siblings, 0 replies; 28+ messages in thread
From: Ezequiel Garcia @ 2018-06-15 19:07 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Ezequiel Garcia

video_device and vb2_queue locks are now both
mandatory. Add them, remove driver ad-hoc locking,
and implement wait_{prepare, finish}.

To stay on the safe side, this commit uses a single mutex
for both locks. Better latency can be obtained by separating
these if needed.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/media/platform/qcom/venus/core.h    |  4 +++-
 drivers/media/platform/qcom/venus/helpers.c | 16 ++++++-------
 drivers/media/platform/qcom/venus/vdec.c    | 25 +++++++++------------
 drivers/media/platform/qcom/venus/venc.c    | 19 ++++++++--------
 4 files changed, 30 insertions(+), 34 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
index 0360d295f4c8..5617d0af990f 100644
--- a/drivers/media/platform/qcom/venus/core.h
+++ b/drivers/media/platform/qcom/venus/core.h
@@ -102,6 +102,8 @@ struct venus_core {
 	struct device *dev_dec;
 	struct device *dev_enc;
 	struct mutex lock;
+	struct mutex dec_lock;
+	struct mutex enc_lock;
 	struct list_head instances;
 	atomic_t insts_count;
 	unsigned int state;
@@ -243,7 +245,7 @@ struct venus_buffer {
  */
 struct venus_inst {
 	struct list_head list;
-	struct mutex lock;
+	struct mutex *lock;
 	struct venus_core *core;
 	struct list_head internalbufs;
 	struct list_head registeredbufs;
diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
index 0ce9559a2924..5a2dda6fb984 100644
--- a/drivers/media/platform/qcom/venus/helpers.c
+++ b/drivers/media/platform/qcom/venus/helpers.c
@@ -512,7 +512,7 @@ static void delayed_process_buf_func(struct work_struct *work)
 
 	inst = container_of(work, struct venus_inst, delayed_process_work);
 
-	mutex_lock(&inst->lock);
+	mutex_lock(inst->lock);
 
 	if (!(inst->streamon_out & inst->streamon_cap))
 		goto unlock;
@@ -528,7 +528,7 @@ static void delayed_process_buf_func(struct work_struct *work)
 		list_del_init(&buf->ref_list);
 	}
 unlock:
-	mutex_unlock(&inst->lock);
+	mutex_unlock(inst->lock);
 }
 
 void venus_helper_release_buf_ref(struct venus_inst *inst, unsigned int idx)
@@ -621,7 +621,7 @@ void venus_helper_vb2_buf_queue(struct vb2_buffer *vb)
 	struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
 	int ret;
 
-	mutex_lock(&inst->lock);
+	mutex_lock(inst->lock);
 
 	v4l2_m2m_buf_queue(m2m_ctx, vbuf);
 
@@ -637,7 +637,7 @@ void venus_helper_vb2_buf_queue(struct vb2_buffer *vb)
 		return_buf_error(inst, vbuf);
 
 unlock:
-	mutex_unlock(&inst->lock);
+	mutex_unlock(inst->lock);
 }
 EXPORT_SYMBOL_GPL(venus_helper_vb2_buf_queue);
 
@@ -659,7 +659,7 @@ void venus_helper_vb2_stop_streaming(struct vb2_queue *q)
 	struct venus_core *core = inst->core;
 	int ret;
 
-	mutex_lock(&inst->lock);
+	mutex_lock(inst->lock);
 
 	if (inst->streamon_out & inst->streamon_cap) {
 		ret = hfi_session_stop(inst);
@@ -685,7 +685,7 @@ void venus_helper_vb2_stop_streaming(struct vb2_queue *q)
 	else
 		inst->streamon_cap = 0;
 
-	mutex_unlock(&inst->lock);
+	mutex_unlock(inst->lock);
 }
 EXPORT_SYMBOL_GPL(venus_helper_vb2_stop_streaming);
 
@@ -731,7 +731,7 @@ void venus_helper_m2m_device_run(void *priv)
 	struct v4l2_m2m_buffer *buf, *n;
 	int ret;
 
-	mutex_lock(&inst->lock);
+	mutex_lock(inst->lock);
 
 	v4l2_m2m_for_each_dst_buf_safe(m2m_ctx, buf, n) {
 		ret = session_process_buf(inst, &buf->vb);
@@ -745,7 +745,7 @@ void venus_helper_m2m_device_run(void *priv)
 			return_buf_error(inst, &buf->vb);
 	}
 
-	mutex_unlock(&inst->lock);
+	mutex_unlock(inst->lock);
 }
 EXPORT_SYMBOL_GPL(venus_helper_m2m_device_run);
 
diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
index 49bbd1861d3a..41d14df46f5d 100644
--- a/drivers/media/platform/qcom/venus/vdec.c
+++ b/drivers/media/platform/qcom/venus/vdec.c
@@ -493,14 +493,12 @@ vdec_decoder_cmd(struct file *file, void *fh, struct v4l2_decoder_cmd *cmd)
 	if (ret)
 		return ret;
 
-	mutex_lock(&inst->lock);
-
 	/*
 	 * Implement V4L2_DEC_CMD_STOP by enqueue an empty buffer on decoder
 	 * input to signal EOS.
 	 */
 	if (!(inst->streamon_out & inst->streamon_cap))
-		goto unlock;
+		return 0;
 
 	fdata.buffer_type = HFI_BUFFER_INPUT;
 	fdata.flags |= HFI_BUFFERFLAG_EOS;
@@ -508,8 +506,6 @@ vdec_decoder_cmd(struct file *file, void *fh, struct v4l2_decoder_cmd *cmd)
 
 	ret = hfi_session_process_buf(inst, &fdata);
 
-unlock:
-	mutex_unlock(&inst->lock);
 	return ret;
 }
 
@@ -720,17 +716,13 @@ static int vdec_start_streaming(struct vb2_queue *q, unsigned int count)
 	u32 ptype;
 	int ret;
 
-	mutex_lock(&inst->lock);
-
 	if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
 		inst->streamon_out = 1;
 	else
 		inst->streamon_cap = 1;
 
-	if (!(inst->streamon_out & inst->streamon_cap)) {
-		mutex_unlock(&inst->lock);
+	if (!(inst->streamon_out & inst->streamon_cap))
 		return 0;
-	}
 
 	venus_helper_init_instance(inst);
 
@@ -771,8 +763,6 @@ static int vdec_start_streaming(struct vb2_queue *q, unsigned int count)
 	if (ret)
 		goto deinit_sess;
 
-	mutex_unlock(&inst->lock);
-
 	return 0;
 
 deinit_sess:
@@ -783,7 +773,6 @@ static int vdec_start_streaming(struct vb2_queue *q, unsigned int count)
 		inst->streamon_out = 0;
 	else
 		inst->streamon_cap = 0;
-	mutex_unlock(&inst->lock);
 	return ret;
 }
 
@@ -794,6 +783,8 @@ static const struct vb2_ops vdec_vb2_ops = {
 	.start_streaming = vdec_start_streaming,
 	.stop_streaming = venus_helper_vb2_stop_streaming,
 	.buf_queue = venus_helper_vb2_buf_queue,
+	.wait_prepare = vb2_ops_wait_prepare,
+	.wait_finish = vb2_ops_wait_finish,
 };
 
 static void vdec_buf_done(struct venus_inst *inst, unsigned int buf_type,
@@ -940,6 +931,7 @@ static int m2m_queue_init(void *priv, struct vb2_queue *src_vq,
 	src_vq->allow_zero_bytesused = 1;
 	src_vq->min_buffers_needed = 1;
 	src_vq->dev = inst->core->dev;
+	src_vq->lock = &inst->core->dec_lock;
 	ret = vb2_queue_init(src_vq);
 	if (ret)
 		return ret;
@@ -954,6 +946,7 @@ static int m2m_queue_init(void *priv, struct vb2_queue *src_vq,
 	dst_vq->allow_zero_bytesused = 1;
 	dst_vq->min_buffers_needed = 1;
 	dst_vq->dev = inst->core->dev;
+	dst_vq->lock = &inst->core->dec_lock;
 	ret = vb2_queue_init(dst_vq);
 	if (ret) {
 		vb2_queue_release(src_vq);
@@ -976,9 +969,9 @@ static int vdec_open(struct file *file)
 	INIT_LIST_HEAD(&inst->registeredbufs);
 	INIT_LIST_HEAD(&inst->internalbufs);
 	INIT_LIST_HEAD(&inst->list);
-	mutex_init(&inst->lock);
 
 	inst->core = core;
+	inst->lock = &core->dec_lock;
 	inst->session_type = VIDC_SESSION_TYPE_DEC;
 	inst->num_output_bufs = 1;
 
@@ -1044,7 +1037,6 @@ static int vdec_close(struct file *file)
 	v4l2_m2m_release(inst->m2m_dev);
 	vdec_ctrl_deinit(inst);
 	hfi_session_destroy(inst);
-	mutex_destroy(&inst->lock);
 	v4l2_fh_del(&inst->fh);
 	v4l2_fh_exit(&inst->fh);
 
@@ -1092,12 +1084,14 @@ static int vdec_probe(struct platform_device *pdev)
 	if (!vdev)
 		return -ENOMEM;
 
+	mutex_init(&core->dec_lock);
 	strlcpy(vdev->name, "qcom-venus-decoder", sizeof(vdev->name));
 	vdev->release = video_device_release;
 	vdev->fops = &vdec_fops;
 	vdev->ioctl_ops = &vdec_ioctl_ops;
 	vdev->vfl_dir = VFL_DIR_M2M;
 	vdev->v4l2_dev = &core->v4l2_dev;
+	vdev->lock = &core->dec_lock;
 	vdev->device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING;
 
 	ret = video_register_device(vdev, VFL_TYPE_GRABBER, -1);
@@ -1123,6 +1117,7 @@ static int vdec_remove(struct platform_device *pdev)
 
 	video_unregister_device(core->vdev_dec);
 	pm_runtime_disable(core->dev_dec);
+	mutex_destroy(&core->dec_lock);
 
 	return 0;
 }
diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
index 6b2ce479584e..016af21abf5d 100644
--- a/drivers/media/platform/qcom/venus/venc.c
+++ b/drivers/media/platform/qcom/venus/venc.c
@@ -922,17 +922,13 @@ static int venc_start_streaming(struct vb2_queue *q, unsigned int count)
 	struct venus_inst *inst = vb2_get_drv_priv(q);
 	int ret;
 
-	mutex_lock(&inst->lock);
-
 	if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
 		inst->streamon_out = 1;
 	else
 		inst->streamon_cap = 1;
 
-	if (!(inst->streamon_out & inst->streamon_cap)) {
-		mutex_unlock(&inst->lock);
+	if (!(inst->streamon_out & inst->streamon_cap))
 		return 0;
-	}
 
 	venus_helper_init_instance(inst);
 
@@ -960,8 +956,6 @@ static int venc_start_streaming(struct vb2_queue *q, unsigned int count)
 	if (ret)
 		goto deinit_sess;
 
-	mutex_unlock(&inst->lock);
-
 	return 0;
 
 deinit_sess:
@@ -972,7 +966,6 @@ static int venc_start_streaming(struct vb2_queue *q, unsigned int count)
 		inst->streamon_out = 0;
 	else
 		inst->streamon_cap = 0;
-	mutex_unlock(&inst->lock);
 	return ret;
 }
 
@@ -983,6 +976,8 @@ static const struct vb2_ops venc_vb2_ops = {
 	.start_streaming = venc_start_streaming,
 	.stop_streaming = venus_helper_vb2_stop_streaming,
 	.buf_queue = venus_helper_vb2_buf_queue,
+	.wait_prepare = vb2_ops_wait_prepare,
+	.wait_finish = vb2_ops_wait_finish,
 };
 
 static void venc_buf_done(struct venus_inst *inst, unsigned int buf_type,
@@ -1054,6 +1049,7 @@ static int m2m_queue_init(void *priv, struct vb2_queue *src_vq,
 	src_vq->allow_zero_bytesused = 1;
 	src_vq->min_buffers_needed = 1;
 	src_vq->dev = inst->core->dev;
+	src_vq->lock = &inst->core->enc_lock;
 	if (inst->core->res->hfi_version == HFI_VERSION_1XX)
 		src_vq->bidirectional = 1;
 	ret = vb2_queue_init(src_vq);
@@ -1070,6 +1066,7 @@ static int m2m_queue_init(void *priv, struct vb2_queue *src_vq,
 	dst_vq->allow_zero_bytesused = 1;
 	dst_vq->min_buffers_needed = 1;
 	dst_vq->dev = inst->core->dev;
+	dst_vq->lock = &inst->core->enc_lock;
 	ret = vb2_queue_init(dst_vq);
 	if (ret) {
 		vb2_queue_release(src_vq);
@@ -1121,9 +1118,9 @@ static int venc_open(struct file *file)
 	INIT_LIST_HEAD(&inst->registeredbufs);
 	INIT_LIST_HEAD(&inst->internalbufs);
 	INIT_LIST_HEAD(&inst->list);
-	mutex_init(&inst->lock);
 
 	inst->core = core;
+	inst->lock = &core->enc_lock;
 	inst->session_type = VIDC_SESSION_TYPE_ENC;
 
 	venus_helper_init_instance(inst);
@@ -1188,7 +1185,6 @@ static int venc_close(struct file *file)
 	v4l2_m2m_release(inst->m2m_dev);
 	venc_ctrl_deinit(inst);
 	hfi_session_destroy(inst);
-	mutex_destroy(&inst->lock);
 	v4l2_fh_del(&inst->fh);
 	v4l2_fh_exit(&inst->fh);
 
@@ -1237,11 +1233,13 @@ static int venc_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	strlcpy(vdev->name, "qcom-venus-encoder", sizeof(vdev->name));
+	mutex_init(&core->enc_lock);
 	vdev->release = video_device_release;
 	vdev->fops = &venc_fops;
 	vdev->ioctl_ops = &venc_ioctl_ops;
 	vdev->vfl_dir = VFL_DIR_M2M;
 	vdev->v4l2_dev = &core->v4l2_dev;
+	vdev->lock = &core->enc_lock;
 	vdev->device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING;
 
 	ret = video_register_device(vdev, VFL_TYPE_GRABBER, -1);
@@ -1267,6 +1265,7 @@ static int venc_remove(struct platform_device *pdev)
 
 	video_unregister_device(core->vdev_enc);
 	pm_runtime_disable(core->dev_enc);
+	mutex_destroy(&core->enc_lock);
 
 	return 0;
 }
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v4 09/17] davinci_vpfe: Add video_device and vb2_queue locks
  2018-06-15 19:07 [PATCH v4 00/17] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
                   ` (7 preceding siblings ...)
  2018-06-15 19:07 ` [PATCH v4 08/17] venus: Add video_device and vb2_queue locks Ezequiel Garcia
@ 2018-06-15 19:07 ` Ezequiel Garcia
  2018-06-15 19:07 ` [PATCH v4 10/17] mx_emmaprp: Implement wait_prepare and wait_finish Ezequiel Garcia
                   ` (8 subsequent siblings)
  17 siblings, 0 replies; 28+ messages in thread
From: Ezequiel Garcia @ 2018-06-15 19:07 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Ezequiel Garcia

Currently, this driver does not serialize its video4linux
ioctls, which is a bug, as race conditions might appear.

In addition, video_device and vb2_queue locks are now both
mandatory. Add them, and implement wait_prepare and
wait_finish.

To stay on the safe side, this commit uses a single mutex
for both locks. Better latency can be obtained by separating
these if needed.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/staging/media/davinci_vpfe/vpfe_video.c | 6 +++++-
 drivers/staging/media/davinci_vpfe/vpfe_video.h | 2 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.c b/drivers/staging/media/davinci_vpfe/vpfe_video.c
index 390fc98d07dd..1269a983455e 100644
--- a/drivers/staging/media/davinci_vpfe/vpfe_video.c
+++ b/drivers/staging/media/davinci_vpfe/vpfe_video.c
@@ -1312,6 +1312,8 @@ static const struct vb2_ops video_qops = {
 	.stop_streaming		= vpfe_stop_streaming,
 	.buf_cleanup		= vpfe_buf_cleanup,
 	.buf_queue		= vpfe_buffer_queue,
+	.wait_prepare		= vb2_ops_wait_prepare,
+	.wait_finish		= vb2_ops_wait_finish,
 };
 
 /*
@@ -1357,6 +1359,7 @@ static int vpfe_reqbufs(struct file *file, void *priv,
 	q->buf_struct_size = sizeof(struct vpfe_cap_buffer);
 	q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
 	q->dev = vpfe_dev->pdev;
+	q->lock = &video->lock;
 
 	ret = vb2_queue_init(q);
 	if (ret) {
@@ -1598,17 +1601,18 @@ int vpfe_video_init(struct vpfe_video_device *video, const char *name)
 		return -EINVAL;
 	}
 	/* Initialize field of video device */
+	mutex_init(&video->lock);
 	video->video_dev.release = video_device_release;
 	video->video_dev.fops = &vpfe_fops;
 	video->video_dev.ioctl_ops = &vpfe_ioctl_ops;
 	video->video_dev.minor = -1;
 	video->video_dev.tvnorms = 0;
+	video->video_dev.lock = &video->lock;
 	snprintf(video->video_dev.name, sizeof(video->video_dev.name),
 		 "DAVINCI VIDEO %s %s", name, direction);
 
 	spin_lock_init(&video->irqlock);
 	spin_lock_init(&video->dma_queue_lock);
-	mutex_init(&video->lock);
 	ret = media_entity_pads_init(&video->video_dev.entity,
 				1, &video->pad);
 	if (ret < 0)
diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.h b/drivers/staging/media/davinci_vpfe/vpfe_video.h
index 22136d3dadcb..4bbd219e8329 100644
--- a/drivers/staging/media/davinci_vpfe/vpfe_video.h
+++ b/drivers/staging/media/davinci_vpfe/vpfe_video.h
@@ -128,7 +128,7 @@ struct vpfe_video_device {
 	spinlock_t				irqlock;
 	/* IRQ lock for DMA queue */
 	spinlock_t				dma_queue_lock;
-	/* lock used to access this structure */
+	/* lock used to serialize all video4linux ioctls */
 	struct mutex				lock;
 	/* number of users performing IO */
 	u32					io_usrs;
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v4 10/17] mx_emmaprp: Implement wait_prepare and wait_finish
  2018-06-15 19:07 [PATCH v4 00/17] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
                   ` (8 preceding siblings ...)
  2018-06-15 19:07 ` [PATCH v4 09/17] davinci_vpfe: " Ezequiel Garcia
@ 2018-06-15 19:07 ` Ezequiel Garcia
  2018-06-15 19:07 ` [PATCH v4 11/17] m2m-deinterlace: " Ezequiel Garcia
                   ` (7 subsequent siblings)
  17 siblings, 0 replies; 28+ messages in thread
From: Ezequiel Garcia @ 2018-06-15 19:07 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Ezequiel Garcia

This driver is currently specifying a video_device lock,
which means it is protecting all the ioctls (including
queue ioctls) with a single mutex.

It's therefore straightforward to implement wait_prepare
and wait_finish, by explicitly setting the vb2_queue lock.

Having these callbacks releases the queue lock while blocking,
which improves latency by allowing for example streamoff
or qbuf operations while waiting in dqbuf.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/media/platform/mx2_emmaprp.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/media/platform/mx2_emmaprp.c b/drivers/media/platform/mx2_emmaprp.c
index 5a8eff60e95f..7f9b356e7cc7 100644
--- a/drivers/media/platform/mx2_emmaprp.c
+++ b/drivers/media/platform/mx2_emmaprp.c
@@ -747,6 +747,8 @@ static const struct vb2_ops emmaprp_qops = {
 	.queue_setup	 = emmaprp_queue_setup,
 	.buf_prepare	 = emmaprp_buf_prepare,
 	.buf_queue	 = emmaprp_buf_queue,
+	.wait_prepare	 = vb2_ops_wait_prepare,
+	.wait_finish	 = vb2_ops_wait_finish,
 };
 
 static int queue_init(void *priv, struct vb2_queue *src_vq,
@@ -763,6 +765,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
 	src_vq->mem_ops = &vb2_dma_contig_memops;
 	src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
 	src_vq->dev = ctx->dev->v4l2_dev.dev;
+	src_vq->lock = &ctx->dev->dev_mutex;
 
 	ret = vb2_queue_init(src_vq);
 	if (ret)
@@ -776,6 +779,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
 	dst_vq->mem_ops = &vb2_dma_contig_memops;
 	dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
 	dst_vq->dev = ctx->dev->v4l2_dev.dev;
+	dst_vq->lock = &ctx->dev->dev_mutex;
 
 	return vb2_queue_init(dst_vq);
 }
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v4 11/17] m2m-deinterlace: Implement wait_prepare and wait_finish
  2018-06-15 19:07 [PATCH v4 00/17] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
                   ` (9 preceding siblings ...)
  2018-06-15 19:07 ` [PATCH v4 10/17] mx_emmaprp: Implement wait_prepare and wait_finish Ezequiel Garcia
@ 2018-06-15 19:07 ` Ezequiel Garcia
  2018-06-15 19:07 ` [PATCH v4 12/17] stk1160: Set the vb2_queue lock before calling vb2_queue_init Ezequiel Garcia
                   ` (6 subsequent siblings)
  17 siblings, 0 replies; 28+ messages in thread
From: Ezequiel Garcia @ 2018-06-15 19:07 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Ezequiel Garcia

This driver is currently specifying a video_device lock,
which means it is protecting all the ioctls (including
queue ioctls) with a single mutex.

It's therefore straightforward to implement wait_prepare
and wait_finish, by explicitly setting the vb2_queue lock.

Having these callbacks releases the queue lock while blocking,
which improves latency by allowing for example streamoff
or qbuf operations while waiting in dqbuf.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/media/platform/m2m-deinterlace.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/media/platform/m2m-deinterlace.c b/drivers/media/platform/m2m-deinterlace.c
index 1e4195144f39..94dd8ec0f265 100644
--- a/drivers/media/platform/m2m-deinterlace.c
+++ b/drivers/media/platform/m2m-deinterlace.c
@@ -856,6 +856,8 @@ static const struct vb2_ops deinterlace_qops = {
 	.queue_setup	 = deinterlace_queue_setup,
 	.buf_prepare	 = deinterlace_buf_prepare,
 	.buf_queue	 = deinterlace_buf_queue,
+	.wait_prepare	 = vb2_ops_wait_prepare,
+	.wait_finish	 = vb2_ops_wait_finish,
 };
 
 static int queue_init(void *priv, struct vb2_queue *src_vq,
@@ -872,6 +874,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
 	src_vq->mem_ops = &vb2_dma_contig_memops;
 	src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
 	src_vq->dev = ctx->dev->v4l2_dev.dev;
+	src_vq->lock = &ctx->dev->dev_mutex;
 	q_data[V4L2_M2M_SRC].fmt = &formats[0];
 	q_data[V4L2_M2M_SRC].width = 640;
 	q_data[V4L2_M2M_SRC].height = 480;
@@ -890,6 +893,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
 	dst_vq->mem_ops = &vb2_dma_contig_memops;
 	dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
 	dst_vq->dev = ctx->dev->v4l2_dev.dev;
+	dst_vq->lock = &ctx->dev->dev_mutex;
 	q_data[V4L2_M2M_DST].fmt = &formats[0];
 	q_data[V4L2_M2M_DST].width = 640;
 	q_data[V4L2_M2M_DST].height = 480;
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v4 12/17] stk1160: Set the vb2_queue lock before calling vb2_queue_init
  2018-06-15 19:07 [PATCH v4 00/17] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
                   ` (10 preceding siblings ...)
  2018-06-15 19:07 ` [PATCH v4 11/17] m2m-deinterlace: " Ezequiel Garcia
@ 2018-06-15 19:07 ` Ezequiel Garcia
  2018-06-15 19:07 ` [PATCH v4 13/17] dvb-core: Provide lock for vb2_queue Ezequiel Garcia
                   ` (5 subsequent siblings)
  17 siblings, 0 replies; 28+ messages in thread
From: Ezequiel Garcia @ 2018-06-15 19:07 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Ezequiel Garcia

The vb2_queue will soon be mandatory. The videobuf2 core
will throw a verbose warning if it's not set.

The stk1160 driver is setting the queue lock, but after
the vb2_queue_init call. Avoid the warning by setting
the lock before the queue initialization.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/media/usb/stk1160/stk1160-v4l.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/stk1160/stk1160-v4l.c b/drivers/media/usb/stk1160/stk1160-v4l.c
index 77b759a0bcd9..504e413edcd2 100644
--- a/drivers/media/usb/stk1160/stk1160-v4l.c
+++ b/drivers/media/usb/stk1160/stk1160-v4l.c
@@ -802,6 +802,7 @@ int stk1160_vb2_setup(struct stk1160 *dev)
 	q->buf_struct_size = sizeof(struct stk1160_buffer);
 	q->ops = &stk1160_video_qops;
 	q->mem_ops = &vb2_vmalloc_memops;
+	q->lock = &dev->vb_queue_lock;
 	q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
 
 	rc = vb2_queue_init(q);
@@ -827,7 +828,6 @@ int stk1160_video_register(struct stk1160 *dev)
 	 * It will be used to protect *only* v4l2 ioctls.
 	 */
 	dev->vdev.lock = &dev->v4l_lock;
-	dev->vdev.queue->lock = &dev->vb_queue_lock;
 
 	/* This will be used to set video_device parent */
 	dev->vdev.v4l2_dev = &dev->v4l2_dev;
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v4 13/17] dvb-core: Provide lock for vb2_queue
  2018-06-15 19:07 [PATCH v4 00/17] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
                   ` (11 preceding siblings ...)
  2018-06-15 19:07 ` [PATCH v4 12/17] stk1160: Set the vb2_queue lock before calling vb2_queue_init Ezequiel Garcia
@ 2018-06-15 19:07 ` Ezequiel Garcia
  2018-06-15 19:07 ` [PATCH v4 14/17] videobuf2-core: require q->lock Ezequiel Garcia
                   ` (4 subsequent siblings)
  17 siblings, 0 replies; 28+ messages in thread
From: Ezequiel Garcia @ 2018-06-15 19:07 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Ezequiel Garcia

Use the vb2 context mutex to protect the vb2_queue.
This allows to replace the ad-hoc wait_{prepare, finish}
with vb2_ops_wait_{prepare, finish}.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/media/dvb-core/dvb_vb2.c | 22 +++-------------------
 1 file changed, 3 insertions(+), 19 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_vb2.c b/drivers/media/dvb-core/dvb_vb2.c
index b811adf88afa..cd3ea44f0ae9 100644
--- a/drivers/media/dvb-core/dvb_vb2.c
+++ b/drivers/media/dvb-core/dvb_vb2.c
@@ -107,31 +107,14 @@ static void _stop_streaming(struct vb2_queue *vq)
 	spin_unlock_irqrestore(&ctx->slock, flags);
 }
 
-static void _dmxdev_lock(struct vb2_queue *vq)
-{
-	struct dvb_vb2_ctx *ctx = vb2_get_drv_priv(vq);
-
-	mutex_lock(&ctx->mutex);
-	dprintk(3, "[%s]\n", ctx->name);
-}
-
-static void _dmxdev_unlock(struct vb2_queue *vq)
-{
-	struct dvb_vb2_ctx *ctx = vb2_get_drv_priv(vq);
-
-	if (mutex_is_locked(&ctx->mutex))
-		mutex_unlock(&ctx->mutex);
-	dprintk(3, "[%s]\n", ctx->name);
-}
-
 static const struct vb2_ops dvb_vb2_qops = {
 	.queue_setup		= _queue_setup,
 	.buf_prepare		= _buffer_prepare,
 	.buf_queue		= _buffer_queue,
 	.start_streaming	= _start_streaming,
 	.stop_streaming		= _stop_streaming,
-	.wait_prepare		= _dmxdev_unlock,
-	.wait_finish		= _dmxdev_lock,
+	.wait_prepare		= vb2_ops_wait_prepare,
+	.wait_finish		= vb2_ops_wait_finish,
 };
 
 static void _fill_dmx_buffer(struct vb2_buffer *vb, void *pb)
@@ -183,6 +166,7 @@ int dvb_vb2_init(struct dvb_vb2_ctx *ctx, const char *name, int nonblocking)
 	q->mem_ops = &vb2_vmalloc_memops;
 	q->buf_ops = &dvb_vb2_buf_ops;
 	q->num_buffers = 0;
+	q->lock = &ctx->mutex;
 	ret = vb2_core_queue_init(q);
 	if (ret) {
 		ctx->state = DVB_VB2_STATE_NONE;
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v4 14/17] videobuf2-core: require q->lock
  2018-06-15 19:07 [PATCH v4 00/17] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
                   ` (12 preceding siblings ...)
  2018-06-15 19:07 ` [PATCH v4 13/17] dvb-core: Provide lock for vb2_queue Ezequiel Garcia
@ 2018-06-15 19:07 ` Ezequiel Garcia
  2018-06-15 19:07 ` [PATCH v4 15/17] videobuf2: assume q->lock is always set Ezequiel Garcia
                   ` (3 subsequent siblings)
  17 siblings, 0 replies; 28+ messages in thread
From: Ezequiel Garcia @ 2018-06-15 19:07 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Hans Verkuil

From: Hans Verkuil <hansverk@cisco.com>

Refuse to initialize a vb2 queue if there is no lock specified.

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

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index f32ec7342ef0..0d8e93f9a622 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -2005,6 +2005,7 @@ int vb2_core_queue_init(struct vb2_queue *q)
 	if (WARN_ON(!q)			  ||
 	    WARN_ON(!q->ops)		  ||
 	    WARN_ON(!q->mem_ops)	  ||
+	    WARN_ON(!q->lock)		  ||
 	    WARN_ON(!q->type)		  ||
 	    WARN_ON(!q->io_modes)	  ||
 	    WARN_ON(!q->ops->queue_setup) ||
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v4 15/17] videobuf2: assume q->lock is always set
  2018-06-15 19:07 [PATCH v4 00/17] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
                   ` (13 preceding siblings ...)
  2018-06-15 19:07 ` [PATCH v4 14/17] videobuf2-core: require q->lock Ezequiel Garcia
@ 2018-06-15 19:07 ` Ezequiel Garcia
  2018-06-15 19:07 ` [PATCH v4 16/17] v4l2-ioctl.c: assume queue->lock " Ezequiel Garcia
                   ` (2 subsequent siblings)
  17 siblings, 0 replies; 28+ messages in thread
From: Ezequiel Garcia @ 2018-06-15 19:07 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Hans Verkuil

From: Hans Verkuil <hansverk@cisco.com>

Drop checks for q->lock. Drop calls to wait_finish/prepare, just lock/unlock
q->lock.

Signed-off-by: Hans Verkuil <hansverk@cisco.com>
---
 .../media/common/videobuf2/videobuf2-core.c   | 21 ++++++---------
 .../media/common/videobuf2/videobuf2-v4l2.c   | 27 +++++--------------
 include/media/videobuf2-core.h                |  2 --
 3 files changed, 15 insertions(+), 35 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index 0d8e93f9a622..e0c4e1a8f311 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -462,8 +462,7 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned int buffers)
 	 * counters to the kernel log.
 	 */
 	if (q->num_buffers) {
-		bool unbalanced = q->cnt_start_streaming != q->cnt_stop_streaming ||
-				  q->cnt_wait_prepare != q->cnt_wait_finish;
+		bool unbalanced = q->cnt_start_streaming != q->cnt_stop_streaming;
 
 		if (unbalanced || debug) {
 			pr_info("counters for queue %p:%s\n", q,
@@ -471,12 +470,8 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned int buffers)
 			pr_info("     setup: %u start_streaming: %u stop_streaming: %u\n",
 				q->cnt_queue_setup, q->cnt_start_streaming,
 				q->cnt_stop_streaming);
-			pr_info("     wait_prepare: %u wait_finish: %u\n",
-				q->cnt_wait_prepare, q->cnt_wait_finish);
 		}
 		q->cnt_queue_setup = 0;
-		q->cnt_wait_prepare = 0;
-		q->cnt_wait_finish = 0;
 		q->cnt_start_streaming = 0;
 		q->cnt_stop_streaming = 0;
 	}
@@ -1487,10 +1482,10 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, int nonblocking)
 
 		/*
 		 * We are streaming and blocking, wait for another buffer to
-		 * become ready or for streamoff. Driver's lock is released to
+		 * become ready or for streamoff. The queue's lock is released to
 		 * allow streamoff or qbuf to be called while waiting.
 		 */
-		call_void_qop(q, wait_prepare, q);
+		mutex_unlock(q->lock);
 
 		/*
 		 * All locks have been released, it is safe to sleep now.
@@ -1504,7 +1499,7 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, int nonblocking)
 		 * We need to reevaluate both conditions again after reacquiring
 		 * the locks or return an error if one occurred.
 		 */
-		call_void_qop(q, wait_finish, q);
+		mutex_lock(q->lock);
 		if (ret) {
 			dprintk(1, "sleep was interrupted\n");
 			return ret;
@@ -2531,10 +2526,10 @@ static int vb2_thread(void *data)
 			vb = q->bufs[index++];
 			prequeue--;
 		} else {
-			call_void_qop(q, wait_finish, q);
+			mutex_lock(q->lock);
 			if (!threadio->stop)
 				ret = vb2_core_dqbuf(q, &index, NULL, 0);
-			call_void_qop(q, wait_prepare, q);
+			mutex_unlock(q->lock);
 			dprintk(5, "file io: vb2_dqbuf result: %d\n", ret);
 			if (!ret)
 				vb = q->bufs[index];
@@ -2546,12 +2541,12 @@ static int vb2_thread(void *data)
 		if (vb->state != VB2_BUF_STATE_ERROR)
 			if (threadio->fnc(vb, threadio->priv))
 				break;
-		call_void_qop(q, wait_finish, q);
+		mutex_lock(q->lock);
 		if (copy_timestamp)
 			vb->timestamp = ktime_get_ns();
 		if (!threadio->stop)
 			ret = vb2_core_qbuf(q, vb->index, NULL);
-		call_void_qop(q, wait_prepare, q);
+		mutex_unlock(q->lock);
 		if (ret || threadio->stop)
 			break;
 	}
diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index 886a2d8d5c6c..7d2172468f72 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -852,9 +852,8 @@ EXPORT_SYMBOL_GPL(_vb2_fop_release);
 int vb2_fop_release(struct file *file)
 {
 	struct video_device *vdev = video_devdata(file);
-	struct mutex *lock = vdev->queue->lock ? vdev->queue->lock : vdev->lock;
 
-	return _vb2_fop_release(file, lock);
+	return _vb2_fop_release(file, vdev->queue->lock);
 }
 EXPORT_SYMBOL_GPL(vb2_fop_release);
 
@@ -862,12 +861,11 @@ ssize_t vb2_fop_write(struct file *file, const char __user *buf,
 		size_t count, loff_t *ppos)
 {
 	struct video_device *vdev = video_devdata(file);
-	struct mutex *lock = vdev->queue->lock ? vdev->queue->lock : vdev->lock;
 	int err = -EBUSY;
 
 	if (!(vdev->queue->io_modes & VB2_WRITE))
 		return -EINVAL;
-	if (lock && mutex_lock_interruptible(lock))
+	if (mutex_lock_interruptible(vdev->queue->lock))
 		return -ERESTARTSYS;
 	if (vb2_queue_is_busy(vdev, file))
 		goto exit;
@@ -876,8 +874,7 @@ ssize_t vb2_fop_write(struct file *file, const char __user *buf,
 	if (vdev->queue->fileio)
 		vdev->queue->owner = file->private_data;
 exit:
-	if (lock)
-		mutex_unlock(lock);
+	mutex_unlock(vdev->queue->lock);
 	return err;
 }
 EXPORT_SYMBOL_GPL(vb2_fop_write);
@@ -886,12 +883,11 @@ ssize_t vb2_fop_read(struct file *file, char __user *buf,
 		size_t count, loff_t *ppos)
 {
 	struct video_device *vdev = video_devdata(file);
-	struct mutex *lock = vdev->queue->lock ? vdev->queue->lock : vdev->lock;
 	int err = -EBUSY;
 
 	if (!(vdev->queue->io_modes & VB2_READ))
 		return -EINVAL;
-	if (lock && mutex_lock_interruptible(lock))
+	if (mutex_lock_interruptible(vdev->queue->lock))
 		return -ERESTARTSYS;
 	if (vb2_queue_is_busy(vdev, file))
 		goto exit;
@@ -900,8 +896,7 @@ ssize_t vb2_fop_read(struct file *file, char __user *buf,
 	if (vdev->queue->fileio)
 		vdev->queue->owner = file->private_data;
 exit:
-	if (lock)
-		mutex_unlock(lock);
+	mutex_unlock(vdev->queue->lock);
 	return err;
 }
 EXPORT_SYMBOL_GPL(vb2_fop_read);
@@ -910,17 +905,10 @@ __poll_t vb2_fop_poll(struct file *file, poll_table *wait)
 {
 	struct video_device *vdev = video_devdata(file);
 	struct vb2_queue *q = vdev->queue;
-	struct mutex *lock = q->lock ? q->lock : vdev->lock;
 	__poll_t res;
 	void *fileio;
 
-	/*
-	 * If this helper doesn't know how to lock, then you shouldn't be using
-	 * it but you should write your own.
-	 */
-	WARN_ON(!lock);
-
-	if (lock && mutex_lock_interruptible(lock))
+	if (mutex_lock_interruptible(q->lock))
 		return EPOLLERR;
 
 	fileio = q->fileio;
@@ -930,8 +918,7 @@ __poll_t vb2_fop_poll(struct file *file, poll_table *wait)
 	/* If fileio was started, then we have a new queue owner. */
 	if (!fileio && q->fileio)
 		q->owner = file->private_data;
-	if (lock)
-		mutex_unlock(lock);
+	mutex_unlock(q->lock);
 	return res;
 }
 EXPORT_SYMBOL_GPL(vb2_fop_poll);
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index f6818f732f34..d4e557b4f820 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -565,8 +565,6 @@ struct vb2_queue {
 	 * called. Used to check for unbalanced ops.
 	 */
 	u32				cnt_queue_setup;
-	u32				cnt_wait_prepare;
-	u32				cnt_wait_finish;
 	u32				cnt_start_streaming;
 	u32				cnt_stop_streaming;
 #endif
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v4 16/17] v4l2-ioctl.c: assume queue->lock is always set
  2018-06-15 19:07 [PATCH v4 00/17] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
                   ` (14 preceding siblings ...)
  2018-06-15 19:07 ` [PATCH v4 15/17] videobuf2: assume q->lock is always set Ezequiel Garcia
@ 2018-06-15 19:07 ` Ezequiel Garcia
  2018-06-15 19:07 ` [PATCH v4 17/17] media: Remove wait_{prepare, finish} Ezequiel Garcia
  2018-06-29 18:12 ` [PATCH v4 00/17] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
  17 siblings, 0 replies; 28+ messages in thread
From: Ezequiel Garcia @ 2018-06-15 19:07 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Hans Verkuil

From: Hans Verkuil <hansverk@cisco.com>

vb2_queue now expects a valid lock pointer, so drop the checks for
that in v4l2-ioctl.c.

Signed-off-by: Hans Verkuil <hansverk@cisco.com>
---
 drivers/media/v4l2-core/v4l2-ioctl.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index db835578e21f..c3da0f9a86e4 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -2715,12 +2715,11 @@ static struct mutex *v4l2_ioctl_get_lock(struct video_device *vdev,
 		struct v4l2_m2m_queue_ctx *ctx = is_output ?
 			&vfh->m2m_ctx->out_q_ctx : &vfh->m2m_ctx->cap_q_ctx;
 
-		if (ctx->q.lock)
-			return ctx->q.lock;
+		return ctx->q.lock;
 	}
 #endif
-	if (vdev->queue && vdev->queue->lock &&
-			(v4l2_ioctls[_IOC_NR(cmd)].flags & INFO_FL_QUEUE))
+	if (vdev->queue &&
+	    (v4l2_ioctls[_IOC_NR(cmd)].flags & INFO_FL_QUEUE))
 		return vdev->queue->lock;
 	return vdev->lock;
 }
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v4 17/17] media: Remove wait_{prepare, finish}
  2018-06-15 19:07 [PATCH v4 00/17] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
                   ` (15 preceding siblings ...)
  2018-06-15 19:07 ` [PATCH v4 16/17] v4l2-ioctl.c: assume queue->lock " Ezequiel Garcia
@ 2018-06-15 19:07 ` Ezequiel Garcia
  2018-06-29 18:12 ` [PATCH v4 00/17] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
  17 siblings, 0 replies; 28+ messages in thread
From: Ezequiel Garcia @ 2018-06-15 19:07 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Ezequiel Garcia

Now that all drivers provide a proper vb2_queue lock,
and that wait_{prepare, finish} are no longer in use,
get rid of them.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 Documentation/media/kapi/v4l2-dev.rst          |  7 +++----
 drivers/input/rmi4/rmi_f54.c                   |  2 --
 drivers/input/touchscreen/atmel_mxt_ts.c       |  2 --
 drivers/input/touchscreen/sur40.c              |  2 --
 .../media/common/videobuf2/videobuf2-v4l2.c    | 14 --------------
 drivers/media/dvb-core/dvb_vb2.c               |  2 --
 drivers/media/dvb-frontends/rtl2832_sdr.c      |  2 --
 drivers/media/i2c/video-i2c.c                  |  2 --
 drivers/media/pci/cobalt/cobalt-v4l2.c         |  2 --
 drivers/media/pci/cx23885/cx23885-417.c        |  2 --
 drivers/media/pci/cx23885/cx23885-dvb.c        |  2 --
 drivers/media/pci/cx23885/cx23885-vbi.c        |  2 --
 drivers/media/pci/cx23885/cx23885-video.c      |  2 --
 drivers/media/pci/cx25821/cx25821-video.c      |  2 --
 drivers/media/pci/cx88/cx88-blackbird.c        |  2 --
 drivers/media/pci/cx88/cx88-dvb.c              |  2 --
 drivers/media/pci/cx88/cx88-vbi.c              |  2 --
 drivers/media/pci/cx88/cx88-video.c            |  2 --
 drivers/media/pci/dt3155/dt3155.c              |  2 --
 drivers/media/pci/intel/ipu3/ipu3-cio2.c       |  2 --
 drivers/media/pci/saa7134/saa7134-empress.c    |  2 --
 drivers/media/pci/saa7134/saa7134-ts.c         |  2 --
 drivers/media/pci/saa7134/saa7134-vbi.c        |  2 --
 drivers/media/pci/saa7134/saa7134-video.c      |  2 --
 drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c |  2 --
 drivers/media/pci/solo6x10/solo6x10-v4l2.c     |  2 --
 drivers/media/pci/sta2x11/sta2x11_vip.c        |  2 --
 drivers/media/pci/tw5864/tw5864-video.c        |  2 --
 drivers/media/pci/tw68/tw68-video.c            |  2 --
 drivers/media/pci/tw686x/tw686x-video.c        |  2 --
 drivers/media/platform/am437x/am437x-vpfe.c    |  2 --
 drivers/media/platform/atmel/atmel-isc.c       |  2 --
 drivers/media/platform/atmel/atmel-isi.c       |  2 --
 drivers/media/platform/coda/coda-common.c      |  2 --
 drivers/media/platform/davinci/vpbe_display.c  |  2 --
 drivers/media/platform/davinci/vpif_capture.c  |  2 --
 drivers/media/platform/davinci/vpif_display.c  |  2 --
 drivers/media/platform/exynos-gsc/gsc-m2m.c    |  2 --
 .../media/platform/exynos4-is/fimc-capture.c   |  2 --
 .../media/platform/exynos4-is/fimc-isp-video.c |  2 --
 drivers/media/platform/exynos4-is/fimc-lite.c  |  2 --
 drivers/media/platform/exynos4-is/fimc-m2m.c   |  2 --
 drivers/media/platform/m2m-deinterlace.c       |  2 --
 .../media/platform/marvell-ccic/mcam-core.c    |  4 ----
 .../media/platform/mtk-jpeg/mtk_jpeg_core.c    |  2 --
 drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c   |  2 --
 .../media/platform/mtk-vcodec/mtk_vcodec_dec.c |  2 --
 .../media/platform/mtk-vcodec/mtk_vcodec_enc.c |  2 --
 drivers/media/platform/mx2_emmaprp.c           |  2 --
 drivers/media/platform/omap3isp/ispvideo.c     |  2 --
 drivers/media/platform/pxa_camera.c            |  2 --
 .../platform/qcom/camss-8x16/camss-video.c     |  2 --
 drivers/media/platform/qcom/venus/vdec.c       |  2 --
 drivers/media/platform/qcom/venus/venc.c       |  2 --
 drivers/media/platform/rcar-vin/rcar-dma.c     |  2 --
 drivers/media/platform/rcar_drif.c             |  2 --
 drivers/media/platform/rcar_fdp1.c             |  2 --
 drivers/media/platform/rcar_jpu.c              |  2 --
 drivers/media/platform/renesas-ceu.c           |  2 --
 drivers/media/platform/rockchip/rga/rga-buf.c  |  2 --
 .../media/platform/s3c-camif/camif-capture.c   |  2 --
 drivers/media/platform/s5p-g2d/g2d.c           |  2 --
 drivers/media/platform/s5p-jpeg/jpeg-core.c    |  2 --
 drivers/media/platform/s5p-mfc/s5p_mfc_dec.c   |  2 --
 drivers/media/platform/s5p-mfc/s5p_mfc_enc.c   |  2 --
 drivers/media/platform/sh_veu.c                |  2 --
 drivers/media/platform/sh_vou.c                |  2 --
 .../platform/soc_camera/sh_mobile_ceu_camera.c |  2 --
 drivers/media/platform/sti/bdisp/bdisp-v4l2.c  |  2 --
 drivers/media/platform/sti/delta/delta-v4l2.c  |  4 ----
 drivers/media/platform/sti/hva/hva-v4l2.c      |  2 --
 drivers/media/platform/stm32/stm32-dcmi.c      |  2 --
 drivers/media/platform/ti-vpe/cal.c            |  2 --
 drivers/media/platform/ti-vpe/vpe.c            |  2 --
 drivers/media/platform/vim2m.c                 |  2 --
 drivers/media/platform/vimc/vimc-capture.c     |  6 ------
 drivers/media/platform/vivid/vivid-sdr-cap.c   |  2 --
 drivers/media/platform/vivid/vivid-vbi-cap.c   |  2 --
 drivers/media/platform/vivid/vivid-vbi-out.c   |  2 --
 drivers/media/platform/vivid/vivid-vid-cap.c   |  2 --
 drivers/media/platform/vivid/vivid-vid-out.c   |  2 --
 drivers/media/platform/vsp1/vsp1_histo.c       |  2 --
 drivers/media/platform/vsp1/vsp1_video.c       |  2 --
 drivers/media/platform/xilinx/xilinx-dma.c     |  2 --
 drivers/media/usb/airspy/airspy.c              |  2 --
 drivers/media/usb/au0828/au0828-vbi.c          |  2 --
 drivers/media/usb/au0828/au0828-video.c        |  2 --
 drivers/media/usb/em28xx/em28xx-vbi.c          |  2 --
 drivers/media/usb/em28xx/em28xx-video.c        |  2 --
 drivers/media/usb/go7007/go7007-v4l2.c         |  2 --
 drivers/media/usb/gspca/gspca.c                |  2 --
 drivers/media/usb/hackrf/hackrf.c              |  2 --
 drivers/media/usb/msi2500/msi2500.c            |  2 --
 drivers/media/usb/pwc/pwc-if.c                 |  2 --
 drivers/media/usb/s2255/s2255drv.c             |  2 --
 drivers/media/usb/stk1160/stk1160-v4l.c        |  2 --
 drivers/media/usb/usbtv/usbtv-video.c          |  2 --
 drivers/media/usb/uvc/uvc_queue.c              |  4 ----
 .../staging/media/davinci_vpfe/vpfe_video.c    |  2 --
 drivers/staging/media/imx/imx-media-capture.c  |  2 --
 .../bcm2835-camera/bcm2835-camera.c            |  2 --
 drivers/usb/gadget/function/uvc_queue.c        |  2 --
 include/media/videobuf2-v4l2.h                 | 18 ------------------
 samples/v4l/v4l2-pci-skeleton.c                |  7 -------
 104 files changed, 3 insertions(+), 253 deletions(-)

diff --git a/Documentation/media/kapi/v4l2-dev.rst b/Documentation/media/kapi/v4l2-dev.rst
index eb03ccc41c41..ec25531308d3 100644
--- a/Documentation/media/kapi/v4l2-dev.rst
+++ b/Documentation/media/kapi/v4l2-dev.rst
@@ -163,10 +163,9 @@ waits in the code, then you should do the same to allow other
 processes to access the device node while the first process is waiting for
 something.
 
-In the case of :ref:`videobuf2 <vb2_framework>` you will need to implement the
-``wait_prepare()`` and ``wait_finish()`` callbacks to unlock/lock if applicable.
-If you use the ``queue->lock`` pointer, then you can use the helper functions
-:c:func:`vb2_ops_wait_prepare` and :c:func:`vb2_ops_wait_finish`.
+In the case of :ref:`videobuf2 <vb2_framework>` you are required to define a
+``queue->lock`` mutex, which is used to protect all the queue-specific
+ioctls.
 
 The implementation of a hotplug disconnect should also take the lock from
 :c:type:`video_device` before calling v4l2_device_disconnect. If you are also
diff --git a/drivers/input/rmi4/rmi_f54.c b/drivers/input/rmi4/rmi_f54.c
index 98a935efec8e..3f88e9f4610f 100644
--- a/drivers/input/rmi4/rmi_f54.c
+++ b/drivers/input/rmi4/rmi_f54.c
@@ -355,8 +355,6 @@ static void rmi_f54_buffer_queue(struct vb2_buffer *vb)
 static const struct vb2_ops rmi_f54_queue_ops = {
 	.queue_setup            = rmi_f54_queue_setup,
 	.buf_queue              = rmi_f54_buffer_queue,
-	.wait_prepare           = vb2_ops_wait_prepare,
-	.wait_finish            = vb2_ops_wait_finish,
 };
 
 static const struct vb2_queue rmi_f54_queue = {
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 54fe190fd4bc..499664ff754f 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -2368,8 +2368,6 @@ static void mxt_buffer_queue(struct vb2_buffer *vb)
 static const struct vb2_ops mxt_queue_ops = {
 	.queue_setup		= mxt_queue_setup,
 	.buf_queue		= mxt_buffer_queue,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 };
 
 static const struct vb2_queue mxt_queue = {
diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index 894843a7ec7b..8b22b742fff2 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -1102,8 +1102,6 @@ static const struct vb2_ops sur40_queue_ops = {
 	.buf_queue		= sur40_buffer_queue,
 	.start_streaming	= sur40_start_streaming,
 	.stop_streaming		= sur40_stop_streaming,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 };
 
 static const struct vb2_queue sur40_queue = {
diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index 7d2172468f72..d0d87483e5c3 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -934,20 +934,6 @@ unsigned long vb2_fop_get_unmapped_area(struct file *file, unsigned long addr,
 EXPORT_SYMBOL_GPL(vb2_fop_get_unmapped_area);
 #endif
 
-/* vb2_ops helpers. Only use if vq->lock is non-NULL. */
-
-void vb2_ops_wait_prepare(struct vb2_queue *vq)
-{
-	mutex_unlock(vq->lock);
-}
-EXPORT_SYMBOL_GPL(vb2_ops_wait_prepare);
-
-void vb2_ops_wait_finish(struct vb2_queue *vq)
-{
-	mutex_lock(vq->lock);
-}
-EXPORT_SYMBOL_GPL(vb2_ops_wait_finish);
-
 MODULE_DESCRIPTION("Driver helper framework for Video for Linux 2");
 MODULE_AUTHOR("Pawel Osciak <pawel@osciak.com>, Marek Szyprowski");
 MODULE_LICENSE("GPL");
diff --git a/drivers/media/dvb-core/dvb_vb2.c b/drivers/media/dvb-core/dvb_vb2.c
index cd3ea44f0ae9..b2bb238459c9 100644
--- a/drivers/media/dvb-core/dvb_vb2.c
+++ b/drivers/media/dvb-core/dvb_vb2.c
@@ -113,8 +113,6 @@ static const struct vb2_ops dvb_vb2_qops = {
 	.buf_queue		= _buffer_queue,
 	.start_streaming	= _start_streaming,
 	.stop_streaming		= _stop_streaming,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 };
 
 static void _fill_dmx_buffer(struct vb2_buffer *vb, void *pb)
diff --git a/drivers/media/dvb-frontends/rtl2832_sdr.c b/drivers/media/dvb-frontends/rtl2832_sdr.c
index c6e78d870ccd..b1836c7f3581 100644
--- a/drivers/media/dvb-frontends/rtl2832_sdr.c
+++ b/drivers/media/dvb-frontends/rtl2832_sdr.c
@@ -962,8 +962,6 @@ static const struct vb2_ops rtl2832_sdr_vb2_ops = {
 	.buf_queue              = rtl2832_sdr_buf_queue,
 	.start_streaming        = rtl2832_sdr_start_streaming,
 	.stop_streaming         = rtl2832_sdr_stop_streaming,
-	.wait_prepare           = vb2_ops_wait_prepare,
-	.wait_finish            = vb2_ops_wait_finish,
 };
 
 static int rtl2832_sdr_g_tuner(struct file *file, void *priv,
diff --git a/drivers/media/i2c/video-i2c.c b/drivers/media/i2c/video-i2c.c
index 0b347cc19aa5..a6fb7cc9219c 100644
--- a/drivers/media/i2c/video-i2c.c
+++ b/drivers/media/i2c/video-i2c.c
@@ -269,8 +269,6 @@ static struct vb2_ops video_i2c_video_qops = {
 	.buf_queue		= buffer_queue,
 	.start_streaming	= start_streaming,
 	.stop_streaming		= stop_streaming,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 };
 
 static int video_i2c_querycap(struct file *file, void  *priv,
diff --git a/drivers/media/pci/cobalt/cobalt-v4l2.c b/drivers/media/pci/cobalt/cobalt-v4l2.c
index e2a4c705d353..9415ae2ee694 100644
--- a/drivers/media/pci/cobalt/cobalt-v4l2.c
+++ b/drivers/media/pci/cobalt/cobalt-v4l2.c
@@ -428,8 +428,6 @@ static const struct vb2_ops cobalt_qops = {
 	.buf_queue = cobalt_buf_queue,
 	.start_streaming = cobalt_start_streaming,
 	.stop_streaming = cobalt_stop_streaming,
-	.wait_prepare = vb2_ops_wait_prepare,
-	.wait_finish = vb2_ops_wait_finish,
 };
 
 /* V4L2 ioctls */
diff --git a/drivers/media/pci/cx23885/cx23885-417.c b/drivers/media/pci/cx23885/cx23885-417.c
index a71f3c7569ce..84fff69e6df3 100644
--- a/drivers/media/pci/cx23885/cx23885-417.c
+++ b/drivers/media/pci/cx23885/cx23885-417.c
@@ -1219,8 +1219,6 @@ static const struct vb2_ops cx23885_qops = {
 	.buf_prepare  = buffer_prepare,
 	.buf_finish = buffer_finish,
 	.buf_queue    = buffer_queue,
-	.wait_prepare = vb2_ops_wait_prepare,
-	.wait_finish = vb2_ops_wait_finish,
 	.start_streaming = cx23885_start_streaming,
 	.stop_streaming = cx23885_stop_streaming,
 };
diff --git a/drivers/media/pci/cx23885/cx23885-dvb.c b/drivers/media/pci/cx23885/cx23885-dvb.c
index 7d52173073d6..e8f4c4150a0b 100644
--- a/drivers/media/pci/cx23885/cx23885-dvb.c
+++ b/drivers/media/pci/cx23885/cx23885-dvb.c
@@ -180,8 +180,6 @@ static const struct vb2_ops dvb_qops = {
 	.buf_prepare  = buffer_prepare,
 	.buf_finish = buffer_finish,
 	.buf_queue    = buffer_queue,
-	.wait_prepare = vb2_ops_wait_prepare,
-	.wait_finish = vb2_ops_wait_finish,
 	.start_streaming = cx23885_start_streaming,
 	.stop_streaming = cx23885_stop_streaming,
 };
diff --git a/drivers/media/pci/cx23885/cx23885-vbi.c b/drivers/media/pci/cx23885/cx23885-vbi.c
index 70f9f13bded3..cb766f042cb9 100644
--- a/drivers/media/pci/cx23885/cx23885-vbi.c
+++ b/drivers/media/pci/cx23885/cx23885-vbi.c
@@ -259,8 +259,6 @@ const struct vb2_ops cx23885_vbi_qops = {
 	.buf_prepare  = buffer_prepare,
 	.buf_finish = buffer_finish,
 	.buf_queue    = buffer_queue,
-	.wait_prepare = vb2_ops_wait_prepare,
-	.wait_finish = vb2_ops_wait_finish,
 	.start_streaming = cx23885_start_streaming,
 	.stop_streaming = cx23885_stop_streaming,
 };
diff --git a/drivers/media/pci/cx23885/cx23885-video.c b/drivers/media/pci/cx23885/cx23885-video.c
index f8a3deadc77a..4401ed1c81c4 100644
--- a/drivers/media/pci/cx23885/cx23885-video.c
+++ b/drivers/media/pci/cx23885/cx23885-video.c
@@ -525,8 +525,6 @@ static const struct vb2_ops cx23885_video_qops = {
 	.buf_prepare  = buffer_prepare,
 	.buf_finish = buffer_finish,
 	.buf_queue    = buffer_queue,
-	.wait_prepare = vb2_ops_wait_prepare,
-	.wait_finish = vb2_ops_wait_finish,
 	.start_streaming = cx23885_start_streaming,
 	.stop_streaming = cx23885_stop_streaming,
 };
diff --git a/drivers/media/pci/cx25821/cx25821-video.c b/drivers/media/pci/cx25821/cx25821-video.c
index dbaf42ec26cd..0256043487d8 100644
--- a/drivers/media/pci/cx25821/cx25821-video.c
+++ b/drivers/media/pci/cx25821/cx25821-video.c
@@ -308,8 +308,6 @@ static const struct vb2_ops cx25821_video_qops = {
 	.buf_prepare  = cx25821_buffer_prepare,
 	.buf_finish = cx25821_buffer_finish,
 	.buf_queue    = cx25821_buffer_queue,
-	.wait_prepare = vb2_ops_wait_prepare,
-	.wait_finish = vb2_ops_wait_finish,
 	.start_streaming = cx25821_start_streaming,
 	.stop_streaming = cx25821_stop_streaming,
 };
diff --git a/drivers/media/pci/cx88/cx88-blackbird.c b/drivers/media/pci/cx88/cx88-blackbird.c
index 7a4876cf9f08..0ccffecd0b51 100644
--- a/drivers/media/pci/cx88/cx88-blackbird.c
+++ b/drivers/media/pci/cx88/cx88-blackbird.c
@@ -789,8 +789,6 @@ static const struct vb2_ops blackbird_qops = {
 	.buf_prepare  = buffer_prepare,
 	.buf_finish = buffer_finish,
 	.buf_queue    = buffer_queue,
-	.wait_prepare = vb2_ops_wait_prepare,
-	.wait_finish = vb2_ops_wait_finish,
 	.start_streaming = start_streaming,
 	.stop_streaming = stop_streaming,
 };
diff --git a/drivers/media/pci/cx88/cx88-dvb.c b/drivers/media/pci/cx88/cx88-dvb.c
index ccfde28d4af2..b27587d119b6 100644
--- a/drivers/media/pci/cx88/cx88-dvb.c
+++ b/drivers/media/pci/cx88/cx88-dvb.c
@@ -160,8 +160,6 @@ static const struct vb2_ops dvb_qops = {
 	.buf_prepare  = buffer_prepare,
 	.buf_finish = buffer_finish,
 	.buf_queue    = buffer_queue,
-	.wait_prepare = vb2_ops_wait_prepare,
-	.wait_finish = vb2_ops_wait_finish,
 	.start_streaming = start_streaming,
 	.stop_streaming = stop_streaming,
 };
diff --git a/drivers/media/pci/cx88/cx88-vbi.c b/drivers/media/pci/cx88/cx88-vbi.c
index 58489ea0c1da..635fb719fa99 100644
--- a/drivers/media/pci/cx88/cx88-vbi.c
+++ b/drivers/media/pci/cx88/cx88-vbi.c
@@ -228,8 +228,6 @@ const struct vb2_ops cx8800_vbi_qops = {
 	.buf_prepare  = buffer_prepare,
 	.buf_finish = buffer_finish,
 	.buf_queue    = buffer_queue,
-	.wait_prepare = vb2_ops_wait_prepare,
-	.wait_finish = vb2_ops_wait_finish,
 	.start_streaming = start_streaming,
 	.stop_streaming = stop_streaming,
 };
diff --git a/drivers/media/pci/cx88/cx88-video.c b/drivers/media/pci/cx88/cx88-video.c
index 7b113bad70d2..a6acbeeb69c8 100644
--- a/drivers/media/pci/cx88/cx88-video.c
+++ b/drivers/media/pci/cx88/cx88-video.c
@@ -581,8 +581,6 @@ static const struct vb2_ops cx8800_video_qops = {
 	.buf_prepare  = buffer_prepare,
 	.buf_finish = buffer_finish,
 	.buf_queue    = buffer_queue,
-	.wait_prepare = vb2_ops_wait_prepare,
-	.wait_finish = vb2_ops_wait_finish,
 	.start_streaming = start_streaming,
 	.stop_streaming = stop_streaming,
 };
diff --git a/drivers/media/pci/dt3155/dt3155.c b/drivers/media/pci/dt3155/dt3155.c
index 1775c36891ae..cf2666d754b2 100644
--- a/drivers/media/pci/dt3155/dt3155.c
+++ b/drivers/media/pci/dt3155/dt3155.c
@@ -235,8 +235,6 @@ static void dt3155_buf_queue(struct vb2_buffer *vb)
 
 static const struct vb2_ops q_ops = {
 	.queue_setup = dt3155_queue_setup,
-	.wait_prepare = vb2_ops_wait_prepare,
-	.wait_finish = vb2_ops_wait_finish,
 	.buf_prepare = dt3155_buf_prepare,
 	.start_streaming = dt3155_start_streaming,
 	.stop_streaming = dt3155_stop_streaming,
diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
index 29027159eced..3fc3cbf2e2af 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
+++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
@@ -1055,8 +1055,6 @@ static const struct vb2_ops cio2_vb2_ops = {
 	.queue_setup = cio2_vb2_queue_setup,
 	.start_streaming = cio2_vb2_start_streaming,
 	.stop_streaming = cio2_vb2_stop_streaming,
-	.wait_prepare = vb2_ops_wait_prepare,
-	.wait_finish = vb2_ops_wait_finish,
 };
 
 /**************** V4L2 interface ****************/
diff --git a/drivers/media/pci/saa7134/saa7134-empress.c b/drivers/media/pci/saa7134/saa7134-empress.c
index 66acfd35ffc6..f01096a22b59 100644
--- a/drivers/media/pci/saa7134/saa7134-empress.c
+++ b/drivers/media/pci/saa7134/saa7134-empress.c
@@ -86,8 +86,6 @@ static const struct vb2_ops saa7134_empress_qops = {
 	.buf_init	= saa7134_ts_buffer_init,
 	.buf_prepare	= saa7134_ts_buffer_prepare,
 	.buf_queue	= saa7134_vb2_buffer_queue,
-	.wait_prepare	= vb2_ops_wait_prepare,
-	.wait_finish	= vb2_ops_wait_finish,
 	.start_streaming = start_streaming,
 	.stop_streaming = stop_streaming,
 };
diff --git a/drivers/media/pci/saa7134/saa7134-ts.c b/drivers/media/pci/saa7134/saa7134-ts.c
index 2be703617e29..bcf66d1b8ba0 100644
--- a/drivers/media/pci/saa7134/saa7134-ts.c
+++ b/drivers/media/pci/saa7134/saa7134-ts.c
@@ -175,8 +175,6 @@ struct vb2_ops saa7134_ts_qops = {
 	.buf_init	= saa7134_ts_buffer_init,
 	.buf_prepare	= saa7134_ts_buffer_prepare,
 	.buf_queue	= saa7134_vb2_buffer_queue,
-	.wait_prepare	= vb2_ops_wait_prepare,
-	.wait_finish	= vb2_ops_wait_finish,
 	.stop_streaming = saa7134_ts_stop_streaming,
 };
 EXPORT_SYMBOL_GPL(saa7134_ts_qops);
diff --git a/drivers/media/pci/saa7134/saa7134-vbi.c b/drivers/media/pci/saa7134/saa7134-vbi.c
index 57bea543c39b..f8e17370265b 100644
--- a/drivers/media/pci/saa7134/saa7134-vbi.c
+++ b/drivers/media/pci/saa7134/saa7134-vbi.c
@@ -170,8 +170,6 @@ const struct vb2_ops saa7134_vbi_qops = {
 	.buf_init	= buffer_init,
 	.buf_prepare	= buffer_prepare,
 	.buf_queue	= saa7134_vb2_buffer_queue,
-	.wait_prepare	= vb2_ops_wait_prepare,
-	.wait_finish	= vb2_ops_wait_finish,
 	.start_streaming = saa7134_vb2_start_streaming,
 	.stop_streaming = saa7134_vb2_stop_streaming,
 };
diff --git a/drivers/media/pci/saa7134/saa7134-video.c b/drivers/media/pci/saa7134/saa7134-video.c
index 1a50ec9d084f..57da6a1d5b4c 100644
--- a/drivers/media/pci/saa7134/saa7134-video.c
+++ b/drivers/media/pci/saa7134/saa7134-video.c
@@ -1055,8 +1055,6 @@ static const struct vb2_ops vb2_qops = {
 	.buf_init	= buffer_init,
 	.buf_prepare	= buffer_prepare,
 	.buf_queue	= saa7134_vb2_buffer_queue,
-	.wait_prepare	= vb2_ops_wait_prepare,
-	.wait_finish	= vb2_ops_wait_finish,
 	.start_streaming = saa7134_vb2_start_streaming,
 	.stop_streaming = saa7134_vb2_stop_streaming,
 };
diff --git a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
index 25f9f2ebff1d..e7f01d29092b 100644
--- a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
+++ b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
@@ -765,8 +765,6 @@ static const struct vb2_ops solo_enc_video_qops = {
 	.buf_finish	= solo_enc_buf_finish,
 	.start_streaming = solo_enc_start_streaming,
 	.stop_streaming = solo_enc_stop_streaming,
-	.wait_prepare	= vb2_ops_wait_prepare,
-	.wait_finish	= vb2_ops_wait_finish,
 };
 
 static int solo_enc_querycap(struct file *file, void  *priv,
diff --git a/drivers/media/pci/solo6x10/solo6x10-v4l2.c b/drivers/media/pci/solo6x10/solo6x10-v4l2.c
index 99ffd1ed4a73..7c172d793b1b 100644
--- a/drivers/media/pci/solo6x10/solo6x10-v4l2.c
+++ b/drivers/media/pci/solo6x10/solo6x10-v4l2.c
@@ -374,8 +374,6 @@ static const struct vb2_ops solo_video_qops = {
 	.buf_queue	= solo_buf_queue,
 	.start_streaming = solo_start_streaming,
 	.stop_streaming = solo_stop_streaming,
-	.wait_prepare	= vb2_ops_wait_prepare,
-	.wait_finish	= vb2_ops_wait_finish,
 };
 
 static int solo_querycap(struct file *file, void  *priv,
diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c b/drivers/media/pci/sta2x11/sta2x11_vip.c
index da2b2a2292d0..c5ddff6f6458 100644
--- a/drivers/media/pci/sta2x11/sta2x11_vip.c
+++ b/drivers/media/pci/sta2x11/sta2x11_vip.c
@@ -386,8 +386,6 @@ static const struct vb2_ops vip_video_qops = {
 	.buf_queue		= buffer_queue,
 	.start_streaming	= start_streaming,
 	.stop_streaming		= stop_streaming,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 };
 
 
diff --git a/drivers/media/pci/tw5864/tw5864-video.c b/drivers/media/pci/tw5864/tw5864-video.c
index ff2b7da90c08..49de5db67783 100644
--- a/drivers/media/pci/tw5864/tw5864-video.c
+++ b/drivers/media/pci/tw5864/tw5864-video.c
@@ -479,8 +479,6 @@ static const struct vb2_ops tw5864_video_qops = {
 	.buf_queue = tw5864_buf_queue,
 	.start_streaming = tw5864_start_streaming,
 	.stop_streaming = tw5864_stop_streaming,
-	.wait_prepare = vb2_ops_wait_prepare,
-	.wait_finish = vb2_ops_wait_finish,
 };
 
 static int tw5864_s_ctrl(struct v4l2_ctrl *ctrl)
diff --git a/drivers/media/pci/tw68/tw68-video.c b/drivers/media/pci/tw68/tw68-video.c
index 8c1f4a049764..2841604d0514 100644
--- a/drivers/media/pci/tw68/tw68-video.c
+++ b/drivers/media/pci/tw68/tw68-video.c
@@ -540,8 +540,6 @@ static const struct vb2_ops tw68_video_qops = {
 	.buf_finish	= tw68_buf_finish,
 	.start_streaming = tw68_start_streaming,
 	.stop_streaming = tw68_stop_streaming,
-	.wait_prepare	= vb2_ops_wait_prepare,
-	.wait_finish	= vb2_ops_wait_finish,
 };
 
 /* ------------------------------------------------------------------ */
diff --git a/drivers/media/pci/tw686x/tw686x-video.c b/drivers/media/pci/tw686x/tw686x-video.c
index 0ea8dd44026c..ed5730ab1d72 100644
--- a/drivers/media/pci/tw686x/tw686x-video.c
+++ b/drivers/media/pci/tw686x/tw686x-video.c
@@ -583,8 +583,6 @@ static const struct vb2_ops tw686x_video_qops = {
 	.buf_prepare		= tw686x_buf_prepare,
 	.start_streaming	= tw686x_start_streaming,
 	.stop_streaming		= tw686x_stop_streaming,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 };
 
 static int tw686x_s_ctrl(struct v4l2_ctrl *ctrl)
diff --git a/drivers/media/platform/am437x/am437x-vpfe.c b/drivers/media/platform/am437x/am437x-vpfe.c
index 58ebc2220d0e..a3926348baea 100644
--- a/drivers/media/platform/am437x/am437x-vpfe.c
+++ b/drivers/media/platform/am437x/am437x-vpfe.c
@@ -2230,8 +2230,6 @@ static long vpfe_ioctl_default(struct file *file, void *priv,
 }
 
 static const struct vb2_ops vpfe_video_qops = {
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 	.queue_setup		= vpfe_queue_setup,
 	.buf_prepare		= vpfe_buffer_prepare,
 	.buf_queue		= vpfe_buffer_queue,
diff --git a/drivers/media/platform/atmel/atmel-isc.c b/drivers/media/platform/atmel/atmel-isc.c
index d89e14524d42..be5aec085de0 100644
--- a/drivers/media/platform/atmel/atmel-isc.c
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -1225,8 +1225,6 @@ static void isc_buffer_queue(struct vb2_buffer *vb)
 
 static const struct vb2_ops isc_vb2_ops = {
 	.queue_setup		= isc_queue_setup,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 	.buf_prepare		= isc_buffer_prepare,
 	.start_streaming	= isc_start_streaming,
 	.stop_streaming		= isc_stop_streaming,
diff --git a/drivers/media/platform/atmel/atmel-isi.c b/drivers/media/platform/atmel/atmel-isi.c
index e5be21a31640..a6febaf7634b 100644
--- a/drivers/media/platform/atmel/atmel-isi.c
+++ b/drivers/media/platform/atmel/atmel-isi.c
@@ -527,8 +527,6 @@ static const struct vb2_ops isi_video_qops = {
 	.buf_queue		= buffer_queue,
 	.start_streaming	= start_streaming,
 	.stop_streaming		= stop_streaming,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 };
 
 static int isi_g_fmt_vid_cap(struct file *file, void *priv,
diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
index c7631e117dd3..08f618cc4cbe 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1664,8 +1664,6 @@ static const struct vb2_ops coda_qops = {
 	.buf_queue		= coda_buf_queue,
 	.start_streaming	= coda_start_streaming,
 	.stop_streaming		= coda_stop_streaming,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 };
 
 static int coda_s_ctrl(struct v4l2_ctrl *ctrl)
diff --git a/drivers/media/platform/davinci/vpbe_display.c b/drivers/media/platform/davinci/vpbe_display.c
index b0eb3d899eb4..974d735e5bb2 100644
--- a/drivers/media/platform/davinci/vpbe_display.c
+++ b/drivers/media/platform/davinci/vpbe_display.c
@@ -360,8 +360,6 @@ static void vpbe_stop_streaming(struct vb2_queue *vq)
 
 static const struct vb2_ops video_qops = {
 	.queue_setup = vpbe_buffer_queue_setup,
-	.wait_prepare = vb2_ops_wait_prepare,
-	.wait_finish = vb2_ops_wait_finish,
 	.buf_prepare = vpbe_buffer_prepare,
 	.start_streaming = vpbe_start_streaming,
 	.stop_streaming = vpbe_stop_streaming,
diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index 9364cdf62f54..e29e39de036b 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -318,8 +318,6 @@ static const struct vb2_ops video_qops = {
 	.start_streaming	= vpif_start_streaming,
 	.stop_streaming		= vpif_stop_streaming,
 	.buf_queue		= vpif_buffer_queue,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 };
 
 /**
diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c
index 7be636237acf..2aa32d31922e 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -292,8 +292,6 @@ static void vpif_stop_streaming(struct vb2_queue *vq)
 
 static const struct vb2_ops video_qops = {
 	.queue_setup		= vpif_buffer_queue_setup,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 	.buf_prepare		= vpif_buffer_prepare,
 	.start_streaming	= vpif_start_streaming,
 	.stop_streaming		= vpif_stop_streaming,
diff --git a/drivers/media/platform/exynos-gsc/gsc-m2m.c b/drivers/media/platform/exynos-gsc/gsc-m2m.c
index e9ff27949a91..673fb3710abd 100644
--- a/drivers/media/platform/exynos-gsc/gsc-m2m.c
+++ b/drivers/media/platform/exynos-gsc/gsc-m2m.c
@@ -282,8 +282,6 @@ static const struct vb2_ops gsc_m2m_qops = {
 	.queue_setup	 = gsc_m2m_queue_setup,
 	.buf_prepare	 = gsc_m2m_buf_prepare,
 	.buf_queue	 = gsc_m2m_buf_queue,
-	.wait_prepare	 = vb2_ops_wait_prepare,
-	.wait_finish	 = vb2_ops_wait_finish,
 	.stop_streaming	 = gsc_m2m_stop_streaming,
 	.start_streaming = gsc_m2m_start_streaming,
 };
diff --git a/drivers/media/platform/exynos4-is/fimc-capture.c b/drivers/media/platform/exynos4-is/fimc-capture.c
index a3cdac188190..8b0aa1eca9cb 100644
--- a/drivers/media/platform/exynos4-is/fimc-capture.c
+++ b/drivers/media/platform/exynos4-is/fimc-capture.c
@@ -457,8 +457,6 @@ static const struct vb2_ops fimc_capture_qops = {
 	.queue_setup		= queue_setup,
 	.buf_prepare		= buffer_prepare,
 	.buf_queue		= buffer_queue,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 	.start_streaming	= start_streaming,
 	.stop_streaming		= stop_streaming,
 };
diff --git a/drivers/media/platform/exynos4-is/fimc-isp-video.c b/drivers/media/platform/exynos4-is/fimc-isp-video.c
index 55ba696b8cf4..570c29d7f1a2 100644
--- a/drivers/media/platform/exynos4-is/fimc-isp-video.c
+++ b/drivers/media/platform/exynos4-is/fimc-isp-video.c
@@ -258,8 +258,6 @@ static const struct vb2_ops isp_video_capture_qops = {
 	.queue_setup	 = isp_video_capture_queue_setup,
 	.buf_prepare	 = isp_video_capture_buffer_prepare,
 	.buf_queue	 = isp_video_capture_buffer_queue,
-	.wait_prepare	 = vb2_ops_wait_prepare,
-	.wait_finish	 = vb2_ops_wait_finish,
 	.start_streaming = isp_video_capture_start_streaming,
 	.stop_streaming	 = isp_video_capture_stop_streaming,
 };
diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c b/drivers/media/platform/exynos4-is/fimc-lite.c
index 70d5f5586a5d..b00f36e93118 100644
--- a/drivers/media/platform/exynos4-is/fimc-lite.c
+++ b/drivers/media/platform/exynos4-is/fimc-lite.c
@@ -450,8 +450,6 @@ static const struct vb2_ops fimc_lite_qops = {
 	.queue_setup	 = queue_setup,
 	.buf_prepare	 = buffer_prepare,
 	.buf_queue	 = buffer_queue,
-	.wait_prepare	 = vb2_ops_wait_prepare,
-	.wait_finish	 = vb2_ops_wait_finish,
 	.start_streaming = start_streaming,
 	.stop_streaming	 = stop_streaming,
 };
diff --git a/drivers/media/platform/exynos4-is/fimc-m2m.c b/drivers/media/platform/exynos4-is/fimc-m2m.c
index a19f8b164a47..cc0289be3e18 100644
--- a/drivers/media/platform/exynos4-is/fimc-m2m.c
+++ b/drivers/media/platform/exynos4-is/fimc-m2m.c
@@ -223,8 +223,6 @@ static const struct vb2_ops fimc_qops = {
 	.queue_setup	 = fimc_queue_setup,
 	.buf_prepare	 = fimc_buf_prepare,
 	.buf_queue	 = fimc_buf_queue,
-	.wait_prepare	 = vb2_ops_wait_prepare,
-	.wait_finish	 = vb2_ops_wait_finish,
 	.stop_streaming	 = stop_streaming,
 	.start_streaming = start_streaming,
 };
diff --git a/drivers/media/platform/m2m-deinterlace.c b/drivers/media/platform/m2m-deinterlace.c
index 94dd8ec0f265..c264e4c92980 100644
--- a/drivers/media/platform/m2m-deinterlace.c
+++ b/drivers/media/platform/m2m-deinterlace.c
@@ -856,8 +856,6 @@ static const struct vb2_ops deinterlace_qops = {
 	.queue_setup	 = deinterlace_queue_setup,
 	.buf_prepare	 = deinterlace_buf_prepare,
 	.buf_queue	 = deinterlace_buf_queue,
-	.wait_prepare	 = vb2_ops_wait_prepare,
-	.wait_finish	 = vb2_ops_wait_finish,
 };
 
 static int queue_init(void *priv, struct vb2_queue *src_vq,
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c
index dfdbd4354b74..5932e417d0c9 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -1178,8 +1178,6 @@ static const struct vb2_ops mcam_vb2_ops = {
 	.buf_queue		= mcam_vb_buf_queue,
 	.start_streaming	= mcam_vb_start_streaming,
 	.stop_streaming		= mcam_vb_stop_streaming,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 };
 
 
@@ -1242,8 +1240,6 @@ static const struct vb2_ops mcam_vb2_sg_ops = {
 	.buf_cleanup		= mcam_vb_sg_buf_cleanup,
 	.start_streaming	= mcam_vb_start_streaming,
 	.stop_streaming		= mcam_vb_stop_streaming,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 };
 
 #endif /* MCAM_MODE_DMA_SG */
diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
index 328e8f650d9b..948a02ef88f4 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
@@ -760,8 +760,6 @@ static const struct vb2_ops mtk_jpeg_qops = {
 	.queue_setup        = mtk_jpeg_queue_setup,
 	.buf_prepare        = mtk_jpeg_buf_prepare,
 	.buf_queue          = mtk_jpeg_buf_queue,
-	.wait_prepare       = vb2_ops_wait_prepare,
-	.wait_finish        = vb2_ops_wait_finish,
 	.start_streaming    = mtk_jpeg_start_streaming,
 	.stop_streaming     = mtk_jpeg_stop_streaming,
 };
diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
index c2e2f5f1ebf1..4bee830b1071 100644
--- a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
+++ b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
@@ -613,8 +613,6 @@ static const struct vb2_ops mtk_mdp_m2m_qops = {
 	.buf_queue	 = mtk_mdp_m2m_buf_queue,
 	.stop_streaming	 = mtk_mdp_m2m_stop_streaming,
 	.start_streaming = mtk_mdp_m2m_start_streaming,
-	.wait_prepare	 = vb2_ops_wait_prepare,
-	.wait_finish	 = vb2_ops_wait_finish,
 };
 
 static int mtk_mdp_m2m_querycap(struct file *file, void *fh,
diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c
index 86f0a7134365..c7e709dff7dd 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c
@@ -1439,8 +1439,6 @@ static const struct vb2_ops mtk_vdec_vb2_ops = {
 	.queue_setup	= vb2ops_vdec_queue_setup,
 	.buf_prepare	= vb2ops_vdec_buf_prepare,
 	.buf_queue	= vb2ops_vdec_buf_queue,
-	.wait_prepare	= vb2_ops_wait_prepare,
-	.wait_finish	= vb2_ops_wait_finish,
 	.buf_init	= vb2ops_vdec_buf_init,
 	.buf_finish	= vb2ops_vdec_buf_finish,
 	.start_streaming	= vb2ops_vdec_start_streaming,
diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
index 1b1a28abbf1f..fe1b908161cd 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
@@ -927,8 +927,6 @@ static const struct vb2_ops mtk_venc_vb2_ops = {
 	.queue_setup		= vb2ops_venc_queue_setup,
 	.buf_prepare		= vb2ops_venc_buf_prepare,
 	.buf_queue		= vb2ops_venc_buf_queue,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 	.start_streaming	= vb2ops_venc_start_streaming,
 	.stop_streaming		= vb2ops_venc_stop_streaming,
 };
diff --git a/drivers/media/platform/mx2_emmaprp.c b/drivers/media/platform/mx2_emmaprp.c
index 7f9b356e7cc7..ce029cfdfe52 100644
--- a/drivers/media/platform/mx2_emmaprp.c
+++ b/drivers/media/platform/mx2_emmaprp.c
@@ -747,8 +747,6 @@ static const struct vb2_ops emmaprp_qops = {
 	.queue_setup	 = emmaprp_queue_setup,
 	.buf_prepare	 = emmaprp_buf_prepare,
 	.buf_queue	 = emmaprp_buf_queue,
-	.wait_prepare	 = vb2_ops_wait_prepare,
-	.wait_finish	 = vb2_ops_wait_finish,
 };
 
 static int queue_init(void *priv, struct vb2_queue *src_vq,
diff --git a/drivers/media/platform/omap3isp/ispvideo.c b/drivers/media/platform/omap3isp/ispvideo.c
index f835aeb9ddc5..5f13b6b465fc 100644
--- a/drivers/media/platform/omap3isp/ispvideo.c
+++ b/drivers/media/platform/omap3isp/ispvideo.c
@@ -496,8 +496,6 @@ static const struct vb2_ops isp_video_queue_ops = {
 	.buf_prepare = isp_video_buffer_prepare,
 	.buf_queue = isp_video_buffer_queue,
 	.start_streaming = isp_video_start_streaming,
-	.wait_prepare = vb2_ops_wait_prepare,
-	.wait_finish = vb2_ops_wait_finish,
 };
 
 /*
diff --git a/drivers/media/platform/pxa_camera.c b/drivers/media/platform/pxa_camera.c
index 4d5a26b4cdda..4ee02c2afdea 100644
--- a/drivers/media/platform/pxa_camera.c
+++ b/drivers/media/platform/pxa_camera.c
@@ -1570,8 +1570,6 @@ static const struct vb2_ops pxac_vb2_ops = {
 	.buf_cleanup		= pxac_vb2_cleanup,
 	.start_streaming	= pxac_vb2_start_streaming,
 	.stop_streaming		= pxac_vb2_stop_streaming,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 };
 
 static int pxa_camera_init_videobuf2(struct pxa_camera_dev *pcdev)
diff --git a/drivers/media/platform/qcom/camss-8x16/camss-video.c b/drivers/media/platform/qcom/camss-8x16/camss-video.c
index ffaa2849e0c1..4b40fa7b10bd 100644
--- a/drivers/media/platform/qcom/camss-8x16/camss-video.c
+++ b/drivers/media/platform/qcom/camss-8x16/camss-video.c
@@ -417,8 +417,6 @@ static void video_stop_streaming(struct vb2_queue *q)
 
 static const struct vb2_ops msm_video_vb2_q_ops = {
 	.queue_setup     = video_queue_setup,
-	.wait_prepare    = vb2_ops_wait_prepare,
-	.wait_finish     = vb2_ops_wait_finish,
 	.buf_init        = video_buf_init,
 	.buf_prepare     = video_buf_prepare,
 	.buf_queue       = video_buf_queue,
diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
index 41d14df46f5d..be1bd9b44138 100644
--- a/drivers/media/platform/qcom/venus/vdec.c
+++ b/drivers/media/platform/qcom/venus/vdec.c
@@ -783,8 +783,6 @@ static const struct vb2_ops vdec_vb2_ops = {
 	.start_streaming = vdec_start_streaming,
 	.stop_streaming = venus_helper_vb2_stop_streaming,
 	.buf_queue = venus_helper_vb2_buf_queue,
-	.wait_prepare = vb2_ops_wait_prepare,
-	.wait_finish = vb2_ops_wait_finish,
 };
 
 static void vdec_buf_done(struct venus_inst *inst, unsigned int buf_type,
diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
index 016af21abf5d..2084d3055160 100644
--- a/drivers/media/platform/qcom/venus/venc.c
+++ b/drivers/media/platform/qcom/venus/venc.c
@@ -976,8 +976,6 @@ static const struct vb2_ops venc_vb2_ops = {
 	.start_streaming = venc_start_streaming,
 	.stop_streaming = venus_helper_vb2_stop_streaming,
 	.buf_queue = venus_helper_vb2_buf_queue,
-	.wait_prepare = vb2_ops_wait_prepare,
-	.wait_finish = vb2_ops_wait_finish,
 };
 
 static void venc_buf_done(struct venus_inst *inst, unsigned int buf_type,
diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
index ac07f99e3516..d7d23620158d 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -1219,8 +1219,6 @@ static const struct vb2_ops rvin_qops = {
 	.buf_queue		= rvin_buffer_queue,
 	.start_streaming	= rvin_start_streaming,
 	.stop_streaming		= rvin_stop_streaming,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 };
 
 void rvin_dma_unregister(struct rvin_dev *vin)
diff --git a/drivers/media/platform/rcar_drif.c b/drivers/media/platform/rcar_drif.c
index dc7e280c91b4..fb719613a171 100644
--- a/drivers/media/platform/rcar_drif.c
+++ b/drivers/media/platform/rcar_drif.c
@@ -865,8 +865,6 @@ static const struct vb2_ops rcar_drif_vb2_ops = {
 	.buf_queue              = rcar_drif_buf_queue,
 	.start_streaming        = rcar_drif_start_streaming,
 	.stop_streaming         = rcar_drif_stop_streaming,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 };
 
 static int rcar_drif_querycap(struct file *file, void *fh,
diff --git a/drivers/media/platform/rcar_fdp1.c b/drivers/media/platform/rcar_fdp1.c
index b13dec3081e5..d2585e6b6364 100644
--- a/drivers/media/platform/rcar_fdp1.c
+++ b/drivers/media/platform/rcar_fdp1.c
@@ -2038,8 +2038,6 @@ static const struct vb2_ops fdp1_qops = {
 	.buf_queue	 = fdp1_buf_queue,
 	.start_streaming = fdp1_start_streaming,
 	.stop_streaming  = fdp1_stop_streaming,
-	.wait_prepare	 = vb2_ops_wait_prepare,
-	.wait_finish	 = vb2_ops_wait_finish,
 };
 
 static int queue_init(void *priv, struct vb2_queue *src_vq,
diff --git a/drivers/media/platform/rcar_jpu.c b/drivers/media/platform/rcar_jpu.c
index 8b44a849ab41..2ff194f70ed1 100644
--- a/drivers/media/platform/rcar_jpu.c
+++ b/drivers/media/platform/rcar_jpu.c
@@ -1190,8 +1190,6 @@ static const struct vb2_ops jpu_qops = {
 	.buf_finish		= jpu_buf_finish,
 	.start_streaming	= jpu_start_streaming,
 	.stop_streaming		= jpu_stop_streaming,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 };
 
 static int jpu_queue_init(void *priv, struct vb2_queue *src_vq,
diff --git a/drivers/media/platform/renesas-ceu.c b/drivers/media/platform/renesas-ceu.c
index fe4fe944592d..bf6bbeaceca6 100644
--- a/drivers/media/platform/renesas-ceu.c
+++ b/drivers/media/platform/renesas-ceu.c
@@ -753,8 +753,6 @@ static const struct vb2_ops ceu_vb2_ops = {
 	.queue_setup		= ceu_vb2_setup,
 	.buf_queue		= ceu_vb2_queue,
 	.buf_prepare		= ceu_vb2_prepare,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 	.start_streaming	= ceu_start_streaming,
 	.stop_streaming		= ceu_stop_streaming,
 };
diff --git a/drivers/media/platform/rockchip/rga/rga-buf.c b/drivers/media/platform/rockchip/rga/rga-buf.c
index fa1ba98c96dc..09b4c877021d 100644
--- a/drivers/media/platform/rockchip/rga/rga-buf.c
+++ b/drivers/media/platform/rockchip/rga/rga-buf.c
@@ -108,8 +108,6 @@ const struct vb2_ops rga_qops = {
 	.queue_setup = rga_queue_setup,
 	.buf_prepare = rga_buf_prepare,
 	.buf_queue = rga_buf_queue,
-	.wait_prepare = vb2_ops_wait_prepare,
-	.wait_finish = vb2_ops_wait_finish,
 	.start_streaming = rga_buf_start_streaming,
 	.stop_streaming = rga_buf_stop_streaming,
 };
diff --git a/drivers/media/platform/s3c-camif/camif-capture.c b/drivers/media/platform/s3c-camif/camif-capture.c
index 9ab8e7ee2e1e..ec767a650a59 100644
--- a/drivers/media/platform/s3c-camif/camif-capture.c
+++ b/drivers/media/platform/s3c-camif/camif-capture.c
@@ -526,8 +526,6 @@ static const struct vb2_ops s3c_camif_qops = {
 	.queue_setup	 = queue_setup,
 	.buf_prepare	 = buffer_prepare,
 	.buf_queue	 = buffer_queue,
-	.wait_prepare	 = vb2_ops_wait_prepare,
-	.wait_finish	 = vb2_ops_wait_finish,
 	.start_streaming = start_streaming,
 	.stop_streaming	 = stop_streaming,
 };
diff --git a/drivers/media/platform/s5p-g2d/g2d.c b/drivers/media/platform/s5p-g2d/g2d.c
index ce4280730835..66aa8cf1d048 100644
--- a/drivers/media/platform/s5p-g2d/g2d.c
+++ b/drivers/media/platform/s5p-g2d/g2d.c
@@ -142,8 +142,6 @@ static const struct vb2_ops g2d_qops = {
 	.queue_setup	= g2d_queue_setup,
 	.buf_prepare	= g2d_buf_prepare,
 	.buf_queue	= g2d_buf_queue,
-	.wait_prepare	= vb2_ops_wait_prepare,
-	.wait_finish	= vb2_ops_wait_finish,
 };
 
 static int queue_init(void *priv, struct vb2_queue *src_vq,
diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 79b63da27f53..69f85dc09366 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -2644,8 +2644,6 @@ static const struct vb2_ops s5p_jpeg_qops = {
 	.queue_setup		= s5p_jpeg_queue_setup,
 	.buf_prepare		= s5p_jpeg_buf_prepare,
 	.buf_queue		= s5p_jpeg_buf_queue,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 	.start_streaming	= s5p_jpeg_start_streaming,
 	.stop_streaming		= s5p_jpeg_stop_streaming,
 };
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
index 6a3cc4f86c5d..e872b71bb52d 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
@@ -1099,8 +1099,6 @@ static void s5p_mfc_buf_queue(struct vb2_buffer *vb)
 
 static struct vb2_ops s5p_mfc_dec_qops = {
 	.queue_setup		= s5p_mfc_queue_setup,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 	.buf_init		= s5p_mfc_buf_init,
 	.start_streaming	= s5p_mfc_start_streaming,
 	.stop_streaming		= s5p_mfc_stop_streaming,
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
index 570f391f2cfd..0b4dae3cc87d 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
@@ -2606,8 +2606,6 @@ static void s5p_mfc_buf_queue(struct vb2_buffer *vb)
 
 static struct vb2_ops s5p_mfc_enc_qops = {
 	.queue_setup		= s5p_mfc_queue_setup,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 	.buf_init		= s5p_mfc_buf_init,
 	.buf_prepare		= s5p_mfc_buf_prepare,
 	.start_streaming	= s5p_mfc_start_streaming,
diff --git a/drivers/media/platform/sh_veu.c b/drivers/media/platform/sh_veu.c
index 1a0cde017fdf..3dd550523033 100644
--- a/drivers/media/platform/sh_veu.c
+++ b/drivers/media/platform/sh_veu.c
@@ -925,8 +925,6 @@ static const struct vb2_ops sh_veu_qops = {
 	.queue_setup	 = sh_veu_queue_setup,
 	.buf_prepare	 = sh_veu_buf_prepare,
 	.buf_queue	 = sh_veu_buf_queue,
-	.wait_prepare	 = vb2_ops_wait_prepare,
-	.wait_finish	 = vb2_ops_wait_finish,
 };
 
 static int sh_veu_queue_init(void *priv, struct vb2_queue *src_vq,
diff --git a/drivers/media/platform/sh_vou.c b/drivers/media/platform/sh_vou.c
index 4dccf29e9d78..9da2c121a03a 100644
--- a/drivers/media/platform/sh_vou.c
+++ b/drivers/media/platform/sh_vou.c
@@ -369,8 +369,6 @@ static const struct vb2_ops sh_vou_qops = {
 	.buf_queue		= sh_vou_buf_queue,
 	.start_streaming	= sh_vou_start_streaming,
 	.stop_streaming		= sh_vou_stop_streaming,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 };
 
 /* Video IOCTLs */
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 242342fd7ede..ec7db9cfefaa 100644
--- a/drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c
+++ b/drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c
@@ -475,8 +475,6 @@ static const struct vb2_ops sh_mobile_ceu_videobuf_ops = {
 	.buf_queue	= sh_mobile_ceu_videobuf_queue,
 	.buf_cleanup	= sh_mobile_ceu_videobuf_release,
 	.buf_init	= sh_mobile_ceu_videobuf_init,
-	.wait_prepare	= vb2_ops_wait_prepare,
-	.wait_finish	= vb2_ops_wait_finish,
 	.stop_streaming	= sh_mobile_ceu_stop_streaming,
 };
 
diff --git a/drivers/media/platform/sti/bdisp/bdisp-v4l2.c b/drivers/media/platform/sti/bdisp/bdisp-v4l2.c
index 66b64096f5de..6e3bdae5ed2f 100644
--- a/drivers/media/platform/sti/bdisp/bdisp-v4l2.c
+++ b/drivers/media/platform/sti/bdisp/bdisp-v4l2.c
@@ -531,8 +531,6 @@ static const struct vb2_ops bdisp_qops = {
 	.queue_setup     = bdisp_queue_setup,
 	.buf_prepare     = bdisp_buf_prepare,
 	.buf_queue       = bdisp_buf_queue,
-	.wait_prepare    = vb2_ops_wait_prepare,
-	.wait_finish     = vb2_ops_wait_finish,
 	.stop_streaming  = bdisp_stop_streaming,
 	.start_streaming = bdisp_start_streaming,
 };
diff --git a/drivers/media/platform/sti/delta/delta-v4l2.c b/drivers/media/platform/sti/delta/delta-v4l2.c
index 232d508c5b66..a2686aef3296 100644
--- a/drivers/media/platform/sti/delta/delta-v4l2.c
+++ b/drivers/media/platform/sti/delta/delta-v4l2.c
@@ -1578,8 +1578,6 @@ static const struct vb2_ops delta_vb2_au_ops = {
 	.queue_setup = delta_vb2_au_queue_setup,
 	.buf_prepare = delta_vb2_au_prepare,
 	.buf_queue = delta_vb2_au_queue,
-	.wait_prepare = vb2_ops_wait_prepare,
-	.wait_finish = vb2_ops_wait_finish,
 	.start_streaming = delta_vb2_au_start_streaming,
 	.stop_streaming = delta_vb2_au_stop_streaming,
 };
@@ -1589,8 +1587,6 @@ static const struct vb2_ops delta_vb2_frame_ops = {
 	.buf_prepare = delta_vb2_frame_prepare,
 	.buf_finish = delta_vb2_frame_finish,
 	.buf_queue = delta_vb2_frame_queue,
-	.wait_prepare = vb2_ops_wait_prepare,
-	.wait_finish = vb2_ops_wait_finish,
 	.stop_streaming = delta_vb2_frame_stop_streaming,
 };
 
diff --git a/drivers/media/platform/sti/hva/hva-v4l2.c b/drivers/media/platform/sti/hva/hva-v4l2.c
index 15080cb00fa7..69e1514a8c34 100644
--- a/drivers/media/platform/sti/hva/hva-v4l2.c
+++ b/drivers/media/platform/sti/hva/hva-v4l2.c
@@ -1114,8 +1114,6 @@ static const struct vb2_ops hva_qops = {
 	.buf_queue		= hva_buf_queue,
 	.start_streaming	= hva_start_streaming,
 	.stop_streaming		= hva_stop_streaming,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 };
 
 /*
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c b/drivers/media/platform/stm32/stm32-dcmi.c
index 2e1933d872ee..f9909d7a8704 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -767,8 +767,6 @@ static const struct vb2_ops dcmi_video_qops = {
 	.buf_queue		= dcmi_buf_queue,
 	.start_streaming	= dcmi_start_streaming,
 	.stop_streaming		= dcmi_stop_streaming,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 };
 
 static int dcmi_g_fmt_vid_cap(struct file *file, void *priv,
diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
index d1febe5baa6d..13656060fcdb 100644
--- a/drivers/media/platform/ti-vpe/cal.c
+++ b/drivers/media/platform/ti-vpe/cal.c
@@ -1379,8 +1379,6 @@ static const struct vb2_ops cal_video_qops = {
 	.buf_queue		= cal_buffer_queue,
 	.start_streaming	= cal_start_streaming,
 	.stop_streaming		= cal_stop_streaming,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 };
 
 static const struct v4l2_file_operations cal_fops = {
diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c
index e395aa85c8ad..abe12bbe1f90 100644
--- a/drivers/media/platform/ti-vpe/vpe.c
+++ b/drivers/media/platform/ti-vpe/vpe.c
@@ -2198,8 +2198,6 @@ static const struct vb2_ops vpe_qops = {
 	.queue_setup	 = vpe_queue_setup,
 	.buf_prepare	 = vpe_buf_prepare,
 	.buf_queue	 = vpe_buf_queue,
-	.wait_prepare	 = vb2_ops_wait_prepare,
-	.wait_finish	 = vb2_ops_wait_finish,
 	.start_streaming = vpe_start_streaming,
 	.stop_streaming  = vpe_stop_streaming,
 };
diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c
index 065483e62db4..e64489c8106c 100644
--- a/drivers/media/platform/vim2m.c
+++ b/drivers/media/platform/vim2m.c
@@ -821,8 +821,6 @@ static const struct vb2_ops vim2m_qops = {
 	.buf_queue	 = vim2m_buf_queue,
 	.start_streaming = vim2m_start_streaming,
 	.stop_streaming  = vim2m_stop_streaming,
-	.wait_prepare	 = vb2_ops_wait_prepare,
-	.wait_finish	 = vb2_ops_wait_finish,
 };
 
 static int queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq)
diff --git a/drivers/media/platform/vimc/vimc-capture.c b/drivers/media/platform/vimc/vimc-capture.c
index 88a1e5670c72..cd889c50b248 100644
--- a/drivers/media/platform/vimc/vimc-capture.c
+++ b/drivers/media/platform/vimc/vimc-capture.c
@@ -328,12 +328,6 @@ static const struct vb2_ops vimc_cap_qops = {
 	.buf_queue		= vimc_cap_buf_queue,
 	.queue_setup		= vimc_cap_queue_setup,
 	.buf_prepare		= vimc_cap_buffer_prepare,
-	/*
-	 * Since q->lock is set we can use the standard
-	 * vb2_ops_wait_prepare/finish helper functions.
-	 */
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 };
 
 static const struct media_entity_operations vimc_cap_mops = {
diff --git a/drivers/media/platform/vivid/vivid-sdr-cap.c b/drivers/media/platform/vivid/vivid-sdr-cap.c
index cfb7cb4d37a8..053a3e036b33 100644
--- a/drivers/media/platform/vivid/vivid-sdr-cap.c
+++ b/drivers/media/platform/vivid/vivid-sdr-cap.c
@@ -309,8 +309,6 @@ const struct vb2_ops vivid_sdr_cap_qops = {
 	.buf_queue		= sdr_cap_buf_queue,
 	.start_streaming	= sdr_cap_start_streaming,
 	.stop_streaming		= sdr_cap_stop_streaming,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 };
 
 int vivid_sdr_enum_freq_bands(struct file *file, void *fh,
diff --git a/drivers/media/platform/vivid/vivid-vbi-cap.c b/drivers/media/platform/vivid/vivid-vbi-cap.c
index 92a852955173..d1d04e7316e6 100644
--- a/drivers/media/platform/vivid/vivid-vbi-cap.c
+++ b/drivers/media/platform/vivid/vivid-vbi-cap.c
@@ -226,8 +226,6 @@ const struct vb2_ops vivid_vbi_cap_qops = {
 	.buf_queue		= vbi_cap_buf_queue,
 	.start_streaming	= vbi_cap_start_streaming,
 	.stop_streaming		= vbi_cap_stop_streaming,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 };
 
 int vidioc_g_fmt_vbi_cap(struct file *file, void *priv,
diff --git a/drivers/media/platform/vivid/vivid-vbi-out.c b/drivers/media/platform/vivid/vivid-vbi-out.c
index 69486c130a7e..98b5a966faff 100644
--- a/drivers/media/platform/vivid/vivid-vbi-out.c
+++ b/drivers/media/platform/vivid/vivid-vbi-out.c
@@ -121,8 +121,6 @@ const struct vb2_ops vivid_vbi_out_qops = {
 	.buf_queue		= vbi_out_buf_queue,
 	.start_streaming	= vbi_out_start_streaming,
 	.stop_streaming		= vbi_out_stop_streaming,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 };
 
 int vidioc_g_fmt_vbi_out(struct file *file, void *priv,
diff --git a/drivers/media/platform/vivid/vivid-vid-cap.c b/drivers/media/platform/vivid/vivid-vid-cap.c
index 1599159f2574..2dba4bcad5c8 100644
--- a/drivers/media/platform/vivid/vivid-vid-cap.c
+++ b/drivers/media/platform/vivid/vivid-vid-cap.c
@@ -264,8 +264,6 @@ const struct vb2_ops vivid_vid_cap_qops = {
 	.buf_queue		= vid_cap_buf_queue,
 	.start_streaming	= vid_cap_start_streaming,
 	.stop_streaming		= vid_cap_stop_streaming,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 };
 
 /*
diff --git a/drivers/media/platform/vivid/vivid-vid-out.c b/drivers/media/platform/vivid/vivid-vid-out.c
index 51fec66d8d45..f7c6112fb89c 100644
--- a/drivers/media/platform/vivid/vivid-vid-out.c
+++ b/drivers/media/platform/vivid/vivid-vid-out.c
@@ -185,8 +185,6 @@ const struct vb2_ops vivid_vid_out_qops = {
 	.buf_queue		= vid_out_buf_queue,
 	.start_streaming	= vid_out_start_streaming,
 	.stop_streaming		= vid_out_stop_streaming,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 };
 
 /*
diff --git a/drivers/media/platform/vsp1/vsp1_histo.c b/drivers/media/platform/vsp1/vsp1_histo.c
index 5e15c8ff88d9..7a9c574c6c4a 100644
--- a/drivers/media/platform/vsp1/vsp1_histo.c
+++ b/drivers/media/platform/vsp1/vsp1_histo.c
@@ -159,8 +159,6 @@ static const struct vb2_ops histo_video_queue_qops = {
 	.queue_setup = histo_queue_setup,
 	.buf_prepare = histo_buffer_prepare,
 	.buf_queue = histo_buffer_queue,
-	.wait_prepare = vb2_ops_wait_prepare,
-	.wait_finish = vb2_ops_wait_finish,
 	.start_streaming = histo_start_streaming,
 	.stop_streaming = histo_stop_streaming,
 };
diff --git a/drivers/media/platform/vsp1/vsp1_video.c b/drivers/media/platform/vsp1/vsp1_video.c
index 81d47a09d7bc..85903e46c1ab 100644
--- a/drivers/media/platform/vsp1/vsp1_video.c
+++ b/drivers/media/platform/vsp1/vsp1_video.c
@@ -949,8 +949,6 @@ static const struct vb2_ops vsp1_video_queue_qops = {
 	.queue_setup = vsp1_video_queue_setup,
 	.buf_prepare = vsp1_video_buffer_prepare,
 	.buf_queue = vsp1_video_buffer_queue,
-	.wait_prepare = vb2_ops_wait_prepare,
-	.wait_finish = vb2_ops_wait_finish,
 	.start_streaming = vsp1_video_start_streaming,
 	.stop_streaming = vsp1_video_stop_streaming,
 };
diff --git a/drivers/media/platform/xilinx/xilinx-dma.c b/drivers/media/platform/xilinx/xilinx-dma.c
index d041f94be832..1666412d9ff8 100644
--- a/drivers/media/platform/xilinx/xilinx-dma.c
+++ b/drivers/media/platform/xilinx/xilinx-dma.c
@@ -478,8 +478,6 @@ static const struct vb2_ops xvip_dma_queue_qops = {
 	.queue_setup = xvip_dma_queue_setup,
 	.buf_prepare = xvip_dma_buffer_prepare,
 	.buf_queue = xvip_dma_buffer_queue,
-	.wait_prepare = vb2_ops_wait_prepare,
-	.wait_finish = vb2_ops_wait_finish,
 	.start_streaming = xvip_dma_start_streaming,
 	.stop_streaming = xvip_dma_stop_streaming,
 };
diff --git a/drivers/media/usb/airspy/airspy.c b/drivers/media/usb/airspy/airspy.c
index e70c9e2f3798..97f01db5dcbb 100644
--- a/drivers/media/usb/airspy/airspy.c
+++ b/drivers/media/usb/airspy/airspy.c
@@ -610,8 +610,6 @@ static const struct vb2_ops airspy_vb2_ops = {
 	.buf_queue              = airspy_buf_queue,
 	.start_streaming        = airspy_start_streaming,
 	.stop_streaming         = airspy_stop_streaming,
-	.wait_prepare           = vb2_ops_wait_prepare,
-	.wait_finish            = vb2_ops_wait_finish,
 };
 
 static int airspy_querycap(struct file *file, void *fh,
diff --git a/drivers/media/usb/au0828/au0828-vbi.c b/drivers/media/usb/au0828/au0828-vbi.c
index 9dd6bdb7304f..e09fcbcea290 100644
--- a/drivers/media/usb/au0828/au0828-vbi.c
+++ b/drivers/media/usb/au0828/au0828-vbi.c
@@ -85,6 +85,4 @@ const struct vb2_ops au0828_vbi_qops = {
 	.buf_queue       = vbi_buffer_queue,
 	.start_streaming = au0828_start_analog_streaming,
 	.stop_streaming  = au0828_stop_vbi_streaming,
-	.wait_prepare    = vb2_ops_wait_prepare,
-	.wait_finish     = vb2_ops_wait_finish,
 };
diff --git a/drivers/media/usb/au0828/au0828-video.c b/drivers/media/usb/au0828/au0828-video.c
index 964cd7bcdd2c..f4469efaaa63 100644
--- a/drivers/media/usb/au0828/au0828-video.c
+++ b/drivers/media/usb/au0828/au0828-video.c
@@ -921,8 +921,6 @@ static const struct vb2_ops au0828_video_qops = {
 	.buf_queue       = buffer_queue,
 	.start_streaming = au0828_start_analog_streaming,
 	.stop_streaming  = au0828_stop_streaming,
-	.wait_prepare    = vb2_ops_wait_prepare,
-	.wait_finish     = vb2_ops_wait_finish,
 };
 
 /* ------------------------------------------------------------------
diff --git a/drivers/media/usb/em28xx/em28xx-vbi.c b/drivers/media/usb/em28xx/em28xx-vbi.c
index 63c48361d3f2..847ae9627bd0 100644
--- a/drivers/media/usb/em28xx/em28xx-vbi.c
+++ b/drivers/media/usb/em28xx/em28xx-vbi.c
@@ -94,6 +94,4 @@ const struct vb2_ops em28xx_vbi_qops = {
 	.buf_queue      = vbi_buffer_queue,
 	.start_streaming = em28xx_start_analog_streaming,
 	.stop_streaming = em28xx_stop_vbi_streaming,
-	.wait_prepare   = vb2_ops_wait_prepare,
-	.wait_finish    = vb2_ops_wait_finish,
 };
diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 68571bf36d28..9b588daaf646 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -1244,8 +1244,6 @@ static const struct vb2_ops em28xx_video_qops = {
 	.buf_queue      = buffer_queue,
 	.start_streaming = em28xx_start_analog_streaming,
 	.stop_streaming = em28xx_stop_streaming,
-	.wait_prepare   = vb2_ops_wait_prepare,
-	.wait_finish    = vb2_ops_wait_finish,
 };
 
 static int em28xx_vb2_setup(struct em28xx *dev)
diff --git a/drivers/media/usb/go7007/go7007-v4l2.c b/drivers/media/usb/go7007/go7007-v4l2.c
index c55c82f70e54..21d32838f54e 100644
--- a/drivers/media/usb/go7007/go7007-v4l2.c
+++ b/drivers/media/usb/go7007/go7007-v4l2.c
@@ -484,8 +484,6 @@ static const struct vb2_ops go7007_video_qops = {
 	.buf_finish     = go7007_buf_finish,
 	.start_streaming = go7007_start_streaming,
 	.stop_streaming = go7007_stop_streaming,
-	.wait_prepare   = vb2_ops_wait_prepare,
-	.wait_finish    = vb2_ops_wait_finish,
 };
 
 static int vidioc_g_parm(struct file *filp, void *priv,
diff --git a/drivers/media/usb/gspca/gspca.c b/drivers/media/usb/gspca/gspca.c
index 57aa521e16b1..f823dba94580 100644
--- a/drivers/media/usb/gspca/gspca.c
+++ b/drivers/media/usb/gspca/gspca.c
@@ -1385,8 +1385,6 @@ static const struct vb2_ops gspca_qops = {
 	.buf_queue		= gspca_buffer_queue,
 	.start_streaming	= gspca_start_streaming,
 	.stop_streaming		= gspca_stop_streaming,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 };
 
 static const struct v4l2_file_operations dev_fops = {
diff --git a/drivers/media/usb/hackrf/hackrf.c b/drivers/media/usb/hackrf/hackrf.c
index 6d692fb3e8dd..1125da75f4dc 100644
--- a/drivers/media/usb/hackrf/hackrf.c
+++ b/drivers/media/usb/hackrf/hackrf.c
@@ -896,8 +896,6 @@ static const struct vb2_ops hackrf_vb2_ops = {
 	.buf_queue              = hackrf_buf_queue,
 	.start_streaming        = hackrf_start_streaming,
 	.stop_streaming         = hackrf_stop_streaming,
-	.wait_prepare           = vb2_ops_wait_prepare,
-	.wait_finish            = vb2_ops_wait_finish,
 };
 
 static int hackrf_querycap(struct file *file, void *fh,
diff --git a/drivers/media/usb/msi2500/msi2500.c b/drivers/media/usb/msi2500/msi2500.c
index 65ef755adfdc..9bf2e6dad624 100644
--- a/drivers/media/usb/msi2500/msi2500.c
+++ b/drivers/media/usb/msi2500/msi2500.c
@@ -902,8 +902,6 @@ static const struct vb2_ops msi2500_vb2_ops = {
 	.buf_queue              = msi2500_buf_queue,
 	.start_streaming        = msi2500_start_streaming,
 	.stop_streaming         = msi2500_stop_streaming,
-	.wait_prepare           = vb2_ops_wait_prepare,
-	.wait_finish            = vb2_ops_wait_finish,
 };
 
 static int msi2500_enum_fmt_sdr_cap(struct file *file, void *priv,
diff --git a/drivers/media/usb/pwc/pwc-if.c b/drivers/media/usb/pwc/pwc-if.c
index 54b036d39c5b..6d9be76a3380 100644
--- a/drivers/media/usb/pwc/pwc-if.c
+++ b/drivers/media/usb/pwc/pwc-if.c
@@ -717,8 +717,6 @@ static const struct vb2_ops pwc_vb_queue_ops = {
 	.buf_queue		= buffer_queue,
 	.start_streaming	= start_streaming,
 	.stop_streaming		= stop_streaming,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 };
 
 /***************************************************************************/
diff --git a/drivers/media/usb/s2255/s2255drv.c b/drivers/media/usb/s2255/s2255drv.c
index 82927eb334c4..52f5ef3fb431 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -720,8 +720,6 @@ static const struct vb2_ops s2255_video_qops = {
 	.buf_queue = buffer_queue,
 	.start_streaming = start_streaming,
 	.stop_streaming = stop_streaming,
-	.wait_prepare = vb2_ops_wait_prepare,
-	.wait_finish = vb2_ops_wait_finish,
 };
 
 static int vidioc_querycap(struct file *file, void *priv,
diff --git a/drivers/media/usb/stk1160/stk1160-v4l.c b/drivers/media/usb/stk1160/stk1160-v4l.c
index 504e413edcd2..eb010c88205e 100644
--- a/drivers/media/usb/stk1160/stk1160-v4l.c
+++ b/drivers/media/usb/stk1160/stk1160-v4l.c
@@ -747,8 +747,6 @@ static const struct vb2_ops stk1160_video_qops = {
 	.buf_queue		= buffer_queue,
 	.start_streaming	= start_streaming,
 	.stop_streaming		= stop_streaming,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 };
 
 static const struct video_device v4l_template = {
diff --git a/drivers/media/usb/usbtv/usbtv-video.c b/drivers/media/usb/usbtv/usbtv-video.c
index ce79df643c7e..c2f8e50228ac 100644
--- a/drivers/media/usb/usbtv/usbtv-video.c
+++ b/drivers/media/usb/usbtv/usbtv-video.c
@@ -781,8 +781,6 @@ static const struct vb2_ops usbtv_vb2_ops = {
 	.buf_queue = usbtv_buf_queue,
 	.start_streaming = usbtv_start_streaming,
 	.stop_streaming = usbtv_stop_streaming,
-	.wait_prepare = vb2_ops_wait_prepare,
-	.wait_finish = vb2_ops_wait_finish,
 };
 
 static int usbtv_s_ctrl(struct v4l2_ctrl *ctrl)
diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c
index fecccb5e7628..c6a14f7af4e2 100644
--- a/drivers/media/usb/uvc/uvc_queue.c
+++ b/drivers/media/usb/uvc/uvc_queue.c
@@ -203,8 +203,6 @@ static const struct vb2_ops uvc_queue_qops = {
 	.buf_prepare = uvc_buffer_prepare,
 	.buf_queue = uvc_buffer_queue,
 	.buf_finish = uvc_buffer_finish,
-	.wait_prepare = vb2_ops_wait_prepare,
-	.wait_finish = vb2_ops_wait_finish,
 	.start_streaming = uvc_start_streaming,
 	.stop_streaming = uvc_stop_streaming,
 };
@@ -213,8 +211,6 @@ static const struct vb2_ops uvc_meta_queue_qops = {
 	.queue_setup = uvc_queue_setup,
 	.buf_prepare = uvc_buffer_prepare,
 	.buf_queue = uvc_buffer_queue,
-	.wait_prepare = vb2_ops_wait_prepare,
-	.wait_finish = vb2_ops_wait_finish,
 	.stop_streaming = uvc_stop_streaming,
 };
 
diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.c b/drivers/staging/media/davinci_vpfe/vpfe_video.c
index 1269a983455e..3b470e40988f 100644
--- a/drivers/staging/media/davinci_vpfe/vpfe_video.c
+++ b/drivers/staging/media/davinci_vpfe/vpfe_video.c
@@ -1312,8 +1312,6 @@ static const struct vb2_ops video_qops = {
 	.stop_streaming		= vpfe_stop_streaming,
 	.buf_cleanup		= vpfe_buf_cleanup,
 	.buf_queue		= vpfe_buffer_queue,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 };
 
 /*
diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c
index 4e3fdf8aeef5..d66a1f4236b3 100644
--- a/drivers/staging/media/imx/imx-media-capture.c
+++ b/drivers/staging/media/imx/imx-media-capture.c
@@ -503,8 +503,6 @@ static const struct vb2_ops capture_qops = {
 	.buf_init        = capture_buf_init,
 	.buf_prepare	 = capture_buf_prepare,
 	.buf_queue	 = capture_buf_queue,
-	.wait_prepare	 = vb2_ops_wait_prepare,
-	.wait_finish	 = vb2_ops_wait_finish,
 	.start_streaming = capture_start_streaming,
 	.stop_streaming  = capture_stop_streaming,
 };
diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index 6dd0c838db05..b549ff154b03 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -636,8 +636,6 @@ static const struct vb2_ops bm2835_mmal_video_qops = {
 	.buf_queue = buffer_queue,
 	.start_streaming = start_streaming,
 	.stop_streaming = stop_streaming,
-	.wait_prepare = vb2_ops_wait_prepare,
-	.wait_finish = vb2_ops_wait_finish,
 };
 
 /* ------------------------------------------------------------------
diff --git a/drivers/usb/gadget/function/uvc_queue.c b/drivers/usb/gadget/function/uvc_queue.c
index 9e33d5206d54..f1090107d0ec 100644
--- a/drivers/usb/gadget/function/uvc_queue.c
+++ b/drivers/usb/gadget/function/uvc_queue.c
@@ -106,8 +106,6 @@ static struct vb2_ops uvc_queue_qops = {
 	.queue_setup = uvc_queue_setup,
 	.buf_prepare = uvc_buffer_prepare,
 	.buf_queue = uvc_buffer_queue,
-	.wait_prepare = vb2_ops_wait_prepare,
-	.wait_finish = vb2_ops_wait_finish,
 };
 
 int uvcg_queue_init(struct uvc_video_queue *queue, enum v4l2_buf_type type,
diff --git a/include/media/videobuf2-v4l2.h b/include/media/videobuf2-v4l2.h
index 3d5e2d739f05..cf83b01dc44e 100644
--- a/include/media/videobuf2-v4l2.h
+++ b/include/media/videobuf2-v4l2.h
@@ -273,22 +273,4 @@ unsigned long vb2_fop_get_unmapped_area(struct file *file, unsigned long addr,
 		unsigned long len, unsigned long pgoff, unsigned long flags);
 #endif
 
-/**
- * vb2_ops_wait_prepare - helper function to lock a struct &vb2_queue
- *
- * @vq: pointer to &struct vb2_queue
- *
- * ..note:: only use if vq->lock is non-NULL.
- */
-void vb2_ops_wait_prepare(struct vb2_queue *vq);
-
-/**
- * vb2_ops_wait_finish - helper function to unlock a struct &vb2_queue
- *
- * @vq: pointer to &struct vb2_queue
- *
- * ..note:: only use if vq->lock is non-NULL.
- */
-void vb2_ops_wait_finish(struct vb2_queue *vq);
-
 #endif /* _MEDIA_VIDEOBUF2_V4L2_H */
diff --git a/samples/v4l/v4l2-pci-skeleton.c b/samples/v4l/v4l2-pci-skeleton.c
index f520e3aef9c6..74f9c817c8bd 100644
--- a/samples/v4l/v4l2-pci-skeleton.c
+++ b/samples/v4l/v4l2-pci-skeleton.c
@@ -277,19 +277,12 @@ static void stop_streaming(struct vb2_queue *vq)
 	return_all_buffers(skel, VB2_BUF_STATE_ERROR);
 }
 
-/*
- * The vb2 queue ops. Note that since q->lock is set we can use the standard
- * vb2_ops_wait_prepare/finish helper functions. If q->lock would be NULL,
- * then this driver would have to provide these ops.
- */
 static const struct vb2_ops skel_qops = {
 	.queue_setup		= queue_setup,
 	.buf_prepare		= buffer_prepare,
 	.buf_queue		= buffer_queue,
 	.start_streaming	= start_streaming,
 	.stop_streaming		= stop_streaming,
-	.wait_prepare		= vb2_ops_wait_prepare,
-	.wait_finish		= vb2_ops_wait_finish,
 };
 
 /*
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH v4 04/17] omap3isp: Add vb2_queue lock
  2018-06-15 19:07 ` [PATCH v4 04/17] omap3isp: " Ezequiel Garcia
@ 2018-06-18  7:31   ` Hans Verkuil
  2018-06-20 17:42     ` [PATCH v5 " Ezequiel Garcia
  0 siblings, 1 reply; 28+ messages in thread
From: Hans Verkuil @ 2018-06-18  7:31 UTC (permalink / raw)
  To: Ezequiel Garcia, linux-media; +Cc: Hans Verkuil, kernel

On 06/15/2018 09:07 PM, Ezequiel Garcia wrote:
> vb2_queue locks is now mandatory. Add it, remove driver ad-hoc locks,
> and implement wait_{prepare, finish}.

You are also removing stream_lock, but that is not mentioned here.

This needs an improved commit log.

Regards,

	Hans

> 
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
>  drivers/media/platform/omap3isp/ispvideo.c | 65 ++++------------------
>  drivers/media/platform/omap3isp/ispvideo.h |  1 -
>  2 files changed, 11 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/media/platform/omap3isp/ispvideo.c b/drivers/media/platform/omap3isp/ispvideo.c
> index 9d228eac24ea..f835aeb9ddc5 100644
> --- a/drivers/media/platform/omap3isp/ispvideo.c
> +++ b/drivers/media/platform/omap3isp/ispvideo.c
> @@ -496,6 +496,8 @@ static const struct vb2_ops isp_video_queue_ops = {
>  	.buf_prepare = isp_video_buffer_prepare,
>  	.buf_queue = isp_video_buffer_queue,
>  	.start_streaming = isp_video_start_streaming,
> +	.wait_prepare = vb2_ops_wait_prepare,
> +	.wait_finish = vb2_ops_wait_finish,
>  };
>  
>  /*
> @@ -628,11 +630,8 @@ void omap3isp_video_resume(struct isp_video *video, int continuous)
>  {
>  	struct isp_buffer *buf = NULL;
>  
> -	if (continuous && video->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
> -		mutex_lock(&video->queue_lock);
> +	if (continuous && video->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
>  		vb2_discard_done(video->queue);
> -		mutex_unlock(&video->queue_lock);
> -	}
>  
>  	if (!list_empty(&video->dmaqueue)) {
>  		buf = list_first_entry(&video->dmaqueue,
> @@ -909,13 +908,8 @@ isp_video_reqbufs(struct file *file, void *fh, struct v4l2_requestbuffers *rb)
>  {
>  	struct isp_video_fh *vfh = to_isp_video_fh(fh);
>  	struct isp_video *video = video_drvdata(file);
> -	int ret;
> -
> -	mutex_lock(&video->queue_lock);
> -	ret = vb2_reqbufs(&vfh->queue, rb);
> -	mutex_unlock(&video->queue_lock);
>  
> -	return ret;
> +	return vb2_reqbufs(&vfh->queue, rb);
>  }
>  
>  static int
> @@ -923,13 +917,8 @@ isp_video_querybuf(struct file *file, void *fh, struct v4l2_buffer *b)
>  {
>  	struct isp_video_fh *vfh = to_isp_video_fh(fh);
>  	struct isp_video *video = video_drvdata(file);
> -	int ret;
>  
> -	mutex_lock(&video->queue_lock);
> -	ret = vb2_querybuf(&vfh->queue, b);
> -	mutex_unlock(&video->queue_lock);
> -
> -	return ret;
> +	return vb2_querybuf(&vfh->queue, b);
>  }
>  
>  static int
> @@ -937,13 +926,8 @@ isp_video_qbuf(struct file *file, void *fh, struct v4l2_buffer *b)
>  {
>  	struct isp_video_fh *vfh = to_isp_video_fh(fh);
>  	struct isp_video *video = video_drvdata(file);
> -	int ret;
>  
> -	mutex_lock(&video->queue_lock);
> -	ret = vb2_qbuf(&vfh->queue, b);
> -	mutex_unlock(&video->queue_lock);
> -
> -	return ret;
> +	return vb2_qbuf(&vfh->queue, b);
>  }
>  
>  static int
> @@ -951,13 +935,8 @@ isp_video_dqbuf(struct file *file, void *fh, struct v4l2_buffer *b)
>  {
>  	struct isp_video_fh *vfh = to_isp_video_fh(fh);
>  	struct isp_video *video = video_drvdata(file);
> -	int ret;
>  
> -	mutex_lock(&video->queue_lock);
> -	ret = vb2_dqbuf(&vfh->queue, b, file->f_flags & O_NONBLOCK);
> -	mutex_unlock(&video->queue_lock);
> -
> -	return ret;
> +	return vb2_dqbuf(&vfh->queue, b, file->f_flags & O_NONBLOCK);
>  }
>  
>  static int isp_video_check_external_subdevs(struct isp_video *video,
> @@ -1096,8 +1075,6 @@ isp_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
>  	if (type != video->type)
>  		return -EINVAL;
>  
> -	mutex_lock(&video->stream_lock);
> -
>  	/* Start streaming on the pipeline. No link touching an entity in the
>  	 * pipeline can be activated or deactivated once streaming is started.
>  	 */
> @@ -1106,7 +1083,7 @@ isp_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
>  
>  	ret = media_entity_enum_init(&pipe->ent_enum, &video->isp->media_dev);
>  	if (ret)
> -		goto err_enum_init;
> +		return ret;
>  
>  	/* TODO: Implement PM QoS */
>  	pipe->l3_ick = clk_get_rate(video->isp->clock[ISP_CLK_L3_ICK]);
> @@ -1158,14 +1135,10 @@ isp_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
>  	atomic_set(&pipe->frame_number, -1);
>  	pipe->field = vfh->format.fmt.pix.field;
>  
> -	mutex_lock(&video->queue_lock);
>  	ret = vb2_streamon(&vfh->queue, type);
> -	mutex_unlock(&video->queue_lock);
>  	if (ret < 0)
>  		goto err_check_format;
>  
> -	mutex_unlock(&video->stream_lock);
> -
>  	return 0;
>  
>  err_check_format:
> @@ -1184,9 +1157,6 @@ isp_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
>  
>  	media_entity_enum_cleanup(&pipe->ent_enum);
>  
> -err_enum_init:
> -	mutex_unlock(&video->stream_lock);
> -
>  	return ret;
>  }
>  
> @@ -1203,15 +1173,10 @@ isp_video_streamoff(struct file *file, void *fh, enum v4l2_buf_type type)
>  	if (type != video->type)
>  		return -EINVAL;
>  
> -	mutex_lock(&video->stream_lock);
> -
>  	/* Make sure we're not streaming yet. */
> -	mutex_lock(&video->queue_lock);
>  	streaming = vb2_is_streaming(&vfh->queue);
> -	mutex_unlock(&video->queue_lock);
> -
>  	if (!streaming)
> -		goto done;
> +		return 0;
>  
>  	/* Update the pipeline state. */
>  	if (video->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
> @@ -1229,19 +1194,13 @@ isp_video_streamoff(struct file *file, void *fh, enum v4l2_buf_type type)
>  	omap3isp_pipeline_set_stream(pipe, ISP_PIPELINE_STREAM_STOPPED);
>  	omap3isp_video_cancel_stream(video);
>  
> -	mutex_lock(&video->queue_lock);
>  	vb2_streamoff(&vfh->queue, type);
> -	mutex_unlock(&video->queue_lock);
>  	video->queue = NULL;
>  	video->error = false;
>  
>  	/* TODO: Implement PM QoS */
>  	media_pipeline_stop(&video->video.entity);
> -
>  	media_entity_enum_cleanup(&pipe->ent_enum);
> -
> -done:
> -	mutex_unlock(&video->stream_lock);
>  	return 0;
>  }
>  
> @@ -1333,6 +1292,7 @@ static int isp_video_open(struct file *file)
>  	queue->buf_struct_size = sizeof(struct isp_buffer);
>  	queue->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
>  	queue->dev = video->isp->dev;
> +	queue->lock = &video->queue_lock;
>  
>  	ret = vb2_queue_init(&handle->queue);
>  	if (ret < 0) {
> @@ -1363,10 +1323,9 @@ static int isp_video_release(struct file *file)
>  	struct v4l2_fh *vfh = file->private_data;
>  	struct isp_video_fh *handle = to_isp_video_fh(vfh);
>  
> +	mutex_lock(&video->queue_lock);
>  	/* Disable streaming and free the buffers queue resources. */
>  	isp_video_streamoff(file, vfh, video->type);
> -
> -	mutex_lock(&video->queue_lock);
>  	vb2_queue_release(&handle->queue);
>  	mutex_unlock(&video->queue_lock);
>  
> @@ -1449,7 +1408,6 @@ int omap3isp_video_init(struct isp_video *video, const char *name)
>  	atomic_set(&video->active, 0);
>  
>  	spin_lock_init(&video->pipe.lock);
> -	mutex_init(&video->stream_lock);
>  	mutex_init(&video->queue_lock);
>  	spin_lock_init(&video->irqlock);
>  
> @@ -1474,7 +1432,6 @@ void omap3isp_video_cleanup(struct isp_video *video)
>  {
>  	media_entity_cleanup(&video->video.entity);
>  	mutex_destroy(&video->queue_lock);
> -	mutex_destroy(&video->stream_lock);
>  	mutex_destroy(&video->mutex);
>  }
>  
> diff --git a/drivers/media/platform/omap3isp/ispvideo.h b/drivers/media/platform/omap3isp/ispvideo.h
> index f6a2082b4a0a..5a8fba85e0eb 100644
> --- a/drivers/media/platform/omap3isp/ispvideo.h
> +++ b/drivers/media/platform/omap3isp/ispvideo.h
> @@ -167,7 +167,6 @@ struct isp_video {
>  
>  	/* Pipeline state */
>  	struct isp_pipeline pipe;
> -	struct mutex stream_lock;	/* pipeline and stream states */
>  	bool error;
>  
>  	/* Video buffers queue */
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v5 04/17] omap3isp: Add vb2_queue lock
  2018-06-18  7:31   ` Hans Verkuil
@ 2018-06-20 17:42     ` Ezequiel Garcia
  2018-06-21  8:18       ` Hans Verkuil
  2018-06-22  3:53       ` [PATCH v6 " Ezequiel Garcia
  0 siblings, 2 replies; 28+ messages in thread
From: Ezequiel Garcia @ 2018-06-20 17:42 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Ezequiel Garcia

vb2_queue locks is now mandatory. Add it, remove driver ad-hoc locks,
and implement wait_{prepare, finish}.

Also, remove stream_lock mutex. Sicen the ioctls operations
are protected by the queue mutex, the stream_lock mutex is
not needed.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/media/platform/omap3isp/ispvideo.c | 65 ++++------------------
 drivers/media/platform/omap3isp/ispvideo.h |  1 -
 2 files changed, 11 insertions(+), 55 deletions(-)

diff --git a/drivers/media/platform/omap3isp/ispvideo.c b/drivers/media/platform/omap3isp/ispvideo.c
index 9d228eac24ea..f835aeb9ddc5 100644
--- a/drivers/media/platform/omap3isp/ispvideo.c
+++ b/drivers/media/platform/omap3isp/ispvideo.c
@@ -496,6 +496,8 @@ static const struct vb2_ops isp_video_queue_ops = {
 	.buf_prepare = isp_video_buffer_prepare,
 	.buf_queue = isp_video_buffer_queue,
 	.start_streaming = isp_video_start_streaming,
+	.wait_prepare = vb2_ops_wait_prepare,
+	.wait_finish = vb2_ops_wait_finish,
 };
 
 /*
@@ -628,11 +630,8 @@ void omap3isp_video_resume(struct isp_video *video, int continuous)
 {
 	struct isp_buffer *buf = NULL;
 
-	if (continuous && video->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
-		mutex_lock(&video->queue_lock);
+	if (continuous && video->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		vb2_discard_done(video->queue);
-		mutex_unlock(&video->queue_lock);
-	}
 
 	if (!list_empty(&video->dmaqueue)) {
 		buf = list_first_entry(&video->dmaqueue,
@@ -909,13 +908,8 @@ isp_video_reqbufs(struct file *file, void *fh, struct v4l2_requestbuffers *rb)
 {
 	struct isp_video_fh *vfh = to_isp_video_fh(fh);
 	struct isp_video *video = video_drvdata(file);
-	int ret;
-
-	mutex_lock(&video->queue_lock);
-	ret = vb2_reqbufs(&vfh->queue, rb);
-	mutex_unlock(&video->queue_lock);
 
-	return ret;
+	return vb2_reqbufs(&vfh->queue, rb);
 }
 
 static int
@@ -923,13 +917,8 @@ isp_video_querybuf(struct file *file, void *fh, struct v4l2_buffer *b)
 {
 	struct isp_video_fh *vfh = to_isp_video_fh(fh);
 	struct isp_video *video = video_drvdata(file);
-	int ret;
 
-	mutex_lock(&video->queue_lock);
-	ret = vb2_querybuf(&vfh->queue, b);
-	mutex_unlock(&video->queue_lock);
-
-	return ret;
+	return vb2_querybuf(&vfh->queue, b);
 }
 
 static int
@@ -937,13 +926,8 @@ isp_video_qbuf(struct file *file, void *fh, struct v4l2_buffer *b)
 {
 	struct isp_video_fh *vfh = to_isp_video_fh(fh);
 	struct isp_video *video = video_drvdata(file);
-	int ret;
 
-	mutex_lock(&video->queue_lock);
-	ret = vb2_qbuf(&vfh->queue, b);
-	mutex_unlock(&video->queue_lock);
-
-	return ret;
+	return vb2_qbuf(&vfh->queue, b);
 }
 
 static int
@@ -951,13 +935,8 @@ isp_video_dqbuf(struct file *file, void *fh, struct v4l2_buffer *b)
 {
 	struct isp_video_fh *vfh = to_isp_video_fh(fh);
 	struct isp_video *video = video_drvdata(file);
-	int ret;
 
-	mutex_lock(&video->queue_lock);
-	ret = vb2_dqbuf(&vfh->queue, b, file->f_flags & O_NONBLOCK);
-	mutex_unlock(&video->queue_lock);
-
-	return ret;
+	return vb2_dqbuf(&vfh->queue, b, file->f_flags & O_NONBLOCK);
 }
 
 static int isp_video_check_external_subdevs(struct isp_video *video,
@@ -1096,8 +1075,6 @@ isp_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
 	if (type != video->type)
 		return -EINVAL;
 
-	mutex_lock(&video->stream_lock);
-
 	/* Start streaming on the pipeline. No link touching an entity in the
 	 * pipeline can be activated or deactivated once streaming is started.
 	 */
@@ -1106,7 +1083,7 @@ isp_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
 
 	ret = media_entity_enum_init(&pipe->ent_enum, &video->isp->media_dev);
 	if (ret)
-		goto err_enum_init;
+		return ret;
 
 	/* TODO: Implement PM QoS */
 	pipe->l3_ick = clk_get_rate(video->isp->clock[ISP_CLK_L3_ICK]);
@@ -1158,14 +1135,10 @@ isp_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
 	atomic_set(&pipe->frame_number, -1);
 	pipe->field = vfh->format.fmt.pix.field;
 
-	mutex_lock(&video->queue_lock);
 	ret = vb2_streamon(&vfh->queue, type);
-	mutex_unlock(&video->queue_lock);
 	if (ret < 0)
 		goto err_check_format;
 
-	mutex_unlock(&video->stream_lock);
-
 	return 0;
 
 err_check_format:
@@ -1184,9 +1157,6 @@ isp_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
 
 	media_entity_enum_cleanup(&pipe->ent_enum);
 
-err_enum_init:
-	mutex_unlock(&video->stream_lock);
-
 	return ret;
 }
 
@@ -1203,15 +1173,10 @@ isp_video_streamoff(struct file *file, void *fh, enum v4l2_buf_type type)
 	if (type != video->type)
 		return -EINVAL;
 
-	mutex_lock(&video->stream_lock);
-
 	/* Make sure we're not streaming yet. */
-	mutex_lock(&video->queue_lock);
 	streaming = vb2_is_streaming(&vfh->queue);
-	mutex_unlock(&video->queue_lock);
-
 	if (!streaming)
-		goto done;
+		return 0;
 
 	/* Update the pipeline state. */
 	if (video->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
@@ -1229,19 +1194,13 @@ isp_video_streamoff(struct file *file, void *fh, enum v4l2_buf_type type)
 	omap3isp_pipeline_set_stream(pipe, ISP_PIPELINE_STREAM_STOPPED);
 	omap3isp_video_cancel_stream(video);
 
-	mutex_lock(&video->queue_lock);
 	vb2_streamoff(&vfh->queue, type);
-	mutex_unlock(&video->queue_lock);
 	video->queue = NULL;
 	video->error = false;
 
 	/* TODO: Implement PM QoS */
 	media_pipeline_stop(&video->video.entity);
-
 	media_entity_enum_cleanup(&pipe->ent_enum);
-
-done:
-	mutex_unlock(&video->stream_lock);
 	return 0;
 }
 
@@ -1333,6 +1292,7 @@ static int isp_video_open(struct file *file)
 	queue->buf_struct_size = sizeof(struct isp_buffer);
 	queue->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
 	queue->dev = video->isp->dev;
+	queue->lock = &video->queue_lock;
 
 	ret = vb2_queue_init(&handle->queue);
 	if (ret < 0) {
@@ -1363,10 +1323,9 @@ static int isp_video_release(struct file *file)
 	struct v4l2_fh *vfh = file->private_data;
 	struct isp_video_fh *handle = to_isp_video_fh(vfh);
 
+	mutex_lock(&video->queue_lock);
 	/* Disable streaming and free the buffers queue resources. */
 	isp_video_streamoff(file, vfh, video->type);
-
-	mutex_lock(&video->queue_lock);
 	vb2_queue_release(&handle->queue);
 	mutex_unlock(&video->queue_lock);
 
@@ -1449,7 +1408,6 @@ int omap3isp_video_init(struct isp_video *video, const char *name)
 	atomic_set(&video->active, 0);
 
 	spin_lock_init(&video->pipe.lock);
-	mutex_init(&video->stream_lock);
 	mutex_init(&video->queue_lock);
 	spin_lock_init(&video->irqlock);
 
@@ -1474,7 +1432,6 @@ void omap3isp_video_cleanup(struct isp_video *video)
 {
 	media_entity_cleanup(&video->video.entity);
 	mutex_destroy(&video->queue_lock);
-	mutex_destroy(&video->stream_lock);
 	mutex_destroy(&video->mutex);
 }
 
diff --git a/drivers/media/platform/omap3isp/ispvideo.h b/drivers/media/platform/omap3isp/ispvideo.h
index f6a2082b4a0a..5a8fba85e0eb 100644
--- a/drivers/media/platform/omap3isp/ispvideo.h
+++ b/drivers/media/platform/omap3isp/ispvideo.h
@@ -167,7 +167,6 @@ struct isp_video {
 
 	/* Pipeline state */
 	struct isp_pipeline pipe;
-	struct mutex stream_lock;	/* pipeline and stream states */
 	bool error;
 
 	/* Video buffers queue */
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH v5 04/17] omap3isp: Add vb2_queue lock
  2018-06-20 17:42     ` [PATCH v5 " Ezequiel Garcia
@ 2018-06-21  8:18       ` Hans Verkuil
  2018-06-22  3:51         ` Ezequiel Garcia
  2018-06-22  3:53       ` [PATCH v6 " Ezequiel Garcia
  1 sibling, 1 reply; 28+ messages in thread
From: Hans Verkuil @ 2018-06-21  8:18 UTC (permalink / raw)
  To: Ezequiel Garcia, linux-media; +Cc: Hans Verkuil, kernel

On 06/20/18 19:42, Ezequiel Garcia wrote:
> vb2_queue locks is now mandatory. Add it, remove driver ad-hoc locks,
> and implement wait_{prepare, finish}.
> 
> Also, remove stream_lock mutex. Sicen the ioctls operations

Sicen -> Since

> are protected by the queue mutex, the stream_lock mutex is

are protected -> are now protected

> not needed.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
>  drivers/media/platform/omap3isp/ispvideo.c | 65 ++++------------------
>  drivers/media/platform/omap3isp/ispvideo.h |  1 -
>  2 files changed, 11 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/media/platform/omap3isp/ispvideo.c b/drivers/media/platform/omap3isp/ispvideo.c
> index 9d228eac24ea..f835aeb9ddc5 100644
> --- a/drivers/media/platform/omap3isp/ispvideo.c
> +++ b/drivers/media/platform/omap3isp/ispvideo.c
> @@ -496,6 +496,8 @@ static const struct vb2_ops isp_video_queue_ops = {
>  	.buf_prepare = isp_video_buffer_prepare,
>  	.buf_queue = isp_video_buffer_queue,
>  	.start_streaming = isp_video_start_streaming,
> +	.wait_prepare = vb2_ops_wait_prepare,
> +	.wait_finish = vb2_ops_wait_finish,
>  };
>  
>  /*
> @@ -628,11 +630,8 @@ void omap3isp_video_resume(struct isp_video *video, int continuous)
>  {
>  	struct isp_buffer *buf = NULL;
>  
> -	if (continuous && video->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
> -		mutex_lock(&video->queue_lock);
> +	if (continuous && video->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
>  		vb2_discard_done(video->queue);
> -		mutex_unlock(&video->queue_lock);
> -	}
>  
>  	if (!list_empty(&video->dmaqueue)) {
>  		buf = list_first_entry(&video->dmaqueue,
> @@ -909,13 +908,8 @@ isp_video_reqbufs(struct file *file, void *fh, struct v4l2_requestbuffers *rb)
>  {
>  	struct isp_video_fh *vfh = to_isp_video_fh(fh);
>  	struct isp_video *video = video_drvdata(file);
> -	int ret;
> -
> -	mutex_lock(&video->queue_lock);
> -	ret = vb2_reqbufs(&vfh->queue, rb);
> -	mutex_unlock(&video->queue_lock);
>  
> -	return ret;
> +	return vb2_reqbufs(&vfh->queue, rb);
>  }
>  
>  static int
> @@ -923,13 +917,8 @@ isp_video_querybuf(struct file *file, void *fh, struct v4l2_buffer *b)
>  {
>  	struct isp_video_fh *vfh = to_isp_video_fh(fh);
>  	struct isp_video *video = video_drvdata(file);
> -	int ret;
>  
> -	mutex_lock(&video->queue_lock);
> -	ret = vb2_querybuf(&vfh->queue, b);
> -	mutex_unlock(&video->queue_lock);
> -
> -	return ret;
> +	return vb2_querybuf(&vfh->queue, b);
>  }
>  
>  static int
> @@ -937,13 +926,8 @@ isp_video_qbuf(struct file *file, void *fh, struct v4l2_buffer *b)
>  {
>  	struct isp_video_fh *vfh = to_isp_video_fh(fh);
>  	struct isp_video *video = video_drvdata(file);
> -	int ret;
>  
> -	mutex_lock(&video->queue_lock);
> -	ret = vb2_qbuf(&vfh->queue, b);
> -	mutex_unlock(&video->queue_lock);
> -
> -	return ret;
> +	return vb2_qbuf(&vfh->queue, b);
>  }
>  
>  static int
> @@ -951,13 +935,8 @@ isp_video_dqbuf(struct file *file, void *fh, struct v4l2_buffer *b)
>  {
>  	struct isp_video_fh *vfh = to_isp_video_fh(fh);
>  	struct isp_video *video = video_drvdata(file);
> -	int ret;
>  
> -	mutex_lock(&video->queue_lock);
> -	ret = vb2_dqbuf(&vfh->queue, b, file->f_flags & O_NONBLOCK);
> -	mutex_unlock(&video->queue_lock);
> -
> -	return ret;
> +	return vb2_dqbuf(&vfh->queue, b, file->f_flags & O_NONBLOCK);
>  }
>  
>  static int isp_video_check_external_subdevs(struct isp_video *video,
> @@ -1096,8 +1075,6 @@ isp_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
>  	if (type != video->type)
>  		return -EINVAL;
>  
> -	mutex_lock(&video->stream_lock);
> -
>  	/* Start streaming on the pipeline. No link touching an entity in the
>  	 * pipeline can be activated or deactivated once streaming is started.
>  	 */
> @@ -1106,7 +1083,7 @@ isp_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
>  
>  	ret = media_entity_enum_init(&pipe->ent_enum, &video->isp->media_dev);
>  	if (ret)
> -		goto err_enum_init;
> +		return ret;
>  
>  	/* TODO: Implement PM QoS */
>  	pipe->l3_ick = clk_get_rate(video->isp->clock[ISP_CLK_L3_ICK]);
> @@ -1158,14 +1135,10 @@ isp_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
>  	atomic_set(&pipe->frame_number, -1);
>  	pipe->field = vfh->format.fmt.pix.field;
>  
> -	mutex_lock(&video->queue_lock);
>  	ret = vb2_streamon(&vfh->queue, type);
> -	mutex_unlock(&video->queue_lock);
>  	if (ret < 0)
>  		goto err_check_format;
>  
> -	mutex_unlock(&video->stream_lock);
> -
>  	return 0;
>  
>  err_check_format:
> @@ -1184,9 +1157,6 @@ isp_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
>  
>  	media_entity_enum_cleanup(&pipe->ent_enum);
>  
> -err_enum_init:
> -	mutex_unlock(&video->stream_lock);
> -
>  	return ret;
>  }
>  
> @@ -1203,15 +1173,10 @@ isp_video_streamoff(struct file *file, void *fh, enum v4l2_buf_type type)
>  	if (type != video->type)
>  		return -EINVAL;
>  
> -	mutex_lock(&video->stream_lock);
> -
>  	/* Make sure we're not streaming yet. */
> -	mutex_lock(&video->queue_lock);
>  	streaming = vb2_is_streaming(&vfh->queue);
> -	mutex_unlock(&video->queue_lock);
> -
>  	if (!streaming)
> -		goto done;
> +		return 0;
>  
>  	/* Update the pipeline state. */
>  	if (video->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
> @@ -1229,19 +1194,13 @@ isp_video_streamoff(struct file *file, void *fh, enum v4l2_buf_type type)
>  	omap3isp_pipeline_set_stream(pipe, ISP_PIPELINE_STREAM_STOPPED);
>  	omap3isp_video_cancel_stream(video);
>  
> -	mutex_lock(&video->queue_lock);
>  	vb2_streamoff(&vfh->queue, type);
> -	mutex_unlock(&video->queue_lock);
>  	video->queue = NULL;
>  	video->error = false;
>  
>  	/* TODO: Implement PM QoS */
>  	media_pipeline_stop(&video->video.entity);
> -
>  	media_entity_enum_cleanup(&pipe->ent_enum);
> -
> -done:
> -	mutex_unlock(&video->stream_lock);
>  	return 0;
>  }
>  
> @@ -1333,6 +1292,7 @@ static int isp_video_open(struct file *file)
>  	queue->buf_struct_size = sizeof(struct isp_buffer);
>  	queue->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
>  	queue->dev = video->isp->dev;
> +	queue->lock = &video->queue_lock;
>  
>  	ret = vb2_queue_init(&handle->queue);
>  	if (ret < 0) {
> @@ -1363,10 +1323,9 @@ static int isp_video_release(struct file *file)
>  	struct v4l2_fh *vfh = file->private_data;
>  	struct isp_video_fh *handle = to_isp_video_fh(vfh);
>  
> +	mutex_lock(&video->queue_lock);
>  	/* Disable streaming and free the buffers queue resources. */
>  	isp_video_streamoff(file, vfh, video->type);
> -
> -	mutex_lock(&video->queue_lock);
>  	vb2_queue_release(&handle->queue);
>  	mutex_unlock(&video->queue_lock);

Hmm, this mutex_unlock is not removed, did you miss this one?

>  
> @@ -1449,7 +1408,6 @@ int omap3isp_video_init(struct isp_video *video, const char *name)
>  	atomic_set(&video->active, 0);
>  
>  	spin_lock_init(&video->pipe.lock);
> -	mutex_init(&video->stream_lock);
>  	mutex_init(&video->queue_lock);
>  	spin_lock_init(&video->irqlock);
>  
> @@ -1474,7 +1432,6 @@ void omap3isp_video_cleanup(struct isp_video *video)
>  {
>  	media_entity_cleanup(&video->video.entity);
>  	mutex_destroy(&video->queue_lock);
> -	mutex_destroy(&video->stream_lock);
>  	mutex_destroy(&video->mutex);
>  }
>  
> diff --git a/drivers/media/platform/omap3isp/ispvideo.h b/drivers/media/platform/omap3isp/ispvideo.h
> index f6a2082b4a0a..5a8fba85e0eb 100644
> --- a/drivers/media/platform/omap3isp/ispvideo.h
> +++ b/drivers/media/platform/omap3isp/ispvideo.h
> @@ -167,7 +167,6 @@ struct isp_video {
>  
>  	/* Pipeline state */
>  	struct isp_pipeline pipe;
> -	struct mutex stream_lock;	/* pipeline and stream states */
>  	bool error;
>  
>  	/* Video buffers queue */
> 

Regards,

	Hans

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v5 04/17] omap3isp: Add vb2_queue lock
  2018-06-21  8:18       ` Hans Verkuil
@ 2018-06-22  3:51         ` Ezequiel Garcia
  0 siblings, 0 replies; 28+ messages in thread
From: Ezequiel Garcia @ 2018-06-22  3:51 UTC (permalink / raw)
  To: Hans Verkuil, linux-media; +Cc: Hans Verkuil, kernel

On Thu, 2018-06-21 at 10:18 +0200, Hans Verkuil wrote:
> On 06/20/18 19:42, Ezequiel Garcia wrote:
> > vb2_queue locks is now mandatory. Add it, remove driver ad-hoc
> > locks,
> > and implement wait_{prepare, finish}.
> > 
> > Also, remove stream_lock mutex. Sicen the ioctls operations
> 
> Sicen -> Since
> 
> > are protected by the queue mutex, the stream_lock mutex is
> 
> are protected -> are now protected
> 

Will fix.

[snip]
> > @@ -1363,10 +1323,9 @@ static int isp_video_release(struct file
> > *file)
> > 
> >  	struct v4l2_fh *vfh = file->private_data;
> >  	struct isp_video_fh *handle = to_isp_video_fh(vfh);
> >  
> > +	mutex_lock(&video->queue_lock);

See below.

> >  	/* Disable streaming and free the buffers queue resources.
> > */
> >  	isp_video_streamoff(file, vfh, video->type);
> > -
> > -	mutex_lock(&video->queue_lock);
> >  	vb2_queue_release(&handle->queue);
> >  	mutex_unlock(&video->queue_lock);
> 
> Hmm, this mutex_unlock is not removed, did you miss this one?
> 

The mutex_lock call is moved before the call to isp_video_streamoff,
because .streamoff is called with the queue lock held,
and so it seemed more consistent.

I will send a v6 with the amended commit log.

Let me know if you catch anything else, it's a long series
and the devil is always in the detail!

Thanks,
Eze

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v6 04/17] omap3isp: Add vb2_queue lock
  2018-06-20 17:42     ` [PATCH v5 " Ezequiel Garcia
  2018-06-21  8:18       ` Hans Verkuil
@ 2018-06-22  3:53       ` Ezequiel Garcia
  2018-07-02 16:49         ` Laurent Pinchart
  1 sibling, 1 reply; 28+ messages in thread
From: Ezequiel Garcia @ 2018-06-22  3:53 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Ezequiel Garcia

vb2_queue locks is now mandatory. Add it, remove driver ad-hoc locks,
and implement wait_{prepare, finish}.

Also, remove stream_lock mutex. Since the ioctls operations
are now protected by the queue mutex, the stream_lock mutex is
not needed.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/media/platform/omap3isp/ispvideo.c | 65 ++++------------------
 drivers/media/platform/omap3isp/ispvideo.h |  1 -
 2 files changed, 11 insertions(+), 55 deletions(-)

diff --git a/drivers/media/platform/omap3isp/ispvideo.c b/drivers/media/platform/omap3isp/ispvideo.c
index 9d228eac24ea..f835aeb9ddc5 100644
--- a/drivers/media/platform/omap3isp/ispvideo.c
+++ b/drivers/media/platform/omap3isp/ispvideo.c
@@ -496,6 +496,8 @@ static const struct vb2_ops isp_video_queue_ops = {
 	.buf_prepare = isp_video_buffer_prepare,
 	.buf_queue = isp_video_buffer_queue,
 	.start_streaming = isp_video_start_streaming,
+	.wait_prepare = vb2_ops_wait_prepare,
+	.wait_finish = vb2_ops_wait_finish,
 };
 
 /*
@@ -628,11 +630,8 @@ void omap3isp_video_resume(struct isp_video *video, int continuous)
 {
 	struct isp_buffer *buf = NULL;
 
-	if (continuous && video->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
-		mutex_lock(&video->queue_lock);
+	if (continuous && video->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		vb2_discard_done(video->queue);
-		mutex_unlock(&video->queue_lock);
-	}
 
 	if (!list_empty(&video->dmaqueue)) {
 		buf = list_first_entry(&video->dmaqueue,
@@ -909,13 +908,8 @@ isp_video_reqbufs(struct file *file, void *fh, struct v4l2_requestbuffers *rb)
 {
 	struct isp_video_fh *vfh = to_isp_video_fh(fh);
 	struct isp_video *video = video_drvdata(file);
-	int ret;
-
-	mutex_lock(&video->queue_lock);
-	ret = vb2_reqbufs(&vfh->queue, rb);
-	mutex_unlock(&video->queue_lock);
 
-	return ret;
+	return vb2_reqbufs(&vfh->queue, rb);
 }
 
 static int
@@ -923,13 +917,8 @@ isp_video_querybuf(struct file *file, void *fh, struct v4l2_buffer *b)
 {
 	struct isp_video_fh *vfh = to_isp_video_fh(fh);
 	struct isp_video *video = video_drvdata(file);
-	int ret;
 
-	mutex_lock(&video->queue_lock);
-	ret = vb2_querybuf(&vfh->queue, b);
-	mutex_unlock(&video->queue_lock);
-
-	return ret;
+	return vb2_querybuf(&vfh->queue, b);
 }
 
 static int
@@ -937,13 +926,8 @@ isp_video_qbuf(struct file *file, void *fh, struct v4l2_buffer *b)
 {
 	struct isp_video_fh *vfh = to_isp_video_fh(fh);
 	struct isp_video *video = video_drvdata(file);
-	int ret;
 
-	mutex_lock(&video->queue_lock);
-	ret = vb2_qbuf(&vfh->queue, b);
-	mutex_unlock(&video->queue_lock);
-
-	return ret;
+	return vb2_qbuf(&vfh->queue, b);
 }
 
 static int
@@ -951,13 +935,8 @@ isp_video_dqbuf(struct file *file, void *fh, struct v4l2_buffer *b)
 {
 	struct isp_video_fh *vfh = to_isp_video_fh(fh);
 	struct isp_video *video = video_drvdata(file);
-	int ret;
 
-	mutex_lock(&video->queue_lock);
-	ret = vb2_dqbuf(&vfh->queue, b, file->f_flags & O_NONBLOCK);
-	mutex_unlock(&video->queue_lock);
-
-	return ret;
+	return vb2_dqbuf(&vfh->queue, b, file->f_flags & O_NONBLOCK);
 }
 
 static int isp_video_check_external_subdevs(struct isp_video *video,
@@ -1096,8 +1075,6 @@ isp_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
 	if (type != video->type)
 		return -EINVAL;
 
-	mutex_lock(&video->stream_lock);
-
 	/* Start streaming on the pipeline. No link touching an entity in the
 	 * pipeline can be activated or deactivated once streaming is started.
 	 */
@@ -1106,7 +1083,7 @@ isp_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
 
 	ret = media_entity_enum_init(&pipe->ent_enum, &video->isp->media_dev);
 	if (ret)
-		goto err_enum_init;
+		return ret;
 
 	/* TODO: Implement PM QoS */
 	pipe->l3_ick = clk_get_rate(video->isp->clock[ISP_CLK_L3_ICK]);
@@ -1158,14 +1135,10 @@ isp_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
 	atomic_set(&pipe->frame_number, -1);
 	pipe->field = vfh->format.fmt.pix.field;
 
-	mutex_lock(&video->queue_lock);
 	ret = vb2_streamon(&vfh->queue, type);
-	mutex_unlock(&video->queue_lock);
 	if (ret < 0)
 		goto err_check_format;
 
-	mutex_unlock(&video->stream_lock);
-
 	return 0;
 
 err_check_format:
@@ -1184,9 +1157,6 @@ isp_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
 
 	media_entity_enum_cleanup(&pipe->ent_enum);
 
-err_enum_init:
-	mutex_unlock(&video->stream_lock);
-
 	return ret;
 }
 
@@ -1203,15 +1173,10 @@ isp_video_streamoff(struct file *file, void *fh, enum v4l2_buf_type type)
 	if (type != video->type)
 		return -EINVAL;
 
-	mutex_lock(&video->stream_lock);
-
 	/* Make sure we're not streaming yet. */
-	mutex_lock(&video->queue_lock);
 	streaming = vb2_is_streaming(&vfh->queue);
-	mutex_unlock(&video->queue_lock);
-
 	if (!streaming)
-		goto done;
+		return 0;
 
 	/* Update the pipeline state. */
 	if (video->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
@@ -1229,19 +1194,13 @@ isp_video_streamoff(struct file *file, void *fh, enum v4l2_buf_type type)
 	omap3isp_pipeline_set_stream(pipe, ISP_PIPELINE_STREAM_STOPPED);
 	omap3isp_video_cancel_stream(video);
 
-	mutex_lock(&video->queue_lock);
 	vb2_streamoff(&vfh->queue, type);
-	mutex_unlock(&video->queue_lock);
 	video->queue = NULL;
 	video->error = false;
 
 	/* TODO: Implement PM QoS */
 	media_pipeline_stop(&video->video.entity);
-
 	media_entity_enum_cleanup(&pipe->ent_enum);
-
-done:
-	mutex_unlock(&video->stream_lock);
 	return 0;
 }
 
@@ -1333,6 +1292,7 @@ static int isp_video_open(struct file *file)
 	queue->buf_struct_size = sizeof(struct isp_buffer);
 	queue->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
 	queue->dev = video->isp->dev;
+	queue->lock = &video->queue_lock;
 
 	ret = vb2_queue_init(&handle->queue);
 	if (ret < 0) {
@@ -1363,10 +1323,9 @@ static int isp_video_release(struct file *file)
 	struct v4l2_fh *vfh = file->private_data;
 	struct isp_video_fh *handle = to_isp_video_fh(vfh);
 
+	mutex_lock(&video->queue_lock);
 	/* Disable streaming and free the buffers queue resources. */
 	isp_video_streamoff(file, vfh, video->type);
-
-	mutex_lock(&video->queue_lock);
 	vb2_queue_release(&handle->queue);
 	mutex_unlock(&video->queue_lock);
 
@@ -1449,7 +1408,6 @@ int omap3isp_video_init(struct isp_video *video, const char *name)
 	atomic_set(&video->active, 0);
 
 	spin_lock_init(&video->pipe.lock);
-	mutex_init(&video->stream_lock);
 	mutex_init(&video->queue_lock);
 	spin_lock_init(&video->irqlock);
 
@@ -1474,7 +1432,6 @@ void omap3isp_video_cleanup(struct isp_video *video)
 {
 	media_entity_cleanup(&video->video.entity);
 	mutex_destroy(&video->queue_lock);
-	mutex_destroy(&video->stream_lock);
 	mutex_destroy(&video->mutex);
 }
 
diff --git a/drivers/media/platform/omap3isp/ispvideo.h b/drivers/media/platform/omap3isp/ispvideo.h
index f6a2082b4a0a..5a8fba85e0eb 100644
--- a/drivers/media/platform/omap3isp/ispvideo.h
+++ b/drivers/media/platform/omap3isp/ispvideo.h
@@ -167,7 +167,6 @@ struct isp_video {
 
 	/* Pipeline state */
 	struct isp_pipeline pipe;
-	struct mutex stream_lock;	/* pipeline and stream states */
 	bool error;
 
 	/* Video buffers queue */
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH v4 00/17] v4l2 core: push ioctl lock down to ioctl handler
  2018-06-15 19:07 [PATCH v4 00/17] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
                   ` (16 preceding siblings ...)
  2018-06-15 19:07 ` [PATCH v4 17/17] media: Remove wait_{prepare, finish} Ezequiel Garcia
@ 2018-06-29 18:12 ` Ezequiel Garcia
  17 siblings, 0 replies; 28+ messages in thread
From: Ezequiel Garcia @ 2018-06-29 18:12 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel

On Fri, 2018-06-15 at 16:07 -0300, Ezequiel Garcia wrote:
> Fourth spin of the series posted by Hans:
> 
> https://www.mail-archive.com/linux-media@vger.kernel.org/msg131363.ht
> ml
> 
> There aren't any changes from v3, aside from rebasing
> and re-ordering the patches as requested by Hans.
> 
> See v3 cover letter for more details.
> 
> Series was tested with tw686x, gspca sonixj and UVC devices.
> Build tested with the 0-day kbuild test robot.
> 
> 

AFAIK, nothing is preventing this series.

Hans, is there any other feedback?

Thanks,
Ezequiel

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v4 03/17] omap4iss: Add vb2_queue lock
  2018-06-15 19:07 ` [PATCH v4 03/17] omap4iss: Add vb2_queue lock Ezequiel Garcia
@ 2018-07-02  8:28   ` Hans Verkuil
  2018-07-06 15:24     ` Ezequiel Garcia
  0 siblings, 1 reply; 28+ messages in thread
From: Hans Verkuil @ 2018-07-02  8:28 UTC (permalink / raw)
  To: Ezequiel Garcia, linux-media; +Cc: Hans Verkuil, kernel

On 15/06/18 21:07, Ezequiel Garcia wrote:
> vb2_queue lock is now mandatory. Add it, remove driver ad-hoc
> locks, and implement wait_{prepare, finish}.

I don't see any wait_prepare/finish implementation?!

Regards,

	Hans

> 
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
>  drivers/staging/media/omap4iss/iss_video.c | 13 ++-----------
>  1 file changed, 2 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/staging/media/omap4iss/iss_video.c b/drivers/staging/media/omap4iss/iss_video.c
> index a3a83424a926..d919bae83828 100644
> --- a/drivers/staging/media/omap4iss/iss_video.c
> +++ b/drivers/staging/media/omap4iss/iss_video.c
> @@ -873,8 +873,6 @@ iss_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
>  	if (type != video->type)
>  		return -EINVAL;
>  
> -	mutex_lock(&video->stream_lock);
> -
>  	/*
>  	 * Start streaming on the pipeline. No link touching an entity in the
>  	 * pipeline can be activated or deactivated once streaming is started.
> @@ -978,8 +976,6 @@ iss_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
>  
>  	media_graph_walk_cleanup(&graph);
>  
> -	mutex_unlock(&video->stream_lock);
> -
>  	return 0;
>  
>  err_omap4iss_set_stream:
> @@ -996,8 +992,6 @@ iss_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
>  err_graph_walk_init:
>  	media_entity_enum_cleanup(&pipe->ent_enum);
>  
> -	mutex_unlock(&video->stream_lock);
> -
>  	return ret;
>  }
>  
> @@ -1013,10 +1007,8 @@ iss_video_streamoff(struct file *file, void *fh, enum v4l2_buf_type type)
>  	if (type != video->type)
>  		return -EINVAL;
>  
> -	mutex_lock(&video->stream_lock);
> -
>  	if (!vb2_is_streaming(&vfh->queue))
> -		goto done;
> +		return 0;
>  
>  	/* Update the pipeline state. */
>  	if (video->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
> @@ -1041,8 +1033,6 @@ iss_video_streamoff(struct file *file, void *fh, enum v4l2_buf_type type)
>  		video->iss->pdata->set_constraints(video->iss, false);
>  	media_pipeline_stop(&video->video.entity);
>  
> -done:
> -	mutex_unlock(&video->stream_lock);
>  	return 0;
>  }
>  
> @@ -1137,6 +1127,7 @@ static int iss_video_open(struct file *file)
>  	q->buf_struct_size = sizeof(struct iss_buffer);
>  	q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
>  	q->dev = video->iss->dev;
> +	q->lock = &video->stream_lock;
>  
>  	ret = vb2_queue_init(q);
>  	if (ret) {
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v6 04/17] omap3isp: Add vb2_queue lock
  2018-06-22  3:53       ` [PATCH v6 " Ezequiel Garcia
@ 2018-07-02 16:49         ` Laurent Pinchart
  2018-07-06 15:11           ` Ezequiel Garcia
  0 siblings, 1 reply; 28+ messages in thread
From: Laurent Pinchart @ 2018-07-02 16:49 UTC (permalink / raw)
  To: Ezequiel Garcia; +Cc: linux-media, Hans Verkuil, kernel, sakari.ailus

Hi Ezequiel,

(CC'ing Sakari)

Thank you for the patch.

On Friday, 22 June 2018 06:53:58 EEST Ezequiel Garcia wrote:
> vb2_queue locks is now mandatory. Add it, remove driver ad-hoc locks,
> and implement wait_{prepare, finish}.
> 
> Also, remove stream_lock mutex. Since the ioctls operations
> are now protected by the queue mutex, the stream_lock mutex is
> not needed.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
>  drivers/media/platform/omap3isp/ispvideo.c | 65 ++++------------------
>  drivers/media/platform/omap3isp/ispvideo.h |  1 -
>  2 files changed, 11 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/media/platform/omap3isp/ispvideo.c
> b/drivers/media/platform/omap3isp/ispvideo.c index
> 9d228eac24ea..f835aeb9ddc5 100644
> --- a/drivers/media/platform/omap3isp/ispvideo.c
> +++ b/drivers/media/platform/omap3isp/ispvideo.c
> @@ -496,6 +496,8 @@ static const struct vb2_ops isp_video_queue_ops = {
>  	.buf_prepare = isp_video_buffer_prepare,
>  	.buf_queue = isp_video_buffer_queue,
>  	.start_streaming = isp_video_start_streaming,
> +	.wait_prepare = vb2_ops_wait_prepare,
> +	.wait_finish = vb2_ops_wait_finish,
>  };
> 
>  /*
> @@ -628,11 +630,8 @@ void omap3isp_video_resume(struct isp_video *video, int
> continuous) {
>  	struct isp_buffer *buf = NULL;
> 
> -	if (continuous && video->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
> -		mutex_lock(&video->queue_lock);
> +	if (continuous && video->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
>  		vb2_discard_done(video->queue);
> -		mutex_unlock(&video->queue_lock);
> -	}

Why can locking be removed here ? As far as I know the lock isn't taken by the 
caller. It would help to explain the rationale in the commit message.

>  	if (!list_empty(&video->dmaqueue)) {
>  		buf = list_first_entry(&video->dmaqueue,
> @@ -909,13 +908,8 @@ isp_video_reqbufs(struct file *file, void *fh, struct
> v4l2_requestbuffers *rb) {
>  	struct isp_video_fh *vfh = to_isp_video_fh(fh);
>  	struct isp_video *video = video_drvdata(file);

The video variable isn't used anymore. Didn't gcc warn you ?

> -	int ret;
> -
> -	mutex_lock(&video->queue_lock);
> -	ret = vb2_reqbufs(&vfh->queue, rb);
> -	mutex_unlock(&video->queue_lock);
> 
> -	return ret;
> +	return vb2_reqbufs(&vfh->queue, rb);
>  }
> 
>  static int
> @@ -923,13 +917,8 @@ isp_video_querybuf(struct file *file, void *fh, struct
> v4l2_buffer *b) {
>  	struct isp_video_fh *vfh = to_isp_video_fh(fh);
>  	struct isp_video *video = video_drvdata(file);

Same here and in other functions below.

> -	int ret;
> 
> -	mutex_lock(&video->queue_lock);
> -	ret = vb2_querybuf(&vfh->queue, b);
> -	mutex_unlock(&video->queue_lock);
> -
> -	return ret;
> +	return vb2_querybuf(&vfh->queue, b);
>  }
> 
>  static int
> @@ -937,13 +926,8 @@ isp_video_qbuf(struct file *file, void *fh, struct
> v4l2_buffer *b) {
>  	struct isp_video_fh *vfh = to_isp_video_fh(fh);
>  	struct isp_video *video = video_drvdata(file);
> -	int ret;
> 
> -	mutex_lock(&video->queue_lock);
> -	ret = vb2_qbuf(&vfh->queue, b);
> -	mutex_unlock(&video->queue_lock);
> -
> -	return ret;
> +	return vb2_qbuf(&vfh->queue, b);
>  }
> 
>  static int
> @@ -951,13 +935,8 @@ isp_video_dqbuf(struct file *file, void *fh, struct
> v4l2_buffer *b) {
>  	struct isp_video_fh *vfh = to_isp_video_fh(fh);
>  	struct isp_video *video = video_drvdata(file);
> -	int ret;
> 
> -	mutex_lock(&video->queue_lock);
> -	ret = vb2_dqbuf(&vfh->queue, b, file->f_flags & O_NONBLOCK);
> -	mutex_unlock(&video->queue_lock);
> -
> -	return ret;
> +	return vb2_dqbuf(&vfh->queue, b, file->f_flags & O_NONBLOCK);
>  }
> 
>  static int isp_video_check_external_subdevs(struct isp_video *video,
> @@ -1096,8 +1075,6 @@ isp_video_streamon(struct file *file, void *fh, enum
> v4l2_buf_type type) if (type != video->type)
>  		return -EINVAL;
> 
> -	mutex_lock(&video->stream_lock);
> -
>  	/* Start streaming on the pipeline. No link touching an entity in the
>  	 * pipeline can be activated or deactivated once streaming is started.
>  	 */
> @@ -1106,7 +1083,7 @@ isp_video_streamon(struct file *file, void *fh, enum
> v4l2_buf_type type)
> 
>  	ret = media_entity_enum_init(&pipe->ent_enum, &video->isp->media_dev);
>  	if (ret)
> -		goto err_enum_init;
> +		return ret;
> 
>  	/* TODO: Implement PM QoS */
>  	pipe->l3_ick = clk_get_rate(video->isp->clock[ISP_CLK_L3_ICK]);
> @@ -1158,14 +1135,10 @@ isp_video_streamon(struct file *file, void *fh, enum
> v4l2_buf_type type) atomic_set(&pipe->frame_number, -1);
>  	pipe->field = vfh->format.fmt.pix.field;
> 
> -	mutex_lock(&video->queue_lock);
>  	ret = vb2_streamon(&vfh->queue, type);
> -	mutex_unlock(&video->queue_lock);
>  	if (ret < 0)
>  		goto err_check_format;
> 
> -	mutex_unlock(&video->stream_lock);
> -
>  	return 0;
> 
>  err_check_format:
> @@ -1184,9 +1157,6 @@ isp_video_streamon(struct file *file, void *fh, enum
> v4l2_buf_type type)
> 
>  	media_entity_enum_cleanup(&pipe->ent_enum);
> 
> -err_enum_init:
> -	mutex_unlock(&video->stream_lock);
> -
>  	return ret;
>  }
> 
> @@ -1203,15 +1173,10 @@ isp_video_streamoff(struct file *file, void *fh,
> enum v4l2_buf_type type) if (type != video->type)
>  		return -EINVAL;
> 
> -	mutex_lock(&video->stream_lock);
> -
>  	/* Make sure we're not streaming yet. */
> -	mutex_lock(&video->queue_lock);
>  	streaming = vb2_is_streaming(&vfh->queue);
> -	mutex_unlock(&video->queue_lock);
> -
>  	if (!streaming)
> -		goto done;
> +		return 0;
> 
>  	/* Update the pipeline state. */
>  	if (video->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
> @@ -1229,19 +1194,13 @@ isp_video_streamoff(struct file *file, void *fh,
> enum v4l2_buf_type type)
>  	omap3isp_pipeline_set_stream(pipe, ISP_PIPELINE_STREAM_STOPPED);

With this patch, the omap3isp_pipeline_set_stream() function will wait for the 
pipeline to stop with the queue lock held. This means that all concurrent 
access to buffer-related functions (and in particular DQBUF and and poll()) 
will potentially wait for much longer than they do now. Could this be a 
problem ? Could waiting for the pipeline to stop with the lock held also cause 
other problems than that ?

>  	omap3isp_video_cancel_stream(video);
> 
> -	mutex_lock(&video->queue_lock);
>  	vb2_streamoff(&vfh->queue, type);
> -	mutex_unlock(&video->queue_lock);
>  	video->queue = NULL;
>  	video->error = false;
> 
>  	/* TODO: Implement PM QoS */
>  	media_pipeline_stop(&video->video.entity);
> -
>  	media_entity_enum_cleanup(&pipe->ent_enum);
> -
> -done:
> -	mutex_unlock(&video->stream_lock);
>  	return 0;
>  }
> 
> @@ -1333,6 +1292,7 @@ static int isp_video_open(struct file *file)
>  	queue->buf_struct_size = sizeof(struct isp_buffer);
>  	queue->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
>  	queue->dev = video->isp->dev;
> +	queue->lock = &video->queue_lock;
> 
>  	ret = vb2_queue_init(&handle->queue);
>  	if (ret < 0) {
> @@ -1363,10 +1323,9 @@ static int isp_video_release(struct file *file)
>  	struct v4l2_fh *vfh = file->private_data;
>  	struct isp_video_fh *handle = to_isp_video_fh(vfh);
> 
> +	mutex_lock(&video->queue_lock);
>  	/* Disable streaming and free the buffers queue resources. */
>  	isp_video_streamoff(file, vfh, video->type);
> -
> -	mutex_lock(&video->queue_lock);
>  	vb2_queue_release(&handle->queue);
>  	mutex_unlock(&video->queue_lock);
> 
> @@ -1449,7 +1408,6 @@ int omap3isp_video_init(struct isp_video *video, const
> char *name) atomic_set(&video->active, 0);
> 
>  	spin_lock_init(&video->pipe.lock);
> -	mutex_init(&video->stream_lock);
>  	mutex_init(&video->queue_lock);
>  	spin_lock_init(&video->irqlock);
> 
> @@ -1474,7 +1432,6 @@ void omap3isp_video_cleanup(struct isp_video *video)
>  {
>  	media_entity_cleanup(&video->video.entity);
>  	mutex_destroy(&video->queue_lock);
> -	mutex_destroy(&video->stream_lock);
>  	mutex_destroy(&video->mutex);
>  }
> 
> diff --git a/drivers/media/platform/omap3isp/ispvideo.h
> b/drivers/media/platform/omap3isp/ispvideo.h index
> f6a2082b4a0a..5a8fba85e0eb 100644
> --- a/drivers/media/platform/omap3isp/ispvideo.h
> +++ b/drivers/media/platform/omap3isp/ispvideo.h
> @@ -167,7 +167,6 @@ struct isp_video {
> 
>  	/* Pipeline state */
>  	struct isp_pipeline pipe;
> -	struct mutex stream_lock;	/* pipeline and stream states */

The queue_lock now covers more than just the queue, please update the comment 
to list the fields that are protected by the mutex.

>  	bool error;
> 
>  	/* Video buffers queue */

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v6 04/17] omap3isp: Add vb2_queue lock
  2018-07-02 16:49         ` Laurent Pinchart
@ 2018-07-06 15:11           ` Ezequiel Garcia
  0 siblings, 0 replies; 28+ messages in thread
From: Ezequiel Garcia @ 2018-07-06 15:11 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linux-media, Hans Verkuil, kernel, sakari.ailus

On Mon, 2018-07-02 at 19:49 +0300, Laurent Pinchart wrote:
> Hi Ezequiel,
> 
> (CC'ing Sakari)
> 
> Thank you for the patch.
> 
> On Friday, 22 June 2018 06:53:58 EEST Ezequiel Garcia wrote:
> > vb2_queue locks is now mandatory. Add it, remove driver ad-hoc
> > locks,
> > and implement wait_{prepare, finish}.
> > 
> > Also, remove stream_lock mutex. Since the ioctls operations
> > are now protected by the queue mutex, the stream_lock mutex is
> > not needed.
> > 
> > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > ---
> >  drivers/media/platform/omap3isp/ispvideo.c | 65 ++++------------
> > ------
> >  drivers/media/platform/omap3isp/ispvideo.h |  1 -
> >  2 files changed, 11 insertions(+), 55 deletions(-)
> > 
> > diff --git a/drivers/media/platform/omap3isp/ispvideo.c
> > b/drivers/media/platform/omap3isp/ispvideo.c index
> > 9d228eac24ea..f835aeb9ddc5 100644
> > --- a/drivers/media/platform/omap3isp/ispvideo.c
> > +++ b/drivers/media/platform/omap3isp/ispvideo.c
> > @@ -496,6 +496,8 @@ static const struct vb2_ops isp_video_queue_ops
> > = {
> >  	.buf_prepare = isp_video_buffer_prepare,
> >  	.buf_queue = isp_video_buffer_queue,
> >  	.start_streaming = isp_video_start_streaming,
> > +	.wait_prepare = vb2_ops_wait_prepare,
> > +	.wait_finish = vb2_ops_wait_finish,
> >  };
> > 
> >  /*
> > @@ -628,11 +630,8 @@ void omap3isp_video_resume(struct isp_video
> > *video, int
> > continuous) {
> >  	struct isp_buffer *buf = NULL;
> > 
> > -	if (continuous && video->type ==
> > V4L2_BUF_TYPE_VIDEO_CAPTURE) {
> > -		mutex_lock(&video->queue_lock);
> > +	if (continuous && video->type ==
> > V4L2_BUF_TYPE_VIDEO_CAPTURE)
> >  		vb2_discard_done(video->queue);
> > -		mutex_unlock(&video->queue_lock);
> > -	}
> 
> Why can locking be removed here ? As far as I know the lock isn't
> taken by the 
> caller. It would help to explain the rationale in the commit message.
> 

AFAICS, vb2_discard_done() simply iterates over a list, and it's
protected by a spinlock. It's already thread-safe.

> >  	if (!list_empty(&video->dmaqueue)) {
> >  		buf = list_first_entry(&video->dmaqueue,
> > @@ -909,13 +908,8 @@ isp_video_reqbufs(struct file *file, void *fh,
> > struct
> > v4l2_requestbuffers *rb) {
> >  	struct isp_video_fh *vfh = to_isp_video_fh(fh);
> >  	struct isp_video *video = video_drvdata(file);
> 
> The video variable isn't used anymore. Didn't gcc warn you ?
> 

Hm, I don't remember getting warnings.

> > -	int ret;
> > -
> > -	mutex_lock(&video->queue_lock);
> > -	ret = vb2_reqbufs(&vfh->queue, rb);
> > -	mutex_unlock(&video->queue_lock);
> > 
> > -	return ret;
> > +	return vb2_reqbufs(&vfh->queue, rb);
> >  }
> > 
> >  static int
> > @@ -923,13 +917,8 @@ isp_video_querybuf(struct file *file, void
> > *fh, struct
> > v4l2_buffer *b) {
> >  	struct isp_video_fh *vfh = to_isp_video_fh(fh);
> >  	struct isp_video *video = video_drvdata(file);
> 
> Same here and in other functions below.
> 
> > -	int ret;
> > 
> > -	mutex_lock(&video->queue_lock);
> > -	ret = vb2_querybuf(&vfh->queue, b);
> > -	mutex_unlock(&video->queue_lock);
> > -
> > -	return ret;
> > +	return vb2_querybuf(&vfh->queue, b);
> >  }
> > 
> >  static int
> > @@ -937,13 +926,8 @@ isp_video_qbuf(struct file *file, void *fh,
> > struct
> > v4l2_buffer *b) {
> >  	struct isp_video_fh *vfh = to_isp_video_fh(fh);
> >  	struct isp_video *video = video_drvdata(file);
> > -	int ret;
> > 
> > -	mutex_lock(&video->queue_lock);
> > -	ret = vb2_qbuf(&vfh->queue, b);
> > -	mutex_unlock(&video->queue_lock);
> > -
> > -	return ret;
> > +	return vb2_qbuf(&vfh->queue, b);
> >  }
> > 
> >  static int
> > @@ -951,13 +935,8 @@ isp_video_dqbuf(struct file *file, void *fh,
> > struct
> > v4l2_buffer *b) {
> >  	struct isp_video_fh *vfh = to_isp_video_fh(fh);
> >  	struct isp_video *video = video_drvdata(file);
> > -	int ret;
> > 
> > -	mutex_lock(&video->queue_lock);
> > -	ret = vb2_dqbuf(&vfh->queue, b, file->f_flags &
> > O_NONBLOCK);
> > -	mutex_unlock(&video->queue_lock);
> > -
> > -	return ret;
> > +	return vb2_dqbuf(&vfh->queue, b, file->f_flags &
> > O_NONBLOCK);
> >  }
> > 
> >  static int isp_video_check_external_subdevs(struct isp_video
> > *video,
> > @@ -1096,8 +1075,6 @@ isp_video_streamon(struct file *file, void
> > *fh, enum
> > v4l2_buf_type type) if (type != video->type)
> >  		return -EINVAL;
> > 
> > -	mutex_lock(&video->stream_lock);
> > -
> >  	/* Start streaming on the pipeline. No link touching an
> > entity in the
> >  	 * pipeline can be activated or deactivated once streaming
> > is started.
> >  	 */
> > @@ -1106,7 +1083,7 @@ isp_video_streamon(struct file *file, void
> > *fh, enum
> > v4l2_buf_type type)
> > 
> >  	ret = media_entity_enum_init(&pipe->ent_enum, &video->isp-
> > >media_dev);
> >  	if (ret)
> > -		goto err_enum_init;
> > +		return ret;
> > 
> >  	/* TODO: Implement PM QoS */
> >  	pipe->l3_ick = clk_get_rate(video->isp-
> > >clock[ISP_CLK_L3_ICK]);
> > @@ -1158,14 +1135,10 @@ isp_video_streamon(struct file *file, void
> > *fh, enum
> > v4l2_buf_type type) atomic_set(&pipe->frame_number, -1);
> >  	pipe->field = vfh->format.fmt.pix.field;
> > 
> > -	mutex_lock(&video->queue_lock);
> >  	ret = vb2_streamon(&vfh->queue, type);
> > -	mutex_unlock(&video->queue_lock);
> >  	if (ret < 0)
> >  		goto err_check_format;
> > 
> > -	mutex_unlock(&video->stream_lock);
> > -
> >  	return 0;
> > 
> >  err_check_format:
> > @@ -1184,9 +1157,6 @@ isp_video_streamon(struct file *file, void
> > *fh, enum
> > v4l2_buf_type type)
> > 
> >  	media_entity_enum_cleanup(&pipe->ent_enum);
> > 
> > -err_enum_init:
> > -	mutex_unlock(&video->stream_lock);
> > -
> >  	return ret;
> >  }
> > 
> > @@ -1203,15 +1173,10 @@ isp_video_streamoff(struct file *file, void
> > *fh,
> > enum v4l2_buf_type type) if (type != video->type)
> >  		return -EINVAL;
> > 
> > -	mutex_lock(&video->stream_lock);
> > -
> >  	/* Make sure we're not streaming yet. */
> > -	mutex_lock(&video->queue_lock);
> >  	streaming = vb2_is_streaming(&vfh->queue);
> > -	mutex_unlock(&video->queue_lock);
> > -
> >  	if (!streaming)
> > -		goto done;
> > +		return 0;
> > 
> >  	/* Update the pipeline state. */
> >  	if (video->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
> > @@ -1229,19 +1194,13 @@ isp_video_streamoff(struct file *file, void
> > *fh,
> > enum v4l2_buf_type type)
> >  	omap3isp_pipeline_set_stream(pipe,
> > ISP_PIPELINE_STREAM_STOPPED);
> 
> With this patch, the omap3isp_pipeline_set_stream() function will
> wait for the 
> pipeline to stop with the queue lock held. This means that all
> concurrent 
> access to buffer-related functions (and in particular DQBUF and and
> poll()) 
> will potentially wait for much longer than they do now. Could this be
> a 
> problem ? Could waiting for the pipeline to stop with the lock held
> also cause 
> other problems than that ?
> 

Yes, I can see that now. It will only cause problems if the subdev
calls, then try to take the queue lock, either directly or by re-
entering into the driver.

I don't have access to this hardware, so I do not have other answers.

> >  	omap3isp_video_cancel_stream(video);
> > 
> > -	mutex_lock(&video->queue_lock);
> >  	vb2_streamoff(&vfh->queue, type);
> > -	mutex_unlock(&video->queue_lock);
> >  	video->queue = NULL;
> >  	video->error = false;
> > 
> >  	/* TODO: Implement PM QoS */
> >  	media_pipeline_stop(&video->video.entity);
> > -
> >  	media_entity_enum_cleanup(&pipe->ent_enum);
> > -
> > -done:
> > -	mutex_unlock(&video->stream_lock);
> >  	return 0;
> >  }
> > 
> > @@ -1333,6 +1292,7 @@ static int isp_video_open(struct file *file)
> >  	queue->buf_struct_size = sizeof(struct isp_buffer);
> >  	queue->timestamp_flags =
> > V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> >  	queue->dev = video->isp->dev;
> > +	queue->lock = &video->queue_lock;
> > 
> >  	ret = vb2_queue_init(&handle->queue);
> >  	if (ret < 0) {
> > @@ -1363,10 +1323,9 @@ static int isp_video_release(struct file
> > *file)
> >  	struct v4l2_fh *vfh = file->private_data;
> >  	struct isp_video_fh *handle = to_isp_video_fh(vfh);
> > 
> > +	mutex_lock(&video->queue_lock);
> >  	/* Disable streaming and free the buffers queue resources.
> > */
> >  	isp_video_streamoff(file, vfh, video->type);
> > -
> > -	mutex_lock(&video->queue_lock);
> >  	vb2_queue_release(&handle->queue);
> >  	mutex_unlock(&video->queue_lock);
> > 
> > @@ -1449,7 +1408,6 @@ int omap3isp_video_init(struct isp_video
> > *video, const
> > char *name) atomic_set(&video->active, 0);
> > 
> >  	spin_lock_init(&video->pipe.lock);
> > -	mutex_init(&video->stream_lock);
> >  	mutex_init(&video->queue_lock);
> >  	spin_lock_init(&video->irqlock);
> > 
> > @@ -1474,7 +1432,6 @@ void omap3isp_video_cleanup(struct isp_video
> > *video)
> >  {
> >  	media_entity_cleanup(&video->video.entity);
> >  	mutex_destroy(&video->queue_lock);
> > -	mutex_destroy(&video->stream_lock);
> >  	mutex_destroy(&video->mutex);
> >  }
> > 
> > diff --git a/drivers/media/platform/omap3isp/ispvideo.h
> > b/drivers/media/platform/omap3isp/ispvideo.h index
> > f6a2082b4a0a..5a8fba85e0eb 100644
> > --- a/drivers/media/platform/omap3isp/ispvideo.h
> > +++ b/drivers/media/platform/omap3isp/ispvideo.h
> > @@ -167,7 +167,6 @@ struct isp_video {
> > 
> >  	/* Pipeline state */
> >  	struct isp_pipeline pipe;
> > -	struct mutex stream_lock;	/* pipeline and stream
> > states */
> 
> The queue_lock now covers more than just the queue, please update the
> comment 
> to list the fields that are protected by the mutex.
> 
> 

Right.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v4 03/17] omap4iss: Add vb2_queue lock
  2018-07-02  8:28   ` Hans Verkuil
@ 2018-07-06 15:24     ` Ezequiel Garcia
  0 siblings, 0 replies; 28+ messages in thread
From: Ezequiel Garcia @ 2018-07-06 15:24 UTC (permalink / raw)
  To: Hans Verkuil, linux-media, Laurent Pinchart, sakari.ailus
  Cc: Hans Verkuil, kernel

On Mon, 2018-07-02 at 10:28 +0200, Hans Verkuil wrote:
> On 15/06/18 21:07, Ezequiel Garcia wrote:
> > vb2_queue lock is now mandatory. Add it, remove driver ad-hoc
> > locks, and implement wait_{prepare, finish}.
> 
> I don't see any wait_prepare/finish implementation?!
> 
> 

Oops, seems it felt through the cracks.

Anyway, we need to solve the omap3 issues before going forward with
this series.

Thanks,
Eze

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2018-07-06 15:24 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-15 19:07 [PATCH v4 00/17] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
2018-06-15 19:07 ` [PATCH v4 01/17] v4l2-ioctl.c: use correct vb2_queue lock for m2m devices Ezequiel Garcia
2018-06-15 19:07 ` [PATCH v4 02/17] sta2x11: Add video_device and vb2_queue locks Ezequiel Garcia
2018-06-15 19:07 ` [PATCH v4 03/17] omap4iss: Add vb2_queue lock Ezequiel Garcia
2018-07-02  8:28   ` Hans Verkuil
2018-07-06 15:24     ` Ezequiel Garcia
2018-06-15 19:07 ` [PATCH v4 04/17] omap3isp: " Ezequiel Garcia
2018-06-18  7:31   ` Hans Verkuil
2018-06-20 17:42     ` [PATCH v5 " Ezequiel Garcia
2018-06-21  8:18       ` Hans Verkuil
2018-06-22  3:51         ` Ezequiel Garcia
2018-06-22  3:53       ` [PATCH v6 " Ezequiel Garcia
2018-07-02 16:49         ` Laurent Pinchart
2018-07-06 15:11           ` Ezequiel Garcia
2018-06-15 19:07 ` [PATCH v4 05/17] mtk-mdp: Add locks for capture and output vb2_queues Ezequiel Garcia
2018-06-15 19:07 ` [PATCH v4 06/17] s5p-g2d: Implement wait_prepare and wait_finish Ezequiel Garcia
2018-06-15 19:07 ` [PATCH v4 07/17] staging: bcm2835-camera: Provide lock for vb2_queue Ezequiel Garcia
2018-06-15 19:07 ` [PATCH v4 08/17] venus: Add video_device and vb2_queue locks Ezequiel Garcia
2018-06-15 19:07 ` [PATCH v4 09/17] davinci_vpfe: " Ezequiel Garcia
2018-06-15 19:07 ` [PATCH v4 10/17] mx_emmaprp: Implement wait_prepare and wait_finish Ezequiel Garcia
2018-06-15 19:07 ` [PATCH v4 11/17] m2m-deinterlace: " Ezequiel Garcia
2018-06-15 19:07 ` [PATCH v4 12/17] stk1160: Set the vb2_queue lock before calling vb2_queue_init Ezequiel Garcia
2018-06-15 19:07 ` [PATCH v4 13/17] dvb-core: Provide lock for vb2_queue Ezequiel Garcia
2018-06-15 19:07 ` [PATCH v4 14/17] videobuf2-core: require q->lock Ezequiel Garcia
2018-06-15 19:07 ` [PATCH v4 15/17] videobuf2: assume q->lock is always set Ezequiel Garcia
2018-06-15 19:07 ` [PATCH v4 16/17] v4l2-ioctl.c: assume queue->lock " Ezequiel Garcia
2018-06-15 19:07 ` [PATCH v4 17/17] media: Remove wait_{prepare, finish} Ezequiel Garcia
2018-06-29 18:12 ` [PATCH v4 00/17] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.