linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* uvc & pwc: Add support for control events (v2)
@ 2012-03-25 11:56 Hans de Goede
  2012-03-25 11:56 ` [PATCH 01/10] pwc: Add support for control events Hans de Goede
                   ` (9 more replies)
  0 siblings, 10 replies; 20+ messages in thread
From: Hans de Goede @ 2012-03-25 11:56 UTC (permalink / raw)
  To: Linux Media Mailing List; +Cc: Laurent Pinchart

Hi All,

This patch series adds supports for control events to the uvc and pwc
drivers.

Note:
-This series depends on Hans Verkuil's poll work, the latest version of
-which
 can be found here:
 http://git.linuxtv.org/hgoede/gspca.git/shortlog/refs/heads/poll_req_events
-This series has been posted before, this version is rebased on top of
 the latest media_tree.git/staging/for_v3.4 and has some remarks from 
 earlier reviews addressed

Changes in v2:
- Make the last param of __uvc_ctrl_get a s32 rather then an xctrl, as only
  the value part of the xctrl was used
- Make uvc_ctrl_send_event static as it is only used in uvc_ctrls.c
- Move some functions around in uvc_ctrls.c to avoid the need for forward
  declarations for static / private helper functions
- Refactor uvc_ctrl_send_event into uvc_ctrl_send_event and
  uvc_ctrl_send_events in preparation for the next patches
- Properly report the INACTIVE flag for inactive controls
- Send ctrl change events when the inactive flag changes due to the master
  ctrl changing

Regards,

Hans

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

* [PATCH 01/10] pwc: Add support for control events
  2012-03-25 11:56 uvc & pwc: Add support for control events (v2) Hans de Goede
@ 2012-03-25 11:56 ` Hans de Goede
  2012-03-25 11:56 ` [PATCH 02/10] media/radio: use v4l2_ctrl_subscribe_event where possible Hans de Goede
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Hans de Goede @ 2012-03-25 11:56 UTC (permalink / raw)
  To: Linux Media Mailing List; +Cc: Laurent Pinchart, Hans de Goede

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/media/video/pwc/pwc-v4l.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/video/pwc/pwc-v4l.c b/drivers/media/video/pwc/pwc-v4l.c
index 2834e3e..c1ba1a0 100644
--- a/drivers/media/video/pwc/pwc-v4l.c
+++ b/drivers/media/video/pwc/pwc-v4l.c
@@ -1166,4 +1166,6 @@ const struct v4l2_ioctl_ops pwc_ioctl_ops = {
 	.vidioc_enum_frameintervals	    = pwc_enum_frameintervals,
 	.vidioc_g_parm			    = pwc_g_parm,
 	.vidioc_s_parm			    = pwc_s_parm,
+	.vidioc_subscribe_event		    = v4l2_ctrl_subscribe_event,
+	.vidioc_unsubscribe_event	    = v4l2_event_unsubscribe,
 };
-- 
1.7.9.3


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

* [PATCH 02/10] media/radio: use v4l2_ctrl_subscribe_event where possible
  2012-03-25 11:56 uvc & pwc: Add support for control events (v2) Hans de Goede
  2012-03-25 11:56 ` [PATCH 01/10] pwc: Add support for control events Hans de Goede
@ 2012-03-25 11:56 ` Hans de Goede
  2012-03-25 11:56 ` [PATCH 03/10] v4l2-event: Add v4l2_subscribed_event_ops Hans de Goede
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Hans de Goede @ 2012-03-25 11:56 UTC (permalink / raw)
  To: Linux Media Mailing List; +Cc: Laurent Pinchart, Hans de Goede

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/media/radio/radio-isa.c   |   10 +---------
 drivers/media/radio/radio-keene.c |   14 +-------------
 2 files changed, 2 insertions(+), 22 deletions(-)

diff --git a/drivers/media/radio/radio-isa.c b/drivers/media/radio/radio-isa.c
index 06f9063..1346046 100644
--- a/drivers/media/radio/radio-isa.c
+++ b/drivers/media/radio/radio-isa.c
@@ -150,14 +150,6 @@ static int radio_isa_log_status(struct file *file, void *priv)
 	return 0;
 }
 
-static int radio_isa_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 -EINVAL;
-}
-
 static const struct v4l2_ctrl_ops radio_isa_ctrl_ops = {
 	.s_ctrl = radio_isa_s_ctrl,
 };
@@ -177,7 +169,7 @@ static const struct v4l2_ioctl_ops radio_isa_ioctl_ops = {
 	.vidioc_g_frequency = radio_isa_g_frequency,
 	.vidioc_s_frequency = radio_isa_s_frequency,
 	.vidioc_log_status  = radio_isa_log_status,
-	.vidioc_subscribe_event   = radio_isa_subscribe_event,
+	.vidioc_subscribe_event   = v4l2_ctrl_subscribe_event,
 	.vidioc_unsubscribe_event = v4l2_event_unsubscribe,
 };
 
diff --git a/drivers/media/radio/radio-keene.c b/drivers/media/radio/radio-keene.c
index 55bd1d2..e1c72b0 100644
--- a/drivers/media/radio/radio-keene.c
+++ b/drivers/media/radio/radio-keene.c
@@ -256,18 +256,6 @@ static int keene_s_ctrl(struct v4l2_ctrl *ctrl)
 	return -EINVAL;
 }
 
-static int vidioc_subscribe_event(struct v4l2_fh *fh,
-				struct v4l2_event_subscription *sub)
-{
-	switch (sub->type) {
-	case V4L2_EVENT_CTRL:
-		return v4l2_event_subscribe(fh, sub, 0);
-	default:
-		return -EINVAL;
-	}
-}
-
-
 /* File system interface */
 static const struct v4l2_file_operations usb_keene_fops = {
 	.owner		= THIS_MODULE,
@@ -288,7 +276,7 @@ static const struct v4l2_ioctl_ops usb_keene_ioctl_ops = {
 	.vidioc_g_frequency = vidioc_g_frequency,
 	.vidioc_s_frequency = vidioc_s_frequency,
 	.vidioc_log_status = v4l2_ctrl_log_status,
-	.vidioc_subscribe_event = vidioc_subscribe_event,
+	.vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
 	.vidioc_unsubscribe_event = v4l2_event_unsubscribe,
 };
 
-- 
1.7.9.3


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

* [PATCH 03/10] v4l2-event: Add v4l2_subscribed_event_ops
  2012-03-25 11:56 uvc & pwc: Add support for control events (v2) Hans de Goede
  2012-03-25 11:56 ` [PATCH 01/10] pwc: Add support for control events Hans de Goede
  2012-03-25 11:56 ` [PATCH 02/10] media/radio: use v4l2_ctrl_subscribe_event where possible Hans de Goede
@ 2012-03-25 11:56 ` Hans de Goede
  2012-03-25 11:56 ` [PATCH 04/10] v4l2-ctrls: Use v4l2_subscribed_event_ops Hans de Goede
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Hans de Goede @ 2012-03-25 11:56 UTC (permalink / raw)
  To: Linux Media Mailing List; +Cc: Laurent Pinchart, Hans de Goede

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 <hdegoede@redhat.com>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 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(-)

diff --git a/Documentation/video4linux/v4l2-framework.txt b/Documentation/video4linux/v4l2-framework.txt
index 659b2ba..b77bb48 100644
--- a/Documentation/video4linux/v4l2-framework.txt
+++ b/Documentation/video4linux/v4l2-framework.txt
@@ -941,21 +941,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
@@ -964,7 +978,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 5452bee..5b41e5b 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 eaabc27..1f3c16d 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 11871ec..b8640be 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 18015c0..7023e6d 100644
--- a/drivers/media/video/v4l2-ctrls.c
+++ b/drivers/media/video/v4l2-ctrls.c
@@ -2425,7 +2425,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 c26ad96..0ba2dfa 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 f6e083b..90db5fe 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 5f14e88..88fa9a1 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);
-- 
1.7.9.3


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

* [PATCH 04/10] v4l2-ctrls: Use v4l2_subscribed_event_ops
  2012-03-25 11:56 uvc & pwc: Add support for control events (v2) Hans de Goede
                   ` (2 preceding siblings ...)
  2012-03-25 11:56 ` [PATCH 03/10] v4l2-event: Add v4l2_subscribed_event_ops Hans de Goede
@ 2012-03-25 11:56 ` Hans de Goede
  2012-03-25 11:56 ` [PATCH 05/10] uvcvideo: Fix a "ignoring return value of ‘__clear_user’" warning Hans de Goede
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Hans de Goede @ 2012-03-25 11:56 UTC (permalink / raw)
  To: Linux Media Mailing List; +Cc: Laurent Pinchart, Hans de Goede

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/video/ivtv/ivtv-ioctl.c |    3 +-
 drivers/media/video/v4l2-ctrls.c      |   58 ++++++++++++++++++++++++++-------
 drivers/media/video/v4l2-event.c      |   39 ----------------------
 include/media/v4l2-ctrls.h            |    7 ++--
 4 files changed, 52 insertions(+), 55 deletions(-)

diff --git a/drivers/media/video/ivtv/ivtv-ioctl.c b/drivers/media/video/ivtv/ivtv-ioctl.c
index 5b41e5b..0c9cf0d 100644
--- a/drivers/media/video/ivtv/ivtv-ioctl.c
+++ b/drivers/media/video/ivtv/ivtv-ioctl.c
@@ -1468,8 +1468,9 @@ static int ivtv_subscribe_event(struct v4l2_fh *fh, struct v4l2_event_subscripti
 	switch (sub->type) {
 	case V4L2_EVENT_VSYNC:
 	case V4L2_EVENT_EOS:
-	case V4L2_EVENT_CTRL:
 		return v4l2_event_subscribe(fh, sub, 0, NULL);
+	case V4L2_EVENT_CTRL:
+		return v4l2_event_subscribe(fh, sub, 0, &v4l2_ctrl_sub_ev_ops);
 	default:
 		return -EINVAL;
 	}
diff --git a/drivers/media/video/v4l2-ctrls.c b/drivers/media/video/v4l2-ctrls.c
index 7023e6d..2a44355 100644
--- a/drivers/media/video/v4l2-ctrls.c
+++ b/drivers/media/video/v4l2-ctrls.c
@@ -2381,10 +2381,22 @@ int v4l2_ctrl_s_ctrl(struct v4l2_ctrl *ctrl, s32 val)
 }
 EXPORT_SYMBOL(v4l2_ctrl_s_ctrl);
 
-void v4l2_ctrl_add_event(struct v4l2_ctrl *ctrl,
-				struct v4l2_subscribed_event *sev)
+static int v4l2_ctrl_add_event(struct v4l2_subscribed_event *sev)
 {
-	v4l2_ctrl_lock(ctrl);
+	struct v4l2_ctrl_handler *hdl = sev->fh->ctrl_handler;
+	struct v4l2_ctrl_ref *ref;
+	struct v4l2_ctrl *ctrl;
+	int ret = 0;
+
+	mutex_lock(&hdl->lock);
+
+	ref = find_ref(hdl, sev->id);
+	if (!ref) {
+		ret = -EINVAL;
+		goto leave;
+	}
+	ctrl = ref->ctrl;
+
 	list_add_tail(&sev->node, &ctrl->ev_subs);
 	if (ctrl->type != V4L2_CTRL_TYPE_CTRL_CLASS &&
 	    (sev->flags & V4L2_EVENT_SUB_FL_SEND_INITIAL)) {
@@ -2396,18 +2408,42 @@ void v4l2_ctrl_add_event(struct v4l2_ctrl *ctrl,
 		fill_event(&ev, ctrl, changes);
 		v4l2_event_queue_fh(sev->fh, &ev);
 	}
-	v4l2_ctrl_unlock(ctrl);
+leave:
+	mutex_unlock(&hdl->lock);
+	return ret;
 }
-EXPORT_SYMBOL(v4l2_ctrl_add_event);
 
-void v4l2_ctrl_del_event(struct v4l2_ctrl *ctrl,
-				struct v4l2_subscribed_event *sev)
+static void v4l2_ctrl_del_event(struct v4l2_subscribed_event *sev)
 {
-	v4l2_ctrl_lock(ctrl);
+	struct v4l2_ctrl_handler *hdl = sev->fh->ctrl_handler;
+
+	mutex_lock(&hdl->lock);
 	list_del(&sev->node);
-	v4l2_ctrl_unlock(ctrl);
+	mutex_unlock(&hdl->lock);
 }
-EXPORT_SYMBOL(v4l2_ctrl_del_event);
+
+void v4l2_ctrl_replace(struct v4l2_event *old, const struct v4l2_event *new)
+{
+	u32 old_changes = old->u.ctrl.changes;
+
+	old->u.ctrl = new->u.ctrl;
+	old->u.ctrl.changes |= old_changes;
+}
+EXPORT_SYMBOL(v4l2_ctrl_replace);
+
+void v4l2_ctrl_merge(const struct v4l2_event *old, struct v4l2_event *new)
+{
+	new->u.ctrl.changes |= old->u.ctrl.changes;
+}
+EXPORT_SYMBOL(v4l2_ctrl_merge);
+
+const struct v4l2_subscribed_event_ops v4l2_ctrl_sub_ev_ops = {
+	.add = v4l2_ctrl_add_event,
+	.del = v4l2_ctrl_del_event,
+	.replace = v4l2_ctrl_replace,
+	.merge = v4l2_ctrl_merge,
+};
+EXPORT_SYMBOL(v4l2_ctrl_sub_ev_ops);
 
 int v4l2_ctrl_log_status(struct file *file, void *fh)
 {
@@ -2425,7 +2461,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, NULL);
+		return v4l2_event_subscribe(fh, sub, 0, &v4l2_ctrl_sub_ev_ops);
 	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 0ba2dfa..60b4e2e 100644
--- a/drivers/media/video/v4l2-event.c
+++ b/drivers/media/video/v4l2-event.c
@@ -25,7 +25,6 @@
 #include <media/v4l2-dev.h>
 #include <media/v4l2-fh.h>
 #include <media/v4l2-event.h>
-#include <media/v4l2-ctrls.h>
 
 #include <linux/sched.h>
 #include <linux/slab.h>
@@ -203,30 +202,11 @@ int v4l2_event_pending(struct v4l2_fh *fh)
 }
 EXPORT_SYMBOL_GPL(v4l2_event_pending);
 
-static void ctrls_replace(struct v4l2_event *old, const struct v4l2_event *new)
-{
-	u32 old_changes = old->u.ctrl.changes;
-
-	old->u.ctrl = new->u.ctrl;
-	old->u.ctrl.changes |= old_changes;
-}
-
-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,
 			 const struct v4l2_subscribed_event_ops *ops)
 {
 	struct v4l2_subscribed_event *sev, *found_ev;
-	struct v4l2_ctrl *ctrl = NULL;
 	unsigned long flags;
 	unsigned i;
 
@@ -235,11 +215,6 @@ int v4l2_event_subscribe(struct v4l2_fh *fh,
 
 	if (elems < 1)
 		elems = 1;
-	if (sub->type == V4L2_EVENT_CTRL) {
-		ctrl = v4l2_ctrl_find(fh->ctrl_handler, sub->id);
-		if (ctrl == NULL)
-			return -EINVAL;
-	}
 
 	sev = kzalloc(sizeof(*sev) + sizeof(struct v4l2_kevent) * elems, GFP_KERNEL);
 	if (!sev)
@@ -251,9 +226,6 @@ int v4l2_event_subscribe(struct v4l2_fh *fh,
 	sev->flags = sub->flags;
 	sev->fh = fh;
 	sev->ops = ops;
-	if (ctrl) {
-		sev->ops = &ctrl_ops;
-	}
 
 	spin_lock_irqsave(&fh->vdev->fh_lock, flags);
 	found_ev = v4l2_event_subscribed(fh, sub->type, sub->id);
@@ -275,10 +247,6 @@ int v4l2_event_subscribe(struct v4l2_fh *fh,
 		}
 	}
 
-	/* 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;
 
@@ -338,13 +306,6 @@ int v4l2_event_unsubscribe(struct v4l2_fh *fh,
 	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);
-
-		if (ctrl)
-			v4l2_ctrl_del_event(ctrl, sev);
-	}
-
 	kfree(sev);
 
 	return 0;
diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
index 3dbd066..e5194f9 100644
--- a/include/media/v4l2-ctrls.h
+++ b/include/media/v4l2-ctrls.h
@@ -488,10 +488,9 @@ s32 v4l2_ctrl_g_ctrl(struct v4l2_ctrl *ctrl);
 int v4l2_ctrl_s_ctrl(struct v4l2_ctrl *ctrl, s32 val);
 
 /* Internal helper functions that deal with control events. */
-void v4l2_ctrl_add_event(struct v4l2_ctrl *ctrl,
-		struct v4l2_subscribed_event *sev);
-void v4l2_ctrl_del_event(struct v4l2_ctrl *ctrl,
-		struct v4l2_subscribed_event *sev);
+extern const struct v4l2_subscribed_event_ops v4l2_ctrl_sub_ev_ops;
+void v4l2_ctrl_replace(struct v4l2_event *old, const struct v4l2_event *new);
+void v4l2_ctrl_merge(const struct v4l2_event *old, struct v4l2_event *new);
 
 /* Can be used as a vidioc_log_status function that just dumps all controls
    associated with the filehandle. */
-- 
1.7.9.3


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

* [PATCH 05/10] uvcvideo: Fix a "ignoring return value of ‘__clear_user’" warning
  2012-03-25 11:56 uvc & pwc: Add support for control events (v2) Hans de Goede
                   ` (3 preceding siblings ...)
  2012-03-25 11:56 ` [PATCH 04/10] v4l2-ctrls: Use v4l2_subscribed_event_ops Hans de Goede
@ 2012-03-25 11:56 ` Hans de Goede
  2012-03-27 15:48   ` Laurent Pinchart
  2012-03-25 11:56 ` [PATCH 06/10] uvcvideo: Refactor uvc_ctrl_get and query Hans de Goede
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: Hans de Goede @ 2012-03-25 11:56 UTC (permalink / raw)
  To: Linux Media Mailing List; +Cc: Laurent Pinchart, Hans de Goede

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/media/video/uvc/uvc_v4l2.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/video/uvc/uvc_v4l2.c b/drivers/media/video/uvc/uvc_v4l2.c
index ff2cddd..8db90ef 100644
--- a/drivers/media/video/uvc/uvc_v4l2.c
+++ b/drivers/media/video/uvc/uvc_v4l2.c
@@ -1097,7 +1097,8 @@ static int uvc_v4l2_put_xu_mapping(const struct uvc_xu_control_mapping *kp,
 	    __put_user(kp->menu_count, &up->menu_count))
 		return -EFAULT;
 
-	__clear_user(up->reserved, sizeof(up->reserved));
+	if (__clear_user(up->reserved, sizeof(up->reserved)))
+		return -EFAULT;
 
 	if (kp->menu_count == 0)
 		return 0;
-- 
1.7.9.3


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

* [PATCH 06/10] uvcvideo: Refactor uvc_ctrl_get and query
  2012-03-25 11:56 uvc & pwc: Add support for control events (v2) Hans de Goede
                   ` (4 preceding siblings ...)
  2012-03-25 11:56 ` [PATCH 05/10] uvcvideo: Fix a "ignoring return value of ‘__clear_user’" warning Hans de Goede
@ 2012-03-25 11:56 ` Hans de Goede
  2012-03-28  9:26   ` Laurent Pinchart
  2012-03-25 11:56 ` [PATCH 07/10] uvcvideo: Move __uvc_ctrl_get() up Hans de Goede
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: Hans de Goede @ 2012-03-25 11:56 UTC (permalink / raw)
  To: Linux Media Mailing List; +Cc: Laurent Pinchart, Hans de Goede

This is a preparation patch for adding ctrl event support.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/media/video/uvc/uvc_ctrl.c |   69 ++++++++++++++++++++++++------------
 1 file changed, 46 insertions(+), 23 deletions(-)

diff --git a/drivers/media/video/uvc/uvc_ctrl.c b/drivers/media/video/uvc/uvc_ctrl.c
index 0efd3b1..4002b5b 100644
--- a/drivers/media/video/uvc/uvc_ctrl.c
+++ b/drivers/media/video/uvc/uvc_ctrl.c
@@ -899,24 +899,14 @@ static int uvc_ctrl_populate_cache(struct uvc_video_chain *chain,
 	return 0;
 }
 
-int uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
+static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
+	struct uvc_control *ctrl,
+	struct uvc_control_mapping *mapping,
 	struct v4l2_queryctrl *v4l2_ctrl)
 {
-	struct uvc_control *ctrl;
-	struct uvc_control_mapping *mapping;
 	struct uvc_menu_info *menu;
 	unsigned int i;
-	int ret;
-
-	ret = mutex_lock_interruptible(&chain->ctrl_mutex);
-	if (ret < 0)
-		return -ERESTARTSYS;
-
-	ctrl = uvc_find_control(chain, v4l2_ctrl->id, &mapping);
-	if (ctrl == NULL) {
-		ret = -EINVAL;
-		goto done;
-	}
+	int ret = 0;
 
 	memset(v4l2_ctrl, 0, sizeof *v4l2_ctrl);
 	v4l2_ctrl->id = mapping->id;
@@ -985,6 +975,28 @@ int uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
 				  uvc_ctrl_data(ctrl, UVC_CTRL_DATA_RES));
 
 done:
+	return ret;
+}
+
+int uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
+	struct v4l2_queryctrl *v4l2_ctrl)
+{
+	struct uvc_control *ctrl;
+	struct uvc_control_mapping *mapping;
+	int ret;
+
+	ret = mutex_lock_interruptible(&chain->ctrl_mutex);
+	if (ret < 0)
+		return -ERESTARTSYS;
+
+	ctrl = uvc_find_control(chain, v4l2_ctrl->id, &mapping);
+	if (ctrl == NULL) {
+		ret = -EINVAL;
+		goto done;
+	}
+
+	ret = __uvc_query_v4l2_ctrl(chain, ctrl, mapping, v4l2_ctrl);
+done:
 	mutex_unlock(&chain->ctrl_mutex);
 	return ret;
 }
@@ -1148,17 +1160,15 @@ done:
 	return ret;
 }
 
-int uvc_ctrl_get(struct uvc_video_chain *chain,
-	struct v4l2_ext_control *xctrl)
+static int __uvc_ctrl_get(struct uvc_video_chain *chain,
+	struct uvc_control *ctrl, struct uvc_control_mapping *mapping,
+	s32 *value)
 {
-	struct uvc_control *ctrl;
-	struct uvc_control_mapping *mapping;
 	struct uvc_menu_info *menu;
 	unsigned int i;
 	int ret;
 
-	ctrl = uvc_find_control(chain, xctrl->id, &mapping);
-	if (ctrl == NULL || (ctrl->info.flags & UVC_CTRL_FLAG_GET_CUR) == 0)
+	if ((ctrl->info.flags & UVC_CTRL_FLAG_GET_CUR) == 0)
 		return -EINVAL;
 
 	if (!ctrl->loaded) {
@@ -1172,14 +1182,14 @@ int uvc_ctrl_get(struct uvc_video_chain *chain,
 		ctrl->loaded = 1;
 	}
 
-	xctrl->value = mapping->get(mapping, UVC_GET_CUR,
+	*value = mapping->get(mapping, UVC_GET_CUR,
 		uvc_ctrl_data(ctrl, UVC_CTRL_DATA_CURRENT));
 
 	if (mapping->v4l2_type == V4L2_CTRL_TYPE_MENU) {
 		menu = mapping->menu_info;
 		for (i = 0; i < mapping->menu_count; ++i, ++menu) {
-			if (menu->value == xctrl->value) {
-				xctrl->value = i;
+			if (menu->value == *value) {
+				*value = i;
 				break;
 			}
 		}
@@ -1188,6 +1198,19 @@ int uvc_ctrl_get(struct uvc_video_chain *chain,
 	return 0;
 }
 
+int uvc_ctrl_get(struct uvc_video_chain *chain,
+	struct v4l2_ext_control *xctrl)
+{
+	struct uvc_control *ctrl;
+	struct uvc_control_mapping *mapping;
+
+	ctrl = uvc_find_control(chain, xctrl->id, &mapping);
+	if (ctrl == NULL)
+		return -EINVAL;
+
+	return __uvc_ctrl_get(chain, ctrl, mapping, &xctrl->value);
+}
+
 int uvc_ctrl_set(struct uvc_video_chain *chain,
 	struct v4l2_ext_control *xctrl)
 {
-- 
1.7.9.3


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

* [PATCH 07/10] uvcvideo: Move __uvc_ctrl_get() up
  2012-03-25 11:56 uvc & pwc: Add support for control events (v2) Hans de Goede
                   ` (5 preceding siblings ...)
  2012-03-25 11:56 ` [PATCH 06/10] uvcvideo: Refactor uvc_ctrl_get and query Hans de Goede
@ 2012-03-25 11:56 ` Hans de Goede
  2012-03-28  8:59   ` Laurent Pinchart
  2012-03-25 11:56 ` [PATCH 08/10] uvcvideo: Add support for control events Hans de Goede
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: Hans de Goede @ 2012-03-25 11:56 UTC (permalink / raw)
  To: Linux Media Mailing List; +Cc: Laurent Pinchart, Hans de Goede

This avoids the need for doing a forward declaration of __uvc_ctrl_get
(which is a static function) in later patches in this series.

Note to reviewers this patch does not change a single line of code, it
just moves the function up in uvc_ctrl.c a bit.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/media/video/uvc/uvc_ctrl.c |   76 ++++++++++++++++++------------------
 1 file changed, 38 insertions(+), 38 deletions(-)

diff --git a/drivers/media/video/uvc/uvc_ctrl.c b/drivers/media/video/uvc/uvc_ctrl.c
index 4002b5b..fc979c6 100644
--- a/drivers/media/video/uvc/uvc_ctrl.c
+++ b/drivers/media/video/uvc/uvc_ctrl.c
@@ -899,6 +899,44 @@ static int uvc_ctrl_populate_cache(struct uvc_video_chain *chain,
 	return 0;
 }
 
+static int __uvc_ctrl_get(struct uvc_video_chain *chain,
+	struct uvc_control *ctrl, struct uvc_control_mapping *mapping,
+	s32 *value)
+{
+	struct uvc_menu_info *menu;
+	unsigned int i;
+	int ret;
+
+	if ((ctrl->info.flags & UVC_CTRL_FLAG_GET_CUR) == 0)
+		return -EINVAL;
+
+	if (!ctrl->loaded) {
+		ret = uvc_query_ctrl(chain->dev, UVC_GET_CUR, ctrl->entity->id,
+				chain->dev->intfnum, ctrl->info.selector,
+				uvc_ctrl_data(ctrl, UVC_CTRL_DATA_CURRENT),
+				ctrl->info.size);
+		if (ret < 0)
+			return ret;
+
+		ctrl->loaded = 1;
+	}
+
+	*value = mapping->get(mapping, UVC_GET_CUR,
+		uvc_ctrl_data(ctrl, UVC_CTRL_DATA_CURRENT));
+
+	if (mapping->v4l2_type == V4L2_CTRL_TYPE_MENU) {
+		menu = mapping->menu_info;
+		for (i = 0; i < mapping->menu_count; ++i, ++menu) {
+			if (menu->value == *value) {
+				*value = i;
+				break;
+			}
+		}
+	}
+
+	return 0;
+}
+
 static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
 	struct uvc_control *ctrl,
 	struct uvc_control_mapping *mapping,
@@ -1160,44 +1198,6 @@ done:
 	return ret;
 }
 
-static int __uvc_ctrl_get(struct uvc_video_chain *chain,
-	struct uvc_control *ctrl, struct uvc_control_mapping *mapping,
-	s32 *value)
-{
-	struct uvc_menu_info *menu;
-	unsigned int i;
-	int ret;
-
-	if ((ctrl->info.flags & UVC_CTRL_FLAG_GET_CUR) == 0)
-		return -EINVAL;
-
-	if (!ctrl->loaded) {
-		ret = uvc_query_ctrl(chain->dev, UVC_GET_CUR, ctrl->entity->id,
-				chain->dev->intfnum, ctrl->info.selector,
-				uvc_ctrl_data(ctrl, UVC_CTRL_DATA_CURRENT),
-				ctrl->info.size);
-		if (ret < 0)
-			return ret;
-
-		ctrl->loaded = 1;
-	}
-
-	*value = mapping->get(mapping, UVC_GET_CUR,
-		uvc_ctrl_data(ctrl, UVC_CTRL_DATA_CURRENT));
-
-	if (mapping->v4l2_type == V4L2_CTRL_TYPE_MENU) {
-		menu = mapping->menu_info;
-		for (i = 0; i < mapping->menu_count; ++i, ++menu) {
-			if (menu->value == *value) {
-				*value = i;
-				break;
-			}
-		}
-	}
-
-	return 0;
-}
-
 int uvc_ctrl_get(struct uvc_video_chain *chain,
 	struct v4l2_ext_control *xctrl)
 {
-- 
1.7.9.3


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

* [PATCH 08/10] uvcvideo: Add support for control events
  2012-03-25 11:56 uvc & pwc: Add support for control events (v2) Hans de Goede
                   ` (6 preceding siblings ...)
  2012-03-25 11:56 ` [PATCH 07/10] uvcvideo: Move __uvc_ctrl_get() up Hans de Goede
@ 2012-03-25 11:56 ` Hans de Goede
  2012-03-28  9:25   ` Laurent Pinchart
  2012-03-25 11:56 ` [PATCH 09/10] uvcvideo: Properly report the inactive flag for inactive controls Hans de Goede
  2012-03-25 11:56 ` [PATCH 10/10] uvcvideo: Send control change events for slave ctrls when the master changes Hans de Goede
  9 siblings, 1 reply; 20+ messages in thread
From: Hans de Goede @ 2012-03-25 11:56 UTC (permalink / raw)
  To: Linux Media Mailing List; +Cc: Laurent Pinchart, Hans de Goede

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/media/video/uvc/uvc_ctrl.c |  119 +++++++++++++++++++++++++++++++++++-
 drivers/media/video/uvc/uvc_v4l2.c |   42 ++++++++++---
 drivers/media/video/uvc/uvcvideo.h |   18 ++++--
 3 files changed, 165 insertions(+), 14 deletions(-)

diff --git a/drivers/media/video/uvc/uvc_ctrl.c b/drivers/media/video/uvc/uvc_ctrl.c
index fc979c6..742496f 100644
--- a/drivers/media/video/uvc/uvc_ctrl.c
+++ b/drivers/media/video/uvc/uvc_ctrl.c
@@ -21,6 +21,7 @@
 #include <linux/vmalloc.h>
 #include <linux/wait.h>
 #include <linux/atomic.h>
+#include <media/v4l2-ctrls.h>
 
 #include "uvcvideo.h"
 
@@ -1104,6 +1105,116 @@ done:
 	return ret;
 }
 
+/* --------------------------------------------------------------------------
+ * Ctrl event handling
+ */
+
+static void uvc_ctrl_fill_event(struct uvc_video_chain *chain,
+	struct v4l2_event *ev,
+	struct uvc_control *ctrl,
+	struct uvc_control_mapping *mapping,
+	s32 value, u32 changes)
+{
+	struct v4l2_queryctrl v4l2_ctrl;
+
+	__uvc_query_v4l2_ctrl(chain, ctrl, mapping, &v4l2_ctrl);
+
+	memset(ev->reserved, 0, sizeof(ev->reserved));
+	ev->type = V4L2_EVENT_CTRL;
+	ev->id = v4l2_ctrl.id;
+	ev->u.ctrl.value = value;
+	ev->u.ctrl.changes = changes;
+	ev->u.ctrl.type = v4l2_ctrl.type;
+	ev->u.ctrl.flags = v4l2_ctrl.flags;
+	ev->u.ctrl.minimum = v4l2_ctrl.minimum;
+	ev->u.ctrl.maximum = v4l2_ctrl.maximum;
+	ev->u.ctrl.step = v4l2_ctrl.step;
+	ev->u.ctrl.default_value = v4l2_ctrl.default_value;
+}
+
+static void uvc_ctrl_send_event(struct uvc_fh *handle,
+	struct uvc_control *ctrl, struct uvc_control_mapping *mapping,
+	s32 value, u32 changes)
+{
+	struct v4l2_subscribed_event *sev;
+	struct v4l2_event ev;
+
+	if (list_empty(&mapping->ev_subs))
+		return;
+
+	uvc_ctrl_fill_event(handle->chain, &ev, ctrl, mapping, value, changes);
+
+	list_for_each_entry(sev, &mapping->ev_subs, node)
+		if (sev->fh && (sev->fh != &handle->vfh ||
+		    (sev->flags & V4L2_EVENT_SUB_FL_ALLOW_FEEDBACK)))
+			v4l2_event_queue_fh(sev->fh, &ev);
+}
+
+static void uvc_ctrl_send_events(struct uvc_fh *handle,
+	struct v4l2_ext_control *xctrls, int xctrls_count)
+{
+	struct uvc_control_mapping *mapping;
+	struct uvc_control *ctrl;
+	int i;
+
+	for (i = 0; i < xctrls_count; ++i) {
+		ctrl = uvc_find_control(handle->chain, xctrls[i].id, &mapping);
+		uvc_ctrl_send_event(handle, ctrl, mapping, xctrls[i].value,
+				    V4L2_EVENT_CTRL_CH_VALUE);
+	}
+}
+
+static int uvc_ctrl_add_event(struct v4l2_subscribed_event *sev)
+{
+	struct uvc_fh *handle = container_of(sev->fh, struct uvc_fh, vfh);
+	struct uvc_control_mapping *mapping;
+	struct uvc_control *ctrl;
+	int ret;
+
+	ret = mutex_lock_interruptible(&handle->chain->ctrl_mutex);
+	if (ret < 0)
+		return -ERESTARTSYS;
+
+	ctrl = uvc_find_control(handle->chain, sev->id, &mapping);
+	if (ctrl == NULL) {
+		ret = -EINVAL;
+		goto done;
+	}
+
+	list_add_tail(&sev->node, &mapping->ev_subs);
+	if (sev->flags & V4L2_EVENT_SUB_FL_SEND_INITIAL) {
+		struct v4l2_event ev;
+		u32 changes = V4L2_EVENT_CTRL_CH_FLAGS;
+		s32 val = 0;
+
+		if (__uvc_ctrl_get(handle->chain, ctrl, mapping, &val) == 0)
+			changes |= V4L2_EVENT_CTRL_CH_VALUE;
+
+		uvc_ctrl_fill_event(handle->chain, &ev, ctrl, mapping, val,
+				    changes);
+		v4l2_event_queue_fh(sev->fh, &ev);
+	}
+
+done:
+	mutex_unlock(&handle->chain->ctrl_mutex);
+	return ret;
+}
+
+static void uvc_ctrl_del_event(struct v4l2_subscribed_event *sev)
+{
+	struct uvc_fh *handle = container_of(sev->fh, struct uvc_fh, vfh);
+
+	mutex_lock(&handle->chain->ctrl_mutex);
+	list_del(&sev->node);
+	mutex_unlock(&handle->chain->ctrl_mutex);
+}
+
+const struct v4l2_subscribed_event_ops uvc_ctrl_sub_ev_ops = {
+	.add = uvc_ctrl_add_event,
+	.del = uvc_ctrl_del_event,
+	.replace = v4l2_ctrl_replace,
+	.merge = v4l2_ctrl_merge,
+};
 
 /* --------------------------------------------------------------------------
  * Control transactions
@@ -1181,8 +1292,10 @@ static int uvc_ctrl_commit_entity(struct uvc_device *dev,
 	return 0;
 }
 
-int __uvc_ctrl_commit(struct uvc_video_chain *chain, int rollback)
+int __uvc_ctrl_commit(struct uvc_fh *handle, int rollback,
+		      struct v4l2_ext_control *xctrls, int xctrls_count)
 {
+	struct uvc_video_chain *chain = handle->chain;
 	struct uvc_entity *entity;
 	int ret = 0;
 
@@ -1193,6 +1306,8 @@ int __uvc_ctrl_commit(struct uvc_video_chain *chain, int rollback)
 			goto done;
 	}
 
+	if (!rollback)
+		uvc_ctrl_send_events(handle, xctrls, xctrls_count);
 done:
 	mutex_unlock(&chain->ctrl_mutex);
 	return ret;
@@ -1664,6 +1779,8 @@ static int __uvc_ctrl_add_mapping(struct uvc_device *dev,
 	if (map == NULL)
 		return -ENOMEM;
 
+	INIT_LIST_HEAD(&map->ev_subs);
+
 	size = sizeof(*mapping->menu_info) * mapping->menu_count;
 	map->menu_info = kmemdup(mapping->menu_info, size, GFP_KERNEL);
 	if (map->menu_info == NULL) {
diff --git a/drivers/media/video/uvc/uvc_v4l2.c b/drivers/media/video/uvc/uvc_v4l2.c
index 8db90ef..f7cf6f5 100644
--- a/drivers/media/video/uvc/uvc_v4l2.c
+++ b/drivers/media/video/uvc/uvc_v4l2.c
@@ -25,6 +25,7 @@
 #include <linux/atomic.h>
 
 #include <media/v4l2-common.h>
+#include <media/v4l2-event.h>
 #include <media/v4l2-ioctl.h>
 
 #include "uvcvideo.h"
@@ -505,6 +506,8 @@ static int uvc_v4l2_open(struct file *file)
 		}
 	}
 
+	v4l2_fh_init(&handle->vfh, stream->vdev);
+	v4l2_fh_add(&handle->vfh);
 	handle->chain = stream->chain;
 	handle->stream = stream;
 	handle->state = UVC_HANDLE_PASSIVE;
@@ -528,6 +531,8 @@ static int uvc_v4l2_release(struct file *file)
 
 	/* Release the file handle. */
 	uvc_dismiss_privileges(handle);
+	v4l2_fh_del(&handle->vfh);
+	v4l2_fh_exit(&handle->vfh);
 	kfree(handle);
 	file->private_data = NULL;
 
@@ -584,7 +589,7 @@ static long uvc_v4l2_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 			return ret;
 
 		ret = uvc_ctrl_get(chain, &xctrl);
-		uvc_ctrl_rollback(chain);
+		uvc_ctrl_rollback(handle);
 		if (ret >= 0)
 			ctrl->value = xctrl.value;
 		break;
@@ -605,10 +610,10 @@ static long uvc_v4l2_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 
 		ret = uvc_ctrl_set(chain, &xctrl);
 		if (ret < 0) {
-			uvc_ctrl_rollback(chain);
+			uvc_ctrl_rollback(handle);
 			return ret;
 		}
-		ret = uvc_ctrl_commit(chain);
+		ret = uvc_ctrl_commit(handle, &xctrl, 1);
 		if (ret == 0)
 			ctrl->value = xctrl.value;
 		break;
@@ -630,13 +635,13 @@ static long uvc_v4l2_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 		for (i = 0; i < ctrls->count; ++ctrl, ++i) {
 			ret = uvc_ctrl_get(chain, ctrl);
 			if (ret < 0) {
-				uvc_ctrl_rollback(chain);
+				uvc_ctrl_rollback(handle);
 				ctrls->error_idx = i;
 				return ret;
 			}
 		}
 		ctrls->error_idx = 0;
-		ret = uvc_ctrl_rollback(chain);
+		ret = uvc_ctrl_rollback(handle);
 		break;
 	}
 
@@ -654,7 +659,7 @@ static long uvc_v4l2_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 		for (i = 0; i < ctrls->count; ++ctrl, ++i) {
 			ret = uvc_ctrl_set(chain, ctrl);
 			if (ret < 0) {
-				uvc_ctrl_rollback(chain);
+				uvc_ctrl_rollback(handle);
 				ctrls->error_idx = i;
 				return ret;
 			}
@@ -663,9 +668,10 @@ static long uvc_v4l2_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 		ctrls->error_idx = 0;
 
 		if (cmd == VIDIOC_S_EXT_CTRLS)
-			ret = uvc_ctrl_commit(chain);
+			ret = uvc_ctrl_commit(handle,
+					      ctrls->controls, ctrls->count);
 		else
-			ret = uvc_ctrl_rollback(chain);
+			ret = uvc_ctrl_rollback(handle);
 		break;
 	}
 
@@ -990,6 +996,26 @@ static long uvc_v4l2_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 		return uvc_video_enable(stream, 0);
 	}
 
+	case VIDIOC_SUBSCRIBE_EVENT:
+	{
+		struct v4l2_event_subscription *sub = arg;
+
+		switch (sub->type) {
+		case V4L2_EVENT_CTRL:
+			return v4l2_event_subscribe(&handle->vfh, sub, 0,
+						    &uvc_ctrl_sub_ev_ops);
+		default:
+			return -EINVAL;
+		}
+	}
+
+	case VIDIOC_UNSUBSCRIBE_EVENT:
+		return v4l2_event_unsubscribe(&handle->vfh, arg);
+
+	case VIDIOC_DQEVENT:
+		return v4l2_event_dequeue(&handle->vfh, arg,
+					  file->f_flags & O_NONBLOCK);
+
 	/* Analog video standards make no sense for digital cameras. */
 	case VIDIOC_ENUMSTD:
 	case VIDIOC_QUERYSTD:
diff --git a/drivers/media/video/uvc/uvcvideo.h b/drivers/media/video/uvc/uvcvideo.h
index 67f88d8..649a0bb 100644
--- a/drivers/media/video/uvc/uvcvideo.h
+++ b/drivers/media/video/uvc/uvcvideo.h
@@ -13,6 +13,8 @@
 #include <linux/videodev2.h>
 #include <media/media-device.h>
 #include <media/v4l2-device.h>
+#include <media/v4l2-event.h>
+#include <media/v4l2-fh.h>
 #include <media/videobuf2-core.h>
 
 /* --------------------------------------------------------------------------
@@ -153,6 +155,7 @@ struct uvc_control_info {
 
 struct uvc_control_mapping {
 	struct list_head list;
+	struct list_head ev_subs;
 
 	struct uvc_control_info *ctrl;
 
@@ -524,6 +527,7 @@ enum uvc_handle_state {
 };
 
 struct uvc_fh {
+	struct v4l2_fh vfh;
 	struct uvc_video_chain *chain;
 	struct uvc_streaming *stream;
 	enum uvc_handle_state state;
@@ -643,6 +647,8 @@ extern int uvc_status_suspend(struct uvc_device *dev);
 extern int uvc_status_resume(struct uvc_device *dev);
 
 /* Controls */
+extern const struct v4l2_subscribed_event_ops uvc_ctrl_sub_ev_ops;
+
 extern int uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
 		struct v4l2_queryctrl *v4l2_ctrl);
 extern int uvc_query_v4l2_menu(struct uvc_video_chain *chain,
@@ -655,14 +661,16 @@ extern void uvc_ctrl_cleanup_device(struct uvc_device *dev);
 extern int uvc_ctrl_resume_device(struct uvc_device *dev);
 
 extern int uvc_ctrl_begin(struct uvc_video_chain *chain);
-extern int __uvc_ctrl_commit(struct uvc_video_chain *chain, int rollback);
-static inline int uvc_ctrl_commit(struct uvc_video_chain *chain)
+extern int __uvc_ctrl_commit(struct uvc_fh *handle, int rollback,
+			struct v4l2_ext_control *xctrls, int xctrls_count);
+static inline int uvc_ctrl_commit(struct uvc_fh *handle,
+			struct v4l2_ext_control *xctrls, int xctrls_count)
 {
-	return __uvc_ctrl_commit(chain, 0);
+	return __uvc_ctrl_commit(handle, 0, xctrls, xctrls_count);
 }
-static inline int uvc_ctrl_rollback(struct uvc_video_chain *chain)
+static inline int uvc_ctrl_rollback(struct uvc_fh *handle)
 {
-	return __uvc_ctrl_commit(chain, 1);
+	return __uvc_ctrl_commit(handle, 1, NULL, 0);
 }
 
 extern int uvc_ctrl_get(struct uvc_video_chain *chain,
-- 
1.7.9.3


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

* [PATCH 09/10] uvcvideo: Properly report the inactive flag for inactive controls
  2012-03-25 11:56 uvc & pwc: Add support for control events (v2) Hans de Goede
                   ` (7 preceding siblings ...)
  2012-03-25 11:56 ` [PATCH 08/10] uvcvideo: Add support for control events Hans de Goede
@ 2012-03-25 11:56 ` Hans de Goede
  2012-03-28  9:12   ` Laurent Pinchart
  2012-03-25 11:56 ` [PATCH 10/10] uvcvideo: Send control change events for slave ctrls when the master changes Hans de Goede
  9 siblings, 1 reply; 20+ messages in thread
From: Hans de Goede @ 2012-03-25 11:56 UTC (permalink / raw)
  To: Linux Media Mailing List; +Cc: Laurent Pinchart, Hans de Goede

Note the unused in this patch slave_ids addition to the mappings will get
used in a follow up patch to generate control change events for the slave
ctrls when their flags change due to the master control changing value.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/media/video/uvc/uvc_ctrl.c |   33 +++++++++++++++++++++++++++++++++
 drivers/media/video/uvc/uvcvideo.h |    4 ++++
 2 files changed, 37 insertions(+)

diff --git a/drivers/media/video/uvc/uvc_ctrl.c b/drivers/media/video/uvc/uvc_ctrl.c
index 742496f..91d9007 100644
--- a/drivers/media/video/uvc/uvc_ctrl.c
+++ b/drivers/media/video/uvc/uvc_ctrl.c
@@ -421,6 +421,8 @@ static struct uvc_control_mapping uvc_ctrl_mappings[] = {
 		.offset		= 0,
 		.v4l2_type	= V4L2_CTRL_TYPE_INTEGER,
 		.data_type	= UVC_CTRL_DATA_TYPE_SIGNED,
+		.master_id	= V4L2_CID_HUE_AUTO,
+		.master_manual	= 0,
 	},
 	{
 		.id		= V4L2_CID_SATURATION,
@@ -493,6 +495,7 @@ static struct uvc_control_mapping uvc_ctrl_mappings[] = {
 		.offset		= 0,
 		.v4l2_type	= V4L2_CTRL_TYPE_BOOLEAN,
 		.data_type	= UVC_CTRL_DATA_TYPE_BOOLEAN,
+		.slave_ids	= { V4L2_CID_HUE, },
 	},
 	{
 		.id		= V4L2_CID_EXPOSURE_AUTO,
@@ -505,6 +508,7 @@ static struct uvc_control_mapping uvc_ctrl_mappings[] = {
 		.data_type	= UVC_CTRL_DATA_TYPE_BITMASK,
 		.menu_info	= exposure_auto_controls,
 		.menu_count	= ARRAY_SIZE(exposure_auto_controls),
+		.slave_ids	= { V4L2_CID_EXPOSURE_ABSOLUTE, },
 	},
 	{
 		.id		= V4L2_CID_EXPOSURE_AUTO_PRIORITY,
@@ -525,6 +529,8 @@ static struct uvc_control_mapping uvc_ctrl_mappings[] = {
 		.offset		= 0,
 		.v4l2_type	= V4L2_CTRL_TYPE_INTEGER,
 		.data_type	= UVC_CTRL_DATA_TYPE_UNSIGNED,
+		.master_id	= V4L2_CID_EXPOSURE_AUTO,
+		.master_manual	= V4L2_EXPOSURE_MANUAL,
 	},
 	{
 		.id		= V4L2_CID_AUTO_WHITE_BALANCE,
@@ -535,6 +541,7 @@ static struct uvc_control_mapping uvc_ctrl_mappings[] = {
 		.offset		= 0,
 		.v4l2_type	= V4L2_CTRL_TYPE_BOOLEAN,
 		.data_type	= UVC_CTRL_DATA_TYPE_BOOLEAN,
+		.slave_ids	= { V4L2_CID_WHITE_BALANCE_TEMPERATURE, },
 	},
 	{
 		.id		= V4L2_CID_WHITE_BALANCE_TEMPERATURE,
@@ -545,6 +552,8 @@ static struct uvc_control_mapping uvc_ctrl_mappings[] = {
 		.offset		= 0,
 		.v4l2_type	= V4L2_CTRL_TYPE_INTEGER,
 		.data_type	= UVC_CTRL_DATA_TYPE_UNSIGNED,
+		.master_id	= V4L2_CID_AUTO_WHITE_BALANCE,
+		.master_manual	= 0,
 	},
 	{
 		.id		= V4L2_CID_AUTO_WHITE_BALANCE,
@@ -555,6 +564,8 @@ static struct uvc_control_mapping uvc_ctrl_mappings[] = {
 		.offset		= 0,
 		.v4l2_type	= V4L2_CTRL_TYPE_BOOLEAN,
 		.data_type	= UVC_CTRL_DATA_TYPE_BOOLEAN,
+		.slave_ids	= { V4L2_CID_BLUE_BALANCE,
+				    V4L2_CID_RED_BALANCE },
 	},
 	{
 		.id		= V4L2_CID_BLUE_BALANCE,
@@ -565,6 +576,8 @@ static struct uvc_control_mapping uvc_ctrl_mappings[] = {
 		.offset		= 0,
 		.v4l2_type	= V4L2_CTRL_TYPE_INTEGER,
 		.data_type	= UVC_CTRL_DATA_TYPE_SIGNED,
+		.master_id	= V4L2_CID_AUTO_WHITE_BALANCE,
+		.master_manual	= 0,
 	},
 	{
 		.id		= V4L2_CID_RED_BALANCE,
@@ -575,6 +588,8 @@ static struct uvc_control_mapping uvc_ctrl_mappings[] = {
 		.offset		= 16,
 		.v4l2_type	= V4L2_CTRL_TYPE_INTEGER,
 		.data_type	= UVC_CTRL_DATA_TYPE_SIGNED,
+		.master_id	= V4L2_CID_AUTO_WHITE_BALANCE,
+		.master_manual	= 0,
 	},
 	{
 		.id		= V4L2_CID_FOCUS_ABSOLUTE,
@@ -585,6 +600,8 @@ static struct uvc_control_mapping uvc_ctrl_mappings[] = {
 		.offset		= 0,
 		.v4l2_type	= V4L2_CTRL_TYPE_INTEGER,
 		.data_type	= UVC_CTRL_DATA_TYPE_UNSIGNED,
+		.master_id	= V4L2_CID_FOCUS_AUTO,
+		.master_manual	= 0,
 	},
 	{
 		.id		= V4L2_CID_FOCUS_AUTO,
@@ -595,6 +612,7 @@ static struct uvc_control_mapping uvc_ctrl_mappings[] = {
 		.offset		= 0,
 		.v4l2_type	= V4L2_CTRL_TYPE_BOOLEAN,
 		.data_type	= UVC_CTRL_DATA_TYPE_BOOLEAN,
+		.slave_ids	= { V4L2_CID_FOCUS_ABSOLUTE, },
 	},
 	{
 		.id		= V4L2_CID_IRIS_ABSOLUTE,
@@ -943,6 +961,8 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
 	struct uvc_control_mapping *mapping,
 	struct v4l2_queryctrl *v4l2_ctrl)
 {
+	struct uvc_control_mapping *master_map;
+	struct uvc_control *master_ctrl = NULL;
 	struct uvc_menu_info *menu;
 	unsigned int i;
 	int ret = 0;
@@ -958,6 +978,19 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
 	if (!(ctrl->info.flags & UVC_CTRL_FLAG_SET_CUR))
 		v4l2_ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY;
 
+	if (mapping->master_id)
+		master_ctrl = uvc_find_control(chain, mapping->master_id,
+					       &master_map);
+	if (master_ctrl && (master_ctrl->info.flags & UVC_CTRL_FLAG_GET_CUR)) {
+		s32 val;
+		ret = __uvc_ctrl_get(chain, master_ctrl, master_map, &val);
+		if (ret < 0)
+			goto done;
+
+		if (val != mapping->master_manual)
+				v4l2_ctrl->flags |= V4L2_CTRL_FLAG_INACTIVE;
+	}
+
 	if (!ctrl->cached) {
 		ret = uvc_ctrl_populate_cache(chain, ctrl);
 		if (ret < 0)
diff --git a/drivers/media/video/uvc/uvcvideo.h b/drivers/media/video/uvc/uvcvideo.h
index 649a0bb..79a83e0 100644
--- a/drivers/media/video/uvc/uvcvideo.h
+++ b/drivers/media/video/uvc/uvcvideo.h
@@ -172,6 +172,10 @@ struct uvc_control_mapping {
 	struct uvc_menu_info *menu_info;
 	__u32 menu_count;
 
+	__u32 master_id;
+	__s32 master_manual;
+	__u32 slave_ids[2];
+
 	__s32 (*get) (struct uvc_control_mapping *mapping, __u8 query,
 		      const __u8 *data);
 	void (*set) (struct uvc_control_mapping *mapping, __s32 value,
-- 
1.7.9.3


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

* [PATCH 10/10] uvcvideo: Send control change events for slave ctrls when the master changes
  2012-03-25 11:56 uvc & pwc: Add support for control events (v2) Hans de Goede
                   ` (8 preceding siblings ...)
  2012-03-25 11:56 ` [PATCH 09/10] uvcvideo: Properly report the inactive flag for inactive controls Hans de Goede
@ 2012-03-25 11:56 ` Hans de Goede
  2012-03-28  9:34   ` Laurent Pinchart
  9 siblings, 1 reply; 20+ messages in thread
From: Hans de Goede @ 2012-03-25 11:56 UTC (permalink / raw)
  To: Linux Media Mailing List; +Cc: Laurent Pinchart, Hans de Goede

This allows v4l2 control UI-s to update the inactive state (ie grey-ing
out of controls) for slave controls when the master control changes.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/media/video/uvc/uvc_ctrl.c |   55 ++++++++++++++++++++++++++++++++++--
 1 file changed, 52 insertions(+), 3 deletions(-)

diff --git a/drivers/media/video/uvc/uvc_ctrl.c b/drivers/media/video/uvc/uvc_ctrl.c
index 91d9007..2d06fd8 100644
--- a/drivers/media/video/uvc/uvc_ctrl.c
+++ b/drivers/media/video/uvc/uvc_ctrl.c
@@ -1179,21 +1179,70 @@ static void uvc_ctrl_send_event(struct uvc_fh *handle,
 
 	list_for_each_entry(sev, &mapping->ev_subs, node)
 		if (sev->fh && (sev->fh != &handle->vfh ||
-		    (sev->flags & V4L2_EVENT_SUB_FL_ALLOW_FEEDBACK)))
+		    (sev->flags & V4L2_EVENT_SUB_FL_ALLOW_FEEDBACK) ||
+		    (changes & V4L2_EVENT_CTRL_CH_FLAGS)))
 			v4l2_event_queue_fh(sev->fh, &ev);
 }
 
-static void uvc_ctrl_send_events(struct uvc_fh *handle,
+static void uvc_ctrl_send_slave_event(struct uvc_fh *handle, u32 slave_id,
 	struct v4l2_ext_control *xctrls, int xctrls_count)
 {
 	struct uvc_control_mapping *mapping;
 	struct uvc_control *ctrl;
+	u32 changes = V4L2_EVENT_CTRL_CH_FLAGS;
+	s32 val = 0;
 	int i;
 
+	/*
+	 * We can skip sending an event for the slave if the slave
+	 * is being modified in the same transaction.
+	 */
+	for (i = 0; i < xctrls_count; i++)
+		if (xctrls[i].id == slave_id)
+			return;
+
+	ctrl = uvc_find_control(handle->chain, slave_id, &mapping);
+	if (ctrl == NULL)
+		return;
+
+	if (__uvc_ctrl_get(handle->chain, ctrl, mapping, &val) == 0)
+		changes |= V4L2_EVENT_CTRL_CH_VALUE;
+
+	uvc_ctrl_send_event(handle, ctrl, mapping, val, changes);
+}
+
+static void uvc_ctrl_send_events(struct uvc_fh *handle,
+	struct v4l2_ext_control *xctrls, int xctrls_count)
+{
+	struct uvc_control_mapping *mapping;
+	struct uvc_control *ctrl;
+	u32 changes = V4L2_EVENT_CTRL_CH_VALUE;
+	int i, j;
+
 	for (i = 0; i < xctrls_count; ++i) {
 		ctrl = uvc_find_control(handle->chain, xctrls[i].id, &mapping);
+
+		for (j = 0; j < ARRAY_SIZE(mapping->slave_ids); ++j) {
+			if (!mapping->slave_ids[j])
+				break;
+			uvc_ctrl_send_slave_event(handle,
+						  mapping->slave_ids[j],
+						  xctrls, xctrls_count);
+		}
+
+		/*
+		 * If the master is being modified in the same transaction
+		 * flags may change too.
+		 */
+		if (mapping->master_id)
+			for (j = 0; j < xctrls_count; j++)
+				if (xctrls[j].id == mapping->master_id) {
+					changes |= V4L2_EVENT_CTRL_CH_FLAGS;
+					break;
+				}
+
 		uvc_ctrl_send_event(handle, ctrl, mapping, xctrls[i].value,
-				    V4L2_EVENT_CTRL_CH_VALUE);
+				    changes);
 	}
 }
 
-- 
1.7.9.3


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

* Re: [PATCH 05/10] uvcvideo: Fix a "ignoring return value of ‘__clear_user’" warning
  2012-03-25 11:56 ` [PATCH 05/10] uvcvideo: Fix a "ignoring return value of ‘__clear_user’" warning Hans de Goede
@ 2012-03-27 15:48   ` Laurent Pinchart
  0 siblings, 0 replies; 20+ messages in thread
From: Laurent Pinchart @ 2012-03-27 15:48 UTC (permalink / raw)
  To: Hans de Goede; +Cc: Linux Media Mailing List

Hi Hans,

Thanks for the patch.

On Sunday 25 March 2012 13:56:45 Hans de Goede wrote:
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/video/uvc/uvc_v4l2.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/video/uvc/uvc_v4l2.c
> b/drivers/media/video/uvc/uvc_v4l2.c index ff2cddd..8db90ef 100644
> --- a/drivers/media/video/uvc/uvc_v4l2.c
> +++ b/drivers/media/video/uvc/uvc_v4l2.c
> @@ -1097,7 +1097,8 @@ static int uvc_v4l2_put_xu_mapping(const struct
> uvc_xu_control_mapping *kp, __put_user(kp->menu_count, &up->menu_count))
>  		return -EFAULT;
> 
> -	__clear_user(up->reserved, sizeof(up->reserved));
> +	if (__clear_user(up->reserved, sizeof(up->reserved)))
> +		return -EFAULT;
> 
>  	if (kp->menu_count == 0)
>  		return 0;

-- 
Regards,

Laurent Pinchart


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

* Re: [PATCH 07/10] uvcvideo: Move __uvc_ctrl_get() up
  2012-03-25 11:56 ` [PATCH 07/10] uvcvideo: Move __uvc_ctrl_get() up Hans de Goede
@ 2012-03-28  8:59   ` Laurent Pinchart
  0 siblings, 0 replies; 20+ messages in thread
From: Laurent Pinchart @ 2012-03-28  8:59 UTC (permalink / raw)
  To: Hans de Goede; +Cc: Linux Media Mailing List

Hi Hans,

Thanks for the patch.

On Sunday 25 March 2012 13:56:47 Hans de Goede wrote:
> This avoids the need for doing a forward declaration of __uvc_ctrl_get
> (which is a static function) in later patches in this series.
> 
> Note to reviewers this patch does not change a single line of code, it
> just moves the function up in uvc_ctrl.c a bit.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH 09/10] uvcvideo: Properly report the inactive flag for inactive controls
  2012-03-25 11:56 ` [PATCH 09/10] uvcvideo: Properly report the inactive flag for inactive controls Hans de Goede
@ 2012-03-28  9:12   ` Laurent Pinchart
  2012-03-28 14:09     ` Hans de Goede
  2012-04-08 15:19     ` Hans de Goede
  0 siblings, 2 replies; 20+ messages in thread
From: Laurent Pinchart @ 2012-03-28  9:12 UTC (permalink / raw)
  To: Hans de Goede; +Cc: Linux Media Mailing List

Hi Hans,

Thanks for the patch.

On Sunday 25 March 2012 13:56:49 Hans de Goede wrote:
> Note the unused in this patch slave_ids addition to the mappings will get
> used in a follow up patch to generate control change events for the slave
> ctrls when their flags change due to the master control changing value.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/media/video/uvc/uvc_ctrl.c |   33 +++++++++++++++++++++++++++++++++
> drivers/media/video/uvc/uvcvideo.h |    4 ++++
>  2 files changed, 37 insertions(+)
> 
> diff --git a/drivers/media/video/uvc/uvc_ctrl.c
> b/drivers/media/video/uvc/uvc_ctrl.c index 742496f..91d9007 100644
> --- a/drivers/media/video/uvc/uvc_ctrl.c
> +++ b/drivers/media/video/uvc/uvc_ctrl.c

[snip]

> @@ -943,6 +961,8 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain
> *chain, struct uvc_control_mapping *mapping,
>  	struct v4l2_queryctrl *v4l2_ctrl)
>  {
> +	struct uvc_control_mapping *master_map;
> +	struct uvc_control *master_ctrl = NULL;
>  	struct uvc_menu_info *menu;
>  	unsigned int i;
>  	int ret = 0;
> @@ -958,6 +978,19 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain
> *chain, if (!(ctrl->info.flags & UVC_CTRL_FLAG_SET_CUR))
>  		v4l2_ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> 
> +	if (mapping->master_id)
> +		master_ctrl = uvc_find_control(chain, mapping->master_id,
> +					       &master_map);

As an optimization, do you think it would make sense to add a struct 
uvc_contro *master_ctrl field to struct uvc_control_mapping, and fill it in 
uvc_ctrl_init_ctrl() ? That would require a loop over the mappings at 
initialization time, but would get rid of the search operation at runtime. The 
master_ctrl->info.flags & UVC_CTRL_FLAG_GET_CUR check would be performed at 
initialization time as well, and master_ctrl would be left NULL it the master 
control doesn't support the GET_CUR query.

> +	if (master_ctrl && (master_ctrl->info.flags & UVC_CTRL_FLAG_GET_CUR)) {
> +		s32 val;
> +		ret = __uvc_ctrl_get(chain, master_ctrl, master_map, &val);
> +		if (ret < 0)
> +			goto done;
> +
> +		if (val != mapping->master_manual)
> +				v4l2_ctrl->flags |= V4L2_CTRL_FLAG_INACTIVE;
> +	}
> +
>  	if (!ctrl->cached) {
>  		ret = uvc_ctrl_populate_cache(chain, ctrl);
>  		if (ret < 0)

-- 
Regards,

Laurent Pinchart


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

* Re: [PATCH 08/10] uvcvideo: Add support for control events
  2012-03-25 11:56 ` [PATCH 08/10] uvcvideo: Add support for control events Hans de Goede
@ 2012-03-28  9:25   ` Laurent Pinchart
  0 siblings, 0 replies; 20+ messages in thread
From: Laurent Pinchart @ 2012-03-28  9:25 UTC (permalink / raw)
  To: Hans de Goede; +Cc: Linux Media Mailing List

Hi Hans,

Thanks for the patch.

On Sunday 25 March 2012 13:56:48 Hans de Goede wrote:
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

After addressing the two small comments below,

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/video/uvc/uvc_ctrl.c |  119 ++++++++++++++++++++++++++++++++-
>  drivers/media/video/uvc/uvc_v4l2.c |   42 ++++++++++---
>  drivers/media/video/uvc/uvcvideo.h |   18 ++++--
>  3 files changed, 165 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/media/video/uvc/uvc_ctrl.c
> b/drivers/media/video/uvc/uvc_ctrl.c index fc979c6..742496f 100644
> --- a/drivers/media/video/uvc/uvc_ctrl.c
> +++ b/drivers/media/video/uvc/uvc_ctrl.c

[snip]

> +static void uvc_ctrl_send_events(struct uvc_fh *handle,
> +	struct v4l2_ext_control *xctrls, int xctrls_count)
> +{
> +	struct uvc_control_mapping *mapping;
> +	struct uvc_control *ctrl;
> +	int i;

Could you please make i an unsigned int ?

> +
> +	for (i = 0; i < xctrls_count; ++i) {
> +		ctrl = uvc_find_control(handle->chain, xctrls[i].id, &mapping);
> +		uvc_ctrl_send_event(handle, ctrl, mapping, xctrls[i].value,
> +				    V4L2_EVENT_CTRL_CH_VALUE);
> +	}
> +}
> +

[snip]

> diff --git a/drivers/media/video/uvc/uvcvideo.h
> b/drivers/media/video/uvc/uvcvideo.h index 67f88d8..649a0bb 100644
> --- a/drivers/media/video/uvc/uvcvideo.h
> +++ b/drivers/media/video/uvc/uvcvideo.h

[snip]

> @@ -655,14 +661,16 @@ extern void uvc_ctrl_cleanup_device(struct uvc_device
> *dev); extern int uvc_ctrl_resume_device(struct uvc_device *dev);
> 
>  extern int uvc_ctrl_begin(struct uvc_video_chain *chain);
> -extern int __uvc_ctrl_commit(struct uvc_video_chain *chain, int rollback);
> -static inline int uvc_ctrl_commit(struct uvc_video_chain *chain)
> +extern int __uvc_ctrl_commit(struct uvc_fh *handle, int rollback,
> +			struct v4l2_ext_control *xctrls, int xctrls_count);

You can make the struct v4l2_ext_control *xctrls argument const.

> +static inline int uvc_ctrl_commit(struct uvc_fh *handle,
> +			struct v4l2_ext_control *xctrls, int xctrls_count)
>  {
> -	return __uvc_ctrl_commit(chain, 0);
> +	return __uvc_ctrl_commit(handle, 0, xctrls, xctrls_count);
>  }
> -static inline int uvc_ctrl_rollback(struct uvc_video_chain *chain)
> +static inline int uvc_ctrl_rollback(struct uvc_fh *handle)
>  {
> -	return __uvc_ctrl_commit(chain, 1);
> +	return __uvc_ctrl_commit(handle, 1, NULL, 0);
>  }
> 
>  extern int uvc_ctrl_get(struct uvc_video_chain *chain,

-- 
Regards,

Laurent Pinchart


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

* Re: [PATCH 06/10] uvcvideo: Refactor uvc_ctrl_get and query
  2012-03-25 11:56 ` [PATCH 06/10] uvcvideo: Refactor uvc_ctrl_get and query Hans de Goede
@ 2012-03-28  9:26   ` Laurent Pinchart
  0 siblings, 0 replies; 20+ messages in thread
From: Laurent Pinchart @ 2012-03-28  9:26 UTC (permalink / raw)
  To: Hans de Goede; +Cc: Linux Media Mailing List

Hi Hans,

Thanks for the patch.

On Sunday 25 March 2012 13:56:46 Hans de Goede wrote:
> This is a preparation patch for adding ctrl event support.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/media/video/uvc/uvc_ctrl.c |   69
> ++++++++++++++++++++++++------------ 1 file changed, 46 insertions(+), 23
> deletions(-)
> 
> diff --git a/drivers/media/video/uvc/uvc_ctrl.c
> b/drivers/media/video/uvc/uvc_ctrl.c index 0efd3b1..4002b5b 100644
> --- a/drivers/media/video/uvc/uvc_ctrl.c
> +++ b/drivers/media/video/uvc/uvc_ctrl.c
> @@ -899,24 +899,14 @@ static int uvc_ctrl_populate_cache(struct
> uvc_video_chain *chain, return 0;
>  }
> 
> -int uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
> +static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
> +	struct uvc_control *ctrl,
> +	struct uvc_control_mapping *mapping,
>  	struct v4l2_queryctrl *v4l2_ctrl)
>  {
> -	struct uvc_control *ctrl;
> -	struct uvc_control_mapping *mapping;
>  	struct uvc_menu_info *menu;
>  	unsigned int i;
> -	int ret;
> -
> -	ret = mutex_lock_interruptible(&chain->ctrl_mutex);
> -	if (ret < 0)
> -		return -ERESTARTSYS;
> -

Now that this function doesn't lock a mutex, you can replace the goto done 
statements with return ret (patch 09/10 adds a goto done that will need to be 
replaced as well). With this change,

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> -	ctrl = uvc_find_control(chain, v4l2_ctrl->id, &mapping);
> -	if (ctrl == NULL) {
> -		ret = -EINVAL;
> -		goto done;
> -	}
> +	int ret = 0;
> 
>  	memset(v4l2_ctrl, 0, sizeof *v4l2_ctrl);
>  	v4l2_ctrl->id = mapping->id;
> @@ -985,6 +975,28 @@ int uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
>  				  uvc_ctrl_data(ctrl, UVC_CTRL_DATA_RES));
> 
>  done:
> +	return ret;
> +}

-- 
Regards,

Laurent Pinchart


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

* Re: [PATCH 10/10] uvcvideo: Send control change events for slave ctrls when the master changes
  2012-03-25 11:56 ` [PATCH 10/10] uvcvideo: Send control change events for slave ctrls when the master changes Hans de Goede
@ 2012-03-28  9:34   ` Laurent Pinchart
  2012-03-28 14:11     ` Hans de Goede
  0 siblings, 1 reply; 20+ messages in thread
From: Laurent Pinchart @ 2012-03-28  9:34 UTC (permalink / raw)
  To: Hans de Goede; +Cc: Linux Media Mailing List

Hi Hans,

Thanks for the patch.

On Sunday 25 March 2012 13:56:50 Hans de Goede wrote:
> This allows v4l2 control UI-s to update the inactive state (ie grey-ing
> out of controls) for slave controls when the master control changes.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/media/video/uvc/uvc_ctrl.c |   55 +++++++++++++++++++++++++++++++--
>  1 file changed, 52 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/video/uvc/uvc_ctrl.c
> b/drivers/media/video/uvc/uvc_ctrl.c index 91d9007..2d06fd8 100644
> --- a/drivers/media/video/uvc/uvc_ctrl.c
> +++ b/drivers/media/video/uvc/uvc_ctrl.c

[snip]

> +static void uvc_ctrl_send_events(struct uvc_fh *handle,
> +	struct v4l2_ext_control *xctrls, int xctrls_count)
> +{
> +	struct uvc_control_mapping *mapping;
> +	struct uvc_control *ctrl;
> +	u32 changes = V4L2_EVENT_CTRL_CH_VALUE;
> +	int i, j;
> +
>  	for (i = 0; i < xctrls_count; ++i) {
>  		ctrl = uvc_find_control(handle->chain, xctrls[i].id, &mapping);
> +
> +		for (j = 0; j < ARRAY_SIZE(mapping->slave_ids); ++j) {
> +			if (!mapping->slave_ids[j])
> +				break;
> +			uvc_ctrl_send_slave_event(handle,
> +						  mapping->slave_ids[j],
> +						  xctrls, xctrls_count);
> +		}
> +
> +		/*
> +		 * If the master is being modified in the same transaction
> +		 * flags may change too.
> +		 */
> +		if (mapping->master_id)
> +			for (j = 0; j < xctrls_count; j++)
> +				if (xctrls[j].id == mapping->master_id) {
> +					changes |= V4L2_EVENT_CTRL_CH_FLAGS;

Should you verify that the modification to the master control actually caused 
a slave control flags change, or would that be overkill ?

> +					break;
> +				}

Could you please put brackets around the for and the if ?

> +
>  		uvc_ctrl_send_event(handle, ctrl, mapping, xctrls[i].value,
> -				    V4L2_EVENT_CTRL_CH_VALUE);
> +				    changes);
>  	}
>  }

-- 
Regards,

Laurent Pinchart


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

* Re: [PATCH 09/10] uvcvideo: Properly report the inactive flag for inactive controls
  2012-03-28  9:12   ` Laurent Pinchart
@ 2012-03-28 14:09     ` Hans de Goede
  2012-04-08 15:19     ` Hans de Goede
  1 sibling, 0 replies; 20+ messages in thread
From: Hans de Goede @ 2012-03-28 14:09 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: Linux Media Mailing List

Hi,

Thanks for the reviews!

On 03/28/2012 11:12 AM, Laurent Pinchart wrote:
> Hi Hans,
>
> Thanks for the patch.
>
> On Sunday 25 March 2012 13:56:49 Hans de Goede wrote:
>> Note the unused in this patch slave_ids addition to the mappings will get
>> used in a follow up patch to generate control change events for the slave
>> ctrls when their flags change due to the master control changing value.
>>
>> Signed-off-by: Hans de Goede<hdegoede@redhat.com>
>> ---
>>   drivers/media/video/uvc/uvc_ctrl.c |   33 +++++++++++++++++++++++++++++++++
>> drivers/media/video/uvc/uvcvideo.h |    4 ++++
>>   2 files changed, 37 insertions(+)
>>
>> diff --git a/drivers/media/video/uvc/uvc_ctrl.c
>> b/drivers/media/video/uvc/uvc_ctrl.c index 742496f..91d9007 100644
>> --- a/drivers/media/video/uvc/uvc_ctrl.c
>> +++ b/drivers/media/video/uvc/uvc_ctrl.c
>
> [snip]
>
>> @@ -943,6 +961,8 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain
>> *chain, struct uvc_control_mapping *mapping,
>>   	struct v4l2_queryctrl *v4l2_ctrl)
>>   {
>> +	struct uvc_control_mapping *master_map;
>> +	struct uvc_control *master_ctrl = NULL;
>>   	struct uvc_menu_info *menu;
>>   	unsigned int i;
>>   	int ret = 0;
>> @@ -958,6 +978,19 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain
>> *chain, if (!(ctrl->info.flags&  UVC_CTRL_FLAG_SET_CUR))
>>   		v4l2_ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY;
>>
>> +	if (mapping->master_id)
>> +		master_ctrl = uvc_find_control(chain, mapping->master_id,
>> +					&master_map);
>
> As an optimization, do you think it would make sense to add a struct
> uvc_contro *master_ctrl field to struct uvc_control_mapping, and fill it in
> uvc_ctrl_init_ctrl() ? That would require a loop over the mappings at
> initialization time, but would get rid of the search operation at runtime. The
> master_ctrl->info.flags&  UVC_CTRL_FLAG_GET_CUR check would be performed at
> initialization time as well, and master_ctrl would be left NULL it the master
> control doesn't support the GET_CUR query.

That sounds like a worthwhile optimization. I'll do that in my next revision
of this patch-set. I'm not entirely sure when I'll have time to do the next
revision, but I assume this won't go in until 3.5 so we've plenty of time
for now.

Regards,

Hans

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

* Re: [PATCH 10/10] uvcvideo: Send control change events for slave ctrls when the master changes
  2012-03-28  9:34   ` Laurent Pinchart
@ 2012-03-28 14:11     ` Hans de Goede
  0 siblings, 0 replies; 20+ messages in thread
From: Hans de Goede @ 2012-03-28 14:11 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: Linux Media Mailing List

Hi,

On 03/28/2012 11:34 AM, Laurent Pinchart wrote:
> Hi Hans,
>
> Thanks for the patch.
>
> On Sunday 25 March 2012 13:56:50 Hans de Goede wrote:
>> This allows v4l2 control UI-s to update the inactive state (ie grey-ing
>> out of controls) for slave controls when the master control changes.
>>
>> Signed-off-by: Hans de Goede<hdegoede@redhat.com>
>> ---
>>   drivers/media/video/uvc/uvc_ctrl.c |   55 +++++++++++++++++++++++++++++++--
>>   1 file changed, 52 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/media/video/uvc/uvc_ctrl.c
>> b/drivers/media/video/uvc/uvc_ctrl.c index 91d9007..2d06fd8 100644
>> --- a/drivers/media/video/uvc/uvc_ctrl.c
>> +++ b/drivers/media/video/uvc/uvc_ctrl.c
>
> [snip]
>
>> +static void uvc_ctrl_send_events(struct uvc_fh *handle,
>> +	struct v4l2_ext_control *xctrls, int xctrls_count)
>> +{
>> +	struct uvc_control_mapping *mapping;
>> +	struct uvc_control *ctrl;
>> +	u32 changes = V4L2_EVENT_CTRL_CH_VALUE;
>> +	int i, j;
>> +
>>   	for (i = 0; i<  xctrls_count; ++i) {
>>   		ctrl = uvc_find_control(handle->chain, xctrls[i].id,&mapping);
>> +
>> +		for (j = 0; j<  ARRAY_SIZE(mapping->slave_ids); ++j) {
>> +			if (!mapping->slave_ids[j])
>> +				break;
>> +			uvc_ctrl_send_slave_event(handle,
>> +						  mapping->slave_ids[j],
>> +						  xctrls, xctrls_count);
>> +		}
>> +
>> +		/*
>> +		 * If the master is being modified in the same transaction
>> +		 * flags may change too.
>> +		 */
>> +		if (mapping->master_id)
>> +			for (j = 0; j<  xctrls_count; j++)
>> +				if (xctrls[j].id == mapping->master_id) {
>> +					changes |= V4L2_EVENT_CTRL_CH_FLAGS;
>
> Should you verify that the modification to the master control actually caused
> a slave control flags change, or would that be overkill ?

I think that that would be overkill. This sending of events for flag changes
already is not 100% trivial and I think keeping it KISS and occasionally sending
an unneeded change event is better then making the code more complicated.


>
>> +					break;
>> +				}
>
> Could you please put brackets around the for and the if ?

Will do.

Regards,

Hans

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

* Re: [PATCH 09/10] uvcvideo: Properly report the inactive flag for inactive controls
  2012-03-28  9:12   ` Laurent Pinchart
  2012-03-28 14:09     ` Hans de Goede
@ 2012-04-08 15:19     ` Hans de Goede
  1 sibling, 0 replies; 20+ messages in thread
From: Hans de Goede @ 2012-04-08 15:19 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: Linux Media Mailing List

Hi,

On 03/28/2012 11:12 AM, Laurent Pinchart wrote:
> Hi Hans,
>
> Thanks for the patch.
>
> On Sunday 25 March 2012 13:56:49 Hans de Goede wrote:
>> Note the unused in this patch slave_ids addition to the mappings will get
>> used in a follow up patch to generate control change events for the slave
>> ctrls when their flags change due to the master control changing value.
>>
>> Signed-off-by: Hans de Goede<hdegoede@redhat.com>
>> ---
>>   drivers/media/video/uvc/uvc_ctrl.c |   33 +++++++++++++++++++++++++++++++++
>> drivers/media/video/uvc/uvcvideo.h |    4 ++++
>>   2 files changed, 37 insertions(+)
>>
>> diff --git a/drivers/media/video/uvc/uvc_ctrl.c
>> b/drivers/media/video/uvc/uvc_ctrl.c index 742496f..91d9007 100644
>> --- a/drivers/media/video/uvc/uvc_ctrl.c
>> +++ b/drivers/media/video/uvc/uvc_ctrl.c
>
> [snip]
>
>> @@ -943,6 +961,8 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain
>> *chain, struct uvc_control_mapping *mapping,
>>   	struct v4l2_queryctrl *v4l2_ctrl)
>>   {
>> +	struct uvc_control_mapping *master_map;
>> +	struct uvc_control *master_ctrl = NULL;
>>   	struct uvc_menu_info *menu;
>>   	unsigned int i;
>>   	int ret = 0;
>> @@ -958,6 +978,19 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain
>> *chain, if (!(ctrl->info.flags&  UVC_CTRL_FLAG_SET_CUR))
>>   		v4l2_ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY;
>>
>> +	if (mapping->master_id)
>> +		master_ctrl = uvc_find_control(chain, mapping->master_id,
>> +					&master_map);
>
> As an optimization, do you think it would make sense to add a struct
> uvc_contro *master_ctrl field to struct uvc_control_mapping, and fill it in
> uvc_ctrl_init_ctrl() ? That would require a loop over the mappings at
> initialization time, but would get rid of the search operation at runtime. The
> master_ctrl->info.flags&  UVC_CTRL_FLAG_GET_CUR check would be performed at
> initialization time as well, and master_ctrl would be left NULL it the master
> control doesn't support the GET_CUR query.
>

I've just tried to implement this, and it is a bit tricky:
1) The looking up of master_id cannot be done from uvc_ctrl_init_ctrl(), since
all the mappings of the entity need to be added before the lookup can be done,
making it necessary to add another loop to uvc_ctrl_init_device().
2) Not only the ctrl itself, but also the mapping needs to be cached in the
uvc_control_mapping, since we need both.

Looking closer at things, I've opted to implement a simpler optimization,
replacing the uvc_find_control with a __uvc_find_control, since the master
and it slaves are always part of the same entity. This means we still do
some searching runtime, but a lot less.

I'll send an updated patchset with this change in it soon.

Regards,

Hans

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

end of thread, other threads:[~2012-04-08 15:17 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-25 11:56 uvc & pwc: Add support for control events (v2) Hans de Goede
2012-03-25 11:56 ` [PATCH 01/10] pwc: Add support for control events Hans de Goede
2012-03-25 11:56 ` [PATCH 02/10] media/radio: use v4l2_ctrl_subscribe_event where possible Hans de Goede
2012-03-25 11:56 ` [PATCH 03/10] v4l2-event: Add v4l2_subscribed_event_ops Hans de Goede
2012-03-25 11:56 ` [PATCH 04/10] v4l2-ctrls: Use v4l2_subscribed_event_ops Hans de Goede
2012-03-25 11:56 ` [PATCH 05/10] uvcvideo: Fix a "ignoring return value of ‘__clear_user’" warning Hans de Goede
2012-03-27 15:48   ` Laurent Pinchart
2012-03-25 11:56 ` [PATCH 06/10] uvcvideo: Refactor uvc_ctrl_get and query Hans de Goede
2012-03-28  9:26   ` Laurent Pinchart
2012-03-25 11:56 ` [PATCH 07/10] uvcvideo: Move __uvc_ctrl_get() up Hans de Goede
2012-03-28  8:59   ` Laurent Pinchart
2012-03-25 11:56 ` [PATCH 08/10] uvcvideo: Add support for control events Hans de Goede
2012-03-28  9:25   ` Laurent Pinchart
2012-03-25 11:56 ` [PATCH 09/10] uvcvideo: Properly report the inactive flag for inactive controls Hans de Goede
2012-03-28  9:12   ` Laurent Pinchart
2012-03-28 14:09     ` Hans de Goede
2012-04-08 15:19     ` Hans de Goede
2012-03-25 11:56 ` [PATCH 10/10] uvcvideo: Send control change events for slave ctrls when the master changes Hans de Goede
2012-03-28  9:34   ` Laurent Pinchart
2012-03-28 14:11     ` Hans de Goede

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).