2014-11-25 05:00:28

by Ethan Zhao

[permalink] [raw]
Subject: [PATCH 0/2 v3]intel_pstate: skip this driver if hit Sun X86 servers

Oracle Sun servers(X86) have power capping features that work via ACPI _PPC method,
patch No.1 is used to skip this driver if Oracle Sun server and _PPC detected.
Patch No.2 introduces a kernel command line parameter for those who would like
to enable intel_pstate on Sun X86 servers and be aware of the risk.

Compiled and tested with stable-3.18-rc5 on Oracle Sun server X4-2 series.

Thanks test and review by Linda Knippers <[email protected]>
and the suggestion,review from Dirk Brandewie <[email protected]>.
--
v2: fix break HP Proliant issue.
v3: expand the hardware vendor list.

Ethan Zhao (2):
intel_pstate: skip this driver if Sun server has _PPC method
intel_pstate: add kernel parameter to enable loading on Sun X86
servers.

Documentation/kernel-parameters.txt | 3 +++
drivers/cpufreq/intel_pstate.c | 49 ++++++++++++++++++++++++++++++++++---
2 files changed, 49 insertions(+), 3 deletions(-)

--
1.8.3.1


2014-11-25 05:00:32

by Ethan Zhao

[permalink] [raw]
Subject: [PATCH 1/2 v3] intel_pstate: skip this driver if Sun server has _PPC method

Oracle Sun X86 servers have dynamic power capping capability that works via
ACPI _PPC method etc, so skip loading this driver if Sun server has ACPI _PPC
enabled.

Signed-off-by: Ethan Zhao <[email protected]>
---
v2: fix break HP Proliant issue.
v3: expand the hardware vendor list.

drivers/cpufreq/intel_pstate.c | 45 +++++++++++++++++++++++++++++++++++++++---
1 file changed, 42 insertions(+), 3 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 27bb6d3..fa67fb3 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -943,6 +943,21 @@ static bool intel_pstate_no_acpi_pss(void)
return true;
}

+static bool intel_pstate_has_acpi_ppc(void)
+{
+ int i;
+
+ for_each_possible_cpu(i) {
+ struct acpi_processor *pr = per_cpu(processors, i);
+
+ if (!pr)
+ continue;
+ if (acpi_has_method(pr->handle, "_PPC"))
+ return true;
+ }
+ return false;
+}
+
struct hw_vendor_info {
u16 valid;
char oem_id[ACPI_OEM_ID_SIZE];
@@ -950,11 +965,26 @@ struct hw_vendor_info {
};

/* Hardware vendor-specific info that has its own power management modes */
-static struct hw_vendor_info vendor_info[] = {
+static struct hw_vendor_info hp_vendor_info[] = {
{1, "HP ", "ProLiant"},
{0, "", ""},
};

+/* The list of Oracle Sun server that has power capping capability */
+static struct hw_vendor_info ora_vendor_info[] = {
+ {1, "ORACLE", "X4-2 "},
+ {1, "ORACLE", "X4-2L "},
+ {1, "ORACLE", "X4-2B "},
+ {1, "ORACLE", "X3-2 "},
+ {1, "ORACLE", "X3-2L "},
+ {1, "ORACLE", "X3-2B "},
+ {1, "ORACLE", "X4470M2 "},
+ {1, "ORACLE", "X4270M3 "},
+ {1, "ORACLE", "X4270M2 "},
+ {1, "ORACLE", "X4170M2 "},
+ {0, "", ""},
+};
+
static bool intel_pstate_platform_pwr_mgmt_exists(void)
{
struct acpi_table_header hdr;
@@ -963,18 +993,27 @@ static bool intel_pstate_platform_pwr_mgmt_exists(void)
if (acpi_disabled ||
ACPI_FAILURE(acpi_get_table_header(ACPI_SIG_FADT, 0, &hdr)))
return false;
-
- for (v_info = vendor_info; v_info->valid; v_info++) {
+ /* Check HP devices */
+ for (v_info = hp_vendor_info; v_info->valid; v_info++) {
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;
}
+ /* Check Oracle Sun X86 servers */
+ for (v_info = ora_vendor_info; v_info->valid; v_info++) {
+ 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_has_acpi_ppc())
+ return true;
+ }

return false;
}
#else /* CONFIG_ACPI not enabled */
static inline bool intel_pstate_platform_pwr_mgmt_exists(void) { return false; }
+static inline bool intel_pstate_has_acpi_ppc(void) { return false; }
#endif /* CONFIG_ACPI */

static int __init intel_pstate_init(void)
--
1.8.3.1

2014-11-25 05:00:41

by Ethan Zhao

[permalink] [raw]
Subject: [PATCH 2/2 v3] intel_pstate: add kernel parameter to enable loading on Sun X86 servers.

To force loading on Oracle Sun X86 servers, provide one kernel command line
parameter

intel_pstate = onora

For those who be aware of the risk doing so.

Signed-off-by: Ethan Zhao <[email protected]>
---
v2: change to hardware vendor specific naming parameter.

Documentation/kernel-parameters.txt | 3 +++
drivers/cpufreq/intel_pstate.c | 6 +++++-
2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 479f332..e4b1b81 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1446,6 +1446,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
disable
Do not enable intel_pstate as the default
scaling driver for the supported processors
+ onora
+ Enable loading intel_pstate on Oracle Sun Servers(X86).
+ only for those who be aware of the risk.

intremap= [X86-64, Intel-IOMMU]
on enable Interrupt Remapping (default)
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index fa67fb3..e49b050 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -866,6 +866,7 @@ static struct cpufreq_driver intel_pstate_driver = {
};

static int __initdata no_load;
+static unsigned int load_on_sun;

static int intel_pstate_msrs_not_valid(void)
{
@@ -1005,7 +1006,8 @@ static bool intel_pstate_platform_pwr_mgmt_exists(void)
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_has_acpi_ppc())
+ intel_pstate_has_acpi_ppc() &&
+ !load_on_sun)
return true;
}

@@ -1080,6 +1082,8 @@ static int __init intel_pstate_setup(char *str)

if (!strcmp(str, "disable"))
no_load = 1;
+ if (!strcmp(str, "onora"))
+ load_on_sun = 1;
return 0;
}
early_param("intel_pstate", intel_pstate_setup);
--
1.8.3.1

2014-11-25 14:51:41

by Dirk Brandewie

[permalink] [raw]
Subject: Re: [PATCH 2/2 v3] intel_pstate: add kernel parameter to enable loading on Sun X86 servers.

On 11/24/2014 08:59 PM, Ethan Zhao wrote:
> To force loading on Oracle Sun X86 servers, provide one kernel command line
> parameter
>
> intel_pstate = onora
>
> For those who be aware of the risk doing so.
>
> Signed-off-by: Ethan Zhao <[email protected]>
> ---
> v2: change to hardware vendor specific naming parameter.
>
> Documentation/kernel-parameters.txt | 3 +++
> drivers/cpufreq/intel_pstate.c | 6 +++++-
> 2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 479f332..e4b1b81 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -1446,6 +1446,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
> disable
> Do not enable intel_pstate as the default
> scaling driver for the supported processors
> + onora
> + Enable loading intel_pstate on Oracle Sun Servers(X86).
> + only for those who be aware of the risk.

What are the risks? What is the behaviour if platform power management is
enabled and intel_pstate is trying to control P state selection as well?

If intel_pstate will be able to successfully control P state selection
with platform power management enabled then how about the name "oracle_force"?
Also the documentation should say what the risks are.

>
> intremap= [X86-64, Intel-IOMMU]
> on enable Interrupt Remapping (default)
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index fa67fb3..e49b050 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -866,6 +866,7 @@ static struct cpufreq_driver intel_pstate_driver = {
> };
>
> static int __initdata no_load;
> +static unsigned int load_on_sun;
>
> static int intel_pstate_msrs_not_valid(void)
> {
> @@ -1005,7 +1006,8 @@ static bool intel_pstate_platform_pwr_mgmt_exists(void)
> 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_has_acpi_ppc())
> + intel_pstate_has_acpi_ppc() &&
> + !load_on_sun)
> return true;
> }
>
> @@ -1080,6 +1082,8 @@ static int __init intel_pstate_setup(char *str)
>
> if (!strcmp(str, "disable"))
> no_load = 1;
> + if (!strcmp(str, "onora"))
> + load_on_sun = 1;
> return 0;
> }
> early_param("intel_pstate", intel_pstate_setup);
>

2014-11-25 14:57:08

by Dirk Brandewie

[permalink] [raw]
Subject: Re: [PATCH 1/2 v3] intel_pstate: skip this driver if Sun server has _PPC method

On 11/24/2014 08:59 PM, Ethan Zhao wrote:
> Oracle Sun X86 servers have dynamic power capping capability that works via
> ACPI _PPC method etc, so skip loading this driver if Sun server has ACPI _PPC
> enabled.
>

How about this patch? only compile tested.

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 3468387..db7b8b2 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -1025,15 +1025,46 @@ static bool intel_pstate_no_acpi_pss(void)
return true;
}

+static bool intel_pstate_has_acpi_ppc(void)
+{
+ int i;
+
+ for_each_possible_cpu(i) {
+ struct acpi_processor *pr = per_cpu(processors, i);
+
+ if (!pr)
+ continue;
+ if (acpi_has_method(pr->handle, "_PPC"))
+ return true;
+ }
+ return false;
+}
+
+enum {
+ PSS,
+ PCC,
+};
+
struct hw_vendor_info {
u16 valid;
char oem_id[ACPI_OEM_ID_SIZE];
char oem_table_id[ACPI_OEM_TABLE_ID_SIZE];
+ int oem_pwr_table;
};

/* Hardware vendor-specific info that has its own power management modes */
static struct hw_vendor_info vendor_info[] = {
- {1, "HP ", "ProLiant"},
+ {1, "HP ", "ProLiant", PSS},
+ {1, "ORACLE", "X4-2 ", PCC},
+ {1, "ORACLE", "X4-2L ", PCC},
+ {1, "ORACLE", "X4-2B ", PCC},
+ {1, "ORACLE", "X3-2 ", PCC},
+ {1, "ORACLE", "X3-2L ", PCC},
+ {1, "ORACLE", "X3-2B ", PCC},
+ {1, "ORACLE", "X4470M2 ", PCC},
+ {1, "ORACLE", "X4270M3 ", PCC},
+ {1, "ORACLE", "X4270M2 ", PCC},
+ {1, "ORACLE", "X4170M2 ", PCC},
{0, "", ""},
};

@@ -1057,15 +1088,20 @@ static bool intel_pstate_platform_pwr_mgmt_exists(void)

for (v_info = vendor_info; v_info->valid; v_info++) {
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;
+ !strncmp(hdr.oem_table_id, v_info->oem_table_id, ACPI_OEM_TABLE_ID_SIZE))
+ switch (v_info->oem_pwr_table) {
+ case PSS:
+ return intel_pstate_no_acpi_pss();
+ case PCC:
+ return intel_pstate_has_acpi_ppc();
+ }
}

return false;
}
#else /* CONFIG_ACPI not enabled */
static inline bool intel_pstate_platform_pwr_mgmt_exists(void) { return false; }
+static inline bool intel_pstate_has_acpi_ppc(void) { return false; }
#endif /* CONFIG_ACPI */

static int __init intel_pstate_init(void)

2014-11-26 00:08:41

by ethan zhao

[permalink] [raw]
Subject: Re: [PATCH 2/2 v3] intel_pstate: add kernel parameter to enable loading on Sun X86 servers.

Dirk,

> ?? 2014??11??25?գ?22:51??Dirk Brandewie <[email protected]> д????
>
>> On 11/24/2014 08:59 PM, Ethan Zhao wrote:
>> To force loading on Oracle Sun X86 servers, provide one kernel command line
>> parameter
>>
>> intel_pstate = onora
>>
>> For those who be aware of the risk doing so.
>>
>> Signed-off-by: Ethan Zhao <[email protected]>
>> ---
>> v2: change to hardware vendor specific naming parameter.
>>
>> Documentation/kernel-parameters.txt | 3 +++
>> drivers/cpufreq/intel_pstate.c | 6 +++++-
>> 2 files changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
>> index 479f332..e4b1b81 100644
>> --- a/Documentation/kernel-parameters.txt
>> +++ b/Documentation/kernel-parameters.txt
>> @@ -1446,6 +1446,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>> disable
>> Do not enable intel_pstate as the default
>> scaling driver for the supported processors
>> + onora
>> + Enable loading intel_pstate on Oracle Sun Servers(X86).
>> + only for those who be aware of the risk.
>
> What are the risks? What is the behaviour if platform power management is
> enabled and intel_pstate is trying to control P state selection as well?
So far test shows intel_pstate could run.
>
> If intel_pstate will be able to successfully control P state selection
> with platform power management enabled then how about the name "oracle_force"?
Oracle_force seems a suggestion from oracle, not an option would be used as choice
In case of no other method.

> Also the documentation should say what the risks are.

The risk is the sever lost power capping capability, that may cause service down, for example,
The server has two power supply units, one is faulty, without power capping, could't limit the power consumption within one power supply's capability, this could cause the last power supply
Unit overload and power down.

Thanks,
Ethan
>
>>
>> intremap= [X86-64, Intel-IOMMU]
>> on enable Interrupt Remapping (default)
>> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
>> index fa67fb3..e49b050 100644
>> --- a/drivers/cpufreq/intel_pstate.c
>> +++ b/drivers/cpufreq/intel_pstate.c
>> @@ -866,6 +866,7 @@ static struct cpufreq_driver intel_pstate_driver = {
>> };
>>
>> static int __initdata no_load;
>> +static unsigned int load_on_sun;
>>
>> static int intel_pstate_msrs_not_valid(void)
>> {
>> @@ -1005,7 +1006,8 @@ static bool intel_pstate_platform_pwr_mgmt_exists(void)
>> 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_has_acpi_ppc())
>> + intel_pstate_has_acpi_ppc() &&
>> + !load_on_sun)
>> return true;
>> }
>>
>> @@ -1080,6 +1082,8 @@ static int __init intel_pstate_setup(char *str)
>>
>> if (!strcmp(str, "disable"))
>> no_load = 1;
>> + if (!strcmp(str, "onora"))
>> + load_on_sun = 1;
>> return 0;
>> }
>> early_param("intel_pstate", intel_pstate_setup);
>

2014-11-26 00:15:38

by ethan zhao

[permalink] [raw]
Subject: Re: [PATCH 1/2 v3] intel_pstate: skip this driver if Sun server has _PPC method

Dirk,

> ?? 2014??11??25?գ?22:57??Dirk Brandewie <[email protected]> д????
>
>> On 11/24/2014 08:59 PM, Ethan Zhao wrote:
>> Oracle Sun X86 servers have dynamic power capping capability that works via
>> ACPI _PPC method etc, so skip loading this driver if Sun server has ACPI _PPC
>> enabled.
> How about this patch? only compile tested.
>
Looks good to me, and obviously my one is much simpler :)
I will test it and give you feed back.

Thanks,
Ethan

> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 3468387..db7b8b2 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -1025,15 +1025,46 @@ static bool intel_pstate_no_acpi_pss(void)
> return true;
> }
>
> +static bool intel_pstate_has_acpi_ppc(void)
> +{
> + int i;
> +
> + for_each_possible_cpu(i) {
> + struct acpi_processor *pr = per_cpu(processors, i);
> +
> + if (!pr)
> + continue;
> + if (acpi_has_method(pr->handle, "_PPC"))
> + return true;
> + }
> + return false;
> +}
> +
> +enum {
> + PSS,
> + PCC,
> +};
> +
> struct hw_vendor_info {
> u16 valid;
> char oem_id[ACPI_OEM_ID_SIZE];
> char oem_table_id[ACPI_OEM_TABLE_ID_SIZE];
> + int oem_pwr_table;
> };
>
> /* Hardware vendor-specific info that has its own power management modes */
> static struct hw_vendor_info vendor_info[] = {
> - {1, "HP ", "ProLiant"},
> + {1, "HP ", "ProLiant", PSS},
> + {1, "ORACLE", "X4-2 ", PCC},
> + {1, "ORACLE", "X4-2L ", PCC},
> + {1, "ORACLE", "X4-2B ", PCC},
> + {1, "ORACLE", "X3-2 ", PCC},
> + {1, "ORACLE", "X3-2L ", PCC},
> + {1, "ORACLE", "X3-2B ", PCC},
> + {1, "ORACLE", "X4470M2 ", PCC},
> + {1, "ORACLE", "X4270M3 ", PCC},
> + {1, "ORACLE", "X4270M2 ", PCC},
> + {1, "ORACLE", "X4170M2 ", PCC},
> {0, "", ""},
> };
>
> @@ -1057,15 +1088,20 @@ static bool intel_pstate_platform_pwr_mgmt_exists(void)
>
> for (v_info = vendor_info; v_info->valid; v_info++) {
> 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;
> + !strncmp(hdr.oem_table_id, v_info->oem_table_id, ACPI_OEM_TABLE_ID_SIZE))
> + switch (v_info->oem_pwr_table) {
> + case PSS:
> + return intel_pstate_no_acpi_pss();
> + case PCC:
> + return intel_pstate_has_acpi_ppc();
> + }
> }
>
> return false;
> }
> #else /* CONFIG_ACPI not enabled */
> static inline bool intel_pstate_platform_pwr_mgmt_exists(void) { return false; }
> +static inline bool intel_pstate_has_acpi_ppc(void) { return false; }
> #endif /* CONFIG_ACPI */
>
> static int __init intel_pstate_init(void)
>
>

2014-11-26 17:55:00

by Linda Knippers

[permalink] [raw]
Subject: Re: [PATCH 1/2 v3] intel_pstate: skip this driver if Sun server has _PPC method

On 11/25/2014 9:57 AM, Dirk Brandewie wrote:
> On 11/24/2014 08:59 PM, Ethan Zhao wrote:
>> Oracle Sun X86 servers have dynamic power capping capability that works via
>> ACPI _PPC method etc, so skip loading this driver if Sun server has ACPI _PPC
>> enabled.
>>
>
> How about this patch? only compile tested.

It looks much better to me. I'll give it a try.

-- ljk

>
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 3468387..db7b8b2 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -1025,15 +1025,46 @@ static bool intel_pstate_no_acpi_pss(void)
> return true;
> }
>
> +static bool intel_pstate_has_acpi_ppc(void)
> +{
> + int i;
> +
> + for_each_possible_cpu(i) {
> + struct acpi_processor *pr = per_cpu(processors, i);
> +
> + if (!pr)
> + continue;
> + if (acpi_has_method(pr->handle, "_PPC"))
> + return true;
> + }
> + return false;
> +}
> +
> +enum {
> + PSS,
> + PCC,
> +};
> +
> struct hw_vendor_info {
> u16 valid;
> char oem_id[ACPI_OEM_ID_SIZE];
> char oem_table_id[ACPI_OEM_TABLE_ID_SIZE];
> + int oem_pwr_table;
> };
>
> /* Hardware vendor-specific info that has its own power management modes */
> static struct hw_vendor_info vendor_info[] = {
> - {1, "HP ", "ProLiant"},
> + {1, "HP ", "ProLiant", PSS},
> + {1, "ORACLE", "X4-2 ", PCC},
> + {1, "ORACLE", "X4-2L ", PCC},
> + {1, "ORACLE", "X4-2B ", PCC},
> + {1, "ORACLE", "X3-2 ", PCC},
> + {1, "ORACLE", "X3-2L ", PCC},
> + {1, "ORACLE", "X3-2B ", PCC},
> + {1, "ORACLE", "X4470M2 ", PCC},
> + {1, "ORACLE", "X4270M3 ", PCC},
> + {1, "ORACLE", "X4270M2 ", PCC},
> + {1, "ORACLE", "X4170M2 ", PCC},
> {0, "", ""},
> };
>
> @@ -1057,15 +1088,20 @@ static bool intel_pstate_platform_pwr_mgmt_exists(void)
>
> for (v_info = vendor_info; v_info->valid; v_info++) {
> 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;
> + !strncmp(hdr.oem_table_id, v_info->oem_table_id,
> ACPI_OEM_TABLE_ID_SIZE))
> + switch (v_info->oem_pwr_table) {
> + case PSS:
> + return intel_pstate_no_acpi_pss();
> + case PCC:
> + return intel_pstate_has_acpi_ppc();
> + }
> }
>
> return false;
> }
> #else /* CONFIG_ACPI not enabled */
> static inline bool intel_pstate_platform_pwr_mgmt_exists(void) { return false; }
> +static inline bool intel_pstate_has_acpi_ppc(void) { return false; }
> #endif /* CONFIG_ACPI */
>
> static int __init intel_pstate_init(void)
>
>

2014-11-26 20:50:21

by Linda Knippers

[permalink] [raw]
Subject: Re: [PATCH 1/2 v3] intel_pstate: skip this driver if Sun server has _PPC method

On 11/26/2014 12:54 PM, Linda Knippers wrote:
> On 11/25/2014 9:57 AM, Dirk Brandewie wrote:
>> On 11/24/2014 08:59 PM, Ethan Zhao wrote:
>>> Oracle Sun X86 servers have dynamic power capping capability that works via
>>> ACPI _PPC method etc, so skip loading this driver if Sun server has ACPI _PPC
>>> enabled.
>>>
>>
>> How about this patch? only compile tested.
>
> It looks much better to me. I'll give it a try.

As expected, that works just fine for ProLiant.
Thanks Dirk.

-- ljk

>
> -- ljk
>
>>
>> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
>> index 3468387..db7b8b2 100644
>> --- a/drivers/cpufreq/intel_pstate.c
>> +++ b/drivers/cpufreq/intel_pstate.c
>> @@ -1025,15 +1025,46 @@ static bool intel_pstate_no_acpi_pss(void)
>> return true;
>> }
>>
>> +static bool intel_pstate_has_acpi_ppc(void)
>> +{
>> + int i;
>> +
>> + for_each_possible_cpu(i) {
>> + struct acpi_processor *pr = per_cpu(processors, i);
>> +
>> + if (!pr)
>> + continue;
>> + if (acpi_has_method(pr->handle, "_PPC"))
>> + return true;
>> + }
>> + return false;
>> +}
>> +
>> +enum {
>> + PSS,
>> + PCC,
>> +};
>> +
>> struct hw_vendor_info {
>> u16 valid;
>> char oem_id[ACPI_OEM_ID_SIZE];
>> char oem_table_id[ACPI_OEM_TABLE_ID_SIZE];
>> + int oem_pwr_table;
>> };
>>
>> /* Hardware vendor-specific info that has its own power management modes */
>> static struct hw_vendor_info vendor_info[] = {
>> - {1, "HP ", "ProLiant"},
>> + {1, "HP ", "ProLiant", PSS},
>> + {1, "ORACLE", "X4-2 ", PCC},
>> + {1, "ORACLE", "X4-2L ", PCC},
>> + {1, "ORACLE", "X4-2B ", PCC},
>> + {1, "ORACLE", "X3-2 ", PCC},
>> + {1, "ORACLE", "X3-2L ", PCC},
>> + {1, "ORACLE", "X3-2B ", PCC},
>> + {1, "ORACLE", "X4470M2 ", PCC},
>> + {1, "ORACLE", "X4270M3 ", PCC},
>> + {1, "ORACLE", "X4270M2 ", PCC},
>> + {1, "ORACLE", "X4170M2 ", PCC},
>> {0, "", ""},
>> };
>>
>> @@ -1057,15 +1088,20 @@ static bool intel_pstate_platform_pwr_mgmt_exists(void)
>>
>> for (v_info = vendor_info; v_info->valid; v_info++) {
>> 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;
>> + !strncmp(hdr.oem_table_id, v_info->oem_table_id,
>> ACPI_OEM_TABLE_ID_SIZE))
>> + switch (v_info->oem_pwr_table) {
>> + case PSS:
>> + return intel_pstate_no_acpi_pss();
>> + case PCC:
>> + return intel_pstate_has_acpi_ppc();
>> + }
>> }
>>
>> return false;
>> }
>> #else /* CONFIG_ACPI not enabled */
>> static inline bool intel_pstate_platform_pwr_mgmt_exists(void) { return false; }
>> +static inline bool intel_pstate_has_acpi_ppc(void) { return false; }
>> #endif /* CONFIG_ACPI */
>>
>> static int __init intel_pstate_init(void)
>>
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2014-11-27 03:28:39

by Ethan Zhao

[permalink] [raw]
Subject: Re: [PATCH 1/2 v3] intel_pstate: skip this driver if Sun server has _PPC method

Dirk,

On 2014/11/25 22:57, Dirk Brandewie wrote:
> On 11/24/2014 08:59 PM, Ethan Zhao wrote:
>> Oracle Sun X86 servers have dynamic power capping capability that
>> works via
>> ACPI _PPC method etc, so skip loading this driver if Sun server has
>> ACPI _PPC
>> enabled.
>>
>
> How about this patch? only compile tested.
>
> diff --git a/drivers/cpufreq/intel_pstate.c
> b/drivers/cpufreq/intel_pstate.c
> index 3468387..db7b8b2 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -1025,15 +1025,46 @@ static bool intel_pstate_no_acpi_pss(void)
> return true;
> }
>
> +static bool intel_pstate_has_acpi_ppc(void)
> +{
> + int i;
> +
> + for_each_possible_cpu(i) {
> + struct acpi_processor *pr = per_cpu(processors, i);
> +
> + if (!pr)
> + continue;
> + if (acpi_has_method(pr->handle, "_PPC"))
> + return true;
> + }
> + return false;
> +}
> +
> +enum {
> + PSS,
> + PCC,
> +};
> +
> struct hw_vendor_info {
> u16 valid;
> char oem_id[ACPI_OEM_ID_SIZE];
> char oem_table_id[ACPI_OEM_TABLE_ID_SIZE];
> + int oem_pwr_table;
> };
>
> /* Hardware vendor-specific info that has its own power management
> modes */
> static struct hw_vendor_info vendor_info[] = {
> - {1, "HP ", "ProLiant"},
> + {1, "HP ", "ProLiant", PSS},
> + {1, "ORACLE", "X4-2 ", PCC},
> + {1, "ORACLE", "X4-2L ", PCC},
> + {1, "ORACLE", "X4-2B ", PCC},
> + {1, "ORACLE", "X3-2 ", PCC},
> + {1, "ORACLE", "X3-2L ", PCC},
> + {1, "ORACLE", "X3-2B ", PCC},
> + {1, "ORACLE", "X4470M2 ", PCC},
> + {1, "ORACLE", "X4270M3 ", PCC},
> + {1, "ORACLE", "X4270M2 ", PCC},
> + {1, "ORACLE", "X4170M2 ", PCC},
But if someone would append line here and...
+ {1, "VENDOR","XXXIX", PPC},
> {0, "", ""},
> };
>
> @@ -1057,15 +1088,20 @@ static bool
> intel_pstate_platform_pwr_mgmt_exists(void)
>
> for (v_info = vendor_info; v_info->valid; v_info++) {
> 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;
> + !strncmp(hdr.oem_table_id, v_info->oem_table_id,
> ACPI_OEM_TABLE_ID_SIZE))
> + switch (v_info->oem_pwr_table) {
> + case PSS:
> + return intel_pstate_no_acpi_pss();
> + case PCC:
> + return intel_pstate_has_acpi_ppc();
All good till append the force loading parameter here, looks ugly if
someone would add one line to vendor_info[] as above.

return intel_pstate_has_acpi_ppc() & (!load_on_sun);

Any idea ?

Thanks,
Ethan


> + }
> }
>
> return false;
> }
> #else /* CONFIG_ACPI not enabled */
> static inline bool intel_pstate_platform_pwr_mgmt_exists(void) {
> return false; }
> +static inline bool intel_pstate_has_acpi_ppc(void) { return false; }
> #endif /* CONFIG_ACPI */
>
> static int __init intel_pstate_init(void)
>
>