[PATCH 0/3] Support for autonomous selection in cppc_cpufreq

[PATCH 0/3] Support for autonomous selection in cppc_cpufreq

[Lifeng Zheng]

Add sysfs interfaces for CPPC autonomous selection in the cppc_cpufreq
driver.

The patch series is organized in two parts:

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

 - patches 2-3 expose sysfs files for users to control CPPC autonomous
   selection when supported

Lifeng Zheng (3):
  ACPI: CPPC: Refactor register 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                      | 235 +++++++++---------
 drivers/cpufreq/cppc_cpufreq.c                | 141 +++++++++++
 include/acpi/cppc_acpi.h                      |  20 ++
 4 files changed, 331 insertions(+), 119 deletions(-)

-- 
2.33.0

[PATCH 1/3] ACPI: CPPC: Refactor register get and set ABIs

[Lifeng Zheng]

Refactor register get and set ABIs using cppc_get_reg() and cppc_set_reg().

Rename cppc_get_perf() to cppc_get_reg() as a generic function to read cppc
registers, with two changes:

1. Change the error kind to "no such device" when pcc_ss_id < 0, which
means that this cpu cannot get a valid pcc_ss_id.

2. Add a check to verify if the register is a cpc supported one before
using it.

Add cppc_set_reg() as a generic function for setting cppc registers. Unlike
other set reg ABIs, this function checks CPC_SUPPORTED right after getting
the register, because the rest of the operations are meaningless if this
register is not a cpc supported one.

Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
---
 drivers/acpi/cppc_acpi.c | 191 +++++++++++++++------------------------
 1 file changed, 72 insertions(+), 119 deletions(-)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index c1f3568d0c50..306ced9c3376 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -1179,10 +1179,13 @@ static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val)
 	return ret_val;
 }
 
-static int cppc_get_perf(int cpunum, enum cppc_regs reg_idx, u64 *perf)
+static int cppc_get_reg(int cpunum, enum cppc_regs reg_idx, u64 *val)
 {
 	struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpunum);
+	struct cppc_pcc_data *pcc_ss_data = NULL;
 	struct cpc_register_resource *reg;
+	int pcc_ss_id;
+	int ret = 0;
 
 	if (!cpc_desc) {
 		pr_debug("No CPC descriptor for CPU:%d\n", cpunum);
@@ -1191,20 +1194,23 @@ static int cppc_get_perf(int cpunum, enum cppc_regs reg_idx, u64 *perf)
 
 	reg = &cpc_desc->cpc_regs[reg_idx];
 
+	if (!CPC_SUPPORTED(reg)) {
+		pr_debug("CPC register (reg_idx=%u) is not supported\n", reg_idx);
+		return -EOPNOTSUPP;
+	}
+
 	if (CPC_IN_PCC(reg)) {
-		int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpunum);
-		struct cppc_pcc_data *pcc_ss_data = NULL;
-		int ret = 0;
+		pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpunum);
 
 		if (pcc_ss_id < 0)
-			return -EIO;
+			return -ENODEV;
 
 		pcc_ss_data = pcc_data[pcc_ss_id];
 
 		down_write(&pcc_ss_data->pcc_lock);
 
 		if (send_pcc_cmd(pcc_ss_id, CMD_READ) >= 0)
-			cpc_read(cpunum, reg, perf);
+			cpc_read(cpunum, reg, val);
 		else
 			ret = -EIO;
 
@@ -1213,21 +1219,65 @@ static int cppc_get_perf(int cpunum, enum cppc_regs reg_idx, u64 *perf)
 		return ret;
 	}
 
-	cpc_read(cpunum, reg, perf);
+	cpc_read(cpunum, reg, val);
 
 	return 0;
 }
 
+static int cppc_set_reg(int cpu, enum cppc_regs reg_idx, u64 val)
+{
+	struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpu);
+	struct cppc_pcc_data *pcc_ss_data = NULL;
+	struct cpc_register_resource *reg;
+	int pcc_ss_id;
+	int ret;
+
+	if (!cpc_desc) {
+		pr_debug("No CPC descriptor for CPU:%d\n", cpu);
+		return -ENODEV;
+	}
+
+	reg = &cpc_desc->cpc_regs[reg_idx];
+
+	if (!CPC_SUPPORTED(reg)) {
+		pr_debug("CPC register (reg_idx=%u) is not supported\n", reg_idx);
+		return -EOPNOTSUPP;
+	}
+
+	if (CPC_IN_PCC(reg)) {
+		pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpu);
+
+		if (pcc_ss_id < 0) {
+			pr_debug("Invalid pcc_ss_id\n");
+			return -ENODEV;
+		}
+
+		ret = cpc_write(cpu, reg, val);
+		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);
+		return ret;
+	}
+
+	return cpc_write(cpu, reg, val);
+}
+
 /**
  * cppc_get_desired_perf - Get the desired performance register value.
  * @cpunum: CPU from which to get desired performance.
  * @desired_perf: Return address.
  *
- * Return: 0 for success, -EIO otherwise.
+ * Return: 0 for success, -ERRNO otherwise.
  */
 int cppc_get_desired_perf(int cpunum, u64 *desired_perf)
 {
-	return cppc_get_perf(cpunum, DESIRED_PERF, desired_perf);
+	return cppc_get_reg(cpunum, DESIRED_PERF, desired_perf);
 }
 EXPORT_SYMBOL_GPL(cppc_get_desired_perf);
 
@@ -1236,11 +1286,11 @@ EXPORT_SYMBOL_GPL(cppc_get_desired_perf);
  * @cpunum: CPU from which to get nominal performance.
  * @nominal_perf: Return address.
  *
- * Return: 0 for success, -EIO otherwise.
+ * Return: 0 for success, -ERRNO otherwise.
  */
 int cppc_get_nominal_perf(int cpunum, u64 *nominal_perf)
 {
-	return cppc_get_perf(cpunum, NOMINAL_PERF, nominal_perf);
+	return cppc_get_reg(cpunum, NOMINAL_PERF, nominal_perf);
 }
 
 /**
@@ -1248,11 +1298,11 @@ int cppc_get_nominal_perf(int cpunum, u64 *nominal_perf)
  * @cpunum: CPU from which to get highest performance.
  * @highest_perf: Return address.
  *
- * Return: 0 for success, -EIO otherwise.
+ * Return: 0 for success, -ERRNO otherwise.
  */
 int cppc_get_highest_perf(int cpunum, u64 *highest_perf)
 {
-	return cppc_get_perf(cpunum, HIGHEST_PERF, highest_perf);
+	return cppc_get_reg(cpunum, HIGHEST_PERF, highest_perf);
 }
 EXPORT_SYMBOL_GPL(cppc_get_highest_perf);
 
@@ -1261,11 +1311,11 @@ EXPORT_SYMBOL_GPL(cppc_get_highest_perf);
  * @cpunum: CPU from which to get epp preference value.
  * @epp_perf: Return address.
  *
- * Return: 0 for success, -EIO otherwise.
+ * Return: 0 for success, -ERRNO otherwise.
  */
 int cppc_get_epp_perf(int cpunum, u64 *epp_perf)
 {
-	return cppc_get_perf(cpunum, ENERGY_PERF, epp_perf);
+	return cppc_get_reg(cpunum, ENERGY_PERF, epp_perf);
 }
 EXPORT_SYMBOL_GPL(cppc_get_epp_perf);
 
@@ -1545,44 +1595,14 @@ EXPORT_SYMBOL_GPL(cppc_set_epp_perf);
  */
 int cppc_get_auto_sel_caps(int cpunum, struct cppc_perf_caps *perf_caps)
 {
-	struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpunum);
-	struct cpc_register_resource *auto_sel_reg;
-	u64  auto_sel;
-
-	if (!cpc_desc) {
-		pr_debug("No CPC descriptor for CPU:%d\n", cpunum);
-		return -ENODEV;
-	}
-
-	auto_sel_reg = &cpc_desc->cpc_regs[AUTO_SEL_ENABLE];
-
-	if (!CPC_SUPPORTED(auto_sel_reg))
-		pr_warn_once("Autonomous mode is not unsupported!\n");
-
-	if (CPC_IN_PCC(auto_sel_reg)) {
-		int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpunum);
-		struct cppc_pcc_data *pcc_ss_data = NULL;
-		int ret = 0;
-
-		if (pcc_ss_id < 0)
-			return -ENODEV;
-
-		pcc_ss_data = pcc_data[pcc_ss_id];
-
-		down_write(&pcc_ss_data->pcc_lock);
-
-		if (send_pcc_cmd(pcc_ss_id, CMD_READ) >= 0) {
-			cpc_read(cpunum, auto_sel_reg, &auto_sel);
-			perf_caps->auto_sel = (bool)auto_sel;
-		} else {
-			ret = -EIO;
-		}
-
-		up_write(&pcc_ss_data->pcc_lock);
+	u64 auto_sel;
+	int ret;
 
+	ret = cppc_get_reg(cpunum, AUTO_SEL_ENABLE, &auto_sel);
+	if (ret)
 		return ret;
-	}
 
+	perf_caps->auto_sel = (bool)auto_sel;
 	return 0;
 }
 EXPORT_SYMBOL_GPL(cppc_get_auto_sel_caps);
@@ -1594,43 +1614,7 @@ EXPORT_SYMBOL_GPL(cppc_get_auto_sel_caps);
  */
 int cppc_set_auto_sel(int cpu, bool enable)
 {
-	int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpu);
-	struct cpc_register_resource *auto_sel_reg;
-	struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpu);
-	struct cppc_pcc_data *pcc_ss_data = NULL;
-	int ret = -EINVAL;
-
-	if (!cpc_desc) {
-		pr_debug("No CPC descriptor for CPU:%d\n", cpu);
-		return -ENODEV;
-	}
-
-	auto_sel_reg = &cpc_desc->cpc_regs[AUTO_SEL_ENABLE];
-
-	if (CPC_IN_PCC(auto_sel_reg)) {
-		if (pcc_ss_id < 0) {
-			pr_debug("Invalid pcc_ss_id\n");
-			return -ENODEV;
-		}
-
-		if (CPC_SUPPORTED(auto_sel_reg)) {
-			ret = cpc_write(cpu, auto_sel_reg, enable);
-			if (ret)
-				return ret;
-		}
-
-		pcc_ss_data = pcc_data[pcc_ss_id];
-
-		down_write(&pcc_ss_data->pcc_lock);
-		/* after writing CPC, transfer the ownership of PCC to platform */
-		ret = send_pcc_cmd(pcc_ss_id, CMD_WRITE);
-		up_write(&pcc_ss_data->pcc_lock);
-	} else {
-		ret = -ENOTSUPP;
-		pr_debug("_CPC in PCC is not supported\n");
-	}
-
-	return ret;
+	return cppc_set_reg(cpu, AUTO_SEL_ENABLE, enable);
 }
 EXPORT_SYMBOL_GPL(cppc_set_auto_sel);
 
@@ -1644,38 +1628,7 @@ EXPORT_SYMBOL_GPL(cppc_set_auto_sel);
  */
 int cppc_set_enable(int cpu, bool enable)
 {
-	int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpu);
-	struct cpc_register_resource *enable_reg;
-	struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpu);
-	struct cppc_pcc_data *pcc_ss_data = NULL;
-	int ret = -EINVAL;
-
-	if (!cpc_desc) {
-		pr_debug("No CPC descriptor for CPU:%d\n", cpu);
-		return -EINVAL;
-	}
-
-	enable_reg = &cpc_desc->cpc_regs[ENABLE];
-
-	if (CPC_IN_PCC(enable_reg)) {
-
-		if (pcc_ss_id < 0)
-			return -EIO;
-
-		ret = cpc_write(cpu, enable_reg, enable);
-		if (ret)
-			return ret;
-
-		pcc_ss_data = pcc_data[pcc_ss_id];
-
-		down_write(&pcc_ss_data->pcc_lock);
-		/* after writing CPC, transfer the ownership of PCC to platfrom */
-		ret = send_pcc_cmd(pcc_ss_id, CMD_WRITE);
-		up_write(&pcc_ss_data->pcc_lock);
-		return ret;
-	}
-
-	return cpc_write(cpu, enable_reg, enable);
+	return cppc_set_reg(cpu, ENABLE, enable);
 }
 EXPORT_SYMBOL_GPL(cppc_set_enable);
 
-- 
2.33.0

Re: [PATCH 1/3] ACPI: CPPC: Refactor register get and set ABIs

[Pierre Gondois]

Hello Lifeng,

On 11/14/24 09:48, Lifeng Zheng wrote:
> Refactor register get and set ABIs using cppc_get_reg() and cppc_set_reg().
> 
> Rename cppc_get_perf() to cppc_get_reg() as a generic function to read cppc
> registers, with two changes:
> 
> 1. Change the error kind to "no such device" when pcc_ss_id < 0, which
> means that this cpu cannot get a valid pcc_ss_id.
> 
> 2. Add a check to verify if the register is a cpc supported one before
> using it.
> 
> Add cppc_set_reg() as a generic function for setting cppc registers. Unlike
> other set reg ABIs, this function checks CPC_SUPPORTED right after getting
> the register, because the rest of the operations are meaningless if this
> register is not a cpc supported one.
> 
> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
> ---
>   drivers/acpi/cppc_acpi.c | 191 +++++++++++++++------------------------
>   1 file changed, 72 insertions(+), 119 deletions(-)
> 
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index c1f3568d0c50..306ced9c3376 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -1179,10 +1179,13 @@ static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val)
>   	return ret_val;
>   }
>   
> -static int cppc_get_perf(int cpunum, enum cppc_regs reg_idx, u64 *perf)
> +static int cppc_get_reg(int cpunum, enum cppc_regs reg_idx, u64 *val)
>   {
>   	struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpunum);
> +	struct cppc_pcc_data *pcc_ss_data = NULL;
>   	struct cpc_register_resource *reg;
> +	int pcc_ss_id;
> +	int ret = 0;

NIT: Might not be necessary if we save the value returned by cpc_read(),
cf. other comment below.

>   
>   	if (!cpc_desc) {
>   		pr_debug("No CPC descriptor for CPU:%d\n", cpunum);
> @@ -1191,20 +1194,23 @@ static int cppc_get_perf(int cpunum, enum cppc_regs reg_idx, u64 *perf)
>   
>   	reg = &cpc_desc->cpc_regs[reg_idx];
>   
> +	if (!CPC_SUPPORTED(reg)) {
> +		pr_debug("CPC register (reg_idx=%u) is not supported\n", reg_idx);
> +		return -EOPNOTSUPP;
> +	}
> +
>   	if (CPC_IN_PCC(reg)) {
> -		int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpunum);
> -		struct cppc_pcc_data *pcc_ss_data = NULL;
> -		int ret = 0;
> +		pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpunum);
>   
>   		if (pcc_ss_id < 0)
> -			return -EIO;
> +			return -ENODEV;

NIT: Could add here:
   pr_debug("Invalid pcc_ss_id\n");
just as you did in cppc_set_reg()

>   
>   		pcc_ss_data = pcc_data[pcc_ss_id];
>   
>   		down_write(&pcc_ss_data->pcc_lock);
>   
>   		if (send_pcc_cmd(pcc_ss_id, CMD_READ) >= 0)
> -			cpc_read(cpunum, reg, perf);
> +			cpc_read(cpunum, reg, val);

This was not introduced by your patch, but cpc_read() return a value.
Shouldn't we return it instead of 0 ?

>   		else
>   			ret = -EIO;
>   
> @@ -1213,21 +1219,65 @@ static int cppc_get_perf(int cpunum, enum cppc_regs reg_idx, u64 *perf)
>   		return ret;
>   	}
>   
> -	cpc_read(cpunum, reg, perf);
> +	cpc_read(cpunum, reg, val);

Same comment as above

>   
>   	return 0;
>   }
>   
> +static int cppc_set_reg(int cpu, enum cppc_regs reg_idx, u64 val)

Just to have similar functions, maybe 'cpu' should be renamed to 'cpunum' ?
Or the other way around.

> +{
> +	struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpu);
> +	struct cppc_pcc_data *pcc_ss_data = NULL;
> +	struct cpc_register_resource *reg;
> +	int pcc_ss_id;
> +	int ret;
> +
> +	if (!cpc_desc) {
> +		pr_debug("No CPC descriptor for CPU:%d\n", cpu);
> +		return -ENODEV;
> +	}
> +
> +	reg = &cpc_desc->cpc_regs[reg_idx];
> +
> +	if (!CPC_SUPPORTED(reg)) {
> +		pr_debug("CPC register (reg_idx=%u) is not supported\n", reg_idx);
> +		return -EOPNOTSUPP;
> +	}
> +
> +	if (CPC_IN_PCC(reg)) {
> +		pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpu);
> +
> +		if (pcc_ss_id < 0) {
> +			pr_debug("Invalid pcc_ss_id\n");
> +			return -ENODEV;
> +		}
> +
> +		ret = cpc_write(cpu, reg, val);
> +		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);
> +		return ret;
> +	}
> +
> +	return cpc_write(cpu, reg, val);
> +}
> +

[snip]

Re: [PATCH 1/3] ACPI: CPPC: Refactor register get and set ABIs

[zhenglifeng (A)]

Hello Pierre，

On 2024/12/6 22:23, Pierre Gondois wrote:
> Hello Lifeng,
> 
> On 11/14/24 09:48, Lifeng Zheng wrote:
>> Refactor register get and set ABIs using cppc_get_reg() and cppc_set_reg().
>>
>> Rename cppc_get_perf() to cppc_get_reg() as a generic function to read cppc
>> registers, with two changes:
>>
>> 1. Change the error kind to "no such device" when pcc_ss_id < 0, which
>> means that this cpu cannot get a valid pcc_ss_id.
>>
>> 2. Add a check to verify if the register is a cpc supported one before
>> using it.
>>
>> Add cppc_set_reg() as a generic function for setting cppc registers. Unlike
>> other set reg ABIs, this function checks CPC_SUPPORTED right after getting
>> the register, because the rest of the operations are meaningless if this
>> register is not a cpc supported one.
>>
>> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
>> ---
>>   drivers/acpi/cppc_acpi.c | 191 +++++++++++++++------------------------
>>   1 file changed, 72 insertions(+), 119 deletions(-)
>>
>> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
>> index c1f3568d0c50..306ced9c3376 100644
>> --- a/drivers/acpi/cppc_acpi.c
>> +++ b/drivers/acpi/cppc_acpi.c
>> @@ -1179,10 +1179,13 @@ static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val)
>>       return ret_val;
>>   }
>>   -static int cppc_get_perf(int cpunum, enum cppc_regs reg_idx, u64 *perf)
>> +static int cppc_get_reg(int cpunum, enum cppc_regs reg_idx, u64 *val)
>>   {
>>       struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpunum);
>> +    struct cppc_pcc_data *pcc_ss_data = NULL;
>>       struct cpc_register_resource *reg;
>> +    int pcc_ss_id;
>> +    int ret = 0;
> 
> NIT: Might not be necessary if we save the value returned by cpc_read(),
> cf. other comment below.
> 
>>         if (!cpc_desc) {
>>           pr_debug("No CPC descriptor for CPU:%d\n", cpunum);
>> @@ -1191,20 +1194,23 @@ static int cppc_get_perf(int cpunum, enum cppc_regs reg_idx, u64 *perf)
>>         reg = &cpc_desc->cpc_regs[reg_idx];
>>   +    if (!CPC_SUPPORTED(reg)) {
>> +        pr_debug("CPC register (reg_idx=%u) is not supported\n", reg_idx);
>> +        return -EOPNOTSUPP;
>> +    }
>> +
>>       if (CPC_IN_PCC(reg)) {
>> -        int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpunum);
>> -        struct cppc_pcc_data *pcc_ss_data = NULL;
>> -        int ret = 0;
>> +        pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpunum);
>>             if (pcc_ss_id < 0)
>> -            return -EIO;
>> +            return -ENODEV;
> 
> NIT: Could add here:
>   pr_debug("Invalid pcc_ss_id\n");
> just as you did in cppc_set_reg()

Will add it in next version, Thanks.

> 
>>             pcc_ss_data = pcc_data[pcc_ss_id];
>>             down_write(&pcc_ss_data->pcc_lock);
>>             if (send_pcc_cmd(pcc_ss_id, CMD_READ) >= 0)
>> -            cpc_read(cpunum, reg, perf);
>> +            cpc_read(cpunum, reg, val);
> 
> This was not introduced by your patch, but cpc_read() return a value.
> Shouldn't we return it instead of 0 ?

Indeed. Will optimize it, Thanks.

> 
>>           else
>>               ret = -EIO;
>>   @@ -1213,21 +1219,65 @@ static int cppc_get_perf(int cpunum, enum cppc_regs reg_idx, u64 *perf)
>>           return ret;
>>       }
>>   -    cpc_read(cpunum, reg, perf);
>> +    cpc_read(cpunum, reg, val);
> 
> Same comment as above
> 
>>         return 0;
>>   }
>>   +static int cppc_set_reg(int cpu, enum cppc_regs reg_idx, u64 val)
> 
> Just to have similar functions, maybe 'cpu' should be renamed to 'cpunum' ?
> Or the other way around.

I prefer 'cpu', 'cpunum' looks like the number of cpus to me.
Will rename 'cpunum' to 'cpu' in cppc_get_reg(). Thanks.

> 
>> +{
>> +    struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpu);
>> +    struct cppc_pcc_data *pcc_ss_data = NULL;
>> +    struct cpc_register_resource *reg;
>> +    int pcc_ss_id;
>> +    int ret;
>> +
>> +    if (!cpc_desc) {
>> +        pr_debug("No CPC descriptor for CPU:%d\n", cpu);
>> +        return -ENODEV;
>> +    }
>> +
>> +    reg = &cpc_desc->cpc_regs[reg_idx];
>> +
>> +    if (!CPC_SUPPORTED(reg)) {
>> +        pr_debug("CPC register (reg_idx=%u) is not supported\n", reg_idx);
>> +        return -EOPNOTSUPP;
>> +    }
>> +
>> +    if (CPC_IN_PCC(reg)) {
>> +        pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpu);
>> +
>> +        if (pcc_ss_id < 0) {
>> +            pr_debug("Invalid pcc_ss_id\n");
>> +            return -ENODEV;
>> +        }
>> +
>> +        ret = cpc_write(cpu, reg, val);
>> +        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);
>> +        return ret;
>> +    }
>> +
>> +    return cpc_write(cpu, reg, val);
>> +}
>> +
> 
> [snip]
> 

[PATCH 2/3] ACPI: CPPC: Add autonomous selection ABIs

[Lifeng Zheng]

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 306ced9c3376..f69ef7cc0caf 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -1588,6 +1588,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(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(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(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(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;
-- 
2.33.0

[PATCH 3/3] cpufreq: CPPC: Support for autonomous selection in cppc_cpufreq

[Lifeng Zheng]

Add sysfs interfaces for CPPC autonomous selection in the cppc_cpufreq
driver.

Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
---
 .../ABI/testing/sysfs-devices-system-cpu      |  54 +++++++
 drivers/cpufreq/cppc_cpufreq.c                | 141 ++++++++++++++++++
 2 files changed, 195 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu
index 206079d3bd5b..ba7b8ea613e5 100644
--- a/Documentation/ABI/testing/sysfs-devices-system-cpu
+++ b/Documentation/ABI/testing/sysfs-devices-system-cpu
@@ -268,6 +268,60 @@ Description:	Discover CPUs in the same CPU frequency coordination domain
 		This file is only present if the acpi-cpufreq or the cppc-cpufreq
 		drivers are in use.
 
+What:		/sys/devices/system/cpu/cpuX/cpufreq/auto_select
+Date:		October 2024
+Contact:	linux-pm@vger.kernel.org
+Description:	Autonomous selection enable
+
+		Read/write interface to control autonomous selection enable
+			Read returns autonomous selection status:
+				0: autonomous selection is disabled
+				1: autonomous selection is enabled
+
+			Write '1' to enable autonomous selection.
+			Write '0' to disable autonomous selection.
+
+		This file only presents if the cppc-cpufreq driver is in use.
+
+What:		/sys/devices/system/cpu/cpuX/cpufreq/auto_act_window
+Date:		October 2024
+Contact:	linux-pm@vger.kernel.org
+Description:	Autonomous activity window
+
+		This file indicates a moving utilization sensitivity window to
+		the platform's autonomous selection policy.
+
+		Read/write an integer represents autonomous activity window (in
+		microseconds) from/to this file. The max value to write is
+		1270000000 but the max significand is 127. This means that if 128
+		is written to this file, 127 will be stored. If the value is
+		greater than 130, only the first two digits will be saved as
+		significand.
+
+		Writing a zero value to this file enable the platform to
+		determine an appropriate Activity Window depending on the workload.
+
+		Writing to this file only has meaning when Autonomous Selection is
+		enabled.
+
+		This file only presents if the cppc-cpufreq driver is in use.
+
+What:		/sys/devices/system/cpu/cpuX/cpufreq/energy_perf
+Date:		October 2024
+Contact:	linux-pm@vger.kernel.org
+Description:	Energy performance preference
+
+		Read/write an 8-bit integer from/to this file. This file
+		represents a range of values from 0 (performance preference) to
+		0xFF (energy efficiency preference) that influences the rate of
+		performance increase/decrease and the result of the hardware's
+		energy efficiency and performance optimization policies.
+
+		Writing to this file only has meaning when Autonomous Selection is
+		enabled.
+
+		This file only presents if the cppc-cpufreq driver is in use.
+
 
 What:		/sys/devices/system/cpu/cpu*/cache/index3/cache_disable_{0,1}
 Date:		August 2008
diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
index 2b8708475ac7..b435e1751d0d 100644
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -792,10 +792,151 @@ static ssize_t show_freqdomain_cpus(struct cpufreq_policy *policy, char *buf)
 
 	return cpufreq_show_cpus(cpu_data->shared_cpu_map, buf);
 }
+
+static ssize_t show_auto_select(struct cpufreq_policy *policy, char *buf)
+{
+	u64 val;
+	int ret;
+
+	ret = cppc_get_auto_sel(policy->cpu, &val);
+
+	/* show "<unsupported>" when this register is not supported by cpc */
+	if (ret == -EOPNOTSUPP)
+		return sysfs_emit(buf, "%s\n", "<unsupported>");
+
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "%lld\n", val);
+}
+
+static ssize_t store_auto_select(struct cpufreq_policy *policy,
+				 const char *buf, size_t count)
+{
+	unsigned long val;
+	int ret;
+
+	ret = kstrtoul(buf, 0, &val);
+	if (ret)
+		return ret;
+
+	if (val > 1)
+		return -EINVAL;
+
+	ret = cppc_set_auto_sel(policy->cpu, val);
+	if (ret)
+		return ret;
+
+	return count;
+}
+
+#define AUTO_ACT_WINDOW_SIG_BIT_SIZE	(7)
+#define AUTO_ACT_WINDOW_EXP_BIT_SIZE	(3)
+#define AUTO_ACT_WINDOW_MAX_SIG	((1 << AUTO_ACT_WINDOW_SIG_BIT_SIZE) - 1)
+#define AUTO_ACT_WINDOW_MAX_EXP	((1 << AUTO_ACT_WINDOW_EXP_BIT_SIZE) - 1)
+/* AUTO_ACT_WINDOW_MAX_SIG is 127, so 128 and 129 will decay to 127 when writing */
+#define AUTO_ACT_WINDOW_SIG_CARRY_THRESH 129
+
+static ssize_t show_auto_act_window(struct cpufreq_policy *policy, char *buf)
+{
+	int sig, exp;
+	u64 val;
+	int ret;
+
+	ret = cppc_get_auto_act_window(policy->cpu, &val);
+
+	/* show "<unsupported>" when this register is not supported by cpc */
+	if (ret == -EOPNOTSUPP)
+		return sysfs_emit(buf, "%s\n", "<unsupported>");
+
+	if (ret)
+		return ret;
+
+	sig = val & AUTO_ACT_WINDOW_MAX_SIG;
+	exp = (val >> AUTO_ACT_WINDOW_SIG_BIT_SIZE) & AUTO_ACT_WINDOW_MAX_EXP;
+
+	return sysfs_emit(buf, "%lld\n", sig * int_pow(10, exp));
+}
+
+static ssize_t store_auto_act_window(struct cpufreq_policy *policy,
+				     const char *buf, size_t count)
+{
+	unsigned long usec;
+	int digits = 0;
+	int ret;
+
+	ret = kstrtoul(buf, 0, &usec);
+	if (ret)
+		return ret;
+
+	if (usec > AUTO_ACT_WINDOW_MAX_SIG * int_pow(10, AUTO_ACT_WINDOW_MAX_EXP))
+		return -EINVAL;
+
+	while (usec > AUTO_ACT_WINDOW_SIG_CARRY_THRESH) {
+		usec /= 10;
+		digits += 1;
+	}
+
+	if (usec > AUTO_ACT_WINDOW_MAX_SIG)
+		usec = AUTO_ACT_WINDOW_MAX_SIG;
+
+	ret = cppc_set_auto_act_window(policy->cpu,
+				       (digits << AUTO_ACT_WINDOW_SIG_BIT_SIZE) + usec);
+	if (ret)
+		return ret;
+
+	return count;
+}
+
+static ssize_t show_energy_perf(struct cpufreq_policy *policy, char *buf)
+{
+	u64 val;
+	int ret;
+
+	ret = cppc_get_epp_perf(policy->cpu, &val);
+
+	/* show "<unsupported>" when this register is not supported by cpc */
+	if (ret == -EOPNOTSUPP)
+		return sysfs_emit(buf, "%s\n", "<unsupported>");
+
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "%lld\n", val);
+}
+
+#define ENERGY_PERF_MAX	(0xFF)
+
+static ssize_t store_energy_perf(struct cpufreq_policy *policy,
+				 const char *buf, size_t count)
+{
+	unsigned long val;
+	int ret;
+
+	ret = kstrtoul(buf, 0, &val);
+	if (ret)
+		return ret;
+
+	if (val > ENERGY_PERF_MAX)
+		return -EINVAL;
+
+	ret = cppc_set_epp(policy->cpu, val);
+	if (ret)
+		return ret;
+
+	return count;
+}
+
 cpufreq_freq_attr_ro(freqdomain_cpus);
+cpufreq_freq_attr_rw(auto_select);
+cpufreq_freq_attr_rw(auto_act_window);
+cpufreq_freq_attr_rw(energy_perf);
 
 static struct freq_attr *cppc_cpufreq_attr[] = {
 	&freqdomain_cpus,
+	&auto_select,
+	&auto_act_window,
+	&energy_perf,
 	NULL,
 };
 
-- 
2.33.0

Re: [PATCH 3/3] cpufreq: CPPC: Support for autonomous selection in cppc_cpufreq

[Pierre Gondois]

Hello Lifeng,

On 11/14/24 09:48, Lifeng Zheng wrote:
> Add sysfs interfaces for CPPC autonomous selection in the cppc_cpufreq
> driver.
> 
> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
> ---
>   .../ABI/testing/sysfs-devices-system-cpu      |  54 +++++++
>   drivers/cpufreq/cppc_cpufreq.c                | 141 ++++++++++++++++++
>   2 files changed, 195 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu
> index 206079d3bd5b..ba7b8ea613e5 100644
> --- a/Documentation/ABI/testing/sysfs-devices-system-cpu
> +++ b/Documentation/ABI/testing/sysfs-devices-system-cpu
> @@ -268,6 +268,60 @@ Description:	Discover CPUs in the same CPU frequency coordination domain
>   		This file is only present if the acpi-cpufreq or the cppc-cpufreq
>   		drivers are in use.
>   
> +What:		/sys/devices/system/cpu/cpuX/cpufreq/auto_select
> +Date:		October 2024
> +Contact:	linux-pm@vger.kernel.org
> +Description:	Autonomous selection enable
> +
> +		Read/write interface to control autonomous selection enable
> +			Read returns autonomous selection status:
> +				0: autonomous selection is disabled
> +				1: autonomous selection is enabled
> +
> +			Write '1' to enable autonomous selection.
> +			Write '0' to disable autonomous selection.
> +
> +		This file only presents if the cppc-cpufreq driver is in use.
> +
> +What:		/sys/devices/system/cpu/cpuX/cpufreq/auto_act_window
> +Date:		October 2024
> +Contact:	linux-pm@vger.kernel.org
> +Description:	Autonomous activity window
> +
> +		This file indicates a moving utilization sensitivity window to
> +		the platform's autonomous selection policy.
> +
> +		Read/write an integer represents autonomous activity window (in
> +		microseconds) from/to this file. The max value to write is
> +		1270000000 but the max significand is 127. This means that if 128
> +		is written to this file, 127 will be stored. If the value is
> +		greater than 130, only the first two digits will be saved as
> +		significand.
> +
> +		Writing a zero value to this file enable the platform to
> +		determine an appropriate Activity Window depending on the workload.
> +
> +		Writing to this file only has meaning when Autonomous Selection is
> +		enabled.
> +
> +		This file only presents if the cppc-cpufreq driver is in use.
> +
> +What:		/sys/devices/system/cpu/cpuX/cpufreq/energy_perf
> +Date:		October 2024
> +Contact:	linux-pm@vger.kernel.org
> +Description:	Energy performance preference
> +
> +		Read/write an 8-bit integer from/to this file. This file
> +		represents a range of values from 0 (performance preference) to
> +		0xFF (energy efficiency preference) that influences the rate of
> +		performance increase/decrease and the result of the hardware's
> +		energy efficiency and performance optimization policies.
> +
> +		Writing to this file only has meaning when Autonomous Selection is
> +		enabled.
> +
> +		This file only presents if the cppc-cpufreq driver is in use.
> +
>   
>   What:		/sys/devices/system/cpu/cpu*/cache/index3/cache_disable_{0,1}
>   Date:		August 2008
> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> index 2b8708475ac7..b435e1751d0d 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -792,10 +792,151 @@ static ssize_t show_freqdomain_cpus(struct cpufreq_policy *policy, char *buf)
>   
>   	return cpufreq_show_cpus(cpu_data->shared_cpu_map, buf);
>   }
> +
> +static ssize_t show_auto_select(struct cpufreq_policy *policy, char *buf)
> +{
> +	u64 val;
> +	int ret;
> +
> +	ret = cppc_get_auto_sel(policy->cpu, &val);
> +
> +	/* show "<unsupported>" when this register is not supported by cpc */
> +	if (ret == -EOPNOTSUPP)
> +		return sysfs_emit(buf, "%s\n", "<unsupported>");
> +
> +	if (ret)
> +		return ret;
> +
> +	return sysfs_emit(buf, "%lld\n", val);
> +}
> +
> +static ssize_t store_auto_select(struct cpufreq_policy *policy,
> +				 const char *buf, size_t count)
> +{
> +	unsigned long val;
> +	int ret;
> +
> +	ret = kstrtoul(buf, 0, &val);
> +	if (ret)
> +		return ret;
> +
> +	if (val > 1)
> +		return -EINVAL;
> +
> +	ret = cppc_set_auto_sel(policy->cpu, val);
> +	if (ret)
> +		return ret;
> +
> +	return count;
> +}
> +
> +#define AUTO_ACT_WINDOW_SIG_BIT_SIZE	(7)
> +#define AUTO_ACT_WINDOW_EXP_BIT_SIZE	(3)
> +#define AUTO_ACT_WINDOW_MAX_SIG	((1 << AUTO_ACT_WINDOW_SIG_BIT_SIZE) - 1)
> +#define AUTO_ACT_WINDOW_MAX_EXP	((1 << AUTO_ACT_WINDOW_EXP_BIT_SIZE) - 1)
> +/* AUTO_ACT_WINDOW_MAX_SIG is 127, so 128 and 129 will decay to 127 when writing */
> +#define AUTO_ACT_WINDOW_SIG_CARRY_THRESH 129

Maybe this would be better to place these macros in include/acpi/cppc_acpi.h
(with a CPPC_XXX prefix)

> +
> +static ssize_t show_auto_act_window(struct cpufreq_policy *policy, char *buf)
> +{
> +	int sig, exp;
> +	u64 val;
> +	int ret;
> +
> +	ret = cppc_get_auto_act_window(policy->cpu, &val);
> +
> +	/* show "<unsupported>" when this register is not supported by cpc */
> +	if (ret == -EOPNOTSUPP)
> +		return sysfs_emit(buf, "%s\n", "<unsupported>");
> +
> +	if (ret)
> +		return ret;
> +
> +	sig = val & AUTO_ACT_WINDOW_MAX_SIG;
> +	exp = (val >> AUTO_ACT_WINDOW_SIG_BIT_SIZE) & AUTO_ACT_WINDOW_MAX_EXP;
> +
> +	return sysfs_emit(buf, "%lld\n", sig * int_pow(10, exp));
> +}
> +
> +static ssize_t store_auto_act_window(struct cpufreq_policy *policy,
> +				     const char *buf, size_t count)
> +{
> +	unsigned long usec;
> +	int digits = 0;
> +	int ret;
> +
> +	ret = kstrtoul(buf, 0, &usec);
> +	if (ret)
> +		return ret;
> +
> +	if (usec > AUTO_ACT_WINDOW_MAX_SIG * int_pow(10, AUTO_ACT_WINDOW_MAX_EXP))
> +		return -EINVAL;
> +
> +	while (usec > AUTO_ACT_WINDOW_SIG_CARRY_THRESH) {
> +		usec /= 10;
> +		digits += 1;
> +	}
> +
> +	if (usec > AUTO_ACT_WINDOW_MAX_SIG)
> +		usec = AUTO_ACT_WINDOW_MAX_SIG;
> +
> +	ret = cppc_set_auto_act_window(policy->cpu,
> +				       (digits << AUTO_ACT_WINDOW_SIG_BIT_SIZE) + usec);
> +	if (ret)
> +		return ret;
> +
> +	return count;
> +}
> +
> +static ssize_t show_energy_perf(struct cpufreq_policy *policy, char *buf)
> +{
> +	u64 val;
> +	int ret;
> +
> +	ret = cppc_get_epp_perf(policy->cpu, &val);
> +
> +	/* show "<unsupported>" when this register is not supported by cpc */
> +	if (ret == -EOPNOTSUPP)
> +		return sysfs_emit(buf, "%s\n", "<unsupported>");
> +
> +	if (ret)
> +		return ret;
> +
> +	return sysfs_emit(buf, "%lld\n", val);
> +}
> +
> +#define ENERGY_PERF_MAX	(0xFF)

Same comment to move to include/acpi/cppc_acpi.h

> +
> +static ssize_t store_energy_perf(struct cpufreq_policy *policy,
> +				 const char *buf, size_t count)
> +{
> +	unsigned long val;
> +	int ret;
> +
> +	ret = kstrtoul(buf, 0, &val);
> +	if (ret)
> +		return ret;
> +
> +	if (val > ENERGY_PERF_MAX)
> +		return -EINVAL;
> +
> +	ret = cppc_set_epp(policy->cpu, val);
> +	if (ret)
> +		return ret;
> +
> +	return count;
> +}
> +
>   cpufreq_freq_attr_ro(freqdomain_cpus);
> +cpufreq_freq_attr_rw(auto_select);
> +cpufreq_freq_attr_rw(auto_act_window);
> +cpufreq_freq_attr_rw(energy_perf);

It might be better from a user PoV to hide the following entries:
- auto_act_window
- energy_perf
if auto_select is not available or disabled.

------

Also just for reference, in ACPI 6.5, s8.4.6.1.2.3 Desired Performance Register
"""
When Autonomous Selection is enabled, it is not necessary for OSPM to assess processor workload performance
demand and convey a corresponding performance delivery request to the platform via the Desired Register. If the
Desired Performance Register exists, OSPM may provide an explicit performance requirement hint to the platform by
writing a non-zero value.
"""

So it seems it still makes sense to have cpufreq requesting a certain performance
level even though autonomous selection is enabled.

Re: [PATCH 3/3] cpufreq: CPPC: Support for autonomous selection in cppc_cpufreq

[zhenglifeng (A)]

Hello Pierre,

On 2024/12/6 22:23, Pierre Gondois wrote:
> Hello Lifeng,
> 
> On 11/14/24 09:48, Lifeng Zheng wrote:
>> Add sysfs interfaces for CPPC autonomous selection in the cppc_cpufreq
>> driver.
>>
>> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
>> ---
>>   .../ABI/testing/sysfs-devices-system-cpu      |  54 +++++++
>>   drivers/cpufreq/cppc_cpufreq.c                | 141 ++++++++++++++++++
>>   2 files changed, 195 insertions(+)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu
>> index 206079d3bd5b..ba7b8ea613e5 100644
>> --- a/Documentation/ABI/testing/sysfs-devices-system-cpu
>> +++ b/Documentation/ABI/testing/sysfs-devices-system-cpu
>> @@ -268,6 +268,60 @@ Description:    Discover CPUs in the same CPU frequency coordination domain
>>           This file is only present if the acpi-cpufreq or the cppc-cpufreq
>>           drivers are in use.
>>   +What:        /sys/devices/system/cpu/cpuX/cpufreq/auto_select
>> +Date:        October 2024
>> +Contact:    linux-pm@vger.kernel.org
>> +Description:    Autonomous selection enable
>> +
>> +        Read/write interface to control autonomous selection enable
>> +            Read returns autonomous selection status:
>> +                0: autonomous selection is disabled
>> +                1: autonomous selection is enabled
>> +
>> +            Write '1' to enable autonomous selection.
>> +            Write '0' to disable autonomous selection.
>> +
>> +        This file only presents if the cppc-cpufreq driver is in use.
>> +
>> +What:        /sys/devices/system/cpu/cpuX/cpufreq/auto_act_window
>> +Date:        October 2024
>> +Contact:    linux-pm@vger.kernel.org
>> +Description:    Autonomous activity window
>> +
>> +        This file indicates a moving utilization sensitivity window to
>> +        the platform's autonomous selection policy.
>> +
>> +        Read/write an integer represents autonomous activity window (in
>> +        microseconds) from/to this file. The max value to write is
>> +        1270000000 but the max significand is 127. This means that if 128
>> +        is written to this file, 127 will be stored. If the value is
>> +        greater than 130, only the first two digits will be saved as
>> +        significand.
>> +
>> +        Writing a zero value to this file enable the platform to
>> +        determine an appropriate Activity Window depending on the workload.
>> +
>> +        Writing to this file only has meaning when Autonomous Selection is
>> +        enabled.
>> +
>> +        This file only presents if the cppc-cpufreq driver is in use.
>> +
>> +What:        /sys/devices/system/cpu/cpuX/cpufreq/energy_perf
>> +Date:        October 2024
>> +Contact:    linux-pm@vger.kernel.org
>> +Description:    Energy performance preference
>> +
>> +        Read/write an 8-bit integer from/to this file. This file
>> +        represents a range of values from 0 (performance preference) to
>> +        0xFF (energy efficiency preference) that influences the rate of
>> +        performance increase/decrease and the result of the hardware's
>> +        energy efficiency and performance optimization policies.
>> +
>> +        Writing to this file only has meaning when Autonomous Selection is
>> +        enabled.
>> +
>> +        This file only presents if the cppc-cpufreq driver is in use.
>> +
>>     What:        /sys/devices/system/cpu/cpu*/cache/index3/cache_disable_{0,1}
>>   Date:        August 2008
>> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
>> index 2b8708475ac7..b435e1751d0d 100644
>> --- a/drivers/cpufreq/cppc_cpufreq.c
>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>> @@ -792,10 +792,151 @@ static ssize_t show_freqdomain_cpus(struct cpufreq_policy *policy, char *buf)
>>         return cpufreq_show_cpus(cpu_data->shared_cpu_map, buf);
>>   }
>> +
>> +static ssize_t show_auto_select(struct cpufreq_policy *policy, char *buf)
>> +{
>> +    u64 val;
>> +    int ret;
>> +
>> +    ret = cppc_get_auto_sel(policy->cpu, &val);
>> +
>> +    /* show "<unsupported>" when this register is not supported by cpc */
>> +    if (ret == -EOPNOTSUPP)
>> +        return sysfs_emit(buf, "%s\n", "<unsupported>");
>> +
>> +    if (ret)
>> +        return ret;
>> +
>> +    return sysfs_emit(buf, "%lld\n", val);
>> +}
>> +
>> +static ssize_t store_auto_select(struct cpufreq_policy *policy,
>> +                 const char *buf, size_t count)
>> +{
>> +    unsigned long val;
>> +    int ret;
>> +
>> +    ret = kstrtoul(buf, 0, &val);
>> +    if (ret)
>> +        return ret;
>> +
>> +    if (val > 1)
>> +        return -EINVAL;
>> +
>> +    ret = cppc_set_auto_sel(policy->cpu, val);
>> +    if (ret)
>> +        return ret;
>> +
>> +    return count;
>> +}
>> +
>> +#define AUTO_ACT_WINDOW_SIG_BIT_SIZE    (7)
>> +#define AUTO_ACT_WINDOW_EXP_BIT_SIZE    (3)
>> +#define AUTO_ACT_WINDOW_MAX_SIG    ((1 << AUTO_ACT_WINDOW_SIG_BIT_SIZE) - 1)
>> +#define AUTO_ACT_WINDOW_MAX_EXP    ((1 << AUTO_ACT_WINDOW_EXP_BIT_SIZE) - 1)
>> +/* AUTO_ACT_WINDOW_MAX_SIG is 127, so 128 and 129 will decay to 127 when writing */
>> +#define AUTO_ACT_WINDOW_SIG_CARRY_THRESH 129
> 
> Maybe this would be better to place these macros in include/acpi/cppc_acpi.h
> (with a CPPC_XXX prefix)

Will move them, Thanks.

> 
>> +
>> +static ssize_t show_auto_act_window(struct cpufreq_policy *policy, char *buf)
>> +{
>> +    int sig, exp;
>> +    u64 val;
>> +    int ret;
>> +
>> +    ret = cppc_get_auto_act_window(policy->cpu, &val);
>> +
>> +    /* show "<unsupported>" when this register is not supported by cpc */
>> +    if (ret == -EOPNOTSUPP)
>> +        return sysfs_emit(buf, "%s\n", "<unsupported>");
>> +
>> +    if (ret)
>> +        return ret;
>> +
>> +    sig = val & AUTO_ACT_WINDOW_MAX_SIG;
>> +    exp = (val >> AUTO_ACT_WINDOW_SIG_BIT_SIZE) & AUTO_ACT_WINDOW_MAX_EXP;
>> +
>> +    return sysfs_emit(buf, "%lld\n", sig * int_pow(10, exp));
>> +}
>> +
>> +static ssize_t store_auto_act_window(struct cpufreq_policy *policy,
>> +                     const char *buf, size_t count)
>> +{
>> +    unsigned long usec;
>> +    int digits = 0;
>> +    int ret;
>> +
>> +    ret = kstrtoul(buf, 0, &usec);
>> +    if (ret)
>> +        return ret;
>> +
>> +    if (usec > AUTO_ACT_WINDOW_MAX_SIG * int_pow(10, AUTO_ACT_WINDOW_MAX_EXP))
>> +        return -EINVAL;
>> +
>> +    while (usec > AUTO_ACT_WINDOW_SIG_CARRY_THRESH) {
>> +        usec /= 10;
>> +        digits += 1;
>> +    }
>> +
>> +    if (usec > AUTO_ACT_WINDOW_MAX_SIG)
>> +        usec = AUTO_ACT_WINDOW_MAX_SIG;
>> +
>> +    ret = cppc_set_auto_act_window(policy->cpu,
>> +                       (digits << AUTO_ACT_WINDOW_SIG_BIT_SIZE) + usec);
>> +    if (ret)
>> +        return ret;
>> +
>> +    return count;
>> +}
>> +
>> +static ssize_t show_energy_perf(struct cpufreq_policy *policy, char *buf)
>> +{
>> +    u64 val;
>> +    int ret;
>> +
>> +    ret = cppc_get_epp_perf(policy->cpu, &val);
>> +
>> +    /* show "<unsupported>" when this register is not supported by cpc */
>> +    if (ret == -EOPNOTSUPP)
>> +        return sysfs_emit(buf, "%s\n", "<unsupported>");
>> +
>> +    if (ret)
>> +        return ret;
>> +
>> +    return sysfs_emit(buf, "%lld\n", val);
>> +}
>> +
>> +#define ENERGY_PERF_MAX    (0xFF)
> 
> Same comment to move to include/acpi/cppc_acpi.h
> 
>> +
>> +static ssize_t store_energy_perf(struct cpufreq_policy *policy,
>> +                 const char *buf, size_t count)
>> +{
>> +    unsigned long val;
>> +    int ret;
>> +
>> +    ret = kstrtoul(buf, 0, &val);
>> +    if (ret)
>> +        return ret;
>> +
>> +    if (val > ENERGY_PERF_MAX)
>> +        return -EINVAL;
>> +
>> +    ret = cppc_set_epp(policy->cpu, val);
>> +    if (ret)
>> +        return ret;
>> +
>> +    return count;
>> +}
>> +
>>   cpufreq_freq_attr_ro(freqdomain_cpus);
>> +cpufreq_freq_attr_rw(auto_select);
>> +cpufreq_freq_attr_rw(auto_act_window);
>> +cpufreq_freq_attr_rw(energy_perf);
> 
> It might be better from a user PoV to hide the following entries:
> - auto_act_window
> - energy_perf
> if auto_select is not available or disabled.

Users might like to modify the value of auto_act_window and energy_perf
before turning on auto_select. So I think it is freer for users to read and
write them no matter what auto_select is. What do you think?

> 
> ------
> 
> Also just for reference, in ACPI 6.5, s8.4.6.1.2.3 Desired Performance Register
> """
> When Autonomous Selection is enabled, it is not necessary for OSPM to assess processor workload performance
> demand and convey a corresponding performance delivery request to the platform via the Desired Register. If the
> Desired Performance Register exists, OSPM may provide an explicit performance requirement hint to the platform by
> writing a non-zero value.
> """
> 
> So it seems it still makes sense to have cpufreq requesting a certain performance
> level even though autonomous selection is enabled.

We did struggle with this. This solves our doubts. Thanks!

> 

Re: [PATCH 3/3] cpufreq: CPPC: Support for autonomous selection in cppc_cpufreq

[Pierre Gondois]

Hello Lifeng,

On 12/9/24 09:40, zhenglifeng (A) wrote:
> Hello Pierre,
> 
> On 2024/12/6 22:23, Pierre Gondois wrote:
>> Hello Lifeng,
>>
>> On 11/14/24 09:48, Lifeng Zheng wrote:
>>> Add sysfs interfaces for CPPC autonomous selection in the cppc_cpufreq
>>> driver.
>>>
>>> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
>>> ---
>>>    .../ABI/testing/sysfs-devices-system-cpu      |  54 +++++++
>>>    drivers/cpufreq/cppc_cpufreq.c                | 141 ++++++++++++++++++
>>>    2 files changed, 195 insertions(+)
>>>
>>> diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu
>>> index 206079d3bd5b..ba7b8ea613e5 100644
>>> --- a/Documentation/ABI/testing/sysfs-devices-system-cpu
>>> +++ b/Documentation/ABI/testing/sysfs-devices-system-cpu
>>> @@ -268,6 +268,60 @@ Description:    Discover CPUs in the same CPU frequency coordination domain
>>>            This file is only present if the acpi-cpufreq or the cppc-cpufreq
>>>            drivers are in use.
>>>    +What:        /sys/devices/system/cpu/cpuX/cpufreq/auto_select
>>> +Date:        October 2024
>>> +Contact:    linux-pm@vger.kernel.org
>>> +Description:    Autonomous selection enable
>>> +
>>> +        Read/write interface to control autonomous selection enable
>>> +            Read returns autonomous selection status:
>>> +                0: autonomous selection is disabled
>>> +                1: autonomous selection is enabled
>>> +
>>> +            Write '1' to enable autonomous selection.
>>> +            Write '0' to disable autonomous selection.
>>> +
>>> +        This file only presents if the cppc-cpufreq driver is in use.
>>> +
>>> +What:        /sys/devices/system/cpu/cpuX/cpufreq/auto_act_window
>>> +Date:        October 2024
>>> +Contact:    linux-pm@vger.kernel.org
>>> +Description:    Autonomous activity window
>>> +
>>> +        This file indicates a moving utilization sensitivity window to
>>> +        the platform's autonomous selection policy.
>>> +
>>> +        Read/write an integer represents autonomous activity window (in
>>> +        microseconds) from/to this file. The max value to write is
>>> +        1270000000 but the max significand is 127. This means that if 128
>>> +        is written to this file, 127 will be stored. If the value is
>>> +        greater than 130, only the first two digits will be saved as
>>> +        significand.
>>> +
>>> +        Writing a zero value to this file enable the platform to
>>> +        determine an appropriate Activity Window depending on the workload.
>>> +
>>> +        Writing to this file only has meaning when Autonomous Selection is
>>> +        enabled.
>>> +
>>> +        This file only presents if the cppc-cpufreq driver is in use.
>>> +
>>> +What:        /sys/devices/system/cpu/cpuX/cpufreq/energy_perf
>>> +Date:        October 2024
>>> +Contact:    linux-pm@vger.kernel.org
>>> +Description:    Energy performance preference
>>> +
>>> +        Read/write an 8-bit integer from/to this file. This file
>>> +        represents a range of values from 0 (performance preference) to
>>> +        0xFF (energy efficiency preference) that influences the rate of
>>> +        performance increase/decrease and the result of the hardware's
>>> +        energy efficiency and performance optimization policies.
>>> +
>>> +        Writing to this file only has meaning when Autonomous Selection is
>>> +        enabled.
>>> +
>>> +        This file only presents if the cppc-cpufreq driver is in use.
>>> +
>>>      What:        /sys/devices/system/cpu/cpu*/cache/index3/cache_disable_{0,1}
>>>    Date:        August 2008
>>> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
>>> index 2b8708475ac7..b435e1751d0d 100644
>>> --- a/drivers/cpufreq/cppc_cpufreq.c
>>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>>> @@ -792,10 +792,151 @@ static ssize_t show_freqdomain_cpus(struct cpufreq_policy *policy, char *buf)
>>>          return cpufreq_show_cpus(cpu_data->shared_cpu_map, buf);
>>>    }
>>> +
>>> +static ssize_t show_auto_select(struct cpufreq_policy *policy, char *buf)
>>> +{
>>> +    u64 val;
>>> +    int ret;
>>> +
>>> +    ret = cppc_get_auto_sel(policy->cpu, &val);
>>> +
>>> +    /* show "<unsupported>" when this register is not supported by cpc */
>>> +    if (ret == -EOPNOTSUPP)
>>> +        return sysfs_emit(buf, "%s\n", "<unsupported>");
>>> +
>>> +    if (ret)
>>> +        return ret;
>>> +
>>> +    return sysfs_emit(buf, "%lld\n", val);
>>> +}
>>> +
>>> +static ssize_t store_auto_select(struct cpufreq_policy *policy,
>>> +                 const char *buf, size_t count)
>>> +{
>>> +    unsigned long val;
>>> +    int ret;
>>> +
>>> +    ret = kstrtoul(buf, 0, &val);
>>> +    if (ret)
>>> +        return ret;
>>> +
>>> +    if (val > 1)
>>> +        return -EINVAL;
>>> +
>>> +    ret = cppc_set_auto_sel(policy->cpu, val);
>>> +    if (ret)
>>> +        return ret;
>>> +
>>> +    return count;
>>> +}
>>> +
>>> +#define AUTO_ACT_WINDOW_SIG_BIT_SIZE    (7)
>>> +#define AUTO_ACT_WINDOW_EXP_BIT_SIZE    (3)
>>> +#define AUTO_ACT_WINDOW_MAX_SIG    ((1 << AUTO_ACT_WINDOW_SIG_BIT_SIZE) - 1)
>>> +#define AUTO_ACT_WINDOW_MAX_EXP    ((1 << AUTO_ACT_WINDOW_EXP_BIT_SIZE) - 1)
>>> +/* AUTO_ACT_WINDOW_MAX_SIG is 127, so 128 and 129 will decay to 127 when writing */
>>> +#define AUTO_ACT_WINDOW_SIG_CARRY_THRESH 129
>>
>> Maybe this would be better to place these macros in include/acpi/cppc_acpi.h
>> (with a CPPC_XXX prefix)
> 
> Will move them, Thanks.
> 
>>
>>> +
>>> +static ssize_t show_auto_act_window(struct cpufreq_policy *policy, char *buf)
>>> +{
>>> +    int sig, exp;
>>> +    u64 val;
>>> +    int ret;
>>> +
>>> +    ret = cppc_get_auto_act_window(policy->cpu, &val);
>>> +
>>> +    /* show "<unsupported>" when this register is not supported by cpc */
>>> +    if (ret == -EOPNOTSUPP)
>>> +        return sysfs_emit(buf, "%s\n", "<unsupported>");
>>> +
>>> +    if (ret)
>>> +        return ret;
>>> +
>>> +    sig = val & AUTO_ACT_WINDOW_MAX_SIG;
>>> +    exp = (val >> AUTO_ACT_WINDOW_SIG_BIT_SIZE) & AUTO_ACT_WINDOW_MAX_EXP;
>>> +
>>> +    return sysfs_emit(buf, "%lld\n", sig * int_pow(10, exp));
>>> +}
>>> +
>>> +static ssize_t store_auto_act_window(struct cpufreq_policy *policy,
>>> +                     const char *buf, size_t count)
>>> +{
>>> +    unsigned long usec;
>>> +    int digits = 0;
>>> +    int ret;
>>> +
>>> +    ret = kstrtoul(buf, 0, &usec);
>>> +    if (ret)
>>> +        return ret;
>>> +
>>> +    if (usec > AUTO_ACT_WINDOW_MAX_SIG * int_pow(10, AUTO_ACT_WINDOW_MAX_EXP))
>>> +        return -EINVAL;
>>> +
>>> +    while (usec > AUTO_ACT_WINDOW_SIG_CARRY_THRESH) {
>>> +        usec /= 10;
>>> +        digits += 1;
>>> +    }
>>> +
>>> +    if (usec > AUTO_ACT_WINDOW_MAX_SIG)
>>> +        usec = AUTO_ACT_WINDOW_MAX_SIG;
>>> +
>>> +    ret = cppc_set_auto_act_window(policy->cpu,
>>> +                       (digits << AUTO_ACT_WINDOW_SIG_BIT_SIZE) + usec);
>>> +    if (ret)
>>> +        return ret;
>>> +
>>> +    return count;
>>> +}

Also I tested the logic and it was working correctly for me.

>>> +
>>> +static ssize_t show_energy_perf(struct cpufreq_policy *policy, char *buf)
>>> +{
>>> +    u64 val;
>>> +    int ret;
>>> +
>>> +    ret = cppc_get_epp_perf(policy->cpu, &val);
>>> +
>>> +    /* show "<unsupported>" when this register is not supported by cpc */
>>> +    if (ret == -EOPNOTSUPP)
>>> +        return sysfs_emit(buf, "%s\n", "<unsupported>");
>>> +
>>> +    if (ret)
>>> +        return ret;
>>> +
>>> +    return sysfs_emit(buf, "%lld\n", val);
>>> +}
>>> +
>>> +#define ENERGY_PERF_MAX    (0xFF)
>>
>> Same comment to move to include/acpi/cppc_acpi.h
>>
>>> +
>>> +static ssize_t store_energy_perf(struct cpufreq_policy *policy,
>>> +                 const char *buf, size_t count)
>>> +{
>>> +    unsigned long val;
>>> +    int ret;
>>> +
>>> +    ret = kstrtoul(buf, 0, &val);
>>> +    if (ret)
>>> +        return ret;
>>> +
>>> +    if (val > ENERGY_PERF_MAX)
>>> +        return -EINVAL;
>>> +
>>> +    ret = cppc_set_epp(policy->cpu, val);
>>> +    if (ret)
>>> +        return ret;
>>> +
>>> +    return count;
>>> +}
>>> +
>>>    cpufreq_freq_attr_ro(freqdomain_cpus);
>>> +cpufreq_freq_attr_rw(auto_select);
>>> +cpufreq_freq_attr_rw(auto_act_window);
>>> +cpufreq_freq_attr_rw(energy_perf);
>>
>> It might be better from a user PoV to hide the following entries:
>> - auto_act_window
>> - energy_perf
>> if auto_select is not available or disabled.
> 
> Users might like to modify the value of auto_act_window and energy_perf
> before turning on auto_select. So I think it is freer for users to read and
> write them no matter what auto_select is. What do you think?

Autonomous selection is not the most common case for the CPPC cpufreq drivers,
so these new files might bring questions to people currently using it.

On the other side, making these files visible only when 'auto_select' is enabled
will require additional logic in the code (while the current implementation is
quite clear).

I think Rafael or Viresh should take the decision. So it might be better to
directly ping them,

Regards,
Pierre

> 
>>
>> ------
>>
>> Also just for reference, in ACPI 6.5, s8.4.6.1.2.3 Desired Performance Register
>> """
>> When Autonomous Selection is enabled, it is not necessary for OSPM to assess processor workload performance
>> demand and convey a corresponding performance delivery request to the platform via the Desired Register. If the
>> Desired Performance Register exists, OSPM may provide an explicit performance requirement hint to the platform by
>> writing a non-zero value.
>> """
>>
>> So it seems it still makes sense to have cpufreq requesting a certain performance
>> level even though autonomous selection is enabled.
> 
> We did struggle with this. This solves our doubts. Thanks!
> 
>>
> 

Re: [PATCH 3/3] cpufreq: CPPC: Support for autonomous selection in cppc_cpufreq

[zhenglifeng (A)]

Hello Rafael & Viresh

On 2024/12/9 21:15, Pierre Gondois wrote:
> Hello Lifeng,
> 
> On 12/9/24 09:40, zhenglifeng (A) wrote:
>> Hello Pierre,
>>
>> On 2024/12/6 22:23, Pierre Gondois wrote:
>>> Hello Lifeng,
>>>
>>> On 11/14/24 09:48, Lifeng Zheng wrote:
>>>> Add sysfs interfaces for CPPC autonomous selection in the cppc_cpufreq
>>>> driver.
>>>>
>>>> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
>>>> ---
>>>>    .../ABI/testing/sysfs-devices-system-cpu      |  54 +++++++
>>>>    drivers/cpufreq/cppc_cpufreq.c                | 141 ++++++++++++++++++
>>>>    2 files changed, 195 insertions(+)
>>>>
>>>> diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu
>>>> index 206079d3bd5b..ba7b8ea613e5 100644
>>>> --- a/Documentation/ABI/testing/sysfs-devices-system-cpu
>>>> +++ b/Documentation/ABI/testing/sysfs-devices-system-cpu
>>>> @@ -268,6 +268,60 @@ Description:    Discover CPUs in the same CPU frequency coordination domain
>>>>            This file is only present if the acpi-cpufreq or the cppc-cpufreq
>>>>            drivers are in use.
>>>>    +What:        /sys/devices/system/cpu/cpuX/cpufreq/auto_select
>>>> +Date:        October 2024
>>>> +Contact:    linux-pm@vger.kernel.org
>>>> +Description:    Autonomous selection enable
>>>> +
>>>> +        Read/write interface to control autonomous selection enable
>>>> +            Read returns autonomous selection status:
>>>> +                0: autonomous selection is disabled
>>>> +                1: autonomous selection is enabled
>>>> +
>>>> +            Write '1' to enable autonomous selection.
>>>> +            Write '0' to disable autonomous selection.
>>>> +
>>>> +        This file only presents if the cppc-cpufreq driver is in use.
>>>> +
>>>> +What:        /sys/devices/system/cpu/cpuX/cpufreq/auto_act_window
>>>> +Date:        October 2024
>>>> +Contact:    linux-pm@vger.kernel.org
>>>> +Description:    Autonomous activity window
>>>> +
>>>> +        This file indicates a moving utilization sensitivity window to
>>>> +        the platform's autonomous selection policy.
>>>> +
>>>> +        Read/write an integer represents autonomous activity window (in
>>>> +        microseconds) from/to this file. The max value to write is
>>>> +        1270000000 but the max significand is 127. This means that if 128
>>>> +        is written to this file, 127 will be stored. If the value is
>>>> +        greater than 130, only the first two digits will be saved as
>>>> +        significand.
>>>> +
>>>> +        Writing a zero value to this file enable the platform to
>>>> +        determine an appropriate Activity Window depending on the workload.
>>>> +
>>>> +        Writing to this file only has meaning when Autonomous Selection is
>>>> +        enabled.
>>>> +
>>>> +        This file only presents if the cppc-cpufreq driver is in use.
>>>> +
>>>> +What:        /sys/devices/system/cpu/cpuX/cpufreq/energy_perf
>>>> +Date:        October 2024
>>>> +Contact:    linux-pm@vger.kernel.org
>>>> +Description:    Energy performance preference
>>>> +
>>>> +        Read/write an 8-bit integer from/to this file. This file
>>>> +        represents a range of values from 0 (performance preference) to
>>>> +        0xFF (energy efficiency preference) that influences the rate of
>>>> +        performance increase/decrease and the result of the hardware's
>>>> +        energy efficiency and performance optimization policies.
>>>> +
>>>> +        Writing to this file only has meaning when Autonomous Selection is
>>>> +        enabled.
>>>> +
>>>> +        This file only presents if the cppc-cpufreq driver is in use.
>>>> +
>>>>      What:        /sys/devices/system/cpu/cpu*/cache/index3/cache_disable_{0,1}
>>>>    Date:        August 2008
>>>> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
>>>> index 2b8708475ac7..b435e1751d0d 100644
>>>> --- a/drivers/cpufreq/cppc_cpufreq.c
>>>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>>>> @@ -792,10 +792,151 @@ static ssize_t show_freqdomain_cpus(struct cpufreq_policy *policy, char *buf)
>>>>          return cpufreq_show_cpus(cpu_data->shared_cpu_map, buf);
>>>>    }
>>>> +
>>>> +static ssize_t show_auto_select(struct cpufreq_policy *policy, char *buf)
>>>> +{
>>>> +    u64 val;
>>>> +    int ret;
>>>> +
>>>> +    ret = cppc_get_auto_sel(policy->cpu, &val);
>>>> +
>>>> +    /* show "<unsupported>" when this register is not supported by cpc */
>>>> +    if (ret == -EOPNOTSUPP)
>>>> +        return sysfs_emit(buf, "%s\n", "<unsupported>");
>>>> +
>>>> +    if (ret)
>>>> +        return ret;
>>>> +
>>>> +    return sysfs_emit(buf, "%lld\n", val);
>>>> +}
>>>> +
>>>> +static ssize_t store_auto_select(struct cpufreq_policy *policy,
>>>> +                 const char *buf, size_t count)
>>>> +{
>>>> +    unsigned long val;
>>>> +    int ret;
>>>> +
>>>> +    ret = kstrtoul(buf, 0, &val);
>>>> +    if (ret)
>>>> +        return ret;
>>>> +
>>>> +    if (val > 1)
>>>> +        return -EINVAL;
>>>> +
>>>> +    ret = cppc_set_auto_sel(policy->cpu, val);
>>>> +    if (ret)
>>>> +        return ret;
>>>> +
>>>> +    return count;
>>>> +}
>>>> +
>>>> +#define AUTO_ACT_WINDOW_SIG_BIT_SIZE    (7)
>>>> +#define AUTO_ACT_WINDOW_EXP_BIT_SIZE    (3)
>>>> +#define AUTO_ACT_WINDOW_MAX_SIG    ((1 << AUTO_ACT_WINDOW_SIG_BIT_SIZE) - 1)
>>>> +#define AUTO_ACT_WINDOW_MAX_EXP    ((1 << AUTO_ACT_WINDOW_EXP_BIT_SIZE) - 1)
>>>> +/* AUTO_ACT_WINDOW_MAX_SIG is 127, so 128 and 129 will decay to 127 when writing */
>>>> +#define AUTO_ACT_WINDOW_SIG_CARRY_THRESH 129
>>>
>>> Maybe this would be better to place these macros in include/acpi/cppc_acpi.h
>>> (with a CPPC_XXX prefix)
>>
>> Will move them, Thanks.
>>
>>>
>>>> +
>>>> +static ssize_t show_auto_act_window(struct cpufreq_policy *policy, char *buf)
>>>> +{
>>>> +    int sig, exp;
>>>> +    u64 val;
>>>> +    int ret;
>>>> +
>>>> +    ret = cppc_get_auto_act_window(policy->cpu, &val);
>>>> +
>>>> +    /* show "<unsupported>" when this register is not supported by cpc */
>>>> +    if (ret == -EOPNOTSUPP)
>>>> +        return sysfs_emit(buf, "%s\n", "<unsupported>");
>>>> +
>>>> +    if (ret)
>>>> +        return ret;
>>>> +
>>>> +    sig = val & AUTO_ACT_WINDOW_MAX_SIG;
>>>> +    exp = (val >> AUTO_ACT_WINDOW_SIG_BIT_SIZE) & AUTO_ACT_WINDOW_MAX_EXP;
>>>> +
>>>> +    return sysfs_emit(buf, "%lld\n", sig * int_pow(10, exp));
>>>> +}
>>>> +
>>>> +static ssize_t store_auto_act_window(struct cpufreq_policy *policy,
>>>> +                     const char *buf, size_t count)
>>>> +{
>>>> +    unsigned long usec;
>>>> +    int digits = 0;
>>>> +    int ret;
>>>> +
>>>> +    ret = kstrtoul(buf, 0, &usec);
>>>> +    if (ret)
>>>> +        return ret;
>>>> +
>>>> +    if (usec > AUTO_ACT_WINDOW_MAX_SIG * int_pow(10, AUTO_ACT_WINDOW_MAX_EXP))
>>>> +        return -EINVAL;
>>>> +
>>>> +    while (usec > AUTO_ACT_WINDOW_SIG_CARRY_THRESH) {
>>>> +        usec /= 10;
>>>> +        digits += 1;
>>>> +    }
>>>> +
>>>> +    if (usec > AUTO_ACT_WINDOW_MAX_SIG)
>>>> +        usec = AUTO_ACT_WINDOW_MAX_SIG;
>>>> +
>>>> +    ret = cppc_set_auto_act_window(policy->cpu,
>>>> +                       (digits << AUTO_ACT_WINDOW_SIG_BIT_SIZE) + usec);
>>>> +    if (ret)
>>>> +        return ret;
>>>> +
>>>> +    return count;
>>>> +}
> 
> Also I tested the logic and it was working correctly for me.
> 
>>>> +
>>>> +static ssize_t show_energy_perf(struct cpufreq_policy *policy, char *buf)
>>>> +{
>>>> +    u64 val;
>>>> +    int ret;
>>>> +
>>>> +    ret = cppc_get_epp_perf(policy->cpu, &val);
>>>> +
>>>> +    /* show "<unsupported>" when this register is not supported by cpc */
>>>> +    if (ret == -EOPNOTSUPP)
>>>> +        return sysfs_emit(buf, "%s\n", "<unsupported>");
>>>> +
>>>> +    if (ret)
>>>> +        return ret;
>>>> +
>>>> +    return sysfs_emit(buf, "%lld\n", val);
>>>> +}
>>>> +
>>>> +#define ENERGY_PERF_MAX    (0xFF)
>>>
>>> Same comment to move to include/acpi/cppc_acpi.h
>>>
>>>> +
>>>> +static ssize_t store_energy_perf(struct cpufreq_policy *policy,
>>>> +                 const char *buf, size_t count)
>>>> +{
>>>> +    unsigned long val;
>>>> +    int ret;
>>>> +
>>>> +    ret = kstrtoul(buf, 0, &val);
>>>> +    if (ret)
>>>> +        return ret;
>>>> +
>>>> +    if (val > ENERGY_PERF_MAX)
>>>> +        return -EINVAL;
>>>> +
>>>> +    ret = cppc_set_epp(policy->cpu, val);
>>>> +    if (ret)
>>>> +        return ret;
>>>> +
>>>> +    return count;
>>>> +}
>>>> +
>>>>    cpufreq_freq_attr_ro(freqdomain_cpus);
>>>> +cpufreq_freq_attr_rw(auto_select);
>>>> +cpufreq_freq_attr_rw(auto_act_window);
>>>> +cpufreq_freq_attr_rw(energy_perf);
>>>
>>> It might be better from a user PoV to hide the following entries:
>>> - auto_act_window
>>> - energy_perf
>>> if auto_select is not available or disabled.
>>
>> Users might like to modify the value of auto_act_window and energy_perf
>> before turning on auto_select. So I think it is freer for users to read and
>> write them no matter what auto_select is. What do you think?
> 
> Autonomous selection is not the most common case for the CPPC cpufreq drivers,
> so these new files might bring questions to people currently using it.
> 
> On the other side, making these files visible only when 'auto_select' is enabled
> will require additional logic in the code (while the current implementation is
> quite clear).
> 
> I think Rafael or Viresh should take the decision. So it might be better to
> directly ping them,
> 
> Regards,
> Pierre

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 your advice and look forward to your reply!

Regards,
Lifeng

> 
>>
>>>
>>> ------
>>>
>>> Also just for reference, in ACPI 6.5, s8.4.6.1.2.3 Desired Performance Register
>>> """
>>> When Autonomous Selection is enabled, it is not necessary for OSPM to assess processor workload performance
>>> demand and convey a corresponding performance delivery request to the platform via the Desired Register. If the
>>> Desired Performance Register exists, OSPM may provide an explicit performance requirement hint to the platform by
>>> writing a non-zero value.
>>> """
>>>
>>> So it seems it still makes sense to have cpufreq requesting a certain performance
>>> level even though autonomous selection is enabled.
>>
>> We did struggle with this. This solves our doubts. Thanks!
>>
>>>
>>
>