On Fri, Jan 6, 2012 at 10:50 AM, John Stultz <john.stultz@xxxxxxxxxx> wrote:
> Here's another revision, incorperating Dmitry's suggestion.
>
> As noted by Arve and others, since wall time can jump backwards, it
> is difficult to use for input because one cannot determine if one
> event occured before another or for how long a key was pressed.
>
> However, the timestamp field is part of the kernel ABI, and cannot
> be changed without possibly breaking existing users.
>
> This patch adds a new IOCTL that allows a clockid to be set in
> the evdev_client struct that will specify which time base to
> use for event timestamps (ie: CLOCK_MONOTONIC instead
> of CLOCK_REALTIME).
>
> For now we only support CLOCK_MONOTONIC and CLOCK_REALTIME, but
> in the future we could support other clockids if appropriate.
What about CLOCK_MONOTONIC_RAW?
Last time we discussed, I thought this clock was the most useful for
use with input devices. But, you wrote this:
> So rawmonotonic isn't frequency corrected via NTP, while the monotonic
> clock is. So if you're calculating intervals, you will get more accurate
> times (where a second is a second) w/ ktime_get_ts().
Does this frequency correction involve timestamp jumps?
If so, of what magnitude?
In my experience, input event timestamp intervals are usually on the
order of a few milliseconds (5-25 ms). If CLOCK_MONOTONIC experiences
frequent adjustments near this order of magnitude, I still think
CLOCK_MONOTONIC_RAW might be a better choice for event timestamps.
>
> The default remains CLOCK_REALTIME, so we don't change the ABI.
>
> CC: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
> CC: Daniel Kurtz <djkurtz@xxxxxxxxxx>
> CC: linux-input@xxxxxxxxxxxxxxx
> CC: Arve Hjønnevåg <arve@xxxxxxxxxxx>
> Signed-off-by: John Stultz <john.stultz@xxxxxxxxxx>
> ---
> drivers/input/evdev.c | 33 +++++++++++++++++++++++++++++----
> include/linux/input.h | 2 ++
> 2 files changed, 31 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> index 4cf2534..4b71484 100644
> --- a/drivers/input/evdev.c
> +++ b/drivers/input/evdev.c
> @@ -46,6 +46,7 @@ struct evdev_client {
> struct fasync_struct *fasync;
> struct evdev *evdev;
> struct list_head node;
> + bool timestamp_clkid;
This isn't bool anymore.
> unsigned int bufsize;
> struct input_event buffer[];
> };
> @@ -54,8 +55,19 @@ static struct evdev *evdev_table[EVDEV_MINORS];
> static DEFINE_MUTEX(evdev_table_mutex);
>
> static void evdev_pass_event(struct evdev_client *client,
> - struct input_event *event)
> + struct input_event *event,
> + ktime_t mono, ktime_t real)
> {
> + struct timespec ts;
> +
> + if (client->timestamp_clkid == CLOCK_MONOTONIC)
> + ts = ktime_to_timespec(mono);
> + else
> + ts = ktime_to_timespec(real);
> + event->time.tv_sec = ts.tv_sec;
> + event->time.tv_usec = ts.tv_nsec / NSEC_PER_USEC;
> +
> +
> /* Interrupts are disabled, just acquire the lock. */
> spin_lock(&client->buffer_lock);
>
> @@ -94,8 +106,11 @@ static void evdev_event(struct input_handle *handle,
> struct evdev *evdev = handle->private;
> struct evdev_client *client;
> struct input_event event;
> + ktime_t time_mono, time_real;
> +
> + time_mono = ktime_get();
> + time_real = ktime_sub(time_mono, ktime_get_monotonic_offset());
>
> - do_gettimeofday(&event.time);
> event.type = type;
> event.code = code;
> event.value = value;
> @@ -103,11 +118,12 @@ static void evdev_event(struct input_handle *handle,
> rcu_read_lock();
>
> client = rcu_dereference(evdev->grab);
> +
> if (client)
> - evdev_pass_event(client, &event);
> + evdev_pass_event(client, &event, time_mono, time_real);
> else
> list_for_each_entry_rcu(client, &evdev->client_list, node)
> - evdev_pass_event(client, &event);
> + evdev_pass_event(client, &event, time_mono, time_real);
>
> rcu_read_unlock();
>
> @@ -683,6 +699,15 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
> else
> return evdev_ungrab(evdev, client);
>
> + case EVIOCCLOCKID:
> + if (copy_from_user(&i, p, sizeof(unsigned int)))
> + return -EFAULT;
> + if ((i == CLOCK_MONOTONIC) || (i == CLOCK_REALTIME)) {
> + client->timestamp_clkid = i;
> + return 0;
> + }
> + return -EINVAL;
> +
> case EVIOCGKEYCODE:
> return evdev_handle_get_keycode(dev, p);
>
> diff --git a/include/linux/input.h b/include/linux/input.h
> index 3862e32..9618e14 100644
> --- a/include/linux/input.h
> +++ b/include/linux/input.h
> @@ -129,6 +129,8 @@ struct input_keymap_entry {
>
> #define EVIOCGRAB _IOW('E', 0x90, int) /* Grab/Release device */
>
> +#define EVIOCCLOCKID _IOW('E', 0xA0, int) /* Set clockid to be used for timestamps */
> +
> /*
> * Device properties and quirks
> */
> --
> 1.7.3.2.146.gca209
>
Thanks,
-Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
