From d917b5dde660b11abd757bf99a29353c36880b2c Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Mon, 15 Feb 2021 15:21:15 +0100 Subject: ASoC: rt5670: Remove 'OUT Channel Switch' control The "OUT Channel Switch" control is a left over from code copied from thr rt5640 codec driver. With the rt5640 codec driver the output volume controls have 2 pairs of mute bits: bit 7, 15: Mute Control for Spk/Headphone/Line Output Port bit 6, 14: Mute Control for Spk/Headphone/Line Volume Channel Bits 7 and 15 are normal mute bits on the rt5670/5672 which are controlled by 2 dapm widgets: SND_SOC_DAPM_SWITCH("LOUT L Playback", SND_SOC_NOPM, 0, 0, &lout_l_enable_control), SND_SOC_DAPM_SWITCH("LOUT R Playback", SND_SOC_NOPM, 0, 0, &lout_r_enable_control), But on the 5670/5672 bit 6 is always reserved, where as bit 14 is "LOUT Differential Mode" on the 5670 and also reserved on the 5672. So the "OUT Channel Switch" control which is controlling bits 6+14 of the "LINE Output Control" register is bogus -> remove it. This should not cause any issues for userspace. AFAICT the rt567x codecs are only used on x86/ACPI devices and the UCM profiles used there do not use the "OUT Channel Switch" control. Signed-off-by: Hans de Goede Link: https://lore.kernel.org/r/20210215142118.308516-2-hdegoede@redhat.com Signed-off-by: Mark Brown --- sound/soc/codecs/rt5670.c | 2 -- sound/soc/codecs/rt5670.h | 4 ---- 2 files changed, 6 deletions(-) (limited to 'sound/soc') diff --git a/sound/soc/codecs/rt5670.c b/sound/soc/codecs/rt5670.c index c29317ea5df2..2e799e21dbda 100644 --- a/sound/soc/codecs/rt5670.c +++ b/sound/soc/codecs/rt5670.c @@ -637,8 +637,6 @@ static const struct snd_kcontrol_new rt5670_snd_controls[] = { RT5670_L_VOL_SFT, RT5670_R_VOL_SFT, 39, 1, out_vol_tlv), /* OUTPUT Control */ - SOC_DOUBLE("OUT Channel Switch", RT5670_LOUT1, - RT5670_VOL_L_SFT, RT5670_VOL_R_SFT, 1, 1), SOC_DOUBLE_TLV("OUT Playback Volume", RT5670_LOUT1, RT5670_L_VOL_SFT, RT5670_R_VOL_SFT, 39, 1, out_vol_tlv), /* DAC Digital Volume */ diff --git a/sound/soc/codecs/rt5670.h b/sound/soc/codecs/rt5670.h index 56b13fe6bd3c..f9c4db156c80 100644 --- a/sound/soc/codecs/rt5670.h +++ b/sound/soc/codecs/rt5670.h @@ -212,12 +212,8 @@ /* global definition */ #define RT5670_L_MUTE (0x1 << 15) #define RT5670_L_MUTE_SFT 15 -#define RT5670_VOL_L_MUTE (0x1 << 14) -#define RT5670_VOL_L_SFT 14 #define RT5670_R_MUTE (0x1 << 7) #define RT5670_R_MUTE_SFT 7 -#define RT5670_VOL_R_MUTE (0x1 << 6) -#define RT5670_VOL_R_SFT 6 #define RT5670_L_VOL_MASK (0x3f << 8) #define RT5670_L_VOL_SFT 8 #define RT5670_R_VOL_MASK (0x3f) -- cgit v1.2.3 From caba8d764770b6824391c5bf3c3eba6e51b69330 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Mon, 15 Feb 2021 15:21:16 +0100 Subject: ASoC: rt5670: Remove 'HP Playback Switch' control The RT5670_L_MUTE_SFT and RT5670_R_MUTE_SFT bits (bits 15 and 7) of the RT5670_HP_VOL register are set / unset by the headphones deplop code run by rt5670_hp_event() on SND_SOC_DAPM_POST_PMU / SND_SOC_DAPM_PRE_PMD. So we should not also export a control to userspace which toggles these same bits. This should not cause any issues for userspace. AFAICT the rt567x codecs are only used on x86/ACPI devices and the UCM profiles used there do not use the "HP Playback Switch" control. Signed-off-by: Hans de Goede Link: https://lore.kernel.org/r/20210215142118.308516-3-hdegoede@redhat.com Signed-off-by: Mark Brown --- sound/soc/codecs/rt5670.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'sound/soc') diff --git a/sound/soc/codecs/rt5670.c b/sound/soc/codecs/rt5670.c index 2e799e21dbda..932e4cd1e9a6 100644 --- a/sound/soc/codecs/rt5670.c +++ b/sound/soc/codecs/rt5670.c @@ -631,8 +631,6 @@ static SOC_ENUM_SINGLE_DECL(rt5670_if2_adc_enum, RT5670_DIG_INF1_DATA, static const struct snd_kcontrol_new rt5670_snd_controls[] = { /* Headphone Output Volume */ - SOC_DOUBLE("HP Playback Switch", RT5670_HP_VOL, - RT5670_L_MUTE_SFT, RT5670_R_MUTE_SFT, 1, 1), SOC_DOUBLE_TLV("HP Playback Volume", RT5670_HP_VOL, RT5670_L_VOL_SFT, RT5670_R_VOL_SFT, 39, 1, out_vol_tlv), -- cgit v1.2.3 From 02aa946ef3762aa456d87cc55606667942b3f354 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Mon, 15 Feb 2021 15:21:17 +0100 Subject: ASoC: rt5670: Remove ADC vol-ctrl mute bits poking from Sto1 ADC mixer settings The SND_SOC_DAPM_MIXER declaration for "Sto1 ADC MIXL" and "Sto1 ADC MIXR" was using the mute bits from the RT5670_STO1_ADC_DIG_VOL control as mixer master mute bits. But these bits are already exposed to userspace as controls as part of the "ADC Capture Volume" / "ADC Capture Switch" control pair: SOC_DOUBLE("ADC Capture Switch", RT5670_STO1_ADC_DIG_VOL, RT5670_L_MUTE_SFT, RT5670_R_MUTE_SFT, 1, 1), SOC_DOUBLE_TLV("ADC Capture Volume", RT5670_STO1_ADC_DIG_VOL, RT5670_L_VOL_SFT, RT5670_R_VOL_SFT, 127, 0, adc_vol_tlv), Both the fact that the mute bits belong to the same reg as the vol-ctrl and the "Digital Mixer Path" diagram in the datasheet clearly shows that these mute bits are not part of the mixer and having 2 separate controls poking at the same bits is a bad idea. Remove the master-mute bits settings from the "Sto1 ADC MIXL" and "Sto1 ADC MIXR" DAPM widget declarations, avoiding these bits getting poked from 2 different places. This should not cause any issues for userspace. AFAICT the rt567x codecs are only used on x86/ACPI devices and the UCM profiles used there already set the "ADC Capture Switch" as needed. Signed-off-by: Hans de Goede Link: https://lore.kernel.org/r/20210215142118.308516-4-hdegoede@redhat.com Signed-off-by: Mark Brown --- sound/soc/codecs/rt5670.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'sound/soc') diff --git a/sound/soc/codecs/rt5670.c b/sound/soc/codecs/rt5670.c index 932e4cd1e9a6..2f015c24c637 100644 --- a/sound/soc/codecs/rt5670.c +++ b/sound/soc/codecs/rt5670.c @@ -1652,12 +1652,10 @@ static const struct snd_soc_dapm_widget rt5670_dapm_widgets[] = { RT5670_PWR_ADC_S1F_BIT, 0, NULL, 0), SND_SOC_DAPM_SUPPLY("ADC Stereo2 Filter", RT5670_PWR_DIG2, RT5670_PWR_ADC_S2F_BIT, 0, NULL, 0), - SND_SOC_DAPM_MIXER("Sto1 ADC MIXL", RT5670_STO1_ADC_DIG_VOL, - RT5670_L_MUTE_SFT, 1, rt5670_sto1_adc_l_mix, - ARRAY_SIZE(rt5670_sto1_adc_l_mix)), - SND_SOC_DAPM_MIXER("Sto1 ADC MIXR", RT5670_STO1_ADC_DIG_VOL, - RT5670_R_MUTE_SFT, 1, rt5670_sto1_adc_r_mix, - ARRAY_SIZE(rt5670_sto1_adc_r_mix)), + SND_SOC_DAPM_MIXER("Sto1 ADC MIXL", SND_SOC_NOPM, 0, 0, + rt5670_sto1_adc_l_mix, ARRAY_SIZE(rt5670_sto1_adc_l_mix)), + SND_SOC_DAPM_MIXER("Sto1 ADC MIXR", SND_SOC_NOPM, 0, 0, + rt5670_sto1_adc_r_mix, ARRAY_SIZE(rt5670_sto1_adc_r_mix)), SND_SOC_DAPM_MIXER("Sto2 ADC MIXL", SND_SOC_NOPM, 0, 0, rt5670_sto2_adc_l_mix, ARRAY_SIZE(rt5670_sto2_adc_l_mix)), -- cgit v1.2.3 From 42121c2645d229d348399ad278b6c3fd224bd6a2 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Mon, 15 Feb 2021 15:21:18 +0100 Subject: ASoC: rt5670: Add emulated 'DAC1 Playback Switch' control For reliable output-mute LED control we need a "DAC1 Playback Switch" control. The "DAC Playback volume" control is the only control in the path from the DAC1 data input to the speaker output, so the UCM profile for the speaker output will have its PlaybackMixerElem set to "DAC1". But userspace (pulseaudio) will set the "DAC1 Playback Volume" control to its softest setting (which is not fully muted) while still showing the speaker as being enabled at a low volume in the UI. If we were to set the SNDRV_CTL_ELEM_ACCESS_SPK_LED on the "DAC1 Playback Volume" control, this would mean then what pressing KEY_VOLUMEDOWN the speaker-mute LED (embedded in the volume-mute toggle key) would light while the UI is still showing the speaker as being enabled at a low volume, meaning that the UI and the LED are out of sync. Only after an _extra_ KEY_VOLUMEDOWN press would the UI show the speaker as being muted. The path from DAC1 data input to the speaker output does have a digital mixer with DAC1's data as one of its inputs direclty after the "DAC1 Playback Volume" control. This commit adds an emulated "DAC1 Playback Switch" control by: 1. Declaring the enable flag for that mixers DAC1 input as well as the "DAC1 Playback Switch" control both as SND_SOC_NOPM controls. 2. Storing the settings of both controls as driver-private data 3. Only clearing the mute flag for the DAC1 input of that mixer if the stored values indicate both controls are enabled. This is a preparation patch for adding "audio-mute" LED trigger support. Signed-off-by: Hans de Goede Link: https://lore.kernel.org/r/20210215142118.308516-5-hdegoede@redhat.com Signed-off-by: Mark Brown --- sound/soc/codecs/rt5670.c | 96 +++++++++++++++++++++++++++++++++++++++++++++-- sound/soc/codecs/rt5670.h | 5 +++ 2 files changed, 97 insertions(+), 4 deletions(-) (limited to 'sound/soc') diff --git a/sound/soc/codecs/rt5670.c b/sound/soc/codecs/rt5670.c index 2f015c24c637..4063aac2a443 100644 --- a/sound/soc/codecs/rt5670.c +++ b/sound/soc/codecs/rt5670.c @@ -629,6 +629,56 @@ static SOC_ENUM_SINGLE_DECL(rt5670_if2_dac_enum, RT5670_DIG_INF1_DATA, static SOC_ENUM_SINGLE_DECL(rt5670_if2_adc_enum, RT5670_DIG_INF1_DATA, RT5670_IF2_ADC_SEL_SFT, rt5670_data_select); +/* + * For reliable output-mute LED control we need a "DAC1 Playback Switch" control. + * We emulate this by only clearing the RT5670_M_DAC1_L/_R AD_DA_MIXER register + * bits when both our emulated DAC1 Playback Switch control and the DAC1 MIXL/R + * DAPM-mixer DAC1 input are enabled. + */ +static void rt5670_update_ad_da_mixer_dac1_m_bits(struct rt5670_priv *rt5670) +{ + int val = RT5670_M_DAC1_L | RT5670_M_DAC1_R; + + if (rt5670->dac1_mixl_dac1_switch && rt5670->dac1_playback_switch_l) + val &= ~RT5670_M_DAC1_L; + + if (rt5670->dac1_mixr_dac1_switch && rt5670->dac1_playback_switch_r) + val &= ~RT5670_M_DAC1_R; + + regmap_update_bits(rt5670->regmap, RT5670_AD_DA_MIXER, + RT5670_M_DAC1_L | RT5670_M_DAC1_R, val); +} + +static int rt5670_dac1_playback_switch_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_soc_component *component = snd_soc_kcontrol_component(kcontrol); + struct rt5670_priv *rt5670 = snd_soc_component_get_drvdata(component); + + ucontrol->value.integer.value[0] = rt5670->dac1_playback_switch_l; + ucontrol->value.integer.value[1] = rt5670->dac1_playback_switch_r; + + return 0; +} + +static int rt5670_dac1_playback_switch_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_soc_component *component = snd_soc_kcontrol_component(kcontrol); + struct rt5670_priv *rt5670 = snd_soc_component_get_drvdata(component); + + if (rt5670->dac1_playback_switch_l == ucontrol->value.integer.value[0] && + rt5670->dac1_playback_switch_r == ucontrol->value.integer.value[1]) + return 0; + + rt5670->dac1_playback_switch_l = ucontrol->value.integer.value[0]; + rt5670->dac1_playback_switch_r = ucontrol->value.integer.value[1]; + + rt5670_update_ad_da_mixer_dac1_m_bits(rt5670); + + return 1; +} + static const struct snd_kcontrol_new rt5670_snd_controls[] = { /* Headphone Output Volume */ SOC_DOUBLE_TLV("HP Playback Volume", RT5670_HP_VOL, @@ -640,6 +690,8 @@ static const struct snd_kcontrol_new rt5670_snd_controls[] = { /* DAC Digital Volume */ SOC_DOUBLE("DAC2 Playback Switch", RT5670_DAC_CTRL, RT5670_M_DAC_L2_VOL_SFT, RT5670_M_DAC_R2_VOL_SFT, 1, 1), + SOC_DOUBLE_EXT("DAC1 Playback Switch", SND_SOC_NOPM, 0, 1, 1, 0, + rt5670_dac1_playback_switch_get, rt5670_dac1_playback_switch_put), SOC_DOUBLE_TLV("DAC1 Playback Volume", RT5670_DAC1_DIG_VOL, RT5670_L_VOL_SFT, RT5670_R_VOL_SFT, 175, 0, dac_vol_tlv), @@ -909,18 +961,44 @@ static const struct snd_kcontrol_new rt5670_mono_adc_r_mix[] = { RT5670_M_MONO_ADC_R2_SFT, 1, 1), }; +/* See comment above rt5670_update_ad_da_mixer_dac1_m_bits() */ +static int rt5670_put_dac1_mix_dac1_switch(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct soc_mixer_control *mc = (struct soc_mixer_control *)kcontrol->private_value; + struct snd_soc_component *component = snd_soc_dapm_kcontrol_component(kcontrol); + struct rt5670_priv *rt5670 = snd_soc_component_get_drvdata(component); + int ret; + + if (mc->shift == 0) + rt5670->dac1_mixl_dac1_switch = ucontrol->value.integer.value[0]; + else + rt5670->dac1_mixr_dac1_switch = ucontrol->value.integer.value[0]; + + /* Apply the update (if any) */ + ret = snd_soc_dapm_put_volsw(kcontrol, ucontrol); + if (ret == 0) + return 0; + + rt5670_update_ad_da_mixer_dac1_m_bits(rt5670); + + return 1; +} + +#define SOC_DAPM_SINGLE_RT5670_DAC1_SW(name, shift) \ + SOC_SINGLE_EXT(name, SND_SOC_NOPM, shift, 1, 0, \ + snd_soc_dapm_get_volsw, rt5670_put_dac1_mix_dac1_switch) + static const struct snd_kcontrol_new rt5670_dac_l_mix[] = { SOC_DAPM_SINGLE("Stereo ADC Switch", RT5670_AD_DA_MIXER, RT5670_M_ADCMIX_L_SFT, 1, 1), - SOC_DAPM_SINGLE("DAC1 Switch", RT5670_AD_DA_MIXER, - RT5670_M_DAC1_L_SFT, 1, 1), + SOC_DAPM_SINGLE_RT5670_DAC1_SW("DAC1 Switch", 0), }; static const struct snd_kcontrol_new rt5670_dac_r_mix[] = { SOC_DAPM_SINGLE("Stereo ADC Switch", RT5670_AD_DA_MIXER, RT5670_M_ADCMIX_R_SFT, 1, 1), - SOC_DAPM_SINGLE("DAC1 Switch", RT5670_AD_DA_MIXER, - RT5670_M_DAC1_R_SFT, 1, 1), + SOC_DAPM_SINGLE_RT5670_DAC1_SW("DAC1 Switch", 1), }; static const struct snd_kcontrol_new rt5670_sto_dac_l_mix[] = { @@ -2993,6 +3071,16 @@ static int rt5670_i2c_probe(struct i2c_client *i2c, dev_info(&i2c->dev, "quirk JD mode 3\n"); } + /* + * Enable the emulated "DAC1 Playback Switch" by default to avoid + * muting the output with older UCM profiles. + */ + rt5670->dac1_playback_switch_l = true; + rt5670->dac1_playback_switch_r = true; + /* The Power-On-Reset values for the DAC1 mixer have the DAC1 input enabled. */ + rt5670->dac1_mixl_dac1_switch = true; + rt5670->dac1_mixr_dac1_switch = true; + rt5670->regmap = devm_regmap_init_i2c(i2c, &rt5670_regmap); if (IS_ERR(rt5670->regmap)) { ret = PTR_ERR(rt5670->regmap); diff --git a/sound/soc/codecs/rt5670.h b/sound/soc/codecs/rt5670.h index f9c4db156c80..6fb3c369ee98 100644 --- a/sound/soc/codecs/rt5670.h +++ b/sound/soc/codecs/rt5670.h @@ -2013,6 +2013,11 @@ struct rt5670_priv { int dsp_rate; int jack_type; int jack_type_saved; + + bool dac1_mixl_dac1_switch; + bool dac1_mixr_dac1_switch; + bool dac1_playback_switch_l; + bool dac1_playback_switch_r; }; void rt5670_jack_suspend(struct snd_soc_component *component); -- cgit v1.2.3