The laptop won't suspend with the USB drivers (ehci, xhci) loaded. Suspend bounce works fine - everything gets powered down fine, then comes back up. Unloading the USB drivers fixes the problem, but that's hiding the underlying causes - the trick is the transition of the USB ports into D3. The ACPI BIOS has this: Device (EHC1) { Name (_ADR, 0x001D0000) // _ADR: Address OperationRegion (PWKE, PCI_Config, 0x62, 0x04) Field (PWKE, DWordAcc, NoLock, Preserve) { , 1, PWUC, 8 } Method (_PSW, 1, NotSerialized) // _PSW: Power State Wake { If (Arg0) { Store (Ones, PWUC) /* \_SB_.PCI0.EHC1.PWUC */ } Else { Store (0x00, PWUC) /* \_SB_.PCI0.EHC1.PWUC */ } } Method (_S3D, 0, NotSerialized) // _S3D: S3 Device State { Return (0x02) } Method (_S4D, 0, NotSerialized) // _S4D: S4 Device State { Return (0x02) } .. so, we should be trying to put it into D2, not D3. But then: ehci0 pnpinfo vendor=0x8086 device=0x1c26 subvendor=0x1043 subdevice=0x1427 class=0x0c0320 at slot=29 function=0 handle=\_SB_.PCI0.EHC1 ehci0@pci0:0:29:0: class=0x0c0320 card=0x14271043 chip=0x1c268086 rev=0x05 hdr=0x00 vendor = 'Intel Corporation' device = '6 Series/C200 Series Chipset Family USB Enhanced Host Controller' class = serial bus subclass = USB cap 01[50] = powerspec 2 supports D0 D3 current D0 cap 0a[58] = EHCI Debug Port at offset 0xa0 in map 0x14 cap 13[98] = PCI Advanced Features: FLR TP .. we shouldn't be putting it into D2, because the device doesn't support it. However, we are actually transitioning it into D3. I don't have a bootverbose output here, but just assume we are. If I leave the driver loaded but I disable the suspend-to-d3 option, things suspend/resume fine. If I set the unconfigured-devices-get-d3 option and unload the usb modules, then the ehci controller ends up at d3 when I unload it. Then if I suspend, the laptop hangs. So, jhb@ asked me to add some debugging. Which I did, and I got this: acpi_device_pwr_for_sleep: \134_SB_.PCI0: dstate=3 acpi_device_pwr_for_sleep: \134_SB_.PCI0: now dstate=3 acpi_device_pwr_for_sleep: \134_SB_.PCI0: dstate=3 acpi_device_pwr_for_sleep: \134_SB_.PCI0: now dstate=3 acpi_device_pwr_for_sleep: \134_SB_.PCI0: dstate=3 acpi_device_pwr_for_sleep: \134_SB_.PCI0: now dstate=3 acpi_device_pwr_for_sleep: \134_SB_.PCI0.RP01: dstate=3 acpi_device_pwr_for_sleep: \134_SB_.PCI0.RP01: now dstate=3 acpi_device_pwr_for_sleep: \134_SB_.PCI0: dstate=3 acpi_device_pwr_for_sleep: \134_SB_.PCI0: now dstate=3 ath_hal_reg_write: reg=0x000000a0, val=0x00000000, pm=1 ath_hal_reg_write: reg=0x000000ac, val=0x00000000, pm=1 acpi_device_pwr_for_sleep: \134_SB_.PCI0.RP02: dstate=3 acpi_device_pwr_for_sleep: \134_SB_.PCI0.RP02: now dstate=3 acpi_device_pwr_for_sleep: \134_SB_.PCI0.RP02: dstate=3 acpi_device_pwr_for_sleep: \134_SB_.PCI0.RP02: now dstate=3 acpi_device_pwr_for_sleep: \134_SB_.PCI0: dstate=3 acpi_device_pwr_for_sleep: \134_SB_.PCI0: now dstate=3 acpi_device_pwr_for_sleep: \134_SB_.PCI0.RP04: dstate=3 acpi_device_pwr_for_sleep: \134_SB_.PCI0.RP04: now dstate=3 acpi_device_pwr_for_sleep: \134_SB_.PCI0: dstate=3 acpi_device_pwr_for_sleep: \134_SB_.PCI0: now dstate=3 uhub0: at usbus0, port 1, addr 1 (disconnected) ugen0.2: <vendor 0x8087> at usbus0 (disconnected) uhub1: at uhub0, port 1, addr 2 (disconnected) ugen0.3: <Azurewave> at usbus0 (disconnected) ugen0.4: <Generic> at usbus0 (disconnected) ugen0.5: <Atheros Communications> at usbus0 (disconnected) acpi_device_pwr_for_sleep: \134_SB_.PCI0: dstate=3 acpi_device_pwr_for_sleep: \134_SB_.PCI0: now dstate=3 acpi_device_pwr_for_sleep: \134_SB_.PCI0: dstate=3 acpi_device_pwr_for_sleep: \134_SB_.PCI0: now dstate=3 acpi_device_pwr_for_sleep: \134_SB_.PCI0: dstate=3 acpi_device_pwr_for_sleep: \134_SB_.PCI0: now dstate=3 acpi_device_pwr_for_sleep: \134_SB_.PCI0: dstate=0 acpi_device_pwr_for_sleep: \134_SB_.PCI0: now dstate=0 acpi_device_pwr_for_sleep: \134_SB_.PCI0: dstate=0 acpi_device_pwr_for_sleep: \134_SB_.PCI0: now dstate=0 info: [drm] Enabling RC6 states: RC6 off, RC6p off, RC6pp off acpi_device_pwr_for_sleep: \134_SB_.PCI0: dstate=0 acpi_device_pwr_for_sleep: \134_SB_.PCI0: now dstate=0 acpi_device_pwr_for_sleep: \134_SB_.PCI0: dstate=0 acpi_device_pwr_for_sleep: \134_SB_.PCI0: now dstate=0 acpi_device_pwr_for_sleep: \134_SB_.PCI0.RP02: dstate=0 acpi_device_pwr_for_sleep: \134_SB_.PCI0.RP02: now dstate=0 acpi_device_pwr_for_sleep: \134_SB_.PCI0: dstate=0 acpi_device_pwr_for_sleep: \134_SB_.PCI0: now dstate=0 acpi_device_pwr_for_sleep: \134_SB_.PCI0: dstate=0 acpi_device_pwr_for_sleep: \134_SB_.PCI0: now dstate=0 acpi_device_pwr_for_sleep: \134_SB_.PCI0: dstate=0 acpi_device_pwr_for_sleep: \134_SB_.PCI0: now dstate=0 acpi_device_pwr_for_sleep: \134_SB_.PCI0: dstate=0 acpi_device_pwr_for_sleep: \134_SB_.PCI0: now dstate=0 acpi_device_pwr_for_sleep: \134_SB_.PCI0: dstate=0 acpi_device_pwr_for_sleep: \134_SB_.PCI0: now dstate=0 uhub0: <Intel EHCI root HUB, class 9/0, rev 2.00/1.00, addr 1> on usbus0 uhub_attach: Turning port 1 power on uhub_attach: Turning port 2 power on uhub0: 2 ports with 2 removable, self powered ugen0.2: <vendor 0x8087> at usbus0 uhub1: <vendor 0x8087 product 0x0024, class 9/0, rev 2.00/0.00, addr 2> on usbus0 uhub_attach: Turning port 1 power on uhub_attach: Turning port 2 power on uhub_attach: Turning port 3 power on uhub_attach: Turning port 4 power on uhub_attach: Turning port 5 power on uhub_attach: Turning port 6 power on uhub_attach: Turning port 7 power on uhub_attach: Turning port 8 power on uhub1: 8 ports with 8 removable, self powered ugen0.3: <Azurewave> at usbus0 ugen0.4: <Generic> at usbus0 ugen0.5: <Atheros Communications> at usbus0 These printf()s are in acpi_device_pwr_for_sleep(), printing out the acpi_name(handle) for the handle we're doing the lookup on. It turns out that 'dev' here is the underlying bus, not the device it's attached to. So, all these lookups are for PCIx, not the leaf devices - and it never sees the EHC* nodes. So I'm not sure what to do here. Should acpi_device_pwr_for_sleep() be being called for leaf nodes rather than just the busses? As a side note - I think the xhci controller does the same thing, but I'm going to worry about that particular problem -after- we solve the ehci problem. The full files can be found at http://people.freebsd.org/~adrian/asus_zenbook/
Created attachment 149219 [details] Proposed fix
Proposed fix makes Suspend/Resume on this ASUS UX32VD useful: Before the fix, I could suspend/resume, music would continue playing but the display wouldn't come back on. With this fix, and acpi_video loaded, I can suspend/resume properly and have everything working as before.
Comment on attachment 149219 [details] Proposed fix Hah, that's a dumb bug. Nice fix.
Created attachment 149269 [details] A proposed fix + acpi.c patch I think you need to fix r214072, too. It seems I just copied the logic from r211430.
Ok, that looks sane. Just need jhb@ to weigh in and OK it and I'll commit both patches to -HEAD. Thanks!
Looks good to me, fire away!
A commit references this bug: Author: adrian Date: Tue Nov 11 17:14:12 UTC 2014 New revision: 274386 URL: https://svnweb.freebsd.org/changeset/base/274386 Log: Use the correct device (child) when asking the bus layer about which power state said device should go into. This was a snafu introduced in the ACPI/PCI awareness separation. When putting a device into a power state, the bus (and thus firmware, eg ACPI) should be asked before hand to check whether the device can indeed go into that power state. There's a set of nodes in ACPI under each device - the _SxD nodes - which state which ACPI power state to put the device into when the system is going into power save state 'x'. So when going into S3, the existence of an _S3D node would override whatever the system was trying to do. By default the PCI code wants to put devices into D3 before suspending. I have a laptop here (Asus Zenbook - check the PR) whose EHCI controller really wants to be in D2 during suspend, not D3. So if we put it into D3 and then try to enter S3, everything hangs. The device itself can go into D3 - it just can't be there when the call to ACPI to enter S3 occurs. The PCI patch fixes this. jkim@ noticed that the same is needed for the ACPI child device enumeration. Thankyou to Matt Dillon (the programmer, not the actor) for buying me this particular laptop so I could debug the issues with the Atheros AR9485 that is in it. It's his fault that I ended up with this laptop and was sufficiently annoyed by the lack of USB suspend to go down this rabbit hole. Tested: * Thinkpad T400 * Thinkpad X230 * Thinkpad T42 * Thinkpad T60 * Asus Zenbook (see PR) * Asus EEEPC 701 * Asus EEEPC 1001PX TODO: * Figure out what we should do about devices we unload drivers for that want to be in a specific state when entering S3 / S4 - the "put devices into D3 if they're not bound to a driver" option may also mess with things. PR: kern/194884 Reviewed by: jhb, jkim MFC after: 1 week Relnotes: yes Sponsored by: Matt Dillon <dillon@apollo.backplane.com> (hardware) Changes: head/sys/dev/acpica/acpi.c head/sys/dev/pci/pci.c
A commit references this bug: Author: jkim Date: Tue Nov 11 19:42:10 UTC 2014 New revision: 274397 URL: https://svnweb.freebsd.org/changeset/base/274397 Log: Use the correct device. Note this commit complements r274386. PR: 194884 Changes: head/sys/dev/acpica/acpi.c
Fixed!
It appears these commits may not have made stable/* Can original committers to HEAD (now Assigned/CC) please check to confirm this please, and set mfc_* flags to + once committed
Keep freebsd-acpi informed about this issue, so that it makes 10.3-RELEASE
if someone's going to backport this to stable/10 then please hit up jhb@ for the full story. IIRC, there were a bunch of things unearthed around this change that required some subsequent commits. (I don't know if everything has been resolved with it in -HEAD yet; I need to check -acpi and -current..)
Well the combined two patches amount to 3 lines of code changed, the patch applies cleanly to stable/10 and you have an independent confirmation that the bug identified in the title of this PR disappears when applying this patch on stable/10. I have also investigated the commit and subsequent changes in pci.c and acpi.c and was unable to find any related changes that also need backporting. Curious to see what the "unearthed bunch of things" becomes.
I applied these patches (both acpi.c and pci.c) to my then 9.2-R system (Lenovo X200) on 2014-11-14. I'd never had Adrian's problem with suspend but the patches did get rid of 3 apparently harmless error messages, and add one new one, also apparently harmless. It's been running fine since. Having updated it to stable/9 a month ago, I assumed these would have been included, but checking dmesg I confirm they weren't, but are safe for 9.x This is the new one introduced by the patches (that has now gone) -pcib0: failed to set ACPI power state D2 on \134_SB_.PCI0: AE_BAD_PARAMETER And these are the ones the two patches removed (thus, have returned :) +pci0: failed to set ACPI power state D2 on \134_SB_.PCI0.EXP0: AE_BAD_PARAMETER +pci0: failed to set ACPI power state D2 on \134_SB_.PCI0.EXP2: AE_BAD_PARAMETER +pci0: failed to set ACPI power state D2 on \134_SB_.PCI0.EXP3: AE_BAD_PARAMETER Ian
There is a commit referencing this PR, but it's still not closed and has been inactive for some time. Closing the PR as fixed but feel free to re-open it if the issue hasn't been completely resolved. Thanks