| Message ID | 20200606035017.7271-1-GNUtoo@cyberdimension.org |
|---|---|
| State | New |
| Headers | show |
| Series | Input: mms114: don't report 0 pressure while still tracking contact(s) | expand |
On Fri, 26 Jun 2020 10:04:39 +1000 Peter Hutterer <peter.hutterer@who-t.net> wrote: > thanks for the log. Basically - the problem is that > ABS_MT_TOUCH_MAJOR and ABS_PRESSURE are completely unrelated on the > device and the latter has apparently random values. 1585880999.248531 > is an event where you go from almost max pressure to 0 without > changing touch major. I also tried not to touch the screen too hard, so it's normal to have some pressure variation as well. > Since pressure is more common, you'll have to expect that userspace > may ignore major/minor and handle pressure instead where available. > Doubly so since historically the major/minor value range has been > completely random while pressure was at least somewhat predictable. > In this sequence, your touch major ranges from 4-14 despite the axis > range being 0-255. > > Historically, pressure has also been used as equivalent to touch > size, so decoupling touch size and pressure is tricky anyway. > Speaking from libinput's POV I would disable ABS_(MT_)PRESSURE in > this device since it's not reliable to detect a touch. But then we'd > still need a quirk in place to tell us what the possible touch major > range could be to make sense of that number. I didn't understood if I needed to do something about that patch or not. Here I'm mostly interested in fixing that issue for future kernels and/or userspace input stack releases. Am I supposed to fix the issue in userspace? Or is the advise on libinput a way to deal with older kernel versions? Is the quirk meant to be in Linux or in libinput? I'm currently testing with GNU/Linux as it's faster, but eventually I'm also interested in running Android with a Linux kernel that is as much upstream as possible, so I also need to understand the API here: Is it up to userspace to interpret if the values are somewhat valid, or is it up to the kernel to return valid values? Denis.
apparently I never replied to this, apologies. On Sun, Jul 26, 2020 at 11:42:29PM +0200, Denis 'GNUtoo' Carikli wrote: > On Fri, 26 Jun 2020 10:04:39 +1000 > Peter Hutterer <peter.hutterer@who-t.net> wrote: > > > thanks for the log. Basically - the problem is that > > ABS_MT_TOUCH_MAJOR and ABS_PRESSURE are completely unrelated on the > > device and the latter has apparently random values. 1585880999.248531 > > is an event where you go from almost max pressure to 0 without > > changing touch major. > I also tried not to touch the screen too hard, so it's normal to have > some pressure variation as well. some pressure variation is fine, but having major unchanged while pressure changes significantly is a problem. Especially with a human finger the touch size would uusally change as you increase or decrease pressure simply because the finger gets squished. > > Since pressure is more common, you'll have to expect that userspace > > may ignore major/minor and handle pressure instead where available. > > Doubly so since historically the major/minor value range has been > > completely random while pressure was at least somewhat predictable. > > In this sequence, your touch major ranges from 4-14 despite the axis > > range being 0-255. > > > > Historically, pressure has also been used as equivalent to touch > > size, so decoupling touch size and pressure is tricky anyway. > > Speaking from libinput's POV I would disable ABS_(MT_)PRESSURE in > > this device since it's not reliable to detect a touch. But then we'd > > still need a quirk in place to tell us what the possible touch major > > range could be to make sense of that number. > > I didn't understood if I needed to do something about that patch or > not. > > Here I'm mostly interested in fixing that issue for future kernels > and/or userspace input stack releases. > > Am I supposed to fix the issue in userspace? Or is the advise on > libinput a way to deal with older kernel versions? Is the quirk > meant to be in Linux or in libinput? libinput uses ABS_MT_PRESSURE with some defaults based on the pressure range unless a (libinput) quirk tells it to use the ABS_MT_TOUCH_MAJOR axis ranges. git grep for the AttrTouchSizeRange, AttrThumbSizeThreshold and AttrPalmSizeThreshold and that'll get you there. Given the recording, i'm assuming pressure is not reliable on this device so you will have to add the quirk. > I'm currently testing with GNU/Linux as it's faster, but eventually I'm > also interested in running Android with a Linux kernel that is as much > upstream as possible, so I also need to understand the API here: Is it > up to userspace to interpret if the values are somewhat valid, or is it > up to the kernel to return valid values? yes, it's up to userspace. there's some documentation in the kernel regarding the major/minor axis ranges but not a lot of devices use it that way. Hence libinput requiring a quirk for this. Not 100% what other input stacks do though. Cheers, Peter
diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c index 2ef1adaed9af..adc18cd9a437 100644 --- a/drivers/input/touchscreen/mms114.c +++ b/drivers/input/touchscreen/mms114.c @@ -183,7 +183,10 @@ static void mms114_process_mt(struct mms114_data *data, struct mms114_touch *tou if (touch->pressed) { touchscreen_report_pos(input_dev, &data->props, x, y, true); input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR, touch->width); - input_report_abs(input_dev, ABS_MT_PRESSURE, touch->strength); + if (touch->strength) { + input_report_abs(input_dev, ABS_MT_PRESSURE, + touch->strength); + } } }
In the middle of a sliding gesture, we manage to have events that look like that: Event: time 1571859641.595517, -------------- SYN_REPORT ------------ Event: time 1571859641.606593, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value 1193 Event: time 1571859641.606593, type 3 (EV_ABS), code 48 (ABS_MT_TOUCH_MAJOR), value 21 Event: time 1571859641.606593, type 3 (EV_ABS), code 58 (ABS_MT_PRESSURE), value 0 Event: time 1571859641.606593, type 3 (EV_ABS), code 1 (ABS_Y), value 1193 Event: time 1571859641.606593, type 3 (EV_ABS), code 24 (ABS_PRESSURE), value 0 Event: time 1571859641.606593, -------------- SYN_REPORT ------------ In such cases, we still have a valid ABS_MT_TRACKING_ID along with an ABS_MT_TOUCH_MAJOR that is > 0, which both indicates that the sliding is still in progress. However in Documentation/input/multi-touch-protocol.rst, we have: ABS_MT_PRESSURE The pressure, in arbitrary units, on the contact area. May be used instead of TOUCH and WIDTH for pressure-based devices or any device with a spatial signal intensity distribution. Because of that userspace may consider an ABS_MT_PRESSURE of 0 as an indication that the sliding stopped. This has side effects such as making it difficult to unlock the screen under Android. This fix was tested on the following devices: - GT-I9300 with a glass screen protection - GT-I9300 without any screen protection - GT-N7105 with a glass screen protection Reported-by: Javi Ferrer <javi.f.o@gmail.com> Signed-off-by: Denis 'GNUtoo' Carikli <GNUtoo@cyberdimension.org> --- drivers/input/touchscreen/mms114.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)