Message-ID: <0ec01666-a3b6-1204-60a9-20e1107db81c@codeaurora.org>
Date:   Wed, 2 Sep 2020 11:11:05 +0530
From:   Rajendra Nayak <rnayak@...eaurora.org>
To:     Matthias Kaehlcke <mka@...omium.org>,
        Doug Anderson <dianders@...omium.org>
Cc:     Andy Gross <agross@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Amit Kucheria <amit.kucheria@...aro.org>,
        Sai Prakash Ranjan <saiprakash.ranjan@...eaurora.org>
Subject: Re: [PATCH] arm64: dts: qcom: sc7180: Add 'sustainable_power' for CPU
 thermal zones


>> I'm not massively familiar with this area of the code, but I guess I
>> shouldn't let that stop me from having an opinion!  :-P
>>
>> * I would agree that it seems highly unlikely that someone would put
>> one of these chips in a device that could only dissipate the heat from
>> the lowest OPP, so having some higher estimate definitely makes sense.
>>
>> * In terms of the numbers here, I believe that you're claiming that we
>> can dissipate 768 mW * 6 + 1202 mW * 2 = ~7 Watts of power.
> 
> No, I'm claiming it's 768 mW + 1202 mW = ~2 W.
> 
> SC7180 has a 6 thermal zones for the 6 little cores and 4 zones for the
> 2 big cores. Each of these thermal zones uses either all little or all big
> cores as cooling devices, hence the power sustainable power of the
> individual zones doesn't add up. 768 mW corresponds to 6x 128 mW (aka all
> little cores at 1.8 GHz), and 1202 mW to 2x 601 mW (both big cores at 1.9 GHz).
> 
>> My memory
>> of how much power we could dissipate in previous laptops I worked on
>> is a little fuzzy, but that doesn't seem insane for a passively-cooled
>> laptop.  However, I think someone could conceivably put this chip in a
>> smaller form factor.  In such a case, it seems like we'd want these
>> things to sum up to ~2000 (if it would ever make sense for someone to
>> put this chip in a phone) or ~4000 (if it would ever make sense for
>> someone to put this chip in a small tablet).
> 
> See above, the sustainable power with this patch only adds up to ~2000.
> It is possible though that it would be lower in a smaller form factor
> device.
> 
> I'd be ok with posting something lower for SC7180 (it would be a guess
> though) and use the specific numbers in the device specific DT.
> 
>> It seems possible that,
>> to achieve this, we might have to tweak the
>> "dynamic-power-coefficient".  I don't know how much thought was put
>> into those numbers, but the fact that the little cores have a super
>> round 100 for their dynamic-power-coefficient makes me feel like they
>> might have been more schwags than anything.  Rajendra maybe knows?
> 
> Yeah, it's possible that that was just an approximation

No, these are based on actual power measurements.

> 
>> * I'm curious about the fact that there are two numbers here: one for
>> littles and one for bigs.  If I had to guess I'd say that since all
>> the cores are in one package so the contributions kinda need to be
>> thought of together, right?  If we're sitting there thermally
>> throttled then we'd want to pick the best perf-per-watt for the
>> overall package.  This is why your patch says we can sustain the
>> little cores at max and the big cores get whatever is left over,
>> right?
> 
> It's derived from how Qualcomm specified the thermal zones and cooling
> devices. Any ("cpu") zone is either cooled by (all) big cores or by (all)
> little cores, but not a mix of them. In my tests I also saw that the big
> cores seemed to have little impact on the little ones. The little cores
> are at max because even running at max frequency the temperature in the
> 'little zones' wouldn't come close to the trip point.
> 
>> * Should we be leaving some room in here for the GPU?  ...or I guess
>> once we list it as a cooling device we'll have to decrease the amount
>> the CPUs can use?
> 
> I don't know for sure, but judging from the CPU zones I wouldn't be
> surprised if the GPU was managed exclusively in the dedicated GPU
> thermal zones (I guess that's what 'gpuss0-thermal' and 'gpuss1-thermal'
> are). If that's not the case the values in the CPU zones can be
> adjusted when specific data is available.
> 
>> So I guess the tl; dr is:
>>
>> a) We should check "dynamic-power-coefficient" and possibly adjust.
> 
> ok, lets see if Rajendra can check if there is room for tweaking.

I suggest we don't :)

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Powered by blists - more mailing lists