Add frequency setting for RP2040 boards.

[Lanzaa]

Allow setting the cpu frequency for RP2040 boards.

My first PR here. I did very basic testing using a Pico board. Being able to lower the system frequency helps with power usage. At the standard 125MHz my board uses ~20mA and at 20MHz it uses ~6mA.

[microdev1]

Thanks! @Lanzaa, a few things to address.

[dhalbert]

One thing to check is if any code is assuming 125000000 as the clock frequency. I searched for 125:

I think this could be fixed:

circuitpython/ports/raspberrypi/common-hal/rgbmatrix/RGBMatrix.c

Line 42 in 037bfb3

int result = pwmout_allocate(slice, channel, true, 125000000 / 3);

Another issue is code that has timing derived from assuming 125MHz. This may also be fixable, at least within a certain range:

circuitpython/ports/raspberrypi/common-hal/floppyio/__init__.h

Lines 29 to 35 in 037bfb3

	// empirical-ish from RP2040 @ 125MHz for floppy_flux_readinto
	#define FLOPPYIO_SAMPLERATE (24000000)
	// empirical-ish from RP2040 @ 125MHz for floppy_mfm_readinto
	// my guess is these are slower because the more complex routine falls out of cache, but it's just
	// speculation because the loops are very similar. When looking at raw bins with a modified
	// version of adafruit_floppy, it can be seen that there are _two_ peaks for T2 and T3, rather
	// than a single one, around 36 (mostly) and 43 (rarer), compared to a single peak around 48.

(found due to comment) (@fyi @jepler)

For https://github.com/adafruit/circuitpython/blob/main/ports/raspberrypi/common-hal/neopixel_write/__init__.c, the PIO programs have been tuned assuming 125 MHz.
here may be other PIO uses as well. These are a lot more difficult to parameterize. It should be documented that they assume 125MHz and may not work when the clock frequency is changed.

[jepler]

Thank you for working on this PR! A number of people would like functionality like this. However, I have some remarks about problems that seem likely to occur when this functionality is made available and would like to see the documentation improved to anticipate them.

Say that I create an object like PWMOut that has an associated frequency (1MHz), then change the CPU frequency from the default (125MHz) to 20MHz. As I understand it, the frequency of the PWMOut will also change, because it uses a divisor of the CPU frequency. In this case, the PWMOut frequency would change from 1MHz to 160kHz (20MHz/125).

(audio, pulseio, spi, i2c, and anything using pio would also be affected)

I am sure this is acceptable for some use cases but it deserves a big caveat in the documentation.

Another concern is that this function will set high frequencies above the datasheet maximum. e.g., this led to the spurious report that calling the function twice didn't work: raspberrypi/pico-sdk#94 and even at frequencies that do "work" it reportedly interferes with USB functionality (reported in the same issue). Should the function allow "above datasheet maximum" settings, or should it reject them? Again, I think this needs a note in the documentation.

Also only 440 of the 1-kHz granularity frequencies from 1kHz to 133MHz succeed in configuring, the other 132559 fail. Every 1MHz multiple 20MHz or above does succeed. This is because the clock setting routine in the SDK only sets the frequency if it can be exactly attained. But, confusingly, if setting 20_000_000Hz works then setting 20_000_001Hz will also work, because the 1Hz difference has been divided away already. (this is based not on testing on-device but on a standalone program that uses code copied out of pico-sdk: https://gist.github.com/fd42b3b7d99bbb321dd6db6ef77499aa)

[Lanzaa]

Sounds good. I will see what I can address. Hopefully I will update the PR before the start of next week.

[bill88t]

First of all I believe it would be best to 'by-default' prohibit overclocking.
A typo could be very problematic..
One extra digit is all it takes for the rp2040 to become a potato chip.
Ideally an cpu.safety/cpu.overclock/cpu.cook_mah_pi kind of property should be made available.

I think it's good to try to implement this so that all the modules are ready for any future rp2 chips with different frequencies.

Also, @Lanzaa, to fix the pre-commit check run pip install pre-commit && pre-commit install && pre-commit run -a in the repo directory, after that, it will automatically run on every commit for this clone of the repo.
For more information on pre-commit please refer to: https://pre-commit.com/#install

[Lanzaa]

I have changed common_hal_mcu_processor_set_frequency to a void return and improved the documentation:

First of all I believe it would be best to 'by-default' prohibit overclocking. ... Ideally an cpu.safety/cpu.overclock/cpu.cook_mah_pi kind of property should be made available.

Any advice on where to put these properties?

In this case, the PWMOut frequency would change from 1MHz to 160kHz (20MHz/125). (audio, pulseio, spi, i2c, and anything using pio would also be affected)

@jepler Do you have any advice on how I could best test these issues?

[tannewt]

I have changed common_hal_mcu_processor_set_frequency to a void return and improved the documentation:

Thanks for the note! Please make it generic. The same clock caveat will apply across all micros. Please add a suggestion that one sets the frequency before starting to using anything else. That way they should be able to adjust to the new clock.

Any advice on where to put these properties?

Don't add them. Frequency is enough. The common_hal code should verify the value is somewhat reasonable.

@jepler Do you have any advice on how I could best test these issues?

A logic analyzer or oscilloscope should be able to show you the frequency change in the pwm output.

[microdev1]

Thanks for making the requested changes.

[jepler]

I hadn't even considered the impact on USB. Does changing the frequency at all mean USB won't work any longer?

[jepler]

[20:13:24] >>> microcontroller.cpu.frequency=125_000_000; t0 = time.monotonic_ns(); t0; sum(range(1_000_000)); t1 = time.monotonic_ns(); (t1-t0) / 1e9
[20:13:44] 207238281250
[20:13:54] 499999500000
[20:13:54] 10.6943
[20:13:54] >>> microcontroller.cpu.frequency=300_000_000; t0 = time.monotonic_ns(); t0; sum(range(1_000_000)); t1 = time.monotonic_ns(); (t1-t0) / 1e9
[20:14:03] 226430664062
[20:14:07] 499999500000
[20:14:07] 4.45605
[20:14:07] >>> microcontroller.cpu.frequency=600_000_000; t0 = time.monotonic_ns(); t0; sum(range(1_000_000)); t1 = time.monotonic_ns(); (t1-t0) / 1e9

[tio 20:14:21] Disconnected

1. Setting the frequency doesn't immediately disconnect USB
2. and a computational task does get faster
3. but setting it too high just breaks everything. What 'maximum overclock' should be accepted? Arduino IDE with
4. before & after frequency changes, neopixel_write continues to work so clocks set up after the frequency change are correct. neopixel_write constructs a fresh StateMachine instance every time
5. as expected, an existing PIO state machine operates at the wrong frequency. For instance, when using adafruit_neopxl8, changing the frequency after creating a pixel object caused it to malfunction, showing bright white instead of dim blue.

[jepler]

why were these lines removed? doesn't it stop the clock setting from becoming effective?

[jepler]

[20:21:51] >>> n[0] = 0
[20:21:55] >>> n[0] = 1 # correctly changes color
[20:21:57] >>> microcontroller.cpu.frequency
[20:22:03] 250000000
[20:22:03] >>> microcontroller.cpu.frequency = 125_000_000
[20:22:06] >>> n[0] = 0 # sets incorrect color because PIO operating at wrong frequency
[20:22:08] >>> n[0] = 1

[dhalbert]

Changing this to draft for now.

[tannewt]

I've polished this up. @dhalbert please re-review since jepler is out.

[dhalbert]

@tannewt Can we wait for @jepler to also re-review, or do we need this in a build for Pico DVI?

[tannewt]

It can wait. It isn't needed for DVI.

[jepler]

Thanks!