applespi: Fix touchpad ranges for MacBookPro13,*.

[roadrunner2]

The MacBookPro's have larger touchpads, so the x/y ranges need to be adjusted. I haven't actually gotten info from a MacBookPro13,2, only from a 13,1 and 13,3 model, but AFAICT the 13,2 has the same touchpad as the 13,1 (both being 13inch models), so this sets the 13,2 values to those of the 13,1.

[Dunedan]

Compilation doesn't work for me:

/foo/bar/macbook12-spi-driver/applespi.c:177:62: error: initializer element is not constant
 static struct applespi_tp_info applespi_macbookpro132_info = applespi_macbookpro131_info;
                                                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~

[roadrunner2]

Aargh, I'm very sorry, I forgot I had made a last-minute change but not tested it yet. I've just re-pushed (and this has now been truly fully tested). Again, apologies for this screwup.

[Dunedan]

No worries, all regarding these MBPs is still pretty experimental. Your patch works now, but independently if I use applespi with or without the patch, Xorg (1.19.1) always prints:

[ 1286.504] (II) Apple SPI Touchpad: no resolution or size hints, assuming a size of 69x50mm

[roadrunner2]

Yes, an entry in the hwdb is also needed - see https://gist.github.com/roadrunner2/1289542a748d9a104e7baec6a92f9cd7 in the Keyboard/Touchpad section, near the end, where it says how to add the 61-evdev-local.hwdb entry. I plan on submitting a patch upstream to systemd. You can also verify things know the correct size by running sudo libinput-list-devices.

[Dunedan]

Perfect, thanks. I skipped the keyboard section without further reading before. My bad. :blush:

Patch works apparently. Xorg doesn't complain anymore, libinput-list-devices lists the correct touchpad size (135.33x82.68mm) and mouse movement is notable faster than before (don't know if that's intentional or just a side effect).

[cb22]

Looks good to me 👍

On the resolution note, I've found

evdev:name:Apple SPI Touchpad:dmi:*:svnAppleInc.:pnMacBook9,1:*
 EVDEV_ABS_00=::91
 EVDEV_ABS_01=::103
 EVDEV_ABS_35=::91
 EVDEV_ABS_36=::103

To be ideal for my MacBook9,1.

[roadrunner2]

Thanks for merging!

Regarding those resolutions, they look odd: all the others I've seen (https://github.com/systemd/systemd/blob/master/hwdb/60-evdev.hwdb) and the values measured for the current MBP's show dpi's around the low 90's and in particular having only a very small different between x and y dpi's. So your y dpi values of 103 look off. Did you get these from touchpad-edge-detector (https://wayland.freedesktop.org/libinput/doc/latest/absolute_coordinate_ranges.html)?

[roadrunner2]

Ping - any comment on those y dpi values?

[cb22]

@roadrunner2 you're correct; the values were indeed wrong. I think I manually worked them out by scaling the values with the dimension of the touchpad. I used touchpad-edge-detector and got some values more inline with what you're seeing.

[roadrunner2]

@cb22 Can you post those values you got here? I'll then add them to my patch and submit it upstream.

[tudorbarascu]

@roadrunner2 Following @cb22 suggestions on how to do it, I got the following values on my 13,1 MacBook Pro:

# MacBookPro13,(1,2) (Late 2016)
evdev:name:Apple SPI Touchpad:dmi:bvnAppleInc.:bvrMBP131.88Z.0205.B15.1702161827:bd02/16/2017:svnAppleInc.:pnMacBookPro13,1:pvr1.0:rvnAppleInc.:rnMac-473D31EABEB93F9B:rvrMacBookPro13,1:cvnAppleInc.:ct9:cvrMac-473D31EABEB93F9B:*
 EVDEV_ABS_00=-6243:6749:96
 EVDEV_ABS_01=-2000:5855:92
 EVDEV_ABS_35=-6243:6749:96
 EVDEV_ABS_36=-2000:5855:92

They probably are different from the 12 inch macbook or from the 15 inch ones. You can do it on Debian/Ubuntu by: sudo apt-get install libinput-tools sudo libinput-list-devices should output:

Device:           Apple SPI Keyboard
Kernel:           /dev/input/event14
Group:            7
Seat:             seat0, default
Capabilities:     keyboard 
Tap-to-click:     n/a
Tap-and-drag:     n/a
Tap drag lock:    n/a
Left-handed:      n/a
Nat.scrolling:    n/a
Middle emulation: n/a
Calibration:      n/a
Scroll methods:   none
Click methods:    none
Disable-w-typing: n/a
Accel profiles:   n/a
Rotation:         n/a

Device:           Apple SPI Touchpad
Kernel:           /dev/input/event15
Group:            7
Seat:             seat0, default
Size:             135.33x85.38mm
Capabilities:     pointer 
Tap-to-click:     disabled
Tap-and-drag:     enabled
Tap drag lock:    disabled
Left-handed:      disabled
Nat.scrolling:    disabled
Middle emulation: disabled
Calibration:      n/a
Scroll methods:   *two-finger edge 
Click methods:    *button-areas clickfinger 
Disable-w-typing: enabled
Accel profiles:   none
Rotation:         n/a

Note the /dev/input/event15 is the touchpad.

Install touchpad-edge-detector with sudo apt-get install libevdev-tools. Now run sudo touchpad-edge-detector 135x85 /dev/input/event15. Note that for a MacBook Pro 13,3 you should use its touchpad size (159x99 instead of 135x85)

This should output:

Touchpad Apple SPI Touchpad on /dev/input/event15
Move one finger around the touchpad to detect the actual edges
Kernel says:    x [-6243..6749], y [-2000..5855]

After moving the finger on the touchpad edges I had to press CTRL + C to stop it with the full output:

Touchpad sends: x [-6243..6749], y [-2000..5855] \^C

Touchpad size as listed by the kernel: 135x85mm
User-specified touchpad size: 135x85mm
Calculated ranges: 12992/7855

Suggested udev rule:
# <Laptop model description goes here>
evdev:name:Apple SPI Touchpad:dmi:bvnAppleInc.:bvrMBP131.88Z.0205.B15.1702161827:bd02/16/2017:svnAppleInc.:pnMacBookPro13,1:pvr1.0:rvnAppleInc.:rnMac-473D31EABEB93F9B:rvrMacBookPro13,1:cvnAppleInc.:ct9:cvrMac-473D31EABEB93F9B:*
 EVDEV_ABS_00=-6243:6749:96
 EVDEV_ABS_01=-2000:5855:92
 EVDEV_ABS_35=-6243:6749:96
 EVDEV_ABS_36=-2000:5855:92

[roadrunner2]

@tudorbarascu Your x dpi look correct, but your y dpi look a bit off - for both 13,3 and 13,2 we got 96 and 95 respectively, and I'm pretty sure the 13,1's trackpad is identical to the one on the 13,2. I see you specified the height as 85mm, whereas @Dunedan listed the height as 82.68mm - that's the cause for the dpi difference. Can you double check your measurements again? Generally from what I've seen of the Apple trackpads, while the x and y dpi's aren't exactly the same, they're usually only 1 or at max 2 different, hence why I'm a bit skeptical at the larger difference you got.

[tudorbarascu]

@roadrunner2 Thanks for the info. 13,1 and 13,2 have identical touchpads for sure. From what I'm measuring, the actual height is between 84 and 85, certainly not 82.68mm. I wonder how @Dunedan got that number. If I put it with 84 mm, I get:

evdev:name:Apple SPI Touchpad:dmi:bvnAppleInc.:bvrMBP131.88Z.0205.B15.1702161827:bd02/16/2017:svnAppleInc.:pnMacBookPro13,1:pvr1.0:rvnAppleInc.:rnMac-473D31EABEB93F9B:rvrMacBookPro13,1:cvnAppleInc.:ct9:cvrMac-473D31EABEB93F9B:*
 EVDEV_ABS_00=-6243:6749:96
 EVDEV_ABS_01=-2000:5855:94
 EVDEV_ABS_35=-6243:6749:96
 EVDEV_ABS_36=-2000:5855:94

[tudorbarascu]

@roadrunner2 I actually had an error in the Xorg log with the 95/96 settings from your patch (I'll post it here in a few hours) so that's why I recalculated.

[Dunedan]

I wonder how @Dunedan got that number.

By running libinput-list-devices and reporting the value of the size of the "Apple SPI Touchpad" back. :wink:

I just verified it's still the same value of 135.33x82.68mm with Debian/unstable and Linux 4.11.0-rc4.

[tudorbarascu]

@Dunedan To my understanding, that value is dependent on initial settings, if you make the content of the /etc/udev/hwdb.d/61-evdev-local.hwdb into what I've written above and then restart.. you will get different values when doing libinput-list-devices. That's why I actually measured the trackpad with a ruler (as recommended here https://wayland.freedesktop.org/libinput/doc/latest/absolute_coordinate_ranges.html ) and while the length is 135, the width is between 84 and 85.

I'm also using debian stretch (with kernel 4.11-rc6) and with your settings I noticed an error in /home/web/.local/share/xorg/Xorg.0.log that was basically saying (quoting from memory) something like "the clicked touchpad region is outside the boundaries". I'm currently trying to find the error.

[roadrunner2]

@Dunedan Unfortunately, that calculates the size based on the max x/y (as reported by the driver, applespi in this, i.e. the values I added in this pull), and the resolution, which it gets from the evdev-local override we provide. So this is a bit circular...

[roadrunner2]

@tudorbarascu I just noticed something: while the total y range is the same as I got for the 13,2 (7855), the min and max values are different. So it looks like that is probably your issue. Now, it's definitely odd that you would be seeing different min/max, and only for the y axis at that, but we can easily accommodate that. Try this patch:

--- a/applespi.c
+++ b/applespi.c
@@ -173,7 +173,8 @@ static const struct applespi_key_translation applespi_fn_codes[] = {
        { 79,  KEY_END },
 };

-static struct applespi_tp_info applespi_macbookpro131_info = { -6243, 6749, -1085, 6770 };
+static struct applespi_tp_info applespi_macbookpro131_info = { -6243, 6749, -2000, 5855 };
+static struct applespi_tp_info applespi_macbookpro132_info = { -6243, 6749, -1085, 6770 };
 static struct applespi_tp_info applespi_macbookpro133_info = { -7456, 7976, -163, 9283 };
 // MacBook11, MacBook12
 static struct applespi_tp_info applespi_default_info = { -4828, 5345, -203, 6803 };
@@ -193,7 +194,7 @@ static const struct dmi_system_id applespi_touchpad_infos[] = {
                        DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."),
                        DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro13,2")
                },
-               .driver_data = &applespi_macbookpro131_info,    // same touchpad
+               .driver_data = &applespi_macbookpro132_info,
        },
        {
                .ident = "Apple MacBookPro13,3",

[tudorbarascu]

Just redid all the steps from scratch (DSDT patch in the kernel + building the driver) and the first output of libinput-list-devices shows a size of 69.11x50.03mm which is clearly wrong. @roadrunner2 I will try later on your patch, thanks!

[tudorbarascu]

@roadrunner2 Tried your patch, same results.

Axis 0x36 value -2458 is outside expected range [-2392, 6247]
Axis 0x36 value -3771 is outside expected range [-2392, 6247]

I manage to setup the settings that don't spawn any error, the first with 135x85mm size:

evdev:name:Apple SPI Touchpad:dmi:bvnAppleInc.:bvrMBP131.88Z.0205.B15.1702161827:bd02/16/2017:svnAppleInc.:pnMacBookPro13,1:pvr1.0:rvnAppleInc.:rnMac-473D31EABEB93F9B:rvrMacBookPro13,1:cvnAppleInc.:ct9:cvrMac-473D31EABEB93F9B:*
 EVDEV_ABS_00=-6243:6749:96
 EVDEV_ABS_01=-3830:4025:92
 EVDEV_ABS_35=-6243:6749:96
 EVDEV_ABS_36=-3830:4025:92

The other with 135x84mm size:

evdev:name:Apple SPI Touchpad:dmi:bvnAppleInc.:bvrMBP131.88Z.0205.B15.1702161827:bd02/16/2017:svnAppleInc.:pnMacBookPro13,1:pvr1.0:rvnAppleInc.:rnMac-473D31EABEB93F9B:rvrMacBookPro13,1:cvnAppleInc.:ct9:cvrMac-473D31EABEB93F9B:*
 EVDEV_ABS_00=-6243:6749:96
 EVDEV_ABS_01=-3830:4025:94
 EVDEV_ABS_35=-6243:6749:96
 EVDEV_ABS_36=-3830:4025:94

@Dunedan @roadrunner2 With your settings aren't you getting any errors like the ones above?

[tudorbarascu]

The reason I'm asking is because I want to add the override to https://github.com/systemd/systemd/blob/master/hwdb/60-evdev.hwdb (as recommended in https://wayland.freedesktop.org/libinput/doc/1.6.3//absolute_coordinate_ranges.html ) and I want to be sure that it's ok. As I don't have the 15 inch model, maybe someone can follow the steps and paste the override parameters.

[roadrunner2]

@tudorbarascu Something is seriously weird on your system: the latest values you posted are showing yet a different range for the y axis, namely [-3830, 4025] - what on earth is going on?

As to getting errors, no, things work perfectly for me (15-inch model).

And yes, I've been wanting to upstream these parameters, but was waiting for @cb22 to get back to me on the correct ones for the 9,1 model.

[roadrunner2]

@tudorbarascu Oh, and make sure you remove those bounds from the EVDEV_ABS_... entries, i.e. specify only the dpi (e.g. ...=::94) - specifying different ranges than the driver claims will definitely screw things up (mainly for the y axis).

[tudorbarascu]

@roadrunner2 :) I obtained those numbers by reusing libinput-list-devices and touchpad-edge-detector until the values shown by the kernel and those outputted by the touchpad-edge-detector have stabilized (equal).

If I only do it once, I get different values when reusing touchpad-edge-detector. So I just rerun it until everything was a match.

I've installed now for the first time on the actual macbook ssd and I'll document the process towards those numbers.

[roadrunner2]

@tudorbarascu Sorry, but I don't trust numbers obtained that way in the least. Here's a patch that will cause the driver to print out the boundaries (similar to touchpad-edge-detector: move your finger to all edges, and keep doing so until the reported values are stable - you can use dmesg | tail or dmesg -w (if your distro has a version of dmesg with -w) to see the reported values - values are only reported once every 3 seconds): boundaries.patch.txt

@Dunedan If you could try this patch too and verify the reported boundries (just to make sure), as well as measure the physical dimensions of your touchpad with a ruler or something, that would be great.

[tudorbarascu]

@roadrunner2 The reported values from the patched driver are: applespi: Touchpad boundaries: x=[-6243, 6749], y=[-170, 7685]. I tried different settings and they're always the same. However, I cannot ditch the error message in /home/web/..user../.local/share/xorg/Xorg.0.log with the default settings or with the ones from your patch as I still get: Axis 0x36 value -1508 is outside expected range [-1477, 7162]. They might work on 13,3 but I guess not so well 13,1 and 13,2. Although the log is not clean from usability perspective all is well.

[Dunedan]

I get the same values as @tudorbarascu: applespi: Touchpad boundaries: x=[-6243, 6749], y=[-170, 7685]

[tudorbarascu]

@Dunedan Are you getting the outside expected range errors in the /home/web/..user../.local/share/xorg/Xorg.0.log?

[Dunedan]

Nope, not at all.

[roadrunner2]

Thanks to both of you. So now we have the correct range (fixed now in the series of patches I'll push in a few days).

@tudorbarascu We need to start with a known correct setup to debug this. Can you A) patch your applespi driver to use the correct y range, i.e.

static struct applespi_tp_info applespi_macbookpro131_info = { -6243, 6749, -170, 7685 };

and B) make sure your hwdb override contains only dpi entries, i.e.

 EVDEV_ABS_00=::96
 EVDEV_ABS_01=::94
 EVDEV_ABS_35=::96
 EVDEV_ABS_36=::94

(and make sure you at the least restart the X server, though a reboot might be safer - not sure when the hwdb overrides are read). Then report the error again, please. This should ensure that range of values reported by the driver is kosher and nothing else is modifying them.

[roadrunner2]

Btw., the reason that it's very important that the driver be configured with the correct values in the y axis case is that it need to "invert" the raw y values received before reporting them, and for that it uses the bounds - so if the bounds in the driver are wrong, then it produces values in a range that is both different from the actual and the configured range, and things become confused.

[tudorbarascu]

@roadrunner2 I applied the patch and I cannot replicate the error anymore, which is really cool. I'm using the hwdb override with dpi entries only. Moreover, libinput-list-devices outputs a size of 135x83mm, which is almost the same as the actual one (before, I sometimes got much lower values). Really cool. Thanks again for your work.

[roadrunner2]

@tudorbarascu Great that this resolved the issue, and thanks for the help in debugging this. The physical size discrepency is a bit annoying though. A y dpi of 94 yields a height of 83.6mm; and you reported 85mm (exactly? slightly more? slightly less?). A dpi of 93 would give 84.5mm - maybe that's better then. Btw., I presume you measured the physical size of the pad itself, not the well it sits in?

[tudorbarascu]

I've measured the size of the pad, yes. Updated numbers with a better ruler: 135 and 83.8 I'm attaching some images, taken with the phone, with the camera above the measuring point, as far as I could (I zoomed in). I mitigated the measuring errors as much as possible.

The images show a little distortion (from what I could see) but the measuring is as precise as I could do.

So, yes, a dpi of 93 would better fit for the height.

Also, I'm using gnome on wayland I'm also getting this rare error (probably the way I'm using the touchpad):

[  3405.732] (EE) kernel bug: Touch jump detected and discarded.
See https://wayland.freedesktop.org/libinput/doc/1.6.3/touchpad_jumping_cursor.html for details

[roadrunner2]

@tudorbarascu Nice job on the photos! So I agree it looks like about 83.8mm - but that would be pretty precisely 94 dpi (which is 83.6mm). I.e. it seems to me that 94 is the correct y dpi value here.

Regarding the "Touch jump detected": yes, I see those occasionally too. So far I've been assuming they are due to spurious "touches" by my palm (the palm rejection needs some work for these large trackpads). But I haven't actually spent time trying to verify for certain that there isn't some bug in the driver (or even libinput) here.