Re: [PATCH v2 2/2] ASoC: codec: lpass-rx-macro: add suppor for 2.5 codec version

From: Dmitry Baryshkov
Date: Fri Jun 07 2024 - 07:04:04 EST

On Thu, Jun 06, 2024 at 01:25:59PM +0100, srinivas.kandagatla@xxxxxxxxxx wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx>
>
> LPASS Codec v2.5 has significant changes in the rx register offsets.
> Due to this headset playback on SM8550, SM8650, x1e80100 and all SoCs
> after SM8450 have only Left working.
>
> This patch adjusts the registers to accomdate 2.5 changes. With this
> fixed now L and R are functional on Headset playback.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx>
> ---
> sound/soc/codecs/lpass-rx-macro.c | 565 ++++++++++++++++++++++--------
> 1 file changed, 410 insertions(+), 155 deletions(-)
>
> diff --git a/sound/soc/codecs/lpass-rx-macro.c b/sound/soc/codecs/lpass-rx-macro.c
> index f35187d69cac..bb8ede0e7076 100644
> --- a/sound/soc/codecs/lpass-rx-macro.c
> +++ b/sound/soc/codecs/lpass-rx-macro.c
> @@ -158,7 +158,7 @@
> #define CDC_RX_INTR_CTRL_LEVEL0 (0x03C0)
> #define CDC_RX_INTR_CTRL_BYPASS0 (0x03C8)
> #define CDC_RX_INTR_CTRL_SET0 (0x03D0)
> -#define CDC_RX_RXn_RX_PATH_CTL(n) (0x0400 + 0x80 * n)
> +#define CDC_RX_RXn_RX_PATH_CTL(rx, n) (0x0400 + rx->rxn_reg_offset * n)

It's probably a stride or pitch, not an offset.

> #define CDC_RX_RX0_RX_PATH_CTL (0x0400)
> #define CDC_RX_PATH_RESET_EN_MASK BIT(6)
> #define CDC_RX_PATH_CLK_EN_MASK BIT(5)
> @@ -166,45 +166,47 @@
> #define CDC_RX_PATH_PGA_MUTE_MASK BIT(4)
> #define CDC_RX_PATH_PGA_MUTE_ENABLE BIT(4)
> #define CDC_RX_PATH_PCM_RATE_MASK GENMASK(3, 0)
> -#define CDC_RX_RXn_RX_PATH_CFG0(n) (0x0404 + 0x80 * n)
> +#define CDC_RX_RXn_RX_PATH_CFG0(rx, n) (0x0404 + rx->rxn_reg_offset * n)
> #define CDC_RX_RXn_COMP_EN_MASK BIT(1)
> #define CDC_RX_RX0_RX_PATH_CFG0 (0x0404)
> #define CDC_RX_RXn_CLSH_EN_MASK BIT(6)
> #define CDC_RX_DLY_ZN_EN_MASK BIT(3)
> #define CDC_RX_DLY_ZN_ENABLE BIT(3)
> #define CDC_RX_RXn_HD2_EN_MASK BIT(2)
> -#define CDC_RX_RXn_RX_PATH_CFG1(n) (0x0408 + 0x80 * n)
> +#define CDC_RX_RXn_RX_PATH_CFG1(rx, n) (0x0408 + rx->rxn_reg_offset * n)
> #define CDC_RX_RXn_SIDETONE_EN_MASK BIT(4)
> #define CDC_RX_RX0_RX_PATH_CFG1 (0x0408)
> #define CDC_RX_RX0_HPH_L_EAR_SEL_MASK BIT(1)
> -#define CDC_RX_RXn_RX_PATH_CFG2(n) (0x040C + 0x80 * n)
> +#define CDC_RX_RXn_RX_PATH_CFG2(rx, n) (0x040C + rx->rxn_reg_offset * n)
> #define CDC_RX_RXn_HPF_CUT_FREQ_MASK GENMASK(1, 0)
> #define CDC_RX_RX0_RX_PATH_CFG2 (0x040C)
> -#define CDC_RX_RXn_RX_PATH_CFG3(n) (0x0410 + 0x80 * n)
> +#define CDC_RX_RXn_RX_PATH_CFG3(rx, n) (0x0410 + rx->rxn_reg_offset * n)
> #define CDC_RX_RX0_RX_PATH_CFG3 (0x0410)
> #define CDC_RX_DC_COEFF_SEL_MASK GENMASK(1, 0)
> #define CDC_RX_DC_COEFF_SEL_TWO 0x2
> -#define CDC_RX_RXn_RX_VOL_CTL(n) (0x0414 + 0x80 * n)
> +#define CDC_RX_RXn_RX_VOL_CTL(rx, n) (0x0414 + rx->rxn_reg_offset * n)
> #define CDC_RX_RX0_RX_VOL_CTL (0x0414)
> -#define CDC_RX_RXn_RX_PATH_MIX_CTL(n) (0x0418 + 0x80 * n)
> +#define CDC_RX_RXn_RX_PATH_MIX_CTL(rx, n) (0x0418 + rx->rxn_reg_offset * n)
> #define CDC_RX_RXn_MIX_PCM_RATE_MASK GENMASK(3, 0)
> #define CDC_RX_RXn_MIX_RESET_MASK BIT(6)
> #define CDC_RX_RXn_MIX_RESET BIT(6)
> #define CDC_RX_RXn_MIX_CLK_EN_MASK BIT(5)
> #define CDC_RX_RX0_RX_PATH_MIX_CTL (0x0418)
> #define CDC_RX_RX0_RX_PATH_MIX_CFG (0x041C)
> -#define CDC_RX_RXn_RX_VOL_MIX_CTL(n) (0x0420 + 0x80 * n)
> +#define CDC_RX_RXn_RX_VOL_MIX_CTL(rx, n) (0x0420 + rx->rxn_reg_offset * n)
> #define CDC_RX_RX0_RX_VOL_MIX_CTL (0x0420)
> #define CDC_RX_RX0_RX_PATH_SEC1 (0x0424)
> #define CDC_RX_RX0_RX_PATH_SEC2 (0x0428)
> #define CDC_RX_RX0_RX_PATH_SEC3 (0x042C)
> +#define CDC_RX_RXn_RX_PATH_SEC3(rx, n) (0x042c + rx->rxn_reg_offset * n)
> #define CDC_RX_RX0_RX_PATH_SEC4 (0x0430)
> #define CDC_RX_RX0_RX_PATH_SEC7 (0x0434)
> +#define CDC_RX_RXn_RX_PATH_SEC7(rx, n) (0x0434 + rx->rxn_reg_offset * n)
> #define CDC_RX_DSM_OUT_DELAY_SEL_MASK GENMASK(2, 0)
> #define CDC_RX_DSM_OUT_DELAY_TWO_SAMPLE 0x2
> #define CDC_RX_RX0_RX_PATH_MIX_SEC0 (0x0438)
> #define CDC_RX_RX0_RX_PATH_MIX_SEC1 (0x043C)
> -#define CDC_RX_RXn_RX_PATH_DSM_CTL(n) (0x0440 + 0x80 * n)
> +#define CDC_RX_RXn_RX_PATH_DSM_CTL(rx, n) (0x0440 + rx->rxn_reg_offset * n)
> #define CDC_RX_RXn_DSM_CLK_EN_MASK BIT(0)
> #define CDC_RX_RX0_RX_PATH_DSM_CTL (0x0440)
> #define CDC_RX_RX0_RX_PATH_DSM_DATA1 (0x0444)
> @@ -213,6 +215,7 @@
> #define CDC_RX_RX0_RX_PATH_DSM_DATA4 (0x0450)
> #define CDC_RX_RX0_RX_PATH_DSM_DATA5 (0x0454)
> #define CDC_RX_RX0_RX_PATH_DSM_DATA6 (0x0458)
> +/* RX offsets prior to 2.5 codec version */
> #define CDC_RX_RX1_RX_PATH_CTL (0x0480)
> #define CDC_RX_RX1_RX_PATH_CFG0 (0x0484)
> #define CDC_RX_RX1_RX_PATH_CFG1 (0x0488)
> @@ -259,10 +262,58 @@
> #define CDC_RX_RX2_RX_PATH_MIX_SEC0 (0x0544)
> #define CDC_RX_RX2_RX_PATH_MIX_SEC1 (0x0548)
> #define CDC_RX_RX2_RX_PATH_DSM_CTL (0x054C)
> +
> +/* LPASS CODEC version 2.5 rx reg offsets */

Please separate 2.5 support into a separate patch.

> +#define CDC_2_5_RX_RX1_RX_PATH_CTL (0x04c0)
> +#define CDC_2_5_RX_RX1_RX_PATH_CFG0 (0x04c4)
> +#define CDC_2_5_RX_RX1_RX_PATH_CFG1 (0x04c8)
> +#define CDC_2_5_RX_RX1_RX_PATH_CFG2 (0x04cC)
> +#define CDC_2_5_RX_RX1_RX_PATH_CFG3 (0x04d0)
> +#define CDC_2_5_RX_RX1_RX_VOL_CTL (0x04d4)
> +#define CDC_2_5_RX_RX1_RX_PATH_MIX_CTL (0x04d8)
> +#define CDC_2_5_RX_RX1_RX_PATH_MIX_CFG (0x04dC)
> +#define CDC_2_5_RX_RX1_RX_VOL_MIX_CTL (0x04e0)
> +#define CDC_2_5_RX_RX1_RX_PATH_SEC1 (0x04e4)
> +#define CDC_2_5_RX_RX1_RX_PATH_SEC2 (0x04e8)
> +#define CDC_2_5_RX_RX1_RX_PATH_SEC3 (0x04eC)
> +#define CDC_2_5_RX_RX1_RX_PATH_SEC4 (0x04f0)
> +#define CDC_2_5_RX_RX1_RX_PATH_SEC7 (0x04f4)
> +#define CDC_2_5_RX_RX1_RX_PATH_MIX_SEC0 (0x04f8)
> +#define CDC_2_5_RX_RX1_RX_PATH_MIX_SEC1 (0x04fC)
> +#define CDC_2_5_RX_RX1_RX_PATH_DSM_CTL (0x0500)
> +#define CDC_2_5_RX_RX1_RX_PATH_DSM_DATA1 (0x0504)
> +#define CDC_2_5_RX_RX1_RX_PATH_DSM_DATA2 (0x0508)
> +#define CDC_2_5_RX_RX1_RX_PATH_DSM_DATA3 (0x050C)
> +#define CDC_2_5_RX_RX1_RX_PATH_DSM_DATA4 (0x0510)
> +#define CDC_2_5_RX_RX1_RX_PATH_DSM_DATA5 (0x0514)
> +#define CDC_2_5_RX_RX1_RX_PATH_DSM_DATA6 (0x0518)
> +
> +#define CDC_2_5_RX_RX2_RX_PATH_CTL (0x0580)
> +#define CDC_2_5_RX_RX2_RX_PATH_CFG0 (0x0584)
> +#define CDC_2_5_RX_RX2_RX_PATH_CFG1 (0x0588)
> +#define CDC_2_5_RX_RX2_RX_PATH_CFG2 (0x058C)
> +#define CDC_2_5_RX_RX2_RX_PATH_CFG3 (0x0590)
> +#define CDC_2_5_RX_RX2_RX_VOL_CTL (0x0594)
> +#define CDC_2_5_RX_RX2_RX_PATH_MIX_CTL (0x0598)
> +#define CDC_2_5_RX_RX2_RX_PATH_MIX_CFG (0x059C)
> +#define CDC_2_5_RX_RX2_RX_VOL_MIX_CTL (0x05a0)
> +#define CDC_2_5_RX_RX2_RX_PATH_SEC0 (0x05a4)
> +#define CDC_2_5_RX_RX2_RX_PATH_SEC1 (0x05a8)
> +#define CDC_2_5_RX_RX2_RX_PATH_SEC2 (0x05aC)
> +#define CDC_2_5_RX_RX2_RX_PATH_SEC3 (0x05b0)
> +#define CDC_2_5_RX_RX2_RX_PATH_SEC4 (0x05b4)
> +#define CDC_2_5_RX_RX2_RX_PATH_SEC5 (0x05b8)
> +#define CDC_2_5_RX_RX2_RX_PATH_SEC6 (0x05bC)
> +#define CDC_2_5_RX_RX2_RX_PATH_SEC7 (0x05c0)
> +#define CDC_2_5_RX_RX2_RX_PATH_MIX_SEC0 (0x05c4)
> +#define CDC_2_5_RX_RX2_RX_PATH_MIX_SEC1 (0x05c8)
> +#define CDC_2_5_RX_RX2_RX_PATH_DSM_CTL (0x05cC)
> +
> #define CDC_RX_IDLE_DETECT_PATH_CTL (0x0780)
> #define CDC_RX_IDLE_DETECT_CFG0 (0x0784)
> #define CDC_RX_IDLE_DETECT_CFG1 (0x0788)
> #define CDC_RX_IDLE_DETECT_CFG2 (0x078C)
> +
> #define CDC_RX_IDLE_DETECT_CFG3 (0x0790)
> #define CDC_RX_COMPANDERn_CTL0(n) (0x0800 + 0x40 * n)
> #define CDC_RX_COMPANDERn_CLK_EN_MASK BIT(0)
> @@ -598,6 +649,8 @@ struct rx_macro {
> int rx_mclk_users;
> int clsh_users;
> int rx_mclk_cnt;
> + int codec_version;
> + int rxn_reg_offset;
> bool is_ear_mode_on;
> bool hph_pwr_mode;
> bool hph_hd2_mode;
> @@ -755,11 +808,15 @@ static SOC_ENUM_SINGLE_DECL(rx_int1_2_interp_enum, SND_SOC_NOPM, 0,
> rx_int1_2_interp_mux_text);
> static SOC_ENUM_SINGLE_DECL(rx_int2_2_interp_enum, SND_SOC_NOPM, 0,
> rx_int2_2_interp_mux_text);
> +

unrelated?

> static SOC_ENUM_SINGLE_DECL(rx_int0_dem_inp_enum, CDC_RX_RX0_RX_PATH_CFG1, 0,
> rx_int_dem_inp_mux_text);
> static SOC_ENUM_SINGLE_DECL(rx_int1_dem_inp_enum, CDC_RX_RX1_RX_PATH_CFG1, 0,
> rx_int_dem_inp_mux_text);
>
> +static SOC_ENUM_SINGLE_DECL(rx_2_5_int1_dem_inp_enum, CDC_2_5_RX_RX1_RX_PATH_CFG1, 0,
> + rx_int_dem_inp_mux_text);
> +
> static SOC_ENUM_SINGLE_DECL(rx_macro_rx0_enum, SND_SOC_NOPM, 0, rx_macro_mux_text);
> static SOC_ENUM_SINGLE_DECL(rx_macro_rx1_enum, SND_SOC_NOPM, 0, rx_macro_mux_text);
> static SOC_ENUM_SINGLE_DECL(rx_macro_rx2_enum, SND_SOC_NOPM, 0, rx_macro_mux_text);

[skipped all split default + add 2.5] chunks

> static int rx_macro_component_probe(struct snd_soc_component *component)
> {
> + struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
> struct rx_macro *rx = snd_soc_component_get_drvdata(component);
> + const struct snd_soc_dapm_widget *widgets;
> + const struct snd_kcontrol_new *controls;
> + unsigned int num_controls;
> + int ret, num_widgets;
>
> snd_soc_component_init_regmap(component, rx->regmap);
>
> - snd_soc_component_update_bits(component, CDC_RX_RX0_RX_PATH_SEC7,
> + snd_soc_component_update_bits(component, CDC_RX_RXn_RX_PATH_SEC7(rx, 0),
> CDC_RX_DSM_OUT_DELAY_SEL_MASK,
> CDC_RX_DSM_OUT_DELAY_TWO_SAMPLE);
> - snd_soc_component_update_bits(component, CDC_RX_RX1_RX_PATH_SEC7,
> + snd_soc_component_update_bits(component, CDC_RX_RXn_RX_PATH_SEC7(rx, 1),
> CDC_RX_DSM_OUT_DELAY_SEL_MASK,
> CDC_RX_DSM_OUT_DELAY_TWO_SAMPLE);
> - snd_soc_component_update_bits(component, CDC_RX_RX2_RX_PATH_SEC7,
> + snd_soc_component_update_bits(component, CDC_RX_RXn_RX_PATH_SEC7(rx, 2),
> CDC_RX_DSM_OUT_DELAY_SEL_MASK,
> CDC_RX_DSM_OUT_DELAY_TWO_SAMPLE);
> - snd_soc_component_update_bits(component, CDC_RX_RX0_RX_PATH_CFG3,
> + snd_soc_component_update_bits(component, CDC_RX_RXn_RX_PATH_CFG3(rx, 0),
> CDC_RX_DC_COEFF_SEL_MASK,
> CDC_RX_DC_COEFF_SEL_TWO);
> - snd_soc_component_update_bits(component, CDC_RX_RX1_RX_PATH_CFG3,
> + snd_soc_component_update_bits(component, CDC_RX_RXn_RX_PATH_CFG3(rx, 1),
> CDC_RX_DC_COEFF_SEL_MASK,
> CDC_RX_DC_COEFF_SEL_TWO);
> - snd_soc_component_update_bits(component, CDC_RX_RX2_RX_PATH_CFG3,
> + snd_soc_component_update_bits(component, CDC_RX_RXn_RX_PATH_CFG3(rx, 2),
> CDC_RX_DC_COEFF_SEL_MASK,
> CDC_RX_DC_COEFF_SEL_TWO);
>
> + if (rx->codec_version >= LPASS_CODEC_VERSION_2_5) {
> + controls = rx_macro_2_5_snd_controls;
> + num_controls = ARRAY_SIZE(rx_macro_2_5_snd_controls);
> + widgets = rx_macro_2_5_dapm_widgets;
> + num_widgets = ARRAY_SIZE(rx_macro_2_5_dapm_widgets);
> + } else {
> + controls = rx_macro_def_snd_controls;
> + num_controls = ARRAY_SIZE(rx_macro_def_snd_controls);
> + widgets = rx_macro_def_dapm_widgets;
> + num_widgets = ARRAY_SIZE(rx_macro_def_dapm_widgets);
> + }
> +
> rx->component = component;
>
> - return 0;
> + ret = snd_soc_add_component_controls(component, controls, num_controls);
> + if (ret)
> + return ret;
> +
> + return snd_soc_dapm_new_controls(dapm, widgets, num_widgets);
> }
>
> static int swclk_gate_enable(struct clk_hw *hw)
> @@ -3523,11 +3748,12 @@ static const struct snd_soc_component_driver rx_macro_component_drv = {
>
> static int rx_macro_probe(struct platform_device *pdev)
> {
> + struct reg_default *reg_defaults;
> struct device *dev = &pdev->dev;
> kernel_ulong_t flags;
> struct rx_macro *rx;
> void __iomem *base;
> - int ret;
> + int ret, def_count;
>
> flags = (kernel_ulong_t)device_get_match_data(dev);
>
> @@ -3567,6 +3793,33 @@ static int rx_macro_probe(struct platform_device *pdev)
> goto err;
> }
>
> + rx->codec_version = lpass_macro_get_codec_version();

What guarantees that VA macro has been probed already? If I'm not
mistaken, we might easily get a default '0' here instead of a correct
version.

> + switch (rx->codec_version) {
> + case LPASS_CODEC_VERSION_2_5 ... LPASS_CODEC_VERSION_2_8:
> + rx->rxn_reg_offset = 0xc0;
> + def_count = ARRAY_SIZE(rx_defaults) + ARRAY_SIZE(rx_2_5_defaults);
> + reg_defaults = kmalloc_array(def_count, sizeof(struct reg_default), GFP_KERNEL);
> + if (!reg_defaults)
> + return -ENOMEM;
> + memcpy(&reg_defaults[0], rx_defaults, sizeof(rx_defaults));
> + memcpy(&reg_defaults[ARRAY_SIZE(rx_defaults)],
> + rx_2_5_defaults, sizeof(rx_2_5_defaults));
> + break;
> + default:
> + rx->rxn_reg_offset = 0x80;
> + def_count = ARRAY_SIZE(rx_defaults) + ARRAY_SIZE(rx_pre_2_5_defaults);
> + reg_defaults = kmalloc_array(def_count, sizeof(struct reg_default), GFP_KERNEL);
> + if (!reg_defaults)
> + return -ENOMEM;
> + memcpy(&reg_defaults[0], rx_defaults, sizeof(rx_defaults));
> + memcpy(&reg_defaults[ARRAY_SIZE(rx_defaults)],
> + rx_pre_2_5_defaults, sizeof(rx_pre_2_5_defaults));
> + break;
> + }
> +
> + rx_regmap_config.reg_defaults = reg_defaults,
> + rx_regmap_config.num_reg_defaults = def_count;
> +
> rx->regmap = devm_regmap_init_mmio(dev, base, &rx_regmap_config);
> if (IS_ERR(rx->regmap)) {
> ret = PTR_ERR(rx->regmap);
> @@ -3629,6 +3882,7 @@ static int rx_macro_probe(struct platform_device *pdev)
> if (ret)
> goto err_clkout;
>
> + kfree(reg_defaults);
> return 0;
>
> err_clkout:
> @@ -3642,6 +3896,7 @@ static int rx_macro_probe(struct platform_device *pdev)
> err_dcodec:
> clk_disable_unprepare(rx->macro);
> err:
> + kfree(reg_defaults);
> lpass_macro_pds_exit(rx->pds);
>
> return ret;
> --
> 2.21.0
>

--
With best wishes
Dmitry