Prio handling and v4l2_device release callback
Subsystems (2)
cover Prio handling and v4l2_device release callback
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(-)
RFC PATCH 1/10 v4l2_prio: move from v4l2-common to v4l2-dev.
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.
--- 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(-)
drivers/media/video/v4l2-common.c +0 -63
@@ -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
drivers/media/video/v4l2-dev.c +64 -0
70 lines not on this page · raw message
include/media/v4l2-common.h +0 -15
@@ -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,
include/media/v4l2-dev.h +15 -0
@@ -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 *);
Signature · 1 line
1.6.4.2
RFC PATCH 2/10 v4l2: add v4l2_prio_state to v4l2_device and video_device
In reply to Hans VerkuilIntegrate 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.
--- 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(-)
drivers/media/video/v4l2-dev.c +6 -0
@@ -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. */
drivers/media/video/v4l2-device.c +1 -0
@@ -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 */
include/media/v4l2-dev.h +3 -0
@@ -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;
include/media/v4l2-device.h +3 -0
@@ -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; };
Signature · 1 line
1.6.4.2
RFC PATCH 3/10 v4l2-fh: implement v4l2_priority support.
In reply to Hans Verkuil--- drivers/media/video/v4l2-fh.c | 4 ++++ include/media/v4l2-fh.h | 1 + 2 files changed, 5 insertions(+), 0 deletions(-)
drivers/media/video/v4l2-fh.c +4 -0
@@ -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);
include/media/v4l2-fh.h +1 -0
@@ -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; };/*
Signature · 1 line
1.6.4.2
RFC PATCH 4/10 v4l2-dev: add and support flag V4L2_FH_USE_PRIO.
In reply to Hans VerkuilFlag 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.
--- drivers/media/video/v4l2-dev.c | 23 +++++++++++++++++++++-- include/media/v4l2-dev.h | 6 ++++++ 2 files changed, 27 insertions(+), 2 deletions(-)
drivers/media/video/v4l2-dev.c +21 -2
@@ -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 ) {
include/media/v4l2-dev.h +6 -0
@@ -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 */
Signature · 1 line
1.6.4.2
RFC PATCH 5/10 v4l2-ioctl: add priority handling support.
In reply to Hans VerkuilDrivers that use either v4l2_fh or don't use filehandles at all can now use the core framework support of g/s_priority.
--- 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(-)
drivers/media/radio/radio-si4713.c +2 -1
@@ -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);
drivers/media/video/cx18/cx18-ioctl.c +2 -1
@@ -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;
drivers/media/video/davinci/vpfe_capture.c +1 -1
@@ -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 ;
drivers/media/video/ivtv/ivtv-ioctl.c +2 -1
@@ -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;
drivers/media/video/meye.c +2 -1
@@ -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 :
drivers/media/video/mxb.c +2 -1
@@ -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;
drivers/media/video/v4l2-ioctl.c +66 -7
116 lines not on this page · raw message
include/media/v4l2-ioctl.h +1 -1
@@ -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); };
Signature · 1 line
1.6.4.2
RFC PATCH 6/10 ivtv: convert to core priority handling.
In reply to Hans Verkuil--- 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(-)
drivers/media/video/ivtv/ivtv-driver.h +0 -2
@@ -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 */
drivers/media/video/ivtv/ivtv-fileops.c +0 -2
@@ -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;
drivers/media/video/ivtv/ivtv-ioctl.c +15 -41
82 lines not on this page · raw message
Signature · 1 line
1.6.4.2
RFC PATCH 7/10 ivtv: use core-assisted locking.
In reply to Hans Verkuil--- 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(-)
drivers/media/video/ivtv/ivtv-fileops.c +1 -14
@@ -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 cset_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)
drivers/media/video/ivtv/ivtv-ioctl.c +1 -17
@@ -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,
drivers/media/video/ivtv/ivtv-streams.c +1 -0
@@ -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 ; }
Signature · 1 line
1.6.4.2
RFC PATCH 8/10 v4l2-framework: update documentation for new prio field
In reply to Hans Verkuil--- Documentation/video4linux/v4l2-framework.txt | 19 +++++++++++++++++-- 1 files changed, 17 insertions(+), 2 deletions(-)
Documentation/video4linux/v4l2-framework.txt +17 -2
@@ -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.
Signature · 1 line
1.6.4.2
RFC PATCH 9/10 v4l2-device: add kref and a release function
In reply to Hans VerkuilThe 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.
--- 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(-)
drivers/media/video/v4l2-dev.c +8 -0
@@ -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);
drivers/media/video/v4l2-device.c +15 -0
@@ -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) {
include/media/v4l2-device.h +11 -0
@@ -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. */
Signature · 1 line
1.6.4.2
RFC PATCH 10/10 v4l2-framework.txt: document new v4l2_device release() callback
In reply to Hans Verkuil--- Documentation/video4linux/v4l2-framework.txt | 15 +++++++++++++++ 1 files changed, 15 insertions(+), 0 deletions(-)
Documentation/video4linux/v4l2-framework.txt +15 -0
@@ -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 ------------------
Signature · 1 line
1.6.4.2
Em 29-12-2010 19:43, Hans Verkuil escreveu:
Quoted 7 lines
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.
Quoted 1 line
Eventually all drivers should either use
v4l2_fhor never setfilp->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.
Quoted 73 lines
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 invideo_deviceto 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.ccode will check the ioctls whether or not they are allowed based on the current priority. Thevidioc_defaultcallback has a new argument that just tells the driver whether the prio is OK or not. The driver can use this argument to return-EBUSYfor 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_devicewhich 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 thevideo_devicerelease()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: addv4l2_prio_statetov4l2_deviceandvideo_device
v4l2-fh: implementv4l2_prioritysupport.
v4l2-dev: add and support flagV4L2_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 newv4l2_devicerelease()callbackDocumentation/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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 11 lines · On Friday, December 31, 2010 12:01:17 Mauro Carvalho Chehab wrote:
Em 29-12-2010 19:43, Hans Verkuil escreveu:
Quoted 7 lines
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.
Quoted 4 lines
Quoted 1 line
Eventually all drivers should either use
v4l2_fhor never setfilp->private_data.I made a series of patches, due to BKL stuff converting the core to always use
v4l2_fhon 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
Signature · 1 line
Hans Verkuil - video4linux developer - sponsored by Cisco
Em 31-12-2010 09:25, Hans Verkuil escreveu:
Quoted 10 lines · On Friday, December 31, 2010 12:01:17 Mauro Carvalho Chehab wrote:
Quoted 9 lines
Em 29-12-2010 19:43, Hans Verkuil escreveu:
Quoted 7 lines
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.
Quoted 4 lines
Quoted 2 lines
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.
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
Eventually all drivers should either use
v4l2_fhor never setfilp->private_data.I made a series of patches, due to BKL stuff converting the core to always use
v4l2_fhon 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.
Quoted 4 lines
I see two potential problems with this approach:
1) A lot of drivers do not actually need to allocate a
v4l2_fhstruct, 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.
Quoted 2 lines
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.
Quoted 8 lines
But I would have to see your patch series again to see the impact of such a change.
Regards,
Hans
Quoted 23 lines · On Friday, December 31, 2010 12:39:09 Mauro Carvalho Chehab wrote:
Em 31-12-2010 09:25, Hans Verkuil escreveu:
Quoted 10 lines · On Friday, December 31, 2010 12:01:17 Mauro Carvalho Chehab wrote:
Quoted 9 lines
Em 29-12-2010 19:43, Hans Verkuil escreveu:
Quoted 7 lines
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.Quoted 4 lines
Quoted 2 lines
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.
Quoted 6 lines
Quoted 5 lines
Quoted 2 lines
Eventually all drivers should either use
v4l2_fhor never setfilp->private_data.I made a series of patches, due to BKL stuff converting the core to always use
v4l2_fhon all drivers. This seems to be the right solution for it.
Quoted 7 lines
Quoted 1 line
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.
Quoted 10 lines
Quoted 4 lines
I see two potential problems with this approach:
1) A lot of drivers do not actually need to allocate a
v4l2_fhstruct, 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_fhstruct 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
Quoted 21 lines
Quoted 3 lines
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.
Quoted 8 lines
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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 1 line
Hans Verkuil - video4linux developer - sponsored by Cisco