Hi Mauro,
Mauro Carvalho Chehab wrote:
> Em 09-07-2010 12:31, Laurent Pinchart escreveu:
>> From: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxxxxxxxxxxxxx>
>>
>> Provide v4l2_subdevs with v4l2_event support. Subdev drivers only need very
>> little to support events.
>>
>> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxxxxxxxxxxxxx>
>> Signed-off-by: David Cohen <david.cohen@xxxxxxxxx>
>> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
>> ---
>> 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 -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.
Best regards,
--
Sakari Ailus
sakari.ailus@xxxxxxxxxxxxxxxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
