All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: sudeep.holla@arm.com (Sudeep Holla)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 3/3] drivers: firmware: psci: add system suspend support
Date: Tue, 14 Jul 2015 14:18:10 +0100	[thread overview]
Message-ID: <55A50C12.60901@arm.com> (raw)
In-Reply-To: <20150714194038.47ccf2e5@xhacker>



On 14/07/15 12:40, Jisheng Zhang wrote:
> Dear Sudeep,
>
> On Tue, 14 Jul 2015 12:02:15 +0100
> Sudeep Holla <sudeep.holla@arm.com> wrote:
>
>>
>>
>> On 14/07/15 10:50, Jisheng Zhang wrote:
>>> Dear Sudeep,
>>>
>>> On Tue, 14 Jul 2015 10:14:25 +0100
>>> Sudeep Holla <sudeep.holla@arm.com> wrote:
>>>
>>>>
>>>>
>>>> On 14/07/15 07:17, Jisheng Zhang wrote:
>>>>> Hi Sudeep,
>>>>>
>>>>> I'm sorry to being late. Just have some comments below.
>>>>>
>>>>> On Thu, 18 Jun 2015 15:41:34 +0100
>>>>> Sudeep Holla <sudeep.holla@arm.com> wrote:
>>>>>
>>>>>> PSCI v1.0 introduces a new API called PSCI_SYSTEM_SUSPEND. This API
>>>>>> provides the mechanism by which the calling OS can request entry into
>>>>>> the deepest possible system sleep state.
>>>>>>
>>>>>> It meets all the necessary preconditions for entering suspend to RAM
>>>>>> state in Linux. This patch adds support for PSCI_SYSTEM_SUSPEND in psci
>>>>>> firmware and registers a psci system suspend operation to implement the
>>>>>> suspend-to-RAM(s2r) in a generic way on all the platforms implementing
>>>>>> PSCI.
>>>>>>
>>>>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>>>>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>>>>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>>>>>> ---
>>>>>>     drivers/firmware/psci.c   | 31 +++++++++++++++++++++++++++++++
>>>>>>     include/uapi/linux/psci.h |  3 +++
>>>>>>     2 files changed, 34 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
>>>>>> index 752ca7c9eb97..f2b2b3e1c6e4 100644
>>>>>> --- a/drivers/firmware/psci.c
>>>>>> +++ b/drivers/firmware/psci.c
>>>>>> @@ -20,11 +20,13 @@
>>>>>>     #include <linux/printk.h>
>>>>>>     #include <linux/psci.h>
>>>>>>     #include <linux/reboot.h>
>>>>>> +#include <linux/suspend.h>
>>>>>>
>>>>>>     #include <uapi/linux/psci.h>
>>>>>>
>>>>>>     #include <asm/system_misc.h>
>>>>>>     #include <asm/smp_plat.h>
>>>>>> +#include <asm/suspend.h>
>>>>>>
>>>>>>     /*
>>>>>>      * While a 64-bit OS can make calls with SMC32 calling conventions, for some
>>>>>> @@ -222,6 +224,33 @@ static int __init psci_features(u32 psci_func_id)
>>>>>>     			      psci_func_id, 0, 0);
>>>>>>     }
>>>>>>
>>>>>> +static int psci_system_suspend(unsigned long unused)
>>>>>> +{
>>>>>> +	return invoke_psci_fn(PSCI_FN_NATIVE(1_0, SYSTEM_SUSPEND),
>>>>>> +			      virt_to_phys(cpu_resume), 0, 0);
>>>>>> +}
>>>>>> +
>>>>>> +static int psci_system_suspend_enter(suspend_state_t state)
>>>>>> +{
>>>>>> +	return cpu_suspend(0, psci_system_suspend);
>>>>>> +}
>>>>>> +
>>>>>> +static const struct platform_suspend_ops psci_suspend_ops = {
>>>>>> +	.valid          = suspend_valid_only_mem,
>>>>>> +	.enter          = psci_system_suspend_enter,
>>>>>> +};
>>>>>> +
>>>>>> +static void __init psci_init_system_suspend(void)
>>>>>> +{
>>>>>> +	if (!IS_ENABLED(CONFIG_SUSPEND))
>>>>>> +		return;
>>>>>> +
>>>>>> +	if (psci_features(PSCI_FN_NATIVE(1_0, SYSTEM_SUSPEND)))
>>>>>> +		return;
>>>>>
>>>>> So this requires the firmware implements SYSTEM_SUSPEND which doesn't exist
>>>>> until PSCI 1.0,
>>>>
>>>> Correct, if you need System to RAM in Linux on a platform with PSCI
>>>> firmware, firmware *must implement* SYSTEM_SUSPEND.
>>>>
>>>>> and even in PSCI 1.0 SYSTEM_SUSPEND is optional,
>>>>
>>>> Optional here means, that you may chose to implement or not in the
>>>> firmware. It doesn't mean we can implement S2R in Linux using other
>>>> PSCI methods. SYSTEM_SUSPEND was added mainly to avoid since workarounds
>>>> in the kernel.
>>>>
>>>>> we also want
>>>>> suspend to ram feature on PSCI 0.2 or PSCI 1.0 w/o SYSTEM_SUSPEND,
>>>>
>>>> Why do you choose to have PSCIv1.0 w/o SYSTEM_SUSPEND when you need that
>>>> feature in Linux. Is it just because we can manage with workarounds ?
>>>> Sorry, that's not an option.
>>>
>>> The problem is the PSCI 1.0 SYSTEM_SUSPEND definition: we can't pass something
>>> like stateid as in CPU_SUSPEND to firmware. The stateid is used to tell
>>> firmware to go to different Sleep state, for example S2 or S3.
>>>
>>
>> Can you elaborate on S2 here with an example ? You are using ACPI
>> Sleep State definitions here which should not be mixed with PSCI.
>
> Yes, it's ACPI concept. Here "S2" can be the "S2" mentioned in PSCI 1.0 spec.
>
> PSCI 1.0 SPEC says:
>
> "While ACPI distinguishes between two system level suspend to RAM states,
> S2 and S3, PSCI provides a single API. In practice, operating systems use
> only one suspend to RAM state, so this is not seen as a limitation. Note
> that this specification is using the state definitions of S2 and S3 provided
> by ACPI, but does not limit use of this PSCI function to ACPI based systems.
> Semantics such as those described for sleep states S2 and S3 are used in
> non-ACPI based systems."
>

OK, though similar semantics may be used else where, we need to define
the state similar to the way ACPI does it for x86 systems.

> Here is an example:
>
> S3 is the state where All devices are suspended and power down,  memory is
> placed in self-refresh mode. Usually this is the suspend to ram state.
>

Agreed.

> S2 is the state where devices except cpus are suspended and put in a
> low-power state(not power off), if the deivce doesn't support lower power mode,
> the device is left on. memory is placed in self-refresh mode. secondary CPUs
> are powered off via. PSCI_CPU_OFF, boot cpu is put into WFI state and runs at
> low freq.
>

Is this a standard state or custom tailored for some power optimization
on particular system ? Anyway I am not sure if I quite understand the
exact definition of this state. When and how often do you use this
use this state ?

Have you looked at PM_SUSPEND_FREEZE state implemented in Linux ?
For me this S2 you explained sounds similar to PM_SUSPEND_FREEZE state.
In short, PM_SUSPEND_FREEZE = all the processes are frozen + all the
devices are suspended + all the processors enter deepest idle state
possible.

Though I don't understand why you need to power-off secondary cpus
and stay in WFI on boot cpu as that would have increased latency
defeating the purpose of S2 state. Also why you are mixing DVFS here ?
PSCI deals with just low power states and has nothing to do with DVFS.

Further IMO we can also achieve a low power state almost close to
PM_SUSPEND_FREEZE having run-time PM for all the devices and cpuidle.

Regards,
Sudeep

  reply	other threads:[~2015-07-14 13:18 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-16 13:50 [PATCH 0/2] PSCI: system suspend support Sudeep Holla
2015-06-16 13:50 ` [PATCH 1/2] arm64: kernel: rename __cpu_suspend to keep it aligned with arm Sudeep Holla
2015-06-17 13:57   ` Lorenzo Pieralisi
2015-06-16 13:50 ` [PATCH 2/2] drivers: firmware: psci: add system suspend support Sudeep Holla
2015-06-17 15:08   ` Lorenzo Pieralisi
2015-06-17 15:41     ` Sudeep Holla
2015-06-18 14:41 ` [PATCH v2 0/3] PSCI: " Sudeep Holla
2015-06-18 14:41   ` [PATCH v2 1/3] arm64: kernel: rename __cpu_suspend to keep it aligned with arm Sudeep Holla
2015-06-18 14:55     ` Catalin Marinas
2015-06-18 15:08       ` Sudeep Holla
2015-06-19 13:50         ` Catalin Marinas
2015-06-18 14:41   ` [PATCH v2 2/3] drivers: firmware: psci: define more generic PSCI_FN_NATIVE macro Sudeep Holla
2015-09-14 13:17     ` Lorenzo Pieralisi
2015-09-14 13:21       ` Sudeep Holla
2015-06-18 14:41   ` [PATCH v2 3/3] drivers: firmware: psci: add system suspend support Sudeep Holla
2015-07-14  6:17     ` Jisheng Zhang
2015-07-14  9:14       ` Sudeep Holla
2015-07-14  9:50         ` Jisheng Zhang
2015-07-14 11:02           ` Sudeep Holla
2015-07-14 11:40             ` Jisheng Zhang
2015-07-14 13:18               ` Sudeep Holla [this message]
2015-07-15  2:34                 ` Jisheng Zhang
2015-07-15 10:20                   ` Sudeep Holla
2015-09-14 13:23     ` Lorenzo Pieralisi
2015-09-14 13:32       ` Sudeep Holla
2015-06-18 18:13   ` [PATCH v2 0/3] PSCI: " Ashwin Chaugule

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55A50C12.60901@arm.com \
    --to=sudeep.holla@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.