diff mbox

[v2,5/5] arm64: dts: rockchip: update the thermal zones for RK3399 SoCs

Message ID 1500279271-15249-6-git-send-email-wxt@rock-chips.com (mailing list archive)
State New, archived
Headers show

Commit Message

Caesar Wang July 17, 2017, 8:14 a.m. UTC
As RK3399 had used the Power allocator thermal governor by default,
enabled this to manage thermals by dynamically allocating and limiting
power to devices.

Also, this patch supported the dynamic-power-coefficient/sustainable_power
and GPU's power model for needed parameters with thermal IPA.

The Thermal power allocator governor works optimatly with two passive trip
points, for the better performance we will use the trip-point0 with 70
degree above which the governor control starts operating and trip-point1
with 85 degree is the target temperature by controlling.

Signed-off-by: Caesar Wang <wxt@rock-chips.com>

---

Changes in v2:
- foo@ will produce warnings when used without reg property.
- update the commit to explain the two passive trip points changed.

 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 62 +++++++++++++++-----------------
 1 file changed, 29 insertions(+), 33 deletions(-)

Comments

Heiko Stuebner July 22, 2017, 9:48 p.m. UTC | #1
Hi Caesar,

Am Montag, 17. Juli 2017, 16:14:31 CEST schrieb Caesar Wang:
> As RK3399 had used the Power allocator thermal governor by default,
> enabled this to manage thermals by dynamically allocating and limiting
> power to devices.
> 
> Also, this patch supported the dynamic-power-coefficient/sustainable_power
> and GPU's power model for needed parameters with thermal IPA.
> 
> The Thermal power allocator governor works optimatly with two passive trip
> points, for the better performance we will use the trip-point0 with 70
> degree above which the governor control starts operating and trip-point1
> with 85 degree is the target temperature by controlling.
> 
> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
> 
> ---
> 
> Changes in v2:
> - foo@ will produce warnings when used without reg property.
> - update the commit to explain the two passive trip points changed.
> 
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 62 +++++++++++++++-----------------
>  1 file changed, 29 insertions(+), 33 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> index 77d67cb..6d8a5eb 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -147,7 +147,7 @@
>  			enable-method = "psci";
>  			#cooling-cells = <2>; /* min followed by max */
>  			clocks = <&cru ARMCLKB>;
> -			dynamic-power-coefficient = <100>;
> +			dynamic-power-coefficient = <436>;
>  		};
>  
>  		cpu_b1: cpu@101 {
> @@ -156,7 +156,7 @@
>  			reg = <0x0 0x101>;
>  			enable-method = "psci";
>  			clocks = <&cru ARMCLKB>;
> -			dynamic-power-coefficient = <100>;
> +			dynamic-power-coefficient = <436>;

Adjusting the coefficients should be a separate patch and the
commit message should explain how they were calculated and
why they are the exacter ones over the old values.


>  		};
>  	};
>  
> @@ -690,24 +690,25 @@
>  	};
>  
>  	thermal_zones: thermal-zones {
> -		cpu_thermal: cpu {
> +		soc_thermal: soc-thermal {
>  			polling-delay-passive = <100>;
>  			polling-delay = <1000>;
> +			sustainable-power = <1000>;
>  
>  			thermal-sensors = <&tsadc 0>;
>  
>  			trips {
> -				cpu_alert0: cpu_alert0 {
> +				threshold: trip-point0 {
>  					temperature = <70000>;
>  					hysteresis = <2000>;
>  					type = "passive";
>  				};
> -				cpu_alert1: cpu_alert1 {
> -					temperature = <75000>;
> +				target: trip-point1 {
> +					temperature = <85000>;
>  					hysteresis = <2000>;
>  					type = "passive";
>  				};
> -				cpu_crit: cpu_crit {
> +				soc_crit: soc-crit {
>  					temperature = <95000>;
>  					hysteresis = <2000>;
>  					type = "critical";
> @@ -716,45 +717,31 @@
>  
>  			cooling-maps {
>  				map0 {
> -					trip = <&cpu_alert0>;
> +					trip = <&target>;

still both maps use &target as trip point. Is that intentional
and if so, why is the &threshold trip point never referenced?


>  					cooling-device =
> -						<&cpu_b0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> +						<&cpu_l0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> +					contribution = <4096>;
>  				};
>  				map1 {
> -					trip = <&cpu_alert1>;
> +					trip = <&target>;
>  					cooling-device =
> -						<&cpu_l0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
>  						<&cpu_b0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> +					contribution = <1024>;
> +				};
> +				map2 {
> +					trip = <&target>;
> +					cooling-device =
> +						<&gpu THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> +					contribution = <4096>;
>  				};
>  			};
>  		};
>  
> -		gpu_thermal: gpu {
> +		gpu_thermal: gpu-thermal {
>  			polling-delay-passive = <100>;
>  			polling-delay = <1000>;
>  
>  			thermal-sensors = <&tsadc 1>;
> -
> -			trips {
> -				gpu_alert0: gpu_alert0 {
> -					temperature = <75000>;
> -					hysteresis = <2000>;
> -					type = "passive";
> -				};
> -				gpu_crit: gpu_crit {
> -					temperature = <95000>;
> -					hysteresis = <2000>;
> -					type = "critical";
> -				};
> -			};
> -
> -			cooling-maps {
> -				map0 {
> -					trip = <&gpu_alert0>;
> -					cooling-device =
> -						<&cpu_b0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> -				};
> -			};
>  		};
>  	};
>  
> @@ -1451,8 +1438,17 @@
>  			     <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH 0>;
>  		interrupt-names = "gpu", "job", "mmu";
>  		clocks = <&cru ACLK_GPU>;
> +		#cooling-cells = <2>;
>  		power-domains = <&power RK3399_PD_GPU>;
>  		status = "disabled";
> +
> +		gpu_power_model: power_model {
> +			compatible = "arm,mali-simple-power-model";
> +			static-coefficient = <1079403>;
> +			dynamic-coefficient = <977>;
> +			ts = <32000 4700 (-80) 2>;
> +			thermal-zone = "gpu-thermal";
> +		};

You might want to have the gpu thermal work without the
power-model-thingy for now, so most likely just drop that
gpu-related change for now.


Heiko
Caesar Wang July 25, 2017, 9:15 a.m. UTC | #2
Hi Heiko,

Thanks your comments.

在 2017年07月23日 05:48, Heiko Stuebner 写道:

> Hi Caesar,
>
> Am Montag, 17. Juli 2017, 16:14:31 CEST schrieb Caesar Wang:
>> As RK3399 had used the Power allocator thermal governor by default,
>> enabled this to manage thermals by dynamically allocating and limiting
>> power to devices.
>>
>> Also, this patch supported the dynamic-power-coefficient/sustainable_power
>> and GPU's power model for needed parameters with thermal IPA.
>>
>> The Thermal power allocator governor works optimatly with two passive trip
>> points, for the better performance we will use the trip-point0 with 70
>> degree above which the governor control starts operating and trip-point1
>> with 85 degree is the target temperature by controlling.
>>
>> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
>>
>> ---
>>
>> Changes in v2:
>> - foo@ will produce warnings when used without reg property.
>> - update the commit to explain the two passive trip points changed.
>>
>>   arch/arm64/boot/dts/rockchip/rk3399.dtsi | 62 +++++++++++++++-----------------
>>   1 file changed, 29 insertions(+), 33 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> index 77d67cb..6d8a5eb 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> @@ -147,7 +147,7 @@
>>   			enable-method = "psci";
>>   			#cooling-cells = <2>; /* min followed by max */
>>   			clocks = <&cru ARMCLKB>;
>> -			dynamic-power-coefficient = <100>;
>> +			dynamic-power-coefficient = <436>;
>>   		};
>>   
>>   		cpu_b1: cpu@101 {
>> @@ -156,7 +156,7 @@
>>   			reg = <0x0 0x101>;
>>   			enable-method = "psci";
>>   			clocks = <&cru ARMCLKB>;
>> -			dynamic-power-coefficient = <100>;
>> +			dynamic-power-coefficient = <436>;
> Adjusting the coefficients should be a separate patch and the
> commit message should explain how they were calculated and
> why they are the exacter ones over the old values.

Okay, i don't know why the dynamic-power-coefficient is 100 for 
b-cluster before.:-)

>
>
>>   		};
>>   	};
>>   
>> @@ -690,24 +690,25 @@
>>   	};
>>   
>>   	thermal_zones: thermal-zones {
>> -		cpu_thermal: cpu {
>> +		soc_thermal: soc-thermal {
>>   			polling-delay-passive = <100>;
>>   			polling-delay = <1000>;
>> +			sustainable-power = <1000>;
>>   
>>   			thermal-sensors = <&tsadc 0>;
>>   
>>   			trips {
>> -				cpu_alert0: cpu_alert0 {
>> +				threshold: trip-point0 {
>>   					temperature = <70000>;
>>   					hysteresis = <2000>;
>>   					type = "passive";
>>   				};
>> -				cpu_alert1: cpu_alert1 {
>> -					temperature = <75000>;
>> +				target: trip-point1 {
>> +					temperature = <85000>;
>>   					hysteresis = <2000>;
>>   					type = "passive";
>>   				};
>> -				cpu_crit: cpu_crit {
>> +				soc_crit: soc-crit {
>>   					temperature = <95000>;
>>   					hysteresis = <2000>;
>>   					type = "critical";
>> @@ -716,45 +717,31 @@
>>   
>>   			cooling-maps {
>>   				map0 {
>> -					trip = <&cpu_alert0>;
>> +					trip = <&target>;
> still both maps use &target as trip point. Is that intentional
> and if so, why is the &threshold trip point never referenced?

For the power allocator governor, the &threshold trip point just control 
starts operating, not need for map.
For other governor (e.g: step_wise) will need the first trip point. 
Looks like we have to think about how to support them.


>   		};
>   	};
>   
> @@ -1451,8 +1438,17 @@
>   			     <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH 0>;
>   		interrupt-names = "gpu", "job", "mmu";
>   		clocks = <&cru ACLK_GPU>;
> +		#cooling-cells = <2>;
>   		power-domains = <&power RK3399_PD_GPU>;
>   		status = "disabled";
> +
> +		gpu_power_model: power_model {
> +			compatible = "arm,mali-simple-power-model";
> +			static-coefficient = <1079403>;
> +			dynamic-coefficient = <977>;
> +			ts = <32000 4700 (-80) 2>;
> +			thermal-zone = "gpu-thermal";
> +		};
> You might want to have the gpu thermal work without the
> power-model-thingy for now, so most likely just drop that
> gpu-related change for now.

Well,  I guess the mediatek/hisilicon/rockchip ARM SoCs should support 
the gpu's power mode with private kernel,
not in the upstream kernel.

-Caesar

>
>
> Heiko
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
diff mbox

Patch

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 77d67cb..6d8a5eb 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -147,7 +147,7 @@ 
 			enable-method = "psci";
 			#cooling-cells = <2>; /* min followed by max */
 			clocks = <&cru ARMCLKB>;
-			dynamic-power-coefficient = <100>;
+			dynamic-power-coefficient = <436>;
 		};
 
 		cpu_b1: cpu@101 {
@@ -156,7 +156,7 @@ 
 			reg = <0x0 0x101>;
 			enable-method = "psci";
 			clocks = <&cru ARMCLKB>;
-			dynamic-power-coefficient = <100>;
+			dynamic-power-coefficient = <436>;
 		};
 	};
 
@@ -690,24 +690,25 @@ 
 	};
 
 	thermal_zones: thermal-zones {
-		cpu_thermal: cpu {
+		soc_thermal: soc-thermal {
 			polling-delay-passive = <100>;
 			polling-delay = <1000>;
+			sustainable-power = <1000>;
 
 			thermal-sensors = <&tsadc 0>;
 
 			trips {
-				cpu_alert0: cpu_alert0 {
+				threshold: trip-point0 {
 					temperature = <70000>;
 					hysteresis = <2000>;
 					type = "passive";
 				};
-				cpu_alert1: cpu_alert1 {
-					temperature = <75000>;
+				target: trip-point1 {
+					temperature = <85000>;
 					hysteresis = <2000>;
 					type = "passive";
 				};
-				cpu_crit: cpu_crit {
+				soc_crit: soc-crit {
 					temperature = <95000>;
 					hysteresis = <2000>;
 					type = "critical";
@@ -716,45 +717,31 @@ 
 
 			cooling-maps {
 				map0 {
-					trip = <&cpu_alert0>;
+					trip = <&target>;
 					cooling-device =
-						<&cpu_b0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+						<&cpu_l0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+					contribution = <4096>;
 				};
 				map1 {
-					trip = <&cpu_alert1>;
+					trip = <&target>;
 					cooling-device =
-						<&cpu_l0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
 						<&cpu_b0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+					contribution = <1024>;
+				};
+				map2 {
+					trip = <&target>;
+					cooling-device =
+						<&gpu THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+					contribution = <4096>;
 				};
 			};
 		};
 
-		gpu_thermal: gpu {
+		gpu_thermal: gpu-thermal {
 			polling-delay-passive = <100>;
 			polling-delay = <1000>;
 
 			thermal-sensors = <&tsadc 1>;
-
-			trips {
-				gpu_alert0: gpu_alert0 {
-					temperature = <75000>;
-					hysteresis = <2000>;
-					type = "passive";
-				};
-				gpu_crit: gpu_crit {
-					temperature = <95000>;
-					hysteresis = <2000>;
-					type = "critical";
-				};
-			};
-
-			cooling-maps {
-				map0 {
-					trip = <&gpu_alert0>;
-					cooling-device =
-						<&cpu_b0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
-				};
-			};
 		};
 	};
 
@@ -1451,8 +1438,17 @@ 
 			     <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH 0>;
 		interrupt-names = "gpu", "job", "mmu";
 		clocks = <&cru ACLK_GPU>;
+		#cooling-cells = <2>;
 		power-domains = <&power RK3399_PD_GPU>;
 		status = "disabled";
+
+		gpu_power_model: power_model {
+			compatible = "arm,mali-simple-power-model";
+			static-coefficient = <1079403>;
+			dynamic-coefficient = <977>;
+			ts = <32000 4700 (-80) 2>;
+			thermal-zone = "gpu-thermal";
+		};
 	};
 
 	pinctrl: pinctrl {