From 1acf65ee226d5957442ce60964c7f02569826ecf Mon Sep 17 00:00:00 2001 From: Scott Shawcroft Date: Fri, 17 Feb 2023 16:13:18 -0800 Subject: [PATCH] Fix `pwmio` on iMX RT. It now handles deinit, never_reset and sharing tracking. PWM now runs in the WAIT state as well during a time.sleep(). _reset_ok() was removed because it was called in one spot right before deinit(). Some PWMOut were also switched to a bitmap for use instead of reference count. That way init and deinit are idempotent. Fixes #6589. Fixes #4841. Fixes #4541. --- ports/atmel-samd/common-hal/pwmio/PWMOut.c | 5 +- ports/cxd56/common-hal/pwmio/PWMOut.c | 6 +- ports/espressif/common-hal/pwmio/PWMOut.c | 25 +- ports/mimxrt10xx/common-hal/pwmio/PWMOut.c | 331 ++++++++++++-------- ports/mimxrt10xx/common-hal/pwmio/PWMOut.h | 6 +- ports/mimxrt10xx/supervisor/port.c | 2 +- ports/nrf/common-hal/pwmio/PWMOut.c | 26 +- ports/raspberrypi/common-hal/pwmio/PWMOut.c | 8 - ports/stm/common-hal/pwmio/PWMOut.c | 38 ++- shared-module/displayio/Display.c | 1 - 10 files changed, 246 insertions(+), 202 deletions(-) diff --git a/ports/atmel-samd/common-hal/pwmio/PWMOut.c b/ports/atmel-samd/common-hal/pwmio/PWMOut.c index c6e9e07304..1bb955fce8 100644 --- a/ports/atmel-samd/common-hal/pwmio/PWMOut.c +++ b/ports/atmel-samd/common-hal/pwmio/PWMOut.c @@ -67,10 +67,6 @@ void common_hal_pwmio_pwmout_never_reset(pwmio_pwmout_obj_t *self) { never_reset_pin_number(self->pin->number); } -void common_hal_pwmio_pwmout_reset_ok(pwmio_pwmout_obj_t *self) { - timer_reset_ok(self->timer->index, self->timer->is_tc); -} - void pwmout_reset(void) { // Reset all timers for (int i = 0; i < TCC_INST_NUM; i++) { @@ -267,6 +263,7 @@ void common_hal_pwmio_pwmout_deinit(pwmio_pwmout_obj_t *self) { if (common_hal_pwmio_pwmout_deinited(self)) { return; } + timer_reset_ok(self->timer->index, self->timer->is_tc); const pin_timer_t *t = self->timer; if (t->is_tc) { Tc *tc = tc_insts[t->index]; diff --git a/ports/cxd56/common-hal/pwmio/PWMOut.c b/ports/cxd56/common-hal/pwmio/PWMOut.c index 7e27817aab..10689dc55a 100644 --- a/ports/cxd56/common-hal/pwmio/PWMOut.c +++ b/ports/cxd56/common-hal/pwmio/PWMOut.c @@ -90,6 +90,8 @@ void common_hal_pwmio_pwmout_deinit(pwmio_pwmout_obj_t *self) { return; } + pwmout_dev[self->number].reset = true; + ioctl(pwmout_dev[self->number].fd, PWMIOC_STOP, 0); close(pwmout_dev[self->number].fd); pwmout_dev[self->number].fd = -1; @@ -134,10 +136,6 @@ void common_hal_pwmio_pwmout_never_reset(pwmio_pwmout_obj_t *self) { pwmout_dev[self->number].reset = false; } -void common_hal_pwmio_pwmout_reset_ok(pwmio_pwmout_obj_t *self) { - pwmout_dev[self->number].reset = true; -} - void pwmout_reset(void) { for (int i = 0; i < MP_ARRAY_SIZE(pwmout_dev); i++) { if (pwmout_dev[i].fd >= 0 && pwmout_dev[i].reset) { diff --git a/ports/espressif/common-hal/pwmio/PWMOut.c b/ports/espressif/common-hal/pwmio/PWMOut.c index 68518fbd25..2cb1e6254b 100644 --- a/ports/espressif/common-hal/pwmio/PWMOut.c +++ b/ports/espressif/common-hal/pwmio/PWMOut.c @@ -164,24 +164,6 @@ void common_hal_pwmio_pwmout_never_reset(pwmio_pwmout_obj_t *self) { never_reset_pin_number(self->pin->number); } -void common_hal_pwmio_pwmout_reset_ok(pwmio_pwmout_obj_t *self) { - never_reset_tim[self->tim_handle.timer_num] = false; - // Search if any other channel is using the timer and is never reset. - // Otherwise, we clear never_reset for the timer as well. - bool other_never_reset = false; - for (size_t i = 0; i < LEDC_CHANNEL_MAX; i++) { - if (i != self->chan_handle.channel && - reserved_channels[i] == self->tim_handle.timer_num && - never_reset_chan[i]) { - other_never_reset = true; - break; - } - } - if (!other_never_reset) { - never_reset_chan[self->chan_handle.channel] = false; - } -} - bool common_hal_pwmio_pwmout_deinited(pwmio_pwmout_obj_t *self) { return self->deinited == true; } @@ -196,14 +178,21 @@ void common_hal_pwmio_pwmout_deinit(pwmio_pwmout_obj_t *self) { } reserved_channels[self->chan_handle.channel] = INDEX_EMPTY; never_reset_chan[self->chan_handle.channel] = false; + // Search if any other channel is using the timer bool taken = false; + bool other_never_reset = false; for (size_t i = 0; i < LEDC_CHANNEL_MAX; i++) { if (reserved_channels[i] == self->tim_handle.timer_num) { taken = true; + other_never_reset = never_reset_chan[i]; break; } } + // Clear the timer's never reset if the other channel isn't never reset. + if (!other_never_reset) { + never_reset_tim[self->tim_handle.timer_num] = false; + } // Variable frequency means there's only one channel on the timer if (!taken || self->variable_frequency) { ledc_timer_rst(LEDC_LOW_SPEED_MODE, self->tim_handle.timer_num); diff --git a/ports/mimxrt10xx/common-hal/pwmio/PWMOut.c b/ports/mimxrt10xx/common-hal/pwmio/PWMOut.c index 97892b1095..c2afb38664 100644 --- a/ports/mimxrt10xx/common-hal/pwmio/PWMOut.c +++ b/ports/mimxrt10xx/common-hal/pwmio/PWMOut.c @@ -38,9 +38,19 @@ #include "supervisor/shared/translate/translate.h" #include "periph.h" -// Debug print support set to zero to enable debug printing -#define ENABLE_DEBUG_PRINTING 0 +static PWM_Type *const _flexpwms[] = PWM_BASE_PTRS; +// 4 bits for each submodule in each FlexPWM. +static uint16_t _pwm_never_reset[MP_ARRAY_SIZE(_flexpwms)]; +// Bitmask of whether state machines are use for variable frequency. +static uint8_t _pwm_variable_frequency[MP_ARRAY_SIZE(_flexpwms)]; +// Configured frequency for each submodule. +static uint32_t _pwm_sm_frequencies[MP_ARRAY_SIZE(_flexpwms)][FSL_FEATURE_PWM_SUBMODULE_COUNT]; +// Channels use is tracked using the OUTEN register. + +// The SDK gives use clocks per submodule but they all share the same value! So, ignore the +// submodule and only turn off the clock when no other submodules are in use. +static const clock_ip_name_t _flexpwm_clocks[][FSL_FEATURE_PWM_SUBMODULE_COUNT] = PWM_CLOCKS; static void config_periph_pin(const mcu_pwm_obj_t *periph) { IOMUXC_SetPinMux( @@ -61,13 +71,59 @@ static void config_periph_pin(const mcu_pwm_obj_t *periph) { | IOMUXC_SW_PAD_CTL_PAD_SRE(0)); } +static uint16_t _outen_mask(pwm_submodule_t submodule, pwm_channels_t channel) { + uint16_t outen_mask = 0; + uint8_t sm_mask = 1 << submodule; + switch (channel) { + case kPWM_PwmX: + outen_mask |= PWM_OUTEN_PWMX_EN(sm_mask); + break; + case kPWM_PwmA: + outen_mask |= PWM_OUTEN_PWMA_EN(sm_mask); + break; + case kPWM_PwmB: + outen_mask |= PWM_OUTEN_PWMB_EN(sm_mask); + break; + } + return outen_mask; +} + void common_hal_pwmio_pwmout_never_reset(pwmio_pwmout_obj_t *self) { + common_hal_never_reset_pin(self->pin); + _pwm_never_reset[self->flexpwm_index] |= (1 << (self->pwm->submodule * 4 + self->pwm->channel)); } -void common_hal_pwmio_pwmout_reset_ok(pwmio_pwmout_obj_t *self) { +STATIC void _maybe_disable_clock(uint8_t instance) { + if ((_flexpwms[instance]->MCTRL & PWM_MCTRL_RUN_MASK) == 0) { + CLOCK_DisableClock(_flexpwm_clocks[instance][0]); + } } -void pwmout_reset(void) { +void reset_all_flexpwm(void) { + for (size_t i = 1; i < MP_ARRAY_SIZE(_pwm_never_reset); i++) { + PWM_Type *flexpwm = _flexpwms[i]; + for (size_t submodule = 0; submodule < FSL_FEATURE_PWM_SUBMODULE_COUNT; submodule++) { + uint8_t sm_mask = 1 << submodule; + for (size_t channel = 0; channel < 3; channel++) { + uint16_t channel_mask = 0x1 << (submodule * 4 + channel); + if ((_pwm_never_reset[i] & channel_mask) != 0) { + continue; + } + + // Turn off the channel. + flexpwm->OUTEN &= ~_outen_mask(submodule, channel); + } + uint16_t submodule_mask = 0xf << (submodule * 4); + if ((_pwm_never_reset[i] & submodule_mask) != 0) { + // Leave the submodule on since a channel is marked for never_reset. + continue; + } + flexpwm->MCTRL &= ~(sm_mask << PWM_MCTRL_RUN_SHIFT); + _pwm_variable_frequency[i] &= ~sm_mask; + _pwm_sm_frequencies[i][submodule] = 0; + } + _maybe_disable_clock(i); + } } #define PWM_SRC_CLK_FREQ CLOCK_GetFreq(kCLOCK_IpgClk) @@ -87,33 +143,6 @@ static int calculate_pulse_count(uint32_t frequency, uint8_t *prescaler) { return 0; } -// ========================================================== -// Debug code -// ========================================================== -#if ENABLE_DEBUG_PRINTING -#define DBGPrintf mp_printf -extern void debug_print_flexpwm_registers(PWM_Type *base); - -void debug_print_flexpwm_registers(PWM_Type *base) { - mp_printf(&mp_plat_print, - "\t\tPWM OUTEN:%x MASK:%x SWCOUT:%x DTSRCSEL:%x MCTRL:%x MCTRL2:%x FCTRL:%x FSTS:%x FFILT:%x FTST:%x FCTRL2:%x\n", - base->OUTEN, base->MASK, base->SWCOUT, base->DTSRCSEL, base->MCTRL, base->MCTRL2, base->FCTRL, - base->FSTS, base->FFILT, base->FTST, base->FCTRL2); - for (uint8_t i = 0; i < 4; i++) { - mp_printf(&mp_plat_print, - "\t\t(%u) INIT:%x CTRL2:%x CTRL:%x VAL0:%x VAL1:%x VAL2:%x VAL3:%x VAL4:%x VAL5:%x OCTRL:%x DTCNT0:%x DTCNT1:%x DISMAP: %x %x\n", i, - base->SM[i].INIT, base->SM[i].CTRL2, base->SM[i].CTRL, base->SM[i].VAL0, base->SM[i].VAL1, base->SM[i].VAL2, - base->SM[i].VAL3, base->SM[i].VAL4, base->SM[i].VAL5, base->SM[i].OCTRL, base->SM[i].DTCNT0, base->SM[i].DTCNT1, - base->SM[i].DISMAP[0], base->SM[i].DISMAP[1]); - } - -} -#else -#define DBGPrintf(p,...) -inline void debug_print_flexpwm_registers(PWM_Type *base) { -} -#endif - pwmout_result_t common_hal_pwmio_pwmout_construct(pwmio_pwmout_obj_t *self, const mcu_pin_obj_t *pin, uint16_t duty, @@ -122,12 +151,7 @@ pwmout_result_t common_hal_pwmio_pwmout_construct(pwmio_pwmout_obj_t *self, self->pin = pin; self->variable_frequency = variable_frequency; - const uint32_t pwm_count = sizeof(mcu_pwm_list) / sizeof(mcu_pwm_obj_t); - - DBGPrintf(&mp_plat_print, ">>> common_hal_pwmio_pwmout_construct called: pin: %p %u freq:%u duty:%u var:%u\n", - self->pin->gpio, self->pin->number, frequency, duty, variable_frequency); - - for (uint32_t i = 0; i < pwm_count; ++i) { + for (uint32_t i = 0; i < MP_ARRAY_SIZE(mcu_pwm_list); ++i) { if (mcu_pwm_list[i].pin != pin) { continue; } @@ -141,30 +165,20 @@ pwmout_result_t common_hal_pwmio_pwmout_construct(pwmio_pwmout_obj_t *self, return PWMOUT_INVALID_PIN; } - DBGPrintf(&mp_plat_print, "\tFound in PWM List\n"); + PWM_Type *flexpwm = self->pwm->pwm; + pwm_submodule_t submodule = self->pwm->submodule; + uint16_t sm_mask = 1 << submodule; + pwm_channels_t channel = self->pwm->channel; - config_periph_pin(self->pwm); + uint8_t flexpwm_index = 1; + for (; flexpwm_index < MP_ARRAY_SIZE(_flexpwms); flexpwm_index++) { + if (_flexpwms[flexpwm_index] == flexpwm) { + break; + } + } + self->flexpwm_index = flexpwm_index; - pwm_config_t pwmConfig; - - /* - * pwmConfig.enableDebugMode = false; - * pwmConfig.enableWait = false; - * pwmConfig.reloadSelect = kPWM_LocalReload; - * pwmConfig.faultFilterCount = 0; - * pwmConfig.faultFilterPeriod = 0; - * pwmConfig.clockSource = kPWM_BusClock; - * pwmConfig.prescale = kPWM_Prescale_Divide_1; - * pwmConfig.initializationControl = kPWM_Initialize_LocalSync; - * pwmConfig.forceTrigger = kPWM_Force_Local; - * pwmConfig.reloadFrequency = kPWM_LoadEveryOportunity; - * pwmConfig.reloadLogic = kPWM_ReloadImmediate; - * pwmConfig.pairOperation = kPWM_Independent; - */ - PWM_GetDefaultConfig(&pwmConfig); - - // pwmConfig.reloadLogic = kPWM_ReloadPwmFullCycle; - pwmConfig.enableDebugMode = true; + uint16_t outen_mask = _outen_mask(submodule, channel); self->pulse_count = calculate_pulse_count(frequency, &self->prescaler); @@ -172,69 +186,92 @@ pwmout_result_t common_hal_pwmio_pwmout_construct(pwmio_pwmout_obj_t *self, return PWMOUT_INVALID_FREQUENCY; } - pwmConfig.prescale = self->prescaler; - - DBGPrintf(&mp_plat_print, "\tCall PWM_Init\n"); - if (PWM_Init(self->pwm->pwm, self->pwm->submodule, &pwmConfig) == kStatus_Fail) { - return PWMOUT_INVALID_PIN; - } - - // Disable all fault inputs - self->pwm->pwm->SM[self->pwm->submodule].DISMAP[0] = 0; - self->pwm->pwm->SM[self->pwm->submodule].DISMAP[1] = 0; - - DBGPrintf(&mp_plat_print, "\tCall PWM_SetupPwm %p %x %u\n", self->pwm->pwm, self->pwm->submodule); - // ======================================================================================================== - // Not calling the PWM_SetupPwm as it was setup to only work for PWM output on chan A and B but not X - // I have done some experimenting, probably could try others, but again they do not work with X. - // Most of the code checks to see if A if not, then it assume B. - // - // Instead I set it up to work similar to what the Teensy 4.x code does. - // - // That is we set the PWM_CTRL_FULL_MASK, which then uses base->SM[submodule].VAL1 to control - // when the timer is reset, so it sets up your cycle/frequency. But then this implies that X channel - // which uses 0, 1 has to be handled specially. So for the different channels: - // A - Uses VAL2 to turn on (0) and VAL3=duty to turn off - // B - Uses VAL4 to turn on (0) and VAL5 to turn off - // X - As mentioned above VAL1 turns off, but it's set to the timing for frequency. so - // VAL0 turns on, so we set it to VAL1 - duty - // - PWM_Type *base = self->pwm->pwm; - uint8_t submodule = self->pwm->submodule; - - uint32_t mask = 1 << submodule; - uint32_t olddiv = base->SM[submodule].VAL1 + 1; - if (self->pulse_count != olddiv) { - base->MCTRL |= PWM_MCTRL_CLDOK(mask); - base->SM[submodule].CTRL = PWM_CTRL_PRSC_MASK | PWM_CTRL_PRSC(self->prescaler); - base->SM[submodule].VAL1 = self->pulse_count - 1; - base->SM[submodule].CTRL2 = PWM_CTRL2_INDEP_MASK | PWM_CTRL2_WAITEN_MASK | PWM_CTRL2_DBGEN_MASK; - - if (olddiv == 1) { - base->SM[submodule].CTRL = PWM_CTRL_FULL_MASK; - base->SM[submodule].VAL0 = 0; - base->SM[submodule].VAL2 = 0; - base->SM[submodule].VAL3 = 0; - base->SM[submodule].VAL4 = 0; - base->SM[submodule].VAL5 = 0; - } else { - base->SM[submodule].VAL0 = (base->SM[submodule].VAL0 * self->pulse_count) / olddiv; - base->SM[submodule].VAL3 = (base->SM[submodule].VAL3 * self->pulse_count) / olddiv; - base->SM[submodule].VAL5 = (base->SM[submodule].VAL5 * self->pulse_count) / olddiv; + // The submodule is already running + if (((flexpwm->MCTRL >> PWM_MCTRL_RUN_SHIFT) & sm_mask) != 0) { + // Another output has claimed this submodule for variable frequency already. + if ((_pwm_variable_frequency[flexpwm_index] & sm_mask) != 0) { + return PWMOUT_ALL_TIMERS_ON_PIN_IN_USE; + } + + // We want variable frequency but another class has already claim a fixed frequency. + if (variable_frequency) { + return PWMOUT_VARIABLE_FREQUENCY_NOT_AVAILABLE; + } + + // Another pin is already using this output. + if ((flexpwm->OUTEN & outen_mask) != 0) { + return PWMOUT_ALL_TIMERS_ON_PIN_IN_USE; + } + + if (frequency != _pwm_sm_frequencies[flexpwm_index][submodule]) { + return PWMOUT_INVALID_FREQUENCY_ON_PIN; + } + + // Submodule is already running at our target frequency and the output + // is free. + } else { + pwm_config_t pwmConfig; + + /* + * pwmConfig.enableDebugMode = false; + * pwmConfig.enableWait = false; + * pwmConfig.reloadSelect = kPWM_LocalReload; + * pwmConfig.faultFilterCount = 0; + * pwmConfig.faultFilterPeriod = 0; + * pwmConfig.clockSource = kPWM_BusClock; + * pwmConfig.prescale = kPWM_Prescale_Divide_1; + * pwmConfig.initializationControl = kPWM_Initialize_LocalSync; + * pwmConfig.forceTrigger = kPWM_Force_Local; + * pwmConfig.reloadFrequency = kPWM_LoadEveryOportunity; + * pwmConfig.reloadLogic = kPWM_ReloadImmediate; + * pwmConfig.pairOperation = kPWM_Independent; + */ + PWM_GetDefaultConfig(&pwmConfig); + + pwmConfig.reloadLogic = kPWM_ReloadPwmFullCycle; + pwmConfig.enableWait = true; + pwmConfig.enableDebugMode = true; + + pwmConfig.prescale = self->prescaler; + + if (PWM_Init(flexpwm, submodule, &pwmConfig) != kStatus_Success) { + return PWMOUT_INITIALIZATION_ERROR; + } + + // Disable all fault inputs + flexpwm->SM[submodule].DISMAP[0] = 0; + flexpwm->SM[submodule].DISMAP[1] = 0; + + PWM_SetPwmLdok(flexpwm, sm_mask, false); + flexpwm->SM[submodule].CTRL = PWM_CTRL_FULL_MASK | PWM_CTRL_PRSC(self->prescaler); + flexpwm->SM[submodule].CTRL2 = PWM_CTRL2_INDEP_MASK | PWM_CTRL2_WAITEN_MASK | PWM_CTRL2_DBGEN_MASK; + // Set the reload value to zero so we're in unsigned mode. + flexpwm->SM[submodule].INIT = 0; + // Set the top/reload value. + flexpwm->SM[submodule].VAL1 = self->pulse_count; + // Clear the other channels. + flexpwm->SM[submodule].VAL0 = 0; + flexpwm->SM[submodule].VAL2 = 0; + flexpwm->SM[submodule].VAL3 = 0; + flexpwm->SM[submodule].VAL4 = 0; + flexpwm->SM[submodule].VAL5 = 0; + PWM_SetPwmLdok(flexpwm, sm_mask, true); + + PWM_StartTimer(flexpwm, sm_mask); + _pwm_sm_frequencies[flexpwm_index][submodule] = frequency; + + if (variable_frequency) { + _pwm_variable_frequency[flexpwm_index] = sm_mask; } - base->MCTRL |= PWM_MCTRL_LDOK(mask); } - debug_print_flexpwm_registers(self->pwm->pwm); - PWM_SetPwmLdok(self->pwm->pwm, 1 << self->pwm->submodule, true); - - PWM_StartTimer(self->pwm->pwm, 1 << self->pwm->submodule); - - - DBGPrintf(&mp_plat_print, "\tCall common_hal_pwmio_pwmout_set_duty_cycle\n"); common_hal_pwmio_pwmout_set_duty_cycle(self, duty); - DBGPrintf(&mp_plat_print, "\tReturn OK\n"); + flexpwm->OUTEN |= outen_mask; + + // Configure the IOMUX once we know everything else is working. + config_periph_pin(self->pwm); + return PWMOUT_OK; } @@ -247,8 +284,29 @@ void common_hal_pwmio_pwmout_deinit(pwmio_pwmout_obj_t *self) { return; } + _pwm_never_reset[self->flexpwm_index] &= ~(1 << (self->pwm->submodule * 4 + self->pwm->channel)); + + PWM_Type *flexpwm = self->pwm->pwm; + pwm_submodule_t submodule = self->pwm->submodule; + uint16_t sm_mask = 1 << submodule; + + // Reset the pin before we turn it off. common_hal_reset_pin(self->pin); self->pin = NULL; + + // Always disable the output. + flexpwm->OUTEN &= ~_outen_mask(submodule, self->pwm->channel); + + uint16_t all_sm_channels = _outen_mask(submodule, kPWM_PwmX) | _outen_mask(submodule, kPWM_PwmA) | _outen_mask(submodule, kPWM_PwmB); + + // Turn off the submodule if it doesn't have any outputs active. + if ((flexpwm->OUTEN & all_sm_channels) == 0) { + // Deinit ourselves because the SDK turns off the clock to the whole FlexPWM on deinit. + flexpwm->MCTRL &= ~(sm_mask << PWM_MCTRL_RUN_SHIFT); + _pwm_variable_frequency[self->flexpwm_index] &= ~sm_mask; + _pwm_sm_frequencies[self->flexpwm_index][submodule] = 0; + } + _maybe_disable_clock(self->flexpwm_index); } void common_hal_pwmio_pwmout_set_duty_cycle(pwmio_pwmout_obj_t *self, uint16_t duty) { @@ -261,39 +319,40 @@ void common_hal_pwmio_pwmout_set_duty_cycle(pwmio_pwmout_obj_t *self, uint16_t d // X - As mentioned above VAL1 turns off, but it's set to the timing for frequency. so // VAL0 turns on, so we set it to VAL1 - duty - DBGPrintf(&mp_plat_print, "common_hal_pwmio_pwmout_set_duty_cycle %u\n", duty); self->duty_cycle = duty; PWM_Type *base = self->pwm->pwm; - uint8_t mask = 1 << self->pwm->submodule; + uint8_t sm_mask = 1 << self->pwm->submodule; + uint16_t duty_scaled; if (duty == 65535) { - self->duty_scaled = self->pulse_count + 1; + // X channels can't do a full 100% duty cycle. + if (self->pwm->channel == kPWM_PwmX) { + mp_raise_ValueError_varg(translate("Invalid %q"), MP_QSTR_duty_cycle); + } + duty_scaled = self->pulse_count + 1; } else { - self->duty_scaled = ((uint32_t)duty * self->pulse_count + self->pulse_count / 2) / 65535; + duty_scaled = ((uint32_t)duty * self->pulse_count) / 65535; } + PWM_SetPwmLdok(self->pwm->pwm, sm_mask, false); switch (self->pwm->channel) { case kPWM_PwmX: - base->SM[self->pwm->submodule].VAL0 = self->pulse_count - self->duty_scaled; - base->OUTEN |= PWM_OUTEN_PWMX_EN(mask); + // PWM X Signals always having a falling edge at the reload value. (Otherwise we'd + // change the PWM frequency.) So, we adjust the rising edge to get the correct duty + // cycle. + base->SM[self->pwm->submodule].VAL0 = self->pulse_count - duty_scaled; break; case kPWM_PwmA: - base->SM[self->pwm->submodule].VAL3 = self->duty_scaled; - base->OUTEN |= PWM_OUTEN_PWMA_EN(mask); + // The other two channels always have their rising edge at 0 and vary their falling + // edge. + base->SM[self->pwm->submodule].VAL3 = duty_scaled; break; case kPWM_PwmB: - base->SM[self->pwm->submodule].VAL5 = self->duty_scaled; - base->OUTEN |= PWM_OUTEN_PWMB_EN(mask); + base->SM[self->pwm->submodule].VAL5 = duty_scaled; } - PWM_SetPwmLdok(self->pwm->pwm, 1 << self->pwm->submodule, true); - - debug_print_flexpwm_registers(self->pwm->pwm); - + PWM_SetPwmLdok(self->pwm->pwm, sm_mask, true); } uint16_t common_hal_pwmio_pwmout_get_duty_cycle(pwmio_pwmout_obj_t *self) { - if (self->duty_cycle == 65535) { - return 65535; - } - return ((uint32_t)self->duty_scaled * 65535 + 65535 / 2) / self->pulse_count; + return self->duty_cycle; } void common_hal_pwmio_pwmout_set_frequency(pwmio_pwmout_obj_t *self, @@ -309,6 +368,8 @@ void common_hal_pwmio_pwmout_set_frequency(pwmio_pwmout_obj_t *self, // a small glitch can occur when adjusting the prescaler, from the setting // of CTRL just below to the setting of the Ldok register in // set_duty_cycle. + // Clear LDOK so that we can update the values. + PWM_SetPwmLdok(self->pwm->pwm, 1 << self->pwm->submodule, false); uint32_t reg = self->pwm->pwm->SM[self->pwm->submodule].CTRL; reg &= ~(PWM_CTRL_PRSC_MASK); reg |= PWM_CTRL_PRSC(self->prescaler); diff --git a/ports/mimxrt10xx/common-hal/pwmio/PWMOut.h b/ports/mimxrt10xx/common-hal/pwmio/PWMOut.h index fa4ce46780..6542d67e1f 100644 --- a/ports/mimxrt10xx/common-hal/pwmio/PWMOut.h +++ b/ports/mimxrt10xx/common-hal/pwmio/PWMOut.h @@ -37,10 +37,12 @@ typedef struct { const mcu_pin_obj_t *pin; const mcu_pwm_obj_t *pwm; bool variable_frequency; + uint8_t flexpwm_index; uint8_t prescaler; - uint16_t duty_cycle, duty_scaled, pulse_count; + uint16_t duty_cycle; + uint16_t pulse_count; } pwmio_pwmout_obj_t; -void pwmout_reset(void); +void reset_all_flexpwm(void); #endif // MICROPY_INCLUDED_MIMXRT10XX_COMMON_HAL_PWMIO_PWMOUT_H diff --git a/ports/mimxrt10xx/supervisor/port.c b/ports/mimxrt10xx/supervisor/port.c index 2ac7995943..13ccf96c8c 100644 --- a/ports/mimxrt10xx/supervisor/port.c +++ b/ports/mimxrt10xx/supervisor/port.c @@ -284,7 +284,7 @@ void reset_port(void) { // eic_reset(); #if CIRCUITPY_PWMIO - pwmout_reset(); + reset_all_flexpwm(); #endif #if CIRCUITPY_RTC diff --git a/ports/nrf/common-hal/pwmio/PWMOut.c b/ports/nrf/common-hal/pwmio/PWMOut.c index 1bd38e7a6e..f73e3d3952 100644 --- a/ports/nrf/common-hal/pwmio/PWMOut.c +++ b/ports/nrf/common-hal/pwmio/PWMOut.c @@ -67,25 +67,11 @@ STATIC int pwm_idx(NRF_PWM_Type *pwm) { } void common_hal_pwmio_pwmout_never_reset(pwmio_pwmout_obj_t *self) { - for (size_t i = 0; i < MP_ARRAY_SIZE(pwms); i++) { - NRF_PWM_Type *pwm = pwms[i]; - if (pwm == self->pwm) { - never_reset_pwm[i] += 1; - } - } + never_reset_pwm[pwm_idx(self->pwm)] |= 1 << self->channel; common_hal_never_reset_pin(self->pin); } -void common_hal_pwmio_pwmout_reset_ok(pwmio_pwmout_obj_t *self) { - for (size_t i = 0; i < MP_ARRAY_SIZE(pwms); i++) { - NRF_PWM_Type *pwm = pwms[i]; - if (pwm == self->pwm) { - never_reset_pwm[i] -= 1; - } - } -} - STATIC void reset_single_pwmout(uint8_t i) { NRF_PWM_Type *pwm = pwms[i]; @@ -114,7 +100,13 @@ STATIC void reset_single_pwmout(uint8_t i) { void pwmout_reset(void) { for (size_t i = 0; i < MP_ARRAY_SIZE(pwms); i++) { - if (never_reset_pwm[i] > 0) { + for (size_t c = 0; c < CHANNELS_PER_PWM; c++) { + if ((never_reset_pwm[i] & (1 << c)) != 0) { + continue; + } + pwms[i]->PSEL.OUT[c] = 0xFFFFFFFF; + } + if (never_reset_pwm[i] != 0) { continue; } reset_single_pwmout(i); @@ -270,6 +262,8 @@ void common_hal_pwmio_pwmout_deinit(pwmio_pwmout_obj_t *self) { nrf_gpio_cfg_default(self->pin->number); + never_reset_pwm[pwm_idx(self->pwm)] &= ~(1 << self->channel); + NRF_PWM_Type *pwm = self->pwm; self->pwm = NULL; diff --git a/ports/raspberrypi/common-hal/pwmio/PWMOut.c b/ports/raspberrypi/common-hal/pwmio/PWMOut.c index f75c4b3451..7925a734fb 100644 --- a/ports/raspberrypi/common-hal/pwmio/PWMOut.c +++ b/ports/raspberrypi/common-hal/pwmio/PWMOut.c @@ -89,20 +89,12 @@ void pwmout_never_reset(uint8_t slice, uint8_t ab_channel) { never_reset_channel |= _mask(slice, ab_channel); } -void pwmout_reset_ok(uint8_t slice, uint8_t ab_channel) { - never_reset_channel &= ~_mask(slice, ab_channel); -} - void common_hal_pwmio_pwmout_never_reset(pwmio_pwmout_obj_t *self) { pwmout_never_reset(self->slice, self->ab_channel); never_reset_pin_number(self->pin->number); } -void common_hal_pwmio_pwmout_reset_ok(pwmio_pwmout_obj_t *self) { - pwmout_reset_ok(self->slice, self->ab_channel); -} - void pwmout_reset(void) { // Reset all slices for (size_t slice = 0; slice < NUM_PWM_SLICES; slice++) { diff --git a/ports/stm/common-hal/pwmio/PWMOut.c b/ports/stm/common-hal/pwmio/PWMOut.c index cc712497d2..a983718ff2 100644 --- a/ports/stm/common-hal/pwmio/PWMOut.c +++ b/ports/stm/common-hal/pwmio/PWMOut.c @@ -40,7 +40,8 @@ STATIC uint8_t tim_channels_taken[TIM_BANK_ARRAY_LEN]; // Initial frequency timer is set to. STATIC uint32_t tim_frequencies[TIM_BANK_ARRAY_LEN]; -STATIC bool never_reset_tim[TIM_BANK_ARRAY_LEN]; +STATIC uint8_t never_reset_tim[TIM_BANK_ARRAY_LEN]; +STATIC TIM_HandleTypeDef *active_handles[TIM_BANK_ARRAY_LEN]; STATIC uint32_t timer_get_internal_duty(uint16_t duty, uint32_t period) { // duty cycle is duty/0xFFFF fraction x (number of pulses per period) @@ -64,10 +65,25 @@ STATIC bool timer_get_optimal_divisors(uint32_t *period, uint32_t *prescaler, void pwmout_reset(void) { for (int i = 0; i < TIM_BANK_ARRAY_LEN; i++) { - if (!never_reset_tim[i]) { - tim_channels_taken[i] = 0x00; - tim_frequencies[i] = 0; + if (active_handles[i] == NULL) { + continue; } + for (int c = 0; c < 8; c++) { + if ((never_reset_tim[i] & (1 << c)) != 0 || + (tim_channels_taken[i] & (1 << c)) == 0) { + continue; + } + HAL_TIM_PWM_Stop(active_handles[i], c); + } + // TODO: Actually shut down individual channels and PWM. + if (never_reset_tim[i] != 0) { + continue; + } + tim_channels_taken[i] = 0x00; + tim_frequencies[i] = 0; + stm_peripherals_timer_free(mcu_tim_banks[i]); + HAL_TIM_PWM_DeInit(active_handles[i]); + active_handles[i] = NULL; } } @@ -176,6 +192,7 @@ pwmout_result_t common_hal_pwmio_pwmout_construct(pwmio_pwmout_obj_t *self, if (HAL_TIM_PWM_Init(&self->handle) != HAL_OK) { return PWMOUT_INITIALIZATION_ERROR; } + active_handles[tim_index] = &self->handle; } // Channel/PWM init @@ -208,15 +225,6 @@ void common_hal_pwmio_pwmout_never_reset(pwmio_pwmout_obj_t *self) { } } -void common_hal_pwmio_pwmout_reset_ok(pwmio_pwmout_obj_t *self) { - for (size_t i = 0; i < TIM_BANK_ARRAY_LEN; i++) { - if (mcu_tim_banks[i] == self->handle.Instance) { - never_reset_tim[i] = false; - break; - } - } -} - bool common_hal_pwmio_pwmout_deinited(pwmio_pwmout_obj_t *self) { return self->tim == NULL; } @@ -234,9 +242,13 @@ void common_hal_pwmio_pwmout_deinit(pwmio_pwmout_obj_t *self) { } common_hal_reset_pin(self->pin); + never_reset_tim[self->tim->tim_index] &= ~(1 << self->tim->channel_index); + // if reserved timer has no active channels, we can disable it if (tim_channels_taken[self->tim->tim_index] == 0) { tim_frequencies[self->tim->tim_index] = 0x00; + HAL_TIM_PWM_DeInit(&self->handle); + active_handles[self->tim->tim_index] = NULL; stm_peripherals_timer_free(self->handle.Instance); } diff --git a/shared-module/displayio/Display.c b/shared-module/displayio/Display.c index 8367e23b59..5af97a4144 100644 --- a/shared-module/displayio/Display.c +++ b/shared-module/displayio/Display.c @@ -423,7 +423,6 @@ void release_display(displayio_display_obj_t *self) { release_display_core(&self->core); #if (CIRCUITPY_PWMIO) if (self->backlight_pwm.base.type == &pwmio_pwmout_type) { - common_hal_pwmio_pwmout_reset_ok(&self->backlight_pwm); common_hal_pwmio_pwmout_deinit(&self->backlight_pwm); } else if (self->backlight_inout.base.type == &digitalio_digitalinout_type) { common_hal_digitalio_digitalinout_deinit(&self->backlight_inout);