Re: Synaptics RMI4 touchpad regression in 4.11-rc1

From: Andrew Duggan
Date: Fri Mar 17 2017 - 15:24:20 EST


On 03/17/2017 09:57 AM, Benjamin Tissoires wrote:
On Wed, Mar 15, 2017 at 2:19 AM, Andrew Duggan <aduggan@xxxxxxxxxxxxx> wrote:
On 03/13/2017 10:10 PM, Cameron Gutman wrote:

On 03/13/2017 06:35 PM, Andrew Duggan wrote:

On 03/13/2017 06:15 AM, Benjamin Tissoires wrote:
[Resending, forgot to add Jiri in CC]

On Mar 13 2017 or thereabouts, Benjamin Tissoires wrote:
On Mar 13 2017 or thereabouts, Thorsten Leemhuis wrote:
Lo! On 12.03.2017 02:55, Cameron Gutman wrote:
Beginning in 4.11-rc1, it looks like RMI4 is binding to my XPS 13
9343's
Synaptics touchpad and dropping some errors into dmesg. Here are the
messages that seem RMI-related:

rmi4_f34 rmi4-00.fn34: rmi_f34v7_probe: Unrecognized bootloader
version
rmi4_f34: probe of rmi4-00.fn34 failed with error -22
rmi4_f01 rmi4-00.fn01: found RMI device, manufacturer: Synaptics,
product: TM3038-001, fw id: 1832324
input: Synaptics TM3038-001 as
/devices/pci0000:00/INT3433:00/i2c-7/i2c-DLL0665:01/0018:06CB:76AD.0001/input/input19
hid-rmi 0018:06CB:76AD.0001: input,hidraw0: I2C HID v1.00 Mouse
[DLL0665:01 06CB:76AD] on i2c-DLL0665:01
FWIW, I get this on my XPS 13 DE (9360) with 4.11-rc1:

input: SynPS/2 Synaptics TouchPad as
/devices/platform/i8042/serio1/input/input6
rmi4_f34 rmi4-00.fn34: rmi_f34v7_probe: Unrecognized bootloader
version
rmi4_f34: probe of rmi4-00.fn34 failed with error -22
rmi4_f01 rmi4-00.fn01: found RMI device, manufacturer: Synaptics,
product: TM3038-003, fw id: 2375007
input: Synaptics TM3038-003 as

/devices/pci0000:00/0000:00:15.1/i2c_designware.1/i2c-8/i2c-DLL075B:01/0018:06CB:76AF.0001/input/input20
hid-rmi 0018:06CB:76AF.0001: input,hidraw0: I2C HID v1.00 Mouse
[DLL075B:01 06CB:76AF] on i2c-DLL075B:01

[â]
Compared to hid-multitouch, the RMI stack seems to have completely
broken
palm rejection and introduced some random jumpiness during fine
pointing
motions. I don't know if these issues are caused by the above errors
or
are a separate issue.
The error about the bootloader version not being recognized just means
that updating the firmware is not supported on this touchpad. It is only the
F34 firmware update functionality which is failing to load. The palm
rejection and jumps are not related to this error.

Maybe that code path should be changed to not make as much noise when it
runs
on known unsupported hardware. Something like the attached patch?

Looking at how hid-multitouch handles palms it looks like palms should
not be reported as active when calling input_mt_report_slot_state(). I'm
setting the tool type to MT_TOOL_PALM when the firmware determines that a
contact is a palm. But, that does not seem to be sufficient enough to have
the palms filtered out. After some quick testing it looks like the change
below will restore palm rejection similar to that provided by
hid-multitouch.

It looks like your email client ate the tabs, but if I apply the change
myself it seems to fix the palm rejection regression for me.

Tested-by: Cameron Gutman <aicommander@xxxxxxxxx>

Sorry, I was short on time and just copied the diff into the email. I'll
submit a proper patch soon with your tested-by included. Thanks for testing.


I just pointed out this patch (well the actual submission) to Jason
(Cc-ed). Given that there is no proper handling of MT_TOOL_PALM yet in
userspace, I thought it was the easiest way.
However, it seems that this doesn't enhance the jumps and just make it worse.

I was assuming that the jumps and palm rejection where two separate issues. But, the palm rejection patch makes things worse?

Is there anything we can do to fix it (besides temporary disabling the
automatic loading of hid-rmi)?

I do not have a fix for the jumps yet. My next step is to file a bug against libinput or the kernel. I used evemu-record to capture a log with jumps, but when I play it back with a different userspace input stack with an older version of libinput I do not see the jumps. I see the jumps on Fedora 25 with libinput 1.6.3 vs Ubuntu 16.10 with libinput 1.4.3 with X). Or at least the jumps are not as significant. But, its possible there is an issue with how the events are being reported which is incorrect and confusing libinput. The X and Y coordinates being reported by the firmware seem correct and I haven't found a problem yet. I thought a bug would be a better place to collect evemu-record logs and compare.

Hopefully, this will end up being a quick fix in the kernel and we can get it applied to 4.11 without having to hold off on disabling hid-rmi for PTPs.

Andrew

Cheers,
Benjamin


Just to confirm: I noticed "jumpiness during fine pointing motions" as
well since switching to 4.11-rc.
One of my test systems is a XPS 13 9343 and I have not really seen any
jumpiness. But, based on the data I am seeing that if I lift my finger and
place it again in a short period of time the first event or so will be at
the location of the previous contact. Then it will switch over to the
current location. When switching over to hid-multitouch I was unable to
reproduce this behavior. This definitely could be the source of the jumps.

The jumpiness definitely happens without lifting my finger, but I'm
willing
to test any patch you think would improve the situation. Moving one finger
slowly in a figure-8 across my touchpad shows the issue clearly for me.
The
small variations in speed of my finger due to the friction on the trackpad
get magnified to relatively large jumpy pointer movements on screen. It
seems much more noticeable in diagonal movements than completely vertical
or horizontal movements.

Regards,
Cameron

---
diff --git a/drivers/input/rmi4/rmi_f34v7.c
b/drivers/input/rmi4/rmi_f34v7.c
index 56c6c39..1291d9a 100644
--- a/drivers/input/rmi4/rmi_f34v7.c
+++ b/drivers/input/rmi4/rmi_f34v7.c
@@ -1369,9 +1369,9 @@ int rmi_f34v7_probe(struct f34_data *f34)
} else if (f34->bootloader_id[1] == 7) {
f34->bl_version = 7;
} else {
- dev_err(&f34->fn->dev, "%s: Unrecognized bootloader
version\n",
- __func__);
- return -EINVAL;
+ dev_info(&f34->fn->dev, "%s: Unsupported bootloader
version: %u\n",
+ __func__, f34->bootloader_id[1]);
+ return -ENODEV;
}
memset(&f34->v7.blkcount, 0x00, sizeof(f34->v7.blkcount));