Hi Allen,

After our discussion today I tested the patch below and everything works OK:

diff --git a/sound/soc/sof/sof-of-dev.c b/sound/soc/sof/sof-of-dev.c index 403b393528eb..f358435aca21 100644 --- a/sound/soc/sof/sof-of-dev.c +++ b/sound/soc/sof/sof-of-dev.c @@ -73,7 +73,6 @@ static void sof_of_probe_complete(struct device *dev) pm_runtime_enable(dev);

pm_runtime_mark_last_busy(dev); - pm_runtime_put_autosuspend(dev); }

I wonder why it previously worked! 🙂

Please send a patch as we've discussed to fix this. Also, as Pierre said for consistency please reorder of PM calls as follows:

pm_runtime_set_autosuspend_delay(dev, DELAY_MS); pm_runtime_use_autosuspend(dev); pm_runtime_mark_last_busy(dev); pm_runtime_set_active(dev); pm_runtime_enable(dev); ________________________________ From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Sent: Wednesday, October 20, 2021 7:44 PM To: Allen-KH Cheng (程冠勳) Allen-KH.Cheng@mediatek.com; Daniel Baluta daniel.baluta@nxp.com Cc: sound-open-firmware@alsa-project.org sound-open-firmware@alsa-project.org; YC Hung (洪堯俊) yc.hung@mediatek.com; Nathan Chung (仲崇寧) nathan.chung@mediatek.com; Randy Wu (吳柏樹) Randy.Wu@mediatek.com Subject: Re: [Sound-open-firmware] [SOF][KERNEL] pm_runtime flow in sof

In sof-of-dev.c static void sof_of_probe_complete(struct device *dev) { /* allow runtime_pm */ pm_runtime_set_autosuspend_delay(dev, SND_SOF_SUSPEND_DELAY_MS); pm_runtime_use_autosuspend(dev); pm_runtime_set_active(dev); pm_runtime_enable(dev);

pm_runtime_mark_last_busy(dev); pm_runtime_put_autosuspend(dev);

this last line looks wrong

it looks like the code in sof-of-dev.c is imbalanced. The last pm_runtime_put_autosuspend() is not matching any pm_runtime_get_sync(), is it?

}

After pm_runtime_put_autosuspend, the current device power usage count should minus one. (1->0)

And wait SND_SOF_SUSPEND_DELAY_MS seconds, sof device will autosuspend (because the power usage count is 0 ).

Indeed.

We have this comment for the PCI case:

/* follow recommendation in pci-driver.c to decrement usage counter */ pm_runtime_put_noidle(dev);

and a matching one for remove: /* follow recommendation in pci-driver.c to increment usage counter */ if (snd_sof_device_probe_completed(&pci->dev) && !(sof_pci_debug & SOF_PCI_DISABLE_PM_RUNTIME)) pm_runtime_get_noresume(&pci->dev);

In the general case, I don't know why you would need to add a put_autosuspend in the probe complete, even for cases where the device does support pm_runtime.

Having a mark_last_busy() is fine though, it would reset the counters and let the autosuspend happen at a later time. But I would call this before enable for consistency.

Can you try with the sequence below in probe_complete?

pm_runtime_set_autosuspend_delay(dev, DELAY_MS); pm_runtime_use_autosuspend(dev); pm_runtime_mark_last_busy(dev); pm_runtime_set_active(dev); pm_runtime_enable(dev);