[alsa-devel] New platform CherryTrail/ES8316 audio output is silent

[Daniel Drake]

 .On Fri, Jan 13, 2017 at 1:33 PM, Pierre-Louis Bossart
<pierre-louis.bossart at linux.intel.com> 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.

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.

>> +       /* 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?

Thanks
Daniel