Re: [PATCH 0/6] cpuidle : per cpu latencies

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Friday, September 07, 2012, Daniel Lezcano wrote:
> Since commit 46bcfad7a819bd17ac4e831b04405152d59784ab,
>         cpuidle: Single/Global registration of idle states
> 
> we have a single registration for the cpuidle states which makes
> sense. But now two new architectures are coming: tegra3 and big.LITTLE.
> 
> These architectures have different cpus with different caracteristics
> for power saving. High load => powerfull processors, idle => small processors.
> 
> That implies different cpu latencies.
> 
> This patchset keeps the current behavior as introduced by Deepthi without
> breaking the drivers and add the possibility to specify a per cpu states.
> 
>  * Tested on intel core 2 duo T9500
>  * Tested on vexpress by Lorenzo Pieralsi
>  * Tested on tegra3 by Peter De Schrijver
> 
> Daniel Lezcano (6):
>   acpi : move the acpi_idle_driver variable declaration
>   acpi : move cpuidle_device field out of the acpi_processor_power
>     structure
>   acpi : remove pointless cpuidle device state_count init

I've posted comments about patches [1-3/6] already.  In short, I don't like
[1/6], [2/6] would require some more work IMO and I'm not sure about the
validity of the observation that [3/6] is based on.

Yes, I agree that the ACPI processor driver as a whole might be cleaner
and it probably would be good to spend some time on cleaning it up, but
not necessarily in a hurry.

Unfortunately, I also don't agree with the approach used by the remaining
patches, which is to try to use a separate array of states for each
individual CPU core.  This way we end up with quite some duplicated data
if the CPU cores in question actually happen to be identical.

What about using a separate cpuidle driver for every kind of different CPUs in
the system (e.g. one driver for "big" CPUs and the other for "little" ones)?

Have you considered this approach already?

>   cpuidle : add a pointer for cpuidle_state in the cpuidle_device
>   cpuidle : use per cpuidle device cpu states
>   cpuidle : add cpuidle_register_states function
> 
>  drivers/acpi/processor_driver.c    |    2 +-
>  drivers/acpi/processor_idle.c      |   27 +++++++++++++++-------
>  drivers/cpuidle/cpuidle.c          |   42 ++++++++++++++++++++++++++++-------
>  drivers/cpuidle/governors/ladder.c |   22 +++++++++---------
>  drivers/cpuidle/governors/menu.c   |    8 +++---
>  drivers/cpuidle/sysfs.c            |    3 +-
>  include/acpi/processor.h           |    3 --
>  include/linux/cpuidle.h            |   11 ++++++--
>  8 files changed, 76 insertions(+), 42 deletions(-)

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux