* [PATCH v2 1/5] ACPI / processor : add support for ACPI0010 processor container
2015-09-16 13:59 ` [PATCH v2 0/5] ACPI / processor_idle: Add ACPIv6.0 LPI support Sudeep Holla
@ 2015-09-16 13:59 ` Sudeep Holla
2015-09-16 13:59 ` [PATCH v2 2/5] ACPI / sleep: move acpi_processor_sleep to sleep.c Sudeep Holla
` (3 subsequent siblings)
4 siblings, 0 replies; 20+ messages in thread
From: Sudeep Holla @ 2015-09-16 13:59 UTC (permalink / raw)
To: linux-acpi, Rafael J. Wysocki
Cc: Sudeep Holla, linux-kernel, linux-ia64, x86, Al Stone,
Lorenzo Pieralisi, Mahesh Sivasubramanian, Ashwin Chaugule
ACPI 6.0 adds support for optional processor container device which may
contain child objects that are either processor devices or other processor
containers. This allows representing hierarchical processor topologies.
It is declared using the _HID of ACPI0010. It is an abstract container
used to represent CPU topology and should not be used to hotplug
purposes.
This patch enables the support for these ACPI processor containers and
ensures the generic container/module devices are not created for them.
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
drivers/acpi/acpi_processor.c | 17 +++++++++++++++++
include/acpi/processor.h | 1 +
2 files changed, 18 insertions(+)
diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
index 985b8a83184e..6df6c0270cb1 100644
--- a/drivers/acpi/acpi_processor.c
+++ b/drivers/acpi/acpi_processor.c
@@ -496,7 +496,24 @@ static struct acpi_scan_handler processor_handler = {
},
};
+static int acpi_processor_container_attach(struct acpi_device *dev,
+ const struct acpi_device_id *id)
+{
+ return 1;
+}
+
+static const struct acpi_device_id processor_container_ids[] = {
+ { ACPI_PROCESSOR_CONTAINER_HID, },
+ { }
+};
+
+static struct acpi_scan_handler processor_container_handler = {
+ .ids = processor_container_ids,
+ .attach = acpi_processor_container_attach,
+};
+
void __init acpi_processor_init(void)
{
acpi_scan_add_handler_with_hotplug(&processor_handler, "processor");
+ acpi_scan_add_handler(&processor_container_handler);
}
diff --git a/include/acpi/processor.h b/include/acpi/processor.h
index ff5f135f16b1..51b61dcdbd9c 100644
--- a/include/acpi/processor.h
+++ b/include/acpi/processor.h
@@ -9,6 +9,7 @@
#define ACPI_PROCESSOR_CLASS "processor"
#define ACPI_PROCESSOR_DEVICE_NAME "Processor"
#define ACPI_PROCESSOR_DEVICE_HID "ACPI0007"
+#define ACPI_PROCESSOR_CONTAINER_HID "ACPI0010"
#define ACPI_PROCESSOR_BUSY_METRIC 10
--
1.9.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v2 2/5] ACPI / sleep: move acpi_processor_sleep to sleep.c
2015-09-16 13:59 ` [PATCH v2 0/5] ACPI / processor_idle: Add ACPIv6.0 LPI support Sudeep Holla
2015-09-16 13:59 ` [PATCH v2 1/5] ACPI / processor : add support for ACPI0010 processor container Sudeep Holla
@ 2015-09-16 13:59 ` Sudeep Holla
2015-09-16 13:59 ` [PATCH v2 3/5] ACPI / processor_idle: replace PREFIX with pr_fmt Sudeep Holla
` (2 subsequent siblings)
4 siblings, 0 replies; 20+ messages in thread
From: Sudeep Holla @ 2015-09-16 13:59 UTC (permalink / raw)
To: linux-acpi, Rafael J. Wysocki
Cc: Sudeep Holla, linux-kernel, linux-ia64, x86, Al Stone,
Lorenzo Pieralisi, Mahesh Sivasubramanian, Ashwin Chaugule
acpi_processor_sleep is neither related nor used by CPUIdle framework.
It's used in system suspend/resume path as a syscore operation. It makes
more sense to move it to acpi/sleep.c where all the S-state transition
(a.k.a. Linux system suspend/hiberate) related code are present.
Also make it depend on CONFIG_ACPI_SYSTEM_POWER_STATES_SUPPORT so that
it's not compiled on architecture like ARM64 where S-states are not
yet defined in ACPI.
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
drivers/acpi/processor_driver.c | 2 --
drivers/acpi/processor_idle.c | 37 -------------------------------------
drivers/acpi/sleep.c | 35 +++++++++++++++++++++++++++++++++++
include/acpi/processor.h | 8 --------
4 files changed, 35 insertions(+), 47 deletions(-)
diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
index 51e658f21e95..dff584a7137b 100644
--- a/drivers/acpi/processor_driver.c
+++ b/drivers/acpi/processor_driver.c
@@ -307,7 +307,6 @@ static int __init acpi_processor_driver_init(void)
if (result < 0)
return result;
- acpi_processor_syscore_init();
register_hotcpu_notifier(&acpi_cpu_notifier);
acpi_thermal_cpufreq_init();
acpi_processor_ppc_init();
@@ -323,7 +322,6 @@ static void __exit acpi_processor_driver_exit(void)
acpi_processor_ppc_exit();
acpi_thermal_cpufreq_exit();
unregister_hotcpu_notifier(&acpi_cpu_notifier);
- acpi_processor_syscore_exit();
driver_unregister(&acpi_processor_driver);
}
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 175c86bee3a9..a24067b850d2 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -30,7 +30,6 @@
#include <linux/sched.h> /* need_resched() */
#include <linux/tick.h>
#include <linux/cpuidle.h>
-#include <linux/syscore_ops.h>
#include <acpi/processor.h>
/*
@@ -194,42 +193,6 @@ static void lapic_timer_state_broadcast(struct acpi_processor *pr,
#endif
-#ifdef CONFIG_PM_SLEEP
-static u32 saved_bm_rld;
-
-static int acpi_processor_suspend(void)
-{
- acpi_read_bit_register(ACPI_BITREG_BUS_MASTER_RLD, &saved_bm_rld);
- return 0;
-}
-
-static void acpi_processor_resume(void)
-{
- u32 resumed_bm_rld = 0;
-
- acpi_read_bit_register(ACPI_BITREG_BUS_MASTER_RLD, &resumed_bm_rld);
- if (resumed_bm_rld == saved_bm_rld)
- return;
-
- acpi_write_bit_register(ACPI_BITREG_BUS_MASTER_RLD, saved_bm_rld);
-}
-
-static struct syscore_ops acpi_processor_syscore_ops = {
- .suspend = acpi_processor_suspend,
- .resume = acpi_processor_resume,
-};
-
-void acpi_processor_syscore_init(void)
-{
- register_syscore_ops(&acpi_processor_syscore_ops);
-}
-
-void acpi_processor_syscore_exit(void)
-{
- unregister_syscore_ops(&acpi_processor_syscore_ops);
-}
-#endif /* CONFIG_PM_SLEEP */
-
#if defined(CONFIG_X86)
static void tsc_check_state(int state)
{
diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index 2f0d4db40a9e..b920ada49ae9 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -19,6 +19,7 @@
#include <linux/reboot.h>
#include <linux/acpi.h>
#include <linux/module.h>
+#include <linux/syscore_ops.h>
#include <asm/io.h>
#include <trace/events/power.h>
@@ -672,6 +673,39 @@ static void acpi_sleep_suspend_setup(void)
static inline void acpi_sleep_suspend_setup(void) {}
#endif /* !CONFIG_SUSPEND */
+#ifdef CONFIG_PM_SLEEP
+static u32 saved_bm_rld;
+
+static int acpi_processor_suspend(void)
+{
+ acpi_read_bit_register(ACPI_BITREG_BUS_MASTER_RLD, &saved_bm_rld);
+ return 0;
+}
+
+static void acpi_processor_resume(void)
+{
+ u32 resumed_bm_rld = 0;
+
+ acpi_read_bit_register(ACPI_BITREG_BUS_MASTER_RLD, &resumed_bm_rld);
+ if (resumed_bm_rld == saved_bm_rld)
+ return;
+
+ acpi_write_bit_register(ACPI_BITREG_BUS_MASTER_RLD, saved_bm_rld);
+}
+
+static struct syscore_ops acpi_processor_syscore_ops = {
+ .suspend = acpi_processor_suspend,
+ .resume = acpi_processor_resume,
+};
+
+void acpi_processor_syscore_init(void)
+{
+ register_syscore_ops(&acpi_processor_syscore_ops);
+}
+#else
+static inline void acpi_processor_syscore_init(void) {}
+#endif /* CONFIG_PM_SLEEP */
+
#ifdef CONFIG_HIBERNATION
static unsigned long s4_hardware_signature;
static struct acpi_table_facs *facs;
@@ -834,6 +868,7 @@ int __init acpi_sleep_init(void)
sleep_states[ACPI_STATE_S0] = 1;
+ acpi_processor_syscore_init();
acpi_sleep_suspend_setup();
acpi_sleep_hibernate_setup();
diff --git a/include/acpi/processor.h b/include/acpi/processor.h
index 51b61dcdbd9c..0abcab53dbe5 100644
--- a/include/acpi/processor.h
+++ b/include/acpi/processor.h
@@ -381,14 +381,6 @@ static inline int acpi_processor_hotplug(struct acpi_processor *pr)
}
#endif /* CONFIG_ACPI_PROCESSOR_IDLE */
-#if defined(CONFIG_PM_SLEEP) & defined(CONFIG_ACPI_PROCESSOR_IDLE)
-void acpi_processor_syscore_init(void);
-void acpi_processor_syscore_exit(void);
-#else
-static inline void acpi_processor_syscore_init(void) {}
-static inline void acpi_processor_syscore_exit(void) {}
-#endif
-
/* in processor_thermal.c */
int acpi_processor_get_limit_info(struct acpi_processor *pr);
extern const struct thermal_cooling_device_ops processor_cooling_ops;
--
1.9.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v2 3/5] ACPI / processor_idle: replace PREFIX with pr_fmt
2015-09-16 13:59 ` [PATCH v2 0/5] ACPI / processor_idle: Add ACPIv6.0 LPI support Sudeep Holla
2015-09-16 13:59 ` [PATCH v2 1/5] ACPI / processor : add support for ACPI0010 processor container Sudeep Holla
2015-09-16 13:59 ` [PATCH v2 2/5] ACPI / sleep: move acpi_processor_sleep to sleep.c Sudeep Holla
@ 2015-09-16 13:59 ` Sudeep Holla
2015-09-16 13:59 ` [PATCH v2 4/5] ACPI / processor_idle : introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE Sudeep Holla
2015-09-16 13:59 ` [PATCH v2 5/5] ACPI / processor_idle: Add support for Low Power Idle(LPI) states Sudeep Holla
4 siblings, 0 replies; 20+ messages in thread
From: Sudeep Holla @ 2015-09-16 13:59 UTC (permalink / raw)
To: linux-acpi, Rafael J. Wysocki
Cc: Sudeep Holla, linux-kernel, linux-ia64, x86, Al Stone,
Lorenzo Pieralisi, Mahesh Sivasubramanian, Ashwin Chaugule
Like few of the other ACPI modules, replace PREFIX with pr_fmt and
change all the printk call sites to use pr_* companion functions
in processor_idle.
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
drivers/acpi/processor_idle.c | 25 +++++++++++--------------
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index a24067b850d2..fadce354d2b7 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -23,6 +23,7 @@
*
* ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
*/
+#define pr_fmt(fmt) "ACPI: " fmt
#include <linux/module.h>
#include <linux/acpi.h>
@@ -42,8 +43,6 @@
#include <asm/apic.h>
#endif
-#define PREFIX "ACPI: "
-
#define ACPI_PROCESSOR_CLASS "processor"
#define _COMPONENT ACPI_PROCESSOR_COMPONENT
ACPI_MODULE_NAME("processor_idle");
@@ -80,9 +79,9 @@ static int set_max_cstate(const struct dmi_system_id *id)
if (max_cstate > ACPI_PROCESSOR_MAX_POWER)
return 0;
- printk(KERN_NOTICE PREFIX "%s detected - limiting to C%ld max_cstate."
- " Override with \"processor.max_cstate=%d\"\n", id->ident,
- (long)id->driver_data, ACPI_PROCESSOR_MAX_POWER + 1);
+ pr_notice("%s detected - limiting to C%ld max_cstate."
+ " Override with \"processor.max_cstate=%d\"\n", id->ident,
+ (long)id->driver_data, ACPI_PROCESSOR_MAX_POWER + 1);
max_cstate = (long)id->driver_data;
@@ -314,7 +313,7 @@ static int acpi_processor_get_power_info_cst(struct acpi_processor *pr)
/* There must be at least 2 elements */
if (!cst || (cst->type != ACPI_TYPE_PACKAGE) || cst->package.count < 2) {
- printk(KERN_ERR PREFIX "not enough elements in _CST\n");
+ pr_err("not enough elements in _CST\n");
ret = -EFAULT;
goto end;
}
@@ -323,7 +322,7 @@ static int acpi_processor_get_power_info_cst(struct acpi_processor *pr)
/* Validate number of power states. */
if (count < 1 || count != cst->package.count - 1) {
- printk(KERN_ERR PREFIX "count given by _CST is not valid\n");
+ pr_err("count given by _CST is not valid\n");
ret = -EFAULT;
goto end;
}
@@ -432,11 +431,9 @@ static int acpi_processor_get_power_info_cst(struct acpi_processor *pr)
* (From 1 through ACPI_PROCESSOR_MAX_POWER - 1)
*/
if (current_count >= (ACPI_PROCESSOR_MAX_POWER - 1)) {
- printk(KERN_WARNING
- "Limiting number of power states to max (%d)\n",
- ACPI_PROCESSOR_MAX_POWER);
- printk(KERN_WARNING
- "Please increase ACPI_PROCESSOR_MAX_POWER if needed.\n");
+ pr_warn("Limiting number of power states to max (%d)\n",
+ ACPI_PROCESSOR_MAX_POWER);
+ pr_warn("Please increase ACPI_PROCESSOR_MAX_POWER if needed.\n");
break;
}
}
@@ -1060,8 +1057,8 @@ int acpi_processor_power_init(struct acpi_processor *pr)
retval = cpuidle_register_driver(&acpi_idle_driver);
if (retval)
return retval;
- printk(KERN_DEBUG "ACPI: %s registered with cpuidle\n",
- acpi_idle_driver.name);
+ pr_debug("%s registered with cpuidle\n",
+ acpi_idle_driver.name);
}
dev = kzalloc(sizeof(*dev), GFP_KERNEL);
--
1.9.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v2 4/5] ACPI / processor_idle : introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE
2015-09-16 13:59 ` [PATCH v2 0/5] ACPI / processor_idle: Add ACPIv6.0 LPI support Sudeep Holla
` (2 preceding siblings ...)
2015-09-16 13:59 ` [PATCH v2 3/5] ACPI / processor_idle: replace PREFIX with pr_fmt Sudeep Holla
@ 2015-09-16 13:59 ` Sudeep Holla
2015-09-24 10:31 ` Ashwin Chaugule
2015-09-16 13:59 ` [PATCH v2 5/5] ACPI / processor_idle: Add support for Low Power Idle(LPI) states Sudeep Holla
4 siblings, 1 reply; 20+ messages in thread
From: Sudeep Holla @ 2015-09-16 13:59 UTC (permalink / raw)
To: linux-acpi, Rafael J. Wysocki
Cc: Sudeep Holla, linux-kernel, linux-ia64, x86, Al Stone,
Lorenzo Pieralisi, Mahesh Sivasubramanian, Ashwin Chaugule
ACPI 6.0 adds a new method to specify the CPU idle states(C-states)
called Low Power Idle(LPI) states. Since new architectures like ARM64
use only LPIs, introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE to
encapsulate all the code supporting the old style C-states(_CST)
This patch will help to extend the processor_idle module to support
LPI.
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
arch/ia64/Kconfig | 1 +
arch/x86/Kconfig | 1 +
drivers/acpi/Kconfig | 3 +++
drivers/acpi/processor_idle.c | 57 +++++++++++++++++++++++++++++++------------
include/acpi/processor.h | 3 ++-
5 files changed, 49 insertions(+), 16 deletions(-)
diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
index eb0249e37981..dbfa3c3a49e1 100644
--- a/arch/ia64/Kconfig
+++ b/arch/ia64/Kconfig
@@ -16,6 +16,7 @@ config IA64
select PCI if (!IA64_HP_SIM)
select ACPI if (!IA64_HP_SIM)
select ACPI_SYSTEM_POWER_STATES_SUPPORT if ACPI
+ select ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE if ACPI
select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI
select HAVE_UNSTABLE_SCHED_CLOCK
select HAVE_IDE
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 7aef2d52daa0..173075c48df8 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -34,6 +34,7 @@ config X86
select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI
select ARCH_MIGHT_HAVE_PC_PARPORT
select ARCH_MIGHT_HAVE_PC_SERIO
+ select ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE if ACPI
select ARCH_SUPPORTS_ATOMIC_RMW
select ARCH_SUPPORTS_DEFERRED_STRUCT_PAGE_INIT
select ARCH_SUPPORTS_INT128 if X86_64
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 5d1015c26ff4..1eb0b8a84a65 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -48,6 +48,9 @@ config ACPI_LEGACY_TABLES_LOOKUP
config ARCH_MIGHT_HAVE_ACPI_PDC
bool
+config ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE
+ bool
+
config ACPI_GENERIC_GSI
bool
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index fadce354d2b7..9ca840c88f48 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -59,6 +59,12 @@ module_param(latency_factor, uint, 0644);
static DEFINE_PER_CPU(struct cpuidle_device *, acpi_cpuidle_device);
+struct cpuidle_driver acpi_idle_driver = {
+ .name = "acpi_idle",
+ .owner = THIS_MODULE,
+};
+
+#ifdef CONFIG_ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE
static DEFINE_PER_CPU(struct acpi_processor_cx * [CPUIDLE_STATE_MAX],
acpi_cstate);
@@ -804,11 +810,6 @@ static void acpi_idle_enter_freeze(struct cpuidle_device *dev,
acpi_idle_do_entry(cx);
}
-struct cpuidle_driver acpi_idle_driver = {
- .name = "acpi_idle",
- .owner = THIS_MODULE,
-};
-
/**
* acpi_processor_setup_cpuidle_cx - prepares and configures CPUIDLE
* device i.e. per-cpu data
@@ -925,6 +926,41 @@ static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr)
return 0;
}
+static inline void acpi_processor_cstate_first_run_checks(void)
+{
+ static int first_run;
+
+ if (first_run)
+ return;
+ dmi_check_system(processor_power_dmi_table);
+ max_cstate = acpi_processor_cstate_check(max_cstate);
+ if (max_cstate < ACPI_C_STATES_MAX)
+ pr_notice("ACPI: processor limited to max C-state %d\n",
+ max_cstate);
+ first_run++;
+}
+#else
+
+static inline int disabled_by_idle_boot_param(void) { return 0; }
+static inline void acpi_processor_cstate_first_run_checks(void) { }
+static int acpi_processor_get_power_info(struct acpi_processor *pr)
+{
+ return -ENODEV;
+}
+
+static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr,
+ struct cpuidle_device *dev)
+{
+ return -EINVAL;
+}
+
+static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr)
+{
+ return -EINVAL;
+}
+
+#endif
+
int acpi_processor_hotplug(struct acpi_processor *pr)
{
int ret = 0;
@@ -1018,20 +1054,11 @@ int acpi_processor_power_init(struct acpi_processor *pr)
acpi_status status;
int retval;
struct cpuidle_device *dev;
- static int first_run;
if (disabled_by_idle_boot_param())
return 0;
- if (!first_run) {
- dmi_check_system(processor_power_dmi_table);
- max_cstate = acpi_processor_cstate_check(max_cstate);
- if (max_cstate < ACPI_C_STATES_MAX)
- printk(KERN_NOTICE
- "ACPI: processor limited to max C-state %d\n",
- max_cstate);
- first_run++;
- }
+ acpi_processor_cstate_first_run_checks();
if (acpi_gbl_FADT.cst_control && !nocst) {
status =
diff --git a/include/acpi/processor.h b/include/acpi/processor.h
index 0abcab53dbe5..b447952f8236 100644
--- a/include/acpi/processor.h
+++ b/include/acpi/processor.h
@@ -242,7 +242,8 @@ extern int acpi_processor_get_performance_info(struct acpi_processor *pr);
DECLARE_PER_CPU(struct acpi_processor *, processors);
extern struct acpi_processor_errata errata;
-#ifdef ARCH_HAS_POWER_INIT
+#if defined(ARCH_HAS_POWER_INIT) && \
+ defined(CONFIG_ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE)
void acpi_processor_power_init_bm_check(struct acpi_processor_flags *flags,
unsigned int cpu);
int acpi_processor_ffh_cstate_probe(unsigned int cpu,
--
1.9.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v2 4/5] ACPI / processor_idle : introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE
2015-09-16 13:59 ` [PATCH v2 4/5] ACPI / processor_idle : introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE Sudeep Holla
@ 2015-09-24 10:31 ` Ashwin Chaugule
2015-09-24 13:05 ` Sudeep Holla
0 siblings, 1 reply; 20+ messages in thread
From: Ashwin Chaugule @ 2015-09-24 10:31 UTC (permalink / raw)
To: Sudeep Holla
Cc: linux acpi, Rafael J. Wysocki, lkml, linux-ia64, x86, Al Stone,
Lorenzo Pieralisi, Mahesh Sivasubramanian
Hi Sudeep,
On 16 September 2015 at 09:59, Sudeep Holla <sudeep.holla@arm.com> wrote:
> ACPI 6.0 adds a new method to specify the CPU idle states(C-states)
> called Low Power Idle(LPI) states. Since new architectures like ARM64
> use only LPIs, introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE to
> encapsulate all the code supporting the old style C-states(_CST)
>
> This patch will help to extend the processor_idle module to support
> LPI.
>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> arch/ia64/Kconfig | 1 +
> arch/x86/Kconfig | 1 +
> drivers/acpi/Kconfig | 3 +++
> drivers/acpi/processor_idle.c | 57 +++++++++++++++++++++++++++++++------------
> include/acpi/processor.h | 3 ++-
> 5 files changed, 49 insertions(+), 16 deletions(-)
>
> diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
> index eb0249e37981..dbfa3c3a49e1 100644
> --- a/arch/ia64/Kconfig
> +++ b/arch/ia64/Kconfig
> @@ -16,6 +16,7 @@ config IA64
> select PCI if (!IA64_HP_SIM)
> select ACPI if (!IA64_HP_SIM)
> select ACPI_SYSTEM_POWER_STATES_SUPPORT if ACPI
> + select ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE if ACPI
> select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI
> select HAVE_UNSTABLE_SCHED_CLOCK
> select HAVE_IDE
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 7aef2d52daa0..173075c48df8 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -34,6 +34,7 @@ config X86
> select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI
> select ARCH_MIGHT_HAVE_PC_PARPORT
> select ARCH_MIGHT_HAVE_PC_SERIO
> + select ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE if ACPI
> select ARCH_SUPPORTS_ATOMIC_RMW
> select ARCH_SUPPORTS_DEFERRED_STRUCT_PAGE_INIT
> select ARCH_SUPPORTS_INT128 if X86_64
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 5d1015c26ff4..1eb0b8a84a65 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -48,6 +48,9 @@ config ACPI_LEGACY_TABLES_LOOKUP
> config ARCH_MIGHT_HAVE_ACPI_PDC
> bool
>
> +config ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE
> + bool
> +
> config ACPI_GENERIC_GSI
> bool
>
Can't this be auto selected under ACPI_PROCESSOR_IDLE if X86 || IA64 instead?
Regards,
Ashwin.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 4/5] ACPI / processor_idle : introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE
2015-09-24 10:31 ` Ashwin Chaugule
@ 2015-09-24 13:05 ` Sudeep Holla
0 siblings, 0 replies; 20+ messages in thread
From: Sudeep Holla @ 2015-09-24 13:05 UTC (permalink / raw)
To: Ashwin Chaugule
Cc: Sudeep Holla, linux acpi, Rafael J. Wysocki, lkml,
linux-ia64@vger.kernel.org, x86@kernel.org, Al Stone,
Lorenzo Pieralisi, Mahesh Sivasubramanian
On 24/09/15 11:31, Ashwin Chaugule wrote:
> Hi Sudeep,
>
> On 16 September 2015 at 09:59, Sudeep Holla <sudeep.holla@arm.com> wrote:
>> ACPI 6.0 adds a new method to specify the CPU idle states(C-states)
>> called Low Power Idle(LPI) states. Since new architectures like ARM64
>> use only LPIs, introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE to
>> encapsulate all the code supporting the old style C-states(_CST)
>>
>> This patch will help to extend the processor_idle module to support
>> LPI.
>>
>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
[...]
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index 7aef2d52daa0..173075c48df8 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -34,6 +34,7 @@ config X86
>> select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI
>> select ARCH_MIGHT_HAVE_PC_PARPORT
>> select ARCH_MIGHT_HAVE_PC_SERIO
>> + select ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE if ACPI
>> select ARCH_SUPPORTS_ATOMIC_RMW
>> select ARCH_SUPPORTS_DEFERRED_STRUCT_PAGE_INIT
>> select ARCH_SUPPORTS_INT128 if X86_64
>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>> index 5d1015c26ff4..1eb0b8a84a65 100644
>> --- a/drivers/acpi/Kconfig
>> +++ b/drivers/acpi/Kconfig
>> @@ -48,6 +48,9 @@ config ACPI_LEGACY_TABLES_LOOKUP
>> config ARCH_MIGHT_HAVE_ACPI_PDC
>> bool
>>
>> +config ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE
>> + bool
>> +
>> config ACPI_GENERIC_GSI
>> bool
>>
>
> Can't this be auto selected under ACPI_PROCESSOR_IDLE if X86 || IA64
> instead?
It can be done. The idea was to have ACPI_PROCESSOR_IDLE selected on
both x86 and ARM64.
processor_idle driver will support both _CST and _LPI
1. to avoid some code duplication
2. for easy enabling of _LPI on x86 if needed in future
Regards,
Sudeep
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 5/5] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
2015-09-16 13:59 ` [PATCH v2 0/5] ACPI / processor_idle: Add ACPIv6.0 LPI support Sudeep Holla
` (3 preceding siblings ...)
2015-09-16 13:59 ` [PATCH v2 4/5] ACPI / processor_idle : introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE Sudeep Holla
@ 2015-09-16 13:59 ` Sudeep Holla
2015-10-02 17:07 ` Ashwin Chaugule
2015-10-26 19:13 ` Ashwin Chaugule
4 siblings, 2 replies; 20+ messages in thread
From: Sudeep Holla @ 2015-09-16 13:59 UTC (permalink / raw)
To: linux-acpi, Rafael J. Wysocki
Cc: Sudeep Holla, linux-kernel, linux-ia64, x86, Al Stone,
Lorenzo Pieralisi, Mahesh Sivasubramanian, Ashwin Chaugule
ACPI 6.0 introduced an optional object _LPI that provides an alternate
method to describe Low Power Idle states. It defines the local power
states for each node in a hierarchical processor topology. The OSPM can
use _LPI object to select a local power state for each level of processor
hierarchy in the system. They used to produce a composite power state
request that is presented to the platform by the OSPM.
Since multiple processors affect the idle state for any non-leaf hierarchy
node, coordination of idle state requests between the processors is
required. ACPI supports two different coordination schemes: Platform
coordinated and OS initiated.
This patch adds initial support for Platform coordination scheme of LPI.
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
drivers/acpi/Kconfig | 3 +
drivers/acpi/bus.c | 8 +-
drivers/acpi/processor_driver.c | 2 +-
drivers/acpi/processor_idle.c | 412 +++++++++++++++++++++++++++++++++++-----
include/acpi/processor.h | 26 ++-
include/linux/acpi.h | 4 +
6 files changed, 405 insertions(+), 50 deletions(-)
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 1eb0b8a84a65..1bb6fade84bc 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -51,6 +51,9 @@ config ARCH_MIGHT_HAVE_ACPI_PDC
config ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE
bool
+config ARCH_SUPPORTS_ACPI_PROCESSOR_LPI
+ bool
+
config ACPI_GENERIC_GSI
bool
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index a212cefae524..2e9e2e3fde6a 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -301,6 +301,7 @@ acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context)
EXPORT_SYMBOL(acpi_run_osc);
bool osc_sb_apei_support_acked;
+bool osc_pc_lpi_support_acked;
static u8 sb_uuid_str[] = "0811B06E-4A27-44F9-8D60-3CBBC22E7B48";
static void acpi_bus_osc_support(void)
{
@@ -321,6 +322,8 @@ static void acpi_bus_osc_support(void)
capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_PPC_OST_SUPPORT;
capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_HOTPLUG_OST_SUPPORT;
+ if (IS_ENABLED(CONFIG_ARCH_SUPPORTS_ACPI_PROCESSOR_LPI))
+ capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_PCLPI_SUPPORT;
if (!ghes_disable)
capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_APEI_SUPPORT;
@@ -328,9 +331,12 @@ static void acpi_bus_osc_support(void)
return;
if (ACPI_SUCCESS(acpi_run_osc(handle, &context))) {
u32 *capbuf_ret = context.ret.pointer;
- if (context.ret.length > OSC_SUPPORT_DWORD)
+ if (context.ret.length > OSC_SUPPORT_DWORD) {
osc_sb_apei_support_acked =
capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_APEI_SUPPORT;
+ osc_pc_lpi_support_acked =
+ capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_PCLPI_SUPPORT;
+ }
kfree(context.ret.pointer);
}
/* do we need to check other returned cap? Sounds no */
diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
index dff584a7137b..5000f4af1d5e 100644
--- a/drivers/acpi/processor_driver.c
+++ b/drivers/acpi/processor_driver.c
@@ -90,7 +90,7 @@ static void acpi_processor_notify(acpi_handle handle, u32 event, void *data)
pr->performance_platform_limit);
break;
case ACPI_PROCESSOR_NOTIFY_POWER:
- acpi_processor_cst_has_changed(pr);
+ acpi_processor_power_state_has_changed(pr);
acpi_bus_generate_netlink_event(device->pnp.device_class,
dev_name(&device->dev), event, 0);
break;
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 9ca840c88f48..af851f16bb2e 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -576,7 +576,7 @@ static int acpi_processor_power_verify(struct acpi_processor *pr)
return (working);
}
-static int acpi_processor_get_power_info(struct acpi_processor *pr)
+static int acpi_processor_get_cstate_info(struct acpi_processor *pr)
{
unsigned int i;
int result;
@@ -810,31 +810,12 @@ static void acpi_idle_enter_freeze(struct cpuidle_device *dev,
acpi_idle_do_entry(cx);
}
-/**
- * acpi_processor_setup_cpuidle_cx - prepares and configures CPUIDLE
- * device i.e. per-cpu data
- *
- * @pr: the ACPI processor
- * @dev : the cpuidle device
- */
static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr,
struct cpuidle_device *dev)
{
int i, count = CPUIDLE_DRIVER_STATE_START;
struct acpi_processor_cx *cx;
- if (!pr->flags.power_setup_done)
- return -EINVAL;
-
- if (pr->flags.power == 0) {
- return -EINVAL;
- }
-
- if (!dev)
- return -EINVAL;
-
- dev->cpu = pr->id;
-
if (max_cstate == 0)
max_cstate = 1;
@@ -857,31 +838,13 @@ static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr,
return 0;
}
-/**
- * acpi_processor_setup_cpuidle states- prepares and configures cpuidle
- * global state data i.e. idle routines
- *
- * @pr: the ACPI processor
- */
-static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr)
+static int acpi_processor_setup_cstates(struct acpi_processor *pr)
{
int i, count = CPUIDLE_DRIVER_STATE_START;
struct acpi_processor_cx *cx;
struct cpuidle_state *state;
struct cpuidle_driver *drv = &acpi_idle_driver;
- if (!pr->flags.power_setup_done)
- return -EINVAL;
-
- if (pr->flags.power == 0)
- return -EINVAL;
-
- drv->safe_state_index = -1;
- for (i = CPUIDLE_DRIVER_STATE_START; i < CPUIDLE_STATE_MAX; i++) {
- drv->states[i].name[0] = '\0';
- drv->states[i].desc[0] = '\0';
- }
-
if (max_cstate == 0)
max_cstate = 1;
@@ -943,24 +906,381 @@ static inline void acpi_processor_cstate_first_run_checks(void)
static inline int disabled_by_idle_boot_param(void) { return 0; }
static inline void acpi_processor_cstate_first_run_checks(void) { }
-static int acpi_processor_get_power_info(struct acpi_processor *pr)
+static int acpi_processor_get_cstate_info(struct acpi_processor *pr)
{
return -ENODEV;
}
-
static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr,
struct cpuidle_device *dev)
{
return -EINVAL;
}
-static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr)
+static int acpi_processor_setup_cstates(struct acpi_processor *pr)
+{
+ return -EINVAL;
+}
+#endif
+
+#ifdef CONFIG_ARCH_SUPPORTS_ACPI_PROCESSOR_LPI
+
+struct acpi_processor_lpi_info {
+ int state_count;
+ struct acpi_processor_lpi *lpix;
+};
+
+static int acpi_processor_evaluate_lpi(acpi_handle handle,
+ struct acpi_processor_lpi_info *info)
+{
+ acpi_status status = 0;
+ int ret;
+ int version, level, pkg_count, state_count = 1, loop;
+ struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+ union acpi_object *lpi;
+ struct acpi_processor_lpi *lpix;
+
+ status = acpi_evaluate_object(handle, "_LPI", NULL, &buffer);
+ if (ACPI_FAILURE(status)) {
+ ACPI_DEBUG_PRINT((ACPI_DB_INFO, "No _LPI, giving up\n"));
+ return -ENODEV;
+ }
+
+ lpi = buffer.pointer;
+
+ /* There must be at least 4 elements = 3 elements + 1 package */
+ if (!lpi || (lpi->type != ACPI_TYPE_PACKAGE) || lpi->package.count < 4) {
+ pr_info("not enough elements in _LPI\n");
+ ret = -EFAULT;
+ goto end;
+ }
+
+ version = lpi->package.elements[0].integer.value;
+ level = lpi->package.elements[1].integer.value;
+ pkg_count = lpi->package.elements[2].integer.value;
+
+ /* Validate number of power states. */
+ if (pkg_count < 1 || pkg_count != lpi->package.count - 3) {
+ pr_err("count given by _LPI is not valid\n");
+ ret = -EFAULT;
+ goto end;
+ }
+
+ lpix = kcalloc(pkg_count, sizeof(*lpix), GFP_KERNEL);
+ if (!lpix) {
+ ret = -ENOMEM;
+ goto end;
+ }
+
+ info->state_count = pkg_count;
+ info->lpix = lpix;
+ for (loop = 3; state_count <= pkg_count; loop++, state_count++, lpix++) {
+ union acpi_object *element, *obj;
+
+ element = &lpi->package.elements[loop];
+ if (element->type != ACPI_TYPE_PACKAGE)
+ continue;
+
+ if (element->package.count < 7)
+ continue;
+
+ /* TODO
+ * this long list is looking insane now
+ * need a cleaner and saner way to read the elements
+ */
+ obj = &element->package.elements[6];
+ if (obj->type == ACPI_TYPE_BUFFER) {
+ struct acpi_power_register *reg;
+
+ reg = (struct acpi_power_register *)obj->buffer.pointer;
+ if (reg->space_id != ACPI_ADR_SPACE_SYSTEM_IO &&
+ (reg->space_id != ACPI_ADR_SPACE_FIXED_HARDWARE))
+ continue;
+ lpix->address = reg->address;
+ if (reg->space_id == ACPI_ADR_SPACE_FIXED_HARDWARE)
+ lpix->entry_method = ACPI_CSTATE_FFH;
+ else
+ lpix->entry_method = ACPI_CSTATE_SYSTEMIO;
+ } else if (obj->type == ACPI_TYPE_INTEGER)
+ lpix->address = obj->integer.value;
+ else
+ continue;
+
+ /* elements[7,8] skipped for now i.e. Residency/Usage counter*/
+
+ obj = &element->package.elements[9];
+ if (obj->type == ACPI_TYPE_STRING)
+ strncpy(lpix->desc, obj->string.pointer, ACPI_CX_DESC_LEN);
+
+ lpix->index = state_count;
+
+ obj = &element->package.elements[0];
+ if (obj->type != ACPI_TYPE_INTEGER)
+ continue;
+ lpix->min_residency = obj->integer.value;
+
+ obj = &element->package.elements[1];
+ if (obj->type != ACPI_TYPE_INTEGER)
+ continue;
+ lpix->wake_latency = obj->integer.value;
+
+ obj = &element->package.elements[2];
+ if (obj->type != ACPI_TYPE_INTEGER)
+ continue;
+ lpix->flags = obj->integer.value;
+
+ obj = &element->package.elements[3];
+ if (obj->type != ACPI_TYPE_INTEGER)
+ continue;
+ lpix->arch_flags = obj->integer.value;
+
+ obj = &element->package.elements[4];
+ if (obj->type != ACPI_TYPE_INTEGER)
+ continue;
+ lpix->res_cnt_freq = obj->integer.value;
+
+ obj = &element->package.elements[5];
+ if (obj->type != ACPI_TYPE_INTEGER)
+ continue;
+ lpix->enable_parent_state = obj->integer.value;
+ }
+ ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Found %d power states\n",
+ state_count));
+end:
+ kfree(buffer.pointer);
+ return status;
+}
+
+static int max_leaf_depth, fl_scnt;
+/*
+ * l_lpi - local LPI state
+ * p_lpi - parent LPI state
+ * c_lpi - composite LPI state
+ */
+static void combine_lpi_states(struct acpi_processor_lpi *l_lpi,
+ struct acpi_processor_lpi *p_lpi,
+ struct acpi_processor_lpi *c_lpi)
+{
+ c_lpi->min_residency = max(l_lpi->min_residency, p_lpi->min_residency);
+ c_lpi->wake_latency = l_lpi->wake_latency + p_lpi->wake_latency;
+ c_lpi->enable_parent_state = p_lpi->enable_parent_state;
+ c_lpi->entry_method = l_lpi->entry_method;
+ c_lpi->address = l_lpi->address + p_lpi->address;
+ c_lpi->index = p_lpi->index;
+ c_lpi->flags = p_lpi->flags;
+ c_lpi->arch_flags = p_lpi->arch_flags;
+ strncpy(c_lpi->desc, l_lpi->desc, ACPI_CX_DESC_LEN);
+ strncat(c_lpi->desc, "+", ACPI_CX_DESC_LEN);
+ strncat(c_lpi->desc, p_lpi->desc, ACPI_CX_DESC_LEN);
+}
+
+static int flatten_lpi_states(struct acpi_processor *pr,
+ struct acpi_processor_lpi_info *info,
+ struct acpi_processor_lpi *lpi,
+ uint32_t depth)
+{
+ int j, scount = info[depth].state_count;
+ struct acpi_processor_lpi *t = info[depth].lpix;
+
+ for (j = 0; j < scount; j++, t++) {
+ struct acpi_processor_lpi *flpi = &pr->power.lpi_states[fl_scnt];
+ bool valid = false;
+
+ if (depth == max_leaf_depth) { /* leaf/processor node */
+ memcpy(flpi, t, sizeof(*t));
+ fl_scnt++;
+ valid = true;
+ } else if (lpi && t->index <= lpi->enable_parent_state) {
+ combine_lpi_states(lpi, t, flpi);
+ fl_scnt++;
+ valid = true;
+ }
+ if (valid && depth)
+ flatten_lpi_states(pr, info, flpi, depth - 1);
+ }
+ return 0;
+}
+
+static int acpi_processor_get_lpi_info(struct acpi_processor *pr)
+{
+ int ret, i;
+ struct acpi_processor_lpi_info *info;
+ struct acpi_device *d = NULL;
+ acpi_handle handle = pr->handle, phandle;
+ acpi_status status;
+
+ if (!osc_pc_lpi_support_acked)
+ return -EOPNOTSUPP;
+
+ max_leaf_depth = 0;
+ if (!acpi_has_method(handle, "_LPI"))
+ return -EINVAL;
+ fl_scnt = 0;
+
+ while (ACPI_SUCCESS(status = acpi_get_parent(handle, &phandle))) {
+ if (!acpi_has_method(handle, "_LPI"))
+ continue;
+ acpi_bus_get_device(handle, &d);
+ if (!strcmp(acpi_device_hid(d), ACPI_PROCESSOR_CONTAINER_HID))
+ break;
+ max_leaf_depth++;
+ handle = phandle;
+ }
+
+ info = kcalloc(max_leaf_depth + 1, sizeof(*info), GFP_KERNEL);
+ if (!info)
+ return -ENOMEM;
+
+ phandle = pr->handle;
+ for (i = max_leaf_depth; i >= 0 && ACPI_SUCCESS(status); i--) {
+ handle = phandle;
+ ret = acpi_processor_evaluate_lpi(handle, info + i);
+ if (ret)
+ break;
+ status = acpi_get_parent(handle, &phandle);
+ }
+
+ flatten_lpi_states(pr, info, NULL, max_leaf_depth);
+
+ pr->power.count = fl_scnt;
+ for (i = 0; i <= max_leaf_depth; i++)
+ kfree(info[i].lpix);
+ kfree(info);
+
+ /* Tell driver that _LPI is supported. */
+ pr->flags.has_lpi = 1;
+ pr->flags.power = 1;
+
+ return 0;
+}
+
+/**
+ * acpi_idle_lpi_enter - enters an ACPI any LPI state
+ * @dev: the target CPU
+ * @drv: cpuidle driver containing cpuidle state info
+ * @index: index of target state
+ *
+ */
+static int acpi_idle_lpi_enter(struct cpuidle_device *dev,
+ struct cpuidle_driver *drv, int index)
{
+ struct acpi_processor *pr;
+ struct acpi_processor_lpi *lpi;
+
+ pr = __this_cpu_read(processors);
+
+ if (unlikely(!pr))
+ return -EINVAL;
+
+ lpi = &pr->power.lpi_states[index];
+ if (lpi->entry_method == ACPI_CSTATE_FFH)
+ /* Call into architectural FFH based C-state */
+ return acpi_processor_ffh_lpi_enter(lpi, index);
return -EINVAL;
}
+static int acpi_processor_setup_lpi_states(struct acpi_processor *pr)
+{
+ int i;
+ struct acpi_processor_lpi *lpi;
+ struct cpuidle_state *state;
+ struct cpuidle_driver *drv = &acpi_idle_driver;
+
+ for (i = 0; i < fl_scnt && i < CPUIDLE_STATE_MAX; i++) {
+ lpi = &pr->power.lpi_states[i];
+
+ state = &drv->states[i];
+ snprintf(state->name, CPUIDLE_NAME_LEN, "LPI-%d", i);
+ strncpy(state->desc, lpi->desc, CPUIDLE_DESC_LEN);
+ state->exit_latency = lpi->wake_latency;
+ state->target_residency = lpi->min_residency;
+ if (lpi->arch_flags)
+ state->flags |= CPUIDLE_FLAG_TIMER_STOP;
+ state->enter = acpi_idle_lpi_enter;
+ drv->safe_state_index = i;
+ }
+
+ drv->state_count = i;
+
+ return 0;
+}
+
+#else
+static int acpi_processor_ffh_lpi_probe(unsigned int cpu) { return -ENODEV; }
+static int acpi_processor_get_lpi_info(struct acpi_processor *pr)
+{
+ return -ENODEV;
+}
+
+static int acpi_processor_setup_lpi_states(struct acpi_processor *pr)
+{
+ return -EINVAL;
+}
#endif
+/**
+ * acpi_processor_setup_cpuidle_states- prepares and configures cpuidle
+ * global state data i.e. idle routines
+ *
+ * @pr: the ACPI processor
+ */
+static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr)
+{
+ int i;
+ struct cpuidle_driver *drv = &acpi_idle_driver;
+
+ if (!pr->flags.power_setup_done)
+ return -EINVAL;
+
+ if (pr->flags.power == 0)
+ return -EINVAL;
+
+ drv->safe_state_index = -1;
+ for (i = CPUIDLE_DRIVER_STATE_START; i < CPUIDLE_STATE_MAX; i++) {
+ drv->states[i].name[0] = '\0';
+ drv->states[i].desc[0] = '\0';
+ }
+
+ if (pr->flags.has_lpi)
+ return acpi_processor_setup_lpi_states(pr);
+ return acpi_processor_setup_cstates(pr);
+}
+
+/**
+ * acpi_processor_setup_cpuidle_dev - prepares and configures CPUIDLE
+ * device i.e. per-cpu data
+ *
+ * @pr: the ACPI processor
+ * @dev : the cpuidle device
+ */
+static int acpi_processor_setup_cpuidle_dev(struct acpi_processor *pr,
+ struct cpuidle_device *dev)
+{
+ if (!pr->flags.power_setup_done)
+ return -EINVAL;
+
+ if (pr->flags.power == 0)
+ return -EINVAL;
+
+ if (!dev)
+ return -EINVAL;
+
+ dev->cpu = pr->id;
+ if (pr->flags.has_lpi)
+ return acpi_processor_ffh_lpi_probe(pr->id);
+ else
+ return acpi_processor_setup_cpuidle_cx(pr, dev);
+}
+
+static int acpi_processor_get_power_info(struct acpi_processor *pr)
+{
+ int ret = 0;
+
+ ret = acpi_processor_get_cstate_info(pr);
+ if (ret)
+ ret = acpi_processor_get_lpi_info(pr);
+ return ret;
+}
+
int acpi_processor_hotplug(struct acpi_processor *pr)
{
int ret = 0;
@@ -980,7 +1300,7 @@ int acpi_processor_hotplug(struct acpi_processor *pr)
cpuidle_disable_device(dev);
acpi_processor_get_power_info(pr);
if (pr->flags.power) {
- acpi_processor_setup_cpuidle_cx(pr, dev);
+ acpi_processor_setup_cpuidle_dev(pr, dev);
ret = cpuidle_enable_device(dev);
}
cpuidle_resume_and_unlock();
@@ -988,7 +1308,7 @@ int acpi_processor_hotplug(struct acpi_processor *pr)
return ret;
}
-int acpi_processor_cst_has_changed(struct acpi_processor *pr)
+int acpi_processor_power_state_has_changed(struct acpi_processor *pr)
{
int cpu;
struct acpi_processor *_pr;
@@ -1036,7 +1356,7 @@ int acpi_processor_cst_has_changed(struct acpi_processor *pr)
acpi_processor_get_power_info(_pr);
if (_pr->flags.power) {
dev = per_cpu(acpi_cpuidle_device, cpu);
- acpi_processor_setup_cpuidle_cx(_pr, dev);
+ acpi_processor_setup_cpuidle_dev(_pr, dev);
cpuidle_enable_device(dev);
}
}
@@ -1093,7 +1413,7 @@ int acpi_processor_power_init(struct acpi_processor *pr)
return -ENOMEM;
per_cpu(acpi_cpuidle_device, pr->id) = dev;
- acpi_processor_setup_cpuidle_cx(pr, dev);
+ acpi_processor_setup_cpuidle_dev(pr, dev);
/* Register per-cpu cpuidle_device. Cpuidle driver
* must already be registered before registering device
diff --git a/include/acpi/processor.h b/include/acpi/processor.h
index b447952f8236..5ab11f978e6d 100644
--- a/include/acpi/processor.h
+++ b/include/acpi/processor.h
@@ -67,9 +67,25 @@ struct acpi_processor_cx {
char desc[ACPI_CX_DESC_LEN];
};
+struct acpi_processor_lpi {
+ u32 min_residency;
+ u32 wake_latency; /* worst case */
+ u32 flags;
+ u32 arch_flags;
+ u32 res_cnt_freq;
+ u32 enable_parent_state;
+ u64 address;
+ u8 index;
+ u8 entry_method;
+ char desc[ACPI_CX_DESC_LEN];
+};
+
struct acpi_processor_power {
int count;
- struct acpi_processor_cx states[ACPI_PROCESSOR_MAX_POWER];
+ union {
+ struct acpi_processor_cx states[ACPI_PROCESSOR_MAX_POWER];
+ struct acpi_processor_lpi lpi_states[ACPI_PROCESSOR_MAX_POWER];
+ };
int timer_broadcast_on_state;
};
@@ -189,6 +205,7 @@ struct acpi_processor_flags {
u8 bm_control:1;
u8 bm_check:1;
u8 has_cst:1;
+ u8 has_lpi:1;
u8 power_setup_done:1;
u8 bm_rld_set:1;
u8 need_hotplug_init:1;
@@ -272,6 +289,11 @@ static inline void acpi_processor_ffh_cstate_enter(struct acpi_processor_cx
}
#endif
+#ifdef CONFIG_ARCH_SUPPORTS_ACPI_PROCESSOR_LPI
+int acpi_processor_ffh_lpi_probe(unsigned int cpu);
+int acpi_processor_ffh_lpi_enter(struct acpi_processor_lpi *lpi, int idx);
+#endif
+
/* in processor_perflib.c */
#ifdef CONFIG_CPU_FREQ
@@ -358,7 +380,7 @@ extern struct cpuidle_driver acpi_idle_driver;
#ifdef CONFIG_ACPI_PROCESSOR_IDLE
int acpi_processor_power_init(struct acpi_processor *pr);
int acpi_processor_power_exit(struct acpi_processor *pr);
-int acpi_processor_cst_has_changed(struct acpi_processor *pr);
+int acpi_processor_power_state_has_changed(struct acpi_processor *pr);
int acpi_processor_hotplug(struct acpi_processor *pr);
#else
static inline int acpi_processor_power_init(struct acpi_processor *pr)
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index a9e25c4a44d1..6d5208d136ac 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -352,8 +352,12 @@ acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context);
#define OSC_SB_HOTPLUG_OST_SUPPORT 0x00000008
#define OSC_SB_APEI_SUPPORT 0x00000010
#define OSC_SB_CPC_SUPPORT 0x00000020
+#define OSC_SB_CPCV2_SUPPORT 0x00000040
+#define OSC_SB_PCLPI_SUPPORT 0x00000080
+#define OSC_SB_OSLPI_SUPPORT 0x00000100
extern bool osc_sb_apei_support_acked;
+extern bool osc_pc_lpi_support_acked;
/* PCI Host Bridge _OSC: Capabilities DWORD 2: Support Field */
#define OSC_PCI_EXT_CONFIG_SUPPORT 0x00000001
--
1.9.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v2 5/5] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
2015-09-16 13:59 ` [PATCH v2 5/5] ACPI / processor_idle: Add support for Low Power Idle(LPI) states Sudeep Holla
@ 2015-10-02 17:07 ` Ashwin Chaugule
2015-10-26 19:13 ` Ashwin Chaugule
1 sibling, 0 replies; 20+ messages in thread
From: Ashwin Chaugule @ 2015-10-02 17:07 UTC (permalink / raw)
To: Sudeep Holla
Cc: linux acpi, Rafael J. Wysocki, lkml, linux-ia64, x86, Al Stone,
Lorenzo Pieralisi, Mahesh Sivasubramanian
Hi Sudeep,
On 16 September 2015 at 09:59, Sudeep Holla <sudeep.holla@arm.com> wrote:
> ACPI 6.0 introduced an optional object _LPI that provides an alternate
> method to describe Low Power Idle states. It defines the local power
> states for each node in a hierarchical processor topology. The OSPM can
> use _LPI object to select a local power state for each level of processor
> hierarchy in the system. They used to produce a composite power state
> request that is presented to the platform by the OSPM.
>
> Since multiple processors affect the idle state for any non-leaf hierarchy
> node, coordination of idle state requests between the processors is
> required. ACPI supports two different coordination schemes: Platform
> coordinated and OS initiated.
>
> This patch adds initial support for Platform coordination scheme of LPI.
>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> drivers/acpi/Kconfig | 3 +
> drivers/acpi/bus.c | 8 +-
> drivers/acpi/processor_driver.c | 2 +-
> drivers/acpi/processor_idle.c | 412 +++++++++++++++++++++++++++++++++++-----
> include/acpi/processor.h | 26 ++-
> include/linux/acpi.h | 4 +
> 6 files changed, 405 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 1eb0b8a84a65..1bb6fade84bc 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -51,6 +51,9 @@ config ARCH_MIGHT_HAVE_ACPI_PDC
> config ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE
> bool
>
> +config ARCH_SUPPORTS_ACPI_PROCESSOR_LPI
> + bool
> +
> config ACPI_GENERIC_GSI
> bool
>
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index a212cefae524..2e9e2e3fde6a 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -301,6 +301,7 @@ acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context)
> EXPORT_SYMBOL(acpi_run_osc);
>
> bool osc_sb_apei_support_acked;
> +bool osc_pc_lpi_support_acked;
> static u8 sb_uuid_str[] = "0811B06E-4A27-44F9-8D60-3CBBC22E7B48";
> static void acpi_bus_osc_support(void)
> {
> @@ -321,6 +322,8 @@ static void acpi_bus_osc_support(void)
> capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_PPC_OST_SUPPORT;
>
> capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_HOTPLUG_OST_SUPPORT;
> + if (IS_ENABLED(CONFIG_ARCH_SUPPORTS_ACPI_PROCESSOR_LPI))
> + capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_PCLPI_SUPPORT;
>
> if (!ghes_disable)
> capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_APEI_SUPPORT;
> @@ -328,9 +331,12 @@ static void acpi_bus_osc_support(void)
> return;
> if (ACPI_SUCCESS(acpi_run_osc(handle, &context))) {
> u32 *capbuf_ret = context.ret.pointer;
> - if (context.ret.length > OSC_SUPPORT_DWORD)
> + if (context.ret.length > OSC_SUPPORT_DWORD) {
> osc_sb_apei_support_acked =
> capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_APEI_SUPPORT;
> + osc_pc_lpi_support_acked =
> + capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_PCLPI_SUPPORT;
> + }
Not sure we want to keep adding more OSC global flags, since theyre
not really used elsewhere outside the drivers. I'm not strongly
against it, but seems like for LPI, CPPC and maybe a few others, we
could do the check locally, like pcc-cpufreq does.
> kfree(context.ret.pointer);
> }
> /* do we need to check other returned cap? Sounds no */
> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
> index dff584a7137b..5000f4af1d5e 100644
> --- a/drivers/acpi/processor_driver.c
> +++ b/drivers/acpi/processor_driver.c
> @@ -90,7 +90,7 @@ static void acpi_processor_notify(acpi_handle handle, u32 event, void *data)
> pr->performance_platform_limit);
> break;
> case ACPI_PROCESSOR_NOTIFY_POWER:
> - acpi_processor_cst_has_changed(pr);
> + acpi_processor_power_state_has_changed(pr);
> acpi_bus_generate_netlink_event(device->pnp.device_class,
> dev_name(&device->dev), event, 0);
> break;
> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
> index 9ca840c88f48..af851f16bb2e 100644
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -576,7 +576,7 @@ static int acpi_processor_power_verify(struct acpi_processor *pr)
> return (working);
> }
>
> -static int acpi_processor_get_power_info(struct acpi_processor *pr)
> +static int acpi_processor_get_cstate_info(struct acpi_processor *pr)
> {
> unsigned int i;
> int result;
> @@ -810,31 +810,12 @@ static void acpi_idle_enter_freeze(struct cpuidle_device *dev,
> acpi_idle_do_entry(cx);
> }
>
> -/**
> - * acpi_processor_setup_cpuidle_cx - prepares and configures CPUIDLE
> - * device i.e. per-cpu data
> - *
> - * @pr: the ACPI processor
> - * @dev : the cpuidle device
> - */
> static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr,
> struct cpuidle_device *dev)
> {
> int i, count = CPUIDLE_DRIVER_STATE_START;
> struct acpi_processor_cx *cx;
>
> - if (!pr->flags.power_setup_done)
> - return -EINVAL;
> -
> - if (pr->flags.power == 0) {
> - return -EINVAL;
> - }
> -
> - if (!dev)
> - return -EINVAL;
> -
> - dev->cpu = pr->id;
> -
> if (max_cstate == 0)
> max_cstate = 1;
>
> @@ -857,31 +838,13 @@ static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr,
> return 0;
> }
>
> -/**
> - * acpi_processor_setup_cpuidle states- prepares and configures cpuidle
> - * global state data i.e. idle routines
> - *
> - * @pr: the ACPI processor
> - */
> -static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr)
> +static int acpi_processor_setup_cstates(struct acpi_processor *pr)
> {
> int i, count = CPUIDLE_DRIVER_STATE_START;
> struct acpi_processor_cx *cx;
> struct cpuidle_state *state;
> struct cpuidle_driver *drv = &acpi_idle_driver;
>
> - if (!pr->flags.power_setup_done)
> - return -EINVAL;
> -
> - if (pr->flags.power == 0)
> - return -EINVAL;
> -
> - drv->safe_state_index = -1;
> - for (i = CPUIDLE_DRIVER_STATE_START; i < CPUIDLE_STATE_MAX; i++) {
> - drv->states[i].name[0] = '\0';
> - drv->states[i].desc[0] = '\0';
> - }
> -
> if (max_cstate == 0)
> max_cstate = 1;
>
> @@ -943,24 +906,381 @@ static inline void acpi_processor_cstate_first_run_checks(void)
>
> static inline int disabled_by_idle_boot_param(void) { return 0; }
> static inline void acpi_processor_cstate_first_run_checks(void) { }
> -static int acpi_processor_get_power_info(struct acpi_processor *pr)
> +static int acpi_processor_get_cstate_info(struct acpi_processor *pr)
> {
> return -ENODEV;
> }
> -
> static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr,
> struct cpuidle_device *dev)
> {
> return -EINVAL;
> }
>
> -static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr)
> +static int acpi_processor_setup_cstates(struct acpi_processor *pr)
> +{
> + return -EINVAL;
> +}
> +#endif
> +
> +#ifdef CONFIG_ARCH_SUPPORTS_ACPI_PROCESSOR_LPI
> +
> +struct acpi_processor_lpi_info {
> + int state_count;
> + struct acpi_processor_lpi *lpix;
> +};
> +
> +static int acpi_processor_evaluate_lpi(acpi_handle handle,
> + struct acpi_processor_lpi_info *info)
> +{
> + acpi_status status = 0;
> + int ret;
> + int version, level, pkg_count, state_count = 1, loop;
> + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> + union acpi_object *lpi;
> + struct acpi_processor_lpi *lpix;
> +
> + status = acpi_evaluate_object(handle, "_LPI", NULL, &buffer);
> + if (ACPI_FAILURE(status)) {
> + ACPI_DEBUG_PRINT((ACPI_DB_INFO, "No _LPI, giving up\n"));
> + return -ENODEV;
> + }
> +
> + lpi = buffer.pointer;
> +
> + /* There must be at least 4 elements = 3 elements + 1 package */
> + if (!lpi || (lpi->type != ACPI_TYPE_PACKAGE) || lpi->package.count < 4) {
> + pr_info("not enough elements in _LPI\n");
> + ret = -EFAULT;
> + goto end;
> + }
> +
> + version = lpi->package.elements[0].integer.value;
> + level = lpi->package.elements[1].integer.value;
> + pkg_count = lpi->package.elements[2].integer.value;
> +
> + /* Validate number of power states. */
> + if (pkg_count < 1 || pkg_count != lpi->package.count - 3) {
> + pr_err("count given by _LPI is not valid\n");
> + ret = -EFAULT;
> + goto end;
> + }
> +
> + lpix = kcalloc(pkg_count, sizeof(*lpix), GFP_KERNEL);
> + if (!lpix) {
> + ret = -ENOMEM;
> + goto end;
> + }
> +
> + info->state_count = pkg_count;
> + info->lpix = lpix;
> + for (loop = 3; state_count <= pkg_count; loop++, state_count++, lpix++) {
Reusing state_count can be confusing. Maybe use state_idx, state_ctr
or something. Or add comments to explain how this thing is laid out.
> + union acpi_object *element, *obj;
> +
> + element = &lpi->package.elements[loop];
> + if (element->type != ACPI_TYPE_PACKAGE)
> + continue;
> +
> + if (element->package.count < 7)
> + continue;
> +
> + /* TODO
> + * this long list is looking insane now
> + * need a cleaner and saner way to read the elements
> + */
> + obj = &element->package.elements[6];
> + if (obj->type == ACPI_TYPE_BUFFER) {
> + struct acpi_power_register *reg;
> +
> + reg = (struct acpi_power_register *)obj->buffer.pointer;
> + if (reg->space_id != ACPI_ADR_SPACE_SYSTEM_IO &&
> + (reg->space_id != ACPI_ADR_SPACE_FIXED_HARDWARE))
> + continue;
> + lpix->address = reg->address;
> + if (reg->space_id == ACPI_ADR_SPACE_FIXED_HARDWARE)
> + lpix->entry_method = ACPI_CSTATE_FFH;
> + else
> + lpix->entry_method = ACPI_CSTATE_SYSTEMIO;
> + } else if (obj->type == ACPI_TYPE_INTEGER)
> + lpix->address = obj->integer.value;
> + else
> + continue;
> +
> + /* elements[7,8] skipped for now i.e. Residency/Usage counter*/
> +
> + obj = &element->package.elements[9];
> + if (obj->type == ACPI_TYPE_STRING)
> + strncpy(lpix->desc, obj->string.pointer, ACPI_CX_DESC_LEN);
> +
> + lpix->index = state_count;
> +
> + obj = &element->package.elements[0];
> + if (obj->type != ACPI_TYPE_INTEGER)
> + continue;
> + lpix->min_residency = obj->integer.value;
> +
> + obj = &element->package.elements[1];
> + if (obj->type != ACPI_TYPE_INTEGER)
> + continue;
> + lpix->wake_latency = obj->integer.value;
> +
> + obj = &element->package.elements[2];
> + if (obj->type != ACPI_TYPE_INTEGER)
> + continue;
> + lpix->flags = obj->integer.value;
> +
> + obj = &element->package.elements[3];
> + if (obj->type != ACPI_TYPE_INTEGER)
> + continue;
> + lpix->arch_flags = obj->integer.value;
> +
> + obj = &element->package.elements[4];
> + if (obj->type != ACPI_TYPE_INTEGER)
> + continue;
> + lpix->res_cnt_freq = obj->integer.value;
> +
> + obj = &element->package.elements[5];
> + if (obj->type != ACPI_TYPE_INTEGER)
> + continue;
> + lpix->enable_parent_state = obj->integer.value;
> + }
> + ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Found %d power states\n",
> + state_count));
> +end:
> + kfree(buffer.pointer);
> + return status;
> +}
> +
> +static int max_leaf_depth, fl_scnt;
> +/*
> + * l_lpi - local LPI state
> + * p_lpi - parent LPI state
> + * c_lpi - composite LPI state
> + */
> +static void combine_lpi_states(struct acpi_processor_lpi *l_lpi,
> + struct acpi_processor_lpi *p_lpi,
> + struct acpi_processor_lpi *c_lpi)
> +{
> + c_lpi->min_residency = max(l_lpi->min_residency, p_lpi->min_residency);
> + c_lpi->wake_latency = l_lpi->wake_latency + p_lpi->wake_latency;
> + c_lpi->enable_parent_state = p_lpi->enable_parent_state;
> + c_lpi->entry_method = l_lpi->entry_method;
> + c_lpi->address = l_lpi->address + p_lpi->address;
Aren't there some constraints to how this addr is worked out? Need to
look at the LPI spec. but IIRC, in some cases its not additive and the
parent value is used for overwriting.
> + c_lpi->index = p_lpi->index;
> + c_lpi->flags = p_lpi->flags;
> + c_lpi->arch_flags = p_lpi->arch_flags;
> + strncpy(c_lpi->desc, l_lpi->desc, ACPI_CX_DESC_LEN);
> + strncat(c_lpi->desc, "+", ACPI_CX_DESC_LEN);
> + strncat(c_lpi->desc, p_lpi->desc, ACPI_CX_DESC_LEN);
> +}
> +
> +static int flatten_lpi_states(struct acpi_processor *pr,
> + struct acpi_processor_lpi_info *info,
> + struct acpi_processor_lpi *lpi,
> + uint32_t depth)
> +{
> + int j, scount = info[depth].state_count;
> + struct acpi_processor_lpi *t = info[depth].lpix;
> +
> + for (j = 0; j < scount; j++, t++) {
> + struct acpi_processor_lpi *flpi = &pr->power.lpi_states[fl_scnt];
> + bool valid = false;
> +
> + if (depth == max_leaf_depth) { /* leaf/processor node */
> + memcpy(flpi, t, sizeof(*t));
> + fl_scnt++;
> + valid = true;
> + } else if (lpi && t->index <= lpi->enable_parent_state) {
> + combine_lpi_states(lpi, t, flpi);
> + fl_scnt++;
> + valid = true;
> + }
> + if (valid && depth)
> + flatten_lpi_states(pr, info, flpi, depth - 1);
> + }
> + return 0;
> +}
So is this thing taking one idle state at a time from each node
(proc/cluster..) and checking if it can be collapsed with one state
from each parent node (using the EPS constraint) then moving to the
next idle state at the lower node and up again?
> +
> +static int acpi_processor_get_lpi_info(struct acpi_processor *pr)
> +{
> + int ret, i;
> + struct acpi_processor_lpi_info *info;
> + struct acpi_device *d = NULL;
> + acpi_handle handle = pr->handle, phandle;
> + acpi_status status;
> +
> + if (!osc_pc_lpi_support_acked)
> + return -EOPNOTSUPP;
> +
> + max_leaf_depth = 0;
> + if (!acpi_has_method(handle, "_LPI"))
> + return -EINVAL;
> + fl_scnt = 0;
> +
> + while (ACPI_SUCCESS(status = acpi_get_parent(handle, &phandle))) {
> + if (!acpi_has_method(handle, "_LPI"))
> + continue;
> + acpi_bus_get_device(handle, &d);
> + if (!strcmp(acpi_device_hid(d), ACPI_PROCESSOR_CONTAINER_HID))
> + break;
> + max_leaf_depth++;
> + handle = phandle;
> + }
> +
> + info = kcalloc(max_leaf_depth + 1, sizeof(*info), GFP_KERNEL);
> + if (!info)
> + return -ENOMEM;
> +
> + phandle = pr->handle;
> + for (i = max_leaf_depth; i >= 0 && ACPI_SUCCESS(status); i--) {
> + handle = phandle;
> + ret = acpi_processor_evaluate_lpi(handle, info + i);
> + if (ret)
> + break;
> + status = acpi_get_parent(handle, &phandle);
> + }
> +
> + flatten_lpi_states(pr, info, NULL, max_leaf_depth);
> +
> + pr->power.count = fl_scnt;
> + for (i = 0; i <= max_leaf_depth; i++)
> + kfree(info[i].lpix);
> + kfree(info);
> +
> + /* Tell driver that _LPI is supported. */
> + pr->flags.has_lpi = 1;
> + pr->flags.power = 1;
> +
> + return 0;
> +}
> +
> +/**
> + * acpi_idle_lpi_enter - enters an ACPI any LPI state
> + * @dev: the target CPU
> + * @drv: cpuidle driver containing cpuidle state info
> + * @index: index of target state
> + *
> + */
Might as well add Return types to the kernel doc. here and elsewhere.
> +static int acpi_idle_lpi_enter(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv, int index)
> {
> + struct acpi_processor *pr;
> + struct acpi_processor_lpi *lpi;
> +
> + pr = __this_cpu_read(processors);
> +
> + if (unlikely(!pr))
> + return -EINVAL;
> +
> + lpi = &pr->power.lpi_states[index];
> + if (lpi->entry_method == ACPI_CSTATE_FFH)
> + /* Call into architectural FFH based C-state */
> + return acpi_processor_ffh_lpi_enter(lpi, index);
Where is this thing (and the lpi_probe()) defined ?
> return -EINVAL;
> }
>
> +static int acpi_processor_setup_lpi_states(struct acpi_processor *pr)
> +{
> + int i;
> + struct acpi_processor_lpi *lpi;
> + struct cpuidle_state *state;
> + struct cpuidle_driver *drv = &acpi_idle_driver;
> +
> + for (i = 0; i < fl_scnt && i < CPUIDLE_STATE_MAX; i++) {
> + lpi = &pr->power.lpi_states[i];
> +
> + state = &drv->states[i];
> + snprintf(state->name, CPUIDLE_NAME_LEN, "LPI-%d", i);
> + strncpy(state->desc, lpi->desc, CPUIDLE_DESC_LEN);
> + state->exit_latency = lpi->wake_latency;
> + state->target_residency = lpi->min_residency;
> + if (lpi->arch_flags)
> + state->flags |= CPUIDLE_FLAG_TIMER_STOP;
Plan to add arch specific callbacks later? FFH spec has some details
for ARM64 on what to save/restore.
Regards,
Ashwin
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2 5/5] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
2015-09-16 13:59 ` [PATCH v2 5/5] ACPI / processor_idle: Add support for Low Power Idle(LPI) states Sudeep Holla
2015-10-02 17:07 ` Ashwin Chaugule
@ 2015-10-26 19:13 ` Ashwin Chaugule
[not found] ` <562EC3A5.2070402@codeaurora.org>
2015-12-01 17:23 ` Prakash, Prashanth
1 sibling, 2 replies; 20+ messages in thread
From: Ashwin Chaugule @ 2015-10-26 19:13 UTC (permalink / raw)
To: Sudeep Holla
Cc: linux acpi, Rafael J. Wysocki, lkml, linux-ia64, x86, Al Stone,
Lorenzo Pieralisi, Mahesh Sivasubramanian, pprakash
+ Prashanth (who has successfully run this patch on Qcomm hardware)
On 16 September 2015 at 09:59, Sudeep Holla <sudeep.holla@arm.com> wrote:
> ACPI 6.0 introduced an optional object _LPI that provides an alternate
> method to describe Low Power Idle states. It defines the local power
> states for each node in a hierarchical processor topology. The OSPM can
> use _LPI object to select a local power state for each level of processor
> hierarchy in the system. They used to produce a composite power state
> request that is presented to the platform by the OSPM.
>
> Since multiple processors affect the idle state for any non-leaf hierarchy
> node, coordination of idle state requests between the processors is
> required. ACPI supports two different coordination schemes: Platform
> coordinated and OS initiated.
>
> This patch adds initial support for Platform coordination scheme of LPI.
>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> drivers/acpi/Kconfig | 3 +
> drivers/acpi/bus.c | 8 +-
> drivers/acpi/processor_driver.c | 2 +-
> drivers/acpi/processor_idle.c | 412 +++++++++++++++++++++++++++++++++++-----
> include/acpi/processor.h | 26 ++-
> include/linux/acpi.h | 4 +
> 6 files changed, 405 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 1eb0b8a84a65..1bb6fade84bc 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -51,6 +51,9 @@ config ARCH_MIGHT_HAVE_ACPI_PDC
> config ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE
> bool
>
> +config ARCH_SUPPORTS_ACPI_PROCESSOR_LPI
> + bool
> +
> config ACPI_GENERIC_GSI
> bool
>
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index a212cefae524..2e9e2e3fde6a 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -301,6 +301,7 @@ acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context)
> EXPORT_SYMBOL(acpi_run_osc);
>
> bool osc_sb_apei_support_acked;
> +bool osc_pc_lpi_support_acked;
> static u8 sb_uuid_str[] = "0811B06E-4A27-44F9-8D60-3CBBC22E7B48";
> static void acpi_bus_osc_support(void)
> {
> @@ -321,6 +322,8 @@ static void acpi_bus_osc_support(void)
> capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_PPC_OST_SUPPORT;
>
> capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_HOTPLUG_OST_SUPPORT;
> + if (IS_ENABLED(CONFIG_ARCH_SUPPORTS_ACPI_PROCESSOR_LPI))
> + capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_PCLPI_SUPPORT;
>
> if (!ghes_disable)
> capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_APEI_SUPPORT;
> @@ -328,9 +331,12 @@ static void acpi_bus_osc_support(void)
> return;
> if (ACPI_SUCCESS(acpi_run_osc(handle, &context))) {
> u32 *capbuf_ret = context.ret.pointer;
> - if (context.ret.length > OSC_SUPPORT_DWORD)
> + if (context.ret.length > OSC_SUPPORT_DWORD) {
> osc_sb_apei_support_acked =
> capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_APEI_SUPPORT;
> + osc_pc_lpi_support_acked =
> + capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_PCLPI_SUPPORT;
> + }
> kfree(context.ret.pointer);
> }
> /* do we need to check other returned cap? Sounds no */
> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
> index dff584a7137b..5000f4af1d5e 100644
> --- a/drivers/acpi/processor_driver.c
> +++ b/drivers/acpi/processor_driver.c
> @@ -90,7 +90,7 @@ static void acpi_processor_notify(acpi_handle handle, u32 event, void *data)
> pr->performance_platform_limit);
> break;
> case ACPI_PROCESSOR_NOTIFY_POWER:
> - acpi_processor_cst_has_changed(pr);
> + acpi_processor_power_state_has_changed(pr);
> acpi_bus_generate_netlink_event(device->pnp.device_class,
> dev_name(&device->dev), event, 0);
> break;
> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
> index 9ca840c88f48..af851f16bb2e 100644
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -576,7 +576,7 @@ static int acpi_processor_power_verify(struct acpi_processor *pr)
> return (working);
> }
>
> -static int acpi_processor_get_power_info(struct acpi_processor *pr)
> +static int acpi_processor_get_cstate_info(struct acpi_processor *pr)
> {
> unsigned int i;
> int result;
> @@ -810,31 +810,12 @@ static void acpi_idle_enter_freeze(struct cpuidle_device *dev,
> acpi_idle_do_entry(cx);
> }
>
> -/**
> - * acpi_processor_setup_cpuidle_cx - prepares and configures CPUIDLE
> - * device i.e. per-cpu data
> - *
> - * @pr: the ACPI processor
> - * @dev : the cpuidle device
> - */
> static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr,
> struct cpuidle_device *dev)
> {
> int i, count = CPUIDLE_DRIVER_STATE_START;
> struct acpi_processor_cx *cx;
>
> - if (!pr->flags.power_setup_done)
> - return -EINVAL;
> -
> - if (pr->flags.power == 0) {
> - return -EINVAL;
> - }
> -
> - if (!dev)
> - return -EINVAL;
> -
> - dev->cpu = pr->id;
> -
> if (max_cstate == 0)
> max_cstate = 1;
>
> @@ -857,31 +838,13 @@ static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr,
> return 0;
> }
>
> -/**
> - * acpi_processor_setup_cpuidle states- prepares and configures cpuidle
> - * global state data i.e. idle routines
> - *
> - * @pr: the ACPI processor
> - */
> -static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr)
> +static int acpi_processor_setup_cstates(struct acpi_processor *pr)
> {
> int i, count = CPUIDLE_DRIVER_STATE_START;
> struct acpi_processor_cx *cx;
> struct cpuidle_state *state;
> struct cpuidle_driver *drv = &acpi_idle_driver;
>
> - if (!pr->flags.power_setup_done)
> - return -EINVAL;
> -
> - if (pr->flags.power == 0)
> - return -EINVAL;
> -
> - drv->safe_state_index = -1;
> - for (i = CPUIDLE_DRIVER_STATE_START; i < CPUIDLE_STATE_MAX; i++) {
> - drv->states[i].name[0] = '\0';
> - drv->states[i].desc[0] = '\0';
> - }
> -
> if (max_cstate == 0)
> max_cstate = 1;
>
> @@ -943,24 +906,381 @@ static inline void acpi_processor_cstate_first_run_checks(void)
>
> static inline int disabled_by_idle_boot_param(void) { return 0; }
> static inline void acpi_processor_cstate_first_run_checks(void) { }
> -static int acpi_processor_get_power_info(struct acpi_processor *pr)
> +static int acpi_processor_get_cstate_info(struct acpi_processor *pr)
> {
> return -ENODEV;
> }
> -
> static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr,
> struct cpuidle_device *dev)
> {
> return -EINVAL;
> }
>
> -static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr)
> +static int acpi_processor_setup_cstates(struct acpi_processor *pr)
> +{
> + return -EINVAL;
> +}
> +#endif
> +
> +#ifdef CONFIG_ARCH_SUPPORTS_ACPI_PROCESSOR_LPI
> +
> +struct acpi_processor_lpi_info {
> + int state_count;
> + struct acpi_processor_lpi *lpix;
> +};
> +
> +static int acpi_processor_evaluate_lpi(acpi_handle handle,
> + struct acpi_processor_lpi_info *info)
> +{
> + acpi_status status = 0;
> + int ret;
> + int version, level, pkg_count, state_count = 1, loop;
> + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> + union acpi_object *lpi;
> + struct acpi_processor_lpi *lpix;
> +
> + status = acpi_evaluate_object(handle, "_LPI", NULL, &buffer);
> + if (ACPI_FAILURE(status)) {
> + ACPI_DEBUG_PRINT((ACPI_DB_INFO, "No _LPI, giving up\n"));
> + return -ENODEV;
> + }
> +
> + lpi = buffer.pointer;
> +
> + /* There must be at least 4 elements = 3 elements + 1 package */
> + if (!lpi || (lpi->type != ACPI_TYPE_PACKAGE) || lpi->package.count < 4) {
> + pr_info("not enough elements in _LPI\n");
> + ret = -EFAULT;
> + goto end;
> + }
> +
> + version = lpi->package.elements[0].integer.value;
> + level = lpi->package.elements[1].integer.value;
> + pkg_count = lpi->package.elements[2].integer.value;
> +
> + /* Validate number of power states. */
> + if (pkg_count < 1 || pkg_count != lpi->package.count - 3) {
> + pr_err("count given by _LPI is not valid\n");
> + ret = -EFAULT;
> + goto end;
> + }
> +
> + lpix = kcalloc(pkg_count, sizeof(*lpix), GFP_KERNEL);
> + if (!lpix) {
> + ret = -ENOMEM;
> + goto end;
> + }
> +
> + info->state_count = pkg_count;
> + info->lpix = lpix;
> + for (loop = 3; state_count <= pkg_count; loop++, state_count++, lpix++) {
> + union acpi_object *element, *obj;
> +
> + element = &lpi->package.elements[loop];
> + if (element->type != ACPI_TYPE_PACKAGE)
> + continue;
> +
> + if (element->package.count < 7)
> + continue;
> +
> + /* TODO
> + * this long list is looking insane now
> + * need a cleaner and saner way to read the elements
> + */
> + obj = &element->package.elements[6];
> + if (obj->type == ACPI_TYPE_BUFFER) {
> + struct acpi_power_register *reg;
> +
> + reg = (struct acpi_power_register *)obj->buffer.pointer;
> + if (reg->space_id != ACPI_ADR_SPACE_SYSTEM_IO &&
> + (reg->space_id != ACPI_ADR_SPACE_FIXED_HARDWARE))
> + continue;
> + lpix->address = reg->address;
> + if (reg->space_id == ACPI_ADR_SPACE_FIXED_HARDWARE)
> + lpix->entry_method = ACPI_CSTATE_FFH;
> + else
> + lpix->entry_method = ACPI_CSTATE_SYSTEMIO;
> + } else if (obj->type == ACPI_TYPE_INTEGER)
> + lpix->address = obj->integer.value;
> + else
> + continue;
> +
> + /* elements[7,8] skipped for now i.e. Residency/Usage counter*/
> +
> + obj = &element->package.elements[9];
> + if (obj->type == ACPI_TYPE_STRING)
> + strncpy(lpix->desc, obj->string.pointer, ACPI_CX_DESC_LEN);
> +
> + lpix->index = state_count;
> +
> + obj = &element->package.elements[0];
> + if (obj->type != ACPI_TYPE_INTEGER)
> + continue;
> + lpix->min_residency = obj->integer.value;
> +
> + obj = &element->package.elements[1];
> + if (obj->type != ACPI_TYPE_INTEGER)
> + continue;
> + lpix->wake_latency = obj->integer.value;
> +
> + obj = &element->package.elements[2];
> + if (obj->type != ACPI_TYPE_INTEGER)
> + continue;
> + lpix->flags = obj->integer.value;
> +
> + obj = &element->package.elements[3];
> + if (obj->type != ACPI_TYPE_INTEGER)
> + continue;
> + lpix->arch_flags = obj->integer.value;
> +
> + obj = &element->package.elements[4];
> + if (obj->type != ACPI_TYPE_INTEGER)
> + continue;
> + lpix->res_cnt_freq = obj->integer.value;
> +
> + obj = &element->package.elements[5];
> + if (obj->type != ACPI_TYPE_INTEGER)
> + continue;
> + lpix->enable_parent_state = obj->integer.value;
> + }
> + ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Found %d power states\n",
> + state_count));
> +end:
> + kfree(buffer.pointer);
> + return status;
> +}
> +
> +static int max_leaf_depth, fl_scnt;
> +/*
> + * l_lpi - local LPI state
> + * p_lpi - parent LPI state
> + * c_lpi - composite LPI state
> + */
> +static void combine_lpi_states(struct acpi_processor_lpi *l_lpi,
> + struct acpi_processor_lpi *p_lpi,
> + struct acpi_processor_lpi *c_lpi)
> +{
> + c_lpi->min_residency = max(l_lpi->min_residency, p_lpi->min_residency);
> + c_lpi->wake_latency = l_lpi->wake_latency + p_lpi->wake_latency;
> + c_lpi->enable_parent_state = p_lpi->enable_parent_state;
> + c_lpi->entry_method = l_lpi->entry_method;
> + c_lpi->address = l_lpi->address + p_lpi->address;
> + c_lpi->index = p_lpi->index;
> + c_lpi->flags = p_lpi->flags;
> + c_lpi->arch_flags = p_lpi->arch_flags;
> + strncpy(c_lpi->desc, l_lpi->desc, ACPI_CX_DESC_LEN);
> + strncat(c_lpi->desc, "+", ACPI_CX_DESC_LEN);
> + strncat(c_lpi->desc, p_lpi->desc, ACPI_CX_DESC_LEN);
> +}
> +
> +static int flatten_lpi_states(struct acpi_processor *pr,
> + struct acpi_processor_lpi_info *info,
> + struct acpi_processor_lpi *lpi,
> + uint32_t depth)
> +{
> + int j, scount = info[depth].state_count;
> + struct acpi_processor_lpi *t = info[depth].lpix;
> +
> + for (j = 0; j < scount; j++, t++) {
> + struct acpi_processor_lpi *flpi = &pr->power.lpi_states[fl_scnt];
> + bool valid = false;
> +
> + if (depth == max_leaf_depth) { /* leaf/processor node */
> + memcpy(flpi, t, sizeof(*t));
> + fl_scnt++;
> + valid = true;
> + } else if (lpi && t->index <= lpi->enable_parent_state) {
> + combine_lpi_states(lpi, t, flpi);
> + fl_scnt++;
> + valid = true;
> + }
> + if (valid && depth)
> + flatten_lpi_states(pr, info, flpi, depth - 1);
> + }
> + return 0;
> +}
> +
> +static int acpi_processor_get_lpi_info(struct acpi_processor *pr)
> +{
> + int ret, i;
> + struct acpi_processor_lpi_info *info;
> + struct acpi_device *d = NULL;
> + acpi_handle handle = pr->handle, phandle;
> + acpi_status status;
> +
> + if (!osc_pc_lpi_support_acked)
> + return -EOPNOTSUPP;
> +
> + max_leaf_depth = 0;
> + if (!acpi_has_method(handle, "_LPI"))
> + return -EINVAL;
> + fl_scnt = 0;
> +
> + while (ACPI_SUCCESS(status = acpi_get_parent(handle, &phandle))) {
> + if (!acpi_has_method(handle, "_LPI"))
> + continue;
> + acpi_bus_get_device(handle, &d);
> + if (!strcmp(acpi_device_hid(d), ACPI_PROCESSOR_CONTAINER_HID))
> + break;
> + max_leaf_depth++;
> + handle = phandle;
> + }
> +
> + info = kcalloc(max_leaf_depth + 1, sizeof(*info), GFP_KERNEL);
> + if (!info)
> + return -ENOMEM;
> +
> + phandle = pr->handle;
> + for (i = max_leaf_depth; i >= 0 && ACPI_SUCCESS(status); i--) {
> + handle = phandle;
> + ret = acpi_processor_evaluate_lpi(handle, info + i);
> + if (ret)
> + break;
> + status = acpi_get_parent(handle, &phandle);
> + }
> +
> + flatten_lpi_states(pr, info, NULL, max_leaf_depth);
> +
> + pr->power.count = fl_scnt;
> + for (i = 0; i <= max_leaf_depth; i++)
> + kfree(info[i].lpix);
> + kfree(info);
> +
> + /* Tell driver that _LPI is supported. */
> + pr->flags.has_lpi = 1;
> + pr->flags.power = 1;
> +
> + return 0;
> +}
> +
> +/**
> + * acpi_idle_lpi_enter - enters an ACPI any LPI state
> + * @dev: the target CPU
> + * @drv: cpuidle driver containing cpuidle state info
> + * @index: index of target state
> + *
> + */
> +static int acpi_idle_lpi_enter(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv, int index)
> {
> + struct acpi_processor *pr;
> + struct acpi_processor_lpi *lpi;
> +
> + pr = __this_cpu_read(processors);
> +
> + if (unlikely(!pr))
> + return -EINVAL;
> +
> + lpi = &pr->power.lpi_states[index];
> + if (lpi->entry_method == ACPI_CSTATE_FFH)
> + /* Call into architectural FFH based C-state */
> + return acpi_processor_ffh_lpi_enter(lpi, index);
> return -EINVAL;
> }
>
> +static int acpi_processor_setup_lpi_states(struct acpi_processor *pr)
> +{
> + int i;
> + struct acpi_processor_lpi *lpi;
> + struct cpuidle_state *state;
> + struct cpuidle_driver *drv = &acpi_idle_driver;
> +
> + for (i = 0; i < fl_scnt && i < CPUIDLE_STATE_MAX; i++) {
> + lpi = &pr->power.lpi_states[i];
> +
> + state = &drv->states[i];
> + snprintf(state->name, CPUIDLE_NAME_LEN, "LPI-%d", i);
> + strncpy(state->desc, lpi->desc, CPUIDLE_DESC_LEN);
> + state->exit_latency = lpi->wake_latency;
> + state->target_residency = lpi->min_residency;
> + if (lpi->arch_flags)
> + state->flags |= CPUIDLE_FLAG_TIMER_STOP;
> + state->enter = acpi_idle_lpi_enter;
> + drv->safe_state_index = i;
> + }
> +
> + drv->state_count = i;
> +
> + return 0;
> +}
> +
> +#else
> +static int acpi_processor_ffh_lpi_probe(unsigned int cpu) { return -ENODEV; }
> +static int acpi_processor_get_lpi_info(struct acpi_processor *pr)
> +{
> + return -ENODEV;
> +}
> +
> +static int acpi_processor_setup_lpi_states(struct acpi_processor *pr)
> +{
> + return -EINVAL;
> +}
> #endif
>
> +/**
> + * acpi_processor_setup_cpuidle_states- prepares and configures cpuidle
> + * global state data i.e. idle routines
> + *
> + * @pr: the ACPI processor
> + */
> +static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr)
> +{
> + int i;
> + struct cpuidle_driver *drv = &acpi_idle_driver;
> +
> + if (!pr->flags.power_setup_done)
> + return -EINVAL;
> +
> + if (pr->flags.power == 0)
> + return -EINVAL;
> +
> + drv->safe_state_index = -1;
> + for (i = CPUIDLE_DRIVER_STATE_START; i < CPUIDLE_STATE_MAX; i++) {
> + drv->states[i].name[0] = '\0';
> + drv->states[i].desc[0] = '\0';
> + }
> +
> + if (pr->flags.has_lpi)
> + return acpi_processor_setup_lpi_states(pr);
> + return acpi_processor_setup_cstates(pr);
> +}
> +
> +/**
> + * acpi_processor_setup_cpuidle_dev - prepares and configures CPUIDLE
> + * device i.e. per-cpu data
> + *
> + * @pr: the ACPI processor
> + * @dev : the cpuidle device
> + */
> +static int acpi_processor_setup_cpuidle_dev(struct acpi_processor *pr,
> + struct cpuidle_device *dev)
> +{
> + if (!pr->flags.power_setup_done)
> + return -EINVAL;
> +
> + if (pr->flags.power == 0)
> + return -EINVAL;
> +
> + if (!dev)
> + return -EINVAL;
> +
> + dev->cpu = pr->id;
> + if (pr->flags.has_lpi)
> + return acpi_processor_ffh_lpi_probe(pr->id);
> + else
> + return acpi_processor_setup_cpuidle_cx(pr, dev);
> +}
> +
> +static int acpi_processor_get_power_info(struct acpi_processor *pr)
> +{
> + int ret = 0;
> +
> + ret = acpi_processor_get_cstate_info(pr);
> + if (ret)
> + ret = acpi_processor_get_lpi_info(pr);
> + return ret;
> +}
> +
> int acpi_processor_hotplug(struct acpi_processor *pr)
> {
> int ret = 0;
> @@ -980,7 +1300,7 @@ int acpi_processor_hotplug(struct acpi_processor *pr)
> cpuidle_disable_device(dev);
> acpi_processor_get_power_info(pr);
> if (pr->flags.power) {
> - acpi_processor_setup_cpuidle_cx(pr, dev);
> + acpi_processor_setup_cpuidle_dev(pr, dev);
> ret = cpuidle_enable_device(dev);
> }
> cpuidle_resume_and_unlock();
> @@ -988,7 +1308,7 @@ int acpi_processor_hotplug(struct acpi_processor *pr)
> return ret;
> }
>
> -int acpi_processor_cst_has_changed(struct acpi_processor *pr)
> +int acpi_processor_power_state_has_changed(struct acpi_processor *pr)
> {
> int cpu;
> struct acpi_processor *_pr;
> @@ -1036,7 +1356,7 @@ int acpi_processor_cst_has_changed(struct acpi_processor *pr)
> acpi_processor_get_power_info(_pr);
> if (_pr->flags.power) {
> dev = per_cpu(acpi_cpuidle_device, cpu);
> - acpi_processor_setup_cpuidle_cx(_pr, dev);
> + acpi_processor_setup_cpuidle_dev(_pr, dev);
> cpuidle_enable_device(dev);
> }
> }
> @@ -1093,7 +1413,7 @@ int acpi_processor_power_init(struct acpi_processor *pr)
> return -ENOMEM;
> per_cpu(acpi_cpuidle_device, pr->id) = dev;
>
> - acpi_processor_setup_cpuidle_cx(pr, dev);
> + acpi_processor_setup_cpuidle_dev(pr, dev);
>
> /* Register per-cpu cpuidle_device. Cpuidle driver
> * must already be registered before registering device
> diff --git a/include/acpi/processor.h b/include/acpi/processor.h
> index b447952f8236..5ab11f978e6d 100644
> --- a/include/acpi/processor.h
> +++ b/include/acpi/processor.h
> @@ -67,9 +67,25 @@ struct acpi_processor_cx {
> char desc[ACPI_CX_DESC_LEN];
> };
>
> +struct acpi_processor_lpi {
> + u32 min_residency;
> + u32 wake_latency; /* worst case */
> + u32 flags;
> + u32 arch_flags;
> + u32 res_cnt_freq;
> + u32 enable_parent_state;
> + u64 address;
> + u8 index;
> + u8 entry_method;
> + char desc[ACPI_CX_DESC_LEN];
> +};
> +
> struct acpi_processor_power {
> int count;
> - struct acpi_processor_cx states[ACPI_PROCESSOR_MAX_POWER];
> + union {
> + struct acpi_processor_cx states[ACPI_PROCESSOR_MAX_POWER];
> + struct acpi_processor_lpi lpi_states[ACPI_PROCESSOR_MAX_POWER];
> + };
> int timer_broadcast_on_state;
> };
>
> @@ -189,6 +205,7 @@ struct acpi_processor_flags {
> u8 bm_control:1;
> u8 bm_check:1;
> u8 has_cst:1;
> + u8 has_lpi:1;
> u8 power_setup_done:1;
> u8 bm_rld_set:1;
> u8 need_hotplug_init:1;
> @@ -272,6 +289,11 @@ static inline void acpi_processor_ffh_cstate_enter(struct acpi_processor_cx
> }
> #endif
>
> +#ifdef CONFIG_ARCH_SUPPORTS_ACPI_PROCESSOR_LPI
> +int acpi_processor_ffh_lpi_probe(unsigned int cpu);
> +int acpi_processor_ffh_lpi_enter(struct acpi_processor_lpi *lpi, int idx);
> +#endif
> +
> /* in processor_perflib.c */
>
> #ifdef CONFIG_CPU_FREQ
> @@ -358,7 +380,7 @@ extern struct cpuidle_driver acpi_idle_driver;
> #ifdef CONFIG_ACPI_PROCESSOR_IDLE
> int acpi_processor_power_init(struct acpi_processor *pr);
> int acpi_processor_power_exit(struct acpi_processor *pr);
> -int acpi_processor_cst_has_changed(struct acpi_processor *pr);
> +int acpi_processor_power_state_has_changed(struct acpi_processor *pr);
> int acpi_processor_hotplug(struct acpi_processor *pr);
> #else
> static inline int acpi_processor_power_init(struct acpi_processor *pr)
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index a9e25c4a44d1..6d5208d136ac 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -352,8 +352,12 @@ acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context);
> #define OSC_SB_HOTPLUG_OST_SUPPORT 0x00000008
> #define OSC_SB_APEI_SUPPORT 0x00000010
> #define OSC_SB_CPC_SUPPORT 0x00000020
> +#define OSC_SB_CPCV2_SUPPORT 0x00000040
> +#define OSC_SB_PCLPI_SUPPORT 0x00000080
> +#define OSC_SB_OSLPI_SUPPORT 0x00000100
>
> extern bool osc_sb_apei_support_acked;
> +extern bool osc_pc_lpi_support_acked;
>
> /* PCI Host Bridge _OSC: Capabilities DWORD 2: Support Field */
> #define OSC_PCI_EXT_CONFIG_SUPPORT 0x00000001
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 20+ messages in thread[parent not found: <562EC3A5.2070402@codeaurora.org>]
* Re: [PATCH v2 5/5] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
[not found] ` <562EC3A5.2070402@codeaurora.org>
@ 2015-10-27 14:09 ` Sudeep Holla
2015-11-19 22:03 ` Prakash, Prashanth
0 siblings, 1 reply; 20+ messages in thread
From: Sudeep Holla @ 2015-10-27 14:09 UTC (permalink / raw)
To: Prakash, Prashanth, Ashwin Chaugule
Cc: Sudeep Holla, linux acpi, Rafael J. Wysocki, lkml, linux-ia64,
x86, Al Stone, Lorenzo Pieralisi, Mahesh Sivasubramanian
Hi Prashanth,
On 27/10/15 00:21, Prakash, Prashanth wrote:
> Hi Sudeep,
>
> While testing with these patches everything looked fine except that we
> are flattening all the LPI
> states even if it is disabled. I added a simple check to fix it. Please
> let me know if you have any
> feedback on the same.
>
Looks good to me and thanks for the nice catch. I completely missed to
handle that case. Is it OK if I fold this patch into the next version ?
I have worked on Ashwin's comments, but was waiting to get some feedback
from Rafael before I post newer version.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2 5/5] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
2015-10-27 14:09 ` Sudeep Holla
@ 2015-11-19 22:03 ` Prakash, Prashanth
2015-11-26 11:09 ` Sudeep Holla
0 siblings, 1 reply; 20+ messages in thread
From: Prakash, Prashanth @ 2015-11-19 22:03 UTC (permalink / raw)
To: Sudeep Holla, Ashwin Chaugule
Cc: linux acpi, Rafael J. Wysocki, lkml, linux-ia64, x86, Al Stone,
Lorenzo Pieralisi, Mahesh Sivasubramanian
Hi Sudeep,
On 10/27/2015 8:09 AM, Sudeep Holla wrote:
> Hi Prashanth,
>
> On 27/10/15 00:21, Prakash, Prashanth wrote:
>> Hi Sudeep,
>>
>> While testing with these patches everything looked fine except that we
>> are flattening all the LPI
>> states even if it is disabled. I added a simple check to fix it. Please
>> let me know if you have any
>> feedback on the same.
>>
>
> Looks good to me and thanks for the nice catch. I completely missed to
> handle that case. Is it OK if I fold this patch into the next version ?
>
> I have worked on Ashwin's comments, but was waiting to get some feedback
> from Rafael before I post newer version.
>
Did you had an opportunity to update the LPI patch set? Please let me know
if you need any help in test/verification of the new patch set.
--
Thanks,
Prashanth
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 5/5] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
2015-11-19 22:03 ` Prakash, Prashanth
@ 2015-11-26 11:09 ` Sudeep Holla
0 siblings, 0 replies; 20+ messages in thread
From: Sudeep Holla @ 2015-11-26 11:09 UTC (permalink / raw)
To: Prakash, Prashanth, Ashwin Chaugule
Cc: Sudeep Holla, linux acpi, Rafael J. Wysocki, lkml, linux-ia64,
x86, Al Stone, Lorenzo Pieralisi, Mahesh Sivasubramanian
Hi Prashanth,
On 19/11/15 22:03, Prakash, Prashanth wrote:
> Hi Sudeep,
>
> On 10/27/2015 8:09 AM, Sudeep Holla wrote:
>> Hi Prashanth,
>>
>> On 27/10/15 00:21, Prakash, Prashanth wrote:
>>> Hi Sudeep,
>>>
>>> While testing with these patches everything looked fine except that we
>>> are flattening all the LPI
>>> states even if it is disabled. I added a simple check to fix it. Please
>>> let me know if you have any
>>> feedback on the same.
>>>
>>
>> Looks good to me and thanks for the nice catch. I completely missed to
>> handle that case. Is it OK if I fold this patch into the next version ?
>>
>> I have worked on Ashwin's comments, but was waiting to get some feedback
>> from Rafael before I post newer version.
>>
> Did you had an opportunity to update the LPI patch set? Please let me know
> if you need any help in test/verification of the new patch set.
>
Sorry for the late response, was off for 3 weeks. I will post the new
version early next week after some testing.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 5/5] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
2015-10-26 19:13 ` Ashwin Chaugule
[not found] ` <562EC3A5.2070402@codeaurora.org>
@ 2015-12-01 17:23 ` Prakash, Prashanth
2015-12-01 17:43 ` Sudeep Holla
1 sibling, 1 reply; 20+ messages in thread
From: Prakash, Prashanth @ 2015-12-01 17:23 UTC (permalink / raw)
To: Ashwin Chaugule, Sudeep Holla
Cc: linux acpi, Rafael J. Wysocki, lkml, linux-ia64, x86, Al Stone,
Lorenzo Pieralisi, Mahesh Sivasubramanian, wufan
Hi Sudeep,
>> +static void combine_lpi_states(struct acpi_processor_lpi *l_lpi,
>> + struct acpi_processor_lpi *p_lpi,
>> + struct acpi_processor_lpi *c_lpi)
>> +{
>> + c_lpi->min_residency = max(l_lpi->min_residency, p_lpi->min_residency);
>> + c_lpi->wake_latency = l_lpi->wake_latency + p_lpi->wake_latency;
>> + c_lpi->enable_parent_state = p_lpi->enable_parent_state;
>> + c_lpi->entry_method = l_lpi->entry_method;
>> + c_lpi->address = l_lpi->address + p_lpi->address;
>> + c_lpi->index = p_lpi->index;
>> + c_lpi->flags = p_lpi->flags;
>> + c_lpi->arch_flags = p_lpi->arch_flags;
>> + strncpy(c_lpi->desc, l_lpi->desc, ACPI_CX_DESC_LEN);
>> + strncat(c_lpi->desc, "+", ACPI_CX_DESC_LEN);
>> + strncat(c_lpi->desc, p_lpi->desc, ACPI_CX_DESC_LEN);
>> +}
I suppose you meant to use strl* instead of strn* operations. Below is a
simple patch to fix these. Can you please fold these changes into your next
version as well?
ACPI / Processor: fix buffer overflow caused by strncat/strncpy
The misuse of strncat in LPI code is causing buffer overflow. The fix
is to replace strncat with strlcat.
Signed-off-by: Fan Wu <wufan@codeaurora.org>
Signed-off-by: Prashanth Prakash <pprakash@codeaurora.org>
---
drivers/acpi/processor_idle.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index af851f1..4ca42a7 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -856,7 +856,7 @@ static int acpi_processor_setup_cstates(struct acpi_processor *pr)
state = &drv->states[count];
snprintf(state->name, CPUIDLE_NAME_LEN, "C%d", i);
- strncpy(state->desc, cx->desc, CPUIDLE_DESC_LEN);
+ strlcpy(state->desc, cx->desc, CPUIDLE_DESC_LEN);
state->exit_latency = cx->latency;
state->target_residency = cx->latency * latency_factor;
state->enter = acpi_idle_enter;
@@ -1009,7 +1009,7 @@ static int acpi_processor_evaluate_lpi(acpi_handle handle,
obj = &element->package.elements[9];
if (obj->type == ACPI_TYPE_STRING)
- strncpy(lpix->desc, obj->string.pointer, ACPI_CX_DESC_LEN);
+ strlcpy(lpix->desc, obj->string.pointer, ACPI_CX_DESC_LEN);
lpix->index = state_count;
@@ -1068,9 +1068,9 @@ static void combine_lpi_states(struct acpi_processor_lpi *l_lpi,
c_lpi->index = p_lpi->index;
c_lpi->flags = p_lpi->flags;
c_lpi->arch_flags = p_lpi->arch_flags;
- strncpy(c_lpi->desc, l_lpi->desc, ACPI_CX_DESC_LEN);
- strncat(c_lpi->desc, "+", ACPI_CX_DESC_LEN);
- strncat(c_lpi->desc, p_lpi->desc, ACPI_CX_DESC_LEN);
+ strlcpy(c_lpi->desc, l_lpi->desc, ACPI_CX_DESC_LEN);
+ strlcat(c_lpi->desc, "+", ACPI_CX_DESC_LEN);
+ strlcat(c_lpi->desc, p_lpi->desc, ACPI_CX_DESC_LEN);
}
static int flatten_lpi_states(struct acpi_processor *pr,
@@ -1190,7 +1190,7 @@ static int acpi_processor_setup_lpi_states(struct acpi_processor *pr)
state = &drv->states[i];
snprintf(state->name, CPUIDLE_NAME_LEN, "LPI-%d", i);
- strncpy(state->desc, lpi->desc, CPUIDLE_DESC_LEN);
+ strlcpy(state->desc, lpi->desc, CPUIDLE_DESC_LEN);
state->exit_latency = lpi->wake_latency;
state->target_residency = lpi->min_residency;
if (lpi->arch_flags)
--
1.8.2.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v2 5/5] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
2015-12-01 17:23 ` Prakash, Prashanth
@ 2015-12-01 17:43 ` Sudeep Holla
0 siblings, 0 replies; 20+ messages in thread
From: Sudeep Holla @ 2015-12-01 17:43 UTC (permalink / raw)
To: Prakash, Prashanth, Ashwin Chaugule
Cc: Sudeep Holla, linux acpi, Rafael J. Wysocki, lkml, linux-ia64,
x86, Al Stone, Lorenzo Pieralisi, Mahesh Sivasubramanian, wufan
Hi Prashanth,
On 01/12/15 17:23, Prakash, Prashanth wrote:
> Hi Sudeep,
>>> +static void combine_lpi_states(struct acpi_processor_lpi *l_lpi,
>>> + struct acpi_processor_lpi *p_lpi,
>>> + struct acpi_processor_lpi *c_lpi)
>>> +{
>>> + c_lpi->min_residency = max(l_lpi->min_residency, p_lpi->min_residency);
>>> + c_lpi->wake_latency = l_lpi->wake_latency + p_lpi->wake_latency;
>>> + c_lpi->enable_parent_state = p_lpi->enable_parent_state;
>>> + c_lpi->entry_method = l_lpi->entry_method;
>>> + c_lpi->address = l_lpi->address + p_lpi->address;
>>> + c_lpi->index = p_lpi->index;
>>> + c_lpi->flags = p_lpi->flags;
>>> + c_lpi->arch_flags = p_lpi->arch_flags;
>>> + strncpy(c_lpi->desc, l_lpi->desc, ACPI_CX_DESC_LEN);
>>> + strncat(c_lpi->desc, "+", ACPI_CX_DESC_LEN);
>>> + strncat(c_lpi->desc, p_lpi->desc, ACPI_CX_DESC_LEN);
>>> +}
> I suppose you meant to use strl* instead of strn* operations. Below is a
> simple patch to fix these. Can you please fold these changes into your next
> version as well?
>
Thanks for reporting, I had fixed it already as I ran into same issue
when I was playing around with the description string in the LPI tables.
Just adding some comments to the code now where ever it's not so
obvious, will post it tomorrow.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 20+ messages in thread