[PATCH] ASoC: rt711-sdca: remove capture switch controls

[Jaroslav Kysela]

Dne 19. 04. 21 v 9:17 Yang, Libin napsal(a):
> Hi Jaroslav,
> 
>> -----Original Message-----
>> From: Jaroslav Kysela <perex at perex.cz>
>> Sent: 2021年4月19日 14:57
>> To: Yang, Libin <libin.yang at intel.com>; Pierre-Louis Bossart <pierre-
>> louis.bossart at linux.intel.com>; shumingf at realtek.com; broonie at kernel.org;
>> lgirdwood at gmail.com
>> Cc: oder_chiou at realtek.com; jack.yu at realtek.com; alsa-devel at alsa-
>> project.org; lars at metafoo.de; derek.fang at realtek.com; flove at realtek.com
>> Subject: Re: [PATCH] ASoC: rt711-sdca: remove capture switch controls
>>
>> Dne 19. 04. 21 v 8:50 Yang, Libin napsal(a):
>>> Hi Jaroslav,
>>>
>>>> -----Original Message-----
>>>> From: Jaroslav Kysela <perex at perex.cz>
>>>> Sent: 2021年4月19日 14:32
>>>> To: Yang, Libin <libin.yang at intel.com>; Pierre-Louis Bossart <pierre-
>>>> louis.bossart at linux.intel.com>; shumingf at realtek.com;
>>>> broonie at kernel.org; lgirdwood at gmail.com
>>>> Cc: oder_chiou at realtek.com; jack.yu at realtek.com; alsa-devel at alsa-
>>>> project.org; lars at metafoo.de; derek.fang at realtek.com;
>>>> flove at realtek.com
>>>> Subject: Re: [PATCH] ASoC: rt711-sdca: remove capture switch controls
>>>>
>>>> Dne 19. 04. 21 v 7:14 Yang, Libin napsal(a):
>>>>> Hi Pierre,
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Pierre-Louis Bossart <pierre-louis.bossart at linux.intel.com>
>>>>>> Sent: 2021年4月15日 19:26
>>>>>> To: shumingf at realtek.com; broonie at kernel.org;
>> lgirdwood at gmail.com
>>>>>> Cc: oder_chiou at realtek.com; jack.yu at realtek.com; alsa-devel at alsa-
>>>>>> project.org; lars at metafoo.de; Yang, Libin <libin.yang at intel.com>;
>>>>>> derek.fang at realtek.com; flove at realtek.com
>>>>>> Subject: Re: [PATCH] ASoC: rt711-sdca: remove capture switch
>>>>>> controls
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 4/15/21 4:16 AM, shumingf at realtek.com wrote:
>>>>>>> From: Shuming Fan <shumingf at realtek.com>
>>>>>>>
>>>>>>> The settings of the switch control already set by DAPM event.
>>>>>>> These switch controls might the user confused why it can't disable
>>>>>>> the
>>>>>> capture.
>>>>>>
>>>>>> Sorry, not following. This control is used in the alsa-ucm-conf
>>>>>> that was just merged
>>>>>>
>>>>>> https://github.com/alsa-project/alsa-ucm-
>>>>>> conf/commit/197025656ec456331d1a34357b113913ec3b187f#diff-
>>>>>>
>>>>
>> 0e1c627ea89ee148fdb41aa6b3ba7851ba9c20eb43c1b87b9e0ce92164273dc3R
>>>>>> 7
>>>>>>
>>>>>> Is this saying this commit needs to be fixed with a follow-up PR?
>>>>>> Libin, are you working on this?
>>>>>
>>>>> Yes, I will submit a patch for UCM after this patch is merged.
>>>>>
>>>>> The background is Jaroslav requires using codec HW kcontrol for
>>>>> capture volume/switch. Shuming and I worked on it. At first we
>>>>> wanted to use FU0F to replace PGA kcontrol. But Shuming found FU0F
>>>>> is used in
>>>> DAPM.
>>>>> So it is not proper for the capture volume/switch. And Shuming will
>>>>> remove the FU0F kcontrol.
>>>>
>>>> Is switch control name aligned with the volume control name after
>>>> this change?
>>>
>>> This patch is removing "FU0F Capture Switch". Before this patch, there
>>> is "FU0F Capture Switch" and "FU0F Capture Volume". After this patch
>>> is applied, "FU0F Capture Switch" is removed. So the UCM
>>> https://github.com/alsa-project/alsa-ucm-
>> conf/blob/57ead84278f641d411e
>>> 3ccbb5c8a4b64141904ba/ucm2/codecs/rt711-sdca/init.conf#L7
>>> of "FU0F Capture Switch" setting needs to be removed.
>>>
>>>> I mean "A Capture Switch" + "A Capture Volume" not "A Capture Switch"
>>>> + "B Capture Volume".
>>>
>>> We couldn't find a proper codec kcontrol for RT711 capture. So we have
>>> to continue using PGA kcontrol.
>>
>> It's really confusing then. What does "FU0F Capture Volume" ? It's really
>> difficult to judge something when I don't know the codec diagram.
> 
> "FU0F Capture Volume" will still control the volume of rt711 capture.
> But "FU0F Capture Switch" will be removed. Because DAPM is using
> this the same node. These is a conflict when we do below:
> 1. mute "FU0F Capture Switch". And the kcontrol will mute the
>     Corresponding node in codec.
> 2. use arecord to capture from the rt711-sdca headset
> 3. DAPM will auto unmute the node which "FU0F Capture Switch" has 
>      muted.
> 4. So we will capture the sound, even "FU0F Capture Switch" shows the 
>     state is "muted" (this is wrong)
> 
> So Shuming decided to remove the "FU0F Capture Switch" kcontrol.

I see. In this case, the auto-route settings should differ from the mixer
settings. So the mute flag should be logical _OR_ from both DAPM and the mixer
settings. And because the codec is able to do the hw mute, why to prevent the
export of this feature?

So I propose do do (pseudo code):

struct rt711_sdca_priv {
	bool fu0f_dapm_mute;
	bool fu0f_mixer_mute;
};

/* called from both dapm event and kontrol put callback (on change) */
/* the dapm event and put callback will modify only rt711_sdca_priv fields */
static void set_f0f_mute(rt711_priv)
{
	int mute = rt711_priv->fu0f_dapm_mute || rt711_priv->fu0f_mixer_mute;
	set_fu0f_mute_register(mute);
}

With this implementation, all is consistent to the user space.

>> Anyway, the switch and volume for the given I/O should have identical name
>> and they should differ only in the suffix describing the stream and
>> functionality.
> 
> We won't touch "CaptureSwitch" and "CaptureVolume" for rt711-sdca.

Yes, but the hw controls should be used instead DSP controls, if they are
available.

					Jaroslav

-- 
Jaroslav Kysela <perex at perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.