[PATCH 0/4] cpufreq: OMAP: if available, scale the iva coprocessor

[PATCH 0/4] cpufreq: OMAP: if available, scale the iva coprocessor

[Joshua Emele]

The iva coprocessor, available on some omap platforms, shares a voltage domain
with the mpu. If cpufreq is active and the mpu processor is scaled down, the iva
coprocessor should also be scaled. The goal is to make sure we do not ramp down
the voltage on the domain and affect clocking on the iva coprocessor leading to
a dsp crash.

I only have access to an omap3evm-ish device, so I do not know what the iva
clock name is for omap24xx and omap44xx. This detail can be added later if the
general approach is approved.

I have tested a version of this patch against the linux-3.3 kernel, so this my
attempt at a forward port against the current mainline. I have based my patch
series against linux-omap-pm/pm-next.

Joshua Emele (4):
  cpufreq: OMAP: if an iva clock name is specified, load iva resources
  cpufreq: OMAP: for omap3 devices, specify the iva clock name
  cpufreq: OMAP: ensure the iva coprocessor is at the same opp as the
    mpu
  cpufreq: OMAP: scale the iva coprocessor if available

 drivers/cpufreq/omap-cpufreq.c |  113 +++++++++++++++++++++++++++++++++------
 1 files changed, 95 insertions(+), 18 deletions(-)

-- 
1.7.6.5

[PATCH 1/4] cpufreq: OMAP: if an iva clock name is specified, load iva resources

[Joshua Emele]

Signed-off-by: Joshua Emele <jemele@gmail.com>
---
 drivers/cpufreq/omap-cpufreq.c |   73 ++++++++++++++++++++++++++++++++-------
 1 files changed, 60 insertions(+), 13 deletions(-)

diff --git a/drivers/cpufreq/omap-cpufreq.c b/drivers/cpufreq/omap-cpufreq.c
index 17fa04d..1621208 100644
--- a/drivers/cpufreq/omap-cpufreq.c
+++ b/drivers/cpufreq/omap-cpufreq.c
@@ -50,11 +50,11 @@ static DEFINE_PER_CPU(struct lpj_info, lpj_ref);
 static struct lpj_info global_lpj_ref;
 #endif
 
-static struct cpufreq_frequency_table *freq_table;
+static struct cpufreq_frequency_table *freq_table, *iva_freq_table;
 static atomic_t freq_table_users = ATOMIC_INIT(0);
-static struct clk *mpu_clk;
-static char *mpu_clk_name;
-static struct device *mpu_dev;
+static struct clk *mpu_clk, *iva_clk;
+static char *mpu_clk_name, *iva_clk_name;
+static struct device *mpu_dev, *iva_dev;
 static struct regulator *mpu_reg;
 
 static int omap_verify_speed(struct cpufreq_policy *policy)
@@ -199,8 +199,49 @@ done:
 
 static inline void freq_table_free(void)
 {
-	if (atomic_dec_and_test(&freq_table_users))
+	if (atomic_dec_and_test(&freq_table_users)) {
 		opp_free_cpufreq_table(mpu_dev, &freq_table);
+		opp_free_cpufreq_table(iva_dev, &iva_freq_table);
+	}
+}
+
+static inline void clk_free(void)
+{
+	if (mpu_clk) {
+		clk_put(mpu_clk);
+		mpu_clk = NULL;
+	}
+	if (iva_clk) {
+		clk_put(iva_clk);
+		iva_clk = NULL;
+	}
+}
+
+static int __cpuinit omap_iva_init(struct cpufreq_policy *policy)
+{
+	int result;
+	if (!iva_clk_name) {
+		pr_info("%s: iva unavailable\n", __func__);
+		return 0;
+	}
+	iva_dev = omap_device_get_by_hwmod_name("iva");
+	if (!iva_dev) {
+		pr_err("%s: unable to get the iva device\n", __func__);
+		return -EINVAL;
+	}
+	iva_clk = clk_get(NULL, iva_clk_name);
+	if (IS_ERR(iva_clk)) {
+		dev_err(iva_dev, "%s: cpu%d: %s clock unavailable\n",
+			__func__, policy->cpu, iva_clk_name);
+		return PTR_ERR(iva_clk);
+	}
+	result = opp_init_cpufreq_table(iva_dev, &iva_freq_table);
+	if (result) {
+		dev_err(iva_dev, "%s: cpu%d: failed creating freq table[%d]\n",
+				__func__, policy->cpu, result);
+		return result;
+	}
+	return 0;
 }
 
 static int __cpuinit omap_cpu_init(struct cpufreq_policy *policy)
@@ -218,13 +259,19 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *policy)
 
 	policy->cur = policy->min = policy->max = omap_getspeed(policy->cpu);
 
-	if (atomic_inc_return(&freq_table_users) == 1)
+	if (atomic_inc_return(&freq_table_users) == 1) {
 		result = opp_init_cpufreq_table(mpu_dev, &freq_table);
-
-	if (result) {
-		dev_err(mpu_dev, "%s: cpu%d: failed creating freq table[%d]\n",
-				__func__, policy->cpu, result);
-		goto fail_ck;
+		if (result) {
+			dev_err(mpu_dev, "%s: cpu%d: failed creating freq table[%d]\n",
+					__func__, policy->cpu, result);
+			goto fail_ck;
+		}
+		result = omap_iva_init(policy);
+		if (result) {
+			pr_err("%s: cpu%d: failed to initialize iva[%d]\n",
+					__func__, policy->cpu, result);
+			goto fail_table;
+		}
 	}
 
 	result = cpufreq_frequency_table_cpuinfo(policy, freq_table);
@@ -257,14 +304,14 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *policy)
 fail_table:
 	freq_table_free();
 fail_ck:
-	clk_put(mpu_clk);
+	clk_free();
 	return result;
 }
 
 static int omap_cpu_exit(struct cpufreq_policy *policy)
 {
 	freq_table_free();
-	clk_put(mpu_clk);
+	clk_free();
 	return 0;
 }
 
-- 
1.7.6.5

Re: [PATCH 1/4] cpufreq: OMAP: if an iva clock name is specified, load iva resources

[Nishanth Menon]

On 17:47-20121106, Joshua Emele wrote:
> +static int __cpuinit omap_iva_init(struct cpufreq_policy *policy)
> +{
> +	int result;
> +	if (!iva_clk_name) {
> +		pr_info("%s: iva unavailable\n", __func__);
> +		return 0;
> +	}
> +	iva_dev = omap_device_get_by_hwmod_name("iva");

NAK. Reasons as follows:
a) cpufreq is purely meant for cpu, not IVA. we should instead be using
devfreq which is designed around the usage for co-processor
b) clubbing ARM's frequency decision is definitely NOT equal to IVA
frequency decision, not only is it wrong in terms of modularization, it
is wrong in terms of power benefits as well (not to mention weirdness
needed when you look at all OMAP SoC variants)
c) DVFS is not trivial around multiple co-processor transitions -> core
OPPs (as dependent OPP) needs to be addressed as well. ideally common
clock framework could take care of clock dependencies.

-- 
Regards,
Nishanth Menon

Re: [PATCH 1/4] cpufreq: OMAP: if an iva clock name is specified, load iva resources

[Nishanth Menon]

On 11:16-20121107, Joshua Emele wrote:
> On Wed, Nov 7, 2012 at 6:53 AM, Nishanth Menon <nm@ti.com> wrote:
> 
> > On 17:47-20121106, Joshua Emele wrote:
> > > +static int __cpuinit omap_iva_init(struct cpufreq_policy *policy)
> > > +{
> > > +     int result;
> > > +     if (!iva_clk_name) {
> > > +             pr_info("%s: iva unavailable\n", __func__);
> > > +             return 0;
> > > +     }
> > > +     iva_dev = omap_device_get_by_hwmod_name("iva");
> >
> > NAK. Reasons as follows:
> > a) cpufreq is purely meant for cpu, not IVA. we should instead be using
> > devfreq which is designed around the usage for co-processor
> > b) clubbing ARM's frequency decision is definitely NOT equal to IVA
> > frequency decision, not only is it wrong in terms of modularization, it
> > is wrong in terms of power benefits as well (not to mention weirdness
> > needed when you look at all OMAP SoC variants)
> > c) DVFS is not trivial around multiple co-processor transitions -> core
> > OPPs (as dependent OPP) needs to be addressed as well. ideally common
> > clock framework could take care of clock dependencies.
> It looks like you've done some work on an omap devfreq coprocessor driver (
> http://pastebin.pandaboard.org/index.php/view/85100576). Are there any
> plans to merge this driver?
It is an sample driver about how it could be done - this was not in any
form meant for merge. there are few angles to it:
a) ability for coprocessors to provide load and idle information back to
master processor
b) integrating it into regular existing framework.

Yes, the need is definitely identified, there are many different
solutions floating around in TI kernel forks, devfreq at the moment
seems to be the rationale choice in terms of a solution that is aligned
with community needs, so we are slowly building towards it.

Any contributions towards that is very appreciated ofcourse :). the
quick reference to latest code meant for 3.8 rc1 target could be found
here:
http://git.kernel.org/?p=linux/kernel/git/mzx/devfreq.git;a=summary

-- 
Regards,
Nishanth Menon

[PATCH 2/4] cpufreq: OMAP: for omap3 devices, specify the iva clock name

[Joshua Emele]

Signed-off-by: Joshua Emele <jemele@gmail.com>
---
 drivers/cpufreq/omap-cpufreq.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/omap-cpufreq.c b/drivers/cpufreq/omap-cpufreq.c
index 1621208..d8a751f 100644
--- a/drivers/cpufreq/omap-cpufreq.c
+++ b/drivers/cpufreq/omap-cpufreq.c
@@ -335,9 +335,10 @@ static int __init omap_cpufreq_init(void)
 {
 	if (cpu_is_omap24xx())
 		mpu_clk_name = "virt_prcm_set";
-	else if (cpu_is_omap34xx())
+	else if (cpu_is_omap34xx()) {
 		mpu_clk_name = "dpll1_ck";
-	else if (cpu_is_omap44xx())
+		iva_clk_name = "dpll2_ck";
+	} else if (cpu_is_omap44xx())
 		mpu_clk_name = "dpll_mpu_ck";
 
 	if (!mpu_clk_name) {
-- 
1.7.6.5

[PATCH 3/4] cpufreq: OMAP: ensure the iva coprocessor is at the same opp as the mpu

[Joshua Emele]

Signed-off-by: Joshua Emele <jemele@gmail.com>
---
 drivers/cpufreq/omap-cpufreq.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/drivers/cpufreq/omap-cpufreq.c b/drivers/cpufreq/omap-cpufreq.c
index d8a751f..e8bcad8 100644
--- a/drivers/cpufreq/omap-cpufreq.c
+++ b/drivers/cpufreq/omap-cpufreq.c
@@ -220,6 +220,9 @@ static inline void clk_free(void)
 static int __cpuinit omap_iva_init(struct cpufreq_policy *policy)
 {
 	int result;
+	unsigned long iva_rate;
+	unsigned int opp_index, mpu_freq = omap_getspeed(policy->cpu);
+
 	if (!iva_clk_name) {
 		pr_info("%s: iva unavailable\n", __func__);
 		return 0;
@@ -241,6 +244,21 @@ static int __cpuinit omap_iva_init(struct cpufreq_policy *policy)
 				__func__, policy->cpu, result);
 		return result;
 	}
+	result = cpufreq_frequency_table_target(policy, freq_table, mpu_freq,
+		CPUFREQ_RELATION_L, &opp_index);
+	if (result) {
+		dev_err(mpu_dev, "%s: cpu%d: no freq match for %u[%d]\n",
+				__func__, policy->cpu, mpu_freq, result);
+		return result;
+	}
+	iva_rate = iva_freq_table[opp_index].frequency * 1000;
+	result = clk_set_rate(iva_clk, iva_rate);
+	if (result) {
+		pr_err("%s: cpu%d: failed to set %s rate %lu[%d]\n",
+				__func__, policy->cpu, iva_clk->name, iva_rate,
+				result);
+		return result;
+	}
 	return 0;
 }
 
-- 
1.7.6.5

[PATCH 4/4] cpufreq: OMAP: scale the iva coprocessor if available

[Joshua Emele]

Signed-off-by: Joshua Emele <jemele@gmail.com>
---
 drivers/cpufreq/omap-cpufreq.c |   17 ++++++++++++++---
 1 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/cpufreq/omap-cpufreq.c b/drivers/cpufreq/omap-cpufreq.c
index e8bcad8..103fa8b 100644
--- a/drivers/cpufreq/omap-cpufreq.c
+++ b/drivers/cpufreq/omap-cpufreq.c
@@ -79,7 +79,7 @@ static int omap_target(struct cpufreq_policy *policy,
 		       unsigned int target_freq,
 		       unsigned int relation)
 {
-	unsigned int i;
+	unsigned int i, opp_index;
 	int r, ret = 0;
 	struct cpufreq_freqs freqs;
 	struct opp *opp;
@@ -92,13 +92,13 @@ static int omap_target(struct cpufreq_policy *policy,
 	}
 
 	ret = cpufreq_frequency_table_target(policy, freq_table, target_freq,
-			relation, &i);
+			relation, &opp_index);
 	if (ret) {
 		dev_dbg(mpu_dev, "%s: cpu%d: no freq match for %d(ret=%d)\n",
 			__func__, policy->cpu, target_freq, ret);
 		return ret;
 	}
-	freqs.new = freq_table[i].frequency;
+	freqs.new = freq_table[opp_index].frequency;
 	if (!freqs.new) {
 		dev_err(mpu_dev, "%s: cpu%d: no match for freq %d\n", __func__,
 			policy->cpu, target_freq);
@@ -161,6 +161,17 @@ static int omap_target(struct cpufreq_policy *policy,
 	}
 
 	freqs.new = omap_getspeed(policy->cpu);
+
+	if (!ret && iva_freq_table && iva_clk) {
+		const unsigned long iva_rate =
+			iva_freq_table[opp_index].frequency * 1000;
+		ret = clk_set_rate(iva_clk, iva_rate);
+		if (ret) {
+			pr_err("%s: failed to set %s rate %lu[%d]\n",
+					__func__, iva_clk->name, iva_rate, ret);
+		}
+	}
+
 #ifdef CONFIG_SMP
 	/*
 	 * Note that loops_per_jiffy is not updated on SMP systems in
-- 
1.7.6.5

Re: [PATCH 0/4] cpufreq: OMAP: if available, scale the iva coprocessor

[Santosh Shilimkar]

Hi,

On Tuesday 06 November 2012 07:47 PM, Joshua Emele wrote:
> The iva coprocessor, available on some omap platforms, shares a voltage domain
> with the mpu. If cpufreq is active and the mpu processor is scaled down, the iva
> coprocessor should also be scaled. The goal is to make sure we do not ramp down
> the voltage on the domain and affect clocking on the iva coprocessor leading to
> a dsp crash.
>
> I only have access to an omap3evm-ish device, so I do not know what the iva
> clock name is for omap24xx and omap44xx. This detail can be added later if the
> general approach is approved.
>
> I have tested a version of this patch against the linux-3.3 kernel, so this my
> attempt at a forward port against the current mainline. I have based my patch
> series against linux-omap-pm/pm-next.
>
How about making use of CPUFREQ notifiers to handle it. I don't like
this series since it adds multi-media co-processor scaling into the
CPUFreq driver.

Regards
Santosh