[PATCH v9 0/7] ACPI / processor_idle: Add ACPI v6.0 LPI support

[PATCH v9 0/7] ACPI / processor_idle: Add ACPI v6.0 LPI support

[Sudeep Holla]

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.

v8[8]->v9:
	- Split the psci and arm64 changes into separate patches as
	  suggested by Lorenzo and added his ACKs
	- Renamed CPU_IDLE_ENTER_WRAPPED to CPU_PM_CPU_IDLE_ENTER

v7[7]->v8:
	- Replaced HAVE_GENERIC_CPUIDLE_ENTER with CPU_IDLE_ENTER_WRAPPED
	  macro, which is more cleaner and definately less confusing :)
	  (Thanks to Rafael for the suggestion)

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 @[9]

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=146591573516602&w=2
[7] http://marc.info/?l=linux-acpi&m=146712226917361&w=2
[8] http://marc.info/?l=linux-acpi&m=146791146605094&w=2
[9] git://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git for_review/arm64_lpi

Sudeep Holla (7):
  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 CPU_PM_CPU_IDLE_ENTER macro for ARM{32,64}
  drivers: firmware: psci: initialise idle states using ACPI LPI
  arm64: add support for ACPI Low Power Idle(LPI)
  ACPI : enable ACPI_PROCESSOR_IDLE on ARM64

 arch/arm64/kernel/cpuidle.c     |  20 +-
 drivers/acpi/Kconfig            |   6 +-
 drivers/acpi/bus.c              |  14 +-
 drivers/acpi/processor_driver.c |   2 +-
 drivers/acpi/processor_idle.c   | 544 ++++++++++++++++++++++++++++++++++------
 drivers/cpuidle/cpuidle-arm.c   |  26 +-
 drivers/firmware/psci.c         |  66 ++++-
 include/acpi/processor.h        |  26 +-
 include/linux/acpi.h            |   4 +
 include/linux/cpuidle.h         |  18 ++
 10 files changed, 611 insertions(+), 115 deletions(-)

-- 
2.7.4

[PATCH v9 1/7] ACPI / processor_idle: introduce ACPI_PROCESSOR_CSTATE

[Sudeep Holla]

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

[PATCH v9 2/7] ACPI / processor_idle: Add support for Low Power Idle(LPI) states

[Sudeep Holla]

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

[PATCH v9 3/7] arm64: cpuidle: drop __init section marker to arm_cpuidle_init

[Sudeep Holla]

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

[PATCH v9 4/7] cpuidle: introduce CPU_PM_CPU_IDLE_ENTER macro for ARM{32, 64}

[Sudeep Holla]

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 to a
common place by introducing CPU_PM_CPU_IDLE_ENTER macro that can be
used in all places avoiding duplication.

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>
Suggested-by: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/cpuidle/cpuidle-arm.c | 26 ++++++--------------------
 include/linux/cpuidle.h       | 18 ++++++++++++++++++
 2 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/drivers/cpuidle/cpuidle-arm.c b/drivers/cpuidle/cpuidle-arm.c
index e342565e8715..4ba3d3fe142f 100644
--- a/drivers/cpuidle/cpuidle-arm.c
+++ b/drivers/cpuidle/cpuidle-arm.c
@@ -36,26 +36,12 @@
 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;
+	/*
+	 * Pass idle state index to arm_cpuidle_suspend which in turn
+	 * will call the CPU ops suspend protocol with idle index as a
+	 * parameter.
+	 */
+	return CPU_PM_CPU_IDLE_ENTER(arm_cpuidle_suspend, idx);
 }
 
 static struct cpuidle_driver arm_idle_driver = {
diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index 07b83d32f66c..bb31373c3478 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -252,4 +252,22 @@ static inline int cpuidle_register_governor(struct cpuidle_governor *gov)
 #define CPUIDLE_DRIVER_STATE_START	0
 #endif
 
+#define CPU_PM_CPU_IDLE_ENTER(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;					\
+})
+
 #endif /* _LINUX_CPUIDLE_H */
-- 
2.7.4

[PATCH v9 5/7] drivers: firmware: psci: initialise idle states using ACPI LPI

[Sudeep Holla]

This patch adds support for initialisation of PSCI CPUIdle states
from Low Power Idle(_LPI) entries in the ACPI tables when acpi is
enabled.

Cc: Mark Rutland <mark.rutland@arm.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/psci.c | 66 +++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 59 insertions(+), 7 deletions(-)

diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
index 03e04582791c..8263429e21b8 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>
@@ -256,13 +257,6 @@ static int psci_dt_cpu_init_idle(struct device_node *cpu_node, int cpu)
 	u32 *psci_states;
 	struct device_node *state_node;
 
-	/*
-	 * If the PSCI cpu_suspend function hook has not been initialized
-	 * idle states must not be enabled, so bail out
-	 */
-	if (!psci_ops.cpu_suspend)
-		return -EOPNOTSUPP;
-
 	/* Count idle states */
 	while ((state_node = of_parse_phandle(cpu_node, "cpu-idle-states",
 					      count))) {
@@ -310,11 +304,69 @@ static int psci_dt_cpu_init_idle(struct device_node *cpu_node, int cpu)
 	return ret;
 }
 
+#ifdef CONFIG_ACPI
+#include <acpi/processor.h>
+
+static int __maybe_unused psci_acpi_cpu_init_idle(unsigned int cpu)
+{
+	int i, count;
+	u32 *psci_states;
+	struct acpi_lpi_state *lpi;
+	struct acpi_processor *pr = per_cpu(processors, cpu);
+
+	if (unlikely(!pr || !pr->flags.has_lpi))
+		return -EINVAL;
+
+	count = pr->power.count - 1;
+	if (count <= 0)
+		return -ENODEV;
+
+	psci_states = kcalloc(count, sizeof(*psci_states), GFP_KERNEL);
+	if (!psci_states)
+		return -ENOMEM;
+
+	for (i = 0; i < count; i++) {
+		u32 state;
+
+		lpi = &pr->power.lpi_states[i + 1];
+		/*
+		 * 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;
+		if (!psci_power_state_is_valid(state)) {
+			pr_warn("Invalid PSCI power state %#x\n", state);
+			kfree(psci_states);
+			return -EINVAL;
+		}
+		psci_states[i] = state;
+	}
+	/* Idle states parsed correctly, initialize per-cpu pointer */
+	per_cpu(psci_power_state, cpu) = psci_states;
+	return 0;
+}
+#else
+static int __maybe_unused psci_acpi_cpu_init_idle(unsigned int cpu)
+{
+	return -EINVAL;
+}
+#endif
+
 int psci_cpu_init_idle(unsigned int cpu)
 {
 	struct device_node *cpu_node;
 	int ret;
 
+	/*
+	 * If the PSCI cpu_suspend function hook has not been initialized
+	 * idle states must not be enabled, so bail out
+	 */
+	if (!psci_ops.cpu_suspend)
+		return -EOPNOTSUPP;
+
+	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

[PATCH v9 6/7] arm64: add support for ACPI Low Power Idle(LPI)

[Sudeep Holla]

This patch adds appropriate callbacks to support ACPI Low Power Idle
(LPI) on ARM64.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/arm64/kernel/cpuidle.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/arm64/kernel/cpuidle.c b/arch/arm64/kernel/cpuidle.c
index 06786fdaadeb..75a0f8acef66 100644
--- a/arch/arm64/kernel/cpuidle.c
+++ b/arch/arm64/kernel/cpuidle.c
@@ -9,6 +9,9 @@
  * published by the Free Software Foundation.
  */
 
+#include <linux/acpi.h>
+#include <linux/cpuidle.h>
+#include <linux/cpu_pm.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
 
@@ -39,3 +42,18 @@ int arm_cpuidle_suspend(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 CPU_PM_CPU_IDLE_ENTER(arm_cpuidle_suspend, lpi->index);
+}
+#endif
-- 
2.7.4

[PATCH v9 7/7] ACPI : enable ACPI_PROCESSOR_IDLE on ARM64

[Sudeep Holla]

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

[PATCH v9 0/7] ACPI / processor_idle: Add ACPI v6.0 LPI support

[Sudeep Holla]

On 08/07/16 18:07, Sudeep Holla wrote:
> 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.
>

I was hoping to get this in v4.8 now that merge window is
delayed/extended if you have no further comments on this series.

-- 
Regards,
Sudeep

[PATCH v9 0/7] ACPI / processor_idle: Add ACPI v6.0 LPI support

[Rafael J. Wysocki]

On Tuesday, July 12, 2016 12:42:06 PM Sudeep Holla wrote:
> 
> On 08/07/16 18:07, Sudeep Holla wrote:
> > 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.
> >
> 
> I was hoping to get this in v4.8 now that merge window is
> delayed/extended if you have no further comments on this series.

I'll get to it in the next couple of days.  If it looks all good and there are
no comments, I'll queue it up.

Thanks,
Rafael

[PATCH v9 0/7] ACPI / processor_idle: Add ACPI v6.0 LPI support

[Sudeep Holla]

On 12/07/16 13:09, Rafael J. Wysocki wrote:
> On Tuesday, July 12, 2016 12:42:06 PM Sudeep Holla wrote:
>>
>> On 08/07/16 18:07, Sudeep Holla wrote:
>>> 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.
>>>
>>
>> I was hoping to get this in v4.8 now that merge window is
>> delayed/extended if you have no further comments on this series.
>
> I'll get to it in the next couple of days.  If it looks all good and there are
> no comments, I'll queue it up.
>

Thanks for the update.

-- 
Regards,
Sudeep

[PATCH v9 6/7] arm64: add support for ACPI Low Power Idle(LPI)

[Catalin Marinas]

On Fri, Jul 08, 2016 at 06:07:57PM +0100, Sudeep Holla wrote:
> This patch adds appropriate callbacks to support ACPI Low Power Idle
> (LPI) on ARM64.
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

(in case Rafael needs it)

[PATCH v9 3/7] arm64: cpuidle: drop __init section marker to arm_cpuidle_init

[Catalin Marinas]

On Fri, Jul 08, 2016 at 06:07:54PM +0100, Sudeep Holla wrote:
> 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>

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

[PATCH v9 0/7] ACPI / processor_idle: Add ACPI v6.0 LPI support

[Rafael J. Wysocki]

On Tue, Jul 12, 2016 at 2:59 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
>
> On 12/07/16 13:09, Rafael J. Wysocki wrote:
>>
>> On Tuesday, July 12, 2016 12:42:06 PM Sudeep Holla wrote:
>>>
>>>
>>> On 08/07/16 18:07, Sudeep Holla wrote:
>>>>
>>>> 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.
>>>>
>>>
>>> I was hoping to get this in v4.8 now that merge window is
>>> delayed/extended if you have no further comments on this series.
>>
>>
>> I'll get to it in the next couple of days.  If it looks all good and there
>> are
>> no comments, I'll queue it up.
>>
>
> Thanks for the update.

I've just looked at patch [2/7] and there still are things I'm not
particularly liking in it.

Apart from a bunch of minor stuff, acpi_processor_get_lpi_info() is
not my favorite.  The way it creates all of those temporary data
structures and then walks them recursively seems a bit excessive.

I need to think about it a bit more, so I'll send my comments on that
patch later this week.

Thanks,
Rafael

[PATCH v9 2/7] ACPI / processor_idle: Add support for Low Power Idle(LPI) states

[Rafael J. Wysocki]

On Friday, July 08, 2016 06:07:53 PM Sudeep Holla wrote:
> ACPI 6.0 introduced an optional object _LPI that provides an alternate
> method to describe Low Power Idle states. It defines the local power
> states for each node in a hierarchical processor topology. The OSPM can
> use _LPI object to select a local power state for each level of processor
> hierarchy in the system. They used to produce a composite power state
> request that is presented to the platform by the OSPM.
> 
> Since multiple processors affect the idle state for any non-leaf hierarchy
> node, coordination of idle state requests between the processors is
> required. ACPI supports two different coordination schemes: Platform
> coordinated and  OS initiated.
> 
> This patch adds initial support for Platform coordination scheme of LPI.
> 
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  drivers/acpi/bus.c              |  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++) {

The line break is not necessary.

> +		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;

The two conditions above could be ORed.

> +
> +		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))

Inner parens not needed.

> +				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;

I'd use the ternary operator here, but that's me.

> +		} 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));

I'd prefer acpi_handle_debug() here.  ACPI_DEBUG_PRINT() is an ACPICA thing
really.

> +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);
> +

The above code doesn't look particularly straightforward and it seems to be
using more memory than necessary.

For instance, you don't need to store the acpi_processor_evaluate_lpi() data
for all levels at the same time.  It only needs to be stored for the current
level and you need a list of states enabling the ones at the current from
the previous one.  Plus the current list of effective states available to the
CPU.

It also should check the upper bound of pr->power.lpi_states[] somewhere which
it doesn't do AFAICS.

> +	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;

The checks above can be combined into one.

> +
> +	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;
> +

Again, the checks above can be combined.

> +	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);
> +}
> +

The rest of the patch looks OK to me.

Thanks,
Rafael

[PATCH v9 2/7] ACPI / processor_idle: Add support for Low Power Idle(LPI) states

[Sudeep Holla]

On 16/07/16 01:32, Rafael J. Wysocki wrote:
> On Friday, July 08, 2016 06:07:53 PM Sudeep Holla wrote:
>> ACPI 6.0 introduced an optional object _LPI that provides an alternate
>> method to describe Low Power Idle states. It defines the local power
>> states for each node in a hierarchical processor topology. The OSPM can
>> use _LPI object to select a local power state for each level of processor
>> hierarchy in the system. They used to produce a composite power state
>> request that is presented to the platform by the OSPM.
>>
>> Since multiple processors affect the idle state for any non-leaf hierarchy
>> node, coordination of idle state requests between the processors is
>> required. ACPI supports two different coordination schemes: Platform
>> coordinated and  OS initiated.
>>
>> This patch adds initial support for Platform coordination scheme of LPI.
>>
>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>> ---
>>   drivers/acpi/bus.c              |  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/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

[....]

>> +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);
>> +
>
> The above code doesn't look particularly straightforward and it seems to be
> using more memory than necessary.
>
> For instance, you don't need to store the acpi_processor_evaluate_lpi() data
> for all levels at the same time.  It only needs to be stored for the current
> level and you need a list of states enabling the ones at the current from
> the previous one.  Plus the current list of effective states available to the
> CPU.
>

Agreed with all the comments and fixed locally except this one. This
approach considered one leaf node and searched till the root each time.
To save memory we need to complete one level at a time. I am making
changes for this now but make require more testing with all combinations.

> It also should check the upper bound of pr->power.lpi_states[] somewhere which
> it doesn't do AFAICS.
>

Fixed now

-- 
Regards,
Sudeep