Skip to content

PIO clock divider freq= parameter behaviour at extreme values. #7025

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.

Sign up for GitHub

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

Jump to bottom
Closed
richardm292 opened this issue Mar 13, 2021 · 9 comments
Closed

PIO clock divider freq= parameter behaviour at extreme values. #7025

richardm292 opened this issue Mar 13, 2021 · 9 comments
Labels
port-rp2

Comments

@richardm292
Copy link

richardm292 commented Mar 13, 2021 •
edited
Loading

Setting a low frequency that would generate a high number of decimal places causes the clock to be off but more than jitter.

sm = rp2.StateMachine(0, irq_test, freq=1907)

The edge case of freq=1907 generates tens of thousands of interrupts when expecting around 2000. Appears to be rounding issue more than just jitter?

Sample code:

import time
import rp2

i=int(0)

def i_inc(q):
    global i
    i += 1    
    #print(q.irq().flags())

@rp2.asm_pio()
def irq_test():
    nop()          [31]  # 32 cyclces
    nop()          [31]  # 64
    nop()          [31]  # 96
    nop()          [2]   # 99
    irq(0)               # 100 cycles 
    nop()          [31]
    nop()          [31]
    nop()          [31]
    nop()          [2]
    irq(1)
    
# 1907 causes a crazy amount of interrrupts. 

sm = rp2.StateMachine(0, irq_test, freq=1907)
rp2.PIO(0).irq(i_inc)
sm.active(1)
time.sleep(1)
sm.active(0)
rp2.PIO(0).irq()

print(i)
@richardm292
Copy link
Author

richardm292 commented Mar 13, 2021 •
edited
Loading

possible duplicate of this issue.

raspberrypi/pico-micropython-examples#18

@dpgeorge
Copy link
Member

The problem is that the frequency cannot go below 1908Hz. When trying to set it to 1907 the calculation of the clock divider overflows a 16-bit integer and you get about 10MHz PIO frequency.

To get a lower frequency for the PIO you can decrease the CPU frequency using machine.freq(<hz>), then set the PIO frequency.

It would probably make sense to raise an exception when trying to set the PIO frequency too high or low. And also provide a way to inspect the actual frequency (which may be slightly different to the one requested).

@richardm292
Copy link
Author

acknowledged.

@lurch
Copy link
Contributor

lurch commented Mar 13, 2021

IMHO this should be reopened, as (although they're related) these issues aren't duplicates...

This issue is about the PIO clock divider overflowing when the frequency is set too low (a bug in MicroPython).

raspberrypi/pico-micropython-examples#18 is asking about the frequency in the provided examples being too low (a bug in pico-micropython-examples).

@lurch
Copy link
Contributor

lurch commented Mar 13, 2021

@dpgeorge I did a bit of poking about, and (although it's not documented at https://raspberrypi.github.io/pico-sdk-doxygen/group__sm__config.html#gae8c09c7a4372da95ad777faae51c5a24 ), section 3.5.5. of https://datasheets.raspberrypi.org/rp2040/rp2040-datasheet.pdf says "The clock dividers are 16-bit integer, 8-bit fractional, with first-order delta-sigma for the fractional divider. The clock divisor can vary between 1 and 65536, in increments of 1/256".

@richardbarlow I was curious about how the maths works out, so I dug into that too... 😃

  1. MicroPython runs on the Pico at 125MHz (125000000Hz)
  2. When you request a PIO-frequency of 1907Hz, the code here calculates div as 125000000 / 1907 i.e. 65547.981
  3. This then gets passed through to the pico-sdk, and the code here converts that to a 16.8 representation with div_int of 11 (i.e. 65547 - 65536) and div_frac of 251
  4. We thus get a PIO frequency of 125000000 / (11 + 251/256) i.e. 10433648.5 Hz ! (10.4MHz)

This was referenced Mar 13, 2021
Closed
Merged
@dpgeorge dpgeorge reopened this Mar 14, 2021
@dpgeorge
Copy link
Member

This issue is about the PIO clock divider overflowing when the frequency is set too low (a bug in MicroPython).

Yep, agreed. What do you think about raising a ValueError if the user passes in a value that is too small or too big?

@lurch
Copy link
Contributor

lurch commented Mar 14, 2021

raising a ValueError if the user passes in a value that is too small or too big?

Sounds like a good idea 👍 It has the potential to "break" old code that was previously setting a freq < 1908, but that code wouldn't have been doing what the user expected anyway. (and RP2040 is only a couple of months old, so not much "legacy code" yet!)
I just checked, and the minimum PIO frequency used in any of the examples in https://hsmag.cc/picobook is 10 kHz, and (when talking about PIO StateMachine) it also says "The frequency (which must be between 2000 and 125000000)".

dpgeorge added a commit to dpgeorge/micropython that referenced this issue Mar 15, 2021
@dpgeorge
rp2/rp2_pio: Validate state machine frequency in constructor.
d6ec455 
Fixes issue micropython#7025.

Signed-off-by: Damien George <damien@micropython.org>
@dpgeorge dpgeorge mentioned this issue Mar 15, 2021
Merged
@dpgeorge
Copy link
Member

See #7033 for a patch which validates the frequency.

dpgeorge added a commit to dpgeorge/micropython that referenced this issue Apr 9, 2021
@dpgeorge
rp2/rp2_pio: Validate state machine frequency in constructor.
2c9af1c 
Fixes issue micropython#7025.

Signed-off-by: Damien George <damien@micropython.org>
@dpgeorge
Copy link
Member

dpgeorge commented Apr 9, 2021

Fixed by 2c9af1c

@dpgeorge dpgeorge closed this as completed Apr 9, 2021
@kevindawson kevindawson mentioned this issue Apr 14, 2021
Closed
ksekimoto pushed a commit to ksekimoto/micropython that referenced this issue Jul 16, 2021
@dpgeorge
rp2/rp2_pio: Validate state machine frequency in constructor.
afc675e 
Fixes issue micropython#7025.

Signed-off-by: Damien George <damien@micropython.org>
@lurch lurch mentioned this issue Jan 16, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
port-rp2
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@lurch @dpgeorge @richardm292