| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAOs-w0JSFOZf_H3puAAkwbhZH1cWKit8R_2BqmEtx8y9n=ebEA@mail.gmail.com>
Date: Thu, 10 Sep 2020 17:48:34 +0200
From: Radosław Biernacki <rad@...ihalf.com>
To: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>,
Liam Girdwood <liam.r.girdwood@...ux.intel.com>,
Jie Yang <yang.jie@...ux.intel.com>,
Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>
Cc: Ben Zhang <benzh@...omium.org>, Marcin Wojtas <mw@...ihalf.com>,
Todd Broch <tbroch@...gle.com>,
Alex Levin <levinale@...gle.com>,
Vamshi Krishna <vamshi.krishna.gopal@...el.com>,
Harshapriya <harshapriya.n@...el.com>,
"Sienkiewicz, Michal" <michal.sienkiewicz@...el.com>,
Lech Betlej <Lech.Betlej@...el.com>,
alsa-devel@...a-project.org, linux-kernel@...r.kernel.org,
John Hsu <KCHSU0@...oton.com>, Yong Zhi <yong.zhi@...el.com>,
Mac Chiang <mac.chiang@...el.com>
Subject: Re: [PATCH V4] ASoC: Intel: boards: Use FS as nau8825 sysclk in
nau88125_* machine
Sorry, V4 got one stray change for SSP0 ops.
Need to send V5.
czw., 10 wrz 2020 o 17:40 Radoslaw Biernacki <rad@...ihalf.com> napisał(a):
>
> Since 256xFS clocks cannot be generated by SKL, the NAU8825 is
> configured to re-generate its system clock from the BCLK using the
> FLL. The link is configured to use a 48kHz frame rate, and 24 bits in
> 25-bit slot. The SSP configuration is extracted from NHLT settings and
> not dynamically changed. Listening tests and measurements do not show
> any distortion or issues
>
> Signed-off-by: John Hsu <KCHSU0@...oton.com>
> Signed-off-by: Yong Zhi <yong.zhi@...el.com>
> Signed-off-by: Mac Chiang <mac.chiang@...el.com>
> Signed-off-by: Ben Zhang <benzh@...omium.org>
> Signed-off-by: Radoslaw Biernacki <rad@...ihalf.com>
> ---
>
> Notes:
> v1 -> v2:
> - adding same changes to skl_nau88l25_max98357a.c
> v2 -> v3:
> - removing msleep() in SNDRV_PCM_TRIGGER_RESUME as it unnecessarily increase
> playback/capture latency while actually FLL does not require it.
> - simplifing commit message
> v3 -> v4:
> - simplifing the PM resume callback code for setting the FLL
> - adding comment for the stream START/RESUME sequence which prevent audio pops
> - fixing mising var initialization in platform_clock_control()
>
> .../soc/intel/boards/skl_nau88l25_max98357a.c | 63 ++++++++++++------
> sound/soc/intel/boards/skl_nau88l25_ssm4567.c | 65 +++++++++++++------
> 2 files changed, 86 insertions(+), 42 deletions(-)
>
> diff --git a/sound/soc/intel/boards/skl_nau88l25_max98357a.c b/sound/soc/intel/boards/skl_nau88l25_max98357a.c
> index d7b8154c43a4..2f0abbd2dd8d 100644
> --- a/sound/soc/intel/boards/skl_nau88l25_max98357a.c
> +++ b/sound/soc/intel/boards/skl_nau88l25_max98357a.c
> @@ -8,6 +8,7 @@
>
> #include <linux/module.h>
> #include <linux/platform_device.h>
> +#include <linux/delay.h>
> #include <sound/core.h>
> #include <sound/jack.h>
> #include <sound/pcm.h>
> @@ -47,12 +48,12 @@ enum {
> };
>
> static int platform_clock_control(struct snd_soc_dapm_widget *w,
> - struct snd_kcontrol *k, int event)
> + struct snd_kcontrol *k, int event)
> {
> struct snd_soc_dapm_context *dapm = w->dapm;
> struct snd_soc_card *card = dapm->card;
> struct snd_soc_dai *codec_dai;
> - int ret;
> + int ret = 0;
>
> codec_dai = snd_soc_card_get_codec_dai(card, SKL_NUVOTON_CODEC_DAI);
> if (!codec_dai) {
> @@ -60,14 +61,7 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
> return -EIO;
> }
>
> - if (SND_SOC_DAPM_EVENT_ON(event)) {
> - ret = snd_soc_dai_set_sysclk(codec_dai,
> - NAU8825_CLK_MCLK, 24000000, SND_SOC_CLOCK_IN);
> - if (ret < 0) {
> - dev_err(card->dev, "set sysclk err = %d\n", ret);
> - return -EIO;
> - }
> - } else {
> + if (!SND_SOC_DAPM_EVENT_ON(event)) {
> ret = snd_soc_dai_set_sysclk(codec_dai,
> NAU8825_CLK_INTERNAL, 0, SND_SOC_CLOCK_IN);
> if (ret < 0) {
> @@ -292,24 +286,51 @@ static const struct snd_soc_ops skylake_nau8825_fe_ops = {
> .startup = skl_fe_startup,
> };
>
> -static int skylake_nau8825_hw_params(struct snd_pcm_substream *substream,
> - struct snd_pcm_hw_params *params)
> +static int skylake_nau8825_trigger(struct snd_pcm_substream *substream, int cmd)
> {
> struct snd_soc_pcm_runtime *rtd = substream->private_data;
> + struct snd_pcm_runtime *runtime = substream->runtime;
> struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
> - int ret;
> -
> - ret = snd_soc_dai_set_sysclk(codec_dai,
> - NAU8825_CLK_MCLK, 24000000, SND_SOC_CLOCK_IN);
> -
> - if (ret < 0)
> - dev_err(rtd->dev, "snd_soc_dai_set_sysclk err = %d\n", ret);
> + int ret = 0;
> +
> + switch (cmd) {
> + case SNDRV_PCM_TRIGGER_START:
> + /* Since 256xFS clocks cannot be generated by SKL, the NAU8825
> + * is configured to re-generate its system clock from the BCLK
> + * using the FLL.
> + * We must switch system clock (FLL to use BCLK) here as it is
> + * not given eariler by FW (like in hw_param). We let nau8825 to
> + * use internal VCO clock till now which reduces the audiable
> + * pop's. */
> +
> + /* fall through */
> +
> + case SNDRV_PCM_TRIGGER_RESUME:
> + /* Once device resumes, the system will only enable power
> + * sequence for playback without doing hardware parameter, audio
> + * format, and PLL configure. In the mean time, the jack
> + * detecion sequence has changed PLL parameters and switched to
> + * internal clock. Thus, the playback signal distorted without
> + * correct PLL parameters. Therefore we need to configure PLL
> + * again */
> + ret = snd_soc_dai_set_sysclk(codec_dai, NAU8825_CLK_FLL_FS, 0,
> + SND_SOC_CLOCK_IN);
> + if (ret < 0) {
> + dev_err(codec_dai->dev, "can't set FS clock %d\n", ret);
> + break;
> + }
> + ret = snd_soc_dai_set_pll(codec_dai, 0, 0, runtime->rate,
> + runtime->rate * 256);
> + if (ret < 0)
> + dev_err(codec_dai->dev, "can't set FLL: %d\n", ret);
> + break;
> + }
>
> return ret;
> }
>
> -static const struct snd_soc_ops skylake_nau8825_ops = {
> - .hw_params = skylake_nau8825_hw_params,
> +static struct snd_soc_ops skylake_nau8825_ops = {
> + .trigger = skylake_nau8825_trigger,
> };
>
> static int skylake_dmic_fixup(struct snd_soc_pcm_runtime *rtd,
> diff --git a/sound/soc/intel/boards/skl_nau88l25_ssm4567.c b/sound/soc/intel/boards/skl_nau88l25_ssm4567.c
> index 4b317bcf6ea0..d076f19f9b78 100644
> --- a/sound/soc/intel/boards/skl_nau88l25_ssm4567.c
> +++ b/sound/soc/intel/boards/skl_nau88l25_ssm4567.c
> @@ -12,6 +12,7 @@
>
> #include <linux/module.h>
> #include <linux/platform_device.h>
> +#include <linux/delay.h>
> #include <sound/core.h>
> #include <sound/pcm.h>
> #include <sound/soc.h>
> @@ -57,12 +58,12 @@ static const struct snd_kcontrol_new skylake_controls[] = {
> };
>
> static int platform_clock_control(struct snd_soc_dapm_widget *w,
> - struct snd_kcontrol *k, int event)
> + struct snd_kcontrol *k, int event)
> {
> struct snd_soc_dapm_context *dapm = w->dapm;
> struct snd_soc_card *card = dapm->card;
> struct snd_soc_dai *codec_dai;
> - int ret;
> + int ret = 0;
>
> codec_dai = snd_soc_card_get_codec_dai(card, SKL_NUVOTON_CODEC_DAI);
> if (!codec_dai) {
> @@ -70,14 +71,7 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
> return -EIO;
> }
>
> - if (SND_SOC_DAPM_EVENT_ON(event)) {
> - ret = snd_soc_dai_set_sysclk(codec_dai,
> - NAU8825_CLK_MCLK, 24000000, SND_SOC_CLOCK_IN);
> - if (ret < 0) {
> - dev_err(card->dev, "set sysclk err = %d\n", ret);
> - return -EIO;
> - }
> - } else {
> + if (!SND_SOC_DAPM_EVENT_ON(event)) {
> ret = snd_soc_dai_set_sysclk(codec_dai,
> NAU8825_CLK_INTERNAL, 0, SND_SOC_CLOCK_IN);
> if (ret < 0) {
> @@ -85,6 +79,7 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
> return -EIO;
> }
> }
> +
> return ret;
> }
>
> @@ -344,24 +339,51 @@ static int skylake_dmic_fixup(struct snd_soc_pcm_runtime *rtd,
> return 0;
> }
>
> -static int skylake_nau8825_hw_params(struct snd_pcm_substream *substream,
> - struct snd_pcm_hw_params *params)
> +static int skylake_nau8825_trigger(struct snd_pcm_substream *substream, int cmd)
> {
> struct snd_soc_pcm_runtime *rtd = substream->private_data;
> + struct snd_pcm_runtime *runtime = substream->runtime;
> struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
> - int ret;
> -
> - ret = snd_soc_dai_set_sysclk(codec_dai,
> - NAU8825_CLK_MCLK, 24000000, SND_SOC_CLOCK_IN);
> -
> - if (ret < 0)
> - dev_err(rtd->dev, "snd_soc_dai_set_sysclk err = %d\n", ret);
> + int ret = 0;
> +
> + switch (cmd) {
> + case SNDRV_PCM_TRIGGER_START:
> + /* Since 256xFS clocks cannot be generated by SKL, the NAU8825
> + * is configured to re-generate its system clock from the BCLK
> + * using the FLL.
> + * We must switch system clock (FLL to use BCLK) here as it is
> + * not given eariler by FW (like in hw_param). We let nau8825 to
> + * use internal VCO clock till now which reduces the audiable
> + * pop's. */
> +
> + /* fall through */
> +
> + case SNDRV_PCM_TRIGGER_RESUME:
> + /* Once device resumes, the system will only enable power
> + * sequence for playback without doing hardware parameter, audio
> + * format, and PLL configure. In the mean time, the jack
> + * detecion sequence has changed PLL parameters and switched to
> + * internal clock. Thus, the playback signal distorted without
> + * correct PLL parameters. Therefore we need to configure PLL
> + * again */
> + ret = snd_soc_dai_set_sysclk(codec_dai, NAU8825_CLK_FLL_FS, 0,
> + SND_SOC_CLOCK_IN);
> + if (ret < 0) {
> + dev_err(codec_dai->dev, "can't set FS clock %d\n", ret);
> + break;
> + }
> + ret = snd_soc_dai_set_pll(codec_dai, 0, 0, runtime->rate,
> + runtime->rate * 256);
> + if (ret < 0)
> + dev_err(codec_dai->dev, "can't set FLL: %d\n", ret);
> + break;
> + }
>
> return ret;
> }
>
> -static const struct snd_soc_ops skylake_nau8825_ops = {
> - .hw_params = skylake_nau8825_hw_params,
> +static struct snd_soc_ops skylake_nau8825_ops = {
> + .trigger = skylake_nau8825_trigger,
> };
>
> static const unsigned int channels_dmic[] = {
> @@ -582,6 +604,7 @@ static struct snd_soc_dai_link skylake_dais[] = {
> .init = skylake_ssm4567_codec_init,
> .ignore_pmdown_time = 1,
> .be_hw_params_fixup = skylake_ssp_fixup,
> + .ops = &skylake_nau8825_ops,
> .dpcm_playback = 1,
> .dpcm_capture = 1,
> SND_SOC_DAILINK_REG(ssp0_pin, ssp0_codec, platform),
> --
> 2.17.1
>
Powered by blists - more mailing lists