Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Surface Pro 2017 / Book 2: Power/Volume Buttons Don't Work #34

Closed
andreweskild opened this issue Nov 14, 2017 · 22 comments
Closed

Surface Pro 2017 / Book 2: Power/Volume Buttons Don't Work #34

andreweskild opened this issue Nov 14, 2017 · 22 comments
Assignees
Labels
surface-acpi module Bug/feature-request for the surface-acpi module

Comments

@andreweskild
Copy link

Everything else is pretty much perfect, but unfortunately power and volume buttons are still non functional.
I'm not entirely familiar with how they are implemented, but from what I can tell, they are supposed to be implemented as acpi buttons. However, when pressed, they do not do anything, no messages appear in dmesg, and nothing shows when running acpi_listen. I also tried evtest, which displays the device as Surface Pro 3/4/2017 Buttons, but no events appear there either.

@jakeday
Copy link
Owner

jakeday commented Nov 14, 2017

I haven't finished the buttons for the Pro 2017 yet. I'll do that soon and report back here.

@andreweskild
Copy link
Author

No worries, take your time

@jakeday jakeday changed the title Power/Volume buttons don't work on Surface Pro 2017 Surface Pro 2017: Power/Volume Buttons Don't Work Nov 14, 2017
@jakeday jakeday changed the title Surface Pro 2017: Power/Volume Buttons Don't Work Surface Pro 2017 / Book 2: Power/Volume Buttons Don't Work Nov 18, 2017
@jakeday
Copy link
Owner

jakeday commented Nov 18, 2017

The Surface Pro 2017 and Surface Book 2 both share the same MSHW0040 to control the buttons. I'll have to brush up on my DSDT to fix this unless someone is kind enough to help.

@andreweskild
Copy link
Author

I'm not entirely familiar with dsdt patching, I'll spend some time with over the next couple days, see what I can come up with. On that note, to your knowledge is this an issue that only affects the pro 17/book 2, because I was under the impression that the pro 4 also used the MSHW0040 device to control the buttons

@jakeday
Copy link
Owner

jakeday commented Nov 18, 2017

@andreweskild It's only an issue with the Pro 2017 and Book 2. The others use more of a standard DSDT from the BIOS.

@andreweskild
Copy link
Author

I see. It would probably be helpful to have a look at the DSDT of the surface pro 4. I don't suppose you have a Surface Pro 4? And if so, would it be too much trouble to ask for a copy of your DSDT/SSDTs?
Thanks.

@npjohnson
Copy link
Contributor

I happen to be decent at DSDT-related work. Can somebody dump theirs and post it here? (preferably with model info, etc.)

@carlblomqvist
Copy link

carlblomqvist commented Dec 17, 2017

@npjohnson demlow posted this in another issue: https://bugzilla.kernel.org/show_bug.cgi?id=198145
I think you'll find what you need there.

For some reason my DSDT was different (9000 more lines) from the one posted in the link above. Here's a diff https://hastebin.com/talifiguka.php

@versipellis
Copy link

@npjohnson I'm not sure how to dump the DSDT/SSDT, if there's something you can point me to, I'd be more than willing to help where I can.

@jakeday jakeday self-assigned this Jan 20, 2018
@jakeday jakeday added the bug label Jan 20, 2018
@JoshKaufman
Copy link

JoshKaufman commented Jan 25, 2018

@npjohnson Here is the DSDT for surface book 2 15" any help or tips will be greatly appreciated.

On the SurfacePro4 Device(VGBI) handles the power/volume-keys but looking at the dsdt for the SP4 it has a different _DSM and I see a few Notify(VGBI, 0xXX) but i don't see those calls on the SB2

@jakeday jakeday added surface-acpi module Bug/feature-request for the surface-acpi module and removed bug labels Feb 14, 2018
@net-monitor
Copy link

Auto rotate screen dont work..

@lucasvanlierop
Copy link

Hi all, I'm considering to buy a Surface Book 2 and run Linux on it (of course helping out this project with debugging and a donation). I was wondering: is it completely useless when the power button does not work. Or is it just that you have to type shutdown to power it down, if so what if it hangs?

@andrew-ws
Copy link

You can use shutdown or whatever shutdown utilities your window manager/bar/whatever provides. I can only speak for the surface pro 2017, but I believe this doesn't affect the long press of the power-button, as that's a hardware-level function. Thus, if your laptop hangs on shutdown (mine does pretty frequently) you can long-press the power button and that will force shutdown. One inconvenience of the issues with the buttons that you may not have anticipated is that if your computer goes into a lock/sleep state while it's in clipboard mode, there's no way to wake it up, as the touchscreen (purposefully) doesn't respond, and the buttons don't either.

@forlornidealist
Copy link

@lucasvanlierop I have a Surface Book 2 and run Ubuntu 18.04 on it. I ran 17.10 previously, but 18.04 got rid of some of the bugs I was seeing, likely because 17.10 was running wayland vs. xorg. I can second what @andrew-w-sherman says about the unanticipated issue of not being able to wake it up (and yes, if you hold down the button on an SB2 it forces the machine off). That actually is the only major problem I have with the buttons not working, but I don't use mine as a tablet/undocked very often. I have faith jakeday will be able to come up with the ACPI module to fix this though. I know he's been working on it.

Since the SB2 is a very pricey machine I'll tell you a bit more on what my experience has been. The only other issues I have besides the buttons and battery reading is hibernation. It works for me but only one time, then I have to reboot again. If I try to hibernate a second time without rebooting, it just kills WiFi and locks the screen. Suspend is not possible so this may be a bigger deal than you think. The Wi-Fi also drops from time to time, although this is much less frequent for me now. I can get WiFi back up by reloading the modules which is pretty quick and painless so it isn't really a big deal to me.

So in short, depending on what distro/release you're planning to use, the only real issues for the SB2 that I've experienced at this point are the buttons not working, the lack of battery information, hibernating more than one time, and Wi-Fi randomly dropping, which happens < 1/day for me.

@lucasvanlierop
Copy link

@andrew-w-sherman @forlornidealist Thanks sharing your experiences! I did buy a Surface book 2 in the meantime. It's not my main machine yet but I'll try to install Ubuntu 18.04 on it soon and see how that works out.

@qzed
Copy link
Collaborator

qzed commented Jun 26, 2018

I've tinkered around with this a bit on the Surface Book 2 today. Basically, I have no idea of ACPI/Linux kernel development, but I've got some free time, so... If I've understood that correctly, the MSHW0040 is responsible for the buttons. However, it doesn't send ACPI notify signals, so a basic ACPI driver isn't going to work.

Now I found that this is quite similar to the Surface 3 (non-pro, DSDT, TEV2/MSHW0028 is the relevant device here), which has support in the kernel. As I understood, they're mapping the GPIO interrupts defined in the DSDT to ACPI events, which are then going to be handled by a generic ACPI button driver.

I tried to adapt this for the Surface Book 2, but I can't even get the probe function to be called. Basically I tried to compile this as a kernel module, changed the surface3_acpi_match, SURFACE_BUTTON_OBJ_NAME, and surface3_button_surface3 array using the Surface Book DSDT. Any Ideas what I'm doing wrong?

@qzed
Copy link
Collaborator

qzed commented Jun 27, 2018

And apparently I'm stupid and missed that the Surface 3 has an I2C controller specified. So of course it will not probe because there's no I2C device for the I2C driver to probe. I guess I'll have a look at the ACPI spec then.

@qzed
Copy link
Collaborator

qzed commented Jun 28, 2018

Ok, I got a working driver here. Feedback is appreciated. I've tested it on a Surface Book 2 (Debian Buster, 4.16) but it should also work on the Surface Pro (2017).

There is an issue though. Both, Surface Book 2/Surface Pro (2017) and Surface Pro 4 use the same _HID (namely MSHW0040) but the IO model is different. The Surface Pro 4 uses ACPI notifications, whereas the Surface Book 2 uses GPIO interrupts. The problem now is that both drivers want to be registered for MSHW0040. So we need a way to differentiate between SB2 and SP4 when loading those drivers. I have no idea how that should be done, I'm open for hints/suggestions.

Also, this driver is largely based on soc_button_array.c, so it might make sense to write a patch for this once the issue above has been handled.

@mr-remington
Copy link
Contributor

mr-remington commented Jun 28, 2018

@qzed I just tested your module on my SB2 15 and holy crap all the buttons work, thats amazing!

Edit: I've also tested this on my SP2017 and it works! Both devices without the need to modprobe - rsurfacepro3_button

@tworzenieweb
Copy link

@gzed I am speechless 😲 you did an amazing job. Works also for me on SB2 15 on archlinux

@jakeday
Copy link
Owner

jakeday commented Jun 28, 2018

@qzed I agree, we could just add the device hid to the existing module and make the change for the mapping there. I went ahead and included your module next to the other surface button drivers (cause they mostly did the same thing) and it'll be in my next kernel release in a few minutes. Great job!

@jakeday jakeday closed this as completed Jun 28, 2018
@qzed
Copy link
Collaborator

qzed commented Jun 28, 2018

Alright, very nice! I'll update my readme so that every one knows It's integrated with the new release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
surface-acpi module Bug/feature-request for the surface-acpi module
Projects
None yet
Development

No branches or pull requests