[PATCH v3 0/5] soundwire: Fix bandwidth leak in multi-link aggregation failure

[PATCH v3 0/5] soundwire: Fix bandwidth leak in multi-link aggregation failure

[Bard Liao]

The existing code only copies the params from the first link to the
bus params, but it doesn't copy the bandwidth from the other links.
This can lead to a bandwidth leak when multi-link aggregation fails.
Fix this by saving a snapshot of each bus's parameters before making
any modifications.

Changes in v3:
- Add Fixes and Cc stable tags to "Select the correct runtime for lane
  allocation".
- Fix bandwidth leak in multi-link aggregation failure:
  - Reset the lanes of the stream in the rollback path so they match
    the restored lane_used_bandwidth. This is skipped when the
    parameters were not updated.
  - Keep curr_bank and next_bank when restoring the snapshot, as the
    register bank may already have been switched when the failure
    happens.
  - Update the commit message accordingly.

Changes in v2:
- Fix lane and rate group handling in the generic bandwidth allocation
  that the rollback depends on:
  - Pass the prepare/deprepare context to compute_params().
  - Select the runtime of the stream being prepared for lane allocation
    instead of the first runtime on the bus, which could be an already
    running stream.
  - Include the current stream in the group and port computation when
    it is prepared again from the DEPREPARED state.
- Reset the lane of the stream being prepared to 0 when no multi-lane
  is selected, so the lane assignment matches the restored
  lane_used_bandwidth.

Bard Liao (4):
  soundwire: move lane bandwidth into bus params
  soundwire: Pass prepare context to parameter computation
  soundwire: Select the correct runtime for lane allocation
  soundwire: Include the current stream when preparing again

David Lin (1):
  soundwire: stream: Fix bandwidth leak in multi-link aggregation
    failure

 drivers/soundwire/amd_manager.c               |  3 +-
 drivers/soundwire/bus.h                       |  3 +
 .../soundwire/generic_bandwidth_allocation.c  | 81 ++++++++++++++-----
 drivers/soundwire/qcom.c                      |  3 +-
 drivers/soundwire/stream.c                    | 48 +++++++++--
 include/linux/soundwire/sdw.h                 | 13 +--
 6 files changed, 115 insertions(+), 36 deletions(-)

-- 
2.43.0

[PATCH v3 1/5] soundwire: move lane bandwidth into bus params

[Bard Liao]

lane_used_bandwidth is used to store the used bandwidth for each lane.
When we restore the bus->params, the lane_used_bandwidth value will not
be restored in current code. Move it to sdw_bus_params to fix the issue.

Fixes: 366fd59fb894 ("soundwire: generic_bandwidth_allocation: add lane in sdw_group_params")
Cc: stable@vger.kernel.org
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: David Lin <david.lin@intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
---
 drivers/soundwire/generic_bandwidth_allocation.c | 8 ++++----
 drivers/soundwire/stream.c                       | 4 ++--
 include/linux/soundwire/sdw.h                    | 4 ++--
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/soundwire/generic_bandwidth_allocation.c b/drivers/soundwire/generic_bandwidth_allocation.c
index 30a495abe19e..3d6b9c895e46 100644
--- a/drivers/soundwire/generic_bandwidth_allocation.c
+++ b/drivers/soundwire/generic_bandwidth_allocation.c
@@ -196,7 +196,7 @@ static void _sdw_compute_port_params(struct sdw_bus *bus,
 
 	/* Run loop for all groups to compute transport parameters */
 	for (l = 0; l < SDW_MAX_LANES; l++) {
-		if (l > 0 && !bus->lane_used_bandwidth[l])
+		if (l > 0 && !bus->params.lane_used_bandwidth[l])
 			continue;
 		/* reset hstop for each lane */
 		hstop = bus->params.col - 1;
@@ -267,7 +267,7 @@ static int sdw_compute_group_params(struct sdw_bus *bus,
 	}
 
 	for (l = 0; l < SDW_MAX_LANES; l++) {
-		if (l > 0 && !bus->lane_used_bandwidth[l])
+		if (l > 0 && !bus->params.lane_used_bandwidth[l])
 			continue;
 		/* reset column_needed for each lane */
 		column_needed = 0;
@@ -507,7 +507,7 @@ static int get_manager_lane(struct sdw_bus *bus, struct sdw_master_runtime *m_rt
 					      m_rt->stream->params.bps;
 		}
 		if (required_bandwidth <=
-		    curr_dr_freq - bus->lane_used_bandwidth[l]) {
+		    curr_dr_freq - bus->params.lane_used_bandwidth[l]) {
 			/* Check if m_lane is connected to all Peripherals */
 			if (!is_lane_connected_to_all_peripherals(m_rt,
 				slave_prop->lane_maps[l])) {
@@ -518,7 +518,7 @@ static int get_manager_lane(struct sdw_bus *bus, struct sdw_master_runtime *m_rt
 			}
 			m_lane = slave_prop->lane_maps[l];
 			dev_dbg(&s_rt->slave->dev, "M lane %d is used\n", m_lane);
-			bus->lane_used_bandwidth[l] += required_bandwidth;
+			bus->params.lane_used_bandwidth[l] += required_bandwidth;
 			/*
 			 * Use non-zero manager lane, subtract the lane 0
 			 * bandwidth that is already calculated
diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 31db368ba24b..1d74aa982a4c 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1794,8 +1794,8 @@ static int _sdw_deprepare_stream(struct sdw_stream_runtime *stream)
 			bandwidth = m_rt->stream->params.rate * hweight32(p_rt->ch_mask) *
 				    m_rt->stream->params.bps;
 			multi_lane_bandwidth += bandwidth;
-			bus->lane_used_bandwidth[p_rt->lane] -= bandwidth;
-			if (!bus->lane_used_bandwidth[p_rt->lane])
+			bus->params.lane_used_bandwidth[p_rt->lane] -= bandwidth;
+			if (!bus->params.lane_used_bandwidth[p_rt->lane])
 				p_rt->lane = 0;
 		}
 		/* TODO: Update this during Device-Device support */
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index f710e5932b4b..fff30bb2d9ed 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -592,6 +592,7 @@ enum sdw_port_prep_ops {
  * @bandwidth: Current bandwidth
  * @col: Active columns
  * @row: Active rows
+ * @lane_used_bandwidth: how much bandwidth in bits per second is used by each lane
  * @s_data_mode: NORMAL, STATIC or PRBS mode for all Slave ports
  * @m_data_mode: NORMAL, STATIC or PRBS mode for all Master ports. The value
  * should be the same to detect transmission issues, but can be different to
@@ -605,6 +606,7 @@ struct sdw_bus_params {
 	unsigned int bandwidth;
 	unsigned int col;
 	unsigned int row;
+	unsigned int lane_used_bandwidth[SDW_MAX_LANES];
 	int s_data_mode;
 	int m_data_mode;
 };
@@ -1028,7 +1030,6 @@ struct sdw_stream_runtime {
  * @multi_link: Store bus property that indicates if multi links
  * are supported. This flag is populated by drivers after reading
  * appropriate firmware (ACPI/DT).
- * @lane_used_bandwidth: how much bandwidth in bits per second is used by each lane
  */
 struct sdw_bus {
 	struct device *dev;
@@ -1063,7 +1064,6 @@ struct sdw_bus {
 	struct dentry *debugfs;
 #endif
 	bool multi_link;
-	unsigned int lane_used_bandwidth[SDW_MAX_LANES];
 };
 
 struct sdw_stream_runtime *sdw_alloc_stream(const char *stream_name, enum sdw_stream_type type);
-- 
2.43.0

[PATCH v3 2/5] soundwire: Pass prepare context to parameter computation

[Bard Liao]

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>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
---
 drivers/soundwire/amd_manager.c                  | 3 ++-
 drivers/soundwire/generic_bandwidth_allocation.c | 4 +++-
 drivers/soundwire/qcom.c                         | 3 ++-
 drivers/soundwire/stream.c                       | 4 ++--
 include/linux/soundwire/sdw.h                    | 9 ++++++---
 5 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/soundwire/amd_manager.c b/drivers/soundwire/amd_manager.c
index a3316efdf8ac..f40a3af4351f 100644
--- a/drivers/soundwire/amd_manager.c
+++ b/drivers/soundwire/amd_manager.c
@@ -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};
diff --git a/drivers/soundwire/generic_bandwidth_allocation.c b/drivers/soundwire/generic_bandwidth_allocation.c
index 3d6b9c895e46..1250fe21a3ff 100644
--- a/drivers/soundwire/generic_bandwidth_allocation.c
+++ b/drivers/soundwire/generic_bandwidth_allocation.c
@@ -657,8 +657,10 @@ 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;
 
diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 6e0c5fb10efa..73d3bdc4afc2 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -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;
diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 1d74aa982a4c..06f051f03a21 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1502,7 +1502,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);
@@ -1804,7 +1804,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);
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index fff30bb2d9ed..0e37b7d5ec48 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -1019,7 +1019,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
@@ -1054,7 +1055,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;
@@ -1069,7 +1071,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,
-- 
2.43.0

[PATCH v3 3/5] soundwire: Select the correct runtime for lane allocation

[Bard Liao]

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.

Fixes: 7a30292ffa2a ("soundwire: generic_bandwidth_allocation: select data lane")
Cc: stable@vger.kernel.org
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
---
 .../soundwire/generic_bandwidth_allocation.c  | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/soundwire/generic_bandwidth_allocation.c b/drivers/soundwire/generic_bandwidth_allocation.c
index 1250fe21a3ff..e8e68dbe180b 100644
--- a/drivers/soundwire/generic_bandwidth_allocation.c
+++ b/drivers/soundwire/generic_bandwidth_allocation.c
@@ -536,8 +536,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;
@@ -580,6 +583,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.
+			 */
+			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
@@ -627,8 +638,8 @@ 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;
@@ -665,7 +676,7 @@ int sdw_compute_params(struct sdw_bus *bus, struct sdw_stream_runtime *stream,
 	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;
 
-- 
2.43.0

[PATCH v3 4/5] soundwire: Include the current stream when preparing again

[Bard Liao]

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>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
---
 .../soundwire/generic_bandwidth_allocation.c  | 36 ++++++++++++-------
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/drivers/soundwire/generic_bandwidth_allocation.c b/drivers/soundwire/generic_bandwidth_allocation.c
index e8e68dbe180b..9bacde2c67a1 100644
--- a/drivers/soundwire/generic_bandwidth_allocation.c
+++ b/drivers/soundwire/generic_bandwidth_allocation.c
@@ -188,7 +188,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;
@@ -209,9 +210,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);
 			}
@@ -224,7 +228,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;
@@ -242,7 +246,7 @@ 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)
+			if (!prepare)
 				continue;
 		} else {
 			/*
@@ -331,7 +335,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;
@@ -352,7 +357,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;
@@ -383,14 +393,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;
 
@@ -404,11 +416,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);
@@ -686,7 +698,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;
-- 
2.43.0

[PATCH v3 5/5] soundwire: stream: Fix bandwidth leak in multi-link aggregation failure

[Bard Liao]

From: David Lin <david.lin@intel.com>

When preparing a multi-link aggregation stream, a failure on a subsequent
link during _sdw_prepare_stream() causes the stream preparation to fail.
However, the bandwidth allocated on the preceding successful links is never
released, causing a cumulative bandwidth leak.

Fix this by saving a snapshot of each bus's parameters before making any
modifications. In case of failure, we iterate through the stream's master
list and restore each bus to its original parameter state, ensuring an
atomic rollback and preventing any bandwidth leaks.

The lane assignments are stored in the port runtimes rather than in
bus->params, so they are not restored with the snapshot. Reset the lanes of
the stream to 0 during the rollback so that they match the restored
lane_used_bandwidth. Otherwise, the stale lanes would be used by the port
computation of other streams on the same bus. Skip this when the parameters
were not updated, as the lanes are still valid in that case.

A previous prepare can also leave a stale lane behind, because
_sdw_deprepare_stream() resets p_rt->lane only when the lane's used
bandwidth drops to zero. Reset the lanes of the stream being prepared in
sdw_compute_bus_params() when no multi-lane is selected.

Keep curr_bank and next_bank when restoring the snapshot. The failure may
happen after the register bank has already been switched, and restoring the
old values would make them differ from the hardware.

Fixes: 7a30292ffa2a ("soundwire: generic_bandwidth_allocation: select data lane")
Cc: stable@vger.kernel.org
Signed-off-by: David Lin <david.lin@intel.com>
Co-developed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
---
 drivers/soundwire/bus.h                       |  3 ++
 .../soundwire/generic_bandwidth_allocation.c  | 14 +++++++
 drivers/soundwire/stream.c                    | 40 +++++++++++++++++--
 3 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/drivers/soundwire/bus.h b/drivers/soundwire/bus.h
index 44e4f5193917..476387b70472 100644
--- a/drivers/soundwire/bus.h
+++ b/drivers/soundwire/bus.h
@@ -155,6 +155,8 @@ struct sdw_slave_runtime {
  *
  * @bus: Bus handle
  * @stream: Stream runtime handle
+ * @params_backup: Snapshot of original bus parameters saved before stream
+ * preparation, used for rollback recovery on failure.
  * @direction: Data direction for Master
  * @ch_count: Number of channels handled by the Master for
  * this stream, can be zero.
@@ -166,6 +168,7 @@ struct sdw_slave_runtime {
 struct sdw_master_runtime {
 	struct sdw_bus *bus;
 	struct sdw_stream_runtime *stream;
+	struct sdw_bus_params params_backup;
 	enum sdw_data_direction direction;
 	unsigned int ch_count;
 	struct list_head slave_rt_list;
diff --git a/drivers/soundwire/generic_bandwidth_allocation.c b/drivers/soundwire/generic_bandwidth_allocation.c
index 9bacde2c67a1..214a1be6c33e 100644
--- a/drivers/soundwire/generic_bandwidth_allocation.c
+++ b/drivers/soundwire/generic_bandwidth_allocation.c
@@ -656,6 +656,20 @@ static int sdw_compute_bus_params(struct sdw_bus *bus,
 		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)
diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 06f051f03a21..bc0e2d5eaf0c 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1477,21 +1477,26 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime *stream,
 			       bool update_params)
 {
 	struct sdw_master_runtime *m_rt;
+	struct sdw_slave_runtime *s_rt;
+	struct sdw_port_runtime *p_rt;
 	struct sdw_bus *bus;
 	struct sdw_master_prop *prop;
-	struct sdw_bus_params params;
 	int ret;
 
+	/* Pre-pass: snapshot original bus parameters for all masters */
+	list_for_each_entry(m_rt, &stream->master_list, stream_node)
+		m_rt->params_backup = m_rt->bus->params;
+
 	/* Prepare  Master(s) and Slave(s) port(s) associated with stream */
 	list_for_each_entry(m_rt, &stream->master_list, stream_node) {
 		bus = m_rt->bus;
 		prop = &bus->prop;
-		memcpy(&params, &bus->params, sizeof(params));
 
 		/* TODO: Support Asynchronous mode */
 		if ((prop->max_clk_freq % stream->params.rate) != 0) {
 			dev_err(bus->dev, "Async mode not supported\n");
-			return -EINVAL;
+			ret = -EINVAL;
+			goto restore_params;
 		}
 
 		if (update_params) {
@@ -1542,7 +1547,34 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime *stream,
 	return ret;
 
 restore_params:
-	memcpy(&bus->params, &params, sizeof(params));
+	/*
+	 * Revert all managers to their snapshotted original parameters.
+	 * Any manager that was not modified will safely revert to its own original state.
+	 */
+	list_for_each_entry(m_rt, &stream->master_list, stream_node) {
+		enum sdw_reg_bank curr_bank = m_rt->bus->params.curr_bank;
+		enum sdw_reg_bank next_bank = m_rt->bus->params.next_bank;
+
+		m_rt->bus->params = m_rt->params_backup;
+		/* The hardware bank may already have been switched */
+		m_rt->bus->params.curr_bank = curr_bank;
+		m_rt->bus->params.next_bank = next_bank;
+
+		/*
+		 * The lane_used_bandwidth of this stream was reverted above, so clear its
+		 * lanes too; a stale lane would be used by other streams' port computation.
+		 */
+		if (!update_params)
+			continue;
+
+		list_for_each_entry(p_rt, &m_rt->port_list, port_node)
+			p_rt->lane = 0;
+
+		list_for_each_entry(s_rt, &m_rt->slave_rt_list, m_rt_node)
+			list_for_each_entry(p_rt, &s_rt->port_list, port_node)
+				p_rt->lane = 0;
+	}
+
 	return ret;
 }
 
-- 
2.43.0

Re: [PATCH v3 4/5] soundwire: Include the current stream when preparing again

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] soundwire: missing Fixes trailers for DEPREPARED stream filter
  commits
--

commit 0e099f6ac5f748061c8ae06b3d0b581ba6e28f1f
Author: Bard Liao <yung-chuan.liao@linux.intel.com>

> soundwire: Include the current stream when preparing again
>
> 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>
> Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>

[Severity: Low]
This fixes a bug where re-preparing a DEPREPARED stream skips transport and
port parameter computation due to state filters introduced in earlier
commits.  Should this include Fixes tags for the commits that added those
checks:

  Fixes: fdd1faeeec14 ("soundwire: generic_bandwidth_allocation: skip DEPREPARED streams")
  Fixes: 25befdf32aa4 ("soundwire: generic_bandwidth_allocation: count the bandwidth of active streams only")
  Fixes: 62ada17a6217 ("soundwire: only compute port params in specific stream states")

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112141905.1372770-1-yung-chuan.liao@linux.intel.com?part=4