* [PATCH v9 01/13] ACPI: CPPC: Add AMD pstate energy performance preference cppc control
2022-12-25 16:34 [PATCH v9 00/13] Implement AMD Pstate EPP Driver Perry Yuan
@ 2022-12-25 16:34 ` Perry Yuan
2023-01-05 5:31 ` Wyes Karny
2022-12-25 16:34 ` [PATCH v9 02/13] Documentation: amd-pstate: add EPP profiles introduction Perry Yuan
` (11 subsequent siblings)
12 siblings, 1 reply; 40+ messages in thread
From: Perry Yuan @ 2022-12-25 16:34 UTC (permalink / raw)
To: rafael.j.wysocki, Mario.Limonciello, ray.huang, viresh.kumar
Cc: Deepak.Sharma, Nathan.Fontenot, Alexander.Deucher, Shimmer.Huang,
Xiaojian.Du, Li.Meng, wyes.karny, linux-pm, linux-kernel
From: Perry Yuan <Perry.Yuan@amd.com>
Add support for setting and querying EPP preferences to the generic
CPPC driver. This enables downstream drivers such as amd-pstate to discover
and use these values.
Downstream drivers that want to use the new symbols cppc_get_epp_caps
and cppc_set_epp_perf for querying and setting EPP preferences will need
to call cppc_set_epp_perf to enable the EPP function firstly.
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
---
drivers/acpi/cppc_acpi.c | 67 ++++++++++++++++++++++++++++++++++++++++
include/acpi/cppc_acpi.h | 12 +++++++
2 files changed, 79 insertions(+)
diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 093675b1a1ff..0ce6c55a76ca 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -1153,6 +1153,19 @@ int cppc_get_nominal_perf(int cpunum, u64 *nominal_perf)
return cppc_get_perf(cpunum, NOMINAL_PERF, nominal_perf);
}
+/**
+ * cppc_get_epp_perf - Get the epp register value.
+ * @cpunum: CPU from which to get epp preference value.
+ * @epp_perf: Return address.
+ *
+ * Return: 0 for success, -EIO otherwise.
+ */
+int cppc_get_epp_perf(int cpunum, u64 *epp_perf)
+{
+ return cppc_get_perf(cpunum, ENERGY_PERF, epp_perf);
+}
+EXPORT_SYMBOL_GPL(cppc_get_epp_perf);
+
/**
* cppc_get_perf_caps - Get a CPU's performance capabilities.
* @cpunum: CPU from which to get capabilities info.
@@ -1365,6 +1378,60 @@ int cppc_get_perf_ctrs(int cpunum, struct cppc_perf_fb_ctrs *perf_fb_ctrs)
}
EXPORT_SYMBOL_GPL(cppc_get_perf_ctrs);
+/*
+ * Set Energy Performance Preference Register value through
+ * Performance Controls Interface
+ */
+int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable)
+{
+ int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpu);
+ struct cpc_register_resource *epp_set_reg;
+ 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;
+
+ 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];
+ epp_set_reg = &cpc_desc->cpc_regs[ENERGY_PERF];
+
+ if (CPC_IN_PCC(epp_set_reg) || CPC_IN_PCC(auto_sel_reg)) {
+ if (pcc_ss_id < 0) {
+ pr_debug("Invalid pcc_ss_id for CPU:%d\n", cpu);
+ return -ENODEV;
+ }
+
+ if (CPC_SUPPORTED(auto_sel_reg)) {
+ ret = cpc_write(cpu, auto_sel_reg, enable);
+ if (ret)
+ return ret;
+ }
+
+ if (CPC_SUPPORTED(epp_set_reg)) {
+ ret = cpc_write(cpu, epp_set_reg, perf_ctrls->energy_perf);
+ 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_epp_perf);
+
/**
* 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 c5614444031f..6b487a5bd638 100644
--- a/include/acpi/cppc_acpi.h
+++ b/include/acpi/cppc_acpi.h
@@ -108,12 +108,14 @@ struct cppc_perf_caps {
u32 lowest_nonlinear_perf;
u32 lowest_freq;
u32 nominal_freq;
+ u32 energy_perf;
};
struct cppc_perf_ctrls {
u32 max_perf;
u32 min_perf;
u32 desired_perf;
+ u32 energy_perf;
};
struct cppc_perf_fb_ctrs {
@@ -149,6 +151,8 @@ extern bool cpc_ffh_supported(void);
extern bool cpc_supported_by_cpu(void);
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);
#else /* !CONFIG_ACPI_CPPC_LIB */
static inline int cppc_get_desired_perf(int cpunum, u64 *desired_perf)
{
@@ -202,6 +206,14 @@ static inline int cpc_write_ffh(int cpunum, struct cpc_reg *reg, u64 val)
{
return -ENOTSUPP;
}
+static inline int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable)
+{
+ return -ENOTSUPP;
+}
+static inline int cppc_get_epp_perf(int cpunum, u64 *epp_perf)
+{
+ return -ENOTSUPP;
+}
#endif /* !CONFIG_ACPI_CPPC_LIB */
#endif /* _CPPC_ACPI_H*/
--
2.34.1
^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH v9 01/13] ACPI: CPPC: Add AMD pstate energy performance preference cppc control
2022-12-25 16:34 ` [PATCH v9 01/13] ACPI: CPPC: Add AMD pstate energy performance preference cppc control Perry Yuan
@ 2023-01-05 5:31 ` Wyes Karny
2023-01-05 6:02 ` Yuan, Perry
0 siblings, 1 reply; 40+ messages in thread
From: Wyes Karny @ 2023-01-05 5:31 UTC (permalink / raw)
To: Perry Yuan, rafael.j.wysocki, Mario.Limonciello, ray.huang, viresh.kumar
Cc: Deepak.Sharma, Nathan.Fontenot, Alexander.Deucher, Shimmer.Huang,
Xiaojian.Du, Li.Meng, linux-pm, linux-kernel
On 12/25/2022 10:04 PM, Perry Yuan wrote:
> From: Perry Yuan <Perry.Yuan@amd.com>
>
> Add support for setting and querying EPP preferences to the generic
> CPPC driver. This enables downstream drivers such as amd-pstate to discover
> and use these values.
>
> Downstream drivers that want to use the new symbols cppc_get_epp_caps
> and cppc_set_epp_perf for querying and setting EPP preferences will need
> to call cppc_set_epp_perf to enable the EPP function firstly.
>
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
Reviewed-by: Wyes Karny <wyes.karny@amd.com>
> ---
> drivers/acpi/cppc_acpi.c | 67 ++++++++++++++++++++++++++++++++++++++++
> include/acpi/cppc_acpi.h | 12 +++++++
> 2 files changed, 79 insertions(+)
>
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index 093675b1a1ff..0ce6c55a76ca 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -1153,6 +1153,19 @@ int cppc_get_nominal_perf(int cpunum, u64 *nominal_perf)
> return cppc_get_perf(cpunum, NOMINAL_PERF, nominal_perf);
> }
>
> +/**
> + * cppc_get_epp_perf - Get the epp register value.
> + * @cpunum: CPU from which to get epp preference value.
> + * @epp_perf: Return address.
> + *
> + * Return: 0 for success, -EIO otherwise.
> + */
> +int cppc_get_epp_perf(int cpunum, u64 *epp_perf)
> +{
> + return cppc_get_perf(cpunum, ENERGY_PERF, epp_perf);
> +}
> +EXPORT_SYMBOL_GPL(cppc_get_epp_perf);
> +
> /**
> * cppc_get_perf_caps - Get a CPU's performance capabilities.
> * @cpunum: CPU from which to get capabilities info.
> @@ -1365,6 +1378,60 @@ int cppc_get_perf_ctrs(int cpunum, struct cppc_perf_fb_ctrs *perf_fb_ctrs)
> }
> EXPORT_SYMBOL_GPL(cppc_get_perf_ctrs);
>
> +/*
> + * Set Energy Performance Preference Register value through
> + * Performance Controls Interface
> + */
> +int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable)
> +{
> + int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpu);
> + struct cpc_register_resource *epp_set_reg;
> + 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;
> +
> + 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];
> + epp_set_reg = &cpc_desc->cpc_regs[ENERGY_PERF];
> +
> + if (CPC_IN_PCC(epp_set_reg) || CPC_IN_PCC(auto_sel_reg)) {
> + if (pcc_ss_id < 0) {
> + pr_debug("Invalid pcc_ss_id for CPU:%d\n", cpu);
> + return -ENODEV;
> + }
> +
> + if (CPC_SUPPORTED(auto_sel_reg)) {
> + ret = cpc_write(cpu, auto_sel_reg, enable);
> + if (ret)
> + return ret;
> + }
> +
> + if (CPC_SUPPORTED(epp_set_reg)) {
> + ret = cpc_write(cpu, epp_set_reg, perf_ctrls->energy_perf);
> + 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_epp_perf);
> +
> /**
> * 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 c5614444031f..6b487a5bd638 100644
> --- a/include/acpi/cppc_acpi.h
> +++ b/include/acpi/cppc_acpi.h
> @@ -108,12 +108,14 @@ struct cppc_perf_caps {
> u32 lowest_nonlinear_perf;
> u32 lowest_freq;
> u32 nominal_freq;
> + u32 energy_perf;
> };
>
> struct cppc_perf_ctrls {
> u32 max_perf;
> u32 min_perf;
> u32 desired_perf;
> + u32 energy_perf;
> };
>
> struct cppc_perf_fb_ctrs {
> @@ -149,6 +151,8 @@ extern bool cpc_ffh_supported(void);
> extern bool cpc_supported_by_cpu(void);
> 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);
> #else /* !CONFIG_ACPI_CPPC_LIB */
> static inline int cppc_get_desired_perf(int cpunum, u64 *desired_perf)
> {
> @@ -202,6 +206,14 @@ static inline int cpc_write_ffh(int cpunum, struct cpc_reg *reg, u64 val)
> {
> return -ENOTSUPP;
> }
> +static inline int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable)
> +{
> + return -ENOTSUPP;
> +}
> +static inline int cppc_get_epp_perf(int cpunum, u64 *epp_perf)
> +{
> + return -ENOTSUPP;
> +}
> #endif /* !CONFIG_ACPI_CPPC_LIB */
>
> #endif /* _CPPC_ACPI_H*/
--
Thanks & Regards,
Wyes
^ permalink raw reply [flat|nested] 40+ messages in thread
* RE: [PATCH v9 01/13] ACPI: CPPC: Add AMD pstate energy performance preference cppc control
2023-01-05 5:31 ` Wyes Karny
@ 2023-01-05 6:02 ` Yuan, Perry
0 siblings, 0 replies; 40+ messages in thread
From: Yuan, Perry @ 2023-01-05 6:02 UTC (permalink / raw)
To: Karny, Wyes, rafael.j.wysocki, Limonciello, Mario, Huang, Ray,
viresh.kumar
Cc: Sharma, Deepak, Fontenot, Nathan, Deucher, Alexander, Huang,
Shimmer, Du, Xiaojian, Meng, Li (Jassmine),
linux-pm, linux-kernel
[AMD Official Use Only - General]
Hi Wyes.
> -----Original Message-----
> From: Karny, Wyes <Wyes.Karny@amd.com>
> Sent: Thursday, January 5, 2023 1:31 PM
> To: Yuan, Perry <Perry.Yuan@amd.com>; rafael.j.wysocki@intel.com;
> Limonciello, Mario <Mario.Limonciello@amd.com>; Huang, Ray
> <Ray.Huang@amd.com>; viresh.kumar@linaro.org
> Cc: Sharma, Deepak <Deepak.Sharma@amd.com>; Fontenot, Nathan
> <Nathan.Fontenot@amd.com>; Deucher, Alexander
> <Alexander.Deucher@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 v9 01/13] ACPI: CPPC: Add AMD pstate energy
> performance preference cppc control
>
>
>
> On 12/25/2022 10:04 PM, Perry Yuan wrote:
> > From: Perry Yuan <Perry.Yuan@amd.com>
> >
> > Add support for setting and querying EPP preferences to the generic
> > CPPC driver. This enables downstream drivers such as amd-pstate to
> > discover and use these values.
> >
> > Downstream drivers that want to use the new symbols
> cppc_get_epp_caps
> > and cppc_set_epp_perf for querying and setting EPP preferences will
> > need to call cppc_set_epp_perf to enable the EPP function firstly.
> >
> > Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> > Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
>
> Reviewed-by: Wyes Karny <wyes.karny@amd.com>
Thanks for your review, will pick up the tag in v10.
>
> > ---
> > drivers/acpi/cppc_acpi.c | 67
> > ++++++++++++++++++++++++++++++++++++++++
> > include/acpi/cppc_acpi.h | 12 +++++++
> > 2 files changed, 79 insertions(+)
> >
> > diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c index
> > 093675b1a1ff..0ce6c55a76ca 100644
> > --- a/drivers/acpi/cppc_acpi.c
> > +++ b/drivers/acpi/cppc_acpi.c
> > @@ -1153,6 +1153,19 @@ int cppc_get_nominal_perf(int cpunum, u64
> *nominal_perf)
> > return cppc_get_perf(cpunum, NOMINAL_PERF, nominal_perf); }
> >
> > +/**
> > + * cppc_get_epp_perf - Get the epp register value.
> > + * @cpunum: CPU from which to get epp preference value.
> > + * @epp_perf: Return address.
> > + *
> > + * Return: 0 for success, -EIO otherwise.
> > + */
> > +int cppc_get_epp_perf(int cpunum, u64 *epp_perf) {
> > + return cppc_get_perf(cpunum, ENERGY_PERF, epp_perf); }
> > +EXPORT_SYMBOL_GPL(cppc_get_epp_perf);
> > +
> > /**
> > * cppc_get_perf_caps - Get a CPU's performance capabilities.
> > * @cpunum: CPU from which to get capabilities info.
> > @@ -1365,6 +1378,60 @@ int cppc_get_perf_ctrs(int cpunum, struct
> > cppc_perf_fb_ctrs *perf_fb_ctrs) }
> > EXPORT_SYMBOL_GPL(cppc_get_perf_ctrs);
> >
> > +/*
> > + * Set Energy Performance Preference Register value through
> > + * Performance Controls Interface
> > + */
> > +int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls,
> > +bool enable) {
> > + int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpu);
> > + struct cpc_register_resource *epp_set_reg;
> > + 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;
> > +
> > + 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];
> > + epp_set_reg = &cpc_desc->cpc_regs[ENERGY_PERF];
> > +
> > + if (CPC_IN_PCC(epp_set_reg) || CPC_IN_PCC(auto_sel_reg)) {
> > + if (pcc_ss_id < 0) {
> > + pr_debug("Invalid pcc_ss_id for CPU:%d\n", cpu);
> > + return -ENODEV;
> > + }
> > +
> > + if (CPC_SUPPORTED(auto_sel_reg)) {
> > + ret = cpc_write(cpu, auto_sel_reg, enable);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + if (CPC_SUPPORTED(epp_set_reg)) {
> > + ret = cpc_write(cpu, epp_set_reg, perf_ctrls-
> >energy_perf);
> > + 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_epp_perf);
> > +
> > /**
> > * 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
> > c5614444031f..6b487a5bd638 100644
> > --- a/include/acpi/cppc_acpi.h
> > +++ b/include/acpi/cppc_acpi.h
> > @@ -108,12 +108,14 @@ struct cppc_perf_caps {
> > u32 lowest_nonlinear_perf;
> > u32 lowest_freq;
> > u32 nominal_freq;
> > + u32 energy_perf;
> > };
> >
> > struct cppc_perf_ctrls {
> > u32 max_perf;
> > u32 min_perf;
> > u32 desired_perf;
> > + u32 energy_perf;
> > };
> >
> > struct cppc_perf_fb_ctrs {
> > @@ -149,6 +151,8 @@ extern bool cpc_ffh_supported(void); extern bool
> > cpc_supported_by_cpu(void); 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);
> > #else /* !CONFIG_ACPI_CPPC_LIB */
> > static inline int cppc_get_desired_perf(int cpunum, u64
> > *desired_perf) { @@ -202,6 +206,14 @@ static inline int
> > cpc_write_ffh(int cpunum, struct cpc_reg *reg, u64 val) {
> > return -ENOTSUPP;
> > }
> > +static inline int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls
> > +*perf_ctrls, bool enable) {
> > + return -ENOTSUPP;
> > +}
> > +static inline int cppc_get_epp_perf(int cpunum, u64 *epp_perf) {
> > + return -ENOTSUPP;
> > +}
> > #endif /* !CONFIG_ACPI_CPPC_LIB */
> >
> > #endif /* _CPPC_ACPI_H*/
>
> --
> Thanks & Regards,
> Wyes
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v9 02/13] Documentation: amd-pstate: add EPP profiles introduction
2022-12-25 16:34 [PATCH v9 00/13] Implement AMD Pstate EPP Driver Perry Yuan
2022-12-25 16:34 ` [PATCH v9 01/13] ACPI: CPPC: Add AMD pstate energy performance preference cppc control Perry Yuan
@ 2022-12-25 16:34 ` Perry Yuan
2023-01-04 0:29 ` Limonciello, Mario
2022-12-25 16:34 ` [PATCH v9 03/13] cpufreq: intel_pstate: use common macro definition for Energy Preference Performance(EPP) Perry Yuan
` (10 subsequent siblings)
12 siblings, 1 reply; 40+ messages in thread
From: Perry Yuan @ 2022-12-25 16:34 UTC (permalink / raw)
To: rafael.j.wysocki, Mario.Limonciello, ray.huang, viresh.kumar
Cc: Deepak.Sharma, Nathan.Fontenot, Alexander.Deucher, Shimmer.Huang,
Xiaojian.Du, Li.Meng, wyes.karny, linux-pm, linux-kernel
From: Perry Yuan <Perry.Yuan@amd.com>
The amd-pstate driver supports a feature called energy performance
preference (EPP). Add information to the documentation to explain
how users can interact with the sysfs files for this feature.
1) See all EPP profiles
$ sudo cat /sys/devices/system/cpu/cpu0/cpufreq/energy_performance_available_preferences
default performance balance_performance balance_power power
2) Check current EPP profile
$ sudo cat /sys/devices/system/cpu/cpu0/cpufreq/energy_performance_preference
performance
3) Set new EPP profile
$ sudo bash -c "echo power > /sys/devices/system/cpu/cpu0/cpufreq/energy_performance_preference"
Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
---
Documentation/admin-guide/pm/amd-pstate.rst | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/Documentation/admin-guide/pm/amd-pstate.rst b/Documentation/admin-guide/pm/amd-pstate.rst
index 06e23538f79c..33ab8ec8fc2f 100644
--- a/Documentation/admin-guide/pm/amd-pstate.rst
+++ b/Documentation/admin-guide/pm/amd-pstate.rst
@@ -262,6 +262,25 @@ lowest non-linear performance in `AMD CPPC Performance Capability
<perf_cap_>`_.)
This attribute is read-only.
+``energy_performance_available_preferences``
+
+A list of all the supported EPP preferences that could be used for
+``energy_performance_preference`` on this system.
+These profiles represent different hints that are provided
+to the low-level firmware about the user's desired energy vs efficiency
+tradeoff. ``default`` represents the epp value is set by platform
+firmware. This attribute is read-only.
+
+``energy_performance_preference``
+
+The current energy performance preference can be read from this attribute.
+and user can change current preference according to energy or performance needs
+Please get all support profiles list from
+``energy_performance_available_preferences`` attribute, all the profiles are
+integer values defined between 0 to 255 when EPP feature is enabled by platform
+firmware, if EPP feature is disabled, driver will ignore the written value
+This attribute is read-write.
+
Other performance and frequency values can be read back from
``/sys/devices/system/cpu/cpuX/acpi_cppc/``, see :ref:`cppc_sysfs`.
--
2.34.1
^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH v9 02/13] Documentation: amd-pstate: add EPP profiles introduction
2022-12-25 16:34 ` [PATCH v9 02/13] Documentation: amd-pstate: add EPP profiles introduction Perry Yuan
@ 2023-01-04 0:29 ` Limonciello, Mario
2023-01-05 3:18 ` Yuan, Perry
0 siblings, 1 reply; 40+ messages in thread
From: Limonciello, Mario @ 2023-01-04 0:29 UTC (permalink / raw)
To: Perry Yuan, rafael.j.wysocki, ray.huang, viresh.kumar
Cc: Deepak.Sharma, Nathan.Fontenot, Alexander.Deucher, Shimmer.Huang,
Xiaojian.Du, Li.Meng, wyes.karny, linux-pm, linux-kernel
On 12/25/2022 10:34, Perry Yuan wrote:
> From: Perry Yuan <Perry.Yuan@amd.com>
>
> The amd-pstate driver supports a feature called energy performance
> preference (EPP). Add information to the documentation to explain
> how users can interact with the sysfs files for this feature.
>
> 1) See all EPP profiles
> $ sudo cat /sys/devices/system/cpu/cpu0/cpufreq/energy_performance_available_preferences
> default performance balance_performance balance_power power
>
> 2) Check current EPP profile
> $ sudo cat /sys/devices/system/cpu/cpu0/cpufreq/energy_performance_preference
> performance
>
> 3) Set new EPP profile
> $ sudo bash -c "echo power > /sys/devices/system/cpu/cpu0/cpufreq/energy_performance_preference"
>
> Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> Documentation/admin-guide/pm/amd-pstate.rst | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/Documentation/admin-guide/pm/amd-pstate.rst b/Documentation/admin-guide/pm/amd-pstate.rst
> index 06e23538f79c..33ab8ec8fc2f 100644
> --- a/Documentation/admin-guide/pm/amd-pstate.rst
> +++ b/Documentation/admin-guide/pm/amd-pstate.rst
> @@ -262,6 +262,25 @@ lowest non-linear performance in `AMD CPPC Performance Capability
> <perf_cap_>`_.)
> This attribute is read-only.
>
> +``energy_performance_available_preferences``
> +
> +A list of all the supported EPP preferences that could be used for
> +``energy_performance_preference`` on this system.
> +These profiles represent different hints that are provided
> +to the low-level firmware about the user's desired energy vs efficiency
> +tradeoff. ``default`` represents the epp value is set by platform
> +firmware. This attribute is read-only.
> +
> +``energy_performance_preference``
> +
> +The current energy performance preference can be read from this attribute.
> +and user can change current preference according to energy or performance needs
> +Please get all support profiles list from
> +``energy_performance_available_preferences`` attribute, all the profiles are
> +integer values defined between 0 to 255 when EPP feature is enabled by platform
> +firmware, if EPP feature is disabled, driver will ignore the written value
> +This attribute is read-write.
> +
> Other performance and frequency values can be read back from
> ``/sys/devices/system/cpu/cpuX/acpi_cppc/``, see :ref:`cppc_sysfs`.
>
^ permalink raw reply [flat|nested] 40+ messages in thread
* RE: [PATCH v9 02/13] Documentation: amd-pstate: add EPP profiles introduction
2023-01-04 0:29 ` Limonciello, Mario
@ 2023-01-05 3:18 ` Yuan, Perry
0 siblings, 0 replies; 40+ messages in thread
From: Yuan, Perry @ 2023-01-05 3:18 UTC (permalink / raw)
To: Limonciello, Mario, rafael.j.wysocki, Huang, Ray, viresh.kumar
Cc: Sharma, Deepak, Fontenot, Nathan, Deucher, Alexander, Huang,
Shimmer, Du, Xiaojian, Meng, Li (Jassmine),
Karny, Wyes, linux-pm, linux-kernel
[AMD Official Use Only - General]
Hi Mario.
> -----Original Message-----
> From: Limonciello, Mario <Mario.Limonciello@amd.com>
> Sent: Wednesday, January 4, 2023 8:29 AM
> To: Yuan, Perry <Perry.Yuan@amd.com>; rafael.j.wysocki@intel.com; Huang,
> Ray <Ray.Huang@amd.com>; viresh.kumar@linaro.org
> Cc: Sharma, Deepak <Deepak.Sharma@amd.com>; Fontenot, Nathan
> <Nathan.Fontenot@amd.com>; Deucher, Alexander
> <Alexander.Deucher@amd.com>; Huang, Shimmer
> <Shimmer.Huang@amd.com>; Du, Xiaojian <Xiaojian.Du@amd.com>; Meng,
> Li (Jassmine) <Li.Meng@amd.com>; Karny, Wyes <Wyes.Karny@amd.com>;
> linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v9 02/13] Documentation: amd-pstate: add EPP profiles
> introduction
>
> On 12/25/2022 10:34, Perry Yuan wrote:
> > From: Perry Yuan <Perry.Yuan@amd.com>
> >
> > The amd-pstate driver supports a feature called energy performance
> > preference (EPP). Add information to the documentation to explain how
> > users can interact with the sysfs files for this feature.
> >
> > 1) See all EPP profiles
> > $ sudo cat
> >
> /sys/devices/system/cpu/cpu0/cpufreq/energy_performance_available_pr
> ef
> > erences default performance balance_performance balance_power power
> >
> > 2) Check current EPP profile
> > $ sudo cat
> > /sys/devices/system/cpu/cpu0/cpufreq/energy_performance_preference
> > performance
> >
> > 3) Set new EPP profile
> > $ sudo bash -c "echo power >
> /sys/devices/system/cpu/cpu0/cpufreq/energy_performance_preference"
> >
> > Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
>
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
>
I just got back from sick leave, will pick all your R-B flags and send v10 out.
Thank you help to review this version.
Perry.
> > ---
> > Documentation/admin-guide/pm/amd-pstate.rst | 19
> +++++++++++++++++++
> > 1 file changed, 19 insertions(+)
> >
> > diff --git a/Documentation/admin-guide/pm/amd-pstate.rst
> > b/Documentation/admin-guide/pm/amd-pstate.rst
> > index 06e23538f79c..33ab8ec8fc2f 100644
> > --- a/Documentation/admin-guide/pm/amd-pstate.rst
> > +++ b/Documentation/admin-guide/pm/amd-pstate.rst
> > @@ -262,6 +262,25 @@ lowest non-linear performance in `AMD CPPC
> Performance Capability
> > <perf_cap_>`_.)
> > This attribute is read-only.
> >
> > +``energy_performance_available_preferences``
> > +
> > +A list of all the supported EPP preferences that could be used for
> > +``energy_performance_preference`` on this system.
> > +These profiles represent different hints that are provided to the
> > +low-level firmware about the user's desired energy vs efficiency
> > +tradeoff. ``default`` represents the epp value is set by platform
> > +firmware. This attribute is read-only.
> > +
> > +``energy_performance_preference``
> > +
> > +The current energy performance preference can be read from this
> attribute.
> > +and user can change current preference according to energy or
> > +performance needs Please get all support profiles list from
> > +``energy_performance_available_preferences`` attribute, all the
> > +profiles are integer values defined between 0 to 255 when EPP feature
> > +is enabled by platform firmware, if EPP feature is disabled, driver
> > +will ignore the written value This attribute is read-write.
> > +
> > Other performance and frequency values can be read back from
> > ``/sys/devices/system/cpu/cpuX/acpi_cppc/``, see :ref:`cppc_sysfs`.
> >
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v9 03/13] cpufreq: intel_pstate: use common macro definition for Energy Preference Performance(EPP)
2022-12-25 16:34 [PATCH v9 00/13] Implement AMD Pstate EPP Driver Perry Yuan
2022-12-25 16:34 ` [PATCH v9 01/13] ACPI: CPPC: Add AMD pstate energy performance preference cppc control Perry Yuan
2022-12-25 16:34 ` [PATCH v9 02/13] Documentation: amd-pstate: add EPP profiles introduction Perry Yuan
@ 2022-12-25 16:34 ` Perry Yuan
2023-01-04 0:31 ` Limonciello, Mario
2022-12-25 16:34 ` [PATCH v9 04/13] cpufreq: amd-pstate: fix kernel hang issue while amd-pstate unregistering Perry Yuan
` (9 subsequent siblings)
12 siblings, 1 reply; 40+ messages in thread
From: Perry Yuan @ 2022-12-25 16:34 UTC (permalink / raw)
To: rafael.j.wysocki, Mario.Limonciello, ray.huang, viresh.kumar
Cc: Deepak.Sharma, Nathan.Fontenot, Alexander.Deucher, Shimmer.Huang,
Xiaojian.Du, Li.Meng, wyes.karny, linux-pm, linux-kernel
make the energy preference performance strings and profiles using one
common header for intel_pstate driver, then the amd_pstate epp driver can
use the common header as well. This will simpify the intel_pstate and
amd_pstate driver.
Signed-off-by: Perry Yuan <perry.yuan@amd.com>
---
drivers/cpufreq/Kconfig.x86 | 2 +-
drivers/cpufreq/intel_pstate.c | 13 +++----------
include/linux/cpufreq.h | 10 ++++++++++
3 files changed, 14 insertions(+), 11 deletions(-)
diff --git a/drivers/cpufreq/Kconfig.x86 b/drivers/cpufreq/Kconfig.x86
index 00476e94db90..f64aef1e093d 100644
--- a/drivers/cpufreq/Kconfig.x86
+++ b/drivers/cpufreq/Kconfig.x86
@@ -36,7 +36,7 @@ config X86_PCC_CPUFREQ
config X86_AMD_PSTATE
bool "AMD Processor P-State driver"
- depends on X86 && ACPI
+ depends on X86 && ACPI && X86_INTEL_PSTATE
select ACPI_PROCESSOR
select ACPI_CPPC_LIB if X86_64
select CPU_FREQ_GOV_SCHEDUTIL if SMP
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index ad9be31753b6..93a60fdac0fc 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -640,15 +640,7 @@ static int intel_pstate_set_epb(int cpu, s16 pref)
* 4 power
*/
-enum energy_perf_value_index {
- EPP_INDEX_DEFAULT = 0,
- EPP_INDEX_PERFORMANCE,
- EPP_INDEX_BALANCE_PERFORMANCE,
- EPP_INDEX_BALANCE_POWERSAVE,
- EPP_INDEX_POWERSAVE,
-};
-
-static const char * const energy_perf_strings[] = {
+const char * const energy_perf_strings[] = {
[EPP_INDEX_DEFAULT] = "default",
[EPP_INDEX_PERFORMANCE] = "performance",
[EPP_INDEX_BALANCE_PERFORMANCE] = "balance_performance",
@@ -656,7 +648,8 @@ static const char * const energy_perf_strings[] = {
[EPP_INDEX_POWERSAVE] = "power",
NULL
};
-static unsigned int epp_values[] = {
+
+unsigned int epp_values[] = {
[EPP_INDEX_DEFAULT] = 0, /* Unused index */
[EPP_INDEX_PERFORMANCE] = HWP_EPP_PERFORMANCE,
[EPP_INDEX_BALANCE_PERFORMANCE] = HWP_EPP_BALANCE_PERFORMANCE,
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index d5595d57f4e5..0693269fb775 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -185,6 +185,16 @@ struct cpufreq_freqs {
u8 flags; /* flags of cpufreq_driver, see below. */
};
+enum energy_perf_value_index {
+ EPP_INDEX_DEFAULT = 0,
+ EPP_INDEX_PERFORMANCE,
+ EPP_INDEX_BALANCE_PERFORMANCE,
+ EPP_INDEX_BALANCE_POWERSAVE,
+ EPP_INDEX_POWERSAVE,
+};
+extern const char * const energy_perf_strings[];
+extern unsigned int epp_values[];
+
/* Only for ACPI */
#define CPUFREQ_SHARED_TYPE_NONE (0) /* None */
#define CPUFREQ_SHARED_TYPE_HW (1) /* HW does needed coordination */
--
2.34.1
^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH v9 03/13] cpufreq: intel_pstate: use common macro definition for Energy Preference Performance(EPP)
2022-12-25 16:34 ` [PATCH v9 03/13] cpufreq: intel_pstate: use common macro definition for Energy Preference Performance(EPP) Perry Yuan
@ 2023-01-04 0:31 ` Limonciello, Mario
2023-01-05 5:49 ` Yuan, Perry
0 siblings, 1 reply; 40+ messages in thread
From: Limonciello, Mario @ 2023-01-04 0:31 UTC (permalink / raw)
To: Perry Yuan, rafael.j.wysocki, ray.huang, viresh.kumar
Cc: Deepak.Sharma, Nathan.Fontenot, Alexander.Deucher, Shimmer.Huang,
Xiaojian.Du, Li.Meng, wyes.karny, linux-pm, linux-kernel
On 12/25/2022 10:34, Perry Yuan wrote:
> make the energy preference performance strings and profiles using one
> common header for intel_pstate driver, then the amd_pstate epp driver can
> use the common header as well. This will simpify the intel_pstate and
> amd_pstate driver.
>
> Signed-off-by: Perry Yuan <perry.yuan@amd.com>
> ---
> drivers/cpufreq/Kconfig.x86 | 2 +-
> drivers/cpufreq/intel_pstate.c | 13 +++----------
> include/linux/cpufreq.h | 10 ++++++++++
> 3 files changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/cpufreq/Kconfig.x86 b/drivers/cpufreq/Kconfig.x86
> index 00476e94db90..f64aef1e093d 100644
> --- a/drivers/cpufreq/Kconfig.x86
> +++ b/drivers/cpufreq/Kconfig.x86
> @@ -36,7 +36,7 @@ config X86_PCC_CPUFREQ
>
> config X86_AMD_PSTATE
> bool "AMD Processor P-State driver"
> - depends on X86 && ACPI
> + depends on X86 && ACPI && X86_INTEL_PSTATE
This doesn't seem right to me. What if someone didn't compile in Intel
x86 support for their kernel? They wouldn't be able to pick X86_AMD_PSTATE.
> select ACPI_PROCESSOR
> select ACPI_CPPC_LIB if X86_64
> select CPU_FREQ_GOV_SCHEDUTIL if SMP
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index ad9be31753b6..93a60fdac0fc 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -640,15 +640,7 @@ static int intel_pstate_set_epb(int cpu, s16 pref)
> * 4 power
> */
>
> -enum energy_perf_value_index {
> - EPP_INDEX_DEFAULT = 0,
> - EPP_INDEX_PERFORMANCE,
> - EPP_INDEX_BALANCE_PERFORMANCE,
> - EPP_INDEX_BALANCE_POWERSAVE,
> - EPP_INDEX_POWERSAVE,
> -};
> -
> -static const char * const energy_perf_strings[] = {
> +const char * const energy_perf_strings[] = {
> [EPP_INDEX_DEFAULT] = "default",
> [EPP_INDEX_PERFORMANCE] = "performance",
> [EPP_INDEX_BALANCE_PERFORMANCE] = "balance_performance",
> @@ -656,7 +648,8 @@ static const char * const energy_perf_strings[] = {
> [EPP_INDEX_POWERSAVE] = "power",
> NULL
> };
> -static unsigned int epp_values[] = {
> +
> +unsigned int epp_values[] = {
> [EPP_INDEX_DEFAULT] = 0, /* Unused index */
> [EPP_INDEX_PERFORMANCE] = HWP_EPP_PERFORMANCE,
> [EPP_INDEX_BALANCE_PERFORMANCE] = HWP_EPP_BALANCE_PERFORMANCE,
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index d5595d57f4e5..0693269fb775 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -185,6 +185,16 @@ struct cpufreq_freqs {
> u8 flags; /* flags of cpufreq_driver, see below. */
> };
>
> +enum energy_perf_value_index {
> + EPP_INDEX_DEFAULT = 0,
> + EPP_INDEX_PERFORMANCE,
> + EPP_INDEX_BALANCE_PERFORMANCE,
> + EPP_INDEX_BALANCE_POWERSAVE,
> + EPP_INDEX_POWERSAVE,
> +};
> +extern const char * const energy_perf_strings[];
> +extern unsigned int epp_values[];
> +
> /* Only for ACPI */
> #define CPUFREQ_SHARED_TYPE_NONE (0) /* None */
> #define CPUFREQ_SHARED_TYPE_HW (1) /* HW does needed coordination */
I think the right place for these variables and strings is in the cppc
library source file that is common across CPPC implementations.
^ permalink raw reply [flat|nested] 40+ messages in thread
* RE: [PATCH v9 03/13] cpufreq: intel_pstate: use common macro definition for Energy Preference Performance(EPP)
2023-01-04 0:31 ` Limonciello, Mario
@ 2023-01-05 5:49 ` Yuan, Perry
2023-01-05 5:56 ` Mario Limonciello
0 siblings, 1 reply; 40+ messages in thread
From: Yuan, Perry @ 2023-01-05 5:49 UTC (permalink / raw)
To: Limonciello, Mario, rafael.j.wysocki, Huang, Ray, viresh.kumar
Cc: Sharma, Deepak, Fontenot, Nathan, Deucher, Alexander, Huang,
Shimmer, Du, Xiaojian, Meng, Li (Jassmine),
Karny, Wyes, linux-pm, linux-kernel
[AMD Official Use Only - General]
Hi Mario
> -----Original Message-----
> From: Limonciello, Mario <Mario.Limonciello@amd.com>
> Sent: Wednesday, January 4, 2023 8:31 AM
> To: Yuan, Perry <Perry.Yuan@amd.com>; rafael.j.wysocki@intel.com; Huang,
> Ray <Ray.Huang@amd.com>; viresh.kumar@linaro.org
> Cc: Sharma, Deepak <Deepak.Sharma@amd.com>; Fontenot, Nathan
> <Nathan.Fontenot@amd.com>; Deucher, Alexander
> <Alexander.Deucher@amd.com>; Huang, Shimmer
> <Shimmer.Huang@amd.com>; Du, Xiaojian <Xiaojian.Du@amd.com>; Meng,
> Li (Jassmine) <Li.Meng@amd.com>; Karny, Wyes <Wyes.Karny@amd.com>;
> linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v9 03/13] cpufreq: intel_pstate: use common macro
> definition for Energy Preference Performance(EPP)
>
> On 12/25/2022 10:34, Perry Yuan wrote:
> > make the energy preference performance strings and profiles using one
> > common header for intel_pstate driver, then the amd_pstate epp driver
> > can use the common header as well. This will simpify the intel_pstate
> > and amd_pstate driver.
> >
> > Signed-off-by: Perry Yuan <perry.yuan@amd.com>
> > ---
> > drivers/cpufreq/Kconfig.x86 | 2 +-
> > drivers/cpufreq/intel_pstate.c | 13 +++----------
> > include/linux/cpufreq.h | 10 ++++++++++
> > 3 files changed, 14 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/cpufreq/Kconfig.x86 b/drivers/cpufreq/Kconfig.x86
> > index 00476e94db90..f64aef1e093d 100644
> > --- a/drivers/cpufreq/Kconfig.x86
> > +++ b/drivers/cpufreq/Kconfig.x86
> > @@ -36,7 +36,7 @@ config X86_PCC_CPUFREQ
> >
> > config X86_AMD_PSTATE
> > bool "AMD Processor P-State driver"
> > - depends on X86 && ACPI
> > + depends on X86 && ACPI && X86_INTEL_PSTATE
>
> This doesn't seem right to me. What if someone didn't compile in Intel
> x86 support for their kernel? They wouldn't be able to pick
> X86_AMD_PSTATE.
How about changed like this ? when amd pstate enabled, it will build intel-pstate.c as well.
depends on X86 && ACPI
+ select X86_INTEL_PSTATE
>
> > select ACPI_PROCESSOR
> > select ACPI_CPPC_LIB if X86_64
> > select CPU_FREQ_GOV_SCHEDUTIL if SMP diff --git
> > a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> > index ad9be31753b6..93a60fdac0fc 100644
> > --- a/drivers/cpufreq/intel_pstate.c
> > +++ b/drivers/cpufreq/intel_pstate.c
> > @@ -640,15 +640,7 @@ static int intel_pstate_set_epb(int cpu, s16 pref)
> > * 4 power
> > */
> >
> > -enum energy_perf_value_index {
> > - EPP_INDEX_DEFAULT = 0,
> > - EPP_INDEX_PERFORMANCE,
> > - EPP_INDEX_BALANCE_PERFORMANCE,
> > - EPP_INDEX_BALANCE_POWERSAVE,
> > - EPP_INDEX_POWERSAVE,
> > -};
> > -
> > -static const char * const energy_perf_strings[] = {
> > +const char * const energy_perf_strings[] = {
> > [EPP_INDEX_DEFAULT] = "default",
> > [EPP_INDEX_PERFORMANCE] = "performance",
> > [EPP_INDEX_BALANCE_PERFORMANCE] = "balance_performance",
> @@ -656,7
> > +648,8 @@ static const char * const energy_perf_strings[] = {
> > [EPP_INDEX_POWERSAVE] = "power",
> > NULL
> > };
> > -static unsigned int epp_values[] = {
> > +
> > +unsigned int epp_values[] = {
> > [EPP_INDEX_DEFAULT] = 0, /* Unused index */
> > [EPP_INDEX_PERFORMANCE] = HWP_EPP_PERFORMANCE,
> > [EPP_INDEX_BALANCE_PERFORMANCE] =
> HWP_EPP_BALANCE_PERFORMANCE, diff
> > --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index
> > d5595d57f4e5..0693269fb775 100644
> > --- a/include/linux/cpufreq.h
> > +++ b/include/linux/cpufreq.h
> > @@ -185,6 +185,16 @@ struct cpufreq_freqs {
> > u8 flags; /* flags of cpufreq_driver, see below. */
> > };
> >
> > +enum energy_perf_value_index {
> > + EPP_INDEX_DEFAULT = 0,
> > + EPP_INDEX_PERFORMANCE,
> > + EPP_INDEX_BALANCE_PERFORMANCE,
> > + EPP_INDEX_BALANCE_POWERSAVE,
> > + EPP_INDEX_POWERSAVE,
> > +};
> > +extern const char * const energy_perf_strings[]; extern unsigned int
> > +epp_values[];
> > +
> > /* Only for ACPI */
> > #define CPUFREQ_SHARED_TYPE_NONE (0) /* None */
> > #define CPUFREQ_SHARED_TYPE_HW (1) /* HW does needed
> coordination */
>
> I think the right place for these variables and strings is in the cppc library
> source file that is common across CPPC implementations.
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v9 03/13] cpufreq: intel_pstate: use common macro definition for Energy Preference Performance(EPP)
2023-01-05 5:49 ` Yuan, Perry
@ 2023-01-05 5:56 ` Mario Limonciello
2023-01-05 6:14 ` Yuan, Perry
0 siblings, 1 reply; 40+ messages in thread
From: Mario Limonciello @ 2023-01-05 5:56 UTC (permalink / raw)
To: Yuan, Perry, rafael.j.wysocki, Huang, Ray, viresh.kumar
Cc: Sharma, Deepak, Fontenot, Nathan, Deucher, Alexander, Huang,
Shimmer, Du, Xiaojian, Meng, Li (Jassmine),
Karny, Wyes, linux-pm, linux-kernel
On 1/4/23 23:49, Yuan, Perry wrote:
> [AMD Official Use Only - General]
>
> Hi Mario
>
>> -----Original Message-----
>> From: Limonciello, Mario <Mario.Limonciello@amd.com>
>> Sent: Wednesday, January 4, 2023 8:31 AM
>> To: Yuan, Perry <Perry.Yuan@amd.com>; rafael.j.wysocki@intel.com; Huang,
>> Ray <Ray.Huang@amd.com>; viresh.kumar@linaro.org
>> Cc: Sharma, Deepak <Deepak.Sharma@amd.com>; Fontenot, Nathan
>> <Nathan.Fontenot@amd.com>; Deucher, Alexander
>> <Alexander.Deucher@amd.com>; Huang, Shimmer
>> <Shimmer.Huang@amd.com>; Du, Xiaojian <Xiaojian.Du@amd.com>; Meng,
>> Li (Jassmine) <Li.Meng@amd.com>; Karny, Wyes <Wyes.Karny@amd.com>;
>> linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org
>> Subject: Re: [PATCH v9 03/13] cpufreq: intel_pstate: use common macro
>> definition for Energy Preference Performance(EPP)
>>
>> On 12/25/2022 10:34, Perry Yuan wrote:
>>> make the energy preference performance strings and profiles using one
>>> common header for intel_pstate driver, then the amd_pstate epp driver
>>> can use the common header as well. This will simpify the intel_pstate
>>> and amd_pstate driver.
>>>
>>> Signed-off-by: Perry Yuan <perry.yuan@amd.com>
>>> ---
>>> drivers/cpufreq/Kconfig.x86 | 2 +-
>>> drivers/cpufreq/intel_pstate.c | 13 +++----------
>>> include/linux/cpufreq.h | 10 ++++++++++
>>> 3 files changed, 14 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/drivers/cpufreq/Kconfig.x86 b/drivers/cpufreq/Kconfig.x86
>>> index 00476e94db90..f64aef1e093d 100644
>>> --- a/drivers/cpufreq/Kconfig.x86
>>> +++ b/drivers/cpufreq/Kconfig.x86
>>> @@ -36,7 +36,7 @@ config X86_PCC_CPUFREQ
>>>
>>> config X86_AMD_PSTATE
>>> bool "AMD Processor P-State driver"
>>> - depends on X86 && ACPI
>>> + depends on X86 && ACPI && X86_INTEL_PSTATE
>>
>> This doesn't seem right to me. What if someone didn't compile in Intel
>> x86 support for their kernel? They wouldn't be able to pick
>> X86_AMD_PSTATE.
>
> How about changed like this ? when amd pstate enabled, it will build intel-pstate.c as well.
>
> depends on X86 && ACPI
> + select X86_INTEL_PSTATE
I still don't think that's the right way to do this. Why not move the
variables to the cppc library file? Both intel and amd pstate drivers
select it, so it can be a common place both wil use.
>
>
>
>>
>>> select ACPI_PROCESSOR
>>> select ACPI_CPPC_LIB if X86_64
>>> select CPU_FREQ_GOV_SCHEDUTIL if SMP diff --git
>>> a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
>>> index ad9be31753b6..93a60fdac0fc 100644
>>> --- a/drivers/cpufreq/intel_pstate.c
>>> +++ b/drivers/cpufreq/intel_pstate.c
>>> @@ -640,15 +640,7 @@ static int intel_pstate_set_epb(int cpu, s16 pref)
>>> * 4 power
>>> */
>>>
>>> -enum energy_perf_value_index {
>>> - EPP_INDEX_DEFAULT = 0,
>>> - EPP_INDEX_PERFORMANCE,
>>> - EPP_INDEX_BALANCE_PERFORMANCE,
>>> - EPP_INDEX_BALANCE_POWERSAVE,
>>> - EPP_INDEX_POWERSAVE,
>>> -};
>>> -
>>> -static const char * const energy_perf_strings[] = {
>>> +const char * const energy_perf_strings[] = {
>>> [EPP_INDEX_DEFAULT] = "default",
>>> [EPP_INDEX_PERFORMANCE] = "performance",
>>> [EPP_INDEX_BALANCE_PERFORMANCE] = "balance_performance",
>> @@ -656,7
>>> +648,8 @@ static const char * const energy_perf_strings[] = {
>>> [EPP_INDEX_POWERSAVE] = "power",
>>> NULL
>>> };
>>> -static unsigned int epp_values[] = {
>>> +
>>> +unsigned int epp_values[] = {
>>> [EPP_INDEX_DEFAULT] = 0, /* Unused index */
>>> [EPP_INDEX_PERFORMANCE] = HWP_EPP_PERFORMANCE,
>>> [EPP_INDEX_BALANCE_PERFORMANCE] =
>> HWP_EPP_BALANCE_PERFORMANCE, diff
>>> --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index
>>> d5595d57f4e5..0693269fb775 100644
>>> --- a/include/linux/cpufreq.h
>>> +++ b/include/linux/cpufreq.h
>>> @@ -185,6 +185,16 @@ struct cpufreq_freqs {
>>> u8 flags; /* flags of cpufreq_driver, see below. */
>>> };
>>>
>>> +enum energy_perf_value_index {
>>> + EPP_INDEX_DEFAULT = 0,
>>> + EPP_INDEX_PERFORMANCE,
>>> + EPP_INDEX_BALANCE_PERFORMANCE,
>>> + EPP_INDEX_BALANCE_POWERSAVE,
>>> + EPP_INDEX_POWERSAVE,
>>> +};
>>> +extern const char * const energy_perf_strings[]; extern unsigned int
>>> +epp_values[];
>>> +
>>> /* Only for ACPI */
>>> #define CPUFREQ_SHARED_TYPE_NONE (0) /* None */
>>> #define CPUFREQ_SHARED_TYPE_HW (1) /* HW does needed
>> coordination */
>>
>> I think the right place for these variables and strings is in the cppc library
>> source file that is common across CPPC implementations.
^ permalink raw reply [flat|nested] 40+ messages in thread
* RE: [PATCH v9 03/13] cpufreq: intel_pstate: use common macro definition for Energy Preference Performance(EPP)
2023-01-05 5:56 ` Mario Limonciello
@ 2023-01-05 6:14 ` Yuan, Perry
0 siblings, 0 replies; 40+ messages in thread
From: Yuan, Perry @ 2023-01-05 6:14 UTC (permalink / raw)
To: Limonciello, Mario, rafael.j.wysocki, Huang, Ray, viresh.kumar
Cc: Sharma, Deepak, Fontenot, Nathan, Deucher, Alexander, Huang,
Shimmer, Du, Xiaojian, Meng, Li (Jassmine),
Karny, Wyes, linux-pm, linux-kernel
[AMD Official Use Only - General]
Hi Mario.
> -----Original Message-----
> From: Limonciello, Mario <Mario.Limonciello@amd.com>
> Sent: Thursday, January 5, 2023 1:56 PM
> To: Yuan, Perry <Perry.Yuan@amd.com>; rafael.j.wysocki@intel.com; Huang,
> Ray <Ray.Huang@amd.com>; viresh.kumar@linaro.org
> Cc: Sharma, Deepak <Deepak.Sharma@amd.com>; Fontenot, Nathan
> <Nathan.Fontenot@amd.com>; Deucher, Alexander
> <Alexander.Deucher@amd.com>; Huang, Shimmer
> <Shimmer.Huang@amd.com>; Du, Xiaojian <Xiaojian.Du@amd.com>; Meng,
> Li (Jassmine) <Li.Meng@amd.com>; Karny, Wyes <Wyes.Karny@amd.com>;
> linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v9 03/13] cpufreq: intel_pstate: use common macro
> definition for Energy Preference Performance(EPP)
>
> On 1/4/23 23:49, Yuan, Perry wrote:
> > [AMD Official Use Only - General]
> >
> > Hi Mario
> >
> >> -----Original Message-----
> >> From: Limonciello, Mario <Mario.Limonciello@amd.com>
> >> Sent: Wednesday, January 4, 2023 8:31 AM
> >> To: Yuan, Perry <Perry.Yuan@amd.com>; rafael.j.wysocki@intel.com;
> >> Huang, Ray <Ray.Huang@amd.com>; viresh.kumar@linaro.org
> >> Cc: Sharma, Deepak <Deepak.Sharma@amd.com>; Fontenot, Nathan
> >> <Nathan.Fontenot@amd.com>; Deucher, Alexander
> >> <Alexander.Deucher@amd.com>; Huang, Shimmer
> <Shimmer.Huang@amd.com>;
> >> Du, Xiaojian <Xiaojian.Du@amd.com>; Meng, Li (Jassmine)
> >> <Li.Meng@amd.com>; Karny, Wyes <Wyes.Karny@amd.com>;
> >> linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org
> >> Subject: Re: [PATCH v9 03/13] cpufreq: intel_pstate: use common macro
> >> definition for Energy Preference Performance(EPP)
> >>
> >> On 12/25/2022 10:34, Perry Yuan wrote:
> >>> make the energy preference performance strings and profiles using
> >>> one common header for intel_pstate driver, then the amd_pstate epp
> >>> driver can use the common header as well. This will simpify the
> >>> intel_pstate and amd_pstate driver.
> >>>
> >>> Signed-off-by: Perry Yuan <perry.yuan@amd.com>
> >>> ---
> >>> drivers/cpufreq/Kconfig.x86 | 2 +-
> >>> drivers/cpufreq/intel_pstate.c | 13 +++----------
> >>> include/linux/cpufreq.h | 10 ++++++++++
> >>> 3 files changed, 14 insertions(+), 11 deletions(-)
> >>>
> >>> diff --git a/drivers/cpufreq/Kconfig.x86
> >>> b/drivers/cpufreq/Kconfig.x86 index 00476e94db90..f64aef1e093d
> >>> 100644
> >>> --- a/drivers/cpufreq/Kconfig.x86
> >>> +++ b/drivers/cpufreq/Kconfig.x86
> >>> @@ -36,7 +36,7 @@ config X86_PCC_CPUFREQ
> >>>
> >>> config X86_AMD_PSTATE
> >>> bool "AMD Processor P-State driver"
> >>> - depends on X86 && ACPI
> >>> + depends on X86 && ACPI && X86_INTEL_PSTATE
> >>
> >> This doesn't seem right to me. What if someone didn't compile in
> >> Intel
> >> x86 support for their kernel? They wouldn't be able to pick
> >> X86_AMD_PSTATE.
> >
> > How about changed like this ? when amd pstate enabled, it will build intel-
> pstate.c as well.
> >
> > depends on X86 && ACPI
> > + select X86_INTEL_PSTATE
>
> I still don't think that's the right way to do this. Why not move the variables
> to the cppc library file? Both intel and amd pstate drivers select it, so it can
> be a common place both wil use.
>
That will be good then.
Ray suggested to put the code to cpufreq.h , I will move the code to cppc library in v10 if he has no objection.
Perry.
>
> >
> >
> >
> >>
> >>> select ACPI_PROCESSOR
> >>> select ACPI_CPPC_LIB if X86_64
> >>> select CPU_FREQ_GOV_SCHEDUTIL if SMP diff --git
> >>> a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> >>> index ad9be31753b6..93a60fdac0fc 100644
> >>> --- a/drivers/cpufreq/intel_pstate.c
> >>> +++ b/drivers/cpufreq/intel_pstate.c
> >>> @@ -640,15 +640,7 @@ static int intel_pstate_set_epb(int cpu, s16 pref)
> >>> * 4 power
> >>> */
> >>>
> >>> -enum energy_perf_value_index {
> >>> - EPP_INDEX_DEFAULT = 0,
> >>> - EPP_INDEX_PERFORMANCE,
> >>> - EPP_INDEX_BALANCE_PERFORMANCE,
> >>> - EPP_INDEX_BALANCE_POWERSAVE,
> >>> - EPP_INDEX_POWERSAVE,
> >>> -};
> >>> -
> >>> -static const char * const energy_perf_strings[] = {
> >>> +const char * const energy_perf_strings[] = {
> >>> [EPP_INDEX_DEFAULT] = "default",
> >>> [EPP_INDEX_PERFORMANCE] = "performance",
> >>> [EPP_INDEX_BALANCE_PERFORMANCE] = "balance_performance",
> >> @@ -656,7
> >>> +648,8 @@ static const char * const energy_perf_strings[] = {
> >>> [EPP_INDEX_POWERSAVE] = "power",
> >>> NULL
> >>> };
> >>> -static unsigned int epp_values[] = {
> >>> +
> >>> +unsigned int epp_values[] = {
> >>> [EPP_INDEX_DEFAULT] = 0, /* Unused index */
> >>> [EPP_INDEX_PERFORMANCE] = HWP_EPP_PERFORMANCE,
> >>> [EPP_INDEX_BALANCE_PERFORMANCE] =
> >> HWP_EPP_BALANCE_PERFORMANCE, diff
> >>> --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index
> >>> d5595d57f4e5..0693269fb775 100644
> >>> --- a/include/linux/cpufreq.h
> >>> +++ b/include/linux/cpufreq.h
> >>> @@ -185,6 +185,16 @@ struct cpufreq_freqs {
> >>> u8 flags; /* flags of cpufreq_driver, see below. */
> >>> };
> >>>
> >>> +enum energy_perf_value_index {
> >>> + EPP_INDEX_DEFAULT = 0,
> >>> + EPP_INDEX_PERFORMANCE,
> >>> + EPP_INDEX_BALANCE_PERFORMANCE,
> >>> + EPP_INDEX_BALANCE_POWERSAVE,
> >>> + EPP_INDEX_POWERSAVE,
> >>> +};
> >>> +extern const char * const energy_perf_strings[]; extern unsigned
> >>> +int epp_values[];
> >>> +
> >>> /* Only for ACPI */
> >>> #define CPUFREQ_SHARED_TYPE_NONE (0) /* None */
> >>> #define CPUFREQ_SHARED_TYPE_HW (1) /* HW does needed
> >> coordination */
> >>
> >> I think the right place for these variables and strings is in the
> >> cppc library source file that is common across CPPC implementations.
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v9 04/13] cpufreq: amd-pstate: fix kernel hang issue while amd-pstate unregistering
2022-12-25 16:34 [PATCH v9 00/13] Implement AMD Pstate EPP Driver Perry Yuan
` (2 preceding siblings ...)
2022-12-25 16:34 ` [PATCH v9 03/13] cpufreq: intel_pstate: use common macro definition for Energy Preference Performance(EPP) Perry Yuan
@ 2022-12-25 16:34 ` Perry Yuan
2022-12-27 2:53 ` Viresh Kumar
2022-12-25 16:34 ` [PATCH v9 05/13] cpufreq: amd-pstate: optimize driver working mode selection in amd_pstate_param() Perry Yuan
` (8 subsequent siblings)
12 siblings, 1 reply; 40+ messages in thread
From: Perry Yuan @ 2022-12-25 16:34 UTC (permalink / raw)
To: rafael.j.wysocki, Mario.Limonciello, ray.huang, viresh.kumar
Cc: Deepak.Sharma, Nathan.Fontenot, Alexander.Deucher, Shimmer.Huang,
Xiaojian.Du, Li.Meng, wyes.karny, linux-pm, linux-kernel
In the amd_pstate_adjust_perf(), there is one cpufreq_cpu_get() call to
increase increments the kobject reference count of policy and make it as
busy. Therefore, a corresponding call to cpufreq_cpu_put() is needed to
decrement the kobject reference count back, it will resolve the kernel
hang issue when unregistering the amd-pstate driver and register the
`amd_pstate_epp` driver instance.
Acked-by: Huang Rui <ray.huang@amd.com>
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Perry Yuan <perry.yuan@amd.com>
Cc: stable@vger.kernel.org
---
drivers/cpufreq/amd-pstate.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 204e39006dda..c17bd845f5fc 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -307,6 +307,7 @@ static void amd_pstate_adjust_perf(unsigned int cpu,
max_perf = min_perf;
amd_pstate_update(cpudata, min_perf, des_perf, max_perf, true);
+ cpufreq_cpu_put(policy);
}
static int amd_get_min_freq(struct amd_cpudata *cpudata)
--
2.34.1
^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH v9 04/13] cpufreq: amd-pstate: fix kernel hang issue while amd-pstate unregistering
2022-12-25 16:34 ` [PATCH v9 04/13] cpufreq: amd-pstate: fix kernel hang issue while amd-pstate unregistering Perry Yuan
@ 2022-12-27 2:53 ` Viresh Kumar
2022-12-27 6:32 ` Yuan, Perry
0 siblings, 1 reply; 40+ messages in thread
From: Viresh Kumar @ 2022-12-27 2:53 UTC (permalink / raw)
To: Perry Yuan
Cc: rafael.j.wysocki, Mario.Limonciello, ray.huang, Deepak.Sharma,
Nathan.Fontenot, Alexander.Deucher, Shimmer.Huang, Xiaojian.Du,
Li.Meng, wyes.karny, linux-pm, linux-kernel
On 26-12-22, 00:34, Perry Yuan wrote:
> In the amd_pstate_adjust_perf(), there is one cpufreq_cpu_get() call to
> increase increments the kobject reference count of policy and make it as
> busy. Therefore, a corresponding call to cpufreq_cpu_put() is needed to
> decrement the kobject reference count back, it will resolve the kernel
> hang issue when unregistering the amd-pstate driver and register the
> `amd_pstate_epp` driver instance.
>
> Acked-by: Huang Rui <ray.huang@amd.com>
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> Signed-off-by: Perry Yuan <perry.yuan@amd.com>
> Cc: stable@vger.kernel.org
> ---
> drivers/cpufreq/amd-pstate.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 204e39006dda..c17bd845f5fc 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -307,6 +307,7 @@ static void amd_pstate_adjust_perf(unsigned int cpu,
> max_perf = min_perf;
>
> amd_pstate_update(cpudata, min_perf, des_perf, max_perf, true);
> + cpufreq_cpu_put(policy);
> }
>
> static int amd_get_min_freq(struct amd_cpudata *cpudata)
This should have been sent separately and not in this series. It is a
bug fix, which could have been merged in the 6.1 itself and now is
candidate for 6.2-rc1, while the rest of the series needs to wait for
6.3.
--
viresh
^ permalink raw reply [flat|nested] 40+ messages in thread
* RE: [PATCH v9 04/13] cpufreq: amd-pstate: fix kernel hang issue while amd-pstate unregistering
2022-12-27 2:53 ` Viresh Kumar
@ 2022-12-27 6:32 ` Yuan, Perry
0 siblings, 0 replies; 40+ messages in thread
From: Yuan, Perry @ 2022-12-27 6:32 UTC (permalink / raw)
To: Viresh Kumar
Cc: rafael.j.wysocki, Limonciello, Mario, Huang, Ray, Sharma, Deepak,
Fontenot, Nathan, Deucher, Alexander, Huang, Shimmer, Du,
Xiaojian, Meng, Li (Jassmine),
Karny, Wyes, linux-pm, linux-kernel
[AMD Official Use Only - General]
Hi Viresh.
> -----Original Message-----
> From: Viresh Kumar <viresh.kumar@linaro.org>
> Sent: Tuesday, December 27, 2022 10:54 AM
> To: Yuan, Perry <Perry.Yuan@amd.com>
> Cc: rafael.j.wysocki@intel.com; Limonciello, Mario
> <Mario.Limonciello@amd.com>; Huang, Ray <Ray.Huang@amd.com>;
> Sharma, Deepak <Deepak.Sharma@amd.com>; Fontenot, Nathan
> <Nathan.Fontenot@amd.com>; Deucher, Alexander
> <Alexander.Deucher@amd.com>; Huang, Shimmer
> <Shimmer.Huang@amd.com>; Du, Xiaojian <Xiaojian.Du@amd.com>; Meng,
> Li (Jassmine) <Li.Meng@amd.com>; Karny, Wyes <Wyes.Karny@amd.com>;
> linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v9 04/13] cpufreq: amd-pstate: fix kernel hang issue
> while amd-pstate unregistering
>
> On 26-12-22, 00:34, Perry Yuan wrote:
> > In the amd_pstate_adjust_perf(), there is one cpufreq_cpu_get() call
> > to increase increments the kobject reference count of policy and make
> > it as busy. Therefore, a corresponding call to cpufreq_cpu_put() is
> > needed to decrement the kobject reference count back, it will resolve
> > the kernel hang issue when unregistering the amd-pstate driver and
> > register the `amd_pstate_epp` driver instance.
> >
> > Acked-by: Huang Rui <ray.huang@amd.com>
> > Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> > Signed-off-by: Perry Yuan <perry.yuan@amd.com>
> > Cc: stable@vger.kernel.org
> > ---
> > drivers/cpufreq/amd-pstate.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/cpufreq/amd-pstate.c
> > b/drivers/cpufreq/amd-pstate.c index 204e39006dda..c17bd845f5fc 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -307,6 +307,7 @@ static void amd_pstate_adjust_perf(unsigned int
> cpu,
> > max_perf = min_perf;
> >
> > amd_pstate_update(cpudata, min_perf, des_perf, max_perf, true);
> > + cpufreq_cpu_put(policy);
> > }
> >
> > static int amd_get_min_freq(struct amd_cpudata *cpudata)
>
> This should have been sent separately and not in this series. It is a bug fix,
> which could have been merged in the 6.1 itself and now is candidate for 6.2-
> rc1, while the rest of the series needs to wait for 6.3.
>
> --
> Viresh
Thanks for your feedback, will extract the patch and send it out separately.
Perry.
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v9 05/13] cpufreq: amd-pstate: optimize driver working mode selection in amd_pstate_param()
2022-12-25 16:34 [PATCH v9 00/13] Implement AMD Pstate EPP Driver Perry Yuan
` (3 preceding siblings ...)
2022-12-25 16:34 ` [PATCH v9 04/13] cpufreq: amd-pstate: fix kernel hang issue while amd-pstate unregistering Perry Yuan
@ 2022-12-25 16:34 ` Perry Yuan
2023-01-04 0:32 ` Limonciello, Mario
2022-12-25 16:34 ` [PATCH v9 06/13] cpufreq: amd-pstate: implement Pstate EPP support for the AMD processors Perry Yuan
` (7 subsequent siblings)
12 siblings, 1 reply; 40+ messages in thread
From: Perry Yuan @ 2022-12-25 16:34 UTC (permalink / raw)
To: rafael.j.wysocki, Mario.Limonciello, ray.huang, viresh.kumar
Cc: Deepak.Sharma, Nathan.Fontenot, Alexander.Deucher, Shimmer.Huang,
Xiaojian.Du, Li.Meng, wyes.karny, linux-pm, linux-kernel
From: Wyes Karny <wyes.karny@amd.com>
The amd-pstate driver may support multiple working modes.
Introduce a variable to keep track of which mode is currently enabled.
Here we use cppc_state var to indicate which mode is enabled.
This change will help to simplify the the amd_pstate_param() to choose
which mode used for the following driver registration.
Signed-off-by: Perry Yuan <perry.yuan@amd.com>
Signed-off-by: Wyes Karny <wyes.karny@amd.com>
---
drivers/cpufreq/amd-pstate.c | 39 +++++++++++++++++++++++++++---------
include/linux/amd-pstate.h | 17 ++++++++++++++++
2 files changed, 46 insertions(+), 10 deletions(-)
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index c17bd845f5fc..65c16edbbb20 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -60,7 +60,18 @@
* module parameter to be able to enable it manually for debugging.
*/
static struct cpufreq_driver amd_pstate_driver;
-static int cppc_load __initdata;
+static int cppc_state = AMD_PSTATE_DISABLE;
+
+static inline int get_mode_idx_from_str(const char *str, size_t size)
+{
+ int i;
+
+ for (i=0; i < AMD_PSTATE_MAX; i++) {
+ if (!strncmp(str, amd_pstate_mode_string[i], size))
+ return i;
+ }
+ return -EINVAL;
+}
static inline int pstate_enable(bool enable)
{
@@ -626,10 +637,10 @@ static int __init amd_pstate_init(void)
/*
* by default the pstate driver is disabled to load
* enable the amd_pstate passive mode driver explicitly
- * with amd_pstate=passive in kernel command line
+ * with amd_pstate=passive or other modes in kernel command line
*/
- if (!cppc_load) {
- pr_debug("driver load is disabled, boot with amd_pstate=passive to enable this\n");
+ if (cppc_state == AMD_PSTATE_DISABLE) {
+ pr_debug("driver load is disabled, boot with specific mode to enable this\n");
return -ENODEV;
}
@@ -671,16 +682,24 @@ device_initcall(amd_pstate_init);
static int __init amd_pstate_param(char *str)
{
+ size_t size;
+ int mode_idx;
+
if (!str)
return -EINVAL;
- if (!strcmp(str, "disable")) {
- cppc_load = 0;
- pr_info("driver is explicitly disabled\n");
- } else if (!strcmp(str, "passive"))
- cppc_load = 1;
+ size = strlen(str);
+ mode_idx = get_mode_idx_from_str(str, size);
- return 0;
+ if (mode_idx >= AMD_PSTATE_DISABLE && mode_idx < AMD_PSTATE_MAX) {
+ cppc_state = mode_idx;
+ if (cppc_state == AMD_PSTATE_DISABLE)
+ pr_info("driver is explicitly disabled\n");
+
+ return 0;
+ }
+
+ return -EINVAL;
}
early_param("amd_pstate", amd_pstate_param);
diff --git a/include/linux/amd-pstate.h b/include/linux/amd-pstate.h
index 1c4b8659f171..dae2ce0f6735 100644
--- a/include/linux/amd-pstate.h
+++ b/include/linux/amd-pstate.h
@@ -74,4 +74,21 @@ struct amd_cpudata {
bool boost_supported;
};
+/*
+ * enum amd_pstate_mode - driver working mode of amd pstate
+ */
+enum amd_pstate_mode {
+ AMD_PSTATE_DISABLE = 0,
+ AMD_PSTATE_PASSIVE,
+ AMD_PSTATE_ACTIVE,
+ AMD_PSTATE_MAX,
+};
+
+static const char * const amd_pstate_mode_string[] = {
+ [AMD_PSTATE_DISABLE] = "disable",
+ [AMD_PSTATE_PASSIVE] = "passive",
+ [AMD_PSTATE_ACTIVE] = "active",
+ NULL,
+};
+
#endif /* _LINUX_AMD_PSTATE_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH v9 05/13] cpufreq: amd-pstate: optimize driver working mode selection in amd_pstate_param()
2022-12-25 16:34 ` [PATCH v9 05/13] cpufreq: amd-pstate: optimize driver working mode selection in amd_pstate_param() Perry Yuan
@ 2023-01-04 0:32 ` Limonciello, Mario
0 siblings, 0 replies; 40+ messages in thread
From: Limonciello, Mario @ 2023-01-04 0:32 UTC (permalink / raw)
To: Perry Yuan, rafael.j.wysocki, ray.huang, viresh.kumar
Cc: Deepak.Sharma, Nathan.Fontenot, Alexander.Deucher, Shimmer.Huang,
Xiaojian.Du, Li.Meng, wyes.karny, linux-pm, linux-kernel
On 12/25/2022 10:34, Perry Yuan wrote:
> From: Wyes Karny <wyes.karny@amd.com>
>
> The amd-pstate driver may support multiple working modes.
> Introduce a variable to keep track of which mode is currently enabled.
> Here we use cppc_state var to indicate which mode is enabled.
> This change will help to simplify the the amd_pstate_param() to choose
> which mode used for the following driver registration.
>
> Signed-off-by: Perry Yuan <perry.yuan@amd.com>
> Signed-off-by: Wyes Karny <wyes.karny@amd.com>
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> drivers/cpufreq/amd-pstate.c | 39 +++++++++++++++++++++++++++---------
> include/linux/amd-pstate.h | 17 ++++++++++++++++
> 2 files changed, 46 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index c17bd845f5fc..65c16edbbb20 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -60,7 +60,18 @@
> * module parameter to be able to enable it manually for debugging.
> */
> static struct cpufreq_driver amd_pstate_driver;
> -static int cppc_load __initdata;
> +static int cppc_state = AMD_PSTATE_DISABLE;
> +
> +static inline int get_mode_idx_from_str(const char *str, size_t size)
> +{
> + int i;
> +
> + for (i=0; i < AMD_PSTATE_MAX; i++) {
> + if (!strncmp(str, amd_pstate_mode_string[i], size))
> + return i;
> + }
> + return -EINVAL;
> +}
>
> static inline int pstate_enable(bool enable)
> {
> @@ -626,10 +637,10 @@ static int __init amd_pstate_init(void)
> /*
> * by default the pstate driver is disabled to load
> * enable the amd_pstate passive mode driver explicitly
> - * with amd_pstate=passive in kernel command line
> + * with amd_pstate=passive or other modes in kernel command line
> */
> - if (!cppc_load) {
> - pr_debug("driver load is disabled, boot with amd_pstate=passive to enable this\n");
> + if (cppc_state == AMD_PSTATE_DISABLE) {
> + pr_debug("driver load is disabled, boot with specific mode to enable this\n");
> return -ENODEV;
> }
>
> @@ -671,16 +682,24 @@ device_initcall(amd_pstate_init);
>
> static int __init amd_pstate_param(char *str)
> {
> + size_t size;
> + int mode_idx;
> +
> if (!str)
> return -EINVAL;
>
> - if (!strcmp(str, "disable")) {
> - cppc_load = 0;
> - pr_info("driver is explicitly disabled\n");
> - } else if (!strcmp(str, "passive"))
> - cppc_load = 1;
> + size = strlen(str);
> + mode_idx = get_mode_idx_from_str(str, size);
>
> - return 0;
> + if (mode_idx >= AMD_PSTATE_DISABLE && mode_idx < AMD_PSTATE_MAX) {
> + cppc_state = mode_idx;
> + if (cppc_state == AMD_PSTATE_DISABLE)
> + pr_info("driver is explicitly disabled\n");
> +
> + return 0;
> + }
> +
> + return -EINVAL;
> }
> early_param("amd_pstate", amd_pstate_param);
>
> diff --git a/include/linux/amd-pstate.h b/include/linux/amd-pstate.h
> index 1c4b8659f171..dae2ce0f6735 100644
> --- a/include/linux/amd-pstate.h
> +++ b/include/linux/amd-pstate.h
> @@ -74,4 +74,21 @@ struct amd_cpudata {
> bool boost_supported;
> };
>
> +/*
> + * enum amd_pstate_mode - driver working mode of amd pstate
> + */
> +enum amd_pstate_mode {
> + AMD_PSTATE_DISABLE = 0,
> + AMD_PSTATE_PASSIVE,
> + AMD_PSTATE_ACTIVE,
> + AMD_PSTATE_MAX,
> +};
> +
> +static const char * const amd_pstate_mode_string[] = {
> + [AMD_PSTATE_DISABLE] = "disable",
> + [AMD_PSTATE_PASSIVE] = "passive",
> + [AMD_PSTATE_ACTIVE] = "active",
> + NULL,
> +};
> +
> #endif /* _LINUX_AMD_PSTATE_H */
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v9 06/13] cpufreq: amd-pstate: implement Pstate EPP support for the AMD processors
2022-12-25 16:34 [PATCH v9 00/13] Implement AMD Pstate EPP Driver Perry Yuan
` (4 preceding siblings ...)
2022-12-25 16:34 ` [PATCH v9 05/13] cpufreq: amd-pstate: optimize driver working mode selection in amd_pstate_param() Perry Yuan
@ 2022-12-25 16:34 ` Perry Yuan
2023-01-04 1:05 ` Limonciello, Mario
2023-01-06 5:22 ` Wyes Karny
2022-12-25 16:34 ` [PATCH v9 07/13] cpufreq: amd-pstate: implement amd pstate cpu online and offline callback Perry Yuan
` (6 subsequent siblings)
12 siblings, 2 replies; 40+ messages in thread
From: Perry Yuan @ 2022-12-25 16:34 UTC (permalink / raw)
To: rafael.j.wysocki, Mario.Limonciello, ray.huang, viresh.kumar
Cc: Deepak.Sharma, Nathan.Fontenot, Alexander.Deucher, Shimmer.Huang,
Xiaojian.Du, Li.Meng, wyes.karny, linux-pm, linux-kernel
From: Perry Yuan <Perry.Yuan@amd.com>
Add EPP driver support for AMD SoCs which support a dedicated MSR for
CPPC. EPP is used by the DPM controller to configure the frequency that
a core operates at during short periods of activity.
The SoC EPP targets are configured on a scale from 0 to 255 where 0
represents maximum performance and 255 represents maximum efficiency.
The amd-pstate driver exports profile string names to userspace that are
tied to specific EPP values.
The balance_performance string (0x80) provides the best balance for
efficiency versus power on most systems, but users can choose other
strings to meet their needs as well.
$ cat /sys/devices/system/cpu/cpufreq/policy0/energy_performance_available_preferences
default performance balance_performance balance_power power
$ cat /sys/devices/system/cpu/cpufreq/policy0/energy_performance_preference
balance_performance
To enable the driver,it needs to add `amd_pstate=active` to kernel
command line and kernel will load the active mode epp driver
Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
---
drivers/cpufreq/amd-pstate.c | 374 ++++++++++++++++++++++++++++++++++-
include/linux/amd-pstate.h | 10 +
2 files changed, 378 insertions(+), 6 deletions(-)
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 65c16edbbb20..779bbb58d909 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -59,7 +59,9 @@
* we disable it by default to go acpi-cpufreq on these processors and add a
* module parameter to be able to enable it manually for debugging.
*/
+static struct cpufreq_driver *default_pstate_driver;
static struct cpufreq_driver amd_pstate_driver;
+static struct cpufreq_driver amd_pstate_epp_driver;
static int cppc_state = AMD_PSTATE_DISABLE;
static inline int get_mode_idx_from_str(const char *str, size_t size)
@@ -73,6 +75,114 @@ static inline int get_mode_idx_from_str(const char *str, size_t size)
return -EINVAL;
}
+static DEFINE_MUTEX(amd_pstate_limits_lock);
+static DEFINE_MUTEX(amd_pstate_driver_lock);
+
+static s16 amd_pstate_get_epp(struct amd_cpudata *cpudata, u64 cppc_req_cached)
+{
+ u64 epp;
+ int ret;
+
+ if (boot_cpu_has(X86_FEATURE_CPPC)) {
+ if (!cppc_req_cached) {
+ epp = rdmsrl_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ,
+ &cppc_req_cached);
+ if (epp)
+ return epp;
+ }
+ epp = (cppc_req_cached >> 24) & 0xFF;
+ } else {
+ ret = cppc_get_epp_perf(cpudata->cpu, &epp);
+ if (ret < 0) {
+ pr_debug("Could not retrieve energy perf value (%d)\n", ret);
+ return -EIO;
+ }
+ }
+
+ return (s16)(epp & 0xff);
+}
+
+static int amd_pstate_get_energy_pref_index(struct amd_cpudata *cpudata)
+{
+ s16 epp;
+ int index = -EINVAL;
+
+ epp = amd_pstate_get_epp(cpudata, 0);
+ if (epp < 0)
+ return epp;
+
+ switch (epp) {
+ case HWP_EPP_PERFORMANCE:
+ index = EPP_INDEX_PERFORMANCE;
+ break;
+ case HWP_EPP_BALANCE_PERFORMANCE:
+ index = EPP_INDEX_BALANCE_PERFORMANCE;
+ break;
+ case HWP_EPP_BALANCE_POWERSAVE:
+ index = EPP_INDEX_BALANCE_POWERSAVE;
+ break;
+ case HWP_EPP_POWERSAVE:
+ index = EPP_INDEX_POWERSAVE;
+ break;
+ default:
+ break;
+ }
+
+ return index;
+}
+
+static int amd_pstate_set_epp(struct amd_cpudata *cpudata, u32 epp)
+{
+ int ret;
+ struct cppc_perf_ctrls perf_ctrls;
+
+ if (boot_cpu_has(X86_FEATURE_CPPC)) {
+ u64 value = READ_ONCE(cpudata->cppc_req_cached);
+
+ value &= ~GENMASK_ULL(31, 24);
+ value |= (u64)epp << 24;
+ WRITE_ONCE(cpudata->cppc_req_cached, value);
+
+ ret = wrmsrl_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ, value);
+ if (!ret)
+ cpudata->epp_cached = epp;
+ } else {
+ perf_ctrls.energy_perf = epp;
+ ret = cppc_set_epp_perf(cpudata->cpu, &perf_ctrls, 1);
+ if (ret) {
+ pr_debug("failed to set energy perf value (%d)\n", ret);
+ return ret;
+ }
+ cpudata->epp_cached = epp;
+ }
+
+ return ret;
+}
+
+static int amd_pstate_set_energy_pref_index(struct amd_cpudata *cpudata,
+ int pref_index)
+{
+ int epp = -EINVAL;
+ int ret;
+
+ if (!pref_index) {
+ pr_debug("EPP pref_index is invalid\n");
+ return -EINVAL;
+ }
+
+ if (epp == -EINVAL)
+ epp = epp_values[pref_index];
+
+ if (epp > 0 && cpudata->policy == CPUFREQ_POLICY_PERFORMANCE) {
+ pr_debug("EPP cannot be set under performance policy\n");
+ return -EBUSY;
+ }
+
+ ret = amd_pstate_set_epp(cpudata, epp);
+
+ return ret;
+}
+
static inline int pstate_enable(bool enable)
{
return wrmsrl_safe(MSR_AMD_CPPC_ENABLE, enable);
@@ -81,11 +191,21 @@ static inline int pstate_enable(bool enable)
static int cppc_enable(bool enable)
{
int cpu, ret = 0;
+ struct cppc_perf_ctrls perf_ctrls;
for_each_present_cpu(cpu) {
ret = cppc_set_enable(cpu, enable);
if (ret)
return ret;
+
+ /* Enable autonomous mode for EPP */
+ if (cppc_state == AMD_PSTATE_ACTIVE) {
+ /* Set desired perf as zero to allow EPP firmware control */
+ perf_ctrls.desired_perf = 0;
+ ret = cppc_set_perf(cpu, &perf_ctrls);
+ if (ret)
+ return ret;
+ }
}
return ret;
@@ -429,7 +549,7 @@ static void amd_pstate_boost_init(struct amd_cpudata *cpudata)
return;
cpudata->boost_supported = true;
- amd_pstate_driver.boost_enabled = true;
+ default_pstate_driver->boost_enabled = true;
}
static void amd_perf_ctl_reset(unsigned int cpu)
@@ -603,10 +723,61 @@ static ssize_t show_amd_pstate_highest_perf(struct cpufreq_policy *policy,
return sprintf(&buf[0], "%u\n", perf);
}
+static ssize_t show_energy_performance_available_preferences(
+ struct cpufreq_policy *policy, char *buf)
+{
+ int i = 0;
+ int offset = 0;
+
+ while (energy_perf_strings[i] != NULL)
+ offset += sysfs_emit_at(buf, offset, "%s ", energy_perf_strings[i++]);
+
+ sysfs_emit_at(buf, offset, "\n");
+
+ return offset;
+}
+
+static ssize_t store_energy_performance_preference(
+ struct cpufreq_policy *policy, const char *buf, size_t count)
+{
+ struct amd_cpudata *cpudata = policy->driver_data;
+ char str_preference[21];
+ ssize_t ret;
+
+ ret = sscanf(buf, "%20s", str_preference);
+ if (ret != 1)
+ return -EINVAL;
+
+ ret = match_string(energy_perf_strings, -1, str_preference);
+ if (ret < 0)
+ return -EINVAL;
+
+ mutex_lock(&amd_pstate_limits_lock);
+ ret = amd_pstate_set_energy_pref_index(cpudata, ret);
+ mutex_unlock(&amd_pstate_limits_lock);
+
+ return ret ?: count;
+}
+
+static ssize_t show_energy_performance_preference(
+ struct cpufreq_policy *policy, char *buf)
+{
+ struct amd_cpudata *cpudata = policy->driver_data;
+ int preference;
+
+ preference = amd_pstate_get_energy_pref_index(cpudata);
+ if (preference < 0)
+ return preference;
+
+ return sysfs_emit(buf, "%s\n", energy_perf_strings[preference]);
+}
+
cpufreq_freq_attr_ro(amd_pstate_max_freq);
cpufreq_freq_attr_ro(amd_pstate_lowest_nonlinear_freq);
cpufreq_freq_attr_ro(amd_pstate_highest_perf);
+cpufreq_freq_attr_rw(energy_performance_preference);
+cpufreq_freq_attr_ro(energy_performance_available_preferences);
static struct freq_attr *amd_pstate_attr[] = {
&amd_pstate_max_freq,
@@ -615,6 +786,181 @@ static struct freq_attr *amd_pstate_attr[] = {
NULL,
};
+static struct freq_attr *amd_pstate_epp_attr[] = {
+ &amd_pstate_max_freq,
+ &amd_pstate_lowest_nonlinear_freq,
+ &amd_pstate_highest_perf,
+ &energy_performance_preference,
+ &energy_performance_available_preferences,
+ NULL,
+};
+
+static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
+{
+ int min_freq, max_freq, nominal_freq, lowest_nonlinear_freq, ret;
+ struct amd_cpudata *cpudata;
+ struct device *dev;
+ int rc;
+ u64 value;
+
+ dev = get_cpu_device(policy->cpu);
+ if (!dev)
+ goto free_cpudata1;
+
+ cpudata = kzalloc(sizeof(*cpudata), GFP_KERNEL);
+ if (!cpudata)
+ return -ENOMEM;
+
+ cpudata->cpu = policy->cpu;
+ cpudata->epp_policy = 0;
+
+ rc = amd_pstate_init_perf(cpudata);
+ if (rc)
+ goto free_cpudata1;
+
+ min_freq = amd_get_min_freq(cpudata);
+ max_freq = amd_get_max_freq(cpudata);
+ nominal_freq = amd_get_nominal_freq(cpudata);
+ lowest_nonlinear_freq = amd_get_lowest_nonlinear_freq(cpudata);
+ if (min_freq < 0 || max_freq < 0 || min_freq > max_freq) {
+ dev_err(dev, "min_freq(%d) or max_freq(%d) value is incorrect\n",
+ min_freq, max_freq);
+ ret = -EINVAL;
+ goto free_cpudata1;
+ }
+
+ policy->cpuinfo.min_freq = min_freq;
+ policy->cpuinfo.max_freq = max_freq;
+ /* It will be updated by governor */
+ policy->cur = policy->cpuinfo.min_freq;
+
+ /* Initial processor data capability frequencies */
+ cpudata->max_freq = max_freq;
+ cpudata->min_freq = min_freq;
+ cpudata->nominal_freq = nominal_freq;
+ cpudata->lowest_nonlinear_freq = lowest_nonlinear_freq;
+
+ policy->driver_data = cpudata;
+
+ cpudata->epp_cached = amd_pstate_get_epp(cpudata, value);
+
+ policy->min = policy->cpuinfo.min_freq;
+ policy->max = policy->cpuinfo.max_freq;
+
+ /*
+ * Set the policy to powersave to provide a valid fallback value in case
+ * the default cpufreq governor is neither powersave nor performance.
+ */
+ policy->policy = CPUFREQ_POLICY_POWERSAVE;
+
+ if (boot_cpu_has(X86_FEATURE_CPPC)) {
+ policy->fast_switch_possible = true;
+ ret = rdmsrl_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ, &value);
+ if (ret)
+ return ret;
+ WRITE_ONCE(cpudata->cppc_req_cached, value);
+
+ ret = rdmsrl_on_cpu(cpudata->cpu, MSR_AMD_CPPC_CAP1, &value);
+ if (ret)
+ return ret;
+ WRITE_ONCE(cpudata->cppc_cap1_cached, value);
+ }
+ amd_pstate_boost_init(cpudata);
+
+ return 0;
+
+free_cpudata1:
+ kfree(cpudata);
+ return ret;
+}
+
+static int amd_pstate_epp_cpu_exit(struct cpufreq_policy *policy)
+{
+ pr_debug("CPU %d exiting\n", policy->cpu);
+ policy->fast_switch_possible = false;
+ return 0;
+}
+
+static void amd_pstate_epp_init(unsigned int cpu)
+{
+ struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
+ struct amd_cpudata *cpudata = policy->driver_data;
+ u32 max_perf, min_perf;
+ u64 value;
+ s16 epp;
+
+ max_perf = READ_ONCE(cpudata->highest_perf);
+ min_perf = READ_ONCE(cpudata->lowest_perf);
+
+ value = READ_ONCE(cpudata->cppc_req_cached);
+
+ if (cpudata->policy == CPUFREQ_POLICY_PERFORMANCE)
+ min_perf = max_perf;
+
+ /* Initial min/max values for CPPC Performance Controls Register */
+ value &= ~AMD_CPPC_MIN_PERF(~0L);
+ value |= AMD_CPPC_MIN_PERF(min_perf);
+
+ value &= ~AMD_CPPC_MAX_PERF(~0L);
+ value |= AMD_CPPC_MAX_PERF(max_perf);
+
+ /* CPPC EPP feature require to set zero to the desire perf bit */
+ value &= ~AMD_CPPC_DES_PERF(~0L);
+ value |= AMD_CPPC_DES_PERF(0);
+
+ if (cpudata->epp_policy == cpudata->policy)
+ goto skip_epp;
+
+ cpudata->epp_policy = cpudata->policy;
+
+ if (cpudata->policy == CPUFREQ_POLICY_PERFORMANCE) {
+ epp = amd_pstate_get_epp(cpudata, value);
+ if (epp < 0)
+ goto skip_epp;
+ /* force the epp value to be zero for performance policy */
+ epp = 0;
+ } else {
+ /* Get BIOS pre-defined epp value */
+ epp = amd_pstate_get_epp(cpudata, value);
+ if (epp)
+ goto skip_epp;
+ }
+ /* Set initial EPP value */
+ if (boot_cpu_has(X86_FEATURE_CPPC)) {
+ value &= ~GENMASK_ULL(31, 24);
+ value |= (u64)epp << 24;
+ }
+
+skip_epp:
+ WRITE_ONCE(cpudata->cppc_req_cached, value);
+ amd_pstate_set_epp(cpudata, epp);
+ cpufreq_cpu_put(policy);
+}
+
+static int amd_pstate_epp_set_policy(struct cpufreq_policy *policy)
+{
+ struct amd_cpudata *cpudata = policy->driver_data;
+
+ if (!policy->cpuinfo.max_freq)
+ return -ENODEV;
+
+ pr_debug("set_policy: cpuinfo.max %u policy->max %u\n",
+ policy->cpuinfo.max_freq, policy->max);
+
+ cpudata->policy = policy->policy;
+
+ amd_pstate_epp_init(policy->cpu);
+
+ return 0;
+}
+
+static int amd_pstate_epp_verify_policy(struct cpufreq_policy_data *policy)
+{
+ cpufreq_verify_within_cpu_limits(policy);
+ pr_debug("policy_max =%d, policy_min=%d\n", policy->max, policy->min);
+ return 0;
+}
+
static struct cpufreq_driver amd_pstate_driver = {
.flags = CPUFREQ_CONST_LOOPS | CPUFREQ_NEED_UPDATE_LIMITS,
.verify = amd_pstate_verify,
@@ -628,6 +974,16 @@ static struct cpufreq_driver amd_pstate_driver = {
.attr = amd_pstate_attr,
};
+static struct cpufreq_driver amd_pstate_epp_driver = {
+ .flags = CPUFREQ_CONST_LOOPS,
+ .verify = amd_pstate_epp_verify_policy,
+ .setpolicy = amd_pstate_epp_set_policy,
+ .init = amd_pstate_epp_cpu_init,
+ .exit = amd_pstate_epp_cpu_exit,
+ .name = "amd_pstate_epp",
+ .attr = amd_pstate_epp_attr,
+};
+
static int __init amd_pstate_init(void)
{
int ret;
@@ -656,7 +1012,8 @@ 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");
- amd_pstate_driver.adjust_perf = amd_pstate_adjust_perf;
+ if (cppc_state == AMD_PSTATE_PASSIVE)
+ default_pstate_driver->adjust_perf = amd_pstate_adjust_perf;
} else {
pr_debug("AMD CPPC shared memory based functionality is supported\n");
static_call_update(amd_pstate_enable, cppc_enable);
@@ -667,14 +1024,13 @@ static int __init amd_pstate_init(void)
/* enable amd pstate feature */
ret = amd_pstate_enable(true);
if (ret) {
- pr_err("failed to enable amd-pstate with return %d\n", ret);
+ pr_err("failed to enable with return %d\n", ret);
return ret;
}
- ret = cpufreq_register_driver(&amd_pstate_driver);
+ ret = cpufreq_register_driver(default_pstate_driver);
if (ret)
- pr_err("failed to register amd_pstate_driver with return %d\n",
- ret);
+ pr_err("failed to register with return %d\n", ret);
return ret;
}
@@ -696,6 +1052,12 @@ static int __init amd_pstate_param(char *str)
if (cppc_state == AMD_PSTATE_DISABLE)
pr_info("driver is explicitly disabled\n");
+ if (cppc_state == AMD_PSTATE_ACTIVE)
+ default_pstate_driver = &amd_pstate_epp_driver;
+
+ if (cppc_state == AMD_PSTATE_PASSIVE)
+ default_pstate_driver = &amd_pstate_driver;
+
return 0;
}
diff --git a/include/linux/amd-pstate.h b/include/linux/amd-pstate.h
index dae2ce0f6735..8341a2a2948a 100644
--- a/include/linux/amd-pstate.h
+++ b/include/linux/amd-pstate.h
@@ -47,6 +47,10 @@ struct amd_aperf_mperf {
* @prev: Last Aperf/Mperf/tsc count value read from register
* @freq: current cpu frequency value
* @boost_supported: check whether the Processor or SBIOS supports boost mode
+ * @epp_policy: Last saved policy used to set energy-performance preference
+ * @epp_cached: Cached CPPC energy-performance preference value
+ * @policy: Cpufreq policy value
+ * @cppc_cap1_cached Cached MSR_AMD_CPPC_CAP1 register value
*
* The amd_cpudata is key private data for each CPU thread in AMD P-State, and
* represents all the attributes and goals that AMD P-State requests at runtime.
@@ -72,6 +76,12 @@ struct amd_cpudata {
u64 freq;
bool boost_supported;
+
+ /* EPP feature related attributes*/
+ s16 epp_policy;
+ s16 epp_cached;
+ u32 policy;
+ u64 cppc_cap1_cached;
};
/*
--
2.34.1
^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH v9 06/13] cpufreq: amd-pstate: implement Pstate EPP support for the AMD processors
2022-12-25 16:34 ` [PATCH v9 06/13] cpufreq: amd-pstate: implement Pstate EPP support for the AMD processors Perry Yuan
@ 2023-01-04 1:05 ` Limonciello, Mario
2023-01-05 6:20 ` Yuan, Perry
2023-01-06 5:22 ` Wyes Karny
1 sibling, 1 reply; 40+ messages in thread
From: Limonciello, Mario @ 2023-01-04 1:05 UTC (permalink / raw)
To: Perry Yuan, rafael.j.wysocki, ray.huang, viresh.kumar
Cc: Deepak.Sharma, Nathan.Fontenot, Alexander.Deucher, Shimmer.Huang,
Xiaojian.Du, Li.Meng, wyes.karny, linux-pm, linux-kernel
On 12/25/2022 10:34, Perry Yuan wrote:
> From: Perry Yuan <Perry.Yuan@amd.com>
>
> Add EPP driver support for AMD SoCs which support a dedicated MSR for
> CPPC. EPP is used by the DPM controller to configure the frequency that
> a core operates at during short periods of activity.
>
> The SoC EPP targets are configured on a scale from 0 to 255 where 0
> represents maximum performance and 255 represents maximum efficiency.
>
> The amd-pstate driver exports profile string names to userspace that are
> tied to specific EPP values.
>
> The balance_performance string (0x80) provides the best balance for
> efficiency versus power on most systems, but users can choose other
> strings to meet their needs as well.
>
> $ cat /sys/devices/system/cpu/cpufreq/policy0/energy_performance_available_preferences
> default performance balance_performance balance_power power
>
> $ cat /sys/devices/system/cpu/cpufreq/policy0/energy_performance_preference
> balance_performance
>
> To enable the driver,it needs to add `amd_pstate=active` to kernel
> command line and kernel will load the active mode epp driver
>
> Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
> ---
> drivers/cpufreq/amd-pstate.c | 374 ++++++++++++++++++++++++++++++++++-
> include/linux/amd-pstate.h | 10 +
> 2 files changed, 378 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 65c16edbbb20..779bbb58d909 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -59,7 +59,9 @@
> * we disable it by default to go acpi-cpufreq on these processors and add a
> * module parameter to be able to enable it manually for debugging.
> */
> +static struct cpufreq_driver *default_pstate_driver;
Considering how this is used and that it can be changed dynamically via
sysfs, I think this pointer would be better named
"current_pstate_driver" or perhaps "active_pstate_driver".
> static struct cpufreq_driver amd_pstate_driver;
> +static struct cpufreq_driver amd_pstate_epp_driver;
> static int cppc_state = AMD_PSTATE_DISABLE;
>
> static inline int get_mode_idx_from_str(const char *str, size_t size)
> @@ -73,6 +75,114 @@ static inline int get_mode_idx_from_str(const char *str, size_t size)
> return -EINVAL;
> }
>
> +static DEFINE_MUTEX(amd_pstate_limits_lock);
> +static DEFINE_MUTEX(amd_pstate_driver_lock);
> +
> +static s16 amd_pstate_get_epp(struct amd_cpudata *cpudata, u64 cppc_req_cached)
> +{
> + u64 epp;
> + int ret;
> +
> + if (boot_cpu_has(X86_FEATURE_CPPC)) {
> + if (!cppc_req_cached) {
> + epp = rdmsrl_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ,
> + &cppc_req_cached);
> + if (epp)
> + return epp;
> + }
> + epp = (cppc_req_cached >> 24) & 0xFF;
> + } else {
> + ret = cppc_get_epp_perf(cpudata->cpu, &epp);
> + if (ret < 0) {
> + pr_debug("Could not retrieve energy perf value (%d)\n", ret);
> + return -EIO;
> + }
> + }
> +
> + return (s16)(epp & 0xff);
> +}
> +
> +static int amd_pstate_get_energy_pref_index(struct amd_cpudata *cpudata)
> +{
> + s16 epp;
> + int index = -EINVAL;
> +
> + epp = amd_pstate_get_epp(cpudata, 0);
> + if (epp < 0)
> + return epp;
> +
> + switch (epp) {
> + case HWP_EPP_PERFORMANCE:
> + index = EPP_INDEX_PERFORMANCE;
> + break;
> + case HWP_EPP_BALANCE_PERFORMANCE:
> + index = EPP_INDEX_BALANCE_PERFORMANCE;
> + break;
> + case HWP_EPP_BALANCE_POWERSAVE:
> + index = EPP_INDEX_BALANCE_POWERSAVE;
> + break;
> + case HWP_EPP_POWERSAVE:
> + index = EPP_INDEX_POWERSAVE;
> + break;
> + default:
> + break;
> + }
> +
> + return index;
> +}
> +
> +static int amd_pstate_set_epp(struct amd_cpudata *cpudata, u32 epp)
> +{
> + int ret;
> + struct cppc_perf_ctrls perf_ctrls;
> +
> + if (boot_cpu_has(X86_FEATURE_CPPC)) {
> + u64 value = READ_ONCE(cpudata->cppc_req_cached);
> +
> + value &= ~GENMASK_ULL(31, 24);
> + value |= (u64)epp << 24;
> + WRITE_ONCE(cpudata->cppc_req_cached, value);
> +
> + ret = wrmsrl_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ, value);
> + if (!ret)
> + cpudata->epp_cached = epp;
> + } else {
> + perf_ctrls.energy_perf = epp;
> + ret = cppc_set_epp_perf(cpudata->cpu, &perf_ctrls, 1);
> + if (ret) {
> + pr_debug("failed to set energy perf value (%d)\n", ret);
> + return ret;
> + }
> + cpudata->epp_cached = epp;
> + }
> +
> + return ret;
> +}
> +
> +static int amd_pstate_set_energy_pref_index(struct amd_cpudata *cpudata,
> + int pref_index)
> +{
> + int epp = -EINVAL;
> + int ret;
> +
> + if (!pref_index) {
> + pr_debug("EPP pref_index is invalid\n");
> + return -EINVAL;
> + }
> +
> + if (epp == -EINVAL)
> + epp = epp_values[pref_index];
> +
> + if (epp > 0 && cpudata->policy == CPUFREQ_POLICY_PERFORMANCE) {
> + pr_debug("EPP cannot be set under performance policy\n");
> + return -EBUSY;
> + }
> +
> + ret = amd_pstate_set_epp(cpudata, epp);
> +
> + return ret;
> +}
> +
> static inline int pstate_enable(bool enable)
> {
> return wrmsrl_safe(MSR_AMD_CPPC_ENABLE, enable);
> @@ -81,11 +191,21 @@ static inline int pstate_enable(bool enable)
> static int cppc_enable(bool enable)
> {
> int cpu, ret = 0;
> + struct cppc_perf_ctrls perf_ctrls;
>
> for_each_present_cpu(cpu) {
> ret = cppc_set_enable(cpu, enable);
> if (ret)
> return ret;
> +
> + /* Enable autonomous mode for EPP */
> + if (cppc_state == AMD_PSTATE_ACTIVE) {
> + /* Set desired perf as zero to allow EPP firmware control */
> + perf_ctrls.desired_perf = 0;
> + ret = cppc_set_perf(cpu, &perf_ctrls);
> + if (ret)
> + return ret;
> + }
> }
>
> return ret;
> @@ -429,7 +549,7 @@ static void amd_pstate_boost_init(struct amd_cpudata *cpudata)
> return;
>
> cpudata->boost_supported = true;
> - amd_pstate_driver.boost_enabled = true;
> + default_pstate_driver->boost_enabled = true;
> }
>
> static void amd_perf_ctl_reset(unsigned int cpu)
> @@ -603,10 +723,61 @@ static ssize_t show_amd_pstate_highest_perf(struct cpufreq_policy *policy,
> return sprintf(&buf[0], "%u\n", perf);
> }
>
> +static ssize_t show_energy_performance_available_preferences(
> + struct cpufreq_policy *policy, char *buf)
> +{
> + int i = 0;
> + int offset = 0;
> +
> + while (energy_perf_strings[i] != NULL)
> + offset += sysfs_emit_at(buf, offset, "%s ", energy_perf_strings[i++]);
> +
> + sysfs_emit_at(buf, offset, "\n");
> +
> + return offset;
> +}
> +
> +static ssize_t store_energy_performance_preference(
> + struct cpufreq_policy *policy, const char *buf, size_t count)
> +{
> + struct amd_cpudata *cpudata = policy->driver_data;
> + char str_preference[21];
> + ssize_t ret;
> +
> + ret = sscanf(buf, "%20s", str_preference);
> + if (ret != 1)
> + return -EINVAL;
> +
> + ret = match_string(energy_perf_strings, -1, str_preference);
> + if (ret < 0)
> + return -EINVAL;
> +
> + mutex_lock(&amd_pstate_limits_lock);
> + ret = amd_pstate_set_energy_pref_index(cpudata, ret);
> + mutex_unlock(&amd_pstate_limits_lock);
> +
> + return ret ?: count;
> +}
> +
> +static ssize_t show_energy_performance_preference(
> + struct cpufreq_policy *policy, char *buf)
> +{
> + struct amd_cpudata *cpudata = policy->driver_data;
> + int preference;
> +
> + preference = amd_pstate_get_energy_pref_index(cpudata);
> + if (preference < 0)
> + return preference;
> +
> + return sysfs_emit(buf, "%s\n", energy_perf_strings[preference]);
> +}
> +
> cpufreq_freq_attr_ro(amd_pstate_max_freq);
> cpufreq_freq_attr_ro(amd_pstate_lowest_nonlinear_freq);
>
> cpufreq_freq_attr_ro(amd_pstate_highest_perf);
> +cpufreq_freq_attr_rw(energy_performance_preference);
> +cpufreq_freq_attr_ro(energy_performance_available_preferences);
>
> static struct freq_attr *amd_pstate_attr[] = {
> &amd_pstate_max_freq,
> @@ -615,6 +786,181 @@ static struct freq_attr *amd_pstate_attr[] = {
> NULL,
> };
>
> +static struct freq_attr *amd_pstate_epp_attr[] = {
> + &amd_pstate_max_freq,
> + &amd_pstate_lowest_nonlinear_freq,
> + &amd_pstate_highest_perf,
> + &energy_performance_preference,
> + &energy_performance_available_preferences,
> + NULL,
> +};
> +
> +static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
> +{
> + int min_freq, max_freq, nominal_freq, lowest_nonlinear_freq, ret;
> + struct amd_cpudata *cpudata;
> + struct device *dev;
> + int rc;
> + u64 value;
> +
> + dev = get_cpu_device(policy->cpu);
> + if (!dev)
> + goto free_cpudata1;
> +
> + cpudata = kzalloc(sizeof(*cpudata), GFP_KERNEL);
> + if (!cpudata)
> + return -ENOMEM;
> +
> + cpudata->cpu = policy->cpu;
> + cpudata->epp_policy = 0;
> +
> + rc = amd_pstate_init_perf(cpudata);
> + if (rc)
> + goto free_cpudata1;
> +
> + min_freq = amd_get_min_freq(cpudata);
> + max_freq = amd_get_max_freq(cpudata);
> + nominal_freq = amd_get_nominal_freq(cpudata);
> + lowest_nonlinear_freq = amd_get_lowest_nonlinear_freq(cpudata);
> + if (min_freq < 0 || max_freq < 0 || min_freq > max_freq) {
> + dev_err(dev, "min_freq(%d) or max_freq(%d) value is incorrect\n",
> + min_freq, max_freq);
> + ret = -EINVAL;
> + goto free_cpudata1;
> + }
> +
> + policy->cpuinfo.min_freq = min_freq;
> + policy->cpuinfo.max_freq = max_freq;
> + /* It will be updated by governor */
> + policy->cur = policy->cpuinfo.min_freq;
> +
> + /* Initial processor data capability frequencies */
> + cpudata->max_freq = max_freq;
> + cpudata->min_freq = min_freq;
> + cpudata->nominal_freq = nominal_freq;
> + cpudata->lowest_nonlinear_freq = lowest_nonlinear_freq;
> +
> + policy->driver_data = cpudata;
> +
> + cpudata->epp_cached = amd_pstate_get_epp(cpudata, value);
> +
> + policy->min = policy->cpuinfo.min_freq;
> + policy->max = policy->cpuinfo.max_freq;
> +
> + /*
> + * Set the policy to powersave to provide a valid fallback value in case
> + * the default cpufreq governor is neither powersave nor performance.
> + */
> + policy->policy = CPUFREQ_POLICY_POWERSAVE;
> +
> + if (boot_cpu_has(X86_FEATURE_CPPC)) {
> + policy->fast_switch_possible = true;
> + ret = rdmsrl_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ, &value);
> + if (ret)
> + return ret;
> + WRITE_ONCE(cpudata->cppc_req_cached, value);
> +
> + ret = rdmsrl_on_cpu(cpudata->cpu, MSR_AMD_CPPC_CAP1, &value);
> + if (ret)
> + return ret;
> + WRITE_ONCE(cpudata->cppc_cap1_cached, value);
> + }
> + amd_pstate_boost_init(cpudata);
> +
> + return 0;
> +
> +free_cpudata1:
> + kfree(cpudata);
> + return ret;
> +}
> +
> +static int amd_pstate_epp_cpu_exit(struct cpufreq_policy *policy)
> +{
> + pr_debug("CPU %d exiting\n", policy->cpu);
> + policy->fast_switch_possible = false;
> + return 0;
> +}
> +
> +static void amd_pstate_epp_init(unsigned int cpu)
> +{
> + struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
> + struct amd_cpudata *cpudata = policy->driver_data;
> + u32 max_perf, min_perf;
> + u64 value;
> + s16 epp;
> +
> + max_perf = READ_ONCE(cpudata->highest_perf);
> + min_perf = READ_ONCE(cpudata->lowest_perf);
> +
> + value = READ_ONCE(cpudata->cppc_req_cached);
> +
> + if (cpudata->policy == CPUFREQ_POLICY_PERFORMANCE)
> + min_perf = max_perf;
> +
> + /* Initial min/max values for CPPC Performance Controls Register */
> + value &= ~AMD_CPPC_MIN_PERF(~0L);
> + value |= AMD_CPPC_MIN_PERF(min_perf);
> +
> + value &= ~AMD_CPPC_MAX_PERF(~0L);
> + value |= AMD_CPPC_MAX_PERF(max_perf);
> +
> + /* CPPC EPP feature require to set zero to the desire perf bit */
> + value &= ~AMD_CPPC_DES_PERF(~0L);
> + value |= AMD_CPPC_DES_PERF(0);
> +
> + if (cpudata->epp_policy == cpudata->policy)
> + goto skip_epp;
> +
> + cpudata->epp_policy = cpudata->policy;
> +
> + if (cpudata->policy == CPUFREQ_POLICY_PERFORMANCE) {
> + epp = amd_pstate_get_epp(cpudata, value);
> + if (epp < 0)
> + goto skip_epp;
> + /* force the epp value to be zero for performance policy */
> + epp = 0;
> + } else {
> + /* Get BIOS pre-defined epp value */
> + epp = amd_pstate_get_epp(cpudata, value);
> + if (epp)
> + goto skip_epp;
> + }
> + /* Set initial EPP value */
> + if (boot_cpu_has(X86_FEATURE_CPPC)) {
> + value &= ~GENMASK_ULL(31, 24);
> + value |= (u64)epp << 24;
> + }
> +
> +skip_epp:
> + WRITE_ONCE(cpudata->cppc_req_cached, value);
> + amd_pstate_set_epp(cpudata, epp);
> + cpufreq_cpu_put(policy);
> +}
> +
> +static int amd_pstate_epp_set_policy(struct cpufreq_policy *policy)
> +{
> + struct amd_cpudata *cpudata = policy->driver_data;
> +
> + if (!policy->cpuinfo.max_freq)
> + return -ENODEV;
> +
> + pr_debug("set_policy: cpuinfo.max %u policy->max %u\n",
> + policy->cpuinfo.max_freq, policy->max);
> +
> + cpudata->policy = policy->policy;
> +
> + amd_pstate_epp_init(policy->cpu);
> +
> + return 0;
> +}
> +
> +static int amd_pstate_epp_verify_policy(struct cpufreq_policy_data *policy)
> +{
> + cpufreq_verify_within_cpu_limits(policy);
> + pr_debug("policy_max =%d, policy_min=%d\n", policy->max, policy->min);
> + return 0;
> +}
> +
> static struct cpufreq_driver amd_pstate_driver = {
> .flags = CPUFREQ_CONST_LOOPS | CPUFREQ_NEED_UPDATE_LIMITS,
> .verify = amd_pstate_verify,
> @@ -628,6 +974,16 @@ static struct cpufreq_driver amd_pstate_driver = {
> .attr = amd_pstate_attr,
> };
>
> +static struct cpufreq_driver amd_pstate_epp_driver = {
> + .flags = CPUFREQ_CONST_LOOPS,
> + .verify = amd_pstate_epp_verify_policy,
> + .setpolicy = amd_pstate_epp_set_policy,
> + .init = amd_pstate_epp_cpu_init,
> + .exit = amd_pstate_epp_cpu_exit,
> + .name = "amd_pstate_epp",
> + .attr = amd_pstate_epp_attr,
> +};
> +
> static int __init amd_pstate_init(void)
> {
> int ret;
> @@ -656,7 +1012,8 @@ 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");
> - amd_pstate_driver.adjust_perf = amd_pstate_adjust_perf;
> + if (cppc_state == AMD_PSTATE_PASSIVE)
> + default_pstate_driver->adjust_perf = amd_pstate_adjust_perf;
> } else {
> pr_debug("AMD CPPC shared memory based functionality is supported\n");
> static_call_update(amd_pstate_enable, cppc_enable);
> @@ -667,14 +1024,13 @@ static int __init amd_pstate_init(void)
> /* enable amd pstate feature */
> ret = amd_pstate_enable(true);
> if (ret) {
> - pr_err("failed to enable amd-pstate with return %d\n", ret);
> + pr_err("failed to enable with return %d\n", ret);
> return ret;
> }
>
> - ret = cpufreq_register_driver(&amd_pstate_driver);
> + ret = cpufreq_register_driver(default_pstate_driver);
> if (ret)
> - pr_err("failed to register amd_pstate_driver with return %d\n",
> - ret);
> + pr_err("failed to register with return %d\n", ret);
>
> return ret;
> }
> @@ -696,6 +1052,12 @@ static int __init amd_pstate_param(char *str)
> if (cppc_state == AMD_PSTATE_DISABLE)
> pr_info("driver is explicitly disabled\n");
>
> + if (cppc_state == AMD_PSTATE_ACTIVE)
> + default_pstate_driver = &amd_pstate_epp_driver;
> +
> + if (cppc_state == AMD_PSTATE_PASSIVE)
> + default_pstate_driver = &amd_pstate_driver;
> +
> return 0;
> }
>
> diff --git a/include/linux/amd-pstate.h b/include/linux/amd-pstate.h
> index dae2ce0f6735..8341a2a2948a 100644
> --- a/include/linux/amd-pstate.h
> +++ b/include/linux/amd-pstate.h
> @@ -47,6 +47,10 @@ struct amd_aperf_mperf {
> * @prev: Last Aperf/Mperf/tsc count value read from register
> * @freq: current cpu frequency value
> * @boost_supported: check whether the Processor or SBIOS supports boost mode
> + * @epp_policy: Last saved policy used to set energy-performance preference
> + * @epp_cached: Cached CPPC energy-performance preference value
> + * @policy: Cpufreq policy value
> + * @cppc_cap1_cached Cached MSR_AMD_CPPC_CAP1 register value
> *
> * The amd_cpudata is key private data for each CPU thread in AMD P-State, and
> * represents all the attributes and goals that AMD P-State requests at runtime.
> @@ -72,6 +76,12 @@ struct amd_cpudata {
>
> u64 freq;
> bool boost_supported;
> +
> + /* EPP feature related attributes*/
> + s16 epp_policy;
> + s16 epp_cached;
> + u32 policy;
> + u64 cppc_cap1_cached;
> };
>
> /*
^ permalink raw reply [flat|nested] 40+ messages in thread
* RE: [PATCH v9 06/13] cpufreq: amd-pstate: implement Pstate EPP support for the AMD processors
2023-01-04 1:05 ` Limonciello, Mario
@ 2023-01-05 6:20 ` Yuan, Perry
0 siblings, 0 replies; 40+ messages in thread
From: Yuan, Perry @ 2023-01-05 6:20 UTC (permalink / raw)
To: Limonciello, Mario, rafael.j.wysocki, Huang, Ray, viresh.kumar
Cc: Sharma, Deepak, Fontenot, Nathan, Deucher, Alexander, Huang,
Shimmer, Du, Xiaojian, Meng, Li (Jassmine),
Karny, Wyes, linux-pm, linux-kernel
[AMD Official Use Only - General]
Hi Mario.
> -----Original Message-----
> From: Limonciello, Mario <Mario.Limonciello@amd.com>
> Sent: Wednesday, January 4, 2023 9:05 AM
> To: Yuan, Perry <Perry.Yuan@amd.com>; rafael.j.wysocki@intel.com; Huang,
> Ray <Ray.Huang@amd.com>; viresh.kumar@linaro.org
> Cc: Sharma, Deepak <Deepak.Sharma@amd.com>; Fontenot, Nathan
> <Nathan.Fontenot@amd.com>; Deucher, Alexander
> <Alexander.Deucher@amd.com>; Huang, Shimmer
> <Shimmer.Huang@amd.com>; Du, Xiaojian <Xiaojian.Du@amd.com>; Meng,
> Li (Jassmine) <Li.Meng@amd.com>; Karny, Wyes <Wyes.Karny@amd.com>;
> linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v9 06/13] cpufreq: amd-pstate: implement Pstate EPP
> support for the AMD processors
>
> On 12/25/2022 10:34, Perry Yuan wrote:
> > From: Perry Yuan <Perry.Yuan@amd.com>
> >
> > Add EPP driver support for AMD SoCs which support a dedicated MSR for
> > CPPC. EPP is used by the DPM controller to configure the frequency
> > that a core operates at during short periods of activity.
> >
> > The SoC EPP targets are configured on a scale from 0 to 255 where 0
> > represents maximum performance and 255 represents maximum
> efficiency.
> >
> > The amd-pstate driver exports profile string names to userspace that
> > are tied to specific EPP values.
> >
> > The balance_performance string (0x80) provides the best balance for
> > efficiency versus power on most systems, but users can choose other
> > strings to meet their needs as well.
> >
> > $ cat
> >
> /sys/devices/system/cpu/cpufreq/policy0/energy_performance_available_
> p
> > references default performance balance_performance balance_power
> power
> >
> > $ cat
> >
> /sys/devices/system/cpu/cpufreq/policy0/energy_performance_preferenc
> e
> > balance_performance
> >
> > To enable the driver,it needs to add `amd_pstate=active` to kernel
> > command line and kernel will load the active mode epp driver
> >
> > Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
> > ---
> > drivers/cpufreq/amd-pstate.c | 374
> ++++++++++++++++++++++++++++++++++-
> > include/linux/amd-pstate.h | 10 +
> > 2 files changed, 378 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/cpufreq/amd-pstate.c
> > b/drivers/cpufreq/amd-pstate.c index 65c16edbbb20..779bbb58d909
> 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -59,7 +59,9 @@
> > * we disable it by default to go acpi-cpufreq on these processors and add
> a
> > * module parameter to be able to enable it manually for debugging.
> > */
> > +static struct cpufreq_driver *default_pstate_driver;
>
> Considering how this is used and that it can be changed dynamically via sysfs,
> I think this pointer would be better named "current_pstate_driver" or
> perhaps "active_pstate_driver".
current_pstate_driver looks better, will change it in v10.
thanks for the suggestion.
Perry.
>
> > static struct cpufreq_driver amd_pstate_driver;
> > +static struct cpufreq_driver amd_pstate_epp_driver;
> > static int cppc_state = AMD_PSTATE_DISABLE;
> >
> > static inline int get_mode_idx_from_str(const char *str, size_t
> > size) @@ -73,6 +75,114 @@ static inline int get_mode_idx_from_str(const
> char *str, size_t size)
> > return -EINVAL;
> > }
> >
> > +static DEFINE_MUTEX(amd_pstate_limits_lock);
> > +static DEFINE_MUTEX(amd_pstate_driver_lock);
> > +
> > +static s16 amd_pstate_get_epp(struct amd_cpudata *cpudata, u64
> > +cppc_req_cached) {
> > + u64 epp;
> > + int ret;
> > +
> > + if (boot_cpu_has(X86_FEATURE_CPPC)) {
> > + if (!cppc_req_cached) {
> > + epp = rdmsrl_on_cpu(cpudata->cpu,
> MSR_AMD_CPPC_REQ,
> > + &cppc_req_cached);
> > + if (epp)
> > + return epp;
> > + }
> > + epp = (cppc_req_cached >> 24) & 0xFF;
> > + } else {
> > + ret = cppc_get_epp_perf(cpudata->cpu, &epp);
> > + if (ret < 0) {
> > + pr_debug("Could not retrieve energy perf value
> (%d)\n", ret);
> > + return -EIO;
> > + }
> > + }
> > +
> > + return (s16)(epp & 0xff);
> > +}
> > +
> > +static int amd_pstate_get_energy_pref_index(struct amd_cpudata
> > +*cpudata) {
> > + s16 epp;
> > + int index = -EINVAL;
> > +
> > + epp = amd_pstate_get_epp(cpudata, 0);
> > + if (epp < 0)
> > + return epp;
> > +
> > + switch (epp) {
> > + case HWP_EPP_PERFORMANCE:
> > + index = EPP_INDEX_PERFORMANCE;
> > + break;
> > + case HWP_EPP_BALANCE_PERFORMANCE:
> > + index = EPP_INDEX_BALANCE_PERFORMANCE;
> > + break;
> > + case HWP_EPP_BALANCE_POWERSAVE:
> > + index = EPP_INDEX_BALANCE_POWERSAVE;
> > + break;
> > + case HWP_EPP_POWERSAVE:
> > + index = EPP_INDEX_POWERSAVE;
> > + break;
> > + default:
> > + break;
> > + }
> > +
> > + return index;
> > +}
> > +
> > +static int amd_pstate_set_epp(struct amd_cpudata *cpudata, u32 epp) {
> > + int ret;
> > + struct cppc_perf_ctrls perf_ctrls;
> > +
> > + if (boot_cpu_has(X86_FEATURE_CPPC)) {
> > + u64 value = READ_ONCE(cpudata->cppc_req_cached);
> > +
> > + value &= ~GENMASK_ULL(31, 24);
> > + value |= (u64)epp << 24;
> > + WRITE_ONCE(cpudata->cppc_req_cached, value);
> > +
> > + ret = wrmsrl_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ,
> value);
> > + if (!ret)
> > + cpudata->epp_cached = epp;
> > + } else {
> > + perf_ctrls.energy_perf = epp;
> > + ret = cppc_set_epp_perf(cpudata->cpu, &perf_ctrls, 1);
> > + if (ret) {
> > + pr_debug("failed to set energy perf value (%d)\n",
> ret);
> > + return ret;
> > + }
> > + cpudata->epp_cached = epp;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static int amd_pstate_set_energy_pref_index(struct amd_cpudata
> *cpudata,
> > + int pref_index)
> > +{
> > + int epp = -EINVAL;
> > + int ret;
> > +
> > + if (!pref_index) {
> > + pr_debug("EPP pref_index is invalid\n");
> > + return -EINVAL;
> > + }
> > +
> > + if (epp == -EINVAL)
> > + epp = epp_values[pref_index];
> > +
> > + if (epp > 0 && cpudata->policy == CPUFREQ_POLICY_PERFORMANCE)
> {
> > + pr_debug("EPP cannot be set under performance policy\n");
> > + return -EBUSY;
> > + }
> > +
> > + ret = amd_pstate_set_epp(cpudata, epp);
> > +
> > + return ret;
> > +}
> > +
> > static inline int pstate_enable(bool enable)
> > {
> > return wrmsrl_safe(MSR_AMD_CPPC_ENABLE, enable); @@ -81,11
> +191,21
> > @@ static inline int pstate_enable(bool enable)
> > static int cppc_enable(bool enable)
> > {
> > int cpu, ret = 0;
> > + struct cppc_perf_ctrls perf_ctrls;
> >
> > for_each_present_cpu(cpu) {
> > ret = cppc_set_enable(cpu, enable);
> > if (ret)
> > return ret;
> > +
> > + /* Enable autonomous mode for EPP */
> > + if (cppc_state == AMD_PSTATE_ACTIVE) {
> > + /* Set desired perf as zero to allow EPP firmware
> control */
> > + perf_ctrls.desired_perf = 0;
> > + ret = cppc_set_perf(cpu, &perf_ctrls);
> > + if (ret)
> > + return ret;
> > + }
> > }
> >
> > return ret;
> > @@ -429,7 +549,7 @@ static void amd_pstate_boost_init(struct
> amd_cpudata *cpudata)
> > return;
> >
> > cpudata->boost_supported = true;
> > - amd_pstate_driver.boost_enabled = true;
> > + default_pstate_driver->boost_enabled = true;
> > }
> >
> > static void amd_perf_ctl_reset(unsigned int cpu) @@ -603,10 +723,61
> > @@ static ssize_t show_amd_pstate_highest_perf(struct cpufreq_policy
> *policy,
> > return sprintf(&buf[0], "%u\n", perf);
> > }
> >
> > +static ssize_t show_energy_performance_available_preferences(
> > + struct cpufreq_policy *policy, char *buf) {
> > + int i = 0;
> > + int offset = 0;
> > +
> > + while (energy_perf_strings[i] != NULL)
> > + offset += sysfs_emit_at(buf, offset, "%s ",
> > +energy_perf_strings[i++]);
> > +
> > + sysfs_emit_at(buf, offset, "\n");
> > +
> > + return offset;
> > +}
> > +
> > +static ssize_t store_energy_performance_preference(
> > + struct cpufreq_policy *policy, const char *buf, size_t count) {
> > + struct amd_cpudata *cpudata = policy->driver_data;
> > + char str_preference[21];
> > + ssize_t ret;
> > +
> > + ret = sscanf(buf, "%20s", str_preference);
> > + if (ret != 1)
> > + return -EINVAL;
> > +
> > + ret = match_string(energy_perf_strings, -1, str_preference);
> > + if (ret < 0)
> > + return -EINVAL;
> > +
> > + mutex_lock(&amd_pstate_limits_lock);
> > + ret = amd_pstate_set_energy_pref_index(cpudata, ret);
> > + mutex_unlock(&amd_pstate_limits_lock);
> > +
> > + return ret ?: count;
> > +}
> > +
> > +static ssize_t show_energy_performance_preference(
> > + struct cpufreq_policy *policy, char *buf) {
> > + struct amd_cpudata *cpudata = policy->driver_data;
> > + int preference;
> > +
> > + preference = amd_pstate_get_energy_pref_index(cpudata);
> > + if (preference < 0)
> > + return preference;
> > +
> > + return sysfs_emit(buf, "%s\n", energy_perf_strings[preference]); }
> > +
> > cpufreq_freq_attr_ro(amd_pstate_max_freq);
> > cpufreq_freq_attr_ro(amd_pstate_lowest_nonlinear_freq);
> >
> > cpufreq_freq_attr_ro(amd_pstate_highest_perf);
> > +cpufreq_freq_attr_rw(energy_performance_preference);
> > +cpufreq_freq_attr_ro(energy_performance_available_preferences);
> >
> > static struct freq_attr *amd_pstate_attr[] = {
> > &amd_pstate_max_freq,
> > @@ -615,6 +786,181 @@ static struct freq_attr *amd_pstate_attr[] = {
> > NULL,
> > };
> >
> > +static struct freq_attr *amd_pstate_epp_attr[] = {
> > + &amd_pstate_max_freq,
> > + &amd_pstate_lowest_nonlinear_freq,
> > + &amd_pstate_highest_perf,
> > + &energy_performance_preference,
> > + &energy_performance_available_preferences,
> > + NULL,
> > +};
> > +
> > +static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy) {
> > + int min_freq, max_freq, nominal_freq, lowest_nonlinear_freq, ret;
> > + struct amd_cpudata *cpudata;
> > + struct device *dev;
> > + int rc;
> > + u64 value;
> > +
> > + dev = get_cpu_device(policy->cpu);
> > + if (!dev)
> > + goto free_cpudata1;
> > +
> > + cpudata = kzalloc(sizeof(*cpudata), GFP_KERNEL);
> > + if (!cpudata)
> > + return -ENOMEM;
> > +
> > + cpudata->cpu = policy->cpu;
> > + cpudata->epp_policy = 0;
> > +
> > + rc = amd_pstate_init_perf(cpudata);
> > + if (rc)
> > + goto free_cpudata1;
> > +
> > + min_freq = amd_get_min_freq(cpudata);
> > + max_freq = amd_get_max_freq(cpudata);
> > + nominal_freq = amd_get_nominal_freq(cpudata);
> > + lowest_nonlinear_freq = amd_get_lowest_nonlinear_freq(cpudata);
> > + if (min_freq < 0 || max_freq < 0 || min_freq > max_freq) {
> > + dev_err(dev, "min_freq(%d) or max_freq(%d) value is
> incorrect\n",
> > + min_freq, max_freq);
> > + ret = -EINVAL;
> > + goto free_cpudata1;
> > + }
> > +
> > + policy->cpuinfo.min_freq = min_freq;
> > + policy->cpuinfo.max_freq = max_freq;
> > + /* It will be updated by governor */
> > + policy->cur = policy->cpuinfo.min_freq;
> > +
> > + /* Initial processor data capability frequencies */
> > + cpudata->max_freq = max_freq;
> > + cpudata->min_freq = min_freq;
> > + cpudata->nominal_freq = nominal_freq;
> > + cpudata->lowest_nonlinear_freq = lowest_nonlinear_freq;
> > +
> > + policy->driver_data = cpudata;
> > +
> > + cpudata->epp_cached = amd_pstate_get_epp(cpudata, value);
> > +
> > + policy->min = policy->cpuinfo.min_freq;
> > + policy->max = policy->cpuinfo.max_freq;
> > +
> > + /*
> > + * Set the policy to powersave to provide a valid fallback value in case
> > + * the default cpufreq governor is neither powersave nor
> performance.
> > + */
> > + policy->policy = CPUFREQ_POLICY_POWERSAVE;
> > +
> > + if (boot_cpu_has(X86_FEATURE_CPPC)) {
> > + policy->fast_switch_possible = true;
> > + ret = rdmsrl_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ,
> &value);
> > + if (ret)
> > + return ret;
> > + WRITE_ONCE(cpudata->cppc_req_cached, value);
> > +
> > + ret = rdmsrl_on_cpu(cpudata->cpu, MSR_AMD_CPPC_CAP1,
> &value);
> > + if (ret)
> > + return ret;
> > + WRITE_ONCE(cpudata->cppc_cap1_cached, value);
> > + }
> > + amd_pstate_boost_init(cpudata);
> > +
> > + return 0;
> > +
> > +free_cpudata1:
> > + kfree(cpudata);
> > + return ret;
> > +}
> > +
> > +static int amd_pstate_epp_cpu_exit(struct cpufreq_policy *policy) {
> > + pr_debug("CPU %d exiting\n", policy->cpu);
> > + policy->fast_switch_possible = false;
> > + return 0;
> > +}
> > +
> > +static void amd_pstate_epp_init(unsigned int cpu) {
> > + struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
> > + struct amd_cpudata *cpudata = policy->driver_data;
> > + u32 max_perf, min_perf;
> > + u64 value;
> > + s16 epp;
> > +
> > + max_perf = READ_ONCE(cpudata->highest_perf);
> > + min_perf = READ_ONCE(cpudata->lowest_perf);
> > +
> > + value = READ_ONCE(cpudata->cppc_req_cached);
> > +
> > + if (cpudata->policy == CPUFREQ_POLICY_PERFORMANCE)
> > + min_perf = max_perf;
> > +
> > + /* Initial min/max values for CPPC Performance Controls Register */
> > + value &= ~AMD_CPPC_MIN_PERF(~0L);
> > + value |= AMD_CPPC_MIN_PERF(min_perf);
> > +
> > + value &= ~AMD_CPPC_MAX_PERF(~0L);
> > + value |= AMD_CPPC_MAX_PERF(max_perf);
> > +
> > + /* CPPC EPP feature require to set zero to the desire perf bit */
> > + value &= ~AMD_CPPC_DES_PERF(~0L);
> > + value |= AMD_CPPC_DES_PERF(0);
> > +
> > + if (cpudata->epp_policy == cpudata->policy)
> > + goto skip_epp;
> > +
> > + cpudata->epp_policy = cpudata->policy;
> > +
> > + if (cpudata->policy == CPUFREQ_POLICY_PERFORMANCE) {
> > + epp = amd_pstate_get_epp(cpudata, value);
> > + if (epp < 0)
> > + goto skip_epp;
> > + /* force the epp value to be zero for performance policy */
> > + epp = 0;
> > + } else {
> > + /* Get BIOS pre-defined epp value */
> > + epp = amd_pstate_get_epp(cpudata, value);
> > + if (epp)
> > + goto skip_epp;
> > + }
> > + /* Set initial EPP value */
> > + if (boot_cpu_has(X86_FEATURE_CPPC)) {
> > + value &= ~GENMASK_ULL(31, 24);
> > + value |= (u64)epp << 24;
> > + }
> > +
> > +skip_epp:
> > + WRITE_ONCE(cpudata->cppc_req_cached, value);
> > + amd_pstate_set_epp(cpudata, epp);
> > + cpufreq_cpu_put(policy);
> > +}
> > +
> > +static int amd_pstate_epp_set_policy(struct cpufreq_policy *policy) {
> > + struct amd_cpudata *cpudata = policy->driver_data;
> > +
> > + if (!policy->cpuinfo.max_freq)
> > + return -ENODEV;
> > +
> > + pr_debug("set_policy: cpuinfo.max %u policy->max %u\n",
> > + policy->cpuinfo.max_freq, policy->max);
> > +
> > + cpudata->policy = policy->policy;
> > +
> > + amd_pstate_epp_init(policy->cpu);
> > +
> > + return 0;
> > +}
> > +
> > +static int amd_pstate_epp_verify_policy(struct cpufreq_policy_data
> > +*policy) {
> > + cpufreq_verify_within_cpu_limits(policy);
> > + pr_debug("policy_max =%d, policy_min=%d\n", policy->max, policy-
> >min);
> > + return 0;
> > +}
> > +
> > static struct cpufreq_driver amd_pstate_driver = {
> > .flags = CPUFREQ_CONST_LOOPS |
> CPUFREQ_NEED_UPDATE_LIMITS,
> > .verify = amd_pstate_verify,
> > @@ -628,6 +974,16 @@ static struct cpufreq_driver amd_pstate_driver = {
> > .attr = amd_pstate_attr,
> > };
> >
> > +static struct cpufreq_driver amd_pstate_epp_driver = {
> > + .flags = CPUFREQ_CONST_LOOPS,
> > + .verify = amd_pstate_epp_verify_policy,
> > + .setpolicy = amd_pstate_epp_set_policy,
> > + .init = amd_pstate_epp_cpu_init,
> > + .exit = amd_pstate_epp_cpu_exit,
> > + .name = "amd_pstate_epp",
> > + .attr = amd_pstate_epp_attr,
> > +};
> > +
> > static int __init amd_pstate_init(void)
> > {
> > int ret;
> > @@ -656,7 +1012,8 @@ 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");
> > - amd_pstate_driver.adjust_perf = amd_pstate_adjust_perf;
> > + if (cppc_state == AMD_PSTATE_PASSIVE)
> > + default_pstate_driver->adjust_perf =
> amd_pstate_adjust_perf;
> > } else {
> > pr_debug("AMD CPPC shared memory based functionality is
> supported\n");
> > static_call_update(amd_pstate_enable, cppc_enable); @@ -
> 667,14
> > +1024,13 @@ static int __init amd_pstate_init(void)
> > /* enable amd pstate feature */
> > ret = amd_pstate_enable(true);
> > if (ret) {
> > - pr_err("failed to enable amd-pstate with return %d\n", ret);
> > + pr_err("failed to enable with return %d\n", ret);
> > return ret;
> > }
> >
> > - ret = cpufreq_register_driver(&amd_pstate_driver);
> > + ret = cpufreq_register_driver(default_pstate_driver);
> > if (ret)
> > - pr_err("failed to register amd_pstate_driver with
> return %d\n",
> > - ret);
> > + pr_err("failed to register with return %d\n", ret);
> >
> > return ret;
> > }
> > @@ -696,6 +1052,12 @@ static int __init amd_pstate_param(char *str)
> > if (cppc_state == AMD_PSTATE_DISABLE)
> > pr_info("driver is explicitly disabled\n");
> >
> > + if (cppc_state == AMD_PSTATE_ACTIVE)
> > + default_pstate_driver = &amd_pstate_epp_driver;
> > +
> > + if (cppc_state == AMD_PSTATE_PASSIVE)
> > + default_pstate_driver = &amd_pstate_driver;
> > +
> > return 0;
> > }
> >
> > diff --git a/include/linux/amd-pstate.h b/include/linux/amd-pstate.h
> > index dae2ce0f6735..8341a2a2948a 100644
> > --- a/include/linux/amd-pstate.h
> > +++ b/include/linux/amd-pstate.h
> > @@ -47,6 +47,10 @@ struct amd_aperf_mperf {
> > * @prev: Last Aperf/Mperf/tsc count value read from register
> > * @freq: current cpu frequency value
> > * @boost_supported: check whether the Processor or SBIOS supports
> > boost mode
> > + * @epp_policy: Last saved policy used to set energy-performance
> > + preference
> > + * @epp_cached: Cached CPPC energy-performance preference value
> > + * @policy: Cpufreq policy value
> > + * @cppc_cap1_cached Cached MSR_AMD_CPPC_CAP1 register value
> > *
> > * The amd_cpudata is key private data for each CPU thread in AMD P-
> State, and
> > * represents all the attributes and goals that AMD P-State requests at
> runtime.
> > @@ -72,6 +76,12 @@ struct amd_cpudata {
> >
> > u64 freq;
> > bool boost_supported;
> > +
> > + /* EPP feature related attributes*/
> > + s16 epp_policy;
> > + s16 epp_cached;
> > + u32 policy;
> > + u64 cppc_cap1_cached;
> > };
> >
> > /*
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v9 06/13] cpufreq: amd-pstate: implement Pstate EPP support for the AMD processors
2022-12-25 16:34 ` [PATCH v9 06/13] cpufreq: amd-pstate: implement Pstate EPP support for the AMD processors Perry Yuan
2023-01-04 1:05 ` Limonciello, Mario
@ 2023-01-06 5:22 ` Wyes Karny
2023-01-06 5:58 ` Yuan, Perry
1 sibling, 1 reply; 40+ messages in thread
From: Wyes Karny @ 2023-01-06 5:22 UTC (permalink / raw)
To: Perry Yuan, rafael.j.wysocki, Mario.Limonciello, ray.huang, viresh.kumar
Cc: Deepak.Sharma, Nathan.Fontenot, Alexander.Deucher, Shimmer.Huang,
Xiaojian.Du, Li.Meng, linux-pm, linux-kernel
Hi Perry,
On 12/25/2022 10:04 PM, Perry Yuan wrote:
----------------------------------->8-------------------------------
> +static struct cpufreq_driver amd_pstate_epp_driver = {
> + .flags = CPUFREQ_CONST_LOOPS,
> + .verify = amd_pstate_epp_verify_policy,
> + .setpolicy = amd_pstate_epp_set_policy,
> + .init = amd_pstate_epp_cpu_init,
> + .exit = amd_pstate_epp_cpu_exit,
> + .name = "amd_pstate_epp",
Small nit, amd_pstate_driver name is "amd-pstate", whereas here for
amd_pstate_epp_driver name "amd_pstate_epp" is used.
Should amd_pstate_driver renamed "amd_pstate"?
> + .attr = amd_pstate_epp_attr,
> +};
> +
> static int __init amd_pstate_init(void)
> {
> int ret;
> @@ -656,7 +1012,8 @@ 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");
> - amd_pstate_driver.adjust_perf = amd_pstate_adjust_perf;
> + if (cppc_state == AMD_PSTATE_PASSIVE)
> + default_pstate_driver->adjust_perf = amd_pstate_adjust_perf;
> } else {
> pr_debug("AMD CPPC shared memory based functionality is supported\n");
> static_call_update(amd_pstate_enable, cppc_enable);
> @@ -667,14 +1024,13 @@ static int __init amd_pstate_init(void)
> /* enable amd pstate feature */
> ret = amd_pstate_enable(true);
> if (ret) {
> - pr_err("failed to enable amd-pstate with return %d\n", ret);
> + pr_err("failed to enable with return %d\n", ret);
> return ret;
> }
>
> - ret = cpufreq_register_driver(&amd_pstate_driver);
> + ret = cpufreq_register_driver(default_pstate_driver);
> if (ret)
> - pr_err("failed to register amd_pstate_driver with return %d\n",
> - ret);
> + pr_err("failed to register with return %d\n", ret);
>
> return ret;
> }
> @@ -696,6 +1052,12 @@ static int __init amd_pstate_param(char *str)
> if (cppc_state == AMD_PSTATE_DISABLE)
> pr_info("driver is explicitly disabled\n");
>
> + if (cppc_state == AMD_PSTATE_ACTIVE)
> + default_pstate_driver = &amd_pstate_epp_driver;
> +
> + if (cppc_state == AMD_PSTATE_PASSIVE)
> + default_pstate_driver = &amd_pstate_driver;
> +
> return 0;
> }
>
> diff --git a/include/linux/amd-pstate.h b/include/linux/amd-pstate.h
> index dae2ce0f6735..8341a2a2948a 100644
> --- a/include/linux/amd-pstate.h
> +++ b/include/linux/amd-pstate.h
> @@ -47,6 +47,10 @@ struct amd_aperf_mperf {
> * @prev: Last Aperf/Mperf/tsc count value read from register
> * @freq: current cpu frequency value
> * @boost_supported: check whether the Processor or SBIOS supports boost mode
> + * @epp_policy: Last saved policy used to set energy-performance preference
> + * @epp_cached: Cached CPPC energy-performance preference value
> + * @policy: Cpufreq policy value
> + * @cppc_cap1_cached Cached MSR_AMD_CPPC_CAP1 register value
> *
> * The amd_cpudata is key private data for each CPU thread in AMD P-State, and
> * represents all the attributes and goals that AMD P-State requests at runtime.
> @@ -72,6 +76,12 @@ struct amd_cpudata {
>
> u64 freq;
> bool boost_supported;
> +
> + /* EPP feature related attributes*/
> + s16 epp_policy;
> + s16 epp_cached;
> + u32 policy;
> + u64 cppc_cap1_cached;
> };
>
> /*
--
Thanks & Regards,
Wyes
^ permalink raw reply [flat|nested] 40+ messages in thread
* RE: [PATCH v9 06/13] cpufreq: amd-pstate: implement Pstate EPP support for the AMD processors
2023-01-06 5:22 ` Wyes Karny
@ 2023-01-06 5:58 ` Yuan, Perry
0 siblings, 0 replies; 40+ messages in thread
From: Yuan, Perry @ 2023-01-06 5:58 UTC (permalink / raw)
To: Karny, Wyes, rafael.j.wysocki, Limonciello, Mario, Huang, Ray,
viresh.kumar
Cc: Sharma, Deepak, Fontenot, Nathan, Deucher, Alexander, Huang,
Shimmer, Du, Xiaojian, Meng, Li (Jassmine),
linux-pm, linux-kernel
[AMD Official Use Only - General]
Hi Wyes.
> -----Original Message-----
> From: Karny, Wyes <Wyes.Karny@amd.com>
> Sent: Friday, January 6, 2023 1:22 PM
> To: Yuan, Perry <Perry.Yuan@amd.com>; rafael.j.wysocki@intel.com;
> Limonciello, Mario <Mario.Limonciello@amd.com>; Huang, Ray
> <Ray.Huang@amd.com>; viresh.kumar@linaro.org
> Cc: Sharma, Deepak <Deepak.Sharma@amd.com>; Fontenot, Nathan
> <Nathan.Fontenot@amd.com>; Deucher, Alexander
> <Alexander.Deucher@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 v9 06/13] cpufreq: amd-pstate: implement Pstate EPP
> support for the AMD processors
>
> Hi Perry,
>
> On 12/25/2022 10:04 PM, Perry Yuan wrote:
> ----------------------------------->8-------------------------------
> > +static struct cpufreq_driver amd_pstate_epp_driver = {
> > + .flags = CPUFREQ_CONST_LOOPS,
> > + .verify = amd_pstate_epp_verify_policy,
> > + .setpolicy = amd_pstate_epp_set_policy,
> > + .init = amd_pstate_epp_cpu_init,
> > + .exit = amd_pstate_epp_cpu_exit,
> > + .name = "amd_pstate_epp",
>
> Small nit, amd_pstate_driver name is "amd-pstate", whereas here for
> amd_pstate_epp_driver name "amd_pstate_epp" is used.
> Should amd_pstate_driver renamed "amd_pstate"?
For the non-epp and epp driver name, I suggest to keep the same rule for the name. but it depends on Ray would like to change it or not for non-epp driver name.
>
> > + .attr = amd_pstate_epp_attr,
> > +};
> > +
> > static int __init amd_pstate_init(void) {
> > int ret;
> > @@ -656,7 +1012,8 @@ 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");
> > - amd_pstate_driver.adjust_perf = amd_pstate_adjust_perf;
> > + if (cppc_state == AMD_PSTATE_PASSIVE)
> > + default_pstate_driver->adjust_perf =
> amd_pstate_adjust_perf;
> > } else {
> > pr_debug("AMD CPPC shared memory based functionality is
> supported\n");
> > static_call_update(amd_pstate_enable, cppc_enable); @@ -
> 667,14
> > +1024,13 @@ static int __init amd_pstate_init(void)
> > /* enable amd pstate feature */
> > ret = amd_pstate_enable(true);
> > if (ret) {
> > - pr_err("failed to enable amd-pstate with return %d\n", ret);
> > + pr_err("failed to enable with return %d\n", ret);
> > return ret;
> > }
> >
> > - ret = cpufreq_register_driver(&amd_pstate_driver);
> > + ret = cpufreq_register_driver(default_pstate_driver);
> > if (ret)
> > - pr_err("failed to register amd_pstate_driver with
> return %d\n",
> > - ret);
> > + pr_err("failed to register with return %d\n", ret);
> >
> > return ret;
> > }
> > @@ -696,6 +1052,12 @@ static int __init amd_pstate_param(char *str)
> > if (cppc_state == AMD_PSTATE_DISABLE)
> > pr_info("driver is explicitly disabled\n");
> >
> > + if (cppc_state == AMD_PSTATE_ACTIVE)
> > + default_pstate_driver = &amd_pstate_epp_driver;
> > +
> > + if (cppc_state == AMD_PSTATE_PASSIVE)
> > + default_pstate_driver = &amd_pstate_driver;
> > +
> > return 0;
> > }
> >
> > diff --git a/include/linux/amd-pstate.h b/include/linux/amd-pstate.h
> > index dae2ce0f6735..8341a2a2948a 100644
> > --- a/include/linux/amd-pstate.h
> > +++ b/include/linux/amd-pstate.h
> > @@ -47,6 +47,10 @@ struct amd_aperf_mperf {
> > * @prev: Last Aperf/Mperf/tsc count value read from register
> > * @freq: current cpu frequency value
> > * @boost_supported: check whether the Processor or SBIOS supports
> > boost mode
> > + * @epp_policy: Last saved policy used to set energy-performance
> > + preference
> > + * @epp_cached: Cached CPPC energy-performance preference value
> > + * @policy: Cpufreq policy value
> > + * @cppc_cap1_cached Cached MSR_AMD_CPPC_CAP1 register value
> > *
> > * The amd_cpudata is key private data for each CPU thread in AMD P-
> State, and
> > * represents all the attributes and goals that AMD P-State requests at
> runtime.
> > @@ -72,6 +76,12 @@ struct amd_cpudata {
> >
> > u64 freq;
> > bool boost_supported;
> > +
> > + /* EPP feature related attributes*/
> > + s16 epp_policy;
> > + s16 epp_cached;
> > + u32 policy;
> > + u64 cppc_cap1_cached;
> > };
> >
> > /*
>
> --
> Thanks & Regards,
> Wyes
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v9 07/13] cpufreq: amd-pstate: implement amd pstate cpu online and offline callback
2022-12-25 16:34 [PATCH v9 00/13] Implement AMD Pstate EPP Driver Perry Yuan
` (5 preceding siblings ...)
2022-12-25 16:34 ` [PATCH v9 06/13] cpufreq: amd-pstate: implement Pstate EPP support for the AMD processors Perry Yuan
@ 2022-12-25 16:34 ` Perry Yuan
2022-12-25 16:34 ` [PATCH v9 08/13] cpufreq: amd-pstate: implement suspend and resume callbacks Perry Yuan
` (5 subsequent siblings)
12 siblings, 0 replies; 40+ messages in thread
From: Perry Yuan @ 2022-12-25 16:34 UTC (permalink / raw)
To: rafael.j.wysocki, Mario.Limonciello, ray.huang, viresh.kumar
Cc: Deepak.Sharma, Nathan.Fontenot, Alexander.Deucher, Shimmer.Huang,
Xiaojian.Du, Li.Meng, wyes.karny, linux-pm, linux-kernel
From: Perry Yuan <Perry.Yuan@amd.com>
Adds online and offline driver callback support to allow cpu cores go
offline and help to restore the previous working states when core goes
back online later for EPP driver mode.
Acked-by: Huang Rui <ray.huang@amd.com>
Reviewed-by: Mario Limonciello <Mario.Limonciello@amd.com>
Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
---
drivers/cpufreq/amd-pstate.c | 82 ++++++++++++++++++++++++++++++++++++
include/linux/amd-pstate.h | 1 +
2 files changed, 83 insertions(+)
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 779bbb58d909..c671f4955766 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -954,6 +954,86 @@ static int amd_pstate_epp_set_policy(struct cpufreq_policy *policy)
return 0;
}
+static void amd_pstate_epp_reenable(struct amd_cpudata *cpudata)
+{
+ struct cppc_perf_ctrls perf_ctrls;
+ u64 value, max_perf;
+ int ret;
+
+ ret = amd_pstate_enable(true);
+ if (ret)
+ pr_err("failed to enable amd pstate during resume, return %d\n", ret);
+
+ value = READ_ONCE(cpudata->cppc_req_cached);
+ max_perf = READ_ONCE(cpudata->highest_perf);
+
+ if (boot_cpu_has(X86_FEATURE_CPPC)) {
+ wrmsrl_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ, value);
+ } else {
+ perf_ctrls.max_perf = max_perf;
+ perf_ctrls.energy_perf = AMD_CPPC_ENERGY_PERF_PREF(cpudata->epp_cached);
+ cppc_set_perf(cpudata->cpu, &perf_ctrls);
+ }
+}
+
+static int amd_pstate_epp_cpu_online(struct cpufreq_policy *policy)
+{
+ struct amd_cpudata *cpudata = policy->driver_data;
+
+ pr_debug("AMD CPU Core %d going online\n", cpudata->cpu);
+
+ if (cppc_state == AMD_PSTATE_ACTIVE) {
+ amd_pstate_epp_reenable(cpudata);
+ cpudata->suspended = false;
+ }
+
+ return 0;
+}
+
+static void amd_pstate_epp_offline(struct cpufreq_policy *policy)
+{
+ struct amd_cpudata *cpudata = policy->driver_data;
+ struct cppc_perf_ctrls perf_ctrls;
+ int min_perf;
+ u64 value;
+
+ min_perf = READ_ONCE(cpudata->lowest_perf);
+ value = READ_ONCE(cpudata->cppc_req_cached);
+
+ mutex_lock(&amd_pstate_limits_lock);
+ if (boot_cpu_has(X86_FEATURE_CPPC)) {
+ cpudata->epp_policy = CPUFREQ_POLICY_UNKNOWN;
+
+ /* Set max perf same as min perf */
+ value &= ~AMD_CPPC_MAX_PERF(~0L);
+ value |= AMD_CPPC_MAX_PERF(min_perf);
+ value &= ~AMD_CPPC_MIN_PERF(~0L);
+ value |= AMD_CPPC_MIN_PERF(min_perf);
+ wrmsrl_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ, value);
+ } else {
+ perf_ctrls.desired_perf = 0;
+ perf_ctrls.max_perf = min_perf;
+ perf_ctrls.energy_perf = AMD_CPPC_ENERGY_PERF_PREF(HWP_EPP_BALANCE_POWERSAVE);
+ cppc_set_perf(cpudata->cpu, &perf_ctrls);
+ }
+ mutex_unlock(&amd_pstate_limits_lock);
+}
+
+static int amd_pstate_epp_cpu_offline(struct cpufreq_policy *policy)
+{
+ struct amd_cpudata *cpudata = policy->driver_data;
+
+ pr_debug("AMD CPU Core %d going offline\n", cpudata->cpu);
+
+ if (cpudata->suspended)
+ return 0;
+
+ if (cppc_state == AMD_PSTATE_ACTIVE)
+ amd_pstate_epp_offline(policy);
+
+ return 0;
+}
+
static int amd_pstate_epp_verify_policy(struct cpufreq_policy_data *policy)
{
cpufreq_verify_within_cpu_limits(policy);
@@ -980,6 +1060,8 @@ static struct cpufreq_driver amd_pstate_epp_driver = {
.setpolicy = amd_pstate_epp_set_policy,
.init = amd_pstate_epp_cpu_init,
.exit = amd_pstate_epp_cpu_exit,
+ .offline = amd_pstate_epp_cpu_offline,
+ .online = amd_pstate_epp_cpu_online,
.name = "amd_pstate_epp",
.attr = amd_pstate_epp_attr,
};
diff --git a/include/linux/amd-pstate.h b/include/linux/amd-pstate.h
index 8341a2a2948a..15761a581e82 100644
--- a/include/linux/amd-pstate.h
+++ b/include/linux/amd-pstate.h
@@ -82,6 +82,7 @@ struct amd_cpudata {
s16 epp_cached;
u32 policy;
u64 cppc_cap1_cached;
+ bool suspended;
};
/*
--
2.34.1
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH v9 08/13] cpufreq: amd-pstate: implement suspend and resume callbacks
2022-12-25 16:34 [PATCH v9 00/13] Implement AMD Pstate EPP Driver Perry Yuan
` (6 preceding siblings ...)
2022-12-25 16:34 ` [PATCH v9 07/13] cpufreq: amd-pstate: implement amd pstate cpu online and offline callback Perry Yuan
@ 2022-12-25 16:34 ` Perry Yuan
2022-12-27 2:52 ` Viresh Kumar
2022-12-25 16:34 ` [PATCH v9 09/13] cpufreq: amd-pstate: add driver working mode switch support Perry Yuan
` (4 subsequent siblings)
12 siblings, 1 reply; 40+ messages in thread
From: Perry Yuan @ 2022-12-25 16:34 UTC (permalink / raw)
To: rafael.j.wysocki, Mario.Limonciello, ray.huang, viresh.kumar
Cc: Deepak.Sharma, Nathan.Fontenot, Alexander.Deucher, Shimmer.Huang,
Xiaojian.Du, Li.Meng, wyes.karny, linux-pm, linux-kernel
From: Perry Yuan <Perry.Yuan@amd.com>
add suspend and resume support for the AMD processors by amd_pstate_epp
driver instance.
When the CPPC is suspended, EPP driver will set EPP profile to 'power'
profile and set max/min perf to lowest perf value.
When resume happens, it will restore the MSR registers with
previous cached value.
Acked-by: Huang Rui <ray.huang@amd.com>
Reviewed-by: Mario Limonciello <Mario.Limonciello@amd.com>
Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
---
drivers/cpufreq/amd-pstate.c | 40 ++++++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index c671f4955766..e3676d1a85c7 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -1041,6 +1041,44 @@ static int amd_pstate_epp_verify_policy(struct cpufreq_policy_data *policy)
return 0;
}
+static int amd_pstate_epp_suspend(struct cpufreq_policy *policy)
+{
+ struct amd_cpudata *cpudata = policy->driver_data;
+ int ret;
+
+ /* avoid suspending when EPP is not enabled */
+ if (cppc_state != AMD_PSTATE_ACTIVE)
+ return 0;
+
+ /* set this flag to avoid setting core offline*/
+ cpudata->suspended = true;
+
+ /* disable CPPC in lowlevel firmware */
+ ret = amd_pstate_enable(false);
+ if (ret)
+ pr_err("failed to suspend, return %d\n", ret);
+
+ return 0;
+}
+
+static int amd_pstate_epp_resume(struct cpufreq_policy *policy)
+{
+ struct amd_cpudata *cpudata = policy->driver_data;
+
+ if (cpudata->suspended) {
+ mutex_lock(&amd_pstate_limits_lock);
+
+ /* enable amd pstate from suspend state*/
+ amd_pstate_epp_reenable(cpudata);
+
+ mutex_unlock(&amd_pstate_limits_lock);
+
+ cpudata->suspended = false;
+ }
+
+ return 0;
+}
+
static struct cpufreq_driver amd_pstate_driver = {
.flags = CPUFREQ_CONST_LOOPS | CPUFREQ_NEED_UPDATE_LIMITS,
.verify = amd_pstate_verify,
@@ -1062,6 +1100,8 @@ static struct cpufreq_driver amd_pstate_epp_driver = {
.exit = amd_pstate_epp_cpu_exit,
.offline = amd_pstate_epp_cpu_offline,
.online = amd_pstate_epp_cpu_online,
+ .suspend = amd_pstate_epp_suspend,
+ .resume = amd_pstate_epp_resume,
.name = "amd_pstate_epp",
.attr = amd_pstate_epp_attr,
};
--
2.34.1
^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH v9 08/13] cpufreq: amd-pstate: implement suspend and resume callbacks
2022-12-25 16:34 ` [PATCH v9 08/13] cpufreq: amd-pstate: implement suspend and resume callbacks Perry Yuan
@ 2022-12-27 2:52 ` Viresh Kumar
2023-01-05 15:08 ` Yuan, Perry
0 siblings, 1 reply; 40+ messages in thread
From: Viresh Kumar @ 2022-12-27 2:52 UTC (permalink / raw)
To: Perry Yuan
Cc: rafael.j.wysocki, Mario.Limonciello, ray.huang, Deepak.Sharma,
Nathan.Fontenot, Alexander.Deucher, Shimmer.Huang, Xiaojian.Du,
Li.Meng, wyes.karny, linux-pm, linux-kernel
On 26-12-22, 00:34, Perry Yuan wrote:
> From: Perry Yuan <Perry.Yuan@amd.com>
>
> add suspend and resume support for the AMD processors by amd_pstate_epp
> driver instance.
>
> When the CPPC is suspended, EPP driver will set EPP profile to 'power'
> profile and set max/min perf to lowest perf value.
> When resume happens, it will restore the MSR registers with
> previous cached value.
>
> Acked-by: Huang Rui <ray.huang@amd.com>
> Reviewed-by: Mario Limonciello <Mario.Limonciello@amd.com>
> Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
> ---
> drivers/cpufreq/amd-pstate.c | 40 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
>
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index c671f4955766..e3676d1a85c7 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -1041,6 +1041,44 @@ static int amd_pstate_epp_verify_policy(struct cpufreq_policy_data *policy)
> return 0;
> }
>
> +static int amd_pstate_epp_suspend(struct cpufreq_policy *policy)
> +{
> + struct amd_cpudata *cpudata = policy->driver_data;
> + int ret;
> +
> + /* avoid suspending when EPP is not enabled */
> + if (cppc_state != AMD_PSTATE_ACTIVE)
> + return 0;
> +
> + /* set this flag to avoid setting core offline*/
> + cpudata->suspended = true;
> +
> + /* disable CPPC in lowlevel firmware */
> + ret = amd_pstate_enable(false);
> + if (ret)
> + pr_err("failed to suspend, return %d\n", ret);
> +
> + return 0;
> +}
> +
> +static int amd_pstate_epp_resume(struct cpufreq_policy *policy)
> +{
> + struct amd_cpudata *cpudata = policy->driver_data;
> +
> + if (cpudata->suspended) {
When will resume get called without being suspended first ?
> + mutex_lock(&amd_pstate_limits_lock);
> +
> + /* enable amd pstate from suspend state*/
> + amd_pstate_epp_reenable(cpudata);
> +
> + mutex_unlock(&amd_pstate_limits_lock);
> +
> + cpudata->suspended = false;
> + }
> +
> + return 0;
> +}
> +
> static struct cpufreq_driver amd_pstate_driver = {
> .flags = CPUFREQ_CONST_LOOPS | CPUFREQ_NEED_UPDATE_LIMITS,
> .verify = amd_pstate_verify,
> @@ -1062,6 +1100,8 @@ static struct cpufreq_driver amd_pstate_epp_driver = {
> .exit = amd_pstate_epp_cpu_exit,
> .offline = amd_pstate_epp_cpu_offline,
> .online = amd_pstate_epp_cpu_online,
> + .suspend = amd_pstate_epp_suspend,
> + .resume = amd_pstate_epp_resume,
> .name = "amd_pstate_epp",
> .attr = amd_pstate_epp_attr,
> };
> --
> 2.34.1
--
viresh
^ permalink raw reply [flat|nested] 40+ messages in thread
* RE: [PATCH v9 08/13] cpufreq: amd-pstate: implement suspend and resume callbacks
2022-12-27 2:52 ` Viresh Kumar
@ 2023-01-05 15:08 ` Yuan, Perry
0 siblings, 0 replies; 40+ messages in thread
From: Yuan, Perry @ 2023-01-05 15:08 UTC (permalink / raw)
To: Viresh Kumar, Limonciello, Mario
Cc: rafael.j.wysocki, Huang, Ray, Sharma, Deepak, Fontenot, Nathan,
Deucher, Alexander, Huang, Shimmer, Du, Xiaojian, Meng,
Li (Jassmine),
Karny, Wyes, linux-pm, linux-kernel
[AMD Official Use Only - General]
Hi Viresh.
> -----Original Message-----
> From: Viresh Kumar <viresh.kumar@linaro.org>
> Sent: Tuesday, December 27, 2022 10:52 AM
> To: Yuan, Perry <Perry.Yuan@amd.com>
> Cc: rafael.j.wysocki@intel.com; Limonciello, Mario
> <Mario.Limonciello@amd.com>; Huang, Ray <Ray.Huang@amd.com>;
> Sharma, Deepak <Deepak.Sharma@amd.com>; Fontenot, Nathan
> <Nathan.Fontenot@amd.com>; Deucher, Alexander
> <Alexander.Deucher@amd.com>; Huang, Shimmer
> <Shimmer.Huang@amd.com>; Du, Xiaojian <Xiaojian.Du@amd.com>; Meng,
> Li (Jassmine) <Li.Meng@amd.com>; Karny, Wyes <Wyes.Karny@amd.com>;
> linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v9 08/13] cpufreq: amd-pstate: implement suspend and
> resume callbacks
>
> On 26-12-22, 00:34, Perry Yuan wrote:
> > From: Perry Yuan <Perry.Yuan@amd.com>
> >
> > add suspend and resume support for the AMD processors by
> > amd_pstate_epp driver instance.
> >
> > When the CPPC is suspended, EPP driver will set EPP profile to 'power'
> > profile and set max/min perf to lowest perf value.
> > When resume happens, it will restore the MSR registers with previous
> > cached value.
> >
> > Acked-by: Huang Rui <ray.huang@amd.com>
> > Reviewed-by: Mario Limonciello <Mario.Limonciello@amd.com>
> > Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
> > ---
> > drivers/cpufreq/amd-pstate.c | 40
> > ++++++++++++++++++++++++++++++++++++
> > 1 file changed, 40 insertions(+)
> >
> > diff --git a/drivers/cpufreq/amd-pstate.c
> > b/drivers/cpufreq/amd-pstate.c index c671f4955766..e3676d1a85c7 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -1041,6 +1041,44 @@ static int amd_pstate_epp_verify_policy(struct
> cpufreq_policy_data *policy)
> > return 0;
> > }
> >
> > +static int amd_pstate_epp_suspend(struct cpufreq_policy *policy) {
> > + struct amd_cpudata *cpudata = policy->driver_data;
> > + int ret;
> > +
> > + /* avoid suspending when EPP is not enabled */
> > + if (cppc_state != AMD_PSTATE_ACTIVE)
> > + return 0;
> > +
> > + /* set this flag to avoid setting core offline*/
> > + cpudata->suspended = true;
> > +
> > + /* disable CPPC in lowlevel firmware */
> > + ret = amd_pstate_enable(false);
> > + if (ret)
> > + pr_err("failed to suspend, return %d\n", ret);
> > +
> > + return 0;
> > +}
> > +
> > +static int amd_pstate_epp_resume(struct cpufreq_policy *policy) {
> > + struct amd_cpudata *cpudata = policy->driver_data;
> > +
> > + if (cpudata->suspended) {
>
> When will resume get called without being suspended first ?
Sorry for the late reply.
Theoretically the resume() will get called when system suspended firstly.
Checking cpudata->suspended flag to make sure eachtime resume() is called to resume the previous MSR values safely in my view.
Maybe we can drop the checking code, but it will take more time to run testing~
So to be safe , we can keep this, I will try to do some optimization in future.
Perry.
>
> > + mutex_lock(&amd_pstate_limits_lock);
> > +
> > + /* enable amd pstate from suspend state*/
> > + amd_pstate_epp_reenable(cpudata);
> > +
> > + mutex_unlock(&amd_pstate_limits_lock);
> > +
> > + cpudata->suspended = false;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static struct cpufreq_driver amd_pstate_driver = {
> > .flags = CPUFREQ_CONST_LOOPS |
> CPUFREQ_NEED_UPDATE_LIMITS,
> > .verify = amd_pstate_verify,
> > @@ -1062,6 +1100,8 @@ static struct cpufreq_driver
> amd_pstate_epp_driver = {
> > .exit = amd_pstate_epp_cpu_exit,
> > .offline = amd_pstate_epp_cpu_offline,
> > .online = amd_pstate_epp_cpu_online,
> > + .suspend = amd_pstate_epp_suspend,
> > + .resume = amd_pstate_epp_resume,
> > .name = "amd_pstate_epp",
> > .attr = amd_pstate_epp_attr,
> > };
> > --
> > 2.34.1
>
> --
> viresh
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v9 09/13] cpufreq: amd-pstate: add driver working mode switch support
2022-12-25 16:34 [PATCH v9 00/13] Implement AMD Pstate EPP Driver Perry Yuan
` (7 preceding siblings ...)
2022-12-25 16:34 ` [PATCH v9 08/13] cpufreq: amd-pstate: implement suspend and resume callbacks Perry Yuan
@ 2022-12-25 16:34 ` Perry Yuan
2023-01-04 1:06 ` Limonciello, Mario
` (4 more replies)
2022-12-25 16:34 ` [PATCH v9 10/13] Documentation: amd-pstate: add amd pstate driver mode introduction Perry Yuan
` (3 subsequent siblings)
12 siblings, 5 replies; 40+ messages in thread
From: Perry Yuan @ 2022-12-25 16:34 UTC (permalink / raw)
To: rafael.j.wysocki, Mario.Limonciello, ray.huang, viresh.kumar
Cc: Deepak.Sharma, Nathan.Fontenot, Alexander.Deucher, Shimmer.Huang,
Xiaojian.Du, Li.Meng, wyes.karny, linux-pm, linux-kernel
From: Perry Yuan <Perry.Yuan@amd.com>
While amd-pstate driver was loaded with specific driver mode, it will
need to check which mode is enabled for the pstate driver,add this sysfs
entry to show the current status
$ cat /sys/devices/system/cpu/amd-pstate/status
active
Meanwhile, user can switch the pstate driver mode with writing mode
string to sysfs entry as below.
Enable passive mode:
$ sudo bash -c "echo passive > /sys/devices/system/cpu/amd-pstate/status"
Enable active mode (EPP driver mode):
$ sudo bash -c "echo active > /sys/devices/system/cpu/amd-pstate/status"
Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
---
drivers/cpufreq/amd-pstate.c | 118 +++++++++++++++++++++++++++++++++++
1 file changed, 118 insertions(+)
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index e3676d1a85c7..8ceca4524fc1 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -63,6 +63,7 @@ static struct cpufreq_driver *default_pstate_driver;
static struct cpufreq_driver amd_pstate_driver;
static struct cpufreq_driver amd_pstate_epp_driver;
static int cppc_state = AMD_PSTATE_DISABLE;
+struct kobject *amd_pstate_kobj;
static inline int get_mode_idx_from_str(const char *str, size_t size)
{
@@ -632,6 +633,8 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy)
policy->driver_data = cpudata;
amd_pstate_boost_init(cpudata);
+ if (!default_pstate_driver->adjust_perf)
+ default_pstate_driver->adjust_perf = amd_pstate_adjust_perf;
return 0;
@@ -772,12 +775,99 @@ static ssize_t show_energy_performance_preference(
return sysfs_emit(buf, "%s\n", energy_perf_strings[preference]);
}
+static ssize_t amd_pstate_show_status(char *buf)
+{
+ if (!default_pstate_driver)
+ return sysfs_emit(buf, "off\n");
+
+ return sysfs_emit(buf, "%s\n", amd_pstate_mode_string[cppc_state]);
+}
+
+static void amd_pstate_driver_cleanup(void)
+{
+ default_pstate_driver = NULL;
+}
+
+static int amd_pstate_update_status(const char *buf, size_t size)
+{
+ int ret;
+ int mode_idx;
+
+ if (size > 7 || size < 3)
+ return -EINVAL;
+ mode_idx = get_mode_idx_from_str(buf, size);
+
+ switch(mode_idx) {
+ case AMD_PSTATE_DISABLE:
+ if (!default_pstate_driver)
+ return -EINVAL;
+ if (cppc_state == AMD_PSTATE_ACTIVE)
+ return -EBUSY;
+ ret = cpufreq_unregister_driver(default_pstate_driver);
+ amd_pstate_driver_cleanup();
+ break;
+ case AMD_PSTATE_PASSIVE:
+ if (default_pstate_driver) {
+ if (default_pstate_driver == &amd_pstate_driver)
+ return 0;
+ cpufreq_unregister_driver(default_pstate_driver);
+ cppc_state = AMD_PSTATE_PASSIVE;
+ default_pstate_driver = &amd_pstate_driver;
+ }
+
+ ret = cpufreq_register_driver(default_pstate_driver);
+ break;
+ case AMD_PSTATE_ACTIVE:
+ if (default_pstate_driver) {
+ if (default_pstate_driver == &amd_pstate_epp_driver)
+ return 0;
+ cpufreq_unregister_driver(default_pstate_driver);
+ default_pstate_driver = &amd_pstate_epp_driver;
+ cppc_state = AMD_PSTATE_ACTIVE;
+ }
+
+ ret = cpufreq_register_driver(default_pstate_driver);
+ break;
+ default:
+ break;
+ ret = -EINVAL;
+ }
+
+ return ret;
+}
+
+static ssize_t show_status(struct kobject *kobj,
+ struct kobj_attribute *attr, char *buf)
+{
+ ssize_t ret;
+
+ mutex_lock(&amd_pstate_driver_lock);
+ ret = amd_pstate_show_status(buf);
+ mutex_unlock(&amd_pstate_driver_lock);
+
+ return ret;
+}
+
+static ssize_t store_status(struct kobject *a, struct kobj_attribute *b,
+ const char *buf, size_t count)
+{
+ char *p = memchr(buf, '\n', count);
+ int ret;
+
+ mutex_lock(&amd_pstate_driver_lock);
+ ret = amd_pstate_update_status(buf, p ? p - buf : count);
+ mutex_unlock(&amd_pstate_driver_lock);
+
+ return ret < 0 ? ret : count;
+}
+
cpufreq_freq_attr_ro(amd_pstate_max_freq);
cpufreq_freq_attr_ro(amd_pstate_lowest_nonlinear_freq);
cpufreq_freq_attr_ro(amd_pstate_highest_perf);
cpufreq_freq_attr_rw(energy_performance_preference);
cpufreq_freq_attr_ro(energy_performance_available_preferences);
+define_one_global_rw(status);
static struct freq_attr *amd_pstate_attr[] = {
&amd_pstate_max_freq,
@@ -795,6 +885,15 @@ static struct freq_attr *amd_pstate_epp_attr[] = {
NULL,
};
+static struct attribute *pstate_global_attributes[] = {
+ &status.attr,
+ NULL
+};
+
+static const struct attribute_group amd_pstate_global_attr_group = {
+ .attrs = pstate_global_attributes,
+};
+
static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
{
int min_freq, max_freq, nominal_freq, lowest_nonlinear_freq, ret;
@@ -1154,6 +1253,25 @@ static int __init amd_pstate_init(void)
if (ret)
pr_err("failed to register with return %d\n", ret);
+ amd_pstate_kobj = kobject_create_and_add("amd-pstate", &cpu_subsys.dev_root->kobj);
+ if (!amd_pstate_kobj) {
+ ret = -EINVAL;
+ pr_err("global sysfs registration failed.\n");
+ goto kobject_free;
+ }
+
+ ret = sysfs_create_group(amd_pstate_kobj, &amd_pstate_global_attr_group);
+ if (ret) {
+ pr_err("sysfs attribute export failed with error %d.\n", ret);
+ goto global_attr_free;
+ }
+
+ return ret;
+
+global_attr_free:
+ kobject_put(amd_pstate_kobj);
+kobject_free:
+ cpufreq_unregister_driver(default_pstate_driver);
return ret;
}
device_initcall(amd_pstate_init);
--
2.34.1
^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH v9 09/13] cpufreq: amd-pstate: add driver working mode switch support
2022-12-25 16:34 ` [PATCH v9 09/13] cpufreq: amd-pstate: add driver working mode switch support Perry Yuan
@ 2023-01-04 1:06 ` Limonciello, Mario
2023-01-05 6:02 ` Wyes Karny
` (3 subsequent siblings)
4 siblings, 0 replies; 40+ messages in thread
From: Limonciello, Mario @ 2023-01-04 1:06 UTC (permalink / raw)
To: Perry Yuan, rafael.j.wysocki, ray.huang, viresh.kumar
Cc: Deepak.Sharma, Nathan.Fontenot, Alexander.Deucher, Shimmer.Huang,
Xiaojian.Du, Li.Meng, wyes.karny, linux-pm, linux-kernel
On 12/25/2022 10:34, Perry Yuan wrote:
> From: Perry Yuan <Perry.Yuan@amd.com>
>
> While amd-pstate driver was loaded with specific driver mode, it will
> need to check which mode is enabled for the pstate driver,add this sysfs
> entry to show the current status
>
> $ cat /sys/devices/system/cpu/amd-pstate/status
> active
>
> Meanwhile, user can switch the pstate driver mode with writing mode
> string to sysfs entry as below.
>
> Enable passive mode:
> $ sudo bash -c "echo passive > /sys/devices/system/cpu/amd-pstate/status"
>
> Enable active mode (EPP driver mode):
> $ sudo bash -c "echo active > /sys/devices/system/cpu/amd-pstate/status"
>
> Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
> ---
> drivers/cpufreq/amd-pstate.c | 118 +++++++++++++++++++++++++++++++++++
> 1 file changed, 118 insertions(+)
>
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index e3676d1a85c7..8ceca4524fc1 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -63,6 +63,7 @@ static struct cpufreq_driver *default_pstate_driver;
> static struct cpufreq_driver amd_pstate_driver;
> static struct cpufreq_driver amd_pstate_epp_driver;
> static int cppc_state = AMD_PSTATE_DISABLE;
> +struct kobject *amd_pstate_kobj;
>
> static inline int get_mode_idx_from_str(const char *str, size_t size)
> {
> @@ -632,6 +633,8 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy)
> policy->driver_data = cpudata;
>
> amd_pstate_boost_init(cpudata);
> + if (!default_pstate_driver->adjust_perf)
As mentioned in the other patch I think this pointer should be renamed,
but otherwise this patch LGTM.
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> + default_pstate_driver->adjust_perf = amd_pstate_adjust_perf;
>
> return 0;
>
> @@ -772,12 +775,99 @@ static ssize_t show_energy_performance_preference(
> return sysfs_emit(buf, "%s\n", energy_perf_strings[preference]);
> }
>
> +static ssize_t amd_pstate_show_status(char *buf)
> +{
> + if (!default_pstate_driver)
> + return sysfs_emit(buf, "off\n");
> +
> + return sysfs_emit(buf, "%s\n", amd_pstate_mode_string[cppc_state]);
> +}
> +
> +static void amd_pstate_driver_cleanup(void)
> +{
> + default_pstate_driver = NULL;
> +}
> +
> +static int amd_pstate_update_status(const char *buf, size_t size)
> +{
> + int ret;
> + int mode_idx;
> +
> + if (size > 7 || size < 3)
> + return -EINVAL;
> + mode_idx = get_mode_idx_from_str(buf, size);
> +
> + switch(mode_idx) {
> + case AMD_PSTATE_DISABLE:
> + if (!default_pstate_driver)
> + return -EINVAL;
> + if (cppc_state == AMD_PSTATE_ACTIVE)
> + return -EBUSY;
> + ret = cpufreq_unregister_driver(default_pstate_driver);
> + amd_pstate_driver_cleanup();
> + break;
> + case AMD_PSTATE_PASSIVE:
> + if (default_pstate_driver) {
> + if (default_pstate_driver == &amd_pstate_driver)
> + return 0;
> + cpufreq_unregister_driver(default_pstate_driver);
> + cppc_state = AMD_PSTATE_PASSIVE;
> + default_pstate_driver = &amd_pstate_driver;
> + }
> +
> + ret = cpufreq_register_driver(default_pstate_driver);
> + break;
> + case AMD_PSTATE_ACTIVE:
> + if (default_pstate_driver) {
> + if (default_pstate_driver == &amd_pstate_epp_driver)
> + return 0;
> + cpufreq_unregister_driver(default_pstate_driver);
> + default_pstate_driver = &amd_pstate_epp_driver;
> + cppc_state = AMD_PSTATE_ACTIVE;
> + }
> +
> + ret = cpufreq_register_driver(default_pstate_driver);
> + break;
> + default:
> + break;
> + ret = -EINVAL;
> + }
> +
> + return ret;
> +}
> +
> +static ssize_t show_status(struct kobject *kobj,
> + struct kobj_attribute *attr, char *buf)
> +{
> + ssize_t ret;
> +
> + mutex_lock(&amd_pstate_driver_lock);
> + ret = amd_pstate_show_status(buf);
> + mutex_unlock(&amd_pstate_driver_lock);
> +
> + return ret;
> +}
> +
> +static ssize_t store_status(struct kobject *a, struct kobj_attribute *b,
> + const char *buf, size_t count)
> +{
> + char *p = memchr(buf, '\n', count);
> + int ret;
> +
> + mutex_lock(&amd_pstate_driver_lock);
> + ret = amd_pstate_update_status(buf, p ? p - buf : count);
> + mutex_unlock(&amd_pstate_driver_lock);
> +
> + return ret < 0 ? ret : count;
> +}
> +
> cpufreq_freq_attr_ro(amd_pstate_max_freq);
> cpufreq_freq_attr_ro(amd_pstate_lowest_nonlinear_freq);
>
> cpufreq_freq_attr_ro(amd_pstate_highest_perf);
> cpufreq_freq_attr_rw(energy_performance_preference);
> cpufreq_freq_attr_ro(energy_performance_available_preferences);
> +define_one_global_rw(status);
>
> static struct freq_attr *amd_pstate_attr[] = {
> &amd_pstate_max_freq,
> @@ -795,6 +885,15 @@ static struct freq_attr *amd_pstate_epp_attr[] = {
> NULL,
> };
>
> +static struct attribute *pstate_global_attributes[] = {
> + &status.attr,
> + NULL
> +};
> +
> +static const struct attribute_group amd_pstate_global_attr_group = {
> + .attrs = pstate_global_attributes,
> +};
> +
> static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
> {
> int min_freq, max_freq, nominal_freq, lowest_nonlinear_freq, ret;
> @@ -1154,6 +1253,25 @@ static int __init amd_pstate_init(void)
> if (ret)
> pr_err("failed to register with return %d\n", ret);
>
> + amd_pstate_kobj = kobject_create_and_add("amd-pstate", &cpu_subsys.dev_root->kobj);
> + if (!amd_pstate_kobj) {
> + ret = -EINVAL;
> + pr_err("global sysfs registration failed.\n");
> + goto kobject_free;
> + }
> +
> + ret = sysfs_create_group(amd_pstate_kobj, &amd_pstate_global_attr_group);
> + if (ret) {
> + pr_err("sysfs attribute export failed with error %d.\n", ret);
> + goto global_attr_free;
> + }
> +
> + return ret;
> +
> +global_attr_free:
> + kobject_put(amd_pstate_kobj);
> +kobject_free:
> + cpufreq_unregister_driver(default_pstate_driver);
> return ret;
> }
> device_initcall(amd_pstate_init);
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v9 09/13] cpufreq: amd-pstate: add driver working mode switch support
2022-12-25 16:34 ` [PATCH v9 09/13] cpufreq: amd-pstate: add driver working mode switch support Perry Yuan
2023-01-04 1:06 ` Limonciello, Mario
@ 2023-01-05 6:02 ` Wyes Karny
2023-01-05 6:57 ` Yuan, Perry
2023-01-05 7:29 ` Wyes Karny
` (2 subsequent siblings)
4 siblings, 1 reply; 40+ messages in thread
From: Wyes Karny @ 2023-01-05 6:02 UTC (permalink / raw)
To: Perry Yuan, rafael.j.wysocki, Mario.Limonciello, ray.huang, viresh.kumar
Cc: Deepak.Sharma, Nathan.Fontenot, Alexander.Deucher, Shimmer.Huang,
Xiaojian.Du, Li.Meng, linux-pm, linux-kernel
Hi Perry,
On 12/25/2022 10:04 PM, Perry Yuan wrote:
> From: Perry Yuan <Perry.Yuan@amd.com>
>
> While amd-pstate driver was loaded with specific driver mode, it will
> need to check which mode is enabled for the pstate driver,add this sysfs
> entry to show the current status
>
> $ cat /sys/devices/system/cpu/amd-pstate/status
> active
>
> Meanwhile, user can switch the pstate driver mode with writing mode
> string to sysfs entry as below.
>
> Enable passive mode:
> $ sudo bash -c "echo passive > /sys/devices/system/cpu/amd-pstate/status"
>
> Enable active mode (EPP driver mode):
> $ sudo bash -c "echo active > /sys/devices/system/cpu/amd-pstate/status"
>
> Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
> ---
> drivers/cpufreq/amd-pstate.c | 118 +++++++++++++++++++++++++++++++++++
> 1 file changed, 118 insertions(+)
>
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index e3676d1a85c7..8ceca4524fc1 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -63,6 +63,7 @@ static struct cpufreq_driver *default_pstate_driver;
> static struct cpufreq_driver amd_pstate_driver;
> static struct cpufreq_driver amd_pstate_epp_driver;
> static int cppc_state = AMD_PSTATE_DISABLE;
> +struct kobject *amd_pstate_kobj;
>
> static inline int get_mode_idx_from_str(const char *str, size_t size)
> {
> @@ -632,6 +633,8 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy)
> policy->driver_data = cpudata;
>
> amd_pstate_boost_init(cpudata);
> + if (!default_pstate_driver->adjust_perf)
> + default_pstate_driver->adjust_perf = amd_pstate_adjust_perf;
>
> return 0;
>
> @@ -772,12 +775,99 @@ static ssize_t show_energy_performance_preference(
> return sysfs_emit(buf, "%s\n", energy_perf_strings[preference]);
> }
>
> +static ssize_t amd_pstate_show_status(char *buf)
> +{
> + if (!default_pstate_driver)
> + return sysfs_emit(buf, "off\n");
IMO 'disable' is more consistent to cppc_state.
> +
> + return sysfs_emit(buf, "%s\n", amd_pstate_mode_string[cppc_state]);
> +}
> +
> +static void amd_pstate_driver_cleanup(void)
> +{
> + default_pstate_driver = NULL;
> +}
> +
> +static int amd_pstate_update_status(const char *buf, size_t size)
> +{
> + int ret;
> + int mode_idx;
> +
> + if (size > 7 || size < 3)
> + return -EINVAL;
> + mode_idx = get_mode_idx_from_str(buf, size);
> +
> + switch(mode_idx) {
> + case AMD_PSTATE_DISABLE:
> + if (!default_pstate_driver)
> + return -EINVAL;
> + if (cppc_state == AMD_PSTATE_ACTIVE)
> + return -EBUSY;
> + ret = cpufreq_unregister_driver(default_pstate_driver);
> + amd_pstate_driver_cleanup();
> + break;
> + case AMD_PSTATE_PASSIVE:
> + if (default_pstate_driver) {
> + if (default_pstate_driver == &amd_pstate_driver)
> + return 0;
> + cpufreq_unregister_driver(default_pstate_driver);
> + cppc_state = AMD_PSTATE_PASSIVE;
> + default_pstate_driver = &amd_pstate_driver;
> + }
> +
> + ret = cpufreq_register_driver(default_pstate_driver);
> + break;
> + case AMD_PSTATE_ACTIVE:
> + if (default_pstate_driver) {
> + if (default_pstate_driver == &amd_pstate_epp_driver)
> + return 0;
> + cpufreq_unregister_driver(default_pstate_driver);
> + default_pstate_driver = &amd_pstate_epp_driver;
> + cppc_state = AMD_PSTATE_ACTIVE;
> + }
> +
> + ret = cpufreq_register_driver(default_pstate_driver);
> + break;
> + default:
> + break;
> + ret = -EINVAL;
> + }
> +
> + return ret;
> +}
The implementation of amd_pstate_update_status function is good as long as the possible states are less.
Currently prev_state and next_state has 9 combinations. But with guided mode this becomes 16 combinations.
Do you have any concerns if I optimize this function by creating a state transition table in guided patch series?
> +
> +static ssize_t show_status(struct kobject *kobj,
> + struct kobj_attribute *attr, char *buf)
> +{
> + ssize_t ret;
> +
> + mutex_lock(&amd_pstate_driver_lock);
> + ret = amd_pstate_show_status(buf);
> + mutex_unlock(&amd_pstate_driver_lock);
> +
> + return ret;
> +}
> +
> +static ssize_t store_status(struct kobject *a, struct kobj_attribute *b,
> + const char *buf, size_t count)
> +{
> + char *p = memchr(buf, '\n', count);
> + int ret;
> +
> + mutex_lock(&amd_pstate_driver_lock);
> + ret = amd_pstate_update_status(buf, p ? p - buf : count);
> + mutex_unlock(&amd_pstate_driver_lock);
> +
> + return ret < 0 ? ret : count;
> +}
> +
> cpufreq_freq_attr_ro(amd_pstate_max_freq);
> cpufreq_freq_attr_ro(amd_pstate_lowest_nonlinear_freq);
>
> cpufreq_freq_attr_ro(amd_pstate_highest_perf);
> cpufreq_freq_attr_rw(energy_performance_preference);
> cpufreq_freq_attr_ro(energy_performance_available_preferences);
> +define_one_global_rw(status);
>
> static struct freq_attr *amd_pstate_attr[] = {
> &amd_pstate_max_freq,
> @@ -795,6 +885,15 @@ static struct freq_attr *amd_pstate_epp_attr[] = {
> NULL,
> };
>
> +static struct attribute *pstate_global_attributes[] = {
> + &status.attr,
> + NULL
> +};
> +
> +static const struct attribute_group amd_pstate_global_attr_group = {
> + .attrs = pstate_global_attributes,
> +};
> +
> static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
> {
> int min_freq, max_freq, nominal_freq, lowest_nonlinear_freq, ret;
> @@ -1154,6 +1253,25 @@ static int __init amd_pstate_init(void)
> if (ret)
> pr_err("failed to register with return %d\n", ret);
>
> + amd_pstate_kobj = kobject_create_and_add("amd-pstate", &cpu_subsys.dev_root->kobj);
> + if (!amd_pstate_kobj) {
> + ret = -EINVAL;
> + pr_err("global sysfs registration failed.\n");
> + goto kobject_free;
> + }
> +
> + ret = sysfs_create_group(amd_pstate_kobj, &amd_pstate_global_attr_group);
> + if (ret) {
> + pr_err("sysfs attribute export failed with error %d.\n", ret);
> + goto global_attr_free;
> + }
> +
> + return ret;
> +
> +global_attr_free:
> + kobject_put(amd_pstate_kobj);
> +kobject_free:
> + cpufreq_unregister_driver(default_pstate_driver);
> return ret;
> }
> device_initcall(amd_pstate_init);
--
Thanks & Regards,
Wyes
^ permalink raw reply [flat|nested] 40+ messages in thread
* RE: [PATCH v9 09/13] cpufreq: amd-pstate: add driver working mode switch support
2023-01-05 6:02 ` Wyes Karny
@ 2023-01-05 6:57 ` Yuan, Perry
0 siblings, 0 replies; 40+ messages in thread
From: Yuan, Perry @ 2023-01-05 6:57 UTC (permalink / raw)
To: Karny, Wyes, rafael.j.wysocki, Limonciello, Mario, Huang, Ray,
viresh.kumar
Cc: Sharma, Deepak, Fontenot, Nathan, Deucher, Alexander, Huang,
Shimmer, Du, Xiaojian, Meng, Li (Jassmine),
linux-pm, linux-kernel
[AMD Official Use Only - General]
> -----Original Message-----
> From: Karny, Wyes <Wyes.Karny@amd.com>
> Sent: Thursday, January 5, 2023 2:02 PM
> To: Yuan, Perry <Perry.Yuan@amd.com>; rafael.j.wysocki@intel.com;
> Limonciello, Mario <Mario.Limonciello@amd.com>; Huang, Ray
> <Ray.Huang@amd.com>; viresh.kumar@linaro.org
> Cc: Sharma, Deepak <Deepak.Sharma@amd.com>; Fontenot, Nathan
> <Nathan.Fontenot@amd.com>; Deucher, Alexander
> <Alexander.Deucher@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 v9 09/13] cpufreq: amd-pstate: add driver working mode
> switch support
>
> Hi Perry,
>
> On 12/25/2022 10:04 PM, Perry Yuan wrote:
> > From: Perry Yuan <Perry.Yuan@amd.com>
> >
> > While amd-pstate driver was loaded with specific driver mode, it will
> > need to check which mode is enabled for the pstate driver,add this
> > sysfs entry to show the current status
> >
> > $ cat /sys/devices/system/cpu/amd-pstate/status
> > active
> >
> > Meanwhile, user can switch the pstate driver mode with writing mode
> > string to sysfs entry as below.
> >
> > Enable passive mode:
> > $ sudo bash -c "echo passive > /sys/devices/system/cpu/amd-
> pstate/status"
> >
> > Enable active mode (EPP driver mode):
> > $ sudo bash -c "echo active > /sys/devices/system/cpu/amd-pstate/status"
> >
> > Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
> > ---
> > drivers/cpufreq/amd-pstate.c | 118
> > +++++++++++++++++++++++++++++++++++
> > 1 file changed, 118 insertions(+)
> >
> > diff --git a/drivers/cpufreq/amd-pstate.c
> > b/drivers/cpufreq/amd-pstate.c index e3676d1a85c7..8ceca4524fc1 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -63,6 +63,7 @@ static struct cpufreq_driver *default_pstate_driver;
> > static struct cpufreq_driver amd_pstate_driver; static struct
> > cpufreq_driver amd_pstate_epp_driver; static int cppc_state =
> > AMD_PSTATE_DISABLE;
> > +struct kobject *amd_pstate_kobj;
> >
> > static inline int get_mode_idx_from_str(const char *str, size_t size)
> > { @@ -632,6 +633,8 @@ static int amd_pstate_cpu_init(struct
> > cpufreq_policy *policy)
> > policy->driver_data = cpudata;
> >
> > amd_pstate_boost_init(cpudata);
> > + if (!default_pstate_driver->adjust_perf)
> > + default_pstate_driver->adjust_perf =
> amd_pstate_adjust_perf;
> >
> > return 0;
> >
> > @@ -772,12 +775,99 @@ static ssize_t
> show_energy_performance_preference(
> > return sysfs_emit(buf, "%s\n", energy_perf_strings[preference]); }
> >
> > +static ssize_t amd_pstate_show_status(char *buf) {
> > + if (!default_pstate_driver)
> > + return sysfs_emit(buf, "off\n");
>
> IMO 'disable' is more consistent to cppc_state.
Will change the string to be "disable" in v10.
>
> > +
> > + return sysfs_emit(buf, "%s\n",
> amd_pstate_mode_string[cppc_state]);
> > +}
> > +
> > +static void amd_pstate_driver_cleanup(void) {
> > + default_pstate_driver = NULL;
> > +}
> > +
> > +static int amd_pstate_update_status(const char *buf, size_t size) {
> > + int ret;
> > + int mode_idx;
> > +
> > + if (size > 7 || size < 3)
> > + return -EINVAL;
> > + mode_idx = get_mode_idx_from_str(buf, size);
> > +
> > + switch(mode_idx) {
> > + case AMD_PSTATE_DISABLE:
> > + if (!default_pstate_driver)
> > + return -EINVAL;
> > + if (cppc_state == AMD_PSTATE_ACTIVE)
> > + return -EBUSY;
> > + ret = cpufreq_unregister_driver(default_pstate_driver);
> > + amd_pstate_driver_cleanup();
> > + break;
> > + case AMD_PSTATE_PASSIVE:
> > + if (default_pstate_driver) {
> > + if (default_pstate_driver == &amd_pstate_driver)
> > + return 0;
> > + cpufreq_unregister_driver(default_pstate_driver);
> > + cppc_state = AMD_PSTATE_PASSIVE;
> > + default_pstate_driver = &amd_pstate_driver;
> > + }
> > +
> > + ret = cpufreq_register_driver(default_pstate_driver);
> > + break;
> > + case AMD_PSTATE_ACTIVE:
> > + if (default_pstate_driver) {
> > + if (default_pstate_driver ==
> &amd_pstate_epp_driver)
> > + return 0;
> > + cpufreq_unregister_driver(default_pstate_driver);
> > + default_pstate_driver = &amd_pstate_epp_driver;
> > + cppc_state = AMD_PSTATE_ACTIVE;
> > + }
> > +
> > + ret = cpufreq_register_driver(default_pstate_driver);
> > + break;
> > + default:
> > + break;
> > + ret = -EINVAL;
> > + }
> > +
> > + return ret;
> > +}
>
> The implementation of amd_pstate_update_status function is good as long
> as the possible states are less.
> Currently prev_state and next_state has 9 combinations. But with guided
> mode this becomes 16 combinations.
> Do you have any concerns if I optimize this function by creating a state
> transition table in guided patch series?
No concern , please go ahead in your series.
>
> > +
> > +static ssize_t show_status(struct kobject *kobj,
> > + struct kobj_attribute *attr, char *buf) {
> > + ssize_t ret;
> > +
> > + mutex_lock(&amd_pstate_driver_lock);
> > + ret = amd_pstate_show_status(buf);
> > + mutex_unlock(&amd_pstate_driver_lock);
> > +
> > + return ret;
> > +}
> > +
> > +static ssize_t store_status(struct kobject *a, struct kobj_attribute *b,
> > + const char *buf, size_t count) {
> > + char *p = memchr(buf, '\n', count);
> > + int ret;
> > +
> > + mutex_lock(&amd_pstate_driver_lock);
> > + ret = amd_pstate_update_status(buf, p ? p - buf : count);
> > + mutex_unlock(&amd_pstate_driver_lock);
> > +
> > + return ret < 0 ? ret : count;
> > +}
> > +
> > cpufreq_freq_attr_ro(amd_pstate_max_freq);
> > cpufreq_freq_attr_ro(amd_pstate_lowest_nonlinear_freq);
> >
> > cpufreq_freq_attr_ro(amd_pstate_highest_perf);
> > cpufreq_freq_attr_rw(energy_performance_preference);
> > cpufreq_freq_attr_ro(energy_performance_available_preferences);
> > +define_one_global_rw(status);
> >
> > static struct freq_attr *amd_pstate_attr[] = {
> > &amd_pstate_max_freq,
> > @@ -795,6 +885,15 @@ static struct freq_attr *amd_pstate_epp_attr[] = {
> > NULL,
> > };
> >
> > +static struct attribute *pstate_global_attributes[] = {
> > + &status.attr,
> > + NULL
> > +};
> > +
> > +static const struct attribute_group amd_pstate_global_attr_group = {
> > + .attrs = pstate_global_attributes,
> > +};
> > +
> > static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy) {
> > int min_freq, max_freq, nominal_freq, lowest_nonlinear_freq, ret;
> @@
> > -1154,6 +1253,25 @@ static int __init amd_pstate_init(void)
> > if (ret)
> > pr_err("failed to register with return %d\n", ret);
> >
> > + amd_pstate_kobj = kobject_create_and_add("amd-pstate",
> &cpu_subsys.dev_root->kobj);
> > + if (!amd_pstate_kobj) {
> > + ret = -EINVAL;
> > + pr_err("global sysfs registration failed.\n");
> > + goto kobject_free;
> > + }
> > +
> > + ret = sysfs_create_group(amd_pstate_kobj,
> &amd_pstate_global_attr_group);
> > + if (ret) {
> > + pr_err("sysfs attribute export failed with error %d.\n", ret);
> > + goto global_attr_free;
> > + }
> > +
> > + return ret;
> > +
> > +global_attr_free:
> > + kobject_put(amd_pstate_kobj);
> > +kobject_free:
> > + cpufreq_unregister_driver(default_pstate_driver);
> > return ret;
> > }
> > device_initcall(amd_pstate_init);
>
> --
> Thanks & Regards,
> Wyes
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v9 09/13] cpufreq: amd-pstate: add driver working mode switch support
2022-12-25 16:34 ` [PATCH v9 09/13] cpufreq: amd-pstate: add driver working mode switch support Perry Yuan
2023-01-04 1:06 ` Limonciello, Mario
2023-01-05 6:02 ` Wyes Karny
@ 2023-01-05 7:29 ` Wyes Karny
2023-01-05 16:03 ` Wyes Karny
2023-01-06 3:46 ` Wyes Karny
4 siblings, 0 replies; 40+ messages in thread
From: Wyes Karny @ 2023-01-05 7:29 UTC (permalink / raw)
To: Perry Yuan, rafael.j.wysocki, Mario.Limonciello, ray.huang, viresh.kumar
Cc: Deepak.Sharma, Nathan.Fontenot, Alexander.Deucher, Shimmer.Huang,
Xiaojian.Du, Li.Meng, linux-pm, linux-kernel
Hi Perry,
On 12/25/2022 10:04 PM, Perry Yuan wrote:
> From: Perry Yuan <Perry.Yuan@amd.com>
>
> While amd-pstate driver was loaded with specific driver mode, it will
> need to check which mode is enabled for the pstate driver,add this sysfs
> entry to show the current status
>
> $ cat /sys/devices/system/cpu/amd-pstate/status
> active
>
> Meanwhile, user can switch the pstate driver mode with writing mode
> string to sysfs entry as below.
>
> Enable passive mode:
> $ sudo bash -c "echo passive > /sys/devices/system/cpu/amd-pstate/status"
>
> Enable active mode (EPP driver mode):
> $ sudo bash -c "echo active > /sys/devices/system/cpu/amd-pstate/status"
>
> Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
> ---
> drivers/cpufreq/amd-pstate.c | 118 +++++++++++++++++++++++++++++++++++
> 1 file changed, 118 insertions(+)
>
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index e3676d1a85c7..8ceca4524fc1 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -63,6 +63,7 @@ static struct cpufreq_driver *default_pstate_driver;
> static struct cpufreq_driver amd_pstate_driver;
> static struct cpufreq_driver amd_pstate_epp_driver;
> static int cppc_state = AMD_PSTATE_DISABLE;
> +struct kobject *amd_pstate_kobj;
>
> static inline int get_mode_idx_from_str(const char *str, size_t size)
> {
> @@ -632,6 +633,8 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy)
> policy->driver_data = cpudata;
>
> amd_pstate_boost_init(cpudata);
> + if (!default_pstate_driver->adjust_perf)
> + default_pstate_driver->adjust_perf = amd_pstate_adjust_perf;
>
> return 0;
>
> @@ -772,12 +775,99 @@ static ssize_t show_energy_performance_preference(
> return sysfs_emit(buf, "%s\n", energy_perf_strings[preference]);
> }
>
> +static ssize_t amd_pstate_show_status(char *buf)
> +{
> + if (!default_pstate_driver)
> + return sysfs_emit(buf, "off\n");
> +
> + return sysfs_emit(buf, "%s\n", amd_pstate_mode_string[cppc_state]);
> +}
> +
> +static void amd_pstate_driver_cleanup(void)
> +{
> + default_pstate_driver = NULL;
> +}
> +
> +static int amd_pstate_update_status(const char *buf, size_t size)
> +{
> + int ret;
> + int mode_idx;
> +
> + if (size > 7 || size < 3)
> + return -EINVAL;
> + mode_idx = get_mode_idx_from_str(buf, size);
> +
> + switch(mode_idx) {
> + case AMD_PSTATE_DISABLE:
> + if (!default_pstate_driver)
> + return -EINVAL;
> + if (cppc_state == AMD_PSTATE_ACTIVE)
> + return -EBUSY;
> + ret = cpufreq_unregister_driver(default_pstate_driver);
> + amd_pstate_driver_cleanup();
> + break;
> + case AMD_PSTATE_PASSIVE:
> + if (default_pstate_driver) {
> + if (default_pstate_driver == &amd_pstate_driver)
> + return 0;
> + cpufreq_unregister_driver(default_pstate_driver);
> + cppc_state = AMD_PSTATE_PASSIVE;
> + default_pstate_driver = &amd_pstate_driver;
> + }
> +
> + ret = cpufreq_register_driver(default_pstate_driver);
> + break;
> + case AMD_PSTATE_ACTIVE:
> + if (default_pstate_driver) {
> + if (default_pstate_driver == &amd_pstate_epp_driver)
> + return 0;
> + cpufreq_unregister_driver(default_pstate_driver);
> + default_pstate_driver = &amd_pstate_epp_driver;
> + cppc_state = AMD_PSTATE_ACTIVE;
> + }
> +
> + ret = cpufreq_register_driver(default_pstate_driver);
> + break;
> + default:
> + break;
> + ret = -EINVAL;
> + }
> +
> + return ret;
> +}
> +
> +static ssize_t show_status(struct kobject *kobj,
> + struct kobj_attribute *attr, char *buf)
> +{
> + ssize_t ret;
> +
> + mutex_lock(&amd_pstate_driver_lock);
> + ret = amd_pstate_show_status(buf);
> + mutex_unlock(&amd_pstate_driver_lock);
> +
> + return ret;
> +}
> +
> +static ssize_t store_status(struct kobject *a, struct kobj_attribute *b,
> + const char *buf, size_t count)
> +{
> + char *p = memchr(buf, '\n', count);
> + int ret;
> +
> + mutex_lock(&amd_pstate_driver_lock);
> + ret = amd_pstate_update_status(buf, p ? p - buf : count);
> + mutex_unlock(&amd_pstate_driver_lock);
> +
> + return ret < 0 ? ret : count;
> +}
> +
> cpufreq_freq_attr_ro(amd_pstate_max_freq);
> cpufreq_freq_attr_ro(amd_pstate_lowest_nonlinear_freq);
>
> cpufreq_freq_attr_ro(amd_pstate_highest_perf);
> cpufreq_freq_attr_rw(energy_performance_preference);
> cpufreq_freq_attr_ro(energy_performance_available_preferences);
> +define_one_global_rw(status);
>
> static struct freq_attr *amd_pstate_attr[] = {
> &amd_pstate_max_freq,
> @@ -795,6 +885,15 @@ static struct freq_attr *amd_pstate_epp_attr[] = {
> NULL,
> };
>
> +static struct attribute *pstate_global_attributes[] = {
> + &status.attr,
> + NULL
> +};
> +
> +static const struct attribute_group amd_pstate_global_attr_group = {
> + .attrs = pstate_global_attributes,
> +};
> +
> static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
> {
> int min_freq, max_freq, nominal_freq, lowest_nonlinear_freq, ret;
> @@ -1154,6 +1253,25 @@ static int __init amd_pstate_init(void)
> if (ret)
> pr_err("failed to register with return %d\n", ret);
>
> + amd_pstate_kobj = kobject_create_and_add("amd-pstate", &cpu_subsys.dev_root->kobj);
IMO amd_pstate would make it more consistent because the boot param is called "amd_pstate".
But no strong opinion on this.
> + if (!amd_pstate_kobj) {
> + ret = -EINVAL;
> + pr_err("global sysfs registration failed.\n");
> + goto kobject_free;
> + }
> +
> + ret = sysfs_create_group(amd_pstate_kobj, &amd_pstate_global_attr_group);
> + if (ret) {
> + pr_err("sysfs attribute export failed with error %d.\n", ret);
> + goto global_attr_free;
> + }
> +
> + return ret;
> +
> +global_attr_free:
> + kobject_put(amd_pstate_kobj);
> +kobject_free:
> + cpufreq_unregister_driver(default_pstate_driver);
> return ret;
> }
> device_initcall(amd_pstate_init);
--
Thanks & Regards,
Wyes
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v9 09/13] cpufreq: amd-pstate: add driver working mode switch support
2022-12-25 16:34 ` [PATCH v9 09/13] cpufreq: amd-pstate: add driver working mode switch support Perry Yuan
` (2 preceding siblings ...)
2023-01-05 7:29 ` Wyes Karny
@ 2023-01-05 16:03 ` Wyes Karny
2023-01-06 2:52 ` Yuan, Perry
2023-01-06 3:46 ` Wyes Karny
4 siblings, 1 reply; 40+ messages in thread
From: Wyes Karny @ 2023-01-05 16:03 UTC (permalink / raw)
To: Perry Yuan, rafael.j.wysocki, Mario.Limonciello, ray.huang, viresh.kumar
Cc: Deepak.Sharma, Nathan.Fontenot, Alexander.Deucher, Shimmer.Huang,
Xiaojian.Du, Li.Meng, linux-pm, linux-kernel
Hi Perry,
On 12/25/2022 10:04 PM, Perry Yuan wrote:
-------------------------------->8--------------------------------
> +
> +static int amd_pstate_update_status(const char *buf, size_t size)
> +{
> + int ret;
> + int mode_idx;
> +
> + if (size > 7 || size < 3)
> + return -EINVAL;
> + mode_idx = get_mode_idx_from_str(buf, size);
if (size > 7 || size < 6) right?
because possible strings are: "disable", "passive" and "active".
--
Thanks & Regards,
Wyes
^ permalink raw reply [flat|nested] 40+ messages in thread
* RE: [PATCH v9 09/13] cpufreq: amd-pstate: add driver working mode switch support
2023-01-05 16:03 ` Wyes Karny
@ 2023-01-06 2:52 ` Yuan, Perry
0 siblings, 0 replies; 40+ messages in thread
From: Yuan, Perry @ 2023-01-06 2:52 UTC (permalink / raw)
To: Karny, Wyes, rafael.j.wysocki, Limonciello, Mario, Huang, Ray,
viresh.kumar
Cc: Sharma, Deepak, Fontenot, Nathan, Deucher, Alexander, Huang,
Shimmer, Du, Xiaojian, Meng, Li (Jassmine),
linux-pm, linux-kernel
[AMD Official Use Only - General]
Hi Wyes.
> -----Original Message-----
> From: Karny, Wyes <Wyes.Karny@amd.com>
> Sent: Friday, January 6, 2023 12:04 AM
> To: Yuan, Perry <Perry.Yuan@amd.com>; rafael.j.wysocki@intel.com;
> Limonciello, Mario <Mario.Limonciello@amd.com>; Huang, Ray
> <Ray.Huang@amd.com>; viresh.kumar@linaro.org
> Cc: Sharma, Deepak <Deepak.Sharma@amd.com>; Fontenot, Nathan
> <Nathan.Fontenot@amd.com>; Deucher, Alexander
> <Alexander.Deucher@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 v9 09/13] cpufreq: amd-pstate: add driver working mode
> switch support
>
> Hi Perry,
>
> On 12/25/2022 10:04 PM, Perry Yuan wrote:
> -------------------------------->8--------------------------------
> > +
> > +static int amd_pstate_update_status(const char *buf, size_t size) {
> > + int ret;
> > + int mode_idx;
> > +
> > + if (size > 7 || size < 3)
> > + return -EINVAL;
> > + mode_idx = get_mode_idx_from_str(buf, size);
>
> if (size > 7 || size < 6) right?
> because possible strings are: "disable", "passive" and "active".
>
Since I remove the "off" string for the v10 series, So Yes, It will be good to update the size checking.
Perry.
> --
> Thanks & Regards,
> Wyes
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v9 09/13] cpufreq: amd-pstate: add driver working mode switch support
2022-12-25 16:34 ` [PATCH v9 09/13] cpufreq: amd-pstate: add driver working mode switch support Perry Yuan
` (3 preceding siblings ...)
2023-01-05 16:03 ` Wyes Karny
@ 2023-01-06 3:46 ` Wyes Karny
4 siblings, 0 replies; 40+ messages in thread
From: Wyes Karny @ 2023-01-06 3:46 UTC (permalink / raw)
To: Perry Yuan, rafael.j.wysocki, Mario.Limonciello, ray.huang, viresh.kumar
Cc: Deepak.Sharma, Nathan.Fontenot, Alexander.Deucher, Shimmer.Huang,
Xiaojian.Du, Li.Meng, linux-pm, linux-kernel
Hi Perry,
On 12/25/2022 10:04 PM, Perry Yuan wrote:
----------------------------------->8----------------------------------
> +static int amd_pstate_update_status(const char *buf, size_t size)
> +{
> + int ret;
> + int mode_idx;
> +
> + if (size > 7 || size < 3)
> + return -EINVAL;
> + mode_idx = get_mode_idx_from_str(buf, size);
> +
> + switch(mode_idx) {
> + case AMD_PSTATE_DISABLE:
> + if (!default_pstate_driver)
> + return -EINVAL;
> + if (cppc_state == AMD_PSTATE_ACTIVE)
> + return -EBUSY;
what's the reason for not supporting active -> disable?
> + ret = cpufreq_unregister_driver(default_pstate_driver);
> + amd_pstate_driver_cleanup();
> + break;
> + case AMD_PSTATE_PASSIVE:
> + if (default_pstate_driver) {
> + if (default_pstate_driver == &amd_pstate_driver)
> + return 0;
> + cpufreq_unregister_driver(default_pstate_driver);
> + cppc_state = AMD_PSTATE_PASSIVE;
> + default_pstate_driver = &amd_pstate_driver;
> + }
> +
> + ret = cpufreq_register_driver(default_pstate_driver);
> + break;
> + case AMD_PSTATE_ACTIVE:
> + if (default_pstate_driver) {
> + if (default_pstate_driver == &amd_pstate_epp_driver)
> + return 0;
> + cpufreq_unregister_driver(default_pstate_driver);
> + default_pstate_driver = &amd_pstate_epp_driver;
> + cppc_state = AMD_PSTATE_ACTIVE;
> + }
> +
> + ret = cpufreq_register_driver(default_pstate_driver);
> + break;
> + default:
> + break;
> + ret = -EINVAL;
> + }
> +
> + return ret;
> +}
> +
> +static ssize_t show_status(struct kobject *kobj,
> + struct kobj_attribute *attr, char *buf)
> +{
> + ssize_t ret;
> +
> + mutex_lock(&amd_pstate_driver_lock);
> + ret = amd_pstate_show_status(buf);
> + mutex_unlock(&amd_pstate_driver_lock);
> +
> + return ret;
> +}
> +
> +static ssize_t store_status(struct kobject *a, struct kobj_attribute *b,
> + const char *buf, size_t count)
> +{
> + char *p = memchr(buf, '\n', count);
> + int ret;
> +
> + mutex_lock(&amd_pstate_driver_lock);
> + ret = amd_pstate_update_status(buf, p ? p - buf : count);
> + mutex_unlock(&amd_pstate_driver_lock);
> +
> + return ret < 0 ? ret : count;
> +}
> +
> cpufreq_freq_attr_ro(amd_pstate_max_freq);
> cpufreq_freq_attr_ro(amd_pstate_lowest_nonlinear_freq);
>
> cpufreq_freq_attr_ro(amd_pstate_highest_perf);
> cpufreq_freq_attr_rw(energy_performance_preference);
> cpufreq_freq_attr_ro(energy_performance_available_preferences);
> +define_one_global_rw(status);
>
> static struct freq_attr *amd_pstate_attr[] = {
> &amd_pstate_max_freq,
> @@ -795,6 +885,15 @@ static struct freq_attr *amd_pstate_epp_attr[] = {
> NULL,
> };
>
> +static struct attribute *pstate_global_attributes[] = {
> + &status.attr,
> + NULL
> +};
> +
> +static const struct attribute_group amd_pstate_global_attr_group = {
> + .attrs = pstate_global_attributes,
> +};
> +
> static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
> {
> int min_freq, max_freq, nominal_freq, lowest_nonlinear_freq, ret;
> @@ -1154,6 +1253,25 @@ static int __init amd_pstate_init(void)
> if (ret)
> pr_err("failed to register with return %d\n", ret);
>
> + amd_pstate_kobj = kobject_create_and_add("amd-pstate", &cpu_subsys.dev_root->kobj);
> + if (!amd_pstate_kobj) {
> + ret = -EINVAL;
> + pr_err("global sysfs registration failed.\n");
> + goto kobject_free;
> + }
> +
> + ret = sysfs_create_group(amd_pstate_kobj, &amd_pstate_global_attr_group);
> + if (ret) {
> + pr_err("sysfs attribute export failed with error %d.\n", ret);
> + goto global_attr_free;
> + }
> +
> + return ret;
> +
> +global_attr_free:
> + kobject_put(amd_pstate_kobj);
> +kobject_free:
> + cpufreq_unregister_driver(default_pstate_driver);
> return ret;
> }
> device_initcall(amd_pstate_init);
--
Thanks & Regards,
Wyes
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v9 10/13] Documentation: amd-pstate: add amd pstate driver mode introduction
2022-12-25 16:34 [PATCH v9 00/13] Implement AMD Pstate EPP Driver Perry Yuan
` (8 preceding siblings ...)
2022-12-25 16:34 ` [PATCH v9 09/13] cpufreq: amd-pstate: add driver working mode switch support Perry Yuan
@ 2022-12-25 16:34 ` Perry Yuan
2022-12-25 16:34 ` [PATCH v9 11/13] Documentation: introduce amd pstate active mode kernel command line options Perry Yuan
` (2 subsequent siblings)
12 siblings, 0 replies; 40+ messages in thread
From: Perry Yuan @ 2022-12-25 16:34 UTC (permalink / raw)
To: rafael.j.wysocki, Mario.Limonciello, ray.huang, viresh.kumar
Cc: Deepak.Sharma, Nathan.Fontenot, Alexander.Deucher, Shimmer.Huang,
Xiaojian.Du, Li.Meng, wyes.karny, linux-pm, linux-kernel
From: Perry Yuan <Perry.Yuan@amd.com>
The amd-pstate driver has two operation modes supported:
* CPPC Autonomous (active) mode
* CPPC non-autonomous (passive) mode.
active mode and passive mode can be chosen by different kernel parameters.
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
---
Documentation/admin-guide/pm/amd-pstate.rst | 26 +++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/Documentation/admin-guide/pm/amd-pstate.rst b/Documentation/admin-guide/pm/amd-pstate.rst
index 33ab8ec8fc2f..62744dae3c5f 100644
--- a/Documentation/admin-guide/pm/amd-pstate.rst
+++ b/Documentation/admin-guide/pm/amd-pstate.rst
@@ -299,8 +299,30 @@ module which supports the new AMD P-States mechanism on most of the future AMD
platforms. The AMD P-States mechanism is the more performance and energy
efficiency frequency management method on AMD processors.
-Kernel Module Options for ``amd-pstate``
-=========================================
+
+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.
+
+Active Mode
+------------
+
+``amd_pstate=active``
+
+This is the low-level firmware control mode which is implemented by ``amd_pstate_epp``
+driver with ``amd_pstate=active`` passed to the kernel in the command line.
+In this mode, ``amd_pstate_epp`` 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 according to the power supply and thermal, core voltage and some other
+hardware conditions.
Passive Mode
------------
--
2.34.1
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH v9 11/13] Documentation: introduce amd pstate active mode kernel command line options
2022-12-25 16:34 [PATCH v9 00/13] Implement AMD Pstate EPP Driver Perry Yuan
` (9 preceding siblings ...)
2022-12-25 16:34 ` [PATCH v9 10/13] Documentation: amd-pstate: add amd pstate driver mode introduction Perry Yuan
@ 2022-12-25 16:34 ` Perry Yuan
2022-12-25 16:34 ` [PATCH v9 12/13] cpufreq: amd-pstate: convert sprintf with sysfs_emit() Perry Yuan
2022-12-25 16:34 ` [PATCH v9 13/13] Documentation: amd-pstate: introduce new global sysfs attributes Perry Yuan
12 siblings, 0 replies; 40+ messages in thread
From: Perry Yuan @ 2022-12-25 16:34 UTC (permalink / raw)
To: rafael.j.wysocki, Mario.Limonciello, ray.huang, viresh.kumar
Cc: Deepak.Sharma, Nathan.Fontenot, Alexander.Deucher, Shimmer.Huang,
Xiaojian.Du, Li.Meng, wyes.karny, linux-pm, linux-kernel
AMD Pstate driver support another firmware based autonomous mode
with "amd_pstate=active" added to the kernel command line.
In autonomous mode SMU firmware decides frequencies at runtime
based on workload utilization, usage in other IPs, infrastructure
limits such as power, thermals and so on.
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Perry Yuan <perry.yuan@amd.com>
---
Documentation/admin-guide/kernel-parameters.txt | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 42af9ca0127e..73a02816f6f8 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -6970,3 +6970,10 @@
management firmware translates the requests into actual
hardware states (core frequency, data fabric and memory
clocks etc.)
+ 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.
--
2.34.1
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH v9 12/13] cpufreq: amd-pstate: convert sprintf with sysfs_emit()
2022-12-25 16:34 [PATCH v9 00/13] Implement AMD Pstate EPP Driver Perry Yuan
` (10 preceding siblings ...)
2022-12-25 16:34 ` [PATCH v9 11/13] Documentation: introduce amd pstate active mode kernel command line options Perry Yuan
@ 2022-12-25 16:34 ` Perry Yuan
2022-12-25 16:34 ` [PATCH v9 13/13] Documentation: amd-pstate: introduce new global sysfs attributes Perry Yuan
12 siblings, 0 replies; 40+ messages in thread
From: Perry Yuan @ 2022-12-25 16:34 UTC (permalink / raw)
To: rafael.j.wysocki, Mario.Limonciello, ray.huang, viresh.kumar
Cc: Deepak.Sharma, Nathan.Fontenot, Alexander.Deucher, Shimmer.Huang,
Xiaojian.Du, Li.Meng, wyes.karny, linux-pm, linux-kernel
replace the sprintf with a more generic sysfs_emit function
No intended potential function impact
Acked-by: Huang Rui <ray.huang@amd.com>
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Perry Yuan <perry.yuan@amd.com>
---
drivers/cpufreq/amd-pstate.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 8ceca4524fc1..87450413cf45 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -695,7 +695,7 @@ static ssize_t show_amd_pstate_max_freq(struct cpufreq_policy *policy,
if (max_freq < 0)
return max_freq;
- return sprintf(&buf[0], "%u\n", max_freq);
+ return sysfs_emit(buf, "%u\n", max_freq);
}
static ssize_t show_amd_pstate_lowest_nonlinear_freq(struct cpufreq_policy *policy,
@@ -708,7 +708,7 @@ static ssize_t show_amd_pstate_lowest_nonlinear_freq(struct cpufreq_policy *poli
if (freq < 0)
return freq;
- return sprintf(&buf[0], "%u\n", freq);
+ return sysfs_emit(buf, "%u\n", freq);
}
/*
@@ -723,7 +723,7 @@ static ssize_t show_amd_pstate_highest_perf(struct cpufreq_policy *policy,
perf = READ_ONCE(cpudata->highest_perf);
- return sprintf(&buf[0], "%u\n", perf);
+ return sysfs_emit(buf, "%u\n", perf);
}
static ssize_t show_energy_performance_available_preferences(
--
2.34.1
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH v9 13/13] Documentation: amd-pstate: introduce new global sysfs attributes
2022-12-25 16:34 [PATCH v9 00/13] Implement AMD Pstate EPP Driver Perry Yuan
` (11 preceding siblings ...)
2022-12-25 16:34 ` [PATCH v9 12/13] cpufreq: amd-pstate: convert sprintf with sysfs_emit() Perry Yuan
@ 2022-12-25 16:34 ` Perry Yuan
2023-01-04 0:35 ` Limonciello, Mario
12 siblings, 1 reply; 40+ messages in thread
From: Perry Yuan @ 2022-12-25 16:34 UTC (permalink / raw)
To: rafael.j.wysocki, Mario.Limonciello, ray.huang, viresh.kumar
Cc: Deepak.Sharma, Nathan.Fontenot, Alexander.Deucher, Shimmer.Huang,
Xiaojian.Du, Li.Meng, wyes.karny, linux-pm, linux-kernel
The amd-pstate driver supports switching working modes at runtime.
Users can view and change modes by interacting with the "status" sysfs
attribute.
1) check driver mode:
$ cat /sys/devices/system/cpu/amd-pstate/status
2) switch mode:
`# echo "passive" | sudo tee /sys/devices/system/cpu/amd-pstate/status`
or
`# echo "active" | sudo tee /sys/devices/system/cpu/amd-pstate/status`
Signed-off-by: Perry Yuan <perry.yuan@amd.com>
---
Documentation/admin-guide/pm/amd-pstate.rst | 29 +++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/Documentation/admin-guide/pm/amd-pstate.rst b/Documentation/admin-guide/pm/amd-pstate.rst
index 62744dae3c5f..5f6379475b32 100644
--- a/Documentation/admin-guide/pm/amd-pstate.rst
+++ b/Documentation/admin-guide/pm/amd-pstate.rst
@@ -339,6 +339,35 @@ processor must provide at least nominal performance requested and go higher if c
operating conditions allow.
+User Space Interface in ``sysfs``
+=================================
+
+Global Attributes
+-----------------
+
+``amd-pstate`` exposes several global attributes (files) in ``sysfs`` to
+control its functionality at the system level. They are located in the
+``/sys/devices/system/cpu/amd-pstate/`` directory and affect all CPUs.
+
+``status``
+ Operation mode of the driver: "active", "passive" or "disable".
+
+ "active"
+ The driver is functional and in the ``active mode``
+
+ "passive"
+ The driver is functional and in the ``passive mode``
+
+ "disable"
+ The driver is unregistered and not functional now.
+
+ This attribute can be written to in order to change the driver's
+ operation mode or to unregister it. The string written to it must be
+ one of the possible values of it and, if successful, writing one of
+ these values to the sysfs file will cause the driver to cause the driver
+ to switch over to the operation mode represented by that string - or to be
+ unregistered in the "disable" case.
+
``cpupower`` tool support for ``amd-pstate``
===============================================
--
2.34.1
^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH v9 13/13] Documentation: amd-pstate: introduce new global sysfs attributes
2022-12-25 16:34 ` [PATCH v9 13/13] Documentation: amd-pstate: introduce new global sysfs attributes Perry Yuan
@ 2023-01-04 0:35 ` Limonciello, Mario
2023-01-05 6:21 ` Yuan, Perry
0 siblings, 1 reply; 40+ messages in thread
From: Limonciello, Mario @ 2023-01-04 0:35 UTC (permalink / raw)
To: Perry Yuan, rafael.j.wysocki, ray.huang, viresh.kumar
Cc: Deepak.Sharma, Nathan.Fontenot, Alexander.Deucher, Shimmer.Huang,
Xiaojian.Du, Li.Meng, wyes.karny, linux-pm, linux-kernel
On 12/25/2022 10:34, Perry Yuan wrote:
> The amd-pstate driver supports switching working modes at runtime.
> Users can view and change modes by interacting with the "status" sysfs
> attribute.
>
> 1) check driver mode:
> $ cat /sys/devices/system/cpu/amd-pstate/status
>
> 2) switch mode:
> `# echo "passive" | sudo tee /sys/devices/system/cpu/amd-pstate/status`
> or
> `# echo "active" | sudo tee /sys/devices/system/cpu/amd-pstate/status`
>
> Signed-off-by: Perry Yuan <perry.yuan@amd.com>
> ---
> Documentation/admin-guide/pm/amd-pstate.rst | 29 +++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/Documentation/admin-guide/pm/amd-pstate.rst b/Documentation/admin-guide/pm/amd-pstate.rst
> index 62744dae3c5f..5f6379475b32 100644
> --- a/Documentation/admin-guide/pm/amd-pstate.rst
> +++ b/Documentation/admin-guide/pm/amd-pstate.rst
> @@ -339,6 +339,35 @@ processor must provide at least nominal performance requested and go higher if c
> operating conditions allow.
>
>
> +User Space Interface in ``sysfs``
> +=================================
> +
> +Global Attributes
> +-----------------
> +
> +``amd-pstate`` exposes several global attributes (files) in ``sysfs`` to
> +control its functionality at the system level. They are located in the
> +``/sys/devices/system/cpu/amd-pstate/`` directory and affect all CPUs.
> +
> +``status``
> + Operation mode of the driver: "active", "passive" or "disable".
> +
> + "active"
> + The driver is functional and in the ``active mode``
> +
> + "passive"
> + The driver is functional and in the ``passive mode``
> +
> + "disable"
> + The driver is unregistered and not functional now.
> +
> + This attribute can be written to in order to change the driver's
> + operation mode or to unregister it. The string written to it must be
> + one of the possible values of it and, if successful, writing one of
> + these values to the sysfs file will cause the driver to cause the driver
"will cause the driver to cause the driver to"?
Presumably you mean just "will cause the driver to"
With that fixed:
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> + to switch over to the operation mode represented by that string - or to be
> + unregistered in the "disable" case.
> +
> ``cpupower`` tool support for ``amd-pstate``
> ===============================================
>
^ permalink raw reply [flat|nested] 40+ messages in thread
* RE: [PATCH v9 13/13] Documentation: amd-pstate: introduce new global sysfs attributes
2023-01-04 0:35 ` Limonciello, Mario
@ 2023-01-05 6:21 ` Yuan, Perry
0 siblings, 0 replies; 40+ messages in thread
From: Yuan, Perry @ 2023-01-05 6:21 UTC (permalink / raw)
To: Limonciello, Mario, rafael.j.wysocki, Huang, Ray, viresh.kumar
Cc: Sharma, Deepak, Fontenot, Nathan, Deucher, Alexander, Huang,
Shimmer, Du, Xiaojian, Meng, Li (Jassmine),
Karny, Wyes, linux-pm, linux-kernel
[AMD Official Use Only - General]
Hi Mario.
> -----Original Message-----
> From: Limonciello, Mario <Mario.Limonciello@amd.com>
> Sent: Wednesday, January 4, 2023 8:35 AM
> To: Yuan, Perry <Perry.Yuan@amd.com>; rafael.j.wysocki@intel.com; Huang,
> Ray <Ray.Huang@amd.com>; viresh.kumar@linaro.org
> Cc: Sharma, Deepak <Deepak.Sharma@amd.com>; Fontenot, Nathan
> <Nathan.Fontenot@amd.com>; Deucher, Alexander
> <Alexander.Deucher@amd.com>; Huang, Shimmer
> <Shimmer.Huang@amd.com>; Du, Xiaojian <Xiaojian.Du@amd.com>; Meng,
> Li (Jassmine) <Li.Meng@amd.com>; Karny, Wyes <Wyes.Karny@amd.com>;
> linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v9 13/13] Documentation: amd-pstate: introduce new
> global sysfs attributes
>
> On 12/25/2022 10:34, Perry Yuan wrote:
> > The amd-pstate driver supports switching working modes at runtime.
> > Users can view and change modes by interacting with the "status" sysfs
> > attribute.
> >
> > 1) check driver mode:
> > $ cat /sys/devices/system/cpu/amd-pstate/status
> >
> > 2) switch mode:
> > `# echo "passive" | sudo tee
> > /sys/devices/system/cpu/amd-pstate/status`
> > or
> > `# echo "active" | sudo tee /sys/devices/system/cpu/amd-pstate/status`
> >
> > Signed-off-by: Perry Yuan <perry.yuan@amd.com>
> > ---
> > Documentation/admin-guide/pm/amd-pstate.rst | 29
> +++++++++++++++++++++
> > 1 file changed, 29 insertions(+)
> >
> > diff --git a/Documentation/admin-guide/pm/amd-pstate.rst
> > b/Documentation/admin-guide/pm/amd-pstate.rst
> > index 62744dae3c5f..5f6379475b32 100644
> > --- a/Documentation/admin-guide/pm/amd-pstate.rst
> > +++ b/Documentation/admin-guide/pm/amd-pstate.rst
> > @@ -339,6 +339,35 @@ processor must provide at least nominal
> performance requested and go higher if c
> > operating conditions allow.
> >
> >
> > +User Space Interface in ``sysfs``
> > +=================================
> > +
> > +Global Attributes
> > +-----------------
> > +
> > +``amd-pstate`` exposes several global attributes (files) in ``sysfs``
> > +to control its functionality at the system level. They are located
> > +in the ``/sys/devices/system/cpu/amd-pstate/`` directory and affect all
> CPUs.
> > +
> > +``status``
> > + Operation mode of the driver: "active", "passive" or "disable".
> > +
> > + "active"
> > + The driver is functional and in the ``active mode``
> > +
> > + "passive"
> > + The driver is functional and in the ``passive mode``
> > +
> > + "disable"
> > + The driver is unregistered and not functional now.
> > +
> > + This attribute can be written to in order to change the driver's
> > + operation mode or to unregister it. The string written to it must be
> > + one of the possible values of it and, if successful, writing one of
> > + these values to the sysfs file will cause the driver to cause
> > + the driver
>
> "will cause the driver to cause the driver to"?
>
> Presumably you mean just "will cause the driver to"
>
> With that fixed:
>
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Thanks for the review, will fix the words and pick up the RB tag in v10.
Perry.
>
> > + to switch over to the operation mode represented by that string - or
> to be
> > + unregistered in the "disable" case.
> > +
> > ``cpupower`` tool support for ``amd-pstate``
> > ===============================================
> >
^ permalink raw reply [flat|nested] 40+ messages in thread