| Message ID | 20210215142118.308516-3-hdegoede@redhat.com |
|---|---|
| State | Accepted |
| Commit | caba8d764770b6824391c5bf3c3eba6e51b69330 |
| Headers | show |
| Series | ASoC: rt5670: Various kcontrol fixes | expand |
Hi, On 2/15/21 7:09 PM, Jaroslav Kysela wrote: > Dne 15. 02. 21 v 15:21 Hans de Goede napsal(a): >> 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. > > I think that it may be worth to preserve the full-mute functionality. > According the datasheet, the register 0x02 has volume range -46.5dB..12dB. > Even the lowest volume may produce an audible output. > > I would cache the mute switch value in rt5670_priv and update the > POST_PMU/PRE_PMD code to use this settings. Yes that should work. Note though that patch 4/4 adds a mute for the master volume control, even though I call it an "emulated" mute it is a full mute, it is just that we now have 2 switches, 1 mixer switch and 1 mute switch, which must both be true before we enable the path from the DAC through the mixer which sits directly behind the "DAC1 Playback Volume" control. And we need that mute anyways because the speaker output does not have any volume control other then the master "DAC1 Playback Volume" which has the same issue of only going very soft and not going to a full mute. So since we have a working mute in the master volume control, we don't really need one for the "HP Playback Volume" control. with that all said, your suggestion should work fine. So the question is do we want to do as you suggest, or are we happy with just having the master mute ? Note I'm fine doing things either way I just wanted to ask before spending time on implementing and testing your suggestion. Esp. the testing which often seems to take at least as much time as actually implementing things... So if you can let me know how you want to proceed then I'll get right to it. Regards, Hans
diff --git a/sound/soc/codecs/rt5670.c b/sound/soc/codecs/rt5670.c index b13d9e0e0e61..0f372a748b0e 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),
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 <hdegoede@redhat.com> --- sound/soc/codecs/rt5670.c | 2 -- 1 file changed, 2 deletions(-)