All of lore.kernel.org
 help / color / mirror / Atom feed
From: "H. Nikolaus Schaller" <hns@goldelico.com>
To: Adam Ford <aford173@gmail.com>
Cc: Linux-OMAP <linux-omap@vger.kernel.org>,
	"Tony Lindgren" <tony@atomide.com>,
	"André Roth" <neolynx@gmail.com>,
	"Discussions about the Letux Kernel"
	<letux-kernel@openphoenux.org>,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	"Andreas Kemnade" <andreas@kemnade.info>,
	"Nishanth Menon" <nm@ti.com>, "Adam Ford" <adam.ford@logicpd.com>,
	kernel@pyra-handheld.com
Subject: Re: [RFC] ARM: dts: omap36xx: Enable thermal throttling
Date: Fri, 13 Sep 2019 16:24:18 +0200	[thread overview]
Message-ID: <ABCE2ACA-D19A-42D2-9606-C60F1A5CBCCB@goldelico.com> (raw)
In-Reply-To: <CAHCN7xL-CmwmXP3PLdwAHiC-9tMjrpY4k7ZhxQ9WoXY6yUz8BA@mail.gmail.com>


> Am 13.09.2019 um 16:05 schrieb Adam Ford <aford173@gmail.com>:
> 
> On Fri, Sep 13, 2019 at 8:32 AM H. Nikolaus Schaller <hns@goldelico.com> wrote:
>> 
>> Hi Adam,
>> 
>>> Am 13.09.2019 um 13:07 schrieb Adam Ford <aford173@gmail.com>:
>> 
>>>>> +     cpu_cooling_maps: cooling-maps {
>>>>> +             map0 {
>>>>> +                     trip = <&cpu_alert0>;
>>>>> +                     /* Only allow OPP50 and OPP100 */
>>>>> +                     cooling-device = <&cpu 0 1>;
>>>> 
>>>> omap4-cpu-thermal.dtsi uses THERMAL_NO_LIMIT constants but I do not
>>>> understand their meaning (and how it relates to the opp list).
>>> 
>>> I read through the documentation, but it wasn't completely clear to
>>> me. AFAICT, the numbers after &cpu represent the min and max index in
>>> the OPP table when the condition is hit.
>> 
>> Ok. It seems to use "cooling state" for those and the first is minimum
>> and the last is maximum. Using THERMAL_NO_LIMIT (-1UL) means to have
>> no limits.
>> 
>> Since here we use the &cpu node it is likely that the "cooling state"
>> is the same as the OPP index currently in use.
>> 
>> I have looked through the .dts which use cpu_crit and the picture is
>> not unique...
>> 
>> omap4           seems to only define it
>> am57xx          has two different grade dtsi files
>> dra7            overwrites critical temperature value
>> am57xx-beagle   defines a gpio to control a fan
> 
> Checkout rk3288-veyron-mickey.dts
> 
> They have almost_warm, warm, almost_hot, hot, hotter, very_hot, and
> critical for trips, and they have as many corresponding cooling maps
> which appear to limit the CPU speeds, but their index references are
> still confusing to me.

Seems to be quite sophistcated.

The arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi also has a lot
of trip points. So there may be very different needs...

But it has potentially helpful comments...

				/* 
				 * When reaching cpu0_alert3, reduce CPU
				 * by 2 steps. On Exynos5422/5800 that would
				 * be: 1600 MHz and 1100 MHz.
				 */
				map3 {
					trip = <&cpu0_alert3>;
					cooling-device = <&cpu0 0 2>;
				};
				map4 {
					trip = <&cpu0_alert3>;
					cooling-device = <&cpu4 0 2>;
				};
				/*
				 * When reaching cpu0_alert4, reduce CPU
				 * further, down to 600 MHz (12 steps for big,
				 * 7 steps for LITTLE).
				 */
				map5 {
					trip = <&cpu0_alert4>;
					cooling-device = <&cpu0 3 7>;
				};
				map6 {
					trip = <&cpu0_alert4>;
					cooling-device = <&cpu4 3 12>;
				};

That would mean the second integer is something about how
many steps to reduce.

But the first is not explained.

BTW: this also demonstrates how a single trip point can map to multiple
cooling-device actions (something we likely do not need).

> 
> For that device,
> Warm and no limit first, then 4:   coolling-device = <&cpu0 THERMAL_NO_LIMIT 4>
> ...
> very_hot uses a number then no limit: cooling-device = <&cpu0 8
> THERMAL_NO_LIMIT>
> 
> This makes me wonder if the min and max are switched or the index
> values go backwards.

It may depend on the specific cpu driver? Maybe even omap rk and exynos
have different interpretation in code?

>> 
>> Then we can use the data sheet limits of 90°C and 105°C in the trip point
>> table (which should not be tweaked for sensor inaccuracy).
> 
> I can see not compensating if it reads high, but if the temp reads
> low, shouldn't compensate so we don't over temp the processor?

I just mean that we must ensure that the TJ is <= 90° if the bandgap
ever reports 90°. So it may report 10 or 20 or even 30 degrees more than the
real temperature but never less (reaching the critical temperature too early
but not too late).

We can achieve that by adding bias or changing slope etc. in the bandgap sensor
driver.

If I find some time I am curious enough to look into the code and the data
sheets to understand why it is said to be inaccurate... Maybe there is
jitter from some LDO and it needs a median filter?

And why it seems to add a bias of ca. 10° as soon as I read it more than
for the first time. And how well temperature correlates to ambient temperature
(it definitively correlates to cpufreq-set -f).

But we should not modify the trip temperatures by 10 or 20 or 30 degrees.
IMHO they should have the values defined by the data sheet.

BR,
Nikolaus


  reply	other threads:[~2019-09-13 14:24 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-12 18:30 [RFC] ARM: dts: omap36xx: Enable thermal throttling Adam Ford
2019-09-12 21:12 ` Daniel Lezcano
2019-09-12 21:19   ` Adam Ford
2019-09-12 22:33     ` Daniel Lezcano
2019-09-18  9:24       ` Viresh Kumar
2019-09-18  9:36         ` H. Nikolaus Schaller
2019-09-18  9:37         ` Daniel Lezcano
2019-09-13  6:55 ` H. Nikolaus Schaller
2019-09-13 11:07   ` Adam Ford
2019-09-13 13:28     ` Adam Ford
2019-09-13 13:32     ` H. Nikolaus Schaller
2019-09-13 14:05       ` Adam Ford
2019-09-13 14:24         ` H. Nikolaus Schaller [this message]
2019-09-13 15:01           ` Adam Ford
2019-09-13 15:09             ` H. Nikolaus Schaller
2019-09-13 16:35               ` Adam Ford
2019-09-13 16:42                 ` Adam Ford
2019-09-13 16:51                   ` H. Nikolaus Schaller
2019-09-13 17:18                     ` Daniel Lezcano
2019-09-13 18:46                       ` Adam Ford
2019-09-13 20:01                         ` Adam Ford
2019-09-13 20:11                         ` Daniel Lezcano
2019-09-13 20:34                           ` H. Nikolaus Schaller
2019-09-13 20:34                             ` H. Nikolaus Schaller
2019-09-13 21:01                             ` Adam Ford
2019-09-14  9:53                             ` Daniel Lezcano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ABCE2ACA-D19A-42D2-9606-C60F1A5CBCCB@goldelico.com \
    --to=hns@goldelico.com \
    --cc=adam.ford@logicpd.com \
    --cc=aford173@gmail.com \
    --cc=andreas@kemnade.info \
    --cc=kernel@pyra-handheld.com \
    --cc=letux-kernel@openphoenux.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=neolynx@gmail.com \
    --cc=nm@ti.com \
    --cc=tony@atomide.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.