headers
Lorenzo Pieralisi | 1 May 2012 12:43
Favicon

Re: [PATCHv3 0/5] coupled cpuidle state support

Hi Colin,

On Mon, Apr 30, 2012 at 10:37:30PM +0100, Colin Cross wrote:
> On Mon, Apr 30, 2012 at 2:25 PM, Rafael J. Wysocki <rjw <at> sisk.pl> wrote:
> > Hi,
> >
> > I have a comment, which isn't about the series itself, but something
> > thay may be worth thinking about.
> >
> > On Monday, April 30, 2012, Colin Cross wrote:
> >> On some ARM SMP SoCs (OMAP4460, Tegra 2, and probably more), the
> >> cpus cannot be independently powered down, either due to
> >> sequencing restrictions (on Tegra 2, cpu 0 must be the last to
> >> power down), or due to HW bugs (on OMAP4460, a cpu powering up
> >> will corrupt the gic state unless the other cpu runs a work
> >> around).  Each cpu has a power state that it can enter without
> >> coordinating with the other cpu (usually Wait For Interrupt, or
> >> WFI), and one or more "coupled" power states that affect blocks
> >> shared between the cpus (L2 cache, interrupt controller, and
> >> sometimes the whole SoC).  Entering a coupled power state must
> >> be tightly controlled on both cpus.
> >
> > That seems to be a special case of a more general situation where
> > a number of CPU cores belong into a single power domain, possibly along
> > some I/O devices.
> >
> > We'll need to handle the general case at one point anyway, so I wonder if
> > the approach shown here may get us in the way?
> 
> I can't parse what you're saying here.
(Continue reading)

Permalink | Reply | 2 comments |
headers
Adrian Fita | 1 May 2012 21:00
Picon

Re: [bug?] Battery notifications produce flashing battery icon, syslog spam (Re: [PATCH 11/11] ACPI / Battery: Update information on info notification and resume)

On 01/05/12 21:47, Jonathan Nieder wrote:
 >
> [...]
 >
> [...]  More background
> at [2].
>
> [2] http://bugs.debian.org/670958

Also, searching on Google after "upowerd device 
removed:/org/freedesktop/UPower/devices/battery_BAT0", reveals much more 
bug reports with the exact issue.

Thanks,
--

-- 
Fita Adrian
Permalink | Reply | 9 comments |
headers
Colin Cross | 2 May 2012 02:11
Favicon

Re: [PATCHv3 0/5] coupled cpuidle state support

On Tue, May 1, 2012 at 3:43 AM, Lorenzo Pieralisi
<lorenzo.pieralisi <at> arm.com> wrote:
> Hi Colin,
>
> On Mon, Apr 30, 2012 at 10:37:30PM +0100, Colin Cross wrote:
<snip>

>> On Tegra3, the deepest individual cpu state for cpus 1-3 is OFF, the
>> same state the cpu would go into as the first step of a transition to
>> a deeper power state (cpus 0-3 OFF).  It would be more optimal in that
>> case to bypass the SMP cross call, and leave the cpu in OFF, but that
>> would require some way of disabling all wakeups for the secondary cpus
>> and then verifying that they didn't start waking up just before the
>> wakeups were disabled.  I have just started considering this
>> optimization, but I don't see anything in the existing code that would
>> prevent adding it later.
>
> I agree it is certainly an optimization that can be added later if benchmarks
> show it is needed (but again it is heavily platform dependent, ie technology
> dependent).
> On a side note, disabling (or move to the primary) wake-ups for "secondaries"
> on platforms where every core is in a different power domain is still needed
> to avoid having a situation where a CPU can independently get out of idle, ie
> abort idle, after hitting the coupled barrier.
> Still do not know if for those platforms coupled C-states should be used, but
> it is much better to have a choice there IMHO.

Yes, that is the primary need for the coupled_cpuidle_parallel_barrier
function - secondary cpus need to disable their wakeup sources, then
check that a wakeup was not already pending and abort if necessary.
(Continue reading)

Permalink | Reply | 1 comment |
headers
Santosh Shilimkar | 2 May 2012 09:22
Picon
Favicon

Re: [PATCHv3 0/5] coupled cpuidle state support

On Wednesday 02 May 2012 05:41 AM, Colin Cross wrote:
> On Tue, May 1, 2012 at 3:43 AM, Lorenzo Pieralisi
> <lorenzo.pieralisi <at> arm.com> wrote:
>> Hi Colin,
>>
>> On Mon, Apr 30, 2012 at 10:37:30PM +0100, Colin Cross wrote:
> <snip>
> 
>>> On Tegra3, the deepest individual cpu state for cpus 1-3 is OFF, the
>>> same state the cpu would go into as the first step of a transition to
>>> a deeper power state (cpus 0-3 OFF).  It would be more optimal in that
>>> case to bypass the SMP cross call, and leave the cpu in OFF, but that
>>> would require some way of disabling all wakeups for the secondary cpus
>>> and then verifying that they didn't start waking up just before the
>>> wakeups were disabled.  I have just started considering this
>>> optimization, but I don't see anything in the existing code that would
>>> prevent adding it later.
>>
I was also looking at how we can avoid the un-necessary wakeup on
secondary CPUs if the timer event is not for those CPUs. As you
rightly said, we can add all the optimisations once we have the
base patches merged.

>> I agree it is certainly an optimisation that can be added later if benchmarks
>> show it is needed (but again it is heavily platform dependent, ie technology
>> dependent).
>> On a side note, disabling (or move to the primary) wake-ups for "secondaries"
>> on platforms where every core is in a different power domain is still needed
>> to avoid having a situation where a CPU can independently get out of idle, ie
>> abort idle, after hitting the coupled barrier.
(Continue reading)

Permalink | Reply | 0 comments |
headers
mark gross | 2 May 2012 16:36

Plumbers: PM constraints micro-conf RFP

This is a Request For Participation in a micro-conference at this years
Linux plumbers event.  For this micro conference to happen we need to
reach a certain critical mass WRT participants as measured by submitted
talks associated to Power Management Constraints.

The To: list is populated with folks that I've had interactions with
over extending pm-qos or constraint based PM over the past year.

If you are working on problems related to constraining the power /
performance of devices I am inviting your participation and request you
submit a proposal for presenting your problem space (preferred) and or
solution to a group of developers looking for a good solution to push
upstream.  The talks will be about 20 min long as I want to get into
some design and implementation discussions after the requirements
definition is mostly finished.

I am interested in gathering user mode interface needs as well as kernel
mode.

From a high level pm-constraints is a generalization of pm-qos to include
limiting performance as well as its current limiting of device
throttling.

As performance limiting is typically used for any of the following:
1) staying within thermal operational envelopes
2) avoiding peak current
3) extending battery life in active use cases

I invite anyone working in any of these areas or pm-qos applications to
participate in this micro-conference.
(Continue reading)

Permalink | Reply | 10 comments |
headers
Rafael J. Wysocki | 3 May 2012 22:00
Picon
Gravatar

Re: [PATCHv3 0/5] coupled cpuidle state support

On Tuesday, May 01, 2012, Colin Cross wrote:
> On Mon, Apr 30, 2012 at 2:54 PM, Rafael J. Wysocki <rjw <at> sisk.pl> wrote:
> > On Monday, April 30, 2012, Colin Cross wrote:
> >> On Mon, Apr 30, 2012 at 2:25 PM, Rafael J. Wysocki <rjw <at> sisk.pl> wrote:
> >> > Hi,
> >> >
> >> > I have a comment, which isn't about the series itself, but something
> >> > thay may be worth thinking about.
> >> >
> >> > On Monday, April 30, 2012, Colin Cross wrote:
> >> >> On some ARM SMP SoCs (OMAP4460, Tegra 2, and probably more), the
> >> >> cpus cannot be independently powered down, either due to
> >> >> sequencing restrictions (on Tegra 2, cpu 0 must be the last to
> >> >> power down), or due to HW bugs (on OMAP4460, a cpu powering up
> >> >> will corrupt the gic state unless the other cpu runs a work
> >> >> around).  Each cpu has a power state that it can enter without
> >> >> coordinating with the other cpu (usually Wait For Interrupt, or
> >> >> WFI), and one or more "coupled" power states that affect blocks
> >> >> shared between the cpus (L2 cache, interrupt controller, and
> >> >> sometimes the whole SoC).  Entering a coupled power state must
> >> >> be tightly controlled on both cpus.
> >> >
> >> > That seems to be a special case of a more general situation where
> >> > a number of CPU cores belong into a single power domain, possibly along
> >> > some I/O devices.
> >> >
> >> > We'll need to handle the general case at one point anyway, so I wonder if
> >> > the approach shown here may get us in the way?
> >>
> >> I can't parse what you're saying here.
(Continue reading)

Permalink | Reply | 3 comments |
headers
Colin Cross | 3 May 2012 22:18
Favicon

Re: [PATCHv3 0/5] coupled cpuidle state support

On Thu, May 3, 2012 at 1:00 PM, Rafael J. Wysocki <rjw <at> sisk.pl> wrote:
<snip>
> There are two distinct cases to consider here, (1) when the last I/O
> device in the domain becomes idle and the question is whether or not to
> power off the entire domain and (2) when a CPU core in a power domain
> becomes idle while all of the devices in the domain are idle already.
>
> Case (2) is quite straightforward, the .enter() routine for the
> "domain" C-state has to check whether the domain can be turned off and
> do it eventually.
>
> Case (1) is more difficult and (assuming that all CPU cores in the domain
> are already idle at this point) i see two possible ways to handle it:
> (a) Wake up all of the (idle) CPU cores in the domain and let the
>  "domain" C-state's .enter() do the job (ie. turn it into case (2)),
>  similarly to your patchset.
> (b) If cpuidle has prepared the cores for going into deeper idle,
>  turn the domain off directly without waking up the cores.

Multiple clusters is a design that has been considered in this
patchset (all the data structures are in the right place to support
it), and can be supported in the future, but does not exist in any
current systems that would be using this.  In all of today's SoCs,
there is a single cluster, so (1) can't happen - no code can be
executing while all cpus are idle.

(b) is an optimization that would not be possible on any future SoC
that is similar to the current SoCs, where "turn the domain off" is
very tightly integrated with TrustZone secure code running on the
primary cpu of the cluster.
(Continue reading)

Permalink | Reply | 1 comment |
headers
Rafael J. Wysocki | 3 May 2012 22:43
Picon
Gravatar

Re: [PATCHv3 0/5] coupled cpuidle state support

On Thursday, May 03, 2012, Colin Cross wrote:
> On Thu, May 3, 2012 at 1:00 PM, Rafael J. Wysocki <rjw <at> sisk.pl> wrote:
> <snip>
> > There are two distinct cases to consider here, (1) when the last I/O
> > device in the domain becomes idle and the question is whether or not to
> > power off the entire domain and (2) when a CPU core in a power domain
> > becomes idle while all of the devices in the domain are idle already.
> >
> > Case (2) is quite straightforward, the .enter() routine for the
> > "domain" C-state has to check whether the domain can be turned off and
> > do it eventually.
> >
> > Case (1) is more difficult and (assuming that all CPU cores in the domain
> > are already idle at this point) i see two possible ways to handle it:
> > (a) Wake up all of the (idle) CPU cores in the domain and let the
> >  "domain" C-state's .enter() do the job (ie. turn it into case (2)),
> >  similarly to your patchset.
> > (b) If cpuidle has prepared the cores for going into deeper idle,
> >  turn the domain off directly without waking up the cores.
> 
> Multiple clusters is a design that has been considered in this
> patchset (all the data structures are in the right place to support
> it), and can be supported in the future, but does not exist in any
> current systems that would be using this.  In all of today's SoCs,
> there is a single cluster, so (1) can't happen - no code can be
> executing while all cpus are idle.

OK, but I think it should be taken into consideration nonetheless.

> (b) is an optimization that would not be possible on any future SoC
(Continue reading)

Permalink | Reply | 0 comments |
headers
Rafael J. Wysocki | 3 May 2012 22:50
Picon
Gravatar

Re: [PATCHv3 1/5] cpuidle: refactor out cpuidle_enter_state

On Monday, April 30, 2012, Colin Cross wrote:
> Split the code to enter a state and update the stats into a helper
> function, cpuidle_enter_state, and export it.  This function will
> be called by the coupled state code to handle entering the safe
> state and the final coupled state.
> 
> Reviewed-by: Santosh Shilimkar <santosh.shilimkar <at> ti.com>
> Tested-by: Santosh Shilimkar <santosh.shilimkar <at> ti.com>
> Reviewed-by: Kevin Hilman <khilman <at> ti.com>
> Tested-by: Kevin Hilman <khilman <at> ti.com>
> Signed-off-by: Colin Cross <ccross <at> android.com>

Reviewed-by: Rafael J. Wysocki <rjw <at> sisk.pl>

> ---
>  drivers/cpuidle/cpuidle.c |   42 +++++++++++++++++++++++++++++-------------
>  drivers/cpuidle/cpuidle.h |    2 ++
>  2 files changed, 31 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 2f0083a..3e3e3e4 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
>  <at>  <at>  -103,6 +103,34  <at>  <at>  int cpuidle_play_dead(void)
>  }
>  
>  /**
> + * cpuidle_enter_state - enter the state and update stats
> + *  <at> dev: cpuidle device for this cpu
> + *  <at> drv: cpuidle driver for this cpu
(Continue reading)

Permalink | Reply | 0 comments |
headers
Rafael J. Wysocki | 3 May 2012 22:50
Picon
Gravatar

Re: [PATCHv3 2/5] cpuidle: fix error handling in __cpuidle_register_device

On Monday, April 30, 2012, Colin Cross wrote:
> Fix the error handling in __cpuidle_register_device to include
> the missing list_del.  Move it to a label, which will simplify
> the error handling when coupled states are added.
> 
> Reviewed-by: Santosh Shilimkar <santosh.shilimkar <at> ti.com>
> Tested-by: Santosh Shilimkar <santosh.shilimkar <at> ti.com>
> Reviewed-by: Kevin Hilman <khilman <at> ti.com>
> Tested-by: Kevin Hilman <khilman <at> ti.com>
> Signed-off-by: Colin Cross <ccross <at> android.com>

Reviewed-by: Rafael J. Wysocki <rjw <at> sisk.pl>

> ---
>  drivers/cpuidle/cpuidle.c |   13 +++++++++----
>  1 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 3e3e3e4..4540672 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
>  <at>  <at>  -403,13 +403,18  <at>  <at>  static int __cpuidle_register_device(struct cpuidle_device *dev)
>  
>  	per_cpu(cpuidle_devices, dev->cpu) = dev;
>  	list_add(&dev->device_list, &cpuidle_detected_devices);
> -	if ((ret = cpuidle_add_sysfs(cpu_dev))) {
> -		module_put(cpuidle_driver->owner);
> -		return ret;
> -	}
> +	ret = cpuidle_add_sysfs(cpu_dev);
(Continue reading)

Permalink | Reply | 0 comments |

Gmane