Skip to content

Commit

Permalink
fix(rmt): fixed unstable transfer during DFS
Browse files Browse the repository at this point in the history 
Closes #12292
  • Loading branch information
@suda-morris
suda-morris committed Sep 27, 2023
1 parent 6c9b620 commit 3639236
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 76 deletions.
37 changes: 21 additions & 16 deletions components/driver/rmt/rmt_common.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,36 +124,40 @@ esp_err_t rmt_select_periph_clock(rmt_channel_handle_t chan, rmt_clock_source_t
ESP_RETURN_ON_FALSE(!clock_selection_conflict, ESP_ERR_INVALID_STATE, TAG,
"group clock conflict, already is %d but attempt to %d", group->clk_src, clk_src);

#if CONFIG_PM_ENABLE
// if DMA is not used, we're using CPU to push the data to the RMT FIFO
// if the CPU frequency goes down, the transfer+encoding scheme could be unstable because CPU can't fill the data in time
// so, choose ESP_PM_CPU_FREQ_MAX lock for non-dma mode
// otherwise, chose lock type based on the clock source
esp_pm_lock_type_t pm_lock_type = chan->dma_chan ? ESP_PM_NO_LIGHT_SLEEP : ESP_PM_CPU_FREQ_MAX;

#if SOC_RMT_SUPPORT_APB
if (clk_src == RMT_CLK_SRC_APB) {
// APB clock frequency can be changed during DFS
pm_lock_type = ESP_PM_APB_FREQ_MAX;
}
#endif // SOC_RMT_SUPPORT_APB

sprintf(chan->pm_lock_name, "rmt_%d_%d", group->group_id, channel_id); // e.g. rmt_0_0
ret = esp_pm_lock_create(pm_lock_type, 0, chan->pm_lock_name, &chan->pm_lock);
ESP_RETURN_ON_ERROR(ret, TAG, "create pm lock failed");
#endif

// [clk_tree] TODO: replace the following switch table by clk_tree API
switch (clk_src) {
#if SOC_RMT_SUPPORT_APB
case RMT_CLK_SRC_APB:
periph_src_clk_hz = esp_clk_apb_freq();
#if CONFIG_PM_ENABLE
sprintf(chan->pm_lock_name, "rmt_%d_%d", group->group_id, channel_id); // e.g. rmt_0_0
ret = esp_pm_lock_create(ESP_PM_APB_FREQ_MAX, 0, chan->pm_lock_name, &chan->pm_lock);
ESP_RETURN_ON_ERROR(ret, TAG, "create APB_FREQ_MAX lock failed");
ESP_LOGD(TAG, "install APB_FREQ_MAX lock for RMT channel (%d,%d)", group->group_id, channel_id);
#endif // CONFIG_PM_ENABLE
#endif // SOC_RMT_SUPPORT_APB
break;
#if SOC_RMT_SUPPORT_AHB
case RMT_CLK_SRC_AHB:
// TODO: decide which kind of PM lock we should use for such clock
periph_src_clk_hz = 48 * 1000 * 1000;
break;
#endif // SOC_RMT_SUPPORT_AHB
#if SOC_RMT_SUPPORT_XTAL
case RMT_CLK_SRC_XTAL:
periph_src_clk_hz = esp_clk_xtal_freq();
#if CONFIG_PM_ENABLE
sprintf(chan->pm_lock_name, "rmt_%d_%d", group->group_id, channel_id); // e.g. rmt_0_0
// XTAL will be power down in the light sleep (predefined low power modes)
// acquire a NO_LIGHT_SLEEP lock here to prevent the system go into light sleep automatically
ret = esp_pm_lock_create(ESP_PM_NO_LIGHT_SLEEP, 0, chan->pm_lock_name, &chan->pm_lock);
ESP_RETURN_ON_ERROR(ret, TAG, "create NO_LIGHT_SLEEP lock failed");
ESP_LOGD(TAG, "install NO_LIGHT_SLEEP lock for RMT channel (%d,%d)", group->group_id, channel_id);
#endif // CONFIG_PM_ENABLE
break;
#endif // SOC_RMT_SUPPORT_XTAL
#if SOC_RMT_SUPPORT_REF_TICK
Expand Down Expand Up @@ -240,7 +244,8 @@ bool rmt_set_intr_priority_to_group(rmt_group_t *group, int intr_priority)
return priority_conflict;
}

int rmt_get_isr_flags(rmt_group_t *group) {
int rmt_get_isr_flags(rmt_group_t *group)
{
int isr_flags = RMT_INTR_ALLOC_FLAG;
if (group->intr_priority) {
// Use user-specified priority bit
Expand Down
4 changes: 2 additions & 2 deletions components/driver/rmt/rmt_rx.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,8 +215,6 @@ esp_err_t rmt_new_rx_channel(const rmt_rx_channel_config_t *config, rmt_channel_
rmt_hal_context_t *hal = &group->hal;
int channel_id = rx_channel->base.channel_id;
int group_id = group->group_id;
// select the clock source
ESP_GOTO_ON_ERROR(rmt_select_periph_clock(&rx_channel->base, config->clk_src), err, TAG, "set group clock failed");

// reset channel, make sure the RX engine is not working, and events are cleared
portENTER_CRITICAL(&group->spinlock);
Expand Down Expand Up @@ -245,6 +243,8 @@ esp_err_t rmt_new_rx_channel(const rmt_rx_channel_config_t *config, rmt_channel_
ESP_GOTO_ON_ERROR(ret, err, TAG, "install rx interrupt failed");
}

// select the clock source
ESP_GOTO_ON_ERROR(rmt_select_periph_clock(&rx_channel->base, config->clk_src), err, TAG, "set group clock failed");
// set channel clock resolution
uint32_t real_div = group->resolution_hz / config->resolution_hz;
rmt_ll_rx_set_channel_clock_div(hal->regs, channel_id, real_div);
Expand Down
4 changes: 2 additions & 2 deletions components/driver/rmt/rmt_tx.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,6 @@ esp_err_t rmt_new_tx_channel(const rmt_tx_channel_config_t *config, rmt_channel_
rmt_hal_context_t *hal = &group->hal;
int channel_id = tx_channel->base.channel_id;
int group_id = group->group_id;
// select the clock source
ESP_GOTO_ON_ERROR(rmt_select_periph_clock(&tx_channel->base, config->clk_src), err, TAG, "set group clock failed");

// reset channel, make sure the TX engine is not working, and events are cleared
portENTER_CRITICAL(&group->spinlock);
Expand All @@ -267,6 +265,8 @@ esp_err_t rmt_new_tx_channel(const rmt_tx_channel_config_t *config, rmt_channel_
ESP_GOTO_ON_ERROR(rmt_tx_init_dma_link(tx_channel, config), err, TAG, "install tx DMA failed");
}
#endif
// select the clock source
ESP_GOTO_ON_ERROR(rmt_select_periph_clock(&tx_channel->base, config->clk_src), err, TAG, "set group clock failed");
// set channel clock resolution
uint32_t real_div = group->resolution_hz / config->resolution_hz;
rmt_ll_tx_set_channel_clock_div(hal->regs, channel_id, real_div);
Expand Down
111 changes: 55 additions & 56 deletions components/driver/test_apps/rmt/main/test_rmt_tx.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,10 @@ TEST_CASE("rmt_single_trans_with_dma", "[rmt]")

static void test_rmt_ping_pong_trans(size_t mem_block_symbols, bool with_dma)
{
const int test_led_num = 10000;
uint8_t *leds_grb = heap_caps_malloc(3 * test_led_num, MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT);
TEST_ASSERT_NOT_NULL(leds_grb);

rmt_tx_channel_config_t tx_channel_cfg = {
.mem_block_symbols = mem_block_symbols,
.clk_src = RMT_CLK_SRC_DEFAULT,
Expand All @@ -162,22 +166,21 @@ static void test_rmt_ping_pong_trans(size_t mem_block_symbols, bool with_dma)
TEST_ESP_OK(rmt_enable(tx_channel_multi_leds));

// Mutiple LEDs (ping-pong in the background)
printf("ping pong transmission: light up 100 RGB LEDs\r\n");
printf("ping pong transmission: light up %d RGB LEDs\r\n", test_led_num);
rmt_transmit_config_t transmit_config = {
.loop_count = 0, // no loop
};
#define TEST_LED_NUM 100
uint8_t leds_grb[TEST_LED_NUM * 3] = {};

// color: Material Design Green-A200 (#69F0AE)
for (int i = 0; i < TEST_LED_NUM * 3; i += 3) {
for (int i = 0; i < test_led_num * 3; i += 3) {
leds_grb[i + 0] = 0xF0;
leds_grb[i + 1] = 0x69;
leds_grb[i + 2] = 0xAE;
}
printf("start transmission and stop immediately, only a few LEDs are light up\r\n");
TEST_ESP_OK(rmt_transmit(tx_channel_multi_leds, led_strip_encoder, leds_grb, TEST_LED_NUM * 3, &transmit_config));
TEST_ESP_OK(rmt_transmit(tx_channel_multi_leds, led_strip_encoder, leds_grb, test_led_num * 3, &transmit_config));
// this second transmission will stay in the queue and shouldn't be dispatched until we restart the tx channel later
TEST_ESP_OK(rmt_transmit(tx_channel_multi_leds, led_strip_encoder, leds_grb, TEST_LED_NUM * 3, &transmit_config));
TEST_ESP_OK(rmt_transmit(tx_channel_multi_leds, led_strip_encoder, leds_grb, test_led_num * 3, &transmit_config));
esp_rom_delay_us(100);
TEST_ESP_OK(rmt_disable(tx_channel_multi_leds));
vTaskDelay(pdTICKS_TO_MS(500));
Expand All @@ -187,33 +190,33 @@ static void test_rmt_ping_pong_trans(size_t mem_block_symbols, bool with_dma)
// adding extra delay here for visualizing
vTaskDelay(pdTICKS_TO_MS(500));
// color: Material Design Pink-A200 (#FF4081)
for (int i = 0; i < TEST_LED_NUM * 3; i += 3) {
for (int i = 0; i < test_led_num * 3; i += 3) {
leds_grb[i + 0] = 0x40;
leds_grb[i + 1] = 0xFF;
leds_grb[i + 2] = 0x81;
}
TEST_ESP_OK(rmt_transmit(tx_channel_multi_leds, led_strip_encoder, leds_grb, TEST_LED_NUM * 3, &transmit_config));
TEST_ESP_OK(rmt_transmit(tx_channel_multi_leds, led_strip_encoder, leds_grb, test_led_num * 3, &transmit_config));
vTaskDelay(pdTICKS_TO_MS(500));
// color: Material Design Orange-900 (#E65100)
for (int i = 0; i < TEST_LED_NUM * 3; i += 3) {
for (int i = 0; i < test_led_num * 3; i += 3) {
leds_grb[i + 0] = 0x51;
leds_grb[i + 1] = 0xE6;
leds_grb[i + 2] = 0x00;
}
TEST_ESP_OK(rmt_transmit(tx_channel_multi_leds, led_strip_encoder, leds_grb, TEST_LED_NUM * 3, &transmit_config));
vTaskDelay(pdTICKS_TO_MS(500));
TEST_ESP_OK(rmt_transmit(tx_channel_multi_leds, led_strip_encoder, leds_grb, test_led_num * 3, &transmit_config));
vTaskDelay(pdTICKS_TO_MS(2000));

printf("disable tx channel\r\n");
TEST_ESP_OK(rmt_disable(tx_channel_multi_leds));
printf("remove tx channel and led strip encoder\r\n");
TEST_ESP_OK(rmt_del_channel(tx_channel_multi_leds));
TEST_ESP_OK(rmt_del_encoder(led_strip_encoder));
#undef TEST_LED_NUM
free(leds_grb);
}

TEST_CASE("rmt_ping_pong_trans_no_dma", "[rmt]")
{
test_rmt_ping_pong_trans(SOC_RMT_MEM_WORDS_PER_CHANNEL, false);
test_rmt_ping_pong_trans(SOC_RMT_MEM_WORDS_PER_CHANNEL * 2, false);
}

#if SOC_RMT_SUPPORT_DMA
Expand Down Expand Up @@ -264,15 +267,15 @@ static void test_rmt_trans_done_event(size_t mem_block_symbols, bool with_dma)
};

printf("transmit dynamic number of LEDs\r\n");
#define TEST_LED_NUM 40
uint8_t leds_grb[TEST_LED_NUM * 3] = {};
const int test_led_num = 40;
uint8_t leds_grb[test_led_num * 3];
// color: Material Design Purple-800 (6A1B9A)
for (int i = 0; i < TEST_LED_NUM * 3; i += 3) {
for (int i = 0; i < test_led_num * 3; i += 3) {
leds_grb[i + 0] = 0x1B;
leds_grb[i + 1] = 0x6A;
leds_grb[i + 2] = 0x9A;
}
for (int i = 1; i <= TEST_LED_NUM; i++) {
for (int i = 1; i <= test_led_num; i++) {
expected_encoded_size = 2 + i * 24; // 2 = 1 reset symbol + 1 eof symbol, 24 = 8*3(RGB)
TEST_ESP_OK(rmt_transmit(tx_channel_multi_leds, led_strip_encoder, leds_grb, i * 3, &transmit_config));
// wait for the transmission finished and recycled
Expand All @@ -284,7 +287,6 @@ static void test_rmt_trans_done_event(size_t mem_block_symbols, bool with_dma)
printf("remove tx channel and led strip encoder\r\n");
TEST_ESP_OK(rmt_del_channel(tx_channel_multi_leds));
TEST_ESP_OK(rmt_del_encoder(led_strip_encoder));
#undef TEST_LED_NUM
}

TEST_CASE("rmt_trans_done_event_callback_no_dma", "[rmt]")
Expand Down Expand Up @@ -341,13 +343,13 @@ static void test_rmt_loop_trans(size_t mem_block_symbols, bool with_dma)
rmt_transmit_config_t transmit_config = {
.loop_count = 5,
};
#define TEST_LED_NUM 3
uint8_t leds_grb[TEST_LED_NUM * 3] = {};
for (int i = 0; i < TEST_LED_NUM * 3; i++) {
const int test_led_num = 3;
uint8_t leds_grb[test_led_num * 3];
for (int i = 0; i < test_led_num * 3; i++) {
leds_grb[i] = 0x10 + i;
}
expected_encoded_size = 2 + 24 * TEST_LED_NUM;
TEST_ESP_OK(rmt_transmit(tx_channel_multi_leds, led_strip_encoder, leds_grb, TEST_LED_NUM * 3, &transmit_config));
expected_encoded_size = 2 + 24 * test_led_num;
TEST_ESP_OK(rmt_transmit(tx_channel_multi_leds, led_strip_encoder, leds_grb, test_led_num * 3, &transmit_config));
vTaskDelay(pdTICKS_TO_MS(100));

printf("wait for loop transactions done\r\n");
Expand All @@ -357,7 +359,6 @@ static void test_rmt_loop_trans(size_t mem_block_symbols, bool with_dma)
printf("remove tx channel and led strip encoder\r\n");
TEST_ESP_OK(rmt_del_channel(tx_channel_multi_leds));
TEST_ESP_OK(rmt_del_encoder(led_strip_encoder));
#undef TEST_LED_NUM
}

TEST_CASE("rmt_loop_trans_no_dma", "[rmt]")
Expand Down Expand Up @@ -531,49 +532,49 @@ static bool test_rmt_tx_done_cb_record_time(rmt_channel_handle_t channel, const

static void test_rmt_multi_channels_trans(size_t channel0_mem_block_symbols, size_t channel1_mem_block_symbols, bool channel0_with_dma, bool channel1_with_dma)
{
#define TEST_RMT_CHANS 2
#define TEST_LED_NUM 24
const int test_led_num = 1;
rmt_tx_channel_config_t tx_channel_cfg = {
.clk_src = RMT_CLK_SRC_DEFAULT,
.resolution_hz = 10000000, // 10MHz, 1 tick = 0.1us (led strip needs a high resolution)
.trans_queue_depth = 4,
.intr_priority = 3
};
printf("install tx channels\r\n");
rmt_channel_handle_t tx_channels[TEST_RMT_CHANS] = {NULL};
int gpio_nums[TEST_RMT_CHANS] = {0, 2};
size_t mem_blk_syms[TEST_RMT_CHANS] = {channel0_mem_block_symbols, channel1_mem_block_symbols};
bool dma_flags[TEST_RMT_CHANS] = {channel0_with_dma, channel1_with_dma};
for (int i = 0; i < TEST_RMT_CHANS; i++) {
rmt_channel_handle_t tx_channels[] = {NULL, NULL};
const int test_rmt_chans = sizeof(tx_channels) / sizeof(tx_channels[0]);
int gpio_nums[] = {0, 2};
size_t mem_blk_syms[] = {channel0_mem_block_symbols, channel1_mem_block_symbols};
bool dma_flags[] = {channel0_with_dma, channel1_with_dma};
for (int i = 0; i < test_rmt_chans; i++) {
tx_channel_cfg.gpio_num = gpio_nums[i];
tx_channel_cfg.mem_block_symbols = mem_blk_syms[i];
tx_channel_cfg.flags.with_dma = dma_flags[i];
TEST_ESP_OK(rmt_new_tx_channel(&tx_channel_cfg, &tx_channels[i]));
}

printf("install led strip encoders\r\n");
rmt_encoder_handle_t led_strip_encoders[TEST_RMT_CHANS] = {NULL};
for (int i = 0; i < TEST_RMT_CHANS; i++) {
rmt_encoder_handle_t led_strip_encoders[test_rmt_chans];
for (int i = 0; i < test_rmt_chans; i++) {
TEST_ESP_OK(test_rmt_new_led_strip_encoder(&led_strip_encoders[i]));
}

printf("register tx event callback\r\n");
rmt_tx_event_callbacks_t cbs = {
.on_trans_done = test_rmt_tx_done_cb_record_time
};
int64_t record_stop_time[TEST_RMT_CHANS] = {};
for (int i = 0; i < TEST_RMT_CHANS; i++) {
int64_t record_stop_time[test_rmt_chans];
for (int i = 0; i < test_rmt_chans; i++) {
TEST_ESP_OK(rmt_tx_register_event_callbacks(tx_channels[i], &cbs, &record_stop_time[i]));
}

printf("enable tx channels\r\n");
for (int i = 0; i < TEST_RMT_CHANS; i++) {
for (int i = 0; i < test_rmt_chans; i++) {
TEST_ESP_OK(rmt_enable(tx_channels[i]));
}

uint8_t leds_grb[TEST_LED_NUM * 3] = {};
uint8_t leds_grb[test_led_num * 3];
// color: Material Design Green-A200 (#69F0AE)
for (int i = 0; i < TEST_LED_NUM * 3; i += 3) {
for (int i = 0; i < test_led_num * 3; i += 3) {
leds_grb[i + 0] = 0xF0;
leds_grb[i + 1] = 0x69;
leds_grb[i + 2] = 0xAE;
Expand All @@ -584,14 +585,14 @@ static void test_rmt_multi_channels_trans(size_t channel0_mem_block_symbols, siz
.loop_count = 0, // no loop
};
// the channels should work independently, without synchronization
for (int i = 0; i < TEST_RMT_CHANS; i++) {
TEST_ESP_OK(rmt_transmit(tx_channels[i], led_strip_encoders[i], leds_grb, TEST_LED_NUM * 3, &transmit_config));
for (int i = 0; i < test_rmt_chans; i++) {
TEST_ESP_OK(rmt_transmit(tx_channels[i], led_strip_encoders[i], leds_grb, test_led_num * 3, &transmit_config));
}
for (int i = 0; i < TEST_RMT_CHANS; i++) {
for (int i = 0; i < test_rmt_chans; i++) {
TEST_ESP_OK(rmt_tx_wait_all_done(tx_channels[i], -1));
}
printf("stop time (no sync):\r\n");
for (int i = 0; i < TEST_RMT_CHANS; i++) {
for (int i = 0; i < test_rmt_chans; i++) {
printf("\t%lld\r\n", record_stop_time[i]);
}
// without synchronization, there will be obvious time shift
Expand All @@ -601,7 +602,7 @@ static void test_rmt_multi_channels_trans(size_t channel0_mem_block_symbols, siz
rmt_sync_manager_handle_t synchro = NULL;
rmt_sync_manager_config_t synchro_config = {
.tx_channel_array = tx_channels,
.array_size = TEST_RMT_CHANS,
.array_size = test_rmt_chans,
};
#if SOC_RMT_SUPPORT_TX_SYNCHRO
TEST_ESP_OK(rmt_new_sync_manager(&synchro_config, &synchro));
Expand All @@ -611,16 +612,16 @@ static void test_rmt_multi_channels_trans(size_t channel0_mem_block_symbols, siz

#if SOC_RMT_SUPPORT_TX_SYNCHRO
printf("transmit with synchronization\r\n");
for (int i = 0; i < TEST_RMT_CHANS; i++) {
TEST_ESP_OK(rmt_transmit(tx_channels[i], led_strip_encoders[i], leds_grb, TEST_LED_NUM * 3, &transmit_config));
for (int i = 0; i < test_rmt_chans; i++) {
TEST_ESP_OK(rmt_transmit(tx_channels[i], led_strip_encoders[i], leds_grb, test_led_num * 3, &transmit_config));
// manually introduce the delay, to show the managed channels are indeed in sync
vTaskDelay(pdMS_TO_TICKS(10));
}
for (int i = 0; i < TEST_RMT_CHANS; i++) {
for (int i = 0; i < test_rmt_chans; i++) {
TEST_ESP_OK(rmt_tx_wait_all_done(tx_channels[i], -1));
}
printf("stop time (with sync):\r\n");
for (int i = 0; i < TEST_RMT_CHANS; i++) {
for (int i = 0; i < test_rmt_chans; i++) {
printf("\t%lld\r\n", record_stop_time[i]);
}
// because of synchronization, the managed channels will stop at the same time
Expand All @@ -630,16 +631,16 @@ static void test_rmt_multi_channels_trans(size_t channel0_mem_block_symbols, siz
printf("reset sync manager\r\n");
TEST_ESP_OK(rmt_sync_reset(synchro));
printf("transmit with synchronization again\r\n");
for (int i = 0; i < TEST_RMT_CHANS; i++) {
TEST_ESP_OK(rmt_transmit(tx_channels[i], led_strip_encoders[i], leds_grb, TEST_LED_NUM * 3, &transmit_config));
for (int i = 0; i < test_rmt_chans; i++) {
TEST_ESP_OK(rmt_transmit(tx_channels[i], led_strip_encoders[i], leds_grb, test_led_num * 3, &transmit_config));
// manually introduce the delay, ensure the channels get synchronization
vTaskDelay(pdMS_TO_TICKS(10));
}
for (int i = 0; i < TEST_RMT_CHANS; i++) {
for (int i = 0; i < test_rmt_chans; i++) {
TEST_ESP_OK(rmt_tx_wait_all_done(tx_channels[i], -1));
}
printf("stop time (with sync):\r\n");
for (int i = 0; i < TEST_RMT_CHANS; i++) {
for (int i = 0; i < test_rmt_chans; i++) {
printf("\t%lld\r\n", record_stop_time[i]);
}
TEST_ASSERT((record_stop_time[1] - record_stop_time[0]) < 10);
Expand All @@ -649,18 +650,16 @@ static void test_rmt_multi_channels_trans(size_t channel0_mem_block_symbols, siz
#endif // SOC_RMT_SUPPORT_TX_SYNCHRO

printf("disable tx channels\r\n");
for (int i = 0; i < TEST_RMT_CHANS; i++) {
for (int i = 0; i < test_rmt_chans; i++) {
TEST_ESP_OK(rmt_disable(tx_channels[i]));
}
printf("delete channels and encoders\r\n");
for (int i = 0; i < TEST_RMT_CHANS; i++) {
for (int i = 0; i < test_rmt_chans; i++) {
TEST_ESP_OK(rmt_del_channel(tx_channels[i]));
}
for (int i = 0; i < TEST_RMT_CHANS; i++) {
for (int i = 0; i < test_rmt_chans; i++) {
TEST_ESP_OK(rmt_del_encoder(led_strip_encoders[i]));
}
#undef TEST_LED_NUM
#undef TEST_RMT_CHANS
}

TEST_CASE("rmt_multi_channels_trans_no_dma", "[rmt]")
Expand Down

0 comments on commit 3639236

Please sign in to comment.