linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Input: bcm5974 - report highest finger pressure to Synaptics
@ 2015-06-03 11:49 Matt Whitlock
  2015-06-03 12:24 ` Henrik Rydberg
  0 siblings, 1 reply; 11+ messages in thread
From: Matt Whitlock @ 2015-06-03 11:49 UTC (permalink / raw)
  To: linux-input, Henrik Rydberg, Dmitry Torokhov; +Cc: Matt Whitlock

The Synaptics X.org driver compares the ABS_PRESSURE values in evdev input
events to its FingerLow and FingerHigh parameters to determine how it
should translate touchpad events into X pointer and button events. During a
transition between single-touch and multi-touch, the touchpad on the
MacBookPro6,2 sometimes reports the second finger in the first struct
tp_finger on the wire. If this second finger has a touch_major that (when
normalized) results in an ABS_PRESSURE of less than FingerLow, then the
Synaptics driver gets confused and can generate a spurious event, such as a
tap-and-drag or a secondary button click. This spurious event can manifest
as an undesired GUI operation, such as opening a context menu, selecting
text, or dragging a selection, during a multi-touch scrolling gesture.

This patch changes the report_synaptics_data() function so that it reports
the highest touch_major and highest tool_major of any touching finger as
ABS_PRESSURE and ABS_TOOL_WIDTH, respectively. Formerly this function would
report these values only from the first struct tp_finger and only if its
f->origin were non-zero. This could often lead to spurious reports of
ABS_PRESSURE==0 during multi-touch scrolling, which would also trigger the
aforementioned errant behavior in the Synaptics driver. The new behavior is
for this function to report ABS_PRESSURE==0 only in the case that no finger
has a non-zero f->origin and to report the highest observed ABS_PRESSURE in
the case of one or more fingers having non-zero f->origin.

Signed-off-by: Matt Whitlock <linux@mattwhitlock.name>

---

What follows is sample (abridged) output from evtest and xev when the errant behavior is triggered by beginning a two-finger scrolling gesture (placing the right finger on the touchpad slightly before the left finger), then dragging both fingers down, and then releasing both fingers. No button 1 events should be generated, and yet they are. Of critical importance is the ABS_PRESSURE==0 report at timestamp 1433325385.512505, which is reported even though MT slot 1 still has a finger touching. Producing this behavior is no longer possible after this patch is applied.

Event: time 1433325385.478496, type 3 (EV_ABS), code 47 (ABS_MT_SLOT), value 0
Event: time 1433325385.478496, type 3 (EV_ABS), code 57 (ABS_MT_TRACKING_ID), value 517
Event: time 1433325385.478496, type 3 (EV_ABS), code 48 (ABS_MT_TOUCH_MAJOR), value 388
Event: time 1433325385.478496, type 3 (EV_ABS), code 49 (ABS_MT_TOUCH_MINOR), value 660
Event: time 1433325385.478496, type 3 (EV_ABS), code 50 (ABS_MT_WIDTH_MAJOR), value 1879
Event: time 1433325385.478496, type 3 (EV_ABS), code 51 (ABS_MT_WIDTH_MINOR), value 1697
Event: time 1433325385.478496, type 3 (EV_ABS), code 53 (ABS_MT_POSITION_X), value 2414
Event: time 1433325385.478496, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value 679
Event: time 1433325385.478496, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 1
Event: time 1433325385.478496, type 1 (EV_KEY), code 325 (BTN_TOOL_FINGER), value 1
Event: time 1433325385.478496, type 3 (EV_ABS), code 0 (ABS_X), value 2414
Event: time 1433325385.478496, type 3 (EV_ABS), code 1 (ABS_Y), value 679
Event: time 1433325385.478496, type 3 (EV_ABS), code 24 (ABS_PRESSURE), value 165
Event: time 1433325385.478496, type 3 (EV_ABS), code 28 (ABS_TOOL_WIDTH), value 7
Event: time 1433325385.478496, -------------- EV_SYN ------------
Event: time 1433325385.512505, type 3 (EV_ABS), code 47 (ABS_MT_SLOT), value 1
Event: time 1433325385.512505, type 3 (EV_ABS), code 57 (ABS_MT_TRACKING_ID), value 518
Event: time 1433325385.512505, type 3 (EV_ABS), code 48 (ABS_MT_TOUCH_MAJOR), value 148
Event: time 1433325385.512505, type 3 (EV_ABS), code 49 (ABS_MT_TOUCH_MINOR), value 72
Event: time 1433325385.512505, type 3 (EV_ABS), code 50 (ABS_MT_WIDTH_MAJOR), value 2376
Event: time 1433325385.512505, type 3 (EV_ABS), code 52 (ABS_MT_ORIENTATION), value -3214
Event: time 1433325385.512505, type 3 (EV_ABS), code 53 (ABS_MT_POSITION_X), value -511
Event: time 1433325385.512505, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value 2994
Event: time 1433325385.512505, type 3 (EV_ABS), code 47 (ABS_MT_SLOT), value 0
Event: time 1433325385.512505, type 3 (EV_ABS), code 49 (ABS_MT_TOUCH_MINOR), value 670
Event: time 1433325385.512505, type 3 (EV_ABS), code 50 (ABS_MT_WIDTH_MAJOR), value 1865
Event: time 1433325385.512505, type 3 (EV_ABS), code 53 (ABS_MT_POSITION_X), value 2403
Event: time 1433325385.512505, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value 692
Event: time 1433325385.512505, type 1 (EV_KEY), code 325 (BTN_TOOL_FINGER), value 0
Event: time 1433325385.512505, type 1 (EV_KEY), code 333 (BTN_TOOL_DOUBLETAP), value 1
Event: time 1433325385.512505, type 3 (EV_ABS), code 0 (ABS_X), value 2403
Event: time 1433325385.512505, type 3 (EV_ABS), code 1 (ABS_Y), value 692
Event: time 1433325385.512505, type 3 (EV_ABS), code 24 (ABS_PRESSURE), value 0
Event: time 1433325385.512505, type 3 (EV_ABS), code 28 (ABS_TOOL_WIDTH), value 0
Event: time 1433325385.512505, -------------- EV_SYN ------------
Event: time 1433325385.520494, type 3 (EV_ABS), code 47 (ABS_MT_SLOT), value 1
Event: time 1433325385.520494, type 3 (EV_ABS), code 48 (ABS_MT_TOUCH_MAJOR), value 448
Event: time 1433325385.520494, type 3 (EV_ABS), code 49 (ABS_MT_TOUCH_MINOR), value 532
Event: time 1433325385.520494, type 3 (EV_ABS), code 50 (ABS_MT_WIDTH_MAJOR), value 2325
Event: time 1433325385.520494, type 3 (EV_ABS), code 52 (ABS_MT_ORIENTATION), value -2410
Event: time 1433325385.520494, type 3 (EV_ABS), code 53 (ABS_MT_POSITION_X), value -531
Event: time 1433325385.520494, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value 2978
Event: time 1433325385.520494, type 3 (EV_ABS), code 47 (ABS_MT_SLOT), value 0
Event: time 1433325385.520494, type 3 (EV_ABS), code 50 (ABS_MT_WIDTH_MAJOR), value 1853
Event: time 1433325385.520494, type 3 (EV_ABS), code 53 (ABS_MT_POSITION_X), value 2398
Event: time 1433325385.520494, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value 697
Event: time 1433325385.520494, type 3 (EV_ABS), code 0 (ABS_X), value 2398
Event: time 1433325385.520494, type 3 (EV_ABS), code 1 (ABS_Y), value 697
Event: time 1433325385.520494, type 3 (EV_ABS), code 24 (ABS_PRESSURE), value 191
Event: time 1433325385.520494, type 3 (EV_ABS), code 28 (ABS_TOOL_WIDTH), value 8
Event: time 1433325385.520494, -------------- EV_SYN ------------

ButtonPress event, serial 40, synthetic NO, window 0x1800001,
    root 0x2ef, subw 0x0, time 29415321, (113,82), root:(1493,108),
    state 0x0, button 1, same_screen YES
Event: time 1433325385.592484, type 3 (EV_ABS), code 47 (ABS_MT_SLOT), value 1
Event: time 1433325385.592484, type 3 (EV_ABS), code 49 (ABS_MT_TOUCH_MINOR), value 579
Event: time 1433325385.592484, type 3 (EV_ABS), code 50 (ABS_MT_WIDTH_MAJOR), value 2073
Event: time 1433325385.592484, type 3 (EV_ABS), code 53 (ABS_MT_POSITION_X), value -567
Event: time 1433325385.592484, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value 3121
Event: time 1433325385.592484, type 3 (EV_ABS), code 47 (ABS_MT_SLOT), value 0
Event: time 1433325385.592484, type 3 (EV_ABS), code 53 (ABS_MT_POSITION_X), value 2361
Event: time 1433325385.592484, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value 886
Event: time 1433325385.592484, type 3 (EV_ABS), code 0 (ABS_X), value 2361
Event: time 1433325385.592484, type 3 (EV_ABS), code 1 (ABS_Y), value 886
Event: time 1433325385.592484, type 3 (EV_ABS), code 24 (ABS_PRESSURE), value 206
Event: time 1433325385.592484, type 3 (EV_ABS), code 28 (ABS_TOOL_WIDTH), value 7
Event: time 1433325385.592484, -------------- EV_SYN ------------
Event: time 1433325385.600506, type 3 (EV_ABS), code 47 (ABS_MT_SLOT), value 1
Event: time 1433325385.600506, type 3 (EV_ABS), code 49 (ABS_MT_TOUCH_MINOR), value 595
Event: time 1433325385.600506, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value 3143
Event: time 1433325385.600506, type 3 (EV_ABS), code 47 (ABS_MT_SLOT), value 0
Event: time 1433325385.600506, type 3 (EV_ABS), code 51 (ABS_MT_WIDTH_MINOR), value 1649
Event: time 1433325385.600506, type 3 (EV_ABS), code 53 (ABS_MT_POSITION_X), value 2355
Event: time 1433325385.600506, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value 903
Event: time 1433325385.600506, type 3 (EV_ABS), code 0 (ABS_X), value 2355
Event: time 1433325385.600506, type 3 (EV_ABS), code 1 (ABS_Y), value 903
Event: time 1433325385.600506, -------------- EV_SYN ------------
Event: time 1433325385.607434, type 3 (EV_ABS), code 47 (ABS_MT_SLOT), value 1
Event: time 1433325385.607434, type 3 (EV_ABS), code 48 (ABS_MT_TOUCH_MAJOR), value 468
Event: time 1433325385.607434, type 3 (EV_ABS), code 49 (ABS_MT_TOUCH_MINOR), value 608
Event: time 1433325385.607434, type 3 (EV_ABS), code 53 (ABS_MT_POSITION_X), value -571
Event: time 1433325385.607434, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value 3164
Event: time 1433325385.607434, type 3 (EV_ABS), code 47 (ABS_MT_SLOT), value 0
Event: time 1433325385.607434, type 3 (EV_ABS), code 53 (ABS_MT_POSITION_X), value 2349
Event: time 1433325385.607434, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value 929
Event: time 1433325385.607434, type 3 (EV_ABS), code 0 (ABS_X), value 2349
Event: time 1433325385.607434, type 3 (EV_ABS), code 1 (ABS_Y), value 929
Event: time 1433325385.607434, type 3 (EV_ABS), code 24 (ABS_PRESSURE), value 209
Event: time 1433325385.607434, -------------- EV_SYN ------------
Event: time 1433325385.616463, type 3 (EV_ABS), code 47 (ABS_MT_SLOT), value 1
Event: time 1433325385.616463, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value 3184
Event: time 1433325385.616463, type 3 (EV_ABS), code 47 (ABS_MT_SLOT), value 0
Event: time 1433325385.616463, type 3 (EV_ABS), code 49 (ABS_MT_TOUCH_MINOR), value 719
Event: time 1433325385.616463, type 3 (EV_ABS), code 53 (ABS_MT_POSITION_X), value 2342
Event: time 1433325385.616463, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value 992
Event: time 1433325385.616463, type 3 (EV_ABS), code 0 (ABS_X), value 2342
Event: time 1433325385.616463, type 3 (EV_ABS), code 1 (ABS_Y), value 992
Event: time 1433325385.616463, -------------- EV_SYN ------------

ButtonPress event, serial 40, synthetic NO, window 0x1800001,
    root 0x2ef, subw 0x0, time 29415418, (113,82), root:(1493,108),
    state 0x100, button 5, same_screen YES

ButtonRelease event, serial 40, synthetic NO, window 0x1800001,
    root 0x2ef, subw 0x0, time 29415418, (113,82), root:(1493,108),
    state 0x1100, button 5, same_screen YES
Event: time 1433325385.648483, type 3 (EV_ABS), code 47 (ABS_MT_SLOT), value 1
Event: time 1433325385.648483, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value 3279
Event: time 1433325385.648483, type 3 (EV_ABS), code 47 (ABS_MT_SLOT), value 0
Event: time 1433325385.648483, type 3 (EV_ABS), code 48 (ABS_MT_TOUCH_MAJOR), value 296
Event: time 1433325385.648483, type 3 (EV_ABS), code 49 (ABS_MT_TOUCH_MINOR), value 392
Event: time 1433325385.648483, type 3 (EV_ABS), code 53 (ABS_MT_POSITION_X), value 2312
Event: time 1433325385.648483, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value 1127
Event: time 1433325385.648483, type 3 (EV_ABS), code 0 (ABS_X), value 2312
Event: time 1433325385.648483, type 3 (EV_ABS), code 1 (ABS_Y), value 1127
Event: time 1433325385.648483, type 3 (EV_ABS), code 24 (ABS_PRESSURE), value 205
Event: time 1433325385.648483, -------------- EV_SYN ------------

ButtonPress event, serial 40, synthetic NO, window 0x1800001,
    root 0x2ef, subw 0x0, time 29415449, (113,82), root:(1493,108),
    state 0x100, button 5, same_screen YES

ButtonRelease event, serial 40, synthetic NO, window 0x1800001,
    root 0x2ef, subw 0x0, time 29415449, (113,82), root:(1493,108),
    state 0x1100, button 5, same_screen YES
Event: time 1433325385.656456, type 3 (EV_ABS), code 47 (ABS_MT_SLOT), value 1
Event: time 1433325385.656456, type 3 (EV_ABS), code 48 (ABS_MT_TOUCH_MAJOR), value 452
Event: time 1433325385.656456, type 3 (EV_ABS), code 49 (ABS_MT_TOUCH_MINOR), value 598
Event: time 1433325385.656456, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value 3301
Event: time 1433325385.656456, type 3 (EV_ABS), code 47 (ABS_MT_SLOT), value 0
Event: time 1433325385.656456, type 3 (EV_ABS), code 48 (ABS_MT_TOUCH_MAJOR), value 58
Event: time 1433325385.656456, type 3 (EV_ABS), code 49 (ABS_MT_TOUCH_MINOR), value 72
Event: time 1433325385.656456, type 3 (EV_ABS), code 50 (ABS_MT_WIDTH_MAJOR), value 1758
Event: time 1433325385.656456, type 3 (EV_ABS), code 53 (ABS_MT_POSITION_X), value 2303
Event: time 1433325385.656456, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value 1148
Event: time 1433325385.656456, type 3 (EV_ABS), code 0 (ABS_X), value 2303
Event: time 1433325385.656456, type 3 (EV_ABS), code 1 (ABS_Y), value 1148
Event: time 1433325385.656456, type 3 (EV_ABS), code 24 (ABS_PRESSURE), value 172
Event: time 1433325385.656456, type 3 (EV_ABS), code 28 (ABS_TOOL_WIDTH), value 7
Event: time 1433325385.656456, -------------- EV_SYN ------------
Event: time 1433325385.663437, type 3 (EV_ABS), code 47 (ABS_MT_SLOT), value 1
Event: time 1433325385.663437, type 3 (EV_ABS), code 48 (ABS_MT_TOUCH_MAJOR), value 132
Event: time 1433325385.663437, type 3 (EV_ABS), code 49 (ABS_MT_TOUCH_MINOR), value 122
Event: time 1433325385.663437, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value 3366
Event: time 1433325385.663437, type 3 (EV_ABS), code 47 (ABS_MT_SLOT), value 0
Event: time 1433325385.663437, type 3 (EV_ABS), code 57 (ABS_MT_TRACKING_ID), value -1
Event: time 1433325385.663437, type 1 (EV_KEY), code 325 (BTN_TOOL_FINGER), value 1
Event: time 1433325385.663437, type 1 (EV_KEY), code 333 (BTN_TOOL_DOUBLETAP), value 0
Event: time 1433325385.663437, type 3 (EV_ABS), code 0 (ABS_X), value -576
Event: time 1433325385.663437, type 3 (EV_ABS), code 1 (ABS_Y), value 3366
Event: time 1433325385.663437, type 3 (EV_ABS), code 24 (ABS_PRESSURE), value 56
Event: time 1433325385.663437, type 3 (EV_ABS), code 28 (ABS_TOOL_WIDTH), value 8
Event: time 1433325385.663437, -------------- EV_SYN ------------
Event: time 1433325385.671508, type 3 (EV_ABS), code 47 (ABS_MT_SLOT), value 1
Event: time 1433325385.671508, type 3 (EV_ABS), code 57 (ABS_MT_TRACKING_ID), value -1
Event: time 1433325385.671508, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 0
Event: time 1433325385.671508, type 1 (EV_KEY), code 325 (BTN_TOOL_FINGER), value 0
Event: time 1433325385.671508, type 3 (EV_ABS), code 24 (ABS_PRESSURE), value 0
Event: time 1433325385.671508, type 3 (EV_ABS), code 28 (ABS_TOOL_WIDTH), value 0
Event: time 1433325385.671508, -------------- EV_SYN ------------

ButtonRelease event, serial 40, synthetic NO, window 0x1800001,
    root 0x2ef, subw 0x0, time 29415797, (113,82), root:(1493,108),
    state 0x100, button 1, same_screen YES
---
 drivers/input/mouse/bcm5974.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/input/mouse/bcm5974.c b/drivers/input/mouse/bcm5974.c
index b10709f..95474f4 100644
--- a/drivers/input/mouse/bcm5974.c
+++ b/drivers/input/mouse/bcm5974.c
@@ -526,19 +526,24 @@ static void report_synaptics_data(struct input_dev *input,
 				  const struct bcm5974_config *cfg,
 				  const struct tp_finger *f, int raw_n)
 {
-	int abs_p = 0, abs_w = 0;
+	int i, max_p = 0, max_w = 0;
 
-	if (raw_n) {
-		int p = raw2int(f->touch_major);
-		int w = raw2int(f->tool_major);
-		if (p > 0 && raw2int(f->origin)) {
-			abs_p = clamp_val(256 * p / cfg->p.max, 0, 255);
-			abs_w = clamp_val(16 * w / cfg->w.max, 0, 15);
+	for (i = 0; i < raw_n; ++i) {
+		if (f[i].origin) {
+			int p = raw2int(f[i].touch_major);
+			int w = raw2int(f[i].tool_major);
+
+			if (p > max_p)
+				max_p = p;
+			if (w > max_w)
+				max_w = w;
 		}
 	}
 
-	input_report_abs(input, ABS_PRESSURE, abs_p);
-	input_report_abs(input, ABS_TOOL_WIDTH, abs_w);
+	input_report_abs(input, ABS_PRESSURE,
+			clamp_val(256 * max_p / cfg->p.max, 0, 255));
+	input_report_abs(input, ABS_TOOL_WIDTH,
+			clamp_val(16 * max_w / cfg->w.max, 0, 15));
 }
 
 /* report trackpad data as logical trackpad state */
-- 
2.4.2


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

* Re: [PATCH] Input: bcm5974 - report highest finger pressure to Synaptics
  2015-06-03 11:49 [PATCH] Input: bcm5974 - report highest finger pressure to Synaptics Matt Whitlock
@ 2015-06-03 12:24 ` Henrik Rydberg
  2015-06-03 12:31   ` Matt Whitlock
  0 siblings, 1 reply; 11+ messages in thread
From: Henrik Rydberg @ 2015-06-03 12:24 UTC (permalink / raw)
  To: Matt Whitlock, linux-input, Dmitry Torokhov

Hi Matt,

> This patch changes the report_synaptics_data() function so that it reports
> the highest touch_major and highest tool_major of any touching finger as
> ABS_PRESSURE and ABS_TOOL_WIDTH, respectively.

I appreciate the problem, but I would much rather have bcm5974 converted
to use input_mt_report_pointer_emulation() instead. The emulation code
has been left as is for a long while, but this seems to be a good time
to convert it.

Thanks,
Henrik


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

* Re: [PATCH] Input: bcm5974 - report highest finger pressure to Synaptics
  2015-06-03 12:24 ` Henrik Rydberg
@ 2015-06-03 12:31   ` Matt Whitlock
  2015-06-03 12:52     ` Henrik Rydberg
  0 siblings, 1 reply; 11+ messages in thread
From: Matt Whitlock @ 2015-06-03 12:31 UTC (permalink / raw)
  To: Henrik Rydberg; +Cc: linux-input, Dmitry Torokhov

On Wednesday, 3 June 2015, at 2:24 pm, Henrik Rydberg wrote:
> > This patch changes the report_synaptics_data() function so that it reports
> > the highest touch_major and highest tool_major of any touching finger as
> > ABS_PRESSURE and ABS_TOOL_WIDTH, respectively.
> 
> I appreciate the problem, but I would much rather have bcm5974 converted
> to use input_mt_report_pointer_emulation() instead. The emulation code
> has been left as is for a long while, but this seems to be a good time
> to convert it.

report_tp_state() calls input_mt_sync_frame(), which in turn calls input_mt_report_pointer_emulation() already. So does this mean that report_synaptics_data() can be removed entirely?

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

* Re: [PATCH] Input: bcm5974 - report highest finger pressure to Synaptics
  2015-06-03 12:31   ` Matt Whitlock
@ 2015-06-03 12:52     ` Henrik Rydberg
  2015-06-09  1:07       ` [PATCH 1/2] Input: input_mt_report_pointer_emulation(), rescale pressure + tool width Matt Whitlock
  0 siblings, 1 reply; 11+ messages in thread
From: Henrik Rydberg @ 2015-06-03 12:52 UTC (permalink / raw)
  To: Matt Whitlock; +Cc: linux-input, Dmitry Torokhov

On 06/03/2015 02:31 PM, Matt Whitlock wrote:
> On Wednesday, 3 June 2015, at 2:24 pm, Henrik Rydberg wrote:
>>> This patch changes the report_synaptics_data() function so that it reports
>>> the highest touch_major and highest tool_major of any touching finger as
>>> ABS_PRESSURE and ABS_TOOL_WIDTH, respectively.
>>
>> I appreciate the problem, but I would much rather have bcm5974 converted
>> to use input_mt_report_pointer_emulation() instead. The emulation code
>> has been left as is for a long while, but this seems to be a good time
>> to convert it.
> 
> report_tp_state() calls input_mt_sync_frame(), which in turn calls input_mt_report_pointer_emulation() already. So does this mean that report_synaptics_data() can be removed entirely?
> 

The bcm5974 driver does not advertise ABS_MT_PRESSURE, and therefore
ABS_PRESSURE is not reported via input_mt_sync_frame(). Furthermore,
ABS_TOOL_WIDTH is not handled at all in input_mt_report_pointer_emulation().

To handle pressure automatically, one would need to advertise (and use)
ABS_MT_PRESSURE. There is a misnomer in the driver, let say for historical
reasons, so that ABS_MT_TOUCH_MAJOR gets reported in the ABS_PRESSURE field.
This is a bit of a problem, but quite solvable.

For tool width, it is hard to imagine ever being able to remove that
functionality completely. However, it can be transferred to
input_mt_report_pointer_emulation() instead, provided every user of that
function is analyzed for consequences and usage of ABS_MT_WIDTH_MAJOR.

It would be great to simply remove report_synaptics_data(), if possible,
retaining the precise behavior. Barring the bug, of course.

Thanks,
Henrik


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

* [PATCH 1/2] Input: input_mt_report_pointer_emulation(), rescale pressure + tool width
  2015-06-03 12:52     ` Henrik Rydberg
@ 2015-06-09  1:07       ` Matt Whitlock
  2015-06-09  1:07         ` [PATCH 2/2] Input: bcm5974 - report ABS_MT_PRESSURE + remove redundant emulation code Matt Whitlock
  0 siblings, 1 reply; 11+ messages in thread
From: Matt Whitlock @ 2015-06-09  1:07 UTC (permalink / raw)
  To: linux-input, Henrik Rydberg, Dmitry Torokhov; +Cc: Matt Whitlock

This patch teaches input_mt_report_pointer_emulation() to synthesize
ABS_TOOL_WIDTH from ABS_MT_WIDTH_MAJOR if the touchpad driver reports
ABS_MT_WIDTH_MAJOR. (As of this commit, the only in-tree drivers that do
are mouse/bcm5974.c and touchscreen/goodix.c.)

Additionally, this patch makes input_mt_report_pointer_emulation() rescale
values from each MT axis range to the respective emulated axis range to
allow touchpad drivers to redefine the emulated axis ranges after calling
input_mt_init_slots(). (The X and Y axes presently are not rescaled.)

Signed-off-by: Matt Whitlock <linux@mattwhitlock.name>
---
 drivers/input/input-mt.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/input/input-mt.c b/drivers/input/input-mt.c
index 54fce56..ccd55d9 100644
--- a/drivers/input/input-mt.c
+++ b/drivers/input/input-mt.c
@@ -23,6 +23,19 @@ static void copy_abs(struct input_dev *dev, unsigned int dst, unsigned int src)
 	}
 }
 
+static int rescale_abs_value(const struct input_dev *dev, unsigned int dst,
+			unsigned int src, int value)
+{
+	if (dev->absinfo && test_bit(dst, dev->absbit)
+			&& test_bit(src, dev->absbit)) {
+		struct input_absinfo *di = &dev->absinfo[dst], *si = &dev->absinfo[src];
+
+		value = di->minimum + (di->maximum - di->minimum) *
+				(value - si->minimum) / (si->maximum - si->minimum);
+	}
+	return value;
+}
+
 /**
  * input_mt_init_slots() - initialize MT input slots
  * @dev: input device supporting MT events and finger tracking
@@ -65,6 +78,7 @@ int input_mt_init_slots(struct input_dev *dev, unsigned int num_slots,
 		copy_abs(dev, ABS_X, ABS_MT_POSITION_X);
 		copy_abs(dev, ABS_Y, ABS_MT_POSITION_Y);
 		copy_abs(dev, ABS_PRESSURE, ABS_MT_PRESSURE);
+		copy_abs(dev, ABS_TOOL_WIDTH, ABS_MT_WIDTH_MAJOR);
 	}
 	if (flags & INPUT_MT_POINTER) {
 		__set_bit(BTN_TOOL_FINGER, dev->keybit);
@@ -230,11 +244,19 @@ void input_mt_report_pointer_emulation(struct input_dev *dev, bool use_count)
 
 		if (test_bit(ABS_MT_PRESSURE, dev->absbit)) {
 			int p = input_mt_get_value(oldest, ABS_MT_PRESSURE);
+			p = rescale_abs_value(dev, ABS_PRESSURE, ABS_MT_PRESSURE, p);
 			input_event(dev, EV_ABS, ABS_PRESSURE, p);
 		}
+		if (test_bit(ABS_MT_WIDTH_MAJOR, dev->absbit)) {
+			int w = input_mt_get_value(oldest, ABS_MT_WIDTH_MAJOR);
+			w = rescale_abs_value(dev, ABS_TOOL_WIDTH, ABS_MT_WIDTH_MAJOR, w);
+			input_event(dev, EV_ABS, ABS_TOOL_WIDTH, w);
+		}
 	} else {
 		if (test_bit(ABS_MT_PRESSURE, dev->absbit))
 			input_event(dev, EV_ABS, ABS_PRESSURE, 0);
+		if (test_bit(ABS_MT_WIDTH_MAJOR, dev->absbit))
+			input_event(dev, EV_ABS, ABS_TOOL_WIDTH, 0);
 	}
 }
 EXPORT_SYMBOL(input_mt_report_pointer_emulation);
-- 
2.4.2


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

* [PATCH 2/2] Input: bcm5974 - report ABS_MT_PRESSURE + remove redundant emulation code
  2015-06-09  1:07       ` [PATCH 1/2] Input: input_mt_report_pointer_emulation(), rescale pressure + tool width Matt Whitlock
@ 2015-06-09  1:07         ` Matt Whitlock
  2015-07-06 18:37           ` Matt Whitlock
  0 siblings, 1 reply; 11+ messages in thread
From: Matt Whitlock @ 2015-06-09  1:07 UTC (permalink / raw)
  To: linux-input, Henrik Rydberg, Dmitry Torokhov; +Cc: Matt Whitlock

Now that input_mt_report_pointer_emulation() can synthesize ABS_TOOL_WIDTH
from ABS_MT_WIDTH_MAJOR, the report_synaptics_data() function in bcm5974.c
is entirely redundant. This patch removes this function and introduces
reporting of ABS_MT_PRESSURE (faked from f->touch_major) to cause the
emulation code to synthesize ABS_PRESSURE.

Signed-off-by: Matt Whitlock <linux@mattwhitlock.name>

---

== Test case #1: Single-finger touch ==

Old behavior:

Event: time 1433421663.308600, type 3 (EV_ABS), code 57 (ABS_MT_TRACKING_ID), value 0
Event: time 1433421663.308600, type 3 (EV_ABS), code 48 (ABS_MT_TOUCH_MAJOR), value 316
Event: time 1433421663.308600, type 3 (EV_ABS), code 49 (ABS_MT_TOUCH_MINOR), value 668
Event: time 1433421663.308600, type 3 (EV_ABS), code 50 (ABS_MT_WIDTH_MAJOR), value 1840
Event: time 1433421663.308600, type 3 (EV_ABS), code 51 (ABS_MT_WIDTH_MINOR), value 1482
Event: time 1433421663.308600, type 3 (EV_ABS), code 52 (ABS_MT_ORIENTATION), value 13588
Event: time 1433421663.308600, type 3 (EV_ABS), code 53 (ABS_MT_POSITION_X), value 434
Event: time 1433421663.308600, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value 3148
Event: time 1433421663.308600, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 1
Event: time 1433421663.308600, type 1 (EV_KEY), code 325 (BTN_TOOL_FINGER), value 1
Event: time 1433421663.308600, type 3 (EV_ABS), code 0 (ABS_X), value 434
Event: time 1433421663.308600, type 3 (EV_ABS), code 1 (ABS_Y), value 3148
Event: time 1433421663.308600, type 3 (EV_ABS), code 24 (ABS_PRESSURE), value 134
Event: time 1433421663.308600, type 3 (EV_ABS), code 28 (ABS_TOOL_WIDTH), value 7
Event: time 1433421663.308600, -------------- EV_SYN ------------

New behavior:

Event: time 1433421077.994753, type 3 (EV_ABS), code 57 (ABS_MT_TRACKING_ID), value 0
Event: time 1433421077.994753, type 3 (EV_ABS), code 48 (ABS_MT_TOUCH_MAJOR), value 224
Event: time 1433421077.994753, type 3 (EV_ABS), code 49 (ABS_MT_TOUCH_MINOR), value 466
Event: time 1433421077.994753, type 3 (EV_ABS), code 50 (ABS_MT_WIDTH_MAJOR), value 1616
Event: time 1433421077.994753, type 3 (EV_ABS), code 51 (ABS_MT_WIDTH_MINOR), value 1464
Event: time 1433421077.994753, type 3 (EV_ABS), code 53 (ABS_MT_POSITION_X), value 501
Event: time 1433421077.994753, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value 3288
Event: time 1433421077.994753, type 3 (EV_ABS), code 58 (ABS_MT_PRESSURE), value 112
Event: time 1433421077.994753, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 1
Event: time 1433421077.994753, type 1 (EV_KEY), code 325 (BTN_TOOL_FINGER), value 1
Event: time 1433421077.994753, type 3 (EV_ABS), code 0 (ABS_X), value 501
Event: time 1433421077.994753, type 3 (EV_ABS), code 1 (ABS_Y), value 3288
Event: time 1433421077.994753, type 3 (EV_ABS), code 24 (ABS_PRESSURE), value 95
Event: time 1433421077.994753, type 3 (EV_ABS), code 28 (ABS_TOOL_WIDTH), value 11
Event: time 1433421077.994753, -------------- EV_SYN ------------

== Test case #2: Two-finger touch ==

Old behavior:

Event: time 1433421678.186983, type 3 (EV_ABS), code 57 (ABS_MT_TRACKING_ID), value 1
Event: time 1433421678.186983, type 3 (EV_ABS), code 48 (ABS_MT_TOUCH_MAJOR), value 384
Event: time 1433421678.186983, type 3 (EV_ABS), code 49 (ABS_MT_TOUCH_MINOR), value 648
Event: time 1433421678.186983, type 3 (EV_ABS), code 51 (ABS_MT_WIDTH_MINOR), value 1672
Event: time 1433421678.186983, type 3 (EV_ABS), code 52 (ABS_MT_ORIENTATION), value 0
Event: time 1433421678.186983, type 3 (EV_ABS), code 53 (ABS_MT_POSITION_X), value -1810
Event: time 1433421678.186983, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value 3298
Event: time 1433421678.186983, type 3 (EV_ABS), code 47 (ABS_MT_SLOT), value 1
Event: time 1433421678.186983, type 3 (EV_ABS), code 57 (ABS_MT_TRACKING_ID), value 2
Event: time 1433421678.186983, type 3 (EV_ABS), code 48 (ABS_MT_TOUCH_MAJOR), value 404
Event: time 1433421678.186983, type 3 (EV_ABS), code 49 (ABS_MT_TOUCH_MINOR), value 616
Event: time 1433421678.186983, type 3 (EV_ABS), code 50 (ABS_MT_WIDTH_MAJOR), value 1950
Event: time 1433421678.186983, type 3 (EV_ABS), code 51 (ABS_MT_WIDTH_MINOR), value 1488
Event: time 1433421678.186983, type 3 (EV_ABS), code 52 (ABS_MT_ORIENTATION), value 16009
Event: time 1433421678.186983, type 3 (EV_ABS), code 53 (ABS_MT_POSITION_X), value 2076
Event: time 1433421678.186983, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value 3384
Event: time 1433421678.186983, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 1
Event: time 1433421678.186983, type 1 (EV_KEY), code 333 (BTN_TOOL_DOUBLETAP), value 1
Event: time 1433421678.186983, type 3 (EV_ABS), code 0 (ABS_X), value -1810
Event: time 1433421678.186983, type 3 (EV_ABS), code 1 (ABS_Y), value 3298
Event: time 1433421678.186983, type 3 (EV_ABS), code 24 (ABS_PRESSURE), value 163
Event: time 1433421678.186983, type 3 (EV_ABS), code 28 (ABS_TOOL_WIDTH), value 7
Event: time 1433421678.186983, -------------- EV_SYN ------------

New behavior:

Event: time 1433421166.097247, type 3 (EV_ABS), code 57 (ABS_MT_TRACKING_ID), value 1
Event: time 1433421166.097247, type 3 (EV_ABS), code 48 (ABS_MT_TOUCH_MAJOR), value 285
Event: time 1433421166.097247, type 3 (EV_ABS), code 49 (ABS_MT_TOUCH_MINOR), value 483
Event: time 1433421166.097247, type 3 (EV_ABS), code 50 (ABS_MT_WIDTH_MAJOR), value 1840
Event: time 1433421166.097247, type 3 (EV_ABS), code 51 (ABS_MT_WIDTH_MINOR), value 1696
Event: time 1433421166.097247, type 3 (EV_ABS), code 53 (ABS_MT_POSITION_X), value -1718
Event: time 1433421166.097247, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value 3518
Event: time 1433421166.097247, type 3 (EV_ABS), code 58 (ABS_MT_PRESSURE), value 180
Event: time 1433421166.097247, type 3 (EV_ABS), code 47 (ABS_MT_SLOT), value 1
Event: time 1433421166.097247, type 3 (EV_ABS), code 57 (ABS_MT_TRACKING_ID), value 2
Event: time 1433421166.097247, type 3 (EV_ABS), code 48 (ABS_MT_TOUCH_MAJOR), value 364
Event: time 1433421166.097247, type 3 (EV_ABS), code 49 (ABS_MT_TOUCH_MINOR), value 554
Event: time 1433421166.097247, type 3 (EV_ABS), code 50 (ABS_MT_WIDTH_MAJOR), value 1794
Event: time 1433421166.097247, type 3 (EV_ABS), code 51 (ABS_MT_WIDTH_MINOR), value 1738
Event: time 1433421166.097247, type 3 (EV_ABS), code 53 (ABS_MT_POSITION_X), value 2501
Event: time 1433421166.097247, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value 3276
Event: time 1433421166.097247, type 3 (EV_ABS), code 58 (ABS_MT_PRESSURE), value 182
Event: time 1433421166.097247, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 1
Event: time 1433421166.097247, type 1 (EV_KEY), code 333 (BTN_TOOL_DOUBLETAP), value 1
Event: time 1433421166.097247, type 3 (EV_ABS), code 0 (ABS_X), value -1718
Event: time 1433421166.097247, type 3 (EV_ABS), code 1 (ABS_Y), value 3518
Event: time 1433421166.097247, type 3 (EV_ABS), code 24 (ABS_PRESSURE), value 153
Event: time 1433421166.097247, type 3 (EV_ABS), code 28 (ABS_TOOL_WIDTH), value 13
Event: time 1433421166.097247, -------------- EV_SYN ------------

== Test case #3: Side-of-thumb touch ==

Old behavior:

Event: time 1433421696.969475, type 3 (EV_ABS), code 47 (ABS_MT_SLOT), value 0
Event: time 1433421696.969475, type 3 (EV_ABS), code 57 (ABS_MT_TRACKING_ID), value 3
Event: time 1433421696.969475, type 3 (EV_ABS), code 48 (ABS_MT_TOUCH_MAJOR), value 952
Event: time 1433421696.969475, type 3 (EV_ABS), code 49 (ABS_MT_TOUCH_MINOR), value 642
Event: time 1433421696.969475, type 3 (EV_ABS), code 50 (ABS_MT_WIDTH_MAJOR), value 3810
Event: time 1433421696.969475, type 3 (EV_ABS), code 52 (ABS_MT_ORIENTATION), value -14967
Event: time 1433421696.969475, type 3 (EV_ABS), code 53 (ABS_MT_POSITION_X), value 678
Event: time 1433421696.969475, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value 3379
Event: time 1433421696.969475, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 1
Event: time 1433421696.969475, type 1 (EV_KEY), code 325 (BTN_TOOL_FINGER), value 1
Event: time 1433421696.969475, type 3 (EV_ABS), code 0 (ABS_X), value 678
Event: time 1433421696.969475, type 3 (EV_ABS), code 1 (ABS_Y), value 3379
Event: time 1433421696.969475, type 3 (EV_ABS), code 24 (ABS_PRESSURE), value 255
Event: time 1433421696.969475, type 3 (EV_ABS), code 28 (ABS_TOOL_WIDTH), value 14
Event: time 1433421696.969475, -------------- EV_SYN ------------

New behavior:

Event: time 1433421224.271508, type 3 (EV_ABS), code 57 (ABS_MT_TRACKING_ID), value 3
Event: time 1433421224.271508, type 3 (EV_ABS), code 48 (ABS_MT_TOUCH_MAJOR), value 1020
Event: time 1433421224.271508, type 3 (EV_ABS), code 49 (ABS_MT_TOUCH_MINOR), value 606
Event: time 1433421224.271508, type 3 (EV_ABS), code 50 (ABS_MT_WIDTH_MAJOR), value 3828
Event: time 1433421224.271508, type 3 (EV_ABS), code 51 (ABS_MT_WIDTH_MINOR), value 1828
Event: time 1433421224.271508, type 3 (EV_ABS), code 52 (ABS_MT_ORIENTATION), value -16206
Event: time 1433421224.271508, type 3 (EV_ABS), code 53 (ABS_MT_POSITION_X), value 193
Event: time 1433421224.271508, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value 3119
Event: time 1433421224.271508, type 3 (EV_ABS), code 58 (ABS_MT_PRESSURE), value 300
Event: time 1433421224.271508, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 1
Event: time 1433421224.271508, type 1 (EV_KEY), code 325 (BTN_TOOL_FINGER), value 1
Event: time 1433421224.271508, type 3 (EV_ABS), code 0 (ABS_X), value 193
Event: time 1433421224.271508, type 3 (EV_ABS), code 1 (ABS_Y), value 3119
Event: time 1433421224.271508, type 3 (EV_ABS), code 24 (ABS_PRESSURE), value 255
Event: time 1433421224.271508, type 3 (EV_ABS), code 28 (ABS_TOOL_WIDTH), value 28
Event: time 1433421224.271508, -------------- EV_SYN ------------
---
 drivers/input/mouse/bcm5974.c | 41 ++++++++++++++---------------------------
 1 file changed, 14 insertions(+), 27 deletions(-)

diff --git a/drivers/input/mouse/bcm5974.c b/drivers/input/mouse/bcm5974.c
index b10709f..7e5df76 100644
--- a/drivers/input/mouse/bcm5974.c
+++ b/drivers/input/mouse/bcm5974.c
@@ -458,10 +458,6 @@ static void setup_events_to_report(struct input_dev *input_dev,
 {
 	__set_bit(EV_ABS, input_dev->evbit);
 
-	/* for synaptics only */
-	input_set_abs_params(input_dev, ABS_PRESSURE, 0, 256, 5, 0);
-	input_set_abs_params(input_dev, ABS_TOOL_WIDTH, 0, 16, 0, 0);
-
 	/* finger touch area */
 	set_abs(input_dev, ABS_MT_TOUCH_MAJOR, &cfg->w);
 	set_abs(input_dev, ABS_MT_TOUCH_MINOR, &cfg->w);
@@ -473,6 +469,8 @@ static void setup_events_to_report(struct input_dev *input_dev,
 	/* finger position */
 	set_abs(input_dev, ABS_MT_POSITION_X, &cfg->x);
 	set_abs(input_dev, ABS_MT_POSITION_Y, &cfg->y);
+	/* finger pressure (synthesized from touch area) */
+	set_abs(input_dev, ABS_MT_PRESSURE, &cfg->p);
 
 	__set_bit(EV_KEY, input_dev->evbit);
 	__set_bit(BTN_LEFT, input_dev->keybit);
@@ -482,6 +480,10 @@ static void setup_events_to_report(struct input_dev *input_dev,
 
 	input_mt_init_slots(input_dev, MAX_FINGERS,
 		INPUT_MT_POINTER | INPUT_MT_DROP_UNUSED | INPUT_MT_TRACK);
+
+	/* override axis ranges for synaptics */
+	input_set_abs_params(input_dev, ABS_PRESSURE, 0, 256, 5, 0);
+	input_set_abs_params(input_dev, ABS_TOOL_WIDTH, 0, 16, 0, 0);
 }
 
 /* report button data as logical button state */
@@ -501,10 +503,13 @@ static int report_bt_state(struct bcm5974 *dev, int size)
 	return 0;
 }
 
-static void report_finger_data(struct input_dev *input, int slot,
+static void report_finger_data(struct bcm5974 *dev, int slot,
 			       const struct input_mt_pos *pos,
 			       const struct tp_finger *f)
 {
+	struct input_dev *input = dev->input;
+	const struct bcm5974_config *cfg = &dev->cfg;
+
 	input_mt_slot(input, slot);
 	input_mt_report_slot_state(input, MT_TOOL_FINGER, true);
 
@@ -520,25 +525,9 @@ static void report_finger_data(struct input_dev *input, int slot,
 			 MAX_FINGER_ORIENTATION - raw2int(f->orientation));
 	input_report_abs(input, ABS_MT_POSITION_X, pos->x);
 	input_report_abs(input, ABS_MT_POSITION_Y, pos->y);
-}
-
-static void report_synaptics_data(struct input_dev *input,
-				  const struct bcm5974_config *cfg,
-				  const struct tp_finger *f, int raw_n)
-{
-	int abs_p = 0, abs_w = 0;
-
-	if (raw_n) {
-		int p = raw2int(f->touch_major);
-		int w = raw2int(f->tool_major);
-		if (p > 0 && raw2int(f->origin)) {
-			abs_p = clamp_val(256 * p / cfg->p.max, 0, 255);
-			abs_w = clamp_val(16 * w / cfg->w.max, 0, 15);
-		}
-	}
-
-	input_report_abs(input, ABS_PRESSURE, abs_p);
-	input_report_abs(input, ABS_TOOL_WIDTH, abs_w);
+	/* clamp to avoid two-finger emulation in synaptics driver */
+	input_report_abs(input, ABS_MT_PRESSURE,
+			 clamp_val(raw2int(f->touch_major), cfg->p.min, cfg->p.max));
 }
 
 /* report trackpad data as logical trackpad state */
@@ -567,13 +556,11 @@ static int report_tp_state(struct bcm5974 *dev, int size)
 	input_mt_assign_slots(input, dev->slots, dev->pos, n, 0);
 
 	for (i = 0; i < n; i++)
-		report_finger_data(input, dev->slots[i],
+		report_finger_data(dev, dev->slots[i],
 				   &dev->pos[i], dev->index[i]);
 
 	input_mt_sync_frame(input);
 
-	report_synaptics_data(input, c, f, raw_n);
-
 	/* type 2 reports button events via ibt only */
 	if (c->tp_type == TYPE2) {
 		int ibt = raw2int(dev->tp_data[BUTTON_TYPE2]);
-- 
2.4.2


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

* Re: [PATCH 2/2] Input: bcm5974 - report ABS_MT_PRESSURE + remove redundant emulation code
  2015-06-09  1:07         ` [PATCH 2/2] Input: bcm5974 - report ABS_MT_PRESSURE + remove redundant emulation code Matt Whitlock
@ 2015-07-06 18:37           ` Matt Whitlock
  2015-07-06 19:11             ` Henrik Rydberg
  0 siblings, 1 reply; 11+ messages in thread
From: Matt Whitlock @ 2015-07-06 18:37 UTC (permalink / raw)
  To: linux-input, Henrik Rydberg, Dmitry Torokhov

It's been four weeks, and I haven't heard back on this patch. I believe I've implemented what was requested. Is there a problem with it?


On Monday, 8 June 2015, at 9:07 pm, Matt Whitlock wrote:
> Now that input_mt_report_pointer_emulation() can synthesize ABS_TOOL_WIDTH
> from ABS_MT_WIDTH_MAJOR, the report_synaptics_data() function in bcm5974.c
> is entirely redundant. This patch removes this function and introduces
> reporting of ABS_MT_PRESSURE (faked from f->touch_major) to cause the
> emulation code to synthesize ABS_PRESSURE.
> 
> Signed-off-by: Matt Whitlock <linux@mattwhitlock.name>

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

* Re: [PATCH 2/2] Input: bcm5974 - report ABS_MT_PRESSURE + remove redundant emulation code
  2015-07-06 18:37           ` Matt Whitlock
@ 2015-07-06 19:11             ` Henrik Rydberg
  2015-07-06 19:35               ` Matt Whitlock
  0 siblings, 1 reply; 11+ messages in thread
From: Henrik Rydberg @ 2015-07-06 19:11 UTC (permalink / raw)
  To: Matt Whitlock, linux-input, Dmitry Torokhov

Hi Matt,

> It's been four weeks, and I haven't heard back on this patch. I believe I've
> implemented what was requested. Is there a problem with it?

I thought you did get response on this patch set, concerning the fabrication of
force data? I seem to remember a mail from Dmitry about this, although I cannot
find it now.

I think your patch is a good reduction, all in accord with what we talked about,
but I have doubts that it will not create regressions. From what I have seen so
far, patch-wise, the best solution is to leave the code as it is.

I understand that the constraints now seem to depict the empty set, but maybe
there is a third solution.

Henrik


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

* Re: [PATCH 2/2] Input: bcm5974 - report ABS_MT_PRESSURE + remove redundant emulation code
  2015-07-06 19:11             ` Henrik Rydberg
@ 2015-07-06 19:35               ` Matt Whitlock
  2015-07-06 19:44                 ` Henrik Rydberg
  0 siblings, 1 reply; 11+ messages in thread
From: Matt Whitlock @ 2015-07-06 19:35 UTC (permalink / raw)
  To: Henrik Rydberg; +Cc: linux-input, Dmitry Torokhov

On Monday, 6 July 2015, at 9:11 pm, Henrik Rydberg wrote:
> > It's been four weeks, and I haven't heard back on this patch. I believe I've
> > implemented what was requested. Is there a problem with it?
> 
> I thought you did get response on this patch set, concerning the fabrication of
> force data? I seem to remember a mail from Dmitry about this, although I cannot
> find it now.

I don't have anything from Dmitry at all, even in my spam box. Maybe he discussed it with you only?

> I think your patch is a good reduction, all in accord with what we talked about,
> but I have doubts that it will not create regressions. From what I have seen so
> far, patch-wise, the best solution is to leave the code as it is.

So you believe that the current (in-tree) behavior is superior to either of my attempts thus far to fix it?

> I understand that the constraints now seem to depict the empty set, but maybe
> there is a third solution.

Can you characterize what an acceptable solution would look like? The behavior of the code as it stands is really not acceptable; the MacBook Pro touchpad's buggy behavior in Linux is extremely frustrating, relative to its flawless behavior in OS X. My patchset has not introduced any regressions that I can observe, and in fact it has nearly eliminated the spurious drags. (I still get one occasionally, but maybe at a rate of ~1% as often as I was seeing them before.)

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

* Re: [PATCH 2/2] Input: bcm5974 - report ABS_MT_PRESSURE + remove redundant emulation code
  2015-07-06 19:35               ` Matt Whitlock
@ 2015-07-06 19:44                 ` Henrik Rydberg
  2015-07-06 19:59                   ` Matt Whitlock
  0 siblings, 1 reply; 11+ messages in thread
From: Henrik Rydberg @ 2015-07-06 19:44 UTC (permalink / raw)
  To: Matt Whitlock; +Cc: linux-input, Dmitry Torokhov

>> I think your patch is a good reduction, all in accord with what we talked about,
>> but I have doubts that it will not create regressions. From what I have seen so
>> far, patch-wise, the best solution is to leave the code as it is.
> 
> So you believe that the current (in-tree) behavior is superior to either of my attempts thus far to fix it?

That is not at all what I said. I like the result of your patch set. The
question is how it works with linux in general.

>> I understand that the constraints now seem to depict the empty set, but maybe
>> there is a third solution.
> 
> Can you characterize what an acceptable solution would look like? The behavior of the code as it stands is really not acceptable; the MacBook Pro touchpad's buggy behavior in Linux is extremely frustrating, relative to its flawless behavior in OS X. My patchset has not introduced any regressions that I can observe, and in fact it has nearly eliminated the spurious drags. (I still get one occasionally, but maybe at a rate of ~1% as often as I was seeing them before.)

The constraints so far are:

1. Make the situation on the problematic devices better
2. Do not invent sensor data
3. Do not create regressions

Taken together, it suggests that the patch should be similar, but not equal to,
the original patch.

Thanks,
Henrik


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

* Re: [PATCH 2/2] Input: bcm5974 - report ABS_MT_PRESSURE + remove redundant emulation code
  2015-07-06 19:44                 ` Henrik Rydberg
@ 2015-07-06 19:59                   ` Matt Whitlock
  0 siblings, 0 replies; 11+ messages in thread
From: Matt Whitlock @ 2015-07-06 19:59 UTC (permalink / raw)
  To: Henrik Rydberg; +Cc: linux-input, Dmitry Torokhov

On Monday, 6 July 2015, at 9:44 pm, Henrik Rydberg wrote:
> >> I think your patch is a good reduction, all in accord with what we talked about,
> >> but I have doubts that it will not create regressions. From what I have seen so
> >> far, patch-wise, the best solution is to leave the code as it is.
> > 
> > So you believe that the current (in-tree) behavior is superior to either of my attempts thus far to fix it?
> 
> That is not at all what I said. I like the result of your patch set. The
> question is how it works with linux in general.

I don't understand "works with Linux in general." Does my patch set incorrectly call the evdev MT API? With what else in Linux might it be incompatible? Or are you talking more generally about the X.org synaptics input driver and various client applications that consume touch data? No input axes are removed by this patch set, and it maintains the (arguably incorrect) status quo of synthesizing ABS_PRESSURE from the major tool width axis, so why do you anticipate regressions?

> The constraints so far are:
> 
> 1. Make the situation on the problematic devices better
> 2. Do not invent sensor data
> 3. Do not create regressions
> 
> Taken together, it suggests that the patch should be similar, but not equal to,
> the original patch.

I believe that I have achieved #1 and #3. I am not sure how best to achieve #2, given that the *existing* code invents ABS_PRESSURE. If I were to change the code so that it no longer invents sensor data (to comply with #2), then I would almost certainly create regressions (thus violating #3).

Perhaps I could eliminate reporting of ABS_PRESSURE and also write a patch for the synaptics driver so that it can alternatively utilize ABS_TOOL_WIDTH in the absence of ABS_PRESSURE. But such a cross-project patch set would be difficult to apply and deploy.

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

end of thread, other threads:[~2015-07-06 19:59 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-06-03 11:49 [PATCH] Input: bcm5974 - report highest finger pressure to Synaptics Matt Whitlock
2015-06-03 12:24 ` Henrik Rydberg
2015-06-03 12:31   ` Matt Whitlock
2015-06-03 12:52     ` Henrik Rydberg
2015-06-09  1:07       ` [PATCH 1/2] Input: input_mt_report_pointer_emulation(), rescale pressure + tool width Matt Whitlock
2015-06-09  1:07         ` [PATCH 2/2] Input: bcm5974 - report ABS_MT_PRESSURE + remove redundant emulation code Matt Whitlock
2015-07-06 18:37           ` Matt Whitlock
2015-07-06 19:11             ` Henrik Rydberg
2015-07-06 19:35               ` Matt Whitlock
2015-07-06 19:44                 ` Henrik Rydberg
2015-07-06 19:59                   ` Matt Whitlock

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).