From d5d21ab9011bf8ea019b150f2a23de8f7ecac176 Mon Sep 17 00:00:00 2001 From: Seppo Ingalsuo Date: Wed, 23 Oct 2024 17:11:01 +0300 Subject: [PATCH 1/6] Audio: Aria: Fix in generic C handling of S24_LE format The bits 31:24 are defined as don't care for S24_LE format while the code in functions aria_algo_calc_gain() and aria_algo_get_data() can work only if the bits contain the sign extension. If such data is received, the processed audio output is be corrupt. The generic C version is fixed with use of helper function sign_extend_s24(). Signed-off-by: Seppo Ingalsuo --- src/audio/aria/aria_generic.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/audio/aria/aria_generic.c b/src/audio/aria/aria_generic.c index 2739c2e4653b..93ec0f308dfe 100644 --- a/src/audio/aria/aria_generic.c +++ b/src/audio/aria/aria_generic.c @@ -23,7 +23,7 @@ inline void aria_algo_calc_gain(struct aria_data *cd, size_t gain_idx, n = audio_stream_samples_without_wrap_s32(source, src); n = MIN(samples, n); for (i = 0; i < n; i++) { - sample_abs = ABS(src[i]); + sample_abs = ABS(sign_extend_s24(src[i])); max_data = MAX(max_data, sample_abs); } @@ -71,7 +71,7 @@ static void aria_algo_get_data(struct processing_module *mod, n = MIN(m, n); for (i = 0; i < n; i += ch_n) { for (ch = 0; ch < ch_n; ch++) { - in_sample = *in++; + in_sample = sign_extend_s24(*in++); out[ch] = q_multsr_sat_32x32_24(in_sample, gain, shift); } gain += step; From c538d335365526abde8400111315de3caf097797 Mon Sep 17 00:00:00 2001 From: Seppo Ingalsuo Date: Wed, 23 Oct 2024 17:13:24 +0300 Subject: [PATCH 2/6] Audio: Aria: Fix in HiFi3 version S24_LE input data handling These changes implement the missing sign extension handling for input data. The highest 8 bits in S24_LE format are don't care. The presence of sign extension can't be assumed. The max of max_data can be moved out from while loop as a small improvement. Signed-off-by: Seppo Ingalsuo --- src/audio/aria/aria_hifi3.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/audio/aria/aria_hifi3.c b/src/audio/aria/aria_hifi3.c index 244fc88d6ced..21fe7e82fffe 100644 --- a/src/audio/aria/aria_hifi3.c +++ b/src/audio/aria/aria_hifi3.c @@ -32,16 +32,18 @@ inline void aria_algo_calc_gain(struct aria_data *cd, size_t gain_idx, inu = AE_LA64_PP(in); for (i = 0; i < m; i++) { AE_LA32X2_IP(in_sample, inu, in); - max_data = AE_MAXABS32S(max_data, in_sample); + max_data = AE_MAXABS32S(max_data, AE_SLAI32(in_sample, 8)); } if (n & 1) { AE_L32_IP(in_sample, (ae_int32 *)in, sizeof(ae_int32)); - max_data = AE_MAXABS32S(max_data, in_sample); + max_data = AE_MAXABS32S(max_data, AE_SLAI32(in_sample, 8)); } - max = MAX(max_ptr[0], max_ptr[1]); in = audio_stream_wrap(source, in); samples -= n; } + + max = MAX(max_ptr[0], max_ptr[1]) >> 8; + /*zero check for maxis not needed since att is in range <0;3>*/ if (max > (0x007fffff >> att)) gain = (0x007fffffULL << 32) / max; @@ -93,6 +95,7 @@ static void aria_algo_get_data_odd_channel(struct processing_module *mod, /*process data one by one if ch_n is odd*/ for (ch = 0; ch < ch_n; ch++) { AE_L32_XP(in_sample, (ae_int32 *)in, inc); + in_sample = AE_SRAI32(AE_SLAI32(in_sample, 8), 8); out1 = AE_MUL32_HH(in_sample, gain); out1 = AE_SRAA64(out1, shift_bits); out_sample = AE_ROUND24X2F48SSYM(out1, out1); @@ -150,6 +153,7 @@ static void aria_algo_get_data_even_channel(struct processing_module *mod, /*process 2 samples per time if ch_n is even*/ for (ch = 0; ch < ch_n; ch += 2) { AE_LA32X2_IP(in_sample, inu, in); + in_sample = AE_SRAI32(AE_SLAI32(in_sample, 8), 8); out1 = AE_MUL32_HH(in_sample, gain); out1 = AE_SRAA64(out1, shift_bits); out2 = AE_MUL32_LL(in_sample, gain); From 98ae51c7fcd0ec8c2259115a0e65a2ad8b54712d Mon Sep 17 00:00:00 2001 From: Seppo Ingalsuo Date: Thu, 24 Oct 2024 15:57:46 +0300 Subject: [PATCH 3/6] Audio: Aria: Fix HiFi5 version S24_LE input data handling These changes implement the missing sign extension handling for input data. The highest 8 bits in S24_LE format are don't care. The presence of sign extension can't be assumed. The max of the two max values can be moved out from while loop as a small improvement. Signed-off-by: Seppo Ingalsuo --- src/audio/aria/aria_hifi5.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/audio/aria/aria_hifi5.c b/src/audio/aria/aria_hifi5.c index 76184af9b0f7..ebe044ab8a48 100644 --- a/src/audio/aria/aria_hifi5.c +++ b/src/audio/aria/aria_hifi5.c @@ -47,17 +47,19 @@ inline void aria_algo_calc_gain(struct aria_data *cd, size_t gain_idx, inu = AE_LA128_PP(in); for (i = 0; i < n; i += 4) { AE_LA32X2X2_IP(in_sample, in_sample1, inu, in); - in_sample = AE_MAXABS32S(in_sample, in_sample1); - max_data = AE_MAXABS32S(max_data, in_sample); + max_data = AE_MAXABS32S(max_data, AE_SLAI32(in_sample1, 8)); + max_data = AE_MAXABS32S(max_data, AE_SLAI32(in_sample, 8)); } for (i = 0; i < left; i++) { AE_L32_IP(in_sample, (ae_int32 *)in, sizeof(ae_int32)); - max_data = AE_MAXABS32S(max_data, in_sample); + max_data = AE_MAXABS32S(max_data, AE_SLAI32(in_sample, 8)); } - max = MAX(max_ptr[0], max_ptr[1]); in = audio_stream_wrap(source, in); samples -= n; } + + max = MAX(max_ptr[0], max_ptr[1]) >> 8; + /*zero check for maxis not needed since att is in range <0;3>*/ if (max > (0x007fffff >> att)) gain = (0x007fffffULL << 32) / max; @@ -104,6 +106,7 @@ static void aria_algo_get_data_odd_channel(struct processing_module *mod, /*process data one by one if ch_n is odd*/ for (ch = 0; ch < ch_n; ch++) { AE_L32_XC(in_sample, in, inc); + in_sample = AE_SRAI32(AE_SLAI32(in_sample, 8), 8); out1 = AE_MUL32_HH(in_sample, gain); out1 = AE_SRAA64(out1, shift_bits); out_sample = AE_ROUND24X2F48SSYM(out1, out1); @@ -153,6 +156,7 @@ static void aria_algo_get_data_even_channel(struct processing_module *mod, /*process 2 samples per time if ch_n is even*/ for (ch = 0; ch < ch_n; ch += 2) { AE_L32X2_XC(in_sample, in, inc); + in_sample = AE_SRAI32(AE_SLAI32(in_sample, 8), 8); out1 = AE_MUL32_HH(in_sample, gain); out1 = AE_SRAA64(out1, shift_bits); out2 = AE_MUL32_LL(in_sample, gain); From 8cb87d929f7c0b1f71d96210b95bc40de0754d4b Mon Sep 17 00:00:00 2001 From: Seppo Ingalsuo Date: Wed, 23 Oct 2024 19:42:45 +0300 Subject: [PATCH 4/6] Audio: Aria: Update component description in kconfig The impact of parameter attenuation applied at aria_init() and in bytes control blob is explained in more clear way. Signed-off-by: Seppo Ingalsuo --- src/audio/aria/Kconfig | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/audio/aria/Kconfig b/src/audio/aria/Kconfig index b1249ccb816a..74a344588667 100644 --- a/src/audio/aria/Kconfig +++ b/src/audio/aria/Kconfig @@ -6,10 +6,11 @@ config COMP_ARIA depends on IPC_MAJOR_4 help Select for Automatic Regressive Input Amplifier Module component - ARIA applies variable gain into incoming signal. - Applied gain is in range <1, 2 power attenuation> - Currently ARIA introduces gain transition and algorithmic - latency equal to 1 ms. + ARIA applies variable gain into incoming signal. Target gain is + 0, 6, 12, 18 dB with allowed configuration parameter values + 0, 1, 2, 3. Gain is reduced from target with high level signals + to avoid clipping. Currently ARIA introduces gain transition and + algorithmic latency equal to 1 ms. choice prompt "ARIA HIFI level" From 81eb004d71ac93c150377a552fb46dea6ed7fa88 Mon Sep 17 00:00:00 2001 From: Seppo Ingalsuo Date: Wed, 23 Oct 2024 19:58:21 +0300 Subject: [PATCH 5/6] Audio: Aria: Improve aria_set_config(), warn illegal blob It depends on architecture but size_t and uint32_t are not necessarily bits compatible (on 64bit arch like testbench on x86). Therefore the type for att in component data of Aria is changed to uint32_t. A similar check as in aria_init() is added to avoid an illegal value in the blob to break the processing algorithm. An info level trace is added to see in trace if the blob is applied by the Linux topology or in run-time. Signed-off-by: Seppo Ingalsuo --- src/audio/aria/aria.c | 12 ++++++++++-- src/audio/aria/aria.h | 2 +- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/audio/aria/aria.c b/src/audio/aria/aria.c index 2d01e2baf902..7fe6e319e79a 100644 --- a/src/audio/aria/aria.c +++ b/src/audio/aria/aria.c @@ -276,16 +276,24 @@ static int aria_set_config(struct processing_module *mod, uint32_t param_id, size_t response_size) { struct aria_data *cd = module_get_private_data(mod); + struct comp_dev *dev = mod->dev; + comp_info(dev, "aria_set_config()"); if (param_id == ARIA_SET_ATTENUATION) { if (fragment_size != sizeof(uint32_t)) { - comp_err(mod->dev, "Illegal fragment_size = %d", fragment_size); + comp_err(dev, "Illegal fragment_size = %d", fragment_size); return -EINVAL; } memcpy_s(&cd->att, sizeof(uint32_t), fragment, sizeof(uint32_t)); + if (cd->att > ARIA_MAX_ATT) { + comp_warn(dev, + "aria_set_config(): Attenuation parameter %d is limited to %d", + cd->att, ARIA_MAX_ATT); + cd->att = ARIA_MAX_ATT; + } aria_set_gains(cd); } else { - comp_err(mod->dev, "Illegal param_id = %d", param_id); + comp_err(dev, "Illegal param_id = %d", param_id); return -EINVAL; } diff --git a/src/audio/aria/aria.h b/src/audio/aria/aria.h index a35acc46b152..368520b43f40 100644 --- a/src/audio/aria/aria.h +++ b/src/audio/aria/aria.h @@ -64,7 +64,7 @@ struct aria_data { /* current data position in circular buffer */ size_t buff_pos; /* Attenuation parameter */ - size_t att; + uint32_t att; /* Gain states */ int32_t gains[ARIA_MAX_GAIN_STATES]; /* cyclic buffer pointer data */ From 63921071e5c6f519a7113af89677963f8c130753 Mon Sep 17 00:00:00 2001 From: Seppo Ingalsuo Date: Thu, 24 Oct 2024 13:01:36 +0300 Subject: [PATCH 6/6] Tools: Topology2: Update configuration blobs for Aria The blob for Aria is updated for current ABI version and with options for all possible configurations. The new blobs are passthrough.conf, and blobs for parameter values 1 - 3. The benchmark topologies are configured to to use parameter value 2 (+12 dB level boost target) and parameter value 1 (+6 dB boost target). Signed-off-by: Seppo Ingalsuo --- tools/topology/topology2/cavs-benchmark-hda.conf | 6 ++++++ .../topology2/development/tplg-targets-bench.cmake | 2 +- .../topology/topology2/development/tplg-targets.cmake | 2 +- .../topology2/include/bench/aria_controls_capture.conf | 6 ++++++ .../include/bench/aria_controls_playback.conf | 6 ++++++ tools/topology/topology2/include/components/aria.conf | 8 -------- .../topology2/include/components/aria/param_1.conf | 10 ++++++++++ .../topology2/include/components/aria/param_2.conf | 10 ++++++++++ .../topology2/include/components/aria/param_3.conf | 10 ++++++++++ .../topology2/include/components/aria/passthrough.conf | 10 ++++++++++ 10 files changed, 60 insertions(+), 10 deletions(-) create mode 100644 tools/topology/topology2/include/components/aria/param_1.conf create mode 100644 tools/topology/topology2/include/components/aria/param_2.conf create mode 100644 tools/topology/topology2/include/components/aria/param_3.conf create mode 100644 tools/topology/topology2/include/components/aria/passthrough.conf diff --git a/tools/topology/topology2/cavs-benchmark-hda.conf b/tools/topology/topology2/cavs-benchmark-hda.conf index 7d6f7f38a00a..bd4aaf2229c4 100644 --- a/tools/topology/topology2/cavs-benchmark-hda.conf +++ b/tools/topology/topology2/cavs-benchmark-hda.conf @@ -161,6 +161,12 @@ IncludeByKey.BENCH_CONFIG { ] Object.Control.bytes."1" { name '$ANALOG_PLAYBACK_PCM ARIA bytes' + IncludeByKey.BENCH_ARIA_PARAMS { + "passthrough" "include/components/aria/passthrough.conf" + "param_1" "include/components/aria/param_1.conf" + "param_2" "include/components/aria/param_2.conf" + "param_3" "include/components/aria/param_3.conf" + } } } Object.Widget.mixout.1 { diff --git a/tools/topology/topology2/development/tplg-targets-bench.cmake b/tools/topology/topology2/development/tplg-targets-bench.cmake index e76aab343443..527dbfce3a20 100644 --- a/tools/topology/topology2/development/tplg-targets-bench.cmake +++ b/tools/topology/topology2/development/tplg-targets-bench.cmake @@ -42,7 +42,7 @@ set(components_s24 ) set(component_parameters_s24 - "BENCH_ARIA_PARAMS=default" + "BENCH_ARIA_PARAMS=param_2" ) # Add components with all sample formats diff --git a/tools/topology/topology2/development/tplg-targets.cmake b/tools/topology/topology2/development/tplg-targets.cmake index f5d5159afba0..d4cb68da4741 100644 --- a/tools/topology/topology2/development/tplg-targets.cmake +++ b/tools/topology/topology2/development/tplg-targets.cmake @@ -258,7 +258,7 @@ PREPROCESS_PLUGINS=nhlt,NHLT_BIN=nhlt-sof-nocodec-bt-mtl-lbm.bin" # CAVS HDA topology for benchmarking performance # Copier - peak volume - mixin - mixout - aria - peak volume - mixin - mixout - copier -"sof-hda-generic\;sof-hda-benchmark-generic\;HDA_CONFIG=benchmark,BENCH_CONFIG=benchmark" +"sof-hda-generic\;sof-hda-benchmark-generic\;HDA_CONFIG=benchmark,BENCH_CONFIG=benchmark,BENCH_ARIA_PARAMS=param_1" # Topology to test IPC4 Crossover "development/cavs-nocodec-crossover\;sof-tgl-nocodec-crossover-2way\;PLATFORM=tgl,\ diff --git a/tools/topology/topology2/include/bench/aria_controls_capture.conf b/tools/topology/topology2/include/bench/aria_controls_capture.conf index 9c5f0c476d0d..1b064a64667c 100644 --- a/tools/topology/topology2/include/bench/aria_controls_capture.conf +++ b/tools/topology/topology2/include/bench/aria_controls_capture.conf @@ -3,6 +3,12 @@ Object.Control { bytes."1" { name '$ANALOG_CAPTURE_PCM ARIA bytes' + IncludeByKey.BENCH_ARIA_PARAMS { + "passthrough" "include/components/aria/passthrough.conf" + "param_1" "include/components/aria/param_1.conf" + "param_2" "include/components/aria/param_2.conf" + "param_3" "include/components/aria/param_3.conf" + } } #mixer."1" { # name '$ANALOG_CAPTURE_PCM ARIA switch or volume' diff --git a/tools/topology/topology2/include/bench/aria_controls_playback.conf b/tools/topology/topology2/include/bench/aria_controls_playback.conf index a3ab068b5095..809e4a65c530 100644 --- a/tools/topology/topology2/include/bench/aria_controls_playback.conf +++ b/tools/topology/topology2/include/bench/aria_controls_playback.conf @@ -3,6 +3,12 @@ Object.Control { bytes."1" { name '$ANALOG_PLAYBACK_PCM ARIA bytes' + IncludeByKey.BENCH_ARIA_PARAMS { + "passthrough" "include/components/aria/passthrough.conf" + "param_1" "include/components/aria/param_1.conf" + "param_2" "include/components/aria/param_2.conf" + "param_3" "include/components/aria/param_3.conf" + } } #mixer."1" { # name '$ANALOG_PLAYBACK_PCM ARIA switch or volume' diff --git a/tools/topology/topology2/include/components/aria.conf b/tools/topology/topology2/include/components/aria.conf index 436dbab49ad9..fe34b5cd5070 100644 --- a/tools/topology/topology2/include/components/aria.conf +++ b/tools/topology/topology2/include/components/aria.conf @@ -43,14 +43,6 @@ Class.Widget."aria" { put 0 } max 4096 - Object.Base.data. 1 { - # attenuation for the aria module - bytes "0x53, 0x4f, 0x46, 0x34, 0x01, 0x00, 0x00, 0x00, - 0x4, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x08, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x01, 0x00, 0x00, 0x00" - } } # Attribute categories diff --git a/tools/topology/topology2/include/components/aria/param_1.conf b/tools/topology/topology2/include/components/aria/param_1.conf new file mode 100644 index 000000000000..bfec76526ca6 --- /dev/null +++ b/tools/topology/topology2/include/components/aria/param_1.conf @@ -0,0 +1,10 @@ +# Exported with script sof_aria_blobs.m 24-Oct-2024 +# cd tools/tune/aria; octave sof_aria_blobs.m +Object.Base.data."aria_config" { + bytes " + 0x53,0x4f,0x46,0x34,0x01,0x00,0x00,0x00, + 0x04,0x00,0x00,0x00,0x00,0xa0,0x01,0x03, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x01,0x00,0x00,0x00" +} diff --git a/tools/topology/topology2/include/components/aria/param_2.conf b/tools/topology/topology2/include/components/aria/param_2.conf new file mode 100644 index 000000000000..8cb11358a44b --- /dev/null +++ b/tools/topology/topology2/include/components/aria/param_2.conf @@ -0,0 +1,10 @@ +# Exported with script sof_aria_blobs.m 24-Oct-2024 +# cd tools/tune/aria; octave sof_aria_blobs.m +Object.Base.data."aria_config" { + bytes " + 0x53,0x4f,0x46,0x34,0x01,0x00,0x00,0x00, + 0x04,0x00,0x00,0x00,0x00,0xa0,0x01,0x03, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x02,0x00,0x00,0x00" +} diff --git a/tools/topology/topology2/include/components/aria/param_3.conf b/tools/topology/topology2/include/components/aria/param_3.conf new file mode 100644 index 000000000000..c5cbfa30cb93 --- /dev/null +++ b/tools/topology/topology2/include/components/aria/param_3.conf @@ -0,0 +1,10 @@ +# Exported with script sof_aria_blobs.m 24-Oct-2024 +# cd tools/tune/aria; octave sof_aria_blobs.m +Object.Base.data."aria_config" { + bytes " + 0x53,0x4f,0x46,0x34,0x01,0x00,0x00,0x00, + 0x04,0x00,0x00,0x00,0x00,0xa0,0x01,0x03, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x03,0x00,0x00,0x00" +} diff --git a/tools/topology/topology2/include/components/aria/passthrough.conf b/tools/topology/topology2/include/components/aria/passthrough.conf new file mode 100644 index 000000000000..1fe0ff71adf0 --- /dev/null +++ b/tools/topology/topology2/include/components/aria/passthrough.conf @@ -0,0 +1,10 @@ +# Exported with script sof_aria_blobs.m 24-Oct-2024 +# cd tools/tune/aria; octave sof_aria_blobs.m +Object.Base.data."aria_config" { + bytes " + 0x53,0x4f,0x46,0x34,0x01,0x00,0x00,0x00, + 0x04,0x00,0x00,0x00,0x00,0xa0,0x01,0x03, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00" +}