ASoC: SOF: Intel: reserve link DMA for sdw bpt stream #5567

bardliao · 2025-10-23T06:25:56Z

Currently, hda_sdw_bpt_dma_prepare() get a HDA stream and use the link DMA but doesn't reserve it. It works fine because
we assume the SwoundWire BPT will not run with audio streams simultaneously. Create and use the new helpers to reserve the link DMA and allow running BTP and audio stream simultaneously.

ujfalusi

@bardliao, I think this is much cleaner than the previous version, few comments to address.

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

ujfalusi · 2025-10-23T07:10:52Z

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

+		goto out_put;
+	}
+
+	hstream->period_bytes = 0;/* initialize period_bytes */


space before the comment

not resolved yet

ujfalusi · 2025-10-23T07:11:34Z

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

+
+	ret = hda_dsp_stream_hw_params(sdev, hext_stream, dmab, NULL);
+	if (ret < 0) {
+		dev_err(sdev->dev, "error: hdac prepare failed: %d\n", ret);


no error: in prints, but func helps.

ujfalusi · 2025-10-23T07:12:59Z

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

+				 struct hdac_ext_stream *hext_stream)
+{
+	struct snd_sof_dev *sdev =  dev_get_drvdata(dev);
+	struct hdac_stream *hstream = &hext_stream->hstream;


we have this helper: hdac_stream(hext_stream), might be better to use it here as well?

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

ujfalusi · 2025-10-23T09:33:01Z

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

+	hstream->bufsize = 0;
+	hstream->format_val = 0;
+out_put:
+	hda_dsp_stream_put(sdev, direction, hstream->stream_tag);


you likely need to do snd_hdac_ext_stream_release() as well?

you likely need to do snd_hdac_ext_stream_release() as well?

Yes, thanks for the reminding.

ujfalusi · 2025-10-23T10:20:55Z

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

 }
 EXPORT_SYMBOL_NS(hda_dsp_get_stream_ldp, "SND_SOC_SOF_INTEL_HDA_COMMON");
+
+struct hdac_ext_stream*


I think a space before the * is due. To align with hda_dsp_stream_get()

I think a space before the * is due. To align with hda_dsp_stream_get()

Updated.

missing space still?

I know what happened. I fixed it previously. Then I removed in the last version and copy the function from hda-loader.c again in this version. Fixed it again now.

ujfalusi · 2025-10-23T13:09:52Z

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

+
+struct hdac_ext_stream *
+hda_dsp_dma_channel_pair_get(struct device *dev, unsigned int format, unsigned int size,
+			     struct snd_dma_buffer *dmab, int direction)


Looking at this again, I think the format/size/dmab should not be required to get the channel pair. Strictly speaking from API point of view.

This is how the BPT is using it, yes, but that does not mean that if we have new user (we might never) it will also need this to be done.

Should we restrict this to get the channel pair and add a wrapper in bpt code to request the pair and do the configuration?

@kv2019i, @ranj063, what do you think?

It sounds better. Let's make the helpers more generic.

ujfalusi · 2025-10-29T13:25:52Z

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

+	struct hdac_ext_stream *hext_stream = _hda_dsp_stream_get(sdev, direction, flags, true);
+
+	if (hext_stream)
+		hext_stream->link_locked = true;


In theory the link could be snatched by other stream between the call _hda_dsp_stream_get() and this point.

In theory the link could be snatched by other stream between the call _hda_dsp_stream_get() and this point.

Fixed, thanks.

ranj063 · 2025-11-03T19:50:43Z

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

 	return sof_ipc_tx_message_no_reply(sdev->ipc, &msg, 0);
 }

+static struct hdac_ext_stream *


Can you fix the typo with BPT in the commit message?

ranj063 · 2025-11-03T19:53:11Z

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

 }

+static struct hdac_ext_stream *
+hda_sdw_prepare(struct device *dev, unsigned int format, unsigned int size,


Is the only difference te use of hda_dsp_stream_pair_get() instead of stream_get() compared to hda_cl_prepare()? Can we reduce code duplication by reusing part of the code?

Is the only difference te use of hda_dsp_stream_pair_get() instead of stream_get() compared to hda_cl_prepare()? Can we reduce code duplication by reusing part of the code?

Yes, updated.

Currently, hda_dsp_stream_get/put are used to get/put the host dma. However, we may want to use a hda stream that both host and link dma are available. Add helper to find the hda stream and reserve/release it. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>

ranj063 · 2025-11-04T19:48:39Z

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

+		goto out_put;
+	}
+
+	hstream->period_bytes = 0;/* initialize period_bytes */


not resolved yet

ranj063 · 2025-11-04T19:48:56Z

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

 }
 EXPORT_SYMBOL_NS(hda_dsp_get_stream_ldp, "SND_SOC_SOF_INTEL_HDA_COMMON");
+
+struct hdac_ext_stream*


missing space still?

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

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

ujfalusi

only few nitpicks..

ujfalusi · 2025-11-05T07:02:58Z

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

+		hext_stream = hda_dsp_stream_get(sdev, direction, 0);
+
+	if (!hext_stream) {
+		dev_err(sdev->dev, "error: no stream available\n");


drop the error: and replace it with func

ujfalusi · 2025-11-05T07:05:15Z

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

+	} else {
+		ret = hda_dsp_stream_hw_params(sdev, hext_stream, dmab, NULL);
+		if (ret < 0) {
+			dev_err(sdev->dev, "error: hdac prepare failed: %d\n", ret);


and same for the other error: in prints.

ujfalusi · 2025-11-06T09:43:44Z

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

+struct hdac_ext_stream *
+hda_dma_prepare(struct device *dev, unsigned int format, unsigned int size,
+		struct snd_dma_buffer *dmab, bool persistent_buffer, int direction,
+		bool is_iccmax, bool pair)


The generic naming of hda_dma_prepare() / hda_dma_cleanup() I think is confusing.
This is implementing a special use of HD-DMA for data transfer, it does buffer allocation also.

It handles the CL (Code Loader - pair == false) and a data transfer channel to external device.

Can we find a better name? (sorry)

Yeah, I thought about it. But I can't find a better name at that moment. Maybe we can figure one out together. How about hda_dt_prepare/cleanup DT: Data Transfer

To stick with the existing (not too intuitive) naming convention used within hda-stream.c, what about:
hda_data_stream_prepare() / hda_data_stream_cleanup()
?

Thanks, will update.

SoundWire BPT stream needs to use link and host DMAs. Thus we need helpers to prepare and cleanup the link and host DMAs. Currently the SoundWire BPT stream uses hda_cl_prepare/cleanup helpers. It works fine because we assume the SwoundWire BPT will not run with audio streams simultaneously. The new helpers are copied from hda_cl_prepare/cleanup and add a flag to reserve the paired host and link DMAs. The new helpers will be used by both code loader and SoundWire BPT. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>

…eams Currently the SoundWire BPT stream uses the paired link DMA but not reserve it. It works without any issue because we assume the SoundWire BPT will not run with audio streams simultaneously. To support simultaneous audio and BPT streams, we need to use the hda_dma_prepare/cleanup helpers to reserve the pair link host DMA. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>

ujfalusi

@bardliao, I think this is a great version!

bardliao · 2025-11-13T03:19:08Z

SOFCI TEST

bardliao · 2025-11-14T07:10:22Z

SOFCI TEST

bardliao requested review from kv2019i, ranj063 and ujfalusi October 23, 2025 06:25

bardliao requested review from dbaluta, lgirdwood, lyakh and plbossart as code owners October 23, 2025 06:25

ujfalusi reviewed Oct 23, 2025

View reviewed changes

bardliao force-pushed the dma_pair branch from 187b00e to 975bcdd Compare October 23, 2025 08:17

ujfalusi reviewed Oct 23, 2025

View reviewed changes

bardliao force-pushed the dma_pair branch from 975bcdd to d89df85 Compare October 23, 2025 10:06

ujfalusi reviewed Oct 23, 2025

View reviewed changes

bardliao force-pushed the dma_pair branch from d89df85 to c9c35cd Compare October 23, 2025 12:54

ujfalusi reviewed Oct 23, 2025

View reviewed changes

bardliao force-pushed the dma_pair branch from c9c35cd to 4f14058 Compare October 27, 2025 09:56

ujfalusi reviewed Oct 29, 2025

View reviewed changes

bardliao force-pushed the dma_pair branch from 4f14058 to 6b54837 Compare October 29, 2025 14:12

ranj063 reviewed Nov 3, 2025

View reviewed changes

bardliao force-pushed the dma_pair branch from 6b54837 to f177999 Compare November 4, 2025 05:52

ranj063 reviewed Nov 4, 2025

View reviewed changes

bardliao force-pushed the dma_pair branch from f177999 to 0082f87 Compare November 5, 2025 02:27

bardliao requested review from ranj063 and ujfalusi November 5, 2025 02:28

ujfalusi reviewed Nov 5, 2025

View reviewed changes

bardliao force-pushed the dma_pair branch from 0082f87 to e47c6a8 Compare November 5, 2025 13:25

ranj063 previously approved these changes Nov 5, 2025

View reviewed changes

ujfalusi reviewed Nov 6, 2025

View reviewed changes

bardliao added 2 commits November 11, 2025 16:22

bardliao dismissed ranj063’s stale review via 5558ee3 November 11, 2025 08:33

bardliao force-pushed the dma_pair branch from e47c6a8 to 5558ee3 Compare November 11, 2025 08:33

ujfalusi approved these changes Nov 12, 2025

View reviewed changes

bardliao requested a review from ranj063 November 12, 2025 07:45

ranj063 approved these changes Nov 12, 2025

View reviewed changes

bardliao merged commit 964c624 into thesofproject:topic/sof-dev Nov 14, 2025
9 of 16 checks passed

ASoC: SOF: Intel: reserve link DMA for sdw bpt stream #5567

ASoC: SOF: Intel: reserve link DMA for sdw bpt stream #5567

Uh oh!

Conversation

bardliao commented Oct 23, 2025

Uh oh!

ujfalusi left a comment

Choose a reason for hiding this comment

Uh oh!

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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ujfalusi 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!

ujfalusi left a comment

Choose a reason for hiding this comment

Uh oh!

bardliao commented Nov 13, 2025

Uh oh!

bardliao commented Nov 14, 2025

Uh oh!

Uh oh!