May 2010 - Alsa-devel - mailman.alsa-project.org

[alsa-devel] RFC:
by Peter Ujfalusi 18 May '10

18 May '10

Hello, I apologize for the long mail, but I feel that I need to explain this in a bit longish way... ASoC core recently gained support for limiting the volume control's range (maximum), which is really useful in embedded systems. The implementation for this is really simple: The codec drivers are implementing the full range the HW can support. Machine drivers can than override the maximum volume on a given control if it is necessary. This works well with most of the controls: Volume controls without dB scale. Volume controls with DECLARE_TLV_DB_SCALE() Problems started to emerge, when the volume is limited on control, which has SNDRV_CTL_TLVT_DB_RANGE (from the sound/soc/codecs/tpa6130a2.c): static const unsigned int tpa6140_tlv[] = { TLV_DB_RANGE_HEAD(3), 0, 8, TLV_DB_SCALE_ITEM(-5900, 400, 0), 9, 16, TLV_DB_SCALE_ITEM(-2500, 200, 0), 17, 31, TLV_DB_SCALE_ITEM(-1000, 100, 0), }; static const struct snd_kcontrol_new tpa6140a2_controls[] = { SOC_SINGLE_EXT_TLV("TPA6140A2 Headphone Playback Volume", TPA6130A2_REG_VOL_MUTE, 1, 0x1f, 0, tpa6130a2_get_volsw, tpa6130a2_put_volsw, tpa6140_tlv), }; The original HW supported range is 0 .. 31 (-59 .. +4 dB). When the machine driver sets the limit to 21 (-6 dB), than the volume range is reported correctly as 0 .. 21, but snd_mixer_selem_get_playback_dB_range() reports range of -59 .. +4 dB. This is not correct, it should report: -59 .. -6 dB. If the control has single DECLARE_TLV_DB_SCALE() after the limiting both volume and dB range is reported correctly. The explanation for this is in the alsa-lib: src/control/tlv.c: /** * \brief Get the dB min/max values * \param tlv the TLV source returned by #snd_tlv_parse_dB_info() * \param rangemin the minimum value of the raw volume * \param rangemax the maximum value of the raw volume * \param min the pointer to store the minimum dB value (in 0.01dB unit) * \param max the pointer to store the maximum dB value (in 0.01dB unit) * \return 0 if successful, or a negative error code */ int snd_tlv_get_dB_range(unsigned int *tlv, long rangemin, long rangemax, long *min, long *max) { int err; switch (tlv[0]) { case SND_CTL_TLVT_DB_RANGE: { unsigned int pos, len; len = int_index(tlv[1]); if (len > MAX_TLV_RANGE_SIZE) return -EINVAL; pos = 2; while (pos + 4 <= len) { long rmin, rmax; rangemin = (int)tlv[pos]; rangemax = (int)tlv[pos + 1]; err = snd_tlv_get_dB_range(tlv + pos + 2, rangemin, rangemax, &rmin, &rmax); if (err < 0) return err; if (pos > 2) { if (rmin < *min) *min = rmin; if (rmax > *max) *max = rmax; } else { *min = rmin; *max = rmax; } pos += int_index(tlv[pos + 3]) + 4; } return 0; } case SND_CTL_TLVT_DB_SCALE: { int step; *min = (int)tlv[2]; step = (tlv[3] & 0xffff); *max = *min + (long)(step * (rangemax - rangemin)); return 0; } case SND_CTL_TLVT_DB_MINMAX: case SND_CTL_TLVT_DB_MINMAX_MUTE: case SND_CTL_TLVT_DB_LINEAR: *min = (int)tlv[2]; *max = (int)tlv[3]; return 0; } return -EINVAL; } In case of DECLARE_TLV_DB_SCALE(), the SND_CTL_TLVT_DB_SCALE case is taken, and the control's maximum, and minimum range are used for the dB scale limit calculation. But, when SNDRV_CTL_TLVT_DB_RANGE is used (tpa6130a2 driver), than that case will dismisses the control's maximum and minimum range, and goes through the whole TLV struct, and reports the HW supported range. If it would stop at the driver reported maximum, than the dB range reported would be correct. So a 'simple' patch to alsa-lib would solve this. But what happens, if we want to limit control, which has DB_MINMAX, DB_MINMAX_MUTE, or DB_LINEAR? There is no way to actually limit those in a way, that the dB scale reported would be correct. We just stretch the steps bigger, like what happens with the SNDRV_CTL_TLVT_DB_RANGE in the tpa6130a2 driver. When looking for users of DB_LINEAR I have found 8 drivers, DB_MINMAX is used by usbmixer only, the DB_MINMAX_MUTE has no users at all. Since both DB_LINEAR and DB_MINMAX can be presented as DB_SCALE, and the effort to convert the drivers should not be that big. In this way, we will only have drivers using DB_SCALE in kernel, we can mark the other DB_ types as deprecated (but keeping the support for out of tree drivers), and the alsa-lib's tlv code can be adjusted to support properly the volume limiting. If there is interest, than the ALSA core could also gain support for platform/configuration dependent volume limiting. If this is too big change to ALSA (which I think it is not, since for the user it makes no difference, and the volume range, dB scale not going to be changed), than probably cleaning up the sound/soc/ to use _only_ DB_SCALE type for volume controls might be the solution, since the volume limiting feature is only in ASoC AFAIK. Either way, the alsa-lib's src/control/tlv.c:snd_tlv_get_dB_range function in case of SND_CTL_TLVT_DB_RANGE needs some fix to handle the situation, when the volume range is _not_ at the HW supported range. To summarize, I'd like to do (or ask help to do) one of the following [1] Replace the use of DB_LINEAR, DB_MINMAX with DB_SCALE in all drivers under sound/ Document, that the now unused DB_ types are deprecated Fix alsa-lib to support the volume limiting in regards of dB scale [2] Replace the use of DB_LINEAR with DB_SCALE in all drivers under sound/soc/ Fix alsa-lib to support the volume limiting in regards of dB scale What do you think? -- Péter

1 1

Re: [alsa-devel] [PATCH] NUC900/audio: add nuc900 audio driver support
by Mark Brown 18 May '10

18 May '10

On Sun, May 16, 2010 at 11:34:01PM +0800, Wan ZongShun wrote: This all looks pretty good. A few comments below but they're all fairly minor and should be easy to address. > index b1749bc..81d4848 100644 > --- a/sound/soc/Kconfig > +++ b/sound/soc/Kconfig > @@ -36,6 +36,7 @@ source "sound/soc/s3c24xx/Kconfig" > source "sound/soc/s6000/Kconfig" > source "sound/soc/sh/Kconfig" > source "sound/soc/txx9/Kconfig" > +source "sound/soc/nuc900/Kconfig" Please keep the Kconfig and Makefile sorted, this helps avoid merge issues. > +#define NUC900_AC97_RATES (SNDRV_PCM_RATE_8000 | SNDRV_PCM_RATE_11025 |\ > + SNDRV_PCM_RATE_16000 | SNDRV_PCM_RATE_22050 |\ > + SNDRV_PCM_RATE_44100 | SNDRV_PCM_RATE_48000) SNDRV_PCM_RATE_8000_48000. > + if (!timeout) > + dev_err(nuc900_audio->dev, "AC97 read register time out !\n"); > + > + val = AUDIO_READ(nuc900_audio->mmio + ACTL_ACOS0) ; If the read timed out shouldn't we be returning rather than continuing? > + val = AUDIO_READ(nuc900_audio->mmio + ACTL_ACCON); > + val &= (~AC_C_RES); > + AUDIO_WRITE(nuc900_audio->mmio + ACTL_ACCON, val); > + udelay(1000); > + if (!(AUDIO_READ(nuc900_audio->mmio + ACTL_ACIS0) & CODEC_READY)) > + dev_err(nuc900_audio->dev, "AC97 codec cold reset failed!\n"); What is this actually checking in the hardware? Not all CODECs enable the AC97 link by default after a cold reset, the standard allows them to power up in a low power state which will On the other hand, given that there's no warm reset operation perhaps this isn't a big deal. > +static void nuc900_ac97_remove(struct platform_device *pdev, > + struct snd_soc_dai *dai) > +{ > + struct nuc900_audio *nuc900_audio = nuc900_ac97_data; > + > + /* Enable unit clock */ > + clk_disable(nuc900_audio->clk); Bit rot in the comments here. > +/* bit definition of REG_ACTL_CON register */ > +#define AUDCLK_EN 0x8000 > +#define PFIFO_EN 0x4000 > +#define RFIFO_EN 0x2000 These constants (and the rest) really should be namespaced - they're likely to collide with other definitions in, for example, CODEC drivers used by machines. > +#define IIS_EN 0x0002 Looks like there's I2S support to come? > +static irqreturn_t nuc900_dma_interrupt(int irq, void *dev_id) > +{ > + > + snd_pcm_period_elapsed(substream); > + > + return IRQ_HANDLED; This is done unconditionally - are you sure there can't be any spurious interrupts (eg, error interrupts). It shouldn't cause any harm, of course.

2 4

[alsa-devel] soc multi chip support
by Attila RS 17 May '10

17 May '10

Hi, I have 2 of the same audio codecs connected to an omap (each codec has it's own i2c address and each has its own mcbsp channel). I can configure and use each one individually, but have not found a way to configure both at the same time so that each codec shows up as an alsa device. I did some searching and found that this may not be supported yet. Is that the case or has anyone done this yet or is working on it? Attila

2 3

[alsa-devel] [PATCH 0/2] ASoC: tlv320dac33: small power handling updates
by Peter Ujfalusi 17 May '10

17 May '10

Hello, While testing suspend, it has been noticed, that the driver would try to power off the codec more than once. Since this case handled in the driver, and results no functional defect we can lower the noise in the kernel log. Also avoid the root cause of the multiple off calls by filtering BIAS_OFF request when the codec was already OFF. --- Felipe Balbi (1): ASoC: tlv320dac33: Use dev_dbg in dac33_hard_power function Peter Ujfalusi (1): ASoC: tlv320dac33: Avoid powering off while in BIAS_OFF sound/soc/codecs/tlv320dac33.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)

3 4

[alsa-devel] enhanced Creative X-Fi support
by Sebastian Reinhardt 17 May '10

17 May '10

@Takashi: I own a Creative X-Fi Elite Pro and I use openSuSE. Since few months, the base card is supported by alsa. Big thanks, great work. But I like to use also the external I/O- terminal, delivered with ElitePro- cards. Is there any chance to get this working. I'm no real linux kernel hacker, but I can give You every infomation about the terminal, if You need (I had already disassembled the I/O- box). I do not need the effects, but In like to use the additional in-/outputs and volume knob. To use the ir- control, would be great, too. -- Kind Regaards Sebastian Reinhardt

1 0

[alsa-devel] [patch -next] asihpi: testing the wrong variable
by Dan Carpenter 17 May '10

17 May '10

There is a typo here. We want to test "*dst" not "dst". Signed-off-by: Dan Carpenter <error27(a)gmail.com> diff --git a/sound/pci/asihpi/hpifunc.c b/sound/pci/asihpi/hpifunc.c index 254c580..15f0c7e 100644 --- a/sound/pci/asihpi/hpifunc.c +++ b/sound/pci/asihpi/hpifunc.c @@ -3279,7 +3279,7 @@ static u16 hpi_entity_alloc_and_copy(struct hpi_entity *src, buf_size = hpi_entity_size(src); *dst = kmalloc(buf_size, GFP_KERNEL); - if (dst == NULL) + if (*dst == NULL) return HPI_ERROR_MEMORY_ALLOC; memcpy(*dst, src, buf_size); return 0;

2 1

[alsa-devel] [patch -next] sound/es1688: add pedantic range checks
by Dan Carpenter 17 May '10

17 May '10

Smatch complains that if (dev == SNDRV_CARDS) we're one past the end of the array. That's unlikely to happen in real life, I suppose. Also smatch complains about "strcpy(card->shortname, pcm->name);" The "pcm->name" buffer is 80 characters and "card->shortname" is 32 characters. If you follow the call paths it turns out we never actually use more than 16 characters so it's not a problem. But anyway, let's make it easy for people auditing this in the future. Signed-off-by: Dan Carpenter <error27(a)gmail.com> diff --git a/sound/isa/es1688/es1688.c b/sound/isa/es1688/es1688.c index fdcce31..0cde813 100644 --- a/sound/isa/es1688/es1688.c +++ b/sound/isa/es1688/es1688.c @@ -149,10 +149,11 @@ static int __devinit snd_es1688_probe(struct snd_card *card, unsigned int n) if (error < 0) return error; - strcpy(card->driver, "ES1688"); - strcpy(card->shortname, pcm->name); - sprintf(card->longname, "%s at 0x%lx, irq %i, dma %i", pcm->name, - chip->port, chip->irq, chip->dma8); + strlcpy(card->driver, "ES1688", sizeof(card->driver)); + strlcpy(card->shortname, pcm->name, sizeof(card->shortname)); + snprintf(card->longname, sizeof(card->longname), + "%s at 0x%lx, irq %i, dma %i", pcm->name, chip->port, + chip->irq, chip->dma8); if (fm_port[n] == SNDRV_AUTO_PORT) fm_port[n] = port[n]; /* share the same port */ @@ -271,6 +272,8 @@ static int __devinit snd_es968_pnp_detect(struct pnp_card_link *pcard, if (enable[dev] && isapnp[dev]) break; } + if (dev == SNDRV_CARDS) + return -ENODEV; error = snd_card_create(index[dev], id[dev], THIS_MODULE, sizeof(struct snd_es1688), &card);

2 1

[alsa-devel] [PATCH 0/4] updates for Intel SandyBridge/CougarPoint HDMI codec V2
by Wu Fengguang 17 May '10

17 May '10

Hi Takashi, The Intel SandyBridge/CougarPoint HDMI codec is very similar to the IbexPeak HDMI codec, except that it exports one more Audio Converter. Now we have 3 Audio Converters, each of them present one HDMI PCM device: (and expect a 4th HDMI PCM device in future) $ aplay -l | grep HDMI card 0: PCH [HDA Intel PCH], device 3: INTEL HDMI 0 [INTEL HDMI 0] card 0: PCH [HDA Intel PCH], device 7: INTEL HDMI 1 [INTEL HDMI 1] card 0: PCH [HDA Intel PCH], device 8: INTEL HDMI 2 [INTEL HDMI 2] So enable devices minors to support more than 8 PCM playback devices. Thanks, Fengguang

2 5

[alsa-devel] [PATCH 1/20] sound/pci/asihpi: Use kzalloc
by Julia Lawall 17 May '10

17 May '10

From: Julia Lawall <julia(a)diku.dk> Use kzalloc rather than the combination of kmalloc and memset. The semantic patch that makes this change is as follows: (http://coccinelle.lip6.fr/) // <smpl> @@ expression x,size,flags; statement S; @@ -x = kmalloc(size,flags); +x = kzalloc(size,flags); if (x == NULL) S -memset(x, 0, size); // </smpl> Signed-off-by: Julia Lawall <julia(a)diku.dk> --- sound/pci/asihpi/hpi6000.c | 3 +-- sound/pci/asihpi/hpi6205.c | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff -u -p a/sound/pci/asihpi/hpi6000.c b/sound/pci/asihpi/hpi6000.c --- a/sound/pci/asihpi/hpi6000.c +++ b/sound/pci/asihpi/hpi6000.c @@ -448,14 +448,13 @@ static void subsys_create_adapter(struct if (phm->u.s.resource.r.pci->device_id != HPI_PCI_DEV_ID_PCI2040) return; - ao.priv = kmalloc(sizeof(struct hpi_hw_obj), GFP_KERNEL); + ao.priv = kzalloc(sizeof(struct hpi_hw_obj), GFP_KERNEL); if (!ao.priv) { HPI_DEBUG_LOG(ERROR, "cant get mem for adapter object\n"); phr->error = HPI_ERROR_MEMORY_ALLOC; return; } - memset(ao.priv, 0, sizeof(struct hpi_hw_obj)); /* create the adapter object based on the resource information */ /*? memcpy(&ao.Pci,&phm->u.s.Resource.r.Pci,sizeof(ao.Pci)); */ ao.pci = *phm->u.s.resource.r.pci; diff -u -p a/sound/pci/asihpi/hpi6205.c b/sound/pci/asihpi/hpi6205.c --- a/sound/pci/asihpi/hpi6205.c +++ b/sound/pci/asihpi/hpi6205.c @@ -482,13 +482,12 @@ static void subsys_create_adapter(struct if (phm->u.s.resource.r.pci->device_id != HPI_PCI_DEV_ID_DSP6205) return; - ao.priv = kmalloc(sizeof(struct hpi_hw_obj), GFP_KERNEL); + ao.priv = kzalloc(sizeof(struct hpi_hw_obj), GFP_KERNEL); if (!ao.priv) { HPI_DEBUG_LOG(ERROR, "cant get mem for adapter object\n"); phr->error = HPI_ERROR_MEMORY_ALLOC; return; } - memset(ao.priv, 0, sizeof(struct hpi_hw_obj)); ao.pci = *phm->u.s.resource.r.pci; err = create_adapter_obj(&ao, &os_error_code);

2 1

[alsa-devel] [patch 0/6] kirkwood openrd client audio support - v2
by apatard＠mandriva.com 17 May '10

17 May '10

[ added forgotten linux-arm-kernel cc: ] Hi, It's a new try to get merged support for openrd client audio. I've tried to fix all comments, I hope I didn't miss something. In the changes, there's : - s/kirkwood/orion/ - Added a comment about data centered around 0 in the new control entry - fixed quilt diffstat header braindamage - added dapm support to cs42l51 codec - use ASoC register cache support - modified i2s driver to be a platform driver and move resource stuff to the right place Arnaud Patard

3 13