[PATCH] ASoC: Intel: Fix Kabylake machine driver for pops and clicks

[PATCH] ASoC: Intel: Fix Kabylake machine driver for pops and clicks

[Harsha Priya]

This patch adds fixes for the following issues in Kabylake machine driver

1. crackling noise in rt5663 headset codec 
2. enabling 4 slot IV feedback for max98927 speaker amp codec 
3. pop noise while recording from rt5514 dmic codec 

Signed-off-by: Harsha Priya <harshapriya.n@intel.com>
---
 .../soc/intel/boards/kbl_rt5663_rt5514_max98927.c  | 28 ++++++++++++----------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/sound/soc/intel/boards/kbl_rt5663_rt5514_max98927.c b/sound/soc/intel/boards/kbl_rt5663_rt5514_max98927.c
index 3fe4a08..4c51965 100644
--- a/sound/soc/intel/boards/kbl_rt5663_rt5514_max98927.c
+++ b/sound/soc/intel/boards/kbl_rt5663_rt5514_max98927.c
@@ -319,7 +319,9 @@ static int kabylake_rt5663_hw_params(struct snd_pcm_substream *substream,
 	int ret;
 
 	/* use ASRC for internal clocks, as PLL rate isn't multiple of BCLK */
-	rt5663_sel_asrc_clk_src(codec_dai->codec, RT5663_DA_STEREO_FILTER, 1);
+	rt5663_sel_asrc_clk_src(codec_dai->codec,
+			RT5663_DA_STEREO_FILTER | RT5663_AD_STEREO_FILTER,
+			RT5663_CLK_SEL_I2S1_ASRC);
 
 	ret = snd_soc_dai_set_sysclk(codec_dai,
 			RT5663_SCLK_S_MCLK, 24576000, SND_SOC_CLOCK_IN);
@@ -349,27 +351,27 @@ static int kabylake_ssp0_hw_params(struct snd_pcm_substream *substream,
 				return ret;
 			}
 
-			ret = snd_soc_dai_set_pll(codec_dai, 0,
-				RT5514_PLL1_S_BCLK, RT5514_AIF1_BCLK_FREQ,
-						RT5514_AIF1_SYSCLK_FREQ);
+			ret = snd_soc_dai_set_sysclk(codec_dai,
+				RT5514_SCLK_S_MCLK, 24576000, SND_SOC_CLOCK_IN);
+
 			if (ret < 0) {
-				dev_err(rtd->dev, "set bclk err: %d\n", ret);
+				dev_err(rtd->dev, "set sysclk err: %d\n", ret);
 				return ret;
 			}
+		}
 
-			ret = snd_soc_dai_set_sysclk(codec_dai,
-				RT5514_SCLK_S_PLL1, RT5514_AIF1_SYSCLK_FREQ,
-							SND_SOC_CLOCK_IN);
+		if (!strcmp(codec_dai->component->name, MAXIM_DEV0_NAME)) {
+			ret = snd_soc_dai_set_tdm_slot(codec_dai, 0x30, 3, 8, 16);
 			if (ret < 0) {
-				dev_err(rtd->dev, "set sclk err: %d\n", ret);
+				dev_err(rtd->dev, "DEV0 TDM slot err:%d\n", ret);
 				return ret;
 			}
 		}
-		if (!strcmp(codec_dai->component->name, MAXIM_DEV0_NAME) ||
-			!strcmp(codec_dai->component->name, MAXIM_DEV1_NAME)) {
-			ret = snd_soc_dai_set_tdm_slot(codec_dai, 0xF0, 3, 8, 16);
+
+		if (!strcmp(codec_dai->component->name, MAXIM_DEV1_NAME)) {
+			ret = snd_soc_dai_set_tdm_slot(codec_dai, 0xC0, 3, 8, 16);
 			if (ret < 0) {
-				dev_err(rtd->dev, "set TDM slot err:%d\n", ret);
+				dev_err(rtd->dev, "DEV1 TDM slot err:%d\n", ret);
 				return ret;
 			}
 		}
-- 
1.9.1

Re: [PATCH] ASoC: Intel: Fix Kabylake machine driver for pops and clicks

[Vinod Koul]

On Tue, Jul 25, 2017 at 03:27:39PM -0700, Harsha Priya wrote:
> This patch adds fixes for the following issues in Kabylake machine driver
> 
> 1. crackling noise in rt5663 headset codec 

And the fix was..?

> 2. enabling 4 slot IV feedback for max98927 speaker amp codec 

should that be a separate patch?

> 3. pop noise while recording from rt5514 dmic codec 

and this fix was..

Harsha, from the changelog, apart from 2, I am still not clear on the
contents of the patch. A patch changelog should document the code changes
done, its impact (done here) to help reviewer understand the patch

> 
> Signed-off-by: Harsha Priya <harshapriya.n@intel.com>
> ---
>  .../soc/intel/boards/kbl_rt5663_rt5514_max98927.c  | 28 ++++++++++++----------
>  1 file changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/sound/soc/intel/boards/kbl_rt5663_rt5514_max98927.c b/sound/soc/intel/boards/kbl_rt5663_rt5514_max98927.c
> index 3fe4a08..4c51965 100644
> --- a/sound/soc/intel/boards/kbl_rt5663_rt5514_max98927.c
> +++ b/sound/soc/intel/boards/kbl_rt5663_rt5514_max98927.c
> @@ -319,7 +319,9 @@ static int kabylake_rt5663_hw_params(struct snd_pcm_substream *substream,
>  	int ret;
>  
>  	/* use ASRC for internal clocks, as PLL rate isn't multiple of BCLK */
> -	rt5663_sel_asrc_clk_src(codec_dai->codec, RT5663_DA_STEREO_FILTER, 1);
> +	rt5663_sel_asrc_clk_src(codec_dai->codec,
> +			RT5663_DA_STEREO_FILTER | RT5663_AD_STEREO_FILTER,
> +			RT5663_CLK_SEL_I2S1_ASRC);

So now the clock source is ARSC right, how will that fix something?

>  
>  	ret = snd_soc_dai_set_sysclk(codec_dai,
>  			RT5663_SCLK_S_MCLK, 24576000, SND_SOC_CLOCK_IN);
> @@ -349,27 +351,27 @@ static int kabylake_ssp0_hw_params(struct snd_pcm_substream *substream,
>  				return ret;
>  			}
>  
> -			ret = snd_soc_dai_set_pll(codec_dai, 0,
> -				RT5514_PLL1_S_BCLK, RT5514_AIF1_BCLK_FREQ,
> -						RT5514_AIF1_SYSCLK_FREQ);
> +			ret = snd_soc_dai_set_sysclk(codec_dai,
> +				RT5514_SCLK_S_MCLK, 24576000, SND_SOC_CLOCK_IN);

now you no longer need to set pll?

> +
>  			if (ret < 0) {
> -				dev_err(rtd->dev, "set bclk err: %d\n", ret);
> +				dev_err(rtd->dev, "set sysclk err: %d\n", ret);
>  				return ret;
>  			}
> +		}
>  
> -			ret = snd_soc_dai_set_sysclk(codec_dai,
> -				RT5514_SCLK_S_PLL1, RT5514_AIF1_SYSCLK_FREQ,
> -							SND_SOC_CLOCK_IN);
> +		if (!strcmp(codec_dai->component->name, MAXIM_DEV0_NAME)) {
> +			ret = snd_soc_dai_set_tdm_slot(codec_dai, 0x30, 3, 8, 16);
>  			if (ret < 0) {
> -				dev_err(rtd->dev, "set sclk err: %d\n", ret);
> +				dev_err(rtd->dev, "DEV0 TDM slot err:%d\n", ret);
>  				return ret;
>  			}
>  		}
> -		if (!strcmp(codec_dai->component->name, MAXIM_DEV0_NAME) ||
> -			!strcmp(codec_dai->component->name, MAXIM_DEV1_NAME)) {
> -			ret = snd_soc_dai_set_tdm_slot(codec_dai, 0xF0, 3, 8, 16);
> +
> +		if (!strcmp(codec_dai->component->name, MAXIM_DEV1_NAME)) {
> +			ret = snd_soc_dai_set_tdm_slot(codec_dai, 0xC0, 3, 8, 16);
>  			if (ret < 0) {
> -				dev_err(rtd->dev, "set TDM slot err:%d\n", ret);
> +				dev_err(rtd->dev, "DEV1 TDM slot err:%d\n", ret);
>  				return ret;
>  			}
>  		}

-- 
~Vinod

Re: [PATCH] ASoC: Intel: Fix Kabylake machine driver for pops and clicks

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 486 bytes --]

On Tue, Jul 25, 2017 at 03:27:39PM -0700, Harsha Priya wrote:
> This patch adds fixes for the following issues in Kabylake machine driver
> 
> 1. crackling noise in rt5663 headset codec 
> 2. enabling 4 slot IV feedback for max98927 speaker amp codec 
> 3. pop noise while recording from rt5514 dmic codec 

This is three different fixes so I'd expect to see three different
patches and the changelog doesn't describe what the fixes actually do so
it's much harder to review.

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] ASoC: Intel: Fix Kabylake machine driver for pops and clicks

[N, Harshapriya]

> -----Original Message-----
> From: Mark Brown [mailto:broonie@kernel.org]
> Sent: Wednesday, July 26, 2017 4:50 AM
> To: N, Harshapriya <harshapriya.n@intel.com>
> Cc: tiwai@suse.de; Koul, Vinod <vinod.koul@intel.com>; alsa-devel@alsa-
> project.org
> Subject: Re: [PATCH] ASoC: Intel: Fix Kabylake machine driver for pops and
> clicks
> 
> On Tue, Jul 25, 2017 at 03:27:39PM -0700, Harsha Priya wrote:
> > This patch adds fixes for the following issues in Kabylake machine
> > driver
> >
> > 1. crackling noise in rt5663 headset codec 2. enabling 4 slot IV
> > feedback for max98927 speaker amp codec 3. pop noise while recording
> > from rt5514 dmic codec
> 
> This is three different fixes so I'd expect to see three different patches and
> the changelog doesn't describe what the fixes actually do so it's much harder
> to review.
I am sending a patch series with 3 patches in it each fixing one issue. I have tried
explaining in detail the fix in the commit message as well.