[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-id: <57B3B3E4.4020500@samsung.com>
Date: Wed, 17 Aug 2016 09:46:28 +0900
From: Chanwoo Choi <cw00.choi@...sung.com>
To: Krzysztof Kozlowski <k.kozlowski@...sung.com>
Cc: kgene@...nel.org, robh+dt@...nel.org, mark.rutland@....com,
catalin.marinas@....com, will.deacon@....com,
devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-samsung-soc@...r.kernel.org, linux-kernel@...r.kernel.org,
krzk@...nel.org, jh80.chung@...sung.com, sw0312.kim@...sung.com,
jy0922.shim@...sung.com, inki.dae@...sung.com,
jonghwa3.lee@...sung.com, beomho.seo@...sung.com,
jaewon02.kim@...sung.com, human.hwang@...sung.com,
ideal.song@...sung.com, ingi2.kim@...sung.com,
m.szyprowski@...sung.com, a.hajda@...sung.com,
s.nawrocki@...sung.com, chanwoo@...nel.org
Subject: Re: [PATCH 5/7] arm64: dts: exynos: Add dts files for Samsung
Exynos5433 64bit SoC
Hi Krzysztof,
On 2016년 08월 17일 02:51, Krzysztof Kozlowski wrote:
> On Tue, Aug 16, 2016 at 09:59:26PM +0900, Chanwoo Choi wrote:
>>>> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tmu.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tmu.dtsi
>>>> new file mode 100644
>>>> index 000000000000..175121db367e
>>>> --- /dev/null
>>>> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tmu.dtsi
>>>> @@ -0,0 +1,306 @@
>>>> +/*
>>>> + * Device tree sources for Exynos5433 thermal zone
>>>> + *
>>>> + * Copyright (c) 2016 Chanwoo Choi <cw00.choi@...sung.com>
>>>> + *
>>>> + * This program is free software; you can redistribute it and/or modify
>>>> + * it under the terms of the GNU General Public License version 2 as
>>>> + * published by the Free Software Foundation.
>>>> + */
>>>> +
>>>> +#include <dt-bindings/thermal/thermal.h>
>>>> +
>>>> +/ {
>>>> +thermal-zones {
>>>> + atlas0_thermal: atlas0-thermal {
>>>> + thermal-sensors = <&tmu_atlas0>;
>>>> + polling-delay-passive = <0>;
>>>> + polling-delay = <0>;
>>>> + trips {
>>>> + atlas0_alert_0: atlas0-alert-0 {
>>>> + temperature = <50000>; /* millicelsius */
>>>> + hysteresis = <1000>; /* millicelsius */
>>>> + type = "active";
>>>> + };
>>>> + atlas0_alert_1: atlas0-alert-1 {
>>>> + temperature = <55000>; /* millicelsius */
>>>> + hysteresis = <1000>; /* millicelsius */
>>>> + type = "active";
>>>> + };
>>>> + atlas0_alert_2: atlas0-alert-2 {
>>>> + temperature = <60000>; /* millicelsius */
>>>> + hysteresis = <1000>; /* millicelsius */
>>>> + type = "active";
>>>> + };
>>>> + atlas0_alert_3: atlas0-alert-3 {
>>>> + temperature = <70000>; /* millicelsius */
>>>> + hysteresis = <1000>; /* millicelsius */
>>>> + type = "active";
>>>> + };
>>>> + atlas0_alert_4: atlas0-alert-4 {
>>>> + temperature = <80000>; /* millicelsius */
>>>> + hysteresis = <1000>; /* millicelsius */
>>>> + type = "active";
>>>> + };
>>>> + atlas0_alert_5: atlas0-alert-5 {
>>>> + temperature = <90000>; /* millicelsius */
>>>> + hysteresis = <1000>; /* millicelsius */
>>>> + type = "active";
>>>> + };
>>>> + atlas0_alert_6: atlas0-alert-6 {
>>>> + temperature = <95000>; /* millicelsius */
>>>> + hysteresis = <1000>; /* millicelsius */
>>>> + type = "active";
>>>> + };
>>>
>>> No critical trip? I think it might be useful to shutdown the system in a
>>> user-friendly way.
>>
>> When I use the critical trip, the following event occur[1].
>> But, I guess that this temperature is not correct temperature
>> because after completing the kernel booting, the temperature of big.LITTLE/G3D
>> are normal when checking the /sys/class/thermal/thermal_zoneX/temp right after booting.
>> - Maintain a uniform temperature(38 ~ 45 millicelsius) right after kernel booting.
>>
>> I guess that the critical interrupt may occur before initializing the exynos tmu.
>> But, I don't spend the many time to check the exynos-tmu.c driver.
>>
>> [1]
>> [ 445.122122] thermal thermal_zone0: critical temperature reached(108 C),shutting down
>> [ 445.122399] exynos-tmu 10060000.tmu: Temperature sensor ID: 0xa
>> [ 445.122588] exynos-tmu 10060000.tmu: Calibration type is 2-point calibration
>> [ 445.127942] reboot: Failed to start orderly shutdown: forcing the issue
>> [ 445.134586] Emergency Sync complete
>> [ 1.097954] reboot: Power down
>
> I understand. Apparently the exynos-tmu driver needs some fixes for
> this race. Skipping critical then makes sense.
>
>>
>>>
>>>> + };
>>>> +
>>>> + cooling-maps {
>>>> + map0 {
>>>> + /* Set maximum frequency as 1800MHz */
>>>> + trip = <&atlas0_alert_0>;
>>>> + cooling-device = <&cpu4 1 1>;
>>>
>>> Out of curiosity: why choosing specific cooling level (so quite fast
>>> the device will slow down) instead of letting cooling framework to
>>> decide how much to cool? Any particular reason behind this?
>>
>> This cooling level is just default value in cooling-maps.
>> This value is able to overwrite on dts file.
>>
>> And the thermal subsystem support the cpu cooling features
>> with 'cooling-maps'.
>>
>> Also, when I tested the performance and stress test
>> with GLBenchmark, the temperature of big.LITTLE cores/G3D
>> reach easily the critical temperature with 8 online cores.
>> So, I add the cooling level aggressively to protect
>> the system fault of CPU and GPU and to maintain
>> the system state.
>
> I was asking why you do not let cooling framework decide which cooling
> level to use but instead you force a specific cooling level. Maybe code
> will be a better example. Why not use:
> map0 {
> /* Set maximum frequency as 1800MHz */
> trip = <&atlas0_alert_0>;
> cooling-device = <&cpu4 0 1>;
> }
> map1 {
> /* Set maximum frequency as 1700MHz */
> trip = <&atlas0_alert_1>;
> cooling-device = <&cpu4 1 2>;
> };
>
> For higher frequencies it makes even more sense:
> map6 {
> /* Set maximum frequency as 800MHz */
> trip = <&atlas0_alert_6>;
> cooling-device = <&cpu4 7 11>;
> };
>
> which allows the system to use suitable cooling level to maintain the
> balance between performance and temperature dissipance, instead of some
> fixed cooling level which might not be accurate to the system load.
Ah. I misunderstand of your question. OK. I'll expand the range of
cooling level as you suggested.
Best Regards,
Chanwoo Choi
Powered by blists - more mailing lists