mbox series

[v4,0/6] Support for autonomous selection in cppc_cpufreq

Message ID 20250113122104.3870673-1-zhenglifeng1@huawei.com
Headers show
Series Support for autonomous selection in cppc_cpufreq | expand

Message

zhenglifeng (A) Jan. 13, 2025, 12:20 p.m. UTC
Add sysfs interfaces for CPPC autonomous selection in the cppc_cpufreq
driver.

The patch series is organized in two parts:

 - patch 1-4 refactor out the general CPPC register get and set functions
   in cppc_acpi.c

 - patches 5-6 expose sysfs files for users to control CPPC autonomous
   selection when supported

---
Since Pierre and me have discussed about whether or not to show
auto_act_window and energy_perf when auto_select is disabled. It seems
like whether to show these two files has their own points. We'd like to
ask for some advice.

Relevant discussion:
[1] https://lore.kernel.org/all/522721da-1a5c-439c-96a8-d0300dd0f906@huawei.com/

Changelog:

v4:

 - add REG_OPTIONAL and IS_OPTIONAL_CPC_REG to judge if a cpc register is
   an optional one
 - check whether the register is optional before CPC_SUPPORTED check in
   cppc_get_reg_val() and cppc_set_reg_val()
 - check the register's type in cppc_set_reg_val()
 - add macros to generally implement registers getting and setting
   functions
 - move some logic codes from cppc_cpufreq.c to cppc_acpi.c
 - replace cppc_get_auto_sel_caps() by cppc_get_auto_sel()

v3:

 - change cppc_get_reg() and cppc_set_reg() name to cppc_get_reg_val() and
   cppc_set_reg_val()
 - extract cppc_get_reg_val_in_pcc() and cppc_set_reg_val_in_pcc()
 - return the result of cpc_read() in cppc_get_reg_val()
 - add pr_debug() in cppc_get_reg_val_in_pcc() when pcc_ss_id < 0
 - rename 'cpunum' to 'cpu' in cppc_get_reg_val()
 - move some macros from drivers/cpufreq/cppc_cpufreq.c to
   include/acpi/cppc_acpi.h with a CPPC_XXX prefix

v2:

 - fix some incorrect placeholder
 - change kstrtoul to kstrtobool in store_auto_select

Lifeng Zheng (6):
  ACPI: CPPC: Add IS_OPTIONAL_CPC_REG macro
  ACPI: CPPC: Add cppc_get_reg_val and cppc_set_reg_val function
  ACPI: CPPC: Add macros to generally implement registers getting and
    setting functions
  ACPI: CPPC: Refactor register value get and set ABIs
  ACPI: CPPC: Add autonomous selection ABIs
  cpufreq: CPPC: Support for autonomous selection in cppc_cpufreq

 .../ABI/testing/sysfs-devices-system-cpu      |  54 +++
 drivers/acpi/cppc_acpi.c                      | 332 +++++++++---------
 drivers/cpufreq/amd-pstate.c                  |   3 +-
 drivers/cpufreq/cppc_cpufreq.c                | 109 ++++++
 include/acpi/cppc_acpi.h                      |  30 +-
 5 files changed, 359 insertions(+), 169 deletions(-)

Comments

Rafael J. Wysocki Jan. 14, 2025, 1:27 p.m. UTC | #1
On Mon, Jan 13, 2025 at 1:21 PM Lifeng Zheng <zhenglifeng1@huawei.com> wrote:
>
> Add IS_OPTIONAL_CPC_REG macro to judge if a cpc_reg is an optional one.

This requires a bit more explanation, especially what's the purpose of
it (ie. the "why").

> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
> ---
>  drivers/acpi/cppc_acpi.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index f193e713825a..6454b469338f 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -129,6 +129,12 @@ static DEFINE_PER_CPU(struct cpc_desc *, cpc_desc_ptr);
>  #define CPC_SUPPORTED(cpc) ((cpc)->type == ACPI_TYPE_INTEGER ?         \
>                                 !!(cpc)->cpc_entry.int_value :          \
>                                 !IS_NULL_REG(&(cpc)->cpc_entry.reg))
> +
> +/* These indicate optional of the per-cpu cpc_regs[]. */

Again, you need to say more here, like how this is supposed to work.

> +#define REG_OPTIONAL (0b111111100011111010000)

A hex literal would work too AFAICS.

> +
> +#define IS_OPTIONAL_CPC_REG(reg_idx) (REG_OPTIONAL & (1U << (reg_idx)))

You need to explain what reg_idx is.

> +
>  /*
>   * Arbitrary Retries in case the remote processor is slow to respond
>   * to PCC commands. Keeping it high enough to cover emulators where
> --