[Patch v6 0/2] Add support for _TFP and change throttle pctg

[Patch v6 0/2] Add support for _TFP and change throttle pctg

[Sumit Gupta]

This patch set adds two improvements to get a finer control over the
impact of thermal throttling on performance. Requesting to merge the
patches if no further changes needed.

 1) Patch 1: Adds support to read "Thermal fast Sampling Period (_TFP)"
    ACPI object and use it over "Thermal Sampling Period (_TSP)" for
    Passive cooling if both are present.

 2) Patch 2: Adds support to reduce the CPUFREQ reduction percentage
    and not always cause throttling in steps of "20%" for Tegra241 SoC.

Both patches can be applied independently.

---
v5[5] -> 6:
- Patch 1: rename passive_delay variable to delay [Rafael]
         : re-structured the if check when reading _TFP [Rafael]
- Patch 2: compile thermal_cpufreq.c by default [Sudeep]
         : rename arch hook to acpi_arch_thermal_cpufreq_pctg() [Sudeep]
         : move null functions from acpi.h to internal.h [Rafael]
         : rename existing reduction_pctg to reduction_step
         : rename cpufreq_thermal_pctg to cpufreq_thermal_reduction_pctg
         : update formula to get same num of max steps as old with 20%

v4[4] -> 5:
- Patch 2: fix kernel robot warning for acpi_thermal_cpufreq_pctg().

v3[3] -> 4:
- Patch 2: move ARM code from generic to new file 'thermal_cpufreq.c'.
         : get 'cpufreq_thermal_pctg' value for Tegra241 from new file.
         : move dummy/null function to 'acpi.h'.

v2[2] -> v3:
- Patch1: rebased on top of linux-next.
- Patch2: use __read_mostly for the cpufreq_thermal_* variables.
        : add static to new function acpi_thermal_cpufreq_config_nvidia.
        : add null function if CONFIG_HAVE_ARM_SMCCC_DISCOVERY undefined
        : removed redundant parenthesis.

v1[1] -> v2:
- Patch1: add ACPI spec section info in commit description and rebased.
- Patch2: add info about hardware in the commit description.
        : switched CPUFREQ THERMAL tuning macros to static variables.
        : update the tunings for Tegra241 SoC only using soc_id check.

Jeff Brasen (1):
  ACPI: thermal: Add Thermal fast Sampling Period (_TFP) support

Srikar Srimath Tirumala (1):
  ACPI: processor: reduce CPUFREQ thermal reduction pctg for Tegra241

 drivers/acpi/arm64/Makefile          |  1 +
 drivers/acpi/arm64/thermal_cpufreq.c | 22 +++++++++++++
 drivers/acpi/internal.h              |  9 +++++
 drivers/acpi/processor_thermal.c     | 49 +++++++++++++++++++++++-----
 drivers/acpi/thermal.c               | 12 +++++--
 5 files changed, 81 insertions(+), 12 deletions(-)
 create mode 100644 drivers/acpi/arm64/thermal_cpufreq.c

[5] https://lore.kernel.org/all/20231014105426.26389-1-sumitg@nvidia.com/
[4] https://lore.kernel.org/lkml/20231009171839.12267-1-sumitg@nvidia.com/
[3] https://lore.kernel.org/linux-acpi/20231006153612.5851-1-sumitg@nvidia.com/
[2] https://lore.kernel.org/lkml/20230913164659.9345-1-sumitg@nvidia.com/
[1] https://lore.kernel.org/lkml/20230817093011.1378-1-sumitg@nvidia.com/

-- 
2.17.1

[Patch v6 1/2] ACPI: thermal: Add Thermal fast Sampling Period (_TFP) support

[Sumit Gupta]

From: Jeff Brasen <jbrasen@nvidia.com>

Add support of "Thermal fast Sampling Period (_TFP)" for Passive cooling.
As per [1], _TFP overrides the "Thermal Sampling Period (_TSP)" if both
are present in a Thermal zone.

[1] ACPI Specification 6.4 - section 11.4.17. _TFP (Thermal fast Sampling
    Period)"

Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
Co-developed-by: Sumit Gupta <sumitg@nvidia.com>
Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
---
 drivers/acpi/thermal.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index f74d81abdbfc..3b75eb2260d7 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -90,7 +90,7 @@ struct acpi_thermal_passive {
 	struct acpi_thermal_trip trip;
 	unsigned long tc1;
 	unsigned long tc2;
-	unsigned long tsp;
+	unsigned long delay;
 };
 
 struct acpi_thermal_active {
@@ -404,11 +404,17 @@ static bool passive_trip_params_init(struct acpi_thermal *tz)
 
 	tz->trips.passive.tc2 = tmp;
 
+	status = acpi_evaluate_integer(tz->device->handle, "_TFP", NULL, &tmp);
+	if (ACPI_SUCCESS(status)) {
+		tz->trips.passive.delay = tmp;
+		return true;
+	}
+
 	status = acpi_evaluate_integer(tz->device->handle, "_TSP", NULL, &tmp);
 	if (ACPI_FAILURE(status))
 		return false;
 
-	tz->trips.passive.tsp = tmp;
+	tz->trips.passive.delay = tmp * 100;
 
 	return true;
 }
@@ -904,7 +910,7 @@ static int acpi_thermal_add(struct acpi_device *device)
 
 	acpi_trip = &tz->trips.passive.trip;
 	if (acpi_thermal_trip_valid(acpi_trip)) {
-		passive_delay = tz->trips.passive.tsp * 100;
+		passive_delay = tz->trips.passive.delay;
 
 		trip->type = THERMAL_TRIP_PASSIVE;
 		trip->temperature = acpi_thermal_temp(tz, acpi_trip->temp_dk);
-- 
2.17.1

[Patch v6 2/2] ACPI: processor: reduce CPUFREQ thermal reduction pctg for Tegra241

[Sumit Gupta]

From: Srikar Srimath Tirumala <srikars@nvidia.com>

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 <srikars@nvidia.com>
Co-developed-by: Sumit Gupta <sumitg@nvidia.com>
Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
---
 drivers/acpi/arm64/Makefile          |  1 +
 drivers/acpi/arm64/thermal_cpufreq.c | 22 +++++++++++++
 drivers/acpi/internal.h              |  9 +++++
 drivers/acpi/processor_thermal.c     | 49 +++++++++++++++++++++++-----
 4 files changed, 72 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..40d5806ed528
--- /dev/null
+++ b/drivers/acpi/arm64/thermal_cpufreq.c
@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/acpi.h>
+
+#include "../internal.h"
+
+#ifdef CONFIG_HAVE_ARM_SMCCC_DISCOVERY
+#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;
+}
+#endif
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 866c7c4ed233..ee213a8cddc5 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -85,6 +85,15 @@ bool acpi_scan_is_offline(struct acpi_device *adev, bool uevent);
 acpi_status acpi_sysfs_table_handler(u32 event, void *table, void *context);
 void acpi_scan_table_notify(void);
 
+#ifdef CONFIG_HAVE_ARM_SMCCC_DISCOVERY
+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

Re: [Patch v6 2/2] ACPI: processor: reduce CPUFREQ thermal reduction pctg for Tegra241

[Sudeep Holla]

On Fri, Nov 10, 2023 at 12:03:22AM +0530, Sumit Gupta wrote:
> From: Srikar Srimath Tirumala <srikars@nvidia.com>
> 
> 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 <srikars@nvidia.com>
> Co-developed-by: Sumit Gupta <sumitg@nvidia.com>
> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
> ---
>  drivers/acpi/arm64/Makefile          |  1 +
>  drivers/acpi/arm64/thermal_cpufreq.c | 22 +++++++++++++
>  drivers/acpi/internal.h              |  9 +++++
>  drivers/acpi/processor_thermal.c     | 49 +++++++++++++++++++++++-----
>  4 files changed, 72 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..40d5806ed528
> --- /dev/null
> +++ b/drivers/acpi/arm64/thermal_cpufreq.c
> @@ -0,0 +1,22 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include <linux/acpi.h>
> +
> +#include "../internal.h"
> +
> +#ifdef CONFIG_HAVE_ARM_SMCCC_DISCOVERY
> +#define SMCCC_SOC_ID_T241      0x036b0241

Sorry for missing this earlier. Not sure if the above define needs to be
conditional. Even if it has to be, CONFIG_ARM_SMCCC_SOC_ID is more
appropriate.

> +
> +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;
> +}
> +#endif
> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
> index 866c7c4ed233..ee213a8cddc5 100644
> --- a/drivers/acpi/internal.h
> +++ b/drivers/acpi/internal.h
> @@ -85,6 +85,15 @@ bool acpi_scan_is_offline(struct acpi_device *adev, bool uevent);
>  acpi_status acpi_sysfs_table_handler(u32 event, void *table, void *context);
>  void acpi_scan_table_notify(void);
>  
> +#ifdef CONFIG_HAVE_ARM_SMCCC_DISCOVERY

It looks weird to add a such specific ARM config option in generic ACPI
code/header.

Does it make sense to add some new config this new feature you are adding
or just use ARM64 and have CONFIG_HAVE_ARM_SMCCC_DISCOVERY check internally
in the arch specific call.

-- 
Regards,
Sudeep

Re: [Patch v6 2/2] ACPI: processor: reduce CPUFREQ thermal reduction pctg for Tegra241

[Sumit Gupta]

>> 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 <srikars@nvidia.com>
>> Co-developed-by: Sumit Gupta <sumitg@nvidia.com>
>> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
>> ---
>>   drivers/acpi/arm64/Makefile          |  1 +
>>   drivers/acpi/arm64/thermal_cpufreq.c | 22 +++++++++++++
>>   drivers/acpi/internal.h              |  9 +++++
>>   drivers/acpi/processor_thermal.c     | 49 +++++++++++++++++++++++-----
>>   4 files changed, 72 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..40d5806ed528
>> --- /dev/null
>> +++ b/drivers/acpi/arm64/thermal_cpufreq.c
>> @@ -0,0 +1,22 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +#include <linux/acpi.h>
>> +
>> +#include "../internal.h"
>> +
>> +#ifdef CONFIG_HAVE_ARM_SMCCC_DISCOVERY
>> +#define SMCCC_SOC_ID_T241      0x036b0241
> 
> Sorry for missing this earlier. Not sure if the above define needs to be
> conditional. Even if it has to be, CONFIG_ARM_SMCCC_SOC_ID is more
> appropriate.
> 

Will remove the ifdef.

>> +
>> +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;
>> +}
>> +#endif
>> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
>> index 866c7c4ed233..ee213a8cddc5 100644
>> --- a/drivers/acpi/internal.h
>> +++ b/drivers/acpi/internal.h
>> @@ -85,6 +85,15 @@ bool acpi_scan_is_offline(struct acpi_device *adev, bool uevent);
>>   acpi_status acpi_sysfs_table_handler(u32 event, void *table, void *context);
>>   void acpi_scan_table_notify(void);
>>
>> +#ifdef CONFIG_HAVE_ARM_SMCCC_DISCOVERY
> 
> It looks weird to add a such specific ARM config option in generic ACPI
> code/header.
> 
> Does it make sense to add some new config this new feature you are adding
> or just use ARM64 and have CONFIG_HAVE_ARM_SMCCC_DISCOVERY check internally
> in the arch specific call.
> 
> --
> Regards,
> Sudeep

Ok, will use CONFIG_ARM64 instead.
I think we don't need to check for CONFIG_HAVE_ARM_SMCCC_DISCOVERY 
inside the arch call as it returns zero if the soc_id value is different 
from Tegra241.

Best Regards,
Sumit Gupta

Re: [Patch v6 1/2] ACPI: thermal: Add Thermal fast Sampling Period (_TFP) support

[Rafael J. Wysocki]

On Thu, Nov 9, 2023 at 7:34 PM Sumit Gupta <sumitg@nvidia.com> wrote:
>
> From: Jeff Brasen <jbrasen@nvidia.com>
>
> Add support of "Thermal fast Sampling Period (_TFP)" for Passive cooling.
> As per [1], _TFP overrides the "Thermal Sampling Period (_TSP)" if both
> are present in a Thermal zone.
>
> [1] ACPI Specification 6.4 - section 11.4.17. _TFP (Thermal fast Sampling
>     Period)"
>
> Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
> Co-developed-by: Sumit Gupta <sumitg@nvidia.com>
> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
> ---
>  drivers/acpi/thermal.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
> index f74d81abdbfc..3b75eb2260d7 100644
> --- a/drivers/acpi/thermal.c
> +++ b/drivers/acpi/thermal.c
> @@ -90,7 +90,7 @@ struct acpi_thermal_passive {
>         struct acpi_thermal_trip trip;
>         unsigned long tc1;
>         unsigned long tc2;
> -       unsigned long tsp;
> +       unsigned long delay;
>  };
>
>  struct acpi_thermal_active {
> @@ -404,11 +404,17 @@ static bool passive_trip_params_init(struct acpi_thermal *tz)
>
>         tz->trips.passive.tc2 = tmp;
>
> +       status = acpi_evaluate_integer(tz->device->handle, "_TFP", NULL, &tmp);
> +       if (ACPI_SUCCESS(status)) {
> +               tz->trips.passive.delay = tmp;
> +               return true;
> +       }
> +
>         status = acpi_evaluate_integer(tz->device->handle, "_TSP", NULL, &tmp);
>         if (ACPI_FAILURE(status))
>                 return false;
>
> -       tz->trips.passive.tsp = tmp;
> +       tz->trips.passive.delay = tmp * 100;
>
>         return true;
>  }
> @@ -904,7 +910,7 @@ static int acpi_thermal_add(struct acpi_device *device)
>
>         acpi_trip = &tz->trips.passive.trip;
>         if (acpi_thermal_trip_valid(acpi_trip)) {
> -               passive_delay = tz->trips.passive.tsp * 100;
> +               passive_delay = tz->trips.passive.delay;
>
>                 trip->type = THERMAL_TRIP_PASSIVE;
>                 trip->temperature = acpi_thermal_temp(tz, acpi_trip->temp_dk);
> --

So does the second patch in the series really depend on this one?

If not, I can apply it I think.

Re: [Patch v6 1/2] ACPI: thermal: Add Thermal fast Sampling Period (_TFP) support

[Sumit Gupta]

On 22/11/23 01:29, Rafael J. Wysocki wrote:
> External email: Use caution opening links or attachments
> 
> 
> On Thu, Nov 9, 2023 at 7:34 PM Sumit Gupta <sumitg@nvidia.com> wrote:
>>
>> From: Jeff Brasen <jbrasen@nvidia.com>
>>
>> Add support of "Thermal fast Sampling Period (_TFP)" for Passive cooling.
>> As per [1], _TFP overrides the "Thermal Sampling Period (_TSP)" if both
>> are present in a Thermal zone.
>>
>> [1] ACPI Specification 6.4 - section 11.4.17. _TFP (Thermal fast Sampling
>>      Period)"
>>
>> Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
>> Co-developed-by: Sumit Gupta <sumitg@nvidia.com>
>> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
>> ---
>>   drivers/acpi/thermal.c | 12 +++++++++---
>>   1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
>> index f74d81abdbfc..3b75eb2260d7 100644
>> --- a/drivers/acpi/thermal.c
>> +++ b/drivers/acpi/thermal.c
>> @@ -90,7 +90,7 @@ struct acpi_thermal_passive {
>>          struct acpi_thermal_trip trip;
>>          unsigned long tc1;
>>          unsigned long tc2;
>> -       unsigned long tsp;
>> +       unsigned long delay;
>>   };
>>
>>   struct acpi_thermal_active {
>> @@ -404,11 +404,17 @@ static bool passive_trip_params_init(struct acpi_thermal *tz)
>>
>>          tz->trips.passive.tc2 = tmp;
>>
>> +       status = acpi_evaluate_integer(tz->device->handle, "_TFP", NULL, &tmp);
>> +       if (ACPI_SUCCESS(status)) {
>> +               tz->trips.passive.delay = tmp;
>> +               return true;
>> +       }
>> +
>>          status = acpi_evaluate_integer(tz->device->handle, "_TSP", NULL, &tmp);
>>          if (ACPI_FAILURE(status))
>>                  return false;
>>
>> -       tz->trips.passive.tsp = tmp;
>> +       tz->trips.passive.delay = tmp * 100;
>>
>>          return true;
>>   }
>> @@ -904,7 +910,7 @@ static int acpi_thermal_add(struct acpi_device *device)
>>
>>          acpi_trip = &tz->trips.passive.trip;
>>          if (acpi_thermal_trip_valid(acpi_trip)) {
>> -               passive_delay = tz->trips.passive.tsp * 100;
>> +               passive_delay = tz->trips.passive.delay;
>>
>>                  trip->type = THERMAL_TRIP_PASSIVE;
>>                  trip->temperature = acpi_thermal_temp(tz, acpi_trip->temp_dk);
>> --
> 
> So does the second patch in the series really depend on this one?
> 
> If not, I can apply it I think.

Yes, this patch can be applied independently. Thank you!

Best Regards,
Sumit Gupta

Re: [Patch v6 1/2] ACPI: thermal: Add Thermal fast Sampling Period (_TFP) support

[Rafael J. Wysocki]

On Wed, Nov 22, 2023 at 1:55 PM Sumit Gupta <sumitg@nvidia.com> wrote:
>
>
>
> On 22/11/23 01:29, Rafael J. Wysocki wrote:
> > External email: Use caution opening links or attachments
> >
> >
> > On Thu, Nov 9, 2023 at 7:34 PM Sumit Gupta <sumitg@nvidia.com> wrote:
> >>
> >> From: Jeff Brasen <jbrasen@nvidia.com>
> >>
> >> Add support of "Thermal fast Sampling Period (_TFP)" for Passive cooling.
> >> As per [1], _TFP overrides the "Thermal Sampling Period (_TSP)" if both
> >> are present in a Thermal zone.
> >>
> >> [1] ACPI Specification 6.4 - section 11.4.17. _TFP (Thermal fast Sampling
> >>      Period)"
> >>
> >> Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
> >> Co-developed-by: Sumit Gupta <sumitg@nvidia.com>
> >> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
> >> ---
> >>   drivers/acpi/thermal.c | 12 +++++++++---
> >>   1 file changed, 9 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
> >> index f74d81abdbfc..3b75eb2260d7 100644
> >> --- a/drivers/acpi/thermal.c
> >> +++ b/drivers/acpi/thermal.c
> >> @@ -90,7 +90,7 @@ struct acpi_thermal_passive {
> >>          struct acpi_thermal_trip trip;
> >>          unsigned long tc1;
> >>          unsigned long tc2;
> >> -       unsigned long tsp;
> >> +       unsigned long delay;
> >>   };
> >>
> >>   struct acpi_thermal_active {
> >> @@ -404,11 +404,17 @@ static bool passive_trip_params_init(struct acpi_thermal *tz)
> >>
> >>          tz->trips.passive.tc2 = tmp;
> >>
> >> +       status = acpi_evaluate_integer(tz->device->handle, "_TFP", NULL, &tmp);
> >> +       if (ACPI_SUCCESS(status)) {
> >> +               tz->trips.passive.delay = tmp;
> >> +               return true;
> >> +       }
> >> +
> >>          status = acpi_evaluate_integer(tz->device->handle, "_TSP", NULL, &tmp);
> >>          if (ACPI_FAILURE(status))
> >>                  return false;
> >>
> >> -       tz->trips.passive.tsp = tmp;
> >> +       tz->trips.passive.delay = tmp * 100;
> >>
> >>          return true;
> >>   }
> >> @@ -904,7 +910,7 @@ static int acpi_thermal_add(struct acpi_device *device)
> >>
> >>          acpi_trip = &tz->trips.passive.trip;
> >>          if (acpi_thermal_trip_valid(acpi_trip)) {
> >> -               passive_delay = tz->trips.passive.tsp * 100;
> >> +               passive_delay = tz->trips.passive.delay;
> >>
> >>                  trip->type = THERMAL_TRIP_PASSIVE;
> >>                  trip->temperature = acpi_thermal_temp(tz, acpi_trip->temp_dk);
> >> --
> >
> > So does the second patch in the series really depend on this one?
> >
> > If not, I can apply it I think.
>
> Yes, this patch can be applied independently. Thank you!

OK, applied as 6.8 material (with some changelog edits), thanks!