[PATCH RFC 0/8] Input: atmel_mxt_ts - raw data via debugfs

[PATCH RFC 0/8] Input: atmel_mxt_ts - raw data via debugfs

[Nick Dyer]

Hello-

This is a series of patches to add diagnostic data support to the Atmel
maXTouch driver. There's an existing implementation in the open-source mxt-app
tool, however there are performance advantages to moving this code into the driver.
The algorithm for retrieving the data has been fairly consistent across a range of
chips, with the exception of the mXT1386 series (see patch).

The intention is to open-source a utility which can read/display this data, this
should be available very shortly.

It would be good if we could agree a single debugfs interface which could be
supported by all touchscreen chips that have this kind of feature, so I've attempted
to keep that part of this vendor neutral.

This patch sequence is also available from
    https://github.com/ndyer/linux/commits/diagnostic-debug

Any feedback appreciated.

Best regards

Nick Dyer

[PATCH RFC 1/8] Input: atmel_mxt_ts - improve touchscreen size/orientation handling

[Nick Dyer]

Both T100 and T9 handle range and orientation in a similar fashion.
Reduce duplication between the two implementations.

Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 68 ++++++++++++--------------------
 1 file changed, 26 insertions(+), 42 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index c562205..4a1d218 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -113,8 +113,8 @@ struct t7_config {
 #define MXT_T9_DETECT		(1 << 7)
 
 struct t9_range {
-	u16 x;
-	u16 y;
+	__le16 x;
+	__le16 y;
 } __packed;
 
 /* MXT_TOUCH_MULTI_T9 orient */
@@ -216,6 +216,7 @@ struct mxt_data {
 	unsigned int irq;
 	unsigned int max_x;
 	unsigned int max_y;
+	bool xy_switch;
 	bool in_bootloader;
 	u16 mem_size;
 	u8 t100_aux_ampl;
@@ -1665,8 +1666,8 @@ static int mxt_read_t9_resolution(struct mxt_data *data)
 	if (error)
 		return error;
 
-	le16_to_cpus(&range.x);
-	le16_to_cpus(&range.y);
+	data->max_x = get_unaligned_le16(&range.x);
+	data->max_y = get_unaligned_le16(&range.y);
 
 	error =  __mxt_read_reg(client,
 				object->start_address + MXT_T9_ORIENT,
@@ -1674,23 +1675,7 @@ static int mxt_read_t9_resolution(struct mxt_data *data)
 	if (error)
 		return error;
 
-	/* Handle default values */
-	if (range.x == 0)
-		range.x = 1023;
-
-	if (range.y == 0)
-		range.y = 1023;
-
-	if (orient & MXT_T9_ORIENT_SWITCH) {
-		data->max_x = range.y;
-		data->max_y = range.x;
-	} else {
-		data->max_x = range.x;
-		data->max_y = range.y;
-	}
-
-	dev_dbg(&client->dev,
-		"Touchscreen size X%uY%u\n", data->max_x, data->max_y);
+	data->xy_switch = orient & MXT_T9_ORIENT_SWITCH;
 
 	return 0;
 }
@@ -1708,13 +1693,14 @@ static int mxt_read_t100_config(struct mxt_data *data)
 	if (!object)
 		return -EINVAL;
 
+	/* read touchscreen dimensions */
 	error = __mxt_read_reg(client,
 			       object->start_address + MXT_T100_XRANGE,
 			       sizeof(range_x), &range_x);
 	if (error)
 		return error;
 
-	le16_to_cpus(&range_x);
+	data->max_x = get_unaligned_le16(&range_x);
 
 	error = __mxt_read_reg(client,
 			       object->start_address + MXT_T100_YRANGE,
@@ -1722,36 +1708,24 @@ static int mxt_read_t100_config(struct mxt_data *data)
 	if (error)
 		return error;
 
-	le16_to_cpus(&range_y);
+	data->max_y = get_unaligned_le16(&range_y);
 
+	/* read orientation config */
 	error =  __mxt_read_reg(client,
 				object->start_address + MXT_T100_CFG1,
 				1, &cfg);
 	if (error)
 		return error;
 
+	data->xy_switch = cfg & MXT_T100_CFG_SWITCHXY;
+
+	/* allocate aux bytes */
 	error =  __mxt_read_reg(client,
 				object->start_address + MXT_T100_TCHAUX,
 				1, &tchaux);
 	if (error)
 		return error;
 
-	/* Handle default values */
-	if (range_x == 0)
-		range_x = 1023;
-
-	if (range_y == 0)
-		range_y = 1023;
-
-	if (cfg & MXT_T100_CFG_SWITCHXY) {
-		data->max_x = range_y;
-		data->max_y = range_x;
-	} else {
-		data->max_x = range_x;
-		data->max_y = range_y;
-	}
-
-	/* allocate aux bytes */
 	aux = 6;
 
 	if (tchaux & MXT_T100_TCHAUX_VECT)
@@ -1767,9 +1741,6 @@ static int mxt_read_t100_config(struct mxt_data *data)
 		"T100 aux mappings vect:%u ampl:%u area:%u\n",
 		data->t100_aux_vect, data->t100_aux_ampl, data->t100_aux_area);
 
-	dev_info(&client->dev,
-		 "T100 Touchscreen size X%uY%u\n", data->max_x, data->max_y);
-
 	return 0;
 }
 
@@ -1828,6 +1799,19 @@ static int mxt_initialize_input_device(struct mxt_data *data)
 		return -EINVAL;
 	}
 
+	/* Handle default values and orientation switch */
+	if (data->max_x == 0)
+		data->max_x = 1023;
+
+	if (data->max_y == 0)
+		data->max_y = 1023;
+
+	if (data->xy_switch)
+		swap(data->max_x, data->max_y);
+
+	dev_info(dev, "Touchscreen size X%uY%u\n", data->max_x, data->max_y);
+
+	/* Register input device */
 	input_dev = input_allocate_device();
 	if (!input_dev) {
 		dev_err(dev, "Failed to allocate memory\n");
-- 
2.5.0

[PATCH RFC 2/8] Input: atmel_mxt_ts - add diagnostic data retrieval via debugfs

[Nick Dyer]

Retrieve refs data from the T37 diagnostic data object and expose it via
a binary attribute in debugfs.

Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 212 +++++++++++++++++++++++++++++++
 1 file changed, 212 insertions(+)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 4a1d218..a96fec4 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -28,6 +28,7 @@
 #include <linux/of.h>
 #include <linux/slab.h>
 #include <asm/unaligned.h>
+#include <linux/debugfs.h>
 
 /* Firmware files */
 #define MXT_FW_NAME		"maxtouch.fw"
@@ -124,6 +125,17 @@ struct t9_range {
 #define MXT_COMMS_CTRL		0
 #define MXT_COMMS_CMD		1
 
+/* MXT_DEBUG_DIAGNOSTIC_T37 */
+#define MXT_DIAGNOSTIC_PAGEUP 0x01
+#define MXT_DIAGNOSTIC_DELTAS 0x10
+#define MXT_DIAGNOSTIC_SIZE    128
+
+struct t37_debug {
+	u8 mode;
+	u8 page;
+	u8 data[MXT_DIAGNOSTIC_SIZE];
+};
+
 /* Define for MXT_GEN_COMMAND_T6 */
 #define MXT_BOOT_VALUE		0xa5
 #define MXT_RESET_VALUE		0x01
@@ -205,6 +217,20 @@ struct mxt_object {
 	u8 num_report_ids;
 } __packed;
 
+#ifdef CONFIG_DEBUG_FS
+struct mxt_dbg {
+	u16 t37_address;
+	u16 diag_cmd_address;
+	struct t37_debug *t37_buf;
+	u16 *debug_buf;
+	unsigned int t37_pages;
+	unsigned int t37_nodes;
+
+	struct dentry *debugfs_dir;
+	struct dentry *deltas_file;
+};
+#endif
+
 /* Each client has this additional data */
 struct mxt_data {
 	struct i2c_client *client;
@@ -233,6 +259,7 @@ struct mxt_data {
 	u8 num_touchids;
 	u8 multitouch;
 	struct t7_config t7_cfg;
+	struct mxt_dbg dbg;
 
 	/* Cached parameters from object table */
 	u16 T5_address;
@@ -2043,6 +2070,188 @@ recheck:
 	return 0;
 }
 
+#ifdef CONFIG_DEBUG_FS
+static u16 mxt_get_debug_value(struct mxt_data *data, unsigned int x,
+			       unsigned int y)
+{
+	struct mxt_dbg *dbg = &data->dbg;
+	unsigned int ofs, page;
+
+	ofs = (y + (x * (data->info.matrix_ysize))) * sizeof(u16);
+	page = ofs / MXT_DIAGNOSTIC_SIZE;
+	ofs %= MXT_DIAGNOSTIC_SIZE;
+
+	return get_unaligned_le16(&dbg->t37_buf[page].data[ofs]);
+}
+
+static void mxt_convert_debug_pages(struct mxt_data *data)
+{
+	struct mxt_dbg *dbg = &data->dbg;
+	unsigned int x = 0;
+	unsigned int y = 0;
+	unsigned int i;
+
+	for (i = 0; i < dbg->t37_nodes; i++) {
+		dbg->debug_buf[i] = mxt_get_debug_value(data, x, y);
+
+		/* Next value */
+		if (++x >= data->info.matrix_xsize) {
+			x = 0;
+			y++;
+		}
+	}
+}
+
+static int mxt_open_deltas(struct inode *inode, struct file *file)
+{
+	struct mxt_data *data = inode->i_private;
+	struct mxt_dbg *dbg = &data->dbg;
+	int retries = 0;
+	int page;
+	int ret;
+	u8 mode = MXT_DIAGNOSTIC_DELTAS;
+	u8 cmd = mode;
+	struct t37_debug *p;
+
+	for (page = 0; page < dbg->t37_pages; page++) {
+		p = dbg->t37_buf + page;
+
+		ret = mxt_write_reg(data->client, dbg->diag_cmd_address,
+				    cmd);
+		if (ret)
+			return ret;
+
+		retries = 0;
+
+		/* Poll until command is actioned */
+		msleep(20);
+wait_cmd:
+		/* Read first two bytes only */
+		ret = __mxt_read_reg(data->client, dbg->t37_address,
+				     2, p);
+		if (ret)
+			return ret;
+
+		if ((p->mode != mode) || (p->page != page)) {
+			if (retries++ > 100)
+				return -EINVAL;
+
+			msleep(20);
+			goto wait_cmd;
+		}
+
+		/* Read entire T37 page */
+		ret = __mxt_read_reg(data->client, dbg->t37_address,
+				sizeof(struct t37_debug), p);
+		if (ret)
+			return ret;
+
+		dev_dbg(&data->client->dev, "%s page:%d retries:%d\n",
+			__func__, page, retries);
+
+		/* For remaining pages, write PAGEUP rather than mode */
+		cmd = MXT_DIAGNOSTIC_PAGEUP;
+	}
+
+	mxt_convert_debug_pages(data);
+	file->private_data = data;
+
+	return 0;
+}
+
+static ssize_t mxt_read_deltas(struct file *file, char __user *ubuf,
+			       size_t count, loff_t *offp)
+{
+	struct mxt_data *data = file->private_data;
+
+	return simple_read_from_buffer(ubuf, count, offp,
+			data->dbg.debug_buf,
+			data->dbg.t37_nodes * sizeof(u16));
+}
+
+static const struct file_operations atmel_mxt_deltas_fops = {
+	.open = mxt_open_deltas,
+	.read = mxt_read_deltas,
+};
+
+static void mxt_debugfs_remove(struct mxt_data *data)
+{
+	debugfs_remove_recursive(data->dbg.debugfs_dir);
+}
+
+static void mxt_debugfs_init(struct mxt_data *data)
+{
+	struct mxt_dbg *dbg = &data->dbg;
+	struct mxt_object *object;
+	char dirname[50];
+
+	object = mxt_get_object(data, MXT_GEN_COMMAND_T6);
+	if (!object)
+		return;
+
+	dbg->diag_cmd_address = object->start_address + MXT_COMMAND_DIAGNOSTIC;
+
+	object = mxt_get_object(data, MXT_DEBUG_DIAGNOSTIC_T37);
+	if (!object)
+		return;
+
+	if (mxt_obj_size(object) != sizeof(struct t37_debug)) {
+		dev_warn(&data->client->dev, "Bad T37 size");
+		return;
+	}
+
+	dbg->t37_address = object->start_address;
+
+	snprintf(dirname, sizeof(dirname), "heatmap-%s-%s",
+		 dev_driver_string(&data->client->dev),
+		 dev_name(&data->client->dev));
+
+	dbg->debugfs_dir = debugfs_create_dir(dirname, NULL);
+	if (!dbg->debugfs_dir) {
+		dev_err(&data->client->dev, "Error creating debugfs dir\n");
+		return;
+	}
+
+	/* Calculate size of data and allocate buffer */
+	dbg->t37_nodes = data->info.matrix_xsize * data->info.matrix_ysize;
+	dbg->t37_pages = dbg->t37_nodes * sizeof(u16)
+					/ sizeof(dbg->t37_buf->data) + 1;
+	dbg->debug_buf = devm_kzalloc(&data->client->dev,
+				     dbg->t37_nodes * sizeof(u16),
+				     GFP_KERNEL);
+	if (!dbg->debug_buf)
+		goto error;
+
+	dbg->t37_buf = devm_kzalloc(&data->client->dev,
+				     sizeof(struct t37_debug) * dbg->t37_pages,
+				     GFP_KERNEL);
+	if (!dbg->t37_buf)
+		goto error;
+
+	dbg->deltas_file = debugfs_create_file("deltas", S_IRUGO,
+						dbg->debugfs_dir, data,
+						&atmel_mxt_deltas_fops);
+	if (!dbg->deltas_file)
+		goto error;
+
+	dbg->deltas_file->d_inode->i_size = dbg->t37_nodes * sizeof(u16);
+
+	return;
+
+error:
+	dev_err(&data->client->dev, "Error creating debugfs entry\n");
+	mxt_debugfs_remove(data);
+}
+#else
+static inline void mxt_debugfs_remove(struct mxt_data *data)
+{
+}
+
+static inline void mxt_debugfs_init(struct mxt_data *data)
+{
+}
+#endif /* CONFIG_DEBUG_FS */
+
 static int mxt_configure_objects(struct mxt_data *data,
 				 const struct firmware *cfg)
 {
@@ -2070,6 +2279,8 @@ static int mxt_configure_objects(struct mxt_data *data,
 		dev_warn(dev, "No touch object detected\n");
 	}
 
+	mxt_debugfs_init(data);
+
 	dev_info(dev,
 		 "Family: %u Variant: %u Firmware V%u.%u.%02X Objects: %u\n",
 		 info->family_id, info->variant_id, info->version >> 4,
@@ -2617,6 +2828,7 @@ static int mxt_remove(struct i2c_client *client)
 {
 	struct mxt_data *data = i2c_get_clientdata(client);
 
+	mxt_debugfs_remove(data);
 	sysfs_remove_group(&client->dev.kobj, &mxt_attr_group);
 	free_irq(data->irq, data);
 	mxt_free_input_device(data);
-- 
2.5.0

[PATCH RFC 3/8] Input: atmel_mxt_ts - read touchscreen position in matrix

[Nick Dyer]

The touchscreen may have a margin where not all the matrix is used. Read
the parameters from T9 and T100 and take account of the difference.

Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 41 ++++++++++++++++++++++++++++----
 1 file changed, 36 insertions(+), 5 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index a96fec4..9bc42f4 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -100,6 +100,8 @@ struct t7_config {
 
 /* MXT_TOUCH_MULTI_T9 field */
 #define MXT_T9_CTRL		0
+#define MXT_T9_XSIZE		3
+#define MXT_T9_YSIZE		4
 #define MXT_T9_ORIENT		9
 #define MXT_T9_RANGE		18
 
@@ -145,7 +147,9 @@ struct t37_debug {
 #define MXT_T100_CTRL		0
 #define MXT_T100_CFG1		1
 #define MXT_T100_TCHAUX		3
+#define MXT_T100_XSIZE		9
 #define MXT_T100_XRANGE		13
+#define MXT_T100_YSIZE		20
 #define MXT_T100_YRANGE		24
 
 #define MXT_T100_CFG_SWITCHXY	BIT(5)
@@ -243,6 +247,8 @@ struct mxt_data {
 	unsigned int max_x;
 	unsigned int max_y;
 	bool xy_switch;
+	u8 xsize;
+	u8 ysize;
 	bool in_bootloader;
 	u16 mem_size;
 	u8 t100_aux_ampl;
@@ -1688,6 +1694,18 @@ static int mxt_read_t9_resolution(struct mxt_data *data)
 		return -EINVAL;
 
 	error = __mxt_read_reg(client,
+			       object->start_address + MXT_T9_XSIZE,
+			       sizeof(data->xsize), &data->xsize);
+	if (error)
+		return error;
+
+	error = __mxt_read_reg(client,
+			       object->start_address + MXT_T9_YSIZE,
+			       sizeof(data->ysize), &data->ysize);
+	if (error)
+		return error;
+
+	error = __mxt_read_reg(client,
 			       object->start_address + MXT_T9_RANGE,
 			       sizeof(range), &range);
 	if (error)
@@ -1737,6 +1755,18 @@ static int mxt_read_t100_config(struct mxt_data *data)
 
 	data->max_y = get_unaligned_le16(&range_y);
 
+	error = __mxt_read_reg(client,
+			       object->start_address + MXT_T100_XSIZE,
+			       sizeof(data->xsize), &data->xsize);
+	if (error)
+		return error;
+
+	error = __mxt_read_reg(client,
+			       object->start_address + MXT_T100_YSIZE,
+			       sizeof(data->ysize), &data->ysize);
+	if (error)
+		return error;
+
 	/* read orientation config */
 	error =  __mxt_read_reg(client,
 				object->start_address + MXT_T100_CFG1,
@@ -2077,7 +2107,7 @@ static u16 mxt_get_debug_value(struct mxt_data *data, unsigned int x,
 	struct mxt_dbg *dbg = &data->dbg;
 	unsigned int ofs, page;
 
-	ofs = (y + (x * (data->info.matrix_ysize))) * sizeof(u16);
+	ofs = (y + (x * data->info.matrix_ysize)) * sizeof(u16);
 	page = ofs / MXT_DIAGNOSTIC_SIZE;
 	ofs %= MXT_DIAGNOSTIC_SIZE;
 
@@ -2095,7 +2125,7 @@ static void mxt_convert_debug_pages(struct mxt_data *data)
 		dbg->debug_buf[i] = mxt_get_debug_value(data, x, y);
 
 		/* Next value */
-		if (++x >= data->info.matrix_xsize) {
+		if (++x >= data->xsize) {
 			x = 0;
 			y++;
 		}
@@ -2213,9 +2243,10 @@ static void mxt_debugfs_init(struct mxt_data *data)
 	}
 
 	/* Calculate size of data and allocate buffer */
-	dbg->t37_nodes = data->info.matrix_xsize * data->info.matrix_ysize;
-	dbg->t37_pages = dbg->t37_nodes * sizeof(u16)
-					/ sizeof(dbg->t37_buf->data) + 1;
+	dbg->t37_nodes = data->xsize * data->ysize;
+	dbg->t37_pages = ((data->xsize * data->info.matrix_ysize)
+			  * sizeof(u16) / sizeof(dbg->t37_buf->data)) + 1;
+
 	dbg->debug_buf = devm_kzalloc(&data->client->dev,
 				     dbg->t37_nodes * sizeof(u16),
 				     GFP_KERNEL);
-- 
2.5.0

[PATCH RFC 4/8] Input: atmel_mxt_ts - handle diagnostic data orientation

[Nick Dyer]

Invert the diagnostic data to match the orientation of the input device.

Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 9bc42f4..3b1a938 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -122,6 +122,8 @@ struct t9_range {
 
 /* MXT_TOUCH_MULTI_T9 orient */
 #define MXT_T9_ORIENT_SWITCH	(1 << 0)
+#define MXT_T9_ORIENT_INVERTX	(1 << 1)
+#define MXT_T9_ORIENT_INVERTY	(1 << 2)
 
 /* MXT_SPT_COMMSCONFIG_T18 */
 #define MXT_COMMS_CTRL		0
@@ -153,6 +155,8 @@ struct t37_debug {
 #define MXT_T100_YRANGE		24
 
 #define MXT_T100_CFG_SWITCHXY	BIT(5)
+#define MXT_T100_CFG_INVERTY	BIT(6)
+#define MXT_T100_CFG_INVERTX	BIT(7)
 
 #define MXT_T100_TCHAUX_VECT	BIT(0)
 #define MXT_T100_TCHAUX_AMPL	BIT(1)
@@ -246,6 +250,8 @@ struct mxt_data {
 	unsigned int irq;
 	unsigned int max_x;
 	unsigned int max_y;
+	bool invertx;
+	bool inverty;
 	bool xy_switch;
 	u8 xsize;
 	u8 ysize;
@@ -1721,6 +1727,8 @@ static int mxt_read_t9_resolution(struct mxt_data *data)
 		return error;
 
 	data->xy_switch = orient & MXT_T9_ORIENT_SWITCH;
+	data->invertx = orient & MXT_T9_ORIENT_INVERTX;
+	data->inverty = orient & MXT_T9_ORIENT_INVERTY;
 
 	return 0;
 }
@@ -1775,6 +1783,8 @@ static int mxt_read_t100_config(struct mxt_data *data)
 		return error;
 
 	data->xy_switch = cfg & MXT_T100_CFG_SWITCHXY;
+	data->invertx = cfg & MXT_T100_CFG_INVERTX;
+	data->inverty = cfg & MXT_T100_CFG_INVERTY;
 
 	/* allocate aux bytes */
 	error =  __mxt_read_reg(client,
@@ -2119,13 +2129,19 @@ static void mxt_convert_debug_pages(struct mxt_data *data)
 	struct mxt_dbg *dbg = &data->dbg;
 	unsigned int x = 0;
 	unsigned int y = 0;
-	unsigned int i;
+	unsigned int i, rx, ry;
 
 	for (i = 0; i < dbg->t37_nodes; i++) {
-		dbg->debug_buf[i] = mxt_get_debug_value(data, x, y);
+		/* Handle orientation */
+		rx = data->xy_switch ? y : x;
+		ry = data->xy_switch ? x : y;
+		rx = data->invertx ? (data->xsize - 1 - rx) : rx;
+		ry = data->inverty ? (data->ysize - 1 - ry) : ry;
+
+		dbg->debug_buf[i] = mxt_get_debug_value(data, rx, ry);
 
 		/* Next value */
-		if (++x >= data->xsize) {
+		if (++x >= (data->xy_switch ? data->ysize : data->xsize)) {
 			x = 0;
 			y++;
 		}
-- 
2.5.0

[PATCH RFC 5/8] Input: atmel_mxt_ts - add diagnostic data support for mXT1386

[Nick Dyer]

The mXT1386 family of chips have a different architecture which splits
the diagnostic data into 3 columns.

Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 3b1a938..6c9fbc0 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -134,6 +134,10 @@ struct t9_range {
 #define MXT_DIAGNOSTIC_DELTAS 0x10
 #define MXT_DIAGNOSTIC_SIZE    128
 
+#define MXT_FAMILY_1386			160
+#define MXT1386_COLUMNS			3
+#define MXT1386_PAGES_PER_COLUMN	8
+
 struct t37_debug {
 	u8 mode;
 	u8 page;
@@ -2114,13 +2118,27 @@ recheck:
 static u16 mxt_get_debug_value(struct mxt_data *data, unsigned int x,
 			       unsigned int y)
 {
+	struct mxt_info *info = &data->info;
 	struct mxt_dbg *dbg = &data->dbg;
 	unsigned int ofs, page;
+	unsigned int col = 0;
+	unsigned int col_width;
+
+	if (info->family_id == MXT_FAMILY_1386) {
+		col_width = info->matrix_ysize / MXT1386_COLUMNS;
+		col = y / col_width;
+		y = y % col_width;
+	} else {
+		col_width = info->matrix_ysize;
+	}
 
-	ofs = (y + (x * data->info.matrix_ysize)) * sizeof(u16);
+	ofs = (y + (x * col_width)) * sizeof(u16);
 	page = ofs / MXT_DIAGNOSTIC_SIZE;
 	ofs %= MXT_DIAGNOSTIC_SIZE;
 
+	if (info->family_id == MXT_FAMILY_1386)
+		page += col * MXT1386_PAGES_PER_COLUMN;
+
 	return get_unaligned_le16(&dbg->t37_buf[page].data[ofs]);
 }
 
@@ -2227,6 +2245,7 @@ static void mxt_debugfs_remove(struct mxt_data *data)
 
 static void mxt_debugfs_init(struct mxt_data *data)
 {
+	struct mxt_info *info = &data->info;
 	struct mxt_dbg *dbg = &data->dbg;
 	struct mxt_object *object;
 	char dirname[50];
@@ -2260,8 +2279,13 @@ static void mxt_debugfs_init(struct mxt_data *data)
 
 	/* Calculate size of data and allocate buffer */
 	dbg->t37_nodes = data->xsize * data->ysize;
-	dbg->t37_pages = ((data->xsize * data->info.matrix_ysize)
-			  * sizeof(u16) / sizeof(dbg->t37_buf->data)) + 1;
+
+	if (info->family_id == MXT_FAMILY_1386)
+		dbg->t37_pages = MXT1386_COLUMNS * MXT1386_PAGES_PER_COLUMN;
+	else
+		dbg->t37_pages = ((data->xsize * info->matrix_ysize)
+				   * sizeof(u16) / sizeof(dbg->t37_buf->data))
+				   + 1;
 
 	dbg->debug_buf = devm_kzalloc(&data->client->dev,
 				     dbg->t37_nodes * sizeof(u16),
-- 
2.5.0

[PATCH RFC 6/8] Input: atmel_mxt_ts - Add support for reference data

[Nick Dyer]

There are different datatypes available from a maXTouch chip. Add
support to retrieve reference data as well.

Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 78 ++++++++++++++++++++++++--------
 1 file changed, 60 insertions(+), 18 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 6c9fbc0..49fbe2a 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -132,6 +132,7 @@ struct t9_range {
 /* MXT_DEBUG_DIAGNOSTIC_T37 */
 #define MXT_DIAGNOSTIC_PAGEUP 0x01
 #define MXT_DIAGNOSTIC_DELTAS 0x10
+#define MXT_DIAGNOSTIC_REFS   0x11
 #define MXT_DIAGNOSTIC_SIZE    128
 
 #define MXT_FAMILY_1386			160
@@ -211,6 +212,8 @@ enum t100_type {
 
 #define MXT_PIXELS_PER_MM	20
 
+struct mxt_data;
+
 struct mxt_info {
 	u8 family_id;
 	u8 variant_id;
@@ -230,6 +233,29 @@ struct mxt_object {
 } __packed;
 
 #ifdef CONFIG_DEBUG_FS
+struct mxt_debug_datatype {
+	u8 mode;
+	char *name;
+};
+
+struct mxt_debug_datatype_meta {
+	struct mxt_data *data;
+	const struct mxt_debug_datatype *datatype;
+	struct dentry *dent;
+	void *buf;
+};
+
+static const struct mxt_debug_datatype datatypes[] = {
+	{
+		.mode = MXT_DIAGNOSTIC_REFS,
+		.name = "refs",
+	},
+	{
+		.mode = MXT_DIAGNOSTIC_DELTAS,
+		.name = "deltas",
+	}
+};
+
 struct mxt_dbg {
 	u16 t37_address;
 	u16 diag_cmd_address;
@@ -240,6 +266,8 @@ struct mxt_dbg {
 
 	struct dentry *debugfs_dir;
 	struct dentry *deltas_file;
+	struct dentry *refs_file;
+	struct mxt_debug_datatype_meta dt_meta[ARRAY_SIZE(datatypes)];
 };
 #endif
 
@@ -2166,14 +2194,15 @@ static void mxt_convert_debug_pages(struct mxt_data *data)
 	}
 }
 
-static int mxt_open_deltas(struct inode *inode, struct file *file)
+static int mxt_dbg_open(struct inode *inode, struct file *file)
 {
-	struct mxt_data *data = inode->i_private;
+	struct mxt_debug_datatype_meta *dtm = inode->i_private;
+	struct mxt_data *data = dtm->data;
 	struct mxt_dbg *dbg = &data->dbg;
 	int retries = 0;
 	int page;
 	int ret;
-	u8 mode = MXT_DIAGNOSTIC_DELTAS;
+	u8 mode = dtm->datatype->mode;
 	u8 cmd = mode;
 	struct t37_debug *p;
 
@@ -2218,24 +2247,23 @@ wait_cmd:
 	}
 
 	mxt_convert_debug_pages(data);
-	file->private_data = data;
+	file->private_data = dtm;
 
 	return 0;
 }
 
-static ssize_t mxt_read_deltas(struct file *file, char __user *ubuf,
-			       size_t count, loff_t *offp)
+static ssize_t mxt_dbg_read(struct file *file, char __user *ubuf,
+			     size_t count, loff_t *offp)
 {
-	struct mxt_data *data = file->private_data;
+	struct mxt_debug_datatype_meta *dtm = file->private_data;
 
-	return simple_read_from_buffer(ubuf, count, offp,
-			data->dbg.debug_buf,
-			data->dbg.t37_nodes * sizeof(u16));
+	return simple_read_from_buffer(ubuf, count, offp, dtm->buf,
+				       dtm->dent->d_inode->i_size);
 }
 
-static const struct file_operations atmel_mxt_deltas_fops = {
-	.open = mxt_open_deltas,
-	.read = mxt_read_deltas,
+static const struct file_operations atmel_mxt_dbg_fops = {
+	.open = mxt_dbg_open,
+	.read = mxt_dbg_read,
 };
 
 static void mxt_debugfs_remove(struct mxt_data *data)
@@ -2248,7 +2276,10 @@ static void mxt_debugfs_init(struct mxt_data *data)
 	struct mxt_info *info = &data->info;
 	struct mxt_dbg *dbg = &data->dbg;
 	struct mxt_object *object;
+	struct dentry *dent;
+	struct mxt_debug_datatype_meta *dtm;
 	char dirname[50];
+	int i;
 
 	object = mxt_get_object(data, MXT_GEN_COMMAND_T6);
 	if (!object)
@@ -2299,11 +2330,22 @@ static void mxt_debugfs_init(struct mxt_data *data)
 	if (!dbg->t37_buf)
 		goto error;
 
-	dbg->deltas_file = debugfs_create_file("deltas", S_IRUGO,
-						dbg->debugfs_dir, data,
-						&atmel_mxt_deltas_fops);
-	if (!dbg->deltas_file)
-		goto error;
+	for (i = 0; i < ARRAY_SIZE(datatypes); i++) {
+		dtm = &data->dbg.dt_meta[i];
+
+		dtm->data = data;
+		dtm->datatype = datatypes + i;
+		dtm->buf = dbg->debug_buf;
+
+		dent = debugfs_create_file(datatypes[i].name,
+				S_IRUGO, dbg->debugfs_dir,
+				dtm, &atmel_mxt_dbg_fops);
+		if (!dent)
+			goto error;
+
+		dtm->dent = dent;
+		dent->d_inode->i_size = dbg->t37_nodes * sizeof(u16);
+	}
 
 	dbg->deltas_file->d_inode->i_size = dbg->t37_nodes * sizeof(u16);
 
-- 
2.5.0

[PATCH RFC 7/8] Input: atmel_mxt_ts - add metadata to debugfs

[Nick Dyer]

Add information to debugfs to allow a generic utility to retrieve
screen parameters and info.

Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
---
 Documentation/ABI/testing/debugfs-heatmap | 60 +++++++++++++++++++++++++++++++
 drivers/input/touchscreen/atmel_mxt_ts.c  | 46 ++++++++++++++++++++++--
 2 files changed, 103 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/ABI/testing/debugfs-heatmap

diff --git a/Documentation/ABI/testing/debugfs-heatmap b/Documentation/ABI/testing/debugfs-heatmap
new file mode 100644
index 0000000..9246340
--- /dev/null
+++ b/Documentation/ABI/testing/debugfs-heatmap
@@ -0,0 +1,60 @@
+What:		/sys/kernel/debug/heatmap-dev_driver_string-dev_name/
+Date:
+KernelVersion:
+Contact:
+Description:
+	A directory will be created under heatmap for each device which
+	provides heatmap data.
+
+What:		/sys/kernel/debug/heatmap-dev_driver_string-dev_name/datatype/
+Date:
+KernelVersion:
+Contact:
+Description:
+	The device can have multiple heatmap data types. A directory is created
+	for each one.
+
+What:		/sys/kernel/debug/heatmap-xxx/datatype/format
+Date:
+KernelVersion:
+Contact:
+Description:
+	Specifies the type of each data value, one of:
+		uint8
+		uint16
+		uint32
+		int8
+		int16
+		int32
+
+What:		/sys/kernel/debug/heatmap-xxx/datatype/width
+Date:
+KernelVersion:
+Contact:
+Description:
+	The width of the data.
+
+What:		/sys/kernel/debug/heatmap-xxx/datatype/height
+Date:
+KernelVersion:
+Contact:
+Description:
+	The height of the data.
+
+What:		/sys/kernel/debug/heatmap-xxx/datatype/name
+Date:
+KernelVersion:
+Contact:
+Description:
+	Display name for the data.
+
+What:		/sys/kernel/debug/heatmap-xxx/datatype/data
+Date:
+KernelVersion:
+Contact:
+Description:
+	Binary attribute for the data.
+
+	The orientation of the data should correspond to the co-ordinates
+	reported to the input layer. Starting at the top left hand corner, rows
+	then columns.  The endianness of data values will be as per host cpu.
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 49fbe2a..dbe9ebb 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -236,23 +236,33 @@ struct mxt_object {
 struct mxt_debug_datatype {
 	u8 mode;
 	char *name;
+	char *desc;
+	char *format;
 };
 
 struct mxt_debug_datatype_meta {
 	struct mxt_data *data;
 	const struct mxt_debug_datatype *datatype;
 	struct dentry *dent;
+	u16 width;
+	u16 height;
 	void *buf;
+	struct debugfs_blob_wrapper format_wrapper;
+	struct debugfs_blob_wrapper desc_wrapper;
 };
 
 static const struct mxt_debug_datatype datatypes[] = {
 	{
 		.mode = MXT_DIAGNOSTIC_REFS,
 		.name = "refs",
+		.desc = "Mutual Capacitance References",
+		.format = "uint16",
 	},
 	{
 		.mode = MXT_DIAGNOSTIC_DELTAS,
 		.name = "deltas",
+		.format = "int16",
+		.desc = "Mutual Capacitance Deltas",
 	}
 };
 
@@ -2277,6 +2287,7 @@ static void mxt_debugfs_init(struct mxt_data *data)
 	struct mxt_dbg *dbg = &data->dbg;
 	struct mxt_object *object;
 	struct dentry *dent;
+	struct dentry *dir;
 	struct mxt_debug_datatype_meta *dtm;
 	char dirname[50];
 	int i;
@@ -2333,13 +2344,42 @@ static void mxt_debugfs_init(struct mxt_data *data)
 	for (i = 0; i < ARRAY_SIZE(datatypes); i++) {
 		dtm = &data->dbg.dt_meta[i];
 
+		dir = debugfs_create_dir(datatypes[i].name, dbg->debugfs_dir);
+		if (!dir)
+			goto error;
+
 		dtm->data = data;
 		dtm->datatype = datatypes + i;
+		dtm->width = data->xy_switch ? data->ysize : data->xsize;
+		dtm->height = data->xy_switch ? data->xsize: data->ysize;
 		dtm->buf = dbg->debug_buf;
 
-		dent = debugfs_create_file(datatypes[i].name,
-				S_IRUGO, dbg->debugfs_dir,
-				dtm, &atmel_mxt_dbg_fops);
+		dtm->format_wrapper.data = (void *)dtm->datatype->format;
+		dtm->format_wrapper.size = strlen(dtm->datatype->format);
+		dent = debugfs_create_blob("format", S_IRUGO,
+					   dir, &dtm->format_wrapper);
+		if (!dent)
+			goto error;
+
+		dtm->desc_wrapper.data = (void *)dtm->datatype->desc;
+		dtm->desc_wrapper.size = strlen(dtm->datatype->desc);
+		dent = debugfs_create_blob("name", S_IRUGO,
+					   dir, &dtm->desc_wrapper);
+		if (!dent)
+			goto error;
+
+		dent = debugfs_create_u16("width", S_IRUGO,
+					  dir, &dtm->width);
+		if (!dent)
+			goto error;
+
+		dent = debugfs_create_u16("height", S_IRUGO,
+					  dir, &dtm->height);
+		if (!dent)
+			goto error;
+
+		dent = debugfs_create_file("data", S_IRUGO, dir, dtm,
+					   &atmel_mxt_dbg_fops);
 		if (!dent)
 			goto error;
 
-- 
2.5.0

[PATCH RFC 8/8] Input: atmel_mxt_ts - create debugfs info file

[Nick Dyer]

Add a file in debugfs directory with info about the chip.

Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
---
 Documentation/ABI/testing/debugfs-heatmap | 14 ++++++++++++++
 drivers/input/touchscreen/atmel_mxt_ts.c  | 22 ++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/Documentation/ABI/testing/debugfs-heatmap b/Documentation/ABI/testing/debugfs-heatmap
index 9246340..95668a5 100644
--- a/Documentation/ABI/testing/debugfs-heatmap
+++ b/Documentation/ABI/testing/debugfs-heatmap
@@ -6,6 +6,13 @@ Description:
 	A directory will be created under heatmap for each device which
 	provides heatmap data.
 
+What:           /sys/kernel/debug/heatmap-dev_driver_string-dev_name/info
+Date:
+KernelVersion:
+Contact:
+Description:
+        Info relating to the device, eg hardware/firmware version
+
 What:		/sys/kernel/debug/heatmap-dev_driver_string-dev_name/datatype/
 Date:
 KernelVersion:
@@ -48,6 +55,13 @@ Contact:
 Description:
 	Display name for the data.
 
+What:		/sys/kernel/debug/heatmap-xxx/datatype/input_name
+Date:
+KernelVersion:
+Contact:
+Description:
+	The name of the corresponding input device, if relevant.
+
 What:		/sys/kernel/debug/heatmap-xxx/datatype/data
 Date:
 KernelVersion:
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index dbe9ebb..34f3d86 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -277,7 +277,10 @@ struct mxt_dbg {
 	struct dentry *debugfs_dir;
 	struct dentry *deltas_file;
 	struct dentry *refs_file;
+	struct debugfs_blob_wrapper input_name_wrapper;
 	struct mxt_debug_datatype_meta dt_meta[ARRAY_SIZE(datatypes)];
+	char info[50];
+	struct debugfs_blob_wrapper info_wrapper;
 };
 #endif
 
@@ -2319,6 +2322,18 @@ static void mxt_debugfs_init(struct mxt_data *data)
 		return;
 	}
 
+	dbg->info_wrapper.data = dbg->info;
+	dbg->info_wrapper.size = sizeof(dbg->info);
+	scnprintf(dbg->info, sizeof(dbg->info),
+		 "Family: %u Variant: %u Firmware V%u.%u.%02X",
+		 info->family_id, info->variant_id, info->version >> 4,
+		 info->version & 0xf, info->build);
+
+	dent = debugfs_create_blob("info", S_IRUGO, dbg->debugfs_dir,
+			&dbg->info_wrapper);
+	if (!dent)
+		goto error;
+
 	/* Calculate size of data and allocate buffer */
 	dbg->t37_nodes = data->xsize * data->ysize;
 
@@ -2383,6 +2398,13 @@ static void mxt_debugfs_init(struct mxt_data *data)
 		if (!dent)
 			goto error;
 
+		dbg->input_name_wrapper.data = (void *)data->input_dev->name;
+		dbg->input_name_wrapper.size = strlen(data->input_dev->name);
+		dent = debugfs_create_blob("input_name", S_IRUGO,
+					   dir, &dbg->input_name_wrapper);
+		if (!dent)
+			goto error;
+
 		dtm->dent = dent;
 		dent->d_inode->i_size = dbg->t37_nodes * sizeof(u16);
 	}
-- 
2.5.0

Re: [PATCH RFC 0/8] Input: atmel_mxt_ts - raw data via debugfs

[Nick Dyer]

On 02/12/15 20:42, Nick Dyer wrote:
> This is a series of patches to add diagnostic data support to the Atmel
> maXTouch driver. There's an existing implementation in the open-source mxt-app
> tool, however there are performance advantages to moving this code into the driver.
> The algorithm for retrieving the data has been fairly consistent across a range of
> chips, with the exception of the mXT1386 series (see patch).
> 
> The intention is to open-source a utility which can read/display this data, this
> should be available very shortly.

Hi-

The utility to read this data has now been released, and you can find it at:
https://github.com/ndyer/heatmap

I've recorded a couple of videos of the utility in action on a Pixel 2:
* https://youtu.be/M0VD2gZt8Zk
* https://youtu.be/nwDLB4zikzU

regards

Nick

Re: [PATCH RFC 0/8] Input: atmel_mxt_ts - raw data via debugfs

[Dmitry Torokhov]

Hi Nick,

On Thu, Dec 17, 2015 at 05:22:48PM +0000, Nick Dyer wrote:
> On 02/12/15 20:42, Nick Dyer wrote:
> > This is a series of patches to add diagnostic data support to the Atmel
> > maXTouch driver. There's an existing implementation in the open-source mxt-app
> > tool, however there are performance advantages to moving this code into the driver.
> > The algorithm for retrieving the data has been fairly consistent across a range of
> > chips, with the exception of the mXT1386 series (see patch).
> > 
> > The intention is to open-source a utility which can read/display this data, this
> > should be available very shortly.
> 
> Hi-
> 
> The utility to read this data has now been released, and you can find it at:
> https://github.com/ndyer/heatmap
> 
> I've recorded a couple of videos of the utility in action on a Pixel 2:
> * https://youtu.be/M0VD2gZt8Zk
> * https://youtu.be/nwDLB4zikzU

Thank you for sharing the utility and the recording, but it seems that
there is a desire to get access to the heat maps not only for
validation, but also for certain processing purposes, and so I do not
think that we should try to standardize on debugfs as the interface, but
rather look for something that allows better performance.

I wonder if the interface should look similar to the V4L2 capture API
where application opens a character device, uses several ioctls to query
its capabilities/set up capture parameters (i.e reference or deltas),
select()s file descriptor for reading and then uses mmap() to access the
captured heat map.

I've CCed a few people who might be interested in this topic.

Thanks.

-- 
Dmitry

Re: [PATCH RFC 0/8] Input: atmel_mxt_ts - raw data via debugfs

[Benjamin Tissoires]

On Jan 11 2016 or thereabouts, Dmitry Torokhov wrote:
> Hi Nick,
> 
> On Thu, Dec 17, 2015 at 05:22:48PM +0000, Nick Dyer wrote:
> > On 02/12/15 20:42, Nick Dyer wrote:
> > > This is a series of patches to add diagnostic data support to the Atmel
> > > maXTouch driver. There's an existing implementation in the open-source mxt-app
> > > tool, however there are performance advantages to moving this code into the driver.
> > > The algorithm for retrieving the data has been fairly consistent across a range of
> > > chips, with the exception of the mXT1386 series (see patch).
> > > 
> > > The intention is to open-source a utility which can read/display this data, this
> > > should be available very shortly.
> > 
> > Hi-
> > 
> > The utility to read this data has now been released, and you can find it at:
> > https://github.com/ndyer/heatmap
> > 
> > I've recorded a couple of videos of the utility in action on a Pixel 2:
> > * https://youtu.be/M0VD2gZt8Zk
> > * https://youtu.be/nwDLB4zikzU
> 
> Thank you for sharing the utility and the recording, but it seems that
> there is a desire to get access to the heat maps not only for
> validation, but also for certain processing purposes, and so I do not
> think that we should try to standardize on debugfs as the interface, but
> rather look for something that allows better performance.
> 
> I wonder if the interface should look similar to the V4L2 capture API
> where application opens a character device, uses several ioctls to query
> its capabilities/set up capture parameters (i.e reference or deltas),
> select()s file descriptor for reading and then uses mmap() to access the
> captured heat map.
> 
> I've CCed a few people who might be interested in this topic.

I've added Florian, who worked on the driver for the Surface 2.0 which
does exactly that, exports the heat map through V4L2.

See drivers/input/touchscreen/sur40.c for his driver.

Cheers,
Benjamin

> Thanks.
> 
> -- 
> Dmitry

Re: [PATCH RFC 0/8] Input: atmel_mxt_ts - raw data via debugfs

[Nick Dyer]

Hi Ben/Dmitry-

On 12/01/16 08:10, Benjamin Tissoires wrote:
> On Jan 11 2016 or thereabouts, Dmitry Torokhov wrote:
>> On Thu, Dec 17, 2015 at 05:22:48PM +0000, Nick Dyer wrote:
>>>> This is a series of patches to add diagnostic data support to the Atmel
>>>> maXTouch driver. There's an existing implementation in the open-source mxt-app
>>>> tool, however there are performance advantages to moving this code into the driver.
>>>> The algorithm for retrieving the data has been fairly consistent across a range of
>>>> chips, with the exception of the mXT1386 series (see patch).
>>>>
>>>> The intention is to open-source a utility which can read/display this data, this
>>>> should be available very shortly.
>>>
>>> The utility to read this data has now been released, and you can find it at:
>>> https://github.com/ndyer/heatmap
>>>
>>> I've recorded a couple of videos of the utility in action on a Pixel 2:
>>> * https://youtu.be/M0VD2gZt8Zk
>>> * https://youtu.be/nwDLB4zikzU
>>
>> Thank you for sharing the utility and the recording, but it seems that
>> there is a desire to get access to the heat maps not only for
>> validation, but also for certain processing purposes, and so I do not
>> think that we should try to standardize on debugfs as the interface, but
>> rather look for something that allows better performance.
>>
>> I wonder if the interface should look similar to the V4L2 capture API
>> where application opens a character device, uses several ioctls to query
>> its capabilities/set up capture parameters (i.e reference or deltas),
>> select()s file descriptor for reading and then uses mmap() to access the
>> captured heat map.
>>
>> I've CCed a few people who might be interested in this topic.
> 
> I've added Florian, who worked on the driver for the Surface 2.0 which
> does exactly that, exports the heat map through V4L2.
> 
> See drivers/input/touchscreen/sur40.c for his driver.

That's very interesting, thanks. I do think there is a difference in intent
between the debugfs interface we are proposing and the video interface.

The devices we want to support are I2C based and the frame rate is low:
typically less than 10 fps with 1386 nodes, as an example. So performance
is not critical and doing meaningful processing is not really feasible.

The use cases we want to support are diagnostic, for instance tuning and
analysing touch sensor manufacturing defects. It's useful in this context
to be able to interrogate the device using the command line or shell scripts.

Nick

Re: [PATCH RFC 0/8] Input: atmel_mxt_ts - raw data via debugfs

[Dmitry Torokhov]

On Tue, Jan 12, 2016 at 09:10:41AM +0100, Benjamin Tissoires wrote:
> On Jan 11 2016 or thereabouts, Dmitry Torokhov wrote:
> > Hi Nick,
> > 
> > On Thu, Dec 17, 2015 at 05:22:48PM +0000, Nick Dyer wrote:
> > > On 02/12/15 20:42, Nick Dyer wrote:
> > > > This is a series of patches to add diagnostic data support to the Atmel
> > > > maXTouch driver. There's an existing implementation in the open-source mxt-app
> > > > tool, however there are performance advantages to moving this code into the driver.
> > > > The algorithm for retrieving the data has been fairly consistent across a range of
> > > > chips, with the exception of the mXT1386 series (see patch).
> > > > 
> > > > The intention is to open-source a utility which can read/display this data, this
> > > > should be available very shortly.
> > > 
> > > Hi-
> > > 
> > > The utility to read this data has now been released, and you can find it at:
> > > https://github.com/ndyer/heatmap
> > > 
> > > I've recorded a couple of videos of the utility in action on a Pixel 2:
> > > * https://youtu.be/M0VD2gZt8Zk
> > > * https://youtu.be/nwDLB4zikzU
> > 
> > Thank you for sharing the utility and the recording, but it seems that
> > there is a desire to get access to the heat maps not only for
> > validation, but also for certain processing purposes, and so I do not
> > think that we should try to standardize on debugfs as the interface, but
> > rather look for something that allows better performance.
> > 
> > I wonder if the interface should look similar to the V4L2 capture API
> > where application opens a character device, uses several ioctls to query
> > its capabilities/set up capture parameters (i.e reference or deltas),
> > select()s file descriptor for reading and then uses mmap() to access the
> > captured heat map.
> > 
> > I've CCed a few people who might be interested in this topic.
> 
> I've added Florian, who worked on the driver for the Surface 2.0 which
> does exactly that, exports the heat map through V4L2.
> 
> See drivers/input/touchscreen/sur40.c for his driver.

I am not sure if it best to settle on using V4L2 API or creating smaller
API that behaves similarly to V4L2 API though. One of the main
differences is that we only need capture here.

Thanks.

-- 
Dmitry

Re: [PATCH RFC 0/8] Input: atmel_mxt_ts - raw data via debugfs

[Dmitry Torokhov]

On Wed, Jan 13, 2016 at 05:20:57PM +0000, Nick Dyer wrote:
> Hi Ben/Dmitry-
> 
> On 12/01/16 08:10, Benjamin Tissoires wrote:
> > On Jan 11 2016 or thereabouts, Dmitry Torokhov wrote:
> >> On Thu, Dec 17, 2015 at 05:22:48PM +0000, Nick Dyer wrote:
> >>>> This is a series of patches to add diagnostic data support to the Atmel
> >>>> maXTouch driver. There's an existing implementation in the open-source mxt-app
> >>>> tool, however there are performance advantages to moving this code into the driver.
> >>>> The algorithm for retrieving the data has been fairly consistent across a range of
> >>>> chips, with the exception of the mXT1386 series (see patch).
> >>>>
> >>>> The intention is to open-source a utility which can read/display this data, this
> >>>> should be available very shortly.
> >>>
> >>> The utility to read this data has now been released, and you can find it at:
> >>> https://github.com/ndyer/heatmap
> >>>
> >>> I've recorded a couple of videos of the utility in action on a Pixel 2:
> >>> * https://youtu.be/M0VD2gZt8Zk
> >>> * https://youtu.be/nwDLB4zikzU
> >>
> >> Thank you for sharing the utility and the recording, but it seems that
> >> there is a desire to get access to the heat maps not only for
> >> validation, but also for certain processing purposes, and so I do not
> >> think that we should try to standardize on debugfs as the interface, but
> >> rather look for something that allows better performance.
> >>
> >> I wonder if the interface should look similar to the V4L2 capture API
> >> where application opens a character device, uses several ioctls to query
> >> its capabilities/set up capture parameters (i.e reference or deltas),
> >> select()s file descriptor for reading and then uses mmap() to access the
> >> captured heat map.
> >>
> >> I've CCed a few people who might be interested in this topic.
> > 
> > I've added Florian, who worked on the driver for the Surface 2.0 which
> > does exactly that, exports the heat map through V4L2.
> > 
> > See drivers/input/touchscreen/sur40.c for his driver.
> 
> That's very interesting, thanks. I do think there is a difference in intent
> between the debugfs interface we are proposing and the video interface.
> 
> The devices we want to support are I2C based and the frame rate is low:
> typically less than 10 fps with 1386 nodes, as an example. So performance
> is not critical and doing meaningful processing is not really feasible.
> 
> The use cases we want to support are diagnostic, for instance tuning and
> analysing touch sensor manufacturing defects. It's useful in this context
> to be able to interrogate the device using the command line or shell scripts.

I understand that the current controller and firmware you are working on
is not suitable for actual processing and the data rate is only useful
for diagnostic. This does not mean however that we can't use the same
high-speed interface for both diagnostic and processing, if such
interface is available. And given that there is desire to do some of the
host-side processing I'd prefer to standardize on interface that is
suitable for both instead of stuffing driver-specific bits into debugfs.

Thanks.

-- 
Dmitry

Re: [PATCH RFC 0/8] Input: atmel_mxt_ts - raw data via debugfs

[Henrik Rydberg]

>> I understand that the current controller and firmware you are working on
>> is not suitable for actual processing and the data rate is only useful
>> for diagnostic. This does not mean however that we can't use the same
>> high-speed interface for both diagnostic and processing, if such
>> interface is available. And given that there is desire to do some of the
>> host-side processing I'd prefer to standardize on interface that is
>> suitable for both instead of stuffing driver-specific bits into debugfs.

[snip]

> All that said, my hope is that there is room for both APIs so we can retain some
> simple console accessible debug API that works with typical shell commands,
> (cat, hexdump, sed, awk) to support simple testing, while the high performance
> userspace processing API gets what it needs too, which I would think should
> include some low-latency means of getting input events back into the kernel
> without needing uinput.

The notion of a simple high-speed input interface is surely attractive, but
perhaps the mismatch in scope says something important here.

For high-speed io, there are several subsystems that can be used as-is: video,
sound and iio for instance. The last one has matured through industrial beating,
and is now heavily used for bidirectional io (adding Jonathan).

But for the input subsystem, the versatile io device stands in juxtaposition to
the generic userland interface, which is the essence of the objection here.
Input devices and their capabilities should be easily enumerable and
recognizable. An ad-hoc debug interface is not.

So if we see an interest to incorporate high-speed io nodes in the world of
enumerable input devices, maybe it can be as easy as tucking some property and
control ioctls on top a device node driven by any or all of the mentioned
subsystems. It ought to be even simpler to use than debufs. You should certainly
be able to use it as you describe.

Henrik

Re: [PATCH RFC 0/8] Input: atmel_mxt_ts - raw data via debugfs

[Nick Dyer]

On 13/01/16 20:42, Henrik Rydberg wrote:
>>> I understand that the current controller and firmware you are working on
>>> is not suitable for actual processing and the data rate is only useful
>>> for diagnostic. This does not mean however that we can't use the same
>>> high-speed interface for both diagnostic and processing, if such
>>> interface is available. And given that there is desire to do some of the
>>> host-side processing I'd prefer to standardize on interface that is
>>> suitable for both instead of stuffing driver-specific bits into debugfs.
> 
> [snip]
> 
>> All that said, my hope is that there is room for both APIs so we can retain some
>> simple console accessible debug API that works with typical shell commands,
>> (cat, hexdump, sed, awk) to support simple testing, while the high performance
>> userspace processing API gets what it needs too, which I would think should
>> include some low-latency means of getting input events back into the kernel
>> without needing uinput.
> 
> The notion of a simple high-speed input interface is surely attractive, but
> perhaps the mismatch in scope says something important here.
> 
> For high-speed io, there are several subsystems that can be used as-is: video,
> sound and iio for instance. The last one has matured through industrial beating,
> and is now heavily used for bidirectional io (adding Jonathan).
> 
> But for the input subsystem, the versatile io device stands in juxtaposition to
> the generic userland interface, which is the essence of the objection here.
> Input devices and their capabilities should be easily enumerable and
> recognizable. An ad-hoc debug interface is not.
> 
> So if we see an interest to incorporate high-speed io nodes in the world of
> enumerable input devices, maybe it can be as easy as tucking some property and
> control ioctls on top a device node driven by any or all of the mentioned
> subsystems. It ought to be even simpler to use than debufs. You should certainly
> be able to use it as you describe.

Thanks for the useful replies. We've taken a bit of time to have a look at
the V4L2 and IIO subsystems to see how we might fit raw touch data in.

Although IIO has a lot of flexibility, I don't believe it is really
suitable for the case where a single sample is a frame of up to a few
thousand 16-bit ints. I found this mail from Jonathan Cameron last year
that seems to be a similar type of use case:
http://marc.info/?l=linux-iio&m=144585499631661&w=2

Whereas V4L2 is absolutely suitable for handling frames of date, and in
fact we already have an example of it working in the sur40 driver.

It looks to me as though we would need to define a new V4L2_CTRL_CLASS
(v4l2-controls.h) to handle any touch-specific setup from user space
(mainly selecting which data types to capture, since V4L2 already has
metadata for size and data format).

I suppose it depends whether the V4L2 maintainers would be amenable to that
(added some more CCs).

Nick

Re: [PATCH RFC 0/8] Input: atmel_mxt_ts - raw data via debugfs

[Nick Dyer]

> On Wed, Jan 13, 2016 at 05:20:57PM +0000, Nick Dyer wrote:
>> The devices we want to support are I2C based and the frame rate is low:
>> typically less than 10 fps with 1386 nodes, as an example. So performance
>> is not critical and doing meaningful processing is not really feasible.
>>
>> The use cases we want to support are diagnostic, for instance tuning and
>> analysing touch sensor manufacturing defects. It's useful in this context
>> to be able to interrogate the device using the command line or shell scripts.
> 
> I understand that the current controller and firmware you are working on
> is not suitable for actual processing and the data rate is only useful
> for diagnostic. This does not mean however that we can't use the same
> high-speed interface for both diagnostic and processing, if such
> interface is available. And given that there is desire to do some of the
> host-side processing I'd prefer to standardize on interface that is
> suitable for both instead of stuffing driver-specific bits into debugfs.

Hi Dmitry-

We've begun to look at re-implementing this using V4L2.

One additional thing that I wanted to raise in relation to this feature.
The diagnostic data available from these chips is not limited to raw
capacitance value data for each node on the touch matrix. Examples might be:

* One reading per X/Y line, eg a noise measurement
* Single values, for example a frame counter
* Raw pressure/button data from an active stylus

It's unclear exactly how we are going to expose this kind of information at
the moment.

One option seems to be the multi-plane support in V4L2, or defining a
custom struct for embedding this data in the V4L2 frame.

But the number of differences between chip versions means that we might
have to settle for having some way to get the raw data stream out and
forward it to some user space tool which is able to decode.

Nick