V4L2 file handles and event interface

Add support for subscribing all events with a special id V4L2_EVENT_ALL. If V4L2_EVENT_ALL is subscribed, no other events may be subscribed. Otherwise V4L2_EVENT_ALL is considered just as any other event.

Signed-off-by Sakari Ailus <[email protected]>

---
 drivers/media/video/v4l2-event.c |   13 ++++++++++++-
 include/linux/videodev2.h        |    1 +
 2 files changed, 13 insertions(+), 1 deletions(-)

drivers/media/video/v4l2-event.c +12 -1

index 0af0de5..68b3cf4 100644
@@ -139,6 +139,14 @@ static struct v4l2_subscribed_event *__v4l2_event_subscribed(
 	struct v4l2_events *events = fh->events;
 	struct v4l2_subscribed_event *sev;
 
+	if (list_empty(&events->subscribed))
+		return NULL;
+
+	sev = list_entry(events->subscribed.next,
+			 struct v4l2_subscribed_event, list);
+	if (sev->type == V4L2_EVENT_ALL)
+		return sev;
+
 	list_for_each_entry(sev, &events->subscribed, list) {
 		if (sev->type == type)
 			return sev;

@@ -222,6 +230,8 @@ int v4l2_event_subscribe(struct v4l2_fh *fh,
 	/* Allow subscribing to valid events only. */
 	if (sub->type < V4L2_EVENT_PRIVATE_START)
 		switch (sub->type) {
+		case V4L2_EVENT_ALL:
+			break;
 		default:
 			return -EINVAL;
 		}

@@ -262,7 +272,8 @@ int v4l2_event_unsubscribe(struct v4l2_fh *fh,
 
 	sev = __v4l2_event_subscribed(fh, sub->type);
 
-	if (sev == NULL) {
+	if (sev == NULL ||
+	    (sub->type != V4L2_EVENT_ALL && sev->type == V4L2_EVENT_ALL)) {
 		spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
 		return -EINVAL;
 	}

include/linux/videodev2.h +1 -0

index a19ae89..9ae9a1c 100644
@@ -1553,6 +1553,7 @@ struct v4l2_event_subscription {
 	__u32		reserved[7];
 };
 
+#define V4L2_EVENT_ALL				0
 #define V4L2_EVENT_PRIVATE_START		0x08000000
 
 /*

Quoted 3 lines · On Wednesday 10 February 2010 15:58:09 Sakari Ailus wrote:

Add support for subscribing all events with a special id V4L2_EVENT_ALL. If V4L2_EVENT_ALL is subscribed, no other events may be subscribed. Otherwise V4L2_EVENT_ALL is considered just as any other event.

We should do this differently. I think that EVENT_ALL should not be used internally (i.e. in the actual list of subscribed events), but just as a special value for the subscribe and unsubscribe ioctls. So when used with unsubscribe you can just unsubscribe all subscribed events and when used with subscribe, then you just subscribe all valid events (valid for that device node).

So in v4l2-event.c you will have a v4l2_event_unsubscribe_all() to quickly unsubscribe all events.

In order to easily add all events from the driver it would help if the v4l2_event_subscribe and v4l2_event_unsubscribe just take the event type as argument rather than the whole v4l2_event_subscription struct.

You will then get something like this in the driver:

	if (sub->type == V4L2_EVENT_ALL) {
		int ret = v4l2_event_alloc(fh, 60);

		ret = ret ? ret : v4l2_event_subscribe(fh, V4L2_EVENT_EOS);
		ret = ret ? ret : v4l2_event_subscribe(fh, V4L2_EVENT_VSYNC);
		return ret;
	}

An alternative might be to add a v4l2_event_subscribe_all(fh, const u32 *events) where 'events' is a 0 terminated list of events that need to be subscribed.

For each event this function would then call:

fh->vdev->ioctl_ops->vidioc_subscribe_event(fh, sub);

The nice thing about that is that in the driver you have a minimum of fuss.

I'm leaning towards this second solution due to the simple driver implementation.

Handling EVENT_ALL will simplify things substantially IMHO.

Hi Hans,

Quoted 32 lines · On Saturday 13 February 2010 15:42:20 Hans Verkuil wrote:

Quoted 1 line · On Wednesday 10 February 2010 15:58:09 Sakari Ailus wrote:

Add support for subscribing all events with a special id

V4L2_EVENT_ALL.

Quoted 2 lines

If V4L2_EVENT_ALL is subscribed, no other events may be subscribed. Otherwise V4L2_EVENT_ALL is considered just as any other event.

We should do this differently. I think that EVENT_ALL should not be used internally (i.e. in the actual list of subscribed events), but just as a special value for the subscribe and unsubscribe ioctls. So when used with
unsubscribe you can just unsubscribe all subscribed events and when used with subscribe, then you just subscribe all valid events (valid for that device node).

So in v4l2-event.c you will have a v4l2_event_unsubscribe_all() to quickly
unsubscribe all events.

In order to easily add all events from the driver it would help if the v4l2_event_subscribe and v4l2_event_unsubscribe just take the event type as argument rather than the whole v4l2_event_subscription struct.

You will then get something like this in the driver:

	if (sub->type == V4L2_EVENT_ALL) {
		int ret = v4l2_event_alloc(fh, 60);

		ret = ret ? ret : v4l2_event_subscribe(fh, V4L2_EVENT_EOS);
		ret = ret ? ret : v4l2_event_subscribe(fh, V4L2_EVENT_VSYNC);
		return ret;
	}

Quoted 3 lines

An alternative might be to add a v4l2_event_subscribe_all(fh, const u32 *events) where 'events' is a 0 terminated list of events that need to be subscribed.

Then don't call it v4l2_event_subscribe_all if it only subscribes to a set of
event :-)

Quoted 11 lines

For each event this function would then call:

fh->vdev->ioctl_ops->vidioc_subscribe_event(fh, sub);

The nice thing about that is that in the driver you have a minimum of fuss.

I'm leaning towards this second solution due to the simple driver implementation.

Handling EVENT_ALL will simplify things substantially IMHO.

I'm wondering if subscribing to all events should be allowed. Do we have use
cases for that ? I'm always a bit cautious when adding APIs with no users, as
that means the API has often not been properly tested against possible use cases and mistakes will need to be supported forever (or at least for a long
time).

--
Regards,

Laurent Pinchart
--
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 16 lines

Then don't call it v4l2_event_subscribe_all if it only subscribes to a set of
event :-)

Quoted 11 lines

For each event this function would then call:

fh->vdev->ioctl_ops->vidioc_subscribe_event(fh, sub);

The nice thing about that is that in the driver you have a minimum of fuss.

I'm leaning towards this second solution due to the simple driver implementation.

Handling EVENT_ALL will simplify things substantially IMHO.

Quoted 8 lines

I'm wondering if subscribing to all events should be allowed. Do we have use
cases for that ? I'm always a bit cautious when adding APIs with no users, as
that means the API has often not been properly tested against possible use cases and mistakes will need to be supported forever (or at least for a long
time).

I think that is a good point. Supporting V4L2_EVENT_ALL makes sense for unsubscribe, but does it makes sense for subscribe as well? I think it does not. It just doesn't feel right when I tried to implement it in ivtv.

I also wonder whether the unsubscribe API shouldn't just receive the event type instead of the big subscription struct. Or get its own struct. I don't think it makes much sense that they both have the same struct.

Update the count field properly by setting it to exactly to number of further available events.

Signed-off-by Sakari Ailus <[email protected]>

---
 drivers/media/video/v4l2-event.c |   29 +++++++++++++++++------------
 include/media/v4l2-event.h       |    6 +++++-
 2 files changed, 22 insertions(+), 13 deletions(-)

include/media/v4l2-event.h +4 -1

+
 struct v4l2_fh;
 struct video_device;
 

@@ -45,11 +47,13 @@ struct v4l2_events {
 	wait_queue_head_t	wait;
 	struct list_head	subscribed; /* Subscribed events */
 	struct list_head	available; /* Dequeueable event */
+	unsigned int		navailable;
+	unsigned int		max_alloc; /* Never allocate more. */
 	struct list_head	free; /* Events ready for use */
 };
 
 int v4l2_event_alloc(struct v4l2_fh *fh, unsigned int n);
-int v4l2_event_init(struct v4l2_fh *fh, unsigned int n);
+int v4l2_event_init(struct v4l2_fh *fh, unsigned int n, unsigned int max_alloc);
 void v4l2_event_exit(struct v4l2_fh *fh);
 int v4l2_event_dequeue(struct v4l2_fh *fh, struct v4l2_event *event);
 struct v4l2_subscribed_event *v4l2_event_subscribed(

Add support for event handling to do_ioctl.

Signed-off-by Sakari Ailus <[email protected]>

---
 drivers/media/video/Makefile     |    2 +-
 drivers/media/video/v4l2-ioctl.c |   49 ++++++++++++++++++++++++++++++++++++++
 include/media/v4l2-ioctl.h       |    5 ++++
 3 files changed, 55 insertions(+), 1 deletions(-)

drivers/media/video/Makefile +1 -1

index b888ad1..68253d6 100644
@@ -11,7 +11,7 @@ stkwebcam-objs	:=	stk-webcam.o stk-sensor.o
 omap2cam-objs	:=	omap24xxcam.o omap24xxcam-dma.o
 
 videodev-objs	:=	v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-subdev.o \
-			v4l2-fh.o
+			v4l2-fh.o v4l2-event.o
 
 # V4L2 core modules
 

drivers/media/video/v4l2-ioctl.c +16 -0

index bfc4696..e0b9401 100644
@@ -25,6 +25,7 @@
 #endif
 #include <media/v4l2-common.h>
 #include <media/v4l2-ioctl.h>
+#include <media/v4l2-event.h>
 #include <media/v4l2-chip-ident.h>
 
 #define dbgarg(cmd, fmt, arg...) \

@@ -1797,7 +1798,55 @@ static long __video_do_ioctl(struct file *file,
 		}
 		break;
 	}
+	case VIDIOC_DQEVENT:
+	{
+		struct v4l2_event *ev = arg;
+
+		if (!ops->vidioc_subscribe_event)
+			break;
+
+		ret = v4l2_event_dequeue(fh, ev);
+		if (ret < 0) {
+			dbgarg(cmd, "no pending events?");
+			break;
+		}
+		dbgarg(cmd,
+		       "count=%d, type=0x%8.8x, sequence=%d, "
+		       "timestamp=%lu.%9.9lu ",

drivers/media/video/v4l2-ioctl.c +7 -0

+
+		ret = ops->vidioc_subscribe_event(fh, sub);
+		if (ret < 0) {
+			dbgarg(cmd, "failed, ret=%ld", ret);
+			break;
+		}
+		dbgarg(cmd, "type=0x%8.8x", sub->type);

drivers/media/video/v4l2-ioctl.c +7 -0

+		if (ret < 0) {
+			dbgarg(cmd, "failed, ret=%ld", ret);
+			break;
+		}
+		dbgarg(cmd, "type=0x%8.8x", sub->type);
+		break;
+	}
 	default:
 	{
 		if (!ops->vidioc_default)

include/media/v4l2-ioctl.h +5 -0

index 7a4529d..ed3fff5 100644
@@ -21,6 +21,8 @@
 #include <linux/videodev2.h>
 #endif
 
+struct v4l2_fh;
+
 struct v4l2_ioctl_ops {
 	/* ioctl callbacks */
 

@@ -239,6 +241,9 @@ struct v4l2_ioctl_ops {
 	int (*vidioc_enum_frameintervals) (struct file *file, void *fh,
 					   struct v4l2_frmivalenum *fival);
 
+	int (*vidioc_subscribe_event)  (struct v4l2_fh *fh,
+					struct v4l2_event_subscription *sub);
+
 	/* For other private ioctls */
 	long (*vidioc_default)	       (struct file *file, void *fh,
 					int cmd, void *arg);

Quoted 64 lines · On Wednesday 10 February 2010 15:58:06 Sakari Ailus wrote:

Add support for event handling to do_ioctl.

Signed-off-by Sakari Ailus <[email protected]>

---

 drivers/media/video/Makefile     |    2 +-
 drivers/media/video/v4l2-ioctl.c |   49
 ++++++++++++++++++++++++++++++++++++++ include/media/v4l2-ioctl.h      
 |    5 ++++
 3 files changed, 55 insertions(+), 1 deletions(-)

drivers/media/video/Makefile +1 -1

index b888ad1..68253d6 100644
@@ -11,7 +11,7 @@ stkwebcam-objs	:=	stk-webcam.o stk-sensor.o
 
 omap2cam-objs	:=	omap24xxcam.o omap24xxcam-dma.o
 
 videodev-objs	:=	v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-subdev.o \
 
-			v4l2-fh.o
+			v4l2-fh.o v4l2-event.o
 

# V4L2 core modules

(quoted diff) +0 -0

b/drivers/media/video/v4l2-ioctl.c index bfc4696..e0b9401 100644

drivers/media/video/v4l2-ioctl.c +1 -0

@@ -25,6 +25,7 @@
 
 #endif
 #include <media/v4l2-common.h>
 #include <media/v4l2-ioctl.h>
 
+#include <media/v4l2-event.h>
 

#include <media/v4l2-chip-ident.h>

#define dbgarg(cmd, fmt, arg...) \

drivers/media/video/v4l2-ioctl.c +15 -0

@@ -1797,7 +1798,55 @@ static long __video_do_ioctl(struct file *file,
 
 		}
 		break;
 
 	}
 
+	case VIDIOC_DQEVENT:
+	{
+		struct v4l2_event *ev = arg;
+
+		if (!ops->vidioc_subscribe_event)
+			break;
+
+		ret = v4l2_event_dequeue(fh, ev);
+		if (ret < 0) {
+			dbgarg(cmd, "no pending events?");
+			break;
+		}
+		dbgarg(cmd,
+		       "count=%d, type=0x%8.8x, sequence=%d, "
+		       "timestamp=%lu.%9.9lu ",

drivers/media/video/v4l2-ioctl.c static long __video_do_ioctl(struct file *file, · line 1828

Quoted 10 lines

drivers/media/video/v4l2-ioctl.c +9 -0

+		       ev->count, ev->type, ev->sequence,
+		       ev->timestamp.tv_sec, ev->timestamp.tv_nsec);
+		break;
+	}
+	case VIDIOC_SUBSCRIBE_EVENT:
+	{
+		struct v4l2_event_subscription *sub = arg;
 
+		if (!ops->vidioc_subscribe_event)
+			break;

I know I said that we could use this test to determine if fh is of type v4l2_fh, but that only works in this specific case, but not in the general case. For example, I want to add support for the prio ioctls to v4l2_fh, and then I probably have no vidioc_subscribe_event set since few drivers will need that.

Instead I suggest that we add a new flag to v4l2-dev.h:

V4L2_FL_USES_V4L2_FH (1)

The v4l2_fh_add() function can then set this flag.

Quoted 14 lines

drivers/media/video/v4l2-ioctl.c +14 -0

+
+		ret = ops->vidioc_subscribe_event(fh, sub);
+		if (ret < 0) {
+			dbgarg(cmd, "failed, ret=%ld", ret);
+			break;
+		}
+		dbgarg(cmd, "type=0x%8.8x", sub->type);
+		break;
+	}
+	case VIDIOC_UNSUBSCRIBE_EVENT:
+	{
+		struct v4l2_event_subscription *sub = arg;
+
+		if (!ops->vidioc_subscribe_event)

drivers/media/video/v4l2-ioctl.c static long __video_do_ioctl(struct file *file, · line 1842

Quoted 3 lines

drivers/media/video/v4l2-ioctl.c +3 -0

+			break;
+
+		ret = v4l2_event_unsubscribe(fh, sub);

We should add an unsubscribe op as well. One reason is to add EVENT_ALL support (see my comments in patch 7/7), the other is that in some cases drivers might need to take some special action in response to subscribing an event. And a driver needs a way to undo that when unsubscribing.

Quoted 106 lines · On Wednesday 10 February 2010 15:58:06 Sakari Ailus wrote:

Quoted 5 lines

Add support for event handling to do_ioctl.

Signed-off-by Sakari Ailus <[email protected]>

---

Review

Quoted 7 lines

 drivers/media/video/Makefile     |    2 +-
 drivers/media/video/v4l2-ioctl.c |   49
 ++++++++++++++++++++++++++++++++++++++ include/media/v4l2-ioctl.h
 |    5 ++++
 3 files changed, 55 insertions(+), 1 deletions(-)

(quoted diff) +0 -0

b/drivers/media/video/Makefile

drivers/media/video/Makefile stkwebcam-objs := stk-webcam.o stk-sensor.o · line 14

Quoted 8 lines

index b888ad1..68253d6 100644

drivers/media/video/Makefile +0 -0

@@ -11,7 +11,7 @@ stkwebcam-objs	:=	stk-webcam.o stk-sensor.o
 
 omap2cam-objs	:=	omap24xxcam.o omap24xxcam-dma.o
 
 videodev-objs	:=	v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-subdev.o

\

Quoted 44 lines

drivers/media/video/Makefile +1 -1

-			v4l2-fh.o
+			v4l2-fh.o v4l2-event.o
 
 # V4L2 core modules

(quoted diff) +0 -0

b/drivers/media/video/v4l2-ioctl.c index bfc4696..e0b9401 100644

drivers/media/video/v4l2-ioctl.c +1 -0

@@ -25,6 +25,7 @@
 
 #endif
 #include <media/v4l2-common.h>
 #include <media/v4l2-ioctl.h>
 
+#include <media/v4l2-event.h>
 

#include <media/v4l2-chip-ident.h>

#define dbgarg(cmd, fmt, arg...) \

drivers/media/video/v4l2-ioctl.c +15 -0

@@ -1797,7 +1798,55 @@ static long __video_do_ioctl(struct file *file,
 
 		}
 		break;
 
 	}
 
+	case VIDIOC_DQEVENT:
+	{
+		struct v4l2_event *ev = arg;
+
+		if (!ops->vidioc_subscribe_event)
+			break;
+
+		ret = v4l2_event_dequeue(fh, ev);
+		if (ret < 0) {
+			dbgarg(cmd, "no pending events?");
+			break;
+		}
+		dbgarg(cmd,
+		       "count=%d, type=0x%8.8x, sequence=%d, "
+		       "timestamp=%lu.%9.9lu ",

drivers/media/video/v4l2-ioctl.c static long __video_do_ioctl(struct file *file, · line 1828

Quoted 10 lines

drivers/media/video/v4l2-ioctl.c +9 -0

+		       ev->count, ev->type, ev->sequence,
+		       ev->timestamp.tv_sec, ev->timestamp.tv_nsec);
+		break;
+	}
+	case VIDIOC_SUBSCRIBE_EVENT:
+	{
+		struct v4l2_event_subscription *sub = arg;
 
+		if (!ops->vidioc_subscribe_event)
+			break;

I know I said that we could use this test to determine if fh is of type v4l2_fh, but that only works in this specific case, but not in the general
case. For example, I want to add support for the prio ioctls to v4l2_fh, and then I probably have no vidioc_subscribe_event set since few drivers will need that.

Instead I suggest that we add a new flag to v4l2-dev.h:

V4L2_FL_USES_V4L2_FH (1)

The v4l2_fh_add() function can then set this flag.

Quoted 15 lines

drivers/media/video/v4l2-ioctl.c +15 -0

+
+		ret = ops->vidioc_subscribe_event(fh, sub);
+		if (ret < 0) {
+			dbgarg(cmd, "failed, ret=%ld", ret);
+			break;
+		}
+		dbgarg(cmd, "type=0x%8.8x", sub->type);
+		break;
+	}
+	case VIDIOC_UNSUBSCRIBE_EVENT:
+	{
+		struct v4l2_event_subscription *sub = arg;
+
+		if (!ops->vidioc_subscribe_event)
+			break;

Quoted 8 lines

drivers/media/video/v4l2-ioctl.c static long __video_do_ioctl(struct file *file, · line 1845

Quoted 2 lines

drivers/media/video/v4l2-ioctl.c +2 -0

+
+		ret = v4l2_event_unsubscribe(fh, sub);

We should add an unsubscribe op as well. One reason is to add EVENT_ALL support (see my comments in patch 7/7), the other is that in some cases drivers might need to take some special action in response to subscribing
an event. And a driver needs a way to undo that when unsubscribing.

Agreed. Should we allow drivers not to define the unsubscribe operation when
they don't need it ? In that case v4l2_event_unsubscribe should be called in
VIDIOC_UNSUBSCRIBE_EVENT, outside of the operation handler.

Similarly, shouldn't v4l2_event_subscribe be called in VIDIOC_SUBSCRIBE_EVENT,
outside of the operation handler ?

--
Regards,

Laurent Pinchart

Add event handling backend to V4L2. The backend handles event subscription and delivery to file handles. Event subscriptions are based on file handle. Events may be delivered to all subscribed file handles on a device independent of where they originate from.

Signed-off-by Sakari Ailus <[email protected]>

---
 drivers/media/video/v4l2-event.c |  261 ++++++++++++++++++++++++++++++++++++++
 drivers/media/video/v4l2-fh.c    |    4 +
 include/media/v4l2-event.h       |   64 +++++++++
 include/media/v4l2-fh.h          |    2 +
 4 files changed, 331 insertions(+), 0 deletions(-)
 create mode 100644 drivers/media/video/v4l2-event.c
 create mode 100644 include/media/v4l2-event.h

drivers/media/video/v4l2-event.c new file +142 -0

new file mode 100644
index 0000000..d13c1e9
@@ -0,0 +1,261 @@
+/*
+ * drivers/media/video/v4l2-event.c
+ *
+ * V4L2 events.
+ *
+ * Copyright (C) 2009 Nokia Corporation.
+ *
+ * Contact: Sakari Ailus <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ */
+
+#include <media/v4l2-dev.h>
+#include <media/v4l2-event.h>
+
+#include <linux/sched.h>
+
+/* In error case, return number of events *not* allocated. */
+int v4l2_event_alloc(struct v4l2_fh *fh, unsigned int n)
+{
+	struct v4l2_events *events = fh->events;
+	unsigned long flags;
+
+	for (; n > 0; n--) {
+		struct v4l2_kevent *kev;
+
+		kev = kzalloc(sizeof(*kev), GFP_KERNEL);
+		if (kev == NULL)
+			return -ENOMEM;
+
+		spin_lock_irqsave(&fh->vdev->fh_lock, flags);
+		list_add_tail(&kev->list, &events->free);
+		spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(v4l2_event_alloc);
+
+#define list_kfree(list, type, member)				\
+	while (!list_empty(list)) {				\
+		type *hi;					\
+		hi = list_first_entry(list, type, member);	\
+		list_del(&hi->member);				\
+		kfree(hi);					\
+	}
+
+void v4l2_event_exit(struct v4l2_fh *fh)
+{
+	struct v4l2_events *events = fh->events;
+
+	if (!events)
+		return;
+
+	list_kfree(&events->free, struct v4l2_kevent, list);
+	list_kfree(&events->available, struct v4l2_kevent, list);
+	list_kfree(&events->subscribed, struct v4l2_subscribed_event, list);
+
+	kfree(events);
+	fh->events = NULL;
+}
+EXPORT_SYMBOL_GPL(v4l2_event_exit);
+
+int v4l2_event_init(struct v4l2_fh *fh, unsigned int n)
+{
+	int ret;
+
+	fh->events = kzalloc(sizeof(*fh->events), GFP_KERNEL);
+	if (fh->events == NULL)
+		return -ENOMEM;
+
+	init_waitqueue_head(&fh->events->wait);
+
+	INIT_LIST_HEAD(&fh->events->free);
+	INIT_LIST_HEAD(&fh->events->available);
+	INIT_LIST_HEAD(&fh->events->subscribed);
+
+	ret = v4l2_event_alloc(fh, n);
+	if (ret < 0)
+		v4l2_event_exit(fh);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(v4l2_event_init);
+
+int v4l2_event_dequeue(struct v4l2_fh *fh, struct v4l2_event *event)
+{
+	struct v4l2_events *events = fh->events;
+	struct v4l2_kevent *kev;
+	unsigned long flags;
+
+	spin_lock_irqsave(&fh->vdev->fh_lock, flags);
+
+	if (list_empty(&events->available)) {
+		spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
+		return -ENOENT;
+	}
+
+	kev = list_first_entry(&events->available, struct v4l2_kevent, list);
+	list_move(&kev->list, &events->free);
+
+	kev->event.count = !list_empty(&events->available);
+
+	*event = kev->event;
+
+	spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(v4l2_event_dequeue);
+
+static struct v4l2_subscribed_event *__v4l2_event_subscribed(
+	struct v4l2_fh *fh, u32 type)
+{
+	struct v4l2_events *events = fh->events;
+	struct v4l2_subscribed_event *sev;
+
+	list_for_each_entry(sev, &events->subscribed, list) {
+		if (sev->type == type)
+			return sev;
+	}
+
+	return NULL;
+}
+
+struct v4l2_subscribed_event *v4l2_event_subscribed(
+	struct v4l2_fh *fh, u32 type)
+{
+	struct v4l2_subscribed_event *sev;
+	unsigned long flags;

drivers/media/video/v4l2-event.c new file +5 -0

+
+	spin_lock_irqsave(&vdev->fh_lock, flags);
+
+	list_for_each_entry(fh, &vdev->fh_list, list) {
+		struct v4l2_events *events = fh->events;

drivers/media/video/v4l2-event.c new file +27 -0

+		list_move_tail(&kev->list, &events->available);
+
+		wake_up_all(&events->wait);
+	}
+
+	spin_unlock_irqrestore(&vdev->fh_lock, flags);
+}
+EXPORT_SYMBOL_GPL(v4l2_event_queue);
+
+int v4l2_event_pending(struct v4l2_fh *fh)
+{
+	struct v4l2_events *events = fh->events;
+	unsigned long flags;
+	int ret;
+
+	spin_lock_irqsave(&fh->vdev->fh_lock, flags);
+	ret = !list_empty(&events->available);
+	spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(v4l2_event_pending);
+
+int v4l2_event_subscribe(struct v4l2_fh *fh,
+			 struct v4l2_event_subscription *sub)
+{
+	struct v4l2_events *events = fh->events;

drivers/media/video/v4l2-event.c new file +14 -0

+		goto out;
+	}
+
+	INIT_LIST_HEAD(&sev->list);
+	sev->type = sub->type;
+
+	list_add(&sev->list, &events->subscribed);
+
+out:
+	spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
+
+	if (ret)
+		kfree(sev);
+

drivers/media/video/v4l2-event.c new file +6 -0

+
+	spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(v4l2_event_unsubscribe);

drivers/media/video/v4l2-fh.c +4 -0

index 3c1cea2..7c13f5b 100644
@@ -24,6 +24,7 @@
 
 #include <media/v4l2-dev.h>
 #include <media/v4l2-fh.h>
+#include <media/v4l2-event.h>
 
 void v4l2_fh_init(struct v4l2_fh *fh, struct video_device *vdev)
 {

@@ -54,6 +55,9 @@ EXPORT_SYMBOL_GPL(v4l2_fh_del);
 void v4l2_fh_exit(struct v4l2_fh *fh)
 {
 	BUG_ON(fh->vdev == NULL);
+
+	v4l2_event_exit(fh);
+
 	fh->vdev = NULL;
 }
 EXPORT_SYMBOL_GPL(v4l2_fh_exit);

include/media/v4l2-event.h new file +64 -0

new file mode 100644
index 0000000..580c9d4
@@ -0,0 +1,64 @@
+/*
+ * include/media/v4l2-event.h
+ *
+ * V4L2 events.
+ *
+ * Copyright (C) 2009 Nokia Corporation.
+ *
+ * Contact: Sakari Ailus <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ */
+
+#ifndef V4L2_EVENT_H
+#define V4L2_EVENT_H
+
+#include <linux/types.h>
+#include <linux/videodev2.h>
+
+struct v4l2_fh;
+struct video_device;
+
+struct v4l2_kevent {
+	struct list_head	list;
+	struct v4l2_event	event;
+};
+
+struct v4l2_subscribed_event {
+	struct list_head	list;
+	u32			type;
+};
+
+struct v4l2_events {
+	wait_queue_head_t	wait;
+	struct list_head	subscribed; /* Subscribed events */
+	struct list_head	available; /* Dequeueable event */
+	struct list_head	free; /* Events ready for use */
+};
+
+int v4l2_event_alloc(struct v4l2_fh *fh, unsigned int n);
+int v4l2_event_init(struct v4l2_fh *fh, unsigned int n);
+void v4l2_event_exit(struct v4l2_fh *fh);
+int v4l2_event_dequeue(struct v4l2_fh *fh, struct v4l2_event *event);
+struct v4l2_subscribed_event *v4l2_event_subscribed(
+	struct v4l2_fh *fh, u32 type);
+void v4l2_event_queue(struct video_device *vdev, 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);
+int v4l2_event_unsubscribe(struct v4l2_fh *fh,
+			   struct v4l2_event_subscription *sub);
+
+#endif /* V4L2_EVENT_H */

include/media/v4l2-fh.h +2 -0

index 2e88031..6d03a1e 100644
@@ -31,10 +31,12 @@
 #include <asm/atomic.h>
 
 struct video_device;
+struct v4l2_events;
 
 struct v4l2_fh {
 	struct list_head	list;
 	struct video_device	*vdev;
+	struct v4l2_events      *events; /* events, pending and subscribed */
 };
 
 void v4l2_fh_init(struct v4l2_fh *fh, struct video_device *vdev);

This patch adds a set of new ioctls to the V4L2 API. The ioctls conform to V4L2 Events RFC version 2.3:

<URL:http://www.spinics.net/lists/linux-media/msg12033.html>

Signed-off-by Sakari Ailus <[email protected]>

---
 drivers/media/video/v4l2-compat-ioctl32.c |    3 +++
 drivers/media/video/v4l2-ioctl.c          |    3 +++
 include/linux/videodev2.h                 |   23 +++++++++++++++++++++++
 3 files changed, 29 insertions(+), 0 deletions(-)

drivers/media/video/v4l2-compat-ioctl32.c +3 -0

index 997975d..cba704c 100644
@@ -1077,6 +1077,9 @@ long v4l2_compat_ioctl32(struct file *file, unsigned int cmd, unsigned long arg)
 	case VIDIOC_DBG_G_REGISTER:
 	case VIDIOC_DBG_G_CHIP_IDENT:
 	case VIDIOC_S_HW_FREQ_SEEK:
+	case VIDIOC_DQEVENT:
+	case VIDIOC_SUBSCRIBE_EVENT:
+	case VIDIOC_UNSUBSCRIBE_EVENT:
 		ret = do_video_ioctl(file, cmd, arg);
 		break;
 

drivers/media/video/v4l2-ioctl.c +3 -0

index 30cc334..bfc4696 100644
@@ -283,6 +283,9 @@ static const char *v4l2_ioctls[] = {
 
 	[_IOC_NR(VIDIOC_DBG_G_CHIP_IDENT)] = "VIDIOC_DBG_G_CHIP_IDENT",
 	[_IOC_NR(VIDIOC_S_HW_FREQ_SEEK)]   = "VIDIOC_S_HW_FREQ_SEEK",
+	[_IOC_NR(VIDIOC_DQEVENT)]	   = "VIDIOC_DQEVENT",
+	[_IOC_NR(VIDIOC_SUBSCRIBE_EVENT)]  = "VIDIOC_SUBSCRIBE_EVENT",
+	[_IOC_NR(VIDIOC_UNSUBSCRIBE_EVENT)] = "VIDIOC_UNSUBSCRIBE_EVENT",
 #endif
 };
 #define V4L2_IOCTLS ARRAY_SIZE(v4l2_ioctls)

include/linux/videodev2.h +0 -0

index 54af357..a19ae89 100644

include/linux/videodev2.h +12 -0

+
+struct v4l2_event_subscription {
+	__u32		type;
+	__u32		reserved[7];
+};
+
+#define V4L2_EVENT_PRIVATE_START		0x08000000
+
+/*
  *	A D V A N C E D   D E B U G G I N G
  *
  *	NOTE: EXPERIMENTAL API, NEVER RELY ON THIS IN APPLICATIONS!

@@ -1651,6 +1671,9 @@ struct v4l2_dbg_chip_ident {
 #endif
 
 #define VIDIOC_S_HW_FREQ_SEEK	 _IOW('V', 82, struct v4l2_hw_freq_seek)
+#define VIDIOC_DQEVENT		 _IOR('V', 83, struct v4l2_event)
+#define VIDIOC_SUBSCRIBE_EVENT	 _IOW('V', 84, struct v4l2_event_subscription)
+#define VIDIOC_UNSUBSCRIBE_EVENT _IOW('V', 85, struct v4l2_event_subscription)
 /* Reminder: when adding new ioctls please add support for them to
    drivers/media/video/v4l2-compat-ioctl32.c as well! */
 

Quoted 2 lines · On Wednesday 10 February 2010 15:58:04 Sakari Ailus wrote:

This patch adds a set of new ioctls to the V4L2 API. The ioctls conform to V4L2 Events RFC version 2.3:

I've experimented with the events API to try and support it with ivtv and I realized that it had some problems.

See comments below.

Quoted 51 lines

<URL:http://www.spinics.net/lists/linux-media/msg12033.html>

Signed-off-by Sakari Ailus <[email protected]>

---

 drivers/media/video/v4l2-compat-ioctl32.c |    3 +++
 drivers/media/video/v4l2-ioctl.c          |    3 +++
 include/linux/videodev2.h                 |   23 +++++++++++++++++++++++
 3 files changed, 29 insertions(+), 0 deletions(-)

(quoted diff) +0 -0

b/drivers/media/video/v4l2-compat-ioctl32.c index 997975d..cba704c 100644

drivers/media/video/v4l2-compat-ioctl32.c +3 -0

@@ -1077,6 +1077,9 @@ long v4l2_compat_ioctl32(struct file *file,
 unsigned int cmd, unsigned long arg)
 
 	case VIDIOC_DBG_G_REGISTER:
 	case VIDIOC_DBG_G_CHIP_IDENT:
 
 	case VIDIOC_S_HW_FREQ_SEEK:
+	case VIDIOC_DQEVENT:
+	case VIDIOC_SUBSCRIBE_EVENT:
 
+	case VIDIOC_UNSUBSCRIBE_EVENT:
 		ret = do_video_ioctl(file, cmd, arg);
 		break;

(quoted diff) +0 -0

b/drivers/media/video/v4l2-ioctl.c index 30cc334..bfc4696 100644

drivers/media/video/v4l2-ioctl.c +3 -0

@@ -283,6 +283,9 @@ static const char *v4l2_ioctls[] = {
 
 	[_IOC_NR(VIDIOC_DBG_G_CHIP_IDENT)] = "VIDIOC_DBG_G_CHIP_IDENT",
 	[_IOC_NR(VIDIOC_S_HW_FREQ_SEEK)]   = "VIDIOC_S_HW_FREQ_SEEK",
 
+	[_IOC_NR(VIDIOC_DQEVENT)]	   = "VIDIOC_DQEVENT",
+	[_IOC_NR(VIDIOC_SUBSCRIBE_EVENT)]  = "VIDIOC_SUBSCRIBE_EVENT",
+	[_IOC_NR(VIDIOC_UNSUBSCRIBE_EVENT)] = "VIDIOC_UNSUBSCRIBE_EVENT",
 
 #endif

};
#define V4L2_IOCTLS ARRAY_SIZE(v4l2_ioctls)

include/linux/videodev2.h +0 -0

index 54af357..a19ae89 100644

include/linux/videodev2.h struct v4l2_streamparm { · line 1545

Quoted 10 lines

include/linux/videodev2.h +5 -0

@@ -1536,6 +1536,26 @@ struct v4l2_streamparm {
 
 };
 
 /*
 
+ *	E V E N T S
+ */
+
+struct v4l2_event {
+	__u32		count;

The name 'count' is confusing. Count of what? I think the name 'pending' might be more understandable. A comment after the definition would also help.

include/linux/videodev2.h struct v4l2_streamparm { · line 1549

Quoted 6 lines

include/linux/videodev2.h +6 -0

+	__u32		type;
+	__u32		sequence;
+	struct timespec	timestamp;
+	__u32		reserved[9];
+	__u8		data[64];
+};

I also think we should reorder the fields and add a union. For ivtv I would need this:

#define V4L2_EVENT_ALL                          0
#define V4L2_EVENT_VSYNC                        1
#define V4L2_EVENT_EOS                          2
#define V4L2_EVENT_PRIVATE_START                0x08000000

/* Payload for V4L2_EVENT_VSYNC */

struct v4l2_event_vsync {
        /* Can be V4L2_FIELD_ANY, _NONE, _TOP or _BOTTOM */
        u8 field;
} __attribute__ ((packed));

struct v4l2_event {
        __u32           type;
        union {
                struct v4l2_event_vsync vsync;
                __u8    data[64];
        } u;
        __u32           sequence;
        struct timespec timestamp;
        __u32           pending;
        __u32           reserved[9];
};

The reason for rearranging the fields has to do with the fact that the first two fields (type and the union) form the actual event data. The others are more for administrative purposes. Separating those two makes sense to me.

So when I define an event for queuing it is nice if I can do just this:

static const struct v4l2_event ev_top = {
	.type = V4L2_EVENT_VSYNC,
	.u.vsync.field = V4L2_FIELD_TOP,
};

I would have preferred to have an anonymous union. Unfortunately gcc has problems with initializers for fields inside an anonymous union. Hence the need for a named union.

Hi Hans,

Quoted 123 lines · On Saturday 13 February 2010 14:19:55 Hans Verkuil wrote:

Quoted 1 line · On Wednesday 10 February 2010 15:58:04 Sakari Ailus wrote:

This patch adds a set of new ioctls to the V4L2 API. The ioctls

conform

Quoted 1 line

to V4L2 Events RFC version 2.3:

I've experimented with the events API to try and support it with ivtv and
I realized that it had some problems.

See comments below.

Quoted 8 lines

<URL:http://www.spinics.net/lists/linux-media/msg12033.html>

Signed-off-by Sakari Ailus <[email protected]>

---

 drivers/media/video/v4l2-compat-ioctl32.c |    3 +++
 drivers/media/video/v4l2-ioctl.c          |    3 +++
 include/linux/videodev2.h                 |   23

+++++++++++++++++++++++

Quoted 43 lines

 3 files changed, 29 insertions(+), 0 deletions(-)

(quoted diff) +0 -0

b/drivers/media/video/v4l2-compat-ioctl32.c index 997975d..cba704c 100644

drivers/media/video/v4l2-compat-ioctl32.c +3 -0

@@ -1077,6 +1077,9 @@ long v4l2_compat_ioctl32(struct file *file,
 unsigned int cmd, unsigned long arg)
 
 	case VIDIOC_DBG_G_REGISTER:
 	case VIDIOC_DBG_G_CHIP_IDENT:
 
 	case VIDIOC_S_HW_FREQ_SEEK:
+	case VIDIOC_DQEVENT:
+	case VIDIOC_SUBSCRIBE_EVENT:
 
+	case VIDIOC_UNSUBSCRIBE_EVENT:
 		ret = do_video_ioctl(file, cmd, arg);
 		break;

(quoted diff) +0 -0

b/drivers/media/video/v4l2-ioctl.c index 30cc334..bfc4696 100644

drivers/media/video/v4l2-ioctl.c +3 -0

@@ -283,6 +283,9 @@ static const char *v4l2_ioctls[] = {
 
 	[_IOC_NR(VIDIOC_DBG_G_CHIP_IDENT)] = "VIDIOC_DBG_G_CHIP_IDENT",
 	[_IOC_NR(VIDIOC_S_HW_FREQ_SEEK)]   = "VIDIOC_S_HW_FREQ_SEEK",
 
+	[_IOC_NR(VIDIOC_DQEVENT)]	   = "VIDIOC_DQEVENT",
+	[_IOC_NR(VIDIOC_SUBSCRIBE_EVENT)]  = "VIDIOC_SUBSCRIBE_EVENT",
+	[_IOC_NR(VIDIOC_UNSUBSCRIBE_EVENT)] = "VIDIOC_UNSUBSCRIBE_EVENT",
 
 #endif

};
#define V4L2_IOCTLS ARRAY_SIZE(v4l2_ioctls)

include/linux/videodev2.h +0 -0

index 54af357..a19ae89 100644

include/linux/videodev2.h struct v4l2_streamparm { · line 1545

Quoted 10 lines

include/linux/videodev2.h +5 -0

@@ -1536,6 +1536,26 @@ struct v4l2_streamparm {
 
 };
 
 /*
 
+ *	E V E N T S
+ */
+
+struct v4l2_event {
+	__u32		count;

The name 'count' is confusing. Count of what? I think the name 'pending' might be more understandable. A comment after the definition would also help.

include/linux/videodev2.h struct v4l2_streamparm { · line 1549

Quoted 6 lines

include/linux/videodev2.h +6 -0

+	__u32		type;
+	__u32		sequence;
+	struct timespec	timestamp;
+	__u32		reserved[9];
+	__u8		data[64];
+};

I also think we should reorder the fields and add a union. For ivtv I would
need this:

#define V4L2_EVENT_ALL                          0
#define V4L2_EVENT_VSYNC                        1
#define V4L2_EVENT_EOS                          2
#define V4L2_EVENT_PRIVATE_START                0x08000000

/* Payload for V4L2_EVENT_VSYNC */

struct v4l2_event_vsync {
        /* Can be V4L2_FIELD_ANY, _NONE, _TOP or _BOTTOM */
        u8 field;
} __attribute__ ((packed));

struct v4l2_event {
        __u32           type;
        union {
                struct v4l2_event_vsync vsync;
                __u8    data[64];
        } u;
        __u32           sequence;
        struct timespec timestamp;
        __u32           pending;
        __u32           reserved[9];
};

The reason for rearranging the fields has to do with the fact that the first two fields (type and the union) form the actual event data. The others are more for administrative purposes. Separating those two makes sense to me.

So when I define an event for queuing it is nice if I can do just this:

static const struct v4l2_event ev_top = {
	.type = V4L2_EVENT_VSYNC,
	.u.vsync.field = V4L2_FIELD_TOP,
};

Quoted 4 lines

I would have preferred to have an anonymous union. Unfortunately gcc has problems with initializers for fields inside an anonymous union. Hence the
need for a named union.

Will all drivers add private events to the union ? This would then soon become
a mess. Wouldn't it be better for drivers to define their own event structures
(standard ones could be shared between drivers in videodev2.h) and cast the
pointer to data to a pointer to the appropriate event structure ?

--
Regards,

Laurent Pinchart

Quoted 142 lines

Hi Hans,

Quoted 130 lines · On Saturday 13 February 2010 14:19:55 Hans Verkuil wrote:

Quoted 1 line · On Wednesday 10 February 2010 15:58:04 Sakari Ailus wrote:

This patch adds a set of new ioctls to the V4L2 API. The ioctls

conform

Quoted 1 line

to V4L2 Events RFC version 2.3:

I've experimented with the events API to try and support it with ivtv and
I realized that it had some problems.

See comments below.

Quoted 8 lines

<URL:http://www.spinics.net/lists/linux-media/msg12033.html>

Signed-off-by Sakari Ailus <[email protected]>

---

 drivers/media/video/v4l2-compat-ioctl32.c |    3 +++
 drivers/media/video/v4l2-ioctl.c          |    3 +++
 include/linux/videodev2.h                 |   23

+++++++++++++++++++++++

Quoted 43 lines

 3 files changed, 29 insertions(+), 0 deletions(-)

(quoted diff) +0 -0

b/drivers/media/video/v4l2-compat-ioctl32.c index 997975d..cba704c 100644

drivers/media/video/v4l2-compat-ioctl32.c +3 -0

@@ -1077,6 +1077,9 @@ long v4l2_compat_ioctl32(struct file *file,
 unsigned int cmd, unsigned long arg)
 
 	case VIDIOC_DBG_G_REGISTER:
 	case VIDIOC_DBG_G_CHIP_IDENT:
 
 	case VIDIOC_S_HW_FREQ_SEEK:
+	case VIDIOC_DQEVENT:
+	case VIDIOC_SUBSCRIBE_EVENT:
 
+	case VIDIOC_UNSUBSCRIBE_EVENT:
 		ret = do_video_ioctl(file, cmd, arg);
 		break;

(quoted diff) +0 -0

b/drivers/media/video/v4l2-ioctl.c index 30cc334..bfc4696 100644

drivers/media/video/v4l2-ioctl.c +3 -0

@@ -283,6 +283,9 @@ static const char *v4l2_ioctls[] = {
 
 	[_IOC_NR(VIDIOC_DBG_G_CHIP_IDENT)] = "VIDIOC_DBG_G_CHIP_IDENT",
 	[_IOC_NR(VIDIOC_S_HW_FREQ_SEEK)]   = "VIDIOC_S_HW_FREQ_SEEK",
 
+	[_IOC_NR(VIDIOC_DQEVENT)]	   = "VIDIOC_DQEVENT",
+	[_IOC_NR(VIDIOC_SUBSCRIBE_EVENT)]  = "VIDIOC_SUBSCRIBE_EVENT",
+	[_IOC_NR(VIDIOC_UNSUBSCRIBE_EVENT)] = "VIDIOC_UNSUBSCRIBE_EVENT",
 
 #endif

};
#define V4L2_IOCTLS ARRAY_SIZE(v4l2_ioctls)

include/linux/videodev2.h +0 -0

index 54af357..a19ae89 100644

include/linux/videodev2.h struct v4l2_streamparm { · line 1545

Quoted 10 lines

include/linux/videodev2.h +5 -0

@@ -1536,6 +1536,26 @@ struct v4l2_streamparm {
 
 };
 
 /*
 
+ *	E V E N T S
+ */
+
+struct v4l2_event {
+	__u32		count;

The name 'count' is confusing. Count of what? I think the name 'pending' might be more understandable. A comment after the definition would also help.

include/linux/videodev2.h struct v4l2_streamparm { · line 1549

Quoted 6 lines

include/linux/videodev2.h +6 -0

+	__u32		type;
+	__u32		sequence;
+	struct timespec	timestamp;
+	__u32		reserved[9];
+	__u8		data[64];
+};

I also think we should reorder the fields and add a union. For ivtv I would
need this:

#define V4L2_EVENT_ALL                          0
#define V4L2_EVENT_VSYNC                        1
#define V4L2_EVENT_EOS                          2
#define V4L2_EVENT_PRIVATE_START                0x08000000

/* Payload for V4L2_EVENT_VSYNC */
struct v4l2_event_vsync {

        /* Can be V4L2_FIELD_ANY, _NONE, _TOP or _BOTTOM */
        u8 field;

} __attribute__ ((packed));

struct v4l2_event {

        __u32           type;
        union {
        
                struct v4l2_event_vsync vsync;
                __u8    data[64];
        
        } u;
        __u32           sequence;
        struct timespec timestamp;
        __u32           pending;
        __u32           reserved[9];

};

The reason for rearranging the fields has to do with the fact that the first two fields (type and the union) form the actual event data. The others are more for administrative purposes. Separating those two makes sense to me.

So when I define an event for queuing it is nice if I can do just this:

static const struct v4l2_event ev_top = {

Quoted 8 lines

	.type = V4L2_EVENT_VSYNC,
	.u.vsync.field = V4L2_FIELD_TOP,

};

I would have preferred to have an anonymous union. Unfortunately gcc has problems with initializers for fields inside an anonymous union. Hence the need for a named union.

Quoted 5 lines

Will all drivers add private events to the union ? This would then soon become a mess. Wouldn't it be better for drivers to define their own event structures (standard ones could be shared between drivers in videodev2.h) and cast the pointer to data to a pointer to the appropriate event structure ?

I would prefer to have the actual event type defines in videodev2.h (just as we do for private control IDs). The actual payload structure can be defined elsewhere as far as I am concerned.

An alternative in the long run might be to split off the event structs into a separate public header. I have been thinking along those lines as well for the controls. videodev2.h is getting pretty huge and it might be more managable if it is split up in multiple parts with videodev2.h including those parts.

This patch adds a list of v4l2_fh structures to every video_device. It allows using file handle related information in V4L2. The event interface is one example of such use.

Video device drivers should use the v4l2_fh pointer as their file->private_data.

Signed-off-by Sakari Ailus <[email protected]>

---
 drivers/media/video/Makefile   |    3 +-
 drivers/media/video/v4l2-dev.c |    2 +
 drivers/media/video/v4l2-fh.c  |   65 ++++++++++++++++++++++++++++++++++++++++
 include/media/v4l2-dev.h       |    6 ++++
 include/media/v4l2-fh.h        |   47 +++++++++++++++++++++++++++++
 5 files changed, 122 insertions(+), 1 deletions(-)
 create mode 100644 drivers/media/video/v4l2-fh.c
 create mode 100644 include/media/v4l2-fh.h

drivers/media/video/Makefile +2 -1

index 6e75647..b888ad1 100644
@@ -10,7 +10,8 @@ stkwebcam-objs	:=	stk-webcam.o stk-sensor.o
 
 omap2cam-objs	:=	omap24xxcam.o omap24xxcam-dma.o
 
-videodev-objs	:=	v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-subdev.o
+videodev-objs	:=	v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-subdev.o \
+			v4l2-fh.o
 
 # V4L2 core modules
 

drivers/media/video/v4l2-dev.c +2 -0

index 13a899d..c24c832 100644
@@ -423,6 +423,8 @@ static int __video_register_device(struct video_device *vdev, int type, int nr,
 	if (!vdev->release)
 		return -EINVAL;
 
+	v4l2_fhs_init(vdev);
+
 	/* Part 1: check device type */
 	switch (type) {
 	case VFL_TYPE_GRABBER:

drivers/media/video/v4l2-fh.c new file +55 -0

new file mode 100644
index 0000000..3c1cea2
@@ -0,0 +1,65 @@
+/*
+ * drivers/media/video/v4l2-fh.c
+ *
+ * V4L2 file handles.
+ *
+ * Copyright (C) 2009 Nokia Corporation.
+ *
+ * Contact: Sakari Ailus <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ */
+
+#include <media/v4l2-dev.h>
+#include <media/v4l2-fh.h>
+
+void v4l2_fh_init(struct v4l2_fh *fh, struct video_device *vdev)
+{
+	fh->vdev = vdev;
+}
+EXPORT_SYMBOL_GPL(v4l2_fh_init);
+
+void v4l2_fh_add(struct v4l2_fh *fh)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&fh->vdev->fh_lock, flags);
+	list_add(&fh->list, &fh->vdev->fh_list);
+	spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
+}
+EXPORT_SYMBOL_GPL(v4l2_fh_add);
+
+void v4l2_fh_del(struct v4l2_fh *fh)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&fh->vdev->fh_lock, flags);
+	list_del(&fh->list);
+	spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
+}
+EXPORT_SYMBOL_GPL(v4l2_fh_del);
+
+void v4l2_fh_exit(struct v4l2_fh *fh)
+{

include/media/v4l2-dev.h +6 -0

index 26d4e79..ee3a0c9 100644
@@ -18,6 +18,8 @@
 
 #include <media/media-entity.h>
 
+#include <media/v4l2-fh.h>
+
 #define VIDEO_MAJOR	81
 
 #define VFL_TYPE_GRABBER	0

@@ -82,6 +84,10 @@ struct video_device
 	/* attribute to differentiate multiple indices on one physical device */
 	int index;
 
+	/* V4L2 file handles */
+	spinlock_t		fh_lock; /* Lock for all v4l2_fhs */
+	struct list_head	fh_list; /* List of struct v4l2_fh */
+
 	int debug;			/* Activates debug level*/
 
 	/* Video standard vars */

include/media/v4l2-fh.h new file +47 -0

new file mode 100644
index 0000000..2e88031
@@ -0,0 +1,47 @@
+/*
+ * include/media/v4l2-fh.h
+ *
+ * V4L2 file handle.
+ *
+ * Copyright (C) 2009 Nokia Corporation.
+ *
+ * Contact: Sakari Ailus <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ */
+
+#ifndef V4L2_FH_H
+#define V4L2_FH_H
+
+#include <linux/types.h>
+#include <linux/list.h>
+
+#include <asm/atomic.h>
+
+struct video_device;
+
+struct v4l2_fh {
+	struct list_head	list;
+	struct video_device	*vdev;
+};
+
+void v4l2_fh_init(struct v4l2_fh *fh, struct video_device *vdev);
+void v4l2_fh_add(struct v4l2_fh *fh);
+void v4l2_fh_del(struct v4l2_fh *fh);
+void v4l2_fh_exit(struct v4l2_fh *fh);
+
+void v4l2_fhs_init(struct video_device *vdev);
+
+#endif /* V4L2_EVENT_H */