[PATCH] add support to reorder CPU and Codec DAI suspend/resume

[PATCH] add support to reorder CPU and Codec DAI suspend/resume

[Barry Song]

Due to some hardware dependence issues, some devices maybe require
different suspend/resume execution order for CPU and Codec DAI to
make sure the hardware can work normally after executing sleep and
wakeup.

In old ALSA driver, people include all suspend/resume operations of
both CPU and Codec in same entries. Then they change order according
to actual require. But in ASoC, we should give a chance to users to
reorder CPU/Codec DAI suspend/resume instead of defining the fixed
order if there are some strange hardware dependence problems.

Signed-off-by: Barry Song <21cnbao@gmail.com>
---
 include/sound/soc.h  |    4 ++++
 sound/soc/soc-core.c |   48 ++++++++++++++++++++++++++++++++++++------------
 2 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index b1245e3..701c362 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -494,6 +494,10 @@ struct snd_soc_card {
 	struct snd_soc_dai_link *dai_link;
 	int num_links;
 
+	/* CPU <--> Codec DAI suspend/resume order */
+	int cpu_suspend_earlier; /* CPU DAI and platform suspend will be called earlier than Codec */
+	int cpu_resume_earlier;  /* CPU DAI and platform resume will be called earlier than Codec */
+
 	struct snd_soc_device *socdev;
 
 	struct snd_soc_codec *codec;
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 2d190df..a1cc855 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -660,12 +660,14 @@ static int soc_suspend(struct device *dev)
 	if (card->suspend_pre)
 		card->suspend_pre(pdev, PMSG_SUSPEND);
 
-	for (i = 0; i < card->num_links; i++) {
-		struct snd_soc_dai  *cpu_dai = card->dai_link[i].cpu_dai;
-		if (cpu_dai->suspend && !cpu_dai->ac97_control)
-			cpu_dai->suspend(cpu_dai);
-		if (platform->suspend)
-			platform->suspend(cpu_dai);
+	if (card->cpu_suspend_earlier) {
+		for (i = 0; i < card->num_links; i++) {
+			struct snd_soc_dai  *cpu_dai = card->dai_link[i].cpu_dai;
+			if (cpu_dai->suspend && !cpu_dai->ac97_control)
+				cpu_dai->suspend(cpu_dai);
+			if (platform->suspend)
+				platform->suspend(cpu_dai);
+		}
 	}
 
 	/* close any waiting streams and save state */
@@ -692,6 +694,16 @@ static int soc_suspend(struct device *dev)
 			cpu_dai->suspend(cpu_dai);
 	}
 
+	if (!card->cpu_suspend_earlier) {
+		for (i = 0; i < card->num_links; i++) {
+			struct snd_soc_dai  *cpu_dai = card->dai_link[i].cpu_dai;
+			if (cpu_dai->suspend && !cpu_dai->ac97_control)
+				cpu_dai->suspend(cpu_dai);
+			if (platform->suspend)
+				platform->suspend(cpu_dai);
+		}
+	}
+
 	if (card->suspend_post)
 		card->suspend_post(pdev, PMSG_SUSPEND);
 
@@ -728,6 +740,16 @@ static void soc_resume_deferred(struct work_struct *work)
 			cpu_dai->resume(cpu_dai);
 	}
 
+	if (card->cpu_resume_earlier) {
+		for (i = 0; i < card->num_links; i++) {
+			struct snd_soc_dai *cpu_dai = card->dai_link[i].cpu_dai;
+			if (cpu_dai->resume && !cpu_dai->ac97_control)
+				cpu_dai->resume(cpu_dai);
+			if (platform->resume)
+				platform->resume(cpu_dai);
+		}
+	}
+
 	if (codec_dev->resume)
 		codec_dev->resume(pdev);
 
@@ -749,12 +771,14 @@ static void soc_resume_deferred(struct work_struct *work)
 			dai->ops->digital_mute(dai, 0);
 	}
 
-	for (i = 0; i < card->num_links; i++) {
-		struct snd_soc_dai *cpu_dai = card->dai_link[i].cpu_dai;
-		if (cpu_dai->resume && !cpu_dai->ac97_control)
-			cpu_dai->resume(cpu_dai);
-		if (platform->resume)
-			platform->resume(cpu_dai);
+	if (!card->cpu_resume_earlier) {
+		for (i = 0; i < card->num_links; i++) {
+			struct snd_soc_dai *cpu_dai = card->dai_link[i].cpu_dai;
+			if (cpu_dai->resume && !cpu_dai->ac97_control)
+				cpu_dai->resume(cpu_dai);
+			if (platform->resume)
+				platform->resume(cpu_dai);
+		}
 	}
 
 	if (card->resume_post)
-- 
1.5.6.3

Re: [PATCH] add support to reorder CPU and Codec DAI suspend/resume

[Mark Brown]

On Wed, Nov 11, 2009 at 03:10:57PM +0800, Barry Song wrote:
> Due to some hardware dependence issues, some devices maybe require
> different suspend/resume execution order for CPU and Codec DAI to
> make sure the hardware can work normally after executing sleep and
> wakeup.

Are there any actual systems that this solves problems for - if so, what
are the problems?  I don't think this solution is general enough to be
scalable, it'll work for one particular use case but if we get further
reorderings needed it'll become difficult to manage.

If we're going to be doing anything with this I think it would be much
better to use the pm_link infrastructure which will be appearing in
2.6.34.  This should allow us to provide arbatrary ordering of device
level suspend and resume.

Re: [PATCH] add support to reorder CPU and Codec DAI suspend/resume

[Barry Song]

On Wed, Nov 11, 2009 at 7:24 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, Nov 11, 2009 at 03:10:57PM +0800, Barry Song wrote:
>> Due to some hardware dependence issues, some devices maybe require
>> different suspend/resume execution order for CPU and Codec DAI to
>> make sure the hardware can work normally after executing sleep and
>> wakeup.
>
> Are there any actual systems that this solves problems for - if so, what
> are the problems?  I don't think this solution is general enough to be
> scalable, it'll work for one particular use case but if we get further
> reorderings needed it'll become difficult to manage.
>
> If we're going to be doing anything with this I think it would be much
> better to use the pm_link infrastructure which will be appearing in
> 2.6.34.  This should allow us to provide arbatrary ordering of device
> level suspend and resume.
I am glad to hear there is a plan about pm_link. I think that will
provide generic support for all related issues. And I also think my
solution can only solve some particular cases.
Here I have a problem about blackfin TDM. After a deep sleep(all
suspend to memory and most parts power-down), If codec resumes at
first, clock ans sync are provided by it at onece. Then while CPU TDM
interface resumes, it will fail to go back to right status for work.
In the reverse orders,  CPU interfaces stand there at first and wait
coming sync and clocks, it can go to right status.
Do you have any idea about fixing it better?

>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] add support to reorder CPU and Codec DAI suspend/resume

[Mark Brown]

On Wed, Nov 11, 2009 at 08:48:19PM +0800, Barry Song wrote:

> Here I have a problem about blackfin TDM. After a deep sleep(all
> suspend to memory and most parts power-down), If codec resumes at
> first, clock ans sync are provided by it at onece. Then while CPU TDM
> interface resumes, it will fail to go back to right status for work.

What trouble does this give the TDM stuff?  It sounds very fragile if
things get confused like this (presumably as a result of having the
clocks running).

Re: [PATCH] add support to reorder CPU and Codec DAI suspend/resume

[Barry Song]

On Wed, Nov 11, 2009 at 10:20 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, Nov 11, 2009 at 08:48:19PM +0800, Barry Song wrote:
>
>> Here I have a problem about blackfin TDM. After a deep sleep(all
>> suspend to memory and most parts power-down), If codec resumes at
>> first, clock ans sync are provided by it at onece. Then while CPU TDM
>> interface resumes, it will fail to go back to right status for work.
>
> What trouble does this give the TDM stuff?  It sounds very fragile if
> things get confused like this (presumably as a result of having the
> clocks running).
The only difference should be whether clock/sync is fired from codec
before or after TDM interfaces are resumed to be ready for the
different resume order of CPU and Codec DAI, as I guess. That's
difficult to explain why.

>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] add support to reorder CPU and Codec DAI suspend/resume

[Mark Brown]

On Wed, Nov 11, 2009 at 10:34:00PM +0800, Barry Song wrote:

> The only difference should be whether clock/sync is fired from codec
> before or after TDM interfaces are resumed to be ready for the
> different resume order of CPU and Codec DAI, as I guess. That's
> difficult to explain why.

It'd really like to understand what's going wrong with the controller
here - it should just sync up off the frame sync signal.

Re: [PATCH] add support to reorder CPU and Codec DAI suspend/resume

[Barry Song]

On Wed, Nov 11, 2009 at 10:42 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, Nov 11, 2009 at 10:34:00PM +0800, Barry Song wrote:
>
>> The only difference should be whether clock/sync is fired from codec
>> before or after TDM interfaces are resumed to be ready for the
>> different resume order of CPU and Codec DAI, as I guess. That's
>> difficult to explain why.
>
> It'd really like to understand what's going wrong with the controller
> here - it should just sync up off the frame sync signal.

Don't know what happens exactly in hardware controllers . But I
"fixed" this porblem by isolating interfaces's sync/clock pins from
Codec(config them to GPIOs), then the whole system gets normal. Since
you have a plan for pm-link, I will use this way to keep the fix in
blackfin locally.

>

Re: [PATCH] add support to reorder CPU and Codec DAI suspend/resume

[Mark Brown]

On Thu, Nov 12, 2009 at 01:47:36PM +0800, Barry Song wrote:
> On Wed, Nov 11, 2009 at 10:42 PM, Mark Brown

> > It'd really like to understand what's going wrong with the controller
> > here - it should just sync up off the frame sync signal.

> Don't know what happens exactly in hardware controllers . But I
> "fixed" this porblem by isolating interfaces's sync/clock pins from
> Codec(config them to GPIOs), then the whole system gets normal. Since
> you have a plan for pm-link, I will use this way to keep the fix in
> blackfin locally.

Thinking about this further I think you need the GPIO fix anyway.  There
will be some CODECs (I'm mostly thinking of fixed function ones with no
software control) that won't stop generating clocks unless power is
removed.