| Message ID | 20241216091603.1247644-4-zhenglifeng1@huawei.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | Support for autonomous selection in cppc_cpufreq | expand |
Hello Lifeng, Huang, Gautham, Mario, On 12/16/24 10:16, Lifeng Zheng wrote: > cppc_set_epp - write energy performance preference register > > cppc_get_auto_act_window - read autonomous activity window register > > cppc_set_auto_act_window - write autonomous activity window register > > cppc_get_auto_sel - read autonomous selection enable register > > Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com> > --- > drivers/acpi/cppc_acpi.c | 44 ++++++++++++++++++++++++++++++++++++++++ > include/acpi/cppc_acpi.h | 20 ++++++++++++++++++ > 2 files changed, 64 insertions(+) > > diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c > index 83c7fcad74ad..645f2366c888 100644 > --- a/drivers/acpi/cppc_acpi.c > +++ b/drivers/acpi/cppc_acpi.c > @@ -1595,6 +1595,50 @@ int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable) > } > EXPORT_SYMBOL_GPL(cppc_set_epp_perf); > > +/** > + * cppc_set_epp() - Write the EPP register. > + * @cpu: CPU on which to write register. > + * @epp_val: Value to write to the EPP register. > + */ > +int cppc_set_epp(int cpu, u64 epp_val) > +{ > + return cppc_set_reg_val(cpu, ENERGY_PERF, epp_val); > +} > +EXPORT_SYMBOL_GPL(cppc_set_epp); > + > +/** > + * cppc_get_auto_act_window() - Read autonomous activity window register. > + * @cpu: CPU from which to read register. > + * @auto_act_window: Return address. > + */ > +int cppc_get_auto_act_window(int cpu, u64 *auto_act_window) As there is only one way to interpret the value of the 'Autonomous Activity Window Register', maybe the logic to convert from/to the register value to a value in us should be placed here rather than in the cppc_cpufreq driver. Meaning, maybe the prototype should be: int cppc_get_auto_act_window(int cpu, unsigned int *auto_act_window); Similar remark for cppc_set_epp() and other functions. > +{ > + return cppc_get_reg_val(cpu, AUTO_ACT_WINDOW, auto_act_window); > +} > +EXPORT_SYMBOL_GPL(cppc_get_auto_act_window); > + > +/** > + * cppc_set_auto_act_window() - Write autonomous activity window register. > + * @cpu: CPU on which to write register. > + * @auto_act_window: Value to write to the autonomous activity window register. > + */ > +int cppc_set_auto_act_window(int cpu, u64 auto_act_window) > +{ > + return cppc_set_reg_val(cpu, AUTO_ACT_WINDOW, auto_act_window); > +} > +EXPORT_SYMBOL_GPL(cppc_set_auto_act_window); > + > +/** > + * cppc_get_auto_sel() - Read autonomous selection register. > + * @cpu: CPU from which to read register. > + * @auto_sel: Return address. > + */ > +int cppc_get_auto_sel(int cpu, u64 *auto_sel) Similarly, maybe it would be better to use: int cppc_get_auto_sel(int cpu, bool *auto_sel); > +{ > + return cppc_get_reg_val(cpu, AUTO_SEL_ENABLE, auto_sel); > +} > +EXPORT_SYMBOL_GPL(cppc_get_auto_sel); > + > /** > * cppc_get_auto_sel_caps - Read autonomous selection register. > * @cpunum : CPU from which to read register. > diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h > index 62d368bcd9ec..134931b081a0 100644 > --- a/include/acpi/cppc_acpi.h > +++ b/include/acpi/cppc_acpi.h > @@ -159,6 +159,10 @@ extern int cpc_read_ffh(int cpunum, struct cpc_reg *reg, u64 *val); > extern int cpc_write_ffh(int cpunum, struct cpc_reg *reg, u64 val); > extern int cppc_get_epp_perf(int cpunum, u64 *epp_perf); > extern int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable); > +extern int cppc_set_epp(int cpu, u64 epp_val); > +extern int cppc_get_auto_act_window(int cpu, u64 *auto_act_window); > +extern int cppc_set_auto_act_window(int cpu, u64 auto_act_window); > +extern int cppc_get_auto_sel(int cpu, u64 *auto_sel); > extern int cppc_get_auto_sel_caps(int cpunum, struct cppc_perf_caps *perf_caps); This is a bit annoying, but maybe only one function between: - cppc_get_auto_sel_caps() - cppc_get_auto_sel() is necessary. I added the owners of the amd-pstate driver to ask if this would be ok to replace cppc_get_auto_sel_caps() by cppc_get_auto_sel(). > extern int cppc_set_auto_sel(int cpu, bool enable); > extern int amd_get_highest_perf(unsigned int cpu, u32 *highest_perf); > @@ -225,6 +229,22 @@ static inline int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, > { > return -EOPNOTSUPP; > } > +static inline int cppc_set_epp(int cpu, u64 epp_val) > +{ > + return -EOPNOTSUPP; > +} > +static inline int cppc_get_auto_act_window(int cpu, u64 *auto_act_window) > +{ > + return -EOPNOTSUPP; > +} > +static inline int cppc_set_auto_act_window(int cpu, u64 auto_act_window) > +{ > + return -EOPNOTSUPP; > +} > +static inline int cppc_get_auto_sel(int cpu, u64 *auto_sel) > +{ > + return -EOPNOTSUPP; > +} > static inline int cppc_get_epp_perf(int cpunum, u64 *epp_perf) > { > return -EOPNOTSUPP;
On 12/17/2024 07:48, Pierre Gondois wrote: > Hello Lifeng, Huang, Gautham, Mario, > > On 12/16/24 10:16, Lifeng Zheng wrote: >> cppc_set_epp - write energy performance preference register >> >> cppc_get_auto_act_window - read autonomous activity window register >> >> cppc_set_auto_act_window - write autonomous activity window register >> >> cppc_get_auto_sel - read autonomous selection enable register >> >> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com> >> --- >> drivers/acpi/cppc_acpi.c | 44 ++++++++++++++++++++++++++++++++++++++++ >> include/acpi/cppc_acpi.h | 20 ++++++++++++++++++ >> 2 files changed, 64 insertions(+) >> >> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c >> index 83c7fcad74ad..645f2366c888 100644 >> --- a/drivers/acpi/cppc_acpi.c >> +++ b/drivers/acpi/cppc_acpi.c >> @@ -1595,6 +1595,50 @@ int cppc_set_epp_perf(int cpu, struct >> cppc_perf_ctrls *perf_ctrls, bool enable) >> } >> EXPORT_SYMBOL_GPL(cppc_set_epp_perf); >> +/** >> + * cppc_set_epp() - Write the EPP register. >> + * @cpu: CPU on which to write register. >> + * @epp_val: Value to write to the EPP register. >> + */ >> +int cppc_set_epp(int cpu, u64 epp_val) >> +{ >> + return cppc_set_reg_val(cpu, ENERGY_PERF, epp_val); >> +} >> +EXPORT_SYMBOL_GPL(cppc_set_epp); >> + >> +/** >> + * cppc_get_auto_act_window() - Read autonomous activity window >> register. >> + * @cpu: CPU from which to read register. >> + * @auto_act_window: Return address. >> + */ >> +int cppc_get_auto_act_window(int cpu, u64 *auto_act_window) > > As there is only one way to interpret the value of the > 'Autonomous Activity Window Register', maybe the logic to convert > from/to the register value to a value in us should be placed here > rather than in the cppc_cpufreq driver. > Meaning, maybe the prototype should be: > > int cppc_get_auto_act_window(int cpu, unsigned int *auto_act_window); > > Similar remark for cppc_set_epp() and other functions. > >> +{ >> + return cppc_get_reg_val(cpu, AUTO_ACT_WINDOW, auto_act_window); >> +} >> +EXPORT_SYMBOL_GPL(cppc_get_auto_act_window); >> + >> +/** >> + * cppc_set_auto_act_window() - Write autonomous activity window >> register. >> + * @cpu: CPU on which to write register. >> + * @auto_act_window: Value to write to the autonomous activity window >> register. >> + */ >> +int cppc_set_auto_act_window(int cpu, u64 auto_act_window) >> +{ >> + return cppc_set_reg_val(cpu, AUTO_ACT_WINDOW, auto_act_window); >> +} >> +EXPORT_SYMBOL_GPL(cppc_set_auto_act_window); >> + >> +/** >> + * cppc_get_auto_sel() - Read autonomous selection register. >> + * @cpu: CPU from which to read register. >> + * @auto_sel: Return address. >> + */ >> +int cppc_get_auto_sel(int cpu, u64 *auto_sel) > > Similarly, maybe it would be better to use: > int cppc_get_auto_sel(int cpu, bool *auto_sel); > >> +{ >> + return cppc_get_reg_val(cpu, AUTO_SEL_ENABLE, auto_sel); >> +} >> +EXPORT_SYMBOL_GPL(cppc_get_auto_sel); >> + >> /** >> * cppc_get_auto_sel_caps - Read autonomous selection register. >> * @cpunum : CPU from which to read register. >> diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h >> index 62d368bcd9ec..134931b081a0 100644 >> --- a/include/acpi/cppc_acpi.h >> +++ b/include/acpi/cppc_acpi.h >> @@ -159,6 +159,10 @@ extern int cpc_read_ffh(int cpunum, struct >> cpc_reg *reg, u64 *val); >> extern int cpc_write_ffh(int cpunum, struct cpc_reg *reg, u64 val); >> extern int cppc_get_epp_perf(int cpunum, u64 *epp_perf); >> extern int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls >> *perf_ctrls, bool enable); >> +extern int cppc_set_epp(int cpu, u64 epp_val); >> +extern int cppc_get_auto_act_window(int cpu, u64 *auto_act_window); >> +extern int cppc_set_auto_act_window(int cpu, u64 auto_act_window); >> +extern int cppc_get_auto_sel(int cpu, u64 *auto_sel); >> extern int cppc_get_auto_sel_caps(int cpunum, struct cppc_perf_caps >> *perf_caps); > > This is a bit annoying, but maybe only one function between: > - cppc_get_auto_sel_caps() > - cppc_get_auto_sel() > is necessary. > > I added the owners of the amd-pstate driver to ask if this would > be ok to replace cppc_get_auto_sel_caps() by cppc_get_auto_sel(). Yeah I have no concerns with this if that's the direction this patch series goes. Feel free to change amd-pstate in the patch that introduces cppc_get_auto_sel(). I'll be out around the US holiday, so I might not be able to review it for a while, but CC Gautham on the series and he may be able to. > >> extern int cppc_set_auto_sel(int cpu, bool enable); >> extern int amd_get_highest_perf(unsigned int cpu, u32 *highest_perf); >> @@ -225,6 +229,22 @@ static inline int cppc_set_epp_perf(int cpu, >> struct cppc_perf_ctrls *perf_ctrls, >> { >> return -EOPNOTSUPP; >> } >> +static inline int cppc_set_epp(int cpu, u64 epp_val) >> +{ >> + return -EOPNOTSUPP; >> +} >> +static inline int cppc_get_auto_act_window(int cpu, u64 >> *auto_act_window) >> +{ >> + return -EOPNOTSUPP; >> +} >> +static inline int cppc_set_auto_act_window(int cpu, u64 auto_act_window) >> +{ >> + return -EOPNOTSUPP; >> +} >> +static inline int cppc_get_auto_sel(int cpu, u64 *auto_sel) >> +{ >> + return -EOPNOTSUPP; >> +} >> static inline int cppc_get_epp_perf(int cpunum, u64 *epp_perf) >> { >> return -EOPNOTSUPP;
diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c index 83c7fcad74ad..645f2366c888 100644 --- a/drivers/acpi/cppc_acpi.c +++ b/drivers/acpi/cppc_acpi.c @@ -1595,6 +1595,50 @@ int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable) } EXPORT_SYMBOL_GPL(cppc_set_epp_perf); +/** + * cppc_set_epp() - Write the EPP register. + * @cpu: CPU on which to write register. + * @epp_val: Value to write to the EPP register. + */ +int cppc_set_epp(int cpu, u64 epp_val) +{ + return cppc_set_reg_val(cpu, ENERGY_PERF, epp_val); +} +EXPORT_SYMBOL_GPL(cppc_set_epp); + +/** + * cppc_get_auto_act_window() - Read autonomous activity window register. + * @cpu: CPU from which to read register. + * @auto_act_window: Return address. + */ +int cppc_get_auto_act_window(int cpu, u64 *auto_act_window) +{ + return cppc_get_reg_val(cpu, AUTO_ACT_WINDOW, auto_act_window); +} +EXPORT_SYMBOL_GPL(cppc_get_auto_act_window); + +/** + * cppc_set_auto_act_window() - Write autonomous activity window register. + * @cpu: CPU on which to write register. + * @auto_act_window: Value to write to the autonomous activity window register. + */ +int cppc_set_auto_act_window(int cpu, u64 auto_act_window) +{ + return cppc_set_reg_val(cpu, AUTO_ACT_WINDOW, auto_act_window); +} +EXPORT_SYMBOL_GPL(cppc_set_auto_act_window); + +/** + * cppc_get_auto_sel() - Read autonomous selection register. + * @cpu: CPU from which to read register. + * @auto_sel: Return address. + */ +int cppc_get_auto_sel(int cpu, u64 *auto_sel) +{ + return cppc_get_reg_val(cpu, AUTO_SEL_ENABLE, auto_sel); +} +EXPORT_SYMBOL_GPL(cppc_get_auto_sel); + /** * cppc_get_auto_sel_caps - Read autonomous selection register. * @cpunum : CPU from which to read register. diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h index 62d368bcd9ec..134931b081a0 100644 --- a/include/acpi/cppc_acpi.h +++ b/include/acpi/cppc_acpi.h @@ -159,6 +159,10 @@ extern int cpc_read_ffh(int cpunum, struct cpc_reg *reg, u64 *val); extern int cpc_write_ffh(int cpunum, struct cpc_reg *reg, u64 val); extern int cppc_get_epp_perf(int cpunum, u64 *epp_perf); extern int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable); +extern int cppc_set_epp(int cpu, u64 epp_val); +extern int cppc_get_auto_act_window(int cpu, u64 *auto_act_window); +extern int cppc_set_auto_act_window(int cpu, u64 auto_act_window); +extern int cppc_get_auto_sel(int cpu, u64 *auto_sel); extern int cppc_get_auto_sel_caps(int cpunum, struct cppc_perf_caps *perf_caps); extern int cppc_set_auto_sel(int cpu, bool enable); extern int amd_get_highest_perf(unsigned int cpu, u32 *highest_perf); @@ -225,6 +229,22 @@ static inline int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, { return -EOPNOTSUPP; } +static inline int cppc_set_epp(int cpu, u64 epp_val) +{ + return -EOPNOTSUPP; +} +static inline int cppc_get_auto_act_window(int cpu, u64 *auto_act_window) +{ + return -EOPNOTSUPP; +} +static inline int cppc_set_auto_act_window(int cpu, u64 auto_act_window) +{ + return -EOPNOTSUPP; +} +static inline int cppc_get_auto_sel(int cpu, u64 *auto_sel) +{ + return -EOPNOTSUPP; +} static inline int cppc_get_epp_perf(int cpunum, u64 *epp_perf) { return -EOPNOTSUPP;
cppc_set_epp - write energy performance preference register cppc_get_auto_act_window - read autonomous activity window register cppc_set_auto_act_window - write autonomous activity window register cppc_get_auto_sel - read autonomous selection enable register Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com> --- drivers/acpi/cppc_acpi.c | 44 ++++++++++++++++++++++++++++++++++++++++ include/acpi/cppc_acpi.h | 20 ++++++++++++++++++ 2 files changed, 64 insertions(+)