[PATCH 1/2] add additional reverse-engineered information

Due to recent reverse engineering efforts, a lot more information is now 
available about the internals of the SUR40. We document this in the kernel
driver for future use.

Signed-off-by: Martin Kaltenbrunner <redacted>
Signed-off-by: Florian Echtler <redacted>
---
 drivers/input/touchscreen/sur40.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index 128e5bd..12bdee9 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c

@@ -59,7 +59,7 @@ struct sur40_blob {
 	__le16 blob_id;
 
 	u8 action;         /* 0x02 = enter/exit, 0x03 = update (?) */
-	u8 unknown;        /* always 0x01 or 0x02 (no idea what this is?) */
+	u8 type;           /* bitmask (0x01 blob,  0x02 touch, 0x04 tag) */
 
 	__le16 bb_pos_x;   /* upper left corner of bounding box */
 	__le16 bb_pos_y;

@@ -133,12 +133,19 @@ struct sur40_image_header {
 
 /* control commands */
 #define SUR40_GET_VERSION 0xb0 /* 12 bytes string    */
-#define SUR40_UNKNOWN1    0xb3 /*  5 bytes           */
-#define SUR40_UNKNOWN2    0xc1 /* 24 bytes           */
+#define SUR40_ACCEL_CAPS  0xb3 /*  5 bytes           */
+#define SUR40_SENSOR_CAPS 0xc1 /* 24 bytes           */
+
+#define SUR40_POKE        0xc5 /* poke register byte */
+#define SUR40_PEEK        0xc4 /* 48 bytes registers */
 
 #define SUR40_GET_STATE   0xc5 /*  4 bytes state (?) */
 #define SUR40_GET_SENSORS 0xb1 /*  8 bytes sensors   */
 
+#define SUR40_BLOB   0x01
+#define SUR40_TOUCH	0x02
+#define SUR40_TAG    0x04
+
 static const struct v4l2_pix_format sur40_pix_format[] = {
 	{
 		.pixelformat = V4L2_TCH_FMT_TU08,

@@ -238,11 +245,11 @@ static int sur40_init(struct sur40_state *dev)
 	if (result < 0)
 		goto error;
 
-	result = sur40_command(dev, SUR40_UNKNOWN2,    0x00, buffer, 24);
+	result = sur40_command(dev, SUR40_SENSOR_CAPS, 0x00, buffer, 24);
 	if (result < 0)
 		goto error;
 
-	result = sur40_command(dev, SUR40_UNKNOWN1,    0x00, buffer,  5);
+	result = sur40_command(dev, SUR40_ACCEL_CAPS, 0x00, buffer, 5);
 	if (result < 0)
 		goto error;
 

@@ -367,10 +374,13 @@ static void sur40_poll(struct input_polled_dev *polldev)
 		/*
 		 * Sanity check. when video data is also being retrieved, the
 		 * packet ID will usually increase in the middle of a series
-		 * instead of at the end.
-		 */
-		if (packet_id != le32_to_cpu(header->packet_id))
+		 * instead of at the end. however, the data is still consistent,
+		 * so the packet ID is probably just valid for the first packet
+		 * in a series.
+
+		if (packet_id != header->packet_id)
 			dev_dbg(sur40->dev, "packet ID mismatch\n");
+		 */
 
 		packet_blobs = result / sizeof(struct sur40_blob);
 		dev_dbg(sur40->dev, "received %d blobs\n", packet_blobs);

-- 
2.7.4

The SUR40 labels all reported blobs as touch, token, or generic blob.
Previously, all blobs were reported as touch regardless of type, causing
lots of false positives. Present patch fixes this.

Signed-off-by: Martin Kaltenbrunner <redacted>
Signed-off-by: Florian Echtler <redacted>
---
 drivers/input/touchscreen/sur40.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index 12bdee9..fbd4010 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c

@@ -309,6 +309,8 @@ static void sur40_report_blob(struct sur40_blob *blob, struct input_dev *input)
 	int slotnum = input_mt_get_slot_by_key(input, blob->blob_id);
 	if (slotnum < 0 || slotnum >= MAX_CONTACTS)
 		return;
+	if (blob->type != SUR40_TOUCH)
+		return;
 
 	input_mt_slot(input, slotnum);
 	input_mt_report_slot_state(input, MT_TOOL_FINGER, 1);

-- 
2.7.4

On Sun, Jul 09, 2017 at 08:54:50PM +0200, Florian Echtler wrote:

This chunk does not match the patch description. Please split out.

-- 
Dmitry

On Sun, Jul 09, 2017 at 08:54:51PM +0200, Florian Echtler wrote:

I think we should be checking blob type before trying to get slot
number.

 
 	input_mt_slot(input, slotnum);
 	input_mt_report_slot_state(input, MT_TOOL_FINGER, 1);
-- 
2.7.4

Thanks.

-- 
Dmitry

Good point, I'll send an updated patch ASAP.

Related question: we first attempted to label non-touch objects as MT_TOOL_PALM,
but it looks like userspace (Xorg in particular) doesn't actually distinguish
between MT_TOOL_* types; is that correct?

Best, Florian

On 09.07.2017 23:41, Dmitry Torokhov wrote:

On Sun, Jul 09, 2017 at 08:54:51PM +0200, Florian Echtler wrote:

quoted

The SUR40 labels all reported blobs as touch, token, or generic blob.
Previously, all blobs were reported as touch regardless of type, causing
lots of false positives. Present patch fixes this.

Signed-off-by: Martin Kaltenbrunner <redacted>
Signed-off-by: Florian Echtler <redacted>
---
 drivers/input/touchscreen/sur40.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index 12bdee9..fbd4010 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c

@@ -309,6 +309,8 @@ static void sur40_report_blob(struct sur40_blob *blob, struct input_dev *input)
 	int slotnum = input_mt_get_slot_by_key(input, blob->blob_id);
 	if (slotnum < 0 || slotnum >= MAX_CONTACTS)
 		return;
+	if (blob->type != SUR40_TOUCH)
+		return;

I think we should be checking blob type before trying to get slot
number.

quoted

 
 	input_mt_slot(input, slotnum);
 	input_mt_report_slot_state(input, MT_TOOL_FINGER, 1);
-- 
2.7.4

Thanks.

-- 
SENT FROM MY DEC VT50 TERMINAL

On Mon, Jul 10, 2017 at 09:11:53AM +0200, Florian Echtler wrote:

Good point, I'll send an updated patch ASAP.

Related question: we first attempted to label non-touch objects as MT_TOOL_PALM,
but it looks like userspace (Xorg in particular) doesn't actually distinguish
between MT_TOOL_* types; is that correct?

It really should, but I think Peter never got around implementing this.

Also, I think it is a good idea to set touch major to max in this case.
I believe that that will help clients that do no understand MT_TOOL_PALM
to still do palm rejection.

Peter?

Best, Florian

On 09.07.2017 23:41, Dmitry Torokhov wrote:

quoted

On Sun, Jul 09, 2017 at 08:54:51PM +0200, Florian Echtler wrote:

quoted

The SUR40 labels all reported blobs as touch, token, or generic blob.
Previously, all blobs were reported as touch regardless of type, causing
lots of false positives. Present patch fixes this.

Signed-off-by: Martin Kaltenbrunner <redacted>
Signed-off-by: Florian Echtler <redacted>
---
 drivers/input/touchscreen/sur40.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index 12bdee9..fbd4010 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c

@@ -309,6 +309,8 @@ static void sur40_report_blob(struct sur40_blob *blob, struct input_dev *input)
 	int slotnum = input_mt_get_slot_by_key(input, blob->blob_id);
 	if (slotnum < 0 || slotnum >= MAX_CONTACTS)
 		return;
+	if (blob->type != SUR40_TOUCH)
+		return;

I think we should be checking blob type before trying to get slot
number.

quoted

 
 	input_mt_slot(input, slotnum);
 	input_mt_report_slot_state(input, MT_TOOL_FINGER, 1);
-- 
2.7.4

Thanks.

-- 
SENT FROM MY DEC VT50 TERMINAL

-- 
Dmitry

On Mon, 10 Jul 2017, Dmitry Torokhov wrote:

On Mon, Jul 10, 2017 at 09:11:53AM +0200, Florian Echtler wrote:

quoted

Related question: we first attempted to label non-touch objects as MT_TOOL_PALM,
but it looks like userspace (Xorg in particular) doesn't actually distinguish
between MT_TOOL_* types; is that correct?

It really should, but I think Peter never got around implementing this.

Also, I think it is a good idea to set touch major to max in this case.
I believe that that will help clients that do no understand MT_TOOL_PALM
to still do palm rejection.

Peter?

Would you consider merging v2 of the patch regardless of the Xorg situation? Right now, it's a useful bugfix in any case, and we can deal with how to represent blobs/palms/tokens later on.

Best, Florian
-- 
"_Nothing_ brightens up my morning. Coffee simply provides a shade of
grey just above the pitch-black of the infinite depths of the _abyss_."

On Fri, Jul 14, 2017 at 09:54:03AM +0200, Florian Echtler wrote:

On Mon, 10 Jul 2017, Dmitry Torokhov wrote:

quoted

On Mon, Jul 10, 2017 at 09:11:53AM +0200, Florian Echtler wrote:

quoted

Related question: we first attempted to label non-touch objects as MT_TOOL_PALM,
but it looks like userspace (Xorg in particular) doesn't actually distinguish
between MT_TOOL_* types; is that correct?

It really should, but I think Peter never got around implementing this.

Also, I think it is a good idea to set touch major to max in this case.
I believe that that will help clients that do no understand MT_TOOL_PALM
to still do palm rejection.

Peter?

Would you consider merging v2 of the patch regardless of the Xorg situation?
Right now, it's a useful bugfix in any case, and we can deal with how to
represent blobs/palms/tokens later on.

sorry about the delay, bit chaotic here. libinput 1.8 was released a week or
so ago and it supports MT_TOOL_PALM, so consider userspace ready for that. I
also have patches to use major/minor for palm detection where appropriate
which will hit git master (my) tonight.

That's the libinput situation sorted I think, don't expect synaptics to
catch up with that though (unless someone ends up writing the patches).

Cheers,
   Peter

On 14.07.2017 10:24, Peter Hutterer wrote:

On Fri, Jul 14, 2017 at 09:54:03AM +0200, Florian Echtler wrote:

quoted

On Mon, 10 Jul 2017, Dmitry Torokhov wrote:

quoted

On Mon, Jul 10, 2017 at 09:11:53AM +0200, Florian Echtler wrote:

quoted

Related question: we first attempted to label non-touch objects as MT_TOOL_PALM,
but it looks like userspace (Xorg in particular) doesn't actually distinguish
between MT_TOOL_* types; is that correct?

It really should, but I think Peter never got around implementing this.

Also, I think it is a good idea to set touch major to max in this case.
I believe that that will help clients that do no understand MT_TOOL_PALM
to still do palm rejection.

Peter?

Would you consider merging v2 of the patch regardless of the Xorg situation?
Right now, it's a useful bugfix in any case, and we can deal with how to
represent blobs/palms/tokens later on.

sorry about the delay, bit chaotic here. libinput 1.8 was released a week or
so ago and it supports MT_TOOL_PALM, so consider userspace ready for that. I
also have patches to use major/minor for palm detection where appropriate
which will hit git master (my) tonight.

Nevermind :-) Just to clarify, I can now set MT_TOOL_PALM on generic blob
objects, and they will still be available in userspace "on request", but will
not be considered as touch points?

Do you know how "legacy" xserver-xorg-input-evdev will handle this case?

And final question: the SUR40 also is able to identify specific patterns as
tokens (so-called "bytetags", see
https://github.com/floe/surface-2.0/blob/master/bytetag/bytetag.pdf ). What
would be a sensible way to expose these to userspace, too? Add another
MT_TOOL_TOKEN type?

Thanks & best regards, Florian
-- 
SENT FROM MY DEC VT50 TERMINAL

On 14/07/2017 19:28 , Florian Echtler wrote:

On 14.07.2017 10:24, Peter Hutterer wrote:

quoted

On Fri, Jul 14, 2017 at 09:54:03AM +0200, Florian Echtler wrote:

quoted

On Mon, 10 Jul 2017, Dmitry Torokhov wrote:

quoted

On Mon, Jul 10, 2017 at 09:11:53AM +0200, Florian Echtler wrote:

quoted

Related question: we first attempted to label non-touch objects as MT_TOOL_PALM,
but it looks like userspace (Xorg in particular) doesn't actually distinguish
between MT_TOOL_* types; is that correct?

It really should, but I think Peter never got around implementing this.

Also, I think it is a good idea to set touch major to max in this case.
I believe that that will help clients that do no understand MT_TOOL_PALM
to still do palm rejection.

Peter?

Would you consider merging v2 of the patch regardless of the Xorg situation?
Right now, it's a useful bugfix in any case, and we can deal with how to
represent blobs/palms/tokens later on.

sorry about the delay, bit chaotic here. libinput 1.8 was released a week or
so ago and it supports MT_TOOL_PALM, so consider userspace ready for that. I
also have patches to use major/minor for palm detection where appropriate
which will hit git master (my) tonight.

Nevermind :-) Just to clarify, I can now set MT_TOOL_PALM on generic blob
objects, and they will still be available in userspace "on request", but will
not be considered as touch points?

Do you know how "legacy" xserver-xorg-input-evdev will handle this case?

sorry, I just went back to read the whole thread and realised this is a touchscreen, not a touchpad. Short answer: we don't have tool type handling in touchscreens because no-one asked for it yet. I can probably get this done by the end of next week. File a bug against libinput and assign it to me please.

That's for libinput. evdev is in maintenance mode, it may be possible to add that bit but it largely depends on when the tool type is set. If it's set in the first event already that's trivial to add (I think).

And final question: the SUR40 also is able to identify specific patterns as
tokens (so-called "bytetags", see
https://github.com/floe/surface-2.0/blob/master/bytetag/bytetag.pdf ). What
would be a sensible way to expose these to userspace, too? Add another
MT_TOOL_TOKEN type?

You'd have to add one for each token here, but that may not be too bad, beyond adding a lot of #defines. The tool type is the value field, so we have 32 bits before we run out. 64 tokens won't make a dent there. Maybe use MT_TOKEN_MIN and MT_TOKEN_MAX to keep the defines in check, though I'd recommend considering more specific naming and lots of documentation to detail what the tokens stand for. And consider whether we want to use something overly specific, userspace won't be happy if we have thirty different vendor-specific token ranges.

Cheers,
  Peter