Skip to contentSkip to conversation

arm: omap: clk/clkdm/pwrdm/voltdm changes to usecounting

PATCH Superseded Tero Kristo 4 participants 19 messages Feb 14, 2012 – Feb 16, 2012

Subsystems (5)

v1 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

cover arm: omap: clk/clkdm/pwrdm/voltdm changes to usecounting

This set is split out from a larger set earlier circulated on linux-omap list. The main purpose of this set is to provide useful recursive usecounting support for clockdomains / powerdomains / voltagedomains which can be used for debugging purposes and also to control certain chip power features once a sleep condition is reached (voltdm usecount=0).

This set provides only the baseline support for the usecounting, and also adds a pm-debug related patch that can be used to check out stuff (patch 6), and should later prove rather useful for debugging. Rest of the functionality will be posted in separate sets.

This set has been tested on omap3 beagle board with suspend / resume / cpuidle. Also boot tested on omap4 panda (sleep is not functional yet on that platform.)

Applies on mainline kernel.

-Tero

#1 lore raw

PATCH 1/6 arm: omap: clk: add support for omap_clk_for_each

This works similarly to e.g. pwrdm_for_each(). Needed by enhanced usecounting debug functionality that will be added to pm-debug.

Cc Paul Walmsley <[email protected]>
Cc Kevin Hilman <[email protected]>
---
 arch/arm/plat-omap/clock.c              |   30 ++++++++++++++++++++++++++++++
 arch/arm/plat-omap/include/plat/clock.h |    2 ++
 2 files changed, 32 insertions(+), 0 deletions(-)
arch/arm/plat-omap/clock.c +30 -0
index 567e4b5..e17ee5e2 100644
@@ -356,6 +356,36 @@ int omap_clk_enable_autoidle_all(void)
 	return 0;
 }
 
+/**
+ * omap_clk_for_each - call a function for each registered clock
+ * @fn: pointer to callback function
+ * @data: void * data to pass to callback function
+ *
+ * Call @fn for each registered clock, passing @data to each function.
+ * @fn must return 0 for success or any other value for failure. If
+ * @fn returns non-zero, the iteration across clocks will stop and
+ * the non-zero return value will be passed to the caller of
+ * omap_clk_for_each(). @fn is called with clockfw_lock held.
+ */
+int omap_clk_for_each(int (*fn)(struct clk *clk, void *user), void *user)
+{
+	struct clk *c;
+	unsigned long flags;
+	int ret = 0;
+
+	spin_lock_irqsave(&clockfw_lock, flags);
+
+	list_for_each_entry(c, &clocks, node) {
+		ret = fn(c, user);
+		if (ret)
+			break;
+	}
+
+	spin_unlock_irqrestore(&clockfw_lock, flags);
+
+	return ret;
+}
+
 int omap_clk_disable_autoidle_all(void)
 {
 	struct clk *c;
arch/arm/plat-omap/include/plat/clock.h +2 -0
index 240a7b9..c84f2a4 100644
@@ -300,6 +300,8 @@ extern void propagate_rate(struct clk *clk);
 extern void recalculate_root_clocks(void);
 extern unsigned long followparent_recalc(struct clk *clk);
 extern void clk_enable_init_clocks(void);
+extern int omap_clk_for_each(int (*fn)(struct clk *clk, void *user),
+				void *user);
 unsigned long omap_fixed_divisor_recalc(struct clk *clk);
 #ifdef CONFIG_CPU_FREQ
 extern void clk_init_cpufreq_table(struct cpufreq_frequency_table **table);
Signature · 1 line
1.7.4.1
#2 lore raw

PATCH 2/6 arm: omap3+: voltage/pwrdm/clkdm/clock add recursive usecount tracking

In reply to Tero Kristo

This patch fixes the usecount tracking for omap3+, previously the usecount numbers were rather bogus and were not really useful for any purpose. Now usecount numbers track the number of really active clients on each domain. This patch also adds support for usecount tracking on powerdomain level and autoidle flag for clocks that are hardware controlled and should be skipped in usecount calculations.

Cc Paul Walmsley <[email protected]>
Cc Kevin Hilman <[email protected]>
---
 arch/arm/mach-omap2/clkt_iclk.c         |   21 +++++++++++++++++++
 arch/arm/mach-omap2/clockdomain.c       |   34 +++++++++++++++++++++++++++++-
 arch/arm/mach-omap2/clockdomain.h       |    2 +
 arch/arm/mach-omap2/powerdomain.c       |   10 +++++++++
 arch/arm/mach-omap2/powerdomain.h       |    5 ++++
 arch/arm/plat-omap/clock.c              |    6 +++++
 arch/arm/plat-omap/include/plat/clock.h |    2 +
 7 files changed, 78 insertions(+), 2 deletions(-)
arch/arm/mach-omap2/clkt_iclk.c +21 -0
index 3d43fba..f8c2e77 100644
@@ -21,6 +21,7 @@
 #include "clock2xxx.h"
 #include "cm2xxx_3xxx.h"
 #include "cm-regbits-24xx.h"
+#include "clockdomain.h"
 
 /* Private functions */
 
@@ -34,6 +35,16 @@ void omap2_clkt_iclk_allow_idle(struct clk *clk)
 	v = __raw_readl((__force void __iomem *)r);
 	v |= (1 << clk->enable_bit);
 	__raw_writel(v, (__force void __iomem *)r);
+
+	/* Remove this clock from parent clockdomain usecounts */
+	if (clk->usecount && clk->clkdm)
+		clkdm_usecount_dec(clk->clkdm);
+
+	/*
+	 * Mark as autoidle, so we continue to ignore this clock in
+	 * parent clkdm usecount calculations
+	 */
+	clk->autoidle = true;
 }
 
 /* XXX */
@@ -46,6 +57,16 @@ void omap2_clkt_iclk_deny_idle(struct clk *clk)
 	v = __raw_readl((__force void __iomem *)r);
 	v &= ~(1 << clk->enable_bit);
 	__raw_writel(v, (__force void __iomem *)r);
+
+	/* Add clock back to parent clockdomain usecount */
+	if (clk->usecount && clk->clkdm)
+		clkdm_usecount_inc(clk->clkdm);
+
+	/*
+	 * Disable autoidle flag so further clkdm usecounts take this
+	 * clock into account
+	 */
+	clk->autoidle = false;
 }
 
 /* Public data */
arch/arm/mach-omap2/clockdomain.c +32 -2
index ad07689..c0b6187 100644
@@ -907,6 +907,28 @@ bool clkdm_in_hwsup(struct clockdomain *clkdm)
 
 /* Clockdomain-to-clock/hwmod framework interface code */
 
+int clkdm_usecount_inc(struct clockdomain *clkdm)
+{
+	int usecount;
+
+	usecount = atomic_inc_return(&clkdm->usecount);
+
+	if (usecount == 1)
+		pwrdm_clkdm_enable(clkdm->pwrdm.ptr);
+	return usecount;
+}
+
+int clkdm_usecount_dec(struct clockdomain *clkdm)
+{
+	int usecount;
+
+	usecount = atomic_dec_return(&clkdm->usecount);
+
+	if (usecount == 0)
+		pwrdm_clkdm_disable(clkdm->pwrdm.ptr);
+	return usecount;
+}
+
 static int _clkdm_clk_hwmod_enable(struct clockdomain *clkdm)
 {
 	unsigned long flags;
@@ -919,7 +941,7 @@ static int _clkdm_clk_hwmod_enable(struct clockdomain *clkdm)
 	 * should be called for every clock instance or hwmod that is
 	 * enabled, so the clkdm can be force woken up.
 	 */
-	if ((atomic_inc_return(&clkdm->usecount) > 1) && autodeps)
+	if ((clkdm_usecount_inc(clkdm) > 1) && autodeps)
 		return 0;
 
 	spin_lock_irqsave(&clkdm->lock, flags);
@@ -945,7 +967,7 @@ static int _clkdm_clk_hwmod_disable(struct clockdomain *clkdm)
 		return -ERANGE;
 	}
 
-	if (atomic_dec_return(&clkdm->usecount) > 0)
+	if (clkdm_usecount_dec(clkdm) > 0)
 		return 0;
 
 	spin_lock_irqsave(&clkdm->lock, flags);
@@ -982,6 +1004,10 @@ int clkdm_clk_enable(struct clockdomain *clkdm, struct clk *clk)
 	if (!clk)
 		return -EINVAL;
 
+	/* If autoidle clock, do not update clkdm usecounts */
+	if (clk->autoidle)
+		return 0;
+
 	return _clkdm_clk_hwmod_enable(clkdm);
 }
 
@@ -1008,6 +1034,10 @@ int clkdm_clk_disable(struct clockdomain *clkdm, struct clk *clk)
 	if (!clk)
 		return -EINVAL;
 
+	/* If autoidle clock, do not update clkdm usecounts */
+	if (clk->autoidle)
+		return 0;
+
 	return _clkdm_clk_hwmod_disable(clkdm);
 }
 
arch/arm/mach-omap2/clockdomain.h +2 -0
index f7b5860..373399a 100644
@@ -191,6 +191,8 @@ int clkdm_clk_enable(struct clockdomain *clkdm, struct clk *clk);
 int clkdm_clk_disable(struct clockdomain *clkdm, struct clk *clk);
 int clkdm_hwmod_enable(struct clockdomain *clkdm, struct omap_hwmod *oh);
 int clkdm_hwmod_disable(struct clockdomain *clkdm, struct omap_hwmod *oh);
+int clkdm_usecount_inc(struct clockdomain *clkdm);
+int clkdm_usecount_dec(struct clockdomain *clkdm);
 
 extern void __init omap242x_clockdomains_init(void);
 extern void __init omap243x_clockdomains_init(void);
arch/arm/mach-omap2/powerdomain.c +10 -0
index 8a18d1b..78bf3c0 100644
@@ -985,6 +985,16 @@ int pwrdm_clkdm_state_switch(struct clockdomain *clkdm)
 	return -EINVAL;
 }
 
+void pwrdm_clkdm_enable(struct powerdomain *pwrdm)
+{
+	atomic_inc(&pwrdm->usecount);
+}
+
+void pwrdm_clkdm_disable(struct powerdomain *pwrdm)
+{
+	atomic_dec(&pwrdm->usecount);
+}
+
 int pwrdm_pre_transition(void)
 {
 	pwrdm_for_each(_pwrdm_pre_transition_cb, NULL);
arch/arm/mach-omap2/powerdomain.h +5 -0
index 0d72a8a..32f44c8 100644
@@ -120,6 +120,7 @@ struct powerdomain {
 	unsigned state_counter[PWRDM_MAX_PWRSTS];
 	unsigned ret_logic_off_counter;
 	unsigned ret_mem_off_counter[PWRDM_MAX_MEM_BANKS];
+	atomic_t usecount;
 
 #ifdef CONFIG_PM_DEBUG
 	s64 timer;
@@ -216,6 +217,10 @@ int pwrdm_state_switch(struct powerdomain *pwrdm);
 int pwrdm_clkdm_state_switch(struct clockdomain *clkdm);
 int pwrdm_pre_transition(void);
 int pwrdm_post_transition(void);
+
+void pwrdm_clkdm_enable(struct powerdomain *pwrdm);
+void pwrdm_clkdm_disable(struct powerdomain *pwrdm);
+
 int pwrdm_set_lowpwrstchange(struct powerdomain *pwrdm);
 int pwrdm_get_context_loss_count(struct powerdomain *pwrdm);
 bool pwrdm_can_ever_lose_context(struct powerdomain *pwrdm);
arch/arm/plat-omap/clock.c +6 -0
index e17ee5e2..1f80df8 100644
@@ -283,6 +283,12 @@ int clk_register(struct clk *clk)
 		list_add(&clk->sibling, &root_clks);
 
 	list_add(&clk->node, &clocks);
+	/*
+	 * If clock has no ops, it is handled by hardware and thus will
+	 * idle automatically
+	 */
+	if (clk->ops == &clkops_null)
+		clk->autoidle = true;
 	if (clk->init)
 		clk->init(clk);
 	mutex_unlock(&clocks_mutex);
arch/arm/plat-omap/include/plat/clock.h +2 -0
index c84f2a4..14b44c0 100644
@@ -208,6 +208,7 @@ struct dpll_data {
  * @init: fn ptr to do clock-specific initialization
  * @enable_bit: bitshift to write to enable/disable the clock (see @enable_reg)
  * @usecount: number of users that have requested this clock to be enabled
+ * @autoidle: indicates hardware controlled clock (not used in domain usecounts)
  * @fixed_div: when > 0, this clock's rate is its parent's rate / @fixed_div
  * @flags: see "struct clk.flags possibilities" above
  * @clksel_reg: for clksel clks, register va containing src/divisor select
@@ -254,6 +255,7 @@ struct clk {
 	void			(*init)(struct clk *);
 	u8			enable_bit;
 	s8			usecount;
+	bool			autoidle;
 	u8			fixed_div;
 	u8			flags;
 #ifdef CONFIG_ARCH_OMAP2PLUS
Signature · 1 line
1.7.4.1
#3 lore raw

PATCH 3/6 arm: omap3+: voltage: add support for voltagedomain usecounts

In reply to Tero Kristo

These are updated based on powerdomain usecounts. Also added support for voltdm->sleep and voltdm->wakeup calls that will be invoked once voltagedomain enters sleep or wakes up based on usecount numbers. These will be used for controlling voltage scaling functionality.

Cc Paul Walmsley <[email protected]>
Cc Kevin Hilman <[email protected]>
---
 arch/arm/mach-omap2/powerdomain.c |    6 +++-
 arch/arm/mach-omap2/voltage.c     |   48 +++++++++++++++++++++++++++++++++++++
 arch/arm/mach-omap2/voltage.h     |   11 ++++++++
 3 files changed, 63 insertions(+), 2 deletions(-)
arch/arm/mach-omap2/powerdomain.c +4 -2
index 78bf3c0..4ded0c8 100644
@@ -987,12 +987,14 @@ int pwrdm_clkdm_state_switch(struct clockdomain *clkdm)
 
 void pwrdm_clkdm_enable(struct powerdomain *pwrdm)
 {
-	atomic_inc(&pwrdm->usecount);
+	if (atomic_inc_return(&pwrdm->usecount) == 1)
+		voltdm_pwrdm_enable(pwrdm->voltdm.ptr);
 }
 
 void pwrdm_clkdm_disable(struct powerdomain *pwrdm)
 {
-	atomic_dec(&pwrdm->usecount);
+	if (!atomic_dec_return(&pwrdm->usecount))
+		voltdm_pwrdm_disable(pwrdm->voltdm.ptr);
 }
 
 int pwrdm_pre_transition(void)
arch/arm/mach-omap2/voltage.c +48 -0

60 lines not on this page · raw message

arch/arm/mach-omap2/voltage.h +11 -0
index 16a1b09..8829201 100644
@@ -54,10 +54,14 @@ struct omap_vfsm_instance {
  * @pwrdm_list: list_head linking all powerdomains in this voltagedomain
  * @vc: pointer to VC channel associated with this voltagedomain
  * @vp: pointer to VP associated with this voltagedomain
+ * @usecount: number of users for this voltagedomain
+ * @target_state: calculated target state for the children of this domain
  * @read: read a VC/VP register
  * @write: write a VC/VP register
  * @read: read-modify-write a VC/VP register
  * @sys_clk: system clock name/frequency, used for various timing calculations
+ * @sleep: function to call once the domain enters idle
+ * @wakeup: function to call once the domain wakes up from idle
  * @scale: function used to scale the voltage of the voltagedomain
  * @nominal_volt: current nominal voltage for this voltage domain
  * @volt_data: voltage table having the distinct voltages supported
@@ -73,6 +77,9 @@ struct voltagedomain {
 	struct omap_vp_instance *vp;
 	struct omap_voltdm_pmic *pmic;
 
+	atomic_t usecount;
+	int target_state;
+
 	/* VC/VP register access functions: SoC specific */
 	u32 (*read) (u8 offset);
 	void (*write) (u32 val, u8 offset);
@@ -83,6 +90,8 @@ struct voltagedomain {
 		u32 rate;
 	} sys_clk;
 
+	void (*sleep) (struct voltagedomain *voltdm);
+	void (*wakeup) (struct voltagedomain *voltdm);
 	int (*scale) (struct voltagedomain *voltdm,
 		      unsigned long target_volt);
 
@@ -161,6 +170,8 @@ extern void omap44xx_voltagedomains_init(void);
 struct voltagedomain *voltdm_lookup(const char *name);
 void voltdm_init(struct voltagedomain **voltdm_list);
 int voltdm_add_pwrdm(struct voltagedomain *voltdm, struct powerdomain *pwrdm);
+void voltdm_pwrdm_enable(struct voltagedomain *voltdm);
+void voltdm_pwrdm_disable(struct voltagedomain *voltdm);
 int voltdm_for_each(int (*fn)(struct voltagedomain *voltdm, void *user),
 		    void *user);
 int voltdm_for_each_pwrdm(struct voltagedomain *voltdm,
Signature · 1 line
1.7.4.1
#4 lore raw

PATCH 4/6 arm: omap3: add manual control for mpu_iva voltdm usecounting

In reply to Tero Kristo

mpu_iva voltdm usecount is now decreased during idle. This will allow the voltagedomain to reflect actual usage, and will allow proper usage of vc callbacks.

Cc Paul Walmsley <[email protected]>
Cc Kevin Hilman <[email protected]>
---
 arch/arm/mach-omap2/pm34xx.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)
arch/arm/mach-omap2/pm34xx.c +8 -0
index fc69875..14defaf 100644
@@ -74,6 +74,7 @@ void (*omap3_do_wfi_sram)(void);
 static struct powerdomain *mpu_pwrdm, *neon_pwrdm;
 static struct powerdomain *core_pwrdm, *per_pwrdm;
 static struct powerdomain *cam_pwrdm;
+static struct voltagedomain *mpu_iva_voltdm;
 
 static inline void omap3_per_save_context(void)
 {
@@ -311,6 +312,8 @@ void omap_sram_idle(void)
 		return;
 	}
 
+	voltdm_pwrdm_disable(mpu_iva_voltdm);
+
 	/* NEON control */
 	if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON)
 		pwrdm_set_next_pwrst(neon_pwrdm, mpu_next_state);
@@ -393,6 +396,8 @@ void omap_sram_idle(void)
 	}
 	omap3_intc_resume_idle();
 
+	voltdm_pwrdm_enable(mpu_iva_voltdm);
+
 	pwrdm_post_transition();
 
 	/* PER */
@@ -828,6 +833,9 @@ static int __init omap3_pm_init(void)
 
 	(void) clkdm_for_each(clkdms_setup, NULL);
 
+	mpu_iva_voltdm = voltdm_lookup("mpu_iva");
+	voltdm_pwrdm_enable(mpu_iva_voltdm);
+
 	mpu_pwrdm = pwrdm_lookup("mpu_pwrdm");
 	if (mpu_pwrdm == NULL) {
 		printk(KERN_ERR "Failed to get mpu_pwrdm\n");
Signature · 1 line
1.7.4.1
#5 lore raw

PATCH 4/6 arm: omap3: add manual control for mpu_iva voltdm usecounting

Quoted 3 lines · Tero Kristo

mpu_iva voltdm usecount is now decreased during idle. This will allow the voltagedomain to reflect actual usage, and will allow proper usage of vc callbacks.

I don't follow why this is needed in the idle path.

Why aren't the usecounts from clock/powerdomain disables (enables) causing this voltdm disable (enable) to happen automatcially?

If these are needed, seems like they should be in the pwrdm_[pre|post]_transition() hooks so they can be generic.

Kevin

Quoted 47 lines
Cc Paul Walmsley <[email protected]>
Cc Kevin Hilman <[email protected]>
---
 arch/arm/mach-omap2/pm34xx.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)
arch/arm/mach-omap2/pm34xx.c +8 -0
index fc69875..14defaf 100644
@@ -74,6 +74,7 @@ void (*omap3_do_wfi_sram)(void);
 static struct powerdomain *mpu_pwrdm, *neon_pwrdm;
 static struct powerdomain *core_pwrdm, *per_pwrdm;
 static struct powerdomain *cam_pwrdm;
+static struct voltagedomain *mpu_iva_voltdm;
 
 static inline void omap3_per_save_context(void)
 {
@@ -311,6 +312,8 @@ void omap_sram_idle(void)
 		return;
 	}
 
+	voltdm_pwrdm_disable(mpu_iva_voltdm);
+
 	/* NEON control */
 	if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON)
 		pwrdm_set_next_pwrst(neon_pwrdm, mpu_next_state);
@@ -393,6 +396,8 @@ void omap_sram_idle(void)
 	}
 	omap3_intc_resume_idle();
 
+	voltdm_pwrdm_enable(mpu_iva_voltdm);
+
 	pwrdm_post_transition();
 
 	/* PER */
@@ -828,6 +833,9 @@ static int __init omap3_pm_init(void)
 
 	(void) clkdm_for_each(clkdms_setup, NULL);
 
+	mpu_iva_voltdm = voltdm_lookup("mpu_iva");
+	voltdm_pwrdm_enable(mpu_iva_voltdm);
+
 	mpu_pwrdm = pwrdm_lookup("mpu_pwrdm");
 	if (mpu_pwrdm == NULL) {
 		printk(KERN_ERR "Failed to get mpu_pwrdm\n");
#6 lore raw

PATCH 4/6 arm: omap3: add manual control for mpu_iva voltdm usecounting

Quoted 10 lines · Kevin Hilman
Quoted 3 lines · Tero Kristo

mpu_iva voltdm usecount is now decreased during idle. This will allow the voltagedomain to reflect actual usage, and will allow proper usage of vc callbacks.

I don't follow why this is needed in the idle path.

Why aren't the usecounts from clock/powerdomain disables (enables) causing this voltdm disable (enable) to happen automatcially?

The main issue is mpu powerdomain which doesn't contain any clocks that are controlled actively. Without adding a "virtual" usecount enable/disable hook, the usecount for mpu_iva will always remain at 0 (or just follow the iva domain on omap3.) We want the usecount to reflect the actual idling of this domain also, so that we can control SR / auto-ret based on MPU activity.

Quoted 2 lines

If these are needed, seems like they should be in the pwrdm_[pre|post]_transition() hooks so they can be generic.

This is actually a good idea, I could move them here. I could also add a manual triggering for core domain here, as I have a feeling it might be good to have so that core will also follow the idle cycle more tightly (basically as a virtual follow for sdrc clock which is idled by HW during wfi.)

-Tero

Quoted 50 lines

Kevin

Quoted 47 lines
Cc Paul Walmsley <[email protected]>
Cc Kevin Hilman <[email protected]>
---
 arch/arm/mach-omap2/pm34xx.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)
arch/arm/mach-omap2/pm34xx.c +8 -0
index fc69875..14defaf 100644
@@ -74,6 +74,7 @@ void (*omap3_do_wfi_sram)(void);
 static struct powerdomain *mpu_pwrdm, *neon_pwrdm;
 static struct powerdomain *core_pwrdm, *per_pwrdm;
 static struct powerdomain *cam_pwrdm;
+static struct voltagedomain *mpu_iva_voltdm;
 
 static inline void omap3_per_save_context(void)
 {
@@ -311,6 +312,8 @@ void omap_sram_idle(void)
 		return;
 	}
 
+	voltdm_pwrdm_disable(mpu_iva_voltdm);
+
 	/* NEON control */
 	if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON)
 		pwrdm_set_next_pwrst(neon_pwrdm, mpu_next_state);
@@ -393,6 +396,8 @@ void omap_sram_idle(void)
 	}
 	omap3_intc_resume_idle();
 
+	voltdm_pwrdm_enable(mpu_iva_voltdm);
+
 	pwrdm_post_transition();
 
 	/* PER */
@@ -828,6 +833,9 @@ static int __init omap3_pm_init(void)
 
 	(void) clkdm_for_each(clkdms_setup, NULL);
 
+	mpu_iva_voltdm = voltdm_lookup("mpu_iva");
+	voltdm_pwrdm_enable(mpu_iva_voltdm);
+
 	mpu_pwrdm = pwrdm_lookup("mpu_pwrdm");
 	if (mpu_pwrdm == NULL) {
 		printk(KERN_ERR "Failed to get mpu_pwrdm\n");
#7 lore raw

PATCH 4/6 arm: omap3: add manual control for mpu_iva voltdm usecounting

Quoted 9 lines · Tero Kristo
Quoted 8 lines · Kevin Hilman
Quoted 3 lines · Tero Kristo

mpu_iva voltdm usecount is now decreased during idle. This will allow the voltagedomain to reflect actual usage, and will allow proper usage of vc callbacks.

I don't follow why this is needed in the idle path.

Quoted 7 lines
Quoted 2 lines

Why aren't the usecounts from clock/powerdomain disables (enables) causing this voltdm disable (enable) to happen automatcially?

The main issue is mpu powerdomain which doesn't contain any clocks that are controlled actively. Without adding a "virtual" usecount enable/disable hook, the usecount for mpu_iva will always remain at 0 (or just follow the iva domain on omap3.)

Rather than faking the MPU clock gating by disabling the voltage/power
domains, why not just gate the MPU clock in idle? That would trigger
all the rest and would also be much clearer what is happening in the idle path.

Quoted 2 lines

We want the usecount to reflect the actual idling of this domain also, so that we can control SR / auto-ret based on MPU activity.

Yes, makes sense now. Thanks for the explanation.

Quoted 8 lines
Quoted 2 lines

If these are needed, seems like they should be in the pwrdm_[pre|post]_transition() hooks so they can be generic.

This is actually a good idea, I could move them here. I could also add a manual triggering for core domain here, as I have a feeling it might be good to have so that core will also follow the idle cycle more tightly (basically as a virtual follow for sdrc clock which is idled by HW during wfi.)

In the comments on the new version, you should also describe in detail the reasons for the "manual" idling of CORE (due to clocks that are hardware managed.)

Kevin

#8 lore raw

PATCH 5/6 arm: omap3: set autoidle flags for a few clocks

In reply to Tero Kristo

dpll3, dpll4 and sdrc_ick are controlled automatically by hardware. Thus, reflect this with the autoidle flags, the clocks will no longer show as active in usecount dumps and will allow the voltdm->sleep / wakeup calls to work properly.

Cc Paul Walmsley <[email protected]>
Cc Kevin Hilman <[email protected]>
---
 arch/arm/mach-omap2/clock3xxx_data.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
arch/arm/mach-omap2/clock3xxx_data.c +3 -0
index d75e5f6..f3ee983 100644
@@ -432,6 +432,7 @@ static struct clk dpll3_ck = {
 	.round_rate	= &omap2_dpll_round_rate,
 	.clkdm_name	= "dpll3_clkdm",
 	.recalc		= &omap3_dpll_recalc,
+	.autoidle	= true,
 };
 
 /*
@@ -615,6 +616,7 @@ static struct clk dpll4_ck = {
 	.set_rate	= &omap3_dpll4_set_rate,
 	.clkdm_name	= "dpll4_clkdm",
 	.recalc		= &omap3_dpll_recalc,
+	.autoidle	= true,
 };
 
 /*
@@ -1744,6 +1746,7 @@ static struct clk sdrc_ick = {
 	.flags		= ENABLE_ON_INIT,
 	.clkdm_name	= "core_l3_clkdm",
 	.recalc		= &followparent_recalc,
+	.autoidle	= true,
 };
 
 static struct clk gpmc_fck = {
Signature · 1 line
1.7.4.1
#9 lore raw

PATCH 6/6 arm: omap: pm-debug: enhanced usecount debug support

In reply to Tero Kristo

Voltdm, pwrdm, clkdm, hwmod and clk usecounts are now separeted to their own file, 'usecount'. This file shows the usecounts for every active domain and their children recursively. 'count' file now only shows power state counts for powerdomains.

This patch also provices a way to do printk dumps from kernel code, by calling the pm_dbg_dump_X functions. The plan is to call these functions once an error condition is detected, e.g. failed suspend.

Cc Paul Walmsley <[email protected]>
Cc Kevin Hilman <[email protected]>
---
 arch/arm/mach-omap2/pm-debug.c |  128 ++++++++++++++++++++++++++++++++++------
 arch/arm/mach-omap2/pm.h       |    6 ++
 2 files changed, 115 insertions(+), 19 deletions(-)
arch/arm/mach-omap2/pm-debug.c +109 -19
index 4411163..444588e 100644
@@ -51,6 +51,7 @@ static int pm_dbg_init(void);
 enum {
 	DEBUG_FILE_COUNTERS = 0,
 	DEBUG_FILE_TIMERS,
+	DEBUG_FILE_USECOUNT,
 };
 
 static const char pwrdm_state_names[][PWRDM_MAX_PWRSTS] = {
@@ -75,23 +76,6 @@ void pm_dbg_update_time(struct powerdomain *pwrdm, int prev)
 	pwrdm->timer = t;
 }
 
-static int clkdm_dbg_show_counter(struct clockdomain *clkdm, void *user)
-{
-	struct seq_file *s = (struct seq_file *)user;
-
-	if (strcmp(clkdm->name, "emu_clkdm") == 0 ||
-		strcmp(clkdm->name, "wkup_clkdm") == 0 ||
-		strncmp(clkdm->name, "dpll", 4) == 0)
-		return 0;
-
-	seq_printf(s, "%s->%s (%d)", clkdm->name,
-			clkdm->pwrdm.ptr->name,
-			atomic_read(&clkdm->usecount));
-	seq_printf(s, "\n");
-
-	return 0;
-}
-
 static int pwrdm_dbg_show_counter(struct powerdomain *pwrdm, void *user)
 {
 	struct seq_file *s = (struct seq_file *)user;
@@ -145,11 +129,112 @@ static int pwrdm_dbg_show_timer(struct powerdomain *pwrdm, void *user)
 	return 0;
 }
 
+static struct voltagedomain *parent_voltdm;
+static struct powerdomain *parent_pwrdm;
+static struct clockdomain *parent_clkdm;
+
+#define PM_DBG_PRINT(s, fmt, args...)			\
+	{						\
+		if (s)					\
+			seq_printf(s, fmt, ## args);	\
+		else					\
+			pr_info(fmt, ## args);		\
+	}
+
+static int _pm_dbg_dump_clk(struct clk *clk, void *user)
+{
+	struct seq_file *s = user;
+
+	if (clk->clkdm == parent_clkdm && clk->usecount && !clk->autoidle)
+		PM_DBG_PRINT(s, "      ck:%s: %d\n", clk->name, clk->usecount);
+
+	return 0;
+}
+
+static int _pm_dbg_dump_hwmod(struct omap_hwmod *oh, void *user)
+{
+	struct seq_file *s = user;
+
+	if (oh->clkdm != parent_clkdm)
+		return 0;
+
+	if (oh->_state != _HWMOD_STATE_ENABLED)
+		return 0;
+
+	PM_DBG_PRINT(s, "      oh:%s: enabled\n", oh->name);
+
+	return 0;
+}
+
+static int _pm_dbg_dump_clkdm(struct clockdomain *clkdm, void *user)
+{
+	struct seq_file *s = user;
+	u32 usecount;
+
+	if (clkdm->pwrdm.ptr == parent_pwrdm) {
+		usecount = atomic_read(&clkdm->usecount);
+		if (usecount) {
+			PM_DBG_PRINT(s, "    cd:%s: %d\n", clkdm->name,
+				usecount);
+			parent_clkdm = clkdm;
+			omap_hwmod_for_each(_pm_dbg_dump_hwmod, s);
+			omap_clk_for_each(_pm_dbg_dump_clk, s);
+		}
+	}
+	return 0;
+}
+
+static int _pm_dbg_dump_pwrdm(struct powerdomain *pwrdm, void *user)
+{
+	struct seq_file *s = user;
+	u32 usecount;
+
+	if (pwrdm->voltdm.ptr == parent_voltdm) {
+		usecount = atomic_read(&pwrdm->usecount);
+		if (usecount) {
+			PM_DBG_PRINT(s, "  pd:%s: %d\n", pwrdm->name, usecount);
+			parent_pwrdm = pwrdm;
+			clkdm_for_each(_pm_dbg_dump_clkdm, s);
+		}
+	}
+	return 0;
+}
+
+void pm_dbg_dump_pwrdm(struct powerdomain *pwrdm)
+{
+	pr_info("pd:%s: %d\n", pwrdm->name, atomic_read(&pwrdm->usecount));
+	parent_pwrdm = pwrdm;
+	clkdm_for_each(_pm_dbg_dump_clkdm, NULL);
+}
+
+void pm_dbg_dump_voltdm(struct voltagedomain *voltdm)
+{
+	pr_info("vd:%s: %d\n", voltdm->name, atomic_read(&voltdm->usecount));
+	parent_voltdm = voltdm;
+	pwrdm_for_each(_pm_dbg_dump_pwrdm, NULL);
+}
+
+static int _voltdm_dbg_show_counters(struct voltagedomain *voltdm, void *user)
+{
+	struct seq_file *s = user;
+
+	seq_printf(s, "vd:%s: %d\n", voltdm->name,
+		atomic_read(&voltdm->usecount));
+
+	parent_voltdm = voltdm;
+	pwrdm_for_each(_pm_dbg_dump_pwrdm, s);
+	return 0;
+}
+
+static int pm_dbg_show_usecount(struct seq_file *s, void *unused)
+{
+	voltdm_for_each(_voltdm_dbg_show_counters, s);
+	return 0;
+}
+
 static int pm_dbg_show_counters(struct seq_file *s, void *unused)
 {
 	pwrdm_for_each(pwrdm_dbg_show_counter, s);
-	clkdm_for_each(clkdm_dbg_show_counter, s);
-
 	return 0;
 }
 
@@ -162,6 +247,9 @@ static int pm_dbg_show_timers(struct seq_file *s, void *unused)
 static int pm_dbg_open(struct inode *inode, struct file *file)
 {
 	switch ((int)inode->i_private) {
+	case DEBUG_FILE_USECOUNT:
+		return single_open(file, pm_dbg_show_usecount,
+			&inode->i_private);
 	case DEBUG_FILE_COUNTERS:
 		return single_open(file, pm_dbg_show_counters,
 			&inode->i_private);
@@ -271,6 +359,8 @@ static int __init pm_dbg_init(void)
 		d, (void *)DEBUG_FILE_COUNTERS, &debug_fops);
 	(void) debugfs_create_file("time", S_IRUGO,
 		d, (void *)DEBUG_FILE_TIMERS, &debug_fops);
+	(void) debugfs_create_file("usecount", S_IRUGO,
+		d, (void *)DEBUG_FILE_USECOUNT, &debug_fops);
 
 	pwrdm_for_each(pwrdms_setup, (void *)d);
 
arch/arm/mach-omap2/pm.h +6 -0
index b737b11..ec8f874 100644
@@ -61,10 +61,16 @@ inline void omap3_pm_init_cpuidle(struct cpuidle_params *cpuidle_board_params)
 extern int omap3_pm_get_suspend_state(struct powerdomain *pwrdm);
 extern int omap3_pm_set_suspend_state(struct powerdomain *pwrdm, int state);
 
+struct clk;
+
 #ifdef CONFIG_PM_DEBUG
 extern u32 enable_off_mode;
+extern void pm_dbg_dump_pwrdm(struct powerdomain *pwrdm);
+extern void pm_dbg_dump_voltdm(struct voltagedomain *voltdm);
 #else
 #define enable_off_mode 0
+static inline void pm_dbg_dump_pwrdm(struct powerdomain *pwrdm) { }
+static inline void pm_dbg_dump_voltdm(struct voltagedomain *voltdm) { }
 #endif
 
 #if defined(CONFIG_PM_DEBUG) && defined(CONFIG_DEBUG_FS)
Signature · 1 line
1.7.4.1
maintainer
#10 lore raw

PATCH 6/6 arm: omap: pm-debug: enhanced usecount debug support

Quoted 8 lines · Tero Kristo

Voltdm, pwrdm, clkdm, hwmod and clk usecounts are now separeted to their own file, 'usecount'. This file shows the usecounts for every active domain and their children recursively. 'count' file now only shows power state counts for powerdomains.

This patch also provices a way to do printk dumps from kernel code, by calling the pm_dbg_dump_X functions. The plan is to call these functions once an error condition is detected, e.g. failed suspend.

Why don't you replace this all with a userspace tool that deciphers the registers for you?

Tony

#11 lore raw

PATCH 6/6 arm: omap: pm-debug: enhanced usecount debug support

Quoted 6 lines · Tony Lindgren
Quoted 5 lines · Tero Kristo

Voltdm, pwrdm, clkdm, hwmod and clk usecounts are now separeted to their own file, 'usecount'. This file shows the usecounts for every active domain and their children recursively. 'count' file now only shows power state counts for powerdomains.

Quoted 6 lines
Quoted 3 lines

This patch also provices a way to do printk dumps from kernel code, by calling the pm_dbg_dump_X functions. The plan is to call these functions once an error condition is detected, e.g. failed suspend.

Why don't you replace this all with a userspace tool that deciphers the registers for you?

This patch isn't deciphering registers, it's just dumping usecounts, and I think that is extremely useful to have in debugfs.

I've already removed all the register dumping from the kernel in the hopes that someone will write a userspace tool for that.

Kevin

maintainer
#12 lore raw

PATCH 6/6 arm: omap: pm-debug: enhanced usecount debug support

Quoted 12 lines · Kevin Hilman
Quoted 10 lines · Tony Lindgren
Quoted 8 lines · Tero Kristo

Voltdm, pwrdm, clkdm, hwmod and clk usecounts are now separeted to their own file, 'usecount'. This file shows the usecounts for every active domain and their children recursively. 'count' file now only shows power state counts for powerdomains.

This patch also provices a way to do printk dumps from kernel code, by calling the pm_dbg_dump_X functions. The plan is to call these functions once an error condition is detected, e.g. failed suspend.

Quoted 8 lines
Quoted 2 lines

Why don't you replace this all with a userspace tool that deciphers the registers for you?

This patch isn't deciphering registers, it's just dumping usecounts, and I think that is extremely useful to have in debugfs.

I've already removed all the register dumping from the kernel in the hopes that someone will write a userspace tool for that.

OK good to hear you're already considering it.

Tony

#13 lore raw

PATCH 6/6 arm: omap: pm-debug: enhanced usecount debug support

Quoted 16 lines · Tony Lindgren
Quoted 15 lines · Kevin Hilman
Quoted 12 lines · Tony Lindgren
Quoted 8 lines · Tero Kristo

Voltdm, pwrdm, clkdm, hwmod and clk usecounts are now separeted to their own file, 'usecount'. This file shows the usecounts for every active domain and their children recursively. 'count' file now only shows power state counts for powerdomains.

This patch also provices a way to do printk dumps from kernel code, by calling the pm_dbg_dump_X functions. The plan is to call these functions once an error condition is detected, e.g. failed suspend.

Why don't you replace this all with a userspace tool that deciphers the registers for you?

Quoted 7 lines
Quoted 5 lines

This patch isn't deciphering registers, it's just dumping usecounts, and I think that is extremely useful to have in debugfs.

I've already removed all the register dumping from the kernel in the hopes that someone will write a userspace tool for that.

OK good to hear you're already considering it.

Yes, register dumps are gone, and I am actually one of the persons who is missing it.

I think there should still be some capability to get register snapshots from certain points during kernel execution, this is useful for debugging purposes. I don't know if it would be possible to do a call_usermodehelper() or something from kernel space just before wfi to read all (or part of) the PRCM registers, store them somewhere, and then decipher this data later with another tool. Any comments to this?

-Tero

#14 lore raw

PATCH 6/6 arm: omap: pm-debug: enhanced usecount debug support

Quoted 28 lines · Tero Kristo
Quoted 16 lines · Tony Lindgren
Quoted 15 lines · Kevin Hilman
Quoted 12 lines · Tony Lindgren
Quoted 8 lines · Tero Kristo

Voltdm, pwrdm, clkdm, hwmod and clk usecounts are now separeted to their own file, 'usecount'. This file shows the usecounts for every active domain and their children recursively. 'count' file now only shows power state counts for powerdomains.

This patch also provices a way to do printk dumps from kernel code, by calling the pm_dbg_dump_X functions. The plan is to call these functions once an error condition is detected, e.g. failed suspend.

Why don't you replace this all with a userspace tool that deciphers the registers for you?

Quoted 7 lines
Quoted 5 lines

This patch isn't deciphering registers, it's just dumping usecounts, and I think that is extremely useful to have in debugfs.

I've already removed all the register dumping from the kernel in the hopes that someone will write a userspace tool for that.

OK good to hear you're already considering it.

Yes, register dumps are gone, and I am actually one of the persons who is missing it.

Quoted 6 lines

I think there should still be some capability to get register snapshots from certain points during kernel execution, this is useful for debugging purposes. I don't know if it would be possible to do a call_usermodehelper() or something from kernel space just before wfi to read all (or part of) the PRCM registers, store them somewhere, and then decipher this data later with another tool. Any comments to this?

You should look into the omapconf tool (TI internal only currently)

This tool already has the ability to use /dev/mem to read/decipher OMAP PM related registers.

IMO, the one thing we're still missing is the ability to take register snapshots (like immediately before and after WFI) and somehow feed those to omapconf for deciphering.

Kevin

#15 lore raw

PATCH 6/6 arm: omap: pm-debug: enhanced usecount debug support

Quoted 37 lines · Kevin Hilman
Quoted 28 lines · Tero Kristo
Quoted 16 lines · Tony Lindgren
Quoted 15 lines · Kevin Hilman
Quoted 12 lines · Tony Lindgren
Quoted 8 lines · Tero Kristo

Voltdm, pwrdm, clkdm, hwmod and clk usecounts are now separeted to their own file, 'usecount'. This file shows the usecounts for every active domain and their children recursively. 'count' file now only shows power state counts for powerdomains.

This patch also provices a way to do printk dumps from kernel code, by calling the pm_dbg_dump_X functions. The plan is to call these functions once an error condition is detected, e.g. failed suspend.

Why don't you replace this all with a userspace tool that deciphers the registers for you?

Quoted 7 lines
Quoted 5 lines

This patch isn't deciphering registers, it's just dumping usecounts, and I think that is extremely useful to have in debugfs.

I've already removed all the register dumping from the kernel in the hopes that someone will write a userspace tool for that.

OK good to hear you're already considering it.

Yes, register dumps are gone, and I am actually one of the persons who is missing it.

Quoted 6 lines

I think there should still be some capability to get register snapshots from certain points during kernel execution, this is useful for debugging purposes. I don't know if it would be possible to do a call_usermodehelper() or something from kernel space just before wfi to read all (or part of) the PRCM registers, store them somewhere, and then decipher this data later with another tool. Any comments to this?

Quoted 9 lines

You should look into the omapconf tool (TI internal only currently)

This tool already has the ability to use /dev/mem to read/decipher OMAP PM related registers.

IMO, the one thing we're still missing is the ability to take register snapshots (like immediately before and after WFI) and somehow feed those to omapconf for deciphering.

Something like this perhaps?
https://github.com/nmenon/linux-omap-ti-pm/commit/3cd1994fc0df9a8e3e0be74ec3f3add3ff3aef95

Regards,
Nishanth Menon

#16 lore raw

PATCH 6/6 arm: omap: pm-debug: enhanced usecount debug support

Quoted 43 lines · Nishanth
Quoted 42 lines · Kevin Hilman
Quoted 28 lines · Tero Kristo
Quoted 16 lines · Tony Lindgren
Quoted 15 lines · Kevin Hilman
Quoted 12 lines · Tony Lindgren
Quoted 8 lines · Tero Kristo

Voltdm, pwrdm, clkdm, hwmod and clk usecounts are now separeted to their own file, 'usecount'. This file shows the usecounts for every active domain and their children recursively. 'count' file now only shows power state counts for powerdomains.

This patch also provices a way to do printk dumps from kernel code, by calling the pm_dbg_dump_X functions. The plan is to call these functions once an error condition is detected, e.g. failed suspend.

Why don't you replace this all with a userspace tool that deciphers the registers for you?

Quoted 7 lines
Quoted 5 lines

This patch isn't deciphering registers, it's just dumping usecounts, and I think that is extremely useful to have in debugfs.

I've already removed all the register dumping from the kernel in the hopes that someone will write a userspace tool for that.

OK good to hear you're already considering it.

Yes, register dumps are gone, and I am actually one of the persons who is missing it.

Quoted 6 lines

I think there should still be some capability to get register snapshots from certain points during kernel execution, this is useful for debugging purposes. I don't know if it would be possible to do a call_usermodehelper() or something from kernel space just before wfi to read all (or part of) the PRCM registers, store them somewhere, and then decipher this data later with another tool. Any comments to this?

You should look into the omapconf tool (TI internal only currently)

This tool already has the ability to use /dev/mem to read/decipher OMAP PM related registers.

Quoted 6 lines
Quoted 4 lines

IMO, the one thing we're still missing is the ability to take register snapshots (like immediately before and after WFI) and somehow feed those to omapconf for deciphering.

Something like this perhaps?
https://github.com/nmenon/linux-omap-ti-pm/commit/3cd1994fc0df9a8e3e0be74ec3f3add3ff3aef95

Yes, although that is targetted pretty narrowly at suspend and only PRM registers.

Do you then use omapconf to display the results?

Kevin

#17 lore raw

PATCH 6/6 arm: omap: pm-debug: enhanced usecount debug support

Quoted 5 lines · Kevin Hilman
Quoted 5 lines
Quoted 5 lines

IMO, the one thing we're still missing is the ability to take register snapshots (like immediately before and after WFI) and somehow feed those to omapconf for deciphering.

Quoted 7 lines
Quoted 2 lines

Something like this perhaps?
https://github.com/nmenon/linux-omap-ti-pm/commit/3cd1994fc0df9a8e3e0be74ec3f3add3ff3aef95

Yes, although that is targetted pretty narrowly at suspend and only PRM registers.

Do you then use omapconf to display the results?

on the platform we worked on, not everyone had access to omapconf, it was just 'cat' to take the dump.
omapconf support makes sense if this is a consistent interface cross OMAP variants, even better if a similar
framework is available cross SoCs.

Regards,
Nishanth Menon

#18 lore raw

cover arm: omap: clk/clkdm/pwrdm/voltdm changes to usecounting

In reply to Tero Kristo
Quoted 6 lines · Tero Kristo

This set is split out from a larger set earlier circulated on linux-omap list. The main purpose of this set is to provide useful recursive usecounting support for clockdomains / powerdomains / voltagedomains which can be used for debugging purposes and also to control certain chip power features once a sleep condition is reached (voltdm usecount=0).

Quoted 8 lines

This set provides only the baseline support for the usecounting, and also adds a pm-debug related patch that can be used to check out stuff (patch 6), and should later prove rather useful for debugging. Rest of the functionality will be posted in separate sets.

This set has been tested on omap3 beagle board with suspend / resume / cpuidle. Also boot tested on omap4 panda (sleep is not functional yet on that platform.)

This is a great series, and results in pm_debug debugfs data that's actually useful for real debug. Thanks!

I had a minor comment on the idle path changes, but all the other stuff is

Paul, this should probably merge via you (when it's ready) since the important changes are clock and powerdomain core changes.

Kevin