d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

hdac_intr_handler: keep working until global interrupt status clears

Browse Source 
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
This commit is contained in:
Andriy Gapon 2020-06-18 06:12:06 +00:00
parent d9a8abf6c2
commit 4c7d1ab06d
1 changed files with 33 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

54
sys/dev/sound/pci/hda/hdac.c
View File

@ -303,30 +303,13 @@ hdac_config_fetch(struct hdac_softc *sc, uint32_t *on, uint32_t *off)
}
}
/****************************************************************************
* 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