diff mbox

[001/001,Input:] Propagate hardware generated event timestamp to evdev.

Message ID CAJVTVr=rbkYHD93N4H9ANy1Hsivdd_XFLONXxOBp+TR9LfAyaQ@mail.gmail.com
State New, archived
Headers show

Commit Message

Alexander Levitskiy July 10, 2013, 8:38 p.m. UTC
From: Sasha Levitskiy <sanek@google.com>

Input: Propagate hardware event timestamp to evdev.

Convey hardware generated timestamp associated with the current event packet.
The use of these event codes by hardware drivers is optional.
Used to reduce jitter and improve velocity tracking in ABS_MT and other timing
sensitive devices.

kernel v. 3.4

Signed-off-by: Sasha Levitskiy <sanek@google.com>
---
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

dmitry.torokhov@gmail.com July 13, 2013, 6:44 a.m. UTC | #1
Hi Sasha,

On Wed, Jul 10, 2013 at 01:38:00PM -0700, Alexander Levitskiy wrote:
> From: Sasha Levitskiy <sanek@google.com>
> 
> Input: Propagate hardware event timestamp to evdev.
> 
> Convey hardware generated timestamp associated with the current event packet.
> The use of these event codes by hardware drivers is optional.
> Used to reduce jitter and improve velocity tracking in ABS_MT and other timing
> sensitive devices.
> 
> kernel v. 3.4
> 
> Signed-off-by: Sasha Levitskiy <sanek@google.com>
> ---
> diff --git a/Documentation/input/event-codes.txt
> b/Documentation/input/event-codes.txt
> index 53305bd..f0f0e07 100644
> --- a/Documentation/input/event-codes.txt
> +++ b/Documentation/input/event-codes.txt
> @@ -91,6 +91,15 @@ sent in the evdev event stream.
>      event and query the device (using EVIOCG* ioctls) to obtain its
>      current state.
> 
> +* SYN_TIME_SEC, SYN_TIME_NSEC:
> +  - Used to convey hardware timestamp associated with the current
> +    event packet. The use of these event codes by hardware drivers
> +    is optional. If used, the hardware driver should send the timestamp
> +    ahead of any other events associated with this packet. The timestamp
> +    should be adjusted to CLOCK_MONOTONIC base.
> +    This becomes useful for drivers of hardware that handle batching
> +    without involving the main CPU.

We already have MSC_TIMESTAMP (which is in usec), do you really need
nsec resolution?

> +
>  EV_KEY:
>  ----------
>  EV_KEY events take the form KEY_<name> or BTN_<name>. For example,
> KEY_A is used
> diff --git a/Documentation/input/multi-touch-protocol.txt
> b/Documentation/input/multi-touch-protocol.txt
> index 543101c..71af317 100644
> --- a/Documentation/input/multi-touch-protocol.txt
> +++ b/Documentation/input/multi-touch-protocol.txt
> @@ -80,6 +80,10 @@ Userspace can detect that a driver can report more
> total contacts than slots
>  by noting that the largest supported BTN_TOOL_*TAP event is larger than the
>  total number of type B slots reported in the absinfo for the ABS_MT_SLOT axis.
> 
> +Velocity tracking and temporal precision can be improved if device provides
> +exact timestamp for touches reported through SYN_TIME_SEC and SYN_TIME_NSEC.
> +The timestamp should be reported ahead of everything else in the packet.
> +
>  Protocol Example A
>  ------------------
> 
> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> index 41f79be..48baf6f 100644
> --- a/drivers/input/evdev.c
> +++ b/drivers/input/evdev.c
> @@ -37,6 +37,8 @@ struct evdev {
>   struct mutex mutex;
>   struct device dev;
>   bool exist;
> + int hw_ts_sec;
> + int hw_ts_nsec;
>  };
> 
>  struct evdev_client {
> @@ -109,7 +111,20 @@ static void evdev_event(struct input_handle *handle,
>   struct input_event event;
>   ktime_t time_mono, time_real;
> 
> - time_mono = ktime_get();
> + if (type == EV_SYN && code == SYN_TIME_SEC) {
> + evdev->hw_ts_sec = value;
> + return;

Why do you need this special handling? Can you simply have MSC_TIMESTAMP
to be delivered as part of the event packet and use it instead of the
event timestamp if userspace chooses to do so?

Thanks.
Michael Wright July 16, 2013, 11:22 p.m. UTC | #2
Apologies, forgot to make sure it was plain text.

On Tue, Jul 16, 2013 at 4:16 PM, Michael Wright <michaelwr@android.com> wrote:
> Hi Dmitry,
>
>
> On Fri, Jul 12, 2013 at 11:44 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
>> We already have MSC_TIMESTAMP (which is in usec), do you really need
>> nsec resolution?
>
> We don't actually need nsec resolution, usec resolution would be sufficient.
>
>
>> Why do you need this special handling? Can you simply have MSC_TIMESTAMP
>> to be delivered as part of the event packet and use it instead of the
>> event timestamp if userspace chooses to do so?
>
> We need stronger guarantees than MSC_TIMESTAMP gives us. Much of
> our system relies on the fact that timestamps produced by evdev are based on
> the systems monotonic clock, whereas MSC_TIMESTAMP provides no such
> guarantee.
> As for whether it's a MSC or SYN event, I don't think we really have
> preference.
>
> Thanks,
>
> Michael
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Levitskiy July 17, 2013, 8:31 p.m. UTC | #3
Dmitry,

I didn't see the MSC_TIMESTAMP before. I should have been more up to
date. I've looked at MSC_TIMESTAMP change and the first impression was
that it is something we can live with. After discussing details with
my colleagues here we came to a conclusion that:
a) we do need absolute time.
b) since we have to use 64-bit timestamp there is no point to discard
extra precision especially in light of event timestamp being of ns
precision.
c) there is no point to propagate the timestamp to userspace since
there is an existing mechanism in place.

I think the concept of propagating hardware timestamp in ABS_MT fits
nicely in the "simultaneous contact" model of multitouch operation.
I'd like to keep this functionality.

Thank you,
- Sasha.

On Fri, Jul 12, 2013 at 11:44 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Hi Sasha,
>
> On Wed, Jul 10, 2013 at 01:38:00PM -0700, Alexander Levitskiy wrote:
>> From: Sasha Levitskiy <sanek@google.com>
>>
>> Input: Propagate hardware event timestamp to evdev.
>>
>> Convey hardware generated timestamp associated with the current event packet.
>> The use of these event codes by hardware drivers is optional.
>> Used to reduce jitter and improve velocity tracking in ABS_MT and other timing
>> sensitive devices.
>>
>> kernel v. 3.4
>>
>> Signed-off-by: Sasha Levitskiy <sanek@google.com>
>> ---
>> diff --git a/Documentation/input/event-codes.txt
>> b/Documentation/input/event-codes.txt
>> index 53305bd..f0f0e07 100644
>> --- a/Documentation/input/event-codes.txt
>> +++ b/Documentation/input/event-codes.txt
>> @@ -91,6 +91,15 @@ sent in the evdev event stream.
>>      event and query the device (using EVIOCG* ioctls) to obtain its
>>      current state.
>>
>> +* SYN_TIME_SEC, SYN_TIME_NSEC:
>> +  - Used to convey hardware timestamp associated with the current
>> +    event packet. The use of these event codes by hardware drivers
>> +    is optional. If used, the hardware driver should send the timestamp
>> +    ahead of any other events associated with this packet. The timestamp
>> +    should be adjusted to CLOCK_MONOTONIC base.
>> +    This becomes useful for drivers of hardware that handle batching
>> +    without involving the main CPU.
>
> We already have MSC_TIMESTAMP (which is in usec), do you really need
> nsec resolution?
>
>> +
>>  EV_KEY:
>>  ----------
>>  EV_KEY events take the form KEY_<name> or BTN_<name>. For example,
>> KEY_A is used
>> diff --git a/Documentation/input/multi-touch-protocol.txt
>> b/Documentation/input/multi-touch-protocol.txt
>> index 543101c..71af317 100644
>> --- a/Documentation/input/multi-touch-protocol.txt
>> +++ b/Documentation/input/multi-touch-protocol.txt
>> @@ -80,6 +80,10 @@ Userspace can detect that a driver can report more
>> total contacts than slots
>>  by noting that the largest supported BTN_TOOL_*TAP event is larger than the
>>  total number of type B slots reported in the absinfo for the ABS_MT_SLOT axis.
>>
>> +Velocity tracking and temporal precision can be improved if device provides
>> +exact timestamp for touches reported through SYN_TIME_SEC and SYN_TIME_NSEC.
>> +The timestamp should be reported ahead of everything else in the packet.
>> +
>>  Protocol Example A
>>  ------------------
>>
>> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
>> index 41f79be..48baf6f 100644
>> --- a/drivers/input/evdev.c
>> +++ b/drivers/input/evdev.c
>> @@ -37,6 +37,8 @@ struct evdev {
>>   struct mutex mutex;
>>   struct device dev;
>>   bool exist;
>> + int hw_ts_sec;
>> + int hw_ts_nsec;
>>  };
>>
>>  struct evdev_client {
>> @@ -109,7 +111,20 @@ static void evdev_event(struct input_handle *handle,
>>   struct input_event event;
>>   ktime_t time_mono, time_real;
>>
>> - time_mono = ktime_get();
>> + if (type == EV_SYN && code == SYN_TIME_SEC) {
>> + evdev->hw_ts_sec = value;
>> + return;
>
> Why do you need this special handling? Can you simply have MSC_TIMESTAMP
> to be delivered as part of the event packet and use it instead of the
> event timestamp if userspace chooses to do so?
>
> Thanks.
>
> --
> Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/input/event-codes.txt
b/Documentation/input/event-codes.txt
index 53305bd..f0f0e07 100644
--- a/Documentation/input/event-codes.txt
+++ b/Documentation/input/event-codes.txt
@@ -91,6 +91,15 @@  sent in the evdev event stream.
     event and query the device (using EVIOCG* ioctls) to obtain its
     current state.

+* SYN_TIME_SEC, SYN_TIME_NSEC:
+  - Used to convey hardware timestamp associated with the current
+    event packet. The use of these event codes by hardware drivers
+    is optional. If used, the hardware driver should send the timestamp
+    ahead of any other events associated with this packet. The timestamp
+    should be adjusted to CLOCK_MONOTONIC base.
+    This becomes useful for drivers of hardware that handle batching
+    without involving the main CPU.
+
 EV_KEY:
 ----------
 EV_KEY events take the form KEY_<name> or BTN_<name>. For example,
KEY_A is used
diff --git a/Documentation/input/multi-touch-protocol.txt
b/Documentation/input/multi-touch-protocol.txt
index 543101c..71af317 100644
--- a/Documentation/input/multi-touch-protocol.txt
+++ b/Documentation/input/multi-touch-protocol.txt
@@ -80,6 +80,10 @@  Userspace can detect that a driver can report more
total contacts than slots
 by noting that the largest supported BTN_TOOL_*TAP event is larger than the
 total number of type B slots reported in the absinfo for the ABS_MT_SLOT axis.

+Velocity tracking and temporal precision can be improved if device provides
+exact timestamp for touches reported through SYN_TIME_SEC and SYN_TIME_NSEC.
+The timestamp should be reported ahead of everything else in the packet.
+
 Protocol Example A
 ------------------

diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index 41f79be..48baf6f 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -37,6 +37,8 @@  struct evdev {
  struct mutex mutex;
  struct device dev;
  bool exist;
+ int hw_ts_sec;
+ int hw_ts_nsec;
 };

 struct evdev_client {
@@ -109,7 +111,20 @@  static void evdev_event(struct input_handle *handle,
  struct input_event event;
  ktime_t time_mono, time_real;

- time_mono = ktime_get();
+ if (type == EV_SYN && code == SYN_TIME_SEC) {
+ evdev->hw_ts_sec = value;
+ return;
+ }
+ if (type == EV_SYN && code == SYN_TIME_NSEC) {
+ evdev->hw_ts_nsec = value;
+ return;
+ }
+
+ if (evdev->hw_ts_sec != -1 && evdev->hw_ts_nsec != -1)
+ time_mono = ktime_set(evdev->hw_ts_sec, evdev->hw_ts_nsec);
+ else
+ time_mono = ktime_get();
+
  time_real = ktime_sub(time_mono, ktime_get_monotonic_offset());

  event.type = type;
@@ -128,8 +143,11 @@  static void evdev_event(struct input_handle *handle,

  rcu_read_unlock();

- if (type == EV_SYN && code == SYN_REPORT)
+ if (type == EV_SYN && code == SYN_REPORT) {
+ evdev->hw_ts_sec = -1;
+ evdev->hw_ts_nsec = -1;
  wake_up_interruptible(&evdev->wait);
+ }
 }

 static int evdev_fasync(int fd, struct file *file, int on)
@@ -1031,6 +1049,8 @@  static int evdev_connect(struct input_handler
*handler, struct input_dev *dev,
  dev_set_name(&evdev->dev, "event%d", minor);
  evdev->exist = true;
  evdev->minor = minor;
+ evdev->hw_ts_sec = -1;
+ evdev->hw_ts_nsec = -1;

  evdev->handle.dev = input_get_device(dev);
  evdev->handle.name = dev_name(&evdev->dev);
diff --git a/drivers/input/input.c b/drivers/input/input.c
index aeccc69..378b0d1 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -223,6 +223,8 @@  static void input_handle_event(struct input_dev *dev,
  case EV_SYN:
  switch (code) {
  case SYN_CONFIG:
+ case SYN_TIME_SEC:
+ case SYN_TIME_NSEC:
  disposition = INPUT_PASS_TO_ALL;
  break;

diff --git a/include/linux/input.h b/include/linux/input.h
index 870e297..3d45b48 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -198,6 +198,8 @@  struct input_keymap_entry {
 #define SYN_CONFIG 1
 #define SYN_MT_REPORT 2
 #define SYN_DROPPED 3
+#define SYN_TIME_SEC 4
+#define SYN_TIME_NSEC 5

 /*
  * Keys and buttons