From: Srikar Srimath Tirumala <[email protected]>
Current implementation of processor_thermal performs software throttling
in fixed steps of "20%" which can be too coarse for some platforms.
We observed some performance gain after reducing the throttle percentage.
Change the CPUFREQ thermal reduction percentage and maximum thermal steps
to be configurable. Also, update the default values of both for Nvidia
Tegra241 (Grace) SoC. The thermal reduction percentage is reduced to "5%"
and accordingly the maximum number of thermal steps are increased as they
are derived from the reduction percentage.
Signed-off-by: Srikar Srimath Tirumala <[email protected]>
Co-developed-by: Sumit Gupta <[email protected]>
Signed-off-by: Sumit Gupta <[email protected]>
---
Sending this patch separately as the other patch in the series is
applied by Rafael in v6[1]. Revision history before this version is
in the cover letter of v6[1].
Please review and provide ACK if looks fine.
v6[6] -> 7:
- Patch 2: remove CONFIG_HAVE_ARM_SMCCC_DISCOVERY ifdef check [Sudeep].
: replace SMCC macro with CONFIG_ARM64 in internal.h [Sudeep].
[1] https://lore.kernel.org/lkml/[email protected]/
drivers/acpi/arm64/Makefile | 1 +
drivers/acpi/arm64/thermal_cpufreq.c | 20 ++++++++++++
drivers/acpi/internal.h | 9 +++++
drivers/acpi/processor_thermal.c | 49 +++++++++++++++++++++++-----
4 files changed, 70 insertions(+), 9 deletions(-)
create mode 100644 drivers/acpi/arm64/thermal_cpufreq.c
diff --git a/drivers/acpi/arm64/Makefile b/drivers/acpi/arm64/Makefile
index 143debc1ba4a..726944648c9b 100644
--- a/drivers/acpi/arm64/Makefile
+++ b/drivers/acpi/arm64/Makefile
@@ -5,3 +5,4 @@ obj-$(CONFIG_ACPI_GTDT) += gtdt.o
obj-$(CONFIG_ACPI_APMT) += apmt.o
obj-$(CONFIG_ARM_AMBA) += amba.o
obj-y += dma.o init.o
+obj-y += thermal_cpufreq.o
diff --git a/drivers/acpi/arm64/thermal_cpufreq.c b/drivers/acpi/arm64/thermal_cpufreq.c
new file mode 100644
index 000000000000..d524f2cd6044
--- /dev/null
+++ b/drivers/acpi/arm64/thermal_cpufreq.c
@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/acpi.h>
+
+#include "../internal.h"
+
+#define SMCCC_SOC_ID_T241 0x036b0241
+
+int acpi_arch_thermal_cpufreq_pctg(void)
+{
+ s32 soc_id = arm_smccc_get_soc_id_version();
+
+ /*
+ * Check JEP106 code for NVIDIA Tegra241 chip (036b:0241) and
+ * reduce the CPUFREQ Thermal reduction percentage to 5%.
+ */
+ if (soc_id == SMCCC_SOC_ID_T241)
+ return 5;
+
+ return 0;
+}
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 8cd2d15dec11..bcef1ce1aed9 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -90,6 +90,15 @@ int acpi_passive_trip_temp(struct acpi_device *adev, int *ret_temp);
int acpi_hot_trip_temp(struct acpi_device *adev, int *ret_temp);
int acpi_critical_trip_temp(struct acpi_device *adev, int *ret_temp);
+#ifdef CONFIG_ARM64
+int acpi_arch_thermal_cpufreq_pctg(void);
+#else
+static inline int acpi_arch_thermal_cpufreq_pctg(void)
+{
+ return 0;
+}
+#endif
+
/* --------------------------------------------------------------------------
Device Node Initialization / Removal
-------------------------------------------------------------------------- */
diff --git a/drivers/acpi/processor_thermal.c b/drivers/acpi/processor_thermal.c
index b7c6287eccca..1219adb11ab9 100644
--- a/drivers/acpi/processor_thermal.c
+++ b/drivers/acpi/processor_thermal.c
@@ -17,6 +17,8 @@
#include <acpi/processor.h>
#include <linux/uaccess.h>
+#include "internal.h"
+
#ifdef CONFIG_CPU_FREQ
/* If a passive cooling situation is detected, primarily CPUfreq is used, as it
@@ -26,12 +28,21 @@
*/
#define CPUFREQ_THERMAL_MIN_STEP 0
-#define CPUFREQ_THERMAL_MAX_STEP 3
-static DEFINE_PER_CPU(unsigned int, cpufreq_thermal_reduction_pctg);
+static int cpufreq_thermal_max_step __read_mostly = 3;
+
+/*
+ * Minimum throttle percentage for processor_thermal cooling device.
+ * The processor_thermal driver uses it to calculate the percentage amount by
+ * which cpu frequency must be reduced for each cooling state. This is also used
+ * to calculate the maximum number of throttling steps or cooling states.
+ */
+static int cpufreq_thermal_reduction_pctg __read_mostly = 20;
-#define reduction_pctg(cpu) \
- per_cpu(cpufreq_thermal_reduction_pctg, phys_package_first_cpu(cpu))
+static DEFINE_PER_CPU(unsigned int, cpufreq_thermal_reduction_step);
+
+#define reduction_step(cpu) \
+ per_cpu(cpufreq_thermal_reduction_step, phys_package_first_cpu(cpu))
/*
* Emulate "per package data" using per cpu data (which should really be
@@ -71,7 +82,7 @@ static int cpufreq_get_max_state(unsigned int cpu)
if (!cpu_has_cpufreq(cpu))
return 0;
- return CPUFREQ_THERMAL_MAX_STEP;
+ return cpufreq_thermal_max_step;
}
static int cpufreq_get_cur_state(unsigned int cpu)
@@ -79,7 +90,7 @@ static int cpufreq_get_cur_state(unsigned int cpu)
if (!cpu_has_cpufreq(cpu))
return 0;
- return reduction_pctg(cpu);
+ return reduction_step(cpu);
}
static int cpufreq_set_cur_state(unsigned int cpu, int state)
@@ -92,7 +103,7 @@ static int cpufreq_set_cur_state(unsigned int cpu, int state)
if (!cpu_has_cpufreq(cpu))
return 0;
- reduction_pctg(cpu) = state;
+ reduction_step(cpu) = state;
/*
* Update all the CPUs in the same package because they all
@@ -113,7 +124,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_step(i) * cpufreq_thermal_reduction_pctg)) / 100;
cpufreq_cpu_put(policy);
@@ -126,10 +138,29 @@ static int cpufreq_set_cur_state(unsigned int cpu, int state)
return 0;
}
+static void acpi_thermal_cpufreq_config(void)
+{
+ int cpufreq_pctg = acpi_arch_thermal_cpufreq_pctg();
+
+ if (!cpufreq_pctg)
+ return;
+
+ cpufreq_thermal_reduction_pctg = cpufreq_pctg;
+
+ /*
+ * Derive the MAX_STEP from minimum throttle percentage so that the reduction
+ * percentage doesn't end up becoming negative. Also, cap the MAX_STEP so that
+ * the CPU performance doesn't become 0.
+ */
+ cpufreq_thermal_max_step = (100 / cpufreq_pctg) - 2;
+}
+
void acpi_thermal_cpufreq_init(struct cpufreq_policy *policy)
{
unsigned int cpu;
+ acpi_thermal_cpufreq_config();
+
for_each_cpu(cpu, policy->related_cpus) {
struct acpi_processor *pr = per_cpu(processors, cpu);
int ret;
@@ -190,7 +221,7 @@ static int acpi_processor_max_state(struct acpi_processor *pr)
/*
* There exists four states according to
- * cpufreq_thermal_reduction_pctg. 0, 1, 2, 3
+ * cpufreq_thermal_reduction_step. 0, 1, 2, 3
*/
max_state += cpufreq_get_max_state(pr->id);
if (pr->flags.throttling)
--
2.17.1
On Thu, Nov 23, 2023 at 05:44:33PM +0530, Sumit Gupta wrote:
> From: Srikar Srimath Tirumala <[email protected]>
>
> Current implementation of processor_thermal performs software throttling
> in fixed steps of "20%" which can be too coarse for some platforms.
> We observed some performance gain after reducing the throttle percentage.
> Change the CPUFREQ thermal reduction percentage and maximum thermal steps
> to be configurable. Also, update the default values of both for Nvidia
> Tegra241 (Grace) SoC. The thermal reduction percentage is reduced to "5%"
> and accordingly the maximum number of thermal steps are increased as they
> are derived from the reduction percentage.
>
> Signed-off-by: Srikar Srimath Tirumala <[email protected]>
> Co-developed-by: Sumit Gupta <[email protected]>
> Signed-off-by: Sumit Gupta <[email protected]>
> ---
>
> Sending this patch separately as the other patch in the series is
> applied by Rafael in v6[1]. Revision history before this version is
> in the cover letter of v6[1].
>
> Please review and provide ACK if looks fine.
>
For arm64 specific changes(a minor nit below though),
Acked-by: Sudeep Holla <[email protected]>
[...]
> diff --git a/drivers/acpi/arm64/thermal_cpufreq.c b/drivers/acpi/arm64/thermal_cpufreq.c
> new file mode 100644
> index 000000000000..d524f2cd6044
> --- /dev/null
> +++ b/drivers/acpi/arm64/thermal_cpufreq.c
> @@ -0,0 +1,20 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include <linux/acpi.h>
> +
> +#include "../internal.h"
> +
> +#define SMCCC_SOC_ID_T241 0x036b0241
> +
[nit] We really need to find better place to define this globally and not
locally at each usage site like this. We already have it in GICv3 driver.
But that can come as a cleanup later if it causes issue for merging this
change.
--
Regards,
Sudeep
On 23/11/23 19:30, Sudeep Holla wrote:
> External email: Use caution opening links or attachments
>
>
> On Thu, Nov 23, 2023 at 05:44:33PM +0530, Sumit Gupta wrote:
>> From: Srikar Srimath Tirumala <[email protected]>
>>
>> Current implementation of processor_thermal performs software throttling
>> in fixed steps of "20%" which can be too coarse for some platforms.
>> We observed some performance gain after reducing the throttle percentage.
>> Change the CPUFREQ thermal reduction percentage and maximum thermal steps
>> to be configurable. Also, update the default values of both for Nvidia
>> Tegra241 (Grace) SoC. The thermal reduction percentage is reduced to "5%"
>> and accordingly the maximum number of thermal steps are increased as they
>> are derived from the reduction percentage.
>>
>> Signed-off-by: Srikar Srimath Tirumala <[email protected]>
>> Co-developed-by: Sumit Gupta <[email protected]>
>> Signed-off-by: Sumit Gupta <[email protected]>
>> ---
>>
>> Sending this patch separately as the other patch in the series is
>> applied by Rafael in v6[1]. Revision history before this version is
>> in the cover letter of v6[1].
>>
>> Please review and provide ACK if looks fine.
>>
>
> For arm64 specific changes(a minor nit below though),
>
> Acked-by: Sudeep Holla <[email protected]>
>
>
> [...]
>
Thank you.
>> diff --git a/drivers/acpi/arm64/thermal_cpufreq.c b/drivers/acpi/arm64/thermal_cpufreq.c
>> new file mode 100644
>> index 000000000000..d524f2cd6044
>> --- /dev/null
>> +++ b/drivers/acpi/arm64/thermal_cpufreq.c
>> @@ -0,0 +1,20 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +#include <linux/acpi.h>
>> +
>> +#include "../internal.h"
>> +
>> +#define SMCCC_SOC_ID_T241 0x036b0241
>> +
>
> [nit] We really need to find better place to define this globally and not
> locally at each usage site like this. We already have it in GICv3 driver.
> But that can come as a cleanup later if it causes issue for merging this
> change.
>
> --
> Regards,
> Sudeep
Sure, will check and send a separate patch later on top.
Best Regards,
Sumit Gupta
On 2023/11/23 22:00, Sudeep Holla wrote:
> On Thu, Nov 23, 2023 at 05:44:33PM +0530, Sumit Gupta wrote:
>> From: Srikar Srimath Tirumala <[email protected]>
>>
>> Current implementation of processor_thermal performs software throttling
>> in fixed steps of "20%" which can be too coarse for some platforms.
>> We observed some performance gain after reducing the throttle percentage.
>> Change the CPUFREQ thermal reduction percentage and maximum thermal steps
>> to be configurable. Also, update the default values of both for Nvidia
>> Tegra241 (Grace) SoC. The thermal reduction percentage is reduced to "5%"
>> and accordingly the maximum number of thermal steps are increased as they
>> are derived from the reduction percentage.
>>
>> Signed-off-by: Srikar Srimath Tirumala <[email protected]>
>> Co-developed-by: Sumit Gupta <[email protected]>
>> Signed-off-by: Sumit Gupta <[email protected]>
>> ---
>>
>> Sending this patch separately as the other patch in the series is
>> applied by Rafael in v6[1]. Revision history before this version is
>> in the cover letter of v6[1].
>>
>> Please review and provide ACK if looks fine.
>>
>
> For arm64 specific changes(a minor nit below though),
>
> Acked-by: Sudeep Holla <[email protected]>
>
>
> [...]
>
>> diff --git a/drivers/acpi/arm64/thermal_cpufreq.c b/drivers/acpi/arm64/thermal_cpufreq.c
>> new file mode 100644
>> index 000000000000..d524f2cd6044
>> --- /dev/null
>> +++ b/drivers/acpi/arm64/thermal_cpufreq.c
>> @@ -0,0 +1,20 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +#include <linux/acpi.h>
>> +
>> +#include "../internal.h"
>> +
>> +#define SMCCC_SOC_ID_T241 0x036b0241
>> +
>
> [nit] We really need to find better place to define this globally and not
> locally at each usage site like this. We already have it in GICv3 driver.
> But that can come as a cleanup later if it causes issue for merging this
> change.
Agreed.
Looks good to me as well.
Acked-by: Hanjun Guo <[email protected]>
Thanks
Hanjun
>>> From: Srikar Srimath Tirumala <[email protected]>
>>>
>>> Current implementation of processor_thermal performs software throttling
>>> in fixed steps of "20%" which can be too coarse for some platforms.
>>> We observed some performance gain after reducing the throttle
>>> percentage.
>>> Change the CPUFREQ thermal reduction percentage and maximum thermal
>>> steps
>>> to be configurable. Also, update the default values of both for Nvidia
>>> Tegra241 (Grace) SoC. The thermal reduction percentage is reduced to
>>> "5%"
>>> and accordingly the maximum number of thermal steps are increased as
>>> they
>>> are derived from the reduction percentage.
>>>
>>> Signed-off-by: Srikar Srimath Tirumala <[email protected]>
>>> Co-developed-by: Sumit Gupta <[email protected]>
>>> Signed-off-by: Sumit Gupta <[email protected]>
>>> ---
>>>
>>> Sending this patch separately as the other patch in the series is
>>> applied by Rafael in v6[1]. Revision history before this version is
>>> in the cover letter of v6[1].
>>>
>>> Please review and provide ACK if looks fine.
>>>
>>
>> For arm64 specific changes(a minor nit below though),
>>
>> Acked-by: Sudeep Holla <[email protected]>
>>
>>
>> [...]
>>
>>> diff --git a/drivers/acpi/arm64/thermal_cpufreq.c
>>> b/drivers/acpi/arm64/thermal_cpufreq.c
>>> new file mode 100644
>>> index 000000000000..d524f2cd6044
>>> --- /dev/null
>>> +++ b/drivers/acpi/arm64/thermal_cpufreq.c
>>> @@ -0,0 +1,20 @@
>>> +// SPDX-License-Identifier: GPL-2.0-only
>>> +#include <linux/acpi.h>
>>> +
>>> +#include "../internal.h"
>>> +
>>> +#define SMCCC_SOC_ID_T241 0x036b0241
>>> +
>>
>> [nit] We really need to find better place to define this globally and not
>> locally at each usage site like this. We already have it in GICv3 driver.
>> But that can come as a cleanup later if it causes issue for merging this
>> change.
>
> Agreed.
>
> Looks good to me as well.
>
> Acked-by: Hanjun Guo <[email protected]>
>
Hi Rafael,
If the change seems fine, could you please apply this patch?
Thanks,
Sumit
On Thu, Nov 23, 2023 at 3:32 PM Hanjun Guo <[email protected]> wrote:
>
> On 2023/11/23 22:00, Sudeep Holla wrote:
> > On Thu, Nov 23, 2023 at 05:44:33PM +0530, Sumit Gupta wrote:
> >> From: Srikar Srimath Tirumala <[email protected]>
> >>
> >> Current implementation of processor_thermal performs software throttling
> >> in fixed steps of "20%" which can be too coarse for some platforms.
> >> We observed some performance gain after reducing the throttle percentage.
> >> Change the CPUFREQ thermal reduction percentage and maximum thermal steps
> >> to be configurable. Also, update the default values of both for Nvidia
> >> Tegra241 (Grace) SoC. The thermal reduction percentage is reduced to "5%"
> >> and accordingly the maximum number of thermal steps are increased as they
> >> are derived from the reduction percentage.
> >>
> >> Signed-off-by: Srikar Srimath Tirumala <[email protected]>
> >> Co-developed-by: Sumit Gupta <[email protected]>
> >> Signed-off-by: Sumit Gupta <[email protected]>
> >> ---
> >>
> >> Sending this patch separately as the other patch in the series is
> >> applied by Rafael in v6[1]. Revision history before this version is
> >> in the cover letter of v6[1].
> >>
> >> Please review and provide ACK if looks fine.
> >>
> >
> > For arm64 specific changes(a minor nit below though),
> >
> > Acked-by: Sudeep Holla <[email protected]>
Applied as 6.8 material, thanks!