Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1384009 > unrolled thread

[PATCH 0/8] Input: atmel_mxt_ts - output raw touch diagnostic data via V4L

Started byNick Dyer <nick.dyer@itdev.co.uk>
First post2016-04-21 11:40 +0200
Last post2016-04-22 17:50 +0200
Articles 9 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 0/8] Input: atmel_mxt_ts - output raw touch diagnostic data via V4L Nick Dyer <nick.dyer@itdev.co.uk> - 2016-04-21 11:40 +0200
    [PATCH 7/8] Input: atmel_mxt_ts - single node diagnostic data support Nick Dyer <nick.dyer@itdev.co.uk> - 2016-04-21 11:40 +0200
    [PATCH 6/8] Input: atmel_mxt_ts - add support for reference data Nick Dyer <nick.dyer@itdev.co.uk> - 2016-04-21 11:40 +0200
    [PATCH 4/8] Input: atmel_mxt_ts - handle diagnostic data orientation Nick Dyer <nick.dyer@itdev.co.uk> - 2016-04-21 11:40 +0200
    Re: [PATCH 0/8] Input: atmel_mxt_ts - output raw touch diagnostic  data via V4L Hans Verkuil <hverkuil@xs4all.nl> - 2016-04-22 10:30 +0200
      Re: [PATCH 0/8] Input: atmel_mxt_ts - output raw touch diagnostic  data via V4L Mauro Carvalho Chehab <mchehab@osg.samsung.com> - 2016-04-22 16:50 +0200
        Re: [PATCH 0/8] Input: atmel_mxt_ts - output raw touch diagnostic  data via V4L Nick Dyer <nick.dyer@itdev.co.uk> - 2016-04-22 17:10 +0200
          Re: [PATCH 0/8] Input: atmel_mxt_ts - output raw touch diagnostic  data via V4L Hans Verkuil <hverkuil@xs4all.nl> - 2016-04-22 17:20 +0200
            Re: [PATCH 0/8] Input: atmel_mxt_ts - output raw touch diagnostic  data via V4L Mauro Carvalho Chehab <mchehab@osg.samsung.com> - 2016-04-22 17:50 +0200

#1384009 — [PATCH 0/8] Input: atmel_mxt_ts - output raw touch diagnostic data via V4L

FromNick Dyer <nick.dyer@itdev.co.uk>
Date2016-04-21 11:40 +0200
Subject[PATCH 0/8] Input: atmel_mxt_ts - output raw touch diagnostic data via V4L
Message-ID<rqjfz-70c-3@gated-at.bofh.it>
This is a series of patches to add diagnostic data support to the Atmel
maXTouch driver. It's a rewrite of the previous implementation which output via
debugfs: it now uses a V4L2 device in a similar way to the sur40 driver.

There are significant performance advantages to putting 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).

We have a utility which can read the data and display it in a useful format:
	https://github.com/ndyer/heatmap/commits/heatmap-v4l

These patches are also available from
	https://github.com/ndyer/linux/commits/diagnostic-v4l

Any feedback appreciated.

[toc] | [next] | [standalone]


#1384010 — [PATCH 7/8] Input: atmel_mxt_ts - single node diagnostic data support

FromNick Dyer <nick.dyer@itdev.co.uk>
Date2016-04-21 11:40 +0200
Subject[PATCH 7/8] Input: atmel_mxt_ts - single node diagnostic data support
Message-ID<rqjfz-70c-17@gated-at.bofh.it>
In reply to#1384009
Add support for retrieving a single node of data at high rate.
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 79 ++++++++++++++++++++++++++++----
 1 file changed, 70 insertions(+), 9 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 6a35d94..3bb1179 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -239,6 +239,7 @@ struct mxt_dbg {
 	struct t37_debug *t37_buf;
 	unsigned int t37_pages;
 	unsigned int t37_nodes;
+	unsigned int single_node_ofs;
 
 	struct v4l2_device v4l2;
 	struct v4l2_pix_format format;
@@ -251,6 +252,8 @@ struct mxt_dbg {
 enum v4l_dbg_inputs {
 	MXT_V4L_INPUT_DELTAS,
 	MXT_V4L_INPUT_REFS,
+	MXT_V4L_INPUT_DELTAS_SINGLE,
+	MXT_V4L_INPUT_REFS_SINGLE,
 	MXT_V4L_INPUT_MAX,
 };
 
@@ -2197,17 +2200,19 @@ static int mxt_convert_debug_pages(struct mxt_data *data, u16 *outbuf)
 }
 
 static int mxt_read_diagnostic_debug(struct mxt_data *data, u8 mode,
-				     u16 *outbuf)
+				     u16 *outbuf, bool single_node)
 {
 	struct mxt_dbg *dbg = &data->dbg;
 	int retries = 0;
 	int page;
+	int pages = single_node ? 1 : dbg->t37_pages;
 	int ret;
 	u8 cmd = mode;
 	struct t37_debug *p;
 	u8 cmd_poll;
 
-	for (page = 0; page < dbg->t37_pages; page++) {
+
+	for (page = 0; page < pages; page++) {
 		p = dbg->t37_buf + page;
 
 		ret = mxt_write_reg(data->client, dbg->diag_cmd_address,
@@ -2251,7 +2256,15 @@ wait_cmd:
 		cmd = MXT_DIAGNOSTIC_PAGEUP;
 	}
 
-	return mxt_convert_debug_pages(data, outbuf);
+	if (single_node) {
+		*outbuf = get_unaligned_le16(&dbg->t37_buf[0]
+					     .data[dbg->single_node_ofs]);
+		ret = 0;
+	} else {
+		ret = mxt_convert_debug_pages(data, outbuf);
+	}
+
+	return ret;
 }
 
 static int mxt_queue_setup(struct vb2_queue *q,
@@ -2278,6 +2291,7 @@ static void mxt_buffer_queue(struct vb2_buffer *vb)
 	u16 *ptr;
 	int ret;
 	u8 mode;
+	bool single_node = false;
 
 	ptr = vb2_plane_vaddr(vb, 0);
 	if (!ptr) {
@@ -2286,17 +2300,21 @@ static void mxt_buffer_queue(struct vb2_buffer *vb)
 	}
 
 	switch (data->dbg.input) {
+	case MXT_V4L_INPUT_DELTAS_SINGLE:
+		single_node = true; /* fall through */
 	case MXT_V4L_INPUT_DELTAS:
 	default:
 		mode = MXT_DIAGNOSTIC_DELTAS;
 		break;
 
+	case MXT_V4L_INPUT_REFS_SINGLE:
+		single_node = true; /* fall through */
 	case MXT_V4L_INPUT_REFS:
 		mode = MXT_DIAGNOSTIC_REFS;
 		break;
 	}
 
-	ret = mxt_read_diagnostic_debug(data, mode, ptr);
+	ret = mxt_read_diagnostic_debug(data, mode, ptr, single_node);
 	if (ret)
 		goto fault;
 
@@ -2360,6 +2378,12 @@ static int mxt_vidioc_enum_input(struct file *file, void *priv,
 	case MXT_V4L_INPUT_DELTAS:
 		strlcpy(i->name, "Mutual Deltas", sizeof(i->name));
 		break;
+	case MXT_V4L_INPUT_REFS_SINGLE:
+		strlcpy(i->name, "Single node refs", sizeof(i->name));
+		break;
+	case MXT_V4L_INPUT_DELTAS_SINGLE:
+		strlcpy(i->name, "Single node deltas", sizeof(i->name));
+		break;
 	}
 
 	return 0;
@@ -2372,8 +2396,20 @@ static int mxt_set_input(struct mxt_data *data, unsigned int i)
 	if (i >= MXT_V4L_INPUT_MAX)
 		return -EINVAL;
 
-	f->width = data->xy_switch ? data->ysize : data->xsize;
-	f->height = data->xy_switch ? data->xsize : data->ysize;
+	switch (i) {
+	case MXT_V4L_INPUT_REFS:
+	case MXT_V4L_INPUT_DELTAS:
+		f->width = data->xy_switch ? data->ysize : data->xsize;
+		f->height = data->xy_switch ? data->xsize : data->ysize;
+		break;
+
+	case MXT_V4L_INPUT_REFS_SINGLE:
+	case MXT_V4L_INPUT_DELTAS_SINGLE:
+		f->width = 1;
+		f->height = 1;
+		break;
+	}
+
 	f->pixelformat = V4L2_PIX_FMT_Y16;
 	f->field = V4L2_FIELD_NONE;
 	f->colorspace = V4L2_COLORSPACE_SRGB;
@@ -2426,11 +2462,21 @@ static int mxt_vidioc_enum_framesizes(struct file *file, void *priv,
 {
 	struct mxt_data *data = video_drvdata(file);
 
-	if (f->index > 0)
+	switch (f->index) {
+	case 0:
+		f->discrete.width = data->xy_switch ? data->ysize:data->xsize;
+		f->discrete.height = data->xy_switch ? data->xsize:data->ysize;
+		break;
+
+	case 1:
+		f->discrete.width = 1;
+		f->discrete.height = 1;
+		break;
+
+	default:
 		return -EINVAL;
+	}
 
-	f->discrete.width = data->xy_switch ? data->ysize : data->xsize;
-	f->discrete.height = data->xy_switch ? data->xsize : data->ysize;
 	f->type = V4L2_FRMSIZE_TYPE_DISCRETE;
 	return 0;
 }
@@ -2479,6 +2525,19 @@ static const struct video_device mxt_video_device = {
 	.release = video_device_release_empty,
 };
 
+static void mxt_debugfs_calc_single_node_ofs(struct mxt_data *data)
+{
+	struct mxt_info *info = &data->info;
+	int ofs = data->ysize / 2;
+
+	while ((ofs + info->matrix_ysize) <= (MXT_DIAGNOSTIC_SIZE/sizeof(u16)))
+		ofs += info->matrix_ysize;
+
+	dev_dbg(&data->client->dev, "Single node ofs: %d\n", ofs);
+
+	data->dbg.single_node_ofs = ofs;
+}
+
 static void mxt_debug_init(struct mxt_data *data)
 {
 	struct mxt_info *info = &data->info;
@@ -2519,6 +2578,8 @@ static void mxt_debug_init(struct mxt_data *data)
 	if (!dbg->t37_buf)
 		goto error;
 
+	mxt_debugfs_calc_single_node_ofs(data);
+
 	/* init channel to zero */
 	mxt_set_input(data, 0);
 
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]


#1384015 — [PATCH 6/8] Input: atmel_mxt_ts - add support for reference data

FromNick Dyer <nick.dyer@itdev.co.uk>
Date2016-04-21 11:40 +0200
Subject[PATCH 6/8] Input: atmel_mxt_ts - add support for reference data
Message-ID<rqjfA-70c-33@gated-at.bofh.it>
In reply to#1384009
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 | 36 ++++++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 4 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index bac0aa0..6a35d94 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -135,6 +135,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
@@ -247,6 +248,12 @@ struct mxt_dbg {
 	int input;
 };
 
+enum v4l_dbg_inputs {
+	MXT_V4L_INPUT_DELTAS,
+	MXT_V4L_INPUT_REFS,
+	MXT_V4L_INPUT_MAX,
+};
+
 static const struct v4l2_file_operations mxt_video_fops = {
 	.owner = THIS_MODULE,
 	.open = v4l2_fh_open,
@@ -2270,6 +2277,7 @@ static void mxt_buffer_queue(struct vb2_buffer *vb)
 	struct mxt_data *data = vb2_get_drv_priv(vb->vb2_queue);
 	u16 *ptr;
 	int ret;
+	u8 mode;
 
 	ptr = vb2_plane_vaddr(vb, 0);
 	if (!ptr) {
@@ -2277,7 +2285,18 @@ static void mxt_buffer_queue(struct vb2_buffer *vb)
 		goto fault;
 	}
 
-	ret = mxt_read_diagnostic_debug(data, MXT_DIAGNOSTIC_DELTAS, ptr);
+	switch (data->dbg.input) {
+	case MXT_V4L_INPUT_DELTAS:
+	default:
+		mode = MXT_DIAGNOSTIC_DELTAS;
+		break;
+
+	case MXT_V4L_INPUT_REFS:
+		mode = MXT_DIAGNOSTIC_REFS;
+		break;
+	}
+
+	ret = mxt_read_diagnostic_debug(data, mode, ptr);
 	if (ret)
 		goto fault;
 
@@ -2327,13 +2346,22 @@ static int mxt_vidioc_querycap(struct file *file, void *priv,
 static int mxt_vidioc_enum_input(struct file *file, void *priv,
 				   struct v4l2_input *i)
 {
-	if (i->index > 0)
+	if (i->index >= MXT_V4L_INPUT_MAX)
 		return -EINVAL;
 
 	i->type = V4L2_INPUT_TYPE_CAMERA;
 	i->std = V4L2_STD_UNKNOWN;
 	i->capabilities = 0;
-	strlcpy(i->name, "Mutual References", sizeof(i->name));
+
+	switch (i->index) {
+	case MXT_V4L_INPUT_REFS:
+		strlcpy(i->name, "Mutual References", sizeof(i->name));
+		break;
+	case MXT_V4L_INPUT_DELTAS:
+		strlcpy(i->name, "Mutual Deltas", sizeof(i->name));
+		break;
+	}
+
 	return 0;
 }
 
@@ -2341,7 +2369,7 @@ static int mxt_set_input(struct mxt_data *data, unsigned int i)
 {
 	struct v4l2_pix_format *f = &data->dbg.format;
 
-	if (i > 0)
+	if (i >= MXT_V4L_INPUT_MAX)
 		return -EINVAL;
 
 	f->width = data->xy_switch ? data->ysize : data->xsize;
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]


#1384016 — [PATCH 4/8] Input: atmel_mxt_ts - handle diagnostic data orientation

FromNick Dyer <nick.dyer@itdev.co.uk>
Date2016-04-21 11:40 +0200
Subject[PATCH 4/8] Input: atmel_mxt_ts - handle diagnostic data orientation
Message-ID<rqjfA-70c-35@gated-at.bofh.it>
In reply to#1384009
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 | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index bcace51..3dd312f 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -125,6 +125,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
@@ -156,6 +158,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)
@@ -260,6 +264,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;
@@ -1743,6 +1749,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;
 }
@@ -1797,6 +1805,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,
@@ -2140,13 +2150,19 @@ static int mxt_convert_debug_pages(struct mxt_data *data, u16 *outbuf)
 	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++) {
-		outbuf[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;
+
+		outbuf[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++;
 		}
@@ -2310,8 +2326,8 @@ static int mxt_set_input(struct mxt_data *data, unsigned int i)
 	if (i > 0)
 		return -EINVAL;
 
-	f->width = data->xsize;
-	f->height = data->ysize;
+	f->width = data->xy_switch ? data->ysize : data->xsize;
+	f->height = data->xy_switch ? data->xsize : data->ysize;
 	f->pixelformat = V4L2_PIX_FMT_Y16;
 	f->field = V4L2_FIELD_NONE;
 	f->colorspace = V4L2_COLORSPACE_SRGB;
@@ -2367,8 +2383,8 @@ static int mxt_vidioc_enum_framesizes(struct file *file, void *priv,
 	if (f->index > 0)
 		return -EINVAL;
 
-	f->discrete.width = data->xsize;
-	f->discrete.height = data->ysize;
+	f->discrete.width = data->xy_switch ? data->ysize : data->xsize;
+	f->discrete.height = data->xy_switch ? data->xsize : data->ysize;
 	f->type = V4L2_FRMSIZE_TYPE_DISCRETE;
 	return 0;
 }
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]


#1384784 — Re: [PATCH 0/8] Input: atmel_mxt_ts - output raw touch diagnostic data via V4L

FromHans Verkuil <hverkuil@xs4all.nl>
Date2016-04-22 10:30 +0200
SubjectRe: [PATCH 0/8] Input: atmel_mxt_ts - output raw touch diagnostic data via V4L
Message-ID<rqEDp-7qZ-29@gated-at.bofh.it>
In reply to#1384009
Hi Nick,

On 04/21/2016 11:31 AM, Nick Dyer wrote:
> This is a series of patches to add diagnostic data support to the Atmel
> maXTouch driver. It's a rewrite of the previous implementation which output via
> debugfs: it now uses a V4L2 device in a similar way to the sur40 driver.
> 
> There are significant performance advantages to putting 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).
> 
> We have a utility which can read the data and display it in a useful format:
> 	https://github.com/ndyer/heatmap/commits/heatmap-v4l
> 
> These patches are also available from
> 	https://github.com/ndyer/linux/commits/diagnostic-v4l
> 
> Any feedback appreciated.

FYI: we're working on a new buffer type for meta data:

https://patchwork.linuxtv.org/patch/33938/
https://patchwork.linuxtv.org/patch/33939/

This would be an excellent fit for you. I expect that this new feature would be
merged soon (for 4.7 or 4.8 at the latest) since it looks all pretty good to me.

So let's wait for this to be merged and then you can migrate to the new buffer
type.

Regards,

	Hans

[toc] | [prev] | [next] | [standalone]


#1385241 — Re: [PATCH 0/8] Input: atmel_mxt_ts - output raw touch diagnostic data via V4L

FromMauro Carvalho Chehab <mchehab@osg.samsung.com>
Date2016-04-22 16:50 +0200
SubjectRe: [PATCH 0/8] Input: atmel_mxt_ts - output raw touch diagnostic data via V4L
Message-ID<rqKz8-3sD-3@gated-at.bofh.it>
In reply to#1384784
Em Fri, 22 Apr 2016 10:26:37 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> Hi Nick,
> 
> On 04/21/2016 11:31 AM, Nick Dyer wrote:
> > This is a series of patches to add diagnostic data support to the Atmel
> > maXTouch driver. It's a rewrite of the previous implementation which output via
> > debugfs: it now uses a V4L2 device in a similar way to the sur40 driver.
> > 
> > There are significant performance advantages to putting 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).
> > 
> > We have a utility which can read the data and display it in a useful format:
> > 	https://github.com/ndyer/heatmap/commits/heatmap-v4l
> > 
> > These patches are also available from
> > 	https://github.com/ndyer/linux/commits/diagnostic-v4l
> > 
> > Any feedback appreciated.  
> 
> FYI: we're working on a new buffer type for meta data:
> 
> https://patchwork.linuxtv.org/patch/33938/
> https://patchwork.linuxtv.org/patch/33939/

Nick,

One of the things I missed on your patchset is the content of the
new format you added (V4L2_PIX_FMT_YS16). You should be patching
the V4L2 docbook too, in order to add it there.

That's said, if the output is really an image, I don't think it
should be mapped via the new V4L2_BUF_TYPE_META_CAPTURE. This type of
buffer is meant to be used on non-image metadata, like image statistics
to feed auto whitebalance and other similar AAA algorithms.

It could still make sense to use the new device type (VFL_TYPE_META) for
such drivers, as we don't want applications to identify those devices as
if they are a webcam.

> 
> This would be an excellent fit for you. I expect that this new feature would be
> merged soon (for 4.7 or 4.8 at the latest) since it looks all pretty good to me.
> 
> So let's wait for this to be merged and then you can migrate to the new buffer
> type.
> 
> Regards,
> 
> 	Hans
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 
Thanks,
Mauro

[toc] | [prev] | [next] | [standalone]


#1385262 — Re: [PATCH 0/8] Input: atmel_mxt_ts - output raw touch diagnostic data via V4L

FromNick Dyer <nick.dyer@itdev.co.uk>
Date2016-04-22 17:10 +0200
SubjectRe: [PATCH 0/8] Input: atmel_mxt_ts - output raw touch diagnostic data via V4L
Message-ID<rqKSu-3S9-23@gated-at.bofh.it>
In reply to#1385241
On 22/04/2016 15:45, Mauro Carvalho Chehab wrote:
> Em Fri, 22 Apr 2016 10:26:37 +0200
> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
>> On 04/21/2016 11:31 AM, Nick Dyer wrote:
>>> This is a series of patches to add diagnostic data support to the Atmel
>>> maXTouch driver. It's a rewrite of the previous implementation which output via
>>> debugfs: it now uses a V4L2 device in a similar way to the sur40 driver.
>>>
>>> There are significant performance advantages to putting 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).
>>>
>>> We have a utility which can read the data and display it in a useful format:
>>> 	https://github.com/ndyer/heatmap/commits/heatmap-v4l
>>>
>>> These patches are also available from
>>> 	https://github.com/ndyer/linux/commits/diagnostic-v4l
>>>
>>> Any feedback appreciated.  
>>
>> FYI: we're working on a new buffer type for meta data:
>>
>> https://patchwork.linuxtv.org/patch/33938/
>> https://patchwork.linuxtv.org/patch/33939/
> 
> One of the things I missed on your patchset is the content of the
> new format you added (V4L2_PIX_FMT_YS16). You should be patching
> the V4L2 docbook too, in order to add it there.

OK, will do. I also see that I forgot Kconfig changes for CONFIG_VIDEO_V4L2
etc.

> That's said, if the output is really an image, I don't think it
> should be mapped via the new V4L2_BUF_TYPE_META_CAPTURE. This type of
> buffer is meant to be used on non-image metadata, like image statistics
> to feed auto whitebalance and other similar AAA algorithms.

The output is raw touch data - i.e. a rectangular grid of nodes each having
an integer value. I think it is an image in some senses, although perhaps
it's a matter of opinion!

You can see an example of a Atmel MXT capacitive touch device here (using
this patchset):
https://www.youtube.com/watch?v=Uj4T6fUCySw

There are touch devices which can deliver much higher resolution/framerate.
For example here's the data coming from a SUR40 which is an optical touch
sensor but uses V4L in a similar way:
https://www.youtube.com/watch?v=e-JNqTY_3b0

> It could still make sense to use the new device type (VFL_TYPE_META) for
> such drivers, as we don't want applications to identify those devices as
> if they are a webcam.

I agree it may be a little confusing if things like Skype start picking up
these devices. Could we #define V4L2_INPUT_TYPE_TOUCH_SENSOR to solve that
problem?

[toc] | [prev] | [next] | [standalone]


#1385265 — Re: [PATCH 0/8] Input: atmel_mxt_ts - output raw touch diagnostic data via V4L

FromHans Verkuil <hverkuil@xs4all.nl>
Date2016-04-22 17:20 +0200
SubjectRe: [PATCH 0/8] Input: atmel_mxt_ts - output raw touch diagnostic data via V4L
Message-ID<rqL2a-3VA-1@gated-at.bofh.it>
In reply to#1385262
On 04/22/2016 05:07 PM, Nick Dyer wrote:
> On 22/04/2016 15:45, Mauro Carvalho Chehab wrote:
>> Em Fri, 22 Apr 2016 10:26:37 +0200
>> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
>>> On 04/21/2016 11:31 AM, Nick Dyer wrote:
>>>> This is a series of patches to add diagnostic data support to the Atmel
>>>> maXTouch driver. It's a rewrite of the previous implementation which output via
>>>> debugfs: it now uses a V4L2 device in a similar way to the sur40 driver.
>>>>
>>>> There are significant performance advantages to putting 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).
>>>>
>>>> We have a utility which can read the data and display it in a useful format:
>>>> 	https://github.com/ndyer/heatmap/commits/heatmap-v4l
>>>>
>>>> These patches are also available from
>>>> 	https://github.com/ndyer/linux/commits/diagnostic-v4l
>>>>
>>>> Any feedback appreciated.  
>>>
>>> FYI: we're working on a new buffer type for meta data:
>>>
>>> https://patchwork.linuxtv.org/patch/33938/
>>> https://patchwork.linuxtv.org/patch/33939/
>>
>> One of the things I missed on your patchset is the content of the
>> new format you added (V4L2_PIX_FMT_YS16). You should be patching
>> the V4L2 docbook too, in order to add it there.
> 
> OK, will do. I also see that I forgot Kconfig changes for CONFIG_VIDEO_V4L2
> etc.
> 
>> That's said, if the output is really an image, I don't think it
>> should be mapped via the new V4L2_BUF_TYPE_META_CAPTURE. This type of
>> buffer is meant to be used on non-image metadata, like image statistics
>> to feed auto whitebalance and other similar AAA algorithms.
> 
> The output is raw touch data - i.e. a rectangular grid of nodes each having
> an integer value. I think it is an image in some senses, although perhaps
> it's a matter of opinion!
> 
> You can see an example of a Atmel MXT capacitive touch device here (using
> this patchset):
> https://www.youtube.com/watch?v=Uj4T6fUCySw
> 
> There are touch devices which can deliver much higher resolution/framerate.
> For example here's the data coming from a SUR40 which is an optical touch
> sensor but uses V4L in a similar way:
> https://www.youtube.com/watch?v=e-JNqTY_3b0
> 
>> It could still make sense to use the new device type (VFL_TYPE_META) for
>> such drivers, as we don't want applications to identify those devices as
>> if they are a webcam.
> 
> I agree it may be a little confusing if things like Skype start picking up
> these devices. Could we #define V4L2_INPUT_TYPE_TOUCH_SENSOR to solve that
> problem?
> 

That might be an idea. I have to admit that I didn't look at the patches in
detail. It mentioned diagnostics, so I didn't realize that it is a image
with a width and height, even though it is not a regular video input.

Adding a new input type won't prevent anyone from picking it up, since
nobody tests that field :-)

On the other hand, it would be a good place to tell the user that it
is from a touch sensor.

Using the upcoming metadata feature wouldn't work since there is no width
and height in the metadata format.

I wonder what others think about adding a new type value.

Regards,

	Hans

[toc] | [prev] | [next] | [standalone]


#1385304 — Re: [PATCH 0/8] Input: atmel_mxt_ts - output raw touch diagnostic data via V4L

FromMauro Carvalho Chehab <mchehab@osg.samsung.com>
Date2016-04-22 17:50 +0200
SubjectRe: [PATCH 0/8] Input: atmel_mxt_ts - output raw touch diagnostic data via V4L
Message-ID<rqLvd-49a-35@gated-at.bofh.it>
In reply to#1385265
Em Fri, 22 Apr 2016 17:18:24 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On 04/22/2016 05:07 PM, Nick Dyer wrote:
> > On 22/04/2016 15:45, Mauro Carvalho Chehab wrote:  
> >> Em Fri, 22 Apr 2016 10:26:37 +0200
> >> Hans Verkuil <hverkuil@xs4all.nl> escreveu:  
> >>> On 04/21/2016 11:31 AM, Nick Dyer wrote:  
> >>>> This is a series of patches to add diagnostic data support to the Atmel
> >>>> maXTouch driver. It's a rewrite of the previous implementation which output via
> >>>> debugfs: it now uses a V4L2 device in a similar way to the sur40 driver.
> >>>>
> >>>> There are significant performance advantages to putting 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).
> >>>>
> >>>> We have a utility which can read the data and display it in a useful format:
> >>>> 	https://github.com/ndyer/heatmap/commits/heatmap-v4l
> >>>>
> >>>> These patches are also available from
> >>>> 	https://github.com/ndyer/linux/commits/diagnostic-v4l
> >>>>
> >>>> Any feedback appreciated.    
> >>>
> >>> FYI: we're working on a new buffer type for meta data:
> >>>
> >>> https://patchwork.linuxtv.org/patch/33938/
> >>> https://patchwork.linuxtv.org/patch/33939/  
> >>
> >> One of the things I missed on your patchset is the content of the
> >> new format you added (V4L2_PIX_FMT_YS16). You should be patching
> >> the V4L2 docbook too, in order to add it there.  
> > 
> > OK, will do. I also see that I forgot Kconfig changes for CONFIG_VIDEO_V4L2
> > etc.
> >   
> >> That's said, if the output is really an image, I don't think it
> >> should be mapped via the new V4L2_BUF_TYPE_META_CAPTURE. This type of
> >> buffer is meant to be used on non-image metadata, like image statistics
> >> to feed auto whitebalance and other similar AAA algorithms.  
> > 
> > The output is raw touch data - i.e. a rectangular grid of nodes each having
> > an integer value. I think it is an image in some senses, although perhaps
> > it's a matter of opinion!
> > 
> > You can see an example of a Atmel MXT capacitive touch device here (using
> > this patchset):
> > https://www.youtube.com/watch?v=Uj4T6fUCySw
> > 
> > There are touch devices which can deliver much higher resolution/framerate.
> > For example here's the data coming from a SUR40 which is an optical touch
> > sensor but uses V4L in a similar way:
> > https://www.youtube.com/watch?v=e-JNqTY_3b0
> >   
> >> It could still make sense to use the new device type (VFL_TYPE_META) for
> >> such drivers, as we don't want applications to identify those devices as
> >> if they are a webcam.  
> > 
> > I agree it may be a little confusing if things like Skype start picking up
> > these devices. Could we #define V4L2_INPUT_TYPE_TOUCH_SENSOR to solve that
> > problem?
> >   
> 
> That might be an idea. I have to admit that I didn't look at the patches in
> detail. It mentioned diagnostics, so I didn't realize that it is a image
> with a width and height, even though it is not a regular video input.
> 
> Adding a new input type won't prevent anyone from picking it up, since
> nobody tests that field :-)

Yeah, I agree.

> On the other hand, it would be a good place to tell the user that it
> is from a touch sensor.
> 
> Using the upcoming metadata feature wouldn't work since there is no width
> and height in the metadata format.
> 
> I wonder what others think about adding a new type value.

IMO, two things should be done here:

1) Add some caps flag to help userspace to identify what's there
   on those devices;

2) Make sure that udev/systemd won't be naming the devnodes as
   "/dev/video";


The latter one could be solved with either the new dev meta or
with another VFL_TYPE for input systems (like VFL_TYPE_TOUCH_SENSOR)
and use this code snippet:

diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index d8e5994cccf1..4d3e574eba49 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -887,6 +887,9 @@ int __video_register_device(struct video_device *vdev, int type, int nr,
                /* Use device name 'swradio' because 'sdr' was already taken. */
                name_base = "swradio";
                break;
+       case VFL_TYPE_TOUCH_SENSOR:
+               name_base = "v4l-touch";
+               break;
        default:
                printk(KERN_ERR "%s called with unknown type: %d\n",
                       __func__, type);


Such change would cause __video_register_device() to pass a different
name_base to:
	dev_set_name(&vdev->dev, "%s%d", name_base, vdev->num);

This way, udev/systemd will use a different name (by default, 
/dev/v4l-touch0), and existing apps won't identify this as a
webcam.

Regards,
Mauro

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web