Re: [PATCH 0/3] Allow specifying an S2RAM sleep on pre-SYSTEM_SUSPEND PSCI impls

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 11/12/24 19:43, Manivannan Sadhasivam wrote:
On Tue, Nov 12, 2024 at 07:32:36PM +0100, Konrad Dybcio wrote:


On 11/12/24 19:01, Manivannan Sadhasivam wrote:
On Mon, Oct 28, 2024 at 03:22:56PM +0100, Konrad Dybcio wrote:
Certain firmwares expose exactly what PSCI_SYSTEM_SUSPEND does through
CPU_SUSPEND instead. Inform Linux about that.
Please see the commit messages for a more detailed explanation.


It is still not PSCI_SYSTEM_SUSPEND though...

It *literally* does the same thing on devices where it's exposed.


But still...

Still-what? We can't replace the signed firmware on (unironically) tens
of millions of devices in the wild and this is how it exposes that sleep
state. This is how arm platforms did it before the PSCI spec was
updated and SYSTEM_SUSPEND is *still optional today*.


This is effectively a more educated follow-up to [1].

The ultimate goal is to stop making Linux think that certain states
only concern cores/clusters, and consequently setting
pm_set_suspend/resume_via_firmware(), so that client drivers (such as
NVMe, see related discussion over at [2]) can make informed decisions
about assuming the power state of the device they govern.

If this series gets green light, I'll push a follow-up one that wires
up said sleep state on Qualcomm SoCs across the board.


Sorry. I don't think PSCI is the right place for this. Qcom SoCs have a common
firmware across all segments (mostly),

This ^

so there is no S2R involved and only S2Idle.

is not at all related to this ^, the "so" makes no sense.

(also you're wrong, this *is* S2RAM)


What? Qcom SoCs supporting S2R? I'm unheard of.

Maybe you're thinking of hibernation, which is not widely (if at all)
supported.


If you use PSCI to implement suspend_via_firmware(), then all the SoCs
making use of the PSCI implementation will have the same behavior. I don't think
we would want that.

This is an issue with the NVMe framework that is totally unrelated to this
change, see below. Also, the code only sets that on targets where such state
exists and is described.


Well, you are doing it just because you want the NVMe device to learn about the
platform requirement.

And I can't see why you're having a problem with this. It's exactly how it
works on x86 too. Modern Standby also shuts down storage on Windows,
regardless of the CPU architecture.
For instance, if a Qcom SoC is used in an android tablet with the same firmware,
then this would allow the NVMe device to be turned off during system suspend all
the time when user presses the lock button. And this will cause NVMe device to
wear out faster. The said approach will work fine for non-android usecases
though.

The NVMe framework doesn't make a distinction between "phone screen off" and
"laptop lid closed & thrown in a bag" on *any* platform. The usecase you're
describing is not supported as of today since nobody *actually* has NVMe on a
phone that also happens to run upstream Linux.
I'm not going to solve imaginary problems.


Not just phone, NVMe device could be running on an android tablet.

'Could' very much makes it imaginary. There are no supported devices that
fall into this category.

I'm not
talking about an imaginary problem, but a real problem that is in a forseeable
future

Keyword: future. This issue has been on hold for years because of 'issues'
that are pinky promised to happen eventually, without anyone suggesting any
actually acceptable solutions. This just undermines progress.

(that is also the reason why NVMe developers doesn't want to put the
device into power down mode always during system suspend).

This is the current behavior on any new x86 laptop, and has been for a
couple of years.

And with this change, you are just going to make the NVMe lifetime miserable on
those platforms.

Fearmongering and hearsay. See above.

Konrad




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux