From ba06b06923e1b2b9090417de4c1e122a438ae0fe Mon Sep 17 00:00:00 2001 From: Kai Vehmanen Date: Wed, 5 Jun 2019 13:39:31 +0300 Subject: [PATCH 1/3] ASoC: SOF: intel: handle non-stream irqs correctly The HDA interrupt handler incorrectly returns IRQ_NONE in case the Stream Interrupt Status bits are all zeroes. This is incorrect as the controller interrupt can be raised to signal various other events such as Response Interrupt, SDI State Change among others. Signed-off-by: Kai Vehmanen --- sound/soc/sof/intel/hda-stream.c | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/sound/soc/sof/intel/hda-stream.c b/sound/soc/sof/intel/hda-stream.c index ddfd80b44e6e8c..1a915a310bfcc3 100644 --- a/sound/soc/sof/intel/hda-stream.c +++ b/sound/soc/sof/intel/hda-stream.c @@ -462,36 +462,42 @@ irqreturn_t hda_dsp_stream_interrupt(int irq, void *context) { struct hdac_bus *bus = context; struct sof_intel_hda_dev *sof_hda = bus_to_sof_hda(bus); - u32 stream_mask; + int ret = IRQ_NONE; + u32 rirb_status; u32 status; if (!pm_runtime_active(bus->dev)) - return IRQ_NONE; + return ret; spin_lock(&bus->reg_lock); status = snd_hdac_chip_readl(bus, INTSTS); - stream_mask = GENMASK(sof_hda->stream_max - 1, 0) | AZX_INT_CTRL_EN; - /* Not stream interrupt or register inaccessible, ignore it.*/ - if (!(status & stream_mask) || status == 0xffffffff) { + /* Register inaccessible, ignore it.*/ + if (status == 0xffffffff) { spin_unlock(&bus->reg_lock); - return IRQ_NONE; + return ret; } #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) - /* clear rirb int */ - status = snd_hdac_chip_readb(bus, RIRBSTS); - if (status & RIRB_INT_MASK) { - if (status & RIRB_INT_RESPONSE) - snd_hdac_bus_update_rirb(bus); - snd_hdac_chip_writeb(bus, RIRBSTS, RIRB_INT_MASK); + if (status & AZX_INT_CTRL_EN) { + /* clear rirb int */ + rirb_status = snd_hdac_chip_readb(bus, RIRBSTS); + if (rirb_status & RIRB_INT_MASK) { + if (rirb_status & RIRB_INT_RESPONSE) + snd_hdac_bus_update_rirb(bus); + snd_hdac_chip_writeb(bus, RIRBSTS, RIRB_INT_MASK); + } + ret = IRQ_HANDLED; } #endif + if (status & GENMASK(sof_hda->stream_max - 1, 0)) + ret = IRQ_WAKE_THREAD; + spin_unlock(&bus->reg_lock); - return snd_hdac_chip_readl(bus, INTSTS) ? IRQ_WAKE_THREAD : IRQ_HANDLED; + return ret; } irqreturn_t hda_dsp_stream_threaded_handler(int irq, void *context) From 5bb53dec726fe647cfbb2600986095bcdd8056e9 Mon Sep 17 00:00:00 2001 From: Kai Vehmanen Date: Wed, 5 Jun 2019 21:09:25 +0300 Subject: [PATCH 2/3] ASoC: SOF: intel: address missed RIRB events in irq handler It is possible to get interrupts from the HDA controller even during suspend process. If RIRB events are not acked properly, this will block other interrupts and e.g. block the CTX_SAVE IPC that is used as part of run runtime suspend process. Remove the check for device runtime state and process irqs whenever the INTSTS and RIRBSTS registers are set. Signed-off-by: Kai Vehmanen --- sound/soc/sof/intel/hda-stream.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/sound/soc/sof/intel/hda-stream.c b/sound/soc/sof/intel/hda-stream.c index 1a915a310bfcc3..3f2a81976f2ec9 100644 --- a/sound/soc/sof/intel/hda-stream.c +++ b/sound/soc/sof/intel/hda-stream.c @@ -466,9 +466,6 @@ irqreturn_t hda_dsp_stream_interrupt(int irq, void *context) u32 rirb_status; u32 status; - if (!pm_runtime_active(bus->dev)) - return ret; - spin_lock(&bus->reg_lock); status = snd_hdac_chip_readl(bus, INTSTS); From 1848733905308a5f649e86237156a09bd94fbc00 Mon Sep 17 00:00:00 2001 From: Kai Vehmanen Date: Thu, 6 Jun 2019 13:33:15 +0300 Subject: [PATCH 3/3] ASoC: SOF: intel: move RIRBSTS handling to irq thread To avoid issue with 1st level interrupt bits (like RIRBSTS) being raised while irq handlers (AudioHDA and AudioDSP) are executing, mask the 2nd level CTRL_EN/CIS interrupt in hda-stream hard handler and handle all work in the hda-stream thread. In the irq thread, unmask CTRL_EN/CIS and handle all work as reported by 1st level bits. Clearing the bits may cause a new irq to be triggered, but this is ok as we clear the bits in the irq thread. Signed-off-by: Kai Vehmanen Co-developed-by: Libin Yang Co-developed-by: Ranjani Sridharan --- sound/soc/sof/intel/hda-stream.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/sound/soc/sof/intel/hda-stream.c b/sound/soc/sof/intel/hda-stream.c index 3f2a81976f2ec9..acdac79484edfb 100644 --- a/sound/soc/sof/intel/hda-stream.c +++ b/sound/soc/sof/intel/hda-stream.c @@ -463,7 +463,6 @@ irqreturn_t hda_dsp_stream_interrupt(int irq, void *context) struct hdac_bus *bus = context; struct sof_intel_hda_dev *sof_hda = bus_to_sof_hda(bus); int ret = IRQ_NONE; - u32 rirb_status; u32 status; spin_lock(&bus->reg_lock); @@ -478,14 +477,8 @@ irqreturn_t hda_dsp_stream_interrupt(int irq, void *context) #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) if (status & AZX_INT_CTRL_EN) { - /* clear rirb int */ - rirb_status = snd_hdac_chip_readb(bus, RIRBSTS); - if (rirb_status & RIRB_INT_MASK) { - if (rirb_status & RIRB_INT_RESPONSE) - snd_hdac_bus_update_rirb(bus); - snd_hdac_chip_writeb(bus, RIRBSTS, RIRB_INT_MASK); - } - ret = IRQ_HANDLED; + snd_hdac_chip_updatel(bus, INTCTL, AZX_INT_CTRL_EN, 0); + ret = IRQ_WAKE_THREAD; } #endif @@ -503,6 +496,7 @@ irqreturn_t hda_dsp_stream_threaded_handler(int irq, void *context) struct sof_intel_hda_dev *sof_hda = bus_to_sof_hda(bus); u32 status = snd_hdac_chip_readl(bus, INTSTS); struct hdac_stream *s; + u32 rirb_status; u32 sd_status; /* check streams */ @@ -527,6 +521,17 @@ irqreturn_t hda_dsp_stream_threaded_handler(int irq, void *context) } } + /* unmask 2nd level CTRL_EN (CIS) before clearing 1st level RIRBSTS */ + snd_hdac_chip_updatel(bus, INTCTL, AZX_INT_CTRL_EN, 1); + + /* clear rirb int */ + rirb_status = snd_hdac_chip_readb(bus, RIRBSTS); + if (rirb_status & RIRB_INT_MASK) { + if (rirb_status & RIRB_INT_RESPONSE) + snd_hdac_bus_update_rirb(bus); + snd_hdac_chip_writeb(bus, RIRBSTS, RIRB_INT_MASK); + } + return IRQ_HANDLED; }