-
Notifications
You must be signed in to change notification settings - Fork 139
ASoC: SOF: topology: add pm-get/put around topology load #241
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
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>
| ret = -EINVAL; | ||
| } | ||
|
|
||
| ret = pm_runtime_put(sdev->dev); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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);There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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?
|
@plbossart i'm sorry I think I messed up the compilation last time. There are 2 issues with this patch:
|
|
@ranj063 can you confirm this can be dropped - already handled in your own PM-related PR? |
|
I think it's better to merge this, as it's possible that we have possibility entered runtime PM already at topology load? |
|
@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? |
|
@plbossart i ran into issue with adding a get_sync() before topology load. |
|
@plbossart the commit 1ac91fe in #249 fixes this issue. |
|
closing since fixed by #249 |
rust: module: improve parsing and errors
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