[alsa-devel] [PATCH] ASoC: SOF - topology - do not change the link triger order for old firmare

[alsa-devel] [PATCH] ASoC: SOF - topology - do not change the link triger order for old firmare

[Jaroslav Kysela]

BugLink: https://github.com/thesofproject/sof/issues/2102
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: Mark Brown <broonie@kernel.org>
---
 sound/soc/sof/topology.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c
index 143b8259a70a..d24268794a03 100644
--- a/sound/soc/sof/topology.c
+++ b/sound/soc/sof/topology.c
@@ -2935,6 +2935,7 @@ static int sof_link_load(struct snd_soc_component *scomp, int index,
 	struct snd_soc_tplg_private *private = &cfg->priv;
 	struct sof_ipc_dai_config config;
 	struct snd_soc_tplg_hw_config *hw_config;
+	struct sof_ipc_fw_version *v = &sdev->fw_ready.version;
 	int num_hw_configs;
 	int ret;
 	int i = 0;
@@ -2952,9 +2953,12 @@ static int sof_link_load(struct snd_soc_component *scomp, int index,
 	if (!link->no_pcm) {
 		link->nonatomic = true;
 
-		/* set trigger order */
-		link->trigger[0] = SND_SOC_DPCM_TRIGGER_POST;
-		link->trigger[1] = SND_SOC_DPCM_TRIGGER_POST;
+		/* this causes DSP panic on firmware v1.3 */
+		if (SOF_ABI_VER(v->major, v->minor, v->micro) > SOF_ABI_VER(3, 7, 0)) {
+			/* set trigger order */
+			link->trigger[0] = SND_SOC_DPCM_TRIGGER_POST;
+			link->trigger[1] = SND_SOC_DPCM_TRIGGER_POST;
+		}
 
 		/* nothing more to do for FE dai links */
 		return 0;
-- 
2.20.1
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [PATCH] ASoC: SOF - topology - do not change the link triger order for old firmare

[Pierre-Louis Bossart]

On 11/22/19 2:38 AM, Jaroslav Kysela wrote:
> BugLink: https://github.com/thesofproject/sof/issues/2102

This one is complicated.

The change of the trigger order is required in order to avoid DMA 
underflows.

But if you make this change, this exposes another issue in the firmware 
that leads to the a panic on some platforms (I couldn't reproduce it 
myself on a WHL HDAudio+dmic device), and unfortunately the fix for this 
DSP panic is not in the released 1.3 firmware.

With this proposal from Jaroslav, users of the older firmware will not 
see the panic but they are still facing potential underflows.

So long story short, I don't mind if we add this patch to solve the DSP 
panic, but there should be a clear explanation in the commit message 
that this is far from ideal and that an update to 1.4 is really desirable.

We may also need to look at different ways to identify the firmware, in 
this case the problem is not due to the ABI proper but a change in the 
timing sequences, we may need a different sort of ID here?

> Signed-off-by: Jaroslav Kysela <perex@perex.cz>
> Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Cc: Mark Brown <broonie@kernel.org>
> ---
>   sound/soc/sof/topology.c | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c
> index 143b8259a70a..d24268794a03 100644
> --- a/sound/soc/sof/topology.c
> +++ b/sound/soc/sof/topology.c
> @@ -2935,6 +2935,7 @@ static int sof_link_load(struct snd_soc_component *scomp, int index,
>   	struct snd_soc_tplg_private *private = &cfg->priv;
>   	struct sof_ipc_dai_config config;
>   	struct snd_soc_tplg_hw_config *hw_config;
> +	struct sof_ipc_fw_version *v = &sdev->fw_ready.version;
>   	int num_hw_configs;
>   	int ret;
>   	int i = 0;
> @@ -2952,9 +2953,12 @@ static int sof_link_load(struct snd_soc_component *scomp, int index,
>   	if (!link->no_pcm) {
>   		link->nonatomic = true;
>   
> -		/* set trigger order */
> -		link->trigger[0] = SND_SOC_DPCM_TRIGGER_POST;
> -		link->trigger[1] = SND_SOC_DPCM_TRIGGER_POST;
> +		/* this causes DSP panic on firmware v1.3 */
> +		if (SOF_ABI_VER(v->major, v->minor, v->micro) > SOF_ABI_VER(3, 7, 0)) {
> +			/* set trigger order */
> +			link->trigger[0] = SND_SOC_DPCM_TRIGGER_POST;
> +			link->trigger[1] = SND_SOC_DPCM_TRIGGER_POST;
> +		}
>   
>   		/* nothing more to do for FE dai links */
>   		return 0;
> 
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [PATCH] ASoC: SOF - topology - do not change the link triger order for old firmare

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 602 bytes --]

On Fri, Nov 22, 2019 at 08:50:15AM -0600, Pierre-Louis Bossart wrote:

> With this proposal from Jaroslav, users of the older firmware will not see
> the panic but they are still facing potential underflows.

Underflows do seem better and more recoverable than panics.

> We may also need to look at different ways to identify the firmware, in this
> case the problem is not due to the ABI proper but a change in the timing
> sequences, we may need a different sort of ID here?

Yeah.  I guess it depends how widespread the different behaviours of the
racy firmware are and what they're influenced by?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

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