* [PATCH v4 0/5] ACPI / processor_idle: Add ACPI v6.0 LPI support
@ 2016-04-19 12:30 Sudeep Holla
2016-04-19 12:30 ` [PATCH v4 1/5] ACPI / processor_idle: introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE Sudeep Holla
` (4 more replies)
0 siblings, 5 replies; 27+ messages in thread
From: Sudeep Holla @ 2016-04-19 12:30 UTC (permalink / raw)
To: Rafael J. Wysocki, linux-acpi, linux-kernel, linux-arm-kernel
Cc: Sudeep Holla, Lorenzo Pieralisi, Al Stone, Prashanth Prakash,
Ashwin Chaugule
ACPI 6.0 introduced LPI(Low Power Idle) states that provides an alternate
method to describe processor idle states. It extends the specification
to allow the expression of idle states like C-states selectable by the
OSPM when a processor goes idle, but may affect more than one processor,
and may affect other system components.
LPI extensions leverages the processor container device(again introduced
in ACPI 6.0) allowing to express which parts of the system are affected
by a given LPI state. 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 series aims at providing basic and initial support for platform
coordinated LPI states.
v3[3]->v4:
- Dropped the preparatory patches that are merged already
- Added ARM64 arch specific callback implementations
- Addressed most of the review comments from Rafael
v2[2]->v3:
- rebased against v4.4-rc3
- fixed couple of issues reported by Prashanth and review comments
from Ashwin
v1[1]->v2[2]:
- Fixed support for ACPI0010 processor container
- moved sleep state code out of processor_idle
Code is also available @[4]
Regards,
Sudeep
[1] http://marc.info/?l=linux-acpi&m=143871041601132&w=2
[2] http://marc.info/?l=linux-acpi&m=144241209800788&w=2
[3] http://marc.info/?l=linux-acpi&m=144906557814813&w=2
[4] git://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git for_review/arm64_lpi
Sudeep Holla (5):
ACPI / processor_idle: introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE
ACPI / processor_idle: Add support for Low Power Idle(LPI) states
drivers: psci: refactor psci_cpu_init_idle in preparation for ACPI LPI support
arm64: add support for ACPI Low Power Idle(LPI)
ACPI : enable ACPI_PROCESSOR_IDLE on ARM64
arch/arm64/Kconfig | 1 +
arch/arm64/kernel/acpi.c | 34 +++
arch/ia64/Kconfig | 1 +
arch/x86/Kconfig | 1 +
drivers/acpi/Kconfig | 5 +-
drivers/acpi/bus.c | 11 +-
drivers/acpi/processor_driver.c | 2 +-
drivers/acpi/processor_idle.c | 511 ++++++++++++++++++++++++++++++++++------
drivers/firmware/psci.c | 63 ++++-
include/acpi/processor.h | 28 ++-
include/linux/acpi.h | 4 +
11 files changed, 566 insertions(+), 95 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v4 1/5] ACPI / processor_idle: introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE
2016-04-19 12:30 [PATCH v4 0/5] ACPI / processor_idle: Add ACPI v6.0 LPI support Sudeep Holla
@ 2016-04-19 12:30 ` Sudeep Holla
2016-04-19 12:49 ` kbuild test robot
` (2 more replies)
2016-04-19 12:30 ` [PATCH v4 2/5] ACPI / processor_idle: Add support for Low Power Idle(LPI) states Sudeep Holla
` (3 subsequent siblings)
4 siblings, 3 replies; 27+ messages in thread
From: Sudeep Holla @ 2016-04-19 12:30 UTC (permalink / raw)
To: Rafael J. Wysocki, linux-acpi, linux-kernel, linux-arm-kernel
Cc: Sudeep Holla, Lorenzo Pieralisi, Al Stone, Prashanth Prakash,
Ashwin Chaugule, x86, linux-ia64
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: x86@kernel.org
Cc: linux-ia64@vger.kernel.org
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 | 74 +++++++++++++++++++++++++++++--------------
include/acpi/processor.h | 3 +-
5 files changed, 57 insertions(+), 25 deletions(-)
diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
index b534ebab36ea..717de2a146e2 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 2dc18605831f..eb03fd0d63b9 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -37,6 +37,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 82b96ee8624c..ec289078667c 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 444e3745c8b3..1f3fe54194b5 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,49 @@ 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++;
+
+ if (acpi_gbl_FADT.cst_control && !nocst) {
+ status = acpi_os_write_port(acpi_gbl_FADT.smi_command,
+ acpi_gbl_FADT.cst_control, 8);
+ if (ACPI_FAILURE(status))
+ ACPI_EXCEPTION((AE_INFO, status,
+ "Notifying BIOS of _CST ability failed"));
+ }
+}
+#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,29 +1062,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++;
- }
-
- if (acpi_gbl_FADT.cst_control && !nocst) {
- status =
- acpi_os_write_port(acpi_gbl_FADT.smi_command, acpi_gbl_FADT.cst_control, 8);
- if (ACPI_FAILURE(status)) {
- ACPI_EXCEPTION((AE_INFO, status,
- "Notifying BIOS of _CST ability failed"));
- }
- }
+ acpi_processor_cstate_first_run_checks();
acpi_processor_get_power_info(pr);
pr->flags.power_setup_done = 1;
diff --git a/include/acpi/processor.h b/include/acpi/processor.h
index 6f1805dd5d3c..50f2423d31fa 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] 27+ messages in thread
* [PATCH v4 2/5] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
2016-04-19 12:30 [PATCH v4 0/5] ACPI / processor_idle: Add ACPI v6.0 LPI support Sudeep Holla
2016-04-19 12:30 ` [PATCH v4 1/5] ACPI / processor_idle: introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE Sudeep Holla
@ 2016-04-19 12:30 ` Sudeep Holla
2016-05-10 0:04 ` Rafael J. Wysocki
2016-05-11 0:03 ` Rafael J. Wysocki
2016-04-19 12:30 ` [PATCH v4 3/5] drivers: psci: refactor psci_cpu_init_idle in preparation for ACPI LPI support Sudeep Holla
` (2 subsequent siblings)
4 siblings, 2 replies; 27+ messages in thread
From: Sudeep Holla @ 2016-04-19 12:30 UTC (permalink / raw)
To: Rafael J. Wysocki, linux-acpi, linux-kernel, linux-arm-kernel
Cc: Sudeep Holla, Lorenzo Pieralisi, Al Stone, Prashanth Prakash,
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/bus.c | 11 +-
drivers/acpi/processor_driver.c | 2 +-
drivers/acpi/processor_idle.c | 441 +++++++++++++++++++++++++++++++++++-----
include/acpi/processor.h | 25 ++-
include/linux/acpi.h | 4 +
5 files changed, 422 insertions(+), 61 deletions(-)
Hi Rafael,
Yet to be discussed(retained as in from previous version):
- Kconfig entry removal: Need feedback on how to deal with that
without having to introduce dummy _CST related ARM64 callbacks
- Didn't defer processing of LPI buffers to flattening as it
results in the same buffer decoded multiple times
- ACPI_CSTATE_INTEGER : IMO it's reasonable to keep it aroundsince the
it's part of LPI specification(not just ARM FFH)
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index c068c829b453..94fe6e0ccfd3 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -302,6 +302,11 @@ acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context)
EXPORT_SYMBOL(acpi_run_osc);
bool osc_sb_apei_support_acked;
+/*
+ * ACPI 6.0 Section 8.4.4.2 Idle State Coordination
+ * OSPM supports platform coordinated low power idle(LPI) states
+ */
+bool osc_pc_lpi_support_confirmed;
static u8 sb_uuid_str[] = "0811B06E-4A27-44F9-8D60-3CBBC22E7B48";
static void acpi_bus_osc_support(void)
{
@@ -322,6 +327,7 @@ 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;
+ capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_PCLPI_SUPPORT;
if (!ghes_disable)
capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_APEI_SUPPORT;
@@ -329,9 +335,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_confirmed =
+ 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 d2fa8cb82d2b..0ca14ac7bb28 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 1f3fe54194b5..5c78d68c7311 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -303,7 +303,6 @@ static int acpi_processor_get_power_info_cst(struct acpi_processor *pr)
struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
union acpi_object *cst;
-
if (nocst)
return -ENODEV;
@@ -576,7 +575,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 +809,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 +837,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;
@@ -893,7 +855,7 @@ static int acpi_processor_setup_cpuidle_states(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;
@@ -929,6 +891,7 @@ static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr)
static inline void acpi_processor_cstate_first_run_checks(void)
{
static int first_run;
+ acpi_status status;
if (first_run)
return;
@@ -951,7 +914,7 @@ 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;
}
@@ -962,13 +925,386 @@ static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr,
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
+struct acpi_processor_lpi_info {
+ int state_count;
+ struct acpi_processor_lpi *lpix;
+};
+
+static int obj_get_integer(union acpi_object *obj, u32 *value)
+{
+ if (obj->type != ACPI_TYPE_INTEGER)
+ return -EINVAL;
+ *value = obj->integer.value;
+ return 0;
+}
+
+static int acpi_processor_evaluate_lpi(acpi_handle handle,
+ struct acpi_processor_lpi_info *info)
+{
+ acpi_status status = 0;
+ int ret = 0;
+ int pkg_count, state_idx = 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_debug("not enough elements in _LPI\n");
+ ret = -ENXIO;
+ goto end;
+ }
+
+ pkg_count = lpi->package.elements[2].integer.value;
+
+ /* Validate number of power states. */
+ if (pkg_count < 1 || pkg_count != lpi->package.count - 3) {
+ pr_debug("count given by _LPI is not valid\n");
+ ret = -ENXIO;
+ goto end;
+ }
+
+ lpix = kcalloc(pkg_count, sizeof(*lpix), GFP_KERNEL);
+ if (!lpix) {
+ ret = -ENOMEM;
+ goto end;
+ }
+
+ info->state_count = pkg_count;
+ info->lpix = lpix;
+ /* LPI States start at index 3 */
+ for (loop = 3; state_idx <= pkg_count; loop++, state_idx++, lpix++) {
+ union acpi_object *element, *pkg_elem, *obj;
+
+ element = &lpi->package.elements[loop];
+ if (element->type != ACPI_TYPE_PACKAGE)
+ continue;
+
+ if (element->package.count < 7)
+ continue;
+
+ pkg_elem = element->package.elements;
+
+ obj = pkg_elem + 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->entry_method = ACPI_CSTATE_INTEGER;
+ lpix->address = obj->integer.value;
+ } else {
+ continue;
+ }
+
+ /* elements[7,8] skipped for now i.e. Residency/Usage counter*/
+
+ obj = pkg_elem + 9;
+ if (obj->type == ACPI_TYPE_STRING)
+ strlcpy(lpix->desc, obj->string.pointer,
+ ACPI_CX_DESC_LEN);
+
+ lpix->index = state_idx;
+ if (obj_get_integer(pkg_elem + 0, &lpix->min_residency)) {
+ pr_debug("No min. residency found, assuming 10 us\n");
+ lpix->min_residency = 10;
+ }
+
+ if (obj_get_integer(pkg_elem + 1, &lpix->wake_latency)) {
+ pr_debug("No wakeup residency found, assuming 10 us\n");
+ lpix->wake_latency = 10;
+ }
+
+ if (obj_get_integer(pkg_elem + 2, &lpix->flags))
+ lpix->flags = 0;
+
+ if (obj_get_integer(pkg_elem + 3, &lpix->arch_flags))
+ lpix->arch_flags = 0;
+
+ if (obj_get_integer(pkg_elem + 4, &lpix->res_cnt_freq))
+ lpix->res_cnt_freq = 1;
+
+ if (obj_get_integer(pkg_elem + 5, &lpix->enable_parent_state))
+ lpix->enable_parent_state = 0;
+ }
+ ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Found %d power states\n",
+ state_idx));
+end:
+ kfree(buffer.pointer);
+ return ret;
+}
+
+static int max_leaf_depth, fl_scnt;
+/**
+ * combine_lpi_states - combine local and parent LPI states to form a
+ * composite LPI state
+ * @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;
+
+ if (p_lpi->entry_method == ACPI_CSTATE_INTEGER)
+ c_lpi->address = l_lpi->address + p_lpi->address;
+ else
+ c_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;
+
+ 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);
+}
+
+#define ACPI_LPI_STATE_FLAGS_ENABLED BIT(0)
+
+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;
+ bool valid = false;
+
+ if (!(t->flags & ACPI_LPI_STATE_FLAGS_ENABLED))
+ continue;
+
+ flpi = &pr->power.lpi_states[fl_scnt];
+ 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;
+ }
+
+ /*
+ * flatten recursively from leaf until the highest level
+ * (e.g. system) is reached
+ */
+ 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, pr_ahandle;
+ acpi_status status;
+
+ if (!osc_pc_lpi_support_confirmed)
+ 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, &pr_ahandle))) {
+ 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 = pr_ahandle;
+ }
+
+ info = kcalloc(max_leaf_depth + 1, sizeof(*info), GFP_KERNEL);
+ if (!info)
+ return -ENOMEM;
+
+ pr_ahandle = pr->handle;
+ for (i = max_leaf_depth; i >= 0 && ACPI_SUCCESS(status); i--) {
+ handle = pr_ahandle;
+ ret = acpi_processor_evaluate_lpi(handle, info + i);
+ if (ret)
+ break;
+ status = acpi_get_parent(handle, &pr_ahandle);
+ }
+
+ /* flatten all the LPI states in the entire hierarchy */
+ 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;
+}
+
+int __weak acpi_processor_ffh_lpi_probe(unsigned int cpu)
+{
+ return -ENODEV;
+}
+
+int __weak acpi_processor_ffh_lpi_enter(struct acpi_processor_lpi *lpi, int idx)
+{
+ return -ENODEV;
+}
+
+/**
+ * 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
+ *
+ * Return: 0 for success or negative value for error
+ */
+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;
+
+ if (!pr->flags.has_lpi)
+ return -EOPNOTSUPP;
+
+ 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);
+ strlcpy(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;
+}
+
+/**
+ * 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);
+ return acpi_processor_setup_cpuidle_cx(pr, dev);
+}
+
+static int acpi_processor_get_power_info(struct acpi_processor *pr)
+{
+ if (acpi_processor_get_lpi_info(pr))
+ return acpi_processor_get_cstate_info(pr);
+ return 0;
+}
+
int acpi_processor_hotplug(struct acpi_processor *pr)
{
int ret = 0;
@@ -977,18 +1313,15 @@ int acpi_processor_hotplug(struct acpi_processor *pr)
if (disabled_by_idle_boot_param())
return 0;
- if (nocst)
- return -ENODEV;
-
if (!pr->flags.power_setup_done)
return -ENODEV;
dev = per_cpu(acpi_cpuidle_device, pr->id);
cpuidle_pause_and_lock();
cpuidle_disable_device(dev);
- acpi_processor_get_power_info(pr);
- if (pr->flags.power) {
- acpi_processor_setup_cpuidle_cx(pr, dev);
+ ret = acpi_processor_get_power_info(pr);
+ if (!ret && pr->flags.power) {
+ acpi_processor_setup_cpuidle_dev(pr, dev);
ret = cpuidle_enable_device(dev);
}
cpuidle_resume_and_unlock();
@@ -996,7 +1329,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;
@@ -1005,9 +1338,6 @@ int acpi_processor_cst_has_changed(struct acpi_processor *pr)
if (disabled_by_idle_boot_param())
return 0;
- if (nocst)
- return -ENODEV;
-
if (!pr->flags.power_setup_done)
return -ENODEV;
@@ -1044,7 +1374,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);
}
}
@@ -1059,7 +1389,6 @@ static int acpi_processor_registered;
int acpi_processor_power_init(struct acpi_processor *pr)
{
- acpi_status status;
int retval;
struct cpuidle_device *dev;
@@ -1092,7 +1421,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 50f2423d31fa..7dd1e63199ed 100644
--- a/include/acpi/processor.h
+++ b/include/acpi/processor.h
@@ -39,6 +39,7 @@
#define ACPI_CSTATE_SYSTEMIO 0
#define ACPI_CSTATE_FFH 1
#define ACPI_CSTATE_HALT 2
+#define ACPI_CSTATE_INTEGER 3
#define ACPI_CX_DESC_LEN 32
@@ -67,9 +68,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 +206,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;
@@ -372,7 +390,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)
@@ -385,7 +403,8 @@ static inline int acpi_processor_power_exit(struct acpi_processor *pr)
return -ENODEV;
}
-static inline int acpi_processor_cst_has_changed(struct acpi_processor *pr)
+static inline int
+acpi_processor_power_state_has_changed(struct acpi_processor *pr)
{
return -ENODEV;
}
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 06ed7e54033e..127fdc04e2b5 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -452,8 +452,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_confirmed;
/* 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] 27+ messages in thread
* [PATCH v4 3/5] drivers: psci: refactor psci_cpu_init_idle in preparation for ACPI LPI support
2016-04-19 12:30 [PATCH v4 0/5] ACPI / processor_idle: Add ACPI v6.0 LPI support Sudeep Holla
2016-04-19 12:30 ` [PATCH v4 1/5] ACPI / processor_idle: introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE Sudeep Holla
2016-04-19 12:30 ` [PATCH v4 2/5] ACPI / processor_idle: Add support for Low Power Idle(LPI) states Sudeep Holla
@ 2016-04-19 12:30 ` Sudeep Holla
2016-06-09 13:24 ` Lorenzo Pieralisi
2016-04-19 12:30 ` [PATCH v4 4/5] arm64: add support for ACPI Low Power Idle(LPI) Sudeep Holla
2016-04-19 12:30 ` [PATCH v4 5/5] ACPI : enable ACPI_PROCESSOR_IDLE on ARM64 Sudeep Holla
4 siblings, 1 reply; 27+ messages in thread
From: Sudeep Holla @ 2016-04-19 12:30 UTC (permalink / raw)
To: Rafael J. Wysocki, linux-acpi, linux-kernel, linux-arm-kernel
Cc: Sudeep Holla, Lorenzo Pieralisi, Al Stone, Prashanth Prakash,
Ashwin Chaugule, Mark Rutland
Inorder to accomodate bot DT and ACPI LPI support in psci_cpu_init_idle,
move the device tree specific into psci_dt_cpu_init_idle.
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
drivers/firmware/psci.c | 23 +++++++++--------------
1 file changed, 9 insertions(+), 14 deletions(-)
diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
index 11bfee8b79a9..af6c5c839568 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -250,11 +250,11 @@ static int __init psci_features(u32 psci_func_id)
#ifdef CONFIG_CPU_IDLE
static DEFINE_PER_CPU_READ_MOSTLY(u32 *, psci_power_state);
-static int psci_dt_cpu_init_idle(struct device_node *cpu_node, int cpu)
+static int psci_dt_cpu_init_idle(unsigned int cpu)
{
int i, ret, count = 0;
u32 *psci_states;
- struct device_node *state_node;
+ struct device_node *state_node, *cpu_node;
/*
* If the PSCI cpu_suspend function hook has not been initialized
@@ -263,6 +263,10 @@ static int psci_dt_cpu_init_idle(struct device_node *cpu_node, int cpu)
if (!psci_ops.cpu_suspend)
return -EOPNOTSUPP;
+ cpu_node = of_get_cpu_node(cpu, NULL);
+ if (!cpu_node)
+ return -ENODEV;
+
/* Count idle states */
while ((state_node = of_parse_phandle(cpu_node, "cpu-idle-states",
count))) {
@@ -303,27 +307,18 @@ static int psci_dt_cpu_init_idle(struct device_node *cpu_node, int cpu)
}
/* Idle states parsed correctly, initialize per-cpu pointer */
per_cpu(psci_power_state, cpu) = psci_states;
+ of_node_put(cpu_node);
return 0;
free_mem:
+ of_node_put(cpu_node);
kfree(psci_states);
return ret;
}
int psci_cpu_init_idle(unsigned int cpu)
{
- struct device_node *cpu_node;
- int ret;
-
- cpu_node = of_get_cpu_node(cpu, NULL);
- if (!cpu_node)
- return -ENODEV;
-
- ret = psci_dt_cpu_init_idle(cpu_node, cpu);
-
- of_node_put(cpu_node);
-
- return ret;
+ return psci_dt_cpu_init_idle(cpu);
}
static int psci_suspend_finisher(unsigned long index)
--
1.9.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v4 4/5] arm64: add support for ACPI Low Power Idle(LPI)
2016-04-19 12:30 [PATCH v4 0/5] ACPI / processor_idle: Add ACPI v6.0 LPI support Sudeep Holla
` (2 preceding siblings ...)
2016-04-19 12:30 ` [PATCH v4 3/5] drivers: psci: refactor psci_cpu_init_idle in preparation for ACPI LPI support Sudeep Holla
@ 2016-04-19 12:30 ` Sudeep Holla
2016-04-19 13:59 ` kbuild test robot
` (4 more replies)
2016-04-19 12:30 ` [PATCH v4 5/5] ACPI : enable ACPI_PROCESSOR_IDLE on ARM64 Sudeep Holla
4 siblings, 5 replies; 27+ messages in thread
From: Sudeep Holla @ 2016-04-19 12:30 UTC (permalink / raw)
To: Rafael J. Wysocki, linux-acpi, linux-kernel, linux-arm-kernel
Cc: Sudeep Holla, Lorenzo Pieralisi, Al Stone, Prashanth Prakash,
Ashwin Chaugule, Mark Rutland
This patch adds appropriate callbacks to support ACPI Low Power Idle
(LPI) on ARM64.
It also selects ARCH_SUPPORTS_ACPI_PROCESSOR_LPI if ACPI is enabled
on ARM64.
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
arch/arm64/Kconfig | 1 +
arch/arm64/kernel/acpi.c | 34 ++++++++++++++++++++++++++++++++++
drivers/firmware/psci.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 83 insertions(+)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 4f436220384f..e7536540387d 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -10,6 +10,7 @@ config ARM64
select ARCH_HAS_SG_CHAIN
select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
select ARCH_USE_CMPXCHG_LOCKREF
+ select ARCH_SUPPORTS_ACPI_PROCESSOR_LPI if ACPI
select ARCH_SUPPORTS_ATOMIC_RMW
select ARCH_WANT_OPTIONAL_GPIOLIB
select ARCH_WANT_COMPAT_IPC_PARSE_VERSION
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index d1ce8e2f98b9..3c05ad5957be 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -18,6 +18,7 @@
#include <linux/acpi.h>
#include <linux/bootmem.h>
#include <linux/cpumask.h>
+#include <linux/cpu_pm.h>
#include <linux/init.h>
#include <linux/irq.h>
#include <linux/irqdomain.h>
@@ -25,9 +26,11 @@
#include <linux/of_fdt.h>
#include <linux/smp.h>
+#include <asm/cpuidle.h>
#include <asm/cputype.h>
#include <asm/cpu_ops.h>
#include <asm/smp_plat.h>
+#include <acpi/processor.h>
#ifdef CONFIG_ACPI_APEI
# include <linux/efi.h>
@@ -211,6 +214,37 @@ void __init acpi_boot_table_init(void)
}
}
+int acpi_processor_ffh_lpi_probe(unsigned int cpu)
+{
+ return arm_cpuidle_init(cpu);
+}
+
+struct acpi_processor_lpi *lpi;
+int acpi_processor_ffh_lpi_enter(struct acpi_processor_lpi *lpi, int idx)
+{
+ int ret;
+
+ if (!idx) {
+ cpu_do_idle();
+ return idx;
+ }
+
+ /* TODO cpu_pm_{enter,exit} can be done in generic code ? */
+ ret = cpu_pm_enter();
+ if (!ret) {
+ /*
+ * Pass idle state index to cpu_suspend which in turn will
+ * call the CPU ops suspend protocol with idle index as a
+ * parameter.
+ */
+ ret = arm_cpuidle_suspend(idx);
+
+ cpu_pm_exit();
+ }
+
+ return ret ? -1 : idx;
+}
+
#ifdef CONFIG_ACPI_APEI
pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
{
diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
index af6c5c839568..70cf2a500d4b 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -24,6 +24,7 @@
#include <linux/reboot.h>
#include <linux/slab.h>
#include <linux/suspend.h>
+#include <linux/acpi.h>
#include <uapi/linux/psci.h>
@@ -32,6 +33,7 @@
#include <asm/system_misc.h>
#include <asm/smp_plat.h>
#include <asm/suspend.h>
+#include <acpi/processor.h>
/*
* While a 64-bit OS can make calls with SMC32 calling conventions, for some
@@ -316,8 +318,54 @@ static int psci_dt_cpu_init_idle(unsigned int cpu)
return ret;
}
+static int __maybe_unused psci_acpi_cpu_init_idle(unsigned int cpu)
+{
+ int i, count;
+ u32 *psci_states;
+ struct acpi_processor *pr;
+ struct acpi_processor_lpi *lpi;
+
+ pr = per_cpu(processors, cpu);
+ if (unlikely(!pr || !pr->flags.has_lpi))
+ return -EINVAL;
+
+ /*
+ * If the PSCI cpu_suspend function hook has not been initialized
+ * idle states must not be enabled, so bail out
+ */
+ if (!psci_ops.cpu_suspend)
+ return -EOPNOTSUPP;
+
+ count = pr->power.count - 1;
+ if (!count)
+ return -ENODEV;
+
+ psci_states = kcalloc(count, sizeof(*psci_states), GFP_KERNEL);
+ if (!psci_states)
+ return -ENOMEM;
+
+ for (i = 0; i < count; i++) {
+ u32 state;
+
+ lpi = &pr->power.lpi_states[i + 1];
+ state = lpi->address & 0xFFFFFFFF;
+ if (!psci_power_state_is_valid(state)) {
+ pr_warn("Invalid PSCI power state %#x\n", state);
+ kfree(psci_states);
+ return -EINVAL;
+ }
+ psci_states[i] = state;
+ }
+ /* Idle states parsed correctly, initialize per-cpu pointer */
+ per_cpu(psci_power_state, cpu) = psci_states;
+ return 0;
+}
+
int psci_cpu_init_idle(unsigned int cpu)
{
+ if (!acpi_disabled)
+ return psci_acpi_cpu_init_idle(cpu);
+
return psci_dt_cpu_init_idle(cpu);
}
--
1.9.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v4 5/5] ACPI : enable ACPI_PROCESSOR_IDLE on ARM64
2016-04-19 12:30 [PATCH v4 0/5] ACPI / processor_idle: Add ACPI v6.0 LPI support Sudeep Holla
` (3 preceding siblings ...)
2016-04-19 12:30 ` [PATCH v4 4/5] arm64: add support for ACPI Low Power Idle(LPI) Sudeep Holla
@ 2016-04-19 12:30 ` Sudeep Holla
4 siblings, 0 replies; 27+ messages in thread
From: Sudeep Holla @ 2016-04-19 12:30 UTC (permalink / raw)
To: Rafael J. Wysocki, linux-acpi, linux-kernel, linux-arm-kernel
Cc: Sudeep Holla, Lorenzo Pieralisi, Al Stone, Prashanth Prakash,
Ashwin Chaugule
Now that ACPI processor idle driver supports LPI(Low Power Idle), lets
enable ACPI_PROCESSOR_IDLE for ARM64
Cc: linux-arm-kernel@lists.infradead.org
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
drivers/acpi/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index ec289078667c..33780ae005fc 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -237,7 +237,7 @@ config ACPI_CPPC_LIB
config ACPI_PROCESSOR
tristate "Processor"
depends on X86 || IA64 || ARM64
- select ACPI_PROCESSOR_IDLE if X86 || IA64
+ select ACPI_PROCESSOR_IDLE
select ACPI_CPU_FREQ_PSS if X86 || IA64
default y
help
--
1.9.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/5] ACPI / processor_idle: introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE
2016-04-19 12:30 ` [PATCH v4 1/5] ACPI / processor_idle: introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE Sudeep Holla
@ 2016-04-19 12:49 ` kbuild test robot
2016-04-19 13:00 ` Sudeep Holla
2016-04-20 9:56 ` Vikas Sajjan
2016-05-10 0:02 ` Rafael J. Wysocki
2 siblings, 1 reply; 27+ messages in thread
From: kbuild test robot @ 2016-04-19 12:49 UTC (permalink / raw)
To: Sudeep Holla
Cc: kbuild-all, Rafael J. Wysocki, linux-acpi, linux-kernel,
linux-arm-kernel, Sudeep Holla, Lorenzo Pieralisi, Al Stone,
Prashanth Prakash, Ashwin Chaugule, x86, linux-ia64
[-- Attachment #1: Type: text/plain, Size: 5448 bytes --]
Hi,
[auto build test ERROR on pm/linux-next]
[also build test ERROR on v4.6-rc4 next-20160419]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/Sudeep-Holla/ACPI-processor_idle-Add-ACPI-v6-0-LPI-support/20160419-203500
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: i386-randconfig-x000-201616 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
Note: the linux-review/Sudeep-Holla/ACPI-processor_idle-Add-ACPI-v6-0-LPI-support/20160419-203500 HEAD c51fc2a756d7b0dce908a4ca043d1d458c400af5 builds fine.
It only hurts bisectibility.
All error/warnings (new ones prefixed by >>):
drivers/acpi/processor_idle.c: In function 'acpi_processor_cstate_first_run_checks':
>> drivers/acpi/processor_idle.c:943:3: error: 'status' undeclared (first use in this function)
status = acpi_os_write_port(acpi_gbl_FADT.smi_command,
^
drivers/acpi/processor_idle.c:943:3: note: each undeclared identifier is reported only once for each function it appears in
drivers/acpi/processor_idle.c: In function 'acpi_processor_power_init':
>> drivers/acpi/processor_idle.c:1062:14: warning: unused variable 'status' [-Wunused-variable]
acpi_status status;
^
vim +/status +943 drivers/acpi/processor_idle.c
937 if (max_cstate < ACPI_C_STATES_MAX)
938 pr_notice("ACPI: processor limited to max C-state %d\n",
939 max_cstate);
940 first_run++;
941
942 if (acpi_gbl_FADT.cst_control && !nocst) {
> 943 status = acpi_os_write_port(acpi_gbl_FADT.smi_command,
944 acpi_gbl_FADT.cst_control, 8);
945 if (ACPI_FAILURE(status))
946 ACPI_EXCEPTION((AE_INFO, status,
947 "Notifying BIOS of _CST ability failed"));
948 }
949 }
950 #else
951
952 static inline int disabled_by_idle_boot_param(void) { return 0; }
953 static inline void acpi_processor_cstate_first_run_checks(void) { }
954 static int acpi_processor_get_power_info(struct acpi_processor *pr)
955 {
956 return -ENODEV;
957 }
958
959 static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr,
960 struct cpuidle_device *dev)
961 {
962 return -EINVAL;
963 }
964
965 static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr)
966 {
967 return -EINVAL;
968 }
969
970 #endif
971
972 int acpi_processor_hotplug(struct acpi_processor *pr)
973 {
974 int ret = 0;
975 struct cpuidle_device *dev;
976
977 if (disabled_by_idle_boot_param())
978 return 0;
979
980 if (nocst)
981 return -ENODEV;
982
983 if (!pr->flags.power_setup_done)
984 return -ENODEV;
985
986 dev = per_cpu(acpi_cpuidle_device, pr->id);
987 cpuidle_pause_and_lock();
988 cpuidle_disable_device(dev);
989 acpi_processor_get_power_info(pr);
990 if (pr->flags.power) {
991 acpi_processor_setup_cpuidle_cx(pr, dev);
992 ret = cpuidle_enable_device(dev);
993 }
994 cpuidle_resume_and_unlock();
995
996 return ret;
997 }
998
999 int acpi_processor_cst_has_changed(struct acpi_processor *pr)
1000 {
1001 int cpu;
1002 struct acpi_processor *_pr;
1003 struct cpuidle_device *dev;
1004
1005 if (disabled_by_idle_boot_param())
1006 return 0;
1007
1008 if (nocst)
1009 return -ENODEV;
1010
1011 if (!pr->flags.power_setup_done)
1012 return -ENODEV;
1013
1014 /*
1015 * FIXME: Design the ACPI notification to make it once per
1016 * system instead of once per-cpu. This condition is a hack
1017 * to make the code that updates C-States be called once.
1018 */
1019
1020 if (pr->id == 0 && cpuidle_get_driver() == &acpi_idle_driver) {
1021
1022 /* Protect against cpu-hotplug */
1023 get_online_cpus();
1024 cpuidle_pause_and_lock();
1025
1026 /* Disable all cpuidle devices */
1027 for_each_online_cpu(cpu) {
1028 _pr = per_cpu(processors, cpu);
1029 if (!_pr || !_pr->flags.power_setup_done)
1030 continue;
1031 dev = per_cpu(acpi_cpuidle_device, cpu);
1032 cpuidle_disable_device(dev);
1033 }
1034
1035 /* Populate Updated C-state information */
1036 acpi_processor_get_power_info(pr);
1037 acpi_processor_setup_cpuidle_states(pr);
1038
1039 /* Enable all cpuidle devices */
1040 for_each_online_cpu(cpu) {
1041 _pr = per_cpu(processors, cpu);
1042 if (!_pr || !_pr->flags.power_setup_done)
1043 continue;
1044 acpi_processor_get_power_info(_pr);
1045 if (_pr->flags.power) {
1046 dev = per_cpu(acpi_cpuidle_device, cpu);
1047 acpi_processor_setup_cpuidle_cx(_pr, dev);
1048 cpuidle_enable_device(dev);
1049 }
1050 }
1051 cpuidle_resume_and_unlock();
1052 put_online_cpus();
1053 }
1054
1055 return 0;
1056 }
1057
1058 static int acpi_processor_registered;
1059
1060 int acpi_processor_power_init(struct acpi_processor *pr)
1061 {
> 1062 acpi_status status;
1063 int retval;
1064 struct cpuidle_device *dev;
1065
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 24581 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/5] ACPI / processor_idle: introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE
2016-04-19 12:49 ` kbuild test robot
@ 2016-04-19 13:00 ` Sudeep Holla
0 siblings, 0 replies; 27+ messages in thread
From: Sudeep Holla @ 2016-04-19 13:00 UTC (permalink / raw)
To: kbuild test robot
Cc: Sudeep Holla, kbuild-all, Rafael J. Wysocki, linux-acpi,
linux-kernel, linux-arm-kernel, Lorenzo Pieralisi, Al Stone,
Prashanth Prakash, Ashwin Chaugule, x86, linux-ia64
Hi,
On 19/04/16 13:49, kbuild test robot wrote:
> Hi,
>
> [auto build test ERROR on pm/linux-next]
> [also build test ERROR on v4.6-rc4 next-20160419]
> [if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
>
> url: https://github.com/0day-ci/linux/commits/Sudeep-Holla/ACPI-processor_idle-Add-ACPI-v6-0-LPI-support/20160419-203500
> base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
> config: i386-randconfig-x000-201616 (attached as .config)
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=i386
>
> Note: the linux-review/Sudeep-Holla/ACPI-processor_idle-Add-ACPI-v6-0-LPI-support/20160419-203500 HEAD c51fc2a756d7b0dce908a4ca043d1d458c400af5 builds fine.
> It only hurts bisectibility.
>
Thanks for the report, it's now fixed locally.
It will part of the next version.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 4/5] arm64: add support for ACPI Low Power Idle(LPI)
2016-04-19 12:30 ` [PATCH v4 4/5] arm64: add support for ACPI Low Power Idle(LPI) Sudeep Holla
@ 2016-04-19 13:59 ` kbuild test robot
2016-04-19 15:42 ` Sudeep Holla
2016-04-20 9:59 ` Vikas Sajjan
` (3 subsequent siblings)
4 siblings, 1 reply; 27+ messages in thread
From: kbuild test robot @ 2016-04-19 13:59 UTC (permalink / raw)
To: Sudeep Holla
Cc: kbuild-all, Rafael J. Wysocki, linux-acpi, linux-kernel,
linux-arm-kernel, Sudeep Holla, Lorenzo Pieralisi, Al Stone,
Prashanth Prakash, Ashwin Chaugule, Mark Rutland
[-- Attachment #1: Type: text/plain, Size: 2185 bytes --]
Hi,
[auto build test ERROR on pm/linux-next]
[also build test ERROR on v4.6-rc4 next-20160419]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/Sudeep-Holla/ACPI-processor_idle-Add-ACPI-v6-0-LPI-support/20160419-203500
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: arm-sunxi_defconfig (attached as .config)
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm
All errors (new ones prefixed by >>):
In file included from drivers/firmware/psci.c:36:0:
>> include/acpi/processor.h:7:22: fatal error: asm/acpi.h: No such file or directory
#include <asm/acpi.h>
^
compilation terminated.
vim +7 include/acpi/processor.h
^1da177e4c Linus Torvalds 2005-04-16 1 #ifndef __ACPI_PROCESSOR_H
^1da177e4c Linus Torvalds 2005-04-16 2 #define __ACPI_PROCESSOR_H
^1da177e4c Linus Torvalds 2005-04-16 3
^1da177e4c Linus Torvalds 2005-04-16 4 #include <linux/kernel.h>
3b2d99429e Venkatesh Pallipadi 2005-12-14 5 #include <linux/cpu.h>
d9460fd227 Zhang Rui 2008-01-17 6 #include <linux/thermal.h>
02df8b9385 Venkatesh Pallipadi 2005-04-15 @7 #include <asm/acpi.h>
02df8b9385 Venkatesh Pallipadi 2005-04-15 8
ac212b6980 Rafael J. Wysocki 2013-05-03 9 #define ACPI_PROCESSOR_CLASS "processor"
ac212b6980 Rafael J. Wysocki 2013-05-03 10 #define ACPI_PROCESSOR_DEVICE_NAME "Processor"
:::::: The code at line 7 was first introduced by commit
:::::: 02df8b9385c21fdba165bd380f60eca1d3b0578b [ACPI] enable C2 and C3 idle power states on SMP http://bugzilla.kernel.org/show_bug.cgi?id=4401
:::::: TO: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
:::::: CC: Len Brown <len.brown@intel.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 19252 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 4/5] arm64: add support for ACPI Low Power Idle(LPI)
2016-04-19 13:59 ` kbuild test robot
@ 2016-04-19 15:42 ` Sudeep Holla
0 siblings, 0 replies; 27+ messages in thread
From: Sudeep Holla @ 2016-04-19 15:42 UTC (permalink / raw)
To: kbuild test robot
Cc: Sudeep Holla, kbuild-all, Rafael J. Wysocki, linux-acpi,
linux-kernel, linux-arm-kernel, Lorenzo Pieralisi, Al Stone,
Prashanth Prakash, Ashwin Chaugule, Mark Rutland
On 19/04/16 14:59, kbuild test robot wrote:
> Hi,
>
> [auto build test ERROR on pm/linux-next]
> [also build test ERROR on v4.6-rc4 next-20160419]
> [if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
>
> url: https://github.com/0day-ci/linux/commits/Sudeep-Holla/ACPI-processor_idle-Add-ACPI-v6-0-LPI-support/20160419-203500
> base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
> config: arm-sunxi_defconfig (attached as .config)
> reproduce:
> wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=arm
>
> All errors (new ones prefixed by >>):
>
> In file included from drivers/firmware/psci.c:36:0:
>>> include/acpi/processor.h:7:22: fatal error: asm/acpi.h: No such file or directory
> #include <asm/acpi.h>
> ^
Thanks again for the report, I had totally forgotten the need to check
ARM32 build. I now realize that PSCI driver is common now. Sorry for
that, fixed locally for now.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/5] ACPI / processor_idle: introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE
2016-04-19 12:30 ` [PATCH v4 1/5] ACPI / processor_idle: introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE Sudeep Holla
2016-04-19 12:49 ` kbuild test robot
@ 2016-04-20 9:56 ` Vikas Sajjan
2016-04-20 10:09 ` Sudeep Holla
2016-05-10 0:02 ` Rafael J. Wysocki
2 siblings, 1 reply; 27+ messages in thread
From: Vikas Sajjan @ 2016-04-20 9:56 UTC (permalink / raw)
To: Sudeep Holla
Cc: Rafael J. Wysocki, linux-acpi, linux-kernel, linux-arm-kernel,
Lorenzo Pieralisi, linux-ia64, Al Stone, Prashanth Prakash, x86,
Ashwin Chaugule
Hi Sudeep,
On Tue, Apr 19, 2016 at 6:00 PM, 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: x86@kernel.org
> Cc: linux-ia64@vger.kernel.org
> 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 | 74 +++++++++++++++++++++++++++++--------------
> include/acpi/processor.h | 3 +-
> 5 files changed, 57 insertions(+), 25 deletions(-)
>
> diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
> index b534ebab36ea..717de2a146e2 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 2dc18605831f..eb03fd0d63b9 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -37,6 +37,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 82b96ee8624c..ec289078667c 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 444e3745c8b3..1f3fe54194b5 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,49 @@ 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++;
> +
> + if (acpi_gbl_FADT.cst_control && !nocst) {
> + status = acpi_os_write_port(acpi_gbl_FADT.smi_command,
> + acpi_gbl_FADT.cst_control, 8);
> + if (ACPI_FAILURE(status))
> + ACPI_EXCEPTION((AE_INFO, status,
> + "Notifying BIOS of _CST ability failed"));
> + }
> +}
> +#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,29 +1062,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++;
> - }
> -
> - if (acpi_gbl_FADT.cst_control && !nocst) {
> - status =
> - acpi_os_write_port(acpi_gbl_FADT.smi_command, acpi_gbl_FADT.cst_control, 8);
> - if (ACPI_FAILURE(status)) {
> - ACPI_EXCEPTION((AE_INFO, status,
> - "Notifying BIOS of _CST ability failed"));
> - }
> - }
> + acpi_processor_cstate_first_run_checks();
>
> acpi_processor_get_power_info(pr);
> pr->flags.power_setup_done = 1;
Not related to your change,
The acpi_processor_get_power_info() function can return failure, so i
thought it makes sense to check for the return value
and then set the flag pr->flags.power_setup_done appropriately.
> diff --git a/include/acpi/processor.h b/include/acpi/processor.h
> index 6f1805dd5d3c..50f2423d31fa 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
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 4/5] arm64: add support for ACPI Low Power Idle(LPI)
2016-04-19 12:30 ` [PATCH v4 4/5] arm64: add support for ACPI Low Power Idle(LPI) Sudeep Holla
2016-04-19 13:59 ` kbuild test robot
@ 2016-04-20 9:59 ` Vikas Sajjan
2016-04-20 10:20 ` Sudeep Holla
2016-04-20 10:39 ` Jisheng Zhang
` (2 subsequent siblings)
4 siblings, 1 reply; 27+ messages in thread
From: Vikas Sajjan @ 2016-04-20 9:59 UTC (permalink / raw)
To: Sudeep Holla
Cc: Rafael J. Wysocki, linux-acpi, linux-kernel, linux-arm-kernel,
Lorenzo Pieralisi, Al Stone, Prashanth Prakash, Ashwin Chaugule,
Mark Rutland, Vikas C Sajjan, Sunil V L
Hi Sudeep,
On Tue, Apr 19, 2016 at 6:00 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
> This patch adds appropriate callbacks to support ACPI Low Power Idle
> (LPI) on ARM64.
>
> It also selects ARCH_SUPPORTS_ACPI_PROCESSOR_LPI if ACPI is enabled
> on ARM64.
>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> arch/arm64/Kconfig | 1 +
> arch/arm64/kernel/acpi.c | 34 ++++++++++++++++++++++++++++++++++
> drivers/firmware/psci.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 83 insertions(+)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 4f436220384f..e7536540387d 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -10,6 +10,7 @@ config ARM64
> select ARCH_HAS_SG_CHAIN
> select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
> select ARCH_USE_CMPXCHG_LOCKREF
> + select ARCH_SUPPORTS_ACPI_PROCESSOR_LPI if ACPI
> select ARCH_SUPPORTS_ATOMIC_RMW
> select ARCH_WANT_OPTIONAL_GPIOLIB
> select ARCH_WANT_COMPAT_IPC_PARSE_VERSION
> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> index d1ce8e2f98b9..3c05ad5957be 100644
> --- a/arch/arm64/kernel/acpi.c
> +++ b/arch/arm64/kernel/acpi.c
> @@ -18,6 +18,7 @@
> #include <linux/acpi.h>
> #include <linux/bootmem.h>
> #include <linux/cpumask.h>
> +#include <linux/cpu_pm.h>
> #include <linux/init.h>
> #include <linux/irq.h>
> #include <linux/irqdomain.h>
> @@ -25,9 +26,11 @@
> #include <linux/of_fdt.h>
> #include <linux/smp.h>
>
> +#include <asm/cpuidle.h>
> #include <asm/cputype.h>
> #include <asm/cpu_ops.h>
> #include <asm/smp_plat.h>
> +#include <acpi/processor.h>
>
> #ifdef CONFIG_ACPI_APEI
> # include <linux/efi.h>
> @@ -211,6 +214,37 @@ void __init acpi_boot_table_init(void)
> }
> }
>
> +int acpi_processor_ffh_lpi_probe(unsigned int cpu)
> +{
> + return arm_cpuidle_init(cpu);
> +}
> +
This is generating warning as below:
WARNING: vmlinux.o(.text+0x11024): Section mismatch in reference from
the function acpi_processor_ffh_lpi_probe() to the function
.init.text:arm_cpuidle_init()
The function acpi_processor_ffh_lpi_probe() references
the function __init arm_cpuidle_init().
This is often because acpi_processor_ffh_lpi_probe lacks a __init
annotation or the annotation of arm_cpuidle_init is wrong.
> +struct acpi_processor_lpi *lpi;
> +int acpi_processor_ffh_lpi_enter(struct acpi_processor_lpi *lpi, int idx)
Wondering how are you handling with Resource Dependencies for Idle.
I mean _RDI needs to be taken care, since the dependency between the
power resources and the LPI state is described in _RDI.
> +{
> + int ret;
> +
> + if (!idx) {
> + cpu_do_idle();
> + return idx;
> + }
> +
> + /* TODO cpu_pm_{enter,exit} can be done in generic code ? */
> + ret = cpu_pm_enter();
> + if (!ret) {
> + /*
> + * Pass idle state index to cpu_suspend which in turn will
> + * call the CPU ops suspend protocol with idle index as a
> + * parameter.
> + */
> + ret = arm_cpuidle_suspend(idx);
> +
> + cpu_pm_exit();
> + }
> +
> + return ret ? -1 : idx;
> +}
> +
> #ifdef CONFIG_ACPI_APEI
> pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
> {
> diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
> index af6c5c839568..70cf2a500d4b 100644
> --- a/drivers/firmware/psci.c
> +++ b/drivers/firmware/psci.c
> @@ -24,6 +24,7 @@
> #include <linux/reboot.h>
> #include <linux/slab.h>
> #include <linux/suspend.h>
> +#include <linux/acpi.h>
>
> #include <uapi/linux/psci.h>
>
> @@ -32,6 +33,7 @@
> #include <asm/system_misc.h>
> #include <asm/smp_plat.h>
> #include <asm/suspend.h>
> +#include <acpi/processor.h>
>
> /*
> * While a 64-bit OS can make calls with SMC32 calling conventions, for some
> @@ -316,8 +318,54 @@ static int psci_dt_cpu_init_idle(unsigned int cpu)
> return ret;
> }
>
> +static int __maybe_unused psci_acpi_cpu_init_idle(unsigned int cpu)
> +{
> + int i, count;
> + u32 *psci_states;
> + struct acpi_processor *pr;
> + struct acpi_processor_lpi *lpi;
> +
> + pr = per_cpu(processors, cpu);
> + if (unlikely(!pr || !pr->flags.has_lpi))
> + return -EINVAL;
> +
> + /*
> + * If the PSCI cpu_suspend function hook has not been initialized
> + * idle states must not be enabled, so bail out
> + */
> + if (!psci_ops.cpu_suspend)
> + return -EOPNOTSUPP;
> +
> + count = pr->power.count - 1;
> + if (!count)
> + return -ENODEV;
> +
> + psci_states = kcalloc(count, sizeof(*psci_states), GFP_KERNEL);
> + if (!psci_states)
> + return -ENOMEM;
> +
> + for (i = 0; i < count; i++) {
> + u32 state;
> +
> + lpi = &pr->power.lpi_states[i + 1];
> + state = lpi->address & 0xFFFFFFFF;
> + if (!psci_power_state_is_valid(state)) {
> + pr_warn("Invalid PSCI power state %#x\n", state);
> + kfree(psci_states);
> + return -EINVAL;
> + }
> + psci_states[i] = state;
> + }
> + /* Idle states parsed correctly, initialize per-cpu pointer */
> + per_cpu(psci_power_state, cpu) = psci_states;
> + return 0;
> +}
> +
> int psci_cpu_init_idle(unsigned int cpu)
> {
> + if (!acpi_disabled)
> + return psci_acpi_cpu_init_idle(cpu);
> +
> return psci_dt_cpu_init_idle(cpu);
> }
>
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/5] ACPI / processor_idle: introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE
2016-04-20 9:56 ` Vikas Sajjan
@ 2016-04-20 10:09 ` Sudeep Holla
0 siblings, 0 replies; 27+ messages in thread
From: Sudeep Holla @ 2016-04-20 10:09 UTC (permalink / raw)
To: Vikas Sajjan
Cc: Sudeep Holla, Rafael J. Wysocki, linux-acpi, linux-kernel,
linux-arm-kernel, Lorenzo Pieralisi, linux-ia64, Al Stone,
Prashanth Prakash, x86, Ashwin Chaugule
On 20/04/16 10:56, Vikas Sajjan wrote:
> Hi Sudeep,
>
> On Tue, Apr 19, 2016 at 6:00 PM, 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.
>>
[...]
>> @@ -1018,29 +1062,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++;
>> - }
>> -
>> - if (acpi_gbl_FADT.cst_control && !nocst) {
>> - status =
>> - acpi_os_write_port(acpi_gbl_FADT.smi_command, acpi_gbl_FADT.cst_control, 8);
>> - if (ACPI_FAILURE(status)) {
>> - ACPI_EXCEPTION((AE_INFO, status,
>> - "Notifying BIOS of _CST ability failed"));
>> - }
>> - }
>> + acpi_processor_cstate_first_run_checks();
>>
>> acpi_processor_get_power_info(pr);
>> pr->flags.power_setup_done = 1;
>
> Not related to your change,
> The acpi_processor_get_power_info() function can return failure, so i
> thought it makes sense to check for the return value
> and then set the flag pr->flags.power_setup_done appropriately.
>
Makes sense, will do that.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 4/5] arm64: add support for ACPI Low Power Idle(LPI)
2016-04-20 9:59 ` Vikas Sajjan
@ 2016-04-20 10:20 ` Sudeep Holla
0 siblings, 0 replies; 27+ messages in thread
From: Sudeep Holla @ 2016-04-20 10:20 UTC (permalink / raw)
To: Vikas Sajjan
Cc: Sudeep Holla, Rafael J. Wysocki, linux-acpi, linux-kernel,
linux-arm-kernel, Lorenzo Pieralisi, Al Stone, Prashanth Prakash,
Ashwin Chaugule, Mark Rutland, Vikas C Sajjan, Sunil V L
On 20/04/16 10:59, Vikas Sajjan wrote:
> Hi Sudeep,
>
> On Tue, Apr 19, 2016 at 6:00 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>> This patch adds appropriate callbacks to support ACPI Low Power Idle
>> (LPI) on ARM64.
>>
>> It also selects ARCH_SUPPORTS_ACPI_PROCESSOR_LPI if ACPI is enabled
>> on ARM64.
>>
[...]
>> @@ -211,6 +214,37 @@ void __init acpi_boot_table_init(void)
>> }
>> }
>>
>> +int acpi_processor_ffh_lpi_probe(unsigned int cpu)
>> +{
>> + return arm_cpuidle_init(cpu);
>> +}
>> +
>
> This is generating warning as below:
>
> WARNING: vmlinux.o(.text+0x11024): Section mismatch in reference from
> the function acpi_processor_ffh_lpi_probe() to the function
> .init.text:arm_cpuidle_init()
> The function acpi_processor_ffh_lpi_probe() references
> the function __init arm_cpuidle_init().
> This is often because acpi_processor_ffh_lpi_probe lacks a __init
> annotation or the annotation of arm_cpuidle_init is wrong.
>
I am aware of this and needs to be fixed. I posted ARM64/PSCI related
patches for completeness.
We can't have __init annotation for ..ffh_lpi_probe as it can be called
from hotplug paths in ACPI. Only solution I see is to remove __init tag
for arm_cpuidle_init. I raised similar concern on the other thread
yesterday[1]
Thanks for looking at these patches, much appreciated.
>
>> +struct acpi_processor_lpi *lpi;
>> +int acpi_processor_ffh_lpi_enter(struct acpi_processor_lpi *lpi, int idx)
>
> Wondering how are you handling with Resource Dependencies for Idle.
> I mean _RDI needs to be taken care, since the dependency between the
> power resources and the LPI state is described in _RDI.
>
Correct, right now I haven't considered RDI yet as I don't have proper
platform to test. IMO it can be added later as RDI is optional and not
used on all platforms.
--
Regards,
Sudeep
[1] http://lkml.iu.edu/hypermail/linux/kernel/1604.2/02181.html
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 4/5] arm64: add support for ACPI Low Power Idle(LPI)
2016-04-19 12:30 ` [PATCH v4 4/5] arm64: add support for ACPI Low Power Idle(LPI) Sudeep Holla
2016-04-19 13:59 ` kbuild test robot
2016-04-20 9:59 ` Vikas Sajjan
@ 2016-04-20 10:39 ` Jisheng Zhang
2016-04-26 15:51 ` Prakash, Prashanth
2016-05-11 0:07 ` Rafael J. Wysocki
4 siblings, 0 replies; 27+ messages in thread
From: Jisheng Zhang @ 2016-04-20 10:39 UTC (permalink / raw)
To: Sudeep Holla
Cc: Rafael J. Wysocki, linux-acpi, linux-kernel, linux-arm-kernel,
Mark Rutland, Lorenzo Pieralisi, Al Stone, Prashanth Prakash,
Ashwin Chaugule
Dear Sudeep,
On Tue, 19 Apr 2016 13:30:12 +0100 Sudeep Holla wrote:
> This patch adds appropriate callbacks to support ACPI Low Power Idle
> (LPI) on ARM64.
>
> It also selects ARCH_SUPPORTS_ACPI_PROCESSOR_LPI if ACPI is enabled
> on ARM64.
>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> arch/arm64/Kconfig | 1 +
> arch/arm64/kernel/acpi.c | 34 ++++++++++++++++++++++++++++++++++
> drivers/firmware/psci.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 83 insertions(+)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 4f436220384f..e7536540387d 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -10,6 +10,7 @@ config ARM64
> select ARCH_HAS_SG_CHAIN
> select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
> select ARCH_USE_CMPXCHG_LOCKREF
> + select ARCH_SUPPORTS_ACPI_PROCESSOR_LPI if ACPI
> select ARCH_SUPPORTS_ATOMIC_RMW
> select ARCH_WANT_OPTIONAL_GPIOLIB
> select ARCH_WANT_COMPAT_IPC_PARSE_VERSION
> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> index d1ce8e2f98b9..3c05ad5957be 100644
> --- a/arch/arm64/kernel/acpi.c
> +++ b/arch/arm64/kernel/acpi.c
> @@ -18,6 +18,7 @@
> #include <linux/acpi.h>
> #include <linux/bootmem.h>
> #include <linux/cpumask.h>
> +#include <linux/cpu_pm.h>
> #include <linux/init.h>
> #include <linux/irq.h>
> #include <linux/irqdomain.h>
> @@ -25,9 +26,11 @@
> #include <linux/of_fdt.h>
> #include <linux/smp.h>
>
> +#include <asm/cpuidle.h>
> #include <asm/cputype.h>
> #include <asm/cpu_ops.h>
> #include <asm/smp_plat.h>
> +#include <acpi/processor.h>
It's better to keep headers alphabetic sorted
>
> #ifdef CONFIG_ACPI_APEI
> # include <linux/efi.h>
> @@ -211,6 +214,37 @@ void __init acpi_boot_table_init(void)
> }
> }
>
> +int acpi_processor_ffh_lpi_probe(unsigned int cpu)
> +{
> + return arm_cpuidle_init(cpu);
> +}
> +
> +struct acpi_processor_lpi *lpi;
> +int acpi_processor_ffh_lpi_enter(struct acpi_processor_lpi *lpi, int idx)
> +{
> + int ret;
> +
> + if (!idx) {
> + cpu_do_idle();
> + return idx;
> + }
> +
> + /* TODO cpu_pm_{enter,exit} can be done in generic code ? */
> + ret = cpu_pm_enter();
> + if (!ret) {
> + /*
> + * Pass idle state index to cpu_suspend which in turn will
> + * call the CPU ops suspend protocol with idle index as a
> + * parameter.
> + */
> + ret = arm_cpuidle_suspend(idx);
> +
> + cpu_pm_exit();
> + }
> +
> + return ret ? -1 : idx;
> +}
> +
> #ifdef CONFIG_ACPI_APEI
> pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
> {
> diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
> index af6c5c839568..70cf2a500d4b 100644
> --- a/drivers/firmware/psci.c
> +++ b/drivers/firmware/psci.c
> @@ -24,6 +24,7 @@
> #include <linux/reboot.h>
> #include <linux/slab.h>
> #include <linux/suspend.h>
> +#include <linux/acpi.h>
same here
>
> #include <uapi/linux/psci.h>
>
> @@ -32,6 +33,7 @@
> #include <asm/system_misc.h>
> #include <asm/smp_plat.h>
> #include <asm/suspend.h>
> +#include <acpi/processor.h>
ditto
>
> /*
> * While a 64-bit OS can make calls with SMC32 calling conventions, for some
> @@ -316,8 +318,54 @@ static int psci_dt_cpu_init_idle(unsigned int cpu)
> return ret;
> }
>
> +static int __maybe_unused psci_acpi_cpu_init_idle(unsigned int cpu)
> +{
> + int i, count;
> + u32 *psci_states;
> + struct acpi_processor *pr;
> + struct acpi_processor_lpi *lpi;
> +
> + pr = per_cpu(processors, cpu);
> + if (unlikely(!pr || !pr->flags.has_lpi))
> + return -EINVAL;
> +
> + /*
> + * If the PSCI cpu_suspend function hook has not been initialized
> + * idle states must not be enabled, so bail out
> + */
> + if (!psci_ops.cpu_suspend)
> + return -EOPNOTSUPP;
> +
> + count = pr->power.count - 1;
> + if (!count)
> + return -ENODEV;
> +
> + psci_states = kcalloc(count, sizeof(*psci_states), GFP_KERNEL);
> + if (!psci_states)
> + return -ENOMEM;
> +
> + for (i = 0; i < count; i++) {
> + u32 state;
> +
> + lpi = &pr->power.lpi_states[i + 1];
> + state = lpi->address & 0xFFFFFFFF;
> + if (!psci_power_state_is_valid(state)) {
> + pr_warn("Invalid PSCI power state %#x\n", state);
> + kfree(psci_states);
> + return -EINVAL;
> + }
> + psci_states[i] = state;
> + }
> + /* Idle states parsed correctly, initialize per-cpu pointer */
> + per_cpu(psci_power_state, cpu) = psci_states;
> + return 0;
> +}
> +
> int psci_cpu_init_idle(unsigned int cpu)
> {
> + if (!acpi_disabled)
> + return psci_acpi_cpu_init_idle(cpu);
> +
> return psci_dt_cpu_init_idle(cpu);
> }
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 4/5] arm64: add support for ACPI Low Power Idle(LPI)
2016-04-19 12:30 ` [PATCH v4 4/5] arm64: add support for ACPI Low Power Idle(LPI) Sudeep Holla
` (2 preceding siblings ...)
2016-04-20 10:39 ` Jisheng Zhang
@ 2016-04-26 15:51 ` Prakash, Prashanth
2016-04-26 16:01 ` Sudeep Holla
2016-05-11 0:07 ` Rafael J. Wysocki
4 siblings, 1 reply; 27+ messages in thread
From: Prakash, Prashanth @ 2016-04-26 15:51 UTC (permalink / raw)
To: Sudeep Holla, Rafael J. Wysocki, linux-acpi, linux-kernel,
linux-arm-kernel
Cc: Lorenzo Pieralisi, Al Stone, Ashwin Chaugule, Mark Rutland
Hi Sudeep,
On 4/19/2016 6:30 AM, Sudeep Holla wrote:
> +struct acpi_processor_lpi *lpi;
> +int acpi_processor_ffh_lpi_enter(struct acpi_processor_lpi *lpi, int idx)
> +{
> + int ret;
> +
> + if (!idx) {
> + cpu_do_idle();
> + return idx;
> + }
> +
> + /* TODO cpu_pm_{enter,exit} can be done in generic code ? */
> + ret = cpu_pm_enter();
Can we avoid calling cpu_pm_enter and cpu_pm_exit for retention states as it is not necessary?
May be we can check LPI architecture specific context loss flags prior to calling these.
> + if (!ret) {
> + /*
> + * Pass idle state index to cpu_suspend which in turn will
> + * call the CPU ops suspend protocol with idle index as a
> + * parameter.
> + */
> + ret = arm_cpuidle_suspend(idx);
> +
> + cpu_pm_exit();
same here
> + }
> +
> + return ret ? -1 : idx;
> +}
> +
By the way, thanks for posting these patches!
-Prashanth
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 4/5] arm64: add support for ACPI Low Power Idle(LPI)
2016-04-26 15:51 ` Prakash, Prashanth
@ 2016-04-26 16:01 ` Sudeep Holla
0 siblings, 0 replies; 27+ messages in thread
From: Sudeep Holla @ 2016-04-26 16:01 UTC (permalink / raw)
To: Prakash, Prashanth, Rafael J. Wysocki, linux-acpi, linux-kernel,
linux-arm-kernel
Cc: Sudeep Holla, Lorenzo Pieralisi, Al Stone, Ashwin Chaugule, Mark Rutland
Hi Prashanth,
On 26/04/16 16:51, Prakash, Prashanth wrote:
> Hi Sudeep,
>
> On 4/19/2016 6:30 AM, Sudeep Holla wrote:
>> +struct acpi_processor_lpi *lpi;
>> +int acpi_processor_ffh_lpi_enter(struct acpi_processor_lpi *lpi, int idx)
>> +{
>> + int ret;
>> +
>> + if (!idx) {
>> + cpu_do_idle();
>> + return idx;
>> + }
>> +
>> + /* TODO cpu_pm_{enter,exit} can be done in generic code ? */
>> + ret = cpu_pm_enter();
> Can we avoid calling cpu_pm_enter and cpu_pm_exit for retention
> states as it is not necessary? May be we can check LPI architecture
> specific context loss flags prior to calling these.
Ah right, you had mentioned this before. Sorry for missing that.
Anyways, we need to get the driver reviewed before we can finalize arch
specific callbacks, so I will include this change when I post next version.
>> + }
>> +
>> + return ret ? -1 : idx;
>> +}
>> +
>
> By the way, thanks for posting these patches!
>
Thanks for taking look at these patches again :)
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/5] ACPI / processor_idle: introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE
2016-04-19 12:30 ` [PATCH v4 1/5] ACPI / processor_idle: introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE Sudeep Holla
2016-04-19 12:49 ` kbuild test robot
2016-04-20 9:56 ` Vikas Sajjan
@ 2016-05-10 0:02 ` Rafael J. Wysocki
2016-05-11 15:07 ` Sudeep Holla
2 siblings, 1 reply; 27+ messages in thread
From: Rafael J. Wysocki @ 2016-05-10 0:02 UTC (permalink / raw)
To: Sudeep Holla
Cc: linux-acpi, linux-kernel, linux-arm-kernel, Lorenzo Pieralisi,
Al Stone, Prashanth Prakash, Ashwin Chaugule, x86, linux-ia64
On Tuesday, April 19, 2016 01:30:09 PM Sudeep Holla 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: x86@kernel.org
> Cc: linux-ia64@vger.kernel.org
> 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 | 74 +++++++++++++++++++++++++++++--------------
> include/acpi/processor.h | 3 +-
> 5 files changed, 57 insertions(+), 25 deletions(-)
>
> diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
> index b534ebab36ea..717de2a146e2 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 2dc18605831f..eb03fd0d63b9 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -37,6 +37,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 82b96ee8624c..ec289078667c 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
It might be better to do
config ACPI_PROCESSOR_CST
bool
depends on ia64 || x86
Should it depend on ARCH_HAS_POWER_INIT too?
> +
> config ACPI_GENERIC_GSI
> bool
>
> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
> index 444e3745c8b3..1f3fe54194b5 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,49 @@ 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++;
> +
> + if (acpi_gbl_FADT.cst_control && !nocst) {
> + status = acpi_os_write_port(acpi_gbl_FADT.smi_command,
> + acpi_gbl_FADT.cst_control, 8);
> + if (ACPI_FAILURE(status))
> + ACPI_EXCEPTION((AE_INFO, status,
> + "Notifying BIOS of _CST ability failed"));
> + }
> +}
> +#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
Add a comment indicating what #ifdef block ends here?
> +
> int acpi_processor_hotplug(struct acpi_processor *pr)
> {
> int ret = 0;
> @@ -1018,29 +1062,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++;
> - }
> -
> - if (acpi_gbl_FADT.cst_control && !nocst) {
> - status =
> - acpi_os_write_port(acpi_gbl_FADT.smi_command, acpi_gbl_FADT.cst_control, 8);
> - if (ACPI_FAILURE(status)) {
> - ACPI_EXCEPTION((AE_INFO, status,
> - "Notifying BIOS of _CST ability failed"));
> - }
> - }
> + acpi_processor_cstate_first_run_checks();
>
> acpi_processor_get_power_info(pr);
> pr->flags.power_setup_done = 1;
> diff --git a/include/acpi/processor.h b/include/acpi/processor.h
> index 6f1805dd5d3c..50f2423d31fa 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,
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 2/5] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
2016-04-19 12:30 ` [PATCH v4 2/5] ACPI / processor_idle: Add support for Low Power Idle(LPI) states Sudeep Holla
@ 2016-05-10 0:04 ` Rafael J. Wysocki
2016-05-11 0:03 ` Rafael J. Wysocki
1 sibling, 0 replies; 27+ messages in thread
From: Rafael J. Wysocki @ 2016-05-10 0:04 UTC (permalink / raw)
To: Sudeep Holla
Cc: linux-acpi, linux-kernel, linux-arm-kernel, Lorenzo Pieralisi,
Al Stone, Prashanth Prakash, Ashwin Chaugule
On Tuesday, April 19, 2016 01:30:10 PM Sudeep Holla 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/bus.c | 11 +-
> drivers/acpi/processor_driver.c | 2 +-
> drivers/acpi/processor_idle.c | 441 +++++++++++++++++++++++++++++++++++-----
> include/acpi/processor.h | 25 ++-
> include/linux/acpi.h | 4 +
> 5 files changed, 422 insertions(+), 61 deletions(-)
>
> Hi Rafael,
>
> Yet to be discussed(retained as in from previous version):
I somehow didn't realize that you had been waiting for my feedback. Sorry
about that.
> - Kconfig entry removal: Need feedback on how to deal with that
> without having to introduce dummy _CST related ARM64 callbacks
> - Didn't defer processing of LPI buffers to flattening as it
> results in the same buffer decoded multiple times
> - ACPI_CSTATE_INTEGER : IMO it's reasonable to keep it aroundsince the
> it's part of LPI specification(not just ARM FFH)
OK, I'll have a look tomorrow.
Thanks,
Rafael
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 2/5] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
2016-04-19 12:30 ` [PATCH v4 2/5] ACPI / processor_idle: Add support for Low Power Idle(LPI) states Sudeep Holla
2016-05-10 0:04 ` Rafael J. Wysocki
@ 2016-05-11 0:03 ` Rafael J. Wysocki
2016-05-11 15:06 ` Sudeep Holla
1 sibling, 1 reply; 27+ messages in thread
From: Rafael J. Wysocki @ 2016-05-11 0:03 UTC (permalink / raw)
To: Sudeep Holla
Cc: linux-acpi, linux-kernel, linux-arm-kernel, Lorenzo Pieralisi,
Al Stone, Prashanth Prakash, Ashwin Chaugule
On Tuesday, April 19, 2016 01:30:10 PM Sudeep Holla 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/bus.c | 11 +-
> drivers/acpi/processor_driver.c | 2 +-
> drivers/acpi/processor_idle.c | 441 +++++++++++++++++++++++++++++++++++-----
> include/acpi/processor.h | 25 ++-
> include/linux/acpi.h | 4 +
> 5 files changed, 422 insertions(+), 61 deletions(-)
>
> Hi Rafael,
>
> Yet to be discussed(retained as in from previous version):
> - Kconfig entry removal: Need feedback on how to deal with that
> without having to introduce dummy _CST related ARM64 callbacks
> - Didn't defer processing of LPI buffers to flattening as it
> results in the same buffer decoded multiple times
> - ACPI_CSTATE_INTEGER : IMO it's reasonable to keep it aroundsince the
> it's part of LPI specification(not just ARM FFH)
I'm basically fine with the current set, up to some minor points.
I've sent my comments on patch [1/5] already.
My main concern about the flattening of _LPI is that at one point we'll
probably decide to unflatten it and that will change the behavior for
current users. There needs to be a plan for that IMO.
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index c068c829b453..94fe6e0ccfd3 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -302,6 +302,11 @@ acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context)
> EXPORT_SYMBOL(acpi_run_osc);
>
> bool osc_sb_apei_support_acked;
> +/*
> + * ACPI 6.0 Section 8.4.4.2 Idle State Coordination
> + * OSPM supports platform coordinated low power idle(LPI) states
> + */
> +bool osc_pc_lpi_support_confirmed;
> static u8 sb_uuid_str[] = "0811B06E-4A27-44F9-8D60-3CBBC22E7B48";
> static void acpi_bus_osc_support(void)
> {
> @@ -322,6 +327,7 @@ 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;
> + capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_PCLPI_SUPPORT;
>
> if (!ghes_disable)
> capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_APEI_SUPPORT;
> @@ -329,9 +335,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_confirmed =
> + 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 d2fa8cb82d2b..0ca14ac7bb28 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 1f3fe54194b5..5c78d68c7311 100644
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -303,7 +303,6 @@ static int acpi_processor_get_power_info_cst(struct acpi_processor *pr)
> struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> union acpi_object *cst;
>
> -
> if (nocst)
> return -ENODEV;
>
> @@ -576,7 +575,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 +809,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 +837,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;
>
> @@ -893,7 +855,7 @@ static int acpi_processor_setup_cpuidle_states(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;
> @@ -929,6 +891,7 @@ static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr)
> static inline void acpi_processor_cstate_first_run_checks(void)
> {
> static int first_run;
> + acpi_status status;
>
> if (first_run)
> return;
> @@ -951,7 +914,7 @@ 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;
> }
> @@ -962,13 +925,386 @@ static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr,
> 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
>
> +struct acpi_processor_lpi_info {
> + int state_count;
> + struct acpi_processor_lpi *lpix;
> +};
This is a bit cryptic, especially the name of the lpix field.
I'd do something like
struct acpi_lpi_states_array {
unsigned int size;
struct acpi_lpi_state *entries;
};
and that is sort of self-documenting.
> +
> +static int obj_get_integer(union acpi_object *obj, u32 *value)
> +{
> + if (obj->type != ACPI_TYPE_INTEGER)
> + return -EINVAL;
I'd add an empty line here and everywhere where there's only one
statement after if () or for () etc.
You've done that in some places IIRC, but please stick to one convention
everywhere.
> + *value = obj->integer.value;
> + return 0;
> +}
> +
> +static int acpi_processor_evaluate_lpi(acpi_handle handle,
> + struct acpi_processor_lpi_info *info)
> +{
> + acpi_status status = 0;
> + int ret = 0;
> + int pkg_count, state_idx = 1, loop;
> + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> + union acpi_object *lpi;
I'd call this lpi_data.
> + struct acpi_processor_lpi *lpix;
What about:
struct acpi_lpi_state *lpi_state;
> +
> + 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_debug("not enough elements in _LPI\n");
> + ret = -ENXIO;
-ENODATA ?
> + goto end;
> + }
> +
> + pkg_count = lpi->package.elements[2].integer.value;
> +
> + /* Validate number of power states. */
> + if (pkg_count < 1 || pkg_count != lpi->package.count - 3) {
> + pr_debug("count given by _LPI is not valid\n");
> + ret = -ENXIO;
And here.
> + goto end;
> + }
> +
> + lpix = kcalloc(pkg_count, sizeof(*lpix), GFP_KERNEL);
> + if (!lpix) {
> + ret = -ENOMEM;
> + goto end;
> + }
> +
> + info->state_count = pkg_count;
> + info->lpix = lpix;
> + /* LPI States start at index 3 */
> + for (loop = 3; state_idx <= pkg_count; loop++, state_idx++, lpix++) {
> + union acpi_object *element, *pkg_elem, *obj;
> +
> + element = &lpi->package.elements[loop];
> + if (element->type != ACPI_TYPE_PACKAGE)
> + continue;
> +
> + if (element->package.count < 7)
> + continue;
> +
> + pkg_elem = element->package.elements;
> +
> + obj = pkg_elem + 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->entry_method = ACPI_CSTATE_INTEGER;
> + lpix->address = obj->integer.value;
> + } else {
> + continue;
> + }
> +
> + /* elements[7,8] skipped for now i.e. Residency/Usage counter*/
> +
> + obj = pkg_elem + 9;
> + if (obj->type == ACPI_TYPE_STRING)
> + strlcpy(lpix->desc, obj->string.pointer,
> + ACPI_CX_DESC_LEN);
> +
> + lpix->index = state_idx;
> + if (obj_get_integer(pkg_elem + 0, &lpix->min_residency)) {
> + pr_debug("No min. residency found, assuming 10 us\n");
> + lpix->min_residency = 10;
> + }
> +
> + if (obj_get_integer(pkg_elem + 1, &lpix->wake_latency)) {
> + pr_debug("No wakeup residency found, assuming 10 us\n");
> + lpix->wake_latency = 10;
> + }
> +
> + if (obj_get_integer(pkg_elem + 2, &lpix->flags))
> + lpix->flags = 0;
> +
> + if (obj_get_integer(pkg_elem + 3, &lpix->arch_flags))
> + lpix->arch_flags = 0;
> +
> + if (obj_get_integer(pkg_elem + 4, &lpix->res_cnt_freq))
> + lpix->res_cnt_freq = 1;
> +
> + if (obj_get_integer(pkg_elem + 5, &lpix->enable_parent_state))
> + lpix->enable_parent_state = 0;
> + }
> + ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Found %d power states\n",
> + state_idx));
> +end:
> + kfree(buffer.pointer);
> + return ret;
> +}
> +
> +static int max_leaf_depth, fl_scnt;
Add a comment describing these things?
> +/**
> + * combine_lpi_states - combine local and parent LPI states to form a
> + * composite LPI state
One line, please.
> + * @l_lpi: local LPI state
> + * @p_lpi: parent LPI state
> + * @c_lpi: composite LPI state
I'd call these local, parent and result, respectively.
> + */
> +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;
> +
> + if (p_lpi->entry_method == ACPI_CSTATE_INTEGER)
> + c_lpi->address = l_lpi->address + p_lpi->address;
> + else
> + c_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;
> +
> + 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);
> +}
> +
> +#define ACPI_LPI_STATE_FLAGS_ENABLED BIT(0)
> +
> +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;
state_count ?
> + struct acpi_processor_lpi *t = info[depth].lpix;
struct acpi_lpi_state *lpi_state = states[depth].entries;
> +
> + for (j = 0; j < scount; j++, t++) {
> + struct acpi_processor_lpi *flpi;
> + bool valid = false;
> +
> + if (!(t->flags & ACPI_LPI_STATE_FLAGS_ENABLED))
> + continue;
> +
> + flpi = &pr->power.lpi_states[fl_scnt];
> + 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;
> + }
> +
> + /*
> + * flatten recursively from leaf until the highest level
> + * (e.g. system) is reached
> + */
> + 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, pr_ahandle;
> + acpi_status status;
> +
> + if (!osc_pc_lpi_support_confirmed)
> + 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, &pr_ahandle))) {
> + 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 = pr_ahandle;
> + }
> +
> + info = kcalloc(max_leaf_depth + 1, sizeof(*info), GFP_KERNEL);
> + if (!info)
> + return -ENOMEM;
> +
> + pr_ahandle = pr->handle;
> + for (i = max_leaf_depth; i >= 0 && ACPI_SUCCESS(status); i--) {
> + handle = pr_ahandle;
> + ret = acpi_processor_evaluate_lpi(handle, info + i);
> + if (ret)
> + break;
> + status = acpi_get_parent(handle, &pr_ahandle);
> + }
> +
> + /* flatten all the LPI states in the entire hierarchy */
> + 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;
> +}
> +
> +int __weak acpi_processor_ffh_lpi_probe(unsigned int cpu)
> +{
> + return -ENODEV;
> +}
> +
> +int __weak acpi_processor_ffh_lpi_enter(struct acpi_processor_lpi *lpi, int idx)
> +{
> + return -ENODEV;
> +}
> +
> +/**
> + * 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
> + *
> + * Return: 0 for success or negative value for error
> + */
> +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 */
The comment is not necessary here.
> + 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;
> +
> + if (!pr->flags.has_lpi)
> + return -EOPNOTSUPP;
> +
> + 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);
> + strlcpy(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;
No ->enter_freeze ?
> + drv->safe_state_index = i;
> + }
> +
> + drv->state_count = i;
> +
> + 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)
> +{
> + 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);
> + return acpi_processor_setup_cpuidle_cx(pr, dev);
> +}
> +
> +static int acpi_processor_get_power_info(struct acpi_processor *pr)
> +{
> + if (acpi_processor_get_lpi_info(pr))
> + return acpi_processor_get_cstate_info(pr);
> + return 0;
ret = acpi_processor_get_lpi_info(pr);
if (ret)
ret = acpi_processor_get_cstate_info(pr);
return ret;
> +}
> +
> int acpi_processor_hotplug(struct acpi_processor *pr)
> {
> int ret = 0;
> @@ -977,18 +1313,15 @@ int acpi_processor_hotplug(struct acpi_processor *pr)
> if (disabled_by_idle_boot_param())
> return 0;
>
> - if (nocst)
> - return -ENODEV;
> -
> if (!pr->flags.power_setup_done)
> return -ENODEV;
>
> dev = per_cpu(acpi_cpuidle_device, pr->id);
> cpuidle_pause_and_lock();
> cpuidle_disable_device(dev);
> - acpi_processor_get_power_info(pr);
> - if (pr->flags.power) {
> - acpi_processor_setup_cpuidle_cx(pr, dev);
> + ret = acpi_processor_get_power_info(pr);
> + if (!ret && pr->flags.power) {
> + acpi_processor_setup_cpuidle_dev(pr, dev);
> ret = cpuidle_enable_device(dev);
> }
> cpuidle_resume_and_unlock();
> @@ -996,7 +1329,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;
> @@ -1005,9 +1338,6 @@ int acpi_processor_cst_has_changed(struct acpi_processor *pr)
> if (disabled_by_idle_boot_param())
> return 0;
>
> - if (nocst)
> - return -ENODEV;
> -
> if (!pr->flags.power_setup_done)
> return -ENODEV;
>
> @@ -1044,7 +1374,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);
> }
> }
> @@ -1059,7 +1389,6 @@ static int acpi_processor_registered;
>
> int acpi_processor_power_init(struct acpi_processor *pr)
> {
> - acpi_status status;
> int retval;
> struct cpuidle_device *dev;
>
> @@ -1092,7 +1421,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 50f2423d31fa..7dd1e63199ed 100644
> --- a/include/acpi/processor.h
> +++ b/include/acpi/processor.h
> @@ -39,6 +39,7 @@
> #define ACPI_CSTATE_SYSTEMIO 0
> #define ACPI_CSTATE_FFH 1
> #define ACPI_CSTATE_HALT 2
> +#define ACPI_CSTATE_INTEGER 3
>
> #define ACPI_CX_DESC_LEN 32
>
> @@ -67,9 +68,25 @@ struct acpi_processor_cx {
> char desc[ACPI_CX_DESC_LEN];
> };
>
> +struct acpi_processor_lpi {
As I said above, I'd call this
struct acpi_lpi_state {
because (a) it represents a state and (b) that doesn't have to be a state
of a processor.
> + 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 +206,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;
> @@ -372,7 +390,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)
> @@ -385,7 +403,8 @@ static inline int acpi_processor_power_exit(struct acpi_processor *pr)
> return -ENODEV;
> }
>
> -static inline int acpi_processor_cst_has_changed(struct acpi_processor *pr)
> +static inline int
> +acpi_processor_power_state_has_changed(struct acpi_processor *pr)
Please don't break lines like this. It doesn't hurt if it is longer that 80 chars.
> {
> return -ENODEV;
> }
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 06ed7e54033e..127fdc04e2b5 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -452,8 +452,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_confirmed;
>
> /* PCI Host Bridge _OSC: Capabilities DWORD 2: Support Field */
> #define OSC_PCI_EXT_CONFIG_SUPPORT 0x00000001
>
Thanks,
Rafael
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 4/5] arm64: add support for ACPI Low Power Idle(LPI)
2016-04-19 12:30 ` [PATCH v4 4/5] arm64: add support for ACPI Low Power Idle(LPI) Sudeep Holla
` (3 preceding siblings ...)
2016-04-26 15:51 ` Prakash, Prashanth
@ 2016-05-11 0:07 ` Rafael J. Wysocki
2016-05-11 15:06 ` Sudeep Holla
4 siblings, 1 reply; 27+ messages in thread
From: Rafael J. Wysocki @ 2016-05-11 0:07 UTC (permalink / raw)
To: Sudeep Holla
Cc: linux-acpi, linux-kernel, linux-arm-kernel, Lorenzo Pieralisi,
Al Stone, Prashanth Prakash, Ashwin Chaugule, Mark Rutland
On Tuesday, April 19, 2016 01:30:12 PM Sudeep Holla wrote:
> This patch adds appropriate callbacks to support ACPI Low Power Idle
> (LPI) on ARM64.
>
> It also selects ARCH_SUPPORTS_ACPI_PROCESSOR_LPI if ACPI is enabled
> on ARM64.
Can you remind me why we need the ARCH_SUPPORTS_ACPI_PROCESSOR_LPI thing in
addition to the previously defined ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE?
I thought supporting LPI would not require an arch Kconfig option.
Thanks,
Rafael
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 2/5] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
2016-05-11 0:03 ` Rafael J. Wysocki
@ 2016-05-11 15:06 ` Sudeep Holla
2016-05-11 20:45 ` Rafael J. Wysocki
0 siblings, 1 reply; 27+ messages in thread
From: Sudeep Holla @ 2016-05-11 15:06 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Sudeep Holla, linux-acpi, linux-kernel, linux-arm-kernel,
Lorenzo Pieralisi, Al Stone, Prashanth Prakash, Ashwin Chaugule
On 11/05/16 01:03, Rafael J. Wysocki wrote:
> On Tuesday, April 19, 2016 01:30:10 PM Sudeep Holla 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/bus.c | 11 +-
>> drivers/acpi/processor_driver.c | 2 +-
>> drivers/acpi/processor_idle.c | 441 +++++++++++++++++++++++++++++++++++-----
>> include/acpi/processor.h | 25 ++-
>> include/linux/acpi.h | 4 +
>> 5 files changed, 422 insertions(+), 61 deletions(-)
>>
>> Hi Rafael,
>>
>> Yet to be discussed(retained as in from previous version):
>> - Kconfig entry removal: Need feedback on how to deal with that
>> without having to introduce dummy _CST related ARM64 callbacks
>> - Didn't defer processing of LPI buffers to flattening as it
>> results in the same buffer decoded multiple times
>> - ACPI_CSTATE_INTEGER : IMO it's reasonable to keep it aroundsince the
>> it's part of LPI specification(not just ARM FFH)
>
> I'm basically fine with the current set, up to some minor points.
>
> I've sent my comments on patch [1/5] already.
>
> My main concern about the flattening of _LPI is that at one point we'll
> probably decide to unflatten it and that will change the behavior for
> current users. There needs to be a plan for that IMO.
>
Are you referring the OS co-ordinated mode ? If yes, I agree. If not,
can you explain why would we not flatten the LPI states ?
[...]
>>
>> +struct acpi_processor_lpi_info {
>> + int state_count;
>> + struct acpi_processor_lpi *lpix;
>> +};
>
> This is a bit cryptic, especially the name of the lpix field.
>
> I'd do something like
>
> struct acpi_lpi_states_array {
> unsigned int size;
> struct acpi_lpi_state *entries;
> };
>
> and that is sort of self-documenting.
>
Agreed and that's looks much better.
>> +
>> +static int obj_get_integer(union acpi_object *obj, u32 *value)
>> +{
>> + if (obj->type != ACPI_TYPE_INTEGER)
>> + return -EINVAL;
>
> I'd add an empty line here and everywhere where there's only one
> statement after if () or for () etc.
>
> You've done that in some places IIRC, but please stick to one convention
> everywhere.
>
I have taken all the review comments and fixed them as suggested.
[...]
>> +
>> + 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);
>> + strlcpy(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;
>
> No ->enter_freeze ?
>
I don't have a system to test this. Also IIUC the cpuidle does support
suspend-to-idle even when ->enter_freeze is not implemented right.
Can we add it later once I find a way to test. Correctly no wakeup on my
test platform :(
>>
>> +struct acpi_processor_lpi {
>
> As I said above, I'd call this
>
> struct acpi_lpi_state {
>
> because (a) it represents a state and (b) that doesn't have to be a state
> of a processor.
>
Agreed, that's mainly copy paste from _CST which calls it
acpi_processor_cx :)
--
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 4/5] arm64: add support for ACPI Low Power Idle(LPI)
2016-05-11 0:07 ` Rafael J. Wysocki
@ 2016-05-11 15:06 ` Sudeep Holla
0 siblings, 0 replies; 27+ messages in thread
From: Sudeep Holla @ 2016-05-11 15:06 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Sudeep Holla, linux-acpi, linux-kernel, linux-arm-kernel,
Lorenzo Pieralisi, Al Stone, Prashanth Prakash, Ashwin Chaugule,
Mark Rutland
On 11/05/16 01:07, Rafael J. Wysocki wrote:
> On Tuesday, April 19, 2016 01:30:12 PM Sudeep Holla wrote:
>> This patch adds appropriate callbacks to support ACPI Low Power Idle
>> (LPI) on ARM64.
>>
>> It also selects ARCH_SUPPORTS_ACPI_PROCESSOR_LPI if ACPI is enabled
>> on ARM64.
>
> Can you remind me why we need the ARCH_SUPPORTS_ACPI_PROCESSOR_LPI thing in
> addition to the previously defined ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE?
>
That's stupid of me, I removed it from the driver in v4 but the few
remnants of it stayed by mistake(mainly in this patch alone)
> I thought supporting LPI would not require an arch Kconfig option.
>
That's correct.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/5] ACPI / processor_idle: introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE
2016-05-10 0:02 ` Rafael J. Wysocki
@ 2016-05-11 15:07 ` Sudeep Holla
0 siblings, 0 replies; 27+ messages in thread
From: Sudeep Holla @ 2016-05-11 15:07 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Sudeep Holla, linux-acpi, linux-kernel, linux-arm-kernel,
Lorenzo Pieralisi, Al Stone, Prashanth Prakash, Ashwin Chaugule,
x86, linux-ia64
On 10/05/16 01:02, Rafael J. Wysocki wrote:
> On Tuesday, April 19, 2016 01:30:09 PM Sudeep Holla 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: x86@kernel.org
>> Cc: linux-ia64@vger.kernel.org
>> 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 | 74 +++++++++++++++++++++++++++++--------------
>> include/acpi/processor.h | 3 +-
>> 5 files changed, 57 insertions(+), 25 deletions(-)
>>
>> diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
>> index b534ebab36ea..717de2a146e2 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 2dc18605831f..eb03fd0d63b9 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -37,6 +37,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 82b96ee8624c..ec289078667c 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
>
> It might be better to do
>
> config ACPI_PROCESSOR_CST
> bool
> depends on ia64 || x86
>
Done
> Should it depend on ARCH_HAS_POWER_INIT too?
>
IIUC it's not defined for ia64, so better not to add that dependency
here IMO. include/acpi/processor.h encapsulates it. Let me know if I
have misunderstood it.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 2/5] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
2016-05-11 15:06 ` Sudeep Holla
@ 2016-05-11 20:45 ` Rafael J. Wysocki
0 siblings, 0 replies; 27+ messages in thread
From: Rafael J. Wysocki @ 2016-05-11 20:45 UTC (permalink / raw)
To: Sudeep Holla
Cc: Rafael J. Wysocki, ACPI Devel Maling List,
Linux Kernel Mailing List, linux-arm-kernel, Lorenzo Pieralisi,
Al Stone, Prashanth Prakash, Ashwin Chaugule
On Wed, May 11, 2016 at 5:06 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
>
> On 11/05/16 01:03, Rafael J. Wysocki wrote:
>>
>> On Tuesday, April 19, 2016 01:30:10 PM Sudeep Holla wrote:
>>>
[cut]
>>
>> My main concern about the flattening of _LPI is that at one point we'll
>> probably decide to unflatten it and that will change the behavior for
>> current users. There needs to be a plan for that IMO.
>>
>
> Are you referring the OS co-ordinated mode ? If yes, I agree. If not,
> can you explain why would we not flatten the LPI states ?
If the platform doesn't autopromote core-level states into
package-level states, then having access to unflattened hierarchy may
be useful. In some cases the exit latency of core states may be
acceptable, while the exit latency of package states may not be, for
example.
Also the scheduler may want to know which cores are in one package
(from the idle states perspective) at one point.
[cut]
>>> +
>>> + 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);
>>> + strlcpy(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;
>>
>>
>> No ->enter_freeze ?
>>
>
> I don't have a system to test this. Also IIUC the cpuidle does support
> suspend-to-idle even when ->enter_freeze is not implemented right.
> Can we add it later once I find a way to test. Correctly no wakeup on my
> test platform :(
Fair enough.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 3/5] drivers: psci: refactor psci_cpu_init_idle in preparation for ACPI LPI support
2016-04-19 12:30 ` [PATCH v4 3/5] drivers: psci: refactor psci_cpu_init_idle in preparation for ACPI LPI support Sudeep Holla
@ 2016-06-09 13:24 ` Lorenzo Pieralisi
2016-06-09 14:26 ` Sudeep Holla
0 siblings, 1 reply; 27+ messages in thread
From: Lorenzo Pieralisi @ 2016-06-09 13:24 UTC (permalink / raw)
To: Sudeep Holla
Cc: Rafael J. Wysocki, linux-acpi, linux-kernel, linux-arm-kernel,
Al Stone, Prashanth Prakash, Ashwin Chaugule, Mark Rutland
On Tue, Apr 19, 2016 at 01:30:11PM +0100, Sudeep Holla wrote:
> Inorder to accomodate bot DT and ACPI LPI support in psci_cpu_init_idle,
> move the device tree specific into psci_dt_cpu_init_idle.
>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> drivers/firmware/psci.c | 23 +++++++++--------------
> 1 file changed, 9 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
> index 11bfee8b79a9..af6c5c839568 100644
> --- a/drivers/firmware/psci.c
> +++ b/drivers/firmware/psci.c
> @@ -250,11 +250,11 @@ static int __init psci_features(u32 psci_func_id)
> #ifdef CONFIG_CPU_IDLE
> static DEFINE_PER_CPU_READ_MOSTLY(u32 *, psci_power_state);
>
> -static int psci_dt_cpu_init_idle(struct device_node *cpu_node, int cpu)
> +static int psci_dt_cpu_init_idle(unsigned int cpu)
Unfortunately you would break ARM 32-bit if you did that.
> {
> int i, ret, count = 0;
> u32 *psci_states;
> - struct device_node *state_node;
> + struct device_node *state_node, *cpu_node;
>
> /*
> * If the PSCI cpu_suspend function hook has not been initialized
> @@ -263,6 +263,10 @@ static int psci_dt_cpu_init_idle(struct device_node *cpu_node, int cpu)
> if (!psci_ops.cpu_suspend)
> return -EOPNOTSUPP;
>
> + cpu_node = of_get_cpu_node(cpu, NULL);
> + if (!cpu_node)
> + return -ENODEV;
> +
> /* Count idle states */
> while ((state_node = of_parse_phandle(cpu_node, "cpu-idle-states",
> count))) {
> @@ -303,27 +307,18 @@ static int psci_dt_cpu_init_idle(struct device_node *cpu_node, int cpu)
> }
> /* Idle states parsed correctly, initialize per-cpu pointer */
> per_cpu(psci_power_state, cpu) = psci_states;
> + of_node_put(cpu_node);
> return 0;
>
> free_mem:
> + of_node_put(cpu_node);
> kfree(psci_states);
> return ret;
> }
>
> int psci_cpu_init_idle(unsigned int cpu)
> {
> - struct device_node *cpu_node;
> - int ret;
> -
> - cpu_node = of_get_cpu_node(cpu, NULL);
> - if (!cpu_node)
> - return -ENODEV;
> -
> - ret = psci_dt_cpu_init_idle(cpu_node, cpu);
> -
> - of_node_put(cpu_node);
> -
> - return ret;
> + return psci_dt_cpu_init_idle(cpu);
How about leaving code as is and you wrap the cpu_node retrieval:
if (!acpi_disabled) {
acpi_idle_init();
} else {
cpu_node = of_get_cpu_node(cpu, NULL);
if (!cpu_node)
return -ENODEV;
ret = psci_dt_cpu_init_idle(cpu_node, cpu);
of_node_put(cpu_node);
}
?
Alternatively, you could create an intermediate stub
__psci_dt_cpu_init_idle(), that will be used for CONFIG_ARM
cpuidle_ops.init and psci_dt_cpu_init_idle() after retrieving
the cpu_node, which I think is slightly cleaner.
Lorenzo
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 3/5] drivers: psci: refactor psci_cpu_init_idle in preparation for ACPI LPI support
2016-06-09 13:24 ` Lorenzo Pieralisi
@ 2016-06-09 14:26 ` Sudeep Holla
0 siblings, 0 replies; 27+ messages in thread
From: Sudeep Holla @ 2016-06-09 14:26 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Sudeep Holla, Rafael J. Wysocki, linux-acpi, linux-kernel,
linux-arm-kernel, Al Stone, Prashanth Prakash, Ashwin Chaugule,
Mark Rutland
On 09/06/16 14:24, Lorenzo Pieralisi wrote:
> On Tue, Apr 19, 2016 at 01:30:11PM +0100, Sudeep Holla wrote:
>> Inorder to accomodate bot DT and ACPI LPI support in psci_cpu_init_idle,
>> move the device tree specific into psci_dt_cpu_init_idle.
>>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> Cc: linux-arm-kernel@lists.infradead.org
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>> ---
>> drivers/firmware/psci.c | 23 +++++++++--------------
>> 1 file changed, 9 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
>> index 11bfee8b79a9..af6c5c839568 100644
>> --- a/drivers/firmware/psci.c
>> +++ b/drivers/firmware/psci.c
>> @@ -250,11 +250,11 @@ static int __init psci_features(u32 psci_func_id)
>> #ifdef CONFIG_CPU_IDLE
>> static DEFINE_PER_CPU_READ_MOSTLY(u32 *, psci_power_state);
>>
>> -static int psci_dt_cpu_init_idle(struct device_node *cpu_node, int cpu)
>> +static int psci_dt_cpu_init_idle(unsigned int cpu)
>
> Unfortunately you would break ARM 32-bit if you did that.
>
Ah right, I failed to catch this. Thanks for spotting this.
[...]
>> int psci_cpu_init_idle(unsigned int cpu)
>> {
>> - struct device_node *cpu_node;
>> - int ret;
>> -
>> - cpu_node = of_get_cpu_node(cpu, NULL);
>> - if (!cpu_node)
>> - return -ENODEV;
>> -
>> - ret = psci_dt_cpu_init_idle(cpu_node, cpu);
>> -
>> - of_node_put(cpu_node);
>> -
>> - return ret;
>> + return psci_dt_cpu_init_idle(cpu);
>
> How about leaving code as is and you wrap the cpu_node retrieval:
>
> if (!acpi_disabled) {
> acpi_idle_init();
> } else {
> cpu_node = of_get_cpu_node(cpu, NULL);
> if (!cpu_node)
> return -ENODEV;
>
> ret = psci_dt_cpu_init_idle(cpu_node, cpu);
>
> of_node_put(cpu_node);
> }
>
> ?
>
> Alternatively, you could create an intermediate stub
> __psci_dt_cpu_init_idle(), that will be used for CONFIG_ARM
> cpuidle_ops.init and psci_dt_cpu_init_idle() after retrieving
> the cpu_node, which I think is slightly cleaner.
>
I like this approach more.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2016-06-09 14:26 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-19 12:30 [PATCH v4 0/5] ACPI / processor_idle: Add ACPI v6.0 LPI support Sudeep Holla
2016-04-19 12:30 ` [PATCH v4 1/5] ACPI / processor_idle: introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE Sudeep Holla
2016-04-19 12:49 ` kbuild test robot
2016-04-19 13:00 ` Sudeep Holla
2016-04-20 9:56 ` Vikas Sajjan
2016-04-20 10:09 ` Sudeep Holla
2016-05-10 0:02 ` Rafael J. Wysocki
2016-05-11 15:07 ` Sudeep Holla
2016-04-19 12:30 ` [PATCH v4 2/5] ACPI / processor_idle: Add support for Low Power Idle(LPI) states Sudeep Holla
2016-05-10 0:04 ` Rafael J. Wysocki
2016-05-11 0:03 ` Rafael J. Wysocki
2016-05-11 15:06 ` Sudeep Holla
2016-05-11 20:45 ` Rafael J. Wysocki
2016-04-19 12:30 ` [PATCH v4 3/5] drivers: psci: refactor psci_cpu_init_idle in preparation for ACPI LPI support Sudeep Holla
2016-06-09 13:24 ` Lorenzo Pieralisi
2016-06-09 14:26 ` Sudeep Holla
2016-04-19 12:30 ` [PATCH v4 4/5] arm64: add support for ACPI Low Power Idle(LPI) Sudeep Holla
2016-04-19 13:59 ` kbuild test robot
2016-04-19 15:42 ` Sudeep Holla
2016-04-20 9:59 ` Vikas Sajjan
2016-04-20 10:20 ` Sudeep Holla
2016-04-20 10:39 ` Jisheng Zhang
2016-04-26 15:51 ` Prakash, Prashanth
2016-04-26 16:01 ` Sudeep Holla
2016-05-11 0:07 ` Rafael J. Wysocki
2016-05-11 15:06 ` Sudeep Holla
2016-04-19 12:30 ` [PATCH v4 5/5] ACPI : enable ACPI_PROCESSOR_IDLE on ARM64 Sudeep Holla
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).