Author: avg Date: Thu Jun 18 06:12:06 2020 New Revision: 362294 URL: https://svnweb.freebsd.org/changeset/base/362294
Log: hdac_intr_handler: keep working until global interrupt status clears It is plausible that the hardware interrupts a host only when GIS goes from zero to one. GIS is formed by OR-ing multiple hardware statuses, so it's possible that a previously cleared status gets set again while another status has not been cleared yet. Thus, there will be no new interrupt as GIS always stayed set. If we don't re-examine GIS then we can leave it set and never get another interrupt again. Without this change I frequently saw a problem where snd_hda would stop working. Setting dev.hdac.1.polling=1 would bring it back to life and afterwards I could set polling back to zero. Sometimes the problem started right after a boot, sometimes it happened after resuming from S3, frequently it would occur when sound output and input are active concurrently (such as during conferencing). I looked at HDAC_INTSTS while the sound was not working and I saw that both HDAC_INTSTS_GIS and HDAC_INTSTS_CIS were set, but there were no interrupts. I have collected some statistics over a period of several days about how many loops (calls to hdac_one_intr) the new code did for a single interrupt: +--------+--------------+ |Loops |Times Happened| +--------+--------------+ |0 |301 | |1 |12857746 | |2 |280 | |3 |2 | |4+ |0 | +--------+--------------+ I believe that previously the sound would get stuck each time we had to loop more than once. The tested hardware is: hdac1: <AMD (0x15e3) HDA Controller> mem 0xfe680000-0xfe687fff at device 0.6 on pci4 hdacc1: <Realtek ALC269 HDA CODEC> at cad 0 on hdac1 No objections: mav MFC after: 5 weeks Differential Revision: https://reviews.freebsd.org/D25128 Modified: head/sys/dev/sound/pci/hda/hdac.c Modified: head/sys/dev/sound/pci/hda/hdac.c ============================================================================== --- head/sys/dev/sound/pci/hda/hdac.c Wed Jun 17 23:41:04 2020 (r362293) +++ head/sys/dev/sound/pci/hda/hdac.c Thu Jun 18 06:12:06 2020 (r362294) @@ -303,30 +303,13 @@ hdac_config_fetch(struct hdac_softc *sc, uint32_t *on, } } -/**************************************************************************** - * void hdac_intr_handler(void *) - * - * Interrupt handler. Processes interrupts received from the hdac. - ****************************************************************************/ static void -hdac_intr_handler(void *context) +hdac_one_intr(struct hdac_softc *sc, uint32_t intsts) { - struct hdac_softc *sc; device_t dev; - uint32_t intsts; uint8_t rirbsts; int i; - sc = (struct hdac_softc *)context; - hdac_lock(sc); - - /* Do we have anything to do? */ - intsts = HDAC_READ_4(&sc->mem, HDAC_INTSTS); - if ((intsts & HDAC_INTSTS_GIS) == 0) { - hdac_unlock(sc); - return; - } - /* Was this a controller interrupt? */ if (intsts & HDAC_INTSTS_CIS) { rirbsts = HDAC_READ_1(&sc->mem, HDAC_RIRBSTS); @@ -346,16 +329,45 @@ hdac_intr_handler(void *context) if ((intsts & (1 << i)) == 0) continue; HDAC_WRITE_1(&sc->mem, (i << 5) + HDAC_SDSTS, - HDAC_SDSTS_DESE | HDAC_SDSTS_FIFOE | HDAC_SDSTS_BCIS ); + HDAC_SDSTS_DESE | HDAC_SDSTS_FIFOE | HDAC_SDSTS_BCIS); if ((dev = sc->streams[i].dev) != NULL) { HDAC_STREAM_INTR(dev, sc->streams[i].dir, sc->streams[i].stream); } } } +} - HDAC_WRITE_4(&sc->mem, HDAC_INTSTS, intsts); - hdac_unlock(sc); +/**************************************************************************** + * void hdac_intr_handler(void *) + * + * Interrupt handler. Processes interrupts received from the hdac. + ****************************************************************************/ +static void +hdac_intr_handler(void *context) +{ + struct hdac_softc *sc; + uint32_t intsts; + + sc = (struct hdac_softc *)context; + + /* + * Loop until HDAC_INTSTS_GIS gets clear. + * It is plausible that hardware interrupts a host only when GIS goes + * from zero to one. GIS is formed by OR-ing multiple hardware + * statuses, so it's possible that a previously cleared status gets set + * again while another status has not been cleared yet. Thus, there + * will be no new interrupt as GIS always stayed set. If we don't + * re-examine GIS then we can leave it set and never get an interrupt + * again. + */ + intsts = HDAC_READ_4(&sc->mem, HDAC_INTSTS); + while ((intsts & HDAC_INTSTS_GIS) != 0) { + hdac_lock(sc); + hdac_one_intr(sc, intsts); + hdac_unlock(sc); + intsts = HDAC_READ_4(&sc->mem, HDAC_INTSTS); + } } static void _______________________________________________ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
