[PATCH 0/3] input: appletouch: fixes for jagged/uneven cursor movement

[PATCH 0/3] input: appletouch: fixes for jagged/uneven cursor movement

[Clinton Sprain]

Resubmitting the appletouch patches from last week with a couple of
updates, as no one's taken a look yet.

The appletouch driver can make some trackpads feel insensitive, with movement
that tends to jerk in steps. This is particularly evident when moving
diagonally. This can greatly hamper the trackpad's usability. These patches
address this by dialing back the fuzz and threshold parameters, by
implementing a new cursor movement smoothing algorithm and by discarding
movements that directly coincide with a change in the number of fingers
detected on the trackpad.

Revisions since last week:

Patch 1/3: Fuzz/threshold fix is identical, except for a typo (2 -> 3)

Patch 2/3: Smoothing algorithm is no longer inertia-based and is more
predictable / less complicated. I also dropped a change in atp_calculate_abs
that I couldn't definitively prove was beneficial.

Patch 3/3: (New) Addresses issues related to when a second finger enters
or leaves the trackpad, causing the cursor to jump or the page to scroll
unexpectedly; now, we discard any movement that happens at the exact moment
we detect a change in the number of fingers touching the trackpad.

Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>

 drivers/input/mouse/appletouch.c |  206 ++++++++++++++++++++++++++++++--------
 1 file changed, 162 insertions(+), 44 deletions(-)

-- 
1.7.9.5

[PATCH 1/3] input: appletouch: parametrize and set saner defaults for fuzz and threshold

[Clinton Sprain]

Dials back the default fuzz and threshold settings for
most devices using this driver, based on values from
user feedback from forums and bug reports. This
increases smoothness and movement granularity. No
changes were made for the older devices that use the
driver, as I did not find any user feedback for them
regarding these settings; however, the two settings can
also now both be specified as module parameters in case
there is a desire to change the values for devices that
do not have new defaults.

There are also a couple of style cleanups per checkpatch.pl.

Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
 drivers/input/mouse/appletouch.c |   66 ++++++++++++++++++++++++--------------
 1 file changed, 42 insertions(+), 24 deletions(-)

diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index e42f1fa..f5a16ee 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -43,12 +43,14 @@
  */
 struct atp_info {
 	int xsensors;				/* number of X sensors */
-	int xsensors_17;			/* 17" models have more sensors */
+	int xsensors_17;			/* 17" model has more sensors */
 	int ysensors;				/* number of Y sensors */
 	int xfact;				/* X multiplication factor */
 	int yfact;				/* Y multiplication factor */
 	int datalen;				/* size of USB transfers */
 	void (*callback)(struct urb *);		/* callback function */
+	int fuzz;				/* fuzz touchpad generates */
+	int threshold;				/* sensitivity threshold */
 };
 
 static void atp_complete_geyser_1_2(struct urb *urb);
@@ -62,6 +64,8 @@ static const struct atp_info fountain_info = {
 	.yfact		= 43,
 	.datalen	= 81,
 	.callback	= atp_complete_geyser_1_2,
+	.fuzz		= 16,
+	.threshold	= 5,
 };
 
 static const struct atp_info geyser1_info = {
@@ -72,6 +76,8 @@ static const struct atp_info geyser1_info = {
 	.yfact		= 43,
 	.datalen	= 81,
 	.callback	= atp_complete_geyser_1_2,
+	.fuzz		= 16,
+	.threshold	= 5,
 };
 
 static const struct atp_info geyser2_info = {
@@ -82,6 +88,8 @@ static const struct atp_info geyser2_info = {
 	.yfact		= 43,
 	.datalen	= 64,
 	.callback	= atp_complete_geyser_1_2,
+	.fuzz		= 0,
+	.threshold	= 3,
 };
 
 static const struct atp_info geyser3_info = {
@@ -91,6 +99,8 @@ static const struct atp_info geyser3_info = {
 	.yfact		= 64,
 	.datalen	= 64,
 	.callback	= atp_complete_geyser_3_4,
+	.fuzz		= 0,
+	.threshold	= 3,
 };
 
 static const struct atp_info geyser4_info = {
@@ -100,6 +110,8 @@ static const struct atp_info geyser4_info = {
 	.yfact		= 64,
 	.datalen	= 64,
 	.callback	= atp_complete_geyser_3_4,
+	.fuzz		= 0,
+	.threshold	= 3,
 };
 
 #define ATP_DEVICE(prod, info)					\
@@ -156,18 +168,9 @@ MODULE_DEVICE_TABLE(usb, atp_table);
 #define ATP_XSENSORS	26
 #define ATP_YSENSORS	16
 
-/* amount of fuzz this touchpad generates */
-#define ATP_FUZZ	16
-
 /* maximum pressure this driver will report */
 #define ATP_PRESSURE	300
 
-/*
- * Threshold for the touchpad sensors. Any change less than ATP_THRESHOLD is
- * ignored.
- */
-#define ATP_THRESHOLD	 5
-
 /* Geyser initialization constants */
 #define ATP_GEYSER_MODE_READ_REQUEST_ID		1
 #define ATP_GEYSER_MODE_WRITE_REQUEST_ID	9
@@ -213,14 +216,16 @@ struct atp {
 	struct work_struct	work;
 };
 
-#define dbg_dump(msg, tab) \
+#define dbg_dump(msg, tab)						\
+{									\
 	if (debug > 1) {						\
 		int __i;						\
 		printk(KERN_DEBUG "appletouch: %s", msg);		\
 		for (__i = 0; __i < ATP_XSENSORS + ATP_YSENSORS; __i++)	\
 			printk(" %02x", tab[__i]);			\
 		printk("\n");						\
-	}
+	}								\
+}
 
 #define dprintk(format, a...)						\
 	do {								\
@@ -236,14 +241,20 @@ MODULE_AUTHOR("Sven Anders");
 MODULE_DESCRIPTION("Apple PowerBook and MacBook USB touchpad driver");
 MODULE_LICENSE("GPL");
 
-/*
- * Make the threshold a module parameter
- */
-static int threshold = ATP_THRESHOLD;
+static int threshold = -1;
 module_param(threshold, int, 0644);
 MODULE_PARM_DESC(threshold, "Discard any change in data from a sensor"
 			    " (the trackpad has many of these sensors)"
-			    " less than this value.");
+			    " less than this value. Devices have defaults;"
+			    " this parameter overrides them.");
+static int fuzz = -1;
+
+module_param(fuzz, int, 0644);
+MODULE_PARM_DESC(fuzz, "Fuzz the trackpad generates. The higher"
+		       " this is, the less sensitive the trackpad"
+		       " will feel, but values too low may generate"
+		       " random movement. Devices have defaults;"
+		       " this parameter overrides them.");
 
 static int debug;
 module_param(debug, int, 0644);
@@ -363,7 +374,8 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
 		    (!is_increasing && xy_sensors[i - 1] < xy_sensors[i])) {
 			(*fingers)++;
 			is_increasing = 1;
-		} else if (i > 0 && (xy_sensors[i - 1] - xy_sensors[i] > threshold)) {
+		} else if (i > 0 && (xy_sensors[i - 1] -
+			xy_sensors[i] > threshold)) {
 			is_increasing = 0;
 		}
 
@@ -456,7 +468,7 @@ static void atp_detect_size(struct atp *dev)
 			input_set_abs_params(dev->input, ABS_X, 0,
 					     (dev->info->xsensors_17 - 1) *
 							dev->info->xfact - 1,
-					     ATP_FUZZ, 0);
+					     fuzz, 0);
 			break;
 		}
 	}
@@ -513,7 +525,8 @@ static void atp_complete_geyser_1_2(struct urb *urb)
 
 			/* Y values */
 			dev->xy_cur[ATP_XSENSORS + i] = dev->data[5 * i +  1];
-			dev->xy_cur[ATP_XSENSORS + i + 8] = dev->data[5 * i + 3];
+			dev->xy_cur[ATP_XSENSORS + i + 8] =
+				dev->data[5 * i + 3];
 		}
 	}
 
@@ -809,12 +822,17 @@ static int atp_probe(struct usb_interface *iface,
 	dev->info = info;
 	dev->overflow_warned = false;
 
+	if (fuzz < 0)
+		fuzz = dev->info->fuzz;
+	if (threshold < 0)
+		threshold = dev->info->threshold;
+
 	dev->urb = usb_alloc_urb(0, GFP_KERNEL);
 	if (!dev->urb)
 		goto err_free_devs;
 
-	dev->data = usb_alloc_coherent(dev->udev, dev->info->datalen, GFP_KERNEL,
-				       &dev->urb->transfer_dma);
+	dev->data = usb_alloc_coherent(dev->udev, dev->info->datalen,
+					GFP_KERNEL, &dev->urb->transfer_dma);
 	if (!dev->data)
 		goto err_free_urb;
 
@@ -844,10 +862,10 @@ static int atp_probe(struct usb_interface *iface,
 
 	input_set_abs_params(input_dev, ABS_X, 0,
 			     (dev->info->xsensors - 1) * dev->info->xfact - 1,
-			     ATP_FUZZ, 0);
+			     fuzz, 0);
 	input_set_abs_params(input_dev, ABS_Y, 0,
 			     (dev->info->ysensors - 1) * dev->info->yfact - 1,
-			     ATP_FUZZ, 0);
+			     fuzz, 0);
 	input_set_abs_params(input_dev, ABS_PRESSURE, 0, ATP_PRESSURE, 0, 0);
 
 	set_bit(EV_KEY, input_dev->evbit);
-- 
1.7.9.5

Re: [PATCH 1/3] input: appletouch: parametrize and set saner defaults for fuzz and threshold

[Henrik Rydberg]

Hi Clinton,

> Dials back the default fuzz and threshold settings for
> most devices using this driver, based on values from
> user feedback from forums and bug reports. This
> increases smoothness and movement granularity. No
> changes were made for the older devices that use the
> driver, as I did not find any user feedback for them
> regarding these settings; however, the two settings can
> also now both be specified as module parameters in case
> there is a desire to change the values for devices that
> do not have new defaults.
> 
> There are also a couple of style cleanups per checkpatch.pl.
> 
> Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
> ---
>  drivers/input/mouse/appletouch.c |   66 ++++++++++++++++++++++++--------------
>  1 file changed, 42 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
> index e42f1fa..f5a16ee 100644
> --- a/drivers/input/mouse/appletouch.c
> +++ b/drivers/input/mouse/appletouch.c
> @@ -43,12 +43,14 @@
>   */
>  struct atp_info {
>  	int xsensors;				/* number of X sensors */
> -	int xsensors_17;			/* 17" models have more sensors */
> +	int xsensors_17;			/* 17" model has more sensors */
>  	int ysensors;				/* number of Y sensors */
>  	int xfact;				/* X multiplication factor */
>  	int yfact;				/* Y multiplication factor */
>  	int datalen;				/* size of USB transfers */
>  	void (*callback)(struct urb *);		/* callback function */
> +	int fuzz;				/* fuzz touchpad generates */
> +	int threshold;				/* sensitivity threshold */
>  };
>  
>  static void atp_complete_geyser_1_2(struct urb *urb);
> @@ -62,6 +64,8 @@ static const struct atp_info fountain_info = {
>  	.yfact		= 43,
>  	.datalen	= 81,
>  	.callback	= atp_complete_geyser_1_2,
> +	.fuzz		= 16,
> +	.threshold	= 5,
>  };
>  
>  static const struct atp_info geyser1_info = {
> @@ -72,6 +76,8 @@ static const struct atp_info geyser1_info = {
>  	.yfact		= 43,
>  	.datalen	= 81,
>  	.callback	= atp_complete_geyser_1_2,
> +	.fuzz		= 16,
> +	.threshold	= 5,
>  };
>  
>  static const struct atp_info geyser2_info = {
> @@ -82,6 +88,8 @@ static const struct atp_info geyser2_info = {
>  	.yfact		= 43,
>  	.datalen	= 64,
>  	.callback	= atp_complete_geyser_1_2,
> +	.fuzz		= 0,
> +	.threshold	= 3,
>  };
>  
>  static const struct atp_info geyser3_info = {
> @@ -91,6 +99,8 @@ static const struct atp_info geyser3_info = {
>  	.yfact		= 64,
>  	.datalen	= 64,
>  	.callback	= atp_complete_geyser_3_4,
> +	.fuzz		= 0,
> +	.threshold	= 3,
>  };
>  
>  static const struct atp_info geyser4_info = {
> @@ -100,6 +110,8 @@ static const struct atp_info geyser4_info = {
>  	.yfact		= 64,
>  	.datalen	= 64,
>  	.callback	= atp_complete_geyser_3_4,
> +	.fuzz		= 0,
> +	.threshold	= 3,
>  };
>  
>  #define ATP_DEVICE(prod, info)					\
> @@ -156,18 +168,9 @@ MODULE_DEVICE_TABLE(usb, atp_table);
>  #define ATP_XSENSORS	26
>  #define ATP_YSENSORS	16
>  
> -/* amount of fuzz this touchpad generates */
> -#define ATP_FUZZ	16
> -
>  /* maximum pressure this driver will report */
>  #define ATP_PRESSURE	300
>  
> -/*
> - * Threshold for the touchpad sensors. Any change less than ATP_THRESHOLD is
> - * ignored.
> - */
> -#define ATP_THRESHOLD	 5
> -
>  /* Geyser initialization constants */
>  #define ATP_GEYSER_MODE_READ_REQUEST_ID		1
>  #define ATP_GEYSER_MODE_WRITE_REQUEST_ID	9
> @@ -213,14 +216,16 @@ struct atp {
>  	struct work_struct	work;
>  };
>  
> -#define dbg_dump(msg, tab) \
> +#define dbg_dump(msg, tab)						\
> +{									\
>  	if (debug > 1) {						\
>  		int __i;						\
>  		printk(KERN_DEBUG "appletouch: %s", msg);		\
>  		for (__i = 0; __i < ATP_XSENSORS + ATP_YSENSORS; __i++)	\
>  			printk(" %02x", tab[__i]);			\
>  		printk("\n");						\
> -	}
> +	}								\
> +}

Looks like the patch needs cleaning.

>  
>  #define dprintk(format, a...)						\
>  	do {								\
> @@ -236,14 +241,20 @@ MODULE_AUTHOR("Sven Anders");
>  MODULE_DESCRIPTION("Apple PowerBook and MacBook USB touchpad driver");
>  MODULE_LICENSE("GPL");
>  
> -/*
> - * Make the threshold a module parameter
> - */
> -static int threshold = ATP_THRESHOLD;
> +static int threshold = -1;
>  module_param(threshold, int, 0644);
>  MODULE_PARM_DESC(threshold, "Discard any change in data from a sensor"
>  			    " (the trackpad has many of these sensors)"
> -			    " less than this value.");
> +			    " less than this value. Devices have defaults;"
> +			    " this parameter overrides them.");
> +static int fuzz = -1;
> +
> +module_param(fuzz, int, 0644);
> +MODULE_PARM_DESC(fuzz, "Fuzz the trackpad generates. The higher"
> +		       " this is, the less sensitive the trackpad"
> +		       " will feel, but values too low may generate"
> +		       " random movement. Devices have defaults;"
> +		       " this parameter overrides them.");

The fuzz can be modified via the input subsystem ioctls (EVIOCSABS), so no need
to add a parameter here.

>  static int debug;
>  module_param(debug, int, 0644);
> @@ -363,7 +374,8 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
>  		    (!is_increasing && xy_sensors[i - 1] < xy_sensors[i])) {
>  			(*fingers)++;
>  			is_increasing = 1;
> -		} else if (i > 0 && (xy_sensors[i - 1] - xy_sensors[i] > threshold)) {
> +		} else if (i > 0 && (xy_sensors[i - 1] -
> +			xy_sensors[i] > threshold)) {
>  			is_increasing = 0;
>  		}

More noise, please clean the patchset.

> @@ -456,7 +468,7 @@ static void atp_detect_size(struct atp *dev)
>  			input_set_abs_params(dev->input, ABS_X, 0,
>  					     (dev->info->xsensors_17 - 1) *
>  							dev->info->xfact - 1,
> -					     ATP_FUZZ, 0);
> +					     fuzz, 0);

where is this variable defined?

>  			break;
>  		}
>  	}
> @@ -513,7 +525,8 @@ static void atp_complete_geyser_1_2(struct urb *urb)
>  
>  			/* Y values */
>  			dev->xy_cur[ATP_XSENSORS + i] = dev->data[5 * i +  1];
> -			dev->xy_cur[ATP_XSENSORS + i + 8] = dev->data[5 * i + 3];
> +			dev->xy_cur[ATP_XSENSORS + i + 8] =
> +				dev->data[5 * i + 3];
>  		}
>  	}
>  
> @@ -809,12 +822,17 @@ static int atp_probe(struct usb_interface *iface,
>  	dev->info = info;
>  	dev->overflow_warned = false;
>  
> +	if (fuzz < 0)
> +		fuzz = dev->info->fuzz;
> +	if (threshold < 0)
> +		threshold = dev->info->threshold;
> +
>  	dev->urb = usb_alloc_urb(0, GFP_KERNEL);
>  	if (!dev->urb)
>  		goto err_free_devs;
>  
> -	dev->data = usb_alloc_coherent(dev->udev, dev->info->datalen, GFP_KERNEL,
> -				       &dev->urb->transfer_dma);
> +	dev->data = usb_alloc_coherent(dev->udev, dev->info->datalen,
> +					GFP_KERNEL, &dev->urb->transfer_dma);
>  	if (!dev->data)
>  		goto err_free_urb;
>  
> @@ -844,10 +862,10 @@ static int atp_probe(struct usb_interface *iface,
>  
>  	input_set_abs_params(input_dev, ABS_X, 0,
>  			     (dev->info->xsensors - 1) * dev->info->xfact - 1,
> -			     ATP_FUZZ, 0);
> +			     fuzz, 0);
>  	input_set_abs_params(input_dev, ABS_Y, 0,
>  			     (dev->info->ysensors - 1) * dev->info->yfact - 1,
> -			     ATP_FUZZ, 0);
> +			     fuzz, 0);
>  	input_set_abs_params(input_dev, ABS_PRESSURE, 0, ATP_PRESSURE, 0, 0);
>  
>  	set_bit(EV_KEY, input_dev->evbit);
> 

Thanks,
Henrik

[PATCH 2/3] input: appletouch: use better cursor movement smoothing algorithm

[Clinton Sprain]

Implements a new algorithm used to smooth cursor
movement, using a sampling of the cursor's last three
positions to modulate the next cursor movement. This
further mitigates the 'stepping' users see when moving
the cursor diagonally. A 'legacy' module parameter has
been added in case the older devices do not like the
new algorithm.

Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>

---
 drivers/input/mouse/appletouch.c |   99 ++++++++++++++++++++++++++++++++------
 1 file changed, 83 insertions(+), 16 deletions(-)

diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index f5a16ee..4a3bbcd 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -209,13 +209,16 @@ struct atp {
 	bool			overflow_warned;
 	int			x_old;		/* last reported x/y, */
 	int			y_old;		/* used for smoothing */
+	int			x_older;	/* 2nd to last reported x/y,*/
+	int			y_older;	/* used for smoothing */
+	int			x_oldest;       /* 3rd to last reported x/y,*/
+	int			y_oldest;       /* used for smoothing */
 	signed char		xy_cur[ATP_XSENSORS + ATP_YSENSORS];
 	signed char		xy_old[ATP_XSENSORS + ATP_YSENSORS];
 	int			xy_acc[ATP_XSENSORS + ATP_YSENSORS];
 	int			idlecount;	/* number of empty packets */
 	struct work_struct	work;
 };
-
 #define dbg_dump(msg, tab)						\
 {									\
 	if (debug > 1) {						\
@@ -260,6 +263,13 @@ static int debug;
 module_param(debug, int, 0644);
 MODULE_PARM_DESC(debug, "Activate debugging output");
 
+static int legacy;
+module_param(legacy, int, 0644);
+MODULE_PARM_DESC(legacy, "1 = Use old cursor movement smoothing."
+			 " Older behavior averages current with"
+			 " last cursor position. New behavior"
+			 " uses cursor movement inertia.");
+
 /*
  * By default newer Geyser devices send standard USB HID mouse
  * packets (Report ID 2). This code changes device mode, so it
@@ -338,6 +348,49 @@ static void atp_reinit(struct work_struct *work)
 			retval);
 }
 
+static int atp_smooth_legacy(int curr, int old)
+{
+	return (old * 3 + curr) >> 2;
+}
+
+static int atp_smooth(int curr, int old, int older, int oldest)
+{
+	return (oldest + older * 2 + old * 4 + curr) >> 3;
+}
+
+static void atp_refresh_old_xy(int x, int y, struct atp *dev)
+{
+	if (legacy != true) {
+		dev->x_oldest = dev->x_older;
+		dev->y_oldest = dev->y_older;
+		dev->x_older = dev->x_old;
+		dev->y_older = dev->y_old;
+	}
+	dev->x_old = x;
+	dev->y_old = y;
+}
+
+static void atp_reset_old_xy(struct atp *dev)
+{
+	if (legacy != true) {
+		dev->x_oldest = dev->y_oldest = -1;
+		dev->x_older = dev->y_older = -1;
+	}
+	dev->x_old = dev->y_old = -1;
+}
+
+static void atp_default_old_xy(struct atp *dev)
+{
+	if (dev->x_older == -1)
+		dev->x_older = dev->x_old;
+	if (dev->y_older == -1)
+		dev->y_older = dev->y_old;
+	if (dev->x_oldest == -1)
+		dev->x_oldest = dev->x_older;
+	if (dev->y_oldest == -1)
+		dev->y_oldest = dev->y_older;
+}
+
 static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
 			     int *z, int *fingers)
 {
@@ -570,10 +623,18 @@ static void atp_complete_geyser_1_2(struct urb *urb)
 
 	if (x && y) {
 		if (dev->x_old != -1) {
-			x = (dev->x_old * 3 + x) >> 2;
-			y = (dev->y_old * 3 + y) >> 2;
-			dev->x_old = x;
-			dev->y_old = y;
+			if (legacy == true) {
+				x = atp_smooth_legacy(x, dev->x_old);
+				y = atp_smooth_legacy(y, dev->y_old);
+			} else {
+
+				atp_default_old_xy(dev);
+
+				x = atp_smooth(x, dev->x_old,
+					dev->x_older, dev->x_oldest);
+				y = atp_smooth(y, dev->y_old,
+					dev->y_older, dev->y_oldest);
+			}
 
 			if (debug > 1)
 				printk(KERN_DEBUG "appletouch: "
@@ -587,12 +648,11 @@ static void atp_complete_geyser_1_2(struct urb *urb)
 					 min(ATP_PRESSURE, x_z + y_z));
 			atp_report_fingers(dev->input, max(x_f, y_f));
 		}
-		dev->x_old = x;
-		dev->y_old = y;
+		atp_refresh_old_xy(x, y, dev);
 
 	} else if (!x && !y) {
 
-		dev->x_old = dev->y_old = -1;
+		atp_reset_old_xy(dev);
 		input_report_key(dev->input, BTN_TOUCH, 0);
 		input_report_abs(dev->input, ABS_PRESSURE, 0);
 		atp_report_fingers(dev->input, 0);
@@ -682,10 +742,18 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 
 	if (x && y) {
 		if (dev->x_old != -1) {
-			x = (dev->x_old * 3 + x) >> 2;
-			y = (dev->y_old * 3 + y) >> 2;
-			dev->x_old = x;
-			dev->y_old = y;
+			if (legacy == true) {
+				x = atp_smooth_legacy(x, dev->x_old);
+				y = atp_smooth_legacy(y, dev->y_old);
+			} else {
+
+				atp_default_old_xy(dev);
+
+				x = atp_smooth(x, dev->x_old,
+					dev->x_older, dev->x_oldest);
+				y = atp_smooth(y, dev->y_old,
+					dev->y_older, dev->y_oldest);
+			}
 
 			if (debug > 1)
 				printk(KERN_DEBUG "appletouch: X: %3d Y: %3d "
@@ -699,12 +767,11 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 					 min(ATP_PRESSURE, x_z + y_z));
 			atp_report_fingers(dev->input, max(x_f, y_f));
 		}
-		dev->x_old = x;
-		dev->y_old = y;
+		atp_refresh_old_xy(x, y, dev);
 
 	} else if (!x && !y) {
 
-		dev->x_old = dev->y_old = -1;
+		atp_reset_old_xy(dev);
 		input_report_key(dev->input, BTN_TOUCH, 0);
 		input_report_abs(dev->input, ABS_PRESSURE, 0);
 		atp_report_fingers(dev->input, 0);
@@ -730,7 +797,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 	if (!x && !y && !key) {
 		dev->idlecount++;
 		if (dev->idlecount == 10) {
-			dev->x_old = dev->y_old = -1;
+			atp_reset_old_xy(dev);
 			dev->idlecount = 0;
 			schedule_work(&dev->work);
 			/* Don't resubmit urb here, wait for reinit */
-- 
1.7.9.5

[PATCH 3/3] input: appletouch: fix jumps when additional fingers are detected

[Clinton Sprain]

Discard cursor movements if they directly coincide with
a change in the number of fingers detected. This helps
mitigate two issues - a sudden jump of the cursor when
a second finger enters or leaves the trackpad, and an
unexpected jump in page scrolling under the same
scenario. This doesn't completely eliminate all jumps
but does greatly reduce their frequency and velocity.

Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
 drivers/input/mouse/appletouch.c |   41 ++++++++++++++++++++++++++++++++++----
 1 file changed, 37 insertions(+), 4 deletions(-)

diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index 4a3bbcd..406054a 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -218,6 +218,7 @@ struct atp {
 	int			xy_acc[ATP_XSENSORS + ATP_YSENSORS];
 	int			idlecount;	/* number of empty packets */
 	struct work_struct	work;
+	int			fingers_old;	/* last # of fingers detected*/
 };
 #define dbg_dump(msg, tab)						\
 {									\
@@ -538,6 +539,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
 	int x, y, x_z, y_z, x_f, y_f;
 	int retval, i, j;
 	int key;
+	int fingers;
 	struct atp *dev = urb->context;
 	int status = atp_status_check(urb);
 
@@ -621,7 +623,16 @@ static void atp_complete_geyser_1_2(struct urb *urb)
 			      dev->info->yfact, &y_z, &y_f);
 	key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
 
-	if (x && y) {
+	fingers = max(x_f, y_f);
+
+	/*
+	 * Only act if we have valid x/y and # of fingers did not change.
+	 * If the # of fingers just changed, acting on the new x/y values will
+	 * cause the cursor to jump across the screen or the page to scroll
+	 * unexpectedly.
+	 */
+
+	if (x && y && (dev->fingers_old == fingers)) {
 		if (dev->x_old != -1) {
 			if (legacy == true) {
 				x = atp_smooth_legacy(x, dev->x_old);
@@ -646,7 +657,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
 			input_report_abs(dev->input, ABS_Y, y);
 			input_report_abs(dev->input, ABS_PRESSURE,
 					 min(ATP_PRESSURE, x_z + y_z));
-			atp_report_fingers(dev->input, max(x_f, y_f));
+			atp_report_fingers(dev->input, fingers);
 		}
 		atp_refresh_old_xy(x, y, dev);
 
@@ -661,6 +672,12 @@ static void atp_complete_geyser_1_2(struct urb *urb)
 		memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
 	}
 
+	/* if # of fingers changed, old x/y values are no longer useful */
+	if (dev->fingers_old != fingers)
+		atp_reset_old_xy(dev);
+
+	dev->fingers_old = fingers;
+
 	input_report_key(dev->input, BTN_LEFT, key);
 	input_sync(dev->input);
 
@@ -679,6 +696,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 	int x, y, x_z, y_z, x_f, y_f;
 	int retval, i, j;
 	int key;
+	int fingers;
 	struct atp *dev = urb->context;
 	int status = atp_status_check(urb);
 
@@ -740,7 +758,16 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 			      dev->info->yfact, &y_z, &y_f);
 	key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
 
-	if (x && y) {
+	fingers = max(x_f, y_f);
+
+	/*
+	 * Only act if we have valid x/y and # of fingers did not change.
+	 * If the # of fingers just changed, acting on the new x/y values will
+	 * cause the cursor to jump across the screen or the page to scroll
+	 * unexpectedly.
+	 */
+
+	if (x && y && (dev->fingers_old == fingers)) {
 		if (dev->x_old != -1) {
 			if (legacy == true) {
 				x = atp_smooth_legacy(x, dev->x_old);
@@ -765,7 +792,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 			input_report_abs(dev->input, ABS_Y, y);
 			input_report_abs(dev->input, ABS_PRESSURE,
 					 min(ATP_PRESSURE, x_z + y_z));
-			atp_report_fingers(dev->input, max(x_f, y_f));
+			atp_report_fingers(dev->input, fingers);
 		}
 		atp_refresh_old_xy(x, y, dev);
 
@@ -780,6 +807,12 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 		memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
 	}
 
+	/* if # of fingers changed, old x/y values are no longer useful */
+	if (dev->fingers_old != fingers)
+		atp_reset_old_xy(dev);
+
+	dev->fingers_old = fingers;
+
 	input_report_key(dev->input, BTN_LEFT, key);
 	input_sync(dev->input);
 
-- 
1.7.9.5

[PATCH v2 0/3] input: appletouch: fixes for jagged/uneven cursor movement

[Clinton Sprain]

These patches address some usability issues by dialing back the fuzz and threshold parameters, by implementing some smoothing for the sensor data and by discarding movements that directly coincide with a change in the number of fingers detected on the touchpad.

Changes since last time:

1/3: Remove extraneous cleanup - I'll submit a separate patch another day
2/3: Apply smoothing to the sensor data before position calculation, eliminating the need to toss values based on a threshold; simplify formula for averaging with prior values

Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>

 drivers/input/mouse/appletouch.c |  131 ++++++++++++++++++++++++++------------
 1 file changed, 92 insertions(+), 39 deletions(-)

[PATCH v2 1/3] input: appletouch: parametrize and set saner defaults for fuzz and threshold

[Clinton Sprain]

Dials back the default fuzz and threshold settings for most devices using this driver, based on user input from forums and bug reports, increasing sensitivity. These two settings can also now both be overridden as module parameters in the event that users have hardware that does not respond well to the new defaults, or there is a desire to change the values for devices that do not have new defaults.

Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
 drivers/input/mouse/appletouch.c |   48 ++++++++++++++++++++++++--------------
 1 file changed, 31 insertions(+), 17 deletions(-)

diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index 800ca7d..d48181b 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -48,6 +48,8 @@ struct atp_info {
 	int yfact;				/* Y multiplication factor */
 	int datalen;				/* size of USB transfers */
 	void (*callback)(struct urb *);		/* callback function */
+	int fuzz;				/* fuzz touchpad generates */
+	int threshold;				/* sensitivity threshold */
 };
 
 static void atp_complete_geyser_1_2(struct urb *urb);
@@ -61,6 +63,8 @@ static const struct atp_info fountain_info = {
 	.yfact		= 43,
 	.datalen	= 81,
 	.callback	= atp_complete_geyser_1_2,
+	.fuzz		= 16,
+	.threshold	= 5,
 };
 
 static const struct atp_info geyser1_info = {
@@ -71,6 +75,8 @@ static const struct atp_info geyser1_info = {
 	.yfact		= 43,
 	.datalen	= 81,
 	.callback	= atp_complete_geyser_1_2,
+	.fuzz		= 16,
+	.threshold	= 5,
 };
 
 static const struct atp_info geyser2_info = {
@@ -81,6 +87,8 @@ static const struct atp_info geyser2_info = {
 	.yfact		= 43,
 	.datalen	= 64,
 	.callback	= atp_complete_geyser_1_2,
+	.fuzz		= 0,
+	.threshold	= 3,
 };
 
 static const struct atp_info geyser3_info = {
@@ -90,6 +98,8 @@ static const struct atp_info geyser3_info = {
 	.yfact		= 64,
 	.datalen	= 64,
 	.callback	= atp_complete_geyser_3_4,
+	.fuzz		= 0,
+	.threshold	= 3,
 };
 
 static const struct atp_info geyser4_info = {
@@ -99,6 +109,8 @@ static const struct atp_info geyser4_info = {
 	.yfact		= 64,
 	.datalen	= 64,
 	.callback	= atp_complete_geyser_3_4,
+	.fuzz		= 0,
+	.threshold	= 3,
 };
 
 #define ATP_DEVICE(prod, info)					\
@@ -155,18 +167,9 @@ MODULE_DEVICE_TABLE(usb, atp_table);
 #define ATP_XSENSORS	26
 #define ATP_YSENSORS	16
 
-/* amount of fuzz this touchpad generates */
-#define ATP_FUZZ	16
-
 /* maximum pressure this driver will report */
 #define ATP_PRESSURE	300
 
-/*
- * Threshold for the touchpad sensors. Any change less than ATP_THRESHOLD is
- * ignored.
- */
-#define ATP_THRESHOLD	 5
-
 /* Geyser initialization constants */
 #define ATP_GEYSER_MODE_READ_REQUEST_ID		1
 #define ATP_GEYSER_MODE_WRITE_REQUEST_ID	9
@@ -235,14 +238,20 @@ MODULE_AUTHOR("Sven Anders");
 MODULE_DESCRIPTION("Apple PowerBook and MacBook USB touchpad driver");
 MODULE_LICENSE("GPL");
 
-/*
- * Make the threshold a module parameter
- */
-static int threshold = ATP_THRESHOLD;
+static int threshold = -1;
 module_param(threshold, int, 0644);
 MODULE_PARM_DESC(threshold, "Discard any change in data from a sensor"
 			    " (the trackpad has many of these sensors)"
-			    " less than this value.");
+			    " less than this value. Devices have defaults;"
+			    " this parameter overrides them.");
+static int fuzz = -1;
+
+module_param(fuzz, int, 0644);
+MODULE_PARM_DESC(fuzz, "Fuzz the trackpad generates. The higher"
+		       " this is, the less sensitive the trackpad"
+		       " will feel, but values too low may generate"
+		       " random movement. Devices have defaults;"
+		       " this parameter overrides them.");
 
 static int debug;
 module_param(debug, int, 0644);
@@ -455,7 +464,7 @@ static void atp_detect_size(struct atp *dev)
 			input_set_abs_params(dev->input, ABS_X, 0,
 					     (dev->info->xsensors_17 - 1) *
 							dev->info->xfact - 1,
-					     ATP_FUZZ, 0);
+					     fuzz, 0);
 			break;
 		}
 	}
@@ -808,6 +817,11 @@ static int atp_probe(struct usb_interface *iface,
 	dev->info = info;
 	dev->overflow_warned = false;
 
+	if (fuzz < 0)
+		fuzz = dev->info->fuzz;
+	if (threshold < 0)
+		threshold = dev->info->threshold;
+
 	dev->urb = usb_alloc_urb(0, GFP_KERNEL);
 	if (!dev->urb)
 		goto err_free_devs;
@@ -843,10 +857,10 @@ static int atp_probe(struct usb_interface *iface,
 
 	input_set_abs_params(input_dev, ABS_X, 0,
 			     (dev->info->xsensors - 1) * dev->info->xfact - 1,
-			     ATP_FUZZ, 0);
+			     fuzz, 0);
 	input_set_abs_params(input_dev, ABS_Y, 0,
 			     (dev->info->ysensors - 1) * dev->info->yfact - 1,
-			     ATP_FUZZ, 0);
+			     fuzz, 0);
 	input_set_abs_params(input_dev, ABS_PRESSURE, 0, ATP_PRESSURE, 0, 0);
 
 	set_bit(EV_KEY, input_dev->evbit);
-- 
1.7.9.5

Re: [PATCH v2 1/3] input: appletouch: parametrize and set saner defaults for fuzz and threshold

[Henrik Rydberg]

Hi Clinton,

On 02/07/2014 01:40 AM, Clinton Sprain wrote:
> Dials back the default fuzz and threshold settings for most devices using this driver, based on user input from forums and bug reports, increasing sensitivity. These two settings can also now both be overridden as module parameters in the event that users have hardware that does not respond well to the new defaults, or there is a desire to change the values for devices that do not have new defaults.
> 
> Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
> ---
>  drivers/input/mouse/appletouch.c |   48 ++++++++++++++++++++++++--------------
>  1 file changed, 31 insertions(+), 17 deletions(-)

Thanks for the patches, they seem like a good improvement. There are still some
comments, though, you will find them inline.

> diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
> index 800ca7d..d48181b 100644
> --- a/drivers/input/mouse/appletouch.c
> +++ b/drivers/input/mouse/appletouch.c
> @@ -48,6 +48,8 @@ struct atp_info {
>  	int yfact;				/* Y multiplication factor */
>  	int datalen;				/* size of USB transfers */
>  	void (*callback)(struct urb *);		/* callback function */
> +	int fuzz;				/* fuzz touchpad generates */
> +	int threshold;				/* sensitivity threshold */
>  };
>  
>  static void atp_complete_geyser_1_2(struct urb *urb);
> @@ -61,6 +63,8 @@ static const struct atp_info fountain_info = {
>  	.yfact		= 43,
>  	.datalen	= 81,
>  	.callback	= atp_complete_geyser_1_2,
> +	.fuzz		= 16,
> +	.threshold	= 5,
>  };
>  
>  static const struct atp_info geyser1_info = {
> @@ -71,6 +75,8 @@ static const struct atp_info geyser1_info = {
>  	.yfact		= 43,
>  	.datalen	= 81,
>  	.callback	= atp_complete_geyser_1_2,
> +	.fuzz		= 16,
> +	.threshold	= 5,
>  };
>  
>  static const struct atp_info geyser2_info = {
> @@ -81,6 +87,8 @@ static const struct atp_info geyser2_info = {
>  	.yfact		= 43,
>  	.datalen	= 64,
>  	.callback	= atp_complete_geyser_1_2,
> +	.fuzz		= 0,
> +	.threshold	= 3,
>  };
>  
>  static const struct atp_info geyser3_info = {
> @@ -90,6 +98,8 @@ static const struct atp_info geyser3_info = {
>  	.yfact		= 64,
>  	.datalen	= 64,
>  	.callback	= atp_complete_geyser_3_4,
> +	.fuzz		= 0,
> +	.threshold	= 3,
>  };
>  
>  static const struct atp_info geyser4_info = {
> @@ -99,6 +109,8 @@ static const struct atp_info geyser4_info = {
>  	.yfact		= 64,
>  	.datalen	= 64,
>  	.callback	= atp_complete_geyser_3_4,
> +	.fuzz		= 0,
> +	.threshold	= 3,
>  };
>  
>  #define ATP_DEVICE(prod, info)					\
> @@ -155,18 +167,9 @@ MODULE_DEVICE_TABLE(usb, atp_table);
>  #define ATP_XSENSORS	26
>  #define ATP_YSENSORS	16
>  
> -/* amount of fuzz this touchpad generates */
> -#define ATP_FUZZ	16
> -
>  /* maximum pressure this driver will report */
>  #define ATP_PRESSURE	300
>  
> -/*
> - * Threshold for the touchpad sensors. Any change less than ATP_THRESHOLD is
> - * ignored.
> - */
> -#define ATP_THRESHOLD	 5
> -
>  /* Geyser initialization constants */
>  #define ATP_GEYSER_MODE_READ_REQUEST_ID		1
>  #define ATP_GEYSER_MODE_WRITE_REQUEST_ID	9
> @@ -235,14 +238,20 @@ MODULE_AUTHOR("Sven Anders");
>  MODULE_DESCRIPTION("Apple PowerBook and MacBook USB touchpad driver");
>  MODULE_LICENSE("GPL");
>  
> -/*
> - * Make the threshold a module parameter
> - */
> -static int threshold = ATP_THRESHOLD;
> +static int threshold = -1;
>  module_param(threshold, int, 0644);
>  MODULE_PARM_DESC(threshold, "Discard any change in data from a sensor"
>  			    " (the trackpad has many of these sensors)"
> -			    " less than this value.");
> +			    " less than this value. Devices have defaults;"
> +			    " this parameter overrides them.");
> +static int fuzz = -1;
> +
> +module_param(fuzz, int, 0644);
> +MODULE_PARM_DESC(fuzz, "Fuzz the trackpad generates. The higher"
> +		       " this is, the less sensitive the trackpad"
> +		       " will feel, but values too low may generate"
> +		       " random movement. Devices have defaults;"
> +		       " this parameter overrides them.");

Given that there already is a userland interface for the fuzz parameter - and it
is indeed in frequent use - this one does not seem warranted. If we are unsure
if the changes to the default are not all good, perhaps they should not be
changed at all?

>  
>  static int debug;
>  module_param(debug, int, 0644);
> @@ -455,7 +464,7 @@ static void atp_detect_size(struct atp *dev)
>  			input_set_abs_params(dev->input, ABS_X, 0,
>  					     (dev->info->xsensors_17 - 1) *
>  							dev->info->xfact - 1,
> -					     ATP_FUZZ, 0);
> +					     fuzz, 0);
>  			break;
>  		}
>  	}
> @@ -808,6 +817,11 @@ static int atp_probe(struct usb_interface *iface,
>  	dev->info = info;
>  	dev->overflow_warned = false;
>  
> +	if (fuzz < 0)
> +		fuzz = dev->info->fuzz;
> +	if (threshold < 0)
> +		threshold = dev->info->threshold;
> +
>  	dev->urb = usb_alloc_urb(0, GFP_KERNEL);
>  	if (!dev->urb)
>  		goto err_free_devs;
> @@ -843,10 +857,10 @@ static int atp_probe(struct usb_interface *iface,
>  
>  	input_set_abs_params(input_dev, ABS_X, 0,
>  			     (dev->info->xsensors - 1) * dev->info->xfact - 1,
> -			     ATP_FUZZ, 0);
> +			     fuzz, 0);
>  	input_set_abs_params(input_dev, ABS_Y, 0,
>  			     (dev->info->ysensors - 1) * dev->info->yfact - 1,
> -			     ATP_FUZZ, 0);
> +			     fuzz, 0);
>  	input_set_abs_params(input_dev, ABS_PRESSURE, 0, ATP_PRESSURE, 0, 0);
>  
>  	set_bit(EV_KEY, input_dev->evbit);
> 

Thanks,
Henrik

Re: [PATCH v2 1/3] input: appletouch: parametrize and set saner defaults for fuzz and threshold

[Clinton Sprain]

Hi Herik,

On 02/09/2014 06:01 AM, Henrik Rydberg wrote:
> Hi Clinton,
> 
> On 02/07/2014 01:40 AM, Clinton Sprain wrote:
>> Dials back the default fuzz and threshold settings for most devices using this driver, based on user input from forums and bug reports, increasing sensitivity. These two settings can also now both be overridden as module parameters in the event that users have hardware that does not respond well to the new defaults, or there is a desire to change the values for devices that do not have new defaults.
>>
>> Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
>> ---
>>  drivers/input/mouse/appletouch.c |   48 ++++++++++++++++++++++++--------------
>>  1 file changed, 31 insertions(+), 17 deletions(-)
> 
> Thanks for the patches, they seem like a good improvement. There are still some
> comments, though, you will find them inline.
> 
>> diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
>> index 800ca7d..d48181b 100644
>> --- a/drivers/input/mouse/appletouch.c
>> +++ b/drivers/input/mouse/appletouch.c
>> @@ -48,6 +48,8 @@ struct atp_info {
>>  	int yfact;				/* Y multiplication factor */
>>  	int datalen;				/* size of USB transfers */
>>  	void (*callback)(struct urb *);		/* callback function */
>> +	int fuzz;				/* fuzz touchpad generates */
>> +	int threshold;				/* sensitivity threshold */
>>  };
>>  
>>  static void atp_complete_geyser_1_2(struct urb *urb);
>> @@ -61,6 +63,8 @@ static const struct atp_info fountain_info = {
>>  	.yfact		= 43,
>>  	.datalen	= 81,
>>  	.callback	= atp_complete_geyser_1_2,
>> +	.fuzz		= 16,
>> +	.threshold	= 5,
>>  };
>>  
>>  static const struct atp_info geyser1_info = {
>> @@ -71,6 +75,8 @@ static const struct atp_info geyser1_info = {
>>  	.yfact		= 43,
>>  	.datalen	= 81,
>>  	.callback	= atp_complete_geyser_1_2,
>> +	.fuzz		= 16,
>> +	.threshold	= 5,
>>  };
>>  
>>  static const struct atp_info geyser2_info = {
>> @@ -81,6 +87,8 @@ static const struct atp_info geyser2_info = {
>>  	.yfact		= 43,
>>  	.datalen	= 64,
>>  	.callback	= atp_complete_geyser_1_2,
>> +	.fuzz		= 0,
>> +	.threshold	= 3,
>>  };
>>  
>>  static const struct atp_info geyser3_info = {
>> @@ -90,6 +98,8 @@ static const struct atp_info geyser3_info = {
>>  	.yfact		= 64,
>>  	.datalen	= 64,
>>  	.callback	= atp_complete_geyser_3_4,
>> +	.fuzz		= 0,
>> +	.threshold	= 3,
>>  };
>>  
>>  static const struct atp_info geyser4_info = {
>> @@ -99,6 +109,8 @@ static const struct atp_info geyser4_info = {
>>  	.yfact		= 64,
>>  	.datalen	= 64,
>>  	.callback	= atp_complete_geyser_3_4,
>> +	.fuzz		= 0,
>> +	.threshold	= 3,
>>  };
>>  
>>  #define ATP_DEVICE(prod, info)					\
>> @@ -155,18 +167,9 @@ MODULE_DEVICE_TABLE(usb, atp_table);
>>  #define ATP_XSENSORS	26
>>  #define ATP_YSENSORS	16
>>  
>> -/* amount of fuzz this touchpad generates */
>> -#define ATP_FUZZ	16
>> -
>>  /* maximum pressure this driver will report */
>>  #define ATP_PRESSURE	300
>>  
>> -/*
>> - * Threshold for the touchpad sensors. Any change less than ATP_THRESHOLD is
>> - * ignored.
>> - */
>> -#define ATP_THRESHOLD	 5
>> -
>>  /* Geyser initialization constants */
>>  #define ATP_GEYSER_MODE_READ_REQUEST_ID		1
>>  #define ATP_GEYSER_MODE_WRITE_REQUEST_ID	9
>> @@ -235,14 +238,20 @@ MODULE_AUTHOR("Sven Anders");
>>  MODULE_DESCRIPTION("Apple PowerBook and MacBook USB touchpad driver");
>>  MODULE_LICENSE("GPL");
>>  
>> -/*
>> - * Make the threshold a module parameter
>> - */
>> -static int threshold = ATP_THRESHOLD;
>> +static int threshold = -1;
>>  module_param(threshold, int, 0644);
>>  MODULE_PARM_DESC(threshold, "Discard any change in data from a sensor"
>>  			    " (the trackpad has many of these sensors)"
>> -			    " less than this value.");
>> +			    " less than this value. Devices have defaults;"
>> +			    " this parameter overrides them.");
>> +static int fuzz = -1;
>> +
>> +module_param(fuzz, int, 0644);
>> +MODULE_PARM_DESC(fuzz, "Fuzz the trackpad generates. The higher"
>> +		       " this is, the less sensitive the trackpad"
>> +		       " will feel, but values too low may generate"
>> +		       " random movement. Devices have defaults;"
>> +		       " this parameter overrides them.");
> 
> Given that there already is a userland interface for the fuzz parameter - and it
> is indeed in frequent use - this one does not seem warranted. If we are unsure
> if the changes to the default are not all good, perhaps they should not be
> changed at all?

Forgive my ignorance - how would I interact with this parameter in
userland? I searched a bit but didn't find anything. If it's trivial to
fiddle with then I'm OK with dropping it as a module parameter.

I'm reasonably confident the defaults should change. They are based on
positive comments from multiple people with different hardware who
recompiled the driver with these values.

The majority of the benefit they saw was likely from the smaller
threshold value, due to its influence on the return value of
atp_calculate_abs. The second patch of this set takes it completely out
of the equation; however, there are still some small benefits to be
gained from changing these defaults. The threshold is still used for
finger count calculation and benefits from the increased sensitivity,
and the fuzz setting seems to inhibit more granular (1-2 px) cursor
movements.

> 
>>  
>>  static int debug;
>>  module_param(debug, int, 0644);
>> @@ -455,7 +464,7 @@ static void atp_detect_size(struct atp *dev)
>>  			input_set_abs_params(dev->input, ABS_X, 0,
>>  					     (dev->info->xsensors_17 - 1) *
>>  							dev->info->xfact - 1,
>> -					     ATP_FUZZ, 0);
>> +					     fuzz, 0);
>>  			break;
>>  		}
>>  	}
>> @@ -808,6 +817,11 @@ static int atp_probe(struct usb_interface *iface,
>>  	dev->info = info;
>>  	dev->overflow_warned = false;
>>  
>> +	if (fuzz < 0)
>> +		fuzz = dev->info->fuzz;
>> +	if (threshold < 0)
>> +		threshold = dev->info->threshold;
>> +
>>  	dev->urb = usb_alloc_urb(0, GFP_KERNEL);
>>  	if (!dev->urb)
>>  		goto err_free_devs;
>> @@ -843,10 +857,10 @@ static int atp_probe(struct usb_interface *iface,
>>  
>>  	input_set_abs_params(input_dev, ABS_X, 0,
>>  			     (dev->info->xsensors - 1) * dev->info->xfact - 1,
>> -			     ATP_FUZZ, 0);
>> +			     fuzz, 0);
>>  	input_set_abs_params(input_dev, ABS_Y, 0,
>>  			     (dev->info->ysensors - 1) * dev->info->yfact - 1,
>> -			     ATP_FUZZ, 0);
>> +			     fuzz, 0);
>>  	input_set_abs_params(input_dev, ABS_PRESSURE, 0, ATP_PRESSURE, 0, 0);
>>  
>>  	set_bit(EV_KEY, input_dev->evbit);
>>
> 
> Thanks,
> Henrik
> 

Thanks,
Clinton

[PATCH v2 2/3] input: appletouch: use better cursor movement smoothing algorithm

[Clinton Sprain]

Use smoothed version of sensor array data to calculate movement and add weight to prior values when calculating average. This gives more granular and more predictable movement.

Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
 drivers/input/mouse/appletouch.c |   60 ++++++++++++++++++++++++++------------
 1 file changed, 42 insertions(+), 18 deletions(-)

diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index d48181b..edbdd95 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -338,21 +338,51 @@ static void atp_reinit(struct work_struct *work)
 static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
 			     int *z, int *fingers)
 {
-	int i;
+	int i, k;
+	int smooth[nb_sensors + 2];
+	int smooth_tmp[nb_sensors + 2];
+
 	/* values to calculate mean */
 	int pcum = 0, psum = 0;
 	int is_increasing = 0;
 
 	*fingers = 0;
 
+	/*
+	 * Use a smoothed version of sensor data for movement calculations, to
+	 * combat noise without needing to toss out values based on a threshold.
+	 * This improves tracking. Finger count is calculated separately based
+	 * on raw data.
+	 */
+
+	for (i = 0; i < nb_sensors; i++) {           /* Scale up to minimize */
+		smooth[i] = xy_sensors[i] << 12;     /* rounding/truncation. */
+	}
+
+	/* Mitigate some of the data loss from smoothing on the edge sensors. */
+	smooth[-1] = smooth[0] >> 2;
+	smooth[nb_sensors] = smooth[nb_sensors - 1] >> 2;
+
+	for (k = 0; k < 4; k++) {
+		for (i = 0; i < nb_sensors; i++) {   /* Blend with neighbors. */
+			smooth_tmp[i] = (smooth[i - 1] + smooth[i] * 2 + smooth[i + 1]) >> 2;
+		}
+
+		for (i = 0; i < nb_sensors; i++) {
+			smooth[i] = smooth_tmp[i];
+			if (k == 3)     /* Last go-round, so scale back down. */
+				smooth[i] = smooth[i] >> 12;
+		}
+
+		smooth[-1] = smooth[0] >> 2;
+		smooth[nb_sensors] = smooth[nb_sensors - 1] >> 2;
+	}
+
 	for (i = 0; i < nb_sensors; i++) {
 		if (xy_sensors[i] < threshold) {
 			if (is_increasing)
 				is_increasing = 0;
 
-			continue;
-		}
-
 		/*
 		 * Makes the finger detection more versatile.  For example,
 		 * two fingers with no gap will be detected.  Also, my
@@ -367,7 +397,8 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
 		 *
 		 * - Jason Parekh <jasonparekh@gmail.com>
 		 */
-		if (i < 1 ||
+
+		} else if (i < 1 ||
 		    (!is_increasing && xy_sensors[i - 1] < xy_sensors[i])) {
 			(*fingers)++;
 			is_increasing = 1;
@@ -375,15 +406,8 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
 			is_increasing = 0;
 		}
 
-		/*
-		 * Subtracts threshold so a high sensor that just passes the
-		 * threshold won't skew the calculated absolute coordinate.
-		 * Fixes an issue where slowly moving the mouse would
-		 * occasionally jump a number of pixels (slowly moving the
-		 * finger makes this issue most apparent.)
-		 */
-		pcum += (xy_sensors[i] - threshold) * i;
-		psum += (xy_sensors[i] - threshold);
+		pcum += (smooth[i]) * i;
+		psum += (smooth[i]);
 	}
 
 	if (psum > 0) {
@@ -565,8 +589,8 @@ static void atp_complete_geyser_1_2(struct urb *urb)
 
 	if (x && y) {
 		if (dev->x_old != -1) {
-			x = (dev->x_old * 3 + x) >> 2;
-			y = (dev->y_old * 3 + y) >> 2;
+			x = (dev->x_old * 7 + x) >> 3;
+			y = (dev->y_old * 7 + y) >> 3;
 			dev->x_old = x;
 			dev->y_old = y;
 
@@ -677,8 +701,8 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 
 	if (x && y) {
 		if (dev->x_old != -1) {
-			x = (dev->x_old * 3 + x) >> 2;
-			y = (dev->y_old * 3 + y) >> 2;
+			x = (dev->x_old * 7 + x) >> 3;
+			y = (dev->y_old * 7 + y) >> 3;
 			dev->x_old = x;
 			dev->y_old = y;
 
-- 
1.7.9.5

Re: [PATCH v2 2/3] input: appletouch: use better cursor movement smoothing algorithm

[Henrik Rydberg]

Hi Clinton,

> Use smoothed version of sensor array data to calculate movement and add weight
to prior values when calculating average. This gives more granular and more
predictable movement.
> 
> Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
> ---
>  drivers/input/mouse/appletouch.c |   60 ++++++++++++++++++++++++++------------
>  1 file changed, 42 insertions(+), 18 deletions(-)

I like this patch, but there are some technical glitches, comments below.

> diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
> index d48181b..edbdd95 100644
> --- a/drivers/input/mouse/appletouch.c
> +++ b/drivers/input/mouse/appletouch.c
> @@ -338,21 +338,51 @@ static void atp_reinit(struct work_struct *work)
>  static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
>  			     int *z, int *fingers)
>  {
> -	int i;
> +	int i, k;
> +	int smooth[nb_sensors + 2];
> +	int smooth_tmp[nb_sensors + 2];
> +
>  	/* values to calculate mean */
>  	int pcum = 0, psum = 0;
>  	int is_increasing = 0;
>  
>  	*fingers = 0;
>  
> +	/*
> +	 * Use a smoothed version of sensor data for movement calculations, to
> +	 * combat noise without needing to toss out values based on a threshold.
> +	 * This improves tracking. Finger count is calculated separately based
> +	 * on raw data.
> +	 */
> +
> +	for (i = 0; i < nb_sensors; i++) {           /* Scale up to minimize */
> +		smooth[i] = xy_sensors[i] << 12;     /* rounding/truncation. */
> +	}
> +
> +	/* Mitigate some of the data loss from smoothing on the edge sensors. */
> +	smooth[-1] = smooth[0] >> 2;
> +	smooth[nb_sensors] = smooth[nb_sensors - 1] >> 2;

Out of bounds array... also, the boundary condition seems odd. If you want to
extrapolate the edge velocity, and assuming smooth[0] is the first sensor, you
would get something like (N = nb_sensors)

	smooth_tmp[0] = 2 * smooth[1] - smooth[2];
	smooth_tmp[N-1] = 2 * smooth[N-2] - smooth[N-3];

> +	for (k = 0; k < 4; k++) {
> +		for (i = 0; i < nb_sensors; i++) {   /* Blend with neighbors. */

And here would be for (i = 1; i < nb_sensors - 1; i++)

> +			smooth_tmp[i] = (smooth[i - 1] + smooth[i] * 2 + smooth[i + 1]) >> 2;
> +		}
> +
> +		for (i = 0; i < nb_sensors; i++) {
> +			smooth[i] = smooth_tmp[i];
> +			if (k == 3)     /* Last go-round, so scale back down. */
> +				smooth[i] = smooth[i] >> 12;

The scale-up is done separately, so why not make the scale-down separately too?

> +		}
> +
> +		smooth[-1] = smooth[0] >> 2;
> +		smooth[nb_sensors] = smooth[nb_sensors - 1] >> 2;

And these would be dropped.

> +	}
> +
>  	for (i = 0; i < nb_sensors; i++) {
>  		if (xy_sensors[i] < threshold) {
>  			if (is_increasing)
>  				is_increasing = 0;
>  
> -			continue;
> -		}
> -
>  		/*
>  		 * Makes the finger detection more versatile.  For example,
>  		 * two fingers with no gap will be detected.  Also, my
> @@ -367,7 +397,8 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
>  		 *
>  		 * - Jason Parekh <jasonparekh@gmail.com>
>  		 */
> -		if (i < 1 ||
> +
> +		} else if (i < 1 ||
>  		    (!is_increasing && xy_sensors[i - 1] < xy_sensors[i])) {
>  			(*fingers)++;
>  			is_increasing = 1;
> @@ -375,15 +406,8 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
>  			is_increasing = 0;
>  		}
>  
> -		/*
> -		 * Subtracts threshold so a high sensor that just passes the
> -		 * threshold won't skew the calculated absolute coordinate.
> -		 * Fixes an issue where slowly moving the mouse would
> -		 * occasionally jump a number of pixels (slowly moving the
> -		 * finger makes this issue most apparent.)
> -		 */
> -		pcum += (xy_sensors[i] - threshold) * i;
> -		psum += (xy_sensors[i] - threshold);
> +		pcum += (smooth[i]) * i;
> +		psum += (smooth[i]);

Great, this makes so much more sense.

>  	}
>  
>  	if (psum > 0) {
> @@ -565,8 +589,8 @@ static void atp_complete_geyser_1_2(struct urb *urb)
>  
>  	if (x && y) {
>  		if (dev->x_old != -1) {
> -			x = (dev->x_old * 3 + x) >> 2;
> -			y = (dev->y_old * 3 + y) >> 2;
> +			x = (dev->x_old * 7 + x) >> 3;
> +			y = (dev->y_old * 7 + y) >> 3;
>  			dev->x_old = x;
>  			dev->y_old = y;
>  
> @@ -677,8 +701,8 @@ static void atp_complete_geyser_3_4(struct urb *urb)
>  
>  	if (x && y) {
>  		if (dev->x_old != -1) {
> -			x = (dev->x_old * 3 + x) >> 2;
> -			y = (dev->y_old * 3 + y) >> 2;
> +			x = (dev->x_old * 7 + x) >> 3;
> +			y = (dev->y_old * 7 + y) >> 3;
>  			dev->x_old = x;
>  			dev->y_old = y;

Would you say that the cursor slow-down here is noticeable, or are there enough
samples per second that it does not matter?

Thanks,
Henrik

Re: [PATCH v2 2/3] input: appletouch: use better cursor movement smoothing algorithm

[Clinton Sprain]

Hi Henrik,

On 02/09/2014 06:15 AM, Henrik Rydberg wrote:
> Hi Clinton,
> 
>> Use smoothed version of sensor array data to calculate movement and add weight
> to prior values when calculating average. This gives more granular and more
> predictable movement.
>>
>> Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
>> ---
>>  drivers/input/mouse/appletouch.c |   60 ++++++++++++++++++++++++++------------
>>  1 file changed, 42 insertions(+), 18 deletions(-)
> 
> I like this patch, but there are some technical glitches, comments below.
> 
>> diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
>> index d48181b..edbdd95 100644
>> --- a/drivers/input/mouse/appletouch.c
>> +++ b/drivers/input/mouse/appletouch.c
>> @@ -338,21 +338,51 @@ static void atp_reinit(struct work_struct *work)
>>  static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
>>  			     int *z, int *fingers)
>>  {
>> -	int i;
>> +	int i, k;
>> +	int smooth[nb_sensors + 2];
>> +	int smooth_tmp[nb_sensors + 2];
>> +
>>  	/* values to calculate mean */
>>  	int pcum = 0, psum = 0;
>>  	int is_increasing = 0;
>>  
>>  	*fingers = 0;
>>  
>> +	/*
>> +	 * Use a smoothed version of sensor data for movement calculations, to
>> +	 * combat noise without needing to toss out values based on a threshold.
>> +	 * This improves tracking. Finger count is calculated separately based
>> +	 * on raw data.
>> +	 */
>> +
>> +	for (i = 0; i < nb_sensors; i++) {           /* Scale up to minimize */
>> +		smooth[i] = xy_sensors[i] << 12;     /* rounding/truncation. */
>> +	}
>> +
>> +	/* Mitigate some of the data loss from smoothing on the edge sensors. */
>> +	smooth[-1] = smooth[0] >> 2;
>> +	smooth[nb_sensors] = smooth[nb_sensors - 1] >> 2;
> 
> Out of bounds array... also, the boundary condition seems odd. If you want to
> extrapolate the edge velocity, and assuming smooth[0] is the first sensor, you
> would get something like (N = nb_sensors)
> 
> 	smooth_tmp[0] = 2 * smooth[1] - smooth[2];
> 	smooth_tmp[N-1] = 2 * smooth[N-2] - smooth[N-3];
> 

This produces a big velocity increase when the finger crosses over from sensor 0 to sensor 1 or vice versa.

We might get by with something relatively simple:

smooth_tmp[0] = (3 * smooth[0] + smooth[1]) >> 2;

...and the same thing on the other edge. This produces predictable behavior while keeping values from bleeding off the board so to speak. (Less important now than if someone down the line ever decides to increase the number of passes.)

>> +	for (k = 0; k < 4; k++) {
>> +		for (i = 0; i < nb_sensors; i++) {   /* Blend with neighbors. */
> 
> And here would be for (i = 1; i < nb_sensors - 1; i++)
> 
>> +			smooth_tmp[i] = (smooth[i - 1] + smooth[i] * 2 + smooth[i + 1]) >> 2;
>> +		}
>> +
>> +		for (i = 0; i < nb_sensors; i++) {
>> +			smooth[i] = smooth_tmp[i];
>> +			if (k == 3)     /* Last go-round, so scale back down. */
>> +				smooth[i] = smooth[i] >> 12;
> 
> The scale-up is done separately, so why not make the scale-down separately too?
> 
>> +		}
>> +
>> +		smooth[-1] = smooth[0] >> 2;
>> +		smooth[nb_sensors] = smooth[nb_sensors - 1] >> 2;
> 
> And these would be dropped.
> 
>> +	}
>> +
>>  	for (i = 0; i < nb_sensors; i++) {
>>  		if (xy_sensors[i] < threshold) {
>>  			if (is_increasing)
>>  				is_increasing = 0;
>>  
>> -			continue;
>> -		}
>> -
>>  		/*
>>  		 * Makes the finger detection more versatile.  For example,
>>  		 * two fingers with no gap will be detected.  Also, my
>> @@ -367,7 +397,8 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
>>  		 *
>>  		 * - Jason Parekh <jasonparekh@gmail.com>
>>  		 */
>> -		if (i < 1 ||
>> +
>> +		} else if (i < 1 ||
>>  		    (!is_increasing && xy_sensors[i - 1] < xy_sensors[i])) {
>>  			(*fingers)++;
>>  			is_increasing = 1;
>> @@ -375,15 +406,8 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
>>  			is_increasing = 0;
>>  		}
>>  
>> -		/*
>> -		 * Subtracts threshold so a high sensor that just passes the
>> -		 * threshold won't skew the calculated absolute coordinate.
>> -		 * Fixes an issue where slowly moving the mouse would
>> -		 * occasionally jump a number of pixels (slowly moving the
>> -		 * finger makes this issue most apparent.)
>> -		 */
>> -		pcum += (xy_sensors[i] - threshold) * i;
>> -		psum += (xy_sensors[i] - threshold);
>> +		pcum += (smooth[i]) * i;
>> +		psum += (smooth[i]);
> 
> Great, this makes so much more sense.
> 
>>  	}
>>  
>>  	if (psum > 0) {
>> @@ -565,8 +589,8 @@ static void atp_complete_geyser_1_2(struct urb *urb)
>>  
>>  	if (x && y) {
>>  		if (dev->x_old != -1) {
>> -			x = (dev->x_old * 3 + x) >> 2;
>> -			y = (dev->y_old * 3 + y) >> 2;
>> +			x = (dev->x_old * 7 + x) >> 3;
>> +			y = (dev->y_old * 7 + y) >> 3;
>>  			dev->x_old = x;
>>  			dev->y_old = y;
>>  
>> @@ -677,8 +701,8 @@ static void atp_complete_geyser_3_4(struct urb *urb)
>>  
>>  	if (x && y) {
>>  		if (dev->x_old != -1) {
>> -			x = (dev->x_old * 3 + x) >> 2;
>> -			y = (dev->y_old * 3 + y) >> 2;
>> +			x = (dev->x_old * 7 + x) >> 3;
>> +			y = (dev->y_old * 7 + y) >> 3;
>>  			dev->x_old = x;
>>  			dev->y_old = y;
> 
> Would you say that the cursor slow-down here is noticeable, or are there enough
> samples per second that it does not matter?

Noticeable but not major. It's still possible to move the cursor across the screen in one movement with typical defaults, and the gain in smoothness is pretty big, so it seems like a reasonable trade-off. (The 'base' speed is also still quicker than the native OS driver, FWIW.)

> 
> Thanks,
> Henrik
> 

Thanks,
Clinton

[PATCH v2 3/3] input: appletouch: fix jumps when additional fingers are detected

[Clinton Sprain]

Discard cursor movements if they directly coincide with a change in the number of fingers detected. This helps with two issues - a sudden jump of the cursor when a second finger enters or leaves the touchpad, and an unexpected jump in page scrolling under the same scenario. The fix doesn't completely eliminate the problem but does greatly reduce its frequency and severity.

Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
 drivers/input/mouse/appletouch.c |   23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index edbdd95..370d0e9 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -212,6 +212,7 @@ struct atp {
 	signed char		xy_old[ATP_XSENSORS + ATP_YSENSORS];
 	int			xy_acc[ATP_XSENSORS + ATP_YSENSORS];
 	int			idlecount;	/* number of empty packets */
+	int			fingers_old;	/* last reported finger count */
 	struct work_struct	work;
 };
 
@@ -505,6 +506,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
 	int x, y, x_z, y_z, x_f, y_f;
 	int retval, i, j;
 	int key;
+	int fingers;
 	struct atp *dev = urb->context;
 	int status = atp_status_check(urb);
 
@@ -587,7 +589,9 @@ static void atp_complete_geyser_1_2(struct urb *urb)
 			      dev->info->yfact, &y_z, &y_f);
 	key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
 
-	if (x && y) {
+	fingers = max(x_f, y_f);
+
+	if (x && y && (fingers == dev->fingers_old)) {
 		if (dev->x_old != -1) {
 			x = (dev->x_old * 7 + x) >> 3;
 			y = (dev->y_old * 7 + y) >> 3;
@@ -604,7 +608,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
 			input_report_abs(dev->input, ABS_Y, y);
 			input_report_abs(dev->input, ABS_PRESSURE,
 					 min(ATP_PRESSURE, x_z + y_z));
-			atp_report_fingers(dev->input, max(x_f, y_f));
+			atp_report_fingers(dev->input, fingers);
 		}
 		dev->x_old = x;
 		dev->y_old = y;
@@ -620,6 +624,10 @@ static void atp_complete_geyser_1_2(struct urb *urb)
 		memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
 	}
 
+	if (fingers != dev->fingers_old)
+		dev->x_old = dev->y_old = -1;
+	dev->fingers_old = fingers;
+
 	input_report_key(dev->input, BTN_LEFT, key);
 	input_sync(dev->input);
 
@@ -638,6 +646,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 	int x, y, x_z, y_z, x_f, y_f;
 	int retval, i, j;
 	int key;
+	int fingers;
 	struct atp *dev = urb->context;
 	int status = atp_status_check(urb);
 
@@ -699,7 +708,9 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 			      dev->info->yfact, &y_z, &y_f);
 	key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
 
-	if (x && y) {
+	fingers = max(x_f, y_f);
+
+	if (x && y && (fingers == dev->fingers_old)) {
 		if (dev->x_old != -1) {
 			x = (dev->x_old * 7 + x) >> 3;
 			y = (dev->y_old * 7 + y) >> 3;
@@ -716,7 +727,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 			input_report_abs(dev->input, ABS_Y, y);
 			input_report_abs(dev->input, ABS_PRESSURE,
 					 min(ATP_PRESSURE, x_z + y_z));
-			atp_report_fingers(dev->input, max(x_f, y_f));
+			atp_report_fingers(dev->input, fingers);
 		}
 		dev->x_old = x;
 		dev->y_old = y;
@@ -732,6 +743,10 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 		memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
 	}
 
+	if (fingers != dev->fingers_old)
+		dev->x_old = dev->y_old = -1;
+	dev->fingers_old = fingers;
+
 	input_report_key(dev->input, BTN_LEFT, key);
 	input_sync(dev->input);
 
-- 
1.7.9.5

Re: [PATCH v2 3/3] input: appletouch: fix jumps when additional fingers are detected

[Henrik Rydberg]

On 02/07/2014 01:46 AM, Clinton Sprain wrote:
> Discard cursor movements if they directly coincide with a change in the number of fingers detected. This helps with two issues - a sudden jump of the cursor when a second finger enters or leaves the touchpad, and an unexpected jump in page scrolling under the same scenario. The fix doesn't completely eliminate the problem but does greatly reduce its frequency and severity.
> 
> Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
> ---
>  drivers/input/mouse/appletouch.c |   23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
> index edbdd95..370d0e9 100644
> --- a/drivers/input/mouse/appletouch.c
> +++ b/drivers/input/mouse/appletouch.c
> @@ -212,6 +212,7 @@ struct atp {
>  	signed char		xy_old[ATP_XSENSORS + ATP_YSENSORS];
>  	int			xy_acc[ATP_XSENSORS + ATP_YSENSORS];
>  	int			idlecount;	/* number of empty packets */
> +	int			fingers_old;	/* last reported finger count */
>  	struct work_struct	work;
>  };
>  
> @@ -505,6 +506,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
>  	int x, y, x_z, y_z, x_f, y_f;
>  	int retval, i, j;
>  	int key;
> +	int fingers;
>  	struct atp *dev = urb->context;
>  	int status = atp_status_check(urb);
>  
> @@ -587,7 +589,9 @@ static void atp_complete_geyser_1_2(struct urb *urb)
>  			      dev->info->yfact, &y_z, &y_f);
>  	key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
>  
> -	if (x && y) {
> +	fingers = max(x_f, y_f);
> +
> +	if (x && y && (fingers == dev->fingers_old)) {
>  		if (dev->x_old != -1) {
>  			x = (dev->x_old * 7 + x) >> 3;
>  			y = (dev->y_old * 7 + y) >> 3;
> @@ -604,7 +608,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
>  			input_report_abs(dev->input, ABS_Y, y);
>  			input_report_abs(dev->input, ABS_PRESSURE,
>  					 min(ATP_PRESSURE, x_z + y_z));
> -			atp_report_fingers(dev->input, max(x_f, y_f));
> +			atp_report_fingers(dev->input, fingers);
>  		}
>  		dev->x_old = x;
>  		dev->y_old = y;
> @@ -620,6 +624,10 @@ static void atp_complete_geyser_1_2(struct urb *urb)
>  		memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
>  	}
>  
> +	if (fingers != dev->fingers_old)
> +		dev->x_old = dev->y_old = -1;
> +	dev->fingers_old = fingers;
> +
>  	input_report_key(dev->input, BTN_LEFT, key);
>  	input_sync(dev->input);
>  
> @@ -638,6 +646,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
>  	int x, y, x_z, y_z, x_f, y_f;
>  	int retval, i, j;
>  	int key;
> +	int fingers;
>  	struct atp *dev = urb->context;
>  	int status = atp_status_check(urb);
>  
> @@ -699,7 +708,9 @@ static void atp_complete_geyser_3_4(struct urb *urb)
>  			      dev->info->yfact, &y_z, &y_f);
>  	key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
>  
> -	if (x && y) {
> +	fingers = max(x_f, y_f);
> +
> +	if (x && y && (fingers == dev->fingers_old)) {
>  		if (dev->x_old != -1) {
>  			x = (dev->x_old * 7 + x) >> 3;
>  			y = (dev->y_old * 7 + y) >> 3;
> @@ -716,7 +727,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
>  			input_report_abs(dev->input, ABS_Y, y);
>  			input_report_abs(dev->input, ABS_PRESSURE,
>  					 min(ATP_PRESSURE, x_z + y_z));
> -			atp_report_fingers(dev->input, max(x_f, y_f));
> +			atp_report_fingers(dev->input, fingers);
>  		}
>  		dev->x_old = x;
>  		dev->y_old = y;
> @@ -732,6 +743,10 @@ static void atp_complete_geyser_3_4(struct urb *urb)
>  		memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
>  	}
>  
> +	if (fingers != dev->fingers_old)
> +		dev->x_old = dev->y_old = -1;
> +	dev->fingers_old = fingers;
> +
>  	input_report_key(dev->input, BTN_LEFT, key);
>  	input_sync(dev->input);
>  
> 

    Reviewed-by: Henrik Rydberg <rydberg@euromail.se>

Thanks,
Henrik

[PATCH v3 0/3] input: appletouch: fixes for jagged/uneven cursor movement

[Clinton Sprain]

Changes from last time:

1: Dropped fuzz as a module parameter; also changed threshold defaults back to 5, as I found an outside case where the lower threshold was problematic for finger counting

2: Nix negative subscripts, make smooth array large enough that boundary condition is basically irrelevant (though still handled just in case), scale up and down a little bit more effectively, style

3: Unchanged, except for setting dev->fingers_old to 0 when x and y don't come back with coordinates

Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>

Clinton Sprain (3):
  input: appletouch: dial back fuzz setting
  input: appletouch: implement sensor data smoothing
  input: appletouch: fix jumps when additional fingers are detected

 drivers/input/mouse/appletouch.c |  114 +++++++++++++++++++++++++++-----------
 1 file changed, 83 insertions(+), 31 deletions(-)

-- 
1.7.9.5

[PATCH v3 1/3] input: appletouch: dial back fuzz setting

[Clinton Sprain]

Dials back the default fuzz setting for most devices using this driver, based
on values from  user feedback from forums and bug reports.

Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
 drivers/input/mouse/appletouch.c |   15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index 800ca7d..2745832 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -48,6 +48,7 @@ struct atp_info {
 	int yfact;				/* Y multiplication factor */
 	int datalen;				/* size of USB transfers */
 	void (*callback)(struct urb *);		/* callback function */
+	int fuzz;				/* fuzz touchpad generates */
 };
 
 static void atp_complete_geyser_1_2(struct urb *urb);
@@ -61,6 +62,7 @@ static const struct atp_info fountain_info = {
 	.yfact		= 43,
 	.datalen	= 81,
 	.callback	= atp_complete_geyser_1_2,
+	.fuzz		= 16,
 };
 
 static const struct atp_info geyser1_info = {
@@ -71,6 +73,7 @@ static const struct atp_info geyser1_info = {
 	.yfact		= 43,
 	.datalen	= 81,
 	.callback	= atp_complete_geyser_1_2,
+	.fuzz		= 16,
 };
 
 static const struct atp_info geyser2_info = {
@@ -81,6 +84,7 @@ static const struct atp_info geyser2_info = {
 	.yfact		= 43,
 	.datalen	= 64,
 	.callback	= atp_complete_geyser_1_2,
+	.fuzz		= 0,
 };
 
 static const struct atp_info geyser3_info = {
@@ -90,6 +94,7 @@ static const struct atp_info geyser3_info = {
 	.yfact		= 64,
 	.datalen	= 64,
 	.callback	= atp_complete_geyser_3_4,
+	.fuzz		= 0,
 };
 
 static const struct atp_info geyser4_info = {
@@ -99,6 +104,7 @@ static const struct atp_info geyser4_info = {
 	.yfact		= 64,
 	.datalen	= 64,
 	.callback	= atp_complete_geyser_3_4,
+	.fuzz		= 0,
 };
 
 #define ATP_DEVICE(prod, info)					\
@@ -155,9 +161,6 @@ MODULE_DEVICE_TABLE(usb, atp_table);
 #define ATP_XSENSORS	26
 #define ATP_YSENSORS	16
 
-/* amount of fuzz this touchpad generates */
-#define ATP_FUZZ	16
-
 /* maximum pressure this driver will report */
 #define ATP_PRESSURE	300
 
@@ -455,7 +458,7 @@ static void atp_detect_size(struct atp *dev)
 			input_set_abs_params(dev->input, ABS_X, 0,
 					     (dev->info->xsensors_17 - 1) *
 							dev->info->xfact - 1,
-					     ATP_FUZZ, 0);
+					     dev->info->fuzz, 0);
 			break;
 		}
 	}
@@ -843,10 +846,10 @@ static int atp_probe(struct usb_interface *iface,
 
 	input_set_abs_params(input_dev, ABS_X, 0,
 			     (dev->info->xsensors - 1) * dev->info->xfact - 1,
-			     ATP_FUZZ, 0);
+			     dev->info->fuzz, 0);
 	input_set_abs_params(input_dev, ABS_Y, 0,
 			     (dev->info->ysensors - 1) * dev->info->yfact - 1,
-			     ATP_FUZZ, 0);
+			     dev->info->fuzz, 0);
 	input_set_abs_params(input_dev, ABS_PRESSURE, 0, ATP_PRESSURE, 0, 0);
 
 	set_bit(EV_KEY, input_dev->evbit);
-- 
1.7.9.5

[PATCH v3 2/3] input: appletouch: implement sensor data smoothing

[Clinton Sprain]

Use smoothed version of sensor array data to calculate movement and add weight
to prior values when calculating average. This gives more granular and more
predictable movement.

Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
 drivers/input/mouse/appletouch.c |   72 ++++++++++++++++++++++++++++----------
 1 file changed, 53 insertions(+), 19 deletions(-)

diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index 2745832..0252bab 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -332,7 +332,11 @@ static void atp_reinit(struct work_struct *work)
 static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
 			     int *z, int *fingers)
 {
-	int i;
+	int i, k;
+	int smooth[nb_sensors + 8];
+	int smooth_tmp[nb_sensors + 8];
+	int scale = 12;
+
 	/* values to calculate mean */
 	int pcum = 0, psum = 0;
 	int is_increasing = 0;
@@ -344,9 +348,6 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
 			if (is_increasing)
 				is_increasing = 0;
 
-			continue;
-		}
-
 		/*
 		 * Makes the finger detection more versatile.  For example,
 		 * two fingers with no gap will be detected.  Also, my
@@ -361,27 +362,60 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
 		 *
 		 * - Jason Parekh <jasonparekh@gmail.com>
 		 */
-		if (i < 1 ||
+
+		} else if (i < 1 ||
 		    (!is_increasing && xy_sensors[i - 1] < xy_sensors[i])) {
 			(*fingers)++;
 			is_increasing = 1;
 		} else if (i > 0 && (xy_sensors[i - 1] - xy_sensors[i] > threshold)) {
 			is_increasing = 0;
 		}
+	}
 
-		/*
-		 * Subtracts threshold so a high sensor that just passes the
-		 * threshold won't skew the calculated absolute coordinate.
-		 * Fixes an issue where slowly moving the mouse would
-		 * occasionally jump a number of pixels (slowly moving the
-		 * finger makes this issue most apparent.)
-		 */
-		pcum += (xy_sensors[i] - threshold) * i;
-		psum += (xy_sensors[i] - threshold);
+	/*
+	 * Use a smoothed version of sensor data for movement calculations, to
+	 * combat noise without needing to rely so heavily on a threshold.
+	 * This improves tracking.
+	 *
+	 * The smoothed array is bigger than the original so that the smoothing
+	 * doesn't result in edge values being truncated.
+	 */
+
+	for (i = 0; i < 4; i++)
+		smooth[i] = 0;
+	for (i = nb_sensors + 4; i < nb_sensors + 8; i++)
+		smooth[i] = 0;
+
+	/* Pull base values, scaled up to help avoid truncation errors. */
+
+	for (i = 0; i < nb_sensors; i++)
+		smooth[i + 4] = xy_sensors[i] << scale;
+
+	for (k = 0; k < 4; k++) {
+
+		/* Handle edge. */
+		smooth_tmp[0] = (smooth[0] * 1 + smooth[1]) >> 1;
+
+		/* Average values with neighbors. */
+		for (i = 1; i < nb_sensors + 7; i++)
+			smooth_tmp[i] = (smooth[i - 1] + smooth[i] * 2 + smooth[i + 1]) >> 2;
+
+		/* Handle other edge. */
+		smooth_tmp[nb_sensors + 7] = (smooth[nb_sensors + 7] + smooth[nb_sensors + 6]) >> 1;
+
+		for (i = 0; i < nb_sensors + 7; i++)
+			smooth[i] = smooth_tmp[i];
+	}
+
+	for (i = 0; i < nb_sensors + 8; i++) {
+		if ((smooth[i] >> scale) > 0) {  /* Skip individual values if */
+			pcum += (smooth[i]) * i; /* they are small enough to  */
+			psum += (smooth[i]);     /* be truncated to 0 by our  */
+		}                                /* scale; mostly just noise. */
 	}
 
 	if (psum > 0) {
-		*z = psum;
+		*z = psum >> scale;            /* Scale down pressure output. */
 		return pcum * fact / psum;
 	}
 
@@ -559,8 +593,8 @@ static void atp_complete_geyser_1_2(struct urb *urb)
 
 	if (x && y) {
 		if (dev->x_old != -1) {
-			x = (dev->x_old * 3 + x) >> 2;
-			y = (dev->y_old * 3 + y) >> 2;
+			x = (dev->x_old * 7 + x) >> 3;
+			y = (dev->y_old * 7 + y) >> 3;
 			dev->x_old = x;
 			dev->y_old = y;
 
@@ -671,8 +705,8 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 
 	if (x && y) {
 		if (dev->x_old != -1) {
-			x = (dev->x_old * 3 + x) >> 2;
-			y = (dev->y_old * 3 + y) >> 2;
+			x = (dev->x_old * 7 + x) >> 3;
+			y = (dev->y_old * 7 + y) >> 3;
 			dev->x_old = x;
 			dev->y_old = y;
 
-- 
1.7.9.5

Re: [PATCH v3 2/3] input: appletouch: implement sensor data smoothing

[Henrik Rydberg]

Hi Clinton,

> Use smoothed version of sensor array data to calculate movement and add weight
> to prior values when calculating average. This gives more granular and more
> predictable movement.

Great patch, just one thing:

> +		/* Handle other edge. */
> +		smooth_tmp[nb_sensors + 7] = (smooth[nb_sensors + 7] + smooth[nb_sensors + 6]) >> 1;
> +
> +		for (i = 0; i < nb_sensors + 7; i++)
> +			smooth[i] = smooth_tmp[i];

Should be "< nb_sensors + 8" here, no?

Thanks,
Henrik

Re: [PATCH v3 2/3] input: appletouch: implement sensor data smoothing

[Clinton Sprain]

Ah, you are correct. Good catch. The last few far edge values are guaranteed to be 0 on all but the older 17" devices so I didn't see anything askew.

Thanks,
Clinton

On 03/09/2014 08:54 AM, Henrik Rydberg wrote:
> Hi Clinton,
> 
>> Use smoothed version of sensor array data to calculate movement and add weight
>> to prior values when calculating average. This gives more granular and more
>> predictable movement.
> 
> Great patch, just one thing:
> 
>> +		/* Handle other edge. */
>> +		smooth_tmp[nb_sensors + 7] = (smooth[nb_sensors + 7] + smooth[nb_sensors + 6]) >> 1;
>> +
>> +		for (i = 0; i < nb_sensors + 7; i++)
>> +			smooth[i] = smooth_tmp[i];
> 
> Should be "< nb_sensors + 8" here, no?
> 
> Thanks,
> Henrik
> 

[PATCH v3 3/3] input: appletouch: fix jumps when additional fingers are detected

[Clinton Sprain]

Addresses issues related to when a second finger enters or leaves the field,
causing the cursor to jump or the page to scroll unexpectedly; now, we
discard any movement change that happens at the exact moment we detect a
change in the number of fingers touching the trackpad. This doesn't
completely resolve the issue but does greatly mitigate it.

Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
 drivers/input/mouse/appletouch.c |   27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index 0252bab..0c7bc6e 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -206,6 +206,7 @@ struct atp {
 	bool			valid;		/* are the samples valid? */
 	bool			size_detect_done;
 	bool			overflow_warned;
+	int			fingers_old;	/* last reported finger count */
 	int			x_old;		/* last reported x/y, */
 	int			y_old;		/* used for smoothing */
 	signed char		xy_cur[ATP_XSENSORS + ATP_YSENSORS];
@@ -508,7 +509,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
 {
 	int x, y, x_z, y_z, x_f, y_f;
 	int retval, i, j;
-	int key;
+	int key, fingers;
 	struct atp *dev = urb->context;
 	int status = atp_status_check(urb);
 
@@ -591,7 +592,9 @@ static void atp_complete_geyser_1_2(struct urb *urb)
 			      dev->info->yfact, &y_z, &y_f);
 	key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
 
-	if (x && y) {
+	fingers = max(x_f, y_f);
+
+	if (x && y && (fingers == dev->fingers_old)) {
 		if (dev->x_old != -1) {
 			x = (dev->x_old * 7 + x) >> 3;
 			y = (dev->y_old * 7 + y) >> 3;
@@ -608,7 +611,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
 			input_report_abs(dev->input, ABS_Y, y);
 			input_report_abs(dev->input, ABS_PRESSURE,
 					 min(ATP_PRESSURE, x_z + y_z));
-			atp_report_fingers(dev->input, max(x_f, y_f));
+			atp_report_fingers(dev->input, fingers);
 		}
 		dev->x_old = x;
 		dev->y_old = y;
@@ -616,6 +619,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
 	} else if (!x && !y) {
 
 		dev->x_old = dev->y_old = -1;
+		dev->fingers_old = 0;
 		input_report_key(dev->input, BTN_TOUCH, 0);
 		input_report_abs(dev->input, ABS_PRESSURE, 0);
 		atp_report_fingers(dev->input, 0);
@@ -624,6 +628,10 @@ static void atp_complete_geyser_1_2(struct urb *urb)
 		memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
 	}
 
+	if (fingers != dev->fingers_old)
+		dev->x_old = dev->y_old = -1;
+	dev->fingers_old = fingers;
+
 	input_report_key(dev->input, BTN_LEFT, key);
 	input_sync(dev->input);
 
@@ -641,7 +649,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 {
 	int x, y, x_z, y_z, x_f, y_f;
 	int retval, i, j;
-	int key;
+	int key, fingers;
 	struct atp *dev = urb->context;
 	int status = atp_status_check(urb);
 
@@ -703,7 +711,9 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 			      dev->info->yfact, &y_z, &y_f);
 	key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
 
-	if (x && y) {
+	fingers = max(x_f, y_f);
+
+	if (x && y && (fingers == dev->fingers_old)) {
 		if (dev->x_old != -1) {
 			x = (dev->x_old * 7 + x) >> 3;
 			y = (dev->y_old * 7 + y) >> 3;
@@ -720,7 +730,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 			input_report_abs(dev->input, ABS_Y, y);
 			input_report_abs(dev->input, ABS_PRESSURE,
 					 min(ATP_PRESSURE, x_z + y_z));
-			atp_report_fingers(dev->input, max(x_f, y_f));
+			atp_report_fingers(dev->input, fingers);
 		}
 		dev->x_old = x;
 		dev->y_old = y;
@@ -728,6 +738,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 	} else if (!x && !y) {
 
 		dev->x_old = dev->y_old = -1;
+		dev->fingers_old = 0;
 		input_report_key(dev->input, BTN_TOUCH, 0);
 		input_report_abs(dev->input, ABS_PRESSURE, 0);
 		atp_report_fingers(dev->input, 0);
@@ -736,6 +747,10 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 		memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
 	}
 
+	if (fingers != dev->fingers_old)
+		dev->x_old = dev->y_old = -1;
+	dev->fingers_old = fingers;
+
 	input_report_key(dev->input, BTN_LEFT, key);
 	input_sync(dev->input);
 
-- 
1.7.9.5

[PATCH v4 0/3] input: appletouch: fixes for jagged/uneven cursor movement

[Clinton Sprain]

input: appletouch: fixes for jagged/uneven cursor movement

The appletouch driver can make some touchpads feel insensitive, with movement that tends to jerk in steps. This is particularly evident when moving diagonally. There is also undesirable movement when additional fingers enter or leave the touchpad.

These patches address these issues by dialing back the fuzz setting, by implementing sensor data smoothing and by discarding movements that directly coincide with a change in the number of fingers detected on the touchpad.

Changes since v3:

1: No change
2: Fix loop counter in atp_calculate_abs
3: No change

Clinton Sprain (3):
  input: appletouch: dial back fuzz setting
  input: appletouch: implement sensor data smoothing
  input: appletouch: fix jumps when additional fingers are detected

 drivers/input/mouse/appletouch.c |  114 +++++++++++++++++++++++++++-----------
 1 file changed, 83 insertions(+), 31 deletions(-)

-- 
1.7.9.5

[PATCH v4 1/3] input: appletouch: dial back fuzz setting

[Clinton Sprain]

input: appletouch: dial back fuzz setting

Dials back the default fuzz setting for most devices using this driver, based on values from  user feedback from forums and bug reports.

Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
 drivers/input/mouse/appletouch.c |   15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index 800ca7d..2745832 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -48,6 +48,7 @@ struct atp_info {
 	int yfact;				/* Y multiplication factor */
 	int datalen;				/* size of USB transfers */
 	void (*callback)(struct urb *);		/* callback function */
+	int fuzz;				/* fuzz touchpad generates */
 };
 
 static void atp_complete_geyser_1_2(struct urb *urb);
@@ -61,6 +62,7 @@ static const struct atp_info fountain_info = {
 	.yfact		= 43,
 	.datalen	= 81,
 	.callback	= atp_complete_geyser_1_2,
+	.fuzz		= 16,
 };
 
 static const struct atp_info geyser1_info = {
@@ -71,6 +73,7 @@ static const struct atp_info geyser1_info = {
 	.yfact		= 43,
 	.datalen	= 81,
 	.callback	= atp_complete_geyser_1_2,
+	.fuzz		= 16,
 };
 
 static const struct atp_info geyser2_info = {
@@ -81,6 +84,7 @@ static const struct atp_info geyser2_info = {
 	.yfact		= 43,
 	.datalen	= 64,
 	.callback	= atp_complete_geyser_1_2,
+	.fuzz		= 0,
 };
 
 static const struct atp_info geyser3_info = {
@@ -90,6 +94,7 @@ static const struct atp_info geyser3_info = {
 	.yfact		= 64,
 	.datalen	= 64,
 	.callback	= atp_complete_geyser_3_4,
+	.fuzz		= 0,
 };
 
 static const struct atp_info geyser4_info = {
@@ -99,6 +104,7 @@ static const struct atp_info geyser4_info = {
 	.yfact		= 64,
 	.datalen	= 64,
 	.callback	= atp_complete_geyser_3_4,
+	.fuzz		= 0,
 };
 
 #define ATP_DEVICE(prod, info)					\
@@ -155,9 +161,6 @@ MODULE_DEVICE_TABLE(usb, atp_table);
 #define ATP_XSENSORS	26
 #define ATP_YSENSORS	16
 
-/* amount of fuzz this touchpad generates */
-#define ATP_FUZZ	16
-
 /* maximum pressure this driver will report */
 #define ATP_PRESSURE	300
 
@@ -455,7 +458,7 @@ static void atp_detect_size(struct atp *dev)
 			input_set_abs_params(dev->input, ABS_X, 0,
 					     (dev->info->xsensors_17 - 1) *
 							dev->info->xfact - 1,
-					     ATP_FUZZ, 0);
+					     dev->info->fuzz, 0);
 			break;
 		}
 	}
@@ -843,10 +846,10 @@ static int atp_probe(struct usb_interface *iface,
 
 	input_set_abs_params(input_dev, ABS_X, 0,
 			     (dev->info->xsensors - 1) * dev->info->xfact - 1,
-			     ATP_FUZZ, 0);
+			     dev->info->fuzz, 0);
 	input_set_abs_params(input_dev, ABS_Y, 0,
 			     (dev->info->ysensors - 1) * dev->info->yfact - 1,
-			     ATP_FUZZ, 0);
+			     dev->info->fuzz, 0);
 	input_set_abs_params(input_dev, ABS_PRESSURE, 0, ATP_PRESSURE, 0, 0);
 
 	set_bit(EV_KEY, input_dev->evbit);
-- 
1.7.9.5

[PATCH v4 2/3] input: appletouch: implement sensor data smoothing

[Clinton Sprain]

input: appletouch: implement sensor data smoothing

Use smoothed version of sensor array data to calculate movement and add weight
to prior values when calculating average. This gives more granular and more
predictable movement.

Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
 drivers/input/mouse/appletouch.c |   72 ++++++++++++++++++++++++++++----------
 1 file changed, 53 insertions(+), 19 deletions(-)

diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index 2745832..e00f126 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -332,7 +332,11 @@ static void atp_reinit(struct work_struct *work)
 static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
 			     int *z, int *fingers)
 {
-	int i;
+	int i, k;
+	int smooth[nb_sensors + 8];
+	int smooth_tmp[nb_sensors + 8];
+	int scale = 12;
+
 	/* values to calculate mean */
 	int pcum = 0, psum = 0;
 	int is_increasing = 0;
@@ -344,9 +348,6 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
 			if (is_increasing)
 				is_increasing = 0;
 
-			continue;
-		}
-
 		/*
 		 * Makes the finger detection more versatile.  For example,
 		 * two fingers with no gap will be detected.  Also, my
@@ -361,27 +362,60 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
 		 *
 		 * - Jason Parekh <jasonparekh@gmail.com>
 		 */
-		if (i < 1 ||
+
+		} else if (i < 1 ||
 		    (!is_increasing && xy_sensors[i - 1] < xy_sensors[i])) {
 			(*fingers)++;
 			is_increasing = 1;
 		} else if (i > 0 && (xy_sensors[i - 1] - xy_sensors[i] > threshold)) {
 			is_increasing = 0;
 		}
+	}
 
-		/*
-		 * Subtracts threshold so a high sensor that just passes the
-		 * threshold won't skew the calculated absolute coordinate.
-		 * Fixes an issue where slowly moving the mouse would
-		 * occasionally jump a number of pixels (slowly moving the
-		 * finger makes this issue most apparent.)
-		 */
-		pcum += (xy_sensors[i] - threshold) * i;
-		psum += (xy_sensors[i] - threshold);
+	/*
+	 * Use a smoothed version of sensor data for movement calculations, to
+	 * combat noise without needing to rely so heavily on a threshold.
+	 * This improves tracking.
+	 *
+	 * The smoothed array is bigger than the original so that the smoothing
+	 * doesn't result in edge values being truncated.
+	 */
+
+	for (i = 0; i < 4; i++)
+		smooth[i] = 0;
+	for (i = nb_sensors + 4; i < nb_sensors + 8; i++)
+		smooth[i] = 0;
+
+	/* Pull base values, scaled up to help avoid truncation errors. */
+
+	for (i = 0; i < nb_sensors; i++)
+		smooth[i + 4] = xy_sensors[i] << scale;
+
+	for (k = 0; k < 4; k++) {
+
+		/* Handle edge. */
+		smooth_tmp[0] = (smooth[0] + smooth[1]) >> 1;
+
+		/* Average values with neighbors. */
+		for (i = 1; i < nb_sensors + 7; i++)
+			smooth_tmp[i] = (smooth[i - 1] + smooth[i] * 2 + smooth[i + 1]) >> 2;
+
+		/* Handle other edge. */
+		smooth_tmp[nb_sensors + 7] = (smooth[nb_sensors + 7] + smooth[nb_sensors + 6]) >> 1;
+
+		for (i = 0; i < nb_sensors + 8; i++)
+			smooth[i] = smooth_tmp[i];
+	}
+
+	for (i = 0; i < nb_sensors + 8; i++) {
+		if ((smooth[i] >> scale) > 0) {  /* Skip individual values if */
+			pcum += (smooth[i]) * i; /* they are small enough to  */
+			psum += (smooth[i]);     /* be truncated to 0 by our  */
+		}                                /* scale; mostly just noise. */
 	}
 
 	if (psum > 0) {
-		*z = psum;
+		*z = psum >> scale;            /* Scale down pressure output. */
 		return pcum * fact / psum;
 	}
 
@@ -559,8 +593,8 @@ static void atp_complete_geyser_1_2(struct urb *urb)
 
 	if (x && y) {
 		if (dev->x_old != -1) {
-			x = (dev->x_old * 3 + x) >> 2;
-			y = (dev->y_old * 3 + y) >> 2;
+			x = (dev->x_old * 7 + x) >> 3;
+			y = (dev->y_old * 7 + y) >> 3;
 			dev->x_old = x;
 			dev->y_old = y;
 
@@ -671,8 +705,8 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 
 	if (x && y) {
 		if (dev->x_old != -1) {
-			x = (dev->x_old * 3 + x) >> 2;
-			y = (dev->y_old * 3 + y) >> 2;
+			x = (dev->x_old * 7 + x) >> 3;
+			y = (dev->y_old * 7 + y) >> 3;
 			dev->x_old = x;
 			dev->y_old = y;
 
-- 
1.7.9.5

Re: [PATCH v4 2/3] input: appletouch: implement sensor data smoothing

[Henrik Rydberg]

Hi Clinton,
> input: appletouch: implement sensor data smoothing
> 
> Use smoothed version of sensor array data to calculate movement and add weight
> to prior values when calculating average. This gives more granular and more
> predictable movement.
> 
> Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
> ---
>  drivers/input/mouse/appletouch.c |   72 ++++++++++++++++++++++++++++----------
>  1 file changed, 53 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
> index 2745832..e00f126 100644
> --- a/drivers/input/mouse/appletouch.c
> +++ b/drivers/input/mouse/appletouch.c
> @@ -332,7 +332,11 @@ static void atp_reinit(struct work_struct *work)
>  static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
>  			     int *z, int *fingers)
>  {
> -	int i;
> +	int i, k;
> +	int smooth[nb_sensors + 8];
> +	int smooth_tmp[nb_sensors + 8];
> +	int scale = 12;
> +
>  	/* values to calculate mean */
>  	int pcum = 0, psum = 0;
>  	int is_increasing = 0;
> @@ -344,9 +348,6 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
>  			if (is_increasing)
>  				is_increasing = 0;
>  
> -			continue;
> -		}
> -
>  		/*
>  		 * Makes the finger detection more versatile.  For example,
>  		 * two fingers with no gap will be detected.  Also, my
> @@ -361,27 +362,60 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
>  		 *
>  		 * - Jason Parekh <jasonparekh@gmail.com>
>  		 */
> -		if (i < 1 ||
> +
> +		} else if (i < 1 ||
>  		    (!is_increasing && xy_sensors[i - 1] < xy_sensors[i])) {
>  			(*fingers)++;
>  			is_increasing = 1;
>  		} else if (i > 0 && (xy_sensors[i - 1] - xy_sensors[i] > threshold)) {
>  			is_increasing = 0;
>  		}
> +	}
>  
> -		/*
> -		 * Subtracts threshold so a high sensor that just passes the
> -		 * threshold won't skew the calculated absolute coordinate.
> -		 * Fixes an issue where slowly moving the mouse would
> -		 * occasionally jump a number of pixels (slowly moving the
> -		 * finger makes this issue most apparent.)
> -		 */
> -		pcum += (xy_sensors[i] - threshold) * i;
> -		psum += (xy_sensors[i] - threshold);
> +	/*
> +	 * Use a smoothed version of sensor data for movement calculations, to
> +	 * combat noise without needing to rely so heavily on a threshold.
> +	 * This improves tracking.
> +	 *
> +	 * The smoothed array is bigger than the original so that the smoothing
> +	 * doesn't result in edge values being truncated.
> +	 */
> +
> +	for (i = 0; i < 4; i++)
> +		smooth[i] = 0;
> +	for (i = nb_sensors + 4; i < nb_sensors + 8; i++)
> +		smooth[i] = 0;
> +
> +	/* Pull base values, scaled up to help avoid truncation errors. */
> +
> +	for (i = 0; i < nb_sensors; i++)
> +		smooth[i + 4] = xy_sensors[i] << scale;
> +
> +	for (k = 0; k < 4; k++) {
> +
> +		/* Handle edge. */
> +		smooth_tmp[0] = (smooth[0] + smooth[1]) >> 1;
> +
> +		/* Average values with neighbors. */
> +		for (i = 1; i < nb_sensors + 7; i++)
> +			smooth_tmp[i] = (smooth[i - 1] + smooth[i] * 2 + smooth[i + 1]) >> 2;
> +
> +		/* Handle other edge. */
> +		smooth_tmp[nb_sensors + 7] = (smooth[nb_sensors + 7] + smooth[nb_sensors + 6]) >> 1;
> +
> +		for (i = 0; i < nb_sensors + 8; i++)
> +			smooth[i] = smooth_tmp[i];
> +	}
> +
> +	for (i = 0; i < nb_sensors + 8; i++) {
> +		if ((smooth[i] >> scale) > 0) {  /* Skip individual values if */
> +			pcum += (smooth[i]) * i; /* they are small enough to  */
> +			psum += (smooth[i]);     /* be truncated to 0 by our  */
> +		}                                /* scale; mostly just noise. */
>  	}
>  
>  	if (psum > 0) {
> -		*z = psum;
> +		*z = psum >> scale;            /* Scale down pressure output. */
>  		return pcum * fact / psum;
>  	}
>  
> @@ -559,8 +593,8 @@ static void atp_complete_geyser_1_2(struct urb *urb)
>  
>  	if (x && y) {
>  		if (dev->x_old != -1) {
> -			x = (dev->x_old * 3 + x) >> 2;
> -			y = (dev->y_old * 3 + y) >> 2;
> +			x = (dev->x_old * 7 + x) >> 3;
> +			y = (dev->y_old * 7 + y) >> 3;
>  			dev->x_old = x;
>  			dev->y_old = y;
>  
> @@ -671,8 +705,8 @@ static void atp_complete_geyser_3_4(struct urb *urb)
>  
>  	if (x && y) {
>  		if (dev->x_old != -1) {
> -			x = (dev->x_old * 3 + x) >> 2;
> -			y = (dev->y_old * 3 + y) >> 2;
> +			x = (dev->x_old * 7 + x) >> 3;
> +			y = (dev->y_old * 7 + y) >> 3;
>  			dev->x_old = x;
>  			dev->y_old = y;
>  
> 

Reviewed-by: Henrik Rydberg <rydberg@euromail.se>

Thanks,
Henrik

Re: [PATCH v4 2/3] input: appletouch: implement sensor data smoothing

[Dmitry Torokhov]

Hi Clinton,

On Wed, Mar 12, 2014 at 06:16:21PM -0500, Clinton Sprain wrote:
> input: appletouch: implement sensor data smoothing
> 
> Use smoothed version of sensor array data to calculate movement and add weight
> to prior values when calculating average. This gives more granular and more
> predictable movement.
> 
> Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
> ---
>  drivers/input/mouse/appletouch.c |   72 ++++++++++++++++++++++++++++----------
>  1 file changed, 53 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
> index 2745832..e00f126 100644
> --- a/drivers/input/mouse/appletouch.c
> +++ b/drivers/input/mouse/appletouch.c
> @@ -332,7 +332,11 @@ static void atp_reinit(struct work_struct *work)
>  static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
>  			     int *z, int *fingers)
>  {
> -	int i;
> +	int i, k;
> +	int smooth[nb_sensors + 8];
> +	int smooth_tmp[nb_sensors + 8];

This unfortunately introduces variable-length arraus on stack which
makes sparse unhappy. Can we add these scratch buffers to the device
structure instead?

> +	int scale = 12;

Probably better use a define rather than a variable.

> +
>  	/* values to calculate mean */
>  	int pcum = 0, psum = 0;
>  	int is_increasing = 0;
> @@ -344,9 +348,6 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
>  			if (is_increasing)
>  				is_increasing = 0;
>  
> -			continue;
> -		}
> -
>  		/*
>  		 * Makes the finger detection more versatile.  For example,
>  		 * two fingers with no gap will be detected.  Also, my
> @@ -361,27 +362,60 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
>  		 *
>  		 * - Jason Parekh <jasonparekh@gmail.com>
>  		 */
> -		if (i < 1 ||
> +
> +		} else if (i < 1 ||
>  		    (!is_increasing && xy_sensors[i - 1] < xy_sensors[i])) {
>  			(*fingers)++;
>  			is_increasing = 1;
>  		} else if (i > 0 && (xy_sensors[i - 1] - xy_sensors[i] > threshold)) {
>  			is_increasing = 0;
>  		}
> +	}
>  
> -		/*
> -		 * Subtracts threshold so a high sensor that just passes the
> -		 * threshold won't skew the calculated absolute coordinate.
> -		 * Fixes an issue where slowly moving the mouse would
> -		 * occasionally jump a number of pixels (slowly moving the
> -		 * finger makes this issue most apparent.)
> -		 */
> -		pcum += (xy_sensors[i] - threshold) * i;
> -		psum += (xy_sensors[i] - threshold);
> +	/*
> +	 * Use a smoothed version of sensor data for movement calculations, to
> +	 * combat noise without needing to rely so heavily on a threshold.
> +	 * This improves tracking.
> +	 *
> +	 * The smoothed array is bigger than the original so that the smoothing
> +	 * doesn't result in edge values being truncated.
> +	 */
> +
> +	for (i = 0; i < 4; i++)
> +		smooth[i] = 0;

memset might be better?

> +	for (i = nb_sensors + 4; i < nb_sensors + 8; i++)
> +		smooth[i] = 0;

And here as well.

Thanks.

-- 
Dmitry

[PATCH v5 2/3] input: appletouch: implement sensor data smoothing

[Clinton Sprain]

input: appletouch: implement sensor data smoothing

Use smoothed version of sensor array data to calculate movement and add weight
to prior values when calculating average. This gives more granular and more
predictable movement.

Changes since v4:
- Scale is now defined as ATP_SCALE
- Smoothing scratch buffers now a part of dev structure
- Now passing dev into atp_calculate_abs
- Now leaving atp_calculate_abs early if no fingers are detected

Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
 drivers/input/mouse/appletouch.c |  110 +++++++++++++++++++++++++++-----------
 1 file changed, 80 insertions(+), 30 deletions(-)

diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index 2745832..8e01363 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -161,6 +161,12 @@ MODULE_DEVICE_TABLE(usb, atp_table);
 #define ATP_XSENSORS	26
 #define ATP_YSENSORS	16
 
+/*
+ * The largest possible bank of sensors with additional buffer of 4 extra values
+ * on either side, for an array of smoothed sensor values.
+ */
+#define ATP_SMOOTHSIZE	34
+
 /* maximum pressure this driver will report */
 #define ATP_PRESSURE	300
 
@@ -168,7 +174,13 @@ MODULE_DEVICE_TABLE(usb, atp_table);
  * Threshold for the touchpad sensors. Any change less than ATP_THRESHOLD is
  * ignored.
  */
-#define ATP_THRESHOLD	 5
+#define ATP_THRESHOLD	5
+
+/*
+ * How far we'll bitshift our sensor values before averaging them. Mitigates
+ * rounding errors.
+ */
+#define ATP_SCALE	12
 
 /* Geyser initialization constants */
 #define ATP_GEYSER_MODE_READ_REQUEST_ID		1
@@ -211,6 +223,8 @@ struct atp {
 	signed char		xy_cur[ATP_XSENSORS + ATP_YSENSORS];
 	signed char		xy_old[ATP_XSENSORS + ATP_YSENSORS];
 	int			xy_acc[ATP_XSENSORS + ATP_YSENSORS];
+	int			smooth[ATP_SMOOTHSIZE];
+	int			smooth_tmp[ATP_SMOOTHSIZE];
 	int			idlecount;	/* number of empty packets */
 	struct work_struct	work;
 };
@@ -329,10 +343,15 @@ static void atp_reinit(struct work_struct *work)
 			retval);
 }
 
-static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
-			     int *z, int *fingers)
+static int atp_calculate_abs(struct atp *dev, int offset, int nb_sensors,
+			     int fact, int *z, int *fingers)
 {
-	int i;
+	int i, k;
+
+	/* Use offset to point xy_sensors at the first value in dev->xy_acc   */
+	/* for whichever dimension we're looking at this particular go-round. */
+	int *xy_sensors = dev->xy_acc + offset;
+
 	/* values to calculate mean */
 	int pcum = 0, psum = 0;
 	int is_increasing = 0;
@@ -344,9 +363,6 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
 			if (is_increasing)
 				is_increasing = 0;
 
-			continue;
-		}
-
 		/*
 		 * Makes the finger detection more versatile.  For example,
 		 * two fingers with no gap will be detected.  Also, my
@@ -361,27 +377,60 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
 		 *
 		 * - Jason Parekh <jasonparekh@gmail.com>
 		 */
-		if (i < 1 ||
+
+		} else if (i < 1 ||
 		    (!is_increasing && xy_sensors[i - 1] < xy_sensors[i])) {
 			(*fingers)++;
 			is_increasing = 1;
 		} else if (i > 0 && (xy_sensors[i - 1] - xy_sensors[i] > threshold)) {
 			is_increasing = 0;
 		}
+	}
 
-		/*
-		 * Subtracts threshold so a high sensor that just passes the
-		 * threshold won't skew the calculated absolute coordinate.
-		 * Fixes an issue where slowly moving the mouse would
-		 * occasionally jump a number of pixels (slowly moving the
-		 * finger makes this issue most apparent.)
-		 */
-		pcum += (xy_sensors[i] - threshold) * i;
-		psum += (xy_sensors[i] - threshold);
+	if (*fingers < 1)     /* No need to continue if no fingers are found. */
+		return 0;
+
+	/*
+	 * Use a smoothed version of sensor data for movement calculations, to
+	 * combat noise without needing to rely so heavily on a threshold.
+	 * This improves tracking.
+	 *
+	 * The smoothed array is bigger than the original so that the smoothing
+	 * doesn't result in edge values being truncated.
+	 */
+
+	memset(dev->smooth, 0, sizeof(dev->smooth));
+	memset(dev->smooth_tmp, 0, sizeof(dev->smooth_tmp));
+
+	/* Pull base values, scaled up to help avoid truncation errors. */
+
+	for (i = 0; i < nb_sensors; i++)
+		dev->smooth[i + 4] = xy_sensors[i] << ATP_SCALE;
+
+	for (k = 0; k < 4; k++) {
+		/* Handle edge. */
+		dev->smooth_tmp[0] = (dev->smooth[0] + dev->smooth[1]) >> 1;
+
+		/* Average values with neighbors. */
+		for (i = 1; i < nb_sensors + 7; i++)
+			dev->smooth_tmp[i] = (dev->smooth[i - 1] + dev->smooth[i] * 2 + dev->smooth[i + 1]) >> 2;
+
+		/* Handle other edge. */
+		dev->smooth_tmp[nb_sensors + 7] = (dev->smooth[nb_sensors + 7] + dev->smooth[nb_sensors + 6]) >> 1;
+
+		for (i = 0; i < nb_sensors + 8; i++)
+			dev->smooth[i] = dev->smooth_tmp[i];
+	}
+
+	for (i = 0; i < nb_sensors + 8; i++) {
+		if ((dev->smooth[i] >> ATP_SCALE) > 0) {  /* Skip values if   */
+			pcum += (dev->smooth[i]) * i; /* they're small enough */
+			psum += (dev->smooth[i]);     /* to be truncated to 0 */
+		}                                  /* by scale. Mostly noise. */
 	}
 
 	if (psum > 0) {
-		*z = psum;
+		*z = psum >> ATP_SCALE;        /* Scale down pressure output. */
 		return pcum * fact / psum;
 	}
 
@@ -551,16 +600,16 @@ static void atp_complete_geyser_1_2(struct urb *urb)
 
 	dbg_dump("accumulator", dev->xy_acc);
 
-	x = atp_calculate_abs(dev->xy_acc, ATP_XSENSORS,
-			      dev->info->xfact, &x_z, &x_f);
-	y = atp_calculate_abs(dev->xy_acc + ATP_XSENSORS, ATP_YSENSORS,
-			      dev->info->yfact, &y_z, &y_f);
+	x = atp_calculate_abs(dev, 0, ATP_XSENSORS, dev->info->xfact,
+			      &x_z, &x_f);
+	y = atp_calculate_abs(dev, ATP_XSENSORS, ATP_YSENSORS, dev->info->yfact,
+			      &y_z, &y_f);
 	key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
 
 	if (x && y) {
 		if (dev->x_old != -1) {
-			x = (dev->x_old * 3 + x) >> 2;
-			y = (dev->y_old * 3 + y) >> 2;
+			x = (dev->x_old * 7 + x) >> 3;
+			y = (dev->y_old * 7 + y) >> 3;
 			dev->x_old = x;
 			dev->y_old = y;
 
@@ -663,16 +712,17 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 
 	dbg_dump("accumulator", dev->xy_acc);
 
-	x = atp_calculate_abs(dev->xy_acc, ATP_XSENSORS,
-			      dev->info->xfact, &x_z, &x_f);
-	y = atp_calculate_abs(dev->xy_acc + ATP_XSENSORS, ATP_YSENSORS,
-			      dev->info->yfact, &y_z, &y_f);
+	x = atp_calculate_abs(dev, 0, ATP_XSENSORS, dev->info->xfact,
+			      &x_z, &x_f);
+	y = atp_calculate_abs(dev, ATP_XSENSORS, ATP_YSENSORS, dev->info->yfact,
+			      &y_z, &y_f);
+
 	key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
 
 	if (x && y) {
 		if (dev->x_old != -1) {
-			x = (dev->x_old * 3 + x) >> 2;
-			y = (dev->y_old * 3 + y) >> 2;
+			x = (dev->x_old * 7 + x) >> 3;
+			y = (dev->y_old * 7 + y) >> 3;
 			dev->x_old = x;
 			dev->y_old = y;
 
-- 
1.7.9.5

[PATCH v5 3/3] input: appletouch: fix jumps when additional fingers are detected

[Clinton Sprain]

input: appletouch: fix jumps when additional fingers are detected

Addresses issues related to when a second finger enters or leaves the field,
causing the cursor to jump or the page to scroll unexpectedly; now, we
discard any movement change that happens at the exact moment we detect a
change in the number of fingers touching the trackpad. This doesn't
completely resolve the issue but does greatly mitigate it.

(No changes since v4)

Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
 drivers/input/mouse/appletouch.c |   27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index 8e01363..39c7ba2 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -218,6 +218,7 @@ struct atp {
 	bool			valid;		/* are the samples valid? */
 	bool			size_detect_done;
 	bool			overflow_warned;
+	int			fingers_old;	/* last reported finger count */
 	int			x_old;		/* last reported x/y, */
 	int			y_old;		/* used for smoothing */
 	signed char		xy_cur[ATP_XSENSORS + ATP_YSENSORS];
@@ -523,7 +524,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
 {
 	int x, y, x_z, y_z, x_f, y_f;
 	int retval, i, j;
-	int key;
+	int key, fingers;
 	struct atp *dev = urb->context;
 	int status = atp_status_check(urb);
 
@@ -606,7 +607,9 @@ static void atp_complete_geyser_1_2(struct urb *urb)
 			      &y_z, &y_f);
 	key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
 
-	if (x && y) {
+	fingers = max(x_f, y_f);
+
+	if (x && y && (fingers == dev->fingers_old)) {
 		if (dev->x_old != -1) {
 			x = (dev->x_old * 7 + x) >> 3;
 			y = (dev->y_old * 7 + y) >> 3;
@@ -623,7 +626,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
 			input_report_abs(dev->input, ABS_Y, y);
 			input_report_abs(dev->input, ABS_PRESSURE,
 					 min(ATP_PRESSURE, x_z + y_z));
-			atp_report_fingers(dev->input, max(x_f, y_f));
+			atp_report_fingers(dev->input, fingers);
 		}
 		dev->x_old = x;
 		dev->y_old = y;
@@ -631,6 +634,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
 	} else if (!x && !y) {
 
 		dev->x_old = dev->y_old = -1;
+		dev->fingers_old = 0;
 		input_report_key(dev->input, BTN_TOUCH, 0);
 		input_report_abs(dev->input, ABS_PRESSURE, 0);
 		atp_report_fingers(dev->input, 0);
@@ -639,6 +643,10 @@ static void atp_complete_geyser_1_2(struct urb *urb)
 		memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
 	}
 
+	if (fingers != dev->fingers_old)
+		dev->x_old = dev->y_old = -1;
+	dev->fingers_old = fingers;
+
 	input_report_key(dev->input, BTN_LEFT, key);
 	input_sync(dev->input);
 
@@ -656,7 +664,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 {
 	int x, y, x_z, y_z, x_f, y_f;
 	int retval, i, j;
-	int key;
+	int key, fingers;
 	struct atp *dev = urb->context;
 	int status = atp_status_check(urb);
 
@@ -719,7 +727,9 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 
 	key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
 
-	if (x && y) {
+	fingers = max(x_f, y_f);
+
+	if (x && y && (fingers == dev->fingers_old)) {
 		if (dev->x_old != -1) {
 			x = (dev->x_old * 7 + x) >> 3;
 			y = (dev->y_old * 7 + y) >> 3;
@@ -736,7 +746,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 			input_report_abs(dev->input, ABS_Y, y);
 			input_report_abs(dev->input, ABS_PRESSURE,
 					 min(ATP_PRESSURE, x_z + y_z));
-			atp_report_fingers(dev->input, max(x_f, y_f));
+			atp_report_fingers(dev->input, fingers);
 		}
 		dev->x_old = x;
 		dev->y_old = y;
@@ -744,6 +754,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 	} else if (!x && !y) {
 
 		dev->x_old = dev->y_old = -1;
+		dev->fingers_old = 0;
 		input_report_key(dev->input, BTN_TOUCH, 0);
 		input_report_abs(dev->input, ABS_PRESSURE, 0);
 		atp_report_fingers(dev->input, 0);
@@ -752,6 +763,10 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 		memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
 	}
 
+	if (fingers != dev->fingers_old)
+		dev->x_old = dev->y_old = -1;
+	dev->fingers_old = fingers;
+
 	input_report_key(dev->input, BTN_LEFT, key);
 	input_sync(dev->input);
 
-- 
1.7.9.5

[PATCH v4 3/3] input: appletouch: fix jumps when additional fingers are detected

[Clinton Sprain]

input: appletouch: fix jumps when additional fingers are detected

Addresses issues related to when a second finger enters or leaves the field,
causing the cursor to jump or the page to scroll unexpectedly; now, we
discard any movement change that happens at the exact moment we detect a
change in the number of fingers touching the trackpad. This doesn't
completely resolve the issue but does greatly mitigate it.

Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
 drivers/input/mouse/appletouch.c |   27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index e00f126..5140293 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -206,6 +206,7 @@ struct atp {
 	bool			valid;		/* are the samples valid? */
 	bool			size_detect_done;
 	bool			overflow_warned;
+	int			fingers_old;	/* last reported finger count */
 	int			x_old;		/* last reported x/y, */
 	int			y_old;		/* used for smoothing */
 	signed char		xy_cur[ATP_XSENSORS + ATP_YSENSORS];
@@ -508,7 +509,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
 {
 	int x, y, x_z, y_z, x_f, y_f;
 	int retval, i, j;
-	int key;
+	int key, fingers;
 	struct atp *dev = urb->context;
 	int status = atp_status_check(urb);
 
@@ -591,7 +592,9 @@ static void atp_complete_geyser_1_2(struct urb *urb)
 			      dev->info->yfact, &y_z, &y_f);
 	key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
 
-	if (x && y) {
+	fingers = max(x_f, y_f);
+
+	if (x && y && (fingers == dev->fingers_old)) {
 		if (dev->x_old != -1) {
 			x = (dev->x_old * 7 + x) >> 3;
 			y = (dev->y_old * 7 + y) >> 3;
@@ -608,7 +611,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
 			input_report_abs(dev->input, ABS_Y, y);
 			input_report_abs(dev->input, ABS_PRESSURE,
 					 min(ATP_PRESSURE, x_z + y_z));
-			atp_report_fingers(dev->input, max(x_f, y_f));
+			atp_report_fingers(dev->input, fingers);
 		}
 		dev->x_old = x;
 		dev->y_old = y;
@@ -616,6 +619,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
 	} else if (!x && !y) {
 
 		dev->x_old = dev->y_old = -1;
+		dev->fingers_old = 0;
 		input_report_key(dev->input, BTN_TOUCH, 0);
 		input_report_abs(dev->input, ABS_PRESSURE, 0);
 		atp_report_fingers(dev->input, 0);
@@ -624,6 +628,10 @@ static void atp_complete_geyser_1_2(struct urb *urb)
 		memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
 	}
 
+	if (fingers != dev->fingers_old)
+		dev->x_old = dev->y_old = -1;
+	dev->fingers_old = fingers;
+
 	input_report_key(dev->input, BTN_LEFT, key);
 	input_sync(dev->input);
 
@@ -641,7 +649,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 {
 	int x, y, x_z, y_z, x_f, y_f;
 	int retval, i, j;
-	int key;
+	int key, fingers;
 	struct atp *dev = urb->context;
 	int status = atp_status_check(urb);
 
@@ -703,7 +711,9 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 			      dev->info->yfact, &y_z, &y_f);
 	key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
 
-	if (x && y) {
+	fingers = max(x_f, y_f);
+
+	if (x && y && (fingers == dev->fingers_old)) {
 		if (dev->x_old != -1) {
 			x = (dev->x_old * 7 + x) >> 3;
 			y = (dev->y_old * 7 + y) >> 3;
@@ -720,7 +730,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 			input_report_abs(dev->input, ABS_Y, y);
 			input_report_abs(dev->input, ABS_PRESSURE,
 					 min(ATP_PRESSURE, x_z + y_z));
-			atp_report_fingers(dev->input, max(x_f, y_f));
+			atp_report_fingers(dev->input, fingers);
 		}
 		dev->x_old = x;
 		dev->y_old = y;
@@ -728,6 +738,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 	} else if (!x && !y) {
 
 		dev->x_old = dev->y_old = -1;
+		dev->fingers_old = 0;
 		input_report_key(dev->input, BTN_TOUCH, 0);
 		input_report_abs(dev->input, ABS_PRESSURE, 0);
 		atp_report_fingers(dev->input, 0);
@@ -736,6 +747,10 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 		memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
 	}
 
+	if (fingers != dev->fingers_old)
+		dev->x_old = dev->y_old = -1;
+	dev->fingers_old = fingers;
+
 	input_report_key(dev->input, BTN_LEFT, key);
 	input_sync(dev->input);
 
-- 
1.7.9.5

Re: [PATCH v4 0/3] input: appletouch: fixes for jagged/uneven cursor movement

[Clinton Sprain]

Ping?

On 03/12/2014 06:13 PM, Clinton Sprain wrote:
> input: appletouch: fixes for jagged/uneven cursor movement
> 
> The appletouch driver can make some touchpads feel insensitive, with movement that tends to jerk in steps. This is particularly evident when moving diagonally. There is also undesirable movement when additional fingers enter or leave the touchpad.
> 
> These patches address these issues by dialing back the fuzz setting, by implementing sensor data smoothing and by discarding movements that directly coincide with a change in the number of fingers detected on the touchpad.
> 
> Changes since v3:
> 
> 1: No change
> 2: Fix loop counter in atp_calculate_abs
> 3: No change
> 
> Clinton Sprain (3):
>   input: appletouch: dial back fuzz setting
>   input: appletouch: implement sensor data smoothing
>   input: appletouch: fix jumps when additional fingers are detected
> 
>  drivers/input/mouse/appletouch.c |  114 +++++++++++++++++++++++++++-----------
>  1 file changed, 83 insertions(+), 31 deletions(-)
> 

Re: [PATCH v4 0/3] input: appletouch: fixes for jagged/uneven cursor movement

[Henrik Rydberg]

> input: appletouch: fixes for jagged/uneven cursor movement
> 
> The appletouch driver can make some touchpads feel insensitive, with movement that tends to jerk in steps. This is particularly evident when moving diagonally. There is also undesirable movement when additional fingers enter or leave the touchpad.
> 
> These patches address these issues by dialing back the fuzz setting, by implementing sensor data smoothing and by discarding movements that directly coincide with a change in the number of fingers detected on the touchpad.

Thanks Clinton, for resolving this long-standing nuisance. It all looks good to
me now.

Henrik