ASoC: SOF: topology: add pm-get/put around topology load

[plbossart]

Make sure all the individual IPCs to configure the DSP based on
topology info take place with the DSP already in D0 on startup.

Suggested-by: Liam Girdwood liam.r.girdwood@linux.intel.com
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com

[ranj063]

@plbossart this seems to take care of the error. But if we are using autosuspend, shouldnt we use
pm_runtime_mark_last_busy(sdev->dev);
ret = pm_runtime_put_autosuspend(sdev->dev);

On the other hand, instead of this patch what also works is:S ince we are enabling autosuspend on component->dev, we should use runtime_put_autosuspend instead of runtime_idle.

diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c
index f2e256d129bf5..0476aed01fa17 100644
--- a/sound/soc/sof/pcm.c+++ b/sound/soc/sof/pcm.c@@ -712,7 +712,10 @@ static int sof_pcm_probe(struct snd_soc_component *component)
SND_SOF_SUSPEND_DELAY);
pm_runtime_use_autosuspend(component->dev);
pm_runtime_enable(component->dev);
- err = pm_runtime_idle(component->dev);++ /* autosuspend sof device */+ pm_runtime_mark_last_busy(component->dev);+ err = pm_runtime_put_autosuspend(component->dev);
if (err < 0)
dev_err(sdev->dev, "error: failed to enter PM idle %d\n", err);

[plbossart]

@ranj063 I edited your comment to format the patch contents with the 4 backquotes thingy

I am not sure I understand the proposal above at all, where is the pm_runtime_get_ done prior to calling snd_sof_topology_load?

[ranj063]

@plbossart from my observations, I dont think the sof device enter runtime suspend before topology loading occurs.

in pcm.c after topology loading in completed, we enable runtime_pm_enable and use autosuspend for component->dev. And then we call pm_runtime_idle(). This seems to be what is causing the error.

What I was suggesting was that instead of calling pm_runtime_idle, we should use put_autosuspend instead.

[plbossart]

@ranj063 My understanding is that for every pm_runtime_put_xxx you need to have a matching pm_runtime_get_xxx. I don't get how we can use put_autosuspend() without a prior get(). What am I missing?

[ranj063]

@plbossart i'm sorry I think I messed up the compilation last time. There are 2 issues with this patch:

1. Missing header in topology.c
   #include <linux/pm_runtime.h>

2. Secondly, I run into an error with pm_runtime_get_sync()
   [ 4.807965] bxt_da7219_max98357a glk_da7219_max98357a: info: override FE DAI link iDisp1
   [ 4.807968] bxt_da7219_max98357a glk_da7219_max98357a: info: override FE DAI link iDisp2
   [ 4.807970] bxt_da7219_max98357a glk_da7219_max98357a: info: override FE DAI link iDisp3
   [ 4.808280] sof-audio sof-audio: loading topology:intel/sof-glk-da7219.tplg
   [ 4.808697] sof-audio sof-audio: error: topology load pm_runtime_get_sync failed with -13
   [ 4.808702] sof-audio sof-audio: error: failed to load DSP topology -13
   [ 4.808705] sof-audio sof-audio: ASoC: failed to probe component -13
   [ 4.808732] bxt_da7219_max98357a glk_da7219_max98357a: ASoC: failed to instantiate card -13
   [ 4.812075] bxt_da7219_max98357a: probe of glk_da7219_max98357a failed with error -13

[plbossart]

@ranj063 can you confirm this can be dropped - already handled in your own PM-related PR?

[keyonjie]

I think it's better to merge this, as it's possible that we have possibility entered runtime PM already at topology load?
BTW, 2 Seconds for runtime PM timeout looks too short? shall we change it to 5 seconds?

[plbossart]

@keyonjie Ranjani's changes add a get_nosync and a put_autosuspend after the topology is loaded, so not sure if this is still needed. Ranjani?

[ranj063]

@plbossart i ran into issue with adding a get_sync() before topology load.
I think the right fix for is to use pm_runtime_put_autosuspend() instead of pm_runtime_idle() after topology load. Let me verify this.

[ranj063]

@plbossart the commit 1ac91fe in #249 fixes this issue.

[plbossart]

closing since fixed by #249