I2S TDM8 setting incorrect BCK/WS frequency (IDFGH-6710)

ftab commented 2 years ago

Environment

Development Kit: ESP32-S3-DevKitM-1, ESP32-C3-DevKitM-1
Kit version: v1.0
IDF version (run git describe --tags to find it): v4.4, also tried master at v5.0-dev-1426-ge899edd793
Build System: idf.py
Compiler version (run xtensa-esp32-elf-gcc --version to find it): xtensa-esp32-elf-gcc (crosstool-NG esp-2021r2) 8.4.0
Operating System: Linux
Using an IDE?: No
Power Supply: USB

Problem Description

An incorrect BCK frequency causes TDM to not function correctly

Expected Behavior

BCK should run at sample rate bits per channel number of channels

In case of 8 channels and 32 bit channel width, it should match MCK which is set to sample rate * 256 by default

WS should be a 50% duty cycle in the case of standard I2S format

Actual Behavior

BCK runs at half the speed of what it should be for the given sample rate and bit width (actual = 5,644,800 Hz, expected = 11,289,600 Hz)

WS line stays high (apparently?), possibly because the ws_width is 128 (the entire frame cycle when BCK is running at this rate)

i2s_calculate_common_clock appears to be calculating the correct BCK, so the issue happens after it's passed to a lower level.

Tested on both ESP32-S3 and C3 DevKitM, observed using oscilloscope on the clock lines

Steps to reproduce

Clone the linked project
Set target to a TDM compatible target
build flash and monitor

Code to reproduce this issue

https://github.com/ftab/tdm_example.git

Based on the i2s_basic example, modified for 8-channel 24-bit TDM

ftab commented 2 years ago

I tried with 4 channels, and BCLK / WS look correct. 8 and 16 channels do not.

ftab commented 2 years ago

Here's what a Digital Discovery I2S probe looks like for the specified config (8 channels)

ftab commented 2 years ago

Some more information:

If mclk_multiple is set to 512 so that bclk divider is 2 for this config, bclk is the correct frequency, but ws still does not work in I2S_COMM_FORMAT_STAND_I2S (stays high)

We have found that setting communication_format to I2S_COMM_FORMAT_STAND_PCM_LONG or I2S_COMM_FORMAT_STAND_PCM_SHORT appears to give a usable WS signal (1 channel wide or 1 BCLK wide, respectively)

For STAND_I2S in 8 channels x 32-bit wide, the ws_width of 128 sets the register to 127, which means all 7 bits are set to 1 in https://github.com/espressif/esp-idf/blob/5c33570524118873f7bd32490c7a0442fede4bf8/components/soc/esp32s3/include/soc/i2s_struct.h#L138 - if I had to hazard a guess, I'd say all 1s means something special to the peripheral. I cannot find any documentation on the I2S peripheral registers for the S3 to confirm.

If mclk_multiple is left at the default 256 and it is set to 16 bit sample/channel width, bclk is again the correct frequency and ws works, even in I2S_COMM_FORMAT_STAND_I2S mode (the expected 50% duty cycle is observed)

I think the answer seems to be set mclk_multiple to 512 and use another communication format than standard i2s. I will try this when I'm back in the office (there is a great ice-pocalypse going on in my area right now)

I am trying to use the PCM1681 which in TDM mode appears to be able to tolerate any one of those three formats (or even a WS that is high for 255 BCLK and only low for 1 BCLK).

ftab commented 2 years ago

Update to the previous findings: while setting mclk_multiple to 512 and communication_format to I2S_COMM_FORMAT_STAND_PCM_LONG or I2S_COMM_FORMAT_STAND_PCM_SHORT results in the correct bclk, now WS is running at 88.2khz instead of 44.1khz

If tx_half_sample_bits has anything to do with it, this is only 6 bits wide, so it can't contain the value of 128-1 that is being set

ftab commented 2 years ago

@L-KAYA hope it's ok if I mention you, it seems like you're the most familiar with the I2S driver lately.

Should this configuration be possible? (8 channels, 24 bit samples, 32 bit channel spacing)

It would seem there are registers that cannot contain the correct values when channel count exceeds 4 at this width, but I'm also having trouble getting the correct playback even at 4 channels. Trying to get a good data capture to see what's going on.

I have updated the example (by default now set to 4 channels, mclk multiple 512, PCM_SHORT) and it is now outputting just a hardcoded value per channel so that we can try to identify particular channels on the data stream

Channel 1 is a value of 0x80000000, channel 2 is 0xc0000000, channel 3 is 0xe0000000, channel 4 is 0xf0000000, so that the number of BCK wide indicates which channel it should be. (I hope I did that math right)

Here are two sections from a capture:

In the first you see nothing where there is supposed to be channel 1, then at the 57th rising edge of BCLK after the WS goes high, there is the data for channel 2, and after that, there's channel 3, and after that, there's channel 4

In the second section, at 57 BCLKs in, channel 4, followed by channel 1, followed by channel 2

In short, it seems the channels are drifting and offset, such that the peripheral never manages to fit all 4 channels into a single WS cycle

L-KAYA commented 2 years ago

Hi @ftab , thanks for your feedback!

I tested TDM mode with 8 channels, the signal looks exactly what you shown above.According to my roughly observation, I think it is probably a hardware limitation which is not mentioned anywhere. Although the software has given a correct configuration, maybe extra conditions should be added to get it work.

Here is my findings:

I checked clock divisions and all coefficients seem correct.
The ws width register tx_tdm_ws_width has only 7 bits which indicates its maximum value is 127, and the maximum width is tx_tdm_ws_width + 1 = 128 , it is equal to the 8 channels / 32 bits per channel case, but I don't know why it didn't work.
The mclk to bclk division register tx_bck_div_num is right above tx_tdm_ws_width, I'm thinking about if the overflow bit in tx_tdm_ws_width will somehow be added into tx_bck_div_num.
There is a register named tx_half_sample_bits, I don't know what it used for, now it is half the number of bclk in one sample period. It seems also have some effect. By the way, it has only 6 bits which is overflowed in 8 channels / 32 bits per channel case

(You can find the registers I mentioned in esp-idf/components/soc/esp32s3/include/soc/i2s_struct.h)

Anyway, this issue needs further more investigation, I'll post here if I have any solutions. Thank you again for finding this bug!

ftab commented 2 years ago

Thanks for confirming my findings, and for looking into this!

I did find some evidence in the original esp32 technical reference manual (page 304-305 / section 12.3) where it states M (BCK divider) must be at least 2. If this carried over to the s3, it explains why BCK is incorrect at the default MCK multiplier of 256.

I also observed shortly after I commented yesterday, if I set sample width and channel width both to 32, the 4 channel data looks correct. It seems like the I2S driver will now read 24 bits from the byte array for each sample, if set to 24 bit sample width / 32 bit channel width. The i2s_basic example (and my previous experience) were going with the understanding that you still write 32 bits to the byte array for each sample and it just throws out the last 8 bits when it goes over the wire.

I have attempted to do a dual-i2s-transmitter configuration (branch dual-i2s on the tdm_example repo) to see if we can pair up two synchronously running in TDM4 to get 8 channels quickly, but it would seem the slave transmitter does not always respect WS boundaries

^-- it would be workable if it was always like this (that is what I'd expect it to look like), but after some time, it becomes this:

So it looks like the last hope for getting more than 4 channels out of the S3 is some workaround for these overflowing registers.

Knowing next to nothing about this peripheral but just throwing out a guess, maybe it sets a counting register to tx_half_sample_bits * 2 or something similar and counts down before flipping WS again... this may be why WS becomes 88.2khz in the 8 channel 32 bit config, since it's triggering the next WS after half the number of samples it should be, due to the 6 bit limit. But I don't see anywhere in the code where it actually reads that register (or the macro for it defined in i2s_reg.h), so maybe it's internal to the peripheral once it's set up.

L-KAYA commented 2 years ago

Nice assumption! It totally makes sense for this issue. I'm trying to confirm with related hardware designer, if it is indeed a hardware bug, maybe he can give some advice.

Two parallel I2S sounds like a good solution, but the problem is we can't keep them always synchronous. Their might be some way to make them synchronous, because the clock source of them are same, I'll have a try.

L-KAYA commented 2 years ago

According to my test, two parallel I2S can theoretically work synchronously, they can keep a same phase difference in long term.

But the thing is how to start two I2S at the same time, as far as I know, we don't have such a switch to turn on them at the same time.

ftab commented 2 years ago

My test in the dual-i2s branch of tdm_example uses i2s0 as master transmitter and i2s1 as slave transmitter, and jumper wires between the physical pins for i2s0 -> i2s1 BCK (6 -> 41) and WS (4 -> 42). I thought i2s1 would then sync using the i2s0 transmitter's clock lines (starting the transmission only once i2s0 WS went high). But that isn't always true.

L-KAYA commented 2 years ago

One master and one slave is a good idea! Then I have a way to make them work synchronously without connecting any jumper wires. This is a feature for ESP chips, I2S signals can be connected internally in a gpio pin.

Set same bclk and ws pin for both master and slave(like GPIO4 for bclk and GPIO5 for ws), but leave data pin different.
Include esp_rom_gpio.h and hal/i2s_hal.h.

add following lines after i2s_set_pin, I'm using I2S0 as master and I2S1 as slave.

/* Connnect BCLK signal to GPIO4  */
esp_rom_gpio_connect_out_signal(GPIO_NUM_4, i2s_periph_signal[0].m_tx_bck_sig, 0, 0);
esp_rom_gpio_connect_in_signal(GPIO_NUM_4, i2s_periph_signal[1].s_rx_bck_sig, 0);
/* Connect WS signal to GPIO5 */
esp_rom_gpio_connect_out_signal(GPIO_NUM_5, i2s_periph_signal[0].m_tx_ws_sig, 0, 0);
esp_rom_gpio_connect_in_signal(GPIO_NUM_5, i2s_periph_signal[1].s_rx_ws_sig, 0);

Here is my result, though the salve on C3/S3 is not totally edge aligned (the phase error depends on mclk frequency, the faster, the smaller), at least the data is correct on clcok edge.

L-KAYA commented 2 years ago

OK, confirmed, 32 bits & 8 channels is not supported, the number of bclk in one ws period is not supposed to be greater than 128, that means only 32 bits & 4 channels / 16 bits & 8 channels / 8 bits & 16 channels are available.

ftab commented 2 years ago

Thanks for the update. I'll try that suggestion to use the same pins and connect them internally, will report back later with the results.

That uses up both I2S peripherals, which slightly complicates how I planned to receive the audio that it will be processing. My main purpose for the ESP32-S3 was to get audio from the original ESP32 (received over Bluetooth and output via stereo I2S) split out to 8 channels for audio DSP / EQ and TDM output. If this dual TDM4 configuration works, I'll have to send the audio over SPI, or emulate another I2S peripheral with SPI, or something like that

Our other theory is some other companion chip besides the ESP32-S3, unless there's some way to hack this up on the original ESP32 (run I2S at 4x actual sample rate, internal GPIO magic to read the data back and start a 1x sample rate PWM manually at the right time? Otherwise, basically an FPGA to read it, align the channels, then write the true TDM8 signal)

ftab commented 2 years ago

Dual TDM4 looks promising, like yours, now that they're synchronized with the same pins. Thanks for the tip, that is a neat trick to connect pins internally.

How high can MCLK go, theoretically? I think since it's sampling on BCLK rising edge it might be good enough to try. I'll get another wire soldered onto the DAC and cook up a dual i2s stream element for ESP-ADF. (edit: promising results so far, will need more time to test to make sure things stay aligned)

I am mildly curious, I thought based on looking at the i2s_set_pin code that it'd need to set up s_tx_bck_sig and s_tx_ws_sig rather than s_rx_*, but it does need to be s_rx_* or the i2s_write times out.

L-KAYA commented 2 years ago

mclk = mclk_multiple * sample_rate, you can give a integer to mclk_multiple directly, but there is two conditions.

The frequency of clock source is 160MHz, mclk <= 160M / 2 = 80M;
BCLK is divided from MCLK, bclk_div = mclk / bclk should be a integer;

Take 32bit 4 channels 44.1KHz for example,

bclk = sample_rate * bit_per_chan * chan_num = 44.1K * 32 * 4 = 5.6488 MHz
bclk_div = (int)(max_mclk / bclk) = (int)(80 / 5.6488) = 14
The highest mclk_multiple can be mclk_multiple = bclk * bclk_div / sample_rate = 1792

ftab commented 2 years ago

Sorry, forgot to note this as solved.

Thanks!

ftab commented 2 years ago

It may, however, be worth documenting as a limitation of the peripheral, or warned in the code if attempting to exceed the parameters mentioned in this thread.

ruota commented 2 years ago

Can I use 8 channels 32 bit in slave mode ?

L-KAYA commented 2 years ago

Can I use 8 channels 32 bit in slave mode ?

Haven't tested it with slave mode, but probably can't use 8 channels with 32 bit as mentioned above.

knicklicht commented 1 year ago

Is this only a restriction of the ESP32-S3 or are ESP32-C3 and ESP32-C6 also affected?

L-KAYA commented 1 year ago

@knicklicht Yes, the target you mentioned are all affected, but this limitation may be solved in ESP32-H2

espressif / esp-idf