linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/7] AMD Pstate Enhancement And Issue Fixs
@ 2022-08-14 16:35 Perry Yuan
  2022-08-14 16:35 ` [PATCH v5 1/7] cpufreq: amd-pstate: cleanup the unused and duplicated headers declaration Perry Yuan
                   ` (6 more replies)
  0 siblings, 7 replies; 18+ messages in thread
From: Perry Yuan @ 2022-08-14 16:35 UTC (permalink / raw)
  To: rafael.j.wysocki, ray.huang, viresh.kumar
  Cc: Deepak.Sharma, Mario.Limonciello, Nathan.Fontenot,
	Alexander.Deucher, Jinzhou.Su, Shimmer.Huang, Xiaojian.Du,
	Li.Meng, linux-pm, linux-kernel, Perry Yuan

This patchsets adds support for precision boost hardware control
for AMD processors.

Meanwhile the patchset fixs lowest perf query and desired perf scope issues.
Update transition delay and latency default value to meet SMU firmware requirement.
and do some code cleanups,
It also exports cpufreq cpu release and acquire for coming amd-pstate epp mode driver

The patch series are tested on the AMD mobile and EYPC server systems

v4->v5:
- drop the amd precision boost mode control patch
- combine acpi_cpc_valid() changes into single patch
- pick up Reviewed-by flags by Huang Ray

v3->v4:
- check cached bit for core performance boost from hardware configuration
  register
- pick up the Acked-by flags from Viresh and Sudeep

v2->v3:
- drop cpufreq cpu release and acquire export patch
- remove the clamp_t in the amd_pstate_adjust_perf()

v1->v2:
- add two new patches to remove the acpi_disabled check
- fix some typos in the commit info
- move the clamp_t() into amd_pstate_update() function
- rebased to 5.19-rc5

Perry Yuan (7):
  cpufreq: amd-pstate: cleanup the unused and duplicated headers
    declaration
  cpufreq: amd-pstate: simplify cpudata pointer assignment
  cpufreq: amd-pstate: fix white-space
  cpufreq: amd_pstate: fix wrong lowest perf fetch
  cpufreq: amd_pstate: map desired perf into pstate scope for powersave
    governor
  cpufreq: amd-pstate: update pstate frequency transition delay time
  cpufreq: amd-pstate: add ACPI disabled check in acpi_cpc_valid()

 drivers/acpi/cppc_acpi.c       |  3 +++
 drivers/base/arch_topology.c   |  2 +-
 drivers/cpufreq/amd-pstate.c   | 32 ++++++++++----------------------
 drivers/cpufreq/cppc_cpufreq.c |  2 +-
 4 files changed, 15 insertions(+), 24 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v5 1/7] cpufreq: amd-pstate: cleanup the unused and duplicated headers declaration
  2022-08-14 16:35 [PATCH v5 0/7] AMD Pstate Enhancement And Issue Fixs Perry Yuan
@ 2022-08-14 16:35 ` Perry Yuan
  2022-08-15 15:04   ` Punit Agrawal
  2022-08-14 16:35 ` [PATCH v5 2/7] cpufreq: amd-pstate: simplify cpudata pointer assignment Perry Yuan
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Perry Yuan @ 2022-08-14 16:35 UTC (permalink / raw)
  To: rafael.j.wysocki, ray.huang, viresh.kumar
  Cc: Deepak.Sharma, Mario.Limonciello, Nathan.Fontenot,
	Alexander.Deucher, Jinzhou.Su, Shimmer.Huang, Xiaojian.Du,
	Li.Meng, linux-pm, linux-kernel, Perry Yuan

Cleanup the headers declaration which are not used
actually and some duplicated declaration which is declarated in some
other headers already, it will help to simplify the header part.

Reviewed-by: Huang Rui <ray.huang@amd.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
---
 drivers/cpufreq/amd-pstate.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 9ac75c1cde9c..19a078e232dd 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -31,19 +31,14 @@
 #include <linux/compiler.h>
 #include <linux/dmi.h>
 #include <linux/slab.h>
-#include <linux/acpi.h>
 #include <linux/io.h>
 #include <linux/delay.h>
 #include <linux/uaccess.h>
 #include <linux/static_call.h>
 
-#include <acpi/processor.h>
 #include <acpi/cppc_acpi.h>
 
 #include <asm/msr.h>
-#include <asm/processor.h>
-#include <asm/cpufeature.h>
-#include <asm/cpu_device_id.h>
 #include "amd-pstate-trace.h"
 
 #define AMD_PSTATE_TRANSITION_LATENCY	0x20000
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v5 2/7] cpufreq: amd-pstate: simplify cpudata pointer assignment
  2022-08-14 16:35 [PATCH v5 0/7] AMD Pstate Enhancement And Issue Fixs Perry Yuan
  2022-08-14 16:35 ` [PATCH v5 1/7] cpufreq: amd-pstate: cleanup the unused and duplicated headers declaration Perry Yuan
@ 2022-08-14 16:35 ` Perry Yuan
  2022-08-14 16:35 ` [PATCH v5 3/7] cpufreq: amd-pstate: fix white-space Perry Yuan
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 18+ messages in thread
From: Perry Yuan @ 2022-08-14 16:35 UTC (permalink / raw)
  To: rafael.j.wysocki, ray.huang, viresh.kumar
  Cc: Deepak.Sharma, Mario.Limonciello, Nathan.Fontenot,
	Alexander.Deucher, Jinzhou.Su, Shimmer.Huang, Xiaojian.Du,
	Li.Meng, linux-pm, linux-kernel, Perry Yuan

move the cpudata assignment to cpudata declaration which
will simplify the functions.

No functional change intended.

Reviewed-by: Huang Rui <ray.huang@amd.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
---
 drivers/cpufreq/amd-pstate.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 19a078e232dd..b31bb5e6cefc 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -550,9 +550,7 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy)
 
 static int amd_pstate_cpu_exit(struct cpufreq_policy *policy)
 {
-	struct amd_cpudata *cpudata;
-
-	cpudata = policy->driver_data;
+	struct amd_cpudata *cpudata = policy->driver_data;
 
 	freq_qos_remove_request(&cpudata->req[1]);
 	freq_qos_remove_request(&cpudata->req[0]);
@@ -594,9 +592,7 @@ static ssize_t show_amd_pstate_max_freq(struct cpufreq_policy *policy,
 					char *buf)
 {
 	int max_freq;
-	struct amd_cpudata *cpudata;
-
-	cpudata = policy->driver_data;
+	struct amd_cpudata *cpudata = policy->driver_data;
 
 	max_freq = amd_get_max_freq(cpudata);
 	if (max_freq < 0)
@@ -609,9 +605,7 @@ static ssize_t show_amd_pstate_lowest_nonlinear_freq(struct cpufreq_policy *poli
 						     char *buf)
 {
 	int freq;
-	struct amd_cpudata *cpudata;
-
-	cpudata = policy->driver_data;
+	struct amd_cpudata *cpudata = policy->driver_data;
 
 	freq = amd_get_lowest_nonlinear_freq(cpudata);
 	if (freq < 0)
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v5 3/7] cpufreq: amd-pstate: fix white-space
  2022-08-14 16:35 [PATCH v5 0/7] AMD Pstate Enhancement And Issue Fixs Perry Yuan
  2022-08-14 16:35 ` [PATCH v5 1/7] cpufreq: amd-pstate: cleanup the unused and duplicated headers declaration Perry Yuan
  2022-08-14 16:35 ` [PATCH v5 2/7] cpufreq: amd-pstate: simplify cpudata pointer assignment Perry Yuan
@ 2022-08-14 16:35 ` Perry Yuan
  2022-08-31 19:07   ` Rafael J. Wysocki
  2022-08-14 16:35 ` [PATCH v5 4/7] cpufreq: amd_pstate: fix wrong lowest perf fetch Perry Yuan
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Perry Yuan @ 2022-08-14 16:35 UTC (permalink / raw)
  To: rafael.j.wysocki, ray.huang, viresh.kumar
  Cc: Deepak.Sharma, Mario.Limonciello, Nathan.Fontenot,
	Alexander.Deucher, Jinzhou.Su, Shimmer.Huang, Xiaojian.Du,
	Li.Meng, linux-pm, linux-kernel, Perry Yuan

Remove the white space and correct mixed-up indentation

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
---
 drivers/cpufreq/amd-pstate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index b31bb5e6cefc..5cdef6638681 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -115,7 +115,7 @@ struct amd_cpudata {
 	struct amd_aperf_mperf cur;
 	struct amd_aperf_mperf prev;
 
-	u64 freq;
+	u64 	freq;
 	bool	boost_supported;
 };
 
@@ -651,7 +651,7 @@ static struct cpufreq_driver amd_pstate_driver = {
 	.resume		= amd_pstate_cpu_resume,
 	.set_boost	= amd_pstate_set_boost,
 	.name		= "amd-pstate",
-	.attr           = amd_pstate_attr,
+	.attr		= amd_pstate_attr,
 };
 
 static int __init amd_pstate_init(void)
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v5 4/7] cpufreq: amd_pstate: fix wrong lowest perf fetch
  2022-08-14 16:35 [PATCH v5 0/7] AMD Pstate Enhancement And Issue Fixs Perry Yuan
                   ` (2 preceding siblings ...)
  2022-08-14 16:35 ` [PATCH v5 3/7] cpufreq: amd-pstate: fix white-space Perry Yuan
@ 2022-08-14 16:35 ` Perry Yuan
  2022-08-15 15:05   ` Punit Agrawal
  2022-08-14 16:35 ` [PATCH v5 5/7] cpufreq: amd_pstate: map desired perf into pstate scope for powersave governor Perry Yuan
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Perry Yuan @ 2022-08-14 16:35 UTC (permalink / raw)
  To: rafael.j.wysocki, ray.huang, viresh.kumar
  Cc: Deepak.Sharma, Mario.Limonciello, Nathan.Fontenot,
	Alexander.Deucher, Jinzhou.Su, Shimmer.Huang, Xiaojian.Du,
	Li.Meng, linux-pm, linux-kernel, Perry Yuan, Su Jinzhou

Fix the wrong lowest perf value reading which is used for new
des_perf calculation by governor requested, the incorrect min_perf will
get incorrect des_perf to be set , that will cause the system frequency
changing unexpectedly.

Reviewed-by: Huang Rui <ray.huang@amd.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
Signed-off-by: Su Jinzhou <jinzhou.su@amd.com>
---
 drivers/cpufreq/amd-pstate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 5cdef6638681..183cdd4ba00e 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -307,7 +307,7 @@ static int amd_pstate_target(struct cpufreq_policy *policy,
 		return -ENODEV;
 
 	cap_perf = READ_ONCE(cpudata->highest_perf);
-	min_perf = READ_ONCE(cpudata->lowest_nonlinear_perf);
+	min_perf = READ_ONCE(cpudata->lowest_perf);
 	max_perf = cap_perf;
 
 	freqs.old = policy->cur;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v5 5/7] cpufreq: amd_pstate: map desired perf into pstate scope for powersave governor
  2022-08-14 16:35 [PATCH v5 0/7] AMD Pstate Enhancement And Issue Fixs Perry Yuan
                   ` (3 preceding siblings ...)
  2022-08-14 16:35 ` [PATCH v5 4/7] cpufreq: amd_pstate: fix wrong lowest perf fetch Perry Yuan
@ 2022-08-14 16:35 ` Perry Yuan
  2022-08-14 16:35 ` [PATCH v5 6/7] cpufreq: amd-pstate: update pstate frequency transition delay time Perry Yuan
  2022-08-14 16:35 ` [PATCH v5 7/7] cpufreq: amd-pstate: add ACPI disabled check in acpi_cpc_valid() Perry Yuan
  6 siblings, 0 replies; 18+ messages in thread
From: Perry Yuan @ 2022-08-14 16:35 UTC (permalink / raw)
  To: rafael.j.wysocki, ray.huang, viresh.kumar
  Cc: Deepak.Sharma, Mario.Limonciello, Nathan.Fontenot,
	Alexander.Deucher, Jinzhou.Su, Shimmer.Huang, Xiaojian.Du,
	Li.Meng, linux-pm, linux-kernel, Perry Yuan

The patch will fix the invalid desired perf value for powersave
governor. This issue is found when testing on one AMD EPYC system, the
actual des_perf is smaller than the min_perf value, that is invalid
value. because the min_perf is the lowest_perf system can support in
idle state.

Reviewed-by: Huang Rui <ray.huang@amd.com>
Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
---
 drivers/cpufreq/amd-pstate.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 183cdd4ba00e..e40177d14310 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -264,6 +264,7 @@ static void amd_pstate_update(struct amd_cpudata *cpudata, u32 min_perf,
 	u64 prev = READ_ONCE(cpudata->cppc_req_cached);
 	u64 value = prev;
 
+	des_perf = clamp_t(unsigned long, des_perf, min_perf, max_perf);
 	value &= ~AMD_CPPC_MIN_PERF(~0L);
 	value |= AMD_CPPC_MIN_PERF(min_perf);
 
@@ -352,8 +353,6 @@ static void amd_pstate_adjust_perf(unsigned int cpu,
 	if (max_perf < min_perf)
 		max_perf = min_perf;
 
-	des_perf = clamp_t(unsigned long, des_perf, min_perf, max_perf);
-
 	amd_pstate_update(cpudata, min_perf, des_perf, max_perf, true);
 }
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v5 6/7] cpufreq: amd-pstate: update pstate frequency transition delay time
  2022-08-14 16:35 [PATCH v5 0/7] AMD Pstate Enhancement And Issue Fixs Perry Yuan
                   ` (4 preceding siblings ...)
  2022-08-14 16:35 ` [PATCH v5 5/7] cpufreq: amd_pstate: map desired perf into pstate scope for powersave governor Perry Yuan
@ 2022-08-14 16:35 ` Perry Yuan
  2022-08-15 15:05   ` Punit Agrawal
  2022-08-14 16:35 ` [PATCH v5 7/7] cpufreq: amd-pstate: add ACPI disabled check in acpi_cpc_valid() Perry Yuan
  6 siblings, 1 reply; 18+ messages in thread
From: Perry Yuan @ 2022-08-14 16:35 UTC (permalink / raw)
  To: rafael.j.wysocki, ray.huang, viresh.kumar
  Cc: Deepak.Sharma, Mario.Limonciello, Nathan.Fontenot,
	Alexander.Deucher, Jinzhou.Su, Shimmer.Huang, Xiaojian.Du,
	Li.Meng, linux-pm, linux-kernel, Perry Yuan

Change the default transition latency to be 20ms that is more
reasonable transition delay for AMD processors in non-EPP driver mode.

Update transition delay time to 1ms, in the AMD CPU autonomous mode and
non-autonomous mode, CPPC firmware will decide frequency at 1ms timescale
based on the workload utilization.

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
---
 drivers/cpufreq/amd-pstate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index e40177d14310..9cb051d61422 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -41,8 +41,8 @@
 #include <asm/msr.h>
 #include "amd-pstate-trace.h"
 
-#define AMD_PSTATE_TRANSITION_LATENCY	0x20000
-#define AMD_PSTATE_TRANSITION_DELAY	500
+#define AMD_PSTATE_TRANSITION_LATENCY	20000
+#define AMD_PSTATE_TRANSITION_DELAY	1000
 
 /*
  * TODO: We need more time to fine tune processors with shared memory solution
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v5 7/7] cpufreq: amd-pstate: add ACPI disabled check in acpi_cpc_valid()
  2022-08-14 16:35 [PATCH v5 0/7] AMD Pstate Enhancement And Issue Fixs Perry Yuan
                   ` (5 preceding siblings ...)
  2022-08-14 16:35 ` [PATCH v5 6/7] cpufreq: amd-pstate: update pstate frequency transition delay time Perry Yuan
@ 2022-08-14 16:35 ` Perry Yuan
  2022-08-25 11:56   ` Rafael J. Wysocki
  6 siblings, 1 reply; 18+ messages in thread
From: Perry Yuan @ 2022-08-14 16:35 UTC (permalink / raw)
  To: rafael.j.wysocki, ray.huang, viresh.kumar
  Cc: Deepak.Sharma, Mario.Limonciello, Nathan.Fontenot,
	Alexander.Deucher, Jinzhou.Su, Shimmer.Huang, Xiaojian.Du,
	Li.Meng, linux-pm, linux-kernel, Perry Yuan

Add acpi function check in case ACPI is not enabled, that will cause
pstate driver failed to call cppc acpi to change perf or update epp
value for shared memory solution processors.

When CPPC or ACPI is invalid, warning log will be needed to tell
user that AMD pstate driver failed to load and what is wrong.

Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
---
 drivers/acpi/cppc_acpi.c       | 3 +++
 drivers/base/arch_topology.c   | 2 +-
 drivers/cpufreq/amd-pstate.c   | 2 +-
 drivers/cpufreq/cppc_cpufreq.c | 2 +-
 4 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 1e15a9f25ae9..c2309429146f 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -424,6 +424,9 @@ bool acpi_cpc_valid(void)
 	struct cpc_desc *cpc_ptr;
 	int cpu;
 
+	if (acpi_disabled)
+		return false;
+
 	for_each_present_cpu(cpu) {
 		cpc_ptr = per_cpu(cpc_desc_ptr, cpu);
 		if (!cpc_ptr)
diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index 579c851a2bd7..73a8cb31529d 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -352,7 +352,7 @@ void topology_init_cpu_capacity_cppc(void)
 	struct cppc_perf_caps perf_caps;
 	int cpu;
 
-	if (likely(acpi_disabled || !acpi_cpc_valid()))
+	if (likely(!acpi_cpc_valid()))
 		return;
 
 	raw_capacity = kcalloc(num_possible_cpus(), sizeof(*raw_capacity),
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 9cb051d61422..96e4ecddf3f6 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -661,7 +661,7 @@ static int __init amd_pstate_init(void)
 		return -ENODEV;
 
 	if (!acpi_cpc_valid()) {
-		pr_debug("the _CPC object is not present in SBIOS\n");
+		pr_warn_once("the _CPC object is not present in SBIOS or ACPI disabled\n");
 		return -ENODEV;
 	}
 
diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
index 24eaf0ec344d..9adb7612993e 100644
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -947,7 +947,7 @@ static int __init cppc_cpufreq_init(void)
 {
 	int ret;
 
-	if ((acpi_disabled) || !acpi_cpc_valid())
+	if (!acpi_cpc_valid())
 		return -ENODEV;
 
 	cppc_check_hisi_workaround();
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 1/7] cpufreq: amd-pstate: cleanup the unused and duplicated headers declaration
  2022-08-14 16:35 ` [PATCH v5 1/7] cpufreq: amd-pstate: cleanup the unused and duplicated headers declaration Perry Yuan
@ 2022-08-15 15:04   ` Punit Agrawal
  2022-09-01  5:34     ` Yuan, Perry
  0 siblings, 1 reply; 18+ messages in thread
From: Punit Agrawal @ 2022-08-15 15:04 UTC (permalink / raw)
  To: Perry Yuan
  Cc: rafael.j.wysocki, ray.huang, viresh.kumar, Deepak.Sharma,
	Mario.Limonciello, Nathan.Fontenot, Alexander.Deucher,
	Jinzhou.Su, Shimmer.Huang, Xiaojian.Du, Li.Meng, linux-pm,
	linux-kernel

Hi Perry,

Perry Yuan <Perry.Yuan@amd.com> writes:

> Cleanup the headers declaration which are not used
> actually and some duplicated declaration which is declarated in some
> other headers already, it will help to simplify the header part.

We usually don't get rid of indirectly included headers as long as
definitions from header are used in the code. This avoids problems if
for some reason the included header gets dropped - it'll leave the code
in an uncompilable state.

More below.

>
> Reviewed-by: Huang Rui <ray.huang@amd.com>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
> ---
>  drivers/cpufreq/amd-pstate.c | 5 -----
>  1 file changed, 5 deletions(-)
>
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 9ac75c1cde9c..19a078e232dd 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -31,19 +31,14 @@
>  #include <linux/compiler.h>
>  #include <linux/dmi.h>
>  #include <linux/slab.h>
> -#include <linux/acpi.h>
>  #include <linux/io.h>
>  #include <linux/delay.h>
>  #include <linux/uaccess.h>
>  #include <linux/static_call.h>
>  
> -#include <acpi/processor.h>
>  #include <acpi/cppc_acpi.h>
>  
>  #include <asm/msr.h>
> -#include <asm/processor.h>

On a quick scan, I noticed that "boot_cpu_data" and "boot_cpu_has()" in
the module init function are defined in "asm/processor.h" that is being
removed here. It may compile for now but makes the code more fragile as
explained above.

Please ensure that only the header files that have no definitions used
in this file (amd-pstate.c) are dropped.

> -#include <asm/cpufeature.h>
> -#include <asm/cpu_device_id.h>
>  #include "amd-pstate-trace.h"
>  
>  #define AMD_PSTATE_TRANSITION_LATENCY	0x20000

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 4/7] cpufreq: amd_pstate: fix wrong lowest perf fetch
  2022-08-14 16:35 ` [PATCH v5 4/7] cpufreq: amd_pstate: fix wrong lowest perf fetch Perry Yuan
@ 2022-08-15 15:05   ` Punit Agrawal
  2022-08-31  8:53     ` Yuan, Perry
  0 siblings, 1 reply; 18+ messages in thread
From: Punit Agrawal @ 2022-08-15 15:05 UTC (permalink / raw)
  To: Perry Yuan
  Cc: rafael.j.wysocki, ray.huang, viresh.kumar, Deepak.Sharma,
	Mario.Limonciello, Nathan.Fontenot, Alexander.Deucher,
	Jinzhou.Su, Shimmer.Huang, Xiaojian.Du, Li.Meng, linux-pm,
	linux-kernel

Hi Perry,

Perry Yuan <Perry.Yuan@amd.com> writes:

> Fix the wrong lowest perf value reading which is used for new
> des_perf calculation by governor requested, the incorrect min_perf will
> get incorrect des_perf to be set , that will cause the system frequency
> changing unexpectedly.
>
> Reviewed-by: Huang Rui <ray.huang@amd.com>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
> Signed-off-by: Su Jinzhou <jinzhou.su@amd.com>
> ---
>  drivers/cpufreq/amd-pstate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 5cdef6638681..183cdd4ba00e 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -307,7 +307,7 @@ static int amd_pstate_target(struct cpufreq_policy *policy,
>  		return -ENODEV;
>  
>  	cap_perf = READ_ONCE(cpudata->highest_perf);
> -	min_perf = READ_ONCE(cpudata->lowest_nonlinear_perf);
> +	min_perf = READ_ONCE(cpudata->lowest_perf);
>  	max_perf = cap_perf;
>  
>  	freqs.old = policy->cur;

This looks to be a pretty big change (lowest nonlinear vs lowest). Does
the patch need to be backported to older kernels?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 6/7] cpufreq: amd-pstate: update pstate frequency transition delay time
  2022-08-14 16:35 ` [PATCH v5 6/7] cpufreq: amd-pstate: update pstate frequency transition delay time Perry Yuan
@ 2022-08-15 15:05   ` Punit Agrawal
  2022-08-16  7:02     ` Yuan, Perry
  0 siblings, 1 reply; 18+ messages in thread
From: Punit Agrawal @ 2022-08-15 15:05 UTC (permalink / raw)
  To: Perry Yuan
  Cc: rafael.j.wysocki, ray.huang, viresh.kumar, Deepak.Sharma,
	Mario.Limonciello, Nathan.Fontenot, Alexander.Deucher,
	Jinzhou.Su, Shimmer.Huang, Xiaojian.Du, Li.Meng, linux-pm,
	linux-kernel

Perry Yuan <Perry.Yuan@amd.com> writes:

> Change the default transition latency to be 20ms that is more
> reasonable transition delay for AMD processors in non-EPP driver mode.
>
> Update transition delay time to 1ms, in the AMD CPU autonomous mode and
> non-autonomous mode, CPPC firmware will decide frequency at 1ms timescale
> based on the workload utilization.
>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
> ---
>  drivers/cpufreq/amd-pstate.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index e40177d14310..9cb051d61422 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -41,8 +41,8 @@
>  #include <asm/msr.h>
>  #include "amd-pstate-trace.h"
>  
> -#define AMD_PSTATE_TRANSITION_LATENCY	0x20000
> -#define AMD_PSTATE_TRANSITION_DELAY	500
> +#define AMD_PSTATE_TRANSITION_LATENCY	20000
> +#define AMD_PSTATE_TRANSITION_DELAY	1000

How were these values derived? If from documentation, it'll be good to
add a link to the relevant documentation. And if they were derived from
testing, please mention this in the commit log (along with some details
of the tests used to determine the value).

>  
>  /*
>   * TODO: We need more time to fine tune processors with shared memory solution

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH v5 6/7] cpufreq: amd-pstate: update pstate frequency transition delay time
  2022-08-15 15:05   ` Punit Agrawal
@ 2022-08-16  7:02     ` Yuan, Perry
  2022-08-16 16:03       ` Punit Agrawal
  0 siblings, 1 reply; 18+ messages in thread
From: Yuan, Perry @ 2022-08-16  7:02 UTC (permalink / raw)
  To: Punit Agrawal
  Cc: rafael.j.wysocki, Huang, Ray, viresh.kumar, Sharma, Deepak,
	Limonciello, Mario, Fontenot, Nathan, Deucher, Alexander, Su,
	Jinzhou (Joe), Huang, Shimmer, Du, Xiaojian, Meng, Li (Jassmine),
	linux-pm, linux-kernel

[AMD Official Use Only - General]

Hi Punit

> -----Original Message-----
> From: Punit Agrawal <punit.agrawal@bytedance.com>
> Sent: Monday, August 15, 2022 11:06 PM
> To: Yuan, Perry <Perry.Yuan@amd.com>
> Cc: rafael.j.wysocki@intel.com; Huang, Ray <Ray.Huang@amd.com>;
> viresh.kumar@linaro.org; Sharma, Deepak <Deepak.Sharma@amd.com>;
> Limonciello, Mario <Mario.Limonciello@amd.com>; Fontenot, Nathan
> <Nathan.Fontenot@amd.com>; Deucher, Alexander
> <Alexander.Deucher@amd.com>; Su, Jinzhou (Joe) <Jinzhou.Su@amd.com>;
> Huang, Shimmer <Shimmer.Huang@amd.com>; Du, Xiaojian
> <Xiaojian.Du@amd.com>; Meng, Li (Jassmine) <Li.Meng@amd.com>; linux-
> pm@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v5 6/7] cpufreq: amd-pstate: update pstate frequency
> transition delay time
> 
> [CAUTION: External Email]
> 
> Perry Yuan <Perry.Yuan@amd.com> writes:
> 
> > Change the default transition latency to be 20ms that is more
> > reasonable transition delay for AMD processors in non-EPP driver mode.
> >
> > Update transition delay time to 1ms, in the AMD CPU autonomous mode
> > and non-autonomous mode, CPPC firmware will decide frequency at 1ms
> > timescale based on the workload utilization.
> >
> > Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> > Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
> > ---
> >  drivers/cpufreq/amd-pstate.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/cpufreq/amd-pstate.c
> > b/drivers/cpufreq/amd-pstate.c index e40177d14310..9cb051d61422 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -41,8 +41,8 @@
> >  #include <asm/msr.h>
> >  #include "amd-pstate-trace.h"
> >
> > -#define AMD_PSTATE_TRANSITION_LATENCY        0x20000
> > -#define AMD_PSTATE_TRANSITION_DELAY  500
> > +#define AMD_PSTATE_TRANSITION_LATENCY        20000
> > +#define AMD_PSTATE_TRANSITION_DELAY  1000
> 
> How were these values derived? If from documentation, it'll be good to add a
> link to the relevant documentation. And if they were derived from testing,
> please mention this in the commit log (along with some details of the tests used
> to determine the value).

The values are calculated from the CPU PM firmware and hardware design.
There are some latency and delay values defined  in the PM firmware, I have no documents about the detail for now. 

Perry.
> 
> >
> >  /*
> >   * TODO: We need more time to fine tune processors with shared memory
> > solution

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 6/7] cpufreq: amd-pstate: update pstate frequency transition delay time
  2022-08-16  7:02     ` Yuan, Perry
@ 2022-08-16 16:03       ` Punit Agrawal
  0 siblings, 0 replies; 18+ messages in thread
From: Punit Agrawal @ 2022-08-16 16:03 UTC (permalink / raw)
  To: Yuan, Perry
  Cc: Punit Agrawal, rafael.j.wysocki, Huang, Ray, viresh.kumar,
	Sharma, Deepak, Limonciello, Mario, Fontenot, Nathan, Deucher,
	Alexander, Su, Jinzhou (Joe),
	Huang, Shimmer, Du, Xiaojian, Meng, Li (Jassmine),
	linux-pm, linux-kernel

Hi Perry,

"Yuan, Perry" <Perry.Yuan@amd.com> writes:

> [AMD Official Use Only - General]
>
> Hi Punit
>
>> -----Original Message-----
>> From: Punit Agrawal <punit.agrawal@bytedance.com>
>> Sent: Monday, August 15, 2022 11:06 PM
>> To: Yuan, Perry <Perry.Yuan@amd.com>
>> Cc: rafael.j.wysocki@intel.com; Huang, Ray <Ray.Huang@amd.com>;
>> viresh.kumar@linaro.org; Sharma, Deepak <Deepak.Sharma@amd.com>;
>> Limonciello, Mario <Mario.Limonciello@amd.com>; Fontenot, Nathan
>> <Nathan.Fontenot@amd.com>; Deucher, Alexander
>> <Alexander.Deucher@amd.com>; Su, Jinzhou (Joe) <Jinzhou.Su@amd.com>;
>> Huang, Shimmer <Shimmer.Huang@amd.com>; Du, Xiaojian
>> <Xiaojian.Du@amd.com>; Meng, Li (Jassmine) <Li.Meng@amd.com>; linux-
>> pm@vger.kernel.org; linux-kernel@vger.kernel.org
>> Subject: Re: [PATCH v5 6/7] cpufreq: amd-pstate: update pstate frequency
>> transition delay time
>> 
>> [CAUTION: External Email]
>> 
>> Perry Yuan <Perry.Yuan@amd.com> writes:
>> 
>> > Change the default transition latency to be 20ms that is more
>> > reasonable transition delay for AMD processors in non-EPP driver mode.
>> >
>> > Update transition delay time to 1ms, in the AMD CPU autonomous mode
>> > and non-autonomous mode, CPPC firmware will decide frequency at 1ms
>> > timescale based on the workload utilization.
>> >
>> > Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
>> > Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
>> > ---
>> >  drivers/cpufreq/amd-pstate.c | 4 ++--
>> >  1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/cpufreq/amd-pstate.c
>> > b/drivers/cpufreq/amd-pstate.c index e40177d14310..9cb051d61422 100644
>> > --- a/drivers/cpufreq/amd-pstate.c
>> > +++ b/drivers/cpufreq/amd-pstate.c
>> > @@ -41,8 +41,8 @@
>> >  #include <asm/msr.h>
>> >  #include "amd-pstate-trace.h"
>> >
>> > -#define AMD_PSTATE_TRANSITION_LATENCY        0x20000
>> > -#define AMD_PSTATE_TRANSITION_DELAY  500
>> > +#define AMD_PSTATE_TRANSITION_LATENCY        20000
>> > +#define AMD_PSTATE_TRANSITION_DELAY  1000
>> 
>> How were these values derived? If from documentation, it'll be good to add a
>> link to the relevant documentation. And if they were derived from testing,
>> please mention this in the commit log (along with some details of the tests used
>> to determine the value).
>
> The values are calculated from the CPU PM firmware and hardware design.
> There are some latency and delay values defined  in the PM firmware, I have no documents about the detail for now. 

In that case, please mention that the values are calculated from
firmware / hardware design in the commit log (and include a reference to
the firmware sources if available).

Thanks!

[...]


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 7/7] cpufreq: amd-pstate: add ACPI disabled check in acpi_cpc_valid()
  2022-08-14 16:35 ` [PATCH v5 7/7] cpufreq: amd-pstate: add ACPI disabled check in acpi_cpc_valid() Perry Yuan
@ 2022-08-25 11:56   ` Rafael J. Wysocki
  0 siblings, 0 replies; 18+ messages in thread
From: Rafael J. Wysocki @ 2022-08-25 11:56 UTC (permalink / raw)
  To: Perry Yuan
  Cc: Rafael Wysocki, Huang Rui, Viresh Kumar, Deepak Sharma,
	Mario Limonciello, Nathan Fontenot, Alex Deucher, Su,
	Jinzhou (Joe), Shimmer.Huang, Du, Xiaojian, Meng, Li (Jassmine),
	Linux PM, Linux Kernel Mailing List

On Sun, Aug 14, 2022 at 6:53 PM Perry Yuan <Perry.Yuan@amd.com> wrote:
>
> Add acpi function check in case ACPI is not enabled, that will cause
> pstate driver failed to call cppc acpi to change perf or update epp
> value for shared memory solution processors.
>
> When CPPC or ACPI is invalid, warning log will be needed to tell
> user that AMD pstate driver failed to load and what is wrong.
>
> Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
> ---
>  drivers/acpi/cppc_acpi.c       | 3 +++
>  drivers/base/arch_topology.c   | 2 +-
>  drivers/cpufreq/amd-pstate.c   | 2 +-
>  drivers/cpufreq/cppc_cpufreq.c | 2 +-
>  4 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index 1e15a9f25ae9..c2309429146f 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -424,6 +424,9 @@ bool acpi_cpc_valid(void)
>         struct cpc_desc *cpc_ptr;
>         int cpu;
>
> +       if (acpi_disabled)
> +               return false;
> +
>         for_each_present_cpu(cpu) {
>                 cpc_ptr = per_cpu(cpc_desc_ptr, cpu);
>                 if (!cpc_ptr)
> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> index 579c851a2bd7..73a8cb31529d 100644
> --- a/drivers/base/arch_topology.c
> +++ b/drivers/base/arch_topology.c
> @@ -352,7 +352,7 @@ void topology_init_cpu_capacity_cppc(void)
>         struct cppc_perf_caps perf_caps;
>         int cpu;
>
> -       if (likely(acpi_disabled || !acpi_cpc_valid()))
> +       if (likely(!acpi_cpc_valid()))
>                 return;
>
>         raw_capacity = kcalloc(num_possible_cpus(), sizeof(*raw_capacity),
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 9cb051d61422..96e4ecddf3f6 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -661,7 +661,7 @@ static int __init amd_pstate_init(void)
>                 return -ENODEV;
>
>         if (!acpi_cpc_valid()) {
> -               pr_debug("the _CPC object is not present in SBIOS\n");
> +               pr_warn_once("the _CPC object is not present in SBIOS or ACPI disabled\n");
>                 return -ENODEV;
>         }
>
> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> index 24eaf0ec344d..9adb7612993e 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -947,7 +947,7 @@ static int __init cppc_cpufreq_init(void)
>  {
>         int ret;
>
> -       if ((acpi_disabled) || !acpi_cpc_valid())
> +       if (!acpi_cpc_valid())
>                 return -ENODEV;
>
>         cppc_check_hisi_workaround();
> --

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH v5 4/7] cpufreq: amd_pstate: fix wrong lowest perf fetch
  2022-08-15 15:05   ` Punit Agrawal
@ 2022-08-31  8:53     ` Yuan, Perry
  2022-09-01 14:58       ` Punit Agrawal
  0 siblings, 1 reply; 18+ messages in thread
From: Yuan, Perry @ 2022-08-31  8:53 UTC (permalink / raw)
  To: Punit Agrawal
  Cc: rafael.j.wysocki, Huang, Ray, viresh.kumar, Sharma, Deepak,
	Limonciello, Mario, Fontenot, Nathan, Deucher, Alexander, Su,
	Jinzhou (Joe), Huang, Shimmer, Du, Xiaojian, Meng, Li (Jassmine),
	linux-pm, linux-kernel

[AMD Official Use Only - General]

Hi Agrawal,

> -----Original Message-----
> From: Punit Agrawal <punit.agrawal@bytedance.com>
> Sent: Monday, August 15, 2022 11:05 PM
> To: Yuan, Perry <Perry.Yuan@amd.com>
> Cc: rafael.j.wysocki@intel.com; Huang, Ray <Ray.Huang@amd.com>;
> viresh.kumar@linaro.org; Sharma, Deepak <Deepak.Sharma@amd.com>;
> Limonciello, Mario <Mario.Limonciello@amd.com>; Fontenot, Nathan
> <Nathan.Fontenot@amd.com>; Deucher, Alexander
> <Alexander.Deucher@amd.com>; Su, Jinzhou (Joe) <Jinzhou.Su@amd.com>;
> Huang, Shimmer <Shimmer.Huang@amd.com>; Du, Xiaojian
> <Xiaojian.Du@amd.com>; Meng, Li (Jassmine) <Li.Meng@amd.com>; linux-
> pm@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v5 4/7] cpufreq: amd_pstate: fix wrong lowest perf fetch
> 
> [CAUTION: External Email]
> 
> Hi Perry,
> 
> Perry Yuan <Perry.Yuan@amd.com> writes:
> 
> > Fix the wrong lowest perf value reading which is used for new des_perf
> > calculation by governor requested, the incorrect min_perf will get
> > incorrect des_perf to be set , that will cause the system frequency
> > changing unexpectedly.
> >
> > Reviewed-by: Huang Rui <ray.huang@amd.com>
> > Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> > Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
> > Signed-off-by: Su Jinzhou <jinzhou.su@amd.com>
> > ---
> >  drivers/cpufreq/amd-pstate.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/cpufreq/amd-pstate.c
> > b/drivers/cpufreq/amd-pstate.c index 5cdef6638681..183cdd4ba00e 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -307,7 +307,7 @@ static int amd_pstate_target(struct cpufreq_policy
> *policy,
> >               return -ENODEV;
> >
> >       cap_perf = READ_ONCE(cpudata->highest_perf);
> > -     min_perf = READ_ONCE(cpudata->lowest_nonlinear_perf);
> > +     min_perf = READ_ONCE(cpudata->lowest_perf);
> >       max_perf = cap_perf;
> >
> >       freqs.old = policy->cur;
> 
> This looks to be a pretty big change (lowest nonlinear vs lowest). Does the patch
> need to be backported to older kernels?

The patch fixes the min perf initial value, the correct min perf is lowest_perf which is captured through MSR_AMD_CPPC_CAP1 register or the cppc_get_perf_caps().
Yes, the patch will need to be backported to other kernel branch as issue fix.
Sorry to response late.

Perry.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 3/7] cpufreq: amd-pstate: fix white-space
  2022-08-14 16:35 ` [PATCH v5 3/7] cpufreq: amd-pstate: fix white-space Perry Yuan
@ 2022-08-31 19:07   ` Rafael J. Wysocki
  0 siblings, 0 replies; 18+ messages in thread
From: Rafael J. Wysocki @ 2022-08-31 19:07 UTC (permalink / raw)
  To: Perry Yuan
  Cc: Rafael Wysocki, Huang Rui, Viresh Kumar, Deepak Sharma,
	Mario Limonciello, Nathan Fontenot, Alex Deucher, Su,
	Jinzhou (Joe), Shimmer.Huang, Du, Xiaojian, Meng, Li (Jassmine),
	Linux PM, Linux Kernel Mailing List

On Sun, Aug 14, 2022 at 6:49 PM Perry Yuan <Perry.Yuan@amd.com> wrote:
>
> Remove the white space and correct mixed-up indentation
>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
> ---
>  drivers/cpufreq/amd-pstate.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index b31bb5e6cefc..5cdef6638681 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -115,7 +115,7 @@ struct amd_cpudata {
>         struct amd_aperf_mperf cur;
>         struct amd_aperf_mperf prev;
>
> -       u64 freq;
> +       u64     freq;
>         bool    boost_supported;
>  };
>
> @@ -651,7 +651,7 @@ static struct cpufreq_driver amd_pstate_driver = {
>         .resume         = amd_pstate_cpu_resume,
>         .set_boost      = amd_pstate_set_boost,
>         .name           = "amd-pstate",
> -       .attr           = amd_pstate_attr,
> +       .attr           = amd_pstate_attr,
>  };
>
>  static int __init amd_pstate_init(void)
> --

Applied as 6.1 material along with patches [3-6/7].

The [7/7] was applied earlier and there was a change request for patch [1/7].

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH v5 1/7] cpufreq: amd-pstate: cleanup the unused and duplicated headers declaration
  2022-08-15 15:04   ` Punit Agrawal
@ 2022-09-01  5:34     ` Yuan, Perry
  0 siblings, 0 replies; 18+ messages in thread
From: Yuan, Perry @ 2022-09-01  5:34 UTC (permalink / raw)
  To: Punit Agrawal
  Cc: rafael.j.wysocki, Huang, Ray, viresh.kumar, Sharma, Deepak,
	Limonciello, Mario, Fontenot, Nathan, Deucher, Alexander, Su,
	Jinzhou (Joe), Huang, Shimmer, Du, Xiaojian, Meng, Li (Jassmine),
	linux-pm, linux-kernel

[AMD Official Use Only - General]

Hi Punit, 

> -----Original Message-----
> From: Punit Agrawal <punit.agrawal@bytedance.com>
> Sent: Monday, August 15, 2022 11:05 PM
> To: Yuan, Perry <Perry.Yuan@amd.com>
> Cc: rafael.j.wysocki@intel.com; Huang, Ray <Ray.Huang@amd.com>;
> viresh.kumar@linaro.org; Sharma, Deepak <Deepak.Sharma@amd.com>;
> Limonciello, Mario <Mario.Limonciello@amd.com>; Fontenot, Nathan
> <Nathan.Fontenot@amd.com>; Deucher, Alexander
> <Alexander.Deucher@amd.com>; Su, Jinzhou (Joe) <Jinzhou.Su@amd.com>;
> Huang, Shimmer <Shimmer.Huang@amd.com>; Du, Xiaojian
> <Xiaojian.Du@amd.com>; Meng, Li (Jassmine) <Li.Meng@amd.com>; linux-
> pm@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v5 1/7] cpufreq: amd-pstate: cleanup the unused and
> duplicated headers declaration
> 
> [CAUTION: External Email]
> 
> Hi Perry,
> 
> Perry Yuan <Perry.Yuan@amd.com> writes:
> 
> > Cleanup the headers declaration which are not used actually and some
> > duplicated declaration which is declarated in some other headers
> > already, it will help to simplify the header part.
> 
> We usually don't get rid of indirectly included headers as long as definitions from
> header are used in the code. This avoids problems if for some reason the
> included header gets dropped - it'll leave the code in an uncompilable state.
> 
> More below.
> 
> >
> > Reviewed-by: Huang Rui <ray.huang@amd.com>
> > Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> > Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
> > ---
> >  drivers/cpufreq/amd-pstate.c | 5 -----
> >  1 file changed, 5 deletions(-)
> >
> > diff --git a/drivers/cpufreq/amd-pstate.c
> > b/drivers/cpufreq/amd-pstate.c index 9ac75c1cde9c..19a078e232dd 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -31,19 +31,14 @@
> >  #include <linux/compiler.h>
> >  #include <linux/dmi.h>
> >  #include <linux/slab.h>
> > -#include <linux/acpi.h>
> >  #include <linux/io.h>
> >  #include <linux/delay.h>
> >  #include <linux/uaccess.h>
> >  #include <linux/static_call.h>
> >
> > -#include <acpi/processor.h>
> >  #include <acpi/cppc_acpi.h>
> >
> >  #include <asm/msr.h>
> > -#include <asm/processor.h>
> 
> On a quick scan, I noticed that "boot_cpu_data" and "boot_cpu_has()" in the
> module init function are defined in "asm/processor.h" that is being removed
> here. It may compile for now but makes the code more fragile as explained
> above.
> 
> Please ensure that only the header files that have no definitions used in this file
> (amd-pstate.c) are dropped.

Here is the part of the headers we are working on.

==============================================
#include <linux/static_call.h>

#include <acpi/cppc_acpi.h>

#include <asm/msr.h>
#include "amd-pstate-trace.h"

============================================

I have tested the latest Linux-pm/bleeding-edge branch to verify the changes working.
As I know  the "asm/processor.h" is included  by the  "acpi/cppc_acpi.h" subpath

#include <acpi/cppc_acpi.h> 
   #include <linux/acpi.h>
         #include <acpi/acpi.h> 
              #include <asm/acpi.h>  
                       #include <asm/processor.h>
 
To make it comfortable for us, I will send V6 for this patch (1/7)

Perry.

> 
> > -#include <asm/cpufeature.h>
> > -#include <asm/cpu_device_id.h>
> >  #include "amd-pstate-trace.h"
> >
> >  #define AMD_PSTATE_TRANSITION_LATENCY        0x20000

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 4/7] cpufreq: amd_pstate: fix wrong lowest perf fetch
  2022-08-31  8:53     ` Yuan, Perry
@ 2022-09-01 14:58       ` Punit Agrawal
  0 siblings, 0 replies; 18+ messages in thread
From: Punit Agrawal @ 2022-09-01 14:58 UTC (permalink / raw)
  To: Yuan, Perry
  Cc: Punit Agrawal, rafael.j.wysocki, Huang, Ray, viresh.kumar,
	Sharma, Deepak, Limonciello, Mario, Fontenot, Nathan, Deucher,
	Alexander, Su, Jinzhou (Joe),
	Huang, Shimmer, Du, Xiaojian, Meng, Li (Jassmine),
	linux-pm, linux-kernel

"Yuan, Perry" <Perry.Yuan@amd.com> writes:

[...]

>> Perry Yuan <Perry.Yuan@amd.com> writes:
>> 
>> > Fix the wrong lowest perf value reading which is used for new des_perf
>> > calculation by governor requested, the incorrect min_perf will get
>> > incorrect des_perf to be set , that will cause the system frequency
>> > changing unexpectedly.
>> >
>> > Reviewed-by: Huang Rui <ray.huang@amd.com>
>> > Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
>> > Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
>> > Signed-off-by: Su Jinzhou <jinzhou.su@amd.com>
>> > ---
>> >  drivers/cpufreq/amd-pstate.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/cpufreq/amd-pstate.c
>> > b/drivers/cpufreq/amd-pstate.c index 5cdef6638681..183cdd4ba00e 100644
>> > --- a/drivers/cpufreq/amd-pstate.c
>> > +++ b/drivers/cpufreq/amd-pstate.c
>> > @@ -307,7 +307,7 @@ static int amd_pstate_target(struct cpufreq_policy
>> *policy,
>> >               return -ENODEV;
>> >
>> >       cap_perf = READ_ONCE(cpudata->highest_perf);
>> > -     min_perf = READ_ONCE(cpudata->lowest_nonlinear_perf);
>> > +     min_perf = READ_ONCE(cpudata->lowest_perf);
>> >       max_perf = cap_perf;
>> >
>> >       freqs.old = policy->cur;
>> 
>> This looks to be a pretty big change (lowest nonlinear vs lowest). Does the patch
>> need to be backported to older kernels?
>
> The patch fixes the min perf initial value, the correct min perf is lowest_perf which is captured through MSR_AMD_CPPC_CAP1 register or the cppc_get_perf_caps().
> Yes, the patch will need to be backported to other kernel branch as issue fix.

Great, thanks for confirming!

[...]


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2022-09-01 14:59 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-14 16:35 [PATCH v5 0/7] AMD Pstate Enhancement And Issue Fixs Perry Yuan
2022-08-14 16:35 ` [PATCH v5 1/7] cpufreq: amd-pstate: cleanup the unused and duplicated headers declaration Perry Yuan
2022-08-15 15:04   ` Punit Agrawal
2022-09-01  5:34     ` Yuan, Perry
2022-08-14 16:35 ` [PATCH v5 2/7] cpufreq: amd-pstate: simplify cpudata pointer assignment Perry Yuan
2022-08-14 16:35 ` [PATCH v5 3/7] cpufreq: amd-pstate: fix white-space Perry Yuan
2022-08-31 19:07   ` Rafael J. Wysocki
2022-08-14 16:35 ` [PATCH v5 4/7] cpufreq: amd_pstate: fix wrong lowest perf fetch Perry Yuan
2022-08-15 15:05   ` Punit Agrawal
2022-08-31  8:53     ` Yuan, Perry
2022-09-01 14:58       ` Punit Agrawal
2022-08-14 16:35 ` [PATCH v5 5/7] cpufreq: amd_pstate: map desired perf into pstate scope for powersave governor Perry Yuan
2022-08-14 16:35 ` [PATCH v5 6/7] cpufreq: amd-pstate: update pstate frequency transition delay time Perry Yuan
2022-08-15 15:05   ` Punit Agrawal
2022-08-16  7:02     ` Yuan, Perry
2022-08-16 16:03       ` Punit Agrawal
2022-08-14 16:35 ` [PATCH v5 7/7] cpufreq: amd-pstate: add ACPI disabled check in acpi_cpc_valid() Perry Yuan
2022-08-25 11:56   ` Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).