Re: [PATCH v3 2/3] ASoC: codecs: add new pm4125 audio codec driver

[Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>]

On 9/8/25 7:18 PM, Alexey Klimov wrote:
> On Mon Sep 8, 2025 at 4:45 PM BST, Srinivas Kandagatla wrote:
>> On 9/8/25 4:26 PM, Alexey Klimov wrote:
>>> Hi Srini,
>>>
>>> On Fri Aug 15, 2025 at 4:36 PM BST, Srinivas Kandagatla wrote:
>>
>>
>>>>> +static int pm4125_codec_enable_rxclk(struct snd_soc_dapm_widget *w, struct snd_kcontrol *kcontrol,
>>>>> +				     int event)
>>>>> +{
>>>>> +	struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
>>>>> +
>>>>> +	switch (event) {
>>>>> +	case SND_SOC_DAPM_PRE_PMU:
>>>>>
>>>>> +static const struct snd_kcontrol_new pm4125_snd_controls[] = {
>>>>> +	SOC_SINGLE_EXT("HPHL_COMP Switch", SND_SOC_NOPM, 0, 1, 0,
>>>>
>>>>     SOC_SINGLE_EXT("HPHL_COMP Switch", PM4125_COMP_L, 0, 1, 0, ?
>>>>
>>>>> +		       pm4125_get_compander, pm4125_set_compander),
>>>>> +	SOC_SINGLE_EXT("HPHR_COMP Switch", SND_SOC_NOPM, 1, 1, 0,
>>>>
>>>>        SOC_SINGLE_EXT("HPHR_COMP Switch", PM4125_COMP_R, 1, 1, 0,?
>>>>
>>>>> +		       pm4125_get_compander, pm4125_set_compander),
>>>>
>>>> This is same issue in one of the WCD codec, which am going to send fixes
>>>> along with my original wcd fixes series.
>>>
>>> So this was in other codecs for years, right?
>>
>> only in wcd937x since it was added.
>>
>>>
>>>>> +
>>>>> +	SOC_SINGLE_TLV("HPHL Volume", PM4125_ANA_HPHPA_L_GAIN, 0, 20, 1,
>>>>> +		       line_gain),
>>>>> +	SOC_SINGLE_TLV("HPHR Volume", PM4125_ANA_HPHPA_R_GAIN, 0, 20, 1,
>>>>> +		       line_gain),
>> ...
>>                           PM4125_ANA_HPHPA_FSM_DIV_RATIO_MASK,
>>>>                                       PM4125_ANA_HPHPA_FSM_DIV_RATIO_68);
>>>> @@ -309,8 +307,6 @@ static int pm4125_rx_clk_disable(struct
>>>> snd_soc_component *component)
>>>>         snd_soc_component_write_field(component,
>>>> PM4125_DIG_SWR_CDC_RX_CLK_CTL,
>>>>                                       PM4125_DIG_SWR_ANA_RX_CLK_EN_MASK,
>>>>                                       PM4125_DIG_SWR_RX_CLK_DISABLE);
>>>> -       pm4125_global_mbias_disable(component);
>>>> -
>>>>         return 0;
>>>>  }
>>>
>>> This doesn't work. Playback has two issues: 1) volume is very low and probably
>>> not adjustable and 2) sound during playback dies after a couple of seconds.
>>>
>>> Returning these global_mbias() calls restores the good behaviour.
>>
>> What is global micbias to do with rx playback? Maybe something is missing.
> 
> No idea. Spec that I can see doesn't say a lot about PM4125_ANA_MBIAS_EN register.
> Just that these are main biases.
> 
>> Which RX path are you testing?
>>
>> I am testing this with Headphone and Lineout, it works for me.
>>
>>>
>>> Maybe let's make a widget out of it? In such case I am not sure about
>>> routing meaning that I not sure which paths do require mbias enable.
>> Not sure why RX would need mic bias?
> 
> I am testing lineout that goes into wsa881x amplifier in analog mode (mono).
> I use the following amixers:
> 
> amixer -c0 cset iface=MIXER,name='RX_CODEC_DMA_RX_0 Audio Mixer MultiMedia1' 1
> amixer -c0 cset iface=MIXER,name='RX_MACRO RX0 MUX' 1
> amixer -c0 cset iface=MIXER,name='RX INT0 DEM MUX' 1
> amixer -c0 cset iface=MIXER,name='RX INT0_1 MIX1 INP0' 'RX0'
> #merge two channels together (right channel goes into INT0_1 input1)
> amixer -c0 cset iface=MIXER,name='RX INT0_1 MIX1 INP1' 'RX1'
> amixer -c0 cset iface=MIXER,name='EAR_RDAC Switch' 0
> amixer -c0 cset iface=MIXER,name='HPHL_RDAC Switch' 0
> amixer -c0 cset iface=MIXER,name='HPHR_RDAC Switch' 0
> amixer -c0 cset iface=MIXER,name='LO_RDAC Switch' 1
> amixer -c0 cset iface=MIXER,name='HPHL Switch' 1
> amixer -c0 cset iface=MIXER,name='RX_RX0 Digital Volume' 80
> amixer -c0 cset iface=MIXER,name='SpkrMono WSA_RDAC' 1
> 
> and then aplay. With long .wav files the sound dies after few seconds of playback.
> But volume issue is present with any .wav files.
> 
> After playing with pm4125_global_mbias_enable(), tt looks like
> both PM4125_ANA_MBIAS_EN_GLOBAL and PM4125_ANA_MBIAS_EN_V2I should be enabled to avoid this problem.
> 
Looks like this is main bias for all tx and rx circuits. we need a power
dapm widget to reflect this dependency correctly.

--srini

> Thanks,
> Alexey
>