From 3e106d8b1ac2ffc8481cf4af9db34c525d9b8533 Mon Sep 17 00:00:00 2001 From: Cole Leavitt Date: Fri, 13 Feb 2026 23:40:53 -0700 Subject: [PATCH 1/2] ASoC: SOF: Replace IPC TX busy deferral with bounded retry The SOF IPC4 platform send_msg functions (hda_dsp_ipc4_send_msg, mtl_ipc_send_msg, cnl_ipc4_send_msg) previously stored the message in delayed_ipc_tx_msg and returned 0 when the TX register was busy. The deferred message was supposed to be dispatched from the IRQ handler when the DSP acknowledged the previous message. This mechanism silently drops messages during D0i3 power transitions because the IRQ handler never fires while the DSP is in a low-power state. The caller then hangs in wait_event_timeout() for up to 500ms per IPC chunk, causing multi-second audio stalls under CPU load. Fix this by making the platform send_msg functions return -EBUSY immediately when the TX register is busy (safe since they execute under spin_lock_irq in sof_ipc_send_msg), and adding a bounded retry loop with usleep_range() in ipc4_tx_msg_unlocked() which only holds the tx_mutex (a sleepable context). The retry loop attempts up to 50 iterations with 100-200us delays, bounding the maximum busy-wait to approximately 10ms instead of the previous 500ms timeout. Also remove the now-dead delayed_ipc_tx_msg field from sof_intel_hda_dev, the dispatch code, and the ack_received tracking variable from all three IRQ thread handlers (hda_dsp_ipc4_irq_thread, mtl_ipc_irq_thread, cnl_ipc4_irq_thread). Signed-off-by: Cole Leavitt Signed-off-by: Peter Ujfalusi --- sound/soc/sof/intel/cnl.c | 17 ++--------------- sound/soc/sof/intel/hda-ipc.c | 17 ++--------------- sound/soc/sof/intel/hda.h | 8 -------- sound/soc/sof/intel/mtl.c | 17 ++--------------- sound/soc/sof/ipc4.c | 17 +++++++++++++++-- 5 files changed, 21 insertions(+), 55 deletions(-) diff --git a/sound/soc/sof/intel/cnl.c b/sound/soc/sof/intel/cnl.c index 69376fb5b20d41..09ea87cebbc702 100644 --- a/sound/soc/sof/intel/cnl.c +++ b/sound/soc/sof/intel/cnl.c @@ -37,7 +37,6 @@ irqreturn_t cnl_ipc4_irq_thread(int irq, void *context) { struct sof_ipc4_msg notification_data = {{ 0 }}; struct snd_sof_dev *sdev = context; - bool ack_received = false; bool ipc_irq = false; u32 hipcida, hipctdr; @@ -51,7 +50,6 @@ irqreturn_t cnl_ipc4_irq_thread(int irq, void *context) cnl_ipc_dsp_done(sdev); ipc_irq = true; - ack_received = true; } if (hipctdr & CNL_DSP_REG_HIPCTDR_BUSY) { @@ -98,13 +96,6 @@ irqreturn_t cnl_ipc4_irq_thread(int irq, void *context) /* This interrupt is not shared so no need to return IRQ_NONE. */ dev_dbg_ratelimited(sdev->dev, "nothing to do in IPC IRQ thread\n"); - if (ack_received) { - struct sof_intel_hda_dev *hdev = sdev->pdata->hw_pdata; - - if (hdev->delayed_ipc_tx_msg) - cnl_ipc4_send_msg(sdev, hdev->delayed_ipc_tx_msg); - } - return IRQ_HANDLED; } EXPORT_SYMBOL_NS(cnl_ipc4_irq_thread, "SND_SOC_SOF_INTEL_CNL"); @@ -259,12 +250,8 @@ int cnl_ipc4_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg) struct sof_intel_hda_dev *hdev = sdev->pdata->hw_pdata; struct sof_ipc4_msg *msg_data = msg->msg_data; - if (hda_ipc4_tx_is_busy(sdev)) { - hdev->delayed_ipc_tx_msg = msg; - return 0; - } - - hdev->delayed_ipc_tx_msg = NULL; + if (hda_ipc4_tx_is_busy(sdev)) + return -EBUSY; /* send the message via mailbox */ if (msg_data->data_size) diff --git a/sound/soc/sof/intel/hda-ipc.c b/sound/soc/sof/intel/hda-ipc.c index 2aef3954f4f7f7..caf39ca6e16317 100644 --- a/sound/soc/sof/intel/hda-ipc.c +++ b/sound/soc/sof/intel/hda-ipc.c @@ -106,12 +106,8 @@ int hda_dsp_ipc4_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg) struct sof_intel_hda_dev *hdev = sdev->pdata->hw_pdata; struct sof_ipc4_msg *msg_data = msg->msg_data; - if (hda_ipc4_tx_is_busy(sdev)) { - hdev->delayed_ipc_tx_msg = msg; - return 0; - } - - hdev->delayed_ipc_tx_msg = NULL; + if (hda_ipc4_tx_is_busy(sdev)) + return -EBUSY; /* send the message via mailbox */ if (msg_data->data_size) @@ -168,7 +164,6 @@ irqreturn_t hda_dsp_ipc4_irq_thread(int irq, void *context) { struct sof_ipc4_msg notification_data = {{ 0 }}; struct snd_sof_dev *sdev = context; - bool ack_received = false; bool ipc_irq = false; u32 hipcie, hipct; @@ -182,7 +177,6 @@ irqreturn_t hda_dsp_ipc4_irq_thread(int irq, void *context) hda_dsp_ipc_dsp_done(sdev); ipc_irq = true; - ack_received = true; } if (hipct & HDA_DSP_REG_HIPCT_BUSY) { @@ -233,13 +227,6 @@ irqreturn_t hda_dsp_ipc4_irq_thread(int irq, void *context) /* This interrupt is not shared so no need to return IRQ_NONE. */ dev_dbg_ratelimited(sdev->dev, "nothing to do in IPC IRQ thread\n"); - if (ack_received) { - struct sof_intel_hda_dev *hdev = sdev->pdata->hw_pdata; - - if (hdev->delayed_ipc_tx_msg) - hda_dsp_ipc4_send_msg(sdev, hdev->delayed_ipc_tx_msg); - } - return IRQ_HANDLED; } EXPORT_SYMBOL_NS(hda_dsp_ipc4_irq_thread, "SND_SOC_SOF_INTEL_HDA_COMMON"); diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index 3f0966477ace21..44db3121b29cb4 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -549,14 +549,6 @@ struct sof_intel_hda_dev { /* work queue for mic privacy state change notification sending */ struct sof_ace3_mic_privacy mic_privacy; - - /* - * Pointing to the IPC message if immediate sending was not possible - * because the downlink communication channel was BUSY at the time. - * The message will be re-tried when the channel becomes free (the ACK - * is received from the DSP for the previous message) - */ - struct snd_sof_ipc_msg *delayed_ipc_tx_msg; }; static inline struct hdac_bus *sof_to_bus(struct snd_sof_dev *s) diff --git a/sound/soc/sof/intel/mtl.c b/sound/soc/sof/intel/mtl.c index 07644b90c037d0..b44d4b98c65bd5 100644 --- a/sound/soc/sof/intel/mtl.c +++ b/sound/soc/sof/intel/mtl.c @@ -101,12 +101,8 @@ static int mtl_ipc_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *ms struct sof_intel_hda_dev *hdev = sdev->pdata->hw_pdata; struct sof_ipc4_msg *msg_data = msg->msg_data; - if (hda_ipc4_tx_is_busy(sdev)) { - hdev->delayed_ipc_tx_msg = msg; - return 0; - } - - hdev->delayed_ipc_tx_msg = NULL; + if (hda_ipc4_tx_is_busy(sdev)) + return -EBUSY; /* send the message via mailbox */ if (msg_data->data_size) @@ -562,7 +558,6 @@ static irqreturn_t mtl_ipc_irq_thread(int irq, void *context) { struct sof_ipc4_msg notification_data = {{ 0 }}; struct snd_sof_dev *sdev = context; - bool ack_received = false; bool ipc_irq = false; u32 hipcida; u32 hipctdr; @@ -579,7 +574,6 @@ static irqreturn_t mtl_ipc_irq_thread(int irq, void *context) mtl_ipc_dsp_done(sdev); ipc_irq = true; - ack_received = true; } if (hipctdr & MTL_DSP_REG_HFIPCXTDR_BUSY) { @@ -628,13 +622,6 @@ static irqreturn_t mtl_ipc_irq_thread(int irq, void *context) dev_dbg_ratelimited(sdev->dev, "nothing to do in IPC IRQ thread\n"); } - if (ack_received) { - struct sof_intel_hda_dev *hdev = sdev->pdata->hw_pdata; - - if (hdev->delayed_ipc_tx_msg) - mtl_ipc_send_msg(sdev, hdev->delayed_ipc_tx_msg); - } - return IRQ_HANDLED; } diff --git a/sound/soc/sof/ipc4.c b/sound/soc/sof/ipc4.c index d9bb8b79f91ecf..3ddf7ebee7e8e0 100644 --- a/sound/soc/sof/ipc4.c +++ b/sound/soc/sof/ipc4.c @@ -367,20 +367,33 @@ static int ipc4_wait_tx_done(struct snd_sof_ipc *ipc, void *reply_data) return ret; } +#define SOF_IPC4_TX_BUSY_RETRIES 50 +#define SOF_IPC4_TX_BUSY_DELAY_US 100 +#define SOF_IPC4_TX_BUSY_DELAY_MAX_US 200 + static int ipc4_tx_msg_unlocked(struct snd_sof_ipc *ipc, void *msg_data, size_t msg_bytes, void *reply_data, size_t reply_bytes) { struct sof_ipc4_msg *ipc4_msg = msg_data; struct snd_sof_dev *sdev = ipc->sdev; - int ret; + int ret, i; if (msg_bytes > ipc->max_payload_size || reply_bytes > ipc->max_payload_size) return -EINVAL; sof_ipc4_log_header(sdev->dev, "ipc tx ", msg_data, true); - ret = sof_ipc_send_msg(sdev, msg_data, msg_bytes, reply_bytes); + for (i = 0; i < SOF_IPC4_TX_BUSY_RETRIES; i++) { + ret = sof_ipc_send_msg(sdev, msg_data, msg_bytes, reply_bytes); + if (ret != -EBUSY) + break; + usleep_range(SOF_IPC4_TX_BUSY_DELAY_US, + SOF_IPC4_TX_BUSY_DELAY_MAX_US); + } + if (i == SOF_IPC4_TX_BUSY_RETRIES) + dev_dbg(sdev->dev, "%s: TX still busy after %d retries\n", + __func__, i); if (ret) { dev_err_ratelimited(sdev->dev, "%s: ipc message send for %#x|%#x failed: %d\n", From 358f19d627add7915685f947f8fb3a0f6f503db2 Mon Sep 17 00:00:00 2001 From: Cole Leavitt Date: Fri, 13 Feb 2026 23:40:54 -0700 Subject: [PATCH 2/2] ASoC: SOF: Add platform ops callback for DAI link hardware readiness After suspend/resume (D3->D0), the SOF firmware is reloaded fresh and pipelines are recreated lazily when userspace opens a PCM. However, SoundWire slave re-enumeration runs asynchronously via a 100ms delayed work item (SDW_INTEL_DELAYED_ENUMERATION_MS). If userspace attempts to play audio before SoundWire slaves finish re-enumerating, the firmware returns error 9 (resource not found) when creating ALH copier modules, leaving the DSP in an unrecoverable wedged state requiring reboot. Add a new optional dai_link_hw_ready callback to struct snd_sof_dsp_ops that allows platform-specific code to wait for DAI link hardware to become ready before pipeline setup. The generic ipc4-topology.c calls this callback (when set) in sof_ipc4_prepare_copier_module() before configuring DAI copiers, maintaining SOF's platform abstraction. The Intel HDA implementation (hda_sdw_dai_hw_ready) waits for all attached SoundWire slaves to complete initialization using wait_for_completion_interruptible_timeout() with a 2-second timeout. This is safe for multiple waiters since the SoundWire subsystem uses complete_all() for initialization_complete. Unattached slaves (declared in ACPI but not physically present) are skipped to avoid false timeouts. The function returns -ETIMEDOUT on timeout (instead of warn-and-continue) to prevent the DSP from entering a wedged state. On non-resume paths the completions are already done, so the wait returns immediately. Link: https://github.com/thesofproject/sof/issues/8662 Link: https://github.com/thesofproject/sof/issues/9308 Signed-off-by: Cole Leavitt Signed-off-by: Peter Ujfalusi --- sound/soc/sof/intel/hda-common-ops.c | 1 + sound/soc/sof/intel/hda.c | 44 ++++++++++++++++++++++++++++ sound/soc/sof/intel/hda.h | 6 ++++ sound/soc/sof/ipc4-topology.c | 8 +++++ sound/soc/sof/sof-priv.h | 3 ++ 5 files changed, 62 insertions(+) diff --git a/sound/soc/sof/intel/hda-common-ops.c b/sound/soc/sof/intel/hda-common-ops.c index 746b426b1329b0..315cb61426da26 100644 --- a/sound/soc/sof/intel/hda-common-ops.c +++ b/sound/soc/sof/intel/hda-common-ops.c @@ -84,6 +84,7 @@ const struct snd_sof_dsp_ops sof_hda_common_ops = { .unregister_ipc_clients = hda_unregister_clients, /* DAI drivers */ + .dai_link_hw_ready = hda_sdw_dai_hw_ready, .drv = skl_dai, .num_drv = SOF_SKL_NUM_DAIS, .is_chain_dma_supported = hda_is_chain_dma_supported, diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c index c0cc7d3ce5262b..e2edbc9c42b3f9 100644 --- a/sound/soc/sof/intel/hda.c +++ b/sound/soc/sof/intel/hda.c @@ -378,6 +378,50 @@ static void hda_dsp_sdw_process_mic_privacy(struct snd_sof_dev *sdev) chip->process_mic_privacy(sdev, true, AZX_REG_ML_LEPTR_ID_SDW); } +int hda_sdw_dai_hw_ready(struct snd_sof_dev *sdev, int dai_type) +{ + struct sof_intel_hda_dev *hdev = sdev->pdata->hw_pdata; + struct sdw_peripherals *sdw_p; + long ret; + int idx; + + if (dai_type != SOF_DAI_INTEL_ALH) + return 0; + + if (!hdev || !hdev->sdw || !hdev->sdw->peripherals) + return 0; + + sdw_p = hdev->sdw->peripherals; + + for (idx = 0; idx < sdw_p->num_peripherals; idx++) { + struct sdw_slave *slave = sdw_p->array[idx]; + + if (!slave) + continue; + + if (slave->status != SDW_SLAVE_ATTACHED) + continue; + + ret = wait_for_completion_interruptible_timeout( + &slave->initialization_complete, + msecs_to_jiffies(2000)); + if (ret == 0) { + dev_err(sdev->dev, + "timeout waiting for SoundWire slave %s initialization\n", + dev_name(&slave->dev)); + return -ETIMEDOUT; + } + if (ret < 0) { + dev_dbg(sdev->dev, + "interrupted waiting for SoundWire slave %s initialization: %ld\n", + dev_name(&slave->dev), ret); + return ret; + } + } + + return 0; +} + #else /* IS_ENABLED(CONFIG_SND_SOC_SOF_INTEL_SOUNDWIRE) */ static inline int hda_sdw_acpi_scan(struct snd_sof_dev *sdev) { diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index 44db3121b29cb4..436f993e535cf1 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -833,6 +833,7 @@ bool hda_sdw_check_wakeen_irq_common(struct snd_sof_dev *sdev); void hda_sdw_process_wakeen_common(struct snd_sof_dev *sdev); void hda_sdw_process_wakeen(struct snd_sof_dev *sdev); bool hda_common_check_sdw_irq(struct snd_sof_dev *sdev); +int hda_sdw_dai_hw_ready(struct snd_sof_dev *sdev, int dai_type); #else @@ -882,6 +883,11 @@ static inline bool hda_common_check_sdw_irq(struct snd_sof_dev *sdev) return false; } +static inline int hda_sdw_dai_hw_ready(struct snd_sof_dev *sdev, int dai_type) +{ + return 0; +} + #endif int sdw_hda_dai_hw_params(struct snd_pcm_substream *substream, diff --git a/sound/soc/sof/ipc4-topology.c b/sound/soc/sof/ipc4-topology.c index 9379569ca66976..d68345042977d7 100644 --- a/sound/soc/sof/ipc4-topology.c +++ b/sound/soc/sof/ipc4-topology.c @@ -2300,6 +2300,14 @@ sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget, case snd_soc_dapm_dai_in: case snd_soc_dapm_dai_out: { + /* Wait for DAI link hardware (e.g. SoundWire slaves) to be ready */ + if (sdev->pdata->desc->ops->dai_link_hw_ready) { + ret = sdev->pdata->desc->ops->dai_link_hw_ready( + sdev, ipc4_copier->dai_type); + if (ret) + return ret; + } + /* * Only SOF_DAI_INTEL_ALH needs copier_data to set blob. * That's why only ALH dai's blob is set after sof_ipc4_init_input_audio_fmt diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index d90d4524b6002e..bc4ae2291e7c3e 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -350,6 +350,9 @@ struct snd_sof_dsp_ops { int (*register_ipc_clients)(struct snd_sof_dev *sdev); /* optional */ void (*unregister_ipc_clients)(struct snd_sof_dev *sdev); /* optional */ + /* optional: wait for DAI link hardware readiness (e.g. SoundWire slave init) */ + int (*dai_link_hw_ready)(struct snd_sof_dev *sdev, int dai_type); /* optional */ + /* DAI ops */ struct snd_soc_dai_driver *drv; int num_drv;