I2S LRCK aligned with rising edge of BCK in input mode, falling edge of BCK in output mode.

[JimMacA]

Running on an RP2350, 2 separate I2S engines, one output and one input

#include <I2S.h>
I2S i2s(OUTPUT);
I2S i2sIn(INPUT);
...
#define pBCLK 17
#define pDOUT 16
#define INBCLK 11
#define INDOUT 10

const int sampleRate = 44100;

... in Setup:

i2s.setBCLK(pBCLK);
i2s.setDATA(pDOUT);
i2s.setBitsPerSample(32);
i2s.setBuffers(4, 64); //number of buffers, size of each buffer.
i2s.begin(sampleRate);

i2sIn.setBCLK(INBCLK);
i2sIn.setDATA(INDOUT);
i2sIn.setBitsPerSample(32);
i2sIn.setBuffers(4, 64); //number of buffers, size of each buffer.
2sIn.begin(sampleRate);

This setup synchronizes LRCK edges to falling edges of BCK for the output I2S, but rising edges of BCK for the input I2S. Behavior is very robust, and doesn't seem to depend on BitsPerSample or buffer size.

Is this expected behavior? Some ADCs (PCM1808, PCM1820) expect LRCK to change on the falling edge of BCK. For the PCM1808, changing LRCK between 50nsec before and 10nsec after the rising edge of BCK can cause trouble.

[earlephilhower]

Good observation. Looking at the PIO code for I2S receive it does change on BCLK 0->1

arduino-pico/libraries/I2S/src/pio_i2s.pio

Lines 153 to 172 in f5c4136

	.program pio_i2s_in ; Note this is the same as _out, just "in" and not "out"
	.side_set 2 ; 0 = bclk, 1=wclk
	; The C code should place (number of bits/sample - 2) in Y and
	; also update the SHIFTCTRL to be 24 or 32 as appropriate
	; +----- WCLK
	; |+---- BCLK
	mov x, y side 0b00
	left1:
	in pins, 1 side 0b01
	jmp x--, left1 side 0b00
	in pins, 1 side 0b11 ; Last bit of left has WCLK change per I2S spec
	mov x, y side 0b10
	right1:
	in pins, 1 side 0b11
	jmp x--, right1 side 0b10
	in pins, 1 side 0b01 ; Last bit of right also has WCLK change
	; Loop back to beginning...

Do you have an ADC that's failing with the current configuration available? (Un)fortuntely the ones I had on hand while writing the code worked fine for setup/hold with this timing.

If you'd like to take a stab at rewriting the i2s input PIO code, a PR would be much appreciated, of course. :)

[JimMacA]

Thanks for checking on this. The fix looks straightforward:

165: in pins, 1 side 0b01
171: in pins, 1 side 0b11

I do have an ADC that is currently failing (PCM1820) so I can test that against the new code. I do worry about this change breaking other ADCs, though.

[palmerr23]

When you're done, I'll test it on a PCM3060.

[JimMacA]

I tested the code change above -- works as advertised on the PCM1820. I suspect that the PCM3060 will also prefer that timing -- it has similar timing specs.
PR executed, I think. I'm a Github novice, so I hope I didn't break anything.

[palmerr23]

My first pull request was scary too!

I'll test it with the PCM3060 in the next day or two and report back.

[palmerr23]

The PCM 3060 seems OK with the revised code, however I'm seeing LRCLK changing on the RISING edge of BCLK.

Mode is standard I2S (not LSBJ) 24 bits. LA sample rate is 200MHz.

[JimMacA]

Hmm, I must have munged the PR. Can you confirm that you're seeing the following lines from the assembled code, pio_i2s.pio.h, starting at line 286:

0xa022, // 0: mov x, y side 0
0x4801, // 1: in pins, 1 side 1
0x0041, // 2: jmp x--, 1 side 0
0x4801, // 3: in pins, 1 side 1
0xb022, // 4: mov x, y side 2
0x5801, // 5: in pins, 1 side 3
0x1045, // 6: jmp x--, 5 side 2
0x5801, // 7: in pins, 1 side 3

Going from step 3 to step 4, WCLK (bit 1) transitions from 0 to 1 as BCLK (bit 0) transitions from 1 to 0.
And from step 7 to step 0, WCLK transitions from 1 to 0 as BCLK transitions from 1 to 0.

[JimMacA]

What the timing should look like in the field:

Both edges:

BTW, note the 7nsec timing difference between the rising edges of BCLK for the left and right phases. This won't cause trouble with the data clocking, but conceivably could affect the PLL of an A/D or D/A chip. But I'm not seeing any such trouble with my chips, so I'm not reporting it.

[earlephilhower]

The 7ns difference is because you're not running the bclk at an exact divisor of the sysclock so it's got a dithering on the clocks to hit the desired frequency. There's a call in the I2S object to give a better match, see setSysClk

PIOASM builds the same .H file you've committed, so there is no difference there.

If #2592 didn't actually fix things, we can roll it back, no worries...

[JimMacA]

I've confirmed that downloading from here:
https://github.com/earlephilhower/arduino-pico/tree/master/libraries/I2S/src
to a local Arduino sketch folder and using those files fixes the problem. Using release 4.2.0 re-breaks it.

[earlephilhower]

Sorry, you've lost me now. The 4.2.0 release version doesn't have your changes, those came in after it went out.

So, if 4.2.0 fails and the git master works, we have no issue and all's well. In a couple weeks we'll drop a new release.

[palmerr23]

Jim

I copied the src folder over the 4.2.0 release i2s library code to make sure I wasn't compiling the wrong thing, using code directly from github:

284...

static const uint16_t pio_i2s_in_program_instructions[] = {
// .wrap_target
0xa022, // 0: mov x, y side 0
0x4801, // 1: in pins, 1 side 1
0x0041, // 2: jmp x--, 1 side 0
0x4801, // 3: in pins, 1 side 1
0xb022, // 4: mov x, y side 2
0x5801, // 5: in pins, 1 side 3
0x1045, // 6: jmp x--, 5 side 2
0x5801, // 7: in pins, 1 side 3
// .wrap
};

I re-ran the tests to make sure and now have LRCLK changing on the FALLING edge. I'm not sure what happened with yesterday's measurements as I didn't change any code today, just recompiled.

My apologies!