[PATCH libinput] touchpad: warn if we have invalid touchpad ranges
Peter Hutterer
peter.hutterer at who-t.net
Wed Jun 1 22:01:56 UTC 2016
On Wed, Jun 01, 2016 at 08:41:17AM -0500, Yong Bakos wrote:
> On May 31, 2016, at 7:42 PM, Peter Hutterer <peter.hutterer at who-t.net> wrote:
> >
> > Quite a few bugs are caused by touchpad ranges being out of whack. If we get
> > input events significantly outside the expected range (5% width/height as
> > error margin) print a warning to the log.
> >
> > And add a new doc page to explain what is happening and how to fix it.
> >
> > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
>
> Hi Peter,
> FWIW, one minor nit (whildcard) I noticed, inline below.
fixed, thanks.
Cheers,
Peter
> > ---
> > doc/Makefile.am | 1 +
> > doc/absolute-coordinate-ranges.dox | 120 +++++++++++++++++++++++++++++++++++++
> > doc/page-hierarchy.dox | 1 +
> > src/evdev-mt-touchpad.c | 59 ++++++++++++++++++
> > src/evdev-mt-touchpad.h | 5 ++
> > 5 files changed, 186 insertions(+)
> > create mode 100644 doc/absolute-coordinate-ranges.dox
> >
> > diff --git a/doc/Makefile.am b/doc/Makefile.am
> > index f2a47cb..1e501a8 100644
> > --- a/doc/Makefile.am
> > +++ b/doc/Makefile.am
> > @@ -11,6 +11,7 @@ header_files = \
> > $(top_srcdir)/src/libinput.h \
> > $(top_srcdir)/README.txt \
> > $(srcdir)/absolute-axes.dox \
> > + $(srcdir)/absolute-coordinate-ranges.dox \
> > $(srcdir)/clickpad-softbuttons.dox \
> > $(srcdir)/device-configuration-via-udev.dox \
> > $(srcdir)/faqs.dox \
> > diff --git a/doc/absolute-coordinate-ranges.dox b/doc/absolute-coordinate-ranges.dox
> > new file mode 100644
> > index 0000000..f9d9e98
> > --- /dev/null
> > +++ b/doc/absolute-coordinate-ranges.dox
> > @@ -0,0 +1,120 @@
> > +/**
> > + at page absolute_coordinate_ranges Coordinate ranges for absolute axes
> > +
> > +libinput requires that all touchpads provide a correct axis range and
> > +resolution. These are used to enable or disable certain features or adapt
> > +the interaction with the touchpad. For example, the software button area is
> > +narrower on small touchpads to avoid reducing the interactive surface too
> > +much. Likewise, palm detection works differently on small touchpads as palm
> > +interference is less likely to happen.
> > +
> > +Touchpads with incorrect axis ranges generate error messages
> > +in the form:
> > +<blockquote>
> > +Axis 0x35 value 4000 is outside expected range [0, 3000]
> > +</blockquote>
> > +
> > +This error message indicates that the ABS_MT_POSITION_X axis (i.e. the x
> > +axis) generated an event outside the expected range of 0-3000. In this case
> > +the value was 4000.
> > +This discrepancy between the coordinate range the kernels advertises vs.
> > +what the touchpad sends can be the source of a number of perceived
> > +bugs in libinput.
> > +
> > + at section absolute_coordinate_ranges_fix Measuring and fixing touchpad ranges
> > +
> > +To fix the touchpad you need to:
> > +-# measure the physical size of your touchpad in mm
> > +-# run touchpad-edge-detector
> > +-# trim the udev match rule to something sensible
> > +-# replace the resolution with the calculated resolution based on physical
> > + settings
> > +-# test locally
> > +-# send a patch to the systemd project
> > +
> > +Detailed explanations are below.
> > +
> > +[libevdev](http://freedesktop.org/wiki/Software/libevdev/) provides a tool
> > +called **touchpad-edge-detector** that allows measuring the touchpad's input
> > +ranges. Run the tool as root against the device node of your touchpad device
> > +and repeatedly move a finger around the whole outside area of the
> > +touchpad. Then control+c the process and note the output.
> > +An example output is below:
> > +
> > + at code
> > +$> sudo touchpad-edge-detector /dev/input/event4
> > +Touchpad SynPS/2 Synaptics TouchPad on /dev/input/event4
> > +Move one finger around the touchpad to detect the actual edges
> > +Kernel says: x [1024..3112], y [2024..4832]
> > +Touchpad sends: x [2445..4252], y [3464..4071]
> > +
> > +Touchpad size as listed by the kernel: 49x66mm
> > +Calculate resolution as:
> > + x axis: 2088/<width in mm>
> > + y axis: 2808/<height in mm>
> > +
> > +Suggested udev rule:
> > +# <Laptop model description goes here>
> > +evdev:name:SynPS/2 Synaptics TouchPad:dmi:bvnLENOVO:bvrGJET72WW(2.22):bd02/21/2014:svnLENOVO:pn20ARS25701:pvrThinkPadT440s:rvnLENOVO:rn20ARS25701:rvrSDK0E50512STD:cvnLENOVO:ct10:cvrNotAvailable:*
> > + EVDEV_ABS_00=2445:4252:<x resolution>
> > + EVDEV_ABS_01=3464:4071:<y resolution>
> > + EVDEV_ABS_35=2445:4252:<x resolution>
> > + EVDEV_ABS_36=3464:4071:<y resolution>
> > +
> > + at endcode
> > +
> > +Note the discrepancy between the coordinate range the kernels advertises vs.
> > +what the touchpad sends.
> > +To fix the advertised ranges, the udev rule should be taken and trimmed
> > +before being sent to the [systemd project](https://github.com/systemd/systemd).
> > +An example commit can be found
> > +[here](https://github.com/systemd/systemd/commit/26f667eac1c5e89b689aa0a1daef6a80f473e045).
> > +
> > +In most cases the match can and should be trimmed to the system vendor (svn)
> > +and the product version (pvr), with everything else replaced by a wildcard
> > +(*). In this case, a Lenovo T440s, a suitable match string would be: @code
> > +evdev:name:SynPS/2 Synaptics TouchPad:dmi:*svnLENOVO:*pvrThinkPadT440s*
> > + at endcode
> > +
> > + at note hwdb match strings only allow for alphanumeric ascii characters. Use a
> > +whildcard (* or ?, whichever appropriate) for special characters.
>
> wildcard
>
>
>
> > +
> > +The actual axis overrides are in the form:
> > + at code
> > +# axis number=min:max:resolution
> > + EVDEV_ABS_00=2445:4252:42
> > + at endcode
> > +or, if the range is correct but the resolution is wrong
> > + at code
> > +# axis number=::resolution
> > + EVDEV_ABS_00=::42
> > + at endcode
> > +
> > +Note the leading single space. The axis numbers are in hex and can be found
> > +in *linux/input-event-codes.h*. For touchpads ABS_X, ABS_Y,
> > +ABS_MT_POSITION_X and ABS_MT_POSITION_Y are required.
> > +
> > + at note The touchpad's ranges and/or resolution should only be fixed when
> > +there is a significant discrepancy. A few units do not make a difference and
> > +a resolution that is off by 2 or less usually does not matter either.
> > +
> > +Once a match and override rule has been found, follow the instructions at
> > +the top of the
> > +[60-evdev.hwdb](https://github.com/systemd/systemd/blob/master/hwdb/60-evdev.hwdb)
> > +file to save it locally and trigger the udev hwdb reload. Rebooting is
> > +always a good idea. If the match string is correct, the new properties will
> > +show up in the
> > +output of
> > + at code
> > + udevadm info /sys/class/input/event4
> > + at endcode
> > +
> > +Adjust the command for the event node of your touchpad.
> > +A udev builtin will apply the new axis ranges automatically.
> > +
> > +When the axis override is confirmed to work, please submit it as a patch to
> > +the [systemd project](https://github.com/systemd/systemd) or if that is not
> > +possible as a libinput bug here:
> > +https://bugs.freedesktop.org/enter_bug.cgi?product=Wayland&component=libinput
> > +*/
> > +
> > diff --git a/doc/page-hierarchy.dox b/doc/page-hierarchy.dox
> > index e47e98e..8455fca 100644
> > --- a/doc/page-hierarchy.dox
> > +++ b/doc/page-hierarchy.dox
> > @@ -8,6 +8,7 @@
> > - @subpage palm_detection
> > - @subpage t440_support
> > - @subpage touchpad_jumping_cursor
> > +- @subpage absolute_coordinate_ranges
> >
> > @page touchscreens Touchscreens
> >
> > diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
> > index 61d955a..a7b7a68 100644
> > --- a/src/evdev-mt-touchpad.c
> > +++ b/src/evdev-mt-touchpad.c
> > @@ -289,6 +289,39 @@ tp_get_delta(struct tp_touch *t)
> > return tp_normalize_delta(t->tp, delta);
> > }
> >
> > +static inline void
> > +tp_check_axis_range(struct tp_dispatch *tp,
> > + unsigned int code,
> > + int value)
> > +{
> > + int min, max;
> > +
> > + switch(code) {
> > + case ABS_X:
> > + case ABS_MT_POSITION_X:
> > + min = tp->warning_range.min.x;
> > + max = tp->warning_range.max.x;
> > + break;
> > + case ABS_Y:
> > + case ABS_MT_POSITION_Y:
> > + min = tp->warning_range.min.y;
> > + max = tp->warning_range.max.y;
> > + break;
> > + default:
> > + return;
> > + }
> > +
> > + if (value < min || value > max) {
> > + log_info_ratelimit(tp_libinput_context(tp),
> > + &tp->warning_range.range_warn_limit,
> > + "Axis %#x value %d is outside expected range [%d, %d]\n"
> > + "See %s/absolute_coordinate_ranges.html for details\n",
> > + code, value, min, max,
> > + HTTP_DOC_LINK);
> > +
> > + }
> > +}
> > +
> > static void
> > tp_process_absolute(struct tp_dispatch *tp,
> > const struct input_event *e,
> > @@ -298,12 +331,14 @@ tp_process_absolute(struct tp_dispatch *tp,
> >
> > switch(e->code) {
> > case ABS_MT_POSITION_X:
> > + tp_check_axis_range(tp, e->code, e->value);
> > t->point.x = e->value;
> > t->millis = time;
> > t->dirty = true;
> > tp->queued |= TOUCHPAD_EVENT_MOTION;
> > break;
> > case ABS_MT_POSITION_Y:
> > + tp_check_axis_range(tp, e->code, e->value);
> > t->point.y = e->value;
> > t->millis = time;
> > t->dirty = true;
> > @@ -339,12 +374,14 @@ tp_process_absolute_st(struct tp_dispatch *tp,
> >
> > switch(e->code) {
> > case ABS_X:
> > + tp_check_axis_range(tp, e->code, e->value);
> > t->point.x = e->value;
> > t->millis = time;
> > t->dirty = true;
> > tp->queued |= TOUCHPAD_EVENT_MOTION;
> > break;
> > case ABS_Y:
> > + tp_check_axis_range(tp, e->code, e->value);
> > t->point.y = e->value;
> > t->millis = time;
> > t->dirty = true;
> > @@ -2063,6 +2100,26 @@ want_hysteresis:
> > return;
> > }
> >
> > +static void
> > +tp_init_range_warnings(struct tp_dispatch *tp,
> > + struct evdev_device *device,
> > + int width,
> > + int height)
> > +{
> > + const struct input_absinfo *x, *y;
> > +
> > + x = device->abs.absinfo_x;
> > + y = device->abs.absinfo_y;
> > +
> > + tp->warning_range.min.x = x->minimum - 0.05 * width;
> > + tp->warning_range.min.y = y->minimum - 0.05 * height;
> > + tp->warning_range.max.x = x->maximum + 0.05 * width;
> > + tp->warning_range.max.y = y->maximum + 0.05 * height;
> > +
> > + /* One warning every 5 min is enough */
> > + ratelimit_init(&tp->warning_range.range_warn_limit, s2us(3000), 1);
> > +}
> > +
> > static int
> > tp_init(struct tp_dispatch *tp,
> > struct evdev_device *device)
> > @@ -2086,6 +2143,8 @@ tp_init(struct tp_dispatch *tp,
> > height = device->abs.dimensions.y;
> > diagonal = sqrt(width*width + height*height);
> >
> > + tp_init_range_warnings(tp, device, width, height);
> > +
> > tp->reports_distance = libevdev_has_event_code(device->evdev,
> > EV_ABS,
> > ABS_MT_DISTANCE);
> > diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
> > index cbc33aa..1efe25c 100644
> > --- a/src/evdev-mt-touchpad.h
> > +++ b/src/evdev-mt-touchpad.h
> > @@ -245,6 +245,11 @@ struct tp_dispatch {
> > struct device_coords hysteresis_margin;
> >
> > struct {
> > + struct device_coords min, max;
> > + struct ratelimit range_warn_limit;
> > + } warning_range;
> > +
> > + struct {
> > double x_scale_coeff;
> > double y_scale_coeff;
> > } accel;
> > --
> > 2.7.4
>
> yong
>
>
>
More information about the wayland-devel
mailing list