Hi Hans, On Wed, May 25, 2011 at 03:33:51PM +0200, Hans Verkuil wrote: > From: Hans Verkuil <hans.verkuil@xxxxxxxxx> > > Whenever a control changes value or state an event is sent to anyone > that subscribed to it. > > This functionality is useful for control panels but also for applications > that need to wait for (usually status) controls to change value. Thanks for the patch! I agree that it's good to pass more information of the control (min, max etc.) to the user space with the event. However, to support events arriving from interrupt context which we've discussed in the past, such information must be also accessible in those situations. What do you think about more fine-grained locking of controls, say, spinlock for each control (cluster) as an idea? > Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx> > --- > drivers/media/video/v4l2-ctrls.c | 101 ++++++++++++++++++++++++++++++ > drivers/media/video/v4l2-event.c | 126 +++++++++++++++++++++++++++----------- > drivers/media/video/v4l2-fh.c | 4 +- > include/linux/videodev2.h | 29 ++++++++- > include/media/v4l2-ctrls.h | 16 +++++- > include/media/v4l2-event.h | 2 + > 6 files changed, 237 insertions(+), 41 deletions(-) > > diff --git a/drivers/media/video/v4l2-ctrls.c b/drivers/media/video/v4l2-ctrls.c > index 3e0d8ab..e2a7ac7 100644 > --- a/drivers/media/video/v4l2-ctrls.c > +++ b/drivers/media/video/v4l2-ctrls.c > @@ -23,6 +23,7 @@ > #include <media/v4l2-ioctl.h> > #include <media/v4l2-device.h> > #include <media/v4l2-ctrls.h> > +#include <media/v4l2-event.h> > #include <media/v4l2-dev.h> > > #define call_op(master, op) \ > @@ -540,6 +541,44 @@ static bool type_is_int(const struct v4l2_ctrl *ctrl) > } > } > > +static void fill_event(struct v4l2_event *ev, struct v4l2_ctrl *ctrl, u32 changes) > +{ > + memset(ev->reserved, 0, sizeof(ev->reserved)); > + ev->type = V4L2_EVENT_CTRL; > + ev->id = ctrl->id; > + ev->u.ctrl.changes = changes; > + ev->u.ctrl.type = ctrl->type; > + ev->u.ctrl.flags = ctrl->flags; > + if (ctrl->type == V4L2_CTRL_TYPE_STRING) > + ev->u.ctrl.value64 = 0; > + else > + ev->u.ctrl.value64 = ctrl->cur.val64; > + ev->u.ctrl.minimum = ctrl->minimum; > + ev->u.ctrl.maximum = ctrl->maximum; > + if (ctrl->type == V4L2_CTRL_TYPE_MENU) > + ev->u.ctrl.step = 1; > + else > + ev->u.ctrl.step = ctrl->step; > + ev->u.ctrl.default_value = ctrl->default_value; > +} > + > +static void send_event(struct v4l2_fh *fh, struct v4l2_ctrl *ctrl, u32 changes) > +{ > + struct v4l2_event ev; > + struct v4l2_ctrl_fh *pos; > + > + if (list_empty(&ctrl->fhs)) > + return; > + fill_event(&ev, ctrl, changes); > + > + list_for_each_entry(pos, &ctrl->fhs, node) { > + if (pos->fh == fh) > + continue; > + ev.u.ctrl.flags = ctrl->flags; What's the purpose of setting flags here as well? fill_event() above already does it. > + v4l2_event_queue_fh(pos->fh, &ev); > + } > +} > + > /* Helper function: copy the current control value back to the caller */ > static int cur_to_user(struct v4l2_ext_control *c, > struct v4l2_ctrl *ctrl) Regards, -- Sakari Ailus sakari dot ailus at iki dot fi -- 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