nxp-mcuxpresso / mcux-sdk

MCUXpresso SDK
BSD 3-Clause "New" or "Revised" License
301 stars 136 forks source link

Reading from `FIFORD48H` is not popping the FIFO when using I2S with DSP/TDM mode with `ONECHANNEL` #189

Closed carlocaione closed 2 months ago

carlocaione commented 2 months ago

Using the EVK MIMXRT685 as I2S slave configured as:

That is translated in the following configuaration:

    I2S_RxGetDefaultConfig(&s_RxConfig);

    s_RxConfig.masterSlave = kI2S_MasterSlaveNormalSlave; /* Normal Slave */
    s_RxConfig.mode = kI2S_ModeDspWsShort;                /* DSP mode, WS having one clock long pulse */
    s_RxConfig.leftJust = true;                           /* Left Justified */
    s_RxConfig.divider = 1;                               /*  As slave, divider need to set to 1 according to the spec. */
    s_RxConfig.oneChannel = true;                         /* I2S data for this channel pair is treated as a single channel */
    s_RxConfig.dataLength = 24;
    s_RxConfig.frameLength = 32;

When using this configuration in interrupt mode we get a wrong and doubled data coming from channels in the buffer.

So for example if we have 0x01 in the fist channel, 0x02 in the second channel and so on (until 0x10 in the 16th channel), the data we get in the RX buffer is:

0x01 | 0x00 | 0x00 | 0x02 | 0x00 | 0x00 | 0x02 | 0x00 | 0x00 | ...

when we would expect:

0x01 | 0x00 | 0x00 | 0x02 | 0x00 | 0x00 | 0x03 | 0x00 | 0x00 | ...

This is due to how I2S_TxHandleIRQ() is managing incoming data, and in particular the problem is here: https://github.com/nxp-mcuxpresso/mcux-sdk/blob/8f80c64d0b7dadea121cf7a42593c49cf45c1999/drivers/flexcomm/i2s/fsl_i2s.c#L1125-L1140

These are the problematic steps:

  1. On the first interrupt we are retrieving the first 24-bits data (0x01 | 0x00 | 0x00) from FIFORD popping the FIFO.
  2. On the second interrupt we are retrieving the next 24-bits data (0x02 | 0x00 | 0x00) from FIFORD48H but this is not popping the FIFO (while it should according to the documentation, it is basically behaving like FIFORD48HNOPOP).
  3. On the third interrupt we are retrieving again the same data from the FIFO.

So the problem is that with this configuration FIFORD48H is not popping from the FIFO and it is behaving like FIFORD48HNOPOP causing duplicating data in the output buffer.

To be pedantic the I2S_RxHandleIRQ() is not even entirely correct because it is neglecting the ONECHANNEL setting, according to the documentation (section 25.7.4 of UM11147):

If DATALEN specifies a number of data bits from 17 to 24 [...] If a channel pair is configured with ONECHANNEL = 1, then only the left value is transferred using the FIFOWR or FIFORD register.

So when ONECHANNEL is set we should be reading only from FIFORD and not even from FIFORD48H.

carlocaione commented 2 months ago

A possible workaround to this problem is to set:

    s_RxConfig.dataLength = 32;
    s_RxConfig.frameLength = 32;

so that we are only going to read from FIFORD but that means that I have to discard the 4-th byte in each slot in the output buffer.

0x01 | 0x00 | 0x00 | 0x?? | 0x02 | 0x00 | 0x00 | 0x?? | ...
carlocaione commented 2 months ago

The configuration of I2S in my use case is wrong (see https://github.com/nxp-mcuxpresso/mcux-sdk/issues/190#issuecomment-2055928881). Given that I'm not sure if the problem still stands. Closing this down.