fix(linux): Update AM62L standby mode documentation#747
Conversation
akashdeep-ti
left a comment
There was a problem hiding this comment.
Nit: Spelling mistake in commit message
|
|
||
| The ``cluster_sleep_deep_stby`` state is an optional configuration that can be enabled using the device tree overlay | ||
| :file:`k3-am62l3-evm-idle-states.dtso`. | ||
| It provides deeper power savings but requires CPSW and Display drivers to be suspended. Look at section :ref:`deep-standby-mode` for more details. |
There was a problem hiding this comment.
I think it will be better to add clear note stating that this deeper standby should only be enabled if CPSW and display are to be used in the system.
There was a problem hiding this comment.
Also, there is a lot of passive voice usage as vale is reporting, can you correct that?
| - Latency | ||
|
|
||
| * - **cpu_sleep_0** (CPU Level) | ||
| * - **cpu_sleep_stby** (CPU Level) |
There was a problem hiding this comment.
I am missing more idle states that you recently added in the DT (to select deep sleep, DSS + DeepSleep, RTC + IO + DDR) etc. Can you please add those as well?
There was a problem hiding this comment.
i understand that these idle states are not meant for standby but do we have their documentation elsewhere, then please add it there.
There was a problem hiding this comment.
This section I am thinking of keeping for the standby related idle-states only
The s2idle docs will have all the idle-states mentioned
6b568d0 to
f734092
Compare
| overlay :file:`k3-am62l3-evm-idle-states.dtso` disables the CPSW driver to ensure this | ||
| condition is met. Do not override this configuration without understanding the implications for cluster | ||
| idle transitions and hardware stability. | ||
| This overlay adds the ``cluster_sleep_deep_stby`` idle state to the device tree configuration. |
There was a problem hiding this comment.
No need to mention this in power domain hierarchy list?
There was a problem hiding this comment.
This gets added in the CLUSTER_PD domain itself
StaticRocket
left a comment
There was a problem hiding this comment.
I'd prefer if you defined the acronyms you use here, but I'm still evaluating some other solutions for this.
| | The ``cluster_sleep_deep_stby`` state is an optional configuration. Enable it using the device tree overlay | ||
| :file:`k3-am62l3-evm-idle-states.dtso`. | ||
| | It should be used **only** when CPSW and Display drivers are not needed. | ||
| | See :ref:`deep-standby-mode` for more details. |
There was a problem hiding this comment.
Please avoid using | unless explicitly necessary for formatting. It has a habit of breaking other directives.
|
|
||
| | The ``cluster_sleep_deep_stby`` state is an optional configuration. Enable it using the device tree overlay | ||
| :file:`k3-am62l3-evm-idle-states.dtso`. | ||
| | It should be used **only** when CPSW and Display drivers are not needed. |
There was a problem hiding this comment.
| | It should be used **only** when CPSW and Display drivers are not needed. | |
| | **Only do this when CPSW and Display drivers are not needed.** |
| * **CLUSTER_PD** (Cluster Power Domain): Cluster-level power domain that groups multiple CPUs | ||
|
|
||
| These power domains inform CPUIdle about which non-critical domains can be disabled when all cores within them are idle. | ||
| These power domains inform CPUIdle about which non-critical domains to disable when all cores within them are idle. |
There was a problem hiding this comment.
| These power domains inform CPUIdle about which non-critical domains to disable when all cores within them are idle. | |
| These power domains inform CPUIdle about what non-critical domains to disable when all cores within them are idle. |
| To monitor cluster-level standby mode usage, which is the most useful metric for verifying that the system | ||
| is successfully entering the low-latency standby mode when all cores are idle: | ||
| is successfully entering the standby mode when all cores are idle: |
There was a problem hiding this comment.
The cluster-level standby mode usage metric is useful metric for verifying that the system
is successfully entering standby mode. To check this value, issue the following:
| * - **cluster_sleep_0** (Low Latency Cluster standby) | ||
| - Cluster low-latency standby mode when all cores are idle, with reduced clock frequencies and non-critical power domains disabled | ||
| * - **cluster_sleep_stby** (Cluster standby) | ||
| - The default Cluster standby mode when all cores are idle, with reduced clock frequencies and non-critical power domains disabled |
There was a problem hiding this comment.
i am still thinking that this chapter should list the exact hardware optimizations done, that is which clock frequency is reduced, which non-critical power domains are disabled and other system details, maybe a table with that. As the current information is not that useful to a reader because what is non-critical from our point of view may be critical for user.
f734092 to
b887dd7
Compare
| * - **cluster_sleep_deep_stby*** (Cluster Deep standby) | ||
| - Additional Deep standby mode similar to cluster standby, with additional clock gating and power domain shutdown for more aggressive power savings. |
There was a problem hiding this comment.
nit: why is deep capitalized? IMO needs to either be "Deep Standby" or "deep standby" when talking about the mode.
There was a problem hiding this comment.
yes, ill follow this convention
| The ``cluster_sleep_deep_stby`` state is an optional configuration. Enable it using the device tree overlay | ||
| :file:`k3-am62l3-evm-idle-states.dtso`. | ||
|
|
||
| **Only do this when CPSW and Display drivers are not needed.** |
There was a problem hiding this comment.
| **Only do this when CPSW and Display drivers are not needed.** | |
| **Only enable deep standby mode when CPSW and Display drivers are not needed.** |
| ---------------------- | ||
|
|
||
| The AM62L Standby Mode implementation has important prerequisites that must be met for correct operation. | ||
| Meet the following prerequisites before using AM62L Deep Standby Mode. |
There was a problem hiding this comment.
nit
| Meet the following prerequisites before using AM62L Deep Standby Mode. | |
| The following prerequisites must be met before using AM62L Deep Standby Mode. |
|
|
||
| Similarly, the display driver must be in a suspended state for cluster standby due to frame buffer overflow issues. | ||
| Ensure display is not actively driving output when testing or relying on Standby Mode for power savings. | ||
| Similarly, the display driver must not drive output during Deep standby. The DDR enters Auto self-refresh |
There was a problem hiding this comment.
nit: capitalization
| Similarly, the display driver must not drive output during Deep standby. The DDR enters Auto self-refresh | |
| Similarly, the display driver must not drive output during deep standby. DDR enters auto self-refresh |
| overlay :file:`k3-am62l3-evm-idle-states.dtso` disables the CPSW driver to ensure this | ||
| condition is met. Do not override this configuration without understanding the implications for cluster | ||
| idle transitions and hardware stability. | ||
| To enable Deep Standby Mode, apply the device tree overlay :file:`k3-am62l3-evm-idle-states.dtso` to your system. |
There was a problem hiding this comment.
nit: capitalization
| To enable Deep Standby Mode, apply the device tree overlay :file:`k3-am62l3-evm-idle-states.dtso` to your system. | |
| To enable deep standby mode, apply the device tree overlay :file:`k3-am62l3-evm-idle-states.dtso` to your system. |
|
|
||
| * - Mode | ||
| - Hardware Sequence Steps | ||
| * - **Default Standby Mode** |
There was a problem hiding this comment.
Maybe rename to "shallow standby mode"? "Default" isn't really descriptive imo and renaming to "shallow" will make it easier to talk about it in other sections
| * Put PLL8(A53 clock) in bypass | ||
| * - **Deep Standby Mode** | ||
| - * All default standby mode steps | ||
| * Reduce CBASS frequency to lowest functional value(125Mhz) |
There was a problem hiding this comment.
nit:
| * Reduce CBASS frequency to lowest functional value(125Mhz) | |
| * Reduce CBASS frequency to lowest functional value (125Mhz) |
| * - **Deep Standby Mode** | ||
| - * All default standby mode steps | ||
| * Reduce CBASS frequency to lowest functional value(125Mhz) | ||
| * Put DDR in Auto-self-refresh |
There was a problem hiding this comment.
nit:
| * Put DDR in Auto-self-refresh | |
| * Put DDR in auto self-refresh |
|
|
||
| Once Standby Mode is enabled, you can monitor idle state activity through the PM generic power domain (genpd) | ||
| sysfs interface. The power domain names are derived from the PSCI power domain hierarchy defined in the device | ||
| Once you enable Standby Mode, you can monitor idle state activity through the PM generic power domain (genpd) |
There was a problem hiding this comment.
| Once you enable Standby Mode, you can monitor idle state activity through the PM generic power domain (genpd) | |
| Once the standby modes are enabled, you can monitor idle state activity through the PM generic power domain (genpd) |
There was a problem hiding this comment.
This might change it to passive voice, however it sounds correct this way.
|
|
||
| To monitor cluster-level standby mode usage, which is the most useful metric for verifying that the system | ||
| is successfully entering the low-latency standby mode when all cores are idle: | ||
| The cluster-level standby mode usage metric is useful metric for verifying that the system |
There was a problem hiding this comment.
| The cluster-level standby mode usage metric is useful metric for verifying that the system | |
| The cluster-level standby mode usage metric is useful for verifying that the system |
The existing standby mode is renamed from 'low-latency standby' to 'standby' and has been migrated to the default device tree from an overlay. A new 'deep-standby' mode has been added which requires the overlay for use. Update the docs to reflect these changes, introduce the new deep-standby mode along with its pre-requisites and how to enable the given mode. Signed-off-by: Scaria Kochidanadu <s-kochidanadu@ti.com>
b887dd7 to
ee32e83
Compare
The existing standby mode is renamed from 'low-latency standby' to 'standby' and has been migrated to the default device tree from an overlay. A new 'deep-standby' mode has been added which requires the overlay for use.
Update the docs to reflect these changes, introduce the new deep-standby mode along with its pre-requisites and how to enable the given mode.