Re: [PATCH] ASoC: rt715: add main capture switch and main capture volume control - Pierre-Louis Bossart

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
To: Jaroslav Kysela <perex@perex.cz>,
	jack.yu@realtek.com, broonie@kernel.org, lgirdwood@gmail.com,
	Takashi Iwai <tiwai@suse.de>
Cc: oder_chiou@realtek.com, alsa-devel@alsa-project.org,
	lars@metafoo.de, derek.fang@realtek.com, flove@realtek.com,
	shumingf@realtek.com, bard.liao@intel.com
Subject: Re: [PATCH] ASoC: rt715: add main capture switch and main capture volume control
Date: Mon, 14 Dec 2020 11:12:22 -0600	[thread overview]
Message-ID: <7d74544f-77f2-db01-db3d-d1d8a56d576d@linux.intel.com> (raw)
In-Reply-To: <0b0afd81-0595-c661-ec1e-e81b7e8f8fec@perex.cz>

Hi Jaroslav,

>>>> Add main capture switch and main capture volume for callback to be in
>>>> single operation.
>>>
>>> Could you be more verbose, what you're trying to do in the patch description?
> 
> I see that it's just additional volume coupling functionality (one control,
> set all four output volume/switch registers, right?).
> 
> Some points:
> 
> 1) the separate volume controls don't send change events back to the user
> space when the coupled capture settings is applied and versa vice - no sync
> 2) we have already virtual master API - sound/core/vmaster.c which should
> cover this requirement
> 3) I don't see the purpose for this coupling (the capture direction)

That was for UCM integration - agree the context was not well captured 
in the commit message

We have 'Capture Switch' and 'Capture Volume' statements that are 
required, and currently mistakenly set to SOF controls when they should 
be codec controls.

So when we have several possible inputs controls for the codec (ADC 07 
or ADC 27) depending on the microphone settings, which one should be 
used in UCM?

We thought this would simplify the UCM integration by adding one generic 
control. If you have a better suggestion we are all ears.

>>>> +	 * "Main Capture Volume" looks the same as "ADC 07 Capture Volume",
>>>
>>> I would just use "Capture Volume" and "Capture Switch" here without the Main
>>> prefix to follow other drivers.
>>
>> It's similar to the 'Master Capture Switch' used in HDaudio, what other
>> drivers were you referring to?
> 
> HDAudio is using just 'Capture Switch' and 'Capture Volume' for the root
> capture controls plus the input selector (enum). The Master prefix is used
> only for the playback direction. And the word master is not prohibited for the
> audio context, is it?

The naming is not the problem, we can remove the 'main' if needed, the 
point is how to go about UCM integration.