Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

H3 ddr dynamic freq (experimental) #5127

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Conversation

Azq2
Copy link
Collaborator

@Azq2 Azq2 commented May 2, 2023

Description

This is experimental support for DDR dynamic frequency changing for Allwinner H3.

It based on:

Due to H3 hardware bugs (see listing) it implemented by software "emulation" of MDFS:

  • It uses a special helper in the u-boot PSCI code. This is needed, because changing DDR freq is possible only in the SRAM.
  • Before freq changing all secondary CPU's put in the while(true) on PSCI handler (in SRAM). This is needed, because after DDR RAM enters self-refresh mode any RAM access leads to a complete freeze. We must suspend all execution on any secondary cores to prevent this.
  • And, finally, call PSCI callback SUNXI_PSCI_DRAM_DVFS_REQ on CPU0 for changing frequency.

Not great, but works.

Similar way implemented in the original legacy sunxi kernels: https://github.com/Tina-Linux/tina-v83x-linux-4.9/blob/master/drivers/devfreq/dramfreq/sunxi-ddrfreq.c#L1373
But instead of PSCI in u-boot it uses a dirty hack by writing code from kernel to the SRAM.

P.S.II'm not sure if this module should be enabled by default.
Most of users want performance instead of power saving. Maybe better and safer to disable this in DT (by status = "disabled").
And who needs it can enable using DT overlay.

How Has This Been Tested?

Tested:

  • h3 (orangepilite)

Not tested:

  • a33 (support added by this patches)
  • a83t (support added by this patches)
  • a64
  • h5

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules

@igorpecovnik
Copy link
Member

Maybe better and safer to disable this in DT

Yes, this way we can merge this at once.

Ideally adding an overlay for easier manipulation.

@The-going
Copy link
Contributor

Not great, but works.

@Azq2 Do you know how long it takes to switch the frequency?
Or how long are the processors idle?

@paolosabatino
Copy link
Contributor

paolosabatino commented May 8, 2023

Congrats for the study and very hard development work! 👍

Perhaps WFE instructions (Wait for Event) could be used during the idle period to avoid active looping?

@Azq2
Copy link
Collaborator Author

Azq2 commented May 9, 2023

Sorry for the long delay!

@The-going This is a good question. My measurements:

[13916.219635] sun8i_a33_mbus_suspend_secondary_cpus: 22 ms
[13916.396968] sun8i_a33_mbus_set_dram_freq_smc_delayed: 172 ms
[13916.402965] sun8i_a33_mbus_resume_secondary_cpus: 0 ms
  1. sun8i_a33_mbus_suspend_secondary_cpus - at this time at least one CPU still working, no downtime, but impact on performance
  2. sun8i_a33_mbus_set_dram_freq_smc_delayed - all cpus are busy!

It doesn't look very good for simple_ondemand governor. I think this has a big impact on CPU performance at the moment of switching.
And I'm not sure that the situation is any better with hardware MDFS.

Does anyone have A64/h3/a33/a83t and the ability to measure the sun8i_a33_mbus_set_dram_freq_mdfs() function?

Like that:

ktime_t start = ktime_get();

....

s64 elapsed = ktime_to_ms(ktime_get() - start);
printk("sun8i_a33_mbus_set_dram_freq_mdfs: %lld ms\r\n", elapsed);

But anyway even without simple_ondemand this feature is still useful. For example, my use case:

  1. Maximum DDR freq when powering on AC
  2. Minimum DDR freq when powering on battery
    governor=userspace + some scripts for udev
    With this I can achieve power usage less than on legacy image. Even with 312 MHz instead of 132 MHz in comparison to legacy image.

Finally, I have two variants:
One:

  • disable by default sun8i_a33_mbus in dtb + add standard overlay for enabling
  • disable simple_ondemand for H3, instead use governor=userspace by default

Two:

  • disable by default sun8i_a33_mbus in dtb + add standard overlay for enabling
  • disable simple_ondemand for ALL cpus, instead use governor=userspace by default
  • special dtb overlay for sun8i_a33_mbus with simple_ondemand enabled

@paolosabatino Thanks, good point.
Theoretically is safe, at least one timer (which used for scheduler) generating interrupts, no chance to stuck on WFE.

@The-going
Copy link
Contributor

I think this has a big impact on CPU performance at the moment of switching.

In different versions of the documentation for allwinner processors, it is written a little differently about the frequency switching process. Maybe it's just an incorrect translation.
One fact is obvious - you can only switch the entire cluster as a whole. This can be done dynamically, but with some limitations. The frequency grid must correspond to some integer multipliers. One of which can be changed dynamically, and the other is not.

And if part of the frequency grid does not correspond to some dividers and switching occurs at this frequency, then the frequency is first switched to the default minimum, and then switching occurs at the selected frequency. This rule applies if you need to switch to a frequency less than the default one.
And I observe real brakes when switching frequencies.
Everything I wrote is taken from my memory. I did this research a few years ago.
I may be wrong, but experimentally I have deduced the frequency grid that is in this patch.

https://github.com/armbian/build/blob/main/patch/kernel/archive/sunxi-6.1/patches.armbian/fix-cpu-opp-table-sun8i-a83t.patch

After its application, switching occurs faster.

@The-going
Copy link
Contributor

Does anyone have A64/h3/a33/a83t and the ability to measure the sun8i_a33_mbus_set_dram_freq_mdfs() function?

I have A64 and A83T on my test stand.
I can build a kernel 5.15, 6.1, 6.2, 6.3 (whichever you say) using your changes and run the test you wrote. But you will analyze the results yourself.
My interests do not extend to the problems of frequency regulation and energy conservation.
My interest is a fast (< 1000 us) guaranteed switching to a given frequency and processor operation for a long time in this mode.

@igorpecovnik igorpecovnik added the Backlog Stalled work that needs to be completed label Dec 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backlog Stalled work that needs to be completed
Development

Successfully merging this pull request may close these issues.

None yet

4 participants