SM8550 deep suspend/resume - #2954
Conversation
|
awesome! I can help testing if you provide steps to test |
|
Thanks for your work! Already testing on an Odin2 portal — the suspend patch works so far, going to run a few more tests (repeated suspend/resume, wake stability etc). |
|
Works really well for me as well RP6. just once after resuming the device did not recognize the input but after sleeping and resuming again it worked. I haven't been able to reproduce it ever since. EDIT: I lost input recognition again, after suspending and resuming again it's restored. there must be some kind of race condition |
|
@shuuri-labs can you rebase your branch on origin/next? |
7921026 to
68626b6
Compare
aa0946e to
8752710
Compare
|
I'm pretty sure we can drop the ICE patches to simplify things here, they were introduced early on in testing as a correctness patch but in retrospect they dont appear load bearing. I also ran into the controller issues mentioned above and it seems like the controller continues sending interrupts, and storms a whole lot of interupts during wake and the irq eventually gets disabled as "nobody cared". Might be able to replicate by pressing buttons during suspend/wake? Will dig up my old notes on that when I get home next week. |
8752710 to
af78668
Compare
|
@jaewun sweet, thanks for letting me know. I've dropped the ICE set from the branch (the crypto configs went with it, so this PR no longer touches the kernel config at all). As for the controller - I originally carried your MCU suspend/resume + geni UART mask patches and that's mostly what I've tested, so it makes sense that I haven't really seen this issue yet. I removed them when rebasing off your clean set, so at some point I can run the buttons-during-suspend repro on the RP6 and report what pm_wakeup_irq and the irq counters show with the new set. Depending on the results, we can either re-add your older patches or something more targeted (from your your notes) EDIT: actually, since @dcu reported facing the controller issue before my rebase, the now dropped patches probably didn't help. I'll do my investigation anyway, but probably we'll just wait for your notes for a fix :) |
I have not seen those issues again though, not sure what changed or it is just pure luck |
af78668 to
c0a0434
Compare
|
Tested on the Ayaneo Pocket EVO, installed on internal storage. It works in ES, but it still wedges fairly often when running Steam I tested also on Ayaneo pocket DS installed on SD card. Sometimes it cannot enter into sleep with log No wedges here, but sometimes I have to push power button few times to wakeup the device Tomorrow I'll check on DS installed on internal storage and on AYN thor |
|
After installing to internal on Pocket DS also wedges when running steam, similar to pocket EVO |
|
@tiopex cool, thanks for checking. waiting on the Thor test. If that goes OK, then perhaps this should be gated to Odin 2 family/RP6/Thor for now, as the patches have been validated on all of these devices. |
|
tested on AYN Thor on SD card. Sometimes after wakeup buttons doesnt work and such error is visible in dmesg |
Add the UFS core, qcom host, and QMP PHY patches needed for reliable suspend/resume relink on the SM8550 UFS host. The set covers non-MCQ PM completion draining, host-reset IRQ depth balance, hibern8 error propagation, auto-hibern8 versus clk-gating policy, no-retention PHY handling, and RX LineCfg control around link startup. [shuuri-labs: rebased 0201 hunk 9 for 7.1.2, ufshcd_intr() gained the active_uic_cmd thread-handoff condition upstream]
Add the TSENS patch used by the SM8550 suspend path. The TSENS patch leaves AYN Thor uplow threshold IRQs as non-wakeup. Critical thermal wake remains enabled. [shuuri-labs: split the IPCC patch into its own commit to restore its original authorship; it was first added in an earlier revision of this PR]
The ADSP charger firmware pushes an unsolicited BATTMGR_NOTIFICATION (opcode 0x7) about 0.5s after suspend entry. It rings the IPCC mailbox irq, which upstream is IRQF_NO_SUSPEND, so it is never masked and wakes the device every few minutes. Drop IRQF_NO_SUSPEND so the mailbox irq is masked across system suspend. Lifted from ROCKNIX SM8750 (AYN Odin 3), which shares the battmgr/pmic_glink charger model and sleeps cleanly.
Default SM8550 devices to mem suspend and wire power, suspend, and lid handling through logind. Also set the UFS command timeout and deep sleep default on the SM8550 kernel command line.
jaewun's patch scoped the of_machine_is_compatible() check to ayn,thor. RP6's root compatible is "retroidpocket,rp6", "qcom,qcs8550", "qcom,sm8550" with no ayn,thor entry, so the fix silently did nothing there. Broaden the check to the SoC-level compatible so it covers every SM8550 board.
Deep suspend is validated on the Retroid Pocket 6, the AYN Odin 2 family, and the AYN Thor. AYANEO Pocket EVO and DS reports show Steam-load wedges and xhci -16 suspend-entry failures, so unlisted devices keep the previous behavior (suspend off, power key ignored) until those are resolved. Gate on the DT model via QUIRK_DEVICE.
Combine the rsinput resume-reinit and suspend-quiesce into one PM-ops change. The MCU streams over the UART; left running across suspend teardown it storms the geni RX IRQ and trips the spurious-IRQ disable, leaving the gamepad dead after resume. Quiesce on suspend, re-init on resume.
The gamepad MCU UART storms its geni irq during suspend, tripping a spurious-disable that wedges the next suspend. Mask the non-console geni irq over the PM transition.
c0a0434 to
aa7d832
Compare
|
@tiopex I've gated deep suspend is to validated devices: RP6, Odin 2 family, and Thor. On the Ayaneo Pocket EVO and DS your reports show separate problems (the Steam wedges, and the xhci -16 refusing to enter sleep on the DS), so those devices keep suspend disabled for now. They can be added back once those issues get their own investigation. As for the Thor button dropot: the trace shows the gamepad MCU still streaming into its UART while the kernel is suspending that port. There is a window in the teardown where the interrupt fires but the driver no longer services it, so the line refires endlessly until the kernel's spurious-IRQ protection gives up and disables it permanently (irq 190: nobody cared in your log). Nothing re-enables that line on resume, which is why buttons stay dead until reboot. This is the exact failure @jaewun originally wrote two patches for, so I've restored them both: one quiesces the MCU before the UART suspends, the other masks the geni irq across the suspend transition so the storm can't trip the detector. If you get a chance to retest on Thor (ideally pressing buttons around suspend entry, which widens the race window), that would confirm it. Thank you for testing + providing logs :) |
|
Checked the latest. Buttons works fine now, but suspend sometimes takes long time to enter into sleep. It is visible also in log 54s for sync filesystem before entering into sleep. Another issue is that sometimes need to push PWR button few times to wakeup the device |
|
@shuuri-labs After testing this PR on Odin2 Portal: the kernel-side changes are applied correctly. I'm still hitting a Steam Big Picture freeze on suspend/resume, but preliminary analysis suggests this is unrelated to this PR — on my device Note: can't resume from deep suspend by controller button, only can wake up by power button. |
|
can this be merged for now? whatever bugs there are this is better than the alternative |
|
Tested the whole series on an AYN Odin 2. Built from tag 20260801 with this branch merged in, nothing else on top. 13 deep suspend cycles: 3 x 30s, 3 x 120s, 1 x 900s, 1 x 60s, 2 x 120s for a WoWLAN comparison, 3 by power key. 12 clean, 1 abort I caused myself. All 13 entered deep, no s2idle fallthrough, Sync and the suspend path are quick, no sign of the slow sync reported earlier in the thread: Entry to exit was 2.2s to 6.1s. 1004 and 1006: the MCU is re-initialised on every resume and the pad keeps working. A couple of WiFi reloads firmware on every resume ( One failed suspend, self-inflicted: I pressed power again before the resume had finished. One consequence on my side: with real suspend working, my external display PR (#3080) will need updating, since it currently leans on the fake-suspend path. It has to be reworked on top of this series and tested against it. I plan to do that. In the meantime I am tracking battery consumption over the next 10 hours and will report back. |
|
Battery numbers from the same build, AYN Odin 2 left alone overnight on battery. One suspend, 9h 47m in deep sleep, nothing woke it in between ( It really was down rather than idling awake: so 10h 25m of the 10h 46m since boot was spent suspended, with about 20 minutes awake in total including all of last night's testing. Resume after ten hours was clean: wlan0 back up, gamepad still enumerated, and since boot |
|
Retested on my Odin 2 against current I did run into something while stress testing though, and since it's only reachable now that real suspend works, it seems worth writing up. WiFi sometimes doesn't come back, and then you're stuckEvery so often after a resume, the WCN7850 reloads its firmware like normal and then just refuses to scan, forever:
The annoying part isn't that it happens, it's that you can't get out of it. I scripted the recovery attempts so they'd be consistent: 40s of waiting, then That last detail is what sent me looking at the driver, and it makes sense once you do. Worth saying explicitly because it was my first instinct too: adding a "count consecutive scan failures and kick What triggers itShort suspends, as far as I can tell:
I want to be upfront that this isn't nailed down. The long run is only n=3, and I did all the short ones back to back, so run order and duration are tangled together. Fisher exact on the pooled numbers comes out around p=0.08, so it's suggestive and no more. I'm doing a finer interleaved sweep that logs the actual off-time rather than the requested one, and I'll follow up with that. Things I checked and it isn'tI chased a couple of wrong ideas first, so to save anyone repeating them:
Which leaves the chip's actual physical off-time as about the only thing left that scales with how long you slept. On that note, How much does this matterHonestly, probably not much day to day. Nobody suspends for 20 seconds on purpose, so what I'm doing is closer to a stress test than real use. But when it does happen there's no WiFi until you reboot, and the obvious way a handheld would stumble into it is an idle timer that suspends again a few seconds after a wake. Two things that might be worth doing on the ROCKNIX sideNeither needs a kernel change:
Happy to send those as their own PR if you'd rather keep this one focused. Either way none of it blocks #2954 from where I'm sitting, deep suspend and UFS and the gamepad have been solid through every cycle I've thrown at it. |
|
Following from the maintenance discussion on Discord: I think loki666b's concern is fair, and I don't have the bandwidth to keep carrying this against kernel bumps, so I'm closing this PR. The full stack lives on downstream in Pocknix, where all SM8550 devices have been running it for a while; anyone who wants to pick it up is welcome to everything here. I've split out the one piece I do think belongs upstream: #3115, the IPCC wake mask - a small patch that's already applied to the SM8750. Huge thanks to @jaewun for your wisdom and patches, and to @tiopex, @dcu, @Cyfarw9dd and @xiaodoudou for the device time :) |
|
thanks to you |
|
there's also ongoing work here |
Summary
This PR enables the
deepsuspend path (S2RAM) for SM8550, tested on the Retroid Pocket 6.It is @jaewun's #2952 series, cherry-picked with authorship intact and rebased onto current
next(7.1.2). Rebase adjustments, noted in the commit messages:ufshcd_intr()gained theactive_uic_cmdthread-handoff condition upstreamSo this is the same suspend content as #2952, in a form that merges cleanly on
nexttoday.Two commits are mine:
BATTMGR_NOTIFICATION(opcode 0x7) about 0.5s after suspend entry, and withIRQF_NO_SUSPENDthe mailbox irq is never masked, so the device wakes itself every few minutes. Lifted from ROCKNIX's SM8750 patch, which shares the battmgr/pmic_glink charger model.ayn,thortoqcom,sm8550. The RP6's compatible list has noayn,thorentry, so the Thor-only check silently does nothing there and the TSENS threshold IRQs keep waking the device. The SoC-level compatible covers every SM8550 board, Thor included. Critical thermal wake stays enabled.s2idlestays disabled (mem_sleep_default=deepplusSuspendStatepinned tomem): a failed deep attempt that falls through to s2idle has wedged SM8550 devices in testing, so a failed deep should no-op instead.Compared to the previous revision of this PR: the old patch lineage is replaced by the #2952 versions (which add the mphy hibern8-park and RX LineCfg fixes), and the rsinput MCU suspend/resume and geni UART patches are dropped to match #2952. Given the controller irq-storm behaviour jaewun describes in the comments, those three are the likely re-add candidates once we have his notes and an RP6 repro; keeping them out for now so the fix can be evidence-based.
Testing
Built and tested on the Retroid Pocket 6: repeated RTC-woken
deepsuspend/resume cycles, including longer unattended runs. Wake sources checked via/sys/power/pm_wakeup_irq(withPM_SLEEP_DEBUGenabled in my local builds) and/sys/kernel/debug/wakeup_sources, real time-in-suspend confirmed viaCLOCK_BOOTTIMEvsCLOCK_MONOTONICdrift.Note: that testing was against the previous revision of this branch. The kernel patches here are byte-identical to what I build daily against stock 7.1.2, but a full rebuild and retest of this exact set on the RP6 is still to come, including the buttons-during-suspend controller repro. I'll report back here.
Thor coverage is jaewun's testing in #2952.
Test results
Deep suspend resumes reliably on the tested RP6 setup with no self-wakes. Before the IPCC fix the device could not stay asleep past 5 to 10 minutes; with it, clean ~14 minute RTC-bounded suspends with matching boottime drift. UFS recovers cleanly after resume. One UFS host reset per deep resume is expected on this platform because system suspend uses LINK_OFF.
Additional context
This PR and #2952 are the same series. The difference is the rebase onto current
next, the ICE drop, and the two RP6-facing commits. Happy to fold this into #2952 instead if that's preferred.AI Usage
Did you use AI tools to help write this code? Yes.
AI tools were used during the investigation and packaging: correlating wakeup-source and suspend-stat logs to pin the wake to the opcode 0x7 IPCC push, comparing candidate patches across the SM8550 and SM8650 trees, and assembling this branch. The IPCC fix itself is lifted verbatim from ROCKNIX's SM8750 patch, and the final selection was validated by hardware testing on the RP6.