Skip to contentSkip to conversation

perf: add OMAP support for the new power events

PATCH New jean.pihet at newoldbits.com 4 participants 13 messages Feb 18, 2011 – Mar 10, 2011

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 perf: add OMAP support for the new power events

From: Jean Pihet <[email protected]>

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),
- SoC clocks changes (enable, disable, set_rate),
- power domain states: the desired target state and -if different- the actually hit state.

Because of the generic nature of the changes, OMAP3 and OMAP4 are supported.

Tested on OMAP3 with suspend/resume, cpuidle, basic DVFS.

---
 arch/arm/mach-omap2/clock.c       |    8 +++++++-
 arch/arm/mach-omap2/pm34xx.c      |    7 +++++++
 arch/arm/mach-omap2/powerdomain.c |   26 +++++++++++++++++++++++---
 3 files changed, 37 insertions(+), 4 deletions(-)
arch/arm/mach-omap2/clock.c +7 -1
index 2a2f152..72af75d 100644
@@ -22,7 +22,9 @@
 #include <linux/clk.h>
 #include <linux/io.h>
 #include <linux/bitops.h>
+#include <trace/events/power.h>
 
+#include <asm/cpu.h>
 #include <plat/clock.h>
 #include "clockdomain.h"
 #include <plat/cpu.h>
@@ -261,6 +263,7 @@ void omap2_clk_disable(struct clk *clk)
 
 	pr_debug("clock: %s: disabling in hardware\n", clk->name);
 
+	trace_clock_disable(clk->name, 0, smp_processor_id());
 	clk->ops->disable(clk);
 
 	if (clk->clkdm)
@@ -312,6 +315,7 @@ int omap2_clk_enable(struct clk *clk)
 		}
 	}
 
+	trace_clock_enable(clk->name, 1, smp_processor_id());
 	ret = clk->ops->enable(clk);
 	if (ret) {
 		WARN(1, "clock: %s: could not enable: %d\n", clk->name, ret);
@@ -349,8 +353,10 @@ int omap2_clk_set_rate(struct clk *clk, unsigned long rate)
 	pr_debug("clock: set_rate for clock %s to rate %ld\n", clk->name, rate);
 
 	/* dpll_ck, core_ck, virt_prcm_set; plus all clksel clocks */
-	if (clk->set_rate)
+	if (clk->set_rate) {
+		trace_clock_set_rate(clk->name, rate, smp_processor_id());
 		ret = clk->set_rate(clk, rate);
+	}
 
 	return ret;
 }
arch/arm/mach-omap2/pm34xx.c +7 -0
index 2f864e4..d1cc3f4 100644
@@ -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 "clockdomain.h"
@@ -519,8 +520,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();
arch/arm/mach-omap2/powerdomain.c +23 -3
index eaed0df..1495eed 100644
@@ -19,12 +19,15 @@
 #include <linux/list.h>
 #include <linux/errno.h>
 #include <linux/string.h>
+#include <trace/events/power.h>
+
 #include "cm2xxx_3xxx.h"
 #include "prcm44xx.h"
 #include "cm44xx.h"
 #include "prm2xxx_3xxx.h"
 #include "prm44xx.h"
 
+#include <asm/cpu.h>
 #include <plat/cpu.h>
 #include "powerdomain.h"
 #include "clockdomain.h"
@@ -32,6 +35,8 @@
 
 #include "pm.h"
 
+#define PWRDM_TRACE_STATES_FLAG	(1<<31)
+
 enum {
 	PWRDM_STATE_NOW = 0,
 	PWRDM_STATE_PREV,
@@ -130,8 +135,7 @@ static void _update_logic_membank_counters(struct powerdomain *pwrdm)
 static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag)
 {
 
-	int prev;
-	int state;
+	int prev, state, trace_state = 0;
 
 	if (pwrdm == NULL)
 		return -EINVAL;
@@ -148,6 +152,17 @@ static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag)
 			pwrdm->state_counter[prev]++;
 		if (prev == PWRDM_POWER_RET)
 			_update_logic_membank_counters(pwrdm);
+		/*
+		 * If the power domain did not hit the desired state,
+		 * generate a trace event with both the desired and hit states
+		 */
+		if (state != prev) {
+			trace_state = (PWRDM_TRACE_STATES_FLAG |
+				       ((state & OMAP_POWERSTATE_MASK) << 8) |
+				       ((prev & OMAP_POWERSTATE_MASK) << 0));
+			trace_power_domain_target(pwrdm->name, trace_state,
+						  smp_processor_id());
+		}
 		break;
 	default:
 		return -EINVAL;
@@ -406,8 +421,13 @@ int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
 	pr_debug("powerdomain: setting next powerstate for %s to %0x\n",
 		 pwrdm->name, pwrst);
 
-	if (arch_pwrdm && arch_pwrdm->pwrdm_set_next_pwrst)
+	if (arch_pwrdm && arch_pwrdm->pwrdm_set_next_pwrst) {
+		/* Trace the pwrdm desired target state */
+		trace_power_domain_target(pwrdm->name, pwrst,
+					  smp_processor_id());
+		/* Program the pwrdm desired target state */
 		ret = arch_pwrdm->pwrdm_set_next_pwrst(pwrdm, pwrst);
+	}
 
 	return ret;
 }
Signature · 1 line
1.7.2.3
#1 lore raw

PATCH 1/1 perf: add OMAP support for the new power events

Jean,

Quoted 23 lines

-----Original Message-----
From: linux-omap-owner at vger.kernel.org [mailto:linux-omap- owner at vger.kernel.org] On Behalf Of jean.pihet at newoldbits.com
Sent: Friday, February 18, 2011 11:41 PM
To: Kevin Hilman; Thomas Renninger; linux-omap at vger.kernel.org; linux-arm-kernel at lists.infradead.org
Cc: Jean Pihet
Subject: [PATCH] perf: add OMAP support for the new power events

From: Jean Pihet <[email protected]>

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),
- SoC clocks changes (enable, disable, set_rate),
- power domain states: the desired target state and -if different- the actually hit state.

Quoted 6 lines

Because of the generic nature of the changes, OMAP3 and OMAP4 are supported.

Tested on OMAP3 with suspend/resume, cpuidle, basic DVFS.

Thanks for additional power domain trace point as per discussion.

Quoted 131 lines
---
 arch/arm/mach-omap2/clock.c       |    8 +++++++-
 arch/arm/mach-omap2/pm34xx.c      |    7 +++++++
 arch/arm/mach-omap2/powerdomain.c |   26 +++++++++++++++++++++++---
 3 files changed, 37 insertions(+), 4 deletions(-)
arch/arm/mach- renamed from arch/arm/mach-omap2/clock.c +0 -0

omap2/clock.c
index 2a2f152..72af75d 100644

arch/arm/mach-omap2/clock.c +4 -0
@@ -22,7 +22,9 @@
 #include <linux/clk.h>
 #include <linux/io.h>
 #include <linux/bitops.h>
+#include <trace/events/power.h>
 
+#include <asm/cpu.h>
 #include <plat/clock.h>
 #include "clockdomain.h"
 #include <plat/cpu.h>
@@ -261,6 +263,7 @@ void omap2_clk_disable(struct clk *clk)
 
 	pr_debug("clock: %s: disabling in hardware\n", clk->name);
 
+	trace_clock_disable(clk->name, 0, smp_processor_id());
 	clk->ops->disable(clk);
 
 	if (clk->clkdm)
@@ -312,6 +315,7 @@ int omap2_clk_enable(struct clk *clk)
 		}
 	}
 
+	trace_clock_enable(clk->name, 1, smp_processor_id());
 	ret = clk->ops->enable(clk);
 	if (ret) {
 		WARN(1, "clock: %s: could not enable: %d\n", clk->name,

ret);

arch/arm/mach-omap2/clock.c +3 -1
@@ -349,8 +353,10 @@ int omap2_clk_set_rate(struct clk *clk,
 unsigned long rate)
 	pr_debug("clock: set_rate for clock %s to rate %ld\n", clk-
 >name, rate);
 
 	/* dpll_ck, core_ck, virt_prcm_set; plus all clksel clocks */
-	if (clk->set_rate)
+	if (clk->set_rate) {
+		trace_clock_set_rate(clk->name, rate,
 smp_processor_id());
 		ret = clk->set_rate(clk, rate);
+	}
arch/arm/mach-omap2/clock.c +0 -0
 	return ret;
 }
arch/arm/mach- renamed from arch/arm/mach-omap2/pm34xx.c +0 -0

omap2/pm34xx.c
index 2f864e4..d1cc3f4 100644

arch/arm/mach-omap2/pm34xx.c +7 -0
@@ -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 "clockdomain.h"
@@ -519,8 +520,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();
arch/arm/mach- renamed from arch/arm/mach-omap2/powerdomain.c +0 -0

omap2/powerdomain.c
index eaed0df..1495eed 100644

arch/arm/mach-omap2/powerdomain.c +6 -2
@@ -19,12 +19,15 @@
 #include <linux/list.h>
 #include <linux/errno.h>
 #include <linux/string.h>
+#include <trace/events/power.h>
+
 #include "cm2xxx_3xxx.h"
 #include "prcm44xx.h"
 #include "cm44xx.h"
 #include "prm2xxx_3xxx.h"
 #include "prm44xx.h"
 
+#include <asm/cpu.h>
 #include <plat/cpu.h>
 #include "powerdomain.h"
 #include "clockdomain.h"
@@ -32,6 +35,8 @@
 
 #include "pm.h"
 
+#define PWRDM_TRACE_STATES_FLAG	(1<<31)
+
 enum {
 	PWRDM_STATE_NOW = 0,
 	PWRDM_STATE_PREV,
@@ -130,8 +135,7 @@ static void
 _update_logic_membank_counters(struct powerdomain *pwrdm)
 static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag)
 {
 
-	int prev;
-	int state;
+	int prev, state, trace_state = 0;
 
 	if (pwrdm == NULL)
arch/arm/mach-omap2/powerdomain.c +3 -0
 		return -EINVAL;
@@ -148,6 +152,17 @@ static int _pwrdm_state_switch(struct
 powerdomain *pwrdm, int flag)
 			pwrdm->state_counter[prev]++;
 		if (prev == PWRDM_POWER_RET)
 			_update_logic_membank_counters(pwrdm);
+		/*
+		 * If the power domain did not hit the desired state,
+		 * generate a trace event with both the desired and hit

states

arch/arm/mach-omap2/powerdomain.c static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag)

Quoted 4 lines
arch/arm/mach-omap2/powerdomain.c +4 -0
+		 */
+		if (state != prev) {
+			trace_state = (PWRDM_TRACE_STATES_FLAG |
+				       ((state & OMAP_POWERSTATE_MASK) <<

8)

Quoted 35 lines

|

arch/arm/mach-omap2/powerdomain.c +1 -0
+				       ((prev & OMAP_POWERSTATE_MASK) <<

0));

arch/arm/mach-omap2/powerdomain.c +1 -0
+			trace_power_domain_target(pwrdm->name,

trace_state,

arch/arm/mach-omap2/powerdomain.c +8 -1
+						  smp_processor_id());
+		}
 		break;
 	default:
 		return -EINVAL;
@@ -406,8 +421,13 @@ int pwrdm_set_next_pwrst(struct powerdomain
 *pwrdm, u8 pwrst)
 	pr_debug("powerdomain: setting next powerstate for %s to
 %0x\n",
 		 pwrdm->name, pwrst);
 
-	if (arch_pwrdm && arch_pwrdm->pwrdm_set_next_pwrst)
+	if (arch_pwrdm && arch_pwrdm->pwrdm_set_next_pwrst) {
+		/* Trace the pwrdm desired target state */
+		trace_power_domain_target(pwrdm->name, pwrst,
+					  smp_processor_id());
+		/* Program the pwrdm desired target state */
 		ret = arch_pwrdm->pwrdm_set_next_pwrst(pwrdm, pwrst);
+	}
 

return ret;
}
--
1.7.2.3

--
To unsubscribe from this list: send the line "unsubscribe linux- omap" in
the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

#2 lore raw

PATCH 1/1 perf: add OMAP support for the new power events

In reply to jean.pihet at newoldbits.com
Quoted 96 lines

-----Original Message-----
From: linux-omap-owner at vger.kernel.org [mailto:linux-omap- owner at vger.kernel.org] On Behalf Of jean.pihet at newoldbits.com
Sent: Friday, February 18, 2011 11:41 PM
To: Kevin Hilman; Thomas Renninger; linux-omap at vger.kernel.org; linux-arm-kernel at lists.infradead.org
Cc: Jean Pihet
Subject: [PATCH] perf: add OMAP support for the new power events

From: Jean Pihet <[email protected]>

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),
- SoC clocks changes (enable, disable, set_rate),
- power domain states: the desired target state and -if different- the actually hit state.

Because of the generic nature of the changes, OMAP3 and OMAP4 are supported.

Tested on OMAP3 with suspend/resume, cpuidle, basic DVFS.

---
 arch/arm/mach-omap2/clock.c       |    8 +++++++-
 arch/arm/mach-omap2/pm34xx.c      |    7 +++++++
 arch/arm/mach-omap2/powerdomain.c |   26 +++++++++++++++++++++++---
 3 files changed, 37 insertions(+), 4 deletions(-)
arch/arm/mach- renamed from arch/arm/mach-omap2/clock.c +0 -0

omap2/clock.c
index 2a2f152..72af75d 100644

arch/arm/mach-omap2/clock.c +4 -0
@@ -22,7 +22,9 @@
 #include <linux/clk.h>
 #include <linux/io.h>
 #include <linux/bitops.h>
+#include <trace/events/power.h>
 
+#include <asm/cpu.h>
 #include <plat/clock.h>
 #include "clockdomain.h"
 #include <plat/cpu.h>
@@ -261,6 +263,7 @@ void omap2_clk_disable(struct clk *clk)
 
 	pr_debug("clock: %s: disabling in hardware\n", clk->name);
 
+	trace_clock_disable(clk->name, 0, smp_processor_id());
 	clk->ops->disable(clk);
 
 	if (clk->clkdm)
@@ -312,6 +315,7 @@ int omap2_clk_enable(struct clk *clk)
 		}
 	}
 
+	trace_clock_enable(clk->name, 1, smp_processor_id());
 	ret = clk->ops->enable(clk);
 	if (ret) {
 		WARN(1, "clock: %s: could not enable: %d\n", clk->name,

ret);

arch/arm/mach-omap2/clock.c +3 -1
@@ -349,8 +353,10 @@ int omap2_clk_set_rate(struct clk *clk,
 unsigned long rate)
 	pr_debug("clock: set_rate for clock %s to rate %ld\n", clk-
 >name, rate);
 
 	/* dpll_ck, core_ck, virt_prcm_set; plus all clksel clocks */
-	if (clk->set_rate)
+	if (clk->set_rate) {
+		trace_clock_set_rate(clk->name, rate,
 smp_processor_id());
 		ret = clk->set_rate(clk, rate);
+	}
arch/arm/mach-omap2/clock.c +0 -0
 	return ret;
 }
arch/arm/mach- renamed from arch/arm/mach-omap2/pm34xx.c +0 -0

omap2/pm34xx.c
index 2f864e4..d1cc3f4 100644

arch/arm/mach-omap2/pm34xx.c +1 -0
@@ -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 "clockdomain.h"

arch/arm/mach-omap2/pm34xx.c static void omap3_pm_idle(void) · line 525

Quoted 6 lines
arch/arm/mach-omap2/pm34xx.c +3 -0
@@ -519,8 +520,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());
+

This default idle code won't be used when you enable the CONFIG_CPUIDLE. That case the cpuidle34xx.c idle code gets registered.

Shouldn't you patch that code instead? This is more or less dead code and it is just like default idle code when idle drivers isn't registered.

Quoted 58 lines
arch/arm/mach-omap2/pm34xx.c +3 -0
 	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();
arch/arm/mach- renamed from arch/arm/mach-omap2/powerdomain.c +0 -0

omap2/powerdomain.c
index eaed0df..1495eed 100644

arch/arm/mach-omap2/powerdomain.c +6 -2
@@ -19,12 +19,15 @@
 #include <linux/list.h>
 #include <linux/errno.h>
 #include <linux/string.h>
+#include <trace/events/power.h>
+
 #include "cm2xxx_3xxx.h"
 #include "prcm44xx.h"
 #include "cm44xx.h"
 #include "prm2xxx_3xxx.h"
 #include "prm44xx.h"
 
+#include <asm/cpu.h>
 #include <plat/cpu.h>
 #include "powerdomain.h"
 #include "clockdomain.h"
@@ -32,6 +35,8 @@
 
 #include "pm.h"
 
+#define PWRDM_TRACE_STATES_FLAG	(1<<31)
+
 enum {
 	PWRDM_STATE_NOW = 0,
 	PWRDM_STATE_PREV,
@@ -130,8 +135,7 @@ static void
 _update_logic_membank_counters(struct powerdomain *pwrdm)
 static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag)
 {
 
-	int prev;
-	int state;
+	int prev, state, trace_state = 0;
 
 	if (pwrdm == NULL)
arch/arm/mach-omap2/powerdomain.c +3 -0
 		return -EINVAL;
@@ -148,6 +152,17 @@ static int _pwrdm_state_switch(struct
 powerdomain *pwrdm, int flag)
 			pwrdm->state_counter[prev]++;
 		if (prev == PWRDM_POWER_RET)
 			_update_logic_membank_counters(pwrdm);
+		/*
+		 * If the power domain did not hit the desired state,
+		 * generate a trace event with both the desired and hit

states

arch/arm/mach-omap2/powerdomain.c static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag)

Quoted 4 lines
arch/arm/mach-omap2/powerdomain.c +4 -0
+		 */
+		if (state != prev) {
+			trace_state = (PWRDM_TRACE_STATES_FLAG |
+				       ((state & OMAP_POWERSTATE_MASK) <<

8)

Quoted 35 lines

|

arch/arm/mach-omap2/powerdomain.c +1 -0
+				       ((prev & OMAP_POWERSTATE_MASK) <<

0));

arch/arm/mach-omap2/powerdomain.c +1 -0
+			trace_power_domain_target(pwrdm->name,

trace_state,

arch/arm/mach-omap2/powerdomain.c +8 -1
+						  smp_processor_id());
+		}
 		break;
 	default:
 		return -EINVAL;
@@ -406,8 +421,13 @@ int pwrdm_set_next_pwrst(struct powerdomain
 *pwrdm, u8 pwrst)
 	pr_debug("powerdomain: setting next powerstate for %s to
 %0x\n",
 		 pwrdm->name, pwrst);
 
-	if (arch_pwrdm && arch_pwrdm->pwrdm_set_next_pwrst)
+	if (arch_pwrdm && arch_pwrdm->pwrdm_set_next_pwrst) {
+		/* Trace the pwrdm desired target state */
+		trace_power_domain_target(pwrdm->name, pwrst,
+					  smp_processor_id());
+		/* Program the pwrdm desired target state */
 		ret = arch_pwrdm->pwrdm_set_next_pwrst(pwrdm, pwrst);
+	}
 

return ret;
}
--
1.7.2.3

--
To unsubscribe from this list: send the line "unsubscribe linux- omap" in
the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#3 lore raw

PATCH 1/1 perf: add OMAP support for the new power events

Hi Santosh,

Quoted 99 lines · Santosh Shilimkar
Quoted 99 lines

-----Original Message-----
From: linux-omap-owner at vger.kernel.org [mailto:linux-omap- owner at vger.kernel.org] On Behalf Of jean.pihet at newoldbits.com
Sent: Friday, February 18, 2011 11:41 PM
To: Kevin Hilman; Thomas Renninger; linux-omap at vger.kernel.org; linux-arm-kernel at lists.infradead.org
Cc: Jean Pihet
Subject: [PATCH] perf: add OMAP support for the new power events

From: Jean Pihet <[email protected]>

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),
- SoC clocks changes (enable, disable, set_rate),
- power domain states: the desired target state and -if different- ? the actually hit state.

Because of the generic nature of the changes, OMAP3 and OMAP4 are supported.

Tested on OMAP3 with suspend/resume, cpuidle, basic DVFS.

---

?arch/arm/mach-omap2/clock.c ? ? ? | ? ?8 +++++++-
?arch/arm/mach-omap2/pm34xx.c ? ? ?| ? ?7 +++++++
?arch/arm/mach-omap2/powerdomain.c | ? 26 +++++++++++++++++++++++---

?3 files changed, 37 insertions(+), 4 deletions(-)

arch/arm/mach- renamed from arch/arm/mach-omap2/clock.c +0 -0

omap2/clock.c
index 2a2f152..72af75d 100644

arch/arm/mach-omap2/clock.c +0 -0
@@ -22,7 +22,9 @@

?#include <linux/clk.h>
?#include <linux/io.h>
?#include <linux/bitops.h>

arch/arm/mach-omap2/clock.c +2 -0
+#include <trace/events/power.h>
 
+#include <asm/cpu.h>

?#include <plat/clock.h>
?#include "clockdomain.h"
?#include <plat/cpu.h>

arch/arm/mach-omap2/clock.c +2 -0
@@ -261,6 +263,7 @@ void omap2_clk_disable(struct clk *clk)
 
 ? ? ? pr_debug("clock: %s: disabling in hardware\n", clk->name);
 
+ ? ? trace_clock_disable(clk->name, 0, smp_processor_id());
 ? ? ? clk->ops->disable(clk);
 
 ? ? ? if (clk->clkdm)
@@ -312,6 +315,7 @@ int omap2_clk_enable(struct clk *clk)
 ? ? ? ? ? ? ? }
 ? ? ? }
 
+ ? ? trace_clock_enable(clk->name, 1, smp_processor_id());
 ? ? ? ret = clk->ops->enable(clk);
 ? ? ? if (ret) {
 ? ? ? ? ? ? ? WARN(1, "clock: %s: could not enable: %d\n", clk->name,

ret);

arch/arm/mach-omap2/clock.c +0 -0
@@ -349,8 +353,10 @@ int omap2_clk_set_rate(struct clk *clk,
 unsigned long rate)

? ? ? pr_debug("clock: set_rate for clock %s to rate %ld\n", clk-

Quoted 1 line

name, rate);

? ? ? /* dpll_ck, core_ck, virt_prcm_set; plus all clksel clocks */
- ? ? if (clk->set_rate)
+ ? ? if (clk->set_rate) {
+ ? ? ? ? ? ? trace_clock_set_rate(clk->name, rate, smp_processor_id());
? ? ? ? ? ? ? ret = clk->set_rate(clk, rate);
+ ? ? }

? ? ? return ret;
?}

arch/arm/mach- renamed from arch/arm/mach-omap2/pm34xx.c +0 -0

omap2/pm34xx.c
index 2f864e4..d1cc3f4 100644

arch/arm/mach-omap2/pm34xx.c +0 -0
@@ -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 "clockdomain.h"

arch/arm/mach-omap2/pm34xx.c +0 -0
@@ -519,8 +520,14 @@ static void omap3_pm_idle(void)
 ? ? ? if (omap_irq_pending() || need_resched())
 ? ? ? ? ? ? ? goto out;
 
Quoted 7 lines

arch/arm/mach-omap2/pm34xx.c static void omap3_pm_idle(void) · line 525

Quoted 3 lines
arch/arm/mach-omap2/pm34xx.c +3 -0
+ ? ? trace_power_start(POWER_CSTATE, 1, smp_processor_id());
+ ? ? trace_cpu_idle(1, smp_processor_id());
+

This default idle code won't be used when you enable the CONFIG_CPUIDLE. That case the cpuidle34xx.c idle code gets registered.

That is correct. OMAP has a default idle handler (omap3_pm_idle) and a cpuidle handler (omap3_enter_idle in arch/arm/mach-omap2/cpuidle34xx.c).

Quoted 3 lines

Shouldn't you patch that code instead? This is more or less dead code and it is just like default idle code when idle drivers isn't registered.

The cpuidle framework already is instrumented in a generic way. This code adds the instrumentation to the default idle handler so that all cases are covered. BTW the patch description gives that information.

If there is dead code then it is not only the code from this patch but all the code for the default idle handler.

Thanks for reviewing.

Regards,
Jean

Quoted 101 lines
Quoted 59 lines
arch/arm/mach-omap2/pm34xx.c +3 -0
 ? ? ? 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();

arch/arm/mach- renamed from arch/arm/mach-omap2/powerdomain.c +0 -0

omap2/powerdomain.c
index eaed0df..1495eed 100644

arch/arm/mach-omap2/powerdomain.c +0 -0
@@ -19,12 +19,15 @@

?#include <linux/list.h>
?#include <linux/errno.h>
?#include <linux/string.h>

arch/arm/mach-omap2/powerdomain.c +2 -0
+#include <trace/events/power.h>
+

?#include "cm2xxx_3xxx.h"
?#include "prcm44xx.h"
?#include "cm44xx.h"
?#include "prm2xxx_3xxx.h"
?#include "prm44xx.h"

+#include <asm/cpu.h>
?#include <plat/cpu.h>
?#include "powerdomain.h"
?#include "clockdomain.h"

arch/arm/mach-omap2/powerdomain.c +0 -0
@@ -32,6 +35,8 @@
 

?#include "pm.h"

arch/arm/mach-omap2/powerdomain.c +2 -0
+#define PWRDM_TRACE_STATES_FLAG ? ? ?(1<<31)
+

?enum {
? ? ? PWRDM_STATE_NOW = 0,
? ? ? PWRDM_STATE_PREV,

arch/arm/mach-omap2/powerdomain.c +1 -2
@@ -130,8 +135,7 @@ static void
 _update_logic_membank_counters(struct powerdomain *pwrdm)
 ?static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag)
 ?{
 
- ? ? int prev;
- ? ? int state;
+ ? ? int prev, state, trace_state = 0;
 
 ? ? ? if (pwrdm == NULL)

? ? ? ? ? ? ? return -EINVAL;

arch/arm/mach-omap2/powerdomain.c +3 -0
@@ -148,6 +152,17 @@ static int _pwrdm_state_switch(struct
 powerdomain *pwrdm, int flag)
 ? ? ? ? ? ? ? ? ? ? ? pwrdm->state_counter[prev]++;
 ? ? ? ? ? ? ? if (prev == PWRDM_POWER_RET)
 ? ? ? ? ? ? ? ? ? ? ? _update_logic_membank_counters(pwrdm);
+ ? ? ? ? ? ? /*
+ ? ? ? ? ? ? ?* If the power domain did not hit the desired state,
+ ? ? ? ? ? ? ?* generate a trace event with both the desired and hit
Quoted 5 lines

states
+ ? ? ? ? ? ? ?*/
+ ? ? ? ? ? ? if (state != prev) {
+ ? ? ? ? ? ? ? ? ? ? trace_state = (PWRDM_TRACE_STATES_FLAG |
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?((state & OMAP_POWERSTATE_MASK) <<

8)

Quoted 35 lines

|
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?((prev & OMAP_POWERSTATE_MASK) << 0));
+ ? ? ? ? ? ? ? ? ? ? trace_power_domain_target(pwrdm->name, trace_state,
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? smp_processor_id());
+ ? ? ? ? ? ? }
? ? ? ? ? ? ? break;
? ? ? default:
? ? ? ? ? ? ? return -EINVAL;

arch/arm/mach-omap2/powerdomain.c +0 -0
@@ -406,8 +421,13 @@ int pwrdm_set_next_pwrst(struct powerdomain
 *pwrdm, u8 pwrst)

? ? ? pr_debug("powerdomain: setting next powerstate for %s to %0x\n",
? ? ? ? ? ? ? ?pwrdm->name, pwrst);

- ? ? if (arch_pwrdm && arch_pwrdm->pwrdm_set_next_pwrst)
+ ? ? if (arch_pwrdm && arch_pwrdm->pwrdm_set_next_pwrst) {
+ ? ? ? ? ? ? /* Trace the pwrdm desired target state */
+ ? ? ? ? ? ? trace_power_domain_target(pwrdm->name, pwrst,
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? smp_processor_id());
+ ? ? ? ? ? ? /* Program the pwrdm desired target state */ ? ? ? ? ? ? ? ret = arch_pwrdm->pwrdm_set_next_pwrst(pwrdm, pwrst);
+ ? ? }

? ? ? return ret;
?}
--
1.7.2.3

--
To unsubscribe from this list: send the line "unsubscribe linux- omap" in
the body of a message to majordomo at vger.kernel.org More majordomo info at ?http://vger.kernel.org/majordomo-info.html

#4 lore raw

PATCH 1/1 perf: add OMAP support for the new power events

Quoted 10 lines

-----Original Message-----
From: Jean Pihet [mailto:jean.pihet at newoldbits.com]
Sent: Monday, February 21, 2011 2:14 PM
To: Santosh Shilimkar
Cc: Kevin Hilman; Thomas Renninger; linux-omap at vger.kernel.org; linux-arm-kernel at lists.infradead.org; Jean Pihet-XID
Subject: Re: [PATCH] perf: add OMAP support for the new power events

Hi Santosh,

[...]

Quoted 32 lines
Quoted 17 lines
Quoted 17 lines
arch/arm/mach- renamed from arch/arm/mach-omap2/pm34xx.c +0 -0

omap2/pm34xx.c
index 2f864e4..d1cc3f4 100644

arch/arm/mach-omap2/pm34xx.c +0 -0
@@ -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 "clockdomain.h"

arch/arm/mach-omap2/pm34xx.c +0 -0
@@ -519,8 +520,14 @@ static void omap3_pm_idle(void)
 ? ? ? if (omap_irq_pending() || need_resched())
 ? ? ? ? ? ? ? goto out;
 
Quoted 7 lines

arch/arm/mach-omap2/pm34xx.c static void omap3_pm_idle(void) · line 525

Quoted 3 lines
arch/arm/mach-omap2/pm34xx.c +3 -0
+ ? ? trace_power_start(POWER_CSTATE, 1, smp_processor_id());
+ ? ? trace_cpu_idle(1, smp_processor_id());
+

This default idle code won't be used when you enable the CONFIG_CPUIDLE. That case the cpuidle34xx.c idle code gets registered.

That is correct. OMAP has a default idle handler (omap3_pm_idle) and a
cpuidle handler (omap3_enter_idle in arch/arm/mach-omap2/cpuidle34xx.c).

Quoted 3 lines

Shouldn't you patch that code instead? This is more or less dead code and it is just like default idle code when idle drivers isn't registered.

Quoted 9 lines

The cpuidle framework already is instrumented in a generic way. This code adds the instrumentation to the default idle handler so that all
cases are covered. BTW the patch description gives that information.

If there is dead code then it is not only the code from this patch but
all the code for the default idle handler.

I read your change log. It says.

Quoted 4 lines
Quoted 4 lines

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;

Now code in cpuilde34xx.c is also OMAP specific and hence the confusion at least for me.
Regarding dead code, I meant existing code of default handler.

Thanks for clarification.

Regards,
Santosh

author
#5 lore raw

PATCH 1/1 perf: add OMAP support for the new power events

Hi Kevin,

Can you please check this patch? From the previous discussions I understood it was OK.
This one has been submitted to and l-o and l-a-k MLs. How will this one be merged in?

Thanks,
Jean

Quoted 69 lines · Santosh Shilimkar
Quoted 10 lines

-----Original Message-----
From: Jean Pihet [mailto:jean.pihet at newoldbits.com]
Sent: Monday, February 21, 2011 2:14 PM
To: Santosh Shilimkar
Cc: Kevin Hilman; Thomas Renninger; linux-omap at vger.kernel.org; linux-arm-kernel at lists.infradead.org; Jean Pihet-XID
Subject: Re: [PATCH] perf: add OMAP support for the new power events

Hi Santosh,

[...]

Quoted 32 lines
Quoted 17 lines
Quoted 17 lines
arch/arm/mach- renamed from arch/arm/mach-omap2/pm34xx.c +0 -0

omap2/pm34xx.c
index 2f864e4..d1cc3f4 100644

arch/arm/mach-omap2/pm34xx.c +0 -0
@@ -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 "clockdomain.h"

arch/arm/mach-omap2/pm34xx.c +0 -0
@@ -519,8 +520,14 @@ static void omap3_pm_idle(void)
 ? ? ? if (omap_irq_pending() || need_resched())
 ? ? ? ? ? ? ? goto out;
 
Quoted 7 lines

arch/arm/mach-omap2/pm34xx.c static void omap3_pm_idle(void) · line 525

Quoted 3 lines
arch/arm/mach-omap2/pm34xx.c +3 -0
+ ? ? trace_power_start(POWER_CSTATE, 1, smp_processor_id());
+ ? ? trace_cpu_idle(1, smp_processor_id());
+

This default idle code won't be used when you enable the CONFIG_CPUIDLE. That case the cpuidle34xx.c idle code gets registered.

That is correct. OMAP has a default idle handler (omap3_pm_idle) and a
cpuidle handler (omap3_enter_idle in arch/arm/mach-omap2/cpuidle34xx.c).

Quoted 3 lines

Shouldn't you patch that code instead? This is more or less dead code and it is just like default idle code when idle drivers isn't registered.

Quoted 9 lines

The cpuidle framework already is instrumented in a generic way. This code adds the instrumentation to the default idle handler so that all
cases are covered. BTW the patch description gives that information.

If there is dead code then it is not only the code from this patch but
all the code for the default idle handler.

I read your change log. It says.

Quoted 4 lines
Quoted 4 lines

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;

Now code in cpuilde34xx.c is also OMAP specific and hence the confusion at least for me.
Regarding dead code, I meant existing code of default handler.

Thanks for clarification.

Regards,
Santosh

#6 lore raw

PATCH 1/1 perf: add OMAP support for the new power events

In reply to jean.pihet at newoldbits.com

Hi Jean,

Quoted 4 lines · jean.pihet at newoldbits.com

From: Jean Pihet <[email protected]>

The patch adds the new power management trace points for the OMAP architecture.

There are some other core clock/powerdomain changes queued for 2.6.39 ahead of this that conflict with your patch.

Could you rebase this against my pm-core branch where these other changes are queued?

Thanks,

Kevin

Quoted 161 lines

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),
- SoC clocks changes (enable, disable, set_rate),
- power domain states: the desired target state and -if different- the actually hit state.

Because of the generic nature of the changes, OMAP3 and OMAP4 are supported.

Tested on OMAP3 with suspend/resume, cpuidle, basic DVFS.

---
 arch/arm/mach-omap2/clock.c       |    8 +++++++-
 arch/arm/mach-omap2/pm34xx.c      |    7 +++++++
 arch/arm/mach-omap2/powerdomain.c |   26 +++++++++++++++++++++++---
 3 files changed, 37 insertions(+), 4 deletions(-)
arch/arm/mach-omap2/clock.c +7 -1
index 2a2f152..72af75d 100644
@@ -22,7 +22,9 @@
 #include <linux/clk.h>
 #include <linux/io.h>
 #include <linux/bitops.h>
+#include <trace/events/power.h>
 
+#include <asm/cpu.h>
 #include <plat/clock.h>
 #include "clockdomain.h"
 #include <plat/cpu.h>
@@ -261,6 +263,7 @@ void omap2_clk_disable(struct clk *clk)
 
 	pr_debug("clock: %s: disabling in hardware\n", clk->name);
 
+	trace_clock_disable(clk->name, 0, smp_processor_id());
 	clk->ops->disable(clk);
 
 	if (clk->clkdm)
@@ -312,6 +315,7 @@ int omap2_clk_enable(struct clk *clk)
 		}
 	}
 
+	trace_clock_enable(clk->name, 1, smp_processor_id());
 	ret = clk->ops->enable(clk);
 	if (ret) {
 		WARN(1, "clock: %s: could not enable: %d\n", clk->name, ret);
@@ -349,8 +353,10 @@ int omap2_clk_set_rate(struct clk *clk, unsigned long rate)
 	pr_debug("clock: set_rate for clock %s to rate %ld\n", clk->name, rate);
 
 	/* dpll_ck, core_ck, virt_prcm_set; plus all clksel clocks */
-	if (clk->set_rate)
+	if (clk->set_rate) {
+		trace_clock_set_rate(clk->name, rate, smp_processor_id());
 		ret = clk->set_rate(clk, rate);
+	}
 
 	return ret;
 }
arch/arm/mach-omap2/pm34xx.c +7 -0
index 2f864e4..d1cc3f4 100644
@@ -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 "clockdomain.h"
@@ -519,8 +520,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();
arch/arm/mach-omap2/powerdomain.c +23 -3
index eaed0df..1495eed 100644
@@ -19,12 +19,15 @@
 #include <linux/list.h>
 #include <linux/errno.h>
 #include <linux/string.h>
+#include <trace/events/power.h>
+
 #include "cm2xxx_3xxx.h"
 #include "prcm44xx.h"
 #include "cm44xx.h"
 #include "prm2xxx_3xxx.h"
 #include "prm44xx.h"
 
+#include <asm/cpu.h>
 #include <plat/cpu.h>
 #include "powerdomain.h"
 #include "clockdomain.h"
@@ -32,6 +35,8 @@
 
 #include "pm.h"
 
+#define PWRDM_TRACE_STATES_FLAG	(1<<31)
+
 enum {
 	PWRDM_STATE_NOW = 0,
 	PWRDM_STATE_PREV,
@@ -130,8 +135,7 @@ static void _update_logic_membank_counters(struct powerdomain *pwrdm)
 static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag)
 {
 
-	int prev;
-	int state;
+	int prev, state, trace_state = 0;
 
 	if (pwrdm == NULL)
 		return -EINVAL;
@@ -148,6 +152,17 @@ static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag)
 			pwrdm->state_counter[prev]++;
 		if (prev == PWRDM_POWER_RET)
 			_update_logic_membank_counters(pwrdm);
+		/*
+		 * If the power domain did not hit the desired state,
+		 * generate a trace event with both the desired and hit states
+		 */
+		if (state != prev) {
+			trace_state = (PWRDM_TRACE_STATES_FLAG |
+				       ((state & OMAP_POWERSTATE_MASK) << 8) |
+				       ((prev & OMAP_POWERSTATE_MASK) << 0));
+			trace_power_domain_target(pwrdm->name, trace_state,
+						  smp_processor_id());
+		}
 		break;
 	default:
 		return -EINVAL;
@@ -406,8 +421,13 @@ int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
 	pr_debug("powerdomain: setting next powerstate for %s to %0x\n",
 		 pwrdm->name, pwrst);
 
-	if (arch_pwrdm && arch_pwrdm->pwrdm_set_next_pwrst)
+	if (arch_pwrdm && arch_pwrdm->pwrdm_set_next_pwrst) {
+		/* Trace the pwrdm desired target state */
+		trace_power_domain_target(pwrdm->name, pwrst,
+					  smp_processor_id());
+		/* Program the pwrdm desired target state */
 		ret = arch_pwrdm->pwrdm_set_next_pwrst(pwrdm, pwrst);
+	}
 
 	return ret;
 }
author
#7 lore raw

PATCH 1/1 perf: add OMAP support for the new power events

In reply to jean.pihet at newoldbits.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),
- SoC clocks changes (enable, disable, set_rate),
- power domain states: the desired target state and -if different- the actually hit state.

Because of the generic nature of the changes, OMAP3 and OMAP4 are supported.

Tested on OMAP3 with suspend/resume, cpuidle, basic DVFS.

---
 arch/arm/mach-omap2/clock.c       |   11 +++++++++--
 arch/arm/mach-omap2/pm34xx.c      |    7 +++++++
 arch/arm/mach-omap2/powerdomain.c |   26 +++++++++++++++++++++++---
 3 files changed, 39 insertions(+), 5 deletions(-)
arch/arm/mach-omap2/clock.c +9 -2
index 46d03cc..180299e 100644
@@ -22,7 +22,9 @@
 #include <linux/clk.h>
 #include <linux/io.h>
 #include <linux/bitops.h>
+#include <trace/events/power.h>
 
+#include <asm/cpu.h>
 #include <plat/clock.h>
 #include "clockdomain.h"
 #include <plat/cpu.h>
@@ -261,8 +263,10 @@ void omap2_clk_disable(struct clk *clk)
 
 	pr_debug("clock: %s: disabling in hardware\n", clk->name);
 
-	if (clk->ops && clk->ops->disable)
+	if (clk->ops && clk->ops->disable) {
+		trace_clock_disable(clk->name, 0, smp_processor_id());
 		clk->ops->disable(clk);
+	}
 
 	if (clk->clkdm)
 		clkdm_clk_disable(clk->clkdm, clk);
@@ -314,6 +318,7 @@ int omap2_clk_enable(struct clk *clk)
 	}
 
 	if (clk->ops && clk->ops->enable) {
+		trace_clock_enable(clk->name, 1, smp_processor_id());
 		ret = clk->ops->enable(clk);
 		if (ret) {
 			WARN(1, "clock: %s: could not enable: %d\n",
@@ -353,8 +358,10 @@ int omap2_clk_set_rate(struct clk *clk, unsigned long rate)
 	pr_debug("clock: set_rate for clock %s to rate %ld\n", clk->name, rate);
 
 	/* dpll_ck, core_ck, virt_prcm_set; plus all clksel clocks */
-	if (clk->set_rate)
+	if (clk->set_rate) {
+		trace_clock_set_rate(clk->name, rate, smp_processor_id());
 		ret = clk->set_rate(clk, rate);
+	}
 
 	return ret;
 }
arch/arm/mach-omap2/pm34xx.c +7 -0
index bd610bc..d46b9ff 100644
@@ -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 "clockdomain.h"
@@ -514,8 +515,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();
arch/arm/mach-omap2/powerdomain.c +23 -3
index eaed0df..1495eed 100644
@@ -19,12 +19,15 @@
 #include <linux/list.h>
 #include <linux/errno.h>
 #include <linux/string.h>
+#include <trace/events/power.h>
+
 #include "cm2xxx_3xxx.h"
 #include "prcm44xx.h"
 #include "cm44xx.h"
 #include "prm2xxx_3xxx.h"
 #include "prm44xx.h"
 
+#include <asm/cpu.h>
 #include <plat/cpu.h>
 #include "powerdomain.h"
 #include "clockdomain.h"
@@ -32,6 +35,8 @@
 
 #include "pm.h"
 
+#define PWRDM_TRACE_STATES_FLAG	(1<<31)
+
 enum {
 	PWRDM_STATE_NOW = 0,
 	PWRDM_STATE_PREV,
@@ -130,8 +135,7 @@ static void _update_logic_membank_counters(struct powerdomain *pwrdm)
 static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag)
 {
 
-	int prev;
-	int state;
+	int prev, state, trace_state = 0;
 
 	if (pwrdm == NULL)
 		return -EINVAL;
@@ -148,6 +152,17 @@ static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag)
 			pwrdm->state_counter[prev]++;
 		if (prev == PWRDM_POWER_RET)
 			_update_logic_membank_counters(pwrdm);
+		/*
+		 * If the power domain did not hit the desired state,
+		 * generate a trace event with both the desired and hit states
+		 */
+		if (state != prev) {
+			trace_state = (PWRDM_TRACE_STATES_FLAG |
+				       ((state & OMAP_POWERSTATE_MASK) << 8) |
+				       ((prev & OMAP_POWERSTATE_MASK) << 0));
+			trace_power_domain_target(pwrdm->name, trace_state,
+						  smp_processor_id());
+		}
 		break;
 	default:
 		return -EINVAL;
@@ -406,8 +421,13 @@ int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
 	pr_debug("powerdomain: setting next powerstate for %s to %0x\n",
 		 pwrdm->name, pwrst);
 
-	if (arch_pwrdm && arch_pwrdm->pwrdm_set_next_pwrst)
+	if (arch_pwrdm && arch_pwrdm->pwrdm_set_next_pwrst) {
+		/* Trace the pwrdm desired target state */
+		trace_power_domain_target(pwrdm->name, pwrst,
+					  smp_processor_id());
+		/* Program the pwrdm desired target state */
 		ret = arch_pwrdm->pwrdm_set_next_pwrst(pwrdm, pwrst);
+	}
 
 	return ret;
 }
Signature · 1 line
1.7.2.3
#8 lore raw

PATCH 1/1 perf: add OMAP support for the new power events

Quoted 8 lines · Jean Pihet

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),

Minor: AFAICT, this patch doesn't instrument CPUfreq.

Quoted 9 lines

- SoC clocks changes (enable, disable, set_rate),
- power domain states: the desired target state and -if different- the actually hit state.

Because of the generic nature of the changes, OMAP3 and OMAP4 are supported.

Tested on OMAP3 with suspend/resume, cpuidle, basic DVFS.

With Paul's ack on the powerdomain changes, I'll queue this for 2.6.39 after making the above change in the changelog.

Kevin

Quoted 151 lines
---
 arch/arm/mach-omap2/clock.c       |   11 +++++++++--
 arch/arm/mach-omap2/pm34xx.c      |    7 +++++++
 arch/arm/mach-omap2/powerdomain.c |   26 +++++++++++++++++++++++---
 3 files changed, 39 insertions(+), 5 deletions(-)
arch/arm/mach-omap2/clock.c +9 -2
index 46d03cc..180299e 100644
@@ -22,7 +22,9 @@
 #include <linux/clk.h>
 #include <linux/io.h>
 #include <linux/bitops.h>
+#include <trace/events/power.h>
 
+#include <asm/cpu.h>
 #include <plat/clock.h>
 #include "clockdomain.h"
 #include <plat/cpu.h>
@@ -261,8 +263,10 @@ void omap2_clk_disable(struct clk *clk)
 
 	pr_debug("clock: %s: disabling in hardware\n", clk->name);
 
-	if (clk->ops && clk->ops->disable)
+	if (clk->ops && clk->ops->disable) {
+		trace_clock_disable(clk->name, 0, smp_processor_id());
 		clk->ops->disable(clk);
+	}
 
 	if (clk->clkdm)
 		clkdm_clk_disable(clk->clkdm, clk);
@@ -314,6 +318,7 @@ int omap2_clk_enable(struct clk *clk)
 	}
 
 	if (clk->ops && clk->ops->enable) {
+		trace_clock_enable(clk->name, 1, smp_processor_id());
 		ret = clk->ops->enable(clk);
 		if (ret) {
 			WARN(1, "clock: %s: could not enable: %d\n",
@@ -353,8 +358,10 @@ int omap2_clk_set_rate(struct clk *clk, unsigned long rate)
 	pr_debug("clock: set_rate for clock %s to rate %ld\n", clk->name, rate);
 
 	/* dpll_ck, core_ck, virt_prcm_set; plus all clksel clocks */
-	if (clk->set_rate)
+	if (clk->set_rate) {
+		trace_clock_set_rate(clk->name, rate, smp_processor_id());
 		ret = clk->set_rate(clk, rate);
+	}
 
 	return ret;
 }
arch/arm/mach-omap2/pm34xx.c +7 -0
index bd610bc..d46b9ff 100644
@@ -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 "clockdomain.h"
@@ -514,8 +515,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();
arch/arm/mach-omap2/powerdomain.c +23 -3
index eaed0df..1495eed 100644
@@ -19,12 +19,15 @@
 #include <linux/list.h>
 #include <linux/errno.h>
 #include <linux/string.h>
+#include <trace/events/power.h>
+
 #include "cm2xxx_3xxx.h"
 #include "prcm44xx.h"
 #include "cm44xx.h"
 #include "prm2xxx_3xxx.h"
 #include "prm44xx.h"
 
+#include <asm/cpu.h>
 #include <plat/cpu.h>
 #include "powerdomain.h"
 #include "clockdomain.h"
@@ -32,6 +35,8 @@
 
 #include "pm.h"
 
+#define PWRDM_TRACE_STATES_FLAG	(1<<31)
+
 enum {
 	PWRDM_STATE_NOW = 0,
 	PWRDM_STATE_PREV,
@@ -130,8 +135,7 @@ static void _update_logic_membank_counters(struct powerdomain *pwrdm)
 static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag)
 {
 
-	int prev;
-	int state;
+	int prev, state, trace_state = 0;
 
 	if (pwrdm == NULL)
 		return -EINVAL;
@@ -148,6 +152,17 @@ static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag)
 			pwrdm->state_counter[prev]++;
 		if (prev == PWRDM_POWER_RET)
 			_update_logic_membank_counters(pwrdm);
+		/*
+		 * If the power domain did not hit the desired state,
+		 * generate a trace event with both the desired and hit states
+		 */
+		if (state != prev) {
+			trace_state = (PWRDM_TRACE_STATES_FLAG |
+				       ((state & OMAP_POWERSTATE_MASK) << 8) |
+				       ((prev & OMAP_POWERSTATE_MASK) << 0));
+			trace_power_domain_target(pwrdm->name, trace_state,
+						  smp_processor_id());
+		}
 		break;
 	default:
 		return -EINVAL;
@@ -406,8 +421,13 @@ int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
 	pr_debug("powerdomain: setting next powerstate for %s to %0x\n",
 		 pwrdm->name, pwrst);
 
-	if (arch_pwrdm && arch_pwrdm->pwrdm_set_next_pwrst)
+	if (arch_pwrdm && arch_pwrdm->pwrdm_set_next_pwrst) {
+		/* Trace the pwrdm desired target state */
+		trace_power_domain_target(pwrdm->name, pwrst,
+					  smp_processor_id());
+		/* Program the pwrdm desired target state */
 		ret = arch_pwrdm->pwrdm_set_next_pwrst(pwrdm, pwrst);
+	}
 
 	return ret;
 }
author
#9 lore raw

PATCH 1/1 perf: add OMAP support for the new power events

Quoted 5 lines · Kevin Hilman
Quoted 3 lines · Jean Pihet

The patch adds the new power management trace points for the OMAP architecture.

Quoted 7 lines
Quoted 5 lines

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),

Minor: AFAICT, this patch doesn't instrument CPUfreq.

The generic framework provides a cpu_frequency event. Furthermore the clock_set_rate events are reporting the various SoC clock rate changes.
Indeed the changelog is not accurate enough. Is '- cpufreq (DVFS) events are covered by the generic cpu_frequency events and the SoC clock changes events' better?

Quoted 5 lines
Quoted 5 lines

- SoC clocks changes (enable, disable, set_rate),
- power domain states: the desired target state and -if different- ? the actually hit state.

Quoted 8 lines
Quoted 5 lines

Because of the generic nature of the changes, OMAP3 and OMAP4 are supported.

Tested on OMAP3 with suspend/resume, cpuidle, basic DVFS.

With Paul's ack on the powerdomain changes, I'll queue this for 2.6.39 after making the above change in the changelog.

Thanks!
Jean

Quoted 156 lines

Kevin

Quoted 151 lines

---

?arch/arm/mach-omap2/clock.c ? ? ? | ? 11 +++++++++--
?arch/arm/mach-omap2/pm34xx.c ? ? ?| ? ?7 +++++++
?arch/arm/mach-omap2/powerdomain.c | ? 26 +++++++++++++++++++++++---

?3 files changed, 39 insertions(+), 5 deletions(-)

arch/arm/mach-omap2/clock.c +0 -0
index 46d03cc..180299e 100644
@@ -22,7 +22,9 @@

?#include <linux/clk.h>
?#include <linux/io.h>
?#include <linux/bitops.h>

arch/arm/mach-omap2/clock.c +2 -0
+#include <trace/events/power.h>
 
+#include <asm/cpu.h>

?#include <plat/clock.h>
?#include "clockdomain.h"
?#include <plat/cpu.h>

arch/arm/mach-omap2/clock.c +4 -1
@@ -261,8 +263,10 @@ void omap2_clk_disable(struct clk *clk)
 
 ? ? ? pr_debug("clock: %s: disabling in hardware\n", clk->name);
 
- ? ? if (clk->ops && clk->ops->disable)
+ ? ? if (clk->ops && clk->ops->disable) {
+ ? ? ? ? ? ? trace_clock_disable(clk->name, 0, smp_processor_id());
 ? ? ? ? ? ? ? clk->ops->disable(clk);
+ ? ? }
 
 ? ? ? if (clk->clkdm)
 ? ? ? ? ? ? ? clkdm_clk_disable(clk->clkdm, clk);
@@ -314,6 +318,7 @@ int omap2_clk_enable(struct clk *clk)
 ? ? ? }
 
 ? ? ? if (clk->ops && clk->ops->enable) {
+ ? ? ? ? ? ? trace_clock_enable(clk->name, 1, smp_processor_id());
 ? ? ? ? ? ? ? ret = clk->ops->enable(clk);
 ? ? ? ? ? ? ? if (ret) {
 ? ? ? ? ? ? ? ? ? ? ? WARN(1, "clock: %s: could not enable: %d\n",
@@ -353,8 +358,10 @@ int omap2_clk_set_rate(struct clk *clk, unsigned long rate)
 ? ? ? pr_debug("clock: set_rate for clock %s to rate %ld\n", clk->name, rate);
 

? ? ? /* dpll_ck, core_ck, virt_prcm_set; plus all clksel clocks */
- ? ? if (clk->set_rate)
+ ? ? if (clk->set_rate) {
+ ? ? ? ? ? ? trace_clock_set_rate(clk->name, rate, smp_processor_id()); ? ? ? ? ? ? ? ret = clk->set_rate(clk, rate);
+ ? ? }

? ? ? return ret;
?}

arch/arm/mach-omap2/pm34xx.c +0 -0
index bd610bc..d46b9ff 100644
@@ -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 "clockdomain.h"

arch/arm/mach-omap2/pm34xx.c +6 -0
@@ -514,8 +515,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();

arch/arm/mach-omap2/powerdomain.c +0 -0
index eaed0df..1495eed 100644
@@ -19,12 +19,15 @@

?#include <linux/list.h>
?#include <linux/errno.h>
?#include <linux/string.h>

arch/arm/mach-omap2/powerdomain.c +2 -0
+#include <trace/events/power.h>
+

?#include "cm2xxx_3xxx.h"
?#include "prcm44xx.h"
?#include "cm44xx.h"
?#include "prm2xxx_3xxx.h"
?#include "prm44xx.h"

+#include <asm/cpu.h>
?#include <plat/cpu.h>
?#include "powerdomain.h"
?#include "clockdomain.h"

arch/arm/mach-omap2/powerdomain.c +0 -0
@@ -32,6 +35,8 @@
 

?#include "pm.h"

arch/arm/mach-omap2/powerdomain.c +2 -0
+#define PWRDM_TRACE_STATES_FLAG ? ? ?(1<<31)
+

?enum {
? ? ? PWRDM_STATE_NOW = 0,
? ? ? PWRDM_STATE_PREV,

arch/arm/mach-omap2/powerdomain.c +12 -2
@@ -130,8 +135,7 @@ static void _update_logic_membank_counters(struct powerdomain *pwrdm)
 ?static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag)
 ?{
 
- ? ? int prev;
- ? ? int state;
+ ? ? int prev, state, trace_state = 0;
 
 ? ? ? if (pwrdm == NULL)
 ? ? ? ? ? ? ? return -EINVAL;
@@ -148,6 +152,17 @@ static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag)
 ? ? ? ? ? ? ? ? ? ? ? pwrdm->state_counter[prev]++;
 ? ? ? ? ? ? ? if (prev == PWRDM_POWER_RET)
 ? ? ? ? ? ? ? ? ? ? ? _update_logic_membank_counters(pwrdm);
+ ? ? ? ? ? ? /*
+ ? ? ? ? ? ? ?* If the power domain did not hit the desired state,
+ ? ? ? ? ? ? ?* generate a trace event with both the desired and hit states
+ ? ? ? ? ? ? ?*/
+ ? ? ? ? ? ? if (state != prev) {
+ ? ? ? ? ? ? ? ? ? ? trace_state = (PWRDM_TRACE_STATES_FLAG |
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?((state & OMAP_POWERSTATE_MASK) << 8) |
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?((prev & OMAP_POWERSTATE_MASK) << 0));
+ ? ? ? ? ? ? ? ? ? ? trace_power_domain_target(pwrdm->name, trace_state,
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? smp_processor_id());
+ ? ? ? ? ? ? }
 ? ? ? ? ? ? ? break;

? ? ? default:
? ? ? ? ? ? ? return -EINVAL;

arch/arm/mach-omap2/powerdomain.c +6 -1
@@ -406,8 +421,13 @@ int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
 ? ? ? pr_debug("powerdomain: setting next powerstate for %s to %0x\n",
 ? ? ? ? ? ? ? ?pwrdm->name, pwrst);
 
- ? ? if (arch_pwrdm && arch_pwrdm->pwrdm_set_next_pwrst)
+ ? ? if (arch_pwrdm && arch_pwrdm->pwrdm_set_next_pwrst) {
+ ? ? ? ? ? ? /* Trace the pwrdm desired target state */
+ ? ? ? ? ? ? trace_power_domain_target(pwrdm->name, pwrst,
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? smp_processor_id());
+ ? ? ? ? ? ? /* Program the pwrdm desired target state */
 ? ? ? ? ? ? ? ret = arch_pwrdm->pwrdm_set_next_pwrst(pwrdm, pwrst);
+ ? ? }
 
 ? ? ? return ret;
 ?}
#10 lore raw

PATCH 1/1 perf: add OMAP support for the new power events

Quoted 12 lines · Jean Pihet
Quoted 11 lines · Kevin Hilman
Quoted 8 lines · Jean Pihet

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),

Quoted 7 lines
Quoted 1 line

Minor: AFAICT, this patch doesn't instrument CPUfreq.

The generic framework provides a cpu_frequency event. Furthermore the clock_set_rate events are reporting the various SoC clock rate changes.
Indeed the changelog is not accurate enough. Is '- cpufreq (DVFS) events are covered by the generic cpu_frequency events and the SoC clock changes events' better?

I'd prefer to just leave this out as either way, I find it confusing based on the content of this patch.

Kevin

maintainer
#11 lore raw

PATCH 1/1 perf: add OMAP support for the new power events

In reply to Jean Pihet
Quoted 12 lines · Jean Pihet

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),
- SoC clocks changes (enable, disable, set_rate),
- power domain states: the desired target state and -if different- the actually hit state.

Quoted 5 lines

Because of the generic nature of the changes, OMAP3 and OMAP4 are supported.

Tested on OMAP3 with suspend/resume, cpuidle, basic DVFS.

In terms of tracing powerdomain state changes, since OMAP powerdomains can potentially transition without the MPU knowing about it, some powerdomain transitions will be missed by these. (The software counters miss them too.) The only way to be certain about these is to watch the debug observability lines. Still, it is the rare board that brings out debobs lines. So this seems reasonable, as long as people don't expect 100% coverage.

For the clock and powerdomain changes,

- Paul

author
#12 lore raw

PATCH 1/1 perf: add OMAP support for the new power events

Hi Paul,

Quoted 20 lines · Paul Walmsley
Quoted 12 lines · Jean Pihet

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),
- SoC clocks changes (enable, disable, set_rate),
- power domain states: the desired target state and -if different- ? the actually hit state.

Quoted 5 lines

Because of the generic nature of the changes, OMAP3 and OMAP4 are supported.

Tested on OMAP3 with suspend/resume, cpuidle, basic DVFS.

Quoted 7 lines

In terms of tracing powerdomain state changes, since OMAP powerdomains can potentially transition without the MPU knowing about it, some powerdomain transitions will be missed by these. ?(The software counters miss them too.) ?The only way to be certain about these is to watch the debug observability lines. ?Still, it is the rare board that brings out debobs lines. ?So this seems reasonable, as long as people don't expect 100% coverage.

OK that is correct. The events are tracing SW events only, i.e. a trace is generated when a decision is made wrt next power states, clock changes etc.
A remark: the OMAP4+ HW tracing modules do support the changes of power domains states. There definitely is more to come on that topic!

Thanks for reviewing!
Jean

Quoted 8 lines

For the clock and powerdomain changes,

- Paul