svn commit: r362647 - head/sys/dev/sound/pci/hda

Andriy Gapon avg at FreeBSD.org
Fri Jun 26 09:46:04 UTC 2020


Author: avg
Date: Fri Jun 26 09:46:03 2020
New Revision: 362647
URL: https://svnweb.freebsd.org/changeset/base/362647

Log:
  sound/hda: fix interrupt handler endless loop after r362294
  
  Not all interrupt sources that affect CIS bit were acknowledged.
  Specifically, bits in STATESTS (aka WAKESTS) were left set.
  
  The fix is to disable WAKEEN and clear STATESTS bits before the HDA
  interrupt is enabled.  This way we should never get any STATESTS bits.
  
  I also added placeholders for all event bits that we currently do not
  enable, do not handle and do not clear.  This might get useful when / if
  we enable any of them.
  
  Reported by:	kib (Apollo Lake hardware)
  Tested by:	kib (earlier, different change)
  MFC after:	2 weeks
  X-MFC with:	r362294

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	Fri Jun 26 09:39:23 2020	(r362646)
+++ head/sys/dev/sound/pci/hda/hdac.c	Fri Jun 26 09:46:03 2020	(r362647)
@@ -312,8 +312,27 @@ hdac_one_intr(struct hdac_softc *sc, uint32_t intsts)
 
 	/* Was this a controller interrupt? */
 	if (intsts & HDAC_INTSTS_CIS) {
-		rirbsts = HDAC_READ_1(&sc->mem, HDAC_RIRBSTS);
+		/*
+		 * Placeholder: if we ever enable any bits in HDAC_WAKEEN, then
+		 * we will need to check and clear HDAC_STATESTS.
+		 * That event is used to report codec status changes such as
+		 * a reset or a wake-up event.
+		 */
+		/*
+		 * Placeholder: if we ever enable HDAC_CORBCTL_CMEIE, then we
+		 * will need to check and clear HDAC_CORBSTS_CMEI in
+		 * HDAC_CORBSTS.
+		 * That event is used to report CORB memory errors.
+		 */
+		/*
+		 * Placeholder: if we ever enable HDAC_RIRBCTL_RIRBOIC, then we
+		 * will need to check and clear HDAC_RIRBSTS_RIRBOIS in
+		 * HDAC_RIRBSTS.
+		 * That event is used to report response FIFO overruns.
+		 */
+
 		/* Get as many responses that we can */
+		rirbsts = HDAC_READ_1(&sc->mem, HDAC_RIRBSTS);
 		while (rirbsts & HDAC_RIRBSTS_RINTFL) {
 			HDAC_WRITE_1(&sc->mem,
 			    HDAC_RIRBSTS, HDAC_RIRBSTS_RINTFL);
@@ -1514,6 +1533,24 @@ hdac_attach2(void *arg)
 		device_printf(sc->dev, "Starting RIRB Engine...\n");
 	);
 	hdac_rirb_start(sc);
+
+	/*
+	 * Clear HDAC_WAKEEN as at present we have no use for SDI wake
+	 * (status change) interrupts.  The documentation says that we
+	 * should not make any assumptions about the state of this register
+	 * and set it explicitly.
+	 * NB: this needs to be done before the interrupt is enabled as
+	 * the handler does not expect this interrupt source.
+	 */
+	HDAC_WRITE_2(&sc->mem, HDAC_WAKEEN, 0);
+
+	/*
+	 * Read and clear post-reset SDI wake status.
+	 * Each set bit corresponds to a codec that came out of reset.
+	 */
+	statests = HDAC_READ_2(&sc->mem, HDAC_STATESTS);
+	HDAC_WRITE_2(&sc->mem, HDAC_STATESTS, statests);
+
 	HDA_BOOTHVERBOSE(
 		device_printf(sc->dev,
 		    "Enabling controller interrupt...\n");
@@ -1529,7 +1566,6 @@ hdac_attach2(void *arg)
 	HDA_BOOTHVERBOSE(
 		device_printf(sc->dev, "Scanning HDA codecs ...\n");
 	);
-	statests = HDAC_READ_2(&sc->mem, HDAC_STATESTS);
 	hdac_unlock(sc);
 	for (i = 0; i < HDAC_CODEC_MAX; i++) {
 		if (HDAC_STATESTS_SDIWAKE(statests, i)) {
@@ -1643,6 +1679,19 @@ hdac_resume(device_t dev)
 		device_printf(dev, "Starting RIRB Engine...\n");
 	);
 	hdac_rirb_start(sc);
+
+	/*
+	 * Clear HDAC_WAKEEN as at present we have no use for SDI wake
+	 * (status change) events.  The documentation says that we should
+	 * not make any assumptions about the state of this register and
+	 * set it explicitly.
+	 * Also, clear HDAC_STATESTS.
+	 * NB: this needs to be done before the interrupt is enabled as
+	 * the handler does not expect this interrupt source.
+	 */
+	HDAC_WRITE_2(&sc->mem, HDAC_WAKEEN, 0);
+	HDAC_WRITE_2(&sc->mem, HDAC_STATESTS, HDAC_STATESTS_SDIWAKE_MASK);
+
 	HDA_BOOTHVERBOSE(
 		device_printf(dev, "Enabling controller interrupt...\n");
 	);


More information about the svn-src-head mailing list