ASoC: SOF: hda-sdw-bpt: mark bpt_stream->link_locked #5500

bardliao · 2025-07-31T07:30:47Z

Currently, the BPT stream will not run with other streams. So there is no issue we don't mark bpt_stream->link_locked. But we will support the BRA and audio streams run simultaneously. Therefore, marking link_locked is required to prevent the link being used by different streams.

plbossart · 2025-08-28T11:51:54Z

@bardliao the design assumptions was that all the frame was allocated to BPT or audio, I don't think it's possible to mix the two, nor why we would need this capability.

bardliao · 2025-08-28T14:05:25Z

@plbossart The request is from Cirrus. One scenario is that one codec and one amp are on the same SDW link. We may want to write the AMP firmware while the codec is playing. @charleskeepax @stuhenderson Do you have any comment?

kv2019i

Hmm, even if we end up not supporting concurrent use, this would seem a safe thing to align bpt assignement with the flow in hda_link_stream_assign() and mark the link as locked as done here.

ranj063 · 2025-09-12T15:49:14Z

sound/soc/sof/intel/hda-sdw-bpt.c

 		return PTR_ERR(bpt_stream);
 	}
+	/* Mark the HDA stream link is used */
+	bpt_stream->link_locked = 1;


@bardliao IMO this may not be enough to meet the requirement for concurrent BPT and regular streams. I think we;d be oversimplifying it by assuming that the link DMA is always available when picking the host DMA channel isnt it? What if the link is already taken?

I don't think the BPT stream will use a stream that bpt_stream->link_locked = 1. Both BTP and regular stream use hda_dsp_stream_get/put with the opened flag to manage the hdac_stream. So that BTP stream should not get a hdac_stream that is opened. But hda_link_stream_assign checked hext_stream->link_locked and that is why we need to set link_locked as regular streams.

@bardliao I think you should squash the 2 patches and do the link_locked set/clear in the stream_get/put functions

@bardliao I think you should squash the 2 patches and do the link_locked set/clear in the stream_get/put functions

Updated.

sound/soc/sof/intel/hda-stream.c

kv2019i

I'm afraid V2 raised some new questions, please see inline.

kv2019i · 2025-10-02T07:10:26Z

sound/soc/sof/intel/hda-stream.c


+	if (use_link_dma)
+		snd_hdac_ext_stream_release(link_stream, HDAC_EXT_STREAM_TYPE_LINK);
+


And do we really need to remember to pass "use_link_dma" to put? Shouldn't this be recorded in get and we allocated a linked link dma, we release it in put?

IOW, @ujfalusi is adding automatic re-coupling in #5525

Not sure how we book-keep the case where we'd have multiple link DMAs per one host DMA, but shouldn't we have some state to track which case this is so the put() can do the right thing?

And do we really need to remember to pass "use_link_dma" to put? Shouldn't this be recorded in get and we allocated a linked link dma, we release it in put?

I thought about that, but I can't find a place to record the link_locked is set by hda_dsp_stream_get(). That's why originally, I set link_locked and call snd_hdac_ext_stream_release in hda_sdw_bpt_dma_prepare and hda_sdw_bpt_dma_deprepare where the stream is used/released.

Ack @bardliao . If you took look at the options, then I'm ok. @ujfalusi can you also take a look that this does not conflict with your thinking in #5525 ?

kv2019i · 2025-10-02T07:11:37Z

sound/soc/sof/intel/hda-loader.c

 	int ret;

-	hext_stream = hda_dsp_stream_get(sdev, direction, 0);
+	hext_stream = hda_dsp_stream_get(sdev, direction, 0, true);


I'm not a fan of these true/false parameters. Can we add a enum or define for this?

hext_stream = hda_dsp_stream_get(sdev, direction, 0, HDA_STREAM_USE_LINK_DMA);

is much more readable. I know we have many true/false interfaces in existing code, so if you @bardliao @ranj063 @ujfalusi are ok, I'm not going to block, but I do find these hard to read (in the calling code).

I'm not a fan of these true/false parameters. Can we add a enum or define for this?

hext_stream = hda_dsp_stream_get(sdev, direction, 0, HDA_STREAM_USE_LINK_DMA);

is much more readable. I know we have many true/false interfaces in existing code, so if you @bardliao @ranj063 @ujfalusi are ok, I'm not going to block, but I do find these hard to read (in the calling code).

Updated

Currently, we get host and link DMA streams separately by hda_dsp_stream_get() and hda_link_stream_assign(). However, in some cases like SoundWire BPT, we use hda_dsp_stream_get() to get the HDA stream and use the same HDA stream for both host and link DMAs. Add a use_link_dma flag to handle the case. Signed-off-by: Bard Liao <[email protected]>

ujfalusi · 2025-10-20T08:52:43Z

sound/soc/sof/intel/hda-loader.c

 	int ret;

-	hext_stream = hda_dsp_stream_get(sdev, direction, 0);
+	hext_stream = hda_dsp_stream_get(sdev, direction, 0, HDA_STREAM_USE_HOST_LINK_DMA);


Why would you need LinkDMA for Code Loader? It only needs hostDMA.

hda_cl_prepare() is called by hda_sdw_bpt_dma_prepare(). Maybe we should reconsider my first proposal which set link_locked in hda_sdw_bpt_dma_prepare and clear it in hda_sdw_bpt_dma_deprepare c79e2c5
What do you think? @ranj063

ujfalusi · 2025-10-20T09:04:09Z

sound/soc/sof/intel/hda.h

+enum sof_hda_stream_type {
+	HDA_STREAM_USE_HOST_DMA,
+	HDA_STREAM_USE_HOST_LINK_DMA,
+};


I think this is a bool reserve_dma_pair sort of thing at the end, no?

What would happen if BRA would use hda_link_stream_assign() to grab the Link channel or is it mandatory that we must use the host/link with the same index?

Say, you use a wrapper in BR to get the two channels, host via hda_dsp_stream_get() and link via hda_link_stream_assign()

Or for BRA we need to allocate the pairs like for platforms that have the PROCEN_FMT_QUIRK set?

Not mandatory, but we set the same ID for host and link.

msg.primary |= SOF_IPC4_GLB_CHAIN_DMA_HOST_ID(dma_id); msg.primary |= SOF_IPC4_GLB_CHAIN_DMA_LINK_ID(dma_id);

https://github.com/thesofproject/linux/blob/topic/sof-dev/sound/soc/sof/intel/hda-sdw-bpt.c#L73

I think this is a bool reserve_dma_pair sort of thing at the end, no?

To me, yes. Using enum is an ask from @kv2019i to make the code more readable.

What would happen if BRA would use hda_link_stream_assign() to grab the Link channel or is it mandatory that we must use the host/link with the same index?

Say, you use a wrapper in BR to get the two channels, host via hda_dsp_stream_get() and link via hda_link_stream_assign()

I thought about this. However, it will be more complicated. Besides, we can always use the same index for both host and link. Just have to use unused host/link.

bardliao requested a review from kv2019i July 31, 2025 07:30

kv2019i previously approved these changes Sep 8, 2025

View reviewed changes

bardliao marked this pull request as ready for review September 8, 2025 12:41

bardliao requested review from dbaluta, lgirdwood, plbossart, ranj063 and ujfalusi as code owners September 8, 2025 12:41

ranj063 reviewed Sep 12, 2025

View reviewed changes

bardliao dismissed kv2019i’s stale review via c79e2c5 September 25, 2025 14:13

bardliao force-pushed the bpt_stream branch from 5e10347 to c79e2c5 Compare September 25, 2025 14:13

bardliao requested a review from lyakh as a code owner September 25, 2025 14:13

bardliao force-pushed the bpt_stream branch from c79e2c5 to 943ac4c Compare September 26, 2025 11:42

ranj063 reviewed Sep 26, 2025

View reviewed changes

sound/soc/sof/intel/hda-stream.c Show resolved Hide resolved

ranj063 previously approved these changes Sep 30, 2025

View reviewed changes

bardliao requested a review from kv2019i October 1, 2025 01:43

kv2019i reviewed Oct 2, 2025

View reviewed changes

bardliao dismissed ranj063’s stale review via b31d3fa October 13, 2025 03:17

bardliao force-pushed the bpt_stream branch from 943ac4c to b31d3fa Compare October 13, 2025 03:17

bardliao requested review from kv2019i and ranj063 October 17, 2025 04:36

ujfalusi reviewed Oct 20, 2025

View reviewed changes

bardliao closed this Oct 23, 2025


		if (use_link_dma)
		snd_hdac_ext_stream_release(link_stream, HDAC_EXT_STREAM_TYPE_LINK);

ASoC: SOF: hda-sdw-bpt: mark bpt_stream->link_locked #5500

ASoC: SOF: hda-sdw-bpt: mark bpt_stream->link_locked #5500

Uh oh!

Conversation

bardliao commented Jul 31, 2025

Uh oh!

plbossart commented Aug 28, 2025

Uh oh!

bardliao commented Aug 28, 2025

Uh oh!

kv2019i left a comment

Choose a reason for hiding this comment

Uh oh!

ranj063 Sep 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kv2019i left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bardliao Oct 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ranj063 Sep 12, 2025 •

edited

Loading

bardliao Oct 20, 2025 •

edited

Loading