[PATCH] dt: rockchip: rk3399: Add dynamic power coefficient for GPU

Subsystems: the rest

STALE1961d

5 messages, 2 authors, 2021-03-19 · open the first message on its own page

[PATCH] dt: rockchip: rk3399: Add dynamic power coefficient for GPU

From: Daniel Lezcano <hidden>
Date: 2021-03-19 11:06:35

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/

I don't know the precision of this value but it is better than
nothing.

Hopefully, one day SoC vendors will be more generous with the power
numbers at least for the SoC which are from the previous generation
and give the community the opportunity to develop power based
frameworks.
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 1 +
 1 file changed, 1 insertion(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index edbbf35fe19e..1ab1d293d2e9 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -1933,6 +1933,7 @@
 		interrupt-names = "job", "mmu", "gpu";
 		clocks = <&cru ACLK_GPU>;
 		#cooling-cells = <2>;
+		dynamic-power-coefficient = <977>;
 		power-domains = <&power RK3399_PD_GPU>;
 		status = "disabled";
 	};
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] dt: rockchip: rk3399: Add dynamic power coefficient for GPU

From: Robin Murphy <robin.murphy@arm.com>
Date: 2021-03-19 12:18:01

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

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...
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?

Robin.
quoted hunk
Hopefully, one day SoC vendors will be more generous with the power
numbers at least for the SoC which are from the previous generation
and give the community the opportunity to develop power based
frameworks.
---
  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 1 +
  1 file changed, 1 insertion(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index edbbf35fe19e..1ab1d293d2e9 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -1933,6 +1933,7 @@
  		interrupt-names = "job", "mmu", "gpu";
  		clocks = <&cru ACLK_GPU>;
  		#cooling-cells = <2>;
+		dynamic-power-coefficient = <977>;
  		power-domains = <&power RK3399_PD_GPU>;
  		status = "disabled";
  	};
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] dt: rockchip: rk3399: Add dynamic power coefficient for GPU

From: Daniel Lezcano <hidden>
Date: 2021-03-19 14:36:26

Hi Robin,

On 19/03/2021 13:17, Robin Murphy wrote:
On 2021-03-19 11:05, Daniel Lezcano wrote:
quoted
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.
quoted
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.



-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] dt: rockchip: rk3399: Add dynamic power coefficient for GPU

From: Robin Murphy <robin.murphy@arm.com>
Date: 2021-03-19 18:06:48

On 2021-03-19 14:35, Daniel Lezcano wrote:
Hi Robin,

On 19/03/2021 13:17, Robin Murphy wrote:
quoted
On 2021-03-19 11:05, Daniel Lezcano wrote:
quoted
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.
quoted
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.
quoted
quoted
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.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] dt: rockchip: rk3399: Add dynamic power coefficient for GPU

From: Daniel Lezcano <hidden>
Date: 2021-03-19 18:39:26

On 19/03/2021 19:05, Robin Murphy wrote:
On 2021-03-19 14:35, Daniel Lezcano wrote:
quoted
Hi Robin,

On 19/03/2021 13:17, Robin Murphy wrote:
quoted
On 2021-03-19 11:05, Daniel Lezcano wrote:
quoted
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.
quoted
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.
That's a call :)
quoted
quoted
quoted
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? 
Yes, it has when it is combined with other devices having also power
values, like the CPUs and hopefully the DMC soon.

If we can have more or less consistent power numbers for the DMC, CPU
and GPU on the rock960, with the thermal zone having these three heating
sources, we can use the DTPM framework to act on the power of the whole.

I don't know the best coefficient, 733, 977 or 1780 [1]

The value of 977 sound to me as a starting point.



[1]
https://chromium.googlesource.com/chromiumos/third_party/kernel/+/factory-gru-8652.B-chromeos-4.4/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin-r3.dts
If not, what's to stop us from using an obviously
made-up value like 1, and saying so?




-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help