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

[alsa-devel] [PATCH] ASoC: Avoid a redundant read in cs42l51_pdn_event
by Axel Lin 08 Nov '11

08 Nov '11

snd_soc_update_bits already does read-modify-write, no need to read the register before calling snd_soc_update_bits. Signed-off-by: Axel Lin <axel.lin(a)gmail.com> --- sound/soc/codecs/cs42l51.c | 13 +++++-------- 1 files changed, 5 insertions(+), 8 deletions(-) diff --git a/sound/soc/codecs/cs42l51.c b/sound/soc/codecs/cs42l51.c index 8c3c820..00718b5 100644 --- a/sound/soc/codecs/cs42l51.c +++ b/sound/soc/codecs/cs42l51.c @@ -175,21 +175,18 @@ static const struct snd_kcontrol_new cs42l51_snd_controls[] = { static int cs42l51_pdn_event(struct snd_soc_dapm_widget *w, struct snd_kcontrol *kcontrol, int event) { - unsigned long value; - - value = snd_soc_read(w->codec, CS42L51_POWER_CTL1); - value &= ~CS42L51_POWER_CTL1_PDN; - switch (event) { case SND_SOC_DAPM_PRE_PMD: - value |= CS42L51_POWER_CTL1_PDN; + snd_soc_update_bits(w->codec, CS42L51_POWER_CTL1, + CS42L51_POWER_CTL1_PDN, + CS42L51_POWER_CTL1_PDN); break; default: case SND_SOC_DAPM_POST_PMD: + snd_soc_update_bits(w->codec, CS42L51_POWER_CTL1, + CS42L51_POWER_CTL1_PDN, 0); break; } - snd_soc_update_bits(w->codec, CS42L51_POWER_CTL1, - CS42L51_POWER_CTL1_PDN, value); return 0; } -- 1.7.5.4

2 1

[alsa-devel] [PATCH] ASoC: fsi: fixup compile warning
by Kuninori Morimoto 08 Nov '11

08 Nov '11

This patch fixup below warning ${linux}/sound/soc/sh/fsi.c:442:3:\ warning: passing argument 1 of '__fsi_reg_read' makes pointer\ from integer without a cast ${linux}/sound/soc/sh/fsi.c:517:3: \ warning: passing argument 1 of '__fsi_reg_write' makes pointer\ from integer without a cast ${linux}/sound/soc/sh/fsi.c:663:3: \ warning: passing argument 1 of '__fsi_reg_mask_set' makes pointer\ from integer without a cast Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx(a)renesas.com> --- sound/soc/sh/fsi.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c index a32fd16..809f083 100644 --- a/sound/soc/sh/fsi.c +++ b/sound/soc/sh/fsi.c @@ -234,13 +234,13 @@ static void __fsi_reg_mask_set(u32 __iomem *reg, u32 mask, u32 data) } #define fsi_reg_write(p, r, d)\ - __fsi_reg_write((u32)(p->base + REG_##r), d) + __fsi_reg_write((p->base + REG_##r), d) #define fsi_reg_read(p, r)\ - __fsi_reg_read((u32)(p->base + REG_##r)) + __fsi_reg_read((p->base + REG_##r)) #define fsi_reg_mask_set(p, r, m, d)\ - __fsi_reg_mask_set((u32)(p->base + REG_##r), m, d) + __fsi_reg_mask_set((p->base + REG_##r), m, d) #define fsi_master_read(p, r) _fsi_master_read(p, MST_##r) #define fsi_core_read(p, r) _fsi_master_read(p, p->core->r) -- 1.7.5.4

2 1

Re: [alsa-devel] [PATCH] Add SoundCard driver for OKI SEMICONDUCTOR ML7213 IOH
by Mark Brown 08 Nov '11

08 Nov '11

On Tue, Nov 08, 2011 at 06:03:53PM +0900, Tomoya MORINAGA wrote: > Though roughly, we re-created 3 files codec driver, platform drive > and machine driver. (Not debugged/tested) > Could you check these files ? Please send patches for review using the standard kernel review process - see Documentation/SubmittingPatches. Sending a single enormous review makes it much more difficult for people to review as there's so much code in the one mail and attachments don't work with people's normal workflows.

1 0

Re: [alsa-devel] [PATCH 1/1] ALSA: intel8x0: Improve comments for VM optimization
by Takashi Iwai 08 Nov '11

08 Nov '11

At Mon, 7 Nov 2011 20:33:25 +0400, Denis V. Lunev wrote: > > The recently merged 228cf79376f1 looks a bit hackish while it is not. > The change was quite simple. In a virtualized environment the > patch unhacks old kludge introduced for old broken AC97 hardware. > > This patch adds proper comment to "unkludge" code. > > Signed-off-by: Denis V. Lunev <den(a)openvz.org> > Signed-off-by: Konstantin Ozerkov <kozerkov(a)parallels.com> Applied now. Thanks. Takashi > --- > sound/pci/intel8x0.c | 7 +++++++ > 1 files changed, 7 insertions(+), 0 deletions(-) > > diff --git a/sound/pci/intel8x0.c b/sound/pci/intel8x0.c > index 29e3125..c3b9bd0 100644 > --- a/sound/pci/intel8x0.c > +++ b/sound/pci/intel8x0.c > @@ -1077,6 +1077,13 @@ static snd_pcm_uframes_t snd_intel8x0_pcm_pointer(struct snd_pcm_substream *subs > } > if (civ != igetbyte(chip, ichdev->reg_offset + ICH_REG_OFF_CIV)) > continue; > + > + /* IO read operation is very expensive inside virtual machine > + * as it is emulated. The probability that subsequent PICB read > + * will return different result is high enough to loop till > + * timeout here. > + * Same CIV is strict enough condition to be sure that PICB > + * is valid inside VM on emulated card. */ > if (chip->inside_vm) > break; > if (ptr1 == igetword(chip, ichdev->reg_offset + ichdev->roff_picb)) > -- > 1.7.4.4 >

1 0

[alsa-devel] ALSA Snapshot alsa-utils-1.0.24.2.25.g9fa7c No user enum control support in alsa-lib
by b0h3m4 07 Nov '11

07 Nov '11

Hi there I'm trying to compile snapshot of ALSA packages on Oneiric x86 for via8235 >alsa-driver-1.0.24.72.g7c48f.1314.geb1d0.tar.bz2 >alsa-lib-1.0.24.1.46.g15c6f.tar.bz2 >alsa-utils-1.0.24.2.25.g9fa7c.tar.bz2 First two packages go without a hitch on alsa-utils I'm stuck at >checking for snd_ctl_elem_add_enumerated... no >configure: error: No user enum control support in alsa-lib Anything I should be aware of?

2 1

Re: [alsa-devel] Wierd hack to sound/pci/intel8x0.c
by Takashi Iwai 07 Nov '11

07 Nov '11

At Mon, 07 Nov 2011 11:35:49 +0100, Gerd Hoffmann wrote: > > Hi, > > > We discuss Intel ICH/AC'97 (snd-intel8x0) here, but I hope that PCI SSID > > is same. > > Hmm, it's not, the ac97 emulation doesn't use the default qemu subsystem > id for some reason. Here is the entry: > > [root@fedora ~]# lspci -vns6 > 00:06.0 0401: 8086:2415 (rev 01) > Subsystem: 8086:0000 > Physical Slot: 6 > Flags: bus master, medium devsel, latency 0, IRQ 10 > I/O ports at c000 [size=1K] > I/O ports at c400 [size=256] > Kernel driver in use: Intel ICH > Kernel modules: snd-intel8x0 > > I guess that should be fixed. 8086:0000 isn't valid anyway and I doubt > it serves any useful purpose other than avoiding a guest complaining > about the subsystem id being unset. Yeah, especially the value 0 looks pretty bad. > /me goes preparing a patch. Thanks! Takashi

1 0

Re: [alsa-devel] Wierd hack to sound/pci/intel8x0.c
by Takashi Iwai 07 Nov '11

07 Nov '11

At Mon, 07 Nov 2011 10:25:24 +0100, Gerd Hoffmann wrote: > > Hi, > > > Agreed. If we can know the virtual device for KVM in a better way > > (e.g. any specific PCI SSID or such), we can narrow the condition more > > safely. > > PCI Subsystem ID 1af4:1100 is qemu/kvm (not only Intel HDA, most other > emulated pci devices have it too). Complete entry: > > 00:04.0 0403: 8086:2668 (rev 01) > Subsystem: 1af4:1100 > Physical Slot: 4 > Flags: bus master, fast devsel, latency 0, IRQ 11 > Memory at e2030000 (32-bit, non-prefetchable) [size=16K] > Kernel driver in use: HDA Intel > Kernel modules: snd-hda-intel Thanks. BTW, this reminds me of a possible improvement in the current HD-audio driver. It has some workaround for the delayed DMA-position update (the DMA-position isn't completely updated when IRQ is issued) by offsetting the buffer position. This is controlled by bdl_pos_adj option of snd-hda-intel driver, and this could be zero for VMs. Takashi

1 0

Re: [alsa-devel] Wierd hack to sound/pci/intel8x0.c
by Takashi Iwai 06 Nov '11

06 Nov '11

At Sun, 06 Nov 2011 18:31:42 +0200, Avi Kivity wrote: > > On 11/06/2011 06:15 PM, Denis V. Lunev wrote: > > On 11/6/11 6:51 PM, Avi Kivity wrote: > >> The recently merged 228cf79376f1 ("ALSA: intel8x0: Improve performance > >> in virtual environment") is hacky and somewhat wrong. > >> > >> First, the detection code > >> > >> + if (inside_vm< 0) { > >> + /* detect KVM and Parallels virtual environments */ > >> + inside_vm = kvm_para_available(); > >> +#if defined(__i386__) || defined(__x86_64__) > >> + inside_vm = inside_vm || > >> boot_cpu_has(X86_FEATURE_HYPERVISOR); > >> +#endif > >> + } > >> + > >> > >> is incorrect. It detects that you're running in a guest, but that > >> doesn't imply that the device you're accessing is emulated. It may be a > >> host device assigned to the guest; presumably the optimization you apply > >> doesn't work for real devices. > >> > >> Second, the optimization itself looks fishy: > >> > >> spin_lock(&chip->reg_lock); > >> do { > >> civ = igetbyte(chip, ichdev->reg_offset + > >> ICH_REG_OFF_CIV); > >> ptr1 = igetword(chip, ichdev->reg_offset + > >> ichdev->roff_picb); > >> position = ichdev->position; > >> if (ptr1 == 0) { > >> udelay(10); > >> continue; > >> } > >> - if (civ == igetbyte(chip, ichdev->reg_offset + > >> ICH_REG_OFF_CIV)&& > >> - ptr1 == igetword(chip, ichdev->reg_offset + > >> ichdev->roff_picb)) > >> + if (civ != igetbyte(chip, ichdev->reg_offset + > >> ICH_REG_OFF_CIV)) > >> + continue; > >> + if (chip->inside_vm) > >> + break; > >> + if (ptr1 == igetword(chip, ichdev->reg_offset + > >> ichdev->roff_picb)) > >> break; > >> } while (timeout--); > >> > >> > >> Why is the emulated device timing out? Can't the emulation be fixed to > >> behave like real hardware? > >> > >> Last, please copy kvm(a)vger.kernel.org on such issues. > >> > > The problem is that emulation can not be fixed. > > > > How this is working for real hardware? You get data from real sound > > card register. > > The scheduling is off at the moment thus you can not be re-scheduled. > > > > In the virtual environment the situation is different. Any IO > > emulation is expensive. > > The processor is switched from guest to hypervisor and further to > > emulation process > > takes a lot of time. This time is enough to obtain different value on > > next register read. > > That's why this code is really timed out. Please also note that host > > scheduler also > > plays his games and could schedule out VCPU thread. > > > > Note on kvm this is rare, since the guest thread and the emulator thread > are the same. > > > The problem could be potentially fixed reducing precision of PICB > > emulation, > > but this results in lower sound quality. > > > > This kludge has been written this way in order not to break legacy > > card for which we > > do not have an access. The code reading PICB/CIV registers second time > > was added > > to fix issues on unknown for now platform and it looks not possible > > how to find/test > > against this platform now. We have checked Windows drivers written by > > Intel/AMD > > (32/64 bit) and MacOS ones. There is no second reading of CIV/PICB > > inside. We > > hope that this is relay needed only for some rare hadware devices. > > > > Ok, so if I understand correctly, this loop is a hack for broken > hardware, and this patch basically unhacks it back, assuming that the > emulated (or assigned) hardware is not broken. Exactly. The loop itself is still needed, but the check can be less restrictive. > > The only thing we can is to improve detection code. Suggestions are > > welcome. > > I think it's fine to assume that you're not assigning a 2004 era sound > card to a guest. So I think the code is fine as it is, and can only > suggest to add a comment explaining the mess. True. Denis, care to submit a patch? thanks, Takashi

1 0

Re: [alsa-devel] Wierd hack to sound/pci/intel8x0.c
by Takashi Iwai 06 Nov '11

06 Nov '11

At Sun, 6 Nov 2011 20:15:01 +0400, Denis V. Lunev wrote: > > On 11/6/11 6:51 PM, Avi Kivity wrote: > > The recently merged 228cf79376f1 ("ALSA: intel8x0: Improve performance > > in virtual environment") is hacky and somewhat wrong. > > > > First, the detection code > > > > + if (inside_vm< 0) { > > + /* detect KVM and Parallels virtual environments */ > > + inside_vm = kvm_para_available(); > > +#if defined(__i386__) || defined(__x86_64__) > > + inside_vm = inside_vm || > > boot_cpu_has(X86_FEATURE_HYPERVISOR); > > +#endif > > + } > > + > > > > is incorrect. It detects that you're running in a guest, but that > > doesn't imply that the device you're accessing is emulated. It may be a > > host device assigned to the guest; presumably the optimization you apply > > doesn't work for real devices. > > > > Second, the optimization itself looks fishy: > > > > spin_lock(&chip->reg_lock); > > do { > > civ = igetbyte(chip, ichdev->reg_offset + ICH_REG_OFF_CIV); > > ptr1 = igetword(chip, ichdev->reg_offset + > > ichdev->roff_picb); > > position = ichdev->position; > > if (ptr1 == 0) { > > udelay(10); > > continue; > > } > > - if (civ == igetbyte(chip, ichdev->reg_offset + > > ICH_REG_OFF_CIV)&& > > - ptr1 == igetword(chip, ichdev->reg_offset + > > ichdev->roff_picb)) > > + if (civ != igetbyte(chip, ichdev->reg_offset + > > ICH_REG_OFF_CIV)) > > + continue; > > + if (chip->inside_vm) > > + break; > > + if (ptr1 == igetword(chip, ichdev->reg_offset + > > ichdev->roff_picb)) > > break; > > } while (timeout--); > > > > > > Why is the emulated device timing out? Can't the emulation be fixed to > > behave like real hardware? > > > > Last, please copy kvm(a)vger.kernel.org on such issues. > > > The problem is that emulation can not be fixed. > > How this is working for real hardware? You get data from real sound card > register. > The scheduling is off at the moment thus you can not be re-scheduled. > > In the virtual environment the situation is different. Any IO emulation > is expensive. > The processor is switched from guest to hypervisor and further to > emulation process > takes a lot of time. This time is enough to obtain different value on > next register read. > That's why this code is really timed out. Please also note that host > scheduler also > plays his games and could schedule out VCPU thread. > > The problem could be potentially fixed reducing precision of PICB emulation, > but this results in lower sound quality. > > This kludge has been written this way in order not to break legacy card > for which we > do not have an access. The code reading PICB/CIV registers second time > was added > to fix issues on unknown for now platform and it looks not possible how > to find/test > against this platform now. We have checked Windows drivers written by > Intel/AMD > (32/64 bit) and MacOS ones. There is no second reading of CIV/PICB > inside. We > hope that this is relay needed only for some rare hadware devices. > > The only thing we can is to improve detection code. Suggestions are welcome. Agreed. If we can know the virtual device for KVM in a better way (e.g. any specific PCI SSID or such), we can narrow the condition more safely. thanks, Takashi

1 0

[alsa-devel] [PATCH] ALSA: control: remove compilation warning on 32-bit
by Olof Johansson 05 Nov '11

05 Nov '11

This was introduced by 'ALSA: control: add support for ENUMERATED user space controls' which adds a u64 variable that gets cast to a pointer: sound/core/control.c: In function 'snd_ctl_elem_init_enum_names': sound/core/control.c:1089: warning: cast to pointer from integer of different size Since we don't support 64-bit userspace on 32-bit kernel, it's safe to cast to long before casting to pointer to avoid the warning. Signed-off-by: Olof Johansson <olof(a)lixom.net> --- sound/core/control.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sound/core/control.c b/sound/core/control.c index 978fe1a..d09c335 100644 --- a/sound/core/control.c +++ b/sound/core/control.c @@ -1081,12 +1081,12 @@ static int snd_ctl_elem_init_enum_names(struct user_element *ue) char *names, *p; size_t buf_len, name_len; unsigned int i; + const long user_ptrval = ue->info.value.enumerated.names_ptr; if (ue->info.value.enumerated.names_length > 64 * 1024) return -EINVAL; - names = memdup_user( - (const void __user *)ue->info.value.enumerated.names_ptr, + names = memdup_user((const void __user *)user_ptrval, ue->info.value.enumerated.names_length); if (IS_ERR(names)) return PTR_ERR(names); -- 1.7.4.1

2 1