From owner-freebsd-current@FreeBSD.ORG Tue Oct 28 11:26:21 2003 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id BC1E016A4CE for ; Tue, 28 Oct 2003 11:26:21 -0800 (PST) Received: from hak.cnd.mcgill.ca (hak.cnd.mcgill.ca [132.216.11.133]) by mx1.FreeBSD.org (Postfix) with ESMTP id 80E2A43F75 for ; Tue, 28 Oct 2003 11:26:18 -0800 (PST) (envelope-from mat@hak.cnd.mcgill.ca) Received: from hak.cnd.mcgill.ca (localhost [127.0.0.1]) by hak.cnd.mcgill.ca (8.12.9/8.12.8) with ESMTP id h9SJOSk4008118 for ; Tue, 28 Oct 2003 14:24:28 -0500 (EST) (envelope-from mat@hak.cnd.mcgill.ca) Received: (from mat@localhost) by hak.cnd.mcgill.ca (8.12.9/8.12.8/Submit) id h9SJOSms008117 for freebsd-current@freebsd.org; Tue, 28 Oct 2003 14:24:28 -0500 (EST) Date: Tue, 28 Oct 2003 14:24:28 -0500 From: Mathew Kanner To: freebsd-current@freebsd.org Message-ID: <20031028192428.GN31273@cnd.mcgill.ca> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="mR8QP4gmHujQHb1c" Content-Disposition: inline User-Agent: Mutt/1.4.1i Organization: I speak for myself, operating in Montreal, CANADA X-Spam-Status: No, hits=-3.0 required=5.0 tests=PATCH_UNIFIED_DIFF,USER_AGENT_MUTT version=2.55 X-Spam-Checker-Version: SpamAssassin 2.55 (1.174.2.19-2003-05-19-exp) Subject: sound LOR patches X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 28 Oct 2003 19:26:21 -0000 --mR8QP4gmHujQHb1c Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hello All, I tried to fix some LOR in -current and attached you will find some patches. I sent these to the -sound list but I didn't get a response. (Maybe I should mention that I'm also part of the -sound list). So now I don't know what's going in with sound and -current. Anyway, the fixes are: dsp_open: rearrange to only hold one lock at a time dsp_close: ditto mixer_hwvol_init: delete locking, the only consumer seems to be the ess driver and it only call it a creation time, I think the device will be stable across the sleepable malloc in the dyn. sysctl allocation cmi interrupt routine: Release locks while caller chn_intr, We could either this or do what emu10k1 does which is have no locks at in the interrupt handler. Cheers, --Mat -- I don't even know what street Canada is on. - Al Capone --mR8QP4gmHujQHb1c Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="dspopen_lor.patch" --- dspold.c Sun Sep 14 17:49:38 2003 +++ dsp.c Tue Oct 21 14:38:44 2003 @@ -174,6 +174,8 @@ intrmask_t s; u_int32_t fmt; int devtype; + int rdref; + int error; s = spltty(); d = dsp_get_info(i_dev); @@ -209,6 +211,8 @@ panic("impossible devtype %d", devtype); } + rdref = 0; + /* lock snddev so nobody else can monkey with it */ pcm_lock(d); @@ -251,67 +255,66 @@ return EBUSY; } /* got a channel, already locked for us */ - } - - if (flags & FWRITE) { - /* open for write */ - wrch = pcm_chnalloc(d, PCMDIR_PLAY, td->td_proc->p_pid, -1); - if (!wrch) { - /* no channel available */ - if (flags & FREAD) { - /* just opened a read channel, release it */ - pcm_chnrelease(rdch); - } - /* exit */ - pcm_unlock(d); - splx(s); - return EBUSY; - } - /* got a channel, already locked for us */ - } - - i_dev->si_drv1 = rdch; - i_dev->si_drv2 = wrch; - - /* Bump refcounts, reset and unlock any channels that we just opened, - * and then release device lock. - */ - if (flags & FREAD) { if (chn_reset(rdch, fmt)) { pcm_chnrelease(rdch); i_dev->si_drv1 = NULL; - if (wrch && (flags & FWRITE)) { - pcm_chnrelease(wrch); - i_dev->si_drv2 = NULL; - } pcm_unlock(d); splx(s); return ENODEV; } + if (flags & O_NONBLOCK) rdch->flags |= CHN_F_NBIO; pcm_chnref(rdch, 1); CHN_UNLOCK(rdch); + rdref = 1; + /* + * Record channel created, ref'ed and unlocked + */ } + if (flags & FWRITE) { - if (chn_reset(wrch, fmt)) { - pcm_chnrelease(wrch); - i_dev->si_drv2 = NULL; - if (flags & FREAD) { - CHN_LOCK(rdch); - pcm_chnref(rdch, -1); - pcm_chnrelease(rdch); - i_dev->si_drv1 = NULL; - } - pcm_unlock(d); - splx(s); - return ENODEV; + /* open for write */ + wrch = pcm_chnalloc(d, PCMDIR_PLAY, td->td_proc->p_pid, -1); + error = 0; + + if (!wrch) + error = EBUSY; /* XXX Right return code? */ + else if (chn_reset(wrch, fmt)) + error = ENODEV; + + if (error != 0) { + if (wrch) { + /* + * Free play channel + */ + pcm_chnrelease(wrch); + i_dev->si_drv2 = NULL; } - if (flags & O_NONBLOCK) - wrch->flags |= CHN_F_NBIO; - pcm_chnref(wrch, 1); - CHN_UNLOCK(wrch); + if (rdref) { + /* + * Lock, deref and release previously created record channel + */ + CHN_LOCK(rdch); + pcm_chnref(rdch, -1); + pcm_chnrelease(rdch); + i_dev->si_drv1 = NULL; + } + + pcm_unlock(d); + splx(s); + return error; + } + + if (flags & O_NONBLOCK) + wrch->flags |= CHN_F_NBIO; + pcm_chnref(wrch, 1); + CHN_UNLOCK(wrch); } + + i_dev->si_drv1 = rdch; + i_dev->si_drv2 = wrch; + pcm_unlock(d); splx(s); return 0; --mR8QP4gmHujQHb1c Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="dspclose_lor.patch" --- /home/mat/dsp.c.1.66 Fri Oct 24 18:07:23 2003 +++ /usr/src/sys/dev/sound/pcm/dsp.c Sat Oct 25 00:24:49 2003 @@ -321,7 +321,7 @@ struct pcm_channel *rdch, *wrch; struct snddev_info *d; intrmask_t s; - int exit; + int refs; s = spltty(); d = dsp_get_info(i_dev); @@ -329,53 +329,57 @@ rdch = i_dev->si_drv1; wrch = i_dev->si_drv2; - exit = 0; + refs = 0; - /* decrement refcount for each channel, exit if nonzero */ if (rdch) { CHN_LOCK(rdch); - if (pcm_chnref(rdch, -1) > 0) { - CHN_UNLOCK(rdch); - exit = 1; - } + refs += pcm_chnref(rdch, -1); + CHN_UNLOCK(rdch); } if (wrch) { CHN_LOCK(wrch); - if (pcm_chnref(wrch, -1) > 0) { - CHN_UNLOCK(wrch); - exit = 1; - } - } - if (exit) { - pcm_unlock(d); - splx(s); - return 0; + refs += pcm_chnref(wrch, -1); + CHN_UNLOCK(wrch); } - /* both refcounts are zero, abort and release */ + /* + * If there are no more references, release the channels. + */ + if ((rdch || wrch) && refs == 0) { - if (pcm_getfakechan(d)) - pcm_getfakechan(d)->flags = 0; + if (pcm_getfakechan(d)) + pcm_getfakechan(d)->flags = 0; - i_dev->si_drv1 = NULL; - i_dev->si_drv2 = NULL; + i_dev->si_drv1 = NULL; + i_dev->si_drv2 = NULL; - dsp_set_flags(i_dev, dsp_get_flags(i_dev) & ~SD_F_TRANSIENT); - pcm_unlock(d); + dsp_set_flags(i_dev, dsp_get_flags(i_dev) & ~SD_F_TRANSIENT); - if (rdch) { - chn_abort(rdch); /* won't sleep */ - rdch->flags &= ~(CHN_F_RUNNING | CHN_F_MAPPED | CHN_F_DEAD); - chn_reset(rdch, 0); - pcm_chnrelease(rdch); - } - if (wrch) { - chn_flush(wrch); /* may sleep */ - wrch->flags &= ~(CHN_F_RUNNING | CHN_F_MAPPED | CHN_F_DEAD); - chn_reset(wrch, 0); - pcm_chnrelease(wrch); - } + pcm_unlock(d); + if (rdch) { + CHN_LOCK(rdch); + chn_abort(rdch); /* won't sleep */ + rdch->flags &= ~(CHN_F_RUNNING | CHN_F_MAPPED | CHN_F_DEAD); + chn_reset(rdch, 0); + pcm_chnrelease(rdch); + } + if (wrch) { + CHN_LOCK(wrch); + /* + * XXX: Maybe the right behaviour is to abort on non_block. + * It seems that mplayer flushes the audio queue by quickly + * closing and re-opening. In FBSD, there's a long pause + * while the audio queue flushes that I presume isn't there in + * linux. + */ + chn_flush(wrch); /* may sleep */ + wrch->flags &= ~(CHN_F_RUNNING | CHN_F_MAPPED | CHN_F_DEAD); + chn_reset(wrch, 0); + pcm_chnrelease(wrch); + } + } else + pcm_unlock(d); splx(s); return 0; } --mR8QP4gmHujQHb1c Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="mixerhwvol.patch" --- mixer.c.old Sat Oct 25 00:29:26 2003 +++ mixer.c Sat Oct 25 00:30:06 2003 @@ -319,7 +319,6 @@ pdev = mixer_get_devt(dev); m = pdev->si_drv1; - snd_mtxlock(m->lock); m->hwvol_mixer = SOUND_MIXER_VOLUME; m->hwvol_step = 5; @@ -330,7 +329,6 @@ OID_AUTO, "hwvol_mixer", CTLTYPE_STRING | CTLFLAG_RW, m, 0, sysctl_hw_snd_hwvol_mixer, "A", ""); #endif - snd_mtxunlock(m->lock); return 0; } --mR8QP4gmHujQHb1c Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="cmi_intr.patch" --- /home/mat/cmi.c.1.23 Sat Oct 25 00:22:09 2003 +++ /usr/src/sys/dev/sound/pci/cmi.c Fri Oct 24 23:49:03 2003 @@ -51,7 +51,7 @@ #include "mixer_if.h" -SND_DECLARE_FILE("$FreeBSD: /repoman/r/ncvs/src/sys/dev/sound/pci/cmi.c,v 1.23 2003/09/02 17:30:37 jhb Exp $"); +SND_DECLARE_FILE("$FreeBSD: src/sys/dev/sound/pci/cmi.c,v 1.23 2003/09/02 17:30:37 jhb Exp $"); /* Supported chip ID's */ #define CMI8338A_PCI_ID 0x010013f6 @@ -516,41 +516,41 @@ { struct sc_info *sc = data; u_int32_t intrstat; + u_int32_t toclear; snd_mtxlock(sc->lock); intrstat = cmi_rd(sc, CMPCI_REG_INTR_STATUS, 4); - if ((intrstat & CMPCI_REG_ANY_INTR) == 0) { - goto out; - } - - /* Disable interrupts */ - if (intrstat & CMPCI_REG_CH0_INTR) { - cmi_clr4(sc, CMPCI_REG_INTR_CTRL, CMPCI_REG_CH0_INTR_ENABLE); - } + if ((intrstat & CMPCI_REG_ANY_INTR) != 0) { - if (intrstat & CMPCI_REG_CH1_INTR) { - cmi_clr4(sc, CMPCI_REG_INTR_CTRL, CMPCI_REG_CH1_INTR_ENABLE); - } + toclear = 0; + if (intrstat & CMPCI_REG_CH0_INTR) { + toclear |= CMPCI_REG_CH0_INTR_ENABLE; + //cmi_clr4(sc, CMPCI_REG_INTR_CTRL, CMPCI_REG_CH0_INTR_ENABLE); + } - /* Signal interrupts to channel */ - if (intrstat & CMPCI_REG_CH0_INTR) { - chn_intr(sc->pch.channel); - } + if (intrstat & CMPCI_REG_CH1_INTR) { + toclear |= CMPCI_REG_CH1_INTR_ENABLE; + //cmi_clr4(sc, CMPCI_REG_INTR_CTRL, CMPCI_REG_CH1_INTR_ENABLE); + } - if (intrstat & CMPCI_REG_CH1_INTR) { - chn_intr(sc->rch.channel); - } + if (toclear) { + cmi_clr4(sc, CMPCI_REG_INTR_CTRL, toclear); + snd_mtxunlock(sc->lock); + + /* Signal interrupts to channel */ + if (intrstat & CMPCI_REG_CH0_INTR) { + chn_intr(sc->pch.channel); + } + + if (intrstat & CMPCI_REG_CH1_INTR) { + chn_intr(sc->rch.channel); + } - /* Enable interrupts */ - if (intrstat & CMPCI_REG_CH0_INTR) { - cmi_set4(sc, CMPCI_REG_INTR_CTRL, CMPCI_REG_CH0_INTR_ENABLE); - } + snd_mtxlock(sc->lock); + cmi_set4(sc, CMPCI_REG_INTR_CTRL, toclear); - if (intrstat & CMPCI_REG_CH1_INTR) { - cmi_set4(sc, CMPCI_REG_INTR_CTRL, CMPCI_REG_CH1_INTR_ENABLE); + } } - -out: snd_mtxunlock(sc->lock); return; } --mR8QP4gmHujQHb1c--