Message-ID: <11480a47-47da-c5b3-edad-c2dd8edb2cd9@arm.com>
Date:   Fri, 19 Mar 2021 18:05:50 +0000
From:   Robin Murphy <robin.murphy@....com>
To:     Daniel Lezcano <daniel.lezcano@...aro.org>, heiko@...ech.de
Cc:     Rob Herring <robh+dt@...nel.org>, Johan Jonker <jbx6244@...il.com>,
        Helen Koike <helen.koike@...labora.com>,
        Shunqian Zheng <zhengsq@...k-chips.com>,
        Jacob Chen <jacob2.chen@...k-chips.com>,
        Chen-Yu Tsai <wens@...e.org>,
        Markus Reichl <m.reichl@...etechno.de>,
        Boris Brezillon <boris.brezillon@...labora.com>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        "moderated list:ARM/Rockchip SoC support" 
        <linux-arm-kernel@...ts.infradead.org>,
        "open list:ARM/Rockchip SoC support" 
        <linux-rockchip@...ts.infradead.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] dt: rockchip: rk3399: Add dynamic power coefficient for
 GPU

On 2021-03-19 14:35, Daniel Lezcano wrote:
> 
> Hi Robin,
> 
> On 19/03/2021 13:17, Robin Murphy wrote:
>> On 2021-03-19 11:05, Daniel Lezcano wrote:
>>> The DTPM framework is looking for upstream SoC candidates to share the
>>> power numbers.
>>>
>>> We can see around different numbers but the one which seems to be
>>> consistent with the initial post for the values on the CPUs can be
>>> found in the patch https://lore.kernel.org/patchwork/patch/810159/
>>
>> The kernel hacker in me would be more inclined to trust the BSP that the
>> vendor actively supports than a 5-year-old patch that was never pursued
>> upstream. Apparently that was last updated more recently:
>>
>> https://github.com/rockchip-linux/kernel/commit/98d4505e1bd62ff028bd79fbd8284d64b6f468f8
> 
> Yes, I've seen this value also.
> 
>> The ex-mathematician in me can't even comment either way without
>> evidence that whatever model expects to consume this value is even
>> comparable to whatever "arm,mali-simple-power-model" is. >
>> The way the
>> latter apparently needs an explicit "static" coefficient as well as a
>> "dynamic" one, and the value here being nearly 3 times that of a
>> similarly-named one in active use downstream (ChromeOS appears to still
>> be using the values from before the above commit), certainly incline me
>> to think they may not be...
> 
> Sorry, I'm missing the point :/
> 
> We dropped in the kernel any static power computation because as there
> was no value, the resulting code was considered dead. So we rely on the
> dynamic power only.

Right, so a 2-factor model is clearly not identical to a 1-factor model, 
so how do we know that a value for one is valid for the other, even if 
it happens to have a similar name? I'm not saying that it is or isn't; I 
don't know. If someone can point to the downstream coefficient 
definition being identical to the upstream one then great, let's use 
that as justification. If not, then the justification of one arbitrary 
meaningless number over any other is a bit misleading.

>>> I don't know the precision of this value but it is better than
>>> nothing.
>>
>> But is it? If it leads to some throttling mechanism kicking in and
>> crippling GPU performance because it's massively overestimating power
>> consumption, that would be objectively worse for most users, no?
> 
> No because there is no sustainable power specified for the thermal zones
> related to the GPU.
OK, that's some reassurance at least. Does the exact value have any 
material effect? If not, what's to stop us from using an obviously 
made-up value like 1, and saying so?

Robin.

Powered by blists - more mailing lists