[PATCH 0/5] input: wacom: Initial support for Bamboo (rev3)

[PATCH 0/5] input: wacom: Initial support for Bamboo (rev3)

[Henrik Rydberg]

This the fourth iteration of the Bamboo patches. Hopefully no big
surprises.  The ordering has changed to allow setting the pressure_max
which is not parsed in the hid function. Ping's button mapping has
been implemented, relying on middle-button emulation instead.  The
lowres quirks now apply to all touch devices in the family.

Cheers,
Henrik

Henrik Rydberg (4):
  input: wacom: Add fuzz parameters to features
  input: wacom: Collect device quirks into single function (rev2)
  input: wacom: Add support for the Bamboo Touch trackpad (rev4)
  input: wacom: Add a quirk for lowres Bamboo devices (rev2)

Ping Cheng (1):
  input: wacom: Parse the Bamboo device family

 drivers/input/tablet/wacom.h     |    1 +
 drivers/input/tablet/wacom_sys.c |   57 +++++++++++++-----
 drivers/input/tablet/wacom_wac.c |  123 +++++++++++++++++++++++++++++++++++++-
 drivers/input/tablet/wacom_wac.h |   14 ++++
 4 files changed, 176 insertions(+), 19 deletions(-)

[PATCH 1/5] input: wacom: Add fuzz parameters to features

[Henrik Rydberg]

The signal-to-noise ratio varies between devices, but currently all
devices are treated the same way. Add fuzz parameters to the feature
struct, allowing for tailored treatment of devices.

Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
 drivers/input/tablet/wacom_sys.c |    6 +++++-
 drivers/input/tablet/wacom_wac.c |    9 ++++++---
 drivers/input/tablet/wacom_wac.h |    4 ++++
 3 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/input/tablet/wacom_sys.c b/drivers/input/tablet/wacom_sys.c
index 42ba369..e510e4f 100644
--- a/drivers/input/tablet/wacom_sys.c
+++ b/drivers/input/tablet/wacom_sys.c
@@ -333,8 +333,12 @@ static int wacom_retrieve_hid_descriptor(struct usb_interface *intf,
 	struct usb_host_interface *interface = intf->cur_altsetting;
 	struct hid_descriptor *hid_desc;
 
-	/* default device to penabled */
+	/* default features */
 	features->device_type = BTN_TOOL_PEN;
+	features->x_fuzz = 4;
+	features->y_fuzz = 4;
+	features->pressure_fuzz = 0;
+	features->distance_fuzz = 0;
 
 	/* only Tablet PCs need to retrieve the info */
 	if ((features->type != TABLETPC) && (features->type != TABLETPC2FG))
diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
index 40d77ba..bfe5654 100644
--- a/drivers/input/tablet/wacom_wac.c
+++ b/drivers/input/tablet/wacom_wac.c
@@ -951,9 +951,12 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev,
 
 	__set_bit(BTN_TOUCH, input_dev->keybit);
 
-	input_set_abs_params(input_dev, ABS_X, 0, features->x_max, 4, 0);
-	input_set_abs_params(input_dev, ABS_Y, 0, features->y_max, 4, 0);
-	input_set_abs_params(input_dev, ABS_PRESSURE, 0, features->pressure_max, 0, 0);
+	input_set_abs_params(input_dev, ABS_X, 0, features->x_max,
+			     features->x_fuzz, 0);
+	input_set_abs_params(input_dev, ABS_Y, 0, features->y_max,
+			     features->y_fuzz, 0);
+	input_set_abs_params(input_dev, ABS_PRESSURE, 0, features->pressure_max,
+			     features->pressure_fuzz, 0);
 
 	__set_bit(ABS_MISC, input_dev->absbit);
 
diff --git a/drivers/input/tablet/wacom_wac.h b/drivers/input/tablet/wacom_wac.h
index 99e1a54..d769e9a 100644
--- a/drivers/input/tablet/wacom_wac.h
+++ b/drivers/input/tablet/wacom_wac.h
@@ -73,6 +73,10 @@ struct wacom_features {
 	int y_phy;
 	unsigned char unit;
 	unsigned char unitExpo;
+	int x_fuzz;
+	int y_fuzz;
+	int pressure_fuzz;
+	int distance_fuzz;
 };
 
 struct wacom_shared {
-- 
1.7.1

[PATCH 2/5] input: wacom: Parse the Bamboo device family

[Henrik Rydberg]

From: Ping Cheng <pinglinux@gmail.com>

The Bamboo devices have multiple interfaces which need to be setup
separately. Use the HID parsing mechanism to achieve that.

Signed-off-by: Ping Cheng <pinglinux@gmail.com>
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
 drivers/input/tablet/wacom_sys.c |   44 ++++++++++++++++++++++++++++++-------
 drivers/input/tablet/wacom_wac.h |    2 +
 2 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/drivers/input/tablet/wacom_sys.c b/drivers/input/tablet/wacom_sys.c
index e510e4f..98cba08 100644
--- a/drivers/input/tablet/wacom_sys.c
+++ b/drivers/input/tablet/wacom_sys.c
@@ -195,17 +195,30 @@ static int wacom_parse_hid(struct usb_interface *intf, struct hid_descriptor *hi
 							features->pktlen = WACOM_PKGLEN_TPC2FG;
 							features->device_type = BTN_TOOL_TRIPLETAP;
 						}
-						features->x_max =
-							get_unaligned_le16(&report[i + 3]);
-						features->x_phy =
-							get_unaligned_le16(&report[i + 6]);
-						features->unit = report[i + 9];
-						features->unitExpo = report[i + 11];
-						i += 12;
+						if (features->type == BAMBOO_PT) {
+							/* need to reset back */
+							features->pktlen = WACOM_PKGLEN_BBTOUCH;
+							features->device_type = BTN_TOOL_TRIPLETAP;
+							features->x_phy =
+								get_unaligned_le16(&report[i + 5]);
+							features->x_max =
+								get_unaligned_le16(&report[i + 8]);
+							i += 15;
+						} else {
+							features->x_max =
+								get_unaligned_le16(&report[i + 3]);
+							features->x_phy =
+								get_unaligned_le16(&report[i + 6]);
+							features->unit = report[i + 9];
+							features->unitExpo = report[i + 11];
+							i += 12;
+						}
 					} else if (pen) {
 						/* penabled only accepts exact bytes of data */
 						if (features->type == TABLETPC2FG)
 							features->pktlen = WACOM_PKGLEN_GRAPHIRE;
+						if (features->type == BAMBOO_PT)
+							features->pktlen = WACOM_PKGLEN_BBFUN;
 						features->device_type = BTN_TOOL_PEN;
 						features->x_max =
 							get_unaligned_le16(&report[i + 3]);
@@ -234,6 +247,15 @@ static int wacom_parse_hid(struct usb_interface *intf, struct hid_descriptor *hi
 							features->y_phy =
 								get_unaligned_le16(&report[i + 6]);
 							i += 7;
+						} else if (features->type == BAMBOO_PT) {
+							/* need to reset back */
+							features->pktlen = WACOM_PKGLEN_BBTOUCH;
+							features->device_type = BTN_TOOL_TRIPLETAP;
+							features->y_phy =
+								get_unaligned_le16(&report[i + 3]);
+							features->y_max =
+								get_unaligned_le16(&report[i + 6]);
+							i += 12;
 						} else {
 							features->y_max =
 								features->x_max;
@@ -245,6 +267,8 @@ static int wacom_parse_hid(struct usb_interface *intf, struct hid_descriptor *hi
 						/* penabled only accepts exact bytes of data */
 						if (features->type == TABLETPC2FG)
 							features->pktlen = WACOM_PKGLEN_GRAPHIRE;
+						if (features->type == BAMBOO_PT)
+							features->pktlen = WACOM_PKGLEN_BBFUN;
 						features->device_type = BTN_TOOL_PEN;
 						features->y_max =
 							get_unaligned_le16(&report[i + 3]);
@@ -341,7 +365,8 @@ static int wacom_retrieve_hid_descriptor(struct usb_interface *intf,
 	features->distance_fuzz = 0;
 
 	/* only Tablet PCs need to retrieve the info */
-	if ((features->type != TABLETPC) && (features->type != TABLETPC2FG))
+	if ((features->type != TABLETPC) && (features->type != TABLETPC2FG) &&
+	    (features->type != BAMBOO_PT))
 		goto out;
 
 	if (usb_get_extra_descriptor(interface, HID_DEVICET_HID, &hid_desc)) {
@@ -499,7 +524,8 @@ static int wacom_probe(struct usb_interface *intf, const struct usb_device_id *i
 
 	strlcpy(wacom_wac->name, features->name, sizeof(wacom_wac->name));
 
-	if (features->type == TABLETPC || features->type == TABLETPC2FG) {
+	if (features->type == TABLETPC || features->type == TABLETPC2FG ||
+	    features->type == BAMBOO_PT) {
 		/* Append the device type to the name */
 		strlcat(wacom_wac->name,
 			features->device_type == BTN_TOOL_PEN ?
diff --git a/drivers/input/tablet/wacom_wac.h b/drivers/input/tablet/wacom_wac.h
index d769e9a..fb30895 100644
--- a/drivers/input/tablet/wacom_wac.h
+++ b/drivers/input/tablet/wacom_wac.h
@@ -21,6 +21,7 @@
 #define WACOM_PKGLEN_INTUOS	10
 #define WACOM_PKGLEN_TPC1FG	 5
 #define WACOM_PKGLEN_TPC2FG	14
+#define WACOM_PKGLEN_BBTOUCH	20
 
 /* device IDs */
 #define STYLUS_DEVICE_ID	0x02
@@ -44,6 +45,7 @@ enum {
 	PTU,
 	PL,
 	DTU,
+	BAMBOO_PT,
 	INTUOS,
 	INTUOS3S,
 	INTUOS3,
-- 
1.7.1

[PATCH 3/5] input: wacom: Collect device quirks into single function (rev2)

[Henrik Rydberg]

Collect device-specific code into a single function, and use quirks to
flag specific behavior instead.

Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
 drivers/input/tablet/wacom.h     |    1 +
 drivers/input/tablet/wacom_sys.c |   11 +++--------
 drivers/input/tablet/wacom_wac.c |   16 ++++++++++++++++
 drivers/input/tablet/wacom_wac.h |    4 ++++
 4 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/drivers/input/tablet/wacom.h b/drivers/input/tablet/wacom.h
index 284dfaa..de5adb1 100644
--- a/drivers/input/tablet/wacom.h
+++ b/drivers/input/tablet/wacom.h
@@ -118,6 +118,7 @@ struct wacom {
 extern const struct usb_device_id wacom_ids[];
 
 void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len);
+void wacom_setup_device_quirks(struct wacom_features *features);
 void wacom_setup_input_capabilities(struct input_dev *input_dev,
 				    struct wacom_wac *wacom_wac);
 #endif
diff --git a/drivers/input/tablet/wacom_sys.c b/drivers/input/tablet/wacom_sys.c
index 98cba08..fc6fd53 100644
--- a/drivers/input/tablet/wacom_sys.c
+++ b/drivers/input/tablet/wacom_sys.c
@@ -381,12 +381,6 @@ static int wacom_retrieve_hid_descriptor(struct usb_interface *intf,
 	if (error)
 		goto out;
 
-	/* touch device found but size is not defined. use default */
-	if (features->device_type == BTN_TOOL_DOUBLETAP && !features->x_max) {
-		features->x_max = 1023;
-		features->y_max = 1023;
-	}
-
  out:
 	return error;
 }
@@ -522,10 +516,11 @@ static int wacom_probe(struct usb_interface *intf, const struct usb_device_id *i
 	if (error)
 		goto fail2;
 
+	wacom_setup_device_quirks(features);
+
 	strlcpy(wacom_wac->name, features->name, sizeof(wacom_wac->name));
 
-	if (features->type == TABLETPC || features->type == TABLETPC2FG ||
-	    features->type == BAMBOO_PT) {
+	if (features->quirks & WACOM_QUIRK_MULTI_INPUT) {
 		/* Append the device type to the name */
 		strlcat(wacom_wac->name,
 			features->device_type == BTN_TOOL_PEN ?
diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
index bfe5654..ca18b4d 100644
--- a/drivers/input/tablet/wacom_wac.c
+++ b/drivers/input/tablet/wacom_wac.c
@@ -941,6 +941,22 @@ static void wacom_setup_intuos(struct wacom_wac *wacom_wac)
 	input_set_abs_params(input_dev, ABS_THROTTLE, -1023, 1023, 0, 0);
 }
 
+
+void wacom_setup_device_quirks(struct wacom_features *features)
+{
+
+	/* touch device found but size is not defined. use default */
+	if (features->device_type == BTN_TOOL_DOUBLETAP && !features->x_max) {
+		features->x_max = 1023;
+		features->y_max = 1023;
+	}
+
+	/* these device have multiple inputs */
+	if (features->type == TABLETPC || features->type == TABLETPC2FG ||
+	    features->type == BAMBOO_PT)
+		features->quirks |= WACOM_QUIRK_MULTI_INPUT;
+}
+
 void wacom_setup_input_capabilities(struct input_dev *input_dev,
 				    struct wacom_wac *wacom_wac)
 {
diff --git a/drivers/input/tablet/wacom_wac.h b/drivers/input/tablet/wacom_wac.h
index fb30895..6a1ff10 100644
--- a/drivers/input/tablet/wacom_wac.h
+++ b/drivers/input/tablet/wacom_wac.h
@@ -38,6 +38,9 @@
 #define WACOM_REPORT_TPC1FG		6
 #define WACOM_REPORT_TPC2FG		13
 
+/* device quirks */
+#define WACOM_QUIRK_MULTI_INPUT		0x0001
+
 enum {
 	PENPARTNER = 0,
 	GRAPHIRE,
@@ -79,6 +82,7 @@ struct wacom_features {
 	int y_fuzz;
 	int pressure_fuzz;
 	int distance_fuzz;
+	unsigned quirks;
 };
 
 struct wacom_shared {
-- 
1.7.1

[PATCH 4/5] input: wacom: Add support for the Bamboo Touch trackpad (rev4)

[Henrik Rydberg]

Add support for the Bamboo Touch trackpad, and make it work with
both the Synaptics X Driver and the Multitouch X Driver. The device
uses MT slots internally, so the choice of protocol is a given.

Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
 drivers/input/tablet/wacom_wac.c |   88 ++++++++++++++++++++++++++++++++++++++
 drivers/input/tablet/wacom_wac.h |    3 +
 2 files changed, 91 insertions(+), 0 deletions(-)

diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
index ca18b4d..97295aa 100644
--- a/drivers/input/tablet/wacom_wac.c
+++ b/drivers/input/tablet/wacom_wac.c
@@ -855,6 +855,54 @@ static int wacom_tpc_irq(struct wacom_wac *wacom, size_t len)
 	return retval;
 }
 
+static int wacom_bpt_irq(struct wacom_wac *wacom, size_t len)
+{
+	static int trkid;
+	struct input_dev *input = wacom->input;
+	unsigned char *data = wacom->data;
+	int sp = 0, sx = 0, sy = 0, count = 0;
+	int i;
+
+	if (len != WACOM_PKGLEN_BBTOUCH)
+		return 0;
+
+	for (i = 0; i < 2; i++) {
+		int p = data[9 * i + 2];
+		input_mt_slot(input, i);
+		if (p) {
+			int x = get_unaligned_be16(&data[9 * i + 3]) & 0x7ff;
+			int y = get_unaligned_be16(&data[9 * i + 5]) & 0x7ff;
+			input_report_abs(input, ABS_MT_PRESSURE, p);
+			input_report_abs(input, ABS_MT_POSITION_X, x);
+			input_report_abs(input, ABS_MT_POSITION_Y, y);
+			if (wacom->id[i] < 0)
+				wacom->id[i] = trkid++ & MAX_TRACKING_ID;
+			if (!count++)
+				sp = p, sx = x, sy = y;
+		} else {
+			wacom->id[i] = -1;
+		}
+		input_report_abs(input, ABS_MT_TRACKING_ID, wacom->id[i]);
+	}
+
+	input_report_key(input, BTN_TOUCH, count > 0);
+	input_report_key(input, BTN_TOOL_FINGER, count == 1);
+	input_report_key(input, BTN_TOOL_DOUBLETAP, count == 2);
+
+	input_report_abs(input, ABS_PRESSURE, sp);
+	input_report_abs(input, ABS_X, sx);
+	input_report_abs(input, ABS_Y, sy);
+
+	input_report_key(input, BTN_LEFT, (data[1] & 0x08) != 0);
+	input_report_key(input, BTN_FORWARD, (data[1] & 0x04) != 0);
+	input_report_key(input, BTN_BACK, (data[1] & 0x02) != 0);
+	input_report_key(input, BTN_RIGHT, (data[1] & 0x01) != 0);
+
+	input_sync(input);
+
+	return 0;
+}
+
 void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len)
 {
 	bool sync;
@@ -900,6 +948,10 @@ void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len)
 		sync = wacom_tpc_irq(wacom_wac, len);
 		break;
 
+	case BAMBOO_PT:
+		sync = wacom_bpt_irq(wacom_wac, len);
+		break;
+
 	default:
 		sync = false;
 		break;
@@ -955,6 +1007,12 @@ void wacom_setup_device_quirks(struct wacom_features *features)
 	if (features->type == TABLETPC || features->type == TABLETPC2FG ||
 	    features->type == BAMBOO_PT)
 		features->quirks |= WACOM_QUIRK_MULTI_INPUT;
+
+	/* quirks for bamboo touch */
+	if (features->type == BAMBOO_PT) {
+		features->pressure_max = 256;
+		features->pressure_fuzz = 16;
+	}
 }
 
 void wacom_setup_input_capabilities(struct input_dev *input_dev,
@@ -1095,6 +1153,33 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev,
 	case PENPARTNER:
 		__set_bit(BTN_TOOL_RUBBER, input_dev->keybit);
 		break;
+
+	case BAMBOO_PT:
+		__clear_bit(ABS_MISC, input_dev->absbit);
+
+		if (features->device_type == BTN_TOOL_TRIPLETAP) {
+			__set_bit(BTN_LEFT, input_dev->keybit);
+			__set_bit(BTN_FORWARD, input_dev->keybit);
+			__set_bit(BTN_BACK, input_dev->keybit);
+			__set_bit(BTN_RIGHT, input_dev->keybit);
+
+			__set_bit(BTN_TOOL_FINGER, input_dev->keybit);
+			__set_bit(BTN_TOOL_DOUBLETAP, input_dev->keybit);
+
+			input_mt_create_slots(input_dev, 2);
+			input_set_abs_params(input_dev, ABS_MT_POSITION_X,
+					     0, features->x_max,
+					     features->x_fuzz, 0);
+			input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
+					     0, features->y_max,
+					     features->y_fuzz, 0);
+			input_set_abs_params(input_dev, ABS_MT_PRESSURE,
+					     0, features->pressure_max,
+					     features->pressure_fuzz, 0);
+			input_set_abs_params(input_dev, ABS_MT_TRACKING_ID, 0,
+					     MAX_TRACKING_ID, 0, 0);
+		}
+		break;
 	}
 }
 
@@ -1232,6 +1317,8 @@ static const struct wacom_features wacom_features_0xE3 =
 	{ "Wacom ISDv4 E3",       WACOM_PKGLEN_TPC2FG,    26202, 16325,  255,  0, TABLETPC2FG };
 static const struct wacom_features wacom_features_0x47 =
 	{ "Wacom Intuos2 6x8",    WACOM_PKGLEN_INTUOS,    20320, 16240, 1023, 31, INTUOS };
+static struct wacom_features wacom_features_0xD0 =
+	{ "Wacom Bamboo 2FG",     WACOM_PKGLEN_BBFUN,     14720,  9200, 1023, 63, BAMBOO_PT };
 
 #define USB_DEVICE_WACOM(prod)					\
 	USB_DEVICE(USB_VENDOR_ID_WACOM, prod),			\
@@ -1296,6 +1383,7 @@ const struct usb_device_id wacom_ids[] = {
 	{ USB_DEVICE_WACOM(0xC6) },
 	{ USB_DEVICE_WACOM(0xC7) },
 	{ USB_DEVICE_WACOM(0xCE) },
+	{ USB_DEVICE_WACOM(0xD0) },
 	{ USB_DEVICE_WACOM(0xF0) },
 	{ USB_DEVICE_WACOM(0xCC) },
 	{ USB_DEVICE_WACOM(0x90) },
diff --git a/drivers/input/tablet/wacom_wac.h b/drivers/input/tablet/wacom_wac.h
index 6a1ff10..816ab0f 100644
--- a/drivers/input/tablet/wacom_wac.h
+++ b/drivers/input/tablet/wacom_wac.h
@@ -41,6 +41,9 @@
 /* device quirks */
 #define WACOM_QUIRK_MULTI_INPUT		0x0001
 
+/* largest reported tracking id */
+#define MAX_TRACKING_ID			0xfff
+
 enum {
 	PENPARTNER = 0,
 	GRAPHIRE,
-- 
1.7.1

[PATCH 5/5] input: wacom: Add a quirk for lowres Bamboo devices (rev2)

[Henrik Rydberg]

The Bamboo Touch reports a sub-screen resolution of 480x320.
The signal-to-noise ratio is only about 100, so filtering is
needed in order to reduce the jitter to a usable level. However,
the low resolution leads to round-off errors in the EWMA filter,
resulting in extremely jerky pointer motion. This patch explicitly
sets a higher resolution for those devices, and tells this to
the completion handler via a low-resolution quirk.

Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
 drivers/input/tablet/wacom_wac.c |   10 ++++++++++
 drivers/input/tablet/wacom_wac.h |    1 +
 2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
index 97295aa..8193892 100644
--- a/drivers/input/tablet/wacom_wac.c
+++ b/drivers/input/tablet/wacom_wac.c
@@ -858,6 +858,7 @@ static int wacom_tpc_irq(struct wacom_wac *wacom, size_t len)
 static int wacom_bpt_irq(struct wacom_wac *wacom, size_t len)
 {
 	static int trkid;
+	struct wacom_features *features = &wacom->features;
 	struct input_dev *input = wacom->input;
 	unsigned char *data = wacom->data;
 	int sp = 0, sx = 0, sy = 0, count = 0;
@@ -872,6 +873,10 @@ static int wacom_bpt_irq(struct wacom_wac *wacom, size_t len)
 		if (p) {
 			int x = get_unaligned_be16(&data[9 * i + 3]) & 0x7ff;
 			int y = get_unaligned_be16(&data[9 * i + 5]) & 0x7ff;
+			if (features->quirks & WACOM_QUIRK_BBTOUCH_LOWRES) {
+				x <<= 5;
+				y <<= 5;
+			}
 			input_report_abs(input, ABS_MT_PRESSURE, p);
 			input_report_abs(input, ABS_MT_POSITION_X, x);
 			input_report_abs(input, ABS_MT_POSITION_Y, y);
@@ -1010,8 +1015,13 @@ void wacom_setup_device_quirks(struct wacom_features *features)
 
 	/* quirks for bamboo touch */
 	if (features->type == BAMBOO_PT) {
+		features->x_max <<= 5;
+		features->y_max <<= 5;
+		features->x_fuzz <<= 5;
+		features->y_fuzz <<= 5;
 		features->pressure_max = 256;
 		features->pressure_fuzz = 16;
+		features->quirks |= WACOM_QUIRK_BBTOUCH_LOWRES;
 	}
 }
 
diff --git a/drivers/input/tablet/wacom_wac.h b/drivers/input/tablet/wacom_wac.h
index 816ab0f..22cafe2 100644
--- a/drivers/input/tablet/wacom_wac.h
+++ b/drivers/input/tablet/wacom_wac.h
@@ -40,6 +40,7 @@
 
 /* device quirks */
 #define WACOM_QUIRK_MULTI_INPUT		0x0001
+#define WACOM_QUIRK_BBTOUCH_LOWRES	0x0002
 
 /* largest reported tracking id */
 #define MAX_TRACKING_ID			0xfff
-- 
1.7.1

Re: [PATCH 0/5] input: wacom: Initial support for Bamboo (rev3)

[Ping Cheng]

On Saturday, September 4, 2010, Henrik Rydberg <rydberg@euromail.se> wrote:
> This the fourth iteration of the Bamboo patches. Hopefully no big
> surprises.  The ordering has changed to allow setting the pressure_max
> which is not parsed in the hid function. Ping's button mapping has
> been implemented, relying on middle-button emulation instead.  The
> lowres quirks now apply to all touch devices in the family.

Thank you Henrik for the patchset. Very much appreciated.

Acked-by: Ping Cheng < pingc@wacom.com>  for the series.

Ping

> Henrik Rydberg (4):
>   input: wacom: Add fuzz parameters to features
>   input: wacom: Collect device quirks into single function (rev2)
>   input: wacom: Add support for the Bamboo Touch trackpad (rev4)
>   input: wacom: Add a quirk for lowres Bamboo devices (rev2)
>
> Ping Cheng (1):
>   input: wacom: Parse the Bamboo device family
>
>  drivers/input/tablet/wacom.h     |    1 +
>  drivers/input/tablet/wacom_sys.c |   57 +++++++++++++-----
>  drivers/input/tablet/wacom_wac.c |  123 +++++++++++++++++++++++++++++++++++++-
>  drivers/input/tablet/wacom_wac.h |   14 ++++
>  4 files changed, 176 insertions(+), 19 deletions(-)
>
> --
> 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 4/5] input: wacom: Add support for the Bamboo Touch trackpad (rev4)

[Henrik Rydberg]

On 09/04/2010 03:43 PM, Henrik Rydberg wrote:

[...]

>@@ -955,6 +1007,12 @@ void wacom_setup_device_quirks(struct wacom
> 	if (features->type == TABLETPC || features->type == TABLETPC2FG ||
> 	    features->type == BAMBOO_PT)
> 		features->quirks |= WACOM_QUIRK_MULTI_INPUT;
>+
>+	/* quirks for bamboo touch */
>+	if (features->type == BAMBOO_PT) {

This line should have "&& features->device_type == BTN_TOOL_TRIPLETAP" appended
to it, to only target the touch device of Pen & Touch.

>+		features->pressure_max = 256;
>+		features->pressure_fuzz = 16;
>+	}
> }

Henrik

Re: [PATCH 0/5] input: wacom: Initial support for Bamboo (rev3)

[Chris Bagwell]

On Sat, Sep 4, 2010 at 4:24 PM, Ping Cheng <pinglinux@gmail.com> wrote:
> On Saturday, September 4, 2010, Henrik Rydberg <rydberg@euromail.se> wrote:
>> This the fourth iteration of the Bamboo patches. Hopefully no big
>> surprises.  The ordering has changed to allow setting the pressure_max
>> which is not parsed in the hid function. Ping's button mapping has
>> been implemented, relying on middle-button emulation instead.  The
>> lowres quirks now apply to all touch devices in the family.
>
> Thank you Henrik for the patchset. Very much appreciated.
>
> Acked-by: Ping Cheng < pingc@wacom.com>  for the series.
>
> Ping

Thanks for addressing my comments/questions.

Also, Thanks for placing all the hooks for future pen support as well.
 I'll be following up with a patch set for that once your finished.
If there are any remaining pen specific issues, feel free to offload
it on me.

Chris

>
>> Henrik Rydberg (4):
>>   input: wacom: Add fuzz parameters to features
>>   input: wacom: Collect device quirks into single function (rev2)
>>   input: wacom: Add support for the Bamboo Touch trackpad (rev4)
>>   input: wacom: Add a quirk for lowres Bamboo devices (rev2)
>>
>> Ping Cheng (1):
>>   input: wacom: Parse the Bamboo device family
>>
>>  drivers/input/tablet/wacom.h     |    1 +
>>  drivers/input/tablet/wacom_sys.c |   57 +++++++++++++-----
>>  drivers/input/tablet/wacom_wac.c |  123 +++++++++++++++++++++++++++++++++++++-
>>  drivers/input/tablet/wacom_wac.h |   14 ++++
>>  4 files changed, 176 insertions(+), 19 deletions(-)
>>
>> --
>> 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 4/5] input: wacom: Add support for the Bamboo Touch trackpad (rev4)

[Dmitry Torokhov]

On Sun, Sep 05, 2010 at 12:04:00PM +0200, Henrik Rydberg wrote:
> On 09/04/2010 03:43 PM, Henrik Rydberg wrote:
> 
> [...]
> 
> >@@ -955,6 +1007,12 @@ void wacom_setup_device_quirks(struct wacom
> > 	if (features->type == TABLETPC || features->type == TABLETPC2FG ||
> > 	    features->type == BAMBOO_PT)
> > 		features->quirks |= WACOM_QUIRK_MULTI_INPUT;
> >+
> >+	/* quirks for bamboo touch */
> >+	if (features->type == BAMBOO_PT) {
> 
> This line should have "&& features->device_type == BTN_TOOL_TRIPLETAP" appended
> to it, to only target the touch device of Pen & Touch.
> 

Adjusted and applied all 5 patches. I also pulled trkid into wacom_wac
structure to make sure different instances of the device are independent
of each other.

Thanks.

-- 
Dmitry

Re: [PATCH 4/5] input: wacom: Add support for the Bamboo Touch trackpad (rev4)

[Ping Cheng]

Hi Henrik,

I know this patchset is under Linus tree and I've acked it already.
But, I am trying to catch up with the fast moving MT train and to make
sure the train is moving to my destination :).

More comments in line.

Ping

On Sat, Sep 4, 2010 at 6:43 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
> Add support for the Bamboo Touch trackpad, and make it work with
> both the Synaptics X Driver and the Multitouch X Driver. The device
> uses MT slots internally, so the choice of protocol is a given.
>
> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
> ---
>  drivers/input/tablet/wacom_wac.c |   88 ++++++++++++++++++++++++++++++++++++++
>  drivers/input/tablet/wacom_wac.h |    3 +
>  2 files changed, 91 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
> index ca18b4d..97295aa 100644
> --- a/drivers/input/tablet/wacom_wac.c
> +++ b/drivers/input/tablet/wacom_wac.c
> @@ -855,6 +855,54 @@ static int wacom_tpc_irq(struct wacom_wac *wacom, size_t len)
>        return retval;
>  }
>
> +static int wacom_bpt_irq(struct wacom_wac *wacom, size_t len)
> +{
> +       static int trkid;
> +       struct input_dev *input = wacom->input;
> +       unsigned char *data = wacom->data;
> +       int sp = 0, sx = 0, sy = 0, count = 0;
> +       int i;
> +
> +       if (len != WACOM_PKGLEN_BBTOUCH)
> +               return 0;
> +
> +       for (i = 0; i < 2; i++) {
> +               int p = data[9 * i + 2];
> +               input_mt_slot(input, i);
> +               if (p) {
> +                       int x = get_unaligned_be16(&data[9 * i + 3]) & 0x7ff;
> +                       int y = get_unaligned_be16(&data[9 * i + 5]) & 0x7ff;
> +                       input_report_abs(input, ABS_MT_PRESSURE, p);
> +                       input_report_abs(input, ABS_MT_POSITION_X, x);
> +                       input_report_abs(input, ABS_MT_POSITION_Y, y);
> +                       if (wacom->id[i] < 0)
> +                               wacom->id[i] = trkid++ & MAX_TRACKING_ID;

Why do we need an arbitrary MAX_TRACKING_ID when the device can tell
us how many IDs we can have and it tracks the individual fingers for
us? In this case, there are only two ID/fingers and the ID can be
retrieved from the raw data. I must be missing something in the
defintion of MAX_TRACKING_ID.

> +                       if (!count++)
> +                               sp = p, sx = x, sy = y;
> +               } else {
> +                       wacom->id[i] = -1;
> +               }
> +               input_report_abs(input, ABS_MT_TRACKING_ID, wacom->id[i]);
> +       }
> +
> +       input_report_key(input, BTN_TOUCH, count > 0);
> +       input_report_key(input, BTN_TOOL_FINGER, count == 1);
> +       input_report_key(input, BTN_TOOL_DOUBLETAP, count == 2);
> +
> +       input_report_abs(input, ABS_PRESSURE, sp);
> +       input_report_abs(input, ABS_X, sx);
> +       input_report_abs(input, ABS_Y, sy);
> +
> +       input_report_key(input, BTN_LEFT, (data[1] & 0x08) != 0);
> +       input_report_key(input, BTN_FORWARD, (data[1] & 0x04) != 0);
> +       input_report_key(input, BTN_BACK, (data[1] & 0x02) != 0);
> +       input_report_key(input, BTN_RIGHT, (data[1] & 0x01) != 0);

This implementation impairs the value of those buttons since I know a
lot of users want them configurable. If we can not or do not pass a
generic set to the userland, we will need to make it configurable in
the kernel driver as Dmitry suggested (if I remember it correctly).
Which approach, in the userland or the kernel, do you like, 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 4/5] input: wacom: Add support for the Bamboo Touch trackpad (rev4)

[Henrik Rydberg]

Hi Ping,

> I know this patchset is under Linus tree and I've acked it already.

> But, I am trying to catch up with the fast moving MT train and to make
> sure the train is moving to my destination :).


By all means, where ever Ping Station might be located. :-)

[...]

>> +static int wacom_bpt_irq(struct wacom_wac *wacom, size_t len)

>> +{
>> +       static int trkid;
>> +       struct input_dev *input = wacom->input;
>> +       unsigned char *data = wacom->data;
>> +       int sp = 0, sx = 0, sy = 0, count = 0;
>> +       int i;
>> +
>> +       if (len != WACOM_PKGLEN_BBTOUCH)
>> +               return 0;
>> +
>> +       for (i = 0; i < 2; i++) {
>> +               int p = data[9 * i + 2];
>> +               input_mt_slot(input, i);
>> +               if (p) {
>> +                       int x = get_unaligned_be16(&data[9 * i + 3]) & 0x7ff;
>> +                       int y = get_unaligned_be16(&data[9 * i + 5]) & 0x7ff;
>> +                       input_report_abs(input, ABS_MT_PRESSURE, p);
>> +                       input_report_abs(input, ABS_MT_POSITION_X, x);
>> +                       input_report_abs(input, ABS_MT_POSITION_Y, y);
>> +                       if (wacom->id[i] < 0)
>> +                               wacom->id[i] = trkid++ & MAX_TRACKING_ID;
> 
> Why do we need an arbitrary MAX_TRACKING_ID when the device can tell
> us how many IDs we can have and it tracks the individual fingers for
> us? In this case, there are only two ID/fingers and the ID can be
> retrieved from the raw data. I must be missing something in the
> defintion of MAX_TRACKING_ID.


In the language of ABS_MT, there is a distinction between slot and id. The slot
is the handle used to communicate a unique touch. The id is the identifier of
that touch. The device tells us how many slots we have. The range of ids is in
principle infinite. In practice, it is set to a large number.

To answer a possible follow-up question: from the tracking id, you can tell if a
contact is present, if it is new, and if it is older than another contact.

[...]

>> +
>> +       input_report_key(input, BTN_LEFT, (data[1] & 0x08) != 0);
>> +       input_report_key(input, BTN_FORWARD, (data[1] & 0x04) != 0);
>> +       input_report_key(input, BTN_BACK, (data[1] & 0x02) != 0);
>> +       input_report_key(input, BTN_RIGHT, (data[1] & 0x01) != 0);
> 
> This implementation impairs the value of those buttons since I know a
> lot of users want them configurable. If we can not or do not pass a
> generic set to the userland, we will need to make it configurable in
> the kernel driver as Dmitry suggested (if I remember it correctly).
> Which approach, in the userland or the kernel, do you like, Henrik?


Perhaps I should not remind you that the current code was your suggestion. ;-)
In my mind, these are buttons just like the buttons on any mouse or trackpad.

Regarding remapping, I think it matters if buttons have strong or weak semantics.

Henrik

Re: [PATCH 4/5] input: wacom: Add support for the Bamboo Touch trackpad (rev4)

[Ping Cheng]

On Wed, Oct 13, 2010 at 11:21 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
>
> In the language of ABS_MT, there is a distinction between slot and id. The slot
> is the handle used to communicate a unique touch. The id is the identifier of
> that touch. The device tells us how many slots we have. The range of ids is in
> principle infinite. In practice, it is set to a large number.
>
> To answer a possible follow-up question: from the tracking id, you can tell if a
> contact is present, if it is new, and if it is older than another contact.

I see. Somehow I remember tracking ID is the ID from the hardware/firmware.

>>> +
>>> +       input_report_key(input, BTN_LEFT, (data[1] & 0x08) != 0);
>>> +       input_report_key(input, BTN_FORWARD, (data[1] & 0x04) != 0);
>>> +       input_report_key(input, BTN_BACK, (data[1] & 0x02) != 0);
>>> +       input_report_key(input, BTN_RIGHT, (data[1] & 0x01) != 0);
>>
>> This implementation impairs the value of those buttons since I know a
>> lot of users want them configurable. If we can not or do not pass a
>> generic set to the userland, we will need to make it configurable in
>> the kernel driver as Dmitry suggested (if I remember it correctly).
>> Which approach, in the userland or the kernel, do you like, Henrik?
>
> Perhaps I should not remind you that the current code was your suggestion. ;-)

Yes, I remember that. TBH, I did not want to open another can of worms
then. And I did not have time to care about where that train went :).

> In my mind, these are buttons just like the buttons on any mouse or trackpad.

For this particular model, it might be fine. My concern is for the other models.

> Regarding remapping, I think it matters if buttons have strong or weak semantics.

Supporting button remapping is not a question. It is a decision. The
question is how. I hate to open this can of worms even now. But, we do
have to face the reality.

Currently, I use BTN_TOOL_FINGER to group those buttons together and
to pass them to the userland. This has never been an issue before MT
became such a hot topic. Now, we need to EITHER choose a new
BTN_TOOL_something to pass the poor buttons to the userland OR use the
kernel approach that Dmitry suggested. I am open to all suggestions
although I feel passing the buttons to the userland is a
better/cleaner solution.

My goal is to get a consensus so no one can blame me or the maintainer
for the decision :). Fair?

Ping
--
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 4/5] input: wacom: Add support for the Bamboo Touch trackpad (rev4)

[Chris Bagwell]

On Wed, Oct 13, 2010 at 3:15 PM, Ping Cheng <pinglinux@gmail.com> wrote:
> On Wed, Oct 13, 2010 at 11:21 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
>> In my mind, these are buttons just like the buttons on any mouse or trackpad.
>
> For this particular model, it might be fine. My concern is for the other models.
>
>> Regarding remapping, I think it matters if buttons have strong or weak semantics.
>
> Supporting button remapping is not a question. It is a decision. The
> question is how. I hate to open this can of worms even now. But, we do
> have to face the reality.
>
> Currently, I use BTN_TOOL_FINGER to group those buttons together and
> to pass them to the userland. This has never been an issue before MT
> became such a hot topic. Now, we need to EITHER choose a new
> BTN_TOOL_something to pass the poor buttons to the userland OR use the
> kernel approach that Dmitry suggested. I am open to all suggestions
> although I feel passing the buttons to the userland is a
> better/cleaner solution.
>
> My goal is to get a consensus so no one can blame me or the maintainer
> for the decision :). Fair?
>

Let me first throw out that I think the kernel side solution can be
complementary to user side solutions as well.  We may wish to do it in
both places.

I plan to have some patches to xf86-input-wacom in next week or so
related to this topic.

One thing on kernel side is we should try to keep two set classes of
buttons and not mix their meanings to help out user land.

1) Those buttons that are known to be located on a stylus (BTN_STYLUS
and BTN_STYLUS2)
2) Those buttons that are part of a tablet/touchpad (BTN_LEFT/BTN_RIGHT/etc).

For class #1, there is an implied meaning that when tool goes out of
proximity that its buttons also go out of proximity (user land is
required to do button releases for some known cases the kernel driver
doesn't clear class #1 buttons).  For class #2, they are just always
there and have valid meaning.  So my point is keep in mind to not use,
for example, BTN_STYLUS for buttons that should always work.

For Bamboo Touch, I think we chose class #2 BTN_* for tablet buttons
which will help us in user land.

My basic plan for xf86-input-wacom is to decide on a per event which
tracking buffer to store event in instead of deciding per sync window
as its done today.  For class #1 events, it will be stored like today.
 For class #2 buttons/events, it will be stored in buttons buffer
(which happens to already exists) as if BTN_TOOL_FINGER was sent.

Then 1 or 2 buffers are posted each sync window depending on which were updated.

Its my same basic plan for 2nd finger of MT as well which means
between 1 and 3 buffers will be posted each sync window.

If this approach is accepted then you get normal xf86-input-wacom
button remapping features.

Chris

Re: [PATCH 4/5] input: wacom: Add support for the Bamboo Touch trackpad (rev4)

[Ping Cheng]

On Wed, Oct 13, 2010 at 1:50 PM, Chris Bagwell <chris@cnpbagwell.com> wrote:
> On Wed, Oct 13, 2010 at 3:15 PM, Ping Cheng <pinglinux@gmail.com> wrote:
>> On Wed, Oct 13, 2010 at 11:21 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
>>> In my mind, these are buttons just like the buttons on any mouse or trackpad.
>>
>> For this particular model, it might be fine. My concern is for the other models.
>>
>>> Regarding remapping, I think it matters if buttons have strong or weak semantics.
>>
>> Supporting button remapping is not a question. It is a decision. The
>> question is how. I hate to open this can of worms even now. But, we do
>> have to face the reality.
>>
>> Currently, I use BTN_TOOL_FINGER to group those buttons together and
>> to pass them to the userland. This has never been an issue before MT
>> became such a hot topic. Now, we need to EITHER choose a new
>> BTN_TOOL_something to pass the poor buttons to the userland OR use the
>> kernel approach that Dmitry suggested. I am open to all suggestions
>> although I feel passing the buttons to the userland is a
>> better/cleaner solution.
>>
>> My goal is to get a consensus so no one can blame me or the maintainer
>> for the decision :). Fair?
>>
>
> Let me first throw out that I think the kernel side solution can be
> complementary to user side solutions as well.  We may wish to do it in
> both places.

You get my vote for both kernel and userland approaches, Chris.

> I plan to have some patches to xf86-input-wacom in next week or so
> related to this topic.
>
> One thing on kernel side is we should try to keep two set classes of
> buttons and not mix their meanings to help out user land.
>
> 1) Those buttons that are known to be located on a stylus (BTN_STYLUS
> and BTN_STYLUS2)
> 2) Those buttons that are part of a tablet/touchpad (BTN_LEFT/BTN_RIGHT/etc).
>
> For class #1, there is an implied meaning that when tool goes out of
> proximity that its buttons also go out of proximity (user land is
> required to do button releases for some known cases the kernel driver
> doesn't clear class #1 buttons).  For class #2, they are just always
> there and have valid meaning.  So my point is keep in mind to not use,
> for example, BTN_STYLUS for buttons that should always work.
>
> For Bamboo Touch, I think we chose class #2 BTN_* for tablet buttons
> which will help us in user land.
>
> My basic plan for xf86-input-wacom is to decide on a per event which
> tracking buffer to store event in instead of deciding per sync window
> as its done today.  For class #1 events, it will be stored like today.
>  For class #2 buttons/events, it will be stored in buttons buffer
> (which happens to already exists) as if BTN_TOOL_FINGER was sent.
>
> Then 1 or 2 buffers are posted each sync window depending on which were updated.
>
> Its my same basic plan for 2nd finger of MT as well which means
> between 1 and 3 buffers will be posted each sync window.
>
> If this approach is accepted then you get normal xf86-input-wacom
> button remapping features.

Let's see what the others are thinking....

Ping
--
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 4/5] input: wacom: Add support for the Bamboo Touch trackpad (rev4)

[Ping Cheng]

On Wed, Oct 13, 2010 at 11:21 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
>
>>> +static int wacom_bpt_irq(struct wacom_wac *wacom, size_t len)
>
>>> +{
>>> +       static int trkid;
>>> +       struct input_dev *input = wacom->input;
>>> +       unsigned char *data = wacom->data;
>>> +       int sp = 0, sx = 0, sy = 0, count = 0;
>>> +       int i;
>>> +
>>> +       if (len != WACOM_PKGLEN_BBTOUCH)
>>> +               return 0;
>>> +
>>> +       for (i = 0; i < 2; i++) {
>>> +               int p = data[9 * i + 2];
>>> +               input_mt_slot(input, i);
>>> +               if (p) {
>>> +                       int x = get_unaligned_be16(&data[9 * i + 3]) & 0x7ff;
>>> +                       int y = get_unaligned_be16(&data[9 * i + 5]) & 0x7ff;
>>> +                       input_report_abs(input, ABS_MT_PRESSURE, p);
>>> +                       input_report_abs(input, ABS_MT_POSITION_X, x);
>>> +                       input_report_abs(input, ABS_MT_POSITION_Y, y);
>>> +                       if (wacom->id[i] < 0)
>>> +                               wacom->id[i] = trkid++ & MAX_TRACKING_ID;
>>
>> Why do we need an arbitrary MAX_TRACKING_ID when the device can tell
>> us how many IDs we can have and it tracks the individual fingers for
>> us? In this case, there are only two ID/fingers and the ID can be
>> retrieved from the raw data. I must be missing something in the
>> defintion of MAX_TRACKING_ID.
>
> In the language of ABS_MT, there is a distinction between slot and id. The slot
> is the handle used to communicate a unique touch. The id is the identifier of
> that touch. The device tells us how many slots we have. The range of ids is in
> principle infinite. In practice, it is set to a large number.

Sorry to bring this topic back again. I was under the impression that
I was the only one who didn't get the spec clear. The discussion at
yesterday's UDS session made me feel I am not alone (good or bad :).
Let me try it again to see if I can get it right this time.

From the "Multi-touch (MT) Protocol" under Documentation/input, we see
the following:

"The slot protocol requires the use of the ABS_MT_TRACKING_ID, either
provided by the hardware or computed from the raw data".

That means if the hardware provides the tracking ID, we use it.
Otherwise we lose that specific information reported from the
hardware.

For the Bamboo case, the tracking ID happens to be the same as the
slot ID we use. But, there are devices, as far as I know, report up to
10 fingers/touches. So, there would be 10 slots to report the data.
But, the tracking ID we get from the devices is 0 to 255. So, slot 5
could have a tracking ID of 123 now and 9 when tuch 123 is up and
touch 5 is down.

> To answer a possible follow-up question: from the tracking id, you can tell if a
> contact is present, if it is new, and if it is older than another contact.

The new and old attribute can be tracked by a time stamp in the
userland. Kernel driver doesn't need to worry about it. Maybe I am
missing an use case in the kernel?

Ping
--
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 4/5] input: wacom: Add support for the Bamboo Touch trackpad (rev4)

[Chris Bagwell]

On Fri, Oct 29, 2010 at 3:45 PM, Ping Cheng <pinglinux@gmail.com> wrote:
> On Wed, Oct 13, 2010 at 11:21 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
>>
>>>> +static int wacom_bpt_irq(struct wacom_wac *wacom, size_t len)
>>
>>>> +{
>>>> +       static int trkid;
>>>> +       struct input_dev *input = wacom->input;
>>>> +       unsigned char *data = wacom->data;
>>>> +       int sp = 0, sx = 0, sy = 0, count = 0;
>>>> +       int i;
>>>> +
>>>> +       if (len != WACOM_PKGLEN_BBTOUCH)
>>>> +               return 0;
>>>> +
>>>> +       for (i = 0; i < 2; i++) {
>>>> +               int p = data[9 * i + 2];
>>>> +               input_mt_slot(input, i);
>>>> +               if (p) {
>>>> +                       int x = get_unaligned_be16(&data[9 * i + 3]) & 0x7ff;
>>>> +                       int y = get_unaligned_be16(&data[9 * i + 5]) & 0x7ff;
>>>> +                       input_report_abs(input, ABS_MT_PRESSURE, p);
>>>> +                       input_report_abs(input, ABS_MT_POSITION_X, x);
>>>> +                       input_report_abs(input, ABS_MT_POSITION_Y, y);
>>>> +                       if (wacom->id[i] < 0)
>>>> +                               wacom->id[i] = trkid++ & MAX_TRACKING_ID;
>>>
>>> Why do we need an arbitrary MAX_TRACKING_ID when the device can tell
>>> us how many IDs we can have and it tracks the individual fingers for
>>> us? In this case, there are only two ID/fingers and the ID can be
>>> retrieved from the raw data. I must be missing something in the
>>> defintion of MAX_TRACKING_ID.
>>
>> In the language of ABS_MT, there is a distinction between slot and id. The slot
>> is the handle used to communicate a unique touch. The id is the identifier of
>> that touch. The device tells us how many slots we have. The range of ids is in
>> principle infinite. In practice, it is set to a large number.
>
> Sorry to bring this topic back again. I was under the impression that
> I was the only one who didn't get the spec clear. The discussion at
> yesterday's UDS session made me feel I am not alone (good or bad :).
> Let me try it again to see if I can get it right this time.
>
> From the "Multi-touch (MT) Protocol" under Documentation/input, we see
> the following:
>
> "The slot protocol requires the use of the ABS_MT_TRACKING_ID, either
> provided by the hardware or computed from the raw data".
>
> That means if the hardware provides the tracking ID, we use it.
> Otherwise we lose that specific information reported from the
> hardware.
>
> For the Bamboo case, the tracking ID happens to be the same as the
> slot ID we use. But, there are devices, as far as I know, report up to
> 10 fingers/touches. So, there would be 10 slots to report the data.
> But, the tracking ID we get from the devices is 0 to 255. So, slot 5
> could have a tracking ID of 123 now and 9 when tuch 123 is up and
> touch 5 is down.
>
>> To answer a possible follow-up question: from the tracking id, you can tell if a
>> contact is present, if it is new, and if it is older than another contact.
>
> The new and old attribute can be tracked by a time stamp in the
> userland. Kernel driver doesn't need to worry about it. Maybe I am
> missing an use case in the kernel?
>
> Ping
>

Let me ask the same basic question using some examples.

First, let me say I see no issues with MT spec... its just you may
have a usecase in mind for tracking ID always being unique that not
everyone has fully grasped.

As I'm updating xf86-input-wacom, I see ABS_MT_SLOT as mostly same
meaning as BTN_TOOL_PEN/ERASER/MOUSE.  It gives context of all MT
events to follow for a specific touch.

I next see ABS_MT_TRACKING_ID a little like BTN_TOUCH sent along with
that tool's data.  It tells you when touch/tool is in proximity and
thus that the x/y/pressure values have meaning.  Here -1 is out of
proximity and positive value is in proximity and also a little extra
(how old a touch compared to others, etc).

Now, for HW that enforces slot == touch #, if we made
ABS_MT_TRACKING_ID while in proximity/touching the same value as
ABS_MT_SLOT then user app could simplify and totally ignore
ABS_MT_SLOT event (I think).

So I guess the question is does unique ID solve some specific user
space problem that couldn't be solved with user space timestamps taken
when ABS_MT_TRACKING_ID first becomes non-negative?

Mostly, I expect the answer is "stop being lazy and just look at both
events so you'll work with all hardware types and without timestamps".
 :-)

Chris
--
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 4/5] input: wacom: Add support for the Bamboo Touch trackpad (rev4)

[Ping Cheng]

On Fri, Oct 29, 2010 at 5:55 PM, Chris Bagwell <chris@cnpbagwell.com> wrote:
> On Fri, Oct 29, 2010 at 3:45 PM, Ping Cheng <pinglinux@gmail.com> wrote:
>> On Wed, Oct 13, 2010 at 11:21 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
>>>
>>>>> +static int wacom_bpt_irq(struct wacom_wac *wacom, size_t len)
>>>
>>>>> +{
>>>>> +       static int trkid;
>>>>> +       struct input_dev *input = wacom->input;
>>>>> +       unsigned char *data = wacom->data;
>>>>> +       int sp = 0, sx = 0, sy = 0, count = 0;
>>>>> +       int i;
>>>>> +
>>>>> +       if (len != WACOM_PKGLEN_BBTOUCH)
>>>>> +               return 0;
>>>>> +
>>>>> +       for (i = 0; i < 2; i++) {
>>>>> +               int p = data[9 * i + 2];
>>>>> +               input_mt_slot(input, i);
>>>>> +               if (p) {
>>>>> +                       int x = get_unaligned_be16(&data[9 * i + 3]) & 0x7ff;
>>>>> +                       int y = get_unaligned_be16(&data[9 * i + 5]) & 0x7ff;
>>>>> +                       input_report_abs(input, ABS_MT_PRESSURE, p);
>>>>> +                       input_report_abs(input, ABS_MT_POSITION_X, x);
>>>>> +                       input_report_abs(input, ABS_MT_POSITION_Y, y);
>>>>> +                       if (wacom->id[i] < 0)
>>>>> +                               wacom->id[i] = trkid++ & MAX_TRACKING_ID;
>>>>
>>>> Why do we need an arbitrary MAX_TRACKING_ID when the device can tell
>>>> us how many IDs we can have and it tracks the individual fingers for
>>>> us? In this case, there are only two ID/fingers and the ID can be
>>>> retrieved from the raw data. I must be missing something in the
>>>> defintion of MAX_TRACKING_ID.
>>>
>>> In the language of ABS_MT, there is a distinction between slot and id. The slot
>>> is the handle used to communicate a unique touch. The id is the identifier of
>>> that touch. The device tells us how many slots we have. The range of ids is in
>>> principle infinite. In practice, it is set to a large number.
>>
>> Sorry to bring this topic back again. I was under the impression that
>> I was the only one who didn't get the spec clear. The discussion at
>> yesterday's UDS session made me feel I am not alone (good or bad :).
>> Let me try it again to see if I can get it right this time.
>>
>> From the "Multi-touch (MT) Protocol" under Documentation/input, we see
>> the following:
>>
>> "The slot protocol requires the use of the ABS_MT_TRACKING_ID, either
>> provided by the hardware or computed from the raw data".
>>
>> That means if the hardware provides the tracking ID, we use it.
>> Otherwise we lose that specific information reported from the
>> hardware.
>>
>> For the Bamboo case, the tracking ID happens to be the same as the
>> slot ID we use. But, there are devices, as far as I know, report up to
>> 10 fingers/touches. So, there would be 10 slots to report the data.
>> But, the tracking ID we get from the devices is 0 to 255. So, slot 5
>> could have a tracking ID of 123 now and 9 when tuch 123 is up and
>> touch 5 is down.
>>
>>> To answer a possible follow-up question: from the tracking id, you can tell if a
>>> contact is present, if it is new, and if it is older than another contact.
>>
>> The new and old attribute can be tracked by a time stamp in the
>> userland. Kernel driver doesn't need to worry about it. Maybe I am
>> missing an use case in the kernel?
>>
>> Ping
>>
>
> Let me ask the same basic question using some examples.
>
> First, let me say I see no issues with MT spec... its just you may
> have a usecase in mind for tracking ID always being unique that not
> everyone has fully grasped.

I agree with you (how come I always agree with you :). I am not asking
for a spec change. I am only discussing a code change. And I want to
make sure my code change will not affect any exsiting userland
programs, especially Henrik's programs.

> As I'm updating xf86-input-wacom, I see ABS_MT_SLOT as mostly same
> meaning as BTN_TOOL_PEN/ERASER/MOUSE.  It gives context of all MT
> events to follow for a specific touch.

Right.

> I next see ABS_MT_TRACKING_ID a little like BTN_TOUCH sent along with
> that tool's data.  It tells you when touch/tool is in proximity and
> thus that the x/y/pressure values have meaning.  Here -1 is out of
> proximity and positive value is in proximity and also a little extra
> (how old a touch compared to others, etc).

Agree.

> Now, for HW that enforces slot == touch #, if we made
> ABS_MT_TRACKING_ID while in proximity/touching the same value as
> ABS_MT_SLOT then user app could simplify and totally ignore
> ABS_MT_SLOT event (I think).

Bamboo is a special case which does not make a ddifference for SLOT
and TRACKING IDs. My concern is for those devices that do report
TRACKING IDs differently from the touch #. I want to make the code
consistent accross all devices that support TRACKING IDs.

> So I guess the question is does unique ID solve some specific user
> space problem that couldn't be solved with user space timestamps taken
> when ABS_MT_TRACKING_ID first becomes non-negative?

Some user land developers told me so. And they want to get the
TRACKING_IDs if hardware provides them.

> Mostly, I expect the answer is "stop being lazy and just look at both
> events so you'll work with all hardware types and without timestamps".

Do you mean the answer from me or from you :)?

No, I won't answer that to you or anyone. Not all user land programs
need all the information we report. So, It would be up to the app
developers to decide how to track the data. We, as device driver
developer, only need to report the data as close as hardware provides
and that meets the spec.

Making a patch might be easier to show what meant. I'll do that soon.
I want to make sure my change does not affect Henrik's exsiting
userland programs.

Hope this clears some confusion.

Ping
--
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 4/5] input: wacom: Add support for the Bamboo Touch trackpad (rev4)

[Henrik Rydberg]

On 10/29/2010 10:45 PM, Ping Cheng wrote:
[...]

>>>
>>> Why do we need an arbitrary MAX_TRACKING_ID when the device can tell
>>> us how many IDs we can have and it tracks the individual fingers for
>>> us?


The emphasis is on unique, not on arbitrary. :-)

>>> In this case, there are only two ID/fingers and the ID can be
>>> retrieved from the raw data. I must be missing something in the
>>> defintion of MAX_TRACKING_ID.
>>
>> In the language of ABS_MT, there is a distinction between slot and id. The slot
>> is the handle used to communicate a unique touch. The id is the identifier of
>> that touch. The device tells us how many slots we have. The range of ids is in
>> principle infinite. In practice, it is set to a large number.
> 
> Sorry to bring this topic back again. I was under the impression that
> I was the only one who didn't get the spec clear. The discussion at
> yesterday's UDS session made me feel I am not alone (good or bad :).
> Let me try it again to see if I can get it right this time.
> 
> From the "Multi-touch (MT) Protocol" under Documentation/input, we see
> the following:
> 
> "The slot protocol requires the use of the ABS_MT_TRACKING_ID, either
> provided by the hardware or computed from the raw data".
> 
> That means if the hardware provides the tracking ID, we use it.
> Otherwise we lose that specific information reported from the
> hardware.


The recurring question is: what information is lost?


> For the Bamboo case, the tracking ID happens to be the same as the
> slot ID we use. But, there are devices, as far as I know, report up to
> 10 fingers/touches. So, there would be 10 slots to report the data.
> But, the tracking ID we get from the devices is 0 to 255. So, slot 5
> could have a tracking ID of 123 now and 9 when tuch 123 is up and
> touch 5 is down.
> 
>> To answer a possible follow-up question: from the tracking id, you can tell if a
>> contact is present, if it is new, and if it is older than another contact.
> 
> The new and old attribute can be tracked by a time stamp in the
> userland. Kernel driver doesn't need to worry about it. Maybe I am
> missing an use case in the kernel?


The kernel is the most long-lived process, and userspace programs may be
restarted many times over during the life cycle of a single boot. What happens
when we want to restart an application monitoring a large touch table, for instance?

Regardless of what applications and needs we will see in the future, I feel the
main argument put forward so far, that hardware information must be passed on
untouched at all costs, is more political than technical, and I would very much
like to stay out of such discussions.

Thanks,
Henrik