Re: [RFC: PATCH] ALSA: hda - clear IRQ statu twice on some Intel platforms

[Takashi Iwai <tiwai@suse.de>]

On Mon, 22 Feb 2016 08:39:50 +0100,
libin.yang@linux.intel.com wrote:
> 
> From: Libin Yang <libin.yang@linux.intel.com>
> 
> 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 <libin.yang@linux.intel.com>

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

---
diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
index e2b712c90d3f..b74d9c8dceff 100644
--- a/include/sound/hdaudio.h
+++ b/include/sound/hdaudio.h
@@ -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 *));
 
diff --git a/sound/hda/hdac_controller.c b/sound/hda/hdac_controller.c
index b5a17cb510a0..448cd8c990f0 100644
--- a/sound/hda/hdac_controller.c
+++ b/sound/hda/hdac_controller.c
@@ -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);
 
diff --git a/sound/pci/hda/hda_controller.c b/sound/pci/hda/hda_controller.c
index 37cf9cee9835..a4226026b1dc 100644
--- a/sound/pci/hda/hda_controller.c
+++ b/sound/pci/hda/hda_controller.c
@@ -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);