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 <sebcape@xxxxxxxxx>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx>
> ---
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: <phandle>
> Definition: Specifies the ACC[2] node associated with this CPU.
>
> + - cpu-idle-states
> + Usage: Optional
> + Value type: <prop-encoded-array>
> + 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: <stringlist>
> + 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: <stringlist>
> + Definition: Must be "arm,idle-state".
> +
> + - logic-state-retained
> + Usage: See definition
> + Value type: <none>
> + Definition: if present logic is retained on state entry,
> + otherwise it is lost.
> +
> + - cache-state-retained
> + Usage: See definition
> + Value type: <none>
> + Definition: if present cache memory is retained on state entry,
> + otherwise it is lost.
> +
> + - entry-method-param
> + Usage: See definition.
> + Value type: <u32>
> + 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: <prop-encoded-array>
> + 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: <prop-encoded-array>
> + Definition: u32 value representing worst case latency
> + in microseconds required to exit the idle state.
> +
> + - min-residency-us
> + Usage: Required
> + Value type: <prop-encoded-array>
> + 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: <prop-encoded-array>
> + 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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
