From 95faf82bd3ea607f8e1ca78930c49b71078266e6 Mon Sep 17 00:00:00 2001 From: Bhupesh Sharma Date: Thu, 22 Mar 2012 00:50:37 -0300 Subject: [media] usb: gadget/uvc: Remove non-required locking from 'uvc_queue_next_buffer' routine This patch removes the non-required spinlock acquire/release calls on 'queue_irqlock' from 'uvc_queue_next_buffer' routine. This routine is called from 'video->encode' function (which translates to either 'uvc_video_encode_bulk' or 'uvc_video_encode_isoc') in 'uvc_video.c'. As, the 'video->encode' routines are called with 'queue_irqlock' already held, so acquiring a 'queue_irqlock' again in 'uvc_queue_next_buffer' routine causes a spin lock recursion. Signed-off-by: Bhupesh Sharma Acked-by: Laurent Pinchart Signed-off-by: Mauro Carvalho Chehab --- drivers/usb/gadget/uvc_queue.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'drivers/usb/gadget') diff --git a/drivers/usb/gadget/uvc_queue.c b/drivers/usb/gadget/uvc_queue.c index d776adb2da67..104ae9c81251 100644 --- a/drivers/usb/gadget/uvc_queue.c +++ b/drivers/usb/gadget/uvc_queue.c @@ -543,11 +543,11 @@ done: return ret; } +/* called with &queue_irqlock held.. */ static struct uvc_buffer * uvc_queue_next_buffer(struct uvc_video_queue *queue, struct uvc_buffer *buf) { struct uvc_buffer *nextbuf; - unsigned long flags; if ((queue->flags & UVC_QUEUE_DROP_INCOMPLETE) && buf->buf.length != buf->buf.bytesused) { @@ -556,14 +556,12 @@ uvc_queue_next_buffer(struct uvc_video_queue *queue, struct uvc_buffer *buf) return buf; } - spin_lock_irqsave(&queue->irqlock, flags); list_del(&buf->queue); if (!list_empty(&queue->irqqueue)) nextbuf = list_first_entry(&queue->irqqueue, struct uvc_buffer, queue); else nextbuf = NULL; - spin_unlock_irqrestore(&queue->irqlock, flags); buf->buf.sequence = queue->sequence++; do_gettimeofday(&buf->buf.timestamp); -- cgit v1.2.3 From c53c2549333b340e2662dc64ec81323476b69a97 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sun, 8 Apr 2012 12:59:46 -0300 Subject: [media] v4l2-event: Add v4l2_subscribed_event_ops Just like with ctrl events, drivers may want to get called back on listener add / remove for other event types too. Rather then special casing all of this in subscribe / unsubscribe event it is better to use ops for this. Signed-off-by: Hans de Goede Acked-by: Hans Verkuil Signed-off-by: Laurent Pinchart Signed-off-by: Mauro Carvalho Chehab --- Documentation/video4linux/v4l2-framework.txt | 28 +++++++++++---- drivers/media/video/ivtv/ivtv-ioctl.c | 2 +- drivers/media/video/omap3isp/ispccdc.c | 2 +- drivers/media/video/omap3isp/ispstat.c | 2 +- drivers/media/video/v4l2-ctrls.c | 2 +- drivers/media/video/v4l2-event.c | 54 ++++++++++++++++++++++------ drivers/usb/gadget/uvc_v4l2.c | 2 +- include/media/v4l2-event.h | 24 +++++++++---- 8 files changed, 86 insertions(+), 30 deletions(-) (limited to 'drivers/usb/gadget') diff --git a/Documentation/video4linux/v4l2-framework.txt b/Documentation/video4linux/v4l2-framework.txt index e3dfc268d9c1..369d4bc87828 100644 --- a/Documentation/video4linux/v4l2-framework.txt +++ b/Documentation/video4linux/v4l2-framework.txt @@ -945,21 +945,35 @@ fast. Useful functions: -- v4l2_event_queue() +void v4l2_event_queue(struct video_device *vdev, const struct v4l2_event *ev) Queue events to video device. The driver's only responsibility is to fill in the type and the data fields. The other fields will be filled in by V4L2. -- v4l2_event_subscribe() +int v4l2_event_subscribe(struct v4l2_fh *fh, + struct v4l2_event_subscription *sub, unsigned elems, + const struct v4l2_subscribed_event_ops *ops) The video_device->ioctl_ops->vidioc_subscribe_event must check the driver is able to produce events with specified event id. Then it calls - v4l2_event_subscribe() to subscribe the event. The last argument is the - size of the event queue for this event. If it is 0, then the framework - will fill in a default value (this depends on the event type). + v4l2_event_subscribe() to subscribe the event. -- v4l2_event_unsubscribe() + The elems argument is the size of the event queue for this event. If it is 0, + then the framework will fill in a default value (this depends on the event + type). + + The ops argument allows the driver to specify a number of callbacks: + * add: called when a new listener gets added (subscribing to the same + event twice will only cause this callback to get called once) + * del: called when a listener stops listening + * replace: replace event 'old' with event 'new'. + * merge: merge event 'old' into event 'new'. + All 4 callbacks are optional, if you don't want to specify any callbacks + the ops argument itself maybe NULL. + +int v4l2_event_unsubscribe(struct v4l2_fh *fh, + struct v4l2_event_subscription *sub) vidioc_unsubscribe_event in struct v4l2_ioctl_ops. A driver may use v4l2_event_unsubscribe() directly unless it wants to be involved in @@ -968,7 +982,7 @@ Useful functions: The special type V4L2_EVENT_ALL may be used to unsubscribe all events. The drivers may want to handle this in a special way. -- v4l2_event_pending() +int v4l2_event_pending(struct v4l2_fh *fh) Returns the number of pending events. Useful when implementing poll. diff --git a/drivers/media/video/ivtv/ivtv-ioctl.c b/drivers/media/video/ivtv/ivtv-ioctl.c index a151271f60e1..a7730fd4827f 100644 --- a/drivers/media/video/ivtv/ivtv-ioctl.c +++ b/drivers/media/video/ivtv/ivtv-ioctl.c @@ -1469,7 +1469,7 @@ static int ivtv_subscribe_event(struct v4l2_fh *fh, struct v4l2_event_subscripti case V4L2_EVENT_VSYNC: case V4L2_EVENT_EOS: case V4L2_EVENT_CTRL: - return v4l2_event_subscribe(fh, sub, 0); + return v4l2_event_subscribe(fh, sub, 0, NULL); default: return -EINVAL; } diff --git a/drivers/media/video/omap3isp/ispccdc.c b/drivers/media/video/omap3isp/ispccdc.c index eaabc27f0fa2..1f3c16d8f0b4 100644 --- a/drivers/media/video/omap3isp/ispccdc.c +++ b/drivers/media/video/omap3isp/ispccdc.c @@ -1703,7 +1703,7 @@ static int ccdc_subscribe_event(struct v4l2_subdev *sd, struct v4l2_fh *fh, if (sub->id != 0) return -EINVAL; - return v4l2_event_subscribe(fh, sub, OMAP3ISP_CCDC_NEVENTS); + return v4l2_event_subscribe(fh, sub, OMAP3ISP_CCDC_NEVENTS, NULL); } static int ccdc_unsubscribe_event(struct v4l2_subdev *sd, struct v4l2_fh *fh, diff --git a/drivers/media/video/omap3isp/ispstat.c b/drivers/media/video/omap3isp/ispstat.c index 11871ecc6d25..b8640be692f1 100644 --- a/drivers/media/video/omap3isp/ispstat.c +++ b/drivers/media/video/omap3isp/ispstat.c @@ -1032,7 +1032,7 @@ int omap3isp_stat_subscribe_event(struct v4l2_subdev *subdev, if (sub->type != stat->event_type) return -EINVAL; - return v4l2_event_subscribe(fh, sub, STAT_NEVENTS); + return v4l2_event_subscribe(fh, sub, STAT_NEVENTS, NULL); } int omap3isp_stat_unsubscribe_event(struct v4l2_subdev *subdev, diff --git a/drivers/media/video/v4l2-ctrls.c b/drivers/media/video/v4l2-ctrls.c index c93a9796f1fb..91b197819fc2 100644 --- a/drivers/media/video/v4l2-ctrls.c +++ b/drivers/media/video/v4l2-ctrls.c @@ -2468,7 +2468,7 @@ int v4l2_ctrl_subscribe_event(struct v4l2_fh *fh, struct v4l2_event_subscription *sub) { if (sub->type == V4L2_EVENT_CTRL) - return v4l2_event_subscribe(fh, sub, 0); + return v4l2_event_subscribe(fh, sub, 0, NULL); return -EINVAL; } EXPORT_SYMBOL(v4l2_ctrl_subscribe_event); diff --git a/drivers/media/video/v4l2-event.c b/drivers/media/video/v4l2-event.c index c26ad9637143..0ba2dfa86d07 100644 --- a/drivers/media/video/v4l2-event.c +++ b/drivers/media/video/v4l2-event.c @@ -120,6 +120,14 @@ static void __v4l2_event_queue_fh(struct v4l2_fh *fh, const struct v4l2_event *e if (sev == NULL) return; + /* + * If the event has been added to the fh->subscribed list, but its + * add op has not completed yet elems will be 0, treat this as + * not being subscribed. + */ + if (!sev->elems) + return; + /* Increase event sequence number on fh. */ fh->sequence++; @@ -132,14 +140,14 @@ static void __v4l2_event_queue_fh(struct v4l2_fh *fh, const struct v4l2_event *e sev->first = sev_pos(sev, 1); fh->navailable--; if (sev->elems == 1) { - if (sev->replace) { - sev->replace(&kev->event, ev); + if (sev->ops && sev->ops->replace) { + sev->ops->replace(&kev->event, ev); copy_payload = false; } - } else if (sev->merge) { + } else if (sev->ops && sev->ops->merge) { struct v4l2_kevent *second_oldest = sev->events + sev_pos(sev, 0); - sev->merge(&kev->event, &second_oldest->event); + sev->ops->merge(&kev->event, &second_oldest->event); } } @@ -208,8 +216,14 @@ static void ctrls_merge(const struct v4l2_event *old, struct v4l2_event *new) new->u.ctrl.changes |= old->u.ctrl.changes; } +static const struct v4l2_subscribed_event_ops ctrl_ops = { + .replace = ctrls_replace, + .merge = ctrls_merge, +}; + int v4l2_event_subscribe(struct v4l2_fh *fh, - struct v4l2_event_subscription *sub, unsigned elems) + struct v4l2_event_subscription *sub, unsigned elems, + const struct v4l2_subscribed_event_ops *ops) { struct v4l2_subscribed_event *sev, *found_ev; struct v4l2_ctrl *ctrl = NULL; @@ -236,10 +250,9 @@ int v4l2_event_subscribe(struct v4l2_fh *fh, sev->id = sub->id; sev->flags = sub->flags; sev->fh = fh; - sev->elems = elems; + sev->ops = ops; if (ctrl) { - sev->replace = ctrls_replace; - sev->merge = ctrls_merge; + sev->ops = &ctrl_ops; } spin_lock_irqsave(&fh->vdev->fh_lock, flags); @@ -248,12 +261,27 @@ int v4l2_event_subscribe(struct v4l2_fh *fh, list_add(&sev->list, &fh->subscribed); spin_unlock_irqrestore(&fh->vdev->fh_lock, flags); - /* v4l2_ctrl_add_event uses a mutex, so do this outside the spin lock */ - if (found_ev) + if (found_ev) { kfree(sev); - else if (ctrl) + return 0; /* Already listening */ + } + + if (sev->ops && sev->ops->add) { + int ret = sev->ops->add(sev); + if (ret) { + sev->ops = NULL; + v4l2_event_unsubscribe(fh, sub); + return ret; + } + } + + /* v4l2_ctrl_add_event uses a mutex, so do this outside the spin lock */ + if (ctrl) v4l2_ctrl_add_event(ctrl, sev); + /* Mark as ready for use */ + sev->elems = elems; + return 0; } EXPORT_SYMBOL_GPL(v4l2_event_subscribe); @@ -306,6 +334,10 @@ int v4l2_event_unsubscribe(struct v4l2_fh *fh, } spin_unlock_irqrestore(&fh->vdev->fh_lock, flags); + + if (sev && sev->ops && sev->ops->del) + sev->ops->del(sev); + if (sev && sev->type == V4L2_EVENT_CTRL) { struct v4l2_ctrl *ctrl = v4l2_ctrl_find(fh->ctrl_handler, sev->id); diff --git a/drivers/usb/gadget/uvc_v4l2.c b/drivers/usb/gadget/uvc_v4l2.c index f6e083b50191..90db5fe9c56e 100644 --- a/drivers/usb/gadget/uvc_v4l2.c +++ b/drivers/usb/gadget/uvc_v4l2.c @@ -296,7 +296,7 @@ uvc_v4l2_do_ioctl(struct file *file, unsigned int cmd, void *arg) if (sub->type < UVC_EVENT_FIRST || sub->type > UVC_EVENT_LAST) return -EINVAL; - return v4l2_event_subscribe(&handle->vfh, arg, 2); + return v4l2_event_subscribe(&handle->vfh, arg, 2, NULL); } case VIDIOC_UNSUBSCRIBE_EVENT: diff --git a/include/media/v4l2-event.h b/include/media/v4l2-event.h index 5f14e8895ce2..88fa9a1e0df3 100644 --- a/include/media/v4l2-event.h +++ b/include/media/v4l2-event.h @@ -78,6 +78,19 @@ struct v4l2_kevent { struct v4l2_event event; }; +/** struct v4l2_subscribed_event_ops - Subscribed event operations. + * @add: Optional callback, called when a new listener is added + * @del: Optional callback, called when a listener stops listening + * @replace: Optional callback that can replace event 'old' with event 'new'. + * @merge: Optional callback that can merge event 'old' into event 'new'. + */ +struct v4l2_subscribed_event_ops { + int (*add)(struct v4l2_subscribed_event *sev); + void (*del)(struct v4l2_subscribed_event *sev); + void (*replace)(struct v4l2_event *old, const struct v4l2_event *new); + void (*merge)(const struct v4l2_event *old, struct v4l2_event *new); +}; + /** struct v4l2_subscribed_event - Internal struct representing a subscribed event. * @list: List node for the v4l2_fh->subscribed list. * @type: Event type. @@ -85,8 +98,7 @@ struct v4l2_kevent { * @flags: Copy of v4l2_event_subscription->flags. * @fh: Filehandle that subscribed to this event. * @node: List node that hooks into the object's event list (if there is one). - * @replace: Optional callback that can replace event 'old' with event 'new'. - * @merge: Optional callback that can merge event 'old' into event 'new'. + * @ops: v4l2_subscribed_event_ops * @elems: The number of elements in the events array. * @first: The index of the events containing the oldest available event. * @in_use: The number of queued events. @@ -99,10 +111,7 @@ struct v4l2_subscribed_event { u32 flags; struct v4l2_fh *fh; struct list_head node; - void (*replace)(struct v4l2_event *old, - const struct v4l2_event *new); - void (*merge)(const struct v4l2_event *old, - struct v4l2_event *new); + const struct v4l2_subscribed_event_ops *ops; unsigned elems; unsigned first; unsigned in_use; @@ -115,7 +124,8 @@ void v4l2_event_queue(struct video_device *vdev, const struct v4l2_event *ev); void v4l2_event_queue_fh(struct v4l2_fh *fh, const struct v4l2_event *ev); int v4l2_event_pending(struct v4l2_fh *fh); int v4l2_event_subscribe(struct v4l2_fh *fh, - struct v4l2_event_subscription *sub, unsigned elems); + struct v4l2_event_subscription *sub, unsigned elems, + const struct v4l2_subscribed_event_ops *ops); int v4l2_event_unsubscribe(struct v4l2_fh *fh, struct v4l2_event_subscription *sub); void v4l2_event_unsubscribe_all(struct v4l2_fh *fh); -- cgit v1.2.3