On Fri, May 12, 2017 at 4:23 PM, Benjamin Tissoires <benjamin.tissoires@xxxxxxxxx> wrote: > > > On 05/12/2017 09:57 AM, Arek Burdach wrote: >> >> >> On 12.05.2017 09:39, Benjamin Tissoires wrote: >>> On Fri, May 12, 2017 at 9:25 AM, Arek Burdach <arek.burdach@xxxxxxxxx> >>> wrote: >>>> >>>> On 12.05.2017 08:56, Martin Kepplinger wrote: >>>>>>>>>> No, you won't have "move after hold>5s" broken. Because at the HID >>>>>>>>>> level, the device is supposed to send an update on every touch >>>>>>>>>> when >>>>>>>>>> reporting a touch (for Windows 8 devices). So if there are tiny >>>>>>>>>> movements filtered at the input level in the kernel, we will get >>>>>>>>>> those >>>>>>>>>> and I suspect the timeout will only appear when the finger actual >>>>>>>>>> leaves the surface. >>>>>>>>> ok. sounds a little more like a solution in the kernel would be >>>>>>>>> justified. Isn't it? It still feels dangerously ugly. >>>>>>>>> >>>>>>>>> Mainly I wanted to point out that if you somehow have to stay with >>>>>>>>> "no" >>>>>>>>> for such broken devices, tslib would be a garbage can for userspace >>>>>>>>> workarounds. (in this case, most probably a new device-specific >>>>>>>>> hidraw >>>>>>>>> based module). >>>>> (...) >>>>> >>>>>> Thank you for clarification. So do someone have an idea how it is >>>>>> possible that Windows manages this? From my point of view, they can't >>>>>> rely on timeouts because effect is visible immediately after releasing >>>>>> finger. Is it possible that they use other protocol for communication >>>>>> with device then we do? Because beside that, I don't see any other >>>>>> option - there is too few information from device to correctly handle >>>>>> this situation. >>>>> hey, Benjamin explained what most probably is going on, see above, so: >>>>> >>>>> 1. convice yourself that microsoft isn't using out-of-band data by >>>>> sniffing the connection. >>>> Touchscreen is communicating via i2c - am i right? Can you recommend >>>> some >>>> i2c sniffer for windows? >>> I wrote something few months ago: >>> https://github.com/bentiss/SimplePeripheralBusProbe >>> But it requires you to have confidence in not breaking your EFI :) >>> >>> I can help you setting the bits up if you want. >>> >>>>> 2. if not, and Benjamin is right, come up with a timer- and hidraw >>>>> based >>>>> solution (I guess) and convince him and Dmitry to take it. >>>>> >>>>> 3. if they don't see any chance to support such broken behaviour in the >>>>> kernel, which could as well be and also has it's benefits in some way, >>>>> you write the thing in userspace (a tslib raw module is only one >>>>> example >>>>> that would make it easy for you). >>>>> >>>>> Even *if* Synaptics would come up with a firmware update: >>>>> 1. the current firmware is already out there in the wild; >>>>> 2. it takes time and work to get people to update >>>>> >>>>> so, if I had the device, I'd write a workaround. >>>> It is reasonable what you've wrote. The only blocker for me is that I >>>> have >>>> almost zero-experience in low level programming. I'm from java world >>>> :-) It >>>> is why I was looking for some low entry level solution. I'll do my >>>> best but >>>> every helping hand will be appreciated. >>> I am currently checking the requirements for the devices to be >>> validated by Microsoft: >>> https://msdn.microsoft.com/en-us/windows/hardware/commercialize/design/compatibility/1703/device-input-digitizer >>> >>> >>> I would believe that the Latency and ReportRate requirements mean that >>> as long as a finger is there, it should be reported at at least 60Hz, >>> meaning that if there are no input reports for more than 16 ms, all >>> fingers should be lifted. I think we can work something like that >>> (taking an arbitrary multiple of 60 Hz would prevent any system lag), >>> and release the touches if nothing comes in for, lets say, 250ms. >> >> Thank you Benjamin, it is significant brick for building knowledge how >> this devices are handled in Windows. I will take a look in our drivers >> and will try to understand the code and find the way how to handle it. >> >>> >>> I should be able to work on that for Win8 devices. Win7 devices are a >>> different beast, but hopefully they are nearly extinct or at least >>> quirked in the kernel for them to be working. >>> > > I have something which seems to compile, but for various reasons I haven't tested it (yet): > > diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c > index 24d5b6d..2ce0e96 100644 > --- a/drivers/hid/hid-multitouch.c > +++ b/drivers/hid/hid-multitouch.c > @@ -44,6 +44,7 @@ > #include <linux/slab.h> > #include <linux/input/mt.h> > #include <linux/string.h> > +#include <linux/timer.h> > > > MODULE_AUTHOR("Stephane Chatty <chatty@xxxxxxx>"); > @@ -54,22 +55,23 @@ MODULE_LICENSE("GPL"); > #include "hid-ids.h" > > /* quirks to control the device */ > -#define MT_QUIRK_NOT_SEEN_MEANS_UP (1 << 0) > -#define MT_QUIRK_SLOT_IS_CONTACTID (1 << 1) > -#define MT_QUIRK_CYPRESS (1 << 2) > -#define MT_QUIRK_SLOT_IS_CONTACTNUMBER (1 << 3) > -#define MT_QUIRK_ALWAYS_VALID (1 << 4) > -#define MT_QUIRK_VALID_IS_INRANGE (1 << 5) > -#define MT_QUIRK_VALID_IS_CONFIDENCE (1 << 6) > -#define MT_QUIRK_CONFIDENCE (1 << 7) > -#define MT_QUIRK_SLOT_IS_CONTACTID_MINUS_ONE (1 << 8) > -#define MT_QUIRK_NO_AREA (1 << 9) > -#define MT_QUIRK_IGNORE_DUPLICATES (1 << 10) > -#define MT_QUIRK_HOVERING (1 << 11) > -#define MT_QUIRK_CONTACT_CNT_ACCURATE (1 << 12) > -#define MT_QUIRK_FORCE_GET_FEATURE (1 << 13) > -#define MT_QUIRK_FIX_CONST_CONTACT_ID (1 << 14) > -#define MT_QUIRK_TOUCH_SIZE_SCALING (1 << 15) > +#define MT_QUIRK_NOT_SEEN_MEANS_UP BIT(0) > +#define MT_QUIRK_SLOT_IS_CONTACTID BIT(1) > +#define MT_QUIRK_CYPRESS BIT(2) > +#define MT_QUIRK_SLOT_IS_CONTACTNUMBER BIT(3) > +#define MT_QUIRK_ALWAYS_VALID BIT(4) > +#define MT_QUIRK_VALID_IS_INRANGE BIT(5) > +#define MT_QUIRK_VALID_IS_CONFIDENCE BIT(6) > +#define MT_QUIRK_CONFIDENCE BIT(7) > +#define MT_QUIRK_SLOT_IS_CONTACTID_MINUS_ONE BIT(8) > +#define MT_QUIRK_NO_AREA BIT(9) > +#define MT_QUIRK_IGNORE_DUPLICATES BIT(10) > +#define MT_QUIRK_HOVERING BIT(11) > +#define MT_QUIRK_CONTACT_CNT_ACCURATE BIT(12) > +#define MT_QUIRK_FORCE_GET_FEATURE BIT(13) > +#define MT_QUIRK_FIX_CONST_CONTACT_ID BIT(14) > +#define MT_QUIRK_TOUCH_SIZE_SCALING BIT(15) > +#define MT_QUIRK_STICKY_FINGERS BIT(16) > > #define MT_INPUTMODE_TOUCHSCREEN 0x02 > #define MT_INPUTMODE_TOUCHPAD 0x03 > @@ -104,6 +106,7 @@ struct mt_fields { > struct mt_device { > struct mt_slot curdata; /* placeholder of incoming data */ > struct mt_class mtclass; /* our mt device class */ > + struct timer_list release_timer; /* to release sticky fingers */ > struct mt_fields *fields; /* temporary placeholder for storing the > multitouch fields */ > int cc_index; /* contact count field index in the report */ > @@ -212,7 +215,8 @@ static struct mt_class mt_classes[] = { > .quirks = MT_QUIRK_ALWAYS_VALID | > MT_QUIRK_IGNORE_DUPLICATES | > MT_QUIRK_HOVERING | > - MT_QUIRK_CONTACT_CNT_ACCURATE }, > + MT_QUIRK_CONTACT_CNT_ACCURATE | > + MT_QUIRK_STICKY_FINGERS }, > { .name = MT_CLS_EXPORT_ALL_INPUTS, > .quirks = MT_QUIRK_ALWAYS_VALID | > MT_QUIRK_CONTACT_CNT_ACCURATE, > @@ -813,6 +817,9 @@ static void mt_touch_report(struct hid_device *hid, struct hid_report *report) > > if (td->num_received >= td->num_expected) > mt_sync_frame(td, report->field[0]->hidinput->input); > + > + if (td->mtclass.quirks & MT_QUIRK_STICKY_FINGERS) > + mod_timer(&td->release_timer, msecs_to_jiffies(250)); > } > > static int mt_touch_input_configured(struct hid_device *hdev, > @@ -1124,6 +1131,35 @@ static void mt_fix_const_fields(struct hid_device *hdev, unsigned int usage) > } > } > > +static void mt_release_contacts(struct hid_device *hid) > +{ > + struct hid_input *hidinput; > + > + list_for_each_entry(hidinput, &hid->inputs, list) { > + struct input_dev *input_dev = hidinput->input; > + struct input_mt *mt = input_dev->mt; > + int i; > + > + if (mt) { > + for (i = 0; i < mt->num_slots; i++) { > + input_mt_slot(input_dev, i); > + input_mt_report_slot_state(input_dev, > + MT_TOOL_FINGER, > + false); > + } > + input_mt_sync_frame(input_dev); > + input_sync(input_dev); > + } > + } > +} > + > +static void mt_expired_timeout(unsigned long arg) > +{ > + struct hid_device *hdev = (void*)arg; > + > + mt_release_contacts(hdev); > +} > + > static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id) > { > int ret, i; > @@ -1193,6 +1229,8 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id) > */ > hdev->quirks |= HID_QUIRK_NO_INIT_REPORTS; > > + setup_timer(&td->release_timer, mt_expired_timeout, (long)hdev); > + > ret = hid_parse(hdev); > if (ret != 0) > return ret; > @@ -1220,28 +1258,6 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id) > } > > #ifdef CONFIG_PM > -static void mt_release_contacts(struct hid_device *hid) > -{ > - struct hid_input *hidinput; > - > - list_for_each_entry(hidinput, &hid->inputs, list) { > - struct input_dev *input_dev = hidinput->input; > - struct input_mt *mt = input_dev->mt; > - int i; > - > - if (mt) { > - for (i = 0; i < mt->num_slots; i++) { > - input_mt_slot(input_dev, i); > - input_mt_report_slot_state(input_dev, > - MT_TOOL_FINGER, > - false); > - } > - input_mt_sync_frame(input_dev); > - input_sync(input_dev); > - } > - } > -} > - > static int mt_reset_resume(struct hid_device *hdev) > { > mt_release_contacts(hdev); > @@ -1266,6 +1282,8 @@ static void mt_remove(struct hid_device *hdev) > { > struct mt_device *td = hid_get_drvdata(hdev); > > + del_timer_sync(&mt->release_timer); Sorry for the noise: this should be td->release_timer, not "mt". Cheers, Benjamin > + > sysfs_remove_group(&hdev->dev.kobj, &mt_attribute_group); > hid_hw_stop(hdev); > hdev->quirks = td->initial_quirks; > > --- > > (the (1 << X) -> BIT(X) conversion needs to be done in a separate patch). > > From what I can read in the documentation, this might be enough. However, > I wouldn't be surprised if this segfaults for whatever reasons. > Also note that there is no guards between the execution of the release and > the incoming events. So if you wait around 250 ms between 2 touches, nothing > prevents a race between the timeout previously started and the actual true > touches. > > But it might be enough to have a good test case and see if this works well in > your case and in most Win8 devices. > > Cheers, > Benjamin -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
