2024-01-31 09:17:32

by Yuan, Perry

[permalink] [raw]
Subject: [PATCH 0/6] AMD Pstate Fixes And Enhancements

The patch series adds some fixes and enhancements to the AMD pstate driver.
It enables CPPC v2 for certain processors in the family 17H, as requested
by TR40 processor users who expect improved performance and lower system
temperature.

Additionally, it fixes the initialization of nominal_freq for each cpudata
and changes latency and delay values to be read from platform firmware firstly
for more accurate timing.

A new quirk is also added for legacy processors that lack CPPC capabilities,
which caused the pstate driver to fail loading.

I would greatly appreciate any feedbacks.

Thank you!


Perry Yuan (6):
ACPI: CPPC: enable AMD CPPC V2 support for family 17h processors
cpufreq:amd-pstate: fix the nominal freq value set
cpufreq:amd-pstate: initialize nominal_freq of each cpudata
cpufreq:amd-pstate: get pstate transition delay and latency value from
ACPI tables
cppc_acpi: print error message if CPPC is unsupported
cpufreq:amd-pstate: add quirk for the pstate CPPC capabilities missing

arch/x86/kernel/acpi/cppc.c | 2 +-
drivers/acpi/cppc_acpi.c | 6 +-
drivers/cpufreq/amd-pstate.c | 112 ++++++++++++++++++++++++++++-------
include/linux/amd-pstate.h | 6 ++
4 files changed, 102 insertions(+), 24 deletions(-)

--
2.34.1



2024-01-31 09:18:37

by Yuan, Perry

[permalink] [raw]
Subject: [PATCH 3/6] cpufreq:amd-pstate: initialize nominal_freq of each cpudata

Optimizes the process of retrieving the nominal frequency by utilizing
'cpudata->nominal_freq' instead of repeatedly accessing the cppc_acpi interface.

To enhance efficiency and reduce the CPU load, shifted to using
'cpudata->nominal_freq'. It allows for the nominal frequency to be accessed
directly from the cached data in 'cpudata' of each CPU.
It will also slightly reduce the frequency change latency while using pstate
driver passive mode.

Signed-off-by: Perry Yuan <[email protected]>
---
drivers/cpufreq/amd-pstate.c | 26 ++++++++++++++------------
1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 9291a22bd3cc..db7b36afdce2 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -600,7 +600,7 @@ static int amd_get_max_freq(struct amd_cpudata *cpudata)
if (ret)
return ret;

- nominal_freq = cppc_perf.nominal_freq;
+ nominal_freq = READ_ONCE(cpudata->nominal_freq);
nominal_perf = READ_ONCE(cpudata->nominal_perf);
max_perf = READ_ONCE(cpudata->highest_perf);

@@ -639,7 +639,7 @@ static int amd_get_lowest_nonlinear_freq(struct amd_cpudata *cpudata)
if (ret)
return ret;

- nominal_freq = cppc_perf.nominal_freq;
+ nominal_freq = READ_ONCE(cpudata->nominal_freq);
nominal_perf = READ_ONCE(cpudata->nominal_perf);

lowest_nonlinear_perf = cppc_perf.lowest_nonlinear_perf;
@@ -712,13 +712,15 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy)
amd_pstate_boost_init(cpudata);

min_freq = amd_get_min_freq(cpudata);
- max_freq = amd_get_max_freq(cpudata);
nominal_freq = amd_get_nominal_freq(cpudata);
+ cpudata->nominal_freq = nominal_freq;
+ max_freq = amd_get_max_freq(cpudata);
lowest_nonlinear_freq = amd_get_lowest_nonlinear_freq(cpudata);

- if (min_freq < 0 || max_freq < 0 || min_freq > max_freq) {
- dev_err(dev, "min_freq(%d) or max_freq(%d) value is incorrect\n",
- min_freq, max_freq);
+ if (min_freq < 0 || max_freq < 0 || min_freq > max_freq || nominal_freq == 0) {
+ dev_err(dev, "min_freq(%d) or max_freq(%d) or nominal_freq(%d) \
+ value is incorrect\n", \
+ min_freq, max_freq, nominal_freq);
ret = -EINVAL;
goto free_cpudata1;
}
@@ -755,7 +757,6 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy)
/* Initial processor data capability frequencies */
cpudata->max_freq = max_freq;
cpudata->min_freq = min_freq;
- cpudata->nominal_freq = nominal_freq;
cpudata->lowest_nonlinear_freq = lowest_nonlinear_freq;

policy->driver_data = cpudata;
@@ -1266,12 +1267,14 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
amd_pstate_boost_init(cpudata);

min_freq = amd_get_min_freq(cpudata);
- max_freq = amd_get_max_freq(cpudata);
nominal_freq = amd_get_nominal_freq(cpudata);
+ cpudata->nominal_freq = nominal_freq;
+ max_freq = amd_get_max_freq(cpudata);
lowest_nonlinear_freq = amd_get_lowest_nonlinear_freq(cpudata);
- if (min_freq < 0 || max_freq < 0 || min_freq > max_freq) {
- dev_err(dev, "min_freq(%d) or max_freq(%d) value is incorrect\n",
- min_freq, max_freq);
+ if (min_freq < 0 || max_freq < 0 || min_freq > max_freq || nominal_freq == 0) {
+ dev_err(dev, "min_freq(%d) or max_freq(%d) or nominal_freq(%d) \
+ value is incorrect\n", \
+ min_freq, max_freq, nominal_freq);
ret = -EINVAL;
goto free_cpudata1;
}
@@ -1284,7 +1287,6 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
/* Initial processor data capability frequencies */
cpudata->max_freq = max_freq;
cpudata->min_freq = min_freq;
- cpudata->nominal_freq = nominal_freq;
cpudata->lowest_nonlinear_freq = lowest_nonlinear_freq;

policy->driver_data = cpudata;
--
2.34.1


2024-01-31 11:38:19

by Tor Vic

[permalink] [raw]
Subject: Re: [PATCH 0/6] AMD Pstate Fixes And Enhancements



On 1/31/24 09:50, Perry Yuan wrote:
> The patch series adds some fixes and enhancements to the AMD pstate driver.
> It enables CPPC v2 for certain processors in the family 17H, as requested
> by TR40 processor users who expect improved performance and lower system
> temperature.
>
> Additionally, it fixes the initialization of nominal_freq for each cpudata
> and changes latency and delay values to be read from platform firmware firstly
> for more accurate timing.
>
> A new quirk is also added for legacy processors that lack CPPC capabilities,
> which caused the pstate driver to fail loading.
>
> I would greatly appreciate any feedbacks.
>

Hi Perry,

Which tree or patchset is this based on?
It doesn't apply cleanly onto either 6.7 or 6.8.

First I had to revert [1], then apply [2] before applying this patchset
and finally reapply [1].
I did not apply the "prefcore" patchset which I keep in a separate branch.

Is this correct or did I mess up something with my branches?

---
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v6.8-rc2&id=febab20caebac959fdc3d7520bc52de8b1184455

[2]
https://lore.kernel.org/linux-pm/[email protected]/

Cheers,
Tor Vic

> Thank you!
>
>
> Perry Yuan (6):
> ACPI: CPPC: enable AMD CPPC V2 support for family 17h processors
> cpufreq:amd-pstate: fix the nominal freq value set
> cpufreq:amd-pstate: initialize nominal_freq of each cpudata
> cpufreq:amd-pstate: get pstate transition delay and latency value from
> ACPI tables
> cppc_acpi: print error message if CPPC is unsupported
> cpufreq:amd-pstate: add quirk for the pstate CPPC capabilities missing
>
> arch/x86/kernel/acpi/cppc.c | 2 +-
> drivers/acpi/cppc_acpi.c | 6 +-
> drivers/cpufreq/amd-pstate.c | 112 ++++++++++++++++++++++++++++-------
> include/linux/amd-pstate.h | 6 ++
> 4 files changed, 102 insertions(+), 24 deletions(-)
>

2024-01-31 21:47:44

by Mario Limonciello

[permalink] [raw]
Subject: Re: [PATCH 3/6] cpufreq:amd-pstate: initialize nominal_freq of each cpudata

On 1/31/2024 02:50, Perry Yuan wrote:
> Optimizes the process of retrieving the nominal frequency by utilizing
> 'cpudata->nominal_freq' instead of repeatedly accessing the cppc_acpi interface.
>
> To enhance efficiency and reduce the CPU load, shifted to using
> 'cpudata->nominal_freq'. It allows for the nominal frequency to be accessed
> directly from the cached data in 'cpudata' of each CPU.
> It will also slightly reduce the frequency change latency while using pstate
> driver passive mode.
>
> Signed-off-by: Perry Yuan <[email protected]>
Reviewed-by: Mario Limonciello <[email protected]>
> ---
> drivers/cpufreq/amd-pstate.c | 26 ++++++++++++++------------
> 1 file changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 9291a22bd3cc..db7b36afdce2 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -600,7 +600,7 @@ static int amd_get_max_freq(struct amd_cpudata *cpudata)
> if (ret)
> return ret;
>
> - nominal_freq = cppc_perf.nominal_freq;
> + nominal_freq = READ_ONCE(cpudata->nominal_freq);
> nominal_perf = READ_ONCE(cpudata->nominal_perf);
> max_perf = READ_ONCE(cpudata->highest_perf);
>
> @@ -639,7 +639,7 @@ static int amd_get_lowest_nonlinear_freq(struct amd_cpudata *cpudata)
> if (ret)
> return ret;
>
> - nominal_freq = cppc_perf.nominal_freq;
> + nominal_freq = READ_ONCE(cpudata->nominal_freq);
> nominal_perf = READ_ONCE(cpudata->nominal_perf);
>
> lowest_nonlinear_perf = cppc_perf.lowest_nonlinear_perf;
> @@ -712,13 +712,15 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy)
> amd_pstate_boost_init(cpudata);
>
> min_freq = amd_get_min_freq(cpudata);
> - max_freq = amd_get_max_freq(cpudata);
> nominal_freq = amd_get_nominal_freq(cpudata);
> + cpudata->nominal_freq = nominal_freq;
> + max_freq = amd_get_max_freq(cpudata);
> lowest_nonlinear_freq = amd_get_lowest_nonlinear_freq(cpudata);
>
> - if (min_freq < 0 || max_freq < 0 || min_freq > max_freq) {
> - dev_err(dev, "min_freq(%d) or max_freq(%d) value is incorrect\n",
> - min_freq, max_freq);
> + if (min_freq < 0 || max_freq < 0 || min_freq > max_freq || nominal_freq == 0) {
> + dev_err(dev, "min_freq(%d) or max_freq(%d) or nominal_freq(%d) \
> + value is incorrect\n", \
> + min_freq, max_freq, nominal_freq);
> ret = -EINVAL;
> goto free_cpudata1;
> }
> @@ -755,7 +757,6 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy)
> /* Initial processor data capability frequencies */
> cpudata->max_freq = max_freq;
> cpudata->min_freq = min_freq;
> - cpudata->nominal_freq = nominal_freq;
> cpudata->lowest_nonlinear_freq = lowest_nonlinear_freq;
>
> policy->driver_data = cpudata;
> @@ -1266,12 +1267,14 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
> amd_pstate_boost_init(cpudata);
>
> min_freq = amd_get_min_freq(cpudata);
> - max_freq = amd_get_max_freq(cpudata);
> nominal_freq = amd_get_nominal_freq(cpudata);
> + cpudata->nominal_freq = nominal_freq;
> + max_freq = amd_get_max_freq(cpudata);
> lowest_nonlinear_freq = amd_get_lowest_nonlinear_freq(cpudata);
> - if (min_freq < 0 || max_freq < 0 || min_freq > max_freq) {
> - dev_err(dev, "min_freq(%d) or max_freq(%d) value is incorrect\n",
> - min_freq, max_freq);
> + if (min_freq < 0 || max_freq < 0 || min_freq > max_freq || nominal_freq == 0) {
> + dev_err(dev, "min_freq(%d) or max_freq(%d) or nominal_freq(%d) \
> + value is incorrect\n", \
> + min_freq, max_freq, nominal_freq);
> ret = -EINVAL;
> goto free_cpudata1;
> }
> @@ -1284,7 +1287,6 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
> /* Initial processor data capability frequencies */
> cpudata->max_freq = max_freq;
> cpudata->min_freq = min_freq;
> - cpudata->nominal_freq = nominal_freq;
> cpudata->lowest_nonlinear_freq = lowest_nonlinear_freq;
>
> policy->driver_data = cpudata;


2024-02-02 09:13:42

by Yuan, Perry

[permalink] [raw]
Subject: RE: [PATCH 0/6] AMD Pstate Fixes And Enhancements

[AMD Official Use Only - General]

Hi Vic

> -----Original Message-----
> From: Tor Vic <[email protected]>
> Sent: Wednesday, January 31, 2024 7:37 PM
> To: Yuan, Perry <[email protected]>; [email protected]; Limonciello,
> Mario <[email protected]>; [email protected]; Huang, Ray
> <[email protected]>; Shenoy, Gautham Ranjal
> <[email protected]>; Petkov, Borislav <[email protected]>
> Cc: Deucher, Alexander <[email protected]>; Huang, Shimmer
> <[email protected]>; Du, Xiaojian <[email protected]>; Meng, Li
> (Jassmine) <[email protected]>; [email protected]; linux-
> [email protected]
> Subject: Re: [PATCH 0/6] AMD Pstate Fixes And Enhancements
>
>
>
> On 1/31/24 09:50, Perry Yuan wrote:
> > The patch series adds some fixes and enhancements to the AMD pstate driver.
> > It enables CPPC v2 for certain processors in the family 17H, as
> > requested by TR40 processor users who expect improved performance and
> > lower system temperature.
> >
> > Additionally, it fixes the initialization of nominal_freq for each
> > cpudata and changes latency and delay values to be read from platform
> > firmware firstly for more accurate timing.
> >
> > A new quirk is also added for legacy processors that lack CPPC
> > capabilities, which caused the pstate driver to fail loading.
> >
> > I would greatly appreciate any feedbacks.
> >
>
> Hi Perry,
>
> Which tree or patchset is this based on?
> It doesn't apply cleanly onto either 6.7 or 6.8.
>
> First I had to revert [1], then apply [2] before applying this patchset and finally
> reapply [1].
> I did not apply the "prefcore" patchset which I keep in a separate branch.
>
> Is this correct or did I mess up something with my branches?


Hi Tor,

I have rebased the series to Linux-pm 6.8 rc2, you can pull the Linux-pm/bleeding-edge and apply the patches on top of that.
The part 1 prefer core patches are already merged into Linux-pm tree, so you can just merge this series
Here is the V2 https://lore.kernel.org/lkml/[email protected]/


git clone https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git
git checkout bleeding-edge

Perry.

>
> ---
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v6.
> 8-rc2&id=febab20caebac959fdc3d7520bc52de8b1184455
>
> [2]
> https://lore.kernel.org/linux-pm/[email protected]/
>
> Cheers,
> Tor Vic
>
> > Thank you!
> >
> >
> > Perry Yuan (6):
> > ACPI: CPPC: enable AMD CPPC V2 support for family 17h processors
> > cpufreq:amd-pstate: fix the nominal freq value set
> > cpufreq:amd-pstate: initialize nominal_freq of each cpudata
> > cpufreq:amd-pstate: get pstate transition delay and latency value from
> > ACPI tables
> > cppc_acpi: print error message if CPPC is unsupported
> > cpufreq:amd-pstate: add quirk for the pstate CPPC capabilities
> > missing
> >
> > arch/x86/kernel/acpi/cppc.c | 2 +-
> > drivers/acpi/cppc_acpi.c | 6 +-
> > drivers/cpufreq/amd-pstate.c | 112 ++++++++++++++++++++++++++++-------
> > include/linux/amd-pstate.h | 6 ++
> > 4 files changed, 102 insertions(+), 24 deletions(-)
> >