Repository navigation
fixup! soundwire: stream: Fix bandwidth leak in multi-link aggregation failure - #6014
Merged
bardliao merged 4 commits intoNov 11, 2026
Merged
Conversation
The compute_params callback is called from both stream prepare and deprepare. A followup commit will make it perform different actions depending on which operation invokes it. No functional change. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
When multiple master runtimes exist on the same bus, sdw_compute_bus_params() selects the first runtime for which an additional lane is available. This can select an already running stream instead of the stream being prepared. Changing that runtime's port lanes accounts for its bandwidth again. If preparing the new stream fails, restoring the bus parameters leaves the active stream's lanes inconsistent with its bandwidth reservation. Depreparing the active stream can then underflow the lane bandwidth and leave lane 0 bandwidth reserved. Pass the current stream and prepare context to sdw_compute_bus_params() and restrict lane allocation to that stream's runtime during prepare. Skip lane allocation during deprepare, when its bandwidth reservation has already been released. Keep clock recomputation and the existing group and port filtering unchanged. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
bardliao
requested review from
davidlin923,
plbossart and
ujfalusi
and
a balanced review from Copilot
November 3, 2026 14:32
bardliao
requested review from
jack-cy-yu,
oder-chiou and
shumingfan
as code owners
November 3, 2026 14:32
There was a problem hiding this comment.
🟢 Approval recommended
The SoundWire bandwidth-allocation changes have no active review issues blocking approval.
No open issues 🎉
What changed in this PR
This follow-up refines SoundWire bandwidth allocation for multi-link failure handling by explicitly distinguishing stream preparation from depreparation.
Changes:
- Add a
prepareflag to the bandwidth-computation interface and update callers and controller callbacks. - Include the current stream in preparation calculations even when its state is still
DEPREPARED. - Limit lane selection to the stream being prepared and clear stale lane assignments when preparing on lane 0.
| File | Description |
|---|---|
include/linux/soundwire/sdw.h |
Extend and document the computation interface with the prepare flag. |
drivers/soundwire/stream.c |
Pass the appropriate flag during preparation and depreparation. |
drivers/soundwire/qcom.c |
Update the Qualcomm callback signature to match the interface. |
drivers/soundwire/generic_bandwidth_allocation.c |
Adjust stream inclusion and lane assignment using explicit preparation context. |
drivers/soundwire/amd_manager.c |
Update the AMD callback signature to match the interface. |
Review effort: Balanced
ujfalusi
reviewed
Nov 4, 2026
ujfalusi
left a comment
ujfalusi
left a comment
Collaborator
There was a problem hiding this comment.
I'm not sure if I follow all the changes, I guess I should trust your testing?
| (m_rt->stream == stream || | ||
| (m_rt->stream->state != SDW_STREAM_ENABLED && | ||
| m_rt->stream->state != SDW_STREAM_PREPARED && | ||
| m_rt->stream->state != SDW_STREAM_DISABLED))) |
Collaborator
There was a problem hiding this comment.
Now, this is hard to decode....
Collaborator
Author
There was a problem hiding this comment.
_sdw_prepare_stream() could be called when stream->state = SDW_STREAM_CONFIGURED, SDW_STREAM_DEPREPARED or SDW_STREAM_DISABLED, but the previous code only test if (stream->state != SDW_STREAM_CONFIGURED), that skip the case when _sdw_prepare_stream() is called when stream state transfers from deprepare → prepare to prepare. Therefore, I changed the if (stream->state != SDW_STREAM_CONFIGURED) condition to if (prepare) and merge the all of the conditions together. I am happy to change it to
if (m_rt->stream == stream) {
if (!prepare)
continue;
} else {
if (m_rt->stream->state != SDW_STREAM_ENABLED &&
m_rt->stream->state != SDW_STREAM_PREPARED &&
m_rt->stream->state != SDW_STREAM_DISABLED)
continue;
}
bardliao
force-pushed
the
fixup-bandwidth-leak
branch
from
November 9, 2026 14:18
07e21a6 to
37f5285
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The reversed preparation condition excludes the current stream from bandwidth calculations, causing zero-width invalid transport windows and incorrectly counting departing streams during depreparation.
1 open issue
Review effort: Balanced
sdw_prepare_stream() accepts a stream in the DEPREPARED state without changing its state before parameter computation. However, the generic allocator excludes DEPREPARED streams from rate groups and port computation, and only includes a CONFIGURED current stream in group payload calculations. Consequently, preparing an existing stream again can reserve bandwidth without computing the corresponding transport parameters. The filters need to distinguish preparing that stream again from depreparing it. Pass the prepare context through the group and port helpers. Include the current stream during prepare even when it is still DEPREPARED, while preserving the filtering of all other runtimes and the behavior during deprepare. This supports preparing again without temporarily changing stream state. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
…n failure A failed prepare restores the bus parameters but leaves the master and peripheral port lane assignments unchanged. If a subsequent prepare selects lane 0, the existing nonzero-lane-only assignment path leaves the ports using the stale lanes from the failed attempt. Rather than saving and restoring the original port lanes on failure, apply the newly computed lane assignments on each successful bus parameter calculation during prepare. Explicitly clear the current stream's master and peripheral port lanes when lane 0 is selected; the existing path already updates them when a nonzero lane is selected. This ensures that a subsequent prepare uses the newly selected lanes, including when retrying with the same port runtimes, instead of relying on the lane values left by the failed attempt. Lane assignments are not changed during deprepare. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
bardliao
force-pushed
the
fixup-bandwidth-leak
branch
from
November 10, 2026 02:47
37f5285 to
f64b098
Compare
ujfalusi
approved these changes
Nov 10, 2026
ujfalusi
left a comment
ujfalusi
left a comment
Collaborator
There was a problem hiding this comment.
I trust this works as described
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.
To address review comments form sashiko-bot@kernel.org
https://sashiko.dev/#/patchset/20261102060437.824814-1-yung-chuan.liao@linux.intel.com?part=2