March 2018 - Alsa-devel - mailman.alsa-project.org

[alsa-devel] [PATCH] ASoC: mediatek: remove "simple-mfd" in the example
by Ryder Lee 28 Mar '18

28 Mar '18

Remove compatible string "simple-mfd" in the example as we have already added devm_of_platform_populate() in the parent driver. Signed-off-by: Ryder Lee <ryder.lee(a)mediatek.com> --- Documentation/devicetree/bindings/sound/mt2701-afe-pcm.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/sound/mt2701-afe-pcm.txt b/Documentation/devicetree/bindings/sound/mt2701-afe-pcm.txt index 6df87b9..e2f7f49 100644 --- a/Documentation/devicetree/bindings/sound/mt2701-afe-pcm.txt +++ b/Documentation/devicetree/bindings/sound/mt2701-afe-pcm.txt @@ -53,7 +53,7 @@ See ../arm/mediatek/mediatek,audsys.txt for details about the parent node. Example: audsys: audio-subsystem@11220000 { - compatible = "mediatek,mt2701-audsys", "syscon", "simple-mfd"; + compatible = "mediatek,mt2701-audsys", "syscon"; ... afe: audio-controller { -- 1.9.1

2 1

[alsa-devel] [PATCH] ASoC: tscs42xx: Remove owner assignment from i2c_driver
by Fabio Estevam 28 Mar '18

28 Mar '18

From: Fabio Estevam <fabio.estevam(a)nxp.com> Structure i2c_driver does not need to set the owner field, as this will be populated by the driver core. Generated by scripts/coccinelle/api/platform_no_drv_owner.cocci. Signed-off-by: Fabio Estevam <fabio.estevam(a)nxp.com> --- sound/soc/codecs/tscs42xx.c | 1 - 1 file changed, 1 deletion(-) diff --git a/sound/soc/codecs/tscs42xx.c b/sound/soc/codecs/tscs42xx.c index cfc71b5..bbfc73a 100644 --- a/sound/soc/codecs/tscs42xx.c +++ b/sound/soc/codecs/tscs42xx.c @@ -1443,7 +1443,6 @@ MODULE_DEVICE_TABLE(of, tscs42xx_of_match); static struct i2c_driver tscs42xx_i2c_driver = { .driver = { .name = "tscs42xx", - .owner = THIS_MODULE, .of_match_table = tscs42xx_of_match, }, .probe = tscs42xx_i2c_probe, -- 2.7.4

2 1

[alsa-devel] [PATCH] ASoC: Add accessor macros for TLV items
by Ranjani Sridharan 28 Mar '18

28 Mar '18

This patch adds macros for accessing the type, length, min, mute and step items in TLV data. These will be used by drivers to extract the TLV data while loading topology. Signed-off-by: Ranjani Sridharan <ranjani.sridharan(a)linux.intel.com> --- include/sound/tlv.h | 5 +++++ include/uapi/sound/tlv.h | 7 +++++++ 2 files changed, 12 insertions(+) diff --git a/include/sound/tlv.h b/include/sound/tlv.h index 3677ebb928d5..8fa379e8a568 100644 --- a/include/sound/tlv.h +++ b/include/sound/tlv.h @@ -49,6 +49,11 @@ #define TLV_DB_GAIN_MUTE SNDRV_CTL_TLVD_DB_GAIN_MUTE +#define TLV_ITEM_TYPE SNDRV_CTL_TLVD_ITEM_TYPE +#define TLV_ITEM_LEN SNDRV_CTL_TLVD_ITEM_LEN +#define TLV_DB_MIN SNDRV_CTL_TLVD_DB_SCALE_MIN +#define TLV_DB_MUTE_AND_STEP SNDRV_CTL_TLVD_DB_SCALE_MUTE_AND_STEP + /* * The below assumes that each item TLV is 4 words like DB_SCALE or LINEAR. * This is an old fasion and obsoleted by commit bf1d1c9b6179("ALSA: tlv: add diff --git a/include/uapi/sound/tlv.h b/include/uapi/sound/tlv.h index be5371f09a62..f3fba49ef6e8 100644 --- a/include/uapi/sound/tlv.h +++ b/include/uapi/sound/tlv.h @@ -60,6 +60,13 @@ unsigned int name[] = { \ SNDRV_CTL_TLVD_DB_SCALE_ITEM(min, step, mute) \ } +/* Accessor macros for TLV type and len */ +#define SNDRV_CTL_TLVD_ITEM_TYPE 0 +#define SNDRV_CTL_TLVD_ITEM_LEN 1 + +/* Accessor macros for min, mute and step values from dB scale type TLV */ +#define SNDRV_CTL_TLVD_DB_SCALE_MIN 2 +#define SNDRV_CTL_TLVD_DB_SCALE_MUTE_AND_STEP 3 /* dB scale specified with min/max values instead of step */ #define SNDRV_CTL_TLVD_DB_MINMAX_ITEM(min_dB, max_dB) \ -- 2.14.1

3 3

[alsa-devel] [RFC, alsa-utils] RFC: alsamixer: Improve description of playback switches
by Kirill Marinushkin 27 Mar '18

27 Mar '18

Currently, all playback switches are described as mute. It is shown as: [OO] == sound is on (left and right channels) [MM] == sound is muted (left and right channels) But cards can have different kinds of switch controls, not only mute. At least usb-audio provides boolean controls as playback switches for: * clock source validity (read only switch) * processing features on/off (rw switch) * potentially other similar use-cases It becames confusing, because it is shown as: [OO] == clock source is valid [MM] == clock source is invalid [OO] == processing feature is on [MM] == processing feature is off And from the help, it is not clear how to toggle such switches. In this RFC, I suggest to show switches more generic: [yy] == sound on / clock source is valid / processing feature is on [nn] == sound muted / clock source is invalid / processing feature is off And give a more generic description in the help. Signed-off-by: Kirill Marinushkin <k.marinushkin(a)gmail.com> Cc: Jaroslav Kysela <perex(a)perex.cz> Cc: Takashi Iwai <tiwai(a)suse.de> Cc: alsa-devel(a)alsa-project.org --- alsamixer/mixer_display.c | 8 ++++---- alsamixer/mixer_widget.c | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/alsamixer/mixer_display.c b/alsamixer/mixer_display.c index b1f79d0..f73b11a 100644 --- a/alsamixer/mixer_display.c +++ b/alsamixer/mixer_display.c @@ -513,13 +513,13 @@ static void display_control(unsigned int control_index) mvwaddch(mixer_widget.window, base_y + 1, frame_left + 1, switches[0] /* TRANSLATORS: playback on; one character */ - ? _("O")[0] | (control->flags & IS_ACTIVE ? attr_ctl_nomute : 0) + ? _("y")[0] | (control->flags & IS_ACTIVE ? attr_ctl_nomute : 0) /* TRANSLATORS: playback muted; one character */ - : _("M")[0] | (control->flags & IS_ACTIVE ? attr_ctl_mute : 0)); + : _("n")[0] | (control->flags & IS_ACTIVE ? attr_ctl_mute : 0)); waddch(mixer_widget.window, switches[1] - ? _("O")[0] | (control->flags & IS_ACTIVE ? attr_ctl_nomute : 0) - : _("M")[0] | (control->flags & IS_ACTIVE ? attr_ctl_mute : 0)); + ? _("y")[0] | (control->flags & IS_ACTIVE ? attr_ctl_nomute : 0) + : _("n")[0] | (control->flags & IS_ACTIVE ? attr_ctl_mute : 0)); } if (control->flags & TYPE_CSWITCH) { diff --git a/alsamixer/mixer_widget.c b/alsamixer/mixer_widget.c index 0c2b9e0..93d1923 100644 --- a/alsamixer/mixer_widget.c +++ b/alsamixer/mixer_widget.c @@ -210,9 +210,9 @@ static void show_help(void) _("Z X C Decrease left/both/right volumes"), _("B Balance left and right volumes"), "", - _("M Toggle mute"), + _("M Toggle mute / playback switch"), /* TRANSLATORS: or , . */ - _("< > Toggle left/right mute"), + _("< > Toggle left/right mute / playback switch"), "", _("Space Toggle capture"), /* TRANSLATORS: or Insert Delete */ -- 2.13.6

4 12

[alsa-devel] [PATCH v2 1/3] ASoC: AMD: Use single dai for da7219 playback and capture
by Akshu Agrawal 27 Mar '18

27 Mar '18

BT I2S is a bi-directional dai, we will use the same cpu dai for playback and capture. V2: reworded the commit message. TEST=aplay -D hw:0,0 -vv <file> arecord -D hw:0,0 -f dat -d 5 -vv <file> Signed-off-by: Akshu Agrawal <akshu.agrawal(a)amd.com> --- sound/soc/amd/acp-da7219-max98357a.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/sound/soc/amd/acp-da7219-max98357a.c b/sound/soc/amd/acp-da7219-max98357a.c index bbe0f10..d07c2a1 100644 --- a/sound/soc/amd/acp-da7219-max98357a.c +++ b/sound/soc/amd/acp-da7219-max98357a.c @@ -124,8 +124,8 @@ static int cz_fe_startup(struct snd_pcm_substream *substream) static struct snd_soc_dai_link cz_dai_7219_98357[] = { { - .name = "amd-da7219-play", - .stream_name = "Playback", + .name = "amd-da7219-play-cap", + .stream_name = "Playback and Capture", .platform_name = "acp_audio_dma.0.auto", .cpu_dai_name = "designware-i2s.3.auto", .codec_dai_name = "da7219-hifi", @@ -134,16 +134,6 @@ static int cz_fe_startup(struct snd_pcm_substream *substream) | SND_SOC_DAIFMT_CBM_CFM, .init = cz_da7219_init, .dpcm_playback = 1, - }, - { - .name = "amd-da7219-cap", - .stream_name = "Capture", - .platform_name = "acp_audio_dma.0.auto", - .cpu_dai_name = "designware-i2s.4.auto", - .codec_dai_name = "da7219-hifi", - .codec_name = "i2c-DLGS7219:00", - .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF - | SND_SOC_DAIFMT_CBM_CFM, .dpcm_capture = 1, .ops = &cz_da7219_cap_ops, }, -- 1.9.1

2 4

[alsa-devel] [PATCH v2] ASoC: topology: Fix bclk and fsync inversion in set_link_hw_format()
by Kirill Marinushkin 27 Mar '18

27 Mar '18

The values of bclk and fsync are inverted WRT the codec. But the existing solution already works for Broadwell, see the alsa-lib config: `alsa-lib/src/conf/topology/broadwell/broadwell.conf` This commit provides the backwards-compatible solution to fix this misuse. This commit goes in pair with the corresponding patch for alsa-lib. Signed-off-by: Kirill Marinushkin <k.marinushkin(a)gmail.com> Cc: Mark Brown <broonie(a)kernel.org> Cc: Takashi Iwai <tiwai(a)suse.de> Cc: Jaroslav Kysela <perex(a)perex.cz> Cc: Pierre-Louis Bossart <pierre-louis.bossart(a)linux.intel.com> Cc: Pan Xiuli <xiuli.pan(a)linux.intel.com> Cc: Liam Girdwood <liam.r.girdwood(a)linux.intel.com> Cc: linux-kernel(a)vger.kernel.org Cc: alsa-devel(a)alsa-project.org --- include/uapi/sound/asoc.h | 16 ++++++++++++++-- sound/soc/soc-topology.c | 12 +++++++----- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/include/uapi/sound/asoc.h b/include/uapi/sound/asoc.h index 69c37ecbff7e..f0e5e21efa54 100644 --- a/include/uapi/sound/asoc.h +++ b/include/uapi/sound/asoc.h @@ -160,6 +160,18 @@ #define SND_SOC_TPLG_LNK_FLGBIT_SYMMETRIC_SAMPLEBITS (1 << 2) #define SND_SOC_TPLG_LNK_FLGBIT_VOICE_WAKEUP (1 << 3) +/* DAI topology BCLK parameter + * For the backwards capability, by default codec is bclk master + */ +#define SND_SOC_TPLG_BCLK_CM 0 /* codec is bclk master */ +#define SND_SOC_TPLG_BCLK_CS 1 /* codec is bclk slave */ + +/* DAI topology FSYNC parameter + * For the backwards capability, by default codec is fsync master + */ +#define SND_SOC_TPLG_FSYNC_CM 0 /* codec is fsync master */ +#define SND_SOC_TPLG_FSYNC_CS 1 /* codec is fsync slave */ + /* * Block Header. * This header precedes all object and object arrays below. @@ -315,8 +327,8 @@ struct snd_soc_tplg_hw_config { __u8 clock_gated; /* 1 if clock can be gated to save power */ __u8 invert_bclk; /* 1 for inverted BCLK, 0 for normal */ __u8 invert_fsync; /* 1 for inverted frame clock, 0 for normal */ - __u8 bclk_master; /* 1 for master of BCLK, 0 for slave */ - __u8 fsync_master; /* 1 for master of FSYNC, 0 for slave */ + __u8 bclk_master; /* SND_SOC_TPLG_BCLK_ value */ + __u8 fsync_master; /* SND_SOC_TPLG_FSYNC_ value */ __u8 mclk_direction; /* 0 for input, 1 for output */ __le16 reserved; /* for 32bit alignment */ __le32 mclk_rate; /* MCLK or SYSCLK freqency in Hz */ diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index 01a50413c66f..c5bdc673b195 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -1994,13 +1994,15 @@ static void set_link_hw_format(struct snd_soc_dai_link *link, link->dai_fmt |= SND_SOC_DAIFMT_IB_IF; /* clock masters */ - bclk_master = hw_config->bclk_master; - fsync_master = hw_config->fsync_master; - if (!bclk_master && !fsync_master) + bclk_master = (hw_config->bclk_master == + SND_SOC_TPLG_BCLK_CM); + fsync_master = (hw_config->fsync_master == + SND_SOC_TPLG_FSYNC_CM); + if (bclk_master && fsync_master) link->dai_fmt |= SND_SOC_DAIFMT_CBM_CFM; - else if (bclk_master && !fsync_master) - link->dai_fmt |= SND_SOC_DAIFMT_CBS_CFM; else if (!bclk_master && fsync_master) + link->dai_fmt |= SND_SOC_DAIFMT_CBS_CFM; + else if (bclk_master && !fsync_master) link->dai_fmt |= SND_SOC_DAIFMT_CBM_CFS; else link->dai_fmt |= SND_SOC_DAIFMT_CBS_CFS; -- 2.13.6

3 13

[alsa-devel] [PATCH 3/3 v2] ASoC: TSCS42xx: Add CCF support to get sysclk
by Steven Eckhoff 27 Mar '18

27 Mar '18

The TSCS42xx relies on set_sysclk to get a unique clock id and rate, which prevents it from being used with the simple-card. Remove set_sysclk callback Add CCF support to get clock id and rate Add clocks and clock-names to device tree binding Signed-off-by: Steven Eckhoff <steven.eckhoff.opensource(a)gmail.com> --- .../devicetree/bindings/sound/tscs42xx.txt | 6 ++ sound/soc/codecs/tscs42xx.c | 104 ++++++++++++++------- sound/soc/codecs/tscs42xx.h | 2 +- 3 files changed, 75 insertions(+), 37 deletions(-) diff --git a/Documentation/devicetree/bindings/sound/tscs42xx.txt b/Documentation/devicetree/bindings/sound/tscs42xx.txt index 2ac2f0996697..7eea32e9d078 100644 --- a/Documentation/devicetree/bindings/sound/tscs42xx.txt +++ b/Documentation/devicetree/bindings/sound/tscs42xx.txt @@ -8,9 +8,15 @@ Required Properties: - reg : <0x71> for analog mic <0x69> for digital mic + - clock-names: Must one of the following "mclk1", "xtal", "mclk2" + + - clocks: phandle of the clock that provides the codec sysclk + Example: wookie: codec@69 { compatible = "tempo,tscs42A2"; reg = <0x69>; + clock-names = "xtal"; + clocks = <&audio_xtal>; }; diff --git a/sound/soc/codecs/tscs42xx.c b/sound/soc/codecs/tscs42xx.c index d41852a37ac6..69cd4c1ed1d6 100644 --- a/sound/soc/codecs/tscs42xx.c +++ b/sound/soc/codecs/tscs42xx.c @@ -12,6 +12,7 @@ #include <linux/module.h> #include <linux/delay.h> #include <linux/mutex.h> +#include <linux/clk.h> #include <sound/tlv.h> #include <sound/pcm_params.h> #include <sound/soc.h> @@ -40,6 +41,9 @@ struct tscs42xx { struct mutex pll_lock; struct regmap *regmap; + + struct clk *sysclk; + int sysclk_src_id; }; struct coeff_ram_ctl { @@ -1251,13 +1255,46 @@ static int tscs42xx_set_dai_bclk_ratio(struct snd_soc_dai *codec_dai, return 0; } -static int tscs42xx_set_dai_sysclk(struct snd_soc_dai *codec_dai, - int clk_id, unsigned int freq, int dir) +static const struct snd_soc_dai_ops tscs42xx_dai_ops = { + .hw_params = tscs42xx_hw_params, + .mute_stream = tscs42xx_mute_stream, + .set_fmt = tscs42xx_set_dai_fmt, + .set_bclk_ratio = tscs42xx_set_dai_bclk_ratio, +}; + +static int part_is_valid(struct tscs42xx *tscs42xx) { - struct snd_soc_component *component = codec_dai->component; + int val; int ret; + unsigned int reg; + + ret = regmap_read(tscs42xx->regmap, R_DEVIDH, &reg); + if (ret < 0) + return ret; - switch (clk_id) { + val = reg << 8; + ret = regmap_read(tscs42xx->regmap, R_DEVIDL, &reg); + if (ret < 0) + return ret; + + val |= reg; + + switch (val) { + case 0x4A74: + case 0x4A73: + return true; + default: + return false; + }; +} + +static int set_sysclk(struct snd_soc_component *component) +{ + struct tscs42xx *tscs42xx = snd_soc_component_get_drvdata(component); + unsigned long freq; + int ret; + + switch (tscs42xx->sysclk_src_id) { case TSCS42XX_PLL_SRC_XTAL: case TSCS42XX_PLL_SRC_MCLK1: ret = snd_soc_component_write(component, R_PLLREFSEL, @@ -1285,6 +1322,7 @@ static int tscs42xx_set_dai_sysclk(struct snd_soc_dai *codec_dai, return -EINVAL; } + freq = clk_get_rate(tscs42xx->sysclk); ret = set_pll_ctl_from_input_freq(component, freq); if (ret < 0) { dev_err(component->dev, @@ -1295,41 +1333,13 @@ static int tscs42xx_set_dai_sysclk(struct snd_soc_dai *codec_dai, return 0; } -static const struct snd_soc_dai_ops tscs42xx_dai_ops = { - .hw_params = tscs42xx_hw_params, - .mute_stream = tscs42xx_mute_stream, - .set_fmt = tscs42xx_set_dai_fmt, - .set_bclk_ratio = tscs42xx_set_dai_bclk_ratio, - .set_sysclk = tscs42xx_set_dai_sysclk, -}; - -static int part_is_valid(struct tscs42xx *tscs42xx) +static int tscs42xx_probe(struct snd_soc_component *component) { - int val; - int ret; - unsigned int reg; - - ret = regmap_read(tscs42xx->regmap, R_DEVIDH, &reg); - if (ret < 0) - return ret; - - val = reg << 8; - ret = regmap_read(tscs42xx->regmap, R_DEVIDL, &reg); - if (ret < 0) - return ret; - - val |= reg; - - switch (val) { - case 0x4A74: - case 0x4A73: - return true; - default: - return false; - }; + return set_sysclk(component); } static const struct snd_soc_component_driver soc_codec_dev_tscs42xx = { + .probe = tscs42xx_probe, .dapm_widgets = tscs42xx_dapm_widgets, .num_dapm_widgets = ARRAY_SIZE(tscs42xx_dapm_widgets), .dapm_routes = tscs42xx_intercon, @@ -1387,11 +1397,15 @@ static const struct reg_sequence tscs42xx_patch[] = { { R_AIC2, RV_AIC2_BLRCM_DAC_BCLK_LRCLK_SHARED }, }; +static char const * const src_names[TSCS42XX_PLL_SRC_CNT] = { + "xtal", "mclk1", "mclk2"}; + static int tscs42xx_i2c_probe(struct i2c_client *i2c, const struct i2c_device_id *id) { struct tscs42xx *tscs42xx; - int ret = 0; + int src; + int ret; tscs42xx = devm_kzalloc(&i2c->dev, sizeof(*tscs42xx), GFP_KERNEL); if (!tscs42xx) { @@ -1402,6 +1416,24 @@ static int tscs42xx_i2c_probe(struct i2c_client *i2c, } i2c_set_clientdata(i2c, tscs42xx); + for (src = TSCS42XX_PLL_SRC_XTAL; src < TSCS42XX_PLL_SRC_CNT; src++) { + tscs42xx->sysclk = devm_clk_get(&i2c->dev, src_names[src]); + if (!IS_ERR(tscs42xx->sysclk)) { + break; + } else if (PTR_ERR(tscs42xx->sysclk) != -ENOENT) { + ret = PTR_ERR(tscs42xx->sysclk); + dev_err(&i2c->dev, "Failed to get sysclk (%d)\n", ret); + return ret; + } + } + if (src > TSCS42XX_PLL_SRC_CNT) { + ret = -EINVAL; + dev_err(&i2c->dev, "Failed to get a valid clock name (%d)\n", + ret); + return ret; + } + tscs42xx->sysclk_src_id = src; + tscs42xx->regmap = devm_regmap_init_i2c(i2c, &tscs42xx_regmap); if (IS_ERR(tscs42xx->regmap)) { ret = PTR_ERR(tscs42xx->regmap); diff --git a/sound/soc/codecs/tscs42xx.h b/sound/soc/codecs/tscs42xx.h index d4a30bcbf64b..814c8f3c4a68 100644 --- a/sound/soc/codecs/tscs42xx.h +++ b/sound/soc/codecs/tscs42xx.h @@ -7,10 +7,10 @@ #define __WOOKIE_H__ enum { - TSCS42XX_PLL_SRC_NONE, TSCS42XX_PLL_SRC_XTAL, TSCS42XX_PLL_SRC_MCLK1, TSCS42XX_PLL_SRC_MCLK2, + TSCS42XX_PLL_SRC_CNT, }; #define R_HPVOLL 0x0 -- 2.15.1

1 0

[alsa-devel] No audio on Lenovo Miix 320
by Freaky 27 Mar '18

27 Mar '18

Hi there, we're having some issues getting the audio working on a Lenovo Miix 320. On some forums they say it should work after blacklisting snd_hdmi_lpe_audio. Have blacklisted this, and there's one audio card less now, but still no go unfortunately. Currently running with 4.15.10 from Fedora 27. It logs a lot of: "Audio Port: ASoC: no backend DAIs enabled for Audio Port" lines. Any ideas on how to troubleshoot this? Saw similar problems in https://bugzilla.kernel.org/show_bug.cgi?id=96691 but this seems to be for another codec/DAI. Also, their system seems to be a Baytrail Atom, this is a Cherrytrail. Not sure if any of the patches are applicable. dmesg is attached. I can patch, build and test kernels. Coding is another story however :). Any help is appreciated. Kind regards,

2 1

[alsa-devel] [PATCH] ALSA: usb-audio: fix memory leak on cval
by Colin King 27 Mar '18

27 Mar '18

From: Colin Ian King <colin.king(a)canonical.com> With the current exit return path of the ctl_info allocation failure cval is not being freed resulting in a memory leak. Fix this by kfree'ing it on the return. Detected by CoverityScan, CID#1466878 ("Resource Leak") Fixes: 21e9b3e931f7 ("ALSA: usb-audio: fix uac control query argument") Signed-off-by: Colin Ian King <colin.king(a)canonical.com> --- sound/usb/mixer.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c index 04dab6f65535..301ad61ed426 100644 --- a/sound/usb/mixer.c +++ b/sound/usb/mixer.c @@ -1365,8 +1365,10 @@ static void build_feature_ctl(struct mixer_build *state, void *raw_desc, cval->cmask = ctl_mask; ctl_info = get_feature_control_info(control); - if (!ctl_info) + if (!ctl_info) { + kfree(cval); return; + } if (state->mixer->protocol == UAC_VERSION_1) cval->val_type = ctl_info->type; else /* UAC_VERSION_2 */ -- 2.15.1

2 1

Re: [alsa-devel] [PATCH] [sound] hdac-codec runtime suspended at PM:Suspend.
by Subhransu S. Prusty 27 Mar '18

27 Mar '18

On Thu, Mar 15, 2018 at 09:02:30PM +0530, Anshuman Gupta wrote: > On Thu, Mar 15, 2018 at 04:12:44PM +0530, Subhransu S. Prusty wrote: > > On Wed, Mar 14, 2018 at 09:07:14PM +0530, Anshuman Gupta wrote: > > > On Wed, Mar 14, 2018 at 11:53:58AM +0100, Rafael J. Wysocki wrote: > > > > On Wed, Mar 14, 2018 at 11:38 AM, Anshuman Gupta > > > > <anshuman.gupta(a)intel.com> wrote: > > > > > On Mon, Mar 12, 2018 at 12:26:53PM +0100, Rafael J. Wysocki wrote: > > > > >> On Mon, Mar 12, 2018 at 12:17 PM, Anshuman Gupta > > > > >> <anshuman.gupta(a)intel.com> wrote: > > > > >> > > > > > >> > + if (pm_runtime_status_suspended(dev)) > > > > >> > + return; > > > > >> > > > > >> That, again, is somewhat fragile from the concurrency perspective. > > > > >> > > > > > > > > And here you want to avoid the below if the device is still suspended. > > > Yes, if we do not avoid the code below, complete callback takes about > > > 3 seconds due to snd_hdac_codec_read timed out because hdac controller > > > would be in runtime suspend state. > > > > > > > > Why is the below code located in the ->complete callback anyway? > > > > Shouldn't it be there in the ->resume one? > > > > > > > Yes even i am also having same doubt, why these power down and power up > > > sequences are part of prepare and complete callback. > > > Adding driver author "Subhransu S. Prusty" to provide more inputs on this. > > > > This driver needs a late resume as it receives a jack notification from the > > i915 driver and the skl controller driver resume may not have happened and > > in turn hda controller may not ready. This ensures a synchronization for > > jack event during resume from S3. > Let me give you insight of the issue, this driver blocks the direct complete > of hda controller PCI 00:1f.3, sometimes it takes 280ms to resume hda controller > from S3 and s2idle. So it does not make sense to suspend/resume hda controller > when it is already in runtime suspend. I get it now. I think this patch needs rework to address both the issues. > > > > I think this patch defeats the purpose. > Here in this case PCI driver may kick the direct complete for hda controller > https://elixir.bootlin.com/linux/latest/source/drivers/pci/pci-driver.c#L691 > But hdac hdmi codec driver is blocking it. > So i think it will be ok to keep this codec and hda controller in runtime > suspend while entering S3 or s2idle, both can resume by runtime PM as well, > will it brake any audio functionality? There was some PM and jack detection issues (I don't recollect now) because of which this was added. This was due to the gfx display power and hda controller synchronization. The rework may be required in both hdac_hdmi and skylake drivers. Regards, Subhransu > > > > Regards, > > Subhransu > > > > > > >> > /* Power up afg */ > > > > >> > snd_hdac_codec_read(hdac, hdac->afg, 0, AC_VERB_SET_POWER_STATE, > > > > >> > AC_PWRST_D0); > > > > >> > -- > > > > >> > 2.7.4 > > > > > > -- > > > Thanks, > > > Anshuman > > > > -- > > -- > Thanks, > Anshuman --

3 2