soundwire: add higher bus clock rate support #5160

bardliao · 2024-08-28T03:54:39Z

Refactor SoundWire clock support. And add multi-lane support.

plbossart · 2024-08-28T07:59:39Z

if you select the 9.6 MHz clock, you also need to change the default_col_size to 8 instead of 4. Otherwise the frame size will not be correct.

drivers/soundwire/stream.c

bardliao · 2024-08-29T09:54:44Z

drivers/soundwire/generic_bandwidth_allocation.c

+	if (curr_dr_freq == 19200000)
+		mstr_prop->default_col = 8;
+	else
+		mstr_prop->default_col = 4;


@plbossart Do you have a rule of selecting frame shape? Currently, I use a hack to test.

I think you need to start from the root frequency.
19.2: default_row = 50
12: default_row = 125
12.288: default_row = 64

Then you need to take the bus frequency, multiply it by to to get actual DDR rate, divide it by the frame rate and again by default row and you get the default_col.

I think you need to start from the root frequency. 19.2: default_row = 50 12: default_row = 125 12.288: default_row = 64

Then you need to take the bus frequency, multiply it by to to get actual DDR rate, divide it by the frame rate and again by default row and you get the default_col.

Changed to default_col = curr_dr_freq / m_rt->stream->params.rate / mstr_prop->default_row;

drivers/soundwire/stream.c

plbossart · 2024-09-09T07:40:58Z

drivers/soundwire/generic_bandwidth_allocation.c

+					__func__, curr_dr_freq);
+				i++;
+				goto select_clk;
+			}


I am not sure why you would only increase the clock when there is a single lane. The clock should be increased even in the multi-lane case when there's no bandwidth left?

I must be missing something.

That is about the priority of using multi-lane or higher clock rate. At this point, we tried multi-lane but can't find an available lane and it will return -EINVAL if it reaches the highest clock rate. But yeah, I didn't consider about using multi-lane with higher clock rate. I assume that we always have enough bandwidth if we can find an extra lane. Let me redo this.

yes, lanes should be used before cranking the speed up, but an error should only be thrown when multilane+higher speed combined doesn't support the required bandwidth.

plbossart · 2024-09-09T07:43:02Z

drivers/soundwire/bus.c

+	 * frequency base and scale registers are required for SDCA
+	 * devices. They may also be used for 1.2+/non-SDCA devices.
+	 * Driver can set the property, we will need a DisCo property
+	 * to discover this case from platform firmware.


not able to follow this comment, consider rewording.

It was just copy and paste from the existing code.

maybe clarify the comment

"Driver can set the property directly, for now there's no DisCo property to discover support for the scaling registers from platform firmware'

bardliao · 2024-09-10T06:56:54Z

On the second thought, I think the existing code partially support dynamic clock change already. What missed is change frame shape dynamically. So, it will be better if we add the dynamic frame shape support commit first and multi-lane support commits late. It is more reviewable, and the logic is clearer.

drivers/soundwire/bus.c

plbossart · 2024-09-10T08:13:06Z

drivers/soundwire/bus.c

+	 * frequency base and scale registers are required for SDCA
+	 * devices. They may also be used for 1.2+/non-SDCA devices.
+	 * Driver can set the property, we will need a DisCo property
+	 * to discover this case from platform firmware.


maybe clarify the comment

"Driver can set the property directly, for now there's no DisCo property to discover support for the scaling registers from platform firmware'

drivers/soundwire/stream.c

plbossart · 2024-09-10T08:17:43Z

drivers/soundwire/generic_bandwidth_allocation.c

+		/* Check if default_col can be changed */
+		list_for_each_entry(s_rt, &m_rt->slave_rt_list, m_rt_node) {
+			if (!s_rt->slave->id.class_id) {
+				dev_err(&s_rt->slave->dev, "The Peripheral doesn't comply with SDCA\n");


you probably need a comment to explain that some devices expose the class ID but can't support dynamic scaling.

I would actually move this loop to a helper that can be modified/quirked if needed, e.g. is_clock_scaling_supported().

drivers/soundwire/generic_bandwidth_allocation.c

plbossart · 2024-09-10T08:21:15Z

drivers/soundwire/generic_bandwidth_allocation.c

+		frame_int = mstr_prop->default_row * mstr_prop->default_col;
+		frame_freq = curr_dr_freq / frame_int;
+
+		if ((curr_dr_freq - (frame_freq * SDW_FRAME_CTRL_BITS)) <


I think this is partially wrong, because we never use column0 for audio. so for a 50 row frame, this calculation would be off by 2 bits times the frame_freq. I think you need to use curr_dr_freq - default_row * frame_freq.

This is copy from sdw_select_row_col(). Do we need to do the same change in sdw_select_row_col(), too?

yeah, I think so. I don't think anyone can use column0 with 19.2 or 12.228 root frequencies. It's only possible for the 125-row frame but that's such a corner case.

But the formula could be simplifed to avoid a division btw:

curr_dr_freq - (curr_dr_freq / (sdw_rows[r] * sdw_cols[c]) * sdw_rows[r])

curr_d_freq - curr_dr_freq / sdw_cols[c] < bandwidth
curr_d_freq * (sdw_cols[c] - 1) < bandwidth * sdw_cols[c]

plbossart · 2024-09-10T08:22:22Z

drivers/soundwire/stream.c

-				return ret;
-			}
-		}
-


The clocks are always updated on prepare/deprepare, I am not sure why you want to remove this.

I am not sure if we really need to update clocks on both prepare/deprepare. We update clock in sdw_compute_bus_params(), but we still reserve the bit slots for the deprepared stream in sdw_compute_port_params(). That doesn't look correct to me.
This is to see if any issue will happen if I remove this. Looks like CI has told me that compute_params() can not be removed.

let me re-explain. We have two bank switches, one on prepare and one on trigger. This is intentional, we want to go to the 'new' frequency and let the clock toggle for some time before enabling all the channels on the trigger.

So the point is that we do need to update the parameters and reduce the clock on deprepare. I am not sure why the prepare and de-prepare parts would become asymmetrical?

let me re-explain. We have two bank switches, one on prepare and one on trigger. This is intentional, we want to go to the 'new' frequency and let the clock toggle for some time before enabling all the channels on the trigger.

So the point is that we do need to update the parameters and reduce the clock on deprepare.

Agree, but m_rt is removed after deprepared and list_for_each_entry(m_rt, &bus->m_rt_list, bus_node) is used by _sdw_compute_port_params(), sdw_compute_group_params(), etc. As a result, the bit slots of the deprepared stream are still reserved. If we reduce the clock, we will get a no sufficient bandwidth error.

I am not sure why the prepare and de-prepare parts would become asymmetrical?

I don't think it is asymmetrical. Below is the sequence when we start and stop aplay.
intel_hw_params
-> sdw_stream_add_master
asoc_sdw_prepare
-> sdw_prepare_stream

asoc_sdw_hw_free
-> sdw_deprepare_stream
intel_hw_free
-> sdw_stream_remove_master

Not following this. Do we agree when the stream is deprepared, it should use exactly zero bitslots? I don't understand why the bits would remain reserved.

but the code above still accounts for the bandwidth of the stream, so the stream is still taken into account.

It makes no sense to me to reduce the bandwidth count after performing the bandwidth allocation.

Sorry, completely lost here.

Moving the bandwidth reduction count after performing the bandwidth allocation is based on the fact that the m_rt has not been removed when we allocate the bit slots. To fix the issue, I can either move the bandwidth reduction count after performing the bandwidth as the current PR does or remove the m_rt before allocating the bit slots.
I choose the former because removing m_rt first seems break the add m_rt -> prepare -> deprepare ->remove m_rt sequence.
I know the side effects of moving the bandwidth reduction count after performing the bandwidth allocation is that we may keep the sdw bus clock or lane that is not needed. But it will happen only when we reach the threshold that need higher clock rate or more lane. So, I think it is acceptable.

That's a bit convoluted really.

The bits for the stream should no longer be accounted for after this routine:

int asoc_sdw_hw_free(struct snd_pcm_substream *substream) { struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); struct sdw_stream_runtime *sdw_stream; struct snd_soc_dai *dai; /* Find stream from first CPU DAI */ dai = snd_soc_rtd_to_cpu(rtd, 0); sdw_stream = snd_soc_dai_get_stream(dai, substream->stream); if (IS_ERR(sdw_stream)) { dev_err(rtd->dev, "no stream found for DAI %s\n", dai->name); return PTR_ERR(sdw_stream); } return sdw_deprepare_stream(sdw_stream); }

but the problem is that we have a half-baked dai handling and comments

static int intel_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { struct sdw_cdns *cdns = snd_soc_dai_get_drvdata(dai); struct sdw_intel *sdw = cdns_to_intel(cdns); struct sdw_cdns_dai_runtime *dai_runtime; int ret; dai_runtime = cdns->dai_runtime_array[dai->id]; if (!dai_runtime) return -EIO; /* * The sdw stream state will transition to RELEASED when stream-> * master_list is empty. So the stream state will transition to * DEPREPARED for the first cpu-dai and to RELEASED for the last * cpu-dai. */ ret = sdw_stream_remove_master(&cdns->bus, dai_runtime->stream); if (ret < 0) { dev_err(dai->dev, "remove master from stream %s failed: %d\n", dai_runtime->stream->name, ret); return ret; }

That comment is wrong, the stream state will transition to DEPREPARED in the dailink hw_free(), and then to RELEASED when the last DAI hw_free() is called

That still doesn't really tell me why the bits are not released during the sdw_deprepare_stream() part. There should be no dependency on the m_rt list.

asoc_sdw_hw_free() is called before intel_hw_free(). IOW, sdw_deprepare_stream() is called before sdw_stream_remove_master(). So, m_rt is not removed yet when sdw_deprepare_stream() is called.

_sdw_deprepare_stream
-> bus->compute_params(bus)
-> sdw_compute_params()
-> sdw_compute_bus_params(bus)
-> sdw_compute_port_params(bus)

port_bo = 1; list_for_each_entry(m_rt, &bus->m_rt_list, bus_node) { sdw_compute_master_ports(m_rt, &params[i], &port_bo, hstop); } hstop = hstop - params[i].hwidth;

And we calculate hstart, hstop, and offset in sdw_compute_master_ports()

t_data.hstart = hstart; t_data.hstop = hstop; t_data.block_offset = *port_bo; t_data.sub_block_offset = 0; (*port_bo) += bps * ch;

So that we allocate bit slots for each m_rt.

yeah, that's clearly wrong, the generic bit allocation should take the states into account, and skip the deprepared streams.

See #5177 for compile-tested-only proposal.

plbossart · 2024-09-10T08:24:24Z

drivers/soundwire/generic_bandwidth_allocation.c

@@ -353,6 +391,10 @@ static int sdw_compute_bus_params(struct sdw_bus *bus)
 		clk_buf = NULL;
 	}

+	m_rt = list_last_entry(&bus->m_rt_list, struct sdw_master_runtime, bus_node);
+	s_rt = list_first_entry(&m_rt->slave_rt_list, struct sdw_slave_runtime, m_rt_node);


not following what this is supposed to do and why you are not dealing with M and S in symmetrical ways, the last/first isn't clear to me at all.

I use the last m_rt is because that one is recently added to the bus and it is the m_rt that we need to handle. And for the s_rt, it doesn't really matter to use what s_rt in the m_rt. The s_rt will be used to find the peripheral lane, and we will check if the lane is connected to all peripherals.

I don't think that's correct @bardliao. When there is a new stream, we need to compute the parameters for all streams and all _rt contexts. Think of it this way: at any bank switch the position of all bits could change. It could very well be that the new stream will have higher priority in the bandwidth allocation than the new one.

IOW, the generic allocation is NOT a purely additive mechanism. At every bank switch, the bus allocation is a blank slate and ALL streams/ports can be re-programmed.

Changed to list_for_each_entry(m_rt, &bus->m_rt_list, bus_node) to check all m_rt

drivers/soundwire/stream.c

drivers/soundwire/generic_bandwidth_allocation.c

plbossart · 2024-09-13T08:08:27Z

drivers/soundwire/generic_bandwidth_allocation.c

@@ -380,6 +406,10 @@ static int sdw_compute_bus_params(struct sdw_bus *bus)
 		return -EINVAL;
 	}

+	/* Get the last m_rt that is recently added to the bus. */
+	m_rt = list_last_entry(&bus->m_rt_list, struct sdw_master_runtime, bus_node);
+	mstr_prop->default_col = curr_dr_freq / m_rt->stream->params.rate / mstr_prop->default_row;


oh no no no, big confusion here!

The frame shape need to be computed with the SoundWire frame rate, which can be different to all the stream rates. Think e.g. of the example I presented with a 96 kHz frame rate to support two streams, one at 48kHz and one at 48 kHz.

Right, right, fixed now.

plbossart

looks mostly good, I still think the code can be made a bit clearer/compact, see suggestions below.

plbossart · 2024-09-13T15:00:04Z

drivers/soundwire/generic_bandwidth_allocation.c

@@ -354,6 +375,10 @@ static int sdw_compute_bus_params(struct sdw_bus *bus)
 		clk_buf = NULL;
 	}

+	/* If dynamic scaling is supported, don't try higher freq */


logical inversion in comment, should be "NOT supported"?

drivers/soundwire/generic_bandwidth_allocation.c

…p_params" This reverts commit 72e43ef. Signed-off-by: Bard Liao <[email protected]>

ujfalusi

@bardliao, I'm still wrapping my head around the debug prints, sorry.

ujfalusi · 2024-10-21T07:43:55Z

drivers/soundwire/generic_bandwidth_allocation.c

+			if (slave_prop->lane_maps[i] == lane) {
+				dev_dbg(&s_rt->slave->dev,
+					"lane_maps[%d] is connected to %d\n",
+					i, slave_prop->lane_maps[i]);


Can we print this in a human readable fashion, like P lane %d is connected to M lane %d ?

ujfalusi · 2024-10-21T07:44:41Z

drivers/soundwire/generic_bandwidth_allocation.c

+			}
+		}
+		if (i == SDW_MAX_LANES) {
+			dev_dbg(&s_rt->slave->dev, "%d is not connected to %s\n",


Probably simplify it to: M lane %d is not connected (the dev_name(&s_rt->slave->dev) is printed by dev_dbg())

ujfalusi · 2024-10-21T07:47:04Z

drivers/soundwire/generic_bandwidth_allocation.c

+			if (!is_lane_connected_to_all_peripherals(m_rt,
+				slave_prop->lane_maps[l])) {
+				dev_dbg(bus->dev,
+					"some Peripherals are not connected to %d\n",


Not all Peripherals are not connected to M lane %d, M lane is not usable ? If it is correct.

ujfalusi · 2024-10-21T08:06:30Z

drivers/soundwire/generic_bandwidth_allocation.c

+			}
+			m_lane = slave_prop->lane_maps[l];
+			dev_dbg(&s_rt->slave->dev,
+				"M lane %d P lane %d can be used\n",


Used for what?
In theory a peripheral can be connected to multiple manager lane?

Within is_lane_connected_to_all_peripherals() we have already printed all the connections with mentioning P and M lane with the peripheral's device used for dev_dbg(), this is going to state the same but just a bit differently, so perhaps:
`M lane %d is used' and that's it?

Or consolidate the prints somehow?

ujfalusi · 2024-10-21T08:09:10Z

drivers/soundwire/generic_bandwidth_allocation.c

+			/*
+			 * Find available manager lanes that connected to the first Peripheral.
+			 * No need to check all Peripherals available lanes because we can't use
+			 * multi-lane if we can't find any available lane for the first Peripheral.


get_manager_lane() will check all peripherals lane connection via a call to is_lane_connected_to_all_peripherals().
Isn't this a bit contradicting to this comment?

That means something different. "No need to check all Peripherals" here means we only need to check the available lanes of the first Peripheral. Then, is_lane_connected_to_all_peripherals() will check if the lane can be used for all Peripherals. We don't need to repeat checking all Peripherals because one lane can not be used if the lane is not connected to the first Peripheral.

Reworded it. Hope it is more readable. :)

drivers/soundwire/generic_bandwidth_allocation.c

ujfalusi · 2024-10-21T11:55:28Z

drivers/soundwire/generic_bandwidth_allocation.c

+		for (i = 1; i < SDW_MAX_LANES; i++) {
+			if (slave_prop->lane_maps[i] == lane) {
+				dev_dbg(&s_rt->slave->dev,
+					"P lane %d is connected to M lane %d\n",


It might be better to swap the P and M lane in the sentence?
M lane %d is connected to P lane %d

ujfalusi · 2024-10-21T11:58:38Z

drivers/soundwire/generic_bandwidth_allocation.c

+			slave_prop = &s_rt->slave->prop;
+			for (l = 1; l < SDW_MAX_LANES; l++) {
+				if (slave_prop->lane_maps[l] == m_lane) {
+					dev_dbg(&s_rt->slave->dev, "Set Peripheral lane = %d\n", l);


Should we stick with P lane for consistency over Peripheral lane print use only here?
Set P lane %d for ports perhaps, if select makes sense in this context.

or do we even need to print anything in here?

drivers/soundwire/generic_bandwidth_allocation.c

ujfalusi

@bardliao, only one typo that I can see, sorry.

ujfalusi · 2024-10-24T06:47:09Z

drivers/soundwire/generic_bandwidth_allocation.c

+			if (!is_lane_connected_to_all_peripherals(m_rt,
+				slave_prop->lane_maps[l])) {
+				dev_dbg(bus->dev,
+					"Not all Peripherals are not connected to M lane %d\n",


you have a leftover 'not' word:
"Not all Peripherals are not connected to M lane %d\n"

ujfalusi

@bardliao, thank you for the updates, it looks good to me.

bardliao · 2024-10-25T06:20:53Z

@ranj063 Do you have any comment on this PR?

ranj063 · 2024-10-29T02:40:41Z

drivers/soundwire/stream.c

+	 * first mark the state as DEPREPARED so that it is not taken into account
+	 * for bit allocation
+	 */
+	state = stream->state;


this is already set during init

Removed, thanks.

drivers/soundwire/generic_bandwidth_allocation.c

ranj063 · 2024-10-29T02:46:24Z

drivers/soundwire/stream.c

+	else
+		addr1 = SDW_SCP_BUSCLOCK_SCALE_B0;
+
+	/* Progrem SDW_SCP_BUSCLOCK_SCALE is all Peripherals comply with SDCA */


typos: Progrem -> program & is all -> if all

drivers/soundwire/generic_bandwidth_allocation.c

drivers/soundwire/bus.h

lgirdwood

LGTM, just minor questions.

drivers/soundwire/generic_bandwidth_allocation.c

lgirdwood · 2024-10-29T13:44:32Z

drivers/soundwire/generic_bandwidth_allocation.c

-				       &group.rates[0], group.count);
+				       &group.rates[0], &group.lanes[0], group.count);


nitpick: could we just pass group to this API ?

nitpick: could we just pass group to this API ?

Yes, updated

ranj063

@bardliao could you please also add your sign-off on Pierre's patches?

The existing logic is problematic in that we deprepare all the ports, but still take into account the stream for bit allocation by just walking through the bus->m_rt list. This patch sets the state earlier, so that such DEPREPARED streams can be skipped in the bandwidth allocation (to be implemented in a follow-up patch). Signed-off-by: Pierre-Louis Bossart <[email protected]> Signed-off-by: Bard Liao <[email protected]>

We should not blindly walk through all the m_rt list, since it will have the side effect of accounting for deprepared streams. This behavior is the result of the split implementation where the dailink hw_free() handles the stream state change and the bit allocation, and the dai hw_free() modifies the m_rt list. The bit allocation ends-up using m_rt entries in zombie state, no longer relevant but still used. Signed-off-by: Pierre-Louis Bossart <[email protected]> Signed-off-by: Bard Liao <[email protected]>

Currently, we only set peripheral frequency when the peripheral is initialized. However, curr_dr_freq may change to get required bandwidth. For example, curr_dr_freq may increase from 4.8MHz to 9.6MHz when the 4th stream is opened. Add a helper to get the scale index so that we can get the scale index and program it. Signed-off-by: Bard Liao <[email protected]>

We need to program bus clock scale to adjust the bus clock if current bus clock doesn't fit the bandwidth. Signed-off-by: Bard Liao <[email protected]>

We need to recalculate frame shape when sdw bus clock is changed. And need to make sure all Peripherals connected to the Manager support dynamic clock change. Signed-off-by: Bard Liao <[email protected]>

…w_select_row_col The bits in Column 0 of Rows 0 to 47 are for control word and cannot be used for audio. In practice, entire Column 0 is skipped. Signed-off-by: Bard Liao <[email protected]>

Currently, we check curr_dr_freq roughly by "if (curr_dr_freq <= bus->params.bandwidth)" in sdw_compute_bus_params() and check it accurately in sdw_select_row_col(). It works if we only support one freq. But, we need to check it accurately in sdw_select_row_col() to give it a chance to use a higher freq or use multi-lane. Signed-off-by: Bard Liao <[email protected]>

If a peripheral supports multi-lane, we can use data lane x to extend the bandwidth. The patch suggests to select data lane x where x > 0 when bandwidth is not enough on data lane 0. Signed-off-by: Bard Liao <[email protected]>

All active streams with the same parameters are grouped together and the params are stored in the sdw_group struct. We compute the required bandwidth for each group. However, each lane has individual bandwidth. Therefore, we should separate different lanes in different params groups. Add lane variable to separate params groups. Signed-off-by: Bard Liao <[email protected]>

bardliao · 2024-10-30T05:27:51Z

@bardliao could you please also add your sign-off on Pierre's patches?

Yes, added.

bardliao force-pushed the test-different-sdw-clock branch 2 times, most recently from 071461c to feca0c2 Compare August 29, 2024 05:43

plbossart reviewed Aug 29, 2024

View reviewed changes

drivers/soundwire/stream.c Show resolved Hide resolved

bardliao force-pushed the test-different-sdw-clock branch from feca0c2 to 86a0798 Compare August 29, 2024 09:51

bardliao commented Aug 29, 2024

View reviewed changes

bardliao force-pushed the test-different-sdw-clock branch from 86a0798 to b6e82e4 Compare September 2, 2024 11:59

bardliao changed the title ~~Revert "soundwire: intel_auxdevice: start the bus at default frequency"~~ [RFC] soundwire: add higher bus clock rate support Sep 2, 2024

bardliao force-pushed the test-different-sdw-clock branch from b6e82e4 to 0cd7cbf Compare September 3, 2024 04:15

plbossart reviewed Sep 9, 2024

View reviewed changes

bardliao force-pushed the test-different-sdw-clock branch 2 times, most recently from 28b5981 to cf9ed92 Compare September 10, 2024 06:49

plbossart reviewed Sep 10, 2024

View reviewed changes

bardliao force-pushed the test-different-sdw-clock branch 5 times, most recently from 2297111 to e6977d3 Compare September 13, 2024 06:30

bardliao marked this pull request as ready for review September 13, 2024 06:31

bardliao force-pushed the test-different-sdw-clock branch from e6977d3 to 69dd90f Compare September 13, 2024 06:58

plbossart reviewed Sep 13, 2024

View reviewed changes

bardliao force-pushed the test-different-sdw-clock branch from 69dd90f to ac34bb3 Compare September 13, 2024 14:31

plbossart reviewed Sep 13, 2024

View reviewed changes

bardliao force-pushed the test-different-sdw-clock branch from ac34bb3 to 1f3769c Compare September 16, 2024 06:46

plbossart reviewed Sep 16, 2024

View reviewed changes

drivers/soundwire/generic_bandwidth_allocation.c Show resolved Hide resolved

drivers/soundwire/generic_bandwidth_allocation.c Show resolved Hide resolved

bardliao force-pushed the test-different-sdw-clock branch 2 times, most recently from 58739b7 to ffc7d87 Compare September 16, 2024 09:44

bardliao changed the title ~~[RFC] soundwire: add higher bus clock rate support~~ soundwire: add higher bus clock rate support Sep 16, 2024

Revert "soundwire: generic_bandwidth_allocation: add lane in sdw_grou…

f93cdb5

…p_params" This reverts commit 72e43ef. Signed-off-by: Bard Liao <[email protected]>

ujfalusi reviewed Oct 21, 2024

View reviewed changes

bardliao force-pushed the test-different-sdw-clock branch from 15cc2a7 to ea9b722 Compare October 21, 2024 11:42

ujfalusi reviewed Oct 21, 2024

View reviewed changes

bardliao force-pushed the test-different-sdw-clock branch from ea9b722 to 67a699d Compare October 21, 2024 12:59

ujfalusi reviewed Oct 24, 2024

View reviewed changes

bardliao force-pushed the test-different-sdw-clock branch from 67a699d to c77aed6 Compare October 24, 2024 06:54

ujfalusi previously approved these changes Oct 24, 2024

View reviewed changes

bardliao requested review from lgirdwood and charleskeepax October 29, 2024 01:09

ranj063 reviewed Oct 29, 2024

View reviewed changes

bardliao dismissed ujfalusi’s stale review via ca5ded1 October 29, 2024 05:48

bardliao force-pushed the test-different-sdw-clock branch from c77aed6 to ca5ded1 Compare October 29, 2024 05:48

lgirdwood reviewed Oct 29, 2024

View reviewed changes

ranj063 reviewed Oct 29, 2024

View reviewed changes

plbossart and others added 9 commits October 30, 2024 13:24

Soundwire: stream: program BUSCLOCK_SCALE

80ee8d7

We need to program bus clock scale to adjust the bus clock if current bus clock doesn't fit the bandwidth. Signed-off-by: Bard Liao <[email protected]>

Soundwire: generic_bandwidth_allocation: set frame shape on fly

511e3b7

We need to recalculate frame shape when sdw bus clock is changed. And need to make sure all Peripherals connected to the Manager support dynamic clock change. Signed-off-by: Bard Liao <[email protected]>

soundwire: generic_bandwidth_allocation: correct clk_freq check in sd…

52e973a

…w_select_row_col The bits in Column 0 of Rows 0 to 47 are for control word and cannot be used for audio. In practice, entire Column 0 is skipped. Signed-off-by: Bard Liao <[email protected]>

soundwire: generic_bandwidth_allocation: select data lane

9105df8

If a peripheral supports multi-lane, we can use data lane x to extend the bandwidth. The patch suggests to select data lane x where x > 0 when bandwidth is not enough on data lane 0. Signed-off-by: Bard Liao <[email protected]>

bardliao force-pushed the test-different-sdw-clock branch from ca5ded1 to 5df8537 Compare October 30, 2024 05:24

lgirdwood approved these changes Oct 30, 2024

View reviewed changes

ujfalusi approved these changes Oct 30, 2024

View reviewed changes

ranj063 approved these changes Oct 30, 2024

View reviewed changes

ranj063 merged commit f240eba into thesofproject:topic/sof-dev Oct 30, 2024
12 of 14 checks passed

		&group.rates[0], group.count);
		&group.rates[0], &group.lanes[0], group.count);

soundwire: add higher bus clock rate support #5160

soundwire: add higher bus clock rate support #5160

Conversation

bardliao commented Aug 28, 2024 • edited Loading

plbossart commented Aug 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bardliao commented Sep 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

plbossart left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ujfalusi left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bardliao Oct 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ujfalusi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ujfalusi left a comment

Choose a reason for hiding this comment

bardliao commented Oct 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lgirdwood left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ranj063 left a comment

Choose a reason for hiding this comment

bardliao commented Oct 30, 2024

bardliao commented Aug 28, 2024 •

edited

Loading

ujfalusi left a comment •

edited

Loading

bardliao Oct 21, 2024 •

edited

Loading