[PATCH 1/3] Input: Fixed ABS_MT_TOUCH_MINOR scale factor in BCM5974 multitouch driver

[PATCH 1/3] Input: Fixed ABS_MT_TOUCH_MINOR scale factor in BCM5974 multitouch driver

[Friedrich Schöller]

On wellspring3 devices ABS_MT_TOUCH_MINOR was sometimes reported bigger than
ABS_MT_TOUCH_MAJOR. This is fixed by rescaling ABS_MT_TOUCH_MINOR by a factor of
0.85 instead of 2. Excessive tapping on the trackpad shows this to be the right
value. Circular touches should now lead to values for ABS_MT_TOUCH_MAJOR and
ABS_MT_TOUCH_MINOR that are similar, with ABS_MT_TOUCH_MINOR never greater than
ABS_MT_TOUCH_MAJOR.
---
 drivers/input/mouse/bcm5974.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/input/mouse/bcm5974.c b/drivers/input/mouse/bcm5974.c
index a73f961..af15410 100644
--- a/drivers/input/mouse/bcm5974.c
+++ b/drivers/input/mouse/bcm5974.c
@@ -235,6 +235,7 @@ struct bcm5974_config {
 	enum tp_type tp_type;	/* type of trackpad interface */
 	int tp_offset;		/* offset to trackpad finger data */
 	int tp_datalen;		/* data length of the trackpad interface */
+	float touch_minor_f;	/* factor for ABS_MT_TOUCH_MINOR */
 	struct bcm5974_param p;	/* finger pressure limits */
 	struct bcm5974_param w;	/* finger width limits */
 	struct bcm5974_param x;	/* horizontal limits */
@@ -275,6 +276,7 @@ static const struct bcm5974_config bcm5974_config_table[] = {
 		0,
 		0x84, sizeof(struct bt_data),
 		0x81, TYPE1, FINGER_TYPE1, FINGER_TYPE1 + SIZEOF_ALL_FINGERS,
+		2, /* verify me! */
 		{ SN_PRESSURE, 0, 256 },
 		{ SN_WIDTH, 0, 2048 },
 		{ SN_COORD, -4824, 5342 },
@@ -288,6 +290,7 @@ static const struct bcm5974_config bcm5974_config_table[] = {
 		0,
 		0x84, sizeof(struct bt_data),
 		0x81, TYPE1, FINGER_TYPE1, FINGER_TYPE1 + SIZEOF_ALL_FINGERS,
+		2, /* verify me! */
 		{ SN_PRESSURE, 0, 256 },
 		{ SN_WIDTH, 0, 2048 },
 		{ SN_COORD, -4824, 4824 },
@@ -301,6 +304,7 @@ static const struct bcm5974_config bcm5974_config_table[] = {
 		HAS_INTEGRATED_BUTTON,
 		0x84, sizeof(struct bt_data),
 		0x81, TYPE2, FINGER_TYPE2, FINGER_TYPE2 + SIZEOF_ALL_FINGERS,
+		0.85,
 		{ SN_PRESSURE, 0, 300 },
 		{ SN_WIDTH, 0, 2048 },
 		{ SN_COORD, -4460, 5166 },
@@ -314,6 +318,7 @@ static const struct bcm5974_config bcm5974_config_table[] = {
 		HAS_INTEGRATED_BUTTON,
 		0x84, sizeof(struct bt_data),
 		0x81, TYPE2, FINGER_TYPE2, FINGER_TYPE2 + SIZEOF_ALL_FINGERS,
+		2, /* verify me! */
 		{ SN_PRESSURE, 0, 300 },
 		{ SN_WIDTH, 0, 2048 },
 		{ SN_COORD, -4620, 5140 },
@@ -327,6 +332,7 @@ static const struct bcm5974_config bcm5974_config_table[] = {
 		HAS_INTEGRATED_BUTTON,
 		0x84, sizeof(struct bt_data),
 		0x81, TYPE2, FINGER_TYPE2, FINGER_TYPE2 + SIZEOF_ALL_FINGERS,
+		2, /* verify me! */
 		{ SN_PRESSURE, 0, 300 },
 		{ SN_WIDTH, 0, 2048 },
 		{ SN_COORD, -4616, 5112 },
@@ -340,6 +346,7 @@ static const struct bcm5974_config bcm5974_config_table[] = {
 		HAS_INTEGRATED_BUTTON,
 		0x84, sizeof(struct bt_data),
 		0x81, TYPE2, FINGER_TYPE2, FINGER_TYPE2 + SIZEOF_ALL_FINGERS,
+		2, /* verify me! */
 		{ SN_PRESSURE, 0, 300 },
 		{ SN_WIDTH, 0, 2048 },
 		{ SN_COORD, -4415, 5050 },
@@ -353,6 +360,7 @@ static const struct bcm5974_config bcm5974_config_table[] = {
 		HAS_INTEGRATED_BUTTON,
 		0x84, sizeof(struct bt_data),
 		0x81, TYPE2, FINGER_TYPE2, FINGER_TYPE2 + SIZEOF_ALL_FINGERS,
+		2, /* verify me! */
 		{ SN_PRESSURE, 0, 300 },
 		{ SN_WIDTH, 0, 2048 },
 		{ SN_COORD, -4620, 5140 },
@@ -366,6 +374,7 @@ static const struct bcm5974_config bcm5974_config_table[] = {
 		HAS_INTEGRATED_BUTTON,
 		0x84, sizeof(struct bt_data),
 		0x81, TYPE2, FINGER_TYPE2, FINGER_TYPE2 + SIZEOF_ALL_FINGERS,
+		2, /* verify me! */
 		{ SN_PRESSURE, 0, 300 },
 		{ SN_WIDTH, 0, 2048 },
 		{ SN_COORD, -4750, 5280 },
@@ -379,6 +388,7 @@ static const struct bcm5974_config bcm5974_config_table[] = {
 		HAS_INTEGRATED_BUTTON,
 		0x84, sizeof(struct bt_data),
 		0x81, TYPE2, FINGER_TYPE2, FINGER_TYPE2 + SIZEOF_ALL_FINGERS,
+		2, /* verify me! */
 		{ SN_PRESSURE, 0, 300 },
 		{ SN_WIDTH, 0, 2048 },
 		{ SN_COORD, -4620, 5140 },
@@ -392,6 +402,7 @@ static const struct bcm5974_config bcm5974_config_table[] = {
 		HAS_INTEGRATED_BUTTON,
 		0x84, sizeof(struct bt_data),
 		0x81, TYPE2, FINGER_TYPE2, FINGER_TYPE2 + SIZEOF_ALL_FINGERS,
+		2, /* verify me! */
 		{ SN_PRESSURE, 0, 300 },
 		{ SN_WIDTH, 0, 2048 },
 		{ SN_COORD, -4750, 5280 },
@@ -405,6 +416,7 @@ static const struct bcm5974_config bcm5974_config_table[] = {
 		HAS_INTEGRATED_BUTTON,
 		0x84, sizeof(struct bt_data),
 		0x81, TYPE2, FINGER_TYPE2, FINGER_TYPE2 + SIZEOF_ALL_FINGERS,
+		2, /* verify me! */
 		{ SN_PRESSURE, 0, 300 },
 		{ SN_WIDTH, 0, 2048 },
 		{ SN_COORD, -4750, 5280 },
@@ -418,6 +430,7 @@ static const struct bcm5974_config bcm5974_config_table[] = {
 		HAS_INTEGRATED_BUTTON,
 		0, sizeof(struct bt_data),
 		0x83, TYPE3, FINGER_TYPE3, FINGER_TYPE3 + SIZEOF_ALL_FINGERS,
+		2, /* verify me! */
 		{ SN_PRESSURE, 0, 300 },
 		{ SN_WIDTH, 0, 2048 },
 		{ SN_COORD, -4620, 5140 },
@@ -502,7 +515,8 @@ static int report_bt_state(struct bcm5974 *dev, int size)
 	return 0;
 }
 
-static void report_finger_data(struct input_dev *input, int slot,
+static void report_finger_data(struct input_dev *input,
+			       const struct bcm5974_config *cfg, int slot,
 			       const struct input_mt_pos *pos,
 			       const struct tp_finger *f)
 {
@@ -512,7 +526,7 @@ static void report_finger_data(struct input_dev *input, int slot,
 	input_report_abs(input, ABS_MT_TOUCH_MAJOR,
 			 raw2int(f->touch_major) << 1);
 	input_report_abs(input, ABS_MT_TOUCH_MINOR,
-			 raw2int(f->touch_minor) << 1);
+			 raw2int(f->touch_minor) * cfg->touch_minor_f);
 	input_report_abs(input, ABS_MT_WIDTH_MAJOR,
 			 raw2int(f->tool_major) << 1);
 	input_report_abs(input, ABS_MT_WIDTH_MINOR,
@@ -568,7 +582,7 @@ static int report_tp_state(struct bcm5974 *dev, int size)
 	input_mt_assign_slots(input, dev->slots, dev->pos, n);
 
 	for (i = 0; i < n; i++)
-		report_finger_data(input, dev->slots[i],
+		report_finger_data(input, c, dev->slots[i],
 				   &dev->pos[i], dev->index[i]);
 
 	input_mt_sync_frame(input);
-- 
1.8.4.2

[PATCH 2/3] Input: Fixed pressure and tool width calculation in BCM5974 multitouch driver

[Friedrich Schöller]

Previously ABS_PRESSURE and ABS_TOOL_WIDTH were calculated by looking at the
size of the first touch in the list reported by the trackpad. This is not
necessarily the same touch as the one used to perform pointer emulation in the
input multitouch library (input-mt). By using the sum of the sizes of all
touches as a basis for this calculation we get more coherent values.
---
 drivers/input/mouse/bcm5974.c | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/drivers/input/mouse/bcm5974.c b/drivers/input/mouse/bcm5974.c
index af15410..ecbf359 100644
--- a/drivers/input/mouse/bcm5974.c
+++ b/drivers/input/mouse/bcm5974.c
@@ -539,19 +539,10 @@ static void report_finger_data(struct input_dev *input,
 
 static void report_synaptics_data(struct input_dev *input,
 				  const struct bcm5974_config *cfg,
-				  const struct tp_finger *f, int raw_n)
+				  int p, int w)
 {
-	int abs_p = 0, abs_w = 0;
-
-	if (raw_n) {
-		int p = raw2int(f->touch_major);
-		int w = raw2int(f->tool_major);
-		if (p > 0 && raw2int(f->origin)) {
-			abs_p = clamp_val(256 * p / cfg->p.max, 0, 255);
-			abs_w = clamp_val(16 * w / cfg->w.max, 0, 15);
-		}
-	}
-
+	int abs_p = clamp_val(256 * p / cfg->p.max, 0, 255);
+	int abs_w = clamp_val(16 * w / cfg->w.max, 0, 15);
 	input_report_abs(input, ABS_PRESSURE, abs_p);
 	input_report_abs(input, ABS_TOOL_WIDTH, abs_w);
 }
@@ -562,7 +553,7 @@ static int report_tp_state(struct bcm5974 *dev, int size)
 	const struct bcm5974_config *c = &dev->cfg;
 	const struct tp_finger *f;
 	struct input_dev *input = dev->input;
-	int raw_n, i, n = 0;
+	int raw_n, i, n = 0, p = 0, w = 0;
 
 	if (size < c->tp_offset || (size - c->tp_offset) % SIZEOF_FINGER != 0)
 		return -EIO;
@@ -577,6 +568,8 @@ static int report_tp_state(struct bcm5974 *dev, int size)
 		dev->pos[n].x = raw2int(f[i].abs_x);
 		dev->pos[n].y = c->y.min + c->y.max - raw2int(f[i].abs_y);
 		dev->index[n++] = &f[i];
+		p += raw2int(f[i].touch_major);
+		w += raw2int(f[i].tool_major);
 	}
 
 	input_mt_assign_slots(input, dev->slots, dev->pos, n);
@@ -587,7 +580,7 @@ static int report_tp_state(struct bcm5974 *dev, int size)
 
 	input_mt_sync_frame(input);
 
-	report_synaptics_data(input, c, f, raw_n);
+	report_synaptics_data(input, c, p, w);
 
 	/* type 2 reports button events via ibt only */
 	if (c->tp_type == TYPE2) {
-- 
1.8.4.2

[PATCH 3/3] Input: Added thumb detection in BCM5974 multitouch driver

[Friedrich Schöller]

Trackpads with integrated buttons are hard to use when the driver responds to
movements of the thumb that is resting or clicking on the surface of the
trackpad. This patch adds rudimentary support to filter out these touch events.

The feature can be turned on via sysfs:
	/sys/class/input/input[0-9]+/thumb_ignore:
		Enables thumb detection
		Values: 0/1
	/sys/class/input/input[0-9]+/thumb_ratio_on:
		When the ratio of ABS_MT_TOUCH_MINOR / ABS_MT_TOUCH_MAJOR
		times 100 is smaller than this value the touch qualifies
		as a thumb.
	/sys/class/input/input[0-9]+/thumb_ratio_off:
		When the ratio of ABS_MT_TOUCH_MINOR / ABS_MT_TOUCH_MAJOR
		times 100 is bigger than this value the touch no longer
		qualifies as a thumb.
	/sys/class/input/input[0-9]+/thumb_y_on:
		When ABS_MT_POSITION_Y is bigger than this value the touch
		qualifies as a thumb.
	/sys/class/input/input[0-9]+/thumb_y_off:
		When ABS_MT_POSITION_Y is smaller than this value the touch
		no longer qualifies as a thumb.
---
 drivers/input/mouse/bcm5974.c | 150 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 150 insertions(+)

diff --git a/drivers/input/mouse/bcm5974.c b/drivers/input/mouse/bcm5974.c
index ecbf359..826cdb4 100644
--- a/drivers/input/mouse/bcm5974.c
+++ b/drivers/input/mouse/bcm5974.c
@@ -259,6 +259,12 @@ struct bcm5974 {
 	const struct tp_finger *index[MAX_FINGERS];	/* finger index data */
 	struct input_mt_pos pos[MAX_FINGERS];		/* position array */
 	int slots[MAX_FINGERS];				/* slot assignments */
+	bool thb_ignore;		/* ignore thumb */
+	unsigned int thb_r_on;		/* ratio to start ignoring thumb */
+	unsigned int thb_r_off;		/* ratio to stop ignoring thumb */
+	int thb_y_on;			/* y coord. to start ignoring thumb */
+	int thb_y_off;			/* y coord. to stop ignoring thumb */
+	bool thb_found;			/* thumb detected */
 };
 
 /* logical signal quality */
@@ -554,6 +560,7 @@ static int report_tp_state(struct bcm5974 *dev, int size)
 	const struct tp_finger *f;
 	struct input_dev *input = dev->input;
 	int raw_n, i, n = 0, p = 0, w = 0;
+	int thb_r = 0, thb_y = 0;
 
 	if (size < c->tp_offset || (size - c->tp_offset) % SIZEOF_FINGER != 0)
 		return -EIO;
@@ -562,11 +569,30 @@ static int report_tp_state(struct bcm5974 *dev, int size)
 	f = (const struct tp_finger *)(dev->tp_data + c->tp_offset);
 	raw_n = (size - c->tp_offset) / SIZEOF_FINGER;
 
+	if (dev->thb_ignore) {
+		if (dev->thb_found) {
+			thb_r = dev->thb_r_off;
+			thb_y = dev->thb_y_off;
+		} else {
+			thb_r = dev->thb_r_on;
+			thb_y = dev->thb_y_on;
+		}
+		dev->thb_found = false;
+	}
+
 	for (i = 0; i < raw_n; i++) {
 		if (raw2int(f[i].touch_major) == 0)
 			continue;
 		dev->pos[n].x = raw2int(f[i].abs_x);
 		dev->pos[n].y = c->y.min + c->y.max - raw2int(f[i].abs_y);
+
+		if (dev->thb_ignore && thb_y < dev->pos[n].y &&
+		    thb_r * 2 * raw2int(f[i].touch_major) >
+		    100 * c->touch_minor_f * raw2int(f[i].touch_minor)) {
+			dev->thb_found = true;
+			continue;
+		}
+
 		dev->index[n++] = &f[i];
 		p += raw2int(f[i].touch_major);
 		w += raw2int(f[i].tool_major);
@@ -596,6 +622,118 @@ static int report_tp_state(struct bcm5974 *dev, int size)
 	return 0;
 }
 
+static ssize_t bcm5974_thb_ignore_show(struct device *dev,
+				       struct device_attribute *attr, char *buf)
+{
+	struct bcm5974 *bcm5974_dev = dev_get_drvdata(dev);
+	return scnprintf(buf, PAGE_SIZE, "%d\n", bcm5974_dev->thb_ignore);
+}
+
+static ssize_t bcm5974_thb_ignore_store(struct device *dev,
+					struct device_attribute *attr,
+					const char *buf, size_t count)
+{
+	struct bcm5974 *bcm5974_dev = dev_get_drvdata(dev);
+	int val;
+
+	unsigned int error = kstrtoint(buf, 10, &val);
+	if (error)
+		return error;
+
+	bcm5974_dev->thb_ignore = !!val;
+	bcm5974_dev->thb_found = false;
+
+	return count;
+}
+
+static ssize_t bcm5974_thb_r_on_show(struct device *dev,
+				     struct device_attribute *attr, char *buf)
+{
+	struct bcm5974 *bcm5974_dev = dev_get_drvdata(dev);
+	return scnprintf(buf, PAGE_SIZE, "%d%%\n", bcm5974_dev->thb_r_on);
+}
+
+static ssize_t bcm5974_thb_r_on_store(struct device *dev,
+				      struct device_attribute *attr,
+				      const char *buf, size_t count)
+{
+	struct bcm5974 *bcm5974_dev = dev_get_drvdata(dev);
+	unsigned int error = kstrtouint(buf, 10, &bcm5974_dev->thb_r_on);
+	return error ? error : count;
+}
+
+static ssize_t bcm5974_thb_r_off_show(struct device *dev,
+				      struct device_attribute *attr, char *buf)
+{
+	struct bcm5974 *bcm5974_dev = dev_get_drvdata(dev);
+	return scnprintf(buf, PAGE_SIZE, "%d%%\n", bcm5974_dev->thb_r_off);
+}
+
+static ssize_t bcm5974_thb_r_off_store(struct device *dev,
+				       struct device_attribute *attr,
+				       const char *buf, size_t count)
+{
+	struct bcm5974 *bcm5974_dev = dev_get_drvdata(dev);
+	unsigned int error = kstrtouint(buf, 10, &bcm5974_dev->thb_r_off);
+	return error ? error : count;
+}
+
+static ssize_t bcm5974_thb_y_on_show(struct device *dev,
+				     struct device_attribute *attr, char *buf)
+{
+	struct bcm5974 *bcm5974_dev = dev_get_drvdata(dev);
+	return scnprintf(buf, PAGE_SIZE, "%d\n", bcm5974_dev->thb_y_on);
+}
+
+static ssize_t bcm5974_thb_y_on_store(struct device *dev,
+				      struct device_attribute *attr,
+				      const char *buf, size_t count)
+{
+	struct bcm5974 *bcm5974_dev = dev_get_drvdata(dev);
+	int error = kstrtoint(buf, 10, &bcm5974_dev->thb_y_on);
+	return error ? error : count;
+}
+
+static ssize_t bcm5974_thb_y_off_show(struct device *dev,
+				      struct device_attribute *attr, char *buf)
+{
+	struct bcm5974 *bcm5974_dev = dev_get_drvdata(dev);
+	return scnprintf(buf, PAGE_SIZE, "%d\n", bcm5974_dev->thb_y_off);
+}
+
+static ssize_t bcm5974_thb_y_off_store(struct device *dev,
+				       struct device_attribute *attr,
+				       const char *buf, size_t count)
+{
+	struct bcm5974 *bcm5974_dev = dev_get_drvdata(dev);
+	int error = kstrtoint(buf, 10, &bcm5974_dev->thb_y_off);
+	return error ? error : count;
+}
+
+static DEVICE_ATTR(thumb_ignore, 0664, bcm5974_thb_ignore_show,
+		   bcm5974_thb_ignore_store);
+static DEVICE_ATTR(thumb_ratio_on, 0664, bcm5974_thb_r_on_show,
+		   bcm5974_thb_r_on_store);
+static DEVICE_ATTR(thumb_ratio_off, 0664, bcm5974_thb_r_off_show,
+		   bcm5974_thb_r_off_store);
+static DEVICE_ATTR(thumb_y_on, 0664, bcm5974_thb_y_on_show,
+		   bcm5974_thb_y_on_store);
+static DEVICE_ATTR(thumb_y_off, 0664, bcm5974_thb_y_off_show,
+		   bcm5974_thb_y_off_store);
+
+static struct attribute *bcm5974_attributes[] = {
+	&dev_attr_thumb_ignore.attr,
+	&dev_attr_thumb_ratio_on.attr,
+	&dev_attr_thumb_ratio_off.attr,
+	&dev_attr_thumb_y_on.attr,
+	&dev_attr_thumb_y_off.attr,
+	NULL
+};
+
+static const struct attribute_group bcm5974_attr_group = {
+	.attrs = bcm5974_attributes,
+};
+
 /* Wellspring initialization constants */
 #define BCM5974_WELLSPRING_MODE_READ_REQUEST_ID		1
 #define BCM5974_WELLSPRING_MODE_WRITE_REQUEST_ID	9
@@ -880,6 +1018,11 @@ static int bcm5974_probe(struct usb_interface *iface,
 	dev->cfg = *cfg;
 	mutex_init(&dev->pm_mutex);
 
+	dev->thb_r_on  = 40;
+	dev->thb_r_off = 60;
+	dev->thb_y_on  = (cfg->y.max - cfg->y.min) * 0.75 + cfg->y.min;
+	dev->thb_y_off = (cfg->y.max - cfg->y.min) * 0.73 + cfg->y.min;
+
 	/* setup urbs */
 	if (cfg->tp_type == TYPE1) {
 		dev->bt_urb = usb_alloc_urb(0, GFP_KERNEL);
@@ -941,8 +1084,14 @@ static int bcm5974_probe(struct usb_interface *iface,
 	/* save our data pointer in this interface device */
 	usb_set_intfdata(iface, dev);
 
+	error = sysfs_create_group(&input_dev->dev.kobj, &bcm5974_attr_group);
+	if (error)
+		goto err_unregister_input;
+
 	return 0;
 
+err_unregister_input:
+	input_unregister_device(dev->input);
 err_free_buffer:
 	usb_free_coherent(dev->udev, dev->cfg.tp_datalen,
 		dev->tp_data, dev->tp_urb->transfer_dma);
@@ -967,6 +1116,7 @@ static void bcm5974_disconnect(struct usb_interface *iface)
 
 	usb_set_intfdata(iface, NULL);
 
+	sysfs_remove_group(&dev->input->dev.kobj, &bcm5974_attr_group);
 	input_unregister_device(dev->input);
 	usb_free_coherent(dev->udev, dev->cfg.tp_datalen,
 			  dev->tp_data, dev->tp_urb->transfer_dma);
-- 
1.8.4.2

Re: [PATCH 3/3] Input: Added thumb detection in BCM5974 multitouch driver

[Dmitry Torokhov]

Hi Friedrich,

On Wed, Nov 20, 2013 at 11:54:12PM +0100, Friedrich Schöller wrote:
> Trackpads with integrated buttons are hard to use when the driver responds to
> movements of the thumb that is resting or clicking on the surface of the
> trackpad. This patch adds rudimentary support to filter out these touch events.
> 
> The feature can be turned on via sysfs:
> 	/sys/class/input/input[0-9]+/thumb_ignore:
> 		Enables thumb detection
> 		Values: 0/1
> 	/sys/class/input/input[0-9]+/thumb_ratio_on:
> 		When the ratio of ABS_MT_TOUCH_MINOR / ABS_MT_TOUCH_MAJOR
> 		times 100 is smaller than this value the touch qualifies
> 		as a thumb.
> 	/sys/class/input/input[0-9]+/thumb_ratio_off:
> 		When the ratio of ABS_MT_TOUCH_MINOR / ABS_MT_TOUCH_MAJOR
> 		times 100 is bigger than this value the touch no longer
> 		qualifies as a thumb.
> 	/sys/class/input/input[0-9]+/thumb_y_on:
> 		When ABS_MT_POSITION_Y is bigger than this value the touch
> 		qualifies as a thumb.
> 	/sys/class/input/input[0-9]+/thumb_y_off:
> 		When ABS_MT_POSITION_Y is smaller than this value the touch
> 		no longer qualifies as a thumb.

I'd rather this been implemented in userspace (synaptics and/or evdev X
drivers).

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] Input: Fixed ABS_MT_TOUCH_MINOR scale factor in BCM5974 multitouch driver

[Henrik Rydberg]

Hi Friedrich,

> On wellspring3 devices ABS_MT_TOUCH_MINOR was sometimes reported bigger than
> ABS_MT_TOUCH_MAJOR. This is fixed by rescaling ABS_MT_TOUCH_MINOR by a factor of
> 0.85 instead of 2. Excessive tapping on the trackpad shows this to be the right
> value. Circular touches should now lead to values for ABS_MT_TOUCH_MAJOR and
> ABS_MT_TOUCH_MINOR that are similar, with ABS_MT_TOUCH_MINOR never greater than
> ABS_MT_TOUCH_MAJOR.
> ---
>  drivers/input/mouse/bcm5974.c | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)

The major/minor scales are following the aspect ratio of the device, and as such
it could happen that minor > major. Most userland drivers do not use the finger
width limits, which are estimates, but only the device axes limit, which are
accurate.

Also, we cannot have floats in the kernel.

I would consider a patch which tries to make the finger widths more accurate,
but all in all, I think this problem is best solved in userland.

Thanks,
Henrik

Re: [PATCH 2/3] Input: Fixed pressure and tool width calculation in BCM5974 multitouch driver

[Henrik Rydberg]

Hi Friedrich,

> Previously ABS_PRESSURE and ABS_TOOL_WIDTH were calculated by looking at the
> size of the first touch in the list reported by the trackpad. This is not
> necessarily the same touch as the one used to perform pointer emulation in the
> input multitouch library (input-mt). By using the sum of the sizes of all
> touches as a basis for this calculation we get more coherent values.
> ---
>  drivers/input/mouse/bcm5974.c | 21 +++++++--------------
>  1 file changed, 7 insertions(+), 14 deletions(-)

The input-mt core can be used to output the correct pointer-emulated pressure
based on the individual pressures. Devices capable of measuring pressure should
rather use that method. However, this particular device does not actually
support pressure, and is therefore not applicable.

The legacy pressure code was added a long time ago in order to work with the
only userland option at the time (synaptics X driver). Since then, userland has
matured considerably, with drivers such as input-multitouch, input-evdev,
input-mtrack at your disposal. The only reason the pointer pressure and width
are still in the driver is to remain compatible with old userland.

Thanks,
Henrik

Re: [PATCH 3/3] Input: Added thumb detection in BCM5974 multitouch driver

[Henrik Rydberg]

> On Wed, Nov 20, 2013 at 11:54:12PM +0100, Friedrich Schöller wrote:
>> Trackpads with integrated buttons are hard to use when the driver responds to
>> movements of the thumb that is resting or clicking on the surface of the
>> trackpad. This patch adds rudimentary support to filter out these touch events.
>>
>> The feature can be turned on via sysfs:
>> 	/sys/class/input/input[0-9]+/thumb_ignore:
>> 		Enables thumb detection
>> 		Values: 0/1
>> 	/sys/class/input/input[0-9]+/thumb_ratio_on:
>> 		When the ratio of ABS_MT_TOUCH_MINOR / ABS_MT_TOUCH_MAJOR
>> 		times 100 is smaller than this value the touch qualifies
>> 		as a thumb.
>> 	/sys/class/input/input[0-9]+/thumb_ratio_off:
>> 		When the ratio of ABS_MT_TOUCH_MINOR / ABS_MT_TOUCH_MAJOR
>> 		times 100 is bigger than this value the touch no longer
>> 		qualifies as a thumb.
>> 	/sys/class/input/input[0-9]+/thumb_y_on:
>> 		When ABS_MT_POSITION_Y is bigger than this value the touch
>> 		qualifies as a thumb.
>> 	/sys/class/input/input[0-9]+/thumb_y_off:
>> 		When ABS_MT_POSITION_Y is smaller than this value the touch
>> 		no longer qualifies as a thumb.
> 
> I'd rather this been implemented in userspace (synaptics and/or evdev X
> drivers).

And it has been in userland since at least 2008. It can be found in
input-synaptics, input-multitouch, input-mtrack, possibly more places.

Thanks,
Henrik

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] Input: Fixed ABS_MT_TOUCH_MINOR scale factor in BCM5974 multitouch driver

[Friedrich Schöller]

Hello Henrik,

>> On wellspring3 devices ABS_MT_TOUCH_MINOR was sometimes reported bigger than
>> ABS_MT_TOUCH_MAJOR. This is fixed by rescaling ABS_MT_TOUCH_MINOR by a factor of
>> 0.85 instead of 2. Excessive tapping on the trackpad shows this to be the right
>> value. Circular touches should now lead to values for ABS_MT_TOUCH_MAJOR and
>> ABS_MT_TOUCH_MINOR that are similar, with ABS_MT_TOUCH_MINOR never greater than
>> ABS_MT_TOUCH_MAJOR.
>> ---
>>  drivers/input/mouse/bcm5974.c | 20 +++++++++++++++++---
>>  1 file changed, 17 insertions(+), 3 deletions(-)
>
> The major/minor scales are following the aspect ratio of the device, and as such
> it could happen that minor > major. Most userland drivers do not use the finger
> width limits, which are estimates, but only the device axes limit, which are
> accurate.

I know you wrote it, but this seems to me to contradict the
documentation on the multitouch protocol:
"In addition to the MAJOR parameters, the oval shape of the touch and finger
regions can be described by adding the MINOR parameters, such that MAJOR
and MINOR are the major and minor axis of an ellipse."

Why do the limits on the device axes, which are oriented horizontally
and vertically, have any influence on the major and minor parameters
which can be oriented arbitrarily?

Or let me phrase it more pragmatically: How can a userland application
figure out the factor of 0.425 (from 0.85 / 2) by which it has to
multiply ABS_MT_TOUCH_MINOR?

As a side note: The WIDTH_MAJOR and WIDTH_MINOR parameters are already
well behaved in the sense that WIDTH_MINOR is always smaller.

> Also, we cannot have floats in the kernel.

Okay a fraction would do the trick.

Thanks,
Friedrich