Skip to content

Commit

Permalink
ledc: Fix FADE_NO_WAIT mode concurrency problem.
Browse files Browse the repository at this point in the history 
Add test cases for fade concurrency issue and fade timing check.

Closes #6710

(cherry picked from commit be2ab09)
  • Loading branch information
@songruo @espressif-bot
songruo authored and espressif-bot committed Mar 29, 2022
1 parent 01547c4 commit 9e79d0e
Show file tree
Hide file tree
Showing 3 changed files with 137 additions and 100 deletions.
3 changes: 2 additions & 1 deletion components/driver/include/driver/ledc.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,8 @@ void ledc_fade_func_uninstall(void);
* Other duty operations will have to wait until the fade operation has finished.
* @param speed_mode Select the LEDC channel group with specified speed mode. Note that not all targets support high speed mode.
* @param channel LEDC channel number
* @param fade_mode Whether to block until fading done.
* @param fade_mode Whether to block until fading done. See ledc_types.h ledc_fade_mode_t for more info.
* Note that this function will not return until fading to the target duty if LEDC_FADE_WAIT_DONE mode is selected.
*
* @return
* - ESP_OK Success
Expand Down
28 changes: 11 additions & 17 deletions components/driver/ledc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ esp_err_t ledc_timer_set(ledc_mode_t speed_mode, ledc_timer_t timer_sel, uint32_
static IRAM_ATTR esp_err_t ledc_duty_config(ledc_mode_t speed_mode, ledc_channel_t channel, int hpoint_val, int duty_val,
ledc_duty_direction_t duty_direction, uint32_t duty_num, uint32_t duty_cycle, uint32_t duty_scale)
{
portENTER_CRITICAL(&ledc_spinlock);
portENTER_CRITICAL_SAFE(&ledc_spinlock);
if (hpoint_val >= 0) {
ledc_hal_set_hpoint(&(p_ledc_obj[speed_mode]->ledc_hal), channel, hpoint_val);
}
Expand All @@ -211,7 +211,7 @@ static IRAM_ATTR esp_err_t ledc_duty_config(ledc_mode_t speed_mode, ledc_channel
ledc_hal_set_duty_cycle(&(p_ledc_obj[speed_mode]->ledc_hal), channel, duty_cycle);
ledc_hal_set_duty_scale(&(p_ledc_obj[speed_mode]->ledc_hal), channel, duty_scale);
ledc_ls_channel_update(speed_mode, channel);
portEXIT_CRITICAL(&ledc_spinlock);
portEXIT_CRITICAL_SAFE(&ledc_spinlock);
return ESP_OK;
}

Expand Down Expand Up @@ -573,9 +573,9 @@ void IRAM_ATTR ledc_fade_isr(void* arg)
ledc_calc_fade_end_channel(&intr_status, &channel);

// clear interrupt
portENTER_CRITICAL(&ledc_spinlock);
portENTER_CRITICAL_ISR(&ledc_spinlock);
ledc_hal_clear_fade_end_intr_status(&(p_ledc_obj[speed_mode]->ledc_hal), channel);
portEXIT_CRITICAL(&ledc_spinlock);
portEXIT_CRITICAL_ISR(&ledc_spinlock);

if (s_ledc_fade_rec[speed_mode][channel] == NULL) {
//fade object not initialized yet.
Expand Down Expand Up @@ -621,9 +621,9 @@ void IRAM_ATTR ledc_fade_isr(void* arg)
1,
0);
}
portENTER_CRITICAL(&ledc_spinlock);
portENTER_CRITICAL_ISR(&ledc_spinlock);
ledc_hal_set_duty_start(&(p_ledc_obj[speed_mode]->ledc_hal), channel, true);
portEXIT_CRITICAL(&ledc_spinlock);
portEXIT_CRITICAL_ISR(&ledc_spinlock);
}
}
}
Expand Down Expand Up @@ -763,7 +763,10 @@ static void _ledc_fade_start(ledc_mode_t speed_mode, ledc_channel_t channel, led
ledc_enable_intr_type(speed_mode, channel, LEDC_INTR_FADE_END);
ledc_update_duty(speed_mode, channel);
if (fade_mode == LEDC_FADE_WAIT_DONE) {
xSemaphoreTake(s_ledc_fade_rec[speed_mode][channel]->ledc_fade_sem, portMAX_DELAY);
// Waiting for fade done
_ledc_fade_hw_acquire(speed_mode, channel);
// Release hardware to support next time fade configure
_ledc_fade_hw_release(speed_mode, channel);
}
}

Expand Down Expand Up @@ -805,7 +808,6 @@ esp_err_t ledc_fade_start(ledc_mode_t speed_mode, ledc_channel_t channel, ledc_f
LEDC_CHECK(p_ledc_obj[speed_mode] != NULL, LEDC_NOT_INIT, ESP_ERR_INVALID_STATE);
_ledc_fade_hw_acquire(speed_mode, channel);
_ledc_fade_start(speed_mode, channel, fade_mode);
_ledc_fade_hw_release(speed_mode, channel);
return ESP_OK;
}

Expand Down Expand Up @@ -844,13 +846,11 @@ esp_err_t ledc_set_duty_and_update(ledc_mode_t speed_mode, ledc_channel_t channe
LEDC_ARG_CHECK(duty <= ledc_get_max_duty(speed_mode, channel), "target_duty");
LEDC_ARG_CHECK(hpoint <= LEDC_HPOINT_VAL_MAX, "hpoint");
LEDC_CHECK(p_ledc_obj[speed_mode] != NULL, LEDC_NOT_INIT, ESP_ERR_INVALID_STATE);
LEDC_CHECK(ledc_fade_channel_init_check(speed_mode, channel) == ESP_OK , LEDC_FADE_INIT_ERROR_STR, ESP_FAIL);
_ledc_op_lock_acquire(speed_mode, channel);
LEDC_CHECK(ledc_fade_channel_init_check(speed_mode, channel) == ESP_OK, LEDC_FADE_INIT_ERROR_STR, ESP_FAIL);
_ledc_fade_hw_acquire(speed_mode, channel);
ledc_duty_config(speed_mode, channel, hpoint, duty, 1, 0, 0, 0);
ledc_update_duty(speed_mode, channel);
_ledc_fade_hw_release(speed_mode, channel);
_ledc_op_lock_release(speed_mode, channel);
return ESP_OK;
}

Expand All @@ -866,9 +866,6 @@ esp_err_t ledc_set_fade_time_and_start(ledc_mode_t speed_mode, ledc_channel_t ch
_ledc_fade_hw_acquire(speed_mode, channel);
_ledc_set_fade_with_time(speed_mode, channel, target_duty, max_fade_time_ms);
_ledc_fade_start(speed_mode, channel, fade_mode);
if (fade_mode == LEDC_FADE_WAIT_DONE) {
_ledc_fade_hw_release(speed_mode, channel);
}
_ledc_op_lock_release(speed_mode, channel);
return ESP_OK;
}
Expand All @@ -887,9 +884,6 @@ esp_err_t ledc_set_fade_step_and_start(ledc_mode_t speed_mode, ledc_channel_t ch
_ledc_fade_hw_acquire(speed_mode, channel);
_ledc_set_fade_with_step(speed_mode, channel, target_duty, scale, cycle_num);
_ledc_fade_start(speed_mode, channel, fade_mode);
if (fade_mode == LEDC_FADE_WAIT_DONE) {
_ledc_fade_hw_release(speed_mode, channel);
}
_ledc_op_lock_release(speed_mode, channel);
return ESP_OK;
}
206 changes: 124 additions & 82 deletions components/driver/test/test_ledc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@
#define HIGHEST_LIMIT 10000
#define LOWEST_LIMIT -10000

#ifdef CONFIG_IDF_TARGET_ESP32
#define TEST_SPEED_MODE LEDC_HIGH_SPEED_MODE
#elif defined CONFIG_IDF_TARGET_ESP32S2
#define TEST_SPEED_MODE LEDC_LOW_SPEED_MODE
#endif

#if !TEMPORARY_DISABLED_FOR_TARGETS(ESP32S2)
//no runners

Expand Down Expand Up @@ -99,12 +105,39 @@ static void timer_frequency_test(ledc_channel_t channel, ledc_timer_bit_t timer_
frequency_set_get(ledc_ch_config.speed_mode, ledc_ch_config.timer_sel, 9000, 9025, 5);
}

static void fade_setup(void)
{
ledc_channel_config_t ledc_ch_config = {
.gpio_num = PULSE_IO,
.speed_mode = TEST_SPEED_MODE,
.channel = LEDC_CHANNEL_0,
.intr_type = LEDC_INTR_DISABLE,
.timer_sel = LEDC_TIMER_0,
.duty = 0,
.hpoint = 0,
};
ledc_timer_config_t ledc_time_config = {
.speed_mode = TEST_SPEED_MODE,
.duty_resolution = LEDC_TIMER_13_BIT,
.timer_num = LEDC_TIMER_0,
.freq_hz = 2000,
.clk_cfg = LEDC_USE_APB_CLK,
};

TEST_ESP_OK(ledc_channel_config(&ledc_ch_config));
TEST_ESP_OK(ledc_timer_config(&ledc_time_config));
vTaskDelay(5 / portTICK_PERIOD_MS);

//initialize fade service
TEST_ESP_OK(ledc_fade_func_install(0));
}

static void timer_duty_set_get(ledc_mode_t speed_mode, ledc_channel_t channel, uint32_t duty)
{
TEST_ESP_OK(ledc_set_duty(speed_mode, channel, duty));
TEST_ESP_OK(ledc_update_duty(speed_mode, channel));
vTaskDelay(1000 / portTICK_RATE_MS);
TEST_ASSERT_EQUAL_INT32(ledc_get_duty(speed_mode, channel), duty);
vTaskDelay(100 / portTICK_RATE_MS);
TEST_ASSERT_EQUAL_INT32(duty, ledc_get_duty(speed_mode, channel));
}

// use logic analyzer to view
Expand Down Expand Up @@ -277,9 +310,9 @@ TEST_CASE("LEDC set and get frequency", "[ledc][test_env=UT_T1_LEDC][timeout=60]
timer_frequency_test(LEDC_CHANNEL_0, LEDC_TIMER_13_BIT, LEDC_TIMER_3, LEDC_LOW_SPEED_MODE);
}

// the duty need to be detected by waveform given by the logic analyzer
// can't get it directly, so set it "ignore"
TEST_CASE("LEDC set and get dut(with logic analyzer)", "[ledc][ignore]")
// duty should be manually checked from the waveform using a logic analyzer
// this test is enabled only for testting the settings
TEST_CASE("LEDC set and get duty", "[ledc]")
{
ledc_timer_t timer_list[4] = {LEDC_TIMER_0, LEDC_TIMER_1, LEDC_TIMER_2, LEDC_TIMER_3};
#ifdef CONFIG_IDF_TARGET_ESP32
Expand Down Expand Up @@ -331,7 +364,7 @@ TEST_CASE("LEDC timer set", "[ledc][test_env=UT_T1_LEDC]")
count = wave_count(1000);
TEST_ASSERT_UINT32_WITHIN(10, count, freq_get);

//set timer 3 as 500Hz, use APB_CLK
//set timer 0 as 500Hz, use APB_CLK
TEST_ESP_OK(ledc_timer_set(test_speed_mode, LEDC_TIMER_0, 5000, 13, LEDC_APB_CLK));
TEST_ESP_OK(ledc_timer_rst(test_speed_mode, LEDC_TIMER_0));
TEST_ASSERT_EQUAL_INT32(ledc_get_freq(test_speed_mode, LEDC_TIMER_0), 500);
Expand Down Expand Up @@ -403,102 +436,111 @@ TEST_CASE("LEDC timer pause and resume", "[ledc][test_env=UT_T1_LEDC]")
TEST_ASSERT_UINT32_WITHIN(5, count, 5000);
}

TEST_CASE("LEDC fade with time(logic analyzer)", "[ledc][test_env=UT_T1_LEDC]")
TEST_CASE("LEDC fade with time", "[ledc]")
{
#ifdef CONFIG_FREERTOS_CHECK_PORT_CRITICAL_COMPLIANCE
return;
#endif
const ledc_mode_t test_speed_mode = TEST_SPEED_MODE;
fade_setup();

#ifdef CONFIG_IDF_TARGET_ESP32
const ledc_mode_t test_speed_mode = LEDC_HIGH_SPEED_MODE;
#elif defined CONFIG_IDF_TARGET_ESP32S2
const ledc_mode_t test_speed_mode = LEDC_LOW_SPEED_MODE;
#endif
ledc_channel_config_t ledc_ch_config = {
.gpio_num = PULSE_IO,
.speed_mode = test_speed_mode,
.channel = LEDC_CHANNEL_0,
.intr_type = LEDC_INTR_DISABLE,
.timer_sel = LEDC_TIMER_0,
.duty = 0,
.hpoint = 0,
};
TEST_ESP_OK(ledc_channel_config(&ledc_ch_config));
TEST_ESP_OK(ledc_set_fade_with_time(test_speed_mode, LEDC_CHANNEL_0, 4000, 200));
TEST_ESP_OK(ledc_fade_start(test_speed_mode, LEDC_CHANNEL_0, LEDC_FADE_WAIT_DONE));
TEST_ASSERT_EQUAL_INT32(4000, ledc_get_duty(test_speed_mode, LEDC_CHANNEL_0));

ledc_timer_config_t ledc_time_config = {
.speed_mode = test_speed_mode,
.duty_resolution = LEDC_TIMER_13_BIT,
.timer_num = LEDC_TIMER_0,
.freq_hz = 5000,
.clk_cfg = LEDC_USE_APB_CLK,
};
TEST_ESP_OK(ledc_timer_config(&ledc_time_config));
TEST_ESP_OK(ledc_set_fade_with_time(test_speed_mode, LEDC_CHANNEL_0, 0, 200));
TEST_ESP_OK(ledc_fade_start(test_speed_mode, LEDC_CHANNEL_0, LEDC_FADE_NO_WAIT));
// duty should not be too far from initial value
TEST_ASSERT_INT32_WITHIN(20, 4000, ledc_get_duty(test_speed_mode, LEDC_CHANNEL_0));
vTaskDelay(210 / portTICK_PERIOD_MS);
TEST_ASSERT_EQUAL_INT32(0, ledc_get_duty(test_speed_mode, LEDC_CHANNEL_0));

//initialize fade service
TEST_ESP_OK(ledc_fade_func_install(0));
//deinitialize fade service
ledc_fade_func_uninstall();
}

TEST_CASE("LEDC fade with step", "[ledc]")
{
const ledc_mode_t test_speed_mode = TEST_SPEED_MODE;
fade_setup();

TEST_ESP_OK(ledc_set_fade_with_time(test_speed_mode, LEDC_CHANNEL_0, 4000, 1000));
TEST_ESP_OK(ledc_set_fade_with_step(test_speed_mode, LEDC_CHANNEL_0, 4000, 4, 1));
TEST_ESP_OK(ledc_fade_start(test_speed_mode, LEDC_CHANNEL_0, LEDC_FADE_WAIT_DONE));
vTaskDelay(1000 / portTICK_RATE_MS);
TEST_ASSERT_EQUAL_INT32(ledc_get_duty(test_speed_mode, LEDC_CHANNEL_0), 4000);
TEST_ASSERT_EQUAL_INT32(4000, ledc_get_duty(test_speed_mode, LEDC_CHANNEL_0));

TEST_ESP_OK(ledc_set_fade_with_time(test_speed_mode, LEDC_CHANNEL_0, 0, 1000));
TEST_ESP_OK(ledc_set_fade_with_step(test_speed_mode, LEDC_CHANNEL_0, 0, 4, 1));
TEST_ESP_OK(ledc_fade_start(test_speed_mode, LEDC_CHANNEL_0, LEDC_FADE_NO_WAIT));
vTaskDelay(1000 / portTICK_RATE_MS);
TEST_ASSERT_EQUAL_INT32(ledc_get_duty(test_speed_mode, LEDC_CHANNEL_0), 0);
// duty should not be too far from initial value
TEST_ASSERT_INT32_WITHIN(20, 4000, ledc_get_duty(test_speed_mode, LEDC_CHANNEL_0));
vTaskDelay(525 / portTICK_PERIOD_MS);
TEST_ASSERT_EQUAL_INT32(0, ledc_get_duty(test_speed_mode, LEDC_CHANNEL_0));

//scaler=0 check
TEST_ASSERT(ledc_set_fade_with_step(test_speed_mode, LEDC_CHANNEL_0, 4000, 0, 1) == ESP_ERR_INVALID_ARG);

//deinitial fade service
//deinitialize fade service
ledc_fade_func_uninstall();
}

TEST_CASE("LEDC fade with step(logic analyzer)", "[ledc][test_env=UT_T1_LEDC]")
TEST_CASE("LEDC fast switching duty with fade_wait_done", "[ledc]")
{
#ifdef CONFIG_FREERTOS_CHECK_PORT_CRITICAL_COMPLIANCE
return;
#endif
const ledc_mode_t test_speed_mode = TEST_SPEED_MODE;
fade_setup();

#ifdef CONFIG_IDF_TARGET_ESP32
const ledc_mode_t test_speed_mode = LEDC_HIGH_SPEED_MODE;
#elif defined CONFIG_IDF_TARGET_ESP32S2
const ledc_mode_t test_speed_mode = LEDC_LOW_SPEED_MODE;
#endif
ledc_channel_config_t ledc_ch_config = {
.gpio_num = PULSE_IO,
.speed_mode = test_speed_mode,
.channel = LEDC_CHANNEL_0,
.intr_type = LEDC_INTR_DISABLE,
.timer_sel = LEDC_TIMER_0,
.duty = 0,
.hpoint = 0,
};
TEST_ESP_OK(ledc_channel_config(&ledc_ch_config));
// fade function will block until fading to the target duty
int64_t fade_start, fade_stop;
fade_start = esp_timer_get_time();
TEST_ESP_OK(ledc_set_fade_with_time(test_speed_mode, LEDC_CHANNEL_0, 4000, 200));
TEST_ESP_OK(ledc_fade_start(test_speed_mode, LEDC_CHANNEL_0, LEDC_FADE_WAIT_DONE));
TEST_ASSERT_EQUAL_INT32(4000, ledc_get_duty(test_speed_mode, LEDC_CHANNEL_0));
TEST_ESP_OK(ledc_set_fade_with_time(test_speed_mode, LEDC_CHANNEL_0, 1000, 150));
TEST_ESP_OK(ledc_fade_start(test_speed_mode, LEDC_CHANNEL_0, LEDC_FADE_WAIT_DONE));
TEST_ASSERT_EQUAL_INT32(1000, ledc_get_duty(test_speed_mode, LEDC_CHANNEL_0));
fade_stop = esp_timer_get_time();
int time_ms = (fade_stop - fade_start) / 1000;
TEST_ASSERT_TRUE(fabs(time_ms - 350) < 20);

ledc_timer_config_t ledc_time_config = {
.speed_mode = test_speed_mode,
.duty_resolution = LEDC_TIMER_13_BIT,
.timer_num = LEDC_TIMER_0,
.freq_hz = 5000,
.clk_cfg = LEDC_USE_APB_CLK,
};
TEST_ESP_OK(ledc_timer_config(&ledc_time_config));
// next duty update will not take place until last fade reaches its target duty
TEST_ESP_OK(ledc_set_fade_with_time(test_speed_mode, LEDC_CHANNEL_0, 4000, 200));
TEST_ESP_OK(ledc_fade_start(test_speed_mode, LEDC_CHANNEL_0, LEDC_FADE_WAIT_DONE));
TEST_ASSERT_EQUAL_INT32(4000, ledc_get_duty(test_speed_mode, LEDC_CHANNEL_0));
TEST_ESP_OK(ledc_set_duty(test_speed_mode, LEDC_CHANNEL_0, 500));
TEST_ESP_OK(ledc_update_duty(test_speed_mode, LEDC_CHANNEL_0));
vTaskDelay(5 / portTICK_PERIOD_MS);
TEST_ASSERT_EQUAL_INT32(500, ledc_get_duty(test_speed_mode, LEDC_CHANNEL_0));

//initialize fade service.
TEST_ESP_OK(ledc_fade_func_install(0));
//deinitialize fade service
ledc_fade_func_uninstall();
}

TEST_ESP_OK(ledc_set_fade_with_step(test_speed_mode, LEDC_CHANNEL_0, 4000, 2, 1));
TEST_ESP_OK(ledc_fade_start(test_speed_mode, LEDC_CHANNEL_0, LEDC_FADE_WAIT_DONE));
vTaskDelay(1000 / portTICK_RATE_MS);
TEST_ASSERT_EQUAL_INT32(ledc_get_duty(test_speed_mode, LEDC_CHANNEL_0), 4000);
TEST_CASE("LEDC fast switching duty with fade_no_wait", "[ledc]")
{
const ledc_mode_t test_speed_mode = TEST_SPEED_MODE;
fade_setup();

TEST_ESP_OK(ledc_set_fade_with_step(test_speed_mode, LEDC_CHANNEL_0, 0, 4, 2));
// fade function returns immediately, but next fade still needs to wait for last fade ends
int64_t fade_start, first_fade_complete;
fade_start = esp_timer_get_time();
TEST_ESP_OK(ledc_set_fade_with_time(test_speed_mode, LEDC_CHANNEL_0, 4000, 200));
TEST_ESP_OK(ledc_fade_start(test_speed_mode, LEDC_CHANNEL_0, LEDC_FADE_NO_WAIT));
vTaskDelay(1000 / portTICK_RATE_MS);
TEST_ASSERT_EQUAL_INT32(ledc_get_duty(test_speed_mode, LEDC_CHANNEL_0), 0);

//scaler=0 check
TEST_ASSERT(ledc_set_fade_with_step(test_speed_mode, LEDC_CHANNEL_0, 4000, 0, 1) == ESP_ERR_INVALID_ARG);
TEST_ASSERT_LESS_THAN(4000, ledc_get_duty(test_speed_mode, LEDC_CHANNEL_0));
TEST_ESP_OK(ledc_set_fade_with_time(test_speed_mode, LEDC_CHANNEL_0, 1000, 150));
TEST_ESP_OK(ledc_fade_start(test_speed_mode, LEDC_CHANNEL_0, LEDC_FADE_NO_WAIT));
first_fade_complete = esp_timer_get_time();
// duty should not be too far from first fade target duty
TEST_ASSERT_INT32_WITHIN(20, 4000, ledc_get_duty(test_speed_mode, LEDC_CHANNEL_0));
int time_ms = (first_fade_complete - fade_start) / 1000;
TEST_ASSERT_TRUE(fabs(time_ms - 200) < 20);
vTaskDelay(158 / portTICK_PERIOD_MS);
TEST_ASSERT_EQUAL_INT32(1000, ledc_get_duty(test_speed_mode, LEDC_CHANNEL_0));

// next duty update will not take place until last fade reaches its target duty
TEST_ESP_OK(ledc_set_fade_with_time(test_speed_mode, LEDC_CHANNEL_0, 4000, 200));
TEST_ESP_OK(ledc_fade_start(test_speed_mode, LEDC_CHANNEL_0, LEDC_FADE_NO_WAIT));
TEST_ASSERT_LESS_THAN(4000, ledc_get_duty(test_speed_mode, LEDC_CHANNEL_0));
TEST_ESP_OK(ledc_set_duty(test_speed_mode, LEDC_CHANNEL_0, 500));
TEST_ESP_OK(ledc_update_duty(test_speed_mode, LEDC_CHANNEL_0));
vTaskDelay(5 / portTICK_PERIOD_MS);
TEST_ASSERT_EQUAL_INT32(500, ledc_get_duty(test_speed_mode, LEDC_CHANNEL_0));

//deinitial fade service
//deinitialize fade service
ledc_fade_func_uninstall();
}

Expand Down

0 comments on commit 9e79d0e

Please sign in to comment.