[v14,8/8] ASoC: sun4i-codec: Add Left Capture Select, Right Capture Select
diff mbox

Message ID 20180502210800.1971-9-dannym@scratchpost.org
State New
Headers show

Commit Message

Danny Milosavljevic May 2, 2018, 9:08 p.m. UTC
Add Left Capture Select and Right Capture Select for Allwinner A10 and
Allwinner A20.

Signed-off-by: Danny Milosavljevic <dannym@scratchpost.org>
---
 sound/soc/sunxi/sun4i-codec.c | 50 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

Comments

Maxime Ripard May 3, 2018, 2:54 p.m. UTC | #1
Hi,

On Wed, May 02, 2018 at 11:08:00PM +0200, Danny Milosavljevic wrote:
> Add Left Capture Select and Right Capture Select for Allwinner A10 and
> Allwinner A20.
> 
> Signed-off-by: Danny Milosavljevic <dannym@scratchpost.org>
> ---
>  sound/soc/sunxi/sun4i-codec.c | 50 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
> 
> diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
> index 45dcb80a1da9..1a4ba3bae5da 100644
> --- a/sound/soc/sunxi/sun4i-codec.c
> +++ b/sound/soc/sunxi/sun4i-codec.c
> @@ -706,6 +706,25 @@ static DECLARE_TLV_DB_RANGE(sun7i_codec_micin_preamp_gain_scale,
>  			    0, 0, TLV_DB_SCALE_ITEM(0, 0, 0),
>  			    1, 7, TLV_DB_SCALE_ITEM(2400, 300, 0));
>  
> +static const char * const sun4i_codec_capture_source[] = {
> +	"Line",
> +	"FM",
> +	"Mic1",
> +	"Mic2",
> +	"Mic1,Mic2",
> +	"Mic1+Mic2",
> +	"Output Mixer",
> +	"Line,Mic1",
> +};

Shouldn't that be defined in a more generic way? As far as I know,
there's no way to tell what the difference between "Mic1,Mic2" and
"Mic1+Mic2" would be from the userspace.

I guess the larger issue is that you'd need to have controls that are
common to both channels (ie that would change the routing of both
mixers), but aren't sharing the same controls structure since the
label would be different.

Maxime
Danny Milosavljevic May 5, 2018, 10:40 a.m. UTC | #2
Hi Maxime,

On Thu, 3 May 2018 16:54:08 +0200
Maxime Ripard <maxime.ripard@bootlin.com> wrote:

> > +static const char * const sun4i_codec_capture_source[] = {
> > +	"Line",
> > +	"FM",
> > +	"Mic1",
> > +	"Mic2",
> > +	"Mic1,Mic2",
> > +	"Mic1+Mic2",
> > +	"Output Mixer",
> > +	"Line,Mic1",
> > +};  
> 
> Shouldn't that be defined in a more generic way? As far as I know,
> there's no way to tell what the difference between "Mic1,Mic2" and
> "Mic1+Mic2" would be from the userspace.

Sounds good - but how?

Here, "Mic1,Mic2" means the left channel captured is Mic1 and the right
channel captured is Mic2.

On the other hand, "Mic1+Mic2" means that the signals from Mic1 and
Mic2 are added together and that is captured (both as left and as right).

"Mic1" means both the left channel and the right channel captured is
from Mic1.  Likewise "Mic2".
Maxime Ripard May 14, 2018, 2:11 p.m. UTC | #3
On Sat, May 05, 2018 at 12:40:50PM +0200, Danny Milosavljevic wrote:
> Hi Maxime,
> 
> On Thu, 3 May 2018 16:54:08 +0200
> Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> 
> > > +static const char * const sun4i_codec_capture_source[] = {
> > > +	"Line",
> > > +	"FM",
> > > +	"Mic1",
> > > +	"Mic2",
> > > +	"Mic1,Mic2",
> > > +	"Mic1+Mic2",
> > > +	"Output Mixer",
> > > +	"Line,Mic1",
> > > +};  
> > 
> > Shouldn't that be defined in a more generic way? As far as I know,
> > there's no way to tell what the difference between "Mic1,Mic2" and
> > "Mic1+Mic2" would be from the userspace.
> 
> Sounds good - but how?
> 
> Here, "Mic1,Mic2" means the left channel captured is Mic1 and the right
> channel captured is Mic2.
> 
> On the other hand, "Mic1+Mic2" means that the signals from Mic1 and
> Mic2 are added together and that is captured (both as left and as right).
> 
> "Mic1" means both the left channel and the right channel captured is
> from Mic1.  Likewise "Mic2".

Right, and my point isn't that it is difficult to understand or
remember once you get it, but that it's difficult to get it in the
first place, and that this convention is solely based on the one used
in the datasheet, which is an abstraction violation in itself.

I guess that's really up to Mark here, but one solution would be to
allow to couple the controls explicitly instead of relying solely on
the fact that they share the same controls array pointer.

Maxime

Patch
diff mbox

diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
index 45dcb80a1da9..1a4ba3bae5da 100644
--- a/sound/soc/sunxi/sun4i-codec.c
+++ b/sound/soc/sunxi/sun4i-codec.c
@@ -706,6 +706,25 @@  static DECLARE_TLV_DB_RANGE(sun7i_codec_micin_preamp_gain_scale,
 			    0, 0, TLV_DB_SCALE_ITEM(0, 0, 0),
 			    1, 7, TLV_DB_SCALE_ITEM(2400, 300, 0));
 
+static const char * const sun4i_codec_capture_source[] = {
+	"Line",
+	"FM",
+	"Mic1",
+	"Mic2",
+	"Mic1,Mic2",
+	"Mic1+Mic2",
+	"Output Mixer",
+	"Line,Mic1",
+};
+
+static SOC_ENUM_SINGLE_DECL(sun4i_codec_enum_capture_source,
+			    SUN4I_CODEC_ADC_ACTL,
+			    SUN4I_CODEC_ADC_ACTL_ADCIS,
+			    sun4i_codec_capture_source);
+
+static const struct snd_kcontrol_new sun4i_codec_capture_source_controls =
+	SOC_DAPM_ENUM("Capture Source", sun4i_codec_enum_capture_source);
+
 static const char * const sun4i_codec_difflinein_source[] = {
 	"Stereo",
 	"Differential",
@@ -841,6 +860,10 @@  static const struct snd_soc_dapm_widget sun4i_codec_codec_dapm_widgets[] = {
 			    &sun4i_codec_pa_mute),
 
 	/* MUX */
+	SND_SOC_DAPM_MUX("Left Capture Select", SND_SOC_NOPM, 0, 0,
+			 &sun4i_codec_capture_source_controls),
+	SND_SOC_DAPM_MUX("Right Capture Select", SND_SOC_NOPM, 0, 0,
+			 &sun4i_codec_capture_source_controls),
 	SND_SOC_DAPM_MUX("Differential Line Source", SND_SOC_NOPM,
 			 0, 0,
 			 &sun4i_codec_difflinein_source_controls),
@@ -908,6 +931,33 @@  static const struct snd_soc_dapm_route sun4i_codec_codec_dapm_routes[] = {
 	/* LNRDF Routes */
 	{ "Differential Line Source", "Differential", "Line Left" },
 	{ "Differential Line Source", "Differential", "Line Right" },
+
+	/* Right ADC Input Routes */
+	{ "Right Capture Select", "Line", "Line Right" },
+	{ "Right Capture Select", "Line", "Differential Line Capture Switch" },
+	{ "Right Capture Select", "FM", "FM Right" },
+	{ "Right Capture Select", "Mic1", "MIC1 Pre-Amplifier" },
+	{ "Right Capture Select", "Mic2", "MIC2 Pre-Amplifier" },
+	{ "Right Capture Select", "Mic1,Mic2", "MIC2 Pre-Amplifier" },
+	{ "Right Capture Select", "Mic1+Mic2", "MIC2 Pre-Amplifier" },
+	{ "Right Capture Select", "Mic1+Mic2", "MIC1 Pre-Amplifier" },
+	{ "Right Capture Select", "Output Mixer", "Right Mixer" },
+	{ "Right Capture Select", "Line,Mic1", "MIC1 Pre-Amplifier" },
+	{ "Right ADC", NULL, "Right Capture Select" },
+
+	/* Left ADC Input Routes */
+	{ "Left Capture Select", "Line", "Line Left" },
+	{ "Left Capture Select", "Line", "Differential Line Capture Switch" },
+	{ "Left Capture Select", "FM", "FM Left" },
+	{ "Left Capture Select", "Mic1", "MIC1 Pre-Amplifier" },
+	{ "Left Capture Select", "Mic2", "MIC2 Pre-Amplifier" },
+	{ "Left Capture Select", "Mic1,Mic2", "MIC1 Pre-Amplifier" },
+	{ "Left Capture Select", "Mic1+Mic2", "MIC1 Pre-Amplifier" },
+	{ "Left Capture Select", "Mic1+Mic2", "MIC2 Pre-Amplifier" },
+	{ "Left Capture Select", "Output Mixer", "Left Mixer" },
+	{ "Left Capture Select", "Line,Mic1", "Line Left" },
+	{ "Left Capture Select", "Line,Mic1", "Differential Line Capture Switch" },
+	{ "Left ADC", NULL, "Left Capture Select" },
 };
 
 struct sun4i_codec_quirks {