From: Benjamin Tissoires <hidden> Date: 2013-01-25 13:23:32
Hi guys,
finally, I managed to send a new bunch of patches. Sorry for the delay from the
previous version, but meanwhile, I implemented an automatic regressions tests
for hid device [1].
So this series seems pretty big, but it does not break any known devices (I ran
40 successful tests for this series)[2].
To sum up:
- Nexio devices were problematic in the sense they use out of range values for
some of the fields, and consider that the driver won't treat the extra touches
based on the reported contact count.
- fortunately, this behavior (relying on contact count) is compatible with all
the devices I know, which leads to think that this is how the Windows 7/8 driver
manage to handle such a different bunch of devices.
- thanks to the automatic testing, I was able to fix broken devices
(Sharp LC-20FE1-W screen 04dd:9681, Sitronix 1403:5001 and Cando 2087:0a02)
and optimize many others. In order to allow a bisection to be done, I split
the patches in many different ones, one per device type.
- finally, I changed the default class in order to handle the new devices in a
better way.
Cheers,
Benjamin
[1] https://github.com/bentiss/hid-test
[2] https://github.com/bentiss/hid-devices
Benjamin Tissoires (25):
HID: break out hid_extract_field
HID: multitouch: add support for Nexio 42" panel
HID: multitouch: fix Win8 protocol for Sharp like devices
HID: multitouch: ensure that serial devices make no use of contact count
HID: multitouch: fix protocol for Sitronix 1403:5001
HID: multitouch: optimize FlatFrog panels
HID: multitouch: optimize 3M panels
HID: multitouch: optimize Cypress panels
HID: multitouch: optimize eGalax panels
HID: multitouch: optimize Stantum panels
HID: multitouch: optimize Quanta panels
HID: multitouch: optimize Lumio panels
HID: multitouch: optimize MosArt panels
HID: multitouch: optimize Elo panels
HID: multitouch: optimize Hanvon panels
HID: multitouch: optimize IRTouch panels
HID: multitouch: fix protocol for Cando 2087:0a02
HID: multitouch: optimize Cando panels
HID: multitouch: optimize ActionStar panels
HID: multitouch: optimize Atmel panels
HID: multitouch: optimize Ideacom panels
HID: multitouch: optimize LG panels
HID: multitouch: optimize Nexio panels
HID: multitouch: remove useless DUAL_NSMU_CONTACTID class
HID: multitouch: make MT_CLS_ALWAYS_TRUE the new default class
drivers/hid/hid-core.c | 60 +++++++++----
drivers/hid/hid-ids.h | 3 +
drivers/hid/hid-multitouch.c | 206 ++++++++++++++++++++++++++++++-------------
include/linux/hid.h | 1 +
4 files changed, 189 insertions(+), 81 deletions(-)
--
1.8.1
From: Benjamin Tissoires <hidden> Date: 2013-01-25 13:23:37
Breaking out this function allows third parties hid drivers to retrieve
the data fields of a hid report in the raw_event callback.
No functional changes, only exports the function.
Signed-off-by: Benjamin Tissoires <redacted>
---
drivers/hid/hid-core.c | 60 ++++++++++++++++++++++++++++++++++----------------
include/linux/hid.h | 1 +
2 files changed, 42 insertions(+), 19 deletions(-)
From: Benjamin Tissoires <hidden> Date: 2013-01-25 13:23:41
This device is the worst device I saw. It keeps TipSwitch and InRange
at 1 for fingers that are not touching the panel.
The solution is to rely on the field ContactCount, which is accurate
as the correct information are packed at the begining of the frame.
Unfortunately, CountactCount is most of the time at the end of the report.
The solution is to pick it when we have the whole report in raw_event.
Signed-off-by: Benjamin Tissoires <redacted>
---
drivers/hid/hid-ids.h | 3 ++
drivers/hid/hid-multitouch.c | 91 ++++++++++++++++++++++++++++++++++++--------
2 files changed, 78 insertions(+), 16 deletions(-)
@@ -83,6 +86,10 @@ struct mt_device {structmt_classmtclass;/* our mt device class */structmt_fields*fields;/* temporary placeholder for storing themultitouchfields*/+structhid_field*contactcount;/* the hid_field contact count that+willbepickedinmt_raw_event*/+__s8contactcount_index;/* the index of the usage contact count+initshid_field.*/unsignedlast_field_index;/* last field index of the report */unsignedlast_slot_field;/* the last field of a slot */__s8inputmode;/* InputMode HID feature, -1 if non-existent */
@@ -264,24 +278,26 @@ static struct attribute_group mt_attribute_group = {.attrs=sysfs_attrs};+staticintmt_find_usage_index(structhid_field*field,structhid_usage*usage)+{+inti;+for(i=0;i<field->maxusage;i++){+if(field->usage[i].hid==usage->hid)+returni;+}+return-1;+}+staticvoidmt_feature_mapping(structhid_device*hdev,structhid_field*field,structhid_usage*usage){structmt_device*td=hid_get_drvdata(hdev);-inti;switch(usage->hid){caseHID_DG_INPUTMODE:td->inputmode=field->report->id;-td->inputmode_index=0;/* has to be updated below */--for(i=0;i<field->maxusage;i++){-if(field->usage[i].hid==usage->hid){-td->inputmode_index=i;-break;-}-}-+td->inputmode_index=mt_find_usage_index(field,usage);+/* inputmode_index can't be set at -1 */break;caseHID_DG_CONTACTMAX:td->maxcontact_report_id=field->report->id;
@@ -459,6 +475,10 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,td->last_field_index=field->index;return1;caseHID_DG_CONTACTCOUNT:+td->contactcount=field;+td->contactcount_index=mt_find_usage_index(field,+usage);+/* contactcount_index can't be set at -1 */td->last_field_index=field->index;return1;caseHID_DG_CONTACTMAX:
From: Benjamin Tissoires <hidden> Date: 2013-01-25 13:23:44
The serial protocol makes contact count a redondant information, and
sometimes it is not reliable (TRS-Star are in this case).
Disabling the use of contact count for these devices is thus safer.
Signed-off-by: Benjamin Tissoires <redacted>
---
drivers/hid/hid-multitouch.c | 1 +
1 file changed, 1 insertion(+)
From: Benjamin Tissoires <hidden> Date: 2013-01-25 13:23:46
Since the inclusion of this device in hid-multitouch, the device
did not forward any events. Using the serial class makes it working
again.
Signed-off-by: Benjamin Tissoires <redacted>
---
drivers/hid/hid-multitouch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Benjamin Tissoires <hidden> Date: 2013-01-25 13:23:54
Relying on ALWAYS_VALID enhance a little the processing time of
the events.
Signed-off-by: Benjamin Tissoires <redacted>
---
drivers/hid/hid-multitouch.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
From: Benjamin Tissoires <hidden> Date: 2013-01-25 13:23:58
Relying on ALWAYS_VALID enhance a little the processing time of
the events. So using MT_CLS_EGALAX_SERIAL is better than using
MT_CLS_EGALAX.
Signed-off-by: Benjamin Tissoires <redacted>
---
drivers/hid/hid-multitouch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Benjamin Tissoires <hidden> Date: 2013-01-25 13:24:03
Not very different in terms of processing time. However, this
uniformizes the handling of devices.
Signed-off-by: Benjamin Tissoires <redacted>
---
drivers/hid/hid-multitouch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Benjamin Tissoires <hidden> Date: 2013-01-25 13:24:06
Not very different in terms of processing time. However, this
uniformizes the handling of devices.
Signed-off-by: Benjamin Tissoires <redacted>
---
drivers/hid/hid-multitouch.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
From: Benjamin Tissoires <hidden> Date: 2013-01-25 13:24:13
Cando 2087:0a02 was broken, this fixes it.
Signed-off-by: Benjamin Tissoires <redacted>
---
drivers/hid/hid-multitouch.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
From: Benjamin Tissoires <hidden> Date: 2013-01-25 13:24:22
The slot can be extracted using MINUS_ONE.
Signed-off-by: Benjamin Tissoires <redacted>
---
drivers/hid/hid-multitouch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Benjamin Tissoires <hidden> Date: 2013-01-25 13:24:25
The slot can be extracted by using MINUS_ONE.
Signed-off-by: Benjamin Tissoires <redacted>
---
drivers/hid/hid-multitouch.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
From: Benjamin Tissoires <hidden> Date: 2013-01-25 13:24:31
By running a test on all the traces of the devices I have,
I noticed that the class MT_CLS_ALWAYS_TRUE could handle all
the devices I've seen so far without any other quirks.
I guess this is the behavior Win 7 requires in its driver.
We can change the default class then and keep the existing classes
for backward compatibility and performances for some of them.
Two operations have been done:
- replaced MT_CLS_DEFAULT by MT_CLS_NSMU
- then replaced MT_CLS_ALWAYS_TRUE by MT_CLS_DEFAULT
Signed-off-by: Benjamin Tissoires <redacted>
---
drivers/hid/hid-multitouch.c | 53 ++++++++++++++++++++++----------------------
1 file changed, 27 insertions(+), 26 deletions(-)
From: Benjamin Tissoires <hidden> Date: 2013-01-25 13:25:15
The slot can be extracted using by MINUS_ONE.
Signed-off-by: Benjamin Tissoires <redacted>
---
drivers/hid/hid-multitouch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Benjamin Tissoires <hidden> Date: 2013-01-25 13:25:36
Relying on ALWAYS_VALID enhance a little the processing time of
the events.
Also, the slot can be extracted by using the contact ID.
Signed-off-by: Benjamin Tissoires <redacted>
---
drivers/hid/hid-multitouch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Benjamin Tissoires <hidden> Date: 2013-01-25 13:26:12
Relying on ALWAYS_VALID enhance a little the processing time of
the events.
Also the slot can be extracted faster by using MINUS_ONE.
Signed-off-by: Benjamin Tissoires <redacted>
---
drivers/hid/hid-multitouch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Benjamin Tissoires <hidden> Date: 2013-01-25 13:26:33
Not very different in terms of processing time. However, this
uniformizes the handling of devices.
Signed-off-by: Benjamin Tissoires <redacted>
---
drivers/hid/hid-multitouch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Benjamin Tissoires <hidden> Date: 2013-01-25 13:26:36
Not very different in terms of processing time. However, this
uniformizes the handling of devices.
Signed-off-by: Benjamin Tissoires <redacted>
---
drivers/hid/hid-multitouch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Benjamin Tissoires <hidden> Date: 2013-01-25 13:27:03
Not very different in terms of processing time. However, this
uniformizes the handling of devices.
Signed-off-by: Benjamin Tissoires <redacted>
---
drivers/hid/hid-multitouch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Benjamin Tissoires <hidden> Date: 2013-01-25 13:27:20
Relying on ALWAYS_VALID enhance a little the processing time of
the events.
Signed-off-by: Benjamin Tissoires <redacted>
---
drivers/hid/hid-multitouch.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
From: Benjamin Tissoires <hidden> Date: 2013-01-25 13:27:41
Not very different in terms of processing time. However, this
uniformizes the handling of devices.
Signed-off-by: Benjamin Tissoires <redacted>
---
drivers/hid/hid-multitouch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Benjamin Tissoires <hidden> Date: 2013-01-25 13:28:03
Not very different in terms of processing time. However, this
uniformizes the handling of devices.
Signed-off-by: Benjamin Tissoires <redacted>
---
drivers/hid/hid-multitouch.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
From: Benjamin Tissoires <hidden> Date: 2013-01-25 13:28:35
Not very different in terms of processing time. However, this
uniformizes the handling of devices.
Signed-off-by: Benjamin Tissoires <redacted>
---
drivers/hid/hid-multitouch.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
From: Benjamin Tissoires <hidden> Date: 2013-01-25 13:29:02
Relying on ALWAYS_VALID enhance a little the processing time of
the events.
Signed-off-by: Benjamin Tissoires <redacted>
---
drivers/hid/hid-multitouch.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
From: Benjamin Tissoires <hidden> Date: 2013-01-25 13:29:38
The Sharp LC-20FE1-W screen (04dd:9681) behaves like the Nexio 42".
It may report out of ranges values that are filtered out by relying
on the Contact Count HID field.
Adding the quirk MT_QUIRK_CONTACT_CNT_ACCURATE makes hid-multitouch
strongest against this kind of device, without breaking the current
devices.
Signed-off-by: Benjamin Tissoires <redacted>
---
drivers/hid/hid-multitouch.c | 1 +
1 file changed, 1 insertion(+)
From: Henrik Rydberg <hidden> Date: 2013-01-28 14:56:19
Hi Benjamin,
This device is the worst device I saw. It keeps TipSwitch and InRange
at 1 for fingers that are not touching the panel.
The solution is to rely on the field ContactCount, which is accurate
as the correct information are packed at the begining of the frame.
Unfortunately, CountactCount is most of the time at the end of the report.
The solution is to pick it when we have the whole report in raw_event.
Signed-off-by: Benjamin Tissoires <redacted>
---
drivers/hid/hid-ids.h | 3 ++
drivers/hid/hid-multitouch.c | 91 ++++++++++++++++++++++++++++++++++++--------
2 files changed, 78 insertions(+), 16 deletions(-)
I think it would make more sense to introduce a method where the
driver sees all report values at once. We have had reasonable cause to
add it in the past, but never did.
@@ -83,6 +86,10 @@ struct mt_device {structmt_classmtclass;/* our mt device class */structmt_fields*fields;/* temporary placeholder for storing themultitouchfields*/+structhid_field*contactcount;/* the hid_field contact count that+willbepickedinmt_raw_event*/+__s8contactcount_index;/* the index of the usage contact count+initshid_field.*/unsignedlast_field_index;/* last field index of the report */unsignedlast_slot_field;/* the last field of a slot */__s8inputmode;/* InputMode HID feature, -1 if non-existent */
@@ -264,24 +278,26 @@ static struct attribute_group mt_attribute_group = {.attrs=sysfs_attrs};+staticintmt_find_usage_index(structhid_field*field,structhid_usage*usage)+{+inti;+for(i=0;i<field->maxusage;i++){+if(field->usage[i].hid==usage->hid)+returni;+}+return-1;+}+staticvoidmt_feature_mapping(structhid_device*hdev,structhid_field*field,structhid_usage*usage){structmt_device*td=hid_get_drvdata(hdev);-inti;switch(usage->hid){caseHID_DG_INPUTMODE:td->inputmode=field->report->id;-td->inputmode_index=0;/* has to be updated below */--for(i=0;i<field->maxusage;i++){-if(field->usage[i].hid==usage->hid){-td->inputmode_index=i;-break;-}-}-+td->inputmode_index=mt_find_usage_index(field,usage);+/* inputmode_index can't be set at -1 */break;caseHID_DG_CONTACTMAX:td->maxcontact_report_id=field->report->id;
@@ -459,6 +475,10 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,td->last_field_index=field->index;return1;caseHID_DG_CONTACTCOUNT:+td->contactcount=field;+td->contactcount_index=mt_find_usage_index(field,+usage);+/* contactcount_index can't be set at -1 */td->last_field_index=field->index;return1;caseHID_DG_CONTACTMAX:
From: Henrik Rydberg <hidden> Date: 2013-01-28 14:59:20
Hi Benjamin,
quoted hunk
Breaking out this function allows third parties hid drivers to retrieve
the data fields of a hid report in the raw_event callback.
No functional changes, only exports the function.
Signed-off-by: Benjamin Tissoires <redacted>
---
drivers/hid/hid-core.c | 60 ++++++++++++++++++++++++++++++++++----------------
include/linux/hid.h | 1 +
2 files changed, 42 insertions(+), 19 deletions(-)
From: Henrik Rydberg <hidden> Date: 2013-01-28 15:05:00
Hi Benjamin,
Relying on ALWAYS_VALID enhance a little the processing time of
the events.
Signed-off-by: Benjamin Tissoires <redacted>
---
drivers/hid/hid-multitouch.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
Is there a measurement backing up this claim? I suspect
micro-optimizations of this kind drowns completely along the rather
long way from device, via malloc and loops, to the input core.
Considering the must-be-tested-for-regression risk on top of that,
this patch falls far below what is worth doing, IMHO.
From: Henrik Rydberg <hidden> Date: 2013-01-28 15:08:47
Hi Benjamin,
By running a test on all the traces of the devices I have,
I noticed that the class MT_CLS_ALWAYS_TRUE could handle all
the devices I've seen so far without any other quirks.
I guess this is the behavior Win 7 requires in its driver.
We can change the default class then and keep the existing classes
for backward compatibility and performances for some of them.
Nice observation. A simpler route is to add new devices according the
new scheme, and leave the current code intact. Don't fix what ain't
broken.
quoted hunk
Two operations have been done:
- replaced MT_CLS_DEFAULT by MT_CLS_NSMU
- then replaced MT_CLS_ALWAYS_TRUE by MT_CLS_DEFAULT
Signed-off-by: Benjamin Tissoires <redacted>
---
drivers/hid/hid-multitouch.c | 53 ++++++++++++++++++++++----------------------
1 file changed, 27 insertions(+), 26 deletions(-)
From: Henrik Rydberg <hidden> Date: 2013-01-28 15:42:30
Hi Benjamin,
finally, I managed to send a new bunch of patches. Sorry for the delay from the
previous version, but meanwhile, I implemented an automatic regressions tests
for hid device [1].
So this series seems pretty big, but it does not break any known devices (I ran
40 successful tests for this series)[2].
Thanks for the patches.
To sum up:
- Nexio devices were problematic in the sense they use out of range values for
some of the fields, and consider that the driver won't treat the extra touches
based on the reported contact count.
Problematic device, but I think we should add a new event function
which gives all values at the same time, since those are already
present in the core. It seems this will solve the current problem as
well as many older workarounds.
- fortunately, this behavior (relying on contact count) is compatible with all
the devices I know, which leads to think that this is how the Windows 7/8 driver
manage to handle such a different bunch of devices.
This is a nice observation. IIRC, we used to rely more on contact
count in the old drivers.
- thanks to the automatic testing, I was able to fix broken devices
(Sharp LC-20FE1-W screen 04dd:9681, Sitronix 1403:5001 and Cando 2087:0a02)
and optimize many others. In order to allow a bisection to be done, I split
the patches in many different ones, one per device type.
Great tool, thank you Benjamin.
- finally, I changed the default class in order to handle the new devices in a
better way.
From: Benjamin Tissoires <hidden> Date: 2013-01-28 15:54:04
On Mon, Jan 28, 2013 at 4:13 PM, Henrik Rydberg [off-list ref] wrote:
Hi Benjamin,
quoted
By running a test on all the traces of the devices I have,
I noticed that the class MT_CLS_ALWAYS_TRUE could handle all
the devices I've seen so far without any other quirks.
I guess this is the behavior Win 7 requires in its driver.
We can change the default class then and keep the existing classes
for backward compatibility and performances for some of them.
Nice observation. A simpler route is to add new devices according the
new scheme, and leave the current code intact. Don't fix what ain't
broken.
I fully agree with that, and this is the way I built this patch. All
the current working devices supported in the kernel through the 'old'
default class (that I was not able to test) are keeping the very same
class. This class has just been renamed "NSMU".
The new devices will get the new default class.
Maybe that if I do the change with 2 different patches, it will be
more readable and error-proof?
Cheers,
Benjamin
quoted
Two operations have been done:
- replaced MT_CLS_DEFAULT by MT_CLS_NSMU
- then replaced MT_CLS_ALWAYS_TRUE by MT_CLS_DEFAULT
Signed-off-by: Benjamin Tissoires <redacted>
---
drivers/hid/hid-multitouch.c | 53 ++++++++++++++++++++++----------------------
1 file changed, 27 insertions(+), 26 deletions(-)
From: Benjamin Tissoires <hidden> Date: 2013-01-28 16:01:50
On Mon, Jan 28, 2013 at 4:10 PM, Henrik Rydberg [off-list ref] wrote:
Hi Benjamin,
quoted
Relying on ALWAYS_VALID enhance a little the processing time of
the events.
Signed-off-by: Benjamin Tissoires <redacted>
---
drivers/hid/hid-multitouch.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
Is there a measurement backing up this claim? I suspect
micro-optimizations of this kind drowns completely along the rather
long way from device, via malloc and loops, to the input core.
No, the measure I made for this are roughly the same. It's purely
micro-optimization, and yes, mallocs must kill the processing time.
However, the purpose of this patch and the others entitled 'optimize
XXX' is to show that I did not invented the new default class. My true
objective is to remove all the quirks related to the 'valid'
information in order to have a driver as universal as possible and to
be able to clean it up.
Considering the must-be-tested-for-regression risk on top of that,
this patch falls far below what is worth doing, IMHO.
It _has_ been tested. I got the HID traces from FlatFrog and either
they use different firmwares with different processing in their
devices, the kernel processing is good.
Cheers,
Benjamin
From: Benjamin Tissoires <hidden> Date: 2013-01-28 16:08:52
On Mon, Jan 28, 2013 at 4:01 PM, Henrik Rydberg [off-list ref] wrote:
Hi Benjamin,
quoted
This device is the worst device I saw. It keeps TipSwitch and InRange
at 1 for fingers that are not touching the panel.
The solution is to rely on the field ContactCount, which is accurate
as the correct information are packed at the begining of the frame.
Unfortunately, CountactCount is most of the time at the end of the report.
The solution is to pick it when we have the whole report in raw_event.
Signed-off-by: Benjamin Tissoires <redacted>
---
drivers/hid/hid-ids.h | 3 ++
drivers/hid/hid-multitouch.c | 91 ++++++++++++++++++++++++++++++++++++--------
2 files changed, 78 insertions(+), 16 deletions(-)
I think it would make more sense to introduce a method where the
driver sees all report values at once. We have had reasonable cause to
add it in the past, but never did.
I will definitively look into it. It seems a fair idea.
@@ -83,6 +86,10 @@ struct mt_device {structmt_classmtclass;/* our mt device class */structmt_fields*fields;/* temporary placeholder for storing themultitouchfields*/+structhid_field*contactcount;/* the hid_field contact count that+willbepickedinmt_raw_event*/+__s8contactcount_index;/* the index of the usage contact count+initshid_field.*/unsignedlast_field_index;/* last field index of the report */unsignedlast_slot_field;/* the last field of a slot */__s8inputmode;/* InputMode HID feature, -1 if non-existent */
@@ -264,24 +278,26 @@ static struct attribute_group mt_attribute_group = {.attrs=sysfs_attrs};+staticintmt_find_usage_index(structhid_field*field,structhid_usage*usage)+{+inti;+for(i=0;i<field->maxusage;i++){+if(field->usage[i].hid==usage->hid)+returni;+}+return-1;+}+staticvoidmt_feature_mapping(structhid_device*hdev,structhid_field*field,structhid_usage*usage){structmt_device*td=hid_get_drvdata(hdev);-inti;switch(usage->hid){caseHID_DG_INPUTMODE:td->inputmode=field->report->id;-td->inputmode_index=0;/* has to be updated below */--for(i=0;i<field->maxusage;i++){-if(field->usage[i].hid==usage->hid){-td->inputmode_index=i;-break;-}-}-+td->inputmode_index=mt_find_usage_index(field,usage);+/* inputmode_index can't be set at -1 */break;caseHID_DG_CONTACTMAX:td->maxcontact_report_id=field->report->id;
@@ -459,6 +475,10 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,td->last_field_index=field->index;return1;caseHID_DG_CONTACTCOUNT:+td->contactcount=field;+td->contactcount_index=mt_find_usage_index(field,+usage);+/* contactcount_index can't be set at -1 */td->last_field_index=field->index;return1;caseHID_DG_CONTACTMAX:
This is a lot of cycles for something that is already available in the core.
Not that much: raw_event is called once per report (unlike ->event
which is called one per field per report).
So there is only one extra malloc if the quirk is in place compared to
the current implementation.
Cheers,
Benjamin
From: Benjamin Tissoires <hidden> Date: 2013-01-28 16:15:23
Hi Henrik,
On Mon, Jan 28, 2013 at 4:23 PM, Henrik Rydberg [off-list ref] wrote:
Hi Benjamin,
quoted
finally, I managed to send a new bunch of patches. Sorry for the delay from the
previous version, but meanwhile, I implemented an automatic regressions tests
for hid device [1].
So this series seems pretty big, but it does not break any known devices (I ran
40 successful tests for this series)[2].
Thanks for the patches.
And thanks for reviewing.
quoted
To sum up:
- Nexio devices were problematic in the sense they use out of range values for
some of the fields, and consider that the driver won't treat the extra touches
based on the reported contact count.
Problematic device, but I think we should add a new event function
which gives all values at the same time, since those are already
present in the core. It seems this will solve the current problem as
well as many older workarounds.
yeah, makes sense. I think it would also allows us to simplify the
logic of hid-multitouch by removing some of the states we have in it.
quoted
- fortunately, this behavior (relying on contact count) is compatible with all
the devices I know, which leads to think that this is how the Windows 7/8 driver
manage to handle such a different bunch of devices.
This is a nice observation. IIRC, we used to rely more on contact
count in the old drivers.
quoted
- thanks to the automatic testing, I was able to fix broken devices
(Sharp LC-20FE1-W screen 04dd:9681, Sitronix 1403:5001 and Cando 2087:0a02)
and optimize many others. In order to allow a bisection to be done, I split
the patches in many different ones, one per device type.
Great tool, thank you Benjamin.
Once I will do the work on the suppression of usbhid direct use, we
also could rely on that for every HID devices, not only
hid-multitouch.
quoted
- finally, I changed the default class in order to handle the new devices in a
better way.
Old wisdom says differently. ;-)
No: old wisdom says the exact same thing :) I did not break the
current supported devices. I _kept_ the old default class for all the
current supported devices, and I used the new default class only for
the new devices, the one that are not registered.
Cheers,
Benjamin
Le 28 janv. 2013 à 16:01, Henrik Rydberg a écrit :
Hi Benjamin,
quoted
This device is the worst device I saw. It keeps TipSwitch and InRange
at 1 for fingers that are not touching the panel.
The solution is to rely on the field ContactCount, which is accurate
as the correct information are packed at the begining of the frame.
Unfortunately, CountactCount is most of the time at the end of the report.
The solution is to pick it when we have the whole report in raw_event.
Signed-off-by: Benjamin Tissoires <redacted>
---
drivers/hid/hid-ids.h | 3 ++
drivers/hid/hid-multitouch.c | 91 ++++++++++++++++++++++++++++++++++++--------
2 files changed, 78 insertions(+), 16 deletions(-)
I think it would make more sense to introduce a method where the
driver sees all report values at once. We have had reasonable cause to
add it in the past, but never did.
I dreamed of this more than once in the early days of hid-multitouch, yes. But then it requires quite a few changes in hid-core.c, doesn't it?
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
From: Benjamin Tissoires <hidden> Date: 2013-01-28 17:50:59
On 01/28/2013 05:56 PM, Stéphane Chatty wrote:
Le 28 janv. 2013 à 16:01, Henrik Rydberg a écrit :
quoted
Hi Benjamin,
quoted
This device is the worst device I saw. It keeps TipSwitch and InRange
at 1 for fingers that are not touching the panel.
The solution is to rely on the field ContactCount, which is accurate
as the correct information are packed at the begining of the frame.
Unfortunately, CountactCount is most of the time at the end of the report.
The solution is to pick it when we have the whole report in raw_event.
Signed-off-by: Benjamin Tissoires <redacted>
---
drivers/hid/hid-ids.h | 3 ++
drivers/hid/hid-multitouch.c | 91 ++++++++++++++++++++++++++++++++++++--------
2 files changed, 78 insertions(+), 16 deletions(-)
I think it would make more sense to introduce a method where the
driver sees all report values at once. We have had reasonable cause to
add it in the past, but never did.
I dreamed of this more than once in the early days of hid-multitouch, yes. But then it requires quite a few changes in hid-core.c, doesn't it?
Surprisingly, the hid-core part is very small. The difficulties come in hid-multitouch. But the good is that it's working :)
I'll send an updated series soon.
Cheers,
Benjamin
From 2ea45e1f42fdf1991a457b36ad9e4e729d9febc6 Mon Sep 17 00:00:00 2001
From: Benjamin Tissoires <redacted>
Date: Mon, 28 Jan 2013 18:30:32 +0100
Subject: [PATCH 1/4] HID: add "report" callback
This callback is called when the parsing of the report has been done
by hid core. The driver can now rely on the values stored in the
different fields.
Signed-off-by: Benjamin Tissoires <redacted>
---
drivers/hid/hid-core.c | 4 ++++
include/linux/hid.h | 1 +
2 files changed, 5 insertions(+)
@@ -1195,6 +1195,7 @@ int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int size,{structhid_report_enum*report_enum=hid->report_enum+type;structhid_report*report;+structhid_driver*hdrv;unsignedinta;intrsize,csize=size;u8*cdata=data;
@@ -1231,6 +1232,9 @@ int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int size,if(hid->claimed!=HID_CLAIMED_HIDRAW){for(a=0;a<report->maxfield;a++)hid_input_field(hid,report->field[a],cdata,interrupt);+hdrv=hid->driver;+if(hdrv&&hdrv->report)+hdrv->report(hid,report);}if(hid->claimed&HID_CLAIMED_INPUT)