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

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

[Clinton Sprain]

From: Clinton Sprain <clintonsprain@gmail.com>

For many devices, the appletouch driver can make the trackpad feel
insensitive and cursor movement will tend to jerk in steps. This is
particularly noticeable when moving the cursor diagonally. This can
greatly impact the trackpad's usability.

The proposed changes address this by dialing back the fuzz and threshold
parameters and by implementing a new cursor movement smoothing algorithm.

User feedback couldn't be found for some older devices that use this driver,
so module parameters have been added to allow users to manually set the
fuzz and threshold settings and/or opt out of the new smoothing algorithm.

Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
Tested-by: Curtis Rueden <ctrueden@gmail.com>

Clinton Sprain (2):
  input: appletouch: parametrize and set saner defaults for fuzz and
    threshold
  input: appletouch: use better cursor movement smoothing algorithm

 drivers/input/mouse/appletouch.c |  184 +++++++++++++++++++++++++++++---------
 1 file changed, 142 insertions(+), 42 deletions(-)

--
1.7.9.5

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

[Clinton Sprain]

From: Clinton Sprain <clintonsprain@gmail.com>

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..ba6bbd7 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	= 2,
 };

 #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

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

[Clinton Sprain]

From: Clinton Sprain <clintonsprain@gmail.com>

Implements a new algorithm used to smooth cursor movement, using the inertia
of the cursor's prior movements 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 |  118 ++++++++++++++++++++++++++++++++------
 1 file changed, 100 insertions(+), 18 deletions(-)

diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index ba6bbd7..7a33188 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,59 @@ 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)
+{
+	int diff, projected, olderprojected, smooth;
+
+	diff = old - older;
+	projected = old + diff;
+
+	diff = older - oldest;
+	olderprojected = older + diff;
+
+	smooth = (projected * 7 + olderprojected * 8 + curr) >> 4;
+
+	return smooth;
+}
+
+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)
 {
@@ -386,8 +449,13 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
 		 * 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 (legacy == true) {
+			pcum += (xy_sensors[i] - threshold) * i;
+			psum += (xy_sensors[i] - threshold);
+		} else {
+			pcum += (xy_sensors[i]) * i;
+			psum += (xy_sensors[i]);
+		}
 	}

 	if (psum > 0) {
@@ -570,10 +638,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 +663,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 +757,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 +782,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 +812,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