August 2019 - Alsa-devel - mailman.alsa-project.org

[alsa-devel] [PATCH 0/4] A few more HD-audio core fixes / cleanups
by Takashi Iwai 14 Aug '19

14 Aug '19

Hi, this is a series of yet another patches for HD-audio core stuff. The first two are workarounds for possible races in the driver registration and other two are merely cleanups. Takashi === Takashi Iwai (4): ALSA: hda - Drop unsol event handler for Intel HDMI codecs ALSA: hda: Add codec on bus address table lately ALSA: hda: Drop export of snd_hdac_bus_add/remove_device() ALSA: hda: Unexport a few more stuff include/sound/hdaudio.h | 9 --------- sound/hda/hdac_bus.c | 8 ++++---- sound/hda/hdac_device.c | 27 ++++++++++++++------------- sound/hda/hdac_regmap.c | 1 + sound/hda/local.h | 7 +++++++ sound/pci/hda/patch_hdmi.c | 9 ++++++++- 6 files changed, 34 insertions(+), 27 deletions(-) -- 2.16.4

1 4

[alsa-devel] [PATCH] Fix an OOB bug in parse_audio_mixer_unit
by Hui Peng 14 Aug '19

14 Aug '19

The `uac_mixer_unit_descriptor` shown as below is read from the device side. In `parse_audio_mixer_unit`, `baSourceID` field is accessed from index 0 to `bNrInPins` - 1, the current implementation assumes that descriptor is always valid (the length of descriptor is no shorter than 5 + `bNrInPins`). If a descriptor read from the device side is invalid, it may trigger out-of-bound memory access. ``` struct uac_mixer_unit_descriptor { __u8 bLength; __u8 bDescriptorType; __u8 bDescriptorSubtype; __u8 bUnitID; __u8 bNrInPins; __u8 baSourceID[]; } ``` This patch fixes the bug by add a sanity check on the length of the descriptor. Signed-off-by: Hui Peng <benquike(a)gmail.com> Reported-by: Hui Peng <benquike(a)gmail.com> Reported-by: Mathias Payer <mathias.payer(a)nebelwelt.net> --- sound/usb/mixer.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c index 7498b5191b68..38202ce67237 100644 --- a/sound/usb/mixer.c +++ b/sound/usb/mixer.c @@ -2091,6 +2091,15 @@ static int parse_audio_mixer_unit(struct mixer_build *state, int unitid, struct usb_audio_term iterm; int input_pins, num_ins, num_outs; int pin, ich, err; + int desc_len = (int) ((unsigned long) state->buffer + + state->buflen - (unsigned long) raw_desc); + + if (desc_len < sizeof(*desc) + desc->bNrInPins) { + usb_audio_err(state->chip, + "descriptor %d too short\n", + unitid); + return -EINVAL; + } err = uac_mixer_unit_get_channels(state, desc); if (err < 0) { -- 2.22.1

4 8

[alsa-devel] [PATCH] ALSA: sb: remove redundant assignment to variable result
by Colin King 14 Aug '19

14 Aug '19

From: Colin Ian King <colin.king(a)canonical.com> Variable result is initialized to a value that is never read and it is re-assigned later. The initialization is redundant and can be removed. Addresses-Coverity: ("Unused value") Signed-off-by: Colin Ian King <colin.king(a)canonical.com> --- sound/isa/sb/sb_common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/isa/sb/sb_common.c b/sound/isa/sb/sb_common.c index 162338f1b68a..ff031d670400 100644 --- a/sound/isa/sb/sb_common.c +++ b/sound/isa/sb/sb_common.c @@ -80,7 +80,7 @@ int snd_sbdsp_reset(struct snd_sb *chip) static int snd_sbdsp_version(struct snd_sb * chip) { - unsigned int result = -ENODEV; + unsigned int result; snd_sbdsp_command(chip, SB_DSP_GET_VERSION); result = (short) snd_sbdsp_get_byte(chip) << 8; -- 2.20.1

2 1

[alsa-devel] [PATCH] ASoC: fsl_sai: Handle slave mode per TX/RX direction
by Daniel Baluta 14 Aug '19

14 Aug '19

From: Viorel Suman <viorel.suman(a)nxp.com> The SAI interface can be a clock supplier or consumer as a function of stream direction. e.g SAI can be master for Tx and slave for Rx. Signed-off-by: Viorel Suman <viorel.suman(a)nxp.com> Signed-off-by: Daniel Baluta <daniel.baluta(a)nxp.com> --- sound/soc/fsl/fsl_sai.c | 18 +++++++++--------- sound/soc/fsl/fsl_sai.h | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c index 4a346fcb5630..69cf3678c859 100644 --- a/sound/soc/fsl/fsl_sai.c +++ b/sound/soc/fsl/fsl_sai.c @@ -273,18 +273,18 @@ static int fsl_sai_set_dai_fmt_tr(struct snd_soc_dai *cpu_dai, case SND_SOC_DAIFMT_CBS_CFS: val_cr2 |= FSL_SAI_CR2_BCD_MSTR; val_cr4 |= FSL_SAI_CR4_FSD_MSTR; - sai->is_slave_mode = false; + sai->is_slave_mode[tx] = false; break; case SND_SOC_DAIFMT_CBM_CFM: - sai->is_slave_mode = true; + sai->is_slave_mode[tx] = true; break; case SND_SOC_DAIFMT_CBS_CFM: val_cr2 |= FSL_SAI_CR2_BCD_MSTR; - sai->is_slave_mode = false; + sai->is_slave_mode[tx] = false; break; case SND_SOC_DAIFMT_CBM_CFS: val_cr4 |= FSL_SAI_CR4_FSD_MSTR; - sai->is_slave_mode = true; + sai->is_slave_mode[tx] = true; break; default: return -EINVAL; @@ -326,7 +326,7 @@ static int fsl_sai_set_bclk(struct snd_soc_dai *dai, bool tx, u32 freq) int ret = 0; /* Don't apply to slave mode */ - if (sai->is_slave_mode) + if (sai->is_slave_mode[tx]) return 0; for (id = 0; id < FSL_SAI_MCLK_MAX; id++) { @@ -422,7 +422,7 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream, if (sai->slot_width) slot_width = sai->slot_width; - if (!sai->is_slave_mode) { + if (!sai->is_slave_mode[tx]) { ret = fsl_sai_set_bclk(cpu_dai, tx, slots * slot_width * params_rate(params)); if (ret) @@ -458,7 +458,7 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream, * error. */ - if (!sai->is_slave_mode) { + if (!sai->is_slave_mode[tx]) { if (!sai->synchronous[TX] && sai->synchronous[RX] && !tx) { regmap_update_bits(sai->regmap, FSL_SAI_TCR4(ofs), FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK, @@ -497,7 +497,7 @@ static int fsl_sai_hw_free(struct snd_pcm_substream *substream, struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai); bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK; - if (!sai->is_slave_mode && + if (!sai->is_slave_mode[tx] && sai->mclk_streams & BIT(substream->stream)) { clk_disable_unprepare(sai->mclk_clk[sai->mclk_id[tx]]); sai->mclk_streams &= ~BIT(substream->stream); @@ -581,7 +581,7 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd, * This is a hardware bug, and will be fix in the * next sai version. */ - if (!sai->is_slave_mode) { + if (!sai->is_slave_mode[tx]) { /* Software Reset for both Tx and Rx */ regmap_write(sai->regmap, FSL_SAI_TCSR(ofs), FSL_SAI_CSR_SR); diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h index b89b0ca26053..c2c43a7d9ba1 100644 --- a/sound/soc/fsl/fsl_sai.h +++ b/sound/soc/fsl/fsl_sai.h @@ -167,7 +167,7 @@ struct fsl_sai { struct clk *bus_clk; struct clk *mclk_clk[FSL_SAI_MCLK_MAX]; - bool is_slave_mode; + bool is_slave_mode[2]; bool is_lsb_first; bool is_dsp_mode; bool synchronous[2]; -- 2.17.1

3 2

[alsa-devel] [RFC PATCH] ASoC: fsl_sai: Enable data lines based on input channels
by Daniel Baluta 14 Aug '19

14 Aug '19

An audio data frame consists of a number of slots one for each channel. In the case of I2S there are 2 data slots / frame. The maximum number of SAI slots / frame is configurable at IP integration time. This affects the width of Mask Register. SAI supports up to 32 slots per frame. The number of datalines is also configurable (up to 8 datalines) and affects TCE/RCE and the number of data/FIFO registers. The number of needed data lines (pins) is computed as follows: * pins = channels / slots. This can be computed in hw_params function so lets move TRCE bits seting from startup to hw_params. Signed-off-by: Daniel Baluta <daniel.baluta(a)nxp.com> --- sound/soc/fsl/fsl_sai.c | 34 +++++++++++++--------------------- sound/soc/fsl/fsl_sai.h | 2 +- 2 files changed, 14 insertions(+), 22 deletions(-) diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c index 69cf3678c859..b70032c82fe2 100644 --- a/sound/soc/fsl/fsl_sai.c +++ b/sound/soc/fsl/fsl_sai.c @@ -414,6 +414,7 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream, u32 val_cr4 = 0, val_cr5 = 0; u32 slots = (channels == 1) ? 2 : channels; u32 slot_width = word_width; + u32 pins; int ret; if (sai->slots) @@ -422,6 +423,8 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream, if (sai->slot_width) slot_width = sai->slot_width; + pins = DIV_ROUND_UP(channels, slots); + if (!sai->is_slave_mode[tx]) { ret = fsl_sai_set_bclk(cpu_dai, tx, slots * slot_width * params_rate(params)); @@ -480,13 +483,17 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream, } } + regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx, ofs), + FSL_SAI_CR3_TRCE_MASK, + FSL_SAI_CR3_TRCE((1 << pins) - 1)); + regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx, ofs), FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK, val_cr4); regmap_update_bits(sai->regmap, FSL_SAI_xCR5(tx, ofs), FSL_SAI_CR5_WNW_MASK | FSL_SAI_CR5_W0W_MASK | FSL_SAI_CR5_FBT_MASK, val_cr5); - regmap_write(sai->regmap, FSL_SAI_xMR(tx), ~0UL - ((1 << channels) - 1)); + regmap_write(sai->regmap, FSL_SAI_xMR(tx), ~0UL - ((1 << slots) - 1)); return 0; } @@ -496,6 +503,10 @@ static int fsl_sai_hw_free(struct snd_pcm_substream *substream, { struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai); bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK; + unsigned int ofs = sai->soc_data->reg_offset; + + regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx, ofs), + FSL_SAI_CR3_TRCE_MASK, 0); if (!sai->is_slave_mode[tx] && sai->mclk_streams & BIT(substream->stream)) { @@ -603,32 +614,13 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd, static int fsl_sai_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *cpu_dai) { - struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai); - unsigned int ofs = sai->soc_data->reg_offset; - bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK; int ret; - regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx, ofs), - FSL_SAI_CR3_TRCE_MASK, - FSL_SAI_CR3_TRCE); - ret = snd_pcm_hw_constraint_list(substream->runtime, 0, SNDRV_PCM_HW_PARAM_RATE, &fsl_sai_rate_constraints); - return ret; } -static void fsl_sai_shutdown(struct snd_pcm_substream *substream, - struct snd_soc_dai *cpu_dai) -{ - struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai); - unsigned int ofs = sai->soc_data->reg_offset; - bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK; - - regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx, ofs), - FSL_SAI_CR3_TRCE_MASK, 0); -} - static const struct snd_soc_dai_ops fsl_sai_pcm_dai_ops = { .set_sysclk = fsl_sai_set_dai_sysclk, .set_fmt = fsl_sai_set_dai_fmt, @@ -637,7 +629,6 @@ static const struct snd_soc_dai_ops fsl_sai_pcm_dai_ops = { .hw_free = fsl_sai_hw_free, .trigger = fsl_sai_trigger, .startup = fsl_sai_startup, - .shutdown = fsl_sai_shutdown, }; static int fsl_sai_dai_probe(struct snd_soc_dai *cpu_dai) @@ -881,6 +872,7 @@ static int fsl_sai_probe(struct platform_device *pdev) return -ENOMEM; sai->pdev = pdev; + sai->soc_data = of_device_get_match_data(&pdev->dev); sai->is_lsb_first = of_property_read_bool(np, "lsb-first"); diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h index c2c43a7d9ba1..3c5383139dc2 100644 --- a/sound/soc/fsl/fsl_sai.h +++ b/sound/soc/fsl/fsl_sai.h @@ -109,7 +109,7 @@ #define FSL_SAI_CR2_DIV_MASK 0xff /* SAI Transmit and Receive Configuration 3 Register */ -#define FSL_SAI_CR3_TRCE BIT(16) +#define FSL_SAI_CR3_TRCE(x) ((x) << 16) #define FSL_SAI_CR3_TRCE_MASK GENMASK(23, 16) #define FSL_SAI_CR3_WDFL(x) (x) #define FSL_SAI_CR3_WDFL_MASK 0x1f -- 2.17.1

3 2

[alsa-devel] [PATCH] ALSA: hda: Retry codec read on first time failure
by vani.ganji＠intel.com 14 Aug '19

14 Aug '19

From: Vani Ganji <vani.ganji(a)intel.com> Sometimes HDMI audio fails while fast switching between HDMI and Speaker due to codec Get Power state verb read failure. Verb is codec driver to access codec HW. Since issue scenario involves fast switching between HDMI and speaker, there might be chance to fail read from HW and same happened in the error scenario. Based on Display/Graphics architect suggestion, added a retry mechanism for failure on first attempt. Signed-off-by: Vani Ganji <vani.ganji(a)intel.com> --- sound/hda/hdac_device.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/sound/hda/hdac_device.c b/sound/hda/hdac_device.c index b26cc93e7e10..225bd1bfe156 100644 --- a/sound/hda/hdac_device.c +++ b/sound/hda/hdac_device.c @@ -984,10 +984,17 @@ static unsigned int codec_read(struct hdac_device *hdac, hda_nid_t nid, { unsigned int cmd = snd_hdac_make_cmd(hdac, nid, verb, parm); unsigned int res; - - if (snd_hdac_exec_verb(hdac, cmd, flags, &res)) - return -1; - + unsigned int count; + + /* Retry verb once more if it fails first time */ + /* recommended to retry since it is HW related */ + for (count = 0; count <= 1; count++) { + if (snd_hdac_exec_verb(hdac, cmd, flags, &res)) + if (count == 1) + return -1; + } else + break; + } return res; } -- 2.17.1

1 0

[alsa-devel] [PATCH] Add support for Sabre HiFi on System76 and Clevo laptops
by Dennis Mungai 14 Aug '19

14 Aug '19

Hello there, The patch attached below is based on Jeremy Soller's prior work on: https://patchwork.kernel.org/patch/9552671/ , copied herein. Cleaned up to pass checkpatch.pl tests. I can confirm that this patch does indeed fix the audio DAC init on the Clevo based P775TM1-R chassis with no issues. Warm regards, Dennis Mungai

2 1

[alsa-devel] [PATCH 1/2] ALSA: hda - Let all conexant codec enter D3 when rebooting
by Hui Wang 14 Aug '19

14 Aug '19

We have 3 new lenovo laptops which have conexant codec 0x14f11f86, these 3 laptops also have the noise issue when rebooting, after letting the codec enter D3 before rebooting or poweroff, the noise disappers. Instead of adding a new ID again in the reboot_notify(), let us make this function apply to all conexant codec. In theory make codec enter D3 before rebooting or poweroff is harmless, and I tested this change on a couple of other Lenovo laptops which have different conexant codecs, there is no side effect so far. Cc: stable(a)vger.kernel.org Signed-off-by: Hui Wang <hui.wang(a)canonical.com> --- sound/pci/hda/patch_conexant.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c index f299f137eaea..93a303676aea 100644 --- a/sound/pci/hda/patch_conexant.c +++ b/sound/pci/hda/patch_conexant.c @@ -163,15 +163,6 @@ static void cx_auto_reboot_notify(struct hda_codec *codec) { struct conexant_spec *spec = codec->spec; - switch (codec->core.vendor_id) { - case 0x14f12008: /* CX8200 */ - case 0x14f150f2: /* CX20722 */ - case 0x14f150f4: /* CX20724 */ - break; - default: - return; - } - /* Turn the problematic codec into D3 to avoid spurious noises from the internal speaker during (and after) reboot */ cx_auto_turn_eapd(codec, spec->num_eapds, spec->eapds, false); -- 2.17.1

2 3

[alsa-devel] [PATCH] ALSA: hda: Retry codec read on first time failure
by vani.ganji＠intel.com 14 Aug '19

14 Aug '19

From: Vani Ganji <vani.ganji(a)intel.com> Sometimes HDMI audio fails while fast switching between HDMI and Speaker due to codec Get Power state verb read failure. Verb is codec driver to access codec HW. Since issue scenario involves fast switching between HDMI and speaker, there might be chance to fail read from HW and same happened in the error scenario. Based on Display/Graphics architect suggestion, added a retry mechanism for failure on first attempt. Signed-off-by: Vani Ganji <vani.ganji(a)intel.com> --- sound/hda/hdac_device.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/sound/hda/hdac_device.c b/sound/hda/hdac_device.c index b26cc93e7e10..225bd1bfe156 100644 --- a/sound/hda/hdac_device.c +++ b/sound/hda/hdac_device.c @@ -984,10 +984,17 @@ static unsigned int codec_read(struct hdac_device *hdac, hda_nid_t nid, { unsigned int cmd = snd_hdac_make_cmd(hdac, nid, verb, parm); unsigned int res; - - if (snd_hdac_exec_verb(hdac, cmd, flags, &res)) - return -1; - + unsigned int count; + + /* Retry verb once more if it fails first time */ + /* recommended to retry since it is HW related */ + for (count = 0; count <= 1; count++) { + if (snd_hdac_exec_verb(hdac, cmd, flags, &res)) + if (count == 1) + return -1; + } else + break; + } return res; } -- 2.17.1

2 1

[alsa-devel] [PATCH] soundwire: Don't build sound.o without CONFIG_ACPI
by Nathan Chancellor 14 Aug '19

14 Aug '19

clang warns when CONFIG_ACPI is unset: ../drivers/soundwire/slave.c:16:12: warning: unused function 'sdw_slave_add' [-Wunused-function] static int sdw_slave_add(struct sdw_bus *bus, ^ 1 warning generated. Before commit 8676b3ca4673 ("soundwire: fix regmap dependencies and align with other serial links"), this code would only be compiled when ACPI was set because it was only selected by SOUNDWIRE_INTEL, which depends on ACPI. Now, this code can be compiled without CONFIG_ACPI, which causes the above warning. The IS_ENABLED(CONFIG_ACPI) guard could be moved to avoid compiling the function; however, slave.c only contains three functions, two of which are static. Only compile slave.o when CONFIG_ACPI is set, where it will actually be used. bus.h contains a stub for sdw_acpi_find_slaves so there will be no issues with an undefined function. This has been build tested with CONFIG_ACPI set and unset in combination with CONFIG_SOUNDWIRE unset, built in, and a module. Fixes: 8676b3ca4673 ("soundwire: fix regmap dependencies and align with other serial links") Link: https://github.com/ClangBuiltLinux/linux/issues/637 Signed-off-by: Nathan Chancellor <natechancellor(a)gmail.com> --- drivers/soundwire/Makefile | 6 +++++- drivers/soundwire/slave.c | 3 --- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/soundwire/Makefile b/drivers/soundwire/Makefile index 45b7e5001653..226090902716 100644 --- a/drivers/soundwire/Makefile +++ b/drivers/soundwire/Makefile @@ -4,9 +4,13 @@ # #Bus Objs -soundwire-bus-objs := bus_type.o bus.o slave.o mipi_disco.o stream.o +soundwire-bus-objs := bus_type.o bus.o mipi_disco.o stream.o obj-$(CONFIG_SOUNDWIRE) += soundwire-bus.o +ifdef CONFIG_ACPI +soundwire-bus-objs += slave.o +endif + #Cadence Objs soundwire-cadence-objs := cadence_master.o obj-$(CONFIG_SOUNDWIRE_CADENCE) += soundwire-cadence.o diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c index f39a5815e25d..0dc188e6873b 100644 --- a/drivers/soundwire/slave.c +++ b/drivers/soundwire/slave.c @@ -60,7 +60,6 @@ static int sdw_slave_add(struct sdw_bus *bus, return ret; } -#if IS_ENABLED(CONFIG_ACPI) /* * sdw_acpi_find_slaves() - Find Slave devices in Master ACPI node * @bus: SDW bus instance @@ -110,5 +109,3 @@ int sdw_acpi_find_slaves(struct sdw_bus *bus) return 0; } - -#endif -- 2.23.0.rc2

4 8