lkml.org 
[lkml]   [2019]   [Jul]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [RFC 0/2] Add workaround for core wake-up on IPI for i.MX8MQ
    From
    Date
    On 02.07.19 13:33, Abel Vesa wrote:
    > On 19-07-02 08:47:19, Martin Kepplinger wrote:
    >> On 28.06.19 10:54, Abel Vesa wrote:
    >>> On 19-06-23 13:47:26, Martin Kepplinger wrote:
    >>>> On 10.06.19 14:13, Abel Vesa wrote:
    >>>>> This is another alternative for the RFC:
    >>>>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.org%2Flkml%2F2019%2F3%2F27%2F545&data=02%7C01%7Cabel.vesa%40nxp.com%7Ccfc582f9977d479b7dda08d6feb9258a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636976468485275045&sdata=L%2Byn29%2FBS3KMjm9eCPBTZBTl30PmZywSjIj11bMQw5c%3D&reserved=0
    >>>>>
    >>>>> This new workaround proposal is a little bit more hacky but more contained
    >>>>> since everything is done within the irq-imx-gpcv2 driver.
    >>>>>
    >>>>> Basically, it 'hijacks' the registered gic_raise_softirq __smp_cross_call
    >>>>> handler and registers instead a wrapper which calls in the 'hijacked'
    >>>>> handler, after that calling into EL3 which will take care of the actual
    >>>>> wake up. This time, instead of expanding the PSCI ABI, we use a new vendor SIP.
    >>>>>
    >>>>> I also have the patches ready for TF-A but I'll hold on to them until I see if
    >>>>> this has a chance of getting in.
    >>>>
    >>>> Let's leave out of the picture for now, how generally applicable and
    >>>> mergable your changes are. I'd like to reproduce what you do and test
    >>>> cpuidle on imx8mq:
    >>>>
    >>>> When applying your changes here and the corresponding ATF changes (
    >>>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fabelvesa%2Farm-trusted-firmware%2Ftree%2Fimx8mq-err11171&data=02%7C01%7Cabel.vesa%40nxp.com%7Ccfc582f9977d479b7dda08d6feb9258a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636976468485275045&sdata=VT3duSl70DNxcY8Ev4FFrHlWoOjkcckeM8BgxrSkr8A%3D&reserved=0 if
    >>>> I got that right) I don't yet see any difference in the SoC heating up
    >>>> under zero load. __cpu_do_idle() is called about every 1ms (without your
    >>>> changes, that was even more often but I'm not yet sure if that means
    >>>> anything).
    >>>
    >>> You will most probably not see any change in the SoC temp since the cpuidle
    >>> only touches the A53s. There are way many more IPs in the SoC that could
    >>> heat it up. If you want some real numbers you'll have to measure the power
    >>> consumtion on VDD_ARM rail. If you don't want to go through that much trouble
    >>> you can use the idlestat tool to measure the times each A53 speends in cpu-sleep
    >>> state.
    >>>
    >>>>
    >>>> What I also see is that I get about 10x more "arch_timer" (int.3, GICv3)
    >>>> interrupts than without your changes.
    >>
    >>
    >> thanks for getting back at me here. This is run on the imx8mq
    >> librem5-devkit with your wakeup-workaround applied. Typical measurements
    >> under zero load look like this:
    >>
    >> 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 |
    >> ------------------------------------------------------------------------
    >> ...
    >>
    >
    > I don't see the cpu-sleep state at all in your idlestat log. Maybe the cpuidle
    > isn't enabled. Or probably the workaround itself is not applied. You'll have
    > to look into that.
    >
    > Here is how it looks like with the workaround enabled:
    >
    > Log is 10.001685 secs long with 1175 events
    > --------------------------------------------------------------------------------
    > | C-state | min | max | avg | total | hits | over | under |
    > --------------------------------------------------------------------------------
    > | clusterA |
    > --------------------------------------------------------------------------------
    > | WFI | 2us | 50.04ms | 29.63ms | 9.99s | 337 | 0 | 0 |
    > --------------------------------------------------------------------------------
    > | cpu0 |
    > --------------------------------------------------------------------------------
    > | WFI | 11us | 50.04ms | 40.44ms | 9.62s | 238 | 0 | 219 |
    > | cpu-sleep | 537us | 50.58ms | 14.11ms | 366.94ms | 26 | 7 | 0 |
    > --------------------------------------------------------------------------------
    > | cpu1 |
    > --------------------------------------------------------------------------------
    > | WFI | 11us | 539.04ms | 93.20ms | 5.78s | 62 | 0 | 38 |
    > | cpu-sleep | 536us | 607.90ms | 183.38ms | 4.22s | 23 | 12 | 0 |
    > --------------------------------------------------------------------------------
    > | cpu2 |
    > --------------------------------------------------------------------------------
    > | WFI | 41us | 265.99ms | 17.51ms | 332.66ms | 19 | 0 | 11 |
    > | cpu-sleep | 568us | 6.56s | 1.38s | 9.67s | 7 | 2 | 0 |
    > --------------------------------------------------------------------------------
    > | cpu3 |
    > --------------------------------------------------------------------------------
    > | WFI | 7.94ms | 881.50ms | 367.81ms | 1.10s | 3 | 0 | 3 |
    > | cpu-sleep | 549us | 2.02s | 808.72ms | 8.90s | 11 | 1 | 0 |
    > --------------------------------------------------------------------------------
    >
    > You can see that the cpu2 was once for 6.56 seconds (out of 10s) in cpu-sleep.
    >

    So I run this ATF tree
    https://github.com/abelvesa/arm-trusted-firmware/tree/imx8mq-err11171
    and, on top of "v5.2-rc7" I have your commits
    ("irqchip: irq-imx-gpcv2: Add workaround for i.MX8MQ ERR11171") and
    ("arm64: dts: imx8mq: Add idle states and gpcv2 wake_request broken
    property") applied.

    Then simply enabled CONFIG_ARM_CPUIDLE.

    (I also use the "imx-cpufreq-dt" driver, but this should be unrelated here).

    I do see the possible cpuidle states:
    /sys/devices/system/cpu/cpu0/cpuidle$ cat state*/name

    WFI

    cpu-sleep

    but idlestat doesn't see it or it is (thus) never used. Do you know a
    needed change I might be missing?

    thanks again,
    martin


    \
     
     \ /
      Last update: 2019-07-08 09:55    [W:3.316 / U:0.644 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site