January 2013 - Alsa-devel - mailman.alsa-project.org

[alsa-devel] Thread safety of snd_pcm_open?
by Jérôme Forissier 29 Jan '13

29 Jan '13

Hello, I'm having problems using ALSA with LibVLC. The project page http://www.alsa-project.org/main/index.php/SMP_Design claims that the snd_*_open() calls are thread safe, yet it seems that when LibVLC calls them from several threads it can lead to errors like: ALSA lib conf.c:4571:(parse_args) Unknown parameter CARD ALSA lib conf.c:4571:(parse_args) Unknown parameter DEV ALSA lib conf.c:4704:(snd_config_expand) Parse arguments error: No such file or directory ALSA lib conf.c:4687:(snd_config_expand) Unknown parameters CARD=NVidia,DEV=0 ALSA lib conf.c:4571:(parse_args) Unknown parameter CARD ALSA lib conf.c:4687:(snd_config_expand) Unknown parameters CARD=PCH,DEV=0 ALSA lib conf.c:4687:(snd_config_expand) Unknown parameters CARD=NVidia,DEV=0 ALSA lib conf.c:4571:(parse_args) Unknown parameter DEV (Please see http://mailman.videolan.org/pipermail/vlc-devel/2013-January/091603.html and also http://mailman.videolan.org/pipermail/vlc-devel/2013-January/091633.html for a reproducer on top of LibVLC). I'm not familiar with the internals of ALSA lib, nor with the internals of LibVLC. The VLC devs claim that they respect the rules regarding ALSA access, but I can't tell for sure... I'm currently running my reproducer with valgrind, but before I dig too deeply, could you please give some advice? I have noticed that pcm_direct.c calls getgrnam() which is not reentrant (but replacing it with getgrnam_r is not enough to fix my problem). Also there does not seem to be any locking around snd_pcm_open[_noupdate]. Are those functions supposed to be reentrant? Thanks for your help, -- Jerome

1 0

[alsa-devel] [PATCH] ALSA: hda - Fix non-snoop page handling
by Takashi Iwai 29 Jan '13

29 Jan '13

For non-snoop mode, we fiddle with the page attributes of CORB/RIRB and the position buffer, but also the ring buffers. The problem is that the current code blindly assumes that the buffer is contiguous. However, the ring buffers may be SG-buffers, thus a wrong vmapped address is passed there, leading to Oops. This patch fixes the handling for SG-buffers. Bugzilla: https://bugzilla.novell.com/show_bug.cgi?id=800701 Cc: <stable(a)vger.kernel.org> Signed-off-by: Takashi Iwai <tiwai(a)suse.de> --- sound/pci/hda/hda_intel.c | 40 ++++++++++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index bf0a004..c78286f 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -656,29 +656,43 @@ static char *driver_short_names[] = { #define get_azx_dev(substream) (substream->runtime->private_data) #ifdef CONFIG_X86 -static void __mark_pages_wc(struct azx *chip, void *addr, size_t size, bool on) +static void __mark_pages_wc(struct azx *chip, struct snd_dma_buffer *dmab, bool on) { + int pages; + if (azx_snoop(chip)) return; - if (addr && size) { - int pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT; + if (!dmab || !dmab->area || !dmab->bytes) + return; + +#ifdef CONFIG_SND_DMA_SGBUF + if (dmab->dev.type == SNDRV_DMA_TYPE_DEV_SG) { + struct snd_sg_buf *sgbuf = dmab->private_data; if (on) - set_memory_wc((unsigned long)addr, pages); + set_pages_array_wc(sgbuf->page_table, sgbuf->pages); else - set_memory_wb((unsigned long)addr, pages); + set_pages_array_wb(sgbuf->page_table, sgbuf->pages); + return; } +#endif + + pages = (dmab->bytes + PAGE_SIZE - 1) >> PAGE_SHIFT; + if (on) + set_memory_wc((unsigned long)dmab->area, pages); + else + set_memory_wb((unsigned long)dmab->area, pages); } static inline void mark_pages_wc(struct azx *chip, struct snd_dma_buffer *buf, bool on) { - __mark_pages_wc(chip, buf->area, buf->bytes, on); + __mark_pages_wc(chip, buf, on); } static inline void mark_runtime_wc(struct azx *chip, struct azx_dev *azx_dev, - struct snd_pcm_runtime *runtime, bool on) + struct snd_pcm_substream *substream, bool on) { if (azx_dev->wc_marked != on) { - __mark_pages_wc(chip, runtime->dma_area, runtime->dma_bytes, on); + __mark_pages_wc(chip, snd_pcm_get_dma_buf(substream), on); azx_dev->wc_marked = on; } } @@ -689,7 +703,7 @@ static inline void mark_pages_wc(struct azx *chip, struct snd_dma_buffer *buf, { } static inline void mark_runtime_wc(struct azx *chip, struct azx_dev *azx_dev, - struct snd_pcm_runtime *runtime, bool on) + struct snd_pcm_substream *substream, bool on) { } #endif @@ -1968,11 +1982,10 @@ static int azx_pcm_hw_params(struct snd_pcm_substream *substream, { struct azx_pcm *apcm = snd_pcm_substream_chip(substream); struct azx *chip = apcm->chip; - struct snd_pcm_runtime *runtime = substream->runtime; struct azx_dev *azx_dev = get_azx_dev(substream); int ret; - mark_runtime_wc(chip, azx_dev, runtime, false); + mark_runtime_wc(chip, azx_dev, substream, false); azx_dev->bufsize = 0; azx_dev->period_bytes = 0; azx_dev->format_val = 0; @@ -1980,7 +1993,7 @@ static int azx_pcm_hw_params(struct snd_pcm_substream *substream, params_buffer_bytes(hw_params)); if (ret < 0) return ret; - mark_runtime_wc(chip, azx_dev, runtime, true); + mark_runtime_wc(chip, azx_dev, substream, true); return ret; } @@ -1989,7 +2002,6 @@ static int azx_pcm_hw_free(struct snd_pcm_substream *substream) struct azx_pcm *apcm = snd_pcm_substream_chip(substream); struct azx_dev *azx_dev = get_azx_dev(substream); struct azx *chip = apcm->chip; - struct snd_pcm_runtime *runtime = substream->runtime; struct hda_pcm_stream *hinfo = apcm->hinfo[substream->stream]; /* reset BDL address */ @@ -2002,7 +2014,7 @@ static int azx_pcm_hw_free(struct snd_pcm_substream *substream) snd_hda_codec_cleanup(apcm->codec, hinfo, substream); - mark_runtime_wc(chip, azx_dev, runtime, false); + mark_runtime_wc(chip, azx_dev, substream, false); return snd_pcm_lib_free_pages(substream); } -- 1.8.1.1

1 0

[alsa-devel] [PATCH] ALSA: hda - Enable LPIB delay count for Poulsbo / Oaktrail
by Takashi Iwai 29 Jan '13

29 Jan '13

Currently we use LPIB forcibly for both playback and capture for Poulsbo and Oaktrail devices, and this seems rather problematic. The recent fix for LPIB delay count seems working well with these devices, so let's enable it instead. Reported-by: Martin Weishart <martin.weishart(a)telosalliance.com> Signed-off-by: Takashi Iwai <tiwai(a)suse.de> --- sound/pci/hda/hda_intel.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index 0b6aeba..bf0a004 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -3613,13 +3613,12 @@ static DEFINE_PCI_DEVICE_TABLE(azx_ids) = { /* 5 Series/3400 */ { PCI_DEVICE(0x8086, 0x3b56), .driver_data = AZX_DRIVER_SCH | AZX_DCAPS_INTEL_PCH }, - /* SCH */ + /* Poulsbo */ { PCI_DEVICE(0x8086, 0x811b), - .driver_data = AZX_DRIVER_SCH | AZX_DCAPS_SCH_SNOOP | - AZX_DCAPS_BUFSIZE | AZX_DCAPS_POSFIX_LPIB }, /* Poulsbo */ + .driver_data = AZX_DRIVER_SCH | AZX_DCAPS_INTEL_PCH_NOPM }, + /* Oaktrail */ { PCI_DEVICE(0x8086, 0x080a), - .driver_data = AZX_DRIVER_SCH | AZX_DCAPS_SCH_SNOOP | - AZX_DCAPS_BUFSIZE | AZX_DCAPS_POSFIX_LPIB }, /* Oaktrail */ + .driver_data = AZX_DRIVER_SCH | AZX_DCAPS_INTEL_PCH_NOPM }, /* ICH */ { PCI_DEVICE(0x8086, 0x2668), .driver_data = AZX_DRIVER_ICH | AZX_DCAPS_OLD_SSYNC | -- 1.8.1.1

1 0

[alsa-devel] [PATCH 0/6] Misc ALSA changes, documentation, style
by Antonio Ospite 29 Jan '13

29 Jan '13

From: Antonio Ospite <ao2(a)amarulasolutions.com> Hi, here are some fixes for things I noticed while studying ALSA. I'd like some feedback especially on patch 1. Patches 2 and 3 are documentation fixes. Patches 4, 5 and 6, are basically style fixes. Feel free to discard patch 6 if you find it too invasive, it's just cosmetics but some maintainers don't like big style changes even if they are in the "right" direction. I don't know alsa devs' position about that. I am sending the whole series to alsa-devel and CCing topic specific mailing lists or developers on a per-patch basis. Thanks, Antonio Antonio Ospite (6): Force a cast to silence a warning from "sparse" Documentation/DocBook/writing-an-alsa-driver.tmpl: fix some thinkos Documentation/DocBook/writing-an-alsa-driver.tmpl: fix some typos sound/usb/caiaq/device.c: fix use of MODULE_SUPPORTED_DEVICES() sound/usb/card.c: cosmetics, remove a leading space sound/core/pcm_native.c: cosmetics, fix use of spaces Documentation/DocBook/writing-an-alsa-driver.tmpl | 11 +- include/sound/memalloc.h | 2 +- sound/core/pcm_native.c | 120 ++++++++++----------- sound/usb/caiaq/device.c | 8 +- sound/usb/card.c | 2 +- 5 files changed, 71 insertions(+), 72 deletions(-) -- Antonio Ospite http://ao2.it A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing?

3 11

Re: [alsa-devel] [PATCH 1/4] DMA: PL330: Add xlate function
by Arnd Bergmann 29 Jan '13

29 Jan '13

On Friday 18 January 2013, Padmavathi Venna wrote: > +struct dma_chan *of_dma_pl330_xlate(struct of_phandle_args *dma_spec, > + struct of_dma *ofdma) > +{ > + int count = dma_spec->args_count; > + struct of_dma_filter_info *info = ofdma->of_dma_data; > + > + if (!info || !info->filter_fn) > + return NULL; > + > + if (count != 1) > + return NULL; > + > + return dma_request_channel(info->dma_cap, info->filter_fn, dma_spec); > +} > +EXPORT_SYMBOL_GPL(of_dma_pl330_xlate); It seems a little sad that we still have to use dma_request_channel() to implement this, when that function will go off searching all channels and pass them tino the filter, which then has to look for the device node and match it with each channel. We already know the controller and should just be able to get a channel for it, although I don't exactly know how that is done. Further, your function is almost identical to the of_dma_simple_xlate function. Can't you use that one instead? Arnd

2 2

Re: [alsa-devel] [PATCH 4/4] ASOC: wm8962: add some operation in wm8962_runtime_resume()
by Mark Brown 29 Jan '13

29 Jan '13

On Tue, Jan 29, 2013 at 03:50:10PM +0800, Gary Zhang wrote: > 1. in this function, after wm8962_reset() is called, WM8962_SYSCLK_ENA bit > is reset to 1. make sure it is off so we can safely write to registers if > the device is declocked. > 2. add bias level management Split separate changes into separate patches; obviously there's bias level management in the driver since otherwise it'd be totally non-functional. > @@ -3721,6 +3721,24 @@ static int wm8962_runtime_resume(struct device *dev) > > regcache_sync(wm8962->regmap); > > + regmap_update_bits(wm8962->regmap, WM8962_CLOCKING2, > + WM8962_SYSCLK_ENA, 0); > + This seems like a bodge to work around some other problem in your system. The device should never be runtime suspended with an active clock, and this seems like it isn't going to work as most of the register cache restore will be done with SYSCLK enabled. We need to figure out what went wrong to allow the device to enter runtime suspend with SYSCLK on. > + regmap_update_bits(wm8962->regmap, WM8962_ANTI_POP, > + WM8962_STARTUP_BIAS_ENA | WM8962_VMID_BUF_ENA, > + WM8962_STARTUP_BIAS_ENA | WM8962_VMID_BUF_ENA); > + > + /* Bias enable at 2*50k for ramp */ > + regmap_update_bits(wm8962->regmap, WM8962_PWR_MGMT_1, > + WM8962_VMID_SEL_MASK | WM8962_BIAS_ENA, > + WM8962_BIAS_ENA | 0x180); > + > + msleep(5); > + > + /* VMID back to 2x250k for standby */ > + regmap_update_bits(wm8962->regmap, WM8962_PWR_MGMT_1, > + WM8962_VMID_SEL_MASK, 0x100); We already have code to ramp VMID when exiting _OFF...

1 0

Re: [alsa-devel] [PATCH V6 07/10] ASoC: SMDK: WM8994: Add device tree support for machine file
by Mark Brown 29 Jan '13

29 Jan '13

On Fri, Jan 18, 2013 at 05:17:06PM +0530, Padmavathi Venna wrote: > +Samsung SMDK audio complex This is just for SMDKs with WM8994. I'll apply but please send a followup patch to clarify this - it'll be a different binding for others like the older boards using WM8580 and WM9713. > + samsung,i2s-controller = <&i2s0>; > + samsung,audio-codec = <&wm8994>; I was thinking this should really point at the DAI but since it's specific to the SMDKs I don't think it particularly matters - they all use AIF1 anyway.

2 1

Re: [alsa-devel] [PATCH V6 04/10] spi: s3c64xx: Modify SPI driver to use generic DMA DT support
by Mark Brown 29 Jan '13

29 Jan '13

On Fri, Jan 18, 2013 at 05:17:03PM +0530, Padmavathi Venna wrote: > This patch modifies the SPI driver to use generic dma dt bindings > support. This passes all the required arguments to dma dev request > functon which in turn calls the dma_request_slave_channel or dma__ > request_channel based on DT or non-DT respectively. This loooks OK and I'm actually applying SPI patches so I could apply it but I'm not sure I see the relevance of this patch to the rest of the series (which is about ASoC). Is there some reason why it's included in this patch series or can it be applied to the SPI tree? The SMDKs use I2C for their CODEC control rather than SPI. Also is there a binding document update or device tree file update to go with this? > + if (!sdd->pdev->dev.of_node) { > + res = platform_get_resource(pdev, IORESOURCE_DMA, 0); > + if (!res) { > + dev_err(&pdev->dev, "Unable to get SPI tx dma " > + "resource\n"); I appreciate that this is cut'n'paste from the code you're refactoring but please don't split error messages over lines, it makes it hard to grep for them in the kernel source.

2 1

Re: [alsa-devel] [PATCH V6 03/10] ARM: SAMSUNG: Make dma request compatible to generic dma bindings.
by Mark Brown 29 Jan '13

29 Jan '13

On Fri, Jan 18, 2013 at 05:17:02PM +0530, Padmavathi Venna wrote: > This patch make the dma dev request operation compatible for both > DT and non-DT cases. It takes the all the arguments required for > dma_request_slave_channel and dma_request_channel. If the driver > is initiated via DT or non-DT the corresponding call will be made. OK, so this is where the SPI change came from - the API has been changed incompatibly and SPI needs updating. For bisection this API update ought to be done in a single commit, otherwise there will be steps in the bisection where nothing will work. Anyway, I'll go ahead and apply this, the SPI and the ASoC ones but this is something to bear in mind for the future.

2 1

Re: [alsa-devel] [PATCH 3/4] ASOC: imx: add machine driver using wm8962 codec
by Mark Brown 29 Jan '13

29 Jan '13

On Tue, Jan 29, 2013 at 03:49:50PM +0800, Gary Zhang wrote: > +static struct imx_priv card_priv; Global data bad... > +static int imx_wm8962_startup(struct snd_pcm_substream *substream) > +{ > + struct snd_soc_pcm_runtime *rtd = substream->private_data; > + struct snd_soc_dai *codec_dai = rtd->codec_dai; > + struct imx_priv *priv = &card_priv; > + struct imx_wm8962_data *data = platform_get_drvdata(priv->pdev); > + > + if (!codec_dai->active) > + clk_enable(data->codec_clk); Should be clk_prepare_enable() but it's not clear to me why you need this... Also the clock API does refcounting so there should be no need to check for active, you should get matching startups and shutdowns. This will also fail for analogue bypass paths, set_bias_level() would be a good place to cover those. > + int_port--; > + ext_port--; > + ret = imx_audmux_v2_configure_port(int_port, > + IMX_AUDMUX_V2_PTCR_SYN | > + IMX_AUDMUX_V2_PTCR_TFSEL(ext_port) | > + IMX_AUDMUX_V2_PTCR_TCSEL(ext_port) | > + IMX_AUDMUX_V2_PTCR_TFSDIR | > + IMX_AUDMUX_V2_PTCR_TCLKDIR, > + IMX_AUDMUX_V2_PDCR_RXDSEL(ext_port)); > + if (ret) { > + dev_err(&pdev->dev, "audmux internal port setup failed\n"); > + return ret; > + } > + imx_audmux_v2_configure_port(ext_port, > + IMX_AUDMUX_V2_PTCR_SYN, > + IMX_AUDMUX_V2_PDCR_RXDSEL(int_port)); > + if (ret) { > + dev_err(&pdev->dev, "audmux external port setup failed\n"); > + return ret; > + } This stuff is pretty common for i.MX drivers - can we factor it out into a helper library? > + ssi_pdev = of_find_device_by_node(ssi_np); > + if (!ssi_pdev) { > + dev_err(&pdev->dev, "failed to find SSI platform device\n"); > + ret = -EINVAL; > + goto fail; > + } Please remember to include binding documentation for all new device tree bindings.

1 0