[alsa-devel] [PATCH] ASoC: Intel: Add support for PM ops in bxt-da7219_max98357a
We need card to be early suspended and late resumed, so use prepare and complete for card suspend and resume.
Signed-off-by: Vinod Koul vinod.koul@intel.com Tested-by: Harsha Priya harshapriya.n@intel.com --- sound/soc/intel/boards/bxt_da7219_max98357a.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/sound/soc/intel/boards/bxt_da7219_max98357a.c b/sound/soc/intel/boards/bxt_da7219_max98357a.c index 3774b117d365..df5f269a9b67 100644 --- a/sound/soc/intel/boards/bxt_da7219_max98357a.c +++ b/sound/soc/intel/boards/bxt_da7219_max98357a.c @@ -441,11 +441,30 @@ static int broxton_audio_probe(struct platform_device *pdev) return devm_snd_soc_register_card(&pdev->dev, &broxton_audio_card); }
+#ifdef CONFIG_PM_SLEEP + +static void broxton_complete(struct device *dev) +{ + snd_soc_resume(dev); +} + +#else +#define broxton_complete NULL +#endif + +static const struct dev_pm_ops broxton_pm_ops = { + .prepare = snd_soc_suspend, + .complete = broxton_complete, + .freeze = snd_soc_suspend, + .thaw = snd_soc_resume, + .poweroff = snd_soc_poweroff, + .restore = snd_soc_resume, +}; static struct platform_driver broxton_audio = { .probe = broxton_audio_probe, .driver = { .name = "bxt_da7219_max98357a_i2s", - .pm = &snd_soc_pm_ops, + .pm = &broxton_pm_ops, }, }; module_platform_driver(broxton_audio)
On Fri, Jun 17, 2016 at 10:03:30AM +0530, Vinod Koul wrote:
+static void broxton_complete(struct device *dev) +{
- snd_soc_resume(dev);
+}
+#else +#define broxton_complete NULL +#endif
Just use snd_soc_resume() directly.
On Fri, Jun 17, 2016 at 12:14:07PM +0100, Mark Brown wrote:
On Fri, Jun 17, 2016 at 10:03:30AM +0530, Vinod Koul wrote:
+static void broxton_complete(struct device *dev) +{
- snd_soc_resume(dev);
+}
+#else +#define broxton_complete NULL +#endif
Just use snd_soc_resume() directly.
Nope, that leads to warnings.
.complete is the only callback that expects void return whereas snd_soc_resume like other returns int.
struct dev_pm_ops { int (*prepare)(struct device *dev); void (*complete)(struct device *dev); int (*suspend)(struct device *dev); int (*resume)(struct device *dev); ... }
Thanks
On Fri, Jun 17, 2016 at 05:44:00PM +0530, Vinod Koul wrote:
On Fri, Jun 17, 2016 at 12:14:07PM +0100, Mark Brown wrote:
Just use snd_soc_resume() directly.
Nope, that leads to warnings.
.complete is the only callback that expects void return whereas snd_soc_resume like other returns int.
This is possibly an indication that you're abusing things then - this is a very unusual interface to use as well... the patch just looks very fishy as is, and as we were talking about in the other thread Lars started it seems like this is bodging around something rather than solving the right problem.
On Fri, Jun 17, 2016 at 01:35:23PM +0100, Mark Brown wrote:
On Fri, Jun 17, 2016 at 05:44:00PM +0530, Vinod Koul wrote:
On Fri, Jun 17, 2016 at 12:14:07PM +0100, Mark Brown wrote:
Just use snd_soc_resume() directly.
Nope, that leads to warnings.
.complete is the only callback that expects void return whereas snd_soc_resume like other returns int.
This is possibly an indication that you're abusing things then - this is a very unusual interface to use as well... the patch just looks very fishy as is, and as we were talking about in the other thread Lars started it seems like this is bodging around something rather than solving the right problem.
In the hindsight, I do agree that instead of this, we should move platform to do suspend_late and resume_early. That way the dependency between card and platform is taken care in a much bteer fashion.
The problem is two components racing and need of the resume being in serial fashion.
Thanks
On 06/17/2016 06:33 AM, Vinod Koul wrote:
We need card to be early suspended and late resumed, so use prepare and complete for card suspend and resume.
Why and why is this something that other cards do not need to do?
On Fri, Jun 17, 2016 at 01:18:10PM +0200, Lars-Peter Clausen wrote:
On 06/17/2016 06:33 AM, Vinod Koul wrote:
We need card to be early suspended and late resumed, so use prepare and complete for card suspend and resume.
Why and why is this something that other cards do not need to do?
When card suspends, the DAPM suspend closes the widgets, which translates to we sending IPC to DSP for tearing down the pipelines.
So we need the platform to be suspended last and resume first. This way the snd_soc_suspend will tear down pipelines and snd_soc_resume restore those back.
Thanks
On Fri, Jun 17, 2016 at 05:45:48PM +0530, Vinod Koul wrote:
On Fri, Jun 17, 2016 at 01:18:10PM +0200, Lars-Peter Clausen wrote:
Why and why is this something that other cards do not need to do?
When card suspends, the DAPM suspend closes the widgets, which translates to we sending IPC to DSP for tearing down the pipelines.
So we need the platform to be suspended last and resume first. This way the snd_soc_suspend will tear down pipelines and snd_soc_resume restore those back.
That doesn't answer the question - this applies to any control mechanism.
On Fri, Jun 17, 2016 at 01:31:56PM +0100, Mark Brown wrote:
On Fri, Jun 17, 2016 at 05:45:48PM +0530, Vinod Koul wrote:
On Fri, Jun 17, 2016 at 01:18:10PM +0200, Lars-Peter Clausen wrote:
Why and why is this something that other cards do not need to do?
Other systems do not have a DSP sitting and need to redownload code which takes time and results in card being resumed even when the platform is not ready.
The logs are indicating the snd_soc_resume() is triggered even before the platform resume has returned which needs to be avoided.
One of the ways to ensure a dependency for PM is resolved, we tinker with PM callbacks here to ensure the platform is ready before resume is inoked here
When card suspends, the DAPM suspend closes the widgets, which translates to we sending IPC to DSP for tearing down the pipelines.
So we need the platform to be suspended last and resume first. This way the snd_soc_suspend will tear down pipelines and snd_soc_resume restore those back.
That doesn't answer the question - this applies to any control mechanism.
Thanks
On Mon, Jun 20, 2016 at 05:14:29PM +0530, Vinod Koul wrote:
On Fri, Jun 17, 2016 at 01:31:56PM +0100, Mark Brown wrote:
On Fri, Jun 17, 2016 at 05:45:48PM +0530, Vinod Koul wrote:
On Fri, Jun 17, 2016 at 01:18:10PM +0200, Lars-Peter Clausen wrote:
Why and why is this something that other cards do not need to do?
Other systems do not have a DSP sitting and need to redownload code which takes time and results in card being resumed even when the platform is not ready.
So what you're actually trying to do here is serialize the resume - this isn't just a thing with DSPs, it's also a thing with I2C, regulators and so on. A quick glance suggests that disabling async suspend might help here.
On Mon, Jun 20, 2016 at 02:26:40PM +0100, Mark Brown wrote:
On Mon, Jun 20, 2016 at 05:14:29PM +0530, Vinod Koul wrote:
On Fri, Jun 17, 2016 at 01:31:56PM +0100, Mark Brown wrote:
On Fri, Jun 17, 2016 at 05:45:48PM +0530, Vinod Koul wrote:
On Fri, Jun 17, 2016 at 01:18:10PM +0200, Lars-Peter Clausen wrote:
Why and why is this something that other cards do not need to do?
Other systems do not have a DSP sitting and need to redownload code which takes time and results in card being resumed even when the platform is not ready.
So what you're actually trying to do here is serialize the resume - this isn't just a thing with DSPs, it's also a thing with I2C, regulators and so on. A quick glance suggests that disabling async suspend might help here.
Okay let me check that
Thanks
participants (3)
-
Lars-Peter Clausen -
Mark Brown -
Vinod Koul