On 09/29/2017 05:46 PM, Daniel Lezcano wrote:

Hi Valentin,

On 29/09/2017 13:07, Valentin Schneider wrote:

[ ... ]

quoted

I've tested this on HiKey960 (Android 4.9 + upstream patches to apply
your thermal/drivers/hisi series + Kevin's hi3600 support).
I ran a video workload, and noticed I get several interrupts while
passive cooling is already in effect
(I might move part of this discussion to Kevin's posting, but I think
it's still relevant to be here):

[  118.107357] hisi_thermal fff30000.tsensor: THERMAL ALARM: 70495 > 65000
[  119.182531] hisi_thermal fff30000.tsensor: THERMAL ALARM: 76235 > 65000
[  119.361964] hisi_thermal fff30000.tsensor: THERMAL ALARM: 70495 > 65000
[  119.907865] hisi_thermal fff30000.tsensor: THERMAL ALARM: 75620 > 65000
[  119.959076] hisi_thermal fff30000.tsensor: THERMAL ALARM: 70700 > 65000

This isn't optimal for IPA, as the PID is supposed to use a specific
sampling rate, but those interrupts
forced a re-trigger of power_allocator_throttle which changes the PID's
actual sampling rate.
IPA isn't expecting this kind of scenarios, as I can see a
tz->passive_delay in the computation
of the derivative term (although the derivative coefficient defaults to
0...).

Interesting. This patch actually is for the hikey, not for the hikey960.
I didn't tested with Kevin's patches.

The thermal characteristics are very different on hi960. For example,
the temperature increase is must faster on hikey960. So I guess, the
driver itself should be fine tuned to prevent this kind of scenario.

True

quoted

In a perfect world I would see those interrupts being toggled by the
thermal governor, as that is
where we know what to do with each trip point - we could still want the
interrupts, but in the case
of IPA we'd like to disable them while the PID controller is active, and
we would know when to re-enable them
(as soon as IPA is toggled off).

Yes, that is true the thermal framework could be enhanced to support this.

If you're up for it, it could be an interesting discussion. In any case, 
I might spend a little bit of time looking into making such an 
interrupt-toggling work (at least as a prototype just on the HiKey960).

Do you have an hikey (not 960) to test this patch?

I do have one but I'm currently lending it to someone else in my team, 
I'll have to track it down (probably next week).
I'm actually quite interested in having IPA triggered by interrupt 
because of the HiKey960's tendency to heat up insanely fast, which leads 
to IPA starting when the temperature is already way too high. I started 
toying around with Kevin's patches but saw your own patch and thought it 
was related, so I figured I would share some of my thoughts before 
starting to work on something someone else might already be working on.

I should perhaps move some of those comments to Kevin's posting - sorry 
for the noise.