[PATCH synaptics v2 10/21] Use hardware time where possible
Daniel Kurtz
djkurtz at google.com
Tue Jun 14 22:42:13 PDT 2011
On Wed, Jun 15, 2011 at 1:05 AM, Daniel Stone <daniel at fooishbar.org> wrote:
> From: Derek Foreman <derek.foreman at collabora.co.uk>
>
> Rather than always setting hw->millis as the time when we received the
> event in our SIGIO handler, use the time provided by the kernel if
> applicable (i.e. if we're using evdev rather than PS/2 or similar).
>
> Signed-off-by: Derek Foreman <derek.foreman at collabora.co.uk>
> Reviewed-by: Daniel Stone <daniel at fooishbar.org>
> ---
> src/alpscomm.c | 2 +
> src/eventcomm.c | 1 +
> src/ps2comm.c | 2 +-
> src/synaptics.c | 133 +++++++++++++++++++++++++--------------------------
> src/synapticsstr.h | 1 +
> src/synproto.h | 2 +-
> 6 files changed, 71 insertions(+), 70 deletions(-)
>
> v2: Split out addition of 'actual' argument to HandleState into another patch.
>
> diff --git a/src/alpscomm.c b/src/alpscomm.c
> index dc76655..7ae2da1 100644
> --- a/src/alpscomm.c
> +++ b/src/alpscomm.c
> @@ -155,6 +155,8 @@ ALPS_process_packet(unsigned char *packet, struct SynapticsHwState *hw)
> int left = 0, right = 0, middle = 0;
> int i;
>
> + hw->millis = GetTimeInMillis();
> +
> x = (packet[1] & 0x7f) | ((packet[2] & 0x78) << (7-3));
> y = (packet[4] & 0x7f) | ((packet[3] & 0x70) << (7-4));
> z = packet[5];
> diff --git a/src/eventcomm.c b/src/eventcomm.c
> index 9cabd14..d70e735 100644
> --- a/src/eventcomm.c
> +++ b/src/eventcomm.c
> @@ -404,6 +404,7 @@ EventReadHwState(InputInfoPtr pInfo,
> switch (ev.code) {
> case SYN_REPORT:
> hw->numFingers = count_fingers(comm);
> + hw->millis = 1000 * ev.time.tv_sec + ev.time.tv_usec / 1000;
> *hwRet = *hw;
> return TRUE;
> }
> diff --git a/src/ps2comm.c b/src/ps2comm.c
> index 0bbae9b..3178175 100644
> --- a/src/ps2comm.c
> +++ b/src/ps2comm.c
> @@ -647,7 +647,7 @@ PS2ReadHwStateProto(InputInfoPtr pInfo,
> break;
> }
> }
> -
> + hw->millis = GetTimeInMillis();
> *hwRet = *hw;
> return TRUE;
> }
> diff --git a/src/synaptics.c b/src/synaptics.c
> index c14dbf8..1fd9cb8 100644
> --- a/src/synaptics.c
> +++ b/src/synaptics.c
> @@ -14,6 +14,7 @@
> * Copyright � 2007 Joseph P. Skudlarek
> * Copyright � 2008 Fedor P. Goncharov
> * Copyright � 2008-2009 Red Hat, Inc.
> + * Copyright � 2011 The Chromium Authors
The Chromium OS Authors
;)
> *
> * Permission to use, copy, modify, distribute, and sell this software
> * and its documentation for any purpose is hereby granted without
> @@ -134,7 +135,7 @@ static InputInfoPtr SynapticsPreInit(InputDriverPtr drv, IDevPtr dev, int flags)
> static void SynapticsUnInit(InputDriverPtr drv, InputInfoPtr pInfo, int flags);
> static Bool DeviceControl(DeviceIntPtr, int);
> static void ReadInput(InputInfoPtr);
> -static int HandleState(InputInfoPtr, struct SynapticsHwState*);
> +static int HandleState(InputInfoPtr, struct SynapticsHwState*, CARD32 now);
> static int ControlProc(InputInfoPtr, xDeviceCtl*);
> static int SwitchMode(ClientPtr, DeviceIntPtr, int);
> static Bool DeviceInit(DeviceIntPtr);
> @@ -1226,19 +1227,12 @@ timerFunc(OsTimerPtr timer, CARD32 now, pointer arg)
> sigstate = xf86BlockSIGIO();
>
> hw = priv->hwState;
> - hw.millis = now;
> - delay = HandleState(pInfo, &hw);
> + hw.millis += now - priv->timer_time;
> + priv->hwState.millis = hw.millis;
> + delay = HandleState(pInfo, &hw, hw.millis);
/* Update hwState time by time elapsed since this timer was started */
priv->hwState.millis += now - priv->timer_time;
hw = priv->hwState;
delay = HandleState(pInfo, &hw, hw.millis);
>
> - /*
> - * Workaround for wraparound bug in the TimerSet function. This bug is already
> - * fixed in CVS, but this driver needs to work with XFree86 versions 4.2.x and
> - * 4.3.x too.
> - */
> - wakeUpTime = now + delay;
> - if (wakeUpTime <= now)
> - wakeUpTime = 0xffffffffL;
> -
> - priv->timer = TimerSet(priv->timer, TimerAbsolute, wakeUpTime, timerFunc, pInfo);
> + priv->timer_time = now;
> + priv->timer = TimerSet(priv->timer, 0, delay, timerFunc, pInfo);
>
> xf86UnblockSIGIO(sigstate);
>
> @@ -1275,18 +1269,19 @@ ReadInput(InputInfoPtr pInfo)
> Bool newDelay = FALSE;
>
> while (SynapticsGetHwState(pInfo, priv, &hw)) {
> - hw.millis = GetTimeInMillis();
> priv->hwState = hw;
> - delay = HandleState(pInfo, &hw);
> + delay = HandleState(pInfo, &hw, hw.millis);
> newDelay = TRUE;
> }
>
> - if (newDelay)
> + if (newDelay) {
> + priv->timer_time = GetTimeInMillis();
> priv->timer = TimerSet(priv->timer, 0, delay, timerFunc, pInfo);
> + }
> }
>
> static int
> -HandleMidButtonEmulation(SynapticsPrivate *priv, struct SynapticsHwState *hw, int *delay)
> +HandleMidButtonEmulation(SynapticsPrivate *priv, struct SynapticsHwState *hw, CARD32 now, int *delay)
> {
> SynapticsParameters *para = &priv->synpara;
> Bool done = FALSE;
> @@ -1298,7 +1293,7 @@ HandleMidButtonEmulation(SynapticsPrivate *priv, struct SynapticsHwState *hw, in
> case MBE_LEFT_CLICK:
> case MBE_RIGHT_CLICK:
> case MBE_OFF:
> - priv->button_delay_millis = hw->millis;
> + priv->button_delay_millis = now;
> if (hw->left) {
> priv->mid_emu_state = MBE_LEFT;
> } else if (hw->right) {
> @@ -1309,7 +1304,7 @@ HandleMidButtonEmulation(SynapticsPrivate *priv, struct SynapticsHwState *hw, in
> break;
> case MBE_LEFT:
> timeleft = TIME_DIFF(priv->button_delay_millis + para->emulate_mid_button_time,
> - hw->millis);
> + now);
> if (timeleft > 0)
> *delay = MIN(*delay, timeleft);
>
> @@ -1330,7 +1325,7 @@ HandleMidButtonEmulation(SynapticsPrivate *priv, struct SynapticsHwState *hw, in
> break;
> case MBE_RIGHT:
> timeleft = TIME_DIFF(priv->button_delay_millis + para->emulate_mid_button_time,
> - hw->millis);
> + now);
> if (timeleft > 0)
> *delay = MIN(*delay, timeleft);
>
> @@ -1551,7 +1546,8 @@ GetTimeOut(SynapticsPrivate *priv)
>
> static int
> HandleTapProcessing(SynapticsPrivate *priv, struct SynapticsHwState *hw,
> - enum FingerState finger, Bool inside_active_area)
> + CARD32 now, enum FingerState finger,
> + Bool inside_active_area)
> {
> SynapticsParameters *para = &priv->synpara;
> Bool touch, release, is_timeout, move;
> @@ -1572,35 +1568,35 @@ HandleTapProcessing(SynapticsPrivate *priv, struct SynapticsHwState *hw,
> if (touch) {
> priv->touch_on.x = hw->x;
> priv->touch_on.y = hw->y;
> - priv->touch_on.millis = hw->millis;
> + priv->touch_on.millis = now;
> } else if (release) {
> - priv->touch_on.millis = hw->millis;
> + priv->touch_on.millis = now;
> }
> if (hw->z > para->finger_high)
> if (priv->tap_max_fingers < hw->numFingers)
> priv->tap_max_fingers = hw->numFingers;
> timeout = GetTimeOut(priv);
> - timeleft = TIME_DIFF(priv->touch_on.millis + timeout, hw->millis);
> + timeleft = TIME_DIFF(priv->touch_on.millis + timeout, now);
> is_timeout = timeleft <= 0;
>
> restart:
> switch (priv->tap_state) {
> case TS_START:
> if (touch)
> - SetTapState(priv, TS_1, hw->millis);
> + SetTapState(priv, TS_1, now);
> break;
> case TS_1:
> if (move) {
> - SetMovingState(priv, MS_TOUCHPAD_RELATIVE, hw->millis);
> - SetTapState(priv, TS_MOVE, hw->millis);
> + SetMovingState(priv, MS_TOUCHPAD_RELATIVE, now);
> + SetTapState(priv, TS_MOVE, now);
> goto restart;
> } else if (is_timeout) {
> if (finger == FS_TOUCHED) {
> - SetMovingState(priv, MS_TOUCHPAD_RELATIVE, hw->millis);
> + SetMovingState(priv, MS_TOUCHPAD_RELATIVE, now);
> } else if (finger == FS_PRESSED) {
> - SetMovingState(priv, MS_TRACKSTICK, hw->millis);
> + SetMovingState(priv, MS_TRACKSTICK, now);
> }
> - SetTapState(priv, TS_MOVE, hw->millis);
> + SetTapState(priv, TS_MOVE, now);
> goto restart;
> } else if (release) {
> edge = edge_detection(priv, priv->touch_on.x, priv->touch_on.y);
> @@ -1609,97 +1605,97 @@ HandleTapProcessing(SynapticsPrivate *priv, struct SynapticsHwState *hw,
> if (!inside_active_area) {
> priv->tap_button = 0;
> }
> - SetTapState(priv, TS_2A, hw->millis);
> + SetTapState(priv, TS_2A, now);
> }
> break;
> case TS_MOVE:
> if (move && priv->moving_state == MS_TRACKSTICK) {
> - SetMovingState(priv, MS_TOUCHPAD_RELATIVE, hw->millis);
> + SetMovingState(priv, MS_TOUCHPAD_RELATIVE, now);
> }
> if (release) {
> - SetMovingState(priv, MS_FALSE, hw->millis);
> - SetTapState(priv, TS_START, hw->millis);
> + SetMovingState(priv, MS_FALSE, now);
> + SetTapState(priv, TS_START, now);
> }
> break;
> case TS_2A:
> if (touch)
> - SetTapState(priv, TS_3, hw->millis);
> + SetTapState(priv, TS_3, now);
> else if (is_timeout)
> - SetTapState(priv, TS_SINGLETAP, hw->millis);
> + SetTapState(priv, TS_SINGLETAP, now);
> break;
> case TS_2B:
> if (touch) {
> - SetTapState(priv, TS_3, hw->millis);
> + SetTapState(priv, TS_3, now);
> } else if (is_timeout) {
> - SetTapState(priv, TS_START, hw->millis);
> + SetTapState(priv, TS_START, now);
> priv->tap_button_state = TBS_BUTTON_DOWN_UP;
> }
> break;
> case TS_SINGLETAP:
> if (touch)
> - SetTapState(priv, TS_1, hw->millis);
> + SetTapState(priv, TS_1, now);
> else if (is_timeout)
> - SetTapState(priv, TS_START, hw->millis);
> + SetTapState(priv, TS_START, now);
> break;
> case TS_3:
> if (move) {
> if (para->tap_and_drag_gesture) {
> - SetMovingState(priv, MS_TOUCHPAD_RELATIVE, hw->millis);
> - SetTapState(priv, TS_DRAG, hw->millis);
> + SetMovingState(priv, MS_TOUCHPAD_RELATIVE, now);
> + SetTapState(priv, TS_DRAG, now);
> } else {
> - SetTapState(priv, TS_1, hw->millis);
> + SetTapState(priv, TS_1, now);
> }
> goto restart;
> } else if (is_timeout) {
> if (para->tap_and_drag_gesture) {
> if (finger == FS_TOUCHED) {
> - SetMovingState(priv, MS_TOUCHPAD_RELATIVE, hw->millis);
> + SetMovingState(priv, MS_TOUCHPAD_RELATIVE, now);
> } else if (finger == FS_PRESSED) {
> - SetMovingState(priv, MS_TRACKSTICK, hw->millis);
> + SetMovingState(priv, MS_TRACKSTICK, now);
> }
> - SetTapState(priv, TS_DRAG, hw->millis);
> + SetTapState(priv, TS_DRAG, now);
> } else {
> - SetTapState(priv, TS_1, hw->millis);
> + SetTapState(priv, TS_1, now);
> }
> goto restart;
> } else if (release) {
> - SetTapState(priv, TS_2B, hw->millis);
> + SetTapState(priv, TS_2B, now);
> }
> break;
> case TS_DRAG:
> if (move)
> - SetMovingState(priv, MS_TOUCHPAD_RELATIVE, hw->millis);
> + SetMovingState(priv, MS_TOUCHPAD_RELATIVE, now);
> if (release) {
> - SetMovingState(priv, MS_FALSE, hw->millis);
> + SetMovingState(priv, MS_FALSE, now);
> if (para->locked_drags) {
> - SetTapState(priv, TS_4, hw->millis);
> + SetTapState(priv, TS_4, now);
> } else {
> - SetTapState(priv, TS_START, hw->millis);
> + SetTapState(priv, TS_START, now);
> }
> }
> break;
> case TS_4:
> if (is_timeout) {
> - SetTapState(priv, TS_START, hw->millis);
> + SetTapState(priv, TS_START, now);
> goto restart;
> }
> if (touch)
> - SetTapState(priv, TS_5, hw->millis);
> + SetTapState(priv, TS_5, now);
> break;
> case TS_5:
> if (is_timeout || move) {
> - SetTapState(priv, TS_DRAG, hw->millis);
> + SetTapState(priv, TS_DRAG, now);
> goto restart;
> } else if (release) {
> - SetMovingState(priv, MS_FALSE, hw->millis);
> - SetTapState(priv, TS_START, hw->millis);
> + SetMovingState(priv, MS_FALSE, now);
> + SetTapState(priv, TS_START, now);
> }
> break;
> }
>
> timeout = GetTimeOut(priv);
> if (timeout >= 0) {
> - timeleft = TIME_DIFF(priv->touch_on.millis + timeout, hw->millis);
> + timeleft = TIME_DIFF(priv->touch_on.millis + timeout, now);
> delay = clamp(timeleft, 1, delay);
> }
> return delay;
> @@ -2357,7 +2353,8 @@ adjust_state_from_scrollbuttons(const InputInfoPtr pInfo, struct SynapticsHwStat
> }
>
> static void
> -update_hw_button_state(const InputInfoPtr pInfo, struct SynapticsHwState *hw, int *delay)
> +update_hw_button_state(const InputInfoPtr pInfo, struct SynapticsHwState *hw,
> + CARD32 now, int *delay)
> {
> SynapticsPrivate *priv = (SynapticsPrivate *) (pInfo->private);
> SynapticsParameters *para = &priv->synpara;
> @@ -2367,7 +2364,7 @@ update_hw_button_state(const InputInfoPtr pInfo, struct SynapticsHwState *hw, in
> hw->down |= hw->multi[1];
>
> /* 3rd button emulation */
> - hw->middle |= HandleMidButtonEmulation(priv, hw, delay);
> + hw->middle |= HandleMidButtonEmulation(priv, hw, now, delay);
>
> /* Fingers emulate other buttons */
> if(hw->left && hw->numFingers >= 1){
> @@ -2408,7 +2405,7 @@ post_scroll_events(const InputInfoPtr pInfo, struct ScrollData scroll)
> static inline int
> repeat_scrollbuttons(const InputInfoPtr pInfo,
> const struct SynapticsHwState *hw,
> - int buttons, int delay)
> + int buttons, CARD32 now, int delay)
> {
> SynapticsPrivate *priv = (SynapticsPrivate *) (pInfo->private);
> SynapticsParameters *para = &priv->synpara;
> @@ -2424,7 +2421,7 @@ repeat_scrollbuttons(const InputInfoPtr pInfo,
> para->leftright_button_scrolling)) {
> priv->repeatButtons = buttons & rep_buttons;
> if (!priv->nextRepeat) {
> - priv->nextRepeat = hw->millis + repeat_delay * 2;
> + priv->nextRepeat = now + repeat_delay * 2;
> }
> } else {
> priv->repeatButtons = 0;
> @@ -2432,7 +2429,7 @@ repeat_scrollbuttons(const InputInfoPtr pInfo,
> }
>
> if (priv->repeatButtons) {
> - timeleft = TIME_DIFF(priv->nextRepeat, hw->millis);
> + timeleft = TIME_DIFF(priv->nextRepeat, now);
> if (timeleft > 0)
> delay = MIN(delay, timeleft);
> if (timeleft <= 0) {
> @@ -2445,7 +2442,7 @@ repeat_scrollbuttons(const InputInfoPtr pInfo,
> xf86PostButtonEvent(pInfo->dev, FALSE, id, TRUE, 0, 0);
> }
>
> - priv->nextRepeat = hw->millis + repeat_delay;
> + priv->nextRepeat = now + repeat_delay;
> delay = MIN(delay, repeat_delay);
> }
> }
> @@ -2460,7 +2457,7 @@ repeat_scrollbuttons(const InputInfoPtr pInfo,
> * occurs.
> */
> static int
> -HandleState(InputInfoPtr pInfo, struct SynapticsHwState *hw)
> +HandleState(InputInfoPtr pInfo, struct SynapticsHwState *hw, CARD32 now)
Why must you pass in hw.millis as "now"?
Why not just extract and truncate within HandleState:
CARD32 now = (CARD32)hw->millis;
Even better, why not handle this conversation in the three places
where it is currently required:
update_hw_button_state()
HandleTapProcessing()
repeat_scrollbuttons()
Or, at a very low level, like within TIME_DIFF(). This might make
most of this patch disappear.
> {
> SynapticsPrivate *priv = (SynapticsPrivate *) (pInfo->private);
> SynapticsParameters *para = &priv->synpara;
> @@ -2511,7 +2508,7 @@ HandleState(InputInfoPtr pInfo, struct SynapticsHwState *hw)
> }
>
> /* these two just update hw->left, right, etc. */
> - update_hw_button_state(pInfo, hw, &delay);
> + update_hw_button_state(pInfo, hw, now, &delay);
> if (priv->has_scrollbuttons)
> double_click = adjust_state_from_scrollbuttons(pInfo, hw);
>
> @@ -2523,7 +2520,7 @@ HandleState(InputInfoPtr pInfo, struct SynapticsHwState *hw)
>
> /* tap and drag detection. Needs to be performed even if the finger is in
> * the dead area to reset the state. */
> - timeleft = HandleTapProcessing(priv, hw, finger, inside_active_area);
> + timeleft = HandleTapProcessing(priv, hw, now, finger, inside_active_area);
> if (timeleft > 0)
> delay = MIN(delay, timeleft);
>
> @@ -2613,7 +2610,7 @@ HandleState(InputInfoPtr pInfo, struct SynapticsHwState *hw)
> }
>
> if (priv->has_scrollbuttons)
> - delay = repeat_scrollbuttons(pInfo, hw, buttons, delay);
> + delay = repeat_scrollbuttons(pInfo, hw, buttons, now, delay);
>
> /* Save old values of some state variables */
> priv->finger_state = finger;
> diff --git a/src/synapticsstr.h b/src/synapticsstr.h
> index b65069f..1290dc1 100644
> --- a/src/synapticsstr.h
> +++ b/src/synapticsstr.h
> @@ -182,6 +182,7 @@ typedef struct _SynapticsPrivateRec
> char *device; /* device node */
> Bool shm_config; /* True when shared memory area allocated */
>
> + unsigned long timer_time; /* when timer is set to fire */
CARD32 would more accurately reflect that this is 32-bit server time
(from GetTimeInMillis()), not (possibly 64-bit) kernel timestamp time.
This is actually when the timer was most recently _started_, not when
it is set to fire. I find the comment misleading.
> OsTimerPtr timer; /* for up/down-button repeat, tap processing, etc */
>
> struct CommData comm;
> diff --git a/src/synproto.h b/src/synproto.h
> index 75f90e4..25d8960 100644
> --- a/src/synproto.h
> +++ b/src/synproto.h
> @@ -36,7 +36,7 @@
> * A structure to describe the state of the touchpad hardware (buttons and pad)
> */
> struct SynapticsHwState {
> - int millis; /* Timestamp in milliseconds */
> + unsigned long millis; /* Timestamp in milliseconds */
> int x; /* X position of finger */
> int y; /* Y position of finger */
> int z; /* Finger pressure */
> --
> 1.7.5.3
>
>
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>
More information about the xorg-devel
mailing list