adafruit / circuitpython

CircuitPython - a Python implementation for teaching coding with microcontrollers
https://circuitpython.org
MIT License
3.95k stars 1.16k forks source link

Setting PWM duty_cycle causes a blocking delay proportional to PWM frequency #7653

Open CedarGroveStudios opened 1 year ago

CedarGroveStudios commented 1 year ago

CircuitPython version

Adafruit CircuitPython 8.0.2 on 2023-02-14; Adafruit Feather M4 Express with samd51j19

Code/REPL

# SPDX-FileCopyrightText: 2018 Kattni Rembor for Adafruit Industries
#
# SPDX-License-Identifier: MIT

"""CircuitPython Essentials: PWM with Fixed Frequency example."""
import time
import board
import pwmio

# LED setup for most CircuitPython boards:
led = pwmio.PWMOut(board.LED, frequency=5000, duty_cycle=0)

while True:
    t0 = time.monotonic()
    for i in range(100):
        # PWM LED up and down
        if i < 50:
            led.duty_cycle = int(i * 2 * 65535 / 100)  # Up
        else:
            led.duty_cycle = 65535 - int((i - 50) * 2 * 65535 / 100)  # Down
        #time.sleep(0.01)
    print(f"accumulated average duty_cycle set time: {time.monotonic() - t0:0.3f} sec")

Behavior

100 iterations with a PWM frequency of 5000 Hz:

accumulated duty_cycle set time: 0.020 sec

100 iterations with a PWM frequency of 50 Hz:

accumulated duty_cycle set time: 2.004 sec

Description

The delay is approximately equal to the period of the PWM frequency. The delay is noticeable at lower frequencies such as those used for servo and DC motor control.

Although not confirmed (somewhat beyond my skill set), it might be that the SAMD51 timer used for PWM is not being operated in its double-buffered CC (Compare Channel) register mode. Instead it may be waiting to load the CC register when the timer count is at its ZERO value rather than loading the CC buffer register and returning to the calling module. (reference: Microchip SAM D5x/E5x Family Data Sheet , DS60001507E, pp 1719-1721)

If possible, the desired operation would be to load the new PWM duty_cycle value and immediately return to the calling code, allowing the SAMD51's timer hardware to self-synchronize rather than cause a blocking delay condition.

Changing to a double-buffered approach may also benefit asyncio-based code.

Additional information

tannewt commented 1 year ago

This must be related to SYNCBUSY:

https://github.com/adafruit/circuitpython/blob/8a8579a9b9bc3bd2200426d7294dd16f8bd72c6c/ports/atmel-samd/common-hal/pwmio/PWMOut.c#L293-L335

I think we do want the value to be copied on reload but that shouldn't prevent us from writing the buffered value multiple times in the interim. We just changed this on rp2: #7299

CedarGroveStudios commented 1 year ago

Also suspect that the delay leads to DC motor "rattling" when establishing a new motor speed at low PWM frequencies since the two PWM control pins aren't changed simultaneously.

CedarGroveStudios commented 1 year ago

Also suspect that the delay leads to DC motor "rattling" when establishing a new motor speed at low PWM frequencies since the two PWM control pins aren't changed simultaneously.

Confirmed. At low PWM frequencies in slow decay mode, the delay causes a DC motor to noticeably shudder when switching between velocity zero and coasting (velocity = None) and back. Don't have a setup to test stepping motors, but would expect a similar issue.

timchinowsky commented 1 month ago

I have been doing some benchmarking and the slowness of PWM setting immediately popped out. Looks like it is a known issue which still need to be fixed? And is there any background on if/how the SAMD PWM peripheral is different from e.g. the RP2040's, which requires setting it to be handled differently?

tannewt commented 1 month ago

I have been doing some benchmarking and the slowness of PWM setting immediately popped out. Looks like it is a known issue which still need to be fixed? And is there any background on if/how the SAMD PWM peripheral is different from e.g. the RP2040's, which requires setting it to be handled differently?

I don't think it needs to be different. It just hasn't been updated. See https://github.com/adafruit/circuitpython/issues/7653#issuecomment-1447322516

timchinowsky commented 1 month ago

Wondering why is double-buffering disabled here? https://github.com/adafruit/circuitpython/blob/2854dbe37f52dc9ebf48035dfc82bba83c8f9cac/ports/atmel-samd/common-hal/pwmio/PWMOut.c#L278-L286

and why is it necessary to wait for sync? https://github.com/adafruit/circuitpython/blob/2854dbe37f52dc9ebf48035dfc82bba83c8f9cac/ports/atmel-samd/common-hal/pwmio/PWMOut.c#L275-L276 Thanks!

tannewt commented 1 month ago

and why is it necessary to wait for sync?

I don't think it is necessary. It was just a choice we made during the original implementation.

Wondering why is double-buffering disabled here?

No idea. Its been years since this code was added.

timchinowsky commented 1 month ago

Ok! I'm going to be looking at this and #7747