From mboxrd@z Thu Jan 1 00:00:00 1970 From: Santosh Shilimkar Subject: Re: [PATCH v4 1/6] Documentation: arm: define DT idle states bindings Date: Wed, 18 Jun 2014 15:27:20 -0400 Message-ID: <53A1E818.6030400@ti.com> References: <1402503520-8611-1-git-send-email-lorenzo.pieralisi@arm.com> <1402503520-8611-2-git-send-email-lorenzo.pieralisi@arm.com> <20140613164954.GA16745@e102568-lin.cambridge.arm.com> <20140618173648.GB31573@e102568-lin.cambridge.arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20140618173648.GB31573@e102568-lin.cambridge.arm.com> Sender: linux-pm-owner@vger.kernel.org To: Lorenzo Pieralisi , Nicolas Pitre Cc: "linux-arm-kernel@lists.infradead.org" , "linux-pm@vger.kernel.org" , "devicetree@vger.kernel.org" , Mark Rutland , Sudeep Holla , Catalin Marinas , Charles Garcia-Tobin , Rob Herring , "grant.likely@linaro.org" , Peter De Schrijver , Daniel Lezcano , Amit Kucheria , Vincent Guittot , Antti Miettinen , Stephen Boyd , Kevin Hilman , Sebastian Capella , Tomasz Figa , Mark Brown , Paul Walmsley Ch List-Id: devicetree@vger.kernel.org On Wednesday 18 June 2014 01:36 PM, Lorenzo Pieralisi wrote: > On Fri, Jun 13, 2014 at 06:33:35PM +0100, Nicolas Pitre wrote: [..] > Ok, a minor tweak to the diagram above, min-residency should include > energy costs related to idle entry and exit, but not the exit-latency > itself, as long as the energy costs implied by exiting the state are > factored out in the min-residency-us property. > > Hence, to sum it up, I attached below the updated bindings patch: > > I think we are close to an agreement, if anyone disagrees please shout > as soon as possible so that we can still integrate changes. > [..] > > -- >8 -- > Subject: [PATCH] Documentation: arm: define DT idle states bindings > > ARM based platforms implement a variety of power management schemes that > allow processors to enter idle states at run-time. > The parameters defining these idle states vary on a per-platform basis forcing > the OS to hardcode the state parameters in platform specific static tables > whose size grows as the number of platforms supported in the kernel increases > and hampers device drivers standardization. > > Therefore, this patch aims at standardizing idle state device tree bindings for > ARM platforms. Bindings define idle state parameters inclusive of entry methods > and state latencies, to allow operating systems to retrieve the configuration > entries from the device tree and initialize the related power management > drivers, paving the way for common code in the kernel to deal with idle > states and removing the need for static data in current and previous kernel > versions. > > Reviewed-by: Sebastian Capella > Signed-off-by: Lorenzo Pieralisi > --- Nice work Lorenzo !! I have few comments/questions. > Documentation/devicetree/bindings/arm/cpus.txt | 8 + > .../devicetree/bindings/arm/idle-states.txt | 561 +++++++++++++++++++++ > 2 files changed, 569 insertions(+) > create mode 100644 Documentation/devicetree/bindings/arm/idle-states.txt > > diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt > index 1fe72a0..a44d4fd 100644 > --- a/Documentation/devicetree/bindings/arm/cpus.txt > +++ b/Documentation/devicetree/bindings/arm/cpus.txt > @@ -215,6 +215,12 @@ nodes to be present and contain the properties described below. > Value type: > Definition: Specifies the ACC[2] node associated with this CPU. > > + - cpu-idle-states > + Usage: Optional > + Value type: > + Definition: > + # List of phandles to idle state nodes supported > + by this cpu [3]. > > Example 1 (dual-cluster big.LITTLE system 32-bit): > > @@ -411,3 +417,5 @@ cpus { > -- > [1] arm/msm/qcom,saw2.txt > [2] arm/msm/qcom,kpss-acc.txt > +[3] ARM Linux kernel documentation - idle states bindings > + Documentation/devicetree/bindings/arm/idle-states.txt > diff --git a/Documentation/devicetree/bindings/arm/idle-states.txt b/Documentation/devicetree/bindings/arm/idle-states.txt > new file mode 100644 > index 0000000..c9e1ec6 > --- /dev/null > +++ b/Documentation/devicetree/bindings/arm/idle-states.txt > @@ -0,0 +1,561 @@ > +========================================== > +ARM idle states binding description > +========================================== > + > +========================================== > +1 - Introduction > +========================================== > + > +ARM systems contain HW capable of managing power consumption dynamically, > +where cores can be put in different low-power states (ranging from simple > +wfi to power gating) according to OSPM policies. The CPU states representing s/OSPM/OS PM ? > +the range of dynamic idle states that a processor can enter at run-time, can be > +specified through device tree bindings representing the parameters required > +to enter/exit specific idle states on a given processor. > + > +According to the Server Base System Architecture document (SBSA, [3]), the > +power states an ARM CPU can be put into are identified by the following list: > + > +- Running > +- Idle_standby > +- Idle_retention > +- Sleep > +- Off > + > +The power states described in the SBSA document define the basic CPU states on > +top of which ARM platforms implement power management schemes that allow an OS > +PM implementation to put the processor in different idle states (which include > +states listed above; "off" state is not an idle state since it does not have > +wake-up capabilities, hence it is not considered in this document). > + > +Idle state parameters (eg entry latency) are platform specific and need to be > +characterized with bindings that provide the required information to OSPM Ditto > +code so that it can build the required tables and use them at runtime. > + > +The device tree binding definition for ARM idle states is the subject of this > +document. > + > +=========================================== > +2 - idle-states node > +=========================================== > + > +ARM processor idle states are defined within the idle-states node, which is > +a direct child of the cpus node [1] and provides a container where the > +processor idle states, defined as device tree nodes, are listed. > + > +- idle-states node > + > + Usage: Optional - On ARM systems, is a container of processor idle s/is/it is ? > + states nodes. If the system does not provide CPU > + power management capabilities or the processor just > + supports idle_standby an idle-states node is not > + required. > + > + Description: idle-states node is a container node, where its > + subnodes describe the CPU idle states. > + > + Node name must be "idle-states". > + > + The idle-states node's parent node must be the cpus node. > + > + The idle-states node's child nodes can be: s/idle-states/idle-state > + > + - one or more state nodes > + > + Any other configuration is considered invalid. > + > + An idle-states node defines the following properties: > + > + - entry-method > + Usage: Required > + Value type: > + Definition: Describes the method by which a CPU enters the > + idle states. This property is required and must be > + one of: > + > + - "arm,psci" > + ARM PSCI firmware interface [2]. > + > + - "[vendor],[method]" > + An implementation dependent string with > + format "vendor,method", where vendor is a string > + denoting the name of the manufacturer and > + method is a string specifying the mechanism > + used to enter the idle state. > + > +The nodes describing the idle states (state) can only be defined within the > +idle-states node, any other configuration is considered invalid and therefore > +must be ignored. > + > +=========================================== > +3 - state node > +=========================================== > + > +A state node represents an idle state description and must be defined as > +follows: > + > +- state node > + > + Description: must be child of the idle-states node > + > + The state node name shall follow standard device tree naming > + rules ([5], 2.2.1 "Node names"), in particular state nodes which > + are siblings within a single common parent must be given a unique name. > + > + The idle state entered by executing the wfi instruction (idle_standby > + SBSA,[3][4]) is considered standard on all ARM platforms and therefore > + must not be listed. > + > + To correctly specify idle states timing and energy related properties, > + the following definitions identify the different execution phases > + a CPU goes through to enter and exit idle states and the implied > + energy metrics: > + > + ..__[EXEC]__|__[PREP]__|__[ENTRY]__|__[IDLE]__|__[EXIT]__|__[EXEC]__.. > + | | | | | > + > + |<------ entry ------->| > + | latency | > + |<- exit ->| > + | latency | > + |<-------- min-residency -------->| > + |<------- wakeup-latency ------->| > + I don't know the wakeup latency makes much sense and also correct. Hardware wakeup latency is actually exit latency. Is it for failed or abort-able ilde case ? We are adding this as a new parameter at least from idle states perspective. I think we should just avoid it. > + EXEC: Normal CPU execution. > + > + PREP: Preparation phase before committing the hardware to idle mode > + like cache flushing. This is abortable on pending wake-up > + event conditions. The abort latency is assumed to be negligible > + (i.e. less than the ENTRY + EXIT duration). If aborted, CPU > + goes back to EXEC. This phase is optional. If not abortable, > + this should be included in the ENTRY phase instead. > + > + ENTRY: The hardware is committed to idle mode. This period must run > + to completion up to IDLE before anything else can happen. > + > + IDLE: This is the actual energy-saving idle period. This may last > + between 0 and infinite time, until a wake-up event occurs. > + > + EXIT: Period during which the CPU is brought back to operational > + mode (EXEC). > + > + With the definitions provided above, the following list represents > + the valid properties for a state node: > + > + - compatible > + Usage: Required > + Value type: > + Definition: Must be "arm,idle-state". > + > + - logic-state-retained > + Usage: See definition > + Value type: > + Definition: if present logic is retained on state entry, > + otherwise it is lost. > + > + - cache-state-retained > + Usage: See definition > + Value type: > + Definition: if present cache memory is retained on state entry, > + otherwise it is lost. > + > + - entry-method-param > + Usage: See definition. > + Value type: > + Definition: Depends on the idle-states node entry-method > + property value. Refer to the entry-method bindings > + for this property value definition. > + > + - entry-latency-us > + Usage: Required > + Value type: > + Definition: u32 value representing worst case latency in > + microseconds required to enter the idle state. > + The exit-latency-us duration may be guaranteed > + only after entry-latency-us has passed. > + > + - exit-latency-us > + Usage: Required > + Value type: > + Definition: u32 value representing worst case latency > + in microseconds required to exit the idle state. > + > + - min-residency-us > + Usage: Required > + Value type: > + Definition: u32 value representing minimum residency duration > + in microseconds, inclusive of preparation and > + entry, for this idle state to be considered > + worthwhile energy wise. > + The residency time must take into account the > + energy consumed while entering and exiting the > + idle state and is therefore expected to be > + longer than entry-latency-us. > + > + - wakeup-latency-us: > + Usage: Optional > + Value type: > + Definition: u32 value representing maximum delay between the > + signaling of a wake-up event and the CPU being > + able to execute normal code again. If omitted, > + this is assumed to be equal to: > + entry-latency-us + exit-latency-us > + Rest of the patch looks fine by to me. regards, Santosh From mboxrd@z Thu Jan 1 00:00:00 1970 From: santosh.shilimkar@ti.com (Santosh Shilimkar) Date: Wed, 18 Jun 2014 15:27:20 -0400 Subject: [PATCH v4 1/6] Documentation: arm: define DT idle states bindings In-Reply-To: <20140618173648.GB31573@e102568-lin.cambridge.arm.com> References: <1402503520-8611-1-git-send-email-lorenzo.pieralisi@arm.com> <1402503520-8611-2-git-send-email-lorenzo.pieralisi@arm.com> <20140613164954.GA16745@e102568-lin.cambridge.arm.com> <20140618173648.GB31573@e102568-lin.cambridge.arm.com> Message-ID: <53A1E818.6030400@ti.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Wednesday 18 June 2014 01:36 PM, Lorenzo Pieralisi wrote: > On Fri, Jun 13, 2014 at 06:33:35PM +0100, Nicolas Pitre wrote: [..] > Ok, a minor tweak to the diagram above, min-residency should include > energy costs related to idle entry and exit, but not the exit-latency > itself, as long as the energy costs implied by exiting the state are > factored out in the min-residency-us property. > > Hence, to sum it up, I attached below the updated bindings patch: > > I think we are close to an agreement, if anyone disagrees please shout > as soon as possible so that we can still integrate changes. > [..] > > -- >8 -- > Subject: [PATCH] Documentation: arm: define DT idle states bindings > > ARM based platforms implement a variety of power management schemes that > allow processors to enter idle states at run-time. > The parameters defining these idle states vary on a per-platform basis forcing > the OS to hardcode the state parameters in platform specific static tables > whose size grows as the number of platforms supported in the kernel increases > and hampers device drivers standardization. > > Therefore, this patch aims at standardizing idle state device tree bindings for > ARM platforms. Bindings define idle state parameters inclusive of entry methods > and state latencies, to allow operating systems to retrieve the configuration > entries from the device tree and initialize the related power management > drivers, paving the way for common code in the kernel to deal with idle > states and removing the need for static data in current and previous kernel > versions. > > Reviewed-by: Sebastian Capella > Signed-off-by: Lorenzo Pieralisi > --- Nice work Lorenzo !! I have few comments/questions. > Documentation/devicetree/bindings/arm/cpus.txt | 8 + > .../devicetree/bindings/arm/idle-states.txt | 561 +++++++++++++++++++++ > 2 files changed, 569 insertions(+) > create mode 100644 Documentation/devicetree/bindings/arm/idle-states.txt > > diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt > index 1fe72a0..a44d4fd 100644 > --- a/Documentation/devicetree/bindings/arm/cpus.txt > +++ b/Documentation/devicetree/bindings/arm/cpus.txt > @@ -215,6 +215,12 @@ nodes to be present and contain the properties described below. > Value type: > Definition: Specifies the ACC[2] node associated with this CPU. > > + - cpu-idle-states > + Usage: Optional > + Value type: > + Definition: > + # List of phandles to idle state nodes supported > + by this cpu [3]. > > Example 1 (dual-cluster big.LITTLE system 32-bit): > > @@ -411,3 +417,5 @@ cpus { > -- > [1] arm/msm/qcom,saw2.txt > [2] arm/msm/qcom,kpss-acc.txt > +[3] ARM Linux kernel documentation - idle states bindings > + Documentation/devicetree/bindings/arm/idle-states.txt > diff --git a/Documentation/devicetree/bindings/arm/idle-states.txt b/Documentation/devicetree/bindings/arm/idle-states.txt > new file mode 100644 > index 0000000..c9e1ec6 > --- /dev/null > +++ b/Documentation/devicetree/bindings/arm/idle-states.txt > @@ -0,0 +1,561 @@ > +========================================== > +ARM idle states binding description > +========================================== > + > +========================================== > +1 - Introduction > +========================================== > + > +ARM systems contain HW capable of managing power consumption dynamically, > +where cores can be put in different low-power states (ranging from simple > +wfi to power gating) according to OSPM policies. The CPU states representing s/OSPM/OS PM ? > +the range of dynamic idle states that a processor can enter at run-time, can be > +specified through device tree bindings representing the parameters required > +to enter/exit specific idle states on a given processor. > + > +According to the Server Base System Architecture document (SBSA, [3]), the > +power states an ARM CPU can be put into are identified by the following list: > + > +- Running > +- Idle_standby > +- Idle_retention > +- Sleep > +- Off > + > +The power states described in the SBSA document define the basic CPU states on > +top of which ARM platforms implement power management schemes that allow an OS > +PM implementation to put the processor in different idle states (which include > +states listed above; "off" state is not an idle state since it does not have > +wake-up capabilities, hence it is not considered in this document). > + > +Idle state parameters (eg entry latency) are platform specific and need to be > +characterized with bindings that provide the required information to OSPM Ditto > +code so that it can build the required tables and use them at runtime. > + > +The device tree binding definition for ARM idle states is the subject of this > +document. > + > +=========================================== > +2 - idle-states node > +=========================================== > + > +ARM processor idle states are defined within the idle-states node, which is > +a direct child of the cpus node [1] and provides a container where the > +processor idle states, defined as device tree nodes, are listed. > + > +- idle-states node > + > + Usage: Optional - On ARM systems, is a container of processor idle s/is/it is ? > + states nodes. If the system does not provide CPU > + power management capabilities or the processor just > + supports idle_standby an idle-states node is not > + required. > + > + Description: idle-states node is a container node, where its > + subnodes describe the CPU idle states. > + > + Node name must be "idle-states". > + > + The idle-states node's parent node must be the cpus node. > + > + The idle-states node's child nodes can be: s/idle-states/idle-state > + > + - one or more state nodes > + > + Any other configuration is considered invalid. > + > + An idle-states node defines the following properties: > + > + - entry-method > + Usage: Required > + Value type: > + Definition: Describes the method by which a CPU enters the > + idle states. This property is required and must be > + one of: > + > + - "arm,psci" > + ARM PSCI firmware interface [2]. > + > + - "[vendor],[method]" > + An implementation dependent string with > + format "vendor,method", where vendor is a string > + denoting the name of the manufacturer and > + method is a string specifying the mechanism > + used to enter the idle state. > + > +The nodes describing the idle states (state) can only be defined within the > +idle-states node, any other configuration is considered invalid and therefore > +must be ignored. > + > +=========================================== > +3 - state node > +=========================================== > + > +A state node represents an idle state description and must be defined as > +follows: > + > +- state node > + > + Description: must be child of the idle-states node > + > + The state node name shall follow standard device tree naming > + rules ([5], 2.2.1 "Node names"), in particular state nodes which > + are siblings within a single common parent must be given a unique name. > + > + The idle state entered by executing the wfi instruction (idle_standby > + SBSA,[3][4]) is considered standard on all ARM platforms and therefore > + must not be listed. > + > + To correctly specify idle states timing and energy related properties, > + the following definitions identify the different execution phases > + a CPU goes through to enter and exit idle states and the implied > + energy metrics: > + > + ..__[EXEC]__|__[PREP]__|__[ENTRY]__|__[IDLE]__|__[EXIT]__|__[EXEC]__.. > + | | | | | > + > + |<------ entry ------->| > + | latency | > + |<- exit ->| > + | latency | > + |<-------- min-residency -------->| > + |<------- wakeup-latency ------->| > + I don't know the wakeup latency makes much sense and also correct. Hardware wakeup latency is actually exit latency. Is it for failed or abort-able ilde case ? We are adding this as a new parameter at least from idle states perspective. I think we should just avoid it. > + EXEC: Normal CPU execution. > + > + PREP: Preparation phase before committing the hardware to idle mode > + like cache flushing. This is abortable on pending wake-up > + event conditions. The abort latency is assumed to be negligible > + (i.e. less than the ENTRY + EXIT duration). If aborted, CPU > + goes back to EXEC. This phase is optional. If not abortable, > + this should be included in the ENTRY phase instead. > + > + ENTRY: The hardware is committed to idle mode. This period must run > + to completion up to IDLE before anything else can happen. > + > + IDLE: This is the actual energy-saving idle period. This may last > + between 0 and infinite time, until a wake-up event occurs. > + > + EXIT: Period during which the CPU is brought back to operational > + mode (EXEC). > + > + With the definitions provided above, the following list represents > + the valid properties for a state node: > + > + - compatible > + Usage: Required > + Value type: > + Definition: Must be "arm,idle-state". > + > + - logic-state-retained > + Usage: See definition > + Value type: > + Definition: if present logic is retained on state entry, > + otherwise it is lost. > + > + - cache-state-retained > + Usage: See definition > + Value type: > + Definition: if present cache memory is retained on state entry, > + otherwise it is lost. > + > + - entry-method-param > + Usage: See definition. > + Value type: > + Definition: Depends on the idle-states node entry-method > + property value. Refer to the entry-method bindings > + for this property value definition. > + > + - entry-latency-us > + Usage: Required > + Value type: > + Definition: u32 value representing worst case latency in > + microseconds required to enter the idle state. > + The exit-latency-us duration may be guaranteed > + only after entry-latency-us has passed. > + > + - exit-latency-us > + Usage: Required > + Value type: > + Definition: u32 value representing worst case latency > + in microseconds required to exit the idle state. > + > + - min-residency-us > + Usage: Required > + Value type: > + Definition: u32 value representing minimum residency duration > + in microseconds, inclusive of preparation and > + entry, for this idle state to be considered > + worthwhile energy wise. > + The residency time must take into account the > + energy consumed while entering and exiting the > + idle state and is therefore expected to be > + longer than entry-latency-us. > + > + - wakeup-latency-us: > + Usage: Optional > + Value type: > + Definition: u32 value representing maximum delay between the > + signaling of a wake-up event and the CPU being > + able to execute normal code again. If omitted, > + this is assumed to be equal to: > + entry-latency-us + exit-latency-us > + Rest of the patch looks fine by to me. regards, Santosh