[PATCH v8 0/2] ASoc: sun4i-codec: Add FM, Line and Mic inputs

[PATCH v8 0/2] ASoc: sun4i-codec: Add FM, Line and Mic inputs

[Danny Milosavljevic]

Hi,

this is the eighth version of the patch set that adds inputs to sun4i-codec.

Changes compared to v7 are:
 - fix the routes for line and mic capturing.

The patches are on top of the branch "sunxi-next" in 
<git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git>.

Regards,
   Danny
---
Danny Milosavljevic (2):
  sun4i-codec: Add FM, Line and Mic inputs
  sun4i-codec: make it possible to use different codec_widgets for A10 and A20.

 b/sound/soc/sunxi/sun4i-codec.c |  230 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 215 insertions(+), 15 deletions(-)

[PATCH v8 1/2] ASoc: sun4i-codec: Add FM, Line and Mic inputs

[Danny Milosavljevic]

This is the first part:

   sun4i-codec: make it possible to use different codec_widgets for A10 and A20.

Signed-off-by: Danny Milosavljevic <dannym+a-bxPqe3T81XXwRsdMLXbzog@public.gmane.org>
---
  b/sound/soc/sunxi/sun4i-codec.c |   45 ++++++++++++++++++++++++++++++----------
  1 file changed, 34 insertions(+), 11 deletions(-)

diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
index e6cc6a1..6628e6e 100644
--- a/sound/soc/sunxi/sun4i-codec.c
+++ b/sound/soc/sunxi/sun4i-codec.c
@@ -96,8 +96,9 @@
 /* Other various ADC registers */
 #define SUN4I_CODEC_DAC_TXCNT			(0x30)
 #define SUN4I_CODEC_ADC_RXCNT			(0x34)
-#define SUN4I_CODEC_AC_SYS_VERI			(0x38)
-#define SUN4I_CODEC_AC_MIC_PHONE_CAL		(0x3c)
+
+#define SUN7I_CODEC_AC_DAC_CAL			(0x38)
+#define SUN7I_CODEC_AC_MIC_PHONE_CAL		(0x3c)
 
 struct sun4i_codec {
 	struct device	*dev;
@@ -509,10 +510,13 @@ static const struct snd_kcontrol_new sun4i_codec_pa_mute =
 
 static DECLARE_TLV_DB_SCALE(sun4i_codec_pa_volume_scale, -6300, 100, 1);
 
-static const struct snd_kcontrol_new sun4i_codec_widgets[] = {
-	SOC_SINGLE_TLV("Power Amplifier Volume", SUN4I_CODEC_DAC_ACTL,
-		       SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0,
-		       sun4i_codec_pa_volume_scale),
+#define SUN4I_COMMON_CODEC_WIDGETS \
+	SOC_SINGLE_TLV("Power Amplifier Volume", SUN4I_CODEC_DAC_ACTL,\
+		       SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0,\
+		       sun4i_codec_pa_volume_scale)
+
+static const struct snd_kcontrol_new sun4i_codec_widgets_a10[] = {
+	SUN4I_COMMON_CODEC_WIDGETS,
 };
 
 static const struct snd_kcontrol_new sun4i_codec_left_mixer_controls[] = {
@@ -627,9 +631,9 @@ static const struct snd_soc_dapm_route sun4i_codec_codec_dapm_routes[] = {
 	{ "Mic1", NULL, "VMIC" },
 };
 
-static struct snd_soc_codec_driver sun4i_codec_codec = {
-	.controls		= sun4i_codec_widgets,
-	.num_controls		= ARRAY_SIZE(sun4i_codec_widgets),
+static struct snd_soc_codec_driver sun4i_codec_codec_a10 = {
+	.controls		= sun4i_codec_widgets_a10,
+	.num_controls		= ARRAY_SIZE(sun4i_codec_widgets_a10),
 	.dapm_widgets		= sun4i_codec_codec_dapm_widgets,
 	.num_dapm_widgets	= ARRAY_SIZE(sun4i_codec_codec_dapm_widgets),
 	.dapm_routes		= sun4i_codec_codec_dapm_routes,
@@ -680,7 +684,7 @@ static const struct regmap_config sun4i_codec_regmap_config = {
 	.reg_bits	= 32,
 	.reg_stride	= 4,
 	.val_bits	= 32,
-	.max_register	= SUN4I_CODEC_AC_MIC_PHONE_CAL,
+	.max_register	= SUN7I_CODEC_AC_MIC_PHONE_CAL,
 };
 
 static const struct of_device_id sun4i_codec_of_match[] = {
@@ -753,10 +757,24 @@ static struct snd_soc_card *sun4i_codec_create_card(struct device *dev)
 	return card;
 };
 
+static const struct snd_kcontrol_new sun7i_codec_widgets[] = {
+	SUN4I_COMMON_CODEC_WIDGETS,
+};
+
+static struct snd_soc_codec_driver sun7i_codec_codec = {
+	.controls		= sun7i_codec_widgets,
+	.num_controls		= ARRAY_SIZE(sun7i_codec_widgets),
+	.dapm_widgets		= sun4i_codec_codec_dapm_widgets,
+	.num_dapm_widgets	= ARRAY_SIZE(sun4i_codec_codec_dapm_widgets),
+	.dapm_routes		= sun4i_codec_codec_dapm_routes,
+	.num_dapm_routes	= ARRAY_SIZE(sun4i_codec_codec_dapm_routes),
+};
+
 static int sun4i_codec_probe(struct platform_device *pdev)
 {
 	struct snd_soc_card *card;
 	struct sun4i_codec *scodec;
+	struct snd_soc_codec_driver *codec;
 	struct resource *res;
 	void __iomem *base;
 	int ret;
@@ -819,7 +837,12 @@ static int sun4i_codec_probe(struct platform_device *pdev)
 	scodec->capture_dma_data.maxburst = 4;
 	scodec->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
 
-	ret = snd_soc_register_codec(&pdev->dev, &sun4i_codec_codec,
+	if (of_device_is_compatible(pdev->dev.of_node,
+				    "allwinner,sun7i-a20-codec"))
+		codec = &sun7i_codec_codec;
+	else
+		codec = &sun4i_codec_codec_a10;
+	ret = snd_soc_register_codec(&pdev->dev, codec,
 				     &sun4i_codec_dai, 1);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to register our codec\n");

Re: [PATCH v8 1/2] ASoc: sun4i-codec: Add FM, Line and Mic inputs

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 4468 bytes --]

Hi,

On Mon, Dec 21, 2015 at 12:33:03PM +0100, Danny Milosavljevic wrote:
> This is the first part:
> 
>    sun4i-codec: make it possible to use different codec_widgets for A10 and A20.

Please make a meaningful commit log and title explaining what and why
you're doing it.

> 
> Signed-off-by: Danny Milosavljevic <dannym+a-bxPqe3T81XXwRsdMLXbzog@public.gmane.org>
> ---
>   b/sound/soc/sunxi/sun4i-codec.c |   45 ++++++++++++++++++++++++++++++----------
>   1 file changed, 34 insertions(+), 11 deletions(-)
> 
> diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
> index e6cc6a1..6628e6e 100644
> --- a/sound/soc/sunxi/sun4i-codec.c
> +++ b/sound/soc/sunxi/sun4i-codec.c
> @@ -96,8 +96,9 @@
>  /* Other various ADC registers */
>  #define SUN4I_CODEC_DAC_TXCNT			(0x30)
>  #define SUN4I_CODEC_ADC_RXCNT			(0x34)
> -#define SUN4I_CODEC_AC_SYS_VERI			(0x38)
> -#define SUN4I_CODEC_AC_MIC_PHONE_CAL		(0x3c)
> +
> +#define SUN7I_CODEC_AC_DAC_CAL			(0x38)
> +#define SUN7I_CODEC_AC_MIC_PHONE_CAL		(0x3c)
>
>  struct sun4i_codec {
>  	struct device	*dev;
> @@ -509,10 +510,13 @@ static const struct snd_kcontrol_new sun4i_codec_pa_mute =
>  
>  static DECLARE_TLV_DB_SCALE(sun4i_codec_pa_volume_scale, -6300, 100, 1);
>  
> -static const struct snd_kcontrol_new sun4i_codec_widgets[] = {
> -	SOC_SINGLE_TLV("Power Amplifier Volume", SUN4I_CODEC_DAC_ACTL,
> -		       SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0,
> -		       sun4i_codec_pa_volume_scale),
> +#define SUN4I_COMMON_CODEC_WIDGETS \
> +	SOC_SINGLE_TLV("Power Amplifier Volume", SUN4I_CODEC_DAC_ACTL,\
> +		       SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0,\
> +		       sun4i_codec_pa_volume_scale)
> +
> +static const struct snd_kcontrol_new sun4i_codec_widgets_a10[] = {
> +	SUN4I_COMMON_CODEC_WIDGETS,
>  };
>  
>  static const struct snd_kcontrol_new sun4i_codec_left_mixer_controls[] = {
> @@ -627,9 +631,9 @@ static const struct snd_soc_dapm_route sun4i_codec_codec_dapm_routes[] = {
>  	{ "Mic1", NULL, "VMIC" },
>  };
>  
> -static struct snd_soc_codec_driver sun4i_codec_codec = {
> -	.controls		= sun4i_codec_widgets,
> -	.num_controls		= ARRAY_SIZE(sun4i_codec_widgets),
> +static struct snd_soc_codec_driver sun4i_codec_codec_a10 = {
> +	.controls		= sun4i_codec_widgets_a10,
> +	.num_controls		= ARRAY_SIZE(sun4i_codec_widgets_a10),

There's no need to change the structure name.

>  	.dapm_widgets		= sun4i_codec_codec_dapm_widgets,
>  	.num_dapm_widgets	= ARRAY_SIZE(sun4i_codec_codec_dapm_widgets),
>  	.dapm_routes		= sun4i_codec_codec_dapm_routes,
> @@ -680,7 +684,7 @@ static const struct regmap_config sun4i_codec_regmap_config = {
>  	.reg_bits	= 32,
>  	.reg_stride	= 4,
>  	.val_bits	= 32,
> -	.max_register	= SUN4I_CODEC_AC_MIC_PHONE_CAL,
> +	.max_register	= SUN7I_CODEC_AC_MIC_PHONE_CAL,
>  };
>  
>  static const struct of_device_id sun4i_codec_of_match[] = {
> @@ -753,10 +757,24 @@ static struct snd_soc_card *sun4i_codec_create_card(struct device *dev)
>  	return card;
>  };
>  
> +static const struct snd_kcontrol_new sun7i_codec_widgets[] = {
> +	SUN4I_COMMON_CODEC_WIDGETS,
> +};
> +
> +static struct snd_soc_codec_driver sun7i_codec_codec = {
> +	.controls		= sun7i_codec_widgets,
> +	.num_controls		= ARRAY_SIZE(sun7i_codec_widgets),
> +	.dapm_widgets		= sun4i_codec_codec_dapm_widgets,
> +	.num_dapm_widgets	= ARRAY_SIZE(sun4i_codec_codec_dapm_widgets),
> +	.dapm_routes		= sun4i_codec_codec_dapm_routes,
> +	.num_dapm_routes	= ARRAY_SIZE(sun4i_codec_codec_dapm_routes),
> +};

>  static int sun4i_codec_probe(struct platform_device *pdev)
>  {
>  	struct snd_soc_card *card;
>  	struct sun4i_codec *scodec;
> +	struct snd_soc_codec_driver *codec;
>  	struct resource *res;
>  	void __iomem *base;
>  	int ret;
> @@ -819,7 +837,12 @@ static int sun4i_codec_probe(struct platform_device *pdev)
>  	scodec->capture_dma_data.maxburst = 4;
>  	scodec->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
>  
> -	ret = snd_soc_register_codec(&pdev->dev, &sun4i_codec_codec,
> +	if (of_device_is_compatible(pdev->dev.of_node,
> +				    "allwinner,sun7i-a20-codec"))
> +		codec = &sun7i_codec_codec;
> +	else
> +		codec = &sun4i_codec_codec_a10;

A new line here would be nice.

> +	ret = snd_soc_register_codec(&pdev->dev, codec,
>  				     &sun4i_codec_dai, 1);
>  	if (ret) {
>  		dev_err(&pdev->dev, "Failed to register our codec\n");

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH v8 2/2] ASoc: sun4i-codec: Add FM, Line and Mic inputs

[Danny Milosavljevic]

This is the second part, actually adding FM, Line and Mic inputs.

Signed-off-by: Danny Milosavljevic <dannym+a-bxPqe3T81XXwRsdMLXbzog@public.gmane.org>
---
 b/sound/soc/sunxi/sun4i-codec.c |  185 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 181 insertions(+), 4 deletions(-)

diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
index 6628e6e..f23d8a9 100644
--- a/sound/soc/sunxi/sun4i-codec.c
+++ b/sound/soc/sunxi/sun4i-codec.c
@@ -59,9 +59,20 @@
 #define SUN4I_CODEC_DAC_ACTL_DACAENR			(31)
 #define SUN4I_CODEC_DAC_ACTL_DACAENL			(30)
 #define SUN4I_CODEC_DAC_ACTL_MIXEN			(29)
+#define SUN4I_CODEC_DAC_ACTL_LNG			(26)
+#define SUN4I_CODEC_DAC_ACTL_FMG			(23)
+#define SUN4I_CODEC_DAC_ACTL_MICG			(20)
+#define SUN4I_CODEC_DAC_ACTL_LLNS			(19)
+#define SUN4I_CODEC_DAC_ACTL_RLNS			(18)
+#define SUN4I_CODEC_DAC_ACTL_LFMS			(17)
+#define SUN4I_CODEC_DAC_ACTL_RFMS			(16)
 #define SUN4I_CODEC_DAC_ACTL_LDACLMIXS			(15)
 #define SUN4I_CODEC_DAC_ACTL_RDACRMIXS			(14)
 #define SUN4I_CODEC_DAC_ACTL_LDACRMIXS			(13)
+#define SUN4I_CODEC_DAC_ACTL_MIC1LS			(12)
+#define SUN4I_CODEC_DAC_ACTL_MIC1RS			(11)
+#define SUN4I_CODEC_DAC_ACTL_MIC2LS			(10)
+#define SUN4I_CODEC_DAC_ACTL_MIC2RS			(9)
 #define SUN4I_CODEC_DAC_ACTL_DACPAS			(8)
 #define SUN4I_CODEC_DAC_ACTL_MIXPAS			(7)
 #define SUN4I_CODEC_DAC_ACTL_PA_MUTE			(6)
@@ -87,8 +98,11 @@
 #define SUN4I_CODEC_ADC_ACTL_PREG1EN			(29)
 #define SUN4I_CODEC_ADC_ACTL_PREG2EN			(28)
 #define SUN4I_CODEC_ADC_ACTL_VMICEN			(27)
-#define SUN4I_CODEC_ADC_ACTL_VADCG			(20)
+#define SUN4I_CODEC_ADC_ACTL_PREG1_A10			(25)
+#define SUN4I_CODEC_ADC_ACTL_PREG2_A10			(23)
+#define SUN4I_CODEC_ADC_ACTL_ADCG			(20)
 #define SUN4I_CODEC_ADC_ACTL_ADCIS			(17)
+#define SUN4I_CODEC_ADC_ACTL_LNRDF			(16)
 #define SUN4I_CODEC_ADC_ACTL_PA_EN			(4)
 #define SUN4I_CODEC_ADC_ACTL_DDE			(3)
 #define SUN4I_CODEC_ADC_DEBUG			(0x2c)
@@ -100,6 +114,16 @@
 #define SUN7I_CODEC_AC_DAC_CAL			(0x38)
 #define SUN7I_CODEC_AC_MIC_PHONE_CAL		(0x3c)
 
+#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PREG1      (29)
+#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PREG2      (26)
+/* note: no idea where the output pins for the following are. */
+#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PHONEOUTG  (5)
+#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PHONEOUTEN (4)
+#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PHONEOUTS3 (3)
+#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PHONEOUTS2 (2)
+#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PHONEOUTS1 (1)
+#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PHONEOUTS0 (0)
+
 struct sun4i_codec {
 	struct device	*dev;
 	struct regmap	*regmap;
@@ -509,19 +533,102 @@ static const struct snd_kcontrol_new sun4i_codec_pa_mute =
 			SUN4I_CODEC_DAC_ACTL_PA_MUTE, 1, 0);
 
 static DECLARE_TLV_DB_SCALE(sun4i_codec_pa_volume_scale, -6300, 100, 1);
+static DECLARE_TLV_DB_SCALE(sun4i_codec_linein_loopback_gain_scale,
+			    -150,
+			    150,
+			    0);
+static DECLARE_TLV_DB_SCALE(sun4i_codec_fmin_loopback_gain_scale,
+			    -450,
+			    150,
+			    0);
+static DECLARE_TLV_DB_SCALE(sun4i_codec_micin_loopback_gain_scale,
+			    -450,
+			    150,
+			    0);
+static DECLARE_TLV_DB_RANGE(sun4i_codec_micin_preamp_gain_scale_a10,
+			    0, 0, TLV_DB_SCALE_ITEM(0, 0, 0),
+			    1, 7, TLV_DB_SCALE_ITEM(3500, 300, 0));
+static DECLARE_TLV_DB_SCALE(sun4i_codec_adc_gain_scale, -450, 150, 0);
+/* Sources:
+ *   A10 User Manual v1.5 20130820
+ *   A20 User Manual v1.4 20150510
+ */
+static const char * const sun4i_codec_capture_source[] = {
+	"Line-In",
+	"FM",
+	"Mic1",
+	"Mic2",
+	"Mic1,Mic2",
+	"Mic1+Mic2",
+	"Output Mixer",
+	"Line-In,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("Route", sun4i_codec_enum_capture_source);
 
 #define SUN4I_COMMON_CODEC_WIDGETS \
-	SOC_SINGLE_TLV("Power Amplifier Volume", SUN4I_CODEC_DAC_ACTL,\
-		       SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0,\
-		       sun4i_codec_pa_volume_scale)
+	SOC_SINGLE_TLV("Power Amplifier Playback Volume", SUN4I_CODEC_DAC_ACTL,\
+		       SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0, \
+		       sun4i_codec_pa_volume_scale), \
+	/* Line-In, FM, Mic1, Mic2 */ \
+	SOC_SINGLE_TLV("Line-In Playback Volume", \
+		       SUN4I_CODEC_DAC_ACTL, \
+		       SUN4I_CODEC_DAC_ACTL_LNG, \
+		       1, \
+		       0, \
+		       sun4i_codec_linein_loopback_gain_scale), \
+	SOC_SINGLE_TLV("FM Playback Volume", \
+		       SUN4I_CODEC_DAC_ACTL, \
+		       SUN4I_CODEC_DAC_ACTL_FMG, \
+		       3, \
+		       0, \
+		       sun4i_codec_fmin_loopback_gain_scale), \
+	SOC_SINGLE_TLV("Mic Playback Volume", \
+		       SUN4I_CODEC_DAC_ACTL, \
+		       SUN4I_CODEC_DAC_ACTL_MICG, \
+		       7, \
+		       0, \
+		       sun4i_codec_micin_loopback_gain_scale), \
+	/* ADC */ \
+	SOC_SINGLE_TLV("Capture Volume", \
+		       SUN4I_CODEC_ADC_ACTL, \
+		       SUN4I_CODEC_ADC_ACTL_ADCG, \
+		       4, \
+		       0, \
+		       sun4i_codec_adc_gain_scale)
 
 static const struct snd_kcontrol_new sun4i_codec_widgets_a10[] = {
 	SUN4I_COMMON_CODEC_WIDGETS,
+	SOC_SINGLE_TLV("Mic1 Capture Volume",
+		       SUN4I_CODEC_ADC_ACTL,
+		       SUN4I_CODEC_ADC_ACTL_PREG1_A10,
+		       3,
+		       0,
+		       sun4i_codec_micin_preamp_gain_scale_a10),
+	SOC_SINGLE_TLV("Mic2 Capture Volume",
+		       SUN4I_CODEC_ADC_ACTL,
+		       SUN4I_CODEC_ADC_ACTL_PREG2_A10,
+		       3,
+		       0,
+		       sun4i_codec_micin_preamp_gain_scale_a10),
 };
 
 static const struct snd_kcontrol_new sun4i_codec_left_mixer_controls[] = {
 	SOC_DAPM_SINGLE("Left DAC Playback Switch", SUN4I_CODEC_DAC_ACTL,
 			SUN4I_CODEC_DAC_ACTL_LDACLMIXS, 1, 0),
+	SOC_DAPM_SINGLE("Left Line-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
+			SUN4I_CODEC_DAC_ACTL_LLNS, 1, 0),
+	SOC_DAPM_SINGLE("Left FM Playback Switch", SUN4I_CODEC_DAC_ACTL,
+			SUN4I_CODEC_DAC_ACTL_LFMS, 1, 0),
+	SOC_DAPM_SINGLE("Mic1 Playback Switch", SUN4I_CODEC_DAC_ACTL,
+			SUN4I_CODEC_DAC_ACTL_MIC1LS, 1, 0),
+	SOC_DAPM_SINGLE("Mic2 Playback Switch", SUN4I_CODEC_DAC_ACTL,
+			SUN4I_CODEC_DAC_ACTL_MIC2LS, 1, 0),
 };
 
 static const struct snd_kcontrol_new sun4i_codec_right_mixer_controls[] = {
@@ -529,6 +636,14 @@ static const struct snd_kcontrol_new sun4i_codec_right_mixer_controls[] = {
 			SUN4I_CODEC_DAC_ACTL_RDACRMIXS, 1, 0),
 	SOC_DAPM_SINGLE("Left DAC Playback Switch", SUN4I_CODEC_DAC_ACTL,
 			SUN4I_CODEC_DAC_ACTL_LDACRMIXS, 1, 0),
+	SOC_DAPM_SINGLE("Right Line-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
+			SUN4I_CODEC_DAC_ACTL_RLNS, 1, 0),
+	SOC_DAPM_SINGLE("Right FM Playback Switch", SUN4I_CODEC_DAC_ACTL,
+			SUN4I_CODEC_DAC_ACTL_RFMS, 1, 0),
+	SOC_DAPM_SINGLE("Mic1 Playback Switch", SUN4I_CODEC_DAC_ACTL,
+			SUN4I_CODEC_DAC_ACTL_MIC1RS, 1, 0),
+	SOC_DAPM_SINGLE("Mic2 Playback Switch", SUN4I_CODEC_DAC_ACTL,
+			SUN4I_CODEC_DAC_ACTL_MIC2RS, 1, 0),
 };
 
 static const struct snd_kcontrol_new sun4i_codec_pa_mixer_controls[] = {
@@ -561,6 +676,10 @@ static const struct snd_soc_dapm_widget sun4i_codec_codec_dapm_widgets[] = {
 	SND_SOC_DAPM_DAC("Right DAC", "Codec Playback", SUN4I_CODEC_DAC_ACTL,
 			 SUN4I_CODEC_DAC_ACTL_DACAENR, 0),
 
+	/* MUX */
+	SND_SOC_DAPM_MUX("Capture Source", SND_SOC_NOPM, 0, 0,
+			 &sun4i_codec_capture_source_controls),
+
 	/* Mixers */
 	SND_SOC_DAPM_MIXER("Left Mixer", SND_SOC_NOPM, 0, 0,
 			   sun4i_codec_left_mixer_controls,
@@ -580,6 +699,8 @@ static const struct snd_soc_dapm_widget sun4i_codec_codec_dapm_widgets[] = {
 	/* Mic Pre-Amplifiers */
 	SND_SOC_DAPM_PGA("MIC1 Pre-Amplifier", SUN4I_CODEC_ADC_ACTL,
 			 SUN4I_CODEC_ADC_ACTL_PREG1EN, 0, NULL, 0),
+	SND_SOC_DAPM_PGA("MIC2 Pre-Amplifier", SUN4I_CODEC_ADC_ACTL,
+			 SUN4I_CODEC_ADC_ACTL_PREG2EN, 0, NULL, 0),
 
 	/* Power Amplifier */
 	SND_SOC_DAPM_MIXER("Power Amplifier", SUN4I_CODEC_ADC_ACTL,
@@ -590,9 +711,15 @@ static const struct snd_soc_dapm_widget sun4i_codec_codec_dapm_widgets[] = {
 			    &sun4i_codec_pa_mute),
 
 	SND_SOC_DAPM_INPUT("Mic1"),
+	SND_SOC_DAPM_INPUT("Mic2"),
 
 	SND_SOC_DAPM_OUTPUT("HP Right"),
 	SND_SOC_DAPM_OUTPUT("HP Left"),
+
+	SND_SOC_DAPM_INPUT("Line-In Right"),
+	SND_SOC_DAPM_INPUT("Line-In Left"),
+	SND_SOC_DAPM_INPUT("FM Right"),
+	SND_SOC_DAPM_INPUT("FM Left"),
 };
 
 static const struct snd_soc_dapm_route sun4i_codec_codec_dapm_routes[] = {
@@ -629,6 +756,39 @@ static const struct snd_soc_dapm_route sun4i_codec_codec_dapm_routes[] = {
 	{ "Right ADC", NULL, "MIC1 Pre-Amplifier" },
 	{ "MIC1 Pre-Amplifier", NULL, "Mic1"},
 	{ "Mic1", NULL, "VMIC" },
+	{ "Right Mixer", "Mic1 Playback Switch", "MIC1 Pre-Amplifier" },
+	{ "Left Mixer", "Mic1 Playback Switch", "MIC1 Pre-Amplifier" },
+
+	/* Mic2 Routes */
+	{ "Left ADC", NULL, "MIC2 Pre-Amplifier" },
+	{ "Right ADC", NULL, "MIC2 Pre-Amplifier" },
+	{ "MIC2 Pre-Amplifier", NULL, "Mic2"},
+	{ "Mic2", NULL, "VMIC" },
+	{ "Right Mixer", "Mic2 Playback Switch", "MIC2 Pre-Amplifier" },
+	{ "Left Mixer", "Mic2 Playback Switch", "MIC2 Pre-Amplifier" },
+
+	/* Line-In, FM Routes */
+	{ "Right Mixer", "Right Line-In Playback Switch", "Line-In Right" },
+	{ "Left Mixer", "Left Line-In Playback Switch", "Line-In Left" },
+	{ "Right Mixer", "Right FM Playback Switch", "FM Right" },
+	{ "Left Mixer", "Left FM Playback Switch", "FM Left" },
+
+	/* ADC Capturing Sources */
+	{"Capture Source", "Line-In", "Line-In Left"},
+	{"Capture Source", "Line-In", "Line-In Right"},
+	{"Capture Source", "FM", "FM Left"},
+	{"Capture Source", "FM", "FM Right"},
+	{"Capture Source", "Mic1", "MIC1 Pre-Amplifier"},
+	{"Capture Source", "Mic2", "MIC2 Pre-Amplifier"},
+	{"Capture Source", "Mic1,Mic2", "MIC1 Pre-Amplifier"},
+	{"Capture Source", "Mic1,Mic2", "MIC2 Pre-Amplifier"},
+	{"Capture Source", "Mic1+Mic2", "MIC1 Pre-Amplifier"},
+	{"Capture Source", "Mic1+Mic2", "MIC2 Pre-Amplifier"},
+	{"Capture Source", "Output Mixer", "Left Mixer"},
+	{"Capture Source", "Output Mixer", "Right Mixer"},
+	{"Capture Source", "Line-In,Mic1", "Line-In Left"},
+	{"Capture Source", "Line-In,Mic1", "Line-In Right"}, /* depends on LNRDF */
+	{"Capture Source", "Line-In,Mic1", "MIC1 Pre-Amplifier"},
 };
 
 static struct snd_soc_codec_driver sun4i_codec_codec_a10 = {
@@ -757,8 +917,25 @@ static struct snd_soc_card *sun4i_codec_create_card(struct device *dev)
 	return card;
 };
 
+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 struct snd_kcontrol_new sun7i_codec_widgets[] = {
 	SUN4I_COMMON_CODEC_WIDGETS,
+	SOC_SINGLE_TLV("Mic1 Capture Volume",
+		       SUN7I_CODEC_AC_MIC_PHONE_CAL,
+		       SUN7I_CODEC_AC_MIC_PHONE_CAL_PREG1,
+		       7,
+		       0,
+		       sun7i_codec_micin_preamp_gain_scale),
+	SOC_SINGLE_TLV("Mic2 Capture Volume",
+		       SUN7I_CODEC_AC_MIC_PHONE_CAL,
+		       SUN7I_CODEC_AC_MIC_PHONE_CAL_PREG2,
+		       7,
+		       0,
+		       sun7i_codec_micin_preamp_gain_scale),
 };
 
 static struct snd_soc_codec_driver sun7i_codec_codec = {

Re: [PATCH v8 2/2] ASoc: sun4i-codec: Add FM, Line and Mic inputs

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 12777 bytes --]

Hi,

On Mon, Dec 21, 2015 at 12:34:16PM +0100, Danny Milosavljevic wrote:
> This is the second part, actually adding FM, Line and Mic inputs.

Again, having a meaningful and standalone commit log would be nice.

> Signed-off-by: Danny Milosavljevic <dannym+a-bxPqe3T81XXwRsdMLXbzog@public.gmane.org>
> ---
>  b/sound/soc/sunxi/sun4i-codec.c |  185 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 181 insertions(+), 4 deletions(-)
> 
> diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
> index 6628e6e..f23d8a9 100644
> --- a/sound/soc/sunxi/sun4i-codec.c
> +++ b/sound/soc/sunxi/sun4i-codec.c
> @@ -59,9 +59,20 @@
>  #define SUN4I_CODEC_DAC_ACTL_DACAENR			(31)
>  #define SUN4I_CODEC_DAC_ACTL_DACAENL			(30)
>  #define SUN4I_CODEC_DAC_ACTL_MIXEN			(29)
> +#define SUN4I_CODEC_DAC_ACTL_LNG			(26)
> +#define SUN4I_CODEC_DAC_ACTL_FMG			(23)
> +#define SUN4I_CODEC_DAC_ACTL_MICG			(20)
> +#define SUN4I_CODEC_DAC_ACTL_LLNS			(19)
> +#define SUN4I_CODEC_DAC_ACTL_RLNS			(18)
> +#define SUN4I_CODEC_DAC_ACTL_LFMS			(17)
> +#define SUN4I_CODEC_DAC_ACTL_RFMS			(16)
>  #define SUN4I_CODEC_DAC_ACTL_LDACLMIXS			(15)
>  #define SUN4I_CODEC_DAC_ACTL_RDACRMIXS			(14)
>  #define SUN4I_CODEC_DAC_ACTL_LDACRMIXS			(13)
> +#define SUN4I_CODEC_DAC_ACTL_MIC1LS			(12)
> +#define SUN4I_CODEC_DAC_ACTL_MIC1RS			(11)
> +#define SUN4I_CODEC_DAC_ACTL_MIC2LS			(10)
> +#define SUN4I_CODEC_DAC_ACTL_MIC2RS			(9)
>  #define SUN4I_CODEC_DAC_ACTL_DACPAS			(8)
>  #define SUN4I_CODEC_DAC_ACTL_MIXPAS			(7)
>  #define SUN4I_CODEC_DAC_ACTL_PA_MUTE			(6)
> @@ -87,8 +98,11 @@
>  #define SUN4I_CODEC_ADC_ACTL_PREG1EN			(29)
>  #define SUN4I_CODEC_ADC_ACTL_PREG2EN			(28)
>  #define SUN4I_CODEC_ADC_ACTL_VMICEN			(27)
> -#define SUN4I_CODEC_ADC_ACTL_VADCG			(20)
> +#define SUN4I_CODEC_ADC_ACTL_PREG1_A10			(25)
> +#define SUN4I_CODEC_ADC_ACTL_PREG2_A10			(23)

Why the A10 suffix?

> +#define SUN4I_CODEC_ADC_ACTL_ADCG			(20)
>  #define SUN4I_CODEC_ADC_ACTL_ADCIS			(17)
> +#define SUN4I_CODEC_ADC_ACTL_LNRDF			(16)
>  #define SUN4I_CODEC_ADC_ACTL_PA_EN			(4)
>  #define SUN4I_CODEC_ADC_ACTL_DDE			(3)
>  #define SUN4I_CODEC_ADC_DEBUG			(0x2c)
> @@ -100,6 +114,16 @@
>  #define SUN7I_CODEC_AC_DAC_CAL			(0x38)
>  #define SUN7I_CODEC_AC_MIC_PHONE_CAL		(0x3c)
>  
> +#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PREG1      (29)
> +#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PREG2      (26)
> +/* note: no idea where the output pins for the following are. */
> +#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PHONEOUTG  (5)
> +#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PHONEOUTEN (4)
> +#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PHONEOUTS3 (3)
> +#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PHONEOUTS2 (2)
> +#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PHONEOUTS1 (1)
> +#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PHONEOUTS0 (0)

Are you using that PHONEOUT stuff anywhere?

> +
>  struct sun4i_codec {
>  	struct device	*dev;
>  	struct regmap	*regmap;
> @@ -509,19 +533,102 @@ static const struct snd_kcontrol_new sun4i_codec_pa_mute =
>  			SUN4I_CODEC_DAC_ACTL_PA_MUTE, 1, 0);
>  
>  static DECLARE_TLV_DB_SCALE(sun4i_codec_pa_volume_scale, -6300, 100, 1);
> +static DECLARE_TLV_DB_SCALE(sun4i_codec_linein_loopback_gain_scale,
> +			    -150,
> +			    150,
> +			    0);
> +static DECLARE_TLV_DB_SCALE(sun4i_codec_fmin_loopback_gain_scale,
> +			    -450,
> +			    150,
> +			    0);
> +static DECLARE_TLV_DB_SCALE(sun4i_codec_micin_loopback_gain_scale,
> +			    -450,
> +			    150,
> +			    0);
> +static DECLARE_TLV_DB_RANGE(sun4i_codec_micin_preamp_gain_scale_a10,
> +			    0, 0, TLV_DB_SCALE_ITEM(0, 0, 0),
> +			    1, 7, TLV_DB_SCALE_ITEM(3500, 300, 0));
> +static DECLARE_TLV_DB_SCALE(sun4i_codec_adc_gain_scale, -450, 150, 0);
> +/* Sources:
> + *   A10 User Manual v1.5 20130820
> + *   A20 User Manual v1.4 20150510
> + */

There's no need to give your sources here.

> +static const char * const sun4i_codec_capture_source[] = {
> +	"Line-In",
> +	"FM",
> +	"Mic1",
> +	"Mic2",
> +	"Mic1,Mic2",
> +	"Mic1+Mic2",
> +	"Output Mixer",
> +	"Line-In,Mic1",
> +};
> +static SOC_ENUM_SINGLE_DECL(sun4i_codec_enum_capture_source,
> +			    SUN4I_CODEC_ADC_ACTL,
> +			    SUN4I_CODEC_ADC_ACTL_ADCIS,
> +			    sun4i_codec_capture_source);

Isn't it possible to expose this as two (shared) muxes with different
names to make it clear what will go to the left ADC and what will go
to the right?

> +static const struct snd_kcontrol_new sun4i_codec_capture_source_controls =
> +	SOC_DAPM_ENUM("Route", sun4i_codec_enum_capture_source);
>  
>  #define SUN4I_COMMON_CODEC_WIDGETS \
> -	SOC_SINGLE_TLV("Power Amplifier Volume", SUN4I_CODEC_DAC_ACTL,\
> -		       SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0,\
> -		       sun4i_codec_pa_volume_scale)
> +	SOC_SINGLE_TLV("Power Amplifier Playback Volume", SUN4I_CODEC_DAC_ACTL,\
> +		       SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0, \
> +		       sun4i_codec_pa_volume_scale), \

The power amplifier is only for the playback, so there's no need to
differentiate between playback and capture. The current name was fine.

> +	/* Line-In, FM, Mic1, Mic2 */ \
> +	SOC_SINGLE_TLV("Line-In Playback Volume", \
> +		       SUN4I_CODEC_DAC_ACTL, \
> +		       SUN4I_CODEC_DAC_ACTL_LNG, \
> +		       1, \
> +		       0, \
> +		       sun4i_codec_linein_loopback_gain_scale), \
> +	SOC_SINGLE_TLV("FM Playback Volume", \
> +		       SUN4I_CODEC_DAC_ACTL, \
> +		       SUN4I_CODEC_DAC_ACTL_FMG, \
> +		       3, \
> +		       0, \
> +		       sun4i_codec_fmin_loopback_gain_scale), \
> +	SOC_SINGLE_TLV("Mic Playback Volume", \
> +		       SUN4I_CODEC_DAC_ACTL, \
> +		       SUN4I_CODEC_DAC_ACTL_MICG, \
> +		       7, \
> +		       0, \
> +		       sun4i_codec_micin_loopback_gain_scale), \

Those are not volume it's gain, and it should probably be two
different shared controls for mic1 and mic2.

> +	/* ADC */ \
> +	SOC_SINGLE_TLV("Capture Volume", \
> +		       SUN4I_CODEC_ADC_ACTL, \
> +		       SUN4I_CODEC_ADC_ACTL_ADCG, \
> +		       4, \
> +		       0, \
> +		       sun4i_codec_adc_gain_scale)

This one is the ADC Gain. Please name it that way.

>  static const struct snd_kcontrol_new sun4i_codec_widgets_a10[] = {
>  	SUN4I_COMMON_CODEC_WIDGETS,
> +	SOC_SINGLE_TLV("Mic1 Capture Volume",
> +		       SUN4I_CODEC_ADC_ACTL,
> +		       SUN4I_CODEC_ADC_ACTL_PREG1_A10,
> +		       3,
> +		       0,
> +		       sun4i_codec_micin_preamp_gain_scale_a10),
> +	SOC_SINGLE_TLV("Mic2 Capture Volume",
> +		       SUN4I_CODEC_ADC_ACTL,
> +		       SUN4I_CODEC_ADC_ACTL_PREG2_A10,
> +		       3,
> +		       0,
> +		       sun4i_codec_micin_preamp_gain_scale_a10),

And those two are not capture volume, it's the pre-amplifier gain.

>  };
>  
>  static const struct snd_kcontrol_new sun4i_codec_left_mixer_controls[] = {
>  	SOC_DAPM_SINGLE("Left DAC Playback Switch", SUN4I_CODEC_DAC_ACTL,
>  			SUN4I_CODEC_DAC_ACTL_LDACLMIXS, 1, 0),
> +	SOC_DAPM_SINGLE("Left Line-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
> +			SUN4I_CODEC_DAC_ACTL_LLNS, 1, 0),
> +	SOC_DAPM_SINGLE("Left FM Playback Switch", SUN4I_CODEC_DAC_ACTL,
> +			SUN4I_CODEC_DAC_ACTL_LFMS, 1, 0),
> +	SOC_DAPM_SINGLE("Mic1 Playback Switch", SUN4I_CODEC_DAC_ACTL,
> +			SUN4I_CODEC_DAC_ACTL_MIC1LS, 1, 0),
> +	SOC_DAPM_SINGLE("Mic2 Playback Switch", SUN4I_CODEC_DAC_ACTL,
> +			SUN4I_CODEC_DAC_ACTL_MIC2LS, 1, 0),
>  };
>  
>  static const struct snd_kcontrol_new sun4i_codec_right_mixer_controls[] = {
> @@ -529,6 +636,14 @@ static const struct snd_kcontrol_new sun4i_codec_right_mixer_controls[] = {
>  			SUN4I_CODEC_DAC_ACTL_RDACRMIXS, 1, 0),
>  	SOC_DAPM_SINGLE("Left DAC Playback Switch", SUN4I_CODEC_DAC_ACTL,
>  			SUN4I_CODEC_DAC_ACTL_LDACRMIXS, 1, 0),
> +	SOC_DAPM_SINGLE("Right Line-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
> +			SUN4I_CODEC_DAC_ACTL_RLNS, 1, 0),
> +	SOC_DAPM_SINGLE("Right FM Playback Switch", SUN4I_CODEC_DAC_ACTL,
> +			SUN4I_CODEC_DAC_ACTL_RFMS, 1, 0),
> +	SOC_DAPM_SINGLE("Mic1 Playback Switch", SUN4I_CODEC_DAC_ACTL,
> +			SUN4I_CODEC_DAC_ACTL_MIC1RS, 1, 0),
> +	SOC_DAPM_SINGLE("Mic2 Playback Switch", SUN4I_CODEC_DAC_ACTL,
> +			SUN4I_CODEC_DAC_ACTL_MIC2RS, 1, 0),
>  };
>  
>  static const struct snd_kcontrol_new sun4i_codec_pa_mixer_controls[] = {
> @@ -561,6 +676,10 @@ static const struct snd_soc_dapm_widget sun4i_codec_codec_dapm_widgets[] = {
>  	SND_SOC_DAPM_DAC("Right DAC", "Codec Playback", SUN4I_CODEC_DAC_ACTL,
>  			 SUN4I_CODEC_DAC_ACTL_DACAENR, 0),
>  
> +	/* MUX */
> +	SND_SOC_DAPM_MUX("Capture Source", SND_SOC_NOPM, 0, 0,
> +			 &sun4i_codec_capture_source_controls),
> +

Please call it "ADC Source"

>  	/* Mixers */
>  	SND_SOC_DAPM_MIXER("Left Mixer", SND_SOC_NOPM, 0, 0,
>  			   sun4i_codec_left_mixer_controls,
> @@ -580,6 +699,8 @@ static const struct snd_soc_dapm_widget sun4i_codec_codec_dapm_widgets[] = {
>  	/* Mic Pre-Amplifiers */
>  	SND_SOC_DAPM_PGA("MIC1 Pre-Amplifier", SUN4I_CODEC_ADC_ACTL,
>  			 SUN4I_CODEC_ADC_ACTL_PREG1EN, 0, NULL, 0),
> +	SND_SOC_DAPM_PGA("MIC2 Pre-Amplifier", SUN4I_CODEC_ADC_ACTL,
> +			 SUN4I_CODEC_ADC_ACTL_PREG2EN, 0, NULL, 0),
>  
>  	/* Power Amplifier */
>  	SND_SOC_DAPM_MIXER("Power Amplifier", SUN4I_CODEC_ADC_ACTL,
> @@ -590,9 +711,15 @@ static const struct snd_soc_dapm_widget sun4i_codec_codec_dapm_widgets[] = {
>  			    &sun4i_codec_pa_mute),
>  
>  	SND_SOC_DAPM_INPUT("Mic1"),
> +	SND_SOC_DAPM_INPUT("Mic2"),
>  
>  	SND_SOC_DAPM_OUTPUT("HP Right"),
>  	SND_SOC_DAPM_OUTPUT("HP Left"),
> +
> +	SND_SOC_DAPM_INPUT("Line-In Right"),
> +	SND_SOC_DAPM_INPUT("Line-In Left"),
> +	SND_SOC_DAPM_INPUT("FM Right"),
> +	SND_SOC_DAPM_INPUT("FM Left"),

Having all the inputs and all the outputs grouped together would be
nice.

>  };
>  
>  static const struct snd_soc_dapm_route sun4i_codec_codec_dapm_routes[] = {
> @@ -629,6 +756,39 @@ static const struct snd_soc_dapm_route sun4i_codec_codec_dapm_routes[] = {
>  	{ "Right ADC", NULL, "MIC1 Pre-Amplifier" },
>  	{ "MIC1 Pre-Amplifier", NULL, "Mic1"},
>  	{ "Mic1", NULL, "VMIC" },
> +	{ "Right Mixer", "Mic1 Playback Switch", "MIC1 Pre-Amplifier" },
> +	{ "Left Mixer", "Mic1 Playback Switch", "MIC1 Pre-Amplifier" },
> +

The right and left mixer routes should be in the matching sections.

> +	/* Mic2 Routes */
> +	{ "Left ADC", NULL, "MIC2 Pre-Amplifier" },
> +	{ "Right ADC", NULL, "MIC2 Pre-Amplifier" },
> +	{ "MIC2 Pre-Amplifier", NULL, "Mic2"},
> +	{ "Mic2", NULL, "VMIC" },
> +	{ "Right Mixer", "Mic2 Playback Switch", "MIC2 Pre-Amplifier" },
> +	{ "Left Mixer", "Mic2 Playback Switch", "MIC2 Pre-Amplifier" },

Ditto.

> +
> +	/* Line-In, FM Routes */
> +	{ "Right Mixer", "Right Line-In Playback Switch", "Line-In Right" },
> +	{ "Left Mixer", "Left Line-In Playback Switch", "Line-In Left" },
> +	{ "Right Mixer", "Right FM Playback Switch", "FM Right" },
> +	{ "Left Mixer", "Left FM Playback Switch", "FM Left" },

Ditto.

> +
> +	/* ADC Capturing Sources */

"ADC Input Routes"

> +	{"Capture Source", "Line-In", "Line-In Left"},
> +	{"Capture Source", "Line-In", "Line-In Right"},
> +	{"Capture Source", "FM", "FM Left"},
> +	{"Capture Source", "FM", "FM Right"},
> +	{"Capture Source", "Mic1", "MIC1 Pre-Amplifier"},
> +	{"Capture Source", "Mic2", "MIC2 Pre-Amplifier"},
> +	{"Capture Source", "Mic1,Mic2", "MIC1 Pre-Amplifier"},
> +	{"Capture Source", "Mic1,Mic2", "MIC2 Pre-Amplifier"},
> +	{"Capture Source", "Mic1+Mic2", "MIC1 Pre-Amplifier"},
> +	{"Capture Source", "Mic1+Mic2", "MIC2 Pre-Amplifier"},
> +	{"Capture Source", "Output Mixer", "Left Mixer"},
> +	{"Capture Source", "Output Mixer", "Right Mixer"},
> +	{"Capture Source", "Line-In,Mic1", "Line-In Left"},
> +	{"Capture Source", "Line-In,Mic1", "Line-In Right"}, /* depends on LNRDF */
> +	{"Capture Source", "Line-In,Mic1", "MIC1 Pre-Amplifier"},
>  };

You should also have routes from the ADC Input to the Left and Right ADCs

>  static struct snd_soc_codec_driver sun4i_codec_codec_a10 = {
> @@ -757,8 +917,25 @@ static struct snd_soc_card *sun4i_codec_create_card(struct device *dev)
>  	return card;
>  };
>  
> +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 struct snd_kcontrol_new sun7i_codec_widgets[] = {
>  	SUN4I_COMMON_CODEC_WIDGETS,
> +	SOC_SINGLE_TLV("Mic1 Capture Volume",
> +		       SUN7I_CODEC_AC_MIC_PHONE_CAL,
> +		       SUN7I_CODEC_AC_MIC_PHONE_CAL_PREG1,
> +		       7,
> +		       0,
> +		       sun7i_codec_micin_preamp_gain_scale),
> +	SOC_SINGLE_TLV("Mic2 Capture Volume",
> +		       SUN7I_CODEC_AC_MIC_PHONE_CAL,
> +		       SUN7I_CODEC_AC_MIC_PHONE_CAL_PREG2,
> +		       7,
> +		       0,
> +		       sun7i_codec_micin_preamp_gain_scale),

"Mic1 Pre-Amplifier Gain", "Mic2 Pre-Amplifier Gain".

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: Re: [PATCH v8 2/2] ASoc: sun4i-codec: Add FM, Line and Mic inputs

[Danny Milosavljevic]

Hi Maxime,

On Sun, 27 Dec 2015 19:21:57 +0100
Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:

> On Mon, Dec 21, 2015 at 12:34:16PM +0100, Danny Milosavljevic wrote:
> > This is the second part, actually adding FM, Line and Mic inputs.
> 
> Again, having a meaningful and standalone commit log would be nice.

Okay, will elaborate some more in v9.

> > +#define SUN4I_CODEC_ADC_ACTL_PREG1_A10			(25)
> > +#define SUN4I_CODEC_ADC_ACTL_PREG2_A10			(23)
> 
> Why the A10 suffix?

The sun4i*_a10 names are for things that work on A10 but not on A20.
This way whoever touches the driver later can know for which things he has
to consider multiple cases.
Otherwise he will have no indication that he is using a bit index where 
there sometimes is no bit (or, worse, the wrong bit).

It's intended to be used like this:

sun4i_foo_a10
	foo is sun4i_foo_a10 on A10 (only).
sun4i_foo
	foo is sun4i_foo on A10 and also on future chips (like A20).
sun7i_foo
	foo is sun7i_foo on A20 and (hopefully) also on future chips.

In this case these defines are for PREG for A10 only.
The A20+ PREG are in the PHONE_CAL register, at other indices.

> Are you using that PHONEOUT stuff anywhere?

Nope.

I was just including all the bits of the register because I usually aim for 
completeness by ensuring that there are no gaps in the bit index ranges. 

With all the stripping out of the bit range comments that won't be possible 
anymore anyway, so I'll just get rid of the PHONEOUT defines in v9, too.

> There's no need to give your sources here.

Ok, will remove.

For the record, there are multiple versions of each of the documents 
and if you pick the wrong ones it will look as if there were more 
differences between A10 and A20 than there actually are.

> > +static const char * const sun4i_codec_capture_source[] = {
> > +	"Line-In",
> > +	"FM",
> > +	"Mic1",
> > +	"Mic2",
> > +	"Mic1,Mic2",
> > +	"Mic1+Mic2",
> > +	"Output Mixer",
> > +	"Line-In,Mic1",
> > +};
> > +static SOC_ENUM_SINGLE_DECL(sun4i_codec_enum_capture_source,
> > +			    SUN4I_CODEC_ADC_ACTL,
> > +			    SUN4I_CODEC_ADC_ACTL_ADCIS,
> > +			    sun4i_codec_capture_source);
> 
> Isn't it possible to expose this as two (shared) muxes with different
> names to make it clear what will go to the left ADC and what will go
> to the right?

I don't know how to do that. I'll try to find out.

(It would be best if someone who knows how that should act did the alsamixer 
testing later too, though)

Can two muxes use the same bit in the hardware without problems?
Or do you mean reuse the same mux instance? (I think _new1 always creates 
a new instance from each struct kcontrol_new automatically).

> The power amplifier is only for the playback, so there's no need to
> differentiate between playback and capture. The current name was fine.

"Power Amplifier Volume" shows up as Capture control in alsamixer as well.
It isn't supposed to be a Capture control.

> > +	/* Line-In, FM, Mic1, Mic2 */ \
> > +	SOC_SINGLE_TLV("Line-In Playback Volume", \
...
> > +	SOC_SINGLE_TLV("FM Playback Volume", \
...
> > +	SOC_SINGLE_TLV("Mic Playback Volume", \
...
> 
> Those are not volume it's gain, 

We tried to call the things ..." Gain" before and it was difficult to do, 
with some breakage along the way, see below.
Also, Mark said they should be named ..." Volume" (see 
<https://www.mail-archive.com/linux-sunxi-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org/msg15126.html>).

>and it should probably be two different shared controls for mic1 and mic2.

I'll try...

>> "Capture Source"
> This one is the ADC Gain. Please name it that way.

Unfortunately, the strings have meaning to asoc and alsa-lib and you can't go 
renaming them like that without breaking things. The names were carefully 
chosen to make it actually work properly without having to patch alsa-lib and 
parts of ASoC core (which I did before and have since reverted).

In this case, there's a special case in upstream alsa-lib:

alsa-lib-1.0.28:
./src/mixer/simple_none.c:	if (strcmp(name, "Capture Source") == 0) {
...

I'm not totally against naming them like you suggested - but know that you 
are requiring changes in alsa-lib as well then - or presumably breaking the 
user's workflow.

For example the (upstream) "Capture Source" special case in alsa-lib adds 
radio-button-like things to the respective elements. 
You can switch to one of the inputs by pressing Space while its gain element 
is selected.
In the mic case, it's the mic preamplier gain that you press Space on - if it's 
indeed shown as a Capture control...

> And those two are not capture volume, it's the pre-amplifier gain.

I know. There has been a previous small discussion in v4 about that and it's 
really not that easy.

For example for problems with loopback gain see:
<https://www.mail-archive.com/linux-sunxi-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org/msg15115.html>

Summary: you have to patch Linux/sound/soc/soc-ops.c and 
alsa-lib-1.0.28/src/mixer/simple_none.c to allow controls that don't end 
in " Volume" to be sliders... and, in some cases, to be visible at all.

> Having all the inputs and all the outputs grouped together would be
> nice.

Yeah, I'll do that in v9.

> The right and left mixer routes should be in the matching sections.
> Ditto.
> Ditto.

Yeah, I'll do that in v9 and add a comment instead.

> > +	/* ADC Capturing Sources */
> 
> "ADC Input Routes"

Yeah, I'll do that in v9.

> You should also have routes from the ADC Input to the Left and Right ADCs

Yeah, I'll do that in v9.

> [should be named] "Mic1 Pre-Amplifier Gain", "Mic2 Pre-Amplifier Gain".

It cannot be selected for recording then since it's not a Capture control.

These things are interdependent, even crossing syscall boundary.

Regards,
   Danny

Re: Re: [PATCH v8 2/2] ASoc: sun4i-codec: Add FM, Line and Mic inputs

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1587 bytes --]

On Mon, Dec 28, 2015 at 04:06:49AM +0100, Danny Milosavljevic wrote:
> Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:

> > > +static SOC_ENUM_SINGLE_DECL(sun4i_codec_enum_capture_source,
> > > +			    SUN4I_CODEC_ADC_ACTL,
> > > +			    SUN4I_CODEC_ADC_ACTL_ADCIS,
> > > +			    sun4i_codec_capture_source);

> > Isn't it possible to expose this as two (shared) muxes with different
> > names to make it clear what will go to the left ADC and what will go
> > to the right?

> I don't know how to do that. I'll try to find out.

> Can two muxes use the same bit in the hardware without problems?

IIRC we fixed things so that if you've got two controls that share the
same register we do the right thing.  There are other devices with this
issue but it was years ago that I last looked at them, we do have
handling.

> > Those are not volume it's gain, 

> We tried to call the things ..." Gain" before and it was difficult to do, 
> with some breakage along the way, see below.
> Also, Mark said they should be named ..." Volume" (see 
> <https://www.mail-archive.com/linux-sunxi-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org/msg15126.html>).
> 
> >and it should probably be two different shared controls for mic1 and mic2.

Yes, this is part of the ABI and honestly from an end user point of view
there is no meaningful difference between the two words (there would be
more if it were gain and attenuation since one is amplification and the
other isn't so there are different considerations when applying them to
signals but that's not the case here).

Re: Re: [PATCH v8 2/2] ASoc: sun4i-codec: Add FM, Line and Mic inputs

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 4917 bytes --]

On Mon, Dec 28, 2015 at 04:06:49AM +0100, Danny Milosavljevic wrote:
> Hi Maxime,
> 
> On Sun, 27 Dec 2015 19:21:57 +0100
> Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> 
> > On Mon, Dec 21, 2015 at 12:34:16PM +0100, Danny Milosavljevic wrote:
> > > This is the second part, actually adding FM, Line and Mic inputs.
> > 
> > Again, having a meaningful and standalone commit log would be nice.
> 
> Okay, will elaborate some more in v9.
> 
> > > +#define SUN4I_CODEC_ADC_ACTL_PREG1_A10			(25)
> > > +#define SUN4I_CODEC_ADC_ACTL_PREG2_A10			(23)
> > 
> > Why the A10 suffix?
> 
> The sun4i*_a10 names are for things that work on A10 but not on A20.
> This way whoever touches the driver later can know for which things he has
> to consider multiple cases.
> Otherwise he will have no indication that he is using a bit index where 
> there sometimes is no bit (or, worse, the wrong bit).
> 
> It's intended to be used like this:
> 
> sun4i_foo_a10
> 	foo is sun4i_foo_a10 on A10 (only).
> sun4i_foo
> 	foo is sun4i_foo on A10 and also on future chips (like A20).
> sun7i_foo
> 	foo is sun7i_foo on A20 and (hopefully) also on future chips.

I find the sun4i_*_a10 and sun4i highly redundant. If there the same
define for sun7i, then you know that it's not meant to be used for the
A20, and that's it.

My point is also that it will just be an ever growing list of suffixes
when we will support more SoCs, for example for symbols that might be
in the A10, not the A20, but the A31.

> > > +static const char * const sun4i_codec_capture_source[] = {
> > > +	"Line-In",
> > > +	"FM",
> > > +	"Mic1",
> > > +	"Mic2",
> > > +	"Mic1,Mic2",
> > > +	"Mic1+Mic2",
> > > +	"Output Mixer",
> > > +	"Line-In,Mic1",
> > > +};
> > > +static SOC_ENUM_SINGLE_DECL(sun4i_codec_enum_capture_source,
> > > +			    SUN4I_CODEC_ADC_ACTL,
> > > +			    SUN4I_CODEC_ADC_ACTL_ADCIS,
> > > +			    sun4i_codec_capture_source);
> > 
> > Isn't it possible to expose this as two (shared) muxes with different
> > names to make it clear what will go to the left ADC and what will go
> > to the right?
> 
> I don't know how to do that. I'll try to find out.
> 
> (It would be best if someone who knows how that should act did the alsamixer 
> testing later too, though)
> 
> Can two muxes use the same bit in the hardware without problems?
> Or do you mean reuse the same mux instance? (I think _new1 always creates 
> a new instance from each struct kcontrol_new automatically).

Yeah, the case where two widgets share the same bits is handled iirc
but sharing the controls.

> > The power amplifier is only for the playback, so there's no need to
> > differentiate between playback and capture. The current name was fine.
> 
> "Power Amplifier Volume" shows up as Capture control in alsamixer as well.
> It isn't supposed to be a Capture control.
> > > +	/* Line-In, FM, Mic1, Mic2 */ \
> > > +	SOC_SINGLE_TLV("Line-In Playback Volume", \
> ...
> > > +	SOC_SINGLE_TLV("FM Playback Volume", \
> ...
> > > +	SOC_SINGLE_TLV("Mic Playback Volume", \
> ...
> > 
> > Those are not volume it's gain, 
> 
> We tried to call the things ..." Gain" before and it was difficult to do, 
> with some breakage along the way, see below.
> Also, Mark said they should be named ..." Volume" (see 
> <https://www.mail-archive.com/linux-sunxi-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org/msg15126.html>).

Ah, my bad.

Judging from ControlNames.txt, the Power Amplifier Volume should
probably called Headphone Playback Volume then.

> >and it should probably be two different shared controls for mic1 and mic2.
> 
> I'll try...
> 
> >> "Capture Source"
> > This one is the ADC Gain. Please name it that way.
> 
> Unfortunately, the strings have meaning to asoc and alsa-lib and you can't go 
> renaming them like that without breaking things. The names were carefully 
> chosen to make it actually work properly without having to patch alsa-lib and 
> parts of ASoC core (which I did before and have since reverted).
> 
> In this case, there's a special case in upstream alsa-lib:
> 
> alsa-lib-1.0.28:
> ./src/mixer/simple_none.c:	if (strcmp(name, "Capture Source") == 0) {
> ...
> 
> I'm not totally against naming them like you suggested - but know that you 
> are requiring changes in alsa-lib as well then - or presumably breaking the 
> user's workflow.
> 
> For example the (upstream) "Capture Source" special case in alsa-lib adds 
> radio-button-like things to the respective elements. 
> You can switch to one of the inputs by pressing Space while its gain element 
> is selected.
> In the mic case, it's the mic preamplier gain that you press Space on - if it's 
> indeed shown as a Capture control...

No, you're totally right, I just entirely missed that ControlNames
files you pointed me to.

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: Re: [PATCH v8 2/2] ASoc: sun4i-codec: Add FM, Line and Mic inputs

[Danny Milosavljevic]

Hi Maxime,
Hi Mark,

On Wed, 6 Jan 2016 23:09:08 +0100
Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:

> I find the sun4i_*_a10 and sun4i highly redundant. If there the same
> define for sun7i, then you know that it's not meant to be used for the
> A20, and that's it.
> 
> My point is also that it will just be an ever growing list of suffixes
> when we will support more SoCs, for example for symbols that might be
> in the A10, not the A20, but the A31.

Okay. I will remove the "_a10" and "_A10" suffixes for v9. 
There are not many places it's used anyway.

> > > Isn't it possible to expose this as two (shared) muxes with different
> > > names to make it clear what will go to the left ADC and what will go
> > > to the right?

That is possible and is working correctly so far. It will probably be in v9.

> Yeah, the case where two widgets share the same bits is handled iirc
> but sharing the controls.

Yeah. The struct kcontrol_new address (of a dapm control) is compared in order 
to find out whether to share the control or not. 

> Judging from ControlNames.txt, the Power Amplifier Volume should
> probably called Headphone Playback Volume then.

Yeah. I'll rename it to "Headphone Playback Volume" for v9.

> > >and it should probably be two different shared controls for mic1 and mic2

Once a control is shared there's just one control to the user.

I now have Mic Playback Volume shared between Left Mixer and Right Mixer 
(Note: was not in v8).
So far so good. (the name will be neither "Left Mixer Mic Playback 
Volume" nor "Right Mixer Mic Playback Volume" but rather "Mic Playback 
Volume" - because of the sharing)

But I can't get having a control for Mic2 Playback Volume that does the same 
as Mic1 Playback Volume (and updates the same register bitrange) to work 
[yet?] instead.

Sharing uses the same control for two different mixers, muxes etc.

The other direction (making two controls which both do the same and update 
each other's value) doesn't seem to be easily available. 
Should I write a _put handler that does it manually?

(Or should that be handled by alsa-lib mixer modules instead?)

Or, come to think of it, did you mean something else?

And something related but different:

Is it possible to merge the "Left Mixer" control and respective 
"Right Mixer" control into one selem in alsamixer?

Because alsamixer actually has support for one-sided muting.

For example:

Left Mixer Left FM Playback Switch and Right Mixer Right FM Playback Switch and 
FM Playback Volume could be one selem "FM Playback". Muting the left mixer left 
fm playback would be done by pressing "," and the right mixer right fm playback 
would be done by pressing ".".

Right now they are 3 different selem for that. So it would help reduce a lot of 
clutter.

(Right Mixer Left DAC Playback Switch - not a typo - would be the odd duck 
 and still be an extra selem)

For Mic there's:
- Left Mixer Mic1 Playback Switch
- Left Mixer Mic2 Playback Switch
- Right Mixer Mic1 Playback Switch
- Right Mixer Mic2 Playback Switch
- Mic Playback Volume

If possible, it could also then be collected together as:
- Mic1 Playback:
    Left Mixer Mic1 Playback Switch, 
    Right Mixer Mic1 Playback Switch, 
    Mic Playback Volume
- Mic2 Playback:
    Left Mixer Mic2 Playback Switch, 
    Right Mixer Mic2 Playback Switch, 
    Mic Playback Volume [note: there it is again]

Is it possible?

Regards,
    Danny

Re: Re: [PATCH v8 2/2] ASoc: sun4i-codec: Add FM, Line and Mic inputs

[Danny Milosavljevic]

Hi,

does anyone know the answer to the questions below?

> The other direction (making two controls which both do the same and update 
> each other's value) doesn't seem to be easily available. 

> Should I write a _put handler that does it manually?

> (Or should that be handled by alsa-lib mixer modules instead?)

> Is it possible to merge the "Left Mixer" control and respective 
> "Right Mixer" control into one selem in alsamixer?
> 
> Because alsamixer actually has support for one-sided muting.

If that's unknown, I can post v9 without doing the grouping-together of mixer controls and we can use it like that - I don't want the patch to stall indefinitely on mere usability issues...

Regards,
    Danny

Re: Re: [PATCH v8 2/2] ASoc: sun4i-codec: Add FM, Line and Mic inputs

[Code Kipper]

Hi Danny,
I've got some patches to go which covers pretty much everything that
we can manage with the sun4i-codec. I'm only lacking testing of
certain bits as I don't have capable hardware(i.e. FM capture). I've
ignored the phone out part of the A20 as I couldn't find the pins.
I'll try to upload them to my github this weekend. You should have
authorship on some of these so when I send the link feel free to
claim.
BR,
CK

On 12 March 2016 at 08:52, Danny Milosavljevic <dannym-bxPqe3T81XXwRsdMLXbzog@public.gmane.org> wrote:
> Hi,
>
> does anyone know the answer to the questions below?
>
>> The other direction (making two controls which both do the same and update
>> each other's value) doesn't seem to be easily available.
>
>> Should I write a _put handler that does it manually?
>
>> (Or should that be handled by alsa-lib mixer modules instead?)
>
>> Is it possible to merge the "Left Mixer" control and respective
>> "Right Mixer" control into one selem in alsamixer?
>>
>> Because alsamixer actually has support for one-sided muting.
>
> If that's unknown, I can post v9 without doing the grouping-together of mixer controls and we can use it like that - I don't want the patch to stall indefinitely on mere usability issues...
>
> Regards,
>     Danny
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
> For more options, visit https://groups.google.com/d/optout.

Re: Re: [PATCH v8 2/2] ASoc: sun4i-codec: Add FM, Line and Mic inputs

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1143 bytes --]

On Sat, Mar 12, 2016 at 08:52:22AM +0100, Danny Milosavljevic wrote:
> Hi,
> 
> does anyone know the answer to the questions below?
> 
> > The other direction (making two controls which both do the same and update 
> > each other's value) doesn't seem to be easily available. 
> 
> > Should I write a _put handler that does it manually?
> 
> > (Or should that be handled by alsa-lib mixer modules instead?)
> 
> > Is it possible to merge the "Left Mixer" control and respective 
> > "Right Mixer" control into one selem in alsamixer?
> > 
> > Because alsamixer actually has support for one-sided muting.

Mark, do you know how to do it?

IIRC, in order to have shared controls, you just needed to share the
controls structure.

> If that's unknown, I can post v9 without doing the grouping-together
> of mixer controls and we can use it like that - I don't want the
> patch to stall indefinitely on mere usability issues...

Unfortunately, that is exposed to the user-space. Once it's in, you
cannot change it / fix it anymore.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: Re: [PATCH v8 2/2] ASoc: sun4i-codec: Add FM, Line and Mic inputs

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 794 bytes --]

On Mon, Mar 14, 2016 at 11:49:39AM +0100, Maxime Ripard wrote:
> On Sat, Mar 12, 2016 at 08:52:22AM +0100, Danny Milosavljevic wrote:

> > > The other direction (making two controls which both do the same and update 
> > > each other's value) doesn't seem to be easily available. 

> > > Should I write a _put handler that does it manually?

> > > (Or should that be handled by alsa-lib mixer modules instead?)

> > > Is it possible to merge the "Left Mixer" control and respective 
> > > "Right Mixer" control into one selem in alsamixer?

> > > Because alsamixer actually has support for one-sided muting.

> Mark, do you know how to do it?

> IIRC, in order to have shared controls, you just needed to share the
> controls structure.

I'm having a really hard time telling what "it" is here.

Re: Re: [PATCH v8 2/2] ASoc: sun4i-codec: Add FM, Line and Mic inputs

[Danny Milosavljevic]

Hi Mark,

my question is whether it's possible to group together the left and right channel into one selem, and also if it's possible to reuse volumes then. For example:

For sun4i-codec Mic there's right now:
- Left Mixer Mic1 Playback Switch
- Left Mixer Mic2 Playback Switch
- Right Mixer Mic1 Playback Switch
- Right Mixer Mic2 Playback Switch
- Mic Playback Volume

If possible, it could also then be collected together as:
- Mic1 Playback:
    Left Mixer Mic1 Playback Switch, 
    Right Mixer Mic1 Playback Switch, 
    Mic Playback Volume
- Mic2 Playback:
    Left Mixer Mic2 Playback Switch, 
    Right Mixer Mic2 Playback Switch, 
    Mic Playback Volume [note: there it is again]

... where each outer group (f.e. "- Mic1 Playback:") is a selem (slider with two mutes in alsamixer).

alsamixer has support for seperate left and right channel muting so it would cut down a lot on the number of visible controls.

But as you can see, Mic Playback Volume is also shared.

Is it possible to group them together like this? How?

Right now, while everything technically works, it's overwhelming to use alsamixer with sun4i-codec as a user - you have to *scroll* for quite a while.

Additionally, while I'm at it: 

There's one capture volume right in front of the ADC. Whatever it is you capture, this tells you the gain for the ADC. But for some inputs, there are additional preamps. Right now in the v8 patch we don't distinguish between preamps and ADC gains (because I don't think it's possible), although the user has to kinda distinguish them in his head because for inputs which have preamps only the preamp and the adc gain *together* specify the total gain in the end.

E.g. in the hardware it's

  Mic In -> | Mic Preamp | -> | ADC Gain | -> | ADC |

while in alsamixer it's:

  [Capture Controls:]

  | Mic Preamp |  | Capture Volume | | xyz Preamp | | abc Preamp | | qrs Preamp |
                    ^--- ADC Gain

Is it possible to somehow distinguish those in alsamixer? Do we want to?

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

Re: Re: [PATCH v8 2/2] ASoc: sun4i-codec: Add FM, Line and Mic inputs

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1130 bytes --]

On Sat, Mar 19, 2016 at 05:13:36PM +0100, Danny Milosavljevic wrote:
> Hi Mark,

> my question is whether it's possible to group together the left and right channel into one selem, and also if it's possible to reuse volumes then. For example:

Please fix your mail client to word wrap within paragraphs at something
substantially less than 80 columns.  Doing this makes your messages much
easier to read and reply to.  This is particularly important for large
walls of text like this.

> Is it possible to group them together like this? How?

This is something you'd need to change in alsamixer and the control
naming standards.  None of this stuff is really supposed to be directly
edited by normal users, they should mostly be using a UCM configuration,
so at the minute this is essentially bikeshedding.

> Is it possible to somehow distinguish those in alsamixer? Do we want to?

I think you're looking for something that exports routing to userspace.
Doing that would also help with reducing the faff with naming rules.
Currently nobody is working on that but I think people would be very
happy if someone were to work on it.

Re: Re: [PATCH v8 2/2] ASoc: sun4i-codec: Add FM, Line and Mic inputs

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2757 bytes --]

On Sat, Mar 19, 2016 at 05:13:36PM +0100, Danny Milosavljevic wrote:
> Hi Mark,
> 
> my question is whether it's possible to group together the left and
> right channel into one selem, and also if it's possible to reuse
> volumes then. For example:
> 
> For sun4i-codec Mic there's right now:
> - Left Mixer Mic1 Playback Switch
> - Left Mixer Mic2 Playback Switch
> - Right Mixer Mic1 Playback Switch
> - Right Mixer Mic2 Playback Switch
> - Mic Playback Volume
> 
> If possible, it could also then be collected together as:
> - Mic1 Playback:
>     Left Mixer Mic1 Playback Switch, 
>     Right Mixer Mic1 Playback Switch, 
>     Mic Playback Volume
> - Mic2 Playback:
>     Left Mixer Mic2 Playback Switch, 
>     Right Mixer Mic2 Playback Switch, 
>     Mic Playback Volume [note: there it is again]
> 
> ... where each outer group (f.e. "- Mic1 Playback:") is a selem
> (slider with two mutes in alsamixer).
> 
> alsamixer has support for seperate left and right channel muting so
> it would cut down a lot on the number of visible controls.
> 
> But as you can see, Mic Playback Volume is also shared.
> 
> Is it possible to group them together like this? How?
> 
> Right now, while everything technically works, it's overwhelming to
> use alsamixer with sun4i-codec as a user - you have to *scroll* for
> quite a while.
> 
> Additionally, while I'm at it: 
> 
> There's one capture volume right in front of the ADC. Whatever it is
> you capture, this tells you the gain for the ADC. But for some
> inputs, there are additional preamps. Right now in the v8 patch we
> don't distinguish between preamps and ADC gains (because I don't
> think it's possible), although the user has to kinda distinguish
> them in his head because for inputs which have preamps only the
> preamp and the adc gain *together* specify the total gain in the
> end.
> 
> E.g. in the hardware it's
> 
>   Mic In -> | Mic Preamp | -> | ADC Gain | -> | ADC |
> 
> while in alsamixer it's:
> 
>   [Capture Controls:]
> 
>   | Mic Preamp |  | Capture Volume | | xyz Preamp | | abc Preamp | | qrs Preamp |
>                     ^--- ADC Gain
> 
> Is it possible to somehow distinguish those in alsamixer? Do we want
> to?


As I recall it, we were mostly discussing the how to deal with the ADC
muxer that muxes both left and right at the same time (so shared
controls), but with different source on each channel.

For example, some valid values are:
4: left channel Mic 1, right channel mic 2
5: left and right channel are Mic1 and Mic2 mixed

As I understand it, you can have shared controls by sharing the
kcontrols structure, but then, you can't different labels.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH v8 2/2] ASoc: sun4i-codec: Add FM, Line and Mic inputs

[Danny Milosavljevic]

Hi Maxime,

On Mon, 21 Mar 2016 18:54:36 +0100
Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:

> As I recall it, we were mostly discussing the how to deal with the ADC
> muxer that muxes both left and right at the same time (so shared
> controls), but with different source on each channel.

Hmm. Yeah, among other things. 

I've used an enum control for that and the routes just enable the 
right thing depending on the selected value (either "Mic1+Mic2" or 
"Mic1" or "Mic2" or etcetc).

I think this part works fine in v8 - and is as nice as we can 
reasonably do it.

-----

Also, for some outputs, there are multiple channels, multiple muters, 
but just one playback volume (i.e. the volume is mono but the muters are 
stereo AND possibly crossed over, see below).

It would have been possible to retain which channel is which in 
the Mic Playback "volumes" - but not necessary. 
Multiple muters exist there so it would just have been for consistency.

So let's just not retain that.

> As I understand it, you can have shared controls by sharing the
> kcontrols structure, but then, you can't different labels.

Yes. Because that's the case there's no upside in sharing the 
Mic Playback Volume. Therefore, I've moved it back to the common controls 
and made it just one control. At that point the user has to use UCM or 
just notice that Mic Playback Volume changes the playback volume on all of:
- Left Mixer Mic1 Playback
- Left Mixer Mic2 Playback
- Right Mixer Mic1 Playback
- Right Mixer Mic2 Playback
(those four don't have sliders but physically output something if 
 individually unmuted).

(The downside of sharing the controls would be that the sharing 
 detection works by comparing the struct snd_kcontrol_new instance addresses. 
 Good luck in getting that to work in two struct snd_kcontrol_new[]. 
 I did it before by merging the arrays and index gymnastics but I'd rather 
 not do it at all now. Note to self: there's a way to dynamically allocate 
 alsa controls)

----

Also, there's ADC Capture Volume: that one actually affects everything 
that is captured, whatever the source. Now, the user has to know this. For 
him, it looks as if Mic1 had its own capture volume ("Mic1 Capture Volume") 
but that's actually the mic1 preamplifier gain which will be effectively 
multiplied by the ADC Capture Volume in the data he gets.

In the long term I think it would be nice if userspace could see the routes.

----

Then, on my hardware there's destructive interference when I enable the DAC->PA and 
the Mixer->PA and also enable the DAC->Mixer, to get:

- DAC->PA
- DAC->Mixer->PA

Currently, the user is not prevented from doing this, however it will result 
in no perceptible PA sound when he sends something to the DAC.

Is that fine?

----

If all these points are fine like that then let's do it like that.

I'll send a v9 patch along these lines.

Thanks,
   Danny

Re: Re: [PATCH v8 2/2] ASoc: sun4i-codec: Add FM, Line and Mic inputs

[Danny Milosavljevic]

Hi Maxime,

> IIRC, in order to have shared controls, you just needed to share the
> controls structure.

Yeah. I did this and it actually works in a way, but in doesn't do all that I want.

There are two different kinds of sharing I mean:

The one kind with the controls structure causes the control to rename itself
to not state its mixer but there's still just one slider in the end. 
So instead of "Left Mixer Left XXX" it will just say "Left XXX" if you put it 
inside Left Mixer and Right Mixer and share the entry - for this hardware, 
that's of doubtful use.

But what I would like to have is that the same Mic Playback Volume (the very 
same bits) be used for different sliders
(1) Mic1 Left Channel Playback Volume
(2) Mic1 Right Channel Playback Volume
(3) Mic2 Left Channel Playback Volume (!!)
(4) Mic2 Right Channel Playback Volume

Additionally, Mic1 Playback and Mic2 Playback have two mutes each in the 
hardware (all separately switchable).

Now if we could make the alsamixer selems look like the following, 
that would be nice:

Mic1_Playback     Mic2_Playback

  MM                  MM
  ii                  ii
  cc                  cc

  PP                  PP
  ll                  ll
  aa                  aa
  yy                  yy
  bb                  bb
  aa                  aa
  cc                  cc
  kk                  kk

  VV                  VV
  oo                  oo
  ll                  ll
  uu                  uu
  mm                  mm
  ee                  ee

 [Mm]                [Nn]
   ^                  ^^-- so are those, separately
   +-- this one is a different muter :-)

Where the text is the same, it's supposed to signify the same bits 
in the hardware.

Right now in v8 it's (and that's going to take a lot of horizontal space, 
sorry - I know E-Mails like this aren't so nice to read):

Mic_Playback_Volume Left_Mic1_Playback_Switch Right_Mic1_Playback_Switch Left_Mic2_Playback_Switch Right_Mic2_Playback_Switch

  MM                  
  ii                  
  cc                  

  PP                  
  ll                  
  aa                  
  yy                  
  bb                  
  aa                  
  cc                  
  kk                  

  VV                  
  oo                  
  ll                  
  uu                  
  mm                  
  ee                  

                    [MM]                      [NN]                       [OO]                      [PP]
                      ^- WTF                   ^- WTF                      ^- WTF                   ^- WTF

(Actually, it's worse - believe it or not, that's how the simplified explanation looks)

> Unfortunately, that is exposed to the user-space. Once it's in, you
> cannot change it / fix it anymore.

Hmm. Right. Let's get it to work nicely then.

Re: Re: [PATCH v8 2/2] ASoc: sun4i-codec: Add FM, Line and Mic inputs

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 3131 bytes --]

On Sat, Mar 19, 2016 at 05:51:24PM +0100, Danny Milosavljevic wrote:
> Hi Maxime,
> 
> > IIRC, in order to have shared controls, you just needed to share the
> > controls structure.
> 
> Yeah. I did this and it actually works in a way, but in doesn't do all that I want.
> 
> There are two different kinds of sharing I mean:
> 
> The one kind with the controls structure causes the control to rename itself
> to not state its mixer but there's still just one slider in the end. 
> So instead of "Left Mixer Left XXX" it will just say "Left XXX" if you put it 
> inside Left Mixer and Right Mixer and share the entry - for this hardware, 
> that's of doubtful use.
> 
> But what I would like to have is that the same Mic Playback Volume (the very 
> same bits) be used for different sliders
> (1) Mic1 Left Channel Playback Volume
> (2) Mic1 Right Channel Playback Volume
> (3) Mic2 Left Channel Playback Volume (!!)
> (4) Mic2 Right Channel Playback Volume
> 
> Additionally, Mic1 Playback and Mic2 Playback have two mutes each in the 
> hardware (all separately switchable).
> 
> Now if we could make the alsamixer selems look like the following, 
> that would be nice:
> 
> Mic1_Playback     Mic2_Playback
> 
>   MM                  MM
>   ii                  ii
>   cc                  cc
> 
>   PP                  PP
>   ll                  ll
>   aa                  aa
>   yy                  yy
>   bb                  bb
>   aa                  aa
>   cc                  cc
>   kk                  kk
> 
>   VV                  VV
>   oo                  oo
>   ll                  ll
>   uu                  uu
>   mm                  mm
>   ee                  ee
> 
>  [Mm]                [Nn]
>    ^                  ^^-- so are those, separately
>    +-- this one is a different muter :-)
> 
> Where the text is the same, it's supposed to signify the same bits 
> in the hardware.
> 
> Right now in v8 it's (and that's going to take a lot of horizontal space, 
> sorry - I know E-Mails like this aren't so nice to read):
> 
> Mic_Playback_Volume Left_Mic1_Playback_Switch Right_Mic1_Playback_Switch Left_Mic2_Playback_Switch Right_Mic2_Playback_Switch
> 
>   MM                  
>   ii                  
>   cc                  
> 
>   PP                  
>   ll                  
>   aa                  
>   yy                  
>   bb                  
>   aa                  
>   cc                  
>   kk                  
> 
>   VV                  
>   oo                  
>   ll                  
>   uu                  
>   mm                  
>   ee                  
> 
>                     [MM]                      [NN]                       [OO]                      [PP]
>                       ^- WTF                   ^- WTF                      ^- WTF                   ^- WTF
> 
> (Actually, it's worse - believe it or not, that's how the simplified explanation looks)

I don't think you should focus too much on alsamixer, there's other
tools to modify the configuration.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [linux-sunxi] Re: [PATCH v8 2/2] ASoc: sun4i-codec: Add FM, Line and Mic inputs

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 314 bytes --]

On Mon, Mar 21, 2016 at 07:06:05PM +0100, Maxime Ripard wrote:

> I don't think you should focus too much on alsamixer, there's other
> tools to modify the configuration.

The expectation is that most users will use something like UCM and only
system integrators will see the full control set in normal operation.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [linux-sunxi] Re: [PATCH v8 2/2] ASoc: sun4i-codec: Add FM, Line and Mic inputs

[Stefan Monnier]

>> I don't think you should focus too much on alsamixer, there's other
>> tools to modify the configuration.
> The expectation is that most users will use something like UCM and only
> system integrators will see the full control set in normal operation.

This presumes the "system integrator" (which could very well be the end
user in many cases) somehow knows what all the controls do/mean.

Anyway, it seems this is an ALSA problem rather than a sunxi problem.


        Stefan "who wishes ALSA could describe the meaning of all those
                controls, maybe as a graphical diagram, or as some kind
                of (hopefully simple) program"

[PATCH v8 0/2] ASoc: sun4i-codec: Add FM, Line and Mic inputs

[Danny Milosavljevic]

Hi,

this is the eighth version of the patch set that adds inputs to sun4i-codec.

Changes compared to v7 are:
 - fix the routes for line and mic capturing.

The patches are on top of the branch "sunxi-next" in 
<git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git>.

Regards,
   Danny
---
Danny Milosavljevic (2):
  sun4i-codec: Add FM, Line and Mic inputs
  sun4i-codec: make it possible to use different codec_widgets for A10 and A20.

 b/sound/soc/sunxi/sun4i-codec.c |  230 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 215 insertions(+), 15 deletions(-)