[PATCH v4 0/6] amd_pstate: Add guided autonomous mode support

[PATCH v4 0/6] amd_pstate: Add guided autonomous mode support

[Wyes Karny]

From ACPI spec[1] below 3 modes for CPPC can be defined:
1. Non autonomous: OS scaling governor specifies operating frequency/
   performance level through `Desired Performance` register and platform
follows that.
2. Guided autonomous: OS scaling governor specifies min and max
   frequencies/ performance levels through `Minimum Performance` and
`Maximum Performance` register, and platform can autonomously select an
operating frequency in this range.
3. Fully autonomous: OS only hints (via EPP) to platform for the required
   energy performance preference for the workload and platform autonomously
scales the frequency.

Currently (1) is supported by amd_pstate as passive mode, and (3) is
implemented by EPP support[2]. This change is to support (2).

In guided autonomous mode the min_perf is based on the input from the
scaling governor. For example, in case of schedutil this value depends
on the current utilization. And max_perf is set to max capacity.

To activate guided auto mode ``amd_pstate=guided`` command line
parameter has to be passed in the kernel.

Below are the results (normalized) of benchmarks with this patch:
System: Genoa 96C 192T
Kernel: 6.2.0-rc2 + EPP v11 + patch
Scaling governor: schedutil

================ dbench comparisons ================
dbench result comparison:
Here results are throughput (MB/s)
Clients:   acpi-cpufreq		   amd_pst+passive	   amd_pst+guided
    1	   1.00 (0.00 pct)	   1.01 (1.00 pct)	   1.02 (2.00 pct)
    2	   1.07 (0.00 pct)	   1.06 (-0.93 pct)	   1.07 (0.00 pct)
    4	   1.68 (0.00 pct)	   1.70 (1.19 pct)	   1.72 (2.38 pct)
    8	   2.61 (0.00 pct)	   2.68 (2.68 pct)	   2.76 (5.74 pct)
   16	   4.16 (0.00 pct)	   4.24 (1.92 pct)	   4.53 (8.89 pct)
   32	   5.98 (0.00 pct)	   6.17 (3.17 pct)	   7.30 (22.07 pct)
   64	   8.67 (0.00 pct)	   8.99 (3.69 pct)	  10.71 (23.52 pct)
  128	  11.98 (0.00 pct)	  12.52 (4.50 pct)	  14.67 (22.45 pct)
  256	  15.73 (0.00 pct)	  16.13 (2.54 pct)	  17.81 (13.22 pct)
  512	  15.77 (0.00 pct)	  16.32 (3.48 pct)	  16.39 (3.93 pct)
dbench power comparison:
Clients:   acpi-cpufreq		   amd_pst+passive	   amd_pst+guided
    1	   1.00 (0.00 pct)	   1.00 (0.00 pct)	   1.04 (4.00 pct)
    2	   0.99 (0.00 pct)	   0.97 (-2.02 pct)	   1.02 (3.03 pct)
    4	   0.98 (0.00 pct)	   0.98 (0.00 pct)	   1.02 (4.08 pct)
    8	   0.98 (0.00 pct)	   0.99 (1.02 pct)	   1.02 (4.08 pct)
   16	   0.99 (0.00 pct)	   1.00 (1.01 pct)	   1.04 (5.05 pct)
   32	   1.02 (0.00 pct)	   1.02 (0.00 pct)	   1.07 (4.90 pct)
   64	   1.05 (0.00 pct)	   1.05 (0.00 pct)	   1.11 (5.71 pct)
  128	   1.08 (0.00 pct)	   1.08 (0.00 pct)	   1.15 (6.48 pct)
  256	   1.12 (0.00 pct)	   1.12 (0.00 pct)	   1.20 (7.14 pct)
  512	   1.18 (0.00 pct)	   1.17 (-0.84 pct)	   1.26 (6.77 pct)

================ git-source comparisons ================
git-source result comparison:
Here results are throughput (compilations per 1000 sec)
Threads:   acpi-cpufreq		   amd_pst+passive	   amd_pst+guided
  192	   1.00 (0.00 pct)	   0.93 (-7.00 pct)	   1.00 (0.00 pct)
git-source power comparison:
Threads:   acpi-cpufreq		   amd_pst+passive	   amd_pst+guided
  192	   1.00 (0.00 pct)	   1.00 (0.00 pct)	   0.96 (-4.00 pct)

================ kernbench comparisons ================
kernbench result comparison:
Here results are throughput (compilations per 1000 sec)
Load:	   acpi-cpufreq		   amd_pst+passive	   amd_pst+guided
32	   1.00 (0.00 pct)	   1.01 (1.00 pct)	   1.02 (2.00 pct)
48	   1.26 (0.00 pct)	   1.28 (1.58 pct)	   1.25 (-0.79 pct)
64	   1.39 (0.00 pct)	   1.47 (5.75 pct)	   1.43 (2.87 pct)
96	   1.48 (0.00 pct)	   1.50 (1.35 pct)	   1.49 (0.67 pct)
128	   1.29 (0.00 pct)	   1.32 (2.32 pct)	   1.33 (3.10 pct)
192	   1.17 (0.00 pct)	   1.20 (2.56 pct)	   1.21 (3.41 pct)
256	   1.17 (0.00 pct)	   1.18 (0.85 pct)	   1.20 (2.56 pct)
384	   1.16 (0.00 pct)	   1.17 (0.86 pct)	   1.21 (4.31 pct)
kernbench power comparison:
Clients:   acpi-cpufreq		   amd_pst+passive	   amd_pst+guided
   32	   1.00 (0.00 pct)	   0.97 (-3.00 pct)	   1.00 (0.00 pct)
   48	   0.87 (0.00 pct)	   0.81 (-6.89 pct)	   0.88 (1.14 pct)
   64	   0.81 (0.00 pct)	   0.73 (-9.87 pct)	   0.77 (-4.93 pct)
   96	   0.75 (0.00 pct)	   0.74 (-1.33 pct)	   0.75 (0.00 pct)
  128	   0.83 (0.00 pct)	   0.79 (-4.81 pct)	   0.83 (0.00 pct)
  192	   0.92 (0.00 pct)	   0.88 (-4.34 pct)	   0.92 (0.00 pct)
  256	   0.92 (0.00 pct)	   0.88 (-4.34 pct)	   0.92 (0.00 pct)
  384	   0.92 (0.00 pct)	   0.88 (-4.34 pct)	   0.92 (0.00 pct)

================ tbench comparisons ================
tbench result comparison:
Here results are throughput (MB/s)
Clients:   acpi-cpufreq		   amd_pst+passive	   amd_pst+guided
    1	   1.00 (0.00 pct)	   0.70 (-30.00 pct)	   1.37 (37.00 pct)
    2	   2.64 (0.00 pct)	   1.39 (-47.34 pct)	   2.70 (2.27 pct)
    4	   4.89 (0.00 pct)	   2.75 (-43.76 pct)	   5.28 (7.97 pct)
    8	   9.46 (0.00 pct)	   5.42 (-42.70 pct)	  10.22 (8.03 pct)
   16	  19.05 (0.00 pct)	  10.42 (-45.30 pct)	  19.94 (4.67 pct)
   32	  37.50 (0.00 pct)	  20.23 (-46.05 pct)	  36.87 (-1.68 pct)
   64	  61.24 (0.00 pct)	  43.08 (-29.65 pct)	  62.96 (2.80 pct)
  128	  67.16 (0.00 pct)	  69.08 (2.85 pct)	  67.34 (0.26 pct)
  256	 154.59 (0.00 pct)	 162.33 (5.00 pct)	 156.78 (1.41 pct)
  512	 154.02 (0.00 pct)	 156.74 (1.76 pct)	 153.48 (-0.35 pct)
tbench power comparison:
Clients:   acpi-cpufreq		   amd_pst+passive	   amd_pst+guided
    1	   1.00 (0.00 pct)	   0.97 (-3.00 pct)	   1.08 (8.00 pct)
    2	   1.04 (0.00 pct)	   0.97 (-6.73 pct)	   1.11 (6.73 pct)
    4	   1.12 (0.00 pct)	   0.99 (-11.60 pct)	   1.18 (5.35 pct)
    8	   1.25 (0.00 pct)	   1.04 (-16.80 pct)	   1.31 (4.80 pct)
   16	   1.53 (0.00 pct)	   1.13 (-26.14 pct)	   1.58 (3.26 pct)
   32	   2.01 (0.00 pct)	   1.36 (-32.33 pct)	   2.03 (0.99 pct)
   64	   2.58 (0.00 pct)	   2.14 (-17.05 pct)	   2.61 (1.16 pct)
  128	   2.80 (0.00 pct)	   2.81 (0.35 pct)	   2.81 (0.35 pct)
  256	   3.39 (0.00 pct)	   3.43 (1.17 pct)	   3.42 (0.88 pct)
  512	   3.44 (0.00 pct)	   3.44 (0.00 pct)	   3.44 (0.00 pct)

Note: this series is based on top of EPP v11 [3] series

Change log:

v3 -> v4:
- Fixed active mode low frequency issue reported by Peter Jung and Tor Vic
- Documentation modification suggested by Bagas Sanjaya

v2 -> v3:
- Addressed review comments form Mario.
- Picked up RB tag from Mario.
- Rebase on top of EPP v11 [3].

v1 -> v2:
- Fix issue with shared mem systems.
- Rebase on top of EPP series.

[1]: https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf
[2]: https://lore.kernel.org/lkml/20221110175847.3098728-1-Perry.Yuan@amd.com/
[3]: https://lore.kernel.org/linux-pm/20230118075210.447418-1-perry.yuan@amd.com/

Wyes Karny (6):
  acpi: cppc: Add min and max perf reg writing support
  acpi: cppc: Add auto select register read/write support
  cpufreq: amd_pstate: Add guided autonomous mode
  Documentation: amd_pstate: Move amd_pstate param to alphabetical order
  cpufreq: amd_pstate: Add guided mode control support via sysfs
  Documentation: amd_pstate: Update amd_pstate status sysfs for guided

 .../admin-guide/kernel-parameters.txt         |  41 ++--
 Documentation/admin-guide/pm/amd-pstate.rst   |  31 ++-
 drivers/acpi/cppc_acpi.c                      | 121 +++++++++++-
 drivers/cpufreq/amd-pstate.c                  | 184 +++++++++++++-----
 include/acpi/cppc_acpi.h                      |  11 ++
 include/linux/amd-pstate.h                    |   2 +
 6 files changed, 309 insertions(+), 81 deletions(-)

-- 
2.34.1

[PATCH v4 1/6] acpi: cppc: Add min and max perf reg writing support

[Wyes Karny]

Currently writing of min and max perf register is deferred in
cppc_set_perf function. In CPPC guided mode, these registers needed to
be written to guide the platform about min and max perf levels. Add this support
to make guided mode work properly on AMD shared memory systems.

Signed-off-by: Wyes Karny <wyes.karny@amd.com>
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/acpi/cppc_acpi.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 02d83c807271..91f9ef75f7de 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -1487,7 +1487,7 @@ EXPORT_SYMBOL_GPL(cppc_set_enable);
 int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
 {
 	struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpu);
-	struct cpc_register_resource *desired_reg;
+	struct cpc_register_resource *desired_reg, *min_perf_reg, *max_perf_reg;
 	int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpu);
 	struct cppc_pcc_data *pcc_ss_data = NULL;
 	int ret = 0;
@@ -1498,6 +1498,8 @@ int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
 	}
 
 	desired_reg = &cpc_desc->cpc_regs[DESIRED_PERF];
+	min_perf_reg = &cpc_desc->cpc_regs[MIN_PERF];
+	max_perf_reg = &cpc_desc->cpc_regs[MAX_PERF];
 
 	/*
 	 * This is Phase-I where we want to write to CPC registers
@@ -1506,7 +1508,7 @@ int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
 	 * Since read_lock can be acquired by multiple CPUs simultaneously we
 	 * achieve that goal here
 	 */
-	if (CPC_IN_PCC(desired_reg)) {
+	if (CPC_IN_PCC(desired_reg) || CPC_IN_PCC(min_perf_reg) || CPC_IN_PCC(max_perf_reg)) {
 		if (pcc_ss_id < 0) {
 			pr_debug("Invalid pcc_ss_id\n");
 			return -ENODEV;
@@ -1529,13 +1531,19 @@ int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
 		cpc_desc->write_cmd_status = 0;
 	}
 
-	/*
-	 * Skip writing MIN/MAX until Linux knows how to come up with
-	 * useful values.
-	 */
 	cpc_write(cpu, desired_reg, perf_ctrls->desired_perf);
 
-	if (CPC_IN_PCC(desired_reg))
+	/**
+	 * Only write if min_perf and max_perf not zero. Some drivers pass zero
+	 * value to min and max perf, but they don't mean to set the zero value,
+	 * they just don't want to write to those registers.
+	 */
+	if (perf_ctrls->min_perf)
+		cpc_write(cpu, min_perf_reg, perf_ctrls->min_perf);
+	if (perf_ctrls->max_perf)
+		cpc_write(cpu, max_perf_reg, perf_ctrls->max_perf);
+
+	if (CPC_IN_PCC(desired_reg) || CPC_IN_PCC(min_perf_reg) || CPC_IN_PCC(max_perf_reg))
 		up_read(&pcc_ss_data->pcc_lock);	/* END Phase-I */
 	/*
 	 * This is Phase-II where we transfer the ownership of PCC to Platform
@@ -1583,7 +1591,7 @@ int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
 	 * case during a CMD_READ and if there are pending writes it delivers
 	 * the write command before servicing the read command
 	 */
-	if (CPC_IN_PCC(desired_reg)) {
+	if (CPC_IN_PCC(desired_reg) || CPC_IN_PCC(min_perf_reg) || CPC_IN_PCC(max_perf_reg)) {
 		if (down_write_trylock(&pcc_ss_data->pcc_lock)) {/* BEGIN Phase-II */
 			/* Update only if there are pending write commands */
 			if (pcc_ss_data->pending_pcc_write_cmd)
-- 
2.34.1

[PATCH v4 2/6] acpi: cppc: Add auto select register read/write support

[Wyes Karny]

For some AMD shared memory based systems, the autonomous selection bit
needed to be set explicitly. Add autonomous selection register related
APIs to acpi driver, which amd_pstate driver uses later.

Signed-off-by: Wyes Karny <wyes.karny@amd.com>
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/acpi/cppc_acpi.c | 97 ++++++++++++++++++++++++++++++++++++++++
 include/acpi/cppc_acpi.h | 11 +++++
 2 files changed, 108 insertions(+)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 91f9ef75f7de..1806006a51af 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -1432,6 +1432,103 @@ int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable)
 }
 EXPORT_SYMBOL_GPL(cppc_set_epp_perf);
 
+/*
+ * cppc_get_auto_sel_caps - Read autonomous selection register.
+ * @cpunum : CPU from which to read register.
+ * @perf_caps : struct where autonomous selection register value is updated.
+ */
+int cppc_get_auto_sel_caps(int cpunum, struct cppc_perf_caps *perf_caps)
+{
+	struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpunum);
+	struct cpc_register_resource *auto_sel_reg;
+	u64  auto_sel;
+
+	if (!cpc_desc) {
+		pr_debug("No CPC descriptor for CPU:%d\n", cpunum);
+		return -ENODEV;
+	}
+
+	auto_sel_reg = &cpc_desc->cpc_regs[AUTO_SEL_ENABLE];
+
+	if (!CPC_SUPPORTED(auto_sel_reg))
+		pr_warn_once("Autonomous mode is not unsupported!\n");
+
+	if (CPC_IN_PCC(auto_sel_reg)) {
+		int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpunum);
+		struct cppc_pcc_data *pcc_ss_data = NULL;
+		int ret = 0;
+
+		if (pcc_ss_id < 0)
+			return -ENODEV;
+
+		pcc_ss_data = pcc_data[pcc_ss_id];
+
+		down_write(&pcc_ss_data->pcc_lock);
+
+		if (send_pcc_cmd(pcc_ss_id, CMD_READ) >= 0) {
+			cpc_read(cpunum, auto_sel_reg, &auto_sel);
+			perf_caps->auto_sel = (bool)auto_sel;
+		} else {
+			ret = -EIO;
+		}
+
+		up_write(&pcc_ss_data->pcc_lock);
+
+		return ret;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(cppc_get_auto_sel_caps);
+
+/*
+ * cppc_set_auto_sel - Write autonomous selection register.
+ * @cpunum : CPU to which to write register.
+ * @enable : the desired value of autonomous selection resiter to be updated.
+ */
+int cppc_set_auto_sel(int cpu, bool enable)
+{
+	int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpu);
+	struct cpc_register_resource *auto_sel_reg;
+	struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpu);
+	struct cppc_pcc_data *pcc_ss_data = NULL;
+	int ret = -EINVAL;
+
+	if (!cpc_desc) {
+		pr_debug("No CPC descriptor for CPU:%d\n", cpu);
+		return -ENODEV;
+	}
+
+	auto_sel_reg = &cpc_desc->cpc_regs[AUTO_SEL_ENABLE];
+
+	if (CPC_IN_PCC(auto_sel_reg)) {
+		if (pcc_ss_id < 0) {
+			pr_debug("Invalid pcc_ss_id\n");
+			return -ENODEV;
+		}
+
+		if (CPC_SUPPORTED(auto_sel_reg)) {
+			ret = cpc_write(cpu, auto_sel_reg, enable);
+			if (ret)
+				return ret;
+		}
+
+		pcc_ss_data = pcc_data[pcc_ss_id];
+
+		down_write(&pcc_ss_data->pcc_lock);
+		/* after writing CPC, transfer the ownership of PCC to platform */
+		ret = send_pcc_cmd(pcc_ss_id, CMD_WRITE);
+		up_write(&pcc_ss_data->pcc_lock);
+	} else {
+		ret = -ENOTSUPP;
+		pr_debug("_CPC in PCC is not supported\n");
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(cppc_set_auto_sel);
+
+
 /**
  * cppc_set_enable - Set to enable CPPC on the processor by writing the
  * Continuous Performance Control package EnableRegister field.
diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
index 6b487a5bd638..6126c977ece0 100644
--- a/include/acpi/cppc_acpi.h
+++ b/include/acpi/cppc_acpi.h
@@ -109,6 +109,7 @@ struct cppc_perf_caps {
 	u32 lowest_freq;
 	u32 nominal_freq;
 	u32 energy_perf;
+	bool auto_sel;
 };
 
 struct cppc_perf_ctrls {
@@ -153,6 +154,8 @@ extern int cpc_read_ffh(int cpunum, struct cpc_reg *reg, u64 *val);
 extern int cpc_write_ffh(int cpunum, struct cpc_reg *reg, u64 val);
 extern int cppc_get_epp_perf(int cpunum, u64 *epp_perf);
 extern int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable);
+extern int cppc_get_auto_sel_caps(int cpunum, struct cppc_perf_caps *perf_caps);
+extern int cppc_set_auto_sel(int cpu, bool enable);
 #else /* !CONFIG_ACPI_CPPC_LIB */
 static inline int cppc_get_desired_perf(int cpunum, u64 *desired_perf)
 {
@@ -214,6 +217,14 @@ static inline int cppc_get_epp_perf(int cpunum, u64 *epp_perf)
 {
 	return -ENOTSUPP;
 }
+static inline int cppc_set_auto_sel(int cpu, bool enable)
+{
+	return -ENOTSUPP;
+}
+static inline int cppc_get_auto_sel_caps(int cpunum, struct cppc_perf_caps *perf_caps)
+{
+	return -ENOTSUPP;
+}
 #endif /* !CONFIG_ACPI_CPPC_LIB */
 
 #endif /* _CPPC_ACPI_H*/
-- 
2.34.1

[PATCH v4 3/6] cpufreq: amd_pstate: Add guided autonomous mode

[Wyes Karny]

From ACPI spec below 3 modes for CPPC can be defined:
1. Non autonomous: OS scaling governor specifies operating frequency/
   performance level through `Desired Performance` register and platform
follows that.
2. Guided autonomous: OS scaling governor specifies min and max
   frequencies/ performance levels through `Minimum Performance` and
`Maximum Performance` register, and platform can autonomously select an
operating frequency in this range.
3. Fully autonomous: OS only hints (via EPP) to platform for the required
   energy performance preference for the workload and platform autonomously
scales the frequency.

Currently (1) is supported by amd_pstate as passive mode, and (3) is
implemented by EPP support. This change is to support (2).

In guided autonomous mode the min_perf is based on the input from the
scaling governor. For example, in case of schedutil this value depends
on the current utilization. And max_perf is set to max capacity.

To activate guided auto mode ``amd_pstate=guided`` command line
parameter has to be passed in the kernel.

Signed-off-by: Wyes Karny <wyes.karny@amd.com>
---
 .../admin-guide/kernel-parameters.txt         | 15 +++++---
 drivers/cpufreq/amd-pstate.c                  | 34 +++++++++++++++----
 include/linux/amd-pstate.h                    |  2 ++
 3 files changed, 40 insertions(+), 11 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index e3618dfdb36a..0d8486325c9a 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -7015,11 +7015,11 @@
 			  Do not enable amd_pstate as the default
 			  scaling driver for the supported processors
 			passive
-			  Use amd_pstate as a scaling driver, driver requests a
-			  desired performance on this abstract scale and the power
-			  management firmware translates the requests into actual
-			  hardware states (core frequency, data fabric and memory
-			  clocks etc.)
+			  Use amd_pstate with passive mode as a scaling driver.
+			  In this mode autonomous selection is disabled.
+			  Driver requests a desired performance level and platform
+			  tires to match the same performance level (if it is
+			  satisfied by guaranteed performance level).
 			active
 			  Use amd_pstate_epp driver instance as the scaling driver,
 			  driver provides a hint to the hardware if software wants
@@ -7027,3 +7027,8 @@
 			  to the CPPC firmware. then CPPC power algorithm will
 			  calculate the runtime workload and adjust the realtime cores
 			  frequency.
+			guided
+			  Activate guided autonomous mode. Driver requests minimum and
+			  maximum performance level and the platform autonomously
+			  selects a performance level in this range and appropriate
+			  to the current workload.
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index c5731fefcbef..48ab4684c3a5 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -267,7 +267,22 @@ static int cppc_init_perf(struct amd_cpudata *cpudata)
 		   cppc_perf.lowest_nonlinear_perf);
 	WRITE_ONCE(cpudata->lowest_perf, cppc_perf.lowest_perf);
 
-	return 0;
+	if (cppc_state == AMD_PSTATE_ACTIVE)
+		return 0;
+
+	ret = cppc_get_auto_sel_caps(cpudata->cpu, &cppc_perf);
+	if (ret) {
+		pr_warn("failed to get auto_sel, ret: %d\n", ret);
+		return 0;
+	}
+
+	ret = cppc_set_auto_sel(cpudata->cpu,
+			(cppc_state == AMD_PSTATE_PASSIVE) ? 0 : 1);
+
+	if (ret)
+		pr_warn("failed to set auto_sel, ret: %d\n", ret);
+
+	return ret;
 }
 
 DEFINE_STATIC_CALL(amd_pstate_init_perf, pstate_init_perf);
@@ -344,12 +359,18 @@ static inline bool amd_pstate_sample(struct amd_cpudata *cpudata)
 }
 
 static void amd_pstate_update(struct amd_cpudata *cpudata, u32 min_perf,
-			      u32 des_perf, u32 max_perf, bool fast_switch)
+			      u32 des_perf, u32 max_perf, bool fast_switch, int guv_flags)
 {
 	u64 prev = READ_ONCE(cpudata->cppc_req_cached);
 	u64 value = prev;
 
 	des_perf = clamp_t(unsigned long, des_perf, min_perf, max_perf);
+
+	if ((cppc_state == AMD_PSTATE_GUIDED) && (guv_flags & CPUFREQ_GOV_DYNAMIC_SWITCHING)) {
+		min_perf = des_perf;
+		des_perf = 0;
+	}
+
 	value &= ~AMD_CPPC_MIN_PERF(~0L);
 	value |= AMD_CPPC_MIN_PERF(min_perf);
 
@@ -404,7 +425,7 @@ static int amd_pstate_target(struct cpufreq_policy *policy,
 
 	cpufreq_freq_transition_begin(policy, &freqs);
 	amd_pstate_update(cpudata, min_perf, des_perf,
-			  max_perf, false);
+			  max_perf, false, policy->governor->flags);
 	cpufreq_freq_transition_end(policy, &freqs, false);
 
 	return 0;
@@ -438,7 +459,8 @@ static void amd_pstate_adjust_perf(unsigned int cpu,
 	if (max_perf < min_perf)
 		max_perf = min_perf;
 
-	amd_pstate_update(cpudata, min_perf, des_perf, max_perf, true);
+	amd_pstate_update(cpudata, min_perf, des_perf, max_perf, true,
+			policy->governor->flags);
 	cpufreq_cpu_put(policy);
 }
 
@@ -1238,7 +1260,7 @@ static int __init amd_pstate_init(void)
 	/* capability check */
 	if (boot_cpu_has(X86_FEATURE_CPPC)) {
 		pr_debug("AMD CPPC MSR based functionality is supported\n");
-		if (cppc_state == AMD_PSTATE_PASSIVE)
+		if (cppc_state != AMD_PSTATE_ACTIVE)
 			current_pstate_driver->adjust_perf = amd_pstate_adjust_perf;
 	} else {
 		pr_debug("AMD CPPC shared memory based functionality is supported\n");
@@ -1300,7 +1322,7 @@ static int __init amd_pstate_param(char *str)
 		if (cppc_state == AMD_PSTATE_ACTIVE)
 			current_pstate_driver = &amd_pstate_epp_driver;
 
-		if (cppc_state == AMD_PSTATE_PASSIVE)
+		if (cppc_state == AMD_PSTATE_PASSIVE || cppc_state == AMD_PSTATE_GUIDED)
 			current_pstate_driver = &amd_pstate_driver;
 
 		return 0;
diff --git a/include/linux/amd-pstate.h b/include/linux/amd-pstate.h
index e9e291b7c2f9..85e980b8e0ac 100644
--- a/include/linux/amd-pstate.h
+++ b/include/linux/amd-pstate.h
@@ -92,6 +92,7 @@ enum amd_pstate_mode {
 	AMD_PSTATE_DISABLE = 0,
 	AMD_PSTATE_PASSIVE,
 	AMD_PSTATE_ACTIVE,
+	AMD_PSTATE_GUIDED,
 	AMD_PSTATE_MAX,
 };
 
@@ -99,6 +100,7 @@ static const char * const amd_pstate_mode_string[] = {
 	[AMD_PSTATE_DISABLE]     = "disable",
 	[AMD_PSTATE_PASSIVE]     = "passive",
 	[AMD_PSTATE_ACTIVE]      = "active",
+	[AMD_PSTATE_GUIDED]      = "guided",
 	NULL,
 };
 
-- 
2.34.1

[PATCH v4 4/6] Documentation: amd_pstate: Move amd_pstate param to alphabetical order

[Wyes Karny]

Move amd_pstate command line param description to correct alphabetical
order.

Signed-off-by: Wyes Karny <wyes.karny@amd.com>
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
---
 .../admin-guide/kernel-parameters.txt         | 46 +++++++++----------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 0d8486325c9a..5f6a3a34251e 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -339,6 +339,29 @@
 			             This mode requires kvm-amd.avic=1.
 			             (Default when IOMMU HW support is present.)
 
+	amd_pstate=	[X86]
+			disable
+			  Do not enable amd_pstate as the default
+			  scaling driver for the supported processors
+			passive
+			  Use amd_pstate with passive mode as a scaling driver.
+			  In this mode autonomous selection is disabled.
+			  Driver requests a desired performance level and platform
+			  tires to match the same performance level (if it is
+			  satisfied by guaranteed performance level).
+			active
+			  Use amd_pstate_epp driver instance as the scaling driver,
+			  driver provides a hint to the hardware if software wants
+			  to bias toward performance (0x0) or energy efficiency (0xff)
+			  to the CPPC firmware. then CPPC power algorithm will
+			  calculate the runtime workload and adjust the realtime cores
+			  frequency.
+			guided
+			  Activate guided autonomous mode. Driver requests minimum and
+			  maximum performance level and the platform autonomously
+			  selects a performance level in this range and appropriate
+			  to the current workload.
+
 	amijoy.map=	[HW,JOY] Amiga joystick support
 			Map of devices attached to JOY0DAT and JOY1DAT
 			Format: <a>,<b>
@@ -7009,26 +7032,3 @@
 				memory, and other data can't be written using
 				xmon commands.
 			off	xmon is disabled.
-
-	amd_pstate=	[X86]
-			disable
-			  Do not enable amd_pstate as the default
-			  scaling driver for the supported processors
-			passive
-			  Use amd_pstate with passive mode as a scaling driver.
-			  In this mode autonomous selection is disabled.
-			  Driver requests a desired performance level and platform
-			  tires to match the same performance level (if it is
-			  satisfied by guaranteed performance level).
-			active
-			  Use amd_pstate_epp driver instance as the scaling driver,
-			  driver provides a hint to the hardware if software wants
-			  to bias toward performance (0x0) or energy efficiency (0xff)
-			  to the CPPC firmware. then CPPC power algorithm will
-			  calculate the runtime workload and adjust the realtime cores
-			  frequency.
-			guided
-			  Activate guided autonomous mode. Driver requests minimum and
-			  maximum performance level and the platform autonomously
-			  selects a performance level in this range and appropriate
-			  to the current workload.
-- 
2.34.1

[PATCH v4 5/6] cpufreq: amd_pstate: Add guided mode control support via sysfs

[Wyes Karny]

amd_pstate driver's `status` sysfs entry helps to control the driver's
mode dynamically by user. After the addition of guided mode the
combinations of mode transitions have been increased (16 combinations).
Therefore optimise the amd_pstate_update_status function by implementing
a state transition table.

There are 4 states amd_pstate supports, namely: 'disable', 'passive',
'active', and 'guided'.  The transition from any state to any other
state is possible after this change. Only if the state requested matches
with the current state then -EBUSY value is returned.

Sysfs interface:

To disable amd_pstate driver:
 # echo disable > /sys/devices/system/cpu/amd_pstate/status

To enable passive mode:
 # echo passive > /sys/devices/system/cpu/amd_pstate/status

To change mode to active:
 # echo active > /sys/devices/system/cpu/amd_pstate/status

To change mode to guided:
 # echo guided > /sys/devices/system/cpu/amd_pstate/status

Signed-off-by: Wyes Karny <wyes.karny@amd.com>
---
 drivers/cpufreq/amd-pstate.c | 150 +++++++++++++++++++++++++----------
 1 file changed, 108 insertions(+), 42 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 48ab4684c3a5..6c522dec6967 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -65,6 +65,8 @@ static struct cpufreq_driver amd_pstate_epp_driver;
 static int cppc_state = AMD_PSTATE_DISABLE;
 struct kobject *amd_pstate_kobj;
 
+typedef int (*cppc_mode_transition_fn)(int);
+
 static inline int get_mode_idx_from_str(const char *str, size_t size)
 {
 	int i;
@@ -797,6 +799,105 @@ static ssize_t show_energy_performance_preference(
 	return sysfs_emit(buf, "%s\n", energy_perf_strings[preference]);
 }
 
+static void amd_pstate_driver_cleanup(void)
+{
+	amd_pstate_enable(false);
+	cppc_state = AMD_PSTATE_DISABLE;
+	current_pstate_driver = NULL;
+}
+
+static int amd_pstate_register_driver(int mode)
+{
+	int ret;
+
+	if (mode == AMD_PSTATE_PASSIVE || mode == AMD_PSTATE_GUIDED)
+		current_pstate_driver = &amd_pstate_driver;
+	else if (mode == AMD_PSTATE_ACTIVE)
+		current_pstate_driver = &amd_pstate_epp_driver;
+	else
+		return -EINVAL;
+
+	cppc_state = mode;
+	ret = cpufreq_register_driver(current_pstate_driver);
+	if (ret) {
+		amd_pstate_driver_cleanup();
+		return ret;
+	}
+	return 0;
+}
+
+static int amd_pstate_unregister_driver(int dummy)
+{
+	int ret;
+
+	ret = cpufreq_unregister_driver(current_pstate_driver);
+
+	if (ret)
+		return ret;
+
+	amd_pstate_driver_cleanup();
+	return 0;
+}
+
+static int amd_pstate_change_mode_without_dvr_change(int mode)
+{
+	int cpu = 0;
+
+	cppc_state = mode;
+
+	if (boot_cpu_has(X86_FEATURE_CPPC) || cppc_state == AMD_PSTATE_ACTIVE)
+		return 0;
+
+	for_each_present_cpu(cpu) {
+		cppc_set_auto_sel(cpu, (cppc_state == AMD_PSTATE_PASSIVE) ? 0 : 1);
+	}
+
+	return 0;
+}
+
+static int amd_pstate_change_driver_mode(int mode)
+{
+	int ret;
+
+	ret = amd_pstate_unregister_driver(0);
+	if (ret)
+		return ret;
+
+	ret = amd_pstate_register_driver(mode);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+/* Mode transition table */
+cppc_mode_transition_fn mode_state_machine[AMD_PSTATE_MAX][AMD_PSTATE_MAX] = {
+	[AMD_PSTATE_DISABLE]         = {
+		[AMD_PSTATE_DISABLE]     = NULL,
+		[AMD_PSTATE_PASSIVE]     = amd_pstate_register_driver,
+		[AMD_PSTATE_ACTIVE]      = amd_pstate_register_driver,
+		[AMD_PSTATE_GUIDED]      = amd_pstate_register_driver,
+	},
+	[AMD_PSTATE_PASSIVE]         = {
+		[AMD_PSTATE_DISABLE]     = amd_pstate_unregister_driver,
+		[AMD_PSTATE_PASSIVE]     = NULL,
+		[AMD_PSTATE_ACTIVE]      = amd_pstate_change_driver_mode,
+		[AMD_PSTATE_GUIDED]      = amd_pstate_change_mode_without_dvr_change,
+	},
+	[AMD_PSTATE_ACTIVE]          = {
+		[AMD_PSTATE_DISABLE]     = amd_pstate_unregister_driver,
+		[AMD_PSTATE_PASSIVE]     = amd_pstate_change_driver_mode,
+		[AMD_PSTATE_ACTIVE]      = NULL,
+		[AMD_PSTATE_GUIDED]      = amd_pstate_change_driver_mode,
+	},
+	[AMD_PSTATE_GUIDED]          = {
+		[AMD_PSTATE_DISABLE]     = amd_pstate_unregister_driver,
+		[AMD_PSTATE_PASSIVE]     = amd_pstate_change_mode_without_dvr_change,
+		[AMD_PSTATE_ACTIVE]      = amd_pstate_change_driver_mode,
+		[AMD_PSTATE_GUIDED]      = NULL,
+	},
+};
+
 static ssize_t amd_pstate_show_status(char *buf)
 {
 	if (!current_pstate_driver)
@@ -805,57 +906,22 @@ static ssize_t amd_pstate_show_status(char *buf)
 	return sysfs_emit(buf, "%s\n", amd_pstate_mode_string[cppc_state]);
 }
 
-static void amd_pstate_driver_cleanup(void)
-{
-	current_pstate_driver = NULL;
-}
-
 static int amd_pstate_update_status(const char *buf, size_t size)
 {
-	int ret;
 	int mode_idx;
 
-	if (size > 7 || size < 6)
+	if (size > strlen("passive") || size < strlen("active"))
 		return -EINVAL;
-	mode_idx = get_mode_idx_from_str(buf, size);
 
-	switch(mode_idx) {
-	case AMD_PSTATE_DISABLE:
-		if (!current_pstate_driver)
-			return -EINVAL;
-		if (cppc_state == AMD_PSTATE_ACTIVE)
-			return -EBUSY;
-		ret = cpufreq_unregister_driver(current_pstate_driver);
-		amd_pstate_driver_cleanup();
-		break;
-	case AMD_PSTATE_PASSIVE:
-		if (current_pstate_driver) {
-			if (current_pstate_driver == &amd_pstate_driver)
-				return 0;
-			cpufreq_unregister_driver(current_pstate_driver);
-			cppc_state = AMD_PSTATE_PASSIVE;
-			current_pstate_driver = &amd_pstate_driver;
-		}
+	mode_idx = get_mode_idx_from_str(buf, size);
 
-		ret = cpufreq_register_driver(current_pstate_driver);
-		break;
-	case AMD_PSTATE_ACTIVE:
-		if (current_pstate_driver) {
-			if (current_pstate_driver == &amd_pstate_epp_driver)
-				return 0;
-			cpufreq_unregister_driver(current_pstate_driver);
-			current_pstate_driver = &amd_pstate_epp_driver;
-			cppc_state = AMD_PSTATE_ACTIVE;
-		}
+	if (mode_idx < 0 || mode_idx >= AMD_PSTATE_MAX)
+		return -EINVAL;
 
-		ret = cpufreq_register_driver(current_pstate_driver);
-		break;
-	default:
-		ret = -EINVAL;
-		break;
-	}
+	if (mode_state_machine[cppc_state][mode_idx])
+		return mode_state_machine[cppc_state][mode_idx](mode_idx);
 
-	return ret;
+	return -EBUSY;
 }
 
 static ssize_t show_status(struct kobject *kobj,
-- 
2.34.1

[PATCH v4 6/6] Documentation: amd_pstate: Update amd_pstate status sysfs for guided

[Wyes Karny]

Update amd_pstate status sysfs for guided mode.

Signed-off-by: Wyes Karny <wyes.karny@amd.com>
---
 Documentation/admin-guide/pm/amd-pstate.rst | 31 ++++++++++++++++-----
 1 file changed, 24 insertions(+), 7 deletions(-)

diff --git a/Documentation/admin-guide/pm/amd-pstate.rst b/Documentation/admin-guide/pm/amd-pstate.rst
index 5304adf2fc2f..56a3d11f51a5 100644
--- a/Documentation/admin-guide/pm/amd-pstate.rst
+++ b/Documentation/admin-guide/pm/amd-pstate.rst
@@ -303,13 +303,18 @@ efficiency frequency management method on AMD processors.
 AMD Pstate Driver Operation Modes
 =================================
 
-``amd_pstate`` CPPC has two operation modes: CPPC Autonomous(active) mode and
-CPPC non-autonomous(passive) mode.
-active mode and passive mode can be chosen by different kernel parameters.
-When in Autonomous mode, CPPC ignores requests done in the Desired Performance
-Target register and takes into account only the values set to the Minimum requested
-performance, Maximum requested performance, and Energy Performance Preference
-registers. When Autonomous is disabled, it only considers the Desired Performance Target.
+``amd_pstate`` CPPC has 3 operation modes: autonomous (active) mode,
+non-autonomous (passive) mode and guided autonomous (guided) mode.
+Active/passive/guided mode can be chosen by different kernel parameters.
+
+- In autonomous mode, platform ignores the desired performance level request
+  and takes into account only the values set to the minimum, maximum and energy
+  performance preference registers.
+- In non-autonomous mode, platform gets desired performance level
+  from OS directly through Desired Performance Register.
+- In guided-autonomous mode, platform sets operating performance level
+  autonomously according to the current workload and within the limits set by
+  OS through min and max performance registers.
 
 Active Mode
 ------------
@@ -338,6 +343,15 @@ to the Performance Reduction Tolerance register. Above the nominal performance l
 processor must provide at least nominal performance requested and go higher if current
 operating conditions allow.
 
+Guided Mode
+-----------
+
+``amd_pstate=guided``
+
+If ``amd_pstate=guided`` is passed to kernel command line option then this mode
+is activated.  In this mode, driver requests minimum and maximum performance
+level and the platform autonomously selects a performance level in this range
+and appropriate to the current workload.
 
 User Space Interface in ``sysfs``
 =================================
@@ -358,6 +372,9 @@ control its functionality at the system level.  They are located in the
 	"passive"
 		The driver is functional and in the ``passive mode``
 
+        "guided"
+                The driver is functional and in the ``guided mode``
+
 	"disable"
 		The driver is unregistered and not functional now.
 
-- 
2.34.1

Re: [PATCH v4 0/6] amd_pstate: Add guided autonomous mode support

[torvic9]

> Wyes Karny <wyes.karny@amd.com> hat am 31.01.2023 05:21 GMT geschrieben:
> 
>  
> From ACPI spec[1] below 3 modes for CPPC can be defined:
> 1. Non autonomous: OS scaling governor specifies operating frequency/
>    performance level through `Desired Performance` register and platform
> follows that.
> 2. Guided autonomous: OS scaling governor specifies min and max
>    frequencies/ performance levels through `Minimum Performance` and
> `Maximum Performance` register, and platform can autonomously select an
> operating frequency in this range.
> 3. Fully autonomous: OS only hints (via EPP) to platform for the required
>    energy performance preference for the workload and platform autonomously
> scales the frequency.
> 
> Currently (1) is supported by amd_pstate as passive mode, and (3) is
> implemented by EPP support[2]. This change is to support (2).
> 
> In guided autonomous mode the min_perf is based on the input from the
> scaling governor. For example, in case of schedutil this value depends
> on the current utilization. And max_perf is set to max capacity.
> 
> To activate guided auto mode ``amd_pstate=guided`` command line
> parameter has to be passed in the kernel.
> 
> Below are the results (normalized) of benchmarks with this patch:
> System: Genoa 96C 192T
> Kernel: 6.2.0-rc2 + EPP v11 + patch
> Scaling governor: schedutil
> 
> ================ dbench comparisons ================
> dbench result comparison:
> Here results are throughput (MB/s)
> Clients:   acpi-cpufreq		   amd_pst+passive	   amd_pst+guided
>     1	   1.00 (0.00 pct)	   1.01 (1.00 pct)	   1.02 (2.00 pct)
>     2	   1.07 (0.00 pct)	   1.06 (-0.93 pct)	   1.07 (0.00 pct)
>     4	   1.68 (0.00 pct)	   1.70 (1.19 pct)	   1.72 (2.38 pct)
>     8	   2.61 (0.00 pct)	   2.68 (2.68 pct)	   2.76 (5.74 pct)
>    16	   4.16 (0.00 pct)	   4.24 (1.92 pct)	   4.53 (8.89 pct)
>    32	   5.98 (0.00 pct)	   6.17 (3.17 pct)	   7.30 (22.07 pct)
>    64	   8.67 (0.00 pct)	   8.99 (3.69 pct)	  10.71 (23.52 pct)
>   128	  11.98 (0.00 pct)	  12.52 (4.50 pct)	  14.67 (22.45 pct)
>   256	  15.73 (0.00 pct)	  16.13 (2.54 pct)	  17.81 (13.22 pct)
>   512	  15.77 (0.00 pct)	  16.32 (3.48 pct)	  16.39 (3.93 pct)
> dbench power comparison:
> Clients:   acpi-cpufreq		   amd_pst+passive	   amd_pst+guided
>     1	   1.00 (0.00 pct)	   1.00 (0.00 pct)	   1.04 (4.00 pct)
>     2	   0.99 (0.00 pct)	   0.97 (-2.02 pct)	   1.02 (3.03 pct)
>     4	   0.98 (0.00 pct)	   0.98 (0.00 pct)	   1.02 (4.08 pct)
>     8	   0.98 (0.00 pct)	   0.99 (1.02 pct)	   1.02 (4.08 pct)
>    16	   0.99 (0.00 pct)	   1.00 (1.01 pct)	   1.04 (5.05 pct)
>    32	   1.02 (0.00 pct)	   1.02 (0.00 pct)	   1.07 (4.90 pct)
>    64	   1.05 (0.00 pct)	   1.05 (0.00 pct)	   1.11 (5.71 pct)
>   128	   1.08 (0.00 pct)	   1.08 (0.00 pct)	   1.15 (6.48 pct)
>   256	   1.12 (0.00 pct)	   1.12 (0.00 pct)	   1.20 (7.14 pct)
>   512	   1.18 (0.00 pct)	   1.17 (-0.84 pct)	   1.26 (6.77 pct)
> 
> ================ git-source comparisons ================
> git-source result comparison:
> Here results are throughput (compilations per 1000 sec)
> Threads:   acpi-cpufreq		   amd_pst+passive	   amd_pst+guided
>   192	   1.00 (0.00 pct)	   0.93 (-7.00 pct)	   1.00 (0.00 pct)
> git-source power comparison:
> Threads:   acpi-cpufreq		   amd_pst+passive	   amd_pst+guided
>   192	   1.00 (0.00 pct)	   1.00 (0.00 pct)	   0.96 (-4.00 pct)
> 
> ================ kernbench comparisons ================
> kernbench result comparison:
> Here results are throughput (compilations per 1000 sec)
> Load:	   acpi-cpufreq		   amd_pst+passive	   amd_pst+guided
> 32	   1.00 (0.00 pct)	   1.01 (1.00 pct)	   1.02 (2.00 pct)
> 48	   1.26 (0.00 pct)	   1.28 (1.58 pct)	   1.25 (-0.79 pct)
> 64	   1.39 (0.00 pct)	   1.47 (5.75 pct)	   1.43 (2.87 pct)
> 96	   1.48 (0.00 pct)	   1.50 (1.35 pct)	   1.49 (0.67 pct)
> 128	   1.29 (0.00 pct)	   1.32 (2.32 pct)	   1.33 (3.10 pct)
> 192	   1.17 (0.00 pct)	   1.20 (2.56 pct)	   1.21 (3.41 pct)
> 256	   1.17 (0.00 pct)	   1.18 (0.85 pct)	   1.20 (2.56 pct)
> 384	   1.16 (0.00 pct)	   1.17 (0.86 pct)	   1.21 (4.31 pct)
> kernbench power comparison:
> Clients:   acpi-cpufreq		   amd_pst+passive	   amd_pst+guided
>    32	   1.00 (0.00 pct)	   0.97 (-3.00 pct)	   1.00 (0.00 pct)
>    48	   0.87 (0.00 pct)	   0.81 (-6.89 pct)	   0.88 (1.14 pct)
>    64	   0.81 (0.00 pct)	   0.73 (-9.87 pct)	   0.77 (-4.93 pct)
>    96	   0.75 (0.00 pct)	   0.74 (-1.33 pct)	   0.75 (0.00 pct)
>   128	   0.83 (0.00 pct)	   0.79 (-4.81 pct)	   0.83 (0.00 pct)
>   192	   0.92 (0.00 pct)	   0.88 (-4.34 pct)	   0.92 (0.00 pct)
>   256	   0.92 (0.00 pct)	   0.88 (-4.34 pct)	   0.92 (0.00 pct)
>   384	   0.92 (0.00 pct)	   0.88 (-4.34 pct)	   0.92 (0.00 pct)
> 
> ================ tbench comparisons ================
> tbench result comparison:
> Here results are throughput (MB/s)
> Clients:   acpi-cpufreq		   amd_pst+passive	   amd_pst+guided
>     1	   1.00 (0.00 pct)	   0.70 (-30.00 pct)	   1.37 (37.00 pct)
>     2	   2.64 (0.00 pct)	   1.39 (-47.34 pct)	   2.70 (2.27 pct)
>     4	   4.89 (0.00 pct)	   2.75 (-43.76 pct)	   5.28 (7.97 pct)
>     8	   9.46 (0.00 pct)	   5.42 (-42.70 pct)	  10.22 (8.03 pct)
>    16	  19.05 (0.00 pct)	  10.42 (-45.30 pct)	  19.94 (4.67 pct)
>    32	  37.50 (0.00 pct)	  20.23 (-46.05 pct)	  36.87 (-1.68 pct)
>    64	  61.24 (0.00 pct)	  43.08 (-29.65 pct)	  62.96 (2.80 pct)
>   128	  67.16 (0.00 pct)	  69.08 (2.85 pct)	  67.34 (0.26 pct)
>   256	 154.59 (0.00 pct)	 162.33 (5.00 pct)	 156.78 (1.41 pct)
>   512	 154.02 (0.00 pct)	 156.74 (1.76 pct)	 153.48 (-0.35 pct)
> tbench power comparison:
> Clients:   acpi-cpufreq		   amd_pst+passive	   amd_pst+guided
>     1	   1.00 (0.00 pct)	   0.97 (-3.00 pct)	   1.08 (8.00 pct)
>     2	   1.04 (0.00 pct)	   0.97 (-6.73 pct)	   1.11 (6.73 pct)
>     4	   1.12 (0.00 pct)	   0.99 (-11.60 pct)	   1.18 (5.35 pct)
>     8	   1.25 (0.00 pct)	   1.04 (-16.80 pct)	   1.31 (4.80 pct)
>    16	   1.53 (0.00 pct)	   1.13 (-26.14 pct)	   1.58 (3.26 pct)
>    32	   2.01 (0.00 pct)	   1.36 (-32.33 pct)	   2.03 (0.99 pct)
>    64	   2.58 (0.00 pct)	   2.14 (-17.05 pct)	   2.61 (1.16 pct)
>   128	   2.80 (0.00 pct)	   2.81 (0.35 pct)	   2.81 (0.35 pct)
>   256	   3.39 (0.00 pct)	   3.43 (1.17 pct)	   3.42 (0.88 pct)
>   512	   3.44 (0.00 pct)	   3.44 (0.00 pct)	   3.44 (0.00 pct)
> 
> Note: this series is based on top of EPP v11 [3] series

Hi Wyes,

Can you rebase this patchset onto the newest EPP v12 series [1] ?

[1] https://lore.kernel.org/linux-pm/20230131090016.3970625-1-perry.yuan@amd.com/

Cheers,
Tor Vic

> 
> Change log:
> 
> v3 -> v4:
> - Fixed active mode low frequency issue reported by Peter Jung and Tor Vic
> - Documentation modification suggested by Bagas Sanjaya
> 
> v2 -> v3:
> - Addressed review comments form Mario.
> - Picked up RB tag from Mario.
> - Rebase on top of EPP v11 [3].
> 
> v1 -> v2:
> - Fix issue with shared mem systems.
> - Rebase on top of EPP series.
> 
> [1]: https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf
> [2]: https://lore.kernel.org/lkml/20221110175847.3098728-1-Perry.Yuan@amd.com/
> [3]: https://lore.kernel.org/linux-pm/20230118075210.447418-1-perry.yuan@amd.com/
> 
> Wyes Karny (6):
>   acpi: cppc: Add min and max perf reg writing support
>   acpi: cppc: Add auto select register read/write support
>   cpufreq: amd_pstate: Add guided autonomous mode
>   Documentation: amd_pstate: Move amd_pstate param to alphabetical order
>   cpufreq: amd_pstate: Add guided mode control support via sysfs
>   Documentation: amd_pstate: Update amd_pstate status sysfs for guided
> 
>  .../admin-guide/kernel-parameters.txt         |  41 ++--
>  Documentation/admin-guide/pm/amd-pstate.rst   |  31 ++-
>  drivers/acpi/cppc_acpi.c                      | 121 +++++++++++-
>  drivers/cpufreq/amd-pstate.c                  | 184 +++++++++++++-----
>  include/acpi/cppc_acpi.h                      |  11 ++
>  include/linux/amd-pstate.h                    |   2 +
>  6 files changed, 309 insertions(+), 81 deletions(-)
> 
> -- 
> 2.34.1

Re: [PATCH v4 6/6] Documentation: amd_pstate: Update amd_pstate status sysfs for guided

[Bagas Sanjaya]

On 1/31/23 12:21, Wyes Karny wrote:
> diff --git a/Documentation/admin-guide/pm/amd-pstate.rst b/Documentation/admin-guide/pm/amd-pstate.rst
> index 5304adf2fc2f..56a3d11f51a5 100644
> --- a/Documentation/admin-guide/pm/amd-pstate.rst
> +++ b/Documentation/admin-guide/pm/amd-pstate.rst
> @@ -303,13 +303,18 @@ efficiency frequency management method on AMD processors.
>  AMD Pstate Driver Operation Modes
>  =================================
>  
> -``amd_pstate`` CPPC has two operation modes: CPPC Autonomous(active) mode and
> -CPPC non-autonomous(passive) mode.
> -active mode and passive mode can be chosen by different kernel parameters.
> -When in Autonomous mode, CPPC ignores requests done in the Desired Performance
> -Target register and takes into account only the values set to the Minimum requested
> -performance, Maximum requested performance, and Energy Performance Preference
> -registers. When Autonomous is disabled, it only considers the Desired Performance Target.
> +``amd_pstate`` CPPC has 3 operation modes: autonomous (active) mode,
> +non-autonomous (passive) mode and guided autonomous (guided) mode.
> +Active/passive/guided mode can be chosen by different kernel parameters.
> +
> +- In autonomous mode, platform ignores the desired performance level request
> +  and takes into account only the values set to the minimum, maximum and energy
> +  performance preference registers.
> +- In non-autonomous mode, platform gets desired performance level
> +  from OS directly through Desired Performance Register.
> +- In guided-autonomous mode, platform sets operating performance level
> +  autonomously according to the current workload and within the limits set by
> +  OS through min and max performance registers.
>  
>  Active Mode
>  ------------
> @@ -338,6 +343,15 @@ to the Performance Reduction Tolerance register. Above the nominal performance l
>  processor must provide at least nominal performance requested and go higher if current
>  operating conditions allow.
>  
> +Guided Mode
> +-----------
> +
> +``amd_pstate=guided``
> +
> +If ``amd_pstate=guided`` is passed to kernel command line option then this mode
> +is activated.  In this mode, driver requests minimum and maximum performance
> +level and the platform autonomously selects a performance level in this range
> +and appropriate to the current workload.
>  
>  User Space Interface in ``sysfs``
>  =================================
> @@ -358,6 +372,9 @@ control its functionality at the system level.  They are located in the
>  	"passive"
>  		The driver is functional and in the ``passive mode``
>  
> +        "guided"
> +                The driver is functional and in the ``guided mode``
> +
>  	"disable"
>  		The driver is unregistered and not functional now.
>  

LGTM, thanks!

Reviewed-by: Bagas Sanjaya <bagasdotme@gmail.com>

-- 
An old man doll... just what I always wanted! - Clara

Re: [PATCH v4 6/6] Documentation: amd_pstate: Update amd_pstate status sysfs for guided

[Mario Limonciello]

On 1/30/23 23:21, Wyes Karny wrote:
> Update amd_pstate status sysfs for guided mode.
> 
> Signed-off-by: Wyes Karny <wyes.karny@amd.com>
> ---
>   Documentation/admin-guide/pm/amd-pstate.rst | 31 ++++++++++++++++-----
>   1 file changed, 24 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/admin-guide/pm/amd-pstate.rst b/Documentation/admin-guide/pm/amd-pstate.rst
> index 5304adf2fc2f..56a3d11f51a5 100644
> --- a/Documentation/admin-guide/pm/amd-pstate.rst
> +++ b/Documentation/admin-guide/pm/amd-pstate.rst
> @@ -303,13 +303,18 @@ efficiency frequency management method on AMD processors.
>   AMD Pstate Driver Operation Modes
>   =================================
>   
> -``amd_pstate`` CPPC has two operation modes: CPPC Autonomous(active) mode and
> -CPPC non-autonomous(passive) mode.
> -active mode and passive mode can be chosen by different kernel parameters.
> -When in Autonomous mode, CPPC ignores requests done in the Desired Performance
> -Target register and takes into account only the values set to the Minimum requested
> -performance, Maximum requested performance, and Energy Performance Preference
> -registers. When Autonomous is disabled, it only considers the Desired Performance Target.
> +``amd_pstate`` CPPC has 3 operation modes: autonomous (active) mode,
> +non-autonomous (passive) mode and guided autonomous (guided) mode.
> +Active/passive/guided mode can be chosen by different kernel parameters.
> +
> +- In autonomous mode, platform ignores the desired performance level request
> +  and takes into account only the values set to the minimum, maximum and energy
> +  performance preference registers.
> +- In non-autonomous mode, platform gets desired performance level
> +  from OS directly through Desired Performance Register.
> +- In guided-autonomous mode, platform sets operating performance level
> +  autonomously according to the current workload and within the limits set by
> +  OS through min and max performance registers.
>   
>   Active Mode
>   ------------
> @@ -338,6 +343,15 @@ to the Performance Reduction Tolerance register. Above the nominal performance l
>   processor must provide at least nominal performance requested and go higher if current
>   operating conditions allow.
>   
> +Guided Mode
> +-----------
> +
> +``amd_pstate=guided``
> +
> +If ``amd_pstate=guided`` is passed to kernel command line option then this mode
> +is activated.  In this mode, driver requests minimum and maximum performance
> +level and the platform autonomously selects a performance level in this range
> +and appropriate to the current workload.
>   
>   User Space Interface in ``sysfs``
>   =================================
> @@ -358,6 +372,9 @@ control its functionality at the system level.  They are located in the
>   	"passive"
>   		The driver is functional and in the ``passive mode``
>   
> +        "guided"
> +                The driver is functional and in the ``guided mode``
> +
>   	"disable"
>   		The driver is unregistered and not functional now.
>   

Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>

Re: [PATCH v4 3/6] cpufreq: amd_pstate: Add guided autonomous mode

[Mario Limonciello]

On 1/30/23 23:21, Wyes Karny wrote:
>  From ACPI spec below 3 modes for CPPC can be defined:
> 1. Non autonomous: OS scaling governor specifies operating frequency/
>     performance level through `Desired Performance` register and platform
> follows that.
> 2. Guided autonomous: OS scaling governor specifies min and max
>     frequencies/ performance levels through `Minimum Performance` and
> `Maximum Performance` register, and platform can autonomously select an
> operating frequency in this range.
> 3. Fully autonomous: OS only hints (via EPP) to platform for the required
>     energy performance preference for the workload and platform autonomously
> scales the frequency.
> 
> Currently (1) is supported by amd_pstate as passive mode, and (3) is
> implemented by EPP support. This change is to support (2).
> 
> In guided autonomous mode the min_perf is based on the input from the
> scaling governor. For example, in case of schedutil this value depends
> on the current utilization. And max_perf is set to max capacity.
> 
> To activate guided auto mode ``amd_pstate=guided`` command line
> parameter has to be passed in the kernel.
> 
> Signed-off-by: Wyes Karny <wyes.karny@amd.com>
> ---
>   .../admin-guide/kernel-parameters.txt         | 15 +++++---
>   drivers/cpufreq/amd-pstate.c                  | 34 +++++++++++++++----
>   include/linux/amd-pstate.h                    |  2 ++
>   3 files changed, 40 insertions(+), 11 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index e3618dfdb36a..0d8486325c9a 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -7015,11 +7015,11 @@
>   			  Do not enable amd_pstate as the default
>   			  scaling driver for the supported processors
>   			passive
> -			  Use amd_pstate as a scaling driver, driver requests a
> -			  desired performance on this abstract scale and the power
> -			  management firmware translates the requests into actual
> -			  hardware states (core frequency, data fabric and memory
> -			  clocks etc.)
> +			  Use amd_pstate with passive mode as a scaling driver.
> +			  In this mode autonomous selection is disabled.
> +			  Driver requests a desired performance level and platform
> +			  tires to match the same performance level (if it is

s/tires/tries/

I don't think you need the () either in this sentence.

> +			  satisfied by guaranteed performance level).
>   			active
>   			  Use amd_pstate_epp driver instance as the scaling driver,
>   			  driver provides a hint to the hardware if software wants
> @@ -7027,3 +7027,8 @@
>   			  to the CPPC firmware. then CPPC power algorithm will
>   			  calculate the runtime workload and adjust the realtime cores
>   			  frequency.
> +			guided
> +			  Activate guided autonomous mode. Driver requests minimum and
> +			  maximum performance level and the platform autonomously
> +			  selects a performance level in this range and appropriate
> +			  to the current workload.
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index c5731fefcbef..48ab4684c3a5 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -267,7 +267,22 @@ static int cppc_init_perf(struct amd_cpudata *cpudata)
>   		   cppc_perf.lowest_nonlinear_perf);
>   	WRITE_ONCE(cpudata->lowest_perf, cppc_perf.lowest_perf);
>   
> -	return 0;
> +	if (cppc_state == AMD_PSTATE_ACTIVE)
> +		return 0;
> +
> +	ret = cppc_get_auto_sel_caps(cpudata->cpu, &cppc_perf);
> +	if (ret) {
> +		pr_warn("failed to get auto_sel, ret: %d\n", ret);
> +		return 0;
> +	}
> +
> +	ret = cppc_set_auto_sel(cpudata->cpu,
> +			(cppc_state == AMD_PSTATE_PASSIVE) ? 0 : 1);
> +
> +	if (ret)
> +		pr_warn("failed to set auto_sel, ret: %d\n", ret);
> +
> +	return ret;
>   }
>   
>   DEFINE_STATIC_CALL(amd_pstate_init_perf, pstate_init_perf);
> @@ -344,12 +359,18 @@ static inline bool amd_pstate_sample(struct amd_cpudata *cpudata)
>   }
>   
>   static void amd_pstate_update(struct amd_cpudata *cpudata, u32 min_perf,
> -			      u32 des_perf, u32 max_perf, bool fast_switch)
> +			      u32 des_perf, u32 max_perf, bool fast_switch, int guv_flags)

You meant "gov_flags" not "guv_flags" right?

>   {
>   	u64 prev = READ_ONCE(cpudata->cppc_req_cached);
>   	u64 value = prev;
>   
>   	des_perf = clamp_t(unsigned long, des_perf, min_perf, max_perf);
> +
> +	if ((cppc_state == AMD_PSTATE_GUIDED) && (guv_flags & CPUFREQ_GOV_DYNAMIC_SWITCHING)) {
> +		min_perf = des_perf;
> +		des_perf = 0;
> +	}
> +
>   	value &= ~AMD_CPPC_MIN_PERF(~0L);
>   	value |= AMD_CPPC_MIN_PERF(min_perf);
>   
> @@ -404,7 +425,7 @@ static int amd_pstate_target(struct cpufreq_policy *policy,
>   
>   	cpufreq_freq_transition_begin(policy, &freqs);
>   	amd_pstate_update(cpudata, min_perf, des_perf,
> -			  max_perf, false);
> +			  max_perf, false, policy->governor->flags);
>   	cpufreq_freq_transition_end(policy, &freqs, false);
>   
>   	return 0;
> @@ -438,7 +459,8 @@ static void amd_pstate_adjust_perf(unsigned int cpu,
>   	if (max_perf < min_perf)
>   		max_perf = min_perf;
>   
> -	amd_pstate_update(cpudata, min_perf, des_perf, max_perf, true);
> +	amd_pstate_update(cpudata, min_perf, des_perf, max_perf, true,
> +			policy->governor->flags);
>   	cpufreq_cpu_put(policy);
>   }
>   
> @@ -1238,7 +1260,7 @@ static int __init amd_pstate_init(void)
>   	/* capability check */
>   	if (boot_cpu_has(X86_FEATURE_CPPC)) {
>   		pr_debug("AMD CPPC MSR based functionality is supported\n");
> -		if (cppc_state == AMD_PSTATE_PASSIVE)
> +		if (cppc_state != AMD_PSTATE_ACTIVE)
>   			current_pstate_driver->adjust_perf = amd_pstate_adjust_perf;
>   	} else {
>   		pr_debug("AMD CPPC shared memory based functionality is supported\n");
> @@ -1300,7 +1322,7 @@ static int __init amd_pstate_param(char *str)
>   		if (cppc_state == AMD_PSTATE_ACTIVE)
>   			current_pstate_driver = &amd_pstate_epp_driver;
>   
> -		if (cppc_state == AMD_PSTATE_PASSIVE)
> +		if (cppc_state == AMD_PSTATE_PASSIVE || cppc_state == AMD_PSTATE_GUIDED)
>   			current_pstate_driver = &amd_pstate_driver;
>   
>   		return 0;
> diff --git a/include/linux/amd-pstate.h b/include/linux/amd-pstate.h
> index e9e291b7c2f9..85e980b8e0ac 100644
> --- a/include/linux/amd-pstate.h
> +++ b/include/linux/amd-pstate.h
> @@ -92,6 +92,7 @@ enum amd_pstate_mode {
>   	AMD_PSTATE_DISABLE = 0,
>   	AMD_PSTATE_PASSIVE,
>   	AMD_PSTATE_ACTIVE,
> +	AMD_PSTATE_GUIDED,
>   	AMD_PSTATE_MAX,
>   };
>   
> @@ -99,6 +100,7 @@ static const char * const amd_pstate_mode_string[] = {
>   	[AMD_PSTATE_DISABLE]     = "disable",
>   	[AMD_PSTATE_PASSIVE]     = "passive",
>   	[AMD_PSTATE_ACTIVE]      = "active",
> +	[AMD_PSTATE_GUIDED]      = "guided",
>   	NULL,
>   };
>   

With two nits fixed

Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>

Re: [PATCH v4 5/6] cpufreq: amd_pstate: Add guided mode control support via sysfs

[Mario Limonciello]

On 1/30/23 23:21, Wyes Karny wrote:
> amd_pstate driver's `status` sysfs entry helps to control the driver's
> mode dynamically by user. After the addition of guided mode the
> combinations of mode transitions have been increased (16 combinations).
> Therefore optimise the amd_pstate_update_status function by implementing
> a state transition table.
> 
> There are 4 states amd_pstate supports, namely: 'disable', 'passive',
> 'active', and 'guided'.  The transition from any state to any other
> state is possible after this change. Only if the state requested matches
> with the current state then -EBUSY value is returned.

I realized this after I finished reviewing doc patch, but you probably 
want to explain -EBUSY return code in documentation patch too.

> 
> Sysfs interface:
> 
> To disable amd_pstate driver:
>   # echo disable > /sys/devices/system/cpu/amd_pstate/status
> 
> To enable passive mode:
>   # echo passive > /sys/devices/system/cpu/amd_pstate/status
> 
> To change mode to active:
>   # echo active > /sys/devices/system/cpu/amd_pstate/status
> 
> To change mode to guided:
>   # echo guided > /sys/devices/system/cpu/amd_pstate/status
> 
> Signed-off-by: Wyes Karny <wyes.karny@amd.com>
> ---
>   drivers/cpufreq/amd-pstate.c | 150 +++++++++++++++++++++++++----------
>   1 file changed, 108 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 48ab4684c3a5..6c522dec6967 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -65,6 +65,8 @@ static struct cpufreq_driver amd_pstate_epp_driver;
>   static int cppc_state = AMD_PSTATE_DISABLE;
>   struct kobject *amd_pstate_kobj;
>   
> +typedef int (*cppc_mode_transition_fn)(int);
> +
>   static inline int get_mode_idx_from_str(const char *str, size_t size)
>   {
>   	int i;
> @@ -797,6 +799,105 @@ static ssize_t show_energy_performance_preference(
>   	return sysfs_emit(buf, "%s\n", energy_perf_strings[preference]);
>   }
>   
> +static void amd_pstate_driver_cleanup(void)
> +{
> +	amd_pstate_enable(false);
> +	cppc_state = AMD_PSTATE_DISABLE;
> +	current_pstate_driver = NULL;
> +}
> +
> +static int amd_pstate_register_driver(int mode)
> +{
> +	int ret;
> +
> +	if (mode == AMD_PSTATE_PASSIVE || mode == AMD_PSTATE_GUIDED)
> +		current_pstate_driver = &amd_pstate_driver;
> +	else if (mode == AMD_PSTATE_ACTIVE)
> +		current_pstate_driver = &amd_pstate_epp_driver;
> +	else
> +		return -EINVAL;
> +
> +	cppc_state = mode;
> +	ret = cpufreq_register_driver(current_pstate_driver);
> +	if (ret) {
> +		amd_pstate_driver_cleanup();
> +		return ret;
> +	}
> +	return 0;
> +}
> +
> +static int amd_pstate_unregister_driver(int dummy)
> +{
> +	int ret;
> +
> +	ret = cpufreq_unregister_driver(current_pstate_driver);
> +
> +	if (ret)
> +		return ret;
> +
> +	amd_pstate_driver_cleanup();
> +	return 0;
> +}
> +
> +static int amd_pstate_change_mode_without_dvr_change(int mode)
> +{
> +	int cpu = 0;
> +
> +	cppc_state = mode;
> +
> +	if (boot_cpu_has(X86_FEATURE_CPPC) || cppc_state == AMD_PSTATE_ACTIVE)
> +		return 0;
> +
> +	for_each_present_cpu(cpu) {
> +		cppc_set_auto_sel(cpu, (cppc_state == AMD_PSTATE_PASSIVE) ? 0 : 1);
> +	}
> +
> +	return 0;
> +}
> +
> +static int amd_pstate_change_driver_mode(int mode)
> +{
> +	int ret;
> +
> +	ret = amd_pstate_unregister_driver(0);
> +	if (ret)
> +		return ret;
> +
> +	ret = amd_pstate_register_driver(mode);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +/* Mode transition table */

This seems to be a pointless comment to me.

> +cppc_mode_transition_fn mode_state_machine[AMD_PSTATE_MAX][AMD_PSTATE_MAX] = {
> +	[AMD_PSTATE_DISABLE]         = {
> +		[AMD_PSTATE_DISABLE]     = NULL,
> +		[AMD_PSTATE_PASSIVE]     = amd_pstate_register_driver,
> +		[AMD_PSTATE_ACTIVE]      = amd_pstate_register_driver,
> +		[AMD_PSTATE_GUIDED]      = amd_pstate_register_driver,
> +	},
> +	[AMD_PSTATE_PASSIVE]         = {
> +		[AMD_PSTATE_DISABLE]     = amd_pstate_unregister_driver,
> +		[AMD_PSTATE_PASSIVE]     = NULL,
> +		[AMD_PSTATE_ACTIVE]      = amd_pstate_change_driver_mode,
> +		[AMD_PSTATE_GUIDED]      = amd_pstate_change_mode_without_dvr_change,
> +	},
> +	[AMD_PSTATE_ACTIVE]          = {
> +		[AMD_PSTATE_DISABLE]     = amd_pstate_unregister_driver,
> +		[AMD_PSTATE_PASSIVE]     = amd_pstate_change_driver_mode,
> +		[AMD_PSTATE_ACTIVE]      = NULL,
> +		[AMD_PSTATE_GUIDED]      = amd_pstate_change_driver_mode,
> +	},
> +	[AMD_PSTATE_GUIDED]          = {
> +		[AMD_PSTATE_DISABLE]     = amd_pstate_unregister_driver,
> +		[AMD_PSTATE_PASSIVE]     = amd_pstate_change_mode_without_dvr_change,
> +		[AMD_PSTATE_ACTIVE]      = amd_pstate_change_driver_mode,
> +		[AMD_PSTATE_GUIDED]      = NULL,
> +	},
> +};
> +
>   static ssize_t amd_pstate_show_status(char *buf)
>   {
>   	if (!current_pstate_driver)
> @@ -805,57 +906,22 @@ static ssize_t amd_pstate_show_status(char *buf)
>   	return sysfs_emit(buf, "%s\n", amd_pstate_mode_string[cppc_state]);
>   }
>   
> -static void amd_pstate_driver_cleanup(void)
> -{
> -	current_pstate_driver = NULL;
> -}
> -
>   static int amd_pstate_update_status(const char *buf, size_t size)
>   {
> -	int ret;
>   	int mode_idx;
>   
> -	if (size > 7 || size < 6)
> +	if (size > strlen("passive") || size < strlen("active"))
>   		return -EINVAL;
> -	mode_idx = get_mode_idx_from_str(buf, size);
>   
> -	switch(mode_idx) {
> -	case AMD_PSTATE_DISABLE:
> -		if (!current_pstate_driver)
> -			return -EINVAL;
> -		if (cppc_state == AMD_PSTATE_ACTIVE)
> -			return -EBUSY;
> -		ret = cpufreq_unregister_driver(current_pstate_driver);
> -		amd_pstate_driver_cleanup();
> -		break;
> -	case AMD_PSTATE_PASSIVE:
> -		if (current_pstate_driver) {
> -			if (current_pstate_driver == &amd_pstate_driver)
> -				return 0;
> -			cpufreq_unregister_driver(current_pstate_driver);
> -			cppc_state = AMD_PSTATE_PASSIVE;
> -			current_pstate_driver = &amd_pstate_driver;
> -		}
> +	mode_idx = get_mode_idx_from_str(buf, size);
>   
> -		ret = cpufreq_register_driver(current_pstate_driver);
> -		break;
> -	case AMD_PSTATE_ACTIVE:
> -		if (current_pstate_driver) {
> -			if (current_pstate_driver == &amd_pstate_epp_driver)
> -				return 0;
> -			cpufreq_unregister_driver(current_pstate_driver);
> -			current_pstate_driver = &amd_pstate_epp_driver;
> -			cppc_state = AMD_PSTATE_ACTIVE;
> -		}
> +	if (mode_idx < 0 || mode_idx >= AMD_PSTATE_MAX)
> +		return -EINVAL;
>   
> -		ret = cpufreq_register_driver(current_pstate_driver);
> -		break;
> -	default:
> -		ret = -EINVAL;
> -		break;
> -	}
> +	if (mode_state_machine[cppc_state][mode_idx])
> +		return mode_state_machine[cppc_state][mode_idx](mode_idx);
>   
> -	return ret;
> +	return -EBUSY;
>   }
>   
>   static ssize_t show_status(struct kobject *kobj,

With one nit fixed,

Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>

Re: [PATCH v4 0/6] amd_pstate: Add guided autonomous mode support

[Oleksandr Natalenko]

Hello.

On úterý 31. ledna 2023 11:37:51 CET torvic9@mailbox.org wrote:
> Can you rebase this patchset onto the newest EPP v12 series [1] ?
> 
> [1] https://lore.kernel.org/linux-pm/20230131090016.3970625-1-perry.yuan@amd.com/

You may consider trying my rebase here: [1].

This commit applies to v11+v3 series. I hope I didn't miss anything.

If interested, please let me know whether it works for you.

Thanks.

[1] https://codeberg.org/pf-kernel/linux/commit/438525b8029b23967722e9c7af97c6b8deb25029

-- 
Oleksandr Natalenko (post-factum)

Re: [PATCH v4 0/6] amd_pstate: Add guided autonomous mode support

[torvic9]

> Oleksandr Natalenko <oleksandr@natalenko.name> hat am 31.01.2023 15:13 GMT geschrieben:
> 
>  
> Hello.

Hi,

> 
> On úterý 31. ledna 2023 11:37:51 CET torvic9@mailbox.org wrote:
> > Can you rebase this patchset onto the newest EPP v12 series [1] ?
> > 
> > [1] https://lore.kernel.org/linux-pm/20230131090016.3970625-1-perry.yuan@amd.com/
> 
> You may consider trying my rebase here: [1].
> 
> This commit applies to v11+v3 series. I hope I didn't miss anything.
> 
> If interested, please let me know whether it works for you.

Thank you for your work. I rebased it myself in the meantime.

> 
> Thanks.
> 
> [1] https://codeberg.org/pf-kernel/linux/commit/438525b8029b23967722e9c7af97c6b8deb25029
> 
> -- 
> Oleksandr Natalenko (post-factum)

Re: [PATCH v4 3/6] cpufreq: amd_pstate: Add guided autonomous mode

[Wyes Karny]

On 31 Jan 07:37, Mario Limonciello wrote:
> On 1/30/23 23:21, Wyes Karny wrote:
> >  From ACPI spec below 3 modes for CPPC can be defined:
> > 1. Non autonomous: OS scaling governor specifies operating frequency/
> >     performance level through `Desired Performance` register and platform
> > follows that.
> > 2. Guided autonomous: OS scaling governor specifies min and max
> >     frequencies/ performance levels through `Minimum Performance` and
> > `Maximum Performance` register, and platform can autonomously select an
> > operating frequency in this range.
> > 3. Fully autonomous: OS only hints (via EPP) to platform for the required
> >     energy performance preference for the workload and platform autonomously
> > scales the frequency.
> > 
> > Currently (1) is supported by amd_pstate as passive mode, and (3) is
> > implemented by EPP support. This change is to support (2).
> > 
> > In guided autonomous mode the min_perf is based on the input from the
> > scaling governor. For example, in case of schedutil this value depends
> > on the current utilization. And max_perf is set to max capacity.
> > 
> > To activate guided auto mode ``amd_pstate=guided`` command line
> > parameter has to be passed in the kernel.
> > 
> > Signed-off-by: Wyes Karny <wyes.karny@amd.com>
> > ---
> >   .../admin-guide/kernel-parameters.txt         | 15 +++++---
> >   drivers/cpufreq/amd-pstate.c                  | 34 +++++++++++++++----
> >   include/linux/amd-pstate.h                    |  2 ++
> >   3 files changed, 40 insertions(+), 11 deletions(-)
> > 
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > index e3618dfdb36a..0d8486325c9a 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -7015,11 +7015,11 @@
> >   			  Do not enable amd_pstate as the default
> >   			  scaling driver for the supported processors
> >   			passive
> > -			  Use amd_pstate as a scaling driver, driver requests a
> > -			  desired performance on this abstract scale and the power
> > -			  management firmware translates the requests into actual
> > -			  hardware states (core frequency, data fabric and memory
> > -			  clocks etc.)
> > +			  Use amd_pstate with passive mode as a scaling driver.
> > +			  In this mode autonomous selection is disabled.
> > +			  Driver requests a desired performance level and platform
> > +			  tires to match the same performance level (if it is
> 
> s/tires/tries/
> 
> I don't think you need the () either in this sentence.
> 
> > +			  satisfied by guaranteed performance level).
> >   			active
> >   			  Use amd_pstate_epp driver instance as the scaling driver,
> >   			  driver provides a hint to the hardware if software wants
> > @@ -7027,3 +7027,8 @@
> >   			  to the CPPC firmware. then CPPC power algorithm will
> >   			  calculate the runtime workload and adjust the realtime cores
> >   			  frequency.
> > +			guided
> > +			  Activate guided autonomous mode. Driver requests minimum and
> > +			  maximum performance level and the platform autonomously
> > +			  selects a performance level in this range and appropriate
> > +			  to the current workload.
> > diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> > index c5731fefcbef..48ab4684c3a5 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -267,7 +267,22 @@ static int cppc_init_perf(struct amd_cpudata *cpudata)
> >   		   cppc_perf.lowest_nonlinear_perf);
> >   	WRITE_ONCE(cpudata->lowest_perf, cppc_perf.lowest_perf);
> > -	return 0;
> > +	if (cppc_state == AMD_PSTATE_ACTIVE)
> > +		return 0;
> > +
> > +	ret = cppc_get_auto_sel_caps(cpudata->cpu, &cppc_perf);
> > +	if (ret) {
> > +		pr_warn("failed to get auto_sel, ret: %d\n", ret);
> > +		return 0;
> > +	}
> > +
> > +	ret = cppc_set_auto_sel(cpudata->cpu,
> > +			(cppc_state == AMD_PSTATE_PASSIVE) ? 0 : 1);
> > +
> > +	if (ret)
> > +		pr_warn("failed to set auto_sel, ret: %d\n", ret);
> > +
> > +	return ret;
> >   }
> >   DEFINE_STATIC_CALL(amd_pstate_init_perf, pstate_init_perf);
> > @@ -344,12 +359,18 @@ static inline bool amd_pstate_sample(struct amd_cpudata *cpudata)
> >   }
> >   static void amd_pstate_update(struct amd_cpudata *cpudata, u32 min_perf,
> > -			      u32 des_perf, u32 max_perf, bool fast_switch)
> > +			      u32 des_perf, u32 max_perf, bool fast_switch, int guv_flags)
> 
> You meant "gov_flags" not "guv_flags" right?
> 
> >   {
> >   	u64 prev = READ_ONCE(cpudata->cppc_req_cached);
> >   	u64 value = prev;
> >   	des_perf = clamp_t(unsigned long, des_perf, min_perf, max_perf);
> > +
> > +	if ((cppc_state == AMD_PSTATE_GUIDED) && (guv_flags & CPUFREQ_GOV_DYNAMIC_SWITCHING)) {
> > +		min_perf = des_perf;
> > +		des_perf = 0;
> > +	}
> > +
> >   	value &= ~AMD_CPPC_MIN_PERF(~0L);
> >   	value |= AMD_CPPC_MIN_PERF(min_perf);
> > @@ -404,7 +425,7 @@ static int amd_pstate_target(struct cpufreq_policy *policy,
> >   	cpufreq_freq_transition_begin(policy, &freqs);
> >   	amd_pstate_update(cpudata, min_perf, des_perf,
> > -			  max_perf, false);
> > +			  max_perf, false, policy->governor->flags);
> >   	cpufreq_freq_transition_end(policy, &freqs, false);
> >   	return 0;
> > @@ -438,7 +459,8 @@ static void amd_pstate_adjust_perf(unsigned int cpu,
> >   	if (max_perf < min_perf)
> >   		max_perf = min_perf;
> > -	amd_pstate_update(cpudata, min_perf, des_perf, max_perf, true);
> > +	amd_pstate_update(cpudata, min_perf, des_perf, max_perf, true,
> > +			policy->governor->flags);
> >   	cpufreq_cpu_put(policy);
> >   }
> > @@ -1238,7 +1260,7 @@ static int __init amd_pstate_init(void)
> >   	/* capability check */
> >   	if (boot_cpu_has(X86_FEATURE_CPPC)) {
> >   		pr_debug("AMD CPPC MSR based functionality is supported\n");
> > -		if (cppc_state == AMD_PSTATE_PASSIVE)
> > +		if (cppc_state != AMD_PSTATE_ACTIVE)
> >   			current_pstate_driver->adjust_perf = amd_pstate_adjust_perf;
> >   	} else {
> >   		pr_debug("AMD CPPC shared memory based functionality is supported\n");
> > @@ -1300,7 +1322,7 @@ static int __init amd_pstate_param(char *str)
> >   		if (cppc_state == AMD_PSTATE_ACTIVE)
> >   			current_pstate_driver = &amd_pstate_epp_driver;
> > -		if (cppc_state == AMD_PSTATE_PASSIVE)
> > +		if (cppc_state == AMD_PSTATE_PASSIVE || cppc_state == AMD_PSTATE_GUIDED)
> >   			current_pstate_driver = &amd_pstate_driver;
> >   		return 0;
> > diff --git a/include/linux/amd-pstate.h b/include/linux/amd-pstate.h
> > index e9e291b7c2f9..85e980b8e0ac 100644
> > --- a/include/linux/amd-pstate.h
> > +++ b/include/linux/amd-pstate.h
> > @@ -92,6 +92,7 @@ enum amd_pstate_mode {
> >   	AMD_PSTATE_DISABLE = 0,
> >   	AMD_PSTATE_PASSIVE,
> >   	AMD_PSTATE_ACTIVE,
> > +	AMD_PSTATE_GUIDED,
> >   	AMD_PSTATE_MAX,
> >   };
> > @@ -99,6 +100,7 @@ static const char * const amd_pstate_mode_string[] = {
> >   	[AMD_PSTATE_DISABLE]     = "disable",
> >   	[AMD_PSTATE_PASSIVE]     = "passive",
> >   	[AMD_PSTATE_ACTIVE]      = "active",
> > +	[AMD_PSTATE_GUIDED]      = "guided",
> >   	NULL,
> >   };
> 
> With two nits fixed
> 
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>

Thanks! I'll fix those.

> 

Re: [PATCH v4 5/6] cpufreq: amd_pstate: Add guided mode control support via sysfs

[Wyes Karny]

On 31 Jan 07:45, Mario Limonciello wrote:
> On 1/30/23 23:21, Wyes Karny wrote:
> > amd_pstate driver's `status` sysfs entry helps to control the driver's
> > mode dynamically by user. After the addition of guided mode the
> > combinations of mode transitions have been increased (16 combinations).
> > Therefore optimise the amd_pstate_update_status function by implementing
> > a state transition table.
> > 
> > There are 4 states amd_pstate supports, namely: 'disable', 'passive',
> > 'active', and 'guided'.  The transition from any state to any other
> > state is possible after this change. Only if the state requested matches
> > with the current state then -EBUSY value is returned.
> 
> I realized this after I finished reviewing doc patch, but you probably want
> to explain -EBUSY return code in documentation patch too.

Yes, I'll add this to documentation. Thanks!

> 
> > 
> > Sysfs interface:
> > 
> > To disable amd_pstate driver:
> >   # echo disable > /sys/devices/system/cpu/amd_pstate/status
> > 
> > To enable passive mode:
> >   # echo passive > /sys/devices/system/cpu/amd_pstate/status
> > 
> > To change mode to active:
> >   # echo active > /sys/devices/system/cpu/amd_pstate/status
> > 
> > To change mode to guided:
> >   # echo guided > /sys/devices/system/cpu/amd_pstate/status
> > 
> > Signed-off-by: Wyes Karny <wyes.karny@amd.com>
> > ---
> >   drivers/cpufreq/amd-pstate.c | 150 +++++++++++++++++++++++++----------
> >   1 file changed, 108 insertions(+), 42 deletions(-)
> > 
> > diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> > index 48ab4684c3a5..6c522dec6967 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -65,6 +65,8 @@ static struct cpufreq_driver amd_pstate_epp_driver;
> >   static int cppc_state = AMD_PSTATE_DISABLE;
> >   struct kobject *amd_pstate_kobj;
> > +typedef int (*cppc_mode_transition_fn)(int);
> > +
> >   static inline int get_mode_idx_from_str(const char *str, size_t size)
> >   {
> >   	int i;
> > @@ -797,6 +799,105 @@ static ssize_t show_energy_performance_preference(
> >   	return sysfs_emit(buf, "%s\n", energy_perf_strings[preference]);
> >   }
> > +static void amd_pstate_driver_cleanup(void)
> > +{
> > +	amd_pstate_enable(false);
> > +	cppc_state = AMD_PSTATE_DISABLE;
> > +	current_pstate_driver = NULL;
> > +}
> > +
> > +static int amd_pstate_register_driver(int mode)
> > +{
> > +	int ret;
> > +
> > +	if (mode == AMD_PSTATE_PASSIVE || mode == AMD_PSTATE_GUIDED)
> > +		current_pstate_driver = &amd_pstate_driver;
> > +	else if (mode == AMD_PSTATE_ACTIVE)
> > +		current_pstate_driver = &amd_pstate_epp_driver;
> > +	else
> > +		return -EINVAL;
> > +
> > +	cppc_state = mode;
> > +	ret = cpufreq_register_driver(current_pstate_driver);
> > +	if (ret) {
> > +		amd_pstate_driver_cleanup();
> > +		return ret;
> > +	}
> > +	return 0;
> > +}
> > +
> > +static int amd_pstate_unregister_driver(int dummy)
> > +{
> > +	int ret;
> > +
> > +	ret = cpufreq_unregister_driver(current_pstate_driver);
> > +
> > +	if (ret)
> > +		return ret;
> > +
> > +	amd_pstate_driver_cleanup();
> > +	return 0;
> > +}
> > +
> > +static int amd_pstate_change_mode_without_dvr_change(int mode)
> > +{
> > +	int cpu = 0;
> > +
> > +	cppc_state = mode;
> > +
> > +	if (boot_cpu_has(X86_FEATURE_CPPC) || cppc_state == AMD_PSTATE_ACTIVE)
> > +		return 0;
> > +
> > +	for_each_present_cpu(cpu) {
> > +		cppc_set_auto_sel(cpu, (cppc_state == AMD_PSTATE_PASSIVE) ? 0 : 1);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int amd_pstate_change_driver_mode(int mode)
> > +{
> > +	int ret;
> > +
> > +	ret = amd_pstate_unregister_driver(0);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = amd_pstate_register_driver(mode);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return 0;
> > +}
> > +
> > +/* Mode transition table */
> 
> This seems to be a pointless comment to me.
> 
> > +cppc_mode_transition_fn mode_state_machine[AMD_PSTATE_MAX][AMD_PSTATE_MAX] = {
> > +	[AMD_PSTATE_DISABLE]         = {
> > +		[AMD_PSTATE_DISABLE]     = NULL,
> > +		[AMD_PSTATE_PASSIVE]     = amd_pstate_register_driver,
> > +		[AMD_PSTATE_ACTIVE]      = amd_pstate_register_driver,
> > +		[AMD_PSTATE_GUIDED]      = amd_pstate_register_driver,
> > +	},
> > +	[AMD_PSTATE_PASSIVE]         = {
> > +		[AMD_PSTATE_DISABLE]     = amd_pstate_unregister_driver,
> > +		[AMD_PSTATE_PASSIVE]     = NULL,
> > +		[AMD_PSTATE_ACTIVE]      = amd_pstate_change_driver_mode,
> > +		[AMD_PSTATE_GUIDED]      = amd_pstate_change_mode_without_dvr_change,
> > +	},
> > +	[AMD_PSTATE_ACTIVE]          = {
> > +		[AMD_PSTATE_DISABLE]     = amd_pstate_unregister_driver,
> > +		[AMD_PSTATE_PASSIVE]     = amd_pstate_change_driver_mode,
> > +		[AMD_PSTATE_ACTIVE]      = NULL,
> > +		[AMD_PSTATE_GUIDED]      = amd_pstate_change_driver_mode,
> > +	},
> > +	[AMD_PSTATE_GUIDED]          = {
> > +		[AMD_PSTATE_DISABLE]     = amd_pstate_unregister_driver,
> > +		[AMD_PSTATE_PASSIVE]     = amd_pstate_change_mode_without_dvr_change,
> > +		[AMD_PSTATE_ACTIVE]      = amd_pstate_change_driver_mode,
> > +		[AMD_PSTATE_GUIDED]      = NULL,
> > +	},
> > +};
> > +
> >   static ssize_t amd_pstate_show_status(char *buf)
> >   {
> >   	if (!current_pstate_driver)
> > @@ -805,57 +906,22 @@ static ssize_t amd_pstate_show_status(char *buf)
> >   	return sysfs_emit(buf, "%s\n", amd_pstate_mode_string[cppc_state]);
> >   }
> > -static void amd_pstate_driver_cleanup(void)
> > -{
> > -	current_pstate_driver = NULL;
> > -}
> > -
> >   static int amd_pstate_update_status(const char *buf, size_t size)
> >   {
> > -	int ret;
> >   	int mode_idx;
> > -	if (size > 7 || size < 6)
> > +	if (size > strlen("passive") || size < strlen("active"))
> >   		return -EINVAL;
> > -	mode_idx = get_mode_idx_from_str(buf, size);
> > -	switch(mode_idx) {
> > -	case AMD_PSTATE_DISABLE:
> > -		if (!current_pstate_driver)
> > -			return -EINVAL;
> > -		if (cppc_state == AMD_PSTATE_ACTIVE)
> > -			return -EBUSY;
> > -		ret = cpufreq_unregister_driver(current_pstate_driver);
> > -		amd_pstate_driver_cleanup();
> > -		break;
> > -	case AMD_PSTATE_PASSIVE:
> > -		if (current_pstate_driver) {
> > -			if (current_pstate_driver == &amd_pstate_driver)
> > -				return 0;
> > -			cpufreq_unregister_driver(current_pstate_driver);
> > -			cppc_state = AMD_PSTATE_PASSIVE;
> > -			current_pstate_driver = &amd_pstate_driver;
> > -		}
> > +	mode_idx = get_mode_idx_from_str(buf, size);
> > -		ret = cpufreq_register_driver(current_pstate_driver);
> > -		break;
> > -	case AMD_PSTATE_ACTIVE:
> > -		if (current_pstate_driver) {
> > -			if (current_pstate_driver == &amd_pstate_epp_driver)
> > -				return 0;
> > -			cpufreq_unregister_driver(current_pstate_driver);
> > -			current_pstate_driver = &amd_pstate_epp_driver;
> > -			cppc_state = AMD_PSTATE_ACTIVE;
> > -		}
> > +	if (mode_idx < 0 || mode_idx >= AMD_PSTATE_MAX)
> > +		return -EINVAL;
> > -		ret = cpufreq_register_driver(current_pstate_driver);
> > -		break;
> > -	default:
> > -		ret = -EINVAL;
> > -		break;
> > -	}
> > +	if (mode_state_machine[cppc_state][mode_idx])
> > +		return mode_state_machine[cppc_state][mode_idx](mode_idx);
> > -	return ret;
> > +	return -EBUSY;
> >   }
> >   static ssize_t show_status(struct kobject *kobj,
> 
> With one nit fixed,
> 
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>

Re: [PATCH v4 0/6] amd_pstate: Add guided autonomous mode support

[Wyes Karny]

On 31 Jan 11:37, torvic9@mailbox.org wrote:
> 
> > Wyes Karny <wyes.karny@amd.com> hat am 31.01.2023 05:21 GMT geschrieben:
> > 
> >  
> > From ACPI spec[1] below 3 modes for CPPC can be defined:
> > 1. Non autonomous: OS scaling governor specifies operating frequency/
> >    performance level through `Desired Performance` register and platform
> > follows that.
> > 2. Guided autonomous: OS scaling governor specifies min and max
> >    frequencies/ performance levels through `Minimum Performance` and
> > `Maximum Performance` register, and platform can autonomously select an
> > operating frequency in this range.
> > 3. Fully autonomous: OS only hints (via EPP) to platform for the required
> >    energy performance preference for the workload and platform autonomously
> > scales the frequency.
> > 
> > Currently (1) is supported by amd_pstate as passive mode, and (3) is
> > implemented by EPP support[2]. This change is to support (2).
> > 
> > In guided autonomous mode the min_perf is based on the input from the
> > scaling governor. For example, in case of schedutil this value depends
> > on the current utilization. And max_perf is set to max capacity.
> > 
> > To activate guided auto mode ``amd_pstate=guided`` command line
> > parameter has to be passed in the kernel.
> > 
> > Below are the results (normalized) of benchmarks with this patch:
> > System: Genoa 96C 192T
> > Kernel: 6.2.0-rc2 + EPP v11 + patch
> > Scaling governor: schedutil
> > 
> > ================ dbench comparisons ================
> > dbench result comparison:
> > Here results are throughput (MB/s)
> > Clients:   acpi-cpufreq		   amd_pst+passive	   amd_pst+guided
> >     1	   1.00 (0.00 pct)	   1.01 (1.00 pct)	   1.02 (2.00 pct)
> >     2	   1.07 (0.00 pct)	   1.06 (-0.93 pct)	   1.07 (0.00 pct)
> >     4	   1.68 (0.00 pct)	   1.70 (1.19 pct)	   1.72 (2.38 pct)
> >     8	   2.61 (0.00 pct)	   2.68 (2.68 pct)	   2.76 (5.74 pct)
> >    16	   4.16 (0.00 pct)	   4.24 (1.92 pct)	   4.53 (8.89 pct)
> >    32	   5.98 (0.00 pct)	   6.17 (3.17 pct)	   7.30 (22.07 pct)
> >    64	   8.67 (0.00 pct)	   8.99 (3.69 pct)	  10.71 (23.52 pct)
> >   128	  11.98 (0.00 pct)	  12.52 (4.50 pct)	  14.67 (22.45 pct)
> >   256	  15.73 (0.00 pct)	  16.13 (2.54 pct)	  17.81 (13.22 pct)
> >   512	  15.77 (0.00 pct)	  16.32 (3.48 pct)	  16.39 (3.93 pct)
> > dbench power comparison:
> > Clients:   acpi-cpufreq		   amd_pst+passive	   amd_pst+guided
> >     1	   1.00 (0.00 pct)	   1.00 (0.00 pct)	   1.04 (4.00 pct)
> >     2	   0.99 (0.00 pct)	   0.97 (-2.02 pct)	   1.02 (3.03 pct)
> >     4	   0.98 (0.00 pct)	   0.98 (0.00 pct)	   1.02 (4.08 pct)
> >     8	   0.98 (0.00 pct)	   0.99 (1.02 pct)	   1.02 (4.08 pct)
> >    16	   0.99 (0.00 pct)	   1.00 (1.01 pct)	   1.04 (5.05 pct)
> >    32	   1.02 (0.00 pct)	   1.02 (0.00 pct)	   1.07 (4.90 pct)
> >    64	   1.05 (0.00 pct)	   1.05 (0.00 pct)	   1.11 (5.71 pct)
> >   128	   1.08 (0.00 pct)	   1.08 (0.00 pct)	   1.15 (6.48 pct)
> >   256	   1.12 (0.00 pct)	   1.12 (0.00 pct)	   1.20 (7.14 pct)
> >   512	   1.18 (0.00 pct)	   1.17 (-0.84 pct)	   1.26 (6.77 pct)
> > 
> > ================ git-source comparisons ================
> > git-source result comparison:
> > Here results are throughput (compilations per 1000 sec)
> > Threads:   acpi-cpufreq		   amd_pst+passive	   amd_pst+guided
> >   192	   1.00 (0.00 pct)	   0.93 (-7.00 pct)	   1.00 (0.00 pct)
> > git-source power comparison:
> > Threads:   acpi-cpufreq		   amd_pst+passive	   amd_pst+guided
> >   192	   1.00 (0.00 pct)	   1.00 (0.00 pct)	   0.96 (-4.00 pct)
> > 
> > ================ kernbench comparisons ================
> > kernbench result comparison:
> > Here results are throughput (compilations per 1000 sec)
> > Load:	   acpi-cpufreq		   amd_pst+passive	   amd_pst+guided
> > 32	   1.00 (0.00 pct)	   1.01 (1.00 pct)	   1.02 (2.00 pct)
> > 48	   1.26 (0.00 pct)	   1.28 (1.58 pct)	   1.25 (-0.79 pct)
> > 64	   1.39 (0.00 pct)	   1.47 (5.75 pct)	   1.43 (2.87 pct)
> > 96	   1.48 (0.00 pct)	   1.50 (1.35 pct)	   1.49 (0.67 pct)
> > 128	   1.29 (0.00 pct)	   1.32 (2.32 pct)	   1.33 (3.10 pct)
> > 192	   1.17 (0.00 pct)	   1.20 (2.56 pct)	   1.21 (3.41 pct)
> > 256	   1.17 (0.00 pct)	   1.18 (0.85 pct)	   1.20 (2.56 pct)
> > 384	   1.16 (0.00 pct)	   1.17 (0.86 pct)	   1.21 (4.31 pct)
> > kernbench power comparison:
> > Clients:   acpi-cpufreq		   amd_pst+passive	   amd_pst+guided
> >    32	   1.00 (0.00 pct)	   0.97 (-3.00 pct)	   1.00 (0.00 pct)
> >    48	   0.87 (0.00 pct)	   0.81 (-6.89 pct)	   0.88 (1.14 pct)
> >    64	   0.81 (0.00 pct)	   0.73 (-9.87 pct)	   0.77 (-4.93 pct)
> >    96	   0.75 (0.00 pct)	   0.74 (-1.33 pct)	   0.75 (0.00 pct)
> >   128	   0.83 (0.00 pct)	   0.79 (-4.81 pct)	   0.83 (0.00 pct)
> >   192	   0.92 (0.00 pct)	   0.88 (-4.34 pct)	   0.92 (0.00 pct)
> >   256	   0.92 (0.00 pct)	   0.88 (-4.34 pct)	   0.92 (0.00 pct)
> >   384	   0.92 (0.00 pct)	   0.88 (-4.34 pct)	   0.92 (0.00 pct)
> > 
> > ================ tbench comparisons ================
> > tbench result comparison:
> > Here results are throughput (MB/s)
> > Clients:   acpi-cpufreq		   amd_pst+passive	   amd_pst+guided
> >     1	   1.00 (0.00 pct)	   0.70 (-30.00 pct)	   1.37 (37.00 pct)
> >     2	   2.64 (0.00 pct)	   1.39 (-47.34 pct)	   2.70 (2.27 pct)
> >     4	   4.89 (0.00 pct)	   2.75 (-43.76 pct)	   5.28 (7.97 pct)
> >     8	   9.46 (0.00 pct)	   5.42 (-42.70 pct)	  10.22 (8.03 pct)
> >    16	  19.05 (0.00 pct)	  10.42 (-45.30 pct)	  19.94 (4.67 pct)
> >    32	  37.50 (0.00 pct)	  20.23 (-46.05 pct)	  36.87 (-1.68 pct)
> >    64	  61.24 (0.00 pct)	  43.08 (-29.65 pct)	  62.96 (2.80 pct)
> >   128	  67.16 (0.00 pct)	  69.08 (2.85 pct)	  67.34 (0.26 pct)
> >   256	 154.59 (0.00 pct)	 162.33 (5.00 pct)	 156.78 (1.41 pct)
> >   512	 154.02 (0.00 pct)	 156.74 (1.76 pct)	 153.48 (-0.35 pct)
> > tbench power comparison:
> > Clients:   acpi-cpufreq		   amd_pst+passive	   amd_pst+guided
> >     1	   1.00 (0.00 pct)	   0.97 (-3.00 pct)	   1.08 (8.00 pct)
> >     2	   1.04 (0.00 pct)	   0.97 (-6.73 pct)	   1.11 (6.73 pct)
> >     4	   1.12 (0.00 pct)	   0.99 (-11.60 pct)	   1.18 (5.35 pct)
> >     8	   1.25 (0.00 pct)	   1.04 (-16.80 pct)	   1.31 (4.80 pct)
> >    16	   1.53 (0.00 pct)	   1.13 (-26.14 pct)	   1.58 (3.26 pct)
> >    32	   2.01 (0.00 pct)	   1.36 (-32.33 pct)	   2.03 (0.99 pct)
> >    64	   2.58 (0.00 pct)	   2.14 (-17.05 pct)	   2.61 (1.16 pct)
> >   128	   2.80 (0.00 pct)	   2.81 (0.35 pct)	   2.81 (0.35 pct)
> >   256	   3.39 (0.00 pct)	   3.43 (1.17 pct)	   3.42 (0.88 pct)
> >   512	   3.44 (0.00 pct)	   3.44 (0.00 pct)	   3.44 (0.00 pct)
> > 
> > Note: this series is based on top of EPP v11 [3] series
> 
> Hi Wyes,
> 
> Can you rebase this patchset onto the newest EPP v12 series [1] ?

Sure, I'll rebase this on top of EPP v12.

Thanks,
Wyes

> 
> [1] https://lore.kernel.org/linux-pm/20230131090016.3970625-1-perry.yuan@amd.com/
> 
> Cheers,
> Tor Vic
> 
> > 
> > Change log:
> > 
> > v3 -> v4:
> > - Fixed active mode low frequency issue reported by Peter Jung and Tor Vic
> > - Documentation modification suggested by Bagas Sanjaya
> > 
> > v2 -> v3:
> > - Addressed review comments form Mario.
> > - Picked up RB tag from Mario.
> > - Rebase on top of EPP v11 [3].
> > 
> > v1 -> v2:
> > - Fix issue with shared mem systems.
> > - Rebase on top of EPP series.
> > 
> > [1]: https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf
> > [2]: https://lore.kernel.org/lkml/20221110175847.3098728-1-Perry.Yuan@amd.com/
> > [3]: https://lore.kernel.org/linux-pm/20230118075210.447418-1-perry.yuan@amd.com/
> > 
> > Wyes Karny (6):
> >   acpi: cppc: Add min and max perf reg writing support
> >   acpi: cppc: Add auto select register read/write support
> >   cpufreq: amd_pstate: Add guided autonomous mode
> >   Documentation: amd_pstate: Move amd_pstate param to alphabetical order
> >   cpufreq: amd_pstate: Add guided mode control support via sysfs
> >   Documentation: amd_pstate: Update amd_pstate status sysfs for guided
> > 
> >  .../admin-guide/kernel-parameters.txt         |  41 ++--
> >  Documentation/admin-guide/pm/amd-pstate.rst   |  31 ++-
> >  drivers/acpi/cppc_acpi.c                      | 121 +++++++++++-
> >  drivers/cpufreq/amd-pstate.c                  | 184 +++++++++++++-----
> >  include/acpi/cppc_acpi.h                      |  11 ++
> >  include/linux/amd-pstate.h                    |   2 +
> >  6 files changed, 309 insertions(+), 81 deletions(-)
> > 
> > -- 
> > 2.34.1

Re: [PATCH v4 5/6] cpufreq: amd_pstate: Add guided mode control support via sysfs

[Russell Haley]

On 1/30/23 23:21, Wyes Karny wrote:

> Only if the state requested matches
> with the current state then -EBUSY value is returned.

This differs from the behavior of intel_pstate, where writing the
current state to the status file always succeeds.

Why not do the same thing here?

Re: [PATCH v4 5/6] cpufreq: amd_pstate: Add guided mode control support via sysfs

[Wyes Karny]

Hi Russell,

On 02 Feb 01:41, Russell Haley wrote:
> 
> 
> On 1/30/23 23:21, Wyes Karny wrote:
> 
> > Only if the state requested matches
> > with the current state then -EBUSY value is returned.
> 
> This differs from the behavior of intel_pstate, where writing the
> current state to the status file always succeeds.
> 
> Why not do the same thing here?

Make sense. Will change this to return 0.
Thanks for reviewing.

Thanks,
Wyes