imx: thermal: use CPU temperature grade info for thresholds

The IMX6Q/IMX6DL SoC's have a 2-bit temperature grade stored in OTP. Instead of assuming 85C for passive cooling threshold and 105C for critical use the thermal grade for these configurations. For IMX6SX which does not have a temperature grade in OTP (according to the ref manual) assume 105C critical. We will default the user-settable passive threshold to crit - 20C.

Cc Anson Huang <[email protected]>

Cc Fabio Estevam <[email protected]>

Cc Shawn Guo <[email protected]>

Signed-off-by Tim Harvey <[email protected]>

drivers/thermal/imx_thermal.c static int imx_get_sensor_data(struct platform_device *pdev) · line 431

Quoted 10 lines · Tim Harvey

drivers/thermal/imx_thermal.c +10 -0

+               /* The maximum die temp is specified by the Temperature Grade */
+               switch ((val >> 6) & 0x3) {
+               case 0: /* Commercial (0 to 95C) */
+                       data->temp_grade = "Commercial";
+                       data->temp_critical = 95000;
+                       break;
+               case 1: /* Extended Commercial (-20 to 105C) */
+                       data->temp_grade = "Extended Commercial";
+                       data->temp_critical = 105000;
+                       break;

Looks good, only a minor suggestion for consistency: could you please change "Commercial" to "Consumer"?

This is the term that is referenced in the datasheet, so better keep it consistent.

Thanks

Quoted 13 lines · Fabio Estevam

drivers/thermal/imx_thermal.c static int imx_get_sensor_data(struct platform_device *pdev) · line 431

Quoted 10 lines · Tim Harvey

drivers/thermal/imx_thermal.c +10 -0

+               /* The maximum die temp is specified by the Temperature Grade */
+               switch ((val >> 6) & 0x3) {
+               case 0: /* Commercial (0 to 95C) */
+                       data->temp_grade = "Commercial";
+                       data->temp_critical = 95000;
+                       break;
+               case 1: /* Extended Commercial (-20 to 105C) */
+                       data->temp_grade = "Extended Commercial";
+                       data->temp_critical = 105000;
+                       break;

Quoted 7 lines

Looks good, only a minor suggestion for consistency: could you please change "Commercial" to "Consumer"?

This is the term that is referenced in the datasheet, so better keep it consistent.

Thanks

Fabio,

Actually this seems to be very inconsistent from Freescale and the register uses the word 'commercial' - I think you are referring to the part identifier which is part of the ordering information. The Part Number Nomenclature in the 'datasheets' use the word 'Commercial' in the temperature grade field and the word 'Consumer' in some (but not all) of the Part differentiator field. The register description uses the word Commercial. I believe this is more of a marketing term than anything else so I don't really care what word is used. I've heard the word commercial used in terms of temperature grades and not consumer personally.

I should note that the actual OTP fuse I'm looking at is only described in the IMX6DQRM (Reference manual for the DUAL and QUAD) and is shown as 'reserved' in the IMX6SDLRM (SOLO and DUALLITE Reference Manual) and the 'IMX6SXRM' for the SX. I have been able to show expirimentaly that the speed grade and temperature grade OTP's exist in the IMX6SDL (even though they are marked as 'reserved' in the RM) but I do not have an IMX6SX to see if the same is true. I have a query out to our FAE for clarification on the OTP registers.

Tim

Fabio, any particular preference ? Is the patch good to with the current nomenclature?

The IMX6Q/IMX6DL SoC's have a 2-bit temperature grade stored in OTP. Instead of assuming 85C for passive cooling threshold and 105C for critical use the thermal grade for these configurations. For IMX6SX which does not have a temperature grade in OTP (according to the ref manual) assume 105C critical. We will default the user-settable passive threshold to crit - 20C.

Cc Anson Huang <[email protected]>

Cc Fabio Estevam <[email protected]>

Cc Shawn Guo <[email protected]>

Signed-off-by Tim Harvey <[email protected]>

---
 drivers/thermal/imx_thermal.c | 64 +++++++++++++++++++++++++++++++++----------
 1 file changed, 49 insertions(+), 15 deletions(-)

drivers/thermal/imx_thermal.c +49 -16

index 2ccbc07..c6a4eed 100644
@@ -55,6 +55,7 @@
 #define TEMPSENSE2_PANIC_VALUE_SHIFT   16
 #define TEMPSENSE2_PANIC_VALUE_MASK    0xfff0000
 
+#define OCOTP_MEM0                     0x0480
 #define OCOTP_ANA1                     0x04e0
 
 /* The driver supports 1 passive trip point and 1 critical trip point */

@@ -64,12 +65,6 @@ enum imx_thermal_trip {
        IMX_TRIP_NUM,
 };
 
-/*
- * It defines the temperature in millicelsius for passive trip point
- * that will trigger cooling action when crossed.
- */
-#define IMX_TEMP_PASSIVE               85000
-
 #define IMX_POLLING_DELAY              2000 /* millisecond */
 #define IMX_PASSIVE_DELAY              1000
 

@@ -106,6 +101,7 @@ struct imx_thermal_data {
        int irq;
        struct clk *thermal_clk;
        const struct thermal_soc_data *socdata;
+       const char *temp_grade;
 };
 
 static void imx_set_panic_temp(struct imx_thermal_data *data,

@@ -289,7 +285,8 @@ static int imx_set_trip_temp(struct thermal_zone_device *tz, int trip,
        if (trip == IMX_TRIP_CRITICAL)
                return -EPERM;
 
-       if (temp > IMX_TEMP_PASSIVE)
+       /* do not allow passive to be set higher than critical - 20C */
+       if (temp > (data->temp_critical - (1000 * 20)))
                return -EINVAL;
 
        data->temp_passive = temp;

@@ -404,17 +401,50 @@ static int imx_get_sensor_data(struct platform_device *pdev)
        data->c1 = temp64;
        data->c2 = n1 * data->c1 + 1000 * t1;
 
-       /*
-        * Set the default passive cooling trip point,
-        * can be changed from userspace.
-        */
-       data->temp_passive = IMX_TEMP_PASSIVE;
+       /* Assume Extended Commercial temperature grade limits (105C max) */
+       data->temp_grade = "unknown";
+       data->temp_critical = 105000;
+
+       /* For IMX6Q use OTP for thermal grade */
+       if (data->socdata->version == TEMPMON_IMX6Q) {
+               ret = regmap_read(map, OCOTP_MEM0, &val);
+               if (ret) {
+                       dev_err(&pdev->dev, "failed to read temp grade: %d\n",
+                               ret);
+                       return ret;
+               }
+
+               if (val == 0 || val == ~0) {
+                       dev_err(&pdev->dev, "invalid temp grade data\n");
+                       return -EINVAL;
+               }
+
+               /* The maximum die temp is specified by the Temperature Grade */
+               switch ((val >> 6) & 0x3) {
+               case 0: /* Commercial (0 to 95C) */
+                       data->temp_grade = "Commercial";
+                       data->temp_critical = 95000;
+                       break;
+               case 1: /* Extended Commercial (-20 to 105C) */
+                       data->temp_grade = "Extended Commercial";
+                       data->temp_critical = 105000;
+                       break;
+               case 2: /* Industrial (-40 to 105C) */
+                       data->temp_grade = "Industrial";
+                       data->temp_critical = 105000;
+                       break;
+               case 3: /* Automotive (-40 to 125C) */
+                       data->temp_grade = "Automotive";
+                       data->temp_critical = 125000;
+                       break;
+               }
+       }
 
        /*
-        * The maximum die temperature set to 20 C higher than
-        * IMX_TEMP_PASSIVE.
+        * Set the default passive cooling trip point to critical - 20C
+        * (can be changed from userspace)
         */
-       data->temp_critical = 1000 * 20 + data->temp_passive;
+       data->temp_passive = data->temp_critical - (1000 * 20);
 
        return 0;
 }

@@ -559,6 +589,10 @@ static int imx_thermal_probe(struct platform_device *pdev)
                return ret;
        }
 
+       dev_info(&pdev->dev, "%s CPU temperature grade - "
+                "thresholds: crit:%ldC passive:%ldC\n", data->temp_grade,
+                data->temp_critical / 1000, data->temp_passive / 1000);
+
        /* Enable measurements at ~ 10 Hz */
        regmap_write(map, TEMPSENSE1 + REG_CLR, TEMPSENSE1_MEASURE_FREQ);
        measure_freq = DIV_ROUND_UP(32768, 10); /* 10 Hz */
--

1.9.1

The IMX6Q/IMX6DL SoC's have a 2-bit temperature grade stored in OTP which is valid for all IMX6 SoC's (despite the fact that the IMXSDLRM and IMXSXRM do not document this - this has been proven via tests as well as verified by Freescale FAE).

Instead of assuming a fixed 85C for passive cooling threshold and 105C for critical use the thermal grade for these configurations.

We will set the critical to maxT - 5C and passive to maxT - 10C.

Cc Anson Huang <[email protected]>

Cc Fabio Estevam <[email protected]>

Cc Eduardo Valentin <[email protected]>

Cc Zhang Rui <[email protected]>

Signed-off-by Tim Harvey <[email protected]>

Quoted 9 lines · Tim Harvey

The IMX6Q/IMX6DL SoC's have a 2-bit temperature grade stored in OTP which is valid for all IMX6 SoC's (despite the fact that the IMXSDLRM and IMXSXRM do not document this - this has been proven via tests as well as verified by Freescale FAE).

Instead of assuming a fixed 85C for passive cooling threshold and 105C for critical use the thermal grade for these configurations.

We will set the critical to maxT - 5C and passive to maxT - 10C.

Quoted 10 lines · Shawn Guo

Quoted 9 lines · Tim Harvey

The IMX6Q/IMX6DL SoC's have a 2-bit temperature grade stored in OTP which is valid for all IMX6 SoC's (despite the fact that the IMXSDLRM and IMXSXRM do not document this - this has been proven via tests as well as verified by Freescale FAE).

Instead of assuming a fixed 85C for passive cooling threshold and 105C for critical use the thermal grade for these configurations.

We will set the critical to maxT - 5C and passive to maxT - 10C.

I would like to chime in here if you don't mind. I have been carrying a patch similar to this in the SolidRun repo to fix cooling issues that we have had. I would recommend keeping the passive temp at maxT
- 20C due to the thermal properties of the chip. I have found that around 85-90C we can maintain a relatively steady thermal state with only passive cooling. Generally with a hard non-NEON based cpu workload the iMX6 will level off at about 87C with all the cores clocked to 1Ghz, and sometimes dipping down to 800Mhz periodically. With a NEON based workload on all the cores it will push beyond this and generally end up finding steady state at about 800Mhz right around 90C.

If you raise the initial passive threshold by 10C it will allow enough heat to build up in the chip that the only way to avoid reaching critical temps is by dropping the CPU down to its lowest frequency. This is not the best experience as then you have a much warmer chip and if the workload doesn't change you will just be switching between running at the highest cpu frequency or lowest which makes for a choppy experience. A longer passive cooling zone allows the temperature of the chip to be regulated using only passive methods but without drastic performance drops.

I am doing things a bit differently in my implementation as I setup a passive cooling zone for each cpu frequency, but that is just so you can have more control from userspace by changing the different passive trip points.

-Jon

Quoted 39 lines · Jon Nettleton

Quoted 10 lines · Shawn Guo

Quoted 9 lines · Tim Harvey

The IMX6Q/IMX6DL SoC's have a 2-bit temperature grade stored in OTP which is valid for all IMX6 SoC's (despite the fact that the IMXSDLRM and IMXSXRM do not document this - this has been proven via tests as well as verified by Freescale FAE).

Instead of assuming a fixed 85C for passive cooling threshold and 105C for critical use the thermal grade for these configurations.

We will set the critical to maxT - 5C and passive to maxT - 10C.

I would like to chime in here if you don't mind. I have been carrying a patch similar to this in the SolidRun repo to fix cooling issues that we have had. I would recommend keeping the passive temp at maxT
- 20C due to the thermal properties of the chip. I have found that around 85-90C we can maintain a relatively steady thermal state with only passive cooling. Generally with a hard non-NEON based cpu workload the iMX6 will level off at about 87C with all the cores clocked to 1Ghz, and sometimes dipping down to 800Mhz periodically. With a NEON based workload on all the cores it will push beyond this and generally end up finding steady state at about 800Mhz right around 90C.

If you raise the initial passive threshold by 10C it will allow enough heat to build up in the chip that the only way to avoid reaching critical temps is by dropping the CPU down to its lowest frequency. This is not the best experience as then you have a much warmer chip and if the workload doesn't change you will just be switching between running at the highest cpu frequency or lowest which makes for a choppy experience. A longer passive cooling zone allows the temperature of the chip to be regulated using only passive methods but without drastic performance drops.

I am doing things a bit differently in my implementation as I setup a passive cooling zone for each cpu frequency, but that is just so you can have more control from userspace by changing the different passive trip points.

Quoted 1 line

-Jon

Jon,

I can agree with leaving a Max-20C passive delta. What do you think about the critical threshold of Max-5C and rule of not allowing it to be changed?

Quoted 40 lines · Tim Harvey

Quoted 39 lines · Jon Nettleton

Quoted 10 lines · Shawn Guo

Quoted 9 lines · Tim Harvey

The IMX6Q/IMX6DL SoC's have a 2-bit temperature grade stored in OTP which is valid for all IMX6 SoC's (despite the fact that the IMXSDLRM and IMXSXRM do not document this - this has been proven via tests as well as verified by Freescale FAE).

Instead of assuming a fixed 85C for passive cooling threshold and 105C for critical use the thermal grade for these configurations.

We will set the critical to maxT - 5C and passive to maxT - 10C.

I would like to chime in here if you don't mind. I have been carrying a patch similar to this in the SolidRun repo to fix cooling issues that we have had. I would recommend keeping the passive temp at maxT
- 20C due to the thermal properties of the chip. I have found that around 85-90C we can maintain a relatively steady thermal state with only passive cooling. Generally with a hard non-NEON based cpu workload the iMX6 will level off at about 87C with all the cores clocked to 1Ghz, and sometimes dipping down to 800Mhz periodically. With a NEON based workload on all the cores it will push beyond this and generally end up finding steady state at about 800Mhz right around 90C.

If you raise the initial passive threshold by 10C it will allow enough heat to build up in the chip that the only way to avoid reaching critical temps is by dropping the CPU down to its lowest frequency. This is not the best experience as then you have a much warmer chip and if the workload doesn't change you will just be switching between running at the highest cpu frequency or lowest which makes for a choppy experience. A longer passive cooling zone allows the temperature of the chip to be regulated using only passive methods but without drastic performance drops.

I am doing things a bit differently in my implementation as I setup a passive cooling zone for each cpu frequency, but that is just so you can have more control from userspace by changing the different passive trip points.

Quoted 8 lines

Quoted 1 line

-Jon

Jon,

I can agree with leaving a Max-20C passive delta. What do you think about the critical threshold of Max-5C and rule of not allowing it to be changed?

Tim,

I definitely agree that the Critical temp should be a fixed point. Is the purpose of lowering the critical threshold from the hardware default, to allow Linux to shutdown more cleanly rather than just have the hardware shutting down? If that is the case then I think that is fine. If it is to protect the SOC then that is unnecessary. We have heated the SOCs to well beyond the critical threshold and they have survived just fine.

This is a bit out of context but here is the formula I am using to figure out my trip points. By default I use a linear set of trip points for passive cooling.
https://github.com/linux4kix/linux-linaro-stable-mx6/commit/212c17d543739a5fe0bd75b66c10f05177e8bcb0

The short of it is I set a trip delta of 6C and then figure out the lowest passive trip point as Critical - (#passive trip points * trip delta), where each cpu frequency stage is a passive trip point. This will allow an 800Mhz SOC with 2 trip points to run at full speed longer than a 1.2Ghz with 4 trip points. The idea being that the higher the clock rate means we will generate more heat and have more passive cooling levels so it is better to drop the top speed of the CPU earlier in order to let the passive cooling be effective and find a steady state.

This may be a bit over the top but has fixed problems where long running processes would build up heat and eventually cause a thermal shutdown, but doesn't completely cripple the faster SOCs.

Quoted 75 lines · Jon Nettleton

Quoted 40 lines · Tim Harvey

Quoted 39 lines · Jon Nettleton

Quoted 10 lines · Shawn Guo

Quoted 9 lines · Tim Harvey

The IMX6Q/IMX6DL SoC's have a 2-bit temperature grade stored in OTP which is valid for all IMX6 SoC's (despite the fact that the IMXSDLRM and IMXSXRM do not document this - this has been proven via tests as well as verified by Freescale FAE).

Instead of assuming a fixed 85C for passive cooling threshold and 105C for critical use the thermal grade for these configurations.

We will set the critical to maxT - 5C and passive to maxT - 10C.

I would like to chime in here if you don't mind. I have been carrying a patch similar to this in the SolidRun repo to fix cooling issues that we have had. I would recommend keeping the passive temp at maxT
- 20C due to the thermal properties of the chip. I have found that around 85-90C we can maintain a relatively steady thermal state with only passive cooling. Generally with a hard non-NEON based cpu workload the iMX6 will level off at about 87C with all the cores clocked to 1Ghz, and sometimes dipping down to 800Mhz periodically. With a NEON based workload on all the cores it will push beyond this and generally end up finding steady state at about 800Mhz right around 90C.

If you raise the initial passive threshold by 10C it will allow enough heat to build up in the chip that the only way to avoid reaching critical temps is by dropping the CPU down to its lowest frequency. This is not the best experience as then you have a much warmer chip and if the workload doesn't change you will just be switching between running at the highest cpu frequency or lowest which makes for a choppy experience. A longer passive cooling zone allows the temperature of the chip to be regulated using only passive methods but without drastic performance drops.

I am doing things a bit differently in my implementation as I setup a passive cooling zone for each cpu frequency, but that is just so you can have more control from userspace by changing the different passive trip points.

Quoted 8 lines

Quoted 1 line

-Jon

Jon,

I can agree with leaving a Max-20C passive delta. What do you think about the critical threshold of Max-5C and rule of not allowing it to be changed?

Tim,

I definitely agree that the Critical temp should be a fixed point. Is the purpose of lowering the critical threshold from the hardware default, to allow Linux to shutdown more cleanly rather than just have the hardware shutting down? If that is the case then I think that is fine. If it is to protect the SOC then that is unnecessary. We have heated the SOCs to well beyond the critical threshold and they have survived just fine.

This is a bit out of context but here is the formula I am using to figure out my trip points. By default I use a linear set of trip points for passive cooling.
https://github.com/linux4kix/linux-linaro-stable-mx6/commit/212c17d543739a5fe0bd75b66c10f05177e8bcb0

The short of it is I set a trip delta of 6C and then figure out the lowest passive trip point as Critical - (#passive trip points * trip delta), where each cpu frequency stage is a passive trip point. This will allow an 800Mhz SOC with 2 trip points to run at full speed longer than a 1.2Ghz with 4 trip points. The idea being that the higher the clock rate means we will generate more heat and have more passive cooling levels so it is better to drop the top speed of the CPU earlier in order to let the passive cooling be effective and find a steady state.

Quoted 3 lines

This may be a bit over the top but has fixed problems where long running processes would build up heat and eventually cause a thermal shutdown, but doesn't completely cripple the faster SOCs.

Jon,

Yes - the purpose of lowering the critical threshold from the hardware default is to allow Linux to shutdown more cleanly.

If you agree with the fact that the patch here offers the improvement of using OTG temperature grade as a basis can you ack it and if you feel that the thresholds need to be adjusted perhaps propose a follow-on patch? I feel people can debate the temperature delta's endlessly but what I was really after here was to fix the fact that all the processors are not temperature graded equally because they are packaged differently (metal case on automotive offering better thermal conductivity vs plastic case on consumer)

Regards,

Tim

These changes need to be made to enable the canbus in the device-tree. By default we have those pins assigned as GPIO. As soon as I have the device-tree overlay patches pushed this configuration will be more dynamic, however now you must disable and enable the different iomux pin functionality by hand in the device-tree.

arch/arm/boot/dts/imx6qdl-hummingboard.dtsi +4 -3

index 7dcae42..308de69 100644
@@ -168,7 +168,7 @@
 &flexcan1 {
        pinctrl-names = "default";
        pinctrl-0 = <&pinctrl_hummingboard_flexcan1>;
-       status = "disabled";
+       status = "okay";
 };
 
 &hdmi_core {

@@ -278,8 +278,9 @@
                                 MX6QDL_PAD_EIM_DA8__GPIO3_IO08 0x400130b1
                                 MX6QDL_PAD_EIM_DA7__GPIO3_IO07 0x400130b1
                                 MX6QDL_PAD_EIM_DA6__GPIO3_IO06 0x400130b1
-                                MX6QDL_PAD_SD3_CMD__GPIO7_IO02 0x400130b1
-                                MX6QDL_PAD_SD3_CLK__GPIO7_IO03 0x400130b1
+/*                               MX6QDL_PAD_SD3_CMD__GPIO7_IO02 0x400130b1
+ *                               MX6QDL_PAD_SD3_CLK__GPIO7_IO03 0x400130b1
+ */
                                 MX6QDL_PAD_EIM_DA3__GPIO3_IO03 0x400130b1
                         >;
                 };

Quoted 97 lines · Tim Harvey

Quoted 75 lines · Jon Nettleton

Quoted 40 lines · Tim Harvey

Quoted 39 lines · Jon Nettleton

Quoted 10 lines · Shawn Guo

Quoted 9 lines · Tim Harvey

The IMX6Q/IMX6DL SoC's have a 2-bit temperature grade stored in OTP which is valid for all IMX6 SoC's (despite the fact that the IMXSDLRM and IMXSXRM do not document this - this has been proven via tests as well as verified by Freescale FAE).

Instead of assuming a fixed 85C for passive cooling threshold and 105C for critical use the thermal grade for these configurations.

We will set the critical to maxT - 5C and passive to maxT - 10C.

I would like to chime in here if you don't mind. I have been carrying a patch similar to this in the SolidRun repo to fix cooling issues that we have had. I would recommend keeping the passive temp at maxT
- 20C due to the thermal properties of the chip. I have found that around 85-90C we can maintain a relatively steady thermal state with only passive cooling. Generally with a hard non-NEON based cpu workload the iMX6 will level off at about 87C with all the cores clocked to 1Ghz, and sometimes dipping down to 800Mhz periodically. With a NEON based workload on all the cores it will push beyond this and generally end up finding steady state at about 800Mhz right around 90C.

If you raise the initial passive threshold by 10C it will allow enough heat to build up in the chip that the only way to avoid reaching critical temps is by dropping the CPU down to its lowest frequency. This is not the best experience as then you have a much warmer chip and if the workload doesn't change you will just be switching between running at the highest cpu frequency or lowest which makes for a choppy experience. A longer passive cooling zone allows the temperature of the chip to be regulated using only passive methods but without drastic performance drops.

I am doing things a bit differently in my implementation as I setup a passive cooling zone for each cpu frequency, but that is just so you can have more control from userspace by changing the different passive trip points.

Quoted 8 lines

Quoted 1 line

-Jon

Jon,

I can agree with leaving a Max-20C passive delta. What do you think about the critical threshold of Max-5C and rule of not allowing it to be changed?

Tim,

I definitely agree that the Critical temp should be a fixed point. Is the purpose of lowering the critical threshold from the hardware default, to allow Linux to shutdown more cleanly rather than just have the hardware shutting down? If that is the case then I think that is fine. If it is to protect the SOC then that is unnecessary. We have heated the SOCs to well beyond the critical threshold and they have survived just fine.

This is a bit out of context but here is the formula I am using to figure out my trip points. By default I use a linear set of trip points for passive cooling.
https://github.com/linux4kix/linux-linaro-stable-mx6/commit/212c17d543739a5fe0bd75b66c10f05177e8bcb0

The short of it is I set a trip delta of 6C and then figure out the lowest passive trip point as Critical - (#passive trip points * trip delta), where each cpu frequency stage is a passive trip point. This will allow an 800Mhz SOC with 2 trip points to run at full speed longer than a 1.2Ghz with 4 trip points. The idea being that the higher the clock rate means we will generate more heat and have more passive cooling levels so it is better to drop the top speed of the CPU earlier in order to let the passive cooling be effective and find a steady state.

Quoted 3 lines

This may be a bit over the top but has fixed problems where long running processes would build up heat and eventually cause a thermal shutdown, but doesn't completely cripple the faster SOCs.

Jon,

Yes - the purpose of lowering the critical threshold from the hardware default is to allow Linux to shutdown more cleanly.

If you agree with the fact that the patch here offers the improvement of using OTG temperature grade as a basis can you ack it and if you feel that the thresholds need to be adjusted perhaps propose a follow-on patch? I feel people can debate the temperature delta's endlessly but what I was really after here was to fix the fact that all the processors are not temperature graded equally because they are packaged differently (metal case on automotive offering better thermal conductivity vs plastic case on consumer)

Regards,

Tim

Quoted 75 lines · Jon Nettleton

Quoted 40 lines · Tim Harvey

Quoted 39 lines · Jon Nettleton

Quoted 10 lines · Shawn Guo

Quoted 9 lines · Tim Harvey

The IMX6Q/IMX6DL SoC's have a 2-bit temperature grade stored in OTP which is valid for all IMX6 SoC's (despite the fact that the IMXSDLRM and IMXSXRM do not document this - this has been proven via tests as well as verified by Freescale FAE).

Instead of assuming a fixed 85C for passive cooling threshold and 105C for critical use the thermal grade for these configurations.

We will set the critical to maxT - 5C and passive to maxT - 10C.

I would like to chime in here if you don't mind. I have been carrying a patch similar to this in the SolidRun repo to fix cooling issues that we have had. I would recommend keeping the passive temp at maxT
- 20C due to the thermal properties of the chip. I have found that around 85-90C we can maintain a relatively steady thermal state with only passive cooling. Generally with a hard non-NEON based cpu workload the iMX6 will level off at about 87C with all the cores clocked to 1Ghz, and sometimes dipping down to 800Mhz periodically. With a NEON based workload on all the cores it will push beyond this and generally end up finding steady state at about 800Mhz right around 90C.

If you raise the initial passive threshold by 10C it will allow enough heat to build up in the chip that the only way to avoid reaching critical temps is by dropping the CPU down to its lowest frequency. This is not the best experience as then you have a much warmer chip and if the workload doesn't change you will just be switching between running at the highest cpu frequency or lowest which makes for a choppy experience. A longer passive cooling zone allows the temperature of the chip to be regulated using only passive methods but without drastic performance drops.

I am doing things a bit differently in my implementation as I setup a passive cooling zone for each cpu frequency, but that is just so you can have more control from userspace by changing the different passive trip points.

Quoted 8 lines

Quoted 1 line

-Jon

Jon,

I can agree with leaving a Max-20C passive delta. What do you think about the critical threshold of Max-5C and rule of not allowing it to be changed?

Tim,

I definitely agree that the Critical temp should be a fixed point. Is the purpose of lowering the critical threshold from the hardware default, to allow Linux to shutdown more cleanly rather than just have the hardware shutting down? If that is the case then I think that is fine. If it is to protect the SOC then that is unnecessary. We have heated the SOCs to well beyond the critical threshold and they have survived just fine.

This is a bit out of context but here is the formula I am using to figure out my trip points. By default I use a linear set of trip points for passive cooling.
https://github.com/linux4kix/linux-linaro-stable-mx6/commit/212c17d543739a5fe0bd75b66c10f05177e8bcb0

The short of it is I set a trip delta of 6C and then figure out the lowest passive trip point as Critical - (#passive trip points * trip delta), where each cpu frequency stage is a passive trip point. This will allow an 800Mhz SOC with 2 trip points to run at full speed longer than a 1.2Ghz with 4 trip points. The idea being that the higher the clock rate means we will generate more heat and have more passive cooling levels so it is better to drop the top speed of the CPU earlier in order to let the passive cooling be effective and find a steady state.

Quoted 3 lines

This may be a bit over the top but has fixed problems where long running processes would build up heat and eventually cause a thermal shutdown, but doesn't completely cripple the faster SOCs.

Jon,

Yes - the purpose of lowering the critical threshold from the hardware default is to allow Linux to shutdown more cleanly.

If you agree with the fact that the patch here offers the improvement of using OTG temperature grade as a basis can you ack it and if you feel that the thresholds need to be adjusted perhaps propose a follow-on patch? I feel people can debate the temperature delta's endlessly but what I was really after here was to fix the fact that all the processors are not temperature graded equally because they are packaged differently (metal case on automotive offering better thermal conductivity vs plastic case on consumer)

Regards,

Tim

The IMX6Q/IMX6DL SoC's have a 2-bit temperature grade stored in OTP. Instead of assuming 85C for passive cooling threshold and 105C for critical use the thermal grade for these configurations. For IMX6SX which does not have a temperature grade in OTP (according to the ref manual) assume 105C critical. We will default the user-settable passive threshold to crit - 20C.

Cc Anson Huang <[email protected]>

Cc Fabio Estevam <[email protected]>

Cc Shawn Guo <[email protected]>

Signed-off-by Tim Harvey <[email protected]>

---
 drivers/thermal/imx_thermal.c | 64 +++++++++++++++++++++++++++++++++----------
 1 file changed, 49 insertions(+), 15 deletions(-)

drivers/thermal/imx_thermal.c +49 -16

index 2ccbc07..c6a4eed 100644
@@ -55,6 +55,7 @@
 #define TEMPSENSE2_PANIC_VALUE_SHIFT   16
 #define TEMPSENSE2_PANIC_VALUE_MASK    0xfff0000
 
+#define OCOTP_MEM0                     0x0480
 #define OCOTP_ANA1                     0x04e0
 
 /* The driver supports 1 passive trip point and 1 critical trip point */

@@ -64,12 +65,6 @@ enum imx_thermal_trip {
        IMX_TRIP_NUM,
 };
 
-/*
- * It defines the temperature in millicelsius for passive trip point
- * that will trigger cooling action when crossed.
- */
-#define IMX_TEMP_PASSIVE               85000
-
 #define IMX_POLLING_DELAY              2000 /* millisecond */
 #define IMX_PASSIVE_DELAY              1000
 

@@ -106,6 +101,7 @@ struct imx_thermal_data {
        int irq;
        struct clk *thermal_clk;
        const struct thermal_soc_data *socdata;
+       const char *temp_grade;
 };
 
 static void imx_set_panic_temp(struct imx_thermal_data *data,

@@ -289,7 +285,8 @@ static int imx_set_trip_temp(struct thermal_zone_device *tz, int trip,
        if (trip == IMX_TRIP_CRITICAL)
                return -EPERM;
 
-       if (temp > IMX_TEMP_PASSIVE)
+       /* do not allow passive to be set higher than critical - 20C */
+       if (temp > (data->temp_critical - (1000 * 20)))
                return -EINVAL;
 
        data->temp_passive = temp;

@@ -404,17 +401,50 @@ static int imx_get_sensor_data(struct platform_device *pdev)
        data->c1 = temp64;
        data->c2 = n1 * data->c1 + 1000 * t1;
 
-       /*
-        * Set the default passive cooling trip point,
-        * can be changed from userspace.
-        */
-       data->temp_passive = IMX_TEMP_PASSIVE;
+       /* Assume Extended Commercial temperature grade limits (105C max) */
+       data->temp_grade = "unknown";
+       data->temp_critical = 105000;
+
+       /* For IMX6Q use OTP for thermal grade */
+       if (data->socdata->version == TEMPMON_IMX6Q) {
+               ret = regmap_read(map, OCOTP_MEM0, &val);
+               if (ret) {
+                       dev_err(&pdev->dev, "failed to read temp grade: %d\n",
+                               ret);
+                       return ret;
+               }
+
+               if (val == 0 || val == ~0) {
+                       dev_err(&pdev->dev, "invalid temp grade data\n");
+                       return -EINVAL;
+               }
+
+               /* The maximum die temp is specified by the Temperature Grade */
+               switch ((val >> 6) & 0x3) {
+               case 0: /* Commercial (0 to 95C) */
+                       data->temp_grade = "Commercial";
+                       data->temp_critical = 95000;
+                       break;
+               case 1: /* Extended Commercial (-20 to 105C) */
+                       data->temp_grade = "Extended Commercial";
+                       data->temp_critical = 105000;
+                       break;
+               case 2: /* Industrial (-40 to 105C) */
+                       data->temp_grade = "Industrial";
+                       data->temp_critical = 105000;
+                       break;
+               case 3: /* Automotive (-40 to 125C) */
+                       data->temp_grade = "Automotive";
+                       data->temp_critical = 125000;
+                       break;
+               }
+       }
 
        /*
-        * The maximum die temperature set to 20 C higher than
-        * IMX_TEMP_PASSIVE.
+        * Set the default passive cooling trip point to critical - 20C
+        * (can be changed from userspace)
         */
-       data->temp_critical = 1000 * 20 + data->temp_passive;
+       data->temp_passive = data->temp_critical - (1000 * 20);
 
        return 0;
 }

@@ -559,6 +589,10 @@ static int imx_thermal_probe(struct platform_device *pdev)
                return ret;
        }
 
+       dev_info(&pdev->dev, "%s CPU temperature grade - "
+                "thresholds: crit:%ldC passive:%ldC\n", data->temp_grade,
+                data->temp_critical / 1000, data->temp_passive / 1000);
+
        /* Enable measurements at ~ 10 Hz */
        regmap_write(map, TEMPSENSE1 + REG_CLR, TEMPSENSE1_MEASURE_FREQ);
        measure_freq = DIV_ROUND_UP(32768, 10); /* 10 Hz */
--

1.9.1

Quoted 128 lines · Tim Harvey

The IMX6Q/IMX6DL SoC's have a 2-bit temperature grade stored in OTP. Instead of assuming 85C for passive cooling threshold and 105C for critical use the thermal grade for these configurations. For IMX6SX which does not have a temperature grade in OTP (according to the ref manual) assume 105C critical. We will default the user-settable passive threshold to crit - 20C.

Cc Anson Huang <[email protected]>

Cc Fabio Estevam <[email protected]>

Cc Shawn Guo <[email protected]>

Signed-off-by Tim Harvey <[email protected]>

---
 drivers/thermal/imx_thermal.c | 64 +++++++++++++++++++++++++++++++++----------
 1 file changed, 49 insertions(+), 15 deletions(-)

drivers/thermal/imx_thermal.c +49 -16

index 2ccbc07..c6a4eed 100644
@@ -55,6 +55,7 @@
 #define TEMPSENSE2_PANIC_VALUE_SHIFT   16
 #define TEMPSENSE2_PANIC_VALUE_MASK    0xfff0000
 
+#define OCOTP_MEM0                     0x0480
 #define OCOTP_ANA1                     0x04e0
 
 /* The driver supports 1 passive trip point and 1 critical trip point */

@@ -64,12 +65,6 @@ enum imx_thermal_trip {
        IMX_TRIP_NUM,
 };
 
-/*
- * It defines the temperature in millicelsius for passive trip point
- * that will trigger cooling action when crossed.
- */
-#define IMX_TEMP_PASSIVE               85000
-
 #define IMX_POLLING_DELAY              2000 /* millisecond */
 #define IMX_PASSIVE_DELAY              1000
 

@@ -106,6 +101,7 @@ struct imx_thermal_data {
        int irq;
        struct clk *thermal_clk;
        const struct thermal_soc_data *socdata;
+       const char *temp_grade;
 };
 
 static void imx_set_panic_temp(struct imx_thermal_data *data,

@@ -289,7 +285,8 @@ static int imx_set_trip_temp(struct thermal_zone_device *tz, int trip,
        if (trip == IMX_TRIP_CRITICAL)
                return -EPERM;
 
-       if (temp > IMX_TEMP_PASSIVE)
+       /* do not allow passive to be set higher than critical - 20C */
+       if (temp > (data->temp_critical - (1000 * 20)))
                return -EINVAL;
 
        data->temp_passive = temp;

@@ -404,17 +401,50 @@ static int imx_get_sensor_data(struct platform_device *pdev)
        data->c1 = temp64;
        data->c2 = n1 * data->c1 + 1000 * t1;
 
-       /*
-        * Set the default passive cooling trip point,
-        * can be changed from userspace.
-        */
-       data->temp_passive = IMX_TEMP_PASSIVE;
+       /* Assume Extended Commercial temperature grade limits (105C max) */
+       data->temp_grade = "unknown";
+       data->temp_critical = 105000;
+
+       /* For IMX6Q use OTP for thermal grade */
+       if (data->socdata->version == TEMPMON_IMX6Q) {
+               ret = regmap_read(map, OCOTP_MEM0, &val);
+               if (ret) {
+                       dev_err(&pdev->dev, "failed to read temp grade: %d\n",
+                               ret);
+                       return ret;
+               }
+
+               if (val == 0 || val == ~0) {
+                       dev_err(&pdev->dev, "invalid temp grade data\n");
+                       return -EINVAL;
+               }
+
+               /* The maximum die temp is specified by the Temperature Grade */
+               switch ((val >> 6) & 0x3) {
+               case 0: /* Commercial (0 to 95C) */
+                       data->temp_grade = "Commercial";
+                       data->temp_critical = 95000;
+                       break;
+               case 1: /* Extended Commercial (-20 to 105C) */
+                       data->temp_grade = "Extended Commercial";
+                       data->temp_critical = 105000;
+                       break;
+               case 2: /* Industrial (-40 to 105C) */
+                       data->temp_grade = "Industrial";
+                       data->temp_critical = 105000;
+                       break;
+               case 3: /* Automotive (-40 to 125C) */
+                       data->temp_grade = "Automotive";
+                       data->temp_critical = 125000;
+                       break;
+               }
+       }
 
        /*
-        * The maximum die temperature set to 20 C higher than
-        * IMX_TEMP_PASSIVE.
+        * Set the default passive cooling trip point to critical - 20C
+        * (can be changed from userspace)
         */
-       data->temp_critical = 1000 * 20 + data->temp_passive;
+       data->temp_passive = data->temp_critical - (1000 * 20);
 
        return 0;
 }

@@ -559,6 +589,10 @@ static int imx_thermal_probe(struct platform_device *pdev)
                return ret;
        }
 
+       dev_info(&pdev->dev, "%s CPU temperature grade - "
+                "thresholds: crit:%ldC passive:%ldC\n", data->temp_grade,
+                data->temp_critical / 1000, data->temp_passive / 1000);
+
        /* Enable measurements at ~ 10 Hz */
        regmap_write(map, TEMPSENSE1 + REG_CLR, TEMPSENSE1_MEASURE_FREQ);
        measure_freq = DIV_ROUND_UP(32768, 10); /* 10 Hz */
--

Quoted 3 lines

1.9.1

I think this is a fine start and we can build off it.

Acked-by Jon Nettleton <[email protected]>