lkml.org 
[lkml]   [2022]   [Aug]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH v2] ata: ahci: Do not check ACPI_FADT_LOW_POWER_S0
    From
    On 8/25/2022 13:26, Rafael J. Wysocki wrote:
    > On Thu, Aug 25, 2022 at 8:17 PM Limonciello, Mario
    > <mario.limonciello@amd.com> wrote:
    >>
    >> On 8/25/2022 13:01, Rafael J. Wysocki wrote:
    >>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
    >>>
    >>> The ACPI_FADT_LOW_POWER_S0 flag merely means that it is better to
    >>> use low-power S0 idle on the given platform than S3 (provided that
    >>> the latter is supported) and it doesn't preclude using either of
    >>> them (which of them will be used depends on the choices made by user
    >>> space).
    >>>
    >>> For this reason, there is no benefit from checking that flag in
    >>> ahci_update_initial_lpm_policy().
    >>>
    >>> First off, it cannot be a bug to do S3 with policy set to either
    >>> ATA_LPM_MIN_POWER_WITH_PARTIAL or ATA_LPM_MIN_POWER, because S3 can be
    >>> used on systems with ACPI_FADT_LOW_POWER_S0 set and it must work if
    >>> really supported, so the ACPI_FADT_LOW_POWER_S0 check is not needed to
    >>> protect the S3-capable systems from failing.
    >>>
    >>> Second, suspend-to-idle can be carried out on a system with
    >>> ACPI_FADT_LOW_POWER_S0 unset and it is expected to work, so if setting
    >>> policy to either ATA_LPM_MIN_POWER_WITH_PARTIAL or ATA_LPM_MIN_POWER is
    >>> needed to handle that case correctly, it should be done regardless of
    >>> the ACPI_FADT_LOW_POWER_S0 value.
    >>>
    >>> Accordingly, drop the ACPI_FADT_LOW_POWER_S0 check from
    >>> ahci_update_initial_lpm_policy() along with the CONFIG_ACPI #ifdef
    >>> around it that is not necessary any more.
    >>
    >> Looking at the source commit for this behavior:
    >>
    >> b1a9585cc396 ("ata: ahci: Enable DEVSLP by default on x86 with SLP_S0")
    >>
    >> It was trying to set a policy tied to when the system is defaulting to
    >> suspend to idle.
    >>
    >> To try to match the spirit of the original request but not tying it to
    >> the FADT, how about using pm_suspend_default_s2idle()?
    >
    > The user can switch to "default S3" later anyway, so this wouldn't
    > help more than the check being dropped.

    Right, they could also change LPM policy to different policy later too
    if they want.

    This is just for setting up default policy. I think if you matched to
    only when pm_suspend_default_s2idle() it would be the least likelihood
    to change this default policy on unsuspecting people upgrading.

    \
     
     \ /
      Last update: 2022-08-25 20:30    [W:4.285 / U:0.076 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site