From: Benjamin Tissoires <benjamin.tissoires@redhat.com>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
Peter Hutterer <peter.hutterer@who-t.net>,
Hans de Goede <hdegoede@redhat.com>,
Henrik Rydberg <rydberg@bitmath.org>
Subject: Re: [PATCH 2/2] Input - synaptics: pin 3 touches when the firmware reports 3 fingers
Date: Fri, 24 Apr 2015 18:50:49 -0400 [thread overview]
Message-ID: <20150424225049.GA14791@mail.corp.redhat.com> (raw)
In-Reply-To: <20150423184825.GB10937@mail.corp.redhat.com>
Hi Dmitry,
[ adding more relevant people to the discussion ]
On Apr 23 2015 or thereabouts, Benjamin Tissoires wrote:
> On Apr 23 2015 or thereabouts, Dmitry Torokhov wrote:
> > On Wed, Apr 22, 2015 at 11:45:09AM -0400, Benjamin Tissoires wrote:
> > > Synaptics PS/2 touchpad can send only 2 touches in a report. They can
> > > detect 4 or 5 and this information is valuable.
> > >
> > > In commit 63c4fda3c0bb ("Input: synaptics - allocate 3 slots to keep
> > > stability in image sensors"), we allocate 3 slots, but we still continue
> > > to report the 2 available fingers. That means that the client sees 2 used
> > > slots while there is a total of 3 fingers advertised by BTN_TOOL_TRIPLETAP.
> > >
> > > For old kernels this is not a problem because max_slots was 2 and libinput/
> > > xorg-synaptics knew how to deal with that. Now that max_slot is 3, the
> > > clients ignore BTN_TOOL_TRIPLETAP and count the actual used slots (so 2).
> > > It then gets confused when receiving the BTN_TOOL_TRIPLETAP and DOUBLETAP
> > > information, and goes wild.
> > >
> > > We can pin the 3 slots until we get a total number of fingers below 2.
> > >
> > > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1212230
> >
> > Benjamin, I do not quite like it. It seems that original patch was not
> > quite right and we are adding more workarounds.
>
> Agree. And I am starting to hate more and more the synaptics PS/2 and all
> the PS/2 drivers to be honest :) - trying to fit a heavy load data like
> multitouch in a simple and lightweight protocol like PS/2 is insane...
>
> We are internally trying to figure out if we can finally take advantage
> of the SMBus/RMI4 protocol, but we tried for one year without much
> success.
>
> >
> > Synaptics can only track 2 contacts, correct? Why 2 slots to track them
> > is not enough?
>
> IIRC, the problem was that upon a third finger down, with only 2 slots,
> the fingers were silently inverted in most cases. The thing is that the
> firmware forwards 2 fingers, but not necessarily the two first. So you
> generally get fingers 1+3 so the slot 2 needs to be removed. And that
> means the kernel tracking has to track 3 fingers upon transitions.
>
> This may be completely bullshit and we might not need to use 3 slots at
> all. I'll need to do further experiments to validate which one is best
> then.
>
> I am perfectly fine holding this one up for a little bit more testings
> and then we can decide which one needs to be done (revert or an other
> band-aid).
>
So I carefully recorded each situation (initial with 2 slots, 2 slots
and then with the pinning in this patch*), and I am now convinced that
the pinning is the best sequence that we forward to the user space (best
among the 3).
With 2 slots declared, there are 2 problems:
- the first finger jumps to the position of the 3rd when it lands
- the transition between 2 to 3 fingers goes to a state where the kernel
removes the second finger (while jumping the first to the position of
the 3rd finger), send a sync and then reallocate the first finger
position as the second slot in use
-> that means that user space sees a small transition where the slots
count is 1 while the BTN_TOOL advertise triple tap :/
With 3 slots, we have the problem reported in the rhbz bug #1212230:
- during the transition, the fingers are stable, but we have at most 2
active slots in one frame, which confuses libinput/xorg-synaptics.
With the pinning, the user space is no more confused because BTN_TOOL is
always greater or equal than the active slots.
So I think for now we have 3 possibilities:
1. Just carry this patch, and hope that we will be able to switch the
synaptics device in the non-PS/2 mode
2. Revert to 2 patches and fix the kernel tracking to accept 3 fingers
and return the 2 best matches
3. Revert the use of the kernel tracking at all and re-introduce the
spaghetti code that was here before and hope that all cases where
properly handled.
IMO that the solution 2. is the best, but I can not do it because I
don't understand what the code does. I can guess things but I can not
accurately change it because it is not readable IMO.
(yes, there is also the solution 4: "screw up and let the user space deal
with it", but I'd rather not do that given the history of the multitouch
protocol)
Cheers,
Benjamin
* I tried to put side by side the 3 test cases in the following logs:
(init slots 2, no pinning) | (init slots 3, no pinning) | (init slots 3, pinning)
----------------------------- | ----------------------------- | ---------------------------
------------------------------|----- one finger down -------|----------------------------
| |
ABS_MT_SLOT 0 | ABS_MT_SLOT 0 | ABS_MT_SLOT 0
ABS_MT_TRACKING_ID 53 | ABS_MT_TRACKING_ID 53 | ABS_MT_TRACKING_ID 53
ABS_MT_POSITION_X 2000 | ABS_MT_POSITION_X 2000 | ABS_MT_POSITION_X 2000
ABS_MT_POSITION_Y 2000 | ABS_MT_POSITION_Y 2000 | ABS_MT_POSITION_Y 2000
ABS_MT_PRESSURE 68 | ABS_MT_PRESSURE 68 | ABS_MT_PRESSURE 68
BTN_TOUCH 1 | BTN_TOUCH 1 | BTN_TOUCH 1
ABS_X 2000 | ABS_X 2000 | ABS_X 2000
ABS_Y 2000 | ABS_Y 2000 | ABS_Y 2000
ABS_PRESSURE 68 | ABS_PRESSURE 68 | ABS_PRESSURE 68
BTN_TOOL_FINGER 1 | BTN_TOOL_FINGER 1 | BTN_TOOL_FINGER 1
--- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ----------
| |
... | ... | ...
| |
------------------------------------ second finger down ------------------------------------
| |
--- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ----------
ABS_MT_POSITION_X 2000 | ABS_MT_POSITION_X 2000 | ABS_MT_POSITION_X 2000
ABS_MT_POSITION_Y 2000 | ABS_MT_POSITION_Y 2000 | ABS_MT_POSITION_Y 2000
ABS_MT_PRESSURE 78 | ABS_MT_PRESSURE 78 | ABS_MT_PRESSURE 78
ABS_MT_SLOT 1 | ABS_MT_SLOT 1 | ABS_MT_SLOT 1
ABS_MT_TRACKING_ID 54 | ABS_MT_TRACKING_ID 54 | ABS_MT_TRACKING_ID 54
ABS_MT_POSITION_X 3000 | ABS_MT_POSITION_X 3000 | ABS_MT_POSITION_X 3000
ABS_MT_POSITION_Y 3000 | ABS_MT_POSITION_Y 3000 | ABS_MT_POSITION_Y 3000
ABS_MT_PRESSURE 64 | ABS_MT_PRESSURE 64 | ABS_MT_PRESSURE 64
ABS_X 2000 | ABS_X 2000 | ABS_X 2000
ABS_Y 2000 | ABS_Y 2000 | ABS_Y 2000
ABS_PRESSURE 78 | ABS_PRESSURE 78 | ABS_PRESSURE 78
BTN_TOOL_FINGER 0 | BTN_TOOL_FINGER 0 | BTN_TOOL_FINGER 0
BTN_TOOL_DOUBLETAP 1 | BTN_TOOL_DOUBLETAP 1 | BTN_TOOL_DOUBLETAP 1
--- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ----------
| |
... | ... | ...
| |
------------------------------------ third finger down ------------------------------------
| |
--- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ----------
ABS_MT_SLOT 0 | ABS_MT_SLOT 0 | ABS_MT_SLOT 0
ABS_MT_POSITION_X 4000 | ABS_MT_POSITION_X 2000 | ABS_MT_POSITION_X 2000
ABS_MT_POSITION_Y 4000 | ABS_MT_POSITION_Y 2000 | ABS_MT_POSITION_Y 2000
ABS_MT_PRESSURE 78 | ABS_MT_PRESSURE 78 | ABS_MT_PRESSURE 78
ABS_MT_SLOT 1 | ABS_MT_SLOT 2 | ABS_MT_SLOT 2
ABS_MT_TRACKING_ID -1 | ABS_MT_TRACKING_ID 55 | ABS_MT_TRACKING_ID 55
| ABS_MT_POSITION_X 4000 | ABS_MT_POSITION_X 4000
| ABS_MT_POSITION_Y 4000 | ABS_MT_POSITION_Y 4000
| ABS_MT_PRESSURE 72 | ABS_MT_PRESSURE 72
| ABS_MT_SLOT 1 |
| ABS_MT_TRACKING_ID -1 |
ABS_X 4000 | ABS_X 2000 | ABS_X 2000
ABS_Y 4000 | ABS_Y 2000 | ABS_Y 2000
ABS_PRESSURE 78 | ABS_PRESSURE 78 | ABS_PRESSURE 78
BTN_TOOL_DOUBLETAP 0 | BTN_TOOL_DOUBLETAP 0 | BTN_TOOL_DOUBLETAP 0
BTN_TOOL_TRIPLETAP 1 | BTN_TOOL_TRIPLETAP 1 | BTN_TOOL_TRIPLETAP 1
--- SYN_REPORT (0) ---------- | |
ABS_MT_SLOT 0 | |
ABS_MT_POSITION_X 4000 | |
ABS_MT_POSITION_Y 4000 | |
ABS_MT_PRESSURE 78 | |
ABS_MT_SLOT 1 | |
ABS_MT_TRACKING_ID 55 | |
ABS_MT_POSITION_X 2000 | |
ABS_MT_POSITION_Y 2000 | |
ABS_MT_PRESSURE 72 | |
ABS_X 4000 | |
ABS_Y 4000 | |
ABS_PRESSURE 78 | |
--- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ----------
| |
... | ... | ...
| |
------------------------------------ 3 fingers release ------------------------------------
| |
| |
--- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ----------
| | ABS_MT_SLOT 2
| | ABS_MT_POSITION_X 4000
| | ABS_MT_POSITION_Y 4000
| | ABS_MT_PRESSURE 45
ABS_MT_SLOT 0 | ABS_MT_SLOT 0 | ABS_MT_SLOT 0
ABS_MT_TRACKING_ID -1 | ABS_MT_TRACKING_ID -1 | ABS_MT_TRACKING_ID -1
ABS_MT_SLOT 1 | ABS_MT_SLOT 2 | ABS_MT_SLOT 1
ABS_MT_TRACKING_ID -1 | ABS_MT_TRACKING_ID -1 | ABS_MT_TRACKING_ID -1
| | ABS_X 4000
| | ABS_Y 4000
BTN_TOUCH 0 | BTN_TOUCH 0 | ABS_PRESSURE 45
ABS_PRESSURE 0 | ABS_PRESSURE 0 | BTN_TOOL_FINGER 1
BTN_TOOL_TRIPLETAP 0 | BTN_TOOL_TRIPLETAP 0 | BTN_TOOL_TRIPLETAP 0
--- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ----------
| | ABS_MT_SLOT 2
| | ABS_MT_TRACKING_ID -1
| | BTN_TOUCH 0
| | ABS_PRESSURE 0
| | BTN_TOOL_FINGER 0
| | --- SYN_REPORT (0) ----------
next prev parent reply other threads:[~2015-04-24 22:50 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-04-22 15:45 [PATCH 0/2] 2 somewhat related input fixes Benjamin Tissoires
2015-04-22 15:45 ` [PATCH 1/2] Input - elantech: fix semi-mt protocol for v3 HW Benjamin Tissoires
2015-04-23 16:09 ` Dmitry Torokhov
2015-04-22 15:45 ` [PATCH 2/2] Input - synaptics: pin 3 touches when the firmware reports 3 fingers Benjamin Tissoires
2015-04-23 16:38 ` Dmitry Torokhov
2015-04-23 18:48 ` Benjamin Tissoires
2015-04-24 22:50 ` Benjamin Tissoires [this message]
2015-04-25 9:40 ` Henrik Rydberg
2015-04-27 17:48 ` Benjamin Tissoires
2015-06-11 17:29 ` Benjamin Tissoires
2015-07-01 0:26 ` Dmitry Torokhov
2015-07-07 14:02 ` Benjamin Tissoires
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150424225049.GA14791@mail.corp.redhat.com \
--to=benjamin.tissoires@redhat.com \
--cc=dmitry.torokhov@gmail.com \
--cc=hdegoede@redhat.com \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=peter.hutterer@who-t.net \
--cc=rydberg@bitmath.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.