| Message ID | BANLkTi=LUMdrTzsOXtVwU_aTcecodaJiPg@mail.gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Kevin Hilman |
| Headers | show |
On 6/3/2011 8:14 AM, Menon, Nishanth wrote: > On Thu, Jun 2, 2011 at 09:51, Santosh Shilimkar > <santosh.shilimkar@ti.com> wrote: >> Current OMAP2PLUS CPUfreq tagret() functions returns when all >> the CPU's are not online. This will break DVFS when secondary >> CPUs are offlined. >> >> The intention of that check was just avoid CPU frequency change >> during the window when CPU becomes online but it's cpufreq_init is >> not done yet. > is it this requirement a boot requirement or a necessity for > cpufreq_driver.init being called for online cpus? Since we maintain > just a single freq_table... why do we care about multiple cpu_inits? > I put a comment after --- After some thinking, I realised there is no need of that since this is just a counter which maintains the count for online_cpus = cpufreq_init_cpus. And we need inits on all CPUs to ensure the CPU relation is set. It's not all about _one_ table. > Anyways, tried testing this and .config with CONFIG_SMP_ON_UP and > USERSPACE. it works with one cpu and does not scale 2 cpus :( > You mean userspace governor? I don't think why that can happen. Vishwa tested this and it did worked. We will test this again. > After applying this patch on kevin's cpufreq branch, I added some > prints for logging: [..] > > /* FIXME: what's the actual transition time? */ > @@ -212,6 +220,8 @@ static int omap_cpu_exit(struct cpufreq_policy *policy) > if (is_smp()) { > cpus_initialized--; > smp_wmb(); > + pr_err("%s: cpu %d cpus_initialized = %d online=%d\n", __func__, > + policy->cpu, cpus_initialized, num_online_cpus()); > } > return 0; > } > > on boot, this is what I see: > [ 0.421020] omap_cpu_init: cpu 0 cpus_initialized = 1 online=2 > [ 0.421264] omap_target: cpu 0 not ready to go to 1008000 (inits=1 > vs online=2) > [ 0.421630] omap_cpu_init: cpu 1 cpus_initialized = 2 online=2 > [ 0.421691] omap_cpu_exit: cpu 1 cpus_initialized = 1 online=2 This must be becasue hot-plug governor is kicking in here which is taking CPU1 in and OUT. > ... > snip > ... > [ 2.044128] omap_target: cpu 0 not ready to go to 1008000 (inits=1 > vs online=2) > [ 2.051849] omap_target: cpu 0 not ready to go to 1008000 (inits=1 > vs online=2) This is expected as well because CPU1 has not really offline yet. And the code is doing right thing. > ... snip.. > ...boots up to busybox shell.. > / # head /sys/devices/system/cpu/cpu1/online /sys/devices/system/cpu/cpu0/online > > ==> /sys/devices/system/cpu/cpu1/online<== > 1 > > ==> /sys/devices/system/cpu/cpu0/online<== > 1 > / # cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_available_frequencies > 300000 600000 800000 1008000 > / # cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_cur_freq > 1008000 > / # echo -n "300000">/sys/devices/system/cpu/cpu0/cpufreq/scaling_setspeed > [ 130.257385] omap_target: cpu 0 not ready to go to 300000 (inits=1 > vs online=2) > / # echo -n "0"> /sys/devices/system/cpu/cpu1/online > [ 144.749877] CPU1: shutdown > / # head /sys/devices/system/cpu/cpu1/online /sys/devices/system/cpu/cpu0/online > > ==> /sys/devices/system/cpu/cpu1/online<== > 0 > > ==> /sys/devices/system/cpu/cpu0/online<== > 1 > / # echo -n "350000"> /sys/devices/system/cpu/cpu0/cpufreq/scaling_setspeed > [ 165.881927] omap_target: cpu 0 ready to go to 350000 (inits=1 vs online=1) > [ 165.889526] cpufreq-omap: transition: 1008000 --> 0 > / # > / # echo -n "1"> /sys/devices/system/cpu/cpu1/online > [ 176.469360] CPU1: Booted secondary processor > [ 176.469421] CPU1: Unknown IPI message 0x1 > [ 176.475280] Switched to NOHz mode on CPU #1 > [ 176.600891] omap_cpu_init: cpu 1 cpus_initialized = 2 online=2 > [ 176.620178] omap_cpu_exit: cpu 1 cpus_initialized = 1 online=2 > [ 176.626373] omap_target: cpu 0 not ready to go to 350000 (inits=1 > vs online=2) > All the logs look normal to me. There is nothing wrong here. regards Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Nishant, On 6/3/2011 12:09 PM, Santosh Shilimkar wrote: > On 6/3/2011 8:14 AM, Menon, Nishanth wrote: >> On Thu, Jun 2, 2011 at 09:51, Santosh Shilimkar >> <santosh.shilimkar@ti.com> wrote: >>> Current OMAP2PLUS CPUfreq tagret() functions returns when all >>> the CPU's are not online. This will break DVFS when secondary >>> CPUs are offlined. >>> >>> The intention of that check was just avoid CPU frequency change >>> during the window when CPU becomes online but it's cpufreq_init is >>> not done yet. >> is it this requirement a boot requirement or a necessity for >> cpufreq_driver.init being called for online cpus? Since we maintain >> just a single freq_table... why do we care about multiple cpu_inits? >> > I put a comment after --- > After some thinking, I realised > there is no need of that since this is just a counter which > maintains the count for online_cpus = cpufreq_init_cpus. > And we need inits on all CPUs to ensure the CPU relation is > set. It's not all about _one_ table. > > >> Anyways, tried testing this and .config with CONFIG_SMP_ON_UP and >> USERSPACE. it works with one cpu and does not scale 2 cpus :( >> > You mean userspace governor? I don't think why that can happen. > Vishwa tested this and it did worked. We will test this again. > Your observation is right. This patch and earlier tested patch had one difference. We were not decrementing the counter in the exit path. I assumed that during boot you have hot-plug governor selected and later you were switching to user-space. It wasn't the case because I could reproduce same observation as you. Sorry for that. I did some debug on this overall issue. Will send an updated patch. Regards Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/mach-omap2/omap2plus-cpufreq.c b/arch/arm/mach-omap2/omap2plus-cpufreq.c index 909bfcb..89856d5 100644 --- a/arch/arm/mach-omap2/omap2plus-cpufreq.c +++ b/arch/arm/mach-omap2/omap2plus-cpufreq.c @@ -83,8 +83,13 @@ static int omap_target(struct cpufreq_policy *policy, struct cpufreq_freqs freqs; /* Changes not allowed until all CPUs are online */ - if (is_smp() && (cpus_initialized < num_online_cpus())) + if (is_smp() && (cpus_initialized < num_online_cpus())) { + pr_err("%s: cpu %d not ready to go to %d (inits=%d vs online=%d)\n", __func__, + policy->cpu, target_freq, cpus_initialized, num_online_cpus()); return ret; + } + pr_err("%s: cpu %d ready to go to %d (inits=%d vs online=%d)\n", __func__, + policy->cpu, target_freq, cpus_initialized, num_online_cpus());