Re: ddr devfreq on H3 - possible?

[Samuel Holland <samuel@sholland.org>]

Hi Kirill,

On 12/31/22 05:15, Kirill wrote:
> Hi!
> 
> I ported your patches for armbian kernel 6.1 / u-boot and it works!
> 
>> It works, though it did lock up once after playing with the devfreq
>> sysfs for several minutes
> 
> Yes, I have hangs too. And the main reason for this problem - SMP. :(
> 
> By calling SMC we put only one CPU into SRAM. But other CPUs still
> work and use DRAM!
> I don't see any hangs, if I disable all other CPUs:
> ```
> echo 0 > /sys/devices/system/cpu/cpu1/online
> echo 0 > /sys/devices/system/cpu/cpu2/online
> echo 0 > /sys/devices/system/cpu/cpu3/online
> ```

Thanks for investigating. That's good information.

> Original sunxi 3.4 kernel uses some dirty hack[2] to get around this problem.
> 
> They call mdfs_pause_cpu[1] for each CPU core (except current)
> This function located in the SRAM and locks CPU in infinity loop,
> until `set_paused(false)` called on CPU0
> Also, legacy rockchip kernels use same hack[3].
> 
> But this method is not ideal...
> Before changing DDR freq we must make sure which each kernel is stuck
> on the SRAM function. But this is a very long process.
> In my proof-of-concept implementation sometimes elapses *few seconds*
> between I call smp_call_function and all cores stucking in SRAM
> function.
> This method may be suitable for manual freq change. But not for
> automatic governor mode. No chance to change frequency on heavy loaded
> CPUs.
> 
> We need another way.
> 
> I think, code on PSCI must stop/suspend any other working cpu cores
> before update freq. This is possible?

It _shouldn't_ be necessary. Setting bit 8 in PWRCTL blocks the DRAM
controller's host interface, which should cause the L2 cache subsystem
(and thus the other CPUs) to stall when trying to access DRAM. This is
what the MDFS hardware does on A64/H5, and I have seen no hangs there.

Possibly the issue is that such a stall sometimes affects the CPU that
is running from SRAM, even though it should not. (On the other hand,
when using the MDFS hardware, it is okay if all four CPUs temporarily
stall at the same time.)

One thing to check is if sunxi_dram_dvfs_req() completes successfully.
That function contains some unbounded loops, so it is possible to get
stuck. You could toggle a GPIO or something at the end of the function.
That would distinguish between "the secure monitor hung" and "we left
the DRAM controller in a bad state and hung when switching back to code
in DRAM" or even "we trashed the contents of DRAM".

We do use the architectural timer inside sunxi_dram_dvfs_req(), but
those registers are banked between secure/non-secure states, so that
should not interfere with Linux's use of the timer.

However, your test with offlining the other CPUs suggests we may really
need some synchronization. I would suggest doing this inside U-Boot as
well. You can send a SGI IPI to the other three CPUs and force them to
trap into the secure monitor. Not only will this be immediate, but it
will also ensure the other CPUs are running from SRAM during the
reclocking. You can take inspiration from the existing IPI code in psci.c.

It is quite convenient to be truly in control, so you can do things
behind the OS's back, and keep it blissfully unaware. :)

Regards,
Samuel

> [1] https://github.com/Hasiergo/Allwinner-A33-linux-3.4.113/blob/master/drivers/devfreq/dramfreq/sunxi-ddrfreq.c#L638
> [2] https://github.com/Hasiergo/Allwinner-A33-linux-3.4.113/blob/master/drivers/devfreq/dramfreq/sunxi-ddrfreq.c#L1088
> [3] https://github.com/bbelos/rk3188-kernel/blob/master/arch/arm/plat-rk/ddr_freq.c#L168
> 
> P.S. sorry for duplicate, previous message declined by mlmmj
> 
> 
> чт, 29 дек. 2022 г. в 19:29, Samuel Holland <samuel@sholland.org>:
>>
>> Hi Kirill,
>>
>> On 12/28/22 07:10, Kirill wrote:
>>> I'm trying to use your driver with h3, but have this result:
>>> ```
>>> [  387.306429] sun8i-h3-mbus 1c62000.dram-controller: Detected 32-bit
>>> DDRx with ODT
>>> [  388.450262] sun8i-h3-mbus 1c62000.dram-controller: Using 15554/243750
>>> (6%) at 1248 MHz
>>> [  389.906319] sun8i-h3-mbus 1c62000.dram-controller: Using 1079/243750
>>> (0%) at 1248 MHz
>>> [  389.914314] sun8i-h3-mbus 1c62000.dram-controller: Setting DRAM to
>>> 156 MHz, tREFI=19, tRFC=28, ODT=disabled
>>> ```
>>>
>>> After this CPU hangs and not responding.
>>> Is possible (at least theoretically) to use this driver with H3?
>>
>> Yes, although it will need some help from firmware. If you look at the
>> vendor driver[1] (pick any random Allwinner 4.9 tree), you will see
>> there is no mdfs_dfs() implementation for CONFIG_ARCH_SUN8IW7P1 (H3).
>> The driver always calls mdfs_main(), which is a standalone program
>> loaded to SRAM. The reason seems to be that the MDFS hardware is broken,
>> as you found out.
>>
>> Something like this standalone MDFS application is not upstreamable, but
>> conveniently we already have some firmware running from SRAM, namely
>> U-Boot's PSCI/secure monitor implementation. And Allwinner already has
>> some chips where they call a SMC to do this MDFS procedure[2]. So we can
>> reuse that SMC function ID, and put the code in in the secure monitor.
>> I've thrown together U-Boot[3] and Linux[4] patches as a proof of concept.
>>
>> It works, though it did lock up once after playing with the devfreq
>> sysfs for several minutes. The contents of sunxi_dram_dvfs_req() are
>> just copied from the vendor driver; they could surely be improved.
>>
>> The U-Boot patch is based on my series adding Crust support for H3, so I
>> could have interactive peek/poke from the AR100 even when the DRAM
>> controller is dead. It shouldn't be too hard to rebase that out and move
>> the code to psci.c.
>>
>> I'm not sure the best way to upstream the changes to psci.S. Probably we
>> need some platform callback to handle unknown function IDs.
>>
>> Regards,
>> Samuel
>>
>> [1]:
>> https://github.com/Tina-Linux/tina-v83x-linux-4.9/blob/master/drivers/devfreq/dramfreq/sunxi-ddrfreq.c#L1411
>> [2]:
>> https://github.com/Tina-Linux/tina-v83x-linux-4.9/blob/master/drivers/devfreq/sunxi-mdfs.h#L38
>> [3]: https://github.com/smaeul/u-boot/commits/patch/h3-dram-devfreq
>> [4]: https://github.com/smaeul/linux/commits/wip/devfreq-a83t
>>