[PATCH 1/3] hid: 3m: Adjust to sequential MT HID protocol

[PATCH 1/3] hid: 3m: Adjust to sequential MT HID protocol

[Henrik Rydberg]

The multitouch extensions to the HID protocol allows for contact
data to be sent over several reports, which is also the case for
the 3M M2256PW touchscreen. This patch modifies the logic to only
synchronize the input layer when all contacts have been received.
Consequentially, the full 60-finger capacity of the device is enabled.

Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
 drivers/hid/hid-3m-pct.c |   22 ++++++++++++++++++----
 1 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-3m-pct.c b/drivers/hid/hid-3m-pct.c
index 2a0d56b..1057430 100644
--- a/drivers/hid/hid-3m-pct.c
+++ b/drivers/hid/hid-3m-pct.c
@@ -24,6 +24,9 @@ MODULE_LICENSE("GPL");
 
 #include "hid-ids.h"
 
+#define MAX_SLOTS		60
+#define MAX_TRKID		59
+
 struct mmm_finger {
 	__s32 x, y, w, h;
 	__u8 rank;
@@ -31,8 +34,9 @@ struct mmm_finger {
 };
 
 struct mmm_data {
-	struct mmm_finger f[10];
+	struct mmm_finger f[MAX_SLOTS];
 	__u8 curid, num;
+	__u8 nexp, nreal;
 	bool touch, valid;
 };
 
@@ -93,7 +97,7 @@ static int mmm_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 					1, 1, 0, 0);
 			return 1;
 		case HID_DG_CONTACTID:
-			field->logical_maximum = 59;
+			field->logical_maximum = MAX_TRKID;
 			hid_map_usage(hi, usage, bit, max,
 					EV_ABS, ABS_MT_TRACKING_ID);
 			return 1;
@@ -133,7 +137,7 @@ static void mmm_filter_event(struct mmm_data *md, struct input_dev *input)
 	 * we need to iterate on all fingers to decide if we have a press
 	 * or a release event in our touchscreen emulation.
 	 */
-	for (i = 0; i < 10; ++i) {
+	for (i = 0; i < MAX_SLOTS; ++i) {
 		struct mmm_finger *f = &md->f[i];
 		if (!f->valid) {
 			/* this finger is just placeholder data, ignore */
@@ -190,6 +194,7 @@ static void mmm_filter_event(struct mmm_data *md, struct input_dev *input)
 	} else if (released) {
 		input_event(input, EV_KEY, BTN_TOUCH, 0);
 	}
+	input_sync(input);
 }
 
 /*
@@ -223,10 +228,12 @@ static int mmm_event(struct hid_device *hid, struct hid_field *field,
 				md->f[md->curid].h = value;
 			break;
 		case HID_DG_CONTACTID:
+			value = clamp_val(value, 0, MAX_SLOTS - 1);
 			if (md->valid) {
 				md->curid = value;
 				md->f[value].touch = md->touch;
 				md->f[value].valid = 1;
+				md->nreal++;
 			}
 			break;
 		case HID_GD_X:
@@ -238,7 +245,12 @@ static int mmm_event(struct hid_device *hid, struct hid_field *field,
 				md->f[md->curid].y = value;
 			break;
 		case HID_DG_CONTACTCOUNT:
-			mmm_filter_event(md, input);
+			if (value)
+				md->nexp = value;
+			if (md->nreal >= md->nexp) {
+				mmm_filter_event(md, input);
+				md->nreal = 0;
+			}
 			break;
 		}
 	}
@@ -255,6 +267,8 @@ static int mmm_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	int ret;
 	struct mmm_data *md;
 
+	hdev->quirks |= HID_QUIRK_NO_INPUT_SYNC;
+
 	md = kzalloc(sizeof(struct mmm_data), GFP_KERNEL);
 	if (!md) {
 		dev_err(&hdev->dev, "cannot allocate 3M data\n");
-- 
1.7.1

[PATCH 2/3] hid: 3m: Convert to MT slots

[Henrik Rydberg]

The Microtouch controller is capable of doing finger tracking on
up to 60 fingers. To reduce bandwidth and cpu usage, convert the
driver to use the MT slots protocol.

Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
 drivers/hid/hid-3m-pct.c |   50 +++++++++++++++++++++++++++++++++++----------
 1 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/drivers/hid/hid-3m-pct.c b/drivers/hid/hid-3m-pct.c
index 1057430..9a3b047 100644
--- a/drivers/hid/hid-3m-pct.c
+++ b/drivers/hid/hid-3m-pct.c
@@ -25,16 +25,22 @@ MODULE_LICENSE("GPL");
 #include "hid-ids.h"
 
 #define MAX_SLOTS		60
-#define MAX_TRKID		59
+#define MAX_TRKID		USHRT_MAX
+#define MAX_EVENTS		360
+#define SN_MOVE			2048
+#define SN_WIDTH		128
 
 struct mmm_finger {
 	__s32 x, y, w, h;
+	__u16 id;
 	__u8 rank;
+	bool prev_touch;
 	bool touch, valid;
 };
 
 struct mmm_data {
 	struct mmm_finger f[MAX_SLOTS];
+	__u16 id;
 	__u8 curid, num;
 	__u8 nexp, nreal;
 	bool touch, valid;
@@ -44,6 +50,10 @@ static int mmm_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		struct hid_field *field, struct hid_usage *usage,
 		unsigned long **bit, int *max)
 {
+	int f1 = field->logical_minimum;
+	int f2 = field->logical_maximum;
+	int df = f2 - f1;
+
 	switch (usage->hid & HID_USAGE_PAGE) {
 
 	case HID_UP_BUTTON:
@@ -54,18 +64,20 @@ static int mmm_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		case HID_GD_X:
 			hid_map_usage(hi, usage, bit, max,
 					EV_ABS, ABS_MT_POSITION_X);
+			input_set_abs_params(hi->input, ABS_MT_POSITION_X,
+					     f1, f2, df / SN_MOVE, 0);
 			/* touchscreen emulation */
 			input_set_abs_params(hi->input, ABS_X,
-						field->logical_minimum,
-						field->logical_maximum, 0, 0);
+					     f1, f2, df / SN_MOVE, 0);
 			return 1;
 		case HID_GD_Y:
 			hid_map_usage(hi, usage, bit, max,
 					EV_ABS, ABS_MT_POSITION_Y);
+			input_set_abs_params(hi->input, ABS_MT_POSITION_Y,
+					     f1, f2, df / SN_MOVE, 0);
 			/* touchscreen emulation */
 			input_set_abs_params(hi->input, ABS_Y,
-						field->logical_minimum,
-						field->logical_maximum, 0, 0);
+					     f1, f2, df / SN_MOVE, 0);
 			return 1;
 		}
 		return 0;
@@ -85,14 +97,19 @@ static int mmm_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		case HID_DG_TIPSWITCH:
 			/* touchscreen emulation */
 			hid_map_usage(hi, usage, bit, max, EV_KEY, BTN_TOUCH);
+			input_set_capability(hi->input, EV_KEY, BTN_TOUCH);
 			return 1;
 		case HID_DG_WIDTH:
 			hid_map_usage(hi, usage, bit, max,
 					EV_ABS, ABS_MT_TOUCH_MAJOR);
+			input_set_abs_params(hi->input, ABS_MT_TOUCH_MAJOR,
+					     f1, f2, df / SN_WIDTH, 0);
 			return 1;
 		case HID_DG_HEIGHT:
 			hid_map_usage(hi, usage, bit, max,
 					EV_ABS, ABS_MT_TOUCH_MINOR);
+			input_set_abs_params(hi->input, ABS_MT_TOUCH_MINOR,
+					     f1, f2, df / SN_WIDTH, 0);
 			input_set_abs_params(hi->input, ABS_MT_ORIENTATION,
 					1, 1, 0, 0);
 			return 1;
@@ -100,6 +117,11 @@ static int mmm_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 			field->logical_maximum = MAX_TRKID;
 			hid_map_usage(hi, usage, bit, max,
 					EV_ABS, ABS_MT_TRACKING_ID);
+			input_set_abs_params(hi->input, ABS_MT_TRACKING_ID,
+					     0, MAX_TRKID, 0, 0);
+			if (!hi->input->mt)
+				input_mt_create_slots(hi->input, MAX_SLOTS);
+			input_set_events_per_packet(hi->input, MAX_EVENTS);
 			return 1;
 		}
 		/* let hid-input decide for the others */
@@ -117,10 +139,10 @@ static int mmm_input_mapped(struct hid_device *hdev, struct hid_input *hi,
 		struct hid_field *field, struct hid_usage *usage,
 		unsigned long **bit, int *max)
 {
+	/* tell hid-input to skip setup of these event types */
 	if (usage->type == EV_KEY || usage->type == EV_ABS)
-		clear_bit(usage->code, *bit);
-
-	return 0;
+		set_bit(usage->type, hi->input->evbit);
+	return -1;
 }
 
 /*
@@ -141,10 +163,15 @@ static void mmm_filter_event(struct mmm_data *md, struct input_dev *input)
 		struct mmm_finger *f = &md->f[i];
 		if (!f->valid) {
 			/* this finger is just placeholder data, ignore */
-		} else if (f->touch) {
+			continue;
+		}
+		input_mt_slot(input, i);
+		if (f->touch) {
 			/* this finger is on the screen */
 			int wide = (f->w > f->h);
-			input_event(input, EV_ABS, ABS_MT_TRACKING_ID, i);
+			if (!f->prev_touch)
+				f->id = md->id++;
+			input_event(input, EV_ABS, ABS_MT_TRACKING_ID, f->id);
 			input_event(input, EV_ABS, ABS_MT_POSITION_X, f->x);
 			input_event(input, EV_ABS, ABS_MT_POSITION_Y, f->y);
 			input_event(input, EV_ABS, ABS_MT_ORIENTATION, wide);
@@ -152,7 +179,6 @@ static void mmm_filter_event(struct mmm_data *md, struct input_dev *input)
 						wide ? f->w : f->h);
 			input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR,
 						wide ? f->h : f->w);
-			input_mt_sync(input);
 			/*
 			 * touchscreen emulation: maintain the age rank
 			 * of this finger, decide if we have a press
@@ -181,7 +207,9 @@ static void mmm_filter_event(struct mmm_data *md, struct input_dev *input)
 			--(md->num);
 			if (md->num == 0)
 				released = true;
+			input_event(input, EV_ABS, ABS_MT_TRACKING_ID, -1);
 		}
+		f->prev_touch = f->touch;
 		f->valid = 0;
 	}
 
-- 
1.7.1

Re: [PATCH 2/3] hid: 3m: Convert to MT slots

[Stéphane Chatty]

Le 28 août 10 à 16:29, Henrik Rydberg a écrit :

> The Microtouch controller is capable of doing finger tracking on
> up to 60 fingers. To reduce bandwidth and cpu usage, convert the
> driver to use the MT slots protocol.

As I understand it, this patch actually has three roles:
  1. improving the evdev parameters of the device
  2. cleaning the protocol to get rid of useless MISC/SCANCODE messages
  3. convert the protocol to MT slots


> +#define SN_MOVE			2048
> +#define SN_WIDTH		128
>

In the long run, it might be useful to comment these signal/noise  
constants



>  			hid_map_usage(hi, usage, bit, max, EV_KEY, BTN_TOUCH);
> +			input_set_capability(hi->input, EV_KEY, BTN_TOUCH);

>  {
> +	/* tell hid-input to skip setup of these event types */
>  	if (usage->type == EV_KEY || usage->type == EV_ABS)
> -		clear_bit(usage->code, *bit);
> -
> -	return 0;
> +		set_bit(usage->type, hi->input->evbit);
> +	return -1;
>  }
>

I understand this as a trick to get rid of MISC/SCANCODEs that are  
added for every EV_KEY message, consequence of a rule in hid-input.c.  
Wouldn't it be simpler to improve the rule than to work around it?

Cheers,

St.

Re: [PATCH 2/3] hid: 3m: Convert to MT slots

[Henrik Rydberg]

On 09/20/2010 09:43 PM, Stéphane Chatty wrote:

> 
> Le 28 août 10 à 16:29, Henrik Rydberg a écrit :
> 
>> The Microtouch controller is capable of doing finger tracking on
>> up to 60 fingers. To reduce bandwidth and cpu usage, convert the
>> driver to use the MT slots protocol.
> 
> As I understand it, this patch actually has three roles:
>  1. improving the evdev parameters of the device
>  2. cleaning the protocol to get rid of useless MISC/SCANCODE messages
>  3. convert the protocol to MT slots
> 
> 
>> +#define SN_MOVE            2048
>> +#define SN_WIDTH        128
>>
> 
> In the long run, it might be useful to comment these signal/noise constants
> 
> 
> 
>>              hid_map_usage(hi, usage, bit, max, EV_KEY, BTN_TOUCH);
>> +            input_set_capability(hi->input, EV_KEY, BTN_TOUCH);
> 
>>  {
>> +    /* tell hid-input to skip setup of these event types */
>>      if (usage->type == EV_KEY || usage->type == EV_ABS)
>> -        clear_bit(usage->code, *bit);
>> -
>> -    return 0;
>> +        set_bit(usage->type, hi->input->evbit);
>> +    return -1;
>>  }
>>
> 
> I understand this as a trick to get rid of MISC/SCANCODEs that are added for
> every EV_KEY message, consequence of a rule in hid-input.c. Wouldn't it be
> simpler to improve the rule than to work around it?


The main reason is to be able to set the fuzz parameters, which are otherwise
overwritten by hid-input setup.

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

Re: [PATCH 2/3] hid: 3m: Convert to MT slots

[Stéphane Chatty]

Le 20 sept. 10 à 21:48, Henrik Rydberg a écrit :
>>
>> I understand this as a trick to get rid of MISC/SCANCODEs that are  
>> added for
>> every EV_KEY message, consequence of a rule in hid-input.c.  
>> Wouldn't it be
>> simpler to improve the rule than to work around it?
>
>
> The main reason is to be able to set the fuzz parameters, which are  
> otherwise
> overwritten by hid-input setup.

OK, I did not notice that :-) Mmm, I guess my comment about fixing  
hid-input still holds. But we could do it in a later version if time  
is short.

St.

Re: [PATCH 2/3] hid: 3m: Convert to MT slots

[Stéphane Chatty]

Le 28 août 10 à 16:29, Henrik Rydberg a écrit :

>  #define MAX_SLOTS		60
> -#define MAX_TRKID		59
> +#define MAX_TRKID		USHRT_MAX
>

Oops, I forgot this one: in the rest of the code, it seems to me that  
you use your knowledge that TRACKINGID cannot be more than 59 (you  
use it to map the tracking ID onto the slot ID). Therefore, why  
change TRKID to a larger value?

Cheers,

St.

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

Re: [PATCH 2/3] hid: 3m: Convert to MT slots

[Henrik Rydberg]

On 09/20/2010 09:46 PM, Stéphane Chatty wrote:

> 
> Le 28 août 10 à 16:29, Henrik Rydberg a écrit :
> 
>>  #define MAX_SLOTS        60
>> -#define MAX_TRKID        59
>> +#define MAX_TRKID        USHRT_MAX
>>
> 
> Oops, I forgot this one: in the rest of the code, it seems to me that you use
> your knowledge that TRACKINGID cannot be more than 59 (you use it to map the
> tracking ID onto the slot ID). Therefore, why change TRKID to a larger value?


There are only 60 slots available, but an infinite number of tracking ids.
Nowhere is the tracking id assumed to be bounded by anything but MAX_TRKID.

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

Re: [PATCH 2/3] hid: 3m: Convert to MT slots

[Stéphane Chatty]

Le 20 sept. 10 à 21:51, Henrik Rydberg a écrit :

> On 09/20/2010 09:46 PM, Stéphane Chatty wrote:
>
>>
>> Le 28 août 10 à 16:29, Henrik Rydberg a écrit :
>>
>>>  #define MAX_SLOTS        60
>>> -#define MAX_TRKID        59
>>> +#define MAX_TRKID        USHRT_MAX
>>>
>>
>> Oops, I forgot this one: in the rest of the code, it seems to me  
>> that you use
>> your knowledge that TRACKINGID cannot be more than 59 (you use it  
>> to map the
>> tracking ID onto the slot ID). Therefore, why change TRKID to a  
>> larger value?
>
>
> There are only 60 slots available, but an infinite number of  
> tracking ids.
> Nowhere is the tracking id assumed to be bounded by anything but  
> MAX_TRKID.
>

What about this:
+			value = clamp_val(value, 0, MAX_SLOTS - 1);
  			if (md->valid) {
  				md->curid = value;
  				md->f[value].touch = md->touch;

If you had tracking IDs 59, 60 and 61 in the same frame, this would  
not work properly would it?

St.

Re: [PATCH 2/3] hid: 3m: Convert to MT slots

[Henrik Rydberg]

On 09/20/2010 10:01 PM, Stéphane Chatty wrote:

> 
> Le 20 sept. 10 à 21:51, Henrik Rydberg a écrit :
> 
>> On 09/20/2010 09:46 PM, Stéphane Chatty wrote:
>>
>>>
>>> Le 28 août 10 à 16:29, Henrik Rydberg a écrit :
>>>
>>>>  #define MAX_SLOTS        60
>>>> -#define MAX_TRKID        59
>>>> +#define MAX_TRKID        USHRT_MAX
>>>>
>>>
>>> Oops, I forgot this one: in the rest of the code, it seems to me that you use
>>> your knowledge that TRACKINGID cannot be more than 59 (you use it to map the
>>> tracking ID onto the slot ID). Therefore, why change TRKID to a larger value?
>>
>>
>> There are only 60 slots available, but an infinite number of tracking ids.
>> Nowhere is the tracking id assumed to be bounded by anything but MAX_TRKID.
>>
> 
> What about this:
> +            value = clamp_val(value, 0, MAX_SLOTS - 1);
>              if (md->valid) {
>                  md->curid = value;
>                  md->f[value].touch = md->touch;
> 
> If you had tracking IDs 59, 60 and 61 in the same frame, this would not work
> properly would it?


That is the slot id being set. The microtouch controller is also using slots
internally.

Henrik

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

Re: [PATCH 2/3] hid: 3m: Convert to MT slots

[Stéphane Chatty]

Le 20 sept. 10 à 22:04, Henrik Rydberg a écrit :
>>
>> What about this:
>> +            value = clamp_val(value, 0, MAX_SLOTS - 1);
>>              if (md->valid) {
>>                  md->curid = value;
>>                  md->f[value].touch = md->touch;
>>
>> If you had tracking IDs 59, 60 and 61 in the same frame, this  
>> would not work
>> properly would it?
>
>
> That is the slot id being set. The microtouch controller is also  
> using slots
> internally.
>

Oh, right. Once again back to your definition of tracking ID that is  
not so intuitive to me :-) In the end we might end up dealing with  
three IDs: the device's tracking ID, the slot ID, and the Linux  
tracking ID. Here, the first and the second are the same but will  
they always be? I remember when Stantum used to provide us with 16  
bit "device tracking IDs"...

Anyway, I have no more problem with this constant. Just with the  
vocabulary :-)

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

Re: [PATCH 2/3] hid: 3m: Convert to MT slots

[Henrik Rydberg]

On 09/20/2010 10:23 PM, Stéphane Chatty wrote:

> 
> Le 20 sept. 10 à 22:04, Henrik Rydberg a écrit :
>>>
>>> What about this:
>>> +            value = clamp_val(value, 0, MAX_SLOTS - 1);
>>>              if (md->valid) {
>>>                  md->curid = value;
>>>                  md->f[value].touch = md->touch;
>>>
>>> If you had tracking IDs 59, 60 and 61 in the same frame, this would not work
>>> properly would it?
>>
>>
>> That is the slot id being set. The microtouch controller is also using slots
>> internally.
>>
> 
> Oh, right. Once again back to your definition of tracking ID that is not so
> intuitive to me :-) In the end we might end up dealing with three IDs: the
> device's tracking ID, the slot ID, and the Linux tracking ID. Here, the first
> and the second are the same but will they always be? I remember when Stantum
> used to provide us with 16 bit "device tracking IDs"...
> 
> Anyway, I have no more problem with this constant. Just with the vocabulary :-)


In the HID protocol, it is called ContactID... So, Stéphane, are we close to
acking this patch now? :-)

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

Re: [PATCH 2/3] hid: 3m: Convert to MT slots

[Stéphane Chatty]

Le 21 sept. 10 à 19:37, Henrik Rydberg a écrit :

> On 09/20/2010 10:23 PM, Stéphane Chatty wrote:
>
>>
>> Le 20 sept. 10 à 22:04, Henrik Rydberg a écrit :
>>>>
>> Anyway, I have no more problem with this constant. Just with the  
>> vocabulary :-)
>
>
> In the HID protocol, it is called ContactID... So, Stéphane, are we  
> close to
> acking this patch now? :-)

Oh yes of course, I was not clear enough in the above message. To  
make things more formal:

Acked-by: Stéphane Chatty <chaty@enac.fr>


St.

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

Re: [PATCH 2/3] hid: 3m: Convert to MT slots

[Henrik Rydberg]

On 09/21/2010 09:21 PM, Stéphane Chatty wrote:

> 
> Le 21 sept. 10 à 19:37, Henrik Rydberg a écrit :
> 
>> On 09/20/2010 10:23 PM, Stéphane Chatty wrote:
>>
>>>
>>> Le 20 sept. 10 à 22:04, Henrik Rydberg a écrit :
>>>>>
>>> Anyway, I have no more problem with this constant. Just with the vocabulary :-)
>>
>>
>> In the HID protocol, it is called ContactID... So, Stéphane, are we close to
>> acking this patch now? :-)
> 
> Oh yes of course, I was not clear enough in the above message. To make things
> more formal:
> 
> Acked-by: Stéphane Chatty <chaty@enac.fr>


Thanks! I will follow up with the copyright patch right away, then, and we
should be good to go.

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

Re: [PATCH 2/3] hid: 3m: Convert to MT slots

[Jiri Kosina]

On Tue, 21 Sep 2010, Stéphane Chatty wrote:

> > > Anyway, I have no more problem with this constant. Just with the 
> > > vocabulary :-)
> > 
> > In the HID protocol, it is called ContactID... So, Stéphane, are we 
> > close to acking this patch now? :-)
> 
> Oh yes of course, I was not clear enough in the above message. To make things
> more formal:
> 
> Acked-by: Stéphane Chatty <chaty@enac.fr>

Applied, thanks!

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

[PATCH 3/3] hid: 3m: Correct touchscreen emulation

[Henrik Rydberg]

The current code sometimes misses to report the last BTN_TOUCH event
when multiple fingers are lifted simultaneously.  With the
introduction of MT slots, the tracking id is available to determine
the oldest active contact. Use this information to simplify and
correct the touchscreen emulation logic.

Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
 drivers/hid/hid-3m-pct.c |   41 +++++------------------------------------
 1 files changed, 5 insertions(+), 36 deletions(-)

diff --git a/drivers/hid/hid-3m-pct.c b/drivers/hid/hid-3m-pct.c
index 9a3b047..65441e0 100644
--- a/drivers/hid/hid-3m-pct.c
+++ b/drivers/hid/hid-3m-pct.c
@@ -33,7 +33,6 @@ MODULE_LICENSE("GPL");
 struct mmm_finger {
 	__s32 x, y, w, h;
 	__u16 id;
-	__u8 rank;
 	bool prev_touch;
 	bool touch, valid;
 };
@@ -41,7 +40,7 @@ struct mmm_finger {
 struct mmm_data {
 	struct mmm_finger f[MAX_SLOTS];
 	__u16 id;
-	__u8 curid, num;
+	__u8 curid;
 	__u8 nexp, nreal;
 	bool touch, valid;
 };
@@ -152,13 +151,7 @@ static int mmm_input_mapped(struct hid_device *hdev, struct hid_input *hi,
 static void mmm_filter_event(struct mmm_data *md, struct input_dev *input)
 {
 	struct mmm_finger *oldest = 0;
-	bool pressed = false, released = false;
 	int i;
-
-	/*
-	 * we need to iterate on all fingers to decide if we have a press
-	 * or a release event in our touchscreen emulation.
-	 */
 	for (i = 0; i < MAX_SLOTS; ++i) {
 		struct mmm_finger *f = &md->f[i];
 		if (!f->valid) {
@@ -179,34 +172,11 @@ static void mmm_filter_event(struct mmm_data *md, struct input_dev *input)
 						wide ? f->w : f->h);
 			input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR,
 						wide ? f->h : f->w);
-			/*
-			 * touchscreen emulation: maintain the age rank
-			 * of this finger, decide if we have a press
-			 */
-			if (f->rank == 0) {
-				f->rank = ++(md->num);
-				if (f->rank == 1)
-					pressed = true;
-			}
-			if (f->rank == 1)
+			/* touchscreen emulation: pick the oldest contact */
+			if (!oldest || ((f->id - oldest->id) & (SHRT_MAX + 1)))
 				oldest = f;
 		} else {
 			/* this finger took off the screen */
-			/* touchscreen emulation: maintain age rank of others */
-			int j;
-
-			for (j = 0; j < 10; ++j) {
-				struct mmm_finger *g = &md->f[j];
-				if (g->rank > f->rank) {
-					g->rank--;
-					if (g->rank == 1)
-						oldest = g;
-				}
-			}
-			f->rank = 0;
-			--(md->num);
-			if (md->num == 0)
-				released = true;
 			input_event(input, EV_ABS, ABS_MT_TRACKING_ID, -1);
 		}
 		f->prev_touch = f->touch;
@@ -215,11 +185,10 @@ static void mmm_filter_event(struct mmm_data *md, struct input_dev *input)
 
 	/* touchscreen emulation */
 	if (oldest) {
-		if (pressed)
-			input_event(input, EV_KEY, BTN_TOUCH, 1);
+		input_event(input, EV_KEY, BTN_TOUCH, 1);
 		input_event(input, EV_ABS, ABS_X, oldest->x);
 		input_event(input, EV_ABS, ABS_Y, oldest->y);
-	} else if (released) {
+	} else {
 		input_event(input, EV_KEY, BTN_TOUCH, 0);
 	}
 	input_sync(input);
-- 
1.7.1

Re: [PATCH 3/3] hid: 3m: Correct touchscreen emulation

[Stéphane Chatty]

Le 28 août 10 à 16:29, Henrik Rydberg a écrit :

> The current code sometimes misses to report the last BTN_TOUCH event
> when multiple fingers are lifted simultaneously.  With the
> introduction of MT slots, the tracking id is available to determine
> the oldest active contact.

Hi Henrik,

good job on exploiting the sync quirk!

About the oldest active contact, I'm not so sure though. I don't have  
the device with me but I remember that 3M recycle IDs, giving the  
lowest available ID to any new finger; this means that the lowest ID  
is not always the oldest finger.

Cheers,

St.

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

Re: [PATCH 3/3] hid: 3m: Correct touchscreen emulation

[Henrik Rydberg]

On 08/28/2010 05:56 PM, Stéphane Chatty wrote:

> 
> Le 28 août 10 à 16:29, Henrik Rydberg a écrit :
> 
>> The current code sometimes misses to report the last BTN_TOUCH event
>> when multiple fingers are lifted simultaneously.  With the
>> introduction of MT slots, the tracking id is available to determine
>> the oldest active contact.
> 
> Hi Henrik,
> 
> good job on exploiting the sync quirk!
> 
> About the oldest active contact, I'm not so sure though. I don't have the device
> with me but I remember that 3M recycle IDs, giving the lowest available ID to
> any new finger; this means that the lowest ID is not always the oldest finger.


Hi Stéphane,

I think we use the term tracking id differently here. The HID_DG_CONTACTID is
actually the slot id, and the tracking id is a sequential number, increased
whenever the touch is new. I believe the patchset works correctly as long as
there is less than 32768 new touches before releasing the oldest one. :-)

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

Re: [PATCH 3/3] hid: 3m: Correct touchscreen emulation

[Stéphane Chatty]

Le 28 août 10 à 18:08, Henrik Rydberg a écrit :

>
> I think we use the term tracking id differently here. The  
> HID_DG_CONTACTID is
> actually the slot id, and the tracking id is a sequential number,  
> increased
> whenever the touch is new. I believe the patchset works correctly  
> as long as
> there is less than 32768 new touches before releasing the oldest  
> one. :-)

Oh, right. I definitely need to catch up with Protocol B.

Cheers,

St.

Re: [PATCH 3/3] hid: 3m: Correct touchscreen emulation

[Jiri Kosina]

On Sat, 28 Aug 2010, Stéphane Chatty wrote:

> > I think we use the term tracking id differently here. The HID_DG_CONTACTID
> > is
> > actually the slot id, and the tracking id is a sequential number, increased
> > whenever the touch is new. I believe the patchset works correctly as long as
> > there is less than 32768 new touches before releasing the oldest one. :-)
> 
> Oh, right. I definitely need to catch up with Protocol B.

Henrik,

thanks a lot for the patches.

Stephane, could you please send me your Acked-by for those changes? I'd 
like to have this before applying them.

Thanks,

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH 3/3] hid: 3m: Correct touchscreen emulation

[Stéphane Chatty]

Le 28 août 10 à 16:29, Henrik Rydberg a écrit :

> The current code sometimes misses to report the last BTN_TOUCH event
> when multiple fingers are lifted simultaneously.  With the
> introduction of MT slots, the tracking id is available to determine
> the oldest active contact. Use this information to simplify and
> correct the touchscreen emulation logic.
>

Acked-by: Stéphane Chatty <chatty@enac.fr>

Cheers,

St.

Re: [PATCH 3/3] hid: 3m: Correct touchscreen emulation

[Jiri Kosina]

On Tue, 21 Sep 2010, Stéphane Chatty wrote:

> > The current code sometimes misses to report the last BTN_TOUCH event
> > when multiple fingers are lifted simultaneously.  With the
> > introduction of MT slots, the tracking id is available to determine
> > the oldest active contact. Use this information to simplify and
> > correct the touchscreen emulation logic.
> > 
> 
> Acked-by: Stéphane Chatty <chatty@enac.fr>

It doesn't apply without 2/3 being applied as well. So I'll not be 
applying it for now, and either please rebase it, or re-send together with 
2/3 once the situation with 0eef:72xx is sorted out. Ok?

Thanks,

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

Re: [PATCH 3/3] hid: 3m: Correct touchscreen emulation

[Stéphane Chatty]

Le 21 sept. 10 à 19:05, Jiri Kosina a écrit :

> On Tue, 21 Sep 2010, Stéphane Chatty wrote:
>
>>> The current code sometimes misses to report the last BTN_TOUCH event
>>> when multiple fingers are lifted simultaneously.  With the
>>> introduction of MT slots, the tracking id is available to determine
>>> the oldest active contact. Use this information to simplify and
>>> correct the touchscreen emulation logic.
>>>
>>
>> Acked-by: Stéphane Chatty <chatty@enac.fr>
>
> It doesn't apply without 2/3 being applied as well. So I'll not be
> applying it for now, and either please rebase it, or re-send  
> together with
> 2/3 once the situation with 0eef:72xx is sorted out. Ok?

Er, there must be some confusion here: 0eef:72xx is about hid-egalax,  
in a different thread.

Cheers,

St.

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

Re: [PATCH 3/3] hid: 3m: Correct touchscreen emulation

[Jiri Kosina]

On Tue, 21 Sep 2010, Stéphane Chatty wrote:

> > > > when multiple fingers are lifted simultaneously.  With the
> > > > introduction of MT slots, the tracking id is available to determine
> > > > the oldest active contact. Use this information to simplify and
> > > > correct the touchscreen emulation logic.
> > > > 
> > > 
> > > Acked-by: Stéphane Chatty <chatty@enac.fr>
> > 
> > It doesn't apply without 2/3 being applied as well. So I'll not be
> > applying it for now, and either please rebase it, or re-send together with
> > 2/3 once the situation with 0eef:72xx is sorted out. Ok?
> 
> Er, there must be some confusion here: 0eef:72xx is about hid-egalax, in 
> a different thread.

Gah, sorry, have been handling too many things in parallel today. Of 
course you are right.

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

Re: [PATCH 1/3] hid: 3m: Adjust to sequential MT HID protocol

[Stéphane Chatty]

Le 28 août 10 à 16:29, Henrik Rydberg a écrit :

> The multitouch extensions to the HID protocol allows for contact
> data to be sent over several reports, which is also the case for
> the 3M M2256PW touchscreen. This patch modifies the logic to only
> synchronize the input layer when all contacts have been received.
> Consequentially, the full 60-finger capacity of the device is enabled.
>
> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
> ---
>  drivers/hid/hid-3m-pct.c |   22 ++++++++++++++++++----
>  1 files changed, 18 insertions(+), 4 deletions(-)

You can add

Acked-by: Stephane Chatty <chatty@enac.fr>

St.

Re: [PATCH 1/3] hid: 3m: Adjust to sequential MT HID protocol

[Jiri Kosina]

On Mon, 20 Sep 2010, Stéphane Chatty wrote:

> > The multitouch extensions to the HID protocol allows for contact
> > data to be sent over several reports, which is also the case for
> > the 3M M2256PW touchscreen. This patch modifies the logic to only
> > synchronize the input layer when all contacts have been received.
> > Consequentially, the full 60-finger capacity of the device is enabled.
> > 
> > Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
> > ---
> >  drivers/hid/hid-3m-pct.c |   22 ++++++++++++++++++----
> >  1 files changed, 18 insertions(+), 4 deletions(-)
> 
> You can add
> 
> Acked-by: Stephane Chatty <chatty@enac.fr>

Applied, thanks guys.

-- 
Jiri Kosina
SUSE Labs, Novell Inc.