V4L2 file handles and event interface

-----Original Message-----
From: [email protected] [mailto:linux-media- [email protected]] On Behalf Of Sakari Ailus
Sent: Friday, February 19, 2010 1:22 PM
To: [email protected]
Cc: [email protected]; [email protected]; iivanov@mm- sol.com; [email protected]; [email protected]; Sakari Ailus
Subject: [PATCH v5 1/6] V4L: File handles

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   |    2 +-
 drivers/media/video/v4l2-dev.c |    4 ++
 drivers/media/video/v4l2-fh.c  |   64
++++++++++++++++++++++++++++++++++++++++
 include/media/v4l2-dev.h       |    5 +++
 include/media/v4l2-fh.h        |   42 ++++++++++++++++++++++++++
 5 files changed, 116 insertions(+), 1 deletions(-)
 create mode 100644 drivers/media/video/v4l2-fh.c
 create mode 100644 include/media/v4l2-fh.h

drivers/media/video/Makefile +1 -1

index 5163289..14bf69a 100644
@@ -10,7 +10,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
+videodev-objs	:=	v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o
 
 # V4L2 core modules
 

drivers/media/video/v4l2- renamed from drivers/media/video/v4l2-dev.c +0 -0

dev.c
index 7090699..65a7b30 100644

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

@@ -421,6 +421,10 @@ static int __video_register_device(struct
 video_device *vdev, int type, int nr,
 	if (!vdev->release)
 		return -EINVAL;
 
+	/* v4l2_fh support */
+	spin_lock_init(&vdev->fh_lock);
+	INIT_LIST_HEAD(&vdev->fh_list);
+
 	/* Part 1: check device type */
 	switch (type) {

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

 	case VFL_TYPE_GRABBER:

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

new file mode 100644
index 0000000..c707930

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

+ *
+ * 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 <linux/bitops.h>
+#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;
+	INIT_LIST_HEAD(&fh->list);
+	set_bit(V4L2_FL_USES_V4L2_FH, &fh->vdev->flags);
+}
+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_init(&fh->list);
+	spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
+}
+EXPORT_SYMBOL_GPL(v4l2_fh_del);
+
+void v4l2_fh_exit(struct v4l2_fh *fh)
+{
+	if (fh->vdev == NULL)
+		return;
+
+	fh->vdev = NULL;
+}
+EXPORT_SYMBOL_GPL(v4l2_fh_exit);

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

index 2dee938..bebe44b 100644
@@ -32,6 +32,7 @@ struct v4l2_device;
    Drivers can clear this flag if they want to block all future
    device access. It is cleared by video_unregister_device. */
 #define V4L2_FL_REGISTERED	(0)
+#define V4L2_FL_USES_V4L2_FH	(1)
 
 struct v4l2_file_operations {
 	struct module *owner;

@@ -77,6 +78,10 @@ struct video_device
 	/* attribute to differentiate multiple indices on one physical

device */

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

 	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 +0 -0

new file mode 100644
index 0000000..6b486aa

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

+ *
+ * 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.
+ *

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

+
+struct video_device;
+
+struct v4l2_fh {

--
1.5.6.5

--
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

Heippa!

Quoted 31 lines

-----Original Message-----
From: [email protected] [mailto:linux-media- [email protected]] On Behalf Of Sakari Ailus
Sent: Friday, February 19, 2010 1:22 PM
To: [email protected]
Cc: [email protected]; [email protected]; iivanov@mm- sol.com; [email protected]; [email protected]; Sakari Ailus
Subject: [PATCH v5 1/6] V4L: File handles

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   |    2 +-
 drivers/media/video/v4l2-dev.c |    4 ++
 drivers/media/video/v4l2-fh.c  |   64

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

 include/media/v4l2-dev.h       |    5 +++
 include/media/v4l2-fh.h        |   42 ++++++++++++++++++++++++++
 5 files changed, 116 insertions(+), 1 deletions(-)
 create mode 100644 drivers/media/video/v4l2-fh.c
 create mode 100644 include/media/v4l2-fh.h

drivers/media/video/v4l2-fh.c line 2

Quoted 8 lines

(quoted diff) +0 -0

b/drivers/media/video/v4l2-fh.c new file mode 100644 index 0000000..c707930

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

@@ -0,0 +1,64 @@
+/*
+ * drivers/media/video/v4l2-fh.c

[1] AFAIK, putting file paths is frowned upon.

Makes maintenance harder if in the future, this files get moved somewhere else.

drivers/media/video/v4l2-fh.c line 6

Quoted 4 lines

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

+ *
+ * V4L2 file handles.
+ *
+ * Copyright (C) 2009 Nokia Corporation.

[2] Shouldn't it be "(C) 2010" already? :)

include/media/v4l2-fh.h line 2

Quoted 8 lines

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

new file mode 100644
index 0000000..6b486aa
@@ -0,0 +1,42 @@
+/*
+ * include/media/v4l2-fh.h

Same as [1]

include/media/v4l2-fh.h line 6

Quoted 4 lines

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

+ *
+ * V4L2 file handle.
+ *
+ * Copyright (C) 2009 Nokia Corporation.

Same as [2]

Quoted 12 lines

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

+ *
+ * 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.
+ *

include/media/v4l2-fh.h line 28

Quoted 10 lines

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

+ * 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/list.h>

Shouldn't you add one more header here?:

#include <media/v4l2-dev.h>

(for struct video_device)

Quoted 14 lines

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

+
+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);
+
+#endif /* V4L2_EVENT_H */

-----Original Message-----
From: Laurent Pinchart [mailto:[email protected]]
Sent: Friday, February 19, 2010 4:35 PM
To: Aguirre, Sergio
Cc: Sakari Ailus; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH v5 1/6] V4L: File handles

Hi Sergio,

Quoted 9 lines · Sergio

Heippa!

Quoted 7 lines

-----Original Message-----
From: [email protected] [mailto:linux-media- [email protected]] On Behalf Of Sakari Ailus
Sent: Friday, February 19, 2010 1:22 PM
To: [email protected]
Cc: [email protected]; [email protected]; iivanov@mm- sol.com; [email protected]; [email protected]; Sakari

Ailus

Quoted 24 lines

Quoted 24 lines

Subject: [PATCH v5 1/6] V4L: File handles

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   |    2 +-
 drivers/media/video/v4l2-dev.c |    4 ++
 drivers/media/video/v4l2-fh.c  |   64

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

 include/media/v4l2-dev.h       |    5 +++
 include/media/v4l2-fh.h        |   42 ++++++++++++++++++++++++++
 5 files changed, 116 insertions(+), 1 deletions(-)
 create mode 100644 drivers/media/video/v4l2-fh.c
 create mode 100644 include/media/v4l2-fh.h

[snip]

Quoted 11 lines

drivers/media/video/v4l2-fh.c line 2

Quoted 8 lines

(quoted diff) +0 -0

b/drivers/media/video/v4l2-fh.c new file mode 100644 index 0000000..c707930

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

@@ -0,0 +1,64 @@
+/*
+ * drivers/media/video/v4l2-fh.c

[1] AFAIK, putting file paths is frowned upon.

Quoted 1 line

Makes maintenance harder if in the future, this files get moved

somewhere

Quoted 2 lines

else.

Quoted 6 lines

drivers/media/video/v4l2-fh.c line 6

Quoted 4 lines

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

+ *
+ * V4L2 file handles.
+ *
+ * Copyright (C) 2009 Nokia Corporation.

[2] Shouldn't it be "(C) 2010" already? :)

That shows how long the V4L2 events API review is taking ;-)

[snip]

Quoted 11 lines

include/media/v4l2-fh.h line 2

Quoted 8 lines

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

new file mode 100644
index 0000000..6b486aa
@@ -0,0 +1,42 @@
+/*
+ * include/media/v4l2-fh.h

Same as [1]

Quoted 15 lines

include/media/v4l2-fh.h line 6

Quoted 4 lines

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

+ *
+ * V4L2 file handle.
+ *
+ * Copyright (C) 2009 Nokia Corporation.

Same as [2]

Quoted 8 lines

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

+ *
+ * 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

Quoted 17 lines

Quoted 4 lines

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

+ * 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.
+ *

include/media/v4l2-fh.h line 28

Quoted 10 lines

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

+ * 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/list.h>

Shouldn't you add one more header here?:

Quoted 3 lines

#include <media/v4l2-dev.h>

(for struct video_device)

This header only needs struct video_device *, not struct video_device, so adding a forward definition will be more efficient (lower compilation time for
compilation units that include v4l2-fh.h but not v4l2-dev.h).

Quoted 15 lines

Quoted 5 lines

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

+
+struct video_device;
+
+struct v4l2_fh {

include/media/v4l2-fh.h line 42

Quoted 10 lines

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

+	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);
+
+#endif /* V4L2_EVENT_H */

--
Regards,

Laurent Pinchart

Heippa!

drivers/media/video/v4l2-fh.c line 2

Quoted 3 lines

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

@@ -0,0 +1,64 @@
+/*
+ * drivers/media/video/v4l2-fh.c

[1] AFAIK, putting file paths is frowned upon.

Makes maintenance harder if in the future, this files get moved somewhere else.

drivers/media/video/v4l2-fh.c line 6

Quoted 4 lines

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

+ *
+ * V4L2 file handles.
+ *
+ * Copyright (C) 2009 Nokia Corporation.

[2] Shouldn't it be "(C) 2010" already? :)

include/media/v4l2-fh.h line 42

Quoted 1 line

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

+#endif /* V4L2_EVENT_H */

Wrong comment, must have been:

	/* V4L2_FH_H */

Will fix.

--
Sakari Ailus
[email protected]

Hi Sakari,

From: Sakari Ailus [mailto:[email protected]]

...

Quoted 1 line

Will fix.

Seems that you missed to fix this patch comments on your v6 version...

:-/

Add documentation on using V4L2 file handles (v4l2_fh) in V4L2 drivers.

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

---
 Documentation/video4linux/v4l2-framework.txt |   36 ++++++++++++++++++++++++++
 1 files changed, 36 insertions(+), 0 deletions(-)

Documentation/video4linux/v4l2-framework.txt +26 -0

index 74d677c..08f9e59 100644
@@ -695,3 +695,39 @@ The better way to understand it is to take a look at vivi driver. One
 of the main reasons for vivi is to be a videobuf usage example. the
 vivi_thread_tick() does the task that the IRQ callback would do on PCI
 drivers (or the irq callback on USB).
+
+struct v4l2_fh
+--------------
+
+struct v4l2_fh provides a way to easily keep file handle specific data
+that is used by the V4L2 framework.
+
+struct v4l2_fh is allocated as a part of the driver's own file handle
+structure and is set to file->private_data in the driver's open
+function by the driver. Drivers can extract their own file handle
+structure by using the container_of macro.
+
+Useful functions:
+
+- v4l2_fh_init()
+
+  Initialise the file handle.
+
+- v4l2_fh_add()
+
+  Add a v4l2_fh to video_device file handle list. May be called after
+  initialising the file handle.
+
+- v4l2_fh_del()
+
+  Unassociate the file handle from video_device(). The file handle

-----Original Message-----
From: [email protected] [mailto:linux-media- [email protected]] On Behalf Of Sakari Ailus
Sent: Friday, February 19, 2010 1:22 PM
To: [email protected]
Cc: [email protected]; [email protected]; iivanov@mm- sol.com; [email protected]; [email protected]; Sakari Ailus
Subject: [PATCH v5 4/6] V4L: Events: Add backend

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/Makefile     |    3 +-
 drivers/media/video/v4l2-event.c |  286
++++++++++++++++++++++++++++++++++++++
 drivers/media/video/v4l2-fh.c    |    4 +
 include/media/v4l2-event.h       |   65 +++++++++
 include/media/v4l2-fh.h          |    2 +
 5 files changed, 359 insertions(+), 1 deletions(-)
 create mode 100644 drivers/media/video/v4l2-event.c
 create mode 100644 include/media/v4l2-event.h

drivers/media/video/Makefile +2 -1

index 14bf69a..b84abfe 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-fh.o
+videodev-objs	:=	v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \
+			v4l2-event.o
 
 # V4L2 core modules
 

drivers/media/video/v4l2- renamed from drivers/media/video/v4l2-event.c +0 -0

event.c
new file mode 100644
index 0000000..ab31cc6

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

+ *
+ * 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>
+#include <media/v4l2-event.h>
+
+#include <linux/sched.h>
+
+static int v4l2_event_init(struct v4l2_fh *fh)
+{
+	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);
+
+	fh->events->sequence = -1;
+
+	return 0;
+}
+
+int v4l2_event_alloc(struct v4l2_fh *fh, unsigned int n)
+{
+	struct v4l2_events *events;
+	unsigned long flags;
+	int ret;
+
+	if (!fh->events) {
+		ret = v4l2_event_init(fh);
+		if (ret)
+			return ret;
+	}
+
+	events = fh->events;
+
+	while (events->nallocated < 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);
+		events->nallocated++;
+		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_free(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_free);
+
+int v4l2_event_dequeue(struct v4l2_fh *fh, struct v4l2_event *event)
+{
+	struct v4l2_events *events = fh->events;
+	struct v4l2_kevent *kev;

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

+
+	kev = list_first_entry(&events->available, struct v4l2_kevent,

list);

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

+	list_move(&kev->list, &events->free);
+	events->navailable--;
+
+	kev->event.pending = events->navailable;
+	*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;
+}
+
+void v4l2_event_queue(struct video_device *vdev, const struct v4l2_event

*ev)

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

+{
+	struct v4l2_fh *fh;
+	unsigned long flags;
+	struct timespec timestamp;
+
+	ktime_get_ts(&timestamp);
+
+	spin_lock_irqsave(&vdev->fh_lock, flags);
+
+	list_for_each_entry(fh, &vdev->fh_list, list) {
+		struct v4l2_events *events = fh->events;
+		struct v4l2_kevent *kev;
+		u32 sequence;
+
+		/* Are we subscribed? */
+		if (!v4l2_event_subscribed(fh, ev->type))
+			continue;
+
+		/* Increase event sequence number on fh. */
+		events->sequence++;
+		sequence = events->sequence;
+
+		/* Do we have any free events? */
+		if (list_empty(&events->free))
+			continue;
+
+		/* Take one and fill it. */
+		kev = list_first_entry(&events->free, struct v4l2_kevent,

list);

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

+		kev->event.type = ev->type;
+		kev->event.u = ev->u;
+		kev->event.timestamp = timestamp;
+		kev->event.sequence = sequence;
+		list_move_tail(&kev->list, &events->available);
+
+		events->navailable++;
+
+		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)
+{
+	return fh->events->navailable;
+}
+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;
+	struct v4l2_subscribed_event *sev;
+	unsigned long flags;
+
+	sev = kmalloc(sizeof(*sev), GFP_KERNEL);
+	if (!sev)
+		return -ENOMEM;
+
+	spin_lock_irqsave(&fh->vdev->fh_lock, flags);
+
+	if (v4l2_event_subscribed(fh, sub->type) == NULL) {
+		INIT_LIST_HEAD(&sev->list);
+		sev->type = sub->type;
+
+		list_add(&sev->list, &events->subscribed);
+	}
+
+	spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(v4l2_event_subscribe);
+
+/* subscribe a zero-terminated array of events */
+int v4l2_event_subscribe_many(struct v4l2_fh *fh, const u32 *all)
+{
+	int ret;
+
+	for (; *all; all++) {
+		struct v4l2_event_subscription sub;
+
+		sub.type = *all;
+
+		ret = v4l2_event_subscribe(fh, &sub);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(v4l2_event_subscribe_many);
+
+static void v4l2_event_unsubscribe_all(struct v4l2_fh *fh)
+{
+	struct v4l2_events *events = fh->events;
+	unsigned long flags;
+
+	spin_lock_irqsave(&fh->vdev->fh_lock, flags);
+
+	while (!list_empty(&events->subscribed)) {
+		struct v4l2_subscribed_event *sev;
+
+		sev = list_first_entry(&events->subscribed,
+				       struct v4l2_subscribed_event, list);
+
+		list_del(&sev->list);
+		spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
+		kfree(sev);
+		spin_lock_irqsave(&fh->vdev->fh_lock, flags);
+	}
+
+	spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
+}
+
+int v4l2_event_unsubscribe(struct v4l2_fh *fh,
+			   struct v4l2_event_subscription *sub)
+{
+	struct v4l2_subscribed_event *sev;
+	unsigned long flags;
+
+	if (sub->type == V4L2_EVENT_ALL) {
+		v4l2_event_unsubscribe_all(fh);
+		return 0;
+	}
+
+	spin_lock_irqsave(&fh->vdev->fh_lock, flags);
+
+	sev = v4l2_event_subscribed(fh, sub->type);
+	if (sev != NULL)
+		list_del(&sev->list);
+
+	spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
+
+	if (sev != NULL)
+		kfree(sev);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(v4l2_event_unsubscribe);

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

index c707930..3a9bc7d 100644
@@ -25,11 +25,13 @@
 #include <linux/bitops.h>
 #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)
 {
 	fh->vdev = vdev;
 	INIT_LIST_HEAD(&fh->list);
+	fh->events = NULL;
 	set_bit(V4L2_FL_USES_V4L2_FH, &fh->vdev->flags);
 }
 EXPORT_SYMBOL_GPL(v4l2_fh_init);

@@ -60,5 +62,7 @@ void v4l2_fh_exit(struct v4l2_fh *fh)
 		return;
 
 	fh->vdev = NULL;
+
+	v4l2_event_free(fh);
 }
 EXPORT_SYMBOL_GPL(v4l2_fh_exit);

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

new file mode 100644
index 0000000..284d495

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

+ *
+ * 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 */
+	unsigned int		navailable;
+	struct list_head	free; /* Events ready for use */
+	unsigned int		nallocated; /* Number of allocated events */
+	u32			sequence;
+};
+
+int v4l2_event_alloc(struct v4l2_fh *fh, unsigned int n);
+void v4l2_event_free(struct v4l2_fh *fh);
+int v4l2_event_dequeue(struct v4l2_fh *fh, struct v4l2_event *event);
+void v4l2_event_queue(struct video_device *vdev, const struct v4l2_event

*ev);

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

+int v4l2_event_pending(struct v4l2_fh *fh);
+int v4l2_event_subscribe(struct v4l2_fh *fh,
+			 struct v4l2_event_subscription *sub);
+int v4l2_event_subscribe_many(struct v4l2_fh *fh, const u32 *all);
+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 6b486aa..6c9df56 100644
@@ -28,10 +28,12 @@
 #include <linux/list.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);
--
1.5.6.5

--
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

Heippa!

Quoted 8 lines

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

+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);
+

drivers/media/video/v4l2-event.c line 116

Quoted 6 lines

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

+	if (list_empty(&events->available)) {
+		spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
+		return -ENOENT;
+	}
+
+	WARN_ON(events->navailable == 0);

I don't think the above warning will ever happen. Looks a bit over protective to me.

Whenever you update your "events->available" list, you're holding the fh_lock spinlock, so there's no chance that the list of events would contan a different number of elents to what the navailable var is holding. Is it?

Please correct me if I'm missing something...

Or if you insist in checking, you could just have done this instead:

	if (list_empty(&events->available) || (events->navailable == 0)) {
		spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
		return -ENOENT;
	}

As it doesn't make sense to proceed if navailable is zero, I believe...

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/Makefile     |    3 +-
 drivers/media/video/v4l2-event.c |  286 ++++++++++++++++++++++++++++++++++++++
 drivers/media/video/v4l2-fh.c    |    4 +
 include/media/v4l2-event.h       |   65 +++++++++
 include/media/v4l2-fh.h          |    2 +
 5 files changed, 359 insertions(+), 1 deletions(-)
 create mode 100644 drivers/media/video/v4l2-event.c
 create mode 100644 include/media/v4l2-event.h

drivers/media/video/Makefile +2 -1

index 14bf69a..b84abfe 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-fh.o
+videodev-objs	:=	v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \
+			v4l2-event.o
 
 # V4L2 core modules
 

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

new file mode 100644
index 0000000..ab31cc6
@@ -0,0 +1,286 @@
+/*
+ * 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-fh.h>
+#include <media/v4l2-event.h>
+
+#include <linux/sched.h>
+
+static int v4l2_event_init(struct v4l2_fh *fh)
+{
+	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);
+
+	fh->events->sequence = -1;
+
+	return 0;
+}
+
+int v4l2_event_alloc(struct v4l2_fh *fh, unsigned int n)
+{
+	struct v4l2_events *events;
+	unsigned long flags;
+	int ret;
+
+	if (!fh->events) {
+		ret = v4l2_event_init(fh);
+		if (ret)
+			return ret;
+	}
+
+	events = fh->events;
+
+	while (events->nallocated < 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);
+		events->nallocated++;
+		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_free(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_free);
+
+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;
+	}
+
+	WARN_ON(events->navailable == 0);
+
+	kev = list_first_entry(&events->available, struct v4l2_kevent, list);
+	list_move(&kev->list, &events->free);
+	events->navailable--;
+
+	kev->event.pending = events->navailable;

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

+{
+	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;
+}
+
+void v4l2_event_queue(struct video_device *vdev, const struct v4l2_event *ev)
+{
+	struct v4l2_fh *fh;
+	unsigned long flags;
+	struct timespec timestamp;
+
+	ktime_get_ts(&timestamp);
+
+	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 +14 -0

+		list_move_tail(&kev->list, &events->available);
+
+		events->navailable++;
+
+		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)
+{
+	return fh->events->navailable;

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

+	sev = kmalloc(sizeof(*sev), GFP_KERNEL);
+	if (!sev)
+		return -ENOMEM;
+

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

+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(v4l2_event_subscribe);
+
+/* subscribe a zero-terminated array of events */
+int v4l2_event_subscribe_many(struct v4l2_fh *fh, const u32 *all)
+{
+	int ret;
+
+	for (; *all; all++) {
+		struct v4l2_event_subscription sub;
+

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

+
+static void v4l2_event_unsubscribe_all(struct v4l2_fh *fh)
+{
+	struct v4l2_events *events = fh->events;
+	unsigned long flags;
+
+	spin_lock_irqsave(&fh->vdev->fh_lock, flags);
+
+	while (!list_empty(&events->subscribed)) {
+		struct v4l2_subscribed_event *sev;
+
+		sev = list_first_entry(&events->subscribed,

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

+
+int v4l2_event_unsubscribe(struct v4l2_fh *fh,
+			   struct v4l2_event_subscription *sub)
+{
+	struct v4l2_subscribed_event *sev;
+	unsigned long flags;
+
+	if (sub->type == V4L2_EVENT_ALL) {
+		v4l2_event_unsubscribe_all(fh);
+		return 0;
+	}
+

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

+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(v4l2_event_unsubscribe);

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

index c707930..3a9bc7d 100644
@@ -25,11 +25,13 @@
 #include <linux/bitops.h>
 #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)
 {
 	fh->vdev = vdev;
 	INIT_LIST_HEAD(&fh->list);
+	fh->events = NULL;
 	set_bit(V4L2_FL_USES_V4L2_FH, &fh->vdev->flags);
 }
 EXPORT_SYMBOL_GPL(v4l2_fh_init);

@@ -60,5 +62,7 @@ void v4l2_fh_exit(struct v4l2_fh *fh)
 		return;
 
 	fh->vdev = NULL;
+
+	v4l2_event_free(fh);
 }
 EXPORT_SYMBOL_GPL(v4l2_fh_exit);

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

new file mode 100644
index 0000000..284d495
@@ -0,0 +1,65 @@
+/*
+ * 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 {

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

+	unsigned int		nallocated; /* Number of allocated events */
+	u32			sequence;
+};
+
+int v4l2_event_alloc(struct v4l2_fh *fh, unsigned int n);
+void v4l2_event_free(struct v4l2_fh *fh);
+int v4l2_event_dequeue(struct v4l2_fh *fh, struct v4l2_event *event);
+void v4l2_event_queue(struct video_device *vdev, 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);
+int v4l2_event_subscribe_many(struct v4l2_fh *fh, const u32 *all);
+int v4l2_event_unsubscribe(struct v4l2_fh *fh,
+			   struct v4l2_event_subscription *sub);
+
+#endif /* V4L2_EVENT_H */

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

index 6b486aa..6c9df56 100644
@@ -28,10 +28,12 @@
 #include <linux/list.h>
 

Hi Sakari,

Here are some more comments.

Quoted 25 lines

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

+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;
+	}
+
+	WARN_ON(events->navailable == 0);
+
+	kev = list_first_entry(&events->available, struct v4l2_kevent, list);
+	list_move(&kev->list, &events->free);
+	events->navailable--;
+
+	kev->event.pending = events->navailable;
+	*event = kev->event;
+
+	spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
+
+	return 0;

drivers/media/video/v4l2-event.c line 132

Quoted 5 lines

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

+}
+EXPORT_SYMBOL_GPL(v4l2_event_dequeue);
+
+static struct v4l2_subscribed_event *v4l2_event_subscribed(
+	struct v4l2_fh *fh, u32 type)

Add a comment before this function that mentions that fh->vdev->fh_lock must be held before calling this.

Quoted 28 lines

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

+{
+	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;
+}
+
+void v4l2_event_queue(struct video_device *vdev, const struct v4l2_event *ev)
+{
+	struct v4l2_fh *fh;
+	unsigned long flags;
+	struct timespec timestamp;
+
+	ktime_get_ts(&timestamp);
+
+	spin_lock_irqsave(&vdev->fh_lock, flags);
+
+	list_for_each_entry(fh, &vdev->fh_list, list) {
+		struct v4l2_events *events = fh->events;
+		struct v4l2_kevent *kev;
+		u32 sequence;
+
+		/* Are we subscribed? */

drivers/media/video/v4l2-event.c line 166

Quoted 6 lines

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

+		if (!v4l2_event_subscribed(fh, ev->type))
+			continue;
+
+		/* Increase event sequence number on fh. */
+		events->sequence++;
+		sequence = events->sequence;

There is no need for a temp sequence variable...

drivers/media/video/v4l2-event.c line 177

Quoted 11 lines

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

+
+		/* Do we have any free events? */
+		if (list_empty(&events->free))
+			continue;
+
+		/* Take one and fill it. */
+		kev = list_first_entry(&events->free, struct v4l2_kevent, list);
+		kev->event.type = ev->type;
+		kev->event.u = ev->u;
+		kev->event.timestamp = timestamp;
+		kev->event.sequence = sequence;

... you can just use events->sequence directly here.

Quoted 14 lines

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

+		list_move_tail(&kev->list, &events->available);
+
+		events->navailable++;
+
+		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)
+{
+	return fh->events->navailable;

drivers/media/video/v4l2-event.c line 201

Quoted 10 lines

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

+}
+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;
+	struct v4l2_subscribed_event *sev;
+	unsigned long flags;
+

Add this:

	if (events == NULL) {
		/* If we get here, then the driver forgot to allocate events. */
		WARN_ON(1);
		return -ENOMEM;
	}

If subscribe is called without the event queue being allocated, then the driver did something wrong.

See also my review for patch 5/6.

Quoted 9 lines

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

+	sev = kmalloc(sizeof(*sev), GFP_KERNEL);
+	if (!sev)
+		return -ENOMEM;
+
+	spin_lock_irqsave(&fh->vdev->fh_lock, flags);
+
+	if (v4l2_event_subscribed(fh, sub->type) == NULL) {
+		INIT_LIST_HEAD(&sev->list);
+		sev->type = sub->type;

drivers/media/video/v4l2-event.c line 215

Quoted 5 lines

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

+
+		list_add(&sev->list, &events->subscribed);
+	}
+
+	spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);

If the event was already subscribed, then you need to kfree the earlier allocated sev here. Otherwise you have a memory leak.

Quoted 13 lines

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

+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(v4l2_event_subscribe);
+
+/* subscribe a zero-terminated array of events */
+int v4l2_event_subscribe_many(struct v4l2_fh *fh, const u32 *all)
+{
+	int ret;
+
+	for (; *all; all++) {
+		struct v4l2_event_subscription sub;
+

drivers/media/video/v4l2-event.c line 238

Quoted 10 lines

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

+		sub.type = *all;
+
+		ret = v4l2_event_subscribe(fh, &sub);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(v4l2_event_subscribe_many);

Supporting V4L2_EVENT_ALL is a bad idea when subscribing. It sounds nice initially, but the longer I think about it the more convinced I am we should not do this. The main argument is really that it can lead to unexpected behavior. Suppose a userspace application subscribes to all events. And in a later version of the driver new events are added. Suddenly those will also arrive in the userspace app.

This might cause it to crash if it was written badly and it didn't check against unknown events. Or this new event might be a high-volume event which might flood the application unexpectedly.

Quoted 12 lines

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

+
+static void v4l2_event_unsubscribe_all(struct v4l2_fh *fh)
+{
+	struct v4l2_events *events = fh->events;
+	unsigned long flags;
+
+	spin_lock_irqsave(&fh->vdev->fh_lock, flags);
+
+	while (!list_empty(&events->subscribed)) {
+		struct v4l2_subscribed_event *sev;
+
+		sev = list_first_entry(&events->subscribed,

drivers/media/video/v4l2-event.c line 260

Quoted 10 lines

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

+				       struct v4l2_subscribed_event, list);
+
+		list_del(&sev->list);
+		spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
+		kfree(sev);
+		spin_lock_irqsave(&fh->vdev->fh_lock, flags);
+	}
+
+	spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
+}

What about this:

static void v4l2_event_unsubscribe_all(struct v4l2_fh *fh)

{
	struct v4l2_events *events = fh->events;
	struct v4l2_subscribed_event *sev;
	unsigned long flags;

	do {
		sev = NULL;

		spin_lock_irqsave(&fh->vdev->fh_lock, flags);
		if (!list_empty(&events->subscribed)) {
			sev = list_first_entry(&events->subscribed,
				       struct v4l2_subscribed_event, list);
			list_del(&sev->list);
		}
		spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
		kfree(sev);
	} while (sev);
}

This avoids the 'interleaved' locking which I never like.

Quoted 16 lines

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

+
+int v4l2_event_unsubscribe(struct v4l2_fh *fh,
+			   struct v4l2_event_subscription *sub)
+{
+	struct v4l2_subscribed_event *sev;
+	unsigned long flags;
+
+	if (sub->type == V4L2_EVENT_ALL) {
+		v4l2_event_unsubscribe_all(fh);
+		return 0;
+	}
+
+	spin_lock_irqsave(&fh->vdev->fh_lock, flags);
+
+	sev = v4l2_event_subscribed(fh, sub->type);
+	if (sev != NULL)

drivers/media/video/v4l2-event.c line 282

Quoted 6 lines

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

+		list_del(&sev->list);
+
+	spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
+
+	if (sev != NULL)
+		kfree(sev);

No need for the NULL check. kfree(NULL) is allowed.

include/media/v4l2-event.h line 49

Quoted 6 lines

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

+struct v4l2_events {
+	wait_queue_head_t	wait;
+	struct list_head	subscribed; /* Subscribed events */
+	struct list_head	available; /* Dequeueable event */
+	unsigned int		navailable;
+	struct list_head	free; /* Events ready for use */

I would move this between the subscribed and available lists. Purely for grouping the lists together.

Quoted 1 line · Hans Verkuil

Hi Sakari,

Hi Hans,

And many thanks for the comments again!

Quoted 2 lines

Here are some more comments.

...

Quoted 45 lines

Quoted 12 lines

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

+
+static void v4l2_event_unsubscribe_all(struct v4l2_fh *fh)
+{
+	struct v4l2_events *events = fh->events;
+	unsigned long flags;
+
+	spin_lock_irqsave(&fh->vdev->fh_lock, flags);
+
+	while (!list_empty(&events->subscribed)) {
+		struct v4l2_subscribed_event *sev;
+
+		sev = list_first_entry(&events->subscribed,

drivers/media/video/v4l2-event.c line 260

Quoted 10 lines

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

+				       struct v4l2_subscribed_event, list);
+
+		list_del(&sev->list);
+		spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
+		kfree(sev);
+		spin_lock_irqsave(&fh->vdev->fh_lock, flags);
+	}
+
+	spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
+}

What about this:

static void v4l2_event_unsubscribe_all(struct v4l2_fh *fh)

{
	struct v4l2_events *events = fh->events;
	struct v4l2_subscribed_event *sev;
	unsigned long flags;

	do {
		sev = NULL;

		spin_lock_irqsave(&fh->vdev->fh_lock, flags);
		if (!list_empty(&events->subscribed)) {
			sev = list_first_entry(&events->subscribed,
				       struct v4l2_subscribed_event, list);
			list_del(&sev->list);
		}
		spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
		kfree(sev);
	} while (sev);
}

Quoted 1 line

This avoids the 'interleaved' locking which I never like.

Can do. I don't see anything bad in that kind of locking, though. ;-)

Add support for event handling to do_ioctl.

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

---
 drivers/media/video/v4l2-ioctl.c |   58 ++++++++++++++++++++++++++++++++++++++
 include/media/v4l2-ioctl.h       |    7 ++++
 2 files changed, 65 insertions(+), 0 deletions(-)

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

index 34c7d6e..f7d6177 100644
@@ -25,6 +25,8 @@
 #endif
 #include <media/v4l2-common.h>
 #include <media/v4l2-ioctl.h>
+#include <media/v4l2-fh.h>
+#include <media/v4l2-event.h>
 #include <media/v4l2-chip-ident.h>
 
 #define dbgarg(cmd, fmt, arg...) \

@@ -1944,7 +1946,63 @@ static long __video_do_ioctl(struct file *file,
 		}

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

+		if (ret < 0) {
+			dbgarg(cmd, "no pending events?");
+			break;
+		}
+		dbgarg(cmd,
+		       "pending=%d, type=0x%8.8x, sequence=%d, "
+		       "timestamp=%lu.%9.9lu ",

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

+		    || !ops->vidioc_subscribe_event)
+			break;
+
+		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 +10 -0

+			break;
+
+		ret = ops->vidioc_unsubscribe_event(fh, sub);
+		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 +2 -0

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

@@ -254,6 +256,11 @@ struct v4l2_ioctl_ops {
 	int (*vidioc_g_dv_timings) (struct file *file, void *fh,

More comments...

Quoted 23 lines · On Friday 19 February 2010 20:21:59 Sakari Ailus wrote:

Add support for event handling to do_ioctl.

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

---
 drivers/media/video/v4l2-ioctl.c |   58 ++++++++++++++++++++++++++++++++++++++
 include/media/v4l2-ioctl.h       |    7 ++++
 2 files changed, 65 insertions(+), 0 deletions(-)

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

index 34c7d6e..f7d6177 100644
@@ -25,6 +25,8 @@
 #endif
 #include <media/v4l2-common.h>
 #include <media/v4l2-ioctl.h>
+#include <media/v4l2-fh.h>
+#include <media/v4l2-event.h>
 #include <media/v4l2-chip-ident.h>
 
 #define dbgarg(cmd, fmt, arg...) \

@@ -1944,7 +1946,63 @@ static long __video_do_ioctl(struct file *file,
 		}

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

Quoted 10 lines

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

 		break;
 	}
+	case VIDIOC_DQEVENT:
+	{
+		struct v4l2_event *ev = arg;
+		struct v4l2_fh *vfh = fh;
+
+		if (!test_bit(V4L2_FL_USES_V4L2_FH, &vfd->flags)
+		    || vfh->events == NULL)
+			break;

Change this to:

		if (!test_bit(V4L2_FL_USES_V4L2_FH, &vfd->flags))
			break;
		if (vfh->events == NULL)
			return -ENOENT;

But see also the next comment.

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

Quoted 2 lines

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

+
+		ret = v4l2_event_dequeue(fh, ev);

There is a crucial piece of functionality missing here: if the filehandle is in blocking mode, then it should wait until an event arrives. That also means that if vfh->events == NULL, you should still call v4l2_event_dequeue, and that function should initialize vfh->events and wait for an event if the fh is in blocking mode.

Quoted 7 lines

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

+		if (ret < 0) {
+			dbgarg(cmd, "no pending events?");
+			break;
+		}
+		dbgarg(cmd,
+		       "pending=%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 1975

Quoted 10 lines

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

+		       ev->pending, ev->type, ev->sequence,
+		       ev->timestamp.tv_sec, ev->timestamp.tv_nsec);
+		break;
+	}
+	case VIDIOC_SUBSCRIBE_EVENT:
+	{
+		struct v4l2_event_subscription *sub = arg;
+		struct v4l2_fh *vfh = fh;
 
+		if (!test_bit(V4L2_FL_USES_V4L2_FH, &vfd->flags)

Testing for this bit is unnecessarily. Just test for ops->vidioc_subscribe_event.

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

Quoted 1 line

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

+		    || vfh->events == NULL

Remove this test. If you allocate the event queue only when you first subscribe to an event (as ivtv will do), then you have to be able to call vidioc_subscribe_event even if vfh->events == NULL.

Quoted 57 lines · Hans Verkuil

More comments...

Quoted 32 lines · On Friday 19 February 2010 20:21:59 Sakari Ailus wrote:

Add support for event handling to do_ioctl.

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

---

 drivers/media/video/v4l2-ioctl.c |   58
 ++++++++++++++++++++++++++++++++++++++ include/media/v4l2-ioctl.h     
  |    7 ++++
 2 files changed, 65 insertions(+), 0 deletions(-)

(quoted diff) +0 -0

b/drivers/media/video/v4l2-ioctl.c index 34c7d6e..f7d6177 100644

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

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

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

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

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

@@ -1944,7 +1946,63 @@ static long __video_do_ioctl(struct file *file,
 
 		}
 		break;
 

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

Quoted 10 lines

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

 	}
 
+	case VIDIOC_DQEVENT:
+	{
+		struct v4l2_event *ev = arg;
+		struct v4l2_fh *vfh = fh;
+
+		if (!test_bit(V4L2_FL_USES_V4L2_FH, &vfd->flags)
+		    || vfh->events == NULL)
+			break;

Change this to:

		if (!test_bit(V4L2_FL_USES_V4L2_FH, &vfd->flags))
		
			break;
		
		if (vfh->events == NULL)
		
			return -ENOENT;

But see also the next comment.

Quoted 8 lines

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

Quoted 2 lines

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

+
+		ret = v4l2_event_dequeue(fh, ev);

There is a crucial piece of functionality missing here: if the filehandle is in blocking mode, then it should wait until an event arrives. That also means that if vfh->events == NULL, you should still call v4l2_event_dequeue, and that function should initialize vfh->events and wait for an event if the fh is in blocking mode.

I originally left this out intentionally. Most applications using events would use select / poll as well by default. For completeness it should be there, I agree.

This btw. suggests that we perhaps should put back the struct file argument for the event functions in video_ioctl_ops. The blocking flag is indeed part of the file structure. I'm open to better suggestions, too.

Hi Sakari,

Quoted 10 lines

Quoted 5 lines

There is a crucial piece of functionality missing here: if the filehandle is in blocking mode, then it should wait until an event arrives. That also means that if vfh->events == NULL, you should still call v4l2_event_dequeue, and that function should initialize vfh->events and wait for an event if the fh is in blocking mode.

I originally left this out intentionally. Most applications using events would use select / poll as well by default. For completeness it should be there, I agree.

Quoted 3 lines

This btw. suggests that we perhaps should put back the struct file argument for the event functions in video_ioctl_ops. The blocking flag is indeed part of the file structure. I'm open to better suggestions, too.

If the only information we need from struct file is the flags, they could be copied to v4l2_fh in the open handler. We could also put a struct file * member in v4l2_fh.

Quoted 46 lines · Hans Verkuil

More comments...

Quoted 23 lines · On Friday 19 February 2010 20:21:59 Sakari Ailus wrote:

Add support for event handling to do_ioctl.

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

---
 drivers/media/video/v4l2-ioctl.c |   58 ++++++++++++++++++++++++++++++++++++++
 include/media/v4l2-ioctl.h       |    7 ++++
 2 files changed, 65 insertions(+), 0 deletions(-)

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

index 34c7d6e..f7d6177 100644
@@ -25,6 +25,8 @@
 #endif
 #include <media/v4l2-common.h>
 #include <media/v4l2-ioctl.h>
+#include <media/v4l2-fh.h>
+#include <media/v4l2-event.h>
 #include <media/v4l2-chip-ident.h>
 
 #define dbgarg(cmd, fmt, arg...) \

@@ -1944,7 +1946,63 @@ static long __video_do_ioctl(struct file *file,
 		}

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

Quoted 10 lines

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

 		break;
 	}
+	case VIDIOC_DQEVENT:
+	{
+		struct v4l2_event *ev = arg;
+		struct v4l2_fh *vfh = fh;
+
+		if (!test_bit(V4L2_FL_USES_V4L2_FH, &vfd->flags)
+		    || vfh->events == NULL)
+			break;

Change this to:

		if (!test_bit(V4L2_FL_USES_V4L2_FH, &vfd->flags))
			break;
		if (vfh->events == NULL)
			return -ENOENT;

But see also the next comment.

Quoted 8 lines

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

Quoted 2 lines

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

+
+		ret = v4l2_event_dequeue(fh, ev);

There is a crucial piece of functionality missing here: if the filehandle is in blocking mode, then it should wait until an event arrives. That also means that if vfh->events == NULL, you should still call v4l2_event_dequeue, and that function should initialize vfh->events and wait for an event if the fh is in blocking mode.

I originally left this out intentionally. Most applications using events would use select / poll as well by default. For completeness it should be there, I agree.

This btw. suggests that we perhaps should put back the struct file argument for the event functions in video_ioctl_ops. The blocking flag is indeed part of the file structure. I'm open to better suggestions, too.

Quoted 20 lines

Quoted 7 lines

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

+		if (ret < 0) {
+			dbgarg(cmd, "no pending events?");
+			break;
+		}
+		dbgarg(cmd,
+		       "pending=%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 1975

Quoted 10 lines

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

+		       ev->pending, ev->type, ev->sequence,
+		       ev->timestamp.tv_sec, ev->timestamp.tv_nsec);
+		break;
+	}
+	case VIDIOC_SUBSCRIBE_EVENT:
+	{
+		struct v4l2_event_subscription *sub = arg;
+		struct v4l2_fh *vfh = fh;
 
+		if (!test_bit(V4L2_FL_USES_V4L2_FH, &vfd->flags)

Testing for this bit is unnecessarily. Just test for ops->vidioc_subscribe_event.

Quoted 5 lines

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

Quoted 1 line

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

+		    || vfh->events == NULL

Remove this test. If you allocate the event queue only when you first subscribe to an event (as ivtv will do), then you have to be able to call vidioc_subscribe_event even if vfh->events == NULL.

How about calling v4l2_event_alloc() with zero events? That allocates and initialises the v4l2_events structure. That's easier to handle in drivers as well since they don't need to consider special cases like fh->events happens to be NULL even if events are supported by the driver. This is how I first thought it'd work.

Quoted 67 lines · On Sunday 21 February 2010 23:31:43 Sakari Ailus wrote:

Quoted 65 lines · Hans Verkuil

More comments...

Quoted 32 lines · On Friday 19 February 2010 20:21:59 Sakari Ailus wrote:

Add support for event handling to do_ioctl.

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

---

 drivers/media/video/v4l2-ioctl.c |   58
 ++++++++++++++++++++++++++++++++++++++ include/media/v4l2-ioctl.h   
    |    7 ++++
 2 files changed, 65 insertions(+), 0 deletions(-)

(quoted diff) +0 -0

b/drivers/media/video/v4l2-ioctl.c index 34c7d6e..f7d6177 100644

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

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

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

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

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

@@ -1944,7 +1946,63 @@ static long __video_do_ioctl(struct file *file,
 
 		}
 		break;
 

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

Quoted 10 lines

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

 	}
 
+	case VIDIOC_DQEVENT:
+	{
+		struct v4l2_event *ev = arg;
+		struct v4l2_fh *vfh = fh;
+
+		if (!test_bit(V4L2_FL_USES_V4L2_FH, &vfd->flags)
+		    || vfh->events == NULL)
+			break;

Change this to:

		if (!test_bit(V4L2_FL_USES_V4L2_FH, &vfd->flags))
		
			break;
		
		if (vfh->events == NULL)
		
			return -ENOENT;

But see also the next comment.

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

Quoted 2 lines

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

+
+		ret = v4l2_event_dequeue(fh, ev);

There is a crucial piece of functionality missing here: if the filehandle is in blocking mode, then it should wait until an event arrives. That also means that if vfh->events == NULL, you should still call v4l2_event_dequeue, and that function should initialize vfh->events and wait for an event if the fh is in blocking mode.

Quoted 3 lines

I originally left this out intentionally. Most applications using events would use select / poll as well by default. For completeness it should be there, I agree.

It has to be there. This is important functionality. For e.g. ivtv I would use this to wait until the MPEG decoder flushed all buffers and displayed the last frame of the stream. That's something you would often do in blocking mode.

Quoted 4 lines

This btw. suggests that we perhaps should put back the struct file argument for the event functions in video_ioctl_ops. The blocking flag is indeed part of the file structure. I'm open to better suggestions, too.

My long term goal is that the file struct is only used inside v4l2-ioctl.c and not in drivers. Drivers should not need this struct at all. The easiest way to ensure this is by not passing it to the drivers at all :-)

Quoted 27 lines

Quoted 21 lines

Quoted 7 lines

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

+		if (ret < 0) {
+			dbgarg(cmd, "no pending events?");
+			break;
+		}
+		dbgarg(cmd,
+		       "pending=%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 1975

Quoted 10 lines

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

+		       ev->pending, ev->type, ev->sequence,
+		       ev->timestamp.tv_sec, ev->timestamp.tv_nsec);
+		break;
+	}
+	case VIDIOC_SUBSCRIBE_EVENT:
+	{
+		struct v4l2_event_subscription *sub = arg;
+		struct v4l2_fh *vfh = fh;
 
+		if (!test_bit(V4L2_FL_USES_V4L2_FH, &vfd->flags)

Testing for this bit is unnecessarily. Just test for ops->vidioc_subscribe_event.

Quoted 5 lines

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

Quoted 1 line

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

+		    || vfh->events == NULL

Remove this test. If you allocate the event queue only when you first subscribe to an event (as ivtv will do), then you have to be able to call vidioc_subscribe_event even if vfh->events == NULL.

Quoted 5 lines

How about calling v4l2_event_alloc() with zero events? That allocates and initialises the v4l2_events structure. That's easier to handle in drivers as well since they don't need to consider special cases like fh->events happens to be NULL even if events are supported by the driver. This is how I first thought it'd work.

Proposal: export a v4l2_event_init() call that sets up fh->events. Calling v4l2_event_alloc(0) feels like a hack. So drivers that want to be able to handle events should call v4l2_event_init after initializing the file handle.

Or (and that might even be nicer) test in v4l2_fh_init whether there is a subscribe op in the ioctl_ops struct and let v4l2_fh_init set up fh->events automatically.

Hi Hans,

Quoted 76 lines · On Monday 22 February 2010 08:53:53 Hans Verkuil wrote:

Quoted 7 lines · On Sunday 21 February 2010 23:31:43 Sakari Ailus wrote:

Quoted 6 lines · Hans Verkuil

More comments...

Quoted 3 lines · On Friday 19 February 2010 20:21:59 Sakari Ailus wrote:

Add support for event handling to do_ioctl.

Signed-off-by Sakari Ailus

<[email protected]>

Quoted 17 lines

Quoted 17 lines

Quoted 17 lines

---

 drivers/media/video/v4l2-ioctl.c |   58
 ++++++++++++++++++++++++++++++++++++++ include/media/v4l2-ioctl.h
    |    7 ++++
 2 files changed, 65 insertions(+), 0 deletions(-)

(quoted diff) +0 -0

b/drivers/media/video/v4l2-ioctl.c index 34c7d6e..f7d6177 100644

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

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

Quoted 8 lines

Quoted 8 lines

drivers/media/video/v4l2-ioctl.c static long __video_do_ioctl(struct file

Quoted 8 lines

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

+#include <media/v4l2-fh.h>
+#include <media/v4l2-event.h>
 

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

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

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

@@ -1944,7 +1946,63 @@ static long __video_do_ioctl(struct file

*file,

Quoted 24 lines

Quoted 24 lines

Quoted 4 lines

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

 		}
 		break;
 

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

Quoted 10 lines

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

 	}
 
+	case VIDIOC_DQEVENT:
+	{
+		struct v4l2_event *ev = arg;
+		struct v4l2_fh *vfh = fh;
+
+		if (!test_bit(V4L2_FL_USES_V4L2_FH, &vfd->flags)
+		    || vfh->events == NULL)
+			break;

Change this to:

		if (!test_bit(V4L2_FL_USES_V4L2_FH, &vfd->flags))

			break;

		if (vfh->events == NULL)

			return -ENOENT;

Quoted 8 lines

Quoted 8 lines

But see also the next comment.

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

Quoted 2 lines

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

+
+		ret = v4l2_event_dequeue(fh, ev);

There is a crucial piece of functionality missing here: if the filehandle is in blocking mode, then it should wait until an event arrives. That also means that if vfh->events == NULL, you should

still

Quoted 4 lines

Quoted 2 lines

call v4l2_event_dequeue, and that function should initialize vfh->events and wait for an event if the fh is in blocking mode.

I originally left this out intentionally. Most applications using

events

Quoted 2 lines

would use select / poll as well by default. For completeness it should be there, I agree.

Quoted 6 lines

It has to be there. This is important functionality. For e.g. ivtv I would
use this to wait until the MPEG decoder flushed all buffers and displayed
the last frame of the stream. That's something you would often do in blocking mode.

Blocking mode can easily be emulated using select().

Quoted 10 lines

Quoted 4 lines

This btw. suggests that we perhaps should put back the struct file argument for the event functions in video_ioctl_ops. The blocking flag is indeed part of the file structure. I'm open to better suggestions, too.

My long term goal is that the file struct is only used inside v4l2-ioctl.c
and not in drivers. Drivers should not need this struct at all. The easiest
way to ensure this is by not passing it to the drivers at all :-)

Drivers still need a way to access the blocking flag. The interim solution of
adding a file * member to v4l2_fh would allow that, while still removing most
usage of file * from drivers.

Quoted 12 lines

Quoted 12 lines

Quoted 3 lines

Quoted 3 lines

There is a crucial piece of functionality missing here: if the filehandle is in blocking mode, then it should wait until an event arrives. That also means that if vfh->events == NULL, you should

still

Quoted 4 lines

Quoted 2 lines

call v4l2_event_dequeue, and that function should initialize vfh->events and wait for an event if the fh is in blocking mode.

I originally left this out intentionally. Most applications using

events

Quoted 2 lines

would use select / poll as well by default. For completeness it should be there, I agree.

Quoted 8 lines

Quoted 6 lines

It has to be there. This is important functionality. For e.g. ivtv I would
use this to wait until the MPEG decoder flushed all buffers and displayed
the last frame of the stream. That's something you would often do in blocking mode.

Blocking mode can easily be emulated using select().

Quoted 11 lines

Quoted 10 lines

Quoted 4 lines

This btw. suggests that we perhaps should put back the struct file argument for the event functions in video_ioctl_ops. The blocking flag is indeed part of the file structure. I'm open to better suggestions, too.

My long term goal is that the file struct is only used inside v4l2-ioctl.c
and not in drivers. Drivers should not need this struct at all. The easiest
way to ensure this is by not passing it to the drivers at all :-)

Quoted 5 lines

Drivers still need a way to access the blocking flag. The interim solution of
adding a file * member to v4l2_fh would allow that, while still removing most
usage of file * from drivers.

Why not just add a 'blocking' argument to the v4l2_event_dequeue? And let v4l2-ioctl.c fill in that argument? That's how I would do it.

Add documentation on how to use V4L2 events, both for V4L2 drivers and for V4L2 applications.

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

---
 Documentation/DocBook/media-entities.tmpl          |    9 ++
 Documentation/DocBook/v4l/dev-event.xml            |   34 ++++++
 Documentation/DocBook/v4l/v4l2.xml                 |    3 +
 Documentation/DocBook/v4l/vidioc-dqevent.xml       |  124 ++++++++++++++++++++
 .../DocBook/v4l/vidioc-subscribe-event.xml         |  109 +++++++++++++++++
 Documentation/video4linux/v4l2-framework.txt       |   43 +++++++
 6 files changed, 322 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/DocBook/v4l/dev-event.xml
 create mode 100644 Documentation/DocBook/v4l/vidioc-dqevent.xml
 create mode 100644 Documentation/DocBook/v4l/vidioc-subscribe-event.xml

Documentation/DocBook/media-entities.tmpl +9 -0

index c725cb8..770be3c 100644
@@ -17,6 +17,7 @@
 <!ENTITY VIDIOC-DBG-G-REGISTER "<link linkend='vidioc-dbg-g-register'><constant>VIDIOC_DBG_G_REGISTER</constant></link>">
 <!ENTITY VIDIOC-DBG-S-REGISTER "<link linkend='vidioc-dbg-g-register'><constant>VIDIOC_DBG_S_REGISTER</constant></link>">
 <!ENTITY VIDIOC-DQBUF "<link linkend='vidioc-qbuf'><constant>VIDIOC_DQBUF</constant></link>">
+<!ENTITY VIDIOC-DQEVENT "<link linkend='vidioc-dqevent'><constant>VIDIOC_DQEVENT</constant></link>">
 <!ENTITY VIDIOC-ENCODER-CMD "<link linkend='vidioc-encoder-cmd'><constant>VIDIOC_ENCODER_CMD</constant></link>">
 <!ENTITY VIDIOC-ENUMAUDIO "<link linkend='vidioc-enumaudio'><constant>VIDIOC_ENUMAUDIO</constant></link>">
 <!ENTITY VIDIOC-ENUMAUDOUT "<link linkend='vidioc-enumaudioout'><constant>VIDIOC_ENUMAUDOUT</constant></link>">

@@ -60,6 +61,7 @@
 <!ENTITY VIDIOC-REQBUFS "<link linkend='vidioc-reqbufs'><constant>VIDIOC_REQBUFS</constant></link>">
 <!ENTITY VIDIOC-STREAMOFF "<link linkend='vidioc-streamon'><constant>VIDIOC_STREAMOFF</constant></link>">
 <!ENTITY VIDIOC-STREAMON "<link linkend='vidioc-streamon'><constant>VIDIOC_STREAMON</constant></link>">
+<!ENTITY VIDIOC-SUBSCRIBE-EVENT "<link linkend='vidioc-subscribe-event'><constant>VIDIOC_SUBSCRIBE_EVENT</constant></link>">
 <!ENTITY VIDIOC-S-AUDIO "<link linkend='vidioc-g-audio'><constant>VIDIOC_S_AUDIO</constant></link>">
 <!ENTITY VIDIOC-S-AUDOUT "<link linkend='vidioc-g-audioout'><constant>VIDIOC_S_AUDOUT</constant></link>">
 <!ENTITY VIDIOC-S-CROP "<link linkend='vidioc-g-crop'><constant>VIDIOC_S_CROP</constant></link>">

@@ -141,6 +143,8 @@
 <!ENTITY v4l2-enc-idx "struct&nbsp;<link linkend='v4l2-enc-idx'>v4l2_enc_idx</link>">
 <!ENTITY v4l2-enc-idx-entry "struct&nbsp;<link linkend='v4l2-enc-idx-entry'>v4l2_enc_idx_entry</link>">
 <!ENTITY v4l2-encoder-cmd "struct&nbsp;<link linkend='v4l2-encoder-cmd'>v4l2_encoder_cmd</link>">
+<!ENTITY v4l2-event "struct&nbsp;<link linkend='v4l2-event'>v4l2_event</link>">
+<!ENTITY v4l2-event-subscription "struct&nbsp;<link linkend='v4l2-event-subscription'>v4l2_event_subscription</link>">
 <!ENTITY v4l2-ext-control "struct&nbsp;<link linkend='v4l2-ext-control'>v4l2_ext_control</link>">
 <!ENTITY v4l2-ext-controls "struct&nbsp;<link linkend='v4l2-ext-controls'>v4l2_ext_controls</link>">
 <!ENTITY v4l2-fmtdesc "struct&nbsp;<link linkend='v4l2-fmtdesc'>v4l2_fmtdesc</link>">

@@ -200,6 +204,7 @@
 <!ENTITY sub-controls SYSTEM "v4l/controls.xml">
 <!ENTITY sub-dev-capture SYSTEM "v4l/dev-capture.xml">
 <!ENTITY sub-dev-codec SYSTEM "v4l/dev-codec.xml">
+<!ENTITY sub-dev-event SYSTEM "v4l/dev-event.xml">
 <!ENTITY sub-dev-effect SYSTEM "v4l/dev-effect.xml">
 <!ENTITY sub-dev-osd SYSTEM "v4l/dev-osd.xml">
 <!ENTITY sub-dev-output SYSTEM "v4l/dev-output.xml">

@@ -292,6 +297,8 @@
 <!ENTITY sub-v4l2grab-c SYSTEM "v4l/v4l2grab.c.xml">
 <!ENTITY sub-videodev2-h SYSTEM "v4l/videodev2.h.xml">
 <!ENTITY sub-v4l2 SYSTEM "v4l/v4l2.xml">
+<!ENTITY sub-dqevent SYSTEM "v4l/vidioc-dqevent.xml">
+<!ENTITY sub-subscribe-event SYSTEM "v4l/vidioc-subscribe-event.xml">
 <!ENTITY sub-intro SYSTEM "dvb/intro.xml">
 <!ENTITY sub-frontend SYSTEM "dvb/frontend.xml">
 <!ENTITY sub-dvbproperty SYSTEM "dvb/dvbproperty.xml">

@@ -381,3 +388,5 @@
 <!ENTITY reqbufs SYSTEM "v4l/vidioc-reqbufs.xml">
 <!ENTITY s-hw-freq-seek SYSTEM "v4l/vidioc-s-hw-freq-seek.xml">
 <!ENTITY streamon SYSTEM "v4l/vidioc-streamon.xml">
+<!ENTITY dqevent SYSTEM "v4l/vidioc-dqevent.xml">
+<!ENTITY subscribe_event SYSTEM "v4l/vidioc-subscribe-event.xml">

Documentation/DocBook/v4l/dev-event.xml new file +34 -0

new file mode 100644
index 0000000..70a9895
@@ -0,0 +1,34 @@
+  <title>Event Interface</title>
+
+  <para>The V4L2 event interface provides means for user to get
+  immediately notified on certain conditions taking place on a device.
+  This might include start of frame or loss of signal events, for
+  example.
+  </para>
+
+  <para>To receive events, the events the user is interested first
+  must be subscribed using the &VIDIOC-SUBSCRIBE-EVENT; ioctl. Once an
+  event is subscribed, the events of subscribed types are dequeueable
+  using the &VIDIOC-DQEVENT; ioctl. Events may be unsubscribed using
+  VIDIOC_UNSUBSCRIBE_EVENT ioctl. The special event type
+  V4L2_EVENT_ALL may be used to subscribe or unsubscribe all the
+  events the driver supports.</para>
+
+  <para>The event subscriptions and event queues are specific to file
+  handles. Subscribing an event on one file handle does not affect
+  other file handles.
+  </para>
+
+  <para>The information on dequeueable events are obtained by using
+  select or poll system calls on video devices. The V4L2 events use
+  POLLPRI events on poll system call and exceptions on select system
+  call.
+  </para>
+
+  <!--
+Local Variables:
+mode: sgml
+sgml-parent-document: "v4l2.sgml"
+indent-tabs-mode: nil
+End:
+  -->

Documentation/DocBook/v4l/v4l2.xml +3 -0

index 060105a..9737243 100644
@@ -401,6 +401,7 @@ and discussions on the V4L mailing list.</revremark>
     <section id="ttx"> &sub-dev-teletext; </section>
     <section id="radio"> &sub-dev-radio; </section>
     <section id="rds"> &sub-dev-rds; </section>
+    <section id="event"> &sub-dev-event; </section>
   </chapter>
 
   <chapter id="driver">

@@ -426,6 +427,7 @@ and discussions on the V4L mailing list.</revremark>
     &sub-cropcap;
     &sub-dbg-g-chip-ident;
     &sub-dbg-g-register;
+    &sub-dqevent;
     &sub-encoder-cmd;
     &sub-enumaudio;
     &sub-enumaudioout;

@@ -467,6 +469,7 @@ and discussions on the V4L mailing list.</revremark>
     &sub-reqbufs;
     &sub-s-hw-freq-seek;
     &sub-streamon;
+    &sub-subscribe-event;
     <!-- End of ioctls. -->
     &sub-mmap;
     &sub-munmap;

Documentation/DocBook/v4l/vidioc-dqevent.xml new file +124 -0

new file mode 100644
index 0000000..eb45c16
@@ -0,0 +1,124 @@
+<refentry id="vidioc-dqevent">
+  <refmeta>
+    <refentrytitle>ioctl VIDIOC_DQEVENT</refentrytitle>
+    &manvol;
+  </refmeta>
+
+  <refnamediv>
+    <refname>VIDIOC_DQEVENT</refname>
+    <refpurpose>Dequeue event</refpurpose>
+  </refnamediv>
+
+  <refsynopsisdiv>
+    <funcsynopsis>
+      <funcprototype>
+	<funcdef>int <function>ioctl</function></funcdef>
+	<paramdef>int <parameter>fd</parameter></paramdef>
+	<paramdef>int <parameter>request</parameter></paramdef>
+	<paramdef>struct v4l2_event
+*<parameter>argp</parameter></paramdef>
+      </funcprototype>
+    </funcsynopsis>
+  </refsynopsisdiv>
+
+  <refsect1>
+    <title>Arguments</title>
+
+    <variablelist>
+      <varlistentry>
+	<term><parameter>fd</parameter></term>
+	<listitem>
+	  <para>&fd;</para>
+	</listitem>
+      </varlistentry>
+      <varlistentry>
+	<term><parameter>request</parameter></term>
+	<listitem>
+	  <para>VIDIOC_DQEVENT</para>
+	</listitem>
+      </varlistentry>
+      <varlistentry>
+	<term><parameter>argp</parameter></term>
+	<listitem>
+	  <para></para>
+	</listitem>
+      </varlistentry>
+    </variablelist>
+  </refsect1>
+
+  <refsect1>
+    <title>Description</title>
+
+    <para>Dequeue an event from a video device. No input is required
+    for this ioctl. All the fields of the &v4l2-event; structure are
+    filled by the driver. The file handle will also receive exceptions
+    which the application may get by e.g. using the select system
+    call.</para>
+
+    <table frame="none" pgwide="1" id="v4l2-event">
+      <title>struct <structname>v4l2_event</structname></title>
+      <tgroup cols="4">
+	&cs-str;
+	<tbody valign="top">
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>type</structfield></entry>
+            <entry></entry>
+	    <entry>Type of the event.</entry>
+	  </row>
+	  <row>
+	    <entry>union</entry>
+	    <entry><structfield>u</structfield></entry>
+            <entry></entry>
+	    <entry></entry>
+	  </row>
+	  <row>
+	    <entry></entry>
+	    <entry>__u8</entry>
+            <entry><structfield>data</structfield>[64]</entry>
+	    <entry>Event data. Defined by the event type. The union
+            should be used to define easily accessible type for
+            events.</entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>pending</structfield></entry>
+            <entry></entry>
+	    <entry>Number of pending events excluding this one.</entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>sequence</structfield></entry>
+            <entry></entry>
+	    <entry>Event sequence number. The sequence number is
+	    incremented for every subscribed event that takes place.
+	    If sequence numbers are not contiguous it means that
+	    events have been lost.
+	    </entry>
+	  </row>
+	  <row>
+	    <entry>struct timeval</entry>
+	    <entry><structfield>timestamp</structfield></entry>
+            <entry></entry>
+	    <entry>Event timestamp.</entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>reserved</structfield>[9]</entry>
+            <entry></entry>
+	    <entry>Reserved for future extensions. Drivers must set
+	    the array to zero.</entry>
+	  </row>
+	</tbody>
+      </tgroup>
+    </table>
+
+  </refsect1>
+</refentry>
+<!--
+Local Variables:
+mode: sgml
+sgml-parent-document: "v4l2.sgml"
+indent-tabs-mode: nil
+End:
+-->

Documentation/DocBook/v4l/vidioc-subscribe-event.xml new file +43 -0

new file mode 100644
index 0000000..943de5c
@@ -0,0 +1,109 @@
+<refentry id="vidioc-subscribe-event">
+  <refmeta>
+    <refentrytitle>ioctl VIDIOC_SUBSCRIBE_EVENT, VIDIOC_UNSUBSCRIBE_EVENT</refentrytitle>
+    &manvol;
+  </refmeta>
+
+  <refnamediv>
+    <refname>VIDIOC_SUBSCRIBE_EVENT, VIDIOC_UNSUBSCRIBE_EVENT</refname>
+    <refpurpose>Subscribe or unsubscribe event</refpurpose>
+  </refnamediv>
+
+  <refsynopsisdiv>
+    <funcsynopsis>
+      <funcprototype>
+	<funcdef>int <function>ioctl</function></funcdef>
+	<paramdef>int <parameter>fd</parameter></paramdef>
+	<paramdef>int <parameter>request</parameter></paramdef>
+	<paramdef>struct v4l2_event_subscription
+*<parameter>argp</parameter></paramdef>
+      </funcprototype>
+    </funcsynopsis>
+  </refsynopsisdiv>
+
+  <refsect1>
+    <title>Arguments</title>
+
+    <variablelist>
+      <varlistentry>
+	<term><parameter>fd</parameter></term>
+	<listitem>
+	  <para>&fd;</para>
+	</listitem>
+      </varlistentry>
+      <varlistentry>
+	<term><parameter>request</parameter></term>
+	<listitem>
+	  <para>VIDIOC_SUBSCRIBE_EVENT, VIDIOC_UNSUBSCRIBE_EVENT</para>
+	</listitem>
+      </varlistentry>
+      <varlistentry>
+	<term><parameter>argp</parameter></term>
+	<listitem>
+	  <para></para>

Documentation/DocBook/v4l/vidioc-subscribe-event.xml new file +29 -0

+
+    <table frame="none" pgwide="1" id="v4l2-event-subscription">
+      <title>struct <structname>v4l2_event_subscription</structname></title>
+      <tgroup cols="3">
+	&cs-str;
+	<tbody valign="top">
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>type</structfield></entry>
+	    <entry>Type of the event.</entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>reserved</structfield>[7]</entry>
+	    <entry>Reserved for future extensions. Drivers must set
+	    the array to zero.</entry>
+	  </row>
+	</tbody>
+      </tgroup>
+    </table>
+
+    <table frame="none" pgwide="1" id="event-type">
+      <title>Event Types</title>
+      <tgroup cols="3">
+	&cs-def;
+	<tbody valign="top">
+	  <row>
+	    <entry><constant>V4L2_EVENT_ALL</constant></entry>
+	    <entry>0</entry>

Documentation/DocBook/v4l/vidioc-subscribe-event.xml new file +14 -0

+	  </row>
+	</tbody>
+      </tgroup>
+    </table>
+
+  </refsect1>
+</refentry>
+<!--
+Local Variables:
+mode: sgml
+sgml-parent-document: "v4l2.sgml"
+indent-tabs-mode: nil
+End:
+-->

Documentation/video4linux/v4l2-framework.txt +0 -0

index 08f9e59..7494a64 100644