The following is a refresh of the PM_QOS implementation, this patch updates some documentation input I got from Randy. This patch changes the string based list management to a handle base implementation to help with the hot path use of pm-qos, it also renames much of the API to use "request" as opposed to "requirement" that was used in the initial implementation. I did this because request more accurately represents what it actually does. Also, I added a string based ABI for users wanting to use a string interface. So if the user writes 0xDDDDDDDD formatted hex it will be accepted by the interface. (someone asked me for it and I don't think it hurts anything.) I really would like to get this refresh taken care of. Its been taking me too long to close this. please review or include it in next. Thanks! --mgross Ooops! forgot the signed off by line! Signed-off-by: mark gross <mgross@linux.intel.com> From c45d8d86f89ac55fbb9a499fbc754e35258bf818 Mon Sep 17 00:00:00 2001 From: mgross <mark.gross@gmail.com> Date: Sat, 13 Mar 2010 08:18:36 -0800 Subject: [PATCH 1/2] PM_QOS to use handle based list implementation and exported function name changes to be more descriptive of what is actually happening. --- Documentation/power/pm_qos_interface.txt | 48 ++++--- drivers/acpi/processor_idle.c | 2 +- drivers/cpuidle/governors/ladder.c | 2 +- drivers/cpuidle/governors/menu.c | 2 +- drivers/net/e1000e/netdev.c | 22 ++-- drivers/net/igbvf/netdev.c | 6 +- drivers/net/wireless/ipw2x00/ipw2100.c | 11 +- include/linux/netdevice.h | 4 + include/linux/pm_qos_params.h | 14 +- include/sound/pcm.h | 3 +- kernel/pm_qos_params.c | 214 ++++++++++++++--------------- net/mac80211/mlme.c | 2 +- net/mac80211/scan.c | 2 +- sound/core/pcm.c | 3 - ...
Well, I'd take it to suspend-2.6/linux-next, but first, it touches subsystems whose maintainers were not in the Cc list, like the network drivers, wireless and ACPI. The changes are trivial, so I hope they don't mind. Second, my tree is based on the Linus' tree rather than linux-next and the change in net/mac80211/scan.c doesn't seem to match that. Please tell me what I'm supposed to do about that. Thanks, --
You can waite for monday and I'll send a rebased version to linus' tree. I thought linux-next was where folks wanted me to put it. I'll email out a new one monday. Thanks, --
Sorry I'm late, From: markgross <mark.gross@intel.com> Date: Mon, 3 May 2010 12:46:42 -0700 Subject: [PATCH] PM_QOS to use handle based list implementation and exported function name changes to be more descriptive of what is actually happening. --- Documentation/power/pm_qos_interface.txt | 48 ++++--- drivers/acpi/processor_idle.c | 2 +- drivers/cpuidle/governors/ladder.c | 2 +- drivers/cpuidle/governors/menu.c | 2 +- drivers/net/e1000e/netdev.c | 22 ++-- drivers/net/igbvf/netdev.c | 6 +- drivers/net/wireless/ipw2x00/ipw2100.c | 11 +- include/linux/netdevice.h | 4 + include/linux/pm_qos_params.h | 14 +- include/sound/pcm.h | 3 +- kernel/pm_qos_params.c | 214 ++++++++++++++--------------- net/mac80211/mlme.c | 2 +- sound/core/pcm.c | 3 - sound/core/pcm_native.c | 14 +- 14 files changed, 176 insertions(+), 171 deletions(-) diff --git a/Documentation/power/pm_qos_interface.txt b/Documentation/power/pm_qos_interface.txt index c40866e..bfed898 100644 --- a/Documentation/power/pm_qos_interface.txt +++ b/Documentation/power/pm_qos_interface.txt @@ -18,44 +18,46 @@ and pm_qos_params.h. This is done because having the available parameters being runtime configurable or changeable from a driver was seen as too easy to abuse. -For each parameter a list of performance requirements is maintained along with +For each parameter a list of performance requests is maintained along with an aggregated target value. The aggregated target value is updated with -changes to the requirement list or elements of the list. Typically the -aggregated target value is simply the max or min of the requirement values held +changes to the request list or elements of the list. Typically the +aggregated target value is simply the max or min of the request values ...
Applied to suspend-2.6/linux-next. Please verify the changelog. Rafael --
On Fri, 30 Apr 2010 14:20:43 -0700 Having taken a quick look, I think the API change makes a lot of sense. Hot paths are one thing; avoidance of accidental conflicts would be another. One question, though... one clear use of this API is for drivers to say "don't go into C3 or deeper because things go wrong"; I'm about to add another one of those. It works, but the use of a PM_QOS_CPU_DMA_LATENCY requirement with a hard-coded number that one hopes is small enough seems a bit...indirect. I wonder if it would be clearer and more robust to add a new requirement^Wrequest type saying "the quality of service I need is shallow sleeps only"? jon --
The problem with that is portability. What does "shallow" mean? Even between different SoCs in the same family, shallow might have different meaning, but across architectures, this certainly would not be portable. Kevin --
On Mon, 03 May 2010 09:40:11 -0700 Well, shallow could mean that the state lacks the CPUIDLE_FLAG_DEEP flag; that should be relatively portable. In any case, it seems more so than "if I put in a 55us latency requirement, I'll stay out of C3". Just a thought, anyway; it's not like I've really worked through a plausible alternative API. jon --
I guess it depends on your goal. Do you just want to stay out of C3 on your current platform? or do you want to stay out of any low-power state (on any platform) where you'll have a latency of > 55 usecs? The former is not portable (as C-states don't have the same meanings across arches/SoCs) where as using a real-world number in usecs will My main concern is that drivers not be written with latency constraints that assume an Intel-centric set of power states. There are other SoCs out there with different sets of states and corresponding latencies, so keeping things in real-world numbers (latency, throughput, etc.) seems to me to be the only portable way. Kevin --
On Mon, 03 May 2010 10:01:50 -0700 My case is a camera driver; as long as latencies are on the scale of the frame rate, things won't go too wrong. 1ms latencies would not be a huge problem. What happens is that the hardware corrupts the data if it goes into C3 while video capture is happening. Comments in drivers/net/e1000e/netdev.c suggest that they were dealing Understood. But the end result is that I've ended up specifying a latency requirement that is far tighter than the situation really needs Understood. Perhaps the pm_qos interface isn't the best way to achieve the real goal here, but it's all we have at the moment. Thanks, jon --
I'd say that one plausible requirement is "DMA works", with another being "all interrupts work" as distinguished from "wakeup interrupts work". -- Matthew Garrett | mjg59@srcf.ucam.org --
While C3 has maximum allowed latency (101usec, iirc), I believe there's no minimum, so someone could create platform with really quick C3 with perhaps only 10usec to enter... Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html --
Well, adding such an api to cpuidle could do this for you. "shallow" is hard to define, but one could do what google did and define a cpu_wake_lock type of API to block entry into any low power halt/mwait type of state. i.e. shallow would be C0, or just halt in idle. If such a thing fits a "hotpath" API extention to pm_qos basically implementing a low overhead disable lowpower behavior without having to walk the pm_qos lists to get an agrigate value. But, if such an api belongs in cpu_idle thats fine too, as the only user of such and API really is devices (like spi devices hangining of a clock that goes down in the deeper idle states) wanting to block low power idle states, so be it. --
