From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Rafael J. Wysocki" Subject: Re: [PATCH v3] cpufreq: intel_pstate: skip the driver if ACPI has power mgmt option Date: Thu, 31 Oct 2013 23:47:07 +0100 Message-ID: <24720758.HRzPDnyMM4@vostro.rjw.lan> References: <1383233045.2462.7.camel@adrian-F6S> <2937960.e8vxuRXoUT@vostro.rjw.lan> <5272D85C.3030306@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7Bit Return-path: Received: from v094114.home.net.pl ([79.96.170.134]:51129 "HELO v094114.home.net.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1751760Ab3JaWe4 (ORCPT ); Thu, 31 Oct 2013 18:34:56 -0400 In-Reply-To: <5272D85C.3030306@gmail.com> Sender: linux-pm-owner@vger.kernel.org List-Id: linux-pm@vger.kernel.org To: Dirk Brandewie , Adrian Huang Cc: viresh.kumar@linaro.org, linux-pm@vger.kernel.org, linda.knippers@hp.com, adrianhuang0701@gmail.com On Thursday, October 31, 2013 03:23:24 PM Dirk Brandewie wrote: > On 10/31/2013 03:29 PM, Rafael J. Wysocki wrote: > > On Thursday, October 31, 2013 11:24:05 PM Adrian Huang wrote: > >> 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) is defined in ACPI > >> table. For the OS control mode, the Intel pstate driver will be > >> loaded. > >> > >> Signed-off-by: Adrian Huang > > > > Do I assume correctly that this has been tested on the affected hardware? > > > > Dirk, any objections? > > As long as you are comfortable with the ACPI related logic for matching > the entry in the hw_vendor_info vendor_info[] I have no objections. Well, is there any other way to address that? > One question though can this be disabled via the HP BIOS if the > customer does not want to use the firmware power management? Good question. Adrian? > >> --- > >> Changes from v2: > >> - Remove unnecessary assignments and parentheses (Thanks to Rafael) > >> > >> Changes from v1: > >> - Minimize indentation levels (Thanks to Rafael) > >> - Re-define some local variables (Thanks to by Rafael) > >> - Return -ENODEV if platform FW has power management modes (Thanks to by Dirk) > >> > >> drivers/cpufreq/intel_pstate.c | 70 ++++++++++++++++++++++++++++++++++++++++++ > >> 1 file changed, 70 insertions(+) > >> > >> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c > >> index eb3fdc7..9ec1d6c 100644 > >> --- a/drivers/cpufreq/intel_pstate.c > >> +++ b/drivers/cpufreq/intel_pstate.c > >> @@ -26,6 +26,8 @@ > >> #include > >> #include > >> #include > >> +#include > >> +#include > >> > >> #include > >> #include > >> @@ -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; > >> @@ -698,6 +712,55 @@ static int intel_pstate_msrs_not_valid(void) > >> > >> return 0; > >> } > >> + > >> +static bool intel_pstate_no_acpi_pss(void) > >> +{ > >> + int i; > >> + > >> + for_each_possible_cpu(i) { > >> + acpi_status status; > >> + union acpi_object *pss; > >> + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL }; > >> + struct acpi_processor *pr = per_cpu(processors, i); > >> + > >> + if (!pr) > >> + continue; > >> + > >> + status = acpi_evaluate_object(pr->handle, "_PSS", > >> + NULL, &buffer); > >> + if (ACPI_FAILURE(status)) > >> + continue; > >> + > >> + pss = buffer.pointer; > >> + if (pss && pss->type == ACPI_TYPE_PACKAGE) { > >> + kfree(pss); > >> + return false; > >> + } > >> + > >> + kfree(pss); > >> + } > >> + > >> + return true; > >> +} > >> + > >> +static bool intel_pstate_platform_pwr_mgmt_exists(void) > >> +{ > >> + struct acpi_table_header hdr; > >> + struct hw_vendor_info *v_info; > >> + > >> + if (ACPI_FAILURE(acpi_get_table_header(ACPI_SIG_FADT, 0, &hdr))) > >> + return false; > >> + > >> + 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; > >> + } > >> + > >> + return false; > >> +} > >> + > >> static int __init intel_pstate_init(void) > >> { > >> int cpu, rc = 0; > >> @@ -706,6 +769,13 @@ static int __init intel_pstate_init(void) > >> if (no_load) > >> return -ENODEV; > >> > >> + /* > >> + * The Intel pstate driver will be ignored if the platform > >> + * firmware has its own power management modes. > >> + */ > >> + if (!acpi_disabled && intel_pstate_platform_pwr_mgmt_exists()) > >> + return -ENODEV; > >> + > >> id = x86_match_cpu(intel_pstate_cpu_ids); > >> if (!id) > >> return -ENODEV; > >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-pm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center.