SM8550: deep suspend/resume, continued - #3126
Conversation
e91b67b to
62c6c60
Compare
|
nice I'll be testing today |
Yes, from reading the code I think they go together. Fair warning though, I did not write these. I took them from gh123man, and I have never hit the failure he describes in the patch comments. My dmesg has never shown that line either. The way I read it: the storage link gets put to sleep whenever the device is idle, and that runs all the time, not only around suspend. Without 1009, when that sleep fails the driver just marks the clocks on in software and gives up, so the link stays broken with nothing repairing it. 1011 only covers the suspend and resume window, which makes that collision rarer but does nothing for a failure during normal use. That would explain what you saw, but it is a guess and not a diagnosis. If you still have dmesg from that boot, a line starting What hardware did you test on? |
|
I could not test because the build failed |
62c6c60 to
722112a
Compare
|
Reworked the WiFi resume handling in this branch, because the version I pushed earlier was a guess and I finally measured the thing it was guessing at. The old code unblocked the radio 2 seconds after resume, on the assumption that NetworkManager would have processed the wake by then. It has not. NM reports STATE=asleep for about 7 seconds past the thaw, so the radio was coming back roughly 5 seconds too early on every single resume. That gap is the whole bug. iwd associates inside it, NM then wakes, does not recognise the association and tears it down ("deauthenticating by local choice"), and the chip is left where scans fail and association never starts again. Nothing recovers it: an rfkill cycle reloads the firmware and does not help, only a full driver reload or a reboot does. That is why it looked random. It only bites when iwd is quick enough to associate inside those 5 seconds. One boot here lost WiFi for 19 hours without me noticing, because I happened to be on a USB ethernet adapter at the time. So instead of a shorter or longer delay, wait for the actual condition. A new wifi-resume script polls until NM stops reporting asleep, then unblocks the radio and kicks the scan. It is bounded at 30s so a broken or missing NM degrades to the old behaviour rather than leaving the radio off forever. In practice the wait is 600 to 1200ms, because the hook starts it about 6 seconds into the resume already. Two smaller things fixed along the way. The scan kick was hardcoded to wlan0, which silently does nothing whenever the interface is not called that, and it is not after any driver reload since the name follows the wiphy index. And sleep.sh loses the timer plus the AccuracySec tuning that existed only to make that timer land precisely, so this change removes more than it adds. Testing, on an Odin 2:
I will keep running some automated test today, if no issues found, I think that fully resolve the WiFi issues. |
|
Went and tested this, since "a guess and not a diagnosis" was a bad place to leave it. The guess holds up. Two things now confirmed rather than read off the code. First, in 7.1.2 I also forced the failure with a test-only patch that drops the completion for one Link back in 52.8 ms, storage fine afterwards, and a later suspend and resume was clean. So yes, they go together. 1011 removes the collision inside the suspend window, 1009 is what makes the failure survivable when it happens anywhere else. 1011 alone leaves you with the failure and nothing to repair it, which fits the hang you saw. One caveat unchanged from before: I still have never seen this fire on its own, including with the hold disabled and the gate delay at 10 ms. So I have shown the mechanism is real and the recovery works, not that this hardware runs into it. Tested on an Odin 2, kernel 7.1.2. One caveat if you compare against other hardware: this chip puts storage into a lighter power state when it just goes idle, and a deeper one for system suspend. Because those two differ, 1011's hold runs on every suspend here. On a chip that uses the same state for both, the driver skips the hold entirely and 1011 does nothing at all. So if you try it somewhere else and see no difference, that might be why rather than the patch not mattering. |
__assign_str() lost its second parameter in 6.10; the string is taken from the matching __string() entry instead. Both vendored haptics patches still pass two, so the tracepoint header does not compile: include/trace/events/qcom_haptics.h:94:1: error: macro '__assign_str' passed 2 arguments, but takes just 1 This is invisible in the shipping config. CONFIG_FTRACE is off, so the TP_fast_assign body is never really compiled and the stale call is never checked. It breaks the build immediately for anyone who turns tracepoints on, which is how it surfaced: enabling them on SM8550 to investigate the UFS clock-gating behaviour in ROCKNIX#3126. SM8550 and SM8750 carry the same header and both need it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
722112a to
5582c16
Compare
|
Rebased onto current next. That picks up the haptics tracepoint fix from #3140. |
5582c16 to
02e4894
Compare
|
Reworked the docked power key so rocknix-fake-suspend is untouched by this branch. It exists for devices that cannot sleep, and running one that can through its state machine left two things owning the display. input_sense now looks for quirks/platforms/$HW_DEVICE/bin/power-handler and falls back to fake suspend when a platform does not ship one, so nothing changes for other devices. The SM8550 handler itself defers to fake suspend wherever 030-suspend_mode left suspend disabled, so unvalidated SM8550s are byte-identical too. Fixes that came out of it:
Tested on an Odin 2: docked toggle, the unplug-then-suspend sequence, and a plain battery suspend as the regression check. All good, resume around 3s, no UFS errors. If it occasionally takes a bit longer, that is the WiFi settling down. |
02e4894 to
60867dc
Compare
|
Force-pushed a big simplification: I removed the charger-attach wake, the GLINK wake plumbing that existed only to support it, and the never-suspend-on-AC inhibitor. I will update the description tomorrow to match. The numbers made this decision for me, so here they are. Undisturbed deep sleep on my Odin 2 measures 43-48 mA, which is the ~0.6%/h the description promises. But arming the ADSP GLINK edge so that plugging a charger can wake the device also lets the charger firmware's background chatter ring the SoC, and it does, roughly every 10 minutes of standby. Each of those wakes costs about a minute awake, and the sleep that follows one measures 400-520 mA, ten times the clean state and about the same as not sleeping at all. I spent two instrumented nights on this. With the wake armed, a six hour standby averaged 439 mA, which turns "roughly a week of standby" into under a day. I can show that cost precisely, but I cannot yet fully explain why the re-suspend after a chatter wake lands so hot, and I did not want to ship a headline feature that depends on a mystery. With the wake support gone the mystery cannot occur: chatter physically cannot wake the SoC and every suspend is the clean kind. The feature also never bought much. Charging works fine while suspended, the ADSP handles it with the AP asleep, verified through several nights of this testing. All the wake ever did was light the screen when the cable went in. The AC inhibitor went for the same reason. It existed so a plugged-in device never suspends, with the power key doubling as a screen toggle while docked. That is a logind inhibitor service, a udev rule and a per-platform power-key handler, all to take a choice away from the user. If you want to suspend while plugged in, that is your call, and it charges either way. So the suspend story is now one sentence: the power button suspends, the power button wakes, a charger charges. No inhibitors, no wake triage, no special cases. Every fix that fell out of this work stays: the ucsi unplug hang fix, the WiFi resume rework, the USB role toggle for fast resume, the LED handling. One new commit rides along with the push: it makes the UFS inline crypto engine actually bind. SCSI_UFS_CRYPTO was off, so the ICE node sat driverless forever and gcc never completed sync_state, meaning unused clocks were never released, a standing power floor found while chasing the numbers above. It also refreshes the QCE runtime PM patch from v6 to v9 to pick up an interconnect fix. I will re-measure standby over a full night on exactly this head and post the number here. |
87a3c33 to
3374d16
Compare
|
Overnight standby result on this head (3374d16), as promised. Suspended at 01:14 from a full charge, woke it at 09:17 with the power button. The journal shows exactly one suspend entry and one exit, nothing woke it in between, suspend_stats reads success=1 fail=0, and UFS came through with zero errors. That is the honest full-night number for the worst reasonable case: I suspended straight from the ES menu with a theme that runs video previews, battery fresh off the charger. For context, undisturbed daytime measurements on this same build have come in as low as 43-63 mA (~0.6 %/h, which is where the "roughly a week" in the description came from). So the real-world range today is 0.6 to 1.1 %/h, four days to a week, depending on what the device was doing at the moment you pressed the button. That spread is not sloppiness, it is how suspend works on this SoC, and it matches what the mainline Qualcomm suspend work has been finding on related chips. These platforms never enter their deepest sleep state (the aosd/cxsd/ddr counters in qcom_stats read zero on every mainline Snapdragon I could find data for, including the Linaro-backed laptops), so suspend power is really the sum of whatever votes and device states were left standing when the system went down. Suspend with a DSP session open or a bus vote held and the whole night runs at a different level; nothing in the sleep counters will tell you why. On top of that the fuel gauge integrates in ~0.8 mAh steps and only becomes trustworthy over multi-hour windows; short measurements produced physically impossible numbers for me, including a negative one. So treat any single standby figure, mine included, as a regime rather than a spec, and only compare numbers taken over hours. My prime suspect for the 43 vs 88 split is suspending while the theme video is mid-play, which leaves the audio path and DSP session open across the night. That is a one-evening test: same night, static screen instead. If it confirms, the fix is a small pre-suspend hook that releases the audio sinks, and it will come back with the before/after measured properly. I will continue testing and tracing this. |
3374d16 to
a9c255b
Compare
|
Force-pushed again, same 15 commits, five fixes folded into the commits that own them. Being transparent about the method: I ran a Claude-assisted review pass over the branch, feeding it the journals and measurement logs from this weekend's device testing plus the sibling trees (armada, pocknix, upstream lore) for cross-checking. It found real bugs that none of my earlier passes caught, all in code that had already survived my own testing, so I would rather disclose the tooling and ship the fixes than pretend I spotted these by eye. What changed:
Also checked the future of 0201, the 424-line UFS workaround. Linux 7.2 stops routing the storage interrupt through a helper thread (the revert, already in 7.2-rc), which removes the exact mechanism behind the suspend hang 0201 works around; upstream did it for performance and never saw our hang. Keep in mind 7.2 is not final yet, but when we bump to it, we will need to drop 0201. No functional changes beyond the above; the diff is otherwise identical to the previous push. |
a9c255b to
2651f84
Compare
|
Retracting the standby numbers from the 8th and the 9th. They were wrong, and wrong in the flattering direction. The real figure is about 4-5 %/h, so roughly a day from full. Not 0.6, not 1.1, not a week. I trusted the fuel gauge. On this device it under-reports and then resyncs the moment a charger goes in, once by 1.58 Ah. It read 90% at 3.87 V on a 4.4 V pack, and +3.7 A while discharging. One check kills the old figure on its own. Awake and idle this device draws 420-565 mA. At 0.6 %/h it would be under 50 mA asleep, meaning going to sleep buys a full order of magnitude. It does not; standby sits much closer to awake idle than that. Which is what One thing from the 8th survives. Dropping the charger-attach wake was still right, because the ADSP charger chatter really does ring the SoC roughly every ten minutes of standby, about a minute awake each time, and that is counted from suspend entry and exit records rather than the gauge. The "ten times a clean sleep" claim I attached to it is withdrawn. So this branch is worth judging on resume time and stability, not battery: resume in about 3 seconds instead of 14, no UFS errors, no WCN7850 wedge, no hang when the charger comes out while asleep. If anyone has a USB power meter, I would take your standby number over mine. Force-pushed with this. One new commit from @jaewun lets the idle DWC3 controller runtime suspend; it is a udev rule and I cannot show a power difference from it here, so treat it as parity with the sibling trees. The WiFi commit also picks up a race I hit today: suspend a few seconds after waking and the resume helper freezes with its systemd unit still active, so the next resume cannot start its own and silently leaves the radio off. It clears the stale one first now. Checked by hand on an Odin 2 across suspend, wake, re-suspend, wake, plus charger in and out while asleep and two cold boots. |
2651f84 to
39ecee0
Compare
|
Two commits dropped and one claim narrowed, all three out of extended testing and debugging sessions this week. Dropped the UFS inline crypto commit. I said binding the crypto engine would let the clock controller finish starting up and release unused clocks. The engine binds fine, but the clock controller still never finishes, because it is waiting on the GPU's That is a pre-existing SM8550 problem and nothing this branch can fix, so no clocks are ever released and no power is saved. Nothing on ROCKNIX uses inline encryption either: no device-mapper targets, and Dropped the DWC3 runtime suspend rule. It came from @jaewun's Thorch SM8550 tree, along with two others I picked up at the same time. I could not find it in any tree I am able to read, could not source the 151 mA to 107 mA measurement the commit message quotes, and could not test it here because the rule does nothing while a hub is plugged in. The other two from that batch both caused problems this week, one of them killing WiFi outright, so I am not carrying the third on trust. The WiFi fix closes one cause, not all of them. I have since seen the same WiFi death on a build carrying no suspend patches at all, on a device that had never slept. What this branch fixes is still solid: with the bad rail setting, WiFi died on the first suspend 3 times out of 3 and only a reboot brought it back. Without it, 3 of 3 clean, twice over. That leaves 14 commits, rebased on current next, and suspend still passes. By hand I have been through this many times over the past days, across successive builds: power button suspend and wake, back to back cycles, suspending again seconds after waking, longer sleeps, charger plugged and unplugged while the device is asleep, and cold boots. Waking averages about 3 seconds to a usable menu, with WiFi back around 10. Going to sleep can take longer than that, and deliberately so: the WiFi settle wait on the way down is what makes the resume reliable. I have not found an issue in any of it. I am going to keep testing for stability rather than call it done, and I will post anything that turns up. |
c93f6bd to
135d112
Compare
39ecee0 to
f607bed
Compare
|
Found a hole in the grace mute while living with this branch: wireplumber persists sink mute per route on disk, so a power loss while suspended drops the /run flag but keeps the mute, and the device boots silent with no way back from the UI. Pushed a boot quirk that clears any mute found on the internal speaker at startup; the grace hooks are the only sink-mute writers on this platform, so a mute at boot is stale by definition. Named sink rather than @DEFAULT_SINK@ because booting docked makes the display sink the default while the stale mute sits on the speaker. Also rebased onto current next. |
fcf6a4f to
5cf891e
Compare
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]
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.
ufshcd_gate_work() only restores the software gating state when DME_HIBER_ENTER times out; it does not repair the link. During system PM pm_op_in_progress suppresses the generic fatal error handler, so the controller is left operational in software but unable to complete device commands, and a following suspend can block in the filesystem sync after userspace has already blanked the display. That is the shape of the 54s "Filesystems sync" tiopex reported on the Thor. Recover the failed enter inline, using the out-of-band relink completion poller added by 0201, then pin runtime clock gating off for the rest of the boot so the same idle Hibern8 is not retried. System PM keeps working and can still transition the link for suspend. Ported from gh123man/armada-packages#1, written against this same series.
The failure-only diagnostics in the patch above captured the ordering behind that timeout: the gate worker had been admitted and had entered DME_HIBER_ENTER before system PM took its hold, so ufshcd_hold() changed the gating state under a running UIC command and the PM path then waited on that worker. Take a clock-gating hold in ufshcd_suspend_prepare() and release it from ufshcd_resume_complete(). That drains a pending gate worker while normal completion handling is still available, and stops a new idle Hibern8 starting inside the device suspend and resume callbacks. This is the root fix; the inline recovery stays as the storage-safety fallback. Ported from gh123man/armada-packages#1.
qcs8550-ayaneo-pocket-common.dtsi gives pwm-fan an edge-falling TLMM 64 interrupt for the tachometer. Measured on a Pocket EVO it fires about 80 times a second whenever the fan spins and repeatedly pulls the SoC back out of deep suspend, with /proc/interrupts climbing past 230,000; with the fan stopped the same suspend holds. The AYN Odin 3 declares no tacho interrupt and suspends for hours. Overridden in the EVO .dts rather than in the shared DTSI, which six other boards include and whose fan wiring has not been checked. Cost: hwmon fan1_input stops reporting RPM, as on the Odin 3. Found and measured by jesherman in armada-os/armada-packages#23. The EVO stays outside the device gate; this removes one of the two reasons it is there.
Left untouched, the dwc3/type-c stack renegotiates on resume and holds the dwc3 and ufshcd resume paths for around ten seconds each. Toggling the role switch just before suspend forces the type-c re-resolution to happen on the way down instead: measured on a Retroid Pocket 6 over ten cycles it cuts resume from 14.6s to 2.8s. TCPM/pmic_glink reverts the written value within about 300ms and fast resumes happen anyway: the forced re-resolution is the operative part, not the parked value. The pre hook verifies and re-asserts once, logging reverts to kmsg so any remaining slow wake names its cause, and the post hook restores the saved role. Lead from jaewun, measured and carried in Pocknix: shuuri-labs/pocknix-os@10191f03.
Carry the SM8750 ucsi fix for SM8550 as well: both run 7.1.x with the same pmic_glink UCSI model, so a charger-only partner never has its USB data role cleared, stays attached across suspend and keeps the USB power island up. Unplugging the cable while suspended then hard-hangs the SoC in the firmware power-collapse, and only a forced reset recovers. Same patch as SM8750's 0509, added there by ROCKNIX/distribution@f0703a0602 and device-tested on the AYN Odin 3.
Three failure modes, one subject: the WCN7850 surviving sleep.
Teardown: wifictl disable is an rfkill block that returns before the
driver has torn anything down, so whether ath12k reached its off
state before the freezer ran was a race that decided which path
resume took. Run it in the foreground and wait, bounded, for the
interface to go down.
Re-suspend: every resume reloads the WCN7850 firmware, and tearing
the chip down again mid-bring-up wedges it beyond anything but a
full power-off (6 of 6 in stress testing; a soft reboot does not
recover it because the rails never drop). The radio is already
blocked by the time platform hooks run, so the chip's state cannot
be probed from there; what proved reliable over twenty stress
cycles is a bounded hold: a suspend arriving within 15s of the
last resume waits out the remainder behind a blanked backlight, so
the press still looks instant. Audio is muted for the hold, only
when the hook actually changed the state, and the unmute clears its
flag only on success so a sound server still waking up gets retried
instead of leaving the device muted. The resume marker uses
CLOCK_MONOTONIC via /proc/timer_list; /proc/uptime would not do, it
advances while asleep.
Resume: the hook re-enabled the radio while NetworkManager still
believed the system asleep (NM learns of the wake only after the
hooks finish), iwd associated in that gap, and NM then tore the
fresh association down ("deauthenticating by local choice"),
stranding the chip in a scan-failure state until something kicked
NM's policy. Measured, NM stays asleep for about 7s past the thaw,
so no fixed delay closes this window. Wait for NM to leave the
asleep state before unblocking the radio, bounded at 30s so a
broken NM degrades to the old behaviour. If NM still reports
asleep at the timeout, a new suspend owns the radio: exit without
enabling rather than hand the freezer a chip mid-bring-up, and the
fixed unit name keeps a stale instance from ever stacking behind a
newer cycle. Then kick a scan once the firmware settles, since
iwd's first scans fail against a reloading chip and its retry
backoff wastes a minute more; the scan targets the interface that
exists, not wlan0, which is not its name after any driver reload.
Also skip unmatched globs in the quirk loop: a device without
sleep.d scripts executed the literal glob pattern and failed every
resume with exit status 127 in the journal.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Light the power LED solid blue for the whole suspend transition, including the grace hold, and again from the first moment of resume, so both edges are visible without the screen. The power LED is the PMIC tri-led on pm8550_pwm, declared with the same "power-led" label in qcs8550-ayn-common.dtsi and qcs8550-ayaneo-pocket-common.dtsi, so every gated device is covered. Devices without the label keep the previous behavior. The kernel side stays policy-free: 1016 turns the LEDs off in the suspend noirq phase, the last step before the platform sleeps, and restores them in resume noirq, before any other device. The blue survives the whole suspend entry, the sleep itself is dark, and the same blue is the first sign of life on wake, well before userspace thaws. The sleep hooks own the policy: the first sleep hook turns everything off and sets the power LED blue, and the last resume hook hands the LED back to its saved mode once the wake is complete. batteryledstatus is stopped across the window: it repaints the LED with charge colors and would override the indication. Nothing waits on the LED, so no suspend latency is added.
The suspend-grace pre hook mutes the sink and counts on the post hook to undo it, tracked by a flag in /run. wireplumber persists sink mute per route on disk, so losing power while suspended keeps the mute and drops the flag: the device then boots silent, and nothing in the UI can bring the audio back. The grace hooks are the only sink-mute writers on this platform, so a mute found on the internal speaker at boot is stale by definition. Clear it from a boot quirk. The sink registers a few seconds after wireplumber starts, hence the bounded background retry; the sink is addressed by name rather than @DEFAULT_SINK@ because booting docked makes the display sink the default while the stale mute still sits on the speaker.
7.2 removed the threaded UFS interrupt, which was 0201's whole premise: ufshcd_intr now services completions in hardirq, so the out-of-band relink drain is retired. It is kept as .disabled rather than deleted, since anyone back on 7.1.x still needs it. The rest of the series is regenerated against pristine 7.2 rather than fuzz-applied, after fuzz placed hunks in the wrong context twice: - 0203: enable_irq_wake moved out of probe into tsens_suspend_common / tsens_resume_common; the uplow skip now guards both sides so the wake refcount stays balanced. - 1009: the relink poller and hba->system_suspending are gone with 0201 and 7.2 respectively; recovery completes through the hardirq handler and the diagnostics use pm_op_in_progress only. - 1015: 7.2 grew its own POST_CHANGE case in link_startup_notify; the RX LineCfg disable merges into it instead of duplicating the label. - 1016: 7.2 gave struct lpg the same trailing members as struct lpg_led and the fuzzy hunk put the list node in the wrong struct; regenerated with unambiguous anchors.
Two patches from the WiFi suspend investigation: flush a pending firmware restart before the suspend path touches the device, and warn when the system sleeps with the device still up. The scan-priority fix that makes post-resume reconnects fast is not here; it stands on its own and is already proposed separately.
7.2 carries the mainline D3cold series natively, so dw_pcie_suspend_noirq now polls the LTSSM after the PME_Turn_Off broadcast on a path that did not exist on 7.1.2. On SM8550 that register is unreadable at that point and suspend wedges with no resume (confirmed on the AYN Thor, same silicon). Set skip_l23_ready so the DWC core uses the fixed delay, matching i.MX6.
Adopt the qcom-rpmh regulator suspend-state support from thorch: sleep set votes with matching wake set restores, mirrored so runtime RSC idle never sees a stale suspend vote. Board policy opts in exactly one rail: bob2, the gamepad MCU supply, off in suspend. Verified end to end with rpmh tracepoints (sleep TCS 0x40104=0, wake=1) and the pad re-attaches to a live hidapi session after the power cycle, mid-game included. The l15b LPM policy from the same thorch patch is deliberately not taken: l15b is the WiFi vddio rail here and LPM kills the ath12k data path until reboot. Also flip PCIe WAKE# to active-low per the PCIe CEM; active-high reads as permanently asserted so the WLAN endpoint could never signal wake.
logind ignores the power key; input_sense dispatches it to the platform power-handler when one exists, falling back to fake suspend elsewhere. The handler blanks instead of suspending while an external display is attached, releases the blank via the 000-blank-release pre hook so a real suspend never begins on a blanked panel, and gives instant press feedback: power LED blue and backlight off at press time, seconds before the sleep hooks would get there. Safe on an aborted suspend: resume re-powers the panel and the post hooks restore brightness.
The default sink is then the display's, wireplumber persists mute per route, and a sink muted here that is gone by resume stays muted on disk forever with nothing left to unmute it.
5cf891e to
38ae952
Compare
SM8550: deep suspend/resume, continued
Latest update on: 2026/08/27
Summary
What it means for a user:
That is the whole story: the power button suspends, the power button wakes, a charger charges. No inhibitors, no wake filtering, no special cases.
What's in the branch:
Testing
On the standby number specifically: earlier comments on this PR quote 0.6 to 1.1%/h and "roughly a week". Those came off this device's fuel gauge and they are wrong, so I am retracting them. The gauge under-reports over long windows and then resyncs when a charger is attached, by as much as 1.58 Ah in one case; it reported 90% at 3.87 V on a 4.4 V pack, and it reported +3.7 A while the device was discharging.
charge_counterandcapacityare the same quantity rescaled, so they never cross-check each other. Re-derived from voltage endpoints, every window I have collapses to the same answer, about a day. That is consistent withaosd,cxsdandddrinqcom_statsall reading zero on every cycle: the SoC rails never actually power down, so there is a real floor here that this branch does not remove. Suspend on SM8550 today buys you a reliable, fast, non-destructive sleep, not a week of standby.Additional Context
Earlier revisions of this branch also carried a charger-attach wake (plugging a charger woke the device) and a never-suspend-on-AC inhibitor with a docked power-key toggle. Both are gone. Arming the GLINK edge so a charger can wake the device also lets the ADSP charger firmware's background chatter ring the SoC, and the journal shows it doing exactly that, roughly every ten minutes of standby, each wake costing about a minute awake. That part is counted from suspend entry and exit records, so it stands. I originally also claimed the sleep following such a wake cost ten times a clean one; that came off the fuel gauge and I withdraw it. The wake frequency alone is reason enough not to ship the feature, and it bought little: charging works with the AP asleep, so all the wake ever did was light the screen when the cable went in.
The reason #2954 closed was maintenance, not code, so let me address that directly.
The honest weight of this branch is one patch. 0201 is 424 lines in ufshcd.c core, and it is the only place where a kernel bump can really hurt. Everything else is small, sits in drivers ROCKNIX already owns (rsinput, lpg, the quirks), or is plain shell. And 0201 has an expiry date in sight: Linux 7.2 (still in release candidates) reverts the threaded UFS interrupt handler, which removes the mechanism 0201 works around, so the 7.2 bump can likely just delete it. The revert will not be backported to 7.1.x, so it stays needed until then.
The "it will break on every bump" fear has also been tested for real now: Pocknix moved SM8550 to 7.1.5 last week and all 73 patches applied unchanged, no new fuzz.
If a bump does break suspend one day, nobody has to bisect storage patches under release pressure: flip the device gate off in 030-suspend_mode and ship. Users lose suspend until it's fixed and keep everything else.
One cheap hardening for the future: apply the UFS patches with --fuzz=0. By default, patch tolerates small context mismatches and guesses where a hunk belongs; after a kernel bump that guess can be wrong, and the result compiles fine while doing the wrong thing. With --fuzz=0 a mismatched hunk simply fails the build instead. That is exactly the failure mode called acceptable when #2954 was discussed: you see it, you fix or drop the patch, and nothing ever lands silently in the wrong place.
On the ongoing cost: this stack is no longer carried by one person. Armada and Pocknix both ship it and rebase it on their side, and fixes have been flowing between the trees in both directions all week. I've rebased this branch across every next change while building it and I'm happy to keep helping, but I may not always be available, and the good news is nobody has to depend on me: the maintenance is already shared across the projects that rely on these patches.
Credit where it's due: @jaewun wrote the original series and seeded two of the best fixes here, @shuuri-labs carried #2954 and kept improving the stack in Pocknix, @gh123man found the UFS root cause, @jesherman caught the fan tacho, Anze fixed the ucsi hang, and @tiopex, @dcu and @Cyfarw9dd put in the #2954 device time that surfaced half of these bugs in the first place.
AI Usage
Did you use AI tools to help write this code? Initially PARTIALLY but last few days I'm leveraging claude a lot to make this PR clean.
AI tools were used throughout the investigation (log correlation, wake source analysis, standby power measurement design, the sync_state find), for porting patches between trees with authorship preserved, and as a sanity check on this description, so it does not claim anything wrong or overstate what was tested. Ported kernel patches are carried verbatim under their original authors' names; the new hooks come from my own device findings and were validated by hand on hardware.
Update on: 2026/08/27, rebased onto kernel 7.2
What changed since the last update
0201 is gone. The last update called that 424-line ufshcd.c patch "the honest weight of this branch" and predicted 7.2 would let us delete it. That is what happened: 7.2 services UFS completions in hardirq, so the out-of-band relink drain has no premise left. It is kept as
.disabledrather than deleted, since anyone still on 7.1.x needs it. The maintenance argument that closed #2954 is now a much smaller argument.The rest of the series was regenerated against pristine 7.2 rather than fuzz-applied. That was not paranoia: fuzz put hunks in the wrong context twice, once inside the wrong struct in the LPG patch, which compiles fine and behaves subtly wrong.
0203,1009,1015and1016are regenerated with unambiguous anchors. Worth applying with--fuzz=0from here on, as discussed above.Four things are new:
0215). 7.2 carries the mainline D3cold series, sodw_pcie_suspend_noirqnow polls the LTSSM after the PME_Turn_Off broadcast on a path that did not exist on 7.1.2. On SM8550 that register is unreadable at that point and suspend wedges with no resume. Same behaviour on the Thor, same silicon. Setsskip_l23_readyso the DWC core uses the fixed delay, matching i.MX6.0218, adopted from thorch), plus a PCIe WAKE# polarity fix. Regulator sleep-set votes with matching wake-set restores. Board policy opts in exactly one rail:bob2, the gamepad MCU supply, off in suspend. Verified with rpmh tracepoints (sleep TCS0x40104=0, wake=1), and the pad re-attaches to a live hidapi session after the power cycle, mid-game included. Thel15bLPM policy from that same patch is deliberately not taken:l15bis the WiFi vddio rail here and LPM kills the ath12k data path until reboot. WAKE# was active-high in our DTSI, which reads as permanently asserted, so the WLAN endpoint could never signal wake; flipped to active-low per the PCIe CEM.input_sensenow dispatches to a per-platform handler where one exists and falls back to fake suspend everywhere else, so devices without one are byte-identical to today. It gives instant press feedback: power LED blue and backlight off at press time, seconds before the sleep hooks would get there.Removing the external display handling
The power handler can blank instead of suspending while an external display is attached, and the suspend-grace hook skips its mute in that case. Both ask a helper,
/usr/bin/external-display, that is not part of this series; it comes with the external display work in #3080.Both call sites are guarded and degrade to "no external display attached", so on its own this PR behaves exactly as it would on a device with no external output. Nothing here depends on #3080 landing.
If you would rather the external display handling not be in this PR at all, it is two places, both marked in-tree with the comment
External display dependency:quirks/platforms/SM8550/bin/power-handler: theexternal_display()wrapper and the two branches that call it.quirks/platforms/SM8550/sleep.d/pre/001-suspend-grace: reduce the condition to thepactltest alone.Say the word and I will strip both.
On deep sleep, since it came up before
Still no deep sleep.
aosd,cxsdandddrread zero on every cycle, so the standby floor this branch does not remove is still there and the roughly-a-day figure stands.What is new is that the last HLOS-side holder is now identified rather than suspected. With rpmh tracepoints the sleep TCS contains exactly one non-trivial vote:
MC0andSH0, DDR and LLCC, carrying the PCIe link's OPP bandwidth. Everything else in that set is zero. Regulators, clocks andsync_state, the GDSCs, the GMU, USB, the bandwidth monitors and the ADSP were each eliminated by measurement rather than by argument, and the interconnect keepalive flags turn out never to touch the sleep bucket.Removing that last vote does not work from here. Releasing the PCIe OPP at S2RAM also drops its
required-oppscorner, and since the link is deliberately left up the controller never comes back; that needs a forced power off. Halving it changes nothing, because a non-zero vote is a non-zero vote. A true zero means decoupling bandwidth from the power corner, which the OPP binding does not offer without a change in the OPP core. My read is that what remains is AOP or TZ policy we do not drive from Linux, so I am not chasing it further here.