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 > -----Original Message----- > From: Takashi Iwai [mailto:tiwai@xxxxxxx] > Sent: Monday, February 22, 2016 4:44 PM > To: libin.yang@xxxxxxxxxxxxxxx > Cc: alsa-devel@xxxxxxxxxxxxxxxx; Lin, Mengdong; Yang, Libin > Subject: Re: [RFC: PATCH] ALSA: hda - clear IRQ statu twice on > some Intel platforms > > On Mon, 22 Feb 2016 08:39:50 +0100, > libin.yang@xxxxxxxxxxxxxxx wrote: > > > > From: Libin Yang <libin.yang@xxxxxxxxxxxxxxx> > > > > 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@xxxxxxxxxxxxxxx> > > 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); > _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
