Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752393AbdGVVsh (ORCPT ); Sat, 22 Jul 2017 17:48:37 -0400 Received: from gloria.sntech.de ([95.129.55.99]:34924 "EHLO gloria.sntech.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751314AbdGVVsg (ORCPT ); Sat, 22 Jul 2017 17:48:36 -0400 From: Heiko Stuebner To: Caesar Wang Cc: linux-rockchip@lists.infradead.org, rocky.hao@rock-chips.com, Douglas Anderson , William wu , Elaine Zhang , Kever Yang , Brian Norris , linux-kernel@vger.kernel.org, Shawn Lin , devicetree@vger.kernel.org, Rob Herring , linux-arm-kernel@lists.infradead.org, Will Deacon , Mark Rutland , Catalin Marinas , Roger Chen Subject: Re: [PATCH v2 5/5] arm64: dts: rockchip: update the thermal zones for RK3399 SoCs Date: Sat, 22 Jul 2017 23:48:16 +0200 Message-ID: <1703106.KNjF1lku9f@phil> User-Agent: KMail/5.2.3 (Linux/4.9.0-2-amd64; KDE/5.28.0; x86_64; ; ) In-Reply-To: <1500279271-15249-6-git-send-email-wxt@rock-chips.com> References: <1500279271-15249-1-git-send-email-wxt@rock-chips.com> <1500279271-15249-6-git-send-email-wxt@rock-chips.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4774 Lines: 170 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 > > --- > > 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 @@ > ; > 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