Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix test_i2s_basic_slave and test_slave_bclk_invert tests #103

Merged
merged 7 commits into from
Feb 16, 2024
Merged

Fix test_i2s_basic_slave and test_slave_bclk_invert tests #103

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b91b946
Get test_i2s_basic_slave.py tests passing
Feb 15, 2024
d9ab794
Store lrclk freq instead of bcl frequencies in the lut. Verify SMOKE=…
Feb 15, 2024
d2a2bab
Extend num_in_out_args for test_slave_bclk_invert.py
Feb 15, 2024
273666e
Add comments
Feb 15, 2024
be5cfc2
Change macro names
Feb 15, 2024
1af7fbd
format python file
Feb 15, 2024
bca2034
Run all 6 lr frequencies for the test_i2s_basic_slave nightly test
Feb 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions test/lib_i2s/i2s_slave_checker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def run(self):
xsi, self._setup_strobe_port, self._setup_data_port
)
xsi.drive_port_pins(self._bclk, bclk1)
xsi.drive_port_pins(self._lrclk, 1)
xsi.drive_port_pins(self._lrclk, 0)

bclk_frequency = (bclk_frequency_u << 16) + bclk_frequency_l
print(
Expand Down Expand Up @@ -146,6 +146,10 @@ def run(self):
# the range 32 - 63, lr_clock outputs 1, else it outputs 0.
time = float(xsi.get_time())

time = self.wait_until_ret(
time + (clock_half_period * 64)
) # Add extra delay to ensure that the i2s_slave device sees the LRCLK transitions in the first for loop below

lr_counter = data_bits + (data_bits // 2) + (is_i2s_justified)
lr_count_max = (2 * data_bits) - 1

Expand Down Expand Up @@ -213,7 +217,7 @@ def run(self):
time = self.wait_until_ret(
time + clock_half_period - din_sample_offset
)

data_bit_mask = int("1" * data_bits, base=2)

for p in range(0, num_outs):
Expand Down
4 changes: 4 additions & 0 deletions test/lib_i2s/i2s_slave_test/i2s_slave_test.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ set(APP_LINK_OPTIONS
-target=XCORE-AI-EXPLORER
)

# Compile main.c which contains the i2s_callback_group_t functions in O3 mode. Needed for passing the
# test_i2s_basic_slave[4ch_in,4ch_out-32b] and test_i2s_basic_slave[4ch_in,4ch_out-16b] tests
set_source_files_properties(${CMAKE_CURRENT_LIST_DIR}/src/main.c PROPERTIES COMPILE_FLAGS "-O3")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is intended to check that the component is capable of operating using these settings, which it does - which is good. I would caution that this does highlight that the backpressure tolerance for the component in these settings is lower than we expected it to be - it would be useful to be able to profile these in the backpressure "test".


#**********************
# Tile Targets
#**********************
Expand Down
32 changes: 17 additions & 15 deletions test/lib_i2s/i2s_slave_test/src/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,16 @@ xclock_t bclk = XS1_CLKBLK_1;
#define I2S_LOOPBACK_LATENCY 1

#if SMOKE == 1
#define NUM_BCLKS 1
#define NUM_BCLKS_TO_CHECK 1
static const unsigned bclk_freq_lut[NUM_BCLKS] = {
1228800
#define NUM_LRCLKS 1
#define NUM_LRCLKS_TO_CHECK 1
static const unsigned lr_freq_lut[NUM_LRCLKS] = {
192000
};
#else
#define NUM_BCLKS 12
#define NUM_BCLKS_TO_CHECK 3
static const unsigned bclk_freq_lut[NUM_BCLKS] = {
1228800, 614400, 384000, 192000, 44100,
22050, 96000, 176400, 88200, 48000, 24000, 352800
#define NUM_LRCLKS 12
#define NUM_LRCLKS_TO_CHECK 3
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved, but, why are these 12 and 3? To be frank we could just make them 6 and 6, no point having clocks if we're not going to use them

static const unsigned lr_freq_lut[NUM_LRCLKS] = {
192000, 176400, 96000, 88200, 48000, 44100
};
#endif
#ifndef DATA_BITS
Expand Down Expand Up @@ -111,7 +110,7 @@ static int request_response(
return r;
}

static unsigned bclk_freq_index = 0;
static unsigned lr_freq_index = 0;
static unsigned frames_sent = 0;
static unsigned rx_data_counter[MAX_CHANNELS] = {0};
static unsigned tx_data_counter[MAX_CHANNELS] = {0};
Expand Down Expand Up @@ -150,8 +149,9 @@ i2s_restart_t i2s_restart_check(void *app_data)
i2s_restart_t restart;

frames_sent++;
if (frames_sent == 4)
if (frames_sent == 4) {
restart = I2S_RESTART;
}
else
restart = I2S_NO_RESTART;

Expand All @@ -173,15 +173,15 @@ void i2s_init(void *app_data, i2s_config_t *i2s_config)
printf("Error\n");
}

if (bclk_freq_index == NUM_BCLKS_TO_CHECK - 1) {
if (lr_freq_index == NUM_LRCLKS_TO_CHECK - 1) {
if (current_mode == I2S_MODE_I2S) {
current_mode = I2S_MODE_LEFT_JUSTIFIED;
bclk_freq_index = 0;
lr_freq_index = 0;
} else {
_Exit(1);
}
} else {
bclk_freq_index++;
lr_freq_index++;
}
}

Expand All @@ -196,7 +196,9 @@ void i2s_init(void *app_data, i2s_config_t *i2s_config)
rx_data_counter[i] = 0;
}

broadcast(bclk_freq_lut[bclk_freq_index],
unsigned bclk_freq = lr_freq_lut[lr_freq_index] * DATA_BITS * I2S_CHANS_PER_FRAME;

broadcast(bclk_freq,
NUM_IN,
NUM_OUT, DATA_BITS, i2s_config->mode == I2S_MODE_I2S);

Expand Down
1 change: 1 addition & 0 deletions test/lib_i2s/test_i2s_basic_slave.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

num_in_out_args = {
"4ch_in,4ch_out": (4, 4),
"2ch_in,2ch_out": (2, 2),
Copy link
Contributor

@ACascarino ACascarino Feb 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you'll also need to update the CMakeLists to get these to build properly, as well as the .sh files that actually do the building

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 2,2 configuration is already present in the makefile and the build_lib_i2s_tests.sh. I think they were being tested at some point before getting mysteriously removed?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh that's exciting! Carry on then 😂

"1ch_in,1ch_out": (1, 1),
"4ch_in,0ch_out": (4, 0),
"0ch_in,4ch_out": (0, 4),
Expand Down
8 changes: 7 additions & 1 deletion test/lib_i2s/test_slave_bclk_invert.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,13 @@
import pytest
import Pyxsim as px

num_in_out_args = {"2ch_in,2ch_out": (2, 2)}
num_in_out_args = {
"4ch_in,4ch_out": (4, 4),
"2ch_in,2ch_out": (2, 2),
"1ch_in,1ch_out": (1, 1),
"4ch_in,0ch_out": (4, 0),
"0ch_in,4ch_out": (0, 4),
}

bitdepth_args = {"16b": 16, "32b": 32}

Expand Down
Loading