Skip to content

Conversation

@plbossart
Copy link
Member

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

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);
Copy link
Collaborator

@ranj063 ranj063 Nov 1, 2018

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);

Copy link
Member Author

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?

Copy link
Collaborator

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.

Copy link
Member Author

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?

@ranj063
Copy link
Collaborator

ranj063 commented Nov 2, 2018

@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
Copy link
Member Author

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

@keyonjie
Copy link

keyonjie commented Nov 8, 2018

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
Copy link
Member Author

@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
Copy link
Collaborator

ranj063 commented Nov 8, 2018

@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
Copy link
Collaborator

ranj063 commented Nov 8, 2018

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

@plbossart
Copy link
Member Author

closing since fixed by #249

@plbossart plbossart closed this Nov 9, 2018
aiChaoSONG pushed a commit to aiChaoSONG/linux that referenced this pull request May 6, 2021
rust: module: improve parsing and errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants