Enable cpuidle work around
The cpuidle work around needs to be added and tested on mainline.
use-cases#75
Activity
The solution (workaround) here will involve Linux and ATF. To update here what I plan to do (sometime until the 25th of june hopefully): Test the latest upstream RFC patches that are being discussed. Prepared the forks/branches:
- https://source.puri.sm/martin.kepplinger/u-boot-builder/commits/atf_test_martin
- https://source.puri.sm/martin.kepplinger/linux-next/commits/cpuidle_testing
- https://source.puri.sm/martin.kepplinger/trusted-firmware-a/commits/librem5
Also, I created the Librem5/trusted-firmware-a repo for us to use and put possible out-of-tree changes in (librem5 branch).
Up until now it's not clear that cpuidle on imx8mq will be supported by mainline linux. That shouldn't be a big deal, at least as long as it's actively discussed ( https://lists.infradead.org/pipermail/linux-arm-kernel/2019-June/658494.html ). Worst case will be that any serious imx8mq user will need an out-of-tree change anyways.
-
I'm happy for hints for how to better test here! I look at interrupts and what powertop says, but powertop doesn't really work for arm and I'm not even sure if we are supposed to see real "CPU wake" interrupts with Abel's workarounds.
So there's one solution posted by Abel from March, and one from June. The older one I have "tested" too, but that's not pushed into my trees. But what I notice is the same for both versions:
A 10x larger number of arch_timer interrupts:
3: 91469 91214 91089 91190 GICv3 30 Level arch_timer IPI0: 5487 9517 10049 11903 Rescheduling interrupts IPI6: 0 0 0 0 CPU wake-up interruptsOn our linux-next branch, without cpuidle enabled, this is:
3: 16542 14617 13282 13843 GICv3 30 Level arch_timer IPI0: 4458 7706 10429 9543 Rescheduling interruptsboth is recorded after 6 minutes of uptime.
of course, "powertop" then has a cpuidle driver and tries to tell "WFI" stats: cpu-sleep: 0%, WFI: 99+%. But again, I'm not sure if that is accurate... temperature recording don't really show a difference.
-
the heating-curve of the workaround:
looks exactly the same as without it:
(30s units on the x axis, different amount of samples, the details don't matter)
So to the code path:
arch/arm64/kernel/process.c'scpu_do_idle()is called constantly:- With the workaround ca. once every 1ms
- Without it ca. every 100us
And we use
__cpu_do_idle()which goes straight to the "wait for interrupt" assembler instruction.Without the workaround, is the WFI call simply not implemented and just returns?
So while we don't see any difference, and even more interrupts (arch_timer, see above), can it be that this workaround actually does what it should do but we simply constantly interrupt all cpus (due to other bugs or missing things)?
-
Appearently, Anson is still working on this, and TIMER_IMX_SYS_CTR needs to be configured-in see https://lkml.org/lkml/2019/6/21/55 plus the necessary support added, see https://lkml.org/lkml/2019/6/21/103 But simply grabbing these, I don't have a cooler SoC yet, but there might be (a clock definition?) missing still...
-
followed up with the upstream RFC: https://lkml.org/lkml/2019/6/23/85
Edited by Martin Kepplinger -
We comare, witout any interpretation, the Librem5/linux-next linux-next branch, with mainline ATF:
purism@pureos:~$ sudo idlestat --trace -f /tmp/mytrace -t 10 -p -c -w Log is 10.000411 secs long with 2042 events -------------------------------------------------------------------------------- | C-state | min | max | avg | total | hits | over | under | -------------------------------------------------------------------------------- | clusterA | -------------------------------------------------------------------------------- | (null) | 0us | 492.77ms | 20.48ms | 9.97s | 487 | 0 | 0 | -------------------------------------------------------------------------------- | cpu0 | -------------------------------------------------------------------------------- | (null) | 18us | 1.02s | 24.13ms | 9.99s | 414 | 0 | 0 | -------------------------------------------------------------------------------- | cpu1 | -------------------------------------------------------------------------------- | (null) | 6us | 671.98ms | 151.42ms | 9.99s | 66 | 0 | 0 | -------------------------------------------------------------------------------- | cpu2 | -------------------------------------------------------------------------------- | (null) | 4us | 1.06s | 151.48ms | 10.00s | 66 | 0 | 0 | -------------------------------------------------------------------------------- | cpu3 | -------------------------------------------------------------------------------- | (null) | 4us | 1.73s | 109.69ms | 9.98s | 91 | 0 | 0 | --------------------------------------------------------------------------------vs. the workaround-patches on linux-next and ATF:
purism@pureos:~$ sudo idlestat --trace -f /tmp/mytrace -t 10 -p -c -w Log is 10.000395 secs long with 31194 events -------------------------------------------------------------------------------- | C-state | min | max | avg | total | hits | over | under | -------------------------------------------------------------------------------- | clusterA | -------------------------------------------------------------------------------- | WFI | 14us | 3.99ms | 3.90ms | 9.93s | 2543 | 0 | 0 | -------------------------------------------------------------------------------- | cpu0 | -------------------------------------------------------------------------------- | WFI | 14us | 3.99ms | 3.89ms | 9.96s | 2561 | 0 | 0 | -------------------------------------------------------------------------------- | cpu1 | -------------------------------------------------------------------------------- | WFI | 35us | 3.99ms | 3.96ms | 9.96s | 2513 | 0 | 0 | -------------------------------------------------------------------------------- | cpu2 | -------------------------------------------------------------------------------- | WFI | 3us | 3.99ms | 3.95ms | 9.96s | 2519 | 0 | 0 | -------------------------------------------------------------------------------- | cpu3 | -------------------------------------------------------------------------------- | WFI | 3us | 3.99ms | 3.95ms | 9.96s | 2523 | 0 | 0 | --------------------------------------------------------------------------------compare IRQs from the unchanged linux-next/mainline ATF:
------------------------------------------------------- | IRQ | Name | Count | early | late | ------------------------------------------------------- | cpu0 | ------------------------------------------------------- | IPI | Reschedulin | 5 | 0 | 0 | | 206 | dcss_drm | 304 | 0 | 0 | | 3 | arch_timer | 17 | 0 | 0 | | 39 | 30be0000.ethern | 10 | 0 | 0 | | 34 | mmc0 | 62 | 0 | 0 | | 31 | 30a20000.i2c | 11 | 0 | 0 | | 38 | 30be0000.ethern | 1 | 0 | 0 | | IPI | Functio | 1 | 0 | 0 | | 41 | 38000000.gpu | 2 | 0 | 0 | | 205 | dcss_ctxld | 2 | 0 | 0 | ------------------------------------------------------- | cpu1 | ------------------------------------------------------- | IPI | Reschedulin | 16 | 0 | 0 | | 3 | arch_timer | 51 | 0 | 0 | ------------------------------------------------------- | cpu2 | ------------------------------------------------------- | 3 | arch_timer | 37 | 0 | 0 | | IPI | Reschedulin | 28 | 0 | 0 | | IPI | Functio | 1 | 0 | 0 | ------------------------------------------------------- | cpu3 | ------------------------------------------------------- | IPI | Reschedulin | 58 | 0 | 0 | | 3 | arch_timer | 33 | 0 | 0 | | IPI | Functio | 1 | 0 | 0 | -------------------------------------------------------with the wakeup-workaround system:
------------------------------------------------------- | IRQ | Name | Count | early | late | ------------------------------------------------------- | cpu0 | ------------------------------------------------------- | IPI | Reschedulin | 11 | 0 | 0 | | 3 | arch_timer | 2505 | 0 | 0 | | 41 | 30be0000.ethern | 11 | 0 | 0 | | 36 | mmc0 | 6 | 0 | 0 | | 33 | 30a20000.i2c | 12 | 0 | 0 | | 40 | 30be0000.ethern | 1 | 0 | 0 | | 43 | 38000000.gpu | 2 | 0 | 0 | | 208 | dcss_drm | 12 | 0 | 0 | | 207 | dcss_ctxld | 2 | 0 | 0 | ------------------------------------------------------- | cpu1 | ------------------------------------------------------- | IPI | Reschedulin | 13 | 0 | 0 | | 3 | arch_timer | 2500 | 0 | 0 | | IPI | Functio | 1 | 0 | 0 | ------------------------------------------------------- | cpu2 | ------------------------------------------------------- | 3 | arch_timer | 2500 | 0 | 0 | | IPI | Reschedulin | 18 | 0 | 0 | | IPI | Functio | 1 | 0 | 0 | ------------------------------------------------------- | cpu3 | ------------------------------------------------------- | IPI | Reschedulin | 22 | 0 | 0 | | 3 | arch_timer | 2501 | 0 | 0 | | IPI | Functio | 1 | 0 | 0 | -------------------------------------------------------and we compare a "stressed" unchanged linux-next system:
purism@pureos:~$ stress --cpu 4 & [1] 1032 purism@pureos:~$ stress: info: [1032] dispatching hogs: 4 cpu, 0 io, 0 vm, 0 hdd purism@pureos:~$ sudo idlestat --trace -f /tmp/mytrace -t 10 -p -c -w Log is 0.000000 secs long with 10900 events -------------------------------------------------------------------------------- | C-state | min | max | avg | total | hits | over | under | -------------------------------------------------------------------------------- ---------------------------------------------------------------- | P-state | min | max | avg | total | hits | ---------------------------------------------------------------- | clusterA | ---------------------------------------------------------------- | 1.50GHz | 10.00s | 10.00s | 10.00s | 10.00s | 1 | ---------------------------------------------------------------- ------------------------------------------------------- | IRQ | Name | Count | early | late | ------------------------------------------------------- | cpu0 | ------------------------------------------------------- | 3 | arch_timer | 1 | 0 | 0 | ------------------------------------------------------- | cpu1 | ------------------------------------------------------- | IPI | Reschedulin | 1 | 0 | 0 | ------------------------------------------------------- | cpu2 | ------------------------------------------------------- | IPI | Reschedulin | 1 | 0 | 0 | ------------------------------------------------------- | cpu3 | ------------------------------------------------------- | IPI | Reschedulin | 1 | 0 | 0 | -------------------------------------------------------to the workaround:
purism@pureos:~$ stress --cpu 4 & purism@pureos:~$ sudo idlestat --trace -f /tmp/mytrace -t 10 -p -c -w Log is 0.000000 secs long with 10958 events -------------------------------------------------------------------------------- | C-state | min | max | avg | total | hits | over | under | -------------------------------------------------------------------------------- ---------------------------------------------------------------- | P-state | min | max | avg | total | hits | ---------------------------------------------------------------- | clusterA | ---------------------------------------------------------------- | 1.50GHz | 10.00s | 10.00s | 10.00s | 10.00s | 1 | ---------------------------------------------------------------- ------------------------------------------------------- | IRQ | Name | Count | early | late | ------------------------------------------------------- | cpu0 | ------------------------------------------------------- | IPI | Reschedulin | 1 | 0 | 0 | ------------------------------------------------------- | cpu1 | ------------------------------------------------------- | IPI | Reschedulin | 1 | 0 | 0 | ------------------------------------------------------- | cpu2 | ------------------------------------------------------- | IPI | Reschedulin | 1 | 0 | 0 | ------------------------------------------------------- | cpu3 | ------------------------------------------------------- | 3 | arch_timer | 1 | 0 | 0 | -------------------------------------------------------looks like we spend most of the time in C1/WFI in total - but not consecutively, due to "arch_timer" (I think).
-
The changes here are: ATF:
and Linux:
https://source.puri.sm/martin.kepplinger/linux-next/tree/cpuidle_testing (which i have rebased for angus too: https://source.puri.sm/martin.kepplinger/linux-next/tree/next-20190626/devkit-for-angus )
We won't see a difference in the heating-curve (according to Abel, see https://lkml.org/lkml/2019/6/28/237 ). What I tried to do now (don't laugh :) this should be more of a side-note): is to use powertop on my Laptop, with the devkit connected, and compare power-consumption -.-:
I do see a slight difference (but really, these are still not hard numbers you can read anything into): with the workaround I increase ca. 4,4 W. Without it (using our Librem5/linux-next, consumption increases ca. 5,1 W. Although I did a few iterations, there were iterations in when I also saw ca. 5 W increase with the workaround. But on average, it seems to consume less power, if I can trust my battery and all.
I asked Abel about "arch_timer" and showed him our idlestat numbers; I hope he can quickly confirm that this makes sense to him.
-
From an upstream point of view, I should be running the needed workaround. I see:
/sys/devices/system/cpu/cpu0/cpuidle$ cat state*/name WFI cpu-sleepbut "cpu-sleep" is never used, see idlestat output above. Actually, it is, but only during startup, about up until DCSS/DRM loading:
[ 0.922674] dcss-core 32e00000.dcss: dcss_submodules_init 178.: ret: 0 [ 0.923621] cpu_pm_enter [ 0.923638] cpu_pm_enter [ 0.923643] cpu_pm_enter return val: 0 [ 0.923646] arm_cpuidle_suspend index 1 cpu 1 [ 0.929480] cpu_pm_enter return val: 0 [ 0.929483] arm_cpuidle_suspend index 1 cpu 2 [ 0.929531] Registering platform device 'imx-dcss-crtc.0'. Parent at 32e000000 .dcss [ 0.929535] device: 'imx-dcss-crtc.0': device_add [ 0.929546] bus: 'platform': add device imx-dcss-crtc.0 [ 0.929573] PM: Adding info for platform:imx-dcss-crtc.0- The irq-imx-gpcv2 driver is loaded correctly.
- (only until ca. that point during startup)
arch/arm64/kernel/cpuidle.c'sarm_cpuidle_suspend()is called viakernel/cpu_pm.c'scpu_pm_enter(). Never again afterwards.
My guess right now would be that we mess things up somewhere in our out-of-tree (drm) changes.
@guido.gunther is there anything in the DRM patches that you think might disable the cpu_pm ?
-
ok. with the following revert, things finally look better:
Revert "librem5-devkit: config: enable dcss" This reverts commit 881c7ac0beb5421608b9ce85db19bcec248438c4. diff --git a/arch/arm64/configs/librem5-devkit_defconfig b/arch/arm64/configs/librem5-devkit_defconfig index 7ded7806e473..a2a0aaf13470 100644 --- a/arch/arm64/configs/librem5-devkit_defconfig +++ b/arch/arm64/configs/librem5-devkit_defconfig @@ -374,7 +374,6 @@ CONFIG_USB_VIDEO_CLASS=m CONFIG_V4L_PLATFORM_DRIVERS=y CONFIG_V4L_MEM2MEM_DRIVERS=y CONFIG_VIDEO_IMX_PXP=m -CONFIG_IMX_DCSS_CORE=y CONFIG_DRM=y CONFIG_DRM_I2C_CH7006=m CONFIG_DRM_I2C_SIL164=m @@ -382,8 +381,6 @@ CONFIG_DRM_I2C_NXP_TDA9950=m CONFIG_DRM_PANEL_SIMPLE=m CONFIG_DRM_PANEL_ROCKTECH_JH057N00900=y CONFIG_DRM_I2C_ADV7511=y -CONFIG_DRM_IMX=y -CONFIG_DRM_IMX_NWL_DSI=y CONFIG_DRM_ETNAVIV=y CONFIG_DRM_MXSFB=y CONFIG_FB_ARMCLCD=y| cpu0 | -------------------------------------------------------------------------------- | WFI | 1us | 384.75ms | 12.44ms | 4.42s | 355 | 1 | 201 | | cpu-sleep | 13us | 3.97ms | 14us | 1.30s | 93298 | 93294 | 0 | -------------------------------------------------------------------------------- | cpu1 | -------------------------------------------------------------------------------- | WFI | 43us | 447.98ms | 13.42ms | 1.58s | 118 | 0 | 94 | | cpu-sleep | 13us | 4.54ms | 14us | 1.53s | 106121 | 106108 | 0 | -------------------------------------------------------------------------------- | cpu2 | -------------------------------------------------------------------------------- | WFI | 3us | 1.43s | 32.31ms | 8.08s | 250 | 0 | 236 | | cpu-sleep | 13us | 4.54ms | 15us | 1.32s | 88946 | 88935 | 0 | -------------------------------------------------------------------------------- | cpu3 | -------------------------------------------------------------------------------- | WFI | 1us | 895.45ms | 14.79ms | 8.05s | 544 | 1 | 509 | | cpu-sleep | 13us | 4.52ms | 14us | 1.31s | 91974 | 91970 | 0 | --------------------------------------------------------------------------------@guido.gunther Angus was faster :) do you have anything in mind what could go wrong already?
@guido.gunther is there anything in the DRM patches that you think might disable the cpu_pm ?
I'm pretty sure there's nothing in th nwl, mixel and panel drivers triggering this. I'd start poking at dcss or irqsteer.
-
thanks. what makes cpu-sleep work, is removing the usage of the "pm_qos_" interface:
--- a/drivers/gpu/imx/dcss/dcss-common.c +++ b/drivers/gpu/imx/dcss/dcss-common.c @@ -495,7 +495,7 @@ static int dcss_suspend(struct device *dev) dcss_clocks_enable(dcss, false); - pm_qos_remove_request(&dcss->pm_qos_req); + //pm_qos_remove_request(&dcss->pm_qos_req); dcss_bus_freq(dcss, false); @@ -512,7 +512,7 @@ static int dcss_resume(struct device *dev) dcss_bus_freq(dcss, true); - pm_qos_add_request(&dcss->pm_qos_req, PM_QOS_CPU_DMA_LATENCY, 0); + //pm_qos_add_request(&dcss->pm_qos_req, PM_QOS_CPU_DMA_LATENCY, 0); dcss_clocks_enable(dcss, true); @@ -537,7 +537,7 @@ static int dcss_runtime_suspend(struct device *dev) dcss_clocks_enable(dcss, false); - pm_qos_remove_request(&dcss->pm_qos_req); + //pm_qos_remove_request(&dcss->pm_qos_req); dcss_bus_freq(dcss, false); @@ -551,7 +551,7 @@ static int dcss_runtime_resume(struct device *dev) dcss_bus_freq(dcss, true); - pm_qos_add_request(&dcss->pm_qos_req, PM_QOS_CPU_DMA_LATENCY, 0); + //pm_qos_add_request(&dcss->pm_qos_req, PM_QOS_CPU_DMA_LATENCY, 0);I don't know that API yet actually. @guido.gunther Do you know that we need this here? What does it do?
(btw. looks like this it will keep the SoC significantly cooler)
-
It looks like the QOS flag will affect how quickly or often the system goes to idle to keep DMA operations happy