| Message ID | 20150820231339.GA7236@localhost (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Aug 20, 2015 at 04:13:39PM -0700, Dmitry Torokhov wrote: > On Thu, Aug 20, 2015 at 04:08:40PM -0700, Dmitry Torokhov wrote: > > Hi Gabor, > > > > On Fri, Aug 21, 2015 at 01:05:46AM +0200, Gabor Balla wrote: > > > Hi Dmitry, > > > > > > On Fri, Aug 21, 2015 at 1:01 AM, Dmitry Torokhov > > > <dmitry.torokhov@gmail.com> wrote: > > > > On Thu, Aug 20, 2015 at 3:56 PM, Dmitry Torokhov > > > > <dmitry.torokhov@gmail.com> wrote: > > > >> On Fri, Aug 21, 2015 at 12:24:59AM +0200, Gabor Balla wrote: > > > >>> Hi Dmitry, > > > >>> > > > >>> On Thu, Aug 20, 2015 at 11:35 PM, Dmitry Torokhov > > > >>> <dmitry.torokhov@gmail.com> wrote: > > > >>> > On Thu, Aug 20, 2015 at 10:50:27PM +0200, Gabor Balla wrote: > > > [...] > > > > Ah, wait, not quite still. So we actually do want to disable gestures > > > > when in Absolute mode (non extended). Although frankly I do not think > > > > we'll ever see pre 4.0 Synaptics device in a wild. > > > > > > Also notice there is a function called synaptics_set_disable_gesture that can > > > change that bit regardless of current mode. > > > > That attribute is only created when touchpad is used in relative mode, > > so should be OK. > > > > Thanks. > > How about this one? Nick, Benjamin, could you please give this a spin? > > Thanks. > > -- > Dmitry > > > Input: synaptics - fix handling of disabling gesture mode > > From: Dmitry Torokhov <dmitry.torokhov@gmail.com> > > Bit 2 of the mode byte has dual meaning: it can disable reporting of > gestures when touchpad works in Relative mode or normal Absolute mode, > or it can enable so called Extended W-Mode when touchpad uses enhanced > Absolute mode (W-mode). The extended W-Mode confuses our driver and > causes missing button presses on some Thinkpads (x250, T450s), so let's > make sure we do not enable it. > > Also, according to the spec W mode "... bit is defined only in Absolute > mode on pads whose capExtended capability bit is set. In Relative mode and > in TouchPads without this capability, the bit is reserved and should be > left at 0.", so let's make sure we respect this requirement as well. > > Reported-by: Nick Bowler <nbowler@draconx.ca> > Suggested-by: Gabor Balla <gaborwho@gmail.com> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > --- > drivers/input/mouse/synaptics.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c > index 28daca1..d9c9e41 100644 > --- a/drivers/input/mouse/synaptics.c > +++ b/drivers/input/mouse/synaptics.c > @@ -519,14 +519,18 @@ static int synaptics_set_mode(struct psmouse *psmouse) > struct synaptics_data *priv = psmouse->private; > > priv->mode = 0; > - if (priv->absolute_mode) > + > + if (priv->absolute_mode) { > priv->mode |= SYN_BIT_ABSOLUTE_MODE; > - if (priv->disable_gesture) > + if (SYN_CAP_EXTENDED(priv->capabilities)) > + priv->mode |= SYN_BIT_W_MODE; > + } > + > + if (!(priv->mode & SYN_BIT_W_MODE) && priv->disable_gesture) > priv->mode |= SYN_BIT_DISABLE_GESTURE; > + > if (psmouse->rate >= 80) > priv->mode |= SYN_BIT_HIGH_RATE; > - if (SYN_CAP_EXTENDED(priv->capabilities)) > - priv->mode |= SYN_BIT_W_MODE; > > if (synaptics_mode_cmd(psmouse, priv->mode)) > return -1;
On 2015-08-24, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > On Thu, Aug 20, 2015 at 04:13:39PM -0700, Dmitry Torokhov wrote: [...] > > On Fri, Aug 21, 2015 at 1:01 AM, Dmitry Torokhov > > <dmitry.torokhov@gmail.com> wrote: [...] > > > Ah, wait, not quite still. So we actually do want to disable > > > gestures when in Absolute mode (non extended). Although frankly I > > > do not think we'll ever see pre 4.0 Synaptics device in a wild. [...] > > How about this one? > > Nick, Benjamin, could you please give this a spin? [...] > > Input: synaptics - fix handling of disabling gesture mode Applied on top of 4.1.6, appears to fix the trackpoint buttons on my X250. Thanks, Nick -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[re-sending because my gmail client decided to turn on HTML for all mails I wrote. Sorry for the duplicate ] On Sun, Sep 27, 2015 at 8:12 AM, Benjamin Tissoires <benjamin.tissoires@gmail.com> wrote: > > > On Thu, Aug 20, 2015 at 7:13 PM, Dmitry Torokhov <dmitry.torokhov@gmail.com> > wrote: >> >> On Thu, Aug 20, 2015 at 04:08:40PM -0700, Dmitry Torokhov wrote: >> > Hi Gabor, >> > >> > On Fri, Aug 21, 2015 at 01:05:46AM +0200, Gabor Balla wrote: >> > > Hi Dmitry, >> > > >> > > On Fri, Aug 21, 2015 at 1:01 AM, Dmitry Torokhov >> > > <dmitry.torokhov@gmail.com> wrote: >> > > > On Thu, Aug 20, 2015 at 3:56 PM, Dmitry Torokhov >> > > > <dmitry.torokhov@gmail.com> wrote: >> > > >> On Fri, Aug 21, 2015 at 12:24:59AM +0200, Gabor Balla wrote: >> > > >>> Hi Dmitry, >> > > >>> >> > > >>> On Thu, Aug 20, 2015 at 11:35 PM, Dmitry Torokhov >> > > >>> <dmitry.torokhov@gmail.com> wrote: >> > > >>> > On Thu, Aug 20, 2015 at 10:50:27PM +0200, Gabor Balla wrote: >> > > [...] >> > > > Ah, wait, not quite still. So we actually do want to disable >> > > > gestures >> > > > when in Absolute mode (non extended). Although frankly I do not >> > > > think >> > > > we'll ever see pre 4.0 Synaptics device in a wild. >> > > >> > > Also notice there is a function called synaptics_set_disable_gesture >> > > that can >> > > change that bit regardless of current mode. >> > >> > That attribute is only created when touchpad is used in relative mode, >> > so should be OK. >> > >> > Thanks. >> >> How about this one? > Hello Dmitry, unfortunately, I have been reported a breakage in libinput this week following this patch. See https://bugs.freedesktop.org/show_bug.cgi?id=92091 If you look at the evemu recording with 2 fingers scroll, you will see that the kernel sends a slot and a tracking ID for the second finger, but it never sends the actual coordinates. Libinput ignores such slot (I'll check with Peter what we do in such cases when he will be back from his vacations) and this kills the 2 fingers scrolling. I believe the fix is worse than the original situation given that this also removes all gestures support we can have in libinput. And of course, with my turtle speed to test it, this patch is already in stable too (I am entirely at fault here :( ). Cheers, Benjamin >> >> >> Thanks. >> >> -- >> Dmitry >> >> >> Input: synaptics - fix handling of disabling gesture mode >> >> From: Dmitry Torokhov <dmitry.torokhov@gmail.com> >> >> Bit 2 of the mode byte has dual meaning: it can disable reporting of >> gestures when touchpad works in Relative mode or normal Absolute mode, >> or it can enable so called Extended W-Mode when touchpad uses enhanced >> Absolute mode (W-mode). The extended W-Mode confuses our driver and >> causes missing button presses on some Thinkpads (x250, T450s), so let's >> make sure we do not enable it. >> >> Also, according to the spec W mode "... bit is defined only in Absolute >> mode on pads whose capExtended capability bit is set. In Relative mode and >> in TouchPads without this capability, the bit is reserved and should be >> left at 0.", so let's make sure we respect this requirement as well. >> >> Reported-by: Nick Bowler <nbowler@draconx.ca> >> Suggested-by: Gabor Balla <gaborwho@gmail.com> >> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> >> --- >> drivers/input/mouse/synaptics.c | 12 ++++++++---- >> 1 file changed, 8 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/input/mouse/synaptics.c >> b/drivers/input/mouse/synaptics.c >> index 28daca1..d9c9e41 100644 >> --- a/drivers/input/mouse/synaptics.c >> +++ b/drivers/input/mouse/synaptics.c >> @@ -519,14 +519,18 @@ static int synaptics_set_mode(struct psmouse >> *psmouse) >> struct synaptics_data *priv = psmouse->private; >> >> priv->mode = 0; >> - if (priv->absolute_mode) >> + >> + if (priv->absolute_mode) { >> priv->mode |= SYN_BIT_ABSOLUTE_MODE; >> - if (priv->disable_gesture) >> + if (SYN_CAP_EXTENDED(priv->capabilities)) >> + priv->mode |= SYN_BIT_W_MODE; >> + } >> + >> + if (!(priv->mode & SYN_BIT_W_MODE) && priv->disable_gesture) >> priv->mode |= SYN_BIT_DISABLE_GESTURE; >> + >> if (psmouse->rate >= 80) >> priv->mode |= SYN_BIT_HIGH_RATE; >> - if (SYN_CAP_EXTENDED(priv->capabilities)) >> - priv->mode |= SYN_BIT_W_MODE; >> >> if (synaptics_mode_cmd(psmouse, priv->mode)) >> return -1; > > -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Sep 27, 2015 at 5:14 AM, Benjamin Tissoires <benjamin.tissoires@gmail.com> wrote: > [re-sending because my gmail client decided to turn on HTML for all > mails I wrote. Sorry for the duplicate ] > > On Sun, Sep 27, 2015 at 8:12 AM, Benjamin Tissoires > <benjamin.tissoires@gmail.com> wrote: >> >> >> On Thu, Aug 20, 2015 at 7:13 PM, Dmitry Torokhov <dmitry.torokhov@gmail.com> >> wrote: >>> >>> On Thu, Aug 20, 2015 at 04:08:40PM -0700, Dmitry Torokhov wrote: >>> > Hi Gabor, >>> > >>> > On Fri, Aug 21, 2015 at 01:05:46AM +0200, Gabor Balla wrote: >>> > > Hi Dmitry, >>> > > >>> > > On Fri, Aug 21, 2015 at 1:01 AM, Dmitry Torokhov >>> > > <dmitry.torokhov@gmail.com> wrote: >>> > > > On Thu, Aug 20, 2015 at 3:56 PM, Dmitry Torokhov >>> > > > <dmitry.torokhov@gmail.com> wrote: >>> > > >> On Fri, Aug 21, 2015 at 12:24:59AM +0200, Gabor Balla wrote: >>> > > >>> Hi Dmitry, >>> > > >>> >>> > > >>> On Thu, Aug 20, 2015 at 11:35 PM, Dmitry Torokhov >>> > > >>> <dmitry.torokhov@gmail.com> wrote: >>> > > >>> > On Thu, Aug 20, 2015 at 10:50:27PM +0200, Gabor Balla wrote: >>> > > [...] >>> > > > Ah, wait, not quite still. So we actually do want to disable >>> > > > gestures >>> > > > when in Absolute mode (non extended). Although frankly I do not >>> > > > think >>> > > > we'll ever see pre 4.0 Synaptics device in a wild. >>> > > >>> > > Also notice there is a function called synaptics_set_disable_gesture >>> > > that can >>> > > change that bit regardless of current mode. >>> > >>> > That attribute is only created when touchpad is used in relative mode, >>> > so should be OK. >>> > >>> > Thanks. >>> >>> How about this one? >> > > Hello Dmitry, > > unfortunately, I have been reported a breakage in libinput this week > following this patch. > See https://bugs.freedesktop.org/show_bug.cgi?id=92091 > > If you look at the evemu recording with 2 fingers scroll, you will see that > the kernel sends a slot and a tracking ID for the second finger, but it > never sends the actual coordinates. Libinput ignores such slot (I'll check > with Peter what we do in such cases when he will be back from his vacations) > and this kills the 2 fingers scrolling. > > I believe the fix is worse than the original situation given that this also > removes all gestures support we can have in libinput. > Umm, I guess I have to claim temporary insanity or something like that with this one: we do need extended W mode to handle multiple touches in the driver and the patch disabled it ;( Before we revert this, Nick, Gabor, can you tell me what capability bits your touchpads report (in dmesg)? Thanks.
On 9/28/15, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > On Sun, Sep 27, 2015 at 5:14 AM, Benjamin Tissoires <benjamin.tissoires@gmail.com> wrote: >> I believe the fix is worse than the original situation given that this >> also removes all gestures support we can have in libinput. > > Umm, I guess I have to claim temporary insanity or something like that > with this one: we do need extended W mode to handle multiple touches > in the driver and the patch disabled it ;( > > Before we revert this, Nick, Gabor, can you tell me what capability > bits your touchpads report (in dmesg)? Is this the info you're looking for? [ 1.225243] psmouse serio1: synaptics: queried max coordinates: x [..5712], y [..4780] [ 1.256725] psmouse serio1: synaptics: queried min coordinates: x [1232..], y [1074..] [ 1.316644] psmouse serio1: synaptics: Touchpad model: 1, fw: 8.1, id: 0x1e2b1, caps: 0xf002a3/0x943300/0x12e800/0x10000, board id: 3075, fw id: 2560 [ 1.318089] psmouse serio1: synaptics: serio: Synaptics pass-through port at isa0060/serio1/input0 [ 1.356859] input: SynPS/2 Synaptics TouchPad as /devices/platform/i8042/serio1/input/input5 [ 5.135450] psmouse serio2: trackpoint: IBM TrackPoint firmware: 0x0e, buttons: 3/3 [ 5.329582] input: TPPS/2 IBM TrackPoint as /devices/platform/i8042/serio1/serio2/input/input7 Cheers, Nick -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Sep 28, 2015 at 08:22:02PM -0400, Nick Bowler wrote: > On 9/28/15, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > > On Sun, Sep 27, 2015 at 5:14 AM, Benjamin Tissoires <benjamin.tissoires@gmail.com> wrote: > >> I believe the fix is worse than the original situation given that this > >> also removes all gestures support we can have in libinput. > > > > Umm, I guess I have to claim temporary insanity or something like that > > with this one: we do need extended W mode to handle multiple touches > > in the driver and the patch disabled it ;( > > > > Before we revert this, Nick, Gabor, can you tell me what capability > > bits your touchpads report (in dmesg)? > > Is this the info you're looking for? Yep, thanks. > > [ 1.225243] psmouse serio1: synaptics: queried max coordinates: x > [..5712], y [..4780] > [ 1.256725] psmouse serio1: synaptics: queried min coordinates: x > [1232..], y [1074..] > [ 1.316644] psmouse serio1: synaptics: Touchpad model: 1, fw: > 8.1, id: 0x1e2b1, caps: 0xf002a3/0x943300/0x12e800/0x10000, board id: > 3075, fw id: 2560 > [ 1.318089] psmouse serio1: synaptics: serio: Synaptics > pass-through port at isa0060/serio1/input0 > [ 1.356859] input: SynPS/2 Synaptics TouchPad as > /devices/platform/i8042/serio1/input/input5 > [ 5.135450] psmouse serio2: trackpoint: IBM TrackPoint firmware: > 0x0e, buttons: 3/3 > [ 5.329582] input: TPPS/2 IBM TrackPoint as > /devices/platform/i8042/serio1/serio2/input/input7 > > Cheers, > Nick
diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c index 28daca1..d9c9e41 100644 --- a/drivers/input/mouse/synaptics.c +++ b/drivers/input/mouse/synaptics.c @@ -519,14 +519,18 @@ static int synaptics_set_mode(struct psmouse *psmouse) struct synaptics_data *priv = psmouse->private; priv->mode = 0; - if (priv->absolute_mode) + + if (priv->absolute_mode) { priv->mode |= SYN_BIT_ABSOLUTE_MODE; - if (priv->disable_gesture) + if (SYN_CAP_EXTENDED(priv->capabilities)) + priv->mode |= SYN_BIT_W_MODE; + } + + if (!(priv->mode & SYN_BIT_W_MODE) && priv->disable_gesture) priv->mode |= SYN_BIT_DISABLE_GESTURE; + if (psmouse->rate >= 80) priv->mode |= SYN_BIT_HIGH_RATE; - if (SYN_CAP_EXTENDED(priv->capabilities)) - priv->mode |= SYN_BIT_W_MODE; if (synaptics_mode_cmd(psmouse, priv->mode)) return -1;