I2S driver on IMXRT1170_EVK causes DMA error after latest changes related to eDMA #82470

RadekPolyend · 2024-12-03T08:52:20Z

After latest changes in eDMA and related to this I2S MCUX SAI driver eDMA error handler is invoked after first reload of buffer.

Use case:

two audio buffers in a slab
both of them allocated and written before the trigger start
after first buffer is freed it is allocated and written again than edma error occurs

This problem did not happen before the changes.

Breakpoint 3, i2s_mcux_write (dev=0x3002ba0c <__device_dts_ord_458>, mem_block=0x2000a568 <malloc_arena+2664>, size=2048)
at /home/radekp/projects/zephyrproject/zephyr/drivers/i2s/i2s_mcux_sai.c:1019
1019 {
(gdb) c
Continuing.

Breakpoint 3, i2s_mcux_write (dev=0x3002ba0c <__device_dts_ord_458>, mem_block=0x2000ad68 <malloc_arena+4712>, size=2048)
at /home/radekp/projects/zephyrproject/zephyr/drivers/i2s/i2s_mcux_sai.c:1019
1019 {
(gdb) c
Continuing.

Breakpoint 1, i2s_dma_tx_callback (dma_dev=0x3002b8b8 <__device_dts_ord_455>, arg=0x3002ba0c <__device_dts_ord_458>, channel=31, status=0)
at /home/radekp/projects/zephyrproject/zephyr/drivers/i2s/i2s_mcux_sai.c:261
261 {
(gdb) c
Continuing.

Breakpoint 3, i2s_mcux_write (dev=0x3002ba0c <__device_dts_ord_458>, mem_block=0x2000a568 <malloc_arena+2664>, size=2048)
at /home/radekp/projects/zephyrproject/zephyr/drivers/i2s/i2s_mcux_sai.c:1019
1019 {
(gdb) c
Continuing.

Breakpoint 1, i2s_dma_tx_callback (dma_dev=0x3002b8b8 <__device_dts_ord_455>, arg=0x3002ba0c <__device_dts_ord_458>, channel=31, status=0)
at /home/radekp/projects/zephyrproject/zephyr/drivers/i2s/i2s_mcux_sai.c:261
261 {
(gdb) c
Continuing.

Breakpoint 2, dma_mcux_edma_error_irq_handler (dev=0x3002b8b8 <__device_dts_ord_455>) at /home/radekp/projects/zephyrproject/zephyr/drivers/dma/dma_mcux_edma.c:258

(gdb) (gdb) p/x ((struct dma_mcux_edma_data*)dev->data)->data_cb[31]
$6 = {transferConfig = {srcAddr = 0x2000a568, destAddr = 0x40404020, srcTransferSize = 0x2, destTransferSize = 0x2, srcOffset = 0x4, destOffset = 0x0, minorLoopBytes = 0x4,
majorLoopCounts = 0x200}, edma_handle = {callback = 0x30027439, userData = 0x20002fe4, base = 0x40070000, tcdPool = 0x0, channel = 0x1f, header = 0x0, tail = 0x0, tcdUsed = 0x0,
tcdSize = 0x0, flags = 0x0}, dev = 0x3002b8b8, user_data = 0x3002ba0c, dma_callback = 0x30012d31, transfer_settings = {source_data_size = 0x4, dest_data_size = 0x4,
source_burst_length = 0x4, dest_burst_length = 0x4, direction = 0x1, transfer_type = 0x2, valid = 0x1, cyclic = 0x1, write_idx = 0x3, empty_tcds = 0x3}, busy = 0x1}

The situation does not happen when I revert 7145295. but the sound is glitchy.

The text was updated successfully, but these errors were encountered:

dleach02 · 2024-12-03T17:18:20Z

@Raymond0225, can you please look into this as it was bisected back to you change. Thanks

Raymond0225 · 2024-12-03T21:18:26Z

@Raymond0225, can you please look into this as it was bisected back to you change. Thanks

sure, I am looking into this issue. Will get back later.

Raymond0225 · 2024-12-05T23:53:09Z

After latest changes in eDMA and related to this I2S MCUX SAI driver eDMA error handler is invoked after first reload of buffer.

Use case:

two audio buffers in a slab

both of them allocated and written before the trigger start

after first buffer is freed it is allocated and written again than edma error occurs

This problem did not happen before the changes.

Breakpoint 3, i2s_mcux_write (dev=0x3002ba0c <__device_dts_ord_458>, mem_block=0x2000a568 <malloc_arena+2664>, size=2048) at /home/radekp/projects/zephyrproject/zephyr/drivers/i2s/i2s_mcux_sai.c:1019 1019 { (gdb) c Continuing.

Breakpoint 3, i2s_mcux_write (dev=0x3002ba0c <__device_dts_ord_458>, mem_block=0x2000ad68 <malloc_arena+4712>, size=2048) at /home/radekp/projects/zephyrproject/zephyr/drivers/i2s/i2s_mcux_sai.c:1019 1019 { (gdb) c Continuing.

Breakpoint 1, i2s_dma_tx_callback (dma_dev=0x3002b8b8 <__device_dts_ord_455>, arg=0x3002ba0c <__device_dts_ord_458>, channel=31, status=0) at /home/radekp/projects/zephyrproject/zephyr/drivers/i2s/i2s_mcux_sai.c:261 261 { (gdb) c Continuing.

Breakpoint 3, i2s_mcux_write (dev=0x3002ba0c <__device_dts_ord_458>, mem_block=0x2000a568 <malloc_arena+2664>, size=2048) at /home/radekp/projects/zephyrproject/zephyr/drivers/i2s/i2s_mcux_sai.c:1019 1019 { (gdb) c Continuing.

Breakpoint 1, i2s_dma_tx_callback (dma_dev=0x3002b8b8 <__device_dts_ord_455>, arg=0x3002ba0c <__device_dts_ord_458>, channel=31, status=0) at /home/radekp/projects/zephyrproject/zephyr/drivers/i2s/i2s_mcux_sai.c:261 261 { (gdb) c Continuing.

Breakpoint 2, dma_mcux_edma_error_irq_handler (dev=0x3002b8b8 <__device_dts_ord_455>) at /home/radekp/projects/zephyrproject/zephyr/drivers/dma/dma_mcux_edma.c:258

(gdb) (gdb) p/x ((struct dma_mcux_edma_data*)dev->data)->data_cb[31] $6 = {transferConfig = {srcAddr = 0x2000a568, destAddr = 0x40404020, srcTransferSize = 0x2, destTransferSize = 0x2, srcOffset = 0x4, destOffset = 0x0, minorLoopBytes = 0x4, majorLoopCounts = 0x200}, edma_handle = {callback = 0x30027439, userData = 0x20002fe4, base = 0x40070000, tcdPool = 0x0, channel = 0x1f, header = 0x0, tail = 0x0, tcdUsed = 0x0, tcdSize = 0x0, flags = 0x0}, dev = 0x3002b8b8, user_data = 0x3002ba0c, dma_callback = 0x30012d31, transfer_settings = {source_data_size = 0x4, dest_data_size = 0x4, source_burst_length = 0x4, dest_burst_length = 0x4, direction = 0x1, transfer_type = 0x2, valid = 0x1, cyclic = 0x1, write_idx = 0x3, empty_tcds = 0x3}, busy = 0x1}

The situation does not happen when I revert 7145295. but the sound is glitchy.

thanks for your information.
I set up a test case based on your description and try to reproduce this issue. What I found is that when TX buffer is set to 2, TX procedure will be stopped because of no more TX blocks in i2s_dma_tx_callback function.
so I prepared a patch to fixed it.
2 TX buffers works fine with this patch. I share the patch and my test file as attached.

The test file is based on test_i2s_speed.c; I added another test case in this file: in this test case, I started a RX thread for reading while in the test main doing TX. To run the test added, #define I2S_SPEED_TEST_RUN_SPECIAL 1
one thing you need to know that: because of only 2 TX buffers, DMA transfer is so fast so there will be some short period of time without real data on the I2S, RX side may receive some dummy bits.
Let me know if it helps or not.
i2s_patch.patch

Raymond0225 · 2024-12-05T23:53:43Z

speed_test.txt
.c file can't be uploaded, I rename it as *.txt

RadekPolyend · 2024-12-06T11:45:39Z

This patch does not work for me. The procedure should not be stopped because after first buffer is sent over dma the dma callback is invoked and buffer is returned the the pool. In the meantime when second buffer is sent a thread is unlocked on pool, takes the buffer and write it again. For me the buffer is 128b for 44,1k sampling therefore I have more than enough 3ms to deliver the second buffer. You can see in my debug logs that after first dma callback a write is done so the next buffer is there but after second dma callback the edma error occurs.

Raymond0225 · 2024-12-06T17:27:14Z

I paste reference code here, it works fine on my side. There is a standalone RX thread. If you can provide you code, we can help to have a review and try.
Other things you need pay attention is:
a) Enable deferred logging mode: CONFIG_LOG_MODE_DEFERRED=y
b) Check the DMA channel used for RX and TX and ensure they are not used by other peripherals like UART/SPI

#define MY_STACK_SIZE 1024
#define MY_PRIORITY 0

K_THREAD_STACK_DEFINE(my_stack_area, MY_STACK_SIZE);
struct k_thread my_thread_data;
void my_entry_point(void *dev, void * p1, void *p2);

uint32_t g_for_comparing[SAMPLE_NO];

static void fill_special(int16_t *tx_block, int att)
{

memset(tx_block,att,SAMPLE_NO<<2);

}

static int verify_special(int16_t *buf, int att)
{
memset(g_for_comparing,att,sizeof(g_for_comparing));
return memcmp(g_for_comparing,buf,sizeof(g_for_comparing));

}

void my_entry_point(void dev, void * p1, void p2)
{
void rx_data_p;
int rx_idx = (int*)p1;
uint32_t rx_size;
int att = 0;
int ret;
struct k_sem* sem = (struct k_sem*)p2;
int first_unmatch = -1;
int unmatch_count = 0;

/* Start reception */
ret = i2s_trigger(dev_i2s_rx, I2S_DIR_RX, I2S_TRIGGER_START);
zassert_equal(ret, 0, "RX START trigger failed");

/* RX is ready */
k_sem_give(sem);


while(1)
{
	/* Read and verify*/
	ret = i2s_read(dev, &rx_data_p, &rx_size);
	if (ret)
	{
		/* All but one data block read, stop reception */
		ret = i2s_trigger(dev_i2s_rx, I2S_DIR_RX, I2S_TRIGGER_STOP);
		zassert_equal(ret, 0, "RX STOP trigger failed");
		// TC_PRINT("Trigger RX stop");
		break;
	}
	zassert_equal(rx_size, BLOCK_SIZE);
	if (verify_special(rx_data_p,att++))
	{
		unmatch_count++;
		if (unmatch_count == 1)
		{
			first_unmatch = att - 1;
		}
	}
	k_mem_slab_free(&rx_0_mem_slab, rx_data_p);
	(*rx_idx)++;
}

TC_PRINT("RX thread terminiated, umatch(%d/%d)\r\n",first_unmatch,unmatch_count);

}

ZTEST(drivers_i2s_speed_plus, test_i2s_transfer_special)
{

if (IS_ENABLED(CONFIG_I2S_TEST_USE_I2S_DIR_BOTH)) {
	TC_PRINT("RX/TX transfer requires use of I2S_DIR_BOTH.\n");
	ztest_test_skip();
	return;
}

void *tx_p;
int tx_idx = 0;
int rx_idx = 0;
int ret;
struct k_sem sem_init;

k_sem_init(&sem_init,0,1);

k_tid_t my_tid = k_thread_create(&my_thread_data, my_stack_area,
			K_THREAD_STACK_SIZEOF(my_stack_area),
			my_entry_point,
			(void*)dev_i2s_rx, &rx_idx, &sem_init,
			MY_PRIORITY, 0, K_NO_WAIT);

/* Prepare TX data blocks */
for (tx_idx = 0; tx_idx < MIN(NUM_BLOCKS,4); tx_idx++) {
	ret = k_mem_slab_alloc(&tx_0_mem_slab, &tx_p,
			       K_FOREVER);
	zassert_equal(ret, 0);
	fill_special((uint16_t *)tx_p, tx_idx);

	ret = i2s_write(dev_i2s_tx, tx_p, BLOCK_SIZE);
	zassert_equal(ret, 0);
}

/* Wait for RX is ready*/
k_sem_take(&sem_init,K_FOREVER);

/* Start transmission */
ret = i2s_trigger(dev_i2s_tx, I2S_DIR_TX, I2S_TRIGGER_START);
zassert_equal(ret, 0, "TX START trigger failed");

for (; tx_idx < 100; tx_idx++) {
	/* Wait for buffer available */
	void* tx_data_p;

	/* prepare for writing*/
	ret = k_mem_slab_alloc(&tx_0_mem_slab, &tx_data_p,
				K_FOREVER);
	zassert_equal(ret, 0);
	fill_special((uint16_t *)tx_data_p, tx_idx);

	/* Add random delay between 0-10ms */
	// k_msleep(rand()%10);
	ret = i2s_write(dev_i2s_tx, tx_data_p, BLOCK_SIZE);
	zassert_equal(ret, 0);

}

/* All data written, flush TX queue and stop the transmission */
ret = i2s_trigger(dev_i2s_tx, I2S_DIR_TX, I2S_TRIGGER_DRAIN);
zassert_equal(ret, 0, "TX DRAIN trigger failed");

k_thread_join(my_tid,K_FOREVER);
TC_PRINT("%d TX blocks sent\n", tx_idx);
TC_PRINT("%d RX blocks received\n", rx_idx);

}

RadekPolyend · 2024-12-11T07:29:20Z

I have CONFIG_LOG_MODE_DEFERRED=y, DMA channels are used by I2S only. Regarding my code there is nothing special about it as I just wrote you and API does not really make it possible to use differently than it is supposed to be. I will try to run you test and let you know but to be honest if you use API the way it should be used and if it's working on older kernel than there must be some loophole which is visible in some circumstances when non isolated test is performed.

DerekSnell · 2024-12-12T12:56:27Z

Hi @RadekPolyend ,
There is a bug fix for the eDMA driver at #82890. Does that help with this issue?

Thanks

RadekPolyend · 2024-12-12T13:45:11Z

@DerekSnell
Unfortunately it didn't help.
@Raymond0225
I have checked tcdpool for the channel I am using and what I can see is that tcds are not properly linked into circular list after third write and that's possibly the cause of the error

(gdb) p/x dma_tcdpool0[31][0]
$12 = {SADDR = 0x2000a568, SOFF = 0x4, ATTR = 0x202, NBYTES = 0x4, SLAST = 0x0, DADDR = 0x40404020, DOFF = 0x0, CITER = 0x200, DLAST_SGA = 0x20000fa0, CSR = 0x12, BITER = 0x200}
(gdb) p/x dma_tcdpool0[31][1]
$13 = {SADDR = 0x2000ad68, SOFF = 0x4, ATTR = 0x202, NBYTES = 0x4, SLAST = 0x0, DADDR = 0x40404020, DOFF = 0x0, CITER = 0x200, DLAST_SGA = 0x20000fc0, CSR = 0x1a, BITER = 0x200}
(gdb) p/x dma_tcdpool0[31][2]
$14 = {SADDR = 0x2000a568, SOFF = 0x4, ATTR = 0x202, NBYTES = 0x4, SLAST = 0x0, DADDR = 0x40404020, DOFF = 0xbfc, CITER = 0x200, DLAST_SGA = 0x15, CSR = 0x1a, BITER = 0x200}
(gdb) p/x dma_tcdpool0[31][3]
$15 = {SADDR = 0x6, SOFF = 0x118, ATTR = 0x3003, NBYTES = 0x30030250, SLAST = 0x30030260, DADDR = 0xe, DOFF = 0x0, CITER = 0x0, DLAST_SGA = 0x3101, CSR = 0xbfc, BITER = 0x2000}

you can see the third tcd has the source address as it is in the first one (I have two buffers only in the pool) but DLAST_SGA is not linked properly to the first tcd and if I am not mistaken that's how it should be. to create circularity.

RadekPolyend · 2024-12-12T14:00:27Z

So actually when I started the code under debugging it went through the beginning and went further but the sound is still glitchy. Of course there are different things going on the board but it should not influence the behaviour of dma. It looks like there is some race condition going on which is exposed but the fact that the SW does not run in isolated test. When the SW is succesfully started the tcdpool looks like it should be but the sound is horrible

(gdb) p/x dma_tcdpool0[31][0]
$34 = {SADDR = 0x2000a568, SOFF = 0x4, ATTR = 0x202, NBYTES = 0x4, SLAST = 0x0, DADDR = 0x40404020, DOFF = 0x0, CITER = 0x200, DLAST_SGA = 0x20000fa0, CSR = 0x1a, BITER = 0x200}
(gdb) p/x dma_tcdpool0[31][1]
$35 = {SADDR = 0x2000ad68, SOFF = 0x4, ATTR = 0x202, NBYTES = 0x4, SLAST = 0x0, DADDR = 0x40404020, DOFF = 0x0, CITER = 0x200, DLAST_SGA = 0x20000fc0, CSR = 0x1a, BITER = 0x200}
(gdb) p/x dma_tcdpool0[31][2]
$36 = {SADDR = 0x2000a568, SOFF = 0x4, ATTR = 0x202, NBYTES = 0x4, SLAST = 0x0, DADDR = 0x40404020, DOFF = 0x0, CITER = 0x200, DLAST_SGA = 0x20000fe0, CSR = 0x1a, BITER = 0x200}
(gdb) p/x dma_tcdpool0[31][3]
$37 = {SADDR = 0x2000ad68, SOFF = 0x4, ATTR = 0x202, NBYTES = 0x4, SLAST = 0x0, DADDR = 0x40404020, DOFF = 0x0, CITER = 0x200, DLAST_SGA = 0x20000f80, CSR = 0x1a, BITER = 0x200}

buffers delivered to write are as they should, exchangeably

Breakpoint 3, i2s_write (size=2048, mem_block=0x2000ad68 <malloc_arena+4712>, dev=0x3002ba0c <__device_dts_ord_458>)
at /home/radekp/projects/zephyrproject/zephyr/include/zephyr/drivers/i2s.h:482
482 const struct i2s_driver_api *api =
(gdb) p curr_tick
$38 = 945
(gdb) c
Continuing.

Breakpoint 2, i2s_dma_tx_callback (dma_dev=0x3002b8b8 <__device_dts_ord_455>, arg=0x3002ba0c <__device_dts_ord_458>, channel=31, status=0)
at /home/radekp/projects/zephyrproject/zephyr/drivers/i2s/i2s_mcux_sai.c:261
261 {
(gdb) c
Continuing.

Breakpoint 3, i2s_write (size=2048, mem_block=0x2000a568 <malloc_arena+2664>, dev=0x3002ba0c <__device_dts_ord_458>)
at /home/radekp/projects/zephyrproject/zephyr/include/zephyr/drivers/i2s.h:482
482 const struct i2s_driver_api *api =
(gdb) p curr_tick
$39 = 962
(gdb) c
Continuing.

Breakpoint 2, i2s_dma_tx_callback (dma_dev=0x3002b8b8 <__device_dts_ord_455>, arg=0x3002ba0c <__device_dts_ord_458>, channel=31, status=0)
at /home/radekp/projects/zephyrproject/zephyr/drivers/i2s/i2s_mcux_sai.c:261
261 {
(gdb) c
Continuing.

Breakpoint 3, i2s_write (size=2048, mem_block=0x2000ad68 <malloc_arena+4712>, dev=0x3002ba0c <__device_dts_ord_458>)
at /home/radekp/projects/zephyrproject/zephyr/include/zephyr/drivers/i2s.h:482
482 const struct i2s_driver_api *api =

the sound is horrible though

RadekPolyend · 2024-12-12T14:30:40Z

You can try to alter the test a bit by giving it some load e.g. some FPU calculations before every write which also increase the IRQ context than you might actually experience on-site what I do.

RadekPolyend · 2024-12-12T15:00:18Z

I also don't understand what's the point in actually stopping the dma in the dma handler while using cyclic mode. As name suggests it does not stop but go constantly and only buffers are read or updated. In other case this is no different than actually reloading TCD which causes issues for interfaces like SAI due to very strict time constraints. For audio there should be two buffers linked into loop SG and application should only be informed which buffer is "free" at the moment so that it can be filled. No DMA stopping, no dma reloading, nothing.

Raymond0225 · 2024-12-12T17:23:14Z

I also don't understand what's the point in actually stopping the dma in the dma handler while using cyclic mode. As name suggests it does not stop but go constantly and only buffers are read or updated. In other case this is no different than actually reloading TCD which causes issues for interfaces like SAI due to very strict time constraints. For audio there should be two buffers linked into loop SG and application should only be informed which buffer is "free" at the moment so that it can be filled. No DMA stopping, no dma reloading, nothing.
To my understanding, API of Zephyr I2S is designed not to communicate with the caller by notification/interrupt.
for TX, once a slab memory is configured, what the caller need to do is l "memory allocation " and "write"
ret = k_mem_slab_alloc(&tx_0_mem_slab, &tx_data_p, K_FOREVER); // if there is a block of memory available, you will get it immediately, if not, if it blocks until there is one released by the I2S driver.
ret = i2s_write(dev_i2s_tx, tx_data_p, BLOCK_SIZE); //timeout for write is define in i2s configuration

DMA cyclic mode does not introduce more time constraints. if there is no block for transfer, it will stop automatically. Time constraints come from application, application need make sure there is always blocks in the TX queue otherwise a glitch happens.

I noticed that there is a FIFO configuration bug in the i2s driver for TX, could you have a try on it (one line is added):
memcpy(&dev_data->tx.cfg, i2s_cfg, sizeof(struct i2s_config));
LOG_DBG("tx slab free_list = 0x%x", (uint32_t)i2s_cfg->mem_slab->free_list);
LOG_DBG("tx slab num_blocks = %d", (uint32_t)i2s_cfg->mem_slab->info.num_blocks);
LOG_DBG("tx slab block_size = %d", (uint32_t)i2s_cfg->mem_slab->info.block_size);
LOG_DBG("tx slab buffer = 0x%x", (uint32_t)i2s_cfg->mem_slab->buffer);

	config.fifo.fifoWatermark = (uint32_t)FSL_FEATURE_SAI_FIFO_COUNTn(base) - 1;  /* this line is added */
	/* set bit clock divider */
	SAI_TxSetConfig(base, &config);

RadekPolyend · 2024-12-12T19:27:32Z

fifoWatermark is set in SAI_TxSetConfig so no need for that. As for the cyclic you must take into consideration that for 44k1 you need to have new buffer available in around 20us for next sample to be present. If you reload dma in IRQ you may be late if anything in the system blocked IRQs. Therefore dma transfer for SAI must be cyclic in the way it is never reloaded and application is only informed about which buffer can be filled at the moment. Application needs to deliver data on time, if it doesn't the sound will break but the transfers are still ongoing, they just never stop, dma either switched the buffers on its own or you have double buffer and set half dma interrupt.

Raymond0225 · 2024-12-12T22:10:02Z

I know fifoWatermark is set in side SAI_TxSetConfig , but it is set based on the configuration caller input. can you have a look to the register "base->TCR1" double confirm it is not 0? in myside, it is 0.
what you said about the cyclic way: we have to do reload because the buffer input by Zephyr I2S API "I2s_write" is dynamic, there is not guaranteed that buffer address is fixed. So every time when a new buffer is input, DMA reload happens.
if you don't use Zephyr API, use our SDK APIs, sure you can use this "modulo" mode.
you can refer to RT1170 RM 6.4.4.3 Using the modulo feature for more information.

RadekPolyend · 2024-12-13T14:49:35Z

Actually when cyclic DMA is used fifo and the watermark logic does not have any influence. To be honest I was able to make a cyclic dma with much more simpler code than yours. All you need to do is in scatter/gather config code to link last provided block config TCD to the first one e.g. like this

--- a/drivers/dma/dma_mcux_edma.c
+++ b/drivers/dma/dma_mcux_edma.c
@@ -359,8 +359,10 @@ static int dma_mcux_edma_configure(const struct device *dev, uint32_t channel,
EDMA_EnableChannelInterrupts(DEV_BASE(dev), hw_channel, kEDMA_ErrorInterruptEnable);

        if (block_config->source_gather_en || block_config->dest_scatter_en) {
+               int i = 0;
                EDMA_InstallTCDMemory(p_handle, tcdpool[channel], CONFIG_DMA_TCD_QUEUE_SIZE);
                while (block_config != NULL) {
+                       i++;
                        EDMA_PrepareTransfer(
                                &(data->transferConfig),
                                (void *)block_config->source_address,
@@ -378,6 +380,10 @@ static int dma_mcux_edma_configure(const struct device *dev, uint32_t channel,
                        }
                        block_config = block_config->next_block;
                }
+               if(config->cyclic)
+               {
+                       EDMA_TcdSetTransferConfig(&tcdpool[channel][i-1], &(data->transferConfig), &tcdpool[channel][0]);
+               }

this will link the last TCD with first one and remove DREQ flag so they will be cyclic from now on. The rest is to manage which buffer is currently processed so that others can be filled but this must be done in the peripherial driver based on dma callbacks. I've just tested this solution and it works like a charm without any artifacts in the sound.

Raymond0225 · 2024-12-13T22:23:32Z

"Actually when cyclic DMA is used fifo and the watermark logic does not have any influence"
if you could have a look at the reload procedure of eDMA, you will know there may a very short time period we have to pause the eDMA, in this period, if TX FIFO is empty at any time, dump data will be sent out by I2S.
If we set watermark higher we can ensure there is still enough valid data when eDMA is paused and they can not be consumed completely before eDMA resume.

RadekPolyend · 2024-12-14T12:16:04Z

"Actually when cyclic DMA is used fifo and the watermark logic does not have any influence" if you could have a look at the reload procedure of eDMA, you will know there may a very short time period we have to pause the eDMA, in this period, if TX FIFO is empty at any time, dump data will be sent out by I2S. If we set watermark higher we can ensure there is still enough valid data when eDMA is paused and they can not be consumed completely before eDMA resume.

That's exactly what I mean and that's why the current implementation is not actually cyclic. Moreover it's race condition prone as it can be seen on my case when sth more is going on in the system than only single loadless application. Reloading cyclic dma is completely against its name not mentioning that it is unneseccarily complicated and makes it hard to actually find and eliminate the race condition. FIFO might give you time to avoid race condition but what if sample frequency is set let's say to 192kHz or even higher which is way faster faster than 44k1 ? Are you sure even with FIFO it can be handled ? You are relying on delays but they can as well work against your favour.

Raymond0225 · 2024-12-16T17:10:12Z

1.It is cyclic, just not what you understand. If you have a better idea how to implement the cycle with "dynamic buffer" supported, please let me know. here "dynamic buffer" means in i2s_write interface, no constrains on the value of parameter "mem_block" .
2.Based on current code and implementation, I can't reproduce the issue you mentioned "single loadless application" issue.
we need your help to reproduce this issue.

Raymond0225 · 2024-12-16T17:13:24Z

Actually when cyclic DMA is used fifo and the watermark logic does not have any influence. To be honest I was able to make a cyclic dma with much more simpler code than yours. All you need to do is in scatter/gather config code to link last provided block config TCD to the first one e.g. like this

--- a/drivers/dma/dma_mcux_edma.c +++ b/drivers/dma/dma_mcux_edma.c @@ -359,8 +359,10 @@ static int dma_mcux_edma_configure(const struct device *dev, uint32_t channel, EDMA_EnableChannelInterrupts(DEV_BASE(dev), hw_channel, kEDMA_ErrorInterruptEnable);
        if (block_config->source_gather_en || block_config->dest_scatter_en) {
+               int i = 0;
                EDMA_InstallTCDMemory(p_handle, tcdpool[channel], CONFIG_DMA_TCD_QUEUE_SIZE);
                while (block_config != NULL) {
+                       i++;
                        EDMA_PrepareTransfer(
                                &(data->transferConfig),
                                (void *)block_config->source_address,
@@ -378,6 +380,10 @@ static int dma_mcux_edma_configure(const struct device *dev, uint32_t channel,
                        }
                        block_config = block_config->next_block;
                }
+               if(config->cyclic)
+               {
+                       EDMA_TcdSetTransferConfig(&tcdpool[channel][i-1], &(data->transferConfig), &tcdpool[channel][0]);
+               }
this will link the last TCD with first one and remove DREQ flag so they will be cyclic from now on. The rest is to manage which buffer is currently processed so that others can be filled but this must be done in the peripherial driver based on dma callbacks. I've just tested this solution and it works like a charm without any artifacts in the sound.

this solution has constrained that user buffer(as source addr or destination addr) can't be changed once configured. do you have idea how to work with i2s_write and i2s_read interface with this solution?

RadekPolyend · 2024-12-16T17:34:04Z

this solution has constrained that user buffer(as source addr or destination addr) can't be changed once configured. do you have idea how to work with i2s_write and i2s_read interface with this solution?

Yes and that's exactly what cyclic should be about. You don't change anything when it's setup. Handling it is fairly easy you commit let's say two buffers into the queue and start dma. When first dma irq occurs you take the first buffer from the queue (which was sent) and you return it to the pool. In the meantime application is unlocked so it can fill in the first buffer and provide it to the queue again. Than second irq is served so you take the second buffer from the queue and return it to the pool and the cycle continues on and on.

app - fill in 1 and 2 buffer and put it to queue using i2s_write
pool -> 1 -> queue
pool -> 2 -> queue
dma start - 1 buffer is served first
dma irq - 1 buffer was served, 2 is ongoing
queue -> 1 -> pool
app - unlocked by the pool fill in the 1 buffer again and send it using i2s_write
pool -> 1 -> queue
dma irq - 2 buffer was served, 1 is ongoing
queue -> 2 -> pool
etc.

Raymond0225 · 2024-12-16T18:06:29Z

How to sync between DMA transfer and user input?

How to pause DMA if application "write" a buffer and send it later?
Just image there are multi buffers case. How does application know which buffer is the next right one to be filled? Zephyr I2S use k_mem_slab for memory allocation and free, it never says allocate memory block a FIFO way. How about if in DMA configuration we have a fixed buffer list as : A->B->C->D, but allocation list on application might be: A->B->D->C ?
DMA not only used by I2S but also used by SPI, UART peripherals. for example for UART RX, buffer is input by the caller, a fixed buffer list definitely is not possible.

RadekPolyend · 2024-12-17T08:20:53Z

How to sync between DMA transfer and user input?

How to pause DMA if application "write" a buffer and send it later?

Just image there are multi buffers case. How does application know which buffer is the next right one to be filled? Zephyr I2S use k_mem_slab for memory allocation and free, it never says allocate memory block a FIFO way. How about if in DMA configuration we have a fixed buffer list as : A->B->C->D, but allocation list on application might be: A->B->D->C ?

DMA not only used by I2S but also used by SPI, UART peripherals. for example for UART RX, buffer is input by the caller, a fixed buffer list definitely is not possible.

Ad.1 You can pause cyclic DMA as any other DMA by simply disable the dma request
Ad.2 Well multibuffers case should not use slabs or do no use cyclic DMA
Ad.3 If interface is not prepared to use cyclic DMA it should not use it

Ok I think I've pretty much made my point which is that current cyclic implementation is not in fact cyclic because it does not work without reload. Instead of reloading the buffers in the peripheral driver the dma driver does it for it and that's the fact. I understand that you can't reproduce the problem but I can and somebody in the future will and you most probably will circle back to that. If you are not willing to do sth about it that's fine with me but you know the problem is somewhere there...

Raymond0225 · 2024-12-17T16:40:17Z

How to sync between DMA transfer and user input?

How to pause DMA if application "write" a buffer and send it later?

Just image there are multi buffers case. How does application know which buffer is the next right one to be filled? Zephyr I2S use k_mem_slab for memory allocation and free, it never says allocate memory block a FIFO way. How about if in DMA configuration we have a fixed buffer list as : A->B->C->D, but allocation list on application might be: A->B->D->C ?

DMA not only used by I2S but also used by SPI, UART peripherals. for example for UART RX, buffer is input by the caller, a fixed buffer list definitely is not possible.

Ad.1 You can pause cyclic DMA as any other DMA by simply disable the dma request Ad.2 Well multibuffers case should not use slabs or do no use cyclic DMA Ad.3 If interface is not prepared to use cyclic DMA it should not use it

Ok I think I've pretty much made my point which is that current cyclic implementation is not in fact cyclic because it does not work without reload. Instead of reloading the buffers in the peripheral driver the dma driver does it for it and that's the fact. I understand that you can't reproduce the problem but I can and somebody in the future will and you most probably will circle back to that. If you are not willing to do sth about it that's fine with me but you know the problem is somewhere there...

"You can pause cyclic DMA as any other DMA by simply disable the dma request"
the problem is how do you know when to pause DMA? do you have any idea?
Go back to the issue itself. I can't reproduce this issue so far; can you help to give some example code how to reproduce this issue based on current cyclic mechanism? Let's just put down whatever it need reload or not , focus on how to reproduce the issue.

github-actions · 2025-04-23T00:37:22Z

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

RadekPolyend added the bug The issue is a bug, or the PR is fixing a bug label Dec 3, 2024

RadekPolyend mentioned this issue Dec 3, 2024

eDMA loop sg mode and I2S LPUART driver #80584

Merged

nordicjm added the platform: NXP NXP label Dec 3, 2024

zephyrbot assigned mmahadevan108 and dleach02 Dec 3, 2024

kartben added the priority: low Low impact/importance bug label Dec 3, 2024

DerekSnell added the area: I2S label Dec 3, 2024

github-actions bot added the Stale label Feb 16, 2025

dleach02 removed the Stale label Feb 17, 2025

zephyrproject-rtos deleted a comment from github-actions bot Feb 21, 2025

github-actions bot added the Stale label Apr 23, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

I2S driver on IMXRT1170_EVK causes DMA error after latest changes related to eDMA #82470

I2S driver on IMXRT1170_EVK causes DMA error after latest changes related to eDMA #82470

RadekPolyend commented Dec 3, 2024 •

edited

Loading

dleach02 commented Dec 3, 2024

Raymond0225 commented Dec 3, 2024

Raymond0225 commented Dec 5, 2024

Raymond0225 commented Dec 5, 2024 •

edited

Loading

RadekPolyend commented Dec 6, 2024 •

edited

Loading

Raymond0225 commented Dec 6, 2024

RadekPolyend commented Dec 11, 2024

DerekSnell commented Dec 12, 2024

RadekPolyend commented Dec 12, 2024

RadekPolyend commented Dec 12, 2024

RadekPolyend commented Dec 12, 2024

RadekPolyend commented Dec 12, 2024

Raymond0225 commented Dec 12, 2024

RadekPolyend commented Dec 12, 2024

Raymond0225 commented Dec 12, 2024

RadekPolyend commented Dec 13, 2024 •

edited

Loading

Raymond0225 commented Dec 13, 2024

RadekPolyend commented Dec 14, 2024 •

edited

Loading

Raymond0225 commented Dec 16, 2024

Raymond0225 commented Dec 16, 2024

RadekPolyend commented Dec 16, 2024 •

edited

Loading

Raymond0225 commented Dec 16, 2024

RadekPolyend commented Dec 17, 2024 •

edited

Loading

Raymond0225 commented Dec 17, 2024

github-actions bot commented Apr 23, 2025

I2S driver on IMXRT1170_EVK causes DMA error after latest changes related to eDMA #82470

I2S driver on IMXRT1170_EVK causes DMA error after latest changes related to eDMA #82470

Comments

RadekPolyend commented Dec 3, 2024 • edited Loading

dleach02 commented Dec 3, 2024

Raymond0225 commented Dec 3, 2024

Raymond0225 commented Dec 5, 2024

Raymond0225 commented Dec 5, 2024 • edited Loading

RadekPolyend commented Dec 6, 2024 • edited Loading

Raymond0225 commented Dec 6, 2024

RadekPolyend commented Dec 11, 2024

DerekSnell commented Dec 12, 2024

RadekPolyend commented Dec 12, 2024

RadekPolyend commented Dec 12, 2024

RadekPolyend commented Dec 12, 2024

RadekPolyend commented Dec 12, 2024

Raymond0225 commented Dec 12, 2024

RadekPolyend commented Dec 12, 2024

Raymond0225 commented Dec 12, 2024

RadekPolyend commented Dec 13, 2024 • edited Loading

Raymond0225 commented Dec 13, 2024

RadekPolyend commented Dec 14, 2024 • edited Loading

Raymond0225 commented Dec 16, 2024

Raymond0225 commented Dec 16, 2024

RadekPolyend commented Dec 16, 2024 • edited Loading

Raymond0225 commented Dec 16, 2024

RadekPolyend commented Dec 17, 2024 • edited Loading

Raymond0225 commented Dec 17, 2024

github-actions bot commented Apr 23, 2025

RadekPolyend commented Dec 3, 2024 •

edited

Loading

Raymond0225 commented Dec 5, 2024 •

edited

Loading

RadekPolyend commented Dec 6, 2024 •

edited

Loading

RadekPolyend commented Dec 13, 2024 •

edited

Loading

RadekPolyend commented Dec 14, 2024 •

edited

Loading

RadekPolyend commented Dec 16, 2024 •

edited

Loading

RadekPolyend commented Dec 17, 2024 •

edited

Loading