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

Re: [2/2]ALSA: hda: add Zhaoxin HDAC non-snoop support: patch a design limitation
by Takashi Iwai 15 Apr '21

15 Apr '21

On Wed, 14 Apr 2021 14:00:23 +0200, Hans Hu(SH-RD) wrote: > > Hi Takashi, > > ZHAOXIN HDAC controller has one design limitation when it works in non-snoop mode. This design limitation is: hardware can't guarantee that the write CORB cycle always completes first before the write CORBWP cycle. Here is the error scene: > int snd_hdac_bus_send_cmd(struct hdac_bus *bus, unsigned int val) > { > ... > bus->corb.buf[wp] = cpu_to_le32(val); // cycle_1: write value to CORB > snd_hdac_chip_writew(bus, CORBWP, wp); // cycle_2: write wp to CORBWP > ... > } > Normally, after cycle_2, DMA engine will start working and get data from CORB and sent it out. But if cycle_1 haven’t finished yet at this time(limitation occurs), which means the value haven’t been written into CORB, then DMA engine will get unexpected value, then error occurred. (feels like one kind of CORB underrun). > > If we add one read CORB cycle between cycle_1 and cycle_2, cycle_1 and cycle_2 operations will be synchronized, this limitation will be fixed. We have written a draft patch based on this situation and hope to be accepted, the patch example is as follows: > diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h > index 22af68b..c338db00 100644 > --- a/include/sound/hdaudio.h > +++ b/include/sound/hdaudio.h > @@ -339,6 +339,7 @@ struct hdac_bus { > bool sync_write:1; /* sync after verb write */ > bool use_posbuf:1; /* use position buffer */ > bool snoop:1; /* enable snooping */ > + bool no_snoop_corb_sync:1; > bool align_bdle_4k:1; /* BDLE align 4K boundary */ > bool reverse_assign:1; /* assign devices in reverse order */ > bool corbrp_self_clear:1; /* CORBRP clears itself after reset */ > diff --git a/sound/hda/hdac_controller.c b/sound/hda/hdac_controller.c > index 062da7a..6c90cdd 100644 > --- a/sound/hda/hdac_controller.c > +++ b/sound/hda/hdac_controller.c > @@ -167,6 +167,8 @@ int snd_hdac_bus_send_cmd(struct hdac_bus *bus, unsigned int val) > > bus->rirb.cmds[addr]++; > bus->corb.buf[wp] = cpu_to_le32(val); > + if (bus->no_snoop_corb_sync) > + val = bus->corb.buf[wp]; > snd_hdac_chip_writew(bus, CORBWP, wp); Just wondering whether using WRITE_ONCE() macro would be enough? e.g. WRITE_ONCE(bus->corb.buf[wp], cpu_to_le32(val)); Also, no_snoop_corb_sync is a bit confusing. What you do is rather sync of the written value, so it can be taken as if other way round. Maybe no_coherent_corb_write or such would be better understandable. thanks, Takashi

2 2

[PATCH] ALSA: usb-audio: Explicitly set up the clock selector
by Takashi Iwai 14 Apr '21

14 Apr '21

In the current code, we have some assumption that the audio clock selector has been set up implicitly and don't want to touch it unless it's really needed for the fallback autoclock setup. This works for most devices but some seem having a problem. Partially this was covered for the devices with a single connector at the initialization phase (commit 086b957cc17f "ALSA: usb-audio: Skip the clock selector inquiry for single connections"), but also there are cases where the wrong clock set up is kept silently. The latter seems to be the cause of the noises on Behringer devices. In this patch, we explicitly set up the audio clock selector whenever the appropriate node is found. Reported-by: Geraldo Nascimento <geraldogabriel(a)gmail.com> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=199327 Link: https://lore.kernel.org/r/CAEsQvcvF7LnO8PxyyCxuRCx=7jNeSCvFAd-+dE0g_rd1rOxx… Cc: <stable(a)vger.kernel.org> Signed-off-by: Takashi Iwai <tiwai(a)suse.de> --- sound/usb/clock.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/sound/usb/clock.c b/sound/usb/clock.c index a746802d0ac3..17bbde73d4d1 100644 --- a/sound/usb/clock.c +++ b/sound/usb/clock.c @@ -296,7 +296,7 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip, selector = snd_usb_find_clock_selector(chip->ctrl_intf, entity_id); if (selector) { - int ret, i, cur; + int ret, i, cur, err; if (selector->bNrInPins == 1) { ret = 1; @@ -324,13 +324,17 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip, ret = __uac_clock_find_source(chip, fmt, selector->baCSourceID[ret - 1], visited, validate); + if (ret > 0) { + err = uac_clock_selector_set_val(chip, entity_id, cur); + if (err < 0) + return err; + } + if (!validate || ret > 0 || !chip->autoclock) return ret; /* The current clock source is invalid, try others. */ for (i = 1; i <= selector->bNrInPins; i++) { - int err; - if (i == cur) continue; @@ -396,7 +400,7 @@ static int __uac3_clock_find_source(struct snd_usb_audio *chip, selector = snd_usb_find_clock_selector_v3(chip->ctrl_intf, entity_id); if (selector) { - int ret, i, cur; + int ret, i, cur, err; /* the entity ID we are looking for is a selector. * find out what it currently selects */ @@ -418,6 +422,12 @@ static int __uac3_clock_find_source(struct snd_usb_audio *chip, ret = __uac3_clock_find_source(chip, fmt, selector->baCSourceID[ret - 1], visited, validate); + if (ret > 0) { + err = uac_clock_selector_set_val(chip, entity_id, cur); + if (err < 0) + return err; + } + if (!validate || ret > 0 || !chip->autoclock) return ret; -- 2.26.2

2 1

[PATCH v2 -next 1/2] ASoC: ak5558: correct reset polarity
by Shengjiu Wang 14 Apr '21

14 Apr '21

Reset (aka power off) happens when the reset gpio is made active. The reset gpio is GPIO_ACTIVE_LOW Fixes: 920884777480 ("ASoC: ak5558: Add support for AK5558 ADC driver") Signed-off-by: Shengjiu Wang <shengjiu.wang(a)nxp.com> --- changes in v2: - split the patch to two patches. sound/soc/codecs/ak5558.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sound/soc/codecs/ak5558.c b/sound/soc/codecs/ak5558.c index 8e4dca753f0b..f24d91b728dd 100644 --- a/sound/soc/codecs/ak5558.c +++ b/sound/soc/codecs/ak5558.c @@ -323,7 +323,7 @@ static void ak5558_power_off(struct ak5558_priv *ak5558) if (!ak5558->reset_gpiod) return; - gpiod_set_value_cansleep(ak5558->reset_gpiod, 0); + gpiod_set_value_cansleep(ak5558->reset_gpiod, 1); usleep_range(1000, 2000); } @@ -332,7 +332,7 @@ static void ak5558_power_on(struct ak5558_priv *ak5558) if (!ak5558->reset_gpiod) return; - gpiod_set_value_cansleep(ak5558->reset_gpiod, 1); + gpiod_set_value_cansleep(ak5558->reset_gpiod, 0); usleep_range(1000, 2000); } -- 2.27.0

2 2

[PATCH] ASoC: cs35l35: remove unused including <linux/version.h>
by Yang Li 14 Apr '21

14 Apr '21

Fix the following versioncheck warning: ./sound/soc/codecs/cs35l35.c: 12 linux/version.h not needed. Reported-by: Abaci Robot <abaci(a)linux.alibaba.com> Signed-off-by: Yang Li <yang.lee(a)linux.alibaba.com> --- sound/soc/codecs/cs35l35.c | 1 - 1 file changed, 1 deletion(-) diff --git a/sound/soc/codecs/cs35l35.c b/sound/soc/codecs/cs35l35.c index 55d529a..5d361c7 100644 --- a/sound/soc/codecs/cs35l35.c +++ b/sound/soc/codecs/cs35l35.c @@ -9,7 +9,6 @@ #include <linux/module.h> #include <linux/moduleparam.h> -#include <linux/version.h> #include <linux/kernel.h> #include <linux/init.h> #include <linux/delay.h> -- 1.8.3.1

2 1

[PATCH 0/5] ASoC: rsnd: tidyup Renesas sound
by Kuninori Morimoto 14 Apr '21

14 Apr '21

Hi Mark These are tidyup patches for Renesas sound drivers. Kuninori Morimoto (5): ASoC: rsnd: call rsnd_ssi_master_clk_start() from rsnd_ssi_init() ASoC: rsnd: check all BUSIF status when error ASoC: rsnd: add rsnd_ssi_busif_err_status_clear() ASoC: rsnd: add rsnd_ssi_busif_err_irq_enable/disable() ASoC: rsnd: add usage for SRC sound/soc/sh/rcar/src.c | 9 ++ sound/soc/sh/rcar/ssi.c | 225 ++++++++++++++++++---------------------- 2 files changed, 110 insertions(+), 124 deletions(-) -- 2.25.1

2 6

[PATCH v2 0/3] soundwire: add DMI quirks for overridind addr
by Bard Liao 14 Apr '21

14 Apr '21

Platform firmware may have incorrect _ADR values causing the driver probes to fail. Adding the override_ops allows people to override the addr values. v2: - Add the override_adr ops - Move DMI quirks to a new file Pierre-Louis Bossart (2): soundwire: Intel: introduce DMI quirks for HP Spectre x360 Convertible soundwire: Intel: add DMI quirk for Dell SKU 0A3E Vinod Koul (1): soundwire: add override addr ops drivers/soundwire/Makefile | 2 +- drivers/soundwire/bus.h | 2 + drivers/soundwire/dmi-quirks.c | 96 ++++++++++++++++++++++++++++++++++ drivers/soundwire/intel.c | 1 + drivers/soundwire/slave.c | 8 ++- include/linux/soundwire/sdw.h | 4 +- 6 files changed, 110 insertions(+), 3 deletions(-) create mode 100644 drivers/soundwire/dmi-quirks.c -- 2.17.1

4 7

[PATCH] ASoC: codec: remove unused variable
by Jiapeng Chong 14 Apr '21

14 Apr '21

Fix the following gcc warning: sound/soc/codecs/jz4760.c:201:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable]. Reported-by: Abaci Robot <abaci(a)linux.alibaba.com> Signed-off-by: Jiapeng Chong <jiapeng.chong(a)linux.alibaba.com> --- sound/soc/codecs/jz4760.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/sound/soc/codecs/jz4760.c b/sound/soc/codecs/jz4760.c index e8f28cc..ad5e859 100644 --- a/sound/soc/codecs/jz4760.c +++ b/sound/soc/codecs/jz4760.c @@ -198,15 +198,13 @@ static int jz4760_codec_startup(struct snd_pcm_substream *substream, { struct snd_soc_component *codec = dai->component; struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(codec); - int ret; - /* * SYSCLK output from the codec to the AIC is required to keep the * DMA transfer going during playback when all audible outputs have * been disabled. */ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) - ret = snd_soc_dapm_force_enable_pin(dapm, "SYSCLK"); + snd_soc_dapm_force_enable_pin(dapm, "SYSCLK"); return 0; } -- 1.8.3.1

2 1

[PATCH] ALSA: control_led - fix the stack usage (control element ops)
by Jaroslav Kysela 14 Apr '21

14 Apr '21

It's a bad idea to allocate big structures on the stack. Mark the variables as static and add a note for the locking. Fixes: 22d8de62f11b ("ALSA: control - add generic LED trigger module as the new control layer") Signed-off-by: Jaroslav Kysela <perex(a)perex.cz> Cc: Nathan Chancellor <nathan(a)kernel.org> Cc: Takashi Sakamoto <o-takashi(a)sakamocchi.jp> Signed-off-by: Jaroslav Kysela <perex(a)perex.cz> --- sound/core/control_led.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/sound/core/control_led.c b/sound/core/control_led.c index 93b201063c7d..25f57c14f294 100644 --- a/sound/core/control_led.c +++ b/sound/core/control_led.c @@ -94,11 +94,15 @@ static struct snd_ctl_led *snd_ctl_led_get_by_access(unsigned int access) return &snd_ctl_leds[group]; } +/* + * A note for callers: + * The two static variables info and value are protected using snd_ctl_led_mutex. + */ static int snd_ctl_led_get(struct snd_ctl_led_ctl *lctl) { + static struct snd_ctl_elem_info info; + static struct snd_ctl_elem_value value; struct snd_kcontrol *kctl = lctl->kctl; - struct snd_ctl_elem_info info; - struct snd_ctl_elem_value value; unsigned int i; int result; -- 2.30.2

2 2

Re: [1/2]ALSA: hda: add Zhaoxin HDAC non-snoop support
by Takashi Iwai 14 Apr '21

14 Apr '21

On Wed, 14 Apr 2021 14:00:19 +0200, Hans Hu(SH-RD) wrote: > > Hi Takashi, > > Recently, our(AZX_DRIVER_ZHAOXIN) HDAC controller has added the feature of non-snoop mode, we have written a draft patch for it and hope to be accepted. The code change looks good. Please go ahead with the proper submission. thanks, Takashi > > diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index b59b0f3..420b728 100644 > --- a/sound/pci/hda/hda_intel.c > +++ b/sound/pci/hda/hda_intel.c > @@ -1702,6 +1702,14 @@ static void azx_check_snoop_available(struct azx *chip) > snoop = false; > } > > +if (azx_get_snoop_type(chip) == AZX_SNOOP_TYPE_NONE && > + chip->driver_type == AZX_DRIVER_ZHAOXIN) { > +u8 val; > +pci_read_config_byte(chip->pci, 0x42, &val); > +if (!(val & 0x80)) > +snoop = false; > +} > + > if (chip->driver_caps & AZX_DCAPS_SNOOP_OFF) > snoop = false; > > > Thanks, > 　Hans Hu :) > > > 保密声明： > 本邮件含有保密或专有信息，仅供指定收件人使用。严禁对本邮件或其内容做任何未经授权的查阅、使用、复制或转发。 > CONFIDENTIAL NOTE: > This email contains confidential or legally privileged information and is for the sole use of its intended recipient. Any unauthorized review, use, copying or forwarding of this email or the content of this email is strictly prohibited.

1 0

[PATCH] ALSA: control_led - fix the stack usage (control element ops)
by Jaroslav Kysela 14 Apr '21

14 Apr '21

It's a bad idea to allocate big structures on the stack. Allocate the required structures on demand and cache them in the led structure. Fixes: 22d8de62f11b ("ALSA: control - add generic LED trigger module as the new control layer") Signed-off-by: Jaroslav Kysela <perex(a)perex.cz> Cc: Nathan Chancellor <nathan(a)kernel.org> Cc: Takashi Sakamoto <o-takashi(a)sakamocchi.jp> --- sound/core/control_led.c | 52 ++++++++++++++++++++++++---------------- 1 file changed, 32 insertions(+), 20 deletions(-) diff --git a/sound/core/control_led.c b/sound/core/control_led.c index 93b201063c7d..9d1612b1a6ff 100644 --- a/sound/core/control_led.c +++ b/sound/core/control_led.c @@ -37,6 +37,7 @@ struct snd_ctl_led { unsigned int group; enum led_audio trigger_type; enum snd_ctl_led_mode mode; + void *info_and_value; struct snd_ctl_led_card *cards[SNDRV_CARDS]; }; @@ -94,34 +95,41 @@ static struct snd_ctl_led *snd_ctl_led_get_by_access(unsigned int access) return &snd_ctl_leds[group]; } -static int snd_ctl_led_get(struct snd_ctl_led_ctl *lctl) +static int snd_ctl_led_get(struct snd_ctl_led *led, struct snd_ctl_led_ctl *lctl) { struct snd_kcontrol *kctl = lctl->kctl; - struct snd_ctl_elem_info info; - struct snd_ctl_elem_value value; + struct snd_ctl_elem_info *info = led->info_and_value; + struct snd_ctl_elem_value *value; unsigned int i; int result; - memset(&info, 0, sizeof(info)); - info.id = kctl->id; - info.id.index += lctl->index_offset; - info.id.numid += lctl->index_offset; - result = kctl->info(kctl, &info); + if (info == NULL) { + info = kmalloc(sizeof(*info) + sizeof(*value), GFP_KERNEL); + if (info == NULL) + return -1; + led->info_and_value = info; + } + value = (void *)(info + 1); + memset(info, 0, sizeof(*info)); + info->id = kctl->id; + info->id.index += lctl->index_offset; + info->id.numid += lctl->index_offset; + result = kctl->info(kctl, info); if (result < 0) return -1; - memset(&value, 0, sizeof(value)); - value.id = info.id; - result = kctl->get(kctl, &value); + memset(value, 0, sizeof(*value)); + value->id = info->id; + result = kctl->get(kctl, value); if (result < 0) return -1; - if (info.type == SNDRV_CTL_ELEM_TYPE_BOOLEAN || - info.type == SNDRV_CTL_ELEM_TYPE_INTEGER) { - for (i = 0; i < info.count; i++) - if (value.value.integer.value[i] != info.value.integer.min) + if (info->type == SNDRV_CTL_ELEM_TYPE_BOOLEAN || + info->type == SNDRV_CTL_ELEM_TYPE_INTEGER) { + for (i = 0; i < info->count; i++) + if (value->value.integer.value[i] != info->value.integer.min) return 1; - } else if (info.type == SNDRV_CTL_ELEM_TYPE_INTEGER64) { - for (i = 0; i < info.count; i++) - if (value.value.integer64.value[i] != info.value.integer64.min) + } else if (info->type == SNDRV_CTL_ELEM_TYPE_INTEGER64) { + for (i = 0; i < info->count; i++) + if (value->value.integer64.value[i] != info->value.integer64.min) return 1; } return 0; @@ -149,7 +157,7 @@ static void snd_ctl_led_set_state(struct snd_card *card, unsigned int access, list_for_each_entry(lctl, &led->controls, list) { if (lctl->kctl == kctl && lctl->index_offset == ioff) found = true; - UPDATE_ROUTE(route, snd_ctl_led_get(lctl)); + UPDATE_ROUTE(route, snd_ctl_led_get(led, lctl)); } if (!found && kctl && card) { lctl = kzalloc(sizeof(*lctl), GFP_KERNEL); @@ -159,7 +167,7 @@ static void snd_ctl_led_set_state(struct snd_card *card, unsigned int access, lctl->kctl = kctl; lctl->index_offset = ioff; list_add(&lctl->list, &led->controls); - UPDATE_ROUTE(route, snd_ctl_led_get(lctl)); + UPDATE_ROUTE(route, snd_ctl_led_get(led, lctl)); } } mutex_unlock(&snd_ctl_led_mutex); @@ -300,6 +308,10 @@ static void snd_ctl_led_clean(struct snd_card *card) snd_ctl_led_ctl_destroy(lctl); goto repeat; } + if (!card) { + kfree(led->info_and_value); + led->info_and_value = NULL; + } } } -- 2.30.2

2 4