Skip to contentSkip to conversation

ALSA: hda - clear IRQ statu twice on some Intel platforms

RFC PATCH In review [email protected] 3 participants 8 messages Feb 22, 2016 – Feb 26, 2016 Sound

lore
Share & tools
lore

[email protected]

author
#0 lore raw

RFC PATCH 1/1 ALSA: hda - clear IRQ statu twice on some Intel platforms

From: Libin Yang <[email protected]>

On some Intel platforms, we found the interrupt issue in the below scenario:
1. driver is in irq handler
2. there is another interrupt from HW after interrupt status is cleared and before exiting from interrupt handler
3. exit from the current irq handling

After exiting the irq handler, it should raise another interrupt for driver to handle the new interrupt. But actually, it failed to raise the interrupt and driver will never have chance to clear the interrupt status.

The patch clears the interrupt status again just before exiting for interrupt handler. This can reduce the contest dramatically.

Signed-off-by Libin Yang <[email protected]>
---
 sound/pci/hda/hda_controller.c |  3 +++
 sound/pci/hda/hda_controller.h |  3 +++
 sound/pci/hda/hda_intel.c      | 40 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 46 insertions(+)
sound/pci/hda/hda_controller.c +3 -0
index 37cf9ce..2ca95c7 100644
@@ -963,6 +963,9 @@ irqreturn_t azx_interrupt(int irq, void *dev_id)
 		azx_writeb(chip, RIRBSTS, RIRB_INT_MASK);
 	}
 
+	if (chip->post_irq)
+		chip->post_irq(chip);
+
 	spin_unlock(&bus->reg_lock);
 
 	return IRQ_HANDLED;
sound/pci/hda/hda_controller.h +3 -0
index ec63bbf..ce59997 100644
@@ -146,6 +146,9 @@ struct azx {
 	const struct firmware *fw;
 #endif
 
+	/* callback at the end of interrupt handler  */
+	void (*post_irq)(struct azx *);
+
 	/* flags */
 	int bdl_pos_adj;
 	int poll_count;
sound/pci/hda/hda_intel.c +40 -0
index 4045dca..6a47d88 100644
@@ -1663,6 +1663,43 @@ static int azx_create(struct snd_card *card, struct pci_dev *pci,
 	return 0;
 }
 
+/* HSW, BDW, SKL and BXT need do post_irq() */
+#define INTEL_IRQ_POST(chip) \
+	(((chip)->pci->vendor == PCI_VENDOR_ID_INTEL) && \
+	 (((chip)->pci->device == 0x0a0c) || \
+	 ((chip)->pci->device == 0x0c0c) || \
+	 ((chip)->pci->device == 0x0d0c) || \
+	 ((chip)->pci->device == 0x160c) || \
+	 ((chip)->pci->device == 0xa170) || \
+	 ((chip)->pci->device == 0x9d70) || \
+	 ((chip)->pci->device == 0x5a98)))
+
+
+/* on some intel platforms, if there occurs an interrupt
+ * when irq is being handled, interrupt signal will not be raised
+ * after the irq handler returns. And the interrupt status may never
+ * be cleared.
+ * So let's clear all the interrupt status before return from irq handler.
+ * This can help to reduce the contest between the irq handler and the signal.
+ */
+static void intel_post_irq(struct azx *chip)
+{
+	struct hdac_bus *bus = azx_bus(chip);
+	struct hdac_stream *azx_dev;
+
+	list_for_each_entry(azx_dev, &bus->stream_list, list)
+		snd_hdac_stream_writeb(azx_dev, SD_STS, SD_INT_MASK);
+
+	/* clear STATESTS */
+	snd_hdac_chip_writew(bus, STATESTS, STATESTS_INT_MASK);
+
+	/* clear rirb status */
+	snd_hdac_chip_writeb(bus, RIRBSTS, RIRB_INT_MASK);
+
+	/* clear int status */
+	snd_hdac_chip_writel(bus, INTSTS, AZX_INT_CTRL_EN | AZX_INT_ALL_STREAM);
+}
+
 static int azx_first_init(struct azx *chip)
 {
 	int dev = chip->dev_index;

@@ -1717,6 +1754,9 @@ static int azx_first_init(struct azx *chip)
 	if (chip->pci->vendor == PCI_VENDOR_ID_AMD)
 		dma_bits = 40;
 
+	if (INTEL_IRQ_POST(chip))
+		chip->post_irq = intel_post_irq;
+
 	/* disable SB600 64bit support for safety */
 	if (chip->pci->vendor == PCI_VENDOR_ID_ATI) {
 		struct pci_dev *p_smbus;
Signature · 1 line
1.9.1
#1 lore raw
Quoted 15 lines · On Mon, 22 Feb 2016 08:39:50 +0100, [email protected] wrote:

From: Libin Yang <[email protected]>

On some Intel platforms, we found the interrupt issue in the below scenario:
1. driver is in irq handler
2. there is another interrupt from HW after interrupt status is cleared and before exiting from interrupt handler
3. exit from the current irq handling

After exiting the irq handler, it should raise another interrupt for driver to handle the new interrupt. But actually, it failed to raise the interrupt and driver will never have chance to clear the interrupt status.

Quoted 4 lines

The patch clears the interrupt status again just before exiting for interrupt handler. This can reduce the contest dramatically.

Signed-off-by Libin Yang <[email protected]>

An alternative way is to loop while the status bit is reenabled. An untested patch is below. Not sure which is better yet. Just an idea.

Takashi

---

include/sound/hdaudio.h +1 -1
index e2b712c90d3f..b74d9c8dceff 100644
@@ -343,7 +343,7 @@ void snd_hdac_bus_enter_link_reset(struct hdac_bus *bus);
 void snd_hdac_bus_exit_link_reset(struct hdac_bus *bus);
 
 void snd_hdac_bus_update_rirb(struct hdac_bus *bus);
-void snd_hdac_bus_handle_stream_irq(struct hdac_bus *bus, unsigned int status,
+bool snd_hdac_bus_handle_stream_irq(struct hdac_bus *bus, unsigned int status,
 				    void (*ack)(struct hdac_bus *,
 						struct hdac_stream *));
sound/hda/hdac_controller.c +4 -1
index b5a17cb510a0..448cd8c990f0 100644
@@ -427,17 +427,19 @@ EXPORT_SYMBOL_GPL(snd_hdac_bus_stop_chip);
  * @status: INTSTS register value
  * @ask: callback to be called for woken streams
  */
-void snd_hdac_bus_handle_stream_irq(struct hdac_bus *bus, unsigned int status,
+bool snd_hdac_bus_handle_stream_irq(struct hdac_bus *bus, unsigned int status,
 				    void (*ack)(struct hdac_bus *,
 						struct hdac_stream *))
 {
 	struct hdac_stream *azx_dev;
 	u8 sd_status;
+	bool handled = false;
 
 	list_for_each_entry(azx_dev, &bus->stream_list, list) {
 		if (status & azx_dev->sd_int_sta_mask) {
 			sd_status = snd_hdac_stream_readb(azx_dev, SD_STS);
 			snd_hdac_stream_writeb(azx_dev, SD_STS, SD_INT_MASK);
+			handled = true;
 			if (!azx_dev->substream || !azx_dev->running ||
 			    !(sd_status & SD_INT_COMPLETE))
 				continue;

@@ -445,6 +447,7 @@ void snd_hdac_bus_handle_stream_irq(struct hdac_bus *bus, unsigned int status,
 				ack(bus, azx_dev);
 		}
 	}
+	return handled;
 }
 EXPORT_SYMBOL_GPL(snd_hdac_bus_handle_stream_irq);
sound/pci/hda/hda_controller.c +23 -15
index 37cf9cee9835..a4226026b1dc 100644
@@ -930,6 +930,7 @@ irqreturn_t azx_interrupt(int irq, void *dev_id)
 	struct azx *chip = dev_id;
 	struct hdac_bus *bus = azx_bus(chip);
 	u32 status;
+	bool handled = false;
 
 #ifdef CONFIG_PM
 	if (azx_has_pm_runtime(chip))

@@ -944,28 +945,35 @@ irqreturn_t azx_interrupt(int irq, void *dev_id)
 		return IRQ_NONE;
 	}
 
-	status = azx_readl(chip, INTSTS);
-	if (status == 0 || status == 0xffffffff) {
-		spin_unlock(&bus->reg_lock);
-		return IRQ_NONE;
-	}
+	for (;;) {
+		bool active;
 
-	snd_hdac_bus_handle_stream_irq(bus, status, stream_update);
+		status = azx_readl(chip, INTSTS);
+		if (status == 0 || status == 0xffffffff)
+			break;
 
-	/* clear rirb int */
-	status = azx_readb(chip, RIRBSTS);
-	if (status & RIRB_INT_MASK) {
-		if (status & RIRB_INT_RESPONSE) {
-			if (chip->driver_caps & AZX_DCAPS_CTX_WORKAROUND)
-				udelay(80);
-			snd_hdac_bus_update_rirb(bus);
+		active = snd_hdac_bus_handle_stream_irq(bus, status, stream_update);
+
+		/* clear rirb int */
+		status = azx_readb(chip, RIRBSTS);
+		if (status & RIRB_INT_MASK) {
+			active = true;
+			if (status & RIRB_INT_RESPONSE) {
+				if (chip->driver_caps & AZX_DCAPS_CTX_WORKAROUND)
+					udelay(80);
+				snd_hdac_bus_update_rirb(bus);
+			}
+			azx_writeb(chip, RIRBSTS, RIRB_INT_MASK);
 		}
-		azx_writeb(chip, RIRBSTS, RIRB_INT_MASK);
+
+		if (!active)
+			break;
+		handled = true;
 	}
 
 	spin_unlock(&bus->reg_lock);
 
-	return IRQ_HANDLED;
+	return IRQ_RETVAL(handled);
 }
 EXPORT_SYMBOL_GPL(azx_interrupt);
#2 lore raw

Hi Takashi,

I'm testing your patch in stress level. So far, it seems to works smoothly.

Which patch do you like? And your patch seems to impact on all the platforms?

Regards,
Libin

Quoted 160 lines

-----Original Message-----
From: Takashi Iwai [mailto:[email protected]]
Sent: Monday, February 22, 2016 4:44 PM
To: [email protected]
Cc: [email protected]; Lin, Mengdong; Yang, Libin
Subject: Re: [alsa-devel] [RFC: PATCH] ALSA: hda - clear IRQ statu twice on some Intel platforms

Quoted 15 lines · On Mon, 22 Feb 2016 08:39:50 +0100, [email protected] wrote:

From: Libin Yang <[email protected]>

On some Intel platforms, we found the interrupt issue in the below scenario:
1. driver is in irq handler
2. there is another interrupt from HW after interrupt status is cleared and before exiting from interrupt handler
3. exit from the current irq handling

After exiting the irq handler, it should raise another interrupt for driver to handle the new interrupt. But actually, it failed to raise the interrupt and driver will never have chance to clear the interrupt status.

Quoted 4 lines

The patch clears the interrupt status again just before exiting for interrupt handler. This can reduce the contest dramatically.

Signed-off-by Libin Yang <[email protected]>

An alternative way is to loop while the status bit is reenabled. An untested patch is below. Not sure which is better yet. Just an idea.

Takashi

---

include/sound/hdaudio.h +1 -1
index e2b712c90d3f..b74d9c8dceff 100644
@@ -343,7 +343,7 @@ void snd_hdac_bus_enter_link_reset(struct
 hdac_bus *bus);
 void snd_hdac_bus_exit_link_reset(struct hdac_bus *bus);
 
 void snd_hdac_bus_update_rirb(struct hdac_bus *bus);
-void snd_hdac_bus_handle_stream_irq(struct hdac_bus *bus, unsigned
 int status,
+bool snd_hdac_bus_handle_stream_irq(struct hdac_bus *bus, unsigned
 int status,
include/sound/hdaudio.h +0 -0
 				    void (*ack)(struct hdac_bus *,
 						struct hdac_stream *));
sound/hda/hdac_controller.c +2 -1
index b5a17cb510a0..448cd8c990f0 100644
@@ -427,17 +427,19 @@
 EXPORT_SYMBOL_GPL(snd_hdac_bus_stop_chip);
  * @status: INTSTS register value
  * @ask: callback to be called for woken streams
  */
-void snd_hdac_bus_handle_stream_irq(struct hdac_bus *bus, unsigned
 int status,
+bool snd_hdac_bus_handle_stream_irq(struct hdac_bus *bus, unsigned
 int status,
 				    void (*ack)(struct hdac_bus *,
 						struct hdac_stream *))
 {
 	struct hdac_stream *azx_dev;
 	u8 sd_status;
+	bool handled = false;
 
 	list_for_each_entry(azx_dev, &bus->stream_list, list) {
 		if (status & azx_dev->sd_int_sta_mask) {
 			sd_status = snd_hdac_stream_readb(azx_dev,
 SD_STS);
 			snd_hdac_stream_writeb(azx_dev, SD_STS,

SD_INT_MASK);

sound/hda/hdac_controller.c +1 -0
+			handled = true;
 			if (!azx_dev->substream || !azx_dev->running

||

sound/hda/hdac_controller.c +1 -0
 			    !(sd_status & SD_INT_COMPLETE))
 				continue;

@@ -445,6 +447,7 @@ void snd_hdac_bus_handle_stream_irq(struct
 hdac_bus *bus, unsigned int status,
 				ack(bus, azx_dev);
 		}
 	}
+	return handled;
 }
 EXPORT_SYMBOL_GPL(snd_hdac_bus_handle_stream_irq);
sound/pci/hda/hda_controller.c +23 -15
index 37cf9cee9835..a4226026b1dc 100644
@@ -930,6 +930,7 @@ irqreturn_t azx_interrupt(int irq, void *dev_id)
 	struct azx *chip = dev_id;
 	struct hdac_bus *bus = azx_bus(chip);
 	u32 status;
+	bool handled = false;
 
 #ifdef CONFIG_PM
 	if (azx_has_pm_runtime(chip))

@@ -944,28 +945,35 @@ irqreturn_t azx_interrupt(int irq, void *dev_id)
 		return IRQ_NONE;
 	}
 
-	status = azx_readl(chip, INTSTS);
-	if (status == 0 || status == 0xffffffff) {
-		spin_unlock(&bus->reg_lock);
-		return IRQ_NONE;
-	}
+	for (;;) {
+		bool active;
 
-	snd_hdac_bus_handle_stream_irq(bus, status, stream_update);
+		status = azx_readl(chip, INTSTS);
+		if (status == 0 || status == 0xffffffff)
+			break;
 
-	/* clear rirb int */
-	status = azx_readb(chip, RIRBSTS);
-	if (status & RIRB_INT_MASK) {
-		if (status & RIRB_INT_RESPONSE) {
-			if (chip->driver_caps &
 AZX_DCAPS_CTX_WORKAROUND)
-				udelay(80);
-			snd_hdac_bus_update_rirb(bus);
+		active = snd_hdac_bus_handle_stream_irq(bus, status,
 stream_update);
+
+		/* clear rirb int */
+		status = azx_readb(chip, RIRBSTS);
+		if (status & RIRB_INT_MASK) {
+			active = true;
+			if (status & RIRB_INT_RESPONSE) {
+				if (chip->driver_caps &
 AZX_DCAPS_CTX_WORKAROUND)
+					udelay(80);
+				snd_hdac_bus_update_rirb(bus);
+			}
+			azx_writeb(chip, RIRBSTS, RIRB_INT_MASK);
 		}
-		azx_writeb(chip, RIRBSTS, RIRB_INT_MASK);
+
+		if (!active)
+			break;
+		handled = true;
 	}
 
 	spin_unlock(&bus->reg_lock);
 
-	return IRQ_HANDLED;
+	return IRQ_RETVAL(handled);

}
EXPORT_SYMBOL_GPL(azx_interrupt);

#3 lore raw
Quoted 6 lines · Libin

Hi Takashi,

I'm testing your patch in stress level. So far, it seems to works smoothly.

Which patch do you like? And your patch seems to impact on all the platforms?

One potential problem in your patch is that the later triggered event shall be ignored by the post-irq ack.

OTOH, my patch would impact to all platforms, yes. However, in theory, all platforms may have this issue, so it's anyway good in one side. In another flip, of course, it might introduce some regression.

We can limit this loop check only for some known platforms, or we let it tested more :)

I'm going to cook the patch a bit more and give you for testing.

thanks,

Takashi

Quoted 165 lines

Regards,
Libin

Quoted 160 lines

-----Original Message-----
From: Takashi Iwai [mailto:[email protected]]
Sent: Monday, February 22, 2016 4:44 PM
To: [email protected]
Cc: [email protected]; Lin, Mengdong; Yang, Libin
Subject: Re: [alsa-devel] [RFC: PATCH] ALSA: hda - clear IRQ statu twice on some Intel platforms

Quoted 15 lines · On Mon, 22 Feb 2016 08:39:50 +0100, [email protected] wrote:

From: Libin Yang <[email protected]>

On some Intel platforms, we found the interrupt issue in the below scenario:
1. driver is in irq handler
2. there is another interrupt from HW after interrupt status is cleared and before exiting from interrupt handler
3. exit from the current irq handling

After exiting the irq handler, it should raise another interrupt for driver to handle the new interrupt. But actually, it failed to raise the interrupt and driver will never have chance to clear the interrupt status.

Quoted 4 lines

The patch clears the interrupt status again just before exiting for interrupt handler. This can reduce the contest dramatically.

Signed-off-by Libin Yang <[email protected]>

An alternative way is to loop while the status bit is reenabled. An untested patch is below. Not sure which is better yet. Just an idea.

Takashi

---

include/sound/hdaudio.h +1 -1
index e2b712c90d3f..b74d9c8dceff 100644
@@ -343,7 +343,7 @@ void snd_hdac_bus_enter_link_reset(struct
 hdac_bus *bus);
 void snd_hdac_bus_exit_link_reset(struct hdac_bus *bus);
 
 void snd_hdac_bus_update_rirb(struct hdac_bus *bus);
-void snd_hdac_bus_handle_stream_irq(struct hdac_bus *bus, unsigned
 int status,
+bool snd_hdac_bus_handle_stream_irq(struct hdac_bus *bus, unsigned
 int status,
include/sound/hdaudio.h +0 -0
 				    void (*ack)(struct hdac_bus *,
 						struct hdac_stream *));
sound/hda/hdac_controller.c +2 -1
index b5a17cb510a0..448cd8c990f0 100644
@@ -427,17 +427,19 @@
 EXPORT_SYMBOL_GPL(snd_hdac_bus_stop_chip);
  * @status: INTSTS register value
  * @ask: callback to be called for woken streams
  */
-void snd_hdac_bus_handle_stream_irq(struct hdac_bus *bus, unsigned
 int status,
+bool snd_hdac_bus_handle_stream_irq(struct hdac_bus *bus, unsigned
 int status,
 				    void (*ack)(struct hdac_bus *,
 						struct hdac_stream *))
 {
 	struct hdac_stream *azx_dev;
 	u8 sd_status;
+	bool handled = false;
 
 	list_for_each_entry(azx_dev, &bus->stream_list, list) {
 		if (status & azx_dev->sd_int_sta_mask) {
 			sd_status = snd_hdac_stream_readb(azx_dev,
 SD_STS);
 			snd_hdac_stream_writeb(azx_dev, SD_STS,

SD_INT_MASK);

sound/hda/hdac_controller.c +1 -0
+			handled = true;
 			if (!azx_dev->substream || !azx_dev->running

||

sound/hda/hdac_controller.c +1 -0
 			    !(sd_status & SD_INT_COMPLETE))
 				continue;

@@ -445,6 +447,7 @@ void snd_hdac_bus_handle_stream_irq(struct
 hdac_bus *bus, unsigned int status,
 				ack(bus, azx_dev);
 		}
 	}
+	return handled;
 }
 EXPORT_SYMBOL_GPL(snd_hdac_bus_handle_stream_irq);
sound/pci/hda/hda_controller.c +23 -15
index 37cf9cee9835..a4226026b1dc 100644
@@ -930,6 +930,7 @@ irqreturn_t azx_interrupt(int irq, void *dev_id)
 	struct azx *chip = dev_id;
 	struct hdac_bus *bus = azx_bus(chip);
 	u32 status;
+	bool handled = false;
 
 #ifdef CONFIG_PM
 	if (azx_has_pm_runtime(chip))

@@ -944,28 +945,35 @@ irqreturn_t azx_interrupt(int irq, void *dev_id)
 		return IRQ_NONE;
 	}
 
-	status = azx_readl(chip, INTSTS);
-	if (status == 0 || status == 0xffffffff) {
-		spin_unlock(&bus->reg_lock);
-		return IRQ_NONE;
-	}
+	for (;;) {
+		bool active;
 
-	snd_hdac_bus_handle_stream_irq(bus, status, stream_update);
+		status = azx_readl(chip, INTSTS);
+		if (status == 0 || status == 0xffffffff)
+			break;
 
-	/* clear rirb int */
-	status = azx_readb(chip, RIRBSTS);
-	if (status & RIRB_INT_MASK) {
-		if (status & RIRB_INT_RESPONSE) {
-			if (chip->driver_caps &
 AZX_DCAPS_CTX_WORKAROUND)
-				udelay(80);
-			snd_hdac_bus_update_rirb(bus);
+		active = snd_hdac_bus_handle_stream_irq(bus, status,
 stream_update);
+
+		/* clear rirb int */
+		status = azx_readb(chip, RIRBSTS);
+		if (status & RIRB_INT_MASK) {
+			active = true;
+			if (status & RIRB_INT_RESPONSE) {
+				if (chip->driver_caps &
 AZX_DCAPS_CTX_WORKAROUND)
+					udelay(80);
+				snd_hdac_bus_update_rirb(bus);
+			}
+			azx_writeb(chip, RIRBSTS, RIRB_INT_MASK);
 		}
-		azx_writeb(chip, RIRBSTS, RIRB_INT_MASK);
+
+		if (!active)
+			break;
+		handled = true;
 	}
 
 	spin_unlock(&bus->reg_lock);
 
-	return IRQ_HANDLED;
+	return IRQ_RETVAL(handled);

}
EXPORT_SYMBOL_GPL(azx_interrupt);

#4 lore raw
Quoted 15 lines

-----Original Message-----
From: [email protected] [mailto:alsa-devel- [email protected]] On Behalf Of Takashi Iwai
Sent: Tuesday, February 23, 2016 5:28 PM
To: Yang, Libin
Cc: Lin, Mengdong; [email protected]; alsa-devel@alsa- project.org
Subject: Re: [alsa-devel] [RFC: PATCH] ALSA: hda - clear IRQ statu twice on some Intel platforms

Quoted 3 lines · Libin

Hi Takashi,

Quoted 7 lines
Quoted 3 lines

I'm testing your patch in stress level. So far, it seems to works smoothly.

Which patch do you like? And your patch seems to impact on all the

platforms?

One potential problem in your patch is that the later triggered event shall be ignored by the post-irq ack.

Yes. So it seems your patch is better.

Quoted 9 lines

OTOH, my patch would impact to all platforms, yes. However, in theory, all platforms may have this issue, so it's anyway good in one side. In another flip, of course, it might introduce some regression.

We can limit this loop check only for some known platforms, or we let it tested more :)

I'm going to cook the patch a bit more and give you for testing.

I'd like to test your updated patch.

Regards,
Libin

Quoted 184 lines

thanks,

Takashi

Quoted 10 lines

Regards,
Libin

Quoted 6 lines

-----Original Message-----
From: Takashi Iwai [mailto:[email protected]]
Sent: Monday, February 22, 2016 4:44 PM
To: [email protected]
Cc: [email protected]; Lin, Mengdong; Yang, Libin
Subject: Re: [alsa-devel] [RFC: PATCH] ALSA: hda - clear IRQ statu

twice on

Quoted 39 lines
Quoted 39 lines

some Intel platforms

Quoted 15 lines · On Mon, 22 Feb 2016 08:39:50 +0100, [email protected] wrote:

From: Libin Yang <[email protected]>

On some Intel platforms, we found the interrupt issue in the below scenario:
1. driver is in irq handler
2. there is another interrupt from HW after interrupt status is cleared and before exiting from interrupt handler
3. exit from the current irq handling

After exiting the irq handler, it should raise another interrupt for driver to handle the new interrupt. But actually, it failed to raise the interrupt and driver will never have chance to clear the interrupt status.

Quoted 4 lines

The patch clears the interrupt status again just before exiting for interrupt handler. This can reduce the contest dramatically.

Signed-off-by Libin Yang <[email protected]>

An alternative way is to loop while the status bit is reenabled. An untested patch is below. Not sure which is better yet. Just an idea.

Takashi

---

include/sound/hdaudio.h +0 -0
index e2b712c90d3f..b74d9c8dceff 100644
@@ -343,7 +343,7 @@ void snd_hdac_bus_enter_link_reset(struct
 hdac_bus *bus);
 void snd_hdac_bus_exit_link_reset(struct hdac_bus *bus);
Quoted 2 lines

include/sound/hdaudio.h void snd_hdac_bus_enter_link_reset(struct · line 347

Quoted 2 lines
include/sound/hdaudio.h +0 -1
 void snd_hdac_bus_update_rirb(struct hdac_bus *bus);
-void snd_hdac_bus_handle_stream_irq(struct hdac_bus *bus,

unsigned

Quoted 2 lines

include/sound/hdaudio.h void snd_hdac_bus_enter_link_reset(struct · line 348

Quoted 2 lines
include/sound/hdaudio.h +1 -0
 int status,
+bool snd_hdac_bus_handle_stream_irq(struct hdac_bus *bus,

unsigned

Quoted 5 lines

Review

Quoted 5 lines
include/sound/hdaudio.h +0 -0
 int status,
include/sound/hdaudio.h +0 -0
 				    void (*ack)(struct hdac_bus *,
 						struct hdac_stream *));
(quoted diff) +0 -0

b/sound/hda/hdac_controller.c

Quoted 9 lines

sound/hda/hdac_controller.c line 431

Quoted 9 lines

index b5a17cb510a0..448cd8c990f0 100644

sound/hda/hdac_controller.c +0 -1
@@ -427,17 +427,19 @@
 EXPORT_SYMBOL_GPL(snd_hdac_bus_stop_chip);
  * @status: INTSTS register value
  * @ask: callback to be called for woken streams
  */
-void snd_hdac_bus_handle_stream_irq(struct hdac_bus *bus,

unsigned

Quoted 2 lines

sound/hda/hdac_controller.c line 432

Quoted 2 lines
sound/hda/hdac_controller.c +1 -0
 int status,
+bool snd_hdac_bus_handle_stream_irq(struct hdac_bus *bus,

unsigned

Quoted 8 lines
Quoted 8 lines
sound/hda/hdac_controller.c +1 -0
 int status,
 				    void (*ack)(struct hdac_bus *,
 						struct hdac_stream *))
 {
 	struct hdac_stream *azx_dev;
 	u8 sd_status;
+	bool handled = false;
Quoted 12 lines

sound/hda/hdac_controller.c void

Quoted 12 lines
sound/hda/hdac_controller.c +0 -0
 	list_for_each_entry(azx_dev, &bus->stream_list, list) {
 		if (status & azx_dev->sd_int_sta_mask) {
 			sd_status = snd_hdac_stream_readb(azx_dev,
 SD_STS);
 			snd_hdac_stream_writeb(azx_dev, SD_STS,

SD_INT_MASK);

sound/hda/hdac_controller.c +1 -0
+			handled = true;
 			if (!azx_dev->substream || !azx_dev->running

||

sound/hda/hdac_controller.c +0 -0
 			    !(sd_status & SD_INT_COMPLETE))
 				continue;

@@ -445,6 +447,7 @@ void

snd_hdac_bus_handle_stream_irq(struct

Quoted 8 lines
Quoted 8 lines
sound/hda/hdac_controller.c +1 -0
 hdac_bus *bus, unsigned int status,
 				ack(bus, azx_dev);
 		}
 	}
+	return handled;
 }
 EXPORT_SYMBOL_GPL(snd_hdac_bus_handle_stream_irq);
Quoted 6 lines

sound/pci/hda/hda_controller.c irqreturn_t azx_interrupt(int irq, void

Quoted 6 lines
sound/pci/hda/hda_controller.c +0 -0
index 37cf9cee9835..a4226026b1dc 100644
@@ -930,6 +930,7 @@ irqreturn_t azx_interrupt(int irq, void

*dev_id)

Quoted 8 lines

sound/pci/hda/hda_controller.c irqreturn_t azx_interrupt(int irq, void

Quoted 8 lines
sound/pci/hda/hda_controller.c +1 -0
 	struct azx *chip = dev_id;
 	struct hdac_bus *bus = azx_bus(chip);
 	u32 status;
+	bool handled = false;
 
 #ifdef CONFIG_PM
 	if (azx_has_pm_runtime(chip))

@@ -944,28 +945,35 @@ irqreturn_t azx_interrupt(int irq, void

*dev_id)

Quoted 46 lines
Quoted 46 lines
sound/pci/hda/hda_controller.c +21 -14
 		return IRQ_NONE;
 	}
 
-	status = azx_readl(chip, INTSTS);
-	if (status == 0 || status == 0xffffffff) {
-		spin_unlock(&bus->reg_lock);
-		return IRQ_NONE;
-	}
+	for (;;) {
+		bool active;
 
-	snd_hdac_bus_handle_stream_irq(bus, status, stream_update);
+		status = azx_readl(chip, INTSTS);
+		if (status == 0 || status == 0xffffffff)
+			break;
 
-	/* clear rirb int */
-	status = azx_readb(chip, RIRBSTS);
-	if (status & RIRB_INT_MASK) {
-		if (status & RIRB_INT_RESPONSE) {
-			if (chip->driver_caps &
 AZX_DCAPS_CTX_WORKAROUND)
-				udelay(80);
-			snd_hdac_bus_update_rirb(bus);
+		active = snd_hdac_bus_handle_stream_irq(bus, status,
 stream_update);
+
+		/* clear rirb int */
+		status = azx_readb(chip, RIRBSTS);
+		if (status & RIRB_INT_MASK) {
+			active = true;
+			if (status & RIRB_INT_RESPONSE) {
+				if (chip->driver_caps &
 AZX_DCAPS_CTX_WORKAROUND)
+					udelay(80);
+				snd_hdac_bus_update_rirb(bus);
+			}
+			azx_writeb(chip, RIRBSTS, RIRB_INT_MASK);
 		}
-		azx_writeb(chip, RIRBSTS, RIRB_INT_MASK);
+
+		if (!active)
+			break;
+		handled = true;
 	}
Quoted 8 lines
Quoted 7 lines
sound/pci/hda/hda_controller.c +1 -1
 	spin_unlock(&bus->reg_lock);
 
-	return IRQ_HANDLED;
+	return IRQ_RETVAL(handled);

}
EXPORT_SYMBOL_GPL(azx_interrupt);

_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

#5 lore raw
Quoted 31 lines · Libin
Quoted 16 lines

-----Original Message-----
From: [email protected] [mailto:alsa-devel- [email protected]] On Behalf Of Takashi Iwai
Sent: Tuesday, February 23, 2016 5:28 PM
To: Yang, Libin
Cc: Lin, Mengdong; [email protected]; alsa-devel@alsa- project.org
Subject: Re: [alsa-devel] [RFC: PATCH] ALSA: hda - clear IRQ statu twice on some Intel platforms

Quoted 3 lines · Libin

Hi Takashi,

Quoted 7 lines
Quoted 3 lines

I'm testing your patch in stress level. So far, it seems to works smoothly.

Which patch do you like? And your patch seems to impact on all the

platforms?

One potential problem in your patch is that the later triggered event shall be ignored by the post-irq ack.

Yes. So it seems your patch is better.

Quoted 5 lines

OTOH, my patch would impact to all platforms, yes. However, in theory, all platforms may have this issue, so it's anyway good in one side. In another flip, of course, it might introduce some regression.

Quoted 6 lines
Quoted 4 lines

We can limit this loop check only for some known platforms, or we let it tested more :)

I'm going to cook the patch a bit more and give you for testing.

I'd like to test your updated patch.

OK, below is the revised patch. Let me know if this works.

thanks,

Takashi

-- 8< --
From: Takashi Iwai <[email protected]>
Subject: [PATCH] ALSA: hda - Loop interrupt handling until really cleared

Currently the interrupt handler of HD-audio driver assumes that no irq update is needed while processing the irq. But in reality, it has been confirmed that the HW irq is issued even during the irq handling. Since we clear the irq status at the beginning, process the interrupt, then exits from the handler, the lately issued interrupt is left untouched without being properly processed.

This patch changes the interrupt handler code to loop over the check-and-process. The handler tries repeatedly as long as the IRQ status are turned on, and either stream or CORB/RIRB is handled.

For checking the stream handling, snd_hdac_bus_handle_stream_irq() returns a value indicating the stream indices bits. Other than that, the change is only in the irq handler itself.

Reported-by Libin Yang <[email protected]>
Cc <[email protected]>
Signed-off-by Takashi Iwai <[email protected]>
---
 include/sound/hdaudio.h        |  2 +-
 sound/hda/hdac_controller.c    |  7 ++++++-
 sound/pci/hda/hda_controller.c | 47 +++++++++++++++++++++++-------------------
 3 files changed, 33 insertions(+), 23 deletions(-)
include/sound/hdaudio.h +1 -1
index e2b712c90d3f..c21c38ce7450 100644
@@ -343,7 +343,7 @@ void snd_hdac_bus_enter_link_reset(struct hdac_bus *bus);
 void snd_hdac_bus_exit_link_reset(struct hdac_bus *bus);
 
 void snd_hdac_bus_update_rirb(struct hdac_bus *bus);
-void snd_hdac_bus_handle_stream_irq(struct hdac_bus *bus, unsigned int status,
+int snd_hdac_bus_handle_stream_irq(struct hdac_bus *bus, unsigned int status,
 				    void (*ack)(struct hdac_bus *,
 						struct hdac_stream *));
sound/hda/hdac_controller.c +6 -1
index b5a17cb510a0..8c486235c905 100644
@@ -426,18 +426,22 @@ EXPORT_SYMBOL_GPL(snd_hdac_bus_stop_chip);
  * @bus: HD-audio core bus
  * @status: INTSTS register value
  * @ask: callback to be called for woken streams
+ *
+ * Returns the bits of handled streams, or zero if no stream is handled.
  */
-void snd_hdac_bus_handle_stream_irq(struct hdac_bus *bus, unsigned int status,
+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 *azx_dev;
 	u8 sd_status;
+	int handled = 0;
 
 	list_for_each_entry(azx_dev, &bus->stream_list, list) {
 		if (status & azx_dev->sd_int_sta_mask) {
 			sd_status = snd_hdac_stream_readb(azx_dev, SD_STS);
 			snd_hdac_stream_writeb(azx_dev, SD_STS, SD_INT_MASK);
+			handled |= 1 << azx_dev->index;
 			if (!azx_dev->substream || !azx_dev->running ||
 			    !(sd_status & SD_INT_COMPLETE))
 				continue;

@@ -445,6 +449,7 @@ void snd_hdac_bus_handle_stream_irq(struct hdac_bus *bus, unsigned int status,
 				ack(bus, azx_dev);
 		}
 	}
+	return handled;
 }
 EXPORT_SYMBOL_GPL(snd_hdac_bus_handle_stream_irq);
sound/pci/hda/hda_controller.c +26 -21

65 lines not on this page · raw message

Signature · 1 line
2.7.1
#6 lore raw

Hi Takashi,

Our QA has tested your patch. It works and the basic test cases passed.

Regards,
Libin

Quoted 223 lines

-----Original Message-----
From: Takashi Iwai [mailto:[email protected]]
Sent: Tuesday, February 23, 2016 11:04 PM
To: Yang, Libin
Cc: Lin, Mengdong; [email protected]; alsa-devel@alsa- project.org
Subject: Re: [alsa-devel] [RFC: PATCH] ALSA: hda - clear IRQ statu twice on some Intel platforms

Quoted 9 lines · Libin
Quoted 9 lines

-----Original Message-----
From: [email protected] [mailto:alsa-devel- [email protected]] On Behalf Of Takashi Iwai
Sent: Tuesday, February 23, 2016 5:28 PM
To: Yang, Libin
Cc: Lin, Mengdong; [email protected]; alsa-devel@alsa- project.org
Subject: Re: [alsa-devel] [RFC: PATCH] ALSA: hda - clear IRQ statu

twice on

Quoted 8 lines
Quoted 8 lines

some Intel platforms

Quoted 4 lines · Libin

Hi Takashi,

I'm testing your patch in stress level. So far, it seems to works

smoothly.

Quoted 14 lines
Quoted 6 lines
Quoted 2 lines

Which patch do you like? And your patch seems to impact on all the

platforms?

One potential problem in your patch is that the later triggered event shall be ignored by the post-irq ack.

Yes. So it seems your patch is better.

Quoted 5 lines

OTOH, my patch would impact to all platforms, yes. However, in theory, all platforms may have this issue, so it's anyway good in one side. In another flip, of course, it might introduce some regression.

Quoted 6 lines
Quoted 4 lines

We can limit this loop check only for some known platforms, or we let it tested more :)

I'm going to cook the patch a bit more and give you for testing.

I'd like to test your updated patch.

OK, below is the revised patch. Let me know if this works.

thanks,

Takashi

-- 8< --
From: Takashi Iwai <[email protected]>
Subject: [PATCH] ALSA: hda - Loop interrupt handling until really cleared

Currently the interrupt handler of HD-audio driver assumes that no irq update is needed while processing the irq. But in reality, it has been confirmed that the HW irq is issued even during the irq handling. Since we clear the irq status at the beginning, process the interrupt, then exits from the handler, the lately issued interrupt is left untouched without being properly processed.

This patch changes the interrupt handler code to loop over the check-and-process. The handler tries repeatedly as long as the IRQ status are turned on, and either stream or CORB/RIRB is handled.

For checking the stream handling, snd_hdac_bus_handle_stream_irq() returns a value indicating the stream indices bits. Other than that, the change is only in the irq handler itself.

Reported-by Libin Yang <[email protected]>
Cc <[email protected]>
Signed-off-by Takashi Iwai <[email protected]>
---
 include/sound/hdaudio.h        |  2 +-
 sound/hda/hdac_controller.c    |  7 ++++++-
 sound/pci/hda/hda_controller.c | 47 +++++++++++++++++++++++-------
------------
 3 files changed, 33 insertions(+), 23 deletions(-)
include/sound/hdaudio.h +1 -1
index e2b712c90d3f..c21c38ce7450 100644
@@ -343,7 +343,7 @@ void snd_hdac_bus_enter_link_reset(struct
 hdac_bus *bus);
 void snd_hdac_bus_exit_link_reset(struct hdac_bus *bus);
 
 void snd_hdac_bus_update_rirb(struct hdac_bus *bus);
-void snd_hdac_bus_handle_stream_irq(struct hdac_bus *bus, unsigned
 int status,
+int snd_hdac_bus_handle_stream_irq(struct hdac_bus *bus, unsigned
 int status,
include/sound/hdaudio.h +0 -0
 				    void (*ack)(struct hdac_bus *,
 						struct hdac_stream *));
sound/hda/hdac_controller.c +4 -1
index b5a17cb510a0..8c486235c905 100644
@@ -426,18 +426,22 @@
 EXPORT_SYMBOL_GPL(snd_hdac_bus_stop_chip);
  * @bus: HD-audio core bus
  * @status: INTSTS register value
  * @ask: callback to be called for woken streams
+ *
+ * Returns the bits of handled streams, or zero if no stream is handled.
  */
-void snd_hdac_bus_handle_stream_irq(struct hdac_bus *bus, unsigned
 int status,
+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 *azx_dev;
 	u8 sd_status;
+	int handled = 0;
 
 	list_for_each_entry(azx_dev, &bus->stream_list, list) {
 		if (status & azx_dev->sd_int_sta_mask) {
 			sd_status = snd_hdac_stream_readb(azx_dev,
 SD_STS);
 			snd_hdac_stream_writeb(azx_dev, SD_STS,

SD_INT_MASK);

sound/hda/hdac_controller.c +1 -0
+			handled |= 1 << azx_dev->index;
 			if (!azx_dev->substream || !azx_dev->running

||

sound/hda/hdac_controller.c +1 -0
 			    !(sd_status & SD_INT_COMPLETE))
 				continue;

@@ -445,6 +449,7 @@ void snd_hdac_bus_handle_stream_irq(struct
 hdac_bus *bus, unsigned int status,
 				ack(bus, azx_dev);
 		}
 	}
+	return handled;
 }
 EXPORT_SYMBOL_GPL(snd_hdac_bus_handle_stream_irq);
sound/pci/hda/hda_controller.c +26 -21
index 37cf9cee9835..27de8015717d 100644
@@ -930,6 +930,8 @@ irqreturn_t azx_interrupt(int irq, void *dev_id)
 	struct azx *chip = dev_id;
 	struct hdac_bus *bus = azx_bus(chip);
 	u32 status;
+	bool active, handled = false;
+	int repeat = 0; /* count for avoiding endless loop */
 
 #ifdef CONFIG_PM
 	if (azx_has_pm_runtime(chip))

@@ -939,33 +941,36 @@ irqreturn_t azx_interrupt(int irq, void *dev_id)
 
 	spin_lock(&bus->reg_lock);
 
-	if (chip->disabled) {
-		spin_unlock(&bus->reg_lock);
-		return IRQ_NONE;
-	}
-
-	status = azx_readl(chip, INTSTS);
-	if (status == 0 || status == 0xffffffff) {
-		spin_unlock(&bus->reg_lock);
-		return IRQ_NONE;
-	}
+	if (chip->disabled)
+		goto unlock;
 
-	snd_hdac_bus_handle_stream_irq(bus, status, stream_update);
+	do {
+		status = azx_readl(chip, INTSTS);
+		if (status == 0 || status == 0xffffffff)
+			break;
 
-	/* clear rirb int */
-	status = azx_readb(chip, RIRBSTS);
-	if (status & RIRB_INT_MASK) {
-		if (status & RIRB_INT_RESPONSE) {
-			if (chip->driver_caps &
 AZX_DCAPS_CTX_WORKAROUND)
-				udelay(80);
-			snd_hdac_bus_update_rirb(bus);
+		handled = true;
+		active = false;
+		if (snd_hdac_bus_handle_stream_irq(bus, status,
 stream_update))
+			active = true;
+
+		/* clear rirb int */
+		status = azx_readb(chip, RIRBSTS);
+		if (status & RIRB_INT_MASK) {
+			active = true;
+			if (status & RIRB_INT_RESPONSE) {
+				if (chip->driver_caps &
 AZX_DCAPS_CTX_WORKAROUND)
+					udelay(80);
+				snd_hdac_bus_update_rirb(bus);
+			}
+			azx_writeb(chip, RIRBSTS, RIRB_INT_MASK);
 		}
-		azx_writeb(chip, RIRBSTS, RIRB_INT_MASK);
-	}
+	} while (active && ++repeat < 10);
 
+ unlock:
 	spin_unlock(&bus->reg_lock);
 
-	return IRQ_HANDLED;
+	return IRQ_RETVAL(handled);

}
EXPORT_SYMBOL_GPL(azx_interrupt);

--
2.7.1

#7 lore raw
Quoted 4 lines · Libin

Hi Takashi,

Our QA has tested your patch. It works and the basic test cases passed.

OK, I queued it up now.

thanks,

Takashi

Quoted 229 lines

Regards,
Libin

Quoted 223 lines

-----Original Message-----
From: Takashi Iwai [mailto:[email protected]]
Sent: Tuesday, February 23, 2016 11:04 PM
To: Yang, Libin
Cc: Lin, Mengdong; [email protected]; alsa-devel@alsa- project.org
Subject: Re: [alsa-devel] [RFC: PATCH] ALSA: hda - clear IRQ statu twice on some Intel platforms

Quoted 9 lines · Libin
Quoted 9 lines

-----Original Message-----
From: [email protected] [mailto:alsa-devel- [email protected]] On Behalf Of Takashi Iwai
Sent: Tuesday, February 23, 2016 5:28 PM
To: Yang, Libin
Cc: Lin, Mengdong; [email protected]; alsa-devel@alsa- project.org
Subject: Re: [alsa-devel] [RFC: PATCH] ALSA: hda - clear IRQ statu

twice on

Quoted 8 lines
Quoted 8 lines

some Intel platforms

Quoted 4 lines · Libin

Hi Takashi,

I'm testing your patch in stress level. So far, it seems to works

smoothly.

Quoted 14 lines
Quoted 6 lines
Quoted 2 lines

Which patch do you like? And your patch seems to impact on all the

platforms?

One potential problem in your patch is that the later triggered event shall be ignored by the post-irq ack.

Yes. So it seems your patch is better.

Quoted 5 lines

OTOH, my patch would impact to all platforms, yes. However, in theory, all platforms may have this issue, so it's anyway good in one side. In another flip, of course, it might introduce some regression.

Quoted 6 lines
Quoted 4 lines

We can limit this loop check only for some known platforms, or we let it tested more :)

I'm going to cook the patch a bit more and give you for testing.

I'd like to test your updated patch.

OK, below is the revised patch. Let me know if this works.

thanks,

Takashi

-- 8< --
From: Takashi Iwai <[email protected]>
Subject: [PATCH] ALSA: hda - Loop interrupt handling until really cleared

Currently the interrupt handler of HD-audio driver assumes that no irq update is needed while processing the irq. But in reality, it has been confirmed that the HW irq is issued even during the irq handling. Since we clear the irq status at the beginning, process the interrupt, then exits from the handler, the lately issued interrupt is left untouched without being properly processed.

This patch changes the interrupt handler code to loop over the check-and-process. The handler tries repeatedly as long as the IRQ status are turned on, and either stream or CORB/RIRB is handled.

For checking the stream handling, snd_hdac_bus_handle_stream_irq() returns a value indicating the stream indices bits. Other than that, the change is only in the irq handler itself.

Reported-by Libin Yang <[email protected]>
Cc <[email protected]>
Signed-off-by Takashi Iwai <[email protected]>
---
 include/sound/hdaudio.h        |  2 +-
 sound/hda/hdac_controller.c    |  7 ++++++-
 sound/pci/hda/hda_controller.c | 47 +++++++++++++++++++++++-------
------------
 3 files changed, 33 insertions(+), 23 deletions(-)
include/sound/hdaudio.h +1 -1
index e2b712c90d3f..c21c38ce7450 100644
@@ -343,7 +343,7 @@ void snd_hdac_bus_enter_link_reset(struct
 hdac_bus *bus);
 void snd_hdac_bus_exit_link_reset(struct hdac_bus *bus);
 
 void snd_hdac_bus_update_rirb(struct hdac_bus *bus);
-void snd_hdac_bus_handle_stream_irq(struct hdac_bus *bus, unsigned
 int status,
+int snd_hdac_bus_handle_stream_irq(struct hdac_bus *bus, unsigned
 int status,
include/sound/hdaudio.h +0 -0
 				    void (*ack)(struct hdac_bus *,
 						struct hdac_stream *));
sound/hda/hdac_controller.c +4 -1
index b5a17cb510a0..8c486235c905 100644
@@ -426,18 +426,22 @@
 EXPORT_SYMBOL_GPL(snd_hdac_bus_stop_chip);
  * @bus: HD-audio core bus
  * @status: INTSTS register value
  * @ask: callback to be called for woken streams
+ *
+ * Returns the bits of handled streams, or zero if no stream is handled.
  */
-void snd_hdac_bus_handle_stream_irq(struct hdac_bus *bus, unsigned
 int status,
+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 *azx_dev;
 	u8 sd_status;
+	int handled = 0;
 
 	list_for_each_entry(azx_dev, &bus->stream_list, list) {
 		if (status & azx_dev->sd_int_sta_mask) {
 			sd_status = snd_hdac_stream_readb(azx_dev,
 SD_STS);
 			snd_hdac_stream_writeb(azx_dev, SD_STS,

SD_INT_MASK);

sound/hda/hdac_controller.c +1 -0
+			handled |= 1 << azx_dev->index;
 			if (!azx_dev->substream || !azx_dev->running

||

sound/hda/hdac_controller.c +1 -0
 			    !(sd_status & SD_INT_COMPLETE))
 				continue;

@@ -445,6 +449,7 @@ void snd_hdac_bus_handle_stream_irq(struct
 hdac_bus *bus, unsigned int status,
 				ack(bus, azx_dev);
 		}
 	}
+	return handled;
 }
 EXPORT_SYMBOL_GPL(snd_hdac_bus_handle_stream_irq);
sound/pci/hda/hda_controller.c +26 -21
index 37cf9cee9835..27de8015717d 100644
@@ -930,6 +930,8 @@ irqreturn_t azx_interrupt(int irq, void *dev_id)
 	struct azx *chip = dev_id;
 	struct hdac_bus *bus = azx_bus(chip);
 	u32 status;
+	bool active, handled = false;
+	int repeat = 0; /* count for avoiding endless loop */
 
 #ifdef CONFIG_PM
 	if (azx_has_pm_runtime(chip))

@@ -939,33 +941,36 @@ irqreturn_t azx_interrupt(int irq, void *dev_id)
 
 	spin_lock(&bus->reg_lock);
 
-	if (chip->disabled) {
-		spin_unlock(&bus->reg_lock);
-		return IRQ_NONE;
-	}
-
-	status = azx_readl(chip, INTSTS);
-	if (status == 0 || status == 0xffffffff) {
-		spin_unlock(&bus->reg_lock);
-		return IRQ_NONE;
-	}
+	if (chip->disabled)
+		goto unlock;
 
-	snd_hdac_bus_handle_stream_irq(bus, status, stream_update);
+	do {
+		status = azx_readl(chip, INTSTS);
+		if (status == 0 || status == 0xffffffff)
+			break;
 
-	/* clear rirb int */
-	status = azx_readb(chip, RIRBSTS);
-	if (status & RIRB_INT_MASK) {
-		if (status & RIRB_INT_RESPONSE) {
-			if (chip->driver_caps &
 AZX_DCAPS_CTX_WORKAROUND)
-				udelay(80);
-			snd_hdac_bus_update_rirb(bus);
+		handled = true;
+		active = false;
+		if (snd_hdac_bus_handle_stream_irq(bus, status,
 stream_update))
+			active = true;
+
+		/* clear rirb int */
+		status = azx_readb(chip, RIRBSTS);
+		if (status & RIRB_INT_MASK) {
+			active = true;
+			if (status & RIRB_INT_RESPONSE) {
+				if (chip->driver_caps &
 AZX_DCAPS_CTX_WORKAROUND)
+					udelay(80);
+				snd_hdac_bus_update_rirb(bus);
+			}
+			azx_writeb(chip, RIRBSTS, RIRB_INT_MASK);
 		}
-		azx_writeb(chip, RIRBSTS, RIRB_INT_MASK);
-	}
+	} while (active && ++repeat < 10);
 
+ unlock:
 	spin_unlock(&bus->reg_lock);
 
-	return IRQ_HANDLED;
+	return IRQ_RETVAL(handled);

}
EXPORT_SYMBOL_GPL(azx_interrupt);

--
2.7.1