[alsa-devel] [PATCH 1/2] OMAP3: MCBSP: Suspend/resume failure fix
McBSP fucntional clock is not disabled when suspend is triggerd which prevents PER domain entering target low-power state. To fix the problem:
1. Disable/enable McBSP functional clock for suspend/resume.
2. In addition, reconfigure McBSP, this is needed when systerm comes out of OFF state.
Signed-off-by: Jane Wang jwang@ti.com --- arch/arm/plat-omap/include/mach/mcbsp.h | 2 ++ arch/arm/plat-omap/mcbsp.c | 27 +++++++++++++++++++++++++++ sound/soc/omap/omap-mcbsp.c | 14 ++++++++++++-- 3 files changed, 41 insertions(+), 2 deletions(-)
diff --git a/arch/arm/plat-omap/include/mach/mcbsp.h b/arch/arm/plat-omap/include/mach/mcbsp.h index e0d6eca..94e1c66 100644 --- a/arch/arm/plat-omap/include/mach/mcbsp.h +++ b/arch/arm/plat-omap/include/mach/mcbsp.h @@ -440,6 +440,8 @@ static inline int omap_mcbsp_get_dma_op_mode(unsigned int id) { return 0; } #endif int omap_mcbsp_request(unsigned int id); void omap_mcbsp_free(unsigned int id); +void omap_mcbsp_disable_fclk(unsigned int id); +void omap_mcbsp_enable_fclk(unsigned int id); void omap_mcbsp_start(unsigned int id, int tx, int rx); void omap_mcbsp_stop(unsigned int id, int tx, int rx); void omap_mcbsp_xmit_word(unsigned int id, u32 word); diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c index e664b91..bfe3c61 100644 --- a/arch/arm/plat-omap/mcbsp.c +++ b/arch/arm/plat-omap/mcbsp.c @@ -459,6 +459,33 @@ int omap_mcbsp_request(unsigned int id) } EXPORT_SYMBOL(omap_mcbsp_request);
+void omap_mcbsp_disable_fclk(unsigned int id) +{ + struct omap_mcbsp *mcbsp; + + if (!omap_mcbsp_check_valid_id(id)) { + printk(KERN_ERR "%s: Invalid id (%d)\n", __func__, id + 1); + return; + } + mcbsp = id_to_mcbsp_ptr(id); + clk_disable(mcbsp->fclk); +} +EXPORT_SYMBOL(omap_mcbsp_disable_fclk); + + +void omap_mcbsp_enable_fclk(unsigned int id) +{ + struct omap_mcbsp *mcbsp; + + if (!omap_mcbsp_check_valid_id(id)) { + printk(KERN_ERR "%s: Invalid id (%d)\n", __func__, id + 1); + return; + } + mcbsp = id_to_mcbsp_ptr(id); + clk_enable(mcbsp->fclk); +} +EXPORT_SYMBOL(omap_mcbsp_enable_fclk); + void omap_mcbsp_free(unsigned int id) { struct omap_mcbsp *mcbsp; diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index 45be942..e56b5ff 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -229,18 +229,28 @@ static int omap_mcbsp_dai_trigger(struct snd_pcm_substream *substream, int cmd,
switch (cmd) { case SNDRV_PCM_TRIGGER_START: - case SNDRV_PCM_TRIGGER_RESUME: case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: mcbsp_data->active++; omap_mcbsp_start(mcbsp_data->bus_id, play, !play); break; + case SNDRV_PCM_TRIGGER_RESUME: + mcbsp_data->active++; + omap_mcbsp_enable_fclk(mcbsp_data->bus_id); + omap_mcbsp_config(mcbsp_data->bus_id, + &mcbsp_data->regs); + omap_mcbsp_start(mcbsp_data->bus_id, play, !play); + break;
case SNDRV_PCM_TRIGGER_STOP: - case SNDRV_PCM_TRIGGER_SUSPEND: case SNDRV_PCM_TRIGGER_PAUSE_PUSH: omap_mcbsp_stop(mcbsp_data->bus_id, play, !play); mcbsp_data->active--; break; + case SNDRV_PCM_TRIGGER_SUSPEND: + omap_mcbsp_stop(mcbsp_data->bus_id, play, !play); + omap_mcbsp_disable_fclk(mcbsp_data->bus_id); + mcbsp_data->active--; + break; default: err = -EINVAL; }
On 13 Nov 2009, at 17:39, Jane Wang jwang@ti.com wrote:
McBSP fucntional clock is not disabled when suspend is triggerd which prevents PER domain entering target low-power state. To fix the problem:
Disable/enable McBSP functional clock for suspend/resume.
In addition, reconfigure McBSP, this is needed when systerm comes
out of OFF state.
There is no need to resend your patches this iften - please allow at least a week or so for review. You only need to repost if addressing comments or it looks like the patch got missed.
Signed-off-by: Jane Wang jwang@ti.com
arch/arm/plat-omap/include/mach/mcbsp.h | 2 ++ arch/arm/plat-omap/mcbsp.c | 27 ++++++++++++++++++++++ +++++ sound/soc/omap/omap-mcbsp.c | 14 ++++++++++++-- 3 files changed, 41 insertions(+), 2 deletions(-)
diff --git a/arch/arm/plat-omap/include/mach/mcbsp.h b/arch/arm/plat- omap/include/mach/mcbsp.h index e0d6eca..94e1c66 100644 --- a/arch/arm/plat-omap/include/mach/mcbsp.h +++ b/arch/arm/plat-omap/include/mach/mcbsp.h @@ -440,6 +440,8 @@ static inline int omap_mcbsp_get_dma_op_mode (unsigned int id) { return 0; } #endif int omap_mcbsp_request(unsigned int id); void omap_mcbsp_free(unsigned int id); +void omap_mcbsp_disable_fclk(unsigned int id); +void omap_mcbsp_enable_fclk(unsigned int id); void omap_mcbsp_start(unsigned int id, int tx, int rx); void omap_mcbsp_stop(unsigned int id, int tx, int rx); void omap_mcbsp_xmit_word(unsigned int id, u32 word); diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c index e664b91..bfe3c61 100644 --- a/arch/arm/plat-omap/mcbsp.c +++ b/arch/arm/plat-omap/mcbsp.c @@ -459,6 +459,33 @@ int omap_mcbsp_request(unsigned int id) } EXPORT_SYMBOL(omap_mcbsp_request);
+void omap_mcbsp_disable_fclk(unsigned int id) +{
- struct omap_mcbsp *mcbsp;
- if (!omap_mcbsp_check_valid_id(id)) {
printk(KERN_ERR "%s: Invalid id (%d)\n", __func__, id + 1);return;- }
- mcbsp = id_to_mcbsp_ptr(id);
- clk_disable(mcbsp->fclk);
+} +EXPORT_SYMBOL(omap_mcbsp_disable_fclk);
+void omap_mcbsp_enable_fclk(unsigned int id) +{
- struct omap_mcbsp *mcbsp;
- if (!omap_mcbsp_check_valid_id(id)) {
printk(KERN_ERR "%s: Invalid id (%d)\n", __func__, id + 1);return;- }
- mcbsp = id_to_mcbsp_ptr(id);
- clk_enable(mcbsp->fclk);
+} +EXPORT_SYMBOL(omap_mcbsp_enable_fclk);
void omap_mcbsp_free(unsigned int id) { struct omap_mcbsp *mcbsp; diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index 45be942..e56b5ff 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -229,18 +229,28 @@ static int omap_mcbsp_dai_trigger(struct snd_pcm_substream *substream, int cmd,
switch (cmd) { case SNDRV_PCM_TRIGGER_START:
- case SNDRV_PCM_TRIGGER_RESUME: case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: mcbsp_data->active++; omap_mcbsp_start(mcbsp_data->bus_id, play, !play); break;
case SNDRV_PCM_TRIGGER_RESUME:
mcbsp_data->active++;omap_mcbsp_enable_fclk(mcbsp_data->bus_id);omap_mcbsp_config(mcbsp_data->bus_id,&mcbsp_data->regs);omap_mcbsp_start(mcbsp_data->bus_id, play, !play);break;case SNDRV_PCM_TRIGGER_STOP:
- case SNDRV_PCM_TRIGGER_SUSPEND: case SNDRV_PCM_TRIGGER_PAUSE_PUSH: omap_mcbsp_stop(mcbsp_data->bus_id, play, !play); mcbsp_data->active--; break;
- case SNDRV_PCM_TRIGGER_SUSPEND:
omap_mcbsp_stop(mcbsp_data->bus_id, play, !play);omap_mcbsp_disable_fclk(mcbsp_data->bus_id);mcbsp_data->active--;break;-- 1.5.4.3
-----Original Message----- From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel-bounces@alsa- project.org] On Behalf Of Mark Brown Sent: Friday, November 13, 2009 11:49 AM To: Wang, Jane Cc: alsa-devel@alsa-project.org; linux-omap@vger.kernel.org; peter.ujfalusi@nokia.com; Wang, Jane Subject: Re: [alsa-devel] [PATCH 1/2] OMAP3: MCBSP: Suspend/resume failure fix
On 13 Nov 2009, at 17:39, Jane Wang jwang@ti.com wrote:
McBSP fucntional clock is not disabled when suspend is triggerd which prevents PER domain entering target low-power state. To fix the problem:
Disable/enable McBSP functional clock for suspend/resume.
In addition, reconfigure McBSP, this is needed when systerm comes
out of OFF state.
There is no need to resend your patches this iften - please allow at least a week or so for review. You only need to repost if addressing comments or it looks like the patch got missed.
Sorry for sending this again. Something is not set correctly with my git send-email. The email did not reach the mailing lists, it only reached people in cc.
On 13 Nov 2009, at 17:52, "Wang, Jane" jwang@ti.com wrote:
-----Original Message----- From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel- bounces@alsa- project.org] On Behalf Of Mark Brown Sent: Friday, November 13, 2009 11:49 AM To: Wang, Jane Cc: alsa-devel@alsa-project.org; linux-omap@vger.kernel.org; peter.ujfalusi@nokia.com; Wang, Jane Subject: Re: [alsa-devel] [PATCH 1/2] OMAP3: MCBSP: Suspend/resume failure fix
On 13 Nov 2009, at 17:39, Jane Wang jwang@ti.com wrote:
McBSP fucntional clock is not disabled when suspend is triggerd which prevents PER domain entering target low-power state. To fix the problem:
Disable/enable McBSP functional clock for suspend/resume.
In addition, reconfigure McBSP, this is needed when systerm comes
out of OFF state.
There is no need to resend your patches this iften - please allow at least a week or so for review. You only need to repost if addressing comments or it looks like the patch got missed.
Sorry for sending this again. Something is not set correctly with my git send-email. The email did not reach the mailing lists, it only reached people in cc.
No, your posts are making it through to the lists just fine. Note that alsa-devel is moderated for non-subscribers
Hi
On Fri, 13 Nov 2009 11:39:47 -0600 Jane Wang jwang@ti.com wrote:
McBSP fucntional clock is not disabled when suspend is triggerd which prevents PER domain entering target low-power state. To fix the problem:
Disable/enable McBSP functional clock for suspend/resume.
In addition, reconfigure McBSP, this is needed when systerm comes out of OFF state.
...
--- a/arch/arm/plat-omap/include/mach/mcbsp.h +++ b/arch/arm/plat-omap/include/mach/mcbsp.h @@ -440,6 +440,8 @@ static inline int omap_mcbsp_get_dma_op_mode(unsigned int id) { return 0; } #endif int omap_mcbsp_request(unsigned int id); void omap_mcbsp_free(unsigned int id); +void omap_mcbsp_disable_fclk(unsigned int id); +void omap_mcbsp_enable_fclk(unsigned int id);
...
--- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -229,18 +229,28 @@ static int omap_mcbsp_dai_trigger(struct snd_pcm_substream *substream, int cmd,
switch (cmd) { case SNDRV_PCM_TRIGGER_START:
- case SNDRV_PCM_TRIGGER_RESUME: case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: mcbsp_data->active++; omap_mcbsp_start(mcbsp_data->bus_id, play, !play); break;
- case SNDRV_PCM_TRIGGER_RESUME:
mcbsp_data->active++;omap_mcbsp_enable_fclk(mcbsp_data->bus_id);omap_mcbsp_config(mcbsp_data->bus_id,&mcbsp_data->regs);omap_mcbsp_start(mcbsp_data->bus_id, play, !play);break;
For me this looks more like a simple workaround for the audio case than comprehensive implementation to McBSP OFF mode context save/restore. My thoughts:
1. McBSP clock management partially moved out of low-level driver 2. McBSP client must do the context restore
I do know that arch/arm/plat-omap/mcbsp.c enables the clocks in omap_mcbsp_request, I don't know do some API or feature there really require clocks (or McBSP itself) to be active just after the request time but I would look that path instead (even it is more complicated).
If no API or feature would require active McBSP block before the omap_mcbsp_start call, then the clock management could be done inside the omap_mcbsp_start/-stop functions.
Also context save/restore operations belongs more to PM callbacks of .../plat-omap/mcbsp.c. Or probably that can be done also dynamically inside the omap_mcbsp_start/-stop for keeping the whole McBSP shutdown whenever it is idle.
I think it's worth to look McBSP register cache concept [1] from Janusz Krzysztofik would it help all of this context save/restore operations.
-----Original Message----- From: linux-omap-owner@vger.kernel.org [mailto:linux-omap- owner@vger.kernel.org] On Behalf Of Jarkko Nikula Sent: Saturday, November 14, 2009 1:50 PM To: Wang, Jane Cc: alsa-devel@alsa-project.org; linux-omap@vger.kernel.org; peter.ujfalusi@nokia.com; broonie@opensource.wolfsonmicro.com Subject: Re: [PATCH 1/2] OMAP3: MCBSP: Suspend/resume failure fix
Hi
On Fri, 13 Nov 2009 11:39:47 -0600 Jane Wang jwang@ti.com wrote:
McBSP fucntional clock is not disabled when suspend is triggerd which prevents PER domain entering target low-power state. To fix the problem:
Disable/enable McBSP functional clock for suspend/resume.
In addition, reconfigure McBSP, this is needed when systerm comes out
of OFF state.
...
--- a/arch/arm/plat-omap/include/mach/mcbsp.h +++ b/arch/arm/plat-omap/include/mach/mcbsp.h @@ -440,6 +440,8 @@ static inline int
omap_mcbsp_get_dma_op_mode(unsigned int id) { return 0; }
#endif int omap_mcbsp_request(unsigned int id); void omap_mcbsp_free(unsigned int id); +void omap_mcbsp_disable_fclk(unsigned int id); +void omap_mcbsp_enable_fclk(unsigned int id);
...
--- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -229,18 +229,28 @@ static int omap_mcbsp_dai_trigger(struct
snd_pcm_substream *substream, int cmd,
switch (cmd) { case SNDRV_PCM_TRIGGER_START:
- case SNDRV_PCM_TRIGGER_RESUME: case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: mcbsp_data->active++; omap_mcbsp_start(mcbsp_data->bus_id, play, !play); break;
- case SNDRV_PCM_TRIGGER_RESUME:
mcbsp_data->active++;omap_mcbsp_enable_fclk(mcbsp_data->bus_id);omap_mcbsp_config(mcbsp_data->bus_id,&mcbsp_data->regs);omap_mcbsp_start(mcbsp_data->bus_id, play, !play);break;For me this looks more like a simple workaround for the audio case than comprehensive implementation to McBSP OFF mode context save/restore. My thoughts:
- McBSP clock management partially moved out of low-level driver
- McBSP client must do the context restore
I do know that arch/arm/plat-omap/mcbsp.c enables the clocks in omap_mcbsp_request, I don't know do some API or feature there really require clocks (or McBSP itself) to be active just after the request time but I would look that path instead (even it is more complicated).
If no API or feature would require active McBSP block before the omap_mcbsp_start call, then the clock management could be done inside the omap_mcbsp_start/-stop functions.
[Aggarwal, Anuj] I tried doing that but it didn't work because before _start, we need to call set_hw_params which configures mcbsp, for which clocks are required.
Also context save/restore operations belongs more to PM callbacks of .../plat-omap/mcbsp.c. Or probably that can be done also dynamically inside the omap_mcbsp_start/-stop for keeping the whole McBSP shutdown whenever it is idle.
[Aggarwal, Anuj] I am sorry I couldn't understand that. Did you mean that PM hooks can be added in the mcbsp driver which can be registered and called As and when required? Any reference driver for this, if my understanding is correct?
I think it's worth to look McBSP register cache concept [1] from Janusz Krzysztofik would it help all of this context save/restore operations.
[Aggarwal, Anuj] I tried these two patches on omap3 evm but they didn't work for me. The system doesn't go to the suspend state when I do: echo mem > /sys/power/state And the culprits were core (think sdma) and per domain. I tried disabling the mcbsp i/f clock as well but it didn't help. On mcbsp register dump, I couldn't find the sysconfig register getting configured. Could that be the root cause?
-- Jarkko
-- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Also context save/restore operations belongs more to PM callbacks of .../plat-omap/mcbsp.c. Or probably that can be done also dynamically inside the omap_mcbsp_start/-stop for keeping the whole McBSP shutdown whenever it is idle.
[Aggarwal, Anuj] I am sorry I couldn't understand that. Did you mean that PM hooks can be added in the mcbsp driver which can be registered and called As and when required? Any reference driver for this, if my understanding is correct?
I think it's worth to look McBSP register cache concept [1] from Janusz Krzysztofik would it help all of this context save/restore operations.
[Aggarwal, Anuj] I tried these two patches on omap3 evm but they didn't work for me. The system doesn't go to the suspend state when I do: echo mem > /sys/power/state And the culprits were core (think sdma) and per domain. I tried disabling the mcbsp i/f clock as well but it didn't help. On mcbsp register dump, I couldn't find the sysconfig register getting configured. Could that be the root cause?
Did you include this patch? http://git.kernel.org/?p=linux/kernel/git/broonie/sound-2.6.git;a=commit;h=9...
Without this patch, omap_stop_dma() did not disable DMA channel.
Regards,
-Jane
On Wed, Nov 18, 2009 at 8:28 PM, Aggarwal, Anuj anuj.aggarwal@ti.comwrote:
If no API or feature would require active McBSP block before the omap_mcbsp_start call, then the clock management could be done inside the omap_mcbsp_start/-stop functions.
[Aggarwal, Anuj] I tried doing that but it didn't work because before _start, we need to call set_hw_params which configures mcbsp, for which clocks are required.
First modification to .../plat-omap/mcbsp.c could be just a patch which keeps the clocks on only when needed. I.e. keep them disabled when returning from request and activate them only when accessing the McBSP registers or when it is running.
But that said, I don't know are there some use case or API which requires active clocks after request so that should be checked.
Also context save/restore operations belongs more to PM callbacks of .../plat-omap/mcbsp.c. Or probably that can be done also dynamically inside the omap_mcbsp_start/-stop for keeping the whole McBSP shutdown whenever it is idle.
[Aggarwal, Anuj] I am sorry I couldn't understand that. Did you mean that PM hooks can be added in the mcbsp driver which can be registered and called As and when required? Any reference driver for this, if my understanding is correct?
Yep, the PM hooks in .../plat-omap/mcbsp.c should take care of saving the McBSP registers before going into suspend or restore them back when resuming. This would be second patch after the clock change patch.
The comment about dynamic PM was an idea that if McBSP could be unpowered whenever it's not running and do the resume only when calling omap_mcbsp_start. But that would be some future improvements after basic suspend/resume functionality.
I think it's worth to look McBSP register cache concept [1] from Janusz
Krzysztofik would it help all of this context save/restore operations.
[Aggarwal, Anuj] I tried these two patches on omap3 evm but they didn't work for me. The system doesn't go to the suspend state when I do: echo mem > /sys/power/state And the culprits were core (think sdma) and per domain. I tried disabling the mcbsp i/f clock as well but it didn't help. On mcbsp register dump, I couldn't find the sysconfig register getting configured. Could that be the root cause?
Does it go into suspend if the audio is not running (i.e. McBSP not requested) or if you don't even compile the McBSP support? I haven't looked are there checks for active clocks in PM core but PM is easier to start with bare minimum and to see that system can do suspend/resume and then add more features one by one.
On Thu, Nov 19, 2009 at 1:04 PM, Jarkko Nikula jhnikula@gmail.com wrote:
On Wed, Nov 18, 2009 at 8:28 PM, Aggarwal, Anuj anuj.aggarwal@ti.comwrote:
If no API or feature would require active McBSP block before the omap_mcbsp_start call, then the clock management could be done inside the omap_mcbsp_start/-stop functions.
[Aggarwal, Anuj] I tried doing that but it didn't work because before _start, we need to call set_hw_params which configures mcbsp, for which clocks are required.
First modification to .../plat-omap/mcbsp.c could be just a patch which keeps the clocks on only when needed. I.e. keep them disabled when returning from request and activate them only when accessing the McBSP registers or when it is running.
But that said, I don't know are there some use case or API which requires active clocks after request so that should be checked.
Also context save/restore operations belongs more to PM callbacks of .../plat-omap/mcbsp.c. Or probably that can be done also dynamically inside the omap_mcbsp_start/-stop for keeping the whole McBSP shutdown whenever it is idle.
[Aggarwal, Anuj] I am sorry I couldn't understand that. Did you mean that PM hooks can be added in the mcbsp driver which can be registered and called As and when required? Any reference driver for this, if my understanding is correct?
Yep, the PM hooks in .../plat-omap/mcbsp.c should take care of saving the McBSP registers before going into suspend or restore them back when resuming. This would be second patch after the clock change patch.
The comment about dynamic PM was an idea that if McBSP could be unpowered whenever it's not running and do the resume only when calling omap_mcbsp_start. But that would be some future improvements after basic suspend/resume functionality.
I think it's worth to look McBSP register cache concept [1] from Janusz
Krzysztofik would it help all of this context save/restore operations.
[Aggarwal, Anuj] I tried these two patches on omap3 evm but they didn't work for me. The system doesn't go to the suspend state when I do: echo mem > /sys/power/state And the culprits were core (think sdma) and per domain. I tried disabling the mcbsp i/f clock as well but it didn't help. On mcbsp register dump, I couldn't find the sysconfig register getting configured. Could that be the root cause?
Does it go into suspend if the audio is not running (i.e. McBSP not requested) or if you don't even compile the McBSP support? I haven't looked are there checks for active clocks in PM core but PM is easier to start with bare minimum and to see that system can do suspend/resume and then add more features one by one.
[Anuj] Yes, w/o audio, it goes into suspend neatly. After going through the entire patch series "[PATCHv5 00/20] OMAP ASoC changes in DMA utilization", which I missed earlier, I found that the sysfs entry dma_op_mode can be toggled among element/frame/threshold, by default which is element hence not allowing the mcbsp driver to go into idle state. I changed that at run time and after that I am able to hit the suspend state while playing back audio (at least the log doesn't show that the system is not able to hit suspend). My question is: is sysfs the only way to choose threshold or can it be made as default? I could not see that happening in the code so thinking of the possible reason of choosing not to do so. Because the user who wants to hit suspend will not like to issue driver specific commands. Any specific reason of not doing that by default?
-- Jarkko _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
On Thursday 19 November 2009 21:48:26 ext Anuj Aggarwal wrote:
[Anuj] Yes, w/o audio, it goes into suspend neatly. After going through the entire patch series "[PATCHv5 00/20] OMAP ASoC changes in DMA utilization", which I missed earlier, I found that the sysfs entry dma_op_mode can be toggled among element/frame/threshold, by default which is element hence not allowing the mcbsp driver to go into idle state. I changed that at run time and after that I am able to hit the suspend state while playing back audio (at least the log doesn't show that the system is not able to hit suspend). My question is: is sysfs the only way to choose threshold or can it be made as default? I could not see that happening in the code so thinking of the possible reason of choosing not to do so. Because the user who wants to hit suspend will not like to issue driver specific commands. Any specific reason of not doing that by default?
It is not selected as default on OMAP3, since it is a new feature, and we don't want to change the behavior of the audio. If the reports are positive regarding to the threshold mode, than we can make it as default on OMAP3, at least that is the plan AFAIK. In some cases the element mode is desirable over the threshold mode, and probably there are additional modes can be implemented for other cases, hence the sysfs interface is really useful feature.
Back than there were quite a bit of discussion here about the threshold mode: in the original series the threshold mode was the default, but we decided to not to do that before others have the chance to test it. We have been using the threshold mode without issues internally, so it should be really stable, but it is still a new mode, which can break other implementations.
So please test the threshold mode in your HW, and report if it is working there OK, if we have enough positive feedbacks, than we can make it as default.
On Fri, 20 Nov 2009 09:51:13 +0200 Peter Ujfalusi peter.ujfalusi@nokia.com wrote:
My question is: is sysfs the only way to choose threshold or can it be made as default? I could not see that happening in the code so thinking of the possible reason of choosing not to do so. Because the user who wants to hit suspend will not like to issue driver specific commands. Any specific reason of not doing that by default?
It is not selected as default on OMAP3, since it is a new feature, and we don't want to change the behavior of the audio. If the reports are positive regarding to the threshold mode, than we can make it as default on OMAP3, at least that is the plan AFAIK.
Yep. Currently we want to keep DMA behaviour consistent across the OMAPs and McBSP ports since the large internal FIFO is found only from McBSP2 on OMAP3.
This is good finding that you have found that it's the audio preventing the suspend and the threshold transfer mode can make it hit better.
But anyway, I'm afraid that threshold mode doesn't help to create robust suspend. Threshold allow the DMA and core to be mode in idle between the FIFO fills and probably that's why suspend might work during these idle periods. For robust implementation there must be explicit code stopping and resuming all activity gracefully so that it can hit the suspend also if the fill is happening or if using another transfer mode.
On Fri, 2009-11-20 at 14:53 +0100, ext Jarkko Nikula wrote:
On Fri, 20 Nov 2009 09:51:13 +0200 Peter Ujfalusi peter.ujfalusi@nokia.com wrote:
My question is: is sysfs the only way to choose threshold or can it be made as default? I could not see that happening in the code so thinking of the possible reason of choosing not to do so. Because the user who wants to hit suspend will not like to issue driver specific commands. Any specific reason of not doing that by default?
It is not selected as default on OMAP3, since it is a new feature, and we don't want to change the behavior of the audio. If the reports are positive regarding to the threshold mode, than we can make it as default on OMAP3, at least that is the plan AFAIK.
Yep. Currently we want to keep DMA behaviour consistent across the OMAPs and McBSP ports since the large internal FIFO is found only from McBSP2 on OMAP3.
This is good finding that you have found that it's the audio preventing the suspend and the threshold transfer mode can make it hit better.
But anyway, I'm afraid that threshold mode doesn't help to create robust suspend. Threshold allow the DMA and core to be mode in idle between the FIFO fills and probably that's why suspend might work during these idle periods. For robust implementation there must be explicit code stopping and resuming all activity gracefully so that it can hit the suspend also if the fill is happening or if using another transfer mode.
Looking at the very original patch, I don't know how things could get into deep sleep by disabling the fclk only... need to disable the iclk also. In threshold mode, HW autogates the iclk, so you may be just "lucky".
One may want to be aware of this also:
http://git.kernel.org/?p=linux/kernel/git/tmlind/linux-omap-2.6.git;a=commit...
I think it's an undocumented feature.
- Eero
On Fri, Nov 20, 2009 at 7:39 PM, Eero Nurkkala ext-eero.nurkkala@nokia.com wrote:
On Fri, 2009-11-20 at 14:53 +0100, ext Jarkko Nikula wrote:
On Fri, 20 Nov 2009 09:51:13 +0200 Peter Ujfalusi peter.ujfalusi@nokia.com wrote:
My question is: is sysfs the only way to choose threshold or can it be made as default? I could not see that happening in the code so thinking of the possible reason of choosing not to do so. Because the user who wants to hit suspend will not like to issue driver specific commands. Any specific reason of not doing that by default?
It is not selected as default on OMAP3, since it is a new feature, and we don't want to change the behavior of the audio. If the reports are positive regarding to the threshold mode, than we can make it as default on OMAP3, at least that is the plan AFAIK.
Yep. Currently we want to keep DMA behaviour consistent across the OMAPs and McBSP ports since the large internal FIFO is found only from McBSP2 on OMAP3.
This is good finding that you have found that it's the audio preventing the suspend and the threshold transfer mode can make it hit better.
But anyway, I'm afraid that threshold mode doesn't help to create robust suspend. Threshold allow the DMA and core to be mode in idle between the FIFO fills and probably that's why suspend might work during these idle periods. For robust implementation there must be explicit code stopping and resuming all activity gracefully so that it can hit the suspend also if the fill is happening or if using another transfer mode.
Looking at the very original patch, I don't know how things could get into deep sleep by disabling the fclk only... need to disable the iclk also. In threshold mode, HW autogates the iclk, so you may be just "lucky".
[Anuj] No, I am not :(. I had to modify the original patch to include the disable part for the ICLK too. Without that, I was not able to hit retention. I will try to do some further regression on OMAP3 EVM and post my findings. As of now, audio is working fine with these patches + ICLK modification.
One may want to be aware of this also:
http://git.kernel.org/?p=linux/kernel/git/tmlind/linux-omap-2.6.git;a=commit...
I think it's an undocumented feature.
- Eero
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Looking at the very original patch, I don't know how things could get into deep sleep by disabling the fclk only... need to disable the iclk also. In threshold mode, HW autogates the iclk, so you may be just "lucky".
[Anuj] No, I am not :(. I had to modify the original patch to include the disable part for the ICLK too. Without that, I was not able to hit retention. I will try to do some further regression on OMAP3 EVM and post my findings. As of now, audio is working fine with these patches + ICLK modification.
[Aggarwal, Anuj] After further testing, I found that there is some problem in the capture path. While capturing in background, if I tried to do suspend, capture fails after a few seconds giving; arecord: pcm_read:1617: read error: Input/output error This I was discussing in another thread (*) for AIC23/AM3517 on NFS but now I realized after finishing my tests that this behavior is common for both omap3530/twl4030 and am3517/aic23. Moreover, the problem doesn't depend on the underlying file system - both NFS and ramdisk produce the same error. To make matter worse, if suspend/resume is tried while audio loopback is running, system just hangs. Even telnet to the evm doesn't work. So the audio capture path, from suspend/resume point of view, definitely needs some more time. I will continue debugging at my end. But pointers are always welcome.
*) http://www.mail-archive.com/linux-omap@vger.kernel.org/msg19845.html
One may want to be aware of this also:
http://git.kernel.org/?p=linux/kernel/git/tmlind/linux-omap-
2.6.git;a=commitdiff;h=72cc6d715d5b018e2cff4adb68966855850d4e77
I think it's an undocumented feature.
-----Original Message----- From: Aggarwal, Anuj Sent: Monday, November 30, 2009 5:21 PM To: ext-eero.nurkkala@nokia.com Cc: ext Jarkko Nikula; alsa-devel@alsa-project.org; Wang, Jane; broonie@opensource.wolfsonmicro.com; Ujfalusi Peter (Nokia-D/Tampere); linux-omap@vger.kernel.org Subject: RE: [alsa-devel] [PATCH 1/2] OMAP3: MCBSP: Suspend/resume failure fix
Looking at the very original patch, I don't know how things could get into deep sleep by disabling the fclk only... need to disable the iclk also. In threshold mode, HW autogates the iclk, so you may be just "lucky".
[Anuj] No, I am not :(. I had to modify the original patch to include the disable part for the ICLK too. Without that, I was not able to hit retention. I will try to do some further regression on OMAP3 EVM and post my findings. As of now, audio is working fine with these patches + ICLK modification.
[Aggarwal, Anuj] After further testing, I found that there is some problem in the capture path. While capturing in background, if I tried to do suspend, capture fails after a few seconds giving; arecord: pcm_read:1617: read error: Input/output error This I was discussing in another thread (*) for AIC23/AM3517 on NFS but now I realized after finishing my tests that this behavior is common for both omap3530/twl4030 and am3517/aic23. Moreover, the problem doesn't depend on the underlying file system - both NFS and ramdisk produce the same error. To make matter worse, if suspend/resume is tried while audio loopback is running, system just hangs. Even telnet to the evm doesn't work. So the audio capture path, from suspend/resume point of view, definitely needs some more time. I will continue debugging at my end. But pointers are always welcome.
[Aggarwal, Anuj] I think I found one possible root cause of the hang which is observed if suspend/resume is tried while audio loopback is going on.
soc_suspend() calls snd_pcm_suspend_all() which calls snd_pcm_suspend() for all the substreams in the pcm->streams[]. On debugging, I found that I am not able to come out of snd_pcm_suspend_all(). I think this API needs some fix as the comment also suggests: FIXME: the open/close code should lock this as well
Could this be the root cause of this hang? Any pointers on what needs to be fixed?
-----Original Message----- From: Aggarwal, Anuj Sent: Monday, November 30, 2009 5:21 PM To: ext-eero.nurkkala@nokia.com Cc: ext Jarkko Nikula; alsa-devel@alsa-project.org; Wang, Jane; broonie@opensource.wolfsonmicro.com; Ujfalusi Peter (Nokia-D/Tampere); linux-omap@vger.kernel.org Subject: RE: [alsa-devel] [PATCH 1/2] OMAP3: MCBSP: Suspend/resume
failure
fix
Looking at the very original patch, I don't know how things could
get
into deep sleep by disabling the fclk only... need to disable the
iclk
also. In threshold mode, HW autogates the iclk, so you may be just "lucky".
[Anuj] No, I am not :(. I had to modify the original patch to include the disable part for the ICLK too. Without that, I was not able to hit retention. I will try to do some further regression on OMAP3 EVM and post my findings. As of now, audio is working fine with these patches + ICLK modification.
[Aggarwal, Anuj] After further testing, I found that there is some problem in the capture path. While capturing in background, if I tried to do suspend, capture fails after a few seconds giving; arecord: pcm_read:1617: read error: Input/output error This I was discussing in another thread (*) for AIC23/AM3517 on NFS but now I realized after finishing my tests that this behavior is common for both omap3530/twl4030 and am3517/aic23. Moreover, the problem doesn't depend on the underlying file system - both NFS and ramdisk produce the same error. To make matter worse, if suspend/resume is tried while audio loopback is running, system just hangs. Even telnet to the evm doesn't work. So the audio capture path, from suspend/resume point of view, definitely needs some more time. I will continue debugging at my end. But pointers are always welcome.
[Aggarwal, Anuj] I think I found one possible root cause of the hang which is observed if suspend/resume is tried while audio loopback is going on.
soc_suspend() calls snd_pcm_suspend_all() which calls snd_pcm_suspend() for all the substreams in the pcm->streams[]. On debugging, I found that I am not able to come out of snd_pcm_suspend_all(). I think this API needs some fix as the comment also suggests: FIXME: the open/close code should lock this as well
Could this be the root cause of this hang? Any pointers on what needs to be fixed?
[Aggarwal, Anuj] Can someone please confirm that the problem lies in this API? Also, any help on how that can be fixed would be great.
-- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
participants (8)
-
Aggarwal, Anuj -
Anuj Aggarwal
-
Eero Nurkkala -
Jane Wang -
Jarkko Nikula -
Mark Brown -
Peter Ujfalusi -
Wang, Jane