Re: New platform CherryTrail/ES8316 audio output is silent

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 02/20/2017 02:14 PM, Daniel Drake wrote:
  .On Fri, Jan 13, 2017 at 1:33 PM, Pierre-Louis Bossart
<pierre-louis.bossart@xxxxxxxxxxxxxxx> wrote:
That is very unusual. We have not seen any platforms use SSP1 for the codec
connections. Are you really sure? If you can share the schematics privately
I'd like to take a look. We don't have support upstream for SSP1 usage so
that'd be really problematic.
I have now confirmed it is using SSP2, sorry for the noise there.

After modifying it to use SSP2 I am still only getting silence though.
I am now using the code submitted to
https://bugzilla.kernel.org/show_bug.cgi?id=189261 and I'll post my
version shortly. Any further suggestions would be much appreciated.
There were quite a few comments provided to David @ Everest Audio, I'll wait until I see an updated version with a --signoff to chime in.

I'd like to follow up on some of your review comments that also apply
to that code:

+static int cht_aif1_hw_params(struct snd_pcm_substream *substream,
+                       struct snd_pcm_hw_params *params)
+{
+       struct snd_soc_pcm_runtime *rtd = substream->private_data;
+       struct snd_soc_dai *codec_dai = rtd->codec_dai;
+       unsigned int fmt;
+       int ret;
+
+       pr_debug("Enter:%s", __func__);
+
+       //add for voip call no sound by zm 1211
+       if (strncmp(codec_dai->name, "ES8316 HiFi", 11))
+                return 0;
+
+       /* I2S Slave Mode*/
+       fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
+                       SND_SOC_DAIFMT_CBS_CFS;
+
+       /* Set codec DAI configuration */
+       ret = snd_soc_dai_set_fmt(codec_dai, fmt);
+       if (ret < 0) {
+               pr_err("can't set codec DAI configuration %d\n", ret);
+               return ret;
+       }
+
+#if 0
+       ret = snd_soc_dai_set_pll(codec_dai, 0, ES8316_PLL_SRC_FRM_MCLK,
+                               CHT_PLAT_CLK_3_HZ, params_rate(params) *
256);
+       if (ret < 0) {
+               pr_err("can't set codec pll***********: %d\n", ret);
+               return ret;
+       }
+
+
+       if (codec_dai->driver && codec_dai->driver->ops->hw_params)
+               codec_dai->driver->ops->hw_params(substream, params,
codec_dai);
+
+       snd_soc_dai_set_sysclk(codec_dai, ES8316_CLKID_PLLO,
CHT_PLAT_CLK_3_HZ, 0);
+#endif
+#if 0
+       ret = snd_soc_dai_set_sysclk(codec_dai, 0,
+                       params_rate(params) * 512, SND_SOC_CLOCK_IN);
+       if (ret < 0) {
+               pr_err("can't set codec sysclk: %d\n", ret);
+               return ret;
+       }
+#endif

weird, the PLL doesn't seem to be set, only the value for the system clk?
I realise that work will need to be done here before the code can be
accepted. But I think I should be able to get audio output working
first, because after starting to play a sound I am using regmap
debugfs to write all the codec registers using values that I dumped
from windows. Please correct me if I'm missing something.

Also the es8316.c codec driver does not have a set_pll function and
while the hw_params does calculate some coefficients related to sample
rate and clock speed, it never actually uses any of the results of the
calculation.

+       snd_soc_dai_set_sysclk(codec_dai, ES8316_CLKID_PLLO,
CHT_PLAT_CLK_3_HZ, 0);
+
+       return 0;
+}
+
+static int cht_compr_set_params(struct snd_compr_stream *cstream)
+{
+       return 0;
+}
+
+static const struct snd_soc_pcm_stream cht_dai_params = {
+       .formats = SNDRV_PCM_FMTBIT_S24_LE,
+       .rate_min = 48000,
+       .rate_max = 48000,
+       .channels_min = 2,
+       .channels_max = 2,
+};
+
+static struct snd_soc_compr_ops cht_compr_ops = {
+       .set_params = cht_compr_set_params,
+};
+
+static int cht_codec_fixup(struct snd_soc_pcm_runtime *rtd,
+                           struct snd_pcm_hw_params *params)
+{
+       struct snd_interval *rate = hw_param_interval(params,
+                       SNDRV_PCM_HW_PARAM_RATE);
+       struct snd_interval *channels = hw_param_interval(params,
+
SNDRV_PCM_HW_PARAM_CHANNELS);
+
+       pr_debug("Invoked %s for dailink %s\n", __func__,
rtd->dai_link->name);
+
+       /* The DSP will covert the FE rate to 48k, stereo, 24bits */
+       rate->min = rate->max = 48000;
+       channels->min = channels->max = 4;//2

again that part is wild
What is the wild part? The min/max 4 channels instead of 2, or the
whole function?

It's otherwise identical to working code in cht_bsw_rt5645.c.
No. In the code above you set the codec_dai to work in I2S mode, and here you are asking for 4 slots. Unless you have evidence that the codec support TDM, use 2 channels I2S and set the same value on the cpu_dai side.

+       /* Back ends */
+       {
+               .name = "SSP2-Codec",
+               .id = 1,
+               .cpu_dai_name = "ssp1-port",
+               .platform_name = "sst-mfld-platform",
+               .no_pcm = 1,
+               .codec_dai_name = "ES8316 HiFi",
+               .codec_name = "i2c-ESSX8316:00",
+               .dai_fmt = SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_IB_NF
+                                       | SND_SOC_DAIFMT_CBS_CFS,

the format here is not consistent with what you are asking the codec dai to
do, and it's not consistent either with the Intel side which doesn't use
this mode.
The same dai_fmt is working fine in cht_bsw_rt5645.c. Neverthless I'd
be happy to fix it up, can you suggest a correct value?
same comment, pick 2 or 4 channels but be consistent.

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

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



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux