On 20.12.2024 12:39 PM, Sudeep Holla wrote: > On Thu, Dec 19, 2024 at 08:26:51PM +0100, Konrad Dybcio wrote: >> On 14.11.2024 2:10 AM, Elliot Berman wrote: >> >>> I'm not sure why you'd like to support s2ram. Is it *only* that you'd >>> like to be able to set pm_set_supend/resume_via_firmware()? I hope this >>> doesn't sound silly: what if you register a platform_s2idle_ops for the >>> relevant SoCs which calls pm_set_suspend/resume_via_firwmare()? >> >> S2RAM is what you get after entering a certain state, but currently >> it's presented as just another (s2idle) idle state. >> > > Just to be clear, I assume you mean CPU_SUSPEND idle state. There is > no special or different s2idle idle states IIUC. Yeah, right. >> That means some hardware that may need to be reinitialized, isn't as >> Linux has no clue it might have lost power. >> > > Interesting, so this means firmware doesn't automatically save and restore > states yet exposes it as CPU_SUSPEND idle state. Reading the spec, I'm pretty sure PSCI calls should only mess with the power state of the cores, core-adjacent peripherals and GIC. Reading section 5.20.1 (SYSTEM_SUSPEND / Intended use) I think it says mostly what I'm trying to convey: "In a typical implementation, the semantics are equivalent to a CPU_SUSPEND to the deepest low-power state. However, it is possible that an implementation might reserve a deeper state for SYSTEM_SUSPEND than those used with CPU_SUSPEND." - this is the situation on QC platforms, with the case of not reserving a deeper state for SYSTEM_SUSPEND >> One of such cases is the PCIe block, with storage drivers specifically >> looking for pm_suspend_via_firmware, but that's unfortunately not the >> whole list. >> > > Well I can now imagine and I understand what's wrong here. An idle state > is exposed to OS with an expectation that OS saves and restores certain > state. Unless you tie it some other power domains that theses devices > share, it is hard for OS to know the state is being lost and it needs > to save and restore them. It is simple wrong to assume that OS needs > to take care of them even though the power domain hierarchy doesn't > represent this dependency to enter such a state. cpuidle-psci-domain.c > takes care of this IIUC. Ulf can provide details if you are interested. The spec disagrees: "Note that entering the system into S2 or S3 carries with it several preconditions. For example, all devices in the system must be in a state that is compatible with entry into the system state" - this also happens to be relevant here, given PSCI is not supposed to power-govern the entire SoC, but only the CPU block. We have specialty hardware that does power management for non-CPU IPs, but to request a system power rail disablement, it must be done in conjunction with the CPU requesting such CPU_SUSPEND state. And only after the required hardware is de-initialized. Konrad
