BRA/BPT Support for AMD Platforms(ACP 7.0/7.1/7.2 variants) and code fixes - #5867
Open
saba-kareem wants to merge 4 commits into
Open
BRA/BPT Support for AMD Platforms(ACP 7.0/7.1/7.2 variants) and code fixes#5867saba-kareem wants to merge 4 commits into
saba-kareem wants to merge 4 commits into
Conversation
intel_ace2x_bpt_open_stream() calls sdw_slave_bpt_stream_add(), which via sdw_stream_add_slave() -> sdw_master_rt_alloc() allocates the master runtime, links it into bus->m_rt_list and raises bus->bpt_stream_refcount. Several later failure paths (PDI allocation, port-config allocation and sdw_stream_add_master()) jump to the remove_slave label, which only calls sdw_stream_remove_slave() followed by sdw_release_stream(). sdw_stream_remove_slave() frees only the slave runtime and ports; it does not reach sdw_master_rt_free(). The master runtime is therefore left on bus->m_rt_list pointing at the just-freed stream, and bpt_stream_refcount stays non-zero. Because sdw_master_rt_alloc() rejects a new BPT allocation while bpt_stream_refcount > 0, every subsequent BPT transfer on that bus is rejected with -EBUSY until the driver is reloaded. Route these error paths through the remove_master label so that sdw_stream_remove_master() frees the master runtime and drops the refcount before the stream is released, mirroring the error-path unwind in amd_sdw_bpt_open_stream(). Drop the now-unused remove_slave label; its sdw_stream_remove_slave() call still runs by falling through from remove_master, and is a no-op once the master runtime (and with it the slave runtimes) has been freed. Fixes: 4c1ce9f ("soundwire: intel_ace2x: add BPT send_async/wait callbacks") Signed-off-by: Syed Saba Kareem <Syed.SabaKareem@amd.com>
The BPT (Bulk Payload Transport) stream pointer bus->bpt_stream is read locklessly by the SoundWire core to tell whether a BPT transfer owns the bus. For those readers to be safe the pointer and bus->bpt_stream_refcount must stay consistent: an observer that sees refcount == 0 under bus_lock must also see bpt_stream == NULL. Make intel_ace2x maintain that ordering: - Publish bus->bpt_stream with WRITE_ONCE() only after the master runtime has been added and bpt_stream_refcount raised, and (on the open path) before the in-open sdw_prepare_stream(). - Clear it with WRITE_ONCE() before sdw_stream_remove_master() drops the refcount on the close and error paths. Add a clear_bpt_stream label so paths that already published the pointer clear it, while the pre-publish failure paths skip the clear. - Snapshot the pointer into a local once (READ_ONCE()) so the open/close/error paths act on a single stable value instead of repeatedly re-reading the shared field. This is a no-op under the current policy, where BPT and audio streams are mutually exclusive, but establishes the ordering the core relies on once BPT is allowed to run alongside idle audio streams. Signed-off-by: Syed Saba Kareem <Syed.SabaKareem@amd.com>
sdw_master_rt_alloc() rejected a BPT (Bulk Payload Transport) stream allocation whenever any audio stream was allocated on the bus (bus->stream_refcount > 0). On a power-off-mode platform, an amplifier that was left DISABLED across system suspend still holds an allocated but idle stream runtime, yet it must re-download its firmware over BPT on resume before that stream can be re-enabled. The blanket refcount check made the resume-time BPT transfer fail with -EBUSY. Relax the check so that BPT is only blocked by another in-flight BPT transfer or by an audio stream that is actively using the bus. Add sdw_bus_has_active_stream(), which returns true only for streams in the PREPARED or ENABLED state; streams that are merely allocated, configured, disabled or deprepared reserve no active bus bandwidth and need not block BPT. Because the BPT data phase runs with bus_lock released, allowing an idle audio stream to coexist with a BPT transfer opens several windows that did not exist when the two were mutually exclusive. Close them: - sdw_prepare_stream(), sdw_enable_stream() and sdw_deprepare_stream() now refuse to act on a non-BPT stream while a BPT transfer is allocated on any of its buses. Each performs a bank switch (and deprepare also adjusts bus bandwidth) that could corrupt the in-flight BPT frame, which runs without holding bus_lock. The BPT stream itself is exempt so its own transitions still proceed. sdw_disable_stream() needs no guard because an audio stream can never be ENABLED while a BPT transfer is allocated. - sdw_program_params() now skips master runtimes other than the active BPT stream while bus->bpt_stream is set, so BPT preparation no longer rewrites the transport/port parameters of idle audio runtimes or delivers BPT bus parameters to their peripherals via sdw_notify_config(). The bus-wide SDW_SCP_BUSCLOCK_SCALE programming is intentionally left unfiltered, as every attached peripheral must track the actual bus clock. The guards gate on bus->bpt_stream_refcount while the filter keys off bus->bpt_stream. The BPT-capable managers publish bpt_stream (with WRITE_ONCE()) only after raising bpt_stream_refcount and clear it before dropping the refcount; sdw_program_params() reads it with READ_ONCE(). So an audio path that sees refcount == 0 under bus_lock also sees bpt_stream == NULL and programs its own parameters instead of being skipped and reaching PREPARED with nothing written to hardware. The guards and the filter are dormant outside this new case: the guards only reject while a BPT transfer is allocated, and sdw_program_params() only skips while bus->bpt_stream is set, so ordinary audio streaming is unchanged. While at it, make the allocation-time rejection message state the actual reason (another BPT transfer or an active audio stream) instead of printing the now-misleading stream_refcount. Signed-off-by: Syed Saba Kareem <Syed.SabaKareem@amd.com>
saba-kareem
requested review from
bardliao,
bhiregoudar,
dbaluta,
kv2019i,
lgirdwood,
plbossart,
ranj063,
shumingfan and
ujfalusi
as code owners
August 3, 2026 09:08
Collaborator
|
Can one of the admins verify this patch?
|
There was a problem hiding this comment.
Pull request overview
Adds BRA/BPT (bulk register access / bulk payload transfer) support for AMD SoundWire managers on ACP 7.0+ platforms, including synchronization to protect shared BRA resources across multiple manager instances, and tightens SoundWire core/Intel handling around BPT stream lifecycle and exclusivity.
Changes:
- Plumb a new ACP-wide
acp_bra_lockmutex through AMD ACP/PS sound drivers into the SoundWire AMD manager to serialize BRA DMA/PTE programming across instances. - Update SoundWire core stream handling to treat BPT as bus-exclusive vs active audio streams and to avoid (re)programming non-BPT runtimes while a BPT transfer owns the bus.
- Implement AMD BRA/BPT transfer support (DMA + ATU/PTE setup, error handling, runtime PM + suspend/remove safety guards) and fix Intel ACE2x BPT stream publication/teardown ordering.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| sound/soc/sof/amd/acp.h | Adds acp_bra_lock to ACP device data for BRA/BPT cross-instance serialization. |
| sound/soc/sof/amd/acp.c | Initializes and passes acp_bra_lock into SoundWire AMD resources (SOF path). |
| sound/soc/amd/ps/acp63.h | Documents and adds acp_bra_lock to ACP63 device data. |
| sound/soc/amd/ps/pci-ps.c | Initializes and passes acp_bra_lock into SoundWire AMD resources (PS path). |
| include/linux/soundwire/sdw_amd.h | Extends AMD SoundWire resource/pdata and manager state for BRA/BPT support and serialization. |
| drivers/soundwire/stream.c | Ensures consistent BPT snapshot during programming and enforces BPT vs audio-stream exclusivity rules. |
| drivers/soundwire/intel_ace2x.c | Makes BPT stream publish/clear ordering lockless-safe and fixes teardown paths to use the local stream handle. |
| drivers/soundwire/amd_manager.h | Adds BRA/BPT-related registers/bit definitions and timeouts. |
| drivers/soundwire/amd_manager.c | Implements AMD BRA/BPT DMA transfer flow, PTE programming, error handling, and suspend/remove safety sequencing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add Bulk Register Access (BRA) / Bulk Payload Transport (BPT) support for AMD SoundWire platforms. This enables high-speed firmware download to SoundWire peripherals via DP0, using the ACP BRA DMA engine. Key design points: - Uses the SoundWire stream framework (sdw_prepare_stream, sdw_enable_stream, sdw_disable_stream, sdw_deprepare_stream) for all DP0 port programming and bank switches. No manual DP0 register writes or bank mirrors are needed. - BRA transport parameters (hstart, hstop, SampleInterval, BytesPerFrame) are computed dynamically from the current bus frame shape, not hardcoded. - The ACP BPT DMA engine is triggered by the bank switch performed inside sdw_enable_stream(), and stopped by the bank switch in sdw_disable_stream(). - Non-contiguous firmware sections are handled by iterating per-section: large sections use BRA DMA, small sections (< one BRA frame) fall back to sdw_nwrite/sdw_nread. - BPT stream m_rt entries are skipped in audio compute_params to prevent BPT transport parameters from corrupting audio port block offset calculations. - DP0 port_params, xport_params, and port_enable callbacks return early for BPT streams since the ACP BRA descriptor registers handle DP0 configuration independently. - bus->bpt_stream is published with WRITE_ONCE() only after the stream runtime is added and bpt_stream_refcount is raised under bus_lock, and is cleared before the runtime is removed and the stream is freed, so the lockless DP0 port callbacks never observe a half-initialised or freed stream pointer. - On an aborted or timed-out transfer the ACP BPT DMA engine is disarmed (PORT_EN=0) before the sdw_disable_stream() bank switch, so the bank switch cannot re-trigger a DMA write into the buffer that is freed once the transfer returns. - A per-manager bpt_lock serialises concurrent BPT transfers from multiple slave probes. pm_runtime keeps the bus clock active during transfers. - PTE-based ACP ATU mapping provides DMA scatter-gather for the firmware buffer. The ATU maps up to 512 4KB pages (2 MB per transfer); each transfer is additionally bounded by the SoundWire BPT limit of 1 MB (SDW_BPT_MSG_MAX_BYTES), which the driver enforces. Signed-off-by: Syed Saba Kareem <Syed.SabaKareem@amd.com>
saba-kareem
force-pushed
the
topic/sof-dev
branch
from
August 3, 2026 09:56
fe9659b to
1becc08
Compare
Collaborator
|
@saba-kareem You might want to look at #5604 for BPT and audio stream coexisting. The idea is that the BPT stream can use the remaining available bandwidth. |
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.
BRA/BPT Support for AMD Platforms(ACP 7.0/7.1/7.2 variants) and includes fixes in intel code.