Skip to contentSkip to conversation

Add driver for NVIDIA Tegra30 SoC Thermal sensor

PATCH Superseded Dmitry Osipenko 6 participants 22 messages May 29, 2021 – Jun 16, 2021

Subsystems (4)

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

v3 cover Add driver for NVIDIA Tegra30 SoC Thermal sensor

Hi,

This series adds support for the thermal sensor that is found on NVIDIA Tegra30 SoC. Sensor monitors temperature and voltage of the SoC, it also emits signals to the power management and clock controllers that are performing the emergency shut down and the CPU frequency throttling when a pre-programmed temperature levels are reached.

Please note that this series is made on top of ACMTON patches! [1]. Otherwise tegra30.dtsi will fail to compile.

[1] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=243115

Changelog:

v3: - No code changes. CC'ed linux-pm, which was previously missed by accident.
Not sure how much that is important for the thermal patches, but won't hurt to re-send since only DT binding was reviewed so far.

v2: - Made a very minor improvement to one error message, it now prints
number of channel at which error occurred.

- Added r-b from Rob Herring to the binding.

Dmitry Osipenko (7):
dt-bindings: thermal: Add binding for Tegra30 thermal sensor
thermal: thermal_of: Stop zone device before unregistering it thermal/core: Export thermal_cooling_device_stats_update() thermal/drivers/tegra: Add driver for Tegra30 thermal sensor
ARM: tegra_defconfig: Enable CONFIG_TEGRA30_TSENSOR
ARM: multi_v7_defconfig: Enable CONFIG_TEGRA30_TSENSOR
ARM: tegra: Add SoC thermal sensor to Tegra30 device-trees

 .../thermal/nvidia,tegra30-tsensor.yaml       |  78 ++
 arch/arm/boot/dts/tegra30-ouya.dts            |  16 +
 arch/arm/boot/dts/tegra30.dtsi                |  93 ++-
 arch/arm/configs/multi_v7_defconfig           |   1 +
 arch/arm/configs/tegra_defconfig              |   1 +
 drivers/thermal/tegra/Kconfig                 |   7 +
 drivers/thermal/tegra/Makefile                |   1 +
 drivers/thermal/tegra/tegra30-tsensor.c       | 736 ++++++++++++++++++
 drivers/thermal/thermal_of.c                  |   3 +
 drivers/thermal/thermal_sysfs.c               |   1 +
 10 files changed, 933 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/thermal/nvidia,tegra30-tsensor.yaml
 create mode 100644 drivers/thermal/tegra/tegra30-tsensor.c
Signature · 1 line
2.30.2
#1 lore raw

PATCH v3 1/7 dt-bindings: thermal: Add binding for Tegra30 thermal sensor

All NVIDIA Tegra30 SoCs have on-chip sensors which monitor temperature and voltage of the SoC. Sensors also controls CPU x2 freq throttle and emits emergency shutdown signal. TSENSOR has has two separate channels for each sensor placed in a different parts of the SoC. Add binding for the sensor hardware.

---
 .../thermal/nvidia,tegra30-tsensor.yaml       | 78 +++++++++++++++++++
 1 file changed, 78 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/nvidia,tegra30-tsensor.yaml
Documentation/devicetree/bindings/thermal/nvidia,tegra30-tsensor.yaml new file +78 -0
new file mode 100644
index 000000000000..6182090d313c
@@ -0,0 +1,78 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/thermal/nvidia,tegra30-tsensor.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NVIDIA Tegra30 Thermal Sensor
+
+maintainers:
+  - Dmitry Osipenko <[email protected]>
+  - Jon Hunter <[email protected]>
+  - Thierry Reding <[email protected]>
+
+description: |
+  TSENSOR provides thermal and voltage sensors which monitor temperature
+  and voltage of the chip. Sensors are placed across the die to gauge the
+  temperature of the whole chip. The TSENSOR module:
+
+    Generates an interrupt to SW to lower temperature via DVFS on reaching
+    a certain thermal/voltage threshold.
+
+    Generates a signal to the CAR to reduce CPU frequency by half on reaching
+    a certain thermal/voltage threshold.
+
+    Generates a signal to the PMC when the temperature reaches dangerously high
+    levels to reset the chip and sets a flag in the PMC.
+
+  TSENSOR has two channels which monitor two different spots of the SoC.
+
+properties:
+  compatible:
+    const: nvidia,tegra30-tsensor
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  "#thermal-sensor-cells":
+    const: 1
+
+  "#cooling-cells":
+    const: 2
+
+  assigned-clock-parents: true
+  assigned-clock-rates: true
+  assigned-clocks: true
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - resets
+  - interrupts
+  - "#thermal-sensor-cells"
+  - "#cooling-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    thermal-sensor@70014000 {
+      compatible = "nvidia,tegra30-tsensor";
+      reg = <0x70014000 0x500>;
+      interrupts = <0 102 4>;
+      clocks = <&clk 100>;
+      resets = <&rst 100>;
+
+      #thermal-sensor-cells = <1>;
+      #cooling-cells = <2>;
+    };
Signature · 1 line
2.30.2
#2 lore raw

PATCH v3 2/7 thermal: thermal_of: Stop zone device before unregistering it

In reply to Dmitry Osipenko

Zone device is enabled after thermal_zone_of_sensor_register() completion, but it's not disabled before senors if unregistered, leaving temperature polling active. This results in accessing a disabled zone device and produces a warning about this problem. Stop zone device before unregistering it in order to fix this "use-after-free" problem.

---
 drivers/thermal/thermal_of.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/thermal/thermal_of.c +3 -0
index 5b76f9a1280d..6379f26a335f 100644
@@ -559,6 +559,9 @@ void thermal_zone_of_sensor_unregister(struct device *dev,
 	if (!tz)
 		return;
 
+	/* stop temperature polling */
+	thermal_zone_device_disable(tzd);
+
 	mutex_lock(&tzd->lock);
 	tzd->ops->get_temp = NULL;
 	tzd->ops->get_trend = NULL;
Signature · 1 line
2.30.2
#3 lore raw

PATCH v3 3/7 thermal/core: Export thermal_cooling_device_stats_update()

In reply to Dmitry Osipenko

NVIDIA Tegra30 thermal sensor driver has a hardware-controlled CPU cooling feature that halves CPU frequency once a specified trip point is breached. In order to account the hardware state transitions, which are reported by interrupt, the sensor driver needs to report the cooling state transition and this is done by thermal_cooling_device_stats_update(). The sensor driver could be compiled as a loadable driver module, but this API function isn't exported, hence export it.

---
 drivers/thermal/thermal_sysfs.c | 1 +
 1 file changed, 1 insertion(+)
drivers/thermal/thermal_sysfs.c +1 -0
index 1c4aac8464a7..ab373280f853 100644
@@ -690,6 +690,7 @@ void thermal_cooling_device_stats_update(struct thermal_cooling_device *cdev,
 unlock:
 	spin_unlock(&stats->lock);
 }
+EXPORT_SYMBOL_GPL(thermal_cooling_device_stats_update);
 
 static ssize_t total_trans_show(struct device *dev,
 				struct device_attribute *attr, char *buf)
Signature · 1 line
2.30.2
#4 lore raw

PATCH v3 4/7 thermal/drivers/tegra: Add driver for Tegra30 thermal sensor

In reply to Dmitry Osipenko

All NVIDIA Tegra30 SoCs have a two-channel on-chip sensor unit which monitors temperature and voltage of the SoC. Sensors control CPU frequency throttling, which is activated by hardware once preprogrammed temperature level is breached, they also send signal to Power Management controller to perform emergency shutdown on a critical overheat of the SoC die. Add driver for the Tegra30 TSENSOR module, exposing it as a thermal sensor and a cooling device.

---
 drivers/thermal/tegra/Kconfig           |   7 +
 drivers/thermal/tegra/Makefile          |   1 +
 drivers/thermal/tegra/tegra30-tsensor.c | 736 ++++++++++++++++++++++++
 3 files changed, 744 insertions(+)
 create mode 100644 drivers/thermal/tegra/tegra30-tsensor.c
drivers/thermal/tegra/Kconfig +7 -0
index 46c2215867cd..019e3a2eb69e 100644
@@ -18,4 +18,11 @@ config TEGRA_BPMP_THERMAL
 	  Enable this option for support for sensing system temperature of NVIDIA
 	  Tegra systems-on-chip with the BPMP coprocessor (Tegra186).
 
+config TEGRA30_TSENSOR
+	tristate "Tegra30 Thermal Sensor"
+	depends on ARCH_TEGRA_3x_SOC || COMPILE_TEST
+	help
+	  Enable this option to support thermal management of NVIDIA Tegra30
+	  system-on-chip.
+
 endmenu
drivers/thermal/tegra/Makefile +1 -0
index 0f2b66edf0d2..eb27d194c583 100644
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_TEGRA_SOCTHERM)		+= tegra-soctherm.o
 obj-$(CONFIG_TEGRA_BPMP_THERMAL)	+= tegra-bpmp-thermal.o
+obj-$(CONFIG_TEGRA30_TSENSOR)		+= tegra30-tsensor.o
 
 tegra-soctherm-y				:= soctherm.o soctherm-fuse.o
 tegra-soctherm-$(CONFIG_ARCH_TEGRA_124_SOC)	+= tegra124-soctherm.o
drivers/thermal/tegra/tegra30-tsensor.c new file +736 -0

736 lines not on this page · raw message

Signature · 1 line
2.30.2
Acked-by on v3 4/7
#5 lore raw

Re: [PATCH v3 4/7] thermal/drivers/tegra: Add driver for Tegra30 thermal sensor

Quoted 16 lines · Dmitry Osipenko

All NVIDIA Tegra30 SoCs have a two-channel on-chip sensor unit which monitors temperature and voltage of the SoC. Sensors control CPU frequency throttling, which is activated by hardware once preprogrammed temperature level is breached, they also send signal to Power Management controller to perform emergency shutdown on a critical overheat of the SoC die. Add driver for the Tegra30 TSENSOR module, exposing it as a thermal sensor and a cooling device.

Quoted 6 lines
---
 drivers/thermal/tegra/Kconfig           |   7 +
 drivers/thermal/tegra/Makefile          |   1 +
 drivers/thermal/tegra/tegra30-tsensor.c | 736 ++++++++++++++++++++++++
 3 files changed, 744 insertions(+)
 create mode 100644 drivers/thermal/tegra/tegra30-tsensor.c
#6 lore raw

Re: [PATCH v3 4/7] thermal/drivers/tegra: Add driver for Tegra30 thermal sensor

In reply to Dmitry Osipenko

[Cc Viresh]

Quoted 7 lines · Dmitry Osipenko

All NVIDIA Tegra30 SoCs have a two-channel on-chip sensor unit which monitors temperature and voltage of the SoC. Sensors control CPU frequency throttling, which is activated by hardware once preprogrammed temperature level is breached, they also send signal to Power Management controller to perform emergency shutdown on a critical overheat of the SoC die. Add driver for the Tegra30 TSENSOR module, exposing it as a thermal sensor and a cooling device.

IMO it does not make sense to expose the hardware throttling mechanism as a cooling device because it is not usable anywhere from the thermal framework.

Moreover, that will collide with the thermal / cpufreq framework mitigation (hardware sets the frequency but the software thinks the freq is different), right ?

The hardware limiter should let know the cpufreq framework about the frequency change.

	https://lkml.org/lkml/2021/6/8/1792

May be post the sensor without the hw limiter for now and address that in a separate series ?

-- Daniel

Signature · 5 lines
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
#7 lore raw

Re: [PATCH v3 4/7] thermal/drivers/tegra: Add driver for Tegra30 thermal sensor

Quoted 12 lines · Daniel Lezcano

[Cc Viresh]

Quoted 7 lines · Dmitry Osipenko

All NVIDIA Tegra30 SoCs have a two-channel on-chip sensor unit which monitors temperature and voltage of the SoC. Sensors control CPU frequency throttling, which is activated by hardware once preprogrammed temperature level is breached, they also send signal to Power Management controller to perform emergency shutdown on a critical overheat of the SoC die. Add driver for the Tegra30 TSENSOR module, exposing it as a thermal sensor and a cooling device.

Quoted 7 lines

IMO it does not make sense to expose the hardware throttling mechanism as a cooling device because it is not usable anywhere from the thermal framework.

Moreover, that will collide with the thermal / cpufreq framework mitigation (hardware sets the frequency but the software thinks the freq is different), right ?

I am not even sure what the cooling device is doing here:

tegra_tsensor_set_cur_state() is not implemented and it says hardware changed it by itself. What is the benefit you are getting out of the cooling device here ?

Quoted 7 lines

The hardware limiter should let know the cpufreq framework about the frequency change.

	https://lkml.org/lkml/2021/6/8/1792

May be post the sensor without the hw limiter for now and address that in a separate series ?

Signature · 1 line
viresh
#8 lore raw

Re: [PATCH v3 4/7] thermal/drivers/tegra: Add driver for Tegra30 thermal sensor

15.06.2021 13:26, Viresh Kumar пишет:

Quoted 13 lines · Daniel Lezcano
Quoted 12 lines

[Cc Viresh]

Quoted 7 lines · Dmitry Osipenko

All NVIDIA Tegra30 SoCs have a two-channel on-chip sensor unit which monitors temperature and voltage of the SoC. Sensors control CPU frequency throttling, which is activated by hardware once preprogrammed temperature level is breached, they also send signal to Power Management controller to perform emergency shutdown on a critical overheat of the SoC die. Add driver for the Tegra30 TSENSOR module, exposing it as a thermal sensor and a cooling device.

Quoted 7 lines
Quoted 7 lines

IMO it does not make sense to expose the hardware throttling mechanism as a cooling device because it is not usable anywhere from the thermal framework.

Moreover, that will collide with the thermal / cpufreq framework mitigation (hardware sets the frequency but the software thinks the freq is different), right ?

H/w mitigation is additional and should be transparent to the software mitigation. The software mitigation is much more flexible, but it has latency. Software also could crash and hang.

Hardware mitigation doesn't have latency and it will continue to work regardless of the software state.

The CCF driver is aware about the h/w cooling status [1], hence software sees the actual frequency.

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit?id=344d5df34f5abd468267daa98f041abf90b2f660

Quoted 5 lines

I am not even sure what the cooling device is doing here:

tegra_tsensor_set_cur_state() is not implemented and it says hardware changed it by itself. What is the benefit you are getting out of the cooling device here ?

It allows userspace to check whether hardware cooling is active via the cooling_device sysfs. Otherwise we don't have ability to check whether h/w cooling is active, I think it's a useful information. It's also interesting to see the cooling_device stats, showing how many times h/w mitigation was active.

Quoted 8 lines
Quoted 7 lines

The hardware limiter should let know the cpufreq framework about the frequency change.

	https://lkml.org/lkml/2021/6/8/1792

May be post the sensor without the hw limiter for now and address that in a separate series ?

I wasn't aware about existence of the thermal pressure, thank you for pointing at it. At a quick glance it should be possible to benefit from the information about the additional pressure.

Seems the current thermal pressure API assumes that there is only one user of the API. So it's impossible to aggregate the pressure from different sources, like software cpufreq pressure + h/w freq pressure. Correct? If yes, then please let me know yours thoughts about the best approach of supporting the aggregation.

I'll factor out the h/w limiter from this patchset and prepare the v4. Thank you all for taking a look at the patches.

#9 lore raw

Re: [PATCH v3 4/7] thermal/drivers/tegra: Add driver for Tegra30 thermal sensor

Quoted 22 lines · Dmitry Osipenko

15.06.2021 13:26, Viresh Kumar пишет:

Quoted 13 lines · Daniel Lezcano
Quoted 12 lines

[Cc Viresh]

Quoted 7 lines · Dmitry Osipenko

All NVIDIA Tegra30 SoCs have a two-channel on-chip sensor unit which monitors temperature and voltage of the SoC. Sensors control CPU frequency throttling, which is activated by hardware once preprogrammed temperature level is breached, they also send signal to Power Management controller to perform emergency shutdown on a critical overheat of the SoC die. Add driver for the Tegra30 TSENSOR module, exposing it as a thermal sensor and a cooling device.

Quoted 7 lines
Quoted 7 lines

IMO it does not make sense to expose the hardware throttling mechanism as a cooling device because it is not usable anywhere from the thermal framework.

Moreover, that will collide with the thermal / cpufreq framework mitigation (hardware sets the frequency but the software thinks the freq is different), right ?

Quoted 6 lines

H/w mitigation is additional and should be transparent to the software mitigation. The software mitigation is much more flexible, but it has latency. Software also could crash and hang.

Hardware mitigation doesn't have latency and it will continue to work regardless of the software state.

Yes, I agree. Both solutions have their pros and cons. However, I don't think they can co-exist sanely.

Quoted 5 lines

The CCF driver is aware about the h/w cooling status [1], hence software sees the actual frequency.

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit?id=344d5df34f5abd468267daa98f041abf90b2f660

Ah interesting, thanks for the pointer.

What I'm worried about is the consistency with cpufreq.

Probably cpufreq_update_limits() should be called from the interrupt handler.

Quoted 11 lines
Quoted 5 lines

I am not even sure what the cooling device is doing here:

tegra_tsensor_set_cur_state() is not implemented and it says hardware changed it by itself. What is the benefit you are getting out of the cooling device here ?

It allows userspace to check whether hardware cooling is active via the cooling_device sysfs. Otherwise we don't have ability to check whether h/w cooling is active, I think it's a useful information. It's also interesting to see the cooling_device stats, showing how many times h/w mitigation was active.

Actually the stats are for software mitigation. For the driver, create a debugfs entry like what do the other drivers or a module parameter with the stats.

Quoted 13 lines
Quoted 8 lines
Quoted 7 lines

The hardware limiter should let know the cpufreq framework about the frequency change.

	https://lkml.org/lkml/2021/6/8/1792

May be post the sensor without the hw limiter for now and address that in a separate series ?

I wasn't aware about existence of the thermal pressure, thank you for pointing at it. At a quick glance it should be possible to benefit from the information about the additional pressure.

Quoted 5 lines

Seems the current thermal pressure API assumes that there is only one user of the API. So it's impossible to aggregate the pressure from different sources, like software cpufreq pressure + h/w freq pressure. Correct? If yes, then please let me know yours thoughts about the best approach of supporting the aggregation.

That is a good question. IMO, first step would be to call cpufreq_update_limits().

[ Cc Thara who implemented the thermal pressure ]

May be Thara has an idea about how to aggregate both? There is another series floating around with hardware limiter [1] and the same problematic.

[1] https://lkml.org/lkml/2021/6/8/1791

Quoted 2 lines

I'll factor out the h/w limiter from this patchset and prepare the v4. Thank you all for taking a look at the patches.

Signature · 5 lines
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
#10 lore raw

Re: [PATCH v3 4/7] thermal/drivers/tegra: Add driver for Tegra30 thermal sensor

15.06.2021 19:18, Daniel Lezcano пишет:

Quoted 39 lines · Dmitry Osipenko
Quoted 22 lines

15.06.2021 13:26, Viresh Kumar пишет:

Quoted 13 lines · Daniel Lezcano
Quoted 12 lines

[Cc Viresh]

Quoted 7 lines · Dmitry Osipenko

All NVIDIA Tegra30 SoCs have a two-channel on-chip sensor unit which monitors temperature and voltage of the SoC. Sensors control CPU frequency throttling, which is activated by hardware once preprogrammed temperature level is breached, they also send signal to Power Management controller to perform emergency shutdown on a critical overheat of the SoC die. Add driver for the Tegra30 TSENSOR module, exposing it as a thermal sensor and a cooling device.

Quoted 7 lines
Quoted 7 lines

IMO it does not make sense to expose the hardware throttling mechanism as a cooling device because it is not usable anywhere from the thermal framework.

Moreover, that will collide with the thermal / cpufreq framework mitigation (hardware sets the frequency but the software thinks the freq is different), right ?

Quoted 6 lines

H/w mitigation is additional and should be transparent to the software mitigation. The software mitigation is much more flexible, but it has latency. Software also could crash and hang.

Hardware mitigation doesn't have latency and it will continue to work regardless of the software state.

Yes, I agree. Both solutions have their pros and cons. However, I don't think they can co-exist sanely.

Quoted 5 lines

The CCF driver is aware about the h/w cooling status [1], hence software sees the actual frequency.

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit?id=344d5df34f5abd468267daa98f041abf90b2f660

Quoted 6 lines

Ah interesting, thanks for the pointer.

What I'm worried about is the consistency with cpufreq.

Probably cpufreq_update_limits() should be called from the interrupt handler.

IIUC, the cpufreq already should be prepared for the case where firmware may override frequency. Viresh, could you please clarify what are the possible implications of the frequency overriding?

Quoted 12 lines
Quoted 11 lines
Quoted 5 lines

I am not even sure what the cooling device is doing here:

tegra_tsensor_set_cur_state() is not implemented and it says hardware changed it by itself. What is the benefit you are getting out of the cooling device here ?

It allows userspace to check whether hardware cooling is active via the cooling_device sysfs. Otherwise we don't have ability to check whether h/w cooling is active, I think it's a useful information. It's also interesting to see the cooling_device stats, showing how many times h/w mitigation was active.

Quoted 3 lines

Actually the stats are for software mitigation. For the driver, create a debugfs entry like what do the other drivers or a module parameter with the stats.

Okay

Quoted 13 lines
Quoted 13 lines
Quoted 8 lines
Quoted 7 lines

The hardware limiter should let know the cpufreq framework about the frequency change.

	https://lkml.org/lkml/2021/6/8/1792

May be post the sensor without the hw limiter for now and address that in a separate series ?

I wasn't aware about existence of the thermal pressure, thank you for pointing at it. At a quick glance it should be possible to benefit from the information about the additional pressure.

Quoted 8 lines
Quoted 5 lines

Seems the current thermal pressure API assumes that there is only one user of the API. So it's impossible to aggregate the pressure from different sources, like software cpufreq pressure + h/w freq pressure. Correct? If yes, then please let me know yours thoughts about the best approach of supporting the aggregation.

That is a good question. IMO, first step would be to call cpufreq_update_limits().

Right

Quoted 6 lines

[ Cc Thara who implemented the thermal pressure ]

May be Thara has an idea about how to aggregate both? There is another series floating around with hardware limiter [1] and the same problematic.

[1] https://lkml.org/lkml/2021/6/8/1791

Thanks, it indeed looks similar.

I guess the common thermal pressure update code could be moved out into a new special cpufreq thermal QoS handler (policy->thermal_constraints), where handler will select the frequency constraint and set up the pressure accordingly. So there won't be any races in the code.

#11 lore raw

Re: [PATCH v3 4/7] thermal/drivers/tegra: Add driver for Tegra30 thermal sensor

Quoted 63 lines · Dmitry Osipenko

15.06.2021 19:18, Daniel Lezcano пишет:

Quoted 39 lines · Dmitry Osipenko
Quoted 22 lines

15.06.2021 13:26, Viresh Kumar пишет:

Quoted 13 lines · Daniel Lezcano
Quoted 12 lines

[Cc Viresh]

Quoted 7 lines · Dmitry Osipenko

All NVIDIA Tegra30 SoCs have a two-channel on-chip sensor unit which monitors temperature and voltage of the SoC. Sensors control CPU frequency throttling, which is activated by hardware once preprogrammed temperature level is breached, they also send signal to Power Management controller to perform emergency shutdown on a critical overheat of the SoC die. Add driver for the Tegra30 TSENSOR module, exposing it as a thermal sensor and a cooling device.

Quoted 7 lines
Quoted 7 lines

IMO it does not make sense to expose the hardware throttling mechanism as a cooling device because it is not usable anywhere from the thermal framework.

Moreover, that will collide with the thermal / cpufreq framework mitigation (hardware sets the frequency but the software thinks the freq is different), right ?

Quoted 6 lines

H/w mitigation is additional and should be transparent to the software mitigation. The software mitigation is much more flexible, but it has latency. Software also could crash and hang.

Hardware mitigation doesn't have latency and it will continue to work regardless of the software state.

Yes, I agree. Both solutions have their pros and cons. However, I don't think they can co-exist sanely.

Quoted 5 lines

The CCF driver is aware about the h/w cooling status [1], hence software sees the actual frequency.

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit?id=344d5df34f5abd468267daa98f041abf90b2f660

Quoted 6 lines

Ah interesting, thanks for the pointer.

What I'm worried about is the consistency with cpufreq.

Probably cpufreq_update_limits() should be called from the interrupt handler.

IIUC, the cpufreq already should be prepared for the case where firmware may override frequency. Viresh, could you please clarify what are the possible implications of the frequency overriding?

Quoted 12 lines
Quoted 11 lines
Quoted 5 lines

I am not even sure what the cooling device is doing here:

tegra_tsensor_set_cur_state() is not implemented and it says hardware changed it by itself. What is the benefit you are getting out of the cooling device here ?

It allows userspace to check whether hardware cooling is active via the cooling_device sysfs. Otherwise we don't have ability to check whether h/w cooling is active, I think it's a useful information. It's also interesting to see the cooling_device stats, showing how many times h/w mitigation was active.

Quoted 24 lines
Quoted 3 lines

Actually the stats are for software mitigation. For the driver, create a debugfs entry like what do the other drivers or a module parameter with the stats.

Okay

Quoted 18 lines
Quoted 18 lines
Quoted 8 lines
Quoted 7 lines

The hardware limiter should let know the cpufreq framework about the frequency change.

	https://lkml.org/lkml/2021/6/8/1792

May be post the sensor without the hw limiter for now and address that in a separate series ?

I wasn't aware about existence of the thermal pressure, thank you for pointing at it. At a quick glance it should be possible to benefit from the information about the additional pressure.

Seems the current thermal pressure API assumes that there is only one user of the API. So it's impossible to aggregate the pressure from different sources, like software cpufreq pressure + h/w freq pressure. Correct? If yes, then please let me know yours thoughts about the best approach of supporting the aggregation.

Hi,

Thermal pressure is letting scheduler know that the max capacity available for a cpu to schedule tasks is reduced due to a thermal event.
So you cannot have a h/w thermal pressure and s/w thermal pressure. There is eventually only one capping applied at h/w level and the frequency corresponding to this capping should be used for thermal pressure.

Ideally you should not be having both s/w and h/w trying to throttle at the same time. Why is this a scenario and what prevents you from disabling s/w throttling when h/w throttling is enabled. Now if there has to a aggregation for whatever reason this should be done at the thermal driver level and passed to scheduler.

Quoted 11 lines
Quoted 3 lines

That is a good question. IMO, first step would be to call cpufreq_update_limits().

Right

Quoted 5 lines

[ Cc Thara who implemented the thermal pressure ]

May be Thara has an idea about how to aggregate both? There is another series floating around with hardware limiter [1] and the same problematic.

Quoted 9 lines
Quoted 1 line

[1] https://lkml.org/lkml/2021/6/8/1791

Thanks, it indeed looks similar.

I guess the common thermal pressure update code could be moved out into a new special cpufreq thermal QoS handler (policy->thermal_constraints), where handler will select the frequency constraint and set up the pressure accordingly. So there won't be any races in the code.

It was a conscious decision to keep thermal pressure update out of qos max freq update because there are platforms that don't use the qos framework. For eg acpi uses cpufreq_update_policy.
But you are right. We have two platforms now applying h/w throttling and cpufreq_cooling applying s/w throttling. So it does make sense to have one api doing all the computation to update thermal pressure. I am not sure how exactly/where exactly this will reside.

So for starters, I think you should replicate the update of thermal pressure in your h/w driver when you know that h/w is throttling/throttled the frequency. You can refer to cpufreq_cooling.c to see how it is done.

Moving to a common api can be done as a separate patch series.

Signature · 2 lines
Warm Regards
Thara (She/Her/Hers)
#12 lore raw

Re: [PATCH v3 4/7] thermal/drivers/tegra: Add driver for Tegra30 thermal sensor

16.06.2021 05:50, Thara Gopinath пишет:
...

Quoted 10 lines

Hi,

Thermal pressure is letting scheduler know that the max capacity available for a cpu to schedule tasks is reduced due to a thermal event. So you cannot have a h/w thermal pressure and s/w thermal pressure. There is eventually only one capping applied at h/w level and the frequency corresponding to this capping should be used for thermal pressure.

Quoted 5 lines

Ideally you should not be having both s/w and h/w trying to throttle at the same time. Why is this a scenario and what prevents you from disabling s/w throttling when h/w throttling is enabled. Now if there has to a aggregation for whatever reason this should be done at the thermal driver level and passed to scheduler.

Hello,

The h/w mitigation is much more reactive than software, in the same time it's much less flexible than software. It should provide additional protection in a cases where software isn't doing a good job. Ideally h/w mitigation should stay inactive all the time, nevertheless it should be modeled properly by the driver.

Quoted 21 lines
Quoted 12 lines
Quoted 3 lines

That is a good question. IMO, first step would be to call cpufreq_update_limits().

Right

Quoted 6 lines

[ Cc Thara who implemented the thermal pressure ]

May be Thara has an idea about how to aggregate both? There is another series floating around with hardware limiter [1] and the same problematic.

Quoted 9 lines
Quoted 1 line

  [1] https://lkml.org/lkml/2021/6/8/1791

Thanks, it indeed looks similar.

I guess the common thermal pressure update code could be moved out into a new special cpufreq thermal QoS handler (policy->thermal_constraints), where handler will select the frequency constraint and set up the pressure accordingly. So there won't be any races in the code.

Quoted 7 lines

It was a conscious decision to keep thermal pressure update out of qos max freq update because there are platforms that don't use the qos framework. For eg acpi uses cpufreq_update_policy.
But you are right. We have two platforms now applying h/w throttling and cpufreq_cooling applying s/w throttling. So it does make sense to have one api doing all the computation to update thermal pressure. I am not sure how exactly/where exactly this will reside.

The generic cpufreq_cooling already uses QoS for limiting the CPU frequency. It could be okay to use QoS for the OF drivers, this needs a closer look.

We have the case where CPU frequency is changed by the thermal event and the thermal pressure equation is the same for both s/w cpufreq_cooling and h/w thermal driver. The pressure is calculated based on the QoS cpufreq constraint that is already aggregated.

Hence what we may need to do on the thermal event is:

1. Update the QoS request
2. Update the thermal pressure
3. Ensure that updates are not racing

Quoted 7 lines

So for starters, I think you should replicate the update of thermal pressure in your h/w driver when you know that h/w is throttling/throttled the frequency. You can refer to cpufreq_cooling.c to see how it is done.

Moving to a common api can be done as a separate patch series.

Thank you for the clarification and suggestion.

#13 lore raw

Re: [PATCH v3 4/7] thermal/drivers/tegra: Add driver for Tegra30 thermal sensor

Quoted 18 lines · Dmitry Osipenko

16.06.2021 05:50, Thara Gopinath пишет:
...

Quoted 10 lines

Hi,

Thermal pressure is letting scheduler know that the max capacity available for a cpu to schedule tasks is reduced due to a thermal event. So you cannot have a h/w thermal pressure and s/w thermal pressure. There is eventually only one capping applied at h/w level and the frequency corresponding to this capping should be used for thermal pressure.

Quoted 5 lines

Ideally you should not be having both s/w and h/w trying to throttle at the same time. Why is this a scenario and what prevents you from disabling s/w throttling when h/w throttling is enabled. Now if there has to a aggregation for whatever reason this should be done at the thermal driver level and passed to scheduler.

Quoted 7 lines

Hello,

The h/w mitigation is much more reactive than software, in the same time it's much less flexible than software. It should provide additional protection in a cases where software isn't doing a good job. Ideally h/w mitigation should stay inactive all the time, nevertheless it should be modeled properly by the driver.

Ok. This is kind of opposite to what I am doing on the Qcom platform I am working on. The h/w throttling is the default since like you mentioned it is more reactive. And s/w does only critical trip management.

Quoted 39 lines
Quoted 22 lines
Quoted 13 lines
Quoted 4 lines

That is a good question. IMO, first step would be to call cpufreq_update_limits().

Right

Quoted 6 lines

[ Cc Thara who implemented the thermal pressure ]

May be Thara has an idea about how to aggregate both? There is another series floating around with hardware limiter [1] and the same problematic.

Quoted 9 lines
Quoted 1 line

  [1] https://lkml.org/lkml/2021/6/8/1791

Thanks, it indeed looks similar.

I guess the common thermal pressure update code could be moved out into a new special cpufreq thermal QoS handler (policy->thermal_constraints), where handler will select the frequency constraint and set up the pressure accordingly. So there won't be any races in the code.

Quoted 7 lines

It was a conscious decision to keep thermal pressure update out of qos max freq update because there are platforms that don't use the qos framework. For eg acpi uses cpufreq_update_policy.
But you are right. We have two platforms now applying h/w throttling and cpufreq_cooling applying s/w throttling. So it does make sense to have one api doing all the computation to update thermal pressure. I am not sure how exactly/where exactly this will reside.

The generic cpufreq_cooling already uses QoS for limiting the CPU frequency. It could be okay to use QoS for the OF drivers, this needs a closer look.

We have the case where CPU frequency is changed by the thermal event and the thermal pressure equation is the same for both s/w cpufreq_cooling and h/w thermal driver. The pressure is calculated based on the QoS cpufreq constraint that is already aggregated.

Quoted 5 lines

Hence what we may need to do on the thermal event is:

1. Update the QoS request
2. Update the thermal pressure
3. Ensure that updates are not racing

Yes. So the first two steps you mentioned is exactly what cpufreq_cooling.c also does except for the fact that it is a s/w mitigation. Now if you have two sources that is updating the max frequency via qos, I think you can do either of the following before calculating thermal pressure
1. Read the throttled frequency from h/w if your h/w supports this feature.
or
2. Use freq_qos_read_value to get the max frequency value.

Either way only the correct throttled capacity should be passed to scheduler.

Signature · 2 lines
Warm Regards
Thara (She/Her/Hers)
Quoted 11 lines
Quoted 8 lines

So for starters, I think you should replicate the update of thermal pressure in your h/w driver when you know that h/w is throttling/throttled the frequency. You can refer to cpufreq_cooling.c to see how it is done.

Moving to a common api can be done as a separate patch series.

Thank you for the clarification and suggestion.

#14 lore raw

Re: [PATCH v3 4/7] thermal/drivers/tegra: Add driver for Tegra30 thermal sensor

In reply to Dmitry Osipenko

+Vincent.

Quoted 3 lines · Dmitry Osipenko

IIUC, the cpufreq already should be prepared for the case where firmware may override frequency. Viresh, could you please clarify what are the possible implications of the frequency overriding?

The only implication is software would think hardware is running at some other frequency, while it is not. Not sure if something may break as a result of this.

The scheduler's view of CPUs will not be same though, i.e. scheduler will see capacity as X, while in reality it has changed to Y.

Signature · 1 line
viresh
#15 lore raw

Re: [PATCH v3 4/7] thermal/drivers/tegra: Add driver for Tegra30 thermal sensor

Quoted 8 lines · Viresh Kumar

+Vincent.

Quoted 3 lines · Dmitry Osipenko

IIUC, the cpufreq already should be prepared for the case where firmware may override frequency. Viresh, could you please clarify what are the possible implications of the frequency overriding?

Quoted 6 lines

The only implication is software would think hardware is running at some other frequency, while it is not. Not sure if something may break as a result of this.

The scheduler's view of CPUs will not be same though, i.e. scheduler will see capacity as X, while in reality it has changed to Y.

thermal_pressure is used by scheduler to balance the load between CPUs according to the actual max frequency. If the thermal pressure doesn't reflect reality, scheduler will end up enqueuing too many tasks on a throttle CPU.

Quoted 3 lines

--
viresh

#16 lore raw

Re: [PATCH v3 4/7] thermal/drivers/tegra: Add driver for Tegra30 thermal sensor

16.06.2021 11:30, Vincent Guittot пишет:

Quoted 13 lines · Viresh Kumar
Quoted 12 lines

+Vincent.

Quoted 3 lines · Dmitry Osipenko

IIUC, the cpufreq already should be prepared for the case where firmware may override frequency. Viresh, could you please clarify what are the possible implications of the frequency overriding?

The only implication is software would think hardware is running at some other frequency, while it is not. Not sure if something may break as a result of this.

Quoted 7 lines
Quoted 2 lines

The scheduler's view of CPUs will not be same though, i.e. scheduler will see capacity as X, while in reality it has changed to Y.

thermal_pressure is used by scheduler to balance the load between CPUs according to the actual max frequency. If the thermal pressure doesn't reflect reality, scheduler will end up enqueuing too many tasks on a throttle CPU.

What if all CPUs are throttled equally and running on the same frequency, will throttling have any effect on the scheduler decisions?

#17 lore raw

Re: [PATCH v3 4/7] thermal/drivers/tegra: Add driver for Tegra30 thermal sensor

Quoted 18 lines · Dmitry Osipenko

16.06.2021 11:30, Vincent Guittot пишет:

Quoted 16 lines · Viresh Kumar
Quoted 14 lines

+Vincent.

Quoted 3 lines · Dmitry Osipenko

IIUC, the cpufreq already should be prepared for the case where firmware may override frequency. Viresh, could you please clarify what are the possible implications of the frequency overriding?

The only implication is software would think hardware is running at some other frequency, while it is not. Not sure if something may break as a result of this.

The scheduler's view of CPUs will not be same though, i.e. scheduler will see capacity as X, while in reality it has changed to Y.

Quoted 7 lines
Quoted 4 lines

thermal_pressure is used by scheduler to balance the load between CPUs according to the actual max frequency. If the thermal pressure doesn't reflect reality, scheduler will end up enqueuing too many tasks on a throttle CPU.

What if all CPUs are throttled equally and running on the same frequency, will throttling have any effect on the scheduler decisions?

Yes, the capacity is also used to detect when CPUs have spare capacity or are already overloaded. We usually try to fill the spare capacity of a CPU (CPU's max capacity - current utilization) but he max capacity is reduced when the CPU is throttled, and the spare capacity doesn't exist but scheduler could try to it

#18 lore raw

Re: [PATCH v3 4/7] thermal/drivers/tegra: Add driver for Tegra30 thermal sensor

16.06.2021 11:51, Vincent Guittot пишет:

Quoted 24 lines · Dmitry Osipenko
Quoted 23 lines

16.06.2021 11:30, Vincent Guittot пишет:

Quoted 20 lines · Viresh Kumar
Quoted 8 lines

+Vincent.

Quoted 3 lines · Dmitry Osipenko

IIUC, the cpufreq already should be prepared for the case where firmware may override frequency. Viresh, could you please clarify what are the possible implications of the frequency overriding?

Quoted 6 lines

The only implication is software would think hardware is running at some other frequency, while it is not. Not sure if something may break as a result of this.

The scheduler's view of CPUs will not be same though, i.e. scheduler will see capacity as X, while in reality it has changed to Y.

thermal_pressure is used by scheduler to balance the load between CPUs according to the actual max frequency. If the thermal pressure doesn't reflect reality, scheduler will end up enqueuing too many tasks on a throttle CPU.

Quoted 9 lines
Quoted 2 lines

What if all CPUs are throttled equally and running on the same frequency, will throttling have any effect on the scheduler decisions?

Yes, the capacity is also used to detect when CPUs have spare capacity or are already overloaded. We usually try to fill the spare capacity of a CPU (CPU's max capacity - current utilization) but he max capacity is reduced when the CPU is throttled, and the spare capacity doesn't exist but scheduler could try to it

Thank you.

#19 lore raw

PATCH v3 5/7 ARM: tegra_defconfig: Enable CONFIG_TEGRA30_TSENSOR

In reply to Dmitry Osipenko

Enable NVIDIA Tegra30 SoC thermal sensor driver in tegra_defconfig.

---
 arch/arm/configs/tegra_defconfig | 1 +
 1 file changed, 1 insertion(+)
arch/arm/configs/tegra_defconfig +1 -0
index 3d8d8af9524d..63e7f3261e6d 100644
@@ -169,6 +169,7 @@ CONFIG_THERMAL_STATISTICS=y
 CONFIG_CPU_THERMAL=y
 CONFIG_DEVFREQ_THERMAL=y
 CONFIG_TEGRA_SOCTHERM=m
+CONFIG_TEGRA30_TSENSOR=m
 CONFIG_WATCHDOG=y
 CONFIG_MAX77620_WATCHDOG=y
 CONFIG_TEGRA_WATCHDOG=y
Signature · 1 line
2.30.2
#20 lore raw

PATCH v3 6/7 ARM: multi_v7_defconfig: Enable CONFIG_TEGRA30_TSENSOR

In reply to Dmitry Osipenko

Enable NVIDIA Tegra30 SoC thermal sensor driver in multi_v7_defconfig.

---
 arch/arm/configs/multi_v7_defconfig | 1 +
 1 file changed, 1 insertion(+)
arch/arm/configs/multi_v7_defconfig +1 -0
index 52a0400fdd92..fc346f87d7f9 100644
@@ -523,6 +523,7 @@ CONFIG_BRCMSTB_THERMAL=m
 CONFIG_GENERIC_ADC_THERMAL=m
 CONFIG_ST_THERMAL_MEMMAP=y
 CONFIG_TEGRA_SOCTHERM=m
+CONFIG_TEGRA30_TSENSOR=m
 CONFIG_UNIPHIER_THERMAL=y
 CONFIG_DA9063_WATCHDOG=m
 CONFIG_XILINX_WATCHDOG=y
Signature · 1 line
2.30.2
#21 lore raw

PATCH v3 7/7 ARM: tegra: Add SoC thermal sensor to Tegra30 device-trees

In reply to Dmitry Osipenko

Add the on-chip SoC thermal sensor to Tegra30 device-trees. Now CPU temperature reporting and thermal throttling is available on all Tegra30 devices universally.

---
 arch/arm/boot/dts/tegra30-ouya.dts | 16 +++++
 arch/arm/boot/dts/tegra30.dtsi     | 93 ++++++++++++++++++++++++++++--
 2 files changed, 105 insertions(+), 4 deletions(-)
arch/arm/boot/dts/tegra30-ouya.dts +16 -0
index e767ac227a81..d792ce5c64c0 100644
@@ -468,6 +468,22 @@ map1 {
 				};
 			};
 		};
+
+		tsensor-channel0 {
+			trips {
+				dvfs-alert {
+					temperature = <70000>;
+				};
+
+				cpu-div2-throttle {
+					temperature = <75000>;
+				};
+
+				soc-critical {
+					temperature = <90000>;
+				};
+			};
+		};
 	};
 
 	vdd_12v_in: vdd_12v_in {
arch/arm/boot/dts/tegra30.dtsi +89 -4

137 lines not on this page · raw message

Signature · 1 line
2.30.2