Re: Future of resource framework?
On Thu, Jun 3, 2010 at 8:01 PM, Gadiyar, Anand gadi...@ti.com wrote: Kevin Hilman wrote: Mike Chan m...@android.com writes: On Fri, May 21, 2010 at 9:47 AM, Kevin Hilman khil...@deeprootsystems.com wrote: Mike Chan m...@android.com writes: I'm not sure if this has been discussed, yet but since it seems that the resource framework will not be making it upstream, I am curious what are the replacements under consideration. I am starting to see similar issues on other platforms (msm / tegra) so more generic (non-omap) solution might be something to consider. Hi Mike, Which parts of the SRF do you currently use and find useful? It would be helpful for us to to understand the parts you see as useful and potentially helpful to generalize. Off the top of my head, for Droid specifically, OPP values are useful, although in theory if you changed OPP requests to cpu throughput that might give the equivalent functionality. Memory bus speeds / bandwidth, although its tied to CPU, which ultimately ends up in a cpu speed bump. Although most of the usage I've seen are just hacks, ie: the driver knows it needs 550mhz from the cpu so it will request some bogus value. As you know, the current implementation has a several layers and attempts to manage several things: OPPs, latencies etc. Our current plans are essentially to break up the one framework to rule them all philosophy and design of SRF and manage the various pieces by exending other layers such as the new OPP layer and voltage layers. Latencies are being managed by the omap_device layer and we will hopefully have some discussions with the broader linux-pm community about generalizing that more into the generic driver model over this year. Bus speed is a common resource I see for omap / msm / tegra. Clocks for devices also. ie: If I'm doing heavy mem operation and need max memory bus, I might need to request higher performance. (which might mean 600mhz on omap34030, on msm it might mean AXI clock running at 128mhz, and something else on tegra). Or if I'm doing graphics, I may need to up the gfx clock rate, or swich which pll its sourcing etc.. etc.. It doesn't look like pm qos has bus support, or even clock support, and this gets tricky if you want something semi-general. What we're hoping to work towards (and has come up in the suspend blocker related discussions) is moving towards a way to track per-device (or per-subsystem) constraints like latency and throughput in real world terms (usecs, bytes/sec, etc.) that would be general way. These constraints would then be visible to the bus- or platform-specific code that could make intelligent decisions with them (i.e whether or not to raise/lower OPP or bus speed, or whether or not to power down a powerdomain etc.) What if a driver knows that it cannot afford to let the PM layer turn off the power domain at certain points of time (maybe as long as a USB cable is connected). How can this be specified in terms of a latency or throughput constraint? Are there cases for this in omap? From what I've seen with omap3430 (atleast our hw configuration for Droid) we haven't run into this case. If you want to prevent a particular domain from entering RET or OFF, I believe you'd have to look at the latency values in the cpuidle34xx.c (or whatever it maybe called in .35) and match those latency in your driver against the PM_QOS_CPU_DMA_LATENCY. The omap cpuidle registers a table of various cpu states, which are a combination of ON, RET and OFF for various domains. (Note only handles a few domains so maybe you need to extend the table or rev a new constraint type or something to pm_qos). It seems kind of hacky and not very general, but I've only seen a need for something like this on msm7k / msm8k, and the need was an msm specific driver and have not yet seen a need for this on omap[3403] or tegra but things could change. -- Mike Just curious, since I don't understand current OMAP3 PM code as well as I would like to. - Anand -- 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: Future of resource framework?
On Fri, May 21, 2010 at 9:47 AM, Kevin Hilman khil...@deeprootsystems.com wrote: Mike Chan m...@android.com writes: I'm not sure if this has been discussed, yet but since it seems that the resource framework will not be making it upstream, I am curious what are the replacements under consideration. I am starting to see similar issues on other platforms (msm / tegra) so more generic (non-omap) solution might be something to consider. Hi Mike, Which parts of the SRF do you currently use and find useful? It would be helpful for us to to understand the parts you see as useful and potentially helpful to generalize. Off the top of my head, for Droid specifically, OPP values are useful, although in theory if you changed OPP requests to cpu throughput that might give the equivalent functionality. Memory bus speeds / bandwidth, although its tied to CPU, which ultimately ends up in a cpu speed bump. Although most of the usage I've seen are just hacks, ie: the driver knows it needs 550mhz from the cpu so it will request some bogus value. As you know, the current implementation has a several layers and attempts to manage several things: OPPs, latencies etc. Our current plans are essentially to break up the one framework to rule them all philosophy and design of SRF and manage the various pieces by exending other layers such as the new OPP layer and voltage layers. Latencies are being managed by the omap_device layer and we will hopefully have some discussions with the broader linux-pm community about generalizing that more into the generic driver model over this year. Bus speed is a common resource I see for omap / msm / tegra. Clocks for devices also. ie: If I'm doing heavy mem operation and need max memory bus, I might need to request higher performance. (which might mean 600mhz on omap34030, on msm it might mean AXI clock running at 128mhz, and something else on tegra). Or if I'm doing graphics, I may need to up the gfx clock rate, or swich which pll its sourcing etc.. etc.. It doesn't look like pm qos has bus support, or even clock support, and this gets tricky if you want something semi-general. -- Mike For the OPP management parts, you should expect RFC patches in the next week or two that will start discussions on this. Thanks for the input, Kevin -- 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 -- 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: Future of resource framework?
On Thu, Jun 3, 2010 at 4:52 PM, Kevin Hilman khil...@deeprootsystems.com wrote: Mike Chan m...@android.com writes: On Fri, May 21, 2010 at 9:47 AM, Kevin Hilman khil...@deeprootsystems.com wrote: Mike Chan m...@android.com writes: I'm not sure if this has been discussed, yet but since it seems that the resource framework will not be making it upstream, I am curious what are the replacements under consideration. I am starting to see similar issues on other platforms (msm / tegra) so more generic (non-omap) solution might be something to consider. Hi Mike, Which parts of the SRF do you currently use and find useful? It would be helpful for us to to understand the parts you see as useful and potentially helpful to generalize. Off the top of my head, for Droid specifically, OPP values are useful, although in theory if you changed OPP requests to cpu throughput that might give the equivalent functionality. Memory bus speeds / bandwidth, although its tied to CPU, which ultimately ends up in a cpu speed bump. Although most of the usage I've seen are just hacks, ie: the driver knows it needs 550mhz from the cpu so it will request some bogus value. As you know, the current implementation has a several layers and attempts to manage several things: OPPs, latencies etc. Our current plans are essentially to break up the one framework to rule them all philosophy and design of SRF and manage the various pieces by exending other layers such as the new OPP layer and voltage layers. Latencies are being managed by the omap_device layer and we will hopefully have some discussions with the broader linux-pm community about generalizing that more into the generic driver model over this year. Bus speed is a common resource I see for omap / msm / tegra. Clocks for devices also. ie: If I'm doing heavy mem operation and need max memory bus, I might need to request higher performance. (which might mean 600mhz on omap34030, on msm it might mean AXI clock running at 128mhz, and something else on tegra). Or if I'm doing graphics, I may need to up the gfx clock rate, or swich which pll its sourcing etc.. etc.. It doesn't look like pm qos has bus support, or even clock support, and this gets tricky if you want something semi-general. What we're hoping to work towards (and has come up in the suspend blocker related discussions) is moving towards a way to track per-device (or per-subsystem) constraints like latency and throughput in real world terms (usecs, bytes/sec, etc.) that would be general way. These constraints would then be visible to the bus- or platform-specific code that could make intelligent decisions with them (i.e whether or not to raise/lower OPP or bus speed, or whether or not to power down a powerdomain etc.) I saw a little bit on lkml / linux-pm but I may have missed some of the threads. I'm assuming these changes you're talking about are going to be an extension to pm qos. That's the pie-in-the-sky future I'm hoping for, and I hope to get some broader discussions around this going at some conferences this later year (LinuxCon in Aug, Plumbers in Nov, etc.) We had some early discussions in this direction at ELC already, but it needs some more thought and discussion. I think for OMAP we will be ok with the existing / incremental changes to the SRF. Your overview of the pie-in-the-sky might be promising but I'm not sure how timely things will happen. I suppose I should track linux-pm more closely, I'd rather not do a bunch of throw-away work on pm qos. -- Mike Kevin -- 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 v2 0/3] Enable CPU frequency and power tracking in cpuacct cgroup
On Fri, May 21, 2010 at 10:05 AM, Kevin Hilman khil...@deeprootsystems.com wrote: Mike Chan m...@android.com writes: On Thu, May 20, 2010 at 2:01 PM, Thomas Renninger tr...@suse.de wrote: Hi Mike, On Thursday 20 May 2010 08:42:21 pm Mike Chan wrote: v2: Rebased off of Thomas Renninger's patch for cgroups_cpuacct refactoring, thanks. A general comment: I don't know much about the cgroup stuff. Perhaps Paul Menage or Balbir Singh can look and sign off on the API's? I am also not sure how exactly power can be measured on this arch based on If you know how much time was spent at each frequency executing code, you can calculate how much power was consumed if the platform (with hooks) provide power numbers (in milliWatts) for the power at frequency X. I did some initial testing on Motorola Droid comparing to a power meter and I got within 2% variance. frequency accounting (there also were some threads I was not aware of?) A signed-off-by or reviewed-by from someone who is more involved in this omap stuff would probably not that bad. OMAP was the closest with mainline support I could provide an example how to use these hooks. I'm hoping for some blessing from some people on the linux-omap list for that. However can we possibly just stack the first two patches to get the API in? This will make it easier to fixup the omap hooks if they don't get in. This looks like a great enhancement to me. Speaking for OMAP PM... I'd suggest getting the generic stuff upstream (or into -next) soon and then work out the OMAP specifics after. Since the OMAP OPP layer is going through some churn (but stabilizing and will be submitted for 2.6.36), I'd suggest we queue the OMAP-specific parts of this along with the OPP layer changes. So it looks like there is no objections to this API and I'm OK with dropping the omap hooks for now until things are settled in 2.6.36. So are things good with Thomas' re-factoring patch for cpuacct as well as the first 2 patches? -- Mike Kevin -- 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: [linux-pm] [PATCH 0/8] Suspend block api (version 8)
On Thu, May 27, 2010 at 5:05 PM, Rafael J. Wysocki r...@sisk.pl wrote: On Friday 28 May 2010, Alan Cox wrote: The approach with user space power manager suggested by Dmitry and Alan Stern may work, but it still assumes some kind of suspend blockers to be present in the kernel. If we reject that too, I wonder what approach Google is supposed to use and still get the same battery life they get with suspend blockers. I'm getting less convinced it needs suspend blockers at all for this case, assuming that you are willing to have a policy that is based on - assuming apps play nicely - having the information to user space you need (who woke us, who blocked us, events) - dealing with offenders primarily from user space using that information I'm fairly happy about the following so far - we should have a common interface for seeing some pm events (like duh ?) but it does need careful thought so the watcher doesn't change the behaviour and break it. (Message We are suspending, gosh someone is running to receive the message, resume being the obvious case) - Suspend is (for many platforms) just a cotinuation down the power chain. Demonstrated and implemented on ARM. Very much the direction of S0i1/S0i3 on x86 MID devices. Proved by the fact it has been done and made to work, and by reading the Moorestown PR. - Given a non forced (that is 'idle down') transition to a suspend level we can implement a 'suspend as idle' on many embedded platforms in a manner which is not racy at kernel level. Apparently implemented already on ARM - Given a non forced transition to such a suspend level and the reporting of certain events we can do a full user space managed graphical UI type environment policy in a race free fashion - With notification of who caused a resume and maybe a bit of other general stat gathering it is possible to identify and handle abuses of power resource. Proved by the fact we can do this with powertop but more elegance in the interfaces would be nice. I am not sure if a pm event is what is needed for this or a sum 'hardware triggered wake up' event. I accept that current ACPI based laptops probably couldn't make use of such a feature but I don't think this is important at the moment. No, it's not. A resource constraint model might help further in the ACPI case. It's useful for other stuff but it might well be a distraction and implementation detail in terms of the basic question about what is needed for something like Android. At this point the input of the Android team and the Nokia people would be rather more useful to me. OK, I added Arve and Brian to the CC list. Even if we used the proposed QoS replacement, are there suggestions on how to keep the cpu idle for longer than 2 seconds in Linux without using suspend? On a thread somewhere I had real world power numbers on a Motorola Droid idling (screen off) with and without suspend blockers. -- Mike Thanks, Rafael ___ linux-pm mailing list linux...@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/linux-pm -- 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
[PATCH v2 1/3] scheduler: cpuacct: Enable platform hooks to track cpuusage for CPU frequencies
V2:
- Rebased off of Thomas Renninger's cgroup_cpuacct refactoring.
- Renamed function struct to cpuacct_chrage_calls from cpuacct_cpufreq_calls
Introduce new platform callback hooks for cpuacct for tracking CPU frequencies
Not all platforms / architectures have a set CPU_FREQ_TABLE defined
for CPU transition speeds. In order to track time spent in at various
CPU frequencies, we enable platform callbacks from cpuacct for this accounting.
Architectures that support overclock boosting, or don't have pre-defined
frequency tables can implement their own bucketing system that makes sense
given their cpufreq scaling abilities.
New file:
cpuacct.cpufreq reports the CPU time (in nanoseconds) spent at each CPU
frequency.
Signed-off-by: Mike Chan m...@android.com
---
Documentation/cgroups/cpuacct.txt |4 +++
include/linux/cpuacct.h | 41 +++
kernel/cgroup_cpuaccount.c| 49 +
3 files changed, 94 insertions(+), 0 deletions(-)
create mode 100644 include/linux/cpuacct.h
diff --git a/Documentation/cgroups/cpuacct.txt
b/Documentation/cgroups/cpuacct.txt
index 8b93094..600d2d0 100644
--- a/Documentation/cgroups/cpuacct.txt
+++ b/Documentation/cgroups/cpuacct.txt
@@ -40,6 +40,10 @@ system: Time spent by tasks of the cgroup in kernel mode.
user and system are in USER_HZ unit.
+cpuacct.cpufreq file gives CPU time (in nanoseconds) spent at each CPU
+frequency. Platform hooks must be implemented inorder to properly track
+time at each CPU frequency.
+
cpuacct controller uses percpu_counter interface to collect user and
system times. This has two side effects:
diff --git a/include/linux/cpuacct.h b/include/linux/cpuacct.h
new file mode 100644
index 000..6205d29
--- /dev/null
+++ b/include/linux/cpuacct.h
@@ -0,0 +1,41 @@
+/* include/linux/cpuacct.h
+ *
+ * Copyright (C) 2010 Google, Inc.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef _CPUACCT_H_
+#define _CPUACCT_H_
+
+#ifdef CONFIG_CGROUP_CPUACCT
+
+#include linux/cgroup.h
+
+/*
+ * Platform specific CPU frequency hooks for cpuacct. These functions are
+ * called from the scheduler.
+ */
+struct cpuacct_charge_calls {
+ /*
+* Platforms can take advantage of this data and use
+* per-cpu allocations if necessary.
+*/
+ void (*init) (void **cpuacct_data);
+ void (*charge) (void *cpuacct_data, u64 cputime, unsigned int cpu);
+ void (*show) (void *cpuacct_data, struct cgroup_map_cb *cb);
+};
+
+int cpuacct_charge_register(struct cpuacct_charge_calls *fn);
+
+#endif /* CONFIG_CGROUPS */
+
+#endif /* _CPUACCT_H_ */
diff --git a/kernel/cgroup_cpuaccount.c b/kernel/cgroup_cpuaccount.c
index 0ad356a..11799a7 100644
--- a/kernel/cgroup_cpuaccount.c
+++ b/kernel/cgroup_cpuaccount.c
@@ -7,6 +7,7 @@
#include linux/slab.h
#include linux/seq_file.h
#include linux/err.h
+#include linux/cpuacct.h
#include asm/cputime.h
@@ -26,8 +27,30 @@ struct cpuacct {
u64 __percpu *cpuusage;
struct percpu_counter cpustat[CPUACCT_STAT_NSTATS];
struct cpuacct *parent;
+ struct cpuacct_charge_calls *cpufreq_fn;
+ void *cpuacct_data;
};
+static struct cpuacct *cpuacct_root;
+
+/* Default calls for cpufreq accounting */
+static struct cpuacct_charge_calls *cpuacct_cpufreq;
+int cpuacct_charge_register(struct cpuacct_charge_calls *fn)
+{
+ cpuacct_cpufreq = fn;
+
+ /*
+* Root node is created before platform can register callbacks,
+* initalize here.
+*/
+ if (cpuacct_root fn) {
+ cpuacct_root-cpufreq_fn = fn;
+ if (fn-init)
+ fn-init(cpuacct_root-cpuacct_data);
+ }
+ return 0;
+}
+
struct cgroup_subsys cpuacct_subsys;
/* return cpu accounting group corresponding to this container */
@@ -62,8 +85,16 @@ static struct cgroup_subsys_state *cpuacct_create(
if (percpu_counter_init(ca-cpustat[i], 0))
goto out_free_counters;
+ ca-cpufreq_fn = cpuacct_cpufreq;
+
+ /* If available, have platform code initalize cpu frequency data */
+ if (ca-cpufreq_fn ca-cpufreq_fn-init)
+ ca-cpufreq_fn-init(ca-cpuacct_data);
+
if (cgrp-parent)
ca-parent = cgroup_ca(cgrp-parent);
+ else
+ cpuacct_root = ca;
return ca-css;
@@ -191,6 +222,16 @@ static int cpuacct_stats_show(struct cgroup *cgrp, struct
cftype *cft,
return 0;
}
+static int cpuacct_cpufreq_show
[PATCH v2 0/3] Enable CPU frequency and power tracking in cpuacct cgroup
v2: Rebased off of Thomas Renninger's patch for cgroups_cpuacct refactoring, which is based off of linus's tree. Thomas it might be easier to merge our patches if you take these patches and put them in a series ontop of your original patches (presuming there are no objections to your patch). This patch series introduces cpu frequency and power tracking for cpuacct cgroups. A similar patch set was discussed a while back and it was concluded that due to varying architectures (ppc, x86 with overboot) you cannot account for frequencies and their power consumption generically in sched.c, thus we have platform specific hooks the cpuacct can call into (if available). This patch series is not 3 instead of 4. I have left out the power implementation for OMAP due to implementation conflicts in linux-next. Mike Chan (3): scheduler: cpuacct: Enable platform hooks to track cpuusage for CPU frequencies scheduler: cpuacct: Enable platform callbacks for cpuacct power tracking omap: cpu: Implement callbacks for cpu frequency tracking in cpuacct Documentation/cgroups/cpuacct.txt |7 arch/arm/plat-omap/cpu-omap.c | 67 +++- include/linux/cpuacct.h | 43 +++ kernel/cgroup_cpuaccount.c| 69 + 4 files changed, 185 insertions(+), 1 deletions(-) create mode 100644 include/linux/cpuacct.h Signed-off-by: Mike Chan m...@android.com -- 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
[PATCH v2 2/3] scheduler: cpuacct: Enable platform callbacks for cpuacct power tracking
V2:
- Rebased off Thomass Renninger's cgroup_cpuacct refactoring
Platform must register cpu power function that return power in
milliWatt seconds.
New file:
cpuacct.power reports the power consumed in milliWatt seconds
Signed-off-by: Mike Chan m...@android.com
---
Documentation/cgroups/cpuacct.txt |3 +++
include/linux/cpuacct.h |4 +++-
kernel/cgroup_cpuaccount.c| 24 ++--
3 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/Documentation/cgroups/cpuacct.txt
b/Documentation/cgroups/cpuacct.txt
index 600d2d0..84e471b 100644
--- a/Documentation/cgroups/cpuacct.txt
+++ b/Documentation/cgroups/cpuacct.txt
@@ -44,6 +44,9 @@ cpuacct.cpufreq file gives CPU time (in nanoseconds) spent at
each CPU
frequency. Platform hooks must be implemented inorder to properly track
time at each CPU frequency.
+cpuacct.power file gives CPU power consumed (in milliWatt seconds). Platform
+must provide and implement power callback functions.
+
cpuacct controller uses percpu_counter interface to collect user and
system times. This has two side effects:
diff --git a/include/linux/cpuacct.h b/include/linux/cpuacct.h
index 6205d29..c17a634 100644
--- a/include/linux/cpuacct.h
+++ b/include/linux/cpuacct.h
@@ -31,7 +31,9 @@ struct cpuacct_charge_calls {
*/
void (*init) (void **cpuacct_data);
void (*charge) (void *cpuacct_data, u64 cputime, unsigned int cpu);
- void (*show) (void *cpuacct_data, struct cgroup_map_cb *cb);
+ void (*cpufreq_show) (void *cpuacct_data, struct cgroup_map_cb *cb);
+ /* Returns power consumed in milliWatt seconds */
+ u64 (*power_usage) (void *cpuacct_data);
};
int cpuacct_charge_register(struct cpuacct_charge_calls *fn);
diff --git a/kernel/cgroup_cpuaccount.c b/kernel/cgroup_cpuaccount.c
index 11799a7..d9bf889 100644
--- a/kernel/cgroup_cpuaccount.c
+++ b/kernel/cgroup_cpuaccount.c
@@ -226,12 +226,28 @@ static int cpuacct_cpufreq_show(struct cgroup *cgrp,
struct cftype *cft,
struct cgroup_map_cb *cb)
{
struct cpuacct *ca = cgroup_ca(cgrp);
- if (ca-cpufreq_fn ca-cpufreq_fn-show)
- ca-cpufreq_fn-show(ca-cpuacct_data, cb);
+ if (ca-cpufreq_fn ca-cpufreq_fn-cpufreq_show)
+ ca-cpufreq_fn-cpufreq_show(ca-cpuacct_data, cb);
return 0;
}
+/* return total cpu power usage (milliWatt second) of a group */
+static u64 cpuacct_powerusage_read(struct cgroup *cgrp, struct cftype *cft)
+{
+ int i;
+ struct cpuacct *ca = cgroup_ca(cgrp);
+ u64 totalpower = 0;
+
+ if (ca-cpufreq_fn ca-cpufreq_fn-power_usage)
+ for_each_present_cpu(i) {
+ totalpower += ca-cpufreq_fn-power_usage(
+ ca-cpuacct_data);
+ }
+
+ return totalpower;
+}
+
static struct cftype files[] = {
{
.name = usage,
@@ -250,6 +266,10 @@ static struct cftype files[] = {
.name = cpufreq,
.read_map = cpuacct_cpufreq_show,
},
+ {
+ .name = power,
+ .read_u64 = cpuacct_powerusage_read
+ },
};
static int cpuacct_populate(struct cgroup_subsys *ss, struct cgroup *cgrp)
--
1.7.0.1
--
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
[PATCH v2 3/3] omap: cpu: Implement callbacks for cpu frequency tracking in cpuacct
Implement OMAP platform specific scheduler callbacks for tracking
cpu frequencies per cpuacct cgroup.
Signed-off-by: Mike Chan m...@android.com
---
arch/arm/plat-omap/cpu-omap.c | 67 -
1 files changed, 66 insertions(+), 1 deletions(-)
diff --git a/arch/arm/plat-omap/cpu-omap.c b/arch/arm/plat-omap/cpu-omap.c
index 6d3d333..176417a 100644
--- a/arch/arm/plat-omap/cpu-omap.c
+++ b/arch/arm/plat-omap/cpu-omap.c
@@ -21,6 +21,8 @@
#include linux/err.h
#include linux/clk.h
#include linux/io.h
+#include linux/slab.h
+#include linux/cpuacct.h
#include mach/hardware.h
#include plat/clock.h
@@ -38,6 +40,10 @@ static struct cpufreq_frequency_table *freq_table;
static struct clk *mpu_clk;
+#ifdef CONFIG_CGROUP_CPUACCT
+static int freq_index;
+#endif
+
/* TODO: Add support for SDRAM timing changes */
int omap_verify_speed(struct cpufreq_policy *policy)
@@ -96,6 +102,11 @@ static int omap_target(struct cpufreq_policy *policy,
freqs.old, freqs.new);
#endif
ret = clk_set_rate(mpu_clk, freqs.new * 1000);
+#ifdef CONFIG_CGROUP_CPUACCT
+ /* Update freq_index before cpufreq transition post notification. */
+ cpufreq_frequency_table_target(policy, freq_table, freqs.new,
+ CPUFREQ_RELATION_L, freq_index);
+#endif
cpufreq_notify_transition(freqs, CPUFREQ_POSTCHANGE);
return ret;
@@ -125,7 +136,14 @@ static int __init omap_cpu_init(struct cpufreq_policy
*policy)
policy-cpuinfo.max_freq = clk_round_rate(mpu_clk,
VERY_HI_RATE) / 1000;
}
-
+#ifdef CONFIG_CGROUP_CPUACCT
+ /*
+* Update freq_index, since we are using the extact frequency
+* we can use any relation.
+*/
+ cpufreq_frequency_table_target(policy, freq_table, policy-cur,
+ CPUFREQ_RELATION_L, freq_index);
+#endif
/* FIXME: what's the actual transition time? */
policy-cpuinfo.transition_latency = 300 * 1000;
@@ -169,3 +187,50 @@ arch_initcall(omap_cpufreq_init);
* cpufreq_frequency_table_put_attr()
*/
+#ifdef CONFIG_CGROUP_CPUACCT
+/*
+ * Omap platform calls for cpuacct frequency accounting.
+ */
+
+static void omap_cpuacct_freq_init(void **cpuacct_data)
+{
+ /* MAX_VDD1_OPP is gone, define a table size to fit available values */
+ *cpuacct_data = kzalloc(sizeof(u64) * 8, GFP_KERNEL);
+}
+
+/* Called with rcu_read_lock() held. */
+static void omap_cpuacct_freq_charge(void *cpuacct_data, u64 cputime, unsigned
int cpu)
+{
+ u64 *cpuacct_freq = cpuacct_data;
+ if (freq_index 0)
+ return;
+
+ cpuacct_freq[freq_index] += cputime;
+}
+
+static void omap_cpuacct_freq_show(void *cpuacct_data, struct cgroup_map_cb
*cb)
+{
+ int i;
+ char buf[32];
+ u64 *cpuacct_freq = cpuacct_data;
+ for (i = 0; freq_table[i].frequency != CPUFREQ_TABLE_END; i++) {
+ snprintf(buf, sizeof(buf), %u, freq_table[i].frequency);
+ cb-fill(cb, buf, cpuacct_freq[i]);
+ }
+}
+
+static struct cpuacct_charge_calls omap_cpuacct_cpufreq = {
+ .init = omap_cpuacct_freq_init,
+ .charge = omap_cpuacct_freq_charge,
+ .cpufreq_show = omap_cpuacct_freq_show,
+};
+
+static int __init omap_cpuacct_init(void)
+{
+ freq_index = -1;
+ cpuacct_charge_register(omap_cpuacct_cpufreq);
+ return 0;
+}
+
+early_initcall(omap_cpuacct_init);
+#endif
--
1.7.0.1
--
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 v2 0/3] Enable CPU frequency and power tracking in cpuacct cgroup
On Thu, May 20, 2010 at 2:01 PM, Thomas Renninger tr...@suse.de wrote: Hi Mike, On Thursday 20 May 2010 08:42:21 pm Mike Chan wrote: v2: Rebased off of Thomas Renninger's patch for cgroups_cpuacct refactoring, thanks. A general comment: I don't know much about the cgroup stuff. Perhaps Paul Menage or Balbir Singh can look and sign off on the API's? I am also not sure how exactly power can be measured on this arch based on If you know how much time was spent at each frequency executing code, you can calculate how much power was consumed if the platform (with hooks) provide power numbers (in milliWatts) for the power at frequency X. I did some initial testing on Motorola Droid comparing to a power meter and I got within 2% variance. frequency accounting (there also were some threads I was not aware of?) A signed-off-by or reviewed-by from someone who is more involved in this omap stuff would probably not that bad. OMAP was the closest with mainline support I could provide an example how to use these hooks. I'm hoping for some blessing from some people on the linux-omap list for that. However can we possibly just stack the first two patches to get the API in? This will make it easier to fixup the omap hooks if they don't get in. -- Mike Still I Iike this interface and I could imagine others hook into it as well, for whatever has to be cpu cgroup accounted. My two cents..., Thomas -- To unsubscribe from this list: send the line unsubscribe cpufreq in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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
Future of resource framework?
I'm not sure if this has been discussed, yet but since it seems that the resource framework will not be making it upstream, I am curious what are the replacements under consideration. I am starting to see similar issues on other platforms (msm / tegra) so more generic (non-omap) solution might be something to consider. -- Mike -- 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 4/4] omap: cpu: Power tracking support for cgroup cpuacct
On Wed, May 19, 2010 at 8:34 AM, Thomas Renninger tr...@suse.de wrote: On Wednesday 19 May 2010 15:11:03 Nishanth Menon wrote: Mike Chan had written, on 05/18/2010 08:30 PM, the following: Specify new power field in struct omap_opp, which is power exported in milliWatt. ... + totalpower += cpuacct_freq[i] * mpu_opps[i + 1].power; ... + unsigned long power; /* power consumed running at OPP in milliWatts */ this conflicts with the OPP layer implementation. this structure will disappear for good. Okay, I wasn't sure what to use. The omap-pm branch uses omap_opp for the mpu frequencies. However it looks like struct prcm_config is used in linux-next for mpu frequencies, but there are deprecated comments around this struct definition so I'm not sure where what struct I should be using. Ah yes, another question: The power variable in struct omap_opp never gets initialized with a sane value, something is wrong... You're right, this last patch won't work on linux-next, it seems quite a bit has changed in the omap specific. Previously board files can register their OPP table which is an array of struct omp_opp, which is exactly what I do for Droid in our android-omap branch. I'm hoping some omap folks can offer some suggestions for what needs to be changed. -- Mike Thomas -- 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
[PATCH 0/4] Enable cpu frequency and power tracking for cpuacct cgroup
Rebased onto linux-next. This patch series introduces cpu frequency and power tracking for cpuacct cgroups. A similar patch set was discussed a while back and it was concluded that due to varying architectures (ppc, x86 with overboot) you cannot account for frequencies and their power consumption generically in sched.c, thus we have platform specific hooks the cpuacct can call into (if available). I've implemented all the necessary hooks for OMAP architecture as an example. For the OMAP folks, I?m not sure what the state of the OPP API in mainline, as most of my original work was based off of the omap-pm branch. Mike Chan (4): scheduler: cpuacct: Enable platform hooks to track cpuusage for CPU frequencies omap: cpu: Implement callbacks for cpu frequency tracking in cpuacct scheduler: cpuacct: Enable platform callbacks for cpuacct power tracking omap: cpu: Power tracking support for cgroup cpuacct Documentation/cgroups/cpuacct.txt |7 ++ arch/arm/plat-omap/cpu-omap.c | 87 - arch/arm/plat-omap/include/plat/omap-pm.h |1 + include/linux/cpuacct.h | 43 ++ kernel/sched.c| 69 +++ 5 files changed, 206 insertions(+), 1 deletions(-) create mode 100644 include/linux/cpuacct.h Signed-off-by: Mike Chan m...@android.com -- 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
[PATCH 1/4] scheduler: cpuacct: Enable platform hooks to track cpuusage for CPU frequencies
Introduce new platform callback hooks for cpuacct for tracking CPU frequencies
Not all platforms / architectures have a set CPU_FREQ_TABLE defined
for CPU transition speeds. In order to track time spent in at various
CPU frequencies, we enable platform callbacks from cpuacct for this accounting.
Architectures that support overclock boosting, or don't have pre-defined
frequency tables can implement their own bucketing system that makes sense
given their cpufreq scaling abilities.
New file:
cpuacct.cpufreq reports the CPU time (in nanoseconds) spent at each CPU
frequency.
Signed-off-by: Mike Chan m...@android.com
---
Documentation/cgroups/cpuacct.txt |4 +++
include/linux/cpuacct.h | 41 +++
kernel/sched.c| 49 +
3 files changed, 94 insertions(+), 0 deletions(-)
create mode 100644 include/linux/cpuacct.h
diff --git a/Documentation/cgroups/cpuacct.txt
b/Documentation/cgroups/cpuacct.txt
index 8b93094..600d2d0 100644
--- a/Documentation/cgroups/cpuacct.txt
+++ b/Documentation/cgroups/cpuacct.txt
@@ -40,6 +40,10 @@ system: Time spent by tasks of the cgroup in kernel mode.
user and system are in USER_HZ unit.
+cpuacct.cpufreq file gives CPU time (in nanoseconds) spent at each CPU
+frequency. Platform hooks must be implemented inorder to properly track
+time at each CPU frequency.
+
cpuacct controller uses percpu_counter interface to collect user and
system times. This has two side effects:
diff --git a/include/linux/cpuacct.h b/include/linux/cpuacct.h
new file mode 100644
index 000..9ff479e
--- /dev/null
+++ b/include/linux/cpuacct.h
@@ -0,0 +1,41 @@
+/* include/linux/cpuacct.h
+ *
+ * Copyright (C) 2010 Google, Inc.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef _CPUACCT_H_
+#define _CPUACCT_H_
+
+#include linux/cgroup.h
+
+#ifdef CONFIG_CGROUPS
+
+/*
+ * Platform specific CPU frequency hooks for cpuacct. These functions are
+ * called from the scheduler.
+ */
+struct cpuacct_cpufreq_calls {
+ /*
+* Platforms can take advantage of this data and use
+* per-cpu allocations if necessary.
+*/
+ void (*init) (void **cpuacct_data);
+ void (*charge) (void *cpuacct_data, u64 cputime, unsigned int cpu);
+ void (*show) (void *cpuacct_data, struct cgroup_map_cb *cb);
+};
+
+int cpuacct_register_cpufreq(struct cpuacct_cpufreq_calls *fn);
+
+#endif /* CONFIG_CGROUPS */
+
+#endif /* _CPUACCT_H_ */
diff --git a/kernel/sched.c b/kernel/sched.c
index 34a9722..6b6c45a 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -72,6 +72,7 @@
#include linux/ctype.h
#include linux/ftrace.h
#include linux/slab.h
+#include linux/cpuacct.h
#include asm/tlb.h
#include asm/irq_regs.h
@@ -8638,8 +8639,30 @@ struct cpuacct {
u64 __percpu *cpuusage;
struct percpu_counter cpustat[CPUACCT_STAT_NSTATS];
struct cpuacct *parent;
+ struct cpuacct_cpufreq_calls *cpufreq_fn;
+ void *cpuacct_data;
};
+static struct cpuacct *cpuacct_root;
+
+/* Default calls for cpufreq accounting */
+static struct cpuacct_cpufreq_calls *cpuacct_cpufreq;
+int cpuacct_register_cpufreq(struct cpuacct_cpufreq_calls *fn)
+{
+ cpuacct_cpufreq = fn;
+
+ /*
+* Root node is created before platform can register callbacks,
+* initalize here.
+*/
+ if (cpuacct_root fn) {
+ cpuacct_root-cpufreq_fn = fn;
+ if (fn-init)
+ fn-init(cpuacct_root-cpuacct_data);
+ }
+ return 0;
+}
+
struct cgroup_subsys cpuacct_subsys;
/* return cpu accounting group corresponding to this container */
@@ -8674,8 +8697,16 @@ static struct cgroup_subsys_state *cpuacct_create(
if (percpu_counter_init(ca-cpustat[i], 0))
goto out_free_counters;
+ ca-cpufreq_fn = cpuacct_cpufreq;
+
+ /* If available, have platform code initalize cpu frequency table */
+ if (ca-cpufreq_fn ca-cpufreq_fn-init)
+ ca-cpufreq_fn-init(ca-cpuacct_data);
+
if (cgrp-parent)
ca-parent = cgroup_ca(cgrp-parent);
+ else
+ cpuacct_root = ca;
return ca-css;
@@ -8803,6 +8834,16 @@ static int cpuacct_stats_show(struct cgroup *cgrp,
struct cftype *cft,
return 0;
}
+static int cpuacct_cpufreq_show(struct cgroup *cgrp, struct cftype *cft,
+ struct cgroup_map_cb *cb)
+{
+ struct cpuacct *ca = cgroup_ca(cgrp);
+ if (ca-cpufreq_fn ca
[PATCH 2/4] omap: cpu: Implement callbacks for cpu frequency tracking in cpuacct
Implement OMAP platform specific scheduler callbacks for tracking
cpu frequencies per cpuacct cgroup.
Signed-off-by: Mike Chan m...@android.com
---
arch/arm/plat-omap/cpu-omap.c | 66 -
1 files changed, 65 insertions(+), 1 deletions(-)
diff --git a/arch/arm/plat-omap/cpu-omap.c b/arch/arm/plat-omap/cpu-omap.c
index 6d3d333..d27234d 100644
--- a/arch/arm/plat-omap/cpu-omap.c
+++ b/arch/arm/plat-omap/cpu-omap.c
@@ -21,6 +21,7 @@
#include linux/err.h
#include linux/clk.h
#include linux/io.h
+#include linux/cpuacct.h
#include mach/hardware.h
#include plat/clock.h
@@ -38,6 +39,10 @@ static struct cpufreq_frequency_table *freq_table;
static struct clk *mpu_clk;
+#ifdef CONFIG_CGROUP_CPUACCT
+static int freq_index;
+#endif
+
/* TODO: Add support for SDRAM timing changes */
int omap_verify_speed(struct cpufreq_policy *policy)
@@ -96,6 +101,11 @@ static int omap_target(struct cpufreq_policy *policy,
freqs.old, freqs.new);
#endif
ret = clk_set_rate(mpu_clk, freqs.new * 1000);
+#ifdef CONFIG_CGROUP_CPUACCT
+ /* Update freq_index before cpufreq transition post notification. */
+ cpufreq_frequency_table_target(policy, freq_table, freqs.new,
+ CPUFREQ_RELATION_L, freq_index);
+#endif
cpufreq_notify_transition(freqs, CPUFREQ_POSTCHANGE);
return ret;
@@ -125,7 +135,14 @@ static int __init omap_cpu_init(struct cpufreq_policy
*policy)
policy-cpuinfo.max_freq = clk_round_rate(mpu_clk,
VERY_HI_RATE) / 1000;
}
-
+#ifdef CONFIG_CGROUP_CPUACCT
+ /*
+* Update freq_index, since we are using the extact frequency
+* we can use any relation.
+*/
+ cpufreq_frequency_table_target(policy, freq_table, policy-cur,
+ CPUFREQ_RELATION_L, freq_index);
+#endif
/* FIXME: what's the actual transition time? */
policy-cpuinfo.transition_latency = 300 * 1000;
@@ -169,3 +186,50 @@ arch_initcall(omap_cpufreq_init);
* cpufreq_frequency_table_put_attr()
*/
+#ifdef CONFIG_CGROUP_CPUACCT
+/*
+ * Omap platform calls for cpuacct frequency accounting.
+ */
+
+static void omap_cpuacct_freq_init(void **cpuacct_data)
+{
+ *cpuacct_data = kzalloc(sizeof(u64) * MAX_VDD1_OPP,
+ GFP_KERNEL);
+}
+
+/* Called with rcu_read_lock() held. */
+static void omap_cpuacct_freq_charge(void *cpuacct_data, u64 cputime, unsigned
int cpu)
+{
+ u64 *cpuacct_freq = cpuacct_data;
+ if (freq_index 0)
+ return;
+
+ cpuacct_freq[freq_index] += cputime;
+}
+
+static void omap_cpuacct_freq_show(void *cpuacct_data, struct cgroup_map_cb
*cb)
+{
+ int i;
+ char buf[32];
+ u64 *cpuacct_freq = cpuacct_data;
+ for (i = 0; freq_table[i].frequency != CPUFREQ_TABLE_END; i++) {
+ snprintf(buf, sizeof(buf), %u, freq_table[i].frequency);
+ cb-fill(cb, buf, cpuacct_freq[i]);
+ }
+}
+
+static struct cpuacct_cpufreq_calls omap_cpuacct_cpufreq = {
+ .init = omap_cpuacct_freq_init,
+ .charge = omap_cpuacct_freq_charge,
+ .show = omap_cpuacct_freq_show,
+};
+
+static int __init omap_cpuacct_init(void)
+{
+ freq_index = -1;
+ cpuacct_register_cpufreq(omap_cpuacct_cpufreq);
+ return 0;
+}
+
+early_initcall(omap_cpuacct_init);
+#endif
--
1.7.0.1
--
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
[PATCH 3/4] scheduler: cpuacct: Enable platform callbacks for cpuacct power tracking
Platform must register cpu power function that return power in
milliWatt seconds.
Signed-off-by: Mike Chan m...@android.com
---
Documentation/cgroups/cpuacct.txt |3 +++
include/linux/cpuacct.h |4 +++-
kernel/sched.c| 24 ++--
3 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/Documentation/cgroups/cpuacct.txt
b/Documentation/cgroups/cpuacct.txt
index 600d2d0..84e471b 100644
--- a/Documentation/cgroups/cpuacct.txt
+++ b/Documentation/cgroups/cpuacct.txt
@@ -44,6 +44,9 @@ cpuacct.cpufreq file gives CPU time (in nanoseconds) spent at
each CPU
frequency. Platform hooks must be implemented inorder to properly track
time at each CPU frequency.
+cpuacct.power file gives CPU power consumed (in milliWatt seconds). Platform
+must provide and implement power callback functions.
+
cpuacct controller uses percpu_counter interface to collect user and
system times. This has two side effects:
diff --git a/include/linux/cpuacct.h b/include/linux/cpuacct.h
index 9ff479e..effe842 100644
--- a/include/linux/cpuacct.h
+++ b/include/linux/cpuacct.h
@@ -31,7 +31,9 @@ struct cpuacct_cpufreq_calls {
*/
void (*init) (void **cpuacct_data);
void (*charge) (void *cpuacct_data, u64 cputime, unsigned int cpu);
- void (*show) (void *cpuacct_data, struct cgroup_map_cb *cb);
+ void (*cpufreq_show) (void *cpuacct_data, struct cgroup_map_cb *cb);
+ /* Returns power consumed in milliWatt seconds */
+ u64 (*power_usage) (void *cpuacct_data);
};
int cpuacct_register_cpufreq(struct cpuacct_cpufreq_calls *fn);
diff --git a/kernel/sched.c b/kernel/sched.c
index 6b6c45a..d55d8af 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -8838,12 +8838,28 @@ static int cpuacct_cpufreq_show(struct cgroup *cgrp,
struct cftype *cft,
struct cgroup_map_cb *cb)
{
struct cpuacct *ca = cgroup_ca(cgrp);
- if (ca-cpufreq_fn ca-cpufreq_fn-show)
- ca-cpufreq_fn-show(ca-cpuacct_data, cb);
+ if (ca-cpufreq_fn ca-cpufreq_fn-cpufreq_show)
+ ca-cpufreq_fn-cpufreq_show(ca-cpuacct_data, cb);
return 0;
}
+/* return total cpu power usage (milliWatt second) of a group */
+static u64 cpuacct_powerusage_read(struct cgroup *cgrp, struct cftype *cft)
+{
+ int i;
+ struct cpuacct *ca = cgroup_ca(cgrp);
+ u64 totalpower = 0;
+
+ if (ca-cpufreq_fn ca-cpufreq_fn-power_usage)
+ for_each_present_cpu(i) {
+ totalpower += ca-cpufreq_fn-power_usage(
+ ca-cpuacct_data);
+ }
+
+ return totalpower;
+}
+
static struct cftype files[] = {
{
.name = usage,
@@ -8862,6 +8878,10 @@ static struct cftype files[] = {
.name = cpufreq,
.read_map = cpuacct_cpufreq_show,
},
+ {
+ .name = power,
+ .read_u64 = cpuacct_powerusage_read
+ },
};
static int cpuacct_populate(struct cgroup_subsys *ss, struct cgroup *cgrp)
--
1.7.0.1
--
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
[PATCH 4/4] omap: cpu: Power tracking support for cgroup cpuacct
Specify new power field in struct omap_opp, which is
power exported in milliWatt.
power_usage function gives power consumed in milliWatt seconds
Signed-off-by: Mike Chan m...@android.com
---
arch/arm/plat-omap/cpu-omap.c | 23 ++-
arch/arm/plat-omap/include/plat/omap-pm.h |1 +
2 files changed, 23 insertions(+), 1 deletions(-)
diff --git a/arch/arm/plat-omap/cpu-omap.c b/arch/arm/plat-omap/cpu-omap.c
index d27234d..1381539 100644
--- a/arch/arm/plat-omap/cpu-omap.c
+++ b/arch/arm/plat-omap/cpu-omap.c
@@ -22,6 +22,7 @@
#include linux/clk.h
#include linux/io.h
#include linux/cpuacct.h
+#include linux/math64.h
#include mach/hardware.h
#include plat/clock.h
@@ -218,10 +219,30 @@ static void omap_cpuacct_freq_show(void *cpuacct_data,
struct cgroup_map_cb *cb)
}
}
+
+/* Returns power (in milliWatt seconds) for cgroup */
+static u64 omap_cpuacct_power_usage(void *cpuacct_data)
+{
+ int i;
+ u64 *cpuacct_freq = cpuacct_data;
+ u64 totalpower = 0;
+
+ /* mpu_opps table starts at 1 */
+ for (i = 0; freq_table[i].frequency != CPUFREQ_TABLE_END; i++)
+ totalpower += cpuacct_freq[i] * mpu_opps[i + 1].power;
+
+ /* milliwatt seconds */
+ totalpower = div64_u64(totalpower, NSEC_PER_SEC);
+
+ return totalpower;
+}
+
+
static struct cpuacct_cpufreq_calls omap_cpuacct_cpufreq = {
.init = omap_cpuacct_freq_init,
.charge = omap_cpuacct_freq_charge,
- .show = omap_cpuacct_freq_show,
+ .cpufreq_show = omap_cpuacct_freq_show,
+ .power_usage = omap_cpuacct_power_usage,
};
static int __init omap_cpuacct_init(void)
diff --git a/arch/arm/plat-omap/include/plat/omap-pm.h
b/arch/arm/plat-omap/include/plat/omap-pm.h
index 3ee41d7..8f2db6f 100644
--- a/arch/arm/plat-omap/include/plat/omap-pm.h
+++ b/arch/arm/plat-omap/include/plat/omap-pm.h
@@ -31,6 +31,7 @@ struct omap_opp {
unsigned long rate;
u8 opp_id;
u16 min_vdd;
+ unsigned long power; /* power consumed running at OPP in milliWatts */
};
extern struct omap_opp *mpu_opps;
--
1.7.0.1
--
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: [linux-pm] [PATCH 0/8] Suspend block api (version 6)
On Mon, May 17, 2010 at 11:39 AM, Felipe Balbi m...@felipebalbi.com wrote: Hi, On Mon, May 17, 2010 at 11:26:59AM -0700, Brian Swetland wrote: We (Google) would like to allow completely open app distribution with minimal hurdles, and avoid the walled garden approach. Toward this goal we're not even requiring the use of a central app store for distribution. I understand that, but still we should be telling developers what they're doing wrong so that they can improve themselves as professionals and still make the final device better. We currently do track power usage per-application which is displayed in the phone UI. (Settings - About Phone - Battery Usage). We also provide several (although not perfect) command line utilities for developers to see their power impact system. We are constantly working on ways to improve tracking and make such power data more accessible to developers so they can see how their applications are impacting battery life. Obviously, given the ability to run *any* app, users will run into bad (or perhaps just less-than-optimal-powerwise) apps. Being able to provide the best possible battery life (in spite of sometimes-nonoptimal userspace apps) and simultaneously informing users about which apps are better/worse for their battery life is a goal here. I see. Just hope MeeGo doesn't venture on the same waters :-s For a large majority of apps, running in the background while the device is asleep (screen off) is not essential, they don't request the keep device awake permission, never hold a wakelock, etc. Those that do need to do this have the permission, may hold suspend blockers, and are accounted for. but can anyone write an app that holds a suspend_blocker ?? If so, then your goal is already broken, right ? I mean, if anyone can keep a suspend_blocker held forever, you'll never ever sleep, right ? While with runtime, if you keep the keypad open, only the keypad and the paths directly related to it (probably the i2c controller and the power domain where the i2c controller sits) will be kept alive, no ? Any app can grab a suspend blocker, and the stats are logged, and if you're app is abusing wakelocks and CPU resource it will show up in the Battery Use panel. -- Mike Unrelated to apps, the ability to say please enter suspend as soon as there's no more work (kernel or userspace) preventing it, in a simple, non-racy way is useful. I just tend to agree with Kevin on questioning how different how different this actually is from runtime_pm. I guess I would need to dig through some documentation in order to understand but it seems really similar. -- balbi -- 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 -- 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: [linux-pm] [PATCH 0/8] Suspend block api (version 6)
On Mon, May 17, 2010 at 12:27 PM, Vitaly Wool vitalyw...@gmail.com wrote: On Mon, May 17, 2010 at 6:12 PM, Kevin Hilman khil...@deeprootsystems.com wrote: and #2, the battery lifetime on the N770 and N800 (both of which I have) is **appalling** **bad**. Appalling bad compared to what? What's probably more interesting in terms of rough comparisons is comparing similar devices with and without opportunistic suspend. The Nokia n900 (maemo) and the Moto Droid (android) use the same SoC (TI OMAP3) and roughly the same kernel (2.6.2[89], although both are heavily patched from mainline.) The n900 *never* suspends. It only uses dynamic PM + CPUidle. The droid uses opportunistic suspend (as well as dynamic PM + CPUidle) I don't know of any more objective comparison of the two, but as a user of both devices I can say that the active usage is basically the same (around a day) and the idle use is similar as well, even though the Droid has a slightly bigger battery (1400 mAh vs. 1320 mAh.) My own usage suggests the n900 is a bit better in idle time, but I have not done any measuring or objective tests. I'm guessing the difference is probably because the Droid does not use the deepest off-mode power states either in idle or suspend (IIRC) where the n900 does. I suspect that if both were using off-mode and had the same battery, these differences would go away. Although both are OMAP3430 and run 2.6.29 you cannot compare the N900 and Droid's perceived user battery life to one another to evaluate opportunistic suspend. There are many factors uncounted for such as: network reception, screen brightness (and size), button back-light, keyboard back-light, modem stack (CDMA vs UMTS). Also the difference in uerspace. While this is not really a scientific comparison, it at least gives a rough idea. If using opportunistic suspend was adding noticably better battery life, I think this would be a different discussion. Exactly. The point is, opportunistic suspend doesn't in fact add any value compared to dynamic PM + CPUIdle. It only produces some false impression that one can handle power management right without using dynamic PM. And this false impression is the cause for many really ugly designs (like, for instance, 15 minutes touchscreen inactivity delay before forcibly shutting down the wireless, as it's done in stock Android framework). Opportunistic suspend is an extension to the current suspend model, not a replacement dynamic / run-time PM. If you can replace good old suspend then this would be a different story. As you mention, Droid uses opportunistic suspend + dynamic pm + cpuidle + freq. So I decided to do some measurements on a Droid using our 2.9.32 kernel (http://android.git.kernel.org/?p=kernel/omap.git;a=summary). For a little better apples to apples comparison. Droid (idle system, airplane mode, screen off, 3 min interval): measured average current - with opportunity suspend: 3.19mA - without opportunistic suspend: 3.5mA Stock userspace build, all I did was replace the kernel. We are hitting retention on idle as well as suspend for omap (instead of full off-mode). Also, your point about wifi, the 15 min timeout is in the framework and is configurable in the code and via UI, nothing to do with kernel, opportunistic suspend or run time suspend. -- Mike Thanks, Vitaly ___ linux-pm mailing list linux...@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/linux-pm -- 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: [linux-pm] [PATCH 0/8] Suspend block api (version 6)
On Mon, May 17, 2010 at 1:17 PM, Vitaly Wool vitalyw...@gmail.com wrote: On Mon, May 17, 2010 at 10:07 PM, Mike Chan m...@android.com wrote: On Mon, May 17, 2010 at 12:27 PM, Vitaly Wool vitalyw...@gmail.com wrote: On Mon, May 17, 2010 at 6:12 PM, Kevin Hilman khil...@deeprootsystems.com wrote: and #2, the battery lifetime on the N770 and N800 (both of which I have) is **appalling** **bad**. Appalling bad compared to what? What's probably more interesting in terms of rough comparisons is comparing similar devices with and without opportunistic suspend. The Nokia n900 (maemo) and the Moto Droid (android) use the same SoC (TI OMAP3) and roughly the same kernel (2.6.2[89], although both are heavily patched from mainline.) The n900 *never* suspends. It only uses dynamic PM + CPUidle. The droid uses opportunistic suspend (as well as dynamic PM + CPUidle) I don't know of any more objective comparison of the two, but as a user of both devices I can say that the active usage is basically the same (around a day) and the idle use is similar as well, even though the Droid has a slightly bigger battery (1400 mAh vs. 1320 mAh.) My own usage suggests the n900 is a bit better in idle time, but I have not done any measuring or objective tests. I'm guessing the difference is probably because the Droid does not use the deepest off-mode power states either in idle or suspend (IIRC) where the n900 does. I suspect that if both were using off-mode and had the same battery, these differences would go away. Although both are OMAP3430 and run 2.6.29 you cannot compare the N900 and Droid's perceived user battery life to one another to evaluate opportunistic suspend. There are many factors uncounted for such as: network reception, screen brightness (and size), button back-light, keyboard back-light, modem stack (CDMA vs UMTS). Also the difference in uerspace. While this is not really a scientific comparison, it at least gives a rough idea. If using opportunistic suspend was adding noticably better battery life, I think this would be a different discussion. Okay, I measured with and without using the scientific method. With a 1390mAh battery, if you set your device in airplane mode you will get 435.7 hours of standby vs 397.14 hours of standby. Is this data sufficient for a different discussion now? Exactly. The point is, opportunistic suspend doesn't in fact add any value compared to dynamic PM + CPUIdle. It only produces some false impression that one can handle power management right without using dynamic PM. And this false impression is the cause for many really ugly designs (like, for instance, 15 minutes touchscreen inactivity delay before forcibly shutting down the wireless, as it's done in stock Android framework). Opportunistic suspend is an extension to the current suspend model, not a replacement dynamic / run-time PM. If you can replace good old suspend then this would be a different story. Yes, but what does it extend? What aspect it makes the current suspend model better in? Network configuration and cell reception is a big factor here. You can easily get +4-8mA on the original numbers I gave below, so its large enough to skew your perceived power usage. As you mention, Droid uses opportunistic suspend + dynamic pm + cpuidle + freq. So I decided to do some measurements on a Droid using our 2.9.32 kernel (http://android.git.kernel.org/?p=kernel/omap.git;a=summary). For a little better apples to apples comparison. Droid (idle system, airplane mode, screen off, 3 min interval): measured average current - with opportunity suspend: 3.19mA - without opportunistic suspend: 3.5mA Stock userspace build, all I did was replace the kernel. We are hitting retention on idle as well as suspend for omap (instead of full off-mode). That's implementation specific. If CPUIdle implemented CPU deep sleep, I bet you would see different figures. The important part here is not the absolute value, but how they compare relatively. If I added off-mode support then I the averages would both drop, but they would not be the same (key point). I simply wanted show with real numbers that there is a difference in opportunistic suspend and without. Instead of purely hypothesizing that there is no power benefit from using opportunistic suspend. Also, your point about wifi, the 15 min timeout is in the framework and is configurable in the code and via UI, nothing to do with kernel, opportunistic suspend or run time suspend. You don't quite get it :) This should NOT at all be timeout driven. This should be activity driven or constraint driven which perfectly fits into the runtime PM paradigm but is in no way cleanly implemented within the pure Android PM. I admit, our timeout approach is a bit hacky, but there is method for the madness. In order to properly determine when you should turn off wifi for perfect power management, you need to know a few things
[PATCH 2/2] omap: pm34xx: Remove PER wakeup dependency on CORE.
We can remove this wakeup dependency since now, when
GPIO2-6 are enabled for IO-pad wakeup, PER domain is gauranteed
to be awake or be woken up to service.
The previous dependency did not handle all corner cases. Since there
was no sleep dependency between CORE and PER domains, if PER enters
RET and CORE is ON, PER will not be active for GPIO handling.
Signed-off-by: Mike Chan m...@android.com
---
arch/arm/mach-omap2/pm34xx.c |8
1 files changed, 0 insertions(+), 8 deletions(-)
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 2b2eaaa..acf180b 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -1086,14 +1086,6 @@ static int __init omap3_pm_init(void)
omap3_idle_init();
clkdm_add_wkdep(neon_clkdm, mpu_clkdm);
- /*
-* REVISIT: This wkdep is only necessary when GPIO2-6 are enabled for
-* IO-pad wakeup. Otherwise it will unnecessarily waste power
-* waking up PER with every CORE wakeup - see
-* http://marc.info/?l=linux-omapm=121852150710062w=2
- */
- clkdm_add_wkdep(per_clkdm, core_clkdm);
-
if (omap_type() != OMAP2_DEVICE_TYPE_GP) {
omap3_secure_ram_storage =
kmalloc(0x803F, GFP_KERNEL);
--
1.7.0.1
--
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
[PATCH 1/2] omap: pm34xx: Enable IO / IO-CHAIN wakeups for PER
IO events can also come from GPIO modules, which reside in the PER domain.
It is possible for the PER to enter RET while CORE is still in ON.
If GPIO 2-6 are enabled for IO-pad wakeups, the PER domain will not
wakeup in this case, unless we enable it.
Signed-off-by: Mike Chan m...@android.com
---
arch/arm/mach-omap2/pm34xx.c | 14 ++
1 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index eab..2b2eaaa 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -371,6 +371,13 @@ void omap_sram_idle(void)
if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON)
pwrdm_set_next_pwrst(neon_pwrdm, mpu_next_state);
+ /* Enable IO-PAD and IO-CHAIN wakeups */
+ if (per_next_state PWRDM_POWER_ON ||
+ core_next_state PWRDM_POWER_ON) {
+ prm_set_mod_reg_bits(OMAP3430_EN_IO, WKUP_MOD, PM_WKEN);
+ omap3_enable_io_chain();
+ }
+
/* PER */
per_next_state = pwrdm_read_next_pwrst(per_pwrdm);
core_next_state = pwrdm_read_next_pwrst(core_pwrdm);
@@ -398,10 +405,8 @@ void omap_sram_idle(void)
omap3_core_save_context();
omap3_prcm_save_context();
}
- /* Enable IO-PAD and IO-CHAIN wakeups */
- prm_set_mod_reg_bits(OMAP3430_EN_IO, WKUP_MOD, PM_WKEN);
- omap3_enable_io_chain();
}
+
omap3_intc_prepare_idle();
/*
@@ -463,7 +468,8 @@ void omap_sram_idle(void)
}
/* Disable IO-PAD and IO-CHAIN wakeup */
- if (core_next_state PWRDM_POWER_ON) {
+ if (per_next_state PWRDM_POWER_ON ||
+ core_next_state PWRDM_POWER_ON) {
prm_clear_mod_reg_bits(OMAP3430_EN_IO, WKUP_MOD, PM_WKEN);
omap3_disable_io_chain();
}
--
1.7.0.1
--
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 1/2] omap: pm34xx: Enable IO / IO-CHAIN wakeups for PER
On Mon, May 3, 2010 at 3:40 PM, Kevin Hilman
khil...@deeprootsystems.com wrote:
Mike Chan m...@android.com writes:
IO events can also come from GPIO modules, which reside in the PER domain.
It is possible for the PER to enter RET while CORE is still in ON.
If GPIO 2-6 are enabled for IO-pad wakeups, the PER domain will not
wakeup in this case, unless we enable it.
Signed-off-by: Mike Chan m...@android.com
Thanks for moving this up to before the potential transition of PER.
But...
---
arch/arm/mach-omap2/pm34xx.c | 14 ++
1 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index eab..2b2eaaa 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -371,6 +371,13 @@ void omap_sram_idle(void)
if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON)
pwrdm_set_next_pwrst(neon_pwrdm, mpu_next_state);
+ /* Enable IO-PAD and IO-CHAIN wakeups */
+ if (per_next_state PWRDM_POWER_ON ||
+ core_next_state PWRDM_POWER_ON) {
+ prm_set_mod_reg_bits(OMAP3430_EN_IO, WKUP_MOD, PM_WKEN);
+ omap3_enable_io_chain();
+ }
+
oops, both next_states will always be PWRDM_POWER_ON here as they
haven't yet been read...
/* PER */
per_next_state = pwrdm_read_next_pwrst(per_pwrdm);
core_next_state = pwrdm_read_next_pwrst(core_pwrdm);
until here.
Sloppy me, I'll send out a v2 that fixes this. Thanks Kevin for catching this.
-- Mike
Kevin
--
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
[PATCH v2 1/2] omap: pm34xx: Enable IO / IO-CHAIN wakeups for PER
IO events can also come from GPIO modules, which reside in the PER domain.
It is possible for the PER to enter RET while CORE is still in ON.
If GPIO 2-6 are enabled for IO-pad wakeups, the PER domain will not
wakeup in this case, unless we enable it.
Signed-off-by: Mike Chan m...@android.com
---
arch/arm/mach-omap2/pm34xx.c | 16 +++-
1 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index eab..fd76b0d 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -371,9 +371,16 @@ void omap_sram_idle(void)
if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON)
pwrdm_set_next_pwrst(neon_pwrdm, mpu_next_state);
- /* PER */
+ /* Enable IO-PAD and IO-CHAIN wakeups */
per_next_state = pwrdm_read_next_pwrst(per_pwrdm);
core_next_state = pwrdm_read_next_pwrst(core_pwrdm);
+ if (per_next_state PWRDM_POWER_ON ||
+ core_next_state PWRDM_POWER_ON) {
+ prm_set_mod_reg_bits(OMAP3430_EN_IO, WKUP_MOD, PM_WKEN);
+ omap3_enable_io_chain();
+ }
+
+ /* PER */
if (per_next_state PWRDM_POWER_ON) {
omap_uart_prepare_idle(2);
omap2_gpio_prepare_for_retention();
@@ -398,10 +405,8 @@ void omap_sram_idle(void)
omap3_core_save_context();
omap3_prcm_save_context();
}
- /* Enable IO-PAD and IO-CHAIN wakeups */
- prm_set_mod_reg_bits(OMAP3430_EN_IO, WKUP_MOD, PM_WKEN);
- omap3_enable_io_chain();
}
+
omap3_intc_prepare_idle();
/*
@@ -463,7 +468,8 @@ void omap_sram_idle(void)
}
/* Disable IO-PAD and IO-CHAIN wakeup */
- if (core_next_state PWRDM_POWER_ON) {
+ if (per_next_state PWRDM_POWER_ON ||
+ core_next_state PWRDM_POWER_ON) {
prm_clear_mod_reg_bits(OMAP3430_EN_IO, WKUP_MOD, PM_WKEN);
omap3_disable_io_chain();
}
--
1.7.0.1
--
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
[PATCH v2 2/2] omap: pm34xx: Remove PER wakeup dependency on CORE.
We can remove this wakeup dependency since now, when
GPIO2-6 are enabled for IO-pad wakeup, PER domain is gauranteed
to be awake or be woken up to service.
The previous dependency did not handle all corner cases. Since there
was no sleep dependency between CORE and PER domains, if PER enters
RET and CORE is ON, PER will not be active for GPIO handling.
Signed-off-by: Mike Chan m...@android.com
---
arch/arm/mach-omap2/pm34xx.c |8
1 files changed, 0 insertions(+), 8 deletions(-)
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index fd76b0d..88eafb0 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -1086,14 +1086,6 @@ static int __init omap3_pm_init(void)
omap3_idle_init();
clkdm_add_wkdep(neon_clkdm, mpu_clkdm);
- /*
-* REVISIT: This wkdep is only necessary when GPIO2-6 are enabled for
-* IO-pad wakeup. Otherwise it will unnecessarily waste power
-* waking up PER with every CORE wakeup - see
-* http://marc.info/?l=linux-omapm=121852150710062w=2
- */
- clkdm_add_wkdep(per_clkdm, core_clkdm);
-
if (omap_type() != OMAP2_DEVICE_TYPE_GP) {
omap3_secure_ram_storage =
kmalloc(0x803F, GFP_KERNEL);
--
1.7.0.1
--
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 v2 1/2] omap: pm34xx: Enable IO / IO-CHAIN wakeups for PER
On Mon, May 3, 2010 at 4:35 PM, Kevin Hilman khil...@deeprootsystems.com wrote: Mike Chan m...@android.com writes: IO events can also come from GPIO modules, which reside in the PER domain. It is possible for the PER to enter RET while CORE is still in ON. If GPIO 2-6 are enabled for IO-pad wakeups, the PER domain will not wakeup in this case, unless we enable it. Signed-off-by: Mike Chan m...@android.com Thanks, adding both patches to PM branch (pm-fixes branch) and will queue for 2.6.35. Thanks, with this patch looking good, I was hoping for some feedback from people on the 2nd patch in the series. It makes sense to me since PER will be woken up now, not just CORE, but I wanted to double check. -- Mike Kevin -- 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
[PATCH] [ARM] omap-pm: resource: Protect static pool from concurrent get_user() calls
With per-resource mutexes, mulitple threads can call resource_request()
which can cause get_user() to concurrently allocate from the static pool.
Take the res_mutex when allocating from the static pool, which is used
in resouce_request() and resource_release() to guard against this.
Signed-off-by: Mike Chan m...@android.com
---
arch/arm/plat-omap/resource.c | 55 +++-
1 files changed, 32 insertions(+), 23 deletions(-)
diff --git a/arch/arm/plat-omap/resource.c b/arch/arm/plat-omap/resource.c
index 0a7b79b..f769f7c 100644
--- a/arch/arm/plat-omap/resource.c
+++ b/arch/arm/plat-omap/resource.c
@@ -148,34 +148,39 @@ static struct users_list *get_user(void)
int ind = 0;
struct users_list *user;
+ /*
+* When allocating from the static pool, we must lock resource mutex,
+* to protect against concurrent get_user() calls.
+*/
+ down(res_mutex);
/* See if something available in the static pool */
while (ind MAX_USERS) {
- if (usr_list[ind].usage == UNUSED)
- break;
- else
- ind++;
+ if (usr_list[ind].usage == UNUSED) {
+ /* Pick from the static pool */
+ user = usr_list[ind];
+ user-usage = STATIC_ALLOC;
+ up(res_mutex);
+ return user;
+ }
+ ind++;
}
- if (ind MAX_USERS) {
- /* Pick from the static pool */
- user = usr_list[ind];
- user-usage = STATIC_ALLOC;
- } else {
- /* By this time we hope slab is initialized */
- if (slab_is_available()) {
- user = kmalloc(sizeof(struct users_list), GFP_KERNEL);
- if (!user) {
- printk(KERN_ERR SRF:FATAL ERROR: kmalloc
- failed\n);
- return ERR_PTR(-ENOMEM);
- }
- user-usage = DYNAMIC_ALLOC;
- } else {
- /* Dynamic alloc not available yet */
- printk(KERN_ERR SRF: FATAL ERROR: users_list
- initial POOL EMPTY before slab init\n);
+ up(res_mutex);
+ /* By this time we hope slab is initialized */
+ if (slab_is_available()) {
+ user = kmalloc(sizeof(struct users_list), GFP_KERNEL);
+ if (!user) {
+ printk(KERN_ERR SRF:FATAL ERROR: kmalloc
+ failed\n);
return ERR_PTR(-ENOMEM);
}
+ user-usage = DYNAMIC_ALLOC;
+ } else {
+ /* Dynamic alloc not available yet */
+ printk(KERN_ERR SRF: FATAL ERROR: users_list
+ initial POOL EMPTY before slab init\n);
+ return ERR_PTR(-ENOMEM);
}
+
return user;
}
@@ -192,8 +197,12 @@ void free_user(struct users_list *user)
if (user-usage == DYNAMIC_ALLOC) {
kfree(user);
} else {
- user-usage = UNUSED;
+ /*
+* It is not necesssary to lock when returning to the
+* static pool.
+*/
user-dev = NULL;
+ user-usage = UNUSED;
}
}
--
1.7.0.1
--
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
[PATCH 1/2] [ARM] omap-pm: resource: Lock resource list in update_resource_level
Take the resource mutex when iterating over the resource user_list.
A race can occur if resource_request() adds a first time user to
the user_list while update_resource_level() is called.
Signed-off-by: Mike Chan m...@android.com
---
arch/arm/plat-omap/resource.c |9 +++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/arm/plat-omap/resource.c b/arch/arm/plat-omap/resource.c
index f769f7c..6d3bba6 100644
--- a/arch/arm/plat-omap/resource.c
+++ b/arch/arm/plat-omap/resource.c
@@ -106,6 +106,7 @@ static int update_resource_level(struct shared_resource
*resp)
unsigned long target_level;
int ret;
+ mutex_lock(resp-resource_mutex);
/* Regenerate the target_value for the resource */
if (resp-flags RES_TYPE_PERFORMANCE) {
target_level = RES_PERFORMANCE_DEFAULTLEVEL;
@@ -118,9 +119,11 @@ static int update_resource_level(struct shared_resource
*resp)
if (user-level target_level)
target_level = user-level;
} else {
+ mutex_unlock(resp-resource_mutex);
pr_debug(SRF: Unknown resource type\n);
return -EINVAL;
}
+ mutex_unlock(resp-resource_mutex);
pr_debug(SRF: Changing Level for resource %s to %ld\n,
resp-name, target_level);
@@ -439,10 +442,12 @@ int resource_release(const char *name, struct device *dev)
list_del(user-node);
free_user(user);
- /* Recompute and set the current level for the resource */
- ret = update_resource_level(resp);
res_unlock:
mutex_unlock(resp-resource_mutex);
+ /* Recompute and set the current level for the resource */
+ if (!ret)
+ ret = update_resource_level(resp);
+
return ret;
}
EXPORT_SYMBOL(resource_release);
--
1.7.0.1
--
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
[PATCH 2/2] [ARM] omap-pm: resource: Only update resource level if there is a change
Previosuly update_resource_level() would always call the change_level()
function, even if there was no change necessary.
Signed-off-by: Mike Chan m...@android.com
---
arch/arm/plat-omap/resource.c |5 -
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/arch/arm/plat-omap/resource.c b/arch/arm/plat-omap/resource.c
index 6d3bba6..ff5a938 100644
--- a/arch/arm/plat-omap/resource.c
+++ b/arch/arm/plat-omap/resource.c
@@ -104,7 +104,7 @@ static int update_resource_level(struct shared_resource
*resp)
{
struct users_list *user;
unsigned long target_level;
- int ret;
+ int ret = 0;
mutex_lock(resp-resource_mutex);
/* Regenerate the target_value for the resource */
@@ -125,6 +125,9 @@ static int update_resource_level(struct shared_resource
*resp)
}
mutex_unlock(resp-resource_mutex);
+ if (resp-curr_level == target_level)
+ return ret;
+
pr_debug(SRF: Changing Level for resource %s to %ld\n,
resp-name, target_level);
ret = resp-ops-change_level(resp, target_level);
--
1.7.0.1
--
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 1/2] omap: pm34xx: Enable IO / IO-CHAIN wakeups for PER
On Thu, Apr 22, 2010 at 3:31 PM, Kevin Hilman khil...@deeprootsystems.com wrote: Mike Chan m...@android.com writes: On Wed, Apr 21, 2010 at 5:07 PM, Kevin Hilman khil...@deeprootsystems.com wrote: Mike Chan m...@android.com writes: IO events can also come from GPIO modules, which reside in the PER domain. It is possible for the PER to enter RET while CORE is still in ON. If GPIO 2-6 are enabled for IO-pad wakeups, the PER domain will not wakeup in this case, unless we enable it. Signed-off-by: Mike Chan m...@android.com Hi Mike, I'm a little puzzled on this one. My understanding is that the IO pad is only armed when CORE is in RET or OFF. The issue we are seeing is when the device is active but idle, if CORE is ON and PER is in RET and the omap is sitting in swfi. If the user presses a keypad button, IO pad doesn't wake us out of idle. Setting a wakeup if PER or CORE goes into RET solve this. I need to dig a little more in the TRM on this one to clarify. I was looking at 4.11.2.2 I/O Wake-Up Mechanism (pg 421) Yeah, that's the right place. After a little more digging and asking around, this looks like a good fix, but there's a minor problem with the implementation: In the section of the TRM you referenced the following sentence is hiding: Software must wait for the I/O daisy chain to complete before it transitions the PER domain to a nonfunctional state. In the proposed patch, it's likely that PER could transition to INACTIVE/RET/OFF before the IO wakeups are enabled. For example, if nothing in PER is active except UART3, then PER will transition to an idle state right after omap_uart_prepare_idle(2), which is before the IO wakeups are currently enabled. To be perfectly safe, the IO wakeups should be enabled before PER is allowed to transition. Sounds good, I'll spin a v2 and send it out. -- Mike Kevin -- 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
[PATCH 1/2] omap: pm34xx: Enable IO / IO-CHAIN wakeups for PER
IO events can also come from GPIO modules, which reside in the PER domain.
It is possible for the PER to enter RET while CORE is still in ON.
If GPIO 2-6 are enabled for IO-pad wakeups, the PER domain will not
wakeup in this case, unless we enable it.
Signed-off-by: Mike Chan m...@android.com
---
arch/arm/mach-omap2/pm34xx.c | 10 --
1 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index eab..4ef322a 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -398,10 +398,15 @@ void omap_sram_idle(void)
omap3_core_save_context();
omap3_prcm_save_context();
}
- /* Enable IO-PAD and IO-CHAIN wakeups */
+ }
+
+ /* Enable IO-PAD and IO-CHAIN wakeups */
+ if (per_next_state PWRDM_POWER_ON ||
+ core_next_state PWRDM_POWER_ON) {
prm_set_mod_reg_bits(OMAP3430_EN_IO, WKUP_MOD, PM_WKEN);
omap3_enable_io_chain();
}
+
omap3_intc_prepare_idle();
/*
@@ -463,7 +468,8 @@ void omap_sram_idle(void)
}
/* Disable IO-PAD and IO-CHAIN wakeup */
- if (core_next_state PWRDM_POWER_ON) {
+ if (per_next_state PWRDM_POWER_ON ||
+ core_next_state PWRDM_POWER_ON) {
prm_clear_mod_reg_bits(OMAP3430_EN_IO, WKUP_MOD, PM_WKEN);
omap3_disable_io_chain();
}
--
1.7.0.1
--
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
[PATCH 2/2] omap: pm34xx: Remove PER wakeup dependency on CORE.
We can remove this wakeup dependency since now, when
GPIO2-6 are enabled for IO-pad wakeup, PER domain is gauranteed
to be awake or be woken up to service.
The previous dependency did not handle all corner cases. Since there
was no sleep dependency between CORE and PER domains, if PER enters
RET and CORE is ON, PER will not be active for GPIO handling.
Signed-off-by: Mike Chan m...@android.com
---
arch/arm/mach-omap2/pm34xx.c |8
1 files changed, 0 insertions(+), 8 deletions(-)
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 4ef322a..176870f 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -1086,14 +1086,6 @@ static int __init omap3_pm_init(void)
omap3_idle_init();
clkdm_add_wkdep(neon_clkdm, mpu_clkdm);
- /*
-* REVISIT: This wkdep is only necessary when GPIO2-6 are enabled for
-* IO-pad wakeup. Otherwise it will unnecessarily waste power
-* waking up PER with every CORE wakeup - see
-* http://marc.info/?l=linux-omapm=121852150710062w=2
- */
- clkdm_add_wkdep(per_clkdm, core_clkdm);
-
if (omap_type() != OMAP2_DEVICE_TYPE_GP) {
omap3_secure_ram_storage =
kmalloc(0x803F, GFP_KERNEL);
--
1.7.0.1
--
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] omap: resource: Add per-resource mutex for OMAP resource framework
On Tue, Mar 9, 2010 at 4:09 PM, y...@google.com wrote:
From: Chunqiu Wang cqw...@motorola.com
Current OMAP resource fwk uses a global res_mutex
for resource_request and resource_release calls
for all the available resources.It may cause dead
lock if resource_request/resource_release is called
recursively.
For current OMAP3 VDD1/VDD2 resource, the change_level
implementation is mach-omap2/resource34xx.c/set_opp(),
when using resource_release to remove vdd1 constraint,
this function may call resource_release again to release
Vdd2 constrait if target vdd1 level is less than OPP3.
in this scenario, the global res_mutex down operation
will be called again, this will cause the second
down operation hang there.
To fix the problem, per-resource mutex is added
to avoid hangup when resource_request/resource_release
is called recursively.
Signed-off-by: Chunqiu Wang cqw...@motorola.com
Signed-off-by: Mike Chan m...@android.com
---
arch/arm/plat-omap/include/plat/resource.h | 2 ++
arch/arm/plat-omap/resource.c | 21 ++---
2 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/arch/arm/plat-omap/include/plat/resource.h
b/arch/arm/plat-omap/include/plat/resource.h
index 9acebcc..b5aff1f 100644
--- a/arch/arm/plat-omap/include/plat/resource.h
+++ b/arch/arm/plat-omap/include/plat/resource.h
@@ -54,6 +54,8 @@ struct shared_resource {
/* Shared resource operations */
struct shared_resource_ops *ops;
struct list_head node;
+ /* Protect each resource */
+ struct mutex resource_mutex;
};
struct shared_resource_ops {
diff --git a/arch/arm/plat-omap/resource.c b/arch/arm/plat-omap/resource.c
index f1cdecf..0a7b79b 100644
--- a/arch/arm/plat-omap/resource.c
+++ b/arch/arm/plat-omap/resource.c
@@ -277,6 +277,7 @@ int resource_register(struct shared_resource *resp)
}
INIT_LIST_HEAD(resp-users_list);
+ mutex_init(resp-resource_mutex);
/* Add the resource to the resource list */
list_add(resp-node, res_list);
@@ -339,14 +340,13 @@ int resource_request(const char *name, struct device
*dev,
struct users_list *user;
int found = 0, ret = 0;
- down(res_mutex);
- resp = _resource_lookup(name);
+ resp = resource_lookup(name);
if (!resp) {
printk(KERN_ERR resource_request: Invalid resource name\n);
- ret = -EINVAL;
- goto res_unlock;
+ return -EINVAL;
}
+ mutex_lock(resp-resource_mutex);
/* Call the resource specific validate function */
if (resp-ops-validate_level) {
ret = resp-ops-validate_level(resp, level);
@@ -375,7 +375,7 @@ int resource_request(const char *name, struct device *dev,
user-level = level;
res_unlock:
- up(res_mutex);
+ mutex_unlock(resp-resource_mutex);
/*
* Recompute and set the current level for the resource.
* NOTE: update_resource level moved out of spin_lock, as it may call
@@ -406,14 +406,13 @@ int resource_release(const char *name, struct device
*dev)
struct users_list *user;
int found = 0, ret = 0;
- down(res_mutex);
- resp = _resource_lookup(name);
+ resp = resource_lookup(name);
if (!resp) {
printk(KERN_ERR resource_release: Invalid resource name\n);
- ret = -EINVAL;
- goto res_unlock;
+ return -EINVAL;
}
+ mutex_lock(resp-resource_mutex);
list_for_each_entry(user, resp-users_list, node) {
if (user-dev == dev) {
found = 1;
@@ -434,7 +433,7 @@ int resource_release(const char *name, struct device *dev)
/* Recompute and set the current level for the resource */
ret = update_resource_level(resp);
res_unlock:
- up(res_mutex);
+ mutex_unlock(resp-resource_mutex);
return ret;
}
EXPORT_SYMBOL(resource_release);
@@ -458,7 +457,7 @@ int resource_get_level(const char *name)
up(res_mutex);
return -EINVAL;
}
- ret = resp-curr_level;
+ ret = resp-curr_level;
up(res_mutex);
return ret;
}
--
1.6.6.2
Oops, my git client was slightly screwed up, apologies, I meant to
send this from m...@android.com, the patch is still good though :)
-- Mike
--
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] omap: resource: Fix race in register_resource()
On Wed, Sep 30, 2009 at 4:49 PM, Kevin Hilman
khil...@deeprootsystems.com wrote:
Mike Chan m...@android.com writes:
Checking if the resource is already registered and adding to the list
must be atomic or bad things can happen.
Signed-off-by: Mike Chan m...@android.com
---
arch/arm/plat-omap/resource.c | 13 -
1 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/arch/arm/plat-omap/resource.c b/arch/arm/plat-omap/resource.c
index 111020a..4631912 100644
--- a/arch/arm/plat-omap/resource.c
+++ b/arch/arm/plat-omap/resource.c
@@ -255,6 +255,7 @@ int resource_refresh(void)
*/
int resource_register(struct shared_resource *resp)
{
+ int ret = 0;
if (!resp)
return -EINVAL;
@@ -262,12 +263,13 @@ int resource_register(struct shared_resource *resp)
return -EINVAL;
/* Verify that the resource is not already registered */
- if (resource_lookup(resp-name))
- return -EEXIST;
+ down(res_mutex);
+ if (_resource_lookup(resp-name))
+ ret = -EEXIST;
+ goto out;
Ahem, you're rebased version has a little problem. Missing {} block
mans this goto is always executed and no resources are ever added.
I'll fix this up manually before applying, but some more testing next
time would be helpful
Opps, sorry about that :/ and thanks for cleaning it up.
-- Mike
Thanks,
Kevin
INIT_LIST_HEAD(resp-users_list);
- down(res_mutex);
/* Add the resource to the resource list */
list_add(resp-node, res_list);
@@ -275,10 +277,11 @@ int resource_register(struct shared_resource *resp)
if (resp-ops-init)
resp-ops-init(resp);
- up(res_mutex);
pr_debug(resource: registered %s\n, resp-name);
- return 0;
+out:
+ up(res_mutex);
+ return ret;
}
EXPORT_SYMBOL(resource_register);
--
1.5.4.5
--
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
[PATCH] omap: resource: Fix race in register_resource()
Checking if the resource is already registered and adding to the list
must be atomic or bad things can happen.
Signed-off-by: Mike Chan m...@android.com
---
arch/arm/plat-omap/resource.c | 13 -
1 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/arch/arm/plat-omap/resource.c b/arch/arm/plat-omap/resource.c
index 111020a..4631912 100644
--- a/arch/arm/plat-omap/resource.c
+++ b/arch/arm/plat-omap/resource.c
@@ -255,6 +255,7 @@ int resource_refresh(void)
*/
int resource_register(struct shared_resource *resp)
{
+ int ret = 0;
if (!resp)
return -EINVAL;
@@ -262,12 +263,13 @@ int resource_register(struct shared_resource *resp)
return -EINVAL;
/* Verify that the resource is not already registered */
- if (resource_lookup(resp-name))
- return -EEXIST;
+ down(res_mutex);
+ if (_resource_lookup(resp-name))
+ ret = -EEXIST;
+ goto out;
INIT_LIST_HEAD(resp-users_list);
- down(res_mutex);
/* Add the resource to the resource list */
list_add(resp-node, res_list);
@@ -275,10 +277,11 @@ int resource_register(struct shared_resource *resp)
if (resp-ops-init)
resp-ops-init(resp);
- up(res_mutex);
pr_debug(resource: registered %s\n, resp-name);
- return 0;
+out:
+ up(res_mutex);
+ return ret;
}
EXPORT_SYMBOL(resource_register);
--
1.5.4.5
--
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
[PATCH] video: omap2: dss: RET on idle, enable/disable dss clocks only when needed.
Signed-off-by: Mike Chan m...@android.com
---
drivers/video/omap2/dss/dispc.c |6 -
drivers/video/omap2/dss/dsi.c | 50 +++---
2 files changed, 51 insertions(+), 5 deletions(-)
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index d061d75..4216466 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -484,13 +484,17 @@ static inline void enable_clocks(bool enable)
bool dispc_go_busy(enum omap_channel channel)
{
int bit;
+ bool ret;
if (channel == OMAP_DSS_CHANNEL_LCD)
bit = 5; /* GOLCD */
else
bit = 6; /* GODIGIT */
- return REG_GET(DISPC_CONTROL, bit, bit) == 1;
+ enable_clocks(1);
+ ret = REG_GET(DISPC_CONTROL, bit, bit) == 1;
+ enable_clocks(0);
+ return ret;
}
void dispc_go(enum omap_channel channel)
diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index 5e3d653..e9d8a5f 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -1186,6 +1186,9 @@ int dsi_pll_init(bool enable_hsclk, bool enable_hsdiv)
if (r)
goto err1;
+ enable_clocks(0);
+ dsi_enable_pll_clock(0);
+
DSSDBG(PLL init done\n);
return 0;
@@ -1199,11 +1202,9 @@ err0:
void dsi_pll_uninit(void)
{
- enable_clocks(0);
- dsi_enable_pll_clock(0);
-
dsi.pll_locked = 0;
dsi_pll_power(DSI_PLL_POWER_OFF);
+
regulator_disable(dsi.vdds_dsi_reg);
DSSDBG(PLL uninit done\n);
}
@@ -1848,6 +1849,9 @@ static int dsi_vc_send_bta_sync(int channel)
INIT_COMPLETION(dsi.bta_completion);
+ enable_clocks(1);
+ dsi_enable_pll_clock(1);
+
dsi_vc_enable_bta_irq(channel);
r = dsi_vc_send_bta(channel);
@@ -1870,6 +1874,9 @@ static int dsi_vc_send_bta_sync(int channel)
err:
dsi_vc_disable_bta_irq(channel);
+ enable_clocks(0);
+ dsi_enable_pll_clock(0);
+
return r;
}
@@ -1921,6 +1928,9 @@ static int dsi_vc_send_long(int channel, u8 data_type, u8
*data, u16 len,
return -EINVAL;
}
+ enable_clocks(1);
+ dsi_enable_pll_clock(1);
+
dsi_vc_write_long_header(channel, data_type, len, ecc);
/*dsi_vc_print_status(0); */
@@ -1964,6 +1974,9 @@ static int dsi_vc_send_long(int channel, u8 data_type, u8
*data, u16 len,
dsi_vc_write_long_payload(channel, b1, b2, b3, 0);
}
+ enable_clocks(0);
+ dsi_enable_pll_clock(0);
+
return r;
}
@@ -1979,6 +1992,9 @@ static int dsi_vc_send_short(int channel, u8 data_type,
u16 data, u8 ecc)
channel,
data_type, data 0xff, (data 8) 0xff);
+ enable_clocks(1);
+ dsi_enable_pll_clock(1);
+
if (FLD_GET(dsi_read_reg(DSI_VC_CTRL(channel)), 16, 16)) {
DSSERR(ERROR FIFO FULL, aborting transfer\n);
return -EINVAL;
@@ -1990,6 +2006,9 @@ static int dsi_vc_send_short(int channel, u8 data_type,
u16 data, u8 ecc)
dsi_write_reg(DSI_VC_SHORT_PACKET_HEADER(channel), r);
+ enable_clocks(0);
+ dsi_enable_pll_clock(0);
+
return 0;
}
@@ -2775,13 +2794,15 @@ static int dsi_update_thread(void *data)
break;
dsi_bus_lock();
-
if (dsi.update_mode == OMAP_DSS_UPDATE_DISABLED ||
kthread_should_stop()) {
dsi_bus_unlock();
break;
}
+ enable_clocks(1);
+ dsi_enable_pll_clock(1);
+
dsi_perf_mark_setup();
if (dsi.update_region.dirty) {
@@ -2872,6 +2893,9 @@ static int dsi_update_thread(void *data)
complete_all(dsi.update_completion);
+ enable_clocks(0);
+ dsi_enable_pll_clock(0);
+
dsi_bus_unlock();
/* XXX We need to give others chance to get the bus lock. Is
@@ -3068,6 +3092,9 @@ static int dsi_display_enable(struct omap_dss_device
*dssdev)
if (dsi.update_mode == OMAP_DSS_UPDATE_AUTO)
dsi_start_auto_update(dssdev);
+ enable_clocks(0);
+ dsi_enable_pll_clock(0);
+
dsi_bus_unlock();
mutex_unlock(dsi.lock);
@@ -3101,6 +3128,9 @@ static void dsi_display_disable(struct omap_dss_device
*dssdev)
dsi.update_mode = OMAP_DSS_UPDATE_DISABLED;
dssdev-state = OMAP_DSS_DISPLAY_DISABLED;
+ enable_clocks(1);
+ dsi_enable_pll_clock(1);
+
dsi_display_uninit_dispc(dssdev);
dsi_display_uninit_dsi(dssdev);
@@ -3128,6 +3158,9 @@ static int dsi_display_suspend(struct omap_dss_device
*dssdev)
dsi.update_mode = OMAP_DSS_UPDATE_DISABLED;
dssdev-state = OMAP_DSS_DISPLAY_SUSPENDED
Re: [PATCH] video: omap2: dss: RET on idle, enable/disable dss clocks only when needed.
On Thu, Sep 17, 2009 at 4:36 PM, Mike Chan m...@android.com wrote:
Signed-off-by: Mike Chan m...@android.com
---
drivers/video/omap2/dss/dispc.c | 6 -
drivers/video/omap2/dss/dsi.c | 50 +++---
2 files changed, 51 insertions(+), 5 deletions(-)
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index d061d75..4216466 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -484,13 +484,17 @@ static inline void enable_clocks(bool enable)
bool dispc_go_busy(enum omap_channel channel)
{
int bit;
+ bool ret;
if (channel == OMAP_DSS_CHANNEL_LCD)
bit = 5; /* GOLCD */
else
bit = 6; /* GODIGIT */
- return REG_GET(DISPC_CONTROL, bit, bit) == 1;
+ enable_clocks(1);
+ ret = REG_GET(DISPC_CONTROL, bit, bit) == 1;
+ enable_clocks(0);
+ return ret;
}
void dispc_go(enum omap_channel channel)
diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index 5e3d653..e9d8a5f 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -1186,6 +1186,9 @@ int dsi_pll_init(bool enable_hsclk, bool enable_hsdiv)
if (r)
goto err1;
+ enable_clocks(0);
+ dsi_enable_pll_clock(0);
+
DSSDBG(PLL init done\n);
return 0;
@@ -1199,11 +1202,9 @@ err0:
void dsi_pll_uninit(void)
{
- enable_clocks(0);
- dsi_enable_pll_clock(0);
-
dsi.pll_locked = 0;
dsi_pll_power(DSI_PLL_POWER_OFF);
+
regulator_disable(dsi.vdds_dsi_reg);
DSSDBG(PLL uninit done\n);
}
@@ -1848,6 +1849,9 @@ static int dsi_vc_send_bta_sync(int channel)
INIT_COMPLETION(dsi.bta_completion);
+ enable_clocks(1);
+ dsi_enable_pll_clock(1);
+
dsi_vc_enable_bta_irq(channel);
r = dsi_vc_send_bta(channel);
@@ -1870,6 +1874,9 @@ static int dsi_vc_send_bta_sync(int channel)
err:
dsi_vc_disable_bta_irq(channel);
+ enable_clocks(0);
+ dsi_enable_pll_clock(0);
+
return r;
}
@@ -1921,6 +1928,9 @@ static int dsi_vc_send_long(int channel, u8 data_type,
u8 *data, u16 len,
return -EINVAL;
}
+ enable_clocks(1);
+ dsi_enable_pll_clock(1);
+
dsi_vc_write_long_header(channel, data_type, len, ecc);
/*dsi_vc_print_status(0); */
@@ -1964,6 +1974,9 @@ static int dsi_vc_send_long(int channel, u8 data_type,
u8 *data, u16 len,
dsi_vc_write_long_payload(channel, b1, b2, b3, 0);
}
+ enable_clocks(0);
+ dsi_enable_pll_clock(0);
+
return r;
}
@@ -1979,6 +1992,9 @@ static int dsi_vc_send_short(int channel, u8 data_type,
u16 data, u8 ecc)
channel,
data_type, data 0xff, (data 8) 0xff);
+ enable_clocks(1);
+ dsi_enable_pll_clock(1);
+
if (FLD_GET(dsi_read_reg(DSI_VC_CTRL(channel)), 16, 16)) {
DSSERR(ERROR FIFO FULL, aborting transfer\n);
return -EINVAL;
@@ -1990,6 +2006,9 @@ static int dsi_vc_send_short(int channel, u8 data_type,
u16 data, u8 ecc)
dsi_write_reg(DSI_VC_SHORT_PACKET_HEADER(channel), r);
+ enable_clocks(0);
+ dsi_enable_pll_clock(0);
+
return 0;
}
@@ -2775,13 +2794,15 @@ static int dsi_update_thread(void *data)
break;
dsi_bus_lock();
-
if (dsi.update_mode == OMAP_DSS_UPDATE_DISABLED ||
kthread_should_stop()) {
dsi_bus_unlock();
break;
}
+ enable_clocks(1);
+ dsi_enable_pll_clock(1);
+
dsi_perf_mark_setup();
if (dsi.update_region.dirty) {
@@ -2872,6 +2893,9 @@ static int dsi_update_thread(void *data)
complete_all(dsi.update_completion);
+ enable_clocks(0);
+ dsi_enable_pll_clock(0);
+
dsi_bus_unlock();
/* XXX We need to give others chance to get the bus lock. Is
@@ -3068,6 +3092,9 @@ static int dsi_display_enable(struct omap_dss_device
*dssdev)
if (dsi.update_mode == OMAP_DSS_UPDATE_AUTO)
dsi_start_auto_update(dssdev);
+ enable_clocks(0);
+ dsi_enable_pll_clock(0);
+
dsi_bus_unlock();
mutex_unlock(dsi.lock);
@@ -3101,6 +3128,9 @@ static void dsi_display_disable(struct omap_dss_device
*dssdev)
dsi.update_mode = OMAP_DSS_UPDATE_DISABLED;
dssdev-state = OMAP_DSS_DISPLAY_DISABLED;
+ enable_clocks(1);
+ dsi_enable_pll_clock(1);
+
dsi_display_uninit_dispc(dssdev);
dsi_display_uninit_dsi(dssdev);
@@ -3128,6 +3158,9 @@ static int dsi_display_suspend(struct omap_dss_device
*dssdev
Re: [PATCH] [ARM] omap: resource: Make resource_refresh() thread safe.
On Fri, Sep 11, 2009 at 2:10 PM, Kevin
Hilmankhil...@deeprootsystems.com wrote:
Mike Turquette mturque...@ti.com writes:
Kevin Hilman wrote:
Mike Chan m...@android.com writes:
Need to lock the res_mutex when traversing the res_list.
Signed-off-by: Mike Chan m...@android.com
Looks good, thanks.
This patch causes a hang for me when transitioning to OFF mode. This
was tested on the Android 2.6.29 tree and is 100% reproducible. The
moment a user runs 'echo 1 /sys/power/enable_off_mode' the board
hangs without any further output.
Reverting the patch allows me to hit OFF mode again. I haven't yet
tested this on vanilla 2.6.29 or latest L-O.
OK, reverting this in both PM branches. Looks like a deadlock to me,
as update_resource_level() must cause a call to something else that
takes the mutex.
Good catch, I see the deadlock are in the change_level function
pointers. Where set_xxx will call resource_request() or
resource_release() = resource_lookup() which grabs the list mutex we
grabbed in resource_refresh().
Thoughts on changing the mutex to a RW semaphore?
-- Mike
Kevin
Pushed to PM branch and pm-2.6.29.
Kevin
---
arch/arm/plat-omap/resource.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/arch/arm/plat-omap/resource.c b/arch/arm/plat-omap/resource.c
index 25072cd..4631912 100644
--- a/arch/arm/plat-omap/resource.c
+++ b/arch/arm/plat-omap/resource.c
@@ -234,11 +234,13 @@ int resource_refresh(void)
struct shared_resource *resp = NULL;
int ret = 0;
+ down(res_mutex);
list_for_each_entry(resp, res_list, node) {
ret = update_resource_level(resp);
if (ret)
break;
}
+ up(res_mutex);
return ret;
}
--
1.5.4.5
--
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
--
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] omap: resource: Fix race in register_resource()
On Tue, Sep 8, 2009 at 3:39 PM, Kevin Hilmankhil...@deeprootsystems.com wrote:
Mike Chan m...@android.com writes:
Checking if the resource is already registered and adding to the list
must be atomic or bad things can happen.
Signed-off-by: Mike Chan m...@android.com
Functionally, this looks fine. Some procedural details still to work
out:
Are you the original author of this? or was it Chunqiu or Limei? The
changelog should be prefixed with a From: line of the original author
(handled by git-format-patch) if it was not you. Also add the other's
signoffs if appropriate.
I am the original author of this patch. Chunqiu sent two patches out
1) a per-resource level mutex (vs global).
2) Make update_resource_level() thread safe.
I've been meaning to send them out to l-o on behalf of Chunqiu.
What tree does this apply to? It's not specified, and does not apply
to either current PM branch or pm-2.6.29.
I thought I rebased this off of pm but I may have goofed. I can send a new one.
-- Mike
Kevin
---
arch/arm/plat-omap/resource.c | 14 +-
1 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/arch/arm/plat-omap/resource.c b/arch/arm/plat-omap/resource.c
index e9ace13..9d20249 100644
--- a/arch/arm/plat-omap/resource.c
+++ b/arch/arm/plat-omap/resource.c
@@ -253,6 +253,7 @@ int resource_refresh(void)
*/
int resource_register(struct shared_resource *resp)
{
+ int ret = 0;
if (!resp)
return -EINVAL;
@@ -260,13 +261,15 @@ int resource_register(struct shared_resource *resp)
return -EINVAL;
/* Verify that the resource is not already registered */
- if (resource_lookup(resp-name))
- return -EEXIST;
+ down(res_mutex);
+ if (_resource_lookup(resp-name)) {
+ ret = -EEXIST;
+ goto out;
+ }
INIT_LIST_HEAD(resp-users_list);
mutex_init(resp-resource_mutex);
- down(res_mutex);
/* Add the resource to the resource list */
list_add(resp-node, res_list);
@@ -274,10 +277,11 @@ int resource_register(struct shared_resource *resp)
if (resp-ops-init)
resp-ops-init(resp);
- up(res_mutex);
pr_debug(resource: registered %s\n, resp-name);
- return 0;
+out:
+ up(res_mutex);
+ return ret;
}
EXPORT_SYMBOL(resource_register);
--
1.5.4.5
--
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: Linux-omap PM: Fix dead lock condition in resource_release(vdd1_opp)
On Thu, Sep 3, 2009 at 7:01 AM, Kevin Hilmankhil...@deeprootsystems.com wrote:
Mike Chan m...@android.com writes:
On Tue, Aug 18, 2009 at 8:04 AM, Kevin
Hilmankhil...@deeprootsystems.com wrote:
Wang Limei-E12499 e12...@motorola.com writes:
Seems like I did not submit the patch in the right way, before I setup
my mail correctly, attach the patches in the mail.
You're patches are still line-wrapped.
I strongly recommend using git-format-patch and git-send-email to
submit patches. Chunqiu was able to do this. Please consult him.
Also, no need to CC linux-omap-owner. linux-omap is all that is needed.
This patch has been reviewed and merged into our android-omap-2.6.29 tree
http://android.git.kernel.org/?p=kernel/omap.git;a=commit;h=0b6a9b6514c7ccfa0c76e4defdaea3dcbc617633
Hmm, I don't see any other review/signoff that the authors on that
link.
Apologies, I was not aware of the reviewed-by policy but will keep
that in mind for future patches we pull into our branch. In general
any patches that have been applied to the android-omap-2.6.29 tree
have gone under some review/testing.
Kevin if you're having line wrap problems feel free to pull it from
here, assuming everyone's feedback has been addressed
It's not me who has the line-wrap problem. I could unwrap pretty
easily myself, but it gets very old working around various email
client problems, so I choose to reject patches until they can be sent
in a usable form.
I'm still waiting for this so it can get a full review on-list.
Very understandable, if Chunqiu is still having problems I can push
one out to l-o for review on behalf of Chinqiu. Its in our best
interest to get this into mainline so we have less 1-off's to
maintain.
--Mike
Thanks,
Kevin
Thanks,
Kevin
PATCH1:0001-Add-per-resource-mutex-for-OMAP-resource-framework.patch
From b4e9cc01f9d1aaeec39cc1ee794e5efaec61c781 Mon Sep 17 00:00:00 2001
From: Chunqiu Wang cqw...@motorola.com
Date: Fri, 14 Aug 2009 17:34:32 +0800
Subject: [PATCH] Add per-resource mutex for OMAP resource framework
Current OMAP resource fwk uses a global res_mutex
for resource_request and resource_release calls
for all the available resources.It may cause dead
lock if resource_request/resource_release is called
recursively.
For current OMAP3 VDD1/VDD2 resource, the change_level
implementation is mach-omap2/resource34xx.c/set_opp(),
when using resource_release to remove vdd1 constraint,
this function may call resource_release again to release
Vdd2 constrait if target vdd1 level is less than OPP3.
in this scenario, the global res_mutex down operation
will be called again, this will cause the second
down operation hang there.
To fix the problem, per-resource mutex is added
to avoid hangup when resource_request/resource_release
is called recursively.
Signed-off-by: Chunqiu Wang cqw...@motorola.com
---
arch/arm/plat-omap/include/mach/resource.h | 2 ++
arch/arm/plat-omap/resource.c | 27
+++
2 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/arch/arm/plat-omap/include/mach/resource.h
b/arch/arm/plat-omap/include/mach/resource.h
index f91d8ce..d482fb8 100644
--- a/arch/arm/plat-omap/include/mach/resource.h
+++ b/arch/arm/plat-omap/include/mach/resource.h
@@ -46,6 +46,8 @@ struct shared_resource {
/* Shared resource operations */
struct shared_resource_ops *ops;
struct list_head node;
+ /* Protect each resource */
+ struct mutex resource_mutex;
};
struct shared_resource_ops {
diff --git a/arch/arm/plat-omap/resource.c
b/arch/arm/plat-omap/resource.c
index ec31727..5eae4e8 100644
--- a/arch/arm/plat-omap/resource.c
+++ b/arch/arm/plat-omap/resource.c
@@ -264,6 +264,7 @@ int resource_register(struct shared_resource *resp)
return -EEXIST;
INIT_LIST_HEAD(resp-users_list);
+ mutex_init(resp-resource_mutex);
down(res_mutex);
/* Add the resource to the resource list */
@@ -326,14 +327,14 @@ int resource_request(const char *name, struct
device *dev,
struct users_list *user;
int found = 0, ret = 0;
- down(res_mutex);
- resp = _resource_lookup(name);
+ resp = resource_lookup(name);
if (!resp) {
printk(KERN_ERR resource_request: Invalid resource
name\n);
ret = -EINVAL;
- goto res_unlock;
+ goto ret;
}
+ mutex_lock(resp-resource_mutex);
/* Call the resource specific validate function */
if (resp-ops-validate_level) {
ret = resp-ops-validate_level(resp, level);
@@ -362,7 +363,7 @@ int resource_request(const char *name, struct device
*dev,
user-level = level;
res_unlock:
- up(res_mutex);
+ mutex_unlock(resp-resource_mutex);
/*
* Recompute and set the current level for the resource.
* NOTE: update_resource level moved out of spin_lock, as it may
[PATCH] omap: resource: Fix race in register_resource()
Checking if the resource is already registered and adding to the list
must be atomic or bad things can happen.
Signed-off-by: Mike Chan m...@android.com
---
arch/arm/plat-omap/resource.c | 14 +-
1 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/arch/arm/plat-omap/resource.c b/arch/arm/plat-omap/resource.c
index e9ace13..9d20249 100644
--- a/arch/arm/plat-omap/resource.c
+++ b/arch/arm/plat-omap/resource.c
@@ -253,6 +253,7 @@ int resource_refresh(void)
*/
int resource_register(struct shared_resource *resp)
{
+ int ret = 0;
if (!resp)
return -EINVAL;
@@ -260,13 +261,15 @@ int resource_register(struct shared_resource *resp)
return -EINVAL;
/* Verify that the resource is not already registered */
- if (resource_lookup(resp-name))
- return -EEXIST;
+ down(res_mutex);
+ if (_resource_lookup(resp-name)) {
+ ret = -EEXIST;
+ goto out;
+ }
INIT_LIST_HEAD(resp-users_list);
mutex_init(resp-resource_mutex);
- down(res_mutex);
/* Add the resource to the resource list */
list_add(resp-node, res_list);
@@ -274,10 +277,11 @@ int resource_register(struct shared_resource *resp)
if (resp-ops-init)
resp-ops-init(resp);
- up(res_mutex);
pr_debug(resource: registered %s\n, resp-name);
- return 0;
+out:
+ up(res_mutex);
+ return ret;
}
EXPORT_SYMBOL(resource_register);
--
1.5.4.5
--
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
[PATCH] [ARM] omap: resource: Make resource_refresh() thread safe.
Need to lock the res_mutex when traversing the res_list.
Signed-off-by: Mike Chan m...@android.com
---
arch/arm/plat-omap/resource.c |2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/arch/arm/plat-omap/resource.c b/arch/arm/plat-omap/resource.c
index 25072cd..4631912 100644
--- a/arch/arm/plat-omap/resource.c
+++ b/arch/arm/plat-omap/resource.c
@@ -234,11 +234,13 @@ int resource_refresh(void)
struct shared_resource *resp = NULL;
int ret = 0;
+ down(res_mutex);
list_for_each_entry(resp, res_list, node) {
ret = update_resource_level(resp);
if (ret)
break;
}
+ up(res_mutex);
return ret;
}
--
1.5.4.5
--
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: Linux-omap PM: Fix dead lock condition in resource_release(vdd1_opp)
On Tue, Aug 18, 2009 at 8:04 AM, Kevin
Hilmankhil...@deeprootsystems.com wrote:
Wang Limei-E12499 e12...@motorola.com writes:
Seems like I did not submit the patch in the right way, before I setup
my mail correctly, attach the patches in the mail.
You're patches are still line-wrapped.
I strongly recommend using git-format-patch and git-send-email to
submit patches. Chunqiu was able to do this. Please consult him.
Also, no need to CC linux-omap-owner. linux-omap is all that is needed.
This patch has been reviewed and merged into our android-omap-2.6.29 tree
http://android.git.kernel.org/?p=kernel/omap.git;a=commit;h=0b6a9b6514c7ccfa0c76e4defdaea3dcbc617633
Kevin if you're having line wrap problems feel free to pull it from
here, assuming everyone's feedback has been addressed
-- MIke
Thanks,
Kevin
PATCH1:0001-Add-per-resource-mutex-for-OMAP-resource-framework.patch
From b4e9cc01f9d1aaeec39cc1ee794e5efaec61c781 Mon Sep 17 00:00:00 2001
From: Chunqiu Wang cqw...@motorola.com
Date: Fri, 14 Aug 2009 17:34:32 +0800
Subject: [PATCH] Add per-resource mutex for OMAP resource framework
Current OMAP resource fwk uses a global res_mutex
for resource_request and resource_release calls
for all the available resources.It may cause dead
lock if resource_request/resource_release is called
recursively.
For current OMAP3 VDD1/VDD2 resource, the change_level
implementation is mach-omap2/resource34xx.c/set_opp(),
when using resource_release to remove vdd1 constraint,
this function may call resource_release again to release
Vdd2 constrait if target vdd1 level is less than OPP3.
in this scenario, the global res_mutex down operation
will be called again, this will cause the second
down operation hang there.
To fix the problem, per-resource mutex is added
to avoid hangup when resource_request/resource_release
is called recursively.
Signed-off-by: Chunqiu Wang cqw...@motorola.com
---
arch/arm/plat-omap/include/mach/resource.h | 2 ++
arch/arm/plat-omap/resource.c | 27
+++
2 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/arch/arm/plat-omap/include/mach/resource.h
b/arch/arm/plat-omap/include/mach/resource.h
index f91d8ce..d482fb8 100644
--- a/arch/arm/plat-omap/include/mach/resource.h
+++ b/arch/arm/plat-omap/include/mach/resource.h
@@ -46,6 +46,8 @@ struct shared_resource {
/* Shared resource operations */
struct shared_resource_ops *ops;
struct list_head node;
+ /* Protect each resource */
+ struct mutex resource_mutex;
};
struct shared_resource_ops {
diff --git a/arch/arm/plat-omap/resource.c
b/arch/arm/plat-omap/resource.c
index ec31727..5eae4e8 100644
--- a/arch/arm/plat-omap/resource.c
+++ b/arch/arm/plat-omap/resource.c
@@ -264,6 +264,7 @@ int resource_register(struct shared_resource *resp)
return -EEXIST;
INIT_LIST_HEAD(resp-users_list);
+ mutex_init(resp-resource_mutex);
down(res_mutex);
/* Add the resource to the resource list */
@@ -326,14 +327,14 @@ int resource_request(const char *name, struct
device *dev,
struct users_list *user;
int found = 0, ret = 0;
- down(res_mutex);
- resp = _resource_lookup(name);
+ resp = resource_lookup(name);
if (!resp) {
printk(KERN_ERR resource_request: Invalid resource
name\n);
ret = -EINVAL;
- goto res_unlock;
+ goto ret;
}
+ mutex_lock(resp-resource_mutex);
/* Call the resource specific validate function */
if (resp-ops-validate_level) {
ret = resp-ops-validate_level(resp, level);
@@ -362,7 +363,7 @@ int resource_request(const char *name, struct device
*dev,
user-level = level;
res_unlock:
- up(res_mutex);
+ mutex_unlock(resp-resource_mutex);
/*
* Recompute and set the current level for the resource.
* NOTE: update_resource level moved out of spin_lock, as it may
call
@@ -371,6 +372,7 @@ res_unlock:
*/
if (!ret)
ret = update_resource_level(resp);
+ret:
return ret;
}
EXPORT_SYMBOL(resource_request);
@@ -393,14 +395,14 @@ int resource_release(const char *name, struct
device *dev)
struct users_list *user;
int found = 0, ret = 0;
- down(res_mutex);
- resp = _resource_lookup(name);
+ resp = resource_lookup(name);
if (!resp) {
printk(KERN_ERR resource_release: Invalid resource
name\n);
ret = -EINVAL;
- goto res_unlock;
+ goto ret;
}
+ mutex_lock(resp-resource_mutex);
list_for_each_entry(user, resp-users_list, node) {
if (user-dev == dev) {
found = 1;
@@ -421,7 +423,9 @@ int resource_release(const char *name, struct device
*dev)
/* Recompute and set the current level for the resource */
ret =
Re: [PATCH] omap: pm: Fix overflow when doing powerdomain deps lookups.
On Tue, Aug 11, 2009 at 11:32 PM, Paul Walmsleyp...@pwsan.com wrote:
Hi Mike,
On Tue, 11 Aug 2009, Mike Chan wrote:
On Tue, Aug 11, 2009 at 7:38 AM, Kevin
Hilmankhil...@deeprootsystems.com wrote:
Mike Chan m...@android.com writes:
Signed-off-by: Mike Chan m...@android.com
---
arch/arm/mach-omap2/powerdomain.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-omap2/powerdomain.c
b/arch/arm/mach-omap2/powerdomain.c
index 0334609..6077629 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -90,7 +90,7 @@ static struct powerdomain *_pwrdm_deps_lookup(struct
powerdomain *pwrdm,
if (!pwrdm || !deps || !omap_chip_is(pwrdm-omap_chip))
return ERR_PTR(-EINVAL);
- for (pd = deps; pd; pd++) {
+ for (pd = deps; pd-pwrdm_name; pd++) {
Maybe should be:
for (pd = deps; pd pd-pwrdm_name; pd++) {
At the end of the list pd is a pointer to a NULL struct, so checking
if the address == NULL doesn't help here. In fact the original code
will just keep running past the struct to read who knows what in
memory.
This case manifests itself when from clkdms_setup() when enabling auto
idle for a clock domain and the clockdomain usecount is greater than
0. When _clkdm_add_autodeps() tries to add the a dependency that does
not exist in the powerdomain-wkdep_srcs array the for loop will run
past the wkdep_srcs array.
Currently in linux-omap you won't hit this because the not found case
is never executed, unless you start modifying powerdomains and their
wakeup/sleep deps.
--Mike
Thanks for the patch, this is correct. But the patch also should modify:
if (!pd)
return ERR_PTR(-ENOENT);
after the loop to be
if (!pd-pwrdm_name)
...
Could you revise this and resend?
thanks
Good catch, looks like this was already merged into the omap-pm branch :/
Sending a followup patch fine with everyone?
- Paul
--
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
[PATCH] omap: pm: Fix error condition in _pwrdm_deps_lookup when pwrdm not found.
Check pwrdm_name instead of the address of a null struct when at the end of pwrdm_dep array. Reported-by: Paul Walmsley p...@pwsan.com Signed-off-by: Mike Chan m...@android.com --- arch/arm/mach-omap2/powerdomain.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c index 0334609..02c1ef6 100644 --- a/arch/arm/mach-omap2/powerdomain.c +++ b/arch/arm/mach-omap2/powerdomain.c @@ -103,7 +103,7 @@ static struct powerdomain *_pwrdm_deps_lookup(struct powerdomain *pwrdm, } - if (!pd) + if (!pd-pwrdm_name) return ERR_PTR(-ENOENT); return pd-pwrdm; -- 1.5.4.5 -- 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] omap: pm: Fix overflow when doing powerdomain deps lookups.
On Tue, Aug 11, 2009 at 7:38 AM, Kevin
Hilmankhil...@deeprootsystems.com wrote:
Mike Chan m...@android.com writes:
Signed-off-by: Mike Chan m...@android.com
---
arch/arm/mach-omap2/powerdomain.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-omap2/powerdomain.c
b/arch/arm/mach-omap2/powerdomain.c
index 0334609..6077629 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -90,7 +90,7 @@ static struct powerdomain *_pwrdm_deps_lookup(struct
powerdomain *pwrdm,
if (!pwrdm || !deps || !omap_chip_is(pwrdm-omap_chip))
return ERR_PTR(-EINVAL);
- for (pd = deps; pd; pd++) {
+ for (pd = deps; pd-pwrdm_name; pd++) {
Maybe should be:
for (pd = deps; pd pd-pwrdm_name; pd++) {
At the end of the list pd is a pointer to a NULL struct, so checking
if the address == NULL doesn't help here. In fact the original code
will just keep running past the struct to read who knows what in
memory.
This case manifests itself when from clkdms_setup() when enabling auto
idle for a clock domain and the clockdomain usecount is greater than
0. When _clkdm_add_autodeps() tries to add the a dependency that does
not exist in the powerdomain-wkdep_srcs array the for loop will run
past the wkdep_srcs array.
Currently in linux-omap you won't hit this because the not found case
is never executed, unless you start modifying powerdomains and their
wakeup/sleep deps.
--Mike
?
if (!omap_chip_is(pd-omap_chip))
continue;
Also, a descriptive changelog would be helpful here describing the
conditions where you saw overflow etc.
Kevin
--
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
[PATCH] omap: pm: Fix overflow when doing powerdomain deps lookups.
Signed-off-by: Mike Chan m...@android.com
---
arch/arm/mach-omap2/powerdomain.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-omap2/powerdomain.c
b/arch/arm/mach-omap2/powerdomain.c
index 0334609..6077629 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -90,7 +90,7 @@ static struct powerdomain *_pwrdm_deps_lookup(struct
powerdomain *pwrdm,
if (!pwrdm || !deps || !omap_chip_is(pwrdm-omap_chip))
return ERR_PTR(-EINVAL);
- for (pd = deps; pd; pd++) {
+ for (pd = deps; pd-pwrdm_name; pd++) {
if (!omap_chip_is(pd-omap_chip))
continue;
--
1.5.4.5
--
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: drivers that require headers in mach-omap
Thanks for the input guys, Another case (currently not in omap-linux), if when the ohci-omap.c driver needs to check, val = cm_read_mod_reg(OMAP3430ES2_USBHOST_MOD, CM_IDLEST) if (val OMAP3430ES2_ST_USBHOST_STDBY_MASK) when putting the ohci bus into suspend (which I don't think the current ohci-omap.c currently supports) currently we just have a platform_data callback into the board file, but this really feels like omap specific and would be silly for every board file to implement this if they wanted ohci bus suspend. -- Mike On Mon, Aug 3, 2009 at 3:49 AM, Lohithakshan, Ranjithranji...@ti.com wrote: -Original Message- From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of Paul Walmsley Sent: Monday, August 03, 2009 5:04 AM To: Pandita, Vikram Cc: Mike Chan; Kevin Hilman; linux-omap@vger.kernel.org Subject: RE: drivers that require headers in mach-omap On Fri, 31 Jul 2009, Pandita, Vikram wrote: -Original Message- From: Mike Chan [mailto:m...@android.com] Sent: Thursday, July 30, 2009 6:20 PM To: Pandita, Vikram Cc: Kevin Hilman; linux-omap@vger.kernel.org Subject: Re: drivers that require headers in mach-omap On Thu, Jul 30, 2009 at 8:44 AM, Pandita, Vikramvikram.pand...@ti.com wrote: -Original Message- From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of Mike Chan Sent: Tuesday, July 28, 2009 8:49 PM To: Kevin Hilman; linux-omap@vger.kernel.org Subject: drivers that require headers in mach-omap Omap folks, how are drivers that require access to prm and cm registers via cm_read_mod_reg() etc... suppose to access these? For example if drivers/usb/host/ohci-omap.c wanted to call: cm_read_mod_reg(OMAP3430ES2_USBHOST_MOD, CM_IDLEST); The design was supposed to encapsulate the PRCM API's from drivers. Driver has control over the iclk and fclk and the clock framework would take care of any CM/PRM register settings. Accessing these registers in drivers would make the driver non-compatible for non-omap platforms. Are drivers such as drivers/usb/host/ohci-omap.c drivers/usb/musb/omap2430.c suppose to be compatible for non-omap platforms? As you put it, no they are not. All PRM/CM register accesses have been restricted to mach-omap2/plat-omap parts till now. The question to ask is, what functionality is missing on enabling say the usbhost clock that clock framework is not doing, that driver has a need to do. Just to follow up on this: the plan should be to remove all PRM and CM references from those drivers. Some of those can be replaced with clock framework calls, others may need platform_data function pointers. To add to the list, drivers/usb/host/ehci-omap.c need a similar re-work. At the moment, it does some amount of DPLL5 programming in itself before enabling the iclk and fclk. - Paul -- 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 -- 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: drivers that require headers in mach-omap
On Thu, Jul 30, 2009 at 8:44 AM, Pandita, Vikramvikram.pand...@ti.com wrote: -Original Message- From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of Mike Chan Sent: Tuesday, July 28, 2009 8:49 PM To: Kevin Hilman; linux-omap@vger.kernel.org Subject: drivers that require headers in mach-omap Omap folks, how are drivers that require access to prm and cm registers via cm_read_mod_reg() etc... suppose to access these? For example if drivers/usb/host/ohci-omap.c wanted to call: cm_read_mod_reg(OMAP3430ES2_USBHOST_MOD, CM_IDLEST); The design was supposed to encapsulate the PRCM API's from drivers. Driver has control over the iclk and fclk and the clock framework would take care of any CM/PRM register settings. Accessing these registers in drivers would make the driver non-compatible for non-omap platforms. Are drivers such as drivers/usb/host/ohci-omap.c drivers/usb/musb/omap2430.c suppose to be compatible for non-omap platforms? -- Mike It seems some of the headers in mach-omap2 should be in plat-omap/include/mach, or is there a more elegant way? The other alternatives are to register calls in all the board files, or #include ../../.. both do not seem very elegant to me. -- Mike -- 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 -- 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 2/2] OMAP3: PM: Configure CPUidle latencies/thresholds from board files
On Mon, Jun 29, 2009 at 7:35 AM, Rajendra Nayakrna...@ti.com wrote:
The CPUidle C state latencies and thresholds are dependent
on various board specific details.
Hence this patch makes it possible to configure these values from the
respective board files.
Signed-off-by: Rajendra Nayak rna...@ti.com
---
I think cpufreq could probably use something similar. The transition
latency set in the policy is set in cpu-omap.c and is a hard-coded
value. I wonder if its reasonable to piggyback off these values for
the transition latencies, or extend the struct to include those?
-- Mike
arch/arm/mach-omap2/board-3430sdp.c | 22 ++-
arch/arm/mach-omap2/board-apollon.c | 2 +-
arch/arm/mach-omap2/board-generic.c | 2 +-
arch/arm/mach-omap2/board-h4.c | 2 +-
arch/arm/mach-omap2/board-ldp.c | 2 +-
arch/arm/mach-omap2/board-omap3beagle.c | 2 +-
arch/arm/mach-omap2/board-omap3evm.c | 2 +-
arch/arm/mach-omap2/board-overo.c | 3 +-
arch/arm/mach-omap2/board-rx51.c | 2 +-
arch/arm/mach-omap2/board-zoom2.c | 2 +-
arch/arm/mach-omap2/cpuidle34xx.c | 105 --
arch/arm/mach-omap2/io.c | 5 +-
arch/arm/mach-omap2/pm.c | 7 ++-
arch/arm/mach-omap2/pm.h | 12 +++-
arch/arm/plat-omap/include/mach/io.h | 4 +-
15 files changed, 136 insertions(+), 38 deletions(-)
diff --git a/arch/arm/mach-omap2/board-3430sdp.c
b/arch/arm/mach-omap2/board-3430sdp.c
index b43cf94..750d841 100644
--- a/arch/arm/mach-omap2/board-3430sdp.c
+++ b/arch/arm/mach-omap2/board-3430sdp.c
@@ -75,6 +75,24 @@ static struct prm_setup_vc omap3_setuptime_table = {
.vdd1_off = 0x00,
};
+/* FIXME: These values need to be updated based on more profiling on
3430sdp*/
+static struct cpuidle_params omap3_cpuidle_params_table[] = {
+ /* C1 */
+ {2, 2, 5},
+ /* C2 */
+ {10, 10, 30},
+ /* C3 */
+ {50, 50, 300},
+ /* C4 */
+ {1500, 1800, 4000},
+ /* C5 */
+ {2500, 7500, 12000},
+ /* C6 */
+ {3000, 8500, 15000},
+ /* C7 */
+ {1, 3, 30},
+};
+
static int sdp3430_keymap[] = {
KEY(0, 0, KEY_LEFT),
KEY(0, 1, KEY_RIGHT),
@@ -191,8 +209,8 @@ static struct platform_device *sdp3430_devices[]
__initdata = {
static void __init omap_3430sdp_init_irq(void)
{
omap2_init_common_hw(hyb18m512160af6_sdrc_params, omap3_mpu_rate_table,
- omap3_dsp_rate_table, omap3_l3_rate_table,
- omap3_setuptime_table);
+ omap3_dsp_rate_table, omap3_l3_rate_table,
+ omap3_setuptime_table, omap3_cpuidle_params_table);
omap_init_irq();
omap_gpio_init();
}
diff --git a/arch/arm/mach-omap2/board-apollon.c
b/arch/arm/mach-omap2/board-apollon.c
index 293a9b2..fe625a9 100644
--- a/arch/arm/mach-omap2/board-apollon.c
+++ b/arch/arm/mach-omap2/board-apollon.c
@@ -250,7 +250,7 @@ out:
static void __init omap_apollon_init_irq(void)
{
- omap2_init_common_hw(NULL, NULL, NULL, NULL, NULL);
+ omap2_init_common_hw(NULL, NULL, NULL, NULL, NULL, NULL);
omap_init_irq();
omap_gpio_init();
apollon_init_smc91x();
diff --git a/arch/arm/mach-omap2/board-generic.c
b/arch/arm/mach-omap2/board-generic.c
index dfc1b49..ecf2c45 100644
--- a/arch/arm/mach-omap2/board-generic.c
+++ b/arch/arm/mach-omap2/board-generic.c
@@ -33,7 +33,7 @@
static void __init omap_generic_init_irq(void)
{
- omap2_init_common_hw(NULL, NULL, NULL, NULL, NULL);
+ omap2_init_common_hw(NULL, NULL, NULL, NULL, NULL, NULL);
omap_init_irq();
}
diff --git a/arch/arm/mach-omap2/board-h4.c b/arch/arm/mach-omap2/board-h4.c
index af0ebaf..365bc69 100644
--- a/arch/arm/mach-omap2/board-h4.c
+++ b/arch/arm/mach-omap2/board-h4.c
@@ -270,7 +270,7 @@ static void __init h4_init_flash(void)
static void __init omap_h4_init_irq(void)
{
- omap2_init_common_hw(NULL, NULL, NULL, NULL, NULL);
+ omap2_init_common_hw(NULL, NULL, NULL, NULL, NULL, NULL);
omap_init_irq();
omap_gpio_init();
h4_init_flash();
diff --git a/arch/arm/mach-omap2/board-ldp.c b/arch/arm/mach-omap2/board-ldp.c
index eb5e2e0..1265a12 100644
--- a/arch/arm/mach-omap2/board-ldp.c
+++ b/arch/arm/mach-omap2/board-ldp.c
@@ -270,7 +270,7 @@ static inline void __init ldp_init_smsc911x(void)
static void __init omap_ldp_init_irq(void)
{
- omap2_init_common_hw(NULL, NULL, NULL, NULL, NULL);
+ omap2_init_common_hw(NULL, NULL, NULL, NULL, NULL, NULL);
omap_init_irq();
omap_gpio_init();
ldp_init_smsc911x();
diff --git a/arch/arm/mach-omap2/board-omap3beagle.c
b/arch/arm/mach-omap2/board-omap3beagle.c
index e0be6bd..da1f3f6 100644
---
Re: [PATCH 1/2] Support OMAP3 VC adaptation with different Power IC
On Thu, Jun 25, 2009 at 1:12 PM, Wang Sawsd-A24013cqw...@motorola.com wrote:
diff --git a/arch/arm/mach-omap2/smartreflex.c
b/arch/arm/mach-omap2/smartreflex.c
index 9d462e3..bacf602 100644
--- a/arch/arm/mach-omap2/smartreflex.c
+++ b/arch/arm/mach-omap2/smartreflex.c
@@ -52,6 +52,8 @@ struct omap_sr {
#define SR_REGADDR(offs) (sr-srbase_addr + offset)
+static omap3_voltagescale_vcbypass_t omap3_volscale_vcbypass_fun;
Perhaps use
static int (*omap3_volscale_vcbypass_fun) (u32, u32, u8, u8);
This gets rid of the compiler warning on incompatible pointer
types when
you register a function via omap3_voltagescale_vcbypass_setup().
-- Mike
Thanks Mike, but we do not see the warnning with the original
Code, note this patch is slightly different with the one we sent
To you based on K29, this new patch have four parameters
Sinc some change has been made in the
sr_voltagescale_vcbypass to add two more parameters.
Is your warnning caused by this?
This patch stand-alone doesn't give me any warnings. Its only when you
hook into this from your board file. Might be the compiler I'm using.
I used this patch + modified version of the patches you sent me before
to get the two working. (Yes, 4 params).
I just verified the change on both current linux-omap pm branch and
The branch we are using, neither of them reports any warnning.
Just reminder, the patch I sent to you are different in all these files,
Since we should use 2 params in our kernel, the patch I sent to
This list has 4 params. Please check whether the warnning
Is caused by the mismatch in the function declaration and
The function definition in the board file.
It should not cause the warning even you are using different
Compiler.
Ah you're right, I must have done something wrong on my side. It was
probably the 2 vs 4 params.
-- Mike
-- MIke
Thanks,
Chunqiu
--
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] OMAP3: PM: SDRC: ensure mux of SDRC clock enable pins for self-refresh
On Thu, May 21, 2009 at 3:37 PM, Paul Walmsley p...@pwsan.com wrote:
Hi Kevin
On Wed, 20 May 2009, Kevin Hilman wrote:
diff --git a/arch/arm/mach-omap2/sdrc.c b/arch/arm/mach-omap2/sdrc.c
index c832d83..d7807e2 100644
--- a/arch/arm/mach-omap2/sdrc.c
+++ b/arch/arm/mach-omap2/sdrc.c
@@ -136,5 +136,13 @@ void __init omap2_sdrc_init(struct omap_sdrc_params
*sp)
(1 SDRC_POWER_PWDENA_SHIFT) |
(1 SDRC_POWER_PAGEPOLICY_SHIFT);
sdrc_write_reg(l, SDRC_POWER);
+
+ /* Ensure SDRC pins for both chip selcts are mux'd properly
+ * for self-refresh */
+ if (cpu_is_omap34xx()) {
+ omap_cfg_reg(H16_34XX_SDRC_CKE0);
+ omap_cfg_reg(H17_34XX_SDRC_CKE1);
+ }
+
omap2_sms_save_context();
}
would suggest keeping pin remuxing in board-*.c or maybe chip-*.c files;
ideally this file would only pertain to the SDRC IP block itself.
- Paul
Is there a reason why any board-*.c would not want this? It seems a
little silly for everyone to be doing the same repetitive pin muxing
if they want to suspend properly for a reasonable amount of time.
- Mike
--
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
