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

[Takashi Iwai <tiwai@suse.de>]

On Wed, 05 Aug 2026 22:56:40 +0200,
Arun Raghavan wrote:
> 
> On Tue Aug 4, 2026 at 11:18 AM PDT, Takashi Iwai wrote:
> > 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);
> 
> This did work to surface the errors to userspace as XRUNs.
> 
> Of course, because this is an actual error in the transfer between the
> CPU and controller, it does not help mitigate the actual problem itself.

So what's the best option for users if this happens?
That is, what happens if we don't restart the stream?  Is it a
temporary fail-out and the hardware recovers / resync by itself while
streaming further?  If so and it's short, it might be better to leave
it as is.  OTOH, if it's a fatal error that needs some manual
recovery, a notification to user-space for recovery is required (if
any).


thanks,

Takashi