June 2011 - Alsa-devel - mailman.alsa-project.org

[alsa-devel] alsa-lib rate plugins - float samples
by Pavel Hofman 01 Jul '11

01 Jul '11

Hi, This is probably a question for Jaroslav Kysela. The better-quality rate plugins offer float resolution, but currently we are deliberately loosing information by down-converting to s16 in plugin_rate API (convert_s16). In rate_samplerate.c, the samples are up-converted to float before calling the libsamplerate API. Speex offers float API too, while we are using the int16 one. There are two ways to handle this: A. Implementing the generic "convert" API function for these plugins, handling the sample - float conversion in each plugin individually. B. Extending the pcm_rate API (pcm_rate.h) with a new optional function convert_float. Since the conversion from general sample format to float is not trivial (using the plugin_ops.h operations), I would be inclined to suggest the extension (well I have it hacked already :-) ). On the other hand, the conversion routine can be located in some common code shared by all the plugins involved and we could keep the existing API, using the generic convert function. Thanks a lot for suggestions. Best regards, Pavel.

2 5

Re: [alsa-devel] [PATCH 1/5] ASoC: wm8731: rework power management
by Mark Brown 01 Jul '11

01 Jul '11

On Wed, Jun 29, 2011 at 08:00:15PM +0200, Nicolas Ferre wrote: Don't mix multiple changes into a single patch! There's no perceptible code overlap between these so I don't understand why you've merged them, it just makes review harder and the changelog less descriptive. > - preserve crystal oscillator across suspend/resume sequence: > enabled by default, it should be kept enabled on resume. This isn't what your code does... > - if codec is in active state: set the active bit at resume time. Your changelog doesn't explain what the actual problem is here... I can see it by studying the diff but the changelog makes me think the change is bad as the subsystem shuts everything down before it enters CODEC suspend... > snd_soc_write(codec, WM8731_ACTIVE, 0x0); > - snd_soc_write(codec, WM8731_PWR, 0xffff); > + /* standby: keep crystal oscillator enabled */ > + snd_soc_write(codec, WM8731_PWR, 0x00df); This doesn't keep the crystal oscillator enabled, this forces it on in suspend (and without looking at the datasheet it also changes way more than the one register bit I'd expect to be changed). If the system isn't using the oscillator then that's not good. I'd expect to see a change to using snd_soc_update_bits() based on your description, or more likely something more involved. > @@ -533,6 +534,8 @@ static int wm8731_suspend(struct snd_soc_codec *codec, pm_message_t state) > static int wm8731_resume(struct snd_soc_codec *codec) > { > wm8731_set_bias_level(codec, SND_SOC_BIAS_STANDBY); > + if (codec->active) > + snd_soc_write(codec, WM8731_ACTIVE, 0x0001); This should be handled as part of the bias level managemnent and cache sync, if the disable happens in bias level management so should the restore.

2 4

[alsa-devel] [PATCH] ASoC: Improve error reporting in Speyside WM8962 driver
by Mark Brown 30 Jun '11

30 Jun '11

Signed-off-by: Mark Brown <broonie(a)opensource.wolfsonmicro.com> --- sound/soc/samsung/speyside_wm8962.c | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-) diff --git a/sound/soc/samsung/speyside_wm8962.c b/sound/soc/samsung/speyside_wm8962.c index c0ba0bf..55233e8 100644 --- a/sound/soc/samsung/speyside_wm8962.c +++ b/sound/soc/samsung/speyside_wm8962.c @@ -30,14 +30,16 @@ static int speyside_wm8962_set_bias_level(struct snd_soc_card *card, WM8962_FLL_MCLK, 32768, 44100 * 256); if (ret < 0) - pr_err("Failed to start FLL\n"); + pr_err("Failed to start FLL: %d\n"); ret = snd_soc_dai_set_sysclk(codec_dai, WM8962_SYSCLK_FLL, 44100 * 256, SND_SOC_CLOCK_IN); - if (ret < 0) + if (ret < 0) { + pr_err("Failed to set SYSCLK: %d\n"); return ret; + } } break; @@ -59,13 +61,15 @@ static int speyside_wm8962_set_bias_level_post(struct snd_soc_card *card, case SND_SOC_BIAS_STANDBY: ret = snd_soc_dai_set_sysclk(codec_dai, WM8962_SYSCLK_MCLK, 32768, SND_SOC_CLOCK_IN); - if (ret < 0) + if (ret < 0) { + pr_err("Failed to switch away from FLL: %d\n", ret); return ret; + } ret = snd_soc_dai_set_pll(codec_dai, WM8962_FLL, 0, 0, 0); if (ret < 0) { - pr_err("Failed to stop FLL\n"); + pr_err("Failed to stop FLL: %d\n", ret); return ret; } break; -- 1.7.5.4

3 4

[alsa-devel] Via Codecs: playback_multi_pcm_prep_0() might do something unneded in redirected mode - or something else could be wrong.
by alex dot baldacchino dot alsasub at gmail dot com 30 Jun '11

30 Jun '11

I've got a doubt about the following piece of code: ... if (mout->hp_nid && mout->hp_nid != nids[HDA_FRONT] && !spec->hp_independent_mode) /* headphone out will just decode front left/right (stereo) */ snd_hda_codec_setup_stream(codec, mout->hp_nid, stream_tag, 0, format); In redirected mode, if mout->hp_nid is not the same as the front dac, headphone jack (spec->autocfg.hp_pins[0]) is connected to it (get inputs from the front dac) one way or the other, so it would seem useless to make a different dac work on the same stream, given its output is being, let me say, 'discarded'. Moreover, via_independent_hp_put() always cleans that stream up for hp_nid in similar conditions: ... if (spec->multiout.hp_nid && spec->multiout.hp_nid != spec->multiout.dac_nids[HDA_FRONT]) snd_hda_codec_setup_stream(codec, spec->multiout.hp_nid, 0, 0, 0); This means that switching Independent HP mode on and off while playing something would prevent the dac corresponding to hp_nid from decoding that stream any further, whereas a simple printk tells me playback_multi_pcm_prep_0 is called just once at the beginning of the streaming. A simple test showed me that piece of code isn't needed in my case (VT2020) and I'd guess that's the same for most codecs, if not all (not yet tested commenting out that block, though): starting with Independent HP mode 'ON', that block isn't executed (!spec->hp_independent_mode is false), but switching it to 'OFF' while playing I get sound out of my headset connected to my case front audio panel; I can do it repeatedly and always works. Though not extremely frequent, I think such a scenario could happen in "real world use", for instance, I can think of someone turning off Independent HP mode while playing, then turning off a speaker system because of someone complaining for loud sound (or asking for silence), or just by mistake, because thinking redirected mode were active while it wasn't. Therefore, if there were any codecs requiring for stream being setup for hp_nid while in redirected mode, to work properly, that could be made clear in playback_multi_pcm_prep_0(), just to make the code a bit clearer, and, most important, via_independent_hp_put should be corrected to take that into account (and perhaps infos on current stream(s) should be recorded somewhere, to do that). Regards.

1 0

[alsa-devel] Via Codecs: functions mute_aa_path() and is_aa_path_mute() (file patch_via.c) seem to support different codecs.
by alex dot baldacchino dot alsasub at gmail dot com 30 Jun '11

30 Jun '11

These functions seem to support different codecs through different switch statements: mute_aa_path: ... /* get nid of MW0 and start & end index */ switch (spec->codec_type) { case VT1708: nid_mixer = 0x17; start_idx = 2; end_idx = 4; break; case VT1709_10CH: case VT1709_6CH: nid_mixer = 0x18; start_idx = 2; end_idx = 4; break; case VT1708B_8CH: case VT1708B_4CH: case VT1708S: case VT1716S: nid_mixer = 0x16; start_idx = 2; end_idx = 4; break; case VT1718S: nid_mixer = 0x21; start_idx = 1; end_idx = 3; break; default: return; } is_aa_path_mute: ... /* get nid of MW0 and start & end index */ switch (spec->codec_type) { case VT1708B_8CH: case VT1708B_4CH: case VT1708S: case VT1716S: nid_mixer = 0x16; start_idx = 2; end_idx = 4; break; case VT1702: nid_mixer = 0x1a; start_idx = 1; end_idx = 3; break; case VT1718S: nid_mixer = 0x21; start_idx = 1; end_idx = 3; break; case VT2002P: case VT1812: case VT1802: nid_mixer = 0x21; start_idx = 0; end_idx = 2; break; default: return 0; } It would seem that AA-path can be muted but not inquired for mute state for codecs VT1708, VT1709_10CH and VT1709_6CH, vice-versa for codecs VT1702, VT2002P, VT1812, VT1802. If this is the real way such codecs work, I wonder if analog_input_switch_put() works as expected for codec VT1708, in which case, perhaps, it should be modified: static int analog_input_switch_put(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { int change = snd_hda_mixer_amp_switch_put(kcontrol, ucontrol); struct hda_codec *codec = snd_kcontrol_chip(kcontrol); set_widgets_power_state(codec); analog_low_current_mode(snd_kcontrol_chip(kcontrol), -1); if (snd_hda_get_bool_hint(codec, "analog_loopback_hp_detect") == 1) { if (is_aa_path_mute(codec)) vt1708_start_hp_work(codec->spec); else vt1708_stop_hp_work(codec->spec); } return change; } Since is_aa_path_mute() always returns 0 for codec VT1708 (default case), and both vt1708_start_hp_work() and vt1708_stop_hp_work() only execute for such a codec (due to an if statement checking for codec type and returning from the function if that's not VT1708), vt1708_stop_hp_work() is the only one that can be executed, so perhaps the code could be clearer without the if statement checking is_aa_path_mute() result. Moreover: static void vt1708_stop_hp_work(struct via_spec *spec) { if (spec->codec_type != VT1708 || spec->autocfg.hp_pins[0] == 0) return; if (snd_hda_get_bool_hint(spec->codec, "analog_loopback_hp_detect") == 1 && !is_aa_path_mute(spec->codec)) return; snd_hda_codec_write(spec->codec, 0x1, 0, 0xf81, !spec->vt1708_jack_detectect); cancel_delayed_work_sync(&spec->vt1708_hp_work); } Since !is_aa_path_mute(spec->codec) is always true for VT1708, there is no real dependence on it in the if statement above, thus it should be removed. But this also means that, if snd_hda_get_bool_hint(spec->codec, "analog_loopback_hp_detect") == 1, then analog_input_switch_put() will call vt1708_stop_hp_work(), which in turn will return without performing any operation, so that it would seem useless, unless any reconfiguration might happen just in time between the first call to snd_hda_get_bool_hint in analog_input_switch_put() and the second call within vt1708_stop_hp_work(). Otherwise, if those switch statements should match against each other, they could be replaced with a call to a new inline function, so that a single switch could be moved within it and become easier to maintain. For instance: static inline int get_supported_aa_mute_info( const struct via_spec * const spec, hda_nid_t *mixer, int *first_idx, int *last_idx ) { /* get correct nid of mixer and start & end index */ switch (spec->codec_type) { case VT1708: *mixer = 0x17; *first_idx = 2; *last_idx = 4; return 0; case VT1709_10CH: case VT1709_6CH: *mixer = 0x18; *first_idx = 2; *last_idx = 4; return 0; case VT1708B_8CH: case VT1708B_4CH: case VT1708S: case VT1716S: *mixer = 0x16; *first_idx = 2; *last_idx = 4; return 0; case VT1702: *mixer = 0x1a; *first_idx = 1; *last_idx = 3; return 0; case VT1718S: *mixer = 0x21; *first_idx = 1; *last_idx = 3; return 0; case VT2002P: case VT1812: case VT1802: *mixer = 0x21; *first_idx = 0; *last_idx = 2; return 0; default: return -EINVAL; } } In mute_aa_path: ... if( get_supported_aa_mute_info( spec, &nid_mixer, &start_idx, &end_idx ) < 0 ) return; In is_aa_path_mute: ... if( get_supported_aa_mute_info( spec, &nid_mixer, &start_idx, &end_idx ) < 0 ) return 0; It happens that the list of codec types tested by is_aa_path_mute() is the same tested by function analog_low_current_mode(); however, there should be no dependence on that, because whatever is_aa_path_mute() returned for any other codec would be filtered out by the switch statement within analog_low_current_mode(). Regards.

1 0

[alsa-devel] MOTU Midi Express XT driver
by Torsten Schenk 30 Jun '11

30 Jun '11

Hello all, I'm currently trying to write a driver for the MOTU Midi Express XT. The problem I have is that the device has same idVendor and idProduct as the MOTU FastLane Quad MIDI Interface (for which already a driver exists in snd-usbmidi). If I do an lsusb, I see that the field iProduct identifies the device as a Midi Express XT. But I found no way of matching to iProduct in the usb_device_id structure. Is there a possibility to do so? Otherwise I'm thinking of patching snd-usbmidi in a way that it differentiates between the devices there. Thanks, Torsten Here the lsusb output: Bus 004 Device 003: ID 07fd:0001 Mark of the Unicorn FastLane Quad MIDI Interface Device Descriptor: bLength 18 bDescriptorType 1 bcdUSB 1.00 bDeviceClass 255 Vendor Specific Class bDeviceSubClass 1 bDeviceProtocol 0 bMaxPacketSize0 8 idVendor 0x07fd Mark of the Unicorn idProduct 0x0001 FastLane Quad MIDI Interface bcdDevice 1.32 iManufacturer 0 iProduct 119 MIDI Express XT iSerial 0 bNumConfigurations 1 ...

2 3

[alsa-devel] Add combined clock support to Atmel SoC audio
by Michele Da Rold 30 Jun '11

30 Jun '11

I have found this post on the network: http://www.spinics.net/lists/alsa-devel/msg40008.html there is a work on this theme or not? thanks michele

1 0

[alsa-devel] problems on AT91 with SSC not generating frame sync on RF0 pin
by Michele Da Rold 30 Jun '11

30 Jun '11

I have the same problem posted on this mailing list on july 2009 problems on AT91 with SSC not generating frame sync on RF0 pin (http://www.spinics.net/lists/alsa-devel/msg25692.html) someone have a suggestion for the problem? thank you Michele

1 0

Re: [alsa-devel] SB16 build error.
by Ralf Baechle 30 Jun '11

30 Jun '11

On Thu, Jun 30, 2011 at 01:28:03PM +0200, Takashi Iwai wrote: > > > I have no idea how big the soundblaster microcode being loaded actually is, > > > that is if the reduced size of 0x1f00 will be sufficient. > > > > The files found in /lib/firmware/sb16 are all under 2kB, thus likely > > sufficient. > > Too shortly answered. It turned out that some CSP codes (like Qsound) > can be above that size, it's almost 12kB. So the size in the original > code is really the necessary requirement, and the patch breaks for > such a case. > > An ugly workaround would be to fake the ioctl size. But this is > certainly to be avoided, since it has been broken on the specific > platforms for ages, thus breaking for them would be mostly harmless, > too. > > > > Aside of that I > > > don't see a problem - I don't see how the old ioctl can possibly have been > > > used before so there isn't a compatibility problem. > > > > > > Or you could entirely sidestep the problem and use request_firmware() but > > > I guess that's more effort than you want to invest. > > > > Yeah, that's another option I thought of. But it's too intrusive for > > 3.0-rc6, so I'd like waive it for 3.1. > > Actually the request_firmware() was implemented for some auto-loadable > CSP codes. Others need the manual loading, so it is via ioctl. It > can be converted, but I don't think it makes sense for such old > stuff. After all, it still works with x86-ISA as is. In userland an empty definition will be used for _IOC_TYPECHECK so there won't be an error. So userland already is already using the existing value for SNDRV_SB_CSP_IOCTL_LOAD_CODE ... With a crude hack like #define SNDRV_SB_CSP_IOCTL_LOAD_CODE \ _IOC(_IOC_WRITE,'H', 0x11, sizeof(struct snd_sb_csp_microcode)) error checking can be bypassed and all will be fine as long as the resulting value doesn't result in in a a duplicate case value - which it doesn't, at least not in my testing. Should work but isn't nice. Ralf

3 4

Re: [alsa-devel] SB16 build error.
by Ralf Baechle 30 Jun '11

30 Jun '11

On Thu, Jun 30, 2011 at 12:15:56PM +0200, Takashi Iwai wrote: > At Thu, 30 Jun 2011 10:17:54 +0100, > Ralf Baechle wrote: > > > > Found on a MIPS build but certain other architectures will have the same > > issue: > > > > CC sound/isa/sb/sb16_csp.o > > sound/isa/sb/sb16_csp.c: In function ‘snd_sb_csp_ioctl’: > > sound/isa/sb/sb16_csp.c:228: error: case label does not reduce to an integer constant > > make[1]: *** [sound/isa/sb/sb16_csp.o] Error 1 > > make: *** [sound/isa/sb/sb16_csp.o] Error 2 > > > > This error message is caused by the _IOC_TYPECHECK() error check triggering > > due to excessive ioctl size on Alpha, PowerPC, MIPS and SPARC which define > > _IOC_SIZEBITS as 13. On all other architectures define it as 14 so struct > > snd_sb_csp_microcode with it's little over 12kB will just about fit into > > the 16kB limit. > > What about the patch below? I have no idea how big the soundblaster microcode being loaded actually is, that is if the reduced size of 0x1f00 will be sufficient. Aside of that I don't see a problem - I don't see how the old ioctl can possibly have been used before so there isn't a compatibility problem. Or you could entirely sidestep the problem and use request_firmware() but I guess that's more effort than you want to invest. > This is an old ISA driver, so the impact must be very low. True. I notice that sort of stuff in automated mass builds - not necessarily the sort of kernels one would actually use. Still build errors are annoying :) Ralf

2 1