Re: [PATCH] Input: mms114: don't report 0 pressure while still tracking contact(s)

On Sat, Jun 06, 2020 at 11:18:06AM -0700, Dmitry Torokhov wrote:

Hi Denis,

On Sat, Jun 06, 2020 at 05:50:17AM +0200, Denis 'GNUtoo' Carikli wrote:

quoted

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 <redacted>
Signed-off-by: Denis 'GNUtoo' Carikli <redacted>
---
 drivers/input/touchscreen/mms114.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

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

So this will result in userspace believing that pressure is unchanged
from the previous packet. I wonder if we should report

		input_report_abs(input_dev, ABS_MT_PRESSURE,
				 max(touch->strength, 1));

instead.

fwiw, this could still be interpreted as a lifted touch, especially if the
pressure values are comparatively high (say you go from 100 to 1).
we have to use thresholds for what is a touch up because a number of devices
give us nonzero pressure for ever-so-slight touches (or even hovers).
so a nonzero pressure isn't a guarantee for touch down.

not on this device, but from my POV I'd rather have the device pretend the
pressure is the same as the last known touch than just forcing it to 1, a
value that may be completely out of place.

Cheers,
   Peter