[alsa-devel] [PATCH] ASoC: dapm: check suspend state in dapm_dai_check_power()
From: Stephen Warren swarren@nvidia.com
Currently, when the system enters suspend with audio playing, since the DAI widgets are active, the associated CODEC is considered active, and hence is not transition into a low-power bias state as the system suspends. This likely causes audio failure after resume, since the CODEC power has quite possibly been removed during suspend, and the bias level transitions in the CODEC driver are needed to recover from this. At the very least, this issue likely causes warnings in the kernel log due to the CODEC being left on over the suspend transition.
To solve this, modify dapm_dai_check_power() not to consider DAI widgets active if the system is entering suspend.
Suggested-by: Lars-Peter Clausen lars@metafoo.de Signed-off-by: Stephen Warren swarren@nvidia.com --- sound/soc/soc-dapm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index 3507346..bad545a 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -1133,7 +1133,7 @@ static int dapm_dai_check_power(struct snd_soc_dapm_widget *w) DAPM_UPDATE_STAT(w, power_checks);
if (w->active) - return w->active; + return snd_soc_dapm_suspend_check(w);
return dapm_generic_check_power(w); }
On Mon, Jun 03, 2013 at 11:36:40AM -0600, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
Currently, when the system enters suspend with audio playing, since the DAI widgets are active, the associated CODEC is considered active, and hence is not transition into a low-power bias state as the system suspends. This likely causes audio failure after resume, since the CODEC power has quite possibly been removed during suspend, and the bias level transitions in the CODEC driver are needed to recover from this. At the very least, this issue likely causes warnings in the kernel log due to the CODEC being left on over the suspend transition.
To solve this, modify dapm_dai_check_power() not to consider DAI widgets active if the system is entering suspend.
I would expect us to be dealing with this as part of quiescing the DMA operations on the links - it's not just the CODEC that needs handling here. This feels like it ought to be more joined up somewhere along the line.
On 06/03/2013 08:51 PM, Mark Brown wrote:
On Mon, Jun 03, 2013 at 11:36:40AM -0600, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
Currently, when the system enters suspend with audio playing, since the DAI widgets are active, the associated CODEC is considered active, and hence is not transition into a low-power bias state as the system suspends. This likely causes audio failure after resume, since the CODEC power has quite possibly been removed during suspend, and the bias level transitions in the CODEC driver are needed to recover from this. At the very least, this issue likely causes warnings in the kernel log due to the CODEC being left on over the suspend transition.
To solve this, modify dapm_dai_check_power() not to consider DAI widgets active if the system is entering suspend.
I would expect us to be dealing with this as part of quiescing the DMA operations on the links - it's not just the CODEC that needs handling here. This feels like it ought to be more joined up somewhere along the line.
I think it is just an oversight that this is not working properly right now. In snd_soc_suspend() where we also stop the DMA streams etc the DAI link widgets are put into suspend, by calling snd_soc_dapm_stream_event(..., SND_SOC_DAPM_STREAM_SUSPEND). But dapm_dai_check_power() doesn't consider this and hence stays active and also keeps the whole DAPM context (this is not CODEC specific) active.
- Lars
On Mon, Jun 03, 2013 at 09:16:36PM +0200, Lars-Peter Clausen wrote:
I think it is just an oversight that this is not working properly right now. In snd_soc_suspend() where we also stop the DMA streams etc the DAI link widgets are put into suspend, by calling snd_soc_dapm_stream_event(..., SND_SOC_DAPM_STREAM_SUSPEND). But dapm_dai_check_power() doesn't consider this and hence stays active and also keeps the whole DAPM context (this is not CODEC specific) active.
It's clearly an oversight but I don't think this is a joined up fix for it.
On Mon, Jun 03, 2013 at 07:51:20PM +0100, Mark Brown wrote:
I would expect us to be dealing with this as part of quiescing the DMA operations on the links - it's not just the CODEC that needs handling here. This feels like it ought to be more joined up somewhere along the line.
So having grovelled a bit this is of course more annoying than it should be; the trigger operation is in atomic context and there's nowhere convenient to react to it outside of suspend and resume. That said the active flag is only being used for links started and stopped from userspace so this change *ought* to be safe to do what you're doing but it still doesn't seem terribly clever.
Another way of looking at things here is that the DAI widgets are unusual in that they don't require a complete path to power them as widgets generally do (including DACs and ADCs). If we fixed that instead then everything would work just fine. This should just be a case of splitting the playback and capture DAIs into separate widget types and then using the existing DAC and ADC functions. I can't recall why we didn't do that in the first place, it's probably just an oversight...
participants (3)
-
Lars-Peter Clausen -
Mark Brown -
Stephen Warren