Re: [PATCH AUTOSEL 5.7 055/388] ASoC: SOF: Do nothing when DSP PM callbacks are not set
On 6/18/20 6:44 AM, Daniel Baluta wrote: On 6/18/20 2:01 PM, Mark Brown wrote: On Wed, Jun 17, 2020 at 09:02:32PM -0400, Sasha Levin wrote: From: Daniel Baluta [ Upstream commit c26fde3b15ed41f5f452f1da727795f787833287 ] This provides a better separation between runtime and PM sleep callbacks. Only do nothing if given runtime flag is set and calback is not set. With the current implementation, if PM sleep callback is set but runtime callback is not set then at runtime resume we reload the firmware even if we do not support runtime resume callback. This doesn't look like a bugfix, just an optimization? Indeed can be seen as an optimization, but it does unexpected things which can cause trouble and weird behavior for people not familiar with the matter. For example, as explained in the commit message if you only provide System PM handler but not runtime PM handler, then the DSP will be resetted even if this is not the intention. I think it's a bug fix for Intel legacy platforms (Baytrail, Broadwell) where runtime_pm isn't supported. However the additional fixes for system suspend/resume were only provided for 5.8, so this patch in isolation will not do much for those platforms. Put differently, even if this patch is applied to 5.7 suspend/resume would still not work for Baytrail/Broadwell. Daniel, your call if you need this for i.MX?
Re: [PATCH AUTOSEL 5.7 055/388] ASoC: SOF: Do nothing when DSP PM callbacks are not set
On Thu, Jun 18, 2020 at 02:44:18PM +0300, Daniel Baluta wrote: > Indeed can be seen as an optimization, but it does unexpected things which > can cause trouble > and weird behavior for people not familiar with the matter. > For example, as explained in the commit message if you only provide > System PM handler but not runtime PM handler, then the DSP will be resetted > even if this is not the intention. The user shouldn't be thinking about if the DSP is reset during runtime PM, or if it's being reset... signature.asc Description: PGP signature
Re: [PATCH AUTOSEL 5.7 055/388] ASoC: SOF: Do nothing when DSP PM callbacks are not set
On 6/18/20 2:01 PM, Mark Brown wrote: On Wed, Jun 17, 2020 at 09:02:32PM -0400, Sasha Levin wrote: From: Daniel Baluta [ Upstream commit c26fde3b15ed41f5f452f1da727795f787833287 ] This provides a better separation between runtime and PM sleep callbacks. Only do nothing if given runtime flag is set and calback is not set. With the current implementation, if PM sleep callback is set but runtime callback is not set then at runtime resume we reload the firmware even if we do not support runtime resume callback. This doesn't look like a bugfix, just an optimization? Indeed can be seen as an optimization, but it does unexpected things which can cause trouble and weird behavior for people not familiar with the matter. For example, as explained in the commit message if you only provide System PM handler but not runtime PM handler, then the DSP will be resetted even if this is not the intention.
Re: [PATCH AUTOSEL 5.7 055/388] ASoC: SOF: Do nothing when DSP PM callbacks are not set
On Wed, Jun 17, 2020 at 09:02:32PM -0400, Sasha Levin wrote: > From: Daniel Baluta > > [ Upstream commit c26fde3b15ed41f5f452f1da727795f787833287 ] > > This provides a better separation between runtime and PM sleep > callbacks. > > Only do nothing if given runtime flag is set and calback is not set. > > With the current implementation, if PM sleep callback is set but runtime > callback is not set then at runtime resume we reload the firmware even > if we do not support runtime resume callback. This doesn't look like a bugfix, just an optimization? signature.asc Description: PGP signature
[PATCH AUTOSEL 5.7 055/388] ASoC: SOF: Do nothing when DSP PM callbacks are not set
From: Daniel Baluta [ Upstream commit c26fde3b15ed41f5f452f1da727795f787833287 ] This provides a better separation between runtime and PM sleep callbacks. Only do nothing if given runtime flag is set and calback is not set. With the current implementation, if PM sleep callback is set but runtime callback is not set then at runtime resume we reload the firmware even if we do not support runtime resume callback. Signed-off-by: Daniel Baluta Signed-off-by: Kai Vehmanen Reviewed-by: Pierre-Louis Bossart Reviewed-by: Ranjani Sridharan Link: https://lore.kernel.org/r/20200515135958.17511-2-kai.vehma...@linux.intel.com Signed-off-by: Mark Brown Signed-off-by: Sasha Levin --- sound/soc/sof/pm.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/sound/soc/sof/pm.c b/sound/soc/sof/pm.c index c410822d9920..01d83ddc16ba 100644 --- a/sound/soc/sof/pm.c +++ b/sound/soc/sof/pm.c @@ -90,7 +90,10 @@ static int sof_resume(struct device *dev, bool runtime_resume) int ret; /* do nothing if dsp resume callbacks are not set */ - if (!sof_ops(sdev)->resume || !sof_ops(sdev)->runtime_resume) + if (!runtime_resume && !sof_ops(sdev)->resume) + return 0; + + if (runtime_resume && !sof_ops(sdev)->runtime_resume) return 0; /* DSP was never successfully started, nothing to resume */ @@ -175,7 +178,10 @@ static int sof_suspend(struct device *dev, bool runtime_suspend) int ret; /* do nothing if dsp suspend callback is not set */ - if (!sof_ops(sdev)->suspend) + if (!runtime_suspend && !sof_ops(sdev)->suspend) + return 0; + + if (runtime_suspend && !sof_ops(sdev)->runtime_suspend) return 0; if (sdev->fw_state != SOF_FW_BOOT_COMPLETE) -- 2.25.1
