2023-11-20 19:00:47

by Srinivas Pandruvada

[permalink] [raw]
Subject: [PATCH] cpufreq: intel_pstate: Allow firmware balance performance EPP without code change

Firmware can specify balance performance EPP value by enabling HWP and
set EPP to a desired value. The current implementation requires code
change for every generation to add an entry to intel_epp_balance_perf
table.

Some distributions like Chrome, which uses old kernels should be able
to update balance performance EPP, without code change.

There is a check to avoid updating EPP when the balance performance
EPP is not changed and is power up default of 0x80. Move this check
after checking if the HWP is enabled by the firmware and there is
a valid EPP value set by the firmware.

Signed-off-by: Srinivas Pandruvada <[email protected]>
---
drivers/cpufreq/intel_pstate.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index a534a1f7f1ee..dd6d23e389f1 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -1691,13 +1691,6 @@ 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 the EPP is set by firmware, which means that firmware enabled HWP
* - Is equal or less than 0x80 (default balance_perf EPP)
@@ -1710,6 +1703,13 @@ static void intel_pstate_update_epp_defaults(struct cpudata *cpudata)
return;
}

+ /*
+ * 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;
+
/*
* Use hard coded value per gen to update the balance_perf
* and default EPP.
--
2.34.1


2023-12-05 20:32:30

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH] cpufreq: intel_pstate: Allow firmware balance performance EPP without code change

On Mon, Nov 20, 2023 at 7:59 PM Srinivas Pandruvada
<[email protected]> wrote:
>
> Firmware can specify balance performance EPP value by enabling HWP and
> set EPP to a desired value. The current implementation requires code
> change for every generation to add an entry to intel_epp_balance_perf
> table.
>
> Some distributions like Chrome, which uses old kernels should be able
> to update balance performance EPP, without code change.
>
> There is a check to avoid updating EPP when the balance performance
> EPP is not changed and is power up default of 0x80. Move this check
> after checking if the HWP is enabled by the firmware and there is
> a valid EPP value set by the firmware.
>
> Signed-off-by: Srinivas Pandruvada <[email protected]>
> ---
> drivers/cpufreq/intel_pstate.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index a534a1f7f1ee..dd6d23e389f1 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -1691,13 +1691,6 @@ 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 the EPP is set by firmware, which means that firmware enabled HWP
> * - Is equal or less than 0x80 (default balance_perf EPP)
> @@ -1710,6 +1703,13 @@ static void intel_pstate_update_epp_defaults(struct cpudata *cpudata)
> return;
> }
>
> + /*
> + * 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;
> +
> /*
> * Use hard coded value per gen to update the balance_perf
> * and default EPP.
> --

Applied as 6.8 material with modified subject and changelog, thanks!