On Wed, Mar 16, 2022 at 5:43 PM Limonciello, Mario <mario.limonciello@xxxxxxx> wrote: > > >>>> My preference at this point would be to use a notifier chain, unless > >>>> that's not sufficient for some reason, because it appears to match the > >>>> notifier usage model. > >>> > >>> Well, I'm actually not sure about that. > >>> > >>>>> I pointed this out, because making this change would also make 4/5 a bit > >>>>> cleaner. You are recreating the same struct lps0_callback_handler on > >>>>> stack twice there, which looked weird to me. > >>>>> > >>>>> Note if Rafael wants to stick with the approach from this v3, then > >>>>> I guess that the approach in 4/5 is fine. > >>>>>> Rafael - can you please confirm which direction you want to see here > >>> for this? > >>> > >>> So the idea is that the PMC driver's "suspend" method needs to be > >>> invoked from acpi_s2idle_prepare_late(), so it doesn't interfere with > >>> the suspend of the other devices in the system and so it can take the > >>> constraints into account. > >> > >> The reason to do nothing (besides a debug level message right now) with the constraints > >> information is that at least on today's OEM platforms there are some instances constraints > >> aren't met on Linux that need to be investigated and root caused. These particular constraints > >> don't actually cause problems reaching s0ix residency though. > > > > Why are they useful at all, then? > > > >>> > >>> What is it going to do, in the future, depending on whether or not the > >>> constraints are met? > >> > >> The idea was that if constraints were met that it would send the OS_HINT as part of > >> amd_pmc_suspend/amd_pmc_resume, and if they aren't met then skip this step. > >> > >> It would effectively block the system from getting s0ix residency unless the constraints > >> are all met. > > > > Why do that? > > I guess to both of your above questions this begs a comparison of how > things work in Windows versus Linux. > > Windows Modern Standby has this concept of "SW DRIPS" vs "HW DRIPS". > From an end user perspective you close your lid or you click Start > button and hit sleep and the machine is in sleep. Whether it's in the > deepest state is "invisible" to you unless you're running a sleep study. > > Windows will at this time requests devices to go into their deepest > states and will continue to monitor them against the constraints table. > When the constraints table is matched a uPEP driver is notified (this is > the _DSM stuff we have in Linux too for "deepest state") and then it can > do as it pleases. ON AMD's platform this sends OS_HINT. OS_HINT is > meant to be indicate that the OS is done with all it's suspend actions > (caches flushed, devices in D3 etc) and the system can "try" to enter s0ix. > > Windows will then also distinguish between different types of wakeups > and have different behaviors for them. There are wakeups that can be > treated as keep screen off, and then possibly go back into deepest state. > > "As soon as the SoC wakes and the platform exits the DRIPS state, the > CPUs start running code again. However, the screen stays powered off > unless the interrupt was a result of user input or connecting to a power > source" > https://docs.microsoft.com/en-us/windows-hardware/design/device-experiences/transitioning-between-idle-and-active-states > > "During the Sleep state, specific value-adding SW activity may run" > https://docs.microsoft.com/en-us/windows-hardware/design/device-experiences/modern-standby-states > > So from the time that I clicked sleep in the OS, I might NOT get into > the deepest state (HW DRIPS), or I might have gotten in and out several > times. If a device driver failed to put a PCI device in D3 for example > I would not be able to enter HW DRIPS, but the suspend wouldn't "fail". Right. I'm aware of how this is expected to work in the Windows Modern Standby context. > Now to contrast this to Linux when I enter suspend all drivers will run > their various PM callbacks and devices will go into their deepest states. > * If a driver fails, the suspend actually fails and you get an error > code to go investigate what happened. > * If the devices don't get into their deepest state by the time you get > to the s2idle loop you don't get s0ix residency. > > As you can see at least for AMD's platforms OS_HINT is sent too "early". > That's why this series exists in the first place. Right. > So with all that said; why look at constraints at all if stuff is working? > From this design at least on Windows constraints are supposed to be a > safety guard that you don't start the HW process for s0i3 process "too > early". > > The last commit that is getting reverted in this series is an example of > what could happen if the process is started prematurely. Well, it is not "the process started prematurely", but an ordering issue in a process that's already under way. The real difference is that Modern Standby is designed as an extension of the system working state and things can go from "working" to "sleeping" in a kind of transparent fashion, whereas in Linux the system is either suspended or working, or going from one of these states to the other.
