| Message ID | 5c314f5512654aca9fff0195f77264de@realtek.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/3] ASoC: rt715: add main capture switch and main capture volume | expand |
On Mon, Mar 29, 2021 at 06:54:00AM +0000, Jack Yu wrote: > Using new kcontrols "Capture Switch" and "Capture Volume" instead, > remove kcontrols which no longer be used. Is this going to disrupt any UCM profiles?
>>> Using new kcontrols "Capture Switch" and "Capture Volume" instead, >>> remove kcontrols which no longer be used. >> >> Is this going to disrupt any UCM profiles? > > Yes (the rt715 prefix is from the SOF driver): > > # RT715 specific volume control settings > > BootSequence [ > cset "name='rt715 DMIC3 Boost' 2" > cset "name='rt715 DMIC4 Boost' 2" > cset "name='rt715 ADC 24 Mux' 3" > cset "name='rt715 ADC 25 Mux' 4" > cset "name='rt715 ADC 27 Capture Switch' 1" > cset "name='rt715 ADC 07 Capture Switch' 1" > cset "name='rt715 ADC 07 Capture Volume' 58" > ] To be clearer, we wanted to change the UCM files to only try to configure the 'old' controls when they are present. The 'new' controls are aligned between RT715 and RT715-sdca. There will be a minor inconvenience if an existing platform updates the kernel without updating UCM files, but it's the only solution we found in earlier discussions. Distributions are typically faster with alsa-ucmconf updates than kernel changes so that inconvenience is likely very limited (we support 4-5 Dell CML/TGL platforms w/ SoundWire)
On 3/30/21 2:26 PM, Pierre-Louis Bossart wrote: > > >>>> Using new kcontrols "Capture Switch" and "Capture Volume" instead, >>>> remove kcontrols which no longer be used. >>> >>> Is this going to disrupt any UCM profiles? >> >> Yes (the rt715 prefix is from the SOF driver): >> >> # RT715 specific volume control settings >> >> BootSequence [ >> cset "name='rt715 DMIC3 Boost' 2" >> cset "name='rt715 DMIC4 Boost' 2" >> cset "name='rt715 ADC 24 Mux' 3" >> cset "name='rt715 ADC 25 Mux' 4" >> cset "name='rt715 ADC 27 Capture Switch' 1" >> cset "name='rt715 ADC 07 Capture Switch' 1" >> cset "name='rt715 ADC 07 Capture Volume' 58" >> ] > > To be clearer, we wanted to change the UCM files to only try to > configure the 'old' controls when they are present. The 'new' controls > are aligned between RT715 and RT715-sdca. > > There will be a minor inconvenience if an existing platform updates the > kernel without updating UCM files, but it's the only solution we found > in earlier discussions. > > Distributions are typically faster with alsa-ucmconf updates than kernel > changes so that inconvenience is likely very limited (we support 4-5 > Dell CML/TGL platforms w/ SoundWire) Was there any sustained objection to this change? Mark and Jaroslav? I would really like to fix the UCM files so that the mute and volume are handled at the codec driver level, and not at the SOF level as it's currently the case. The existing solution really makes no sense, it was 'inspired' (in the copy-paste sense) by the PCH-DMIC handling for HDaudio platforms. In that case, it made sense to use SOF-level mute/volume because such controls don't exist in microphones. With RT715 we need to use the controls in the codec driver, not the firmware.
On Thu, Apr 15, 2021 at 04:35:14PM -0500, Pierre-Louis Bossart wrote: > Was there any sustained objection to this change? Mark and Jaroslav? > I would really like to fix the UCM files so that the mute and volume are > handled at the codec driver level, and not at the SOF level as it's > currently the case. Well, you seemed to be objecting because it broke the UCM files and Jack didn't reply so as far as I understand it it'll break userspace.
Dne 15. 04. 21 v 23:35 Pierre-Louis Bossart napsal(a): > > > On 3/30/21 2:26 PM, Pierre-Louis Bossart wrote: >> >> >>>>> Using new kcontrols "Capture Switch" and "Capture Volume" instead, >>>>> remove kcontrols which no longer be used. >>>> >>>> Is this going to disrupt any UCM profiles? >>> >>> Yes (the rt715 prefix is from the SOF driver): >>> >>> # RT715 specific volume control settings >>> >>> BootSequence [ >>> cset "name='rt715 DMIC3 Boost' 2" >>> cset "name='rt715 DMIC4 Boost' 2" >>> cset "name='rt715 ADC 24 Mux' 3" >>> cset "name='rt715 ADC 25 Mux' 4" >>> cset "name='rt715 ADC 27 Capture Switch' 1" >>> cset "name='rt715 ADC 07 Capture Switch' 1" >>> cset "name='rt715 ADC 07 Capture Volume' 58" >>> ] >> >> To be clearer, we wanted to change the UCM files to only try to >> configure the 'old' controls when they are present. The 'new' controls >> are aligned between RT715 and RT715-sdca. >> >> There will be a minor inconvenience if an existing platform updates the >> kernel without updating UCM files, but it's the only solution we found >> in earlier discussions. >> >> Distributions are typically faster with alsa-ucmconf updates than kernel >> changes so that inconvenience is likely very limited (we support 4-5 >> Dell CML/TGL platforms w/ SoundWire) > > Was there any sustained objection to this change? Mark and Jaroslav? > > I would really like to fix the UCM files so that the mute and volume are > handled at the codec driver level, and not at the SOF level as it's > currently the case. > > The existing solution really makes no sense, it was 'inspired' (in the > copy-paste sense) by the PCH-DMIC handling for HDaudio platforms. In > that case, it made sense to use SOF-level mute/volume because such > controls don't exist in microphones. > > With RT715 we need to use the controls in the codec driver, not the > firmware. I'm basically ok to do any change which simplifies things. The user-space downgrade is possible but less used. The only my concern is that RT715 as an universal codec can handle the multiple stereo streams (current controls mapping) or the multichannel stream (new proposed controls mapping). The ASoC codec code should be universal, so it's a question, how to model the controls and how to detect and set the model. I think that we're missing a communication way between the DMA / machine driver and the codec driver. Yes, we can add DMI checks to RT715, but usually, the specific machines are already detected in the higher layer (soundwire bus, pci bus). It would be really nice, if the SOF driver can do a query: "If present, I need RT715 codec in the multichannel mode.". So my opinion is that both mappings may exist and the correct one should be selected at runtime. So I won't delete the old mapping, it may be usable. Mark? BTW: I already implemented the control remap plugin to alsa-lib, so you can split / merge controls with similar parameters as you like now. I need to do more test with the UCM integration, but it's here. https://github.com/alsa-project/alsa-lib/blob/master/src/control/control_remap.c#L1197 Jaroslav
> The only my concern is that RT715 as an universal codec can handle the > multiple stereo streams (current controls mapping) or the multichannel stream > (new proposed controls mapping). The ASoC codec code should be universal, so > it's a question, how to model the controls and how to detect and set the > model. I think that we're missing a communication way between the DMA / > machine driver and the codec driver. Yes, we can add DMI checks to RT715, but > usually, the specific machines are already detected in the higher layer > (soundwire bus, pci bus). It would be really nice, if the SOF driver can do a > query: "If present, I need RT715 codec in the multichannel mode.". The RT715 was an early SoundWire device, and it'll likely be superseded by the newer SDCA version. The direction for SDCA 'SmartMic' devices is not to manage independent stereo streams corresponding to different pairs of microphones, and it's not a solution we want to promote in the rest of the stack. There will be multiple streams provided by the codec, but each stream will be tied to a given functionality (regular capture, buffered capture with triggers, voice, etc). In addition, what can be configured by the user for volume/mute in 'Feature Units' is after platform-specific channel remapping/mixing/processing. It could very well be that there are 4 mics in a platform but only 2 channels are provided to the host. In other words, the streams generated by the codec and transmitted on the SoundWire bus will always be exposed with N channels, in a multichannel mode. We will need to report this N to UCM, currently that's missing, and the SOF topologies also have a limitation to 2ch that we will need to remove. > So my opinion is that both mappings may exist and the correct one should be > selected at runtime. So I won't delete the old mapping, it may be usable. Mark? What I was suggesting is not to delete the old controls in UCM but make them conditional. If present you use them, otherwise you use the newer solution. > BTW: I already implemented the control remap plugin to alsa-lib, so you can > split / merge controls with similar parameters as you like now. I need to do > more test with the UCM integration, but it's here. > > https://github.com/alsa-project/alsa-lib/blob/master/src/control/control_remap.c#L1197 thanks for the pointer, I'll have to look into this.
On Mon, 29 Mar 2021 06:54:00 +0000, Jack Yu wrote: > Using new kcontrols "Capture Switch" and "Capture Volume" instead, > remove kcontrols which no longer be used. Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [2/3] ASoC: rt715: remove kcontrols which no longer be used commit: fa2f98378f941786a93f8e63696f59fb4ac7538b 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
diff --git a/sound/soc/codecs/rt715.c b/sound/soc/codecs/rt715.c index 3cb8ab452bf1..b33eabb9f740 100644 --- a/sound/soc/codecs/rt715.c +++ b/sound/soc/codecs/rt715.c @@ -81,12 +81,20 @@ static int rt715_set_amp_gain_put(struct snd_kcontrol *kcontrol, (struct soc_mixer_control *)kcontrol->private_value; struct rt715_priv *rt715 = snd_soc_component_get_drvdata(component); unsigned int addr_h, addr_l, val_h, val_ll, val_lr; - unsigned int read_ll, read_rl; - int i; + unsigned int read_ll, read_rl, i; + unsigned int k_vol_changed = 0; + + for (i = 0; i < 2; i++) { + if (ucontrol->value.integer.value[i] != rt715->kctl_2ch_vol_ori[i]) { + k_vol_changed = 1; + break; + } + } /* Can't use update bit function, so read the original value first */ addr_h = mc->reg; addr_l = mc->rreg; + if (mc->shift == RT715_DIR_OUT_SFT) /* output */ val_h = 0x80; else /* input */ @@ -94,41 +102,27 @@ static int rt715_set_amp_gain_put(struct snd_kcontrol *kcontrol, rt715_get_gain(rt715, addr_h, addr_l, val_h, &read_rl, &read_ll); + if (dapm->bias_level <= SND_SOC_BIAS_STANDBY) + regmap_write(rt715->regmap, + RT715_SET_AUDIO_POWER_STATE, AC_PWRST_D0); + /* L Channel */ - if (mc->invert) { - /* for mute */ - val_ll = (mc->max - ucontrol->value.integer.value[0]) << 7; - /* keep gain */ - read_ll = read_ll & 0x7f; - val_ll |= read_ll; - } else { - /* for gain */ - val_ll = ((ucontrol->value.integer.value[0]) & 0x7f); - if (val_ll > mc->max) - val_ll = mc->max; - /* keep mute status */ - read_ll = read_ll & 0x80; - val_ll |= read_ll; - } + rt715->kctl_2ch_vol_ori[0] = ucontrol->value.integer.value[0]; + /* for gain */ + val_ll = ((ucontrol->value.integer.value[0]) & 0x7f); + if (val_ll > mc->max) + val_ll = mc->max; + /* keep mute status */ + val_ll |= read_ll & 0x80; /* R Channel */ - if (mc->invert) { - regmap_write(rt715->regmap, - RT715_SET_AUDIO_POWER_STATE, AC_PWRST_D0); - /* for mute */ - val_lr = (mc->max - ucontrol->value.integer.value[1]) << 7; - /* keep gain */ - read_rl = read_rl & 0x7f; - val_lr |= read_rl; - } else { - /* for gain */ - val_lr = ((ucontrol->value.integer.value[1]) & 0x7f); - if (val_lr > mc->max) - val_lr = mc->max; - /* keep mute status */ - read_rl = read_rl & 0x80; - val_lr |= read_rl; - } + rt715->kctl_2ch_vol_ori[1] = ucontrol->value.integer.value[1]; + /* for gain */ + val_lr = ((ucontrol->value.integer.value[1]) & 0x7f); + if (val_lr > mc->max) + val_lr = mc->max; + /* keep mute status */ + val_lr |= read_rl & 0x80; for (i = 0; i < 3; i++) { /* retry 3 times at most */ @@ -136,18 +130,18 @@ static int rt715_set_amp_gain_put(struct snd_kcontrol *kcontrol, /* Set both L/R channels at the same time */ val_h = (1 << mc->shift) | (3 << 4); regmap_write(rt715->regmap, addr_h, - (val_h << 8 | val_ll)); + (val_h << 8) | val_ll); regmap_write(rt715->regmap, addr_l, - (val_h << 8 | val_ll)); + (val_h << 8) | val_ll); } else { /* Lch*/ val_h = (1 << mc->shift) | (1 << 5); regmap_write(rt715->regmap, addr_h, - (val_h << 8 | val_ll)); + (val_h << 8) | val_ll); /* Rch */ val_h = (1 << mc->shift) | (1 << 4); regmap_write(rt715->regmap, addr_l, - (val_h << 8 | val_lr)); + (val_h << 8) | val_lr); } /* check result */ if (mc->shift == RT715_DIR_OUT_SFT) /* output */ @@ -156,15 +150,16 @@ static int rt715_set_amp_gain_put(struct snd_kcontrol *kcontrol, val_h = 0x0; rt715_get_gain(rt715, addr_h, addr_l, val_h, - &read_rl, &read_ll); + &read_rl, &read_ll); if (read_rl == val_lr && read_ll == val_ll) break; } + /* D0:power on state, D3: power saving mode */ if (dapm->bias_level <= SND_SOC_BIAS_STANDBY) regmap_write(rt715->regmap, RT715_SET_AUDIO_POWER_STATE, AC_PWRST_D3); - return 0; + return k_vol_changed; } static int rt715_set_amp_gain_get(struct snd_kcontrol *kcontrol, @@ -466,37 +461,9 @@ static int rt715_vol_info(struct snd_kcontrol *kcontrol, static const struct snd_kcontrol_new rt715_snd_controls[] = { /* Capture switch */ - SOC_DOUBLE_R_EXT("ADC 07 Capture Switch", RT715_SET_GAIN_MIC_ADC_H, - RT715_SET_GAIN_MIC_ADC_L, RT715_DIR_IN_SFT, 1, 1, - rt715_set_amp_gain_get, rt715_set_amp_gain_put), - SOC_DOUBLE_R_EXT("ADC 08 Capture Switch", RT715_SET_GAIN_LINE_ADC_H, - RT715_SET_GAIN_LINE_ADC_L, RT715_DIR_IN_SFT, 1, 1, - rt715_set_amp_gain_get, rt715_set_amp_gain_put), - SOC_DOUBLE_R_EXT("ADC 09 Capture Switch", RT715_SET_GAIN_MIX_ADC_H, - RT715_SET_GAIN_MIX_ADC_L, RT715_DIR_IN_SFT, 1, 1, - rt715_set_amp_gain_get, rt715_set_amp_gain_put), - SOC_DOUBLE_R_EXT("ADC 27 Capture Switch", RT715_SET_GAIN_MIX_ADC2_H, - RT715_SET_GAIN_MIX_ADC2_L, RT715_DIR_IN_SFT, 1, 1, - rt715_set_amp_gain_get, rt715_set_amp_gain_put), RT715_MAIN_SWITCH_EXT("Capture Switch", rt715_set_main_switch_get, rt715_set_main_switch_put), /* Volume Control */ - SOC_DOUBLE_R_EXT_TLV("ADC 07 Capture Volume", RT715_SET_GAIN_MIC_ADC_H, - RT715_SET_GAIN_MIC_ADC_L, RT715_DIR_IN_SFT, 0x3f, 0, - rt715_set_amp_gain_get, rt715_set_amp_gain_put, - in_vol_tlv), - SOC_DOUBLE_R_EXT_TLV("ADC 08 Capture Volume", RT715_SET_GAIN_LINE_ADC_H, - RT715_SET_GAIN_LINE_ADC_L, RT715_DIR_IN_SFT, 0x3f, 0, - rt715_set_amp_gain_get, rt715_set_amp_gain_put, - in_vol_tlv), - SOC_DOUBLE_R_EXT_TLV("ADC 09 Capture Volume", RT715_SET_GAIN_MIX_ADC_H, - RT715_SET_GAIN_MIX_ADC_L, RT715_DIR_IN_SFT, 0x3f, 0, - rt715_set_amp_gain_get, rt715_set_amp_gain_put, - in_vol_tlv), - SOC_DOUBLE_R_EXT_TLV("ADC 27 Capture Volume", RT715_SET_GAIN_MIX_ADC2_H, - RT715_SET_GAIN_MIX_ADC2_L, RT715_DIR_IN_SFT, 0x3f, 0, - rt715_set_amp_gain_get, rt715_set_amp_gain_put, - in_vol_tlv), RT715_MAIN_VOL_EXT_TLV("Capture Volume", rt715_set_main_vol_get, rt715_set_main_vol_put, in_vol_tlv), /* MIC Boost Control */ diff --git a/sound/soc/codecs/rt715.h b/sound/soc/codecs/rt715.h index 72c320bd35cb..25dba61f1760 100644 --- a/sound/soc/codecs/rt715.h +++ b/sound/soc/codecs/rt715.h @@ -22,6 +22,7 @@ struct rt715_priv { struct sdw_bus_params params; bool hw_init; bool first_hw_init; + unsigned int kctl_2ch_vol_ori[2]; unsigned int kctl_8ch_switch_ori[8]; unsigned int kctl_8ch_vol_ori[8]; };
Using new kcontrols "Capture Switch" and "Capture Volume" instead, remove kcontrols which no longer be used. Signed-off-by: Jack Yu <jack.yu@realtek.com> --- sound/soc/codecs/rt715.c | 103 +++++++++++++-------------------------- sound/soc/codecs/rt715.h | 1 + 2 files changed, 36 insertions(+), 68 deletions(-) -- 2.29.0