[PATCH] ASoC: SOF - topology - do not change the link trigger order for pre-1.4 firmware

[Jaroslav Kysela]

Dne 21. 02. 20 v 15:44 Kai Vehmanen napsal(a):
> Hi,
> 
> +Ranjani who did the link reorder patch
> 
> On Fri, 21 Feb 2020, Jaroslav Kysela wrote:
> 
>> This patch is for SOF v1.3 firmware. The DSP firmware will crash (DSP oops)
>> without this patch. The 1.4.1 firmare has this issue fixed.
>>
>> The ABI version is used for the comparison, because the firmware version
>> for the firmware files before 1.4.2 was not set properly (git hash was
>> used).
> 
> build fails when this is applied on broonie/for-next. You need an
> additional
> 
> --- a/sound/soc/sof/topology.c
> +++ b/sound/soc/sof/topology.c
> @@ -3108,6 +3108,7 @@ static int sof_link_load(struct snd_soc_component
> *scomp, int index,
>                           struct snd_soc_dai_link *link,
>                           struct snd_soc_tplg_link_config *cfg)
>   {
> +       struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(scomp);
>          struct snd_soc_tplg_private *private = &cfg->priv;
> 
> 
>> -             /* 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 (v->abi_version > 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;
> 
> My results with older firmwares and this patch are a bit mixed. When I
> apply this patch and boot with v1.3 FW on a CFL platform (ABI 3.7.0,
> version 1:1:0-5dd9a), I get a DSP panic at stream stop with this patch,
> but _without_ it, playback is fine. :P
> 
> I tested both v1.3.1 and v1.3, and I get a DSP panic at stream stop with
> your patch (ABI 3:7:0 on both of these so trigger order is not changed).
> With v1.4 and all newer, streaming works as expected.

Ok, then I wonder how the pre-5.5 kernel worked (because it's just revert of
5eee2b3f60065a2530d13f28e771be48b989eb4c for older firmware versions which 
should be tested with the old code).

> The original problem was sensitive to timing, so apparently there is still
> some variation how this triggers on different platforms. With 1.4, 1.4.1
> and 1.4.2 now out, primary solution is just to upgrade the firmware.
> 
> If this fix helps with some real-life case to cope with an old firmware,
> we should probably still consider this. Ranjani, does the above make
> sense?

Ok, it's really weird that we cannot determine the firmware/driver combination 
which cause the DSP lock. I would propose to block the older firmware load 
<1.4 (or 1.4.2 which has the correct firmware version!) then (at least with a 
big warning or module option which is off by default) for the newer kernels.

					Jaroslav

-- 
Jaroslav Kysela <perex at perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.