From owner-svn-src-all@freebsd.org Thu Jun 18 06:12:06 2020 Return-Path: Delivered-To: svn-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id E65823450C7; Thu, 18 Jun 2020 06:12:06 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 49nWmk5vFhz48VF; Thu, 18 Jun 2020 06:12:06 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id C58112C999; Thu, 18 Jun 2020 06:12:06 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 05I6C6tJ013038; Thu, 18 Jun 2020 06:12:06 GMT (envelope-from avg@FreeBSD.org) Received: (from avg@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 05I6C6EE013037; Thu, 18 Jun 2020 06:12:06 GMT (envelope-from avg@FreeBSD.org) Message-Id: <202006180612.05I6C6EE013037@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: avg set sender to avg@FreeBSD.org using -f From: Andriy Gapon Date: Thu, 18 Jun 2020 06:12:06 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r362294 - head/sys/dev/sound/pci/hda X-SVN-Group: head X-SVN-Commit-Author: avg X-SVN-Commit-Paths: head/sys/dev/sound/pci/hda X-SVN-Commit-Revision: 362294 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 18 Jun 2020 06:12:07 -0000 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: mem 0xfe680000-0xfe687fff at device 0.6 on pci4 hdacc1: 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