[PATCH 06/15] Use hardware time where possible

Peter Hutterer peter.hutterer at who-t.net
Mon Jun 13 21:25:32 PDT 2011


On Thu, Jun 09, 2011 at 08:57:27PM +0100, Daniel Stone 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    |  139 ++++++++++++++++++++++++++--------------------------
>  src/synapticsstr.h |    1 +
>  src/synproto.h     |    2 +-
>  6 files changed, 75 insertions(+), 72 deletions(-)
> 
> 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 a4e6a47..8a6d56e 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
>   *
>   * Permission to use, copy, modify, distribute, and sell this software
>   * and its documentation for any purpose is hereby granted without
> @@ -121,7 +122,8 @@ 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,
> +                       Bool actual);
>  static int ControlProc(InputInfoPtr, xDeviceCtl*);
>  static int SwitchMode(ClientPtr, DeviceIntPtr, int);
>  static Bool DeviceInit(DeviceIntPtr);
> @@ -1202,19 +1204,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, FALSE);
>  
> -    /*
> -     * 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);
>  
> @@ -1251,18 +1246,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, TRUE);
>  	newDelay = TRUE;
>      }
>  
> -    if (newDelay)
> +    if (newDelay) {
> +        priv->timer_time = GetTimeInMillis();
>  	priv->timer = TimerSet(priv->timer, 0, delay, timerFunc, pInfo);
> +    }

indentation is busted here.


>  }
>  
>  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;
> @@ -1274,7 +1270,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) {
> @@ -1285,7 +1281,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);
>  
> @@ -1306,7 +1302,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);
>  
> @@ -1527,7 +1523,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;
> @@ -1548,35 +1545,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);
> @@ -1585,97 +1582,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;
> @@ -2190,7 +2187,7 @@ HandleScrolling(SynapticsPrivate *priv, struct SynapticsHwState *hw,
>      }
>  
>      if (priv->autoscroll_yspd) {
> -	double dtime = (hw->millis - HIST(0).millis) / 1000.0;
> +	double dtime = (hw->millis - priv->last_scroll_millis) / 1000.0;
>  	double ddy = para->coasting_friction * dtime;
>  	priv->autoscroll_y += priv->autoscroll_yspd * dtime;
>  	delay = MIN(delay, POLL_MS);
> @@ -2211,7 +2208,7 @@ HandleScrolling(SynapticsPrivate *priv, struct SynapticsHwState *hw,
>      }
>  
>      if (priv->autoscroll_xspd) {
> -	double dtime = (hw->millis - HIST(0).millis) / 1000.0;
> +	double dtime = (hw->millis - priv->last_scroll_millis) / 1000.0;
>  	double ddx = para->coasting_friction * dtime;
>  	priv->autoscroll_x += priv->autoscroll_xspd * dtime;
>  	delay = MIN(delay, POLL_MS);

are these two hunks supposed to be in a different patch?

> @@ -2332,7 +2329,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;
> @@ -2342,7 +2340,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){
> @@ -2383,7 +2381,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;
> @@ -2399,7 +2397,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;
> @@ -2407,7 +2405,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) {
> @@ -2420,7 +2418,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);
>  	}
>      }
> @@ -2435,7 +2433,8 @@ repeat_scrollbuttons(const InputInfoPtr pInfo,
>   * occurs.
>   */
>  static int
> -HandleState(InputInfoPtr pInfo, struct SynapticsHwState *hw)
> +HandleState(InputInfoPtr pInfo, struct SynapticsHwState *hw, CARD32 now,
> +            Bool actual)

"actual"?  maybe add a comment to tell what "actual" stands for.
also, it doesn't seem to be used anywhere in this patch.

>  {
>      SynapticsPrivate *priv = (SynapticsPrivate *) (pInfo->private);
>      SynapticsParameters *para = &priv->synpara;
> @@ -2486,7 +2485,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);
>  
> @@ -2498,7 +2497,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);
>  
> @@ -2588,7 +2587,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 d222cde..0174cdb 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 */
>  
> +    int timer_time;

comment missing

Cheers,
  Peter

>      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
> 



More information about the xorg-devel mailing list