On 19/08/23 00:10, Rafael J. Wysocki wrote:
External email: Use caution opening links or attachments On Thu, Aug 17, 2023 at 11:31 AM Sumit Gupta <sumitg@xxxxxxxxxx> wrote:From: Srikar Srimath Tirumala <srikars@xxxxxxxxxx> Add support to configure the CPUFREQ reduction percentage and set the maximum number of throttling steps accordingly. Current implementation of processor_thermal performs software throttling in fixed steps of "20%" which can be too coarse for some platforms. Change that by adding new config to provide the reduction percentage. Signed-off-by: Srikar Srimath Tirumala <srikars@xxxxxxxxxx> Signed-off-by: Sumit Gupta <sumitg@xxxxxxxxxx> --- drivers/acpi/Kconfig | 15 +++++++++++++++ drivers/acpi/processor_thermal.c | 19 ++++++++++++++++--- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig index 00dd309b6682..287cf58defbf 100644 --- a/drivers/acpi/Kconfig +++ b/drivers/acpi/Kconfig @@ -254,6 +254,21 @@ config ACPI_DOCK config ACPI_CPU_FREQ_PSS bool +config ACPI_CPU_FREQ_THERM_HAS_PARAMS + bool "CPU frequency throttling control" + depends on ACPI_PROCESSOR + +config ACPI_CPU_FREQ_THERM_MIN_THROT_PCTG + int "Minimum throttle percentage for processor_thermal cooling device" + depends on ACPI_CPU_FREQ_THERM_HAS_PARAMS + default 20 + help + The processor_thermal driver uses this config to calculate the + percentage amount by which cpu frequency must be reduced for each + cooling state. The config is also used to calculate the maximum number + of throttling steps or cooling states supported by the driver. Value + must be an unsigned integer in the range [1, 50]. +I don't think that the new Kconfig symbols are particularly useful. At least they don't help the distro vendors that each would need to pick up a specific value for their kernel anyway. I also wonder how the users building their own kernels are supposed to determine the values suitable for the target systems.
We observed some perf gain after reducing the throttle percentage. Currently, kept the default to '20%' as before. Based on need, a vendor can overwrite the default value with macro 'CONFIG_ACPI_CPU_FREQ_THERM_MIN_THROT_PCTG'. Otherwise, the behavior will remain same. Thank you, Sumit Gupta
config ACPI_PROCESSOR_CSTATE def_bool y depends on ACPI_PROCESSOR diff --git a/drivers/acpi/processor_thermal.c b/drivers/acpi/processor_thermal.c index b7c6287eccca..ee443cc69b73 100644 --- a/drivers/acpi/processor_thermal.c +++ b/drivers/acpi/processor_thermal.c @@ -25,8 +25,20 @@ * _any_ cpufreq driver and not only the acpi-cpufreq driver. */ -#define CPUFREQ_THERMAL_MIN_STEP 0 -#define CPUFREQ_THERMAL_MAX_STEP 3 +#define CPUFREQ_THERMAL_MIN_STEP 0 +#ifdef CONFIG_ACPI_CPU_FREQ_THERM_HAS_PARAMS +#define CPUFREQ_THERMAL_PCTG CONFIG_ACPI_CPU_FREQ_THERM_MIN_THROT_PCTG + +/* Derive the MAX_STEP from minimum throttle percentage so that the reduction + * percentage does end up becoming negative. Also cap the MAX_STEP so that + * the CPU performance doesn't become 0. + */ +#define CPUFREQ_THERMAL_MAX_STEP ((100 / CPUFREQ_THERMAL_PCTG) - 1) + +#else +#define CPUFREQ_THERMAL_MAX_STEP 3 +#define CPUFREQ_THERMAL_PCTG 20 +#endif static DEFINE_PER_CPU(unsigned int, cpufreq_thermal_reduction_pctg); @@ -113,7 +125,8 @@ static int cpufreq_set_cur_state(unsigned int cpu, int state) if (!policy) return -EINVAL; - max_freq = (policy->cpuinfo.max_freq * (100 - reduction_pctg(i) * 20)) / 100; + max_freq = (policy->cpuinfo.max_freq * + (100 - reduction_pctg(i) * CPUFREQ_THERMAL_PCTG)) / 100; cpufreq_cpu_put(policy); --
