Re: [PATCH v2 4/9] of: Add bindings of hw throttle for soctherm

[Wei Ni]

On 2016å04æ28æ 07:30, Eduardo Valentin wrote:
> From: Eduardo Valentin <edubezval@xxxxxxxxx>
> To: Wei Ni <wni@xxxxxxxxxx>
> Cc: thierry.reding@xxxxxxxxx, robh+dt@xxxxxxxxxx, rui.zhang@xxxxxxxxx,
> 	MLongnecker@xxxxxxxxxx, swarren@xxxxxxxxxxxxx,
> 	mikko.perttunen@xxxxxxxx, linux-tegra@xxxxxxxxxxxxxxx,
> 	linux-pm@xxxxxxxxxxxxxxx, devicetree@xxxxxxxxxxxxxxx,
> 	linux-kernel@xxxxxxxxxxxxxxx
> Bcc: 
> Subject: Re: [PATCH v2 4/9] of: Add bindings of hw throttle for soctherm
> Reply-To: 
> In-Reply-To: <1461727554-15065-5-git-send-email-wni@xxxxxxxxxx>
> 
> The patch title must say something about the fact that this is specific
> to nvidia thermal driver.

Yes, it's my mistake, will fix it in next series.

> 
> On Wed, Apr 27, 2016 at 11:25:49AM +0800, Wei Ni wrote:
>> Add HW throttle configuration sub-node for soctherm, which
>> is used to describe the throttle event, and worked as a
>> cooling device. The "hot" type trip in thermal zone can
>> be bound to this cooling device, and trigger the throttle
>> function.
>>
>> Signed-off-by: Wei Ni <wni@xxxxxxxxxx>
>> ---
>>  .../bindings/thermal/nvidia,tegra124-soctherm.txt  | 89 +++++++++++++++++++++-
>>  1 file changed, 87 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/thermal/nvidia,tegra124-soctherm.txt b/Documentation/devicetree/bindings/thermal/nvidia,tegra124-soctherm.txt
>> index edebfa0a985e..dc337d139f49 100644
>> --- a/Documentation/devicetree/bindings/thermal/nvidia,tegra124-soctherm.txt
>> +++ b/Documentation/devicetree/bindings/thermal/nvidia,tegra124-soctherm.txt
>> @@ -10,8 +10,14 @@ Required properties :
>>  - compatible : For Tegra124, must contain "nvidia,tegra124-soctherm".
>>    For Tegra132, must contain "nvidia,tegra132-soctherm".
>>    For Tegra210, must contain "nvidia,tegra210-soctherm".
>> -- reg : Should contain 1 entry:
>> +- reg : Should contain at least 2 entries for each entry in reg-names:
>>    - SOCTHERM register set
>> +  - Tegra CAR register set: Required for Tegra124 and Tegra210.
>> +  - CCROC register set: Required for Tegra132.
>> +- reg-names :  Should contain at least 2 entries:
>> +  - soctherm-reg
>> +  - car-reg
>> +  - ccroc-reg
>>  - interrupts : Defines the interrupt used by SOCTHERM
>>  - clocks : Must contain an entry for each entry in clock-names.
>>    See ../clocks/clock-bindings.txt for details.
>> @@ -25,17 +31,44 @@ Required properties :
>>  - #thermal-sensor-cells : Should be 1. See ./thermal.txt for a description
>>      of this property. See <dt-bindings/thermal/tegra124-soctherm.h> for a
>>      list of valid values when referring to thermal sensors.
>> +- throttle-cfgs: A sub-node which is a container of configuration for each
>> +    hardware throttle events. These events can be set as cooling devices.
>> +  * throttle events: Sub-nodes must be named as "light" or "heavy".
>> +      Properties:
>> +      - priority: Each throttles has its own throttle settings, so the SW need
>> +        to set priorities for various throttle, the HW arbiter can select the
>> +        final throttle settings.
>> +        Bigger value indicates higher priority, In general, higher priority
>> +        translates to lower target frequency. SW needs to ensure that critical
>> +        thermal alarms are given higher priority, and ensure that there is
>> +        no race if priority of two vectors is set to the same value.
>> +      - cpu-throt-depth:  This property is for Tegra124 and Tegra210. It is
>> +        the throttling depth of pulse skippers, it's the percentage
>> +        throttling.
>> +      - cpu-throt-level: This property is only for Tegra132, it is the level
>> +        of pulse skippers, which used to throttle clock frequencies. It
>> +        indicates cpu clock throttling depth, and the depth can be programmed.
>> +        Must set as following values:
>> +        TEGRA_SOCTHERM_THROT_LEVEL_LOW, TEGRA_SOCTHERM_THROT_LEVEL_MED
>> +        TEGRA_SOCTHERM_THROT_LEVEL_HIGH, TEGRA_SOCTHERM_THROT_LEVEL_NONE
> 
> These properties are not generic properties. My understanding is that
> you must have vendor prefix in such case. Same applies to the new nodes.

Ok, will do it.

> 
>> +      - #cooling-cells: Should be 1. This cooling device only support on/off state.
>> +        See ./thermal.txt for a description of this property.
>>  
>>  Note:
>>  - the "critical" type trip points will be set to SOC_THERM hardware as the
>>  shut down temperature. Once the temperature of this thermal zone is higher
>>  than it, the system will be shutdown or reset by hardware.
>> +- the "hot" type trip points will be set to SOC_THERM hardware as the throttle
>> +temperature. Once the the temperature of this thermal zone is higher
>> +than it, it will trigger the HW throttle event.
>>  
>>  Example :
>>  
>>  	soctherm@700e2000 {
>>  		compatible = "nvidia,tegra124-soctherm";
>> -		reg = <0x0 0x700e2000 0x0 0x1000>;
>> +		reg = <0x0 0x700e2000 0x0 0x600  /* SOC_THERM reg_base */
>> +			0x0 0x60006000 0x0 0x400 /* CAR reg_base */
>> +		reg-names = "soctherm-reg", "car-reg";
>>  		interrupts = <GIC_SPI 48 IRQ_TYPE_LEVEL_HIGH>;
>>  		clocks = <&tegra_car TEGRA124_CLK_TSENSOR>,
>>  			<&tegra_car TEGRA124_CLK_SOC_THERM>;
>> @@ -44,6 +77,45 @@ Example :
>>  		reset-names = "soctherm";
>>  
>>  		#thermal-sensor-cells = <1>;
>> +
>> +		throttle-cfgs {
>> +			throttle_heavy: heavy {
>> +				priority = <100>;
>> +				cpu-throt-depth = <85>;
>> +
>> +				#cooling-cells = <1>;
>> +			};
>> +			throttle_light: light {
>> +				priority = <80>;
>> +				cpu-throt-depth = <50>;
>> +
>> +				#cooling-cells = <1>;
>> +			};
>> +		};
> 
> This is a sensor, which at the same time, has sub nodes that can be
> cooling devices. Is my understanding correct of what you are trying to
> do?

Yes, we set this sub nodes as cooling devices.
Let me explain it.
The Tegra's soctherm has the HW throttle function, it will be triggered when the
temperatures reach to the "hot" type trip. These throttle events can be
configured to different levels "heavy" and "light" on different sensors. So it
something like cooling devices which can be bound with thermal zones, and in
this way, we can use the linux of-thermal framework to handle it. If not, we
have to add more codes to parse these configuration and set them for those
sensors. I had used this method to handle the shutdown function in Tegra's
soctherm driver in my previous series, however Rob didn't like it, he preferred
to use linux framework to handle it.
So I think it's better to set these throttle-cfgs as cooling devices, so they
can be bound to the cpu/gpu thermal zones easily.

> 
> 
>> +	};
>> +
>> +Example: referring to Tegra132's "reg", "reg-names" and "throttle-cfgs" :
>> +
>> +	soctherm@0,700e2000 {
>> +		compatible = "nvidia,tegra132-soctherm";
>> +		reg = <0x0 0x700e2000 0x0 0x600  /* SOC_THERM reg_base */
>> +			0x0 0x70040000 0x0 0x200>; /* CCROC reg_base */;
>> +		reg-names = "soctherm-reg", "ccroc-reg";
>> +
>> +		throttle-cfgs {
>> +			throttle_heavy: heavy {
>> +				priority = <100>;
>> +				cpu-throt-level = <TEGRA_SOCTHERM_THROT_LEVEL_HIGH>;
>> +
>> +				#cooling-cells = <1>;
>> +			};
>> +			throttle_light: light {
>> +				priority = <80>;
>> +				cpu-throt-level = <TEGRA_SOCTHERM_THROT_LEVEL_MED>;
>> +
>> +				#cooling-cells = <1>;
> 
> Could you please describe a little here what to expect of the above
> setup?

Ok, let me describe it.
When the "heavy" cooling device triggered, the hardware will skip cpu clock's
pulse in HIGH level, and if the "light" triggered, the hardware will skip cpu
clock's pulse in MED level.
If these two devices are triggered in same time, the HW throttle arbiter will
select the highest priority as the final throttle settings to skip cpu pulse.

> 
> 
>> +			};
>> +		};
>>  	};
>>  
>>  Example: referring to thermal sensors :
>> @@ -62,6 +134,19 @@ Example: referring to thermal sensors :
>>  					hysteresis = <1000>;
>>  					type = "critical";
>>  				};
>> +
>> +				cpu_throttle_trip: throttle-trip {
>> +					temperature = <100000>;
>> +					hysteresis = <1000>;
>> +					type = "hot";
>> +				};
>> +			};
>> +
>> +			cooling-maps {
>> +				map0 {
>> +					trip = <&cpu_throttle_trip>;
>> +					cooling-device = <&throttle_heavy 1 1>;
>> +				};
>>  			};
>>                  };
>>  	};
>> -- 
>> 1.9.1
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>