Re: ASoC: amd: acp-pdm: full-scale burst on every DMIC capture start
From: Mukunda,Vijendar
Date: Fri Aug 14 2026 - 04:39:56 EST
On 8/13/26 17:56, Robin Everaars wrote:
One clarification about the isolated -EIO mentioned above: I removed patch 2,Thank you for the detailed testing and for helping isolate both issues.
rebuilt and rebooted into the earlier patch-1-only module. Its SHA-256 was:
cd55e119961b80066c66ae7abb5f05ca97fa2747de7887a17ee45c724f19fc9d
The first runtime-suspended capture after that reboot again returned -EIO
before delivering data, with no matching kernel diagnostic. The next three
runtime-suspended captures each delivered all 48000 frames with no clipped or
=99% full-scale samples.I therefore do not attribute the isolated -EIO to patch 2. The reproducible
patch-2 regression remains the scheduling-while-atomic failure from msleep()
in acp63_pdm_dai_trigger() during active-capture resume.
The patch-1-only module result is conclusive: the first cold-start -EIO
occurs with patch 1 alone, is not introduced by patch 2, and
self-recovers on the next attempt without any corresponding kernel
diagnostic. Since the failure does not reproduce across subsequent
runtime-suspended starts, it appears to be a pre-existing intermittent
condition, most likely related to hardware timing during the first DMA
transaction after a cold ACP power-on. We will continue tracking it
separately, but it does not affect the correctness of either patch in
this series.
Regarding the scheduling-while-atomic warning observed with patch 2,
your analysis is correct. The original assumption that TRIGGER_RESUME
runs in a sleepable PM resume context was incorrect. During resume,
snd_pcm_resume() dispatches TRIGGER_RESUME through
snd_pcm_action_lock_irq(), which holds the PCM stream spinlock with IRQs
disabled before invoking the DAI trigger callback. As a result, calling
msleep() from acp63_pdm_dai_trigger() can trigger a
scheduling-while-atomic BUG.
To address this, the revised series removes acp63_pdm_settle() from the
trigger path and moves it into acp63_pdm_resume(), the system-sleep PM
resume hook (SYSTEM_SLEEP_PM_OPS). This resume callback executes in a
sleepable process context and completes before the PCM layer issues
TRIGGER_RESUME, ensuring the CIC filter has fully settled before DMA is
armed. The trigger callback is therefore reduced to programming the DMA
path without any sleep operations.
In addition, the PDM DAI link is explicitly marked nonatomic = 1 in the
machine driver. While the existing prepare() path was not responsible
for the BUG because snd_pcm_prepare() already uses the nonatomic action
path, the DAI operations legitimately require sleeping and the nonatomic
declaration aligns the machine-driver configuration with those
requirements, helping to avoid future regressions.
With these updates, the scheduling-while-atomic warning remains the only
reproducible regression attributable to patch 2 and is addressed by the
revised implementation.
Could you please re-test with the updated patches, particularly the
active-capture s2idle resume scenario that originally triggered the BUG?
The expected result is a clean resume with no scheduling-while-atomic
warning and successful capture continuation after resume without
requiring a fresh stream restart.
Thanks,From 56300d48776be7f8424ca96b5ca46b74383ba9d1 Mon Sep 17 00:00:00 2001
Robin
From: Vijendar Mukunda <Vijendar.Mukunda@xxxxxxx>
Date: Fri, 14 Aug 2026 12:42:27 +0530
Subject: [PATCH V2 0/3] ASoC: amd: ps: pdm: fix CIC decimator transients and resume BUG
The ACP PDM DMIC on Pink Sardine (ACP6.3/7.0/7.1) emits a full-scale
Nyquist-rate burst at the start of every capture stream and after a
system suspend/resume cycle. This series fixes both cases and also
corrects a scheduling-while-atomic BUG introduced in v1 of the resume
fix.
Patch 1 adds a prepare() DAI callback that enables the PDM decimator
and waits 300 ms for the CIC filter chain to stabilise before the DMA
is armed. A new acp63_pause_pdm_dma() helper keeps the decimator
running across PAUSE_PUSH/PAUSE_RELEASE so no settling delay is needed
on pause resume.
Patch 2 extends the fix to the system resume path. After
TRIGGER_SUSPEND the decimator is disabled; because prepare() is not
called before TRIGGER_RESUME, the settle sequence must run elsewhere.
It is placed in acp63_pdm_resume() (SYSTEM_SLEEP_PM_OPS resume hook),
which runs in sleepable process context before the PCM layer issues
TRIGGER_RESUME. A shared acp63_pdm_settle() helper is factored out so
prepare() and acp63_pdm_resume() use identical logic.
Patch 3 marks the PDM DAI link nonatomic = 1 in the machine driver to
match the sleep requirements of the ops registered on the link.
Changes in v2:
- Patch 1: no code changes.
- Patch 2: fix scheduling-while-atomic BUG on s2idle resume reported
by the reviewer. v1 called acp63_pdm_settle() (msleep(300)) inside
TRIGGER_RESUME, which runs under the PCM stream spinlock with IRQs
disabled via snd_pcm_action_lock_irq(). This triggered:
BUG: scheduling while atomic: .../0x00000002
msleep
acp63_pdm_dai_trigger+0x10d/0x2fc [snd_ps_pdm_dma]
soc_dai_trigger.part.0 [snd_soc_core]
Fix: move acp63_pdm_settle() from the trigger callback to
acp63_pdm_resume(), which is sleepable. Introduce the
acp63_pdm_settle() helper so prepare() and acp63_pdm_resume() share
the sequence without duplication.
- Patch 3: new in v2. Marks the DAI link nonatomic = 1,
to match the sleep requirements of the registered ops.
Vijendar Mukunda (3):
ASoC: amd: ps: pdm: fix decimator transient on stream start
ASoC: amd: ps: pdm: fix decimator transient on system resume
ASoC: amd: ps: pdm: set PDM DAI link as non-atomic
sound/soc/amd/ps/ps-mach.c | 1 +
sound/soc/amd/ps/ps-pdm-dma.c | 136 ++++++++++++++++++++++++++++++++--
2 files changed, 131 insertions(+), 6 deletions(-)
--
2.48.1
From e0bc5065c65a6b7947a55ae4cd8d67826009f777 Mon Sep 17 00:00:00 2001
From: Vijendar Mukunda <Vijendar.Mukunda@xxxxxxx>
Date: Wed, 12 Aug 2026 14:41:42 +0530
Subject: [PATCH V2 1/3] ASoC: amd: ps: pdm: fix decimator transient on stream
start
The ACP PDM DMIC emits a full-scale Nyquist-rate burst at the beginning
of every capture stream. The burst lasts ~4 ms on a warm start (ACP
already running) and ~250 ms on a cold start (ACP runtime-suspended),
and is audible to remote call participants as a click on join.
Root cause: acp63_start_pdm_dma() enables the PDM decimator and the DMA
in the same call, so the CIC filter chain's unflushed initial state goes
directly to userspace. An exponentially growing alternating pattern
collapses into full-scale saturation on both channels in antiphase --
the classic signature of a CIC integrator/comb chain running from a
non-zero initial condition.
Fix: add a prepare() DAI callback that enables PDM_ENABLE and waits
300 ms before the DMA starts. 300 ms is chosen to cover the cold-start
case; the warm-start transient clears in under 5 ms. After the
settling delay, flush ACP_WOV_PDM_FIFO_FLUSH so the DMA sees a clean
buffer. acp63_start_pdm_dma() then only arms the DMA, skipping the
PDM_ENABLE write when prepare() has already set it.
For the RESUME path where prepare() is not called, PDM_ENABLE is set
conditionally in acp63_start_pdm_dma() if not already active.
PAUSE handling: PAUSE_PUSH previously disabled both the DMA and the PDM
decimator via acp63_stop_pdm_dma(). After moving PDM_ENABLE to
prepare(), ALSA does not call prepare() between PAUSE_PUSH and
PAUSE_RELEASE, so PAUSE_RELEASE would have re-enabled PDM without the
300 ms settling delay, reproducing the transient on every resume from
pause.
Fix the pause path by adding acp63_pause_pdm_dma(), which stops the DMA
only and leaves the PDM decimator running. PAUSE_RELEASE flushes the
FIFO to discard samples accumulated while the DMA was stopped, then
restarts the DMA. Since the CIC filter never stopped, no settling delay
is needed. STOP and SUSPEND continue to use acp63_stop_pdm_dma(), which
disables both DMA and PDM.
Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@xxxxxxx>
---
sound/soc/amd/ps/ps-pdm-dma.c | 103 ++++++++++++++++++++++++++++++++--
1 file changed, 97 insertions(+), 6 deletions(-)
diff --git a/sound/soc/amd/ps/ps-pdm-dma.c b/sound/soc/amd/ps/ps-pdm-dma.c
index 04c014349347..e8f84d83a886 100644
--- a/sound/soc/amd/ps/ps-pdm-dma.c
+++ b/sound/soc/amd/ps/ps-pdm-dma.c
@@ -5,6 +5,7 @@
* Copyright 2022, 2025 Advanced Micro Devices, Inc.
*/
+#include <linux/delay.h>
#include <linux/platform_device.h>
#include <linux/module.h>
#include <linux/bitfield.h>
@@ -19,6 +20,17 @@
#define DRV_NAME "acp_ps_pdm_dma"
+/*
+ * Time in milliseconds to wait after enabling the PDM clock before
+ * starting the DMA. The PDM microphone and the CIC/decimation filter
+ * chain need this time to reach a stable state; without it the first
+ * frames delivered to userspace contain a saturated Nyquist-rate
+ * transient (unflushed decimator initial state). 300 ms covers both
+ * the warm-start case (~4 ms) and the cold-start case (~250 ms, when
+ * the ACP has been runtime-suspended).
+ */
+#define PDM_SETTLING_DELAY_MS 300
+
static int pdm_gain = 3;
module_param(pdm_gain, int, 0644);
MODULE_PARM_DESC(pdm_gain, "Gain control (0-3)");
@@ -107,12 +119,19 @@ static int acp63_start_pdm_dma(void __iomem *acp_base)
u32 pdm_dma_enable;
int timeout;
- pdm_enable = 0x01;
- pdm_dma_enable = 0x01;
-
acp63_enable_pdm_clock(acp_base);
- writel(pdm_enable, acp_base + ACP_WOV_PDM_ENABLE);
- writel(pdm_dma_enable, acp_base + ACP_WOV_PDM_DMA_ENABLE);
+
+ /*
+ * PDM_ENABLE and the clock were already set in prepare() for the
+ * initial stream start to allow the CIC filter to settle. Only
+ * write PDM_ENABLE if it is not already set, to cover the
+ * RESUME and PAUSE_RELEASE paths where prepare() was not called.
+ */
+ pdm_enable = readl(acp_base + ACP_WOV_PDM_ENABLE);
+ if (!(pdm_enable & ACP_PDM_ENABLE))
+ writel(ACP_PDM_ENABLE, acp_base + ACP_WOV_PDM_ENABLE);
+
+ writel(0x01, acp_base + ACP_WOV_PDM_DMA_ENABLE);
timeout = 0;
while (++timeout < ACP_COUNTER) {
pdm_dma_enable = readl(acp_base + ACP_WOV_PDM_DMA_ENABLE);
@@ -154,6 +173,32 @@ static int acp63_stop_pdm_dma(void __iomem *acp_base)
return 0;
}
+static int acp63_pause_pdm_dma(void __iomem *acp_base)
+{
+ u32 pdm_dma_enable;
+ int timeout;
+
+ /*
+ * Stop only the DMA; leave the PDM decimator running so that
+ * PAUSE_RELEASE does not need to wait for the CIC filter to
+ * settle again. The caller is responsible for flushing the FIFO
+ * before restarting the DMA on PAUSE_RELEASE.
+ */
+ pdm_dma_enable = readl(acp_base + ACP_WOV_PDM_DMA_ENABLE);
+ if (!(pdm_dma_enable & 0x01))
+ return 0;
+
+ writel(0x02, acp_base + ACP_WOV_PDM_DMA_ENABLE);
+ timeout = 0;
+ while (++timeout < ACP_COUNTER) {
+ pdm_dma_enable = readl(acp_base + ACP_WOV_PDM_DMA_ENABLE);
+ if ((pdm_dma_enable & 0x02) == 0x00)
+ return 0;
+ udelay(DELAY_US);
+ }
+ return -ETIMEDOUT;
+}
+
static void acp63_config_dma(struct pdm_stream_instance *rtd, int direction)
{
u16 page_idx;
@@ -286,6 +331,29 @@ static int acp63_pdm_dma_close(struct snd_soc_component *component,
return 0;
}
+static int acp63_pdm_dma_prepare(struct snd_pcm_substream *substream,
+ struct snd_soc_dai *dai)
+{
+ struct pdm_stream_instance *rtd = substream->runtime->private_data;
+
+ if (!rtd || substream->stream != SNDRV_PCM_STREAM_CAPTURE)
+ return 0;
+
+ /*
+ * Enable the PDM decimator now, before the DMA starts. The
+ * CIC filter chain and the microphone itself need time to reach
+ * a stable state after the clock is applied. Without this
+ * settling period the first frames captured by the DMA contain
+ * a saturated Nyquist-rate transient. Wait for the decimator
+ * to flush, then clear the FIFO so the DMA sees a clean buffer.
+ */
+ acp63_enable_pdm_clock(rtd->acp63_base);
+ writel(ACP_PDM_ENABLE, rtd->acp63_base + ACP_WOV_PDM_ENABLE);
+ msleep(PDM_SETTLING_DELAY_MS);
+ writel(0x01, rtd->acp63_base + ACP_WOV_PDM_FIFO_FLUSH);
+ return 0;
+}
+
static int acp63_pdm_dai_trigger(struct snd_pcm_substream *substream,
int cmd, struct snd_soc_dai *dai)
{
@@ -306,21 +374,43 @@ static int acp63_pdm_dai_trigger(struct snd_pcm_substream *substream,
switch (cmd) {
case SNDRV_PCM_TRIGGER_START:
case SNDRV_PCM_TRIGGER_RESUME:
+ writel(ch_mask, rtd->acp63_base + ACP_WOV_PDM_NO_OF_CHANNELS);
+ writel(PDM_DECIMATION_FACTOR, rtd->acp63_base + ACP_WOV_PDM_DECIMATION_FACTOR);
+ rtd->bytescount = acp63_pdm_get_byte_count(rtd, substream->stream);
+ pdm_status = acp63_check_pdm_dma_status(rtd->acp63_base);
+ if (!pdm_status)
+ ret = acp63_start_pdm_dma(rtd->acp63_base);
+ break;
case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
writel(ch_mask, rtd->acp63_base + ACP_WOV_PDM_NO_OF_CHANNELS);
writel(PDM_DECIMATION_FACTOR, rtd->acp63_base + ACP_WOV_PDM_DECIMATION_FACTOR);
rtd->bytescount = acp63_pdm_get_byte_count(rtd, substream->stream);
+ /*
+ * The PDM decimator was left running during PAUSE_PUSH so no
+ * 300 ms settling delay is needed. Flush the FIFO to discard
+ * samples accumulated while the DMA was stopped, then restart
+ * the DMA.
+ */
+ writel(0x01, rtd->acp63_base + ACP_WOV_PDM_FIFO_FLUSH);
pdm_status = acp63_check_pdm_dma_status(rtd->acp63_base);
if (!pdm_status)
ret = acp63_start_pdm_dma(rtd->acp63_base);
break;
case SNDRV_PCM_TRIGGER_STOP:
case SNDRV_PCM_TRIGGER_SUSPEND:
- case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
pdm_status = acp63_check_pdm_dma_status(rtd->acp63_base);
if (pdm_status)
ret = acp63_stop_pdm_dma(rtd->acp63_base);
break;
+ case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+ /*
+ * Leave the PDM decimator running so that PAUSE_RELEASE does
+ * not need the 300 ms CIC settling delay. Only stop the DMA.
+ */
+ pdm_status = acp63_check_pdm_dma_status(rtd->acp63_base);
+ if (pdm_status)
+ ret = acp63_pause_pdm_dma(rtd->acp63_base);
+ break;
default:
ret = -EINVAL;
break;
@@ -329,6 +419,7 @@ static int acp63_pdm_dai_trigger(struct snd_pcm_substream *substream,
}
static const struct snd_soc_dai_ops acp63_pdm_dai_ops = {
+ .prepare = acp63_pdm_dma_prepare,
.trigger = acp63_pdm_dai_trigger,
};
--
2.48.1
From 1f401ac36327baf67554612b01fd2831a1ac59eb Mon Sep 17 00:00:00 2001
From: Vijendar Mukunda <Vijendar.Mukunda@xxxxxxx>
Date: Fri, 14 Aug 2026 11:38:45 +0530
Subject: [PATCH V2 2/3] ASoC: amd: ps: pdm: fix decimator transient on system
resume
After a system suspend/resume cycle with an active ALSA capture,
TRIGGER_RESUME fires after TRIGGER_SUSPEND without an intervening
prepare() call. TRIGGER_SUSPEND calls acp63_stop_pdm_dma() which
disables both the DMA and the PDM decimator. TRIGGER_RESUME then
called acp63_start_pdm_dma() which re-enables PDM and DMA together
without the 300 ms CIC settling delay or the FIFO flush, producing the
same Nyquist-rate transient that the prepare() fix was designed to
prevent. On hardware the transient caused an immediate XRUN and
broken-pipe error after wake.
Fix by adding the new acp63_pdm_settle() helper that encapsulates the
enable-clock / enable-PDM / msleep / flush-FIFO sequence, and calling
it from acp63_pdm_resume(), the system-sleep PM resume hook, which
runs in sleepable process context before the PCM layer issues
TRIGGER_RESUME. prepare() is also updated to call through this helper.
TRIGGER_RESUME runs under the PCM stream spinlock (IRQs disabled via
snd_pcm_action_lock_irq()), so msleep() is not permitted inside the
trigger callback. Placing the settle in acp63_pdm_resume() ensures the
CIC filter is stable before TRIGGER_RESUME fires, without sleeping in
atomic context.
Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@xxxxxxx>
---
sound/soc/amd/ps/ps-pdm-dma.c | 41 +++++++++++++++++++++++++++++++----
1 file changed, 37 insertions(+), 4 deletions(-)
diff --git a/sound/soc/amd/ps/ps-pdm-dma.c b/sound/soc/amd/ps/ps-pdm-dma.c
index e8f84d83a886..e48dfaf4908d 100644
--- a/sound/soc/amd/ps/ps-pdm-dma.c
+++ b/sound/soc/amd/ps/ps-pdm-dma.c
@@ -331,6 +331,20 @@ static int acp63_pdm_dma_close(struct snd_soc_component *component,
return 0;
}
+/*
+ * acp63_pdm_settle - enable the PDM decimator and wait for it to stabilise.
+ *
+ * Called from prepare() and from TRIGGER_RESUME (where prepare() is not
+ * invoked by the ALSA core). Must not be called from atomic context.
+ */
+static void acp63_pdm_settle(void __iomem *acp_base)
+{
+ acp63_enable_pdm_clock(acp_base);
+ writel(ACP_PDM_ENABLE, acp_base + ACP_WOV_PDM_ENABLE);
+ msleep(PDM_SETTLING_DELAY_MS);
+ writel(0x01, acp_base + ACP_WOV_PDM_FIFO_FLUSH);
+}
+
static int acp63_pdm_dma_prepare(struct snd_pcm_substream *substream,
struct snd_soc_dai *dai)
{
@@ -347,10 +361,7 @@ static int acp63_pdm_dma_prepare(struct snd_pcm_substream *substream,
* a saturated Nyquist-rate transient. Wait for the decimator
* to flush, then clear the FIFO so the DMA sees a clean buffer.
*/
- acp63_enable_pdm_clock(rtd->acp63_base);
- writel(ACP_PDM_ENABLE, rtd->acp63_base + ACP_WOV_PDM_ENABLE);
- msleep(PDM_SETTLING_DELAY_MS);
- writel(0x01, rtd->acp63_base + ACP_WOV_PDM_FIFO_FLUSH);
+ acp63_pdm_settle(rtd->acp63_base);
return 0;
}
@@ -373,10 +384,24 @@ static int acp63_pdm_dai_trigger(struct snd_pcm_substream *substream,
}
switch (cmd) {
case SNDRV_PCM_TRIGGER_START:
+ writel(ch_mask, rtd->acp63_base + ACP_WOV_PDM_NO_OF_CHANNELS);
+ writel(PDM_DECIMATION_FACTOR, rtd->acp63_base + ACP_WOV_PDM_DECIMATION_FACTOR);
+ rtd->bytescount = acp63_pdm_get_byte_count(rtd, substream->stream);
+ pdm_status = acp63_check_pdm_dma_status(rtd->acp63_base);
+ if (!pdm_status)
+ ret = acp63_start_pdm_dma(rtd->acp63_base);
+ break;
case SNDRV_PCM_TRIGGER_RESUME:
writel(ch_mask, rtd->acp63_base + ACP_WOV_PDM_NO_OF_CHANNELS);
writel(PDM_DECIMATION_FACTOR, rtd->acp63_base + ACP_WOV_PDM_DECIMATION_FACTOR);
rtd->bytescount = acp63_pdm_get_byte_count(rtd, substream->stream);
+ /*
+ * acp63_pdm_resume() already called acp63_pdm_settle() in
+ * sleepable PM resume context before the PCM layer issued
+ * TRIGGER_RESUME. The trigger callback runs under the PCM
+ * stream spinlock (irqs disabled), so no msleep() is allowed
+ * here. Just arm the DMA.
+ */
pdm_status = acp63_check_pdm_dma_status(rtd->acp63_base);
if (!pdm_status)
ret = acp63_start_pdm_dma(rtd->acp63_base);
@@ -510,6 +535,14 @@ static int acp63_pdm_resume(struct device *dev)
acp63_config_dma(rtd, SNDRV_PCM_STREAM_CAPTURE);
acp63_init_pdm_ring_buffer(PDM_MEM_WINDOW_START, buffer_len,
period_bytes, adata->acp63_base);
+ /*
+ * TRIGGER_SUSPEND disabled the PDM decimator. The PCM layer
+ * will issue TRIGGER_RESUME under the stream spinlock (IRQs
+ * disabled), so msleep() is not permitted there. Run the
+ * CIC settling sequence here in sleepable PM resume context
+ * before the trigger fires.
+ */
+ acp63_pdm_settle(rtd->acp63_base);
}
acp63_enable_pdm_interrupts(adata);
return 0;
--
2.48.1
From 56300d48776be7f8424ca96b5ca46b74383ba9d1 Mon Sep 17 00:00:00 2001
From: Vijendar Mukunda <Vijendar.Mukunda@xxxxxxx>
Date: Fri, 14 Aug 2026 11:39:10 +0530
Subject: [PATCH V2 3/3] ASoC: amd: ps: pdm: set PDM DAI link as non-atomic
The prepare() DAI callback calls msleep() via acp63_pdm_settle() to
allow the CIC filter to settle before DMA starts. Mark the DAI link
nonatomic = 1 so the trigger path is also nonatomic, matching the
sleep requirements of the ops registered on this link and preventing
any future msleep() inside the trigger from hitting a scheduling-while-
atomic BUG.
Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@xxxxxxx>
---
sound/soc/amd/ps/ps-mach.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/sound/soc/amd/ps/ps-mach.c b/sound/soc/amd/ps/ps-mach.c
index 3e49c255c025..a040b5fcb0b7 100644
--- a/sound/soc/amd/ps/ps-mach.c
+++ b/sound/soc/amd/ps/ps-mach.c
@@ -32,6 +32,7 @@ static struct snd_soc_dai_link acp63_dai_pdm[] = {
.name = "acp63-dmic-capture",
.stream_name = "DMIC capture",
.capture_only = 1,
+ .nonatomic = 1,
SND_SOC_DAILINK_REG(acp63_pdm, dmic_codec, pdm_platform),
},
};
--
2.48.1