Date: Tue, 28 Oct 2003 14:24:28 -0500 From: Mathew Kanner <mat@cnd.mcgill.ca> To: freebsd-current@freebsd.org Subject: sound LOR patches Message-ID: <20031028192428.GN31273@cnd.mcgill.ca>
next in thread | raw e-mail | index | archive | help
--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--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20031028192428.GN31273>