* [PATCH 0/3] perf, tools: new power trace API
@ 2011-01-04 10:17 jean.pihet
2011-01-04 10:17 ` [PATCH 1/3] perf: add calls to suspend trace point jean.pihet
` (6 more replies)
0 siblings, 7 replies; 40+ messages in thread
From: jean.pihet @ 2011-01-04 10:17 UTC (permalink / raw)
To: mingo, linux-kernel, trenn
Cc: linux-omap, Arjan van de Ven, linux-perf-users, rjw, Jean Pihet
From: Jean Pihet <j-pihet@ti.com>
Provides:
. calls to machine_suspend trace point,
. OMAP support,
. API Documentation
Applies on top of Thomas's 8 latest power trace API patches, cf.
http://marc.info/?l=linux-kernel&m=129130827309354&w=2
Jean Pihet (3):
perf: add calls to suspend trace point
perf: add OMAP support for the new power events
tools, perf: Documentation for the power events API
Documentation/trace/events-power.txt | 90 ++++++++++++++++++++++++++++++++++
arch/arm/mach-omap2/pm34xx.c | 7 +++
arch/arm/mach-omap2/powerdomain.c | 3 +
arch/arm/plat-omap/clock.c | 13 ++++-
kernel/power/suspend.c | 3 +
5 files changed, 113 insertions(+), 3 deletions(-)
create mode 100644 Documentation/trace/events-power.txt
--
1.7.2.3
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 1/3] perf: add calls to suspend trace point
2011-01-04 10:17 [PATCH 0/3] perf, tools: new power trace API jean.pihet
@ 2011-01-04 10:17 ` jean.pihet
2011-01-04 10:39 ` Ingo Molnar
2011-01-04 10:17 ` [PATCH 2/3] perf: add OMAP support for the new power events jean.pihet
` (5 subsequent siblings)
6 siblings, 1 reply; 40+ messages in thread
From: jean.pihet @ 2011-01-04 10:17 UTC (permalink / raw)
To: mingo, linux-kernel, trenn
Cc: linux-omap, Arjan van de Ven, linux-perf-users, rjw, Jean Pihet
From: Jean Pihet <j-pihet@ti.com>
Uses the machine_suspend trace point, called from the
generic kernel suspend_enter function.
Signed-off-by: Jean Pihet <j-pihet@ti.com>
CC: Thomas Renninger <trenn@suse.de>
---
kernel/power/suspend.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index ecf7705..0650596 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -22,6 +22,7 @@
#include <linux/mm.h>
#include <linux/slab.h>
#include <linux/suspend.h>
+#include <trace/events/power.h>
#include "power.h"
@@ -164,7 +165,9 @@ static int suspend_enter(suspend_state_t state)
error = sysdev_suspend(PMSG_SUSPEND);
if (!error) {
if (!suspend_test(TEST_CORE) && pm_check_wakeup_events()) {
+ trace_machine_suspend(state);
error = suspend_ops->enter(state);
+ trace_machine_suspend(PWR_EVENT_EXIT);
events_check_enabled = false;
}
sysdev_resume();
--
1.7.2.3
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH 2/3] perf: add OMAP support for the new power events
2011-01-04 10:17 [PATCH 0/3] perf, tools: new power trace API jean.pihet
2011-01-04 10:17 ` [PATCH 1/3] perf: add calls to suspend trace point jean.pihet
@ 2011-01-04 10:17 ` jean.pihet
2011-01-04 10:42 ` Ingo Molnar
2011-01-04 18:03 ` Nishanth Menon
2011-01-04 10:17 ` [PATCH 3/3] tools, perf: Documentation for the power events API jean.pihet
` (4 subsequent siblings)
6 siblings, 2 replies; 40+ messages in thread
From: jean.pihet @ 2011-01-04 10:17 UTC (permalink / raw)
To: mingo, linux-kernel, trenn
Cc: linux-omap, Arjan van de Ven, linux-perf-users, rjw, Jean Pihet
From: Jean Pihet <j-pihet@ti.com>
The patch adds the new power management trace points for
the OMAP architecture.
The trace points are for:
- default idle handler. Since the cpuidle framework is
instrumented in the generic way there is no need to
add trace points in the OMAP specific cpuidle handler;
- cpufreq (DVFS),
- clocks changes (enable, disable, set_rate),
- change of power domains next power states.
Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
arch/arm/mach-omap2/pm34xx.c | 7 +++++++
arch/arm/mach-omap2/powerdomain.c | 3 +++
arch/arm/plat-omap/clock.c | 13 ++++++++++---
3 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 0ec8a04..0ee0b0e 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -29,6 +29,7 @@
#include <linux/delay.h>
#include <linux/slab.h>
#include <linux/console.h>
+#include <trace/events/power.h>
#include <plat/sram.h>
#include <plat/clockdomain.h>
@@ -506,8 +507,14 @@ static void omap3_pm_idle(void)
if (omap_irq_pending() || need_resched())
goto out;
+ trace_power_start(POWER_CSTATE, 1, smp_processor_id());
+ trace_cpu_idle(1, smp_processor_id());
+
omap_sram_idle();
+ trace_power_end(smp_processor_id());
+ trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());
+
out:
local_fiq_enable();
local_irq_enable();
diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index 6527ec3..73cbe9a 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -23,6 +23,7 @@
#include <linux/errno.h>
#include <linux/err.h>
#include <linux/io.h>
+#include <trace/events/power.h>
#include <asm/atomic.h>
@@ -440,6 +441,8 @@ int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
pr_debug("powerdomain: setting next powerstate for %s to %0x\n",
pwrdm->name, pwrst);
+ trace_power_domain_target(pwrdm->name, pwrst, smp_processor_id());
+
prm_rmw_mod_reg_bits(OMAP_POWERSTATE_MASK,
(pwrst << OMAP_POWERSTATE_SHIFT),
pwrdm->prcm_offs, pwrstctrl_reg_offs);
diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
index fc62fb5..7cbb09b 100644
--- a/arch/arm/plat-omap/clock.c
+++ b/arch/arm/plat-omap/clock.c
@@ -21,6 +21,7 @@
#include <linux/cpufreq.h>
#include <linux/debugfs.h>
#include <linux/io.h>
+#include <trace/events/power.h>
#include <plat/clock.h>
@@ -43,8 +44,10 @@ int clk_enable(struct clk *clk)
return -EINVAL;
spin_lock_irqsave(&clockfw_lock, flags);
- if (arch_clock->clk_enable)
+ if (arch_clock->clk_enable) {
+ trace_clock_enable(clk->name, 1, smp_processor_id());
ret = arch_clock->clk_enable(clk);
+ }
spin_unlock_irqrestore(&clockfw_lock, flags);
return ret;
@@ -66,8 +69,10 @@ void clk_disable(struct clk *clk)
goto out;
}
- if (arch_clock->clk_disable)
+ if (arch_clock->clk_disable) {
+ trace_clock_disable(clk->name, 0, smp_processor_id());
arch_clock->clk_disable(clk);
+ }
out:
spin_unlock_irqrestore(&clockfw_lock, flags);
@@ -120,8 +125,10 @@ int clk_set_rate(struct clk *clk, unsigned long rate)
return ret;
spin_lock_irqsave(&clockfw_lock, flags);
- if (arch_clock->clk_set_rate)
+ if (arch_clock->clk_set_rate) {
+ trace_clock_set_rate(clk->name, rate, smp_processor_id());
ret = arch_clock->clk_set_rate(clk, rate);
+ }
if (ret == 0) {
if (clk->recalc)
clk->rate = clk->recalc(clk);
--
1.7.2.3
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH 3/3] tools, perf: Documentation for the power events API
2011-01-04 10:17 [PATCH 0/3] perf, tools: new power trace API jean.pihet
2011-01-04 10:17 ` [PATCH 1/3] perf: add calls to suspend trace point jean.pihet
2011-01-04 10:17 ` [PATCH 2/3] perf: add OMAP support for the new power events jean.pihet
@ 2011-01-04 10:17 ` jean.pihet
2011-01-04 10:48 ` [PATCH 1/3] perf: add calls to suspend trace point jean.pihet
` (3 subsequent siblings)
6 siblings, 0 replies; 40+ messages in thread
From: jean.pihet @ 2011-01-04 10:17 UTC (permalink / raw)
To: mingo, linux-kernel, trenn
Cc: linux-omap, Arjan van de Ven, linux-perf-users, rjw, Jean Pihet
From: Jean Pihet <j-pihet@ti.com>
Provides documentation for the following:
- the new power trace API,
- the old (legacy) power trace API,
- the DEPRECATED Kconfig option usage.
Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
Documentation/trace/events-power.txt | 90 ++++++++++++++++++++++++++++++++++
1 files changed, 90 insertions(+), 0 deletions(-)
create mode 100644 Documentation/trace/events-power.txt
diff --git a/Documentation/trace/events-power.txt b/Documentation/trace/events-power.txt
new file mode 100644
index 0000000..8a50653
--- /dev/null
+++ b/Documentation/trace/events-power.txt
@@ -0,0 +1,90 @@
+
+ Subsystem Trace Points: power
+
+The power tracing system captures events related to power transitions
+within the kernel. Broadly speaking there are three major subheadings:
+
+ o Power state switch which reports events related to suspend (S-states),
+ cpuidle (C-states) and cpufreq (P-states)
+ o System clock related changes
+ o Power domains related changes and transitions
+
+This document describes what each of the tracepoints is and why they
+might be useful.
+
+Cf. include/trace/events/power.h for the events definitions.
+
+1. Power state switch events
+============================
+
+1.1 New trace API
+-----------------
+
+A 'cpu' event class gathers the CPU-related events: cpuidle and
+cpufreq.
+
+cpu_idle "state=%lu cpu_id=%lu"
+cpu_frequency "state=%lu cpu_id=%lu"
+
+A suspend event is used to indicate the system going in and out of the
+suspend mode:
+
+machine_suspend "state=%lu"
+
+
+Note: the value of '-1' or '4294967295' for state means an exit from the current state,
+i.e. trace_cpu_idle(4, smp_processor_id()) means that the system
+enters the idle state 4, while trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id())
+means that the system exits the previous idle state.
+
+The event which has 'state=4294967295' in the trace is very important to the user
+space tools which are using it to detect the end of the current state, and so to
+correctly draw the states diagrams and to calculate accurate statistics etc.
+
+1.2 DEPRECATED trace API
+------------------------
+
+A new Kconfig option CONFIG_EVENT_POWER_TRACING_DEPRECATED with the default value of
+'y' has been created. This allows the legacy trace power API to be used conjointly
+with the new trace API.
+The Kconfig option, the old trace API (in include/trace/events/power.h) and the
+old trace points will disappear in a future release (namely 2.6.41).
+
+power_start "type=%lu state=%lu cpu_id=%lu"
+power_frequency "type=%lu state=%lu cpu_id=%lu"
+power_end "cpu_id=%lu"
+
+The 'type' parameter takes one of those macros:
+ . POWER_NONE = 0,
+ . POWER_CSTATE = 1, /* C-State */
+ . POWER_PSTATE = 2, /* Fequency change or DVFS */
+
+The 'state' parameter is set depending on the type:
+ . Target C-state for type=POWER_CSTATE,
+ . Target frequency for type=POWER_PSTATE,
+
+power_end is used to indicate the exit of a state, corresponding to the latest
+power_start event.
+
+2. Clocks events
+================
+The clock events are used for clock enable/disable and for
+clock rate change.
+
+clock_enable "%s state=%lu cpu_id=%lu"
+clock_disable "%s state=%lu cpu_id=%lu"
+clock_set_rate "%s state=%lu cpu_id=%lu"
+
+The first parameter gives the clock name (e.g. "gpio1_iclk").
+The second parameter is '1' for enable, '0' for disable, the target
+clock rate for set_rate.
+
+3. Power domains events
+=======================
+The power domain events are used for power domains transitions
+
+power_domain_target "%s state=%lu cpu_id=%lu"
+
+The first parameter gives the power domain name (e.g. "mpu_pwrdm").
+The second parameter is the power domain target state.
+
--
1.7.2.3
^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH 1/3] perf: add calls to suspend trace point
2011-01-04 10:17 ` [PATCH 1/3] perf: add calls to suspend trace point jean.pihet
@ 2011-01-04 10:39 ` Ingo Molnar
0 siblings, 0 replies; 40+ messages in thread
From: Ingo Molnar @ 2011-01-04 10:39 UTC (permalink / raw)
To: jean.pihet
Cc: linux-kernel, trenn, linux-omap, Arjan van de Ven,
linux-perf-users, rjw, Jean Pihet
* jean.pihet@newoldbits.com <jean.pihet@newoldbits.com> wrote:
> From: Jean Pihet <j-pihet@ti.com>
>
> Uses the machine_suspend trace point, called from the
> generic kernel suspend_enter function.
>
> Signed-off-by: Jean Pihet <j-pihet@ti.com>
> CC: Thomas Renninger <trenn@suse.de>
> ---
> kernel/power/suspend.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
Please use the scripts/get_maintainer.pl to construct a proper Cc: list and to
gather the necessary Acked-by:
scripts/get_maintainer.pl -f kernel/power/suspend.c
Thanks,
Ingo
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 2/3] perf: add OMAP support for the new power events
2011-01-04 10:17 ` [PATCH 2/3] perf: add OMAP support for the new power events jean.pihet
@ 2011-01-04 10:42 ` Ingo Molnar
2011-01-04 10:58 ` Pihet-XID, Jean
2011-01-04 18:03 ` Nishanth Menon
1 sibling, 1 reply; 40+ messages in thread
From: Ingo Molnar @ 2011-01-04 10:42 UTC (permalink / raw)
To: jean.pihet
Cc: linux-kernel, trenn, linux-omap, Arjan van de Ven,
linux-perf-users, rjw, Jean Pihet
* jean.pihet@newoldbits.com <jean.pihet@newoldbits.com> wrote:
> From: Jean Pihet <j-pihet@ti.com>
>
> The patch adds the new power management trace points for
> the OMAP architecture.
>
> The trace points are for:
> - default idle handler. Since the cpuidle framework is
> instrumented in the generic way there is no need to
> add trace points in the OMAP specific cpuidle handler;
> - cpufreq (DVFS),
> - clocks changes (enable, disable, set_rate),
> - change of power domains next power states.
>
> Signed-off-by: Jean Pihet <j-pihet@ti.com>
> ---
> arch/arm/mach-omap2/pm34xx.c | 7 +++++++
> arch/arm/mach-omap2/powerdomain.c | 3 +++
> arch/arm/plat-omap/clock.c | 13 ++++++++++---
> 3 files changed, 20 insertions(+), 3 deletions(-)
I suspect the gents and mailing lists listed by:
scripts/get_maintainer.pl -f arch/arm/plat-omap/clock.c
scripts/get_maintainer.pl -f arch/arm/mach-omap2/pm34xx.c
Would want to be Cc:-ed as well. That will also get the right Acked-by's. (if you
want these commits to go upstream via the perf tree)
Thanks,
Ingo
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 1/3] perf: add calls to suspend trace point
2011-01-04 10:17 [PATCH 0/3] perf, tools: new power trace API jean.pihet
` (3 preceding siblings ...)
2011-01-04 10:48 ` [PATCH 1/3] perf: add calls to suspend trace point jean.pihet
@ 2011-01-04 10:48 ` jean.pihet
2011-01-04 11:29 ` Pavel Machek
2011-01-04 11:29 ` Pavel Machek
2011-01-04 10:54 ` [PATCH 2/3] perf: add OMAP support for the new power events jean.pihet
2011-01-04 10:54 ` jean.pihet
6 siblings, 2 replies; 40+ messages in thread
From: jean.pihet @ 2011-01-04 10:48 UTC (permalink / raw)
To: mingo, trenn, Len Brown, Pavel Machek, Rafael J. Wysocki,
linux-pm, linux-kernel
Cc: linux-omap, Arjan van de Ven, linux-perf-users, Jean Pihet
From: Jean Pihet <j-pihet@ti.com>
Uses the machine_suspend trace point, called from the
generic kernel suspend_enter function.
Signed-off-by: Jean Pihet <j-pihet@ti.com>
CC: Thomas Renninger <trenn@suse.de>
---
kernel/power/suspend.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index ecf7705..0650596 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -22,6 +22,7 @@
#include <linux/mm.h>
#include <linux/slab.h>
#include <linux/suspend.h>
+#include <trace/events/power.h>
#include "power.h"
@@ -164,7 +165,9 @@ static int suspend_enter(suspend_state_t state)
error = sysdev_suspend(PMSG_SUSPEND);
if (!error) {
if (!suspend_test(TEST_CORE) && pm_check_wakeup_events()) {
+ trace_machine_suspend(state);
error = suspend_ops->enter(state);
+ trace_machine_suspend(PWR_EVENT_EXIT);
events_check_enabled = false;
}
sysdev_resume();
--
1.7.2.3
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH 1/3] perf: add calls to suspend trace point
2011-01-04 10:17 [PATCH 0/3] perf, tools: new power trace API jean.pihet
` (2 preceding siblings ...)
2011-01-04 10:17 ` [PATCH 3/3] tools, perf: Documentation for the power events API jean.pihet
@ 2011-01-04 10:48 ` jean.pihet
2011-01-04 10:48 ` jean.pihet
` (2 subsequent siblings)
6 siblings, 0 replies; 40+ messages in thread
From: jean.pihet @ 2011-01-04 10:48 UTC (permalink / raw)
To: mingo, trenn, Len Brown, Pavel Machek, Rafael J. Wysocki, linux-pm
Cc: linux-perf-users, linux-omap, Arjan van de Ven, Jean Pihet
From: Jean Pihet <j-pihet@ti.com>
Uses the machine_suspend trace point, called from the
generic kernel suspend_enter function.
Signed-off-by: Jean Pihet <j-pihet@ti.com>
CC: Thomas Renninger <trenn@suse.de>
---
kernel/power/suspend.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index ecf7705..0650596 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -22,6 +22,7 @@
#include <linux/mm.h>
#include <linux/slab.h>
#include <linux/suspend.h>
+#include <trace/events/power.h>
#include "power.h"
@@ -164,7 +165,9 @@ static int suspend_enter(suspend_state_t state)
error = sysdev_suspend(PMSG_SUSPEND);
if (!error) {
if (!suspend_test(TEST_CORE) && pm_check_wakeup_events()) {
+ trace_machine_suspend(state);
error = suspend_ops->enter(state);
+ trace_machine_suspend(PWR_EVENT_EXIT);
events_check_enabled = false;
}
sysdev_resume();
--
1.7.2.3
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH 2/3] perf: add OMAP support for the new power events
2011-01-04 10:17 [PATCH 0/3] perf, tools: new power trace API jean.pihet
2011-01-04 10:17 ` [PATCH 1/3] perf: add calls to suspend trace point jean.pihet
@ 2011-01-04 10:54 ` jean.pihet
2011-01-04 10:17 ` [PATCH 3/3] tools, perf: Documentation for the power events API jean.pihet
` (4 subsequent siblings)
6 siblings, 0 replies; 40+ messages in thread
From: jean.pihet @ 2011-01-04 10:54 UTC (permalink / raw)
To: mingo, trenn, Kevin Hilman, Paul Walmsley, Tony Lindgren,
Russell King, linux-omap, linux-arm-kernel, linux-kernel
Cc: Arjan van de Ven, linux-perf-users, rjw, Jean Pihet
From: Jean Pihet <j-pihet@ti.com>
The patch adds the new power management trace points for
the OMAP architecture.
The trace points are for:
- default idle handler. Since the cpuidle framework is
instrumented in the generic way there is no need to
add trace points in the OMAP specific cpuidle handler;
- cpufreq (DVFS),
- clocks changes (enable, disable, set_rate),
- change of power domains next power states.
Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
arch/arm/mach-omap2/pm34xx.c | 7 +++++++
arch/arm/mach-omap2/powerdomain.c | 3 +++
arch/arm/plat-omap/clock.c | 13 ++++++++++---
3 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 0ec8a04..0ee0b0e 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -29,6 +29,7 @@
#include <linux/delay.h>
#include <linux/slab.h>
#include <linux/console.h>
+#include <trace/events/power.h>
#include <plat/sram.h>
#include <plat/clockdomain.h>
@@ -506,8 +507,14 @@ static void omap3_pm_idle(void)
if (omap_irq_pending() || need_resched())
goto out;
+ trace_power_start(POWER_CSTATE, 1, smp_processor_id());
+ trace_cpu_idle(1, smp_processor_id());
+
omap_sram_idle();
+ trace_power_end(smp_processor_id());
+ trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());
+
out:
local_fiq_enable();
local_irq_enable();
diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index 6527ec3..73cbe9a 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -23,6 +23,7 @@
#include <linux/errno.h>
#include <linux/err.h>
#include <linux/io.h>
+#include <trace/events/power.h>
#include <asm/atomic.h>
@@ -440,6 +441,8 @@ int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
pr_debug("powerdomain: setting next powerstate for %s to %0x\n",
pwrdm->name, pwrst);
+ trace_power_domain_target(pwrdm->name, pwrst, smp_processor_id());
+
prm_rmw_mod_reg_bits(OMAP_POWERSTATE_MASK,
(pwrst << OMAP_POWERSTATE_SHIFT),
pwrdm->prcm_offs, pwrstctrl_reg_offs);
diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
index fc62fb5..7cbb09b 100644
--- a/arch/arm/plat-omap/clock.c
+++ b/arch/arm/plat-omap/clock.c
@@ -21,6 +21,7 @@
#include <linux/cpufreq.h>
#include <linux/debugfs.h>
#include <linux/io.h>
+#include <trace/events/power.h>
#include <plat/clock.h>
@@ -43,8 +44,10 @@ int clk_enable(struct clk *clk)
return -EINVAL;
spin_lock_irqsave(&clockfw_lock, flags);
- if (arch_clock->clk_enable)
+ if (arch_clock->clk_enable) {
+ trace_clock_enable(clk->name, 1, smp_processor_id());
ret = arch_clock->clk_enable(clk);
+ }
spin_unlock_irqrestore(&clockfw_lock, flags);
return ret;
@@ -66,8 +69,10 @@ void clk_disable(struct clk *clk)
goto out;
}
- if (arch_clock->clk_disable)
+ if (arch_clock->clk_disable) {
+ trace_clock_disable(clk->name, 0, smp_processor_id());
arch_clock->clk_disable(clk);
+ }
out:
spin_unlock_irqrestore(&clockfw_lock, flags);
@@ -120,8 +125,10 @@ int clk_set_rate(struct clk *clk, unsigned long rate)
return ret;
spin_lock_irqsave(&clockfw_lock, flags);
- if (arch_clock->clk_set_rate)
+ if (arch_clock->clk_set_rate) {
+ trace_clock_set_rate(clk->name, rate, smp_processor_id());
ret = arch_clock->clk_set_rate(clk, rate);
+ }
if (ret == 0) {
if (clk->recalc)
clk->rate = clk->recalc(clk);
--
1.7.2.3
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH 2/3] perf: add OMAP support for the new power events
@ 2011-01-04 10:54 ` jean.pihet
0 siblings, 0 replies; 40+ messages in thread
From: jean.pihet @ 2011-01-04 10:54 UTC (permalink / raw)
To: mingo, trenn, Kevin Hilman, Paul Walmsley, Tony Lindgren, Russell King
Cc: Arjan van de Ven, linux-perf-users, rjw, Jean Pihet
From: Jean Pihet <j-pihet@ti.com>
The patch adds the new power management trace points for
the OMAP architecture.
The trace points are for:
- default idle handler. Since the cpuidle framework is
instrumented in the generic way there is no need to
add trace points in the OMAP specific cpuidle handler;
- cpufreq (DVFS),
- clocks changes (enable, disable, set_rate),
- change of power domains next power states.
Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
arch/arm/mach-omap2/pm34xx.c | 7 +++++++
arch/arm/mach-omap2/powerdomain.c | 3 +++
arch/arm/plat-omap/clock.c | 13 ++++++++++---
3 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 0ec8a04..0ee0b0e 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -29,6 +29,7 @@
#include <linux/delay.h>
#include <linux/slab.h>
#include <linux/console.h>
+#include <trace/events/power.h>
#include <plat/sram.h>
#include <plat/clockdomain.h>
@@ -506,8 +507,14 @@ static void omap3_pm_idle(void)
if (omap_irq_pending() || need_resched())
goto out;
+ trace_power_start(POWER_CSTATE, 1, smp_processor_id());
+ trace_cpu_idle(1, smp_processor_id());
+
omap_sram_idle();
+ trace_power_end(smp_processor_id());
+ trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());
+
out:
local_fiq_enable();
local_irq_enable();
diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index 6527ec3..73cbe9a 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -23,6 +23,7 @@
#include <linux/errno.h>
#include <linux/err.h>
#include <linux/io.h>
+#include <trace/events/power.h>
#include <asm/atomic.h>
@@ -440,6 +441,8 @@ int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
pr_debug("powerdomain: setting next powerstate for %s to %0x\n",
pwrdm->name, pwrst);
+ trace_power_domain_target(pwrdm->name, pwrst, smp_processor_id());
+
prm_rmw_mod_reg_bits(OMAP_POWERSTATE_MASK,
(pwrst << OMAP_POWERSTATE_SHIFT),
pwrdm->prcm_offs, pwrstctrl_reg_offs);
diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
index fc62fb5..7cbb09b 100644
--- a/arch/arm/plat-omap/clock.c
+++ b/arch/arm/plat-omap/clock.c
@@ -21,6 +21,7 @@
#include <linux/cpufreq.h>
#include <linux/debugfs.h>
#include <linux/io.h>
+#include <trace/events/power.h>
#include <plat/clock.h>
@@ -43,8 +44,10 @@ int clk_enable(struct clk *clk)
return -EINVAL;
spin_lock_irqsave(&clockfw_lock, flags);
- if (arch_clock->clk_enable)
+ if (arch_clock->clk_enable) {
+ trace_clock_enable(clk->name, 1, smp_processor_id());
ret = arch_clock->clk_enable(clk);
+ }
spin_unlock_irqrestore(&clockfw_lock, flags);
return ret;
@@ -66,8 +69,10 @@ void clk_disable(struct clk *clk)
goto out;
}
- if (arch_clock->clk_disable)
+ if (arch_clock->clk_disable) {
+ trace_clock_disable(clk->name, 0, smp_processor_id());
arch_clock->clk_disable(clk);
+ }
out:
spin_unlock_irqrestore(&clockfw_lock, flags);
@@ -120,8 +125,10 @@ int clk_set_rate(struct clk *clk, unsigned long rate)
return ret;
spin_lock_irqsave(&clockfw_lock, flags);
- if (arch_clock->clk_set_rate)
+ if (arch_clock->clk_set_rate) {
+ trace_clock_set_rate(clk->name, rate, smp_processor_id());
ret = arch_clock->clk_set_rate(clk, rate);
+ }
if (ret == 0) {
if (clk->recalc)
clk->rate = clk->recalc(clk);
--
1.7.2.3
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH 2/3] perf: add OMAP support for the new power events
2011-01-04 10:17 [PATCH 0/3] perf, tools: new power trace API jean.pihet
` (4 preceding siblings ...)
2011-01-04 10:48 ` jean.pihet
@ 2011-01-04 10:54 ` jean.pihet
2011-01-04 10:54 ` jean.pihet
6 siblings, 0 replies; 40+ messages in thread
From: jean.pihet @ 2011-01-04 10:54 UTC (permalink / raw)
To: mingo, trenn, Kevin Hilman, Paul Walmsley, Tony Lindgren, Russell King
Cc: Arjan van de Ven, linux-perf-users, rjw, Jean Pihet
From: Jean Pihet <j-pihet@ti.com>
The patch adds the new power management trace points for
the OMAP architecture.
The trace points are for:
- default idle handler. Since the cpuidle framework is
instrumented in the generic way there is no need to
add trace points in the OMAP specific cpuidle handler;
- cpufreq (DVFS),
- clocks changes (enable, disable, set_rate),
- change of power domains next power states.
Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
arch/arm/mach-omap2/pm34xx.c | 7 +++++++
arch/arm/mach-omap2/powerdomain.c | 3 +++
arch/arm/plat-omap/clock.c | 13 ++++++++++---
3 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 0ec8a04..0ee0b0e 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -29,6 +29,7 @@
#include <linux/delay.h>
#include <linux/slab.h>
#include <linux/console.h>
+#include <trace/events/power.h>
#include <plat/sram.h>
#include <plat/clockdomain.h>
@@ -506,8 +507,14 @@ static void omap3_pm_idle(void)
if (omap_irq_pending() || need_resched())
goto out;
+ trace_power_start(POWER_CSTATE, 1, smp_processor_id());
+ trace_cpu_idle(1, smp_processor_id());
+
omap_sram_idle();
+ trace_power_end(smp_processor_id());
+ trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());
+
out:
local_fiq_enable();
local_irq_enable();
diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index 6527ec3..73cbe9a 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -23,6 +23,7 @@
#include <linux/errno.h>
#include <linux/err.h>
#include <linux/io.h>
+#include <trace/events/power.h>
#include <asm/atomic.h>
@@ -440,6 +441,8 @@ int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
pr_debug("powerdomain: setting next powerstate for %s to %0x\n",
pwrdm->name, pwrst);
+ trace_power_domain_target(pwrdm->name, pwrst, smp_processor_id());
+
prm_rmw_mod_reg_bits(OMAP_POWERSTATE_MASK,
(pwrst << OMAP_POWERSTATE_SHIFT),
pwrdm->prcm_offs, pwrstctrl_reg_offs);
diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
index fc62fb5..7cbb09b 100644
--- a/arch/arm/plat-omap/clock.c
+++ b/arch/arm/plat-omap/clock.c
@@ -21,6 +21,7 @@
#include <linux/cpufreq.h>
#include <linux/debugfs.h>
#include <linux/io.h>
+#include <trace/events/power.h>
#include <plat/clock.h>
@@ -43,8 +44,10 @@ int clk_enable(struct clk *clk)
return -EINVAL;
spin_lock_irqsave(&clockfw_lock, flags);
- if (arch_clock->clk_enable)
+ if (arch_clock->clk_enable) {
+ trace_clock_enable(clk->name, 1, smp_processor_id());
ret = arch_clock->clk_enable(clk);
+ }
spin_unlock_irqrestore(&clockfw_lock, flags);
return ret;
@@ -66,8 +69,10 @@ void clk_disable(struct clk *clk)
goto out;
}
- if (arch_clock->clk_disable)
+ if (arch_clock->clk_disable) {
+ trace_clock_disable(clk->name, 0, smp_processor_id());
arch_clock->clk_disable(clk);
+ }
out:
spin_unlock_irqrestore(&clockfw_lock, flags);
@@ -120,8 +125,10 @@ int clk_set_rate(struct clk *clk, unsigned long rate)
return ret;
spin_lock_irqsave(&clockfw_lock, flags);
- if (arch_clock->clk_set_rate)
+ if (arch_clock->clk_set_rate) {
+ trace_clock_set_rate(clk->name, rate, smp_processor_id());
ret = arch_clock->clk_set_rate(clk, rate);
+ }
if (ret == 0) {
if (clk->recalc)
clk->rate = clk->recalc(clk);
--
1.7.2.3
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH 2/3] perf: add OMAP support for the new power events
@ 2011-01-04 10:54 ` jean.pihet
0 siblings, 0 replies; 40+ messages in thread
From: jean.pihet at newoldbits.com @ 2011-01-04 10:54 UTC (permalink / raw)
To: linux-arm-kernel
From: Jean Pihet <j-pihet@ti.com>
The patch adds the new power management trace points for
the OMAP architecture.
The trace points are for:
- default idle handler. Since the cpuidle framework is
instrumented in the generic way there is no need to
add trace points in the OMAP specific cpuidle handler;
- cpufreq (DVFS),
- clocks changes (enable, disable, set_rate),
- change of power domains next power states.
Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
arch/arm/mach-omap2/pm34xx.c | 7 +++++++
arch/arm/mach-omap2/powerdomain.c | 3 +++
arch/arm/plat-omap/clock.c | 13 ++++++++++---
3 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 0ec8a04..0ee0b0e 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -29,6 +29,7 @@
#include <linux/delay.h>
#include <linux/slab.h>
#include <linux/console.h>
+#include <trace/events/power.h>
#include <plat/sram.h>
#include <plat/clockdomain.h>
@@ -506,8 +507,14 @@ static void omap3_pm_idle(void)
if (omap_irq_pending() || need_resched())
goto out;
+ trace_power_start(POWER_CSTATE, 1, smp_processor_id());
+ trace_cpu_idle(1, smp_processor_id());
+
omap_sram_idle();
+ trace_power_end(smp_processor_id());
+ trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());
+
out:
local_fiq_enable();
local_irq_enable();
diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index 6527ec3..73cbe9a 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -23,6 +23,7 @@
#include <linux/errno.h>
#include <linux/err.h>
#include <linux/io.h>
+#include <trace/events/power.h>
#include <asm/atomic.h>
@@ -440,6 +441,8 @@ int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
pr_debug("powerdomain: setting next powerstate for %s to %0x\n",
pwrdm->name, pwrst);
+ trace_power_domain_target(pwrdm->name, pwrst, smp_processor_id());
+
prm_rmw_mod_reg_bits(OMAP_POWERSTATE_MASK,
(pwrst << OMAP_POWERSTATE_SHIFT),
pwrdm->prcm_offs, pwrstctrl_reg_offs);
diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
index fc62fb5..7cbb09b 100644
--- a/arch/arm/plat-omap/clock.c
+++ b/arch/arm/plat-omap/clock.c
@@ -21,6 +21,7 @@
#include <linux/cpufreq.h>
#include <linux/debugfs.h>
#include <linux/io.h>
+#include <trace/events/power.h>
#include <plat/clock.h>
@@ -43,8 +44,10 @@ int clk_enable(struct clk *clk)
return -EINVAL;
spin_lock_irqsave(&clockfw_lock, flags);
- if (arch_clock->clk_enable)
+ if (arch_clock->clk_enable) {
+ trace_clock_enable(clk->name, 1, smp_processor_id());
ret = arch_clock->clk_enable(clk);
+ }
spin_unlock_irqrestore(&clockfw_lock, flags);
return ret;
@@ -66,8 +69,10 @@ void clk_disable(struct clk *clk)
goto out;
}
- if (arch_clock->clk_disable)
+ if (arch_clock->clk_disable) {
+ trace_clock_disable(clk->name, 0, smp_processor_id());
arch_clock->clk_disable(clk);
+ }
out:
spin_unlock_irqrestore(&clockfw_lock, flags);
@@ -120,8 +125,10 @@ int clk_set_rate(struct clk *clk, unsigned long rate)
return ret;
spin_lock_irqsave(&clockfw_lock, flags);
- if (arch_clock->clk_set_rate)
+ if (arch_clock->clk_set_rate) {
+ trace_clock_set_rate(clk->name, rate, smp_processor_id());
ret = arch_clock->clk_set_rate(clk, rate);
+ }
if (ret == 0) {
if (clk->recalc)
clk->rate = clk->recalc(clk);
--
1.7.2.3
^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH 2/3] perf: add OMAP support for the new power events
2011-01-04 10:42 ` Ingo Molnar
@ 2011-01-04 10:58 ` Pihet-XID, Jean
0 siblings, 0 replies; 40+ messages in thread
From: Pihet-XID, Jean @ 2011-01-04 10:58 UTC (permalink / raw)
To: Ingo Molnar
Cc: jean.pihet, linux-kernel, trenn, linux-omap, Arjan van de Ven,
linux-perf-users, rjw
On Tue, Jan 4, 2011 at 11:42 AM, Ingo Molnar <mingo@elte.hu> wrote:
>
> * jean.pihet@newoldbits.com <jean.pihet@newoldbits.com> wrote:
>
>> From: Jean Pihet <j-pihet@ti.com>
>>
>> The patch adds the new power management trace points for
>> the OMAP architecture.
>>
>> The trace points are for:
>> - default idle handler. Since the cpuidle framework is
>> instrumented in the generic way there is no need to
>> add trace points in the OMAP specific cpuidle handler;
>> - cpufreq (DVFS),
>> - clocks changes (enable, disable, set_rate),
>> - change of power domains next power states.
>>
>> Signed-off-by: Jean Pihet <j-pihet@ti.com>
>> ---
>> arch/arm/mach-omap2/pm34xx.c | 7 +++++++
>> arch/arm/mach-omap2/powerdomain.c | 3 +++
>> arch/arm/plat-omap/clock.c | 13 ++++++++++---
>> 3 files changed, 20 insertions(+), 3 deletions(-)
>
> I suspect the gents and mailing lists listed by:
>
> scripts/get_maintainer.pl -f arch/arm/plat-omap/clock.c
> scripts/get_maintainer.pl -f arch/arm/mach-omap2/pm34xx.c
>
> Would want to be Cc:-ed as well.
Sorry about that. The patches 1 & 2 have been re-sent with additional Cc's.
> That will also get the right Acked-by's. (if you
> want these commits to go upstream via the perf tree)
Yes the idea is to get those upstream via the tip tree, since it now
contains the perf API clean-up patches from Thomas.
>
> Thanks,
>
> Ingo
>
Thanks,
Jean
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 1/3] perf: add calls to suspend trace point
2011-01-04 10:48 ` jean.pihet
2011-01-04 11:29 ` Pavel Machek
@ 2011-01-04 11:29 ` Pavel Machek
2011-01-04 15:00 ` Jean Pihet
2011-01-04 15:00 ` Jean Pihet
1 sibling, 2 replies; 40+ messages in thread
From: Pavel Machek @ 2011-01-04 11:29 UTC (permalink / raw)
To: jean.pihet
Cc: mingo, trenn, Len Brown, Rafael J. Wysocki, linux-pm,
linux-kernel, linux-omap, Arjan van de Ven, linux-perf-users,
Jean Pihet
Hi!
> Uses the machine_suspend trace point, called from the
> generic kernel suspend_enter function.
>
> Signed-off-by: Jean Pihet <j-pihet@ti.com>
> CC: Thomas Renninger <trenn@suse.de>
> ---
> kernel/power/suspend.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
> index ecf7705..0650596 100644
> --- a/kernel/power/suspend.c
> +++ b/kernel/power/suspend.c
> @@ -22,6 +22,7 @@
> #include <linux/mm.h>
> #include <linux/slab.h>
> #include <linux/suspend.h>
> +#include <trace/events/power.h>
>
> #include "power.h"
>
> @@ -164,7 +165,9 @@ static int suspend_enter(suspend_state_t state)
> error = sysdev_suspend(PMSG_SUSPEND);
> if (!error) {
> if (!suspend_test(TEST_CORE) && pm_check_wakeup_events()) {
> + trace_machine_suspend(state);
> error = suspend_ops->enter(state);
> + trace_machine_suspend(PWR_EVENT_EXIT);
> events_check_enabled = false;
> }
> sysdev_resume();
Ok... why this place? I mean, perhaps suspend time should include
device suspend?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 1/3] perf: add calls to suspend trace point
2011-01-04 10:48 ` jean.pihet
@ 2011-01-04 11:29 ` Pavel Machek
2011-01-04 11:29 ` Pavel Machek
1 sibling, 0 replies; 40+ messages in thread
From: Pavel Machek @ 2011-01-04 11:29 UTC (permalink / raw)
To: jean.pihet
Cc: Len Brown, linux-perf-users, linux-kernel, Jean Pihet, linux-pm,
mingo, linux-omap, Arjan van de Ven
Hi!
> Uses the machine_suspend trace point, called from the
> generic kernel suspend_enter function.
>
> Signed-off-by: Jean Pihet <j-pihet@ti.com>
> CC: Thomas Renninger <trenn@suse.de>
> ---
> kernel/power/suspend.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
> index ecf7705..0650596 100644
> --- a/kernel/power/suspend.c
> +++ b/kernel/power/suspend.c
> @@ -22,6 +22,7 @@
> #include <linux/mm.h>
> #include <linux/slab.h>
> #include <linux/suspend.h>
> +#include <trace/events/power.h>
>
> #include "power.h"
>
> @@ -164,7 +165,9 @@ static int suspend_enter(suspend_state_t state)
> error = sysdev_suspend(PMSG_SUSPEND);
> if (!error) {
> if (!suspend_test(TEST_CORE) && pm_check_wakeup_events()) {
> + trace_machine_suspend(state);
> error = suspend_ops->enter(state);
> + trace_machine_suspend(PWR_EVENT_EXIT);
> events_check_enabled = false;
> }
> sysdev_resume();
Ok... why this place? I mean, perhaps suspend time should include
device suspend?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 1/3] perf: add calls to suspend trace point
2011-01-04 11:29 ` Pavel Machek
@ 2011-01-04 15:00 ` Jean Pihet
2011-01-04 15:00 ` Jean Pihet
1 sibling, 0 replies; 40+ messages in thread
From: Jean Pihet @ 2011-01-04 15:00 UTC (permalink / raw)
To: Pavel Machek
Cc: mingo, trenn, Len Brown, Rafael J. Wysocki, linux-pm,
linux-kernel, linux-omap, Arjan van de Ven, linux-perf-users,
Jean Pihet
Hi,
On Tue, Jan 4, 2011 at 12:29 PM, Pavel Machek <pavel@ucw.cz> wrote:
> Hi!
>
>> Uses the machine_suspend trace point, called from the
>> generic kernel suspend_enter function.
>>
>> Signed-off-by: Jean Pihet <j-pihet@ti.com>
>> CC: Thomas Renninger <trenn@suse.de>
>> ---
>> kernel/power/suspend.c | 3 +++
>> 1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
>> index ecf7705..0650596 100644
>> --- a/kernel/power/suspend.c
>> +++ b/kernel/power/suspend.c
>> @@ -22,6 +22,7 @@
>> #include <linux/mm.h>
>> #include <linux/slab.h>
>> #include <linux/suspend.h>
>> +#include <trace/events/power.h>
>>
>> #include "power.h"
>>
>> @@ -164,7 +165,9 @@ static int suspend_enter(suspend_state_t state)
>> error = sysdev_suspend(PMSG_SUSPEND);
>> if (!error) {
>> if (!suspend_test(TEST_CORE) && pm_check_wakeup_events()) {
>> + trace_machine_suspend(state);
>> error = suspend_ops->enter(state);
>> + trace_machine_suspend(PWR_EVENT_EXIT);
>> events_check_enabled = false;
>> }
>> sysdev_resume();
>
> Ok... why this place?
This trace has been placed here because it traces the machine low
level mode enter.
> I mean, perhaps suspend time should include
> device suspend?
That makes sense. We have a few options here:
1) keep the traces as proposed to trace the low level machine code only,
2) move the traces to the entry and exit of suspend_enter so that it
includes the prepare and late_prepare (+ the associated wake-up)
callbacks as well,
3) move the traces to suspend_devices_and_enter so that it includes 2)
and the handling of the console and the devices,
4) move the traces to enter_state do that it includes 3), the call to
sys_sync and the user space freeze.
Note that the the SNAPSHOT_2RAM ioctl code also calls
suspend_devices_and_enter, so if only 4) is used no trace will be
generated in that case.
I am in favor of 3) of 4).
What do you think?
> Pavel
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
>
Thanks,
Jean
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 1/3] perf: add calls to suspend trace point
@ 2011-01-04 15:00 ` Jean Pihet
0 siblings, 0 replies; 40+ messages in thread
From: Jean Pihet @ 2011-01-04 15:00 UTC (permalink / raw)
To: Pavel Machek
Cc: mingo, trenn, Len Brown, Rafael J. Wysocki, linux-pm,
linux-kernel, linux-omap, Arjan van de Ven, linux-perf-users,
Jean Pihet
Hi,
On Tue, Jan 4, 2011 at 12:29 PM, Pavel Machek <pavel@ucw.cz> wrote:
> Hi!
>
>> Uses the machine_suspend trace point, called from the
>> generic kernel suspend_enter function.
>>
>> Signed-off-by: Jean Pihet <j-pihet@ti.com>
>> CC: Thomas Renninger <trenn@suse.de>
>> ---
>> kernel/power/suspend.c | 3 +++
>> 1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
>> index ecf7705..0650596 100644
>> --- a/kernel/power/suspend.c
>> +++ b/kernel/power/suspend.c
>> @@ -22,6 +22,7 @@
>> #include <linux/mm.h>
>> #include <linux/slab.h>
>> #include <linux/suspend.h>
>> +#include <trace/events/power.h>
>>
>> #include "power.h"
>>
>> @@ -164,7 +165,9 @@ static int suspend_enter(suspend_state_t state)
>> error = sysdev_suspend(PMSG_SUSPEND);
>> if (!error) {
>> if (!suspend_test(TEST_CORE) && pm_check_wakeup_events()) {
>> + trace_machine_suspend(state);
>> error = suspend_ops->enter(state);
>> + trace_machine_suspend(PWR_EVENT_EXIT);
>> events_check_enabled = false;
>> }
>> sysdev_resume();
>
> Ok... why this place?
This trace has been placed here because it traces the machine low
level mode enter.
> I mean, perhaps suspend time should include
> device suspend?
That makes sense. We have a few options here:
1) keep the traces as proposed to trace the low level machine code only,
2) move the traces to the entry and exit of suspend_enter so that it
includes the prepare and late_prepare (+ the associated wake-up)
callbacks as well,
3) move the traces to suspend_devices_and_enter so that it includes 2)
and the handling of the console and the devices,
4) move the traces to enter_state do that it includes 3), the call to
sys_sync and the user space freeze.
Note that the the SNAPSHOT_2RAM ioctl code also calls
suspend_devices_and_enter, so if only 4) is used no trace will be
generated in that case.
I am in favor of 3) of 4).
What do you think?
> Pavel
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
>
Thanks,
Jean
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 1/3] perf: add calls to suspend trace point
2011-01-04 11:29 ` Pavel Machek
2011-01-04 15:00 ` Jean Pihet
@ 2011-01-04 15:00 ` Jean Pihet
1 sibling, 0 replies; 40+ messages in thread
From: Jean Pihet @ 2011-01-04 15:00 UTC (permalink / raw)
To: Pavel Machek
Cc: Len Brown, linux-perf-users, linux-kernel, Jean Pihet, linux-pm,
mingo, linux-omap, Arjan van de Ven
Hi,
On Tue, Jan 4, 2011 at 12:29 PM, Pavel Machek <pavel@ucw.cz> wrote:
> Hi!
>
>> Uses the machine_suspend trace point, called from the
>> generic kernel suspend_enter function.
>>
>> Signed-off-by: Jean Pihet <j-pihet@ti.com>
>> CC: Thomas Renninger <trenn@suse.de>
>> ---
>> kernel/power/suspend.c | 3 +++
>> 1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
>> index ecf7705..0650596 100644
>> --- a/kernel/power/suspend.c
>> +++ b/kernel/power/suspend.c
>> @@ -22,6 +22,7 @@
>> #include <linux/mm.h>
>> #include <linux/slab.h>
>> #include <linux/suspend.h>
>> +#include <trace/events/power.h>
>>
>> #include "power.h"
>>
>> @@ -164,7 +165,9 @@ static int suspend_enter(suspend_state_t state)
>> error = sysdev_suspend(PMSG_SUSPEND);
>> if (!error) {
>> if (!suspend_test(TEST_CORE) && pm_check_wakeup_events()) {
>> + trace_machine_suspend(state);
>> error = suspend_ops->enter(state);
>> + trace_machine_suspend(PWR_EVENT_EXIT);
>> events_check_enabled = false;
>> }
>> sysdev_resume();
>
> Ok... why this place?
This trace has been placed here because it traces the machine low
level mode enter.
> I mean, perhaps suspend time should include
> device suspend?
That makes sense. We have a few options here:
1) keep the traces as proposed to trace the low level machine code only,
2) move the traces to the entry and exit of suspend_enter so that it
includes the prepare and late_prepare (+ the associated wake-up)
callbacks as well,
3) move the traces to suspend_devices_and_enter so that it includes 2)
and the handling of the console and the devices,
4) move the traces to enter_state do that it includes 3), the call to
sys_sync and the user space freeze.
Note that the the SNAPSHOT_2RAM ioctl code also calls
suspend_devices_and_enter, so if only 4) is used no trace will be
generated in that case.
I am in favor of 3) of 4).
What do you think?
> Pavel
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
>
Thanks,
Jean
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 2/3] perf: add OMAP support for the new power events
2011-01-04 10:17 ` [PATCH 2/3] perf: add OMAP support for the new power events jean.pihet
2011-01-04 10:42 ` Ingo Molnar
@ 2011-01-04 18:03 ` Nishanth Menon
2011-01-05 10:56 ` Jean Pihet
1 sibling, 1 reply; 40+ messages in thread
From: Nishanth Menon @ 2011-01-04 18:03 UTC (permalink / raw)
To: jean.pihet
Cc: mingo, linux-kernel, trenn, linux-omap, Arjan van de Ven,
linux-perf-users, rjw, Jean Pihet
jean.pihet@newoldbits.com had written, on 01/04/2011 04:17 AM, the
following:
[..]
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 0ec8a04..0ee0b0e 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -29,6 +29,7 @@
> #include <linux/delay.h>
> #include <linux/slab.h>
> #include <linux/console.h>
> +#include <trace/events/power.h>
>
> #include <plat/sram.h>
> #include <plat/clockdomain.h>
> @@ -506,8 +507,14 @@ static void omap3_pm_idle(void)
> if (omap_irq_pending() || need_resched())
> goto out;
>
> + trace_power_start(POWER_CSTATE, 1, smp_processor_id());
> + trace_cpu_idle(1, smp_processor_id());
> +
> omap_sram_idle();
>
> + trace_power_end(smp_processor_id());
> + trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());
Dumb question: it just tells me which C state was attempted - not if
actually succeeded in hitting it rt? Does'nt this give us a false data?
[..]
> diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
> index fc62fb5..7cbb09b 100644
> --- a/arch/arm/plat-omap/clock.c
> +++ b/arch/arm/plat-omap/clock.c
(from an offline discussion on a related topic): Would it also be nice
to hook on mach-omap2/clock.c points as well to hook on indirect changes?
[..]
--
Regards,
Nishanth Menon
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 2/3] perf: add OMAP support for the new power events
2011-01-04 10:54 ` jean.pihet
@ 2011-01-04 18:48 ` Paul Walmsley
-1 siblings, 0 replies; 40+ messages in thread
From: Paul Walmsley @ 2011-01-04 18:48 UTC (permalink / raw)
To: jean.pihet
Cc: mingo, trenn, Kevin Hilman, Tony Lindgren, Russell King,
linux-omap, linux-arm-kernel, linux-kernel, Arjan van de Ven,
linux-perf-users, rjw, Jean Pihet
Hello Jean,
On Tue, 4 Jan 2011, jean.pihet@newoldbits.com wrote:
> From: Jean Pihet <j-pihet@ti.com>
>
> The patch adds the new power management trace points for
> the OMAP architecture.
>
> The trace points are for:
> - default idle handler. Since the cpuidle framework is
> instrumented in the generic way there is no need to
> add trace points in the OMAP specific cpuidle handler;
> - cpufreq (DVFS),
> - clocks changes (enable, disable, set_rate),
A question about these. Are these only meant to track calls to these
functions from outside the clock code? Or meant to track actual hardware
clock changes? If the latter, then it might make sense to put these
trace points into the functions that actually change the hardware
registers, e.g., omap2_dflt_clk_{enable,disable}(), etc., since a
clk_enable() on a leaf clock may result in many internal system clocks
being enabled up the clock tree.
- Paul
> - change of power domains next power states.
>
> Signed-off-by: Jean Pihet <j-pihet@ti.com>
> ---
> arch/arm/mach-omap2/pm34xx.c | 7 +++++++
> arch/arm/mach-omap2/powerdomain.c | 3 +++
> arch/arm/plat-omap/clock.c | 13 ++++++++++---
> 3 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 0ec8a04..0ee0b0e 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -29,6 +29,7 @@
> #include <linux/delay.h>
> #include <linux/slab.h>
> #include <linux/console.h>
> +#include <trace/events/power.h>
>
> #include <plat/sram.h>
> #include <plat/clockdomain.h>
> @@ -506,8 +507,14 @@ static void omap3_pm_idle(void)
> if (omap_irq_pending() || need_resched())
> goto out;
>
> + trace_power_start(POWER_CSTATE, 1, smp_processor_id());
> + trace_cpu_idle(1, smp_processor_id());
> +
> omap_sram_idle();
>
> + trace_power_end(smp_processor_id());
> + trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());
> +
> out:
> local_fiq_enable();
> local_irq_enable();
> diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
> index 6527ec3..73cbe9a 100644
> --- a/arch/arm/mach-omap2/powerdomain.c
> +++ b/arch/arm/mach-omap2/powerdomain.c
> @@ -23,6 +23,7 @@
> #include <linux/errno.h>
> #include <linux/err.h>
> #include <linux/io.h>
> +#include <trace/events/power.h>
>
> #include <asm/atomic.h>
>
> @@ -440,6 +441,8 @@ int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
> pr_debug("powerdomain: setting next powerstate for %s to %0x\n",
> pwrdm->name, pwrst);
>
> + trace_power_domain_target(pwrdm->name, pwrst, smp_processor_id());
> +
> prm_rmw_mod_reg_bits(OMAP_POWERSTATE_MASK,
> (pwrst << OMAP_POWERSTATE_SHIFT),
> pwrdm->prcm_offs, pwrstctrl_reg_offs);
> diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
> index fc62fb5..7cbb09b 100644
> --- a/arch/arm/plat-omap/clock.c
> +++ b/arch/arm/plat-omap/clock.c
> @@ -21,6 +21,7 @@
> #include <linux/cpufreq.h>
> #include <linux/debugfs.h>
> #include <linux/io.h>
> +#include <trace/events/power.h>
>
> #include <plat/clock.h>
>
> @@ -43,8 +44,10 @@ int clk_enable(struct clk *clk)
> return -EINVAL;
>
> spin_lock_irqsave(&clockfw_lock, flags);
> - if (arch_clock->clk_enable)
> + if (arch_clock->clk_enable) {
> + trace_clock_enable(clk->name, 1, smp_processor_id());
> ret = arch_clock->clk_enable(clk);
> + }
> spin_unlock_irqrestore(&clockfw_lock, flags);
>
> return ret;
> @@ -66,8 +69,10 @@ void clk_disable(struct clk *clk)
> goto out;
> }
>
> - if (arch_clock->clk_disable)
> + if (arch_clock->clk_disable) {
> + trace_clock_disable(clk->name, 0, smp_processor_id());
> arch_clock->clk_disable(clk);
> + }
>
> out:
> spin_unlock_irqrestore(&clockfw_lock, flags);
> @@ -120,8 +125,10 @@ int clk_set_rate(struct clk *clk, unsigned long rate)
> return ret;
>
> spin_lock_irqsave(&clockfw_lock, flags);
> - if (arch_clock->clk_set_rate)
> + if (arch_clock->clk_set_rate) {
> + trace_clock_set_rate(clk->name, rate, smp_processor_id());
> ret = arch_clock->clk_set_rate(clk, rate);
> + }
> if (ret == 0) {
> if (clk->recalc)
> clk->rate = clk->recalc(clk);
> --
> 1.7.2.3
>
- Paul
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 2/3] perf: add OMAP support for the new power events
@ 2011-01-04 18:48 ` Paul Walmsley
0 siblings, 0 replies; 40+ messages in thread
From: Paul Walmsley @ 2011-01-04 18:48 UTC (permalink / raw)
To: linux-arm-kernel
Hello Jean,
On Tue, 4 Jan 2011, jean.pihet at newoldbits.com wrote:
> From: Jean Pihet <j-pihet@ti.com>
>
> The patch adds the new power management trace points for
> the OMAP architecture.
>
> The trace points are for:
> - default idle handler. Since the cpuidle framework is
> instrumented in the generic way there is no need to
> add trace points in the OMAP specific cpuidle handler;
> - cpufreq (DVFS),
> - clocks changes (enable, disable, set_rate),
A question about these. Are these only meant to track calls to these
functions from outside the clock code? Or meant to track actual hardware
clock changes? If the latter, then it might make sense to put these
trace points into the functions that actually change the hardware
registers, e.g., omap2_dflt_clk_{enable,disable}(), etc., since a
clk_enable() on a leaf clock may result in many internal system clocks
being enabled up the clock tree.
- Paul
> - change of power domains next power states.
>
> Signed-off-by: Jean Pihet <j-pihet@ti.com>
> ---
> arch/arm/mach-omap2/pm34xx.c | 7 +++++++
> arch/arm/mach-omap2/powerdomain.c | 3 +++
> arch/arm/plat-omap/clock.c | 13 ++++++++++---
> 3 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 0ec8a04..0ee0b0e 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -29,6 +29,7 @@
> #include <linux/delay.h>
> #include <linux/slab.h>
> #include <linux/console.h>
> +#include <trace/events/power.h>
>
> #include <plat/sram.h>
> #include <plat/clockdomain.h>
> @@ -506,8 +507,14 @@ static void omap3_pm_idle(void)
> if (omap_irq_pending() || need_resched())
> goto out;
>
> + trace_power_start(POWER_CSTATE, 1, smp_processor_id());
> + trace_cpu_idle(1, smp_processor_id());
> +
> omap_sram_idle();
>
> + trace_power_end(smp_processor_id());
> + trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());
> +
> out:
> local_fiq_enable();
> local_irq_enable();
> diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
> index 6527ec3..73cbe9a 100644
> --- a/arch/arm/mach-omap2/powerdomain.c
> +++ b/arch/arm/mach-omap2/powerdomain.c
> @@ -23,6 +23,7 @@
> #include <linux/errno.h>
> #include <linux/err.h>
> #include <linux/io.h>
> +#include <trace/events/power.h>
>
> #include <asm/atomic.h>
>
> @@ -440,6 +441,8 @@ int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
> pr_debug("powerdomain: setting next powerstate for %s to %0x\n",
> pwrdm->name, pwrst);
>
> + trace_power_domain_target(pwrdm->name, pwrst, smp_processor_id());
> +
> prm_rmw_mod_reg_bits(OMAP_POWERSTATE_MASK,
> (pwrst << OMAP_POWERSTATE_SHIFT),
> pwrdm->prcm_offs, pwrstctrl_reg_offs);
> diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
> index fc62fb5..7cbb09b 100644
> --- a/arch/arm/plat-omap/clock.c
> +++ b/arch/arm/plat-omap/clock.c
> @@ -21,6 +21,7 @@
> #include <linux/cpufreq.h>
> #include <linux/debugfs.h>
> #include <linux/io.h>
> +#include <trace/events/power.h>
>
> #include <plat/clock.h>
>
> @@ -43,8 +44,10 @@ int clk_enable(struct clk *clk)
> return -EINVAL;
>
> spin_lock_irqsave(&clockfw_lock, flags);
> - if (arch_clock->clk_enable)
> + if (arch_clock->clk_enable) {
> + trace_clock_enable(clk->name, 1, smp_processor_id());
> ret = arch_clock->clk_enable(clk);
> + }
> spin_unlock_irqrestore(&clockfw_lock, flags);
>
> return ret;
> @@ -66,8 +69,10 @@ void clk_disable(struct clk *clk)
> goto out;
> }
>
> - if (arch_clock->clk_disable)
> + if (arch_clock->clk_disable) {
> + trace_clock_disable(clk->name, 0, smp_processor_id());
> arch_clock->clk_disable(clk);
> + }
>
> out:
> spin_unlock_irqrestore(&clockfw_lock, flags);
> @@ -120,8 +125,10 @@ int clk_set_rate(struct clk *clk, unsigned long rate)
> return ret;
>
> spin_lock_irqsave(&clockfw_lock, flags);
> - if (arch_clock->clk_set_rate)
> + if (arch_clock->clk_set_rate) {
> + trace_clock_set_rate(clk->name, rate, smp_processor_id());
> ret = arch_clock->clk_set_rate(clk, rate);
> + }
> if (ret == 0) {
> if (clk->recalc)
> clk->rate = clk->recalc(clk);
> --
> 1.7.2.3
>
- Paul
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 1/3] perf: add calls to suspend trace point
2011-01-04 15:00 ` Jean Pihet
(?)
@ 2011-01-04 23:01 ` Rafael J. Wysocki
2011-01-05 10:09 ` Jean Pihet
2011-01-05 10:09 ` Jean Pihet
-1 siblings, 2 replies; 40+ messages in thread
From: Rafael J. Wysocki @ 2011-01-04 23:01 UTC (permalink / raw)
To: Jean Pihet
Cc: Pavel Machek, mingo, trenn, Len Brown, linux-pm, linux-kernel,
linux-omap, Arjan van de Ven, linux-perf-users, Jean Pihet
On Tuesday, January 04, 2011, Jean Pihet wrote:
> Hi,
>
> On Tue, Jan 4, 2011 at 12:29 PM, Pavel Machek <pavel@ucw.cz> wrote:
> > Hi!
> >
> >> Uses the machine_suspend trace point, called from the
> >> generic kernel suspend_enter function.
> >>
> >> Signed-off-by: Jean Pihet <j-pihet@ti.com>
> >> CC: Thomas Renninger <trenn@suse.de>
> >> ---
> >> kernel/power/suspend.c | 3 +++
> >> 1 files changed, 3 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
> >> index ecf7705..0650596 100644
> >> --- a/kernel/power/suspend.c
> >> +++ b/kernel/power/suspend.c
> >> @@ -22,6 +22,7 @@
> >> #include <linux/mm.h>
> >> #include <linux/slab.h>
> >> #include <linux/suspend.h>
> >> +#include <trace/events/power.h>
> >>
> >> #include "power.h"
> >>
> >> @@ -164,7 +165,9 @@ static int suspend_enter(suspend_state_t state)
> >> error = sysdev_suspend(PMSG_SUSPEND);
> >> if (!error) {
> >> if (!suspend_test(TEST_CORE) && pm_check_wakeup_events()) {
> >> + trace_machine_suspend(state);
> >> error = suspend_ops->enter(state);
> >> + trace_machine_suspend(PWR_EVENT_EXIT);
> >> events_check_enabled = false;
> >> }
> >> sysdev_resume();
> >
> > Ok... why this place?
> This trace has been placed here because it traces the machine low
> level mode enter.
>
> > I mean, perhaps suspend time should include
> > device suspend?
> That makes sense. We have a few options here:
> 1) keep the traces as proposed to trace the low level machine code only,
> 2) move the traces to the entry and exit of suspend_enter so that it
> includes the prepare and late_prepare (+ the associated wake-up)
> callbacks as well,
> 3) move the traces to suspend_devices_and_enter so that it includes 2)
> and the handling of the console and the devices,
> 4) move the traces to enter_state do that it includes 3), the call to
> sys_sync and the user space freeze.
>
> Note that the the SNAPSHOT_2RAM ioctl code also calls
> suspend_devices_and_enter, so if only 4) is used no trace will be
> generated in that case.
>
> I am in favor of 3) of 4).
> What do you think?
Why don't we keep the tracepoints as proposed _and_ add two additional
tracepoints around device suspend-resume?
Rafael
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 1/3] perf: add calls to suspend trace point
2011-01-04 15:00 ` Jean Pihet
(?)
(?)
@ 2011-01-04 23:01 ` Rafael J. Wysocki
-1 siblings, 0 replies; 40+ messages in thread
From: Rafael J. Wysocki @ 2011-01-04 23:01 UTC (permalink / raw)
To: Jean Pihet
Cc: Len Brown, linux-kernel, Jean Pihet, linux-perf-users, linux-pm,
mingo, linux-omap, Arjan van de Ven
On Tuesday, January 04, 2011, Jean Pihet wrote:
> Hi,
>
> On Tue, Jan 4, 2011 at 12:29 PM, Pavel Machek <pavel@ucw.cz> wrote:
> > Hi!
> >
> >> Uses the machine_suspend trace point, called from the
> >> generic kernel suspend_enter function.
> >>
> >> Signed-off-by: Jean Pihet <j-pihet@ti.com>
> >> CC: Thomas Renninger <trenn@suse.de>
> >> ---
> >> kernel/power/suspend.c | 3 +++
> >> 1 files changed, 3 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
> >> index ecf7705..0650596 100644
> >> --- a/kernel/power/suspend.c
> >> +++ b/kernel/power/suspend.c
> >> @@ -22,6 +22,7 @@
> >> #include <linux/mm.h>
> >> #include <linux/slab.h>
> >> #include <linux/suspend.h>
> >> +#include <trace/events/power.h>
> >>
> >> #include "power.h"
> >>
> >> @@ -164,7 +165,9 @@ static int suspend_enter(suspend_state_t state)
> >> error = sysdev_suspend(PMSG_SUSPEND);
> >> if (!error) {
> >> if (!suspend_test(TEST_CORE) && pm_check_wakeup_events()) {
> >> + trace_machine_suspend(state);
> >> error = suspend_ops->enter(state);
> >> + trace_machine_suspend(PWR_EVENT_EXIT);
> >> events_check_enabled = false;
> >> }
> >> sysdev_resume();
> >
> > Ok... why this place?
> This trace has been placed here because it traces the machine low
> level mode enter.
>
> > I mean, perhaps suspend time should include
> > device suspend?
> That makes sense. We have a few options here:
> 1) keep the traces as proposed to trace the low level machine code only,
> 2) move the traces to the entry and exit of suspend_enter so that it
> includes the prepare and late_prepare (+ the associated wake-up)
> callbacks as well,
> 3) move the traces to suspend_devices_and_enter so that it includes 2)
> and the handling of the console and the devices,
> 4) move the traces to enter_state do that it includes 3), the call to
> sys_sync and the user space freeze.
>
> Note that the the SNAPSHOT_2RAM ioctl code also calls
> suspend_devices_and_enter, so if only 4) is used no trace will be
> generated in that case.
>
> I am in favor of 3) of 4).
> What do you think?
Why don't we keep the tracepoints as proposed _and_ add two additional
tracepoints around device suspend-resume?
Rafael
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 1/3] perf: add calls to suspend trace point
2011-01-04 23:01 ` Rafael J. Wysocki
@ 2011-01-05 10:09 ` Jean Pihet
2011-01-05 10:15 ` Rafael J. Wysocki
2011-01-05 10:15 ` Rafael J. Wysocki
2011-01-05 10:09 ` Jean Pihet
1 sibling, 2 replies; 40+ messages in thread
From: Jean Pihet @ 2011-01-05 10:09 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Pavel Machek, mingo, trenn, Len Brown, linux-pm, linux-kernel,
linux-omap, Arjan van de Ven, linux-perf-users, Jean Pihet
On Wed, Jan 5, 2011 at 12:01 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Tuesday, January 04, 2011, Jean Pihet wrote:
>> Hi,
>>
>> On Tue, Jan 4, 2011 at 12:29 PM, Pavel Machek <pavel@ucw.cz> wrote:
>> > Hi!
>> >
>> >> Uses the machine_suspend trace point, called from the
>> >> generic kernel suspend_enter function.
>> >>
>> >> Signed-off-by: Jean Pihet <j-pihet@ti.com>
>> >> CC: Thomas Renninger <trenn@suse.de>
>> >> ---
>> >> kernel/power/suspend.c | 3 +++
>> >> 1 files changed, 3 insertions(+), 0 deletions(-)
>> >>
>> >> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
>> >> index ecf7705..0650596 100644
>> >> --- a/kernel/power/suspend.c
>> >> +++ b/kernel/power/suspend.c
>> >> @@ -22,6 +22,7 @@
>> >> #include <linux/mm.h>
>> >> #include <linux/slab.h>
>> >> #include <linux/suspend.h>
>> >> +#include <trace/events/power.h>
>> >>
>> >> #include "power.h"
>> >>
>> >> @@ -164,7 +165,9 @@ static int suspend_enter(suspend_state_t state)
>> >> error = sysdev_suspend(PMSG_SUSPEND);
>> >> if (!error) {
>> >> if (!suspend_test(TEST_CORE) && pm_check_wakeup_events()) {
>> >> + trace_machine_suspend(state);
>> >> error = suspend_ops->enter(state);
>> >> + trace_machine_suspend(PWR_EVENT_EXIT);
>> >> events_check_enabled = false;
>> >> }
>> >> sysdev_resume();
>> >
>> > Ok... why this place?
>> This trace has been placed here because it traces the machine low
>> level mode enter.
>>
>> > I mean, perhaps suspend time should include
>> > device suspend?
>> That makes sense. We have a few options here:
>> 1) keep the traces as proposed to trace the low level machine code only,
>> 2) move the traces to the entry and exit of suspend_enter so that it
>> includes the prepare and late_prepare (+ the associated wake-up)
>> callbacks as well,
>> 3) move the traces to suspend_devices_and_enter so that it includes 2)
>> and the handling of the console and the devices,
>> 4) move the traces to enter_state do that it includes 3), the call to
>> sys_sync and the user space freeze.
>>
>> Note that the the SNAPSHOT_2RAM ioctl code also calls
>> suspend_devices_and_enter, so if only 4) is used no trace will be
>> generated in that case.
>>
>> I am in favor of 3) of 4).
>> What do you think?
>
> Why don't we keep the tracepoints as proposed _and_ add two additional
> tracepoints around device suspend-resume?
I like the idea but that requires to extend the current API with
additional suspend tracepoints or device state change tracepoints.
That can be done once the current API is firmly in place.
Today the only trace API for suspend is machine_suspend(unsigned int
state), so I think the best option is 3) here above.
Unless there is an objection I am pushing 3) asap.
Thanks!
Jean
>
> Rafael
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 1/3] perf: add calls to suspend trace point
2011-01-04 23:01 ` Rafael J. Wysocki
2011-01-05 10:09 ` Jean Pihet
@ 2011-01-05 10:09 ` Jean Pihet
1 sibling, 0 replies; 40+ messages in thread
From: Jean Pihet @ 2011-01-05 10:09 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Len Brown, linux-kernel, Jean Pihet, linux-perf-users, linux-pm,
mingo, linux-omap, Arjan van de Ven
On Wed, Jan 5, 2011 at 12:01 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Tuesday, January 04, 2011, Jean Pihet wrote:
>> Hi,
>>
>> On Tue, Jan 4, 2011 at 12:29 PM, Pavel Machek <pavel@ucw.cz> wrote:
>> > Hi!
>> >
>> >> Uses the machine_suspend trace point, called from the
>> >> generic kernel suspend_enter function.
>> >>
>> >> Signed-off-by: Jean Pihet <j-pihet@ti.com>
>> >> CC: Thomas Renninger <trenn@suse.de>
>> >> ---
>> >> kernel/power/suspend.c | 3 +++
>> >> 1 files changed, 3 insertions(+), 0 deletions(-)
>> >>
>> >> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
>> >> index ecf7705..0650596 100644
>> >> --- a/kernel/power/suspend.c
>> >> +++ b/kernel/power/suspend.c
>> >> @@ -22,6 +22,7 @@
>> >> #include <linux/mm.h>
>> >> #include <linux/slab.h>
>> >> #include <linux/suspend.h>
>> >> +#include <trace/events/power.h>
>> >>
>> >> #include "power.h"
>> >>
>> >> @@ -164,7 +165,9 @@ static int suspend_enter(suspend_state_t state)
>> >> error = sysdev_suspend(PMSG_SUSPEND);
>> >> if (!error) {
>> >> if (!suspend_test(TEST_CORE) && pm_check_wakeup_events()) {
>> >> + trace_machine_suspend(state);
>> >> error = suspend_ops->enter(state);
>> >> + trace_machine_suspend(PWR_EVENT_EXIT);
>> >> events_check_enabled = false;
>> >> }
>> >> sysdev_resume();
>> >
>> > Ok... why this place?
>> This trace has been placed here because it traces the machine low
>> level mode enter.
>>
>> > I mean, perhaps suspend time should include
>> > device suspend?
>> That makes sense. We have a few options here:
>> 1) keep the traces as proposed to trace the low level machine code only,
>> 2) move the traces to the entry and exit of suspend_enter so that it
>> includes the prepare and late_prepare (+ the associated wake-up)
>> callbacks as well,
>> 3) move the traces to suspend_devices_and_enter so that it includes 2)
>> and the handling of the console and the devices,
>> 4) move the traces to enter_state do that it includes 3), the call to
>> sys_sync and the user space freeze.
>>
>> Note that the the SNAPSHOT_2RAM ioctl code also calls
>> suspend_devices_and_enter, so if only 4) is used no trace will be
>> generated in that case.
>>
>> I am in favor of 3) of 4).
>> What do you think?
>
> Why don't we keep the tracepoints as proposed _and_ add two additional
> tracepoints around device suspend-resume?
I like the idea but that requires to extend the current API with
additional suspend tracepoints or device state change tracepoints.
That can be done once the current API is firmly in place.
Today the only trace API for suspend is machine_suspend(unsigned int
state), so I think the best option is 3) here above.
Unless there is an objection I am pushing 3) asap.
Thanks!
Jean
>
> Rafael
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 1/3] perf: add calls to suspend trace point
2011-01-05 10:09 ` Jean Pihet
@ 2011-01-05 10:15 ` Rafael J. Wysocki
2011-01-05 11:23 ` Pavel Machek
2011-01-05 11:23 ` Pavel Machek
2011-01-05 10:15 ` Rafael J. Wysocki
1 sibling, 2 replies; 40+ messages in thread
From: Rafael J. Wysocki @ 2011-01-05 10:15 UTC (permalink / raw)
To: Jean Pihet
Cc: Pavel Machek, mingo, trenn, Len Brown, linux-pm, linux-kernel,
linux-omap, Arjan van de Ven, linux-perf-users, Jean Pihet
On Wednesday, January 05, 2011, Jean Pihet wrote:
> On Wed, Jan 5, 2011 at 12:01 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > On Tuesday, January 04, 2011, Jean Pihet wrote:
> >> Hi,
> >>
> >> On Tue, Jan 4, 2011 at 12:29 PM, Pavel Machek <pavel@ucw.cz> wrote:
> >> > Hi!
> >> >
> >> >> Uses the machine_suspend trace point, called from the
> >> >> generic kernel suspend_enter function.
> >> >>
> >> >> Signed-off-by: Jean Pihet <j-pihet@ti.com>
> >> >> CC: Thomas Renninger <trenn@suse.de>
> >> >> ---
> >> >> kernel/power/suspend.c | 3 +++
> >> >> 1 files changed, 3 insertions(+), 0 deletions(-)
> >> >>
> >> >> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
> >> >> index ecf7705..0650596 100644
> >> >> --- a/kernel/power/suspend.c
> >> >> +++ b/kernel/power/suspend.c
> >> >> @@ -22,6 +22,7 @@
> >> >> #include <linux/mm.h>
> >> >> #include <linux/slab.h>
> >> >> #include <linux/suspend.h>
> >> >> +#include <trace/events/power.h>
> >> >>
> >> >> #include "power.h"
> >> >>
> >> >> @@ -164,7 +165,9 @@ static int suspend_enter(suspend_state_t state)
> >> >> error = sysdev_suspend(PMSG_SUSPEND);
> >> >> if (!error) {
> >> >> if (!suspend_test(TEST_CORE) && pm_check_wakeup_events()) {
> >> >> + trace_machine_suspend(state);
> >> >> error = suspend_ops->enter(state);
> >> >> + trace_machine_suspend(PWR_EVENT_EXIT);
> >> >> events_check_enabled = false;
> >> >> }
> >> >> sysdev_resume();
> >> >
> >> > Ok... why this place?
> >> This trace has been placed here because it traces the machine low
> >> level mode enter.
> >>
> >> > I mean, perhaps suspend time should include
> >> > device suspend?
> >> That makes sense. We have a few options here:
> >> 1) keep the traces as proposed to trace the low level machine code only,
> >> 2) move the traces to the entry and exit of suspend_enter so that it
> >> includes the prepare and late_prepare (+ the associated wake-up)
> >> callbacks as well,
> >> 3) move the traces to suspend_devices_and_enter so that it includes 2)
> >> and the handling of the console and the devices,
> >> 4) move the traces to enter_state do that it includes 3), the call to
> >> sys_sync and the user space freeze.
> >>
> >> Note that the the SNAPSHOT_2RAM ioctl code also calls
> >> suspend_devices_and_enter, so if only 4) is used no trace will be
> >> generated in that case.
> >>
> >> I am in favor of 3) of 4).
> >> What do you think?
> >
> > Why don't we keep the tracepoints as proposed _and_ add two additional
> > tracepoints around device suspend-resume?
> I like the idea but that requires to extend the current API with
> additional suspend tracepoints or device state change tracepoints.
> That can be done once the current API is firmly in place.
> Today the only trace API for suspend is machine_suspend(unsigned int
> state), so I think the best option is 3) here above.
>
> Unless there is an objection I am pushing 3) asap.
Fine by me.
Thanks,
Rafael
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 1/3] perf: add calls to suspend trace point
2011-01-05 10:09 ` Jean Pihet
2011-01-05 10:15 ` Rafael J. Wysocki
@ 2011-01-05 10:15 ` Rafael J. Wysocki
1 sibling, 0 replies; 40+ messages in thread
From: Rafael J. Wysocki @ 2011-01-05 10:15 UTC (permalink / raw)
To: Jean Pihet
Cc: Len Brown, linux-kernel, Jean Pihet, linux-perf-users, linux-pm,
mingo, linux-omap, Arjan van de Ven
On Wednesday, January 05, 2011, Jean Pihet wrote:
> On Wed, Jan 5, 2011 at 12:01 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > On Tuesday, January 04, 2011, Jean Pihet wrote:
> >> Hi,
> >>
> >> On Tue, Jan 4, 2011 at 12:29 PM, Pavel Machek <pavel@ucw.cz> wrote:
> >> > Hi!
> >> >
> >> >> Uses the machine_suspend trace point, called from the
> >> >> generic kernel suspend_enter function.
> >> >>
> >> >> Signed-off-by: Jean Pihet <j-pihet@ti.com>
> >> >> CC: Thomas Renninger <trenn@suse.de>
> >> >> ---
> >> >> kernel/power/suspend.c | 3 +++
> >> >> 1 files changed, 3 insertions(+), 0 deletions(-)
> >> >>
> >> >> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
> >> >> index ecf7705..0650596 100644
> >> >> --- a/kernel/power/suspend.c
> >> >> +++ b/kernel/power/suspend.c
> >> >> @@ -22,6 +22,7 @@
> >> >> #include <linux/mm.h>
> >> >> #include <linux/slab.h>
> >> >> #include <linux/suspend.h>
> >> >> +#include <trace/events/power.h>
> >> >>
> >> >> #include "power.h"
> >> >>
> >> >> @@ -164,7 +165,9 @@ static int suspend_enter(suspend_state_t state)
> >> >> error = sysdev_suspend(PMSG_SUSPEND);
> >> >> if (!error) {
> >> >> if (!suspend_test(TEST_CORE) && pm_check_wakeup_events()) {
> >> >> + trace_machine_suspend(state);
> >> >> error = suspend_ops->enter(state);
> >> >> + trace_machine_suspend(PWR_EVENT_EXIT);
> >> >> events_check_enabled = false;
> >> >> }
> >> >> sysdev_resume();
> >> >
> >> > Ok... why this place?
> >> This trace has been placed here because it traces the machine low
> >> level mode enter.
> >>
> >> > I mean, perhaps suspend time should include
> >> > device suspend?
> >> That makes sense. We have a few options here:
> >> 1) keep the traces as proposed to trace the low level machine code only,
> >> 2) move the traces to the entry and exit of suspend_enter so that it
> >> includes the prepare and late_prepare (+ the associated wake-up)
> >> callbacks as well,
> >> 3) move the traces to suspend_devices_and_enter so that it includes 2)
> >> and the handling of the console and the devices,
> >> 4) move the traces to enter_state do that it includes 3), the call to
> >> sys_sync and the user space freeze.
> >>
> >> Note that the the SNAPSHOT_2RAM ioctl code also calls
> >> suspend_devices_and_enter, so if only 4) is used no trace will be
> >> generated in that case.
> >>
> >> I am in favor of 3) of 4).
> >> What do you think?
> >
> > Why don't we keep the tracepoints as proposed _and_ add two additional
> > tracepoints around device suspend-resume?
> I like the idea but that requires to extend the current API with
> additional suspend tracepoints or device state change tracepoints.
> That can be done once the current API is firmly in place.
> Today the only trace API for suspend is machine_suspend(unsigned int
> state), so I think the best option is 3) here above.
>
> Unless there is an objection I am pushing 3) asap.
Fine by me.
Thanks,
Rafael
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 2/3] perf: add OMAP support for the new power events
2011-01-04 18:03 ` Nishanth Menon
@ 2011-01-05 10:56 ` Jean Pihet
0 siblings, 0 replies; 40+ messages in thread
From: Jean Pihet @ 2011-01-05 10:56 UTC (permalink / raw)
To: Nishanth Menon
Cc: mingo, linux-kernel, trenn, linux-omap, Arjan van de Ven,
linux-perf-users, rjw, Jean Pihet
On Tue, Jan 4, 2011 at 7:03 PM, Nishanth Menon <nm@ti.com> wrote:
> jean.pihet@newoldbits.com had written, on 01/04/2011 04:17 AM, the
> following:
> [..]
>>
>> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
>> index 0ec8a04..0ee0b0e 100644
>> --- a/arch/arm/mach-omap2/pm34xx.c
>> +++ b/arch/arm/mach-omap2/pm34xx.c
>> @@ -29,6 +29,7 @@
>> #include <linux/delay.h>
>> #include <linux/slab.h>
>> #include <linux/console.h>
>> +#include <trace/events/power.h>
>> #include <plat/sram.h>
>> #include <plat/clockdomain.h>
>> @@ -506,8 +507,14 @@ static void omap3_pm_idle(void)
>> if (omap_irq_pending() || need_resched())
>> goto out;
>> + trace_power_start(POWER_CSTATE, 1, smp_processor_id());
>> + trace_cpu_idle(1, smp_processor_id());
>> +
>> omap_sram_idle();
>> + trace_power_end(smp_processor_id());
>> + trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());
>
> Dumb question: it just tells me which C state was attempted - not if
> actually succeeded in hitting it rt? Does'nt this give us a false data?
Yes this tracks the idle requests only. The actual hit state might be
different depending on various factors: HW state, enabled clocks,
power domains dependencies etc.
Debugging the actual PM hit state requires other tools, which could
use the trace API. There is definitely more to come on that topic.
>
> [..]
>>
>> diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
>> index fc62fb5..7cbb09b 100644
>> --- a/arch/arm/plat-omap/clock.c
>> +++ b/arch/arm/plat-omap/clock.c
>
> (from an offline discussion on a related topic): Would it also be nice to
> hook on mach-omap2/clock.c points as well to hook on indirect changes?
> [..]
>
> --
> Regards,
> Nishanth Menon
>
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 2/3] perf: add OMAP support for the new power events
2011-01-04 18:48 ` Paul Walmsley
@ 2011-01-05 11:05 ` Jean Pihet
-1 siblings, 0 replies; 40+ messages in thread
From: Jean Pihet @ 2011-01-05 11:05 UTC (permalink / raw)
To: Paul Walmsley
Cc: mingo, trenn, Kevin Hilman, Tony Lindgren, Russell King,
linux-omap, linux-arm-kernel, linux-kernel, Arjan van de Ven,
linux-perf-users, rjw, Jean Pihet
Hi Paul,
On Tue, Jan 4, 2011 at 7:48 PM, Paul Walmsley <paul@pwsan.com> wrote:
> Hello Jean,
>
> On Tue, 4 Jan 2011, jean.pihet@newoldbits.com wrote:
>
>> From: Jean Pihet <j-pihet@ti.com>
>>
>> The patch adds the new power management trace points for
>> the OMAP architecture.
>>
>> The trace points are for:
>> - default idle handler. Since the cpuidle framework is
>> instrumented in the generic way there is no need to
>> add trace points in the OMAP specific cpuidle handler;
>> - cpufreq (DVFS),
>> - clocks changes (enable, disable, set_rate),
>
> A question about these. Are these only meant to track calls to these
> functions from outside the clock code? Or meant to track actual hardware
> clock changes?
The former: this is used to track the clock requests from outside the
clock framework.
> If the latter, then it might make sense to put these
> trace points into the functions that actually change the hardware
> registers, e.g., omap2_dflt_clk_{enable,disable}(), etc., since a
> clk_enable() on a leaf clock may result in many internal system clocks
> being enabled up the clock tree.
I agree with you it is better to track the actual clock changes instead.
I propose to move the tracepoints to omap2_clk_{enable...} which
enables all the clocks irrespectively of the installed handler.
Note about the clock handlers: omap2_dflt_clk_enable happens to be the
handler for all controllable clocks but could that change in the
future?
>
>
> - Paul
Thanks,
Jean
>
>> - change of power domains next power states.
>>
>> Signed-off-by: Jean Pihet <j-pihet@ti.com>
>> ---
>> arch/arm/mach-omap2/pm34xx.c | 7 +++++++
>> arch/arm/mach-omap2/powerdomain.c | 3 +++
>> arch/arm/plat-omap/clock.c | 13 ++++++++++---
>> 3 files changed, 20 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
>> index 0ec8a04..0ee0b0e 100644
>> --- a/arch/arm/mach-omap2/pm34xx.c
>> +++ b/arch/arm/mach-omap2/pm34xx.c
>> @@ -29,6 +29,7 @@
>> #include <linux/delay.h>
>> #include <linux/slab.h>
>> #include <linux/console.h>
>> +#include <trace/events/power.h>
>>
>> #include <plat/sram.h>
>> #include <plat/clockdomain.h>
>> @@ -506,8 +507,14 @@ static void omap3_pm_idle(void)
>> if (omap_irq_pending() || need_resched())
>> goto out;
>>
>> + trace_power_start(POWER_CSTATE, 1, smp_processor_id());
>> + trace_cpu_idle(1, smp_processor_id());
>> +
>> omap_sram_idle();
>>
>> + trace_power_end(smp_processor_id());
>> + trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());
>> +
>> out:
>> local_fiq_enable();
>> local_irq_enable();
>> diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
>> index 6527ec3..73cbe9a 100644
>> --- a/arch/arm/mach-omap2/powerdomain.c
>> +++ b/arch/arm/mach-omap2/powerdomain.c
>> @@ -23,6 +23,7 @@
>> #include <linux/errno.h>
>> #include <linux/err.h>
>> #include <linux/io.h>
>> +#include <trace/events/power.h>
>>
>> #include <asm/atomic.h>
>>
>> @@ -440,6 +441,8 @@ int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
>> pr_debug("powerdomain: setting next powerstate for %s to %0x\n",
>> pwrdm->name, pwrst);
>>
>> + trace_power_domain_target(pwrdm->name, pwrst, smp_processor_id());
>> +
>> prm_rmw_mod_reg_bits(OMAP_POWERSTATE_MASK,
>> (pwrst << OMAP_POWERSTATE_SHIFT),
>> pwrdm->prcm_offs, pwrstctrl_reg_offs);
>> diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
>> index fc62fb5..7cbb09b 100644
>> --- a/arch/arm/plat-omap/clock.c
>> +++ b/arch/arm/plat-omap/clock.c
>> @@ -21,6 +21,7 @@
>> #include <linux/cpufreq.h>
>> #include <linux/debugfs.h>
>> #include <linux/io.h>
>> +#include <trace/events/power.h>
>>
>> #include <plat/clock.h>
>>
>> @@ -43,8 +44,10 @@ int clk_enable(struct clk *clk)
>> return -EINVAL;
>>
>> spin_lock_irqsave(&clockfw_lock, flags);
>> - if (arch_clock->clk_enable)
>> + if (arch_clock->clk_enable) {
>> + trace_clock_enable(clk->name, 1, smp_processor_id());
>> ret = arch_clock->clk_enable(clk);
>> + }
>> spin_unlock_irqrestore(&clockfw_lock, flags);
>>
>> return ret;
>> @@ -66,8 +69,10 @@ void clk_disable(struct clk *clk)
>> goto out;
>> }
>>
>> - if (arch_clock->clk_disable)
>> + if (arch_clock->clk_disable) {
>> + trace_clock_disable(clk->name, 0, smp_processor_id());
>> arch_clock->clk_disable(clk);
>> + }
>>
>> out:
>> spin_unlock_irqrestore(&clockfw_lock, flags);
>> @@ -120,8 +125,10 @@ int clk_set_rate(struct clk *clk, unsigned long rate)
>> return ret;
>>
>> spin_lock_irqsave(&clockfw_lock, flags);
>> - if (arch_clock->clk_set_rate)
>> + if (arch_clock->clk_set_rate) {
>> + trace_clock_set_rate(clk->name, rate, smp_processor_id());
>> ret = arch_clock->clk_set_rate(clk, rate);
>> + }
>> if (ret == 0) {
>> if (clk->recalc)
>> clk->rate = clk->recalc(clk);
>> --
>> 1.7.2.3
>>
>
>
> - Paul
>
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 2/3] perf: add OMAP support for the new power events
@ 2011-01-05 11:05 ` Jean Pihet
0 siblings, 0 replies; 40+ messages in thread
From: Jean Pihet @ 2011-01-05 11:05 UTC (permalink / raw)
To: linux-arm-kernel
Hi Paul,
On Tue, Jan 4, 2011 at 7:48 PM, Paul Walmsley <paul@pwsan.com> wrote:
> Hello Jean,
>
> On Tue, 4 Jan 2011, jean.pihet at newoldbits.com wrote:
>
>> From: Jean Pihet <j-pihet@ti.com>
>>
>> The patch adds the new power management trace points for
>> the OMAP architecture.
>>
>> The trace points are for:
>> - default idle handler. Since the cpuidle framework is
>> ? instrumented in the generic way there is no need to
>> ? add trace points in the OMAP specific cpuidle handler;
>> - cpufreq (DVFS),
>> - clocks changes (enable, disable, set_rate),
>
> A question about these. ?Are these only meant to track calls to these
> functions from outside the clock code? ?Or meant to track actual hardware
> clock changes?
The former: this is used to track the clock requests from outside the
clock framework.
> If the latter, then it might make sense to put these
> trace points into the functions that actually change the hardware
> registers, e.g., omap2_dflt_clk_{enable,disable}(), etc., since a
> clk_enable() on a leaf clock may result in many internal system clocks
> being enabled up the clock tree.
I agree with you it is better to track the actual clock changes instead.
I propose to move the tracepoints to omap2_clk_{enable...} which
enables all the clocks irrespectively of the installed handler.
Note about the clock handlers: omap2_dflt_clk_enable happens to be the
handler for all controllable clocks but could that change in the
future?
>
>
> - Paul
Thanks,
Jean
>
>> - change of power domains next power states.
>>
>> Signed-off-by: Jean Pihet <j-pihet@ti.com>
>> ---
>> ?arch/arm/mach-omap2/pm34xx.c ? ? ?| ? ?7 +++++++
>> ?arch/arm/mach-omap2/powerdomain.c | ? ?3 +++
>> ?arch/arm/plat-omap/clock.c ? ? ? ?| ? 13 ++++++++++---
>> ?3 files changed, 20 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
>> index 0ec8a04..0ee0b0e 100644
>> --- a/arch/arm/mach-omap2/pm34xx.c
>> +++ b/arch/arm/mach-omap2/pm34xx.c
>> @@ -29,6 +29,7 @@
>> ?#include <linux/delay.h>
>> ?#include <linux/slab.h>
>> ?#include <linux/console.h>
>> +#include <trace/events/power.h>
>>
>> ?#include <plat/sram.h>
>> ?#include <plat/clockdomain.h>
>> @@ -506,8 +507,14 @@ static void omap3_pm_idle(void)
>> ? ? ? if (omap_irq_pending() || need_resched())
>> ? ? ? ? ? ? ? goto out;
>>
>> + ? ? trace_power_start(POWER_CSTATE, 1, smp_processor_id());
>> + ? ? trace_cpu_idle(1, smp_processor_id());
>> +
>> ? ? ? omap_sram_idle();
>>
>> + ? ? trace_power_end(smp_processor_id());
>> + ? ? trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());
>> +
>> ?out:
>> ? ? ? local_fiq_enable();
>> ? ? ? local_irq_enable();
>> diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
>> index 6527ec3..73cbe9a 100644
>> --- a/arch/arm/mach-omap2/powerdomain.c
>> +++ b/arch/arm/mach-omap2/powerdomain.c
>> @@ -23,6 +23,7 @@
>> ?#include <linux/errno.h>
>> ?#include <linux/err.h>
>> ?#include <linux/io.h>
>> +#include <trace/events/power.h>
>>
>> ?#include <asm/atomic.h>
>>
>> @@ -440,6 +441,8 @@ int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
>> ? ? ? pr_debug("powerdomain: setting next powerstate for %s to %0x\n",
>> ? ? ? ? ? ? ? ?pwrdm->name, pwrst);
>>
>> + ? ? trace_power_domain_target(pwrdm->name, pwrst, smp_processor_id());
>> +
>> ? ? ? prm_rmw_mod_reg_bits(OMAP_POWERSTATE_MASK,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ?(pwrst << OMAP_POWERSTATE_SHIFT),
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ?pwrdm->prcm_offs, pwrstctrl_reg_offs);
>> diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
>> index fc62fb5..7cbb09b 100644
>> --- a/arch/arm/plat-omap/clock.c
>> +++ b/arch/arm/plat-omap/clock.c
>> @@ -21,6 +21,7 @@
>> ?#include <linux/cpufreq.h>
>> ?#include <linux/debugfs.h>
>> ?#include <linux/io.h>
>> +#include <trace/events/power.h>
>>
>> ?#include <plat/clock.h>
>>
>> @@ -43,8 +44,10 @@ int clk_enable(struct clk *clk)
>> ? ? ? ? ? ? ? return -EINVAL;
>>
>> ? ? ? spin_lock_irqsave(&clockfw_lock, flags);
>> - ? ? if (arch_clock->clk_enable)
>> + ? ? if (arch_clock->clk_enable) {
>> + ? ? ? ? ? ? trace_clock_enable(clk->name, 1, smp_processor_id());
>> ? ? ? ? ? ? ? ret = arch_clock->clk_enable(clk);
>> + ? ? }
>> ? ? ? spin_unlock_irqrestore(&clockfw_lock, flags);
>>
>> ? ? ? return ret;
>> @@ -66,8 +69,10 @@ void clk_disable(struct clk *clk)
>> ? ? ? ? ? ? ? goto out;
>> ? ? ? }
>>
>> - ? ? if (arch_clock->clk_disable)
>> + ? ? if (arch_clock->clk_disable) {
>> + ? ? ? ? ? ? trace_clock_disable(clk->name, 0, smp_processor_id());
>> ? ? ? ? ? ? ? arch_clock->clk_disable(clk);
>> + ? ? }
>>
>> ?out:
>> ? ? ? spin_unlock_irqrestore(&clockfw_lock, flags);
>> @@ -120,8 +125,10 @@ int clk_set_rate(struct clk *clk, unsigned long rate)
>> ? ? ? ? ? ? ? return ret;
>>
>> ? ? ? spin_lock_irqsave(&clockfw_lock, flags);
>> - ? ? if (arch_clock->clk_set_rate)
>> + ? ? if (arch_clock->clk_set_rate) {
>> + ? ? ? ? ? ? trace_clock_set_rate(clk->name, rate, smp_processor_id());
>> ? ? ? ? ? ? ? ret = arch_clock->clk_set_rate(clk, rate);
>> + ? ? }
>> ? ? ? if (ret == 0) {
>> ? ? ? ? ? ? ? if (clk->recalc)
>> ? ? ? ? ? ? ? ? ? ? ? clk->rate = clk->recalc(clk);
>> --
>> 1.7.2.3
>>
>
>
> - Paul
>
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 1/3] perf: add calls to suspend trace point
2011-01-05 10:15 ` Rafael J. Wysocki
2011-01-05 11:23 ` Pavel Machek
@ 2011-01-05 11:23 ` Pavel Machek
1 sibling, 0 replies; 40+ messages in thread
From: Pavel Machek @ 2011-01-05 11:23 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Jean Pihet, mingo, trenn, Len Brown, linux-pm, linux-kernel,
linux-omap, Arjan van de Ven, linux-perf-users, Jean Pihet
Hi!
> > >> I am in favor of 3) of 4).
> > >> What do you think?
> > >
> > > Why don't we keep the tracepoints as proposed _and_ add two additional
> > > tracepoints around device suspend-resume?
> > I like the idea but that requires to extend the current API with
> > additional suspend tracepoints or device state change tracepoints.
> > That can be done once the current API is firmly in place.
> > Today the only trace API for suspend is machine_suspend(unsigned int
> > state), so I think the best option is 3) here above.
> >
> > Unless there is an objection I am pushing 3) asap.
>
> Fine by me.
Why not...
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 1/3] perf: add calls to suspend trace point
2011-01-05 10:15 ` Rafael J. Wysocki
@ 2011-01-05 11:23 ` Pavel Machek
2011-01-05 11:23 ` Pavel Machek
1 sibling, 0 replies; 40+ messages in thread
From: Pavel Machek @ 2011-01-05 11:23 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Len Brown, Jean Pihet, Jean Pihet, linux-kernel,
linux-perf-users, mingo, linux-omap, Arjan van de Ven, linux-pm
Hi!
> > >> I am in favor of 3) of 4).
> > >> What do you think?
> > >
> > > Why don't we keep the tracepoints as proposed _and_ add two additional
> > > tracepoints around device suspend-resume?
> > I like the idea but that requires to extend the current API with
> > additional suspend tracepoints or device state change tracepoints.
> > That can be done once the current API is firmly in place.
> > Today the only trace API for suspend is machine_suspend(unsigned int
> > state), so I think the best option is 3) here above.
> >
> > Unless there is an objection I am pushing 3) asap.
>
> Fine by me.
Why not...
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 2/3] perf: add OMAP support for the new power events
2011-01-05 11:05 ` Jean Pihet
@ 2011-01-10 14:14 ` Thomas Renninger
-1 siblings, 0 replies; 40+ messages in thread
From: Thomas Renninger @ 2011-01-10 14:14 UTC (permalink / raw)
To: Jean Pihet
Cc: Paul Walmsley, mingo, Kevin Hilman, Tony Lindgren, Russell King,
linux-omap, linux-arm-kernel, linux-kernel, Arjan van de Ven,
linux-perf-users, rjw, Jean Pihet
On Wednesday 05 January 2011 12:05:18 Jean Pihet wrote:
> Hi Paul,
>
> On Tue, Jan 4, 2011 at 7:48 PM, Paul Walmsley <paul@pwsan.com> wrote:
> > Hello Jean,
> >
> > On Tue, 4 Jan 2011, jean.pihet@newoldbits.com wrote:
> >
> >> From: Jean Pihet <j-pihet@ti.com>
> >>
> >> The patch adds the new power management trace points for
> >> the OMAP architecture.
> >>
> >> The trace points are for:
> >> - default idle handler. Since the cpuidle framework is
> >> instrumented in the generic way there is no need to
> >> add trace points in the OMAP specific cpuidle handler;
> >> - cpufreq (DVFS),
> >> - clocks changes (enable, disable, set_rate),
> >
> > A question about these. Are these only meant to track calls to these
> > functions from outside the clock code? Or meant to track actual hardware
> > clock changes?
> The former: this is used to track the clock requests from outside the
> clock framework.
>
> > If the latter, then it might make sense to put these
> > trace points into the functions that actually change the hardware
> > registers, e.g., omap2_dflt_clk_{enable,disable}(), etc., since a
> > clk_enable() on a leaf clock may result in many internal system clocks
> > being enabled up the clock tree.
> I agree with you it is better to track the actual clock changes instead.
> I propose to move the tracepoints to omap2_clk_{enable...} which
> enables all the clocks irrespectively of the installed handler.
> Note about the clock handlers: omap2_dflt_clk_enable happens to be the
> handler for all controllable clocks but could that change in the
> future?
Looks like there is cpuidle34xx using cpuidle framework on specific boards
only.
And pm34xx.c and others override pm_idle and use the same low level
functions to reduce power consumption as cpuidle34xx.
Ideally pm34xx.c (and others) would not override pm_idle, but register as
a cpuidle driver. Then the idle events would be tracked by the
cpuidle subsystem automatically (with my latest patches).
But this would be a more intrusive change (are there efforts to do that?).
Even if it should happen at some point, adding some additional events for
people to better debug/monitor the stuff now does not hurt.
Thomas
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 2/3] perf: add OMAP support for the new power events
@ 2011-01-10 14:14 ` Thomas Renninger
0 siblings, 0 replies; 40+ messages in thread
From: Thomas Renninger @ 2011-01-10 14:14 UTC (permalink / raw)
To: linux-arm-kernel
On Wednesday 05 January 2011 12:05:18 Jean Pihet wrote:
> Hi Paul,
>
> On Tue, Jan 4, 2011 at 7:48 PM, Paul Walmsley <paul@pwsan.com> wrote:
> > Hello Jean,
> >
> > On Tue, 4 Jan 2011, jean.pihet at newoldbits.com wrote:
> >
> >> From: Jean Pihet <j-pihet@ti.com>
> >>
> >> The patch adds the new power management trace points for
> >> the OMAP architecture.
> >>
> >> The trace points are for:
> >> - default idle handler. Since the cpuidle framework is
> >> instrumented in the generic way there is no need to
> >> add trace points in the OMAP specific cpuidle handler;
> >> - cpufreq (DVFS),
> >> - clocks changes (enable, disable, set_rate),
> >
> > A question about these. Are these only meant to track calls to these
> > functions from outside the clock code? Or meant to track actual hardware
> > clock changes?
> The former: this is used to track the clock requests from outside the
> clock framework.
>
> > If the latter, then it might make sense to put these
> > trace points into the functions that actually change the hardware
> > registers, e.g., omap2_dflt_clk_{enable,disable}(), etc., since a
> > clk_enable() on a leaf clock may result in many internal system clocks
> > being enabled up the clock tree.
> I agree with you it is better to track the actual clock changes instead.
> I propose to move the tracepoints to omap2_clk_{enable...} which
> enables all the clocks irrespectively of the installed handler.
> Note about the clock handlers: omap2_dflt_clk_enable happens to be the
> handler for all controllable clocks but could that change in the
> future?
Looks like there is cpuidle34xx using cpuidle framework on specific boards
only.
And pm34xx.c and others override pm_idle and use the same low level
functions to reduce power consumption as cpuidle34xx.
Ideally pm34xx.c (and others) would not override pm_idle, but register as
a cpuidle driver. Then the idle events would be tracked by the
cpuidle subsystem automatically (with my latest patches).
But this would be a more intrusive change (are there efforts to do that?).
Even if it should happen at some point, adding some additional events for
people to better debug/monitor the stuff now does not hurt.
Thomas
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 2/3] perf: add OMAP support for the new power events
2011-01-05 11:05 ` Jean Pihet
@ 2011-01-10 19:06 ` Paul Walmsley
-1 siblings, 0 replies; 40+ messages in thread
From: Paul Walmsley @ 2011-01-10 19:06 UTC (permalink / raw)
To: Jean Pihet
Cc: mingo, trenn, Kevin Hilman, Tony Lindgren, Russell King,
linux-omap, linux-arm-kernel, linux-kernel, Arjan van de Ven,
linux-perf-users, rjw, Jean Pihet
Hello Jean,
On Wed, 5 Jan 2011, Jean Pihet wrote:
> On Tue, Jan 4, 2011 at 7:48 PM, Paul Walmsley <paul@pwsan.com> wrote:
>
> > If the latter, then it might make sense to put these
> > trace points into the functions that actually change the hardware
> > registers, e.g., omap2_dflt_clk_{enable,disable}(), etc., since a
> > clk_enable() on a leaf clock may result in many internal system clocks
> > being enabled up the clock tree.
> I agree with you it is better to track the actual clock changes instead.
> I propose to move the tracepoints to omap2_clk_{enable...} which
> enables all the clocks irrespectively of the installed handler.
Yes, that is a better place.
> Note about the clock handlers: omap2_dflt_clk_enable happens to be the
> handler for all controllable clocks but could that change in the
> future?
Yes, it certainly could.
- Paul
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 2/3] perf: add OMAP support for the new power events
@ 2011-01-10 19:06 ` Paul Walmsley
0 siblings, 0 replies; 40+ messages in thread
From: Paul Walmsley @ 2011-01-10 19:06 UTC (permalink / raw)
To: linux-arm-kernel
Hello Jean,
On Wed, 5 Jan 2011, Jean Pihet wrote:
> On Tue, Jan 4, 2011 at 7:48 PM, Paul Walmsley <paul@pwsan.com> wrote:
>
> > If the latter, then it might make sense to put these
> > trace points into the functions that actually change the hardware
> > registers, e.g., omap2_dflt_clk_{enable,disable}(), etc., since a
> > clk_enable() on a leaf clock may result in many internal system clocks
> > being enabled up the clock tree.
> I agree with you it is better to track the actual clock changes instead.
> I propose to move the tracepoints to omap2_clk_{enable...} which
> enables all the clocks irrespectively of the installed handler.
Yes, that is a better place.
> Note about the clock handlers: omap2_dflt_clk_enable happens to be the
> handler for all controllable clocks but could that change in the
> future?
Yes, it certainly could.
- Paul
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 2/3] perf: add OMAP support for the new power events
2011-01-10 14:14 ` Thomas Renninger
@ 2011-01-11 1:24 ` Kevin Hilman
-1 siblings, 0 replies; 40+ messages in thread
From: Kevin Hilman @ 2011-01-11 1:24 UTC (permalink / raw)
To: Thomas Renninger
Cc: Jean Pihet, Paul Walmsley, mingo, Tony Lindgren, Russell King,
linux-omap, linux-arm-kernel, linux-kernel, Arjan van de Ven,
linux-perf-users, rjw, Jean Pihet
Thomas Renninger <trenn@suse.de> writes:
> On Wednesday 05 January 2011 12:05:18 Jean Pihet wrote:
>> Hi Paul,
>>
>> On Tue, Jan 4, 2011 at 7:48 PM, Paul Walmsley <paul@pwsan.com> wrote:
>> > Hello Jean,
>> >
>> > On Tue, 4 Jan 2011, jean.pihet@newoldbits.com wrote:
>> >
>> >> From: Jean Pihet <j-pihet@ti.com>
>> >>
>> >> The patch adds the new power management trace points for
>> >> the OMAP architecture.
>> >>
>> >> The trace points are for:
>> >> - default idle handler. Since the cpuidle framework is
>> >> instrumented in the generic way there is no need to
>> >> add trace points in the OMAP specific cpuidle handler;
>> >> - cpufreq (DVFS),
>> >> - clocks changes (enable, disable, set_rate),
>> >
>> > A question about these. Are these only meant to track calls to these
>> > functions from outside the clock code? Or meant to track actual hardware
>> > clock changes?
>> The former: this is used to track the clock requests from outside the
>> clock framework.
>>
>> > If the latter, then it might make sense to put these
>> > trace points into the functions that actually change the hardware
>> > registers, e.g., omap2_dflt_clk_{enable,disable}(), etc., since a
>> > clk_enable() on a leaf clock may result in many internal system clocks
>> > being enabled up the clock tree.
>> I agree with you it is better to track the actual clock changes instead.
>> I propose to move the tracepoints to omap2_clk_{enable...} which
>> enables all the clocks irrespectively of the installed handler.
>> Note about the clock handlers: omap2_dflt_clk_enable happens to be the
>> handler for all controllable clocks but could that change in the
>> future?
> Looks like there is cpuidle34xx using cpuidle framework on specific
> boards only. And pm34xx.c and others override pm_idle and use the
> same low level functions to reduce power consumption as cpuidle34xx.
> Ideally pm34xx.c (and others) would not override pm_idle, but register as
> a cpuidle driver. Then the idle events would be tracked by the
> cpuidle subsystem automatically (with my latest patches).
> But this would be a more intrusive change (are there efforts to do that?).
Whenever CPUidle is enabled though, the cpuidle34xx code is used so the
pm_idle in pm34xx is not used. This allows us to have a way to do PM
with and without CPUidle, so without CPUidle, we can still get some
basic PM in idle.
> Even if it should happen at some point, adding some additional events for
> people to better debug/monitor the stuff now does not hurt.
I agree, as the two both very useful.
Tracking CPUidle transitions gives us some high-level information on PM
transitions, but what is most interesting for real PM analysis on OMAP
is exactly which powerdomains, clockdomains and clocks transitions (or
didn't transition) for a given state.
Kevin
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 2/3] perf: add OMAP support for the new power events
@ 2011-01-11 1:24 ` Kevin Hilman
0 siblings, 0 replies; 40+ messages in thread
From: Kevin Hilman @ 2011-01-11 1:24 UTC (permalink / raw)
To: linux-arm-kernel
Thomas Renninger <trenn@suse.de> writes:
> On Wednesday 05 January 2011 12:05:18 Jean Pihet wrote:
>> Hi Paul,
>>
>> On Tue, Jan 4, 2011 at 7:48 PM, Paul Walmsley <paul@pwsan.com> wrote:
>> > Hello Jean,
>> >
>> > On Tue, 4 Jan 2011, jean.pihet at newoldbits.com wrote:
>> >
>> >> From: Jean Pihet <j-pihet@ti.com>
>> >>
>> >> The patch adds the new power management trace points for
>> >> the OMAP architecture.
>> >>
>> >> The trace points are for:
>> >> - default idle handler. Since the cpuidle framework is
>> >> instrumented in the generic way there is no need to
>> >> add trace points in the OMAP specific cpuidle handler;
>> >> - cpufreq (DVFS),
>> >> - clocks changes (enable, disable, set_rate),
>> >
>> > A question about these. Are these only meant to track calls to these
>> > functions from outside the clock code? Or meant to track actual hardware
>> > clock changes?
>> The former: this is used to track the clock requests from outside the
>> clock framework.
>>
>> > If the latter, then it might make sense to put these
>> > trace points into the functions that actually change the hardware
>> > registers, e.g., omap2_dflt_clk_{enable,disable}(), etc., since a
>> > clk_enable() on a leaf clock may result in many internal system clocks
>> > being enabled up the clock tree.
>> I agree with you it is better to track the actual clock changes instead.
>> I propose to move the tracepoints to omap2_clk_{enable...} which
>> enables all the clocks irrespectively of the installed handler.
>> Note about the clock handlers: omap2_dflt_clk_enable happens to be the
>> handler for all controllable clocks but could that change in the
>> future?
> Looks like there is cpuidle34xx using cpuidle framework on specific
> boards only. And pm34xx.c and others override pm_idle and use the
> same low level functions to reduce power consumption as cpuidle34xx.
> Ideally pm34xx.c (and others) would not override pm_idle, but register as
> a cpuidle driver. Then the idle events would be tracked by the
> cpuidle subsystem automatically (with my latest patches).
> But this would be a more intrusive change (are there efforts to do that?).
Whenever CPUidle is enabled though, the cpuidle34xx code is used so the
pm_idle in pm34xx is not used. This allows us to have a way to do PM
with and without CPUidle, so without CPUidle, we can still get some
basic PM in idle.
> Even if it should happen at some point, adding some additional events for
> people to better debug/monitor the stuff now does not hurt.
I agree, as the two both very useful.
Tracking CPUidle transitions gives us some high-level information on PM
transitions, but what is most interesting for real PM analysis on OMAP
is exactly which powerdomains, clockdomains and clocks transitions (or
didn't transition) for a given state.
Kevin
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 2/3] perf: add OMAP support for the new power events
2010-12-10 19:51 [PATCH 0/3] perf, tools: new power trace API jean.pihet
@ 2010-12-10 19:52 ` jean.pihet
2010-12-10 19:52 ` jean.pihet
1 sibling, 0 replies; 40+ messages in thread
From: jean.pihet @ 2010-12-10 19:52 UTC (permalink / raw)
To: Ingo Molnar
Cc: Thomas Renninger, linux-pm, Arjan van de Ven, linux-kernel,
linux-omap, Jean Pihet
From: Jean Pihet <j-pihet@ti.com>
The patch adds the new power management trace points for
the OMAP architecture.
The trace points are for:
- default idle handler. Since the cpuidle framework is
instrumented in the generic way there is no need to
add trace points in the OMAP specific cpuidle handler;
- cpufreq (DVFS),
- clocks changes (enable, disable, set_rate),
- change of power domains next power states.
Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
arch/arm/mach-omap2/pm34xx.c | 7 +++++++
arch/arm/mach-omap2/powerdomain.c | 3 +++
arch/arm/plat-omap/clock.c | 13 ++++++++++---
3 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 0ec8a04..0ee0b0e 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -29,6 +29,7 @@
#include <linux/delay.h>
#include <linux/slab.h>
#include <linux/console.h>
+#include <trace/events/power.h>
#include <plat/sram.h>
#include <plat/clockdomain.h>
@@ -506,8 +507,14 @@ static void omap3_pm_idle(void)
if (omap_irq_pending() || need_resched())
goto out;
+ trace_power_start(POWER_CSTATE, 1, smp_processor_id());
+ trace_cpu_idle(1, smp_processor_id());
+
omap_sram_idle();
+ trace_power_end(smp_processor_id());
+ trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());
+
out:
local_fiq_enable();
local_irq_enable();
diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index 6527ec3..73cbe9a 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -23,6 +23,7 @@
#include <linux/errno.h>
#include <linux/err.h>
#include <linux/io.h>
+#include <trace/events/power.h>
#include <asm/atomic.h>
@@ -440,6 +441,8 @@ int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
pr_debug("powerdomain: setting next powerstate for %s to %0x\n",
pwrdm->name, pwrst);
+ trace_power_domain_target(pwrdm->name, pwrst, smp_processor_id());
+
prm_rmw_mod_reg_bits(OMAP_POWERSTATE_MASK,
(pwrst << OMAP_POWERSTATE_SHIFT),
pwrdm->prcm_offs, pwrstctrl_reg_offs);
diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
index fc62fb5..7cbb09b 100644
--- a/arch/arm/plat-omap/clock.c
+++ b/arch/arm/plat-omap/clock.c
@@ -21,6 +21,7 @@
#include <linux/cpufreq.h>
#include <linux/debugfs.h>
#include <linux/io.h>
+#include <trace/events/power.h>
#include <plat/clock.h>
@@ -43,8 +44,10 @@ int clk_enable(struct clk *clk)
return -EINVAL;
spin_lock_irqsave(&clockfw_lock, flags);
- if (arch_clock->clk_enable)
+ if (arch_clock->clk_enable) {
+ trace_clock_enable(clk->name, 1, smp_processor_id());
ret = arch_clock->clk_enable(clk);
+ }
spin_unlock_irqrestore(&clockfw_lock, flags);
return ret;
@@ -66,8 +69,10 @@ void clk_disable(struct clk *clk)
goto out;
}
- if (arch_clock->clk_disable)
+ if (arch_clock->clk_disable) {
+ trace_clock_disable(clk->name, 0, smp_processor_id());
arch_clock->clk_disable(clk);
+ }
out:
spin_unlock_irqrestore(&clockfw_lock, flags);
@@ -120,8 +125,10 @@ int clk_set_rate(struct clk *clk, unsigned long rate)
return ret;
spin_lock_irqsave(&clockfw_lock, flags);
- if (arch_clock->clk_set_rate)
+ if (arch_clock->clk_set_rate) {
+ trace_clock_set_rate(clk->name, rate, smp_processor_id());
ret = arch_clock->clk_set_rate(clk, rate);
+ }
if (ret == 0) {
if (clk->recalc)
clk->rate = clk->recalc(clk);
--
1.7.2.3
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH 2/3] perf: add OMAP support for the new power events
2010-12-10 19:51 [PATCH 0/3] perf, tools: new power trace API jean.pihet
2010-12-10 19:52 ` [PATCH 2/3] perf: add OMAP support for the new power events jean.pihet
@ 2010-12-10 19:52 ` jean.pihet
1 sibling, 0 replies; 40+ messages in thread
From: jean.pihet @ 2010-12-10 19:52 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, linux-pm, linux-omap, Arjan van de Ven, Jean Pihet
From: Jean Pihet <j-pihet@ti.com>
The patch adds the new power management trace points for
the OMAP architecture.
The trace points are for:
- default idle handler. Since the cpuidle framework is
instrumented in the generic way there is no need to
add trace points in the OMAP specific cpuidle handler;
- cpufreq (DVFS),
- clocks changes (enable, disable, set_rate),
- change of power domains next power states.
Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
arch/arm/mach-omap2/pm34xx.c | 7 +++++++
arch/arm/mach-omap2/powerdomain.c | 3 +++
arch/arm/plat-omap/clock.c | 13 ++++++++++---
3 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 0ec8a04..0ee0b0e 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -29,6 +29,7 @@
#include <linux/delay.h>
#include <linux/slab.h>
#include <linux/console.h>
+#include <trace/events/power.h>
#include <plat/sram.h>
#include <plat/clockdomain.h>
@@ -506,8 +507,14 @@ static void omap3_pm_idle(void)
if (omap_irq_pending() || need_resched())
goto out;
+ trace_power_start(POWER_CSTATE, 1, smp_processor_id());
+ trace_cpu_idle(1, smp_processor_id());
+
omap_sram_idle();
+ trace_power_end(smp_processor_id());
+ trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());
+
out:
local_fiq_enable();
local_irq_enable();
diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index 6527ec3..73cbe9a 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -23,6 +23,7 @@
#include <linux/errno.h>
#include <linux/err.h>
#include <linux/io.h>
+#include <trace/events/power.h>
#include <asm/atomic.h>
@@ -440,6 +441,8 @@ int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
pr_debug("powerdomain: setting next powerstate for %s to %0x\n",
pwrdm->name, pwrst);
+ trace_power_domain_target(pwrdm->name, pwrst, smp_processor_id());
+
prm_rmw_mod_reg_bits(OMAP_POWERSTATE_MASK,
(pwrst << OMAP_POWERSTATE_SHIFT),
pwrdm->prcm_offs, pwrstctrl_reg_offs);
diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
index fc62fb5..7cbb09b 100644
--- a/arch/arm/plat-omap/clock.c
+++ b/arch/arm/plat-omap/clock.c
@@ -21,6 +21,7 @@
#include <linux/cpufreq.h>
#include <linux/debugfs.h>
#include <linux/io.h>
+#include <trace/events/power.h>
#include <plat/clock.h>
@@ -43,8 +44,10 @@ int clk_enable(struct clk *clk)
return -EINVAL;
spin_lock_irqsave(&clockfw_lock, flags);
- if (arch_clock->clk_enable)
+ if (arch_clock->clk_enable) {
+ trace_clock_enable(clk->name, 1, smp_processor_id());
ret = arch_clock->clk_enable(clk);
+ }
spin_unlock_irqrestore(&clockfw_lock, flags);
return ret;
@@ -66,8 +69,10 @@ void clk_disable(struct clk *clk)
goto out;
}
- if (arch_clock->clk_disable)
+ if (arch_clock->clk_disable) {
+ trace_clock_disable(clk->name, 0, smp_processor_id());
arch_clock->clk_disable(clk);
+ }
out:
spin_unlock_irqrestore(&clockfw_lock, flags);
@@ -120,8 +125,10 @@ int clk_set_rate(struct clk *clk, unsigned long rate)
return ret;
spin_lock_irqsave(&clockfw_lock, flags);
- if (arch_clock->clk_set_rate)
+ if (arch_clock->clk_set_rate) {
+ trace_clock_set_rate(clk->name, rate, smp_processor_id());
ret = arch_clock->clk_set_rate(clk, rate);
+ }
if (ret == 0) {
if (clk->recalc)
clk->rate = clk->recalc(clk);
--
1.7.2.3
^ permalink raw reply related [flat|nested] 40+ messages in thread
end of thread, other threads:[~2011-01-11 1:24 UTC | newest]
Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-04 10:17 [PATCH 0/3] perf, tools: new power trace API jean.pihet
2011-01-04 10:17 ` [PATCH 1/3] perf: add calls to suspend trace point jean.pihet
2011-01-04 10:39 ` Ingo Molnar
2011-01-04 10:17 ` [PATCH 2/3] perf: add OMAP support for the new power events jean.pihet
2011-01-04 10:42 ` Ingo Molnar
2011-01-04 10:58 ` Pihet-XID, Jean
2011-01-04 18:03 ` Nishanth Menon
2011-01-05 10:56 ` Jean Pihet
2011-01-04 10:17 ` [PATCH 3/3] tools, perf: Documentation for the power events API jean.pihet
2011-01-04 10:48 ` [PATCH 1/3] perf: add calls to suspend trace point jean.pihet
2011-01-04 10:48 ` jean.pihet
2011-01-04 11:29 ` Pavel Machek
2011-01-04 11:29 ` Pavel Machek
2011-01-04 15:00 ` Jean Pihet
2011-01-04 15:00 ` Jean Pihet
2011-01-04 23:01 ` Rafael J. Wysocki
2011-01-05 10:09 ` Jean Pihet
2011-01-05 10:15 ` Rafael J. Wysocki
2011-01-05 11:23 ` Pavel Machek
2011-01-05 11:23 ` Pavel Machek
2011-01-05 10:15 ` Rafael J. Wysocki
2011-01-05 10:09 ` Jean Pihet
2011-01-04 23:01 ` Rafael J. Wysocki
2011-01-04 15:00 ` Jean Pihet
2011-01-04 10:54 ` [PATCH 2/3] perf: add OMAP support for the new power events jean.pihet
2011-01-04 10:54 ` jean.pihet
2011-01-04 10:54 ` jean.pihet at newoldbits.com
2011-01-04 10:54 ` jean.pihet
2011-01-04 18:48 ` Paul Walmsley
2011-01-04 18:48 ` Paul Walmsley
2011-01-05 11:05 ` Jean Pihet
2011-01-05 11:05 ` Jean Pihet
2011-01-10 14:14 ` Thomas Renninger
2011-01-10 14:14 ` Thomas Renninger
2011-01-11 1:24 ` Kevin Hilman
2011-01-11 1:24 ` Kevin Hilman
2011-01-10 19:06 ` Paul Walmsley
2011-01-10 19:06 ` Paul Walmsley
-- strict thread matches above, loose matches on Subject: below --
2010-12-10 19:51 [PATCH 0/3] perf, tools: new power trace API jean.pihet
2010-12-10 19:52 ` [PATCH 2/3] perf: add OMAP support for the new power events jean.pihet
2010-12-10 19:52 ` jean.pihet
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.