On Mon, Aug 21, 2023 at 3:24 PM Sumit Gupta <sumitg@xxxxxxxxxx> wrote: > > > > 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. So you should add this information to the patch changelog, ideally along with the description of the hardware configuration in which the improvement has been observed. > 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. Yes, that's how it works. What I'm saying is that the way it works does not appear to be particularly useful. For example, how exactly is a distribution supposed to guess the "right" value for their general-purpose kernel?