public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: santosh.shilimkar@ti•com (Santosh Shilimkar)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v4 1/6] Documentation: arm: define DT idle states bindings
Date: Wed, 18 Jun 2014 15:27:20 -0400	[thread overview]
Message-ID: <53A1E818.6030400@ti.com> (raw)
In-Reply-To: <20140618173648.GB31573@e102568-lin.cambridge.arm.com>

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@gmail•com>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm•com>
> ---
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

  parent reply	other threads:[~2014-06-18 19:27 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-11 16:18 [PATCH v4 0/6] ARM generic idle states Lorenzo Pieralisi
2014-06-11 16:18 ` [PATCH v4 1/6] Documentation: arm: define DT idle states bindings Lorenzo Pieralisi
2014-06-11 18:15   ` Nicolas Pitre
2014-06-13 16:49     ` Lorenzo Pieralisi
2014-06-13 17:33       ` Nicolas Pitre
2014-06-16 14:23         ` Lorenzo Pieralisi
2014-06-16 14:48           ` Nicolas Pitre
2014-06-18 17:36         ` Lorenzo Pieralisi
2014-06-18 18:20           ` Sebastian Capella
2014-06-18 19:27           ` Santosh Shilimkar [this message]
2014-06-18 20:51             ` Nicolas Pitre
2014-06-18 20:55               ` Santosh Shilimkar
2014-06-18 21:09                 ` Nicolas Pitre
2014-06-18 23:13                   ` Santosh Shilimkar
2014-06-19  7:33             ` Charles Garcia-Tobin
2014-06-19 14:08               ` Santosh Shilimkar
2014-06-19 15:09                 ` Charles Garcia-Tobin
2014-06-18 21:03           ` Nicolas Pitre
2014-06-13 17:40       ` Sebastian Capella
2014-06-11 16:18 ` [PATCH v4 2/6] Documentation: devicetree: psci: define CPU suspend parameter Lorenzo Pieralisi
2014-06-11 16:18 ` [PATCH v4 3/6] drivers: cpuidle: implement OF based idle states infrastructure Lorenzo Pieralisi
2014-06-11 18:24   ` Nicolas Pitre
2014-06-12  8:46     ` Lorenzo Pieralisi
2014-06-11 18:25   ` Rafael J. Wysocki
2014-06-12  9:03     ` Lorenzo Pieralisi
2014-06-13  3:48       ` Preeti U Murthy
2014-06-13 17:16         ` Lorenzo Pieralisi
2014-07-06 10:01       ` Paul Burton
2014-06-11 18:38   ` Nicolas Pitre
2014-06-12  9:19     ` Lorenzo Pieralisi
2014-06-11 16:18 ` [PATCH v4 4/6] arm64: add PSCI CPU_SUSPEND based cpu_suspend support Lorenzo Pieralisi
2014-06-11 16:18 ` [PATCH v4 5/6] drivers: cpuidle: CPU idle ARM64 driver Lorenzo Pieralisi
2014-06-18 21:34   ` Daniel Lezcano
2014-06-19  9:30     ` Lorenzo Pieralisi
2014-06-19  3:02   ` Rob Herring
2014-06-19  9:08     ` Lorenzo Pieralisi
2014-06-11 16:18 ` [PATCH v4 6/6] arm64: boot: dts: update rtsm aemv8 dts with PSCI and idle states Lorenzo Pieralisi

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=53A1E818.6030400@ti.com \
    --to=santosh.shilimkar@ti$(echo .)com \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox