October 2021 - Alsa-devel - mailman.alsa-project.org

[PATCH] sh: Use modern ASoC DAI format terminology
by Mark Brown 11 Oct '21

11 Oct '21

The SH machine drivers have some ASoC DAI format specifications that use older defines based on outdated terminology which we're trying to retire, update to the new bindings. Signed-off-by: Mark Brown <broonie(a)kernel.org> --- arch/sh/boards/mach-ecovec24/setup.c | 2 +- arch/sh/boards/mach-se/7724/setup.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/sh/boards/mach-ecovec24/setup.c b/arch/sh/boards/mach-ecovec24/setup.c index bab91a99124e..97c5703b1818 100644 --- a/arch/sh/boards/mach-ecovec24/setup.c +++ b/arch/sh/boards/mach-ecovec24/setup.c @@ -886,7 +886,7 @@ static struct asoc_simple_card_info fsi_da7210_info = { .card = "FSIB-DA7210", .codec = "da7210.0-001a", .platform = "sh_fsi.0", - .daifmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_CBM_CFM, + .daifmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_CBP_CFP, .cpu_dai = { .name = "fsib-dai", }, diff --git a/arch/sh/boards/mach-se/7724/setup.c b/arch/sh/boards/mach-se/7724/setup.c index 8d6541ba0186..d9b31d4560c0 100644 --- a/arch/sh/boards/mach-se/7724/setup.c +++ b/arch/sh/boards/mach-se/7724/setup.c @@ -305,7 +305,7 @@ static struct asoc_simple_card_info fsi_ak4642_info = { .card = "FSIA-AK4642", .codec = "ak4642-codec.0-0012", .platform = "sh_fsi.0", - .daifmt = SND_SOC_DAIFMT_LEFT_J | SND_SOC_DAIFMT_CBM_CFM, + .daifmt = SND_SOC_DAIFMT_LEFT_J | SND_SOC_DAIFMT_CBP_CFP, .cpu_dai = { .name = "fsia-dai", }, -- 2.20.1

1 1

[PATCH] ALSA: pcm: Workaround for a wrong offset in SYNC_PTR compat ioctl
by Takashi Iwai 11 Oct '21

11 Oct '21

Michael Forney reported an incorrect padding type that was defined in the commit 80fe7430c708 ("ALSA: add new 32-bit layout for snd_pcm_mmap_status/control") for PCM control mmap data. His analysis is correct, and this caused the misplacements of PCM control data on 32bit arch and 32bit compat mode. The bug is that the __pad2 definition in __snd_pcm_mmap_control64 struct was wrongly with __pad_before_uframe, which should have been __pad_after_uframe instead. This struct is used in SYNC_PTR ioctl and control mmap. Basically this bug leads to two problems: - The offset of avail_min field becomes wrong, it's placed right after appl_ptr without padding on little-endian - When appl_ptr and avail_min are read as 64bit values in kernel side, the values become either zero or corrupted (mixed up) One good news is that, because both user-space and kernel misunderstand the wrong offset, at least, 32bit application running on 32bit kernel works as is. Also, 64bit applications are unaffected because the padding size is zero. The remaining problem is the 32bit compat mode; as mentioned in the above, avail_min is placed right after appl_ptr on little-endian archs, 64bit kernel reads bogus values for appl_ptr updates, which may lead to streaming bugs like jumping, XRUN or whatever unexpected. (However, we haven't heard any serious bug reports due to this over years, so practically seen, it's fairly safe to assume that the impact by this bug is limited.) Ideally speaking, we should correct the wrong mmap status control definition. But this would cause again incompatibility with the existing binaries, and fixing it (e.g. by renumbering ioctls) would be really messy. So, as of this patch, we only correct the behavior of 32bit compat mode and keep the rest as is. Namely, the SYNC_PTR ioctl is now handled differently in compat mode to read/write the 32bit values at the right offsets. The control mmap of 32bit apps on 64bit kernels has been already disabled (which is likely rather an overlook, but this worked fine at this time :), so covering SYNC_PTR ioctl should suffice as a fallback. Fixes: 80fe7430c708 ("ALSA: add new 32-bit layout for snd_pcm_mmap_status/control") Reported-by: Michael Forney <mforney(a)mforney.org> Cc: <stable(a)vger.kernel.org> Cc: Arnd Bergmann <arnd(a)arndb.de> Cc: Rich Felker <dalias(a)libc.org> Link: https://lore.kernel.org/r/29QBMJU8DE71E.2YZSH8IHT5HMH@mforney.org Signed-off-by: Takashi Iwai <tiwai(a)suse.de> --- sound/core/pcm_compat.c | 72 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 71 insertions(+), 1 deletion(-) diff --git a/sound/core/pcm_compat.c b/sound/core/pcm_compat.c index a59de24695ec..dfe5a64e19d2 100644 --- a/sound/core/pcm_compat.c +++ b/sound/core/pcm_compat.c @@ -468,6 +468,76 @@ static int snd_pcm_ioctl_sync_ptr_x32(struct snd_pcm_substream *substream, } #endif /* CONFIG_X86_X32 */ +#ifdef __BIG_ENDIAN +typedef char __pad_before_u32[4]; +typedef char __pad_after_u32[0]; +#else +typedef char __pad_before_u32[0]; +typedef char __pad_after_u32[4]; +#endif + +/* PCM 2.0.15 API definition had a bug in mmap control; it puts the avail_min + * at the wrong offset due to a typo in padding type. + * The bug hits only 32bit. + * A workaround for incorrect read/write is needed only in 32bit compat mode. + */ +struct __snd_pcm_mmap_control64_buggy { + __pad_before_u32 __pad1; + __u32 appl_ptr; + __pad_before_u32 __pad2; /* SiC! here is the bug */ + __pad_before_u32 __pad3; + __u32 avail_min; + __pad_after_uframe __pad4; +}; + +static int snd_pcm_ioctl_sync_ptr_buggy(struct snd_pcm_substream *substream, + struct snd_pcm_sync_ptr __user *_sync_ptr) +{ + struct snd_pcm_runtime *runtime = substream->runtime; + struct snd_pcm_sync_ptr sync_ptr; + struct __snd_pcm_mmap_control64_buggy *sync_cp; + volatile struct snd_pcm_mmap_status *status; + volatile struct snd_pcm_mmap_control *control; + int err; + + memset(&sync_ptr, 0, sizeof(sync_ptr)); + sync_cp = (struct __snd_pcm_mmap_control64_buggy *)&sync_ptr.c.control; + if (get_user(sync_ptr.flags, (unsigned __user *)&(_sync_ptr->flags))) + return -EFAULT; + if (copy_from_user(sync_cp, &(_sync_ptr->c.control), sizeof(*sync_cp))) + return -EFAULT; + status = runtime->status; + control = runtime->control; + if (sync_ptr.flags & SNDRV_PCM_SYNC_PTR_HWSYNC) { + err = snd_pcm_hwsync(substream); + if (err < 0) + return err; + } + snd_pcm_stream_lock_irq(substream); + if (!(sync_ptr.flags & SNDRV_PCM_SYNC_PTR_APPL)) { + err = pcm_lib_apply_appl_ptr(substream, sync_cp->appl_ptr); + if (err < 0) { + snd_pcm_stream_unlock_irq(substream); + return err; + } + } else { + sync_cp->appl_ptr = control->appl_ptr; + } + if (!(sync_ptr.flags & SNDRV_PCM_SYNC_PTR_AVAIL_MIN)) + control->avail_min = sync_cp->avail_min; + else + sync_cp->avail_min = control->avail_min; + sync_ptr.s.status.state = status->state; + sync_ptr.s.status.hw_ptr = status->hw_ptr; + sync_ptr.s.status.tstamp = status->tstamp; + sync_ptr.s.status.suspended_state = status->suspended_state; + sync_ptr.s.status.audio_tstamp = status->audio_tstamp; + snd_pcm_stream_unlock_irq(substream); + if (copy_to_user(_sync_ptr, &sync_ptr, sizeof(sync_ptr))) + return -EFAULT; + return 0; +} + /* */ enum { @@ -537,7 +607,7 @@ static long snd_pcm_ioctl_compat(struct file *file, unsigned int cmd, unsigned l if (in_x32_syscall()) return snd_pcm_ioctl_sync_ptr_x32(substream, argp); #endif /* CONFIG_X86_X32 */ - return snd_pcm_common_ioctl(file, substream, cmd, argp); + return snd_pcm_ioctl_sync_ptr_buggy(substream, argp); case SNDRV_PCM_IOCTL_HW_REFINE32: return snd_pcm_ioctl_hw_params_compat(substream, 1, argp); case SNDRV_PCM_IOCTL_HW_PARAMS32: -- 2.31.1

2 3

[PATCH] ASoC: rt1011: Fix 'I2S Reference' enum control caused error
by Peter Ujfalusi 11 Oct '21

11 Oct '21

Access to 'Is@ Reference' enum causes alsamixer to fail to load: $ alsamixer cannot load mixer controls: Invalid argument cml_rt1011_rt5682 cml_rt1011_rt5682: control 2:0:0:TL I2S Reference:0: access overflow The reason is that the original patch adding the code was using ucontrol->value.integer.value[0] instead the correct ucontrol->value.enumerated.item[0] for an ENUM control. Fixes: 87f40af26c262 ("ASoC: rt1011: add i2s reference control for rt1011") Reported-by: Seppo Ingalsuo <seppo.ingalsuo(a)linux.intel.com> Signed-off-by: Peter Ujfalusi <peter.ujfalusi(a)linux.intel.com> --- sound/soc/codecs/rt1011.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sound/soc/codecs/rt1011.c b/sound/soc/codecs/rt1011.c index 508597866dff..3adaff6f0141 100644 --- a/sound/soc/codecs/rt1011.c +++ b/sound/soc/codecs/rt1011.c @@ -1325,7 +1325,7 @@ static int rt1011_i2s_ref_put(struct snd_kcontrol *kcontrol, snd_soc_kcontrol_component(kcontrol); struct rt1011_priv *rt1011 = snd_soc_component_get_drvdata(component); - int i2s_ref_ch = ucontrol->value.integer.value[0]; + int i2s_ref_ch = ucontrol->value.enumerated.item[0]; switch (i2s_ref_ch) { case RT1011_I2S_REF_LEFT_CH: @@ -1344,7 +1344,7 @@ static int rt1011_i2s_ref_put(struct snd_kcontrol *kcontrol, dev_info(component->dev, "I2S Reference: Do nothing\n"); } - rt1011->i2s_ref = ucontrol->value.integer.value[0]; + rt1011->i2s_ref = ucontrol->value.enumerated.item[0]; return 0; } @@ -1357,7 +1357,7 @@ static int rt1011_i2s_ref_get(struct snd_kcontrol *kcontrol, struct rt1011_priv *rt1011 = snd_soc_component_get_drvdata(component); - ucontrol->value.integer.value[0] = rt1011->i2s_ref; + ucontrol->value.enumerated.item[0] = rt1011->i2s_ref; return 0; } -- 2.33.0

1 1

[PATCH v2] ASoC: rt1011: Fix 'I2S Reference' enum control caused error
by Peter Ujfalusi 11 Oct '21

11 Oct '21

Access to 'Is@ Reference' enum causes alsamixer to fail to load: $ alsamixer cannot load mixer controls: Invalid argument cml_rt1011_rt5682 cml_rt1011_rt5682: control 2:0:0:TL I2S Reference:0: access overflow The reason is that the original patch adding the code was using ucontrol->value.integer.value[0] instead the correct ucontrol->value.enumerated.item[0] for an ENUM control. Fixes: 87f40af26c262 ("ASoC: rt1011: add i2s reference control for rt1011") Reported-by: Seppo Ingalsuo <seppo.ingalsuo(a)linux.intel.com> Signed-off-by: Peter Ujfalusi <peter.ujfalusi(a)linux.intel.com> --- Hi, Changes since v1: - Correct the ENUM declaration as well Regards, Peter sound/soc/codecs/rt1011.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/sound/soc/codecs/rt1011.c b/sound/soc/codecs/rt1011.c index 508597866dff..bdfcbb81fa19 100644 --- a/sound/soc/codecs/rt1011.c +++ b/sound/soc/codecs/rt1011.c @@ -1311,12 +1311,11 @@ static int rt1011_r0_load_info(struct snd_kcontrol *kcontrol, .put = rt1011_r0_load_mode_put \ } -static const char * const rt1011_i2s_ref[] = { +static const char * const rt1011_i2s_ref_texts[] = { "None", "Left Channel", "Right Channel" }; -static SOC_ENUM_SINGLE_DECL(rt1011_i2s_ref_enum, 0, 0, - rt1011_i2s_ref); +static SOC_ENUM_SINGLE_EXT_DECL(rt1011_i2s_ref_enum, rt1011_i2s_ref_texts); static int rt1011_i2s_ref_put(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) @@ -1325,7 +1324,7 @@ static int rt1011_i2s_ref_put(struct snd_kcontrol *kcontrol, snd_soc_kcontrol_component(kcontrol); struct rt1011_priv *rt1011 = snd_soc_component_get_drvdata(component); - int i2s_ref_ch = ucontrol->value.integer.value[0]; + int i2s_ref_ch = ucontrol->value.enumerated.item[0]; switch (i2s_ref_ch) { case RT1011_I2S_REF_LEFT_CH: @@ -1344,7 +1343,7 @@ static int rt1011_i2s_ref_put(struct snd_kcontrol *kcontrol, dev_info(component->dev, "I2S Reference: Do nothing\n"); } - rt1011->i2s_ref = ucontrol->value.integer.value[0]; + rt1011->i2s_ref = ucontrol->value.enumerated.item[0]; return 0; } @@ -1357,7 +1356,7 @@ static int rt1011_i2s_ref_get(struct snd_kcontrol *kcontrol, struct rt1011_priv *rt1011 = snd_soc_component_get_drvdata(component); - ucontrol->value.integer.value[0] = rt1011->i2s_ref; + ucontrol->value.enumerated.item[0] = rt1011->i2s_ref; return 0; } -- 2.33.0

1 0

Re: Fwd: [PATCH 4/8] ASoC: amd: acp: Add generic machine driver support for ACP cards
by Ajit Kumar Pandey 11 Oct '21

11 Oct '21

Apologies for corrupted reply format, some issue with thunderbird. We will update all below review comments in v2 patch series > On Thu, Sep 30, 2021 at 06:54:14PM +0530, Ajit Kumar Pandey wrote: > > A couple of things here, most of these are probably fine for now > other than the EXPORT_SYMBOL but I think you're likely to run > into issues going forward and need to refactor. > >> + switch (drvdata->hs_codec_id) { >> + case RT5682: >> + pll_id = RT5682_PLL2; >> + pll_src = RT5682_PLL2_S_MCLK; >> + freq_in = PCO_PLAT_CLK; >> + freq_out = RT5682_PLL_FREQ; >> + clk_id = RT5682_SCLK_S_PLL2; >> + clk_freq = RT5682_PLL_FREQ; >> + wclk_name = "rt5682-dai-wclk"; >> + bclk_name = "rt5682-dai-bclk"; >> + drvdata->dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF >> + | SND_SOC_DAIFMT_CBP_CFP; >> + snd_soc_dapm_add_routes(&rtd->card->dapm, rt5682_map, ARRAY_SIZE(rt5682_map)); >> + break; > > It feels like this is going to run into scaling issues going > forward and you're likely to need separate operations for > different CODECs rather than just different IDs. Similar issues > apply for the amps, it feels like you want to be passing separate > ops in rather than having these switch statements. Ok will change to use separate ops in v2 patch series instead of switch statements. > >> + /* Do nothing for dummy codec */ >> + if (!drvdata->hs_codec_id && drvdata->amp_codec_id) >> + return; > > Wha the test seems to say is do nothing if there's no CODEC but > there is an amp... > >> + >> + clk_disable_unprepare(drvdata->wclk); >> +} > > ...though I'd expect that given that the clock API accepts NULL > clocks you could just remove these checks and unconditionally use > the clocks. Ok will remove this check in v2 patch series. > >> + return 0; >> +} >> +EXPORT_SYMBOL_NS(acp_legacy_dai_links_create, SND_SOC_AMD_MACH); > > EXPORT_SYMBOL_GPL_NS() - ASoC is all EXPORT_SYMBOL_GPL > sure will update to use NS_GPL() in v2 patch chain

1 0

When will we have a new release?
by GitHub issues - opened 11 Oct '21

11 Oct '21

alsa-project/alsa-lib issue #185 was opened from leiserfg: There was an issue in the last release that affects pipewire while using laptops with `Comet Lake PCH` that was solved in alsa after the release. I'm currently using `master` via AUR but will love to be able to use a release version (so I don't have to re-build with every change on master) Issue URL : https://github.com/alsa-project/alsa-lib/issues/185 Repository URL: https://github.com/alsa-project/alsa-lib

1 0

[PATCH 00/13] Add Yellow Carp platform ASoC driver
by Vijendar Mukunda 11 Oct '21

11 Oct '21

Yellow Carp platform is new APU series based on acp6.x design. This patch set adds an ASoC driver for the ACP (Audio CoProcessor) block on AMD Yellow Carp APU with DMIC endpoint support. Vijendar Mukunda (13): ASoC: amd: add Yellow Carp ACP6x IP register header ASoC: amd: add Yellow Carp ACP PCI driver ASoC: amd: add acp6x init/de-init functions ASoC: amd: add platform devices for acp6x pdm driver and dmic driver ASoC: amd: add acp6x pdm platform driver ASoC: amd: add acp6x irq handler ASoC: amd: add acp6x pdm driver dma ops ASoC: amd: add acp6x pci driver pm ops ASoC: amd: add acp6x pdm driver pm ops ASoC: amd: enable Yellow carp acp6x drivers build ASoC: amd: create platform device for acp6x machine driver ASoC: amd: add YC machine driver using dmic ASoC: amd: enable yc machine driver build sound/soc/amd/Kconfig | 13 + sound/soc/amd/Makefile | 1 + sound/soc/amd/yc/Makefile | 9 + sound/soc/amd/yc/acp6x-mach.c | 76 ++++ sound/soc/amd/yc/acp6x-pdm-dma.c | 447 ++++++++++++++++++++++ sound/soc/amd/yc/acp6x.h | 107 ++++++ sound/soc/amd/yc/acp6x_chip_offset_byte.h | 444 +++++++++++++++++++++ sound/soc/amd/yc/pci-acp6x.c | 343 +++++++++++++++++ 8 files changed, 1440 insertions(+) create mode 100644 sound/soc/amd/yc/Makefile create mode 100644 sound/soc/amd/yc/acp6x-mach.c create mode 100644 sound/soc/amd/yc/acp6x-pdm-dma.c create mode 100644 sound/soc/amd/yc/acp6x.h create mode 100644 sound/soc/amd/yc/acp6x_chip_offset_byte.h create mode 100644 sound/soc/amd/yc/pci-acp6x.c -- 2.25.1

5 22

[PATCH] ALSA: usb-audio: Less restriction for low-latency playback mode
by Takashi Iwai 11 Oct '21

11 Oct '21

The recent support for the improved low-latency playback mode applied the SNDRV_PCM_INFO_EXPLICIT_SYNC flag for the target streams, but this was a slight overkill. The use of the flag above disables effectively both PCM status and control mmaps, while basically what we want to track is only about the appl_ptr update. For less restriction, use a more proper flag, SNDRV_PCM_INFO_SYNC_APPLPTR instead, which disables only the control mmap. Fixes: d5f871f89e21 ("ALSA: usb-audio: Improved lowlatency playback support") Signed-off-by: Takashi Iwai <tiwai(a)suse.de> --- sound/usb/pcm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 3bb095a3c9b6..95ec8eec1bb0 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -1102,7 +1102,7 @@ static int snd_usb_pcm_open(struct snd_pcm_substream *substream) /* need an explicit sync to catch applptr update in low-latency mode */ if (direction == SNDRV_PCM_STREAM_PLAYBACK && as->chip->lowlatency) - runtime->hw.info |= SNDRV_PCM_INFO_EXPLICIT_SYNC; + runtime->hw.info |= SNDRV_PCM_INFO_SYNC_APPLPTR; runtime->private_data = subs; subs->pcm_substream = substream; /* runtime PM is also done there */ -- 2.31.1

1 0

None
by srinivas.kandagatla＠linaro.org 11 Oct '21

11 Oct '21

On 08/10/2021 16:30, Mark Brown wrote: > On Fri, Oct 08, 2021 at 07:37:12PM +0530, Srinivasa Rao Mandadapu wrote: >> On 9/28/2021 2:01 PM, Srinivas Kandagatla wrote: > >>> In that case you should probably consider using regmap_register_patch() >>> for corrections to the default registers in sm8250 case. > >> As regmap_register_patch() not working in this use case, updating defaults >> in probe for sc7280 specific differences. > > In what way is regmap_register_patch() not working? SM8250 and SC7280 has different register defaults values for few (atleast 2 that we are aware of) registers. Initially we thought regmap_register_patch() would help in this case, but later learned that it was designed for completely different usecase and this will not update the cache values with the new values from reg_sequence passed to regmap_register_patch(). What we want was to be able to reflect that default values in the regcaches, so that subsequent writes are not ignored based on incorrect cache values. There are two ways to fix this, one is to come up with a full default register list specific to SC7280 or reuse SM8250 defaults with some modifications, other option is to update default registers based on compatible string before regmap creation. I suggested Srinivasa to use the second option as 99% of the defaults match to SM8250. --srini >

1 0

[PATCH] ALSA: hda/realtek: Fix for quirk to enable speaker output on the Lenovo 13s Gen2.
by Cameron Berkenpas 11 Oct '21

11 Oct '21

The previous patch's HDA verb initialization for the Lenovo 13s sequence was slightly off. This updated verb sequence has been tested and confirmed working. Fixes: ad7cc2d41b7a ("ALSA: hda/realtek: Quirks to enable speaker output for Lenovo Legion 7i 15IMHG05, Yoga 7i 14ITL5/15ITL5, and 13s Gen2 laptops.") BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=208555 Cc: <stable(a)vger.kernel.org> Cc: Takashi Iwai <tiwai(a)suse.de> Signed-off-by: Cameron Berkenpas <cam(a)neo-zeon.de> --- sound/pci/hda/patch_realtek.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index 4407f7da57c4..84d3e062a335 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -8344,7 +8344,7 @@ static const struct hda_fixup alc269_fixups[] = { .v.verbs = (const struct hda_verb[]) { { 0x20, AC_VERB_SET_COEF_INDEX, 0x24 }, { 0x20, AC_VERB_SET_PROC_COEF, 0x41 }, - { 0x20, AC_VERB_SET_PROC_COEF, 0xb020 }, + { 0x20, AC_VERB_SET_COEF_INDEX, 0x26 }, { 0x20, AC_VERB_SET_PROC_COEF, 0x2 }, { 0x20, AC_VERB_SET_PROC_COEF, 0x0 }, { 0x20, AC_VERB_SET_PROC_COEF, 0x0 }, -- 2.30.2

2 1