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

[Caesar Wang]

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@xxxxxxxxxxxxxx>
> >
> > ---
> >
> > 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@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-rockchip