From 01c97acaac7cc81bee3d831861ce40af3f8d5564 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Tue, 28 Oct 2025 07:11:33 +0200 Subject: [PATCH 01/27] i2s_out: one line of i2s_out_dma_buffer() DEBUG --- components/i2s_out/esp32/dma.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/components/i2s_out/esp32/dma.c b/components/i2s_out/esp32/dma.c index 23d9ca80..2096d6f1 100644 --- a/components/i2s_out/esp32/dma.c +++ b/components/i2s_out/esp32/dma.c @@ -254,7 +254,7 @@ size_t i2s_out_dma_buffer(struct i2s_out *i2s_out, void **ptr, unsigned count, s // hit dma_eof_desc? if (desc->owner || desc->eof) { - LOG_DEBUG("eof desc=%p (owner=%u eof=%u buf=%p len=%u size=%u)", desc, desc->owner, desc->eof, desc->buf, desc->len, desc->size); + LOG_DEBUG("eof desc=%p (owner=%u eof=%u buf=%p len=%u size=%u) -> NULL[0]", desc, desc->owner, desc->eof, desc->buf, desc->len, desc->size); // unable to find a usable DMA buffer, TX buffers full *ptr = NULL; @@ -274,19 +274,17 @@ size_t i2s_out_dma_buffer(struct i2s_out *i2s_out, void **ptr, unsigned count, s } if (desc->len + count * size > desc->size) { - LOG_DEBUG("limited desc=%p (owner=%u eof=%u buf=%p len=%u size=%u) < count=%u size=%u", desc, desc->owner, desc->eof, desc->buf, desc->len, desc->size, count, size); + LOG_DEBUG("limit desc=%p (owner=%u eof=%u buf=%p len=%u size=%u) -> count=%u size=%u", desc, desc->owner, desc->eof, desc->buf, desc->len, desc->size, count, size); // limit to available buffer size count = (desc->size - desc->len) / size; } else { - LOG_DEBUG("complete desc=%p (owner=%u eof=%u buf=%p len=%u size=%u) >= count=%u size=%u", desc, desc->owner, desc->eof, desc->buf, desc->len, desc->size, count, size); + LOG_DEBUG("full desc=%p (owner=%u eof=%u buf=%p len=%u size=%u) -> count=%u size=%u", desc, desc->owner, desc->eof, desc->buf, desc->len, desc->size, count, size); } *ptr = desc->buf + desc->len; - LOG_DEBUG("return ptr=%p count=%u size=%u", *ptr, count, size); - return count; } } From 549b02174b74aaf4a7d514dcc97ad49684064a0e Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Tue, 28 Oct 2025 07:21:11 +0200 Subject: [PATCH 02/27] i2s_out: unset dma eof desc next loop to fix OUT_DSCR_ERR interrupt --- components/i2s_out/esp32/dma.c | 2 +- components/i2s_out/esp32/dma.h | 5 +++++ components/i2s_out/esp32/intr.c | 6 +++++- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/components/i2s_out/esp32/dma.c b/components/i2s_out/esp32/dma.c index 2096d6f1..655b75db 100644 --- a/components/i2s_out/esp32/dma.c +++ b/components/i2s_out/esp32/dma.c @@ -374,7 +374,7 @@ void i2s_out_dma_start(struct i2s_out *i2s_out) } i2s_out->dma_eof_desc->owner = 1; - i2s_out->dma_eof_desc->next = i2s_out->dma_eof_desc; + i2s_out->dma_eof_desc->next = NULL; for (unsigned i = 0; i < i2s_out->dma_rx_count; i++) { LOG_DEBUG("dma_rx_desc[%u]=%p: owner=%d eof=%d len=%u size=%u buf=%p next=%p", i, diff --git a/components/i2s_out/esp32/dma.h b/components/i2s_out/esp32/dma.h index a0b076a8..38a00d77 100644 --- a/components/i2s_out/esp32/dma.h +++ b/components/i2s_out/esp32/dma.h @@ -19,3 +19,8 @@ struct dma_desc { // linked list struct dma_desc *next; }; + +static inline void i2s_dma_tx_get_des_addr(i2s_dev_t *hw, uint32_t *dscr_addr) +{ + *dscr_addr = hw->out_link_dscr; +} diff --git a/components/i2s_out/esp32/intr.c b/components/i2s_out/esp32/intr.c index c991c31a..c2f72fb7 100644 --- a/components/i2s_out/esp32/intr.c +++ b/components/i2s_out/esp32/intr.c @@ -19,7 +19,11 @@ static const int i2s_irq[I2S_PORT_MAX] = { void IRAM_ATTR i2s_intr_out_dscr_err_handler(struct i2s_out *i2s_out, BaseType_t *task_wokenp) { - LOG_ISR_WARN(""); + uint32_t dscr_addr; + + i2s_dma_tx_get_des_addr(i2s_out->dev, &dscr_addr); + + LOG_ISR_WARN("desc=%p", dscr_addr); i2s_intr_clear(i2s_out->dev, I2S_OUT_DSCR_ERR_INT_CLR); } From 24006d02aa974a6700b9012fabb7fda90e64992d Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Tue, 28 Oct 2025 07:37:08 +0200 Subject: [PATCH 03/27] i2s_out: use I2S_OUT_TOTAL_EOF_INT --- components/i2s_out/esp32/dma.c | 8 ++++---- components/i2s_out/esp32/intr.c | 28 +++++++++++++++++++++------- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/components/i2s_out/esp32/dma.c b/components/i2s_out/esp32/dma.c index 655b75db..06a60703 100644 --- a/components/i2s_out/esp32/dma.c +++ b/components/i2s_out/esp32/dma.c @@ -196,8 +196,8 @@ int i2s_out_dma_setup(struct i2s_out *i2s_out, const struct i2s_out_options *opt i2s_ll_rx_stop_link(i2s_out->dev); i2s_ll_tx_stop_link(i2s_out->dev); - i2s_intr_disable(i2s_out->dev, I2S_OUT_EOF_INT_ENA | I2S_OUT_DSCR_ERR_INT_ENA); - i2s_intr_clear(i2s_out->dev, I2S_OUT_EOF_INT_CLR | I2S_OUT_DSCR_ERR_INT_CLR); + i2s_intr_disable(i2s_out->dev, I2S_OUT_TOTAL_EOF_INT_ENA | I2S_OUT_DSCR_ERR_INT_ENA | I2S_OUT_EOF_INT_ENA); + i2s_intr_clear(i2s_out->dev, I2S_OUT_TOTAL_EOF_INT_CLR | I2S_OUT_DSCR_ERR_INT_CLR | I2S_OUT_EOF_INT_CLR); i2s_ll_enable_dma(i2s_out->dev, false); @@ -418,8 +418,8 @@ void i2s_out_dma_start(struct i2s_out *i2s_out) i2s_ll_tx_reset_fifo(i2s_out->dev); i2s_ll_tx_reset(i2s_out->dev); - i2s_intr_clear(i2s_out->dev, I2S_OUT_EOF_INT_CLR | I2S_OUT_DSCR_ERR_INT_CLR); - i2s_intr_enable(i2s_out->dev, I2S_OUT_EOF_INT_ENA | I2S_OUT_DSCR_ERR_INT_ENA); + i2s_intr_clear(i2s_out->dev, I2S_OUT_TOTAL_EOF_INT_CLR | I2S_OUT_DSCR_ERR_INT_CLR | I2S_OUT_EOF_INT_CLR); + i2s_intr_enable(i2s_out->dev, I2S_OUT_TOTAL_EOF_INT_ENA | I2S_OUT_DSCR_ERR_INT_ENA | I2S_OUT_EOF_INT_ENA); i2s_ll_enable_dma(i2s_out->dev, true); i2s_ll_start_out_link(i2s_out->dev); diff --git a/components/i2s_out/esp32/intr.c b/components/i2s_out/esp32/intr.c index c2f72fb7..47090678 100644 --- a/components/i2s_out/esp32/intr.c +++ b/components/i2s_out/esp32/intr.c @@ -17,6 +17,24 @@ static const int i2s_irq[I2S_PORT_MAX] = { [I2S_PORT_1] = ETS_I2S1_INTR_SOURCE, }; +void IRAM_ATTR i2s_intr_out_total_eof_handler(struct i2s_out *i2s_out, BaseType_t *task_wokenp) +{ + uint32_t eof_addr; + + i2s_ll_tx_get_eof_des_addr(i2s_out->dev, &eof_addr); + + LOG_ISR_DEBUG("desc=%p", eof_addr); + + // NOTE: this is unlikely to stop DMA before this repeats at least once + i2s_ll_tx_stop_link(i2s_out->dev); + i2s_ll_rx_stop_link(i2s_out->dev); + + // unblock flush() task + xEventGroupSetBitsFromISR(i2s_out->event_group, I2S_OUT_EVENT_GROUP_BIT_DMA_EOF, task_wokenp); + + i2s_intr_clear(i2s_out->dev, I2S_OUT_TOTAL_EOF_INT_CLR); +} + void IRAM_ATTR i2s_intr_out_dscr_err_handler(struct i2s_out *i2s_out, BaseType_t *task_wokenp) { uint32_t dscr_addr; @@ -38,13 +56,6 @@ void IRAM_ATTR i2s_intr_out_eof_handler(struct i2s_out *i2s_out, BaseType_t *tas LOG_ISR_DEBUG("desc=%p", eof_desc); - // NOTE: this is unlikely to stop DMA before this repeats at least once - i2s_ll_tx_stop_link(i2s_out->dev); - i2s_ll_rx_stop_link(i2s_out->dev); - - // unblock flush() task - xEventGroupSetBitsFromISR(i2s_out->event_group, I2S_OUT_EVENT_GROUP_BIT_DMA_EOF, task_wokenp); - i2s_intr_clear(i2s_out->dev, I2S_OUT_EOF_INT_CLR); } @@ -72,6 +83,9 @@ void IRAM_ATTR i2s_intr_handler(void *arg) taskENTER_CRITICAL_ISR(&i2s_out->mux); + if (int_st & I2S_OUT_TOTAL_EOF_INT_ST) { + i2s_intr_out_total_eof_handler(i2s_out, &task_woken); + } if (int_st & I2S_OUT_DSCR_ERR_INT_ST) { i2s_intr_out_dscr_err_handler(i2s_out, &task_woken); } From 59a5213958e2e511dc0b2fca6beb17dd33d9aa1e Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Tue, 28 Oct 2025 08:03:45 +0200 Subject: [PATCH 04/27] i2s_out: simplify i2s_out_dma_buffer(), use eof isr to detect free dma links --- components/i2s_out/esp32/dma.c | 73 ++++++++++++++++----------------- components/i2s_out/esp32/intr.c | 2 +- 2 files changed, 37 insertions(+), 38 deletions(-) diff --git a/components/i2s_out/esp32/dma.c b/components/i2s_out/esp32/dma.c index 06a60703..9150469e 100644 --- a/components/i2s_out/esp32/dma.c +++ b/components/i2s_out/esp32/dma.c @@ -47,6 +47,7 @@ void init_dma_desc(struct dma_desc *head, unsigned count, uint8_t *buf, size_t s desc->size = (size > TRUNC(DMA_DESC_SIZE_MAX, align)) ? TRUNC(DMA_DESC_SIZE_MAX, align) : size; desc->len = 0; + desc->eof = 1; // trigger I2S_OUT_EOF_INT on each DMA link desc->owner = 0; desc->buf = buf; @@ -65,17 +66,6 @@ void init_dma_desc(struct dma_desc *head, unsigned count, uint8_t *buf, size_t s } /* Prepare desc for DMA start */ -struct dma_desc *commit_dma_desc(struct dma_desc *desc) -{ - desc->owner = 1; - - if (desc->next) { - return desc->next; - } else { - return desc; - } -} - void init_dma_eof_desc(struct dma_desc *eof_desc, uint32_t value, unsigned count) { uint32_t *ptr = (uint32_t *) eof_desc->buf; @@ -85,7 +75,6 @@ void init_dma_eof_desc(struct dma_desc *eof_desc, uint32_t value, unsigned count } eof_desc->len = count * sizeof(value); - eof_desc->eof = 1; } void reinit_dma_desc(struct dma_desc *head, unsigned count, struct dma_desc *next) @@ -249,44 +238,54 @@ int i2s_out_dma_setup(struct i2s_out *i2s_out, const struct i2s_out_options *opt */ size_t i2s_out_dma_buffer(struct i2s_out *i2s_out, void **ptr, unsigned count, size_t size) { - for (;;) { - struct dma_desc *desc = i2s_out->dma_write_desc; + struct dma_desc *desc = i2s_out->dma_write_desc; - // hit dma_eof_desc? - if (desc->owner || desc->eof) { - LOG_DEBUG("eof desc=%p (owner=%u eof=%u buf=%p len=%u size=%u) -> NULL[0]", desc, desc->owner, desc->eof, desc->buf, desc->len, desc->size); + // stop if last desc already committed + if (desc->owner) { + LOG_WARN("owned desc=%p (owner=%u eof=%u buf=%p len=%u size=%u)", desc, desc->owner, desc->eof, desc->buf, desc->len, desc->size); - // unable to find a usable DMA buffer, TX buffers full - *ptr = NULL; + // unable to find a usable DMA buffer, TX buffers full + *ptr = NULL; - // TODO: start DMA early and wait for a free buffer? - return 0; - } + return 0; + } - // can fit minimum size - if (desc->len + size > desc->size) { - LOG_DEBUG("commit desc=%p (owner=%u eof=%u buf=%p len=%u size=%u) < size=%u -> next=%p", desc, desc->owner, desc->eof, desc->buf, desc->len, desc->size, size, desc->next); + // advance to next desc if full + if (desc->len + size > desc->size) { + LOG_DEBUG("commit desc=%p (owner=%u eof=%u buf=%p len=%u size=%u) < size=%u -> next=%p", desc, desc->owner, desc->eof, desc->buf, desc->len, desc->size, size, desc->next); - // commit, try with the next desc, if available - i2s_out->dma_write_desc = commit_dma_desc(desc); + // commit, try with the next desc, if available + desc->owner = 1; - continue; + if (desc->next) { + desc = i2s_out->dma_write_desc = desc->next; } + } - if (desc->len + count * size > desc->size) { - LOG_DEBUG("limit desc=%p (owner=%u eof=%u buf=%p len=%u size=%u) -> count=%u size=%u", desc, desc->owner, desc->eof, desc->buf, desc->len, desc->size, count, size); + if (desc->len + size > desc->size) { + LOG_WARN("overflow desc=%p (owner=%u eof=%u buf=%p len=%u size=%u) < size=%u", desc, desc->owner, desc->eof, desc->buf, desc->len, desc->size, size); - // limit to available buffer size - count = (desc->size - desc->len) / size; + // unable to find a usable DMA buffer, TX buffers full + *ptr = NULL; - } else { - LOG_DEBUG("full desc=%p (owner=%u eof=%u buf=%p len=%u size=%u) -> count=%u size=%u", desc, desc->owner, desc->eof, desc->buf, desc->len, desc->size, count, size); - } + return 0; + + } else if (desc->len + count * size > desc->size) { + // limit to available buffer size + count = (desc->size - desc->len) / size; - *ptr = desc->buf + desc->len; + LOG_DEBUG("short desc=%p (owner=%u eof=%u buf=%p len=%u size=%u) -> count=%u size=%u", desc, desc->owner, desc->eof, desc->buf, desc->len, desc->size, count, size); - return count; + } else if (desc->len + count * size < desc->size) { + LOG_DEBUG("long desc=%p (owner=%u eof=%u buf=%p len=%u size=%u) -> count=%u size=%u", desc, desc->owner, desc->eof, desc->buf, desc->len, desc->size, count, size); + + } else { + LOG_DEBUG("fill desc=%p (owner=%u eof=%u buf=%p len=%u size=%u) -> count=%u size=%u", desc, desc->owner, desc->eof, desc->buf, desc->len, desc->size, count, size); } + + *ptr = desc->buf + desc->len; + + return count; } void i2s_out_dma_commit(struct i2s_out *i2s_out, unsigned count, size_t size) diff --git a/components/i2s_out/esp32/intr.c b/components/i2s_out/esp32/intr.c index 47090678..82e92caa 100644 --- a/components/i2s_out/esp32/intr.c +++ b/components/i2s_out/esp32/intr.c @@ -54,7 +54,7 @@ void IRAM_ATTR i2s_intr_out_eof_handler(struct i2s_out *i2s_out, BaseType_t *tas struct dma_desc *eof_desc = (struct dma_desc *) eof_addr; - LOG_ISR_DEBUG("desc=%p", eof_desc); + LOG_ISR_DEBUG("desc=%p owner=%u", eof_desc, eof_desc->owner); i2s_intr_clear(i2s_out->dev, I2S_OUT_EOF_INT_CLR); } From 10bc01440d1aebb65fbc723676c7034645f17c1c Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Tue, 28 Oct 2025 08:18:04 +0200 Subject: [PATCH 05/27] i2s_out: rename dma rx -> out --- components/i2s_out/esp32/dma.c | 82 +++++++++++++++++++--------------- components/i2s_out/i2s_out.c | 5 +-- components/i2s_out/i2s_out.h | 7 +-- 3 files changed, 52 insertions(+), 42 deletions(-) diff --git a/components/i2s_out/esp32/dma.c b/components/i2s_out/esp32/dma.c index 9150469e..3d9ed613 100644 --- a/components/i2s_out/esp32/dma.c +++ b/components/i2s_out/esp32/dma.c @@ -125,11 +125,11 @@ int i2s_out_dma_init(struct i2s_out *i2s_out, size_t size, size_t align, unsigne LOG_DEBUG("size=%u align=%u repeat=%u -> desc_count=%u buf_size=%u", size, align, repeat, desc_count, buf_size); // allocate single word-aligned buffer - if (!(i2s_out->dma_rx_buf = dma_malloc(buf_size))) { - LOG_ERROR("dma_malloc(dma_rx_buf)"); + if (!(i2s_out->dma_out_buf = dma_malloc(buf_size))) { + LOG_ERROR("dma_malloc(dma_out_buf)"); return -1; } else { - LOG_DEBUG("dma_rx_buf=%p[%u]", i2s_out->dma_rx_buf, buf_size); + LOG_DEBUG("dma_out_buf=%p[%u]", i2s_out->dma_out_buf, buf_size); } if (!(i2s_out->dma_eof_buf = dma_malloc(DMA_EOF_BUF_SIZE))) { LOG_ERROR("dma_malloc(dma_eof_buf)"); @@ -139,11 +139,11 @@ int i2s_out_dma_init(struct i2s_out *i2s_out, size_t size, size_t align, unsigne } // allocate DMA descriptors - if (!(i2s_out->dma_rx_desc = dma_calloc(desc_count, sizeof(*i2s_out->dma_rx_desc)))) { - LOG_ERROR("dma_calloc(dma_rx_desc)"); + if (!(i2s_out->dma_out_desc = dma_calloc(desc_count, sizeof(*i2s_out->dma_out_desc)))) { + LOG_ERROR("dma_calloc(dma_out_desc)"); return -1; } - if (repeat && !(i2s_out->dma_repeat_desc = dma_calloc(desc_count * repeat, sizeof(*i2s_out->dma_rx_desc)))) { + if (repeat && !(i2s_out->dma_repeat_desc = dma_calloc(desc_count * repeat, sizeof(*i2s_out->dma_repeat_desc)))) { LOG_ERROR("dma_calloc(dma_repeat_desc)"); return -1; } @@ -153,14 +153,14 @@ int i2s_out_dma_init(struct i2s_out *i2s_out, size_t size, size_t align, unsigne } // initialize linked list of DMA descriptors - init_dma_desc(i2s_out->dma_rx_desc, desc_count, i2s_out->dma_rx_buf, buf_size, align, NULL); + init_dma_desc(i2s_out->dma_out_desc, desc_count, i2s_out->dma_out_buf, buf_size, align, NULL); for (unsigned i = 0; i < repeat; i++) { - init_dma_desc(i2s_out->dma_repeat_desc + i * desc_count, desc_count, i2s_out->dma_rx_buf, buf_size, align, NULL); + init_dma_desc(i2s_out->dma_repeat_desc + i * desc_count, desc_count, i2s_out->dma_out_buf, buf_size, align, NULL); } init_dma_desc(i2s_out->dma_eof_desc, 1, i2s_out->dma_eof_buf, DMA_EOF_BUF_SIZE, sizeof(uint32_t), NULL); - i2s_out->dma_rx_count = desc_count; - i2s_out->dma_rx_repeat = repeat; + i2s_out->dma_out_count = desc_count; + i2s_out->dma_repeat_count = repeat; return 0; } @@ -178,7 +178,7 @@ int i2s_out_dma_setup(struct i2s_out *i2s_out, const struct i2s_out_options *opt init_dma_eof_desc(i2s_out->dma_eof_desc, options->eof_value, options->eof_count); // init RX desc - reinit_dma_desc(i2s_out->dma_rx_desc, i2s_out->dma_rx_count, NULL); + reinit_dma_desc(i2s_out->dma_out_desc, i2s_out->dma_out_count, NULL); taskENTER_CRITICAL(&i2s_out->mux); @@ -197,7 +197,7 @@ int i2s_out_dma_setup(struct i2s_out *i2s_out, const struct i2s_out_options *opt i2s_ll_dma_enable_owner_check(i2s_out->dev, true); i2s_ll_dma_enable_auto_write_back(i2s_out->dev, true); - i2s_ll_set_out_link_addr(i2s_out->dev, (uint32_t) i2s_out->dma_rx_desc); + i2s_ll_set_out_link_addr(i2s_out->dev, (uint32_t) i2s_out->dma_out_desc); taskEXIT_CRITICAL(&i2s_out->mux); @@ -206,7 +206,7 @@ int i2s_out_dma_setup(struct i2s_out *i2s_out, const struct i2s_out_options *opt // reset write state i2s_out->dma_start = false; - i2s_out->dma_write_desc = i2s_out->dma_rx_desc; + i2s_out->dma_write_desc = i2s_out->dma_out_desc; LOG_DEBUG("dma_write_desc=%p: owner=%d eof=%d len=%u size=%u -> buf=%p next=%p", i2s_out->dma_write_desc, @@ -321,9 +321,9 @@ void i2s_out_dma_repeat(struct i2s_out *i2s_out, unsigned count) i2s_out->dma_write_desc->owner = 1; for (unsigned i = 0; i < count; i++) { - for (unsigned j = 0; j < i2s_out->dma_rx_count; j++) { - struct dma_desc *s = &i2s_out->dma_rx_desc[j]; - struct dma_desc *d = &i2s_out->dma_repeat_desc[i * i2s_out->dma_rx_count + j]; + for (unsigned j = 0; j < i2s_out->dma_out_count; j++) { + struct dma_desc *s = &i2s_out->dma_out_desc[j]; + struct dma_desc *d = &i2s_out->dma_repeat_desc[i * i2s_out->dma_out_count + j]; if (!s->owner) { break; @@ -353,7 +353,7 @@ int i2s_out_dma_pending(struct i2s_out *i2s_out) return 0; } - if (i2s_out->dma_write_desc != i2s_out->dma_rx_desc || i2s_out->dma_write_desc->len > 0) { + if (i2s_out->dma_write_desc != i2s_out->dma_out_desc || i2s_out->dma_write_desc->len > 0) { // write() happened return 1; } @@ -375,28 +375,31 @@ void i2s_out_dma_start(struct i2s_out *i2s_out) i2s_out->dma_eof_desc->owner = 1; i2s_out->dma_eof_desc->next = NULL; - for (unsigned i = 0; i < i2s_out->dma_rx_count; i++) { - LOG_DEBUG("dma_rx_desc[%u]=%p: owner=%d eof=%d len=%u size=%u buf=%p next=%p", i, - &i2s_out->dma_rx_desc[i], - i2s_out->dma_rx_desc[i].owner, - i2s_out->dma_rx_desc[i].eof, - i2s_out->dma_rx_desc[i].len, - i2s_out->dma_rx_desc[i].size, - i2s_out->dma_rx_desc[i].buf, - i2s_out->dma_rx_desc[i].next +#undef DEBUG +#define DEBUG 1 + + for (unsigned i = 0; i < i2s_out->dma_out_count; i++) { + LOG_DEBUG("dma_out_desc[%u]=%p: owner=%d eof=%d len=%u size=%u buf=%p next=%p", i, + &i2s_out->dma_out_desc[i], + i2s_out->dma_out_desc[i].owner, + i2s_out->dma_out_desc[i].eof, + i2s_out->dma_out_desc[i].len, + i2s_out->dma_out_desc[i].size, + i2s_out->dma_out_desc[i].buf, + i2s_out->dma_out_desc[i].next ); } - for (unsigned i = 0; i < i2s_out->dma_rx_repeat; i++) { - for (unsigned j = 0; j < i2s_out->dma_rx_count; j++) { + for (unsigned i = 0; i < i2s_out->dma_repeat_count; i++) { + for (unsigned j = 0; j < i2s_out->dma_out_count; j++) { LOG_DEBUG("dma_repeat_desc[%u][%u]=%p: owner=%d eof=%d len=%u size=%u buf=%p next=%p", i, j, - &i2s_out->dma_repeat_desc[i * i2s_out->dma_rx_count + j], - i2s_out->dma_repeat_desc[i * i2s_out->dma_rx_count + j].owner, - i2s_out->dma_repeat_desc[i * i2s_out->dma_rx_count + j].eof, - i2s_out->dma_repeat_desc[i * i2s_out->dma_rx_count + j].len, - i2s_out->dma_repeat_desc[i * i2s_out->dma_rx_count + j].size, - i2s_out->dma_repeat_desc[i * i2s_out->dma_rx_count + j].buf, - i2s_out->dma_repeat_desc[i * i2s_out->dma_rx_count + j].next + &i2s_out->dma_repeat_desc[i * i2s_out->dma_out_count + j], + i2s_out->dma_repeat_desc[i * i2s_out->dma_out_count + j].owner, + i2s_out->dma_repeat_desc[i * i2s_out->dma_out_count + j].eof, + i2s_out->dma_repeat_desc[i * i2s_out->dma_out_count + j].len, + i2s_out->dma_repeat_desc[i * i2s_out->dma_out_count + j].size, + i2s_out->dma_repeat_desc[i * i2s_out->dma_out_count + j].buf, + i2s_out->dma_repeat_desc[i * i2s_out->dma_out_count + j].next ); } } @@ -438,3 +441,12 @@ int i2s_out_dma_flush(struct i2s_out *i2s_out) return 0; } + +void i2s_out_dma_free(struct i2s_out *i2s_out) +{ + free(i2s_out->dma_eof_buf); + free(i2s_out->dma_out_buf); + free(i2s_out->dma_out_desc); + free(i2s_out->dma_repeat_desc); + free(i2s_out->dma_eof_desc); +} diff --git a/components/i2s_out/i2s_out.c b/components/i2s_out/i2s_out.c index 4db3ee1e..a93f4d95 100644 --- a/components/i2s_out/i2s_out.c +++ b/components/i2s_out/i2s_out.c @@ -68,10 +68,7 @@ int i2s_out_new(struct i2s_out **i2s_outp, i2s_port_t port, size_t buffer_size, return 0; error: - free(i2s_out->dma_eof_buf); - free(i2s_out->dma_rx_buf); - free(i2s_out->dma_rx_desc); - free(i2s_out->dma_eof_desc); + i2s_out_dma_free(i2s_out); free(i2s_out); return err; diff --git a/components/i2s_out/i2s_out.h b/components/i2s_out/i2s_out.h index f7901750..ce9950f2 100644 --- a/components/i2s_out/i2s_out.h +++ b/components/i2s_out/i2s_out.h @@ -40,12 +40,12 @@ struct i2s_out { #endif /* dma */ - uint8_t *dma_rx_buf, *dma_eof_buf; - struct dma_desc *dma_rx_desc; + uint8_t *dma_out_buf, *dma_eof_buf; + struct dma_desc *dma_out_desc; struct dma_desc *dma_repeat_desc; struct dma_desc *dma_eof_desc; - unsigned dma_rx_count, dma_rx_repeat; + unsigned dma_out_count, dma_repeat_count; // pointer to software-owned dma_rx_desc used for write() struct dma_desc *dma_write_desc; @@ -63,6 +63,7 @@ void i2s_out_dma_repeat(struct i2s_out *i2s_out, unsigned count); int i2s_out_dma_pending(struct i2s_out *i2s_out); void i2s_out_dma_start(struct i2s_out *i2s_out); int i2s_out_dma_flush(struct i2s_out *i2s_out); +void i2s_out_dma_free(struct i2s_out *i2s_out); /* i2s.c */ int i2s_out_i2s_init(struct i2s_out *i2s_out); From 57ea1ccd6d7e7daf52e51aa32da46623cb04b3d8 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Tue, 28 Oct 2025 08:21:04 +0200 Subject: [PATCH 06/27] i2s_out: rename dma eof -> end --- components/i2s_out/esp32/dma.c | 66 +++++++++++++++++----------------- components/i2s_out/i2s_out.h | 4 +-- 2 files changed, 35 insertions(+), 35 deletions(-) diff --git a/components/i2s_out/esp32/dma.c b/components/i2s_out/esp32/dma.c index 3d9ed613..be372820 100644 --- a/components/i2s_out/esp32/dma.c +++ b/components/i2s_out/esp32/dma.c @@ -20,7 +20,7 @@ // shrink size to aligment #define TRUNC(size, align) ((size) & ~((align) - 1)) -#define DMA_EOF_BUF_SIZE (DMA_DESC_SIZE_MIN) +#define DMA_END_BUF_SIZE (DMA_DESC_SIZE_MIN) /* Allocate memory from appropriate heap region for DMA */ static inline void *dma_malloc(size_t size) @@ -66,7 +66,7 @@ void init_dma_desc(struct dma_desc *head, unsigned count, uint8_t *buf, size_t s } /* Prepare desc for DMA start */ -void init_dma_eof_desc(struct dma_desc *eof_desc, uint32_t value, unsigned count) +void init_dma_end_desc(struct dma_desc *eof_desc, uint32_t value, unsigned count) { uint32_t *ptr = (uint32_t *) eof_desc->buf; @@ -131,11 +131,11 @@ int i2s_out_dma_init(struct i2s_out *i2s_out, size_t size, size_t align, unsigne } else { LOG_DEBUG("dma_out_buf=%p[%u]", i2s_out->dma_out_buf, buf_size); } - if (!(i2s_out->dma_eof_buf = dma_malloc(DMA_EOF_BUF_SIZE))) { - LOG_ERROR("dma_malloc(dma_eof_buf)"); + if (!(i2s_out->dma_end_buf = dma_malloc(DMA_END_BUF_SIZE))) { + LOG_ERROR("dma_malloc(dma_end_buf)"); return -1; } else { - LOG_DEBUG("dma_eof_buf=%p[%u]", i2s_out->dma_eof_buf, DMA_EOF_BUF_SIZE); + LOG_DEBUG("dma_end_buf=%p[%u]", i2s_out->dma_end_buf, DMA_END_BUF_SIZE); } // allocate DMA descriptors @@ -147,8 +147,8 @@ int i2s_out_dma_init(struct i2s_out *i2s_out, size_t size, size_t align, unsigne LOG_ERROR("dma_calloc(dma_repeat_desc)"); return -1; } - if (!(i2s_out->dma_eof_desc = dma_calloc(1, sizeof(*i2s_out->dma_eof_desc)))) { - LOG_ERROR("dma_calloc(dma_eof_desc)"); + if (!(i2s_out->dma_end_desc = dma_calloc(1, sizeof(*i2s_out->dma_end_desc)))) { + LOG_ERROR("dma_calloc(dma_end_desc)"); return -1; } @@ -157,7 +157,7 @@ int i2s_out_dma_init(struct i2s_out *i2s_out, size_t size, size_t align, unsigne for (unsigned i = 0; i < repeat; i++) { init_dma_desc(i2s_out->dma_repeat_desc + i * desc_count, desc_count, i2s_out->dma_out_buf, buf_size, align, NULL); } - init_dma_desc(i2s_out->dma_eof_desc, 1, i2s_out->dma_eof_buf, DMA_EOF_BUF_SIZE, sizeof(uint32_t), NULL); + init_dma_desc(i2s_out->dma_end_desc, 1, i2s_out->dma_end_buf, DMA_END_BUF_SIZE, sizeof(uint32_t), NULL); i2s_out->dma_out_count = desc_count; i2s_out->dma_repeat_count = repeat; @@ -169,13 +169,13 @@ int i2s_out_dma_setup(struct i2s_out *i2s_out, const struct i2s_out_options *opt { LOG_DEBUG("..."); - if (options->eof_count * sizeof(options->eof_value) > DMA_EOF_BUF_SIZE) { - LOG_ERROR("eof_count=%u is too large for eof buf size=%u", options->eof_count, DMA_EOF_BUF_SIZE); + if (options->eof_count * sizeof(options->eof_value) > DMA_END_BUF_SIZE) { + LOG_ERROR("eof_count=%u is too large for end buf size=%u", options->eof_count, DMA_END_BUF_SIZE); return -1; } // init EOF buffer - init_dma_eof_desc(i2s_out->dma_eof_desc, options->eof_value, options->eof_count); + init_dma_end_desc(i2s_out->dma_end_desc, options->eof_value, options->eof_count); // init RX desc reinit_dma_desc(i2s_out->dma_out_desc, i2s_out->dma_out_count, NULL); @@ -218,14 +218,14 @@ int i2s_out_dma_setup(struct i2s_out *i2s_out, const struct i2s_out_options *opt i2s_out->dma_write_desc->next ); - LOG_DEBUG("dma_eof_desc=%p: owner=%d eof=%d len=%u size=%u -> buf=%p next=%p", - i2s_out->dma_eof_desc, - i2s_out->dma_eof_desc->owner, - i2s_out->dma_eof_desc->eof, - i2s_out->dma_eof_desc->len, - i2s_out->dma_eof_desc->size, - i2s_out->dma_eof_desc->buf, - i2s_out->dma_eof_desc->next + LOG_DEBUG("dma_end_desc=%p: owner=%d eof=%d len=%u size=%u -> buf=%p next=%p", + i2s_out->dma_end_desc, + i2s_out->dma_end_desc->owner, + i2s_out->dma_end_desc->eof, + i2s_out->dma_end_desc->len, + i2s_out->dma_end_desc->size, + i2s_out->dma_end_desc->buf, + i2s_out->dma_end_desc->next ); return 0; @@ -342,7 +342,7 @@ void i2s_out_dma_repeat(struct i2s_out *i2s_out, unsigned count) if (nextp) { // eof - *nextp = i2s_out->dma_eof_desc; + *nextp = i2s_out->dma_end_desc; } } @@ -369,11 +369,11 @@ void i2s_out_dma_start(struct i2s_out *i2s_out) } if (!i2s_out->dma_write_desc->next) { - i2s_out->dma_write_desc->next = i2s_out->dma_eof_desc; + i2s_out->dma_write_desc->next = i2s_out->dma_end_desc; } - i2s_out->dma_eof_desc->owner = 1; - i2s_out->dma_eof_desc->next = NULL; + i2s_out->dma_end_desc->owner = 1; + i2s_out->dma_end_desc->next = NULL; #undef DEBUG #define DEBUG 1 @@ -404,14 +404,14 @@ void i2s_out_dma_start(struct i2s_out *i2s_out) } } - LOG_DEBUG("dma_eof_desc=%p: owner=%d eof=%d len=%u size=%u -> buf=%p next=%p", - i2s_out->dma_eof_desc, - i2s_out->dma_eof_desc->owner, - i2s_out->dma_eof_desc->eof, - i2s_out->dma_eof_desc->len, - i2s_out->dma_eof_desc->size, - i2s_out->dma_eof_desc->buf, - i2s_out->dma_eof_desc->next + LOG_DEBUG("dma_end_desc=%p: owner=%d eof=%d len=%u size=%u -> buf=%p next=%p", + i2s_out->dma_end_desc, + i2s_out->dma_end_desc->owner, + i2s_out->dma_end_desc->eof, + i2s_out->dma_end_desc->len, + i2s_out->dma_end_desc->size, + i2s_out->dma_end_desc->buf, + i2s_out->dma_end_desc->next ); taskENTER_CRITICAL(&i2s_out->mux); @@ -444,9 +444,9 @@ int i2s_out_dma_flush(struct i2s_out *i2s_out) void i2s_out_dma_free(struct i2s_out *i2s_out) { - free(i2s_out->dma_eof_buf); + free(i2s_out->dma_end_buf); free(i2s_out->dma_out_buf); free(i2s_out->dma_out_desc); free(i2s_out->dma_repeat_desc); - free(i2s_out->dma_eof_desc); + free(i2s_out->dma_end_desc); } diff --git a/components/i2s_out/i2s_out.h b/components/i2s_out/i2s_out.h index ce9950f2..38973293 100644 --- a/components/i2s_out/i2s_out.h +++ b/components/i2s_out/i2s_out.h @@ -40,10 +40,10 @@ struct i2s_out { #endif /* dma */ - uint8_t *dma_out_buf, *dma_eof_buf; + uint8_t *dma_out_buf, *dma_end_buf; struct dma_desc *dma_out_desc; struct dma_desc *dma_repeat_desc; - struct dma_desc *dma_eof_desc; + struct dma_desc *dma_end_desc; unsigned dma_out_count, dma_repeat_count; From aa7bb0eb0a25a8216e120f84c4c2a5a72225bed8 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Tue, 28 Oct 2025 16:42:29 +0200 Subject: [PATCH 07/27] i2s_out: refactor i2s_out_dma_desc/next() --- components/i2s_out/esp32/dma.c | 71 +++++++++++++++++++++------------- components/i2s_out/i2s_out.h | 2 +- 2 files changed, 45 insertions(+), 28 deletions(-) diff --git a/components/i2s_out/esp32/dma.c b/components/i2s_out/esp32/dma.c index be372820..1c75d8d3 100644 --- a/components/i2s_out/esp32/dma.c +++ b/components/i2s_out/esp32/dma.c @@ -208,27 +208,40 @@ int i2s_out_dma_setup(struct i2s_out *i2s_out, const struct i2s_out_options *opt i2s_out->dma_start = false; i2s_out->dma_write_desc = i2s_out->dma_out_desc; - LOG_DEBUG("dma_write_desc=%p: owner=%d eof=%d len=%u size=%u -> buf=%p next=%p", - i2s_out->dma_write_desc, - i2s_out->dma_write_desc->owner, - i2s_out->dma_write_desc->eof, - i2s_out->dma_write_desc->len, - i2s_out->dma_write_desc->size, - i2s_out->dma_write_desc->buf, - i2s_out->dma_write_desc->next - ); + return 0; +} - LOG_DEBUG("dma_end_desc=%p: owner=%d eof=%d len=%u size=%u -> buf=%p next=%p", - i2s_out->dma_end_desc, - i2s_out->dma_end_desc->owner, - i2s_out->dma_end_desc->eof, - i2s_out->dma_end_desc->len, - i2s_out->dma_end_desc->size, - i2s_out->dma_end_desc->buf, - i2s_out->dma_end_desc->next - ); +/* + * Return pointer to current uncommitted dma_write_desc. + */ +struct dma_desc *i2s_out_dma_desc(struct i2s_out *i2s_out) +{ + if (i2s_out->dma_write_desc->owner) { + return NULL; + } - return 0; + return i2s_out->dma_write_desc; +} + +/* + * Commit current dma_write_desc and return next. + */ +struct dma_desc *i2s_out_dma_next(struct i2s_out *i2s_out) +{ + if (!i2s_out->dma_write_desc->owner) { + // prepare for DMA + i2s_out->dma_write_desc->owner = 1; + } + + if (!i2s_out->dma_write_desc->next) { + // no more descs left + return NULL; + } + + // start using next desc + i2s_out->dma_write_desc = i2s_out->dma_write_desc->next; + + return i2s_out->dma_write_desc; } /* @@ -238,10 +251,10 @@ int i2s_out_dma_setup(struct i2s_out *i2s_out, const struct i2s_out_options *opt */ size_t i2s_out_dma_buffer(struct i2s_out *i2s_out, void **ptr, unsigned count, size_t size) { - struct dma_desc *desc = i2s_out->dma_write_desc; + struct dma_desc *desc; // stop if last desc already committed - if (desc->owner) { + if (!(desc = i2s_out_dma_desc(i2s_out))) { LOG_WARN("owned desc=%p (owner=%u eof=%u buf=%p len=%u size=%u)", desc, desc->owner, desc->eof, desc->buf, desc->len, desc->size); // unable to find a usable DMA buffer, TX buffers full @@ -255,14 +268,18 @@ size_t i2s_out_dma_buffer(struct i2s_out *i2s_out, void **ptr, unsigned count, s LOG_DEBUG("commit desc=%p (owner=%u eof=%u buf=%p len=%u size=%u) < size=%u -> next=%p", desc, desc->owner, desc->eof, desc->buf, desc->len, desc->size, size, desc->next); // commit, try with the next desc, if available - desc->owner = 1; - - if (desc->next) { - desc = i2s_out->dma_write_desc = desc->next; - } + desc = i2s_out_dma_next(i2s_out); } - if (desc->len + size > desc->size) { + if (!desc) { + LOG_WARN("last desc=%p (owner=%u eof=%u buf=%p len=%u size=%u) < size=%u", desc, desc->owner, desc->eof, desc->buf, desc->len, desc->size, size); + + // unable to find a usable DMA buffer, TX buffers full + *ptr = NULL; + + return 0; + + } else if (desc->len + size > desc->size) { LOG_WARN("overflow desc=%p (owner=%u eof=%u buf=%p len=%u size=%u) < size=%u", desc, desc->owner, desc->eof, desc->buf, desc->len, desc->size, size); // unable to find a usable DMA buffer, TX buffers full diff --git a/components/i2s_out/i2s_out.h b/components/i2s_out/i2s_out.h index 38973293..7298bbbc 100644 --- a/components/i2s_out/i2s_out.h +++ b/components/i2s_out/i2s_out.h @@ -47,7 +47,7 @@ struct i2s_out { unsigned dma_out_count, dma_repeat_count; - // pointer to software-owned dma_rx_desc used for write() + // pointer to software-owned dma_out_desc used for write() struct dma_desc *dma_write_desc; bool dma_start; // set by i2s_out_dma_start From ee9c74a013298986d0071b69fad06129530fec51 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Tue, 28 Oct 2025 18:39:23 +0200 Subject: [PATCH 08/27] leds: optional i2s interface setup() for async operation --- components/i2s_out/include/i2s_out.h | 7 + components/leds/include/leds.h | 20 ++- components/leds/include/leds_stats.h | 1 + components/leds/interface.c | 98 ++++++++++++- components/leds/interface.h | 2 + components/leds/interfaces/i2s.h | 6 +- components/leds/interfaces/i2s/setup.c | 182 +++++++++++++++++++++++++ components/leds/interfaces/i2s/tx.c | 179 ++++-------------------- components/leds/leds.c | 29 +--- components/leds/stats.c | 1 + 10 files changed, 339 insertions(+), 186 deletions(-) create mode 100644 components/leds/interfaces/i2s/setup.c diff --git a/components/i2s_out/include/i2s_out.h b/components/i2s_out/include/i2s_out.h index b0cc0518..db4fac7f 100644 --- a/components/i2s_out/include/i2s_out.h +++ b/components/i2s_out/include/i2s_out.h @@ -226,6 +226,13 @@ int i2s_out_write_serial32(struct i2s_out *i2s_out, const uint32_t *data, size_t */ int i2s_out_repeat(struct i2s_out *i2s_out, unsigned count); +/** + * Start I2S output, do not wait for TX. + * + * Returns <0 on error, 0 on success. + */ +int i2s_out_start(struct i2s_out *i2s_out); + /** * Start I2S output, and wait for the complete TX buffer and EOF frame to be written. * diff --git a/components/leds/include/leds.h b/components/leds/include/leds.h index 079046d9..35f37b58 100644 --- a/components/leds/include/leds.h +++ b/components/leds/include/leds.h @@ -444,5 +444,23 @@ unsigned leds_count_total_power(struct leds *leds); */ bool leds_is_active(struct leds *leds); -/* Output frames on interface */ +/* + * Setup persistent LEDs interface. + * + * It is also possible to call leds_tx() in sync mode without setup() / close(). + * A persistently setup interface cannot be shared across multiple leds instances, but may perform better. + */ +int leds_interface_setup(struct leds *leds); + +/* + * Output frames on interface. + * + * With a persistent setup(), this does not wait for TX to complete, and leaves the leds interface open. + * Otherwise, this is equivalent to setup() -> tx -> close(), and waits for TX to complete. + */ int leds_tx(struct leds *leds); + +/* + * Close persistent LEDs interface. + */ +int leds_interface_close(struct leds *leds); diff --git a/components/leds/include/leds_stats.h b/components/leds/include/leds_stats.h index b3acff71..08f83746 100644 --- a/components/leds/include/leds_stats.h +++ b/components/leds/include/leds_stats.h @@ -9,6 +9,7 @@ struct leds_interface_i2s_stats { struct stats_timer open; struct stats_timer write; + struct stats_timer start; struct stats_timer flush; }; #endif diff --git a/components/leds/interface.c b/components/leds/interface.c index 9266a7d3..6f5f5a96 100644 --- a/components/leds/interface.c +++ b/components/leds/interface.c @@ -53,7 +53,7 @@ int leds_interface_init(union leds_interface_state *interface, const struct leds LOG_ERROR("unsupported interface=I2S for protocol=%d", options->protocol); return -1; - } else if ((err = leds_interface_i2s_init(&interface->i2s, &options->i2s, protocol_type->i2s_interface_mode, protocol_type->i2s_interface_func, leds_interface_i2s_stats(options->interface)))) { + } else if ((err = leds_interface_i2s_init(&interface->i2s, &options->i2s, protocol_type->i2s_interface_mode, protocol_type->i2s_interface_func, options->count, leds_interface_i2s_stats(options->interface)))) { LOG_ERROR("leds_interface_i2s_init"); return err; } @@ -68,3 +68,99 @@ int leds_interface_init(union leds_interface_state *interface, const struct leds return 0; } + +int leds_interface_setup(struct leds *leds) +{ + switch (leds->options.interface) { + case LEDS_INTERFACE_NONE: + return 0; + + #if CONFIG_LEDS_SPI_ENABLED + case LEDS_INTERFACE_SPI: + return 0; + #endif + + #if CONFIG_LEDS_UART_ENABLED + case LEDS_INTERFACE_UART: + return 0; + #endif + + #if CONFIG_LEDS_I2S_ENABLED + # if LEDS_I2S_INTERFACE_COUNT > 0 + case LEDS_INTERFACE_I2S0: + # endif + # if LEDS_I2S_INTERFACE_COUNT > 1 + case LEDS_INTERFACE_I2S1: + # endif + return leds_interface_i2s_setup(&leds->interface.i2s); + #endif + + default: + LOG_ERROR("unsupported interface=%#x", leds->options.interface); + return -1; + } +} + +int leds_interface_tx(struct leds *leds) +{ + switch (leds->options.interface) { + case LEDS_INTERFACE_NONE: + return 0; + + #if CONFIG_LEDS_SPI_ENABLED + case LEDS_INTERFACE_SPI: + return leds_interface_spi_tx(&leds->interface.spi, leds->pixels, leds->options.count, &leds->limit); + #endif + + #if CONFIG_LEDS_UART_ENABLED + case LEDS_INTERFACE_UART: + return leds_interface_uart_tx(&leds->interface.uart, leds->pixels, leds->options.count, &leds->limit); + #endif + + #if CONFIG_LEDS_I2S_ENABLED + # if LEDS_I2S_INTERFACE_COUNT > 0 + case LEDS_INTERFACE_I2S0: + # endif + # if LEDS_I2S_INTERFACE_COUNT > 1 + case LEDS_INTERFACE_I2S1: + # endif + return leds_interface_i2s_tx(&leds->interface.i2s, leds->pixels, leds->options.count, &leds->limit); + #endif + + default: + LOG_ERROR("unsupported interface=%#x", leds->options.interface); + return -1; + } +} + +int leds_interface_close(struct leds *leds) +{ + switch (leds->options.interface) { + case LEDS_INTERFACE_NONE: + return 0; + + #if CONFIG_LEDS_SPI_ENABLED + case LEDS_INTERFACE_SPI: + return 0; + #endif + + #if CONFIG_LEDS_UART_ENABLED + case LEDS_INTERFACE_UART: + return 0; + #endif + + #if CONFIG_LEDS_I2S_ENABLED + # if LEDS_I2S_INTERFACE_COUNT > 0 + case LEDS_INTERFACE_I2S0: + # endif + # if LEDS_I2S_INTERFACE_COUNT > 1 + case LEDS_INTERFACE_I2S1: + # endif + return leds_interface_i2s_close(&leds->interface.i2s); + #endif + + default: + LOG_ERROR("unsupported interface=%#x", leds->options.interface); + return -1; + } +} diff --git a/components/leds/interface.h b/components/leds/interface.h index cdfcb626..f063bf40 100644 --- a/components/leds/interface.h +++ b/components/leds/interface.h @@ -23,3 +23,5 @@ union leds_interface_state { struct leds_interface_uart uart; #endif }; + +int leds_interface_tx(struct leds *leds); diff --git a/components/leds/interfaces/i2s.h b/components/leds/interfaces/i2s.h index 7850e046..bbf3cc15 100644 --- a/components/leds/interfaces/i2s.h +++ b/components/leds/interfaces/i2s.h @@ -51,6 +51,8 @@ union leds_interface_i2s_func { #define LEDS_INTERFACE_I2S_FUNC(type, func) ((union leds_interface_i2s_func) { .type = func }) struct leds_interface_i2s { + bool setup; // persistent setup() + enum leds_interface_i2s_mode mode; union leds_interface_i2s_func func; union leds_interface_i2s_buf *buf; @@ -68,7 +70,9 @@ struct leds_interface_i2s { size_t leds_interface_i2s_buffer_size(enum leds_interface_i2s_mode mode, unsigned led_count, unsigned pin_count); size_t leds_interface_i2s_buffer_align(enum leds_interface_i2s_mode mode, unsigned pin_count); -int leds_interface_i2s_init(struct leds_interface_i2s *interface, const struct leds_interface_i2s_options *options, enum leds_interface_i2s_mode mode, union leds_interface_i2s_func func, struct leds_interface_i2s_stats *stats); +int leds_interface_i2s_init(struct leds_interface_i2s *interface, const struct leds_interface_i2s_options *options, enum leds_interface_i2s_mode mode, union leds_interface_i2s_func func, unsigned count, struct leds_interface_i2s_stats *stats); +int leds_interface_i2s_setup(struct leds_interface_i2s *interface); int leds_interface_i2s_tx(struct leds_interface_i2s *interface, const struct leds_color *pixels, unsigned count, const struct leds_limit *limit); +int leds_interface_i2s_close(struct leds_interface_i2s *interface); #endif diff --git a/components/leds/interfaces/i2s/setup.c b/components/leds/interfaces/i2s/setup.c new file mode 100644 index 00000000..f898f26c --- /dev/null +++ b/components/leds/interfaces/i2s/setup.c @@ -0,0 +1,182 @@ +#include "../i2s.h" +#include "../../leds.h" +#include "../../stats.h" +#include "../../gpio.h" + +#include + +int leds_interface_i2s_init(struct leds_interface_i2s *interface, const struct leds_interface_i2s_options *options, enum leds_interface_i2s_mode mode, union leds_interface_i2s_func func, unsigned count, struct leds_interface_i2s_stats *stats) +{ + interface->mode = mode; + interface->func = func; + +#if LEDS_I2S_PARALLEL_ENABLED + interface->parallel = options->parallel; +#else + interface->parallel = 0; +#endif + interface->repeat = options->repeat; + + if (!(interface->buf = calloc(1, leds_interface_i2s_buf_size(interface->mode, interface->parallel)))) { + LOG_ERROR("calloc"); + return -1; + } + + interface->i2s_out = options->i2s_out; + interface->i2s_out_options = (struct i2s_out_options) { + // shared IO pins + .pin_mutex = options->pin_mutex, + .pin_timeout = options->pin_timeout, + }; + + switch(mode) { + case LEDS_INTERFACE_I2S_MODE_32BIT_BCK: + #if LEDS_I2S_PARALLEL_ENABLED + if (options->parallel) { + interface->i2s_out_options.mode = I2S_OUT_MODE_8BIT_PARALLEL; + } else { + interface->i2s_out_options.mode = I2S_OUT_MODE_32BIT_SERIAL; + } + #else + interface->i2s_out_options.mode = I2S_OUT_MODE_32BIT_SERIAL; + #endif + + break; + + case LEDS_INTERFACE_I2S_MODE_24BIT_1U200_4X4_80UL: + case LEDS_INTERFACE_I2S_MODE_24BIT_1U250_4X4_80UL: + case LEDS_INTERFACE_I2S_MODE_32BIT_1U250_4X4_80UL: + #if LEDS_I2S_PARALLEL_ENABLED + if (options->parallel) { + interface->i2s_out_options.mode = I2S_OUT_MODE_8BIT_PARALLEL; + } else { + // using 4x4bit -> 16-bit samples + interface->i2s_out_options.mode = I2S_OUT_MODE_16BIT_SERIAL; + } + #else + interface->i2s_out_options.mode = I2S_OUT_MODE_16BIT_SERIAL; + #endif + + break; + + default: + LOG_FATAL("unknown mode=%d", mode); + } + + switch(mode) { + case LEDS_INTERFACE_I2S_MODE_32BIT_BCK: + interface->i2s_out_options.clock = i2s_out_clock(options->clock_rate); + + break; + + case LEDS_INTERFACE_I2S_MODE_24BIT_1U200_4X4_80UL: + // 3.333MHz bit clock => 0.300us per I2S bit + // four I2S bits per 1.20us protocol bit + interface->i2s_out_options.clock = I2S_OUT_CLOCK_3M333; + + break; + + case LEDS_INTERFACE_I2S_MODE_24BIT_1U250_4X4_80UL: + case LEDS_INTERFACE_I2S_MODE_32BIT_1U250_4X4_80UL: + // 3.2MHz bit clock => 0.3125us per I2S bit + // four I2S bits per 1.25us protocol bit + interface->i2s_out_options.clock = I2S_OUT_CLOCK_3M2; + + break; + + default: + LOG_FATAL("unknown mode=%d", mode); + } + + switch (mode) { + case LEDS_INTERFACE_I2S_MODE_32BIT_BCK: + // XXX: required to workaround I2S start glitch looping previous data bits + interface->i2s_out_options.eof_value = 0x00000000; + + // one clock cycle per pixel, min 32 cycles + interface->i2s_out_options.eof_count = (1 + count / 32); + + break; + + case LEDS_INTERFACE_I2S_MODE_24BIT_1U200_4X4_80UL: + case LEDS_INTERFACE_I2S_MODE_24BIT_1U250_4X4_80UL: + case LEDS_INTERFACE_I2S_MODE_32BIT_1U250_4X4_80UL: + // 1.25us per 4-bit = 2.5us per byte * four bytes per I2S sample = 10us per 32-bit I2S sample + interface->i2s_out_options.eof_value = 0x00000000; + + // hold low for 8 * 10us + interface->i2s_out_options.eof_count = 8; + + break; + + default: + LOG_FATAL("unknown mode=%d", interface->mode); + } + + +#if LEDS_I2S_GPIO_PINS_ENABLED + for (int i = 0; i < LEDS_I2S_GPIO_PINS_SIZE; i++) { + interface->i2s_out_options.bck_gpios[i] = (i < options->gpio_pins_count) ? options->clock_pins[i] : GPIO_NUM_NC; + interface->i2s_out_options.data_gpios[i] = (i < options->gpio_pins_count) ? options->data_pins[i] : GPIO_NUM_NC; + interface->i2s_out_options.inv_data_gpios[i] = (i < options->gpio_pins_count) ? options->inv_data_pins[i] : GPIO_NUM_NC; + } +#endif + +#if LEDS_I2S_PARALLEL_ENABLED + // allow multiple copies of parallel data bits on different GPIOs + interface->i2s_out_options.parallel_data_bits = options->parallel; + + if (options->parallel) { + // I2S LCD mode requires the BCK/WS signal to be inverted when routed through the GPIO matrix + // invert BCK to idle high, transition on falling edge, and sample data on rising edge + interface->i2s_out_options.bck_inv = true; + } else { + // BCK is idle low, transition on falling edge, and sample data on rising edge + interface->i2s_out_options.bck_inv = false; + } +#endif + + interface->gpio = options->gpio; + interface->stats = stats; + + return 0; +} + +int leds_interface_i2s_setup(struct leds_interface_i2s *interface) +{ + int err = 0; + + WITH_STATS_TIMER(&interface->stats->open) { + if ((err = i2s_out_open(interface->i2s_out, &interface->i2s_out_options))) { + LOG_ERROR("i2s_out_open"); + return err; + } + } + + interface->setup = true; + +#if CONFIG_LEDS_GPIO_ENABLED + leds_gpio_setup(&interface->gpio); +#endif + + return 0; +} + +int leds_interface_i2s_close(struct leds_interface_i2s *interface) +{ + int err = 0; + + interface->setup = false; + +#if CONFIG_LEDS_GPIO_ENABLED + leds_gpio_close(&interface->gpio); +#endif + + if ((err = i2s_out_close(interface->i2s_out))) { + LOG_ERROR("i2s_out_close"); + return err; + } + + return err; + +} diff --git a/components/leds/interfaces/i2s/tx.c b/components/leds/interfaces/i2s/tx.c index 95e44709..338506d0 100644 --- a/components/leds/interfaces/i2s/tx.c +++ b/components/leds/interfaces/i2s/tx.c @@ -1,7 +1,6 @@ #include "../i2s.h" #include "../../leds.h" #include "../../stats.h" -#include "../../gpio.h" #include @@ -177,157 +176,19 @@ static int leds_interface_i2s_tx_write(struct leds_interface_i2s *interface, con } } -int leds_interface_i2s_init(struct leds_interface_i2s *interface, const struct leds_interface_i2s_options *options, enum leds_interface_i2s_mode mode, union leds_interface_i2s_func func, struct leds_interface_i2s_stats *stats) -{ - interface->mode = mode; - interface->func = func; - -#if LEDS_I2S_PARALLEL_ENABLED - interface->parallel = options->parallel; -#else - interface->parallel = 0; -#endif - interface->repeat = options->repeat; - - if (!(interface->buf = calloc(1, leds_interface_i2s_buf_size(interface->mode, interface->parallel)))) { - LOG_ERROR("calloc"); - return -1; - } - - interface->i2s_out = options->i2s_out; - interface->i2s_out_options = (struct i2s_out_options) { - // shared IO pins - .pin_mutex = options->pin_mutex, - .pin_timeout = options->pin_timeout, - }; - - switch(mode) { - case LEDS_INTERFACE_I2S_MODE_32BIT_BCK: - #if LEDS_I2S_PARALLEL_ENABLED - if (options->parallel) { - interface->i2s_out_options.mode = I2S_OUT_MODE_8BIT_PARALLEL; - } else { - interface->i2s_out_options.mode = I2S_OUT_MODE_32BIT_SERIAL; - } - #else - interface->i2s_out_options.mode = I2S_OUT_MODE_32BIT_SERIAL; - #endif - - break; - - case LEDS_INTERFACE_I2S_MODE_24BIT_1U200_4X4_80UL: - case LEDS_INTERFACE_I2S_MODE_24BIT_1U250_4X4_80UL: - case LEDS_INTERFACE_I2S_MODE_32BIT_1U250_4X4_80UL: - #if LEDS_I2S_PARALLEL_ENABLED - if (options->parallel) { - interface->i2s_out_options.mode = I2S_OUT_MODE_8BIT_PARALLEL; - } else { - // using 4x4bit -> 16-bit samples - interface->i2s_out_options.mode = I2S_OUT_MODE_16BIT_SERIAL; - } - #else - interface->i2s_out_options.mode = I2S_OUT_MODE_16BIT_SERIAL; - #endif - - break; - - default: - LOG_FATAL("unknown mode=%d", mode); - } - - switch(mode) { - case LEDS_INTERFACE_I2S_MODE_32BIT_BCK: - interface->i2s_out_options.clock = i2s_out_clock(options->clock_rate); - - break; - - case LEDS_INTERFACE_I2S_MODE_24BIT_1U200_4X4_80UL: - // 3.333MHz bit clock => 0.300us per I2S bit - // four I2S bits per 1.20us protocol bit - interface->i2s_out_options.clock = I2S_OUT_CLOCK_3M333; - - break; - - case LEDS_INTERFACE_I2S_MODE_24BIT_1U250_4X4_80UL: - case LEDS_INTERFACE_I2S_MODE_32BIT_1U250_4X4_80UL: - // 3.2MHz bit clock => 0.3125us per I2S bit - // four I2S bits per 1.25us protocol bit - interface->i2s_out_options.clock = I2S_OUT_CLOCK_3M2; - - break; - - default: - LOG_FATAL("unknown mode=%d", mode); - } - -#if LEDS_I2S_GPIO_PINS_ENABLED - for (int i = 0; i < LEDS_I2S_GPIO_PINS_SIZE; i++) { - interface->i2s_out_options.bck_gpios[i] = (i < options->gpio_pins_count) ? options->clock_pins[i] : GPIO_NUM_NC; - interface->i2s_out_options.data_gpios[i] = (i < options->gpio_pins_count) ? options->data_pins[i] : GPIO_NUM_NC; - interface->i2s_out_options.inv_data_gpios[i] = (i < options->gpio_pins_count) ? options->inv_data_pins[i] : GPIO_NUM_NC; - } -#endif - -#if LEDS_I2S_PARALLEL_ENABLED - // allow multiple copies of parallel data bits on different GPIOs - interface->i2s_out_options.parallel_data_bits = options->parallel; - - if (options->parallel) { - // I2S LCD mode requires the BCK/WS signal to be inverted when routed through the GPIO matrix - // invert BCK to idle high, transition on falling edge, and sample data on rising edge - interface->i2s_out_options.bck_inv = true; - } else { - // BCK is idle low, transition on falling edge, and sample data on rising edge - interface->i2s_out_options.bck_inv = false; - } -#endif - - interface->gpio = options->gpio; - interface->stats = stats; - - return 0; -} int leds_interface_i2s_tx(struct leds_interface_i2s *interface, const struct leds_color *pixels, unsigned count, const struct leds_limit *limit) { - int err; - - switch (interface->mode) { - case LEDS_INTERFACE_I2S_MODE_32BIT_BCK: - // XXX: required to workaround I2S start glitch looping previous data bits - interface->i2s_out_options.eof_value = 0x00000000; + bool setup = !interface->setup; // sync setup() -> write -> flush -> close()? + int err = 0; - // one clock cycle per pixel, min 32 cycles - interface->i2s_out_options.eof_count = (1 + count / 32); - - break; - - case LEDS_INTERFACE_I2S_MODE_24BIT_1U200_4X4_80UL: - case LEDS_INTERFACE_I2S_MODE_24BIT_1U250_4X4_80UL: - case LEDS_INTERFACE_I2S_MODE_32BIT_1U250_4X4_80UL: - // 1.25us per 4-bit = 2.5us per byte * four bytes per I2S sample = 10us per 32-bit I2S sample - interface->i2s_out_options.eof_value = 0x00000000; - - // hold low for 8 * 10us - interface->i2s_out_options.eof_count = 8; - - break; - - default: - LOG_FATAL("unknown mode=%d", interface->mode); - } - - WITH_STATS_TIMER(&interface->stats->open) { - if ((err = i2s_out_open(interface->i2s_out, &interface->i2s_out_options))) { - LOG_ERROR("i2s_out_open"); + if (setup) { + if ((err = leds_interface_i2s_setup(interface))) { + LOG_ERROR("leds_interface_i2s_setup"); return err; } } -#if CONFIG_LEDS_GPIO_ENABLED - leds_gpio_setup(&interface->gpio); -#endif - WITH_STATS_TIMER(&interface->stats->write) { if ((err = leds_interface_i2s_tx_write(interface, pixels, count, limit))) { goto error; @@ -341,21 +202,29 @@ int leds_interface_i2s_tx(struct leds_interface_i2s *interface, const struct led } } - WITH_STATS_TIMER(&interface->stats->flush) { - if ((err = i2s_out_flush(interface->i2s_out))) { - LOG_ERROR("i2s_out_flush"); - goto error; + if (setup) { + // sync, wait for done before close + WITH_STATS_TIMER(&interface->stats->flush) { + if ((err = i2s_out_flush(interface->i2s_out))) { + LOG_ERROR("i2s_out_flush"); + goto error; + } + } + } else { + // async, do not wait for done + WITH_STATS_TIMER(&interface->stats->start) { + if ((err = i2s_out_start(interface->i2s_out))) { + LOG_ERROR("i2s_out_start"); + goto error; + } } } error: -#if CONFIG_LEDS_GPIO_ENABLED - leds_gpio_close(&interface->gpio); -#endif - - if ((err = i2s_out_close(interface->i2s_out))) { - LOG_ERROR("i2s_out_close"); - return err; + if (setup) { + if (leds_interface_i2s_close(interface)) { + LOG_WARN("leds_interface_i2s_close"); + } } return err; diff --git a/components/leds/leds.c b/components/leds/leds.c index 353245a0..9e629c6d 100644 --- a/components/leds/leds.c +++ b/components/leds/leds.c @@ -163,32 +163,5 @@ int leds_tx(struct leds *leds) { leds_limit_update(leds); - switch (leds->options.interface) { - case LEDS_INTERFACE_NONE: - return 0; - - #if CONFIG_LEDS_SPI_ENABLED - case LEDS_INTERFACE_SPI: - return leds_interface_spi_tx(&leds->interface.spi, leds->pixels, leds->options.count, &leds->limit); - #endif - - #if CONFIG_LEDS_UART_ENABLED - case LEDS_INTERFACE_UART: - return leds_interface_uart_tx(&leds->interface.uart, leds->pixels, leds->options.count, &leds->limit); - #endif - - #if CONFIG_LEDS_I2S_ENABLED - # if LEDS_I2S_INTERFACE_COUNT > 0 - case LEDS_INTERFACE_I2S0: - # endif - # if LEDS_I2S_INTERFACE_COUNT > 1 - case LEDS_INTERFACE_I2S1: - # endif - return leds_interface_i2s_tx(&leds->interface.i2s, leds->pixels, leds->options.count, &leds->limit); - #endif - - default: - LOG_ERROR("unsupported interface=%#x", leds->options.interface); - return -1; - } + return leds_interface_tx(leds); } diff --git a/components/leds/stats.c b/components/leds/stats.c index 7bfa424a..1d4d9b90 100644 --- a/components/leds/stats.c +++ b/components/leds/stats.c @@ -24,6 +24,7 @@ void leds_reset_interface_stats() #if LEDS_I2S_INTERFACE_COUNT > 1 stats_timer_init(&leds_interface_stats.i2s1.open); stats_timer_init(&leds_interface_stats.i2s1.write); + stats_timer_init(&leds_interface_stats.i2s1.start); stats_timer_init(&leds_interface_stats.i2s1.flush); #endif } From 4a6a9ac4943a3e7f81606ed591d0fbfa67e872c1 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Tue, 28 Oct 2025 19:36:51 +0200 Subject: [PATCH 09/27] main: leds setup_interface --- main/leds.c | 23 +++++++++++++++++++++++ main/leds_cmd.c | 2 ++ main/leds_config.h | 3 +++ main/leds_configtab.i | 4 ++++ main/leds_state.h | 5 +++++ main/leds_task.c | 8 ++++++++ 6 files changed, 45 insertions(+) diff --git a/main/leds.c b/main/leds.c index 015707af..d2fead68 100644 --- a/main/leds.c +++ b/main/leds.c @@ -149,6 +149,29 @@ int start_leds() return 0; } +int setup_leds(struct leds_state *state) +{ + int err; + + if (!state->leds) { + LOG_ERROR("leds%d: not initialized", state->index + 1); + return -1; + } + + if (!state->config->interface_setup) { + return 0; + } + + LOG_INFO("Setup LEDS interface in async mode"); + + if ((err = leds_interface_setup(state->leds))) { + LOG_ERROR("leds_interface_setup"); + return err; + } + + return 0; +} + int check_leds_interface(struct leds_state *state) { if (!state->leds) { diff --git a/main/leds_cmd.c b/main/leds_cmd.c index da36a16c..8253d7ea 100644 --- a/main/leds_cmd.c +++ b/main/leds_cmd.c @@ -368,12 +368,14 @@ int leds_cmd_stats(int argc, char **argv, void *ctx) #if LEDS_I2S_INTERFACE_COUNT > 0 print_stats_timer("i2s0", "open", &stats.i2s0.open); print_stats_timer("i2s0", "write", &stats.i2s0.write); + print_stats_timer("i2s0", "start", &stats.i2s0.start); print_stats_timer("i2s0", "flush", &stats.i2s0.flush); printf("\n"); #endif #if LEDS_I2S_INTERFACE_COUNT > 1 print_stats_timer("i2s1", "open", &stats.i2s1.open); print_stats_timer("i2s1", "write", &stats.i2s1.write); + print_stats_timer("i2s1", "start", &stats.i2s1.start); print_stats_timer("i2s1", "flush", &stats.i2s1.flush); printf("\n"); #endif diff --git a/main/leds_config.h b/main/leds_config.h index 945ea423..1f8adfd3 100644 --- a/main/leds_config.h +++ b/main/leds_config.h @@ -110,7 +110,10 @@ struct leds_config { bool enabled; int interface; + bool interface_setup; + int protocol; + uint16_t count; uint16_t limit_total, limit_group; uint16_t limit_groups; diff --git a/main/leds_configtab.i b/main/leds_configtab.i index 8235cc4d..7a5adfc5 100644 --- a/main/leds_configtab.i +++ b/main/leds_configtab.i @@ -13,6 +13,10 @@ const struct configtab LEDS_CONFIGTAB[] = { ), .enum_type = { .value = &LEDS_CONFIG.interface, .values = leds_interface_enum }, }, + { CONFIG_TYPE_BOOL, "interface_setup", + .description = "Setup dedicated interface, operate in async mode. I2S only.", + .bool_type = { .value = &LEDS_CONFIG.interface_setup, .default_value = false }, + }, { CONFIG_TYPE_ENUM, "protocol", .enum_type = { .value = &LEDS_CONFIG.protocol, .values = leds_protocol_enum }, }, diff --git a/main/leds_state.h b/main/leds_state.h index 67435d2b..e8c91706 100644 --- a/main/leds_state.h +++ b/main/leds_state.h @@ -51,6 +51,11 @@ extern struct leds_state leds_states[LEDS_COUNT]; # endif #endif +/* + * Optional persistent leds interface setup. + */ +int setup_leds(struct leds_state *state); + /* * Update LEDs output with given USER_ACTIVITY_LEDS_* source. */ diff --git a/main/leds_task.c b/main/leds_task.c index 5b2eb2a7..1441fe8d 100644 --- a/main/leds_task.c +++ b/main/leds_task.c @@ -102,6 +102,11 @@ static void leds_main(void *ctx) struct leds_state *state = ctx; struct leds_stats *stats = &leds_stats[state->index]; + if (setup_leds(state)) { + LOG_ERROR("setup_leds"); + goto error; + } + for(struct stats_timer_sample loop_sample;; stats_timer_stop(&stats->loop, &loop_sample)) { EventBits_t event_bits = leds_task_wait(state); enum user_activity update_activity = 0; @@ -165,6 +170,9 @@ static void leds_main(void *ctx) } } } + +error: + return; } int init_leds_task(struct leds_state *state, const struct leds_config *config) From 4dd05798e478c632fd2bf1daa25291ff0cb8f03d Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Tue, 28 Oct 2025 19:39:05 +0200 Subject: [PATCH 10/27] fixup! i2s_out: rename dma rx -> out --- components/i2s_out/esp32/dma.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/components/i2s_out/esp32/dma.c b/components/i2s_out/esp32/dma.c index 1c75d8d3..bb0df6bd 100644 --- a/components/i2s_out/esp32/dma.c +++ b/components/i2s_out/esp32/dma.c @@ -392,9 +392,6 @@ void i2s_out_dma_start(struct i2s_out *i2s_out) i2s_out->dma_end_desc->owner = 1; i2s_out->dma_end_desc->next = NULL; -#undef DEBUG -#define DEBUG 1 - for (unsigned i = 0; i < i2s_out->dma_out_count; i++) { LOG_DEBUG("dma_out_desc[%u]=%p: owner=%d eof=%d len=%u size=%u buf=%p next=%p", i, &i2s_out->dma_out_desc[i], From 39f80fa3618f1c97893ebfd32a5ba14068529020 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Tue, 28 Oct 2025 20:15:55 +0200 Subject: [PATCH 11/27] i2s_out: implement async start() mode --- components/i2s_out/esp32/dma.c | 221 ++++++++++++++++++--------- components/i2s_out/esp32/dma.h | 7 + components/i2s_out/esp32/i2s.c | 8 +- components/i2s_out/esp32/intr.c | 61 +++++++- components/i2s_out/i2s_out.c | 79 ++++++++-- components/i2s_out/i2s_out.h | 16 +- components/i2s_out/include/i2s_out.h | 6 +- 7 files changed, 307 insertions(+), 91 deletions(-) diff --git a/components/i2s_out/esp32/dma.c b/components/i2s_out/esp32/dma.c index bb0df6bd..f0169999 100644 --- a/components/i2s_out/esp32/dma.c +++ b/components/i2s_out/esp32/dma.c @@ -65,41 +65,6 @@ void init_dma_desc(struct dma_desc *head, unsigned count, uint8_t *buf, size_t s } } -/* Prepare desc for DMA start */ -void init_dma_end_desc(struct dma_desc *eof_desc, uint32_t value, unsigned count) -{ - uint32_t *ptr = (uint32_t *) eof_desc->buf; - - for (unsigned i = 0; i < count; i++) { - ptr[i] = value; - } - - eof_desc->len = count * sizeof(value); -} - -void reinit_dma_desc(struct dma_desc *head, unsigned count, struct dma_desc *next) -{ - struct dma_desc **nextp = NULL; - - for (unsigned i = 0; i < count; i++) { - struct dma_desc *desc = &head[i]; - - if (nextp) { - *nextp = desc; - } - - desc->len = 0; - desc->owner = 0; - - nextp = &desc->next; - } - - if (nextp) { - // loop - *nextp = next; - } -} - int i2s_out_dma_init(struct i2s_out *i2s_out, size_t size, size_t align, unsigned repeat) { size_t buf_size = 0; @@ -165,6 +130,42 @@ int i2s_out_dma_init(struct i2s_out *i2s_out, size_t size, size_t align, unsigne return 0; } +/* Prepare end desc + buffer */ +void init_dma_end(struct dma_desc *eof_desc, uint32_t value, unsigned count) +{ + uint32_t *ptr = (uint32_t *) eof_desc->buf; + + for (unsigned i = 0; i < count; i++) { + ptr[i] = value; + } + + eof_desc->len = count * sizeof(value); + eof_desc->next = NULL; +} + +void reinit_dma_desc(struct dma_desc *head, unsigned count, struct dma_desc *next) +{ + struct dma_desc **nextp = NULL; + + for (unsigned i = 0; i < count; i++) { + struct dma_desc *desc = &head[i]; + + if (nextp) { + *nextp = desc; + } + + desc->len = 0; + desc->owner = 0; + + nextp = &desc->next; + } + + if (nextp) { + // loop + *nextp = next; + } +} + int i2s_out_dma_setup(struct i2s_out *i2s_out, const struct i2s_out_options *options) { LOG_DEBUG("..."); @@ -173,12 +174,9 @@ int i2s_out_dma_setup(struct i2s_out *i2s_out, const struct i2s_out_options *opt LOG_ERROR("eof_count=%u is too large for end buf size=%u", options->eof_count, DMA_END_BUF_SIZE); return -1; } - - // init EOF buffer - init_dma_end_desc(i2s_out->dma_end_desc, options->eof_value, options->eof_count); - - // init RX desc - reinit_dma_desc(i2s_out->dma_out_desc, i2s_out->dma_out_count, NULL); + // reinit out desc + init_dma_end(i2s_out->dma_end_desc, options->eof_value, options->eof_count); + reinit_dma_desc(i2s_out->dma_out_desc, i2s_out->dma_out_count, i2s_out->dma_end_desc); taskENTER_CRITICAL(&i2s_out->mux); @@ -193,20 +191,15 @@ int i2s_out_dma_setup(struct i2s_out *i2s_out, const struct i2s_out_options *opt i2s_ll_rx_reset_dma(i2s_out->dev); i2s_ll_tx_reset_dma(i2s_out->dev); - i2s_ll_dma_enable_eof_on_fifo_empty(i2s_out->dev, true); - i2s_ll_dma_enable_owner_check(i2s_out->dev, true); - i2s_ll_dma_enable_auto_write_back(i2s_out->dev, true); - - i2s_ll_set_out_link_addr(i2s_out->dev, (uint32_t) i2s_out->dma_out_desc); - taskEXIT_CRITICAL(&i2s_out->mux); // reset eof state - xEventGroupClearBits(i2s_out->event_group, I2S_OUT_EVENT_GROUP_BIT_DMA_EOF); + xEventGroupClearBits(i2s_out->event_group, I2S_OUT_EVENT_GROUP_BIT_DMA_EOF | I2S_OUT_EVENT_GROUP_BIT_DMA_TOTAL_EOF); // reset write state i2s_out->dma_start = false; i2s_out->dma_write_desc = i2s_out->dma_out_desc; + i2s_out->dma_eof_desc = NULL; return 0; } @@ -214,10 +207,19 @@ int i2s_out_dma_setup(struct i2s_out *i2s_out, const struct i2s_out_options *opt /* * Return pointer to current uncommitted dma_write_desc. */ -struct dma_desc *i2s_out_dma_desc(struct i2s_out *i2s_out) +struct dma_desc *i2s_out_dma_wait(struct i2s_out *i2s_out) { - if (i2s_out->dma_write_desc->owner) { - return NULL; + if (i2s_out->dma_start) { + while (!i2s_out->dma_eof_desc || i2s_out->dma_write_desc > i2s_out->dma_eof_desc) { + LOG_WARN("wait for dma_write_desc=%p > dma_eof_desc=%p", i2s_out->dma_write_desc, i2s_out->dma_eof_desc); + + xEventGroupWaitBits(i2s_out->event_group, I2S_OUT_EVENT_GROUP_BIT_DMA_EOF, true, true, portMAX_DELAY); + } + } else { + if (i2s_out->dma_write_desc->owner) { + // last desc was already committed + return NULL; + } } return i2s_out->dma_write_desc; @@ -233,15 +235,15 @@ struct dma_desc *i2s_out_dma_next(struct i2s_out *i2s_out) i2s_out->dma_write_desc->owner = 1; } - if (!i2s_out->dma_write_desc->next) { - // no more descs left + if (i2s_out->dma_write_desc < i2s_out->dma_out_desc + i2s_out->dma_out_count) { + // start using next desc + i2s_out->dma_write_desc++; + } else { + // no more descs available return NULL; } - // start using next desc - i2s_out->dma_write_desc = i2s_out->dma_write_desc->next; - - return i2s_out->dma_write_desc; + return i2s_out_dma_wait(i2s_out); } /* @@ -254,7 +256,7 @@ size_t i2s_out_dma_buffer(struct i2s_out *i2s_out, void **ptr, unsigned count, s struct dma_desc *desc; // stop if last desc already committed - if (!(desc = i2s_out_dma_desc(i2s_out))) { + if (!(desc = i2s_out_dma_wait(i2s_out))) { LOG_WARN("owned desc=%p (owner=%u eof=%u buf=%p len=%u size=%u)", desc, desc->owner, desc->eof, desc->buf, desc->len, desc->size); // unable to find a usable DMA buffer, TX buffers full @@ -330,12 +332,30 @@ int i2s_out_dma_write(struct i2s_out *i2s_out, const void *data, size_t size) return len; } -void i2s_out_dma_repeat(struct i2s_out *i2s_out, unsigned count) +int i2s_out_dma_repeat(struct i2s_out *i2s_out, unsigned count) { struct dma_desc **nextp = &i2s_out->dma_write_desc->next; + if (i2s_out->dma_start) { + LOG_ERROR("dma_start=%u", i2s_out->dma_start); + return -1; + } // commit - i2s_out->dma_write_desc->owner = 1; + if (!i2s_out->dma_write_desc->owner) { + LOG_DEBUG("commit desc=%p: owner=%u eof=%u buf=%p len=%u size=%u next=%p", + i2s_out->dma_write_desc, + i2s_out->dma_write_desc->owner, + i2s_out->dma_write_desc->eof, + i2s_out->dma_write_desc->buf, + i2s_out->dma_write_desc->len, + i2s_out->dma_write_desc->size, + i2s_out->dma_write_desc->next + ); + + i2s_out->dma_write_desc->owner = 1; + } + + LOG_DEBUG("count=%u", count); for (unsigned i = 0; i < count; i++) { for (unsigned j = 0; j < i2s_out->dma_out_count; j++) { @@ -361,27 +381,49 @@ void i2s_out_dma_repeat(struct i2s_out *i2s_out, unsigned count) // eof *nextp = i2s_out->dma_end_desc; } + + return 0; } int i2s_out_dma_pending(struct i2s_out *i2s_out) { - if (i2s_out->dma_start) { - // start() already haṕpened - return 0; + if (i2s_out->dma_write_desc > i2s_out->dma_out_desc || i2s_out->dma_write_desc->len > 0) { + // write() happened + return 1; } - if (i2s_out->dma_write_desc != i2s_out->dma_out_desc || i2s_out->dma_write_desc->len > 0) { - // write() happened + return 0; +} + +int i2s_out_dma_running(struct i2s_out *i2s_out) +{ + if (i2s_out->dma_start) { + // start() has been called, flush() has not return 1; } return 0; } -void i2s_out_dma_start(struct i2s_out *i2s_out) +int i2s_out_dma_start(struct i2s_out *i2s_out) { + if (i2s_out->dma_start) { + LOG_ERROR("dma_start=%u", i2s_out->dma_start); + return -1; + } + // commit if not repeat() if (!i2s_out->dma_write_desc->owner) { + LOG_DEBUG("commit desc=%p: owner=%u eof=%u buf=%p len=%u size=%u next=%p", + i2s_out->dma_write_desc, + i2s_out->dma_write_desc->owner, + i2s_out->dma_write_desc->eof, + i2s_out->dma_write_desc->buf, + i2s_out->dma_write_desc->len, + i2s_out->dma_write_desc->size, + i2s_out->dma_write_desc->next + ); + i2s_out->dma_write_desc->owner = 1; } @@ -428,12 +470,23 @@ void i2s_out_dma_start(struct i2s_out *i2s_out) i2s_out->dma_end_desc->next ); + // reset eof state + xEventGroupClearBits(i2s_out->event_group, I2S_OUT_EVENT_GROUP_BIT_DMA_EOF | I2S_OUT_EVENT_GROUP_BIT_DMA_TOTAL_EOF); + + i2s_out->dma_eof_desc = NULL; + taskENTER_CRITICAL(&i2s_out->mux); i2s_ll_tx_reset_dma(i2s_out->dev); i2s_ll_tx_reset_fifo(i2s_out->dev); i2s_ll_tx_reset(i2s_out->dev); + i2s_ll_set_out_link_addr(i2s_out->dev, (uint32_t) i2s_out->dma_out_desc); + + i2s_ll_dma_enable_eof_on_fifo_empty(i2s_out->dev, true); + i2s_ll_dma_enable_owner_check(i2s_out->dev, true); + i2s_ll_dma_enable_auto_write_back(i2s_out->dev, false); + i2s_intr_clear(i2s_out->dev, I2S_OUT_TOTAL_EOF_INT_CLR | I2S_OUT_DSCR_ERR_INT_CLR | I2S_OUT_EOF_INT_CLR); i2s_intr_enable(i2s_out->dev, I2S_OUT_TOTAL_EOF_INT_ENA | I2S_OUT_DSCR_ERR_INT_ENA | I2S_OUT_EOF_INT_ENA); @@ -442,20 +495,52 @@ void i2s_out_dma_start(struct i2s_out *i2s_out) taskEXIT_CRITICAL(&i2s_out->mux); + // reset state for next write() i2s_out->dma_start = true; + i2s_out->dma_write_desc = i2s_out->dma_out_desc; + + return 0; } int i2s_out_dma_flush(struct i2s_out *i2s_out) { - LOG_DEBUG("wait event_group bits=%08x", I2S_OUT_EVENT_GROUP_BIT_DMA_EOF); + LOG_DEBUG("wait event_group bits=%08x...", I2S_OUT_EVENT_GROUP_BIT_DMA_TOTAL_EOF); - xEventGroupWaitBits(i2s_out->event_group, I2S_OUT_EVENT_GROUP_BIT_DMA_EOF, false, false, portMAX_DELAY); + xEventGroupWaitBits(i2s_out->event_group, I2S_OUT_EVENT_GROUP_BIT_DMA_TOTAL_EOF, false, false, portMAX_DELAY); LOG_DEBUG("wait done"); return 0; } +void i2s_out_dma_stop(struct i2s_out *i2s_out) +{ + LOG_DEBUG(""); + + // reset write state + i2s_out->dma_start = false; + + taskENTER_CRITICAL(&i2s_out->mux); + + i2s_intr_disable(i2s_out->dev, I2S_OUT_TOTAL_EOF_INT_ENA | I2S_OUT_DSCR_ERR_INT_ENA | I2S_OUT_EOF_INT_ENA); + i2s_intr_clear(i2s_out->dev, I2S_OUT_TOTAL_EOF_INT_CLR | I2S_OUT_DSCR_ERR_INT_CLR | I2S_OUT_EOF_INT_CLR); + + i2s_ll_rx_stop_link(i2s_out->dev); + i2s_ll_tx_stop_link(i2s_out->dev); + + i2s_ll_enable_dma(i2s_out->dev, false); + + i2s_ll_rx_reset_dma(i2s_out->dev); + i2s_ll_tx_reset_dma(i2s_out->dev); + + taskEXIT_CRITICAL(&i2s_out->mux); + + // reset eof state + xEventGroupClearBits(i2s_out->event_group, I2S_OUT_EVENT_GROUP_BIT_DMA_EOF | I2S_OUT_EVENT_GROUP_BIT_DMA_TOTAL_EOF); + + i2s_out->dma_eof_desc = NULL; +} + void i2s_out_dma_free(struct i2s_out *i2s_out) { free(i2s_out->dma_end_buf); diff --git a/components/i2s_out/esp32/dma.h b/components/i2s_out/esp32/dma.h index 38a00d77..87da96e3 100644 --- a/components/i2s_out/esp32/dma.h +++ b/components/i2s_out/esp32/dma.h @@ -20,6 +20,13 @@ struct dma_desc { struct dma_desc *next; }; +/* Prepare desc for re-use after DMA EOF */ +static inline void i2s_dma_desc_reset(struct dma_desc *eof_desc) +{ + eof_desc->len = 0; + eof_desc->owner = 0; +} + static inline void i2s_dma_tx_get_des_addr(i2s_dev_t *hw, uint32_t *dscr_addr) { *dscr_addr = hw->out_link_dscr; diff --git a/components/i2s_out/esp32/i2s.c b/components/i2s_out/esp32/i2s.c index 45a79fac..a19940b0 100644 --- a/components/i2s_out/esp32/i2s.c +++ b/components/i2s_out/esp32/i2s.c @@ -116,7 +116,7 @@ int i2s_out_i2s_setup(struct i2s_out *i2s_out, const struct i2s_out_options *opt i2s_out->dev->sample_rate_conf.val ); - // reset eof state + // reset event state xEventGroupClearBits(i2s_out->event_group, I2S_OUT_EVENT_GROUP_BIT_I2S_EOF); return 0; @@ -126,6 +126,9 @@ void i2s_out_i2s_start(struct i2s_out *i2s_out) { LOG_DEBUG(""); + // reset event state + xEventGroupClearBits(i2s_out->event_group, I2S_OUT_EVENT_GROUP_BIT_I2S_EOF); + taskENTER_CRITICAL(&i2s_out->mux); // NOTE: there seems to always be three extra BCK cycles at the start of TX @@ -169,4 +172,7 @@ void i2s_out_i2s_stop(struct i2s_out *i2s_out) i2s_ll_tx_stop(i2s_out->dev); taskEXIT_CRITICAL(&i2s_out->mux); + + // reset event state + xEventGroupClearBits(i2s_out->event_group, I2S_OUT_EVENT_GROUP_BIT_I2S_EOF); } diff --git a/components/i2s_out/esp32/intr.c b/components/i2s_out/esp32/intr.c index 82e92caa..8c3de6dc 100644 --- a/components/i2s_out/esp32/intr.c +++ b/components/i2s_out/esp32/intr.c @@ -25,12 +25,8 @@ void IRAM_ATTR i2s_intr_out_total_eof_handler(struct i2s_out *i2s_out, BaseType_ LOG_ISR_DEBUG("desc=%p", eof_addr); - // NOTE: this is unlikely to stop DMA before this repeats at least once - i2s_ll_tx_stop_link(i2s_out->dev); - i2s_ll_rx_stop_link(i2s_out->dev); - - // unblock flush() task - xEventGroupSetBitsFromISR(i2s_out->event_group, I2S_OUT_EVENT_GROUP_BIT_DMA_EOF, task_wokenp); + // unblock flush() tasks + xEventGroupSetBitsFromISR(i2s_out->event_group, I2S_OUT_EVENT_GROUP_BIT_DMA_EOF | I2S_OUT_EVENT_GROUP_BIT_DMA_TOTAL_EOF, task_wokenp); i2s_intr_clear(i2s_out->dev, I2S_OUT_TOTAL_EOF_INT_CLR); } @@ -54,7 +50,58 @@ void IRAM_ATTR i2s_intr_out_eof_handler(struct i2s_out *i2s_out, BaseType_t *tas struct dma_desc *eof_desc = (struct dma_desc *) eof_addr; - LOG_ISR_DEBUG("desc=%p owner=%u", eof_desc, eof_desc->owner); + // only handle normal out_desc, not repeat_desc or end_desc + if (eof_desc >= i2s_out->dma_out_desc && eof_desc < i2s_out->dma_out_desc + i2s_out->dma_out_count) { + LOG_ISR_DEBUG("eof desc=%p owner=%u len=%u", eof_desc, eof_desc->owner, eof_desc->len); + + // speculation: we may miss sone EOF ISRs + if (i2s_out->dma_eof_desc) { + for (struct dma_desc *desc = eof_desc; desc > i2s_out->dma_eof_desc; desc--) { + if (desc != eof_desc) { + LOG_ISR_WARN("miss desc=%p owner=%u len=%u", desc, desc->owner, desc->len); + } + + i2s_dma_desc_reset(desc); + } + } else { + i2s_dma_desc_reset(eof_desc); + } + + i2s_out->dma_eof_desc = eof_desc; + + // unblock get() task + xEventGroupSetBitsFromISR(i2s_out->event_group, I2S_OUT_EVENT_GROUP_BIT_DMA_EOF, task_wokenp); + + } else if (eof_desc == i2s_out->dma_end_desc) { + + // speculation: we might miss EOF ISR for an intermediate DMA descriptor, and hit TOTAL_EOF with eof_addr at the end_desc + // resulting in wait() deadlocking on dma_eof_desc not being updated + eof_desc = i2s_out->dma_out_desc + i2s_out->dma_out_count - 1; + + if (!i2s_out->dma_eof_desc) { + LOG_ISR_WARN("end desc=%p owner=%u len=%u, dma_write_desc=%p dma_eof_desc=%p", eof_desc, eof_desc->owner, eof_desc->len, i2s_out->dma_write_desc, i2s_out->dma_eof_desc); + + for (struct dma_desc *desc = eof_desc; desc >= i2s_out->dma_out_desc; desc--) { + i2s_dma_desc_reset(desc); + } + } else if (i2s_out->dma_eof_desc != eof_desc) { + LOG_ISR_WARN("end desc=%p owner=%u len=%u, dma_write_desc=%p dma_eof_desc=%p", eof_desc, eof_desc->owner, eof_desc->len, i2s_out->dma_write_desc, i2s_out->dma_eof_desc); + + for (struct dma_desc *desc = eof_desc; desc > i2s_out->dma_eof_desc; desc--) { + i2s_dma_desc_reset(desc); + } + } else { + LOG_ISR_DEBUG("end desc=%p owner=%u len=%u, dma_write_desc=%p dma_eof_desc=%p", eof_desc, eof_desc->owner, eof_desc->len, i2s_out->dma_write_desc, i2s_out->dma_eof_desc); + } + + i2s_out->dma_eof_desc = eof_desc; + + // unblock get() task + xEventGroupSetBitsFromISR(i2s_out->event_group, I2S_OUT_EVENT_GROUP_BIT_DMA_EOF, task_wokenp); + + } else { + LOG_ISR_DEBUG("ignore desc=%p owner=%u len=%u", eof_desc, eof_desc->owner, eof_desc->len); + } i2s_intr_clear(i2s_out->dev, I2S_OUT_EOF_INT_CLR); } diff --git a/components/i2s_out/i2s_out.c b/components/i2s_out/i2s_out.c index a93f4d95..c9b0e173 100644 --- a/components/i2s_out/i2s_out.c +++ b/components/i2s_out/i2s_out.c @@ -335,7 +335,10 @@ int i2s_out_repeat(struct i2s_out *i2s_out, unsigned count) return -1; } - i2s_out_dma_repeat(i2s_out, count); + if ((err = i2s_out_dma_repeat(i2s_out, count))) { + LOG_ERROR("i2s_out_dma_repeat"); + return err; + } if (!xSemaphoreGiveRecursive(i2s_out->mutex)) { LOG_WARN("xSemaphoreGiveRecursive"); @@ -344,7 +347,7 @@ int i2s_out_repeat(struct i2s_out *i2s_out, unsigned count) return err; } -int i2s_out_flush(struct i2s_out *i2s_out) +int i2s_out_wait(struct i2s_out *i2s_out) { int err = 0; @@ -353,20 +356,77 @@ int i2s_out_flush(struct i2s_out *i2s_out) return -1; } + // wait for previous start() to complete? + if (i2s_out_dma_running(i2s_out)) { + if ((err = i2s_out_dma_flush(i2s_out))) { + LOG_ERROR("i2s_out_dma_flush"); + goto error; + } + + if ((err = i2s_out_i2s_flush(i2s_out))) { + LOG_ERROR("i2s_out_i2s_flush"); + goto error; + } + + i2s_out_dma_stop(i2s_out); + i2s_out_i2s_stop(i2s_out); + } + +error: + if (!xSemaphoreGiveRecursive(i2s_out->mutex)) { + LOG_WARN("xSemaphoreGiveRecursive"); + } + + return err; + +} + +int i2s_out_start(struct i2s_out *i2s_out) +{ + int err = 0; + + if (!xSemaphoreTakeRecursive(i2s_out->mutex, portMAX_DELAY)) { + LOG_ERROR("xSemaphoreTakeRecursive"); + return -1; + } + + // wait for previous start() to complete? + if ((err = i2s_out_wait(i2s_out))) { + goto error; + } + + // have write()? if (i2s_out_dma_pending(i2s_out)) { - i2s_out_dma_start(i2s_out); + if ((err = i2s_out_dma_start(i2s_out))) { + LOG_ERROR("i2s_out_dma_start"); + return err; + } + i2s_out_i2s_start(i2s_out); } - // wait for TX DMA EOF - if ((err = i2s_out_dma_flush(i2s_out))) { - LOG_ERROR("i2s_out_dma_flush"); +error: + if (!xSemaphoreGiveRecursive(i2s_out->mutex)) { + LOG_WARN("xSemaphoreGiveRecursive"); + } + + return err; +} + +int i2s_out_flush(struct i2s_out *i2s_out) +{ + int err = 0; + + if (!xSemaphoreTakeRecursive(i2s_out->mutex, portMAX_DELAY)) { + LOG_ERROR("xSemaphoreTakeRecursive"); + return -1; + } + + if ((err = i2s_out_start(i2s_out))) { goto error; } - // wait for I2S TX done - if ((err = i2s_out_i2s_flush(i2s_out))) { - LOG_ERROR("i2s_out_i2s_flush"); + if ((err = i2s_out_wait(i2s_out))) { goto error; } @@ -382,6 +442,7 @@ int i2s_out_close(struct i2s_out *i2s_out) { int err = i2s_out_flush(i2s_out); + i2s_out_dma_stop(i2s_out); i2s_out_i2s_stop(i2s_out); i2s_out_pin_teardown(i2s_out); diff --git a/components/i2s_out/i2s_out.h b/components/i2s_out/i2s_out.h index 7298bbbc..8361e19a 100644 --- a/components/i2s_out/i2s_out.h +++ b/components/i2s_out/i2s_out.h @@ -12,8 +12,9 @@ struct dma_desc; -#define I2S_OUT_EVENT_GROUP_BIT_DMA_EOF (1 << 0) -#define I2S_OUT_EVENT_GROUP_BIT_I2S_EOF (1 << 1) +#define I2S_OUT_EVENT_GROUP_BIT_DMA_EOF (1 << 0) +#define I2S_OUT_EVENT_GROUP_BIT_DMA_TOTAL_EOF (1 << 1) +#define I2S_OUT_EVENT_GROUP_BIT_I2S_EOF (1 << 2) struct i2s_out { i2s_port_t port; @@ -50,7 +51,10 @@ struct i2s_out { // pointer to software-owned dma_out_desc used for write() struct dma_desc *dma_write_desc; - bool dma_start; // set by i2s_out_dma_start + // pointer to previous hardware-owned dma_out_desc, now available for write() - updated by ISR + struct dma_desc *dma_eof_desc; + + bool dma_start; // initialized by i2s_out_dma_setup(), set by i2s_out_dma_start(), cleared by i2s_out_dma_stop() }; /* dma.c */ @@ -59,10 +63,12 @@ int i2s_out_dma_setup(struct i2s_out *i2s_out, const struct i2s_out_options *opt size_t i2s_out_dma_buffer(struct i2s_out *i2s_out, void **ptr, unsigned count, size_t size); void i2s_out_dma_commit(struct i2s_out *i2s_out, unsigned count, size_t size); int i2s_out_dma_write(struct i2s_out *i2s_out, const void *data, size_t size); -void i2s_out_dma_repeat(struct i2s_out *i2s_out, unsigned count); +int i2s_out_dma_repeat(struct i2s_out *i2s_out, unsigned count); +int i2s_out_dma_running(struct i2s_out *i2s_out); int i2s_out_dma_pending(struct i2s_out *i2s_out); -void i2s_out_dma_start(struct i2s_out *i2s_out); +int i2s_out_dma_start(struct i2s_out *i2s_out); int i2s_out_dma_flush(struct i2s_out *i2s_out); +void i2s_out_dma_stop(struct i2s_out *i2s_out); void i2s_out_dma_free(struct i2s_out *i2s_out); /* i2s.c */ diff --git a/components/i2s_out/include/i2s_out.h b/components/i2s_out/include/i2s_out.h index db4fac7f..50dd5127 100644 --- a/components/i2s_out/include/i2s_out.h +++ b/components/i2s_out/include/i2s_out.h @@ -220,7 +220,11 @@ int i2s_out_write_serial32(struct i2s_out *i2s_out, const uint32_t *data, size_t #endif /** - * Setup DMA to repeat all written buffers given count of times. Completes any writes. + * Setup DMA to repeat all written buffers given count of times. + * + * Can only be called between open() -> write() and flush() -> close() in sync mode. + * Cannot be called after start(), and cannot call write() after. + * Remains in effect until next close() and open(). * * Returns <0 on error, 0 on success. */ From e81ea7b260ba11884da71a2936014fc0da585116 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Tue, 28 Oct 2025 20:16:12 +0200 Subject: [PATCH 12/27] main: fix i2s_data_copies ordering --- main/leds_configtab.i | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/main/leds_configtab.i b/main/leds_configtab.i index 7a5adfc5..675913b7 100644 --- a/main/leds_configtab.i +++ b/main/leds_configtab.i @@ -77,15 +77,6 @@ const struct configtab LEDS_CONFIGTAB[] = { .description = "Output I2S bit rate. Only used for protocols with a separate clock/data.", .enum_type = { .value = &LEDS_CONFIG.i2s_clock, .values = leds_i2s_clock_enum, .default_value = I2S_CLOCK_DEFAULT }, }, - { CONFIG_TYPE_UINT16, "i2s_data_copies", - .description = ( - "Output multiple copies of the data on each I2S output, to control a series of identical LEDs.\n" - "\tFor example, use count = 600, i2s_data_width = 4, i2s_data_copies = 4 to control 4 sets of 150 LEDs on each of four outputs." - "Each output will be independently controllable, but each set of 150 LEDs per output will behave identically.\n" - "\t0 -> disabled, 1 -> no effect, N -> repeat data for a total of N copies per output." - ), - .uint16_type = { .value = &LEDS_CONFIG.i2s_data_copies, .max = LEDS_I2S_REPEAT_MAX }, - }, # if LEDS_I2S_PARALLEL_ENABLED { CONFIG_TYPE_UINT16, "i2s_data_width", .description = ( @@ -99,6 +90,15 @@ const struct configtab LEDS_CONFIGTAB[] = { .ctx = &LEDS_CONFIG, }, # endif + { CONFIG_TYPE_UINT16, "i2s_data_copies", + .description = ( + "Output multiple copies of the data on each I2S output, to control a series of identical LEDs.\n" + "\tFor example, use count = 600, i2s_data_width = 4, i2s_data_copies = 4 to control 4 sets of 150 LEDs on each of four outputs." + "Each output will be independently controllable, but each set of 150 LEDs per output will behave identically.\n" + "\t0 -> disabled, 1 -> no effect, N -> repeat data for a total of N copies per output." + ), + .uint16_type = { .value = &LEDS_CONFIG.i2s_data_copies, .max = LEDS_I2S_REPEAT_MAX }, + }, # if LEDS_I2S_GPIO_PINS_ENABLED { CONFIG_TYPE_UINT16, "i2s_clock_pin", .description = "Output I2S bit clock to GPIO pin. Only used for protocols with a separate clock/data.", From d6049af643695c246587ab0fe2d91d12831c4fe4 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Tue, 28 Oct 2025 20:18:34 +0200 Subject: [PATCH 13/27] main: USER_ALERT_ERROR_LEDS on leds_setup() errors --- main/leds_task.c | 5 ++++- main/user.c | 3 ++- main/user.h | 1 + 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/main/leds_task.c b/main/leds_task.c index 1441fe8d..cac7cdb9 100644 --- a/main/leds_task.c +++ b/main/leds_task.c @@ -172,7 +172,10 @@ static void leds_main(void *ctx) } error: - return; + user_alert(USER_ALERT_ERROR_LEDS); + LOG_ERROR("task=%p stopped", state->task); + state->task = NULL; + vTaskDelete(NULL); } int init_leds_task(struct leds_state *state, const struct leds_config *config) diff --git a/main/user.c b/main/user.c index 3bc6f944..232bfb97 100644 --- a/main/user.c +++ b/main/user.c @@ -67,7 +67,8 @@ const char *user_alert_str(enum user_alert alert) case USER_ALERT_ERROR_WIFI: return "ERROR_WIFI"; case USER_ALERT_ERROR_START: return "ERROR_START"; case USER_ALERT_ERROR_DMX: return "ERROR_DMX"; - + + case USER_ALERT_ERROR_LEDS: return "ERROR_LEDS"; case USER_ALERT_ERROR_LEDS_SEQUENCE: return "ERROR_LEDS_SEQUENCE"; case USER_ALERT_ERROR_LEDS_SEQUENCE_READ: return "ERROR_LEDS_SEQUENCE_READ"; case USER_ALERT_ERROR_ATX_PSU_TIMEOUT: return "ERROR_ATX_PSU_TIMEOUT"; diff --git a/main/user.h b/main/user.h index 511d5d66..d4d115f1 100644 --- a/main/user.h +++ b/main/user.h @@ -48,6 +48,7 @@ enum user_alert { USER_ALERT_ERROR_WIFI, USER_ALERT_ERROR_START, USER_ALERT_ERROR_DMX, + USER_ALERT_ERROR_LEDS, USER_ALERT_ERROR_LEDS_SEQUENCE, USER_ALERT_ERROR_LEDS_SEQUENCE_READ, USER_ALERT_ERROR_ATX_PSU_TIMEOUT, From de8b602d60817555ea40ff65130841ad82578270 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Tue, 28 Oct 2025 20:18:49 +0200 Subject: [PATCH 14/27] logging: optional ISR WARN --- components/logging/include/logging.h | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/components/logging/include/logging.h b/components/logging/include/logging.h index 9dc941ad..5ed01844 100644 --- a/components/logging/include/logging.h +++ b/components/logging/include/logging.h @@ -20,14 +20,20 @@ #define DEBUG 0 #endif +#ifdef WARN + #define ISR_WARN 1 +#else + #define ISR_WARN 0 +#endif + #if CONFIG_LOG_COLORS #undef LOG_COLOR_D #define LOG_COLOR_D LOG_COLOR(LOG_COLOR_BLUE) #endif /* Bypass stdio buffering/interrupts, blocking write directly to UART */ -#define LOG_ISR_ERROR(fmt, ...) do { if (DEBUG) esp_rom_printf(LOG_FORMAT(E, fmt), esp_log_timestamp(), __func__, ##__VA_ARGS__); } while(0) -#define LOG_ISR_WARN(fmt, ...) do { if (DEBUG) esp_rom_printf(LOG_FORMAT(W, fmt), esp_log_timestamp(), __func__, ##__VA_ARGS__); } while(0) +#define LOG_ISR_ERROR(fmt, ...) do { if (ISR_WARN) esp_rom_printf(LOG_FORMAT(E, fmt), esp_log_timestamp(), __func__, ##__VA_ARGS__); } while(0) +#define LOG_ISR_WARN(fmt, ...) do { if (ISR_WARN) esp_rom_printf(LOG_FORMAT(W, fmt), esp_log_timestamp(), __func__, ##__VA_ARGS__); } while(0) #define LOG_ISR_INFO(fmt, ...) do { if (DEBUG) esp_rom_printf(LOG_FORMAT(I, fmt), esp_log_timestamp(), __func__, ##__VA_ARGS__); } while(0) #define LOG_ISR_DEBUG(fmt, ...) do { if (DEBUG) esp_rom_printf(LOG_FORMAT(D, fmt), esp_log_timestamp(), __func__, ##__VA_ARGS__); } while(0) From 9b92c398c8fb3490f21ba8e6c6343671bebd22f5 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Tue, 28 Oct 2025 20:25:17 +0200 Subject: [PATCH 15/27] i2s_out: cleanup i2s_out_intr_setup() DEBUG logging --- components/i2s_out/esp32/intr.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/components/i2s_out/esp32/intr.c b/components/i2s_out/esp32/intr.c index 8c3de6dc..78bef578 100644 --- a/components/i2s_out/esp32/intr.c +++ b/components/i2s_out/esp32/intr.c @@ -154,21 +154,22 @@ int i2s_out_intr_setup(struct i2s_out *i2s_out, const struct i2s_out_options *op { esp_err_t err = 0; - LOG_DEBUG("intr=%p", i2s_out->intr); + if (i2s_out->intr) { + return 0; + } taskENTER_CRITICAL(&i2s_out->mux); i2s_intr_disable_all(i2s_out->dev); - - if (!i2s_out->intr) { - err = esp_intr_alloc(i2s_irq[i2s_out->port], I2S_INTR_ALLOC_FLAGS, i2s_intr_handler, i2s_out, &i2s_out->intr); - } + err = esp_intr_alloc(i2s_irq[i2s_out->port], I2S_INTR_ALLOC_FLAGS, i2s_intr_handler, i2s_out, &i2s_out->intr); taskEXIT_CRITICAL(&i2s_out->mux); if (err) { LOG_ERROR("esp_intr_alloc: %s", esp_err_to_name(err)); return -1; + } else { + LOG_DEBUG("intr=%p", i2s_out->intr); } return 0; From 0dd0271d2dace251c922cd8cf8accc30dfb70a20 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Tue, 28 Oct 2025 20:28:11 +0200 Subject: [PATCH 16/27] i2s_out: i2s_out_dma_wait() -> DEBUG --- components/i2s_out/esp32/dma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/i2s_out/esp32/dma.c b/components/i2s_out/esp32/dma.c index f0169999..c0429dbc 100644 --- a/components/i2s_out/esp32/dma.c +++ b/components/i2s_out/esp32/dma.c @@ -211,7 +211,7 @@ struct dma_desc *i2s_out_dma_wait(struct i2s_out *i2s_out) { if (i2s_out->dma_start) { while (!i2s_out->dma_eof_desc || i2s_out->dma_write_desc > i2s_out->dma_eof_desc) { - LOG_WARN("wait for dma_write_desc=%p > dma_eof_desc=%p", i2s_out->dma_write_desc, i2s_out->dma_eof_desc); + LOG_DEBUG("wait for dma_write_desc=%p > dma_eof_desc=%p", i2s_out->dma_write_desc, i2s_out->dma_eof_desc); xEventGroupWaitBits(i2s_out->event_group, I2S_OUT_EVENT_GROUP_BIT_DMA_EOF, true, true, portMAX_DELAY); } From b4aa11a06187e015c739e83d62b9ba718645f86e Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Tue, 28 Oct 2025 20:54:09 +0200 Subject: [PATCH 17/27] logging: extra TRACE level --- components/logging/include/logging.h | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/components/logging/include/logging.h b/components/logging/include/logging.h index 5ed01844..f73233f2 100644 --- a/components/logging/include/logging.h +++ b/components/logging/include/logging.h @@ -13,6 +13,13 @@ #include #include +#ifdef TRACE + #undef TRACE + #define TRACE 1 +#else + #define TRACE 0 +#endif + #ifdef DEBUG #undef DEBUG #define DEBUG 1 @@ -49,11 +56,9 @@ #define LOG_WARN(fmt, ...) do { fprintf(stderr, LOG_FORMAT(W, fmt), esp_log_timestamp(), __func__, ##__VA_ARGS__); } while(0) #define LOG_INFO(fmt, ...) do { fprintf(stderr, LOG_FORMAT(I, fmt), esp_log_timestamp(), __func__, ##__VA_ARGS__); } while(0) #define LOG_DEBUG(fmt, ...) do { if (DEBUG) fprintf(stderr, LOG_FORMAT(D, fmt), esp_log_timestamp(), __func__, ##__VA_ARGS__); } while(0) +#define LOG_TRACE(fmt, ...) do { if (TRACE) fprintf(stderr, LOG_FORMAT(D, fmt), esp_log_timestamp(), __func__, ##__VA_ARGS__); } while(0) -// XXX: CONFIG_LOG_DEFAULT_LEVEL defaults to skip ESP_LOG_DEBUG, and raising will include ALL debug output by default - not possible to override this per-call -#if DEBUG - #define LOG_DEBUG_BUFFER(buf, len) do { if (DEBUG) ESP_LOG_BUFFER_HEX_LEVEL(__func__, buf, len, ESP_LOG_INFO); } while(0) -#else - #define LOG_DEBUG_BUFFER(buf, len) -#endif +// XXX: use ESP_LOG_INFO because CONFIG_LOG_DEFAULT_LEVEL defaults to skip ESP_LOG_DEBUG, and raising will include ALL debug output by default - not possible to override this per-call #define LOG_INFO_BUFFER(buf, len) ESP_LOG_BUFFER_HEX_LEVEL(__func__, buf, len, ESP_LOG_INFO); +#define LOG_DEBUG_BUFFER(buf, len) do { if (DEBUG) ESP_LOG_BUFFER_HEX_LEVEL(__func__, buf, len, ESP_LOG_INFO); } while(0) +#define LOG_TRACE_BUFFER(buf, len) do { if (TRACE) ESP_LOG_BUFFER_HEX_LEVEL(__func__, buf, len, ESP_LOG_INFO); } while(0) From a3f490e68c9ecd3e8818a1b8a98e623b467b05e0 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Tue, 28 Oct 2025 20:54:47 +0200 Subject: [PATCH 18/27] i2s_out: dma TRACE logging --- components/i2s_out/esp32/dma.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/components/i2s_out/esp32/dma.c b/components/i2s_out/esp32/dma.c index c0429dbc..c9fa4f7f 100644 --- a/components/i2s_out/esp32/dma.c +++ b/components/i2s_out/esp32/dma.c @@ -293,13 +293,13 @@ size_t i2s_out_dma_buffer(struct i2s_out *i2s_out, void **ptr, unsigned count, s // limit to available buffer size count = (desc->size - desc->len) / size; - LOG_DEBUG("short desc=%p (owner=%u eof=%u buf=%p len=%u size=%u) -> count=%u size=%u", desc, desc->owner, desc->eof, desc->buf, desc->len, desc->size, count, size); + LOG_TRACE("short desc=%p (owner=%u eof=%u buf=%p len=%u size=%u) -> count=%u size=%u", desc, desc->owner, desc->eof, desc->buf, desc->len, desc->size, count, size); } else if (desc->len + count * size < desc->size) { - LOG_DEBUG("long desc=%p (owner=%u eof=%u buf=%p len=%u size=%u) -> count=%u size=%u", desc, desc->owner, desc->eof, desc->buf, desc->len, desc->size, count, size); + LOG_TRACE("long desc=%p (owner=%u eof=%u buf=%p len=%u size=%u) -> count=%u size=%u", desc, desc->owner, desc->eof, desc->buf, desc->len, desc->size, count, size); } else { - LOG_DEBUG("fill desc=%p (owner=%u eof=%u buf=%p len=%u size=%u) -> count=%u size=%u", desc, desc->owner, desc->eof, desc->buf, desc->len, desc->size, count, size); + LOG_TRACE("fill desc=%p (owner=%u eof=%u buf=%p len=%u size=%u) -> count=%u size=%u", desc, desc->owner, desc->eof, desc->buf, desc->len, desc->size, count, size); } *ptr = desc->buf + desc->len; @@ -321,8 +321,8 @@ int i2s_out_dma_write(struct i2s_out *i2s_out, const void *data, size_t size) if (len) { // copy data to desc buf - LOG_DEBUG("copy len=%u -> ptr=%p", len, ptr); - LOG_DEBUG_BUFFER(data, len); + LOG_TRACE("copy len=%u -> ptr=%p", len, ptr); + LOG_TRACE_BUFFER(data, len); memcpy(ptr, data, len); From 9f4d54bd6b3d0ecebc99f2acd2b6322b0ae5c738 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Tue, 28 Oct 2025 20:58:10 +0200 Subject: [PATCH 19/27] leds: bump TEST_FRAME_RATE -> 30 --- components/leds/test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/leds/test.c b/components/leds/test.c index 1c3db83a..60d521c3 100644 --- a/components/leds/test.c +++ b/components/leds/test.c @@ -8,7 +8,7 @@ #include -#define TEST_FRAME_RATE (25) +#define TEST_FRAME_RATE (30) #define TEST_FRAME_TICKS (1000 / TEST_FRAME_RATE / portTICK_RATE_MS) #define TEST_MODE_COLOR_FRAMES 25 From 745bc1791fa8fd4b152791d6986f4f41377ea36c Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Tue, 28 Oct 2025 21:24:50 +0200 Subject: [PATCH 20/27] i2s_out: fix goto error -> locking --- components/i2s_out/i2s_out.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/components/i2s_out/i2s_out.c b/components/i2s_out/i2s_out.c index c9b0e173..2e8796cf 100644 --- a/components/i2s_out/i2s_out.c +++ b/components/i2s_out/i2s_out.c @@ -337,13 +337,14 @@ int i2s_out_repeat(struct i2s_out *i2s_out, unsigned count) if ((err = i2s_out_dma_repeat(i2s_out, count))) { LOG_ERROR("i2s_out_dma_repeat"); - return err; + goto error; } if (!xSemaphoreGiveRecursive(i2s_out->mutex)) { LOG_WARN("xSemaphoreGiveRecursive"); } +error: return err; } @@ -399,7 +400,7 @@ int i2s_out_start(struct i2s_out *i2s_out) if (i2s_out_dma_pending(i2s_out)) { if ((err = i2s_out_dma_start(i2s_out))) { LOG_ERROR("i2s_out_dma_start"); - return err; + goto error; } i2s_out_i2s_start(i2s_out); From 6b120aff5ac0a48f0951726555f2fb0a0b68816d Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Tue, 28 Oct 2025 21:25:09 +0200 Subject: [PATCH 21/27] i2s_out: timeouts --- components/i2s_out/esp32/dma.c | 44 +++++++++++------- components/i2s_out/esp32/i2s.c | 13 ++++-- components/i2s_out/i2s_out.c | 64 +++++++++++++------------- components/i2s_out/i2s_out.h | 8 ++-- components/i2s_out/include/i2s_out.h | 18 ++++---- components/leds/include/leds.h | 3 ++ components/leds/interfaces/i2s.h | 1 + components/leds/interfaces/i2s/setup.c | 5 +- components/leds/interfaces/i2s/tx.c | 20 ++++---- main/leds_i2s.c | 4 ++ 10 files changed, 103 insertions(+), 77 deletions(-) diff --git a/components/i2s_out/esp32/dma.c b/components/i2s_out/esp32/dma.c index c9fa4f7f..544e79e0 100644 --- a/components/i2s_out/esp32/dma.c +++ b/components/i2s_out/esp32/dma.c @@ -207,13 +207,18 @@ int i2s_out_dma_setup(struct i2s_out *i2s_out, const struct i2s_out_options *opt /* * Return pointer to current uncommitted dma_write_desc. */ -struct dma_desc *i2s_out_dma_wait(struct i2s_out *i2s_out) +struct dma_desc *i2s_out_dma_wait(struct i2s_out *i2s_out, TickType_t timeout) { if (i2s_out->dma_start) { while (!i2s_out->dma_eof_desc || i2s_out->dma_write_desc > i2s_out->dma_eof_desc) { LOG_DEBUG("wait for dma_write_desc=%p > dma_eof_desc=%p", i2s_out->dma_write_desc, i2s_out->dma_eof_desc); - xEventGroupWaitBits(i2s_out->event_group, I2S_OUT_EVENT_GROUP_BIT_DMA_EOF, true, true, portMAX_DELAY); + EventBits_t bits = xEventGroupWaitBits(i2s_out->event_group, I2S_OUT_EVENT_GROUP_BIT_DMA_EOF, true, true, timeout); + + if (!(bits & I2S_OUT_EVENT_GROUP_BIT_DMA_EOF)) { + LOG_WARN("timeout -> bits=%08x", bits); + return NULL; + } } } else { if (i2s_out->dma_write_desc->owner) { @@ -228,7 +233,7 @@ struct dma_desc *i2s_out_dma_wait(struct i2s_out *i2s_out) /* * Commit current dma_write_desc and return next. */ -struct dma_desc *i2s_out_dma_next(struct i2s_out *i2s_out) +struct dma_desc *i2s_out_dma_next(struct i2s_out *i2s_out, TickType_t timeout) { if (!i2s_out->dma_write_desc->owner) { // prepare for DMA @@ -243,7 +248,7 @@ struct dma_desc *i2s_out_dma_next(struct i2s_out *i2s_out) return NULL; } - return i2s_out_dma_wait(i2s_out); + return i2s_out_dma_wait(i2s_out, timeout); } /* @@ -251,15 +256,15 @@ struct dma_desc *i2s_out_dma_next(struct i2s_out *i2s_out) * * @return size of usable buffer in units of size, up to count. 0 if full */ -size_t i2s_out_dma_buffer(struct i2s_out *i2s_out, void **ptr, unsigned count, size_t size) +size_t i2s_out_dma_buffer(struct i2s_out *i2s_out, void **ptr, unsigned count, size_t size, TickType_t timeout) { struct dma_desc *desc; // stop if last desc already committed - if (!(desc = i2s_out_dma_wait(i2s_out))) { - LOG_WARN("owned desc=%p (owner=%u eof=%u buf=%p len=%u size=%u)", desc, desc->owner, desc->eof, desc->buf, desc->len, desc->size); + if (!(desc = i2s_out_dma_wait(i2s_out, timeout))) { + LOG_WARN("i2s_out_dma_wait"); - // unable to find a usable DMA buffer, TX buffers full + // unable to find a usable DMA buffer, timeout or TX buffers full? *ptr = NULL; return 0; @@ -270,13 +275,13 @@ size_t i2s_out_dma_buffer(struct i2s_out *i2s_out, void **ptr, unsigned count, s LOG_DEBUG("commit desc=%p (owner=%u eof=%u buf=%p len=%u size=%u) < size=%u -> next=%p", desc, desc->owner, desc->eof, desc->buf, desc->len, desc->size, size, desc->next); // commit, try with the next desc, if available - desc = i2s_out_dma_next(i2s_out); + desc = i2s_out_dma_next(i2s_out, timeout); } if (!desc) { - LOG_WARN("last desc=%p (owner=%u eof=%u buf=%p len=%u size=%u) < size=%u", desc, desc->owner, desc->eof, desc->buf, desc->len, desc->size, size); + LOG_WARN("i2s_out_dma_next"); - // unable to find a usable DMA buffer, TX buffers full + // unable to find a usable DMA buffer, timeout or TX buffers full? *ptr = NULL; return 0; @@ -314,10 +319,10 @@ void i2s_out_dma_commit(struct i2s_out *i2s_out, unsigned count, size_t size) desc->len += count * size; } -int i2s_out_dma_write(struct i2s_out *i2s_out, const void *data, size_t size) +int i2s_out_dma_write(struct i2s_out *i2s_out, const void *data, size_t size, TickType_t timeout) { void *ptr; - int len = i2s_out_dma_buffer(i2s_out, &ptr, size, 1); // single unaligned bytes + int len = i2s_out_dma_buffer(i2s_out, &ptr, size, 1, timeout); // single unaligned bytes if (len) { // copy data to desc buf @@ -502,13 +507,18 @@ int i2s_out_dma_start(struct i2s_out *i2s_out) return 0; } -int i2s_out_dma_flush(struct i2s_out *i2s_out) +int i2s_out_dma_flush(struct i2s_out *i2s_out, TickType_t timeout) { - LOG_DEBUG("wait event_group bits=%08x...", I2S_OUT_EVENT_GROUP_BIT_DMA_TOTAL_EOF); + LOG_DEBUG("..."); - xEventGroupWaitBits(i2s_out->event_group, I2S_OUT_EVENT_GROUP_BIT_DMA_TOTAL_EOF, false, false, portMAX_DELAY); + EventBits_t bits = xEventGroupWaitBits(i2s_out->event_group, I2S_OUT_EVENT_GROUP_BIT_DMA_TOTAL_EOF, false, false, timeout); - LOG_DEBUG("wait done"); + if (!(bits & I2S_OUT_EVENT_GROUP_BIT_DMA_TOTAL_EOF)) { + LOG_ERROR("timeout -> bits=%08x", bits); + return -1; + } else { + LOG_DEBUG("wait -> bits=%08x", bits); + } return 0; } diff --git a/components/i2s_out/esp32/i2s.c b/components/i2s_out/esp32/i2s.c index a19940b0..7193b2ff 100644 --- a/components/i2s_out/esp32/i2s.c +++ b/components/i2s_out/esp32/i2s.c @@ -140,7 +140,7 @@ void i2s_out_i2s_start(struct i2s_out *i2s_out) taskEXIT_CRITICAL(&i2s_out->mux); } -int i2s_out_i2s_flush(struct i2s_out *i2s_out) +int i2s_out_i2s_flush(struct i2s_out *i2s_out, TickType_t timeout) { EventBits_t bits; @@ -155,9 +155,16 @@ int i2s_out_i2s_flush(struct i2s_out *i2s_out) taskEXIT_CRITICAL(&i2s_out->mux); - LOG_DEBUG("wait event_group bits=%08x", I2S_OUT_EVENT_GROUP_BIT_I2S_EOF); + LOG_DEBUG("..."); - xEventGroupWaitBits(i2s_out->event_group, I2S_OUT_EVENT_GROUP_BIT_I2S_EOF, false, false, portMAX_DELAY); + bits = xEventGroupWaitBits(i2s_out->event_group, I2S_OUT_EVENT_GROUP_BIT_I2S_EOF, false, false, timeout); + + if (!(bits & I2S_OUT_EVENT_GROUP_BIT_I2S_EOF)) { + LOG_ERROR("timeout -> bits=%08x", bits); + return -1; + } else { + LOG_DEBUG("wait -> bits=%08x", bits); + } } return 0; diff --git a/components/i2s_out/i2s_out.c b/components/i2s_out/i2s_out.c index 2e8796cf..74eb46f4 100644 --- a/components/i2s_out/i2s_out.c +++ b/components/i2s_out/i2s_out.c @@ -74,11 +74,11 @@ int i2s_out_new(struct i2s_out **i2s_outp, i2s_port_t port, size_t buffer_size, return err; } -int i2s_out_open(struct i2s_out *i2s_out, const struct i2s_out_options *options) +int i2s_out_open(struct i2s_out *i2s_out, const struct i2s_out_options *options, TickType_t timeout) { int err = 0; - if (!xSemaphoreTakeRecursive(i2s_out->mutex, portMAX_DELAY)) { + if (!xSemaphoreTakeRecursive(i2s_out->mutex, timeout)) { LOG_ERROR("xSemaphoreTakeRecursive"); return -1; } @@ -116,17 +116,17 @@ int i2s_out_open(struct i2s_out *i2s_out, const struct i2s_out_options *options) return err; } -static int i2s_out_write(struct i2s_out *i2s_out, const void *data, size_t size) +static int i2s_out_write(struct i2s_out *i2s_out, const void *data, size_t size, TickType_t timeout) { int ret = 0; - if (!xSemaphoreTakeRecursive(i2s_out->mutex, portMAX_DELAY)) { + if (!xSemaphoreTakeRecursive(i2s_out->mutex, timeout)) { LOG_ERROR("xSemaphoreTakeRecursive"); return -1; } while (size) { - if ((ret = i2s_out_dma_write(i2s_out, data, size)) < 0) { + if ((ret = i2s_out_dma_write(i2s_out, data, size, timeout)) < 0) { LOG_ERROR("i2s_out_dma_write"); break; } else if (!ret) { @@ -147,16 +147,16 @@ static int i2s_out_write(struct i2s_out *i2s_out, const void *data, size_t size) return ret; } -int i2s_out_write_serial16(struct i2s_out *i2s_out, const uint16_t data[], size_t count) +int i2s_out_write_serial16(struct i2s_out *i2s_out, const uint16_t data[], size_t count, TickType_t timeout) { - return i2s_out_write(i2s_out, data, count * sizeof(*data)); + return i2s_out_write(i2s_out, data, count * sizeof(*data), timeout); } -int i2s_out_write_serial32(struct i2s_out *i2s_out, const uint32_t *data, size_t count) +int i2s_out_write_serial32(struct i2s_out *i2s_out, const uint32_t *data, size_t count, TickType_t timeout) { int ret = 0; - if (!xSemaphoreTakeRecursive(i2s_out->mutex, portMAX_DELAY)) { + if (!xSemaphoreTakeRecursive(i2s_out->mutex, timeout)) { LOG_ERROR("xSemaphoreTakeRecursive"); return -1; } @@ -169,7 +169,7 @@ int i2s_out_write_serial32(struct i2s_out *i2s_out, const uint32_t *data, size_t void *ptr; size_t len; - if (!(len = i2s_out_dma_buffer(i2s_out, &ptr, count - index, sizeof(*buf)))) { + if (!(len = i2s_out_dma_buffer(i2s_out, &ptr, count - index, sizeof(*buf), timeout))) { LOG_WARN("i2s_out_dma_buffer: DMA buffer full"); ret = 1; goto error; @@ -196,11 +196,11 @@ int i2s_out_write_serial32(struct i2s_out *i2s_out, const uint32_t *data, size_t } #if I2S_OUT_PARALLEL_SUPPORTED - int i2s_out_write_parallel8x8(struct i2s_out *i2s_out, uint8_t *data, unsigned width) + int i2s_out_write_parallel8x8(struct i2s_out *i2s_out, uint8_t *data, unsigned width, TickType_t timeout) { int ret = 0; - if (!xSemaphoreTakeRecursive(i2s_out->mutex, portMAX_DELAY)) { + if (!xSemaphoreTakeRecursive(i2s_out->mutex, timeout)) { LOG_ERROR("xSemaphoreTakeRecursive"); return -1; } @@ -213,7 +213,7 @@ int i2s_out_write_serial32(struct i2s_out *i2s_out, const uint32_t *data, size_t void *ptr; size_t count; - if (!(count = i2s_out_dma_buffer(i2s_out, &ptr, width - index, sizeof(*buf)))) { + if (!(count = i2s_out_dma_buffer(i2s_out, &ptr, width - index, sizeof(*buf), timeout))) { LOG_WARN("i2s_out_dma_buffer: DMA buffer full"); ret = 1; goto error; @@ -239,11 +239,11 @@ int i2s_out_write_serial32(struct i2s_out *i2s_out, const uint32_t *data, size_t return ret; } - int i2s_out_write_parallel8x16(struct i2s_out *i2s_out, uint16_t *data, unsigned width) + int i2s_out_write_parallel8x16(struct i2s_out *i2s_out, uint16_t *data, unsigned width, TickType_t timeout) { int ret = 0; - if (!xSemaphoreTakeRecursive(i2s_out->mutex, portMAX_DELAY)) { + if (!xSemaphoreTakeRecursive(i2s_out->mutex, timeout)) { LOG_ERROR("xSemaphoreTakeRecursive"); return -1; } @@ -256,7 +256,7 @@ int i2s_out_write_serial32(struct i2s_out *i2s_out, const uint32_t *data, size_t void *ptr; size_t count; - if (!(count = i2s_out_dma_buffer(i2s_out, &ptr, width - index, sizeof(*buf)))) { + if (!(count = i2s_out_dma_buffer(i2s_out, &ptr, width - index, sizeof(*buf), timeout))) { LOG_WARN("i2s_out_dma_buffer: DMA buffer full"); ret = 1; goto error; @@ -282,11 +282,11 @@ int i2s_out_write_serial32(struct i2s_out *i2s_out, const uint32_t *data, size_t return ret; } - int i2s_out_write_parallel8x32(struct i2s_out *i2s_out, uint32_t *data, unsigned width) + int i2s_out_write_parallel8x32(struct i2s_out *i2s_out, uint32_t *data, unsigned width, TickType_t timeout) { int ret = 0; - if (!xSemaphoreTakeRecursive(i2s_out->mutex, portMAX_DELAY)) { + if (!xSemaphoreTakeRecursive(i2s_out->mutex, timeout)) { LOG_ERROR("xSemaphoreTakeRecursive"); return -1; } @@ -299,7 +299,7 @@ int i2s_out_write_serial32(struct i2s_out *i2s_out, const uint32_t *data, size_t void *ptr; size_t count; - if (!(count = i2s_out_dma_buffer(i2s_out, &ptr, width - index, sizeof(*buf)))) { + if (!(count = i2s_out_dma_buffer(i2s_out, &ptr, width - index, sizeof(*buf), timeout))) { LOG_WARN("i2s_out_dma_buffer: DMA buffer full"); ret = 1; goto error; @@ -348,23 +348,23 @@ int i2s_out_repeat(struct i2s_out *i2s_out, unsigned count) return err; } -int i2s_out_wait(struct i2s_out *i2s_out) +int i2s_out_wait(struct i2s_out *i2s_out, TickType_t timeout) { int err = 0; - if (!xSemaphoreTakeRecursive(i2s_out->mutex, portMAX_DELAY)) { + if (!xSemaphoreTakeRecursive(i2s_out->mutex, timeout)) { LOG_ERROR("xSemaphoreTakeRecursive"); return -1; } // wait for previous start() to complete? if (i2s_out_dma_running(i2s_out)) { - if ((err = i2s_out_dma_flush(i2s_out))) { + if ((err = i2s_out_dma_flush(i2s_out, timeout))) { LOG_ERROR("i2s_out_dma_flush"); goto error; } - if ((err = i2s_out_i2s_flush(i2s_out))) { + if ((err = i2s_out_i2s_flush(i2s_out, timeout))) { LOG_ERROR("i2s_out_i2s_flush"); goto error; } @@ -382,17 +382,17 @@ int i2s_out_wait(struct i2s_out *i2s_out) } -int i2s_out_start(struct i2s_out *i2s_out) +int i2s_out_start(struct i2s_out *i2s_out, TickType_t timeout) { int err = 0; - if (!xSemaphoreTakeRecursive(i2s_out->mutex, portMAX_DELAY)) { + if (!xSemaphoreTakeRecursive(i2s_out->mutex, timeout)) { LOG_ERROR("xSemaphoreTakeRecursive"); return -1; } // wait for previous start() to complete? - if ((err = i2s_out_wait(i2s_out))) { + if ((err = i2s_out_wait(i2s_out, timeout))) { goto error; } @@ -414,20 +414,20 @@ int i2s_out_start(struct i2s_out *i2s_out) return err; } -int i2s_out_flush(struct i2s_out *i2s_out) +int i2s_out_flush(struct i2s_out *i2s_out, TickType_t timeout) { int err = 0; - if (!xSemaphoreTakeRecursive(i2s_out->mutex, portMAX_DELAY)) { + if (!xSemaphoreTakeRecursive(i2s_out->mutex, timeout)) { LOG_ERROR("xSemaphoreTakeRecursive"); return -1; } - if ((err = i2s_out_start(i2s_out))) { + if ((err = i2s_out_start(i2s_out, timeout))) { goto error; } - if ((err = i2s_out_wait(i2s_out))) { + if ((err = i2s_out_wait(i2s_out, timeout))) { goto error; } @@ -439,9 +439,9 @@ int i2s_out_flush(struct i2s_out *i2s_out) return err; } -int i2s_out_close(struct i2s_out *i2s_out) +int i2s_out_close(struct i2s_out *i2s_out, TickType_t timeout) { - int err = i2s_out_flush(i2s_out); + int err = i2s_out_flush(i2s_out, timeout); i2s_out_dma_stop(i2s_out); i2s_out_i2s_stop(i2s_out); diff --git a/components/i2s_out/i2s_out.h b/components/i2s_out/i2s_out.h index 8361e19a..a3d18579 100644 --- a/components/i2s_out/i2s_out.h +++ b/components/i2s_out/i2s_out.h @@ -60,14 +60,14 @@ struct i2s_out { /* dma.c */ int i2s_out_dma_init(struct i2s_out *i2s_out, size_t size, size_t align, unsigned repeat); int i2s_out_dma_setup(struct i2s_out *i2s_out, const struct i2s_out_options *options); -size_t i2s_out_dma_buffer(struct i2s_out *i2s_out, void **ptr, unsigned count, size_t size); +size_t i2s_out_dma_buffer(struct i2s_out *i2s_out, void **ptr, unsigned count, size_t size, TickType_t timeout); void i2s_out_dma_commit(struct i2s_out *i2s_out, unsigned count, size_t size); -int i2s_out_dma_write(struct i2s_out *i2s_out, const void *data, size_t size); +int i2s_out_dma_write(struct i2s_out *i2s_out, const void *data, size_t size, TickType_t timeout); int i2s_out_dma_repeat(struct i2s_out *i2s_out, unsigned count); int i2s_out_dma_running(struct i2s_out *i2s_out); int i2s_out_dma_pending(struct i2s_out *i2s_out); int i2s_out_dma_start(struct i2s_out *i2s_out); -int i2s_out_dma_flush(struct i2s_out *i2s_out); +int i2s_out_dma_flush(struct i2s_out *i2s_out, TickType_t timeout); void i2s_out_dma_stop(struct i2s_out *i2s_out); void i2s_out_dma_free(struct i2s_out *i2s_out); @@ -75,7 +75,7 @@ void i2s_out_dma_free(struct i2s_out *i2s_out); int i2s_out_i2s_init(struct i2s_out *i2s_out); int i2s_out_i2s_setup(struct i2s_out *i2s_out, const struct i2s_out_options *options); void i2s_out_i2s_start(struct i2s_out *i2s_out); -int i2s_out_i2s_flush(struct i2s_out *i2s_out); +int i2s_out_i2s_flush(struct i2s_out *i2s_out, TickType_t timeout); void i2s_out_i2s_stop(struct i2s_out *i2s_out); /* dev.c */ diff --git a/components/i2s_out/include/i2s_out.h b/components/i2s_out/include/i2s_out.h index 50dd5127..63b0c5ca 100644 --- a/components/i2s_out/include/i2s_out.h +++ b/components/i2s_out/include/i2s_out.h @@ -145,7 +145,7 @@ int i2s_out_new(struct i2s_out **i2s_outp, i2s_port_t port, size_t buffer_size, * * Returns <0 on error, 0 on success. */ -int i2s_out_open(struct i2s_out *i2s_out, const struct i2s_out_options *options); +int i2s_out_open(struct i2s_out *i2s_out, const struct i2s_out_options *options, TickType_t timeout); /** * Copy exactly `count` 16-bit words from `data` into the internal TX DMA buffer for serial output in little-endian order. @@ -158,7 +158,7 @@ int i2s_out_open(struct i2s_out *i2s_out, const struct i2s_out_options *options) * * Returns <0 error, 0 on success, >0 if TX buffer is full. */ -int i2s_out_write_serial16(struct i2s_out *i2s_out, const uint16_t data[], size_t count); +int i2s_out_write_serial16(struct i2s_out *i2s_out, const uint16_t data[], size_t count, TickType_t timeout); /** * Copy exactly `count` 32-bit words from `data` into the internal TX DMA buffer for serial output in little-endian order. @@ -173,7 +173,7 @@ int i2s_out_write_serial16(struct i2s_out *i2s_out, const uint16_t data[], size_ * * Returns <0 error, 0 on success, >0 if TX buffer is full. */ -int i2s_out_write_serial32(struct i2s_out *i2s_out, const uint32_t *data, size_t count); +int i2s_out_write_serial32(struct i2s_out *i2s_out, const uint32_t *data, size_t count, TickType_t timeout); #if I2S_OUT_PARALLEL_SUPPORTED /** @@ -188,7 +188,7 @@ int i2s_out_write_serial32(struct i2s_out *i2s_out, const uint32_t *data, size_t * * Returns <0 error, 0 on success, >0 if TX buffer is full. */ - int i2s_out_write_parallel8x8(struct i2s_out *i2s_out, uint8_t *data, unsigned width); + int i2s_out_write_parallel8x8(struct i2s_out *i2s_out, uint8_t *data, unsigned width, TickType_t timeout); /** * Copy 8 channels of `width` x 16-bit `data` into the internal TX DMA buffer, transposing the buffers for @@ -202,7 +202,7 @@ int i2s_out_write_serial32(struct i2s_out *i2s_out, const uint32_t *data, size_t * * Returns <0 error, 0 on success, >0 if TX buffer is full. */ - int i2s_out_write_parallel8x16(struct i2s_out *i2s_out, uint16_t *data, unsigned width); + int i2s_out_write_parallel8x16(struct i2s_out *i2s_out, uint16_t *data, unsigned width, TickType_t timeout); /** * Copy 8 channels of `width` x 32-bit `data` into the internal TX DMA buffer, transposing the buffers for @@ -216,7 +216,7 @@ int i2s_out_write_serial32(struct i2s_out *i2s_out, const uint32_t *data, size_t * * Returns <0 error, 0 on success, >0 if TX buffer is full. */ - int i2s_out_write_parallel8x32(struct i2s_out *i2s_out, uint32_t *data, unsigned width); + int i2s_out_write_parallel8x32(struct i2s_out *i2s_out, uint32_t *data, unsigned width, TickType_t timeout); #endif /** @@ -235,21 +235,21 @@ int i2s_out_repeat(struct i2s_out *i2s_out, unsigned count); * * Returns <0 on error, 0 on success. */ -int i2s_out_start(struct i2s_out *i2s_out); +int i2s_out_start(struct i2s_out *i2s_out, TickType_t timeout); /** * Start I2S output, and wait for the complete TX buffer and EOF frame to be written. * * Returns <0 on error, 0 on success. */ -int i2s_out_flush(struct i2s_out *i2s_out); +int i2s_out_flush(struct i2s_out *i2s_out, TickType_t timeout); /** * Flush I2S output, and release the lock acquired using `i2s_out_open()`. * * Tears down the data_gpio pin. */ -int i2s_out_close(struct i2s_out *i2s_out); +int i2s_out_close(struct i2s_out *i2s_out, TickType_t timeout); /** * Tear down all state setup by i2s_out_open(), including state typically shared across multiple open -> close calls. diff --git a/components/leds/include/leds.h b/components/leds/include/leds.h index 35f37b58..b377f521 100644 --- a/components/leds/include/leds.h +++ b/components/leds/include/leds.h @@ -225,6 +225,9 @@ enum leds_interface leds_interface_for_protocol(enum leds_protocol protocol); struct i2s_out *i2s_out; + // General timeout for all i2s_out_*() operations + TickType_t timeout; + SemaphoreHandle_t pin_mutex; TickType_t pin_timeout; diff --git a/components/leds/interfaces/i2s.h b/components/leds/interfaces/i2s.h index bbf3cc15..09c47c2d 100644 --- a/components/leds/interfaces/i2s.h +++ b/components/leds/interfaces/i2s.h @@ -65,6 +65,7 @@ struct leds_interface_i2s { struct leds_interface_options_gpio gpio; struct leds_interface_i2s_stats *stats; + TickType_t timeout; }; size_t leds_interface_i2s_buffer_size(enum leds_interface_i2s_mode mode, unsigned led_count, unsigned pin_count); diff --git a/components/leds/interfaces/i2s/setup.c b/components/leds/interfaces/i2s/setup.c index f898f26c..e8f28112 100644 --- a/components/leds/interfaces/i2s/setup.c +++ b/components/leds/interfaces/i2s/setup.c @@ -138,6 +138,7 @@ int leds_interface_i2s_init(struct leds_interface_i2s *interface, const struct l interface->gpio = options->gpio; interface->stats = stats; + interface->timeout = options->timeout; return 0; } @@ -147,7 +148,7 @@ int leds_interface_i2s_setup(struct leds_interface_i2s *interface) int err = 0; WITH_STATS_TIMER(&interface->stats->open) { - if ((err = i2s_out_open(interface->i2s_out, &interface->i2s_out_options))) { + if ((err = i2s_out_open(interface->i2s_out, &interface->i2s_out_options, interface->timeout))) { LOG_ERROR("i2s_out_open"); return err; } @@ -172,7 +173,7 @@ int leds_interface_i2s_close(struct leds_interface_i2s *interface) leds_gpio_close(&interface->gpio); #endif - if ((err = i2s_out_close(interface->i2s_out))) { + if ((err = i2s_out_close(interface->i2s_out, interface->timeout))) { LOG_ERROR("i2s_out_close"); return err; } diff --git a/components/leds/interfaces/i2s/tx.c b/components/leds/interfaces/i2s/tx.c index 338506d0..117441e3 100644 --- a/components/leds/interfaces/i2s/tx.c +++ b/components/leds/interfaces/i2s/tx.c @@ -11,7 +11,7 @@ static int leds_interface_i2s_tx_32bit_bck_serial32(struct leds_interface_i2s *i // start frame uint32_t start_frame = leds_interface_i2s_mode_start_frame(interface->mode); - if ((err = i2s_out_write_serial32(interface->i2s_out, &start_frame, 1))) { + if ((err = i2s_out_write_serial32(interface->i2s_out, &start_frame, 1, interface->timeout))) { LOG_ERROR("i2s_out_write_serial32"); return err; } @@ -21,7 +21,7 @@ static int leds_interface_i2s_tx_32bit_bck_serial32(struct leds_interface_i2s *i // 32-bit pixel data interface->func.i2s_mode_32bit(interface->buf->i2s_mode_32bit, pixels, i, limit); - if ((err = i2s_out_write_serial32(interface->i2s_out, interface->buf->i2s_mode_32bit, 1))) { + if ((err = i2s_out_write_serial32(interface->i2s_out, interface->buf->i2s_mode_32bit, 1, interface->timeout))) { LOG_ERROR("i2s_out_write_serial32"); return err; } @@ -38,7 +38,7 @@ static int leds_interface_i2s_tx_24bit_4x4_serial16(struct leds_interface_i2s *i // 6x16-bit pixel data interface->func.i2s_mode_24bit_4x4(interface->buf->i2s_mode_24bit_4x4, pixels, i, limit); - if ((err = i2s_out_write_serial16(interface->i2s_out, interface->buf->i2s_mode_24bit_4x4, 6))) { + if ((err = i2s_out_write_serial16(interface->i2s_out, interface->buf->i2s_mode_24bit_4x4, 6, interface->timeout))) { LOG_ERROR("i2s_out_write_serial16"); return err; } @@ -55,7 +55,7 @@ static int leds_interface_i2s_tx_32bit_4x4_serial16(struct leds_interface_i2s *i // 8x16-bit pixel data interface->func.i2s_mode_32bit_4x4(interface->buf->i2s_mode_32bit_4x4, pixels, i, limit); - if ((err = i2s_out_write_serial16(interface->i2s_out, interface->buf->i2s_mode_32bit_4x4, 8))) { + if ((err = i2s_out_write_serial16(interface->i2s_out, interface->buf->i2s_mode_32bit_4x4, 8, interface->timeout))) { LOG_ERROR("i2s_out_write_serial16"); return err; } @@ -73,7 +73,7 @@ static int leds_interface_i2s_tx_32bit_4x4_serial16(struct leds_interface_i2s *i // start frame uint32_t start_frame[8] = { [0 ... 7] = leds_interface_i2s_mode_start_frame(interface->mode) }; - if ((err = i2s_out_write_parallel8x32(interface->i2s_out, start_frame, 1))) { + if ((err = i2s_out_write_parallel8x32(interface->i2s_out, start_frame, 1, interface->timeout))) { LOG_ERROR("i2s_out_write_parallel8x32"); return err; } @@ -84,7 +84,7 @@ static int leds_interface_i2s_tx_32bit_4x4_serial16(struct leds_interface_i2s *i interface->func.i2s_mode_32bit(interface->buf->i2s_mode_32bit_parallel8[j], pixels, j * length + i, limit); } - if ((err = i2s_out_write_parallel8x32(interface->i2s_out, (uint32_t *) interface->buf->i2s_mode_32bit_parallel8, 1))) { + if ((err = i2s_out_write_parallel8x32(interface->i2s_out, (uint32_t *) interface->buf->i2s_mode_32bit_parallel8, 1, interface->timeout))) { LOG_ERROR("i2s_out_write_parallel8x32"); return err; } @@ -104,7 +104,7 @@ static int leds_interface_i2s_tx_32bit_4x4_serial16(struct leds_interface_i2s *i interface->func.i2s_mode_24bit_4x4(interface->buf->i2s_mode_24bit_4x4_parallel8[j], pixels, j * length + i, limit); } - if ((err = i2s_out_write_parallel8x16(interface->i2s_out, (uint16_t *) interface->buf->i2s_mode_24bit_4x4_parallel8, 6))) { + if ((err = i2s_out_write_parallel8x16(interface->i2s_out, (uint16_t *) interface->buf->i2s_mode_24bit_4x4_parallel8, 6, interface->timeout))) { LOG_ERROR("i2s_out_write_parallel8x16"); return err; } @@ -124,7 +124,7 @@ static int leds_interface_i2s_tx_32bit_4x4_serial16(struct leds_interface_i2s *i interface->func.i2s_mode_32bit_4x4(interface->buf->i2s_mode_32bit_4x4_parallel8[j], pixels, j * length + i, limit); } - if ((err = i2s_out_write_parallel8x16(interface->i2s_out, (uint16_t *) interface->buf->i2s_mode_32bit_4x4_parallel8, 8))) { + if ((err = i2s_out_write_parallel8x16(interface->i2s_out, (uint16_t *) interface->buf->i2s_mode_32bit_4x4_parallel8, 8, interface->timeout))) { LOG_ERROR("i2s_out_write_parallel8x16"); return err; } @@ -205,7 +205,7 @@ int leds_interface_i2s_tx(struct leds_interface_i2s *interface, const struct led if (setup) { // sync, wait for done before close WITH_STATS_TIMER(&interface->stats->flush) { - if ((err = i2s_out_flush(interface->i2s_out))) { + if ((err = i2s_out_flush(interface->i2s_out, interface->timeout))) { LOG_ERROR("i2s_out_flush"); goto error; } @@ -213,7 +213,7 @@ int leds_interface_i2s_tx(struct leds_interface_i2s *interface, const struct led } else { // async, do not wait for done WITH_STATS_TIMER(&interface->stats->start) { - if ((err = i2s_out_start(interface->i2s_out))) { + if ((err = i2s_out_start(interface->i2s_out, interface->timeout))) { LOG_ERROR("i2s_out_start"); goto error; } diff --git a/main/leds_i2s.c b/main/leds_i2s.c index dc6c44b2..820bd608 100644 --- a/main/leds_i2s.c +++ b/main/leds_i2s.c @@ -6,6 +6,9 @@ #include +// generic timeout +#define LEDS_I2S_TIMEOUT (1000 / portTICK_RATE_MS) + // do not block if pin in use, timeout immediately #define LEDS_I2S_PIN_TIMEOUT portMAX_DELAY @@ -121,6 +124,7 @@ } options->i2s_out = leds_i2s_out[port]; + options->timeout = LEDS_I2S_TIMEOUT; #if CONFIG_IDF_TARGET_ESP8266 // TODO: use i2s_pin_mutex for arbitrary gpio pins with LEDS_I2S_GPIO_PINS_ENABLED? options->pin_mutex = pin_mutex[PIN_MUTEX_I2S0_DATA]; // shared with console uart0 From 47f2dbc19bb271b076f0a93c00dea1cd6f48e469 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Tue, 28 Oct 2025 21:34:53 +0200 Subject: [PATCH 22/27] i2s_out: enforce setup --- components/i2s_out/i2s_out.c | 81 +++++++++++++++++++++++++++++++++++- components/i2s_out/i2s_out.h | 1 + 2 files changed, 80 insertions(+), 2 deletions(-) diff --git a/components/i2s_out/i2s_out.c b/components/i2s_out/i2s_out.c index 74eb46f4..6dedc92d 100644 --- a/components/i2s_out/i2s_out.c +++ b/components/i2s_out/i2s_out.c @@ -108,6 +108,8 @@ int i2s_out_open(struct i2s_out *i2s_out, const struct i2s_out_options *options, goto error; } + i2s_out->setup = true; + return 0; error: @@ -125,6 +127,11 @@ static int i2s_out_write(struct i2s_out *i2s_out, const void *data, size_t size, return -1; } + if (!i2s_out->setup) { + LOG_ERROR("setup"); + return -1; + } + while (size) { if ((ret = i2s_out_dma_write(i2s_out, data, size, timeout)) < 0) { LOG_ERROR("i2s_out_dma_write"); @@ -161,6 +168,11 @@ int i2s_out_write_serial32(struct i2s_out *i2s_out, const uint32_t *data, size_t return -1; } + if (!i2s_out->setup) { + LOG_ERROR("setup"); + return -1; + } + uint32_t (*buf)[1]; unsigned index = 0; @@ -205,6 +217,11 @@ int i2s_out_write_serial32(struct i2s_out *i2s_out, const uint32_t *data, size_t return -1; } + if (!i2s_out->setup) { + LOG_ERROR("setup"); + return -1; + } + uint32_t (*buf)[2]; unsigned index = 0; @@ -248,6 +265,11 @@ int i2s_out_write_serial32(struct i2s_out *i2s_out, const uint32_t *data, size_t return -1; } + if (!i2s_out->setup) { + LOG_ERROR("setup"); + return -1; + } + uint32_t (*buf)[4]; unsigned index = 0; @@ -291,6 +313,11 @@ int i2s_out_write_serial32(struct i2s_out *i2s_out, const uint32_t *data, size_t return -1; } + if (!i2s_out->setup) { + LOG_ERROR("setup"); + return -1; + } + uint32_t (*buf)[8]; unsigned index = 0; @@ -335,6 +362,11 @@ int i2s_out_repeat(struct i2s_out *i2s_out, unsigned count) return -1; } + if (!i2s_out->setup) { + LOG_ERROR("setup"); + return -1; + } + if ((err = i2s_out_dma_repeat(i2s_out, count))) { LOG_ERROR("i2s_out_dma_repeat"); goto error; @@ -357,6 +389,11 @@ int i2s_out_wait(struct i2s_out *i2s_out, TickType_t timeout) return -1; } + if (!i2s_out->setup) { + LOG_ERROR("setup"); + return -1; + } + // wait for previous start() to complete? if (i2s_out_dma_running(i2s_out)) { if ((err = i2s_out_dma_flush(i2s_out, timeout))) { @@ -391,6 +428,11 @@ int i2s_out_start(struct i2s_out *i2s_out, TickType_t timeout) return -1; } + if (!i2s_out->setup) { + LOG_ERROR("setup"); + return -1; + } + // wait for previous start() to complete? if ((err = i2s_out_wait(i2s_out, timeout))) { goto error; @@ -423,6 +465,11 @@ int i2s_out_flush(struct i2s_out *i2s_out, TickType_t timeout) return -1; } + if (!i2s_out->setup) { + LOG_ERROR("setup"); + return -1; + } + if ((err = i2s_out_start(i2s_out, timeout))) { goto error; } @@ -441,12 +488,33 @@ int i2s_out_flush(struct i2s_out *i2s_out, TickType_t timeout) int i2s_out_close(struct i2s_out *i2s_out, TickType_t timeout) { - int err = i2s_out_flush(i2s_out, timeout); + int err = 0; + + if (!xSemaphoreTakeRecursive(i2s_out->mutex, timeout)) { + LOG_ERROR("xSemaphoreTakeRecursive"); + return -1; + } + + if (!i2s_out->setup) { + LOG_WARN("setup"); + err = 1; + goto error; + } + + err = i2s_out_flush(i2s_out, timeout); i2s_out_dma_stop(i2s_out); i2s_out_i2s_stop(i2s_out); i2s_out_pin_teardown(i2s_out); + i2s_out->setup = false; + + if (!xSemaphoreGiveRecursive(i2s_out->mutex)) { + LOG_WARN("xSemaphoreGiveRecursive"); + } + +error: + // from open() if (!xSemaphoreGiveRecursive(i2s_out->mutex)) { LOG_WARN("xSemaphoreGiveRecursive"); } @@ -456,17 +524,26 @@ int i2s_out_close(struct i2s_out *i2s_out, TickType_t timeout) int i2s_out_teardown(struct i2s_out *i2s_out) { + int err = 0; + if (!xSemaphoreTakeRecursive(i2s_out->mutex, portMAX_DELAY)) { LOG_ERROR("xSemaphoreTakeRecursive"); return -1; } + if (i2s_out->setup) { + LOG_WARN("setup"); + err = 1; + goto error; + } + i2s_out_intr_teardown(i2s_out); i2s_out_dev_teardown(i2s_out); +error: if (!xSemaphoreGiveRecursive(i2s_out->mutex)) { LOG_WARN("xSemaphoreGiveRecursive"); } - return 0; + return err; } diff --git a/components/i2s_out/i2s_out.h b/components/i2s_out/i2s_out.h index a3d18579..ba353aaf 100644 --- a/components/i2s_out/i2s_out.h +++ b/components/i2s_out/i2s_out.h @@ -23,6 +23,7 @@ struct i2s_out { portMUX_TYPE mux; #endif EventGroupHandle_t event_group; + bool setup; /* dev */ SemaphoreHandle_t dev_mutex; From 197df52a236eee2f84852b1152cdabbb50736a7d Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Tue, 28 Oct 2025 21:46:17 +0200 Subject: [PATCH 23/27] leds_interface_i2s->options --- components/leds/interfaces/i2s.h | 5 ++-- components/leds/interfaces/i2s/setup.c | 11 ++++----- components/leds/interfaces/i2s/tx.c | 32 +++++++++++++------------- 3 files changed, 23 insertions(+), 25 deletions(-) diff --git a/components/leds/interfaces/i2s.h b/components/leds/interfaces/i2s.h index 09c47c2d..4047b2bc 100644 --- a/components/leds/interfaces/i2s.h +++ b/components/leds/interfaces/i2s.h @@ -51,21 +51,20 @@ union leds_interface_i2s_func { #define LEDS_INTERFACE_I2S_FUNC(type, func) ((union leds_interface_i2s_func) { .type = func }) struct leds_interface_i2s { - bool setup; // persistent setup() + const struct leds_interface_i2s_options *options; enum leds_interface_i2s_mode mode; union leds_interface_i2s_func func; union leds_interface_i2s_buf *buf; unsigned parallel; - unsigned repeat; struct i2s_out *i2s_out; struct i2s_out_options i2s_out_options; + bool i2s_out_setup; struct leds_interface_options_gpio gpio; struct leds_interface_i2s_stats *stats; - TickType_t timeout; }; size_t leds_interface_i2s_buffer_size(enum leds_interface_i2s_mode mode, unsigned led_count, unsigned pin_count); diff --git a/components/leds/interfaces/i2s/setup.c b/components/leds/interfaces/i2s/setup.c index e8f28112..bbac41f1 100644 --- a/components/leds/interfaces/i2s/setup.c +++ b/components/leds/interfaces/i2s/setup.c @@ -7,6 +7,7 @@ int leds_interface_i2s_init(struct leds_interface_i2s *interface, const struct leds_interface_i2s_options *options, enum leds_interface_i2s_mode mode, union leds_interface_i2s_func func, unsigned count, struct leds_interface_i2s_stats *stats) { + interface->options = options; interface->mode = mode; interface->func = func; @@ -15,7 +16,6 @@ int leds_interface_i2s_init(struct leds_interface_i2s *interface, const struct l #else interface->parallel = 0; #endif - interface->repeat = options->repeat; if (!(interface->buf = calloc(1, leds_interface_i2s_buf_size(interface->mode, interface->parallel)))) { LOG_ERROR("calloc"); @@ -138,7 +138,6 @@ int leds_interface_i2s_init(struct leds_interface_i2s *interface, const struct l interface->gpio = options->gpio; interface->stats = stats; - interface->timeout = options->timeout; return 0; } @@ -148,13 +147,13 @@ int leds_interface_i2s_setup(struct leds_interface_i2s *interface) int err = 0; WITH_STATS_TIMER(&interface->stats->open) { - if ((err = i2s_out_open(interface->i2s_out, &interface->i2s_out_options, interface->timeout))) { + if ((err = i2s_out_open(interface->i2s_out, &interface->i2s_out_options, interface->options->timeout))) { LOG_ERROR("i2s_out_open"); return err; } } - interface->setup = true; + interface->i2s_out_setup = true; #if CONFIG_LEDS_GPIO_ENABLED leds_gpio_setup(&interface->gpio); @@ -167,13 +166,13 @@ int leds_interface_i2s_close(struct leds_interface_i2s *interface) { int err = 0; - interface->setup = false; + interface->i2s_out_setup = false; #if CONFIG_LEDS_GPIO_ENABLED leds_gpio_close(&interface->gpio); #endif - if ((err = i2s_out_close(interface->i2s_out, interface->timeout))) { + if ((err = i2s_out_close(interface->i2s_out, interface->options->timeout))) { LOG_ERROR("i2s_out_close"); return err; } diff --git a/components/leds/interfaces/i2s/tx.c b/components/leds/interfaces/i2s/tx.c index 117441e3..e668e2f7 100644 --- a/components/leds/interfaces/i2s/tx.c +++ b/components/leds/interfaces/i2s/tx.c @@ -11,7 +11,7 @@ static int leds_interface_i2s_tx_32bit_bck_serial32(struct leds_interface_i2s *i // start frame uint32_t start_frame = leds_interface_i2s_mode_start_frame(interface->mode); - if ((err = i2s_out_write_serial32(interface->i2s_out, &start_frame, 1, interface->timeout))) { + if ((err = i2s_out_write_serial32(interface->i2s_out, &start_frame, 1, interface->options->timeout))) { LOG_ERROR("i2s_out_write_serial32"); return err; } @@ -21,7 +21,7 @@ static int leds_interface_i2s_tx_32bit_bck_serial32(struct leds_interface_i2s *i // 32-bit pixel data interface->func.i2s_mode_32bit(interface->buf->i2s_mode_32bit, pixels, i, limit); - if ((err = i2s_out_write_serial32(interface->i2s_out, interface->buf->i2s_mode_32bit, 1, interface->timeout))) { + if ((err = i2s_out_write_serial32(interface->i2s_out, interface->buf->i2s_mode_32bit, 1, interface->options->timeout))) { LOG_ERROR("i2s_out_write_serial32"); return err; } @@ -38,7 +38,7 @@ static int leds_interface_i2s_tx_24bit_4x4_serial16(struct leds_interface_i2s *i // 6x16-bit pixel data interface->func.i2s_mode_24bit_4x4(interface->buf->i2s_mode_24bit_4x4, pixels, i, limit); - if ((err = i2s_out_write_serial16(interface->i2s_out, interface->buf->i2s_mode_24bit_4x4, 6, interface->timeout))) { + if ((err = i2s_out_write_serial16(interface->i2s_out, interface->buf->i2s_mode_24bit_4x4, 6, interface->options->timeout))) { LOG_ERROR("i2s_out_write_serial16"); return err; } @@ -55,7 +55,7 @@ static int leds_interface_i2s_tx_32bit_4x4_serial16(struct leds_interface_i2s *i // 8x16-bit pixel data interface->func.i2s_mode_32bit_4x4(interface->buf->i2s_mode_32bit_4x4, pixels, i, limit); - if ((err = i2s_out_write_serial16(interface->i2s_out, interface->buf->i2s_mode_32bit_4x4, 8, interface->timeout))) { + if ((err = i2s_out_write_serial16(interface->i2s_out, interface->buf->i2s_mode_32bit_4x4, 8, interface->options->timeout))) { LOG_ERROR("i2s_out_write_serial16"); return err; } @@ -73,7 +73,7 @@ static int leds_interface_i2s_tx_32bit_4x4_serial16(struct leds_interface_i2s *i // start frame uint32_t start_frame[8] = { [0 ... 7] = leds_interface_i2s_mode_start_frame(interface->mode) }; - if ((err = i2s_out_write_parallel8x32(interface->i2s_out, start_frame, 1, interface->timeout))) { + if ((err = i2s_out_write_parallel8x32(interface->i2s_out, start_frame, 1, interface->options->timeout))) { LOG_ERROR("i2s_out_write_parallel8x32"); return err; } @@ -84,7 +84,7 @@ static int leds_interface_i2s_tx_32bit_4x4_serial16(struct leds_interface_i2s *i interface->func.i2s_mode_32bit(interface->buf->i2s_mode_32bit_parallel8[j], pixels, j * length + i, limit); } - if ((err = i2s_out_write_parallel8x32(interface->i2s_out, (uint32_t *) interface->buf->i2s_mode_32bit_parallel8, 1, interface->timeout))) { + if ((err = i2s_out_write_parallel8x32(interface->i2s_out, (uint32_t *) interface->buf->i2s_mode_32bit_parallel8, 1, interface->options->timeout))) { LOG_ERROR("i2s_out_write_parallel8x32"); return err; } @@ -104,7 +104,7 @@ static int leds_interface_i2s_tx_32bit_4x4_serial16(struct leds_interface_i2s *i interface->func.i2s_mode_24bit_4x4(interface->buf->i2s_mode_24bit_4x4_parallel8[j], pixels, j * length + i, limit); } - if ((err = i2s_out_write_parallel8x16(interface->i2s_out, (uint16_t *) interface->buf->i2s_mode_24bit_4x4_parallel8, 6, interface->timeout))) { + if ((err = i2s_out_write_parallel8x16(interface->i2s_out, (uint16_t *) interface->buf->i2s_mode_24bit_4x4_parallel8, 6, interface->options->timeout))) { LOG_ERROR("i2s_out_write_parallel8x16"); return err; } @@ -124,7 +124,7 @@ static int leds_interface_i2s_tx_32bit_4x4_serial16(struct leds_interface_i2s *i interface->func.i2s_mode_32bit_4x4(interface->buf->i2s_mode_32bit_4x4_parallel8[j], pixels, j * length + i, limit); } - if ((err = i2s_out_write_parallel8x16(interface->i2s_out, (uint16_t *) interface->buf->i2s_mode_32bit_4x4_parallel8, 8, interface->timeout))) { + if ((err = i2s_out_write_parallel8x16(interface->i2s_out, (uint16_t *) interface->buf->i2s_mode_32bit_4x4_parallel8, 8, interface->options->timeout))) { LOG_ERROR("i2s_out_write_parallel8x16"); return err; } @@ -179,10 +179,10 @@ static int leds_interface_i2s_tx_write(struct leds_interface_i2s *interface, con int leds_interface_i2s_tx(struct leds_interface_i2s *interface, const struct leds_color *pixels, unsigned count, const struct leds_limit *limit) { - bool setup = !interface->setup; // sync setup() -> write -> flush -> close()? + bool setup = interface->i2s_out_setup; // sync setup() -> write -> flush -> close()? int err = 0; - if (setup) { + if (!setup) { if ((err = leds_interface_i2s_setup(interface))) { LOG_ERROR("leds_interface_i2s_setup"); return err; @@ -195,17 +195,17 @@ int leds_interface_i2s_tx(struct leds_interface_i2s *interface, const struct led } } - if (interface->repeat) { - if ((err = i2s_out_repeat(interface->i2s_out, interface->repeat))) { + if (interface->options->repeat) { + if ((err = i2s_out_repeat(interface->i2s_out, interface->options->repeat))) { LOG_ERROR("i2s_out_repeat"); goto error; } } - if (setup) { + if (!setup) { // sync, wait for done before close WITH_STATS_TIMER(&interface->stats->flush) { - if ((err = i2s_out_flush(interface->i2s_out, interface->timeout))) { + if ((err = i2s_out_flush(interface->i2s_out, interface->options->timeout))) { LOG_ERROR("i2s_out_flush"); goto error; } @@ -213,7 +213,7 @@ int leds_interface_i2s_tx(struct leds_interface_i2s *interface, const struct led } else { // async, do not wait for done WITH_STATS_TIMER(&interface->stats->start) { - if ((err = i2s_out_start(interface->i2s_out, interface->timeout))) { + if ((err = i2s_out_start(interface->i2s_out, interface->options->timeout))) { LOG_ERROR("i2s_out_start"); goto error; } @@ -221,7 +221,7 @@ int leds_interface_i2s_tx(struct leds_interface_i2s *interface, const struct led } error: - if (setup) { + if (!setup) { if (leds_interface_i2s_close(interface)) { LOG_WARN("leds_interface_i2s_close"); } From 1b8a42791cf76b6d3157492f3c7ba69f050fad36 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Tue, 28 Oct 2025 21:46:32 +0200 Subject: [PATCH 24/27] main: reset leds interface on update_leds() errors --- components/leds/interfaces/i2s.h | 1 + components/leds/interfaces/i2s/setup.c | 1 - main/leds.c | 28 ++++++++++++++++++++++++++ main/leds_state.h | 5 +++++ main/leds_task.c | 3 ++- 5 files changed, 36 insertions(+), 2 deletions(-) diff --git a/components/leds/interfaces/i2s.h b/components/leds/interfaces/i2s.h index 4047b2bc..a0ee9b02 100644 --- a/components/leds/interfaces/i2s.h +++ b/components/leds/interfaces/i2s.h @@ -74,5 +74,6 @@ int leds_interface_i2s_init(struct leds_interface_i2s *interface, const struct l int leds_interface_i2s_setup(struct leds_interface_i2s *interface); int leds_interface_i2s_tx(struct leds_interface_i2s *interface, const struct leds_color *pixels, unsigned count, const struct leds_limit *limit); int leds_interface_i2s_close(struct leds_interface_i2s *interface); +int leds_interface_i2s_reset(struct leds_interface_i2s *interface); #endif diff --git a/components/leds/interfaces/i2s/setup.c b/components/leds/interfaces/i2s/setup.c index bbac41f1..270f226d 100644 --- a/components/leds/interfaces/i2s/setup.c +++ b/components/leds/interfaces/i2s/setup.c @@ -178,5 +178,4 @@ int leds_interface_i2s_close(struct leds_interface_i2s *interface) } return err; - } diff --git a/main/leds.c b/main/leds.c index d2fead68..dd4d0b9f 100644 --- a/main/leds.c +++ b/main/leds.c @@ -172,6 +172,34 @@ int setup_leds(struct leds_state *state) return 0; } +int reset_leds(struct leds_state *state) +{ + int err; + + if (!state->leds) { + LOG_ERROR("leds%d: not initialized", state->index + 1); + return -1; + } + + if (!state->config->interface_setup) { + return 0; + } + + LOG_WARN("Reset LEDS interface"); + + if ((err = leds_interface_close(state->leds))) { + LOG_WARN("leds_interface_close"); + } + + if ((err = leds_interface_setup(state->leds))) { + // crash and restart + LOG_FATAL("leds_interface_setup"); + return err; + } + + return 0; +} + int check_leds_interface(struct leds_state *state) { if (!state->leds) { diff --git a/main/leds_state.h b/main/leds_state.h index e8c91706..5a205e3e 100644 --- a/main/leds_state.h +++ b/main/leds_state.h @@ -56,6 +56,11 @@ extern struct leds_state leds_states[LEDS_COUNT]; */ int setup_leds(struct leds_state *state); +/* + * Reset leds interface setup if necessary. + */ +int reset_leds(struct leds_state *state); + /* * Update LEDs output with given USER_ACTIVITY_LEDS_* source. */ diff --git a/main/leds_task.c b/main/leds_task.c index cac7cdb9..21ead1ff 100644 --- a/main/leds_task.c +++ b/main/leds_task.c @@ -165,7 +165,8 @@ static void leds_main(void *ctx) WITH_STATS_TIMER(&stats->update) { if (update_leds(state, update_activity)) { LOG_WARN("leds%d: update_leds", state->index + 1); - continue; + user_alert(USER_ALERT_ERROR_LEDS); + reset_leds(state); } } } From 77f5a84cd0bbac3a62f8a942be0539aecc10a2ec Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Tue, 28 Oct 2025 22:09:02 +0200 Subject: [PATCH 25/27] i2s_out: do not set I2S_OUT_EVENT_GROUP_BIT_DMA_EOF event bit on I2S_OUT_TOTAL_EOF_INT --- components/i2s_out/esp32/intr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/i2s_out/esp32/intr.c b/components/i2s_out/esp32/intr.c index 78bef578..4598bbb6 100644 --- a/components/i2s_out/esp32/intr.c +++ b/components/i2s_out/esp32/intr.c @@ -26,7 +26,7 @@ void IRAM_ATTR i2s_intr_out_total_eof_handler(struct i2s_out *i2s_out, BaseType_ LOG_ISR_DEBUG("desc=%p", eof_addr); // unblock flush() tasks - xEventGroupSetBitsFromISR(i2s_out->event_group, I2S_OUT_EVENT_GROUP_BIT_DMA_EOF | I2S_OUT_EVENT_GROUP_BIT_DMA_TOTAL_EOF, task_wokenp); + xEventGroupSetBitsFromISR(i2s_out->event_group, I2S_OUT_EVENT_GROUP_BIT_DMA_TOTAL_EOF, task_wokenp); i2s_intr_clear(i2s_out->dev, I2S_OUT_TOTAL_EOF_INT_CLR); } From 6e7b87bc7b1afd49b57086e0918a51102e723989 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Tue, 28 Oct 2025 22:15:45 +0200 Subject: [PATCH 26/27] i2s_out: workaround unreliable I2S_OUT_TOTAL_EOF_INT --- components/i2s_out/esp32/intr.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/components/i2s_out/esp32/intr.c b/components/i2s_out/esp32/intr.c index 4598bbb6..45199009 100644 --- a/components/i2s_out/esp32/intr.c +++ b/components/i2s_out/esp32/intr.c @@ -99,6 +99,14 @@ void IRAM_ATTR i2s_intr_out_eof_handler(struct i2s_out *i2s_out, BaseType_t *tas // unblock get() task xEventGroupSetBitsFromISR(i2s_out->event_group, I2S_OUT_EVENT_GROUP_BIT_DMA_EOF, task_wokenp); + // XXX: e.g. flash writes seem to drop I2S_OUT_TOTAL_EOF_INT_ST? + if (!(xEventGroupGetBitsFromISR(i2s_out->event_group) & (I2S_OUT_EVENT_GROUP_BIT_DMA_TOTAL_EOF))) { + LOG_ISR_WARN("end -> total_eof, dma_write_desc=%p dma_eof_desc=%p", eof_desc, eof_desc->owner, eof_desc->len, i2s_out->dma_write_desc, i2s_out->dma_eof_desc); + + // unblock flush() tasks + xEventGroupSetBitsFromISR(i2s_out->event_group, I2S_OUT_EVENT_GROUP_BIT_DMA_TOTAL_EOF, task_wokenp); + } + } else { LOG_ISR_DEBUG("ignore desc=%p owner=%u len=%u", eof_desc, eof_desc->owner, eof_desc->len); } From f712c61e70bae23624ec679070d56bc79f3f6f29 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Tue, 28 Oct 2025 22:30:28 +0200 Subject: [PATCH 27/27] main: bump LEDS_I2S_TIMEOUT -> 5000ms to survive config save -> flash write --- main/leds_i2s.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/main/leds_i2s.c b/main/leds_i2s.c index 820bd608..83e52e52 100644 --- a/main/leds_i2s.c +++ b/main/leds_i2s.c @@ -7,7 +7,7 @@ #include // generic timeout -#define LEDS_I2S_TIMEOUT (1000 / portTICK_RATE_MS) +#define LEDS_I2S_TIMEOUT (5000 / portTICK_RATE_MS) // do not block if pin in use, timeout immediately #define LEDS_I2S_PIN_TIMEOUT portMAX_DELAY