On Tue, Jan 11, 2022 at 5:23 PM Limonciello, Mario <mario.limonciello@xxxxxxx> wrote: > > +Alex > > On 1/11/2022 09:52, Rafael J. Wysocki wrote: > > On Wed, Jan 5, 2022 at 8:39 PM Mario Limonciello > > <mario.limonciello@xxxxxxx> wrote: > >> > >> Currently the Linux kernel will offer s2idle regardless of whether the FADT > >> indicates the system should use or on X86 if the LPS0 ACPI device has been > >> activated. > >> > >> On some non-AMD platforms s2idle can be offered even without proper > >> firmware support. The power consumption may be higher in these instances > >> but the system otherwise properly suspends and resumes. > > > > Well, the idea is that s2idle should not require FW support at all. > > > May I ask - why? It's an intentional design decision? Yes, it is. > > It may not be possible to reach the minimum power level of the > > platform without FW support, but that should not prevent s2idle from > > being used. > > > >> On AMD platforms however when the FW has been configured not to offer > >> s2idle some different hardware initialization has occurred such that the > >> system won't properly resume. > > > > That's rather unfortunate. > > > > Can you please share some details on what's going on in those cases? > > > > Technically, without FW support there should be no difference between > > the platform state reachable via s2idle and the platform state > > reachable via runtime idle. > > During resume there is a number of page faults that occur and during > initialization the ring tests fail. The graphics is unusable at this > time as a result. > > The amdgpu code actually *does* distinguish between the 3 different > cases of S3, S0ix, and runtime suspend. But s2idle doesn't guarantee S0ix in any case. > The function "amdgpu_acpi_is_s0ix_active" causes different codepaths to > be used during the suspend routine. Well, as I said, s2idle need not mean S0ix. > In this particular case that FADT doesn't set the low power idle bit > and that function returns false meaning the s3 codepath is taken but > the hardware didn't go through a reset. If there is a separate S3 code path, taking it when pm_suspend_target_state == PM_SUSPEND_TO_IDLE is incorrect. > It *might* also be possible to solve this by mandating an ASIC reset in > such a case (we didn't try). I'd rather do a PM-runtime path equivalent if the target sleep state is PM_SUSPEND_TO_IDLE and there is no FW support for S0ix. > However it comes back to my first upleveveled question - is this a case > we really want to support and encourage? This type of bug and > combination of codepaths is not a case that is going to be well tested. > This patch series will align the kernel behavior to only what AMD validates. But this does not follow the definition of s2idle and its documentation.
