From 601472d8174d1a665caf64c14406ff468e551b37 Mon Sep 17 00:00:00 2001 From: Long Wang Date: Tue, 29 Sep 2026 16:37:06 +0800 Subject: [PATCH 1/3] amd/acp7x: smart_amp: enable Maxim DSM for 2x max98388 on TDM2 Bring up the Maxim DSM smart amplifier path on ACP7x with two max98388 amplifiers on a 2-slot TDM2 link. smart_amp.c: - Fix source buffer detection on IPC3: bound_module is only assigned by module_bind() in module_adapter_ipc4.c and stays NULL on IPC3 platforms, so dereferencing it faulted the DSP. Compare producer pipeline ids instead -- works under both IPC versions. - Add 16-byte alignment for DSP buffers via mod_alloc_align(): without it, Xtensa HiFi AE_L32X2/AE_S64 instructions take LoadStoreAlignmentCause. - Null-guard source_buf after re-prepare to prevent stale pointer reuse. maxim_dsm_stub.c: - Fix stub to fill all output fields read by the caller. The upstream stub returned DSM_API_OK without writing omemsizerequestedbytes, so get_handle_size() returned 0, the PRIVATE memblk was never allocated, dsmhandle stayed NULL and every subsequent DSM call failed with -EINVAL. The fixed stub also passes through audio (input -> output) so the glue layer and topology can be validated without libdsm.a. smart_amp_maxim_dsm.c: - Fix DSM_FF_BUF_DB_SZ: upstream used FF_BUF_SZ * FF_MAX_CH_NUM = 192, which is too small for a 2 ms period (192 samples) + pre-fill frame (96). Change to FF_BUF_SZ * 4 = 384 to cover periods up to 3 ms. - Add ivFormat=3 (DSM_IV_FORMAT_INTERLEAVED_16_BIT_MSB) support for the 2-slot layout where each max98388 interleaves V and I within one slot using the sample MSB as a flag. The upstream 4*idx de-interleave stride reads only one MSB polarity and causes INSUFFICIENT_INPUT_DATA. - Replace memcpy_s with dsm_buf_shift_down() (memmove-based): SOF's memcpy_s detects overlapping src/dst and returns -EINVAL without copying, which occurs whenever period > 2 ms and caused silent buffer desynchronisation. - Change if to while in ff_proc and fb_proc: a 2 ms period delivers 2 DSM frames; if drained only one per call, causing w_ptr to grow until overflow. - Remove duplicate dsm_api_ff_process() call in the 32-bit else branch: libdsm decrements *ipNrSamples in place, making it a no-op today, but any code re-arming ifsamples per branch would double-advance the adaptive state (excursion/thermal model, RDC, LMS). - Set ibsamples = fb_fr_sz_samples * nchannels = 96: libdsm divides by nChannels internally; passing 48 gives 24 % ffFrameSize != 0 and returns INVALID_PARAM on every call. sof-acp_7_x_i2s.m4: - Switch TDM2 from pipe-passthrough to pipe-smart-amplifier + pipe-amp-ref-capture. - Define all required SMART_* macros for 2-channel configuration. - Add cross-pipeline SectionGraph wiring demux -> smart_amp feedback buffer. - Define REF_CHMAP = {0x01, 0x02} so the echo reference second channel comes from slot 1 (second amp), not the non-existent slot 2. - Declare SMART_UUID explicitly to make the active build mode visible. pipe-amp-ref-capture.m4: - Allow platforms to pre-define REF_CHMAP to override the default 4-slot mapping, which routes ch1 to in ch2 (non-existent on 2-slot hardware) and permanently silences ch1 via mux.c:215. app/maxim_dsm_overlay.conf: - Add Kconfig overlay for the three-step DSM bring-up sequence: step 1 PASSTHRU_AMP, step 2 MAXIM_DSM_STUB, step 3 real libdsm.a. Signed-off-by: Long Wang --- app/maxim_dsm_overlay.conf | 35 +++ src/audio/smart_amp/maxim_dsm_stub.c | 135 ++++++++++-- src/audio/smart_amp/smart_amp.c | 62 +++++- src/audio/smart_amp/smart_amp_maxim_dsm.c | 208 +++++++++++++----- tools/topology/topology1/sof-acp_7_x_i2s.m4 | 91 +++++++- .../topology1/sof/pipe-amp-ref-capture.m4 | 11 +- 6 files changed, 465 insertions(+), 77 deletions(-) create mode 100644 app/maxim_dsm_overlay.conf diff --git a/app/maxim_dsm_overlay.conf b/app/maxim_dsm_overlay.conf new file mode 100644 index 000000000000..7353ed620400 --- /dev/null +++ b/app/maxim_dsm_overlay.conf @@ -0,0 +1,35 @@ +# ACP7x smart_amp overlay -- enables Maxim DSM in three steps. +# +# Usage: +# ./scripts/xtensa-build-zephyr.py -p acp_7_x -o app/maxim_dsm_overlay.conf +# +# The three configurations (each step only flips switches in this file, +# opening them up progressively): +# +# Step 1, PASSTHRU_AMP : CONFIG_COMP_SMART_AMP=y only +# -> the Kconfig choice falls back to PASSTHRU_AMP +# -> builds smart_amp.c + smart_amp_generic.c + +# smart_amp_passthru.c +# -> does not link libdsm.a. Use this to validate the +# smart_amp framework, the topology and the duplex +# clocking. +# -> SMART_UUID in the topology must be +# passthru_smart_amp +# (64a794f0-55d3-4bca-9d5b-7b588badd037) +# +# Step 2, MAXIM_DSM_STUB : adds CONFIG_MAXIM_DSM=y + CONFIG_MAXIM_DSM_STUB=y +# -> swaps in smart_amp_maxim_dsm.c + maxim_dsm_stub.c +# -> still does not link libdsm.a (NOT STUB is false +# in CMakeLists) +# -> SMART_UUID in the topology becomes maxim_dsm +# (0cd84e80-ebd3-11ea-adc1-0242ac120002) +# +# Step 3, real libdsm.a : drops CONFIG_MAXIM_DSM_STUB +# -> zephyr_library_import takes effect and links +# src/audio/smart_amp/lib/release/dsm_lib/libdsm.a + + +CONFIG_COMP_SMART_AMP=y +CONFIG_MAXIM_DSM=y +# CONFIG_MAXIM_DSM_STUB=y + diff --git a/src/audio/smart_amp/maxim_dsm_stub.c b/src/audio/smart_amp/maxim_dsm_stub.c index 025db661c62d..eb2725d75793 100644 --- a/src/audio/smart_amp/maxim_dsm_stub.c +++ b/src/audio/smart_amp/maxim_dsm_stub.c @@ -5,11 +5,60 @@ // Author: Curtis Malainey // +/* Stub implementation of the Maxim DSM closed source API. + * + * The upstream stub returns DSM_API_OK from every function without writing + * any output fields. That satisfies the compiler (the Kconfig help text + * also says "only be used for CI and testing"), but always fails at runtime + * during comp_new: + * + * dsm_api_get_mem() never writes omemsizerequestedbytes + * -> maxim_dsm_get_handle_size() returns 0 + * -> smart_amp_alloc_mod_memblk(): the `if (ret == 0) return 0` branch + * skips both the allocation and the set_memblk() call + * -> hspk->dsmhandle remains NULL + * -> maxim_dsm_init(): `if (!hspk->dsmhandle) return -EINVAL` fires + * -> host sees `ipc tx error for 0x30010000 ... -22` + * + `Failed to setup widget SMART_AMP2.0`, card registration fails. + * + * This stub fills all output fields read back by the caller so that the + * smart_amp Maxim glue layer can fully initialise and run the data path. + * Feed-forward processing is a straight pass-through (input -> output), so + * audio is still audible under the stub, allowing the glue layer + topology + * + blob layout to be validated independently from the closed-source + * libdsm.a itself. + */ + #include "dsm_api_public.h" +/* Must match DSM_FRM_SZ in smart_amp_maxim_dsm.c: that file hard-codes + * DSM_FRM_SZ samples per frame in its ff/fb interleave/de-interleave loops, + * and a mismatch here causes sample misalignment. + */ +#define DSM_STUB_FRAME_SZ_SAMPLES 48 + +/* The real libdsm.a handle size is unknown. The stub itself does not use + * this memory; any non-zero value works. 4 KB is chosen to also exercise + * the PRIVATE memblk allocation path. + */ +#define DSM_STUB_HANDLE_SZ_BYTES 4096 + +/* maxim_dsm_get_volatile_param() iterates up to DSM_API_ADAPTIVE_PARAM_END + * (0x14); the parameter count must exceed that value or caldata overflows. + * Use the maximum value the API allows. + */ +#define DSM_STUB_MAX_NUM_PARAM DSM_DEFAULT_MAX_NUM_PARAM + enum DSM_API_MESSAGE dsm_api_get_mem(struct dsm_api_memory_size_ext_t *iopmmemparam, int iparamsize) { + if (!iopmmemparam) + return DSM_API_MSG_NULL_PARAM_POINTER; + if (iparamsize != (int)sizeof(*iopmmemparam)) + return DSM_API_MSG_INVALID_PARAM; + + iopmmemparam->omemsizerequestedbytes = DSM_STUB_HANDLE_SZ_BYTES; + return DSM_API_OK; } @@ -17,36 +66,94 @@ enum DSM_API_MESSAGE dsm_api_init(void *ipmodulehandler, struct dsm_api_init_ext_t *iopparamstruct, int iparamsize) { + if (!ipmodulehandler) + return DSM_API_MSG_NULL_MODULE_HANDLER; + if (!iopparamstruct) + return DSM_API_MSG_NULL_PARAM_POINTER; + if (iparamsize != (int)sizeof(*iopparamstruct)) + return DSM_API_MSG_INVALID_PARAM; + + /* Caller uses these two fields to compute ifsamples / ibsamples. */ + iopparamstruct->off_framesizesamples = DSM_STUB_FRAME_SZ_SAMPLES; + iopparamstruct->ofb_framesizesamples = DSM_STUB_FRAME_SZ_SAMPLES; + return DSM_API_OK; } -enum DSM_API_MESSAGE dsm_api_ff_process(void *ipmodulehandler, - int channelmask, - short *ibufferorg, - int *ipnrsamples, - short *obufferorg, - int *opnrsamples) +enum DSM_API_MESSAGE dsm_api_ff_process(void *ipmodulehandler, int channelmask, + short *ibufferorg, int *ipnrsamples, + short *obufferorg, int *opnrsamples) { + int nsamples; + int idx; + + if (!ipmodulehandler) + return DSM_API_MSG_NULL_MODULE_HANDLER; + if (!ibufferorg || !ipnrsamples) + return DSM_API_MSG_NULL_INPUT_BUFFER_POINTER; + if (!obufferorg || !opnrsamples) + return DSM_API_MSG_NULL_OUTPUT_BUFFER_POINTER; + + /* Pass-through: copy input to output unchanged so audio is audible under the stub. */ + nsamples = *ipnrsamples; + for (idx = 0; idx < nsamples; idx++) + obufferorg[idx] = ibufferorg[idx]; + + *opnrsamples = nsamples; + return DSM_API_OK; } -enum DSM_API_MESSAGE dsm_api_fb_process(void *ipmodulehandler, - int ichannelmask, - short *icurrbuffer, - short *ivoltbuffer, +enum DSM_API_MESSAGE dsm_api_fb_process(void *ipmodulehandler, int ichannelmask, + short *icurrbuffer, short *ivoltbuffer, int *iopnrsamples) { + if (!ipmodulehandler) + return DSM_API_MSG_NULL_MODULE_HANDLER; + if (!icurrbuffer || !ivoltbuffer || !iopnrsamples) + return DSM_API_MSG_NULL_INPUT_BUFFER_POINTER; + + /* Stub does no adaptation; IV feedback data is silently discarded. */ + return DSM_API_OK; } -enum DSM_API_MESSAGE dsm_api_set_params(void *ipmodulehandler, - int icommandnumber, void *ipparamsbuffer) +enum DSM_API_MESSAGE dsm_api_set_params(void *ipmodulehandler, int icommandnumber, + void *ipparamsbuffer) { + if (!ipmodulehandler) + return DSM_API_MSG_NULL_MODULE_HANDLER; + if (!ipparamsbuffer) + return DSM_API_MSG_NULL_PARAM_POINTER; + + /* Stub has no internal state; written tuning parameters are discarded. */ + return DSM_API_OK; } -enum DSM_API_MESSAGE dsm_api_get_params(void *ipmodulehandler, - int icommandnumber, void *opparams) +enum DSM_API_MESSAGE dsm_api_get_params(void *ipmodulehandler, int icommandnumber, + void *opparams) { + int *cmdblock = (int *)opparams; + + if (!ipmodulehandler) + return DSM_API_MSG_NULL_MODULE_HANDLER; + if (!cmdblock) + return DSM_API_MSG_NULL_PARAM_POINTER; + + /* Caller places the command id in cmdblock[DSM_GET_ID_IDX]; return + * values are read from CH1/CH2. The upstream stub leaves these fields + * unwritten, so the caller reads uninitialised stack data. + * maxim_dsm_get_num_param() is especially critical: it uses this result + * to size the caldata allocation. + */ + if (DSM_CH_MASK(cmdblock[DSM_GET_ID_IDX]) == DSM_API_GET_MAXIMUM_CMD_ID) { + cmdblock[DSM_GET_CH1_IDX] = DSM_STUB_MAX_NUM_PARAM; + cmdblock[DSM_GET_CH2_IDX] = DSM_STUB_MAX_NUM_PARAM; + } else { + cmdblock[DSM_GET_CH1_IDX] = 0; + cmdblock[DSM_GET_CH2_IDX] = 0; + } + return DSM_API_OK; } diff --git a/src/audio/smart_amp/smart_amp.c b/src/audio/smart_amp/smart_amp.c index 38577545b36e..52e6ca228f5f 100644 --- a/src/audio/smart_amp/smart_amp.c +++ b/src/audio/smart_amp/smart_amp.c @@ -90,10 +90,23 @@ static inline void smart_amp_free_mod_memories(struct smart_amp_data *sad) sad->mod_data = NULL; } +/* Alignment required for every buffer handed to the inner model. + * + * mod_alloc() requests no particular alignment, and on platforms built with + * CONFIG_SYS_HEAP_SMALL_ONLY=y and without CONFIG_SOF_ZEPHYR_HEAP_CACHED the + * Zephyr sys_heap chunk header is only 4 bytes, so every returned pointer is + * 4 modulo 8 (see chunk_mem() in zephyr/lib/heap/heap.c). Inner models using + * Xtensa HiFi 8-byte loads and stores (AE_L32X2 / AE_S32X2 / AE_L64 / AE_S64) + * then take an unhandled LoadStoreAlignmentCause exception. Ask for 16 bytes + * explicitly, matching the alignment the vendor libraries lay their internal + * memory blocks out on. + */ +#define SMART_AMP_BUF_ALIGN 16 + static inline int smart_amp_buf_alloc(struct processing_module *mod, struct smart_amp_buf *buf, size_t size) { - buf->data = mod_alloc(mod, size); + buf->data = mod_alloc_align(mod, size, SMART_AMP_BUF_ALIGN); if (!buf->data) return -ENOMEM; buf->size = size; @@ -739,21 +752,54 @@ static int smart_amp_prepare(struct processing_module *mod, int ret, i; comp_dbg(dev, "%d sources %d sinks", num_of_sources, num_of_sinks); + #if CONFIG_IPC_MAJOR_4 smart_amp_ipc4_params(mod); #endif /* In module API, state is managed by the framework, so no comp_set_state needed */ + + /* Neither pointer is cleared by smart_amp_reset(), so a re-prepare after + * the feedback branch was disconnected would otherwise keep using the + * stale buffer from the previous run. + */ + sad->source_buf = NULL; + sad->feedback_buf = NULL; + for (i = 0; i < num_of_sources; i++) { - /* NOTE: This should not work in module based environment: - * sources[i]->bound_module->dev->ipc_config.type == SOF_COMP_DEMUX - * So let's check which one of the sources is from a capture stream. - * The code is not tested and may not work. + struct comp_buffer *buf = comp_buffer_get_from_source(sources[i]); + struct comp_dev *producer; + + if (!buf) { + comp_err(dev, "source %d has no buffer", i); + return -ENOTCONN; + } + + /* + * Tell the feedback source from the playback one. + * + * Do not use sources[i]->bound_module here: it is only ever assigned by + * module_bind(), whose sole caller lives in module_adapter_ipc4.c, so it + * stays NULL on IPC3 platforms (e.g. AMD ACP7x) and dereferencing it + * faults the DSP. + * + * The feedback buffer is filled by the demux of the echo reference + * capture pipeline, so its producer belongs to a different pipeline than + * this module, while the playback source is filled by the host component + * of our own pipeline. Comparing pipeline ids works under both IPC + * versions, and unlike comp_dev::direction it is already valid here even + * when the capture pipeline has not been parametrised yet. */ - if (sources[i]->bound_module->dev->direction == SOF_IPC_STREAM_CAPTURE) - sad->feedback_buf = comp_buffer_get_from_source(sources[i]); + producer = comp_buffer_get_source_component(buf); + if (producer && dev_comp_pipe_id(producer) != dev_comp_pipe_id(dev)) + sad->feedback_buf = buf; else - sad->source_buf = comp_buffer_get_from_source(sources[i]); + sad->source_buf = buf; + } + + if (!sad->source_buf) { + comp_err(dev, "no playback source buffer"); + return -ENOTCONN; } /* sink buffer */ diff --git a/src/audio/smart_amp/smart_amp_maxim_dsm.c b/src/audio/smart_amp/smart_amp_maxim_dsm.c index 4b808dfaaf36..9aa9940b7dda 100644 --- a/src/audio/smart_amp/smart_amp_maxim_dsm.c +++ b/src/audio/smart_amp/smart_amp_maxim_dsm.c @@ -27,7 +27,55 @@ LOG_MODULE_DECLARE(smart_amp, CONFIG_SOF_LOG_LEVEL); #define DSM_FF_BUF_SZ (DSM_FRM_SZ * SMART_AMP_FF_MAX_CH_NUM) #define DSM_FB_BUF_SZ (DSM_FRM_SZ * SMART_AMP_FB_MAX_CH_NUM) -#define DSM_FF_BUF_DB_SZ (DSM_FF_BUF_SZ * SMART_AMP_FF_MAX_CH_NUM) +/* Capacity of the ff accumulation ring buffer. + * + * libdsm's frame size is fixed at 1 ms, so one DSM frame is DSM_FRM_SZ=48 + * samples/channel = DSM_FF_BUF_SZ=96 samples and a SOF period holds + * period_us/1000 frames: ACP7x TDM2 at 2000 us delivers 2 frames. + * + * The upstream value (DSM_FF_BUF_SZ * SMART_AMP_FF_MAX_CH_NUM = 192) fits one + * period but not also the pre-fill frame, so maxim_dsm_ff_proc's entry check + * (96 + 192 > 192) is always true: it returns -EOVERFLOW forever and never + * calls dsm_api_ff_process. It was written assuming period = 1000 us. + * + * 4 * DSM_FF_BUF_SZ = 384 holds a residual w_ptr (< 96) plus one period, and + * covers periods up to 3000 us. ff_out shares the constant; at 3000 us its + * r_ptr peaks at 384, exactly saturating the buffer. + */ +#define DSM_FF_BUF_DB_SZ (DSM_FF_BUF_SZ * 4) + +/* IV format for the feedback path. + * + * Upstream de-interleaves at 4*idx, assuming 4 TDM slots in VIVI order. This + * platform (TDM2 / 2x max98388) has only 2 slots: each max98388 + * owns one slot and alternates V and I within it using the sample MSB as the + * flag, i.e. libdsm's DSM_IV_FORMAT_INTERLEAVED_16_BIT_MSB (ivFormat=3). + * libdsm then reads the two input buffers as the left and right channel streams + * (so the "voltage"/"current" names are misleading in this mode), takes + * DSM_FRM_SZ consecutive samples from each, splits them into 24 V + 24 I by MSB + * and duplicates each for 24k -> 48k upsampling. + * + * Keeping the 4*idx stride here would read every other sample of one channel; + * as the MSB strictly alternates, that picks a single polarity and leaves the + * other class empty, making libdsm return DSM_API_MSG_INSUFFICIENT_INPUT_DATA. + */ +#define MAXIM_DSM_FB_IVFMT3 1 + +/* Command ID and IV format enum from the vendor dsm_api.h; not included in + * SOF's trimmed dsm_api_public.h. + */ +#define DSM_API_SETGET_IV_FORMAT 146 +#define DSM_IV_FORMAT_INTERLEAVED_16_BIT_MSB 3 + +/* Samples consumed from the accumulation buffer per DSM fb frame. + * ivFormat=3: DSM_FRM_SZ consecutive samples per channel x 2 physical channels. + * Upstream 4-channel VIVI: DSM_FB_BUF_SZ (= DSM_FRM_SZ * 4). + */ +#if MAXIM_DSM_FB_IVFMT3 +#define DSM_FB_FRM_SZ (DSM_FRM_SZ * 2) +#else +#define DSM_FB_FRM_SZ DSM_FB_BUF_SZ +#endif #define DSM_FB_BUF_DB_SZ (DSM_FB_BUF_SZ * SMART_AMP_FB_MAX_CH_NUM) /* DSM parameter table structure @@ -168,6 +216,22 @@ static int maxim_dsm_init(struct smart_amp_mod_struct_t *hspk) * initparam.ichannels; } +#if MAXIM_DSM_FB_IVFMT3 + /* dsm_api_init_ext_t has no ivFormat field and libdsm defaults to + * DSM_IV_FORMAT_DEINTERLEAVED_16_BIT(0), so the MSB-interleaved layout + * this platform uses must be set explicitly. Runs before + * maxim_dsm_get_all_param() so the parameter db captures it. + */ + { + int value[DSM_SET_PARAM_SZ_PAYLOAD]; + + value[DSM_SET_ID_IDX] = DSM_SET_CMD_ID(DSM_API_SETGET_IV_FORMAT); + value[DSM_SET_VALUE_IDX] = DSM_IV_FORMAT_INTERLEAVED_16_BIT_MSB; + if (dsm_api_set_params(hspk->dsmhandle, 1, value) != DSM_API_OK) + comp_err(dev, "[DSM] set ivFormat failed"); + } +#endif + comp_dbg(dev, "[DSM] Initialization completed. (module:%p, dsm:%p)", (uintptr_t)hspk, (uintptr_t)hspk->dsmhandle); @@ -468,6 +532,29 @@ static int maxim_dsm_set_config(struct smart_amp_mod_data_base *mod, return maxim_dsm_set_param(hspk, cdata); } +/* Shift the unconsumed tail of an accumulation buffer down to its base: copy + * @count samples from [@offset, @offset + @count) to [0, @count), in a buffer + * whose samples are @szsample bytes wide. + * + * memcpy_s() must not be used: SOF's implementation + * (zephyr/include/rtos/string.h) detects overlapping src/dst, returns -EINVAL + * and copies nothing. The ranges overlap as soon as count > offset, which + * every period longer than 2 ms produces on every cycle, and the call sites + * advance their write pointer regardless of the return value -- so the buffer + * would silently desynchronise. Losing memcpy_s()'s bounds check costs + * nothing: every call site passes dest_size == count, making it vacuous, and + * the invariant that matters (offset + count inside the buffer) is held by the + * overflow test each caller runs first. + */ +static void dsm_buf_shift_down(void *base, int offset, int count, int szsample) +{ + if (count <= 0) + return; + + memmove(base, (char *)base + (size_t)offset * szsample, + (size_t)count * szsample); +} + static int maxim_dsm_ff_proc(struct smart_amp_mod_data_base *mod, uint32_t frames, struct smart_amp_mod_stream *in, @@ -484,7 +571,6 @@ static int maxim_dsm_ff_proc(struct smart_amp_mod_data_base *mod, bool is_16bit = (in->frame_fmt == SOF_IPC_FRAME_S16_LE); int szsample = (is_16bit ? 2 : 4); int nsamples = frames * in->channels; - int remain; int idx; int ret = 0; @@ -518,8 +604,13 @@ static int maxim_dsm_ff_proc(struct smart_amp_mod_data_base *mod, goto error; } - /* Run DSM Feedforward process if the buffer is ready */ - if (*w_ptr >= DSM_FF_BUF_SZ) { + /* Run DSM Feedforward process if the buffer is ready. + * Use while, not if: a 2 ms SOF period contains 2 DSM frames (1 ms each); + * an if would drain only one frame per call, causing w_ptr to grow by + * DSM_FF_BUF_SZ every period and the buffer to overflow. + */ + while (*w_ptr >= DSM_FF_BUF_SZ && + *r_ptr + DSM_FF_BUF_SZ <= DSM_FF_BUF_DB_SZ) { if (is_16bit) { /* Buffer ordering for DSM : LRLR... -> LL...RR... */ for (idx = 0; idx < DSM_FRM_SZ; idx++) { @@ -535,19 +626,22 @@ static int maxim_dsm_ff_proc(struct smart_amp_mod_data_base *mod, } } - remain = (*w_ptr - DSM_FF_BUF_SZ); - if (remain) { - if (is_16bit) - memcpy_s(&buf.buf16[0], remain * szsample, - &buf.buf16[DSM_FF_BUF_SZ], - remain * szsample); - else - memcpy_s(&buf.buf32[0], remain * szsample, - &buf.buf32[DSM_FF_BUF_SZ], - remain * szsample); - } + /* Drop the DSM frame just de-interleaved out of the head of + * the accumulation buffer. Overlaps whenever w_ptr > 2 * + * DSM_FF_BUF_SZ, i.e. at any period above 2 ms; see + * dsm_buf_shift_down(). + */ + dsm_buf_shift_down(hspk->buf.ff.buf, DSM_FF_BUF_SZ, + *w_ptr - DSM_FF_BUF_SZ, szsample); *w_ptr -= DSM_FF_BUF_SZ; + /* One call covers both sample widths: input/input32 and + * output/output32 are just differently typed views of the same + * hspk->buf.input / hspk->buf.output, and the short * in the + * prototype is vestigial -- libdsm strides the buffers by + * pCommonParams->iSampleByteWidth, which dsm_api_init() derived + * from initparam.isamplebitwidth (16 -> 2 bytes, 24/32 -> 4). + */ hspk->ifsamples = hspk->nchannels * hspk->ff_fr_sz_samples; dsm_api_ff_process(hspk->dsmhandle, hspk->channelmask, input, &hspk->ifsamples, @@ -561,9 +655,6 @@ static int maxim_dsm_ff_proc(struct smart_amp_mod_data_base *mod, output[idx + DSM_FRM_SZ]; } } else { - dsm_api_ff_process(hspk->dsmhandle, hspk->channelmask, - (short *)input32, &hspk->ifsamples, - (short *)output32, &hspk->ofsamples); for (idx = 0; idx < DSM_FRM_SZ; idx++) { buf_out.buf32[*r_ptr + 2 * idx] = output32[idx]; buf_out.buf32[*r_ptr + 2 * idx + 1] = @@ -583,17 +674,14 @@ static int maxim_dsm_ff_proc(struct smart_amp_mod_data_base *mod, memcpy_s(out->buf.data, nsamples * szsample, buf_out.buf32, nsamples * szsample); - remain = (*r_ptr - nsamples); - if (remain) { - if (is_16bit) - memcpy_s(&buf_out.buf16[0], remain * szsample, - &buf_out.buf16[nsamples], - remain * szsample); - else - memcpy_s(&buf_out.buf32[0], remain * szsample, - &buf_out.buf32[nsamples], - remain * szsample); - } + /* Same construct on the output side. It does not overlap at + * the periods used today (r_ptr never exceeds 2 * nsamples), + * but that bound is an emergent property of the loop guard + * above rather than anything enforced here, so keep it on the + * overlap-safe primitive too. + */ + dsm_buf_shift_down(hspk->buf.ff_out.buf, nsamples, + *r_ptr - nsamples, szsample); *r_ptr -= nsamples; return ret; } @@ -626,7 +714,6 @@ static int maxim_dsm_fb_proc(struct smart_amp_mod_data_base *mod, bool is_16bit = (in->frame_fmt == SOF_IPC_FRAME_S16_LE); int szsample = (is_16bit ? 2 : 4); int nsamples = frames * in->channels; - int remain; int idx; buf.buf16 = (int16_t *)hspk->buf.fb.buf; @@ -654,9 +741,27 @@ static int maxim_dsm_fb_proc(struct smart_amp_mod_data_base *mod, return -EOVERFLOW; } - /* Run DSM Feedback process if the buffer is ready */ - if (*w_ptr >= DSM_FB_BUF_SZ) { + /* Run DSM Feedback process if the buffer is ready. + * Use while, not if: same reason as the ff path -- a 2 ms SOF period + * contains 2 DSM frames. + */ + while (*w_ptr >= DSM_FB_FRM_SZ) { if (is_16bit) { +#if MAXIM_DSM_FB_IVFMT3 + /* De-interleave 2-channel PCM into per-slot streams. + * In ivFormat=3, libdsm treats icurrbuffer as the left + * channel (slot 0 / amp0) and ivoltbuffer as the right + * channel (slot 1 / amp1), extracting V and I from each + * using the sample MSB. The variable names i/v are + * inherited from the upstream VIVI path and are misleading + * here: both buffers contain interleaved V+I, not pure + * current or voltage. + */ + for (idx = 0; idx < DSM_FRM_SZ; idx++) { + i[idx] = buf.buf16[2 * idx]; /* slot0 / amp0 -> left */ + v[idx] = buf.buf16[2 * idx + 1]; /* slot1 / amp1 -> right */ + } +#else for (idx = 0; idx < DSM_FRM_SZ; idx++) { /* Buffer ordering for DSM : VIVI... -> VV... II...*/ v[idx] = buf.buf16[4 * idx]; @@ -664,6 +769,7 @@ static int maxim_dsm_fb_proc(struct smart_amp_mod_data_base *mod, v[idx + DSM_FRM_SZ] = buf.buf16[4 * idx + 2]; i[idx + DSM_FRM_SZ] = buf.buf16[4 * idx + 3]; } +#endif } else { for (idx = 0; idx < DSM_FRM_SZ; idx++) { v[idx] = buf.buf32[4 * idx]; @@ -675,28 +781,30 @@ static int maxim_dsm_fb_proc(struct smart_amp_mod_data_base *mod, } } - remain = (*w_ptr - DSM_FB_BUF_SZ); - if (remain) { - if (is_16bit) - memcpy_s(&buf.buf16[0], remain * szsample, - &buf.buf16[DSM_FB_BUF_SZ], - remain * szsample); - else - memcpy_s(&buf.buf32[0], remain * szsample, - &buf.buf32[DSM_FB_BUF_SZ], - remain * szsample); - } - *w_ptr -= DSM_FB_BUF_SZ; - + /* Drop the DSM frame just de-interleaved; same overlap + * boundary as the ff path. See dsm_buf_shift_down(). + */ + dsm_buf_shift_down(hspk->buf.fb.buf, DSM_FB_FRM_SZ, + *w_ptr - DSM_FB_FRM_SZ, szsample); + *w_ptr -= DSM_FB_FRM_SZ; + + /* *ipNrSamples is the total across both buffers regardless of + * ivFormat: this path divides it by FB_DSM_channels (= 2), then + * rejects the call unless the per-channel count is a multiple of + * ffFrameSize. So it must be fb_fr_sz_samples * nchannels = 96 + * (96 / 2 = 48, 48 % 48 == 0, frameNum = 1); passing 48 gives + * 24 % 48 != 0 and returns INVALID_PARAM every call. The + * ivFormat-aware variant that skips the divide is + * DSM_API_FB_process_v2, which this path does not call. + */ hspk->ibsamples = hspk->fb_fr_sz_samples * hspk->nchannels; if (is_16bit) - dsm_api_fb_process(hspk->dsmhandle, - hspk->channelmask, + dsm_api_fb_process(hspk->dsmhandle, hspk->channelmask, i, v, &hspk->ibsamples); else - dsm_api_fb_process(hspk->dsmhandle, - hspk->channelmask, - (short *)i32, (short *)v32, &hspk->ibsamples); + dsm_api_fb_process(hspk->dsmhandle, hspk->channelmask, + (short *)i32, (short *)v32, + &hspk->ibsamples); } return 0; } diff --git a/tools/topology/topology1/sof-acp_7_x_i2s.m4 b/tools/topology/topology1/sof-acp_7_x_i2s.m4 index 6f633f700a56..1b82c163f6ce 100644 --- a/tools/topology/topology1/sof-acp_7_x_i2s.m4 +++ b/tools/topology/topology1/sof-acp_7_x_i2s.m4 @@ -137,18 +137,70 @@ dnl PCM_DUPLEX_ADD(name, pcm_id, playback_pipeline, capture_pipeline) PCM_DUPLEX_ADD(I2STDM1, 1, PIPELINE_PCM_1, PIPELINE_PCM_4) #==================================================================================================================== -# TDM instance 2 +# TDM instance 2 -- speaker path through smart_amp (2x max98388) +# +# playback: host PCM 2 --B0--> smart_amp --B1--> ACPTDM2 TX +# ^ +# B2 (IV feedback) +# | +# capture : ACPTDM2 RX --B1--> demux --B0--> host PCM 2 (echo reference) +# + +dnl Macros required by both smart_amp pipe files; either missing triggers fatal_error. +dnl Values are for a 2-channel configuration, consistent with the +dnl ACP_TDM(2, 32, 3, 3) (2 slots) used by this link. +dnl SMART_PB_PPL_ID : playback pipeline id +dnl SMART_REF_PPL_ID : echo reference capture pipeline id +dnl SMART_PB_CH_NUM : playback channel count +dnl SMART_TX_CHANNELS/SMART_RX_CHANNELS : DAI TX/RX channel count +dnl SMART_FB_CHANNELS: feedback (IV) channel count +dnl SMART_REF_CH_NUM : echo reference channel count +define(`SMART_PB_PPL_ID', 2) +define(`SMART_REF_PPL_ID', 5) +define(`SMART_PB_CH_NUM', 2) +define(`SMART_TX_CHANNELS', 2) +define(`SMART_RX_CHANNELS', 2) +define(`SMART_FB_CHANNELS', 2) +define(`SMART_REF_CH_NUM', 2) + +dnl ACP7x TDM2 has only 2 slots; each max98388 occupies one slot and +dnl interleaves V and I within that slot (ivFormat=3, MSB flag). The echo +dnl reference second channel must therefore come from in ch1 (slot 1, second +dnl amp), not from the pipe default in ch2 (which is the slot-2 position in a +dnl 4-slot layout, does not exist in a 2-channel stream, and is silently +dnl discarded by mux.c:215, permanently silencing ch1). +define(`REF_CHMAP', `0x01,0x02,0x00,0x00,0x00,0x00,0x00,0x00') + +dnl smart_amp.c registers one of two UUIDs depending on CONFIG_MAXIM_DSM: +dnl CONFIG_MAXIM_DSM=y -> maxim_dsm 0cd84e80-ebd3-11ea-adc1-0242ac120002 +dnl otherwise (PASSTHRU_AMP) -> passthru_smart_amp 64a794f0-55d3-4bca-9d5b-7b588badd037 +dnl A mismatch between the topology and the firmware causes comp_new to fail +dnl with -EINVAL (driver not found), so this must be updated whenever the +dnl Kconfig is switched. +dnl SMART_UUID must be defined before PIPELINE_PCM_ADD: the playback pipe +dnl tests `ifdef(`SMART_UUID',...)`. Defining it explicitly here (rather +dnl than relying on the pipe's own default) makes the current build mode +dnl visible at a glance in this file. +DECLARE_SOF_RT_UUID("maxim_dsm", maxim_dsm_smart_amp_uuid, 0x0cd84e80, 0xebd3, 0x11ea, 0xad, 0xc1, 0x02, 0x42, 0xac, 0x12, 0x00, 0x02) +define(`SMART_UUID', maxim_dsm_smart_amp_uuid) +dnl Passthru variant (use when CONFIG_MAXIM_DSM is not set): +dnl DECLARE_SOF_RT_UUID("passthru_smart_amp", passthru_smart_amp_uuid, 0x64a794f0, 0x55d3, 0x4bca, 0x9d, 0x5b, 0x7b, 0x58, 0x8b, 0xad, 0xd0, 0x37) +dnl define(`SMART_UUID', passthru_smart_amp_uuid) dnl PIPELINE_PCM_ADD(pipeline, dnl pipe id, pcm, max channels, format, dnl period, priority, core, dnl pcm_min_rate, pcm_max_rate, pipeline_rate) -PIPELINE_PCM_ADD(sof/pipe-passthrough-playback.m4, +dnl Playback uses the upstream pipe; SMART_UUID defined above selects the Maxim +dnl DSM build of smart_amp. +PIPELINE_PCM_ADD(sof/pipe-smart-amplifier-playback.m4, 2, 2, 2, s16le, 2000, 0, 0, 48000, 48000, 48000) -PIPELINE_PCM_ADD(sof/pipe-passthrough-capture.m4, + +dnl Capture uses the upstream original (the reference project has no AMD-specific version). +PIPELINE_PCM_ADD(sof/pipe-amp-ref-capture.m4, 5, 2, 2, s16le, 2000, 0, 0, 48000, 48000, 48000) @@ -158,6 +210,9 @@ dnl pipe id, dai type, dai_index, dai_be, dnl buffer, periods, format, dnl deadline, priority, core, time_domain) +dnl Keep SCHEDULE_TIME_DOMAIN_TIMER as used by the existing ACP7x platform. +dnl The reference project uses DMA scheduling, which is an older ACPHS +dnl platform difference that does not apply here. DAI_ADD(sof/pipe-dai-playback.m4, 2, ACPTDM, 2, acp-i2s2-codec, PIPELINE_SOURCE_2, 2, s16le, @@ -180,8 +235,38 @@ DAI_CONFIG(ACPTDM, 2, 2, acp-i2s2-codec, ACP_CLOCK(fsync, 48000, codec_consumer), ACP_TDM(2, 32, 3, 3), ACPTDM_CONFIG_DATA(ACPTDM, 2, 48000, 2, 0))) +dnl Wire the capture pipeline's demux output into the smart_amp feedback buffer +dnl in the playback pipeline. This cross-pipeline connection cannot be +dnl expressed in a single pipe file and must be added here. +dnl N_SMART_REF_BUF is defined by the playback pipe (BUF2.2); +dnl N_SMART_DEMUX is defined by the capture pipe. +ifdef(`N_SMART_REF_BUF',`',`fatal_error(note: N_SMART_REF_BUF undefined - playback pipe missing +)') +ifdef(`N_SMART_DEMUX',`',`fatal_error(note: N_SMART_DEMUX undefined - ref capture pipe missing +)') +SectionGraph."PIPE_SMART_AMP" { + index "0" + + lines [ + # demux -> smart_amp feedback + dapm(N_SMART_REF_BUF, N_SMART_DEMUX) + ] +} + dnl PCM_DUPLEX_ADD(name, pcm_id, playback_pipeline, capture_pipeline) +dnl Keep the name I2STDM2 to avoid disturbing host-side UCM / existing test commands. PCM_DUPLEX_ADD(I2STDM2, 2, PIPELINE_PCM_2, PIPELINE_PCM_5) + +dnl Clean up macros introduced by this section. +undefine(`SMART_PB_PPL_ID') +undefine(`SMART_REF_PPL_ID') +undefine(`SMART_PB_CH_NUM') +undefine(`SMART_TX_CHANNELS') +undefine(`SMART_RX_CHANNELS') +undefine(`SMART_FB_CHANNELS') +undefine(`SMART_REF_CH_NUM') +undefine(`REF_CHMAP') +undefine(`SMART_UUID') #==================================================================================================================== DEBUG_END diff --git a/tools/topology/topology1/sof/pipe-amp-ref-capture.m4 b/tools/topology/topology1/sof/pipe-amp-ref-capture.m4 index f605496144bf..eb4468dbd9ec 100644 --- a/tools/topology/topology1/sof/pipe-amp-ref-capture.m4 +++ b/tools/topology/topology1/sof/pipe-amp-ref-capture.m4 @@ -43,10 +43,17 @@ ifelse(SMART_FB_CHANNELS, `8', `define(`FB_CHMAP',`0x01,0x02,0x04,0x08,0x00,0x00,0x00,0x00')' ) -ifelse(SMART_REF_CH_NUM, `4', +dnl REF_CHMAP may be pre-defined by the platform m4 to override the default. +dnl The default non-4-channel branch maps out ch1 to in ch2, which is correct +dnl for 4-slot VIVI-interleaved hardware. On platforms with only 2 TDM slots +dnl where V and I are interleaved within the same slot (e.g. AMD ACP7x with +dnl 2x max98388), the second amp's data sits in slot 1, not slot 2; using the +dnl default would map out ch1 to a non-existent in ch2 which mux.c silently +dnl discards, making ch1 permanently silent. +ifdef(`REF_CHMAP',`',`ifelse(SMART_REF_CH_NUM, `4', `define(`REF_CHMAP',`0x01,0x02,0x04,0x08,0x00,0x00,0x00,0x00')', `define(`REF_CHMAP',`0x01,0x04,0x00,0x00,0x00,0x00,0x00,0x00')' -) +)') # should be aligned with struct sof_mux_config, used for mux input/output configuration. From 787debb67a47883f2dc90aa3d0a9cf96c32bed92 Mon Sep 17 00:00:00 2001 From: Long Wang Date: Fri, 16 Oct 2026 16:04:31 +0800 Subject: [PATCH 2/3] topology1: amd/acp7x: build one TDM2 tplg per smart_amp configuration The previous commit put smart_amp into sof-acp_7_x_i2s.m4 unconditionally, which silently changed what sof-acp7x.tplg means: it now needs a firmware built with CONFIG_MAXIM_DSM, and comp_new fails with -EINVAL on any other. The firmware has three distinct smart_amp configurations, so build a tplg for each: sof-acp7x.tplg no smart_amp widget on TDM2 at all; runs on any firmware sof-acp7x-smart-amp-passthru.tplg smart_amp with the passthru_smart_amp UUID, for CONFIG_COMP_SMART_AMP alone sof-acp7x-smart-amp.tplg same pipeline with the maxim_dsm UUID, for CONFIG_MAXIM_DSM=y The three steps in app/maxim_dsm_overlay.conf now map to a file name rather than to a source edit. The selector is SMART_UUID, the macro the playback pipe already reads to pick a solution, so no second switch is needed; declaring both UUIDs costs nothing because DECLARE_SOF_RT_UUID only defines an m4 macro and emits no output. The upstream TDM2 block stays verbatim as the undefined-SMART_UUID branch, so this file gains 15 lines and loses none relative to upstream; only the smart_amp form lives in its own file under platform/amd/. Also: - CMakeLists.txt: add a per-input ${input}_M4_DEPS to DEPENDS. MAIN_DEPENDENCY only tracks the top-level .m4, so editing the include()d file would otherwise leave a stale .tplg in production/ and the next build would install it. - acp_7_x-tdm2-smart-amp.m4: guard SMART_UUID, N_SMART_REF_BUF and N_SMART_DEMUX with errprint()+m4exit(1). fatal_error(), used for this throughout topology1, is defined nowhere in the tree: it expands to its own name as a literal and leaves m4 exiting 0. - sof/pipe-amp-ref-capture.m4: condense the REF_CHMAP comment. No functional change. Verified on ACP7x with 2x max98388 on TDM2: each tplg was booted against both a CONFIG_COMP_SMART_AMP-only and a CONFIG_MAXIM_DSM firmware, and only the matching pairs load. sof-acp7x.tplg and sof-acp7x-24bit.tplg are unchanged. Signed-off-by: Long Wang --- tools/topology/topology1/CMakeLists.txt | 10 +- .../platform/amd/acp_7_x-tdm2-smart-amp.m4 | 132 ++++++++++++++++++ tools/topology/topology1/sof-acp_7_x_i2s.m4 | 106 +++----------- .../topology1/sof/pipe-amp-ref-capture.m4 | 10 +- 4 files changed, 163 insertions(+), 95 deletions(-) create mode 100644 tools/topology/topology1/platform/amd/acp_7_x-tdm2-smart-amp.m4 diff --git a/tools/topology/topology1/CMakeLists.txt b/tools/topology/topology1/CMakeLists.txt index 95d81a01629f..4c75bd68a7bb 100644 --- a/tools/topology/topology1/CMakeLists.txt +++ b/tools/topology/topology1/CMakeLists.txt @@ -124,6 +124,8 @@ set(TPLGS "sof-acp_7_0_sdw\;sof-acp_7_0-rt722-l0" "sof-acp_7_x_sdw\;sof-acp7x-rt721-l0" "sof-acp_7_x_i2s\;sof-acp7x" + "sof-acp_7_x_i2s\;sof-acp7x-smart-amp\;-DSMART_UUID=maxim_dsm_smart_amp_uuid" + "sof-acp_7_x_i2s\;sof-acp7x-smart-amp-passthru\;-DSMART_UUID=passthru_smart_amp_uuid" "sof-acp_7_x_i2s_24bit\;sof-acp7x-24bit" ) @@ -139,6 +141,12 @@ add_custom_target(topologies1 ALL COMMAND ${CMAKE_COMMAND} -E copy_directory production ${SOF_TOPOLOGY_BINARY_DIRECTORY} ) +# Extra m4 include() dependencies, per input topology. MAIN_DEPENDENCY below +# only tracks the top-level .m4, so a topology that include()s another file must +# list it here or editing that file leaves a stale .tplg behind. +set(sof-acp_7_x_i2s_M4_DEPS + ${CMAKE_CURRENT_SOURCE_DIR}/platform/amd/acp_7_x-tdm2-smart-amp.m4) + foreach(tplg ${TPLGS}) list(GET tplg 0 input) list(GET tplg 1 output) @@ -171,7 +179,7 @@ foreach(tplg ${TPLGS}) # a rebuild. 'abi_v1' is the target that avoids a race # to rebuild abi.h. See the CMake FAQ or (better) Sam # Thursfield's blog about custom targets. - DEPENDS abi_v1 ${CMAKE_CURRENT_BINARY_DIR}/abi.h + DEPENDS abi_v1 ${CMAKE_CURRENT_BINARY_DIR}/abi.h ${${input}_M4_DEPS} VERBATIM USES_TERMINAL ) diff --git a/tools/topology/topology1/platform/amd/acp_7_x-tdm2-smart-amp.m4 b/tools/topology/topology1/platform/amd/acp_7_x-tdm2-smart-amp.m4 new file mode 100644 index 000000000000..fe53bb495404 --- /dev/null +++ b/tools/topology/topology1/platform/amd/acp_7_x-tdm2-smart-amp.m4 @@ -0,0 +1,132 @@ +#==================================================================================================================== +# TDM instance 2 -- speaker path through smart_amp (2x max98388) +# +# playback: host PCM 2 --B0--> smart_amp --B1--> ACPTDM2 TX +# ^ +# B2 (IV feedback) +# | +# capture : ACPTDM2 RX --B1--> demux --B0--> host PCM 2 (echo reference) +# + +dnl Macros required by both smart_amp pipe files; either missing aborts m4. +dnl Values are for a 2-channel configuration, consistent with the +dnl ACP_TDM(2, 32, 3, 3) (2 slots) used by this link. +dnl SMART_PB_PPL_ID : playback pipeline id +dnl SMART_REF_PPL_ID : echo reference capture pipeline id +dnl SMART_PB_CH_NUM : playback channel count +dnl SMART_TX_CHANNELS/SMART_RX_CHANNELS : DAI TX/RX channel count +dnl SMART_FB_CHANNELS: feedback (IV) channel count +dnl SMART_REF_CH_NUM : echo reference channel count +define(`SMART_PB_PPL_ID', 2) +define(`SMART_REF_PPL_ID', 5) +define(`SMART_PB_CH_NUM', 2) +define(`SMART_TX_CHANNELS', 2) +define(`SMART_RX_CHANNELS', 2) +define(`SMART_FB_CHANNELS', 2) +define(`SMART_REF_CH_NUM', 2) + +dnl ACP7x TDM2 has only 2 slots; each max98388 occupies one slot and +dnl interleaves V and I within that slot (ivFormat=3, MSB flag). The echo +dnl reference second channel must therefore come from in ch1 (slot 1, second +dnl amp), not from the pipe default in ch2 (which is the slot-2 position in a +dnl 4-slot layout, does not exist in a 2-channel stream, and is silently +dnl discarded by mux.c:215, permanently silencing ch1). +define(`REF_CHMAP', `0x01,0x02,0x00,0x00,0x00,0x00,0x00,0x00') + +dnl smart_amp.c registers one of two UUIDs depending on CONFIG_MAXIM_DSM: +dnl CONFIG_MAXIM_DSM=y -> maxim_dsm 0cd84e80-ebd3-11ea-adc1-0242ac120002 +dnl otherwise (PASSTHRU_AMP) -> passthru_smart_amp 64a794f0-55d3-4bca-9d5b-7b588badd037 +dnl Both are declared here: DECLARE_SOF_RT_UUID only defines an m4 macro and +dnl emits nothing, so the unused one costs nothing. SMART_UUID picks one, and +dnl the playback pipe hands it to W_SMART_AMP; a tplg whose UUID does not match +dnl the firmware fails comp_new with -EINVAL (driver not found). +dnl +dnl SMART_UUID arrives from CMakeLists.txt, where it also serves as the switch +dnl that includes this file instead of the passthrough TDM2, so the TPLGS entry +dnl is the one place that says which firmware a given tplg expects. +ifdef(`SMART_UUID',`',`errprint(note: SMART_UUID undefined - this file is selected by it; see tools/topology/topology1/CMakeLists.txt +)m4exit(1)')dnl +DECLARE_SOF_RT_UUID("maxim_dsm", maxim_dsm_smart_amp_uuid, 0x0cd84e80, 0xebd3, 0x11ea, 0xad, 0xc1, 0x02, 0x42, 0xac, 0x12, 0x00, 0x02) +DECLARE_SOF_RT_UUID("passthru_smart_amp", passthru_smart_amp_uuid, 0x64a794f0, 0x55d3, 0x4bca, 0x9d, 0x5b, 0x7b, 0x58, 0x8b, 0xad, 0xd0, 0x37) + +dnl PIPELINE_PCM_ADD(pipeline, +dnl pipe id, pcm, max channels, format, +dnl period, priority, core, +dnl pcm_min_rate, pcm_max_rate, pipeline_rate) + +dnl Playback uses the upstream pipe; SMART_UUID selects the Maxim DSM build of +dnl smart_amp. +PIPELINE_PCM_ADD(sof/pipe-smart-amplifier-playback.m4, + 2, 2, 2, s16le, + 2000, 0, 0, + 48000, 48000, 48000) + +dnl Capture uses the generic echo reference pipe as-is; nothing in it needs a +dnl platform-specific variant. +PIPELINE_PCM_ADD(sof/pipe-amp-ref-capture.m4, + 5, 2, 2, s16le, + 2000, 0, 0, + 48000, 48000, 48000) + +dnl DAI_ADD(pipeline, +dnl pipe id, dai type, dai_index, dai_be, +dnl buffer, periods, format, +dnl deadline, priority, core, time_domain) + +dnl Keep SCHEDULE_TIME_DOMAIN_TIMER, as TDM0 and TDM1 on this platform do. +dnl Older ACPHS-style links schedule off the DMA instead; ACP7x does not. +DAI_ADD(sof/pipe-dai-playback.m4, + 2, ACPTDM, 2, acp-i2s2-codec, + PIPELINE_SOURCE_2, 2, s16le, + 2000, 0, 0, SCHEDULE_TIME_DOMAIN_TIMER) + +DAI_ADD(sof/pipe-dai-capture.m4, + 5, ACPTDM, 2, acp-i2s2-codec, + PIPELINE_SINK_5, 2, s16le, + 2000, 0, 0, SCHEDULE_TIME_DOMAIN_TIMER) + +dnl DAI_CONFIG(type, dai_index, link_id, name, ACPTDM_config/acpdmic_config) +dnl ACPTDM_CONFIG(format, mclk, bclk, fsync, tdm, ACPTDM_config_data) +dnl ACP_CLOCK(clock, freq, codec_provider, polarity) +dnl ACPTDM_CONFIG_DATA(type, idx, valid bits, mclk_id) +dnl mclk_id is optional + +DAI_CONFIG(ACPTDM, 2, 2, acp-i2s2-codec, + ACPTDM_CONFIG(I2S, ACP_CLOCK(mclk, 49152000, codec_mclk_in), + ACP_CLOCK(bclk, 3072000, codec_consumer), + ACP_CLOCK(fsync, 48000, codec_consumer), + ACP_TDM(2, 32, 3, 3), ACPTDM_CONFIG_DATA(ACPTDM, 2, 48000, 2, 0))) + +dnl Wire the capture pipeline's demux output into the smart_amp feedback buffer +dnl in the playback pipeline. This cross-pipeline connection cannot be +dnl expressed in a single pipe file and must be added here. +dnl N_SMART_REF_BUF is defined by the playback pipe (BUF2.2); +dnl N_SMART_DEMUX is defined by the capture pipe. +ifdef(`N_SMART_REF_BUF',`',`errprint(note: N_SMART_REF_BUF undefined - playback pipe missing +)m4exit(1)') +ifdef(`N_SMART_DEMUX',`',`errprint(note: N_SMART_DEMUX undefined - ref capture pipe missing +)m4exit(1)') +SectionGraph."PIPE_SMART_AMP" { + index "0" + + lines [ + # demux -> smart_amp feedback + dapm(N_SMART_REF_BUF, N_SMART_DEMUX) + ] +} + +dnl PCM_DUPLEX_ADD(name, pcm_id, playback_pipeline, capture_pipeline) +dnl Keep the name I2STDM2 to avoid disturbing host-side UCM / existing test commands. +PCM_DUPLEX_ADD(I2STDM2, 2, PIPELINE_PCM_2, PIPELINE_PCM_5) + +dnl Clean up macros introduced by this section. +undefine(`SMART_PB_PPL_ID') +undefine(`SMART_REF_PPL_ID') +undefine(`SMART_PB_CH_NUM') +undefine(`SMART_TX_CHANNELS') +undefine(`SMART_RX_CHANNELS') +undefine(`SMART_FB_CHANNELS') +undefine(`SMART_REF_CH_NUM') +undefine(`REF_CHMAP') +undefine(`SMART_UUID') +#==================================================================================================================== diff --git a/tools/topology/topology1/sof-acp_7_x_i2s.m4 b/tools/topology/topology1/sof-acp_7_x_i2s.m4 index 1b82c163f6ce..9e9591a4dc61 100644 --- a/tools/topology/topology1/sof-acp_7_x_i2s.m4 +++ b/tools/topology/topology1/sof-acp_7_x_i2s.m4 @@ -136,71 +136,33 @@ DAI_CONFIG(ACPTDM, 1, 1, acp-i2s1-codec, dnl PCM_DUPLEX_ADD(name, pcm_id, playback_pipeline, capture_pipeline) PCM_DUPLEX_ADD(I2STDM1, 1, PIPELINE_PCM_1, PIPELINE_PCM_4) +dnl TDM instance 2 is the speaker path, and it comes in two forms. The default +dnl is the passthrough below, same as TDM0 and TDM1. Defining SMART_UUID runs +dnl playback through that smart_amp solution instead, with an IV feedback branch +dnl off an echo reference capture pipeline; that needs a firmware built with the +dnl matching CONFIG_*_SMART_AMP (see app/maxim_dsm_overlay.conf). SMART_UUID is +dnl the macro the playback pipe already reads to pick a solution, so it doubles +dnl as the switch here rather than adding a second one. CMakeLists.txt builds a +dnl tplg for each. +dnl +dnl The block below is an m4 argument now, so keep it free of apostrophes and +dnl backticks: either one would end the quote silently and the failure surfaces +dnl much later, as an alsatplg syntax error. The smart_amp form carries prose +dnl comments for that reason and lives in its own file. +ifdef(`SMART_UUID',`include(`platform/amd/acp_7_x-tdm2-smart-amp.m4')',`dnl #==================================================================================================================== -# TDM instance 2 -- speaker path through smart_amp (2x max98388) -# -# playback: host PCM 2 --B0--> smart_amp --B1--> ACPTDM2 TX -# ^ -# B2 (IV feedback) -# | -# capture : ACPTDM2 RX --B1--> demux --B0--> host PCM 2 (echo reference) -# - -dnl Macros required by both smart_amp pipe files; either missing triggers fatal_error. -dnl Values are for a 2-channel configuration, consistent with the -dnl ACP_TDM(2, 32, 3, 3) (2 slots) used by this link. -dnl SMART_PB_PPL_ID : playback pipeline id -dnl SMART_REF_PPL_ID : echo reference capture pipeline id -dnl SMART_PB_CH_NUM : playback channel count -dnl SMART_TX_CHANNELS/SMART_RX_CHANNELS : DAI TX/RX channel count -dnl SMART_FB_CHANNELS: feedback (IV) channel count -dnl SMART_REF_CH_NUM : echo reference channel count -define(`SMART_PB_PPL_ID', 2) -define(`SMART_REF_PPL_ID', 5) -define(`SMART_PB_CH_NUM', 2) -define(`SMART_TX_CHANNELS', 2) -define(`SMART_RX_CHANNELS', 2) -define(`SMART_FB_CHANNELS', 2) -define(`SMART_REF_CH_NUM', 2) - -dnl ACP7x TDM2 has only 2 slots; each max98388 occupies one slot and -dnl interleaves V and I within that slot (ivFormat=3, MSB flag). The echo -dnl reference second channel must therefore come from in ch1 (slot 1, second -dnl amp), not from the pipe default in ch2 (which is the slot-2 position in a -dnl 4-slot layout, does not exist in a 2-channel stream, and is silently -dnl discarded by mux.c:215, permanently silencing ch1). -define(`REF_CHMAP', `0x01,0x02,0x00,0x00,0x00,0x00,0x00,0x00') - -dnl smart_amp.c registers one of two UUIDs depending on CONFIG_MAXIM_DSM: -dnl CONFIG_MAXIM_DSM=y -> maxim_dsm 0cd84e80-ebd3-11ea-adc1-0242ac120002 -dnl otherwise (PASSTHRU_AMP) -> passthru_smart_amp 64a794f0-55d3-4bca-9d5b-7b588badd037 -dnl A mismatch between the topology and the firmware causes comp_new to fail -dnl with -EINVAL (driver not found), so this must be updated whenever the -dnl Kconfig is switched. -dnl SMART_UUID must be defined before PIPELINE_PCM_ADD: the playback pipe -dnl tests `ifdef(`SMART_UUID',...)`. Defining it explicitly here (rather -dnl than relying on the pipe's own default) makes the current build mode -dnl visible at a glance in this file. -DECLARE_SOF_RT_UUID("maxim_dsm", maxim_dsm_smart_amp_uuid, 0x0cd84e80, 0xebd3, 0x11ea, 0xad, 0xc1, 0x02, 0x42, 0xac, 0x12, 0x00, 0x02) -define(`SMART_UUID', maxim_dsm_smart_amp_uuid) -dnl Passthru variant (use when CONFIG_MAXIM_DSM is not set): -dnl DECLARE_SOF_RT_UUID("passthru_smart_amp", passthru_smart_amp_uuid, 0x64a794f0, 0x55d3, 0x4bca, 0x9d, 0x5b, 0x7b, 0x58, 0x8b, 0xad, 0xd0, 0x37) -dnl define(`SMART_UUID', passthru_smart_amp_uuid) +# TDM instance 2 dnl PIPELINE_PCM_ADD(pipeline, dnl pipe id, pcm, max channels, format, dnl period, priority, core, dnl pcm_min_rate, pcm_max_rate, pipeline_rate) -dnl Playback uses the upstream pipe; SMART_UUID defined above selects the Maxim -dnl DSM build of smart_amp. -PIPELINE_PCM_ADD(sof/pipe-smart-amplifier-playback.m4, +PIPELINE_PCM_ADD(sof/pipe-passthrough-playback.m4, 2, 2, 2, s16le, 2000, 0, 0, 48000, 48000, 48000) - -dnl Capture uses the upstream original (the reference project has no AMD-specific version). -PIPELINE_PCM_ADD(sof/pipe-amp-ref-capture.m4, +PIPELINE_PCM_ADD(sof/pipe-passthrough-capture.m4, 5, 2, 2, s16le, 2000, 0, 0, 48000, 48000, 48000) @@ -210,9 +172,6 @@ dnl pipe id, dai type, dai_index, dai_be, dnl buffer, periods, format, dnl deadline, priority, core, time_domain) -dnl Keep SCHEDULE_TIME_DOMAIN_TIMER as used by the existing ACP7x platform. -dnl The reference project uses DMA scheduling, which is an older ACPHS -dnl platform difference that does not apply here. DAI_ADD(sof/pipe-dai-playback.m4, 2, ACPTDM, 2, acp-i2s2-codec, PIPELINE_SOURCE_2, 2, s16le, @@ -235,38 +194,9 @@ DAI_CONFIG(ACPTDM, 2, 2, acp-i2s2-codec, ACP_CLOCK(fsync, 48000, codec_consumer), ACP_TDM(2, 32, 3, 3), ACPTDM_CONFIG_DATA(ACPTDM, 2, 48000, 2, 0))) -dnl Wire the capture pipeline's demux output into the smart_amp feedback buffer -dnl in the playback pipeline. This cross-pipeline connection cannot be -dnl expressed in a single pipe file and must be added here. -dnl N_SMART_REF_BUF is defined by the playback pipe (BUF2.2); -dnl N_SMART_DEMUX is defined by the capture pipe. -ifdef(`N_SMART_REF_BUF',`',`fatal_error(note: N_SMART_REF_BUF undefined - playback pipe missing -)') -ifdef(`N_SMART_DEMUX',`',`fatal_error(note: N_SMART_DEMUX undefined - ref capture pipe missing -)') -SectionGraph."PIPE_SMART_AMP" { - index "0" - - lines [ - # demux -> smart_amp feedback - dapm(N_SMART_REF_BUF, N_SMART_DEMUX) - ] -} - dnl PCM_DUPLEX_ADD(name, pcm_id, playback_pipeline, capture_pipeline) -dnl Keep the name I2STDM2 to avoid disturbing host-side UCM / existing test commands. PCM_DUPLEX_ADD(I2STDM2, 2, PIPELINE_PCM_2, PIPELINE_PCM_5) - -dnl Clean up macros introduced by this section. -undefine(`SMART_PB_PPL_ID') -undefine(`SMART_REF_PPL_ID') -undefine(`SMART_PB_CH_NUM') -undefine(`SMART_TX_CHANNELS') -undefine(`SMART_RX_CHANNELS') -undefine(`SMART_FB_CHANNELS') -undefine(`SMART_REF_CH_NUM') -undefine(`REF_CHMAP') -undefine(`SMART_UUID') #==================================================================================================================== +')dnl DEBUG_END diff --git a/tools/topology/topology1/sof/pipe-amp-ref-capture.m4 b/tools/topology/topology1/sof/pipe-amp-ref-capture.m4 index eb4468dbd9ec..426d48b62337 100644 --- a/tools/topology/topology1/sof/pipe-amp-ref-capture.m4 +++ b/tools/topology/topology1/sof/pipe-amp-ref-capture.m4 @@ -44,12 +44,10 @@ ifelse(SMART_FB_CHANNELS, `8', ) dnl REF_CHMAP may be pre-defined by the platform m4 to override the default. -dnl The default non-4-channel branch maps out ch1 to in ch2, which is correct -dnl for 4-slot VIVI-interleaved hardware. On platforms with only 2 TDM slots -dnl where V and I are interleaved within the same slot (e.g. AMD ACP7x with -dnl 2x max98388), the second amp's data sits in slot 1, not slot 2; using the -dnl default would map out ch1 to a non-existent in ch2 which mux.c silently -dnl discards, making ch1 permanently silent. +dnl The default non-4-channel branch maps out ch1 to in ch2, which suits 4-slot +dnl VIVI hardware. With only 2 TDM slots and V/I interleaved inside each slot, +dnl the second amp sits in slot 1, so the default maps out ch1 to a +dnl non-existent in ch2 that mux.c silently discards, leaving ch1 silent. ifdef(`REF_CHMAP',`',`ifelse(SMART_REF_CH_NUM, `4', `define(`REF_CHMAP',`0x01,0x02,0x04,0x08,0x00,0x00,0x00,0x00')', `define(`REF_CHMAP',`0x01,0x04,0x00,0x00,0x00,0x00,0x00,0x00')' From 7bffa6e79e584b10827ffc845b0c3eac446a52d9 Mon Sep 17 00:00:00 2001 From: Long Wang Date: Fri, 16 Oct 2026 16:45:10 +0800 Subject: [PATCH 3/3] audio: smart_amp: size the mod buffers by period, not channel count The generic-layer remap buffers were sized as SMART_AMP_FF_BUF_SZ * SMART_AMP_FF_MAX_CH_NUM, but SMART_AMP_FF_BUF_SZ already carries the channel count: the second factor is a dimensional mistake, and the 192 samples it yields hold one 2 ms period only by coincidence. The Maxim inner model worked around that with a ring of its own, capped at 3 ms, and nothing checked the scheduled period against either limit, so a longer one failed deep in the data path instead of at prepare. Replace both with one knob, SMART_AMP_MAX_PERIOD_MS = 5. It is bounded by the inner models' 1 ms frame at 48 kHz, since a fractional period leaves a residual that walks the read pointer down until it underruns, and by the heap cost, roughly 3 kB per instance per millisecond. smart_amp.c now derives ff_max_frames / fb_max_frames in prepare(), clears them there as reset() does not, rejects a zero-channel source before the divide, warns when the scheduled period exceeds the cap, and caps avail_frames in process() instead of rejecting the whole call and emitting a period of silence. smart_amp_maxim_dsm.c derives its ring sizes from the generic limit plus the one resident frame each path carries. Also pass the real destination size to memcpy_s(): all four accumulation copies passed the copy length as dest_size, making the bounds check vacuous. Verified on ACP7x with 2x max98388 on TDM2 at 48 kHz s16le 2 ch: periods of 2, 3, 4 and 5 ms all play and capture IV feedback cleanly over both aplay and PipeWire. The topology remains at 2000 us. Signed-off-by: Long Wang --- src/audio/smart_amp/maxim_dsm_stub.c | 52 +++--- src/audio/smart_amp/smart_amp.c | 97 +++++++----- src/audio/smart_amp/smart_amp_generic.c | 15 +- src/audio/smart_amp/smart_amp_maxim_dsm.c | 167 +++++++++----------- src/include/sof/audio/smart_amp/smart_amp.h | 15 +- 5 files changed, 166 insertions(+), 180 deletions(-) diff --git a/src/audio/smart_amp/maxim_dsm_stub.c b/src/audio/smart_amp/maxim_dsm_stub.c index eb2725d75793..a66e56120ae8 100644 --- a/src/audio/smart_amp/maxim_dsm_stub.c +++ b/src/audio/smart_amp/maxim_dsm_stub.c @@ -5,47 +5,33 @@ // Author: Curtis Malainey // -/* Stub implementation of the Maxim DSM closed source API. +/* Stub implementation of the Maxim DSM API. * - * The upstream stub returns DSM_API_OK from every function without writing - * any output fields. That satisfies the compiler (the Kconfig help text - * also says "only be used for CI and testing"), but always fails at runtime - * during comp_new: + * The upstream stub returns DSM_API_OK from every function without writing any + * output field, which compiles but cannot run: with omemsizerequestedbytes left + * unset the handle size comes out 0, smart_amp_alloc_mod_memblk() skips the + * allocation, dsmhandle stays NULL and maxim_dsm_init() returns -EINVAL, so + * widget setup and card registration fail. * - * dsm_api_get_mem() never writes omemsizerequestedbytes - * -> maxim_dsm_get_handle_size() returns 0 - * -> smart_amp_alloc_mod_memblk(): the `if (ret == 0) return 0` branch - * skips both the allocation and the set_memblk() call - * -> hspk->dsmhandle remains NULL - * -> maxim_dsm_init(): `if (!hspk->dsmhandle) return -EINVAL` fires - * -> host sees `ipc tx error for 0x30010000 ... -22` - * + `Failed to setup widget SMART_AMP2.0`, card registration fails. - * - * This stub fills all output fields read back by the caller so that the - * smart_amp Maxim glue layer can fully initialise and run the data path. - * Feed-forward processing is a straight pass-through (input -> output), so - * audio is still audible under the stub, allowing the glue layer + topology - * + blob layout to be validated independently from the closed-source - * libdsm.a itself. + * This stub writes every field the caller reads back, and passes feed-forward + * audio straight through, so the glue layer, topology and blob layout can be + * exercised without libdsm.a. */ #include "dsm_api_public.h" -/* Must match DSM_FRM_SZ in smart_amp_maxim_dsm.c: that file hard-codes - * DSM_FRM_SZ samples per frame in its ff/fb interleave/de-interleave loops, - * and a mismatch here causes sample misalignment. +/* must match DSM_FRM_SZ in smart_amp_maxim_dsm.c, which hard-codes it in the + * ff/fb interleave loops; a mismatch misaligns the samples */ #define DSM_STUB_FRAME_SZ_SAMPLES 48 -/* The real libdsm.a handle size is unknown. The stub itself does not use - * this memory; any non-zero value works. 4 KB is chosen to also exercise - * the PRIVATE memblk allocation path. +/* any non-zero value works -- the stub never touches this memory; 4 KB also + * exercises the PRIVATE memblk allocation path */ #define DSM_STUB_HANDLE_SZ_BYTES 4096 -/* maxim_dsm_get_volatile_param() iterates up to DSM_API_ADAPTIVE_PARAM_END - * (0x14); the parameter count must exceed that value or caldata overflows. - * Use the maximum value the API allows. +/* must exceed DSM_API_ADAPTIVE_PARAM_END, which + * maxim_dsm_get_volatile_param() iterates up to, or caldata overflows */ #define DSM_STUB_MAX_NUM_PARAM DSM_DEFAULT_MAX_NUM_PARAM @@ -141,11 +127,9 @@ enum DSM_API_MESSAGE dsm_api_get_params(void *ipmodulehandler, int icommandnumbe if (!cmdblock) return DSM_API_MSG_NULL_PARAM_POINTER; - /* Caller places the command id in cmdblock[DSM_GET_ID_IDX]; return - * values are read from CH1/CH2. The upstream stub leaves these fields - * unwritten, so the caller reads uninitialised stack data. - * maxim_dsm_get_num_param() is especially critical: it uses this result - * to size the caldata allocation. + /* The upstream stub leaves CH1/CH2 unwritten and the caller reads + * uninitialised stack data -- maxim_dsm_get_num_param() sizes the + * caldata allocation from it. */ if (DSM_CH_MASK(cmdblock[DSM_GET_ID_IDX]) == DSM_API_GET_MAXIMUM_CMD_ID) { cmdblock[DSM_GET_CH1_IDX] = DSM_STUB_MAX_NUM_PARAM; diff --git a/src/audio/smart_amp/smart_amp.c b/src/audio/smart_amp/smart_amp.c index 52e6ca228f5f..14682f7bf3a3 100644 --- a/src/audio/smart_amp/smart_amp.c +++ b/src/audio/smart_amp/smart_amp.c @@ -51,6 +51,8 @@ struct smart_amp_data { struct smart_amp_mod_stream ff_mod; /**< feed-forward buffer for mod */ struct smart_amp_mod_stream fb_mod; /**< feedback buffer for mod */ struct smart_amp_mod_stream out_mod; /**< output buffer for mod */ + uint32_t ff_max_frames; /**< frames of ff_mod/out_mod capacity, per process() */ + uint32_t fb_max_frames; /**< frames of fb_mod capacity, per process() */ struct smart_amp_buf mod_mems[MOD_MEMBLK_MAX]; /**< memory blocks for mod */ @@ -90,16 +92,10 @@ static inline void smart_amp_free_mod_memories(struct smart_amp_data *sad) sad->mod_data = NULL; } -/* Alignment required for every buffer handed to the inner model. - * - * mod_alloc() requests no particular alignment, and on platforms built with - * CONFIG_SYS_HEAP_SMALL_ONLY=y and without CONFIG_SOF_ZEPHYR_HEAP_CACHED the - * Zephyr sys_heap chunk header is only 4 bytes, so every returned pointer is - * 4 modulo 8 (see chunk_mem() in zephyr/lib/heap/heap.c). Inner models using - * Xtensa HiFi 8-byte loads and stores (AE_L32X2 / AE_S32X2 / AE_L64 / AE_S64) - * then take an unhandled LoadStoreAlignmentCause exception. Ask for 16 bytes - * explicitly, matching the alignment the vendor libraries lay their internal - * memory blocks out on. +/* Alignment required for every buffer handed to the inner model: mod_alloc() + * requests none, and the 4-byte Zephyr sys_heap chunk header leaves pointers at + * 4 modulo 8, which faults inner models using Xtensa HiFi 8-byte loads and + * stores (LoadStoreAlignmentCause). 16 matches the inner models' own. */ #define SMART_AMP_BUF_ALIGN 16 @@ -302,9 +298,8 @@ static int smart_amp_get_config(struct processing_module *mod, comp_dbg(dev, "actual blob size = %zu, expected blob size = %zu", bs, sizeof(struct sof_smart_amp_config)); - /* bs is the host-set config.size and is used as the memcpy source - * length from the fixed-size sad->config, so bound it by the struct - * size. memcpy_s() below already bounds it against the destination. + /* bs is host-set and used as the memcpy source length from the + * fixed-size sad->config, so bound it by the struct size */ if (bs == 0 || bs > sizeof(struct sof_smart_amp_config)) return -EINVAL; @@ -538,11 +533,8 @@ static int smart_amp_ff_process(struct processing_module *mod, return 0; } - /* - * The remap functions write frames * ff_mod.channels samples into - * ff_mod.buf, which holds SMART_AMP_FF_BUF_DB_SZ samples. Bound the - * total sample count, not just the frame count, so an unexpected - * channel count cannot overflow the buffer. + /* the remap writes frames * channels samples, so bound the total + * sample count rather than just the frame count */ if ((uint64_t)frames * sad->ff_mod.channels > SMART_AMP_FF_BUF_DB_SZ) { comp_err(dev, "feed forward frame size overflow: %u frames, %u ch", @@ -620,7 +612,18 @@ static int smart_amp_process(struct processing_module *mod, avail_passthrough_frames = audio_stream_avail_frames(&source_buf->stream, &sink_buf->stream); - avail_frames = avail_passthrough_frames; + /* The mod buffers hold one SMART_AMP_MAX_PERIOD_MS period, but the + * stream buffers are sized at 2 periods, so a missed cycle can leave + * more than that available. Process what fits and leave the rest for + * the next cycle: the overflow checks below would otherwise reject the + * whole call and emit a period of silence, and the LL pipeline drains + * one period per tick, so the backlog does not grow. A permanently + * oversized period is smart_amp_prepare()'s warning, not this cap. + */ + avail_frames = MIN(avail_passthrough_frames, sad->ff_max_frames); + if (avail_frames < avail_passthrough_frames) + comp_dbg(dev, "capped to %u of %u available frames", + avail_frames, avail_passthrough_frames); if (sad->feedback_buf) { struct comp_buffer *feedback_buf = sad->feedback_buf; @@ -630,14 +633,17 @@ static int smart_amp_process(struct processing_module *mod, avail_feedback_frames = audio_stream_get_avail_frames(&feedback_buf->stream); - avail_feedback_frames = MIN(avail_passthrough_frames, - avail_feedback_frames); + /* one fb frame per ff frame, and never more than the + * fb buffer holds (its channel count may exceed ff's) + */ + avail_feedback_frames = MIN(avail_feedback_frames, avail_frames); + avail_feedback_frames = MIN(avail_feedback_frames, sad->fb_max_frames); feedback_bytes = avail_feedback_frames * audio_stream_frame_bytes(&feedback_buf->stream); - comp_dbg(dev, "processing %u feedback frames (avail_passthrough_frames: %u)", - avail_feedback_frames, avail_passthrough_frames); + comp_dbg(dev, "processing %u feedback frames (avail_frames: %u)", + avail_feedback_frames, avail_frames); /* perform buffer writeback after source_buf process */ buffer_stream_invalidate(feedback_buf, feedback_bytes); @@ -759,12 +765,13 @@ static int smart_amp_prepare(struct processing_module *mod, /* In module API, state is managed by the framework, so no comp_set_state needed */ - /* Neither pointer is cleared by smart_amp_reset(), so a re-prepare after - * the feedback branch was disconnected would otherwise keep using the - * stale buffer from the previous run. + /* smart_amp_reset() does not clear these, so a re-prepare after the + * feedback branch was disconnected would reuse the stale buffer */ sad->source_buf = NULL; sad->feedback_buf = NULL; + sad->ff_max_frames = 0; + sad->fb_max_frames = 0; for (i = 0; i < num_of_sources; i++) { struct comp_buffer *buf = comp_buffer_get_from_source(sources[i]); @@ -776,19 +783,13 @@ static int smart_amp_prepare(struct processing_module *mod, } /* - * Tell the feedback source from the playback one. - * - * Do not use sources[i]->bound_module here: it is only ever assigned by - * module_bind(), whose sole caller lives in module_adapter_ipc4.c, so it - * stays NULL on IPC3 platforms (e.g. AMD ACP7x) and dereferencing it - * faults the DSP. + * Tell the feedback source from the playback one by pipeline id: + * the feedback buffer is filled by the echo reference capture + * pipeline's demux, the playback source by our own pipeline's host. * - * The feedback buffer is filled by the demux of the echo reference - * capture pipeline, so its producer belongs to a different pipeline than - * this module, while the playback source is filled by the host component - * of our own pipeline. Comparing pipeline ids works under both IPC - * versions, and unlike comp_dev::direction it is already valid here even - * when the capture pipeline has not been parametrised yet. + * Not sources[i]->bound_module: only module_bind() assigns it and + * its sole caller is in module_adapter_ipc4.c, so it stays NULL on + * IPC3 and dereferencing it faults the DSP. */ producer = comp_buffer_get_source_component(buf); if (producer && dev_comp_pipe_id(producer) != dev_comp_pipe_id(dev)) @@ -815,6 +816,21 @@ static int smart_amp_prepare(struct processing_module *mod, audio_stream_get_channels(&sad->source_buf->stream)); sad->out_mod.channels = sad->ff_mod.channels; + if (!sad->ff_mod.channels) { + comp_err(dev, "no channels on playback source buffer"); + return -EINVAL; + } + sad->ff_max_frames = SMART_AMP_FF_BUF_DB_SZ / sad->ff_mod.channels; + + /* smart_amp_process() caps each call at ff_max_frames, which absorbs a + * transient backlog, but a scheduled period permanently above the cap + * would underrun the DAI forever. Name the knob rather than let that + * look like an amp fault. + */ + if (dev->frames > sad->ff_max_frames) + comp_warn(dev, "period of %u frames exceeds the %u frames buffered for %u ms, raise SMART_AMP_MAX_PERIOD_MS", + dev->frames, sad->ff_max_frames, SMART_AMP_MAX_PERIOD_MS); + /* the least bitdepth required for inner model, which should not be lower than the bitdepth * for input samples of feed-forward, and feedback if exists. */ @@ -828,6 +844,11 @@ static int smart_amp_prepare(struct processing_module *mod, fb_src_fmt = audio_stream_get_frm_fmt(&sad->feedback_buf->stream); sad->fb_mod.channels = MIN(SMART_AMP_FB_MAX_CH_NUM, audio_stream_get_channels(&sad->feedback_buf->stream)); + if (!sad->fb_mod.channels) { + comp_err(dev, "no channels on feedback source buffer"); + return -EINVAL; + } + sad->fb_max_frames = SMART_AMP_FB_BUF_DB_SZ / sad->fb_mod.channels; least_req_depth = MAX(least_req_depth, get_sample_bitdepth(fb_src_fmt)); } diff --git a/src/audio/smart_amp/smart_amp_generic.c b/src/audio/smart_amp/smart_amp_generic.c index 72f81d0e5117..8d03eb5236f1 100644 --- a/src/audio/smart_amp/smart_amp_generic.c +++ b/src/audio/smart_amp/smart_amp_generic.c @@ -35,9 +35,8 @@ static void remap_s32_to_s32(struct smart_amp_mod_stream *src_mod, uint32_t fram n = MIN(num_samples_remaining, nmax); for (ch = 0; ch < src_mod->channels; ch++) { - /* skip unmapped (-1) and out-of-range source channels; - * the uint8_t cast folds the negative case into the - * single upper-bound check (-1 becomes 255 >= src_ch) + /* skip unmapped (-1) and out-of-range channels: the + * cast folds -1 into the upper-bound check as 255 */ if ((uint8_t)chan_map[ch] >= src_ch) continue; @@ -107,10 +106,7 @@ static void remap_s16_to_s16(struct smart_amp_mod_stream *src_mod, uint32_t fram n = MIN(num_samples_remaining, nmax); for (ch = 0; ch < src_mod->channels; ch++) { - /* skip unmapped (-1) and out-of-range source channels; - * the uint8_t cast folds the negative case into the - * single upper-bound check (-1 becomes 255 >= src_ch) - */ + /* skip unmapped (-1) and out-of-range channels */ if ((uint8_t)chan_map[ch] >= src_ch) continue; @@ -155,10 +151,7 @@ static void remap_s16_to_b32(struct smart_amp_mod_stream *src_mod, uint32_t fram n = MIN(num_samples_remaining, nmax); for (ch = 0; ch < src_mod->channels; ch++) { - /* skip unmapped (-1) and out-of-range source channels; - * the uint8_t cast folds the negative case into the - * single upper-bound check (-1 becomes 255 >= src_ch) - */ + /* skip unmapped (-1) and out-of-range channels */ if ((uint8_t)chan_map[ch] >= src_ch) continue; diff --git a/src/audio/smart_amp/smart_amp_maxim_dsm.c b/src/audio/smart_amp/smart_amp_maxim_dsm.c index 9aa9940b7dda..a59d97a2397d 100644 --- a/src/audio/smart_amp/smart_amp_maxim_dsm.c +++ b/src/audio/smart_amp/smart_amp_maxim_dsm.c @@ -27,56 +27,44 @@ LOG_MODULE_DECLARE(smart_amp, CONFIG_SOF_LOG_LEVEL); #define DSM_FF_BUF_SZ (DSM_FRM_SZ * SMART_AMP_FF_MAX_CH_NUM) #define DSM_FB_BUF_SZ (DSM_FRM_SZ * SMART_AMP_FB_MAX_CH_NUM) -/* Capacity of the ff accumulation ring buffer. - * - * libdsm's frame size is fixed at 1 ms, so one DSM frame is DSM_FRM_SZ=48 - * samples/channel = DSM_FF_BUF_SZ=96 samples and a SOF period holds - * period_us/1000 frames: ACP7x TDM2 at 2000 us delivers 2 frames. - * - * The upstream value (DSM_FF_BUF_SZ * SMART_AMP_FF_MAX_CH_NUM = 192) fits one - * period but not also the pre-fill frame, so maxim_dsm_ff_proc's entry check - * (96 + 192 > 192) is always true: it returns -EOVERFLOW forever and never - * calls dsm_api_ff_process. It was written assuming period = 1000 us. - * - * 4 * DSM_FF_BUF_SZ = 384 holds a residual w_ptr (< 96) plus one period, and - * covers periods up to 3000 us. ff_out shares the constant; at 3000 us its - * r_ptr peaks at 384, exactly saturating the buffer. +/* Capacity of the ff accumulation and output ring buffers. libdsm's frame size + * is fixed at 1 ms, so a period of P ms arrives as P frames of DSM_FF_BUF_SZ + * samples. maxim_dsm_flush() pre-fills one frame of silence for delay + * alignment and it stays resident, so both rings peak at that frame plus one + * whole period -- which the generic layer caps at SMART_AMP_FF_BUF_DB_SZ. */ -#define DSM_FF_BUF_DB_SZ (DSM_FF_BUF_SZ * 4) - -/* IV format for the feedback path. - * - * Upstream de-interleaves at 4*idx, assuming 4 TDM slots in VIVI order. This - * platform (TDM2 / 2x max98388) has only 2 slots: each max98388 - * owns one slot and alternates V and I within it using the sample MSB as the - * flag, i.e. libdsm's DSM_IV_FORMAT_INTERLEAVED_16_BIT_MSB (ivFormat=3). - * libdsm then reads the two input buffers as the left and right channel streams - * (so the "voltage"/"current" names are misleading in this mode), takes - * DSM_FRM_SZ consecutive samples from each, splits them into 24 V + 24 I by MSB - * and duplicates each for 24k -> 48k upsampling. - * - * Keeping the 4*idx stride here would read every other sample of one channel; - * as the MSB strictly alternates, that picks a single polarity and leaves the - * other class empty, making libdsm return DSM_API_MSG_INSUFFICIENT_INPUT_DATA. +#define DSM_FF_BUF_DB_SZ (DSM_FF_BUF_SZ + SMART_AMP_FF_BUF_DB_SZ) + +/* IV format for the feedback path. Upstream de-interleaves at 4*idx, assuming + * 4 TDM slots in VIVI order. This platform has 2: each max98388 owns one slot + * and alternates V and I within it using the sample MSB as the flag, i.e. + * libdsm's DSM_IV_FORMAT_INTERLEAVED_16_BIT_MSB (ivFormat=3), where libdsm + * reads its two input buffers as left/right channel streams and splits each by + * MSB itself. The 4*idx stride would pick one polarity and leave the other + * class empty, making libdsm return DSM_API_MSG_INSUFFICIENT_INPUT_DATA. */ #define MAXIM_DSM_FB_IVFMT3 1 -/* Command ID and IV format enum from the vendor dsm_api.h; not included in +/* Command ID and IV format enum from the full dsm_api.h; not included in * SOF's trimmed dsm_api_public.h. */ #define DSM_API_SETGET_IV_FORMAT 146 #define DSM_IV_FORMAT_INTERLEAVED_16_BIT_MSB 3 -/* Samples consumed from the accumulation buffer per DSM fb frame. - * ivFormat=3: DSM_FRM_SZ consecutive samples per channel x 2 physical channels. - * Upstream 4-channel VIVI: DSM_FB_BUF_SZ (= DSM_FRM_SZ * 4). +/* Samples consumed from the accumulation buffer per DSM fb frame: one frame per + * channel x 2 physical channels, against 4-channel VIVI upstream. */ #if MAXIM_DSM_FB_IVFMT3 #define DSM_FB_FRM_SZ (DSM_FRM_SZ * 2) #else #define DSM_FB_FRM_SZ DSM_FB_BUF_SZ #endif -#define DSM_FB_BUF_DB_SZ (DSM_FB_BUF_SZ * SMART_AMP_FB_MAX_CH_NUM) + +/* Capacity of the fb accumulation ring buffer. No pre-fill here, but the drain + * loop consumes whole DSM_FB_FRM_SZ frames, so up to one frame short of it can + * carry over when a period is not a multiple of the fb frame size. + */ +#define DSM_FB_BUF_DB_SZ (DSM_FB_FRM_SZ + SMART_AMP_FB_BUF_DB_SZ) /* DSM parameter table structure * +--------------+-----------------+---------------------------------+ @@ -218,8 +206,7 @@ static int maxim_dsm_init(struct smart_amp_mod_struct_t *hspk) #if MAXIM_DSM_FB_IVFMT3 /* dsm_api_init_ext_t has no ivFormat field and libdsm defaults to - * DSM_IV_FORMAT_DEINTERLEAVED_16_BIT(0), so the MSB-interleaved layout - * this platform uses must be set explicitly. Runs before + * deinterleaved(0), so set it explicitly -- before * maxim_dsm_get_all_param() so the parameter db captures it. */ { @@ -415,6 +402,7 @@ static int maxim_dsm_get_param(struct smart_amp_mod_struct_t *hspk, cdata->data->abi = SOF_ABI_VERSION; cdata->data->size = 0; } + return 0; } @@ -533,18 +521,14 @@ static int maxim_dsm_set_config(struct smart_amp_mod_data_base *mod, } /* Shift the unconsumed tail of an accumulation buffer down to its base: copy - * @count samples from [@offset, @offset + @count) to [0, @count), in a buffer - * whose samples are @szsample bytes wide. + * @count samples of @szsample bytes from [@offset, @offset + @count) to + * [0, @count). * - * memcpy_s() must not be used: SOF's implementation - * (zephyr/include/rtos/string.h) detects overlapping src/dst, returns -EINVAL - * and copies nothing. The ranges overlap as soon as count > offset, which - * every period longer than 2 ms produces on every cycle, and the call sites - * advance their write pointer regardless of the return value -- so the buffer - * would silently desynchronise. Losing memcpy_s()'s bounds check costs - * nothing: every call site passes dest_size == count, making it vacuous, and - * the invariant that matters (offset + count inside the buffer) is held by the - * overflow test each caller runs first. + * Not memcpy_s(): SOF's implementation rejects overlapping src/dst with -EINVAL + * and copies nothing, the ranges overlap as soon as count > offset (every cycle + * at any period above 2 ms), and the call sites advance their write pointer + * regardless of the return value, so the buffer would silently desynchronise. + * Each caller's overflow test already holds offset + count inside the buffer. */ static void dsm_buf_shift_down(void *base, int offset, int count, int szsample) { @@ -589,11 +573,16 @@ static int maxim_dsm_ff_proc(struct smart_amp_mod_data_base *mod, * must be smaller than buffer size limit */ if (*w_ptr + nsamples <= DSM_FF_BUF_DB_SZ) { + /* dest_size is the space left in the ring, not the copy + * length, or memcpy_s()'s check never fires + */ if (is_16bit) - memcpy_s(&buf.buf16[*w_ptr], nsamples * szsample, + memcpy_s(&buf.buf16[*w_ptr], + (DSM_FF_BUF_DB_SZ - *w_ptr) * szsample, in->buf.data, nsamples * szsample); else - memcpy_s(&buf.buf32[*w_ptr], nsamples * szsample, + memcpy_s(&buf.buf32[*w_ptr], + (DSM_FF_BUF_DB_SZ - *w_ptr) * szsample, in->buf.data, nsamples * szsample); *w_ptr += nsamples; } else { @@ -604,10 +593,9 @@ static int maxim_dsm_ff_proc(struct smart_amp_mod_data_base *mod, goto error; } - /* Run DSM Feedforward process if the buffer is ready. - * Use while, not if: a 2 ms SOF period contains 2 DSM frames (1 ms each); - * an if would drain only one frame per call, causing w_ptr to grow by - * DSM_FF_BUF_SZ every period and the buffer to overflow. + /* Run DSM Feedforward process if the buffer is ready. while, not if: + * a P ms period holds P DSM frames, and draining one per call would + * grow w_ptr by a frame every period until it overflows. */ while (*w_ptr >= DSM_FF_BUF_SZ && *r_ptr + DSM_FF_BUF_SZ <= DSM_FF_BUF_DB_SZ) { @@ -626,21 +614,17 @@ static int maxim_dsm_ff_proc(struct smart_amp_mod_data_base *mod, } } - /* Drop the DSM frame just de-interleaved out of the head of - * the accumulation buffer. Overlaps whenever w_ptr > 2 * - * DSM_FF_BUF_SZ, i.e. at any period above 2 ms; see - * dsm_buf_shift_down(). + /* drop the frame just de-interleaved; the ranges overlap at any + * period above 2 ms, hence dsm_buf_shift_down() */ dsm_buf_shift_down(hspk->buf.ff.buf, DSM_FF_BUF_SZ, *w_ptr - DSM_FF_BUF_SZ, szsample); *w_ptr -= DSM_FF_BUF_SZ; - /* One call covers both sample widths: input/input32 and - * output/output32 are just differently typed views of the same - * hspk->buf.input / hspk->buf.output, and the short * in the - * prototype is vestigial -- libdsm strides the buffers by - * pCommonParams->iSampleByteWidth, which dsm_api_init() derived - * from initparam.isamplebitwidth (16 -> 2 bytes, 24/32 -> 4). + /* One call covers both sample widths: input/input32 are typed + * views of one buffer and the short * is vestigial -- libdsm + * strides by iSampleByteWidth, which dsm_api_init() derived + * from initparam.isamplebitwidth. */ hspk->ifsamples = hspk->nchannels * hspk->ff_fr_sz_samples; dsm_api_ff_process(hspk->dsmhandle, hspk->channelmask, @@ -667,18 +651,16 @@ static int maxim_dsm_ff_proc(struct smart_amp_mod_data_base *mod, /* Output buffer preparation */ if (*r_ptr >= nsamples) { + /* dest_size is the sink-side buffer; see the input copy above */ if (is_16bit) - memcpy_s(out->buf.data, nsamples * szsample, + memcpy_s(out->buf.data, out->buf.size, buf_out.buf16, nsamples * szsample); else - memcpy_s(out->buf.data, nsamples * szsample, + memcpy_s(out->buf.data, out->buf.size, buf_out.buf32, nsamples * szsample); - /* Same construct on the output side. It does not overlap at - * the periods used today (r_ptr never exceeds 2 * nsamples), - * but that bound is an emergent property of the loop guard - * above rather than anything enforced here, so keep it on the - * overlap-safe primitive too. + /* same on the output side: it does not overlap at today's + * periods, but only as an emergent property of the loop guard */ dsm_buf_shift_down(hspk->buf.ff_out.buf, nsamples, *r_ptr - nsamples, szsample); @@ -728,11 +710,14 @@ static int maxim_dsm_fb_proc(struct smart_amp_mod_data_base *mod, * must be smaller than buffer size limit */ if (*w_ptr + nsamples <= DSM_FB_BUF_DB_SZ) { + /* dest_size is the space left in the ring; see maxim_dsm_ff_proc() */ if (is_16bit) - memcpy_s(&buf.buf16[*w_ptr], nsamples * szsample, + memcpy_s(&buf.buf16[*w_ptr], + (DSM_FB_BUF_DB_SZ - *w_ptr) * szsample, in->buf.data, nsamples * szsample); else - memcpy_s(&buf.buf32[*w_ptr], nsamples * szsample, + memcpy_s(&buf.buf32[*w_ptr], + (DSM_FB_BUF_DB_SZ - *w_ptr) * szsample, in->buf.data, nsamples * szsample); *w_ptr += nsamples; } else { @@ -741,21 +726,17 @@ static int maxim_dsm_fb_proc(struct smart_amp_mod_data_base *mod, return -EOVERFLOW; } - /* Run DSM Feedback process if the buffer is ready. - * Use while, not if: same reason as the ff path -- a 2 ms SOF period - * contains 2 DSM frames. + /* Run DSM Feedback process if the buffer is ready. while, not if: + * same reason as the ff path. */ while (*w_ptr >= DSM_FB_FRM_SZ) { if (is_16bit) { #if MAXIM_DSM_FB_IVFMT3 - /* De-interleave 2-channel PCM into per-slot streams. - * In ivFormat=3, libdsm treats icurrbuffer as the left - * channel (slot 0 / amp0) and ivoltbuffer as the right - * channel (slot 1 / amp1), extracting V and I from each - * using the sample MSB. The variable names i/v are - * inherited from the upstream VIVI path and are misleading - * here: both buffers contain interleaved V+I, not pure - * current or voltage. + /* In ivFormat=3 libdsm takes icurrbuffer as the left + * channel (slot0/amp0) and ivoltbuffer as the right + * (slot1/amp1), splitting V and I out of each by MSB. + * The i/v names are inherited from the VIVI path and + * mislead here: both hold interleaved V+I. */ for (idx = 0; idx < DSM_FRM_SZ; idx++) { i[idx] = buf.buf16[2 * idx]; /* slot0 / amp0 -> left */ @@ -781,21 +762,17 @@ static int maxim_dsm_fb_proc(struct smart_amp_mod_data_base *mod, } } - /* Drop the DSM frame just de-interleaved; same overlap - * boundary as the ff path. See dsm_buf_shift_down(). - */ + /* drop the frame just de-interleaved; see dsm_buf_shift_down() */ dsm_buf_shift_down(hspk->buf.fb.buf, DSM_FB_FRM_SZ, *w_ptr - DSM_FB_FRM_SZ, szsample); *w_ptr -= DSM_FB_FRM_SZ; - /* *ipNrSamples is the total across both buffers regardless of - * ivFormat: this path divides it by FB_DSM_channels (= 2), then - * rejects the call unless the per-channel count is a multiple of - * ffFrameSize. So it must be fb_fr_sz_samples * nchannels = 96 - * (96 / 2 = 48, 48 % 48 == 0, frameNum = 1); passing 48 gives - * 24 % 48 != 0 and returns INVALID_PARAM every call. The - * ivFormat-aware variant that skips the divide is - * DSM_API_FB_process_v2, which this path does not call. + /* *ipNrSamples is the total across both buffers whatever the + * ivFormat: this path divides it by the channel count and then + * requires a multiple of ffFrameSize, so it must be + * fb_fr_sz_samples * nchannels -- passing one frame's worth + * returns INVALID_PARAM every call. Only the v2 entry point + * skips that divide. */ hspk->ibsamples = hspk->fb_fr_sz_samples * hspk->nchannels; if (is_16bit) diff --git a/src/include/sof/audio/smart_amp/smart_amp.h b/src/include/sof/audio/smart_amp/smart_amp.h index e6007bf28729..b79b339e9b68 100644 --- a/src/include/sof/audio/smart_amp/smart_amp.h +++ b/src/include/sof/audio/smart_amp/smart_amp.h @@ -66,10 +66,21 @@ #define SMART_AMP_FF_BUF_SZ (SMART_AMP_FRM_SZ * SMART_AMP_FF_MAX_CH_NUM) #define SMART_AMP_FB_BUF_SZ (SMART_AMP_FRM_SZ * SMART_AMP_FB_MAX_CH_NUM) +/* The longest SOF period, in whole milliseconds, that one process() call can + * carry. Inner models run at 48 kHz with a 1 ms frame size, so the period must + * be a whole number of milliseconds as well: a fractional one leaves a residual + * that walks the model's output read pointer down until it underruns. Raising + * this costs roughly 3 kB of heap per millisecond per instance. + */ +#define SMART_AMP_MAX_PERIOD_MS 5 + +/* Capacity of the generic-layer remap buffers, in samples: one full period at + * the widest channel count the remap functions can write. + */ #define SMART_AMP_FF_BUF_DB_SZ\ - (SMART_AMP_FF_BUF_SZ * SMART_AMP_FF_MAX_CH_NUM) + (SMART_AMP_FF_BUF_SZ * SMART_AMP_MAX_PERIOD_MS) #define SMART_AMP_FB_BUF_DB_SZ\ - (SMART_AMP_FB_BUF_SZ * SMART_AMP_FB_MAX_CH_NUM) + (SMART_AMP_FB_BUF_SZ * SMART_AMP_MAX_PERIOD_MS) struct inner_model_ops;