thesofproject
Fix DSP suspend after a crash
Looks like a great fix, just trying to understand how it applies in our 5.4 and 5.10 kernels as those don't have the concept of the panic state yet. @cujomalainey [ASoC: SOF: pm: Always tear down pipelines before DSP suspend] This is applicable only to 5.15 kernel since soon after resume we see playback is not happening since the pieplines are not tared down before the system suspends. On 5.10 we don't see this issue. Whereas [ASoC: SOF: pm: Set target state earlier] is applicable to both 5.10 and 5.15.
Whereas [ASoC: SOF: pm: Set target state earlier] is applicable to both 5.10 and 5.15.
Hmm, in 5.10 I still don't see how that is true. In 5.10 fw_state is either SOF_FW_BOOT_NOT_STARTED or SOF_FW_BOOT_COMPLETE by the time FW load is complete. Suspend has an early bail for NOT_STARTED and the bug requires it to be not in the BOOT_COMPLETE state.
Whereas [ASoC: SOF: pm: Set target state earlier] is applicable to both 5.10 and 5.15.
Hmm, in 5.10 I still don't see how that is true. In 5.10
fw_stateis eitherSOF_FW_BOOT_NOT_STARTEDorSOF_FW_BOOT_COMPLETEby the time FW load is complete. Suspend has an early bail forNOT_STARTEDand the bug requires it to be not in theBOOT_COMPLETEstate.
I do not think the fix applies for the kernels that do not change the FW state after a crash. But wouldnt hurt to initialize the target state at the beginning even in those cases.
Whereas [ASoC: SOF: pm: Set target state earlier] is applicable to both 5.10 and 5.15.
Hmm, in 5.10 I still don't see how that is true. In 5.10
fw_stateis eitherSOF_FW_BOOT_NOT_STARTEDorSOF_FW_BOOT_COMPLETEby the time FW load is complete. Suspend has an early bail forNOT_STARTEDand the bug requires it to be not in theBOOT_COMPLETEstate.I do not think the fix applies for the kernels that do not change the FW state after a crash. But wouldnt hurt to initialize the target state at the beginning even in those cases.
That I agree with, but it unfortunately means we likely don't have our smoking gun
looks good but there's no information on the actual issue this PR is supposed to fix. The commit message and comment hint at a Chrome problem? Also shouldn't there be a Fixes: tag for both commits?
@plbossart bug report coming soon. @jairaj-arava please link the bug to this PR once you have posted it. Thanks!
looks good but there's no information on the actual issue this PR is supposed to fix. The commit message and comment hint at a Chrome problem? Also shouldn't there be a Fixes: tag for both commits?
@plbossart I im not sure about a fixes tag. Unfortunately this part of the code hasnt changed for a long time but what changed was the addition of the fw state machine and the addition of the state change during a DSP panic which basically makes the check if (fw_state != BOOT_COMPLETE) not true
Fine @ranj063, I'll let you decide if this needs to be merged now or later.
Fine @ranj063, I'll let you decide if this needs to be merged now or later.
@plbossart should I update the patches with "Link:,..."?
Fine @ranj063, I'll let you decide if this needs to be merged now or later.
@plbossart should I update the patches with "Link:,..."?
it's probably ok without the link - or it can be added when upstreaming.