[PATCH 0/2] ASoC: SOF: Intel: reset spib_addr in stream cleanup

[PATCH 0/2] ASoC: SOF: Intel: reset spib_addr in stream cleanup

[Bard Liao]

The spib_addr register indicates the current position in the buffer
being processed by the host software to the host DMA. It must be reset
before being disabled. Also, add the missing disable call in
hda_data_stream_cleanup().

Bard Liao (2):
  ASoC: SOF: Intel: reset spib_addr before disabling SPIB
  ASoC: SOF: Intel: Disable SPIB in both direction

 sound/soc/sof/intel/hda-stream.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

-- 
2.43.0

[PATCH 1/2] ASoC: SOF: Intel: reset spib_addr before disabling SPIB

[Bard Liao]

The spib_addr register will indicate to the host DMA where the position
is in the buffer currently processed by host SW. The register is ignored
by the host DMA if SPIB is disabled. Reset it to 0 before disabling SPIB.

Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
---
 sound/soc/sof/intel/hda-stream.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/sound/soc/sof/intel/hda-stream.c b/sound/soc/sof/intel/hda-stream.c
index 5c1f3b427cdb..4a46e35c3eac 100644
--- a/sound/soc/sof/intel/hda-stream.c
+++ b/sound/soc/sof/intel/hda-stream.c
@@ -198,13 +198,18 @@ int hda_dsp_stream_spib_config(struct snd_sof_dev *sdev,
 
 	mask = (1 << hstream->index);
 
+	/* Reset the spib_addr before disabling SPIB */
+	if (!enable)
+		sof_io_write(sdev, hstream->spib_addr, 0);
+
 	/* enable/disable SPIB for the stream */
 	snd_sof_dsp_update_bits(sdev, HDA_DSP_SPIB_BAR,
 				SOF_HDA_ADSP_REG_CL_SPBFIFO_SPBFCCTL, mask,
 				enable << hstream->index);
 
 	/* set the SPIB value */
-	sof_io_write(sdev, hstream->spib_addr, size);
+	if (enable)
+		sof_io_write(sdev, hstream->spib_addr, size);
 
 	return 0;
 }
-- 
2.43.0

[PATCH 2/2] ASoC: SOF: Intel: Disable SPIB in both direction

[Bard Liao]

SPIB was enabled in hda_data_stream_prepare() if is_iccmax = false.
We should disable it in hda_data_stream_cleanup().

Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
---
 sound/soc/sof/intel/hda-stream.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/sound/soc/sof/intel/hda-stream.c b/sound/soc/sof/intel/hda-stream.c
index 4a46e35c3eac..6c4447f52c68 100644
--- a/sound/soc/sof/intel/hda-stream.c
+++ b/sound/soc/sof/intel/hda-stream.c
@@ -1333,11 +1333,11 @@ int hda_data_stream_cleanup(struct device *dev, struct snd_dma_buffer *dmab,
 	struct snd_sof_dev *sdev =  dev_get_drvdata(dev);
 	struct hdac_stream *hstream = hdac_stream(hext_stream);
 	int sd_offset = SOF_STREAM_SD_OFFSET(hstream);
-	int ret = 0;
+	int ret;
 
-	if (hstream->direction == SNDRV_PCM_STREAM_PLAYBACK)
-		ret = hda_dsp_stream_spib_config(sdev, hext_stream, HDA_DSP_SPIB_DISABLE, 0);
-	else
+	ret = hda_dsp_stream_spib_config(sdev, hext_stream, HDA_DSP_SPIB_DISABLE, 0);
+
+	if (hstream->direction == SNDRV_PCM_STREAM_CAPTURE)
 		snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, sd_offset,
 					SOF_HDA_SD_CTL_DMA_START, 0);
 
-- 
2.43.0

Re: [PATCH 2/2] ASoC: SOF: Intel: Disable SPIB in both direction

[Mark Brown]

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

On Tue, Jul 07, 2026 at 09:19:45PM +0800, Bard Liao wrote:
> SPIB was enabled in hda_data_stream_prepare() if is_iccmax = false.
> We should disable it in hda_data_stream_cleanup().

> -	if (hstream->direction == SNDRV_PCM_STREAM_PLAYBACK)
> -		ret = hda_dsp_stream_spib_config(sdev, hext_stream, HDA_DSP_SPIB_DISABLE, 0);
> -	else
> +	ret = hda_dsp_stream_spib_config(sdev, hext_stream, HDA_DSP_SPIB_DISABLE, 0);
> +
> +	if (hstream->direction == SNDRV_PCM_STREAM_CAPTURE)

This doesn't check if is_icccmax is true but upgrades any error from the
teardown to be propagated to the caller - should we either pay less
attention to the error or have a check to see if the cleanup is needed?

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

RE: [PATCH 2/2] ASoC: SOF: Intel: Disable SPIB in both direction

[Liao, Bard]

> -----Original Message-----
> From: Mark Brown <broonie@kernel.org>
> Sent: Wednesday, July 8, 2026 7:32 AM
> To: Bard Liao <yung-chuan.liao@linux.intel.com>
> Cc: tiwai@suse.de; linux-sound@vger.kernel.org; Liao, Bard
> <bard.liao@intel.com>; peter.ujfalusi@linux.intel.com
> Subject: Re: [PATCH 2/2] ASoC: SOF: Intel: Disable SPIB in both direction
> 
> On Tue, Jul 07, 2026 at 09:19:45PM +0800, Bard Liao wrote:
> > SPIB was enabled in hda_data_stream_prepare() if is_iccmax = false.
> > We should disable it in hda_data_stream_cleanup().
> 
> > -	if (hstream->direction == SNDRV_PCM_STREAM_PLAYBACK)
> > -		ret = hda_dsp_stream_spib_config(sdev, hext_stream,
> HDA_DSP_SPIB_DISABLE, 0);
> > -	else
> > +	ret = hda_dsp_stream_spib_config(sdev, hext_stream,
> HDA_DSP_SPIB_DISABLE, 0);
> > +
> > +	if (hstream->direction == SNDRV_PCM_STREAM_CAPTURE)
> 
> This doesn't check if is_icccmax is true but upgrades any error from the
> teardown to be propagated to the caller - should we either pay less
> attention to the error or have a check to see if the cleanup is needed?

Thanks Mark for the review. I will fix it in v2.