On Fri, 26 Feb 2016 06:22:34 +0100, Yang, Libin wrote:
Hi Takashi,
Our QA has tested your patch. It works and the basic test cases passed.
OK, I queued it up now.
thanks,
Takashi
Regards, Libin
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Tuesday, February 23, 2016 11:04 PM To: Yang, Libin Cc: Lin, Mengdong; libin.yang@linux.intel.com; alsa-devel@alsa- project.org Subject: Re: [alsa-devel] [RFC: PATCH] ALSA: hda - clear IRQ statu twice on some Intel platforms
On Tue, 23 Feb 2016 15:27:50 +0100, Yang, Libin wrote:
-----Original Message----- From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel- bounces@alsa-project.org] On Behalf Of Takashi Iwai Sent: Tuesday, February 23, 2016 5:28 PM To: Yang, Libin Cc: Lin, Mengdong; libin.yang@linux.intel.com; alsa-devel@alsa- project.org Subject: Re: [alsa-devel] [RFC: PATCH] ALSA: hda - clear IRQ statu
twice on
some Intel platforms
On Tue, 23 Feb 2016 09:57:13 +0100, Yang, Libin wrote:
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.
Yes. So it seems your patch is better.
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.
OK, below is the revised patch. Let me know if this works.
thanks,
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de 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 libin.yang@linux.intel.com Cc: stable@vger.kernel.org Signed-off-by: Takashi Iwai tiwai@suse.de
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(-)
diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h index e2b712c90d3f..c21c38ce7450 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, +int 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..8c486235c905 100644 --- a/sound/hda/hdac_controller.c +++ b/sound/hda/hdac_controller.c @@ -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);
diff --git a/sound/pci/hda/hda_controller.c b/sound/pci/hda/hda_controller.c index 37cf9cee9835..27de8015717d 100644 --- a/sound/pci/hda/hda_controller.c +++ b/sound/pci/hda/hda_controller.c @@ -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