[PATCH 00/10] [RFC] Prio handling and v4l2_device release callback

[PATCH 00/10] [RFC] Prio handling and v4l2_device release callback

[Hans Verkuil]

This patch series adds two new features to the V4L2 framework.

The first 5 patches add support for VIDIOC_G/S_PRIORITY. All prio handling
will be done in the core for any driver that either uses struct v4l2_fh
(ivtv only at the moment) or has no open and release file operations (true
for many simple (radio) drivers). In all other cases the driver will have
to do the work.

Eventually all drivers should either use v4l2_fh or never set filp->private_data.

All drivers that use the control framework must also use core-prio handling
since control handling is no longer done through v4l2_ioctl_ops, so the driver
doesn't have an easy way of checking the priority before changing a control.

By default all device nodes use the same priority state in v4l2_device, but
drivers can assign the prio field in video_device to a different priority state,
allowing e.g. all capture device nodes to use a different priority from all
the display device nodes.

The v4l2_ioctl.c code will check the ioctls whether or not they are allowed
based on the current priority. The vidioc_default callback has a new argument
that just tells the driver whether the prio is OK or not. The driver can use
this argument to return -EBUSY for those commands that modify state of the driver.

The following three patches implement this in ivtv and update the documentation.

The last two patches add a release callback in v4l2_device which will be called
when the last registered device node is removed. This is very useful for
implementing the hotplug disconnect functionality as it provides a single clean
up callback when the last user of any of the unregistered device nodes has
closed its filehandle. For drivers with just a single device node this is not
very relevant since the video_device release() does the same job, but for
drivers that create multiple device nodes (e.g. usbvision) this is a must-have.

An example of how this would be used can be found in the dsbr100 patches in
this branch:

http://git.linuxtv.org/hverkuil/media_tree.git?a=shortlog;h=refs/heads/usbvision

Comments?

Regards,

	Hans

Hans Verkuil (10):
  v4l2_prio: move from v4l2-common to v4l2-dev.
  v4l2: add v4l2_prio_state to v4l2_device and video_device
  v4l2-fh: implement v4l2_priority support.
  v4l2-dev: add and support flag V4L2_FH_USE_PRIO.
  v4l2-ioctl: add priority handling support.
  ivtv: convert to core priority handling.
  ivtv: use core-assisted locking.
  v4l2-framework: update documentation for new prio field
  v4l2-device: add kref and a release function
  v4l2-framework.txt: document new v4l2_device release() callback

 Documentation/video4linux/v4l2-framework.txt |   34 ++++++++-
 drivers/media/radio/radio-si4713.c           |    3 +-
 drivers/media/video/cx18/cx18-ioctl.c        |    3 +-
 drivers/media/video/davinci/vpfe_capture.c   |    2 +-
 drivers/media/video/ivtv/ivtv-driver.h       |    2 -
 drivers/media/video/ivtv/ivtv-fileops.c      |   17 +----
 drivers/media/video/ivtv/ivtv-ioctl.c        |   77 +++++---------------
 drivers/media/video/ivtv/ivtv-streams.c      |    1 +
 drivers/media/video/meye.c                   |    3 +-
 drivers/media/video/mxb.c                    |    3 +-
 drivers/media/video/v4l2-common.c            |   63 ----------------
 drivers/media/video/v4l2-dev.c               |  101 +++++++++++++++++++++++++-
 drivers/media/video/v4l2-device.c            |   16 ++++
 drivers/media/video/v4l2-fh.c                |    4 +
 drivers/media/video/v4l2-ioctl.c             |   73 +++++++++++++++++--
 include/media/v4l2-common.h                  |   15 ----
 include/media/v4l2-dev.h                     |   24 ++++++
 include/media/v4l2-device.h                  |   14 ++++
 include/media/v4l2-fh.h                      |    1 +
 include/media/v4l2-ioctl.h                   |    2 +-
 20 files changed, 286 insertions(+), 172 deletions(-)

[PATCH 01/10] [RFC] v4l2_prio: move from v4l2-common to v4l2-dev.

[Hans Verkuil]

We are going to move priority handling into the v4l2 core. As a consequence
the v4l2_prio helper functions need to be moved into the core videodev
module as well to prevent circular dependencies.

Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
 drivers/media/video/v4l2-common.c |   63 ------------------------------------
 drivers/media/video/v4l2-dev.c    |   64 +++++++++++++++++++++++++++++++++++++
 include/media/v4l2-common.h       |   15 ---------
 include/media/v4l2-dev.h          |   15 +++++++++
 4 files changed, 79 insertions(+), 78 deletions(-)

diff --git a/drivers/media/video/v4l2-common.c b/drivers/media/video/v4l2-common.c
index b5eb1f3..c3ccd47 100644
--- a/drivers/media/video/v4l2-common.c
+++ b/drivers/media/video/v4l2-common.c
@@ -81,69 +81,6 @@ MODULE_LICENSE("GPL");
  *  Video Standard Operations (contributed by Michael Schimek)
  */
 
-
-/* ----------------------------------------------------------------- */
-/* priority handling                                                 */
-
-#define V4L2_PRIO_VALID(val) (val == V4L2_PRIORITY_BACKGROUND   || \
-			      val == V4L2_PRIORITY_INTERACTIVE  || \
-			      val == V4L2_PRIORITY_RECORD)
-
-void v4l2_prio_init(struct v4l2_prio_state *global)
-{
-	memset(global, 0, sizeof(*global));
-}
-EXPORT_SYMBOL(v4l2_prio_init);
-
-int v4l2_prio_change(struct v4l2_prio_state *global, enum v4l2_priority *local,
-		     enum v4l2_priority new)
-{
-	if (!V4L2_PRIO_VALID(new))
-		return -EINVAL;
-	if (*local == new)
-		return 0;
-
-	atomic_inc(&global->prios[new]);
-	if (V4L2_PRIO_VALID(*local))
-		atomic_dec(&global->prios[*local]);
-	*local = new;
-	return 0;
-}
-EXPORT_SYMBOL(v4l2_prio_change);
-
-void v4l2_prio_open(struct v4l2_prio_state *global, enum v4l2_priority *local)
-{
-	v4l2_prio_change(global, local, V4L2_PRIORITY_DEFAULT);
-}
-EXPORT_SYMBOL(v4l2_prio_open);
-
-void v4l2_prio_close(struct v4l2_prio_state *global, enum v4l2_priority local)
-{
-	if (V4L2_PRIO_VALID(local))
-		atomic_dec(&global->prios[local]);
-}
-EXPORT_SYMBOL(v4l2_prio_close);
-
-enum v4l2_priority v4l2_prio_max(struct v4l2_prio_state *global)
-{
-	if (atomic_read(&global->prios[V4L2_PRIORITY_RECORD]) > 0)
-		return V4L2_PRIORITY_RECORD;
-	if (atomic_read(&global->prios[V4L2_PRIORITY_INTERACTIVE]) > 0)
-		return V4L2_PRIORITY_INTERACTIVE;
-	if (atomic_read(&global->prios[V4L2_PRIORITY_BACKGROUND]) > 0)
-		return V4L2_PRIORITY_BACKGROUND;
-	return V4L2_PRIORITY_UNSET;
-}
-EXPORT_SYMBOL(v4l2_prio_max);
-
-int v4l2_prio_check(struct v4l2_prio_state *global, enum v4l2_priority local)
-{
-	return (local < v4l2_prio_max(global)) ? -EBUSY : 0;
-}
-EXPORT_SYMBOL(v4l2_prio_check);
-
-/* ----------------------------------------------------------------- */
-
 /* Helper functions for control handling			     */
 
 /* Check for correctness of the ctrl's value based on the data from
diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-dev.c
index 359e232..8698fe4 100644
--- a/drivers/media/video/v4l2-dev.c
+++ b/drivers/media/video/v4l2-dev.c
@@ -182,6 +182,70 @@ struct video_device *video_devdata(struct file *file)
 }
 EXPORT_SYMBOL(video_devdata);
 
+
+/* Priority handling */
+
+static inline bool prio_is_valid(enum v4l2_priority prio)
+{
+	return prio == V4L2_PRIORITY_BACKGROUND ||
+	       prio == V4L2_PRIORITY_INTERACTIVE ||
+	       prio == V4L2_PRIORITY_RECORD;
+}
+
+void v4l2_prio_init(struct v4l2_prio_state *global)
+{
+	memset(global, 0, sizeof(*global));
+}
+EXPORT_SYMBOL(v4l2_prio_init);
+
+int v4l2_prio_change(struct v4l2_prio_state *global, enum v4l2_priority *local,
+		     enum v4l2_priority new)
+{
+	if (!prio_is_valid(new))
+		return -EINVAL;
+	if (*local == new)
+		return 0;
+
+	atomic_inc(&global->prios[new]);
+	if (prio_is_valid(*local))
+		atomic_dec(&global->prios[*local]);
+	*local = new;
+	return 0;
+}
+EXPORT_SYMBOL(v4l2_prio_change);
+
+void v4l2_prio_open(struct v4l2_prio_state *global, enum v4l2_priority *local)
+{
+	v4l2_prio_change(global, local, V4L2_PRIORITY_DEFAULT);
+}
+EXPORT_SYMBOL(v4l2_prio_open);
+
+void v4l2_prio_close(struct v4l2_prio_state *global, enum v4l2_priority local)
+{
+	if (prio_is_valid(local))
+		atomic_dec(&global->prios[local]);
+}
+EXPORT_SYMBOL(v4l2_prio_close);
+
+enum v4l2_priority v4l2_prio_max(struct v4l2_prio_state *global)
+{
+	if (atomic_read(&global->prios[V4L2_PRIORITY_RECORD]) > 0)
+		return V4L2_PRIORITY_RECORD;
+	if (atomic_read(&global->prios[V4L2_PRIORITY_INTERACTIVE]) > 0)
+		return V4L2_PRIORITY_INTERACTIVE;
+	if (atomic_read(&global->prios[V4L2_PRIORITY_BACKGROUND]) > 0)
+		return V4L2_PRIORITY_BACKGROUND;
+	return V4L2_PRIORITY_UNSET;
+}
+EXPORT_SYMBOL(v4l2_prio_max);
+
+int v4l2_prio_check(struct v4l2_prio_state *global, enum v4l2_priority local)
+{
+	return (local < v4l2_prio_max(global)) ? -EBUSY : 0;
+}
+EXPORT_SYMBOL(v4l2_prio_check);
+
+
 static ssize_t v4l2_read(struct file *filp, char __user *buf,
 		size_t sz, loff_t *off)
 {
diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
index 239125a..4c70d15 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -80,21 +80,6 @@
 
 /* ------------------------------------------------------------------------- */
 
-/* Priority helper functions */
-
-struct v4l2_prio_state {
-	atomic_t prios[4];
-};
-void v4l2_prio_init(struct v4l2_prio_state *global);
-int v4l2_prio_change(struct v4l2_prio_state *global, enum v4l2_priority *local,
-		     enum v4l2_priority new);
-void v4l2_prio_open(struct v4l2_prio_state *global, enum v4l2_priority *local);
-void v4l2_prio_close(struct v4l2_prio_state *global, enum v4l2_priority local);
-enum v4l2_priority v4l2_prio_max(struct v4l2_prio_state *global);
-int v4l2_prio_check(struct v4l2_prio_state *global, enum v4l2_priority local);
-
-/* ------------------------------------------------------------------------- */
-
 /* Control helper functions */
 
 int v4l2_ctrl_check(struct v4l2_ext_control *ctrl, struct v4l2_queryctrl *qctrl,
diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
index 15802a0..861f323 100644
--- a/include/media/v4l2-dev.h
+++ b/include/media/v4l2-dev.h
@@ -34,6 +34,21 @@ struct v4l2_ctrl_handler;
 #define V4L2_FL_REGISTERED	(0)
 #define V4L2_FL_USES_V4L2_FH	(1)
 
+/* Priority helper functions */
+
+struct v4l2_prio_state {
+	atomic_t prios[4];
+};
+
+void v4l2_prio_init(struct v4l2_prio_state *global);
+int v4l2_prio_change(struct v4l2_prio_state *global, enum v4l2_priority *local,
+		     enum v4l2_priority new);
+void v4l2_prio_open(struct v4l2_prio_state *global, enum v4l2_priority *local);
+void v4l2_prio_close(struct v4l2_prio_state *global, enum v4l2_priority local);
+enum v4l2_priority v4l2_prio_max(struct v4l2_prio_state *global);
+int v4l2_prio_check(struct v4l2_prio_state *global, enum v4l2_priority local);
+
+
 struct v4l2_file_operations {
 	struct module *owner;
 	ssize_t (*read) (struct file *, char __user *, size_t, loff_t *);
-- 
1.6.4.2

[PATCH 02/10] [RFC] v4l2: add v4l2_prio_state to v4l2_device and video_device

[Hans Verkuil]

Integrate the v4l2_prio_state into the core, ready for use.

One struct v4l2_prio_state is added to v4l2_device and a pointer
to a prio state is added to video_device.

Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
 drivers/media/video/v4l2-dev.c    |    6 ++++++
 drivers/media/video/v4l2-device.c |    1 +
 include/media/v4l2-dev.h          |    3 +++
 include/media/v4l2-device.h       |    3 +++
 4 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-dev.c
index 8698fe4..c8f6ae1 100644
--- a/drivers/media/video/v4l2-dev.c
+++ b/drivers/media/video/v4l2-dev.c
@@ -543,6 +543,12 @@ static int __video_register_device(struct video_device *vdev, int type, int nr,
 			vdev->parent = vdev->v4l2_dev->dev;
 		if (vdev->ctrl_handler == NULL)
 			vdev->ctrl_handler = vdev->v4l2_dev->ctrl_handler;
+		/* If the prio state pointer is NULL, and if the driver doesn't
+		   handle priorities itself, then use the v4l2_device prio
+		   state. */
+		if (vdev->prio == NULL && vdev->ioctl_ops &&
+				vdev->ioctl_ops->vidioc_s_priority == NULL)
+			vdev->prio = &vdev->v4l2_dev->prio;
 	}
 
 	/* Part 2: find a free minor, device node number and device index. */
diff --git a/drivers/media/video/v4l2-device.c b/drivers/media/video/v4l2-device.c
index 7fe6f92..e12844c 100644
--- a/drivers/media/video/v4l2-device.c
+++ b/drivers/media/video/v4l2-device.c
@@ -36,6 +36,7 @@ int v4l2_device_register(struct device *dev, struct v4l2_device *v4l2_dev)
 	INIT_LIST_HEAD(&v4l2_dev->subdevs);
 	spin_lock_init(&v4l2_dev->lock);
 	mutex_init(&v4l2_dev->ioctl_lock);
+	v4l2_prio_init(&v4l2_dev->prio);
 	v4l2_dev->dev = dev;
 	if (dev == NULL) {
 		/* If dev == NULL, then name must be filled in by the caller */
diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
index 861f323..15dd756 100644
--- a/include/media/v4l2-dev.h
+++ b/include/media/v4l2-dev.h
@@ -83,6 +83,9 @@ struct video_device
 	/* Control handler associated with this device node. May be NULL. */
 	struct v4l2_ctrl_handler *ctrl_handler;
 
+	/* Priority state. If NULL, then v4l2_dev->prio will be used. */
+	struct v4l2_prio_state *prio;
+
 	/* device info */
 	char name[32];
 	int vfl_type;
diff --git a/include/media/v4l2-device.h b/include/media/v4l2-device.h
index b16f307..fd5d450 100644
--- a/include/media/v4l2-device.h
+++ b/include/media/v4l2-device.h
@@ -22,6 +22,7 @@
 #define _V4L2_DEVICE_H
 
 #include <media/v4l2-subdev.h>
+#include <media/v4l2-dev.h>
 
 /* Each instance of a V4L2 device should create the v4l2_device struct,
    either stand-alone or embedded in a larger struct.
@@ -51,6 +52,8 @@ struct v4l2_device {
 			unsigned int notification, void *arg);
 	/* The control handler. May be NULL. */
 	struct v4l2_ctrl_handler *ctrl_handler;
+	/* Device's priority state */
+	struct v4l2_prio_state prio;
 	/* BKL replacement mutex. Temporary solution only. */
 	struct mutex ioctl_lock;
 };
-- 
1.6.4.2

[PATCH 03/10] [RFC] v4l2-fh: implement v4l2_priority support.

[Hans Verkuil]

Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
 drivers/media/video/v4l2-fh.c |    4 ++++
 include/media/v4l2-fh.h       |    1 +
 2 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/v4l2-fh.c b/drivers/media/video/v4l2-fh.c
index d78f184..78a1608 100644
--- a/drivers/media/video/v4l2-fh.c
+++ b/drivers/media/video/v4l2-fh.c
@@ -33,6 +33,8 @@ int v4l2_fh_init(struct v4l2_fh *fh, struct video_device *vdev)
 	fh->vdev = vdev;
 	INIT_LIST_HEAD(&fh->list);
 	set_bit(V4L2_FL_USES_V4L2_FH, &fh->vdev->flags);
+	fh->prio = V4L2_PRIORITY_UNSET;
+	BUG_ON(vdev->prio == NULL);
 
 	/*
 	 * fh->events only needs to be initialized if the driver
@@ -51,6 +53,7 @@ void v4l2_fh_add(struct v4l2_fh *fh)
 {
 	unsigned long flags;
 
+	v4l2_prio_open(fh->vdev->prio, &fh->prio);
 	spin_lock_irqsave(&fh->vdev->fh_lock, flags);
 	list_add(&fh->list, &fh->vdev->fh_list);
 	spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
@@ -64,6 +67,7 @@ void v4l2_fh_del(struct v4l2_fh *fh)
 	spin_lock_irqsave(&fh->vdev->fh_lock, flags);
 	list_del_init(&fh->list);
 	spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
+	v4l2_prio_close(fh->vdev->prio, fh->prio);
 }
 EXPORT_SYMBOL_GPL(v4l2_fh_del);
 
diff --git a/include/media/v4l2-fh.h b/include/media/v4l2-fh.h
index 1d72dde..5fc5ba9 100644
--- a/include/media/v4l2-fh.h
+++ b/include/media/v4l2-fh.h
@@ -35,6 +35,7 @@ struct v4l2_fh {
 	struct list_head	list;
 	struct video_device	*vdev;
 	struct v4l2_events      *events; /* events, pending and subscribed */
+	enum v4l2_priority	prio;
 };
 
 /*
-- 
1.6.4.2

[PATCH 04/10] [RFC] v4l2-dev: add and support flag V4L2_FH_USE_PRIO.

[Hans Verkuil]

Flag V4L2_FH_USE_PRIO is set if we can safely store the priority level
in file->private_data. Support this flag for the open and release file
operations.

Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
 drivers/media/video/v4l2-dev.c |   23 +++++++++++++++++++++--
 include/media/v4l2-dev.h       |    6 ++++++
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-dev.c
index c8f6ae1..96ec954 100644
--- a/drivers/media/video/v4l2-dev.c
+++ b/drivers/media/video/v4l2-dev.c
@@ -380,6 +380,15 @@ static int v4l2_open(struct inode *inode, struct file *filp)
 	/* and increase the device refcount */
 	video_get(vdev);
 	mutex_unlock(&videodev_lock);
+
+	if (test_bit(V4L2_FL_USES_V4L2_PRIO, &vdev->flags)) {
+		enum v4l2_priority prio =
+			(enum v4l2_priority)filp->private_data;
+
+		v4l2_prio_open(vdev->prio, &prio);
+		filp->private_data = (void *)prio;
+	}
+
 	if (vdev->fops->open) {
 		if (vdev->lock && mutex_lock_interruptible(vdev->lock)) {
 			ret = -ERESTARTSYS;
@@ -404,8 +413,13 @@ err:
 static int v4l2_release(struct inode *inode, struct file *filp)
 {
 	struct video_device *vdev = video_devdata(filp);
-	int ret = 0;
 
+	if (test_bit(V4L2_FL_USES_V4L2_PRIO, &vdev->flags)) {
+		enum v4l2_priority prio =
+			(enum v4l2_priority)filp->private_data;
+
+		v4l2_prio_close(vdev->prio, prio);
+	}
 	if (vdev->fops->release) {
 		if (vdev->lock)
 			mutex_lock(vdev->lock);
@@ -417,7 +431,7 @@ static int v4l2_release(struct inode *inode, struct file *filp)
 	/* decrease the refcount unconditionally since the release()
 	   return value is ignored. */
 	video_put(vdev);
-	return ret;
+	return 0;
 }
 
 static const struct file_operations v4l2_fops = {
@@ -612,6 +626,11 @@ static int __video_register_device(struct video_device *vdev, int type, int nr,
 	vdev->index = get_index(vdev);
 	mutex_unlock(&videodev_lock);
 
+	/* Determine if we can store v4l2_priority in file->private_data */
+	if (vdev->fops->open == NULL && vdev->fops->release == NULL &&
+	    vdev->prio != NULL)
+		set_bit(V4L2_FL_USES_V4L2_PRIO, &vdev->flags);
+
 	/* Part 3: Initialize the character device */
 	vdev->cdev = cdev_alloc();
 	if (vdev->cdev == NULL) {
diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
index 15dd756..045dffa 100644
--- a/include/media/v4l2-dev.h
+++ b/include/media/v4l2-dev.h
@@ -32,7 +32,13 @@ struct v4l2_ctrl_handler;
    Drivers can clear this flag if they want to block all future
    device access. It is cleared by video_unregister_device. */
 #define V4L2_FL_REGISTERED	(0)
+/* Flag to mark that the driver stores a pointer to struct v4l2_fh in
+   file->private_data. Is automatically detected. */
 #define V4L2_FL_USES_V4L2_FH	(1)
+/* Flag to mark that the driver stores the local v4l2_priority in
+   file->private_data. Will be set if open and release are both NULL.
+   Drivers can also set it manually. */
+#define V4L2_FL_USES_V4L2_PRIO	(2)
 
 /* Priority helper functions */
 
-- 
1.6.4.2

[PATCH 05/10] [RFC] v4l2-ioctl: add priority handling support.

[Hans Verkuil]

Drivers that use either v4l2_fh or don't use filehandles at all can now
use the core framework support of g/s_priority.

Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
 drivers/media/radio/radio-si4713.c         |    3 +-
 drivers/media/video/cx18/cx18-ioctl.c      |    3 +-
 drivers/media/video/davinci/vpfe_capture.c |    2 +-
 drivers/media/video/ivtv/ivtv-ioctl.c      |    3 +-
 drivers/media/video/meye.c                 |    3 +-
 drivers/media/video/mxb.c                  |    3 +-
 drivers/media/video/v4l2-ioctl.c           |   73 +++++++++++++++++++++++++---
 include/media/v4l2-ioctl.h                 |    2 +-
 8 files changed, 78 insertions(+), 14 deletions(-)

diff --git a/drivers/media/radio/radio-si4713.c b/drivers/media/radio/radio-si4713.c
index 726d367..444b4cf 100644
--- a/drivers/media/radio/radio-si4713.c
+++ b/drivers/media/radio/radio-si4713.c
@@ -224,7 +224,8 @@ static int radio_si4713_s_frequency(struct file *file, void *p,
 							s_frequency, vf);
 }
 
-static long radio_si4713_default(struct file *file, void *p, int cmd, void *arg)
+static long radio_si4713_default(struct file *file, void *p,
+				bool valid_prio, int cmd, void *arg)
 {
 	return v4l2_device_call_until_err(get_v4l2_dev(file), 0, core,
 							ioctl, cmd, arg);
diff --git a/drivers/media/video/cx18/cx18-ioctl.c b/drivers/media/video/cx18/cx18-ioctl.c
index 7150195..6b5bb3e 100644
--- a/drivers/media/video/cx18/cx18-ioctl.c
+++ b/drivers/media/video/cx18/cx18-ioctl.c
@@ -1056,7 +1056,8 @@ static int cx18_log_status(struct file *file, void *fh)
 	return 0;
 }
 
-static long cx18_default(struct file *file, void *fh, int cmd, void *arg)
+static long cx18_default(struct file *file, void *fh, bool valid_prio,
+							int cmd, void *arg)
 {
 	struct cx18 *cx = ((struct cx18_open_id *)fh)->cx;
 
diff --git a/drivers/media/video/davinci/vpfe_capture.c b/drivers/media/video/davinci/vpfe_capture.c
index 353eada..71e961e 100644
--- a/drivers/media/video/davinci/vpfe_capture.c
+++ b/drivers/media/video/davinci/vpfe_capture.c
@@ -1719,7 +1719,7 @@ unlock_out:
 
 
 static long vpfe_param_handler(struct file *file, void *priv,
-		int cmd, void *param)
+		bool valid_prio, int cmd, void *param)
 {
 	struct vpfe_device *vpfe_dev = video_drvdata(file);
 	int ret = 0;
diff --git a/drivers/media/video/ivtv/ivtv-ioctl.c b/drivers/media/video/ivtv/ivtv-ioctl.c
index b686da5..d9386a7 100644
--- a/drivers/media/video/ivtv/ivtv-ioctl.c
+++ b/drivers/media/video/ivtv/ivtv-ioctl.c
@@ -1795,7 +1795,8 @@ static int ivtv_decoder_ioctls(struct file *filp, unsigned int cmd, void *arg)
 	return 0;
 }
 
-static long ivtv_default(struct file *file, void *fh, int cmd, void *arg)
+static long ivtv_default(struct file *file, void *fh, bool valid_prio,
+						int cmd, void *arg)
 {
 	struct ivtv *itv = ((struct ivtv_open_id *)fh)->itv;
 
diff --git a/drivers/media/video/meye.c b/drivers/media/video/meye.c
index 48d2c24..b09a3c8 100644
--- a/drivers/media/video/meye.c
+++ b/drivers/media/video/meye.c
@@ -1547,7 +1547,8 @@ static int vidioc_streamoff(struct file *file, void *fh, enum v4l2_buf_type i)
 	return 0;
 }
 
-static long vidioc_default(struct file *file, void *fh, int cmd, void *arg)
+static long vidioc_default(struct file *file, void *fh, bool valid_prio,
+						int cmd, void *arg)
 {
 	switch (cmd) {
 	case MEYEIOC_G_PARAMS:
diff --git a/drivers/media/video/mxb.c b/drivers/media/video/mxb.c
index 4e8fd96..507bed2 100644
--- a/drivers/media/video/mxb.c
+++ b/drivers/media/video/mxb.c
@@ -643,7 +643,8 @@ static int vidioc_s_register(struct file *file, void *fh, struct v4l2_dbg_regist
 }
 #endif
 
-static long vidioc_default(struct file *file, void *fh, int cmd, void *arg)
+static long vidioc_default(struct file *file, void *fh, bool valid_prio,
+							int cmd, void *arg)
 {
 	struct saa7146_dev *dev = ((struct saa7146_fh *)fh)->dev;
 	struct mxb *mxb = (struct mxb *)dev->ext_priv;
diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c
index 7e47f15..d682a4c 100644
--- a/drivers/media/video/v4l2-ioctl.c
+++ b/drivers/media/video/v4l2-ioctl.c
@@ -25,6 +25,7 @@
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-fh.h>
 #include <media/v4l2-event.h>
+#include <media/v4l2-device.h>
 #include <media/v4l2-chip-ident.h>
 
 #define dbgarg(cmd, fmt, arg...) \
@@ -564,6 +565,7 @@ static long __video_do_ioctl(struct file *file,
 {
 	struct video_device *vfd = video_devdata(file);
 	const struct v4l2_ioctl_ops *ops = vfd->ioctl_ops;
+	enum v4l2_priority prio = V4L2_PRIORITY_UNSET;
 	void *fh = file->private_data;
 	long ret = -EINVAL;
 
@@ -579,6 +581,43 @@ static long __video_do_ioctl(struct file *file,
 		printk(KERN_CONT "\n");
 	}
 
+	if (test_bit(V4L2_FL_USES_V4L2_FH, &vfd->flags))
+		prio = ((struct v4l2_fh *)fh)->prio;
+	else if (test_bit(V4L2_FL_USES_V4L2_PRIO, &vfd->flags))
+		prio = (enum v4l2_priority)fh;
+	if (prio != V4L2_PRIORITY_UNSET) {
+		switch (cmd) {
+		case VIDIOC_S_CTRL:
+		case VIDIOC_S_STD:
+		case VIDIOC_S_INPUT:
+		case VIDIOC_S_OUTPUT:
+		case VIDIOC_S_TUNER:
+		case VIDIOC_S_FREQUENCY:
+		case VIDIOC_S_FMT:
+		case VIDIOC_S_CROP:
+		case VIDIOC_S_AUDIO:
+		case VIDIOC_S_AUDOUT:
+		case VIDIOC_S_EXT_CTRLS:
+		case VIDIOC_S_FBUF:
+		case VIDIOC_S_PRIORITY:
+		case VIDIOC_S_DV_PRESET:
+		case VIDIOC_S_DV_TIMINGS:
+		case VIDIOC_S_JPEGCOMP:
+		case VIDIOC_S_MODULATOR:
+		case VIDIOC_S_PARM:
+		case VIDIOC_S_HW_FREQ_SEEK:
+		case VIDIOC_ENCODER_CMD:
+		case VIDIOC_OVERLAY:
+		case VIDIOC_REQBUFS:
+		case VIDIOC_STREAMON:
+		case VIDIOC_STREAMOFF:
+			ret = v4l2_prio_check(vfd->prio, prio);
+			if (ret)
+				goto exit_prio;
+			break;
+		}
+	}
+
 	switch (cmd) {
 
 	/* --- capabilities ------------------------------------------ */
@@ -605,9 +644,14 @@ static long __video_do_ioctl(struct file *file,
 	{
 		enum v4l2_priority *p = arg;
 
-		if (!ops->vidioc_g_priority)
-			break;
-		ret = ops->vidioc_g_priority(file, fh, p);
+		if (!ops->vidioc_g_priority) {
+			if (prio == V4L2_PRIORITY_UNSET)
+				break;
+			*p = v4l2_prio_max(&vfd->v4l2_dev->prio);
+			ret = 0;
+		} else {
+			ret = ops->vidioc_g_priority(file, fh, p);
+		}
 		if (!ret)
 			dbgarg(cmd, "priority is %d\n", *p);
 		break;
@@ -616,10 +660,20 @@ static long __video_do_ioctl(struct file *file,
 	{
 		enum v4l2_priority *p = arg;
 
-		if (!ops->vidioc_s_priority)
-			break;
+		if (!ops->vidioc_s_priority && prio == V4L2_PRIORITY_UNSET)
+				break;
 		dbgarg(cmd, "setting priority to %d\n", *p);
-		ret = ops->vidioc_s_priority(file, fh, *p);
+		if (ops->vidioc_s_priority) {
+			ret = ops->vidioc_s_priority(file, fh, *p);
+		} else {
+			ret = v4l2_prio_change(&vfd->v4l2_dev->prio, &prio, *p);
+			if (!ret) {
+				if (test_bit(V4L2_FL_USES_V4L2_FH, &vfd->flags))
+					((struct v4l2_fh *)fh)->prio = prio;
+				else
+					file->private_data = (void *)prio;
+			}
+		}
 		break;
 	}
 
@@ -1938,13 +1992,18 @@ static long __video_do_ioctl(struct file *file,
 	}
 	default:
 	{
+		bool valid_prio = true;
+
 		if (!ops->vidioc_default)
 			break;
-		ret = ops->vidioc_default(file, fh, cmd, arg);
+		if (prio != V4L2_PRIORITY_UNSET)
+			valid_prio = v4l2_prio_check(vfd->prio, prio) >= 0;
+		ret = ops->vidioc_default(file, fh, valid_prio, cmd, arg);
 		break;
 	}
 	} /* switch */
 
+exit_prio:
 	if (vfd->debug & V4L2_DEBUG_IOCTL_ARG) {
 		if (ret < 0) {
 			v4l_print_ioctl(vfd->name, cmd);
diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
index 67df375..4d6303b 100644
--- a/include/media/v4l2-ioctl.h
+++ b/include/media/v4l2-ioctl.h
@@ -254,7 +254,7 @@ struct v4l2_ioctl_ops {
 
 	/* For other private ioctls */
 	long (*vidioc_default)	       (struct file *file, void *fh,
-					int cmd, void *arg);
+					bool valid_prio, int cmd, void *arg);
 };
 
 
-- 
1.6.4.2

[PATCH 06/10] [RFC] ivtv: convert to core priority handling.

[Hans Verkuil]

Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
 drivers/media/video/ivtv/ivtv-driver.h  |    2 -
 drivers/media/video/ivtv/ivtv-fileops.c |    2 -
 drivers/media/video/ivtv/ivtv-ioctl.c   |   56 ++++++++----------------------
 3 files changed, 15 insertions(+), 45 deletions(-)

diff --git a/drivers/media/video/ivtv/ivtv-driver.h b/drivers/media/video/ivtv/ivtv-driver.h
index 04bacdb..84bdf0f 100644
--- a/drivers/media/video/ivtv/ivtv-driver.h
+++ b/drivers/media/video/ivtv/ivtv-driver.h
@@ -383,7 +383,6 @@ struct ivtv_open_id {
 	u32 open_id;                    /* unique ID for this file descriptor */
 	int type;                       /* stream type */
 	int yuv_frames;                 /* 1: started OUT_UDMA_YUV output mode */
-	enum v4l2_priority prio;        /* priority */
 	struct ivtv *itv;
 };
 
@@ -710,7 +709,6 @@ struct ivtv {
 
 	/* Miscellaneous */
 	u32 open_id;			/* incremented each time an open occurs, is >= 1 */
-	struct v4l2_prio_state prio;    /* priority state */
 	int search_pack_header;         /* 1 if ivtv_copy_buf_to_user() is scanning for a pack header (0xba) */
 	int speed;                      /* current playback speed setting */
 	u8 speed_mute_audio;            /* 1 if audio should be muted when fast forward */
diff --git a/drivers/media/video/ivtv/ivtv-fileops.c b/drivers/media/video/ivtv/ivtv-fileops.c
index c57a585..4463bf4 100644
--- a/drivers/media/video/ivtv/ivtv-fileops.c
+++ b/drivers/media/video/ivtv/ivtv-fileops.c
@@ -856,7 +856,6 @@ int ivtv_v4l2_close(struct file *filp)
 
 	IVTV_DEBUG_FILE("close %s\n", s->name);
 
-	v4l2_prio_close(&itv->prio, id->prio);
 	v4l2_fh_del(fh);
 	v4l2_fh_exit(fh);
 
@@ -973,7 +972,6 @@ static int ivtv_serialized_open(struct ivtv_stream *s, struct file *filp)
 	}
 	item->itv = itv;
 	item->type = s->type;
-	v4l2_prio_open(&itv->prio, &item->prio);
 
 	item->open_id = itv->open_id++;
 	filp->private_data = &item->fh;
diff --git a/drivers/media/video/ivtv/ivtv-ioctl.c b/drivers/media/video/ivtv/ivtv-ioctl.c
index d9386a7..6fb1837 100644
--- a/drivers/media/video/ivtv/ivtv-ioctl.c
+++ b/drivers/media/video/ivtv/ivtv-ioctl.c
@@ -750,23 +750,6 @@ static int ivtv_s_register(struct file *file, void *fh, struct v4l2_dbg_register
 }
 #endif
 
-static int ivtv_g_priority(struct file *file, void *fh, enum v4l2_priority *p)
-{
-	struct ivtv *itv = ((struct ivtv_open_id *)fh)->itv;
-
-	*p = v4l2_prio_max(&itv->prio);
-
-	return 0;
-}
-
-static int ivtv_s_priority(struct file *file, void *fh, enum v4l2_priority prio)
-{
-	struct ivtv_open_id *id = fh;
-	struct ivtv *itv = id->itv;
-
-	return v4l2_prio_change(&itv->prio, &id->prio, prio);
-}
-
 static int ivtv_querycap(struct file *file, void *fh, struct v4l2_capability *vcap)
 {
 	struct ivtv *itv = ((struct ivtv_open_id *)fh)->itv;
@@ -1800,6 +1783,21 @@ static long ivtv_default(struct file *file, void *fh, bool valid_prio,
 {
 	struct ivtv *itv = ((struct ivtv_open_id *)fh)->itv;
 
+	if (!valid_prio) {
+		switch (cmd) {
+		case VIDEO_PLAY:
+		case VIDEO_STOP:
+		case VIDEO_FREEZE:
+		case VIDEO_CONTINUE:
+		case VIDEO_COMMAND:
+		case VIDEO_SELECT_SOURCE:
+		case AUDIO_SET_MUTE:
+		case AUDIO_CHANNEL_SELECT:
+		case AUDIO_BILINGUAL_CHANNEL_SELECT:
+			return -EBUSY;
+		}
+	}
+
 	switch (cmd) {
 	case VIDIOC_INT_RESET: {
 		u32 val = *(u32 *)arg;
@@ -1837,30 +1835,8 @@ static long ivtv_serialized_ioctl(struct ivtv *itv, struct file *filp,
 		unsigned int cmd, unsigned long arg)
 {
 	struct video_device *vfd = video_devdata(filp);
-	struct ivtv_open_id *id = fh2id(filp->private_data);
 	long ret;
 
-	/* check priority */
-	switch (cmd) {
-	case VIDIOC_S_CTRL:
-	case VIDIOC_S_STD:
-	case VIDIOC_S_INPUT:
-	case VIDIOC_S_OUTPUT:
-	case VIDIOC_S_TUNER:
-	case VIDIOC_S_FREQUENCY:
-	case VIDIOC_S_FMT:
-	case VIDIOC_S_CROP:
-	case VIDIOC_S_AUDIO:
-	case VIDIOC_S_AUDOUT:
-	case VIDIOC_S_EXT_CTRLS:
-	case VIDIOC_S_FBUF:
-	case VIDIOC_S_PRIORITY:
-	case VIDIOC_OVERLAY:
-		ret = v4l2_prio_check(&itv->prio, id->prio);
-		if (ret)
-			return ret;
-	}
-
 	if (ivtv_debug & IVTV_DBGFLG_IOCTL)
 		vfd->debug = V4L2_DEBUG_IOCTL | V4L2_DEBUG_IOCTL_ARG;
 	ret = video_ioctl2(filp, cmd, arg);
@@ -1885,8 +1861,6 @@ long ivtv_v4l2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 
 static const struct v4l2_ioctl_ops ivtv_ioctl_ops = {
 	.vidioc_querycap    		    = ivtv_querycap,
-	.vidioc_g_priority  		    = ivtv_g_priority,
-	.vidioc_s_priority  		    = ivtv_s_priority,
 	.vidioc_s_audio     		    = ivtv_s_audio,
 	.vidioc_g_audio     		    = ivtv_g_audio,
 	.vidioc_enumaudio   		    = ivtv_enumaudio,
-- 
1.6.4.2

[PATCH 07/10] [RFC] ivtv: use core-assisted locking.

[Hans Verkuil]

Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
 drivers/media/video/ivtv/ivtv-fileops.c |   15 +--------------
 drivers/media/video/ivtv/ivtv-ioctl.c   |   18 +-----------------
 drivers/media/video/ivtv/ivtv-streams.c |    1 +
 3 files changed, 3 insertions(+), 31 deletions(-)

diff --git a/drivers/media/video/ivtv/ivtv-fileops.c b/drivers/media/video/ivtv/ivtv-fileops.c
index 4463bf4..ae6e266 100644
--- a/drivers/media/video/ivtv/ivtv-fileops.c
+++ b/drivers/media/video/ivtv/ivtv-fileops.c
@@ -507,9 +507,7 @@ ssize_t ivtv_v4l2_read(struct file * filp, char __user *buf, size_t count, loff_
 
 	IVTV_DEBUG_HI_FILE("read %zd bytes from %s\n", count, s->name);
 
-	mutex_lock(&itv->serialize_lock);
 	rc = ivtv_start_capture(id);
-	mutex_unlock(&itv->serialize_lock);
 	if (rc)
 		return rc;
 	return ivtv_read_pos(s, buf, count, pos, filp->f_flags & O_NONBLOCK);
@@ -584,9 +582,7 @@ ssize_t ivtv_v4l2_write(struct file *filp, const char __user *user_buf, size_t c
 	set_bit(IVTV_F_S_APPL_IO, &s->s_flags);
 
 	/* Start decoder (returns 0 if already started) */
-	mutex_lock(&itv->serialize_lock);
 	rc = ivtv_start_decoding(id, itv->speed);
-	mutex_unlock(&itv->serialize_lock);
 	if (rc) {
 		IVTV_DEBUG_WARN("Failed start decode stream %s\n", s->name);
 
@@ -755,9 +751,7 @@ unsigned int ivtv_v4l2_enc_poll(struct file *filp, poll_table * wait)
 	if (!eof && !test_bit(IVTV_F_S_STREAMING, &s->s_flags)) {
 		int rc;
 
-		mutex_lock(&itv->serialize_lock);
 		rc = ivtv_start_capture(id);
-		mutex_unlock(&itv->serialize_lock);
 		if (rc) {
 			IVTV_DEBUG_INFO("Could not start capture for %s (%d)\n",
 					s->name, rc);
@@ -868,7 +862,6 @@ int ivtv_v4l2_close(struct file *filp)
 	/* 'Unclaim' this stream */
 
 	/* Stop radio */
-	mutex_lock(&itv->serialize_lock);
 	if (id->type == IVTV_ENC_STREAM_TYPE_RAD) {
 		/* Closing radio device, return to TV mode */
 		ivtv_mute(itv);
@@ -906,7 +899,6 @@ int ivtv_v4l2_close(struct file *filp)
 		ivtv_stop_capture(id, 0);
 	}
 	kfree(id);
-	mutex_unlock(&itv->serialize_lock);
 	return 0;
 }
 
@@ -1026,7 +1018,6 @@ static int ivtv_serialized_open(struct ivtv_stream *s, struct file *filp)
 
 int ivtv_v4l2_open(struct file *filp)
 {
-	int res;
 	struct ivtv *itv = NULL;
 	struct ivtv_stream *s = NULL;
 	struct video_device *vdev = video_devdata(filp);
@@ -1034,16 +1025,12 @@ int ivtv_v4l2_open(struct file *filp)
 	s = video_get_drvdata(vdev);
 	itv = s->itv;
 
-	mutex_lock(&itv->serialize_lock);
 	if (ivtv_init_on_first_open(itv)) {
 		IVTV_ERR("Failed to initialize on device %s\n",
 			 video_device_node_name(vdev));
-		mutex_unlock(&itv->serialize_lock);
 		return -ENXIO;
 	}
-	res = ivtv_serialized_open(s, filp);
-	mutex_unlock(&itv->serialize_lock);
-	return res;
+	return ivtv_serialized_open(s, filp);
 }
 
 void ivtv_mute(struct ivtv *itv)
diff --git a/drivers/media/video/ivtv/ivtv-ioctl.c b/drivers/media/video/ivtv/ivtv-ioctl.c
index 6fb1837..d65c4ed 100644
--- a/drivers/media/video/ivtv/ivtv-ioctl.c
+++ b/drivers/media/video/ivtv/ivtv-ioctl.c
@@ -1831,8 +1831,7 @@ static long ivtv_default(struct file *file, void *fh, bool valid_prio,
 	return 0;
 }
 
-static long ivtv_serialized_ioctl(struct ivtv *itv, struct file *filp,
-		unsigned int cmd, unsigned long arg)
+long ivtv_v4l2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 {
 	struct video_device *vfd = video_devdata(filp);
 	long ret;
@@ -1844,21 +1843,6 @@ static long ivtv_serialized_ioctl(struct ivtv *itv, struct file *filp,
 	return ret;
 }
 
-long ivtv_v4l2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
-{
-	struct ivtv_open_id *id = fh2id(filp->private_data);
-	struct ivtv *itv = id->itv;
-	long res;
-
-	/* DQEVENT can block, so this should not run with the serialize lock */
-	if (cmd == VIDIOC_DQEVENT)
-		return ivtv_serialized_ioctl(itv, filp, cmd, arg);
-	mutex_lock(&itv->serialize_lock);
-	res = ivtv_serialized_ioctl(itv, filp, cmd, arg);
-	mutex_unlock(&itv->serialize_lock);
-	return res;
-}
-
 static const struct v4l2_ioctl_ops ivtv_ioctl_ops = {
 	.vidioc_querycap    		    = ivtv_querycap,
 	.vidioc_s_audio     		    = ivtv_s_audio,
diff --git a/drivers/media/video/ivtv/ivtv-streams.c b/drivers/media/video/ivtv/ivtv-streams.c
index 512607e..b588ffc 100644
--- a/drivers/media/video/ivtv/ivtv-streams.c
+++ b/drivers/media/video/ivtv/ivtv-streams.c
@@ -214,6 +214,7 @@ static int ivtv_prep_dev(struct ivtv *itv, int type)
 	s->vdev->fops = ivtv_stream_info[type].fops;
 	s->vdev->release = video_device_release;
 	s->vdev->tvnorms = V4L2_STD_ALL;
+	s->vdev->lock = &itv->serialize_lock;
 	ivtv_set_funcs(s->vdev);
 	return 0;
 }
-- 
1.6.4.2

[PATCH 08/10] [RFC] v4l2-framework: update documentation for new prio field

[Hans Verkuil]

Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
 Documentation/video4linux/v4l2-framework.txt |   19 +++++++++++++++++--
 1 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/Documentation/video4linux/v4l2-framework.txt b/Documentation/video4linux/v4l2-framework.txt
index f22f35c..7739705 100644
--- a/Documentation/video4linux/v4l2-framework.txt
+++ b/Documentation/video4linux/v4l2-framework.txt
@@ -448,15 +448,20 @@ allocated memory.
 You should also set these fields:
 
 - v4l2_dev: set to the v4l2_device parent device.
+
 - name: set to something descriptive and unique.
+
 - fops: set to the v4l2_file_operations struct.
+
 - ioctl_ops: if you use the v4l2_ioctl_ops to simplify ioctl maintenance
   (highly recommended to use this and it might become compulsory in the
   future!), then set this to your v4l2_ioctl_ops struct.
+
 - lock: leave to NULL if you want to do all the locking in the driver.
   Otherwise you give it a pointer to a struct mutex_lock and before any
   of the v4l2_file_operations is called this lock will be taken by the
   core and released afterwards.
+
 - parent: you only set this if v4l2_device was registered with NULL as
   the parent device struct. This only happens in cases where one hardware
   device has multiple PCI devices that all share the same v4l2_device core.
@@ -467,8 +472,18 @@ You should also set these fields:
   PCI device it is setup without a parent device. But when the struct
   video_device is setup you do know which parent PCI device to use.
 
-If you use v4l2_ioctl_ops, then you should set either .unlocked_ioctl or
-.ioctl to video_ioctl2 in your v4l2_file_operations struct.
+- prio: if left to NULL, then the prio state in struct v4l2_device will be
+  used, otherwise you can point it to your own struct v4l2_prio_state.
+  This field is used for checking priorities (see VIDIOC_S_PRIORITY). In most
+  cases this field remains NULL.
+
+  Only if the driver has multiple device nodes and some of those can be used
+  independently from others (e.g. capture and display nodes are often
+  independent), then you need to have multiple v4l2_prio_state structs and
+  point this field to the correct one.
+
+If you use v4l2_ioctl_ops, then you should set .unlocked_ioctl to video_ioctl2
+in your v4l2_file_operations struct.
 
 The v4l2_file_operations struct is a subset of file_operations. The main
 difference is that the inode argument is omitted since it is never used.
-- 
1.6.4.2

[PATCH 09/10] [RFC] v4l2-device: add kref and a release function

[Hans Verkuil]

The video_device struct has proper ref counting and its release function
will be called when the last user releases it. But no such support was
available for struct v4l2_device. This made it hard to determine when a
USB driver can release the device if it has multiple device nodes.

With one device node it is easy of course, since when the device node is
released, the whole device can be released.

This patch adds refcounting to v4l2_device. When registering device nodes
the v4l2_device refcount will be increased, when releasing device nodes
it will be decreased. The (optional) release function will be called when
the last device node was released.

Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
 drivers/media/video/v4l2-dev.c    |    8 ++++++++
 drivers/media/video/v4l2-device.c |   15 +++++++++++++++
 include/media/v4l2-device.h       |   11 +++++++++++
 3 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-dev.c
index 96ec954..c83d8a9 100644
--- a/drivers/media/video/v4l2-dev.c
+++ b/drivers/media/video/v4l2-dev.c
@@ -143,6 +143,7 @@ static inline void video_put(struct video_device *vdev)
 static void v4l2_device_release(struct device *cd)
 {
 	struct video_device *vdev = to_video_device(cd);
+	struct v4l2_device *v4l2_dev = vdev->v4l2_dev;
 
 	mutex_lock(&videodev_lock);
 	if (video_device[vdev->minor] != vdev) {
@@ -169,6 +170,10 @@ static void v4l2_device_release(struct device *cd)
 	/* Release video_device and perform other
 	   cleanups as needed. */
 	vdev->release(vdev);
+
+	/* Decrease v4l2_device refcount */
+	if (v4l2_dev)
+		v4l2_device_put(v4l2_dev);
 }
 
 static struct class video_class = {
@@ -670,6 +675,9 @@ static int __video_register_device(struct video_device *vdev, int type, int nr,
 		printk(KERN_WARNING "%s: requested %s%d, got %s\n", __func__,
 			name_base, nr, video_device_node_name(vdev));
 
+	/* Increase v4l2_device refcount */
+	if (vdev->v4l2_dev)
+		v4l2_device_get(vdev->v4l2_dev);
 	/* Part 5: Activate this minor. The char device can now be used. */
 	set_bit(V4L2_FL_REGISTERED, &vdev->flags);
 	mutex_lock(&videodev_lock);
diff --git a/drivers/media/video/v4l2-device.c b/drivers/media/video/v4l2-device.c
index e12844c..4124016 100644
--- a/drivers/media/video/v4l2-device.c
+++ b/drivers/media/video/v4l2-device.c
@@ -55,6 +55,21 @@ int v4l2_device_register(struct device *dev, struct v4l2_device *v4l2_dev)
 }
 EXPORT_SYMBOL_GPL(v4l2_device_register);
 
+static void v4l2_device_release(struct kref *ref)
+{
+	struct v4l2_device *v4l2_dev =
+		container_of(ref, struct v4l2_device, ref);
+
+	if (v4l2_dev->release)
+		v4l2_dev->release(v4l2_dev);
+}
+
+int v4l2_device_put(struct v4l2_device *v4l2_dev)
+{
+	return kref_put(&v4l2_dev->ref, v4l2_device_release);
+}
+EXPORT_SYMBOL_GPL(v4l2_device_put);
+
 int v4l2_device_set_name(struct v4l2_device *v4l2_dev, const char *basename,
 						atomic_t *instance)
 {
diff --git a/include/media/v4l2-device.h b/include/media/v4l2-device.h
index fd5d450..44763bc 100644
--- a/include/media/v4l2-device.h
+++ b/include/media/v4l2-device.h
@@ -56,8 +56,19 @@ struct v4l2_device {
 	struct v4l2_prio_state prio;
 	/* BKL replacement mutex. Temporary solution only. */
 	struct mutex ioctl_lock;
+	/* Keep track of the references to this struct. */
+	struct kref ref;
+	/* Release function that is called when the ref count goes to 0. */
+	void (*release)(struct v4l2_device *v4l2_dev);
 };
 
+static inline void v4l2_device_get(struct v4l2_device *v4l2_dev)
+{
+	kref_get(&v4l2_dev->ref);
+}
+
+int v4l2_device_put(struct v4l2_device *v4l2_dev);
+
 /* Initialize v4l2_dev and make dev->driver_data point to v4l2_dev.
    dev may be NULL in rare cases (ISA devices). In that case you
    must fill in the v4l2_dev->name field before calling this function. */
-- 
1.6.4.2

[PATCH 10/10] [RFC] v4l2-framework.txt: document new v4l2_device release() callback

[Hans Verkuil]

Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
 Documentation/video4linux/v4l2-framework.txt |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/Documentation/video4linux/v4l2-framework.txt b/Documentation/video4linux/v4l2-framework.txt
index 7739705..86ccdfa 100644
--- a/Documentation/video4linux/v4l2-framework.txt
+++ b/Documentation/video4linux/v4l2-framework.txt
@@ -167,6 +167,21 @@ static int __devinit drv_probe(struct pci_dev *pdev,
 	state->instance = atomic_inc_return(&drv_instance) - 1;
 }
 
+If you have multiple device nodes then it can be difficult to know when it is
+safe to unregister v4l2_device. For this purpose v4l2_device has refcounting
+support. The refcount is increased whenever video_register_device is called and
+it is decreased whenever that device node is released. When the refcount reaches
+zero, then the v4l2_device release() callback is called. You can do your final
+cleanup there.
+
+If other device nodes (e.g. ALSA) are created, then you can increase and
+decrease the refcount manually as well by calling:
+
+void v4l2_device_get(struct v4l2_device *v4l2_dev);
+
+or:
+
+int v4l2_device_put(struct v4l2_device *v4l2_dev);
 
 struct v4l2_subdev
 ------------------
-- 
1.6.4.2

Re: [PATCH 00/10] [RFC] Prio handling and v4l2_device release callback

[Mauro Carvalho Chehab]

Em 29-12-2010 19:43, Hans Verkuil escreveu:
> This patch series adds two new features to the V4L2 framework.
> 
> The first 5 patches add support for VIDIOC_G/S_PRIORITY. All prio handling
> will be done in the core for any driver that either uses struct v4l2_fh
> (ivtv only at the moment) or has no open and release file operations (true
> for many simple (radio) drivers). In all other cases the driver will have
> to do the work.

It doesn't make sense to implement this at core, and for some this will happen
automatically, while, for others, drivers need to do something.

> Eventually all drivers should either use v4l2_fh or never set filp->private_data.

I made a series of patches, due to BKL stuff converting the core to always
use v4l2_fh on all drivers. This seems to be the right solution for it.

> All drivers that use the control framework must also use core-prio handling
> since control handling is no longer done through v4l2_ioctl_ops, so the driver
> doesn't have an easy way of checking the priority before changing a control.
> 
> By default all device nodes use the same priority state in v4l2_device, but
> drivers can assign the prio field in video_device to a different priority state,
> allowing e.g. all capture device nodes to use a different priority from all
> the display device nodes.
> 
> The v4l2_ioctl.c code will check the ioctls whether or not they are allowed
> based on the current priority. The vidioc_default callback has a new argument
> that just tells the driver whether the prio is OK or not. The driver can use
> this argument to return -EBUSY for those commands that modify state of the driver.
> 
> The following three patches implement this in ivtv and update the documentation.
> 
> The last two patches add a release callback in v4l2_device which will be called
> when the last registered device node is removed. This is very useful for
> implementing the hotplug disconnect functionality as it provides a single clean
> up callback when the last user of any of the unregistered device nodes has
> closed its filehandle. For drivers with just a single device node this is not
> very relevant since the video_device release() does the same job, but for
> drivers that create multiple device nodes (e.g. usbvision) this is a must-have.
> 
> An example of how this would be used can be found in the dsbr100 patches in
> this branch:
> 
> http://git.linuxtv.org/hverkuil/media_tree.git?a=shortlog;h=refs/heads/usbvision
> 
> Comments?
> 
> Regards,
> 
> 	Hans
> 
> Hans Verkuil (10):
>   v4l2_prio: move from v4l2-common to v4l2-dev.
>   v4l2: add v4l2_prio_state to v4l2_device and video_device
>   v4l2-fh: implement v4l2_priority support.
>   v4l2-dev: add and support flag V4L2_FH_USE_PRIO.
>   v4l2-ioctl: add priority handling support.
>   ivtv: convert to core priority handling.
>   ivtv: use core-assisted locking.
>   v4l2-framework: update documentation for new prio field
>   v4l2-device: add kref and a release function
>   v4l2-framework.txt: document new v4l2_device release() callback
> 
>  Documentation/video4linux/v4l2-framework.txt |   34 ++++++++-
>  drivers/media/radio/radio-si4713.c           |    3 +-
>  drivers/media/video/cx18/cx18-ioctl.c        |    3 +-
>  drivers/media/video/davinci/vpfe_capture.c   |    2 +-
>  drivers/media/video/ivtv/ivtv-driver.h       |    2 -
>  drivers/media/video/ivtv/ivtv-fileops.c      |   17 +----
>  drivers/media/video/ivtv/ivtv-ioctl.c        |   77 +++++---------------
>  drivers/media/video/ivtv/ivtv-streams.c      |    1 +
>  drivers/media/video/meye.c                   |    3 +-
>  drivers/media/video/mxb.c                    |    3 +-
>  drivers/media/video/v4l2-common.c            |   63 ----------------
>  drivers/media/video/v4l2-dev.c               |  101 +++++++++++++++++++++++++-
>  drivers/media/video/v4l2-device.c            |   16 ++++
>  drivers/media/video/v4l2-fh.c                |    4 +
>  drivers/media/video/v4l2-ioctl.c             |   73 +++++++++++++++++--
>  include/media/v4l2-common.h                  |   15 ----
>  include/media/v4l2-dev.h                     |   24 ++++++
>  include/media/v4l2-device.h                  |   14 ++++
>  include/media/v4l2-fh.h                      |    1 +
>  include/media/v4l2-ioctl.h                   |    2 +-
>  20 files changed, 286 insertions(+), 172 deletions(-)
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/10] [RFC] Prio handling and v4l2_device release callback

[Hans Verkuil]

On Friday, December 31, 2010 12:01:17 Mauro Carvalho Chehab wrote:
> Em 29-12-2010 19:43, Hans Verkuil escreveu:
> > This patch series adds two new features to the V4L2 framework.
> > 
> > The first 5 patches add support for VIDIOC_G/S_PRIORITY. All prio handling
> > will be done in the core for any driver that either uses struct v4l2_fh
> > (ivtv only at the moment) or has no open and release file operations (true
> > for many simple (radio) drivers). In all other cases the driver will have
> > to do the work.
> 
> It doesn't make sense to implement this at core, and for some this will happen
> automatically, while, for others, drivers need to do something.

However, it makes it possible to gradually convert all drivers.
 
> > Eventually all drivers should either use v4l2_fh or never set filp->private_data.
> 
> I made a series of patches, due to BKL stuff converting the core to always
> use v4l2_fh on all drivers. This seems to be the right solution for it.

Can you point me to those patches? I remember seeing them, but can't remember where.

I see two potential problems with this approach:

1) A lot of drivers do not actually need to allocate a v4l2_fh struct, so it
   wastes memory. But on the other hand, it would be nicely consistent.

2) I prefer for core changes to have the least possible impact to existing drivers,
   and just convert existing drivers one by one.

But I would have to see your patch series again to see the impact of such a
change.

Regards,

	Hans

-- 
Hans Verkuil - video4linux developer - sponsored by Cisco

Re: [PATCH 00/10] [RFC] Prio handling and v4l2_device release callback

[Mauro Carvalho Chehab]

Em 31-12-2010 09:25, Hans Verkuil escreveu:
> On Friday, December 31, 2010 12:01:17 Mauro Carvalho Chehab wrote:
>> Em 29-12-2010 19:43, Hans Verkuil escreveu:
>>> This patch series adds two new features to the V4L2 framework.
>>>
>>> The first 5 patches add support for VIDIOC_G/S_PRIORITY. All prio handling
>>> will be done in the core for any driver that either uses struct v4l2_fh
>>> (ivtv only at the moment) or has no open and release file operations (true
>>> for many simple (radio) drivers). In all other cases the driver will have
>>> to do the work.
>>
>> It doesn't make sense to implement this at core, and for some this will happen
>> automatically, while, for others, drivers need to do something.
> 
> However, it makes it possible to gradually convert all drivers.

This will likely mean years of conversion.
>  
>>> Eventually all drivers should either use v4l2_fh or never set filp->private_data.
>>
>> I made a series of patches, due to BKL stuff converting the core to always
>> use v4l2_fh on all drivers. This seems to be the right solution for it.
> 
> Can you point me to those patches? I remember seeing them, but can't remember where.

They are at devel/bkl branch. The main one is this:

http://git.linuxtv.org/media_tree.git?a=commitdiff;h=285267378581fbf852f24f3f99d2e937cd200fd5

I remember you had some issues on it, but it is just a matter of fixing them.

> I see two potential problems with this approach:
> 
> 1) A lot of drivers do not actually need to allocate a v4l2_fh struct, so it
>    wastes memory. But on the other hand, it would be nicely consistent.

A typical driver allocates at least 2 buffers of 640x480x2. How much memory a
v4l2_fh struct would require? I didn't calculate, but probably less than 0.01%.
I don't think that the extra consumption of memory will have any real impact, even
on embedded. On the other hand, if you need to add the priority handling via code,
you'll probably waste more on codespace than the size of the struct.

> 2) I prefer for core changes to have the least possible impact to existing drivers,
>    and just convert existing drivers one by one.

A per-driver implementation means per-driver errors. A per-core implementation means
that, once it is fixed, all drivers will be OK. 

> But I would have to see your patch series again to see the impact of such a
> change.
> 

> Regards,
> 
> 	Hans
> 

Re: [PATCH 00/10] [RFC] Prio handling and v4l2_device release callback

[Hans Verkuil]

On Friday, December 31, 2010 12:39:09 Mauro Carvalho Chehab wrote:
> Em 31-12-2010 09:25, Hans Verkuil escreveu:
> > On Friday, December 31, 2010 12:01:17 Mauro Carvalho Chehab wrote:
> >> Em 29-12-2010 19:43, Hans Verkuil escreveu:
> >>> This patch series adds two new features to the V4L2 framework.
> >>>
> >>> The first 5 patches add support for VIDIOC_G/S_PRIORITY. All prio handling
> >>> will be done in the core for any driver that either uses struct v4l2_fh
> >>> (ivtv only at the moment) or has no open and release file operations (true
> >>> for many simple (radio) drivers). In all other cases the driver will have
> >>> to do the work.
> >>
> >> It doesn't make sense to implement this at core, and for some this will happen
> >> automatically, while, for others, drivers need to do something.
> > 
> > However, it makes it possible to gradually convert all drivers.
> 
> This will likely mean years of conversion.
> >  
> >>> Eventually all drivers should either use v4l2_fh or never set filp->private_data.
> >>
> >> I made a series of patches, due to BKL stuff converting the core to always
> >> use v4l2_fh on all drivers. This seems to be the right solution for it.
> > 
> > Can you point me to those patches? I remember seeing them, but can't remember where.
> 
> They are at devel/bkl branch. The main one is this:
> 
> http://git.linuxtv.org/media_tree.git?a=commitdiff;h=285267378581fbf852f24f3f99d2e937cd200fd5
> 
> I remember you had some issues on it, but it is just a matter of fixing them.

Thanks, I'll take a look.

> > I see two potential problems with this approach:
> > 
> > 1) A lot of drivers do not actually need to allocate a v4l2_fh struct, so it
> >    wastes memory. But on the other hand, it would be nicely consistent.
> 
> A typical driver allocates at least 2 buffers of 640x480x2. How much memory a
> v4l2_fh struct would require? I didn't calculate, but probably less than 0.01%.
> I don't think that the extra consumption of memory will have any real impact, even
> on embedded. On the other hand, if you need to add the priority handling via code,
> you'll probably waste more on codespace than the size of the struct.

Actually, I was thinking about the radio drivers which do not use buffers.

However, I realized that we also want to add an event that is sent whenever a
control changes value. It's not yet implemented, but it's easy to do with the
control framework. And since almost all drivers have controls, this also means
that all drivers need to use v4l2_fh since that is a prerequisite for events.

So it is indeed much better to create v4l2_fh structs in the core.

Regards,

	Hans

> 
> > 2) I prefer for core changes to have the least possible impact to existing drivers,
> >    and just convert existing drivers one by one.
> 
> A per-driver implementation means per-driver errors. A per-core implementation means
> that, once it is fixed, all drivers will be OK. 
> 
> > But I would have to see your patch series again to see the impact of such a
> > change.
> > 
> 
> > Regards,
> > 
> > 	Hans
> > 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
Hans Verkuil - video4linux developer - sponsored by Cisco