linux-surface / surface-aggregator-module

Linux ACPI and Platform Drivers for Surface Devices using the Surface Aggregator Module over Surface Serial Hub (Surface Book 2, Surface Pro 2017, Surface Laptop, and Newer)
GNU General Public License v2.0
97 stars 11 forks source link

Surface Laptop 3: Keyboard/Touchpad support #22

Closed qzed closed 4 years ago

qzed commented 4 years ago

Implement keyboard and touchpad support for Surface Laptop 3.

Keyboard event dump: https://pastebin.com/cVj3Q9xF Touchpad event dump: https://pastebin.com/c65QTcMd

heapwolf commented 4 years ago

@qzed I have a surface laptop 3 if you need any help! :)

qzed commented 4 years ago

@heapwolf Thank you for the offer, could you upload the output of ls -l /sys/bus/acpi/devices and ls -l /sys/bus/platform/devices?

heapwolf commented 4 years ago

@qzed definitely, at the moment i'm still trying to get it booted. See #591.

Michielnuyts commented 4 years ago

I own the Surface Laptop 3 15" AMD Ryzen 5, let me know if I can help with something.

archseer commented 4 years ago

I got the keyboard working (on SL3 13") with these changes: https://github.com/archseer/linux-surfacegen5-acpi/commit/8b6c4a5fc727ca018298478710bdc1901b673f25

It's a bit of a hack but it should work until we can get a more proper patch.

archseer commented 4 years ago

Some notes: RQID is now 0x0015 and TC is also 0x15. A the keyboard frame type is now 0x00 while the touchpad uses 0x80 that was used for all commands previously.

HID is 1 for keyboard, but 2 for touchpad so with some more tweaking we can probably get that working too. (might need a separate driver?)

Keyboard:

00000000: [aa 55 start] [00 frame type] [14 length] [00 padding] [4d sequence] [0a 82 crc] command-frame-start -> [80 type] [15 target] [00 02 always this] [00 subcontroller] [15 00 request id, event id] [00 command id]
00000010: [01 hid?] [00 modifier?] [13 key code?] 00 00 00 00 00 00 00 00 00 [32 46 crc] <-- hid report

Touchpad:

00000000: [aa 55 start] [80 frame type] [0e length] [00 padding] [f1 sequence] [c7 bd crc] command-frame-start -> [80 type] [15 target] [00 02 always this] [00 subcontroller] [15 00 request id, event id] [00 command id]
00000010: [02 hid] [00 modifier?] ff 01 00 00 [3c db crc] <-- hid report
archseer commented 4 years ago

We'll need to construct a HID descriptor for the touchpad. The touchpad payload consists of:

[02 report id] [00 button] [ff 01 direction] [00 00 ?] [3c db crc]

I think the second pair could be tracking for a second finger or something but I wasn't able to get it to be non-zero. Maybe it has to be enabled via the ACPI or something.

Been trying to modify

    //MOUSE TLC
    0x05, 0x01,                         // USAGE_PAGE (Generic Desktop)     
    0x09, 0x02,                         // USAGE (Mouse)                 
    0xa1, 0x01,                         // COLLECTION (Application)        
    0x85, 0x02,                         //   REPORT_ID (Mouse) (2)           
    0x09, 0x01,                         //   USAGE (Pointer)                
    0xa1, 0x00,                         //   COLLECTION (Physical)          
    0x05, 0x09,                         //     USAGE_PAGE (Button)          
    0x19, 0x01,                         //     USAGE_MINIMUM (Button 1)     
    0x29, 0x02,                         //     USAGE_MAXIMUM (Button 2)     
    0x25, 0x01,                         //     LOGICAL_MAXIMUM (1)          
    0x75, 0x01,                         //     REPORT_SIZE (1)              
    0x95, 0x02,                         //     REPORT_COUNT (2)             
    0x81, 0x02,                         //     INPUT (Data,Var,Abs)         
    0x95, 0x06,                         //     REPORT_COUNT (6)             
    0x81, 0x03,                         //     INPUT (Cnst,Var,Abs)         
    0x05, 0x01,                         //     USAGE_PAGE (Generic Desktop) 
    0x09, 0x30,                         //     USAGE (X)                    
    0x09, 0x31,                         //     USAGE (Y)                    
    0x75, 0x10,                         //     REPORT_SIZE (16)             
    0x95, 0x02,                         //     REPORT_COUNT (2)             
    0x25, 0x0a,                          //    LOGICAL_MAXIMUM (10)      
    0x81, 0x06,                         //     INPUT (Data,Var,Rel)         
    0xc0,                               //   END_COLLECTION                 
    0xc0,                                //END_COLLECTION    

https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/windows-precision-touchpad-sample-report-descriptors

archseer commented 4 years ago

I added the missing mouse descriptor by hand based on the windows precision touchpad example and now touchpad works as well 🎊All I had to do was modify it from three mouse buttons to two.

https://github.com/archseer/linux-surfacegen5-acpi/commit/f79b8210ac5fdc3b1fbdb4e8d5b991a2da22f87f

archseer commented 4 years ago

By default, the touchpad works as a plain mouse (no gestures). It seems that to get the touchpad working as a touchpad/digitizer, there's some extra steps involved:

https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/windows-precision-touchpad-required-hid-top-level-collections#configuration-collection

We should also be able to talk to the EC to retrieve the HID descriptors instead of hardcoding them (somehow).

qzed commented 4 years ago

Nice work!

We should also be able to talk to the EC to retrieve the HID descriptors instead of hardcoding them (somehow).

I agree, but as discussed on IRC, figuring out how unfortunately isn't that easy. I'll first try to implement a driver with a hard-coded descriptor and then look a into automated tooling to try to figure out which EC commands exist.

mistay commented 4 years ago

i managed to build a recent vanilla kernel, patched by https://github.com/qzed/linux-surface, see https://github.com/qzed/linux-surface-kernel/issues/17#issuecomment-553652790

after booting that kernel, i inserted this kernel module (https://github.com/qzed/linux-surfacegen5-acpi/tree/master/module)

sudo insmod surface3_sam.ko
$ lsmod | grep surf
surface_sam            65536  0
crc_ccitt              16384  1 surface_sam
mfd_core               16384  2 hid_sensor_hub,surface_sam
hid                   143360  7 i2c_hid,usbhid,hid_multitouch,hid_sensor_hub,intel_ipts,hid_generic,surface_sam

but, although surface_sam.ko loaded successfully into recent kernel, keyboard's still not responding

qzed commented 4 years ago

@mistay I assume you've unloaded all other surface_sam_... modules before loading it? In that case can you check the output of ls -al /sys/bus/serial/devices/serial0-0/. There should be a driver link.

mistay commented 4 years ago

@mistay I assume you've unloaded all other surface_sam_... modules before loading it?

yes

In that case can you check the output of ls -al /sys/bus/serial/devices/serial0-0/. There should be a driver link.

sorry, no such file or directory.

# ls -alh /sys/bus/serial/devices/
total 0
drwxr-xr-x 2 root root 0 Nov 14  2019 .
drwxr-xr-x 4 root root 0 Nov 14  2019 ..
qzed commented 4 years ago

This is on the 13" model, right? You do have the ACPI patches applied? Specifically 0001 and 0002 from https://github.com/qzed/linux-surfacegen5-acpi/tree/master/patches/5.3 (or alternatively https://github.com/qzed/linux-surface/blob/master/patches/5.3/0001-surface-acpi.patch ?

mistay commented 4 years ago

This is on the 13" model, right?

yes

You do have the ACPI patches applied? Specifically 0001 and 0002 from https://github.com/qzed/linux-surfacegen5-acpi/tree/master/patches/5.3 (or alternatively https://github.com/qzed/linux-surface/blob/master/patches/5.3/0001-surface-acpi.patch ?

As linked (https://github.com/qzed/linux-surface-kernel/issues/17#issuecomment-553652790) above, I applied all patches from https://github.com/qzed/linux-surface/tree/master/patches/5.3

Do I have to need to apply the patches in this repro?

A step-by-step tutorial would be great!

And: Thanks für all your work, I really appreciate that, so please let me know how I can support you.

mistay commented 4 years ago

@qzed which patches should i apply to get the keyboard working?

qzed commented 4 years ago

Do I have to need to apply the patches in this repro?

The patches in the linux-surface repo cover the necessary fixes that you need to load the drivers. Specifically the surface-acpi patch. That one basically combines all three patches in this repo here, so no.

The interesting part is that the serial device does not show up at all. Usually this would indicate that the serdev patch was not applied or some configuration is wrong. But it looks like you've got that right.

Can you upload a dmesg log?

Also you should be able to load the module on the release kernels provided at qzed/linux-surface.

mistay commented 4 years ago

i applied patches from https://github.com/qzed/linux-surface/tree/master/patches/5.3 but not from https://github.com/qzed/linux-surfacegen5-acpi/tree/master/patches

mistay commented 4 years ago

dmesg.log

mistay commented 4 years ago

had a closer look and have to add some information here:

  1. cloned vanilla kernel, applied patches from https://github.com/qzed/linux-surface/tree/master/patches/5.3
  2. booted kernel from 1., built this repository's module/surface_sam.ko module, tried to insmod
  3. the step above (2.) failed because of duplicate symbols (i think owned by surface_sam_ssh)
  4. blacklisted all surface* modules that come w/ that recent kernel, i.e.
    
    blacklist surface3_button
    blacklist surface3_power
    blacklist surface3-wmi
    blacklist surfacebook2_dgpu_hps
    blacklist surfacepro3_button

blacklist surface_sam_dtx blacklist surface_sam_san blacklist surface_sam_sid_gpelid blacklist surface_sam_sid blacklist surface_sam_sid_perfmode blacklist surface_sam_ssh blacklist surface_sam_vhf


5. rebooted, again, tried to insmod surface_sam.ko
6. insmodding failed:
[   60.291554] surface_sam: loading out-of-tree module taints kernel.
[   60.291912] surface_sam: Unknown symbol crc_ccitt_false (err -2)
7. successfully modprobed surface_sam_ssh
8. insmod surface_sam.ko -> failed
[   85.098073] surface_sam: exports duplicate symbol surface_sam_ssh_consumer_register (owned by surface_sam_ssh)
9. rmmod surface_sam_ssh
10. insmod surface_sam.ko -> succeeded, so module loaded but w/ errors in dmesg

[  103.932688] ACPI BIOS Error (bug): AE_AML_PACKAGE_LIMIT, Index (0x0000000FF) is beyond end of object (length 0x10) (20190703/exoparg2-393)
[  103.932700] ACPI Error: Aborting method \_SB.GINF due to previous error (AE_AML_PACKAGE_LIMIT) (20190703/psparse-529)
[  103.932702] ACPI Error: Aborting method \_SB.GADR due to previous error (AE_AML_PACKAGE_LIMIT) (20190703/psparse-529)
[  103.932704] ACPI Error: Aborting method \_SB.SGOV due to previous error (AE_AML_PACKAGE_LIMIT) (20190703/psparse-529)
[  103.932706] ACPI Error: Aborting method \_SB.CGWR due to previous error (AE_AML_PACKAGE_LIMIT) (20190703/psparse-529)
[  103.932708] ACPI Error: Aborting method \_SB.TBFP due to previous error (AE_AML_PACKAGE_LIMIT) (20190703/psparse-529)
[  103.932710] ACPI Error: Aborting method \_SB.WMTF.WMTF due to previous error (AE_AML_PACKAGE_LIMIT) (20190703/psparse-529)
[  103.937433] ACPI BIOS Error (bug): AE_AML_PACKAGE_LIMIT, Index (0x0000000FF) is beyond end of object (length 0x10) (20190703/exoparg2-393)
[  103.937447] ACPI Error: Aborting method \_SB.GINF due to previous error (AE_AML_PACKAGE_LIMIT) (20190703/psparse-529)
[  103.937450] ACPI Error: Aborting method \_SB.GADR due to previous error (AE_AML_PACKAGE_LIMIT) (20190703/psparse-529)
[  103.937452] ACPI Error: Aborting method \_SB.SGOV due to previous error (AE_AML_PACKAGE_LIMIT) (20190703/psparse-529)
[  103.937454] ACPI Error: Aborting method \_SB.CGWR due to previous error (AE_AML_PACKAGE_LIMIT) (20190703/psparse-529)
[  103.937456] ACPI Error: Aborting method \_SB.TBFP due to previous error (AE_AML_PACKAGE_LIMIT) (20190703/psparse-529)
[  103.937458] ACPI Error: Aborting method \_SB.WMTF.WMTF due to previous error (AE_AML_PACKAGE_LIMIT) (20190703/psparse-529

so, maybe it just seems that the kernel module successfully loaded but it obviously didn't... @qzed what do you think? could you just give me an idea about what has to be done in order to use my laptop's keyboard?
qzed commented 4 years ago

3. the step above (2.) failed because of duplicate symbols (i think owned by surface_sam_ssh)

Yes, that is to be expected. Just unload the modules provided by the kernel (e.g. by running the unload.sh script in this repository) before loading the module from this repo. No need to blacklist and reboot, but that works too.

6. insmodding failed: [ 60.291554] surface_sam: loading out-of-tree module taints kernel. [ 60.291912] surface_sam: Unknown symbol crc_ccitt_false (err -2)

For some reason the kernel can't find the crc_ccitt_false function which is required for the module. Try loading the crc-ccitt before loading the sam module. (Although I'd thought that the kernel would do that for you automatically.)

10. insmod surface_sam.ko -> succeeded, so module loaded but w/ errors in dmesg

Those seem to come from a PNP0C14 device, which as far as I can see is completely unrelated.

Apart from that, loading the module, even successfully, won't help you right now. For the driver to load you need a device it can attach to. That would (normally) be /sys/bus/serial/devices/serial0-0/, which doesn't exist for you for some reason. Just to be absolutely sure, this is an Intel device, right? In that case there should be a line like

dw-apb-uart.4: ttyS0 at MMIO 0xd4748000 (irq = 20, base_baud = 3000000) is a 16550A

in dmesg, but that seems to be missing. By any chance, do you have intel_lpss and/or intel_lpss_pci blacklisted? Those are required for the UART/tty/serdev device to show up. Also blacklisting them is not required any more provided you have applied https://github.com/qzed/linux-surface/blob/master/patches/5.3/0010-ioremap_uc.patch.

mistay commented 4 years ago

that did the tick, i blacklisted intel_lpss in order to boot non-patched kernels. after removing that blacklist the device showed up and the keyboard works!! thanks @qzed !!

mistay commented 4 years ago

and yes, intel version of 13“ surface laptop 3

xjcl commented 4 years ago

My experience with Linux on the Surface Laptop 3 has been very bad so far.

First I tried about a dozen distros, all of which wouldn't boot even into the live USB. I finally got Linux Mint to boot by adding "noapic nosplash" (while removing "quiet splash") to the boot options, however I was unable to install it as Cinnamon kept crashing every time I restarted it. I finally installed Ubuntu Mate with a ridiculous setup of USB hubs and adapters as neither keyboard nor mouse nor touchscreen nor wifi were working.

Anyway long story short I did run this repo's setup script and booted with the custom kernel and nothing happened, all 4 components are still broken. So this issue definitely shouldn't be closed. I'll try to read this thread and look at suggestions once I have access to the hubs and adapters again.

fxgsell commented 4 years ago

@xjcl runs fine for me with Ubuntu 20.04, upstream Kernel 5.5, with this module.

Did not need the custom kernel or any boot parameters, only the module.

qzed commented 4 years ago

@xjcl For older upstream kernels you'll need to add modprobe.blacklist=intel_lpss_pci to the kernel parameters (as mentioned above and hinted at on the wiki install/setup page). This option however blacklists a module that is required by the SAM driver to work, so you'll have to set it only temporary (or only for upstream kernels). In 5.5.x kernels, this issue has been fixed and the option is not needed any more.

Also as @fxgsell noted, in 5.5 all necessary patches for the Laptop 3 and Pro 7 have been integrated into the kernel, so it's enough to install the module.

archseer commented 4 years ago

@qzed, I think 5.5 still needs https://github.com/torvalds/linux/commit/6d232b29cfce65961db4a668c2c6c6987cd24d45 but 5.6 will ship all the necessary patches to just run surface_sam without a patched kernel.

@xjcl

xjcl commented 4 years ago

I got keyboard, trackpad and wifi to work using the debian-test-2 kernel: https://github.com/linux-surface/linux-surface/releases/tag/debian-test-2 The touchscreen is broken according to the feature matrix so I guess I'll have to wait

I was actually using the jakeday/linux-surface kernel (where everything was broken, even wifi which mostly worked with the vanilla kernel it turns out) instead of the linux-surface/linux-surface kernel. Why are there two repositories anyway? Very confusing

I didn't try the vanilla 5.5 kernel, but thanks for the tip

Thanks for all the help so far!

qzed commented 4 years ago

I think 5.5 still needs torvalds/linux@6d232b2 but 5.6 will ship all the necessary patches to just run surface_sam without a patched kernel.

You can get away without that patch on the SL3. The patch is only required for the SAN driver and AFAIK that is only used for some of the more exotic stuff on the SL3 (the usage may also depend a bit if it's the AMD or Intel version).

Why are there two repositories anyway?

This repo started out as a fork, mainly because Jake was/is inactive.

pairwiserr commented 4 years ago

@qzed @xjcl @archseer I installed Ubuntu 20.04 on SL3 13 (Intel, i7, 16, 256) with 5.4 kernel. I then upgraded the kernel to 5.6.19 but keyboard and trackpad are still not being detected. Can anyone confirm if 5.5 mainline detect keyboard/trackpad ootb without any patches?

archseer commented 4 years ago

The prerequisite patches landed but the ACPI driver patch hasn't been upstreamed yet so of course it's still necessary. With the prerequisites upstream now though you're able to run the vanilla kernel + this driver via dkms.

archseer commented 4 years ago

We recommend that you run a kernel provided by https://github.com/linux-surface/linux-surface which already contains this module (so if you run it you don't have to install it separately), however, since v5.7, you can also manually install this module, without any additional patches required. Prior to v5.7 you will need to use a kernel with the patches provided in this repository applied. For more details on the installation process, see Testing/Installing.

https://github.com/linux-surface/surface-aggregator-module/wiki

pairwiserr commented 4 years ago

@archseer, thanks for your reply. I am new to Linux and was curious if I could confirm the manual install process for this module.

https://www.kernel.org/doc/html/v4.10/process/applying-patches.html

patch -p1 -i path/to/patch-x.y.z

Can I run patch -p1 path/to/0001-ACPI-Fix-buffer-integer-type-mismatch.patch in master/patches/5.4?

archseer commented 4 years ago

Those are the prerequisite patches that were needed before 5.5. They're now in the upstream kernel so all you need to do is install the module

Something like:

git clone https://github.com/linux-surface/surface-aggregator-module
cd module
make
make dkms-install

(you might also need to apt-get install linux-headers build-essential before running make)

The reason we recommend you just use the patched kernel is that if you do it via dkms you'll have to pull in new code and manually update every once a while as new changes land.

Also, I recommend you use the latest kernel (5.7 + mesa 20.1), not 5.4 or earlier. 5.4-5.6 had various issues on SL3/newer intel chips, most notably GPU hangs (https://github.com/linux-surface/linux-surface/issues/99)

pairwiserr commented 4 years ago

@archseer, your suggestion worked like a charm on v5.5.

Also, I recommend you use the latest kernel (5.7 + mesa 20.1),

I haven't had any issues yet, but I will try this as well. When installing kernels from mainline, is it best practice to use the latest version for stability? In this case, 5.7.11?

Thank you, again, @archseer for all your help!