All the mail mirrored from lore.kernel.org
 help / color / mirror / code / 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: " 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.
Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

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.