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

Re: [alsa-devel] ASoC: wm9712: Microphone doesn't work, "Capture Volume" inverted
by Mark Brown 02 May '12

02 May '12

On Tue, May 01, 2012 at 10:07:57PM +0200, Hans J. Koch wrote: > On Tue, May 01, 2012 at 06:12:07PM +0100, Mark Brown wrote: > > The driver knows exactly which registers are volatile > Then this is the first complex chip I've ever seen where the datasheet covers That's... horrifying, really. The WM9712 really isn't at all complex by modern standards for that matter, but the idea that it's common to have devices with documented registers that might spontaneously change in normal operation doesn't seem at all clever. What happens if the configuration that was set was important? > all corner cases. And what about things like dirty supply voltages or EMI? If you've got that sort of problem with supply quality or EMI there's obviously a very serious system issue to contend with, one that typically requires highly aggressive action to resolve - you're starting to get into games like having to constantly poll the device and restart it if you detect that it fell over which isn't pleasant and most likely means you've already hit serious user visible consequences. Things like device initialisation will need to be redone, for example. I can only think of one driver that takes such measures, and it's actually relying on restore from register cache for part of the recovery :) That said, many devices will get a moderate degree of robustness against this through power saving - the closer the device can get to cold during idle the better from this point of view, and in many cases it's possible to completely power down the device when idle. A common idiom is to take the device down to cold when idle then dump the setup changes that don't need sequencing into the device from cache before doing the bits that need sequencing, especially where many of the register settings can be mapped direictly into userspace. Indeed I'm not sure a cache isn't a bad plan for detecting this sort of thing; if you find the cache values diverging from the physical values for non-volatile registers you know something's up. > Just to save a few clock cycles every now and then you happily accept the > risk of the driver simply stop working after an unusual event, without > any chances of recovery. For the sort of devices we're talking about here it's way more than a few clock cycles that are involved, you're talking about fairly slow and at times congested buses like I2C here. Never mind what happens when someone decides to use a bigbanged controller, which is more common than one might dream of. Obviously if you're on a bus with trivial I/O costs the tradeoffs vary but for the sort of buses we're talking about it's much easier to see a win. Indeed with many older devices readback just isn't physically supported in the first place so if you want to do any kind of readback based thing you're going to have to cache in some way. This was true for the majority of devices at the time WM9712 was new. > Sound is not my usual playing ground, but at least I learned now why the > sound system on my desktop refuses to work after a few days uptime... I don't actually think desktop drivers bother with this much, though ICBW. They relatively infrequently use the slow buses. I'd suggest reporting the issue, anyway - it's probably more constructive. > Fortunately, most other subsystems would never accept such code for mainline. This is actually a standard feature of the regmap API that drivers can use if they want, there's a bunch of device classes where it can offer a useful win (those that typically have slow control interfaces and not so mnay volatile registers).

1 0

[alsa-devel] [PATCH] ASoC: wm_hubs: Cache multiple DCS offsets
by Mark Brown 01 May '12

01 May '12

Rather than invalidating the cached DCS value every time the headphone gain changes store multiple values, indexed by gain. This allows the optimisation we get from the cache to take effect more often. Signed-off-by: Mark Brown <broonie(a)opensource.wolfsonmicro.com> --- sound/soc/codecs/wm_hubs.c | 74 ++++++++++++++++++++++++++++++++++++++------ sound/soc/codecs/wm_hubs.h | 3 +- 2 files changed, 67 insertions(+), 10 deletions(-) diff --git a/sound/soc/codecs/wm_hubs.c b/sound/soc/codecs/wm_hubs.c index 8a29843..dfe957a 100644 --- a/sound/soc/codecs/wm_hubs.c +++ b/sound/soc/codecs/wm_hubs.c @@ -143,12 +143,69 @@ static bool wm_hubs_dac_hp_direct(struct snd_soc_codec *codec) return true; } +struct wm_hubs_dcs_cache { + struct list_head list; + unsigned int left; + unsigned int right; + u16 dcs_cfg; +}; + +static bool wm_hubs_dcs_cache_get(struct snd_soc_codec *codec, + struct wm_hubs_dcs_cache **entry) +{ + struct wm_hubs_data *hubs = snd_soc_codec_get_drvdata(codec); + struct wm_hubs_dcs_cache *cache; + unsigned int left, right; + + left = snd_soc_read(codec, WM8993_LEFT_OUTPUT_VOLUME); + left &= WM8993_HPOUT1L_VOL_MASK; + + right = snd_soc_read(codec, WM8993_RIGHT_OUTPUT_VOLUME); + right &= WM8993_HPOUT1R_VOL_MASK; + + list_for_each_entry(cache, &hubs->dcs_cache, list) { + if (cache->left != left || cache->right != right) + continue; + + *entry = cache; + return true; + } + + return false; +} + +static void wm_hubs_dcs_cache_set(struct snd_soc_codec *codec, u16 dcs_cfg) +{ + struct wm_hubs_data *hubs = snd_soc_codec_get_drvdata(codec); + struct wm_hubs_dcs_cache *cache; + + if (hubs->no_cache_dac_hp_direct) + return; + + cache = devm_kzalloc(codec->dev, sizeof(*cache), GFP_KERNEL); + if (!cache) { + dev_err(codec->dev, "Failed to allocate DCS cache entry\n"); + return; + } + + cache->left = snd_soc_read(codec, WM8993_LEFT_OUTPUT_VOLUME); + cache->left &= WM8993_HPOUT1L_VOL_MASK; + + cache->right = snd_soc_read(codec, WM8993_RIGHT_OUTPUT_VOLUME); + cache->right &= WM8993_HPOUT1R_VOL_MASK; + + cache->dcs_cfg = dcs_cfg; + + list_add_tail(&cache->list, &hubs->dcs_cache); +} + /* * Startup calibration of the DC servo */ static void calibrate_dc_servo(struct snd_soc_codec *codec) { struct wm_hubs_data *hubs = snd_soc_codec_get_drvdata(codec); + struct wm_hubs_dcs_cache *cache; s8 offset; u16 reg, reg_l, reg_r, dcs_cfg, dcs_reg; @@ -163,10 +220,11 @@ static void calibrate_dc_servo(struct snd_soc_codec *codec) /* If we're using a digital only path and have a previously * callibrated DC servo offset stored then use that. */ - if (wm_hubs_dac_hp_direct(codec) && hubs->dac_hp_direct_dcs) { - dev_dbg(codec->dev, "Using cached DC servo offset %x\n", - hubs->dac_hp_direct_dcs); - snd_soc_write(codec, dcs_reg, hubs->dac_hp_direct_dcs); + if (wm_hubs_dac_hp_direct(codec) && + wm_hubs_dcs_cache_get(codec, &cache)) { + dev_dbg(codec->dev, "Using cached DCS offset %x for %d,%d\n", + cache->dcs_cfg, cache->left, cache->right); + snd_soc_write(codec, dcs_reg, cache->dcs_cfg); wait_for_dc_servo(codec, WM8993_DCS_TRIG_DAC_WR_0 | WM8993_DCS_TRIG_DAC_WR_1); @@ -241,8 +299,8 @@ static void calibrate_dc_servo(struct snd_soc_codec *codec) /* Save the callibrated offset if we're in class W mode and * therefore don't have any analogue signal mixed in. */ - if (wm_hubs_dac_hp_direct(codec) && !hubs->no_cache_dac_hp_direct) - hubs->dac_hp_direct_dcs = dcs_cfg; + if (wm_hubs_dac_hp_direct(codec)) + wm_hubs_dcs_cache_set(codec, dcs_cfg); } /* @@ -257,9 +315,6 @@ static int wm8993_put_dc_servo(struct snd_kcontrol *kcontrol, ret = snd_soc_put_volsw(kcontrol, ucontrol); - /* Updating the analogue gains invalidates the DC servo cache */ - hubs->dac_hp_direct_dcs = 0; - /* If we're applying an offset correction then updating the * callibration would be likely to introduce further offsets. */ if (hubs->dcs_codes_l || hubs->dcs_codes_r || hubs->no_series_update) @@ -1057,6 +1112,7 @@ int wm_hubs_add_analogue_routes(struct snd_soc_codec *codec, struct wm_hubs_data *hubs = snd_soc_codec_get_drvdata(codec); struct snd_soc_dapm_context *dapm = &codec->dapm; + INIT_LIST_HEAD(&hubs->dcs_cache); init_completion(&hubs->dcs_done); snd_soc_dapm_add_routes(dapm, analogue_routes, diff --git a/sound/soc/codecs/wm_hubs.h b/sound/soc/codecs/wm_hubs.h index 71861fc..da2dc89 100644 --- a/sound/soc/codecs/wm_hubs.h +++ b/sound/soc/codecs/wm_hubs.h @@ -16,6 +16,7 @@ #include <linux/completion.h> #include <linux/interrupt.h> +#include <linux/list.h> #include <sound/control.h> struct snd_soc_codec; @@ -32,7 +33,7 @@ struct wm_hubs_data { int no_series_update; bool no_cache_dac_hp_direct; - u16 dac_hp_direct_dcs; + struct list_head dcs_cache; bool (*check_class_w_digital)(struct snd_soc_codec *); bool lineout1_se; -- 1.7.10

1 0

Re: [alsa-devel] ASoC: wm9712: Microphone doesn't work, "Capture Volume" inverted
by Mark Brown 01 May '12

01 May '12

On Tue, May 01, 2012 at 01:44:58AM +0200, Hans J. Koch wrote: > On Mon, Apr 30, 2012 at 10:43:33PM +0100, Mark Brown wrote: > > I'm not entirely sure you've looked at the code here... > Oh yes, sorry. I got distracted by your mentioning of the suppressed writes. > Actually, we have suppressed reads. ac97_read() does a "return cache[reg];" > for most registers, assuming the hardware register cannot change after a > ac97_write(). That might or might not be true, to me it looks dangerous. The driver knows exactly which registers are volatile and takes advantage of this - this is entirely idiomatic for devices like this and not really that hard to get right.

1 0

[alsa-devel] davinci-mcasp - external MCLK, CPU generates WCLK and BCLK, slave codec(s), full duplex issues
by Jérôme Carretero 01 May '12

01 May '12

Hi, Note: I opened a thread on TI's E2E forums too, the thread is at: http://e2e.ti.com/support/dsp/omap_applications_processors/f/42/t/185810.as… Linux is v3.3. Repeating what I have put there: - As we will be using multiple codecs on McASP, we have put the codecs as slaves. We are also using a 8.192MHz external master clock. - We have patched davinci-mcasp.c to handle this mode ("SND_SOC_DAIFMT_EM_CBS_CFS"), and refer to it for the CPU DAI configuration in the machine driver. - Our setup currently has an AIC3X codec, connected to the TX clocks of McASP. The RX clocks are not used, they are left hanging, so we need to enable the TX clocks even when recording. - We only plan to support 32kHz, so currently we have left an ugly hack in the code. - It almost works, but we have a nasty problem: - When performing a capture alone, the sound is recorded as expected - When performing a playback alone, no problem - When starting a playback then a capture, no problem - When starting a capture then a playback, the playback will hang, the stream won't accept more data; no perturbation on the recording side. A review of the McASP code WRT/ SPRUH77A (OMAP-L138 Technical Reference Manual) does not really help. Now, I performed some tracing: - patched the davinci-mcasp.c code, to trace register accesses See attached file. The patch also contains the differences explained on the e2e forum post. - produced the "yes" file by running the "playback then capture" scenario: dmesg -c cat /dev/urandom | pv -B 3200 | aplay -f S16 -r 32000 aplay -B 20000 -f S16 -r 32000 -C -V mono /dev/null dmesg -c | gzip > yes.gz - produced the "no" file by running the "capture then playback" scenario: dmesg -c aplay -B 20000 -f S16 -r 32000 -C -V mono /dev/null cat /dev/urandom | pv -B 3200 | aplay -f S16 -r 32000 dmesg -c | gzip > no.gz And I still have issues seing what could be wrong. Could you somehow help me ? Best regards, -- cJ PS: sorry for the resend, the initial message exceeded the allowed mailing list message size.

1 0

[alsa-devel] [PATCH] s3c2412-i2s: Fix dai registration
by Heiko Stübner 01 May '12

01 May '12

As s3c2412-i2s is using the s3c_i2sv2 it should call the more specialised s3c_i2sv2_register_dai instead of simply calling snd_soc_register_dai. Without this call the snd_soc_dai_ops structure isn't initialised correctly. Signed-off-by: Heiko Stuebner <heiko(a)sntech.de> --- sound/soc/samsung/s3c2412-i2s.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/sound/soc/samsung/s3c2412-i2s.c b/sound/soc/samsung/s3c2412-i2s.c index 7218507..79fbeea 100644 --- a/sound/soc/samsung/s3c2412-i2s.c +++ b/sound/soc/samsung/s3c2412-i2s.c @@ -166,7 +166,7 @@ static struct snd_soc_dai_driver s3c2412_i2s_dai = { static __devinit int s3c2412_iis_dev_probe(struct platform_device *pdev) { - return snd_soc_register_dai(&pdev->dev, &s3c2412_i2s_dai); + return s3c_i2sv2_register_dai(&pdev->dev, -1, &s3c2412_i2s_dai); } static __devexit int s3c2412_iis_dev_remove(struct platform_device *pdev) -- 1.7.5.4

2 2