[alsa-devel] [PATCH 1/4] ASoC: wm_adsp: Correct cache handling of new kernel control API
The recently added API that exposes firmware mixer controls to the kernel is missing cache handling and all writes bypass the cache, this obviously causes the cache to get out of sync with the hardware. Factor out the cache handling into two new helper functions and call those from both the normal ALSA control handlers and the new kernel API.
Fixes: eb65ccdb0836 ("ASoC: wm_adsp: Expose mixer control API") Signed-off-by: Charles Keepax ckeepax@opensource.cirrus.com --- sound/soc/codecs/wm_adsp.c | 98 ++++++++++++++++++++++++---------------------- 1 file changed, 51 insertions(+), 47 deletions(-)
diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c index 2a9b610f6d435..d3d32b501acae 100644 --- a/sound/soc/codecs/wm_adsp.c +++ b/sound/soc/codecs/wm_adsp.c @@ -1030,8 +1030,8 @@ static int wm_coeff_write_acked_control(struct wm_coeff_ctl *ctl, return -ETIMEDOUT; }
-static int wm_coeff_write_control(struct wm_coeff_ctl *ctl, - const void *buf, size_t len) +static int wm_coeff_write_ctrl_raw(struct wm_coeff_ctl *ctl, + const void *buf, size_t len) { struct wm_adsp *dsp = ctl->dsp; void *scratch; @@ -1061,6 +1061,23 @@ static int wm_coeff_write_control(struct wm_coeff_ctl *ctl, return 0; }
+static int wm_coeff_write_ctrl(struct wm_coeff_ctl *ctl, + const void *buf, size_t len) +{ + int ret = 0; + + if (ctl->flags & WMFW_CTL_FLAG_VOLATILE) + ret = -EPERM; + else if (buf != ctl->cache) + memcpy(ctl->cache, buf, len); + + ctl->set = 1; + if (ctl->enabled && ctl->dsp->running) + ret = wm_coeff_write_ctrl_raw(ctl, buf, len); + + return ret; +} + static int wm_coeff_put(struct snd_kcontrol *kctl, struct snd_ctl_elem_value *ucontrol) { @@ -1071,16 +1088,7 @@ static int wm_coeff_put(struct snd_kcontrol *kctl, int ret = 0;
mutex_lock(&ctl->dsp->pwr_lock); - - if (ctl->flags & WMFW_CTL_FLAG_VOLATILE) - ret = -EPERM; - else - memcpy(ctl->cache, p, ctl->len); - - ctl->set = 1; - if (ctl->enabled && ctl->dsp->running) - ret = wm_coeff_write_control(ctl, p, ctl->len); - + ret = wm_coeff_write_ctrl(ctl, p, ctl->len); mutex_unlock(&ctl->dsp->pwr_lock);
return ret; @@ -1096,15 +1104,10 @@ static int wm_coeff_tlv_put(struct snd_kcontrol *kctl,
mutex_lock(&ctl->dsp->pwr_lock);
- if (copy_from_user(ctl->cache, bytes, size)) { + if (copy_from_user(ctl->cache, bytes, size)) ret = -EFAULT; - } else { - ctl->set = 1; - if (ctl->enabled && ctl->dsp->running) - ret = wm_coeff_write_control(ctl, ctl->cache, size); - else if (ctl->flags & WMFW_CTL_FLAG_VOLATILE) - ret = -EPERM; - } + else + ret = wm_coeff_write_ctrl(ctl, ctl->cache, size);
mutex_unlock(&ctl->dsp->pwr_lock);
@@ -1135,8 +1138,8 @@ static int wm_coeff_put_acked(struct snd_kcontrol *kctl, return ret; }
-static int wm_coeff_read_control(struct wm_coeff_ctl *ctl, - void *buf, size_t len) +static int wm_coeff_read_ctrl_raw(struct wm_coeff_ctl *ctl, + void *buf, size_t len) { struct wm_adsp *dsp = ctl->dsp; void *scratch; @@ -1166,29 +1169,37 @@ static int wm_coeff_read_control(struct wm_coeff_ctl *ctl, return 0; }
-static int wm_coeff_get(struct snd_kcontrol *kctl, - struct snd_ctl_elem_value *ucontrol) +static int wm_coeff_read_ctrl(struct wm_coeff_ctl *ctl, void *buf, size_t len) { - struct soc_bytes_ext *bytes_ext = - (struct soc_bytes_ext *)kctl->private_value; - struct wm_coeff_ctl *ctl = bytes_ext_to_ctl(bytes_ext); - char *p = ucontrol->value.bytes.data; int ret = 0;
- mutex_lock(&ctl->dsp->pwr_lock); - if (ctl->flags & WMFW_CTL_FLAG_VOLATILE) { if (ctl->enabled && ctl->dsp->running) - ret = wm_coeff_read_control(ctl, p, ctl->len); + return wm_coeff_read_ctrl_raw(ctl, buf, len); else - ret = -EPERM; + return -EPERM; } else { if (!ctl->flags && ctl->enabled && ctl->dsp->running) - ret = wm_coeff_read_control(ctl, ctl->cache, ctl->len); + ret = wm_coeff_read_ctrl_raw(ctl, ctl->cache, ctl->len);
- memcpy(p, ctl->cache, ctl->len); + if (buf != ctl->cache) + memcpy(buf, ctl->cache, len); }
+ return ret; +} + +static int wm_coeff_get(struct snd_kcontrol *kctl, + struct snd_ctl_elem_value *ucontrol) +{ + struct soc_bytes_ext *bytes_ext = + (struct soc_bytes_ext *)kctl->private_value; + struct wm_coeff_ctl *ctl = bytes_ext_to_ctl(bytes_ext); + char *p = ucontrol->value.bytes.data; + int ret; + + mutex_lock(&ctl->dsp->pwr_lock); + ret = wm_coeff_read_ctrl(ctl, p, ctl->len); mutex_unlock(&ctl->dsp->pwr_lock);
return ret; @@ -1204,15 +1215,7 @@ static int wm_coeff_tlv_get(struct snd_kcontrol *kctl,
mutex_lock(&ctl->dsp->pwr_lock);
- if (ctl->flags & WMFW_CTL_FLAG_VOLATILE) { - if (ctl->enabled && ctl->dsp->running) - ret = wm_coeff_read_control(ctl, ctl->cache, size); - else - ret = -EPERM; - } else { - if (!ctl->flags && ctl->enabled && ctl->dsp->running) - ret = wm_coeff_read_control(ctl, ctl->cache, size); - } + ret = wm_coeff_read_ctrl_raw(ctl, ctl->cache, size);
if (!ret && copy_to_user(bytes, ctl->cache, size)) ret = -EFAULT; @@ -1340,7 +1343,7 @@ static int wm_coeff_init_control_caches(struct wm_adsp *dsp) * created so we don't need to do anything. */ if (!ctl->flags || (ctl->flags & WMFW_CTL_FLAG_READABLE)) { - ret = wm_coeff_read_control(ctl, ctl->cache, ctl->len); + ret = wm_coeff_read_ctrl_raw(ctl, ctl->cache, ctl->len); if (ret < 0) return ret; } @@ -1358,7 +1361,8 @@ static int wm_coeff_sync_controls(struct wm_adsp *dsp) if (!ctl->enabled) continue; if (ctl->set && !(ctl->flags & WMFW_CTL_FLAG_VOLATILE)) { - ret = wm_coeff_write_control(ctl, ctl->cache, ctl->len); + ret = wm_coeff_write_ctrl_raw(ctl, ctl->cache, + ctl->len); if (ret < 0) return ret; } @@ -2048,7 +2052,7 @@ int wm_adsp_write_ctl(struct wm_adsp *dsp, const char *name, int type, if (len > ctl->len) return -EINVAL;
- ret = wm_coeff_write_control(ctl, buf, len); + ret = wm_coeff_write_ctrl(ctl, buf, len);
kcontrol = snd_soc_card_get_kcontrol(dsp->component->card, ctl->name); snd_ctl_notify(dsp->component->card->snd_card, @@ -2070,7 +2074,7 @@ int wm_adsp_read_ctl(struct wm_adsp *dsp, const char *name, int type, if (len > ctl->len) return -EINVAL;
- return wm_coeff_read_control(ctl, buf, len); + return wm_coeff_read_ctrl(ctl, buf, len); } EXPORT_SYMBOL_GPL(wm_adsp_read_ctl);
Both the data and clock should be connected to both the left and right inputs for DMIC only inputs, add the missing routes.
Signed-off-by: Charles Keepax ckeepax@opensource.cirrus.com --- sound/soc/codecs/cs47l85.c | 6 ++++++ sound/soc/codecs/cs47l90.c | 4 ++++ 2 files changed, 10 insertions(+)
diff --git a/sound/soc/codecs/cs47l85.c b/sound/soc/codecs/cs47l85.c index f85581a97bdc3..05445abcb470b 100644 --- a/sound/soc/codecs/cs47l85.c +++ b/sound/soc/codecs/cs47l85.c @@ -2008,12 +2008,18 @@ static const struct snd_soc_dapm_route cs47l85_dapm_routes[] = { { "IN3R", NULL, "IN3R Mode" },
{ "IN4L", NULL, "DMICCLK4" }, + { "IN4L", NULL, "DMICDAT4" }, + { "IN4R", NULL, "DMICCLK4" }, { "IN4R", NULL, "DMICDAT4" },
{ "IN5L", NULL, "DMICCLK5" }, + { "IN5L", NULL, "DMICDAT5" }, + { "IN5R", NULL, "DMICCLK5" }, { "IN5R", NULL, "DMICDAT5" },
{ "IN6L", NULL, "DMICCLK6" }, + { "IN6L", NULL, "DMICDAT6" }, + { "IN6R", NULL, "DMICCLK6" }, { "IN6R", NULL, "DMICDAT6" },
MADERA_MIXER_ROUTES("OUT1L", "HPOUT1L"), diff --git a/sound/soc/codecs/cs47l90.c b/sound/soc/codecs/cs47l90.c index c3b6f4d41005b..35ff5a923e282 100644 --- a/sound/soc/codecs/cs47l90.c +++ b/sound/soc/codecs/cs47l90.c @@ -1941,9 +1941,13 @@ static const struct snd_soc_dapm_route cs47l90_dapm_routes[] = { { "IN3R", NULL, "DMICDAT3" },
{ "IN4L", NULL, "DMICCLK4" }, + { "IN4L", NULL, "DMICDAT4" }, + { "IN4R", NULL, "DMICCLK4" }, { "IN4R", NULL, "DMICDAT4" },
{ "IN5L", NULL, "DMICCLK5" }, + { "IN5L", NULL, "DMICDAT5" }, + { "IN5R", NULL, "DMICCLK5" }, { "IN5R", NULL, "DMICDAT5" },
MADERA_MIXER_ROUTES("OUT1L", "HPOUT1L"),
On Tue, Jan 14, 2020 at 04:18:39PM +0000, Charles Keepax wrote:
diff --git a/sound/soc/codecs/cs47l90.c b/sound/soc/codecs/cs47l90.c index c3b6f4d41005b..35ff5a923e282 100644 --- a/sound/soc/codecs/cs47l90.c +++ b/sound/soc/codecs/cs47l90.c @@ -1941,9 +1941,13 @@ static const struct snd_soc_dapm_route cs47l90_dapm_routes[] = { { "IN3R", NULL, "DMICDAT3" },
IN3 should be updated here as well, sorry will respin.
Thanks, Charles
{ "IN4L", NULL, "DMICCLK4" },
{ "IN4L", NULL, "DMICDAT4" },
{ "IN4R", NULL, "DMICCLK4" }, { "IN4R", NULL, "DMICDAT4" },
{ "IN5L", NULL, "DMICCLK5" },
{ "IN5L", NULL, "DMICDAT5" },
{ "IN5R", NULL, "DMICCLK5" }, { "IN5R", NULL, "DMICDAT5" },
MADERA_MIXER_ROUTES("OUT1L", "HPOUT1L"),
-- 2.11.0
Should the write to MADERA_OUTPUT_ENABLES_1 fail and out_clamp[0] not be set an additional error message will be printed. Clear the ret variable to avoid this.
Signed-off-by: Charles Keepax ckeepax@opensource.cirrus.com --- sound/soc/codecs/madera.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/sound/soc/codecs/madera.c b/sound/soc/codecs/madera.c index 12dc468ae6bf0..2be64422adf9b 100644 --- a/sound/soc/codecs/madera.c +++ b/sound/soc/codecs/madera.c @@ -575,6 +575,7 @@ int madera_out1_demux_put(struct snd_kcontrol *kcontrol, usleep_range(2000, 3000); /* wait for wseq to complete */
/* change demux setting */ + ret = 0; if (madera->out_clamp[0]) ret = regmap_update_bits(madera->regmap, MADERA_OUTPUT_ENABLES_1,
The patch
ASoC: madera: Correct error path handling in madera_out1_demux_put
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.6
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 4061fdc89ff7704a71dc6df8e5841f861138e7e1 Mon Sep 17 00:00:00 2001
From: Charles Keepax ckeepax@opensource.cirrus.com Date: Tue, 14 Jan 2020 16:18:40 +0000 Subject: [PATCH] ASoC: madera: Correct error path handling in madera_out1_demux_put
Should the write to MADERA_OUTPUT_ENABLES_1 fail and out_clamp[0] not be set an additional error message will be printed. Clear the ret variable to avoid this.
Signed-off-by: Charles Keepax ckeepax@opensource.cirrus.com Link: https://lore.kernel.org/r/20200114161841.451-3-ckeepax@opensource.cirrus.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/codecs/madera.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/sound/soc/codecs/madera.c b/sound/soc/codecs/madera.c index 12dc468ae6bf..2be64422adf9 100644 --- a/sound/soc/codecs/madera.c +++ b/sound/soc/codecs/madera.c @@ -575,6 +575,7 @@ int madera_out1_demux_put(struct snd_kcontrol *kcontrol, usleep_range(2000, 3000); /* wait for wseq to complete */
/* change demux setting */ + ret = 0; if (madera->out_clamp[0]) ret = regmap_update_bits(madera->regmap, MADERA_OUTPUT_ENABLES_1,
Correctly link both channels on the DAC if an output muxed between a stereo and mono output. Without this one channel of the DAC may be erroneously powered down whilst in mono mode.
Signed-off-by: Charles Keepax ckeepax@opensource.cirrus.com --- sound/soc/codecs/cs47l15.c | 13 ++++++++++++- sound/soc/codecs/cs47l35.c | 12 +++++++++++- sound/soc/codecs/cs47l85.c | 3 ++- sound/soc/codecs/cs47l90.c | 3 ++- sound/soc/codecs/cs47l92.c | 14 +++++++++++++- sound/soc/codecs/madera.c | 13 ++++++++++--- sound/soc/codecs/madera.h | 4 +++- 7 files changed, 53 insertions(+), 9 deletions(-)
diff --git a/sound/soc/codecs/cs47l15.c b/sound/soc/codecs/cs47l15.c index 513e2875e4c6e..e8840dc142ef3 100644 --- a/sound/soc/codecs/cs47l15.c +++ b/sound/soc/codecs/cs47l15.c @@ -531,6 +531,7 @@ SND_SOC_DAPM_OUTPUT("DRC2 Signal Activity"), SND_SOC_DAPM_OUTPUT("DSP Trigger Out"),
SND_SOC_DAPM_DEMUX("HPOUT1 Demux", SND_SOC_NOPM, 0, 0, &cs47l15_outdemux), +SND_SOC_DAPM_MUX("HPOUT1 Mono Mux", SND_SOC_NOPM, 0, 0, &cs47l15_outdemux),
SND_SOC_DAPM_PGA("PWM1 Driver", MADERA_PWM_DRIVE_1, MADERA_PWM1_ENA_SHIFT, 0, NULL, 0), @@ -1086,6 +1087,9 @@ static const struct snd_soc_dapm_route cs47l15_dapm_routes[] = { { "AEC2 Loopback", "HPOUT1R", "OUT1R" }, { "HPOUT1 Demux", NULL, "OUT1L" }, { "HPOUT1 Demux", NULL, "OUT1R" }, + + { "OUT1R", NULL, "HPOUT1 Mono Mux" }, + { "HPOUTL", "HPOUT", "HPOUT1 Demux" }, { "HPOUTR", "HPOUT", "HPOUT1 Demux" }, { "EPOUTP", "EPOUT", "HPOUT1 Demux" }, @@ -1263,6 +1267,11 @@ static irqreturn_t cs47l15_adsp2_irq(int irq, void *data) return IRQ_HANDLED; }
+static const struct snd_soc_dapm_route cs47l15_mono_routes[] = { + { "HPOUT1 Mono Mux", "HPOUT", "OUT1L" }, + { "HPOUT1 Mono Mux", "EPOUT", "OUT1L" }, +}; + static int cs47l15_component_probe(struct snd_soc_component *component) { struct cs47l15 *cs47l15 = snd_soc_component_get_drvdata(component); @@ -1279,7 +1288,9 @@ static int cs47l15_component_probe(struct snd_soc_component *component) if (ret) return ret;
- ret = madera_init_outputs(component, CS47L15_MONO_OUTPUTS); + ret = madera_init_outputs(component, cs47l15_mono_routes, + ARRAY_SIZE(cs47l15_mono_routes), + CS47L15_MONO_OUTPUTS); if (ret) return ret;
diff --git a/sound/soc/codecs/cs47l35.c b/sound/soc/codecs/cs47l35.c index ba7aa6b31205c..3d48a0d9ecc58 100644 --- a/sound/soc/codecs/cs47l35.c +++ b/sound/soc/codecs/cs47l35.c @@ -633,6 +633,7 @@ SND_SOC_DAPM_OUTPUT("DRC2 Signal Activity"), SND_SOC_DAPM_OUTPUT("DSP Trigger Out"),
SND_SOC_DAPM_DEMUX("HPOUT1 Demux", SND_SOC_NOPM, 0, 0, &cs47l35_outdemux), +SND_SOC_DAPM_MUX("HPOUT1 Mono Mux", SND_SOC_NOPM, 0, 0, &cs47l35_outdemux),
SND_SOC_DAPM_PGA("PWM1 Driver", MADERA_PWM_DRIVE_1, MADERA_PWM1_ENA_SHIFT, 0, NULL, 0), @@ -1311,6 +1312,8 @@ static const struct snd_soc_dapm_route cs47l35_dapm_routes[] = { { "SPKOUTN", NULL, "OUT4L" }, { "SPKOUTP", NULL, "OUT4L" },
+ { "OUT1R", NULL, "HPOUT1 Mono Mux" }, + { "HPOUTL", "HPOUT", "HPOUT1 Demux" }, { "HPOUTR", "HPOUT", "HPOUT1 Demux" }, { "EPOUTP", "EPOUT", "HPOUT1 Demux" }, @@ -1554,6 +1557,11 @@ static irqreturn_t cs47l35_adsp2_irq(int irq, void *data) return IRQ_HANDLED; }
+static const struct snd_soc_dapm_route cs47l35_mono_routes[] = { + { "HPOUT1 Mono Mux", "HPOUT", "OUT1L" }, + { "HPOUT1 Mono Mux", "EPOUT", "OUT1L" }, +}; + static int cs47l35_component_probe(struct snd_soc_component *component) { struct cs47l35 *cs47l35 = snd_soc_component_get_drvdata(component); @@ -1570,7 +1578,9 @@ static int cs47l35_component_probe(struct snd_soc_component *component) if (ret) return ret;
- ret = madera_init_outputs(component, CS47L35_MONO_OUTPUTS); + ret = madera_init_outputs(component, cs47l35_mono_routes, + ARRAY_SIZE(cs47l35_mono_routes), + CS47L35_MONO_OUTPUTS); if (ret) return ret;
diff --git a/sound/soc/codecs/cs47l85.c b/sound/soc/codecs/cs47l85.c index 05445abcb470b..bef3471f482d4 100644 --- a/sound/soc/codecs/cs47l85.c +++ b/sound/soc/codecs/cs47l85.c @@ -2516,7 +2516,8 @@ static int cs47l85_component_probe(struct snd_soc_component *component) if (ret) return ret;
- ret = madera_init_outputs(component, CS47L85_MONO_OUTPUTS); + ret = madera_init_outputs(component, NULL, CS47L85_MONO_OUTPUTS, + CS47L85_MONO_OUTPUTS); if (ret) return ret;
diff --git a/sound/soc/codecs/cs47l90.c b/sound/soc/codecs/cs47l90.c index 35ff5a923e282..2d24466fca190 100644 --- a/sound/soc/codecs/cs47l90.c +++ b/sound/soc/codecs/cs47l90.c @@ -2425,7 +2425,8 @@ static int cs47l90_component_probe(struct snd_soc_component *component) if (ret) return ret;
- ret = madera_init_outputs(component, CS47L90_MONO_OUTPUTS); + ret = madera_init_outputs(component, NULL, CS47L90_MONO_OUTPUTS, + CS47L90_MONO_OUTPUTS); if (ret) return ret;
diff --git a/sound/soc/codecs/cs47l92.c b/sound/soc/codecs/cs47l92.c index cbffcd84766db..942040fd354f6 100644 --- a/sound/soc/codecs/cs47l92.c +++ b/sound/soc/codecs/cs47l92.c @@ -778,6 +778,7 @@ SND_SOC_DAPM_MUX("IN2L Mode", SND_SOC_NOPM, 0, 0, &madera_inmode[1]), SND_SOC_DAPM_MUX("IN2R Mode", SND_SOC_NOPM, 0, 0, &madera_inmode[1]),
SND_SOC_DAPM_DEMUX("OUT3 Demux", SND_SOC_NOPM, 0, 0, &cs47l92_outdemux), +SND_SOC_DAPM_MUX("OUT3 Mono Mux", SND_SOC_NOPM, 0, 0, &cs47l92_outdemux),
SND_SOC_DAPM_OUTPUT("DRC1 Signal Activity"), SND_SOC_DAPM_OUTPUT("DRC2 Signal Activity"), @@ -1632,6 +1633,8 @@ static const struct snd_soc_dapm_route cs47l92_dapm_routes[] = { { "OUT3 Demux", NULL, "OUT3L" }, { "OUT3 Demux", NULL, "OUT3R" },
+ { "OUT3R", NULL, "OUT3 Mono Mux" }, + { "HPOUT3L", "HPOUT3", "OUT3 Demux" }, { "HPOUT3R", "HPOUT3", "OUT3 Demux" }, { "HPOUT4L", "HPOUT4", "OUT3 Demux" }, @@ -1865,6 +1868,13 @@ static irqreturn_t cs47l92_adsp2_irq(int irq, void *data) return IRQ_HANDLED; }
+static const struct snd_soc_dapm_route cs47l92_mono_routes[] = { + { "OUT1R", NULL, "OUT1L" }, + { "OUT2R", NULL, "OUT2L" }, + { "OUT3 Mono Mux", "HPOUT3", "OUT3L" }, + { "OUT3 Mono Mux", "HPOUT4", "OUT3L" }, +}; + static int cs47l92_component_probe(struct snd_soc_component *component) { struct cs47l92 *cs47l92 = snd_soc_component_get_drvdata(component); @@ -1881,7 +1891,9 @@ static int cs47l92_component_probe(struct snd_soc_component *component) if (ret) return ret;
- ret = madera_init_outputs(component, CS47L92_MONO_OUTPUTS); + ret = madera_init_outputs(component, cs47l92_mono_routes, + ARRAY_SIZE(cs47l92_mono_routes), + CS47L92_MONO_OUTPUTS); if (ret) return ret;
diff --git a/sound/soc/codecs/madera.c b/sound/soc/codecs/madera.c index 2be64422adf9b..f65d123929659 100644 --- a/sound/soc/codecs/madera.c +++ b/sound/soc/codecs/madera.c @@ -1212,7 +1212,9 @@ static const struct snd_soc_dapm_route madera_mono_routes[] = { { "OUT6R", NULL, "OUT6L" }, };
-int madera_init_outputs(struct snd_soc_component *component, int n_mono_routes) +int madera_init_outputs(struct snd_soc_component *component, + const struct snd_soc_dapm_route *routes, + int n_mono_routes, int n_real) { struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component); @@ -1229,16 +1231,21 @@ int madera_init_outputs(struct snd_soc_component *component, int n_mono_routes) n_mono_routes = MADERA_MAX_OUTPUT; }
+ if (!routes) + routes = madera_mono_routes; + for (i = 0; i < n_mono_routes; i++) { /* Default is 0 so noop with defaults */ if (pdata->out_mono[i]) { val = MADERA_OUT1_MONO; - snd_soc_dapm_add_routes(dapm, - &madera_mono_routes[i], 1); + snd_soc_dapm_add_routes(dapm, &routes[i], 1); } else { val = 0; }
+ if (i >= n_real) + continue; + regmap_update_bits(madera->regmap, MADERA_OUTPUT_PATH_CONFIG_1L + (i * 8), MADERA_OUT1_MONO, val); diff --git a/sound/soc/codecs/madera.h b/sound/soc/codecs/madera.h index 49a8f68ec43da..e0c0be59e2ef7 100644 --- a/sound/soc/codecs/madera.h +++ b/sound/soc/codecs/madera.h @@ -423,7 +423,9 @@ int madera_core_free(struct madera_priv *priv); int madera_init_overheat(struct madera_priv *priv); int madera_free_overheat(struct madera_priv *priv); int madera_init_inputs(struct snd_soc_component *component); -int madera_init_outputs(struct snd_soc_component *component, int n_mono_routes); +int madera_init_outputs(struct snd_soc_component *component, + const struct snd_soc_dapm_route *routes, + int n_mono_routes, int n_real); int madera_init_bus_error_irq(struct madera_priv *priv, int dsp_num, irq_handler_t handler); void madera_free_bus_error_irq(struct madera_priv *priv, int dsp_num);
The patch
ASoC: madera: Update handling of DAPM routes for mono muxed outputs
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.5
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 8ab6ddc5c51b09630f3c89eb410ac48e13a2f0ff Mon Sep 17 00:00:00 2001
From: Charles Keepax ckeepax@opensource.cirrus.com Date: Tue, 14 Jan 2020 16:18:41 +0000 Subject: [PATCH] ASoC: madera: Update handling of DAPM routes for mono muxed outputs
Correctly link both channels on the DAC if an output muxed between a stereo and mono output. Without this one channel of the DAC may be erroneously powered down whilst in mono mode.
Signed-off-by: Charles Keepax ckeepax@opensource.cirrus.com Link: https://lore.kernel.org/r/20200114161841.451-4-ckeepax@opensource.cirrus.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/codecs/cs47l15.c | 13 ++++++++++++- sound/soc/codecs/cs47l35.c | 12 +++++++++++- sound/soc/codecs/cs47l85.c | 3 ++- sound/soc/codecs/cs47l90.c | 3 ++- sound/soc/codecs/cs47l92.c | 14 +++++++++++++- sound/soc/codecs/madera.c | 13 ++++++++++--- sound/soc/codecs/madera.h | 4 +++- 7 files changed, 53 insertions(+), 9 deletions(-)
diff --git a/sound/soc/codecs/cs47l15.c b/sound/soc/codecs/cs47l15.c index ece1276f38eb..6533c4f5a1de 100644 --- a/sound/soc/codecs/cs47l15.c +++ b/sound/soc/codecs/cs47l15.c @@ -529,6 +529,7 @@ SND_SOC_DAPM_OUTPUT("DRC2 Signal Activity"), SND_SOC_DAPM_OUTPUT("DSP Trigger Out"),
SND_SOC_DAPM_DEMUX("HPOUT1 Demux", SND_SOC_NOPM, 0, 0, &cs47l15_outdemux), +SND_SOC_DAPM_MUX("HPOUT1 Mono Mux", SND_SOC_NOPM, 0, 0, &cs47l15_outdemux),
SND_SOC_DAPM_PGA("PWM1 Driver", MADERA_PWM_DRIVE_1, MADERA_PWM1_ENA_SHIFT, 0, NULL, 0), @@ -1084,6 +1085,9 @@ static const struct snd_soc_dapm_route cs47l15_dapm_routes[] = { { "AEC2 Loopback", "HPOUT1R", "OUT1R" }, { "HPOUT1 Demux", NULL, "OUT1L" }, { "HPOUT1 Demux", NULL, "OUT1R" }, + + { "OUT1R", NULL, "HPOUT1 Mono Mux" }, + { "HPOUTL", "HPOUT", "HPOUT1 Demux" }, { "HPOUTR", "HPOUT", "HPOUT1 Demux" }, { "EPOUTP", "EPOUT", "HPOUT1 Demux" }, @@ -1261,6 +1265,11 @@ static irqreturn_t cs47l15_adsp2_irq(int irq, void *data) return IRQ_HANDLED; }
+static const struct snd_soc_dapm_route cs47l15_mono_routes[] = { + { "HPOUT1 Mono Mux", "HPOUT", "OUT1L" }, + { "HPOUT1 Mono Mux", "EPOUT", "OUT1L" }, +}; + static int cs47l15_component_probe(struct snd_soc_component *component) { struct cs47l15 *cs47l15 = snd_soc_component_get_drvdata(component); @@ -1277,7 +1286,9 @@ static int cs47l15_component_probe(struct snd_soc_component *component) if (ret) return ret;
- ret = madera_init_outputs(component, CS47L15_MONO_OUTPUTS); + ret = madera_init_outputs(component, cs47l15_mono_routes, + ARRAY_SIZE(cs47l15_mono_routes), + CS47L15_MONO_OUTPUTS); if (ret) return ret;
diff --git a/sound/soc/codecs/cs47l35.c b/sound/soc/codecs/cs47l35.c index d396a8545d51..bd172e170088 100644 --- a/sound/soc/codecs/cs47l35.c +++ b/sound/soc/codecs/cs47l35.c @@ -631,6 +631,7 @@ SND_SOC_DAPM_OUTPUT("DRC2 Signal Activity"), SND_SOC_DAPM_OUTPUT("DSP Trigger Out"),
SND_SOC_DAPM_DEMUX("HPOUT1 Demux", SND_SOC_NOPM, 0, 0, &cs47l35_outdemux), +SND_SOC_DAPM_MUX("HPOUT1 Mono Mux", SND_SOC_NOPM, 0, 0, &cs47l35_outdemux),
SND_SOC_DAPM_PGA("PWM1 Driver", MADERA_PWM_DRIVE_1, MADERA_PWM1_ENA_SHIFT, 0, NULL, 0), @@ -1309,6 +1310,8 @@ static const struct snd_soc_dapm_route cs47l35_dapm_routes[] = { { "SPKOUTN", NULL, "OUT4L" }, { "SPKOUTP", NULL, "OUT4L" },
+ { "OUT1R", NULL, "HPOUT1 Mono Mux" }, + { "HPOUTL", "HPOUT", "HPOUT1 Demux" }, { "HPOUTR", "HPOUT", "HPOUT1 Demux" }, { "EPOUTP", "EPOUT", "HPOUT1 Demux" }, @@ -1552,6 +1555,11 @@ static irqreturn_t cs47l35_adsp2_irq(int irq, void *data) return IRQ_HANDLED; }
+static const struct snd_soc_dapm_route cs47l35_mono_routes[] = { + { "HPOUT1 Mono Mux", "HPOUT", "OUT1L" }, + { "HPOUT1 Mono Mux", "EPOUT", "OUT1L" }, +}; + static int cs47l35_component_probe(struct snd_soc_component *component) { struct cs47l35 *cs47l35 = snd_soc_component_get_drvdata(component); @@ -1568,7 +1576,9 @@ static int cs47l35_component_probe(struct snd_soc_component *component) if (ret) return ret;
- ret = madera_init_outputs(component, CS47L35_MONO_OUTPUTS); + ret = madera_init_outputs(component, cs47l35_mono_routes, + ARRAY_SIZE(cs47l35_mono_routes), + CS47L35_MONO_OUTPUTS); if (ret) return ret;
diff --git a/sound/soc/codecs/cs47l85.c b/sound/soc/codecs/cs47l85.c index 32fe7ffb7526..a69311008134 100644 --- a/sound/soc/codecs/cs47l85.c +++ b/sound/soc/codecs/cs47l85.c @@ -2507,7 +2507,8 @@ static int cs47l85_component_probe(struct snd_soc_component *component) if (ret) return ret;
- ret = madera_init_outputs(component, CS47L85_MONO_OUTPUTS); + ret = madera_init_outputs(component, NULL, CS47L85_MONO_OUTPUTS, + CS47L85_MONO_OUTPUTS); if (ret) return ret;
diff --git a/sound/soc/codecs/cs47l90.c b/sound/soc/codecs/cs47l90.c index 67cac60a859d..5448e4506741 100644 --- a/sound/soc/codecs/cs47l90.c +++ b/sound/soc/codecs/cs47l90.c @@ -2418,7 +2418,8 @@ static int cs47l90_component_probe(struct snd_soc_component *component) if (ret) return ret;
- ret = madera_init_outputs(component, CS47L90_MONO_OUTPUTS); + ret = madera_init_outputs(component, NULL, CS47L90_MONO_OUTPUTS, + CS47L90_MONO_OUTPUTS); if (ret) return ret;
diff --git a/sound/soc/codecs/cs47l92.c b/sound/soc/codecs/cs47l92.c index d50f75f3b3e4..3c108b936419 100644 --- a/sound/soc/codecs/cs47l92.c +++ b/sound/soc/codecs/cs47l92.c @@ -730,6 +730,7 @@ SND_SOC_DAPM_MUX("IN2L Mode", SND_SOC_NOPM, 0, 0, &madera_inmode[1]), SND_SOC_DAPM_MUX("IN2R Mode", SND_SOC_NOPM, 0, 0, &madera_inmode[1]),
SND_SOC_DAPM_DEMUX("OUT3 Demux", SND_SOC_NOPM, 0, 0, &cs47l92_outdemux), +SND_SOC_DAPM_MUX("OUT3 Mono Mux", SND_SOC_NOPM, 0, 0, &cs47l92_outdemux),
SND_SOC_DAPM_OUTPUT("DRC1 Signal Activity"), SND_SOC_DAPM_OUTPUT("DRC2 Signal Activity"), @@ -1584,6 +1585,8 @@ static const struct snd_soc_dapm_route cs47l92_dapm_routes[] = { { "OUT3 Demux", NULL, "OUT3L" }, { "OUT3 Demux", NULL, "OUT3R" },
+ { "OUT3R", NULL, "OUT3 Mono Mux" }, + { "HPOUT3L", "HPOUT3", "OUT3 Demux" }, { "HPOUT3R", "HPOUT3", "OUT3 Demux" }, { "HPOUT4L", "HPOUT4", "OUT3 Demux" }, @@ -1817,6 +1820,13 @@ static irqreturn_t cs47l92_adsp2_irq(int irq, void *data) return IRQ_HANDLED; }
+static const struct snd_soc_dapm_route cs47l92_mono_routes[] = { + { "OUT1R", NULL, "OUT1L" }, + { "OUT2R", NULL, "OUT2L" }, + { "OUT3 Mono Mux", "HPOUT3", "OUT3L" }, + { "OUT3 Mono Mux", "HPOUT4", "OUT3L" }, +}; + static int cs47l92_component_probe(struct snd_soc_component *component) { struct cs47l92 *cs47l92 = snd_soc_component_get_drvdata(component); @@ -1833,7 +1843,9 @@ static int cs47l92_component_probe(struct snd_soc_component *component) if (ret) return ret;
- ret = madera_init_outputs(component, CS47L92_MONO_OUTPUTS); + ret = madera_init_outputs(component, cs47l92_mono_routes, + ARRAY_SIZE(cs47l92_mono_routes), + CS47L92_MONO_OUTPUTS); if (ret) return ret;
diff --git a/sound/soc/codecs/madera.c b/sound/soc/codecs/madera.c index 52639811cc52..b537e60ef7c0 100644 --- a/sound/soc/codecs/madera.c +++ b/sound/soc/codecs/madera.c @@ -1162,7 +1162,9 @@ static const struct snd_soc_dapm_route madera_mono_routes[] = { { "OUT6R", NULL, "OUT6L" }, };
-int madera_init_outputs(struct snd_soc_component *component, int n_mono_routes) +int madera_init_outputs(struct snd_soc_component *component, + const struct snd_soc_dapm_route *routes, + int n_mono_routes, int n_real) { struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component); @@ -1179,16 +1181,21 @@ int madera_init_outputs(struct snd_soc_component *component, int n_mono_routes) n_mono_routes = MADERA_MAX_OUTPUT; }
+ if (!routes) + routes = madera_mono_routes; + for (i = 0; i < n_mono_routes; i++) { /* Default is 0 so noop with defaults */ if (pdata->out_mono[i]) { val = MADERA_OUT1_MONO; - snd_soc_dapm_add_routes(dapm, - &madera_mono_routes[i], 1); + snd_soc_dapm_add_routes(dapm, &routes[i], 1); } else { val = 0; }
+ if (i >= n_real) + continue; + regmap_update_bits(madera->regmap, MADERA_OUTPUT_PATH_CONFIG_1L + (i * 8), MADERA_OUT1_MONO, val); diff --git a/sound/soc/codecs/madera.h b/sound/soc/codecs/madera.h index 6d8938a3fb64..bff084695961 100644 --- a/sound/soc/codecs/madera.h +++ b/sound/soc/codecs/madera.h @@ -421,7 +421,9 @@ int madera_core_free(struct madera_priv *priv); int madera_init_overheat(struct madera_priv *priv); int madera_free_overheat(struct madera_priv *priv); int madera_init_inputs(struct snd_soc_component *component); -int madera_init_outputs(struct snd_soc_component *component, int n_mono_routes); +int madera_init_outputs(struct snd_soc_component *component, + const struct snd_soc_dapm_route *routes, + int n_mono_routes, int n_real); int madera_init_bus_error_irq(struct madera_priv *priv, int dsp_num, irq_handler_t handler); void madera_free_bus_error_irq(struct madera_priv *priv, int dsp_num);
The patch
ASoC: wm_adsp: Correct cache handling of new kernel control API
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.5
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 73ecf1a673d3502dff1445f06675aba65ff20ce7 Mon Sep 17 00:00:00 2001
From: Charles Keepax ckeepax@opensource.cirrus.com Date: Tue, 14 Jan 2020 16:18:38 +0000 Subject: [PATCH] ASoC: wm_adsp: Correct cache handling of new kernel control API
The recently added API that exposes firmware mixer controls to the kernel is missing cache handling and all writes bypass the cache, this obviously causes the cache to get out of sync with the hardware. Factor out the cache handling into two new helper functions and call those from both the normal ALSA control handlers and the new kernel API.
Fixes: eb65ccdb0836 ("ASoC: wm_adsp: Expose mixer control API") Signed-off-by: Charles Keepax ckeepax@opensource.cirrus.com Link: https://lore.kernel.org/r/20200114161841.451-1-ckeepax@opensource.cirrus.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/codecs/wm_adsp.c | 98 ++++++++++++++++++++------------------ 1 file changed, 51 insertions(+), 47 deletions(-)
diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c index 2a9b610f6d43..d3d32b501aca 100644 --- a/sound/soc/codecs/wm_adsp.c +++ b/sound/soc/codecs/wm_adsp.c @@ -1030,8 +1030,8 @@ static int wm_coeff_write_acked_control(struct wm_coeff_ctl *ctl, return -ETIMEDOUT; }
-static int wm_coeff_write_control(struct wm_coeff_ctl *ctl, - const void *buf, size_t len) +static int wm_coeff_write_ctrl_raw(struct wm_coeff_ctl *ctl, + const void *buf, size_t len) { struct wm_adsp *dsp = ctl->dsp; void *scratch; @@ -1061,6 +1061,23 @@ static int wm_coeff_write_control(struct wm_coeff_ctl *ctl, return 0; }
+static int wm_coeff_write_ctrl(struct wm_coeff_ctl *ctl, + const void *buf, size_t len) +{ + int ret = 0; + + if (ctl->flags & WMFW_CTL_FLAG_VOLATILE) + ret = -EPERM; + else if (buf != ctl->cache) + memcpy(ctl->cache, buf, len); + + ctl->set = 1; + if (ctl->enabled && ctl->dsp->running) + ret = wm_coeff_write_ctrl_raw(ctl, buf, len); + + return ret; +} + static int wm_coeff_put(struct snd_kcontrol *kctl, struct snd_ctl_elem_value *ucontrol) { @@ -1071,16 +1088,7 @@ static int wm_coeff_put(struct snd_kcontrol *kctl, int ret = 0;
mutex_lock(&ctl->dsp->pwr_lock); - - if (ctl->flags & WMFW_CTL_FLAG_VOLATILE) - ret = -EPERM; - else - memcpy(ctl->cache, p, ctl->len); - - ctl->set = 1; - if (ctl->enabled && ctl->dsp->running) - ret = wm_coeff_write_control(ctl, p, ctl->len); - + ret = wm_coeff_write_ctrl(ctl, p, ctl->len); mutex_unlock(&ctl->dsp->pwr_lock);
return ret; @@ -1096,15 +1104,10 @@ static int wm_coeff_tlv_put(struct snd_kcontrol *kctl,
mutex_lock(&ctl->dsp->pwr_lock);
- if (copy_from_user(ctl->cache, bytes, size)) { + if (copy_from_user(ctl->cache, bytes, size)) ret = -EFAULT; - } else { - ctl->set = 1; - if (ctl->enabled && ctl->dsp->running) - ret = wm_coeff_write_control(ctl, ctl->cache, size); - else if (ctl->flags & WMFW_CTL_FLAG_VOLATILE) - ret = -EPERM; - } + else + ret = wm_coeff_write_ctrl(ctl, ctl->cache, size);
mutex_unlock(&ctl->dsp->pwr_lock);
@@ -1135,8 +1138,8 @@ static int wm_coeff_put_acked(struct snd_kcontrol *kctl, return ret; }
-static int wm_coeff_read_control(struct wm_coeff_ctl *ctl, - void *buf, size_t len) +static int wm_coeff_read_ctrl_raw(struct wm_coeff_ctl *ctl, + void *buf, size_t len) { struct wm_adsp *dsp = ctl->dsp; void *scratch; @@ -1166,29 +1169,37 @@ static int wm_coeff_read_control(struct wm_coeff_ctl *ctl, return 0; }
-static int wm_coeff_get(struct snd_kcontrol *kctl, - struct snd_ctl_elem_value *ucontrol) +static int wm_coeff_read_ctrl(struct wm_coeff_ctl *ctl, void *buf, size_t len) { - struct soc_bytes_ext *bytes_ext = - (struct soc_bytes_ext *)kctl->private_value; - struct wm_coeff_ctl *ctl = bytes_ext_to_ctl(bytes_ext); - char *p = ucontrol->value.bytes.data; int ret = 0;
- mutex_lock(&ctl->dsp->pwr_lock); - if (ctl->flags & WMFW_CTL_FLAG_VOLATILE) { if (ctl->enabled && ctl->dsp->running) - ret = wm_coeff_read_control(ctl, p, ctl->len); + return wm_coeff_read_ctrl_raw(ctl, buf, len); else - ret = -EPERM; + return -EPERM; } else { if (!ctl->flags && ctl->enabled && ctl->dsp->running) - ret = wm_coeff_read_control(ctl, ctl->cache, ctl->len); + ret = wm_coeff_read_ctrl_raw(ctl, ctl->cache, ctl->len);
- memcpy(p, ctl->cache, ctl->len); + if (buf != ctl->cache) + memcpy(buf, ctl->cache, len); }
+ return ret; +} + +static int wm_coeff_get(struct snd_kcontrol *kctl, + struct snd_ctl_elem_value *ucontrol) +{ + struct soc_bytes_ext *bytes_ext = + (struct soc_bytes_ext *)kctl->private_value; + struct wm_coeff_ctl *ctl = bytes_ext_to_ctl(bytes_ext); + char *p = ucontrol->value.bytes.data; + int ret; + + mutex_lock(&ctl->dsp->pwr_lock); + ret = wm_coeff_read_ctrl(ctl, p, ctl->len); mutex_unlock(&ctl->dsp->pwr_lock);
return ret; @@ -1204,15 +1215,7 @@ static int wm_coeff_tlv_get(struct snd_kcontrol *kctl,
mutex_lock(&ctl->dsp->pwr_lock);
- if (ctl->flags & WMFW_CTL_FLAG_VOLATILE) { - if (ctl->enabled && ctl->dsp->running) - ret = wm_coeff_read_control(ctl, ctl->cache, size); - else - ret = -EPERM; - } else { - if (!ctl->flags && ctl->enabled && ctl->dsp->running) - ret = wm_coeff_read_control(ctl, ctl->cache, size); - } + ret = wm_coeff_read_ctrl_raw(ctl, ctl->cache, size);
if (!ret && copy_to_user(bytes, ctl->cache, size)) ret = -EFAULT; @@ -1340,7 +1343,7 @@ static int wm_coeff_init_control_caches(struct wm_adsp *dsp) * created so we don't need to do anything. */ if (!ctl->flags || (ctl->flags & WMFW_CTL_FLAG_READABLE)) { - ret = wm_coeff_read_control(ctl, ctl->cache, ctl->len); + ret = wm_coeff_read_ctrl_raw(ctl, ctl->cache, ctl->len); if (ret < 0) return ret; } @@ -1358,7 +1361,8 @@ static int wm_coeff_sync_controls(struct wm_adsp *dsp) if (!ctl->enabled) continue; if (ctl->set && !(ctl->flags & WMFW_CTL_FLAG_VOLATILE)) { - ret = wm_coeff_write_control(ctl, ctl->cache, ctl->len); + ret = wm_coeff_write_ctrl_raw(ctl, ctl->cache, + ctl->len); if (ret < 0) return ret; } @@ -2048,7 +2052,7 @@ int wm_adsp_write_ctl(struct wm_adsp *dsp, const char *name, int type, if (len > ctl->len) return -EINVAL;
- ret = wm_coeff_write_control(ctl, buf, len); + ret = wm_coeff_write_ctrl(ctl, buf, len);
kcontrol = snd_soc_card_get_kcontrol(dsp->component->card, ctl->name); snd_ctl_notify(dsp->component->card->snd_card, @@ -2070,7 +2074,7 @@ int wm_adsp_read_ctl(struct wm_adsp *dsp, const char *name, int type, if (len > ctl->len) return -EINVAL;
- return wm_coeff_read_control(ctl, buf, len); + return wm_coeff_read_ctrl(ctl, buf, len); } EXPORT_SYMBOL_GPL(wm_adsp_read_ctl);
participants (2)
-
Charles Keepax -
Mark Brown