Messages in this thread | | | Subject | Re: [RFC PATCH] ata: ahci: Enable DEVSLP by default on x86 modern standby platform | From | Hans de Goede <> | Date | Tue, 3 Jul 2018 12:21:50 +0200 |
| |
Hi,
On 03-07-18 10:57, Hans de Goede wrote: > Hi, > > On 03-07-18 00:08, Srinivas Pandruvada wrote: >> Hi Hans, >> >> On Mon, 2018-07-02 at 23:27 +0200, Hans de Goede wrote: > > <snip> > >>>>> SATA IP block doesn't get turned off till SATA is in DEVSLP mode. >>>>> Here >>>>> user has to either use scsi-host sysfs or tools like powertop to >>>>> set >>>>> the sata-host link_power_management_policy to min_power. >>>>> >>>>> This change sets by default link power management policy to >>>>> min_power >>>>> for some platforms. To avoid regressions, the following >>>>> conditions >>>>> are used: >>>>> - The kernel config is already set to use med_power_with_dipm or >>>>> deeper >>>>> - System is a modern standby system using ACPI low power idle >>>>> flag >>>>> - The platform is not blacklisted for Suspend to Idle and suspend >>>>> to idle is used instead of S3 >>>>> This combination will make sure that systems are fairly recent >>>>> and >>>>> since getting shipped with modern standby standby, the DEVSLP >>>>> function >>>>> is already validated. >>>>> >>>>> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.int >>>>> el.com> >>>> >>>> Seems sane to me. Hans, what do you think? >>> >>> I think this is going in the right direction, but min_power enables >>> both DEVSLP and HIPM, AFAIK Windows at least in the past did not >>> enable HIPM be default. >> >> Windows and Linux will need to meet the same criteria to reach SLP_S0 >> state. OS is not deciding whether we meet criteria or not, it is >> firmware deciding, and it will look for SATA IP to be in certain >> predefined state, before generating this SLP_S0 signal. >> >> >>> Srinivas can you figure out what Windows does wrt HIPM? >> >> We can see that under Windows the SATA rail voltage is dropped. I don't >> know if there is any other method other than DEVSLP configuration. But >> I will surely check around. > > I understand that we need DEVSLP, but AFAIK DEVSLP is entered automatically > after being in slumber for a defined time. The question is how we get into > slumber. Currently with the new med_power_with_dipm policy we always use > DIPM to enter slumber. Your suggested switch to min_power means also > enabling HIPM. What I'm suggesting is that it might better to have > DIPM + DEVSLP rather then DIPM + HIPM + DEVSLP. > > So basically the question is does windows use: > > a) DIPM + DEVSLP; or > b) DIPM + HIPM + DEVSLP > > If b. is the case, then I'm fine with your patch as is, but if Windows > does a. then we should mimic that, which requires a new power-level as > our min_power level is b.
So digging a bit deeper I just realized that another important difference between med_power_with_dipm and min_power is that min_power by default set the ASP bits making the link go to the slumber state instead of to the partial (power-saving) state.
According to: https://www.intel.com/content/dam/doc/reference-guide/sata-devices-implementation-recommendations.pdf
ASP defaults to off in the iRST drivers. But that is a document from before DEVSLP got introduced.
So we need to know if Windows and/or the iRST drivers use HIPM and ASP by default on these systems. If they do then using min_power is fine. If they don't use one or the other we are going to need a new policy reflecting those settings and use that.
Regards,
Hans
| |