Re: [External Mail] Re: [PATCH] ALSA: hda/core: Log stream DMA errors on interrupt

[Takashi Iwai <tiwai@suse.de>]

On Tue, 04 Aug 2026 19:42:17 +0200,
Arun Raghavan wrote:
> 
> On Tue Aug 4, 2026 at 4:21 AM PDT, Takashi Iwai wrote:
> > On Tue, 04 Aug 2026 00:07:52 +0200,
> > Arun Raghavan wrote:
> >> 
> >> The stream descriptor status register reports FIFO and descriptor
> >> errors, but these are currently cleared silently along with the rest
> >> of the interrupt status. Log them, rate-limited, so DMA problems are
> >> visible instead of only manifesting as audible glitches.
> >> 
> >> Observed on some AMD GPU HDMI audio controllers under specific low power
> >> circumstances.
> >> 
> >> Signed-off-by: Arun Raghavan <arunr@valvesoftware.com>
> >> Cc: Arun Raghavan <arun@arunraghavan.net>
> >
> > Applied now to for-next branch.
> >
> > It's interesting at which situation you get the error bit and which
> > one.  If it can be used *reliably* for catching a streaming error, the
> > driver could notify XRUN or error appropriately, too.
> 
> Ah, I should have mentioned that in the commit message. The error bit
> that was signalled was SD_INT_FIFO_ERR -- the status byte was read as
> (SD_STS_FIFO_READY | SD_INT_FIFO_ERR).
> 
> We are still working on pinning down the precise cause in this case, but
> it seems to be related to issues in some specific setups during lower
> frequency memory clock transitions. The error manifests as a short
> dropout caused by what appears to be a stall or missed transfer. The
> frequency of dropouts varies from several per minute to one every few
> minutes.
> 
> In such a case, the existence of XRUNs might be good to know further up
> the stack, though it isn't clear that there is much that userspace can
> autonomously do to mitigate the it.

When we do stop the stream as XRUN and notifies to user-space, usually
it tries to recover / restart -- something like below.

But it's hard to judge whether we should do this, or it can lead
rather to misbehavior.  We need experiments.


thanks,

Takashi

-- 8< --
diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
index aa994d6e6d35..615e48ecb009 100644
--- a/include/sound/hdaudio.h
+++ b/include/sound/hdaudio.h
@@ -412,7 +412,9 @@ void snd_hdac_bus_link_power(struct hdac_device *hdev, bool enable);
 void snd_hdac_bus_update_rirb(struct hdac_bus *bus);
 int snd_hdac_bus_handle_stream_irq(struct hdac_bus *bus, unsigned int status,
 				    void (*ack)(struct hdac_bus *,
-						struct hdac_stream *));
+						struct hdac_stream *),
+				    void (*error)(struct hdac_bus *,
+						  struct hdac_stream *));
 
 int snd_hdac_bus_alloc_stream_pages(struct hdac_bus *bus);
 void snd_hdac_bus_free_stream_pages(struct hdac_bus *bus);
diff --git a/sound/hda/common/controller.c b/sound/hda/common/controller.c
index afec5c5546ec..329854c9fe92 100644
--- a/sound/hda/common/controller.c
+++ b/sound/hda/common/controller.c
@@ -1058,6 +1058,15 @@ static void stream_update(struct hdac_bus *bus, struct hdac_stream *s)
 	}
 }
 
+static void stream_error(struct hdac_bus *bus, struct hdac_stream *s)
+{
+	struct azx_dev *azx_dev = stream_to_azx_dev(s);
+
+	spin_unlock(&bus->reg_lock);
+	snd_pcm_stop_xrun(azx_stream(azx_dev)->substream);
+	spin_lock(&bus->reg_lock);
+}
+
 irqreturn_t azx_interrupt(int irq, void *dev_id)
 {
 	struct azx *chip = dev_id;
@@ -1082,7 +1091,8 @@ irqreturn_t azx_interrupt(int irq, void *dev_id)
 
 		handled = true;
 		active = false;
-		if (snd_hdac_bus_handle_stream_irq(bus, status, stream_update))
+		if (snd_hdac_bus_handle_stream_irq(bus, status, stream_update,
+						   stream_error))
 			active = true;
 
 		status = azx_readb(chip, RIRBSTS);
diff --git a/sound/hda/core/controller.c b/sound/hda/core/controller.c
index 78855ac357c6..67bb74c618bf 100644
--- a/sound/hda/core/controller.c
+++ b/sound/hda/core/controller.c
@@ -676,8 +676,10 @@ EXPORT_SYMBOL_GPL(snd_hdac_bus_stop_chip);
  * Returns the bits of handled streams, or zero if no stream is handled.
  */
 int snd_hdac_bus_handle_stream_irq(struct hdac_bus *bus, unsigned int status,
-				    void (*ack)(struct hdac_bus *,
-						struct hdac_stream *))
+				   void (*ack)(struct hdac_bus *,
+					       struct hdac_stream *),
+				   void (*error)(struct hdac_bus *,
+						 struct hdac_stream *))
 {
 	struct hdac_stream *azx_dev;
 	u8 sd_status;
@@ -692,6 +694,8 @@ int snd_hdac_bus_handle_stream_irq(struct hdac_bus *bus, unsigned int status,
 				dev_warn_ratelimited(bus->dev,
 						"stream %u dma error: 0x%02x\n",
 						azx_dev->index, sd_status);
+				if (error)
+					error(bus, azx_dev);
 			}
 			if ((!azx_dev->substream && !azx_dev->cstream) ||
 			    !azx_dev->running || !(sd_status & SD_INT_COMPLETE))
diff --git a/sound/soc/intel/avs/core.c b/sound/soc/intel/avs/core.c
index 1a53856c2ffb..6a5a6e526c2c 100644
--- a/sound/soc/intel/avs/core.c
+++ b/sound/soc/intel/avs/core.c
@@ -270,7 +270,8 @@ static irqreturn_t avs_hda_interrupt(struct hdac_bus *bus)
 	u32 status;
 
 	status = snd_hdac_chip_readl(bus, INTSTS);
-	if (snd_hdac_bus_handle_stream_irq(bus, status, hdac_update_stream))
+	if (snd_hdac_bus_handle_stream_irq(bus, status, hdac_update_stream,
+					   NULL))
 		ret = IRQ_HANDLED;
 
 	spin_lock_irq(&bus->reg_lock);