Skip to content

fixup! soundwire: stream: Fix bandwidth leak in multi-link aggregation failure #6014

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bardliao wants to merge 4 commits into
base: topic/sof-dev
Choose a base branch
from
Open

fixup! soundwire: stream: Fix bandwidth leak in multi-link aggregation failure #6014

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7869d8b
soundwire: Pass prepare context to parameter computation
bardliao Nov 3, 2026
77ffd2b
soundwire: Select the correct runtime for lane allocation
bardliao Nov 3, 2026
cb335f0
soundwire: Include the current stream when preparing again
bardliao Nov 3, 2026
07e21a6
fixup! soundwire: stream: Fix bandwidth leak in multi-link aggregatio…
bardliao Nov 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion drivers/soundwire/amd_manager.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,8 @@ static u32 amd_sdw_read_ping_status(struct sdw_bus *bus)
return slave_stat;
}

static int amd_sdw_compute_params(struct sdw_bus *bus, struct sdw_stream_runtime *stream)
static int amd_sdw_compute_params(struct sdw_bus *bus, struct sdw_stream_runtime *stream,
bool prepare)
{
struct amd_sdw_manager *amd_manager = to_amd_sdw(bus);
struct sdw_transport_data t_data = {0};
Expand Down
95 changes: 65 additions & 30 deletions drivers/soundwire/generic_bandwidth_allocation.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,8 @@ static void sdw_compute_master_ports(struct sdw_master_runtime *m_rt,
}

static void _sdw_compute_port_params(struct sdw_bus *bus,
struct sdw_group_params *params, int count)
struct sdw_group_params *params, int count,
struct sdw_stream_runtime *stream, bool prepare)
{
struct sdw_master_runtime *m_rt;
int port_bo, i, l;
Expand All @@ -211,9 +212,12 @@ static void _sdw_compute_port_params(struct sdw_bus *bus,
/*
* Only runtimes with CONFIGURED, PREPARED, ENABLED, and DISABLED
* states should be included in the bandwidth calculation.
* Include the current stream during prepare even if it is
* still in the DEPREPARED state.
*/
if (m_rt->stream->state > SDW_STREAM_DISABLED ||
m_rt->stream->state < SDW_STREAM_CONFIGURED)
if (!(prepare && m_rt->stream == stream) &&
(m_rt->stream->state > SDW_STREAM_DISABLED ||
m_rt->stream->state < SDW_STREAM_CONFIGURED))
continue;
sdw_compute_master_ports(m_rt, &params[i], &port_bo, hstop);
}
Expand All @@ -226,7 +230,7 @@ static void _sdw_compute_port_params(struct sdw_bus *bus,
static int sdw_compute_group_params(struct sdw_bus *bus,
struct sdw_stream_runtime *stream,
struct sdw_group_params *params,
struct sdw_group *group)
struct sdw_group *group, bool prepare)
{
struct sdw_master_runtime *m_rt;
struct sdw_port_runtime *p_rt;
Expand All @@ -242,20 +246,16 @@ static int sdw_compute_group_params(struct sdw_bus *bus,
}

list_for_each_entry(m_rt, &bus->m_rt_list, bus_node) {
if (m_rt->stream == stream) {
/* Only runtime during prepare should be added */
if (stream->state != SDW_STREAM_CONFIGURED)
continue;
} else {
/*
* Include runtimes with running (ENABLED/PREPARED state) and
* paused (DISABLED state) streams
*/
if (m_rt->stream->state != SDW_STREAM_ENABLED &&
m_rt->stream->state != SDW_STREAM_PREPARED &&
m_rt->stream->state != SDW_STREAM_DISABLED)
continue;
}
/*
* Include the current stream during prepare even if it is still
* DEPREPARED. Otherwise, include only other running or paused streams.
*/
if (!(prepare && m_rt->stream == stream) &&
(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)))

ghost Nov 4, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Now, this is hard to decode....

ghost Nov 5, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

_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;
}

continue;
list_for_each_entry(p_rt, &m_rt->port_list, port_node) {
rate = m_rt->stream->params.rate;
bps = m_rt->stream->params.bps;
Expand Down Expand Up @@ -333,7 +333,8 @@ static int sdw_add_element_group_count(struct sdw_group *group,
}

static int sdw_get_group_count(struct sdw_bus *bus,
struct sdw_group *group)
struct sdw_group *group,
struct sdw_stream_runtime *stream, bool prepare)
{
struct sdw_master_runtime *m_rt;
struct sdw_port_runtime *p_rt;
Expand All @@ -354,7 +355,12 @@ static int sdw_get_group_count(struct sdw_bus *bus,
}

list_for_each_entry(m_rt, &bus->m_rt_list, bus_node) {
if (m_rt->stream->state == SDW_STREAM_DEPREPARED)
/*
* Include the current stream during prepare even if it is still
* in the DEPREPARED state.
*/
if (!(prepare && m_rt->stream == stream) &&
m_rt->stream->state == SDW_STREAM_DEPREPARED)
continue;

rate = m_rt->stream->params.rate;
Expand Down Expand Up @@ -385,14 +391,16 @@ static int sdw_get_group_count(struct sdw_bus *bus,
*
* @bus: SDW Bus instance
* @stream: Soundwire stream
* @prepare: True when preparing, false when depreparing
*/
static int sdw_compute_port_params(struct sdw_bus *bus, struct sdw_stream_runtime *stream)
static int sdw_compute_port_params(struct sdw_bus *bus, struct sdw_stream_runtime *stream,
bool prepare)
{
struct sdw_group_params *params = NULL;
struct sdw_group group;
int ret;

ret = sdw_get_group_count(bus, &group);
ret = sdw_get_group_count(bus, &group, stream, prepare);
if (ret < 0)
return ret;

Expand All @@ -406,11 +414,11 @@ static int sdw_compute_port_params(struct sdw_bus *bus, struct sdw_stream_runtim
}

/* Compute transport parameters for grouped streams */
ret = sdw_compute_group_params(bus, stream, params, &group);
ret = sdw_compute_group_params(bus, stream, params, &group, prepare);
if (ret < 0)
goto free_params;

_sdw_compute_port_params(bus, params, group.count);
_sdw_compute_port_params(bus, params, group.count, stream, prepare);

free_params:
kfree(params);
Expand Down Expand Up @@ -538,8 +546,11 @@ static int get_manager_lane(struct sdw_bus *bus, struct sdw_master_runtime *m_rt
* sdw_compute_bus_params: Compute bus parameters
*
* @bus: SDW Bus instance
* @stream: SoundWire stream being prepared or deprepared
* @prepare: True when preparing, false when depreparing
*/
static int sdw_compute_bus_params(struct sdw_bus *bus)
static int sdw_compute_bus_params(struct sdw_bus *bus,
struct sdw_stream_runtime *stream, bool prepare)
{
struct sdw_master_prop *mstr_prop = &bus->prop;
struct sdw_slave_prop *slave_prop;
Expand Down Expand Up @@ -582,6 +593,14 @@ static int sdw_compute_bus_params(struct sdw_bus *bus)
break;

list_for_each_entry(m_rt, &bus->m_rt_list, bus_node) {
/*
* Select only the current stream's runtime. Check prepare
* to avoid computing m_lane during deprepare, when the
* stream's bandwidth reservation has already been released.

ghost Nov 4, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

on deprepare you don't need to re-compute ? Remove the leaving bandwidth use?

'Only compute the m_lane when preparing the current stream' ?

ghost Nov 5, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We do re-compute the required clock rate. Only get_manager_lane() is skipped here. The lane bandwidth was computed at _sdw_deprepare_stream(). https://github.com/thesofproject/linux/blob/topic/sof-dev/drivers/soundwire/stream.c#L1814

*/
if (m_rt->stream != stream || !prepare)
continue;

/*
* Get the first s_rt that will be used to find the available lane that
* can be used. No need to check all Peripherals because we can't use
Expand Down Expand Up @@ -629,12 +648,26 @@ static int sdw_compute_bus_params(struct sdw_bus *bus)
}
}
/*
* Set Manager lanes. Configure the last m_rt in bus->m_rt_list only since
* we don't want to touch other m_rts that are already working.
* Set Manager lanes for the stream being prepared without changing
* other runtimes that are already working.
*/
list_for_each_entry(m_p_rt, &m_rt->port_list, port_node) {
m_p_rt->lane = m_lane;
}
} else if (prepare) {
/* Clear stale lane assignments when preparing on lane 0. */
list_for_each_entry(m_rt, &bus->m_rt_list, bus_node) {
if (m_rt->stream != stream)
continue;

list_for_each_entry(m_p_rt, &m_rt->port_list, port_node)
m_p_rt->lane = 0;

list_for_each_entry(s_rt, &m_rt->slave_rt_list, m_rt_node)
list_for_each_entry(s_p_rt, &s_rt->port_list, port_node)
s_p_rt->lane = 0;
break;
}
}

if (!mstr_prop->default_frame_rate || !mstr_prop->default_row)
Expand All @@ -659,13 +692,15 @@ static int sdw_compute_bus_params(struct sdw_bus *bus)
*
* @bus: SDW Bus instance
* @stream: Soundwire stream
* @prepare: True when preparing, false when depreparing
*/
int sdw_compute_params(struct sdw_bus *bus, struct sdw_stream_runtime *stream)
int sdw_compute_params(struct sdw_bus *bus, struct sdw_stream_runtime *stream,
bool prepare)
{
int ret;

/* Computes clock frequency, frame shape and frame frequency */
ret = sdw_compute_bus_params(bus);
ret = sdw_compute_bus_params(bus, stream, prepare);
if (ret < 0)
return ret;

Expand All @@ -675,7 +710,7 @@ int sdw_compute_params(struct sdw_bus *bus, struct sdw_stream_runtime *stream)
}

/* Compute transport and port params */
ret = sdw_compute_port_params(bus, stream);
ret = sdw_compute_port_params(bus, stream, prepare);
if (ret < 0) {
dev_err(bus->dev, "Compute transport params failed: %d\n", ret);
return ret;
Expand Down
3 changes: 2 additions & 1 deletion drivers/soundwire/qcom.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1215,7 +1215,8 @@ static const struct sdw_master_ops qcom_swrm_ops = {
.pre_bank_switch = qcom_swrm_pre_bank_switch,
};

static int qcom_swrm_compute_params(struct sdw_bus *bus, struct sdw_stream_runtime *stream)
static int qcom_swrm_compute_params(struct sdw_bus *bus, struct sdw_stream_runtime *stream,
bool prepare)
{
struct qcom_swrm_ctrl *ctrl = to_qcom_sdw(bus);
struct sdw_master_runtime *m_rt;
Expand Down
4 changes: 2 additions & 2 deletions drivers/soundwire/stream.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1513,7 +1513,7 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime *stream,

/* Compute params */
if (bus->compute_params) {
ret = bus->compute_params(bus, stream);
ret = bus->compute_params(bus, stream, true);
if (ret < 0) {
dev_err(bus->dev, "Compute params failed: %d\n",
ret);
Expand Down Expand Up @@ -1821,7 +1821,7 @@ static int _sdw_deprepare_stream(struct sdw_stream_runtime *stream)

/* Compute params */
if (bus->compute_params) {
ret = bus->compute_params(bus, stream);
ret = bus->compute_params(bus, stream, false);
if (ret < 0) {
dev_err(bus->dev, "Compute params failed: %d\n",
ret);
Expand Down
9 changes: 6 additions & 3 deletions include/linux/soundwire/sdw.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1021,7 +1021,8 @@ struct sdw_stream_runtime {
* @controller_id: system-unique controller ID. If set to -1, the bus @id will be used.
* @link_id: Link id number, can be 0 to N, unique for each Controller
* @id: bus system-wide unique id
* @compute_params: points to Bus resource management implementation
* @compute_params: points to Bus resource management implementation; the prepare
* argument is true when preparing a stream and false when depreparing it
* @assigned: Bitmap for Slave device numbers.
* Bit set implies used number, bit clear implies unused number.
* @clk_stop_timeout: Clock stop timeout computed
Expand Down Expand Up @@ -1058,7 +1059,8 @@ struct sdw_bus {
int controller_id;
unsigned int link_id;
int id;
int (*compute_params)(struct sdw_bus *bus, struct sdw_stream_runtime *stream);
int (*compute_params)(struct sdw_bus *bus, struct sdw_stream_runtime *stream,
bool prepare);
DECLARE_BITMAP(assigned, SDW_MAX_DEVICES);
unsigned int clk_stop_timeout;
u32 bank_switch_timeout;
Expand All @@ -1074,7 +1076,8 @@ struct sdw_bus {
struct sdw_stream_runtime *sdw_alloc_stream(const char *stream_name, enum sdw_stream_type type);
void sdw_release_stream(struct sdw_stream_runtime *stream);

int sdw_compute_params(struct sdw_bus *bus, struct sdw_stream_runtime *stream);
int sdw_compute_params(struct sdw_bus *bus, struct sdw_stream_runtime *stream,
bool prepare);

int sdw_stream_add_master(struct sdw_bus *bus,
struct sdw_stream_config *stream_config,
Expand Down