Merge pull request #104 from shuchitak/feature/fix_tests

Fix test_i2s_basic_master and test_basic_master_external_clock

test/lib_i2s/i2s_master_external_clock_test/i2s_master_external_clock_test.cmake

@@ -15,6 +15,8 @@ set(APP_LINK_OPTIONS
     -report
     -target=XCORE-AI-EXPLORER
 )
+# Compile main.c which contains the i2s_callback_group_t functions in O3 mode
+set_source_files_properties(${CMAKE_CURRENT_LIST_DIR}/src/main.c  PROPERTIES COMPILE_FLAGS "-O3")
 
 #**********************
 # Tile Targets

test/lib_i2s/i2s_master_external_clock_test/src/main.c

@@ -19,41 +19,49 @@ port_t setup_strobe_port = XS1_PORT_1L;
 port_t setup_data_port = XS1_PORT_16A;
 port_t  setup_resp_port = XS1_PORT_1M;
 
-#define MAX_RATIO 4
+#define MAX_RATIO 3 // 192, 96 and 48KHz
 
 #define MAX_CHANNELS 8
 
 #define MAX_NUM_RESTARTS (4)
 
-#if defined(SMOKE)
-#if NUM_OUT > 1 || NUM_IN > 1
+#ifndef DATA_BITS
+#define DATA_BITS 32
+#endif
+
+#if SMOKE
 #define NUM_MCLKS 1
+#if DATA_BITS == 32
+// Choose mclk freq such that mclk_bclk_ratio is atleast 2 for the worst case sampling freq (192KHz), since
+// mclk_bclk_ratio = 1 doesn't seem to be supported.
+static const unsigned mclock_freq[NUM_MCLKS] = {
+        24576000,
+};
+#elif DATA_BITS == 16
 static const unsigned mclock_freq[NUM_MCLKS] = {
         12288000,
 };
 #else
-#define NUM_MCLKS 1
+    #error "Invalid DATA_BITS define"
+#endif
+#else   // SMOKE = 0
+#define NUM_MCLKS 2
+#if DATA_BITS == 32
 static const unsigned mclock_freq[NUM_MCLKS] = {
         24576000,
+        22579200,
 };
-#endif
-#else
-#if NUM_OUT > 1 || NUM_IN > 1
-#define NUM_MCLKS 1
+#elif DATA_BITS == 16
 static const unsigned mclock_freq[NUM_MCLKS] = {
         12288000,
+        11289600,
 };
 #else
-#define NUM_MCLKS 1
-static const unsigned mclock_freq[NUM_MCLKS] = {
-        24576000,
-};
-#endif
+    #error "Invalid DATA_BITS define"
 #endif
-#ifndef DATA_BITS
-#define DATA_BITS 32
 #endif
 
+
 // Applications are expected to define this macro if they want non-32b I2S width
 #define I2S_DATA_BITS DATA_BITS
 
@@ -177,26 +185,42 @@ i2s_restart_t i2s_restart_check(void *app_data)
     return restart;
 }
 
-void i2s_init(void *app_data, i2s_config_t *i2s_config)
+void setup_bclock()
 {
-    //bclock frequency is not changed in restart when using i2s_frame_master_external_clock.
-    //The clock needs to be set externally once, before starting i2s_frame_master_external_clock.
+    mclk_bclk_ratio = (1 << ratio_log2);
+
+    broadcast(mclock_freq[mclock_freq_index],
+              mclk_bclk_ratio,
+              NUM_IN, NUM_OUT, DATA_BITS,
+              current_mode == I2S_MODE_I2S);
+
+    clock_enable(bclk);
+    clock_set_source_port(bclk, p_mclk);
+    clock_set_divide(bclk, mclk_bclk_ratio >> 1);
+}
 
+void i2s_init(void *app_data, i2s_config_t *i2s_config)
+{
     if (!first_time) {
         unsigned x = request_response(setup_strobe_port, setup_resp_port);
         error |= x;
         if (error) {
             printf("Error: test fail\n");
         }
-
-        if (current_mode == I2S_MODE_I2S) {
-            current_mode = I2S_MODE_LEFT_JUSTIFIED;
+        if (ratio_log2 == MAX_RATIO) {
+            ratio_log2 = 1;
+            if (mclock_freq_index == NUM_MCLKS - 1) {
+                mclock_freq_index = 0;
+                if (current_mode == I2S_MODE_I2S) {
+                    current_mode = I2S_MODE_LEFT_JUSTIFIED;
+                } else {
+                    _Exit(1);
+                }
+            } else {
+                mclock_freq_index++;
+            }
         } else {
-            current_mode = I2S_MODE_I2S;
-        }
-
-        if (num_restarts >= MAX_NUM_RESTARTS) {
-            _Exit(1);
+            ratio_log2++;
         }
     }
 
@@ -211,27 +235,7 @@ void i2s_init(void *app_data, i2s_config_t *i2s_config)
         rx_data_counter[i] = 0;
     }
 
-    broadcast(mclock_freq[mclock_freq_index],
-              mclk_bclk_ratio,
-              NUM_IN, NUM_OUT, DATA_BITS,
-              i2s_config->mode == I2S_MODE_I2S);
-}
-
-void setup_bclock()
-{
-    mclock_freq_index = 0;
-    ratio_log2 = 1;
-    mclk_bclk_ratio = (1 << ratio_log2);
-    current_mode = I2S_MODE_I2S;
-
-    broadcast(mclock_freq[mclock_freq_index],
-              mclk_bclk_ratio,
-              NUM_IN, NUM_OUT, DATA_BITS,
-              current_mode == I2S_MODE_I2S);
-
-    clock_enable(bclk);
-    clock_set_source_port(bclk, p_mclk);
-    clock_set_divide(bclk, mclk_bclk_ratio >> 1);
+    setup_bclock();
 }
 
 DECLARE_JOB(spin, (void));
@@ -256,8 +260,6 @@ int main(){
     port_enable(p_mclk);
     port_enable(p_bclk);
 
-    setup_bclock();
-
     PAR_JOBS (
         PJOB(i2s_master_external_clock, (
             &i_i2s,

test/lib_i2s/i2s_master_test/i2s_master_test.cmake

@@ -16,6 +16,9 @@ set(APP_LINK_OPTIONS
     -target=XCORE-AI-EXPLORER
 )
 
+# Compile main.c which contains the i2s_callback_group_t functions in O3 mode
+set_source_files_properties(${CMAKE_CURRENT_LIST_DIR}/src/main.c  PROPERTIES COMPILE_FLAGS "-O3")
+
 #**********************
 # Tile Targets
 #**********************

test/lib_i2s/i2s_master_test/src/main.c

@@ -19,42 +19,46 @@ port_t setup_strobe_port = XS1_PORT_1L;
 port_t setup_data_port = XS1_PORT_16A;
 port_t setup_resp_port = XS1_PORT_1M;
 
-#define MAX_RATIO 4
+#define MAX_RATIO 3 // 192, 96 and 48KHz
 
 #define MAX_CHANNELS 8
 
-#if defined(SMOKE)
-#if NUM_OUT > 1 || NUM_IN > 1
-#define NUM_MCLKS 1
-static const unsigned mclock_freq[NUM_MCLKS] = {
-        12288000,
-};
-#else
+#ifndef DATA_BITS
+#define DATA_BITS 32
+#endif
+
+#if SMOKE
 #define NUM_MCLKS 1
+#if DATA_BITS == 32
+// Choose mclk freq such that mclk_bclk_ratio is atleast 2 for the worst case sampling freq (192KHz), since
+// mclk_bclk_ratio = 1 doesn't seem to be supported.
 static const unsigned mclock_freq[NUM_MCLKS] = {
         24576000,
 };
-#endif
-#else
-#if NUM_OUT > 1 || NUM_IN > 1
-#define NUM_MCLKS 2
+#elif DATA_BITS == 16
 static const unsigned mclock_freq[NUM_MCLKS] = {
         12288000,
-        11289600,
 };
 #else
-#define NUM_MCLKS 4
+    #error "Invalid DATA_BITS define"
+#endif
+#else   // SMOKE = 0
+#define NUM_MCLKS 2
+#if DATA_BITS == 32
 static const unsigned mclock_freq[NUM_MCLKS] = {
         24576000,
         22579200,
+};
+#elif DATA_BITS == 16
+static const unsigned mclock_freq[NUM_MCLKS] = {
         12288000,
         11289600,
 };
+#else
+    #error "Invalid DATA_BITS define"
 #endif
 #endif
-#ifndef DATA_BITS
-#define DATA_BITS 32
-#endif
+
 
 // Applications are expected to define this macro if they want non-32b I2S width
 #define I2S_DATA_BITS DATA_BITS
@@ -173,54 +177,27 @@ i2s_restart_t i2s_restart_check(void *app_data)
 
 void i2s_init(void *app_data, i2s_config_t *i2s_config)
 {
-    /*
-     * We're going to manually disable the 16b 192 kHz 8i8o test as it does not
-     * pass in this implementation (but does in lib_i2s!). This is the first
-     * thing tested in this sequence, so must first advance ratio_log2
-     * by one to start with.
-     */
-    if (DATA_BITS == 16 && NUM_IN == 4 && NUM_OUT == 4 && first_time)
-    {
-        ratio_log2++;
-    }
-
     if (!first_time) {
         unsigned x = request_response(setup_strobe_port, setup_resp_port);
         error |= x;
         if (error) {
             printf("Error: test fail\n");
         }
 
-        int s = 0;
-        while (!s) {
-            if (ratio_log2 == MAX_RATIO) {
-                ratio_log2 = 1;
-                if (mclock_freq_index == NUM_MCLKS - 1) {
-                    mclock_freq_index = 0;
-                    if (current_mode == I2S_MODE_I2S) {
-                        current_mode = I2S_MODE_LEFT_JUSTIFIED;
-                    } else {
-                        _Exit(1);
-                    }
+        if (ratio_log2 == MAX_RATIO) {
+            ratio_log2 = 1;
+            if (mclock_freq_index == NUM_MCLKS - 1) {
+                mclock_freq_index = 0;
+                if (current_mode == I2S_MODE_I2S) {
+                    current_mode = I2S_MODE_LEFT_JUSTIFIED;
                 } else {
-                    mclock_freq_index++;
+                    _Exit(1);
                 }
             } else {
-                ratio_log2++;
-            }
-
-            uint32_t new_sample_rate = mclock_freq[mclock_freq_index] / ((1 << ratio_log2) * (2 * DATA_BITS));
-
-            if (new_sample_rate >= 48000)
-            {
-                s = 1;
-            }
-
-            // And then we need to skip the second time it comes up in testing
-            if (new_sample_rate == 192000 && DATA_BITS == 16 && NUM_IN == 4 && NUM_OUT == 4)
-            {
-                s = 0;
+                mclock_freq_index++;
             }
+        } else {
+            ratio_log2++;
         }
     }
 

test/lib_i2s/test_basic_master_external_clock.py

@@ -16,12 +16,6 @@
 
 bitdepth_args = {"16b": 16, "32b": 32}
 
-# 16b 4i4o (8ch in/8ch out) currently does not pass
-def uncollect_if(bitdepth, num_in, num_out):
-    if bitdepth == 16 and num_in == 4 and num_out == 4:
-        return True
-
-@pytest.mark.uncollect_if(func=uncollect_if)
 @pytest.mark.parametrize("bitdepth", bitdepth_args.values(), ids=bitdepth_args.keys())
 @pytest.mark.parametrize(
     ("num_in", "num_out"), num_in_out_args.values(), ids=num_in_out_args.keys()

test/lib_i2s/test_i2s_basic_master.py

@@ -76,7 +76,7 @@ def test_i2s_basic_master(build, capfd, nightly, request, bitdepth, num_in, num_
                 simthreads=[clk, checker],
                 simargs=[
                     "--vcd-tracing",
-                    f"-o i2s_trace_{num_in}_{num_out}.vcd -tile tile[0] -cycles -ports -ports-detailed -cores -instructions",
+                    f"-o i2s_trace_{num_in}_{num_out}.vcd -tile tile[0] -cycles -ports -ports-detailed -cores -instructions -clock-blocks",
                     "--trace-to",
                     f"i2s_trace_{num_in}_{num_out}.txt",
                 ],