Skip to content

Commit

Permalink
HID: multitouch: fix rare Win 8 cases when the touch up event gets mi…
Browse files Browse the repository at this point in the history 
…ssing

Instead of blindly trusting the hardware to send us release, we should
consider some events can get lost and release them when we judge time has
come.

The Windows 8 spec allows to be confident in the fact that the device
will continuously report events when a finger touches the surface.
This has been tested on the HID recording database I have, and all of
those devices behave properly.
Also, Arek tested it on his Lenovo Yoga 910, which exports such bug in
some situations, when the movements are rather slow.

We use an atomic bit here to guard against concurrent accesses to the mt
slots because both mt_process_mt_event() and mt_expired_timeout() are
called in interrupt context.

Signed-off-by: Arek Burdach <arek.burdach@gmail.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Tested-by: Arek Burdach <arek.burdach@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
  • Loading branch information
Benjamin Tissoires authored and Jiri Kosina committed Jun 23, 2017
1 parent fd91189 commit 4f4001b
Showing 1 changed file with 79 additions and 23 deletions.
102 changes: 79 additions & 23 deletions drivers/hid/hid-multitouch.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>");
Expand All @@ -70,12 +71,15 @@ MODULE_LICENSE("GPL");
#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

#define MT_BUTTONTYPE_CLICKPAD 0

#define MT_IO_FLAGS_RUNNING 0

struct mt_slot {
__s32 x, y, cx, cy, p, w, h;
__s32 contactid; /* the device ContactID assigned to this slot */
Expand Down Expand Up @@ -104,8 +108,10 @@ 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 */
unsigned long mt_io_flags; /* mt flags (MT_IO_FLAGS_*) */
int cc_index; /* contact count field index in the report */
int cc_value_index; /* contact count value index in the field */
unsigned last_slot_field; /* the last field of a slot */
Expand Down Expand Up @@ -214,7 +220,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,
Expand Down Expand Up @@ -804,6 +811,10 @@ static void mt_touch_report(struct hid_device *hid, struct hid_report *report)
unsigned count;
int r, n;

/* sticky fingers release in progress, abort */
if (test_and_set_bit(MT_IO_FLAGS_RUNNING, &td->mt_io_flags))
return;

/*
* Includes multi-packet support where subsequent
* packets are sent with zero contactcount.
Expand All @@ -829,6 +840,29 @@ 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);

/*
* Windows 8 specs says 2 things:
* - once a contact has been reported, it has to be reported in each
* subsequent report
* - the report rate when fingers are present has to be at least
* the refresh rate of the screen, 60 or 120 Hz
*
* I interprete this that the specification forces a report rate of
* at least 60 Hz for a touchscreen to be certified.
* Which means that if we do not get a report whithin 16 ms, either
* something wrong happens, either the touchscreen forgets to send
* a release. Taking a reasonable margin allows to remove issues
* with USB communication or the load of the machine.
*
* Given that Win 8 devices are forced to send a release, this will
* only affect laggish machines and the ones that have a firmware
* defect.
*/
if (td->mtclass.quirks & MT_QUIRK_STICKY_FINGERS)
mod_timer(&td->release_timer, jiffies + msecs_to_jiffies(100));

clear_bit(MT_IO_FLAGS_RUNNING, &td->mt_io_flags);
}

static int mt_touch_input_configured(struct hid_device *hdev,
Expand Down Expand Up @@ -1140,6 +1174,46 @@ 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;
struct mt_device *td = hid_get_drvdata(hid);

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);
}
}

td->num_received = 0;
}

static void mt_expired_timeout(unsigned long arg)
{
struct hid_device *hdev = (void *)arg;
struct mt_device *td = hid_get_drvdata(hdev);

/*
* An input report came in just before we release the sticky fingers,
* it will take care of the sticky fingers.
*/
if (test_and_set_bit(MT_IO_FLAGS_RUNNING, &td->mt_io_flags))
return;
mt_release_contacts(hdev);
clear_bit(MT_IO_FLAGS_RUNNING, &td->mt_io_flags);
}

static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
{
int ret, i;
Expand Down Expand Up @@ -1209,6 +1283,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;
Expand Down Expand Up @@ -1236,28 +1312,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);
Expand All @@ -1282,6 +1336,8 @@ static void mt_remove(struct hid_device *hdev)
{
struct mt_device *td = hid_get_drvdata(hdev);

del_timer_sync(&td->release_timer);

sysfs_remove_group(&hdev->dev.kobj, &mt_attribute_group);
hid_hw_stop(hdev);
hdev->quirks = td->initial_quirks;
Expand Down

0 comments on commit 4f4001b

Please sign in to comment.