Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1397529 > unrolled thread
| Started by | "Rafael J. Wysocki" <rjw@rjwysocki.net> |
|---|---|
| First post | 2016-05-10 02:00 +0200 |
| Last post | 2016-05-11 17:10 +0200 |
| Articles | 2 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH v4 1/5] ACPI / processor_idle: introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-05-10 02:00 +0200
Re: [PATCH v4 1/5] ACPI / processor_idle: introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE Sudeep Holla <sudeep.holla@arm.com> - 2016-05-11 17:10 +0200
| From | "Rafael J. Wysocki" <rjw@rjwysocki.net> |
|---|---|
| Date | 2016-05-10 02:00 +0200 |
| Subject | Re: [PATCH v4 1/5] ACPI / processor_idle: introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE |
| Message-ID | <rx3fI-7Ae-11@gated-at.bofh.it> |
On Tuesday, April 19, 2016 01:30:09 PM Sudeep Holla wrote:
> ACPI 6.0 adds a new method to specify the CPU idle states(C-states)
> called Low Power Idle(LPI) states. Since new architectures like ARM64
> use only LPIs, introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE to
> encapsulate all the code supporting the old style C-states(_CST)
>
> This patch will help to extend the processor_idle module to support
> LPI.
>
> Cc: x86@kernel.org
> Cc: linux-ia64@vger.kernel.org
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> arch/ia64/Kconfig | 1 +
> arch/x86/Kconfig | 1 +
> drivers/acpi/Kconfig | 3 ++
> drivers/acpi/processor_idle.c | 74 +++++++++++++++++++++++++++++--------------
> include/acpi/processor.h | 3 +-
> 5 files changed, 57 insertions(+), 25 deletions(-)
>
> diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
> index b534ebab36ea..717de2a146e2 100644
> --- a/arch/ia64/Kconfig
> +++ b/arch/ia64/Kconfig
> @@ -16,6 +16,7 @@ config IA64
> select PCI if (!IA64_HP_SIM)
> select ACPI if (!IA64_HP_SIM)
> select ACPI_SYSTEM_POWER_STATES_SUPPORT if ACPI
> + select ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE if ACPI
> select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI
> select HAVE_UNSTABLE_SCHED_CLOCK
> select HAVE_IDE
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 2dc18605831f..eb03fd0d63b9 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -37,6 +37,7 @@ config X86
> select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI
> select ARCH_MIGHT_HAVE_PC_PARPORT
> select ARCH_MIGHT_HAVE_PC_SERIO
> + select ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE if ACPI
> select ARCH_SUPPORTS_ATOMIC_RMW
> select ARCH_SUPPORTS_DEFERRED_STRUCT_PAGE_INIT
> select ARCH_SUPPORTS_INT128 if X86_64
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 82b96ee8624c..ec289078667c 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -48,6 +48,9 @@ config ACPI_LEGACY_TABLES_LOOKUP
> config ARCH_MIGHT_HAVE_ACPI_PDC
> bool
>
> +config ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE
> + bool
It might be better to do
config ACPI_PROCESSOR_CST
bool
depends on ia64 || x86
Should it depend on ARCH_HAS_POWER_INIT too?
> +
> config ACPI_GENERIC_GSI
> bool
>
> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
> index 444e3745c8b3..1f3fe54194b5 100644
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -59,6 +59,12 @@ module_param(latency_factor, uint, 0644);
>
> static DEFINE_PER_CPU(struct cpuidle_device *, acpi_cpuidle_device);
>
> +struct cpuidle_driver acpi_idle_driver = {
> + .name = "acpi_idle",
> + .owner = THIS_MODULE,
> +};
> +
> +#ifdef CONFIG_ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE
> static
> DEFINE_PER_CPU(struct acpi_processor_cx * [CPUIDLE_STATE_MAX], acpi_cstate);
>
> @@ -804,11 +810,6 @@ static void acpi_idle_enter_freeze(struct cpuidle_device *dev,
> acpi_idle_do_entry(cx);
> }
>
> -struct cpuidle_driver acpi_idle_driver = {
> - .name = "acpi_idle",
> - .owner = THIS_MODULE,
> -};
> -
> /**
> * acpi_processor_setup_cpuidle_cx - prepares and configures CPUIDLE
> * device i.e. per-cpu data
> @@ -925,6 +926,49 @@ static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr)
> return 0;
> }
>
> +static inline void acpi_processor_cstate_first_run_checks(void)
> +{
> + static int first_run;
> +
> + if (first_run)
> + return;
> + dmi_check_system(processor_power_dmi_table);
> + max_cstate = acpi_processor_cstate_check(max_cstate);
> + if (max_cstate < ACPI_C_STATES_MAX)
> + pr_notice("ACPI: processor limited to max C-state %d\n",
> + max_cstate);
> + first_run++;
> +
> + if (acpi_gbl_FADT.cst_control && !nocst) {
> + status = acpi_os_write_port(acpi_gbl_FADT.smi_command,
> + acpi_gbl_FADT.cst_control, 8);
> + if (ACPI_FAILURE(status))
> + ACPI_EXCEPTION((AE_INFO, status,
> + "Notifying BIOS of _CST ability failed"));
> + }
> +}
> +#else
> +
> +static inline int disabled_by_idle_boot_param(void) { return 0; }
> +static inline void acpi_processor_cstate_first_run_checks(void) { }
> +static int acpi_processor_get_power_info(struct acpi_processor *pr)
> +{
> + return -ENODEV;
> +}
> +
> +static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr,
> + struct cpuidle_device *dev)
> +{
> + return -EINVAL;
> +}
> +
> +static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr)
> +{
> + return -EINVAL;
> +}
> +
> +#endif
Add a comment indicating what #ifdef block ends here?
> +
> int acpi_processor_hotplug(struct acpi_processor *pr)
> {
> int ret = 0;
> @@ -1018,29 +1062,11 @@ int acpi_processor_power_init(struct acpi_processor *pr)
> acpi_status status;
> int retval;
> struct cpuidle_device *dev;
> - static int first_run;
>
> if (disabled_by_idle_boot_param())
> return 0;
>
> - if (!first_run) {
> - dmi_check_system(processor_power_dmi_table);
> - max_cstate = acpi_processor_cstate_check(max_cstate);
> - if (max_cstate < ACPI_C_STATES_MAX)
> - printk(KERN_NOTICE
> - "ACPI: processor limited to max C-state %d\n",
> - max_cstate);
> - first_run++;
> - }
> -
> - if (acpi_gbl_FADT.cst_control && !nocst) {
> - status =
> - acpi_os_write_port(acpi_gbl_FADT.smi_command, acpi_gbl_FADT.cst_control, 8);
> - if (ACPI_FAILURE(status)) {
> - ACPI_EXCEPTION((AE_INFO, status,
> - "Notifying BIOS of _CST ability failed"));
> - }
> - }
> + acpi_processor_cstate_first_run_checks();
>
> acpi_processor_get_power_info(pr);
> pr->flags.power_setup_done = 1;
> diff --git a/include/acpi/processor.h b/include/acpi/processor.h
> index 6f1805dd5d3c..50f2423d31fa 100644
> --- a/include/acpi/processor.h
> +++ b/include/acpi/processor.h
> @@ -242,7 +242,8 @@ extern int acpi_processor_get_performance_info(struct acpi_processor *pr);
> DECLARE_PER_CPU(struct acpi_processor *, processors);
> extern struct acpi_processor_errata errata;
>
> -#ifdef ARCH_HAS_POWER_INIT
> +#if defined(ARCH_HAS_POWER_INIT) && \
> + defined(CONFIG_ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE)
> void acpi_processor_power_init_bm_check(struct acpi_processor_flags *flags,
> unsigned int cpu);
> int acpi_processor_ffh_cstate_probe(unsigned int cpu,
>
[toc] | [next] | [standalone]
| From | Sudeep Holla <sudeep.holla@arm.com> |
|---|---|
| Date | 2016-05-11 17:10 +0200 |
| Subject | Re: [PATCH v4 1/5] ACPI / processor_idle: introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE |
| Message-ID | <rxDVU-2hX-37@gated-at.bofh.it> |
| In reply to | #1397529 |
On 10/05/16 01:02, Rafael J. Wysocki wrote: > On Tuesday, April 19, 2016 01:30:09 PM Sudeep Holla wrote: >> ACPI 6.0 adds a new method to specify the CPU idle states(C-states) >> called Low Power Idle(LPI) states. Since new architectures like ARM64 >> use only LPIs, introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE to >> encapsulate all the code supporting the old style C-states(_CST) >> >> This patch will help to extend the processor_idle module to support >> LPI. >> >> Cc: x86@kernel.org >> Cc: linux-ia64@vger.kernel.org >> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> >> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> >> --- >> arch/ia64/Kconfig | 1 + >> arch/x86/Kconfig | 1 + >> drivers/acpi/Kconfig | 3 ++ >> drivers/acpi/processor_idle.c | 74 +++++++++++++++++++++++++++++-------------- >> include/acpi/processor.h | 3 +- >> 5 files changed, 57 insertions(+), 25 deletions(-) >> >> diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig >> index b534ebab36ea..717de2a146e2 100644 >> --- a/arch/ia64/Kconfig >> +++ b/arch/ia64/Kconfig >> @@ -16,6 +16,7 @@ config IA64 >> select PCI if (!IA64_HP_SIM) >> select ACPI if (!IA64_HP_SIM) >> select ACPI_SYSTEM_POWER_STATES_SUPPORT if ACPI >> + select ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE if ACPI >> select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI >> select HAVE_UNSTABLE_SCHED_CLOCK >> select HAVE_IDE >> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig >> index 2dc18605831f..eb03fd0d63b9 100644 >> --- a/arch/x86/Kconfig >> +++ b/arch/x86/Kconfig >> @@ -37,6 +37,7 @@ config X86 >> select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI >> select ARCH_MIGHT_HAVE_PC_PARPORT >> select ARCH_MIGHT_HAVE_PC_SERIO >> + select ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE if ACPI >> select ARCH_SUPPORTS_ATOMIC_RMW >> select ARCH_SUPPORTS_DEFERRED_STRUCT_PAGE_INIT >> select ARCH_SUPPORTS_INT128 if X86_64 >> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig >> index 82b96ee8624c..ec289078667c 100644 >> --- a/drivers/acpi/Kconfig >> +++ b/drivers/acpi/Kconfig >> @@ -48,6 +48,9 @@ config ACPI_LEGACY_TABLES_LOOKUP >> config ARCH_MIGHT_HAVE_ACPI_PDC >> bool >> >> +config ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE >> + bool > > It might be better to do > > config ACPI_PROCESSOR_CST > bool > depends on ia64 || x86 > Done > Should it depend on ARCH_HAS_POWER_INIT too? > IIUC it's not defined for ia64, so better not to add that dependency here IMO. include/acpi/processor.h encapsulates it. Let me know if I have misunderstood it. -- Regards, Sudeep
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web