input: appletouch: fixes for jagged/uneven cursor movement

PATCH Superseded Clinton Sprain 4 participants 30 messages Jan 18, 2014 – Mar 29, 2014

Subsystems (2)

HID Input drivers

lore

Share & tools

lore

Clinton Sprain

author

2014-01-18 01:53 UTC #0 lore raw

cover input: appletouch: fixes for jagged/uneven cursor movement

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 <[email protected]>

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

Signature · 1 line

1.7.9.5

Clinton Sprain

author

2014-01-18 01:55 UTC #1 lore raw

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

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 <[email protected]>

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

drivers/input/mouse/appletouch.c +42 ~~-24~~

index e42f1fa..f5a16ee 100644
@@ -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);

Signature · 1 line

1.7.9.5

Henrik Rydberg

2014-01-18 07:03 UTC #2 lore raw

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

Hi Clinton,

Quoted 108 lines

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

Signed-off-by Clinton Sprain <[email protected]>

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

drivers/input/mouse/appletouch.c +14 ~~-11~~

index e42f1fa..f5a16ee 100644
@@ -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)						\

drivers/input/mouse/appletouch.c struct atp { · line 228

Quoted 10 lines

drivers/input/mouse/appletouch.c +3 -1

+{									\
 	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.

Quoted 16 lines

drivers/input/mouse/appletouch.c +1 -5

 #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.");

drivers/input/mouse/appletouch.c MODULE_AUTHOR("Sven Anders"); · line 257

Quoted 10 lines

drivers/input/mouse/appletouch.c +10 -0

+			    " 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.

drivers/input/mouse/appletouch.c static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact, · line 380

Quoted 11 lines

drivers/input/mouse/appletouch.c +2 -1

 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.

drivers/input/mouse/appletouch.c static void atp_detect_size(struct atp *dev) · line 471

Quoted 6 lines

drivers/input/mouse/appletouch.c +1 -1

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

Quoted 37 lines

drivers/input/mouse/appletouch.c +9 -3

 			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,

drivers/input/mouse/appletouch.c static int atp_probe(struct usb_interface *iface, · line 871

Quoted 10 lines

drivers/input/mouse/appletouch.c +2 -2

 			     (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

Clinton Sprain

author

2014-01-18 01:56 UTC #3 lore raw

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

In reply to 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 <[email protected]>

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

drivers/input/mouse/appletouch.c +83 ~~-16~~

index f5a16ee..4a3bbcd 100644
@@ -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 */

Signature · 1 line

1.7.9.5

Clinton Sprain

author

2014-01-18 01:58 UTC #4 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/input/mouse/appletouch.c |   41 ++++++++++++++++++++++++++++++++++----
 1 file changed, 37 insertions(+), 4 deletions(-)

drivers/input/mouse/appletouch.c +37 -4

index 4a3bbcd..406054a 100644
@@ -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);

Signature · 1 line

1.7.9.5

Clinton Sprain

author

2014-02-07 00:38 UTC #5 lore raw

v2 cover input: appletouch: fixes for jagged/uneven cursor movement

In reply to 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 <[email protected]>

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

Clinton Sprain

author

2014-02-07 00:40 UTC #6 lore raw

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

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 <[email protected]>

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

drivers/input/mouse/appletouch.c +31 ~~-17~~

index 800ca7d..d48181b 100644
@@ -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);

Signature · 1 line

1.7.9.5

Henrik Rydberg

2014-02-09 12:17 UTC #7 lore raw

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

Hi Clinton,

Quoted 6 lines · 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 <[email protected]>
---
 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.

Quoted 90 lines

drivers/input/mouse/appletouch.c +13 ~~-14~~

index 800ca7d..d48181b 100644
@@ -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.");

drivers/input/mouse/appletouch.c MODULE_AUTHOR("Sven Anders"); · line 254

Quoted 10 lines

drivers/input/mouse/appletouch.c +10 -0

+			    " 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?

Quoted 28 lines

drivers/input/mouse/appletouch.c +6 -1

 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,

drivers/input/mouse/appletouch.c static int atp_probe(struct usb_interface *iface, · line 866

Quoted 10 lines

drivers/input/mouse/appletouch.c +2 -2

 			     (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

Clinton Sprain

author

2014-02-11 04:46 UTC #8 lore raw

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

Hi Herik,

Quoted 110 lines · Henrik Rydberg

Hi Clinton,

Quoted 6 lines · 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 <[email protected]>
---
 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.

Quoted 97 lines

drivers/input/mouse/appletouch.c +20 ~~-14~~

index 800ca7d..d48181b 100644
@@ -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"

Quoted 8 lines

drivers/input/mouse/appletouch.c MODULE_AUTHOR("Sven Anders"); · line 254
Quoted 3 lines
drivers/input/mouse/appletouch.c +3 -0
+		       " 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.

Quoted 37 lines

drivers/input/mouse/appletouch.c +8 -3

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

Quoted 6 lines

drivers/input/mouse/appletouch.c static int atp_probe(struct usb_interface *iface, · line 866
Quoted 2 lines
drivers/input/mouse/appletouch.c +0 -0
 	set_bit(EV_KEY, input_dev->evbit);
Thanks,
Henrik

Thanks,
Clinton

Clinton Sprain

author

2014-02-07 00:43 UTC #9 lore raw

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

In reply to 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 <[email protected]>

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

drivers/input/mouse/appletouch.c +42 ~~-18~~

index d48181b..edbdd95 100644
@@ -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 <[email protected]>
 		 */
-		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;

Signature · 1 line

1.7.9.5

Henrik Rydberg

2014-02-09 12:13 UTC #10 lore raw

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

Hi Clinton,

Quoted 1 line

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.

Quoted 5 lines

Signed-off-by Clinton Sprain <[email protected]>

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

Quoted 23 lines

drivers/input/mouse/appletouch.c +8 -1

index d48181b..edbdd95 100644
@@ -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

drivers/input/mouse/appletouch.c static void atp_reinit(struct work_struct *work) · line 364

Quoted 10 lines

drivers/input/mouse/appletouch.c +10 -0

+	 * 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];

drivers/input/mouse/appletouch.c static void atp_reinit(struct work_struct *work) · line 367

Quoted 2 lines

drivers/input/mouse/appletouch.c +2 -0

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

drivers/input/mouse/appletouch.c static void atp_reinit(struct work_struct *work) · line 374

Quoted 7 lines

drivers/input/mouse/appletouch.c +7 -0

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

drivers/input/mouse/appletouch.c static void atp_reinit(struct work_struct *work) · line 364

Quoted 4 lines

drivers/input/mouse/appletouch.c +4 -0

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

And these would be dropped.

Quoted 28 lines

drivers/input/mouse/appletouch.c +4 -5

+	}
+
 	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 <[email protected]>
 		 */
-		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;
 		}
 
-		/*

drivers/input/mouse/appletouch.c static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact, · line 410

Quoted 10 lines

drivers/input/mouse/appletouch.c +2 -8

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

Quoted 15 lines

drivers/input/mouse/appletouch.c +2 -2

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

drivers/input/mouse/appletouch.c static void atp_complete_geyser_3_4(struct urb *urb) · line 707

Quoted 9 lines

drivers/input/mouse/appletouch.c +2 -2

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

Clinton Sprain

author

2014-02-11 07:19 UTC #11 lore raw

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

Hi Henrik,

Quoted 51 lines · Henrik Rydberg

Hi Clinton,
Quoted 1 line
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.
Quoted 5 lines
Signed-off-by Clinton Sprain <[email protected]>
---
 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.
Quoted 23 lines
drivers/input/mouse/appletouch.c +8 -1
index d48181b..edbdd95 100644
@@ -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
drivers/input/mouse/appletouch.c static void atp_reinit(struct work_struct *work) · line 364
Quoted 10 lines
drivers/input/mouse/appletouch.c +10 -0
+	 * 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)

Quoted 3 lines

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

Quoted 79 lines

drivers/input/mouse/appletouch.c static void atp_reinit(struct work_struct *work) · line 367

Quoted 2 lines

drivers/input/mouse/appletouch.c +2 -0

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

drivers/input/mouse/appletouch.c static void atp_reinit(struct work_struct *work) · line 374

Quoted 7 lines

drivers/input/mouse/appletouch.c +7 -0

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

drivers/input/mouse/appletouch.c static void atp_reinit(struct work_struct *work) · line 364

Quoted 4 lines

drivers/input/mouse/appletouch.c +4 -0

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

And these would be dropped.

Quoted 28 lines

drivers/input/mouse/appletouch.c +4 -5

+	}
+
 	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 <[email protected]>
 		 */
-		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;
 		}
 
-		/*

drivers/input/mouse/appletouch.c static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact, · line 410

Quoted 10 lines

drivers/input/mouse/appletouch.c +2 -8

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

Quoted 16 lines

drivers/input/mouse/appletouch.c +2 -2

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

Quoted 11 lines

drivers/input/mouse/appletouch.c static void atp_complete_geyser_1_2(struct urb *urb) · line 595
Quoted 8 lines
drivers/input/mouse/appletouch.c +2 -2
 	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.)

Quoted 4 lines

Thanks,
Henrik

Thanks,
Clinton

Clinton Sprain

author

2014-02-07 00:46 UTC #12 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/input/mouse/appletouch.c |   23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

drivers/input/mouse/appletouch.c +19 -4

index edbdd95..370d0e9 100644
@@ -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);

Signature · 1 line

1.7.9.5

Henrik Rydberg

Reviewed-by on v2 3/3

2014-02-09 12:15 UTC #13 lore raw

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

Quoted 88 lines · Clinton Sprain

Signed-off-by Clinton Sprain <[email protected]>

---
 drivers/input/mouse/appletouch.c |   23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

drivers/input/mouse/appletouch.c +15 -4

index edbdd95..370d0e9 100644
@@ -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;

drivers/input/mouse/appletouch.c static void atp_complete_geyser_3_4(struct urb *urb) · line 752

Quoted 10 lines

drivers/input/mouse/appletouch.c +4 -0

@@ -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 <[email protected]>

Thanks,
Henrik

Clinton Sprain

author

2014-03-09 06:03 UTC #14 lore raw

v3 cover input: appletouch: fixes for jagged/uneven cursor movement

In reply to 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 <[email protected]>

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

Signature · 1 line

1.7.9.5

Clinton Sprain

author

2014-03-09 06:05 UTC #15 lore raw

PATCH v3 1/3 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 <[email protected]>

---
 drivers/input/mouse/appletouch.c |   15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

drivers/input/mouse/appletouch.c +9 -6

index 800ca7d..2745832 100644
@@ -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);

Signature · 1 line

1.7.9.5

Clinton Sprain

author

2014-03-09 06:18 UTC #16 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/input/mouse/appletouch.c |   72 ++++++++++++++++++++++++++++----------
 1 file changed, 53 insertions(+), 19 deletions(-)

drivers/input/mouse/appletouch.c +53 ~~-19~~

index 2745832..0252bab 100644
@@ -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 <[email protected]>
 		 */
-		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;

Signature · 1 line

1.7.9.5

Henrik Rydberg

2014-03-09 13:50 UTC #17 lore raw

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

Hi Clinton,

Quoted 3 lines

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:

drivers/input/mouse/appletouch.c static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact, · line 407

Quoted 5 lines

drivers/input/mouse/appletouch.c +5 -0

+		/* 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

Clinton Sprain

author

2014-03-09 15:03 UTC #18 lore raw

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

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

Quoted 19 lines · Henrik Rydberg

Hi Clinton,
Quoted 3 lines
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:
drivers/input/mouse/appletouch.c static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact, · line 407
Quoted 5 lines
drivers/input/mouse/appletouch.c +5 -0
+		/* 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

Clinton Sprain

author

2014-03-09 06:27 UTC #19 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/input/mouse/appletouch.c |   27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

drivers/input/mouse/appletouch.c +21 -6

index 0252bab..0c7bc6e 100644
@@ -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);

Signature · 1 line

1.7.9.5

Clinton Sprain

author

2014-03-12 23:13 UTC #20 lore raw

v4 cover input: appletouch: fixes for jagged/uneven cursor movement

In reply to 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(-)

Signature · 1 line

1.7.9.5

Clinton Sprain

author

2014-03-12 23:15 UTC #21 lore raw

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

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 <[email protected]>

---
 drivers/input/mouse/appletouch.c |   15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

drivers/input/mouse/appletouch.c +9 -6

index 800ca7d..2745832 100644
@@ -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);

Signature · 1 line

1.7.9.5

Clinton Sprain

author

2014-03-12 23:16 UTC #22 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/input/mouse/appletouch.c |   72 ++++++++++++++++++++++++++++----------
 1 file changed, 53 insertions(+), 19 deletions(-)

drivers/input/mouse/appletouch.c +53 ~~-19~~

index 2745832..e00f126 100644
@@ -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 <[email protected]>
 		 */
-		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;

Signature · 1 line

1.7.9.5

Henrik Rydberg

Reviewed-by on v4 2/3

2014-03-23 14:06 UTC #23 lore raw

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

Hi Clinton,

Quoted 123 lines

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 <[email protected]>

---
 drivers/input/mouse/appletouch.c |   72 ++++++++++++++++++++++++++++----------
 1 file changed, 53 insertions(+), 19 deletions(-)

drivers/input/mouse/appletouch.c +51 ~~-17~~

index 2745832..e00f126 100644
@@ -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 <[email protected]>
 		 */
-		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;

drivers/input/mouse/appletouch.c static void atp_complete_geyser_3_4(struct urb *urb) · line 712

Quoted 10 lines

drivers/input/mouse/appletouch.c +2 -2

@@ -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 <[email protected]>

Thanks,
Henrik

Dmitry Torokhov

maintainer

2014-03-27 18:26 UTC #24 lore raw

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

In reply to Clinton Sprain

Hi Clinton,

Quoted 16 lines · 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 <[email protected]>
---
 drivers/input/mouse/appletouch.c |   72 ++++++++++++++++++++++++++++----------
 1 file changed, 53 insertions(+), 19 deletions(-)
drivers/input/mouse/appletouch.c +0 -0
index 2745832..e00f126 100644

drivers/input/mouse/appletouch.c static void atp_reinit(struct work_struct *work) · line 337

Quoted 7 lines

drivers/input/mouse/appletouch.c +3 -1

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

drivers/input/mouse/appletouch.c static void atp_reinit(struct work_struct *work) · line 338

Quoted 1 line

drivers/input/mouse/appletouch.c +1 -0
+	int scale = 12;

Probably better use a define rather than a variable.

Quoted 39 lines

drivers/input/mouse/appletouch.c +5 ~~-13~~

+
 	/* 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 <[email protected]>
 		 */
-		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);
+	/*

drivers/input/mouse/appletouch.c static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact, · line 385

Quoted 10 lines

drivers/input/mouse/appletouch.c +10 -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.
+	 */
+
+	for (i = 0; i < 4; i++)
+		smooth[i] = 0;

memset might be better?

drivers/input/mouse/appletouch.c static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact, · line 387

Quoted 2 lines

drivers/input/mouse/appletouch.c +2 -0

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

And here as well.

Thanks.

Signature · 1 line

Dmitry

Clinton Sprain

author

2014-03-29 21:48 UTC #25 lore raw

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

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 <[email protected]>

---
 drivers/input/mouse/appletouch.c |  110 +++++++++++++++++++++++++++-----------
 1 file changed, 80 insertions(+), 30 deletions(-)

drivers/input/mouse/appletouch.c +80 ~~-30~~

index 2745832..8e01363 100644
@@ -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 <[email protected]>
 		 */
-		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;

Signature · 1 line

1.7.9.5

Clinton Sprain

author

2014-03-29 21:48 UTC #26 lore raw

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

In reply to Dmitry Torokhov

input: appletouch: fix jumps when additional fingers are detected

(No changes since v4)

Signed-off-by Clinton Sprain <[email protected]>

---
 drivers/input/mouse/appletouch.c |   27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

drivers/input/mouse/appletouch.c +21 -6

index 8e01363..39c7ba2 100644
@@ -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);

Signature · 1 line

1.7.9.5

Clinton Sprain

author

2014-03-12 23:18 UTC #27 lore raw

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

In reply to Clinton Sprain

input: appletouch: fix jumps when additional fingers are detected

Signed-off-by Clinton Sprain <[email protected]>

---
 drivers/input/mouse/appletouch.c |   27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

drivers/input/mouse/appletouch.c +21 -6

index e00f126..5140293 100644
@@ -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);

Signature · 1 line

1.7.9.5

Clinton Sprain

author

2014-03-23 13:34 UTC #28 lore raw

In reply to Clinton Sprain

Ping?

Quoted 20 lines · 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(-)

Henrik Rydberg

2014-03-23 14:09 UTC #29 lore raw

In reply to Clinton Sprain

Quoted 5 lines

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