Hi Rafael,
> On 2018Jul24, at 18:36, Rafael J. Wysocki <r...@rjwysocki.net> wrote:
>
> On Tuesday, July 24, 2018 11:13:42 AM CEST Rafael J. Wysocki wrote:
>> On Tuesday, July 24, 2018 10:46:09 AM CEST Kai Heng Feng wrote:
>>> Hi Rafael,
>>>
>>>> On Jun 13, 2018, at 7:17 PM, Rafael J. Wysocki <r...@rjwysocki.net> wrote:
>>>>
>>>> From: Rafael J. Wysocki <rafael.j.wyso...@intel.com>
>>>>
>>>> It is reported that commit a192aa923b66a (ACPI / LPSS: Consolidate
>>>> runtime PM and system sleep handling) introduced a system suspend
>>>> regression on some machines, but the only functional change made by
>>>> it was to cause the PM quirks in the LPSS to also be used during
>>>> system suspend and resume. While that should always work for
>>>> suspend-to-idle, it turns out to be problematic for S3
>>>> (suspend-to-RAM).
>>>>
>>>> To address that issue restore the previous S3 suspend and resume
>>>> behavior of the LPSS to avoid applying PM quirks then.
>>>
>>> The users reported that this patch does fix the S3 issue, but the S4 still
>>> fails.
>>>
>>> Please refer to [1] for more for user's testing result.
>>>
>>> [1] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1774950/comments/60
>>
>> Please ask the users to test the patch below, then, on top of the $subject
>> one.
>
> There was a mistake in the previous patch I posted, sorry about that.
>
> Please test this one instead:
The result is positive, thanks!
Kai-Heng
>
> ---
> drivers/acpi/acpi_lpss.c | 26 +++++++++++++++++---------
> 1 file changed, 17 insertions(+), 9 deletions(-)
>
> Index: linux-pm/drivers/acpi/acpi_lpss.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/acpi_lpss.c
> +++ linux-pm/drivers/acpi/acpi_lpss.c
> @@ -879,6 +879,7 @@ static void acpi_lpss_dismiss(struct dev
> #define LPSS_GPIODEF0_DMA_LLP BIT(13)
>
> static DEFINE_MUTEX(lpss_iosf_mutex);
> +static bool lpss_iosf_d3_entered;
>
> static void lpss_iosf_enter_d3_state(void)
> {
> @@ -921,6 +922,9 @@ static void lpss_iosf_enter_d3_state(voi
>
> iosf_mbi_modify(LPSS_IOSF_UNIT_LPIOEP, MBI_CR_WRITE,
> LPSS_IOSF_GPIODEF0, value1, mask1);
> +
> + lpss_iosf_d3_entered = true;
> +
> exit:
> mutex_unlock(&lpss_iosf_mutex);
> }
> @@ -935,6 +939,11 @@ static void lpss_iosf_exit_d3_state(void
>
> mutex_lock(&lpss_iosf_mutex);
>
> + if (!lpss_iosf_d3_entered)
> + goto exit;
> +
> + lpss_iosf_d3_entered = false;
> +
> iosf_mbi_modify(LPSS_IOSF_UNIT_LPIOEP, MBI_CR_WRITE,
> LPSS_IOSF_GPIODEF0, value1, mask1);
>
> @@ -944,13 +953,13 @@ static void lpss_iosf_exit_d3_state(void
> iosf_mbi_modify(LPSS_IOSF_UNIT_LPIO1, MBI_CFG_WRITE,
> LPSS_IOSF_PMCSR, value2, mask2);
>
> +exit:
> mutex_unlock(&lpss_iosf_mutex);
> }
>
> -static int acpi_lpss_suspend(struct device *dev, bool runtime)
> +static int acpi_lpss_suspend(struct device *dev, bool wakeup)
> {
> struct lpss_private_data *pdata = acpi_driver_data(ACPI_COMPANION(dev));
> - bool wakeup = runtime || device_may_wakeup(dev);
> int ret;
>
> if (pdata->dev_desc->flags & LPSS_SAVE_CTX)
> @@ -963,14 +972,14 @@ static int acpi_lpss_suspend(struct devi
> * wrong status for devices being about to be powered off. See
> * lpss_iosf_enter_d3_state() for further information.
> */
> - if ((runtime || !pm_suspend_via_firmware()) &&
> + if (acpi_target_system_state() == ACPI_STATE_S0 &&
> lpss_quirks & LPSS_QUIRK_ALWAYS_POWER_ON && iosf_mbi_available())
> lpss_iosf_enter_d3_state();
>
> return ret;
> }
>
> -static int acpi_lpss_resume(struct device *dev, bool runtime)
> +static int acpi_lpss_resume(struct device *dev)
> {
> struct lpss_private_data *pdata = acpi_driver_data(ACPI_COMPANION(dev));
> int ret;
> @@ -979,8 +988,7 @@ static int acpi_lpss_resume(struct devic
> * This call is kept first to be in symmetry with
> * acpi_lpss_runtime_suspend() one.
> */
> - if ((runtime || !pm_resume_via_firmware()) &&
> - lpss_quirks & LPSS_QUIRK_ALWAYS_POWER_ON && iosf_mbi_available())
> + if (lpss_quirks & LPSS_QUIRK_ALWAYS_POWER_ON && iosf_mbi_available())
> lpss_iosf_exit_d3_state();
>
> ret = acpi_dev_resume(dev);
> @@ -1004,12 +1012,12 @@ static int acpi_lpss_suspend_late(struct
> return 0;
>
> ret = pm_generic_suspend_late(dev);
> - return ret ? ret : acpi_lpss_suspend(dev, false);
> + return ret ? ret : acpi_lpss_suspend(dev, device_may_wakeup(dev));
> }
>
> static int acpi_lpss_resume_early(struct device *dev)
> {
> - int ret = acpi_lpss_resume(dev, false);
> + int ret = acpi_lpss_resume(dev);
>
> return ret ? ret : pm_generic_resume_early(dev);
> }
> @@ -1024,7 +1032,7 @@ static int acpi_lpss_runtime_suspend(str
>
> static int acpi_lpss_runtime_resume(struct device *dev)
> {
> - int ret = acpi_lpss_resume(dev, true);
> + int ret = acpi_lpss_resume(dev);
>
> return ret ? ret : pm_generic_runtime_resume(dev);
> }
