Hi,

On 01-06-16 02:42, Peter Hutterer 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 <[email protected]>

Looks good to me:

Reviewed-by: Hans de Goede <[email protected]>

Regards,

Hans


---
 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 @@
+/**
+@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.
+
+@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:
+
+@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>
+
+@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*
+@endcode
+
+@note hwdb match strings only allow for alphanumeric ascii characters. Use a
+whildcard (* or ?, whichever appropriate) for special characters.
+
+The actual axis overrides are in the form:
+@code
+# axis number=min:max:resolution
+ EVDEV_ABS_00=2445:4252:42
+@endcode
+or, if the range is correct but the resolution is wrong
+@code
+# axis number=::resolution
+ EVDEV_ABS_00=::42
+@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.
+
+@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
+@code
+   udevadm info /sys/class/input/event4
+@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;

_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to