* [PATCH 1/1] cpufreq: intel_pstate: skip the driver if ACPI has power mgmt option
@ 2013-10-21 5:27 Adrian Huang
2013-10-21 16:03 ` Dirk Brandewie
0 siblings, 1 reply; 4+ messages in thread
From: Adrian Huang @ 2013-10-21 5:27 UTC (permalink / raw)
To: rjw, viresh.kumar; +Cc: cpufreq, linux-pm, linda.knippers, Adrian Huang
Do not load the Intel pstate driver if the platform firmware
(ACPI BIOS) supports the power management alternatives.
The ACPI BIOS indicates that the OS control mode can be used
if the _PSS (Performance Supported States) object is defined
in ACPI table. For the OS control mode, the Intel pstate
driver will be loaded.
Signed-off-by: Adrian Huang <adrian.huang@hp.com>
---
drivers/cpufreq/intel_pstate.c | 80 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 80 insertions(+)
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index badf620..c03915e 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -26,6 +26,8 @@
#include <linux/fs.h>
#include <linux/debugfs.h>
#include <trace/events/power.h>
+#include <linux/acpi.h>
+#include <acpi/processor.h>
#include <asm/div64.h>
#include <asm/msr.h>
@@ -129,6 +131,18 @@ static struct perf_limits limits = {
.max_sysfs_pct = 100,
};
+struct hw_vendor_info {
+ u16 valid;
+ char oem_id[ACPI_OEM_ID_SIZE];
+ char oem_table_id[ACPI_OEM_TABLE_ID_SIZE];
+};
+
+/* Hardware vendor-specific info that has its own power management modes */
+static struct hw_vendor_info vendor_info[] = {
+ {1, "HP ", "ProLiant"},
+ {0, "", ""},
+};
+
static inline void pid_reset(struct _pid *pid, int setpoint, int busy,
int deadband, int integral) {
pid->setpoint = setpoint;
@@ -700,6 +714,63 @@ static int intel_pstate_msrs_not_valid(void)
return 0;
}
+
+static bool intel_pstate_no_acpi_pss(void)
+{
+ struct acpi_processor *pr;
+ union acpi_object *pss = NULL;
+ int i;
+
+ for_each_possible_cpu(i) {
+
+ pr = per_cpu(processors, i);
+
+ if (pr) {
+ struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER,
+ NULL };
+
+ if (ACPI_SUCCESS(acpi_evaluate_object(pr->handle,
+ "_PSS", NULL, &buffer))) {
+
+ pss = buffer.pointer;
+
+ if (pss && pss->type == ACPI_TYPE_PACKAGE) {
+ kfree(buffer.pointer);
+ return false;
+ }
+
+ kfree(buffer.pointer);
+ }
+ }
+ }
+
+ return true;
+}
+
+static bool intel_pstate_platform_pwr_mgmt_exists(void)
+{
+ struct acpi_table_header hdr;
+ struct hw_vendor_info *v_info;
+
+ if (acpi_get_table_header(ACPI_SIG_FADT, 0, &hdr) == AE_OK) {
+ for (v_info = vendor_info; v_info->valid; v_info++) {
+ /*
+ * Check if the hardware/platform is in the
+ * predefined vendor data.
+ */
+ if (!strncmp(hdr.oem_id, v_info->oem_id,
+ ACPI_OEM_ID_SIZE) &&
+ !strncmp(hdr.oem_table_id, v_info->oem_table_id,
+ ACPI_OEM_TABLE_ID_SIZE)) {
+ if (intel_pstate_no_acpi_pss())
+ return true;
+ }
+ }
+ }
+
+ return false;
+}
+
static int __init intel_pstate_init(void)
{
int cpu, rc = 0;
@@ -708,6 +779,15 @@ static int __init intel_pstate_init(void)
if (no_load)
return -ENODEV;
+ if (!acpi_disabled) {
+ /*
+ * Check if the platform has its own power management modes.
+ * If so, the pstate cpufreq driver will be ignored.
+ */
+ if (intel_pstate_platform_pwr_mgmt_exists())
+ return 0;
+ }
+
id = x86_match_cpu(intel_pstate_cpu_ids);
if (!id)
return -ENODEV;
--
1.8.1.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] cpufreq: intel_pstate: skip the driver if ACPI has power mgmt option
2013-10-21 5:27 [PATCH 1/1] cpufreq: intel_pstate: skip the driver if ACPI has power mgmt option Adrian Huang
@ 2013-10-21 16:03 ` Dirk Brandewie
2013-10-24 13:23 ` adrian
0 siblings, 1 reply; 4+ messages in thread
From: Dirk Brandewie @ 2013-10-21 16:03 UTC (permalink / raw)
To: Adrian Huang, rjw, viresh.kumar; +Cc: cpufreq, linux-pm, linda.knippers
On 10/20/2013 10:27 PM, Adrian Huang wrote:
> static int __init intel_pstate_init(void)
> {
> int cpu, rc = 0;
> @@ -708,6 +779,15 @@ static int __init intel_pstate_init(void)
> if (no_load)
> return -ENODEV;
>
> + if (!acpi_disabled) {
> + /*
> + * Check if the platform has its own power management modes.
> + * If so, the pstate cpufreq driver will be ignored.
> + */
> + if (intel_pstate_platform_pwr_mgmt_exists())
> + return 0;
Please return -ENODEV here instead of 0
> + }
> +
> id = x86_match_cpu(intel_pstate_cpu_ids);
> if (!id)
> return -ENODEV;
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] cpufreq: intel_pstate: skip the driver if ACPI has power mgmt option
2013-10-21 16:03 ` Dirk Brandewie
@ 2013-10-24 13:23 ` adrian
2013-10-25 22:15 ` Rafael J. Wysocki
0 siblings, 1 reply; 4+ messages in thread
From: adrian @ 2013-10-24 13:23 UTC (permalink / raw)
To: Dirk Brandewie; +Cc: rjw, viresh.kumar, cpufreq, linux-pm, linda.knippers
Thanks, Dirk. The patch has been modified accordingly.
Signed-off-by: Adrian Huang <adrian.huang@hp.com>
---
drivers/cpufreq/intel_pstate.c | 80 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 80 insertions(+)
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index badf620..8c00394 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -26,6 +26,8 @@
#include <linux/fs.h>
#include <linux/debugfs.h>
#include <trace/events/power.h>
+#include <linux/acpi.h>
+#include <acpi/processor.h>
#include <asm/div64.h>
#include <asm/msr.h>
@@ -129,6 +131,18 @@ static struct perf_limits limits = {
.max_sysfs_pct = 100,
};
+struct hw_vendor_info {
+ u16 valid;
+ char oem_id[ACPI_OEM_ID_SIZE];
+ char oem_table_id[ACPI_OEM_TABLE_ID_SIZE];
+};
+
+/* Hardware vendor-specific info that has its own power management modes */
+static struct hw_vendor_info vendor_info[] = {
+ {1, "HP ", "ProLiant"},
+ {0, "", ""},
+};
+
static inline void pid_reset(struct _pid *pid, int setpoint, int busy,
int deadband, int integral) {
pid->setpoint = setpoint;
@@ -700,6 +714,63 @@ static int intel_pstate_msrs_not_valid(void)
return 0;
}
+
+static bool intel_pstate_no_acpi_pss(void)
+{
+ struct acpi_processor *pr;
+ union acpi_object *pss = NULL;
+ int i;
+
+ for_each_possible_cpu(i) {
+
+ pr = per_cpu(processors, i);
+
+ if (pr) {
+ struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER,
+ NULL };
+
+ if (ACPI_SUCCESS(acpi_evaluate_object(pr->handle,
+ "_PSS", NULL, &buffer))) {
+
+ pss = buffer.pointer;
+
+ if (pss && pss->type == ACPI_TYPE_PACKAGE) {
+ kfree(buffer.pointer);
+ return false;
+ }
+
+ kfree(buffer.pointer);
+ }
+ }
+ }
+
+ return true;
+}
+
+static bool intel_pstate_platform_pwr_mgmt_exists(void)
+{
+ struct acpi_table_header hdr;
+ struct hw_vendor_info *v_info;
+
+ if (acpi_get_table_header(ACPI_SIG_FADT, 0, &hdr) == AE_OK) {
+ for (v_info = vendor_info; v_info->valid; v_info++) {
+ /*
+ * Check if the hardware/platform is in the
+ * predefined vendor data.
+ */
+ if (!strncmp(hdr.oem_id, v_info->oem_id,
+ ACPI_OEM_ID_SIZE) &&
+ !strncmp(hdr.oem_table_id, v_info->oem_table_id,
+ ACPI_OEM_TABLE_ID_SIZE)) {
+ if (intel_pstate_no_acpi_pss())
+ return true;
+ }
+ }
+ }
+
+ return false;
+}
+
static int __init intel_pstate_init(void)
{
int cpu, rc = 0;
@@ -708,6 +779,15 @@ static int __init intel_pstate_init(void)
if (no_load)
return -ENODEV;
+ if (!acpi_disabled) {
+ /*
+ * Check if the platform has its own power management modes.
+ * If so, the pstate cpufreq driver will be ignored.
+ */
+ if (intel_pstate_platform_pwr_mgmt_exists())
+ return -ENODEV;
+ }
+
id = x86_match_cpu(intel_pstate_cpu_ids);
if (!id)
return -ENODEV;
--
1.8.1.2
於 一，2013-10-21 於 09:03 -0700，Dirk Brandewie 提到：
> On 10/20/2013 10:27 PM, Adrian Huang wrote:
> > static int __init intel_pstate_init(void)
> > {
> > int cpu, rc = 0;
> > @@ -708,6 +779,15 @@ static int __init intel_pstate_init(void)
> > if (no_load)
> > return -ENODEV;
> >
> > + if (!acpi_disabled) {
> > + /*
> > + * Check if the platform has its own power management modes.
> > + * If so, the pstate cpufreq driver will be ignored.
> > + */
> > + if (intel_pstate_platform_pwr_mgmt_exists())
> > + return 0;
>
> Please return -ENODEV here instead of 0
>
> > + }
> > +
> > id = x86_match_cpu(intel_pstate_cpu_ids);
> > if (!id)
> > return -ENODEV;
> >
>
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] cpufreq: intel_pstate: skip the driver if ACPI has power mgmt option
2013-10-24 13:23 ` adrian
@ 2013-10-25 22:15 ` Rafael J. Wysocki
0 siblings, 0 replies; 4+ messages in thread
From: Rafael J. Wysocki @ 2013-10-25 22:15 UTC (permalink / raw)
To: adrian; +Cc: Dirk Brandewie, viresh.kumar, cpufreq, linux-pm, linda.knippers
On Thursday, October 24, 2013 09:23:08 PM adrian wrote:
> Thanks, Dirk. The patch has been modified accordingly.
>
> Signed-off-by: Adrian Huang <adrian.huang@hp.com>
> ---
> drivers/cpufreq/intel_pstate.c | 80 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 80 insertions(+)
>
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index badf620..8c00394 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -26,6 +26,8 @@
> #include <linux/fs.h>
> #include <linux/debugfs.h>
> #include <trace/events/power.h>
> +#include <linux/acpi.h>
> +#include <acpi/processor.h>
>
> #include <asm/div64.h>
> #include <asm/msr.h>
> @@ -129,6 +131,18 @@ static struct perf_limits limits = {
> .max_sysfs_pct = 100,
> };
>
> +struct hw_vendor_info {
> + u16 valid;
> + char oem_id[ACPI_OEM_ID_SIZE];
> + char oem_table_id[ACPI_OEM_TABLE_ID_SIZE];
> +};
> +
> +/* Hardware vendor-specific info that has its own power management modes */
> +static struct hw_vendor_info vendor_info[] = {
> + {1, "HP ", "ProLiant"},
> + {0, "", ""},
> +};
> +
> static inline void pid_reset(struct _pid *pid, int setpoint, int busy,
> int deadband, int integral) {
> pid->setpoint = setpoint;
> @@ -700,6 +714,63 @@ static int intel_pstate_msrs_not_valid(void)
>
> return 0;
> }
> +
> +static bool intel_pstate_no_acpi_pss(void)
> +{
> + struct acpi_processor *pr;
> + union acpi_object *pss = NULL;
> + int i;
> +
> + for_each_possible_cpu(i) {
Can you possibly reduce the max indentation level of this function,
perhaps using the continue instruction?
Also please move the definition of pss to the block in which that
variable is used.
> +
> + pr = per_cpu(processors, i);
> +
> + if (pr) {
> + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER,
> + NULL };
> +
> + if (ACPI_SUCCESS(acpi_evaluate_object(pr->handle,
> + "_PSS", NULL, &buffer))) {
> +
> + pss = buffer.pointer;
> +
> + if (pss && pss->type == ACPI_TYPE_PACKAGE) {
> + kfree(buffer.pointer);
> + return false;
> + }
> +
> + kfree(buffer.pointer);
> + }
> + }
> + }
> +
> + return true;
> +}
> +
> +static bool intel_pstate_platform_pwr_mgmt_exists(void)
> +{
> + struct acpi_table_header hdr;
> + struct hw_vendor_info *v_info;
> +
> + if (acpi_get_table_header(ACPI_SIG_FADT, 0, &hdr) == AE_OK) {
I'd do
if (ACPI_FAILURE(acpi_get_table_header(ACPI_SIG_FADT, 0, &hdr))
return false;
and then the indentation level can be reduced.
> + for (v_info = vendor_info; v_info->valid; v_info++) {
> + /*
> + * Check if the hardware/platform is in the
> + * predefined vendor data.
> + */
> + if (!strncmp(hdr.oem_id, v_info->oem_id,
> + ACPI_OEM_ID_SIZE) &&
> + !strncmp(hdr.oem_table_id, v_info->oem_table_id,
> + ACPI_OEM_TABLE_ID_SIZE)) {
> + if (intel_pstate_no_acpi_pss())
> + return true;
I think the above should be
if (!strncmp(hdr.oem_id, v_info->oem_id, ACPI_OEM_ID_SIZE)
&& !strncmp(hdr.oem_table_id, v_info->oem_table_id, ACPI_OEM_TABLE_ID_SIZE)
&& intel_pstate_no_acpi_pss())
return true;
(you don't need to break the lines longer than 80 characters if that makes
the code less readable).
> + }
> + }
> + }
> +
> + return false;
> +}
> +
> static int __init intel_pstate_init(void)
> {
> int cpu, rc = 0;
> @@ -708,6 +779,15 @@ static int __init intel_pstate_init(void)
> if (no_load)
> return -ENODEV;
>
> + if (!acpi_disabled) {
> + /*
> + * Check if the platform has its own power management modes.
> + * If so, the pstate cpufreq driver will be ignored.
> + */
> + if (intel_pstate_platform_pwr_mgmt_exists())
> + return -ENODEV;
> + }
> +
> id = x86_match_cpu(intel_pstate_cpu_ids);
> if (!id)
> return -ENODEV;
Thanks!
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-10-25 22:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-21 5:27 [PATCH 1/1] cpufreq: intel_pstate: skip the driver if ACPI has power mgmt option Adrian Huang
2013-10-21 16:03 ` Dirk Brandewie
2013-10-24 13:23 ` adrian
2013-10-25 22:15 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).