Output of my Audio DK is noisy/clipping when coming via i2s

[daubsi]

I am quite new to the nRF5340 Audio DK and used ace.johnny 's project on https://github.com/ace-johnny/nrfadk-hello_codec to get me started.

The overall project goal is to process audio data coming via line-in on the nRF5340 to add filters, effects, etc. and output again via the headphone.

In order to achieve this I transmit the data from the codec via i2s to the MCU, process it there and send it back. However, something in this chain is introducting a lot of noise/clipping and I can't find the reason for this. Right now my processing of the received data is just copying it 1:1 to the transmit buffer - no change whatsoever.

When I directly route the line-in input to the output and not going via i2s to the MCU and back everything is crystal clear.

This is the code I use:

/**
 * @file        main.c
 * 
 * @brief       Audio DK HW_CODEC test using I2S loop and tone/noise generators.
 */

#include <zephyr/kernel.h>
#include <nrf.h>
#include <nrfx_clock.h>

#include "cs47l63_comm.h"

////////////////////////////////////////////////////////////////////////////////
// NRFX_CLOCKS

#define HFCLKAUDIO_12_288_MHZ 0x9BA6
#define ENABLE_LINEIN
#undef ENABLE_MIC


/**
 * @brief       Initialize the high-frequency clocks and wait for each to start.
 * 
 * @details     HFCLK =         128,000,000 Hz
 *              HFCLKAUDIO =     12,288,000 Hz
 */
static int nrfadk_hfclocks_init(void)
{
	nrfx_err_t err;


	// HFCLK
	err = nrfx_clock_divider_set(NRF_CLOCK_DOMAIN_HFCLK, NRF_CLOCK_HFCLK_DIV_1);
	if (err != NRFX_SUCCESS) return (err - NRFX_ERROR_BASE_NUM);

	nrfx_clock_start(NRF_CLOCK_DOMAIN_HFCLK);
	while (!nrfx_clock_is_running(NRF_CLOCK_DOMAIN_HFCLK, NULL)) k_msleep(1);


	// HFCLKAUDIO
	nrfx_clock_hfclkaudio_config_set(HFCLKAUDIO_12_288_MHZ);

	nrfx_clock_start(NRF_CLOCK_DOMAIN_HFCLKAUDIO);
	while (!nrfx_clock_is_running(NRF_CLOCK_DOMAIN_HFCLKAUDIO, NULL)) k_msleep(1);


	return 0;
}



////////////////////////////////////////////////////////////////////////////////
// NRF_I2S

#define MCKFREQ_6_144_MHZ 0x66666000


#define I2S_BUFF_SIZE 256  // Define an appropriate buffer size

static int16_t rx_buffer[I2S_BUFF_SIZE];  // Buffer for received data
static int16_t tx_buffer[I2S_BUFF_SIZE]; 

void process_audio(int16_t *rx, int16_t *tx, size_t size)
{
    for (size_t i = 0; i < size; i++) {
		tx[i] = rx[i];  // Simple passthrough (modify as needed)
        	//tx[i] = (int16_t)(rx[i]*0.1);  // Volume control
		//tx[size-1-i] = rx[i];  // Reverse - doesn't work... -> only outputs silence...?
    }
}

/**
 * @brief       Initialize and start the I2S peripheral using NRF registers.
 * 
 * @details     I2S master, 48kHz 16bit, Left mono, TX only.
 */
static int nrfadk_i2s_reg_init(void)
{
	// Configure and enable
	NRF_I2S0->CONFIG.CLKCONFIG =    I2S_CONFIG_CLKCONFIG_CLKSRC_ACLK;
	NRF_I2S0->CONFIG.MCKFREQ =      MCKFREQ_6_144_MHZ;
	NRF_I2S0->CONFIG.RATIO =        I2S_CONFIG_RATIO_RATIO_128X;
	NRF_I2S0->CONFIG.CHANNELS =     I2S_CONFIG_CHANNELS_CHANNELS_Left;	
	NRF_I2S0->CONFIG.TXEN =         I2S_CONFIG_TXEN_TXEN_Enabled;  // Send audio samples to the nRF5340
	NRF_I2S0->CONFIG.RXEN = 	I2S_CONFIG_RXEN_RXEN_Enabled;  // Enable RX to receive the data from the nRF5340

	NRF_I2S0->ENABLE =              I2S_ENABLE_ENABLE_Enabled;

	// Start TX buffer
	NRF_I2S0->RXD.PTR = 		(uint32_t)rx_buffer;
	NRF_I2S0->TXD.PTR = 		(uint32_t)tx_buffer;
	NRF_I2S0->RXTXD.MAXCNT =        I2S_BUFF_SIZE / sizeof(uint32_t);	

        // Clear pending events
    	NRF_I2S0->EVENTS_RXPTRUPD = 0;
	    NRF_I2S0->EVENTS_TXPTRUPD = 0;

	NRF_I2S0->TASKS_START =         I2S_TASKS_START_TASKS_START_Trigger;
	return 0;
}

void i2s_polling_loop(void)
{
    while (1)
    {
        // Wait for new RX data
        while (NRF_I2S0->EVENTS_RXPTRUPD == 0);
		
        // Clear RX event
        NRF_I2S0->EVENTS_RXPTRUPD = 0;

        // Process audio data
        process_audio(rx_buffer, tx_buffer, I2S_BUFF_SIZE); // For now, just set tx_buffer == rx_buffer

        // Ensure we restart I2S for continuous operation
        NRF_I2S0->TASKS_START = I2S_TASKS_START_TASKS_START_Trigger;
	while (NRF_I2S0->EVENTS_TXPTRUPD == 0);
    }
}

////////////////////////////////////////////////////////////////////////////////
// HW_CODEC

/** CS47L63 driver state handle. */
static cs47l63_t cs47l63_driver;


/** CS47L63 subsystems configuration. */
static const uint32_t cs47l63_cfg[][2] =
{

	// Audio Serial Port 1 (I2S slave, 48kHz 16bit, Left mono, RX and TX)
	{ CS47L63_ASP1_CONTROL2,
		(0x10  << CS47L63_ASP1_RX_WIDTH_SHIFT) |        // 16bit
		(0x10  << CS47L63_ASP1_TX_WIDTH_SHIFT) |        // 16bit
		(0b010 << CS47L63_ASP1_FMT_SHIFT)               // I2S
	},
	{ CS47L63_ASP1_CONTROL3,
		(0b00 << CS47L63_ASP1_DOUT_HIZ_CTRL_SHIFT)      // Always 0
	},
	// Enable the various channels for RX and TX
	{ CS47L63_ASP1_ENABLES1,
		(0 << CS47L63_ASP1_RX2_EN_SHIFT) |              // Disabled
		(1 << CS47L63_ASP1_RX1_EN_SHIFT) |              // Enabled
		(0 << CS47L63_ASP1_TX2_EN_SHIFT) |              // Disabled
		//(0 << CS47L63_ASP1_TX1_EN_SHIFT)                // Disabled
		(1 << CS47L63_ASP1_TX1_EN_SHIFT)                // Enabled - we want to send something to the nRF5340
	},

#ifdef ENABLE_LINEIN
	// Enable line-in
	{ CS47L63_INPUT2_CONTROL1, 0x00050020 },/* MODE=analog */ 
	{ CS47L63_IN2L_CONTROL1, 0x10000000 },  /* SRC=IN2LP */
	{ CS47L63_IN2R_CONTROL1, 0x10000000 },  /* SRC=IN2RP */
	{ CS47L63_INPUT_CONTROL, 0x0000000C },  /* IN2_EN=1 */
	// Set volume for line-in
	{ CS47L63_IN2L_CONTROL2, 0x00800080 },  /* VOL=0dB, MUTE=0 */
	{ CS47L63_IN2R_CONTROL2, 0x00800080 },  /* VOL=0dB, MUTE=0 */
	{ CS47L63_INPUT_CONTROL3, 0x20000000 }, /* VU=1 */
	// Important
	/* Route IN2L and IN2R to I2S */
	{ CS47L63_ASP1TX1_INPUT1, 0x800012 },
	{ CS47L63_ASP1TX2_INPUT1, 0x800013 },
#endif

	// Output 1 Left (reduced MIX_VOLs to prevent clipping summed signals)
	// this here is only there so we hear also that i2s data is sent 
	
	{ CS47L63_OUT1L_INPUT1,
		(0x2B  << CS47L63_OUT1LMIX_VOL1_SHIFT) |        // quite weak
		(0x020 << CS47L63_OUT1L_SRC1_SHIFT)             // ASP1_RX1 // from MCU (currently a sine wave only)
	},

	{ CS47L63_OUT1L_INPUT2,
		(0x2B  << CS47L63_OUT1LMIX_VOL2_SHIFT) |        // 
		(0x021 << CS47L63_OUT1L_SRC2_SHIFT)             // ASP1_RX2
	},
	
#undef ENABLE_LINEIN
#ifdef ENABLE_LINEIN
	// We need both channels here, even if we only have one output channel
	// If we uncomment the next two {} we won´t get any line in pass-through, only i2s
	{
		CS47L63_OUT1L_INPUT3,
		(0x2B  << CS47L63_OUT1LMIX_VOL3_SHIFT) |
		(0x012 << CS47L63_OUT1L_SRC3_SHIFT)    // 0x12=IN2L
	},
	{
		CS47L63_OUT1L_INPUT4,
		(0x2B  << CS47L63_OUT1LMIX_VOL4_SHIFT) |
		(0x013 << CS47L63_OUT1L_SRC4_SHIFT)    // 0x13=IN2R
	},
#endif

	{ CS47L63_OUTPUT_ENABLE_1,
		(1 << CS47L63_OUT1L_EN_SHIFT)                   // Enabled
	},
};


/**
 * @brief       Write a configuration array to multiple CS47L63 registers.
 * 
 * @param[in]   config: Array of address/data pairs.
 * @param[in]   length: Number of registers to write.
 * 
 * @retval      `CS47L63_STATUS_OK`     The operation was successful.
 * @retval      `CS47L63_STATUS_FAIL`   Writing to the control port failed.
 */
static int nrfadk_hwcodec_config(const uint32_t config[][2], uint32_t length)
{
	int ret;
	uint32_t addr;
	uint32_t data;

	for (int i = 0; i < length; i++)
	{
		addr = config[i][0];
		data = config[i][1];

		ret = cs47l63_write_reg(&cs47l63_driver, addr, data);
		if (ret) return ret;
	}

	return CS47L63_STATUS_OK;
}


/**
 * @brief       Initialize the CS47L63, start clocks, and configure subsystems.
 * 
 * @details     MCLK1 =   6,144,000 Hz  (I2S MCK = CONFIG.MCKFREQ)
 *              FSYNC =      48,000 Hz  (I2S LRCK = MCK / CONFIG.RATIO)
 *              BCLK =    1,536,000 Hz  (I2S SCK = LRCK * CONFIG.SWIDTH * 2)
 *              FLL1 =   49,152,000 Hz  (MCLK1 * 8)
 *              SYSCLK = 98,304,000 Hz  (FLL1 * 2)
 * 
 * @retval      `CS47L63_STATUS_OK`     The operation was successful.
 * @retval      `CS47L63_STATUS_FAIL`   Initializing the CS47L63 failed.
 * 
 * @note        I2S MCK must already be running before calling this function.
 */
static int nrfadk_hwcodec_init(void)
{
	int ret = CS47L63_STATUS_OK;


	// Initialize driver
	ret += cs47l63_comm_init(&cs47l63_driver);


	// Start FLL1 and SYSCLK
	ret += cs47l63_fll_config(&cs47l63_driver, CS47L63_FLL1,
	                          CS47L63_FLL_SRC_MCLK1, 6144000, 49152000);

	ret += cs47l63_fll_enable(&cs47l63_driver, CS47L63_FLL1);

	ret += cs47l63_fll_wait_for_lock(&cs47l63_driver, CS47L63_FLL1);

	ret += cs47l63_update_reg(&cs47l63_driver, CS47L63_SYSTEM_CLOCK1,
	                          CS47L63_SYSCLK_EN_MASK, CS47L63_SYSCLK_EN);


	// Configure subsystems
	ret += nrfadk_hwcodec_config(cs47l63_cfg, ARRAY_SIZE(cs47l63_cfg));


	return ret;
}



////////////////////////////////////////////////////////////////////////////////
// MAIN

int main(void)
{
	// Initialize Audio DK

	if (nrfadk_hfclocks_init() ||
	    nrfadk_i2s_reg_init()  ||
	    nrfadk_hwcodec_init())
	{
		printk("\nError initializing Audio DK\n");
		return -1;
	}

	printk("\nAudio DK initialized\n");
	k_msleep(1250);



	// Unmute OUT1L I2S playback and enable NOISE/TONE1 generators

	cs47l63_update_reg(&cs47l63_driver, CS47L63_OUT1L_VOLUME_1,
	                   CS47L63_OUT_VU_MASK | CS47L63_OUT1L_MUTE_MASK,
	                   CS47L63_OUT_VU | 0);

	printk("\nOUT1L unmuted for 5000ms\n");

	k_msleep(5000);
	i2s_polling_loop(); // We actually never return from here (testing)

	while(1);
	cs47l63_update_reg(&cs47l63_driver, CS47L63_OUT1L_VOLUME_1,
	                   CS47L63_OUT_VU_MASK | CS47L63_OUT1L_MUTE_MASK,
	                   CS47L63_OUT_VU | CS47L63_OUT1L_MUTE);

	printk("OUT1L muted\n");
	k_msleep(1250);



	// Shutdown Audio DK (reverse order initialized)

	cs47l63_update_reg(&cs47l63_driver, CS47L63_OUTPUT_ENABLE_1,
	                   CS47L63_OUT1L_EN_MASK, 0);
	printk("\nOUT1L disabled\n");
	k_msleep(250);

	cs47l63_update_reg(&cs47l63_driver, CS47L63_SYSTEM_CLOCK1,
	                   CS47L63_SYSCLK_EN_MASK, 0);
	printk("SYSCLK disabled\n");
	k_msleep(250);

	cs47l63_fll_disable(&cs47l63_driver, CS47L63_FLL1);
	printk("FLL1 disabled\n");
	k_msleep(250);

	NRF_I2S0->TASKS_STOP =  I2S_TASKS_STOP_TASKS_STOP_Trigger;
	NRF_I2S0->ENABLE =      I2S_ENABLE_ENABLE_Disabled;
	printk("I2S disabled\n");
	k_msleep(250);

	nrfx_clock_stop(NRF_CLOCK_DOMAIN_HFCLKAUDIO);
	while (nrfx_clock_is_running(NRF_CLOCK_DOMAIN_HFCLKAUDIO, NULL)) k_msleep(1);
	printk("HFCLKAUDIO stopped\n");
	k_msleep(250);

	nrfx_clock_stop(NRF_CLOCK_DOMAIN_HFCLK);
	while (nrfx_clock_is_running(NRF_CLOCK_DOMAIN_HFCLK, NULL)) k_msleep(1);
	printk("HFCLK stopped\n");
	k_msleep(250);


	printk("\nAudio DK shutdown\n\n");
	k_msleep(100);


	return 0;
}

process_audio() is the simple function which just sets the TX buffer to the RX buffer's values. 

(Here I want eventually to add my processing)

nrfadk_i2s_reg_init() initializes the i2s on the nRF5340 side. I've added here also the reception of the data from the codec

i2s_polling_loop() constantly checks if i2s data is present, then copies it over and starts the back transmission

cs47l63_cfg() configures the line-in input, the i2s in the ASP1 and the output channel coming from i2s 

My assumption is that the error is in one of those regions:

i2s_polling_loop() - maybe done completely wrong at all?

cs47l63_cfg() - I am confused - do I have a stereo input with line-in or only mono? Will I need ASP1RX1,2 and ASP1TX1,2 then or just ASP1RX1 and ASP1TX1?

Is it OK to use int16_t as the values in the buffer but uint32_t to address the elements? Does the i2s config in the codec match?

Is the noise maybe just a result of the MCU not being able to keep up? 

The audio is properly played back in genereal but there is noise/clipping present all the time - not only during loud parts of the music.

[ace.johnny]

There's an incredibly helpful resource available from Nordic's website on the Downloads tab of their nRF5340 Audio DK product page: nRF5340 Audio Development Kit - Hardware files 1_0_0.zip. Inside of which, a few folders deep, you'll find PCA10121_Schematic_And_PCB.pdf, detailing every single electrical connection on the circuit board, complete with clickable links so that you can trace each bus across its many pages.

Studying this is essential to understanding how the system is designed, which is especially important since Nordic's own nrf5340_audio software does not even match the schematic or silkscreen.

Here's a link to the CS47L63 schematic from the nRF5340 Audio DK User Guide:

The CS47L63 has 4 channels of input across 2 stereo pair: IN1(LR) and IN2(LR). Also, those two pair each have several methods of connection: digital or analog, single-ended or differential.

IN1(LR) is dedicated exclusively to the onboard digital PDM1 mic, however its second channel(R) remains unused, so is essentially a mono signal. This is important to remember when routing signals inside the codec.

IN2(LR) is shared between analog stereo LINE-IN and an auxiliary digital stereo PDM2 mic input on connector P15, and only one of these can be selected at a time.

The MICBIAS generator has 3 outputs ABC, and only 2 are used: B drives the onboard mic, and C drives the aux PDM input. A is not used, but could conceivably drive an analog mic if one were wired to the available analog IN1(LR) test points.

Nordic's cs47l63_reg_conf.h file relies on a few register defaults, so does not set every single bitfield, and its settings are spread across several separate configuration arrays, which makes it more difficult to study.

Like you, I went through this exact same painful dissection, tediously referencing each register's changes from default, and eventually constructed my own configuration arrays in a much more legible manner, as follows...

// IN2 Analog Line
{ CS47L63_INPUT2_CONTROL1,
        (0b101 << CS47L63_IN2_OSR_SHIFT)  |             //   Standard
        (0     << CS47L63_IN2_MODE_SHIFT) |             //   Analog
        (1 << 5)                                        //   Required!
},

// IN2 CH1 Left
{ CS47L63_IN2L_CONTROL1,
        (0b01 << CS47L63_IN2L_SRC_SHIFT)        |       // Δ Single
        (0x00 << CS47L63_IN2L_RATE_SHIFT)       |       //   SAMPLE_RATE_1
        (0    << CS47L63_IN2L_HPF_SHIFT)        |       //   Disabled
        (0    << CS47L63_IN2L_SIG_DET_EN_SHIFT) |       //   Disabled
        (0    << CS47L63_IN2L_LP_MODE_SHIFT)            //   Normal
},

// IN2 CH2 Right
{ CS47L63_IN2R_CONTROL1,
        (0b01 << CS47L63_IN2R_SRC_SHIFT)        |       // Δ Single
        (0x00 << CS47L63_IN2R_RATE_SHIFT)       |       //   SAMPLE_RATE_1
        (0    << CS47L63_IN2R_HPF_SHIFT)        |       //   Disabled
        (0    << CS47L63_IN2R_SIG_DET_EN_SHIFT) |       //   Disabled
        (0    << CS47L63_IN2R_LP_MODE_SHIFT)            //   Normal
},

Unfortunately, there does not seem to be any easier method to learn these details.

[daubsi]

Thanks, I actually had a look at that file/schematic when I was trying to understand your point in another discussion with the ASP2 setup ( Error in nRF5340 Audio DK sample application ), and to verify/understand where the but I didn't bother to look again for the PDM. Thanks for your insights about how it is all wired internally.

The MICBIAS generator has 3 outputs ABC, and only 2 are used: B drives the onboard mic, and C drives the aux PDM input. A is not used, but could conceivably drive an analog mic if one were wired to the available analog IN1(LR) test points.

So that means in the excerpt from the original Nordic code I annotated above, if I ONLY want the onboard digital microphone I would only need MICBIAS3 and I actually would only need IN1L as the IN1R is not used anyway? Same for IN2* as it's not related to the PDM? 

{ CS47L63_LDO2_CTRL1, 0x0005 }, // p165, 0b00000101 -> Bit 0, LDO2_EN=1, Bit 2, LDO2_DISCH=1, Voltage = 0x0 (default) 2.4V
{ CS47L63_MICBIAS_CTRL1, 0x00EC }, // p166, 0b11101100 -> Bit 2, MICB1_DISCH=1, discharge when disable, (default = 1)
// Bit 3, MICB1_RATE=1, 1 = Pop-free start-up/shutdown (default = 0)
// Bit 5-7 = 0x7 = 2.2V (default)
{ CS47L63_MICBIAS_CTRL5, 0x0272 }, // p166, 0b0010 0111 0010 -> Bit 1 MICB1A_DISCH=1, discharge when disable (default)
// -> Bit 4 MICB1B_EN=1
// -> Bit 5 MICB1B_DISCH=1, discharge when disable (default)
// -> Bit 6 MICB1B_SRC=1 = VDD_A (default 0, MICBIAS regulator)
// -> Bit 9 MICB1C_DISCH=1, discharge when disable (default)

/* Enable IN1L */
{ CS47L63_INPUT_CONTROL, 0x000F }, // p29, 0b00001111 -> Bit 0, IN1R_EN,
// Bit 1, IN1L_EN,
// Bit 2, IN2R_EN,
// Bit 3, IN2L_EN

Actually I was only able to make sense out of the various control registers by looking a the long explicit listing of all the registers at the end of the datasheet where each and every bit is shown at the same place and not scattered through various pages.

What I don't get for example is:

CS47L63_INPUT1_CONTROL1 also configures the oversampling rate. Is this something which only is applicable to a microphone? Don't we have/need this for line-in? 

I fully agree it would be very nice to have like 3,4,5 minimalistic samples from Nordic how to use the codec on THEIR board with THEIR defined setup (what is wired where), like "route line-in via i2s to MCU", "send i2s to codec for output", etc. etc. That would really speed up the learning with this board.

Another question you might have an answer for:

Currently I use the codec to output to headphones, but is it also possible to connect to Apple AirPods via BLE from the MCU? My stretch goal is to write a simple Guitar FX processor by doing the FXs (chorus, echo, reverb, ...) on the ARM core and then connect to my AirPods for audio output. I know the compatibility of the BLE audio was something which was intensively discussed during the Nordic Techtour 2 years ago with Apple being proprietary but I can easily connect the pods to my Linux and Windows PC like-wise without problems so it probably is not very special? Or is it then standard Bluetooth and not BLE? Any ideas?