kinx-project / kint

kinT keyboard controller (Kinesis controller replacement)
Other
318 stars 39 forks source link

Stapleberg could wake computer from sleep, but not new kinT41 #59

Open aaronjensen opened 2 years ago

aaronjensen commented 2 years ago

I just got a new KA2 w/ a kinT41 (w/ teensy 4.0). I have macOS and, for some reason, I cannot wake my laptop up from sleep using the kinT41. It worked fine with my old KA2 + stapleberg. This is my firmware: https://github.com/aaronjensen/qmk_firmware/tree/aaron/keyboards/kinesis/keymaps/aaronjensen which, aside from mapping, doesn't really have anything custom afaik.

Is this a common thing, or is there anything else that I should look at for this type of issue? Thanks!

CleanShot 2022-08-28 at 22 55 10@2x
stapelberg commented 2 years ago

I’m surprised to hear that it worked for you with the old controller (maybe not running QMK?).

When I tested it, I couldn’t get waking up to work at all with QMK, even with the older kint36. See bug report at https://github.com/qmk/qmk_firmware/issues/16934

aaronjensen commented 2 years ago

Yeah, it was running QMK. It woke fine for a couple years. Thanks for the link to the issue. Feel free to close this one since it’s tracked upstream.

afonsoguerra commented 2 years ago

Just to say that I also can’t wake the computer with the kint41, the trackpad or any other key works, but not the kint41. That said, it is not a problem with QMK per se, because I have a keyboardio 01 and Atreus, both running QMK and they both work for waking up my machine, so it might be a power state thing with the keyboard itself? I’m talking about ‘press-any-key’ wakeup and not specifically the keycode mentioned in the upstream issue.

aaronjensen commented 2 years ago

There must be specific sleep states where it can wake. This morning, it woke for me:

2022-08-29 09:05:52 -0400 Assertions            PID 327(powerd) Created UserIsActive "com.apple.powermanagement.kernel.useractive AppleUserHIDEventDriver:sleepDisplayTickle kIOMessageSystemWillS" 00:00:00  id:0x0x900009129 [System: DeclUser BGTask kCPU kDisp]
2022-08-29 09:05:52 -0400 Assertions            PID 385(WindowServer) Created UserIsActive "com.apple.iohideventsystem.queue.tickle serviceID:100001b0c name:AppleUserHIDEventSe product:kinT (kint41) eventType:3" 00:00:00  id:0x0x90000912a [System: DeclUser BGTask kCPU kDisp]
2022-08-29 09:05:52 -0400 Wake                  DarkWake to FullWake from Deep Idle [CDNVA] : due to UserActivity Assertion Using AC (Charge:100%)
2022-08-29 09:05:52 -0400 WakeDetails           DriverReason:NUB.SPMISw3IRQ - DriverDetails:
DriverReason:nub-spmi0.0x02 - DriverDetails:
DriverReason:rtc - DriverDetails:
2022-08-29 09:05:52 -0400 HibernateStats        hibmode=3 standbydelaylow=0 standbydelayhigh=0                                            11402
2022-08-29 09:05:52 -0400 WakeTime              WakeTime: 0.016 sec

I don't know much about sleep or how to interpret the above, but that's from pmset -g log on my macbook.

And this is not waking, after a shorter sleep, but I was able to wake it with my trackpad:

2022-08-29 09:25:13 -0400 Assertions            PID 385(WindowServer) Created UserIsActive "com.apple.iohideventsystem.queue.tickle serviceID:10000102c name:NULL product:Magic Trackpad eventType:17" 00:00:00  id:0x0x90000930d [System: PrevIdle PrevSleep DeclUser kCPU kDisp]
2022-08-29 09:25:14 -0400 Wake                  Wake from Deep Idle [CDNVA] : due to SMC.OutboxNotEmpty smc.70070000 wifibt bluetooth-pcie/UserActivity Assertion Using AC (Charge:100%) 1 secs
2022-08-29 09:25:14 -0400 WakeDetails           DriverReason:SMC.OutboxNotEmpty - DriverDetails:
DriverReason:smc.70070000 - DriverDetails:
DriverReason:wifibt - DriverDetails:
DriverReason:bluetooth-pcie - DriverDetails:
2022-08-29 09:25:14 -0400 HibernateStats        hibmode=3 standbydelaylow=0 standbydelayhigh=0                                            11404
2022-08-29 09:25:14 -0400 WakeTime              WakeTime: 1.648 sec
stapelberg commented 1 year ago

2022-08-29 09:05:52 -0400 Wake DarkWake to FullWake from Deep Idle [CDNVA] : due to UserActivity Assertion Using AC (Charge:100%)

I suspect that when Apple decides to wake up your computer (“dark wake”) during the night, it no longer keeps the USB devices in standby state, but actually enumerates them and then listens to input.


I have since found out how to use any-key-to-wakeup on my lab PC (Intel i9-9900K on an ASUS PRIME Z370-A) using Linux:

Ensure wakeup is enabled for the USB device that is your keyboard (see lsusb):

echo enabled | sudo tee /sys/bus/usb/devices/1-10/power/wakeup

Then, suspend to RAM (e.g. echo mem | sudo tee /sys/power/state).

When pressing any key on the keyboard (not necessarily the XF86WakeUp key), the machine wakes up from suspend-to-RAM.

This behavior works on:

But it does not work on the kint41 (and apparently kint40, which you are using).

The next step in debugging this is to compare the debug log of the different USB stacks to see if anything stands out.

stapelberg commented 1 year ago

The good news is that the following changes to lib/chibios-contrib make waking a PC from suspend-to-ram work on the kint41.

The bad news is that the keyboard is then stuck and won’t work until un-plugged and re-plugged. Perhaps that’s a QMK-specific problem, so the next step in debugging is to reproduce the issue in a ChibiOS demo, or perhaps even an NXP demo.

We should also verify that the USB device descriptor indicates remote wakeup support, which might influence the value of the power/wakeup /sys file.

diff --git c/os/hal/ports/MIMXRT1062/LLD/USBHSv1/hal_usb_lld.c i/os/hal/ports/MIMXRT1062/LLD/USBHSv1/hal_usb_lld.c
index e4752c18..d143a66c 100644
--- c/os/hal/ports/MIMXRT1062/LLD/USBHSv1/hal_usb_lld.c
+++ i/os/hal/ports/MIMXRT1062/LLD/USBHSv1/hal_usb_lld.c
@@ -116,12 +116,16 @@ static usb_status_t usb_device_callback(usb_device_handle handle, uint32_t callb
     break;

   case kUSB_DeviceEventSuspend:
-    printf_debug("  suspend");
+    printf_debug("  suspend--nxp");
+    // Call USB_DeviceSetStatus() to enable the “detect resume” interrupt.
+    usb_device_struct_t *dev_handle = (usb_device_struct_t *)handle;
+    (void)USB_DeviceSetStatus(dev_handle, kUSB_DeviceStatusBusSuspend, NULL);
+    printf_debug("  suspend--chibi");
     _usb_suspend(usbp);
     break;

   case kUSB_DeviceEventResume:
-    printf_debug("  resume");
+    printf_debug("  resume--chibi");
     _usb_wakeup(usbp);
     break;
   }
diff --git c/os/hal/ports/MIMXRT1062/LLD/USBHSv1/hal_usb_lld.h i/os/hal/ports/MIMXRT1062/LLD/USBHSv1/hal_usb_lld.h
index 764fbc60..655f070c 100644
--- c/os/hal/ports/MIMXRT1062/LLD/USBHSv1/hal_usb_lld.h
+++ i/os/hal/ports/MIMXRT1062/LLD/USBHSv1/hal_usb_lld.h
@@ -408,8 +408,10 @@ struct USBDriver {
  * @notapi
  */
 #define usb_lld_wakeup_host(usbp)                                     \
-  do{                                                                 \
-  } while (false)
+  do{ \
+    usb_device_struct_t *dev_handle = (usb_device_struct_t *)handle; \
+    (void)USB_DeviceSetStatus(dev_handle, kUSB_DeviceStatusBusResume, NULL); \
+  } while (0)

 /*===========================================================================*/
diff --git c/os/hal/ports/MIMXRT1062/LLD/USBHSv1/usb_device_config.h i/os/hal/ports/MIMXRT1062/LLD/USBHSv1/usb_device_config.h
index c00e7bff..a3968d9e 100644
--- c/os/hal/ports/MIMXRT1062/LLD/USBHSv1/usb_device_config.h
+++ i/os/hal/ports/MIMXRT1062/LLD/USBHSv1/usb_device_config.h
@@ -154,11 +154,11 @@
 #define USB_DEVICE_CONFIG_BUFFER_PROPERTY_CACHEABLE (0U)
 #endif
 /*! @brief Whether the low power mode is enabled or not. */
-#define USB_DEVICE_CONFIG_LOW_POWER_MODE (0U)
+#define USB_DEVICE_CONFIG_LOW_POWER_MODE (1U)

 #if ((defined(USB_DEVICE_CONFIG_LOW_POWER_MODE)) && (USB_DEVICE_CONFIG_LOW_POWER_MODE > 0U))
 /*! @brief Whether device remote wakeup supported. 1U supported, 0U not supported */
-#define USB_DEVICE_CONFIG_REMOTE_WAKEUP (0U)
+#define USB_DEVICE_CONFIG_REMOTE_WAKEUP (1U)

 /*! @brief Whether LPM is supported. 1U supported, 0U not supported */
 #define USB_DEVICE_CONFIG_LPM_L1 (0U)

Edit: resources for debugging:

stapelberg commented 1 year ago

The bad news is that the keyboard is then stuck and won’t work until un-plugged and re-plugged. Perhaps that’s a QMK-specific problem, so the next step in debugging is to reproduce the issue in a ChibiOS demo, or perhaps even an NXP demo.

I tested whether older kint controllers work after waking up:

So perhaps it’s a problem with ChibiOS and/or the kint36/kint41 USB HAL. The kint2pp uses LUFA, which is a different stack.

stapelberg commented 1 year ago

I think I got it working!

We need two changes in lib/chibios-contrib.

Fix 1: the NXP USB stack has its own hwTick variable, but we don’t have a SysTick ISR handler from which to feed that hwTick variable, so the NXP USB stack hangs after triggering a remote wakeup. This change works around the hang for now:

--- i/ext/nxp-middleware-usb/device/usb_device_ehci.c
+++ w/ext/nxp-middleware-usb/device/usb_device_ehci.c
@@ -1696,8 +1696,8 @@ usb_status_t USB_DeviceEhciControl(usb_device_controller_handle ehciHandle, usb_
 #endif
             ehciState->registerBase->PORTSC1 &= ~USBHS_PORTSC1_PHCD_MASK;
             ehciState->registerBase->PORTSC1 |= USBHS_PORTSC1_FPR_MASK;
-            startTick = deviceHandle->hwTick;
-            while ((deviceHandle->hwTick - startTick) < 10U)
+            startTick = DWT->CYCCNT;
+            while ((DWT->CYCCNT - startTick) < 10U)
             {
                 __NOP();
             }

Fix 2: we need to skip restarting the USB driver on remote wakeup:

--- i/os/hal/boards/PJRC_TEENSY_4_1/board.c
+++ w/os/hal/boards/PJRC_TEENSY_4_1/board.c
@@ -45,3 +45,7 @@ void __late_init(void) {
  */
 void boardInit(void) {
 }
+
+void restart_usb_driver(USBDriver *usbp) {
+    // Do nothing. Restarting the USB driver on these boards breaks it.
+}

I think that’s all we need, but I have only briefly tested these changes so far.

Will try and clean up these commits into a proper pull request over the next few days.

stapelberg commented 1 year ago

Okay, I have sent https://github.com/ChibiOS/ChibiOS-Contrib/pull/345 (for the kint41) and https://github.com/ChibiOS/ChibiOS-Contrib/pull/346 (for the kint36).

With these pull requests applied, all kinT controllers (tested with kint2pp, kint36, kint41) should do USB remote wakeup (if enabled in the OS) and work after wakeup.

I’ll keep this issue open for visibility for a while (it will be a while until the PRs land in ChibiOS-Contrib, and another delay until they make their way to QMK develop, and then QMK master…).

aaronjensen commented 1 year ago

That’s great, thanks.

aleb commented 1 year ago

Is this an issue with Macs only? I can wake up my Linux from sleep just fine with a KB133+kintlc.

stapelberg commented 1 year ago

No, the issue is with specific USB stacks, specifically the NXP stack used on the kint41 and the ChibiOS driver used on the kint36.

danieldk commented 1 year ago

Is this an issue with Macs only? I can wake up my Linux from sleep just fine with a KB133+kintlc.

Interesting, on Mac (KB600 + KinT + LC) I cannot wake the machine using KC_WAKE. After waking the machine with my trackball, the keyboard is dead and I have to replug it.

stapelberg commented 1 year ago

My current recommendation is to switch to kint36 if you need to wake your machine with your keyboard. It’s been working reliably for me with the kint36.

danieldk commented 1 year ago

I disabled restarting of the USB stack like you did for kint36 and the last few suspend-wake cycles have worked.

https://github.com/danieldk/qmk_firmware/commit/56da1c36b849af9598ecac83e6123e6af57875cc

I'll report back after a few days.

danieldk commented 1 year ago

Since I made this change, resume has worked (with any key) and the keyboard always works without replugging (with a Teensy LC).