All of lore.kernel.org
 help / color / mirror / Atom feed
* Missing release event for Synaptics touchscreen
@ 2017-05-06 19:28 Arek Burdach
  2017-05-09  8:35 ` Benjamin Tissoires
  0 siblings, 1 reply; 37+ messages in thread
From: Arek Burdach @ 2017-05-06 19:28 UTC (permalink / raw)
  To: benjamin.tissoires, chatty, aduggan; +Cc: linux-input

Hi,

A week ago I've reported a bug: 
https://bugzilla.kernel.org/show_bug.cgi?id=195625 Is there anybody that 
can help me with it?
I found out that some touchpads (and possible touchscreens?) are handled 
by both hid-multitouch and hid-rmi drivers. Is there a way to verify how 
the touchscreen would work on hid-rmi drivers? I've tested it with 
kernel 4.11.0-rc1 version where was this change:
279967a65b320d174a507498aea7d44db3fee7f4 HID: rmi: Handle all Synaptics 
touchpads using hid-rmi
which was reverted in later kernel's version. On this version, only my 
touchpad was handled by hid-rmi, touchscreen was still handled by 
hid-multitouch. Maybe I should change something in code or in 
compilation configuration to force hid-rmi?
Or it is a wrong way to go? I would be grateful for your help.

Regards,
Arek

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
  2017-05-06 19:28 Missing release event for Synaptics touchscreen Arek Burdach
@ 2017-05-09  8:35 ` Benjamin Tissoires
  2017-05-09  9:20   ` Arek Burdach
  0 siblings, 1 reply; 37+ messages in thread
From: Benjamin Tissoires @ 2017-05-09  8:35 UTC (permalink / raw)
  To: Arek Burdach; +Cc: Stéphane Chatty, Andrew Duggan, linux-input

On Sat, May 6, 2017 at 9:28 PM, Arek Burdach <arek.burdach@gmail.com> wrote:
> Hi,
>
> A week ago I've reported a bug:
> https://bugzilla.kernel.org/show_bug.cgi?id=195625 Is there anybody that can
> help me with it?

I can have a look at it.
Please attach the full outputs of hid-recorder and evemu-record in the
bugs, or it'll be difficult for us to debug it.

> I found out that some touchpads (and possible touchscreens?) are handled by
> both hid-multitouch and hid-rmi drivers. Is there a way to verify how the
> touchscreen would work on hid-rmi drivers? I've tested it with kernel
> 4.11.0-rc1 version where was this change:
> 279967a65b320d174a507498aea7d44db3fee7f4 HID: rmi: Handle all Synaptics
> touchpads using hid-rmi
> which was reverted in later kernel's version. On this version, only my
> touchpad was handled by hid-rmi, touchscreen was still handled by
> hid-multitouch. Maybe I should change something in code or in compilation
> configuration to force hid-rmi?

Well, with 279967a65 applied, the system would know if the device can
be handled through hid-rmi or not. If hid-multitouch was still used,
that means that the device was not designed to be used as a rmi device
at all (i.e. hid-rmi will not be able to talk to it).

Cheers,
Benjamin

> Or it is a wrong way to go? I would be grateful for your help.
>
> Regards,
> Arek

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
  2017-05-09  8:35 ` Benjamin Tissoires
@ 2017-05-09  9:20   ` Arek Burdach
  2017-05-09 12:20     ` Benjamin Tissoires
  0 siblings, 1 reply; 37+ messages in thread
From: Arek Burdach @ 2017-05-09  9:20 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: Stéphane Chatty, Andrew Duggan, linux-input

Hi,

Thank you for response.

On 09.05.2017 10:35, Benjamin Tissoires wrote:
> On Sat, May 6, 2017 at 9:28 PM, Arek Burdach <arek.burdach@gmail.com> wrote:
>> Hi,
>>
>> A week ago I've reported a bug:
>> https://bugzilla.kernel.org/show_bug.cgi?id=195625 Is there anybody that can
>> help me with it?
> I can have a look at it.
> Please attach the full outputs of hid-recorder and evemu-record in the
> bugs, or it'll be difficult for us to debug it.
I've attached full logs for two situations. More details in the issue.


>> I found out that some touchpads (and possible touchscreens?) are handled by
>> both hid-multitouch and hid-rmi drivers. Is there a way to verify how the
>> touchscreen would work on hid-rmi drivers? I've tested it with kernel
>> 4.11.0-rc1 version where was this change:
>> 279967a65b320d174a507498aea7d44db3fee7f4 HID: rmi: Handle all Synaptics
>> touchpads using hid-rmi
>> which was reverted in later kernel's version. On this version, only my
>> touchpad was handled by hid-rmi, touchscreen was still handled by
>> hid-multitouch. Maybe I should change something in code or in compilation
>> configuration to force hid-rmi?
> Well, with 279967a65 applied, the system would know if the device can
> be handled through hid-rmi or not. If hid-multitouch was still used,
> that means that the device was not designed to be used as a rmi device
> at all (i.e. hid-rmi will not be able to talk to it).
In this patch, there is verified if hid group is 
HID_GROUP_MULTITOUCH_WIN_8. Maybe this touchscreen is in group with 
greater priority during recognition. Can I verify it somehow?

Also HID_GROUP_MULTITOUCH_WIN_8 recognition is done by this:
> usage == 0xff0000c5 && parser->global.report_count == 256 && 
> parser->global.report_size == 8
I assume that it is correct way to verify that. I wonder if it is 
possible that something changed for a new devices.

Cheers,
Arek

> Cheers,
> Benjamin
>
>> Or it is a wrong way to go? I would be grateful for your help.
>>
>> Regards,
>> Arek


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
  2017-05-09  9:20   ` Arek Burdach
@ 2017-05-09 12:20     ` Benjamin Tissoires
  2017-05-09 12:51       ` Arek Burdach
  0 siblings, 1 reply; 37+ messages in thread
From: Benjamin Tissoires @ 2017-05-09 12:20 UTC (permalink / raw)
  To: Arek Burdach; +Cc: Stéphane Chatty, Andrew Duggan, linux-input

On Tue, May 9, 2017 at 11:20 AM, Arek Burdach <arek.burdach@gmail.com> wrote:
> Hi,
>
> Thank you for response.
>
> On 09.05.2017 10:35, Benjamin Tissoires wrote:
>>
>> On Sat, May 6, 2017 at 9:28 PM, Arek Burdach <arek.burdach@gmail.com>
>> wrote:
>>>
>>> Hi,
>>>
>>> A week ago I've reported a bug:
>>> https://bugzilla.kernel.org/show_bug.cgi?id=195625 Is there anybody that
>>> can
>>> help me with it?
>>
>> I can have a look at it.
>> Please attach the full outputs of hid-recorder and evemu-record in the
>> bugs, or it'll be difficult for us to debug it.
>
> I've attached full logs for two situations. More details in the issue.

Thanks, looks like a firmware issue (I'll comment in the bug).

>
>
>>> I found out that some touchpads (and possible touchscreens?) are handled
>>> by
>>> both hid-multitouch and hid-rmi drivers. Is there a way to verify how the
>>> touchscreen would work on hid-rmi drivers? I've tested it with kernel
>>> 4.11.0-rc1 version where was this change:
>>> 279967a65b320d174a507498aea7d44db3fee7f4 HID: rmi: Handle all Synaptics
>>> touchpads using hid-rmi
>>> which was reverted in later kernel's version. On this version, only my
>>> touchpad was handled by hid-rmi, touchscreen was still handled by
>>> hid-multitouch. Maybe I should change something in code or in compilation
>>> configuration to force hid-rmi?
>>
>> Well, with 279967a65 applied, the system would know if the device can
>> be handled through hid-rmi or not. If hid-multitouch was still used,
>> that means that the device was not designed to be used as a rmi device
>> at all (i.e. hid-rmi will not be able to talk to it).
>
> In this patch, there is verified if hid group is HID_GROUP_MULTITOUCH_WIN_8.
> Maybe this touchscreen is in group with greater priority during recognition.
> Can I verify it somehow?

With the logs you gave, the touchscreen is indeed Win 8 certified.

>
> Also HID_GROUP_MULTITOUCH_WIN_8 recognition is done by this:
>>
>> usage == 0xff0000c5 && parser->global.report_count == 256 &&
>> parser->global.report_size == 8
>
> I assume that it is correct way to verify that. I wonder if it is possible
> that something changed for a new devices.

Nope, looks good for your device. The reason why hid-rmi is not
picking up your device is because of the check "parser->scan_flags &
HID_SCAN_FLAG_GD_POINTER": this matches only to indirect devices
(touchpads).

If you can recompile your hid and hid-rmi modules, you can try giving a shot at:
- adding HID_I2C_DEVICE(USB_VENDOR_ID_SYNAPTICS, 0x1786) }, in
hid_have_special_driver[] in hid-core.c
- adding a corresponding line in rmi_id[] in file hid-rmi.c.

This should force the device to bind to hid-rmi and you'll be able to
see if the device works better in this situation or not.

Cheers,
Benjamin

>
> Cheers,
> Arek
>
>
>> Cheers,
>> Benjamin
>>
>>> Or it is a wrong way to go? I would be grateful for your help.
>>>
>>> Regards,
>>> Arek
>
>

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
  2017-05-09 12:20     ` Benjamin Tissoires
@ 2017-05-09 12:51       ` Arek Burdach
  2017-05-09 14:02         ` Benjamin Tissoires
  0 siblings, 1 reply; 37+ messages in thread
From: Arek Burdach @ 2017-05-09 12:51 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: Stéphane Chatty, Andrew Duggan, linux-input


On 09.05.2017 14:20, Benjamin Tissoires wrote:
> On Tue, May 9, 2017 at 11:20 AM, Arek Burdach <arek.burdach@gmail.com> wrote:
>> Hi,
>>
>> Thank you for response.
>>
>> On 09.05.2017 10:35, Benjamin Tissoires wrote:
>>> On Sat, May 6, 2017 at 9:28 PM, Arek Burdach <arek.burdach@gmail.com>
>>> wrote:
>>>> Hi,
>>>>
>>>> A week ago I've reported a bug:
>>>> https://bugzilla.kernel.org/show_bug.cgi?id=195625 Is there anybody that
>>>> can
>>>> help me with it?
>>> I can have a look at it.
>>> Please attach the full outputs of hid-recorder and evemu-record in the
>>> bugs, or it'll be difficult for us to debug it.
>> I've attached full logs for two situations. More details in the issue.
> Thanks, looks like a firmware issue (I'll comment in the bug).
Sorry for my noob questions, but do you suggest that it can't be fixed 
by changes in kernel modules and I need to report it to the manufacturer?
If it so, do you have an idea why it works well on Windows? Do they have 
some strange hacks in their drivers?


>
>>
>>>> I found out that some touchpads (and possible touchscreens?) are handled
>>>> by
>>>> both hid-multitouch and hid-rmi drivers. Is there a way to verify how the
>>>> touchscreen would work on hid-rmi drivers? I've tested it with kernel
>>>> 4.11.0-rc1 version where was this change:
>>>> 279967a65b320d174a507498aea7d44db3fee7f4 HID: rmi: Handle all Synaptics
>>>> touchpads using hid-rmi
>>>> which was reverted in later kernel's version. On this version, only my
>>>> touchpad was handled by hid-rmi, touchscreen was still handled by
>>>> hid-multitouch. Maybe I should change something in code or in compilation
>>>> configuration to force hid-rmi?
>>> Well, with 279967a65 applied, the system would know if the device can
>>> be handled through hid-rmi or not. If hid-multitouch was still used,
>>> that means that the device was not designed to be used as a rmi device
>>> at all (i.e. hid-rmi will not be able to talk to it).
>> In this patch, there is verified if hid group is HID_GROUP_MULTITOUCH_WIN_8.
>> Maybe this touchscreen is in group with greater priority during recognition.
>> Can I verify it somehow?
> With the logs you gave, the touchscreen is indeed Win 8 certified.
>
>> Also HID_GROUP_MULTITOUCH_WIN_8 recognition is done by this:
>>> usage == 0xff0000c5 && parser->global.report_count == 256 &&
>>> parser->global.report_size == 8
>> I assume that it is correct way to verify that. I wonder if it is possible
>> that something changed for a new devices.
> Nope, looks good for your device. The reason why hid-rmi is not
> picking up your device is because of the check "parser->scan_flags &
> HID_SCAN_FLAG_GD_POINTER": this matches only to indirect devices
> (touchpads).
>
> If you can recompile your hid and hid-rmi modules, you can try giving a shot at:
> - adding HID_I2C_DEVICE(USB_VENDOR_ID_SYNAPTICS, 0x1786) }, in
> hid_have_special_driver[] in hid-core.c
> - adding a corresponding line in rmi_id[] in file hid-rmi.c.
>
> This should force the device to bind to hid-rmi and you'll be able to
> see if the device works better in this situation or not.
Sorry for another noob question, but if I switch to hid-rmi, it should 
potentially produce other output in both /dev/hidraw output and 
/dev/input/event output or only in the second one?
I'm just wonder what is the pipeline.

I'll give a try to it and revert back to you with results. Thank you for 
tip!

>
> Cheers,
> Benjamin
>
>> Cheers,
>> Arek
>>
>>
>>> Cheers,
>>> Benjamin
>>>
>>>> Or it is a wrong way to go? I would be grateful for your help.
>>>>
>>>> Regards,
>>>> Arek
>>


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
  2017-05-09 12:51       ` Arek Burdach
@ 2017-05-09 14:02         ` Benjamin Tissoires
  2017-05-09 23:17           ` Arek Burdach
  0 siblings, 1 reply; 37+ messages in thread
From: Benjamin Tissoires @ 2017-05-09 14:02 UTC (permalink / raw)
  To: Arek Burdach; +Cc: Stéphane Chatty, Andrew Duggan, linux-input

On Tue, May 9, 2017 at 2:51 PM, Arek Burdach <arek.burdach@gmail.com> wrote:
>
> On 09.05.2017 14:20, Benjamin Tissoires wrote:
>>
>> On Tue, May 9, 2017 at 11:20 AM, Arek Burdach <arek.burdach@gmail.com>
>> wrote:
>>>
>>> Hi,
>>>
>>> Thank you for response.
>>>
>>> On 09.05.2017 10:35, Benjamin Tissoires wrote:
>>>>
>>>> On Sat, May 6, 2017 at 9:28 PM, Arek Burdach <arek.burdach@gmail.com>
>>>> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> A week ago I've reported a bug:
>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=195625 Is there anybody
>>>>> that
>>>>> can
>>>>> help me with it?
>>>>
>>>> I can have a look at it.
>>>> Please attach the full outputs of hid-recorder and evemu-record in the
>>>> bugs, or it'll be difficult for us to debug it.
>>>
>>> I've attached full logs for two situations. More details in the issue.
>>
>> Thanks, looks like a firmware issue (I'll comment in the bug).
>
> Sorry for my noob questions, but do you suggest that it can't be fixed by
> changes in kernel modules and I need to report it to the manufacturer?

Yes. Though Andrew, in CC, works for Synaptics and might give us some pointers.

> If it so, do you have an idea why it works well on Windows? Do they have
> some strange hacks in their drivers?

I have no ideas how well it works under Windows, and I have no ideas
if there are some strange hacks in the Windows nor in the Syanptics
driver (I would assume so).

>
>
>
>>
>>>
>>>>> I found out that some touchpads (and possible touchscreens?) are
>>>>> handled
>>>>> by
>>>>> both hid-multitouch and hid-rmi drivers. Is there a way to verify how
>>>>> the
>>>>> touchscreen would work on hid-rmi drivers? I've tested it with kernel
>>>>> 4.11.0-rc1 version where was this change:
>>>>> 279967a65b320d174a507498aea7d44db3fee7f4 HID: rmi: Handle all Synaptics
>>>>> touchpads using hid-rmi
>>>>> which was reverted in later kernel's version. On this version, only my
>>>>> touchpad was handled by hid-rmi, touchscreen was still handled by
>>>>> hid-multitouch. Maybe I should change something in code or in
>>>>> compilation
>>>>> configuration to force hid-rmi?
>>>>
>>>> Well, with 279967a65 applied, the system would know if the device can
>>>> be handled through hid-rmi or not. If hid-multitouch was still used,
>>>> that means that the device was not designed to be used as a rmi device
>>>> at all (i.e. hid-rmi will not be able to talk to it).
>>>
>>> In this patch, there is verified if hid group is
>>> HID_GROUP_MULTITOUCH_WIN_8.
>>> Maybe this touchscreen is in group with greater priority during
>>> recognition.
>>> Can I verify it somehow?
>>
>> With the logs you gave, the touchscreen is indeed Win 8 certified.
>>
>>> Also HID_GROUP_MULTITOUCH_WIN_8 recognition is done by this:
>>>>
>>>> usage == 0xff0000c5 && parser->global.report_count == 256 &&
>>>> parser->global.report_size == 8
>>>
>>> I assume that it is correct way to verify that. I wonder if it is
>>> possible
>>> that something changed for a new devices.
>>
>> Nope, looks good for your device. The reason why hid-rmi is not
>> picking up your device is because of the check "parser->scan_flags &
>> HID_SCAN_FLAG_GD_POINTER": this matches only to indirect devices
>> (touchpads).
>>
>> If you can recompile your hid and hid-rmi modules, you can try giving a
>> shot at:
>> - adding HID_I2C_DEVICE(USB_VENDOR_ID_SYNAPTICS, 0x1786) }, in
>> hid_have_special_driver[] in hid-core.c
>> - adding a corresponding line in rmi_id[] in file hid-rmi.c.
>>
>> This should force the device to bind to hid-rmi and you'll be able to
>> see if the device works better in this situation or not.
>
> Sorry for another noob question, but if I switch to hid-rmi, it should
> potentially produce other output in both /dev/hidraw output and
> /dev/input/event output or only in the second one?

The more, the better. Please provide all logs, hidraw and evemu.

> I'm just wonder what is the pipeline.
>
> I'll give a try to it and revert back to you with results. Thank you for
> tip!

No worries.

Cheers,
Benjamin

>
>
>>
>> Cheers,
>> Benjamin
>>
>>> Cheers,
>>> Arek
>>>
>>>
>>>> Cheers,
>>>> Benjamin
>>>>
>>>>> Or it is a wrong way to go? I would be grateful for your help.
>>>>>
>>>>> Regards,
>>>>> Arek
>>>
>>>
>

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
  2017-05-09 14:02         ` Benjamin Tissoires
@ 2017-05-09 23:17           ` Arek Burdach
  2017-05-09 23:47             ` Andrew Duggan
  0 siblings, 1 reply; 37+ messages in thread
From: Arek Burdach @ 2017-05-09 23:17 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: Stéphane Chatty, Andrew Duggan, linux-input

Hi,

I've tried described by you solution:

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 37084b645785..81f271554b6c 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2510,6 +2510,7 @@ static const struct hid_device_id 
hid_ignore_list[] = {
         { HID_USB_DEVICE(USB_VENDOR_ID_SYNAPTICS, 
USB_DEVICE_ID_SYNAPTICS_WTP) },
         { HID_USB_DEVICE(USB_VENDOR_ID_SYNAPTICS, 
USB_DEVICE_ID_SYNAPTICS_DPAD) },
  #endif
+       { HID_I2C_DEVICE(USB_VENDOR_ID_SYNAPTICS, 0x1786) },
         { HID_USB_DEVICE(USB_VENDOR_ID_YEALINK, 
USB_DEVICE_ID_YEALINK_P1K_P4K_B2K) },
         { }
  };
diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c
index 5b40c2614599..ac2ea6ad2e53 100644
--- a/drivers/hid/hid-rmi.c
+++ b/drivers/hid/hid-rmi.c
@@ -714,6 +714,7 @@ static void rmi_remove(struct hid_device *hdev)
  }

  static const struct hid_device_id rmi_id[] = {
+       { HID_I2C_DEVICE(USB_VENDOR_ID_SYNAPTICS, 0x1786) },
         { HID_USB_DEVICE(USB_VENDOR_ID_RAZER, 
USB_DEVICE_ID_RAZER_BLADE_14),
                 .driver_data = RMI_DEVICE_HAS_PHYS_BUTTONS },
         { HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, 
USB_DEVICE_ID_LENOVO_X1_COVER) },


and now neither hid-touchscreen nor hid-rmi peaking up my touchscreen 
(also missing hidraw device)

Any other idea how to force hid-rmi to peak the device?

Cheers,
Arek

On 09.05.2017 16:02, Benjamin Tissoires wrote:
> On Tue, May 9, 2017 at 2:51 PM, Arek Burdach <arek.burdach@gmail.com> wrote:
>> On 09.05.2017 14:20, Benjamin Tissoires wrote:
>>> On Tue, May 9, 2017 at 11:20 AM, Arek Burdach <arek.burdach@gmail.com>
>>> wrote:
>>>> Hi,
>>>>
>>>> Thank you for response.
>>>>
>>>> On 09.05.2017 10:35, Benjamin Tissoires wrote:
>>>>> On Sat, May 6, 2017 at 9:28 PM, Arek Burdach <arek.burdach@gmail.com>
>>>>> wrote:
>>>>>> Hi,
>>>>>>
>>>>>> A week ago I've reported a bug:
>>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=195625 Is there anybody
>>>>>> that
>>>>>> can
>>>>>> help me with it?
>>>>> I can have a look at it.
>>>>> Please attach the full outputs of hid-recorder and evemu-record in the
>>>>> bugs, or it'll be difficult for us to debug it.
>>>> I've attached full logs for two situations. More details in the issue.
>>> Thanks, looks like a firmware issue (I'll comment in the bug).
>> Sorry for my noob questions, but do you suggest that it can't be fixed by
>> changes in kernel modules and I need to report it to the manufacturer?
> Yes. Though Andrew, in CC, works for Synaptics and might give us some pointers.
>
>> If it so, do you have an idea why it works well on Windows? Do they have
>> some strange hacks in their drivers?
> I have no ideas how well it works under Windows, and I have no ideas
> if there are some strange hacks in the Windows nor in the Syanptics
> driver (I would assume so).
>
>>
>>
>>>>>> I found out that some touchpads (and possible touchscreens?) are
>>>>>> handled
>>>>>> by
>>>>>> both hid-multitouch and hid-rmi drivers. Is there a way to verify how
>>>>>> the
>>>>>> touchscreen would work on hid-rmi drivers? I've tested it with kernel
>>>>>> 4.11.0-rc1 version where was this change:
>>>>>> 279967a65b320d174a507498aea7d44db3fee7f4 HID: rmi: Handle all Synaptics
>>>>>> touchpads using hid-rmi
>>>>>> which was reverted in later kernel's version. On this version, only my
>>>>>> touchpad was handled by hid-rmi, touchscreen was still handled by
>>>>>> hid-multitouch. Maybe I should change something in code or in
>>>>>> compilation
>>>>>> configuration to force hid-rmi?
>>>>> Well, with 279967a65 applied, the system would know if the device can
>>>>> be handled through hid-rmi or not. If hid-multitouch was still used,
>>>>> that means that the device was not designed to be used as a rmi device
>>>>> at all (i.e. hid-rmi will not be able to talk to it).
>>>> In this patch, there is verified if hid group is
>>>> HID_GROUP_MULTITOUCH_WIN_8.
>>>> Maybe this touchscreen is in group with greater priority during
>>>> recognition.
>>>> Can I verify it somehow?
>>> With the logs you gave, the touchscreen is indeed Win 8 certified.
>>>
>>>> Also HID_GROUP_MULTITOUCH_WIN_8 recognition is done by this:
>>>>> usage == 0xff0000c5 && parser->global.report_count == 256 &&
>>>>> parser->global.report_size == 8
>>>> I assume that it is correct way to verify that. I wonder if it is
>>>> possible
>>>> that something changed for a new devices.
>>> Nope, looks good for your device. The reason why hid-rmi is not
>>> picking up your device is because of the check "parser->scan_flags &
>>> HID_SCAN_FLAG_GD_POINTER": this matches only to indirect devices
>>> (touchpads).
>>>
>>> If you can recompile your hid and hid-rmi modules, you can try giving a
>>> shot at:
>>> - adding HID_I2C_DEVICE(USB_VENDOR_ID_SYNAPTICS, 0x1786) }, in
>>> hid_have_special_driver[] in hid-core.c
>>> - adding a corresponding line in rmi_id[] in file hid-rmi.c.
>>>
>>> This should force the device to bind to hid-rmi and you'll be able to
>>> see if the device works better in this situation or not.
>> Sorry for another noob question, but if I switch to hid-rmi, it should
>> potentially produce other output in both /dev/hidraw output and
>> /dev/input/event output or only in the second one?
> The more, the better. Please provide all logs, hidraw and evemu.
>
>> I'm just wonder what is the pipeline.
>>
>> I'll give a try to it and revert back to you with results. Thank you for
>> tip!
> No worries.
>
> Cheers,
> Benjamin
>
>>
>>> Cheers,
>>> Benjamin
>>>
>>>> Cheers,
>>>> Arek
>>>>
>>>>
>>>>> Cheers,
>>>>> Benjamin
>>>>>
>>>>>> Or it is a wrong way to go? I would be grateful for your help.
>>>>>>
>>>>>> Regards,
>>>>>> Arek
>>>>


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
  2017-05-09 23:17           ` Arek Burdach
@ 2017-05-09 23:47             ` Andrew Duggan
  2017-05-10  9:36               ` Arek Burdach
  0 siblings, 1 reply; 37+ messages in thread
From: Andrew Duggan @ 2017-05-09 23:47 UTC (permalink / raw)
  To: Arek Burdach, Benjamin Tissoires; +Cc: Stéphane Chatty, linux-input

HI Arek,

On 05/09/2017 04:17 PM, Arek Burdach wrote:
> Hi,
>
> I've tried described by you solution:
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 37084b645785..81f271554b6c 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -2510,6 +2510,7 @@ static const struct hid_device_id 
> hid_ignore_list[] = {

You need to add this to the hid_have_special_driver[] and not the 
hid_ignore_list[].

But, if you do success in binding hid-rmi to a touchscreen it won't 
work. The firmware between touchpads and touchscreens are different 
enough that the hid-rmi driver will be looking for data which does not 
exist in touchscreen's HID report. These differences also mean that it 
really isn't a good idea to try to support touchscreens with hid-rmi. It 
would actually result in more transactions and be less efficient then 
simply using hid-multitouch. That's why hid-core checks for the 
HID_SCAN_FLAG_GD_POINTER in an attempt to make sure it's binding to a 
touchpad and not a touchscreen.

>         { HID_USB_DEVICE(USB_VENDOR_ID_SYNAPTICS, 
> USB_DEVICE_ID_SYNAPTICS_WTP) },
>         { HID_USB_DEVICE(USB_VENDOR_ID_SYNAPTICS, 
> USB_DEVICE_ID_SYNAPTICS_DPAD) },
>  #endif
> +       { HID_I2C_DEVICE(USB_VENDOR_ID_SYNAPTICS, 0x1786) },
>         { HID_USB_DEVICE(USB_VENDOR_ID_YEALINK, 
> USB_DEVICE_ID_YEALINK_P1K_P4K_B2K) },
>         { }
>  };
> diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c
> index 5b40c2614599..ac2ea6ad2e53 100644
> --- a/drivers/hid/hid-rmi.c
> +++ b/drivers/hid/hid-rmi.c
> @@ -714,6 +714,7 @@ static void rmi_remove(struct hid_device *hdev)
>  }
>
>  static const struct hid_device_id rmi_id[] = {
> +       { HID_I2C_DEVICE(USB_VENDOR_ID_SYNAPTICS, 0x1786) },
>         { HID_USB_DEVICE(USB_VENDOR_ID_RAZER, 
> USB_DEVICE_ID_RAZER_BLADE_14),
>                 .driver_data = RMI_DEVICE_HAS_PHYS_BUTTONS },
>         { HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, 
> USB_DEVICE_ID_LENOVO_X1_COVER) },
>
>
> and now neither hid-touchscreen nor hid-rmi peaking up my touchscreen 
> (also missing hidraw device)
>
> Any other idea how to force hid-rmi to peak the device?
>
> Cheers,
> Arek
>
> On 09.05.2017 16:02, Benjamin Tissoires wrote:
>> On Tue, May 9, 2017 at 2:51 PM, Arek Burdach <arek.burdach@gmail.com> 
>> wrote:
>>> On 09.05.2017 14:20, Benjamin Tissoires wrote:
>>>> On Tue, May 9, 2017 at 11:20 AM, Arek Burdach <arek.burdach@gmail.com>
>>>> wrote:
>>>>> Hi,
>>>>>
>>>>> Thank you for response.
>>>>>
>>>>> On 09.05.2017 10:35, Benjamin Tissoires wrote:
>>>>>> On Sat, May 6, 2017 at 9:28 PM, Arek Burdach 
>>>>>> <arek.burdach@gmail.com>
>>>>>> wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> A week ago I've reported a bug:
>>>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=195625 Is there anybody
>>>>>>> that
>>>>>>> can
>>>>>>> help me with it?
>>>>>> I can have a look at it.
>>>>>> Please attach the full outputs of hid-recorder and evemu-record 
>>>>>> in the
>>>>>> bugs, or it'll be difficult for us to debug it.
>>>>> I've attached full logs for two situations. More details in the 
>>>>> issue.
>>>> Thanks, looks like a firmware issue (I'll comment in the bug).
>>> Sorry for my noob questions, but do you suggest that it can't be 
>>> fixed by
>>> changes in kernel modules and I need to report it to the manufacturer?
>> Yes. Though Andrew, in CC, works for Synaptics and might give us some 
>> pointers.
>>
>>> If it so, do you have an idea why it works well on Windows? Do they 
>>> have
>>> some strange hacks in their drivers?
>> I have no ideas how well it works under Windows, and I have no ideas
>> if there are some strange hacks in the Windows nor in the Syanptics
>> driver (I would assume so).
>>

We don't provide any drivers for touchscreens on Windows. So I don't 
know how Microsoft is handling a situation like this.

Andrew

>>>
>>>
>>>>>>> I found out that some touchpads (and possible touchscreens?) are
>>>>>>> handled
>>>>>>> by
>>>>>>> both hid-multitouch and hid-rmi drivers. Is there a way to 
>>>>>>> verify how
>>>>>>> the
>>>>>>> touchscreen would work on hid-rmi drivers? I've tested it with 
>>>>>>> kernel
>>>>>>> 4.11.0-rc1 version where was this change:
>>>>>>> 279967a65b320d174a507498aea7d44db3fee7f4 HID: rmi: Handle all 
>>>>>>> Synaptics
>>>>>>> touchpads using hid-rmi
>>>>>>> which was reverted in later kernel's version. On this version, 
>>>>>>> only my
>>>>>>> touchpad was handled by hid-rmi, touchscreen was still handled by
>>>>>>> hid-multitouch. Maybe I should change something in code or in
>>>>>>> compilation
>>>>>>> configuration to force hid-rmi?
>>>>>> Well, with 279967a65 applied, the system would know if the device 
>>>>>> can
>>>>>> be handled through hid-rmi or not. If hid-multitouch was still used,
>>>>>> that means that the device was not designed to be used as a rmi 
>>>>>> device
>>>>>> at all (i.e. hid-rmi will not be able to talk to it).
>>>>> In this patch, there is verified if hid group is
>>>>> HID_GROUP_MULTITOUCH_WIN_8.
>>>>> Maybe this touchscreen is in group with greater priority during
>>>>> recognition.
>>>>> Can I verify it somehow?
>>>> With the logs you gave, the touchscreen is indeed Win 8 certified.
>>>>
>>>>> Also HID_GROUP_MULTITOUCH_WIN_8 recognition is done by this:
>>>>>> usage == 0xff0000c5 && parser->global.report_count == 256 &&
>>>>>> parser->global.report_size == 8
>>>>> I assume that it is correct way to verify that. I wonder if it is
>>>>> possible
>>>>> that something changed for a new devices.
>>>> Nope, looks good for your device. The reason why hid-rmi is not
>>>> picking up your device is because of the check "parser->scan_flags &
>>>> HID_SCAN_FLAG_GD_POINTER": this matches only to indirect devices
>>>> (touchpads).
>>>>
>>>> If you can recompile your hid and hid-rmi modules, you can try 
>>>> giving a
>>>> shot at:
>>>> - adding HID_I2C_DEVICE(USB_VENDOR_ID_SYNAPTICS, 0x1786) }, in
>>>> hid_have_special_driver[] in hid-core.c
>>>> - adding a corresponding line in rmi_id[] in file hid-rmi.c.
>>>>
>>>> This should force the device to bind to hid-rmi and you'll be able to
>>>> see if the device works better in this situation or not.
>>> Sorry for another noob question, but if I switch to hid-rmi, it should
>>> potentially produce other output in both /dev/hidraw output and
>>> /dev/input/event output or only in the second one?
>> The more, the better. Please provide all logs, hidraw and evemu.
>>
>>> I'm just wonder what is the pipeline.
>>>
>>> I'll give a try to it and revert back to you with results. Thank you 
>>> for
>>> tip!
>> No worries.
>>
>> Cheers,
>> Benjamin
>>
>>>
>>>> Cheers,
>>>> Benjamin
>>>>
>>>>> Cheers,
>>>>> Arek
>>>>>
>>>>>
>>>>>> Cheers,
>>>>>> Benjamin
>>>>>>
>>>>>>> Or it is a wrong way to go? I would be grateful for your help.
>>>>>>>
>>>>>>> Regards,
>>>>>>> Arek
>>>>>
>


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
  2017-05-09 23:47             ` Andrew Duggan
@ 2017-05-10  9:36               ` Arek Burdach
  2017-05-11  9:48                 ` Martin Kepplinger
  0 siblings, 1 reply; 37+ messages in thread
From: Arek Burdach @ 2017-05-10  9:36 UTC (permalink / raw)
  To: Andrew Duggan, Benjamin Tissoires; +Cc: Stéphane Chatty, linux-input

Hi Andrew,

On 10.05.2017 01:47, Andrew Duggan wrote:
> HI Arek,
>
> On 05/09/2017 04:17 PM, Arek Burdach wrote:
>> Hi,
>>
>> I've tried described by you solution:
>>
>> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
>> index 37084b645785..81f271554b6c 100644
>> --- a/drivers/hid/hid-core.c
>> +++ b/drivers/hid/hid-core.c
>> @@ -2510,6 +2510,7 @@ static const struct hid_device_id 
>> hid_ignore_list[] = {
>
> You need to add this to the hid_have_special_driver[] and not the 
> hid_ignore_list[].
Nice score for me - two lines and one bug :-)

> But, if you do success in binding hid-rmi to a touchscreen it won't 
> work. The firmware between touchpads and touchscreens are different 
> enough that the hid-rmi driver will be looking for data which does not 
> exist in touchscreen's HID report. These differences also mean that it 
> really isn't a good idea to try to support touchscreens with hid-rmi. 
> It would actually result in more transactions and be less efficient 
> then simply using hid-multitouch. That's why hid-core checks for the 
> HID_SCAN_FLAG_GD_POINTER in an attempt to make sure it's binding to a 
> touchpad and not a touchscreen.
It was just like you predict. On rmi, after first tap on screen, hidraw 
produced infinite number of events and it is not usable anymore.


>>
>> On 09.05.2017 16:02, Benjamin Tissoires wrote:
>>> On Tue, May 9, 2017 at 2:51 PM, Arek Burdach 
>>> <arek.burdach@gmail.com> wrote:
>>>> On 09.05.2017 14:20, Benjamin Tissoires wrote:
>>>>> On Tue, May 9, 2017 at 11:20 AM, Arek Burdach 
>>>>> <arek.burdach@gmail.com>
>>>>> wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Thank you for response.
>>>>>>
>>>>>> On 09.05.2017 10:35, Benjamin Tissoires wrote:
>>>>>>> On Sat, May 6, 2017 at 9:28 PM, Arek Burdach 
>>>>>>> <arek.burdach@gmail.com>
>>>>>>> wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> A week ago I've reported a bug:
>>>>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=195625 Is there 
>>>>>>>> anybody
>>>>>>>> that
>>>>>>>> can
>>>>>>>> help me with it?
>>>>>>> I can have a look at it.
>>>>>>> Please attach the full outputs of hid-recorder and evemu-record 
>>>>>>> in the
>>>>>>> bugs, or it'll be difficult for us to debug it.
>>>>>> I've attached full logs for two situations. More details in the 
>>>>>> issue.
>>>>> Thanks, looks like a firmware issue (I'll comment in the bug).
>>>> Sorry for my noob questions, but do you suggest that it can't be 
>>>> fixed by
>>>> changes in kernel modules and I need to report it to the manufacturer?
>>> Yes. Though Andrew, in CC, works for Synaptics and might give us 
>>> some pointers.
>>>
>>>> If it so, do you have an idea why it works well on Windows? Do they 
>>>> have
>>>> some strange hacks in their drivers?
>>> I have no ideas how well it works under Windows, and I have no ideas
>>> if there are some strange hacks in the Windows nor in the Syanptics
>>> driver (I would assume so).
>>>
>
> We don't provide any drivers for touchscreens on Windows. So I don't 
> know how Microsoft is handling a situation like this.
Do you know what should be changed in firmware to make hid-touchscreen 
driver works correctly? Or maybe you know someone who is responsible for 
firmware for this device and whom I can call to gather this information?

Cheers,
Arek

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
  2017-05-10  9:36               ` Arek Burdach
@ 2017-05-11  9:48                 ` Martin Kepplinger
  2017-05-11 10:12                   ` Arek Burdach
  0 siblings, 1 reply; 37+ messages in thread
From: Martin Kepplinger @ 2017-05-11  9:48 UTC (permalink / raw)
  To: Arek Burdach, Andrew Duggan, Benjamin Tissoires
  Cc: Stéphane Chatty, linux-input



On 2017-05-10 11:36, Arek Burdach wrote:
> Hi Andrew,
>
> On 10.05.2017 01:47, Andrew Duggan wrote:
>> HI Arek,
>>
>> On 05/09/2017 04:17 PM, Arek Burdach wrote:
>>> Hi,
>>>
>>> I've tried described by you solution:
>>>
>>> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
>>> index 37084b645785..81f271554b6c 100644
>>> --- a/drivers/hid/hid-core.c
>>> +++ b/drivers/hid/hid-core.c
>>> @@ -2510,6 +2510,7 @@ static const struct hid_device_id
>>> hid_ignore_list[] = {
>>
>> You need to add this to the hid_have_special_driver[] and not the
>> hid_ignore_list[].
> Nice score for me - two lines and one bug :-)
>
>> But, if you do success in binding hid-rmi to a touchscreen it won't
>> work. The firmware between touchpads and touchscreens are different
>> enough that the hid-rmi driver will be looking for data which does not
>> exist in touchscreen's HID report. These differences also mean that it
>> really isn't a good idea to try to support touchscreens with hid-rmi.
>> It would actually result in more transactions and be less efficient
>> then simply using hid-multitouch. That's why hid-core checks for the
>> HID_SCAN_FLAG_GD_POINTER in an attempt to make sure it's binding to a
>> touchpad and not a touchscreen.
> It was just like you predict. On rmi, after first tap on screen, hidraw
> produced infinite number of events and it is not usable anymore.
>
>
>>>
>>> On 09.05.2017 16:02, Benjamin Tissoires wrote:
>>>> On Tue, May 9, 2017 at 2:51 PM, Arek Burdach
>>>> <arek.burdach@gmail.com> wrote:
>>>>> On 09.05.2017 14:20, Benjamin Tissoires wrote:
>>>>>> On Tue, May 9, 2017 at 11:20 AM, Arek Burdach
>>>>>> <arek.burdach@gmail.com>
>>>>>> wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> Thank you for response.
>>>>>>>
>>>>>>> On 09.05.2017 10:35, Benjamin Tissoires wrote:
>>>>>>>> On Sat, May 6, 2017 at 9:28 PM, Arek Burdach
>>>>>>>> <arek.burdach@gmail.com>
>>>>>>>> wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> A week ago I've reported a bug:
>>>>>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=195625 Is there
>>>>>>>>> anybody
>>>>>>>>> that
>>>>>>>>> can
>>>>>>>>> help me with it?
>>>>>>>> I can have a look at it.
>>>>>>>> Please attach the full outputs of hid-recorder and evemu-record
>>>>>>>> in the
>>>>>>>> bugs, or it'll be difficult for us to debug it.
>>>>>>> I've attached full logs for two situations. More details in the
>>>>>>> issue.
>>>>>> Thanks, looks like a firmware issue (I'll comment in the bug).
>>>>> Sorry for my noob questions, but do you suggest that it can't be
>>>>> fixed by
>>>>> changes in kernel modules and I need to report it to the manufacturer?
>>>> Yes. Though Andrew, in CC, works for Synaptics and might give us
>>>> some pointers.
>>>>
>>>>> If it so, do you have an idea why it works well on Windows? Do they
>>>>> have
>>>>> some strange hacks in their drivers?
>>>> I have no ideas how well it works under Windows, and I have no ideas
>>>> if there are some strange hacks in the Windows nor in the Syanptics
>>>> driver (I would assume so).
>>>>
>>
>> We don't provide any drivers for touchscreens on Windows. So I don't
>> know how Microsoft is handling a situation like this.
> Do you know what should be changed in firmware to make hid-touchscreen
> driver works correctly? Or maybe you know someone who is responsible for
> firmware for this device and whom I can call to gather this information?
>

In case there *really* is broken firmware out there, we can specifically
identify via struct input_id's version number for example, I want to
point out that I would accept adding a workaround in tslib's input-raw
module ( http://tslib.org ) if it won't be done in the kernel.

So, in case you can and want to use tslib as a workaround here, feel
free to have a look and send the patches that make input-raw.c work for
you over there.

Again, only if it won't be done in-kernel.

thanks
                           martin


________________________________

Ginzinger electronic systems GmbH
Gewerbegebiet Pirath 16
4952 Weng im Innkreis
www.ginzinger.com

Firmenbuchnummer: FN 364958d
Firmenbuchgericht: Ried im Innkreis
UID-Nr.: ATU66521089


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
  2017-05-11  9:48                 ` Martin Kepplinger
@ 2017-05-11 10:12                   ` Arek Burdach
  2017-05-11 11:22                     ` Martin Kepplinger
  0 siblings, 1 reply; 37+ messages in thread
From: Arek Burdach @ 2017-05-11 10:12 UTC (permalink / raw)
  To: Martin Kepplinger, Andrew Duggan, Benjamin Tissoires
  Cc: Stéphane Chatty, linux-input



On 11.05.2017 11:48, Martin Kepplinger wrote:
>
> On 2017-05-10 11:36, Arek Burdach wrote:
>> Hi Andrew,
>>
>> On 10.05.2017 01:47, Andrew Duggan wrote:
>>> HI Arek,
>>>
>>> On 05/09/2017 04:17 PM, Arek Burdach wrote:
>>>> Hi,
>>>>
>>>> I've tried described by you solution:
>>>>
>>>> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
>>>> index 37084b645785..81f271554b6c 100644
>>>> --- a/drivers/hid/hid-core.c
>>>> +++ b/drivers/hid/hid-core.c
>>>> @@ -2510,6 +2510,7 @@ static const struct hid_device_id
>>>> hid_ignore_list[] = {
>>> You need to add this to the hid_have_special_driver[] and not the
>>> hid_ignore_list[].
>> Nice score for me - two lines and one bug :-)
>>
>>> But, if you do success in binding hid-rmi to a touchscreen it won't
>>> work. The firmware between touchpads and touchscreens are different
>>> enough that the hid-rmi driver will be looking for data which does not
>>> exist in touchscreen's HID report. These differences also mean that it
>>> really isn't a good idea to try to support touchscreens with hid-rmi.
>>> It would actually result in more transactions and be less efficient
>>> then simply using hid-multitouch. That's why hid-core checks for the
>>> HID_SCAN_FLAG_GD_POINTER in an attempt to make sure it's binding to a
>>> touchpad and not a touchscreen.
>> It was just like you predict. On rmi, after first tap on screen, hidraw
>> produced infinite number of events and it is not usable anymore.
>>
>>
>>>> On 09.05.2017 16:02, Benjamin Tissoires wrote:
>>>>> On Tue, May 9, 2017 at 2:51 PM, Arek Burdach
>>>>> <arek.burdach@gmail.com> wrote:
>>>>>> On 09.05.2017 14:20, Benjamin Tissoires wrote:
>>>>>>> On Tue, May 9, 2017 at 11:20 AM, Arek Burdach
>>>>>>> <arek.burdach@gmail.com>
>>>>>>> wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> Thank you for response.
>>>>>>>>
>>>>>>>> On 09.05.2017 10:35, Benjamin Tissoires wrote:
>>>>>>>>> On Sat, May 6, 2017 at 9:28 PM, Arek Burdach
>>>>>>>>> <arek.burdach@gmail.com>
>>>>>>>>> wrote:
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> A week ago I've reported a bug:
>>>>>>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=195625 Is there
>>>>>>>>>> anybody
>>>>>>>>>> that
>>>>>>>>>> can
>>>>>>>>>> help me with it?
>>>>>>>>> I can have a look at it.
>>>>>>>>> Please attach the full outputs of hid-recorder and evemu-record
>>>>>>>>> in the
>>>>>>>>> bugs, or it'll be difficult for us to debug it.
>>>>>>>> I've attached full logs for two situations. More details in the
>>>>>>>> issue.
>>>>>>> Thanks, looks like a firmware issue (I'll comment in the bug).
>>>>>> Sorry for my noob questions, but do you suggest that it can't be
>>>>>> fixed by
>>>>>> changes in kernel modules and I need to report it to the manufacturer?
>>>>> Yes. Though Andrew, in CC, works for Synaptics and might give us
>>>>> some pointers.
>>>>>
>>>>>> If it so, do you have an idea why it works well on Windows? Do they
>>>>>> have
>>>>>> some strange hacks in their drivers?
>>>>> I have no ideas how well it works under Windows, and I have no ideas
>>>>> if there are some strange hacks in the Windows nor in the Syanptics
>>>>> driver (I would assume so).
>>>>>
>>> We don't provide any drivers for touchscreens on Windows. So I don't
>>> know how Microsoft is handling a situation like this.
>> Do you know what should be changed in firmware to make hid-touchscreen
>> driver works correctly? Or maybe you know someone who is responsible for
>> firmware for this device and whom I can call to gather this information?
>>
> In case there *really* is broken firmware out there, we can specifically
> identify via struct input_id's version number for example,
I thought that Benjamin identified this as a broken firmware. I've 
attached hidraw log in the issue and there is no release event, so it 
looks like a firmware bug. How do you suggest to handle this situation 
in kernel? We can identify the device but what to do next if we have no 
information if user released finger or not?

> I want to
> point out that I would accept adding a workaround in tslib's input-raw
> module ( http://tslib.org ) if it won't be done in the kernel.
>
> So, in case you can and want to use tslib as a workaround here, feel
> free to have a look and send the patches that make input-raw.c work for
> you over there.
I want to be as handy as I can but I'm not sure how tslib could help in 
this situation. If we have too much data, it can filter out unnecessary 
events but I don't think that it can help when there is lack of events 
or I'm missing something?
> Again, only if it won't be done in-kernel.
>
> thanks
>                             martin
>
>
> ________________________________
>
> Ginzinger electronic systems GmbH
> Gewerbegebiet Pirath 16
> 4952 Weng im Innkreis
> www.ginzinger.com
>
> Firmenbuchnummer: FN 364958d
> Firmenbuchgericht: Ried im Innkreis
> UID-Nr.: ATU66521089
>


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
  2017-05-11 10:12                   ` Arek Burdach
@ 2017-05-11 11:22                     ` Martin Kepplinger
  2017-05-11 11:28                       ` Benjamin Tissoires
                                         ` (2 more replies)
  0 siblings, 3 replies; 37+ messages in thread
From: Martin Kepplinger @ 2017-05-11 11:22 UTC (permalink / raw)
  To: Arek Burdach, Andrew Duggan, Benjamin Tissoires
  Cc: Stéphane Chatty, linux-input



On 2017-05-11 12:12, Arek Burdach wrote:
>
>
> On 11.05.2017 11:48, Martin Kepplinger wrote:
>>
>> On 2017-05-10 11:36, Arek Burdach wrote:
>>> Hi Andrew,
>>>
>>> On 10.05.2017 01:47, Andrew Duggan wrote:
>>>> HI Arek,
>>>>
>>>> On 05/09/2017 04:17 PM, Arek Burdach wrote:
>>>>> Hi,
>>>>>
>>>>> I've tried described by you solution:
>>>>>
>>>>> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
>>>>> index 37084b645785..81f271554b6c 100644
>>>>> --- a/drivers/hid/hid-core.c
>>>>> +++ b/drivers/hid/hid-core.c
>>>>> @@ -2510,6 +2510,7 @@ static const struct hid_device_id
>>>>> hid_ignore_list[] = {
>>>> You need to add this to the hid_have_special_driver[] and not the
>>>> hid_ignore_list[].
>>> Nice score for me - two lines and one bug :-)
>>>
>>>> But, if you do success in binding hid-rmi to a touchscreen it won't
>>>> work. The firmware between touchpads and touchscreens are different
>>>> enough that the hid-rmi driver will be looking for data which does not
>>>> exist in touchscreen's HID report. These differences also mean that it
>>>> really isn't a good idea to try to support touchscreens with hid-rmi.
>>>> It would actually result in more transactions and be less efficient
>>>> then simply using hid-multitouch. That's why hid-core checks for the
>>>> HID_SCAN_FLAG_GD_POINTER in an attempt to make sure it's binding to a
>>>> touchpad and not a touchscreen.
>>> It was just like you predict. On rmi, after first tap on screen, hidraw
>>> produced infinite number of events and it is not usable anymore.
>>>
>>>
>>>>> On 09.05.2017 16:02, Benjamin Tissoires wrote:
>>>>>> On Tue, May 9, 2017 at 2:51 PM, Arek Burdach
>>>>>> <arek.burdach@gmail.com> wrote:
>>>>>>> On 09.05.2017 14:20, Benjamin Tissoires wrote:
>>>>>>>> On Tue, May 9, 2017 at 11:20 AM, Arek Burdach
>>>>>>>> <arek.burdach@gmail.com>
>>>>>>>> wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> Thank you for response.
>>>>>>>>>
>>>>>>>>> On 09.05.2017 10:35, Benjamin Tissoires wrote:
>>>>>>>>>> On Sat, May 6, 2017 at 9:28 PM, Arek Burdach
>>>>>>>>>> <arek.burdach@gmail.com>
>>>>>>>>>> wrote:
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>> A week ago I've reported a bug:
>>>>>>>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=195625 Is there
>>>>>>>>>>> anybody
>>>>>>>>>>> that
>>>>>>>>>>> can
>>>>>>>>>>> help me with it?
>>>>>>>>>> I can have a look at it.
>>>>>>>>>> Please attach the full outputs of hid-recorder and evemu-record
>>>>>>>>>> in the
>>>>>>>>>> bugs, or it'll be difficult for us to debug it.
>>>>>>>>> I've attached full logs for two situations. More details in the
>>>>>>>>> issue.
>>>>>>>> Thanks, looks like a firmware issue (I'll comment in the bug).
>>>>>>> Sorry for my noob questions, but do you suggest that it can't be
>>>>>>> fixed by
>>>>>>> changes in kernel modules and I need to report it to the
>>>>>>> manufacturer?
>>>>>> Yes. Though Andrew, in CC, works for Synaptics and might give us
>>>>>> some pointers.
>>>>>>
>>>>>>> If it so, do you have an idea why it works well on Windows? Do they
>>>>>>> have
>>>>>>> some strange hacks in their drivers?
>>>>>> I have no ideas how well it works under Windows, and I have no ideas
>>>>>> if there are some strange hacks in the Windows nor in the Syanptics
>>>>>> driver (I would assume so).
>>>>>>
>>>> We don't provide any drivers for touchscreens on Windows. So I don't
>>>> know how Microsoft is handling a situation like this.
>>> Do you know what should be changed in firmware to make hid-touchscreen
>>> driver works correctly? Or maybe you know someone who is responsible for
>>> firmware for this device and whom I can call to gather this information?
>>>
>> In case there *really* is broken firmware out there, we can specifically
>> identify via struct input_id's version number for example,
> I thought that Benjamin identified this as a broken firmware. I've
> attached hidraw log in the issue and there is no release event, so it
> looks like a firmware bug. How do you suggest to handle this situation
> in kernel? We can identify the device but what to do next if we have no
> information if user released finger or not?
>
>> I want to
>> point out that I would accept adding a workaround in tslib's input-raw
>> module ( http://tslib.org ) if it won't be done in the kernel.
>>
>> So, in case you can and want to use tslib as a workaround here, feel
>> free to have a look and send the patches that make input-raw.c work for
>> you over there.
> I want to be as handy as I can but I'm not sure how tslib could help in
> this situation. If we have too much data, it can filter out unnecessary
> events but I don't think that it can help when there is lack of events
> or I'm missing something?

Might as well be, I might not have thought it all through, but in
tslib's module_raw input you can can get totally creative: Why not start
*every* sync frame with BTN_TOUCH 1 and end it with BTN_TOUCH 0? You
*are* able to add stuff. Filters don't usually do it though.

                        martin


________________________________

Ginzinger electronic systems GmbH
Gewerbegebiet Pirath 16
4952 Weng im Innkreis
www.ginzinger.com

Firmenbuchnummer: FN 364958d
Firmenbuchgericht: Ried im Innkreis
UID-Nr.: ATU66521089


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
  2017-05-11 11:22                     ` Martin Kepplinger
@ 2017-05-11 11:28                       ` Benjamin Tissoires
  2017-05-11 11:44                         ` Arek Burdach
  2017-05-11 11:47                         ` Martin Kepplinger
  2017-05-11 11:36                       ` Arek Burdach
  2017-05-11 11:36                       ` Martin Kepplinger
  2 siblings, 2 replies; 37+ messages in thread
From: Benjamin Tissoires @ 2017-05-11 11:28 UTC (permalink / raw)
  To: Martin Kepplinger
  Cc: Arek Burdach, Andrew Duggan, Stéphane Chatty, linux-input

On Thu, May 11, 2017 at 1:22 PM, Martin Kepplinger
<martin.kepplinger@ginzinger.com> wrote:
>
>
> On 2017-05-11 12:12, Arek Burdach wrote:
>>
>>
>> On 11.05.2017 11:48, Martin Kepplinger wrote:
>>>
>>> On 2017-05-10 11:36, Arek Burdach wrote:
>>>> Hi Andrew,
>>>>
>>>> On 10.05.2017 01:47, Andrew Duggan wrote:
>>>>> HI Arek,
>>>>>
>>>>> On 05/09/2017 04:17 PM, Arek Burdach wrote:
>>>>>> Hi,
>>>>>>
>>>>>> I've tried described by you solution:
>>>>>>
>>>>>> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
>>>>>> index 37084b645785..81f271554b6c 100644
>>>>>> --- a/drivers/hid/hid-core.c
>>>>>> +++ b/drivers/hid/hid-core.c
>>>>>> @@ -2510,6 +2510,7 @@ static const struct hid_device_id
>>>>>> hid_ignore_list[] = {
>>>>> You need to add this to the hid_have_special_driver[] and not the
>>>>> hid_ignore_list[].
>>>> Nice score for me - two lines and one bug :-)
>>>>
>>>>> But, if you do success in binding hid-rmi to a touchscreen it won't
>>>>> work. The firmware between touchpads and touchscreens are different
>>>>> enough that the hid-rmi driver will be looking for data which does not
>>>>> exist in touchscreen's HID report. These differences also mean that it
>>>>> really isn't a good idea to try to support touchscreens with hid-rmi.
>>>>> It would actually result in more transactions and be less efficient
>>>>> then simply using hid-multitouch. That's why hid-core checks for the
>>>>> HID_SCAN_FLAG_GD_POINTER in an attempt to make sure it's binding to a
>>>>> touchpad and not a touchscreen.
>>>> It was just like you predict. On rmi, after first tap on screen, hidraw
>>>> produced infinite number of events and it is not usable anymore.
>>>>
>>>>
>>>>>> On 09.05.2017 16:02, Benjamin Tissoires wrote:
>>>>>>> On Tue, May 9, 2017 at 2:51 PM, Arek Burdach
>>>>>>> <arek.burdach@gmail.com> wrote:
>>>>>>>> On 09.05.2017 14:20, Benjamin Tissoires wrote:
>>>>>>>>> On Tue, May 9, 2017 at 11:20 AM, Arek Burdach
>>>>>>>>> <arek.burdach@gmail.com>
>>>>>>>>> wrote:
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> Thank you for response.
>>>>>>>>>>
>>>>>>>>>> On 09.05.2017 10:35, Benjamin Tissoires wrote:
>>>>>>>>>>> On Sat, May 6, 2017 at 9:28 PM, Arek Burdach
>>>>>>>>>>> <arek.burdach@gmail.com>
>>>>>>>>>>> wrote:
>>>>>>>>>>>> Hi,
>>>>>>>>>>>>
>>>>>>>>>>>> A week ago I've reported a bug:
>>>>>>>>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=195625 Is there
>>>>>>>>>>>> anybody
>>>>>>>>>>>> that
>>>>>>>>>>>> can
>>>>>>>>>>>> help me with it?
>>>>>>>>>>> I can have a look at it.
>>>>>>>>>>> Please attach the full outputs of hid-recorder and evemu-record
>>>>>>>>>>> in the
>>>>>>>>>>> bugs, or it'll be difficult for us to debug it.
>>>>>>>>>> I've attached full logs for two situations. More details in the
>>>>>>>>>> issue.
>>>>>>>>> Thanks, looks like a firmware issue (I'll comment in the bug).
>>>>>>>> Sorry for my noob questions, but do you suggest that it can't be
>>>>>>>> fixed by
>>>>>>>> changes in kernel modules and I need to report it to the
>>>>>>>> manufacturer?
>>>>>>> Yes. Though Andrew, in CC, works for Synaptics and might give us
>>>>>>> some pointers.
>>>>>>>
>>>>>>>> If it so, do you have an idea why it works well on Windows? Do they
>>>>>>>> have
>>>>>>>> some strange hacks in their drivers?
>>>>>>> I have no ideas how well it works under Windows, and I have no ideas
>>>>>>> if there are some strange hacks in the Windows nor in the Syanptics
>>>>>>> driver (I would assume so).
>>>>>>>
>>>>> We don't provide any drivers for touchscreens on Windows. So I don't
>>>>> know how Microsoft is handling a situation like this.
>>>> Do you know what should be changed in firmware to make hid-touchscreen
>>>> driver works correctly? Or maybe you know someone who is responsible for
>>>> firmware for this device and whom I can call to gather this information?
>>>>
>>> In case there *really* is broken firmware out there, we can specifically
>>> identify via struct input_id's version number for example,
>> I thought that Benjamin identified this as a broken firmware. I've
>> attached hidraw log in the issue and there is no release event, so it
>> looks like a firmware bug. How do you suggest to handle this situation
>> in kernel? We can identify the device but what to do next if we have no
>> information if user released finger or not?
>>
>>> I want to
>>> point out that I would accept adding a workaround in tslib's input-raw
>>> module ( http://tslib.org ) if it won't be done in the kernel.
>>>
>>> So, in case you can and want to use tslib as a workaround here, feel
>>> free to have a look and send the patches that make input-raw.c work for
>>> you over there.
>> I want to be as handy as I can but I'm not sure how tslib could help in
>> this situation. If we have too much data, it can filter out unnecessary
>> events but I don't think that it can help when there is lack of events
>> or I'm missing something?
>
> Might as well be, I might not have thought it all through, but in
> tslib's module_raw input you can can get totally creative: Why not start
> *every* sync frame with BTN_TOUCH 1 and end it with BTN_TOUCH 0? You
> *are* able to add stuff. Filters don't usually do it though.

Ouch, please don't. You'll send an endless click/release sequence
which will break drag and drop, double click and so on.

Also, this won't solve the issue because the multitouch slot will not
be released.

The only solution (which i believe the Windows driver does, but I
believed only for Windows 7 compatible touchscreen), is to arm a timer
for each slot, and when you don't receive an update after let's say 5
seconds, you release the slot.

It's awful and I always have been against adding such pain in the
hid-multitouch driver.

Cheers,
Benjamin

>
>                         martin
>
>
> ________________________________
>
> Ginzinger electronic systems GmbH
> Gewerbegebiet Pirath 16
> 4952 Weng im Innkreis
> www.ginzinger.com
>
> Firmenbuchnummer: FN 364958d
> Firmenbuchgericht: Ried im Innkreis
> UID-Nr.: ATU66521089
>

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
  2017-05-11 11:22                     ` Martin Kepplinger
  2017-05-11 11:28                       ` Benjamin Tissoires
@ 2017-05-11 11:36                       ` Arek Burdach
  2017-05-11 11:36                       ` Martin Kepplinger
  2 siblings, 0 replies; 37+ messages in thread
From: Arek Burdach @ 2017-05-11 11:36 UTC (permalink / raw)
  To: Martin Kepplinger, Andrew Duggan, Benjamin Tissoires
  Cc: Stéphane Chatty, linux-input



On 11.05.2017 13:22, Martin Kepplinger wrote:
>
> On 2017-05-11 12:12, Arek Burdach wrote:
>>
>> On 11.05.2017 11:48, Martin Kepplinger wrote:
>>> On 2017-05-10 11:36, Arek Burdach wrote:
>>>> Hi Andrew,
>>>>
>>>> On 10.05.2017 01:47, Andrew Duggan wrote:
>>>>> HI Arek,
>>>>>
>>>>> On 05/09/2017 04:17 PM, Arek Burdach wrote:
>>>>>> Hi,
>>>>>>
>>>>>> I've tried described by you solution:
>>>>>>
>>>>>> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
>>>>>> index 37084b645785..81f271554b6c 100644
>>>>>> --- a/drivers/hid/hid-core.c
>>>>>> +++ b/drivers/hid/hid-core.c
>>>>>> @@ -2510,6 +2510,7 @@ static const struct hid_device_id
>>>>>> hid_ignore_list[] = {
>>>>> You need to add this to the hid_have_special_driver[] and not the
>>>>> hid_ignore_list[].
>>>> Nice score for me - two lines and one bug :-)
>>>>
>>>>> But, if you do success in binding hid-rmi to a touchscreen it won't
>>>>> work. The firmware between touchpads and touchscreens are different
>>>>> enough that the hid-rmi driver will be looking for data which does not
>>>>> exist in touchscreen's HID report. These differences also mean that it
>>>>> really isn't a good idea to try to support touchscreens with hid-rmi.
>>>>> It would actually result in more transactions and be less efficient
>>>>> then simply using hid-multitouch. That's why hid-core checks for the
>>>>> HID_SCAN_FLAG_GD_POINTER in an attempt to make sure it's binding to a
>>>>> touchpad and not a touchscreen.
>>>> It was just like you predict. On rmi, after first tap on screen, hidraw
>>>> produced infinite number of events and it is not usable anymore.
>>>>
>>>>
>>>>>> On 09.05.2017 16:02, Benjamin Tissoires wrote:
>>>>>>> On Tue, May 9, 2017 at 2:51 PM, Arek Burdach
>>>>>>> <arek.burdach@gmail.com> wrote:
>>>>>>>> On 09.05.2017 14:20, Benjamin Tissoires wrote:
>>>>>>>>> On Tue, May 9, 2017 at 11:20 AM, Arek Burdach
>>>>>>>>> <arek.burdach@gmail.com>
>>>>>>>>> wrote:
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> Thank you for response.
>>>>>>>>>>
>>>>>>>>>> On 09.05.2017 10:35, Benjamin Tissoires wrote:
>>>>>>>>>>> On Sat, May 6, 2017 at 9:28 PM, Arek Burdach
>>>>>>>>>>> <arek.burdach@gmail.com>
>>>>>>>>>>> wrote:
>>>>>>>>>>>> Hi,
>>>>>>>>>>>>
>>>>>>>>>>>> A week ago I've reported a bug:
>>>>>>>>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=195625 Is there
>>>>>>>>>>>> anybody
>>>>>>>>>>>> that
>>>>>>>>>>>> can
>>>>>>>>>>>> help me with it?
>>>>>>>>>>> I can have a look at it.
>>>>>>>>>>> Please attach the full outputs of hid-recorder and evemu-record
>>>>>>>>>>> in the
>>>>>>>>>>> bugs, or it'll be difficult for us to debug it.
>>>>>>>>>> I've attached full logs for two situations. More details in the
>>>>>>>>>> issue.
>>>>>>>>> Thanks, looks like a firmware issue (I'll comment in the bug).
>>>>>>>> Sorry for my noob questions, but do you suggest that it can't be
>>>>>>>> fixed by
>>>>>>>> changes in kernel modules and I need to report it to the
>>>>>>>> manufacturer?
>>>>>>> Yes. Though Andrew, in CC, works for Synaptics and might give us
>>>>>>> some pointers.
>>>>>>>
>>>>>>>> If it so, do you have an idea why it works well on Windows? Do they
>>>>>>>> have
>>>>>>>> some strange hacks in their drivers?
>>>>>>> I have no ideas how well it works under Windows, and I have no ideas
>>>>>>> if there are some strange hacks in the Windows nor in the Syanptics
>>>>>>> driver (I would assume so).
>>>>>>>
>>>>> We don't provide any drivers for touchscreens on Windows. So I don't
>>>>> know how Microsoft is handling a situation like this.
>>>> Do you know what should be changed in firmware to make hid-touchscreen
>>>> driver works correctly? Or maybe you know someone who is responsible for
>>>> firmware for this device and whom I can call to gather this information?
>>>>
>>> In case there *really* is broken firmware out there, we can specifically
>>> identify via struct input_id's version number for example,
>> I thought that Benjamin identified this as a broken firmware. I've
>> attached hidraw log in the issue and there is no release event, so it
>> looks like a firmware bug. How do you suggest to handle this situation
>> in kernel? We can identify the device but what to do next if we have no
>> information if user released finger or not?
>>
>>> I want to
>>> point out that I would accept adding a workaround in tslib's input-raw
>>> module ( http://tslib.org ) if it won't be done in the kernel.
>>>
>>> So, in case you can and want to use tslib as a workaround here, feel
>>> free to have a look and send the patches that make input-raw.c work for
>>> you over there.
>> I want to be as handy as I can but I'm not sure how tslib could help in
>> this situation. If we have too much data, it can filter out unnecessary
>> events but I don't think that it can help when there is lack of events
>> or I'm missing something?
> Might as well be, I might not have thought it all through, but in
> tslib's module_raw input you can can get totally creative: Why not start
> *every* sync frame with BTN_TOUCH 1 and end it with BTN_TOUCH 0? You
> *are* able to add stuff. Filters don't usually do it though.
If I end every BTN_TOUCH 1 with BTN_TOUCH 0 after a while, we won't be 
able to do real drags. Of course we can introduce timeout for cursor 
move events, but this timeout would be extremely long because users 
quite often put finger on touchscreen and after a longer delay start 
dragging. I'm not sure that this workaround would be better than current 
situation.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
  2017-05-11 11:22                     ` Martin Kepplinger
  2017-05-11 11:28                       ` Benjamin Tissoires
  2017-05-11 11:36                       ` Arek Burdach
@ 2017-05-11 11:36                       ` Martin Kepplinger
  2 siblings, 0 replies; 37+ messages in thread
From: Martin Kepplinger @ 2017-05-11 11:36 UTC (permalink / raw)
  To: Arek Burdach, Andrew Duggan, Benjamin Tissoires
  Cc: Stéphane Chatty, linux-input



On 2017-05-11 13:22, Martin Kepplinger wrote:
>
>
> On 2017-05-11 12:12, Arek Burdach wrote:
>>
>>
>> On 11.05.2017 11:48, Martin Kepplinger wrote:
>>>
>>> On 2017-05-10 11:36, Arek Burdach wrote:
>>>> Hi Andrew,
>>>>
>>>> On 10.05.2017 01:47, Andrew Duggan wrote:
>>>>> HI Arek,
>>>>>
>>>>> On 05/09/2017 04:17 PM, Arek Burdach wrote:
>>>>>> Hi,
>>>>>>
>>>>>> I've tried described by you solution:
>>>>>>
>>>>>> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
>>>>>> index 37084b645785..81f271554b6c 100644
>>>>>> --- a/drivers/hid/hid-core.c
>>>>>> +++ b/drivers/hid/hid-core.c
>>>>>> @@ -2510,6 +2510,7 @@ static const struct hid_device_id
>>>>>> hid_ignore_list[] = {
>>>>> You need to add this to the hid_have_special_driver[] and not the
>>>>> hid_ignore_list[].
>>>> Nice score for me - two lines and one bug :-)
>>>>
>>>>> But, if you do success in binding hid-rmi to a touchscreen it won't
>>>>> work. The firmware between touchpads and touchscreens are different
>>>>> enough that the hid-rmi driver will be looking for data which does not
>>>>> exist in touchscreen's HID report. These differences also mean that it
>>>>> really isn't a good idea to try to support touchscreens with hid-rmi.
>>>>> It would actually result in more transactions and be less efficient
>>>>> then simply using hid-multitouch. That's why hid-core checks for the
>>>>> HID_SCAN_FLAG_GD_POINTER in an attempt to make sure it's binding to a
>>>>> touchpad and not a touchscreen.
>>>> It was just like you predict. On rmi, after first tap on screen, hidraw
>>>> produced infinite number of events and it is not usable anymore.
>>>>
>>>>
>>>>>> On 09.05.2017 16:02, Benjamin Tissoires wrote:
>>>>>>> On Tue, May 9, 2017 at 2:51 PM, Arek Burdach
>>>>>>> <arek.burdach@gmail.com> wrote:
>>>>>>>> On 09.05.2017 14:20, Benjamin Tissoires wrote:
>>>>>>>>> On Tue, May 9, 2017 at 11:20 AM, Arek Burdach
>>>>>>>>> <arek.burdach@gmail.com>
>>>>>>>>> wrote:
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> Thank you for response.
>>>>>>>>>>
>>>>>>>>>> On 09.05.2017 10:35, Benjamin Tissoires wrote:
>>>>>>>>>>> On Sat, May 6, 2017 at 9:28 PM, Arek Burdach
>>>>>>>>>>> <arek.burdach@gmail.com>
>>>>>>>>>>> wrote:
>>>>>>>>>>>> Hi,
>>>>>>>>>>>>
>>>>>>>>>>>> A week ago I've reported a bug:
>>>>>>>>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=195625 Is there
>>>>>>>>>>>> anybody
>>>>>>>>>>>> that
>>>>>>>>>>>> can
>>>>>>>>>>>> help me with it?
>>>>>>>>>>> I can have a look at it.
>>>>>>>>>>> Please attach the full outputs of hid-recorder and evemu-record
>>>>>>>>>>> in the
>>>>>>>>>>> bugs, or it'll be difficult for us to debug it.
>>>>>>>>>> I've attached full logs for two situations. More details in the
>>>>>>>>>> issue.
>>>>>>>>> Thanks, looks like a firmware issue (I'll comment in the bug).
>>>>>>>> Sorry for my noob questions, but do you suggest that it can't be
>>>>>>>> fixed by
>>>>>>>> changes in kernel modules and I need to report it to the
>>>>>>>> manufacturer?
>>>>>>> Yes. Though Andrew, in CC, works for Synaptics and might give us
>>>>>>> some pointers.
>>>>>>>
>>>>>>>> If it so, do you have an idea why it works well on Windows? Do they
>>>>>>>> have
>>>>>>>> some strange hacks in their drivers?
>>>>>>> I have no ideas how well it works under Windows, and I have no ideas
>>>>>>> if there are some strange hacks in the Windows nor in the Syanptics
>>>>>>> driver (I would assume so).
>>>>>>>
>>>>> We don't provide any drivers for touchscreens on Windows. So I don't
>>>>> know how Microsoft is handling a situation like this.
>>>> Do you know what should be changed in firmware to make hid-touchscreen
>>>> driver works correctly? Or maybe you know someone who is responsible for
>>>> firmware for this device and whom I can call to gather this information?
>>>>
>>> In case there *really* is broken firmware out there, we can specifically
>>> identify via struct input_id's version number for example,
>> I thought that Benjamin identified this as a broken firmware. I've
>> attached hidraw log in the issue and there is no release event, so it
>> looks like a firmware bug. How do you suggest to handle this situation
>> in kernel? We can identify the device but what to do next if we have no
>> information if user released finger or not?
>>
>>> I want to
>>> point out that I would accept adding a workaround in tslib's input-raw
>>> module ( http://tslib.org ) if it won't be done in the kernel.
>>>
>>> So, in case you can and want to use tslib as a workaround here, feel
>>> free to have a look and send the patches that make input-raw.c work for
>>> you over there.
>> I want to be as handy as I can but I'm not sure how tslib could help in
>> this situation. If we have too much data, it can filter out unnecessary
>> events but I don't think that it can help when there is lack of events
>> or I'm missing something?
>
> Might as well be, I might not have thought it all through, but in
> tslib's module_raw input you can can get totally creative: Why not start
> *every* sync frame with BTN_TOUCH 1 and end it with BTN_TOUCH 0?

I definitely haven't thought it through :) One would have to say it
diffently, and probably work on it properly:

for each sync frame, assume you got "pen_down" first, for this device.
take that sample (ts_sample or ts_sample_mt) and add a 2nd sample
containing only pen_up.

You are in tslib API world then, and should check the documentation on
how to use it.

but don't sue me if I'm just misleading you :)

                          martin

________________________________

Ginzinger electronic systems GmbH
Gewerbegebiet Pirath 16
4952 Weng im Innkreis
www.ginzinger.com

Firmenbuchnummer: FN 364958d
Firmenbuchgericht: Ried im Innkreis
UID-Nr.: ATU66521089


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
  2017-05-11 11:28                       ` Benjamin Tissoires
@ 2017-05-11 11:44                         ` Arek Burdach
  2017-05-11 11:47                         ` Martin Kepplinger
  1 sibling, 0 replies; 37+ messages in thread
From: Arek Burdach @ 2017-05-11 11:44 UTC (permalink / raw)
  To: Benjamin Tissoires, Martin Kepplinger
  Cc: Andrew Duggan, Stéphane Chatty, linux-input



On 11.05.2017 13:28, Benjamin Tissoires wrote:
> On Thu, May 11, 2017 at 1:22 PM, Martin Kepplinger
> <martin.kepplinger@ginzinger.com> wrote:
>>
>> On 2017-05-11 12:12, Arek Burdach wrote:
>>>
>>> On 11.05.2017 11:48, Martin Kepplinger wrote:
>>>> On 2017-05-10 11:36, Arek Burdach wrote:
>>>>> Hi Andrew,
>>>>>
>>>>> On 10.05.2017 01:47, Andrew Duggan wrote:
>>>>>> HI Arek,
>>>>>>
>>>>>> On 05/09/2017 04:17 PM, Arek Burdach wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> I've tried described by you solution:
>>>>>>>
>>>>>>> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
>>>>>>> index 37084b645785..81f271554b6c 100644
>>>>>>> --- a/drivers/hid/hid-core.c
>>>>>>> +++ b/drivers/hid/hid-core.c
>>>>>>> @@ -2510,6 +2510,7 @@ static const struct hid_device_id
>>>>>>> hid_ignore_list[] = {
>>>>>> You need to add this to the hid_have_special_driver[] and not the
>>>>>> hid_ignore_list[].
>>>>> Nice score for me - two lines and one bug :-)
>>>>>
>>>>>> But, if you do success in binding hid-rmi to a touchscreen it won't
>>>>>> work. The firmware between touchpads and touchscreens are different
>>>>>> enough that the hid-rmi driver will be looking for data which does not
>>>>>> exist in touchscreen's HID report. These differences also mean that it
>>>>>> really isn't a good idea to try to support touchscreens with hid-rmi.
>>>>>> It would actually result in more transactions and be less efficient
>>>>>> then simply using hid-multitouch. That's why hid-core checks for the
>>>>>> HID_SCAN_FLAG_GD_POINTER in an attempt to make sure it's binding to a
>>>>>> touchpad and not a touchscreen.
>>>>> It was just like you predict. On rmi, after first tap on screen, hidraw
>>>>> produced infinite number of events and it is not usable anymore.
>>>>>
>>>>>
>>>>>>> On 09.05.2017 16:02, Benjamin Tissoires wrote:
>>>>>>>> On Tue, May 9, 2017 at 2:51 PM, Arek Burdach
>>>>>>>> <arek.burdach@gmail.com> wrote:
>>>>>>>>> On 09.05.2017 14:20, Benjamin Tissoires wrote:
>>>>>>>>>> On Tue, May 9, 2017 at 11:20 AM, Arek Burdach
>>>>>>>>>> <arek.burdach@gmail.com>
>>>>>>>>>> wrote:
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>> Thank you for response.
>>>>>>>>>>>
>>>>>>>>>>> On 09.05.2017 10:35, Benjamin Tissoires wrote:
>>>>>>>>>>>> On Sat, May 6, 2017 at 9:28 PM, Arek Burdach
>>>>>>>>>>>> <arek.burdach@gmail.com>
>>>>>>>>>>>> wrote:
>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>
>>>>>>>>>>>>> A week ago I've reported a bug:
>>>>>>>>>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=195625 Is there
>>>>>>>>>>>>> anybody
>>>>>>>>>>>>> that
>>>>>>>>>>>>> can
>>>>>>>>>>>>> help me with it?
>>>>>>>>>>>> I can have a look at it.
>>>>>>>>>>>> Please attach the full outputs of hid-recorder and evemu-record
>>>>>>>>>>>> in the
>>>>>>>>>>>> bugs, or it'll be difficult for us to debug it.
>>>>>>>>>>> I've attached full logs for two situations. More details in the
>>>>>>>>>>> issue.
>>>>>>>>>> Thanks, looks like a firmware issue (I'll comment in the bug).
>>>>>>>>> Sorry for my noob questions, but do you suggest that it can't be
>>>>>>>>> fixed by
>>>>>>>>> changes in kernel modules and I need to report it to the
>>>>>>>>> manufacturer?
>>>>>>>> Yes. Though Andrew, in CC, works for Synaptics and might give us
>>>>>>>> some pointers.
>>>>>>>>
>>>>>>>>> If it so, do you have an idea why it works well on Windows? Do they
>>>>>>>>> have
>>>>>>>>> some strange hacks in their drivers?
>>>>>>>> I have no ideas how well it works under Windows, and I have no ideas
>>>>>>>> if there are some strange hacks in the Windows nor in the Syanptics
>>>>>>>> driver (I would assume so).
>>>>>>>>
>>>>>> We don't provide any drivers for touchscreens on Windows. So I don't
>>>>>> know how Microsoft is handling a situation like this.
>>>>> Do you know what should be changed in firmware to make hid-touchscreen
>>>>> driver works correctly? Or maybe you know someone who is responsible for
>>>>> firmware for this device and whom I can call to gather this information?
>>>>>
>>>> In case there *really* is broken firmware out there, we can specifically
>>>> identify via struct input_id's version number for example,
>>> I thought that Benjamin identified this as a broken firmware. I've
>>> attached hidraw log in the issue and there is no release event, so it
>>> looks like a firmware bug. How do you suggest to handle this situation
>>> in kernel? We can identify the device but what to do next if we have no
>>> information if user released finger or not?
>>>
>>>> I want to
>>>> point out that I would accept adding a workaround in tslib's input-raw
>>>> module ( http://tslib.org ) if it won't be done in the kernel.
>>>>
>>>> So, in case you can and want to use tslib as a workaround here, feel
>>>> free to have a look and send the patches that make input-raw.c work for
>>>> you over there.
>>> I want to be as handy as I can but I'm not sure how tslib could help in
>>> this situation. If we have too much data, it can filter out unnecessary
>>> events but I don't think that it can help when there is lack of events
>>> or I'm missing something?
>> Might as well be, I might not have thought it all through, but in
>> tslib's module_raw input you can can get totally creative: Why not start
>> *every* sync frame with BTN_TOUCH 1 and end it with BTN_TOUCH 0? You
>> *are* able to add stuff. Filters don't usually do it though.
> Ouch, please don't. You'll send an endless click/release sequence
> which will break drag and drop, double click and so on.
>
> Also, this won't solve the issue because the multitouch slot will not
> be released.
>
> The only solution (which i believe the Windows driver does, but I
> believed only for Windows 7 compatible touchscreen), is to arm a timer
> for each slot, and when you don't receive an update after let's say 5
> seconds, you release the slot.
I also thought that it works like you describe on Windows, but I've 
verified that on Windows you can observe when release appear. There is 
an animation for it. This animation is played just after real release 
happens - not after constant timeout.

>
> It's awful and I always have been against adding such pain in the
> hid-multitouch driver.
>
> Cheers,
> Benjamin
>
>>                          martin
>>
>>
>> ________________________________
>>
>> Ginzinger electronic systems GmbH
>> Gewerbegebiet Pirath 16
>> 4952 Weng im Innkreis
>> www.ginzinger.com
>>
>> Firmenbuchnummer: FN 364958d
>> Firmenbuchgericht: Ried im Innkreis
>> UID-Nr.: ATU66521089
>>


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
  2017-05-11 11:28                       ` Benjamin Tissoires
  2017-05-11 11:44                         ` Arek Burdach
@ 2017-05-11 11:47                         ` Martin Kepplinger
  2017-05-11 12:28                           ` Benjamin Tissoires
  1 sibling, 1 reply; 37+ messages in thread
From: Martin Kepplinger @ 2017-05-11 11:47 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Arek Burdach, Andrew Duggan, Stéphane Chatty, linux-input



On 2017-05-11 13:28, Benjamin Tissoires wrote:
> On Thu, May 11, 2017 at 1:22 PM, Martin Kepplinger
> <martin.kepplinger@ginzinger.com> wrote:
>>
>>
>> On 2017-05-11 12:12, Arek Burdach wrote:
>>>
>>>
>>> On 11.05.2017 11:48, Martin Kepplinger wrote:
>>>>
>>>> On 2017-05-10 11:36, Arek Burdach wrote:
>>>>> Hi Andrew,
>>>>>
>>>>> On 10.05.2017 01:47, Andrew Duggan wrote:
>>>>>> HI Arek,
>>>>>>
>>>>>> On 05/09/2017 04:17 PM, Arek Burdach wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> I've tried described by you solution:
>>>>>>>
>>>>>>> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
>>>>>>> index 37084b645785..81f271554b6c 100644
>>>>>>> --- a/drivers/hid/hid-core.c
>>>>>>> +++ b/drivers/hid/hid-core.c
>>>>>>> @@ -2510,6 +2510,7 @@ static const struct hid_device_id
>>>>>>> hid_ignore_list[] = {
>>>>>> You need to add this to the hid_have_special_driver[] and not the
>>>>>> hid_ignore_list[].
>>>>> Nice score for me - two lines and one bug :-)
>>>>>
>>>>>> But, if you do success in binding hid-rmi to a touchscreen it won't
>>>>>> work. The firmware between touchpads and touchscreens are different
>>>>>> enough that the hid-rmi driver will be looking for data which does not
>>>>>> exist in touchscreen's HID report. These differences also mean that it
>>>>>> really isn't a good idea to try to support touchscreens with hid-rmi.
>>>>>> It would actually result in more transactions and be less efficient
>>>>>> then simply using hid-multitouch. That's why hid-core checks for the
>>>>>> HID_SCAN_FLAG_GD_POINTER in an attempt to make sure it's binding to a
>>>>>> touchpad and not a touchscreen.
>>>>> It was just like you predict. On rmi, after first tap on screen, hidraw
>>>>> produced infinite number of events and it is not usable anymore.
>>>>>
>>>>>
>>>>>>> On 09.05.2017 16:02, Benjamin Tissoires wrote:
>>>>>>>> On Tue, May 9, 2017 at 2:51 PM, Arek Burdach
>>>>>>>> <arek.burdach@gmail.com> wrote:
>>>>>>>>> On 09.05.2017 14:20, Benjamin Tissoires wrote:
>>>>>>>>>> On Tue, May 9, 2017 at 11:20 AM, Arek Burdach
>>>>>>>>>> <arek.burdach@gmail.com>
>>>>>>>>>> wrote:
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>> Thank you for response.
>>>>>>>>>>>
>>>>>>>>>>> On 09.05.2017 10:35, Benjamin Tissoires wrote:
>>>>>>>>>>>> On Sat, May 6, 2017 at 9:28 PM, Arek Burdach
>>>>>>>>>>>> <arek.burdach@gmail.com>
>>>>>>>>>>>> wrote:
>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>
>>>>>>>>>>>>> A week ago I've reported a bug:
>>>>>>>>>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=195625 Is there
>>>>>>>>>>>>> anybody
>>>>>>>>>>>>> that
>>>>>>>>>>>>> can
>>>>>>>>>>>>> help me with it?
>>>>>>>>>>>> I can have a look at it.
>>>>>>>>>>>> Please attach the full outputs of hid-recorder and evemu-record
>>>>>>>>>>>> in the
>>>>>>>>>>>> bugs, or it'll be difficult for us to debug it.
>>>>>>>>>>> I've attached full logs for two situations. More details in the
>>>>>>>>>>> issue.
>>>>>>>>>> Thanks, looks like a firmware issue (I'll comment in the bug).
>>>>>>>>> Sorry for my noob questions, but do you suggest that it can't be
>>>>>>>>> fixed by
>>>>>>>>> changes in kernel modules and I need to report it to the
>>>>>>>>> manufacturer?
>>>>>>>> Yes. Though Andrew, in CC, works for Synaptics and might give us
>>>>>>>> some pointers.
>>>>>>>>
>>>>>>>>> If it so, do you have an idea why it works well on Windows? Do they
>>>>>>>>> have
>>>>>>>>> some strange hacks in their drivers?
>>>>>>>> I have no ideas how well it works under Windows, and I have no ideas
>>>>>>>> if there are some strange hacks in the Windows nor in the Syanptics
>>>>>>>> driver (I would assume so).
>>>>>>>>
>>>>>> We don't provide any drivers for touchscreens on Windows. So I don't
>>>>>> know how Microsoft is handling a situation like this.
>>>>> Do you know what should be changed in firmware to make hid-touchscreen
>>>>> driver works correctly? Or maybe you know someone who is responsible for
>>>>> firmware for this device and whom I can call to gather this information?
>>>>>
>>>> In case there *really* is broken firmware out there, we can specifically
>>>> identify via struct input_id's version number for example,
>>> I thought that Benjamin identified this as a broken firmware. I've
>>> attached hidraw log in the issue and there is no release event, so it
>>> looks like a firmware bug. How do you suggest to handle this situation
>>> in kernel? We can identify the device but what to do next if we have no
>>> information if user released finger or not?
>>>
>>>> I want to
>>>> point out that I would accept adding a workaround in tslib's input-raw
>>>> module ( http://tslib.org ) if it won't be done in the kernel.
>>>>
>>>> So, in case you can and want to use tslib as a workaround here, feel
>>>> free to have a look and send the patches that make input-raw.c work for
>>>> you over there.
>>> I want to be as handy as I can but I'm not sure how tslib could help in
>>> this situation. If we have too much data, it can filter out unnecessary
>>> events but I don't think that it can help when there is lack of events
>>> or I'm missing something?
>>
>> Might as well be, I might not have thought it all through, but in
>> tslib's module_raw input you can can get totally creative: Why not start
>> *every* sync frame with BTN_TOUCH 1 and end it with BTN_TOUCH 0? You
>> *are* able to add stuff. Filters don't usually do it though.
>
> Ouch, please don't. You'll send an endless click/release sequence
> which will break drag and drop, double click and so on.
>

ah you're right, that's nonsense.

> Also, this won't solve the issue because the multitouch slot will not
> be released.
>
> The only solution (which i believe the Windows driver does, but I
> believed only for Windows 7 compatible touchscreen), is to arm a timer
> for each slot, and when you don't receive an update after let's say 5
> seconds, you release the slot.
>
> It's awful and I always have been against adding such pain in the
> hid-multitouch driver.

yes. still breaks "move after hold>5s" but would probably be the only
way to make this somewhat work.

________________________________

Ginzinger electronic systems GmbH
Gewerbegebiet Pirath 16
4952 Weng im Innkreis
www.ginzinger.com

Firmenbuchnummer: FN 364958d
Firmenbuchgericht: Ried im Innkreis
UID-Nr.: ATU66521089


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
  2017-05-11 11:47                         ` Martin Kepplinger
@ 2017-05-11 12:28                           ` Benjamin Tissoires
  2017-05-11 12:50                             ` Martin Kepplinger
  0 siblings, 1 reply; 37+ messages in thread
From: Benjamin Tissoires @ 2017-05-11 12:28 UTC (permalink / raw)
  To: Martin Kepplinger
  Cc: Arek Burdach, Andrew Duggan, Stéphane Chatty, linux-input

On Thu, May 11, 2017 at 1:47 PM, Martin Kepplinger
<martin.kepplinger@ginzinger.com> wrote:
>
>
> On 2017-05-11 13:28, Benjamin Tissoires wrote:
>> On Thu, May 11, 2017 at 1:22 PM, Martin Kepplinger
>> <martin.kepplinger@ginzinger.com> wrote:
>>>
>>>
>>> On 2017-05-11 12:12, Arek Burdach wrote:
>>>>
>>>>
>>>> On 11.05.2017 11:48, Martin Kepplinger wrote:
>>>>>
>>>>> On 2017-05-10 11:36, Arek Burdach wrote:
>>>>>> Hi Andrew,
>>>>>>
>>>>>> On 10.05.2017 01:47, Andrew Duggan wrote:
>>>>>>> HI Arek,
>>>>>>>
>>>>>>> On 05/09/2017 04:17 PM, Arek Burdach wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> I've tried described by you solution:
>>>>>>>>
>>>>>>>> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
>>>>>>>> index 37084b645785..81f271554b6c 100644
>>>>>>>> --- a/drivers/hid/hid-core.c
>>>>>>>> +++ b/drivers/hid/hid-core.c
>>>>>>>> @@ -2510,6 +2510,7 @@ static const struct hid_device_id
>>>>>>>> hid_ignore_list[] = {
>>>>>>> You need to add this to the hid_have_special_driver[] and not the
>>>>>>> hid_ignore_list[].
>>>>>> Nice score for me - two lines and one bug :-)
>>>>>>
>>>>>>> But, if you do success in binding hid-rmi to a touchscreen it won't
>>>>>>> work. The firmware between touchpads and touchscreens are different
>>>>>>> enough that the hid-rmi driver will be looking for data which does not
>>>>>>> exist in touchscreen's HID report. These differences also mean that it
>>>>>>> really isn't a good idea to try to support touchscreens with hid-rmi.
>>>>>>> It would actually result in more transactions and be less efficient
>>>>>>> then simply using hid-multitouch. That's why hid-core checks for the
>>>>>>> HID_SCAN_FLAG_GD_POINTER in an attempt to make sure it's binding to a
>>>>>>> touchpad and not a touchscreen.
>>>>>> It was just like you predict. On rmi, after first tap on screen, hidraw
>>>>>> produced infinite number of events and it is not usable anymore.
>>>>>>
>>>>>>
>>>>>>>> On 09.05.2017 16:02, Benjamin Tissoires wrote:
>>>>>>>>> On Tue, May 9, 2017 at 2:51 PM, Arek Burdach
>>>>>>>>> <arek.burdach@gmail.com> wrote:
>>>>>>>>>> On 09.05.2017 14:20, Benjamin Tissoires wrote:
>>>>>>>>>>> On Tue, May 9, 2017 at 11:20 AM, Arek Burdach
>>>>>>>>>>> <arek.burdach@gmail.com>
>>>>>>>>>>> wrote:
>>>>>>>>>>>> Hi,
>>>>>>>>>>>>
>>>>>>>>>>>> Thank you for response.
>>>>>>>>>>>>
>>>>>>>>>>>> On 09.05.2017 10:35, Benjamin Tissoires wrote:
>>>>>>>>>>>>> On Sat, May 6, 2017 at 9:28 PM, Arek Burdach
>>>>>>>>>>>>> <arek.burdach@gmail.com>
>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> A week ago I've reported a bug:
>>>>>>>>>>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=195625 Is there
>>>>>>>>>>>>>> anybody
>>>>>>>>>>>>>> that
>>>>>>>>>>>>>> can
>>>>>>>>>>>>>> help me with it?
>>>>>>>>>>>>> I can have a look at it.
>>>>>>>>>>>>> Please attach the full outputs of hid-recorder and evemu-record
>>>>>>>>>>>>> in the
>>>>>>>>>>>>> bugs, or it'll be difficult for us to debug it.
>>>>>>>>>>>> I've attached full logs for two situations. More details in the
>>>>>>>>>>>> issue.
>>>>>>>>>>> Thanks, looks like a firmware issue (I'll comment in the bug).
>>>>>>>>>> Sorry for my noob questions, but do you suggest that it can't be
>>>>>>>>>> fixed by
>>>>>>>>>> changes in kernel modules and I need to report it to the
>>>>>>>>>> manufacturer?
>>>>>>>>> Yes. Though Andrew, in CC, works for Synaptics and might give us
>>>>>>>>> some pointers.
>>>>>>>>>
>>>>>>>>>> If it so, do you have an idea why it works well on Windows? Do they
>>>>>>>>>> have
>>>>>>>>>> some strange hacks in their drivers?
>>>>>>>>> I have no ideas how well it works under Windows, and I have no ideas
>>>>>>>>> if there are some strange hacks in the Windows nor in the Syanptics
>>>>>>>>> driver (I would assume so).
>>>>>>>>>
>>>>>>> We don't provide any drivers for touchscreens on Windows. So I don't
>>>>>>> know how Microsoft is handling a situation like this.
>>>>>> Do you know what should be changed in firmware to make hid-touchscreen
>>>>>> driver works correctly? Or maybe you know someone who is responsible for
>>>>>> firmware for this device and whom I can call to gather this information?
>>>>>>
>>>>> In case there *really* is broken firmware out there, we can specifically
>>>>> identify via struct input_id's version number for example,
>>>> I thought that Benjamin identified this as a broken firmware. I've
>>>> attached hidraw log in the issue and there is no release event, so it
>>>> looks like a firmware bug. How do you suggest to handle this situation
>>>> in kernel? We can identify the device but what to do next if we have no
>>>> information if user released finger or not?
>>>>
>>>>> I want to
>>>>> point out that I would accept adding a workaround in tslib's input-raw
>>>>> module ( http://tslib.org ) if it won't be done in the kernel.
>>>>>
>>>>> So, in case you can and want to use tslib as a workaround here, feel
>>>>> free to have a look and send the patches that make input-raw.c work for
>>>>> you over there.
>>>> I want to be as handy as I can but I'm not sure how tslib could help in
>>>> this situation. If we have too much data, it can filter out unnecessary
>>>> events but I don't think that it can help when there is lack of events
>>>> or I'm missing something?
>>>
>>> Might as well be, I might not have thought it all through, but in
>>> tslib's module_raw input you can can get totally creative: Why not start
>>> *every* sync frame with BTN_TOUCH 1 and end it with BTN_TOUCH 0? You
>>> *are* able to add stuff. Filters don't usually do it though.
>>
>> Ouch, please don't. You'll send an endless click/release sequence
>> which will break drag and drop, double click and so on.
>>
>
> ah you're right, that's nonsense.
>
>> Also, this won't solve the issue because the multitouch slot will not
>> be released.
>>
>> The only solution (which i believe the Windows driver does, but I
>> believed only for Windows 7 compatible touchscreen), is to arm a timer
>> for each slot, and when you don't receive an update after let's say 5
>> seconds, you release the slot.
>>
>> It's awful and I always have been against adding such pain in the
>> hid-multitouch driver.
>
> yes. still breaks "move after hold>5s" but would probably be the only
> way to make this somewhat work.

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.

Cheers,
Benjamin

>
> ________________________________
>
> Ginzinger electronic systems GmbH
> Gewerbegebiet Pirath 16
> 4952 Weng im Innkreis
> www.ginzinger.com
>
> Firmenbuchnummer: FN 364958d
> Firmenbuchgericht: Ried im Innkreis
> UID-Nr.: ATU66521089
>

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
  2017-05-11 12:28                           ` Benjamin Tissoires
@ 2017-05-11 12:50                             ` Martin Kepplinger
  2017-05-11 14:30                               ` Arek Burdach
  0 siblings, 1 reply; 37+ messages in thread
From: Martin Kepplinger @ 2017-05-11 12:50 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Arek Burdach, Andrew Duggan, Stéphane Chatty, linux-input



On 2017-05-11 14:28, Benjamin Tissoires wrote:
> On Thu, May 11, 2017 at 1:47 PM, Martin Kepplinger
> <martin.kepplinger@ginzinger.com> wrote:
>>
>>
>> On 2017-05-11 13:28, Benjamin Tissoires wrote:
>>> On Thu, May 11, 2017 at 1:22 PM, Martin Kepplinger
>>> <martin.kepplinger@ginzinger.com> wrote:
>>>>
>>>>
>>>> On 2017-05-11 12:12, Arek Burdach wrote:
>>>>>
>>>>>
>>>>> On 11.05.2017 11:48, Martin Kepplinger wrote:
>>>>>>
>>>>>> On 2017-05-10 11:36, Arek Burdach wrote:
>>>>>>> Hi Andrew,
>>>>>>>
>>>>>>> On 10.05.2017 01:47, Andrew Duggan wrote:
>>>>>>>> HI Arek,
>>>>>>>>
>>>>>>>> On 05/09/2017 04:17 PM, Arek Burdach wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> I've tried described by you solution:
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
>>>>>>>>> index 37084b645785..81f271554b6c 100644
>>>>>>>>> --- a/drivers/hid/hid-core.c
>>>>>>>>> +++ b/drivers/hid/hid-core.c
>>>>>>>>> @@ -2510,6 +2510,7 @@ static const struct hid_device_id
>>>>>>>>> hid_ignore_list[] = {
>>>>>>>> You need to add this to the hid_have_special_driver[] and not the
>>>>>>>> hid_ignore_list[].
>>>>>>> Nice score for me - two lines and one bug :-)
>>>>>>>
>>>>>>>> But, if you do success in binding hid-rmi to a touchscreen it won't
>>>>>>>> work. The firmware between touchpads and touchscreens are different
>>>>>>>> enough that the hid-rmi driver will be looking for data which does not
>>>>>>>> exist in touchscreen's HID report. These differences also mean that it
>>>>>>>> really isn't a good idea to try to support touchscreens with hid-rmi.
>>>>>>>> It would actually result in more transactions and be less efficient
>>>>>>>> then simply using hid-multitouch. That's why hid-core checks for the
>>>>>>>> HID_SCAN_FLAG_GD_POINTER in an attempt to make sure it's binding to a
>>>>>>>> touchpad and not a touchscreen.
>>>>>>> It was just like you predict. On rmi, after first tap on screen, hidraw
>>>>>>> produced infinite number of events and it is not usable anymore.
>>>>>>>
>>>>>>>
>>>>>>>>> On 09.05.2017 16:02, Benjamin Tissoires wrote:
>>>>>>>>>> On Tue, May 9, 2017 at 2:51 PM, Arek Burdach
>>>>>>>>>> <arek.burdach@gmail.com> wrote:
>>>>>>>>>>> On 09.05.2017 14:20, Benjamin Tissoires wrote:
>>>>>>>>>>>> On Tue, May 9, 2017 at 11:20 AM, Arek Burdach
>>>>>>>>>>>> <arek.burdach@gmail.com>
>>>>>>>>>>>> wrote:
>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thank you for response.
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 09.05.2017 10:35, Benjamin Tissoires wrote:
>>>>>>>>>>>>>> On Sat, May 6, 2017 at 9:28 PM, Arek Burdach
>>>>>>>>>>>>>> <arek.burdach@gmail.com>
>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> A week ago I've reported a bug:
>>>>>>>>>>>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=195625 Is there
>>>>>>>>>>>>>>> anybody
>>>>>>>>>>>>>>> that
>>>>>>>>>>>>>>> can
>>>>>>>>>>>>>>> help me with it?
>>>>>>>>>>>>>> I can have a look at it.
>>>>>>>>>>>>>> Please attach the full outputs of hid-recorder and evemu-record
>>>>>>>>>>>>>> in the
>>>>>>>>>>>>>> bugs, or it'll be difficult for us to debug it.
>>>>>>>>>>>>> I've attached full logs for two situations. More details in the
>>>>>>>>>>>>> issue.
>>>>>>>>>>>> Thanks, looks like a firmware issue (I'll comment in the bug).
>>>>>>>>>>> Sorry for my noob questions, but do you suggest that it can't be
>>>>>>>>>>> fixed by
>>>>>>>>>>> changes in kernel modules and I need to report it to the
>>>>>>>>>>> manufacturer?
>>>>>>>>>> Yes. Though Andrew, in CC, works for Synaptics and might give us
>>>>>>>>>> some pointers.
>>>>>>>>>>
>>>>>>>>>>> If it so, do you have an idea why it works well on Windows? Do they
>>>>>>>>>>> have
>>>>>>>>>>> some strange hacks in their drivers?
>>>>>>>>>> I have no ideas how well it works under Windows, and I have no ideas
>>>>>>>>>> if there are some strange hacks in the Windows nor in the Syanptics
>>>>>>>>>> driver (I would assume so).
>>>>>>>>>>
>>>>>>>> We don't provide any drivers for touchscreens on Windows. So I don't
>>>>>>>> know how Microsoft is handling a situation like this.
>>>>>>> Do you know what should be changed in firmware to make hid-touchscreen
>>>>>>> driver works correctly? Or maybe you know someone who is responsible for
>>>>>>> firmware for this device and whom I can call to gather this information?
>>>>>>>
>>>>>> In case there *really* is broken firmware out there, we can specifically
>>>>>> identify via struct input_id's version number for example,
>>>>> I thought that Benjamin identified this as a broken firmware. I've
>>>>> attached hidraw log in the issue and there is no release event, so it
>>>>> looks like a firmware bug. How do you suggest to handle this situation
>>>>> in kernel? We can identify the device but what to do next if we have no
>>>>> information if user released finger or not?
>>>>>
>>>>>> I want to
>>>>>> point out that I would accept adding a workaround in tslib's input-raw
>>>>>> module ( http://tslib.org ) if it won't be done in the kernel.
>>>>>>
>>>>>> So, in case you can and want to use tslib as a workaround here, feel
>>>>>> free to have a look and send the patches that make input-raw.c work for
>>>>>> you over there.
>>>>> I want to be as handy as I can but I'm not sure how tslib could help in
>>>>> this situation. If we have too much data, it can filter out unnecessary
>>>>> events but I don't think that it can help when there is lack of events
>>>>> or I'm missing something?
>>>>
>>>> Might as well be, I might not have thought it all through, but in
>>>> tslib's module_raw input you can can get totally creative: Why not start
>>>> *every* sync frame with BTN_TOUCH 1 and end it with BTN_TOUCH 0? You
>>>> *are* able to add stuff. Filters don't usually do it though.
>>>
>>> Ouch, please don't. You'll send an endless click/release sequence
>>> which will break drag and drop, double click and so on.
>>>
>>
>> ah you're right, that's nonsense.
>>
>>> Also, this won't solve the issue because the multitouch slot will not
>>> be released.
>>>
>>> The only solution (which i believe the Windows driver does, but I
>>> believed only for Windows 7 compatible touchscreen), is to arm a timer
>>> for each slot, and when you don't receive an update after let's say 5
>>> seconds, you release the slot.
>>>
>>> It's awful and I always have been against adding such pain in the
>>> hid-multitouch driver.
>>
>> yes. still breaks "move after hold>5s" but would probably be the only
>> way to make this somewhat work.
>
> 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).

                            martin

________________________________

Ginzinger electronic systems GmbH
Gewerbegebiet Pirath 16
4952 Weng im Innkreis
www.ginzinger.com

Firmenbuchnummer: FN 364958d
Firmenbuchgericht: Ried im Innkreis
UID-Nr.: ATU66521089


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
  2017-05-11 12:50                             ` Martin Kepplinger
@ 2017-05-11 14:30                               ` Arek Burdach
  2017-05-11 14:45                                 ` Benjamin Tissoires
  0 siblings, 1 reply; 37+ messages in thread
From: Arek Burdach @ 2017-05-11 14:30 UTC (permalink / raw)
  To: Martin Kepplinger, Benjamin Tissoires
  Cc: Andrew Duggan, Stéphane Chatty, linux-input



On 11.05.2017 14:50, Martin Kepplinger wrote:
>
> On 2017-05-11 14:28, Benjamin Tissoires wrote:
>> On Thu, May 11, 2017 at 1:47 PM, Martin Kepplinger
>> <martin.kepplinger@ginzinger.com> wrote:
>>>
>>> On 2017-05-11 13:28, Benjamin Tissoires wrote:
>>>> On Thu, May 11, 2017 at 1:22 PM, Martin Kepplinger
>>>> <martin.kepplinger@ginzinger.com> wrote:
>>>>>
>>>>> On 2017-05-11 12:12, Arek Burdach wrote:
>>>>>>
>>>>>> On 11.05.2017 11:48, Martin Kepplinger wrote:
>>>>>>> On 2017-05-10 11:36, Arek Burdach wrote:
>>>>>>>> Hi Andrew,
>>>>>>>>
>>>>>>>> On 10.05.2017 01:47, Andrew Duggan wrote:
>>>>>>>>> HI Arek,
>>>>>>>>>
>>>>>>>>> On 05/09/2017 04:17 PM, Arek Burdach wrote:
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> I've tried described by you solution:
>>>>>>>>>>
>>>>>>>>>> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
>>>>>>>>>> index 37084b645785..81f271554b6c 100644
>>>>>>>>>> --- a/drivers/hid/hid-core.c
>>>>>>>>>> +++ b/drivers/hid/hid-core.c
>>>>>>>>>> @@ -2510,6 +2510,7 @@ static const struct hid_device_id
>>>>>>>>>> hid_ignore_list[] = {
>>>>>>>>> You need to add this to the hid_have_special_driver[] and not the
>>>>>>>>> hid_ignore_list[].
>>>>>>>> Nice score for me - two lines and one bug :-)
>>>>>>>>
>>>>>>>>> But, if you do success in binding hid-rmi to a touchscreen it won't
>>>>>>>>> work. The firmware between touchpads and touchscreens are different
>>>>>>>>> enough that the hid-rmi driver will be looking for data which does not
>>>>>>>>> exist in touchscreen's HID report. These differences also mean that it
>>>>>>>>> really isn't a good idea to try to support touchscreens with hid-rmi.
>>>>>>>>> It would actually result in more transactions and be less efficient
>>>>>>>>> then simply using hid-multitouch. That's why hid-core checks for the
>>>>>>>>> HID_SCAN_FLAG_GD_POINTER in an attempt to make sure it's binding to a
>>>>>>>>> touchpad and not a touchscreen.
>>>>>>>> It was just like you predict. On rmi, after first tap on screen, hidraw
>>>>>>>> produced infinite number of events and it is not usable anymore.
>>>>>>>>
>>>>>>>>
>>>>>>>>>> On 09.05.2017 16:02, Benjamin Tissoires wrote:
>>>>>>>>>>> On Tue, May 9, 2017 at 2:51 PM, Arek Burdach
>>>>>>>>>>> <arek.burdach@gmail.com> wrote:
>>>>>>>>>>>> On 09.05.2017 14:20, Benjamin Tissoires wrote:
>>>>>>>>>>>>> On Tue, May 9, 2017 at 11:20 AM, Arek Burdach
>>>>>>>>>>>>> <arek.burdach@gmail.com>
>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thank you for response.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 09.05.2017 10:35, Benjamin Tissoires wrote:
>>>>>>>>>>>>>>> On Sat, May 6, 2017 at 9:28 PM, Arek Burdach
>>>>>>>>>>>>>>> <arek.burdach@gmail.com>
>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> A week ago I've reported a bug:
>>>>>>>>>>>>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=195625 Is there
>>>>>>>>>>>>>>>> anybody
>>>>>>>>>>>>>>>> that
>>>>>>>>>>>>>>>> can
>>>>>>>>>>>>>>>> help me with it?
>>>>>>>>>>>>>>> I can have a look at it.
>>>>>>>>>>>>>>> Please attach the full outputs of hid-recorder and evemu-record
>>>>>>>>>>>>>>> in the
>>>>>>>>>>>>>>> bugs, or it'll be difficult for us to debug it.
>>>>>>>>>>>>>> I've attached full logs for two situations. More details in the
>>>>>>>>>>>>>> issue.
>>>>>>>>>>>>> Thanks, looks like a firmware issue (I'll comment in the bug).
>>>>>>>>>>>> Sorry for my noob questions, but do you suggest that it can't be
>>>>>>>>>>>> fixed by
>>>>>>>>>>>> changes in kernel modules and I need to report it to the
>>>>>>>>>>>> manufacturer?
>>>>>>>>>>> Yes. Though Andrew, in CC, works for Synaptics and might give us
>>>>>>>>>>> some pointers.
>>>>>>>>>>>
>>>>>>>>>>>> If it so, do you have an idea why it works well on Windows? Do they
>>>>>>>>>>>> have
>>>>>>>>>>>> some strange hacks in their drivers?
>>>>>>>>>>> I have no ideas how well it works under Windows, and I have no ideas
>>>>>>>>>>> if there are some strange hacks in the Windows nor in the Syanptics
>>>>>>>>>>> driver (I would assume so).
>>>>>>>>>>>
>>>>>>>>> We don't provide any drivers for touchscreens on Windows. So I don't
>>>>>>>>> know how Microsoft is handling a situation like this.
>>>>>>>> Do you know what should be changed in firmware to make hid-touchscreen
>>>>>>>> driver works correctly? Or maybe you know someone who is responsible for
>>>>>>>> firmware for this device and whom I can call to gather this information?
>>>>>>>>
>>>>>>> In case there *really* is broken firmware out there, we can specifically
>>>>>>> identify via struct input_id's version number for example,
>>>>>> I thought that Benjamin identified this as a broken firmware. I've
>>>>>> attached hidraw log in the issue and there is no release event, so it
>>>>>> looks like a firmware bug. How do you suggest to handle this situation
>>>>>> in kernel? We can identify the device but what to do next if we have no
>>>>>> information if user released finger or not?
>>>>>>
>>>>>>> I want to
>>>>>>> point out that I would accept adding a workaround in tslib's input-raw
>>>>>>> module ( http://tslib.org ) if it won't be done in the kernel.
>>>>>>>
>>>>>>> So, in case you can and want to use tslib as a workaround here, feel
>>>>>>> free to have a look and send the patches that make input-raw.c work for
>>>>>>> you over there.
>>>>>> I want to be as handy as I can but I'm not sure how tslib could help in
>>>>>> this situation. If we have too much data, it can filter out unnecessary
>>>>>> events but I don't think that it can help when there is lack of events
>>>>>> or I'm missing something?
>>>>> Might as well be, I might not have thought it all through, but in
>>>>> tslib's module_raw input you can can get totally creative: Why not start
>>>>> *every* sync frame with BTN_TOUCH 1 and end it with BTN_TOUCH 0? You
>>>>> *are* able to add stuff. Filters don't usually do it though.
>>>> Ouch, please don't. You'll send an endless click/release sequence
>>>> which will break drag and drop, double click and so on.
>>>>
>>> ah you're right, that's nonsense.
>>>
>>>> Also, this won't solve the issue because the multitouch slot will not
>>>> be released.
>>>>
>>>> The only solution (which i believe the Windows driver does, but I
>>>> believed only for Windows 7 compatible touchscreen), is to arm a timer
>>>> for each slot, and when you don't receive an update after let's say 5
>>>> seconds, you release the slot.
>>>>
>>>> It's awful and I always have been against adding such pain in the
>>>> hid-multitouch driver.
>>> yes. still breaks "move after hold>5s" but would probably be the only
>>> way to make this somewhat work.
>> 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).
Sorry for a stupid question, but do we still discussing a solution for 
this device until Synaptics will correct firmware? What do you 
understand by firmware? A code in C compiled to kernel's module handling 
IRQs? Or a BIOS?

Why we need to think about workarounds and not just solve the problem in 
the root? Will it take a long time and we want to have a quick fix for 
similar cases or for other reasons?


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
  2017-05-11 14:30                               ` Arek Burdach
@ 2017-05-11 14:45                                 ` Benjamin Tissoires
  2017-05-11 15:38                                   ` Arek Burdach
  0 siblings, 1 reply; 37+ messages in thread
From: Benjamin Tissoires @ 2017-05-11 14:45 UTC (permalink / raw)
  To: Arek Burdach
  Cc: Martin Kepplinger, Andrew Duggan, Stéphane Chatty, linux-input

On Thu, May 11, 2017 at 4:30 PM, Arek Burdach <arek.burdach@gmail.com> wrote:
>
>
> On 11.05.2017 14:50, Martin Kepplinger wrote:
>>
>>
>> On 2017-05-11 14:28, Benjamin Tissoires wrote:
>>>
>>> On Thu, May 11, 2017 at 1:47 PM, Martin Kepplinger
>>> <martin.kepplinger@ginzinger.com> wrote:
>>>>
>>>>
>>>> On 2017-05-11 13:28, Benjamin Tissoires wrote:
>>>>>
>>>>> On Thu, May 11, 2017 at 1:22 PM, Martin Kepplinger
>>>>> <martin.kepplinger@ginzinger.com> wrote:
>>>>>>
>>>>>>
>>>>>> On 2017-05-11 12:12, Arek Burdach wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 11.05.2017 11:48, Martin Kepplinger wrote:
>>>>>>>>
>>>>>>>> On 2017-05-10 11:36, Arek Burdach wrote:
>>>>>>>>>
>>>>>>>>> Hi Andrew,
>>>>>>>>>
>>>>>>>>> On 10.05.2017 01:47, Andrew Duggan wrote:
>>>>>>>>>>
>>>>>>>>>> HI Arek,
>>>>>>>>>>
>>>>>>>>>> On 05/09/2017 04:17 PM, Arek Burdach wrote:
>>>>>>>>>>>
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>> I've tried described by you solution:
>>>>>>>>>>>
>>>>>>>>>>> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
>>>>>>>>>>> index 37084b645785..81f271554b6c 100644
>>>>>>>>>>> --- a/drivers/hid/hid-core.c
>>>>>>>>>>> +++ b/drivers/hid/hid-core.c
>>>>>>>>>>> @@ -2510,6 +2510,7 @@ static const struct hid_device_id
>>>>>>>>>>> hid_ignore_list[] = {
>>>>>>>>>>
>>>>>>>>>> You need to add this to the hid_have_special_driver[] and not the
>>>>>>>>>> hid_ignore_list[].
>>>>>>>>>
>>>>>>>>> Nice score for me - two lines and one bug :-)
>>>>>>>>>
>>>>>>>>>> But, if you do success in binding hid-rmi to a touchscreen it
>>>>>>>>>> won't
>>>>>>>>>> work. The firmware between touchpads and touchscreens are
>>>>>>>>>> different
>>>>>>>>>> enough that the hid-rmi driver will be looking for data which does
>>>>>>>>>> not
>>>>>>>>>> exist in touchscreen's HID report. These differences also mean
>>>>>>>>>> that it
>>>>>>>>>> really isn't a good idea to try to support touchscreens with
>>>>>>>>>> hid-rmi.
>>>>>>>>>> It would actually result in more transactions and be less
>>>>>>>>>> efficient
>>>>>>>>>> then simply using hid-multitouch. That's why hid-core checks for
>>>>>>>>>> the
>>>>>>>>>> HID_SCAN_FLAG_GD_POINTER in an attempt to make sure it's binding
>>>>>>>>>> to a
>>>>>>>>>> touchpad and not a touchscreen.
>>>>>>>>>
>>>>>>>>> It was just like you predict. On rmi, after first tap on screen,
>>>>>>>>> hidraw
>>>>>>>>> produced infinite number of events and it is not usable anymore.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>>> On 09.05.2017 16:02, Benjamin Tissoires wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> On Tue, May 9, 2017 at 2:51 PM, Arek Burdach
>>>>>>>>>>>> <arek.burdach@gmail.com> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 09.05.2017 14:20, Benjamin Tissoires wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Tue, May 9, 2017 at 11:20 AM, Arek Burdach
>>>>>>>>>>>>>> <arek.burdach@gmail.com>
>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Thank you for response.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On 09.05.2017 10:35, Benjamin Tissoires wrote:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On Sat, May 6, 2017 at 9:28 PM, Arek Burdach
>>>>>>>>>>>>>>>> <arek.burdach@gmail.com>
>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> A week ago I've reported a bug:
>>>>>>>>>>>>>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=195625 Is there
>>>>>>>>>>>>>>>>> anybody
>>>>>>>>>>>>>>>>> that
>>>>>>>>>>>>>>>>> can
>>>>>>>>>>>>>>>>> help me with it?
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> I can have a look at it.
>>>>>>>>>>>>>>>> Please attach the full outputs of hid-recorder and
>>>>>>>>>>>>>>>> evemu-record
>>>>>>>>>>>>>>>> in the
>>>>>>>>>>>>>>>> bugs, or it'll be difficult for us to debug it.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I've attached full logs for two situations. More details in
>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>> issue.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks, looks like a firmware issue (I'll comment in the bug).
>>>>>>>>>>>>>
>>>>>>>>>>>>> Sorry for my noob questions, but do you suggest that it can't
>>>>>>>>>>>>> be
>>>>>>>>>>>>> fixed by
>>>>>>>>>>>>> changes in kernel modules and I need to report it to the
>>>>>>>>>>>>> manufacturer?
>>>>>>>>>>>>
>>>>>>>>>>>> Yes. Though Andrew, in CC, works for Synaptics and might give us
>>>>>>>>>>>> some pointers.
>>>>>>>>>>>>
>>>>>>>>>>>>> If it so, do you have an idea why it works well on Windows? Do
>>>>>>>>>>>>> they
>>>>>>>>>>>>> have
>>>>>>>>>>>>> some strange hacks in their drivers?
>>>>>>>>>>>>
>>>>>>>>>>>> I have no ideas how well it works under Windows, and I have no
>>>>>>>>>>>> ideas
>>>>>>>>>>>> if there are some strange hacks in the Windows nor in the
>>>>>>>>>>>> Syanptics
>>>>>>>>>>>> driver (I would assume so).
>>>>>>>>>>>>
>>>>>>>>>> We don't provide any drivers for touchscreens on Windows. So I
>>>>>>>>>> don't
>>>>>>>>>> know how Microsoft is handling a situation like this.
>>>>>>>>>
>>>>>>>>> Do you know what should be changed in firmware to make
>>>>>>>>> hid-touchscreen
>>>>>>>>> driver works correctly? Or maybe you know someone who is
>>>>>>>>> responsible for
>>>>>>>>> firmware for this device and whom I can call to gather this
>>>>>>>>> information?
>>>>>>>>>
>>>>>>>> In case there *really* is broken firmware out there, we can
>>>>>>>> specifically
>>>>>>>> identify via struct input_id's version number for example,
>>>>>>>
>>>>>>> I thought that Benjamin identified this as a broken firmware. I've
>>>>>>> attached hidraw log in the issue and there is no release event, so it
>>>>>>> looks like a firmware bug. How do you suggest to handle this
>>>>>>> situation
>>>>>>> in kernel? We can identify the device but what to do next if we have
>>>>>>> no
>>>>>>> information if user released finger or not?
>>>>>>>
>>>>>>>> I want to
>>>>>>>> point out that I would accept adding a workaround in tslib's
>>>>>>>> input-raw
>>>>>>>> module ( http://tslib.org ) if it won't be done in the kernel.
>>>>>>>>
>>>>>>>> So, in case you can and want to use tslib as a workaround here, feel
>>>>>>>> free to have a look and send the patches that make input-raw.c work
>>>>>>>> for
>>>>>>>> you over there.
>>>>>>>
>>>>>>> I want to be as handy as I can but I'm not sure how tslib could help
>>>>>>> in
>>>>>>> this situation. If we have too much data, it can filter out
>>>>>>> unnecessary
>>>>>>> events but I don't think that it can help when there is lack of
>>>>>>> events
>>>>>>> or I'm missing something?
>>>>>>
>>>>>> Might as well be, I might not have thought it all through, but in
>>>>>> tslib's module_raw input you can can get totally creative: Why not
>>>>>> start
>>>>>> *every* sync frame with BTN_TOUCH 1 and end it with BTN_TOUCH 0? You
>>>>>> *are* able to add stuff. Filters don't usually do it though.
>>>>>
>>>>> Ouch, please don't. You'll send an endless click/release sequence
>>>>> which will break drag and drop, double click and so on.
>>>>>
>>>> ah you're right, that's nonsense.
>>>>
>>>>> Also, this won't solve the issue because the multitouch slot will not
>>>>> be released.
>>>>>
>>>>> The only solution (which i believe the Windows driver does, but I
>>>>> believed only for Windows 7 compatible touchscreen), is to arm a timer
>>>>> for each slot, and when you don't receive an update after let's say 5
>>>>> seconds, you release the slot.
>>>>>
>>>>> It's awful and I always have been against adding such pain in the
>>>>> hid-multitouch driver.
>>>>
>>>> yes. still breaks "move after hold>5s" but would probably be the only
>>>> way to make this somewhat work.
>>>
>>> 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).
>
> Sorry for a stupid question, but do we still discussing a solution for this
> device until Synaptics will correct firmware? What do you understand by
> firmware? A code in C compiled to kernel's module handling IRQs? Or a BIOS?

The firmware is the piece of code that is embedded in the touchscreen
chip, so property of Synaptics. Everything we can do us developers
will be a workaround.
To upgrade the firmware, some times you need to upgrade the BIOS
(UEFI) of your laptop, sometimes there is a different (but poprietary)
way of doing it.

>
> Why we need to think about workarounds and not just solve the problem in the
> root? Will it take a long time and we want to have a quick fix for similar
> cases or for other reasons?

That's in Synaptics' hands. If they say, yes, there is a bug on our
side we will fix it in an upgrade, then we don't need to do anything.
But given that Windows somewhat manages to not be affected, I guess
Synaptics won't bother fixing this just for us, Linux users.

Cheers,
Benjamin
>

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
  2017-05-11 14:45                                 ` Benjamin Tissoires
@ 2017-05-11 15:38                                   ` Arek Burdach
  2017-05-12  6:56                                     ` Martin Kepplinger
  0 siblings, 1 reply; 37+ messages in thread
From: Arek Burdach @ 2017-05-11 15:38 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Martin Kepplinger, Andrew Duggan, Stéphane Chatty, linux-input

On 11.05.2017 16:45, Benjamin Tissoires wrote:
> On Thu, May 11, 2017 at 4:30 PM, Arek Burdach <arek.burdach@gmail.com> wrote:
>>
>> On 11.05.2017 14:50, Martin Kepplinger wrote:
>>>
>>> On 2017-05-11 14:28, Benjamin Tissoires wrote:
>>>> On Thu, May 11, 2017 at 1:47 PM, Martin Kepplinger
>>>> <martin.kepplinger@ginzinger.com> wrote:
>>>>>
>>>>> On 2017-05-11 13:28, Benjamin Tissoires wrote:
>>>>>> On Thu, May 11, 2017 at 1:22 PM, Martin Kepplinger
>>>>>> <martin.kepplinger@ginzinger.com> wrote:
>>>>>>>
>>>>>>> On 2017-05-11 12:12, Arek Burdach wrote:
>>>>>>>>
>>>>>>>> On 11.05.2017 11:48, Martin Kepplinger wrote:
>>>>>>>>> On 2017-05-10 11:36, Arek Burdach wrote:
>>>>>>>>>> Hi Andrew,
>>>>>>>>>>
>>>>>>>>>> On 10.05.2017 01:47, Andrew Duggan wrote:
>>>>>>>>>>> HI Arek,
>>>>>>>>>>>
>>>>>>>>>>> On 05/09/2017 04:17 PM, Arek Burdach wrote:
>>>>>>>>>>>> Hi,
>>>>>>>>>>>>
>>>>>>>>>>>> I've tried described by you solution:
>>>>>>>>>>>>
>>>>>>>>>>>> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
>>>>>>>>>>>> index 37084b645785..81f271554b6c 100644
>>>>>>>>>>>> --- a/drivers/hid/hid-core.c
>>>>>>>>>>>> +++ b/drivers/hid/hid-core.c
>>>>>>>>>>>> @@ -2510,6 +2510,7 @@ static const struct hid_device_id
>>>>>>>>>>>> hid_ignore_list[] = {
>>>>>>>>>>> You need to add this to the hid_have_special_driver[] and not the
>>>>>>>>>>> hid_ignore_list[].
>>>>>>>>>> Nice score for me - two lines and one bug :-)
>>>>>>>>>>
>>>>>>>>>>> But, if you do success in binding hid-rmi to a touchscreen it
>>>>>>>>>>> won't
>>>>>>>>>>> work. The firmware between touchpads and touchscreens are
>>>>>>>>>>> different
>>>>>>>>>>> enough that the hid-rmi driver will be looking for data which does
>>>>>>>>>>> not
>>>>>>>>>>> exist in touchscreen's HID report. These differences also mean
>>>>>>>>>>> that it
>>>>>>>>>>> really isn't a good idea to try to support touchscreens with
>>>>>>>>>>> hid-rmi.
>>>>>>>>>>> It would actually result in more transactions and be less
>>>>>>>>>>> efficient
>>>>>>>>>>> then simply using hid-multitouch. That's why hid-core checks for
>>>>>>>>>>> the
>>>>>>>>>>> HID_SCAN_FLAG_GD_POINTER in an attempt to make sure it's binding
>>>>>>>>>>> to a
>>>>>>>>>>> touchpad and not a touchscreen.
>>>>>>>>>> It was just like you predict. On rmi, after first tap on screen,
>>>>>>>>>> hidraw
>>>>>>>>>> produced infinite number of events and it is not usable anymore.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>>> On 09.05.2017 16:02, Benjamin Tissoires wrote:
>>>>>>>>>>>>> On Tue, May 9, 2017 at 2:51 PM, Arek Burdach
>>>>>>>>>>>>> <arek.burdach@gmail.com> wrote:
>>>>>>>>>>>>>> On 09.05.2017 14:20, Benjamin Tissoires wrote:
>>>>>>>>>>>>>>> On Tue, May 9, 2017 at 11:20 AM, Arek Burdach
>>>>>>>>>>>>>>> <arek.burdach@gmail.com>
>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Thank you for response.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On 09.05.2017 10:35, Benjamin Tissoires wrote:
>>>>>>>>>>>>>>>>> On Sat, May 6, 2017 at 9:28 PM, Arek Burdach
>>>>>>>>>>>>>>>>> <arek.burdach@gmail.com>
>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> A week ago I've reported a bug:
>>>>>>>>>>>>>>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=195625 Is there
>>>>>>>>>>>>>>>>>> anybody
>>>>>>>>>>>>>>>>>> that
>>>>>>>>>>>>>>>>>> can
>>>>>>>>>>>>>>>>>> help me with it?
>>>>>>>>>>>>>>>>> I can have a look at it.
>>>>>>>>>>>>>>>>> Please attach the full outputs of hid-recorder and
>>>>>>>>>>>>>>>>> evemu-record
>>>>>>>>>>>>>>>>> in the
>>>>>>>>>>>>>>>>> bugs, or it'll be difficult for us to debug it.
>>>>>>>>>>>>>>>> I've attached full logs for two situations. More details in
>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>> issue.
>>>>>>>>>>>>>>> Thanks, looks like a firmware issue (I'll comment in the bug).
>>>>>>>>>>>>>> Sorry for my noob questions, but do you suggest that it can't
>>>>>>>>>>>>>> be
>>>>>>>>>>>>>> fixed by
>>>>>>>>>>>>>> changes in kernel modules and I need to report it to the
>>>>>>>>>>>>>> manufacturer?
>>>>>>>>>>>>> Yes. Though Andrew, in CC, works for Synaptics and might give us
>>>>>>>>>>>>> some pointers.
>>>>>>>>>>>>>
>>>>>>>>>>>>>> If it so, do you have an idea why it works well on Windows? Do
>>>>>>>>>>>>>> they
>>>>>>>>>>>>>> have
>>>>>>>>>>>>>> some strange hacks in their drivers?
>>>>>>>>>>>>> I have no ideas how well it works under Windows, and I have no
>>>>>>>>>>>>> ideas
>>>>>>>>>>>>> if there are some strange hacks in the Windows nor in the
>>>>>>>>>>>>> Syanptics
>>>>>>>>>>>>> driver (I would assume so).
>>>>>>>>>>>>>
>>>>>>>>>>> We don't provide any drivers for touchscreens on Windows. So I
>>>>>>>>>>> don't
>>>>>>>>>>> know how Microsoft is handling a situation like this.
>>>>>>>>>> Do you know what should be changed in firmware to make
>>>>>>>>>> hid-touchscreen
>>>>>>>>>> driver works correctly? Or maybe you know someone who is
>>>>>>>>>> responsible for
>>>>>>>>>> firmware for this device and whom I can call to gather this
>>>>>>>>>> information?
>>>>>>>>>>
>>>>>>>>> In case there *really* is broken firmware out there, we can
>>>>>>>>> specifically
>>>>>>>>> identify via struct input_id's version number for example,
>>>>>>>> I thought that Benjamin identified this as a broken firmware. I've
>>>>>>>> attached hidraw log in the issue and there is no release event, so it
>>>>>>>> looks like a firmware bug. How do you suggest to handle this
>>>>>>>> situation
>>>>>>>> in kernel? We can identify the device but what to do next if we have
>>>>>>>> no
>>>>>>>> information if user released finger or not?
>>>>>>>>
>>>>>>>>> I want to
>>>>>>>>> point out that I would accept adding a workaround in tslib's
>>>>>>>>> input-raw
>>>>>>>>> module ( http://tslib.org ) if it won't be done in the kernel.
>>>>>>>>>
>>>>>>>>> So, in case you can and want to use tslib as a workaround here, feel
>>>>>>>>> free to have a look and send the patches that make input-raw.c work
>>>>>>>>> for
>>>>>>>>> you over there.
>>>>>>>> I want to be as handy as I can but I'm not sure how tslib could help
>>>>>>>> in
>>>>>>>> this situation. If we have too much data, it can filter out
>>>>>>>> unnecessary
>>>>>>>> events but I don't think that it can help when there is lack of
>>>>>>>> events
>>>>>>>> or I'm missing something?
>>>>>>> Might as well be, I might not have thought it all through, but in
>>>>>>> tslib's module_raw input you can can get totally creative: Why not
>>>>>>> start
>>>>>>> *every* sync frame with BTN_TOUCH 1 and end it with BTN_TOUCH 0? You
>>>>>>> *are* able to add stuff. Filters don't usually do it though.
>>>>>> Ouch, please don't. You'll send an endless click/release sequence
>>>>>> which will break drag and drop, double click and so on.
>>>>>>
>>>>> ah you're right, that's nonsense.
>>>>>
>>>>>> Also, this won't solve the issue because the multitouch slot will not
>>>>>> be released.
>>>>>>
>>>>>> The only solution (which i believe the Windows driver does, but I
>>>>>> believed only for Windows 7 compatible touchscreen), is to arm a timer
>>>>>> for each slot, and when you don't receive an update after let's say 5
>>>>>> seconds, you release the slot.
>>>>>>
>>>>>> It's awful and I always have been against adding such pain in the
>>>>>> hid-multitouch driver.
>>>>> yes. still breaks "move after hold>5s" but would probably be the only
>>>>> way to make this somewhat work.
>>>> 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).
>> Sorry for a stupid question, but do we still discussing a solution for this
>> device until Synaptics will correct firmware? What do you understand by
>> firmware? A code in C compiled to kernel's module handling IRQs? Or a BIOS?
> The firmware is the piece of code that is embedded in the touchscreen
> chip, so property of Synaptics. Everything we can do us developers
> will be a workaround.
> To upgrade the firmware, some times you need to upgrade the BIOS
> (UEFI) of your laptop, sometimes there is a different (but poprietary)
> way of doing it.
>
>> Why we need to think about workarounds and not just solve the problem in the
>> root? Will it take a long time and we want to have a quick fix for similar
>> cases or for other reasons?
> That's in Synaptics' hands. If they say, yes, there is a bug on our
> side we will fix it in an upgrade, then we don't need to do anything.
> But given that Windows somewhat manages to not be affected, I guess
> Synaptics won't bother fixing this just for us, Linux users.
>
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.

Cheers,
Arek

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
  2017-05-11 15:38                                   ` Arek Burdach
@ 2017-05-12  6:56                                     ` Martin Kepplinger
  2017-05-12  7:25                                       ` Arek Burdach
  0 siblings, 1 reply; 37+ messages in thread
From: Martin Kepplinger @ 2017-05-12  6:56 UTC (permalink / raw)
  To: Arek Burdach, Benjamin Tissoires
  Cc: Andrew Duggan, Stéphane Chatty, linux-input

>>>>> 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.

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.

                    martin

________________________________

Ginzinger electronic systems GmbH
Gewerbegebiet Pirath 16
4952 Weng im Innkreis
www.ginzinger.com

Firmenbuchnummer: FN 364958d
Firmenbuchgericht: Ried im Innkreis
UID-Nr.: ATU66521089


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
  2017-05-12  6:56                                     ` Martin Kepplinger
@ 2017-05-12  7:25                                       ` Arek Burdach
  2017-05-12  7:34                                         ` Martin Kepplinger
  2017-05-12  7:39                                         ` Benjamin Tissoires
  0 siblings, 2 replies; 37+ messages in thread
From: Arek Burdach @ 2017-05-12  7:25 UTC (permalink / raw)
  To: Martin Kepplinger, Benjamin Tissoires
  Cc: Andrew Duggan, Stéphane Chatty, linux-input



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?

> 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.

Cheers,
Arek

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
  2017-05-12  7:25                                       ` Arek Burdach
@ 2017-05-12  7:34                                         ` Martin Kepplinger
  2017-05-12  7:39                                         ` Benjamin Tissoires
  1 sibling, 0 replies; 37+ messages in thread
From: Martin Kepplinger @ 2017-05-12  7:34 UTC (permalink / raw)
  To: Arek Burdach, Benjamin Tissoires
  Cc: Andrew Duggan, Stéphane Chatty, linux-input



On 2017-05-12 09:25, Arek Burdach 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?

no, I can only look for one, like you.
https://lmddgtfy.net/?q=buy%20i2c%20sniffer

>
>> 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.
>

Keep asking Synaptics for answers then. Or pay somebody to write a
driver. Or be patient and do it yourself.

                          martin


________________________________

Ginzinger electronic systems GmbH
Gewerbegebiet Pirath 16
4952 Weng im Innkreis
www.ginzinger.com

Firmenbuchnummer: FN 364958d
Firmenbuchgericht: Ried im Innkreis
UID-Nr.: ATU66521089


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
  2017-05-12  7:25                                       ` Arek Burdach
  2017-05-12  7:34                                         ` Martin Kepplinger
@ 2017-05-12  7:39                                         ` Benjamin Tissoires
  2017-05-12  7:57                                           ` Arek Burdach
  1 sibling, 1 reply; 37+ messages in thread
From: Benjamin Tissoires @ 2017-05-12  7:39 UTC (permalink / raw)
  To: Arek Burdach
  Cc: Martin Kepplinger, Andrew Duggan, Stéphane Chatty, linux-input

On Fri, May 12, 2017 at 9:25 AM, Arek Burdach <arek.burdach@gmail.com> 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.

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.

Cheers,
Benjamin

>
> Cheers,
> Arek

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
  2017-05-12  7:39                                         ` Benjamin Tissoires
@ 2017-05-12  7:57                                           ` Arek Burdach
  2017-05-12 14:23                                             ` Benjamin Tissoires
  0 siblings, 1 reply; 37+ messages in thread
From: Arek Burdach @ 2017-05-12  7:57 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Martin Kepplinger, Andrew Duggan, Stéphane Chatty, linux-input



On 12.05.2017 09:39, Benjamin Tissoires wrote:
> On Fri, May 12, 2017 at 9:25 AM, Arek Burdach <arek.burdach@gmail.com> 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.
>
> Cheers,
> Benjamin
>
>> Cheers,
>> Arek


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
  2017-05-12  7:57                                           ` Arek Burdach
@ 2017-05-12 14:23                                             ` Benjamin Tissoires
  2017-05-12 14:28                                               ` Benjamin Tissoires
  0 siblings, 1 reply; 37+ messages in thread
From: Benjamin Tissoires @ 2017-05-12 14:23 UTC (permalink / raw)
  To: Arek Burdach
  Cc: Martin Kepplinger, Andrew Duggan, Stéphane Chatty, linux-input



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@gmail.com>
>> 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@enac.fr>");
@@ -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);
+
 	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

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
  2017-05-12 14:23                                             ` Benjamin Tissoires
@ 2017-05-12 14:28                                               ` Benjamin Tissoires
  2017-05-12 17:50                                                 ` Arek Burdach
                                                                   ` (2 more replies)
  0 siblings, 3 replies; 37+ messages in thread
From: Benjamin Tissoires @ 2017-05-12 14:28 UTC (permalink / raw)
  To: Arek Burdach
  Cc: Martin Kepplinger, Andrew Duggan, Stéphane Chatty, linux-input

On Fri, May 12, 2017 at 4:23 PM, Benjamin Tissoires
<benjamin.tissoires@gmail.com> 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@gmail.com>
>>> 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@enac.fr>");
> @@ -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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
  2017-05-12 14:28                                               ` Benjamin Tissoires
@ 2017-05-12 17:50                                                 ` Arek Burdach
  2017-05-15 18:49                                                 ` Arek Burdach
       [not found]                                                 ` <c0ba39eb-c8d5-2473-36d9-ce7c605ef845@gmail.com>
  2 siblings, 0 replies; 37+ messages in thread
From: Arek Burdach @ 2017-05-12 17:50 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Martin Kepplinger, Andrew Duggan, Stéphane Chatty, linux-input

On 12.05.2017 16:28, Benjamin Tissoires wrote:
> On Fri, May 12, 2017 at 4:23 PM, Benjamin Tissoires
> <benjamin.tissoires@gmail.com> 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@gmail.com>
>>>> 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).
>>>>>> (...)
>>>> 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):
(...)
>>  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.
>>
I've just tested it. Tapping works like a charm! but ... dragging 
stopped to work at all. From logs is see that release event is produced 
even if there is some other event like ABS_MT_POSITION_Y.

Maybe one useful information is that I've observed that average delay 
between events when I'm moving finger on screen or using touchpad is 
about 7-9 ms.

Feel free to send patch proposals for testing. I'll try to debug what is 
wrong.

Cheers,
Arek

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
  2017-05-12 14:28                                               ` Benjamin Tissoires
  2017-05-12 17:50                                                 ` Arek Burdach
@ 2017-05-15 18:49                                                 ` Arek Burdach
       [not found]                                                 ` <c0ba39eb-c8d5-2473-36d9-ce7c605ef845@gmail.com>
  2 siblings, 0 replies; 37+ messages in thread
From: Arek Burdach @ 2017-05-15 18:49 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Martin Kepplinger, Andrew Duggan, Stéphane Chatty, linux-input

Hi Benjamin,

I found out what was wrong in the change that you've provided:

On 12.05.2017 16:28, Benjamin Tissoires wrote:
 >>
 >>     + if (td->mtclass.quirks & MT_QUIRK_STICKY_FINGERS) + 
mod_timer(&td->release_timer, msecs_to_jiffies(250)); }
Should be:
mod_timer(&td->release_timer, jiffies + msecs_to_jiffies(100));

Delay should be added to current jiffies value. Also I found out that 
250 ms is too long delay - xserver recognize such a delay as a drag 
gesture. Using 100 ms everything works perfectly!

What do you think that should be changed more in the patch to make it 
ready for being submitted as an official patch? I thought about some 
unit tests, but can't find any for hid drivers also I don't know how to 
mock timers.

One more time, thank you for your support!

Cheers, Arek

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
       [not found]                                                 ` <c0ba39eb-c8d5-2473-36d9-ce7c605ef845@gmail.com>
@ 2017-05-16  8:18                                                   ` Benjamin Tissoires
  2017-05-16  9:34                                                     ` Benjamin Tissoires
  2017-05-16  9:46                                                     ` Arek Burdach
  0 siblings, 2 replies; 37+ messages in thread
From: Benjamin Tissoires @ 2017-05-16  8:18 UTC (permalink / raw)
  To: Arek Burdach
  Cc: Martin Kepplinger, Andrew Duggan, Stéphane Chatty, linux-input

On Mon, May 15, 2017 at 8:45 PM, Arek Burdach <arek.burdach@gmail.com> wrote:
> Hi Benjamin,
>
> I found out what was wrong in the change that you've provided:
>
> On 12.05.2017 16:28, Benjamin Tissoires wrote:
>
> +       if (td->mtclass.quirks & MT_QUIRK_STICKY_FINGERS)
> +               mod_timer(&td->release_timer, msecs_to_jiffies(250));
>  }
>
> Should be:
>
> mod_timer(&td->release_timer, jiffies + msecs_to_jiffies(100));

Right, thanks!

>
> Delay should be added to current jiffies value. Also I found out that 250 ms
> is too long delay - xserver recognize such a delay as a drag gesture. Using
> 100 ms everything works perfectly!

Does 160 ms works too? I'd rather have 10 times the maximum time
between 2 reports than 6.25 times it :)
We can use 96 ms if 160 doesn't work.


> What do you think that should be changed
> more in the patch to make it ready for being submitted as an official patch?

Not much actually. I need to wrote down a commit message, sign it
(your sign-off-by can also be added given that you debugged it), and
that should be it.

> I thought about some unit tests, but can't find any for hid drivers also I

I have a test suite that can emulate hid devices and inject them in
hid-multitouch. The setup is a little bit manual, so I'll try to run
it today and see if there are differences with or without the patch.

> don't know how to mock timers. One more time, thank you for your support!

No worries :)

Cheers,
Benjamin

> Cheers, Arek

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
  2017-05-16  8:18                                                   ` Benjamin Tissoires
@ 2017-05-16  9:34                                                     ` Benjamin Tissoires
  2017-05-16  9:46                                                     ` Arek Burdach
  1 sibling, 0 replies; 37+ messages in thread
From: Benjamin Tissoires @ 2017-05-16  9:34 UTC (permalink / raw)
  To: Arek Burdach
  Cc: Martin Kepplinger, Andrew Duggan, Stéphane Chatty, linux-input

On Tue, May 16, 2017 at 10:18 AM, Benjamin Tissoires
<benjamin.tissoires@gmail.com> wrote:
> On Mon, May 15, 2017 at 8:45 PM, Arek Burdach <arek.burdach@gmail.com> wrote:
>> Hi Benjamin,
>>
>> I found out what was wrong in the change that you've provided:
>>
>> On 12.05.2017 16:28, Benjamin Tissoires wrote:
>>
>> +       if (td->mtclass.quirks & MT_QUIRK_STICKY_FINGERS)
>> +               mod_timer(&td->release_timer, msecs_to_jiffies(250));
>>  }
>>
>> Should be:
>>
>> mod_timer(&td->release_timer, jiffies + msecs_to_jiffies(100));
>
> Right, thanks!
>
>>
>> Delay should be added to current jiffies value. Also I found out that 250 ms
>> is too long delay - xserver recognize such a delay as a drag gesture. Using
>> 100 ms everything works perfectly!
>
> Does 160 ms works too? I'd rather have 10 times the maximum time
> between 2 reports than 6.25 times it :)
> We can use 96 ms if 160 doesn't work.
>
>
>> What do you think that should be changed
>> more in the patch to make it ready for being submitted as an official patch?
>
> Not much actually. I need to wrote down a commit message, sign it
> (your sign-off-by can also be added given that you debugged it), and
> that should be it.
>
>> I thought about some unit tests, but can't find any for hid drivers also I
>
> I have a test suite that can emulate hid devices and inject them in
> hid-multitouch. The setup is a little bit manual, so I'll try to run
> it today and see if there are differences with or without the patch.

I just run this patch against the various recordings I have. There is
only one difference, but after thorough examination, it seems the
machine blocked for one second for no reasons (the recorded incoming
timestamps show 1 second delay while the scan time forwarded by the
device show 50ms between the 2 events).

So I'd call this patch safe to test and to go upstream.

Cheers,
Benjamin

>
>> don't know how to mock timers. One more time, thank you for your support!
>
> No worries :)
>
> Cheers,
> Benjamin
>
>> Cheers, Arek

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
  2017-05-16  8:18                                                   ` Benjamin Tissoires
  2017-05-16  9:34                                                     ` Benjamin Tissoires
@ 2017-05-16  9:46                                                     ` Arek Burdach
  2017-06-07  7:27                                                       ` Arek Burdach
  1 sibling, 1 reply; 37+ messages in thread
From: Arek Burdach @ 2017-05-16  9:46 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Martin Kepplinger, Andrew Duggan, Stéphane Chatty, linux-input



On 16.05.2017 10:18, Benjamin Tissoires wrote:
> On Mon, May 15, 2017 at 8:45 PM, Arek Burdach <arek.burdach@gmail.com> wrote:
>> Hi Benjamin,
>>
>> I found out what was wrong in the change that you've provided:
>>
>> On 12.05.2017 16:28, Benjamin Tissoires wrote:
>>
>> +       if (td->mtclass.quirks & MT_QUIRK_STICKY_FINGERS)
>> +               mod_timer(&td->release_timer, msecs_to_jiffies(250));
>>   }
>>
>> Should be:
>>
>> mod_timer(&td->release_timer, jiffies + msecs_to_jiffies(100));
> Right, thanks!
>
>> Delay should be added to current jiffies value. Also I found out that 250 ms
>> is too long delay - xserver recognize such a delay as a drag gesture. Using
>> 100 ms everything works perfectly!
> Does 160 ms works too?
160 ms works too but I have a feeling that values closer to 200 ms gives 
quite a laggy experience so I'd suggest to keep value closer to 100 ms. 
Beside that I haven't seen situation that "normal" syn_report have been 
repored after longer delay than 30 ms. Mean value is 9 ms and 95 pp ~10 
ms (tested by drags with evemu-record)

> I'd rather have 10 times the maximum time
> between 2 reports than 6.25 times it :)
> We can use 96 ms if 160 doesn't work.
In fact 100 ms it is exactly 6 times the maximum time between 2 reports, 
because 60 Hz frequency gives 16,(6) ms period

>> What do you think that should be changed
>> more in the patch to make it ready for being submitted as an official patch?
> Not much actually. I need to wrote down a commit message, sign it
> (your sign-off-by can also be added given that you debugged it), and
> that should be it.
>
>> I thought about some unit tests, but can't find any for hid drivers also I
> I have a test suite that can emulate hid devices and inject them in
> hid-multitouch. The setup is a little bit manual, so I'll try to run
> it today and see if there are differences with or without the patch.
Ok, waiting for signed patch

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
  2017-05-16  9:46                                                     ` Arek Burdach
@ 2017-06-07  7:27                                                       ` Arek Burdach
  2017-06-07 13:12                                                         ` Benjamin Tissoires
  0 siblings, 1 reply; 37+ messages in thread
From: Arek Burdach @ 2017-06-07  7:27 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Martin Kepplinger, Andrew Duggan, Stéphane Chatty, linux-input

Hi Benjamin,

On 16.05.2017 11:46, Arek Burdach wrote:
>
>
> On 16.05.2017 10:18, Benjamin Tissoires wrote:
>> On Mon, May 15, 2017 at 8:45 PM, Arek Burdach 
>> <arek.burdach@gmail.com> wrote:
>>> Hi Benjamin,
>>>
>>> I found out what was wrong in the change that you've provided:
>>>
>>> On 12.05.2017 16:28, Benjamin Tissoires wrote:
>>>
>>> +       if (td->mtclass.quirks & MT_QUIRK_STICKY_FINGERS)
>>> +               mod_timer(&td->release_timer, msecs_to_jiffies(250));
>>>   }
>>>
>>> Should be:
>>>
>>> mod_timer(&td->release_timer, jiffies + msecs_to_jiffies(100));
>> Right, thanks!
>>
>>> Delay should be added to current jiffies value. Also I found out 
>>> that 250 ms
>>> is too long delay - xserver recognize such a delay as a drag 
>>> gesture. Using
>>> 100 ms everything works perfectly!
>> Does 160 ms works too?
> 160 ms works too but I have a feeling that values closer to 200 ms 
> gives quite a laggy experience so I'd suggest to keep value closer to 
> 100 ms. Beside that I haven't seen situation that "normal" syn_report 
> have been repored after longer delay than 30 ms. Mean value is 9 ms 
> and 95 pp ~10 ms (tested by drags with evemu-record)
>
>> I'd rather have 10 times the maximum time
>> between 2 reports than 6.25 times it :)
>> We can use 96 ms if 160 doesn't work.
> In fact 100 ms it is exactly 6 times the maximum time between 2 
> reports, because 60 Hz frequency gives 16,(6) ms period
>
>>> What do you think that should be changed
>>> more in the patch to make it ready for being submitted as an 
>>> official patch?
>> Not much actually. I need to wrote down a commit message, sign it
>> (your sign-off-by can also be added given that you debugged it), and
>> that should be it.

Do you planning to submit this change? I think that other users would be 
glad to take advantage of this fix :-)
Or do you want me to do it on your behalf?


>>
>>> I thought about some unit tests, but can't find any for hid drivers 
>>> also I
>> I have a test suite that can emulate hid devices and inject them in
>> hid-multitouch. The setup is a little bit manual, so I'll try to run
>> it today and see if there are differences with or without the patch.
> Ok, waiting for signed patch

Cheers,
Arek

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
  2017-06-07  7:27                                                       ` Arek Burdach
@ 2017-06-07 13:12                                                         ` Benjamin Tissoires
  2017-06-07 17:58                                                           ` Arek Burdach
  0 siblings, 1 reply; 37+ messages in thread
From: Benjamin Tissoires @ 2017-06-07 13:12 UTC (permalink / raw)
  To: Arek Burdach
  Cc: Martin Kepplinger, Andrew Duggan, Stéphane Chatty, linux-input

Hi Arek,

On Wed, Jun 7, 2017 at 9:27 AM, Arek Burdach <arek.burdach@gmail.com> wrote:
> Hi Benjamin,
>
>
> On 16.05.2017 11:46, Arek Burdach wrote:
>>
>>
>>
>> On 16.05.2017 10:18, Benjamin Tissoires wrote:
>>>
>>> On Mon, May 15, 2017 at 8:45 PM, Arek Burdach <arek.burdach@gmail.com>
>>> wrote:
>>>>
>>>> Hi Benjamin,
>>>>
>>>> I found out what was wrong in the change that you've provided:
>>>>
>>>> On 12.05.2017 16:28, Benjamin Tissoires wrote:
>>>>
>>>> +       if (td->mtclass.quirks & MT_QUIRK_STICKY_FINGERS)
>>>> +               mod_timer(&td->release_timer, msecs_to_jiffies(250));
>>>>   }
>>>>
>>>> Should be:
>>>>
>>>> mod_timer(&td->release_timer, jiffies + msecs_to_jiffies(100));
>>>
>>> Right, thanks!
>>>
>>>> Delay should be added to current jiffies value. Also I found out that
>>>> 250 ms
>>>> is too long delay - xserver recognize such a delay as a drag gesture.
>>>> Using
>>>> 100 ms everything works perfectly!
>>>
>>> Does 160 ms works too?
>>
>> 160 ms works too but I have a feeling that values closer to 200 ms gives
>> quite a laggy experience so I'd suggest to keep value closer to 100 ms.
>> Beside that I haven't seen situation that "normal" syn_report have been
>> repored after longer delay than 30 ms. Mean value is 9 ms and 95 pp ~10 ms
>> (tested by drags with evemu-record)
>>
>>> I'd rather have 10 times the maximum time
>>> between 2 reports than 6.25 times it :)
>>> We can use 96 ms if 160 doesn't work.
>>
>> In fact 100 ms it is exactly 6 times the maximum time between 2 reports,
>> because 60 Hz frequency gives 16,(6) ms period
>>
>>>> What do you think that should be changed
>>>> more in the patch to make it ready for being submitted as an official
>>>> patch?
>>>
>>> Not much actually. I need to wrote down a commit message, sign it
>>> (your sign-off-by can also be added given that you debugged it), and
>>> that should be it.
>
>
> Do you planning to submit this change? I think that other users would be
> glad to take advantage of this fix :-)

Yes, sorry, it's still on my todo list. I am trying to fix the last
issue I can think of which is problematic:
if you happen touch again the sensor when the deferred timeout starts,
both threads will access the input node, and this can create some
nasty behaviors.

I am not 100% sure we can rely on classic solutions (spinlock,
mutexes) because in these 2 concurrent threads we are in places where
we can not stop.

So I'll need to conduct more tests and find a better/simpler solution
to not hit that. Sorry for the delay.

> Or do you want me to do it on your behalf?

I'd like you to give a test when I finally get something. However,
it's not on my top priority list, sorry for that.

Cheers,
Benjamin

>
>
>>>
>>>> I thought about some unit tests, but can't find any for hid drivers also
>>>> I
>>>
>>> I have a test suite that can emulate hid devices and inject them in
>>> hid-multitouch. The setup is a little bit manual, so I'll try to run
>>> it today and see if there are differences with or without the patch.
>>
>> Ok, waiting for signed patch
>
>
> Cheers,
> Arek

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: Missing release event for Synaptics touchscreen
  2017-06-07 13:12                                                         ` Benjamin Tissoires
@ 2017-06-07 17:58                                                           ` Arek Burdach
  0 siblings, 0 replies; 37+ messages in thread
From: Arek Burdach @ 2017-06-07 17:58 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Martin Kepplinger, Andrew Duggan, Stéphane Chatty, linux-input

On 07.06.2017 15:12, Benjamin Tissoires wrote:
> Hi Arek,
>
> On Wed, Jun 7, 2017 at 9:27 AM, Arek Burdach <arek.burdach@gmail.com> wrote:
>> Hi Benjamin,
>>
>>
>> On 16.05.2017 11:46, Arek Burdach wrote:
>>>
>>>
>>> On 16.05.2017 10:18, Benjamin Tissoires wrote:
>>>> On Mon, May 15, 2017 at 8:45 PM, Arek Burdach <arek.burdach@gmail.com>
>>>> wrote:
>>>>> Hi Benjamin,
>>>>>
>>>>> I found out what was wrong in the change that you've provided:
>>>>>
>>>>> On 12.05.2017 16:28, Benjamin Tissoires wrote:
>>>>>
>>>>> +       if (td->mtclass.quirks & MT_QUIRK_STICKY_FINGERS)
>>>>> +               mod_timer(&td->release_timer, msecs_to_jiffies(250));
>>>>>    }
>>>>>
>>>>> Should be:
>>>>>
>>>>> mod_timer(&td->release_timer, jiffies + msecs_to_jiffies(100));
>>>> Right, thanks!
>>>>
>>>>> Delay should be added to current jiffies value. Also I found out that
>>>>> 250 ms
>>>>> is too long delay - xserver recognize such a delay as a drag gesture.
>>>>> Using
>>>>> 100 ms everything works perfectly!
>>>> Does 160 ms works too?
>>> 160 ms works too but I have a feeling that values closer to 200 ms gives
>>> quite a laggy experience so I'd suggest to keep value closer to 100 ms.
>>> Beside that I haven't seen situation that "normal" syn_report have been
>>> repored after longer delay than 30 ms. Mean value is 9 ms and 95 pp ~10 ms
>>> (tested by drags with evemu-record)
>>>
>>>> I'd rather have 10 times the maximum time
>>>> between 2 reports than 6.25 times it :)
>>>> We can use 96 ms if 160 doesn't work.
>>> In fact 100 ms it is exactly 6 times the maximum time between 2 reports,
>>> because 60 Hz frequency gives 16,(6) ms period
>>>
>>>>> What do you think that should be changed
>>>>> more in the patch to make it ready for being submitted as an official
>>>>> patch?
>>>> Not much actually. I need to wrote down a commit message, sign it
>>>> (your sign-off-by can also be added given that you debugged it), and
>>>> that should be it.
>>
>> Do you planning to submit this change? I think that other users would be
>> glad to take advantage of this fix :-)
> Yes, sorry, it's still on my todo list. I am trying to fix the last
> issue I can think of which is problematic:
> if you happen touch again the sensor when the deferred timeout starts,
> both threads will access the input node, and this can create some
> nasty behaviors.
Indeed this situation can produce some nasty behavior like interrupted 
dragging gesture but user need to click very fast to occur that.
>
> I am not 100% sure we can rely on classic solutions (spinlock,
> mutexes) because in these 2 concurrent threads we are in places where
> we can not stop.
I'm very intrigued how it could be resolved without mutexes.
>
> So I'll need to conduct more tests and find a better/simpler solution
> to not hit that. Sorry for the delay.
No problem. Thank you for the notice.
>
>> Or do you want me to do it on your behalf?
> I'd like you to give a test when I finally get something. However,
> it's not on my top priority list, sorry for that.
I would test it with pleasure. BTW I didn't noticed any problem with 
current solution so far and I'm using touchscreen quite often.

One more time thank you for your help!
>
> Cheers,
> Benjamin
>
>>
>>>>> I thought about some unit tests, but can't find any for hid drivers also
>>>>> I
>>>> I have a test suite that can emulate hid devices and inject them in
>>>> hid-multitouch. The setup is a little bit manual, so I'll try to run
>>>> it today and see if there are differences with or without the patch.
>>> Ok, waiting for signed patch
>>
>> Cheers,
>> Arek


^ permalink raw reply	[flat|nested] 37+ messages in thread

end of thread, other threads:[~2017-06-07 17:58 UTC | newest]

Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-06 19:28 Missing release event for Synaptics touchscreen Arek Burdach
2017-05-09  8:35 ` Benjamin Tissoires
2017-05-09  9:20   ` Arek Burdach
2017-05-09 12:20     ` Benjamin Tissoires
2017-05-09 12:51       ` Arek Burdach
2017-05-09 14:02         ` Benjamin Tissoires
2017-05-09 23:17           ` Arek Burdach
2017-05-09 23:47             ` Andrew Duggan
2017-05-10  9:36               ` Arek Burdach
2017-05-11  9:48                 ` Martin Kepplinger
2017-05-11 10:12                   ` Arek Burdach
2017-05-11 11:22                     ` Martin Kepplinger
2017-05-11 11:28                       ` Benjamin Tissoires
2017-05-11 11:44                         ` Arek Burdach
2017-05-11 11:47                         ` Martin Kepplinger
2017-05-11 12:28                           ` Benjamin Tissoires
2017-05-11 12:50                             ` Martin Kepplinger
2017-05-11 14:30                               ` Arek Burdach
2017-05-11 14:45                                 ` Benjamin Tissoires
2017-05-11 15:38                                   ` Arek Burdach
2017-05-12  6:56                                     ` Martin Kepplinger
2017-05-12  7:25                                       ` Arek Burdach
2017-05-12  7:34                                         ` Martin Kepplinger
2017-05-12  7:39                                         ` Benjamin Tissoires
2017-05-12  7:57                                           ` Arek Burdach
2017-05-12 14:23                                             ` Benjamin Tissoires
2017-05-12 14:28                                               ` Benjamin Tissoires
2017-05-12 17:50                                                 ` Arek Burdach
2017-05-15 18:49                                                 ` Arek Burdach
     [not found]                                                 ` <c0ba39eb-c8d5-2473-36d9-ce7c605ef845@gmail.com>
2017-05-16  8:18                                                   ` Benjamin Tissoires
2017-05-16  9:34                                                     ` Benjamin Tissoires
2017-05-16  9:46                                                     ` Arek Burdach
2017-06-07  7:27                                                       ` Arek Burdach
2017-06-07 13:12                                                         ` Benjamin Tissoires
2017-06-07 17:58                                                           ` Arek Burdach
2017-05-11 11:36                       ` Arek Burdach
2017-05-11 11:36                       ` Martin Kepplinger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.