* [PATCH v7 0/6] ACPI / processor_idle: Add ACPI v6.0 LPI support
@ 2016-06-28 13:55 ` Sudeep Holla
0 siblings, 0 replies; 37+ messages in thread
From: Sudeep Holla @ 2016-06-28 13:55 UTC (permalink / raw)
To: linux-arm-kernel
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.
v6[6]->v7:
- Removed cpuidle-arm.h and introduced HAVE_GENERIC_CPUIDLE_ENTER
and move the common code to cpuidle_generic_enter{,_state}
- Factored out common code between psci_{dt,acpi}_cpu_init_idle
v5[5]->v6:
- Added support for autopromotable state by not flattening them
- Moved arm_enter_idle_state to cpuidle-arm.h as it can be reused
in ARM64 backend for ACPI LPI
- Other review comments(mainly for ARM64 from Lorenzo)
- Dropped support for skipping PM notifier as it needs to be fixed
in GICv3 code(will be done separately)
v4[4]->v5:
- Addressed all the comments from Rafael
- Added support for retention mode(Prashant)
- Handled acpi_processor_get_power_info return value correctly(Vikas)
- Dropped __init from arm_cpuidle_init
- Merged psci prepartory patch into arm64 lpi support
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 @[7]
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] http://marc.info/?l=linux-acpi&m=146106902731359&w=2
[5] http://marc.info/?l=linux-acpi&m=146298107608349&w=2
[6] http://marc.info/?l=linux-acpi&m=146661754618838&w=2
[7] git://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git for_review/arm64_lpi
Sudeep Holla (6):
ACPI / processor_idle: introduce ACPI_PROCESSOR_CSTATE
ACPI / processor_idle: Add support for Low Power Idle(LPI) states
arm64: cpuidle: drop __init section marker to arm_cpuidle_init
cpuidle: introduce HAVE_GENERIC_CPUIDLE_ENTER for ARM{32,64} platforms
arm64: add support for ACPI Low Power Idle(LPI)
ACPI : enable ACPI_PROCESSOR_IDLE on ARM64
arch/arm/Kconfig | 1 +
arch/arm/kernel/cpuidle.c | 4 +-
arch/arm64/Kconfig | 1 +
arch/arm64/kernel/cpuidle.c | 24 +-
drivers/acpi/Kconfig | 6 +-
drivers/acpi/bus.c | 14 +-
drivers/acpi/processor_driver.c | 2 +-
drivers/acpi/processor_idle.c | 544 ++++++++++++++++++++++++++++++++++------
drivers/cpuidle/Kconfig | 3 +
drivers/cpuidle/cpuidle-arm.c | 21 +-
drivers/cpuidle/cpuidle.c | 35 +++
drivers/firmware/psci.c | 122 +++++++--
include/acpi/processor.h | 26 +-
include/linux/acpi.h | 4 +
include/linux/cpuidle.h | 8 +
15 files changed, 680 insertions(+), 135 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 37+ messages in thread* [PATCH v7 1/6] ACPI / processor_idle: introduce ACPI_PROCESSOR_CSTATE
2016-06-28 13:55 ` Sudeep Holla
@ 2016-06-28 13:55 ` Sudeep Holla
-1 siblings, 0 replies; 37+ messages in thread
From: Sudeep Holla @ 2016-06-28 13:55 UTC (permalink / raw)
To: linux-acpi, Rafael J . Wysocki
Cc: Sudeep Holla, Vikas Sajjan, Sunil, Lorenzo Pieralisi,
PrashanthPrakash, Al Stone, Ashwin Chaugule, Daniel Lezcano,
linux-kernel, linux-arm-kernel
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 ACPI_PROCESSOR_CSTATE to encapsulate all the
code supporting the old style C-states(_CST).
This patch will help to extend the processor_idle module to support
LPI.
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
drivers/acpi/Kconfig | 4 +++
drivers/acpi/processor_idle.c | 80 ++++++++++++++++++++++++++++---------------
include/acpi/processor.h | 2 +-
3 files changed, 58 insertions(+), 28 deletions(-)
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index b7e2e776397d..1358fb7d7a68 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -213,6 +213,10 @@ config ACPI_CPU_FREQ_PSS
bool
select THERMAL
+config ACPI_PROCESSOR_CSTATE
+ def_bool y
+ depends on IA64 || X86
+
config ACPI_PROCESSOR_IDLE
bool
select CPU_IDLE
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 444e3745c8b3..ca0de35d1c3a 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_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,50 @@ static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr)
return 0;
}
+static inline void acpi_processor_cstate_first_run_checks(void)
+{
+ acpi_status status;
+ 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 /* CONFIG_ACPI_PROCESSOR_CSTATE */
+
int acpi_processor_hotplug(struct acpi_processor *pr)
{
int ret = 0;
@@ -1015,35 +1060,16 @@ static int acpi_processor_registered;
int acpi_processor_power_init(struct acpi_processor *pr)
{
- acpi_status status;
int retval;
struct cpuidle_device *dev;
- static int first_run;
if (disabled_by_idle_boot_param())
return 0;
- if (!first_run) {
- dmi_check_system(processor_power_dmi_table);
- max_cstate = acpi_processor_cstate_check(max_cstate);
- if (max_cstate < ACPI_C_STATES_MAX)
- printk(KERN_NOTICE
- "ACPI: processor limited to max C-state %d\n",
- max_cstate);
- first_run++;
- }
+ acpi_processor_cstate_first_run_checks();
- if (acpi_gbl_FADT.cst_control && !nocst) {
- status =
- 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_get_power_info(pr);
- pr->flags.power_setup_done = 1;
+ if (!acpi_processor_get_power_info(pr))
+ pr->flags.power_setup_done = 1;
/*
* Install the idle handler if processor power management is supported.
diff --git a/include/acpi/processor.h b/include/acpi/processor.h
index 6f1805dd5d3c..48779d678122 100644
--- a/include/acpi/processor.h
+++ b/include/acpi/processor.h
@@ -242,7 +242,7 @@ 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_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,
--
2.7.4
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v7 1/6] ACPI / processor_idle: introduce ACPI_PROCESSOR_CSTATE
@ 2016-06-28 13:55 ` Sudeep Holla
0 siblings, 0 replies; 37+ messages in thread
From: Sudeep Holla @ 2016-06-28 13:55 UTC (permalink / raw)
To: linux-arm-kernel
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 ACPI_PROCESSOR_CSTATE to encapsulate all the
code supporting the old style C-states(_CST).
This patch will help to extend the processor_idle module to support
LPI.
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
drivers/acpi/Kconfig | 4 +++
drivers/acpi/processor_idle.c | 80 ++++++++++++++++++++++++++++---------------
include/acpi/processor.h | 2 +-
3 files changed, 58 insertions(+), 28 deletions(-)
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index b7e2e776397d..1358fb7d7a68 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -213,6 +213,10 @@ config ACPI_CPU_FREQ_PSS
bool
select THERMAL
+config ACPI_PROCESSOR_CSTATE
+ def_bool y
+ depends on IA64 || X86
+
config ACPI_PROCESSOR_IDLE
bool
select CPU_IDLE
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 444e3745c8b3..ca0de35d1c3a 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_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,50 @@ static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr)
return 0;
}
+static inline void acpi_processor_cstate_first_run_checks(void)
+{
+ acpi_status status;
+ 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 /* CONFIG_ACPI_PROCESSOR_CSTATE */
+
int acpi_processor_hotplug(struct acpi_processor *pr)
{
int ret = 0;
@@ -1015,35 +1060,16 @@ static int acpi_processor_registered;
int acpi_processor_power_init(struct acpi_processor *pr)
{
- acpi_status status;
int retval;
struct cpuidle_device *dev;
- static int first_run;
if (disabled_by_idle_boot_param())
return 0;
- if (!first_run) {
- dmi_check_system(processor_power_dmi_table);
- max_cstate = acpi_processor_cstate_check(max_cstate);
- if (max_cstate < ACPI_C_STATES_MAX)
- printk(KERN_NOTICE
- "ACPI: processor limited to max C-state %d\n",
- max_cstate);
- first_run++;
- }
+ acpi_processor_cstate_first_run_checks();
- if (acpi_gbl_FADT.cst_control && !nocst) {
- status =
- 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_get_power_info(pr);
- pr->flags.power_setup_done = 1;
+ if (!acpi_processor_get_power_info(pr))
+ pr->flags.power_setup_done = 1;
/*
* Install the idle handler if processor power management is supported.
diff --git a/include/acpi/processor.h b/include/acpi/processor.h
index 6f1805dd5d3c..48779d678122 100644
--- a/include/acpi/processor.h
+++ b/include/acpi/processor.h
@@ -242,7 +242,7 @@ 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_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,
--
2.7.4
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH v7 2/6] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
2016-06-28 13:55 ` Sudeep Holla
(?)
@ 2016-06-28 13:55 ` Sudeep Holla
-1 siblings, 0 replies; 37+ messages in thread
From: Sudeep Holla @ 2016-06-28 13:55 UTC (permalink / raw)
To: linux-acpi, Rafael J . Wysocki
Cc: Lorenzo Pieralisi, PrashanthPrakash, Al Stone, Vikas Sajjan,
Daniel Lezcano, linux-kernel, Ashwin Chaugule, Sudeep Holla,
linux-arm-kernel, Sunil
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 | 14 +-
drivers/acpi/processor_driver.c | 2 +-
drivers/acpi/processor_idle.c | 468 +++++++++++++++++++++++++++++++++++-----
include/acpi/processor.h | 24 ++-
include/linux/acpi.h | 4 +
5 files changed, 452 insertions(+), 60 deletions(-)
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 262ca31b86d9..80ebb05e387c 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -302,6 +302,14 @@ 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;
+EXPORT_SYMBOL_GPL(osc_pc_lpi_support_confirmed);
+
static u8 sb_uuid_str[] = "0811B06E-4A27-44F9-8D60-3CBBC22E7B48";
static void acpi_bus_osc_support(void)
{
@@ -322,6 +330,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 +338,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 ca0de35d1c3a..98e8c62a961c 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;
@@ -952,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;
}
@@ -963,13 +925,415 @@ 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 /* CONFIG_ACPI_PROCESSOR_CSTATE */
+struct acpi_lpi_states_array {
+ unsigned int size;
+ struct acpi_lpi_state *entries;
+};
+
+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_lpi_states_array *info)
+{
+ acpi_status status;
+ int ret = 0;
+ int pkg_count, state_idx = 1, loop;
+ struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+ union acpi_object *lpi_data;
+ 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_data = buffer.pointer;
+
+ /* There must be at least 4 elements = 3 elements + 1 package */
+ if (!lpi_data || lpi_data->type != ACPI_TYPE_PACKAGE ||
+ lpi_data->package.count < 4) {
+ pr_debug("not enough elements in _LPI\n");
+ ret = -ENODATA;
+ goto end;
+ }
+
+ pkg_count = lpi_data->package.elements[2].integer.value;
+
+ /* Validate number of power states. */
+ if (pkg_count < 1 || pkg_count != lpi_data->package.count - 3) {
+ pr_debug("count given by _LPI is not valid\n");
+ ret = -ENODATA;
+ goto end;
+ }
+
+ lpi_state = kcalloc(pkg_count, sizeof(*lpi_state), GFP_KERNEL);
+ if (!lpi_state) {
+ ret = -ENOMEM;
+ goto end;
+ }
+
+ info->size = pkg_count;
+ info->entries = lpi_state;
+
+ /* LPI States start at index 3 */
+ for (loop = 3; state_idx <= pkg_count;
+ loop++, state_idx++, lpi_state++) {
+ union acpi_object *element, *pkg_elem, *obj;
+
+ element = &lpi_data->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;
+
+ lpi_state->address = reg->address;
+
+ if (reg->space_id == ACPI_ADR_SPACE_FIXED_HARDWARE)
+ lpi_state->entry_method = ACPI_CSTATE_FFH;
+ else
+ lpi_state->entry_method = ACPI_CSTATE_SYSTEMIO;
+ } else if (obj->type == ACPI_TYPE_INTEGER) {
+ lpi_state->entry_method = ACPI_CSTATE_INTEGER;
+ lpi_state->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(lpi_state->desc, obj->string.pointer,
+ ACPI_CX_DESC_LEN);
+
+ lpi_state->index = state_idx;
+ if (obj_get_integer(pkg_elem + 0, &lpi_state->min_residency)) {
+ pr_debug("No min. residency found, assuming 10 us\n");
+ lpi_state->min_residency = 10;
+ }
+
+ if (obj_get_integer(pkg_elem + 1, &lpi_state->wake_latency)) {
+ pr_debug("No wakeup residency found, assuming 10 us\n");
+ lpi_state->wake_latency = 10;
+ }
+
+ if (obj_get_integer(pkg_elem + 2, &lpi_state->flags))
+ lpi_state->flags = 0;
+
+ if (obj_get_integer(pkg_elem + 3, &lpi_state->arch_flags))
+ lpi_state->arch_flags = 0;
+
+ if (obj_get_integer(pkg_elem + 4, &lpi_state->res_cnt_freq))
+ lpi_state->res_cnt_freq = 1;
+
+ if (obj_get_integer(pkg_elem + 5, &lpi_state->enable_parent_state))
+ lpi_state->enable_parent_state = 0;
+ }
+
+ ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Found %d power states\n",
+ state_idx));
+end:
+ kfree(buffer.pointer);
+ return ret;
+}
+
+/*
+ * max_leaf_depth - holds the depth of the processor hierarchy/tree
+ * flat_state_cnt - the number of composite LPI states after the process of flattening
+ */
+static int max_leaf_depth, flat_state_cnt;
+
+/**
+ * combine_lpi_states - combine local and parent LPI states to form a composite LPI state
+ *
+ * @local: local LPI state
+ * @parent: parent LPI state
+ * @result: composite LPI state
+ */
+static bool combine_lpi_states(struct acpi_lpi_state *local,
+ struct acpi_lpi_state *parent,
+ struct acpi_lpi_state *result)
+{
+ if (parent->entry_method == ACPI_CSTATE_INTEGER) {
+ if (!parent->address) /* 0 means autopromotable */
+ return false;
+ result->address = local->address + parent->address;
+ } else {
+ result->address = parent->address;
+ }
+
+ result->min_residency = max(local->min_residency, parent->min_residency);
+ result->wake_latency = local->wake_latency + parent->wake_latency;
+ result->enable_parent_state = parent->enable_parent_state;
+ result->entry_method = local->entry_method;
+
+ result->flags = parent->flags;
+ result->arch_flags = parent->arch_flags;
+
+ strlcpy(result->desc, local->desc, ACPI_CX_DESC_LEN);
+ strlcat(result->desc, "+", ACPI_CX_DESC_LEN);
+ strlcat(result->desc, parent->desc, ACPI_CX_DESC_LEN);
+ return true;
+}
+
+#define ACPI_LPI_STATE_FLAGS_ENABLED BIT(0)
+
+static int flatten_lpi_states(struct acpi_processor *pr,
+ struct acpi_lpi_states_array *info,
+ struct acpi_lpi_state *lpi,
+ uint32_t depth)
+{
+ int j, state_count = info[depth].size;
+ struct acpi_lpi_state *t = info[depth].entries;
+
+ for (j = 0; j < state_count; j++, t++) {
+ struct acpi_lpi_state *flpi;
+ bool valid = false;
+
+ if (!(t->flags & ACPI_LPI_STATE_FLAGS_ENABLED))
+ continue;
+
+ flpi = &pr->power.lpi_states[flat_state_cnt];
+
+ if (depth == max_leaf_depth) { /* leaf/processor node */
+ memcpy(flpi, t, sizeof(*t));
+ flpi->index = flat_state_cnt++;
+ valid = true;
+ } else if (lpi && t->index <= lpi->enable_parent_state) {
+ if (combine_lpi_states(lpi, t, flpi)) {
+ flpi->index = flat_state_cnt++;
+ 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_lpi_states_array *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;
+ flat_state_cnt = 0;
+
+ while (ACPI_SUCCESS(status = acpi_get_parent(handle, &pr_ahandle))) {
+ acpi_bus_get_device(pr_ahandle, &d);
+ handle = pr_ahandle;
+
+ if (strcmp(acpi_device_hid(d), ACPI_PROCESSOR_CONTAINER_HID))
+ break;
+
+ if (!acpi_has_method(pr_ahandle, "_LPI"))
+ break;
+
+ max_leaf_depth++;
+ }
+
+ 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 = flat_state_cnt;
+
+ for (i = 0; i <= max_leaf_depth; i++)
+ kfree(info[i].entries);
+
+ 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_lpi_state *lpi)
+{
+ 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_lpi_state *lpi;
+
+ pr = __this_cpu_read(processors);
+
+ if (unlikely(!pr))
+ return -EINVAL;
+
+ lpi = &pr->power.lpi_states[index];
+ if (lpi->entry_method == ACPI_CSTATE_FFH)
+ return acpi_processor_ffh_lpi_enter(lpi);
+
+ return -EINVAL;
+}
+
+static int acpi_processor_setup_lpi_states(struct acpi_processor *pr)
+{
+ int i;
+ struct acpi_lpi_state *lpi;
+ struct cpuidle_state *state;
+ struct cpuidle_driver *drv = &acpi_idle_driver;
+
+ if (!pr->flags.has_lpi)
+ return -EOPNOTSUPP;
+
+ for (i = 0; i < flat_state_cnt && 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)
+{
+ int ret;
+
+ 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;
@@ -978,18 +1342,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();
@@ -997,7 +1358,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;
@@ -1006,9 +1367,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;
@@ -1045,7 +1403,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);
}
}
@@ -1092,7 +1450,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 48779d678122..e2dcb0c51554 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_lpi_state {
+ 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_lpi_state 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;
@@ -371,7 +389,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)
@@ -384,7 +402,7 @@ 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 288fac5294f5..65754566ae17 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -444,8 +444,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
--
2.7.4
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v7 2/6] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
@ 2016-06-28 13:55 ` Sudeep Holla
0 siblings, 0 replies; 37+ messages in thread
From: Sudeep Holla @ 2016-06-28 13:55 UTC (permalink / raw)
To: linux-arm-kernel
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 | 14 +-
drivers/acpi/processor_driver.c | 2 +-
drivers/acpi/processor_idle.c | 468 +++++++++++++++++++++++++++++++++++-----
include/acpi/processor.h | 24 ++-
include/linux/acpi.h | 4 +
5 files changed, 452 insertions(+), 60 deletions(-)
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 262ca31b86d9..80ebb05e387c 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -302,6 +302,14 @@ 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;
+EXPORT_SYMBOL_GPL(osc_pc_lpi_support_confirmed);
+
static u8 sb_uuid_str[] = "0811B06E-4A27-44F9-8D60-3CBBC22E7B48";
static void acpi_bus_osc_support(void)
{
@@ -322,6 +330,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 +338,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 ca0de35d1c3a..98e8c62a961c 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;
@@ -952,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;
}
@@ -963,13 +925,415 @@ 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 /* CONFIG_ACPI_PROCESSOR_CSTATE */
+struct acpi_lpi_states_array {
+ unsigned int size;
+ struct acpi_lpi_state *entries;
+};
+
+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_lpi_states_array *info)
+{
+ acpi_status status;
+ int ret = 0;
+ int pkg_count, state_idx = 1, loop;
+ struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+ union acpi_object *lpi_data;
+ 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_data = buffer.pointer;
+
+ /* There must be at least 4 elements = 3 elements + 1 package */
+ if (!lpi_data || lpi_data->type != ACPI_TYPE_PACKAGE ||
+ lpi_data->package.count < 4) {
+ pr_debug("not enough elements in _LPI\n");
+ ret = -ENODATA;
+ goto end;
+ }
+
+ pkg_count = lpi_data->package.elements[2].integer.value;
+
+ /* Validate number of power states. */
+ if (pkg_count < 1 || pkg_count != lpi_data->package.count - 3) {
+ pr_debug("count given by _LPI is not valid\n");
+ ret = -ENODATA;
+ goto end;
+ }
+
+ lpi_state = kcalloc(pkg_count, sizeof(*lpi_state), GFP_KERNEL);
+ if (!lpi_state) {
+ ret = -ENOMEM;
+ goto end;
+ }
+
+ info->size = pkg_count;
+ info->entries = lpi_state;
+
+ /* LPI States start at index 3 */
+ for (loop = 3; state_idx <= pkg_count;
+ loop++, state_idx++, lpi_state++) {
+ union acpi_object *element, *pkg_elem, *obj;
+
+ element = &lpi_data->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;
+
+ lpi_state->address = reg->address;
+
+ if (reg->space_id == ACPI_ADR_SPACE_FIXED_HARDWARE)
+ lpi_state->entry_method = ACPI_CSTATE_FFH;
+ else
+ lpi_state->entry_method = ACPI_CSTATE_SYSTEMIO;
+ } else if (obj->type == ACPI_TYPE_INTEGER) {
+ lpi_state->entry_method = ACPI_CSTATE_INTEGER;
+ lpi_state->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(lpi_state->desc, obj->string.pointer,
+ ACPI_CX_DESC_LEN);
+
+ lpi_state->index = state_idx;
+ if (obj_get_integer(pkg_elem + 0, &lpi_state->min_residency)) {
+ pr_debug("No min. residency found, assuming 10 us\n");
+ lpi_state->min_residency = 10;
+ }
+
+ if (obj_get_integer(pkg_elem + 1, &lpi_state->wake_latency)) {
+ pr_debug("No wakeup residency found, assuming 10 us\n");
+ lpi_state->wake_latency = 10;
+ }
+
+ if (obj_get_integer(pkg_elem + 2, &lpi_state->flags))
+ lpi_state->flags = 0;
+
+ if (obj_get_integer(pkg_elem + 3, &lpi_state->arch_flags))
+ lpi_state->arch_flags = 0;
+
+ if (obj_get_integer(pkg_elem + 4, &lpi_state->res_cnt_freq))
+ lpi_state->res_cnt_freq = 1;
+
+ if (obj_get_integer(pkg_elem + 5, &lpi_state->enable_parent_state))
+ lpi_state->enable_parent_state = 0;
+ }
+
+ ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Found %d power states\n",
+ state_idx));
+end:
+ kfree(buffer.pointer);
+ return ret;
+}
+
+/*
+ * max_leaf_depth - holds the depth of the processor hierarchy/tree
+ * flat_state_cnt - the number of composite LPI states after the process of flattening
+ */
+static int max_leaf_depth, flat_state_cnt;
+
+/**
+ * combine_lpi_states - combine local and parent LPI states to form a composite LPI state
+ *
+ * @local: local LPI state
+ * @parent: parent LPI state
+ * @result: composite LPI state
+ */
+static bool combine_lpi_states(struct acpi_lpi_state *local,
+ struct acpi_lpi_state *parent,
+ struct acpi_lpi_state *result)
+{
+ if (parent->entry_method == ACPI_CSTATE_INTEGER) {
+ if (!parent->address) /* 0 means autopromotable */
+ return false;
+ result->address = local->address + parent->address;
+ } else {
+ result->address = parent->address;
+ }
+
+ result->min_residency = max(local->min_residency, parent->min_residency);
+ result->wake_latency = local->wake_latency + parent->wake_latency;
+ result->enable_parent_state = parent->enable_parent_state;
+ result->entry_method = local->entry_method;
+
+ result->flags = parent->flags;
+ result->arch_flags = parent->arch_flags;
+
+ strlcpy(result->desc, local->desc, ACPI_CX_DESC_LEN);
+ strlcat(result->desc, "+", ACPI_CX_DESC_LEN);
+ strlcat(result->desc, parent->desc, ACPI_CX_DESC_LEN);
+ return true;
+}
+
+#define ACPI_LPI_STATE_FLAGS_ENABLED BIT(0)
+
+static int flatten_lpi_states(struct acpi_processor *pr,
+ struct acpi_lpi_states_array *info,
+ struct acpi_lpi_state *lpi,
+ uint32_t depth)
+{
+ int j, state_count = info[depth].size;
+ struct acpi_lpi_state *t = info[depth].entries;
+
+ for (j = 0; j < state_count; j++, t++) {
+ struct acpi_lpi_state *flpi;
+ bool valid = false;
+
+ if (!(t->flags & ACPI_LPI_STATE_FLAGS_ENABLED))
+ continue;
+
+ flpi = &pr->power.lpi_states[flat_state_cnt];
+
+ if (depth == max_leaf_depth) { /* leaf/processor node */
+ memcpy(flpi, t, sizeof(*t));
+ flpi->index = flat_state_cnt++;
+ valid = true;
+ } else if (lpi && t->index <= lpi->enable_parent_state) {
+ if (combine_lpi_states(lpi, t, flpi)) {
+ flpi->index = flat_state_cnt++;
+ 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_lpi_states_array *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;
+ flat_state_cnt = 0;
+
+ while (ACPI_SUCCESS(status = acpi_get_parent(handle, &pr_ahandle))) {
+ acpi_bus_get_device(pr_ahandle, &d);
+ handle = pr_ahandle;
+
+ if (strcmp(acpi_device_hid(d), ACPI_PROCESSOR_CONTAINER_HID))
+ break;
+
+ if (!acpi_has_method(pr_ahandle, "_LPI"))
+ break;
+
+ max_leaf_depth++;
+ }
+
+ 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 = flat_state_cnt;
+
+ for (i = 0; i <= max_leaf_depth; i++)
+ kfree(info[i].entries);
+
+ 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_lpi_state *lpi)
+{
+ 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_lpi_state *lpi;
+
+ pr = __this_cpu_read(processors);
+
+ if (unlikely(!pr))
+ return -EINVAL;
+
+ lpi = &pr->power.lpi_states[index];
+ if (lpi->entry_method == ACPI_CSTATE_FFH)
+ return acpi_processor_ffh_lpi_enter(lpi);
+
+ return -EINVAL;
+}
+
+static int acpi_processor_setup_lpi_states(struct acpi_processor *pr)
+{
+ int i;
+ struct acpi_lpi_state *lpi;
+ struct cpuidle_state *state;
+ struct cpuidle_driver *drv = &acpi_idle_driver;
+
+ if (!pr->flags.has_lpi)
+ return -EOPNOTSUPP;
+
+ for (i = 0; i < flat_state_cnt && 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)
+{
+ int ret;
+
+ 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;
@@ -978,18 +1342,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();
@@ -997,7 +1358,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;
@@ -1006,9 +1367,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;
@@ -1045,7 +1403,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);
}
}
@@ -1092,7 +1450,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 48779d678122..e2dcb0c51554 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_lpi_state {
+ 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_lpi_state 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;
@@ -371,7 +389,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)
@@ -384,7 +402,7 @@ 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 288fac5294f5..65754566ae17 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -444,8 +444,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
--
2.7.4
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v7 2/6] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
@ 2016-06-28 13:55 ` Sudeep Holla
0 siblings, 0 replies; 37+ messages in thread
From: Sudeep Holla @ 2016-06-28 13:55 UTC (permalink / raw)
To: linux-acpi, Rafael J . Wysocki
Cc: Sudeep Holla, Vikas Sajjan, Sunil, Lorenzo Pieralisi,
PrashanthPrakash, Al Stone, Ashwin Chaugule, Daniel Lezcano,
linux-kernel, linux-arm-kernel
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 | 14 +-
drivers/acpi/processor_driver.c | 2 +-
drivers/acpi/processor_idle.c | 468 +++++++++++++++++++++++++++++++++++-----
include/acpi/processor.h | 24 ++-
include/linux/acpi.h | 4 +
5 files changed, 452 insertions(+), 60 deletions(-)
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 262ca31b86d9..80ebb05e387c 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -302,6 +302,14 @@ 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;
+EXPORT_SYMBOL_GPL(osc_pc_lpi_support_confirmed);
+
static u8 sb_uuid_str[] = "0811B06E-4A27-44F9-8D60-3CBBC22E7B48";
static void acpi_bus_osc_support(void)
{
@@ -322,6 +330,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 +338,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 ca0de35d1c3a..98e8c62a961c 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;
@@ -952,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;
}
@@ -963,13 +925,415 @@ 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 /* CONFIG_ACPI_PROCESSOR_CSTATE */
+struct acpi_lpi_states_array {
+ unsigned int size;
+ struct acpi_lpi_state *entries;
+};
+
+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_lpi_states_array *info)
+{
+ acpi_status status;
+ int ret = 0;
+ int pkg_count, state_idx = 1, loop;
+ struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+ union acpi_object *lpi_data;
+ 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_data = buffer.pointer;
+
+ /* There must be at least 4 elements = 3 elements + 1 package */
+ if (!lpi_data || lpi_data->type != ACPI_TYPE_PACKAGE ||
+ lpi_data->package.count < 4) {
+ pr_debug("not enough elements in _LPI\n");
+ ret = -ENODATA;
+ goto end;
+ }
+
+ pkg_count = lpi_data->package.elements[2].integer.value;
+
+ /* Validate number of power states. */
+ if (pkg_count < 1 || pkg_count != lpi_data->package.count - 3) {
+ pr_debug("count given by _LPI is not valid\n");
+ ret = -ENODATA;
+ goto end;
+ }
+
+ lpi_state = kcalloc(pkg_count, sizeof(*lpi_state), GFP_KERNEL);
+ if (!lpi_state) {
+ ret = -ENOMEM;
+ goto end;
+ }
+
+ info->size = pkg_count;
+ info->entries = lpi_state;
+
+ /* LPI States start at index 3 */
+ for (loop = 3; state_idx <= pkg_count;
+ loop++, state_idx++, lpi_state++) {
+ union acpi_object *element, *pkg_elem, *obj;
+
+ element = &lpi_data->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;
+
+ lpi_state->address = reg->address;
+
+ if (reg->space_id == ACPI_ADR_SPACE_FIXED_HARDWARE)
+ lpi_state->entry_method = ACPI_CSTATE_FFH;
+ else
+ lpi_state->entry_method = ACPI_CSTATE_SYSTEMIO;
+ } else if (obj->type == ACPI_TYPE_INTEGER) {
+ lpi_state->entry_method = ACPI_CSTATE_INTEGER;
+ lpi_state->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(lpi_state->desc, obj->string.pointer,
+ ACPI_CX_DESC_LEN);
+
+ lpi_state->index = state_idx;
+ if (obj_get_integer(pkg_elem + 0, &lpi_state->min_residency)) {
+ pr_debug("No min. residency found, assuming 10 us\n");
+ lpi_state->min_residency = 10;
+ }
+
+ if (obj_get_integer(pkg_elem + 1, &lpi_state->wake_latency)) {
+ pr_debug("No wakeup residency found, assuming 10 us\n");
+ lpi_state->wake_latency = 10;
+ }
+
+ if (obj_get_integer(pkg_elem + 2, &lpi_state->flags))
+ lpi_state->flags = 0;
+
+ if (obj_get_integer(pkg_elem + 3, &lpi_state->arch_flags))
+ lpi_state->arch_flags = 0;
+
+ if (obj_get_integer(pkg_elem + 4, &lpi_state->res_cnt_freq))
+ lpi_state->res_cnt_freq = 1;
+
+ if (obj_get_integer(pkg_elem + 5, &lpi_state->enable_parent_state))
+ lpi_state->enable_parent_state = 0;
+ }
+
+ ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Found %d power states\n",
+ state_idx));
+end:
+ kfree(buffer.pointer);
+ return ret;
+}
+
+/*
+ * max_leaf_depth - holds the depth of the processor hierarchy/tree
+ * flat_state_cnt - the number of composite LPI states after the process of flattening
+ */
+static int max_leaf_depth, flat_state_cnt;
+
+/**
+ * combine_lpi_states - combine local and parent LPI states to form a composite LPI state
+ *
+ * @local: local LPI state
+ * @parent: parent LPI state
+ * @result: composite LPI state
+ */
+static bool combine_lpi_states(struct acpi_lpi_state *local,
+ struct acpi_lpi_state *parent,
+ struct acpi_lpi_state *result)
+{
+ if (parent->entry_method == ACPI_CSTATE_INTEGER) {
+ if (!parent->address) /* 0 means autopromotable */
+ return false;
+ result->address = local->address + parent->address;
+ } else {
+ result->address = parent->address;
+ }
+
+ result->min_residency = max(local->min_residency, parent->min_residency);
+ result->wake_latency = local->wake_latency + parent->wake_latency;
+ result->enable_parent_state = parent->enable_parent_state;
+ result->entry_method = local->entry_method;
+
+ result->flags = parent->flags;
+ result->arch_flags = parent->arch_flags;
+
+ strlcpy(result->desc, local->desc, ACPI_CX_DESC_LEN);
+ strlcat(result->desc, "+", ACPI_CX_DESC_LEN);
+ strlcat(result->desc, parent->desc, ACPI_CX_DESC_LEN);
+ return true;
+}
+
+#define ACPI_LPI_STATE_FLAGS_ENABLED BIT(0)
+
+static int flatten_lpi_states(struct acpi_processor *pr,
+ struct acpi_lpi_states_array *info,
+ struct acpi_lpi_state *lpi,
+ uint32_t depth)
+{
+ int j, state_count = info[depth].size;
+ struct acpi_lpi_state *t = info[depth].entries;
+
+ for (j = 0; j < state_count; j++, t++) {
+ struct acpi_lpi_state *flpi;
+ bool valid = false;
+
+ if (!(t->flags & ACPI_LPI_STATE_FLAGS_ENABLED))
+ continue;
+
+ flpi = &pr->power.lpi_states[flat_state_cnt];
+
+ if (depth == max_leaf_depth) { /* leaf/processor node */
+ memcpy(flpi, t, sizeof(*t));
+ flpi->index = flat_state_cnt++;
+ valid = true;
+ } else if (lpi && t->index <= lpi->enable_parent_state) {
+ if (combine_lpi_states(lpi, t, flpi)) {
+ flpi->index = flat_state_cnt++;
+ 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_lpi_states_array *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;
+ flat_state_cnt = 0;
+
+ while (ACPI_SUCCESS(status = acpi_get_parent(handle, &pr_ahandle))) {
+ acpi_bus_get_device(pr_ahandle, &d);
+ handle = pr_ahandle;
+
+ if (strcmp(acpi_device_hid(d), ACPI_PROCESSOR_CONTAINER_HID))
+ break;
+
+ if (!acpi_has_method(pr_ahandle, "_LPI"))
+ break;
+
+ max_leaf_depth++;
+ }
+
+ 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 = flat_state_cnt;
+
+ for (i = 0; i <= max_leaf_depth; i++)
+ kfree(info[i].entries);
+
+ 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_lpi_state *lpi)
+{
+ 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_lpi_state *lpi;
+
+ pr = __this_cpu_read(processors);
+
+ if (unlikely(!pr))
+ return -EINVAL;
+
+ lpi = &pr->power.lpi_states[index];
+ if (lpi->entry_method == ACPI_CSTATE_FFH)
+ return acpi_processor_ffh_lpi_enter(lpi);
+
+ return -EINVAL;
+}
+
+static int acpi_processor_setup_lpi_states(struct acpi_processor *pr)
+{
+ int i;
+ struct acpi_lpi_state *lpi;
+ struct cpuidle_state *state;
+ struct cpuidle_driver *drv = &acpi_idle_driver;
+
+ if (!pr->flags.has_lpi)
+ return -EOPNOTSUPP;
+
+ for (i = 0; i < flat_state_cnt && 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)
+{
+ int ret;
+
+ 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;
@@ -978,18 +1342,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();
@@ -997,7 +1358,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;
@@ -1006,9 +1367,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;
@@ -1045,7 +1403,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);
}
}
@@ -1092,7 +1450,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 48779d678122..e2dcb0c51554 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_lpi_state {
+ 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_lpi_state 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;
@@ -371,7 +389,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)
@@ -384,7 +402,7 @@ 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 288fac5294f5..65754566ae17 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -444,8 +444,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
--
2.7.4
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v7 2/6] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
2016-06-28 13:55 ` Sudeep Holla
(?)
@ 2016-07-01 13:07 ` Daniel Lezcano
-1 siblings, 0 replies; 37+ messages in thread
From: Daniel Lezcano @ 2016-07-01 13:07 UTC (permalink / raw)
To: Sudeep Holla, linux-acpi, Rafael J . Wysocki
Cc: Lorenzo Pieralisi, Vincent Guittot, PrashanthPrakash, Al Stone,
Vikas Sajjan, linux-kernel, Ashwin Chaugule, linux-arm-kernel,
Sunil
On 06/28/2016 03:55 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>
> ---
Hi Sudeep,
I looked at the acpi processor idle code sometime ago and from my POV,
it was awful, unnecessary complex and very difficult to maintain. The
usage of flags all over the places is significant of the lack of control
of the written code.
Even if you are not responsible of this implementation, the current
situation forces you to add more awful code on top of that, which is
clearly against "making Linux better".
IMO, the current code deserves a huge cleanup before applying anything
new : cstate and lpi should be investigated to be self-contained in
their respective file and consolidated, the global variable usage should
be killed, redundant flag checking removed by recapturing the code flow,
etc ... I believe the usage of acpi probe table could be one entry point
to begin this cleanup.
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
_______________________________________________
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] 37+ messages in thread
* [PATCH v7 2/6] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
@ 2016-07-01 13:07 ` Daniel Lezcano
0 siblings, 0 replies; 37+ messages in thread
From: Daniel Lezcano @ 2016-07-01 13:07 UTC (permalink / raw)
To: linux-arm-kernel
On 06/28/2016 03:55 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>
> ---
Hi Sudeep,
I looked at the acpi processor idle code sometime ago and from my POV,
it was awful, unnecessary complex and very difficult to maintain. The
usage of flags all over the places is significant of the lack of control
of the written code.
Even if you are not responsible of this implementation, the current
situation forces you to add more awful code on top of that, which is
clearly against "making Linux better".
IMO, the current code deserves a huge cleanup before applying anything
new : cstate and lpi should be investigated to be self-contained in
their respective file and consolidated, the global variable usage should
be killed, redundant flag checking removed by recapturing the code flow,
etc ... I believe the usage of acpi probe table could be one entry point
to begin this cleanup.
--
<http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v7 2/6] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
@ 2016-07-01 13:07 ` Daniel Lezcano
0 siblings, 0 replies; 37+ messages in thread
From: Daniel Lezcano @ 2016-07-01 13:07 UTC (permalink / raw)
To: Sudeep Holla, linux-acpi, Rafael J . Wysocki
Cc: Vikas Sajjan, Sunil, Lorenzo Pieralisi, PrashanthPrakash,
Al Stone, Ashwin Chaugule, linux-kernel, linux-arm-kernel,
Vincent Guittot
On 06/28/2016 03:55 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>
> ---
Hi Sudeep,
I looked at the acpi processor idle code sometime ago and from my POV,
it was awful, unnecessary complex and very difficult to maintain. The
usage of flags all over the places is significant of the lack of control
of the written code.
Even if you are not responsible of this implementation, the current
situation forces you to add more awful code on top of that, which is
clearly against "making Linux better".
IMO, the current code deserves a huge cleanup before applying anything
new : cstate and lpi should be investigated to be self-contained in
their respective file and consolidated, the global variable usage should
be killed, redundant flag checking removed by recapturing the code flow,
etc ... I believe the usage of acpi probe table could be one entry point
to begin this cleanup.
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v7 2/6] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
2016-07-01 13:07 ` Daniel Lezcano
@ 2016-07-04 13:00 ` Sudeep Holla
-1 siblings, 0 replies; 37+ messages in thread
From: Sudeep Holla @ 2016-07-04 13:00 UTC (permalink / raw)
To: Daniel Lezcano, linux-acpi, Rafael J . Wysocki
Cc: Sudeep Holla, Vikas Sajjan, Sunil, Lorenzo Pieralisi,
PrashanthPrakash, Al Stone, Ashwin Chaugule, linux-kernel,
linux-arm-kernel, Vincent Guittot
On 01/07/16 14:07, Daniel Lezcano wrote:
> On 06/28/2016 03:55 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>
>> ---
>
> Hi Sudeep,
>
> I looked at the acpi processor idle code sometime ago and from my POV,
> it was awful, unnecessary complex and very difficult to maintain. The
> usage of flags all over the places is significant of the lack of control
> of the written code.
>
So you have any specific things in mind ? That's too broad and I know
it's not so clean, but it's so for legacy reasons. I would leave that
to Rafael to decide as it takes lots of testing to clean up these code.
> Even if you are not responsible of this implementation, the current
> situation forces you to add more awful code on top of that, which is
> clearly against "making Linux better".
>
OK
> IMO, the current code deserves a huge cleanup before applying anything
> new : cstate and lpi should be investigated to be self-contained in
> their respective file and consolidated, the global variable usage should
> be killed, redundant flag checking removed by recapturing the code flow,
> etc ... I believe the usage of acpi probe table could be one entry point
> to begin this cleanup.
>
This is not a static table.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v7 2/6] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
@ 2016-07-04 13:00 ` Sudeep Holla
0 siblings, 0 replies; 37+ messages in thread
From: Sudeep Holla @ 2016-07-04 13:00 UTC (permalink / raw)
To: linux-arm-kernel
On 01/07/16 14:07, Daniel Lezcano wrote:
> On 06/28/2016 03:55 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>
>> ---
>
> Hi Sudeep,
>
> I looked at the acpi processor idle code sometime ago and from my POV,
> it was awful, unnecessary complex and very difficult to maintain. The
> usage of flags all over the places is significant of the lack of control
> of the written code.
>
So you have any specific things in mind ? That's too broad and I know
it's not so clean, but it's so for legacy reasons. I would leave that
to Rafael to decide as it takes lots of testing to clean up these code.
> Even if you are not responsible of this implementation, the current
> situation forces you to add more awful code on top of that, which is
> clearly against "making Linux better".
>
OK
> IMO, the current code deserves a huge cleanup before applying anything
> new : cstate and lpi should be investigated to be self-contained in
> their respective file and consolidated, the global variable usage should
> be killed, redundant flag checking removed by recapturing the code flow,
> etc ... I believe the usage of acpi probe table could be one entry point
> to begin this cleanup.
>
This is not a static table.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v7 2/6] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
2016-07-04 13:00 ` Sudeep Holla
@ 2016-07-04 13:17 ` Rafael J. Wysocki
-1 siblings, 0 replies; 37+ messages in thread
From: Rafael J. Wysocki @ 2016-07-04 13:17 UTC (permalink / raw)
To: Sudeep Holla
Cc: Daniel Lezcano, linux-acpi, Vikas Sajjan, Sunil,
Lorenzo Pieralisi, PrashanthPrakash, Al Stone, Ashwin Chaugule,
linux-kernel, linux-arm-kernel, Vincent Guittot
On Monday, July 04, 2016 02:00:03 PM Sudeep Holla wrote:
>
> On 01/07/16 14:07, Daniel Lezcano wrote:
> > On 06/28/2016 03:55 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>
> >> ---
> >
> > Hi Sudeep,
> >
> > I looked at the acpi processor idle code sometime ago and from my POV,
> > it was awful, unnecessary complex and very difficult to maintain. The
> > usage of flags all over the places is significant of the lack of control
> > of the written code.
> >
>
> So you have any specific things in mind ? That's too broad and I know
> it's not so clean, but it's so for legacy reasons. I would leave that
> to Rafael to decide as it takes lots of testing to clean up these code.
The cleanup needs to be done at one point.
Question is when to do it, before adding LPI support or after doing that
(and each option has its pros and cons IMO).
> > Even if you are not responsible of this implementation, the current
> > situation forces you to add more awful code on top of that, which is
> > clearly against "making Linux better".
> >
>
> OK
So if there are cases in which you need to make the code more complex
because of the legacy stuff in there, I'd say it's better to clean it up
first.
Thanks,
Rafael
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v7 2/6] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
@ 2016-07-04 13:17 ` Rafael J. Wysocki
0 siblings, 0 replies; 37+ messages in thread
From: Rafael J. Wysocki @ 2016-07-04 13:17 UTC (permalink / raw)
To: linux-arm-kernel
On Monday, July 04, 2016 02:00:03 PM Sudeep Holla wrote:
>
> On 01/07/16 14:07, Daniel Lezcano wrote:
> > On 06/28/2016 03:55 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>
> >> ---
> >
> > Hi Sudeep,
> >
> > I looked at the acpi processor idle code sometime ago and from my POV,
> > it was awful, unnecessary complex and very difficult to maintain. The
> > usage of flags all over the places is significant of the lack of control
> > of the written code.
> >
>
> So you have any specific things in mind ? That's too broad and I know
> it's not so clean, but it's so for legacy reasons. I would leave that
> to Rafael to decide as it takes lots of testing to clean up these code.
The cleanup needs to be done at one point.
Question is when to do it, before adding LPI support or after doing that
(and each option has its pros and cons IMO).
> > Even if you are not responsible of this implementation, the current
> > situation forces you to add more awful code on top of that, which is
> > clearly against "making Linux better".
> >
>
> OK
So if there are cases in which you need to make the code more complex
because of the legacy stuff in there, I'd say it's better to clean it up
first.
Thanks,
Rafael
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v7 2/6] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
2016-07-04 13:17 ` Rafael J. Wysocki
(?)
@ 2016-07-04 13:42 ` Sudeep Holla
-1 siblings, 0 replies; 37+ messages in thread
From: Sudeep Holla @ 2016-07-04 13:42 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Lorenzo Pieralisi, Vincent Guittot, Al Stone, PrashanthPrakash,
Vikas Sajjan, Daniel Lezcano, linux-kernel, linux-acpi,
Ashwin Chaugule, Sudeep Holla, linux-arm-kernel, Sunil
On 04/07/16 14:17, Rafael J. Wysocki wrote:
> On Monday, July 04, 2016 02:00:03 PM Sudeep Holla wrote:
>>
>> On 01/07/16 14:07, Daniel Lezcano wrote:
>>> On 06/28/2016 03:55 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>
>>>> ---
>>>
>>> Hi Sudeep,
>>>
>>> I looked at the acpi processor idle code sometime ago and from my POV,
>>> it was awful, unnecessary complex and very difficult to maintain. The
>>> usage of flags all over the places is significant of the lack of control
>>> of the written code.
>>>
>>
>> So you have any specific things in mind ? That's too broad and I know
>> it's not so clean, but it's so for legacy reasons. I would leave that
>> to Rafael to decide as it takes lots of testing to clean up these code.
>
> The cleanup needs to be done at one point.
>
> Question is when to do it, before adding LPI support or after doing that
> (and each option has its pros and cons IMO).
>
>>> Even if you are not responsible of this implementation, the current
>>> situation forces you to add more awful code on top of that, which is
>>> clearly against "making Linux better".
>>>
>>
>> OK
>
> So if there are cases in which you need to make the code more complex
> because of the legacy stuff in there, I'd say it's better to clean it up
> first.
>
I am not sure if Daniel was referring to anything specific. I have
cleaned up in patch 1/6 for cstate. More cleanups can be done there but
needs better understanding and reasoning for the current code which I
don't have as they are mostly x86 related.
Unless someone points me what they would like to change and how, I don't
have much in my mind to do here. Yes it may not look as clean as other
code in the kernel relatively, but without complete understanding of the
history/reasoning for the current state of code I wouldn't touch
anything I don't understand.
I am open to make changes if there's something specific. Sorry I can't
go ahead making changes the way I think based on some vague idea that
the current code is not clean.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v7 2/6] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
@ 2016-07-04 13:42 ` Sudeep Holla
0 siblings, 0 replies; 37+ messages in thread
From: Sudeep Holla @ 2016-07-04 13:42 UTC (permalink / raw)
To: linux-arm-kernel
On 04/07/16 14:17, Rafael J. Wysocki wrote:
> On Monday, July 04, 2016 02:00:03 PM Sudeep Holla wrote:
>>
>> On 01/07/16 14:07, Daniel Lezcano wrote:
>>> On 06/28/2016 03:55 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>
>>>> ---
>>>
>>> Hi Sudeep,
>>>
>>> I looked at the acpi processor idle code sometime ago and from my POV,
>>> it was awful, unnecessary complex and very difficult to maintain. The
>>> usage of flags all over the places is significant of the lack of control
>>> of the written code.
>>>
>>
>> So you have any specific things in mind ? That's too broad and I know
>> it's not so clean, but it's so for legacy reasons. I would leave that
>> to Rafael to decide as it takes lots of testing to clean up these code.
>
> The cleanup needs to be done at one point.
>
> Question is when to do it, before adding LPI support or after doing that
> (and each option has its pros and cons IMO).
>
>>> Even if you are not responsible of this implementation, the current
>>> situation forces you to add more awful code on top of that, which is
>>> clearly against "making Linux better".
>>>
>>
>> OK
>
> So if there are cases in which you need to make the code more complex
> because of the legacy stuff in there, I'd say it's better to clean it up
> first.
>
I am not sure if Daniel was referring to anything specific. I have
cleaned up in patch 1/6 for cstate. More cleanups can be done there but
needs better understanding and reasoning for the current code which I
don't have as they are mostly x86 related.
Unless someone points me what they would like to change and how, I don't
have much in my mind to do here. Yes it may not look as clean as other
code in the kernel relatively, but without complete understanding of the
history/reasoning for the current state of code I wouldn't touch
anything I don't understand.
I am open to make changes if there's something specific. Sorry I can't
go ahead making changes the way I think based on some vague idea that
the current code is not clean.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v7 2/6] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
@ 2016-07-04 13:42 ` Sudeep Holla
0 siblings, 0 replies; 37+ messages in thread
From: Sudeep Holla @ 2016-07-04 13:42 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Sudeep Holla, Daniel Lezcano, linux-acpi, Vikas Sajjan, Sunil,
Lorenzo Pieralisi, PrashanthPrakash, Al Stone, Ashwin Chaugule,
linux-kernel, linux-arm-kernel, Vincent Guittot
On 04/07/16 14:17, Rafael J. Wysocki wrote:
> On Monday, July 04, 2016 02:00:03 PM Sudeep Holla wrote:
>>
>> On 01/07/16 14:07, Daniel Lezcano wrote:
>>> On 06/28/2016 03:55 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>
>>>> ---
>>>
>>> Hi Sudeep,
>>>
>>> I looked at the acpi processor idle code sometime ago and from my POV,
>>> it was awful, unnecessary complex and very difficult to maintain. The
>>> usage of flags all over the places is significant of the lack of control
>>> of the written code.
>>>
>>
>> So you have any specific things in mind ? That's too broad and I know
>> it's not so clean, but it's so for legacy reasons. I would leave that
>> to Rafael to decide as it takes lots of testing to clean up these code.
>
> The cleanup needs to be done at one point.
>
> Question is when to do it, before adding LPI support or after doing that
> (and each option has its pros and cons IMO).
>
>>> Even if you are not responsible of this implementation, the current
>>> situation forces you to add more awful code on top of that, which is
>>> clearly against "making Linux better".
>>>
>>
>> OK
>
> So if there are cases in which you need to make the code more complex
> because of the legacy stuff in there, I'd say it's better to clean it up
> first.
>
I am not sure if Daniel was referring to anything specific. I have
cleaned up in patch 1/6 for cstate. More cleanups can be done there but
needs better understanding and reasoning for the current code which I
don't have as they are mostly x86 related.
Unless someone points me what they would like to change and how, I don't
have much in my mind to do here. Yes it may not look as clean as other
code in the kernel relatively, but without complete understanding of the
history/reasoning for the current state of code I wouldn't touch
anything I don't understand.
I am open to make changes if there's something specific. Sorry I can't
go ahead making changes the way I think based on some vague idea that
the current code is not clean.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v7 2/6] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
2016-07-04 13:42 ` Sudeep Holla
(?)
@ 2016-07-04 14:11 ` Rafael J. Wysocki
-1 siblings, 0 replies; 37+ messages in thread
From: Rafael J. Wysocki @ 2016-07-04 14:11 UTC (permalink / raw)
To: Sudeep Holla
Cc: Rafael J. Wysocki, Daniel Lezcano, ACPI Devel Maling List,
Vikas Sajjan, Sunil, Lorenzo Pieralisi, PrashanthPrakash,
Al Stone, Ashwin Chaugule, Linux Kernel Mailing List,
linux-arm-kernel, Vincent Guittot
On Mon, Jul 4, 2016 at 3:42 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
>
> On 04/07/16 14:17, Rafael J. Wysocki wrote:
>>
>> On Monday, July 04, 2016 02:00:03 PM Sudeep Holla wrote:
>>>
>>>
>>> On 01/07/16 14:07, Daniel Lezcano wrote:
>>>>
>>>> On 06/28/2016 03:55 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>
>>>>> ---
>>>>
>>>>
>>>> Hi Sudeep,
>>>>
>>>> I looked at the acpi processor idle code sometime ago and from my POV,
>>>> it was awful, unnecessary complex and very difficult to maintain. The
>>>> usage of flags all over the places is significant of the lack of control
>>>> of the written code.
>>>>
>>>
>>> So you have any specific things in mind ? That's too broad and I know
>>> it's not so clean, but it's so for legacy reasons. I would leave that
>>> to Rafael to decide as it takes lots of testing to clean up these code.
>>
>>
>> The cleanup needs to be done at one point.
>>
>> Question is when to do it, before adding LPI support or after doing that
>> (and each option has its pros and cons IMO).
>>
>>>> Even if you are not responsible of this implementation, the current
>>>> situation forces you to add more awful code on top of that, which is
>>>> clearly against "making Linux better".
>>>>
>>>
>>> OK
>>
>>
>> So if there are cases in which you need to make the code more complex
>> because of the legacy stuff in there, I'd say it's better to clean it up
>> first.
>>
>
> I am not sure if Daniel was referring to anything specific. I have
> cleaned up in patch 1/6 for cstate. More cleanups can be done there but
> needs better understanding and reasoning for the current code which I
> don't have as they are mostly x86 related.
>
> Unless someone points me what they would like to change and how, I don't
> have much in my mind to do here. Yes it may not look as clean as other
> code in the kernel relatively, but without complete understanding of the
> history/reasoning for the current state of code I wouldn't touch
> anything I don't understand.
>
> I am open to make changes if there's something specific. Sorry I can't
> go ahead making changes the way I think based on some vague idea that
> the current code is not clean.
Fair enough.
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v7 2/6] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
@ 2016-07-04 14:11 ` Rafael J. Wysocki
0 siblings, 0 replies; 37+ messages in thread
From: Rafael J. Wysocki @ 2016-07-04 14:11 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Jul 4, 2016 at 3:42 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
>
> On 04/07/16 14:17, Rafael J. Wysocki wrote:
>>
>> On Monday, July 04, 2016 02:00:03 PM Sudeep Holla wrote:
>>>
>>>
>>> On 01/07/16 14:07, Daniel Lezcano wrote:
>>>>
>>>> On 06/28/2016 03:55 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>
>>>>> ---
>>>>
>>>>
>>>> Hi Sudeep,
>>>>
>>>> I looked at the acpi processor idle code sometime ago and from my POV,
>>>> it was awful, unnecessary complex and very difficult to maintain. The
>>>> usage of flags all over the places is significant of the lack of control
>>>> of the written code.
>>>>
>>>
>>> So you have any specific things in mind ? That's too broad and I know
>>> it's not so clean, but it's so for legacy reasons. I would leave that
>>> to Rafael to decide as it takes lots of testing to clean up these code.
>>
>>
>> The cleanup needs to be done at one point.
>>
>> Question is when to do it, before adding LPI support or after doing that
>> (and each option has its pros and cons IMO).
>>
>>>> Even if you are not responsible of this implementation, the current
>>>> situation forces you to add more awful code on top of that, which is
>>>> clearly against "making Linux better".
>>>>
>>>
>>> OK
>>
>>
>> So if there are cases in which you need to make the code more complex
>> because of the legacy stuff in there, I'd say it's better to clean it up
>> first.
>>
>
> I am not sure if Daniel was referring to anything specific. I have
> cleaned up in patch 1/6 for cstate. More cleanups can be done there but
> needs better understanding and reasoning for the current code which I
> don't have as they are mostly x86 related.
>
> Unless someone points me what they would like to change and how, I don't
> have much in my mind to do here. Yes it may not look as clean as other
> code in the kernel relatively, but without complete understanding of the
> history/reasoning for the current state of code I wouldn't touch
> anything I don't understand.
>
> I am open to make changes if there's something specific. Sorry I can't
> go ahead making changes the way I think based on some vague idea that
> the current code is not clean.
Fair enough.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v7 2/6] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
@ 2016-07-04 14:11 ` Rafael J. Wysocki
0 siblings, 0 replies; 37+ messages in thread
From: Rafael J. Wysocki @ 2016-07-04 14:11 UTC (permalink / raw)
To: Sudeep Holla
Cc: Rafael J. Wysocki, Daniel Lezcano, ACPI Devel Maling List,
Vikas Sajjan, Sunil, Lorenzo Pieralisi, PrashanthPrakash,
Al Stone, Ashwin Chaugule, Linux Kernel Mailing List,
linux-arm-kernel, Vincent Guittot
On Mon, Jul 4, 2016 at 3:42 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
>
> On 04/07/16 14:17, Rafael J. Wysocki wrote:
>>
>> On Monday, July 04, 2016 02:00:03 PM Sudeep Holla wrote:
>>>
>>>
>>> On 01/07/16 14:07, Daniel Lezcano wrote:
>>>>
>>>> On 06/28/2016 03:55 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>
>>>>> ---
>>>>
>>>>
>>>> Hi Sudeep,
>>>>
>>>> I looked at the acpi processor idle code sometime ago and from my POV,
>>>> it was awful, unnecessary complex and very difficult to maintain. The
>>>> usage of flags all over the places is significant of the lack of control
>>>> of the written code.
>>>>
>>>
>>> So you have any specific things in mind ? That's too broad and I know
>>> it's not so clean, but it's so for legacy reasons. I would leave that
>>> to Rafael to decide as it takes lots of testing to clean up these code.
>>
>>
>> The cleanup needs to be done at one point.
>>
>> Question is when to do it, before adding LPI support or after doing that
>> (and each option has its pros and cons IMO).
>>
>>>> Even if you are not responsible of this implementation, the current
>>>> situation forces you to add more awful code on top of that, which is
>>>> clearly against "making Linux better".
>>>>
>>>
>>> OK
>>
>>
>> So if there are cases in which you need to make the code more complex
>> because of the legacy stuff in there, I'd say it's better to clean it up
>> first.
>>
>
> I am not sure if Daniel was referring to anything specific. I have
> cleaned up in patch 1/6 for cstate. More cleanups can be done there but
> needs better understanding and reasoning for the current code which I
> don't have as they are mostly x86 related.
>
> Unless someone points me what they would like to change and how, I don't
> have much in my mind to do here. Yes it may not look as clean as other
> code in the kernel relatively, but without complete understanding of the
> history/reasoning for the current state of code I wouldn't touch
> anything I don't understand.
>
> I am open to make changes if there's something specific. Sorry I can't
> go ahead making changes the way I think based on some vague idea that
> the current code is not clean.
Fair enough.
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v7 3/6] arm64: cpuidle: drop __init section marker to arm_cpuidle_init
2016-06-28 13:55 ` Sudeep Holla
@ 2016-06-28 13:55 ` Sudeep Holla
-1 siblings, 0 replies; 37+ messages in thread
From: Sudeep Holla @ 2016-06-28 13:55 UTC (permalink / raw)
To: linux-acpi, Rafael J . Wysocki
Cc: Sudeep Holla, Vikas Sajjan, Sunil, Lorenzo Pieralisi,
PrashanthPrakash, Al Stone, Ashwin Chaugule, Daniel Lezcano,
linux-kernel, linux-arm-kernel, Mark Rutland
Commit ea389daa7fd9 ("arm64: cpuidle: add __init section marker to
arm_cpuidle_init") added the __init annotation to arm_cpuidle_init
as it was not needed after booting which was correct at that time.
However with the introduction of ACPI LPI support, this will be used
from cpuhotplug path in ACPI processor driver.
This patch drops the __init annotation from arm_cpuidle_init to avoid
the following warning:
WARNING: vmlinux.o(.text+0x113c8): 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.
Cc: Mark Rutland <mark.rutland@arm.com>
Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
arch/arm64/kernel/cpuidle.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/kernel/cpuidle.c b/arch/arm64/kernel/cpuidle.c
index e11857fce05f..06786fdaadeb 100644
--- a/arch/arm64/kernel/cpuidle.c
+++ b/arch/arm64/kernel/cpuidle.c
@@ -15,7 +15,7 @@
#include <asm/cpuidle.h>
#include <asm/cpu_ops.h>
-int __init arm_cpuidle_init(unsigned int cpu)
+int arm_cpuidle_init(unsigned int cpu)
{
int ret = -EOPNOTSUPP;
--
2.7.4
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v7 3/6] arm64: cpuidle: drop __init section marker to arm_cpuidle_init
@ 2016-06-28 13:55 ` Sudeep Holla
0 siblings, 0 replies; 37+ messages in thread
From: Sudeep Holla @ 2016-06-28 13:55 UTC (permalink / raw)
To: linux-arm-kernel
Commit ea389daa7fd9 ("arm64: cpuidle: add __init section marker to
arm_cpuidle_init") added the __init annotation to arm_cpuidle_init
as it was not needed after booting which was correct at that time.
However with the introduction of ACPI LPI support, this will be used
from cpuhotplug path in ACPI processor driver.
This patch drops the __init annotation from arm_cpuidle_init to avoid
the following warning:
WARNING: vmlinux.o(.text+0x113c8): 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.
Cc: Mark Rutland <mark.rutland@arm.com>
Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
arch/arm64/kernel/cpuidle.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/kernel/cpuidle.c b/arch/arm64/kernel/cpuidle.c
index e11857fce05f..06786fdaadeb 100644
--- a/arch/arm64/kernel/cpuidle.c
+++ b/arch/arm64/kernel/cpuidle.c
@@ -15,7 +15,7 @@
#include <asm/cpuidle.h>
#include <asm/cpu_ops.h>
-int __init arm_cpuidle_init(unsigned int cpu)
+int arm_cpuidle_init(unsigned int cpu)
{
int ret = -EOPNOTSUPP;
--
2.7.4
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH v7 4/6] cpuidle: introduce HAVE_GENERIC_CPUIDLE_ENTER for ARM{32,64} platforms
2016-06-28 13:55 ` Sudeep Holla
@ 2016-06-28 13:55 ` Sudeep Holla
-1 siblings, 0 replies; 37+ messages in thread
From: Sudeep Holla @ 2016-06-28 13:55 UTC (permalink / raw)
To: linux-acpi, Rafael J . Wysocki
Cc: Sudeep Holla, Vikas Sajjan, Sunil, Lorenzo Pieralisi,
PrashanthPrakash, Al Stone, Ashwin Chaugule, Daniel Lezcano,
linux-kernel, linux-arm-kernel
The function arm_enter_idle_state is exactly the same in both generic
ARM{32,64} CPUIdle driver and will be the same even on ARM64 backend
for ACPI processor idle driver. So we can unify it and move it as
generic_cpuidle_enter by introducing HAVE_GENERIC_CPUIDLE_ENTER and
enabling the same on both ARM{32,64}.
This is in preparation of reuse of the generic cpuidle entry function
for ACPI LPI support on ARM64.
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
arch/arm/Kconfig | 1 +
arch/arm/kernel/cpuidle.c | 4 ++--
arch/arm64/Kconfig | 1 +
arch/arm64/kernel/cpuidle.c | 6 +++---
drivers/cpuidle/Kconfig | 3 +++
drivers/cpuidle/cpuidle-arm.c | 21 +--------------------
drivers/cpuidle/cpuidle.c | 35 +++++++++++++++++++++++++++++++++++
include/linux/cpuidle.h | 8 ++++++++
8 files changed, 54 insertions(+), 25 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 90542db1220d..52b3dca0381c 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -54,6 +54,7 @@ config ARM
select HAVE_FTRACE_MCOUNT_RECORD if (!XIP_KERNEL)
select HAVE_FUNCTION_GRAPH_TRACER if (!THUMB2_KERNEL)
select HAVE_FUNCTION_TRACER if (!XIP_KERNEL)
+ select HAVE_GENERIC_CPUIDLE_ENTER
select HAVE_GENERIC_DMA_COHERENT
select HAVE_HW_BREAKPOINT if (PERF_EVENTS && (CPU_V6 || CPU_V6K || CPU_V7))
select HAVE_IDE if PCI || ISA || PCMCIA
diff --git a/arch/arm/kernel/cpuidle.c b/arch/arm/kernel/cpuidle.c
index a44b268e12e1..49704e333bfc 100644
--- a/arch/arm/kernel/cpuidle.c
+++ b/arch/arm/kernel/cpuidle.c
@@ -41,7 +41,7 @@ int arm_cpuidle_simple_enter(struct cpuidle_device *dev,
}
/**
- * arm_cpuidle_suspend() - function to enter low power idle states
+ * cpuidle_generic_enter() - function to enter low power idle states
* @index: an integer used as an identifier for the low level PM callbacks
*
* This function calls the underlying arch specific low level PM code as
@@ -50,7 +50,7 @@ int arm_cpuidle_simple_enter(struct cpuidle_device *dev,
* Returns -EOPNOTSUPP if no suspend callback is defined, the result of the
* callback otherwise.
*/
-int arm_cpuidle_suspend(int index)
+int cpuidle_generic_enter(int index)
{
int ret = -EOPNOTSUPP;
int cpu = smp_processor_id();
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 5a0a691d4220..0916b6e6c8ef 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -76,6 +76,7 @@ config ARM64
select HAVE_FTRACE_MCOUNT_RECORD
select HAVE_FUNCTION_TRACER
select HAVE_FUNCTION_GRAPH_TRACER
+ select HAVE_GENERIC_CPUIDLE_ENTER
select HAVE_GENERIC_DMA_COHERENT
select HAVE_HW_BREAKPOINT if PERF_EVENTS
select HAVE_IRQ_TIME_ACCOUNTING
diff --git a/arch/arm64/kernel/cpuidle.c b/arch/arm64/kernel/cpuidle.c
index 06786fdaadeb..bade1b04ff12 100644
--- a/arch/arm64/kernel/cpuidle.c
+++ b/arch/arm64/kernel/cpuidle.c
@@ -9,10 +9,10 @@
* published by the Free Software Foundation.
*/
+#include <linux/cpuidle.h>
#include <linux/of.h>
#include <linux/of_device.h>
-#include <asm/cpuidle.h>
#include <asm/cpu_ops.h>
int arm_cpuidle_init(unsigned int cpu)
@@ -27,13 +27,13 @@ int arm_cpuidle_init(unsigned int cpu)
}
/**
- * cpu_suspend() - function to enter a low-power idle state
+ * cpuidle_generic_enter() - function to enter a low-power idle state
* @arg: argument to pass to CPU suspend operations
*
* Return: 0 on success, -EOPNOTSUPP if CPU suspend hook not initialized, CPU
* operations back-end error code otherwise.
*/
-int arm_cpuidle_suspend(int index)
+int cpuidle_generic_enter(int index)
{
int cpu = smp_processor_id();
diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig
index 7e48eb5bf0a7..e557289cff90 100644
--- a/drivers/cpuidle/Kconfig
+++ b/drivers/cpuidle/Kconfig
@@ -45,4 +45,7 @@ endif
config ARCH_NEEDS_CPU_IDLE_COUPLED
def_bool n
+
+config HAVE_GENERIC_CPUIDLE_ENTER
+ def_bool n
endmenu
diff --git a/drivers/cpuidle/cpuidle-arm.c b/drivers/cpuidle/cpuidle-arm.c
index e342565e8715..13012d8eba53 100644
--- a/drivers/cpuidle/cpuidle-arm.c
+++ b/drivers/cpuidle/cpuidle-arm.c
@@ -36,26 +36,7 @@
static int arm_enter_idle_state(struct cpuidle_device *dev,
struct cpuidle_driver *drv, int idx)
{
- int ret;
-
- if (!idx) {
- cpu_do_idle();
- return idx;
- }
-
- 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;
+ return cpuidle_generic_enter_state(idx);
}
static struct cpuidle_driver arm_idle_driver = {
diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index a4d0059e232c..e490dea98b89 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -16,6 +16,7 @@
#include <linux/pm_qos.h>
#include <linux/cpu.h>
#include <linux/cpuidle.h>
+#include <linux/cpu_pm.h>
#include <linux/ktime.h>
#include <linux/hrtimer.h>
#include <linux/module.h>
@@ -255,6 +256,40 @@ int cpuidle_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
}
/**
+ * cpuidle_generic_enter_state - enter into the specified idle state using the
+ * generic callback if implemented
+ *
+ * @index: the index in the idle state table
+ *
+ * Returns the index in the idle state, -1 in case of error.
+ */
+int cpuidle_generic_enter_state(int idx)
+{
+ int ret;
+
+ if (!idx) {
+ cpu_do_idle();
+ return idx;
+ }
+
+ 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 = cpuidle_generic_enter(idx);
+
+ cpu_pm_exit();
+ }
+
+ return ret ? -1 : idx;
+}
+
+EXPORT_SYMBOL_GPL(cpuidle_generic_enter_state);
+
+/**
* cpuidle_enter - enter into the specified idle state
*
* @drv: the cpuidle driver tied with the cpu
diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index 07b83d32f66c..9fd3c10166fa 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -154,6 +154,7 @@ extern int cpuidle_play_dead(void);
extern struct cpuidle_driver *cpuidle_get_cpu_driver(struct cpuidle_device *dev);
static inline struct cpuidle_device *cpuidle_get_device(void)
{return __this_cpu_read(cpuidle_devices); }
+extern int cpuidle_generic_enter_state(int idx);
#else
static inline void disable_cpuidle(void) { }
static inline bool cpuidle_not_available(struct cpuidle_driver *drv,
@@ -190,6 +191,7 @@ static inline int cpuidle_play_dead(void) {return -ENODEV; }
static inline struct cpuidle_driver *cpuidle_get_cpu_driver(
struct cpuidle_device *dev) {return NULL; }
static inline struct cpuidle_device *cpuidle_get_device(void) {return NULL; }
+static inline int cpuidle_generic_enter_state(int idx) { return -1; }
#endif
#if defined(CONFIG_CPU_IDLE) && defined(CONFIG_SUSPEND)
@@ -246,6 +248,12 @@ static inline int cpuidle_register_governor(struct cpuidle_governor *gov)
{return 0;}
#endif
+#ifdef CONFIG_HAVE_GENERIC_CPUIDLE_ENTER
+extern int cpuidle_generic_enter(int idx);
+#else
+static inline int cpuidle_generic_enter(int idx) { return -1; }
+#endif
+
#ifdef CONFIG_ARCH_HAS_CPU_RELAX
#define CPUIDLE_DRIVER_STATE_START 1
#else
--
2.7.4
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v7 4/6] cpuidle: introduce HAVE_GENERIC_CPUIDLE_ENTER for ARM{32, 64} platforms
@ 2016-06-28 13:55 ` Sudeep Holla
0 siblings, 0 replies; 37+ messages in thread
From: Sudeep Holla @ 2016-06-28 13:55 UTC (permalink / raw)
To: linux-arm-kernel
The function arm_enter_idle_state is exactly the same in both generic
ARM{32,64} CPUIdle driver and will be the same even on ARM64 backend
for ACPI processor idle driver. So we can unify it and move it as
generic_cpuidle_enter by introducing HAVE_GENERIC_CPUIDLE_ENTER and
enabling the same on both ARM{32,64}.
This is in preparation of reuse of the generic cpuidle entry function
for ACPI LPI support on ARM64.
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
arch/arm/Kconfig | 1 +
arch/arm/kernel/cpuidle.c | 4 ++--
arch/arm64/Kconfig | 1 +
arch/arm64/kernel/cpuidle.c | 6 +++---
drivers/cpuidle/Kconfig | 3 +++
drivers/cpuidle/cpuidle-arm.c | 21 +--------------------
drivers/cpuidle/cpuidle.c | 35 +++++++++++++++++++++++++++++++++++
include/linux/cpuidle.h | 8 ++++++++
8 files changed, 54 insertions(+), 25 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 90542db1220d..52b3dca0381c 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -54,6 +54,7 @@ config ARM
select HAVE_FTRACE_MCOUNT_RECORD if (!XIP_KERNEL)
select HAVE_FUNCTION_GRAPH_TRACER if (!THUMB2_KERNEL)
select HAVE_FUNCTION_TRACER if (!XIP_KERNEL)
+ select HAVE_GENERIC_CPUIDLE_ENTER
select HAVE_GENERIC_DMA_COHERENT
select HAVE_HW_BREAKPOINT if (PERF_EVENTS && (CPU_V6 || CPU_V6K || CPU_V7))
select HAVE_IDE if PCI || ISA || PCMCIA
diff --git a/arch/arm/kernel/cpuidle.c b/arch/arm/kernel/cpuidle.c
index a44b268e12e1..49704e333bfc 100644
--- a/arch/arm/kernel/cpuidle.c
+++ b/arch/arm/kernel/cpuidle.c
@@ -41,7 +41,7 @@ int arm_cpuidle_simple_enter(struct cpuidle_device *dev,
}
/**
- * arm_cpuidle_suspend() - function to enter low power idle states
+ * cpuidle_generic_enter() - function to enter low power idle states
* @index: an integer used as an identifier for the low level PM callbacks
*
* This function calls the underlying arch specific low level PM code as
@@ -50,7 +50,7 @@ int arm_cpuidle_simple_enter(struct cpuidle_device *dev,
* Returns -EOPNOTSUPP if no suspend callback is defined, the result of the
* callback otherwise.
*/
-int arm_cpuidle_suspend(int index)
+int cpuidle_generic_enter(int index)
{
int ret = -EOPNOTSUPP;
int cpu = smp_processor_id();
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 5a0a691d4220..0916b6e6c8ef 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -76,6 +76,7 @@ config ARM64
select HAVE_FTRACE_MCOUNT_RECORD
select HAVE_FUNCTION_TRACER
select HAVE_FUNCTION_GRAPH_TRACER
+ select HAVE_GENERIC_CPUIDLE_ENTER
select HAVE_GENERIC_DMA_COHERENT
select HAVE_HW_BREAKPOINT if PERF_EVENTS
select HAVE_IRQ_TIME_ACCOUNTING
diff --git a/arch/arm64/kernel/cpuidle.c b/arch/arm64/kernel/cpuidle.c
index 06786fdaadeb..bade1b04ff12 100644
--- a/arch/arm64/kernel/cpuidle.c
+++ b/arch/arm64/kernel/cpuidle.c
@@ -9,10 +9,10 @@
* published by the Free Software Foundation.
*/
+#include <linux/cpuidle.h>
#include <linux/of.h>
#include <linux/of_device.h>
-#include <asm/cpuidle.h>
#include <asm/cpu_ops.h>
int arm_cpuidle_init(unsigned int cpu)
@@ -27,13 +27,13 @@ int arm_cpuidle_init(unsigned int cpu)
}
/**
- * cpu_suspend() - function to enter a low-power idle state
+ * cpuidle_generic_enter() - function to enter a low-power idle state
* @arg: argument to pass to CPU suspend operations
*
* Return: 0 on success, -EOPNOTSUPP if CPU suspend hook not initialized, CPU
* operations back-end error code otherwise.
*/
-int arm_cpuidle_suspend(int index)
+int cpuidle_generic_enter(int index)
{
int cpu = smp_processor_id();
diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig
index 7e48eb5bf0a7..e557289cff90 100644
--- a/drivers/cpuidle/Kconfig
+++ b/drivers/cpuidle/Kconfig
@@ -45,4 +45,7 @@ endif
config ARCH_NEEDS_CPU_IDLE_COUPLED
def_bool n
+
+config HAVE_GENERIC_CPUIDLE_ENTER
+ def_bool n
endmenu
diff --git a/drivers/cpuidle/cpuidle-arm.c b/drivers/cpuidle/cpuidle-arm.c
index e342565e8715..13012d8eba53 100644
--- a/drivers/cpuidle/cpuidle-arm.c
+++ b/drivers/cpuidle/cpuidle-arm.c
@@ -36,26 +36,7 @@
static int arm_enter_idle_state(struct cpuidle_device *dev,
struct cpuidle_driver *drv, int idx)
{
- int ret;
-
- if (!idx) {
- cpu_do_idle();
- return idx;
- }
-
- 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;
+ return cpuidle_generic_enter_state(idx);
}
static struct cpuidle_driver arm_idle_driver = {
diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index a4d0059e232c..e490dea98b89 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -16,6 +16,7 @@
#include <linux/pm_qos.h>
#include <linux/cpu.h>
#include <linux/cpuidle.h>
+#include <linux/cpu_pm.h>
#include <linux/ktime.h>
#include <linux/hrtimer.h>
#include <linux/module.h>
@@ -255,6 +256,40 @@ int cpuidle_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
}
/**
+ * cpuidle_generic_enter_state - enter into the specified idle state using the
+ * generic callback if implemented
+ *
+ * @index: the index in the idle state table
+ *
+ * Returns the index in the idle state, -1 in case of error.
+ */
+int cpuidle_generic_enter_state(int idx)
+{
+ int ret;
+
+ if (!idx) {
+ cpu_do_idle();
+ return idx;
+ }
+
+ 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 = cpuidle_generic_enter(idx);
+
+ cpu_pm_exit();
+ }
+
+ return ret ? -1 : idx;
+}
+
+EXPORT_SYMBOL_GPL(cpuidle_generic_enter_state);
+
+/**
* cpuidle_enter - enter into the specified idle state
*
* @drv: the cpuidle driver tied with the cpu
diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index 07b83d32f66c..9fd3c10166fa 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -154,6 +154,7 @@ extern int cpuidle_play_dead(void);
extern struct cpuidle_driver *cpuidle_get_cpu_driver(struct cpuidle_device *dev);
static inline struct cpuidle_device *cpuidle_get_device(void)
{return __this_cpu_read(cpuidle_devices); }
+extern int cpuidle_generic_enter_state(int idx);
#else
static inline void disable_cpuidle(void) { }
static inline bool cpuidle_not_available(struct cpuidle_driver *drv,
@@ -190,6 +191,7 @@ static inline int cpuidle_play_dead(void) {return -ENODEV; }
static inline struct cpuidle_driver *cpuidle_get_cpu_driver(
struct cpuidle_device *dev) {return NULL; }
static inline struct cpuidle_device *cpuidle_get_device(void) {return NULL; }
+static inline int cpuidle_generic_enter_state(int idx) { return -1; }
#endif
#if defined(CONFIG_CPU_IDLE) && defined(CONFIG_SUSPEND)
@@ -246,6 +248,12 @@ static inline int cpuidle_register_governor(struct cpuidle_governor *gov)
{return 0;}
#endif
+#ifdef CONFIG_HAVE_GENERIC_CPUIDLE_ENTER
+extern int cpuidle_generic_enter(int idx);
+#else
+static inline int cpuidle_generic_enter(int idx) { return -1; }
+#endif
+
#ifdef CONFIG_ARCH_HAS_CPU_RELAX
#define CPUIDLE_DRIVER_STATE_START 1
#else
--
2.7.4
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v7 4/6] cpuidle: introduce HAVE_GENERIC_CPUIDLE_ENTER for ARM{32,64} platforms
2016-06-28 13:55 ` [PATCH v7 4/6] cpuidle: introduce HAVE_GENERIC_CPUIDLE_ENTER for ARM{32, 64} platforms Sudeep Holla
@ 2016-07-07 13:21 ` Rafael J. Wysocki
-1 siblings, 0 replies; 37+ messages in thread
From: Rafael J. Wysocki @ 2016-07-07 13:21 UTC (permalink / raw)
To: Sudeep Holla
Cc: linux-acpi, Vikas Sajjan, Sunil, Lorenzo Pieralisi,
PrashanthPrakash, Al Stone, Ashwin Chaugule, Daniel Lezcano,
linux-kernel, linux-arm-kernel
On Tuesday, June 28, 2016 02:55:50 PM Sudeep Holla wrote:
> The function arm_enter_idle_state is exactly the same in both generic
> ARM{32,64} CPUIdle driver and will be the same even on ARM64 backend
> for ACPI processor idle driver. So we can unify it and move it as
> generic_cpuidle_enter by introducing HAVE_GENERIC_CPUIDLE_ENTER and
> enabling the same on both ARM{32,64}.
>
> This is in preparation of reuse of the generic cpuidle entry function
> for ACPI LPI support on ARM64.
>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> arch/arm/Kconfig | 1 +
> arch/arm/kernel/cpuidle.c | 4 ++--
> arch/arm64/Kconfig | 1 +
> arch/arm64/kernel/cpuidle.c | 6 +++---
> drivers/cpuidle/Kconfig | 3 +++
> drivers/cpuidle/cpuidle-arm.c | 21 +--------------------
> drivers/cpuidle/cpuidle.c | 35 +++++++++++++++++++++++++++++++++++
> include/linux/cpuidle.h | 8 ++++++++
> 8 files changed, 54 insertions(+), 25 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 90542db1220d..52b3dca0381c 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -54,6 +54,7 @@ config ARM
> select HAVE_FTRACE_MCOUNT_RECORD if (!XIP_KERNEL)
> select HAVE_FUNCTION_GRAPH_TRACER if (!THUMB2_KERNEL)
> select HAVE_FUNCTION_TRACER if (!XIP_KERNEL)
> + select HAVE_GENERIC_CPUIDLE_ENTER
That "generic" part in the name concerns me a bit, because the thing is not
really generic. It is "common on ARM" rather.
> select HAVE_GENERIC_DMA_COHERENT
> select HAVE_HW_BREAKPOINT if (PERF_EVENTS && (CPU_V6 || CPU_V6K || CPU_V7))
> select HAVE_IDE if PCI || ISA || PCMCIA
> diff --git a/arch/arm/kernel/cpuidle.c b/arch/arm/kernel/cpuidle.c
> index a44b268e12e1..49704e333bfc 100644
> --- a/arch/arm/kernel/cpuidle.c
> +++ b/arch/arm/kernel/cpuidle.c
> @@ -41,7 +41,7 @@ int arm_cpuidle_simple_enter(struct cpuidle_device *dev,
> }
>
> /**
> - * arm_cpuidle_suspend() - function to enter low power idle states
> + * cpuidle_generic_enter() - function to enter low power idle states
> * @index: an integer used as an identifier for the low level PM callbacks
> *
> * This function calls the underlying arch specific low level PM code as
> @@ -50,7 +50,7 @@ int arm_cpuidle_simple_enter(struct cpuidle_device *dev,
> * Returns -EOPNOTSUPP if no suspend callback is defined, the result of the
> * callback otherwise.
> */
> -int arm_cpuidle_suspend(int index)
> +int cpuidle_generic_enter(int index)
> {
> int ret = -EOPNOTSUPP;
> int cpu = smp_processor_id();
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 5a0a691d4220..0916b6e6c8ef 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -76,6 +76,7 @@ config ARM64
> select HAVE_FTRACE_MCOUNT_RECORD
> select HAVE_FUNCTION_TRACER
> select HAVE_FUNCTION_GRAPH_TRACER
> + select HAVE_GENERIC_CPUIDLE_ENTER
> select HAVE_GENERIC_DMA_COHERENT
> select HAVE_HW_BREAKPOINT if PERF_EVENTS
> select HAVE_IRQ_TIME_ACCOUNTING
> diff --git a/arch/arm64/kernel/cpuidle.c b/arch/arm64/kernel/cpuidle.c
> index 06786fdaadeb..bade1b04ff12 100644
> --- a/arch/arm64/kernel/cpuidle.c
> +++ b/arch/arm64/kernel/cpuidle.c
> @@ -9,10 +9,10 @@
> * published by the Free Software Foundation.
> */
>
> +#include <linux/cpuidle.h>
> #include <linux/of.h>
> #include <linux/of_device.h>
>
> -#include <asm/cpuidle.h>
> #include <asm/cpu_ops.h>
>
> int arm_cpuidle_init(unsigned int cpu)
> @@ -27,13 +27,13 @@ int arm_cpuidle_init(unsigned int cpu)
> }
>
> /**
> - * cpu_suspend() - function to enter a low-power idle state
> + * cpuidle_generic_enter() - function to enter a low-power idle state
> * @arg: argument to pass to CPU suspend operations
> *
> * Return: 0 on success, -EOPNOTSUPP if CPU suspend hook not initialized, CPU
> * operations back-end error code otherwise.
> */
> -int arm_cpuidle_suspend(int index)
> +int cpuidle_generic_enter(int index)
> {
> int cpu = smp_processor_id();
>
> diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig
> index 7e48eb5bf0a7..e557289cff90 100644
> --- a/drivers/cpuidle/Kconfig
> +++ b/drivers/cpuidle/Kconfig
> @@ -45,4 +45,7 @@ endif
>
> config ARCH_NEEDS_CPU_IDLE_COUPLED
> def_bool n
> +
> +config HAVE_GENERIC_CPUIDLE_ENTER
> + def_bool n
> endmenu
> diff --git a/drivers/cpuidle/cpuidle-arm.c b/drivers/cpuidle/cpuidle-arm.c
> index e342565e8715..13012d8eba53 100644
> --- a/drivers/cpuidle/cpuidle-arm.c
> +++ b/drivers/cpuidle/cpuidle-arm.c
> @@ -36,26 +36,7 @@
> static int arm_enter_idle_state(struct cpuidle_device *dev,
> struct cpuidle_driver *drv, int idx)
> {
> - int ret;
> -
> - if (!idx) {
> - cpu_do_idle();
> - return idx;
> - }
> -
> - 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;
> + return cpuidle_generic_enter_state(idx);
> }
>
> static struct cpuidle_driver arm_idle_driver = {
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index a4d0059e232c..e490dea98b89 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -16,6 +16,7 @@
> #include <linux/pm_qos.h>
> #include <linux/cpu.h>
> #include <linux/cpuidle.h>
> +#include <linux/cpu_pm.h>
> #include <linux/ktime.h>
> #include <linux/hrtimer.h>
> #include <linux/module.h>
> @@ -255,6 +256,40 @@ int cpuidle_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
> }
>
> /**
> + * cpuidle_generic_enter_state - enter into the specified idle state using the
> + * generic callback if implemented
> + *
> + * @index: the index in the idle state table
> + *
> + * Returns the index in the idle state, -1 in case of error.
> + */
> +int cpuidle_generic_enter_state(int idx)
> +{
> + int ret;
> +
> + if (!idx) {
> + cpu_do_idle();
> + return idx;
> + }
> +
> + ret = cpu_pm_enter();
If you look at the users of cpu_pm_enter(), they are all ARM. Nobody else uses
it, so at least please put cpuidle_generic_enter_state() under a #ifdef to avoid
building it in vain on non-ARM.
> + 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 = cpuidle_generic_enter(idx);
> +
> + cpu_pm_exit();
> + }
> +
> + return ret ? -1 : idx;
> +}
> +
> +EXPORT_SYMBOL_GPL(cpuidle_generic_enter_state);
> +
> +/**
> * cpuidle_enter - enter into the specified idle state
> *
> * @drv: the cpuidle driver tied with the cpu
> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
> index 07b83d32f66c..9fd3c10166fa 100644
> --- a/include/linux/cpuidle.h
> +++ b/include/linux/cpuidle.h
> @@ -154,6 +154,7 @@ extern int cpuidle_play_dead(void);
> extern struct cpuidle_driver *cpuidle_get_cpu_driver(struct cpuidle_device *dev);
> static inline struct cpuidle_device *cpuidle_get_device(void)
> {return __this_cpu_read(cpuidle_devices); }
> +extern int cpuidle_generic_enter_state(int idx);
> #else
> static inline void disable_cpuidle(void) { }
> static inline bool cpuidle_not_available(struct cpuidle_driver *drv,
> @@ -190,6 +191,7 @@ static inline int cpuidle_play_dead(void) {return -ENODEV; }
> static inline struct cpuidle_driver *cpuidle_get_cpu_driver(
> struct cpuidle_device *dev) {return NULL; }
> static inline struct cpuidle_device *cpuidle_get_device(void) {return NULL; }
> +static inline int cpuidle_generic_enter_state(int idx) { return -1; }
> #endif
>
> #if defined(CONFIG_CPU_IDLE) && defined(CONFIG_SUSPEND)
> @@ -246,6 +248,12 @@ static inline int cpuidle_register_governor(struct cpuidle_governor *gov)
> {return 0;}
> #endif
>
> +#ifdef CONFIG_HAVE_GENERIC_CPUIDLE_ENTER
> +extern int cpuidle_generic_enter(int idx);
And if you put it under the #ifdef here, its definition also should go under
the same #ifdef.
> +#else
> +static inline int cpuidle_generic_enter(int idx) { return -1; }
> +#endif
> +
> #ifdef CONFIG_ARCH_HAS_CPU_RELAX
> #define CPUIDLE_DRIVER_STATE_START 1
> #else
>
Thanks,
Rafael
^ permalink raw reply [flat|nested] 37+ messages in thread* [PATCH v7 4/6] cpuidle: introduce HAVE_GENERIC_CPUIDLE_ENTER for ARM{32, 64} platforms
@ 2016-07-07 13:21 ` Rafael J. Wysocki
0 siblings, 0 replies; 37+ messages in thread
From: Rafael J. Wysocki @ 2016-07-07 13:21 UTC (permalink / raw)
To: linux-arm-kernel
On Tuesday, June 28, 2016 02:55:50 PM Sudeep Holla wrote:
> The function arm_enter_idle_state is exactly the same in both generic
> ARM{32,64} CPUIdle driver and will be the same even on ARM64 backend
> for ACPI processor idle driver. So we can unify it and move it as
> generic_cpuidle_enter by introducing HAVE_GENERIC_CPUIDLE_ENTER and
> enabling the same on both ARM{32,64}.
>
> This is in preparation of reuse of the generic cpuidle entry function
> for ACPI LPI support on ARM64.
>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> arch/arm/Kconfig | 1 +
> arch/arm/kernel/cpuidle.c | 4 ++--
> arch/arm64/Kconfig | 1 +
> arch/arm64/kernel/cpuidle.c | 6 +++---
> drivers/cpuidle/Kconfig | 3 +++
> drivers/cpuidle/cpuidle-arm.c | 21 +--------------------
> drivers/cpuidle/cpuidle.c | 35 +++++++++++++++++++++++++++++++++++
> include/linux/cpuidle.h | 8 ++++++++
> 8 files changed, 54 insertions(+), 25 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 90542db1220d..52b3dca0381c 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -54,6 +54,7 @@ config ARM
> select HAVE_FTRACE_MCOUNT_RECORD if (!XIP_KERNEL)
> select HAVE_FUNCTION_GRAPH_TRACER if (!THUMB2_KERNEL)
> select HAVE_FUNCTION_TRACER if (!XIP_KERNEL)
> + select HAVE_GENERIC_CPUIDLE_ENTER
That "generic" part in the name concerns me a bit, because the thing is not
really generic. It is "common on ARM" rather.
> select HAVE_GENERIC_DMA_COHERENT
> select HAVE_HW_BREAKPOINT if (PERF_EVENTS && (CPU_V6 || CPU_V6K || CPU_V7))
> select HAVE_IDE if PCI || ISA || PCMCIA
> diff --git a/arch/arm/kernel/cpuidle.c b/arch/arm/kernel/cpuidle.c
> index a44b268e12e1..49704e333bfc 100644
> --- a/arch/arm/kernel/cpuidle.c
> +++ b/arch/arm/kernel/cpuidle.c
> @@ -41,7 +41,7 @@ int arm_cpuidle_simple_enter(struct cpuidle_device *dev,
> }
>
> /**
> - * arm_cpuidle_suspend() - function to enter low power idle states
> + * cpuidle_generic_enter() - function to enter low power idle states
> * @index: an integer used as an identifier for the low level PM callbacks
> *
> * This function calls the underlying arch specific low level PM code as
> @@ -50,7 +50,7 @@ int arm_cpuidle_simple_enter(struct cpuidle_device *dev,
> * Returns -EOPNOTSUPP if no suspend callback is defined, the result of the
> * callback otherwise.
> */
> -int arm_cpuidle_suspend(int index)
> +int cpuidle_generic_enter(int index)
> {
> int ret = -EOPNOTSUPP;
> int cpu = smp_processor_id();
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 5a0a691d4220..0916b6e6c8ef 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -76,6 +76,7 @@ config ARM64
> select HAVE_FTRACE_MCOUNT_RECORD
> select HAVE_FUNCTION_TRACER
> select HAVE_FUNCTION_GRAPH_TRACER
> + select HAVE_GENERIC_CPUIDLE_ENTER
> select HAVE_GENERIC_DMA_COHERENT
> select HAVE_HW_BREAKPOINT if PERF_EVENTS
> select HAVE_IRQ_TIME_ACCOUNTING
> diff --git a/arch/arm64/kernel/cpuidle.c b/arch/arm64/kernel/cpuidle.c
> index 06786fdaadeb..bade1b04ff12 100644
> --- a/arch/arm64/kernel/cpuidle.c
> +++ b/arch/arm64/kernel/cpuidle.c
> @@ -9,10 +9,10 @@
> * published by the Free Software Foundation.
> */
>
> +#include <linux/cpuidle.h>
> #include <linux/of.h>
> #include <linux/of_device.h>
>
> -#include <asm/cpuidle.h>
> #include <asm/cpu_ops.h>
>
> int arm_cpuidle_init(unsigned int cpu)
> @@ -27,13 +27,13 @@ int arm_cpuidle_init(unsigned int cpu)
> }
>
> /**
> - * cpu_suspend() - function to enter a low-power idle state
> + * cpuidle_generic_enter() - function to enter a low-power idle state
> * @arg: argument to pass to CPU suspend operations
> *
> * Return: 0 on success, -EOPNOTSUPP if CPU suspend hook not initialized, CPU
> * operations back-end error code otherwise.
> */
> -int arm_cpuidle_suspend(int index)
> +int cpuidle_generic_enter(int index)
> {
> int cpu = smp_processor_id();
>
> diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig
> index 7e48eb5bf0a7..e557289cff90 100644
> --- a/drivers/cpuidle/Kconfig
> +++ b/drivers/cpuidle/Kconfig
> @@ -45,4 +45,7 @@ endif
>
> config ARCH_NEEDS_CPU_IDLE_COUPLED
> def_bool n
> +
> +config HAVE_GENERIC_CPUIDLE_ENTER
> + def_bool n
> endmenu
> diff --git a/drivers/cpuidle/cpuidle-arm.c b/drivers/cpuidle/cpuidle-arm.c
> index e342565e8715..13012d8eba53 100644
> --- a/drivers/cpuidle/cpuidle-arm.c
> +++ b/drivers/cpuidle/cpuidle-arm.c
> @@ -36,26 +36,7 @@
> static int arm_enter_idle_state(struct cpuidle_device *dev,
> struct cpuidle_driver *drv, int idx)
> {
> - int ret;
> -
> - if (!idx) {
> - cpu_do_idle();
> - return idx;
> - }
> -
> - 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;
> + return cpuidle_generic_enter_state(idx);
> }
>
> static struct cpuidle_driver arm_idle_driver = {
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index a4d0059e232c..e490dea98b89 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -16,6 +16,7 @@
> #include <linux/pm_qos.h>
> #include <linux/cpu.h>
> #include <linux/cpuidle.h>
> +#include <linux/cpu_pm.h>
> #include <linux/ktime.h>
> #include <linux/hrtimer.h>
> #include <linux/module.h>
> @@ -255,6 +256,40 @@ int cpuidle_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
> }
>
> /**
> + * cpuidle_generic_enter_state - enter into the specified idle state using the
> + * generic callback if implemented
> + *
> + * @index: the index in the idle state table
> + *
> + * Returns the index in the idle state, -1 in case of error.
> + */
> +int cpuidle_generic_enter_state(int idx)
> +{
> + int ret;
> +
> + if (!idx) {
> + cpu_do_idle();
> + return idx;
> + }
> +
> + ret = cpu_pm_enter();
If you look at the users of cpu_pm_enter(), they are all ARM. Nobody else uses
it, so at least please put cpuidle_generic_enter_state() under a #ifdef to avoid
building it in vain on non-ARM.
> + 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 = cpuidle_generic_enter(idx);
> +
> + cpu_pm_exit();
> + }
> +
> + return ret ? -1 : idx;
> +}
> +
> +EXPORT_SYMBOL_GPL(cpuidle_generic_enter_state);
> +
> +/**
> * cpuidle_enter - enter into the specified idle state
> *
> * @drv: the cpuidle driver tied with the cpu
> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
> index 07b83d32f66c..9fd3c10166fa 100644
> --- a/include/linux/cpuidle.h
> +++ b/include/linux/cpuidle.h
> @@ -154,6 +154,7 @@ extern int cpuidle_play_dead(void);
> extern struct cpuidle_driver *cpuidle_get_cpu_driver(struct cpuidle_device *dev);
> static inline struct cpuidle_device *cpuidle_get_device(void)
> {return __this_cpu_read(cpuidle_devices); }
> +extern int cpuidle_generic_enter_state(int idx);
> #else
> static inline void disable_cpuidle(void) { }
> static inline bool cpuidle_not_available(struct cpuidle_driver *drv,
> @@ -190,6 +191,7 @@ static inline int cpuidle_play_dead(void) {return -ENODEV; }
> static inline struct cpuidle_driver *cpuidle_get_cpu_driver(
> struct cpuidle_device *dev) {return NULL; }
> static inline struct cpuidle_device *cpuidle_get_device(void) {return NULL; }
> +static inline int cpuidle_generic_enter_state(int idx) { return -1; }
> #endif
>
> #if defined(CONFIG_CPU_IDLE) && defined(CONFIG_SUSPEND)
> @@ -246,6 +248,12 @@ static inline int cpuidle_register_governor(struct cpuidle_governor *gov)
> {return 0;}
> #endif
>
> +#ifdef CONFIG_HAVE_GENERIC_CPUIDLE_ENTER
> +extern int cpuidle_generic_enter(int idx);
And if you put it under the #ifdef here, its definition also should go under
the same #ifdef.
> +#else
> +static inline int cpuidle_generic_enter(int idx) { return -1; }
> +#endif
> +
> #ifdef CONFIG_ARCH_HAS_CPU_RELAX
> #define CPUIDLE_DRIVER_STATE_START 1
> #else
>
Thanks,
Rafael
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH v7 4/6] cpuidle: introduce HAVE_GENERIC_CPUIDLE_ENTER for ARM{32,64} platforms
2016-07-07 13:21 ` [PATCH v7 4/6] cpuidle: introduce HAVE_GENERIC_CPUIDLE_ENTER for ARM{32, 64} platforms Rafael J. Wysocki
(?)
@ 2016-07-07 13:34 ` Sudeep Holla
-1 siblings, 0 replies; 37+ messages in thread
From: Sudeep Holla @ 2016-07-07 13:34 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Lorenzo Pieralisi, Al Stone, PrashanthPrakash, Vikas Sajjan,
Daniel Lezcano, linux-kernel, linux-acpi, Ashwin Chaugule,
Sudeep Holla, linux-arm-kernel, Sunil
On 07/07/16 14:21, Rafael J. Wysocki wrote:
> On Tuesday, June 28, 2016 02:55:50 PM Sudeep Holla wrote:
>> The function arm_enter_idle_state is exactly the same in both generic
>> ARM{32,64} CPUIdle driver and will be the same even on ARM64 backend
>> for ACPI processor idle driver. So we can unify it and move it as
>> generic_cpuidle_enter by introducing HAVE_GENERIC_CPUIDLE_ENTER and
>> enabling the same on both ARM{32,64}.
>>
>> This is in preparation of reuse of the generic cpuidle entry function
>> for ACPI LPI support on ARM64.
>>
>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>> ---
>> arch/arm/Kconfig | 1 +
>> arch/arm/kernel/cpuidle.c | 4 ++--
>> arch/arm64/Kconfig | 1 +
>> arch/arm64/kernel/cpuidle.c | 6 +++---
>> drivers/cpuidle/Kconfig | 3 +++
>> drivers/cpuidle/cpuidle-arm.c | 21 +--------------------
>> drivers/cpuidle/cpuidle.c | 35 +++++++++++++++++++++++++++++++++++
>> include/linux/cpuidle.h | 8 ++++++++
>> 8 files changed, 54 insertions(+), 25 deletions(-)
>>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index 90542db1220d..52b3dca0381c 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -54,6 +54,7 @@ config ARM
>> select HAVE_FTRACE_MCOUNT_RECORD if (!XIP_KERNEL)
>> select HAVE_FUNCTION_GRAPH_TRACER if (!THUMB2_KERNEL)
>> select HAVE_FUNCTION_TRACER if (!XIP_KERNEL)
>> + select HAVE_GENERIC_CPUIDLE_ENTER
>
> That "generic" part in the name concerns me a bit, because the thing is not
> really generic. It is "common on ARM" rather.
>
I agree and that's exactly what I told Daniel. It's rather just
*ARM Generic*. Any preference on the name ? I had it header file under
include/linu/cpuidle-arm.h in the previous version. Do you prefer that ?
[..]
>> @@ -255,6 +256,40 @@ int cpuidle_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
>> }
>>
>> /**
>> + * cpuidle_generic_enter_state - enter into the specified idle state using the
>> + * generic callback if implemented
>> + *
>> + * @index: the index in the idle state table
>> + *
>> + * Returns the index in the idle state, -1 in case of error.
>> + */
>> +int cpuidle_generic_enter_state(int idx)
>> +{
>> + int ret;
>> +
>> + if (!idx) {
>> + cpu_do_idle();
>> + return idx;
>> + }
>> +
>> + ret = cpu_pm_enter();
>
> If you look at the users of cpu_pm_enter(), they are all ARM. Nobody else uses
> it, so at least please put cpuidle_generic_enter_state() under a #ifdef to avoid
> building it in vain on non-ARM.
>
Agreed, that should be taken care once I do the changes you are
suggesting below.
[...]
>> @@ -246,6 +248,12 @@ static inline int cpuidle_register_governor(struct cpuidle_governor *gov)
>> {return 0;}
>> #endif
>>
>> +#ifdef CONFIG_HAVE_GENERIC_CPUIDLE_ENTER
>> +extern int cpuidle_generic_enter(int idx);
>
> And if you put it under the #ifdef here, its definition also should go under
> the same #ifdef.
>
Yes I will do that.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 37+ messages in thread* [PATCH v7 4/6] cpuidle: introduce HAVE_GENERIC_CPUIDLE_ENTER for ARM{32,64} platforms
@ 2016-07-07 13:34 ` Sudeep Holla
0 siblings, 0 replies; 37+ messages in thread
From: Sudeep Holla @ 2016-07-07 13:34 UTC (permalink / raw)
To: linux-arm-kernel
On 07/07/16 14:21, Rafael J. Wysocki wrote:
> On Tuesday, June 28, 2016 02:55:50 PM Sudeep Holla wrote:
>> The function arm_enter_idle_state is exactly the same in both generic
>> ARM{32,64} CPUIdle driver and will be the same even on ARM64 backend
>> for ACPI processor idle driver. So we can unify it and move it as
>> generic_cpuidle_enter by introducing HAVE_GENERIC_CPUIDLE_ENTER and
>> enabling the same on both ARM{32,64}.
>>
>> This is in preparation of reuse of the generic cpuidle entry function
>> for ACPI LPI support on ARM64.
>>
>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>> ---
>> arch/arm/Kconfig | 1 +
>> arch/arm/kernel/cpuidle.c | 4 ++--
>> arch/arm64/Kconfig | 1 +
>> arch/arm64/kernel/cpuidle.c | 6 +++---
>> drivers/cpuidle/Kconfig | 3 +++
>> drivers/cpuidle/cpuidle-arm.c | 21 +--------------------
>> drivers/cpuidle/cpuidle.c | 35 +++++++++++++++++++++++++++++++++++
>> include/linux/cpuidle.h | 8 ++++++++
>> 8 files changed, 54 insertions(+), 25 deletions(-)
>>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index 90542db1220d..52b3dca0381c 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -54,6 +54,7 @@ config ARM
>> select HAVE_FTRACE_MCOUNT_RECORD if (!XIP_KERNEL)
>> select HAVE_FUNCTION_GRAPH_TRACER if (!THUMB2_KERNEL)
>> select HAVE_FUNCTION_TRACER if (!XIP_KERNEL)
>> + select HAVE_GENERIC_CPUIDLE_ENTER
>
> That "generic" part in the name concerns me a bit, because the thing is not
> really generic. It is "common on ARM" rather.
>
I agree and that's exactly what I told Daniel. It's rather just
*ARM Generic*. Any preference on the name ? I had it header file under
include/linu/cpuidle-arm.h in the previous version. Do you prefer that ?
[..]
>> @@ -255,6 +256,40 @@ int cpuidle_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
>> }
>>
>> /**
>> + * cpuidle_generic_enter_state - enter into the specified idle state using the
>> + * generic callback if implemented
>> + *
>> + * @index: the index in the idle state table
>> + *
>> + * Returns the index in the idle state, -1 in case of error.
>> + */
>> +int cpuidle_generic_enter_state(int idx)
>> +{
>> + int ret;
>> +
>> + if (!idx) {
>> + cpu_do_idle();
>> + return idx;
>> + }
>> +
>> + ret = cpu_pm_enter();
>
> If you look at the users of cpu_pm_enter(), they are all ARM. Nobody else uses
> it, so at least please put cpuidle_generic_enter_state() under a #ifdef to avoid
> building it in vain on non-ARM.
>
Agreed, that should be taken care once I do the changes you are
suggesting below.
[...]
>> @@ -246,6 +248,12 @@ static inline int cpuidle_register_governor(struct cpuidle_governor *gov)
>> {return 0;}
>> #endif
>>
>> +#ifdef CONFIG_HAVE_GENERIC_CPUIDLE_ENTER
>> +extern int cpuidle_generic_enter(int idx);
>
> And if you put it under the #ifdef here, its definition also should go under
> the same #ifdef.
>
Yes I will do that.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH v7 4/6] cpuidle: introduce HAVE_GENERIC_CPUIDLE_ENTER for ARM{32,64} platforms
@ 2016-07-07 13:34 ` Sudeep Holla
0 siblings, 0 replies; 37+ messages in thread
From: Sudeep Holla @ 2016-07-07 13:34 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Sudeep Holla, linux-acpi, Vikas Sajjan, Sunil, Lorenzo Pieralisi,
PrashanthPrakash, Al Stone, Ashwin Chaugule, Daniel Lezcano,
linux-kernel, linux-arm-kernel
On 07/07/16 14:21, Rafael J. Wysocki wrote:
> On Tuesday, June 28, 2016 02:55:50 PM Sudeep Holla wrote:
>> The function arm_enter_idle_state is exactly the same in both generic
>> ARM{32,64} CPUIdle driver and will be the same even on ARM64 backend
>> for ACPI processor idle driver. So we can unify it and move it as
>> generic_cpuidle_enter by introducing HAVE_GENERIC_CPUIDLE_ENTER and
>> enabling the same on both ARM{32,64}.
>>
>> This is in preparation of reuse of the generic cpuidle entry function
>> for ACPI LPI support on ARM64.
>>
>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>> ---
>> arch/arm/Kconfig | 1 +
>> arch/arm/kernel/cpuidle.c | 4 ++--
>> arch/arm64/Kconfig | 1 +
>> arch/arm64/kernel/cpuidle.c | 6 +++---
>> drivers/cpuidle/Kconfig | 3 +++
>> drivers/cpuidle/cpuidle-arm.c | 21 +--------------------
>> drivers/cpuidle/cpuidle.c | 35 +++++++++++++++++++++++++++++++++++
>> include/linux/cpuidle.h | 8 ++++++++
>> 8 files changed, 54 insertions(+), 25 deletions(-)
>>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index 90542db1220d..52b3dca0381c 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -54,6 +54,7 @@ config ARM
>> select HAVE_FTRACE_MCOUNT_RECORD if (!XIP_KERNEL)
>> select HAVE_FUNCTION_GRAPH_TRACER if (!THUMB2_KERNEL)
>> select HAVE_FUNCTION_TRACER if (!XIP_KERNEL)
>> + select HAVE_GENERIC_CPUIDLE_ENTER
>
> That "generic" part in the name concerns me a bit, because the thing is not
> really generic. It is "common on ARM" rather.
>
I agree and that's exactly what I told Daniel. It's rather just
*ARM Generic*. Any preference on the name ? I had it header file under
include/linu/cpuidle-arm.h in the previous version. Do you prefer that ?
[..]
>> @@ -255,6 +256,40 @@ int cpuidle_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
>> }
>>
>> /**
>> + * cpuidle_generic_enter_state - enter into the specified idle state using the
>> + * generic callback if implemented
>> + *
>> + * @index: the index in the idle state table
>> + *
>> + * Returns the index in the idle state, -1 in case of error.
>> + */
>> +int cpuidle_generic_enter_state(int idx)
>> +{
>> + int ret;
>> +
>> + if (!idx) {
>> + cpu_do_idle();
>> + return idx;
>> + }
>> +
>> + ret = cpu_pm_enter();
>
> If you look at the users of cpu_pm_enter(), they are all ARM. Nobody else uses
> it, so at least please put cpuidle_generic_enter_state() under a #ifdef to avoid
> building it in vain on non-ARM.
>
Agreed, that should be taken care once I do the changes you are
suggesting below.
[...]
>> @@ -246,6 +248,12 @@ static inline int cpuidle_register_governor(struct cpuidle_governor *gov)
>> {return 0;}
>> #endif
>>
>> +#ifdef CONFIG_HAVE_GENERIC_CPUIDLE_ENTER
>> +extern int cpuidle_generic_enter(int idx);
>
> And if you put it under the #ifdef here, its definition also should go under
> the same #ifdef.
>
Yes I will do that.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH v7 4/6] cpuidle: introduce HAVE_GENERIC_CPUIDLE_ENTER for ARM{32,64} platforms
2016-07-07 13:34 ` Sudeep Holla
@ 2016-07-07 14:49 ` Rafael J. Wysocki
-1 siblings, 0 replies; 37+ messages in thread
From: Rafael J. Wysocki @ 2016-07-07 14:49 UTC (permalink / raw)
To: Sudeep Holla
Cc: linux-acpi, Vikas Sajjan, Sunil, Lorenzo Pieralisi,
PrashanthPrakash, Al Stone, Ashwin Chaugule, Daniel Lezcano,
linux-kernel, linux-arm-kernel
On Thursday, July 07, 2016 02:34:36 PM Sudeep Holla wrote:
>
> On 07/07/16 14:21, Rafael J. Wysocki wrote:
> > On Tuesday, June 28, 2016 02:55:50 PM Sudeep Holla wrote:
> >> The function arm_enter_idle_state is exactly the same in both generic
> >> ARM{32,64} CPUIdle driver and will be the same even on ARM64 backend
> >> for ACPI processor idle driver. So we can unify it and move it as
> >> generic_cpuidle_enter by introducing HAVE_GENERIC_CPUIDLE_ENTER and
> >> enabling the same on both ARM{32,64}.
> >>
> >> This is in preparation of reuse of the generic cpuidle entry function
> >> for ACPI LPI support on ARM64.
> >>
> >> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> >> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> >> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> >> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> >> ---
> >> arch/arm/Kconfig | 1 +
> >> arch/arm/kernel/cpuidle.c | 4 ++--
> >> arch/arm64/Kconfig | 1 +
> >> arch/arm64/kernel/cpuidle.c | 6 +++---
> >> drivers/cpuidle/Kconfig | 3 +++
> >> drivers/cpuidle/cpuidle-arm.c | 21 +--------------------
> >> drivers/cpuidle/cpuidle.c | 35 +++++++++++++++++++++++++++++++++++
> >> include/linux/cpuidle.h | 8 ++++++++
> >> 8 files changed, 54 insertions(+), 25 deletions(-)
> >>
> >> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> >> index 90542db1220d..52b3dca0381c 100644
> >> --- a/arch/arm/Kconfig
> >> +++ b/arch/arm/Kconfig
> >> @@ -54,6 +54,7 @@ config ARM
> >> select HAVE_FTRACE_MCOUNT_RECORD if (!XIP_KERNEL)
> >> select HAVE_FUNCTION_GRAPH_TRACER if (!THUMB2_KERNEL)
> >> select HAVE_FUNCTION_TRACER if (!XIP_KERNEL)
> >> + select HAVE_GENERIC_CPUIDLE_ENTER
> >
> > That "generic" part in the name concerns me a bit, because the thing is not
> > really generic. It is "common on ARM" rather.
> >
>
> I agree and that's exactly what I told Daniel. It's rather just
> *ARM Generic*. Any preference on the name ? I had it header file under
> include/linu/cpuidle-arm.h in the previous version. Do you prefer that ?
Well, I got confused by these names which probably means that they really
are confusing. :-)
So the underlying observation is that ->enter() callbacks in some ARM code
tend to do the same thing, ie. wrap the cpu_pm_enter()/exit() pair around
the actual "low-level enter" routine, so the idea is to move the wrapping
to the core and add the symbol plus standard header for the "low-level enter"
thing.
But then ->enter has to point to the wrapper and that just invokes a static
function defined somewhere.
So in fact what you want is to avoid code duplication in the source, but not
in the binary.
For that, I'd use a macro like this:
#define CPU_IDLE_ENTER_WRAPPED(low_level_idle_enter, idx) \
({ \
int __ret; \
\
if (!idx) { \
cpu_do_idle(); \
return idx; \
} \
\
__ret = cpu_pm_enter(); \
if (!__ret) { \
__ret = low_level_idle_enter(idx); \
cpu_pm_exit(); \
} \
\
__ret ? -1 : idx; \
})
and then, whoever want's to generate a "wrapped" callback, will need to
define the low_level_idle_enter thing, say my_low_level_idle_enter() and
then do
int idle_enter(int idx)
{
return CPU_IDLE_ENTER_WRAPPED(my_low_level_idle_enter, idx);
}
and point the ->enter callback to idle_enter().
No need for extra symbols, confusing function names and similar.
And the macro can go into cpuidle.h if you want.
Thanks,
Rafael
^ permalink raw reply [flat|nested] 37+ messages in thread* [PATCH v7 4/6] cpuidle: introduce HAVE_GENERIC_CPUIDLE_ENTER for ARM{32, 64} platforms
@ 2016-07-07 14:49 ` Rafael J. Wysocki
0 siblings, 0 replies; 37+ messages in thread
From: Rafael J. Wysocki @ 2016-07-07 14:49 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday, July 07, 2016 02:34:36 PM Sudeep Holla wrote:
>
> On 07/07/16 14:21, Rafael J. Wysocki wrote:
> > On Tuesday, June 28, 2016 02:55:50 PM Sudeep Holla wrote:
> >> The function arm_enter_idle_state is exactly the same in both generic
> >> ARM{32,64} CPUIdle driver and will be the same even on ARM64 backend
> >> for ACPI processor idle driver. So we can unify it and move it as
> >> generic_cpuidle_enter by introducing HAVE_GENERIC_CPUIDLE_ENTER and
> >> enabling the same on both ARM{32,64}.
> >>
> >> This is in preparation of reuse of the generic cpuidle entry function
> >> for ACPI LPI support on ARM64.
> >>
> >> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> >> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> >> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> >> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> >> ---
> >> arch/arm/Kconfig | 1 +
> >> arch/arm/kernel/cpuidle.c | 4 ++--
> >> arch/arm64/Kconfig | 1 +
> >> arch/arm64/kernel/cpuidle.c | 6 +++---
> >> drivers/cpuidle/Kconfig | 3 +++
> >> drivers/cpuidle/cpuidle-arm.c | 21 +--------------------
> >> drivers/cpuidle/cpuidle.c | 35 +++++++++++++++++++++++++++++++++++
> >> include/linux/cpuidle.h | 8 ++++++++
> >> 8 files changed, 54 insertions(+), 25 deletions(-)
> >>
> >> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> >> index 90542db1220d..52b3dca0381c 100644
> >> --- a/arch/arm/Kconfig
> >> +++ b/arch/arm/Kconfig
> >> @@ -54,6 +54,7 @@ config ARM
> >> select HAVE_FTRACE_MCOUNT_RECORD if (!XIP_KERNEL)
> >> select HAVE_FUNCTION_GRAPH_TRACER if (!THUMB2_KERNEL)
> >> select HAVE_FUNCTION_TRACER if (!XIP_KERNEL)
> >> + select HAVE_GENERIC_CPUIDLE_ENTER
> >
> > That "generic" part in the name concerns me a bit, because the thing is not
> > really generic. It is "common on ARM" rather.
> >
>
> I agree and that's exactly what I told Daniel. It's rather just
> *ARM Generic*. Any preference on the name ? I had it header file under
> include/linu/cpuidle-arm.h in the previous version. Do you prefer that ?
Well, I got confused by these names which probably means that they really
are confusing. :-)
So the underlying observation is that ->enter() callbacks in some ARM code
tend to do the same thing, ie. wrap the cpu_pm_enter()/exit() pair around
the actual "low-level enter" routine, so the idea is to move the wrapping
to the core and add the symbol plus standard header for the "low-level enter"
thing.
But then ->enter has to point to the wrapper and that just invokes a static
function defined somewhere.
So in fact what you want is to avoid code duplication in the source, but not
in the binary.
For that, I'd use a macro like this:
#define CPU_IDLE_ENTER_WRAPPED(low_level_idle_enter, idx) \
({ \
int __ret; \
\
if (!idx) { \
cpu_do_idle(); \
return idx; \
} \
\
__ret = cpu_pm_enter(); \
if (!__ret) { \
__ret = low_level_idle_enter(idx); \
cpu_pm_exit(); \
} \
\
__ret ? -1 : idx; \
})
and then, whoever want's to generate a "wrapped" callback, will need to
define the low_level_idle_enter thing, say my_low_level_idle_enter() and
then do
int idle_enter(int idx)
{
return CPU_IDLE_ENTER_WRAPPED(my_low_level_idle_enter, idx);
}
and point the ->enter callback to idle_enter().
No need for extra symbols, confusing function names and similar.
And the macro can go into cpuidle.h if you want.
Thanks,
Rafael
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH v7 4/6] cpuidle: introduce HAVE_GENERIC_CPUIDLE_ENTER for ARM{32,64} platforms
2016-07-07 14:49 ` [PATCH v7 4/6] cpuidle: introduce HAVE_GENERIC_CPUIDLE_ENTER for ARM{32, 64} platforms Rafael J. Wysocki
@ 2016-07-07 15:48 ` Sudeep Holla
-1 siblings, 0 replies; 37+ messages in thread
From: Sudeep Holla @ 2016-07-07 15:48 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Sudeep Holla, linux-acpi, Vikas Sajjan, Sunil, Lorenzo Pieralisi,
PrashanthPrakash, Al Stone, Ashwin Chaugule, Daniel Lezcano,
linux-kernel, linux-arm-kernel
On 07/07/16 15:49, Rafael J. Wysocki wrote:
> On Thursday, July 07, 2016 02:34:36 PM Sudeep Holla wrote:
>>
>> On 07/07/16 14:21, Rafael J. Wysocki wrote:
>>> On Tuesday, June 28, 2016 02:55:50 PM Sudeep Holla wrote:
>>>> The function arm_enter_idle_state is exactly the same in both generic
>>>> ARM{32,64} CPUIdle driver and will be the same even on ARM64 backend
>>>> for ACPI processor idle driver. So we can unify it and move it as
>>>> generic_cpuidle_enter by introducing HAVE_GENERIC_CPUIDLE_ENTER and
>>>> enabling the same on both ARM{32,64}.
>>>>
>>>> This is in preparation of reuse of the generic cpuidle entry function
>>>> for ACPI LPI support on ARM64.
>>>>
>>>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>>>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>>>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>>>> ---
>>>> arch/arm/Kconfig | 1 +
>>>> arch/arm/kernel/cpuidle.c | 4 ++--
>>>> arch/arm64/Kconfig | 1 +
>>>> arch/arm64/kernel/cpuidle.c | 6 +++---
>>>> drivers/cpuidle/Kconfig | 3 +++
>>>> drivers/cpuidle/cpuidle-arm.c | 21 +--------------------
>>>> drivers/cpuidle/cpuidle.c | 35 +++++++++++++++++++++++++++++++++++
>>>> include/linux/cpuidle.h | 8 ++++++++
>>>> 8 files changed, 54 insertions(+), 25 deletions(-)
>>>>
>>>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>>>> index 90542db1220d..52b3dca0381c 100644
>>>> --- a/arch/arm/Kconfig
>>>> +++ b/arch/arm/Kconfig
>>>> @@ -54,6 +54,7 @@ config ARM
>>>> select HAVE_FTRACE_MCOUNT_RECORD if (!XIP_KERNEL)
>>>> select HAVE_FUNCTION_GRAPH_TRACER if (!THUMB2_KERNEL)
>>>> select HAVE_FUNCTION_TRACER if (!XIP_KERNEL)
>>>> + select HAVE_GENERIC_CPUIDLE_ENTER
>>>
>>> That "generic" part in the name concerns me a bit, because the thing is not
>>> really generic. It is "common on ARM" rather.
>>>
>>
>> I agree and that's exactly what I told Daniel. It's rather just
>> *ARM Generic*. Any preference on the name ? I had it header file under
>> include/linu/cpuidle-arm.h in the previous version. Do you prefer that ?
>
> Well, I got confused by these names which probably means that they really
> are confusing. :-)
>
I know and I am all for getting rid of that.
> So the underlying observation is that ->enter() callbacks in some ARM code
> tend to do the same thing, ie. wrap the cpu_pm_enter()/exit() pair around
> the actual "low-level enter" routine, so the idea is to move the wrapping
> to the core and add the symbol plus standard header for the "low-level enter"
> thing.
>
> But then ->enter has to point to the wrapper and that just invokes a static
> function defined somewhere.
>
> So in fact what you want is to avoid code duplication in the source, but not
> in the binary.
>
> For that, I'd use a macro like this:
>
> #define CPU_IDLE_ENTER_WRAPPED(low_level_idle_enter, idx) \
> ({ \
> int __ret; \
> \
> if (!idx) { \
> cpu_do_idle(); \
> return idx; \
> } \
> \
> __ret = cpu_pm_enter(); \
> if (!__ret) { \
> __ret = low_level_idle_enter(idx); \
> cpu_pm_exit(); \
> } \
> \
> __ret ? -1 : idx; \
> })
>
> and then, whoever want's to generate a "wrapped" callback, will need to
> define the low_level_idle_enter thing, say my_low_level_idle_enter() and
> then do
>
> int idle_enter(int idx)
> {
> return CPU_IDLE_ENTER_WRAPPED(my_low_level_idle_enter, idx);
> }
>
> and point the ->enter callback to idle_enter().
>
> No need for extra symbols, confusing function names and similar.
>
> And the macro can go into cpuidle.h if you want.
>
Sounds good. Thanks for the suggestion. I will respin the series with
this change then.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 37+ messages in thread* [PATCH v7 4/6] cpuidle: introduce HAVE_GENERIC_CPUIDLE_ENTER for ARM{32,64} platforms
@ 2016-07-07 15:48 ` Sudeep Holla
0 siblings, 0 replies; 37+ messages in thread
From: Sudeep Holla @ 2016-07-07 15:48 UTC (permalink / raw)
To: linux-arm-kernel
On 07/07/16 15:49, Rafael J. Wysocki wrote:
> On Thursday, July 07, 2016 02:34:36 PM Sudeep Holla wrote:
>>
>> On 07/07/16 14:21, Rafael J. Wysocki wrote:
>>> On Tuesday, June 28, 2016 02:55:50 PM Sudeep Holla wrote:
>>>> The function arm_enter_idle_state is exactly the same in both generic
>>>> ARM{32,64} CPUIdle driver and will be the same even on ARM64 backend
>>>> for ACPI processor idle driver. So we can unify it and move it as
>>>> generic_cpuidle_enter by introducing HAVE_GENERIC_CPUIDLE_ENTER and
>>>> enabling the same on both ARM{32,64}.
>>>>
>>>> This is in preparation of reuse of the generic cpuidle entry function
>>>> for ACPI LPI support on ARM64.
>>>>
>>>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>>>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>>>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>>>> ---
>>>> arch/arm/Kconfig | 1 +
>>>> arch/arm/kernel/cpuidle.c | 4 ++--
>>>> arch/arm64/Kconfig | 1 +
>>>> arch/arm64/kernel/cpuidle.c | 6 +++---
>>>> drivers/cpuidle/Kconfig | 3 +++
>>>> drivers/cpuidle/cpuidle-arm.c | 21 +--------------------
>>>> drivers/cpuidle/cpuidle.c | 35 +++++++++++++++++++++++++++++++++++
>>>> include/linux/cpuidle.h | 8 ++++++++
>>>> 8 files changed, 54 insertions(+), 25 deletions(-)
>>>>
>>>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>>>> index 90542db1220d..52b3dca0381c 100644
>>>> --- a/arch/arm/Kconfig
>>>> +++ b/arch/arm/Kconfig
>>>> @@ -54,6 +54,7 @@ config ARM
>>>> select HAVE_FTRACE_MCOUNT_RECORD if (!XIP_KERNEL)
>>>> select HAVE_FUNCTION_GRAPH_TRACER if (!THUMB2_KERNEL)
>>>> select HAVE_FUNCTION_TRACER if (!XIP_KERNEL)
>>>> + select HAVE_GENERIC_CPUIDLE_ENTER
>>>
>>> That "generic" part in the name concerns me a bit, because the thing is not
>>> really generic. It is "common on ARM" rather.
>>>
>>
>> I agree and that's exactly what I told Daniel. It's rather just
>> *ARM Generic*. Any preference on the name ? I had it header file under
>> include/linu/cpuidle-arm.h in the previous version. Do you prefer that ?
>
> Well, I got confused by these names which probably means that they really
> are confusing. :-)
>
I know and I am all for getting rid of that.
> So the underlying observation is that ->enter() callbacks in some ARM code
> tend to do the same thing, ie. wrap the cpu_pm_enter()/exit() pair around
> the actual "low-level enter" routine, so the idea is to move the wrapping
> to the core and add the symbol plus standard header for the "low-level enter"
> thing.
>
> But then ->enter has to point to the wrapper and that just invokes a static
> function defined somewhere.
>
> So in fact what you want is to avoid code duplication in the source, but not
> in the binary.
>
> For that, I'd use a macro like this:
>
> #define CPU_IDLE_ENTER_WRAPPED(low_level_idle_enter, idx) \
> ({ \
> int __ret; \
> \
> if (!idx) { \
> cpu_do_idle(); \
> return idx; \
> } \
> \
> __ret = cpu_pm_enter(); \
> if (!__ret) { \
> __ret = low_level_idle_enter(idx); \
> cpu_pm_exit(); \
> } \
> \
> __ret ? -1 : idx; \
> })
>
> and then, whoever want's to generate a "wrapped" callback, will need to
> define the low_level_idle_enter thing, say my_low_level_idle_enter() and
> then do
>
> int idle_enter(int idx)
> {
> return CPU_IDLE_ENTER_WRAPPED(my_low_level_idle_enter, idx);
> }
>
> and point the ->enter callback to idle_enter().
>
> No need for extra symbols, confusing function names and similar.
>
> And the macro can go into cpuidle.h if you want.
>
Sounds good. Thanks for the suggestion. I will respin the series with
this change then.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v7 5/6] arm64: add support for ACPI Low Power Idle(LPI)
2016-06-28 13:55 ` Sudeep Holla
@ 2016-06-28 13:55 ` Sudeep Holla
-1 siblings, 0 replies; 37+ messages in thread
From: Sudeep Holla @ 2016-06-28 13:55 UTC (permalink / raw)
To: linux-acpi, Rafael J . Wysocki
Cc: Sudeep Holla, Vikas Sajjan, Sunil, Lorenzo Pieralisi,
PrashanthPrakash, Al Stone, Ashwin Chaugule, Daniel Lezcano,
linux-kernel, linux-arm-kernel, Mark Rutland
This patch adds appropriate callbacks to support ACPI Low Power Idle
(LPI) on ARM64.
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
arch/arm64/kernel/cpuidle.c | 16 ++++++
drivers/firmware/psci.c | 122 ++++++++++++++++++++++++++++++++++++--------
2 files changed, 116 insertions(+), 22 deletions(-)
diff --git a/arch/arm64/kernel/cpuidle.c b/arch/arm64/kernel/cpuidle.c
index bade1b04ff12..dc241e8de75c 100644
--- a/arch/arm64/kernel/cpuidle.c
+++ b/arch/arm64/kernel/cpuidle.c
@@ -9,6 +9,7 @@
* published by the Free Software Foundation.
*/
+#include <linux/acpi.h>
#include <linux/cpuidle.h>
#include <linux/of.h>
#include <linux/of_device.h>
@@ -39,3 +40,18 @@ int cpuidle_generic_enter(int index)
return cpu_ops[cpu]->cpu_suspend(index);
}
+
+#ifdef CONFIG_ACPI
+
+#include <acpi/processor.h>
+
+int acpi_processor_ffh_lpi_probe(unsigned int cpu)
+{
+ return arm_cpuidle_init(cpu);
+}
+
+int acpi_processor_ffh_lpi_enter(struct acpi_lpi_state *lpi)
+{
+ return cpuidle_generic_enter_state(lpi->index);
+}
+#endif
diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
index 03e04582791c..edd83c884a1d 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -13,6 +13,7 @@
#define pr_fmt(fmt) "psci: " fmt
+#include <linux/acpi.h>
#include <linux/arm-smccc.h>
#include <linux/cpuidle.h>
#include <linux/errno.h>
@@ -250,12 +251,84 @@ 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_get_state_count(struct device_node *cpu_dn)
{
- int i, ret, count = 0;
- u32 *psci_states;
+ int count = 0;
struct device_node *state_node;
+ /* Count idle states */
+ while ((state_node = of_parse_phandle(cpu_dn, "cpu-idle-states",
+ count))) {
+ count++;
+ of_node_put(state_node);
+ }
+
+ return count;
+}
+
+static int psci_dt_get_idle_state(struct device_node *node, int idx, u32 *state)
+{
+ int ret;
+ struct device_node *state_node;
+
+ state_node = of_parse_phandle(node, "cpu-idle-states", idx);
+
+ ret = of_property_read_u32(state_node, "arm,psci-suspend-param",
+ state);
+ if (ret)
+ pr_warn(" * %s missing arm,psci-suspend-param property\n",
+ state_node->full_name);
+
+ of_node_put(state_node);
+
+ return ret;
+}
+
+#ifdef CONFIG_ACPI
+#include <acpi/processor.h>
+
+static int psci_acpi_get_state_count(unsigned int cpu)
+{
+ struct acpi_processor *pr = per_cpu(processors, cpu);
+
+ if (unlikely(!pr || !pr->flags.has_lpi))
+ return -EINVAL;
+
+ return pr->power.count - 1;
+}
+
+static int psci_acpi_get_idle_state(unsigned int cpu, int idx, u32 *state)
+{
+ struct acpi_processor *pr = per_cpu(processors, cpu);
+ struct acpi_lpi_state *lpi = &pr->power.lpi_states[idx + 1];
+
+ if (!lpi)
+ return -EINVAL;
+
+ /*
+ * Only bits[31:0] represent a PSCI power_state while
+ * bits[63:32] must be 0x0 as per ARM ACPI FFH Specification
+ */
+ *state = lpi->address;
+ return 0;
+}
+#else
+static int psci_acpi_get_state_count(unsigned int cpu)
+{
+ return -EINVAL;
+}
+
+static int psci_acpi_get_idle_state(unsigned int cpu, int idx, u32 *state)
+{
+ return -EINVAL;
+}
+#endif
+
+static int __psci_cpu_init_idle(struct device_node *cpu_dn, int cpu, bool acpi)
+{
+ int i, count, ret;
+ u32 *psci_states;
+
/*
* If the PSCI cpu_suspend function hook has not been initialized
* idle states must not be enabled, so bail out
@@ -263,14 +336,12 @@ static int psci_dt_cpu_init_idle(struct device_node *cpu_node, int cpu)
if (!psci_ops.cpu_suspend)
return -EOPNOTSUPP;
- /* Count idle states */
- while ((state_node = of_parse_phandle(cpu_node, "cpu-idle-states",
- count))) {
- count++;
- of_node_put(state_node);
- }
+ if (acpi)
+ count = psci_acpi_get_state_count(cpu);
+ else
+ count = psci_dt_get_state_count(cpu_dn);
- if (!count)
+ if (count <= 0)
return -ENODEV;
psci_states = kcalloc(count, sizeof(*psci_states), GFP_KERNEL);
@@ -278,21 +349,15 @@ static int psci_dt_cpu_init_idle(struct device_node *cpu_node, int cpu)
return -ENOMEM;
for (i = 0; i < count; i++) {
- u32 state;
-
- state_node = of_parse_phandle(cpu_node, "cpu-idle-states", i);
+ u32 state = 0;
- ret = of_property_read_u32(state_node,
- "arm,psci-suspend-param",
- &state);
- if (ret) {
- pr_warn(" * %s missing arm,psci-suspend-param property\n",
- state_node->full_name);
- of_node_put(state_node);
+ if (acpi)
+ ret = psci_acpi_get_idle_state(cpu, i, &state);
+ else
+ ret = psci_dt_get_idle_state(cpu_dn, i, &state);
+ if (ret)
goto free_mem;
- }
- of_node_put(state_node);
pr_debug("psci-power-state %#x index %d\n", state, i);
if (!psci_power_state_is_valid(state)) {
pr_warn("Invalid PSCI power state %#x\n", state);
@@ -310,11 +375,24 @@ static int psci_dt_cpu_init_idle(struct device_node *cpu_node, int cpu)
return ret;
}
+static int psci_dt_cpu_init_idle(struct device_node *cpu_node, int cpu)
+{
+ return __psci_cpu_init_idle(cpu_node, cpu, false);
+}
+
+static int psci_acpi_cpu_init_idle(unsigned int cpu)
+{
+ return __psci_cpu_init_idle(NULL, cpu, true);
+}
+
int psci_cpu_init_idle(unsigned int cpu)
{
struct device_node *cpu_node;
int ret;
+ if (!acpi_disabled)
+ return psci_acpi_cpu_init_idle(cpu);
+
cpu_node = of_get_cpu_node(cpu, NULL);
if (!cpu_node)
return -ENODEV;
--
2.7.4
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v7 5/6] arm64: add support for ACPI Low Power Idle(LPI)
@ 2016-06-28 13:55 ` Sudeep Holla
0 siblings, 0 replies; 37+ messages in thread
From: Sudeep Holla @ 2016-06-28 13:55 UTC (permalink / raw)
To: linux-arm-kernel
This patch adds appropriate callbacks to support ACPI Low Power Idle
(LPI) on ARM64.
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
arch/arm64/kernel/cpuidle.c | 16 ++++++
drivers/firmware/psci.c | 122 ++++++++++++++++++++++++++++++++++++--------
2 files changed, 116 insertions(+), 22 deletions(-)
diff --git a/arch/arm64/kernel/cpuidle.c b/arch/arm64/kernel/cpuidle.c
index bade1b04ff12..dc241e8de75c 100644
--- a/arch/arm64/kernel/cpuidle.c
+++ b/arch/arm64/kernel/cpuidle.c
@@ -9,6 +9,7 @@
* published by the Free Software Foundation.
*/
+#include <linux/acpi.h>
#include <linux/cpuidle.h>
#include <linux/of.h>
#include <linux/of_device.h>
@@ -39,3 +40,18 @@ int cpuidle_generic_enter(int index)
return cpu_ops[cpu]->cpu_suspend(index);
}
+
+#ifdef CONFIG_ACPI
+
+#include <acpi/processor.h>
+
+int acpi_processor_ffh_lpi_probe(unsigned int cpu)
+{
+ return arm_cpuidle_init(cpu);
+}
+
+int acpi_processor_ffh_lpi_enter(struct acpi_lpi_state *lpi)
+{
+ return cpuidle_generic_enter_state(lpi->index);
+}
+#endif
diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
index 03e04582791c..edd83c884a1d 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -13,6 +13,7 @@
#define pr_fmt(fmt) "psci: " fmt
+#include <linux/acpi.h>
#include <linux/arm-smccc.h>
#include <linux/cpuidle.h>
#include <linux/errno.h>
@@ -250,12 +251,84 @@ 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_get_state_count(struct device_node *cpu_dn)
{
- int i, ret, count = 0;
- u32 *psci_states;
+ int count = 0;
struct device_node *state_node;
+ /* Count idle states */
+ while ((state_node = of_parse_phandle(cpu_dn, "cpu-idle-states",
+ count))) {
+ count++;
+ of_node_put(state_node);
+ }
+
+ return count;
+}
+
+static int psci_dt_get_idle_state(struct device_node *node, int idx, u32 *state)
+{
+ int ret;
+ struct device_node *state_node;
+
+ state_node = of_parse_phandle(node, "cpu-idle-states", idx);
+
+ ret = of_property_read_u32(state_node, "arm,psci-suspend-param",
+ state);
+ if (ret)
+ pr_warn(" * %s missing arm,psci-suspend-param property\n",
+ state_node->full_name);
+
+ of_node_put(state_node);
+
+ return ret;
+}
+
+#ifdef CONFIG_ACPI
+#include <acpi/processor.h>
+
+static int psci_acpi_get_state_count(unsigned int cpu)
+{
+ struct acpi_processor *pr = per_cpu(processors, cpu);
+
+ if (unlikely(!pr || !pr->flags.has_lpi))
+ return -EINVAL;
+
+ return pr->power.count - 1;
+}
+
+static int psci_acpi_get_idle_state(unsigned int cpu, int idx, u32 *state)
+{
+ struct acpi_processor *pr = per_cpu(processors, cpu);
+ struct acpi_lpi_state *lpi = &pr->power.lpi_states[idx + 1];
+
+ if (!lpi)
+ return -EINVAL;
+
+ /*
+ * Only bits[31:0] represent a PSCI power_state while
+ * bits[63:32] must be 0x0 as per ARM ACPI FFH Specification
+ */
+ *state = lpi->address;
+ return 0;
+}
+#else
+static int psci_acpi_get_state_count(unsigned int cpu)
+{
+ return -EINVAL;
+}
+
+static int psci_acpi_get_idle_state(unsigned int cpu, int idx, u32 *state)
+{
+ return -EINVAL;
+}
+#endif
+
+static int __psci_cpu_init_idle(struct device_node *cpu_dn, int cpu, bool acpi)
+{
+ int i, count, ret;
+ u32 *psci_states;
+
/*
* If the PSCI cpu_suspend function hook has not been initialized
* idle states must not be enabled, so bail out
@@ -263,14 +336,12 @@ static int psci_dt_cpu_init_idle(struct device_node *cpu_node, int cpu)
if (!psci_ops.cpu_suspend)
return -EOPNOTSUPP;
- /* Count idle states */
- while ((state_node = of_parse_phandle(cpu_node, "cpu-idle-states",
- count))) {
- count++;
- of_node_put(state_node);
- }
+ if (acpi)
+ count = psci_acpi_get_state_count(cpu);
+ else
+ count = psci_dt_get_state_count(cpu_dn);
- if (!count)
+ if (count <= 0)
return -ENODEV;
psci_states = kcalloc(count, sizeof(*psci_states), GFP_KERNEL);
@@ -278,21 +349,15 @@ static int psci_dt_cpu_init_idle(struct device_node *cpu_node, int cpu)
return -ENOMEM;
for (i = 0; i < count; i++) {
- u32 state;
-
- state_node = of_parse_phandle(cpu_node, "cpu-idle-states", i);
+ u32 state = 0;
- ret = of_property_read_u32(state_node,
- "arm,psci-suspend-param",
- &state);
- if (ret) {
- pr_warn(" * %s missing arm,psci-suspend-param property\n",
- state_node->full_name);
- of_node_put(state_node);
+ if (acpi)
+ ret = psci_acpi_get_idle_state(cpu, i, &state);
+ else
+ ret = psci_dt_get_idle_state(cpu_dn, i, &state);
+ if (ret)
goto free_mem;
- }
- of_node_put(state_node);
pr_debug("psci-power-state %#x index %d\n", state, i);
if (!psci_power_state_is_valid(state)) {
pr_warn("Invalid PSCI power state %#x\n", state);
@@ -310,11 +375,24 @@ static int psci_dt_cpu_init_idle(struct device_node *cpu_node, int cpu)
return ret;
}
+static int psci_dt_cpu_init_idle(struct device_node *cpu_node, int cpu)
+{
+ return __psci_cpu_init_idle(cpu_node, cpu, false);
+}
+
+static int psci_acpi_cpu_init_idle(unsigned int cpu)
+{
+ return __psci_cpu_init_idle(NULL, cpu, true);
+}
+
int psci_cpu_init_idle(unsigned int cpu)
{
struct device_node *cpu_node;
int ret;
+ if (!acpi_disabled)
+ return psci_acpi_cpu_init_idle(cpu);
+
cpu_node = of_get_cpu_node(cpu, NULL);
if (!cpu_node)
return -ENODEV;
--
2.7.4
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH v7 6/6] ACPI : enable ACPI_PROCESSOR_IDLE on ARM64
2016-06-28 13:55 ` Sudeep Holla
@ 2016-06-28 13:55 ` Sudeep Holla
-1 siblings, 0 replies; 37+ messages in thread
From: Sudeep Holla @ 2016-06-28 13:55 UTC (permalink / raw)
To: linux-acpi, Rafael J . Wysocki
Cc: Sudeep Holla, Vikas Sajjan, Sunil, Lorenzo Pieralisi,
PrashanthPrakash, Al Stone, Ashwin Chaugule, Daniel Lezcano,
linux-kernel, linux-arm-kernel
Now that ACPI processor idle driver supports LPI(Low Power Idle), lets
enable ACPI_PROCESSOR_IDLE for ARM64 too.
This patch just removes the IA64 and X86 dependency on ACPI_PROCESSOR_IDLE
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
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 1358fb7d7a68..d74275c0f374 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -238,7 +238,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
--
2.7.4
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH v7 6/6] ACPI : enable ACPI_PROCESSOR_IDLE on ARM64
@ 2016-06-28 13:55 ` Sudeep Holla
0 siblings, 0 replies; 37+ messages in thread
From: Sudeep Holla @ 2016-06-28 13:55 UTC (permalink / raw)
To: linux-arm-kernel
Now that ACPI processor idle driver supports LPI(Low Power Idle), lets
enable ACPI_PROCESSOR_IDLE for ARM64 too.
This patch just removes the IA64 and X86 dependency on ACPI_PROCESSOR_IDLE
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
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 1358fb7d7a68..d74275c0f374 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -238,7 +238,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
--
2.7.4
^ permalink raw reply related [flat|nested] 37+ messages in thread