[PATCH v2 0/3] ASoC: Intel: avs: Set of streaming fixes

[PATCH v2 0/3] ASoC: Intel: avs: Set of streaming fixes

[Cezary Rojewski]

Small set of patches two of which fix problems observed during shutdown
and XRUN scenarios for PCM streaming. These ensure HDAudio HOST stream
is reset and re-setup during XRUNs and synchronize avs_dai_fe_shutdown()
and period-elapsed work (thread) so that no slab-use-after-free panics
occur.

The last change makes the 'format' parameter provided by a
compress-application taken into account - currently its ignored. Such
approach helps us increase the coverage of data probing (debug)
functionality.


Changes in v2:
- fixes sparse warning (incorrerct type in argument) for patch ("ASoC:
  Intel: avs: Use snd_codec format when initializing probe")


Cezary Rojewski (3):
  ASoC: Intel: avs: Unprepare a stream when XRUN occurs
  ASoC: Intel: avs: Disable periods-elapsed work when closing PCM
  ASoC: Intel: avs: Use snd_codec format when initializing probe

 sound/soc/intel/avs/pcm.c    |  3 +++
 sound/soc/intel/avs/probes.c | 18 ++++++++++--------
 2 files changed, 13 insertions(+), 8 deletions(-)

-- 
2.25.1

[PATCH v2 1/3] ASoC: Intel: avs: Unprepare a stream when XRUN occurs

[Cezary Rojewski]

The pcm->prepare() function may be called multiple times in a row by the
userspace, as mentioned in the documentation. The driver shall take that
into account and prevent redundancy. However, the exact same function is
called during XRUNs and in such case, the particular stream shall be
reset and setup anew.

Fixes: 9114700b496c ("ASoC: Intel: avs: Generic PCM FE operations")
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/intel/avs/pcm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/sound/soc/intel/avs/pcm.c b/sound/soc/intel/avs/pcm.c
index d31058e2de5b..501466bd1f7f 100644
--- a/sound/soc/intel/avs/pcm.c
+++ b/sound/soc/intel/avs/pcm.c
@@ -754,6 +754,8 @@ static int avs_dai_fe_prepare(struct snd_pcm_substream *substream, struct snd_so
 	data = snd_soc_dai_get_dma_data(dai, substream);
 	host_stream = data->host_stream;
 
+	if (runtime->state == SNDRV_PCM_STATE_XRUN)
+		hdac_stream(host_stream)->prepared = false;
 	if (hdac_stream(host_stream)->prepared)
 		return 0;
 
-- 
2.25.1

[PATCH v2 2/3] ASoC: Intel: avs: Disable periods-elapsed work when closing PCM

[Cezary Rojewski]

avs_dai_fe_shutdown() handles the shutdown procedure for HOST HDAudio
stream while period-elapsed work services its IRQs. As the former
frees the DAI's private context, these two operations shall be
synchronized to avoid slab-use-after-free or worse errors.

Fixes: 0dbb186c3510 ("ASoC: Intel: avs: Update stream status in a separate thread")
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/intel/avs/pcm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/soc/intel/avs/pcm.c b/sound/soc/intel/avs/pcm.c
index 501466bd1f7f..80c001120cdd 100644
--- a/sound/soc/intel/avs/pcm.c
+++ b/sound/soc/intel/avs/pcm.c
@@ -651,6 +651,7 @@ static void avs_dai_fe_shutdown(struct snd_pcm_substream *substream, struct snd_
 
 	data = snd_soc_dai_get_dma_data(dai, substream);
 
+	disable_work_sync(&data->period_elapsed_work);
 	snd_hdac_ext_stream_release(data->host_stream, HDAC_EXT_STREAM_TYPE_HOST);
 	avs_dai_shutdown(substream, dai);
 }
-- 
2.25.1

[PATCH v2 3/3] ASoC: Intel: avs: Use snd_codec format when initializing probe

[Cezary Rojewski]

The data probing is a debug feature. Currently parameters channels and
rate specified by the application are read while the format is ignored.
More robust approach is to read all of them.

Audio format, while not used by the Probe module for PCM streaming,
takes part in the gateway initialization on the DSP side. With full
parametrization we gain better coverage with the data probing feature.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/intel/avs/probes.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/sound/soc/intel/avs/probes.c b/sound/soc/intel/avs/probes.c
index c7b70006354a..7bb5dec257b4 100644
--- a/sound/soc/intel/avs/probes.c
+++ b/sound/soc/intel/avs/probes.c
@@ -14,8 +14,8 @@
 #include "debug.h"
 #include "messages.h"
 
-static int avs_dsp_init_probe(struct avs_dev *adev, union avs_connector_node_id node_id,
-			      size_t buffer_size)
+static int avs_dsp_init_probe(struct avs_dev *adev, struct snd_compr_params *params, int bps,
+			      union avs_connector_node_id node_id, size_t buffer_size)
 {
 	struct avs_probe_cfg cfg = {{0}};
 	struct avs_module_entry mentry;
@@ -27,12 +27,16 @@ static int avs_dsp_init_probe(struct avs_dev *adev, union avs_connector_node_id
 		return ret;
 
 	/*
-	 * Probe module uses no cycles, audio data format and input and output
-	 * frame sizes are unused. It is also not owned by any pipeline.
+	 * Probe module uses no cycles, input and output frame sizes are unused.
+	 * It is also not owned by any pipeline.
 	 */
 	cfg.base.ibs = 1;
 	/* BSS module descriptor is always segment of index=2. */
 	cfg.base.is_pages = mentry.segments[2].flags.length;
+	cfg.base.audio_fmt.sampling_freq = params->codec.sample_rate;
+	cfg.base.audio_fmt.bit_depth = bps;
+	cfg.base.audio_fmt.num_channels = params->codec.ch_out;
+	cfg.base.audio_fmt.valid_bit_depth = bps;
 	cfg.gtw_cfg.node_id = node_id;
 	cfg.gtw_cfg.dma_buffer_size = buffer_size;
 
@@ -128,8 +132,6 @@ static int avs_probe_compr_set_params(struct snd_compr_stream *cstream,
 	struct hdac_ext_stream *host_stream = avs_compr_get_host_stream(cstream);
 	struct snd_compr_runtime *rtd = cstream->runtime;
 	struct avs_dev *adev = to_avs_dev(dai->dev);
-	/* compr params do not store bit depth, default to S32_LE. */
-	snd_pcm_format_t format = SNDRV_PCM_FORMAT_S32_LE;
 	unsigned int format_val;
 	int bps, ret;
 
@@ -142,7 +144,7 @@ static int avs_probe_compr_set_params(struct snd_compr_stream *cstream,
 	ret = snd_compr_malloc_pages(cstream, rtd->buffer_size);
 	if (ret < 0)
 		return ret;
-	bps = snd_pcm_format_physical_width(format);
+	bps = snd_pcm_format_physical_width((__force snd_pcm_format_t)params->codec.format);
 	if (bps < 0)
 		return bps;
 	format_val = snd_hdac_stream_format(params->codec.ch_out, bps, params->codec.sample_rate);
@@ -166,7 +168,7 @@ static int avs_probe_compr_set_params(struct snd_compr_stream *cstream,
 		node_id.vindex = hdac_stream(host_stream)->stream_tag - 1;
 		node_id.dma_type = AVS_DMA_HDA_HOST_INPUT;
 
-		ret = avs_dsp_init_probe(adev, node_id, rtd->dma_bytes);
+		ret = avs_dsp_init_probe(adev, params, bps, node_id, rtd->dma_bytes);
 		if (ret < 0) {
 			dev_err(dai->dev, "probe init failed: %d\n", ret);
 			avs_dsp_enable_d0ix(adev);
-- 
2.25.1

Re: [PATCH v2 0/3] ASoC: Intel: avs: Set of streaming fixes

[Mark Brown]

On Mon, 27 Oct 2025 12:01:03 +0100, Cezary Rojewski wrote:
> Small set of patches two of which fix problems observed during shutdown
> and XRUN scenarios for PCM streaming. These ensure HDAudio HOST stream
> is reset and re-setup during XRUNs and synchronize avs_dai_fe_shutdown()
> and period-elapsed work (thread) so that no slab-use-after-free panics
> occur.
> 
> The last change makes the 'format' parameter provided by a
> compress-application taken into account - currently its ignored. Such
> approach helps us increase the coverage of data probing (debug)
> functionality.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/3] ASoC: Intel: avs: Unprepare a stream when XRUN occurs
      commit: cfca1637bc2b6b1e4f191d2f0b25f12402fbbb26
[2/3] ASoC: Intel: avs: Disable periods-elapsed work when closing PCM
      commit: 845f716dc5f354c719f6fda35048b6c2eca99331
[3/3] ASoC: Intel: avs: Use snd_codec format when initializing probe
      commit: 64007ad3e2a0e0a0ded8b2c6a72c0bb7883d3a33

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

Re: [PATCH v2 0/3] ASoC: Intel: avs: Set of streaming fixes

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 495 bytes --]

On Mon, Oct 27, 2025 at 12:01:03PM +0100, Cezary Rojewski wrote:
> Small set of patches two of which fix problems observed during shutdown
> and XRUN scenarios for PCM streaming. These ensure HDAudio HOST stream
> is reset and re-setup during XRUNs and synchronize avs_dai_fe_shutdown()
> and period-elapsed work (thread) so that no slab-use-after-free panics
> occur.

Sorry, this had already made it to the end of my CI pipeline before you
resent - could you send an incremental patch please?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 0/3] ASoC: Intel: avs: Set of streaming fixes

[Cezary Rojewski]

On 2025-10-27 1:44 PM, Mark Brown wrote:
> On Mon, Oct 27, 2025 at 12:01:03PM +0100, Cezary Rojewski wrote:
>> Small set of patches two of which fix problems observed during shutdown
>> and XRUN scenarios for PCM streaming. These ensure HDAudio HOST stream
>> is reset and re-setup during XRUNs and synchronize avs_dai_fe_shutdown()
>> and period-elapsed work (thread) so that no slab-use-after-free panics
>> occur.
> 
> Sorry, this had already made it to the end of my CI pipeline before you
> resent - could you send an incremental patch please?


To be honest, I don't mind either of versions being merged. The 
incremental 'fix' is here to silence a warning reported by sparse. 
However, the codec->format is used by apps with snd_pcm_format_t anyway 
despite documentation (more like a comment really) suggesting otherwise.

Let's leave it as it's right now, in sound/for-next of yours.

Kind regards,
Czarek