| Message ID | 20180702190845.7456-1-srinivas.pandruvada@linux.intel.com |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
| Series | [RFC] ata: ahci: Enable DEVSLP by default on x86 modern standby platform | expand |
On Mon, Jul 02, 2018 at 12:08:45PM -0700, Srinivas Pandruvada wrote: > From: Srinivas <srinivas.pandruvada@linux.intel.com> > > One of the requirement for modern x86 system to enter lowest power mode > (SLP_S0) is SATA IP block to be off. This is true even during when > platform is suspended to idle and not only in opportunistic (runtime) > suspend. > > Several of these system don't have traditional ACPI S3, so it is > important that they enter SLP_S0 state, to avoid draining battery even > during suspend even with out of the box Linux installation. > > 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.intel.com> Seems sane to me. Hans, what do you think? Thanks.
Hi, On 02-07-18 22:21, Tejun Heo wrote: > On Mon, Jul 02, 2018 at 12:08:45PM -0700, Srinivas Pandruvada wrote: >> From: Srinivas <srinivas.pandruvada@linux.intel.com> >> >> One of the requirement for modern x86 system to enter lowest power mode >> (SLP_S0) is SATA IP block to be off. This is true even during when >> platform is suspended to idle and not only in opportunistic (runtime) >> suspend. >> >> Several of these system don't have traditional ACPI S3, so it is >> important that they enter SLP_S0 state, to avoid draining battery even >> during suspend even with out of the box Linux installation. Question can systems which do have S3 not also enter SLP_S0 state during normal operation to save power? I guess the change of that happening without being in a suspend like state (screen turned off, etc), is very small because there will always be something blocking entering of SLP_S0, so that it is not worth bothering looking into SLP_S0 on systems which use S3 for suspend ? >> 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.intel.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. Srinivas can you figure out what Windows does wrt HIPM? 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. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Hans, On Mon, 2018-07-02 at 23:27 +0200, Hans de Goede wrote: > Hi, > > On 02-07-18 22:21, Tejun Heo wrote: > > On Mon, Jul 02, 2018 at 12:08:45PM -0700, Srinivas Pandruvada > > wrote: > > > From: Srinivas <srinivas.pandruvada@linux.intel.com> > > > > > > One of the requirement for modern x86 system to enter lowest > > > power mode > > > (SLP_S0) is SATA IP block to be off. This is true even during > > > when > > > platform is suspended to idle and not only in opportunistic > > > (runtime) > > > suspend. > > > > > > Several of these system don't have traditional ACPI S3, so it is > > > important that they enter SLP_S0 state, to avoid draining battery > > > even > > > during suspend even with out of the box Linux installation. > > Question can systems which do have S3 not also enter SLP_S0 state > during > normal operation to save power? S3 systems don't enter SLP_S0. When in suspend via S3 all devices are simply turn off all except some for wakeup. > > I guess the change of that happening without being in a suspend like > state > (screen turned off, etc), is very small because there will always be > something blocking entering of SLP_S0, so that it is not worth > bothering looking into SLP_S0 on systems which use S3 for suspend ? > SLP_S0 is suspend like state without big exit latency. To achieve this state the IP blocks, clocks and voltage rails needs to be in certain predefined state. If anyone of those constraints not met, the system will not reach this state. Unlike S3, we need to make sure that devices reach right suspend state. In S3 we wouldn't have cared much, as BIOS/ACPI will take action and turn off anyway. If you achieve SLP_S0, the power will be lower or equal to S3 but will be immediately able to resume. > > > 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. > 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. Thanks, Srinivas > > Regards, > > Hans -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Srinivas Pandruvada [mailto:srinivas.pandruvada@linux.intel.com] > Sent: Monday, July 2, 2018 5:09 PM > To: Hans de Goede; Tejun Heo > Cc: rjw@rjwysocki.net; alan.cox@intel.com; linux-ide@vger.kernel.org; linux- > kernel@vger.kernel.org; linux-pm@vger.kernel.org; Limonciello, Mario > Subject: Re: [RFC PATCH] ata: ahci: Enable DEVSLP by default on x86 modern > standby platform > > Hi Hans, > > On Mon, 2018-07-02 at 23:27 +0200, Hans de Goede wrote: > > Hi, > > > > On 02-07-18 22:21, Tejun Heo wrote: > > > On Mon, Jul 02, 2018 at 12:08:45PM -0700, Srinivas Pandruvada > > > wrote: > > > > From: Srinivas <srinivas.pandruvada@linux.intel.com> > > > > > > > > One of the requirement for modern x86 system to enter lowest > > > > power mode > > > > (SLP_S0) is SATA IP block to be off. This is true even during > > > > when > > > > platform is suspended to idle and not only in opportunistic > > > > (runtime) > > > > suspend. > > > > > > > > Several of these system don't have traditional ACPI S3, so it is > > > > important that they enter SLP_S0 state, to avoid draining battery > > > > even > > > > during suspend even with out of the box Linux installation. > > > > Question can systems which do have S3 not also enter SLP_S0 state > > during > > normal operation to save power? > S3 systems don't enter SLP_S0. When in suspend via S3 all devices are > simply turn off all except some for wakeup. > > > > > I guess the change of that happening without being in a suspend like > > state > > (screen turned off, etc), is very small because there will always be > > something blocking entering of SLP_S0, so that it is not worth > > bothering looking into SLP_S0 on systems which use S3 for suspend ? > > > SLP_S0 is suspend like state without big exit latency. To achieve this > state the IP blocks, clocks and voltage rails needs to be in certain > predefined state. If anyone of those constraints not met, the system > will not reach this state. Unlike S3, we need to make sure that devices > reach right suspend state. In S3 we wouldn't have cared much, as > BIOS/ACPI will take action and turn off anyway. > If you achieve SLP_S0, the power will be lower or equal to S3 but will > be immediately able to resume. > > > > > > 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. Something to keep in mind is that on Windows most systems are shipped with RAID ON rather than AHCI. So Windows defaults for AHCI link power management can't apply in the RAID ON case. It would rather be Intel RST driver behaviors for how it internally manages DIPM , HIPM, DEVSLEEP. So I checked a few things today. 1) On a system with modern standby with SATA SSD set to AHCI; link power management isn't even exposed in power options GUI. (Maybe to prevent problems with SLPS0?) I didn't see it in powercfg /q (which is supposed to list configurable options too) I found out how to unhide it with powercfg and after unhiding it I found that this is the option exposed: https://docs.microsoft.com/en-us/windows-hardware/customize/power-settings/disk-settings-link-power-management-mode---hipm-dipm It's got 5 indexes: Active, HIPM HIPM+DIPM DIPM Lowest 2) With some internal folks and they tell me that HIPM and DIPM are defaulting on with RST driver, however this can be changed with registry key as necessary. > > > > 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. Microsoft documentation above doesn't seem to mention DEVSLP but random forums (http://mywindowshub.com/add-ahci-link-power-management-hipmdipm-power-options-windows-10/) seem to indicate that Lowest is "HIPM, DIPM and DEVSLP" (if supported by storage device).
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 -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Hans, [...] > 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-im > plementation-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. Good point. In Windows there are various registry settings, so they can be tuned. It is difficult to know what they mean. Let me do some experiments to check impact of ASP on SLP_S0. Then I will resubmit this patch. Thanks, Srinivas > > Regards, > > Hans > -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c index 738fb22978dd..0b8bd1b6cf7c 100644 --- a/drivers/ata/ahci.c +++ b/drivers/ata/ahci.c @@ -43,6 +43,7 @@ #include <linux/dmi.h> #include <linux/gfp.h> #include <linux/msi.h> +#include <linux/suspend.h> #include <scsi/scsi_host.h> #include <scsi/scsi_cmnd.h> #include <linux/libata.h> @@ -1550,6 +1551,16 @@ static int ahci_init_msi(struct pci_dev *pdev, unsigned int n_ports, return pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSIX); } +static void ahci_update_mobile_policy(void) +{ +#ifdef CONFIG_ACPI + if (mobile_lpm_policy > ATA_LPM_MED_POWER && + (acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0) && + mem_sleep_current == PM_SUSPEND_TO_IDLE) + mobile_lpm_policy = ATA_LPM_MIN_POWER; +#endif +} + static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) { unsigned int board_id = ent->driver_data; @@ -1736,6 +1747,8 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) if (pi.flags & ATA_FLAG_EM) ahci_reset_em(host); + ahci_update_mobile_policy(); + for (i = 0; i < host->n_ports; i++) { struct ata_port *ap = host->ports[i]; diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c index 87331565e505..969e67715cf7 100644 --- a/kernel/power/suspend.c +++ b/kernel/power/suspend.c @@ -49,6 +49,7 @@ static const char * const mem_sleep_labels[] = { const char *mem_sleep_states[PM_SUSPEND_MAX]; suspend_state_t mem_sleep_current = PM_SUSPEND_TO_IDLE; +EXPORT_SYMBOL_GPL(mem_sleep_current); suspend_state_t mem_sleep_default = PM_SUSPEND_MAX; suspend_state_t pm_suspend_target_state; EXPORT_SYMBOL_GPL(pm_suspend_target_state);