[PATCH] cpufreq: intel_pstate: Use firmware default EPP

[PATCH] cpufreq: intel_pstate: Use firmware default EPP

[Srinivas Pandruvada]

For some specific platforms (E.g. AlderLake) the balance performance
EPP is updated from the hard coded value in the driver. This acts as
the default and balance_performance EPP. The purpose of this EPP
update is to reach maximum 1 core turbo frequency (when possible) out
of the box.

Although we can achieve the objective by using hard coded value in the
driver, there can be other EPP which can be better in terms of power.
But that will be very subjective based on platform and use cases.
This is not practical to have a per platform specific default hard coded
in the driver.

If a platform wants to specify default EPP, it can be set in the firmware.
If this EPP is not the chipset default of 0x80 (balance_perf_epp unless
driver changed it) and more performance oriented but not 0, the driver
can use this as the default and balanced_perf EPP. In this case no driver
update is required every time there is some new platform and default EPP.

If the firmware didn't update the EPP from the chipset default then
the hard coded value is used as per existing implementation.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/cpufreq/intel_pstate.c | 38 ++++++++++++++++++++++++++++------
 1 file changed, 32 insertions(+), 6 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index bc7f7e6759bd..846bb3a78788 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -1692,6 +1692,37 @@ static void intel_pstate_enable_hwp_interrupt(struct cpudata *cpudata)
 	}
 }
 
+static void intel_pstate_update_epp_defaults(struct cpudata *cpudata)
+{
+	cpudata->epp_default = intel_pstate_get_epp(cpudata, 0);
+
+	/*
+	 * If this CPU gen doesn't call for change in balance_perf
+	 * EPP return.
+	 */
+	if (epp_values[EPP_INDEX_BALANCE_PERFORMANCE] == HWP_EPP_BALANCE_PERFORMANCE)
+		return;
+
+	/*
+	 * If powerup EPP is something other than chipset default 0x80 and
+	 * - is more performance oriented than 0x80 (default balance_perf EPP)
+	 * - But less performance oriented than performance EPP
+	 *   then use this as new balance_perf EPP.
+	 */
+	if (cpudata->epp_default < HWP_EPP_BALANCE_PERFORMANCE &&
+	    cpudata->epp_default > HWP_EPP_PERFORMANCE) {
+		epp_values[EPP_INDEX_BALANCE_PERFORMANCE] = cpudata->epp_default;
+		return;
+	}
+
+	/*
+	 * Use hard coded value per gen to update the balance_perf
+	 * and default EPP.
+	 */
+	cpudata->epp_default = epp_values[EPP_INDEX_BALANCE_PERFORMANCE];
+	intel_pstate_set_epp(cpudata, cpudata->epp_default);
+}
+
 static void intel_pstate_hwp_enable(struct cpudata *cpudata)
 {
 	/* First disable HWP notification interrupt till we activate again */
@@ -1705,12 +1736,7 @@ static void intel_pstate_hwp_enable(struct cpudata *cpudata)
 	if (cpudata->epp_default >= 0)
 		return;
 
-	if (epp_values[EPP_INDEX_BALANCE_PERFORMANCE] == HWP_EPP_BALANCE_PERFORMANCE) {
-		cpudata->epp_default = intel_pstate_get_epp(cpudata, 0);
-	} else {
-		cpudata->epp_default = epp_values[EPP_INDEX_BALANCE_PERFORMANCE];
-		intel_pstate_set_epp(cpudata, cpudata->epp_default);
-	}
+	intel_pstate_update_epp_defaults(cpudata);
 }
 
 static int atom_get_min_pstate(void)
-- 
2.31.1

Re: [PATCH] cpufreq: intel_pstate: Use firmware default EPP

[Rafael J. Wysocki]

On Thu, Mar 10, 2022 at 11:42 PM Srinivas Pandruvada
<srinivas.pandruvada@linux.intel.com> wrote:
>
> For some specific platforms (E.g. AlderLake) the balance performance
> EPP is updated from the hard coded value in the driver. This acts as
> the default and balance_performance EPP. The purpose of this EPP
> update is to reach maximum 1 core turbo frequency (when possible) out
> of the box.
>
> Although we can achieve the objective by using hard coded value in the
> driver, there can be other EPP which can be better in terms of power.
> But that will be very subjective based on platform and use cases.
> This is not practical to have a per platform specific default hard coded
> in the driver.
>
> If a platform wants to specify default EPP, it can be set in the firmware.
> If this EPP is not the chipset default of 0x80 (balance_perf_epp unless
> driver changed it) and more performance oriented but not 0, the driver
> can use this as the default and balanced_perf EPP. In this case no driver
> update is required every time there is some new platform and default EPP.
>
> If the firmware didn't update the EPP from the chipset default then
> the hard coded value is used as per existing implementation.
>
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
>  drivers/cpufreq/intel_pstate.c | 38 ++++++++++++++++++++++++++++------
>  1 file changed, 32 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index bc7f7e6759bd..846bb3a78788 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -1692,6 +1692,37 @@ static void intel_pstate_enable_hwp_interrupt(struct cpudata *cpudata)
>         }
>  }
>
> +static void intel_pstate_update_epp_defaults(struct cpudata *cpudata)
> +{
> +       cpudata->epp_default = intel_pstate_get_epp(cpudata, 0);
> +
> +       /*
> +        * If this CPU gen doesn't call for change in balance_perf
> +        * EPP return.
> +        */
> +       if (epp_values[EPP_INDEX_BALANCE_PERFORMANCE] == HWP_EPP_BALANCE_PERFORMANCE)
> +               return;
> +
> +       /*
> +        * If powerup EPP is something other than chipset default 0x80 and
> +        * - is more performance oriented than 0x80 (default balance_perf EPP)
> +        * - But less performance oriented than performance EPP
> +        *   then use this as new balance_perf EPP.
> +        */
> +       if (cpudata->epp_default < HWP_EPP_BALANCE_PERFORMANCE &&
> +           cpudata->epp_default > HWP_EPP_PERFORMANCE) {
> +               epp_values[EPP_INDEX_BALANCE_PERFORMANCE] = cpudata->epp_default;
> +               return;
> +       }
> +
> +       /*
> +        * Use hard coded value per gen to update the balance_perf
> +        * and default EPP.
> +        */
> +       cpudata->epp_default = epp_values[EPP_INDEX_BALANCE_PERFORMANCE];
> +       intel_pstate_set_epp(cpudata, cpudata->epp_default);
> +}
> +
>  static void intel_pstate_hwp_enable(struct cpudata *cpudata)
>  {
>         /* First disable HWP notification interrupt till we activate again */
> @@ -1705,12 +1736,7 @@ static void intel_pstate_hwp_enable(struct cpudata *cpudata)
>         if (cpudata->epp_default >= 0)
>                 return;
>
> -       if (epp_values[EPP_INDEX_BALANCE_PERFORMANCE] == HWP_EPP_BALANCE_PERFORMANCE) {
> -               cpudata->epp_default = intel_pstate_get_epp(cpudata, 0);
> -       } else {
> -               cpudata->epp_default = epp_values[EPP_INDEX_BALANCE_PERFORMANCE];
> -               intel_pstate_set_epp(cpudata, cpudata->epp_default);
> -       }
> +       intel_pstate_update_epp_defaults(cpudata);
>  }
>
>  static int atom_get_min_pstate(void)
> --

Applied as 5.18 material, thanks!