esp: make cpu frequency settable

[Sola85]

This PR resolves #9305, making microcontroller.cpu.frequency settable on espressif boards. I followed micropython's implementation of the same feature.

Question 1

This requires enabling CONFIG_PM_ENABLE in esp-idf, which is allows for a uniform API (see https://docs.espressif.com/projects/esp-idf/en/v5.2.2/esp32/api-reference/system/power_management.html) for changing the CPU frequency for all esp variants and internally ensures peripherals continue functioning. This means esp-idf contains many internal locations where CONFIG_PM_ENABLE is checked and extra logic has to be executed when power management is enabled.

This means the circuitpython binary increases by ~3kB for all variants, which means we get -464 bytes free in flash firmware space for 1 board (lolin_c3_pico, japanese translation).

These are the options I see:

1. Don't make cpu.frequency settable on boards with limited space (which would these be? all C3 variants? or only certain C3 variants?)
2. Instead of the power management api, implement cpu frequency changes manually like arduino is doing it. This would not require CONFIG_PM_ENABLE, possibly leading to smaller code size. But I'm not sure what we lose by doing this. All those extra checks that CONFIG_PM_ENABLE induces surely are there for a reason.

(Personally I would prefer option 1, as the code would be more maintainable than with option 2)

Question 2

I think it makes sense to inform the user about the possible frequencies that can be set, as there are not many valid ones (e.g. 20, 40, 80, 160, 240 MHz for the original ESP32).
For now I created a (rather long) new error message for each esp variant containing the valid frequencies, which is consistent with the settable frequency for ports/mimxrt10xx/.
Does this make sense here? A more space-saving but less user-friendly approach would be a generic error message "Invalid Frequency" and a reference to the documentation.

[bill88t]

Removing the error message and instead just jumping to the nearest frequency is also possible to save space.
cpu.frequency = 220_000_000 would set it to 240Mhz.
cpu.frequency = 190_000_000 would set it to 160Mhz.
However this approach would not really work for overclocking.

[tannewt]

(Personally I would prefer option 1, as the code would be more maintainable than with option 2)

Option 1 sounds good to me.

Question 2

I think it makes sense to inform the user about the possible frequencies that can be set, as there are not many valid ones (e.g. 20, 40, 80, 160, 240 MHz for the original ESP32). For now I created a (rather long) new error message for each esp variant containing the valid frequencies, which is consistent with the settable frequency for ports/mimxrt10xx/. Does this make sense here? A more space-saving but less user-friendly approach would be a generic error message "Invalid Frequency" and a reference to the documentation.

What we do for peripherals like SPI is we do the fastest available speed that is at the given frequency or less. This way you don't end up faster than you intended. You read the value back to know what it was set to. I'd rather not have custom errors because that increases translation burden.

[Sola85]

I like the idea of automatically "rounding down" to the next lower supported frequency. As far as I can tell, overclocking is not supported by esp_pm_configure anyway, so that would not be an issue.

Regarding the size of the build: Instead of disabling the feature on C3 builds, would it be acceptable to modify the partition sizes to make the changes fit?
I think it's only partitions-4MB-no-uf2.csv that would require a small reduction in user_fs size, in order to increase the size of ota_0 and ota_1 by ~ 2kB each.

[bill88t]

Touching the partition tables will wipe every existing c3's filesystem on upgrade.

[tannewt]

Regarding the size of the build: Instead of disabling the feature on C3 builds, would it be acceptable to modify the partition sizes to make the changes fit? I think it's only partitions-4MB-no-uf2.csv that would require a small reduction in user_fs size, in order to increase the size of ota_0 and ota_1 by ~ 2kB each.

As @bill88t points out, that's a breaking change. Instead, turn it off when CIRCUITPY_LEGACY_4MB_FLASH_LAYOUT is set. That way new boards will include it. We'll likely remove that setting with CP10 and break user filesystems once for it.

[Sola85]

Makes sense, I didn't realize that changing the partition table would be a breaking change.

Does CIRCUITPY_LEGACY_4MB_FLASH_LAYOUT already exist? I can't find such a symbol in the code base. Are you suggesting I should add this? Would this imply the following changes?

• Add CIRCUITPY_LEGACY_4MB_FLASH_LAYOUT which is enabled as a default for all boards currently using partitions-4MB-no-uf2 (for CP9 at least).
• Disable settable CPU frequency when CIRCUITPY_LEGACY_4MB_FLASH_LAYOUT is set.
• Rename partitions-4MB-no-uf2 to partitions-4MB-no-uf2-legacy.
• Add a new, non-legacy partitions-4MB-no-uf2 that can be used by disabling CIRCUITPY_LEGACY_4MB_FLASH_LAYOUT and which would be the default for new boards.

Or did I misunderstand your suggestion @tannewt?

[tannewt]

@Sola85 It exists already. https://github.com/adafruit/circuitpython/blob/main/ports/espressif/mpconfigport.mk#L190

Your changes may be based on older CP though. It was just added with my deep sleep changes.

[Sola85]

I think both questions are solved now.

• CIRCUITPY_SETTABLE_PROCESSOR_FREQUENCY is disabled if CIRCUITPY_LEGACY_4MB_FLASH_LAYOUT is set (additionally we also need to disable CONFIG_PM_ENABLE for the legacy flash layout. I did this by setting CONFIG_PM_ENABLE=y as a default and overriding this in the problematic sdkconfig-flash-4MB-no-uf2.defaults, which (as I understand) will be removed by CP 10 anyway. If there is a better way of tying CONFIG_PM_ENABLE to CIRCUITPY_LEGACY_4MB_FLASH_LAYOUT I'd be happy for suggestions.)
• Invalid CPU frequencies are "rounded down" to the next lower valid frequency.

[bill88t]

I think it's as good as it can be considering the flash space constrains.

Tested on M5Stack cardputer (8MB flash).
The ADC bug is unaffected by CPU frequencies, and is reproducible even when running at 80Mhz.

[tannewt]

One question about sleep. Good otherwise. Thanks!

[Sola85]

Does that mean all comments are resolved? Or is there anything I can still do?

[tannewt]

Yup! Sorry I didn't merge immediately. I think the CI may have been going still.