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]

Complicated and frustrating is an understatement.

While my nrfadk-hello_codec example is relatively easy to understand, it is undoubtedly a poor basis for proper audio programming technique; it's extremely cavalier with assumed CS47L63 register defaults, it eschews the use of Zephyr's memory management kernel services (which appear to be absolute necessities for correct data alignment), and it ignores the availability of several nrfx libraries designed to mitigate these and other audio stream intricacies.

At the other extreme is Nordic's behemoth BLE nrf5340_audio application, which uses every available Zephyr service and NCS feature under the sun to demonstrate their intended use in a production-ready program, at the expense of easy study for beginners just trying to piece together their first "hello world" audio app.

I wish there were a more easily approachable middle ground, but there's no avoiding the fact that you'll almost certainly have to start with Nordic's bewildering sample instead of mine if you want to master their Audio DK.

There are also several somewhat hidden files that you're missing when trying to decipher Nordic's nrf5340_audio and TomHe's nrf5340_i2s_echo CS47L63 configurations, namely the sdk-mcu-drivers repo, which is stored under /modules/hal/cirrus-logic/cs47l63 in your local SDK directory. I've complained about the discoverability and difficult defaults here.

Some observations after a quick perusal of your code (I admittedly haven't tried running it yet).

First, you shouldn't be retriggering I2S_TASKS_START in every i2s_polling_loop(). If your code requires this to function, then it is clearly an incorrect approach. Once started, the I2S peripheral should run automatically until stopped, so long as it has data available to process.

Second, I've had intractable problems trying to use raw register interrupts in my own application attempts, which is the subsystem that's signalling each NRF_I2S0->EVENTS_FOO event. Nordic's nrfx libraries apparently manage this, but their use is not helpfully explained or documented anywhere other than directly in the nrf5340_audio example code.

Finally, I suspect that your memory allocation is missing needed alignment. All the audio examples I've studied so far seem to use Zephyr's memory slabs or similar services (such as ring buffers, etc.), a topic about which I'm still woefully uneducated, but this could very well explain the noise you're experiencing.

I know this hasn't been super helpful when you're first learning to wrangle such staggering complexity, but hopefully TomHe or Alexander Svensen might chime in with their more advanced knowledge.

[daubsi]

Thank you very much for your insights! I was indeed very busy today learning more about the i2s_echo demo vs. your base app.

I've spend quite some minutes dissecting each and every config register write and trying to understand where the two apps differ. One major difference is that your app samples at 48kHz and the SYSTEMCLK is at 98.304 MHz, whereas the i2s_echo uses 16kHz and presumably a 49.152 MHz.

Then I fired up my trusty Saleae logic analyzer and hooked it to the ASP1 pins on P5 and just sampled 10 seconds of DOUT and DIN, analyzed as i2s and converted to PCM. The data on DOUT, Codec->MCU is crystal clear and - as expected - the data from MCU->Codec on DIN is noisy. As unfortunate as it is that it's noisy, at least it seems to proof that the whole i2s config by itself is correct, something I wasn't sure about either. 

I've seen, as you mentioned above, the i2s_echo uses ringbuffers as one difference to yours and I presume the other topics you mentioned might play a role as well. I'll try to understand more of it now that I know that the noisy audio is not a result of an improper codec configuration but lies somewhere in the MCU program. 

I'll attach the noisy sample (just 10sec of an arbitrary radio station audio stream I fed into LineIn, not usually my type of music ;-) as well as my current app which is based on your demo app.

I took the freedom to annotate each and every config write in even higher detail and also added the datasheet page references in order to read up the stuff. 

I'll try to play with the i2s_polling_loop code and adapt it to your suggestions. I have to admin I used ChatGPT as the starting point of the code as I had no clue how to use i2s before at all. My first idea was to use EasyDMA but as soon as I enabled the interrupts after setting everything up everything froze on return of the function. I presume the interrupt handler was not properly defined and ChatGPT just forgot about that to tell me :-D I'll have another look at the Nordic Academy as well to get some understanding about how to use an interrupt driven i2s. 

As hard as it might be, it's still rewarding to make baby steps towards a better understanding. Especially the manual lookup of all the constants and bitfields with the datasheet really helped a lot to better understand what's going on. Well, apart from the clock definition code maybe. I annotated the clock generation (const uint32_t clock_configuration[][2]) from the i2s_echo example and maybe I did it wrong, but I have no clue what the final clock would be honestly.

// Currently unused in minimum example, just for reference
// Used in i2s_echo
// clocks are different from our barebone example (see comments for nrfadk_hwcodec_init())
const uint32_t clock_configuration[][2] = {
	{ CS47L63_SAMPLE_RATE3, 0x0012 }, // p125, p132, 16kHz
	{ CS47L63_SAMPLE_RATE2, 0x0002 }, // p125, p132, 24kHz
	{ CS47L63_SAMPLE_RATE1, 0x0003 }, // p125, p132, 48kHz
	{ CS47L63_SYSTEM_CLOCK1, 0x034C }, // p131, 0b0011 01001100, Bit 8-10: 0b011 = 49.152 MHz, Bit 6 = SYSCLK_EN, Bit 0-4 = 0x00 = MCLK1
	{ CS47L63_ASYNC_CLOCK1, 0x034C }, // p132, 0b0011 01001100, Bit 8-10: 0b011 = 49.152 MHz, Bit 6 = ASYNCCLK_EN, Bit 0-4 = 0x00 = MCLK1
	{ CS47L63_FLL1_CONTROL2, 0x88200008 }, // p139, 0b10001000 00100000 00000000 00001000, Bit 0-9 = FLL1 Integer multiply for FREF, default 0x4, Bit 21 = FLL1 Reference Detect control, enabled (default), Bit 27 = FLL1_LOCKDET, enabled (default), Bit 28-31 = FLL1 Lock Detect threshold = 0x8 = default
	{ CS47L63_FLL1_CONTROL3, 0x10000 }, // p139, FLL1 Fractional multiply for FREF = 0x1, default is 0x0
	{ CS47L63_FLL1_GPIO_CLOCK, 0x0005 }, // p139, FLL1_GPCLK_DIV = 0x02, default
	{ CS47L63_FLL1_CONTROL1, 0x0001 }, // p139, FLL1 enable
};

nrf5340_fx.zip