Re: [PATCH] ASoC: Intel: eve: Enable mclk and ssp sclk early for rt5514

Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx> · Wed, 23 Oct 2019 10:46:08 -0500

On 10/23/19 10:16 AM, Brent Lu wrote:
The first DMIC capture always fail (zero sequence data from PCM port)
after using DSP hotwording function (i.e. Google assistant). The DMIC
is attached to rt5514 which also require eaily mclk/sclk like rt5663.
Therefore we add a dapm route to provide ssp mclk/sclk early to rt5514.
The rt5514 is attached to SSP0 and the rt5663 is attached to SSP1 so
we create another supply widget to provide SSP0 mclk/sclk to rt5514.

The patch looks fine, but I'd like to clear a doubt I have on how MCLKs 
are handled.

IIRC, the hardware exposes 2 MCLK outputs, and it's not uncommon to 
share the same MCLK between SSPs, or use a different MCLK id for the 
same SSP on different platforms (it's one of the differences between 
apl-da7219 and glk-da7219).

Can you double-check that at the board level the MCLK pins are actually 
different? If they are not, then we should not be enabling/disabling 
them separately, or you'll have side effects between headset and DMICs.

I also don't know what the SKL driver does with 'ssp0_mclk' and 
'ssp1_mclk'? Cezary, would you happen to know how the mapping between 
MCLK and SSPs is handled?


Signed-off-by: Brent Lu <brent.lu@xxxxxxxxx>
---
  .../soc/intel/boards/kbl_rt5663_rt5514_max98927.c  | 136 ++++++++++++++-------
  1 file changed, 92 insertions(+), 44 deletions(-)

diff --git a/sound/soc/intel/boards/kbl_rt5663_rt5514_max98927.c b/sound/soc/intel/boards/kbl_rt5663_rt5514_max98927.c
index dc09a85..1755efa 100644
--- a/sound/soc/intel/boards/kbl_rt5663_rt5514_max98927.c
+++ b/sound/soc/intel/boards/kbl_rt5663_rt5514_max98927.c
@@ -53,8 +53,10 @@ struct kbl_codec_private {
  	struct snd_soc_jack kabylake_headset;
  	struct list_head hdmi_pcm_list;
  	struct snd_soc_jack kabylake_hdmi[2];
-	struct clk *mclk;
-	struct clk *sclk;
+	struct clk *ssp0_mclk;
+	struct clk *ssp0_sclk;
+	struct clk *ssp1_mclk;
+	struct clk *ssp1_sclk;
  };
  
  enum {
@@ -77,13 +79,31 @@ static const struct snd_kcontrol_new kabylake_controls[] = {
  };
  
  static int platform_clock_control(struct snd_soc_dapm_widget *w,
-			struct snd_kcontrol *k, int  event)
+			struct snd_kcontrol *k, int event, int ssp_num)
  {
  	struct snd_soc_dapm_context *dapm = w->dapm;
  	struct snd_soc_card *card = dapm->card;
  	struct kbl_codec_private *priv = snd_soc_card_get_drvdata(card);
+	struct clk *mclk, *sclk;
+	unsigned long sclk_rate;
  	int ret = 0;
  
+	switch (ssp_num) {
+	case 0:
+		mclk = priv->ssp0_mclk;
+		sclk = priv->ssp0_sclk;
+		sclk_rate = 6144000;
+		break;
+	case 1:
+		mclk = priv->ssp1_mclk;
+		sclk = priv->ssp1_sclk;
+		sclk_rate = 3072000;
+		break;
+	default:
+		dev_err(card->dev, "Invalid ssp_num %d\n", ssp_num);
+		return -EINVAL;
+	}
+
  	/*
  	 * MCLK/SCLK need to be ON early for a successful synchronization of
  	 * codec internal clock. And the clocks are turned off during
@@ -92,37 +112,39 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
  	switch (event) {
  	case SND_SOC_DAPM_PRE_PMU:
  		/* Enable MCLK */
-		ret = clk_set_rate(priv->mclk, 24000000);
+		ret = clk_set_rate(mclk, 24000000);
  		if (ret < 0) {
-			dev_err(card->dev, "Can't set rate for mclk, err: %d\n",
-				ret);
+			dev_err(card->dev, "Can't set rate for ssp%d_mclk, err: %d\n",
+				ssp_num, ret);
  			return ret;
  		}
  
-		ret = clk_prepare_enable(priv->mclk);
+		ret = clk_prepare_enable(mclk);
  		if (ret < 0) {
-			dev_err(card->dev, "Can't enable mclk, err: %d\n", ret);
+			dev_err(card->dev, "Can't enable ssp%d_mclk, err: %d\n",
+				ssp_num, ret);
  			return ret;
  		}
  
  		/* Enable SCLK */
-		ret = clk_set_rate(priv->sclk, 3072000);
+		ret = clk_set_rate(sclk, sclk_rate);
  		if (ret < 0) {
-			dev_err(card->dev, "Can't set rate for sclk, err: %d\n",
-				ret);
-			clk_disable_unprepare(priv->mclk);
+			dev_err(card->dev, "Can't set rate for ssp%d_sclk, err: %d\n",
+				ssp_num, ret);
+			clk_disable_unprepare(mclk);
  			return ret;
  		}
  
-		ret = clk_prepare_enable(priv->sclk);
+		ret = clk_prepare_enable(sclk);
  		if (ret < 0) {
-			dev_err(card->dev, "Can't enable sclk, err: %d\n", ret);
-			clk_disable_unprepare(priv->mclk);
+			dev_err(card->dev, "Can't enable ssp%d_sclk, err: %d\n",
+				ssp_num, ret);
+			clk_disable_unprepare(mclk);
  		}
  		break;
  	case SND_SOC_DAPM_POST_PMD:
-		clk_disable_unprepare(priv->mclk);
-		clk_disable_unprepare(priv->sclk);
+		clk_disable_unprepare(mclk);
+		clk_disable_unprepare(sclk);
  		break;
  	default:
  		return 0;
@@ -131,6 +153,18 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
  	return 0;
  }
  
+static int platform_clock_control_ssp0(struct snd_soc_dapm_widget *w,
+			struct snd_kcontrol *k, int event)
+{
+	return platform_clock_control(w, k, event, 0);
+}
+
+static int platform_clock_control_ssp1(struct snd_soc_dapm_widget *w,
+			struct snd_kcontrol *k, int event)
+{
+	return platform_clock_control(w, k, event, 1);
+}
+
  static const struct snd_soc_dapm_widget kabylake_widgets[] = {
  	SND_SOC_DAPM_HP("Headphone Jack", NULL),
  	SND_SOC_DAPM_MIC("Headset Mic", NULL),
@@ -139,15 +173,17 @@ static const struct snd_soc_dapm_widget kabylake_widgets[] = {
  	SND_SOC_DAPM_MIC("DMIC", NULL),
  	SND_SOC_DAPM_SPK("HDMI1", NULL),
  	SND_SOC_DAPM_SPK("HDMI2", NULL),
-	SND_SOC_DAPM_SUPPLY("Platform Clock", SND_SOC_NOPM, 0, 0,
-			platform_clock_control, SND_SOC_DAPM_PRE_PMU |
+	SND_SOC_DAPM_SUPPLY("Platform Clock SSP0", SND_SOC_NOPM, 0, 0,
+			platform_clock_control_ssp0, SND_SOC_DAPM_PRE_PMU |
+			SND_SOC_DAPM_POST_PMD),
+	SND_SOC_DAPM_SUPPLY("Platform Clock SSP1", SND_SOC_NOPM, 0, 0,
+			platform_clock_control_ssp1, SND_SOC_DAPM_PRE_PMU |
  			SND_SOC_DAPM_POST_PMD),
-
  };
  
  static const struct snd_soc_dapm_route kabylake_map[] = {
  	/* Headphones */
-	{ "Headphone Jack", NULL, "Platform Clock" },
+	{ "Headphone Jack", NULL, "Platform Clock SSP1" },
  	{ "Headphone Jack", NULL, "HPOL" },
  	{ "Headphone Jack", NULL, "HPOR" },
  
@@ -156,7 +192,7 @@ static const struct snd_soc_dapm_route kabylake_map[] = {
  	{ "Right Spk", NULL, "Right BE_OUT" },
  
  	/* other jacks */
-	{ "Headset Mic", NULL, "Platform Clock" },
+	{ "Headset Mic", NULL, "Platform Clock SSP1" },
  	{ "IN1P", NULL, "Headset Mic" },
  	{ "IN1N", NULL, "Headset Mic" },
  
@@ -180,6 +216,7 @@ static const struct snd_soc_dapm_route kabylake_map[] = {
  	{ "ssp0 Rx", NULL, "Right HiFi Capture" },
  
  	/* DMIC */
+	{ "DMIC", NULL, "Platform Clock SSP0" },
  	{ "DMIC1L", NULL, "DMIC" },
  	{ "DMIC1R", NULL, "DMIC" },
  	{ "DMIC2L", NULL, "DMIC" },
@@ -704,6 +741,29 @@ static struct snd_soc_card kabylake_audio_card = {
  	.late_probe = kabylake_card_late_probe,
  };
  
+static int kabylake_audio_clk_get(struct device *dev, const char *id,
+	struct clk **clk)
+{
+	int ret = 0;
+
+	if (!clk)
+		return -EINVAL;
+
+	*clk = devm_clk_get(dev, id);
+	if (IS_ERR(*clk)) {
+		ret = PTR_ERR(*clk);
+		if (ret == -ENOENT) {
+			dev_info(dev, "Failed to get %s, defer probe\n", id);
+			return -EPROBE_DEFER;
+		}
+
+		dev_err(dev, "Failed to get %s with err:%d\n", id, ret);
+		return ret;
+	}
+
+	return ret;
+}
+
  static int kabylake_audio_probe(struct platform_device *pdev)
  {
  	struct kbl_codec_private *ctx;
@@ -724,33 +784,21 @@ static int kabylake_audio_probe(struct platform_device *pdev)
  		dmic_constraints = mach->mach_params.dmic_num == 2 ?
  			&constraints_dmic_2ch : &constraints_dmic_channels;
  
-	ctx->mclk = devm_clk_get(&pdev->dev, "ssp1_mclk");
-	if (IS_ERR(ctx->mclk)) {
-		ret = PTR_ERR(ctx->mclk);
-		if (ret == -ENOENT) {
-			dev_info(&pdev->dev,
-				"Failed to get ssp1_mclk, defer probe\n");
-			return -EPROBE_DEFER;
-		}
+	ret = kabylake_audio_clk_get(&pdev->dev, "ssp0_mclk", &ctx->ssp0_mclk);
+	if (ret != 0)
+		return ret;
  
-		dev_err(&pdev->dev, "Failed to get ssp1_mclk with err:%d\n",
-								ret);
+	ret = kabylake_audio_clk_get(&pdev->dev, "ssp0_sclk", &ctx->ssp0_sclk);
+	if (ret != 0)
  		return ret;
-	}
  
-	ctx->sclk = devm_clk_get(&pdev->dev, "ssp1_sclk");
-	if (IS_ERR(ctx->sclk)) {
-		ret = PTR_ERR(ctx->sclk);
-		if (ret == -ENOENT) {
-			dev_info(&pdev->dev,
-				"Failed to get ssp1_sclk, defer probe\n");
-			return -EPROBE_DEFER;
-		}
+	ret = kabylake_audio_clk_get(&pdev->dev, "ssp1_mclk", &ctx->ssp1_mclk);
+	if (ret != 0)
+		return ret;
  
-		dev_err(&pdev->dev, "Failed to get ssp1_sclk with err:%d\n",
-								ret);
+	ret = kabylake_audio_clk_get(&pdev->dev, "ssp1_sclk", &ctx->ssp1_sclk);
+	if (ret != 0)
  		return ret;
-	}
  
  	return devm_snd_soc_register_card(&pdev->dev, &kabylake_audio_card);
  }


_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel






