Re: [PATCH 08/13] OMAP3: cpufreq driver changes for DVFS support
On Wed, 13 Apr 2011 10:57:54 -0700 Vishwanath Sripathy wrote: > > -Original Message- > > From: Jarkko Nikula [mailto:jhnik...@gmail.com] > > Sent: Wednesday, April 13, 2011 7:13 AM > > To: Vishwanath BS > > Cc: linux-omap@vger.kernel.org; patc...@linaro.org; Santosh > > Shilimkar > > Subject: Re: [PATCH 08/13] OMAP3: cpufreq driver changes for DVFS > > support > > > > Hi > > > > A few comments below in case this is still under development as I > > was > > playing a bit with this version on omap3. > There is an updated version of omap cpufreq patches where this issue is > fixed. > https://patchwork.kernel.org/patch/632781/ > Thanks, I didn't notice that patch fixes the issues I noticed. Now also the clk_round_rate works fine since the dpll1_ck does have the round_clock defined and the former arm_fck is anyway derived from it. -- Jarkko -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 08/13] OMAP3: cpufreq driver changes for DVFS support
> -Original Message-
> From: Jarkko Nikula [mailto:jhnik...@gmail.com]
> Sent: Wednesday, April 13, 2011 7:13 AM
> To: Vishwanath BS
> Cc: linux-omap@vger.kernel.org; patc...@linaro.org; Santosh
> Shilimkar
> Subject: Re: [PATCH 08/13] OMAP3: cpufreq driver changes for DVFS
> support
>
> Hi
>
> A few comments below in case this is still under development as I
> was
> playing a bit with this version on omap3.
There is an updated version of omap cpufreq patches where this issue is
fixed.
https://patchwork.kernel.org/patch/632781/
Regards
Vishwa
>
> On Fri, 21 Jan 2011 19:31:00 +0530
> Vishwanath BS wrote:
>
> > Changes in the omap cpufreq driver for DVFS support.
> >
> > Signed-off-by: Vishwanath BS
> > Cc: Santosh Shilimkar
> > ---
> > arch/arm/plat-omap/cpu-omap.c | 35
> ---
> > 1 files changed, 32 insertions(+), 3 deletions(-)
> >
> ...
> > @@ -85,11 +87,11 @@ static int omap_target(struct cpufreq_policy
> *policy,
> >unsigned int target_freq,
> >unsigned int relation)
> > {
> > -#ifdef CONFIG_ARCH_OMAP1
> > struct cpufreq_freqs freqs;
> > -#endif
> > #if defined(CONFIG_ARCH_OMAP3) && !defined(CONFIG_OMAP_PM_NONE)
> > unsigned long freq;
> > + int i;
> > + struct cpufreq_freqs freqs_notify;
> > struct device *mpu_dev = omap2_get_mpuss_device();
> > #endif
> > int ret = 0;
> > @@ -116,9 +118,36 @@ static int omap_target(struct cpufreq_policy
> *policy,
> > ret = clk_set_rate(mpu_clk, freqs.new * 1000);
> > cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
> > #elif defined(CONFIG_ARCH_OMAP3) && !defined(CONFIG_OMAP_PM_NONE)
> > + freqs.old = omap_getspeed(policy->cpu);;
> > + freqs_notify.new = clk_round_rate(mpu_clk, target_freq * 1000)
> / 1000;
> > + freqs.cpu = policy->cpu;
> > +
> > + if (freqs.old == freqs.new)
> > + return ret;
> > +
>
> Here freqs.new is uninitialized which very likely means that code
> falls
> always through, sets the correct target frequency though, but can
> populate the wrong frequency through the cpufreq_notify_transition
> when
> running the pre notifiers below.
>
> I think the code above meant
>
> freqs_notify.new = clk_round_rate(mpu_clk, target_freq * 1000) /
> 1000;
> ->
> freqs.new = clk_round_rate(mpu_clk, target_freq * 1000) / 1000;
>
> as the freqs_notify is otherwise unused?
>
> However, that doesn't work as the clk_round_rate returns the current
> rate for mpu_clk on omap3 and "freqs.old == freqs.new" is always
> true.
> Looks like there is no round_rate function for arm_fck.
>
> I replaced that with "freqs.new = target_freq;" but this means there
> will be unnecessary fall throughs if the real frequency (eg 124800
> kHz)
> will differ from opp table frequency (eg 125000 kHz) and no real
> changes are happening e.g. with on-demand governor.
>
> > + /* pre notifiers */
> > + for_each_cpu(i, policy->cpus) {
> > + freqs.cpu = i;
> > + cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
> > + }
>
> --
> Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/13] OMAP3: cpufreq driver changes for DVFS support
Hi
A few comments below in case this is still under development as I was
playing a bit with this version on omap3.
On Fri, 21 Jan 2011 19:31:00 +0530
Vishwanath BS wrote:
> Changes in the omap cpufreq driver for DVFS support.
>
> Signed-off-by: Vishwanath BS
> Cc: Santosh Shilimkar
> ---
> arch/arm/plat-omap/cpu-omap.c | 35 ---
> 1 files changed, 32 insertions(+), 3 deletions(-)
>
...
> @@ -85,11 +87,11 @@ static int omap_target(struct cpufreq_policy *policy,
> unsigned int target_freq,
> unsigned int relation)
> {
> -#ifdef CONFIG_ARCH_OMAP1
> struct cpufreq_freqs freqs;
> -#endif
> #if defined(CONFIG_ARCH_OMAP3) && !defined(CONFIG_OMAP_PM_NONE)
> unsigned long freq;
> + int i;
> + struct cpufreq_freqs freqs_notify;
> struct device *mpu_dev = omap2_get_mpuss_device();
> #endif
> int ret = 0;
> @@ -116,9 +118,36 @@ static int omap_target(struct cpufreq_policy *policy,
> ret = clk_set_rate(mpu_clk, freqs.new * 1000);
> cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
> #elif defined(CONFIG_ARCH_OMAP3) && !defined(CONFIG_OMAP_PM_NONE)
> + freqs.old = omap_getspeed(policy->cpu);;
> + freqs_notify.new = clk_round_rate(mpu_clk, target_freq * 1000) / 1000;
> + freqs.cpu = policy->cpu;
> +
> + if (freqs.old == freqs.new)
> + return ret;
> +
Here freqs.new is uninitialized which very likely means that code falls
always through, sets the correct target frequency though, but can
populate the wrong frequency through the cpufreq_notify_transition when
running the pre notifiers below.
I think the code above meant
freqs_notify.new = clk_round_rate(mpu_clk, target_freq * 1000) / 1000;
->
freqs.new = clk_round_rate(mpu_clk, target_freq * 1000) / 1000;
as the freqs_notify is otherwise unused?
However, that doesn't work as the clk_round_rate returns the current
rate for mpu_clk on omap3 and "freqs.old == freqs.new" is always true.
Looks like there is no round_rate function for arm_fck.
I replaced that with "freqs.new = target_freq;" but this means there
will be unnecessary fall throughs if the real frequency (eg 124800 kHz)
will differ from opp table frequency (eg 125000 kHz) and no real
changes are happening e.g. with on-demand governor.
> + /* pre notifiers */
> + for_each_cpu(i, policy->cpus) {
> + freqs.cpu = i;
> + cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
> + }
--
Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/13] OMAP3: cpufreq driver changes for DVFS support
Hi Vishwanath, On Mon, Feb 14, 2011 at 14:49, Vishwanath Sripathy wrote: >> >> I took those patches and applied to my .38-rc4 with help from Nishanth >> Menon. >> >> There is problem appeared during compilation: >> >> arch/arm/plat-omap/cpu-omap.c:142: error: 'struct cpuinfo_arm' has no >> member named 'loops_per_jiffy' >> >> My kernel is for Zoom3 board (OMAP3630-GP rev 2, CPU-OPP2 L3- >> 200MHz) >> Looking at the source, arch/arm/include/asm/cpu.h >> loops_per_jiffy is under CONFIG_SMP and since the omap2plus_defconfig >> builds for OMAP4 as well by default, I am guessing this bug was not >> immediately visible... >> >> Is there solution for the issue? > Yes this is an issue if CONFIG_SMP is disabled. We probably will need to > put this code under #ifdef CONFIG_SMP till lpj is handled for SMP in > generic cpufreq code itself . > I tried to go this way, but it doesn't help. It may demand additional changes. > Vishwa >> >> Regards, >> Gery > BTW, i found that this patch provides CPU in 1GHz for omap36xx is there solution for running omap36xx with 1.2GHz? Gery -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 08/13] OMAP3: cpufreq driver changes for DVFS support
> -Original Message- > From: Menon, Nishanth [mailto:n...@ti.com] > Sent: Monday, February 14, 2011 6:33 PM > To: Vishwanath Sripathy > Cc: Gery Kahn; linux-omap@vger.kernel.org; patc...@linaro.org; > Santosh Shilimkar > Subject: Re: [PATCH 08/13] OMAP3: cpufreq driver changes for DVFS > support > > On Mon, Feb 14, 2011 at 18:19, Vishwanath Sripathy > wrote: > > >> Is there solution for the issue? > > Yes this is an issue if CONFIG_SMP is disabled. We probably will need > to > > put this code under #ifdef CONFIG_SMP till lpj is handled for SMP in > > generic cpufreq code itself . > > > Apologies, I am not clear, but how'd we handle the non smp case here? For non SMP case, lpj is updated in common cpufreq code (in adjust_jiffies function). Vishwa > > Regards, > Nishanth Menon -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/13] OMAP3: cpufreq driver changes for DVFS support
On Mon, Feb 14, 2011 at 18:19, Vishwanath Sripathy wrote: >> Is there solution for the issue? > Yes this is an issue if CONFIG_SMP is disabled. We probably will need to > put this code under #ifdef CONFIG_SMP till lpj is handled for SMP in > generic cpufreq code itself . > Apologies, I am not clear, but how'd we handle the non smp case here? Regards, Nishanth Menon -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 08/13] OMAP3: cpufreq driver changes for DVFS support
> -Original Message-
> From: Kahn, Gery [mailto:ge...@ti.com]
> Sent: Monday, February 14, 2011 3:04 PM
> To: Vishwanath BS
> Cc: linux-omap@vger.kernel.org; patc...@linaro.org; Santosh Shilimkar
> Subject: Re: [PATCH 08/13] OMAP3: cpufreq driver changes for DVFS
> support
>
> Dear Vishwanath,
>
> On Fri, Jan 21, 2011 at 16:01, Vishwanath BS
> wrote:
> > Changes in the omap cpufreq driver for DVFS support.
> >
> > Signed-off-by: Vishwanath BS
> > Cc: Santosh Shilimkar
> > ---
> > arch/arm/plat-omap/cpu-omap.c | 35
> ---
> > 1 files changed, 32 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm/plat-omap/cpu-omap.c b/arch/arm/plat-
> omap/cpu-omap.c
> > index 1c1b80b..d965220 100644
> > --- a/arch/arm/plat-omap/cpu-omap.c
> > +++ b/arch/arm/plat-omap/cpu-omap.c
> > @@ -30,10 +30,12 @@
> > #include
> > #include
> > #include
> > +#include
> >
> > #if defined(CONFIG_ARCH_OMAP3) &&
> !defined(CONFIG_OMAP_PM_NONE)
> > #include
> > #include
> > +#include
> > #endif
> >
> > #define VERY_HI_RATE 9
> > @@ -85,11 +87,11 @@ static int omap_target(struct cpufreq_policy
> *policy,
> > unsigned int target_freq,
> > unsigned int relation)
> > {
> > -#ifdef CONFIG_ARCH_OMAP1
> > struct cpufreq_freqs freqs;
> > -#endif
> > #if defined(CONFIG_ARCH_OMAP3) &&
> !defined(CONFIG_OMAP_PM_NONE)
> > unsigned long freq;
> > + int i;
> > + struct cpufreq_freqs freqs_notify;
> > struct device *mpu_dev = omap2_get_mpuss_device();
> > #endif
> > int ret = 0;
> > @@ -116,9 +118,36 @@ static int omap_target(struct cpufreq_policy
> *policy,
> > ret = clk_set_rate(mpu_clk, freqs.new * 1000);
> > cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
> > #elif defined(CONFIG_ARCH_OMAP3) &&
> !defined(CONFIG_OMAP_PM_NONE)
> > + freqs.old = omap_getspeed(policy->cpu);;
> > + freqs_notify.new = clk_round_rate(mpu_clk, target_freq *
> 1000) / 1000;
> > + freqs.cpu = policy->cpu;
> > +
> > + if (freqs.old == freqs.new)
> > + return ret;
> > +
> > + /* pre notifiers */
> > + for_each_cpu(i, policy->cpus) {
> > + freqs.cpu = i;
> > + cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
> > + }
> > +
> > + /* scale the frequency */
> > freq = target_freq * 1000;
> > if (opp_find_freq_ceil(mpu_dev, &freq))
> > - omap_pm_cpu_set_freq(freq);
> > + omap_device_scale(mpu_dev, mpu_dev, freq);
> > +
> > + /* Update loops per jiffy */
> > + freqs.new = omap_getspeed(policy->cpu);
> > + for_each_cpu(i, policy->cpus)
> > + per_cpu(cpu_data, i).loops_per_jiffy =
> > + cpufreq_scale(per_cpu(cpu_data, i).loops_per_jiffy,
> > + freqs.old, freqs.new);
> > +
> > + /* post notifiers */
> > + for_each_cpu(i, policy->cpus) {
> > + freqs.cpu = i;
> > + cpufreq_notify_transition(&freqs,
> CPUFREQ_POSTCHANGE);
> > + }
> > #endif
> > return ret;
> > }
> > --
> > 1.7.0.4
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-omap"
in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
>
> I took those patches and applied to my .38-rc4 with help from Nishanth
> Menon.
>
> There is problem appeared during compilation:
>
> arch/arm/plat-omap/cpu-omap.c:142: error: 'struct cpuinfo_arm' has no
> member named 'loops_per_jiffy'
>
> My kernel is for Zoom3 board (OMAP3630-GP rev 2, CPU-OPP2 L3-
> 200MHz)
> Looking at the source, arch/arm/include/asm/cpu.h
> loops_per_jiffy is under CONFIG_SMP and since the omap2plus_defconfig
> builds for OMAP4 as well by default, I am guessing this bug was not
> immediately visible...
>
> Is there solution for the issue?
Yes this is an issue if CONFIG_SMP is disabled. We probably will need to
put this code under #ifdef CONFIG_SMP till lpj is handled for SMP in
generic cpufreq code itself .
Vishwa
>
> Regards,
> Gery
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/13] OMAP3: cpufreq driver changes for DVFS support
Dear Vishwanath,
On Fri, Jan 21, 2011 at 16:01, Vishwanath BS wrote:
> Changes in the omap cpufreq driver for DVFS support.
>
> Signed-off-by: Vishwanath BS
> Cc: Santosh Shilimkar
> ---
> arch/arm/plat-omap/cpu-omap.c | 35 ---
> 1 files changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/plat-omap/cpu-omap.c b/arch/arm/plat-omap/cpu-omap.c
> index 1c1b80b..d965220 100644
> --- a/arch/arm/plat-omap/cpu-omap.c
> +++ b/arch/arm/plat-omap/cpu-omap.c
> @@ -30,10 +30,12 @@
> #include
> #include
> #include
> +#include
>
> #if defined(CONFIG_ARCH_OMAP3) && !defined(CONFIG_OMAP_PM_NONE)
> #include
> #include
> +#include
> #endif
>
> #define VERY_HI_RATE 9
> @@ -85,11 +87,11 @@ static int omap_target(struct cpufreq_policy *policy,
> unsigned int target_freq,
> unsigned int relation)
> {
> -#ifdef CONFIG_ARCH_OMAP1
> struct cpufreq_freqs freqs;
> -#endif
> #if defined(CONFIG_ARCH_OMAP3) && !defined(CONFIG_OMAP_PM_NONE)
> unsigned long freq;
> + int i;
> + struct cpufreq_freqs freqs_notify;
> struct device *mpu_dev = omap2_get_mpuss_device();
> #endif
> int ret = 0;
> @@ -116,9 +118,36 @@ static int omap_target(struct cpufreq_policy *policy,
> ret = clk_set_rate(mpu_clk, freqs.new * 1000);
> cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
> #elif defined(CONFIG_ARCH_OMAP3) && !defined(CONFIG_OMAP_PM_NONE)
> + freqs.old = omap_getspeed(policy->cpu);;
> + freqs_notify.new = clk_round_rate(mpu_clk, target_freq * 1000) / 1000;
> + freqs.cpu = policy->cpu;
> +
> + if (freqs.old == freqs.new)
> + return ret;
> +
> + /* pre notifiers */
> + for_each_cpu(i, policy->cpus) {
> + freqs.cpu = i;
> + cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
> + }
> +
> + /* scale the frequency */
> freq = target_freq * 1000;
> if (opp_find_freq_ceil(mpu_dev, &freq))
> - omap_pm_cpu_set_freq(freq);
> + omap_device_scale(mpu_dev, mpu_dev, freq);
> +
> + /* Update loops per jiffy */
> + freqs.new = omap_getspeed(policy->cpu);
> + for_each_cpu(i, policy->cpus)
> + per_cpu(cpu_data, i).loops_per_jiffy =
> + cpufreq_scale(per_cpu(cpu_data, i).loops_per_jiffy,
> + freqs.old, freqs.new);
> +
> + /* post notifiers */
> + for_each_cpu(i, policy->cpus) {
> + freqs.cpu = i;
> + cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
> + }
> #endif
> return ret;
> }
> --
> 1.7.0.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
I took those patches and applied to my .38-rc4 with help from Nishanth Menon.
There is problem appeared during compilation:
arch/arm/plat-omap/cpu-omap.c:142: error: 'struct cpuinfo_arm' has no
member named 'loops_per_jiffy'
My kernel is for Zoom3 board (OMAP3630-GP rev 2, CPU-OPP2 L3-200MHz)
Looking at the source, arch/arm/include/asm/cpu.h
loops_per_jiffy is under CONFIG_SMP and since the omap2plus_defconfig
builds for OMAP4 as well by default, I am guessing this bug was not
immediately visible...
Is there solution for the issue?
Regards,
Gery
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/13] OMAP3: cpufreq driver changes for DVFS support
Vishwanath BS writes:
> Changes in the omap cpufreq driver for DVFS support.
Missing descriptive changelog.
Should describe what, why, etc.
Kevin
> Signed-off-by: Vishwanath BS
> Cc: Santosh Shilimkar
> ---
> arch/arm/plat-omap/cpu-omap.c | 35 ---
> 1 files changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/plat-omap/cpu-omap.c b/arch/arm/plat-omap/cpu-omap.c
> index 1c1b80b..d965220 100644
> --- a/arch/arm/plat-omap/cpu-omap.c
> +++ b/arch/arm/plat-omap/cpu-omap.c
> @@ -30,10 +30,12 @@
> #include
> #include
> #include
> +#include
>
> #if defined(CONFIG_ARCH_OMAP3) && !defined(CONFIG_OMAP_PM_NONE)
> #include
> #include
> +#include
> #endif
>
> #define VERY_HI_RATE 9
> @@ -85,11 +87,11 @@ static int omap_target(struct cpufreq_policy *policy,
> unsigned int target_freq,
> unsigned int relation)
> {
> -#ifdef CONFIG_ARCH_OMAP1
> struct cpufreq_freqs freqs;
> -#endif
> #if defined(CONFIG_ARCH_OMAP3) && !defined(CONFIG_OMAP_PM_NONE)
> unsigned long freq;
> + int i;
> + struct cpufreq_freqs freqs_notify;
> struct device *mpu_dev = omap2_get_mpuss_device();
> #endif
> int ret = 0;
> @@ -116,9 +118,36 @@ static int omap_target(struct cpufreq_policy *policy,
> ret = clk_set_rate(mpu_clk, freqs.new * 1000);
> cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
> #elif defined(CONFIG_ARCH_OMAP3) && !defined(CONFIG_OMAP_PM_NONE)
> + freqs.old = omap_getspeed(policy->cpu);;
> + freqs_notify.new = clk_round_rate(mpu_clk, target_freq * 1000) / 1000;
> + freqs.cpu = policy->cpu;
> +
> + if (freqs.old == freqs.new)
> + return ret;
> +
> + /* pre notifiers */
> + for_each_cpu(i, policy->cpus) {
> + freqs.cpu = i;
> + cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
> + }
> +
> + /* scale the frequency */
> freq = target_freq * 1000;
> if (opp_find_freq_ceil(mpu_dev, &freq))
> - omap_pm_cpu_set_freq(freq);
> + omap_device_scale(mpu_dev, mpu_dev, freq);
> +
> + /* Update loops per jiffy */
> + freqs.new = omap_getspeed(policy->cpu);
> + for_each_cpu(i, policy->cpus)
> + per_cpu(cpu_data, i).loops_per_jiffy =
> + cpufreq_scale(per_cpu(cpu_data, i).loops_per_jiffy,
> + freqs.old, freqs.new);
> +
> + /* post notifiers */
> + for_each_cpu(i, policy->cpus) {
> + freqs.cpu = i;
> + cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
> + }
> #endif
> return ret;
> }
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
