Repository navigation
amd/acp7x: smart_amp: enable Maxim DSM for 2x max98388 on TDM2 - #11250
Open
LongWang100 wants to merge 3 commits into
Open
LongWang100 wants to merge 3 commits into
LongWang100 wants to merge 3 commits into
Conversation
LongWang100
requested review from
dbaluta,
kv2019i,
lbetlej,
lgirdwood,
mmaka1,
plbossart and
ranj063
as code owners
September 30, 2026 03:21
Collaborator
|
Can one of the admins verify this patch?
|
LongWang100
force-pushed
the
amd-acp7b-stable-dsm-support
branch
2 times, most recently
from
October 8, 2026 02:03
a268a36 to
601472d
Compare
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 <long.wang@analog.com>
Member
|
@LongWang100 do you have a similar PR for main branch ? This will need approved by AMD developers. |
@lgirdwood : This is a AMD maintained branch, there need not be a PR for main branch. @LongWang100 : https://github.com/thesofproject/sof/tree/amd-acp7b-stable is AMD branch and managed by AMD engineers. AMD will take care of it. |
bhiregoudar
requested changes
Oct 13, 2026
bhiregoudar
left a comment
bhiregoudar
left a comment
There was a problem hiding this comment.
It is better to create a separate topology with smart amp support.
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 <long.wang@analog.com>
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 <long.wang@analog.com>
|
@LongWang100: It seems there one test failure related to topologies. Could you please check |
Contributor
Author
@bhiregoudar The build-and-test failure is unrelated to this PR. The testbench.yml CI invokes build-tools.sh -Y, which tries to build the topologies2 CMake target. That target has been absent since add_subdirectory(topology2) was commented out in commit 2c4b01f (2026-07-27), two months before this PR. The topology1 build (top-level_default_CMake_target_ALL) passes. The two new tplg files (sof-acp7x-smart-amp and sof-acp7x-smart-amp-passthru) compile correctly locally with alsatplg. |
Member
|
@LongWang100 pls also check CI, looks like an issue with topology makefile changes: |
Contributor
Author
@lgirdwood |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Enables the Maxim DSM smart amplifier on AMD ACP7x, driving two MAX98388
amplifiers over a 2-slot TDM2 link, and fixes the generic smart_amp buffer
sizing that the bring-up uncovered.
amd/acp7x: smart_amp: enable Maxim DSM for 2x max98388 on TDM2topology1: amd/acp7x: build one TDM2 tplg per smart_amp configurationaudio: smart_amp: size the mod buffers by period, not channel count1. Enable Maxim DSM on ACP7x TDM2
The link carries two amplifiers on two slots, each interleaving V and I within
its own slot (
ivFormat = 3).On IPC3
bound_moduleis NULL, so the feedback branch is identified by theproducer's pipeline id. The mod buffers need
mod_alloc_align()for the16-byte alignment the Xtensa HiFi
AE_L32X2/AE_S64paths require, andibsamplesisfb_fr_sz_samples * nchannels(96, not 48). Alsoif→whileinff_proc/fb_procso a period longer than one model frame drainsfully, a duplicated
dsm_api_ff_process()dropped from the 32-bit branch, andmemcpy_s()on overlapping ranges replaced with a memmove-based helper.2. One tplg per smart_amp configuration
Commit 1 put smart_amp into
sof-acp_7_x_i2s.m4unconditionally, whichsilently redefined
sof-acp7x.tplg: it would then requireCONFIG_MAXIM_DSMfirmware, and
comp_newfails with-EINVALon any other. Build one tplg perconfiguration instead, selected by
SMART_UUID—sof-acp7x.tplg(nosmart_amp widget),
sof-acp7x-smart-amp-passthru.tplgandsof-acp7x-smart-amp.tplg.Two incidental fixes:
${input}_M4_DEPSadded toDEPENDS, sinceMAIN_DEPENDENCYonly tracks the top-level.m4and editing aninclude()dfile would leave a stale
.tplgbehind; anderrprint()+m4exit(1)in place offatal_error(), which is defined nowhere in topology1 and leaves m4 exiting 0.3. Size the mod buffers by period, not channel count
The generic remap buffers were sized
SMART_AMP_FF_BUF_SZ * SMART_AMP_FF_MAX_CH_NUM, butSMART_AMP_FF_BUF_SZalready carries the channelcount, and the 192 samples it yields hold one 2 ms period only by coincidence.
The Maxim inner model worked around it with a ring of its own capped at 3 ms,
and nothing checked the scheduled period against either limit.
Both are replaced by one knob,
SMART_AMP_MAX_PERIOD_MS = 5, bounded by heapcost and by the inner models' 1 ms frame at 48 kHz.
prepare()derives theper-call frame limits and warns when the period exceeds the cap;
process()caps
avail_framesinstead of emitting a period of silence. Separately, allfour accumulation
memcpy_s()calls passed the copy length asdest_size,making the bounds check vacuous.