From: Rander Wang rander.wang@intel.com
commit 33c8516841ea4fa12fdb8961711bf95095c607ee upstream
On TGL platform with max98373 codec the trigger start sequence is fe first, then codec component and sdw link is the last. Recently a delay was introduced in max98373 codec driver and this resulted to the start of sdw stream transmission was delayed and the data transmitted by fw can't be consumed by sdw controller, so xrun happened.
Adding delay in trigger function is a bad idea. This patch enable spk pin in prepare function and disable it in hw_free to avoid xrun issue caused by delay in trigger.
Fixes: 3a27875e91fb ("ASoC: max98373: Added 30ms turn on/off time delay") BugLink: https://github.com/thesofproject/sof/issues/4066 Reviewed-by: Bard Liao bard.liao@intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Signed-off-by: Rander Wang rander.wang@intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Link: https://lore.kernel.org/r/20210625205042.65181-2-pierre-louis.bossart@linux.... Signed-off-by: Mark Brown broonie@kernel.org ---
backport to stable/linux-5.13.y and stable/linux-5.12.y since upstream commit does not apply directly due to a rename in 9c5046e4b3e7 which creates a conflict.
There is no need to apply this patch to earlier versions since the commit 3a27875e91fb is only present in V5.12+
sound/soc/intel/boards/sof_sdw_max98373.c | 81 +++++++++++++++-------- 1 file changed, 53 insertions(+), 28 deletions(-)
diff --git a/sound/soc/intel/boards/sof_sdw_max98373.c b/sound/soc/intel/boards/sof_sdw_max98373.c index cfdf970c5800..25daef910aee 100644 --- a/sound/soc/intel/boards/sof_sdw_max98373.c +++ b/sound/soc/intel/boards/sof_sdw_max98373.c @@ -55,43 +55,68 @@ static int spk_init(struct snd_soc_pcm_runtime *rtd) return ret; }
-static int max98373_sdw_trigger(struct snd_pcm_substream *substream, int cmd) +static int mx8373_enable_spk_pin(struct snd_pcm_substream *substream, bool enable) { + struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); + struct snd_soc_dai *codec_dai; + struct snd_soc_dai *cpu_dai; int ret; + int j;
- switch (cmd) { - case SNDRV_PCM_TRIGGER_START: - case SNDRV_PCM_TRIGGER_RESUME: - case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: - /* enable max98373 first */ - ret = max98373_trigger(substream, cmd); - if (ret < 0) - break; - - ret = sdw_trigger(substream, cmd); - break; - case SNDRV_PCM_TRIGGER_STOP: - case SNDRV_PCM_TRIGGER_SUSPEND: - case SNDRV_PCM_TRIGGER_PAUSE_PUSH: - ret = sdw_trigger(substream, cmd); - if (ret < 0) - break; - - ret = max98373_trigger(substream, cmd); - break; - default: - ret = -EINVAL; - break; + /* set spk pin by playback only */ + if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) + return 0; + + cpu_dai = asoc_rtd_to_cpu(rtd, 0); + for_each_rtd_codec_dais(rtd, j, codec_dai) { + struct snd_soc_dapm_context *dapm = + snd_soc_component_get_dapm(cpu_dai->component); + char pin_name[16]; + + snprintf(pin_name, ARRAY_SIZE(pin_name), "%s Spk", + codec_dai->component->name_prefix); + + if (enable) + ret = snd_soc_dapm_enable_pin(dapm, pin_name); + else + ret = snd_soc_dapm_disable_pin(dapm, pin_name); + + if (!ret) + snd_soc_dapm_sync(dapm); }
- return ret; + return 0; +} + +static int mx8373_sdw_prepare(struct snd_pcm_substream *substream) +{ + int ret = 0; + + /* according to soc_pcm_prepare dai link prepare is called first */ + ret = sdw_prepare(substream); + if (ret < 0) + return ret; + + return mx8373_enable_spk_pin(substream, true); +} + +static int mx8373_sdw_hw_free(struct snd_pcm_substream *substream) +{ + int ret = 0; + + /* according to soc_pcm_hw_free dai link free is called first */ + ret = sdw_hw_free(substream); + if (ret < 0) + return ret; + + return mx8373_enable_spk_pin(substream, false); }
static const struct snd_soc_ops max_98373_sdw_ops = { .startup = sdw_startup, - .prepare = sdw_prepare, - .trigger = max98373_sdw_trigger, - .hw_free = sdw_hw_free, + .prepare = mx8373_sdw_prepare, + .trigger = sdw_trigger, + .hw_free = mx8373_sdw_hw_free, .shutdown = sdw_shutdown, };
On Mon, Aug 02, 2021 at 01:06:14PM -0500, Pierre-Louis Bossart wrote:
From: Rander Wang rander.wang@intel.com
commit 33c8516841ea4fa12fdb8961711bf95095c607ee upstream
On TGL platform with max98373 codec the trigger start sequence is fe first, then codec component and sdw link is the last. Recently a delay was introduced in max98373 codec driver and this resulted to the start of sdw stream transmission was delayed and the data transmitted by fw can't be consumed by sdw controller, so xrun happened.
Adding delay in trigger function is a bad idea. This patch enable spk pin in prepare function and disable it in hw_free to avoid xrun issue caused by delay in trigger.
Fixes: 3a27875e91fb ("ASoC: max98373: Added 30ms turn on/off time delay") BugLink: https://github.com/thesofproject/sof/issues/4066 Reviewed-by: Bard Liao bard.liao@intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Signed-off-by: Rander Wang rander.wang@intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Link: https://lore.kernel.org/r/20210625205042.65181-2-pierre-louis.bossart@linux.... Signed-off-by: Mark Brown broonie@kernel.org
backport to stable/linux-5.13.y and stable/linux-5.12.y since upstream commit does not apply directly due to a rename in 9c5046e4b3e7 which creates a conflict.
Any objections to bringing in:
9c5046e4b3e7 ("ASoC: Intel: boards: create sof-maxim-common module") f6081af6cf2b ("ASoC: Intel: boards: handle hda-dsp-common as a module")
to 5.13 instead? This way we'll be better aligned with upstream and avoid this type of failures in the future.
On 8/2/21 2:27 PM, Sasha Levin wrote:
On Mon, Aug 02, 2021 at 01:06:14PM -0500, Pierre-Louis Bossart wrote:
From: Rander Wang rander.wang@intel.com
commit 33c8516841ea4fa12fdb8961711bf95095c607ee upstream
On TGL platform with max98373 codec the trigger start sequence is fe first, then codec component and sdw link is the last. Recently a delay was introduced in max98373 codec driver and this resulted to the start of sdw stream transmission was delayed and the data transmitted by fw can't be consumed by sdw controller, so xrun happened.
Adding delay in trigger function is a bad idea. This patch enable spk pin in prepare function and disable it in hw_free to avoid xrun issue caused by delay in trigger.
Fixes: 3a27875e91fb ("ASoC: max98373: Added 30ms turn on/off time delay") BugLink: https://github.com/thesofproject/sof/issues/4066 Reviewed-by: Bard Liao bard.liao@intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Signed-off-by: Rander Wang rander.wang@intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Link: https://lore.kernel.org/r/20210625205042.65181-2-pierre-louis.bossart@linux....
Signed-off-by: Mark Brown broonie@kernel.org
backport to stable/linux-5.13.y and stable/linux-5.12.y since upstream commit does not apply directly due to a rename in 9c5046e4b3e7 which creates a conflict.
Any objections to bringing in:
9c5046e4b3e7 ("ASoC: Intel: boards: create sof-maxim-common module") f6081af6cf2b ("ASoC: Intel: boards: handle hda-dsp-common as a module")
to 5.13 instead? This way we'll be better aligned with upstream and avoid this type of failures in the future.
Thanks for the suggestion Sasha, it'd certainly be easier for us and distros/end-users if such renames/repartition patches could land in -stable.
We have e.g. another recent case we de-duplicated a set of jack-detection enums in "ASoC: Intel: sof_sdw: include rt711.h for RT711 JD mode" and as a result the addition of newer quirks for Dell or NUC devices won't apply cleanly on the stable branch.
On Mon, Aug 02, 2021 at 04:28:23PM -0500, Pierre-Louis Bossart wrote:
On 8/2/21 2:27 PM, Sasha Levin wrote:
On Mon, Aug 02, 2021 at 01:06:14PM -0500, Pierre-Louis Bossart wrote:
From: Rander Wang rander.wang@intel.com
commit 33c8516841ea4fa12fdb8961711bf95095c607ee upstream
On TGL platform with max98373 codec the trigger start sequence is fe first, then codec component and sdw link is the last. Recently a delay was introduced in max98373 codec driver and this resulted to the start of sdw stream transmission was delayed and the data transmitted by fw can't be consumed by sdw controller, so xrun happened.
Adding delay in trigger function is a bad idea. This patch enable spk pin in prepare function and disable it in hw_free to avoid xrun issue caused by delay in trigger.
Fixes: 3a27875e91fb ("ASoC: max98373: Added 30ms turn on/off time delay") BugLink: https://github.com/thesofproject/sof/issues/4066 Reviewed-by: Bard Liao bard.liao@intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Signed-off-by: Rander Wang rander.wang@intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Link: https://lore.kernel.org/r/20210625205042.65181-2-pierre-louis.bossart@linux....
Signed-off-by: Mark Brown broonie@kernel.org
backport to stable/linux-5.13.y and stable/linux-5.12.y since upstream commit does not apply directly due to a rename in 9c5046e4b3e7 which creates a conflict.
Any objections to bringing in:
9c5046e4b3e7 ("ASoC: Intel: boards: create sof-maxim-common module") f6081af6cf2b ("ASoC: Intel: boards: handle hda-dsp-common as a module")
to 5.13 instead? This way we'll be better aligned with upstream and avoid this type of failures in the future.
Thanks for the suggestion Sasha, it'd certainly be easier for us and distros/end-users if such renames/repartition patches could land in -stable.
Okay, so I did just that and queued all 3 commits for 5.13.
linux-stable-mirror@lists.linaro.org