Re: [RFC/PATCH v2 6/7] v4l: subdev: Events support
Hi Mauro,
Mauro Carvalho Chehab wrote:
Em 09-07-2010 12:31, Laurent Pinchart escreveu:
From: Sakari Ailus sakari.ai...@maxwell.research.nokia.com
Provide v4l2_subdevs with v4l2_event support. Subdev drivers only need very
little to support events.
Signed-off-by: Sakari Ailus sakari.ai...@maxwell.research.nokia.com
Signed-off-by: David Cohen david.co...@nokia.com
Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
---
Documentation/video4linux/v4l2-framework.txt | 18 +++
drivers/media/video/v4l2-subdev.c| 71
+-
include/media/v4l2-subdev.h | 10
3 files changed, 98 insertions(+), 1 deletions(-)
diff --git a/Documentation/video4linux/v4l2-framework.txt
b/Documentation/video4linux/v4l2-framework.txt
index 9c3f33c..89bd881 100644
--- a/Documentation/video4linux/v4l2-framework.txt
+++ b/Documentation/video4linux/v4l2-framework.txt
@@ -347,6 +347,24 @@ VIDIOC_TRY_EXT_CTRLS
controls can be also be accessed through one (or several) V4L2 device
nodes.
+VIDIOC_DQEVENT
+VIDIOC_SUBSCRIBE_EVENT
+VIDIOC_UNSUBSCRIBE_EVENT
+
+The events ioctls are identical to the ones defined in V4L2. They
+behave identically, with the only exception that they deal only with
+events generated by the sub-device. Depending on the driver, those
+events can also be reported by one (or several) V4L2 device nodes.
+
+Sub-device drivers that want to use events need to set the
+V4L2_SUBDEV_USES_EVENTS v4l2_subdev::flags and initialize
+v4l2_subdev::nevents to events queue depth before registering the
+sub-device. After registration events can be queued as usual on the
+v4l2_subdev::devnode device node.
+
+To properly support events, the poll() file operation is also
+implemented.
+
I2C sub-device drivers
--
diff --git a/drivers/media/video/v4l2-subdev.c
b/drivers/media/video/v4l2-subdev.c
index 0ebd760..31bec67 100644
--- a/drivers/media/video/v4l2-subdev.c
+++ b/drivers/media/video/v4l2-subdev.c
@@ -18,26 +18,64 @@
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
USA
*/
-#include linux/types.h
#include linux/ioctl.h
+#include linux/slab.h
+#include linux/types.h
#include linux/videodev2.h
#include media/v4l2-device.h
#include media/v4l2-ioctl.h
+#include media/v4l2-fh.h
+#include media/v4l2-event.h
static int subdev_open(struct file *file)
{
struct video_device *vdev = video_devdata(file);
struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
+struct v4l2_fh *vfh;
+int ret;
if (!sd-initialized)
return -EAGAIN;
+vfh = kzalloc(sizeof(*vfh), GFP_KERNEL);
+if (vfh == NULL)
+return -ENOMEM;
+
+ret = v4l2_fh_init(vfh, vdev);
+if (ret)
+goto err;
Why to allocate/init the file handler for devices that don't need it?
I would just move the above logic to happen only if V4L2_SUBDEV_FL_HAS_EVENTS.
This patch actually adds subdev support for V4L2 file handles AND
events. v4l2_fh is also used to support probe formats on subdevs (not
contained in this patchset).
That v4l2_fh_init() function just initialises a few fields, there is no
allocation being done. The v4l2_fh structure will be part of v4l2_subdev_fh:
struct v4l2_subdev_fh {
struct v4l2_fh vfh;
struct v4l2_mbus_framefmt *probe_fmt;
struct v4l2_rect *probe_crop;
};
(Again not yet in this patchset.) The probe formats are a new concept to
allow trying formats across the whole pipeline for which the old try_fmt
wasn't suitable for: memory vs. bus format and pads matter.
+
+if (sd-flags V4L2_SUBDEV_FL_HAS_EVENTS) {
+ret = v4l2_event_init(vfh);
+if (ret)
+goto err;
+
+ret = v4l2_event_alloc(vfh, sd-nevents);
+if (ret)
+goto err;
+}
+
+v4l2_fh_add(vfh);
+file-private_data = vfh;
+
return 0;
+
+err:
+v4l2_fh_exit(vfh);
+kfree(vfh);
+
+return ret;
}
static int subdev_close(struct file *file)
{
+struct v4l2_fh *vfh = file-private_data;
+
+v4l2_fh_del(vfh);
+v4l2_fh_exit(vfh);
+kfree(vfh);
+
return 0;
}
@@ -45,6 +83,7 @@ static long subdev_do_ioctl(struct file *file, unsigned
int cmd, void *arg)
{
struct video_device *vdev = video_devdata(file);
struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
+struct v4l2_fh *fh = file-private_data;
switch (cmd) {
case VIDIOC_QUERYCTRL:
@@ -68,6 +107,18 @@ static long subdev_do_ioctl(struct file *file, unsigned
int cmd, void *arg)
case VIDIOC_TRY_EXT_CTRLS:
return v4l2_subdev_call(sd, core, try_ext_ctrls, arg);
+case VIDIOC_DQEVENT:
+if (!(sd-flags V4L2_SUBDEV_FL_HAS_EVENTS))
+return
Re: [RFC/PATCH v2 6/7] v4l: subdev: Events support
Hi Sakari,
On Monday 12 July 2010 13:06:34 Sakari Ailus wrote:
Mauro Carvalho Chehab wrote:
Em 09-07-2010 12:31, Laurent Pinchart escreveu:
[snip]
diff --git a/drivers/media/video/v4l2-subdev.c
b/drivers/media/video/v4l2-subdev.c index 0ebd760..31bec67 100644
--- a/drivers/media/video/v4l2-subdev.c
+++ b/drivers/media/video/v4l2-subdev.c
@@ -18,26 +18,64 @@
[snip]
static int subdev_open(struct file *file)
{
struct video_device *vdev = video_devdata(file);
struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
+ struct v4l2_fh *vfh;
+ int ret;
if (!sd-initialized)
return -EAGAIN;
+ vfh = kzalloc(sizeof(*vfh), GFP_KERNEL);
+ if (vfh == NULL)
+ return -ENOMEM;
+
+ ret = v4l2_fh_init(vfh, vdev);
+ if (ret)
+ goto err;
Why to allocate/init the file handler for devices that don't need it?
I would just move the above logic to happen only if
V4L2_SUBDEV_FL_HAS_EVENTS.
This patch actually adds subdev support for V4L2 file handles AND
events. v4l2_fh is also used to support probe formats on subdevs (not
contained in this patchset).
That v4l2_fh_init() function just initialises a few fields, there is no
allocation being done. The v4l2_fh structure will be part of
v4l2_subdev_fh:
struct v4l2_subdev_fh {
struct v4l2_fh vfh;
struct v4l2_mbus_framefmt *probe_fmt;
struct v4l2_rect *probe_crop;
};
(Again not yet in this patchset.) The probe formats are a new concept to
allow trying formats across the whole pipeline for which the old try_fmt
wasn't suitable for: memory vs. bus format and pads matter.
Mauro's point was that v4l2_fh isn't needed if the subdev doesn't support
events. subdev_fh will likely be mandatory, but that's for a future patch, so
I can make the v4l2_fh allocation optional for now.
[snip]
@@ -68,6 +107,18 @@ static long subdev_do_ioctl(struct file *file,
unsigned int cmd, void *arg)
case VIDIOC_TRY_EXT_CTRLS:
return v4l2_subdev_call(sd, core, try_ext_ctrls, arg);
+ case VIDIOC_DQEVENT:
+ if (!(sd-flags V4L2_SUBDEV_FL_HAS_EVENTS))
+ return -ENOIOCTLCMD;
+
+ return v4l2_event_dequeue(fh, arg, file-f_flags O_NONBLOCK);
+
+ case VIDIOC_SUBSCRIBE_EVENT:
+ return v4l2_subdev_call(sd, core, subscribe_event, fh, arg);
+
+ case VIDIOC_UNSUBSCRIBE_EVENT:
+ return v4l2_subdev_call(sd, core, unsubscribe_event, fh, arg);
Hmm... shouldn't it test also for V4L2_SUBDEV_FL_HAS_EVENTS?
I would do, instead:
if (fh) {
switch(cmd) {
/* FH events logic */
}
}
No need to. If the subdev doesn't support events it likely should not
define handlers for event specific calls, right? v4l2_subdev_call()
behaves well if the handler is NULL.
Both would work equally well, I guess.
The check in v4l2_subdev_call is fine, I don't think we need another check.
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v2 6/7] v4l: subdev: Events support
Em 09-07-2010 12:31, Laurent Pinchart escreveu:
From: Sakari Ailus sakari.ai...@maxwell.research.nokia.com
Provide v4l2_subdevs with v4l2_event support. Subdev drivers only need very
little to support events.
Signed-off-by: Sakari Ailus sakari.ai...@maxwell.research.nokia.com
Signed-off-by: David Cohen david.co...@nokia.com
Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
---
Documentation/video4linux/v4l2-framework.txt | 18 +++
drivers/media/video/v4l2-subdev.c| 71
+-
include/media/v4l2-subdev.h | 10
3 files changed, 98 insertions(+), 1 deletions(-)
diff --git a/Documentation/video4linux/v4l2-framework.txt
b/Documentation/video4linux/v4l2-framework.txt
index 9c3f33c..89bd881 100644
--- a/Documentation/video4linux/v4l2-framework.txt
+++ b/Documentation/video4linux/v4l2-framework.txt
@@ -347,6 +347,24 @@ VIDIOC_TRY_EXT_CTRLS
controls can be also be accessed through one (or several) V4L2 device
nodes.
+VIDIOC_DQEVENT
+VIDIOC_SUBSCRIBE_EVENT
+VIDIOC_UNSUBSCRIBE_EVENT
+
+ The events ioctls are identical to the ones defined in V4L2. They
+ behave identically, with the only exception that they deal only with
+ events generated by the sub-device. Depending on the driver, those
+ events can also be reported by one (or several) V4L2 device nodes.
+
+ Sub-device drivers that want to use events need to set the
+ V4L2_SUBDEV_USES_EVENTS v4l2_subdev::flags and initialize
+ v4l2_subdev::nevents to events queue depth before registering the
+ sub-device. After registration events can be queued as usual on the
+ v4l2_subdev::devnode device node.
+
+ To properly support events, the poll() file operation is also
+ implemented.
+
I2C sub-device drivers
--
diff --git a/drivers/media/video/v4l2-subdev.c
b/drivers/media/video/v4l2-subdev.c
index 0ebd760..31bec67 100644
--- a/drivers/media/video/v4l2-subdev.c
+++ b/drivers/media/video/v4l2-subdev.c
@@ -18,26 +18,64 @@
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/
-#include linux/types.h
#include linux/ioctl.h
+#include linux/slab.h
+#include linux/types.h
#include linux/videodev2.h
#include media/v4l2-device.h
#include media/v4l2-ioctl.h
+#include media/v4l2-fh.h
+#include media/v4l2-event.h
static int subdev_open(struct file *file)
{
struct video_device *vdev = video_devdata(file);
struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
+ struct v4l2_fh *vfh;
+ int ret;
if (!sd-initialized)
return -EAGAIN;
+ vfh = kzalloc(sizeof(*vfh), GFP_KERNEL);
+ if (vfh == NULL)
+ return -ENOMEM;
+
+ ret = v4l2_fh_init(vfh, vdev);
+ if (ret)
+ goto err;
Why to allocate/init the file handler for devices that don't need it?
I would just move the above logic to happen only if V4L2_SUBDEV_FL_HAS_EVENTS.
+
+ if (sd-flags V4L2_SUBDEV_FL_HAS_EVENTS) {
+ ret = v4l2_event_init(vfh);
+ if (ret)
+ goto err;
+
+ ret = v4l2_event_alloc(vfh, sd-nevents);
+ if (ret)
+ goto err;
+ }
+
+ v4l2_fh_add(vfh);
+ file-private_data = vfh;
+
return 0;
+
+err:
+ v4l2_fh_exit(vfh);
+ kfree(vfh);
+
+ return ret;
}
static int subdev_close(struct file *file)
{
+ struct v4l2_fh *vfh = file-private_data;
+
+ v4l2_fh_del(vfh);
+ v4l2_fh_exit(vfh);
+ kfree(vfh);
+
return 0;
}
@@ -45,6 +83,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int
cmd, void *arg)
{
struct video_device *vdev = video_devdata(file);
struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
+ struct v4l2_fh *fh = file-private_data;
switch (cmd) {
case VIDIOC_QUERYCTRL:
@@ -68,6 +107,18 @@ static long subdev_do_ioctl(struct file *file, unsigned
int cmd, void *arg)
case VIDIOC_TRY_EXT_CTRLS:
return v4l2_subdev_call(sd, core, try_ext_ctrls, arg);
+ case VIDIOC_DQEVENT:
+ if (!(sd-flags V4L2_SUBDEV_FL_HAS_EVENTS))
+ return -ENOIOCTLCMD;
+
+ return v4l2_event_dequeue(fh, arg, file-f_flags O_NONBLOCK);
+
+ case VIDIOC_SUBSCRIBE_EVENT:
+ return v4l2_subdev_call(sd, core, subscribe_event, fh, arg);
+
+ case VIDIOC_UNSUBSCRIBE_EVENT:
+ return v4l2_subdev_call(sd, core, unsubscribe_event, fh, arg);
Hmm... shouldn't it test also for V4L2_SUBDEV_FL_HAS_EVENTS?
I would do, instead:
if (fh) {
switch(cmd) {
/* FH events logic */
}
}
+
default:
return -ENOIOCTLCMD;
}
@@ -81,11 +132,29 @@ static long subdev_ioctl(struct file
[RFC/PATCH v2 6/7] v4l: subdev: Events support
From: Sakari Ailus sakari.ai...@maxwell.research.nokia.com
Provide v4l2_subdevs with v4l2_event support. Subdev drivers only need very
little to support events.
Signed-off-by: Sakari Ailus sakari.ai...@maxwell.research.nokia.com
Signed-off-by: David Cohen david.co...@nokia.com
Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
---
Documentation/video4linux/v4l2-framework.txt | 18 +++
drivers/media/video/v4l2-subdev.c| 71 +-
include/media/v4l2-subdev.h | 10
3 files changed, 98 insertions(+), 1 deletions(-)
diff --git a/Documentation/video4linux/v4l2-framework.txt
b/Documentation/video4linux/v4l2-framework.txt
index 9c3f33c..89bd881 100644
--- a/Documentation/video4linux/v4l2-framework.txt
+++ b/Documentation/video4linux/v4l2-framework.txt
@@ -347,6 +347,24 @@ VIDIOC_TRY_EXT_CTRLS
controls can be also be accessed through one (or several) V4L2 device
nodes.
+VIDIOC_DQEVENT
+VIDIOC_SUBSCRIBE_EVENT
+VIDIOC_UNSUBSCRIBE_EVENT
+
+ The events ioctls are identical to the ones defined in V4L2. They
+ behave identically, with the only exception that they deal only with
+ events generated by the sub-device. Depending on the driver, those
+ events can also be reported by one (or several) V4L2 device nodes.
+
+ Sub-device drivers that want to use events need to set the
+ V4L2_SUBDEV_USES_EVENTS v4l2_subdev::flags and initialize
+ v4l2_subdev::nevents to events queue depth before registering the
+ sub-device. After registration events can be queued as usual on the
+ v4l2_subdev::devnode device node.
+
+ To properly support events, the poll() file operation is also
+ implemented.
+
I2C sub-device drivers
--
diff --git a/drivers/media/video/v4l2-subdev.c
b/drivers/media/video/v4l2-subdev.c
index 0ebd760..31bec67 100644
--- a/drivers/media/video/v4l2-subdev.c
+++ b/drivers/media/video/v4l2-subdev.c
@@ -18,26 +18,64 @@
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/
-#include linux/types.h
#include linux/ioctl.h
+#include linux/slab.h
+#include linux/types.h
#include linux/videodev2.h
#include media/v4l2-device.h
#include media/v4l2-ioctl.h
+#include media/v4l2-fh.h
+#include media/v4l2-event.h
static int subdev_open(struct file *file)
{
struct video_device *vdev = video_devdata(file);
struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
+ struct v4l2_fh *vfh;
+ int ret;
if (!sd-initialized)
return -EAGAIN;
+ vfh = kzalloc(sizeof(*vfh), GFP_KERNEL);
+ if (vfh == NULL)
+ return -ENOMEM;
+
+ ret = v4l2_fh_init(vfh, vdev);
+ if (ret)
+ goto err;
+
+ if (sd-flags V4L2_SUBDEV_FL_HAS_EVENTS) {
+ ret = v4l2_event_init(vfh);
+ if (ret)
+ goto err;
+
+ ret = v4l2_event_alloc(vfh, sd-nevents);
+ if (ret)
+ goto err;
+ }
+
+ v4l2_fh_add(vfh);
+ file-private_data = vfh;
+
return 0;
+
+err:
+ v4l2_fh_exit(vfh);
+ kfree(vfh);
+
+ return ret;
}
static int subdev_close(struct file *file)
{
+ struct v4l2_fh *vfh = file-private_data;
+
+ v4l2_fh_del(vfh);
+ v4l2_fh_exit(vfh);
+ kfree(vfh);
+
return 0;
}
@@ -45,6 +83,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int
cmd, void *arg)
{
struct video_device *vdev = video_devdata(file);
struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
+ struct v4l2_fh *fh = file-private_data;
switch (cmd) {
case VIDIOC_QUERYCTRL:
@@ -68,6 +107,18 @@ static long subdev_do_ioctl(struct file *file, unsigned int
cmd, void *arg)
case VIDIOC_TRY_EXT_CTRLS:
return v4l2_subdev_call(sd, core, try_ext_ctrls, arg);
+ case VIDIOC_DQEVENT:
+ if (!(sd-flags V4L2_SUBDEV_FL_HAS_EVENTS))
+ return -ENOIOCTLCMD;
+
+ return v4l2_event_dequeue(fh, arg, file-f_flags O_NONBLOCK);
+
+ case VIDIOC_SUBSCRIBE_EVENT:
+ return v4l2_subdev_call(sd, core, subscribe_event, fh, arg);
+
+ case VIDIOC_UNSUBSCRIBE_EVENT:
+ return v4l2_subdev_call(sd, core, unsubscribe_event, fh, arg);
+
default:
return -ENOIOCTLCMD;
}
@@ -81,11 +132,29 @@ static long subdev_ioctl(struct file *file, unsigned int
cmd,
return video_usercopy(file, cmd, arg, subdev_do_ioctl);
}
+static unsigned int subdev_poll(struct file *file, poll_table *wait)
+{
+ struct video_device *vdev = video_devdata(file);
+ struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
+ struct v4l2_fh *fh = file-private_data;
+
+ if (!(sd-flags V4L2_SUBDEV_FL_HAS_EVENTS))
+ return POLLERR;
+
+
