[PATCH RFC 0/5] ACPI / core : few cleanups and updates for LPI

[PATCH RFC 0/5] ACPI / core : few cleanups and updates for LPI

[Sudeep Holla]

Hi Rafael,

I am working on adding Low Power Idle(LPI) states support that was
recently added in ACPI 6.0. Before introducing the LPI support, here
are few updates and cleanups/reorganisation so that LPI support can be
added to processor_idle module to co-exist with C-state. Few of these
changes also help in enabling ACPI_PROCESSOR on ARM64 which is currently
disabled and few others are just to be consistent.

Regards,
Sudeep


Sudeep Holla (5):
  ACPI / containers : add support for ACPI0010 processor container
  ACPI / processor: always compile perflib if CONFIG_ACPI_PROCESSOR
  ACPI / sleep: move acpi_processor_sleep to sleep.c
  ACPI / processor_idle: replace PREFIX with pr_fmt
  ACPI / processor_idle : introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE

 Documentation/memory-hotplug.txt |   2 +-
 arch/ia64/Kconfig                |   1 +
 arch/x86/Kconfig                 |   1 +
 arch/x86/include/asm/acpi.h      |   2 -
 drivers/acpi/Kconfig             |   3 +
 drivers/acpi/Makefile            |   2 +-
 drivers/acpi/container.c         |   1 +
 drivers/acpi/processor_idle.c    | 119 +++++++++++++++++----------------------
 drivers/acpi/sleep.c             |  37 ++++++++++++
 include/acpi/processor.h         |  33 +----------
 include/linux/cpufreq.h          |   4 ++
 11 files changed, 104 insertions(+), 101 deletions(-)

-- 
1.9.1

[PATCH RFC 1/5] ACPI / containers : add support for ACPI0010 processor container

[Sudeep Holla]

ACPI 6.0 adds support for optional processor container device which may
contain child objects that are either processor devices or other processor
containers. This allows representing hierarchical processor topologies.

It is declared using the _HID of ACPI0010. It may also have _CID of
PNP0A05, which represents a generic container device.

This patch enables the support for these ACPI processor containers.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 Documentation/memory-hotplug.txt | 2 +-
 drivers/acpi/container.c         | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/memory-hotplug.txt b/Documentation/memory-hotplug.txt
index ce2cfcf35c27..bdbf7bb88d5d 100644
--- a/Documentation/memory-hotplug.txt
+++ b/Documentation/memory-hotplug.txt
@@ -124,7 +124,7 @@ config options.
 
 - As a related configuration, if your box has a feature of NUMA-node hotplug
   via ACPI, then this option is necessary too.
-    ACPI0004,PNP0A05 and PNP0A06 Container Driver (under ACPI Support menu)
+    ACPI0004, ACPI0010, PNP0A05 and PNP0A06 Container Driver (under ACPI Support menu)
     (CONFIG_ACPI_CONTAINER).
     This option can be kernel module too.
 
diff --git a/drivers/acpi/container.c b/drivers/acpi/container.c
index c8ead9f97375..8c041b9ac359 100644
--- a/drivers/acpi/container.c
+++ b/drivers/acpi/container.c
@@ -36,6 +36,7 @@ ACPI_MODULE_NAME("container");
 
 static const struct acpi_device_id container_device_ids[] = {
 	{"ACPI0004", 0},
+	{"ACPI0010", 0},
 	{"PNP0A05", 0},
 	{"PNP0A06", 0},
 	{"", 0},
-- 
1.9.1

[PATCH RFC 2/5] ACPI / processor: always compile perflib if CONFIG_ACPI_PROCESSOR

[Sudeep Holla]

Similar to the  idle, thermal and throttling libraries, always compile
the perflib if CONFIG_ACPI_PROCESSOR is enabled. This not only makes
perflib alligned with other libraries but also helps in some sanity
testing of these ACPI methods even when a particular feature is not
enabled in the kernel configuration.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/acpi/Makefile    |  2 +-
 include/acpi/processor.h | 29 -----------------------------
 include/linux/cpufreq.h  |  4 ++++
 3 files changed, 5 insertions(+), 30 deletions(-)

diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 8a063e276530..33aef9d8b260 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -84,7 +84,7 @@ obj-$(CONFIG_ACPI_BGRT)		+= bgrt.o
 # processor has its own "processor." module_param namespace
 processor-y			:= processor_driver.o processor_throttling.o
 processor-y			+= processor_idle.o processor_thermal.o
-processor-$(CONFIG_CPU_FREQ)	+= processor_perflib.o
+processor-y			+= processor_perflib.o
 
 obj-$(CONFIG_ACPI_PROCESSOR_AGGREGATOR) += acpi_pad.o
 
diff --git a/include/acpi/processor.h b/include/acpi/processor.h
index 4188a4d3b597..42a7ca744ff1 100644
--- a/include/acpi/processor.h
+++ b/include/acpi/processor.h
@@ -275,39 +275,10 @@ static inline void acpi_processor_ffh_cstate_enter(struct acpi_processor_cx
 
 /* in processor_perflib.c */
 
-#ifdef CONFIG_CPU_FREQ
 void acpi_processor_ppc_init(void);
 void acpi_processor_ppc_exit(void);
 int acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag);
 extern int acpi_processor_get_bios_limit(int cpu, unsigned int *limit);
-#else
-static inline void acpi_processor_ppc_init(void)
-{
-	return;
-}
-static inline void acpi_processor_ppc_exit(void)
-{
-	return;
-}
-static inline int acpi_processor_ppc_has_changed(struct acpi_processor *pr,
-								int event_flag)
-{
-	static unsigned int printout = 1;
-	if (printout) {
-		printk(KERN_WARNING
-		       "Warning: Processor Platform Limit event detected, but not handled.\n");
-		printk(KERN_WARNING
-		       "Consider compiling CPUfreq support into your kernel.\n");
-		printout = 0;
-	}
-	return 0;
-}
-static inline int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)
-{
-	return -ENODEV;
-}
-
-#endif				/* CONFIG_CPU_FREQ */
 
 /* in processor_core.c */
 phys_cpuid_t acpi_get_phys_id(acpi_handle, int type, u32 acpi_id);
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 2ee4888c1f47..ee91b295350b 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -171,6 +171,10 @@ static inline unsigned int cpufreq_quick_get_max(unsigned int cpu)
 {
 	return 0;
 }
+static inline int cpufreq_update_policy(unsigned int cpu)
+{
+	return -ENODEV;
+}
 static inline void disable_cpufreq(void) { }
 #endif
 
-- 
1.9.1

[PATCH RFC 3/5] ACPI / sleep: move acpi_processor_sleep to sleep.c

[Sudeep Holla]

acpi_processor_sleep is neither related nor used by CPUIdle framework.
It's used in system suspend/resume path as a syscore operation. It makes
more sense to move it to acpi/sleep.c where all the S-state transition
(a.k.a. Linux system suspend/hiberate) related code are present.

Also make it depend on CONFIG_ACPI_SYSTEM_POWER_STATES_SUPPORT so that
it's not compiled on architecture like ARM64 where S-states are not
yet defined in ACPI.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/acpi/processor_idle.c | 37 -------------------------------------
 drivers/acpi/sleep.c          | 37 +++++++++++++++++++++++++++++++++++++
 include/acpi/processor.h      |  2 +-
 3 files changed, 38 insertions(+), 38 deletions(-)

diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 39e0c8e36244..38bdbf36bf4e 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -34,7 +34,6 @@
 #include <linux/sched.h>       /* need_resched() */
 #include <linux/tick.h>
 #include <linux/cpuidle.h>
-#include <linux/syscore_ops.h>
 #include <acpi/processor.h>
 
 /*
@@ -198,42 +197,6 @@ static void lapic_timer_state_broadcast(struct acpi_processor *pr,
 
 #endif
 
-#ifdef CONFIG_PM_SLEEP
-static u32 saved_bm_rld;
-
-static int acpi_processor_suspend(void)
-{
-	acpi_read_bit_register(ACPI_BITREG_BUS_MASTER_RLD, &saved_bm_rld);
-	return 0;
-}
-
-static void acpi_processor_resume(void)
-{
-	u32 resumed_bm_rld = 0;
-
-	acpi_read_bit_register(ACPI_BITREG_BUS_MASTER_RLD, &resumed_bm_rld);
-	if (resumed_bm_rld == saved_bm_rld)
-		return;
-
-	acpi_write_bit_register(ACPI_BITREG_BUS_MASTER_RLD, saved_bm_rld);
-}
-
-static struct syscore_ops acpi_processor_syscore_ops = {
-	.suspend = acpi_processor_suspend,
-	.resume = acpi_processor_resume,
-};
-
-void acpi_processor_syscore_init(void)
-{
-	register_syscore_ops(&acpi_processor_syscore_ops);
-}
-
-void acpi_processor_syscore_exit(void)
-{
-	unregister_syscore_ops(&acpi_processor_syscore_ops);
-}
-#endif /* CONFIG_PM_SLEEP */
-
 #if defined(CONFIG_X86)
 static void tsc_check_state(int state)
 {
diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index 2f0d4db40a9e..cfea056e3d82 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -19,6 +19,7 @@
 #include <linux/reboot.h>
 #include <linux/acpi.h>
 #include <linux/module.h>
+#include <linux/syscore_ops.h>
 #include <asm/io.h>
 #include <trace/events/power.h>
 
@@ -672,6 +673,42 @@ static void acpi_sleep_suspend_setup(void)
 static inline void acpi_sleep_suspend_setup(void) {}
 #endif /* !CONFIG_SUSPEND */
 
+#ifdef CONFIG_PM_SLEEP
+static u32 saved_bm_rld;
+
+static int acpi_processor_suspend(void)
+{
+	acpi_read_bit_register(ACPI_BITREG_BUS_MASTER_RLD, &saved_bm_rld);
+	return 0;
+}
+
+static void acpi_processor_resume(void)
+{
+	u32 resumed_bm_rld = 0;
+
+	acpi_read_bit_register(ACPI_BITREG_BUS_MASTER_RLD, &resumed_bm_rld);
+	if (resumed_bm_rld == saved_bm_rld)
+		return;
+
+	acpi_write_bit_register(ACPI_BITREG_BUS_MASTER_RLD, saved_bm_rld);
+}
+
+static struct syscore_ops acpi_processor_syscore_ops = {
+	.suspend = acpi_processor_suspend,
+	.resume = acpi_processor_resume,
+};
+
+void acpi_processor_syscore_init(void)
+{
+	register_syscore_ops(&acpi_processor_syscore_ops);
+}
+
+void acpi_processor_syscore_exit(void)
+{
+	unregister_syscore_ops(&acpi_processor_syscore_ops);
+}
+#endif /* CONFIG_PM_SLEEP */
+
 #ifdef CONFIG_HIBERNATION
 static unsigned long s4_hardware_signature;
 static struct acpi_table_facs *facs;
diff --git a/include/acpi/processor.h b/include/acpi/processor.h
index 42a7ca744ff1..ffe675caa766 100644
--- a/include/acpi/processor.h
+++ b/include/acpi/processor.h
@@ -308,7 +308,7 @@ int acpi_processor_cst_has_changed(struct acpi_processor *pr);
 int acpi_processor_hotplug(struct acpi_processor *pr);
 extern struct cpuidle_driver acpi_idle_driver;
 
-#ifdef CONFIG_PM_SLEEP
+#if defined(CONFIG_PM_SLEEP) && defined(CONFIG_ACPI_SYSTEM_POWER_STATES_SUPPORT)
 void acpi_processor_syscore_init(void);
 void acpi_processor_syscore_exit(void);
 #else
-- 
1.9.1

[PATCH RFC 4/5] ACPI / processor_idle: replace PREFIX with pr_fmt

[Sudeep Holla]

Like few of the other ACPI modules, replace PREFIX with pr_fmt and
change all the printk call sites to use pr_* companion functions
in processor_idle.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/acpi/processor_idle.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 38bdbf36bf4e..866179651dd9 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -27,6 +27,7 @@
  *
  * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  */
+#define pr_fmt(fmt) "ACPI: " fmt
 
 #include <linux/module.h>
 #include <linux/acpi.h>
@@ -46,8 +47,6 @@
 #include <asm/apic.h>
 #endif
 
-#define PREFIX "ACPI: "
-
 #define ACPI_PROCESSOR_CLASS            "processor"
 #define _COMPONENT              ACPI_PROCESSOR_COMPONENT
 ACPI_MODULE_NAME("processor_idle");
@@ -84,9 +83,9 @@ static int set_max_cstate(const struct dmi_system_id *id)
 	if (max_cstate > ACPI_PROCESSOR_MAX_POWER)
 		return 0;
 
-	printk(KERN_NOTICE PREFIX "%s detected - limiting to C%ld max_cstate."
-	       " Override with \"processor.max_cstate=%d\"\n", id->ident,
-	       (long)id->driver_data, ACPI_PROCESSOR_MAX_POWER + 1);
+	pr_notice("%s detected - limiting to C%ld max_cstate."
+		  " Override with \"processor.max_cstate=%d\"\n", id->ident,
+		  (long)id->driver_data, ACPI_PROCESSOR_MAX_POWER + 1);
 
 	max_cstate = (long)id->driver_data;
 
@@ -318,7 +317,7 @@ static int acpi_processor_get_power_info_cst(struct acpi_processor *pr)
 
 	/* There must be at least 2 elements */
 	if (!cst || (cst->type != ACPI_TYPE_PACKAGE) || cst->package.count < 2) {
-		printk(KERN_ERR PREFIX "not enough elements in _CST\n");
+		pr_err("not enough elements in _CST\n");
 		ret = -EFAULT;
 		goto end;
 	}
@@ -327,7 +326,7 @@ static int acpi_processor_get_power_info_cst(struct acpi_processor *pr)
 
 	/* Validate number of power states. */
 	if (count < 1 || count != cst->package.count - 1) {
-		printk(KERN_ERR PREFIX "count given by _CST is not valid\n");
+		pr_err("count given by _CST is not valid\n");
 		ret = -EFAULT;
 		goto end;
 	}
@@ -436,11 +435,9 @@ static int acpi_processor_get_power_info_cst(struct acpi_processor *pr)
 		 * (From 1 through ACPI_PROCESSOR_MAX_POWER - 1)
 		 */
 		if (current_count >= (ACPI_PROCESSOR_MAX_POWER - 1)) {
-			printk(KERN_WARNING
-			       "Limiting number of power states to max (%d)\n",
-			       ACPI_PROCESSOR_MAX_POWER);
-			printk(KERN_WARNING
-			       "Please increase ACPI_PROCESSOR_MAX_POWER if needed.\n");
+			pr_warn("Limiting number of power states to max (%d)\n",
+				ACPI_PROCESSOR_MAX_POWER);
+			pr_warn("Please increase ACPI_PROCESSOR_MAX_POWER if needed.\n");
 			break;
 		}
 	}
@@ -1064,8 +1061,8 @@ int acpi_processor_power_init(struct acpi_processor *pr)
 			retval = cpuidle_register_driver(&acpi_idle_driver);
 			if (retval)
 				return retval;
-			printk(KERN_DEBUG "ACPI: %s registered with cpuidle\n",
-					acpi_idle_driver.name);
+			pr_debug("%s registered with cpuidle\n",
+				 acpi_idle_driver.name);
 		}
 
 		dev = kzalloc(sizeof(*dev), GFP_KERNEL);
-- 
1.9.1

[PATCH RFC 5/5] ACPI / processor_idle : introduce ARCH_SUPPORTS_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 ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE to
encapsulate all the code supporting the traditional C-states.

This patch will help to extend the processor_idle module to support
LPI.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/ia64/Kconfig             |  1 +
 arch/x86/Kconfig              |  1 +
 arch/x86/include/asm/acpi.h   |  2 --
 drivers/acpi/Kconfig          |  3 +++
 drivers/acpi/processor_idle.c | 57 +++++++++++++++++++++++++++++++------------
 include/acpi/processor.h      |  2 +-
 6 files changed, 48 insertions(+), 18 deletions(-)

diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
index 76d25b2cfbbe..6d7567efeecc 100644
--- a/arch/ia64/Kconfig
+++ b/arch/ia64/Kconfig
@@ -16,6 +16,7 @@ config IA64
 	select PCI if (!IA64_HP_SIM)
 	select ACPI if (!IA64_HP_SIM)
 	select ACPI_SYSTEM_POWER_STATES_SUPPORT if ACPI
+	select ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE if ACPI
 	select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI
 	select HAVE_UNSTABLE_SCHED_CLOCK
 	select HAVE_IDE
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 226d5696e1d1..b5d58c488409 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -23,6 +23,7 @@ config X86_64
 config X86
 	def_bool y
 	select ACPI_SYSTEM_POWER_STATES_SUPPORT if ACPI
+	select ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE if ACPI
 	select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI
 	select ARCH_HAS_DEBUG_STRICT_USER_COPY_CHECKS
 	select ARCH_HAS_FAST_MULTIPLIER
diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h
index 3a45668f6dc3..0077b318e3fa 100644
--- a/arch/x86/include/asm/acpi.h
+++ b/arch/x86/include/asm/acpi.h
@@ -138,8 +138,6 @@ static inline void disable_acpi(void) { }
 
 #endif /* !CONFIG_ACPI */
 
-#define ARCH_HAS_POWER_INIT	1
-
 #ifdef CONFIG_ACPI_NUMA
 extern int acpi_numa;
 extern int x86_acpi_numa_init(void);
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index ab2cbb51c6aa..9d6885e07040 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -48,6 +48,9 @@ config ACPI_LEGACY_TABLES_LOOKUP
 config ARCH_MIGHT_HAVE_ACPI_PDC
 	bool
 
+config ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE
+	bool
+
 config ACPI_GENERIC_GSI
 	bool
 
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 866179651dd9..55bd1fd21a6d 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -63,6 +63,12 @@ module_param(latency_factor, uint, 0644);
 
 static DEFINE_PER_CPU(struct cpuidle_device *, acpi_cpuidle_device);
 
+struct cpuidle_driver acpi_idle_driver = {
+	.name =		"acpi_idle",
+	.owner =	THIS_MODULE,
+};
+
+#ifdef CONFIG_ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE
 static DEFINE_PER_CPU(struct acpi_processor_cx * [CPUIDLE_STATE_MAX],
 								acpi_cstate);
 
@@ -808,11 +814,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
@@ -929,6 +930,41 @@ static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr)
 	return 0;
 }
 
+static inline void acpi_processor_cstate_first_run_checks(void)
+{
+	static int first_run;
+
+	if (first_run)
+		return;
+	dmi_check_system(processor_power_dmi_table);
+	max_cstate = acpi_processor_cstate_check(max_cstate);
+	if (max_cstate < ACPI_C_STATES_MAX)
+		pr_notice("ACPI: processor limited to max C-state %d\n",
+			  max_cstate);
+	first_run++;
+}
+#else
+
+static inline int disabled_by_idle_boot_param(void) { return 0; }
+static inline void acpi_processor_cstate_first_run_checks(void) { }
+static int acpi_processor_get_power_info(struct acpi_processor *pr)
+{
+	return -ENODEV;
+}
+
+static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr,
+					   struct cpuidle_device *dev)
+{
+	return -EINVAL;
+}
+
+static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr)
+{
+	return -EINVAL;
+}
+
+#endif
+
 int acpi_processor_hotplug(struct acpi_processor *pr)
 {
 	int ret = 0;
@@ -1022,20 +1058,11 @@ int acpi_processor_power_init(struct acpi_processor *pr)
 	acpi_status status;
 	int retval;
 	struct cpuidle_device *dev;
-	static int first_run;
 
 	if (disabled_by_idle_boot_param())
 		return 0;
 
-	if (!first_run) {
-		dmi_check_system(processor_power_dmi_table);
-		max_cstate = acpi_processor_cstate_check(max_cstate);
-		if (max_cstate < ACPI_C_STATES_MAX)
-			printk(KERN_NOTICE
-			       "ACPI: processor limited to max C-state %d\n",
-			       max_cstate);
-		first_run++;
-	}
+	acpi_processor_cstate_first_run_checks();
 
 	if (acpi_gbl_FADT.cst_control && !nocst) {
 		status =
diff --git a/include/acpi/processor.h b/include/acpi/processor.h
index ffe675caa766..32d417d3df04 100644
--- a/include/acpi/processor.h
+++ b/include/acpi/processor.h
@@ -244,7 +244,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
+#ifdef CONFIG_ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE
 void acpi_processor_power_init_bm_check(struct acpi_processor_flags *flags,
 					unsigned int cpu);
 int acpi_processor_ffh_cstate_probe(unsigned int cpu,
-- 
1.9.1

Re: [PATCH RFC 2/5] ACPI / processor: always compile perflib if CONFIG_ACPI_PROCESSOR

[Ashwin Chaugule]

Hello,

On 6 May 2015 at 10:31, Sudeep Holla <sudeep.holla@arm.com> wrote:
> Similar to the  idle, thermal and throttling libraries, always compile
> the perflib if CONFIG_ACPI_PROCESSOR is enabled. This not only makes
> perflib alligned with other libraries but also helps in some sanity
> testing of these ACPI methods even when a particular feature is not
> enabled in the kernel configuration.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  drivers/acpi/Makefile    |  2 +-
>  include/acpi/processor.h | 29 -----------------------------
>  include/linux/cpufreq.h  |  4 ++++
>  3 files changed, 5 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index 8a063e276530..33aef9d8b260 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -84,7 +84,7 @@ obj-$(CONFIG_ACPI_BGRT)               += bgrt.o
>  # processor has its own "processor." module_param namespace
>  processor-y                    := processor_driver.o processor_throttling.o
>  processor-y                    += processor_idle.o processor_thermal.o
> -processor-$(CONFIG_CPU_FREQ)   += processor_perflib.o
> +processor-y                    += processor_perflib.o

I'd prefer that we create a separate kconfig option for this. (perhaps
even default it to 'y'). This library is quite specific to a certain
type of CPU performance management methods (includes _PSS and friends)
which are superseded by CPPC. The OS is not expected to support both
at runtime, so by keeping this a config option, we can then disable it
at compile time when CPPC is enabled. We could couple
processor_throttling, thermal and perflib under this config option
(CONFIG_PSS ?) since they're all under the same CPU performance
management umbrella. Thoughts?

Regards,
Ashwin.

Re: [PATCH RFC 1/5] ACPI / containers : add support for ACPI0010 processor container

[Hanjun Guo]

Hi Sudeep,

On 2015年05月06日 22:31, Sudeep Holla wrote:
> ACPI 6.0 adds support for optional processor container device which may
> contain child objects that are either processor devices or other processor
> containers. This allows representing hierarchical processor topologies.
>
> It is declared using the _HID of ACPI0010. It may also have _CID of
> PNP0A05, which represents a generic container device.

Container device helps support hotplug of nodes, CPUs, and memory,
does this container device ACPI0010 used for the same purpose?

For my understanding, ACPI0010 is used to abstract CPU topology
like cluster, not for hotplug purpose, maybe I missed something,
or you just need this container driver for later use?

Thanks
Hanjun

>
> This patch enables the support for these ACPI processor containers.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>   Documentation/memory-hotplug.txt | 2 +-
>   drivers/acpi/container.c         | 1 +
>   2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/memory-hotplug.txt b/Documentation/memory-hotplug.txt
> index ce2cfcf35c27..bdbf7bb88d5d 100644
> --- a/Documentation/memory-hotplug.txt
> +++ b/Documentation/memory-hotplug.txt
> @@ -124,7 +124,7 @@ config options.
>
>   - As a related configuration, if your box has a feature of NUMA-node hotplug
>     via ACPI, then this option is necessary too.
> -    ACPI0004,PNP0A05 and PNP0A06 Container Driver (under ACPI Support menu)
> +    ACPI0004, ACPI0010, PNP0A05 and PNP0A06 Container Driver (under ACPI Support menu)
>       (CONFIG_ACPI_CONTAINER).
>       This option can be kernel module too.
>
> diff --git a/drivers/acpi/container.c b/drivers/acpi/container.c
> index c8ead9f97375..8c041b9ac359 100644
> --- a/drivers/acpi/container.c
> +++ b/drivers/acpi/container.c
> @@ -36,6 +36,7 @@ ACPI_MODULE_NAME("container");
>
>   static const struct acpi_device_id container_device_ids[] = {
>   	{"ACPI0004", 0},
> +	{"ACPI0010", 0},
>   	{"PNP0A05", 0},
>   	{"PNP0A06", 0},
>   	{"", 0},
>

Re: [PATCH RFC 2/5] ACPI / processor: always compile perflib if CONFIG_ACPI_PROCESSOR

[Hanjun Guo]

On 2015年05月07日 02:36, Ashwin Chaugule wrote:
> Hello,
>
> On 6 May 2015 at 10:31, Sudeep Holla <sudeep.holla@arm.com> wrote:
>> Similar to the  idle, thermal and throttling libraries, always compile
>> the perflib if CONFIG_ACPI_PROCESSOR is enabled. This not only makes
>> perflib alligned with other libraries but also helps in some sanity
>> testing of these ACPI methods even when a particular feature is not
>> enabled in the kernel configuration.
>>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>> ---
>>   drivers/acpi/Makefile    |  2 +-
>>   include/acpi/processor.h | 29 -----------------------------
>>   include/linux/cpufreq.h  |  4 ++++
>>   3 files changed, 5 insertions(+), 30 deletions(-)
>>
>> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
>> index 8a063e276530..33aef9d8b260 100644
>> --- a/drivers/acpi/Makefile
>> +++ b/drivers/acpi/Makefile
>> @@ -84,7 +84,7 @@ obj-$(CONFIG_ACPI_BGRT)               += bgrt.o
>>   # processor has its own "processor." module_param namespace
>>   processor-y                    := processor_driver.o processor_throttling.o
>>   processor-y                    += processor_idle.o processor_thermal.o
>> -processor-$(CONFIG_CPU_FREQ)   += processor_perflib.o
>> +processor-y                    += processor_perflib.o
>
> I'd prefer that we create a separate kconfig option for this. (perhaps
> even default it to 'y'). This library is quite specific to a certain
> type of CPU performance management methods (includes _PSS and friends)
> which are superseded by CPPC. The OS is not expected to support both
> at runtime, so by keeping this a config option, we can then disable it
> at compile time when CPPC is enabled. We could couple

I agree. CPPC and _PSS are different way of controlling CPU freq,
and I think _PSS may not be used on ARM.

Thanks
Hanjun

Re: [PATCH RFC 2/5] ACPI / processor: always compile perflib if CONFIG_ACPI_PROCESSOR

[Sudeep Holla]

On 08/05/15 09:52, Hanjun Guo wrote:
> On 2015年05月07日 02:36, Ashwin Chaugule wrote:
>> Hello,
>>
>> On 6 May 2015 at 10:31, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>> Similar to the  idle, thermal and throttling libraries, always compile
>>> the perflib if CONFIG_ACPI_PROCESSOR is enabled. This not only makes
>>> perflib alligned with other libraries but also helps in some sanity
>>> testing of these ACPI methods even when a particular feature is not
>>> enabled in the kernel configuration.
>>>
>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>>> ---
>>>    drivers/acpi/Makefile    |  2 +-
>>>    include/acpi/processor.h | 29 -----------------------------
>>>    include/linux/cpufreq.h  |  4 ++++
>>>    3 files changed, 5 insertions(+), 30 deletions(-)
>>>
>>> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
>>> index 8a063e276530..33aef9d8b260 100644
>>> --- a/drivers/acpi/Makefile
>>> +++ b/drivers/acpi/Makefile
>>> @@ -84,7 +84,7 @@ obj-$(CONFIG_ACPI_BGRT)               += bgrt.o
>>>    # processor has its own "processor." module_param namespace
>>>    processor-y                    := processor_driver.o processor_throttling.o
>>>    processor-y                    += processor_idle.o processor_thermal.o
>>> -processor-$(CONFIG_CPU_FREQ)   += processor_perflib.o
>>> +processor-y                    += processor_perflib.o
>>
>> I'd prefer that we create a separate kconfig option for this. (perhaps
>> even default it to 'y'). This library is quite specific to a certain
>> type of CPU performance management methods (includes _PSS and friends)
>> which are superseded by CPPC. The OS is not expected to support both
>> at runtime, so by keeping this a config option, we can then disable it
>> at compile time when CPPC is enabled. We could couple
>
> I agree. CPPC and _PSS are different way of controlling CPU freq,
> and I think _PSS may not be used on ARM.
>

While I agree having a separate config option is good, but I won't
assume _PSS might not be used on ARM as I have seen patches posted on
the list in past to use _PSS on ARM platform[1].

Regards,
Sudeep

[1] http://marc.info/?l=linaro-acpi&m=139745485418399&w=2

Re: [PATCH RFC 1/5] ACPI / containers : add support for ACPI0010 processor container

[Rafael J. Wysocki]

On Friday, May 08, 2015 04:50:10 PM Hanjun Guo wrote:
> Hi Sudeep,
> 
> On 2015年05月06日 22:31, Sudeep Holla wrote:
> > ACPI 6.0 adds support for optional processor container device which may
> > contain child objects that are either processor devices or other processor
> > containers. This allows representing hierarchical processor topologies.
> >
> > It is declared using the _HID of ACPI0010. It may also have _CID of
> > PNP0A05, which represents a generic container device.
> 
> Container device helps support hotplug of nodes, CPUs, and memory,
> does this container device ACPI0010 used for the same purpose?

That's correct and the patch isn't.

Thanks!


> For my understanding, ACPI0010 is used to abstract CPU topology
> like cluster, not for hotplug purpose, maybe I missed something,
> or you just need this container driver for later use?
> 
> Thanks
> Hanjun
> 
> >
> > This patch enables the support for these ACPI processor containers.
> >
> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > ---
> >   Documentation/memory-hotplug.txt | 2 +-
> >   drivers/acpi/container.c         | 1 +
> >   2 files changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/memory-hotplug.txt b/Documentation/memory-hotplug.txt
> > index ce2cfcf35c27..bdbf7bb88d5d 100644
> > --- a/Documentation/memory-hotplug.txt
> > +++ b/Documentation/memory-hotplug.txt
> > @@ -124,7 +124,7 @@ config options.
> >
> >   - As a related configuration, if your box has a feature of NUMA-node hotplug
> >     via ACPI, then this option is necessary too.
> > -    ACPI0004,PNP0A05 and PNP0A06 Container Driver (under ACPI Support menu)
> > +    ACPI0004, ACPI0010, PNP0A05 and PNP0A06 Container Driver (under ACPI Support menu)
> >       (CONFIG_ACPI_CONTAINER).
> >       This option can be kernel module too.
> >
> > diff --git a/drivers/acpi/container.c b/drivers/acpi/container.c
> > index c8ead9f97375..8c041b9ac359 100644
> > --- a/drivers/acpi/container.c
> > +++ b/drivers/acpi/container.c
> > @@ -36,6 +36,7 @@ ACPI_MODULE_NAME("container");
> >
> >   static const struct acpi_device_id container_device_ids[] = {
> >   	{"ACPI0004", 0},
> > +	{"ACPI0010", 0},
> >   	{"PNP0A05", 0},
> >   	{"PNP0A06", 0},
> >   	{"", 0},
> >

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH RFC 1/5] ACPI / containers : add support for ACPI0010 processor container

[Sudeep Holla]

On 08/05/15 15:06, Rafael J. Wysocki wrote:
> On Friday, May 08, 2015 04:50:10 PM Hanjun Guo wrote:
>> Hi Sudeep,
>>
>> On 2015年05月06日 22:31, Sudeep Holla wrote:
>>> ACPI 6.0 adds support for optional processor container device which may
>>> contain child objects that are either processor devices or other processor
>>> containers. This allows representing hierarchical processor topologies.
>>>
>>> It is declared using the _HID of ACPI0010. It may also have _CID of
>>> PNP0A05, which represents a generic container device.
>>
>> Container device helps support hotplug of nodes, CPUs, and memory,
>> does this container device ACPI0010 used for the same purpose?
>
> That's correct and the patch isn't.
>

Thanks Rafael for the clarification.

Just curious if the firmware adds _CID of PNP0A05 to support OS that
don't parse processor containers, will the current code not create
containers using _CID ?

Regards,
Sudeep

Re: [PATCH RFC 1/5] ACPI / containers : add support for ACPI0010 processor container

[Rafael J. Wysocki]

On Friday, May 08, 2015 04:32:33 PM Sudeep Holla wrote:
> 
> On 08/05/15 15:06, Rafael J. Wysocki wrote:
> > On Friday, May 08, 2015 04:50:10 PM Hanjun Guo wrote:
> >> Hi Sudeep,
> >>
> >> On 2015年05月06日 22:31, Sudeep Holla wrote:
> >>> ACPI 6.0 adds support for optional processor container device which may
> >>> contain child objects that are either processor devices or other processor
> >>> containers. This allows representing hierarchical processor topologies.
> >>>
> >>> It is declared using the _HID of ACPI0010. It may also have _CID of
> >>> PNP0A05, which represents a generic container device.
> >>
> >> Container device helps support hotplug of nodes, CPUs, and memory,
> >> does this container device ACPI0010 used for the same purpose?
> >
> > That's correct and the patch isn't.
> >
> 
> Thanks Rafael for the clarification.
> 
> Just curious if the firmware adds _CID of PNP0A05 to support OS that
> don't parse processor containers, will the current code not create
> containers using _CID ?

Yes, it will, but you can prevent that from happening by adding a new
scan handler just for ACPI0010 (that may just return 1 from its ->attach
callback and do nothing else).


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.