From 01bec79c1123ef0749a2523a35da7e9eab74ccf7 Mon Sep 17 00:00:00 2001 From: Alexander Leidinger Date: Sun, 31 Jul 2005 13:43:42 +0000 Subject: [PATCH] * Add locking / MPSAFE * Add kernel hint option to disable DXS channels entirely. Report from several skype users / Pav Lucistnik indicate that disabling DXS may fix lots of pop / crackling noise. To disable DXS add hint.pcm..via_dxs_disabled="1" to /boot/device.hints. Further investigation of the issues regarding DXS showed, that the problem is in another (more generic) place, but until the right fix is tested/reviewed this may help a little bit. Added sysctl's to aid testing/debugging: hint.pcm..via_dxs_disabled=X - Disable / Enable DXS channels entirely hint.pcm..via_dxs_channels=X - Limit DXS channels up to X hint.pcm..via_sgd_channels=X - Limit SGD channels up to X hint.pcm..via_dxs_src=X - Enable / Disable DXS sample rate converter. Submitted by: Ariff Abdullah Tested by: multimedia@ --- sys/dev/sound/pci/via8233.c | 288 ++++++++++++++++++++++++++---------- 1 file changed, 206 insertions(+), 82 deletions(-) diff --git a/sys/dev/sound/pci/via8233.c b/sys/dev/sound/pci/via8233.c index 241baea42bc9..d2626136ece7 100644 --- a/sys/dev/sound/pci/via8233.c +++ b/sys/dev/sound/pci/via8233.c @@ -100,12 +100,14 @@ struct via_info { struct ac97_info *codec; unsigned int bufsz; + int spdif_en, dxs_src; struct via_chinfo pch[NDXSCHANS + NMSGDCHANS]; struct via_chinfo rch[NWRCHANS]; struct via_dma_op *sgd_table; u_int16_t codec_caps; u_int16_t n_dxs_registered; + struct mtx *lock; }; static u_int32_t via_fmt[] = { @@ -119,6 +121,91 @@ static u_int32_t via_fmt[] = { static struct pcmchan_caps via_vracaps = { 4000, 48000, via_fmt, 0 }; static struct pcmchan_caps via_caps = { 48000, 48000, via_fmt, 0 }; +#ifdef SND_DYNSYSCTL +static int +sysctl_via8233_spdif_enable(SYSCTL_HANDLER_ARGS) +{ + struct via_info *via; + device_t dev; + int err, new_en, r; + + dev = oidp->oid_arg1; + via = pcm_getdevinfo(dev); + snd_mtxlock(via->lock); + new_en = via->spdif_en; + snd_mtxunlock(via->lock); + err = sysctl_handle_int(oidp, &new_en, sizeof(new_en), req); + + if (err || req->newptr == NULL) + return err; + if (new_en < 0 || new_en > 1) + return EINVAL; + + snd_mtxlock(via->lock); + via->spdif_en = new_en; + + r = pci_read_config(dev, VIA_PCI_SPDIF, 1) & ~VIA_SPDIF_EN; + if (new_en) + r |= VIA_SPDIF_EN; + pci_write_config(dev, VIA_PCI_SPDIF, r, 1); + snd_mtxunlock(via->lock); + + return 0; +} + +static int +sysctl_via8233_dxs_src(SYSCTL_HANDLER_ARGS) +{ + struct via_info *via; + device_t dev; + int err, val; + + dev = oidp->oid_arg1; + via = pcm_getdevinfo(dev); + snd_mtxlock(via->lock); + val = via->dxs_src; + snd_mtxunlock(via->lock); + err = sysctl_handle_int(oidp, &val, sizeof(val), req); + + if (err || req->newptr == NULL) + return err; + if (val < 0 || val > 1) + return EINVAL; + + snd_mtxlock(via->lock); + via->dxs_src = val; + snd_mtxunlock(via->lock); + + return 0; +} +#endif /* SND_DYNSYSCTL */ + +static void +via_init_sysctls(device_t dev) +{ +#ifdef SND_DYNSYSCTL + struct via_info *via; + int r; + + via = pcm_getdevinfo(dev); + r = pci_read_config(dev, VIA_PCI_SPDIF, 1); + via->spdif_en = (r & VIA_SPDIF_EN) ? 1 : 0; + + SYSCTL_ADD_PROC(snd_sysctl_tree(dev), + SYSCTL_CHILDREN(snd_sysctl_tree_top(dev)), + OID_AUTO, "spdif_enabled", + CTLTYPE_INT | CTLFLAG_RW, dev, sizeof(dev), + sysctl_via8233_spdif_enable, "I", + "Enable S/PDIF output on primary playback channel"); + SYSCTL_ADD_PROC(snd_sysctl_tree(dev), + SYSCTL_CHILDREN(snd_sysctl_tree_top(dev)), + OID_AUTO, "via_dxs_src", + CTLTYPE_INT | CTLFLAG_RW, dev, sizeof(dev), + sysctl_via8233_dxs_src, "I", + "Enable VIA DXS Sample Rate Converter"); +#endif +} + static u_int32_t via_rd(struct via_info *via, int regno, int size) { @@ -253,7 +340,7 @@ via8233wr_setformat(kobj_t obj, void *data, u_int32_t format) { struct via_chinfo *ch = data; struct via_info *via = ch->parent; - + u_int32_t f = WR_FORMAT_STOP_INDEX; if (format & AFMT_STEREO) @@ -270,9 +357,10 @@ via8233dxs_setformat(kobj_t obj, void *data, u_int32_t format) { struct via_chinfo *ch = data; struct via_info *via = ch->parent; + u_int32_t r, v; - u_int32_t r = ch->rbase + VIA8233_RP_DXS_RATEFMT; - u_int32_t v = via_rd(via, r, 4); + r = ch->rbase + VIA8233_RP_DXS_RATEFMT; + v = via_rd(via, r, 4); v &= ~(VIA8233_DXS_RATEFMT_STEREO | VIA8233_DXS_RATEFMT_16BIT); if (format & AFMT_STEREO) @@ -316,11 +404,10 @@ via8233wr_setspeed(kobj_t obj, void *data, u_int32_t speed) struct via_chinfo *ch = data; struct via_info *via = ch->parent; - u_int32_t spd = 48000; - if (via->codec_caps & AC97_EXTCAP_VRA) { - spd = ac97_setrate(via->codec, AC97_REGEXT_LADCRATE, speed); - } - return spd; + if (via->codec_caps & AC97_EXTCAP_VRA) + return ac97_setrate(via->codec, AC97_REGEXT_LADCRATE, speed); + + return 48000; } static int @@ -328,9 +415,10 @@ via8233dxs_setspeed(kobj_t obj, void *data, u_int32_t speed) { struct via_chinfo *ch = data; struct via_info *via = ch->parent; + u_int32_t r, v; - u_int32_t r = ch->rbase + VIA8233_RP_DXS_RATEFMT; - u_int32_t v = via_rd(via, r, 4) & ~VIA8233_DXS_RATEFMT_48K; + r = ch->rbase + VIA8233_RP_DXS_RATEFMT; + v = via_rd(via, r, 4) & ~VIA8233_DXS_RATEFMT_48K; /* Careful to avoid overflow (divide by 48 per vt8233c docs) */ @@ -362,7 +450,7 @@ via8233wr_getcaps(kobj_t obj, void *data) struct via_info *via = ch->parent; /* Controlled by ac97 registers */ - if (via->codec_caps & AC97_EXTCAP_VRA) + if (via->codec_caps & AC97_EXTCAP_VRA) return &via_vracaps; return &via_caps; } @@ -370,7 +458,17 @@ via8233wr_getcaps(kobj_t obj, void *data) static struct pcmchan_caps * via8233dxs_getcaps(kobj_t obj, void *data) { - /* Controlled by onboard registers */ + struct via_chinfo *ch = data; + struct via_info *via = ch->parent; + + /* + * Controlled by onboard registers + * + * Apparently, few boards can do DXS sample rate + * conversion. + */ + if (via->dxs_src) + return &via_vracaps; return &via_caps; } @@ -381,7 +479,7 @@ via8233msgd_getcaps(kobj_t obj, void *data) struct via_info *via = ch->parent; /* Controlled by ac97 registers */ - if (via->codec_caps & AC97_EXTCAP_VRA) + if (via->codec_caps & AC97_EXTCAP_VRA) return &via_vracaps; return &via_caps; } @@ -393,6 +491,7 @@ static int via8233chan_setblocksize(kobj_t obj, void *data, u_int32_t blocksize) { struct via_chinfo *ch = data; + sndbuf_resize(ch->buffer, SEGS_PER_CHAN, blocksize); ch->blksz = sndbuf_getblksz(ch->buffer); return ch->blksz; @@ -403,11 +502,13 @@ via8233chan_getptr(kobj_t obj, void *data) { struct via_chinfo *ch = data; struct via_info *via = ch->parent; + u_int32_t v, index, count; + int ptr; - u_int32_t v = via_rd(via, ch->rbase + VIA_RP_CURRENT_COUNT, 4); - u_int32_t index = v >> 24; /* Last completed buffer */ - u_int32_t count = v & 0x00ffffff; /* Bytes remaining */ - int ptr = (index + 1) * ch->blksz - count; + v = via_rd(via, ch->rbase + VIA_RP_CURRENT_COUNT, 4); + index = v >> 24; /* Last completed buffer */ + count = v & 0x00ffffff; /* Bytes remaining */ + ptr = (index + 1) * ch->blksz - count; ptr %= SEGS_PER_CHAN * ch->blksz; /* Wrap to available space */ return ptr; @@ -439,6 +540,7 @@ via8233wr_init(kobj_t obj, void *devinfo, struct snd_dbuf *b, struct via_info *via = devinfo; struct via_chinfo *ch = &via->rch[c->num]; + snd_mtxlock(via->lock); ch->parent = via; ch->channel = c; ch->buffer = b; @@ -446,11 +548,15 @@ via8233wr_init(kobj_t obj, void *devinfo, struct snd_dbuf *b, ch->rbase = VIA_WR_BASE(c->num); via_wr(via, ch->rbase + VIA_WR_RP_SGD_FORMAT, WR_FIFO_ENABLE, 1); + snd_mtxunlock(via->lock); if (sndbuf_alloc(ch->buffer, via->parent_dmat, via->bufsz) != 0) return NULL; + + snd_mtxlock(via->lock); via8233chan_sgdinit(via, ch, c->num); via8233chan_reset(via, ch); + snd_mtxunlock(via->lock); return ch; } @@ -462,6 +568,7 @@ via8233dxs_init(kobj_t obj, void *devinfo, struct snd_dbuf *b, struct via_info *via = devinfo; struct via_chinfo *ch = &via->pch[c->num]; + snd_mtxlock(via->lock); ch->parent = via; ch->channel = c; ch->buffer = b; @@ -474,11 +581,15 @@ via8233dxs_init(kobj_t obj, void *devinfo, struct snd_dbuf *b, */ ch->rbase = VIA_DXS_BASE(NDXSCHANS - 1 - via->n_dxs_registered); via->n_dxs_registered++; + snd_mtxunlock(via->lock); if (sndbuf_alloc(ch->buffer, via->parent_dmat, via->bufsz) != 0) return NULL; + + snd_mtxlock(via->lock); via8233chan_sgdinit(via, ch, NWRCHANS + c->num); via8233chan_reset(via, ch); + snd_mtxunlock(via->lock); return ch; } @@ -490,16 +601,21 @@ via8233msgd_init(kobj_t obj, void *devinfo, struct snd_dbuf *b, struct via_info *via = devinfo; struct via_chinfo *ch = &via->pch[c->num]; + snd_mtxlock(via->lock); ch->parent = via; ch->channel = c; ch->buffer = b; ch->dir = dir; ch->rbase = VIA_MC_SGD_STATUS; + snd_mtxunlock(via->lock); if (sndbuf_alloc(ch->buffer, via->parent_dmat, via->bufsz) != 0) return NULL; + + snd_mtxlock(via->lock); via8233chan_sgdinit(via, ch, NWRCHANS + c->num); via8233chan_reset(via, ch); + snd_mtxunlock(via->lock); return ch; } @@ -590,16 +706,19 @@ via_intr(void *p) int i, stat; /* Poll playback channels */ + snd_mtxlock(via->lock); for (i = 0; i < NDXSCHANS + NMSGDCHANS; i++) { if (via->pch[i].rbase == 0) continue; stat = via->pch[i].rbase + VIA_RP_STATUS; if (via_rd(via, stat, 1) & SGD_STATUS_INTR) { via_wr(via, stat, SGD_STATUS_INTR, 1); + snd_mtxunlock(via->lock); chn_intr(via->pch[i].channel); + snd_mtxlock(via->lock); } } - + /* Poll record channels */ for (i = 0; i < NWRCHANS; i++) { if (via->rch[i].rbase == 0) @@ -607,9 +726,12 @@ via_intr(void *p) stat = via->rch[i].rbase + VIA_RP_STATUS; if (via_rd(via, stat, 1) & SGD_STATUS_INTR) { via_wr(via, stat, SGD_STATUS_INTR, 1); + snd_mtxunlock(via->lock); chn_intr(via->rch[i].channel); + snd_mtxlock(via->lock); } } + snd_mtxunlock(via->lock); } /* @@ -710,65 +832,22 @@ via_chip_init(device_t dev) return ENXIO; } -#ifdef SND_DYNSYSCTL -static int via8233_spdif_en; - -static int -sysctl_via8233_spdif_enable(SYSCTL_HANDLER_ARGS) -{ - device_t dev; - int err, new_en, r; - - new_en = via8233_spdif_en; - err = sysctl_handle_int(oidp, &new_en, sizeof(new_en), req); - if (err || req->newptr == NULL) - return err; - - if (new_en < 0 || new_en > 1) - return EINVAL; - via8233_spdif_en = new_en; - - dev = oidp->oid_arg1; - r = pci_read_config(dev, VIA_PCI_SPDIF, 1) & ~VIA_SPDIF_EN; - if (new_en) - r |= VIA_SPDIF_EN; - pci_write_config(dev, VIA_PCI_SPDIF, r, 1); - return 0; -} -#endif /* SND_DYNSYSCTL */ - -static void -via_init_sysctls(device_t dev) -{ -#ifdef SND_DYNSYSCTL - int r; - - r = pci_read_config(dev, VIA_PCI_SPDIF, 1); - via8233_spdif_en = (r & VIA_SPDIF_EN) ? 1 : 0; - - SYSCTL_ADD_PROC(snd_sysctl_tree(dev), - SYSCTL_CHILDREN(snd_sysctl_tree_top(dev)), - OID_AUTO, "spdif_enabled", - CTLTYPE_INT | CTLFLAG_RW, dev, sizeof(dev), - sysctl_via8233_spdif_enable, "I", - "Enable S/PDIF output on primary playback channel"); -#endif -} - static int via_attach(device_t dev) { struct via_info *via = 0; char status[SND_STATUSLEN]; + int i, via_dxs_disabled, via_dxs_src, via_dxs_chnum, via_sgd_chnum; if ((via = malloc(sizeof *via, M_DEVBUF, M_NOWAIT | M_ZERO)) == NULL) { device_printf(dev, "cannot allocate softc\n"); return ENXIO; } + via->lock = snd_mtxcreate(device_get_nameunit(dev), "sound softc"); pci_set_powerstate(dev, PCI_POWERSTATE_D0); pci_enable_busmaster(dev); - + via->regid = PCIR_BAR(0); via->reg = bus_alloc_resource_any(dev, SYS_RES_IOPORT, &via->regid, RF_ACTIVE); @@ -785,7 +864,7 @@ via_attach(device_t dev) via->irq = bus_alloc_resource_any(dev, SYS_RES_IRQ, &via->irqid, RF_ACTIVE | RF_SHAREABLE); if (!via->irq || - snd_setup_intr(dev, via->irq, 0, via_intr, via, &via->ih)) { + snd_setup_intr(dev, via->irq, INTR_MPSAFE, via_intr, via, &via->ih)) { device_printf(dev, "unable to map interrupt\n"); goto bad; } @@ -796,8 +875,8 @@ via_attach(device_t dev) /*highaddr*/BUS_SPACE_MAXADDR, /*filter*/NULL, /*filterarg*/NULL, /*maxsize*/via->bufsz, /*nsegments*/1, /*maxsegz*/0x3ffff, - /*flags*/0, /*lockfunc*/busdma_lock_mutex, - /*lockarg*/&Giant, &via->parent_dmat) != 0) { + /*flags*/0, /*lockfunc*/NULL, + /*lockarg*/NULL, &via->parent_dmat) != 0) { device_printf(dev, "unable to create dma tag\n"); goto bad; } @@ -813,8 +892,8 @@ via_attach(device_t dev) /*filter*/NULL, /*filterarg*/NULL, /*maxsize*/NSEGS * sizeof(struct via_dma_op), /*nsegments*/1, /*maxsegz*/0x3ffff, - /*flags*/0, /*lockfunc*/busdma_lock_mutex, - /*lockarg*/&Giant, &via->sgd_dmat) != 0) { + /*flags*/0, /*lockfunc*/NULL, + /*lockarg*/NULL, &via->sgd_dmat) != 0) { device_printf(dev, "unable to create dma tag\n"); goto bad; } @@ -850,28 +929,71 @@ via_attach(device_t dev) snprintf(status, SND_STATUSLEN, "at io 0x%lx irq %ld %s", rman_get_start(via->reg), rman_get_start(via->irq),PCM_KLDSTRING(snd_via8233)); - /* Register */ + /* + * Decide whether DXS had to be disabled or not + */ if (pci_get_revid(dev) == VIA8233_REV_ID_8233A) { - if (pcm_register(dev, via, NMSGDCHANS, 1)) goto bad; /* * DXS channel is disabled. Reports from multiple users * that it plays at half-speed. Do not see this behaviour * on available 8233C or when emulating 8233A register set * on 8233C (either with or without ac97 VRA). - pcm_addchan(dev, PCMDIR_PLAY, &via8233dxs_class, via); */ - pcm_addchan(dev, PCMDIR_PLAY, &via8233msgd_class, via); - pcm_addchan(dev, PCMDIR_REC, &via8233wr_class, via); + via_dxs_disabled = 1; + } else if (resource_int_value(device_get_name(dev), + device_get_unit(dev), "via_dxs_disabled", + &via_dxs_disabled) == 0) + via_dxs_disabled = (via_dxs_disabled > 0) ? 1 : 0; + else + via_dxs_disabled = 0; + + if (via_dxs_disabled) { + via_dxs_chnum = 0; + via_sgd_chnum = 1; } else { - int i; - if (pcm_register(dev, via, NMSGDCHANS + NDXSCHANS, NWRCHANS)) goto bad; - for (i = 0; i < NDXSCHANS; i++) - pcm_addchan(dev, PCMDIR_PLAY, &via8233dxs_class, via); - pcm_addchan(dev, PCMDIR_PLAY, &via8233msgd_class, via); - for (i = 0; i < NWRCHANS; i++) - pcm_addchan(dev, PCMDIR_REC, &via8233wr_class, via); - via_init_sysctls(dev); + if (resource_int_value(device_get_name(dev), + device_get_unit(dev), "via_dxs_channels", + &via_dxs_chnum) != 0) + via_dxs_chnum = NDXSCHANS; + if (resource_int_value(device_get_name(dev), + device_get_unit(dev), "via_sgd_channels", + &via_sgd_chnum) != 0) + via_sgd_chnum = NMSGDCHANS; } + if (via_dxs_chnum > NDXSCHANS) + via_dxs_chnum = NDXSCHANS; + else if (via_dxs_chnum < 0) + via_dxs_chnum = 0; + if (via_sgd_chnum > NMSGDCHANS) + via_sgd_chnum = NMSGDCHANS; + else if (via_sgd_chnum < 0) + via_sgd_chnum = 0; + if (via_dxs_chnum + via_sgd_chnum < 1) { + /* Minimalist ? */ + via_dxs_chnum = 1; + via_sgd_chnum = 0; + } + if (via_dxs_chnum > 0 && resource_int_value(device_get_name(dev), + device_get_unit(dev), "via_dxs_src", + &via_dxs_src) == 0) + via->dxs_src = (via_dxs_src > 0) ? 1 : 0; + else + via->dxs_src = 0; + /* Register */ + if (pcm_register(dev, via, via_dxs_chnum + via_sgd_chnum, NWRCHANS)) + goto bad; + for (i = 0; i < via_dxs_chnum; i++) + pcm_addchan(dev, PCMDIR_PLAY, &via8233dxs_class, via); + for (i = 0; i < via_sgd_chnum; i++) + pcm_addchan(dev, PCMDIR_PLAY, &via8233msgd_class, via); + for (i = 0; i < NWRCHANS; i++) + pcm_addchan(dev, PCMDIR_REC, &via8233wr_class, via); + if (via_dxs_chnum > 0) + via_init_sysctls(dev); + device_printf(dev, "\n", + (via_dxs_chnum > 0) ? "En" : "Dis", + (via->dxs_src) ? "(SRC)" : "", + via_dxs_chnum, via_sgd_chnum, NWRCHANS); pcm_setstatus(dev, status); @@ -884,6 +1006,7 @@ via_attach(device_t dev) if (via->parent_dmat) bus_dma_tag_destroy(via->parent_dmat); if (via->sgd_dmamap) bus_dmamap_unload(via->sgd_dmat, via->sgd_dmamap); if (via->sgd_dmat) bus_dma_tag_destroy(via->sgd_dmat); + if (via->lock) snd_mtxfree(via->lock); if (via) free(via, M_DEVBUF); return ENXIO; } @@ -904,6 +1027,7 @@ via_detach(device_t dev) bus_dma_tag_destroy(via->parent_dmat); bus_dmamap_unload(via->sgd_dmat, via->sgd_dmamap); bus_dma_tag_destroy(via->sgd_dmat); + snd_mtxfree(via->lock); free(via, M_DEVBUF); return 0; }