LinuxLists.cc - [PATCH v4 0/5] Add LVTS support for mt8192

[permalink] [raw]

Subject: [PATCH v4 2/5] thermal/drivers/mediatek/lvts_thermal: Add suspend and resume

From: Balsam CHIHI <[email protected]>

Add suspend and resume support to LVTS driver.

Signed-off-by: Balsam CHIHI <[email protected]>
Signed-off-by: Bernhard Rosenkränzer <[email protected]>
Reviewed-by: Matthias Brugger <[email protected]>
---
drivers/thermal/mediatek/lvts_thermal.c | 34 +++++++++++++++++++++++++
1 file changed, 34 insertions(+)

diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
index d0a3f95b7884b..5ea8a9d569ea6 100644
--- a/drivers/thermal/mediatek/lvts_thermal.c
+++ b/drivers/thermal/mediatek/lvts_thermal.c
@@ -1169,6 +1169,38 @@ static int lvts_remove(struct platform_device *pdev)
return 0;
}

+static int lvts_suspend(struct platform_device *pdev, pm_message_t state)
+{
+ struct lvts_domain *lvts_td;
+ int i;
+
+ lvts_td = platform_get_drvdata(pdev);
+
+ for (i = 0; i < lvts_td->num_lvts_ctrl; i++)
+ lvts_ctrl_set_enable(&lvts_td->lvts_ctrl[i], false);
+
+ clk_disable_unprepare(lvts_td->clk);
+
+ return 0;
+}
+
+static int lvts_resume(struct platform_device *pdev)
+{
+ struct lvts_domain *lvts_td;
+ int i, ret;
+
+ lvts_td = platform_get_drvdata(pdev);
+
+ ret = clk_prepare_enable(lvts_td->clk);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < lvts_td->num_lvts_ctrl; i++)
+ lvts_ctrl_set_enable(&lvts_td->lvts_ctrl[i], true);
+
+ return 0;
+}
+
static const struct lvts_ctrl_data mt8195_lvts_mcu_data_ctrl[] = {
{
.cal_offset = { 0x04, 0x07 },
@@ -1268,6 +1300,8 @@ MODULE_DEVICE_TABLE(of, lvts_of_match);
static struct platform_driver lvts_driver = {
.probe = lvts_probe,
.remove = lvts_remove,
+ .suspend = lvts_suspend,
+ .resume = lvts_resume,
.driver = {
.name = "mtk-lvts-thermal",
.of_match_table = lvts_of_match,
--
2.41.0.rc2

2023-05-30 20:01:29

[permalink] [raw]

Subject: [PATCH v4 5/5] thermal/drivers/mediatek/lvts_thermal: Update calibration data documentation

From: Balsam CHIHI <[email protected]>

Update LVTS calibration data documentation for mt8192 and mt8195.

Signed-off-by: Balsam CHIHI <[email protected]>
Reviewed-by: Nícolas F. R. A. Prado <[email protected]>
[[email protected]: Fix issues pointed out by Nícolas F. R. A. Prado <[email protected]>]
Signed-off-by: Bernhard Rosenkränzer <[email protected]>
Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/thermal/mediatek/lvts_thermal.c | 31 +++++++++++++++++++++++--
1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
index d5e5214784ece..9185d02003633 100644
--- a/drivers/thermal/mediatek/lvts_thermal.c
+++ b/drivers/thermal/mediatek/lvts_thermal.c
@@ -531,7 +531,8 @@ static int lvts_sensor_init(struct device *dev, struct lvts_ctrl *lvts_ctrl,
* The efuse blob values follows the sensor enumeration per thermal
* controller. The decoding of the stream is as follow:
*
- * stream index map for MCU Domain :
+ * MT8195 :
+ * Stream index map for MCU Domain mt8195 :
*
* <-----mcu-tc#0-----> <-----sensor#0-----> <-----sensor#1----->
* 0x01 | 0x02 | 0x03 | 0x04 | 0x05 | 0x06 | 0x07 | 0x08 | 0x09
@@ -542,7 +543,7 @@ static int lvts_sensor_init(struct device *dev, struct lvts_ctrl *lvts_ctrl,
* <-----mcu-tc#2-----> <-----sensor#4-----> <-----sensor#5-----> <-----sensor#6-----> <-----sensor#7----->
* 0x13 | 0x14 | 0x15 | 0x16 | 0x17 | 0x18 | 0x19 | 0x1A | 0x1B | 0x1C | 0x1D | 0x1E | 0x1F | 0x20 | 0x21
*
- * stream index map for AP Domain :
+ * Stream index map for AP Domain mt8195 :
*
* <-----ap--tc#0-----> <-----sensor#0-----> <-----sensor#1----->
* 0x22 | 0x23 | 0x24 | 0x25 | 0x26 | 0x27 | 0x28 | 0x29 | 0x2A
@@ -556,6 +557,32 @@ static int lvts_sensor_init(struct device *dev, struct lvts_ctrl *lvts_ctrl,
* <-----ap--tc#3-----> <-----sensor#7-----> <-----sensor#8----->
* 0x40 | 0x41 | 0x42 | 0x43 | 0x44 | 0x45 | 0x46 | 0x47 | 0x48
*
+ * MT8192 :
+ * Stream index map for MCU Domain mt8192 :
+ *
+ * <-----mcu-tc#0-----> <-----sensor#0-----> <-----sensor#1----->
+ * 0x01 | 0x02 | 0x03 | 0x04 | 0x05 | 0x06 | 0x07 | 0x08 | 0x09 | 0x0A | 0x0B
+ *
+ * <-----sensor#2-----> <-----sensor#3----->
+ * 0x0C | 0x0D | 0x0E | 0x0F | 0x10 | 0x11 | 0x12 | 0x13
+ *
+ * <-----sensor#4-----> <-----sensor#5-----> <-----sensor#6-----> <-----sensor#7----->
+ * 0x14 | 0x15 | 0x16 | 0x17 | 0x18 | 0x19 | 0x1A | 0x1B | 0x1C | 0x1D | 0x1E | 0x1F | 0x20 | 0x21 | 0x22 | 0x23
+ *
+ * Stream index map for AP Domain mt8192 :
+ *
+ * <-----sensor#0-----> <-----sensor#1----->
+ * 0x24 | 0x25 | 0x26 | 0x27 | 0x28 | 0x29 | 0x2A | 0x2B
+ *
+ * <-----sensor#2-----> <-----sensor#3----->
+ * 0x2C | 0x2D | 0x2E | 0x2F | 0x30 | 0x31 | 0x32 | 0x33
+ *
+ * <-----sensor#4-----> <-----sensor#5----->
+ * 0x34 | 0x35 | 0x36 | 0x37 | 0x38 | 0x39 | 0x3A | 0x3B
+ *
+ * <-----sensor#6-----> <-----sensor#7-----> <-----sensor#8----->
+ * 0x3C | 0x3D | 0x3E | 0x3F | 0x40 | 0x41 | 0x42 | 0x43 | 0x44 | 0x45 | 0x46 | 0x47
+ *
* The data description gives the offset of the calibration data in
* this bytes stream for each sensor.
*/
--
2.41.0.rc2

2023-05-30 20:02:21

[permalink] [raw]

Subject: [PATCH v4 4/5] arm64: dts: mediatek: mt8192: Add thermal nodes and thermal zones

From: Balsam CHIHI <[email protected]>

Add thermal nodes and thermal zones for the mt8192.
The mt8192 SoC has several hotspots around the CPUs.
Specify the targeted temperature threshold to apply the mitigation
and define the associated cooling devices.

Signed-off-by: Balsam CHIHI <[email protected]>
Reviewed-by: Nícolas F. R. A. Prado <[email protected]>
[[email protected]: cosmetic changes, reduce lvts_ap size]
Signed-off-by: Bernhard Rosenkränzer <[email protected]>
---
arch/arm64/boot/dts/mediatek/mt8192.dtsi | 454 +++++++++++++++++++++++
1 file changed, 454 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
index 65bc8b4046211..82d6629e38c26 100644
--- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
@@ -14,6 +14,8 @@
#include <dt-bindings/phy/phy.h>
#include <dt-bindings/power/mt8192-power.h>
#include <dt-bindings/reset/mt8192-resets.h>
+#include <dt-bindings/thermal/thermal.h>
+#include <dt-bindings/thermal/mediatek,lvts-thermal.h>

/ {
compatible = "mediatek,mt8192";
@@ -71,6 +73,7 @@ cpu0: cpu@0 {
d-cache-sets = <128>;
next-level-cache = <&l2_0>;
capacity-dmips-mhz = <530>;
+ #cooling-cells = <2>;
};

cpu1: cpu@100 {
@@ -88,6 +91,7 @@ cpu1: cpu@100 {
d-cache-sets = <128>;
next-level-cache = <&l2_0>;
capacity-dmips-mhz = <530>;
+ #cooling-cells = <2>;
};

cpu2: cpu@200 {
@@ -105,6 +109,7 @@ cpu2: cpu@200 {
d-cache-sets = <128>;
next-level-cache = <&l2_0>;
capacity-dmips-mhz = <530>;
+ #cooling-cells = <2>;
};

cpu3: cpu@300 {
@@ -122,6 +127,7 @@ cpu3: cpu@300 {
d-cache-sets = <128>;
next-level-cache = <&l2_0>;
capacity-dmips-mhz = <530>;
+ #cooling-cells = <2>;
};

cpu4: cpu@400 {
@@ -139,6 +145,7 @@ cpu4: cpu@400 {
d-cache-sets = <256>;
next-level-cache = <&l2_1>;
capacity-dmips-mhz = <1024>;
+ #cooling-cells = <2>;
};

cpu5: cpu@500 {
@@ -156,6 +163,7 @@ cpu5: cpu@500 {
d-cache-sets = <256>;
next-level-cache = <&l2_1>;
capacity-dmips-mhz = <1024>;
+ #cooling-cells = <2>;
};

cpu6: cpu@600 {
@@ -173,6 +181,7 @@ cpu6: cpu@600 {
d-cache-sets = <256>;
next-level-cache = <&l2_1>;
capacity-dmips-mhz = <1024>;
+ #cooling-cells = <2>;
};

cpu7: cpu@700 {
@@ -190,6 +199,7 @@ cpu7: cpu@700 {
d-cache-sets = <256>;
next-level-cache = <&l2_1>;
capacity-dmips-mhz = <1024>;
+ #cooling-cells = <2>;
};

cpu-map {
@@ -775,6 +785,17 @@ spi0: spi@1100a000 {
status = "disabled";
};

+ lvts_ap: thermal-sensor@1100b000 {
+ compatible = "mediatek,mt8192-lvts-ap";
+ reg = <0 0x1100b000 0 0xc00>;
+ interrupts = <GIC_SPI 169 IRQ_TYPE_LEVEL_HIGH 0>;
+ clocks = <&infracfg CLK_INFRA_THERM>;
+ resets = <&infracfg MT8192_INFRA_RST0_THERM_CTRL_SWRST>;
+ nvmem-cells = <&lvts_e_data1>;
+ nvmem-cell-names = "lvts-calib-data-1";
+ #thermal-sensor-cells = <1>;
+ };
+
pwm0: pwm@1100e000 {
compatible = "mediatek,mt8183-disp-pwm";
reg = <0 0x1100e000 0 0x1000>;
@@ -1101,6 +1122,17 @@ nor_flash: spi@11234000 {
status = "disabled";
};

+ lvts_mcu: thermal-sensor@11278000 {
+ compatible = "mediatek,mt8192-lvts-mcu";
+ reg = <0 0x11278000 0 0x1000>;
+ interrupts = <GIC_SPI 170 IRQ_TYPE_LEVEL_HIGH 0>;
+ clocks = <&infracfg CLK_INFRA_THERM>;
+ resets = <&infracfg MT8192_INFRA_RST4_THERM_CTRL_MCU_SWRST>;
+ nvmem-cells = <&lvts_e_data1>;
+ nvmem-cell-names = "lvts-calib-data-1";
+ #thermal-sensor-cells = <1>;
+ };
+
efuse: efuse@11c10000 {
compatible = "mediatek,mt8192-efuse", "mediatek,efuse";
reg = <0 0x11c10000 0 0x1000>;
@@ -1827,4 +1859,426 @@ larb2: larb@1f002000 {
power-domains = <&spm MT8192_POWER_DOMAIN_MDP>;
};
};
+
+ thermal_zones: thermal-zones {
+ cpu0-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts_mcu MT8192_MCU_LITTLE_CPU0>;
+
+ trips {
+ cpu0_alert: trip-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ cpu0_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+
+ cooling-maps {
+ map0 {
+ trip = <&cpu0_alert>;
+ cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+ };
+ };
+ };
+
+ cpu1-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts_mcu MT8192_MCU_LITTLE_CPU1>;
+
+ trips {
+ cpu1_alert: trip-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ cpu1_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+
+ cooling-maps {
+ map0 {
+ trip = <&cpu1_alert>;
+ cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+ };
+ };
+ };
+
+ cpu2-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts_mcu MT8192_MCU_LITTLE_CPU2>;
+
+ trips {
+ cpu2_alert: trip-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ cpu2_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+
+ cooling-maps {
+ map0 {
+ trip = <&cpu2_alert>;
+ cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+ };
+ };
+ };
+
+ cpu3-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts_mcu MT8192_MCU_LITTLE_CPU3>;
+
+ trips {
+ cpu3_alert: trip-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ cpu3_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+
+ cooling-maps {
+ map0 {
+ trip = <&cpu3_alert>;
+ cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+ };
+ };
+ };
+
+ cpu4-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts_mcu MT8192_MCU_BIG_CPU0>;
+
+ trips {
+ cpu4_alert: trip-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ cpu4_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+
+ cooling-maps {
+ map0 {
+ trip = <&cpu4_alert>;
+ cooling-device = <&cpu4 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu5 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu6 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu7 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+ };
+ };
+ };
+
+ cpu5-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts_mcu MT8192_MCU_BIG_CPU1>;
+
+ trips {
+ cpu5_alert: trip-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ cpu5_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+
+ cooling-maps {
+ map0 {
+ trip = <&cpu5_alert>;
+ cooling-device = <&cpu4 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu5 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu6 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu7 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+ };
+ };
+ };
+
+ cpu6-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts_mcu MT8192_MCU_BIG_CPU2>;
+
+ trips {
+ cpu6_alert: trip-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ cpu6_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+
+ cooling-maps {
+ map0 {
+ trip = <&cpu6_alert>;
+ cooling-device = <&cpu4 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu5 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu6 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu7 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+ };
+ };
+ };
+
+ cpu7-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts_mcu MT8192_MCU_BIG_CPU3>;
+
+ trips {
+ cpu7_alert: trip-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ cpu7_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+
+ cooling-maps {
+ map0 {
+ trip = <&cpu7_alert>;
+ cooling-device = <&cpu4 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu5 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu6 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu7 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+ };
+ };
+ };
+
+ vpu0-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts_ap MT8192_AP_VPU0>;
+
+ trips {
+ vpu0_alert: trip-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ vpu0_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+ };
+
+ vpu1-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts_ap MT8192_AP_VPU1>;
+
+ trips {
+ vpu1_alert: trip-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ vpu1_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+ };
+
+ gpu0-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts_ap MT8192_AP_GPU0>;
+
+ trips {
+ gpu0_alert: trip-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ gpu0_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+ };
+
+ gpu1-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts_ap MT8192_AP_GPU1>;
+
+ trips {
+ gpu1_alert: trip-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ gpu1_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+ };
+
+ infra-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts_ap MT8192_AP_INFRA>;
+
+ trips {
+ infra_alert: trip-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ infra_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+ };
+
+ cam-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts_ap MT8192_AP_CAM>;
+
+ trips {
+ cam_alert: trip-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ cam_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+ };
+
+ md0-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts_ap MT8192_AP_MD0>;
+
+ trips {
+ md0_alert: trip-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ md0_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+ };
+
+ md1-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts_ap MT8192_AP_MD1>;
+
+ trips {
+ md1_alert: trip-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ md1_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+ };
+
+ md2-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts_ap MT8192_AP_MD2>;
+
+ trips {
+ md2_alert: trip-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ md2_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+ };
+ };
};
--
2.41.0.rc2

2023-05-30 20:02:37

[permalink] [raw]

Subject: [PATCH v4 1/5] dt-bindings: thermal: mediatek: Add LVTS thermal controller definition for mt8192

From: Balsam CHIHI <[email protected]>

Add LVTS thermal controller definition for MT8192.

Signed-off-by: Balsam CHIHI <[email protected]>
Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
Acked-by: Krzysztof Kozlowski <[email protected]>
Signed-off-by: Bernhard Rosenkränzer <[email protected]>
Reviewed-by: Matthias Brugger <[email protected]>
---
.../thermal/mediatek,lvts-thermal.h | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)

diff --git a/include/dt-bindings/thermal/mediatek,lvts-thermal.h b/include/dt-bindings/thermal/mediatek,lvts-thermal.h
index 8fa5a46675c46..5e9eb62174268 100644
--- a/include/dt-bindings/thermal/mediatek,lvts-thermal.h
+++ b/include/dt-bindings/thermal/mediatek,lvts-thermal.h
@@ -26,4 +26,23 @@
#define MT8195_AP_CAM0 15
#define MT8195_AP_CAM1 16

+#define MT8192_MCU_BIG_CPU0 0
+#define MT8192_MCU_BIG_CPU1 1
+#define MT8192_MCU_BIG_CPU2 2
+#define MT8192_MCU_BIG_CPU3 3
+#define MT8192_MCU_LITTLE_CPU0 4
+#define MT8192_MCU_LITTLE_CPU1 5
+#define MT8192_MCU_LITTLE_CPU2 6
+#define MT8192_MCU_LITTLE_CPU3 7
+
+#define MT8192_AP_VPU0 8
+#define MT8192_AP_VPU1 9
+#define MT8192_AP_GPU0 10
+#define MT8192_AP_GPU1 11
+#define MT8192_AP_INFRA 12
+#define MT8192_AP_CAM 13
+#define MT8192_AP_MD0 14
+#define MT8192_AP_MD1 15
+#define MT8192_AP_MD2 16
+
#endif /* __MEDIATEK_LVTS_DT_H */
--
2.41.0.rc2

2023-05-30 20:05:00

[permalink] [raw]

Subject: [PATCH v4 3/5] thermal/drivers/mediatek/lvts_thermal: Add mt8192 support

From: Balsam CHIHI <[email protected]>

Add LVTS Driver support for MT8192.

Co-developed-by : Nícolas F. R. A. Prado <[email protected]>
Signed-off-by: Nícolas F. R. A. Prado <[email protected]>
Signed-off-by: Balsam CHIHI <[email protected]>
Reviewed-by: Nícolas F. R. A. Prado <[email protected]>
Signed-off-by: Bernhard Rosenkränzer <[email protected]>
Reviewed-by: Matthias Brugger <[email protected]>
---
drivers/thermal/mediatek/lvts_thermal.c | 95 +++++++++++++++++++++++++
1 file changed, 95 insertions(+)

diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
index 5ea8a9d569ea6..d5e5214784ece 100644
--- a/drivers/thermal/mediatek/lvts_thermal.c
+++ b/drivers/thermal/mediatek/lvts_thermal.c
@@ -80,6 +80,7 @@
#define LVTS_MSR_FILTERED_MODE 1

#define LVTS_HW_SHUTDOWN_MT8195 105000
+#define LVTS_HW_SHUTDOWN_MT8192 105000

static int golden_temp = LVTS_GOLDEN_TEMP_DEFAULT;
static int coeff_b = LVTS_COEFF_B;
@@ -1280,6 +1281,88 @@ static const struct lvts_ctrl_data mt8195_lvts_ap_data_ctrl[] = {
}
};

+static const struct lvts_ctrl_data mt8192_lvts_mcu_data_ctrl[] = {
+ {
+ .cal_offset = { 0x04, 0x08 },
+ .lvts_sensor = {
+ { .dt_id = MT8192_MCU_BIG_CPU0 },
+ { .dt_id = MT8192_MCU_BIG_CPU1 }
+ },
+ .num_lvts_sensor = 2,
+ .offset = 0x0,
+ .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
+ .mode = LVTS_MSR_FILTERED_MODE,
+ },
+ {
+ .cal_offset = { 0x0c, 0x10 },
+ .lvts_sensor = {
+ { .dt_id = MT8192_MCU_BIG_CPU2 },
+ { .dt_id = MT8192_MCU_BIG_CPU3 }
+ },
+ .num_lvts_sensor = 2,
+ .offset = 0x100,
+ .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
+ .mode = LVTS_MSR_FILTERED_MODE,
+ },
+ {
+ .cal_offset = { 0x14, 0x18, 0x1c, 0x20 },
+ .lvts_sensor = {
+ { .dt_id = MT8192_MCU_LITTLE_CPU0 },
+ { .dt_id = MT8192_MCU_LITTLE_CPU1 },
+ { .dt_id = MT8192_MCU_LITTLE_CPU2 },
+ { .dt_id = MT8192_MCU_LITTLE_CPU3 }
+ },
+ .num_lvts_sensor = 4,
+ .offset = 0x200,
+ .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
+ .mode = LVTS_MSR_FILTERED_MODE,
+ }
+};
+
+static const struct lvts_ctrl_data mt8192_lvts_ap_data_ctrl[] = {
+ {
+ .cal_offset = { 0x24, 0x28 },
+ .lvts_sensor = {
+ { .dt_id = MT8192_AP_VPU0 },
+ { .dt_id = MT8192_AP_VPU1 }
+ },
+ .num_lvts_sensor = 2,
+ .offset = 0x0,
+ .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
+ },
+ {
+ .cal_offset = { 0x2c, 0x30 },
+ .lvts_sensor = {
+ { .dt_id = MT8192_AP_GPU0 },
+ { .dt_id = MT8192_AP_GPU1 }
+ },
+ .num_lvts_sensor = 2,
+ .offset = 0x100,
+ .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
+ },
+ {
+ .cal_offset = { 0x34, 0x38 },
+ .lvts_sensor = {
+ { .dt_id = MT8192_AP_INFRA },
+ { .dt_id = MT8192_AP_CAM },
+ },
+ .num_lvts_sensor = 2,
+ .offset = 0x200,
+ .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
+ },
+ {
+ .cal_offset = { 0x3c, 0x40, 0x44 },
+ .lvts_sensor = {
+ { .dt_id = MT8192_AP_MD0 },
+ { .dt_id = MT8192_AP_MD1 },
+ { .dt_id = MT8192_AP_MD2 }
+ },
+ .num_lvts_sensor = 3,
+ .offset = 0x300,
+ .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
+ }
+};
+
static const struct lvts_data mt8195_lvts_mcu_data = {
.lvts_ctrl = mt8195_lvts_mcu_data_ctrl,
.num_lvts_ctrl = ARRAY_SIZE(mt8195_lvts_mcu_data_ctrl),
@@ -1290,9 +1373,21 @@ static const struct lvts_data mt8195_lvts_ap_data = {
.num_lvts_ctrl = ARRAY_SIZE(mt8195_lvts_ap_data_ctrl),
};

+static const struct lvts_data mt8192_lvts_mcu_data = {
+ .lvts_ctrl = mt8192_lvts_mcu_data_ctrl,
+ .num_lvts_ctrl = ARRAY_SIZE(mt8192_lvts_mcu_data_ctrl),
+};
+
+static const struct lvts_data mt8192_lvts_ap_data = {
+ .lvts_ctrl = mt8192_lvts_ap_data_ctrl,
+ .num_lvts_ctrl = ARRAY_SIZE(mt8192_lvts_ap_data_ctrl),
+};
+
static const struct of_device_id lvts_of_match[] = {
{ .compatible = "mediatek,mt8195-lvts-mcu", .data = &mt8195_lvts_mcu_data },
{ .compatible = "mediatek,mt8195-lvts-ap", .data = &mt8195_lvts_ap_data },
+ { .compatible = "mediatek,mt8192-lvts-mcu", .data = &mt8192_lvts_mcu_data },
+ { .compatible = "mediatek,mt8192-lvts-ap", .data = &mt8192_lvts_ap_data },
{},
};
MODULE_DEVICE_TABLE(of, lvts_of_match);
--
2.41.0.rc2

2023-05-31 05:23:11

[permalink] [raw]

Subject: Re: [PATCH v4 0/5] Add LVTS support for mt8192

On Wed, May 31, 2023 at 3:51 AM Bernhard Rosenkränzer <[email protected]> wrote:
>
> From: Balsam CHIHI <[email protected]>
>
> Add full LVTS support (MCU thermal domain + AP thermal domain) to MediaTek MT8192 SoC.
> Also, add Suspend and Resume support to LVTS Driver (all SoCs),
> and update the documentation that describes the Calibration Data Offsets.
>
> Changelog:
> v4 :
> - Shrink the lvts_ap thermal sensor I/O range to 0xc00 to make
> room for SVS support, pointed out by
> AngeloGioacchino Del Regno <[email protected]>
>
> v3 :
> - Rebased :
> base-commit: 6a3d37b4d885129561e1cef361216f00472f7d2e
> - Fix issues in v2 pointed out by Nícolas F. R. A. Prado <[email protected]>:
> Use filtered mode to make sure threshold interrupts are triggered,

I'm seeing sensor readout (either through sysfs/thermal/<x>/temp or hwmon)
fail frequently on MT8192. If I run `sensors` (lm-sensors), at least a couple
of the LVTS sensors would be N/A. Not sure if this is related to this change.

ChenYu

> protocol documentation, cosmetics
> - I ([email protected]) will be taking care of this patchset
> from now on, since Balsam has left BayLibre. Thanks for
> getting it almost ready, Balsam!
>
> v2 :
> - Based on top of thermal/linux-next :
> base-commit: 7ac82227ee046f8234471de4c12a40b8c2d3ddcc
> - Squash "add thermal zones and thermal nodes" and
> "add temperature mitigation threshold" commits together to form
> "arm64: dts: mediatek: mt8192: Add thermal nodes and thermal zones" commit.
> - Add Suspend and Resume support to LVTS Driver.
> - Update Calibration Data documentation.
> - Fix calibration data offsets for mt8192
> (Thanks to "Chen-Yu Tsai" and "Nícolas F. R. A. Prado").
> https://lore.kernel.org/all/[email protected]/
> Tested-by: Chen-Yu Tsai <[email protected]>
>
> v1 :
> - The initial series "Add LVTS support for mt8192" :
> "https://lore.kernel.org/all/[email protected]/".
>
> Balsam CHIHI (5):
> dt-bindings: thermal: mediatek: Add LVTS thermal controller definition
> for mt8192
> thermal/drivers/mediatek/lvts_thermal: Add suspend and resume
> thermal/drivers/mediatek/lvts_thermal: Add mt8192 support
> arm64: dts: mediatek: mt8192: Add thermal nodes and thermal zones
> thermal/drivers/mediatek/lvts_thermal: Update calibration data
> documentation
>
> arch/arm64/boot/dts/mediatek/mt8192.dtsi | 454 ++++++++++++++++++
> drivers/thermal/mediatek/lvts_thermal.c | 160 +++++-
> .../thermal/mediatek,lvts-thermal.h | 19 +
> 3 files changed, 631 insertions(+), 2 deletions(-)
>
> base-commit: 8c33787278ca8db73ad7d23f932c8c39b9f6e543
> --
> 2.41.0.rc2
>

2023-05-31 08:20:05

[permalink] [raw]

Subject: Re: [PATCH v4 2/5] thermal/drivers/mediatek/lvts_thermal: Add suspend and resume

Il 30/05/23 21:51, Bernhard Rosenkränzer ha scritto:
> From: Balsam CHIHI <[email protected]>
>
> Add suspend and resume support to LVTS driver.
>
> Signed-off-by: Balsam CHIHI <[email protected]>
> Signed-off-by: Bernhard Rosenkränzer <[email protected]>
> Reviewed-by: Matthias Brugger <[email protected]>
> ---
> drivers/thermal/mediatek/lvts_thermal.c | 34 +++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
> index d0a3f95b7884b..5ea8a9d569ea6 100644
> --- a/drivers/thermal/mediatek/lvts_thermal.c
> +++ b/drivers/thermal/mediatek/lvts_thermal.c
> @@ -1169,6 +1169,38 @@ static int lvts_remove(struct platform_device *pdev)
> return 0;
> }
>
> +static int lvts_suspend(struct platform_device *pdev, pm_message_t state)
> +{
> + struct lvts_domain *lvts_td;
> + int i;
> +
> + lvts_td = platform_get_drvdata(pdev);
> +
> + for (i = 0; i < lvts_td->num_lvts_ctrl; i++)
> + lvts_ctrl_set_enable(&lvts_td->lvts_ctrl[i], false);
> +
> + clk_disable_unprepare(lvts_td->clk);
> +
> + return 0;
> +}
> +
> +static int lvts_resume(struct platform_device *pdev)
> +{
> + struct lvts_domain *lvts_td;
> + int i, ret;
> +
> + lvts_td = platform_get_drvdata(pdev);
> +
> + ret = clk_prepare_enable(lvts_td->clk);
> + if (ret)
> + return ret;
> +
> + for (i = 0; i < lvts_td->num_lvts_ctrl; i++)
> + lvts_ctrl_set_enable(&lvts_td->lvts_ctrl[i], true);
> +
> + return 0;
> +}
> +
> static const struct lvts_ctrl_data mt8195_lvts_mcu_data_ctrl[] = {
> {
> .cal_offset = { 0x04, 0x07 },
> @@ -1268,6 +1300,8 @@ MODULE_DEVICE_TABLE(of, lvts_of_match);
> static struct platform_driver lvts_driver = {
> .probe = lvts_probe,
> .remove = lvts_remove,
> + .suspend = lvts_suspend,

Should we do that in noirq handlers?
We're risking to miss a thermal interrupt.

Regards,
Angelo

> + .resume = lvts_resume,
> .driver = {
> .name = "mtk-lvts-thermal",
> .of_match_table = lvts_of_match,

2023-05-31 08:20:19

[permalink] [raw]

Subject: Re: [PATCH v4 4/5] arm64: dts: mediatek: mt8192: Add thermal nodes and thermal zones

On Wed, May 31, 2023 at 3:51 AM Bernhard Rosenkränzer <[email protected]> wrote:
>
> From: Balsam CHIHI <[email protected]>
>
> Add thermal nodes and thermal zones for the mt8192.
> The mt8192 SoC has several hotspots around the CPUs.
> Specify the targeted temperature threshold to apply the mitigation
> and define the associated cooling devices.
>
> Signed-off-by: Balsam CHIHI <[email protected]>
> Reviewed-by: Nícolas F. R. A. Prado <[email protected]>
> [[email protected]: cosmetic changes, reduce lvts_ap size]
> Signed-off-by: Bernhard Rosenkränzer <[email protected]>
> ---
> arch/arm64/boot/dts/mediatek/mt8192.dtsi | 454 +++++++++++++++++++++++
> 1 file changed, 454 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
> index 65bc8b4046211..82d6629e38c26 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
> @@ -14,6 +14,8 @@
> #include <dt-bindings/phy/phy.h>
> #include <dt-bindings/power/mt8192-power.h>
> #include <dt-bindings/reset/mt8192-resets.h>
> +#include <dt-bindings/thermal/thermal.h>
> +#include <dt-bindings/thermal/mediatek,lvts-thermal.h>
>
> / {
> compatible = "mediatek,mt8192";
> @@ -71,6 +73,7 @@ cpu0: cpu@0 {
> d-cache-sets = <128>;
> next-level-cache = <&l2_0>;
> capacity-dmips-mhz = <530>;
> + #cooling-cells = <2>;

FYI these changes (for all the CPU cores) will conflict with the cpufreq
patch that Matthias just merged.

ChenYu

2023-05-31 08:24:46

[permalink] [raw]

Subject: Re: [PATCH v4 4/5] arm64: dts: mediatek: mt8192: Add thermal nodes and thermal zones

Il 30/05/23 21:51, Bernhard Rosenkränzer ha scritto:
> From: Balsam CHIHI <[email protected]>
>
> Add thermal nodes and thermal zones for the mt8192.
> The mt8192 SoC has several hotspots around the CPUs.
> Specify the targeted temperature threshold to apply the mitigation
> and define the associated cooling devices.
>
> Signed-off-by: Balsam CHIHI <[email protected]>
> Reviewed-by: Nícolas F. R. A. Prado <[email protected]>
> [[email protected]: cosmetic changes, reduce lvts_ap size]
> Signed-off-by: Bernhard Rosenkränzer <[email protected]>

Reviewed-by: AngeloGioacchino Del Regno <[email protected]>

2023-05-31 08:24:58

[permalink] [raw]

Subject: Re: [PATCH v4 3/5] thermal/drivers/mediatek/lvts_thermal: Add mt8192 support

Il 30/05/23 21:51, Bernhard Rosenkränzer ha scritto:
> From: Balsam CHIHI <[email protected]>
>
> Add LVTS Driver support for MT8192.
>
> Co-developed-by : Nícolas F. R. A. Prado <[email protected]>
> Signed-off-by: Nícolas F. R. A. Prado <[email protected]>
> Signed-off-by: Balsam CHIHI <[email protected]>
> Reviewed-by: Nícolas F. R. A. Prado <[email protected]>
> Signed-off-by: Bernhard Rosenkränzer <[email protected]>
> Reviewed-by: Matthias Brugger <[email protected]>
> ---
> drivers/thermal/mediatek/lvts_thermal.c | 95 +++++++++++++++++++++++++
> 1 file changed, 95 insertions(+)
>
> diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
> index 5ea8a9d569ea6..d5e5214784ece 100644
> --- a/drivers/thermal/mediatek/lvts_thermal.c
> +++ b/drivers/thermal/mediatek/lvts_thermal.c
> @@ -80,6 +80,7 @@
> #define LVTS_MSR_FILTERED_MODE 1
>
> #define LVTS_HW_SHUTDOWN_MT8195 105000
> +#define LVTS_HW_SHUTDOWN_MT8192 105000
>
> static int golden_temp = LVTS_GOLDEN_TEMP_DEFAULT;
> static int coeff_b = LVTS_COEFF_B;
> @@ -1280,6 +1281,88 @@ static const struct lvts_ctrl_data mt8195_lvts_ap_data_ctrl[] = {
> }
> };
>
> +static const struct lvts_ctrl_data mt8192_lvts_mcu_data_ctrl[] = {
> + {
> + .cal_offset = { 0x04, 0x08 },
> + .lvts_sensor = {
> + { .dt_id = MT8192_MCU_BIG_CPU0 },
> + { .dt_id = MT8192_MCU_BIG_CPU1 }
> + },
> + .num_lvts_sensor = 2,
> + .offset = 0x0,
> + .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
> + .mode = LVTS_MSR_FILTERED_MODE,
> + },
> + {
> + .cal_offset = { 0x0c, 0x10 },
> + .lvts_sensor = {
> + { .dt_id = MT8192_MCU_BIG_CPU2 },
> + { .dt_id = MT8192_MCU_BIG_CPU3 }
> + },
> + .num_lvts_sensor = 2,
> + .offset = 0x100,
> + .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
> + .mode = LVTS_MSR_FILTERED_MODE,
> + },
> + {
> + .cal_offset = { 0x14, 0x18, 0x1c, 0x20 },
> + .lvts_sensor = {
> + { .dt_id = MT8192_MCU_LITTLE_CPU0 },
> + { .dt_id = MT8192_MCU_LITTLE_CPU1 },
> + { .dt_id = MT8192_MCU_LITTLE_CPU2 },
> + { .dt_id = MT8192_MCU_LITTLE_CPU3 }
> + },
> + .num_lvts_sensor = 4,
> + .offset = 0x200,
> + .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
> + .mode = LVTS_MSR_FILTERED_MODE,
> + }
> +};
> +
> +static const struct lvts_ctrl_data mt8192_lvts_ap_data_ctrl[] = {
> + {
> + .cal_offset = { 0x24, 0x28 },
> + .lvts_sensor = {
> + { .dt_id = MT8192_AP_VPU0 },
> + { .dt_id = MT8192_AP_VPU1 }
> + },
> + .num_lvts_sensor = 2,
> + .offset = 0x0,
> + .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
> + },
> + {
> + .cal_offset = { 0x2c, 0x30 },
> + .lvts_sensor = {
> + { .dt_id = MT8192_AP_GPU0 },
> + { .dt_id = MT8192_AP_GPU1 }
> + },
> + .num_lvts_sensor = 2,
> + .offset = 0x100,
> + .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,

I'm unable to get readings for the GPU sensors, didn't really check
the others; `cat (xxxx)` gives a resource not available error, is that
the same for you?!

Regards,
Angelo

> + },
> + {
> + .cal_offset = { 0x34, 0x38 },
> + .lvts_sensor = {
> + { .dt_id = MT8192_AP_INFRA },
> + { .dt_id = MT8192_AP_CAM },
> + },
> + .num_lvts_sensor = 2,
> + .offset = 0x200,
> + .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
> + },
> + {
> + .cal_offset = { 0x3c, 0x40, 0x44 },
> + .lvts_sensor = {
> + { .dt_id = MT8192_AP_MD0 },
> + { .dt_id = MT8192_AP_MD1 },
> + { .dt_id = MT8192_AP_MD2 }
> + },
> + .num_lvts_sensor = 3,
> + .offset = 0x300,
> + .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
> + }
> +};
> +
> static const struct lvts_data mt8195_lvts_mcu_data = {
> .lvts_ctrl = mt8195_lvts_mcu_data_ctrl,
> .num_lvts_ctrl = ARRAY_SIZE(mt8195_lvts_mcu_data_ctrl),
> @@ -1290,9 +1373,21 @@ static const struct lvts_data mt8195_lvts_ap_data = {
> .num_lvts_ctrl = ARRAY_SIZE(mt8195_lvts_ap_data_ctrl),
> };
>
> +static const struct lvts_data mt8192_lvts_mcu_data = {
> + .lvts_ctrl = mt8192_lvts_mcu_data_ctrl,
> + .num_lvts_ctrl = ARRAY_SIZE(mt8192_lvts_mcu_data_ctrl),
> +};
> +
> +static const struct lvts_data mt8192_lvts_ap_data = {
> + .lvts_ctrl = mt8192_lvts_ap_data_ctrl,
> + .num_lvts_ctrl = ARRAY_SIZE(mt8192_lvts_ap_data_ctrl),
> +};
> +
> static const struct of_device_id lvts_of_match[] = {
> { .compatible = "mediatek,mt8195-lvts-mcu", .data = &mt8195_lvts_mcu_data },
> { .compatible = "mediatek,mt8195-lvts-ap", .data = &mt8195_lvts_ap_data },
> + { .compatible = "mediatek,mt8192-lvts-mcu", .data = &mt8192_lvts_mcu_data },
> + { .compatible = "mediatek,mt8192-lvts-ap", .data = &mt8192_lvts_ap_data },
> {},
> };
> MODULE_DEVICE_TABLE(of, lvts_of_match);

2023-06-01 17:16:12

by Nícolas F. R. A. Prado

[permalink] [raw]

Subject: Re: [PATCH v4 0/5] Add LVTS support for mt8192

On Wed, May 31, 2023 at 12:49:43PM +0800, Chen-Yu Tsai wrote:
> On Wed, May 31, 2023 at 3:51 AM Bernhard Rosenkränzer <[email protected]> wrote:
> >
> > From: Balsam CHIHI <[email protected]>
> >
> > Add full LVTS support (MCU thermal domain + AP thermal domain) to MediaTek MT8192 SoC.
> > Also, add Suspend and Resume support to LVTS Driver (all SoCs),
> > and update the documentation that describes the Calibration Data Offsets.
> >
> > Changelog:
> > v4 :
> > - Shrink the lvts_ap thermal sensor I/O range to 0xc00 to make
> > room for SVS support, pointed out by
> > AngeloGioacchino Del Regno <[email protected]>
> >
> > v3 :
> > - Rebased :
> > base-commit: 6a3d37b4d885129561e1cef361216f00472f7d2e
> > - Fix issues in v2 pointed out by Nícolas F. R. A. Prado <[email protected]>:
> > Use filtered mode to make sure threshold interrupts are triggered,
>
> I'm seeing sensor readout (either through sysfs/thermal/<x>/temp or hwmon)
> fail frequently on MT8192. If I run `sensors` (lm-sensors), at least a couple
> of the LVTS sensors would be N/A. Not sure if this is related to this change.

Yes, it is. Filtered mode has some delay associated with reading, meaning most
of the time the value isn't ready, while immediate mode is, well, pretty much
immediate and the read always succeeds.

For temperature monitoring, filtered mode should be used. It supports triggering
interrupts when crossing the thresholds. Immediate mode is meant for one-off
readings of the temperature. This is why I suggested using filtered mode.

As far as the thermal framework goes, it's ok that filtered mode doesn't always
return a value, as it will keep the old one. But of course, having the
temperature readout always work would be a desired improvement.

As for ways to achieve that, I think the intended way would be to enable the
interrupts that signal data ready on filtered mode (bits 19, 20, 21, 28), read
the temperature and cache it so it is always available when the get_temp()
callback is called. The issue with this is that it would cause *a lot* of
interrupts, which doesn't seem worth it.

Another option that comes to mind would be to enable immediate mode only during
the get_temp() callback, to immediately read a value, and return to filtered
mode at the end. That might work, but I haven't tried yet.

Thanks,
Nícolas

2023-06-05 08:22:14

[permalink] [raw]

Subject: Re: [PATCH v4 0/5] Add LVTS support for mt8192

Il 01/06/23 19:09, Nícolas F. R. A. Prado ha scritto:
> On Wed, May 31, 2023 at 12:49:43PM +0800, Chen-Yu Tsai wrote:
>> On Wed, May 31, 2023 at 3:51 AM Bernhard Rosenkränzer <[email protected]> wrote:
>>>
>>> From: Balsam CHIHI <[email protected]>
>>>
>>> Add full LVTS support (MCU thermal domain + AP thermal domain) to MediaTek MT8192 SoC.
>>> Also, add Suspend and Resume support to LVTS Driver (all SoCs),
>>> and update the documentation that describes the Calibration Data Offsets.
>>>
>>> Changelog:
>>> v4 :
>>> - Shrink the lvts_ap thermal sensor I/O range to 0xc00 to make
>>> room for SVS support, pointed out by
>>> AngeloGioacchino Del Regno <[email protected]>
>>>
>>> v3 :
>>> - Rebased :
>>> base-commit: 6a3d37b4d885129561e1cef361216f00472f7d2e
>>> - Fix issues in v2 pointed out by Nícolas F. R. A. Prado <[email protected]>:
>>> Use filtered mode to make sure threshold interrupts are triggered,
>>
>> I'm seeing sensor readout (either through sysfs/thermal/<x>/temp or hwmon)
>> fail frequently on MT8192. If I run `sensors` (lm-sensors), at least a couple
>> of the LVTS sensors would be N/A. Not sure if this is related to this change.
>
> Yes, it is. Filtered mode has some delay associated with reading, meaning most
> of the time the value isn't ready, while immediate mode is, well, pretty much
> immediate and the read always succeeds.
>
> For temperature monitoring, filtered mode should be used. It supports triggering
> interrupts when crossing the thresholds. Immediate mode is meant for one-off
> readings of the temperature. This is why I suggested using filtered mode.
>
> As far as the thermal framework goes, it's ok that filtered mode doesn't always
> return a value, as it will keep the old one. But of course, having the
> temperature readout always work would be a desired improvement.
>
> As for ways to achieve that, I think the intended way would be to enable the
> interrupts that signal data ready on filtered mode (bits 19, 20, 21, 28), read
> the temperature and cache it so it is always available when the get_temp()
> callback is called. The issue with this is that it would cause *a lot* of
> interrupts, which doesn't seem worth it.
>
> Another option that comes to mind would be to enable immediate mode only during
> the get_temp() callback, to immediately read a value, and return to filtered
> mode at the end. That might work, but I haven't tried yet.
>

The issue with keeping all as filtered mode comes when we want to add MediaTek
SVS functionality which, on most SoCs, is used only for the GPU (apart from the
MT8183 which has cpu+gpu svs).

It makes sense to cache the readings, but I'm concerned about possible
instabilities that we could get through the SVS voltage adjustment flows, as
that algorithm takes current IP temperature to shape the DVFS "V" curve; please
keep in mind that this concern is valid only if temperature readings get updated
"very slowly" (>100ms would be too slow).

So, point of the situation:
- Filtered mode, less than 100ms per temperature reading -> cache it, it's ok
- Filtered mode, more than 100ms per temp reading -> switch GPU to Immediate mode.

Your call.

Keep up the good work!
- Angelo

2023-06-08 09:48:07

by Daniel Lezcano

[permalink] [raw]

Subject: Re: [PATCH v4 0/5] Add LVTS support for mt8192

On 01/06/2023 19:09, Nícolas F. R. A. Prado wrote:
> On Wed, May 31, 2023 at 12:49:43PM +0800, Chen-Yu Tsai wrote:
>> On Wed, May 31, 2023 at 3:51 AM Bernhard Rosenkränzer <[email protected]> wrote:
>>>
>>> From: Balsam CHIHI <[email protected]>
>>>
>>> Add full LVTS support (MCU thermal domain + AP thermal domain) to MediaTek MT8192 SoC.
>>> Also, add Suspend and Resume support to LVTS Driver (all SoCs),
>>> and update the documentation that describes the Calibration Data Offsets.
>>>
>>> Changelog:
>>> v4 :
>>> - Shrink the lvts_ap thermal sensor I/O range to 0xc00 to make
>>> room for SVS support, pointed out by
>>> AngeloGioacchino Del Regno <[email protected]>
>>>
>>> v3 :
>>> - Rebased :
>>> base-commit: 6a3d37b4d885129561e1cef361216f00472f7d2e
>>> - Fix issues in v2 pointed out by Nícolas F. R. A. Prado <[email protected]>:
>>> Use filtered mode to make sure threshold interrupts are triggered,
>>
>> I'm seeing sensor readout (either through sysfs/thermal/<x>/temp or hwmon)
>> fail frequently on MT8192. If I run `sensors` (lm-sensors), at least a couple
>> of the LVTS sensors would be N/A. Not sure if this is related to this change.
>
> Yes, it is. Filtered mode has some delay associated with reading, meaning most
> of the time the value isn't ready, while immediate mode is, well, pretty much
> immediate and the read always succeeds.
>
> For temperature monitoring, filtered mode should be used. It supports triggering
> interrupts when crossing the thresholds. Immediate mode is meant for one-off
> readings of the temperature. This is why I suggested using filtered mode.
>
> As far as the thermal framework goes, it's ok that filtered mode doesn't always
> return a value, as it will keep the old one. But of course, having the
> temperature readout always work would be a desired improvement.
>
> As for ways to achieve that, I think the intended way would be to enable the
> interrupts that signal data ready on filtered mode (bits 19, 20, 21, 28), read
> the temperature and cache it so it is always available when the get_temp()
> callback is called. The issue with this is that it would cause *a lot* of
> interrupts, which doesn't seem worth it.
>
> Another option that comes to mind would be to enable immediate mode only during
> the get_temp() callback, to immediately read a value, and return to filtered
> mode at the end. That might work, but I haven't tried yet.

Why not understand why the filtered mode is unable to return temperature
values most of the time?

I tried with the filtered mode and I can see 90% of the time it is not
possible to read the temperature.

IIUC there are timings which can be setup, may be understand how to set
them up in order to read the temperature correctly?

Caching values, switching the mode or whatever is hackish :/

--
<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

2023-06-08 10:12:58

by Daniel Lezcano

[permalink] [raw]

Subject: Re: [PATCH v4 0/5] Add LVTS support for mt8192

On 01/06/2023 19:09, Nícolas F. R. A. Prado wrote:
> On Wed, May 31, 2023 at 12:49:43PM +0800, Chen-Yu Tsai wrote:
>> On Wed, May 31, 2023 at 3:51 AM Bernhard Rosenkränzer <[email protected]> wrote:
>>>
>>> From: Balsam CHIHI <[email protected]>
>>>
>>> Add full LVTS support (MCU thermal domain + AP thermal domain) to MediaTek MT8192 SoC.
>>> Also, add Suspend and Resume support to LVTS Driver (all SoCs),
>>> and update the documentation that describes the Calibration Data Offsets.
>>>
>>> Changelog:
>>> v4 :
>>> - Shrink the lvts_ap thermal sensor I/O range to 0xc00 to make
>>> room for SVS support, pointed out by
>>> AngeloGioacchino Del Regno <[email protected]>
>>>
>>> v3 :
>>> - Rebased :
>>> base-commit: 6a3d37b4d885129561e1cef361216f00472f7d2e
>>> - Fix issues in v2 pointed out by Nícolas F. R. A. Prado <[email protected]>:
>>> Use filtered mode to make sure threshold interrupts are triggered,
>>
>> I'm seeing sensor readout (either through sysfs/thermal/<x>/temp or hwmon)
>> fail frequently on MT8192. If I run `sensors` (lm-sensors), at least a couple
>> of the LVTS sensors would be N/A. Not sure if this is related to this change.
>
> Yes, it is. Filtered mode has some delay associated with reading, meaning most
> of the time the value isn't ready, while immediate mode is, well, pretty much
> immediate and the read always succeeds.
>
> For temperature monitoring, filtered mode should be used.

Why?

> As far as the thermal framework goes, it's ok that filtered mode doesn't always
> return a value, as it will keep the old one.

It is not by design but just the code not returning the error when
updating the thermal zone as it should so the side effect is giving the
illusion everything is all right.

> But of course, having the
> temperature readout always work would be a desired improvement.

More than a desired improvement, it is mandatory. How can the thermal
framework protect and mitigate the temperature with a twisted vision of
the thermal situation?

> As for ways to achieve that, I think the intended way would be to enable the
> interrupts that signal data ready on filtered mode (bits 19, 20, 21, 28), read
> the temperature and cache it so it is always available when the get_temp()
> callback is called.

It sounds very strange how the filtered mode is working. We setup the
filtered mode, then we shall receive an interrupt telling the data is
ready so we can read it?

> The issue with this is that it would cause *a lot* of
> interrupts, which doesn't seem worth it.

Why not use the immediate mode + interrupts ?

--
<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

2023-06-15 19:48:25

by Nícolas F. R. A. Prado

[permalink] [raw]

Subject: Re: [PATCH v4 0/5] Add LVTS support for mt8192

On Thu, Jun 08, 2023 at 11:39:27AM +0200, Daniel Lezcano wrote:
> On 01/06/2023 19:09, Nícolas F. R. A. Prado wrote:
> > On Wed, May 31, 2023 at 12:49:43PM +0800, Chen-Yu Tsai wrote:
> > > On Wed, May 31, 2023 at 3:51 AM Bernhard Rosenkränzer <[email protected]> wrote:
> > > >
> > > > From: Balsam CHIHI <[email protected]>
> > > >
> > > > Add full LVTS support (MCU thermal domain + AP thermal domain) to MediaTek MT8192 SoC.
> > > > Also, add Suspend and Resume support to LVTS Driver (all SoCs),
> > > > and update the documentation that describes the Calibration Data Offsets.
> > > >
> > > > Changelog:
> > > > v4 :
> > > > - Shrink the lvts_ap thermal sensor I/O range to 0xc00 to make
> > > > room for SVS support, pointed out by
> > > > AngeloGioacchino Del Regno <[email protected]>
> > > >
> > > > v3 :
> > > > - Rebased :
> > > > base-commit: 6a3d37b4d885129561e1cef361216f00472f7d2e
> > > > - Fix issues in v2 pointed out by Nícolas F. R. A. Prado <[email protected]>:
> > > > Use filtered mode to make sure threshold interrupts are triggered,
> > >
> > > I'm seeing sensor readout (either through sysfs/thermal/<x>/temp or hwmon)
> > > fail frequently on MT8192. If I run `sensors` (lm-sensors), at least a couple
> > > of the LVTS sensors would be N/A. Not sure if this is related to this change.
> >
> > Yes, it is. Filtered mode has some delay associated with reading, meaning most
> > of the time the value isn't ready, while immediate mode is, well, pretty much
> > immediate and the read always succeeds.
> >
> > For temperature monitoring, filtered mode should be used. It supports triggering
> > interrupts when crossing the thresholds. Immediate mode is meant for one-off
> > readings of the temperature. This is why I suggested using filtered mode.
> >
> > As far as the thermal framework goes, it's ok that filtered mode doesn't always
> > return a value, as it will keep the old one. But of course, having the
> > temperature readout always work would be a desired improvement.
> >
> > As for ways to achieve that, I think the intended way would be to enable the
> > interrupts that signal data ready on filtered mode (bits 19, 20, 21, 28), read
> > the temperature and cache it so it is always available when the get_temp()
> > callback is called. The issue with this is that it would cause *a lot* of
> > interrupts, which doesn't seem worth it.
> >
> > Another option that comes to mind would be to enable immediate mode only during
> > the get_temp() callback, to immediately read a value, and return to filtered
> > mode at the end. That might work, but I haven't tried yet.
>
> Why not understand why the filtered mode is unable to return temperature
> values most of the time?
>
> I tried with the filtered mode and I can see 90% of the time it is not
> possible to read the temperature.
>
> IIUC there are timings which can be setup, may be understand how to set them
> up in order to read the temperature correctly?
>
> Caching values, switching the mode or whatever is hackish :/

So this is what I've found after some more testing.

With the current settings, using filtered mode, only about 30% of the
measurement reads return valid results:
rate: 29% (success: 293, fail: 707)

While, as observed, in immediate mode, the reads always succeed:
rate: 100% (success: 1000, fail: 0)

Changing the configurations so that the measurements take less time improve the
rate (and analogously increasing the time worsens the rate). That is, with
PERIOD_UNIT = 0, GROUP_INTERVAL = 0, FILTER_INTERVAL = 0, SENSOR_INTERVAL = 0,
HW_FILTER = 0 (ie single sample) the rate is much improved:
rate: 91% (success: 918, fail: 82)

Though note that even though we're sampling as fast as possible and sampling
only once each time, so supposedly what immediate mode does, it's still not at
100% like in immediate mode.

Enabling the sensor 0 filter IRQ (bit 19) I've observed that it is triggered
about every 3500us (on the controller with all four sensors) with the current
settings, but after changing those timing registers, it happens every 344us.
With that in mind, in addition to those timing changes, if we also read the
register more than once with a timeout longer than that 344, that is,

rc = readl_poll_timeout(msr, value, value & BIT(16), 240, 400);

it's enough to get
rate: 100% (success: 1000, fail: 0)
and even better:
rate: 100% (success: 10000, fail: 0)

So it's still not exactly clear what's the relation of the VALID bit with the
timings in the hardware, but this at least gives us a way to get valid reads
without sacrificing interrupts.

Meanwhile, I've also tried reading the measurement during handling of the sensor
0 filter IRQ (bit 19), and while it definitely works much better than the
current 30%, giving a rate of 92%, it's still not 100%, which is intriguing
given this IRQ is supposed to signal the data is ready... I thought this might
be caused by timing issues, but increasing the timing of the measurements (by
setting PERIOD_UNIT = 120), lowered the rate to 84%.
Simply enabling this interrupt (and not reading the data in the IRQ), gives a
drastically worse rate:
rate: 3% (success: 32, fail: 968)
Which I understand to mean that whenever the IRQ is cleared, the hardware
invalidates the previous measurement. So this IRQ is definitely related to the
VALID bit, but it also is unexpectedly influenced by the timings.

The VALID bit is also updated when read, and it tends to take the same time
between IRQs to be reset, so my understanding is that on every IRQ the VALID
bit is re-set to 1, and reading it clears it. But this does not explain why with
smaller intervals a single read has more chance of succeeding.

At this point, though, I feel like if it is possible to guarantee that readings
in filtered mode will always be valid, it must be some hidden setting in
LVTS_CONFIG. But with what we have access to, the best we can hope for is to
make the invalid reads extremely unlikely, which is what shrinking the intervals
and polling the register as shown above gives us, so it's what I suggest us to
do.

Thanks,
Nícolas

2023-06-16 14:35:35

by Daniel Lezcano

[permalink] [raw]

Subject: Re: [PATCH v4 0/5] Add LVTS support for mt8192

Hi Nicolas,

thanks for investigating !

On 15/06/2023 21:17, Nícolas F. R. A. Prado wrote:
> On Thu, Jun 08, 2023 at 11:39:27AM +0200, Daniel Lezcano wrote:
>> On 01/06/2023 19:09, Nícolas F. R. A. Prado wrote:
>>> On Wed, May 31, 2023 at 12:49:43PM +0800, Chen-Yu Tsai wrote:
>>>> On Wed, May 31, 2023 at 3:51 AM Bernhard Rosenkränzer <[email protected]> wrote:
>>>>>
>>>>> From: Balsam CHIHI <[email protected]>
>>>>>
>>>>> Add full LVTS support (MCU thermal domain + AP thermal domain) to MediaTek MT8192 SoC.
>>>>> Also, add Suspend and Resume support to LVTS Driver (all SoCs),
>>>>> and update the documentation that describes the Calibration Data Offsets.
>>>>>
>>>>> Changelog:
>>>>> v4 :
>>>>> - Shrink the lvts_ap thermal sensor I/O range to 0xc00 to make
>>>>> room for SVS support, pointed out by
>>>>> AngeloGioacchino Del Regno <[email protected]>
>>>>>
>>>>> v3 :
>>>>> - Rebased :
>>>>> base-commit: 6a3d37b4d885129561e1cef361216f00472f7d2e
>>>>> - Fix issues in v2 pointed out by Nícolas F. R. A. Prado <[email protected]>:
>>>>> Use filtered mode to make sure threshold interrupts are triggered,
>>>>
>>>> I'm seeing sensor readout (either through sysfs/thermal/<x>/temp or hwmon)
>>>> fail frequently on MT8192. If I run `sensors` (lm-sensors), at least a couple
>>>> of the LVTS sensors would be N/A. Not sure if this is related to this change.
>>>
>>> Yes, it is. Filtered mode has some delay associated with reading, meaning most
>>> of the time the value isn't ready, while immediate mode is, well, pretty much
>>> immediate and the read always succeeds.
>>>
>>> For temperature monitoring, filtered mode should be used. It supports triggering
>>> interrupts when crossing the thresholds. Immediate mode is meant for one-off
>>> readings of the temperature. This is why I suggested using filtered mode.
>>>
>>> As far as the thermal framework goes, it's ok that filtered mode doesn't always
>>> return a value, as it will keep the old one. But of course, having the
>>> temperature readout always work would be a desired improvement.
>>>
>>> As for ways to achieve that, I think the intended way would be to enable the
>>> interrupts that signal data ready on filtered mode (bits 19, 20, 21, 28), read
>>> the temperature and cache it so it is always available when the get_temp()
>>> callback is called. The issue with this is that it would cause *a lot* of
>>> interrupts, which doesn't seem worth it.
>>>
>>> Another option that comes to mind would be to enable immediate mode only during
>>> the get_temp() callback, to immediately read a value, and return to filtered
>>> mode at the end. That might work, but I haven't tried yet.
>>
>> Why not understand why the filtered mode is unable to return temperature
>> values most of the time?
>>
>> I tried with the filtered mode and I can see 90% of the time it is not
>> possible to read the temperature.
>>
>> IIUC there are timings which can be setup, may be understand how to set them
>> up in order to read the temperature correctly?
>>
>> Caching values, switching the mode or whatever is hackish :/
>
> So this is what I've found after some more testing.
>
> With the current settings, using filtered mode, only about 30% of the
> measurement reads return valid results:
> rate: 29% (success: 293, fail: 707)
>
> While, as observed, in immediate mode, the reads always succeed:
> rate: 100% (success: 1000, fail: 0)
>
> Changing the configurations so that the measurements take less time improve the
> rate (and analogously increasing the time worsens the rate). That is, with
> PERIOD_UNIT = 0, GROUP_INTERVAL = 0, FILTER_INTERVAL = 0, SENSOR_INTERVAL = 0,
> HW_FILTER = 0 (ie single sample) the rate is much improved:
> rate: 91% (success: 918, fail: 82)
>
> Though note that even though we're sampling as fast as possible and sampling
> only once each time, so supposedly what immediate mode does, it's still not at
> 100% like in immediate mode.
>
> Enabling the sensor 0 filter IRQ (bit 19) I've observed that it is triggered
> about every 3500us (on the controller with all four sensors) with the current
> settings, but after changing those timing registers, it happens every 344us.
> With that in mind, in addition to those timing changes, if we also read the
> register more than once with a timeout longer than that 344, that is,
>
> rc = readl_poll_timeout(msr, value, value & BIT(16), 240, 400);
>
> it's enough to get
> rate: 100% (success: 1000, fail: 0)
> and even better:
> rate: 100% (success: 10000, fail: 0)
>
> So it's still not exactly clear what's the relation of the VALID bit with the
> timings in the hardware, but this at least gives us a way to get valid reads
> without sacrificing interrupts.
>
> Meanwhile, I've also tried reading the measurement during handling of the sensor
> 0 filter IRQ (bit 19), and while it definitely works much better than the
> current 30%, giving a rate of 92%, it's still not 100%, which is intriguing
> given this IRQ is supposed to signal the data is ready... I thought this might
> be caused by timing issues, but increasing the timing of the measurements (by
> setting PERIOD_UNIT = 120), lowered the rate to 84%.
> Simply enabling this interrupt (and not reading the data in the IRQ), gives a
> drastically worse rate:
> rate: 3% (success: 32, fail: 968)
> Which I understand to mean that whenever the IRQ is cleared, the hardware
> invalidates the previous measurement. So this IRQ is definitely related to the
> VALID bit, but it also is unexpectedly influenced by the timings.
>
> The VALID bit is also updated when read, and it tends to take the same time
> between IRQs to be reset, so my understanding is that on every IRQ the VALID
> bit is re-set to 1, and reading it clears it. But this does not explain why with
> smaller intervals a single read has more chance of succeeding.
>
> At this point, though, I feel like if it is possible to guarantee that readings
> in filtered mode will always be valid, it must be some hidden setting in
> LVTS_CONFIG. But with what we have access to, the best we can hope for is to
> make the invalid reads extremely unlikely, which is what shrinking the intervals
> and polling the register as shown above gives us, so it's what I suggest us to
> do.
Let me summarize and check I'm understanding correctly:

1. Immediate mode

- 100% successful read, no delay when reading
- No interrupts when crossing the thresholds (at the first glance)

2. Filtered mode

- Interrupts when data is ready
- Interrupts when crossing the thresholds
- Polling read until TMU valid
- maximum two register reads
- minimum delay 240us
- maximum delay 480us

From my POV, the filtered mode is not designed for an OSPM, it is for
real time system for thermal acquisition or similar. It is unthinkable a
sensor is firing so many interrupts waking up the CPU to tell a
temperature is ready to be read. And it is strange we have to poll loop
a register to read a temperature.

The thermal framework is designed to protect the silicon and
consequently reads with non constant delay and/or high delay can have an
impact on time sensitive governor. Skipping the temperature because we
fail to read is also not acceptable, in the case of mitigation, that can
have an impact.

The normal mode should be:

- temperature below threshold => no wakeups
- temperature crosses the threshold => interrupt fires
- mitigation => wake up every 'passive' delay period

With the filtered mode we have:

- temperature below threshold => interrupts telling the value is ready
(we want to ignore that)

- temperature crosses the threshold => interrupt but not sure we can
read the temperature correctly

- mitigation => wake up every 'passive' delay period but not sure we
can read the temperature correctly

With the immediate mode:

- temperature below threshold => interrupts is not working, so we have
to monitor the temperature and wake up every <monitor> delay

- temperature crosses the threshold => no interrupt, detected by the
monitoring

- mitigation => wake up every 'passive' delay period, temperature is
accurate

It seems not logical to have the immediate mode not working with the
interrupts when crossing the thresholds. I would say we should stick to
the immediate mode and double check if the interrupt can work with this
mode.

--
<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

2023-06-16 19:18:06

by Nícolas F. R. A. Prado

[permalink] [raw]

Subject: Re: [PATCH v4 0/5] Add LVTS support for mt8192

On Fri, Jun 16, 2023 at 04:14:43PM +0200, Daniel Lezcano wrote:
>
> Hi Nicolas,
>
> thanks for investigating !
>
> On 15/06/2023 21:17, Nícolas F. R. A. Prado wrote:
> > On Thu, Jun 08, 2023 at 11:39:27AM +0200, Daniel Lezcano wrote:
> > > On 01/06/2023 19:09, Nícolas F. R. A. Prado wrote:
> > > > On Wed, May 31, 2023 at 12:49:43PM +0800, Chen-Yu Tsai wrote:
> > > > > On Wed, May 31, 2023 at 3:51 AM Bernhard Rosenkränzer <[email protected]> wrote:
> > > > > >
> > > > > > From: Balsam CHIHI <[email protected]>
> > > > > >
> > > > > > Add full LVTS support (MCU thermal domain + AP thermal domain) to MediaTek MT8192 SoC.
> > > > > > Also, add Suspend and Resume support to LVTS Driver (all SoCs),
> > > > > > and update the documentation that describes the Calibration Data Offsets.
> > > > > >
> > > > > > Changelog:
> > > > > > v4 :
> > > > > > - Shrink the lvts_ap thermal sensor I/O range to 0xc00 to make
> > > > > > room for SVS support, pointed out by
> > > > > > AngeloGioacchino Del Regno <[email protected]>
> > > > > >
> > > > > > v3 :
> > > > > > - Rebased :
> > > > > > base-commit: 6a3d37b4d885129561e1cef361216f00472f7d2e
> > > > > > - Fix issues in v2 pointed out by Nícolas F. R. A. Prado <[email protected]>:
> > > > > > Use filtered mode to make sure threshold interrupts are triggered,
> > > > >
> > > > > I'm seeing sensor readout (either through sysfs/thermal/<x>/temp or hwmon)
> > > > > fail frequently on MT8192. If I run `sensors` (lm-sensors), at least a couple
> > > > > of the LVTS sensors would be N/A. Not sure if this is related to this change.
> > > >
> > > > Yes, it is. Filtered mode has some delay associated with reading, meaning most
> > > > of the time the value isn't ready, while immediate mode is, well, pretty much
> > > > immediate and the read always succeeds.
> > > >
> > > > For temperature monitoring, filtered mode should be used. It supports triggering
> > > > interrupts when crossing the thresholds. Immediate mode is meant for one-off
> > > > readings of the temperature. This is why I suggested using filtered mode.
> > > >
> > > > As far as the thermal framework goes, it's ok that filtered mode doesn't always
> > > > return a value, as it will keep the old one. But of course, having the
> > > > temperature readout always work would be a desired improvement.
> > > >
> > > > As for ways to achieve that, I think the intended way would be to enable the
> > > > interrupts that signal data ready on filtered mode (bits 19, 20, 21, 28), read
> > > > the temperature and cache it so it is always available when the get_temp()
> > > > callback is called. The issue with this is that it would cause *a lot* of
> > > > interrupts, which doesn't seem worth it.
> > > >
> > > > Another option that comes to mind would be to enable immediate mode only during
> > > > the get_temp() callback, to immediately read a value, and return to filtered
> > > > mode at the end. That might work, but I haven't tried yet.
> > >
> > > Why not understand why the filtered mode is unable to return temperature
> > > values most of the time?
> > >
> > > I tried with the filtered mode and I can see 90% of the time it is not
> > > possible to read the temperature.
> > >
> > > IIUC there are timings which can be setup, may be understand how to set them
> > > up in order to read the temperature correctly?
> > >
> > > Caching values, switching the mode or whatever is hackish :/
> >
> > So this is what I've found after some more testing.
> >
> > With the current settings, using filtered mode, only about 30% of the
> > measurement reads return valid results:
> > rate: 29% (success: 293, fail: 707)
> >
> > While, as observed, in immediate mode, the reads always succeed:
> > rate: 100% (success: 1000, fail: 0)
> >
> > Changing the configurations so that the measurements take less time improve the
> > rate (and analogously increasing the time worsens the rate). That is, with
> > PERIOD_UNIT = 0, GROUP_INTERVAL = 0, FILTER_INTERVAL = 0, SENSOR_INTERVAL = 0,
> > HW_FILTER = 0 (ie single sample) the rate is much improved:
> > rate: 91% (success: 918, fail: 82)
> >
> > Though note that even though we're sampling as fast as possible and sampling
> > only once each time, so supposedly what immediate mode does, it's still not at
> > 100% like in immediate mode.
> >
> > Enabling the sensor 0 filter IRQ (bit 19) I've observed that it is triggered
> > about every 3500us (on the controller with all four sensors) with the current
> > settings, but after changing those timing registers, it happens every 344us.
> > With that in mind, in addition to those timing changes, if we also read the
> > register more than once with a timeout longer than that 344, that is,
> >
> > rc = readl_poll_timeout(msr, value, value & BIT(16), 240, 400);
> >
> > it's enough to get
> > rate: 100% (success: 1000, fail: 0)
> > and even better:
> > rate: 100% (success: 10000, fail: 0)
> >
> > So it's still not exactly clear what's the relation of the VALID bit with the
> > timings in the hardware, but this at least gives us a way to get valid reads
> > without sacrificing interrupts.
> >
> > Meanwhile, I've also tried reading the measurement during handling of the sensor
> > 0 filter IRQ (bit 19), and while it definitely works much better than the
> > current 30%, giving a rate of 92%, it's still not 100%, which is intriguing
> > given this IRQ is supposed to signal the data is ready... I thought this might
> > be caused by timing issues, but increasing the timing of the measurements (by
> > setting PERIOD_UNIT = 120), lowered the rate to 84%.
> > Simply enabling this interrupt (and not reading the data in the IRQ), gives a
> > drastically worse rate:
> > rate: 3% (success: 32, fail: 968)
> > Which I understand to mean that whenever the IRQ is cleared, the hardware
> > invalidates the previous measurement. So this IRQ is definitely related to the
> > VALID bit, but it also is unexpectedly influenced by the timings.
> >
> > The VALID bit is also updated when read, and it tends to take the same time
> > between IRQs to be reset, so my understanding is that on every IRQ the VALID
> > bit is re-set to 1, and reading it clears it. But this does not explain why with
> > smaller intervals a single read has more chance of succeeding.
> >
> > At this point, though, I feel like if it is possible to guarantee that readings
> > in filtered mode will always be valid, it must be some hidden setting in
> > LVTS_CONFIG. But with what we have access to, the best we can hope for is to
> > make the invalid reads extremely unlikely, which is what shrinking the intervals
> > and polling the register as shown above gives us, so it's what I suggest us to
> > do.
> Let me summarize and check I'm understanding correctly:
>
> 1. Immediate mode
>
> - 100% successful read, no delay when reading
> - No interrupts when crossing the thresholds (at the first glance)
>
> 2. Filtered mode
>
> - Interrupts when data is ready

In fact, immediate mode also has interrupts for when data is ready (bits 16, 17,
18, 27 in MONINTST), but its frequency is so quick, that the VALID bit is always
set, and keeping these interrupts enabled would just cause unnecessary wakeups.

> - Interrupts when crossing the thresholds
> - Polling read until TMU valid
> - maximum two register reads
> - minimum delay 240us
> - maximum delay 480us

It's more like minimum 0us (it might already available) and maximum 344us
(though it will vary with timing settings, sampling setting, and number of
sensors in the controller)

>
> From my POV, the filtered mode is not designed for an OSPM, it is for real
> time system for thermal acquisition or similar. It is unthinkable a sensor
> is firing so many interrupts waking up the CPU to tell a temperature is
> ready to be read. And it is strange we have to poll loop a register to read
> a temperature.

Indeed, that would explain the way this hardware behaves.

>
> The thermal framework is designed to protect the silicon and consequently
> reads with non constant delay and/or high delay can have an impact on time
> sensitive governor. Skipping the temperature because we fail to read is also
> not acceptable, in the case of mitigation, that can have an impact.
>
>
> The normal mode should be:
>
> - temperature below threshold => no wakeups
> - temperature crosses the threshold => interrupt fires
> - mitigation => wake up every 'passive' delay period
>
> With the filtered mode we have:
>
> - temperature below threshold => interrupts telling the value is ready (we
> want to ignore that)

And to be clear, we can ignore that.

>
> - temperature crosses the threshold => interrupt but not sure we can read
> the temperature correctly
>
> - mitigation => wake up every 'passive' delay period but not sure we can
> read the temperature correctly
>
> With the immediate mode:
>
> - temperature below threshold => interrupts is not working, so we have to
> monitor the temperature and wake up every <monitor> delay
>
> - temperature crosses the threshold => no interrupt, detected by the
> monitoring
>
> - mitigation => wake up every 'passive' delay period, temperature is
> accurate
>
> It seems not logical to have the immediate mode not working with the
> interrupts when crossing the thresholds. I would say we should stick to the
> immediate mode and double check if the interrupt can work with this mode.

But that is the one thing I'm really really confident about: It's not possible
to have threshold interrupts working in immediate mode. As soon as immediate
mode is enabled for one of the sensors by writing to the MSRCTL1 register, the
threshold interrupts no longer trigger.

I think we'll need to pick between the two non-ideal options we have:
* Immediate mode without working interrupts but reliable reads
* Filtered mode with working interrupts but unreliable reads

Keeping in mind that we can make filtered mode have mostly reliable reads,
with the cost of taking longer to read the measurements. And while I understand
that the governor is time-sensitive like you said, right now I think the numbers
still make it worth it: delaying 400us to get good reads on interrupts in
filtered mode vs waiting for the 1000ms poll in immediate mode.

(Another undiscussed consideration is that immediate mode might return glitch
reads, while filtered mode, by performing multiple samples, averaging, etc,
might actually return measurements that are closer to reality, though of course
if it can't be read, it's no good. And I haven't compared results enough to say
how much the filtering helps).

Thanks,
Nícolas

2023-07-04 06:28:19

[permalink] [raw]

Subject: Re: [PATCH v4 3/5] thermal/drivers/mediatek/lvts_thermal: Add mt8192 support

On Wed, May 31, 2023 at 4:07 PM AngeloGioacchino Del Regno
<[email protected]> wrote:
>
> Il 30/05/23 21:51, Bernhard Rosenkränzer ha scritto:
> > From: Balsam CHIHI <[email protected]>
> >
> > Add LVTS Driver support for MT8192.
> >
> > Co-developed-by : Nícolas F. R. A. Prado <[email protected]>
> > Signed-off-by: Nícolas F. R. A. Prado <[email protected]>
> > Signed-off-by: Balsam CHIHI <[email protected]>
> > Reviewed-by: Nícolas F. R. A. Prado <[email protected]>
> > Signed-off-by: Bernhard Rosenkränzer <[email protected]>
> > Reviewed-by: Matthias Brugger <[email protected]>
> > ---
> > drivers/thermal/mediatek/lvts_thermal.c | 95 +++++++++++++++++++++++++
> > 1 file changed, 95 insertions(+)
> >
> > diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
> > index 5ea8a9d569ea6..d5e5214784ece 100644
> > --- a/drivers/thermal/mediatek/lvts_thermal.c
> > +++ b/drivers/thermal/mediatek/lvts_thermal.c
> > @@ -80,6 +80,7 @@
> > #define LVTS_MSR_FILTERED_MODE 1
> >
> > #define LVTS_HW_SHUTDOWN_MT8195 105000
> > +#define LVTS_HW_SHUTDOWN_MT8192 105000
> >
> > static int golden_temp = LVTS_GOLDEN_TEMP_DEFAULT;
> > static int coeff_b = LVTS_COEFF_B;
> > @@ -1280,6 +1281,88 @@ static const struct lvts_ctrl_data mt8195_lvts_ap_data_ctrl[] = {
> > }
> > };
> >
> > +static const struct lvts_ctrl_data mt8192_lvts_mcu_data_ctrl[] = {
> > + {
> > + .cal_offset = { 0x04, 0x08 },
> > + .lvts_sensor = {
> > + { .dt_id = MT8192_MCU_BIG_CPU0 },
> > + { .dt_id = MT8192_MCU_BIG_CPU1 }
> > + },
> > + .num_lvts_sensor = 2,
> > + .offset = 0x0,
> > + .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
> > + .mode = LVTS_MSR_FILTERED_MODE,
> > + },
> > + {
> > + .cal_offset = { 0x0c, 0x10 },
> > + .lvts_sensor = {
> > + { .dt_id = MT8192_MCU_BIG_CPU2 },
> > + { .dt_id = MT8192_MCU_BIG_CPU3 }
> > + },
> > + .num_lvts_sensor = 2,
> > + .offset = 0x100,
> > + .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
> > + .mode = LVTS_MSR_FILTERED_MODE,
> > + },
> > + {
> > + .cal_offset = { 0x14, 0x18, 0x1c, 0x20 },
> > + .lvts_sensor = {
> > + { .dt_id = MT8192_MCU_LITTLE_CPU0 },
> > + { .dt_id = MT8192_MCU_LITTLE_CPU1 },
> > + { .dt_id = MT8192_MCU_LITTLE_CPU2 },
> > + { .dt_id = MT8192_MCU_LITTLE_CPU3 }
> > + },
> > + .num_lvts_sensor = 4,
> > + .offset = 0x200,
> > + .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
> > + .mode = LVTS_MSR_FILTERED_MODE,
> > + }
> > +};
> > +
> > +static const struct lvts_ctrl_data mt8192_lvts_ap_data_ctrl[] = {
> > + {
> > + .cal_offset = { 0x24, 0x28 },
> > + .lvts_sensor = {
> > + { .dt_id = MT8192_AP_VPU0 },
> > + { .dt_id = MT8192_AP_VPU1 }
> > + },
> > + .num_lvts_sensor = 2,
> > + .offset = 0x0,
> > + .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
> > + },
> > + {
> > + .cal_offset = { 0x2c, 0x30 },
> > + .lvts_sensor = {
> > + { .dt_id = MT8192_AP_GPU0 },
> > + { .dt_id = MT8192_AP_GPU1 }
> > + },
> > + .num_lvts_sensor = 2,
> > + .offset = 0x100,
> > + .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
>
> I'm unable to get readings for the GPU sensors, didn't really check
> the others; `cat (xxxx)` gives a resource not available error, is that
> the same for you?!

That could be related to the filtered mode stuff from Nicolas?

> Regards,
> Angelo
>
> > + },
> > + {
> > + .cal_offset = { 0x34, 0x38 },
> > + .lvts_sensor = {
> > + { .dt_id = MT8192_AP_INFRA },
> > + { .dt_id = MT8192_AP_CAM },
> > + },
> > + .num_lvts_sensor = 2,
> > + .offset = 0x200,
> > + .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
> > + },
> > + {
> > + .cal_offset = { 0x3c, 0x40, 0x44 },
> > + .lvts_sensor = {
> > + { .dt_id = MT8192_AP_MD0 },
> > + { .dt_id = MT8192_AP_MD1 },
> > + { .dt_id = MT8192_AP_MD2 }
> > + },
> > + .num_lvts_sensor = 3,
> > + .offset = 0x300,
> > + .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
> > + }
> > +};
> > +
> > static const struct lvts_data mt8195_lvts_mcu_data = {
> > .lvts_ctrl = mt8195_lvts_mcu_data_ctrl,
> > .num_lvts_ctrl = ARRAY_SIZE(mt8195_lvts_mcu_data_ctrl),
> > @@ -1290,9 +1373,21 @@ static const struct lvts_data mt8195_lvts_ap_data = {
> > .num_lvts_ctrl = ARRAY_SIZE(mt8195_lvts_ap_data_ctrl),
> > };
> >
> > +static const struct lvts_data mt8192_lvts_mcu_data = {
> > + .lvts_ctrl = mt8192_lvts_mcu_data_ctrl,
> > + .num_lvts_ctrl = ARRAY_SIZE(mt8192_lvts_mcu_data_ctrl),
> > +};
> > +
> > +static const struct lvts_data mt8192_lvts_ap_data = {
> > + .lvts_ctrl = mt8192_lvts_ap_data_ctrl,
> > + .num_lvts_ctrl = ARRAY_SIZE(mt8192_lvts_ap_data_ctrl),
> > +};
> > +
> > static const struct of_device_id lvts_of_match[] = {
> > { .compatible = "mediatek,mt8195-lvts-mcu", .data = &mt8195_lvts_mcu_data },
> > { .compatible = "mediatek,mt8195-lvts-ap", .data = &mt8195_lvts_ap_data },
> > + { .compatible = "mediatek,mt8192-lvts-mcu", .data = &mt8192_lvts_mcu_data },
> > + { .compatible = "mediatek,mt8192-lvts-ap", .data = &mt8192_lvts_ap_data },
> > {},
> > };
> > MODULE_DEVICE_TABLE(of, lvts_of_match);
>
>

2023-07-04 06:30:51