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 10:57:14 +0200 |
| |
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.
>> We may need a new med_power_with_dipm_and_devslp level if windows >> does >> not enable HIPM by default on these systems. In hind sight we really >> should have separate bools for each, rather then coding this in >> a single level, ah well. > Agree. We can certainly do this for even safer implementation.
Ack.
Regards,
Hans
| |