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
93 stars 11 forks source link

Add support for Surface Laptop 4 (Intel) #57

Closed Edgarware closed 3 years ago

Edgarware commented 3 years ago

Adds support for Keyboard, Touchpad, and Battery status for Surface Laptop 4 (Intel). I can't seem to get the performance modes working, wasn't sure if it would be better to remove it from the node group for now.

qzed commented 3 years ago

Can you post the output of cat /sys/bus/acpi/devices/MSHW0250:00/path / confirm that that ID does indeed belong to the WSID device? Also I think you should just use the node group for the SL3. That's already in use on the AMD SL4 (since it shares the same ID with the AMD SL3) and AFAIK there aren't any differences to the previous generations wrt SAM.

I can't seem to get the performance modes working, wasn't sure if it would be better to remove it from the node group for now.

What kernel version, what did you try, and what went wrong?

Edgarware commented 3 years ago

Can you post the output of cat /sys/bus/acpi/devices/MSHW0250:00/path / confirm that that ID does indeed belong to the WSID device?

I'm just as confused as you as to why this uses a different ID than the AMD variant, but sure enough...

$ bash -c "grep '.*' /sys/bus/acpi/devices/*/path" | grep MSHW
/sys/bus/acpi/devices/MSHW0005:00/path:\_SB_.SLT_
/sys/bus/acpi/devices/MSHW0040:00/path:\_SB_.MSBT
/sys/bus/acpi/devices/MSHW0084:00/path:\_SB_.SSH_
/sys/bus/acpi/devices/MSHW0125:00/path:\_SB_.PC00.I2C0.FINK
/sys/bus/acpi/devices/MSHW0153:00/path:\_SB_.SHPS
/sys/bus/acpi/devices/MSHW0184:00/path:\_SB_.PC00.I2C4.ACSD
/sys/bus/acpi/devices/MSHW0250:00/path:\_SB_.WSID

Also I think you should just use the node group for the SL3. That's already in use on the AMD SL4 (since it shares the same ID with the AMD SL3) and AFAIK there aren't any differences to the previous generations wrt SAM.

OK, pull request updated.

What kernel version, what did you try, and what went wrong?

5.12.5-surface on Ubuntu 21.04, /sys/bus/surface_aggregator/devices/01:03:01:00:01/perf_mode does not exist.

qzed commented 3 years ago

I'm just as confused as you as to why this uses a different ID than the AMD variant, but sure enough...

$ bash -c "grep '.*' /sys/bus/acpi/devices/*/path" | grep MSHW
/sys/bus/acpi/devices/MSHW0005:00/path:\_SB_.SLT_
/sys/bus/acpi/devices/MSHW0040:00/path:\_SB_.MSBT
/sys/bus/acpi/devices/MSHW0084:00/path:\_SB_.SSH_
/sys/bus/acpi/devices/MSHW0125:00/path:\_SB_.PC00.I2C0.FINK
/sys/bus/acpi/devices/MSHW0153:00/path:\_SB_.SHPS
/sys/bus/acpi/devices/MSHW0184:00/path:\_SB_.PC00.I2C4.ACSD
/sys/bus/acpi/devices/MSHW0250:00/path:\_SB_.WSID

Thanks, yeah just wanted to make sure everything is in order.

qzed commented 3 years ago

5.12.5-surface on Ubuntu 21.04, /sys/bus/surface_aggregator/devices/01:03:01:00:01/perf_mode does not exist.

That interface has been replaced with the platform profile interface, so you should have /sys/firmware/acpi/platform_profile for control and /sys/firmware/acpi/platform_profile_choices for a list of available profiles.

qzed commented 3 years ago

If you don't mind I'd take this upstream with some small cleanup in the registry. Do you have a mail with which I can give you credit for this?

Edgarware commented 3 years ago

That interface has been replaced with the platform profile interface, so you should have /sys/firmware/acpi/platform_profile for control and /sys/firmware/acpi/platform_profile_choices for a list of available profiles.

Ah, yes those are present.

If you don't mind I'd take this upstream with some small cleanup in the registry. Do you have a mail with which I can give you credit for this?

If you have other refactors then don't worry about credit, this is a super minor change.

qzed commented 3 years ago

If you have other refactors then don't worry about credit, this is a super minor change.

I'd submit this as separate change in a small series. Are you sure?

qzed commented 3 years ago

Anyway, thanks for this PR!

Edgarware commented 3 years ago

I'd submit this as separate change in a small series. Are you sure?

Yep! Just glad to help.

qzed commented 3 years ago

Alright, thanks! I'll post a link to the patch once I've submitted it. Should hopefully land with all the other 7th-and-later-gen stuff in 5.13.

qzed commented 3 years ago

Link to series: https://lore.kernel.org/platform-driver-x86/20210523134528.798887-1-luzmaximilian@gmail.com/. Those patches should also show up in our next kernel release.