ASoC: codecs: jz4725b: Various improvements and fixes

Commit Message

Siarhei Volkau Oct. 8, 2022, 6:16 p.m. UTC

The patch fixes:
- incorrectly represented dB values in alsamixer, et al.
- Line In path stays powered off during capturing or
  bypass to mixer.

The patch improves:
- Exposes all mixer inputs (both Mics, LineIn and DAC) with their
  gain controls.
- Exposes output stage (post mixer) gain control and makes it new
  Master playback gain, DAC gain was the previous master.
  However, no Master mute now.

Known issues:
- Bypass path enablement isn't applied immediately, for make
  things going bit clock needs to be triggered for a bit,
  e.g. by aplay dummy.wav
  It might be a hardware bug, since the bit clock isn't
  declared as required for codec operation.

Tested on:
- Ritmix RZX-27 (jz4725b).
- Ritmix RZX-50 (jz4755).

Tested-by: Siarhei Volkau <[e-mail]>
Signed-off-by: Siarhei Volkau <[e-mail]>
---
 sound/soc/codecs/jz4725b.c | 81 ++++++++++++++++++++++++++++++++------
 1 file changed, 70 insertions(+), 11 deletions(-)

Comments

Mark Brown Oct. 10, 2022, 11:19 a.m. UTC | #1

On Sat, Oct 08, 2022 at 09:16:55PM +0300, Siarhei Volkau wrote:
> The patch fixes:
> - incorrectly represented dB values in alsamixer, et al.
> - Line In path stays powered off during capturing or
>   bypass to mixer.
> 
> The patch improves:
> - Exposes all mixer inputs (both Mics, LineIn and DAC) with their
>   gain controls.
> - Exposes output stage (post mixer) gain control and makes it new
>   Master playback gain, DAC gain was the previous master.
>   However, no Master mute now.
> 
> Known issues:
> - Bypass path enablement isn't applied immediately, for make
>   things going bit clock needs to be triggered for a bit,
>   e.g. by aplay dummy.wav
>   It might be a hardware bug, since the bit clock isn't
>   declared as required for codec operation.

As covered in submitting-patches.rst this should really be split up into
multiple patches, with one change per patch.  This is especially the
case here since you have a mix of fixes and new features which should be
applied differently.

> -	SOC_DOUBLE_R_TLV("Master Capture Volume",
> +	SOC_DOUBLE_TLV("Master Capture Volume",
> +		       JZ4725B_CODEC_REG_CGR10,
> +		       REG_CGR10_GIL_OFFSET,
> +		       REG_CGR10_GIR_OFFSET,
> +		       0xf, 0, jz4725b_adc_tlv),
> +	SOC_DOUBLE_R_TLV("Mixer Line In Bypass Playback Volume",
>  			 JZ4725B_CODEC_REG_CGR3,

This doesn't appear to correspond to what your patch description said
and will presumably cause problems for any existing configurations...

Siarhei Volkau Oct. 10, 2022, 3:20 p.m. UTC | #2

пн, 10 окт. 2022 г. в 14:19, Mark Brown <[e-mail]>:
> As covered in submitting-patches.rst this should really be split up into
> multiple patches, with one change per patch.  This is especially the
> case here since you have a mix of fixes and new features which should be
> applied differently.

Got it, will rework.

> > -     SOC_DOUBLE_R_TLV("Master Capture Volume",
> > +     SOC_DOUBLE_TLV("Master Capture Volume",
> > +                    JZ4725B_CODEC_REG_CGR10,
> > +                    REG_CGR10_GIL_OFFSET,
> > +                    REG_CGR10_GIR_OFFSET,
> > +                    0xf, 0, jz4725b_adc_tlv),
> > +     SOC_DOUBLE_R_TLV("Mixer Line In Bypass Playback Volume",
> >                        JZ4725B_CODEC_REG_CGR3,
>
> This doesn't appear to correspond to what your patch description said

I forgot to mention that in the description, thanks.

> and will presumably cause problems for any existing configurations...

I'm curious why this didn't cause problems earlier, as the wrong
control was used
for the Master Capture Volume.

Mark Brown Oct. 10, 2022, 3:53 p.m. UTC | #3

On Mon, Oct 10, 2022 at 06:20:24PM +0300, Siarhei Volkau wrote:
> пн, 10 окт. 2022 г. в 14:19, Mark Brown <[e-mail]>:

> > and will presumably cause problems for any existing configurations...

> I'm curious why this didn't cause problems earlier, as the wrong
> control was used
> for the Master Capture Volume.

The issue is that if someone is relying on the current behaviour and the
control starts doing something completely different they might be
surprised, it at least needs a separate change.

Siarhei Volkau Oct. 10, 2022, 4:26 p.m. UTC | #4

пн, 10 окт. 2022 г. в 18:53, Mark Brown <[e-mail]>:
> The issue is that if someone is relying on the current behaviour and the
> control starts doing something completely different they might be
> surprised, it at least needs a separate change.

At the moment there's only one consumer for the codec in the mainline kernel
Its RetroMini RS-90 and it doesn't seem to have capture capabilities.

Patch

diff --git a/sound/soc/codecs/jz4725b.c b/sound/soc/codecs/jz4725b.c
index 5201a8f6d..29f188800 100644
--- a/sound/soc/codecs/jz4725b.c
+++ b/sound/soc/codecs/jz4725b.c
@@ -136,28 +136,80 @@  enum {
 #define REG_CGR3_GO1L_OFFSET		0
 #define REG_CGR3_GO1L_MASK		(0x1f << REG_CGR3_GO1L_OFFSET)
 
+#define REG_CGR4_GO2R_OFFSET		0
+#define REG_CGR4_GO2R_MASK		(0x1f << REG_CGR4_GO2R_OFFSET)
+
+#define REG_CGR5_GO2L_OFFSET		0
+#define REG_CGR5_GO2L_MASK		(0x1f << REG_CGR5_GO2L_OFFSET)
+
+#define REG_CGR6_GO3R_OFFSET		0
+#define REG_CGR6_GO3R_MASK		(0x1f << REG_CGR6_GO3R_OFFSET)
+
+#define REG_CGR7_GO3L_OFFSET		0
+#define REG_CGR7_GO3L_MASK		(0x1f << REG_CGR7_GO3L_OFFSET)
+
+#define REG_CGR8_GOR_OFFSET		0
+#define REG_CGR8_GOR_MASK		(0x1f << REG_CGR8_GOR_OFFSET)
+
+#define REG_CGR9_GOL_OFFSET		0
+#define REG_CGR9_GOL_MASK		(0x1f << REG_CGR9_GOL_OFFSET)
+
+#define REG_CGR10_GIL_OFFSET		0
+#define REG_CGR10_GIR_OFFSET		4
+
 struct jz_icdc {
 	struct regmap *regmap;
 	void __iomem *base;
 	struct clk *clk;
 };
 
-static const SNDRV_CTL_TLVD_DECLARE_DB_LINEAR(jz4725b_dac_tlv, -2250, 0);
-static const SNDRV_CTL_TLVD_DECLARE_DB_LINEAR(jz4725b_line_tlv, -1500, 600);
+static const SNDRV_CTL_TLVD_DECLARE_DB_SCALE(jz4725b_dac_tlv, -2250, 150, 0);
+static const SNDRV_CTL_TLVD_DECLARE_DB_SCALE(jz4725b_adc_tlv,     0, 150, 0);
+static const SNDRV_CTL_TLVD_DECLARE_DB_RANGE(jz4725b_mix_tlv,
+	 0, 11, TLV_DB_SCALE_ITEM(-2250,   0, 0),
+	12, 31, TLV_DB_SCALE_ITEM(-2250, 150, 0),
+);
+
+static const SNDRV_CTL_TLVD_DECLARE_DB_RANGE(jz4725b_out_tlv,
+	 0, 11, TLV_DB_SCALE_ITEM(-3350, 200, 0),
+	12, 23, TLV_DB_SCALE_ITEM(-1050, 100, 0),
+	24, 31, TLV_DB_SCALE_ITEM(  100,  50, 0),
+);
 
 static const struct snd_kcontrol_new jz4725b_codec_controls[] = {
-	SOC_DOUBLE_TLV("Master Playback Volume",
+	SOC_DOUBLE_TLV("DAC Playback Volume",
 		       JZ4725B_CODEC_REG_CGR1,
 		       REG_CGR1_GODL_OFFSET,
 		       REG_CGR1_GODR_OFFSET,
 		       0xf, 1, jz4725b_dac_tlv),
-	SOC_DOUBLE_R_TLV("Master Capture Volume",
+	SOC_DOUBLE_TLV("Master Capture Volume",
+		       JZ4725B_CODEC_REG_CGR10,
+		       REG_CGR10_GIL_OFFSET,
+		       REG_CGR10_GIR_OFFSET,
+		       0xf, 0, jz4725b_adc_tlv),
+	SOC_DOUBLE_R_TLV("Mixer Line In Bypass Playback Volume",
 			 JZ4725B_CODEC_REG_CGR3,
 			 JZ4725B_CODEC_REG_CGR2,
 			 REG_CGR2_GO1R_OFFSET,
-			 0x1f, 1, jz4725b_line_tlv),
-
-	SOC_SINGLE("Master Playback Switch", JZ4725B_CODEC_REG_CR1,
+			 0x1f, 1, jz4725b_mix_tlv),
+	SOC_DOUBLE_R_TLV("Mixer Mic 1 Bypass Playback Volume",
+			 JZ4725B_CODEC_REG_CGR5,
+			 JZ4725B_CODEC_REG_CGR4,
+			 REG_CGR4_GO2R_OFFSET,
+			 0x1f, 1, jz4725b_mix_tlv),
+	SOC_DOUBLE_R_TLV("Mixer Mic 2 Bypass Playback Volume",
+			 JZ4725B_CODEC_REG_CGR7,
+			 JZ4725B_CODEC_REG_CGR6,
+			 REG_CGR6_GO3R_OFFSET,
+			 0x1f, 1, jz4725b_mix_tlv),
+
+	SOC_DOUBLE_R_TLV("Master Playback Volume",
+			 JZ4725B_CODEC_REG_CGR9,
+			 JZ4725B_CODEC_REG_CGR8,
+			 REG_CGR8_GOR_OFFSET,
+			 0x1f, 1, jz4725b_out_tlv),
+
+	SOC_SINGLE("DAC Playback Switch", JZ4725B_CODEC_REG_CR1,
 		   REG_CR1_DAC_MUTE_OFFSET, 1, 1),
 
 	SOC_SINGLE("Deemphasize Filter Playback Switch",
@@ -180,11 +232,15 @@  static SOC_VALUE_ENUM_SINGLE_DECL(jz4725b_codec_adc_src_enum,
 				  jz4725b_codec_adc_src_texts,
 				  jz4725b_codec_adc_src_values);
 static const struct snd_kcontrol_new jz4725b_codec_adc_src_ctrl =
-			SOC_DAPM_ENUM("Route", jz4725b_codec_adc_src_enum);
+	SOC_DAPM_ENUM("ADC Source Capture Route", jz4725b_codec_adc_src_enum);
 
 static const struct snd_kcontrol_new jz4725b_codec_mixer_controls[] = {
-	SOC_DAPM_SINGLE("Line In Bypass", JZ4725B_CODEC_REG_CR1,
+	SOC_DAPM_SINGLE("Line In Bypass Playback Switch", JZ4725B_CODEC_REG_CR1,
 			REG_CR1_BYPASS_OFFSET, 1, 0),
+	SOC_DAPM_SINGLE("Mic 1 Bypass Playback Switch", JZ4725B_CODEC_REG_CR3,
+			REG_CR3_SIDETONE1_OFFSET, 1, 0),
+	SOC_DAPM_SINGLE("Mic 2 Bypass Playback Switch", JZ4725B_CODEC_REG_CR3,
+			REG_CR3_SIDETONE2_OFFSET, 1, 0),
 };
 
 static int jz4725b_out_stage_enable(struct snd_soc_dapm_widget *w,
@@ -236,7 +292,8 @@  static const struct snd_soc_dapm_widget jz4725b_codec_dapm_widgets[] = {
 	SND_SOC_DAPM_MIXER("DAC to Mixer", JZ4725B_CODEC_REG_CR1,
 			   REG_CR1_DACSEL_OFFSET, 0, NULL, 0),
 
-	SND_SOC_DAPM_MIXER("Line In", SND_SOC_NOPM, 0, 0, NULL, 0),
+	SND_SOC_DAPM_MIXER("Line In", JZ4725B_CODEC_REG_PMR1,
+			   REG_PMR1_SB_LIN_OFFSET, 1, NULL, 0),
 	SND_SOC_DAPM_MIXER("HP Out", JZ4725B_CODEC_REG_CR1,
 			   REG_CR1_HP_DIS_OFFSET, 1, NULL, 0),
 
@@ -278,7 +335,9 @@  static const struct snd_soc_dapm_route jz4725b_codec_dapm_routes[] = {
 	{"Line In", NULL, "LLINEIN"},
 	{"Line In", NULL, "RLINEIN"},
 
-	{"Mixer", "Line In Bypass", "Line In"},
+	{"Mixer", "Mic 1 Bypass Playback Switch", "Mic 1"},
+	{"Mixer", "Mic 2 Bypass Playback Switch", "Mic 2"},
+	{"Mixer", "Line In Bypass Playback Switch", "Line In"},
 	{"DAC to Mixer", NULL, "DAC"},
 	{"Mixer", NULL, "DAC to Mixer"},