Hi Hans,
On Tue, Feb 05, 2019 at 02:49:45PM +0100, Hans Verkuil wrote:
> This patch adds an extended version of VIDIOC_DQEVENT that:
>
> 1) is Y2038 safe by using a __u64 for the timestamp
> 2) needs no compat32 conversion code
> 3) is able to handle control events from 64-bit control types
> by changing the type of the minimum, maximum, step and default_value
> field to __u64
>
> All drivers and frameworks will be using this, and v4l2-ioctl.c would be the
> only place where the old event ioctl and structs are used.
>
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
> ---
> I chose to name this DQ_EXT_EVENT since the struct it dequeues is now called
> v4l2_ext_event. This is also consistent with the names of the G/S/TRY_EXT_CTRLS
> ioctls. An alternative could be VIDIOC_DQEXT_EVENT as that would be consistent
> with the lack of _ between DQ and EVENT in the current ioctl. But somehow it
> doesn't look right.
>
> Changes since v1:
> - rename ioctl from VIDIOC_DQEXTEVENT.
> - move the reserved array up to right after the union: this will allow us to
> extend the union into the reserved array if we ever need more than 64 bytes
> for the event payload (suggested by Sakari).
> ---
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 9a920f071ff9..301e3678bdb0 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -2303,6 +2303,37 @@ struct v4l2_event {
> __u32 reserved[8];
> };
>
> +struct v4l2_event_ext_ctrl {
> + __u32 changes;
> + __u32 type;
> + union {
> + __s32 value;
> + __s64 value64;
> + };
> + __s64 minimum;
> + __s64 maximum;
> + __s64 step;
> + __s64 default_value;
> + __u32 flags;
> +};
> +
> +struct v4l2_ext_event {
> + __u32 type;
> + __u32 id;
> + union {
> + struct v4l2_event_vsync vsync;
> + struct v4l2_event_ext_ctrl ctrl;
> + struct v4l2_event_frame_sync frame_sync;
> + struct v4l2_event_src_change src_change;
> + struct v4l2_event_motion_det motion_det;
> + __u8 data[64];
> + } u;
> + __u32 reserved[8];
> + __u64 timestamp;
> + __u32 pending;
> + __u32 sequence;
> +};
The size of the struct is at the moment 120 bytes. The allocation done by
the kernel is always 128 bytes anyway, and the ext control event above is
just 12 bytes short of the maximum. I'd therefore add two more reserved
fields. That's fine tuning though. The structs look very nice to me.
Acked-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> +
> #define V4L2_EVENT_SUB_FL_SEND_INITIAL (1 << 0)
> #define V4L2_EVENT_SUB_FL_ALLOW_FEEDBACK (1 << 1)
>
> @@ -2475,6 +2506,7 @@ struct v4l2_create_buffers {
> #define VIDIOC_DBG_G_CHIP_INFO _IOWR('V', 102, struct v4l2_dbg_chip_info)
>
> #define VIDIOC_QUERY_EXT_CTRL _IOWR('V', 103, struct v4l2_query_ext_ctrl)
> +#define VIDIOC_DQ_EXT_EVENT _IOR('V', 104, struct v4l2_ext_event)
>
> /* Reminder: when adding new ioctls please add support for them to
> drivers/media/v4l2-core/v4l2-compat-ioctl32.c as well! */
--
Sakari Ailus
sakari.ailus@xxxxxxxxxxxxxxx
