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@xxxxxxxxxx>
>> ---
>> .../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@xxxxxxxxxxxxxxx
>> +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@xxxxxxxxxxxxxxx
>> +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@xxxxxxxxxxxxxxx
>> +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!
>
