[PATCH libinput] touchpad: add quirk for the T450 and T460 generation hardware

Peter Hutterer peter.hutterer at who-t.net
Wed Mar 16 22:44:02 UTC 2016


please don't top-post.

On Wed, Mar 16, 2016 at 11:22:37AM -0700, Bill Spitzak wrote:
> Wouldn't it be better to use the motion without acceleration, rather than
> discarding it entirely?

no, you'll get cursor jumps regardless, resulting in some pixels being
unadressable.

Cheers,
   Peter

> On Wed, Mar 16, 2016 at 2:57 AM, Hans de Goede <hdegoede at redhat.com> wrote:
> 
> > Hi,
> >
> > On 11-03-16 01:24, Peter Hutterer wrote:
> >
> >> The touchpad's sensors are too far apart (or the firmware interferes),
> >> causing
> >> in a jerky movement visible especially on slow motion. We get a bunch of
> >> normal motion events, then only ABS_MT_PRESSURE updates without x/y
> >> updates.
> >> After about one mm of movement x/y updates resume, with the first event
> >> covering the distance between the last motion event. That event is usually
> >> accelerated and thus causes a large jump. Subsequent events are
> >> sufficiently
> >> fine-grained again.
> >>
> >> This patch counts the number of non-motion events. Once we hit 10 in a
> >> row, we
> >> mark the first motion update as non-dirty, effectively discarding the
> >> motion
> >> and thus stopping the pointer jumps.
> >>
> >> https://bugs.freedesktop.org/show_bug.cgi?id=94379
> >>
> >> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> >> Tested-by: Benjamin Tissoires <benjamin.tissoires at gmail.com>
> >> ---
> >>   src/evdev-mt-touchpad.c            | 32 ++++++++++++++++++++++++++++----
> >>   src/evdev-mt-touchpad.h            | 12 ++++++++++++
> >>   src/evdev.c                        |  1 +
> >>   src/evdev.h                        |  1 +
> >>   udev/90-libinput-model-quirks.hwdb |  7 +++++++
> >>   5 files changed, 49 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
> >> index 00d6539..d0a8e27 100644
> >> --- a/src/evdev-mt-touchpad.c
> >> +++ b/src/evdev-mt-touchpad.c
> >> @@ -337,7 +337,7 @@ tp_process_absolute(struct tp_dispatch *tp,
> >>         case ABS_MT_PRESSURE:
> >>                 t->pressure = e->value;
> >>                 t->dirty = true;
> >> -               tp->queued |= TOUCHPAD_EVENT_MOTION;
> >> +               tp->queued |= TOUCHPAD_EVENT_OTHERAXIS;
> >>                 break;
> >>         }
> >>   }
> >> @@ -880,8 +880,10 @@ tp_position_fake_touches(struct tp_dispatch *tp)
> >>   }
> >>
> >>   static inline bool
> >> -tp_need_motion_history_reset(struct tp_dispatch *tp)
> >> +tp_need_motion_history_reset(struct tp_dispatch *tp, uint64_t time)
> >>   {
> >> +       bool rc = false;
> >> +
> >>         /* semi-mt finger postions may "jump" when nfingers changes */
> >>         if (tp->semi_mt && tp->nfingers_down != tp->old_nfingers_down)
> >>                 return true;
> >> @@ -894,7 +896,29 @@ tp_need_motion_history_reset(struct tp_dispatch *tp)
> >>                  tp->old_nfingers_down > tp->num_slots))
> >>                 return true;
> >>
> >> -       return false;
> >> +       /* Quirk: if we had multiple events without x/y axis
> >> +          information, the next x/y event is going to be a jump. So we
> >> +          reset that touch to non-dirty effectively swallowing that event
> >> +          and restarting with the next event again.
> >> +        */
> >> +       if (tp->device->model_flags & EVDEV_MODEL_LENOVO_T450_TOUCHPAD) {
> >> +               if (tp->queued & TOUCHPAD_EVENT_MOTION) {
> >> +                       if (tp->quirks.nonmotion_event_count > 10) {
> >> +                               struct tp_touch *t;
> >> +
> >> +                               tp_for_each_touch(tp, t)
> >> +                               t->dirty = false;
> >> +                               rc = true;
> >> +                       }
> >> +                       tp->quirks.nonmotion_event_count = 0;
> >> +               }
> >> +
> >> +               if ((tp->queued &
> >> (TOUCHPAD_EVENT_OTHERAXIS|TOUCHPAD_EVENT_MOTION)) ==
> >>
> >
> > You could make this line:
> >
> >                 else if (tp->queued & TOUCHPAD_EVENT_OTHERAXIS)
> >
> > Since you reset tp->quirks.nonmotion_event_count above it makes sense
> > IMHO to do the increment in an else-if.
> >
> > Also you're not using the time parameter you're adding to this function.
> >
> > +                   TOUCHPAD_EVENT_OTHERAXIS)
> >> +                       tp->quirks.nonmotion_event_count++;
> >> +       }
> >> +
> >> +       return rc;
> >>   }
> >>
> >>   static void
> >> @@ -909,7 +933,7 @@ tp_process_state(struct tp_dispatch *tp, uint64_t
> >> time)
> >>         tp_unhover_touches(tp, time);
> >>         tp_position_fake_touches(tp);
> >>
> >> -       want_motion_reset = tp_need_motion_history_reset(tp);
> >> +       want_motion_reset = tp_need_motion_history_reset(tp, time);
> >>
> >>         for (i = 0; i < tp->ntouches; i++) {
> >>                 t = tp_get_touch(tp, i);
> >> diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
> >> index eae327b..1f05a03 100644
> >> --- a/src/evdev-mt-touchpad.h
> >> +++ b/src/evdev-mt-touchpad.h
> >> @@ -41,6 +41,7 @@ enum touchpad_event {
> >>         TOUCHPAD_EVENT_MOTION           = (1 << 0),
> >>         TOUCHPAD_EVENT_BUTTON_PRESS     = (1 << 1),
> >>         TOUCHPAD_EVENT_BUTTON_RELEASE   = (1 << 2),
> >> +       TOUCHPAD_EVENT_OTHERAXIS        = (1 << 3),
> >>   };
> >>
> >>   enum touchpad_model {
> >> @@ -353,6 +354,17 @@ struct tp_dispatch {
> >>                 int upper_thumb_line;
> >>                 int lower_thumb_line;
> >>         } thumb;
> >> +
> >> +       struct {
> >> +               /* A quirk used on the T450 series Synaptics hardware.
> >> +                * Slowly moving the finger causes multiple events with
> >> only
> >> +                * ABS_MT_PRESSURE but no x/y information. When the x/y
> >> +                * event comes, it will be a jump of ~20 units. We use the
> >> +                * below to count non-motion events to discard that first
> >> +                * event with the jump.
> >> +                */
> >> +               unsigned int nonmotion_event_count;
> >> +       } quirks;
> >>   };
> >>
> >>   #define tp_for_each_touch(_tp, _t) \
> >> diff --git a/src/evdev.c b/src/evdev.c
> >> index 51768fe..a5c965d 100644
> >> --- a/src/evdev.c
> >> +++ b/src/evdev.c
> >> @@ -1680,6 +1680,7 @@ evdev_read_model_flags(struct evdev_device *device)
> >>                 { "LIBINPUT_MODEL_CYBORG_RAT", EVDEV_MODEL_CYBORG_RAT },
> >>                 { "LIBINPUT_MODEL_CYAPA", EVDEV_MODEL_CYAPA },
> >>                 { "LIBINPUT_MODEL_ALPS_RUSHMORE",
> >> EVDEV_MODEL_ALPS_RUSHMORE },
> >> +               { "LIBINPUT_MODEL_LENOVO_T450_TOUCHPAD",
> >> EVDEV_MODEL_LENOVO_T450_TOUCHPAD },
> >>                 { NULL, EVDEV_MODEL_DEFAULT },
> >>         };
> >>         const struct model_map *m = model_map;
> >> diff --git a/src/evdev.h b/src/evdev.h
> >> index 482712b..4a5d807 100644
> >> --- a/src/evdev.h
> >> +++ b/src/evdev.h
> >> @@ -113,6 +113,7 @@ enum evdev_device_model {
> >>         EVDEV_MODEL_CYBORG_RAT = (1 << 14),
> >>         EVDEV_MODEL_CYAPA = (1 << 15),
> >>         EVDEV_MODEL_ALPS_RUSHMORE = (1 << 16),
> >> +       EVDEV_MODEL_LENOVO_T450_TOUCHPAD= (1 << 17),
> >>   };
> >>
> >>   struct mt_slot {
> >> diff --git a/udev/90-libinput-model-quirks.hwdb
> >> b/udev/90-libinput-model-quirks.hwdb
> >> index eb2859e..d5978f7 100644
> >> --- a/udev/90-libinput-model-quirks.hwdb
> >> +++ b/udev/90-libinput-model-quirks.hwdb
> >> @@ -100,6 +100,13 @@ libinput:name:Cypress APA Trackpad (cyapa):dmi:*
> >>   libinput:name:SynPS/2 Synaptics
> >> TouchPad:dmi:*svnLENOVO:*:pvrThinkPadX230*
> >>    LIBINPUT_MODEL_LENOVO_X230=1
> >>
> >> +# Lenovo T450/T460 and all other Lenovos of the *50 and *60 generation,
> >> +# including the X1 Carbon 3rd gen
> >> +libinput:name:SynPS/2 Synaptics
> >> TouchPad:dmi:*svnLENOVO:*:pvrThinkPad??50*:
> >> +libinput:name:SynPS/2 Synaptics
> >> TouchPad:dmi:*svnLENOVO:*:pvrThinkPad??60*:
> >> +libinput:name:SynPS/2 Synaptics
> >> TouchPad:dmi:*svnLENOVO:*:pvrThinkPadX1Carbon3rd:*
> >> + LIBINPUT_MODEL_LENOVO_T450_TOUCHPAD=1
> >> +
> >>   ##########################################
> >>   # Synaptics
> >>   ##########################################
> >>
> >
> > Otherwise this patch looks good to me and is:
> >
> > Reviewed-by: Hans de Goede <hdegoede at redhat.com>
> >
> > Regards,
> >
> > Hans
> > _______________________________________________
> > wayland-devel mailing list
> > wayland-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/wayland-devel
> >


More information about the wayland-devel mailing list