* Re: [PATCH v9 05/10] libata: separate ATAPI code
From: Tejun Heo @ 2012-11-12 18:57 UTC (permalink / raw)
To: Aaron Lu
Cc: Jeff Garzik, James Bottomley, Rafael J. Wysocki, Alan Stern,
Jeff Wu, Aaron Lu, linux-ide, linux-pm, linux-scsi, linux-acpi
In-Reply-To: <1352443922-13734-6-git-send-email-aaron.lu@intel.com>
On Fri, Nov 09, 2012 at 02:51:57PM +0800, Aaron Lu wrote:
> The atapi_eh_tur and atapi_eh_request_sense can be reused by ZPODD
> code, so separate them out to a file named libata-atapi.c, and the
> Makefile is modified accordingly. No functional changes should result
> from this commit.
Why is this change necessary? This doesn't seem to help anything to
me.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH v9 06/10] ata: zpodd: check zero power ready status
From: Tejun Heo @ 2012-11-12 19:13 UTC (permalink / raw)
To: Aaron Lu
Cc: Jeff Garzik, James Bottomley, Rafael J. Wysocki, Alan Stern,
Jeff Wu, Aaron Lu, linux-ide, linux-pm, linux-scsi, linux-acpi
In-Reply-To: <1352443922-13734-7-git-send-email-aaron.lu@intel.com>
Hello,
On Fri, Nov 09, 2012 at 02:51:58PM +0800, Aaron Lu wrote:
> @@ -784,7 +784,13 @@ static int ata_acpi_push_id(struct ata_device *dev)
> */
> int ata_acpi_on_suspend(struct ata_port *ap)
> {
> - /* nada */
> + struct ata_device *dev;
> +
> + ata_for_each_dev(dev, &ap->link, ENABLED) {
> + if (zpodd_dev_enabled(dev))
> + zpodd_check_zpready(dev);
> + }
> +
Why is it running off ata_acpi_on_suspend() instead of hooking
directly into EH suspend routine?
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index e3bda07..6f235b9 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -2665,6 +2665,10 @@ static void atapi_qc_complete(struct ata_queued_cmd *qc)
> ata_scsi_rbuf_put(cmd, true, &flags);
> }
>
> + if (zpodd_dev_enabled(qc->dev) &&
> + scsicmd[0] == GET_EVENT_STATUS_NOTIFICATION)
> + zpodd_snoop_status(qc->dev, cmd);
> +
Brief comment explaining what's going on here wouldn't hurt.
> +#define POWEROFF_DELAY (30 * 1000) /* 30 seconds for power off delay */
> +
> struct zpodd {
> bool slot:1;
> bool drawer:1;
> bool from_notify:1; /* resumed as a result of acpi notification */
> + bool status_ready:1; /* ready status derived from media event poll,
> + it is not accurate, but serves as a hint */
> + bool zp_ready:1; /* zero power ready state */
> +
> + unsigned long last_ready; /* last zero power ready timestamp */
>
> struct ata_device *dev;
> };
How are accesses to the bit fields synchronized?
> +/*
> + * Snoop the result of GET_STATUS_NOTIFICATION_EVENT, the media
> + * status byte has information on media present/door closed.
> + *
> + * This information serves only as a hint, as it is not accurate.
> + * The sense code method will be used when deciding if the ODD is
> + * really zero power ready.
> + */
It would be great if you can make the above a proper dockbook function
comment. Also, as the snooping for hint thing isn't too obvious it
would be great if the comment contains more info which is explained in
the commit message.
> +void zpodd_snoop_status(struct ata_device *dev, struct scsi_cmnd *scmd)
> +{
> + bool ready;
> + char buf[8];
> + struct event_header *eh = (void *)buf;
> + struct media_event_desc *med = (void *)(buf + 4);
> + struct sg_table *table = &scmd->sdb.table;
> + struct zpodd *zpodd = dev->private_data;
Don't people usually put variables definitions w/ assignments above
the ones without?
> +/*
> + * Check ODD's zero power ready status.
> + *
> + * This function is called during ATA port's suspend path,
> + * when the port is not frozen yet, so that we can still make
> + * some IO to the ODD to decide if it is zero power ready.
> + *
> + * The ODD is regarded as zero power ready when it is in zero
> + * power ready state for some time(defined by POWEROFF_DELAY).
> + */
> +void zpodd_check_zpready(struct ata_device *dev)
> +{
> + bool zp_ready;
> + unsigned long expires;
> + struct zpodd *zpodd = dev->private_data;
> +
> + if (!zpodd->status_ready) {
> + zpodd->last_ready = 0;
> + return;
> + }
> +
> + if (!zpodd->last_ready) {
> + zp_ready = zpready(dev);
> + if (zp_ready)
> + zpodd->last_ready = jiffies;
> + return;
> + }
> +
> + expires = zpodd->last_ready + msecs_to_jiffies(POWEROFF_DELAY);
> + if (time_before(jiffies, expires))
> + return;
> +
> + zpodd->zp_ready = zpready(dev);
> + if (!zpodd->zp_ready)
> + zpodd->last_ready = 0;
> +}
Hmmm... so, the "full" check only happens when autopm kicks in, right?
Is it really worth avoiding an extra TUR on autopm events? That's not
really a hot path. It seems a bit over-engineered to me.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH v9 07/10] block: add a new interface to block events
From: Tejun Heo @ 2012-11-12 19:14 UTC (permalink / raw)
To: Aaron Lu
Cc: Jeff Garzik, James Bottomley, Rafael J. Wysocki, Alan Stern,
Jeff Wu, Aaron Lu, linux-ide, linux-pm, linux-scsi, linux-acpi
In-Reply-To: <1352443922-13734-8-git-send-email-aaron.lu@intel.com>
On Fri, Nov 09, 2012 at 02:51:59PM +0800, Aaron Lu wrote:
> A new interface to block disk events is added, this interface is
> meant to eliminate a race between PM runtime callback and disk events
> checking.
>
> Suppose the following device tree:
> device_sata_port (the parent)
> device_ODD (the child)
Weren't you gonna do something different about this? I mean, if sr
knows that autopm kicked in, it can simply tell the block layer that
nothing is going on. Wouldn't that be simpler?
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH v9 07/10] block: add a new interface to block events
From: Alan Stern @ 2012-11-12 19:18 UTC (permalink / raw)
To: Tejun Heo
Cc: Aaron Lu, Jeff Garzik, James Bottomley, Rafael J. Wysocki,
Jeff Wu, Aaron Lu, linux-ide, linux-pm, linux-scsi, linux-acpi
In-Reply-To: <20121112191440.GF5560@mtj.dyndns.org>
On Mon, 12 Nov 2012, Tejun Heo wrote:
> On Fri, Nov 09, 2012 at 02:51:59PM +0800, Aaron Lu wrote:
> > A new interface to block disk events is added, this interface is
> > meant to eliminate a race between PM runtime callback and disk events
> > checking.
> >
> > Suppose the following device tree:
> > device_sata_port (the parent)
> > device_ODD (the child)
>
> Weren't you gonna do something different about this? I mean, if sr
> knows that autopm kicked in, it can simply tell the block layer that
> nothing is going on. Wouldn't that be simpler?
It wouldn't work for non-ZP drives. They do need to be polled, even
when suspended.
Alan Stern
^ permalink raw reply
* Re: [PATCH v9 07/10] block: add a new interface to block events
From: Tejun Heo @ 2012-11-12 19:21 UTC (permalink / raw)
To: Alan Stern
Cc: Aaron Lu, Jeff Garzik, James Bottomley, Rafael J. Wysocki,
Jeff Wu, Aaron Lu, linux-ide, linux-pm, linux-scsi, linux-acpi
In-Reply-To: <Pine.LNX.4.44L0.1211121417230.31816-100000@netrider.rowland.org>
Hello, Alan.
On Mon, Nov 12, 2012 at 02:18:11PM -0500, Alan Stern wrote:
> > Weren't you gonna do something different about this? I mean, if sr
> > knows that autopm kicked in, it can simply tell the block layer that
> > nothing is going on. Wouldn't that be simpler?
>
> It wouldn't work for non-ZP drives. They do need to be polled, even
> when suspended.
Hmmm... a bit confused, what would autopm do for those non-ZP devices?
Would it make any difference?
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH v9 07/10] block: add a new interface to block events
From: Alan Stern @ 2012-11-12 19:34 UTC (permalink / raw)
To: Tejun Heo
Cc: Aaron Lu, Jeff Garzik, James Bottomley, Rafael J. Wysocki,
Jeff Wu, Aaron Lu, linux-ide, linux-pm, linux-scsi, linux-acpi
In-Reply-To: <20121112192102.GG5560@mtj.dyndns.org>
On Mon, 12 Nov 2012, Tejun Heo wrote:
> Hello, Alan.
>
> On Mon, Nov 12, 2012 at 02:18:11PM -0500, Alan Stern wrote:
> > > Weren't you gonna do something different about this? I mean, if sr
> > > knows that autopm kicked in, it can simply tell the block layer that
> > > nothing is going on. Wouldn't that be simpler?
> >
> > It wouldn't work for non-ZP drives. They do need to be polled, even
> > when suspended.
>
> Hmmm... a bit confused, what would autopm do for those non-ZP devices?
> Would it make any difference?
It wouldn't do anything for the device, but it would allow the device's
parent to go to low power in between polls. That's why Aaron at one
point suggested lengthening the polling interval.
Alan Stern
^ permalink raw reply
* [RFC] cpuidle - remove the power_specified field in the driver
From: Daniel Lezcano @ 2012-11-12 20:26 UTC (permalink / raw)
To: linux-pm
Cc: jwerner, khilman, len.brown, g.trinabh, deepthi, linux-kernel,
rjw, akpm, snanda, linaro-dev
In-Reply-To: <50831CA3.2020602@linaro.org>
This patch follows the discussion about reinitializing the power usage
when a C-state is added/removed.
https://lkml.org/lkml/2012/10/16/518
We realized the power usage field is never filled and when it is
filled for tegra, the power_specified flag is not set making all these
values to be resetted when the driver is initialized with the set_power_state
function.
Julius and I feel this is over-engineered and the power_specified
flag could be simply removed and continue assuming the states are
backward sorted.
The menu governor select function is simplified as the power is ordered.
Actually the condition is always true with the current code.
The cpuidle_play_dead function is also simplified by doing a reverse lookup
on the array.
The set_power_states function is removed as it does no make sense anymore.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
drivers/cpuidle/cpuidle.c | 17 ++++-------------
drivers/cpuidle/driver.c | 25 -------------------------
| 8 ++------
include/linux/cpuidle.h | 2 +-
4 files changed, 7 insertions(+), 45 deletions(-)
diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index 711dd83..f983262 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -69,24 +69,15 @@ int cpuidle_play_dead(void)
{
struct cpuidle_device *dev = __this_cpu_read(cpuidle_devices);
struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev);
- int i, dead_state = -1;
- int power_usage = -1;
+ int i;
if (!drv)
return -ENODEV;
/* Find lowest-power state that supports long-term idle */
- for (i = CPUIDLE_DRIVER_STATE_START; i < drv->state_count; i++) {
- struct cpuidle_state *s = &drv->states[i];
-
- if (s->power_usage < power_usage && s->enter_dead) {
- power_usage = s->power_usage;
- dead_state = i;
- }
- }
-
- if (dead_state != -1)
- return drv->states[dead_state].enter_dead(dev, dead_state);
+ for (i = drv->state_count; i >= CPUIDLE_DRIVER_STATE_START; i--)
+ if (drv->states[i].play_dead)
+ return drv->states[i].enter_dead(dev, i);
return -ENODEV;
}
diff --git a/drivers/cpuidle/driver.c b/drivers/cpuidle/driver.c
index 3af841f..bb045f1 100644
--- a/drivers/cpuidle/driver.c
+++ b/drivers/cpuidle/driver.c
@@ -19,34 +19,9 @@ DEFINE_SPINLOCK(cpuidle_driver_lock);
static void __cpuidle_set_cpu_driver(struct cpuidle_driver *drv, int cpu);
static struct cpuidle_driver * __cpuidle_get_cpu_driver(int cpu);
-static void set_power_states(struct cpuidle_driver *drv)
-{
- int i;
-
- /*
- * cpuidle driver should set the drv->power_specified bit
- * before registering if the driver provides
- * power_usage numbers.
- *
- * If power_specified is not set,
- * we fill in power_usage with decreasing values as the
- * cpuidle code has an implicit assumption that state Cn
- * uses less power than C(n-1).
- *
- * With CONFIG_ARCH_HAS_CPU_RELAX, C0 is already assigned
- * an power value of -1. So we use -2, -3, etc, for other
- * c-states.
- */
- for (i = CPUIDLE_DRIVER_STATE_START; i < drv->state_count; i++)
- drv->states[i].power_usage = -1 - i;
-}
-
static void __cpuidle_driver_init(struct cpuidle_driver *drv)
{
drv->refcnt = 0;
-
- if (!drv->power_specified)
- set_power_states(drv);
}
static int __cpuidle_register_driver(struct cpuidle_driver *drv, int cpu)
--git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c
index 2efee27..14eb11f 100644
--- a/drivers/cpuidle/governors/menu.c
+++ b/drivers/cpuidle/governors/menu.c
@@ -312,7 +312,6 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
{
struct menu_device *data = &__get_cpu_var(menu_devices);
int latency_req = pm_qos_request(PM_QOS_CPU_DMA_LATENCY);
- int power_usage = -1;
int i;
int multiplier;
struct timespec t;
@@ -383,11 +382,8 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
if (s->exit_latency * multiplier > data->predicted_us)
continue;
- if (s->power_usage < power_usage) {
- power_usage = s->power_usage;
- data->last_state_idx = i;
- data->exit_us = s->exit_latency;
- }
+ data->last_state_idx = i;
+ data->exit_us = s->exit_latency;
}
/* not deepest C-state chosen for low predicted residency */
diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index 3711b34..24cd103 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -126,9 +126,9 @@ struct cpuidle_driver {
struct module *owner;
int refcnt;
- unsigned int power_specified:1;
/* set to 1 to use the core cpuidle time keeping (for all states). */
unsigned int en_core_tk_irqen:1;
+ /* states array must be ordered in decreasing power consumption */
struct cpuidle_state states[CPUIDLE_STATE_MAX];
int state_count;
int safe_state_index;
--
1.7.5.4
^ permalink raw reply related
* Re: [PATCH V5 2/2] Thermal: Add ST-Ericsson DB8500 thermal properties and platform data.
From: Linus Walleij @ 2012-11-12 20:46 UTC (permalink / raw)
To: Zhang Rui
Cc: hongbo.zhang, linux-acpi, linux-kernel, linux-pm, amit.kachhap,
patches, linaro-dev, linaro-kernel, STEricsson_nomadik_linux,
kernel, hongbo.zhang
In-Reply-To: <1352700425.1834.11.camel@rzhang1-mobl4>
On Mon, Nov 12, 2012 at 7:07 AM, Zhang Rui <rui.zhang@intel.com> wrote:
> On Fri, 2012-11-09 at 19:29 +0800, hongbo.zhang wrote:
>> From: "hongbo.zhang" <hongbo.zhang@linaro.com>
>>
>> This patch adds device tree properties for ST-Ericsson DB8500 thermal driver,
>> also adds the platform data to support the old fashion.
>>
>> Signed-off-by: hongbo.zhang <hongbo.zhang@linaro.com>
>> Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
>
> hmmm,
>
> who should take this patch?
> I'd like to see ACK from the maintainer of these code before applying
> them to thermal tree.
I guess it needs to go through Thermal.
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Let's hope it's not creating too much conflict...
Yours,
Linus Walleij
^ permalink raw reply
* Re: [RFD][PATCH 7/7] PM / ACPI: Take device PM QoS flags into account
From: Bjorn Helgaas @ 2012-11-12 21:07 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Linux PM list, ACPI Devel Mailing List, Alan Stern, Huang Ying,
Sarah Sharp, Lan Tianyu, Aaron Lu, Jean Pihet, linux-pci,
Greg Kroah-Hartman, mark gross
In-Reply-To: <201209282356.52558.rjw@sisk.pl>
On Fri, Sep 28, 2012 at 3:56 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> Make ACPI power management routines and PCI power management
> routines depending on ACPI take device PM QoS flags into account
> when deciding what power state to put the device into.
>
> In particular, after this change acpi_pm_device_sleep_state() will
> not return ACPI_STATE_D3_COLD as the deepest available low-power
> state if PM_QOS_FLAG_NO_POWER_OFF is requested for the device and it
> will not require remote wakeup to work for the device in the returned
> low-power state if there is at least one PM QoS flags request for the
> device, but PM_QOS_FLAG_REMOTE_WAKEUP is not requested for it.
>
> Accordingly, acpi_pci_set_power_state() will refuse to put the
> device into D3cold if PM_QOS_FLAG_NO_POWER_OFF is requested for it.
>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
> drivers/acpi/sleep.c | 16 ++++++++++++----
> drivers/pci/pci-acpi.c | 8 +++++++-
This touches a file in drivers/pci, but I expect it depends on all the
previous patches in the series, so if you haven't merged it already,
Rafael, it seems like it makes sense for you to merge this instead of
me.
Bjorn
^ permalink raw reply
* Re: [RFC] cpuidle - remove the power_specified field in the driver
From: Julius Werner @ 2012-11-12 21:09 UTC (permalink / raw)
To: Daniel Lezcano
Cc: linux-pm, khilman, len.brown, Trinabh Gupta, deepthi,
linux-kernel, rjw, akpm, Sameer Nanda, Lists Linaro-dev
In-Reply-To: <1352752016-3136-1-git-send-email-daniel.lezcano@linaro.org>
Thanks for moving this along, Daniel. I think this is the right
approach... the cpuidle driver shouldn't be more complex than
necessary.
Note that you are starting your loop too high in cpuidle_play_dead...
states[state_count] is not an actual state anymore, it should start at
state_count - 1. Also, I think you can go ahead and do the same
last-to-first loop transformation with immediate return in the menu
governor, for an extra tiny bit of performance.
On Mon, Nov 12, 2012 at 12:26 PM, Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
> This patch follows the discussion about reinitializing the power usage
> when a C-state is added/removed.
>
> https://lkml.org/lkml/2012/10/16/518
>
> We realized the power usage field is never filled and when it is
> filled for tegra, the power_specified flag is not set making all these
> values to be resetted when the driver is initialized with the set_power_state
> function.
>
> Julius and I feel this is over-engineered and the power_specified
> flag could be simply removed and continue assuming the states are
> backward sorted.
>
> The menu governor select function is simplified as the power is ordered.
> Actually the condition is always true with the current code.
>
> The cpuidle_play_dead function is also simplified by doing a reverse lookup
> on the array.
>
> The set_power_states function is removed as it does no make sense anymore.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
> drivers/cpuidle/cpuidle.c | 17 ++++-------------
> drivers/cpuidle/driver.c | 25 -------------------------
> drivers/cpuidle/governors/menu.c | 8 ++------
> include/linux/cpuidle.h | 2 +-
> 4 files changed, 7 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 711dd83..f983262 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -69,24 +69,15 @@ int cpuidle_play_dead(void)
> {
> struct cpuidle_device *dev = __this_cpu_read(cpuidle_devices);
> struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev);
> - int i, dead_state = -1;
> - int power_usage = -1;
> + int i;
>
> if (!drv)
> return -ENODEV;
>
> /* Find lowest-power state that supports long-term idle */
> - for (i = CPUIDLE_DRIVER_STATE_START; i < drv->state_count; i++) {
> - struct cpuidle_state *s = &drv->states[i];
> -
> - if (s->power_usage < power_usage && s->enter_dead) {
> - power_usage = s->power_usage;
> - dead_state = i;
> - }
> - }
> -
> - if (dead_state != -1)
> - return drv->states[dead_state].enter_dead(dev, dead_state);
> + for (i = drv->state_count; i >= CPUIDLE_DRIVER_STATE_START; i--)
> + if (drv->states[i].play_dead)
> + return drv->states[i].enter_dead(dev, i);
>
> return -ENODEV;
> }
> diff --git a/drivers/cpuidle/driver.c b/drivers/cpuidle/driver.c
> index 3af841f..bb045f1 100644
> --- a/drivers/cpuidle/driver.c
> +++ b/drivers/cpuidle/driver.c
> @@ -19,34 +19,9 @@ DEFINE_SPINLOCK(cpuidle_driver_lock);
> static void __cpuidle_set_cpu_driver(struct cpuidle_driver *drv, int cpu);
> static struct cpuidle_driver * __cpuidle_get_cpu_driver(int cpu);
>
> -static void set_power_states(struct cpuidle_driver *drv)
> -{
> - int i;
> -
> - /*
> - * cpuidle driver should set the drv->power_specified bit
> - * before registering if the driver provides
> - * power_usage numbers.
> - *
> - * If power_specified is not set,
> - * we fill in power_usage with decreasing values as the
> - * cpuidle code has an implicit assumption that state Cn
> - * uses less power than C(n-1).
> - *
> - * With CONFIG_ARCH_HAS_CPU_RELAX, C0 is already assigned
> - * an power value of -1. So we use -2, -3, etc, for other
> - * c-states.
> - */
> - for (i = CPUIDLE_DRIVER_STATE_START; i < drv->state_count; i++)
> - drv->states[i].power_usage = -1 - i;
> -}
> -
> static void __cpuidle_driver_init(struct cpuidle_driver *drv)
> {
> drv->refcnt = 0;
> -
> - if (!drv->power_specified)
> - set_power_states(drv);
> }
>
> static int __cpuidle_register_driver(struct cpuidle_driver *drv, int cpu)
> diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c
> index 2efee27..14eb11f 100644
> --- a/drivers/cpuidle/governors/menu.c
> +++ b/drivers/cpuidle/governors/menu.c
> @@ -312,7 +312,6 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
> {
> struct menu_device *data = &__get_cpu_var(menu_devices);
> int latency_req = pm_qos_request(PM_QOS_CPU_DMA_LATENCY);
> - int power_usage = -1;
> int i;
> int multiplier;
> struct timespec t;
> @@ -383,11 +382,8 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
> if (s->exit_latency * multiplier > data->predicted_us)
> continue;
>
> - if (s->power_usage < power_usage) {
> - power_usage = s->power_usage;
> - data->last_state_idx = i;
> - data->exit_us = s->exit_latency;
> - }
> + data->last_state_idx = i;
> + data->exit_us = s->exit_latency;
> }
>
> /* not deepest C-state chosen for low predicted residency */
> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
> index 3711b34..24cd103 100644
> --- a/include/linux/cpuidle.h
> +++ b/include/linux/cpuidle.h
> @@ -126,9 +126,9 @@ struct cpuidle_driver {
> struct module *owner;
> int refcnt;
>
> - unsigned int power_specified:1;
> /* set to 1 to use the core cpuidle time keeping (for all states). */
> unsigned int en_core_tk_irqen:1;
> + /* states array must be ordered in decreasing power consumption */
> struct cpuidle_state states[CPUIDLE_STATE_MAX];
> int state_count;
> int safe_state_index;
> --
> 1.7.5.4
>
^ permalink raw reply
* Re: [PATCH 2/6 v4] clk, highbank: Prevent glitches in non-bypass reset mode
From: Mike Turquette @ 2012-11-12 21:24 UTC (permalink / raw)
To: linux-kernel, cpufreq, linux-pm; +Cc: Mark Langsdorf, Rob Herring
In-Reply-To: <1352313166-28980-3-git-send-email-mark.langsdorf@calxeda.com>
Quoting Mark Langsdorf (2012-11-07 10:32:42)
> The highbank clock will glitch with the current code if the
> clock rate is reset without relocking the PLL. Program the PLL
> correctly to preven glitches.
>
> Signed-off-by: Mark Langsdorf <mark.langsdorf@calxeda.com>
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> Cc: mturquette@linaro.org
Hi Mark,
Looks fine to me.
I seem to be missing the rest of this series in my mail. Did you want
me to take only this patch (2/6) into clk-next or were you only looking
for my ACK?
Regards,
Mike
> ---
> Changes from v3
> Changelog text and patch name now correspond to the actual patch
> was clk, highbank: remove non-bypass reset mode
> Changes from v2
> None
> Changes from v1:
> Removed erroneous reformating.
>
> drivers/clk/clk-highbank.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/clk/clk-highbank.c b/drivers/clk/clk-highbank.c
> index 52fecad..3a0b723 100644
> --- a/drivers/clk/clk-highbank.c
> +++ b/drivers/clk/clk-highbank.c
> @@ -182,8 +182,10 @@ static int clk_pll_set_rate(struct clk_hw *hwclk, unsigned long rate,
> reg |= HB_PLL_EXT_ENA;
> reg &= ~HB_PLL_EXT_BYPASS;
> } else {
> + writel(reg | HB_PLL_EXT_BYPASS, hbclk->reg);
> reg &= ~HB_PLL_DIVQ_MASK;
> reg |= divq << HB_PLL_DIVQ_SHIFT;
> + writel(reg | HB_PLL_EXT_BYPASS, hbclk->reg);
> }
> writel(reg, hbclk->reg);
>
> --
> 1.7.11.7
^ permalink raw reply
* Re: [PATCH 2/6 v4] clk, highbank: Prevent glitches in non-bypass reset mode
From: Mark Langsdorf @ 2012-11-12 21:35 UTC (permalink / raw)
To: Mike Turquette
Cc: linux-kernel@vger.kernel.org, cpufreq@vger.kernel.org,
linux-pm@vger.kernel.org, Rob Herring
In-Reply-To: <20121112212412.20034.37835@nucleus>
On 11/12/2012 03:24 PM, Mike Turquette wrote:
> Quoting Mark Langsdorf (2012-11-07 10:32:42)
>> The highbank clock will glitch with the current code if the
>> clock rate is reset without relocking the PLL. Program the PLL
>> correctly to preven glitches.
>>
>> Signed-off-by: Mark Langsdorf <mark.langsdorf@calxeda.com>
>> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
>> Cc: mturquette@linaro.org
>
> Hi Mark,
>
> Looks fine to me.
>
> I seem to be missing the rest of this series in my mail. Did you want
> me to take only this patch (2/6) into clk-next or were you only looking
> for my ACK?
The entire series enables highbank cpufreq. Would you normally take this
patch through cpufreq-next with an ACK or directly through clk-next?
--Mark Langsdorf
Calxeda, Inc.
>> ---
>> Changes from v3
>> Changelog text and patch name now correspond to the actual patch
>> was clk, highbank: remove non-bypass reset mode
>> Changes from v2
>> None
>> Changes from v1:
>> Removed erroneous reformating.
>>
>> drivers/clk/clk-highbank.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/clk/clk-highbank.c b/drivers/clk/clk-highbank.c
>> index 52fecad..3a0b723 100644
>> --- a/drivers/clk/clk-highbank.c
>> +++ b/drivers/clk/clk-highbank.c
>> @@ -182,8 +182,10 @@ static int clk_pll_set_rate(struct clk_hw *hwclk, unsigned long rate,
>> reg |= HB_PLL_EXT_ENA;
>> reg &= ~HB_PLL_EXT_BYPASS;
>> } else {
>> + writel(reg | HB_PLL_EXT_BYPASS, hbclk->reg);
>> reg &= ~HB_PLL_DIVQ_MASK;
>> reg |= divq << HB_PLL_DIVQ_SHIFT;
>> + writel(reg | HB_PLL_EXT_BYPASS, hbclk->reg);
>> }
>> writel(reg, hbclk->reg);
>>
>> --
>> 1.7.11.7
^ permalink raw reply
* [PATCH 1/3] tick: export nohz tick idle symbols for module use
From: Jacob Pan @ 2012-11-12 22:03 UTC (permalink / raw)
To: Linux PM, LKML
Cc: Rafael Wysocki, Len Brown, Thomas Gleixner, H. Peter Anvin,
Ingo Molnar, Zhang Rui, Rob Landley, Arjan van de Ven,
Paul McKenney, Jacob Pan
In-Reply-To: <1352757831-5202-1-git-send-email-jacob.jun.pan@linux.intel.com>
Allow drivers such as intel_powerclamp to use these apis for
turning on/off ticks during idle.
Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
---
kernel/time/tick-sched.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index a402608..7c38f08 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -510,6 +510,7 @@ void tick_nohz_idle_enter(void)
local_irq_enable();
}
+EXPORT_SYMBOL_GPL(tick_nohz_idle_enter);
/**
* tick_nohz_irq_exit - update next tick event from interrupt exit
@@ -634,6 +635,7 @@ void tick_nohz_idle_exit(void)
local_irq_enable();
}
+EXPORT_SYMBOL_GPL(tick_nohz_idle_exit);
static int tick_nohz_reprogram(struct tick_sched *ts, ktime_t now)
{
--
1.7.9.5
^ permalink raw reply related
* [PATCH 2/3] x86/nmi: export local_touch_nmi() symbol for modules
From: Jacob Pan @ 2012-11-12 22:03 UTC (permalink / raw)
To: Linux PM, LKML
Cc: Rafael Wysocki, Len Brown, Thomas Gleixner, H. Peter Anvin,
Ingo Molnar, Zhang Rui, Rob Landley, Arjan van de Ven,
Paul McKenney, Jacob Pan
In-Reply-To: <1352757831-5202-1-git-send-email-jacob.jun.pan@linux.intel.com>
Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
---
arch/x86/kernel/nmi.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c
index f84f5c5..6030805 100644
--- a/arch/x86/kernel/nmi.c
+++ b/arch/x86/kernel/nmi.c
@@ -509,3 +509,4 @@ void local_touch_nmi(void)
{
__this_cpu_write(last_nmi_rip, 0);
}
+EXPORT_SYMBOL_GPL(local_touch_nmi);
--
1.7.9.5
^ permalink raw reply related
* [PATCH 3/3] PM: Introduce Intel PowerClamp Driver
From: Jacob Pan @ 2012-11-12 22:03 UTC (permalink / raw)
To: Linux PM, LKML
Cc: Rafael Wysocki, Len Brown, Thomas Gleixner, H. Peter Anvin,
Ingo Molnar, Zhang Rui, Rob Landley, Arjan van de Ven,
Paul McKenney, Jacob Pan
In-Reply-To: <1352757831-5202-1-git-send-email-jacob.jun.pan@linux.intel.com>
Intel PowerClamp driver performs synchronized idle injection across
all online CPUs. The goal is to maintain a given package level C-state
ratio.
Compared to other throttling methods already exist in the kernel,
such as ACPI PAD (taking CPUs offline) and clock modulation, this is often
more efficient in terms of performance per watt.
Please refer to Documentation/thermal/intel_powerclamp.txt for more details.
Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
---
Documentation/thermal/intel_powerclamp.txt | 307 +++++++++++
drivers/thermal/Kconfig | 10 +
drivers/thermal/Makefile | 1 +
drivers/thermal/intel_powerclamp.c | 766 ++++++++++++++++++++++++++++
4 files changed, 1084 insertions(+)
create mode 100644 Documentation/thermal/intel_powerclamp.txt
create mode 100644 drivers/thermal/intel_powerclamp.c
diff --git a/Documentation/thermal/intel_powerclamp.txt b/Documentation/thermal/intel_powerclamp.txt
new file mode 100644
index 0000000..332de4a
--- /dev/null
+++ b/Documentation/thermal/intel_powerclamp.txt
@@ -0,0 +1,307 @@
+ =======================
+ INTEL POWERCLAMP DRIVER
+ =======================
+By: Arjan van de Ven <arjan@linux.intel.com>
+ Jacob Pan <jacob.jun.pan@linux.intel.com>
+
+Contents:
+ (*) Introduction
+ - Goals and Objectives
+
+ (*) Theory of Operation
+ - Idle Injection
+ - Calibration
+
+ (*) Performance Analysis
+ - Effectiveness and Limitations
+ - Power vs Performance
+ - Scalability
+ - Calibration
+ - Comparison with Alternative Techniques
+
+ (*) Usage and Interfaces
+ - Generic Thermal Layer (sysfs)
+ - Kernel APIs (TBD)
+
+============
+INTRODUCTION
+============
+
+Consider the situation where a system’s power consumption must be
+reduced at runtime, due to power budget, thermal constraint, or noise
+level, and where active cooling is not preferred. Software managed
+passive power reduction must be performed to prevent the hardware
+actions that are designed for catastrophic scenarios.
+
+Currently, P-states, T-states (clock modulation), and CPU offlining
+are used for CPU throttling.
+
+On Intel CPUs, C-states provide effective power reduction, but so far
+they’re only used opportunistically, based on workload. With the
+development of intel_powerclamp driver, the method of synchronizing
+idle injection across all online CPU threads was introduced. The goal
+is to achieve forced and controllable C-state residency.
+
+Test/Analysis has been made in the areas of power, performance,
+scalability, and user experience. In many cases, clear advantage is
+shown over taking the CPU offline or modulating the CPU clock.
+
+
+===================
+THEORY OF OPERATION
+===================
+
+Idle Injection
+--------------
+
+On modern Intel processors (Nehalem or later), package level C-state
+residency is available in MSRs, thus also available to the kernel.
+
+These MSRs are:
+ #define MSR_PKG_C2_RESIDENCY 0x60D
+ #define MSR_PKG_C3_RESIDENCY 0x3F8
+ #define MSR_PKG_C6_RESIDENCY 0x3F9
+ #define MSR_PKG_C7_RESIDENCY 0x3FA
+
+If the kernel can also inject idle time to the system, then a
+closed-loop control system can be established that manages package
+level C-state. The intel_powerclamp driver is conceived as such a
+control system, where the target set point is a user-selected idle
+ratio (based on power reduction), and the error is the difference
+between the actual package level C-state residency ratio and the target idle
+ratio.
+
+Injection is controlled by high priority kernel threads, spawned for
+each online CPU.
+
+These kernel threads, with SCHED_FIFO class, are created to perform
+clamping actions of controlled duty ratio and duration. Each per-CPU
+thread synchronizes its idle time and duration, based on the rounding
+of jiffies, so accumulated errors can be prevented to avoid a jittery
+effect. Threads are also bound to the CPU such that they cannot be
+migrated, unless the CPU is taken offline. In this case, threads
+belong to the offlined CPUs will be terminated immediately.
+
+Running as SCHED_FIFO and relatively high priority, also allows such
+scheme to work for both preemptable and non-preemptable kernels.
+Alignment of idle time around jiffies ensures scalability for HZ
+values. This effect can be better visualized using a Perf timechart.
+The following diagram shows the behavior of kernel thread
+kidle_inject/cpu. During idle injection, it runs monitor/mwait idle
+for a given "duration", then relinquishes the CPU to other tasks,
+until the next time interval.
+
+The NOHZ schedule tick is disabled during idle time, but interrupts
+are not masked. Tests show that the extra wakeups from scheduler tick
+have a dramatic impact on the effectiveness of the powerclamp driver
+on large scale systems (Westmere system with 80 processors).
+
+CPU0
+ ____________ ____________
+kidle_inject/0 | sleep | mwait | sleep |
+ _________| |________| |_______
+ duration
+CPU1
+ ____________ ____________
+kidle_inject/1 | sleep | mwait | sleep |
+ _________| |________| |_______
+ ^
+ |
+ |
+ roundup(jiffies, interval)
+
+Only one CPU is allowed to collect statistics and update global
+control parameters. This CPU is referred to as the controlling CPU in
+this document. The controlling CPU is elected at runtime, with a
+policy that favors BSP, taking into account the possibility of a CPU
+hot-plug.
+
+In terms of dynamics of the idle control system, package level idle
+time is considered largely as a non-causal system where its behavior
+cannot be based on the past or current input. Therefore, the
+intel_powerclamp driver attempts to enforce the desired idle time
+instantly as given input (target idle ratio). After injection,
+powerclamp moniors the actual idle for a given time window and adjust
+the next injection accordingly to avoid over/under correction.
+
+When used in a causal control system, such as a temperature control,
+it is up to the user of this driver to implement algorithms where
+past samples and outputs are included in the feedback. For example, a
+PID-based thermal controller can use the powerclamp driver to
+maintain a desired target temperature, based on integral and
+derivative gains of the past samples.
+
+
+
+Calibration
+-----------
+During scalability testing, it is observed that synchronized actions
+among CPUs become challenging as the number of cores grows. This is
+also true for the ability of a system to enter package level C-states.
+
+To make sure the intel_powerclamp driver scales well, online
+calibration is implemented. The goals for doing such a calibration
+are:
+
+a) determine the effective range of idle injection ratio
+b) determine the amount of compensation needed at each target ratio
+
+Compensation to each target ratio consists of two parts:
+
+ a) steady state error compensation
+ This is to offset the error occurring when the system can
+ enter idle without extra wakeups (such as external interrupts).
+
+ b) dynamic error compensation
+ When an excessive amount of wakeups occurs during idle, an
+ additional idle ratio can be added to quiet interrupts, by
+ slowing down CPU activities.
+
+A debugfs file is provided for the user to examine compensation
+progress and results, such as on a Westmere system.
+[jacob@nex01 ~]$ cat
+/sys/kernel/debug/intel_powerclamp/powerclamp_calib
+controlling cpu: 0
+pct confidence steady dynamic (compensation)
+0 0 0 0
+1 1 0 0
+2 1 1 0
+3 3 1 0
+4 3 1 0
+5 3 1 0
+6 3 1 0
+7 3 1 0
+8 3 1 0
+...
+30 3 2 0
+31 3 2 0
+32 3 1 0
+33 3 2 0
+34 3 1 0
+35 3 2 0
+36 3 1 0
+37 3 2 0
+38 3 1 0
+39 3 2 0
+40 3 3 0
+41 3 1 0
+42 3 2 0
+43 3 1 0
+44 3 1 0
+45 3 2 0
+46 3 3 0
+47 3 0 0
+48 3 2 0
+49 3 3 0
+
+Calibration occurs during runtime. No offline method is available.
+Steady state compensation is used only when confidence levels of all
+adjacent ratios have reached satisfactory level. A confidence level
+is accumulated based on clean data collected at runtime. Data
+collected during a period without extra interrupts is considered
+clean.
+
+To compensate for excessive amounts of wakeup during idle, additional
+idle time is injected when such a condition is detected. Currently,
+we have a simple algorithm to double the injection ratio. A possible
+enhancement might be to throttle the offending IRQ, such as delaying
+EOI for level triggered interrupts. But it is a challenge to be
+non-intrusive to the scheduler or the IRQ core code.
+
+
+CPU Online/Offline
+------------------
+Per-CPU kernel threads are started/stopped upon receiving
+notifications of CPU hotplug activities. The intel_powerclamp driver
+keeps track of clamping kernel threads, even after they are migrated
+to other CPUs, after a CPU offline event.
+
+
+=====================
+Performance Analysis
+=====================
+This section describes the general performance data collected on
+multiple systems, including Westmere (80P) and Ivy Bridge (4P, 8P).
+
+Effectiveness and Limitations
+-----------------------------
+The maximum range that idle injection is allowed is capped at 50
+percent. As mentioned earlier, since interrupts are allowed during
+forced idle time, excessive interrupts could result in less
+effectiveness. The extreme case would be doing a ping -f to generated
+flooded network interrupts without much CPU acknowledgement. In this
+case, little can be done from the idle injection threads. In most
+normal cases, such as scp a large file, applications can be throttled
+by the powerclamp driver, since slowing down the CPU also slows down
+network protocol processing, which in turn reduces interrupts.
+
+When control parameters change at runtime by the controlling CPU, it
+may take an additional period for the rest of the CPUs to catch up
+with the changes. During this time, idle injection is out of sync,
+thus not able to enter package C- states at the expected ratio. But
+this effect is minor, in that in most cases change to the target
+ratio is updated much less frequently than the idle injection
+frequency.
+
+Scalability
+-----------
+Tests also show a minor, but measurable, difference between the 4P/8P
+Ivy Bridge system and the 80P Westmere server under 50% idle ratio.
+More compensation is needed on Westmere for the same amount of
+target idle ratio. The compensation also increases as the idle ratio
+gets larger. The above reason constitutes the need for the
+calibration code.
+
+On the IVB 8P system, compared to an offline CPU, powerclamp can
+achieve up to 40% better performance per watt. (measured by a spin
+counter summed over per CPU counting threads spawned for all running
+CPUs).
+
+====================
+Usage and Interfaces
+====================
+The powerclamp driver is registered to the generic thermal layer as a
+cooling device. Currently, it’s not bound to any thermal zones.
+
+jacob@chromoly:/sys/class/thermal/cooling_device14$ grep . *
+cur_state:0
+max_state:50
+type:intel_powerclamp
+
+Example usage:
+- To inject 25% idle time
+$ sudo sh -c "echo 25 > /sys/class/thermal/cooling_device80/cur_state
+"
+
+If the system is not busy and has more than 25% idle time already,
+then the powerclamp driver will not start idle injection. Using Top
+will not show idle injection kernel threads.
+
+If the system is busy (spin test below) and has less than 25% natural
+idle time, powerclamp kernel threads will do idle injection, which
+appear running to the scheduler. But the overall system idle is still
+reflected. In this example, 24.1% idle is shown. This helps the
+system admin or user determine the cause of slowdown, when a
+powerclamp driver is in action.
+
+
+Tasks: 197 total, 1 running, 196 sleeping, 0 stopped, 0 zombie
+Cpu(s): 71.2%us, 4.7%sy, 0.0%ni, 24.1%id, 0.0%wa, 0.0%hi, 0.0%si, 0.0%st
+Mem: 3943228k total, 1689632k used, 2253596k free, 74960k buffers
+Swap: 4087804k total, 0k used, 4087804k free, 945336k cached
+
+ PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND
+ 3352 jacob 20 0 262m 644 428 S 286 0.0 0:17.16 spin
+ 3341 root -51 0 0 0 0 D 25 0.0 0:01.62 kidle_inject/0
+ 3344 root -51 0 0 0 0 D 25 0.0 0:01.60 kidle_inject/3
+ 3342 root -51 0 0 0 0 D 25 0.0 0:01.61 kidle_inject/1
+ 3343 root -51 0 0 0 0 D 25 0.0 0:01.60 kidle_inject/2
+ 2935 jacob 20 0 696m 125m 35m S 5 3.3 0:31.11 firefox
+ 1546 root 20 0 158m 20m 6640 S 3 0.5 0:26.97 Xorg
+ 2100 jacob 20 0 1223m 88m 30m S 3 2.3 0:23.68 compiz
+
+Tests have shown that by using the powerclamp driver as a cooling
+device, a PID based userspace thermal controller can manage to
+control CPU temperature effectively, when no other thermal influence
+is added. For example, a UltraBook user can compile the kernel under
+certain temperature (below most active trip points).
diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index e1cb6bd..4d99c4b 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -55,3 +55,13 @@ config EXYNOS_THERMAL
help
If you say yes here you get support for TMU (Thermal Managment
Unit) on SAMSUNG EXYNOS series of SoC.
+
+config INTEL_POWERCLAMP
+ tristate "Intel PowerClamp idle injection driver"
+ depends on THERMAL
+ depends on X86
+ depends on CPU_SUP_INTEL
+ help
+ Enable this to enable Intel PowerClamp idle injection driver. This
+ enforce idle time which results in more package C-state residency. The
+ user interface is exposed via generic thermal framework.
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 885550d..03e4479 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_CPU_THERMAL) += cpu_cooling.o
obj-$(CONFIG_SPEAR_THERMAL) += spear_thermal.o
obj-$(CONFIG_RCAR_THERMAL) += rcar_thermal.o
obj-$(CONFIG_EXYNOS_THERMAL) += exynos_thermal.o
+obj-$(CONFIG_INTEL_POWERCLAMP) += intel_powerclamp.o
diff --git a/drivers/thermal/intel_powerclamp.c b/drivers/thermal/intel_powerclamp.c
new file mode 100644
index 0000000..cc9fa17
--- /dev/null
+++ b/drivers/thermal/intel_powerclamp.c
@@ -0,0 +1,766 @@
+/*
+ * intel_powerclamp.c - package c-state idle injection
+ *
+ * Copyright (c) 2012, Intel Corporation.
+ *
+ * Authors:
+ * Arjan van de Ven <arjan@linux.intel.com>
+ * Jacob Pan <jacob.jun.pan@linux.intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ *
+ * TODO:
+ * 1. better handle wakeup from external interrupts, currently a fixed
+ * compensation is added to clamping duration when excessive amount
+ * of wakeups are observed during idle time. the reason is that in
+ * case of external interrupts without need for ack, clamping down
+ * cpu in non-irq context does not reduce irq. for majority of the
+ * cases, clamping down cpu does help reduce irq as well, we should
+ * be able to differenciate the two cases and give a quantitative
+ * solution for the irqs that we can control. perhaps based on
+ * get_cpu_iowait_time_us()
+ *
+ * 2. synchronization with other hw blocks
+ *
+ *
+ */
+
+/* #define DEBUG */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/delay.h>
+#include <linux/kthread.h>
+#include <linux/freezer.h>
+#include <linux/cpu.h>
+#include <linux/thermal.h>
+#include <linux/slab.h>
+#include <linux/tick.h>
+#include <linux/debugfs.h>
+#include <linux/seq_file.h>
+#include <linux/nmi.h>
+
+#include <asm/msr.h>
+#include <asm/mwait.h>
+#include <asm/cpu_device_id.h>
+#include <asm/idle.h>
+#include <asm/hardirq.h>
+
+#define MSR_PKG_C2_RESIDENCY 0x60D
+#define MSR_PKG_C3_RESIDENCY 0x3F8
+#define MSR_PKG_C6_RESIDENCY 0x3F9
+#define MSR_PKG_C7_RESIDENCY 0x3FA
+
+#define MAX_TARGET_RATIO (50)
+/* For each undisturbed clamping period (no extra wake ups during idle time),
+ * we increment the confidence counter for the given target ratio.
+ * CONFIDENCE_OK defines the level where runtime calibration results are
+ * valid.
+ */
+#define CONFIDENCE_OK (3)
+/* Default idle injection duration, driver adjust sleep time to meet target
+ * idle ratio. Similar to frequency modulation.
+ */
+#define DEFAULT_DURATION_JIFFIES (6)
+
+static unsigned int target_mwait;
+static struct dentry *debug_dir;
+
+/* user selected target */
+static unsigned int set_target_ratio;
+static unsigned int current_ratio;
+static bool should_skip;
+static bool reduce_irq;
+static atomic_t idle_wakeup_counter;
+static unsigned int control_cpu; /* The cpu assigned to collect stat and update
+ * control parameters. default to BSP but BSP
+ * can be offlined.
+ */
+static int clamping;
+
+
+static struct task_struct __percpu **powerclamp_thread;
+static struct thermal_cooling_device *cooling_dev;
+static unsigned long *cpu_clamping_mask; /* bit map for tracking per cpu
+ * clamping thread
+ */
+static int duration;
+module_param(duration, int, 0600);
+MODULE_PARM_DESC(duration, "forced idle time for each attempt in msec.");
+
+static unsigned int pkg_cstate_ratio_cur;
+static unsigned int window_size;
+
+struct powerclamp_calibration_data {
+ unsigned long confidence; /* used for calibration, basically a counter
+ * gets incremented each time a clamping
+ * period is completed without extra wakeups
+ * once that counter is reached given level,
+ * compensation is deemed usable.
+ */
+ unsigned long steady_comp; /* steady state compensation used when
+ * no extra wakeups occurred.
+ */
+ unsigned long dynamic_comp; /* compensate excessive wakeup from idle
+ * mostly from external interrupts.
+ */
+};
+
+static struct powerclamp_calibration_data cal_data[MAX_TARGET_RATIO];
+
+static int window_size_set(const char *arg, const struct kernel_param *kp)
+{
+ int ret = 0;
+ unsigned long new_window_size;
+
+ ret = kstrtoul(arg, 10, &new_window_size);
+ if (ret)
+ goto exit_win;
+ if (new_window_size >= 10 || new_window_size < 2) {
+ pr_err("PowerClamp: invalid window size %lu, between 2-10\n",
+ new_window_size);
+ ret = -EINVAL;
+ }
+
+ window_size = new_window_size;
+ smp_mb();
+
+exit_win:
+
+ return ret;
+}
+static struct kernel_param_ops window_size_ops = {
+ .set = window_size_set,
+ .get = param_get_int,
+};
+
+module_param_cb(window_size, &window_size_ops, &window_size, 0644);
+MODULE_PARM_DESC(window_size, "sliding window in number of clamping cycles\n"
+ "\tpowerclamp controls idle ratio within this window. larger\n"
+ "\twindow size results in slower response time but more smooth\n"
+ "\tclamping results. default to 2.");
+
+static void find_target_mwait(void)
+{
+ unsigned int eax, ebx, ecx, edx;
+ unsigned int highest_cstate = 0;
+ unsigned int highest_subcstate = 0;
+ int i;
+
+ if (boot_cpu_data.cpuid_level < CPUID_MWAIT_LEAF)
+ return;
+
+ cpuid(CPUID_MWAIT_LEAF, &eax, &ebx, &ecx, &edx);
+
+ if (!(ecx & CPUID5_ECX_EXTENSIONS_SUPPORTED) ||
+ !(ecx & CPUID5_ECX_INTERRUPT_BREAK))
+ return;
+
+ edx >>= MWAIT_SUBSTATE_SIZE;
+ for (i = 0; i < 7 && edx; i++, edx >>= MWAIT_SUBSTATE_SIZE) {
+ if (edx & MWAIT_SUBSTATE_MASK) {
+ highest_cstate = i;
+ highest_subcstate = edx & MWAIT_SUBSTATE_MASK;
+ }
+ }
+ target_mwait = (highest_cstate << MWAIT_SUBSTATE_SIZE) |
+ (highest_subcstate - 1);
+
+}
+
+static u64 pkg_state_counter(void)
+{
+ u64 val;
+ u64 count = 0;
+
+ static int skip_c2;
+ static int skip_c3;
+ static int skip_c6;
+ static int skip_c7;
+
+ if (!skip_c2) {
+ if (!rdmsrl_safe(MSR_PKG_C2_RESIDENCY, &val))
+ count += val;
+ else
+ skip_c2 = 1;
+ }
+
+ if (!skip_c3) {
+ if (!rdmsrl_safe(MSR_PKG_C3_RESIDENCY, &val))
+ count += val;
+ else
+ skip_c3 = 1;
+ }
+
+ if (!skip_c6) {
+ if (!rdmsrl_safe(MSR_PKG_C6_RESIDENCY, &val))
+ count += val;
+ else
+ skip_c6 = 1;
+ }
+
+ if (!skip_c7) {
+ if (!rdmsrl_safe(MSR_PKG_C7_RESIDENCY, &val))
+ count += val;
+ else
+ skip_c7 = 1;
+ }
+
+ return count;
+}
+
+static void noop_timer(unsigned long foo)
+{
+ /* empty... just the fact that we get the interrupt wakes us up */
+}
+
+static unsigned int get_compensation(int ratio)
+{
+ unsigned int comp = 0;
+
+ /* we only use compensation if all adjacent ones are good */
+ if (ratio == 1 &&
+ cal_data[ratio].confidence >= CONFIDENCE_OK &&
+ cal_data[ratio + 1].confidence >= CONFIDENCE_OK &&
+ cal_data[ratio + 2].confidence >= CONFIDENCE_OK) {
+ comp = (cal_data[ratio].steady_comp +
+ cal_data[ratio + 1].steady_comp +
+ cal_data[ratio + 2].steady_comp) / 3;
+ } else if (ratio == MAX_TARGET_RATIO - 1 &&
+ cal_data[ratio].confidence >= CONFIDENCE_OK &&
+ cal_data[ratio - 1].confidence >= CONFIDENCE_OK &&
+ cal_data[ratio - 2].confidence >= CONFIDENCE_OK) {
+ comp = (cal_data[ratio].steady_comp +
+ cal_data[ratio - 1].steady_comp +
+ cal_data[ratio - 2].steady_comp) / 3;
+ } else if (cal_data[ratio].confidence >= CONFIDENCE_OK &&
+ cal_data[ratio - 1].confidence >= CONFIDENCE_OK &&
+ cal_data[ratio + 1].confidence >= CONFIDENCE_OK) {
+ comp = (cal_data[ratio].steady_comp +
+ cal_data[ratio - 1].steady_comp +
+ cal_data[ratio + 1].steady_comp) / 3;
+ }
+
+ /* REVISIT: simple penalty of double idle injection */
+ if (reduce_irq)
+ comp = ratio;
+ /* do not exceed limit */
+ if (comp + ratio >= MAX_TARGET_RATIO)
+ comp = MAX_TARGET_RATIO - ratio - 1;
+
+ return comp;
+}
+
+static void adjust_compensation(int target_ratio, unsigned int win)
+{
+ int delta;
+
+ /*
+ * adjust compensations if confidence level has not been reached or
+ * there are too many wakeups during the last idle injection period, we
+ * cannot trust the data for compensation.
+ */
+ if (cal_data[target_ratio].confidence >= CONFIDENCE_OK ||
+ atomic_read(&idle_wakeup_counter) >
+ win * num_online_cpus())
+ return;
+
+ delta = set_target_ratio - current_ratio;
+ /* filter out bad data */
+ if (delta >= 0 && delta <= (1+target_ratio/10)) {
+ if (cal_data[target_ratio].steady_comp)
+ cal_data[target_ratio].steady_comp =
+ roundup(delta+
+ cal_data[target_ratio].steady_comp,
+ 2)/2;
+ else
+ cal_data[target_ratio].steady_comp = delta;
+ cal_data[target_ratio].confidence++;
+ }
+}
+
+static bool powerclamp_adjust_controls(unsigned int target_ratio,
+ unsigned int guard, unsigned int win)
+{
+ static u64 msr_last, tsc_last;
+ u64 msr_now, tsc_now;
+
+ /* check result for the last window */
+ msr_now = pkg_state_counter();
+ rdtscll(tsc_now);
+
+ /* calculate pkg cstate vs tsc ratio */
+ if (!msr_last || !tsc_last)
+ current_ratio = 1;
+ else if (tsc_now-tsc_last)
+ current_ratio = 100*(msr_now-msr_last)/
+ (tsc_now-tsc_last);
+
+ /* update record */
+ msr_last = msr_now;
+ tsc_last = tsc_now;
+
+ adjust_compensation(target_ratio, win);
+ /*
+ * too many external interrupts, set flag such
+ * that we can take measure later.
+ */
+ reduce_irq = atomic_read(&idle_wakeup_counter) >=
+ 2 * win * num_online_cpus();
+
+ atomic_set(&idle_wakeup_counter, 0);
+ /* if we are above target+guard, skip */
+ return set_target_ratio + guard <= current_ratio;
+}
+
+static int clamp_thread(void *arg)
+{
+ int cpunr = (unsigned long)arg;
+ DEFINE_TIMER(wakeup_timer, noop_timer, 0, 0);
+ static const struct sched_param param = {
+ .sched_priority = MAX_USER_RT_PRIO/2,
+ };
+ unsigned int count = 0;
+ unsigned int target_ratio;
+
+ set_bit(cpunr, cpu_clamping_mask);
+ set_freezable();
+ init_timer_on_stack(&wakeup_timer);
+ sched_setscheduler(current, SCHED_FIFO, ¶m);
+
+ while (clamping && !kthread_should_stop() && cpu_online(cpunr)) {
+ int sleeptime;
+ unsigned long target_jiffies;
+ unsigned int guard;
+ unsigned int compensation = 0;
+ int interval; /* jiffies to sleep for each attempt */
+ unsigned int duration_jiffies = msecs_to_jiffies(duration);
+ unsigned int window_size_now;
+
+ try_to_freeze();
+ /*
+ * make sure user selected ratio does not take effect until
+ * the next round. adjust target_ratio if user has changed
+ * target such that we can converge quickly.
+ */
+ target_ratio = set_target_ratio;
+ guard = 1 + target_ratio/20;
+ window_size_now = window_size;
+ count++;
+
+ /*
+ * systems may have different ability to enter package level
+ * c-states, thus we need to compensate the injected idle ratio
+ * to achieve the actual target reported by the HW.
+ */
+ compensation = get_compensation(target_ratio);
+ interval = duration_jiffies*100/(target_ratio+compensation);
+
+ /* align idle time */
+ target_jiffies = roundup(jiffies, interval);
+ sleeptime = target_jiffies - jiffies;
+ if (sleeptime <= 0)
+ sleeptime = 1;
+ schedule_timeout_interruptible(sleeptime);
+ /*
+ * only elected controlling cpu can collect stats and update
+ * control parameters.
+ */
+ if (cpunr == control_cpu && !(count%window_size_now)) {
+ should_skip =
+ powerclamp_adjust_controls(target_ratio,
+ guard, window_size_now);
+ smp_mb();
+ }
+
+ if (should_skip)
+ continue;
+
+ target_jiffies = jiffies + duration_jiffies;
+ mod_timer(&wakeup_timer, target_jiffies);
+ if (unlikely(local_softirq_pending()))
+ continue;
+ /*
+ * stop tick sched during idle time, interrupts are still
+ * allowed. thus jiffies are updated properly.
+ */
+ preempt_disable();
+ tick_nohz_idle_enter();
+ /* mwait until target jiffies is reached */
+ while (time_before(jiffies, target_jiffies)) {
+ unsigned long ecx = 1;
+ unsigned long eax = target_mwait;
+
+ /*
+ * REVISIT: may call enter_idle() to notify drivers who
+ * can save power during cpu idle. same for exit_idle()
+ */
+ local_touch_nmi();
+ stop_critical_timings();
+ __monitor((void *)¤t_thread_info()->flags, 0, 0);
+ cpu_relax(); /* allow HT sibling to run */
+ __mwait(eax, ecx);
+ start_critical_timings();
+ atomic_inc(&idle_wakeup_counter);
+ }
+ tick_nohz_idle_exit();
+ preempt_enable_no_resched();
+ }
+ del_timer_sync(&wakeup_timer);
+ clear_bit(cpunr, cpu_clamping_mask);
+
+ return 0;
+}
+
+/*
+ * 1 HZ polling while clamping is active, useful for userspace
+ * to monitor actual idle ratio.
+ */
+static void poll_pkg_cstate(struct work_struct *dummy);
+static DECLARE_DELAYED_WORK(poll_pkg_cstate_work, poll_pkg_cstate);
+static void poll_pkg_cstate(struct work_struct *dummy)
+{
+ static u64 msr_last;
+ static u64 tsc_last;
+ static unsigned long jiffies_last;
+
+ u64 msr_now;
+ unsigned long jiffies_now;
+ u64 tsc_now;
+
+ msr_now = pkg_state_counter();
+ rdtscll(tsc_now);
+ jiffies_now = jiffies;
+
+ /* calculate pkg cstate vs tsc ratio */
+ if (!msr_last || !tsc_last)
+ pkg_cstate_ratio_cur = 1;
+ else {
+ if (tsc_now - tsc_last)
+ pkg_cstate_ratio_cur = 100 * (msr_now - msr_last)/
+ (tsc_now - tsc_last);
+ }
+
+ /* update record */
+ msr_last = msr_now;
+ jiffies_last = jiffies_now;
+ tsc_last = tsc_now;
+
+ if (clamping)
+ schedule_delayed_work(&poll_pkg_cstate_work, HZ);
+}
+
+static int start_power_clamp(void)
+{
+ unsigned long cpu;
+ struct task_struct *thread;
+
+ /* check if pkg cstate counter is completely 0, abort in this case */
+ if (!pkg_state_counter()) {
+ pr_err("pkg cstate counter not functional, abort\n");
+ return -EINVAL;
+ }
+
+ if (set_target_ratio > MAX_TARGET_RATIO)
+ set_target_ratio = MAX_TARGET_RATIO;
+
+ /* prevent cpu hotplug */
+ get_online_cpus();
+
+ /* prefer BSP */
+ control_cpu = 0;
+ if (!cpu_online(control_cpu))
+ control_cpu = smp_processor_id();
+
+ clamping = 1;
+ schedule_delayed_work(&poll_pkg_cstate_work, 0);
+
+ /* start one thread per online cpu */
+ for_each_online_cpu(cpu) {
+ struct task_struct **p =
+ per_cpu_ptr(powerclamp_thread, cpu);
+
+ thread = kthread_create_on_node(clamp_thread,
+ (void *) cpu,
+ cpu_to_node(cpu),
+ "kidle_inject/%ld", cpu);
+ /* bind to cpu here */
+ if (likely(!IS_ERR(thread))) {
+ kthread_bind(thread, cpu);
+ wake_up_process(thread);
+ *p = thread;
+ }
+
+ }
+ put_online_cpus();
+
+ return 0;
+}
+
+static void end_power_clamp(void)
+{
+ int i;
+ struct task_struct *thread;
+
+ clamping = 0;
+ /*
+ * make clamping visible to other cpus and give per cpu clamping threads
+ * sometime to exit, or gets killed later.
+ */
+ smp_mb();
+ msleep(20);
+ if (bitmap_weight(cpu_clamping_mask, num_possible_cpus())) {
+ for_each_set_bit(i, cpu_clamping_mask, num_possible_cpus()) {
+ pr_debug("clamping thread for cpu %d alive, kill\n", i);
+ thread = *per_cpu_ptr(powerclamp_thread, i);
+ kthread_stop(thread);
+ }
+ }
+}
+
+static int powerclamp_cpu_callback(struct notifier_block *nfb,
+ unsigned long action, void *hcpu)
+{
+ unsigned long cpu = (unsigned long)hcpu;
+ struct task_struct *thread;
+ struct task_struct **percpu_thread =
+ per_cpu_ptr(powerclamp_thread, cpu);
+
+ if (!clamping)
+ goto exit_ok;
+
+ switch (action) {
+ case CPU_ONLINE:
+ thread = kthread_create_on_node(clamp_thread,
+ (void *) cpu,
+ cpu_to_node(cpu),
+ "kidle_inject/%lu", cpu);
+ if (likely(!IS_ERR(thread))) {
+ kthread_bind(thread, cpu);
+ wake_up_process(thread);
+ *percpu_thread = thread;
+ }
+ /* prefer BSP as controlling CPU */
+ if (cpu == 0) {
+ control_cpu = 0;
+ smp_mb();
+ }
+ break;
+ case CPU_DEAD:
+ if (test_bit(cpu, cpu_clamping_mask)) {
+ pr_err("cpu %lu dead but powerclamping thread is not\n",
+ cpu);
+ kthread_stop(*percpu_thread);
+ }
+ if (cpu == control_cpu) {
+ control_cpu = smp_processor_id();
+ smp_mb();
+ }
+ }
+
+exit_ok:
+ return NOTIFY_OK;
+}
+
+static struct notifier_block powerclamp_cpu_notifier = {
+ .notifier_call = powerclamp_cpu_callback,
+};
+
+static int powerclamp_get_max_state(struct thermal_cooling_device *cdev,
+ unsigned long *state)
+{
+ *state = MAX_TARGET_RATIO;
+
+ return 0;
+}
+
+static int powerclamp_get_cur_state(struct thermal_cooling_device *cdev,
+ unsigned long *state)
+{
+ if (clamping)
+ *state = pkg_cstate_ratio_cur;
+ else
+ /* to save power, do not poll idle ratio while not clamping */
+ *state = -1; /* indicates invalid state */
+
+ return 0;
+}
+
+static int powerclamp_set_cur_state(struct thermal_cooling_device *cdev,
+ unsigned long new_target_ratio)
+{
+ int ret = 0;
+
+ if (new_target_ratio >= MAX_TARGET_RATIO)
+ new_target_ratio = MAX_TARGET_RATIO - 1;
+
+ if (set_target_ratio == 0 && new_target_ratio > 0) {
+ pr_info("Start idle injection to reduce power\n");
+ set_target_ratio = new_target_ratio;
+ ret = start_power_clamp();
+ goto exit_set;
+ } else if (set_target_ratio > 0 && new_target_ratio == 0) {
+ pr_info("Stop forced idle injection\n");
+ set_target_ratio = 0;
+ end_power_clamp();
+ } else /* adjust currently running */ {
+ set_target_ratio = new_target_ratio;
+ /* make new set_target_ratio visible to other cpus */
+ smp_mb();
+ }
+
+exit_set:
+ return ret;
+}
+
+/* bind to generic thermal layer as cooling device*/
+static struct thermal_cooling_device_ops powerclamp_cooling_ops = {
+ .get_max_state = powerclamp_get_max_state,
+ .get_cur_state = powerclamp_get_cur_state,
+ .set_cur_state = powerclamp_set_cur_state,
+};
+
+/* runs on Nehalem and later */
+static const struct x86_cpu_id intel_powerclamp_ids[] = {
+ { X86_VENDOR_INTEL, 6, 0x1a},
+ { X86_VENDOR_INTEL, 6, 0x1c},
+ { X86_VENDOR_INTEL, 6, 0x1e},
+ { X86_VENDOR_INTEL, 6, 0x1f},
+ { X86_VENDOR_INTEL, 6, 0x25},
+ { X86_VENDOR_INTEL, 6, 0x26},
+ { X86_VENDOR_INTEL, 6, 0x2a},
+ { X86_VENDOR_INTEL, 6, 0x2c},
+ { X86_VENDOR_INTEL, 6, 0x2d},
+ { X86_VENDOR_INTEL, 6, 0x2e},
+ { X86_VENDOR_INTEL, 6, 0x2f},
+ { X86_VENDOR_INTEL, 6, 0x3a},
+ {}
+};
+MODULE_DEVICE_TABLE(x86cpu, intel_powerclamp_ids);
+
+static int powerclamp_probe(void)
+{
+ if (!x86_match_cpu(intel_powerclamp_ids)) {
+ pr_err("Intel powerclamp does not run on family %d model %d\n",
+ boot_cpu_data.x86, boot_cpu_data.x86_model);
+ return -ENODEV;
+ }
+ if (!boot_cpu_has(X86_FEATURE_NONSTOP_TSC) ||
+ !boot_cpu_has(X86_FEATURE_CONSTANT_TSC) ||
+ !boot_cpu_has(X86_FEATURE_MWAIT) ||
+ !boot_cpu_has(X86_FEATURE_ARAT))
+ return -ENODEV;
+
+ /* find the deepest mwait value */
+ find_target_mwait();
+
+ return 0;
+}
+
+static int powerclamp_debug_show(struct seq_file *m, void *unused)
+{
+ int i = 0;
+
+ seq_printf(m, "controlling cpu: %d\n", control_cpu);
+ seq_printf(m, "pct confidence steady dynamic (compensation)\n");
+ for (i = 0; i < MAX_TARGET_RATIO; i++) {
+ seq_printf(m, "%d\t%lu\t%lu\t%lu\n",
+ i,
+ cal_data[i].confidence,
+ cal_data[i].steady_comp,
+ cal_data[i].dynamic_comp);
+ }
+
+ return 0;
+}
+
+static int powerclamp_debug_open(struct inode *inode,
+ struct file *file)
+{
+ return single_open(file, powerclamp_debug_show, inode->i_private);
+}
+
+static const struct file_operations powerclamp_debug_fops = {
+ .open = powerclamp_debug_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+ .owner = THIS_MODULE,
+};
+
+static inline void powerclamp_create_debug_files(void)
+{
+ debug_dir = debugfs_create_dir("intel_powerclamp", NULL);
+ if (!debug_dir)
+ return;
+
+ if (!debugfs_create_file("powerclamp_calib", S_IRUGO, debug_dir,
+ cal_data, &powerclamp_debug_fops))
+ goto file_error;
+
+ return;
+
+file_error:
+ debugfs_remove_recursive(debug_dir);
+}
+
+static int powerclamp_init(void)
+{
+ int retval;
+ int bitmap_size;
+
+ bitmap_size = BITS_TO_LONGS(num_possible_cpus()) * sizeof(long);
+ cpu_clamping_mask = kzalloc(bitmap_size, GFP_KERNEL);
+ if (!cpu_clamping_mask)
+ return -ENOMEM;
+
+ /* probe cpu features and ids here */
+ retval = powerclamp_probe();
+ if (retval)
+ return retval;
+ /* set default limit, maybe adjusted during runtime based on feedback */
+ window_size = 2;
+ register_hotcpu_notifier(&powerclamp_cpu_notifier);
+ powerclamp_thread = alloc_percpu(struct task_struct *);
+ cooling_dev = thermal_cooling_device_register("intel_powerclamp", NULL,
+ &powerclamp_cooling_ops);
+ if (IS_ERR(cooling_dev))
+ return -ENODEV;
+
+ if (!duration)
+ duration = jiffies_to_msecs(DEFAULT_DURATION_JIFFIES);
+ powerclamp_create_debug_files();
+
+ return 0;
+}
+module_init(powerclamp_init);
+
+static void powerclamp_exit(void)
+{
+ unregister_hotcpu_notifier(&powerclamp_cpu_notifier);
+ end_power_clamp();
+ free_percpu(powerclamp_thread);
+ thermal_cooling_device_unregister(cooling_dev);
+ kfree(cpu_clamping_mask);
+
+ cancel_delayed_work_sync(&poll_pkg_cstate_work);
+ debugfs_remove_recursive(debug_dir);
+}
+module_exit(powerclamp_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Arjan van de Ven <arjan@linux.intel.com>");
+MODULE_AUTHOR("Jacob Pan <jacob.jun.pan@linux.intel.com>");
+MODULE_DESCRIPTION("Package Level C-state Idle Injection for Intel CPUs");
--
1.7.9.5
^ permalink raw reply related
* Re: [RFC] cpuidle - remove the power_specified field in the driver
From: Daniel Lezcano @ 2012-11-12 22:08 UTC (permalink / raw)
To: Julius Werner
Cc: linux-pm, khilman, len.brown, Trinabh Gupta, deepthi,
linux-kernel, rjw, akpm, Sameer Nanda, Lists Linaro-dev
In-Reply-To: <CAODwPW-yjzrAFRA4Bv-MZETFEfq+cpaLvHaYX5K5HBtQtJKZQA@mail.gmail.com>
On 11/12/2012 10:09 PM, Julius Werner wrote:
> Thanks for moving this along, Daniel. I think this is the right
> approach... the cpuidle driver shouldn't be more complex than
> necessary.
>
> Note that you are starting your loop too high in cpuidle_play_dead...
> states[state_count] is not an actual state anymore, it should start at
> state_count - 1.
Yep. Thanks for catching this.
> Also, I think you can go ahead and do the same
> last-to-first loop transformation with immediate return in the menu
> governor, for an extra tiny bit of performance.
Yes, that makes sense.
Thanks for the review.
-- Daniel
> On Mon, Nov 12, 2012 at 12:26 PM, Daniel Lezcano
> <daniel.lezcano@linaro.org> wrote:
>> This patch follows the discussion about reinitializing the power usage
>> when a C-state is added/removed.
>>
>> https://lkml.org/lkml/2012/10/16/518
>>
>> We realized the power usage field is never filled and when it is
>> filled for tegra, the power_specified flag is not set making all these
>> values to be resetted when the driver is initialized with the set_power_state
>> function.
>>
>> Julius and I feel this is over-engineered and the power_specified
>> flag could be simply removed and continue assuming the states are
>> backward sorted.
>>
>> The menu governor select function is simplified as the power is ordered.
>> Actually the condition is always true with the current code.
>>
>> The cpuidle_play_dead function is also simplified by doing a reverse lookup
>> on the array.
>>
>> The set_power_states function is removed as it does no make sense anymore.
>>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>> ---
>> drivers/cpuidle/cpuidle.c | 17 ++++-------------
>> drivers/cpuidle/driver.c | 25 -------------------------
>> drivers/cpuidle/governors/menu.c | 8 ++------
>> include/linux/cpuidle.h | 2 +-
>> 4 files changed, 7 insertions(+), 45 deletions(-)
>>
>> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
>> index 711dd83..f983262 100644
>> --- a/drivers/cpuidle/cpuidle.c
>> +++ b/drivers/cpuidle/cpuidle.c
>> @@ -69,24 +69,15 @@ int cpuidle_play_dead(void)
>> {
>> struct cpuidle_device *dev = __this_cpu_read(cpuidle_devices);
>> struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev);
>> - int i, dead_state = -1;
>> - int power_usage = -1;
>> + int i;
>>
>> if (!drv)
>> return -ENODEV;
>>
>> /* Find lowest-power state that supports long-term idle */
>> - for (i = CPUIDLE_DRIVER_STATE_START; i < drv->state_count; i++) {
>> - struct cpuidle_state *s = &drv->states[i];
>> -
>> - if (s->power_usage < power_usage && s->enter_dead) {
>> - power_usage = s->power_usage;
>> - dead_state = i;
>> - }
>> - }
>> -
>> - if (dead_state != -1)
>> - return drv->states[dead_state].enter_dead(dev, dead_state);
>> + for (i = drv->state_count; i >= CPUIDLE_DRIVER_STATE_START; i--)
>> + if (drv->states[i].play_dead)
>> + return drv->states[i].enter_dead(dev, i);
>>
>> return -ENODEV;
>> }
>> diff --git a/drivers/cpuidle/driver.c b/drivers/cpuidle/driver.c
>> index 3af841f..bb045f1 100644
>> --- a/drivers/cpuidle/driver.c
>> +++ b/drivers/cpuidle/driver.c
>> @@ -19,34 +19,9 @@ DEFINE_SPINLOCK(cpuidle_driver_lock);
>> static void __cpuidle_set_cpu_driver(struct cpuidle_driver *drv, int cpu);
>> static struct cpuidle_driver * __cpuidle_get_cpu_driver(int cpu);
>>
>> -static void set_power_states(struct cpuidle_driver *drv)
>> -{
>> - int i;
>> -
>> - /*
>> - * cpuidle driver should set the drv->power_specified bit
>> - * before registering if the driver provides
>> - * power_usage numbers.
>> - *
>> - * If power_specified is not set,
>> - * we fill in power_usage with decreasing values as the
>> - * cpuidle code has an implicit assumption that state Cn
>> - * uses less power than C(n-1).
>> - *
>> - * With CONFIG_ARCH_HAS_CPU_RELAX, C0 is already assigned
>> - * an power value of -1. So we use -2, -3, etc, for other
>> - * c-states.
>> - */
>> - for (i = CPUIDLE_DRIVER_STATE_START; i < drv->state_count; i++)
>> - drv->states[i].power_usage = -1 - i;
>> -}
>> -
>> static void __cpuidle_driver_init(struct cpuidle_driver *drv)
>> {
>> drv->refcnt = 0;
>> -
>> - if (!drv->power_specified)
>> - set_power_states(drv);
>> }
>>
>> static int __cpuidle_register_driver(struct cpuidle_driver *drv, int cpu)
>> diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c
>> index 2efee27..14eb11f 100644
>> --- a/drivers/cpuidle/governors/menu.c
>> +++ b/drivers/cpuidle/governors/menu.c
>> @@ -312,7 +312,6 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
>> {
>> struct menu_device *data = &__get_cpu_var(menu_devices);
>> int latency_req = pm_qos_request(PM_QOS_CPU_DMA_LATENCY);
>> - int power_usage = -1;
>> int i;
>> int multiplier;
>> struct timespec t;
>> @@ -383,11 +382,8 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
>> if (s->exit_latency * multiplier > data->predicted_us)
>> continue;
>>
>> - if (s->power_usage < power_usage) {
>> - power_usage = s->power_usage;
>> - data->last_state_idx = i;
>> - data->exit_us = s->exit_latency;
>> - }
>> + data->last_state_idx = i;
>> + data->exit_us = s->exit_latency;
>> }
>>
>> /* not deepest C-state chosen for low predicted residency */
>> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
>> index 3711b34..24cd103 100644
>> --- a/include/linux/cpuidle.h
>> +++ b/include/linux/cpuidle.h
>> @@ -126,9 +126,9 @@ struct cpuidle_driver {
>> struct module *owner;
>> int refcnt;
>>
>> - unsigned int power_specified:1;
>> /* set to 1 to use the core cpuidle time keeping (for all states). */
>> unsigned int en_core_tk_irqen:1;
>> + /* states array must be ordered in decreasing power consumption */
>> struct cpuidle_state states[CPUIDLE_STATE_MAX];
>> int state_count;
>> int safe_state_index;
>> --
>> 1.7.5.4
>>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
* Re: [RFD][PATCH 7/7] PM / ACPI: Take device PM QoS flags into account
From: Rafael J. Wysocki @ 2012-11-12 23:42 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Linux PM list, ACPI Devel Mailing List, Alan Stern, Huang Ying,
Sarah Sharp, Lan Tianyu, Aaron Lu, Jean Pihet, linux-pci,
Greg Kroah-Hartman, mark gross
In-Reply-To: <CAErSpo7kVb1kLbJ0Y4mYLbzEeZM8dSa-YaZE9B2mN5r74ft-Rg@mail.gmail.com>
On Monday, November 12, 2012 02:07:03 PM Bjorn Helgaas wrote:
> On Fri, Sep 28, 2012 at 3:56 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > Make ACPI power management routines and PCI power management
> > routines depending on ACPI take device PM QoS flags into account
> > when deciding what power state to put the device into.
> >
> > In particular, after this change acpi_pm_device_sleep_state() will
> > not return ACPI_STATE_D3_COLD as the deepest available low-power
> > state if PM_QOS_FLAG_NO_POWER_OFF is requested for the device and it
> > will not require remote wakeup to work for the device in the returned
> > low-power state if there is at least one PM QoS flags request for the
> > device, but PM_QOS_FLAG_REMOTE_WAKEUP is not requested for it.
> >
> > Accordingly, acpi_pci_set_power_state() will refuse to put the
> > device into D3cold if PM_QOS_FLAG_NO_POWER_OFF is requested for it.
> >
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > ---
> > drivers/acpi/sleep.c | 16 ++++++++++++----
> > drivers/pci/pci-acpi.c | 8 +++++++-
>
> This touches a file in drivers/pci, but I expect it depends on all the
> previous patches in the series, so if you haven't merged it already,
> Rafael, it seems like it makes sense for you to merge this instead of
> me.
I will, thanks a lot!
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* Re: [BUGFIX] PM: Fix active child counting when disabled and forbidden
From: Huang Ying @ 2012-11-13 1:19 UTC (permalink / raw)
To: Alan Stern; +Cc: Rafael J. Wysocki, linux-kernel, linux-pm
In-Reply-To: <Pine.LNX.4.44L0.1211121110520.26926-100000@netrider.rowland.org>
On Mon, 2012-11-12 at 11:32 -0500, Alan Stern wrote:
> On Mon, 12 Nov 2012, Huang Ying wrote:
>
> > > > Is it absolute necessary to call pm_runtime_set_suspended? If the
> > > > device is disabled, the transition to SUSPENDED state will not be
> > > > triggered even if the device is ACTIVE.
> > >
> > > It's not absolutely necessary to do this, but we ought to because it
> > > will allow the device's parent to be suspended. If we leave the device
> > > in the ACTIVE state then the parent can't be suspended, even when the
> > > device is disabled.
> >
> > I think this is the hard part of the issue. Now "disabled" and
> > SUSPENDED state is managed by hand. IMHO, if we changed
> > pm_runtime_allow() as you said, we need to change the rule too. Maybe
> > something as follow:
> >
> > - remove pm_runtime_set_suspended/pm_runtime_set_active
>
> We can't remove them entirely. They are needed for situations where
> the device's physical state is different from what the PM core thinks;
> they tell the PM core what the actual state is.
>
> > - in pm_runtime_disable/pm_runtime_allow, put device into SUSPENDED
> > state if runtime PM is not forbidden.
>
> That doesn't make sense. Runtime PM is never forbidden after
> pm_runtime_allow is called; that's its purpose.
Sorry, my original idea is:
pm_runtime_disable will put device into SUSPENDED state if
dev->power.runtime_auto is clear. pm_runtime_allow will put
device into SUSPENDED state if dev->power.disable_depth > 0.
So in general, my original idea is to manage device runtime power state
automatically instead of manually, especially when device is in disabled
state.
disabled + forbidden -> ACTIVE
disabled + !forbidden -> SUSPENDED
enabled + forbidden -> ACTIVE
enabled + !forbidden -> auto
Why we can not do that?
> > - in pm_runtime_forbid/pm_runtime_enable, put device into ACTIVE state.
>
> Why should pm_runtime_enable put the device into the ACTIVE state?
>
> No, I think a better approach is simply to say that the device will
> never be allowed to be in the SUSPENDED state if runtime PM is
> forbidden. We already enforce this when the device is enabled for
> runtime PM, so we would have to start enforcing it when the device is
> disabled.
>
> This means:
>
> pm_runtime_set_suspended should fail if dev->power.runtime_auto
> is clear.
I think we can WARN_ON() here. Because the caller should responsible
for state consistence if they decide to manage runtime power state
manually.
> pm_runtime_forbid should call pm_runtime_set_active if
> dev->power.disable_depth > 0. (This would run into a problem
> if the parent is suspended and disabled. Maybe
> pm_runtime_forbid should fail when this happens.)
pm_runtime_forbid() may be called via echo "on" > .../power/control. I
think it is hard to refuse the request from user space to forbid runtime
PM. Device can always work with full power.
> Finally, we probably should make a third change even though it isn't
> strictly necessary:
>
> pm_runtime_allow should call pm_runtime_set_suspended if
> dev->power.disable_depth > 0.
I think this is something similar to manage device power state
automatically if disabled.
Best Regards,
Huang Ying
> Rafael, what do you think?
>
> Alan Stern
>
^ permalink raw reply
* Re: [BUGFIX] PM: Fix active child counting when disabled and forbidden
From: Alan Stern @ 2012-11-13 2:32 UTC (permalink / raw)
To: Huang Ying; +Cc: Rafael J. Wysocki, linux-kernel, linux-pm
In-Reply-To: <1352769596.7176.194.camel@yhuang-dev>
On Tue, 13 Nov 2012, Huang Ying wrote:
> Sorry, my original idea is:
>
> pm_runtime_disable will put device into SUSPENDED state if
> dev->power.runtime_auto is clear. pm_runtime_allow will put
> device into SUSPENDED state if dev->power.disable_depth > 0.
That's close to what I suggested.
> So in general, my original idea is to manage device runtime power state
> automatically instead of manually, especially when device is in disabled
> state.
>
> disabled + forbidden -> ACTIVE
> disabled + !forbidden -> SUSPENDED
This is not quite right. Consider a device that is in runtime suspend
when a system sleep starts. When the system sleep ends, the device
will be resumed but the PM core will still think its state is
SUSPENDED. The subsystem has to tell the PM core that the device is
now ACTIVE. Currently, subsystems do this by calling
pm_runtime_disable, pm_runtime_set_active, pm_runtime_enable. Under
your scheme this wouldn't work; the pm_runtime_set_active call would
fail because the device was !forbidden.
> enabled + forbidden -> ACTIVE
> enabled + !forbidden -> auto
>
> Why we can not do that?
See above. What we can do instead is:
disabled + forbidden -> ACTIVE
disabled + !forbidden -> anything
which is basically what I proposed.
> > This means:
> >
> > pm_runtime_set_suspended should fail if dev->power.runtime_auto
> > is clear.
>
> I think we can WARN_ON() here. Because the caller should responsible
> for state consistence if they decide to manage runtime power state
> manually.
No. Drivers should not have to worry about whether runtime PM is
forbidden. Worrying about that is the PM core's job.
> > pm_runtime_forbid should call pm_runtime_set_active if
> > dev->power.disable_depth > 0. (This would run into a problem
> > if the parent is suspended and disabled. Maybe
> > pm_runtime_forbid should fail when this happens.)
>
> pm_runtime_forbid() may be called via echo "on" > .../power/control. I
> think it is hard to refuse the request from user space to forbid runtime
> PM. Device can always work with full power.
It can't if the parent is in SUSPEND. If necessary, the user can write
"on" to the parent's power/control attribute first.
> > Finally, we probably should make a third change even though it isn't
> > strictly necessary:
> >
> > pm_runtime_allow should call pm_runtime_set_suspended if
> > dev->power.disable_depth > 0.
>
> I think this is something similar to manage device power state
> automatically if disabled.
Yes, it is similar but not exactly the same as your proposal.
Alan Stern
^ permalink raw reply
* Re: [BUGFIX] PM: Fix active child counting when disabled and forbidden
From: Huang Ying @ 2012-11-13 5:12 UTC (permalink / raw)
To: Alan Stern; +Cc: Rafael J. Wysocki, linux-kernel, linux-pm
In-Reply-To: <Pine.LNX.4.44L0.1211122120140.6961-100000@netrider.rowland.org>
On Mon, 2012-11-12 at 21:32 -0500, Alan Stern wrote:
> On Tue, 13 Nov 2012, Huang Ying wrote:
>
> > Sorry, my original idea is:
> >
> > pm_runtime_disable will put device into SUSPENDED state if
> > dev->power.runtime_auto is clear. pm_runtime_allow will put
> > device into SUSPENDED state if dev->power.disable_depth > 0.
>
> That's close to what I suggested.
>
> > So in general, my original idea is to manage device runtime power state
> > automatically instead of manually, especially when device is in disabled
> > state.
> >
> > disabled + forbidden -> ACTIVE
> > disabled + !forbidden -> SUSPENDED
>
> This is not quite right. Consider a device that is in runtime suspend
> when a system sleep starts. When the system sleep ends, the device
> will be resumed but the PM core will still think its state is
> SUSPENDED. The subsystem has to tell the PM core that the device is
> now ACTIVE. Currently, subsystems do this by calling
> pm_runtime_disable, pm_runtime_set_active, pm_runtime_enable. Under
> your scheme this wouldn't work; the pm_runtime_set_active call would
> fail because the device was !forbidden.
Thanks for your information. For this specific situation, is it
possible to call pm_runtime_resume() or pm_request_resume() for the
device?
> > enabled + forbidden -> ACTIVE
> > enabled + !forbidden -> auto
> >
> > Why we can not do that?
>
> See above. What we can do instead is:
>
> disabled + forbidden -> ACTIVE
> disabled + !forbidden -> anything
>
> which is basically what I proposed.
>
> > > This means:
> > >
> > > pm_runtime_set_suspended should fail if dev->power.runtime_auto
> > > is clear.
> >
> > I think we can WARN_ON() here. Because the caller should responsible
> > for state consistence if they decide to manage runtime power state
> > manually.
>
> No. Drivers should not have to worry about whether runtime PM is
> forbidden. Worrying about that is the PM core's job.
En... It appears that what caller can do is just do not call
pm_runtime_set_suspended() if forbidden. So your method should be
better.
> > > pm_runtime_forbid should call pm_runtime_set_active if
> > > dev->power.disable_depth > 0. (This would run into a problem
> > > if the parent is suspended and disabled. Maybe
> > > pm_runtime_forbid should fail when this happens.)
> >
> > pm_runtime_forbid() may be called via echo "on" > .../power/control. I
> > think it is hard to refuse the request from user space to forbid runtime
> > PM. Device can always work with full power.
>
> It can't if the parent is in SUSPEND. If necessary, the user can write
> "on" to the parent's power/control attribute first.
Is it possible to call pm_runtime_set_active() for the parent if the
parent is disabled and SUSPENDED.
> > > Finally, we probably should make a third change even though it isn't
> > > strictly necessary:
> > >
> > > pm_runtime_allow should call pm_runtime_set_suspended if
> > > dev->power.disable_depth > 0.
> >
> > I think this is something similar to manage device power state
> > automatically if disabled.
>
> Yes, it is similar but not exactly the same as your proposal.
It appears that there is race condition between this and the
pm_runtime_disable, pm_runtime_set_active, pm_runtime_enable sequence
you mentioned ealier.
thread 1 thread 2
pm_runtime_disable
pm_runtime_set_active
pm_runtime_allow
pm_runtime_set_suspended
pm_runtime_enable
Best Regards,
Huang Ying
^ permalink raw reply
* [PATCH 0/3] pm: Intel powerclamp driver
From: Jacob Pan @ 2012-11-12 22:03 UTC (permalink / raw)
To: Linux PM, LKML
Cc: Rafael Wysocki, Len Brown, Thomas Gleixner, H. Peter Anvin,
Ingo Molnar, Zhang Rui, Rob Landley, Arjan van de Ven,
Paul McKenney, Jacob Pan
Hi,
We have done some experiment with idle injection on Intel platforms.
The idea is to use the increasingly power efficient package level
C-states for power capping and passive thermal control.
Documentation is included in the patch to explain the theory of
operation, performance implication, calibration, scalability, and user
interface. Please refer to the following file for more details.
Documentation/thermal/intel_powerclamp.txt
Arjan van de Ven created the original idea and driver, I have been
refining driver in hope that they can be to be useful beyond a proof
of concept.
Jacob Pan (3):
tick: export nohz tick idle symbols for module use
x86/nmi: export local_touch_nmi() symbol for modules
PM: Introduce Intel PowerClamp Driver
Documentation/thermal/intel_powerclamp.txt | 307 +++++++++++
arch/x86/kernel/nmi.c | 1 +
drivers/thermal/Kconfig | 10 +
drivers/thermal/Makefile | 1 +
drivers/thermal/intel_powerclamp.c | 766 ++++++++++++++++++++++++++++
kernel/time/tick-sched.c | 2 +
6 files changed, 1087 insertions(+)
create mode 100644 Documentation/thermal/intel_powerclamp.txt
create mode 100644 drivers/thermal/intel_powerclamp.c
--
1.7.9.5
^ permalink raw reply
* Re: [PATCH 3/3] PM: Introduce Intel PowerClamp Driver
From: Joe Perches @ 2012-11-13 6:33 UTC (permalink / raw)
To: Jacob Pan
Cc: Linux PM, LKML, Rafael Wysocki, Len Brown, Thomas Gleixner,
H. Peter Anvin, Ingo Molnar, Zhang Rui, Rob Landley,
Arjan van de Ven, Paul McKenney
In-Reply-To: <1352757831-5202-4-git-send-email-jacob.jun.pan@linux.intel.com>
On Mon, 2012-11-12 at 14:03 -0800, Jacob Pan wrote:
> Intel PowerClamp driver performs synchronized idle injection across
> all online CPUs. The goal is to maintain a given package level C-state
> ratio.
style trivia:
[]
> diff --git a/drivers/thermal/intel_powerclamp.c b/drivers/thermal/intel_powerclamp.c
[]
> +
> +/* #define DEBUG */
> +
Adding this #define before any #include
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +#include <linux/module.h>
> +#include <linux/kernel.h>
[]
> +static int window_size_set(const char *arg, const struct kernel_param *kp)
> +{
[]
> + if (new_window_size >= 10 || new_window_size < 2) {
> + pr_err("PowerClamp: invalid window size %lu, between 2-10\n",
> + new_window_size);
Means there's no need for "PowerClamp: " prefixes with pr_fmt
pr_err("invalid window size %lu...
and all the other pr_<level> uses get prefixed too.
> +static u64 pkg_state_counter(void)
> +{
> + u64 val;
> + u64 count = 0;
> +
> + static int skip_c2;
> + static int skip_c3;
> + static int skip_c6;
> + static int skip_c7;
bool?
^ permalink raw reply
* Re: [PATCH 3/3] PM: Introduce Intel PowerClamp Driver
From: Jacob Pan @ 2012-11-13 6:55 UTC (permalink / raw)
To: Joe Perches
Cc: Linux PM, LKML, Rafael Wysocki, Len Brown, Thomas Gleixner,
H. Peter Anvin, Ingo Molnar, Zhang Rui, Rob Landley,
Arjan van de Ven, Paul McKenney
In-Reply-To: <1352788416.24230.6.camel@joe-AO722>
On Mon, 12 Nov 2012 22:33:36 -0800
Joe Perches <joe@perches.com> wrote:
> > Intel PowerClamp driver performs synchronized idle injection across
> > all online CPUs. The goal is to maintain a given package level
> > C-state ratio.
>
> style trivia:
they are all good catches. will fix in the next version.
--
Thanks,
Jacob
^ permalink raw reply
* [PATCH] cpufreq: exynos: Broadcast frequency change notifications for all cores
From: Tomasz Figa @ 2012-11-13 9:26 UTC (permalink / raw)
To: linux-pm
Cc: linux-arm-kernel, linux-samsung-soc, Kyungmin Park, Kukjin Kim,
Marek Szyprowski, Bartlomiej Zolnierkiewicz, Rafael J. Wysocki
On Exynos SoCs all cores share the same frequency setting, so changing
frequency of one core will affect rest of cores.
This patch modifies the exynos-cpufreq driver to inform cpufreq core
about this behavior and broadcast frequency change notifications for all
cores.
Signed-off-by: Tomasz Figa <t.figa@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
drivers/cpufreq/exynos-cpufreq.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/cpufreq/exynos-cpufreq.c b/drivers/cpufreq/exynos-cpufreq.c
index af2d81e..c0d54a8 100644
--- a/drivers/cpufreq/exynos-cpufreq.c
+++ b/drivers/cpufreq/exynos-cpufreq.c
@@ -100,7 +100,8 @@ static int exynos_target(struct cpufreq_policy *policy,
}
arm_volt = volt_table[index];
- cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
+ for_each_cpu(freqs.cpu, policy->cpus)
+ cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
/* When the new frequency is higher than current frequency */
if ((freqs.new > freqs.old) && !safe_arm_volt) {
@@ -115,7 +116,8 @@ static int exynos_target(struct cpufreq_policy *policy,
if (freqs.new != freqs.old)
exynos_info->set_freq(old_index, index);
- cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
+ for_each_cpu(freqs.cpu, policy->cpus)
+ cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
/* When the new frequency is lower than current frequency */
if ((freqs.new < freqs.old) ||
@@ -235,6 +237,7 @@ static int exynos_cpufreq_cpu_init(struct cpufreq_policy *policy)
cpumask_copy(policy->related_cpus, cpu_possible_mask);
cpumask_copy(policy->cpus, cpu_online_mask);
} else {
+ policy->shared_type = CPUFREQ_SHARED_TYPE_ANY;
cpumask_setall(policy->cpus);
}
--
1.8.0
^ permalink raw reply related
* Re: [PATCH v9 05/10] libata: separate ATAPI code
From: Aaron Lu @ 2012-11-13 12:49 UTC (permalink / raw)
To: Tejun Heo
Cc: Jeff Garzik, James Bottomley, Rafael J. Wysocki, Alan Stern,
Jeff Wu, Aaron Lu, linux-ide, linux-pm, linux-scsi, linux-acpi
In-Reply-To: <20121112185710.GD5560@mtj.dyndns.org>
On Mon, Nov 12, 2012 at 10:57:10AM -0800, Tejun Heo wrote:
> On Fri, Nov 09, 2012 at 02:51:57PM +0800, Aaron Lu wrote:
> > The atapi_eh_tur and atapi_eh_request_sense can be reused by ZPODD
> > code, so separate them out to a file named libata-atapi.c, and the
> > Makefile is modified accordingly. No functional changes should result
> > from this commit.
>
> Why is this change necessary? This doesn't seem to help anything to
> me.
Function zpready introduced in patch 6 used these 2 functions to see if
an ODD is in a zero power ready state.
Thanks,
Aaron
>
> Thanks.
>
> --
> tejun
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox