All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH v2 00/20] v4l2 core: push ioctl lock down to ioctl handler
@ 2018-05-18 18:51 Ezequiel Garcia
  2018-05-18 18:51 ` [PATCH 01/20] pvrusb2: replace pvr2_v4l2_ioctl by video_ioctl2 Ezequiel Garcia
                   ` (19 more replies)
  0 siblings, 20 replies; 23+ messages in thread
From: Ezequiel Garcia @ 2018-05-18 18:51 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Abylay Ospan, Ezequiel Garcia

Here's a second spin of the series posted by Hans:

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

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

A only exception to this is netup_unidvb. It isn't really obvious
to me how this driver should lock its vb2_queue. Neither it is
clear how its vb2_queue is used by the driver in the first place.

Abylay, perhaps you can take a look at it?

Why?
----

While working on the DMA fence API (aka expliciy sync framework)
and the Request API it became clear that the core ioctl scheme
was done at a too-high level.

Being able to actually look at the struct passed as the ioctl
argument would help a lot in decide what lock(s) to take.

This patch series pushes the lock down into v4l2-ioctl.c, after
video_usercopy() was called.

This series seems to improve overall quality of drivers:
in practice, drivers choosing to do their own locking, end up
introducing races and/or not setting wait_prepare/wait_finish
despite being possible to do so.

Patch journal
-------------

The first patch is for the only driver that does not set
unlocked_ioctl to video_ioctl2: pvrusb2. It actually does
call it in its own unlocked_ioctl function.

The second patch pushes the lock down.

The third patch adds support for mem2mem devices by selecting
the correct queue lock (capture vs output): this was never
possible before.

Patches 4 to 16 add the now mandatory vb2_queue lock and then
sets wait_prepare and wait_finish hooks.

Patches 17 to 19 require that queue->lock is always set. This
means wait_prepare and wait_finish is now unused.

The last patch removes the now unused wait_prepare and wait_finish.

This patchset is currently based on top of the gspca vb2
conversion series, hoping it would get merged sooner than this.

Ezequiel Garcia (14):
  usbtv: Implement wait_prepare and wait_finish
  sta2x11: Add video_device and vb2_queue locks
  omap4iss: Add video_device and vb2_queue locks
  omap3isp: Add video_device and vb2_queue locks
  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
  dvb-core: 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
  media: Remove wait_{prepare, finish}

Hans Verkuil (6):
  pvrusb2: replace pvr2_v4l2_ioctl by video_ioctl2
  v4l2-core: push taking ioctl mutex down to ioctl handler.
  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 -
 drivers/media/common/videobuf2/videobuf2-core.c    | 22 +++---
 drivers/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/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            |  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 -
 drivers/media/platform/exynos4-is/fimc-capture.c   |  2 -
 drivers/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 +
 drivers/media/platform/marvell-ccic/mcam-core.c    |  4 -
 drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c    |  2 -
 drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c       | 18 +----
 drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c |  2 -
 drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c |  2 -
 drivers/media/platform/mx2_emmaprp.c               |  2 +
 drivers/media/platform/omap3isp/ispvideo.c         | 92 +++-------------------
 drivers/media/platform/omap3isp/ispvideo.h         |  3 +-
 drivers/media/platform/pxa_camera.c                |  2 -
 .../media/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 -
 drivers/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 -
 .../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/pvrusb2/pvrusb2-v4l2.c           | 83 ++++++++-----------
 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/uvc/uvc_queue.c                  |  4 -
 drivers/media/v4l2-core/v4l2-dev.c                 |  6 --
 drivers/media/v4l2-core/v4l2-ioctl.c               | 75 ++++++++++++++++--
 drivers/media/v4l2-core/v4l2-subdev.c              | 17 +++-
 drivers/staging/media/davinci_vpfe/vpfe_video.c    |  4 +-
 drivers/staging/media/davinci_vpfe/vpfe_video.h    |  2 +-
 drivers/staging/media/imx/imx-media-capture.c      |  2 -
 drivers/staging/media/omap4iss/iss_video.c         | 30 ++-----
 drivers/staging/media/omap4iss/iss_video.h         |  2 +-
 .../vc04_services/bcm2835-camera/bcm2835-camera.c  | 22 +-----
 drivers/usb/gadget/function/uvc_queue.c            |  2 -
 include/media/v4l2-dev.h                           |  9 ---
 include/media/v4l2-ioctl.h                         | 12 ---
 include/media/videobuf2-core.h                     |  2 -
 include/media/videobuf2-v4l2.h                     | 18 -----
 samples/v4l/v4l2-pci-skeleton.c                    |  7 --
 115 files changed, 200 insertions(+), 546 deletions(-)

-- 
2.16.3

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

* [PATCH 01/20] pvrusb2: replace pvr2_v4l2_ioctl by video_ioctl2
  2018-05-18 18:51 [RFC PATCH v2 00/20] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
@ 2018-05-18 18:51 ` Ezequiel Garcia
  2018-05-18 18:51 ` [PATCH 02/20] v4l2-core: push taking ioctl mutex down to ioctl handler Ezequiel Garcia
                   ` (18 subsequent siblings)
  19 siblings, 0 replies; 23+ messages in thread
From: Ezequiel Garcia @ 2018-05-18 18:51 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Abylay Ospan, Hans Verkuil

From: Hans Verkuil <hansverk@cisco.com>

This driver is the only V4L driver that does not set unlocked_ioctl
to video_ioctl2.

The only thing that pvr2_v4l2_ioctl does besides calling video_ioctl2
is calling pvr2_hdw_commit_ctl(). Add pvr2_hdw_commit_ctl() calls to
the various ioctls that need this, and we can replace pvr2_v4l2_ioctl
by video_ioctl2.

Signed-off-by: Hans Verkuil <hansverk@cisco.com>
Tested-by: Hans Verkuil <hansverk@cisco.com>
---
 drivers/media/usb/pvrusb2/pvrusb2-v4l2.c | 83 ++++++++++++--------------------
 1 file changed, 31 insertions(+), 52 deletions(-)

diff --git a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
index 9fdc57c1658f..e53a80b589a1 100644
--- a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
+++ b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
@@ -159,9 +159,12 @@ static int pvr2_s_std(struct file *file, void *priv, v4l2_std_id std)
 {
 	struct pvr2_v4l2_fh *fh = file->private_data;
 	struct pvr2_hdw *hdw = fh->channel.mc_head->hdw;
+	int ret;
 
-	return pvr2_ctrl_set_value(
+	ret = pvr2_ctrl_set_value(
 		pvr2_hdw_get_ctrl_by_id(hdw, PVR2_CID_STDCUR), std);
+	pvr2_hdw_commit_ctl(hdw);
+	return ret;
 }
 
 static int pvr2_querystd(struct file *file, void *priv, v4l2_std_id *std)
@@ -251,12 +254,15 @@ static int pvr2_s_input(struct file *file, void *priv, unsigned int inp)
 {
 	struct pvr2_v4l2_fh *fh = file->private_data;
 	struct pvr2_hdw *hdw = fh->channel.mc_head->hdw;
+	int ret;
 
 	if (inp >= fh->input_cnt)
 		return -EINVAL;
-	return pvr2_ctrl_set_value(
+	ret = pvr2_ctrl_set_value(
 			pvr2_hdw_get_ctrl_by_id(hdw, PVR2_CID_INPUT),
 			fh->input_map[inp]);
+	pvr2_hdw_commit_ctl(hdw);
+	return ret;
 }
 
 static int pvr2_enumaudio(struct file *file, void *priv, struct v4l2_audio *vin)
@@ -315,13 +321,16 @@ static int pvr2_s_tuner(struct file *file, void *priv, const struct v4l2_tuner *
 {
 	struct pvr2_v4l2_fh *fh = file->private_data;
 	struct pvr2_hdw *hdw = fh->channel.mc_head->hdw;
+	int ret;
 
 	if (vt->index != 0)
 		return -EINVAL;
 
-	return pvr2_ctrl_set_value(
+	ret = pvr2_ctrl_set_value(
 			pvr2_hdw_get_ctrl_by_id(hdw, PVR2_CID_AUDIOMODE),
 			vt->audmode);
+	pvr2_hdw_commit_ctl(hdw);
+	return ret;
 }
 
 static int pvr2_s_frequency(struct file *file, void *priv, const struct v4l2_frequency *vf)
@@ -353,8 +362,10 @@ static int pvr2_s_frequency(struct file *file, void *priv, const struct v4l2_fre
 		fv = (fv * 125) / 2;
 	else
 		fv = fv * 62500;
-	return pvr2_ctrl_set_value(
+	ret = pvr2_ctrl_set_value(
 			pvr2_hdw_get_ctrl_by_id(hdw,PVR2_CID_FREQUENCY),fv);
+	pvr2_hdw_commit_ctl(hdw);
+	return ret;
 }
 
 static int pvr2_g_frequency(struct file *file, void *priv, struct v4l2_frequency *vf)
@@ -470,6 +481,7 @@ static int pvr2_s_fmt_vid_cap(struct file *file, void *priv, struct v4l2_format
 	vcp = pvr2_hdw_get_ctrl_by_id(hdw, PVR2_CID_VRES);
 	pvr2_ctrl_set_value(hcp, vf->fmt.pix.width);
 	pvr2_ctrl_set_value(vcp, vf->fmt.pix.height);
+	pvr2_hdw_commit_ctl(hdw);
 	return 0;
 }
 
@@ -597,9 +609,12 @@ static int pvr2_s_ctrl(struct file *file, void *priv, struct v4l2_control *vc)
 {
 	struct pvr2_v4l2_fh *fh = file->private_data;
 	struct pvr2_hdw *hdw = fh->channel.mc_head->hdw;
+	int ret;
 
-	return pvr2_ctrl_set_value(pvr2_hdw_get_ctrl_v4l(hdw, vc->id),
+	ret = pvr2_ctrl_set_value(pvr2_hdw_get_ctrl_v4l(hdw, vc->id),
 			vc->value);
+	pvr2_hdw_commit_ctl(hdw);
+	return ret;
 }
 
 static int pvr2_g_ext_ctrls(struct file *file, void *priv,
@@ -658,10 +673,12 @@ static int pvr2_s_ext_ctrls(struct file *file, void *priv,
 				ctrl->value);
 		if (ret) {
 			ctls->error_idx = idx;
-			return ret;
+			goto commit;
 		}
 	}
-	return 0;
+commit:
+	pvr2_hdw_commit_ctl(hdw);
+	return ret;
 }
 
 static int pvr2_try_ext_ctrls(struct file *file, void *priv,
@@ -764,23 +781,23 @@ static int pvr2_s_selection(struct file *file, void *priv,
 			pvr2_hdw_get_ctrl_by_id(hdw, PVR2_CID_CROPL),
 			sel->r.left);
 	if (ret != 0)
-		return -EINVAL;
+		goto commit;
 	ret = pvr2_ctrl_set_value(
 			pvr2_hdw_get_ctrl_by_id(hdw, PVR2_CID_CROPT),
 			sel->r.top);
 	if (ret != 0)
-		return -EINVAL;
+		goto commit;
 	ret = pvr2_ctrl_set_value(
 			pvr2_hdw_get_ctrl_by_id(hdw, PVR2_CID_CROPW),
 			sel->r.width);
 	if (ret != 0)
-		return -EINVAL;
+		goto commit;
 	ret = pvr2_ctrl_set_value(
 			pvr2_hdw_get_ctrl_by_id(hdw, PVR2_CID_CROPH),
 			sel->r.height);
-	if (ret != 0)
-		return -EINVAL;
-	return 0;
+commit:
+	pvr2_hdw_commit_ctl(hdw);
+	return ret;
 }
 
 static int pvr2_log_status(struct file *file, void *priv)
@@ -905,44 +922,6 @@ static void pvr2_v4l2_internal_check(struct pvr2_channel *chp)
 }
 
 
-static long pvr2_v4l2_ioctl(struct file *file,
-			   unsigned int cmd, unsigned long arg)
-{
-
-	struct pvr2_v4l2_fh *fh = file->private_data;
-	struct pvr2_hdw *hdw = fh->channel.mc_head->hdw;
-	long ret = -EINVAL;
-
-	if (pvrusb2_debug & PVR2_TRACE_V4LIOCTL)
-		v4l_printk_ioctl(pvr2_hdw_get_driver_name(hdw), cmd);
-
-	if (!pvr2_hdw_dev_ok(hdw)) {
-		pvr2_trace(PVR2_TRACE_ERROR_LEGS,
-			   "ioctl failed - bad or no context");
-		return -EFAULT;
-	}
-
-	ret = video_ioctl2(file, cmd, arg);
-
-	pvr2_hdw_commit_ctl(hdw);
-
-	if (ret < 0) {
-		if (pvrusb2_debug & PVR2_TRACE_V4LIOCTL) {
-			pvr2_trace(PVR2_TRACE_V4LIOCTL,
-				   "pvr2_v4l2_do_ioctl failure, ret=%ld command was:",
-ret);
-			v4l_printk_ioctl(pvr2_hdw_get_driver_name(hdw), cmd);
-		}
-	} else {
-		pvr2_trace(PVR2_TRACE_V4LIOCTL,
-			   "pvr2_v4l2_do_ioctl complete, ret=%ld (0x%lx)",
-			   ret, ret);
-	}
-	return ret;
-
-}
-
-
 static int pvr2_v4l2_release(struct file *file)
 {
 	struct pvr2_v4l2_fh *fhp = file->private_data;
@@ -1205,7 +1184,7 @@ static const struct v4l2_file_operations vdev_fops = {
 	.open       = pvr2_v4l2_open,
 	.release    = pvr2_v4l2_release,
 	.read       = pvr2_v4l2_read,
-	.unlocked_ioctl = pvr2_v4l2_ioctl,
+	.unlocked_ioctl = video_ioctl2,
 	.poll       = pvr2_v4l2_poll,
 };
 
-- 
2.16.3

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

* [PATCH 02/20] v4l2-core: push taking ioctl mutex down to ioctl handler.
  2018-05-18 18:51 [RFC PATCH v2 00/20] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
  2018-05-18 18:51 ` [PATCH 01/20] pvrusb2: replace pvr2_v4l2_ioctl by video_ioctl2 Ezequiel Garcia
@ 2018-05-18 18:51 ` Ezequiel Garcia
  2018-05-18 18:51 ` [PATCH 03/20] v4l2-ioctl.c: use correct vb2_queue lock for m2m devices Ezequiel Garcia
                   ` (17 subsequent siblings)
  19 siblings, 0 replies; 23+ messages in thread
From: Ezequiel Garcia @ 2018-05-18 18:51 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Abylay Ospan, Hans Verkuil

From: Hans Verkuil <hansverk@cisco.com>

The ioctl serialization mutex (vdev->lock or q->lock for vb2 queues)
was taken at the highest level in v4l2-dev.c. This prevents more
fine-grained locking since at that level we cannot examine the ioctl
arguments, we can only do that after video_usercopy is called.

So push the locking down to __video_do_ioctl() and subdev_do_ioctl_lock().

This also allows us to make a few functions in v4l2-ioctl.c static and
video_usercopy() is no longer exported.

The locking scheme is not changed by this patch, just pushed down.

Signed-off-by: Hans Verkuil <hansverk@cisco.com>
---
 drivers/media/v4l2-core/v4l2-dev.c    |  6 ------
 drivers/media/v4l2-core/v4l2-ioctl.c  | 17 ++++++++++++++---
 drivers/media/v4l2-core/v4l2-subdev.c | 17 ++++++++++++++++-
 include/media/v4l2-dev.h              |  9 ---------
 include/media/v4l2-ioctl.h            | 12 ------------
 5 files changed, 30 insertions(+), 31 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index 1d0b2208e8fb..baf8cd549128 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -352,14 +352,8 @@ static long v4l2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 	int ret = -ENODEV;
 
 	if (vdev->fops->unlocked_ioctl) {
-		struct mutex *lock = v4l2_ioctl_get_lock(vdev, cmd);
-
-		if (lock && mutex_lock_interruptible(lock))
-			return -ERESTARTSYS;
 		if (video_is_registered(vdev))
 			ret = vdev->fops->unlocked_ioctl(filp, cmd, arg);
-		if (lock)
-			mutex_unlock(lock);
 	} else
 		ret = -ENOTTY;
 
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 63de1c7134cc..de1b868500f3 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -2634,14 +2634,14 @@ static struct v4l2_ioctl_info v4l2_ioctls[] = {
 };
 #define V4L2_IOCTLS ARRAY_SIZE(v4l2_ioctls)
 
-bool v4l2_is_known_ioctl(unsigned int cmd)
+static bool v4l2_is_known_ioctl(unsigned int cmd)
 {
 	if (_IOC_NR(cmd) >= V4L2_IOCTLS)
 		return false;
 	return v4l2_ioctls[_IOC_NR(cmd)].ioctl == cmd;
 }
 
-struct mutex *v4l2_ioctl_get_lock(struct video_device *vdev, unsigned cmd)
+static struct mutex *v4l2_ioctl_get_lock(struct video_device *vdev, unsigned cmd)
 {
 	if (_IOC_NR(cmd) >= V4L2_IOCTLS)
 		return vdev->lock;
@@ -2692,6 +2692,7 @@ static long __video_do_ioctl(struct file *file,
 		unsigned int cmd, void *arg)
 {
 	struct video_device *vfd = video_devdata(file);
+	struct mutex *lock = v4l2_ioctl_get_lock(vfd, cmd);
 	const struct v4l2_ioctl_ops *ops = vfd->ioctl_ops;
 	bool write_only = false;
 	struct v4l2_ioctl_info default_info;
@@ -2710,6 +2711,14 @@ static long __video_do_ioctl(struct file *file,
 	if (test_bit(V4L2_FL_USES_V4L2_FH, &vfd->flags))
 		vfh = file->private_data;
 
+	if (lock && mutex_lock_interruptible(lock))
+		return -ERESTARTSYS;
+
+	if (!video_is_registered(vfd)) {
+		ret = -ENODEV;
+		goto unlock;
+	}
+
 	if (v4l2_is_known_ioctl(cmd)) {
 		info = &v4l2_ioctls[_IOC_NR(cmd)];
 
@@ -2765,6 +2774,9 @@ static long __video_do_ioctl(struct file *file,
 		}
 	}
 
+unlock:
+	if (lock)
+		mutex_unlock(lock);
 	return ret;
 }
 
@@ -2954,7 +2966,6 @@ video_usercopy(struct file *file, unsigned int cmd, unsigned long arg,
 	kvfree(mbuf);
 	return err;
 }
-EXPORT_SYMBOL(video_usercopy);
 
 long video_ioctl2(struct file *file,
 	       unsigned int cmd, unsigned long arg)
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index f9eed938d348..6a7f7f75dfd7 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -502,10 +502,25 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 	return 0;
 }
 
+static long subdev_do_ioctl_lock(struct file *file, unsigned int cmd, void *arg)
+{
+	struct video_device *vdev = video_devdata(file);
+	struct mutex *lock = vdev->lock;
+	long ret = -ENODEV;
+
+	if (lock && mutex_lock_interruptible(lock))
+		return -ERESTARTSYS;
+	if (video_is_registered(vdev))
+		ret = subdev_do_ioctl(file, cmd, arg);
+	if (lock)
+		mutex_unlock(lock);
+	return ret;
+}
+
 static long subdev_ioctl(struct file *file, unsigned int cmd,
 	unsigned long arg)
 {
-	return video_usercopy(file, cmd, arg, subdev_do_ioctl);
+	return video_usercopy(file, cmd, arg, subdev_do_ioctl_lock);
 }
 
 #ifdef CONFIG_COMPAT
diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
index 4546958eeba0..3a0be40749bc 100644
--- a/include/media/v4l2-dev.h
+++ b/include/media/v4l2-dev.h
@@ -434,15 +434,6 @@ void video_device_release(struct video_device *vdev);
  */
 void video_device_release_empty(struct video_device *vdev);
 
-/**
- * v4l2_is_known_ioctl - Checks if a given cmd is a known V4L ioctl
- *
- * @cmd: ioctl command
- *
- * returns true if cmd is a known V4L2 ioctl
- */
-bool v4l2_is_known_ioctl(unsigned int cmd);
-
 /**
  * v4l2_disable_ioctl- mark that a given command isn't implemented.
  *	shouldn't use core locking
diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
index a7b3f7c75d62..a8dbf5b54b5c 100644
--- a/include/media/v4l2-ioctl.h
+++ b/include/media/v4l2-ioctl.h
@@ -658,18 +658,6 @@ void v4l_printk_ioctl(const char *prefix, unsigned int cmd);
 
 struct video_device;
 
-
-/**
- * v4l2_ioctl_get_lock - get the mutex (if any) that it is need to lock for
- *	a given command.
- *
- * @vdev: Pointer to struct &video_device.
- * @cmd: Ioctl name.
- *
- * .. note:: Internal use only. Should not be used outside V4L2 core.
- */
-struct mutex *v4l2_ioctl_get_lock(struct video_device *vdev, unsigned int cmd);
-
 /* names for fancy debug output */
 extern const char *v4l2_field_names[];
 extern const char *v4l2_type_names[];
-- 
2.16.3

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

* [PATCH 03/20] v4l2-ioctl.c: use correct vb2_queue lock for m2m devices
  2018-05-18 18:51 [RFC PATCH v2 00/20] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
  2018-05-18 18:51 ` [PATCH 01/20] pvrusb2: replace pvr2_v4l2_ioctl by video_ioctl2 Ezequiel Garcia
  2018-05-18 18:51 ` [PATCH 02/20] v4l2-core: push taking ioctl mutex down to ioctl handler Ezequiel Garcia
@ 2018-05-18 18:51 ` Ezequiel Garcia
  2018-05-18 18:51 ` [PATCH 04/20] usbtv: Implement wait_prepare and wait_finish Ezequiel Garcia
                   ` (16 subsequent siblings)
  19 siblings, 0 replies; 23+ messages in thread
From: Ezequiel Garcia @ 2018-05-18 18:51 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Abylay Ospan, 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, 57 insertions(+), 2 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index de1b868500f3..ee1eec136e55 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>
 
@@ -2641,10 +2642,62 @@ static bool v4l2_is_known_ioctl(unsigned int cmd)
 	return v4l2_ioctls[_IOC_NR(cmd)].ioctl == cmd;
 }
 
-static struct mutex *v4l2_ioctl_get_lock(struct video_device *vdev, unsigned 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,
+					 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;
@@ -2692,7 +2745,7 @@ static long __video_do_ioctl(struct file *file,
 		unsigned int cmd, void *arg)
 {
 	struct video_device *vfd = video_devdata(file);
-	struct mutex *lock = v4l2_ioctl_get_lock(vfd, cmd);
+	struct mutex *lock;
 	const struct v4l2_ioctl_ops *ops = vfd->ioctl_ops;
 	bool write_only = false;
 	struct v4l2_ioctl_info default_info;
@@ -2711,6 +2764,8 @@ 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, vfh, cmd, arg);
+
 	if (lock && mutex_lock_interruptible(lock))
 		return -ERESTARTSYS;
 
-- 
2.16.3

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

* [PATCH 04/20] usbtv: Implement wait_prepare and wait_finish
  2018-05-18 18:51 [RFC PATCH v2 00/20] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
                   ` (2 preceding siblings ...)
  2018-05-18 18:51 ` [PATCH 03/20] v4l2-ioctl.c: use correct vb2_queue lock for m2m devices Ezequiel Garcia
@ 2018-05-18 18:51 ` Ezequiel Garcia
  2018-05-18 18:51 ` [PATCH 05/20] sta2x11: Add video_device and vb2_queue locks Ezequiel Garcia
                   ` (15 subsequent siblings)
  19 siblings, 0 replies; 23+ messages in thread
From: Ezequiel Garcia @ 2018-05-18 18:51 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Abylay Ospan, 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/usb/usbtv/usbtv-video.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/usb/usbtv/usbtv-video.c b/drivers/media/usb/usbtv/usbtv-video.c
index 3668a04359e8..0c0d0ef71573 100644
--- a/drivers/media/usb/usbtv/usbtv-video.c
+++ b/drivers/media/usb/usbtv/usbtv-video.c
@@ -698,6 +698,8 @@ 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)
-- 
2.16.3

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

* [PATCH 05/20] sta2x11: Add video_device and vb2_queue locks
  2018-05-18 18:51 [RFC PATCH v2 00/20] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
                   ` (3 preceding siblings ...)
  2018-05-18 18:51 ` [PATCH 04/20] usbtv: Implement wait_prepare and wait_finish Ezequiel Garcia
@ 2018-05-18 18:51 ` Ezequiel Garcia
  2018-05-18 18:51 ` [PATCH 06/20] omap4iss: " Ezequiel Garcia
                   ` (14 subsequent siblings)
  19 siblings, 0 replies; 23+ messages in thread
From: Ezequiel Garcia @ 2018-05-18 18:51 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Abylay Ospan, 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>
---
It's not really related to this commit, but there is no
locking around vip->active in vip_irq() ?
Perhaps it's a non-issue, but it looks fishy.

vip_irq()
{
	// ...
        if (vip->active) { /* Acquisition is over on this buffer */
		// ...
                vb2_buffer_done(&vip->active->vb.vb2_buf, VB2_BUF_STATE_DONE);
        }
}
---
 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 dd199bfc1d45..d47b99c2100f 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;
@@ -1035,6 +1039,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)
@@ -1081,6 +1086,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.16.3

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

* [PATCH 06/20] omap4iss: Add video_device and vb2_queue locks
  2018-05-18 18:51 [RFC PATCH v2 00/20] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
                   ` (4 preceding siblings ...)
  2018-05-18 18:51 ` [PATCH 05/20] sta2x11: Add video_device and vb2_queue locks Ezequiel Garcia
@ 2018-05-18 18:51 ` Ezequiel Garcia
  2018-05-22  9:09   ` Hans Verkuil
  2018-05-18 18:51 ` [PATCH 07/20] omap3isp: " Ezequiel Garcia
                   ` (13 subsequent siblings)
  19 siblings, 1 reply; 23+ messages in thread
From: Ezequiel Garcia @ 2018-05-18 18:51 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Abylay Ospan, Ezequiel Garcia

video_device and vb2_queue locks are now both
mandatory. Add them, remove driver ad-hoc locks,
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/staging/media/omap4iss/iss_video.c | 32 +++++++-----------------------
 drivers/staging/media/omap4iss/iss_video.h |  2 +-
 2 files changed, 8 insertions(+), 26 deletions(-)

diff --git a/drivers/staging/media/omap4iss/iss_video.c b/drivers/staging/media/omap4iss/iss_video.c
index a3a83424a926..380cfd230262 100644
--- a/drivers/staging/media/omap4iss/iss_video.c
+++ b/drivers/staging/media/omap4iss/iss_video.c
@@ -260,10 +260,7 @@ __iss_video_get_format(struct iss_video *video,
 	fmt.pad = pad;
 	fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
 
-	mutex_lock(&video->mutex);
 	ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt);
-	mutex_unlock(&video->mutex);
-
 	if (ret)
 		return ret;
 
@@ -411,6 +408,8 @@ static const struct vb2_ops iss_video_vb2ops = {
 	.buf_prepare	= iss_video_buf_prepare,
 	.buf_queue	= iss_video_buf_queue,
 	.buf_cleanup	= iss_video_buf_cleanup,
+	.wait_prepare	= vb2_ops_wait_prepare,
+	.wait_finish	= vb2_ops_wait_finish,
 };
 
 /*
@@ -592,9 +591,7 @@ iss_video_get_format(struct file *file, void *fh, struct v4l2_format *format)
 	if (format->type != video->type)
 		return -EINVAL;
 
-	mutex_lock(&video->mutex);
 	*format = vfh->format;
-	mutex_unlock(&video->mutex);
 
 	return 0;
 }
@@ -609,8 +606,6 @@ iss_video_set_format(struct file *file, void *fh, struct v4l2_format *format)
 	if (format->type != video->type)
 		return -EINVAL;
 
-	mutex_lock(&video->mutex);
-
 	/*
 	 * Fill the bytesperline and sizeimage fields by converting to media bus
 	 * format and back to pixel format.
@@ -620,7 +615,6 @@ iss_video_set_format(struct file *file, void *fh, struct v4l2_format *format)
 
 	vfh->format = *format;
 
-	mutex_unlock(&video->mutex);
 	return 0;
 }
 
@@ -741,9 +735,7 @@ iss_video_set_selection(struct file *file, void *fh, struct v4l2_selection *sel)
 		return -EINVAL;
 
 	sdsel.pad = pad;
-	mutex_lock(&video->mutex);
 	ret = v4l2_subdev_call(subdev, pad, set_selection, NULL, &sdsel);
-	mutex_unlock(&video->mutex);
 	if (!ret)
 		sel->r = sdsel.r;
 
@@ -873,8 +865,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 +968,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 +984,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 +999,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 +1025,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 +1119,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->v4l_lock;
 
 	ret = vb2_queue_init(q);
 	if (ret) {
@@ -1238,12 +1221,11 @@ int omap4iss_video_init(struct iss_video *video, const char *name)
 	if (ret < 0)
 		return ret;
 
+	mutex_init(&video->v4l_lock);
 	spin_lock_init(&video->qlock);
-	mutex_init(&video->mutex);
 	atomic_set(&video->active, 0);
 
 	spin_lock_init(&video->pipe.lock);
-	mutex_init(&video->stream_lock);
 
 	/* Initialize the video device. */
 	if (!video->ops)
@@ -1252,6 +1234,7 @@ int omap4iss_video_init(struct iss_video *video, const char *name)
 	video->video.fops = &iss_video_fops;
 	snprintf(video->video.name, sizeof(video->video.name),
 		 "OMAP4 ISS %s %s", name, direction);
+	video->video.lock = &video->v4l_lock;
 	video->video.vfl_type = VFL_TYPE_GRABBER;
 	video->video.release = video_device_release_empty;
 	video->video.ioctl_ops = &iss_video_ioctl_ops;
@@ -1265,8 +1248,7 @@ int omap4iss_video_init(struct iss_video *video, const char *name)
 void omap4iss_video_cleanup(struct iss_video *video)
 {
 	media_entity_cleanup(&video->video.entity);
-	mutex_destroy(&video->stream_lock);
-	mutex_destroy(&video->mutex);
+	mutex_destroy(&video->v4l_lock);
 }
 
 int omap4iss_video_register(struct iss_video *video, struct v4l2_device *vdev)
diff --git a/drivers/staging/media/omap4iss/iss_video.h b/drivers/staging/media/omap4iss/iss_video.h
index d7e05d04512c..4b8e5a8073fb 100644
--- a/drivers/staging/media/omap4iss/iss_video.h
+++ b/drivers/staging/media/omap4iss/iss_video.h
@@ -148,8 +148,8 @@ struct iss_video {
 	enum v4l2_buf_type type;
 	struct media_pad pad;
 
-	struct mutex mutex;		/* format and crop settings */
 	atomic_t active;
+	struct mutex v4l_lock;
 
 	struct iss_device *iss;
 
-- 
2.16.3

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

* [PATCH 07/20] omap3isp: Add video_device and vb2_queue locks
  2018-05-18 18:51 [RFC PATCH v2 00/20] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
                   ` (5 preceding siblings ...)
  2018-05-18 18:51 ` [PATCH 06/20] omap4iss: " Ezequiel Garcia
@ 2018-05-18 18:51 ` Ezequiel Garcia
  2018-05-18 18:51 ` [PATCH 08/20] mtk-mdp: Add locks for capture and output vb2_queues Ezequiel Garcia
                   ` (12 subsequent siblings)
  19 siblings, 0 replies; 23+ messages in thread
From: Ezequiel Garcia @ 2018-05-18 18:51 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Abylay Ospan, Ezequiel Garcia

video_device and vb2_queue locks are now both
mandatory. Add them, remove driver ad-hoc locks,
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/omap3isp/ispvideo.c | 94 +++++-------------------------
 drivers/media/platform/omap3isp/ispvideo.h |  3 +-
 2 files changed, 15 insertions(+), 82 deletions(-)

diff --git a/drivers/media/platform/omap3isp/ispvideo.c b/drivers/media/platform/omap3isp/ispvideo.c
index 9d228eac24ea..0e73f43ffa2d 100644
--- a/drivers/media/platform/omap3isp/ispvideo.c
+++ b/drivers/media/platform/omap3isp/ispvideo.c
@@ -292,10 +292,7 @@ __isp_video_get_format(struct isp_video *video, struct v4l2_format *format)
 	fmt.pad = pad;
 	fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
 
-	mutex_lock(&video->mutex);
 	ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt);
-	mutex_unlock(&video->mutex);
-
 	if (ret)
 		return ret;
 
@@ -496,6 +493,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 +627,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,
@@ -678,10 +674,7 @@ isp_video_get_format(struct file *file, void *fh, struct v4l2_format *format)
 	if (format->type != video->type)
 		return -EINVAL;
 
-	mutex_lock(&video->mutex);
 	*format = vfh->format;
-	mutex_unlock(&video->mutex);
-
 	return 0;
 }
 
@@ -736,10 +729,7 @@ isp_video_set_format(struct file *file, void *fh, struct v4l2_format *format)
 	isp_video_pix_to_mbus(&format->fmt.pix, &fmt);
 	isp_video_mbus_to_pix(video, &fmt, &format->fmt.pix);
 
-	mutex_lock(&video->mutex);
 	vfh->format = *format;
-	mutex_unlock(&video->mutex);
-
 	return 0;
 }
 
@@ -859,9 +849,7 @@ isp_video_set_selection(struct file *file, void *fh, struct v4l2_selection *sel)
 		return -EINVAL;
 
 	sdsel.pad = pad;
-	mutex_lock(&video->mutex);
 	ret = v4l2_subdev_call(subdev, pad, set_selection, NULL, &sdsel);
-	mutex_unlock(&video->mutex);
 	if (!ret)
 		sel->r = sdsel.r;
 
@@ -908,56 +896,32 @@ static int
 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
 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
 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
 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 +1060,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 +1068,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 +1120,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:
@@ -1183,10 +1141,6 @@ isp_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
 	video->queue = NULL;
 
 	media_entity_enum_cleanup(&pipe->ent_enum);
-
-err_enum_init:
-	mutex_unlock(&video->stream_lock);
-
 	return ret;
 }
 
@@ -1203,15 +1157,9 @@ 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,9 +1177,7 @@ 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;
 
@@ -1240,8 +1186,6 @@ isp_video_streamoff(struct file *file, void *fh, enum v4l2_buf_type type)
 
 	media_entity_enum_cleanup(&pipe->ent_enum);
 
-done:
-	mutex_unlock(&video->stream_lock);
 	return 0;
 }
 
@@ -1333,6 +1277,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->v4l_lock;
 
 	ret = vb2_queue_init(&handle->queue);
 	if (ret < 0) {
@@ -1366,9 +1311,7 @@ static int isp_video_release(struct file *file)
 	/* 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);
 
 	v4l2_pipeline_pm_use(&video->video.entity, 0);
 
@@ -1386,14 +1329,8 @@ static int isp_video_release(struct file *file)
 static __poll_t isp_video_poll(struct file *file, poll_table *wait)
 {
 	struct isp_video_fh *vfh = to_isp_video_fh(file->private_data);
-	struct isp_video *video = video_drvdata(file);
-	__poll_t ret;
 
-	mutex_lock(&video->queue_lock);
-	ret = vb2_poll(&vfh->queue, file, wait);
-	mutex_unlock(&video->queue_lock);
-
-	return ret;
+	return vb2_poll(&vfh->queue, file, wait);
 }
 
 static int isp_video_mmap(struct file *file, struct vm_area_struct *vma)
@@ -1445,12 +1382,10 @@ int omap3isp_video_init(struct isp_video *video, const char *name)
 	if (ret < 0)
 		return ret;
 
-	mutex_init(&video->mutex);
 	atomic_set(&video->active, 0);
 
+	mutex_init(&video->v4l_lock);
 	spin_lock_init(&video->pipe.lock);
-	mutex_init(&video->stream_lock);
-	mutex_init(&video->queue_lock);
 	spin_lock_init(&video->irqlock);
 
 	/* Initialize the video device. */
@@ -1460,6 +1395,7 @@ int omap3isp_video_init(struct isp_video *video, const char *name)
 	video->video.fops = &isp_video_fops;
 	snprintf(video->video.name, sizeof(video->video.name),
 		 "OMAP3 ISP %s %s", name, direction);
+	video->video.lock = &video->v4l_lock;
 	video->video.vfl_type = VFL_TYPE_GRABBER;
 	video->video.release = video_device_release_empty;
 	video->video.ioctl_ops = &isp_video_ioctl_ops;
@@ -1473,9 +1409,7 @@ int omap3isp_video_init(struct isp_video *video, const char *name)
 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);
+	mutex_destroy(&video->v4l_lock);
 }
 
 int omap3isp_video_register(struct isp_video *video, struct v4l2_device *vdev)
diff --git a/drivers/media/platform/omap3isp/ispvideo.h b/drivers/media/platform/omap3isp/ispvideo.h
index f6a2082b4a0a..de6201034960 100644
--- a/drivers/media/platform/omap3isp/ispvideo.h
+++ b/drivers/media/platform/omap3isp/ispvideo.h
@@ -153,7 +153,7 @@ struct isp_video {
 	enum v4l2_buf_type type;
 	struct media_pad pad;
 
-	struct mutex mutex;		/* format and crop settings */
+	struct mutex v4l_lock;
 	atomic_t active;
 
 	struct isp_device *isp;
@@ -172,7 +172,6 @@ struct isp_video {
 
 	/* Video buffers queue */
 	struct vb2_queue *queue;
-	struct mutex queue_lock;	/* protects the queue */
 	spinlock_t irqlock;		/* protects dmaqueue */
 	struct list_head dmaqueue;
 	enum isp_video_dmaqueue_flags dmaqueue_flags;
-- 
2.16.3

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

* [PATCH 08/20] mtk-mdp: Add locks for capture and output vb2_queues
  2018-05-18 18:51 [RFC PATCH v2 00/20] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
                   ` (6 preceding siblings ...)
  2018-05-18 18:51 ` [PATCH 07/20] omap3isp: " Ezequiel Garcia
@ 2018-05-18 18:51 ` Ezequiel Garcia
  2018-05-18 18:51 ` [PATCH 09/20] s5p-g2d: Implement wait_prepare and wait_finish Ezequiel Garcia
                   ` (11 subsequent siblings)
  19 siblings, 0 replies; 23+ messages in thread
From: Ezequiel Garcia @ 2018-05-18 18:51 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Abylay Ospan, 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.16.3

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

* [PATCH 09/20] s5p-g2d: Implement wait_prepare and wait_finish
  2018-05-18 18:51 [RFC PATCH v2 00/20] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
                   ` (7 preceding siblings ...)
  2018-05-18 18:51 ` [PATCH 08/20] mtk-mdp: Add locks for capture and output vb2_queues Ezequiel Garcia
@ 2018-05-18 18:51 ` Ezequiel Garcia
  2018-05-18 18:51 ` [PATCH 10/20] staging: bcm2835-camera: Provide lock for vb2_queue Ezequiel Garcia
                   ` (10 subsequent siblings)
  19 siblings, 0 replies; 23+ messages in thread
From: Ezequiel Garcia @ 2018-05-18 18:51 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Abylay Ospan, 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.16.3

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

* [PATCH 10/20] staging: bcm2835-camera: Provide lock for vb2_queue
  2018-05-18 18:51 [RFC PATCH v2 00/20] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
                   ` (8 preceding siblings ...)
  2018-05-18 18:51 ` [PATCH 09/20] s5p-g2d: Implement wait_prepare and wait_finish Ezequiel Garcia
@ 2018-05-18 18:51 ` Ezequiel Garcia
  2018-05-18 18:51 ` [PATCH 11/20] dvb-core: " Ezequiel Garcia
                   ` (9 subsequent siblings)
  19 siblings, 0 replies; 23+ messages in thread
From: Ezequiel Garcia @ 2018-05-18 18:51 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Abylay Ospan, 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>
---
 .../vc04_services/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 d2262275a870..2a628475a1bd 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -601,28 +601,14 @@ 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_prepare = buffer_prepare,
 	.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,
 };
 
 /* ------------------------------------------------------------------
@@ -1831,6 +1817,8 @@ static int __init bm2835_mmal_init(void)
 			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];
@@ -1875,13 +1863,11 @@ static int __init bm2835_mmal_init(void)
 		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.16.3

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

* [PATCH 11/20] dvb-core: Provide lock for vb2_queue
  2018-05-18 18:51 [RFC PATCH v2 00/20] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
                   ` (9 preceding siblings ...)
  2018-05-18 18:51 ` [PATCH 10/20] staging: bcm2835-camera: Provide lock for vb2_queue Ezequiel Garcia
@ 2018-05-18 18:51 ` Ezequiel Garcia
  2018-05-18 18:52 ` [PATCH 12/20] venus: Add video_device and vb2_queue locks Ezequiel Garcia
                   ` (8 subsequent siblings)
  19 siblings, 0 replies; 23+ messages in thread
From: Ezequiel Garcia @ 2018-05-18 18:51 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Abylay Ospan, 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.16.3

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

* [PATCH 12/20] venus: Add video_device and vb2_queue locks
  2018-05-18 18:51 [RFC PATCH v2 00/20] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
                   ` (10 preceding siblings ...)
  2018-05-18 18:51 ` [PATCH 11/20] dvb-core: " Ezequiel Garcia
@ 2018-05-18 18:52 ` Ezequiel Garcia
  2018-05-18 18:52 ` [PATCH 13/20] davinci_vpfe: " Ezequiel Garcia
                   ` (7 subsequent siblings)
  19 siblings, 0 replies; 23+ messages in thread
From: Ezequiel Garcia @ 2018-05-18 18:52 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Abylay Ospan, 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.16.3

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

* [PATCH 13/20] davinci_vpfe: Add video_device and vb2_queue locks
  2018-05-18 18:51 [RFC PATCH v2 00/20] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
                   ` (11 preceding siblings ...)
  2018-05-18 18:52 ` [PATCH 12/20] venus: Add video_device and vb2_queue locks Ezequiel Garcia
@ 2018-05-18 18:52 ` Ezequiel Garcia
  2018-05-18 18:52 ` [PATCH 14/20] mx_emmaprp: Implement wait_prepare and wait_finish Ezequiel Garcia
                   ` (6 subsequent siblings)
  19 siblings, 0 replies; 23+ messages in thread
From: Ezequiel Garcia @ 2018-05-18 18:52 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Abylay Ospan, 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.16.3

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

* [PATCH 14/20] mx_emmaprp: Implement wait_prepare and wait_finish
  2018-05-18 18:51 [RFC PATCH v2 00/20] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
                   ` (12 preceding siblings ...)
  2018-05-18 18:52 ` [PATCH 13/20] davinci_vpfe: " Ezequiel Garcia
@ 2018-05-18 18:52 ` Ezequiel Garcia
  2018-05-18 18:52 ` [PATCH 15/20] m2m-deinterlace: " Ezequiel Garcia
                   ` (5 subsequent siblings)
  19 siblings, 0 replies; 23+ messages in thread
From: Ezequiel Garcia @ 2018-05-18 18:52 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Abylay Ospan, 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.16.3

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

* [PATCH 15/20] m2m-deinterlace: Implement wait_prepare and wait_finish
  2018-05-18 18:51 [RFC PATCH v2 00/20] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
                   ` (13 preceding siblings ...)
  2018-05-18 18:52 ` [PATCH 14/20] mx_emmaprp: Implement wait_prepare and wait_finish Ezequiel Garcia
@ 2018-05-18 18:52 ` Ezequiel Garcia
  2018-05-18 18:52 ` [PATCH 16/20] stk1160: Set the vb2_queue lock before calling vb2_queue_init Ezequiel Garcia
                   ` (4 subsequent siblings)
  19 siblings, 0 replies; 23+ messages in thread
From: Ezequiel Garcia @ 2018-05-18 18:52 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Abylay Ospan, 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.16.3

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

* [PATCH 16/20] stk1160: Set the vb2_queue lock before calling vb2_queue_init
  2018-05-18 18:51 [RFC PATCH v2 00/20] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
                   ` (14 preceding siblings ...)
  2018-05-18 18:52 ` [PATCH 15/20] m2m-deinterlace: " Ezequiel Garcia
@ 2018-05-18 18:52 ` Ezequiel Garcia
  2018-05-18 18:52 ` [PATCH 17/20] videobuf2-core: require q->lock Ezequiel Garcia
                   ` (3 subsequent siblings)
  19 siblings, 0 replies; 23+ messages in thread
From: Ezequiel Garcia @ 2018-05-18 18:52 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Abylay Ospan, 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.16.3

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

* [PATCH 17/20] videobuf2-core: require q->lock
  2018-05-18 18:51 [RFC PATCH v2 00/20] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
                   ` (15 preceding siblings ...)
  2018-05-18 18:52 ` [PATCH 16/20] stk1160: Set the vb2_queue lock before calling vb2_queue_init Ezequiel Garcia
@ 2018-05-18 18:52 ` Ezequiel Garcia
  2018-05-18 18:52 ` [PATCH 18/20] videobuf2: assume q->lock is always set Ezequiel Garcia
                   ` (2 subsequent siblings)
  19 siblings, 0 replies; 23+ messages in thread
From: Ezequiel Garcia @ 2018-05-18 18:52 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Abylay Ospan, 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 d3f7bb33a54d..3b89ec5e0b2f 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -2002,6 +2002,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.16.3

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

* [PATCH 18/20] videobuf2: assume q->lock is always set
  2018-05-18 18:51 [RFC PATCH v2 00/20] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
                   ` (16 preceding siblings ...)
  2018-05-18 18:52 ` [PATCH 17/20] videobuf2-core: require q->lock Ezequiel Garcia
@ 2018-05-18 18:52 ` Ezequiel Garcia
  2018-05-18 18:52 ` [PATCH 19/20] v4l2-ioctl.c: assume queue->lock " Ezequiel Garcia
  2018-05-18 18:52 ` [PATCH 20/20] media: Remove wait_{prepare, finish} Ezequiel Garcia
  19 siblings, 0 replies; 23+ messages in thread
From: Ezequiel Garcia @ 2018-05-18 18:52 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Abylay Ospan, 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>
---
 drivers/media/common/videobuf2/videobuf2-core.c | 21 ++++++++-----------
 drivers/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 3b89ec5e0b2f..8ca279a43549 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;
 	}
@@ -1484,10 +1479,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.
@@ -1501,7 +1496,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;
@@ -2528,10 +2523,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];
@@ -2543,12 +2538,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.16.3

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

* [PATCH 19/20] v4l2-ioctl.c: assume queue->lock is always set
  2018-05-18 18:51 [RFC PATCH v2 00/20] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
                   ` (17 preceding siblings ...)
  2018-05-18 18:52 ` [PATCH 18/20] videobuf2: assume q->lock is always set Ezequiel Garcia
@ 2018-05-18 18:52 ` Ezequiel Garcia
  2018-05-18 18:52 ` [PATCH 20/20] media: Remove wait_{prepare, finish} Ezequiel Garcia
  19 siblings, 0 replies; 23+ messages in thread
From: Ezequiel Garcia @ 2018-05-18 18:52 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Abylay Ospan, 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 ee1eec136e55..834e3de69992 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -2694,12 +2694,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.16.3

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

* [PATCH 20/20] media: Remove wait_{prepare, finish}
  2018-05-18 18:51 [RFC PATCH v2 00/20] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
                   ` (18 preceding siblings ...)
  2018-05-18 18:52 ` [PATCH 19/20] v4l2-ioctl.c: assume queue->lock " Ezequiel Garcia
@ 2018-05-18 18:52 ` Ezequiel Garcia
  19 siblings, 0 replies; 23+ messages in thread
From: Ezequiel Garcia @ 2018-05-18 18:52 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, kernel, Abylay Ospan, 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 --
 drivers/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/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 --
 drivers/media/platform/exynos4-is/fimc-capture.c       |  2 --
 drivers/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 --
 drivers/media/platform/marvell-ccic/mcam-core.c        |  4 ----
 drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c        |  2 --
 drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c           |  2 --
 drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c     |  2 --
 drivers/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 --
 drivers/media/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 --
 drivers/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 --
 .../media/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 ----
 drivers/staging/media/davinci_vpfe/vpfe_video.c        |  2 --
 drivers/staging/media/imx/imx-media-capture.c          |  2 --
 drivers/staging/media/omap4iss/iss_video.c             |  2 --
 .../vc04_services/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 5343f2c08f15..25a5ed24da65 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 09194721aed2..84034a7f56ac 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -2366,8 +2366,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/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 114d9bcbe4f4..b716fca5079b 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 0e0952e60795..3c99044deeab 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 2f886140dd2e..f6b00c3c712c 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 c637679b01b2..8755dddf0267 100644
--- a/drivers/media/pci/cx88/cx88-vbi.c
+++ b/drivers/media/pci/cx88/cx88-vbi.c
@@ -229,8 +229,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 9be682cdb644..0368d88310c3 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 7d768ec0f824..eafcfc6ba19a 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
+++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
@@ -1043,8 +1043,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 d47b99c2100f..3a15a0b9b109 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 c3fafa97b2d0..16ff914d19d3 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 601ae6487617..784fe28d7885 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 04e35d70ce2e..b8b3e16d08b0 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1647,8 +1647,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 80670eeee142..b8273ab48348 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 af17aaa21f58..f123345b1089 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 0e73f43ffa2d..7eb817cb677d 100644
--- a/drivers/media/platform/omap3isp/ispvideo.c
+++ b/drivers/media/platform/omap3isp/ispvideo.c
@@ -493,8 +493,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 c71a00736541..7659046fb2d0 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 4a3a195e7f59..b638deb2ff94 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -1214,8 +1214,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 f6092ae45912..2c061cc03f6f 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 6599dba5ab84..6d6d2cb19994 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 5cf4d9921264..3fe68cfd85f3 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 5c0462ca9993..d9ec94dfb27b 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 bf4ca16db440..4abb3ed41a46 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 2ab0b5cc5c22..d2cb9a6d4ed8 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 afab77cf4fa5..503226755e94 100644
--- a/drivers/media/platform/vsp1/vsp1_histo.c
+++ b/drivers/media/platform/vsp1/vsp1_histo.c
@@ -163,8 +163,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 c2d3b8f0f487..a8de712f27fd 100644
--- a/drivers/media/platform/vsp1/vsp1_video.c
+++ b/drivers/media/platform/vsp1/vsp1_video.c
@@ -946,8 +946,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 522cdfdd3345..720048bb517e 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 d70ee13cc52e..ec1d88132937 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 98cd57eaf36a..c07a44825351 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 a383058b0cb3..b5998f170675 100644
--- a/drivers/media/usb/gspca/gspca.c
+++ b/drivers/media/usb/gspca/gspca.c
@@ -1376,8 +1376,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 7eb53517a82f..66b18fccb93f 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 0c0d0ef71573..3668a04359e8 100644
--- a/drivers/media/usb/usbtv/usbtv-video.c
+++ b/drivers/media/usb/usbtv/usbtv-video.c
@@ -698,8 +698,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 0ccabe04b0e1..5ae11016cd33 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/media/omap4iss/iss_video.c b/drivers/staging/media/omap4iss/iss_video.c
index 380cfd230262..47f133bb433b 100644
--- a/drivers/staging/media/omap4iss/iss_video.c
+++ b/drivers/staging/media/omap4iss/iss_video.c
@@ -408,8 +408,6 @@ static const struct vb2_ops iss_video_vb2ops = {
 	.buf_prepare	= iss_video_buf_prepare,
 	.buf_queue	= iss_video_buf_queue,
 	.buf_cleanup	= iss_video_buf_cleanup,
-	.wait_prepare	= vb2_ops_wait_prepare,
-	.wait_finish	= vb2_ops_wait_finish,
 };
 
 /*
diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index 2a628475a1bd..f771f4fb7a6b 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -607,8 +607,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.16.3

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

* Re: [PATCH 06/20] omap4iss: Add video_device and vb2_queue locks
  2018-05-18 18:51 ` [PATCH 06/20] omap4iss: " Ezequiel Garcia
@ 2018-05-22  9:09   ` Hans Verkuil
  2018-05-23 21:33     ` Ezequiel Garcia
  0 siblings, 1 reply; 23+ messages in thread
From: Hans Verkuil @ 2018-05-22  9:09 UTC (permalink / raw)
  To: Ezequiel Garcia, linux-media; +Cc: kernel, Abylay Ospan

On 18/05/18 20:51, Ezequiel Garcia wrote:
> video_device and vb2_queue locks are now both
> mandatory. Add them, remove driver ad-hoc locks,
> 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/staging/media/omap4iss/iss_video.c | 32 +++++++-----------------------
>  drivers/staging/media/omap4iss/iss_video.h |  2 +-
>  2 files changed, 8 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/staging/media/omap4iss/iss_video.c b/drivers/staging/media/omap4iss/iss_video.c
> index a3a83424a926..380cfd230262 100644
> --- a/drivers/staging/media/omap4iss/iss_video.c
> +++ b/drivers/staging/media/omap4iss/iss_video.c
> @@ -260,10 +260,7 @@ __iss_video_get_format(struct iss_video *video,
>  	fmt.pad = pad;
>  	fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
>  
> -	mutex_lock(&video->mutex);
>  	ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt);
> -	mutex_unlock(&video->mutex);
> -
>  	if (ret)
>  		return ret;
>  
> @@ -411,6 +408,8 @@ static const struct vb2_ops iss_video_vb2ops = {
>  	.buf_prepare	= iss_video_buf_prepare,
>  	.buf_queue	= iss_video_buf_queue,
>  	.buf_cleanup	= iss_video_buf_cleanup,
> +	.wait_prepare	= vb2_ops_wait_prepare,
> +	.wait_finish	= vb2_ops_wait_finish,
>  };
>  
>  /*
> @@ -592,9 +591,7 @@ iss_video_get_format(struct file *file, void *fh, struct v4l2_format *format)
>  	if (format->type != video->type)
>  		return -EINVAL;
>  
> -	mutex_lock(&video->mutex);
>  	*format = vfh->format;
> -	mutex_unlock(&video->mutex);
>  
>  	return 0;
>  }
> @@ -609,8 +606,6 @@ iss_video_set_format(struct file *file, void *fh, struct v4l2_format *format)
>  	if (format->type != video->type)
>  		return -EINVAL;
>  
> -	mutex_lock(&video->mutex);
> -
>  	/*
>  	 * Fill the bytesperline and sizeimage fields by converting to media bus
>  	 * format and back to pixel format.
> @@ -620,7 +615,6 @@ iss_video_set_format(struct file *file, void *fh, struct v4l2_format *format)
>  
>  	vfh->format = *format;
>  
> -	mutex_unlock(&video->mutex);
>  	return 0;
>  }
>  
> @@ -741,9 +735,7 @@ iss_video_set_selection(struct file *file, void *fh, struct v4l2_selection *sel)
>  		return -EINVAL;
>  
>  	sdsel.pad = pad;
> -	mutex_lock(&video->mutex);
>  	ret = v4l2_subdev_call(subdev, pad, set_selection, NULL, &sdsel);
> -	mutex_unlock(&video->mutex);
>  	if (!ret)
>  		sel->r = sdsel.r;
>  
> @@ -873,8 +865,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 +968,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 +984,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 +999,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 +1025,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 +1119,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->v4l_lock;

I feel too much is changed in this driver (and also the omap3isp driver in the
next patch). Why not just set q->lock to stream_lock (queue_lock for omap3isp)?

There is no need to set video->video.lock at all, that's not compulsory.

Regards,

	Hans

>  
>  	ret = vb2_queue_init(q);
>  	if (ret) {
> @@ -1238,12 +1221,11 @@ int omap4iss_video_init(struct iss_video *video, const char *name)
>  	if (ret < 0)
>  		return ret;
>  
> +	mutex_init(&video->v4l_lock);
>  	spin_lock_init(&video->qlock);
> -	mutex_init(&video->mutex);
>  	atomic_set(&video->active, 0);
>  
>  	spin_lock_init(&video->pipe.lock);
> -	mutex_init(&video->stream_lock);
>  
>  	/* Initialize the video device. */
>  	if (!video->ops)
> @@ -1252,6 +1234,7 @@ int omap4iss_video_init(struct iss_video *video, const char *name)
>  	video->video.fops = &iss_video_fops;
>  	snprintf(video->video.name, sizeof(video->video.name),
>  		 "OMAP4 ISS %s %s", name, direction);
> +	video->video.lock = &video->v4l_lock;
>  	video->video.vfl_type = VFL_TYPE_GRABBER;
>  	video->video.release = video_device_release_empty;
>  	video->video.ioctl_ops = &iss_video_ioctl_ops;
> @@ -1265,8 +1248,7 @@ int omap4iss_video_init(struct iss_video *video, const char *name)
>  void omap4iss_video_cleanup(struct iss_video *video)
>  {
>  	media_entity_cleanup(&video->video.entity);
> -	mutex_destroy(&video->stream_lock);
> -	mutex_destroy(&video->mutex);
> +	mutex_destroy(&video->v4l_lock);
>  }
>  
>  int omap4iss_video_register(struct iss_video *video, struct v4l2_device *vdev)
> diff --git a/drivers/staging/media/omap4iss/iss_video.h b/drivers/staging/media/omap4iss/iss_video.h
> index d7e05d04512c..4b8e5a8073fb 100644
> --- a/drivers/staging/media/omap4iss/iss_video.h
> +++ b/drivers/staging/media/omap4iss/iss_video.h
> @@ -148,8 +148,8 @@ struct iss_video {
>  	enum v4l2_buf_type type;
>  	struct media_pad pad;
>  
> -	struct mutex mutex;		/* format and crop settings */
>  	atomic_t active;
> +	struct mutex v4l_lock;
>  
>  	struct iss_device *iss;
>  
> 

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

* Re: [PATCH 06/20] omap4iss: Add video_device and vb2_queue locks
  2018-05-22  9:09   ` Hans Verkuil
@ 2018-05-23 21:33     ` Ezequiel Garcia
  0 siblings, 0 replies; 23+ messages in thread
From: Ezequiel Garcia @ 2018-05-23 21:33 UTC (permalink / raw)
  To: Hans Verkuil, linux-media; +Cc: kernel, Abylay Ospan

On Tue, 2018-05-22 at 11:09 +0200, Hans Verkuil wrote:
> On 18/05/18 20:51, Ezequiel Garcia wrote:
> > video_device and vb2_queue locks are now both
> > mandatory. Add them, remove driver ad-hoc locks,
> > 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/staging/media/omap4iss/iss_video.c | 32 +++++++-----------------------
> >  drivers/staging/media/omap4iss/iss_video.h |  2 +-
> >  2 files changed, 8 insertions(+), 26 deletions(-)
> > 
> > diff --git a/drivers/staging/media/omap4iss/iss_video.c b/drivers/staging/media/omap4iss/iss_video.c
> > index a3a83424a926..380cfd230262 100644
> > --- a/drivers/staging/media/omap4iss/iss_video.c
> > +++ b/drivers/staging/media/omap4iss/iss_video.c
> > @@ -260,10 +260,7 @@ __iss_video_get_format(struct iss_video *video,
> >  	fmt.pad = pad;
> >  	fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> >  
> > -	mutex_lock(&video->mutex);
> >  	ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt);
> > -	mutex_unlock(&video->mutex);
> > -
> >  	if (ret)
> >  		return ret;
> >  
> > @@ -411,6 +408,8 @@ static const struct vb2_ops iss_video_vb2ops = {
> >  	.buf_prepare	= iss_video_buf_prepare,
> >  	.buf_queue	= iss_video_buf_queue,
> >  	.buf_cleanup	= iss_video_buf_cleanup,
> > +	.wait_prepare	= vb2_ops_wait_prepare,
> > +	.wait_finish	= vb2_ops_wait_finish,
> >  };
> >  
> >  /*
> > @@ -592,9 +591,7 @@ iss_video_get_format(struct file *file, void *fh, struct v4l2_format *format)
> >  	if (format->type != video->type)
> >  		return -EINVAL;
> >  
> > -	mutex_lock(&video->mutex);
> >  	*format = vfh->format;
> > -	mutex_unlock(&video->mutex);
> >  
> >  	return 0;
> >  }
> > @@ -609,8 +606,6 @@ iss_video_set_format(struct file *file, void *fh, struct v4l2_format *format)
> >  	if (format->type != video->type)
> >  		return -EINVAL;
> >  
> > -	mutex_lock(&video->mutex);
> > -
> >  	/*
> >  	 * Fill the bytesperline and sizeimage fields by converting to media bus
> >  	 * format and back to pixel format.
> > @@ -620,7 +615,6 @@ iss_video_set_format(struct file *file, void *fh, struct v4l2_format *format)
> >  
> >  	vfh->format = *format;
> >  
> > -	mutex_unlock(&video->mutex);
> >  	return 0;
> >  }
> >  
> > @@ -741,9 +735,7 @@ iss_video_set_selection(struct file *file, void *fh, struct v4l2_selection *sel)
> >  		return -EINVAL;
> >  
> >  	sdsel.pad = pad;
> > -	mutex_lock(&video->mutex);
> >  	ret = v4l2_subdev_call(subdev, pad, set_selection, NULL, &sdsel);
> > -	mutex_unlock(&video->mutex);
> >  	if (!ret)
> >  		sel->r = sdsel.r;
> >  
> > @@ -873,8 +865,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 +968,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 +984,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 +999,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 +1025,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 +1119,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->v4l_lock;
> 
> I feel too much is changed in this driver (and also the omap3isp driver in the
> next patch). Why not just set q->lock to stream_lock (queue_lock for omap3isp)?
> 

OK. I'll set q->lock to stream_lock (queue_lock) and then of course,
drop all the mutex_lock/unlock that the driver is doing.

> There is no need to set video->video.lock at all, that's not compulsory.
> 

Yeah, perhaps it's a better idea, considering I'm only compile testing these.

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

end of thread, other threads:[~2018-05-23 21:35 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-18 18:51 [RFC PATCH v2 00/20] v4l2 core: push ioctl lock down to ioctl handler Ezequiel Garcia
2018-05-18 18:51 ` [PATCH 01/20] pvrusb2: replace pvr2_v4l2_ioctl by video_ioctl2 Ezequiel Garcia
2018-05-18 18:51 ` [PATCH 02/20] v4l2-core: push taking ioctl mutex down to ioctl handler Ezequiel Garcia
2018-05-18 18:51 ` [PATCH 03/20] v4l2-ioctl.c: use correct vb2_queue lock for m2m devices Ezequiel Garcia
2018-05-18 18:51 ` [PATCH 04/20] usbtv: Implement wait_prepare and wait_finish Ezequiel Garcia
2018-05-18 18:51 ` [PATCH 05/20] sta2x11: Add video_device and vb2_queue locks Ezequiel Garcia
2018-05-18 18:51 ` [PATCH 06/20] omap4iss: " Ezequiel Garcia
2018-05-22  9:09   ` Hans Verkuil
2018-05-23 21:33     ` Ezequiel Garcia
2018-05-18 18:51 ` [PATCH 07/20] omap3isp: " Ezequiel Garcia
2018-05-18 18:51 ` [PATCH 08/20] mtk-mdp: Add locks for capture and output vb2_queues Ezequiel Garcia
2018-05-18 18:51 ` [PATCH 09/20] s5p-g2d: Implement wait_prepare and wait_finish Ezequiel Garcia
2018-05-18 18:51 ` [PATCH 10/20] staging: bcm2835-camera: Provide lock for vb2_queue Ezequiel Garcia
2018-05-18 18:51 ` [PATCH 11/20] dvb-core: " Ezequiel Garcia
2018-05-18 18:52 ` [PATCH 12/20] venus: Add video_device and vb2_queue locks Ezequiel Garcia
2018-05-18 18:52 ` [PATCH 13/20] davinci_vpfe: " Ezequiel Garcia
2018-05-18 18:52 ` [PATCH 14/20] mx_emmaprp: Implement wait_prepare and wait_finish Ezequiel Garcia
2018-05-18 18:52 ` [PATCH 15/20] m2m-deinterlace: " Ezequiel Garcia
2018-05-18 18:52 ` [PATCH 16/20] stk1160: Set the vb2_queue lock before calling vb2_queue_init Ezequiel Garcia
2018-05-18 18:52 ` [PATCH 17/20] videobuf2-core: require q->lock Ezequiel Garcia
2018-05-18 18:52 ` [PATCH 18/20] videobuf2: assume q->lock is always set Ezequiel Garcia
2018-05-18 18:52 ` [PATCH 19/20] v4l2-ioctl.c: assume queue->lock " Ezequiel Garcia
2018-05-18 18:52 ` [PATCH 20/20] media: Remove wait_{prepare, finish} 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.