Skip to content

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
thesofproject:topic/sof-devfrom
saba-kareem:topic/sof-dev
Open

BRA/BPT Support for AMD Platforms(ACP 7.0/7.1/7.2 variants) and code fixes#5867
saba-kareem wants to merge 4 commits into
thesofproject:topic/sof-devfrom
saba-kareem:topic/sof-dev

Conversation

@saba-kareem

Copy link
Copy Markdown

BRA/BPT Support for AMD Platforms(ACP 7.0/7.1/7.2 variants) and includes fixes in intel code.

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>
@sofci

sofci commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Can one of the admins verify this patch?

reply test this please to run this test once

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_lock mutex 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.

Comment thread drivers/soundwire/amd_manager.c Outdated
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>
@bardliao

bardliao commented Aug 3, 2026

Copy link
Copy Markdown
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.

@saba-kareem

Copy link
Copy Markdown
Author

@bardliao At present, we have topology where only speaker peripherals connected to same Soundwire manager instance for this generation platforms. We need to revise our bandwidth calculation to align with your proposed logic. We will adopt the shared bandwidth calculation approach for future platform deployments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants