diff mbox

[08/11] ARM: OMAP3: Remove callback for McBSP sidetone ICLK workaround

Message ID 1344417101-5015-9-git-send-email-peter.ujfalusi@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Peter Ujfalusi Aug. 8, 2012, 9:11 a.m. UTC
We can achieve the same result with SIDLEMODE field within McBSP SYSCONFIG
register.
The ASoC driver has been modified to use this method and we can now remove
the workaround using the control module register.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 arch/arm/mach-omap2/mcbsp.c             |   26 --------------------------
 arch/arm/plat-omap/include/plat/mcbsp.h |    1 -
 2 files changed, 0 insertions(+), 27 deletions(-)

Comments

Jarkko Nikula Aug. 8, 2012, 1:25 p.m. UTC | #1
On 08/08/2012 12:11 PM, Peter Ujfalusi wrote:
> We can achieve the same result with SIDLEMODE field within McBSP SYSCONFIG
> register.
> The ASoC driver has been modified to use this method and we can now remove
> the workaround using the control module register.
> 
Are you sure it works without any changes to sidetone audio quality or
current consumption? What I remember idlemode was broken at least on
3430 and caused bad sidetone audio if autoidle was set and a little bit
higher current consumption (something like 2-3 mA higher where reference
consumption was around 10 mA) when McBSP was set to no-idle.

Here's the old thread:

http://www.mail-archive.com/linux-omap@vger.kernel.org/msg52024.html

I think I tested all of this by using arecord >/dev/null | aplay
/dev/zero with threshold based DMA transfers with all bells and whistles
set for dynamic retention idle in order to hear only McBSP sidetone
while measuring current consumption.

But I guess 2-3 mA higher consumption is ok if it simplifies SW a lot
and base consumption is anyway much higher than 10 mA when sidetone is
in real use (my test above was artificial where actual use is under a
phone call with heavy consumption from cellular modem etc).
Peter Ujfalusi Aug. 8, 2012, 2 p.m. UTC | #2
Hi Jarkko,

On 08/08/2012 04:25 PM, Jarkko Nikula wrote:
> On 08/08/2012 12:11 PM, Peter Ujfalusi wrote:
>> We can achieve the same result with SIDLEMODE field within McBSP SYSCONFIG
>> register.
>> The ASoC driver has been modified to use this method and we can now remove
>> the workaround using the control module register.
>>
> Are you sure it works without any changes to sidetone audio quality or
> current consumption? What I remember idlemode was broken at least on
> 3430 and caused bad sidetone audio if autoidle was set and a little bit
> higher current consumption (something like 2-3 mA higher where reference
> consumption was around 10 mA) when McBSP was set to no-idle.

Yes, the idelmode settings are still broken in the sidetone block. However we
are toggling the SIDLEMODE of the corresponding McBSP instance where the ST
block is attached.
This should have the same effect as doing the same down in PRCM level since
the McBSP SIDLEMODE does work correctly preventing ICLK to be gated when it is
set to no-idle.

Unfortunately I can not get my BeagleBoard to start gating iclk even if I
remove the ICLK gating prevention (on top of 3.6-rc1). So I can not really say
for 100% this is equivalent to the the PRCM level workaround.
However I have also spend long time hacking around in McBSP, and I know that
the SIDLEMODE in McBSP is working correctly.
Do you know how can I get OMAP3 to start gating the clocks (to idle)?

> Here's the old thread:
> 
> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg52024.html
> 
> I think I tested all of this by using arecord >/dev/null | aplay
> /dev/zero with threshold based DMA transfers with all bells and whistles
> set for dynamic retention idle in order to hear only McBSP sidetone
> while measuring current consumption.

I'm doing the same test on BeagleBoard, but this thing does not want to hit
retention despite all the effort :(

> But I guess 2-3 mA higher consumption is ok if it simplifies SW a lot
> and base consumption is anyway much higher than 10 mA when sidetone is
> in real use (my test above was artificial where actual use is under a
> phone call with heavy consumption from cellular modem etc).

Well 2-3 mA is quite big number in my book...
I'm still not sure if I ever measured the difference between the PRCM level
autoidle prevention compared to the McBSP SIDLEMODE(no_idle).

Having said that, it is suspicious how we ended up doing the ICLK fixup in
PRCM level...
Jarkko Nikula Aug. 10, 2012, 1 p.m. UTC | #3
Hi Peter

On 08/08/2012 05:00 PM, Peter Ujfalusi wrote:
>> Are you sure it works without any changes to sidetone audio quality or
>> current consumption? What I remember idlemode was broken at least on
>> 3430 and caused bad sidetone audio if autoidle was set and a little bit
>> higher current consumption (something like 2-3 mA higher where reference
>> consumption was around 10 mA) when McBSP was set to no-idle.
> 
> Yes, the idelmode settings are still broken in the sidetone block. However we
> are toggling the SIDLEMODE of the corresponding McBSP instance where the ST
> block is attached.
> This should have the same effect as doing the same down in PRCM level since
> the McBSP SIDLEMODE does work correctly preventing ICLK to be gated when it is
> set to no-idle.
> 
> Unfortunately I can not get my BeagleBoard to start gating iclk even if I
> remove the ICLK gating prevention (on top of 3.6-rc1). So I can not really say
> for 100% this is equivalent to the the PRCM level workaround.
> However I have also spend long time hacking around in McBSP, and I know that
> the SIDLEMODE in McBSP is working correctly.
> Do you know how can I get OMAP3 to start gating the clocks (to idle)?
> 
I got some odd current consumption results from Beagle to really compare
different kernel versions but I was able get sensible results from N900.

I tested your set on top of 196973c of sound.git.

First consumption when using threshold based transfer has remained the
same between 2.6.38-3.6.0-rc1 (wau!). Active sidetone under "arecord -f
dat >/dev/null |aplay -f dat /dev/zero" test increases consumption from
plain playback about 17 mA in 3.4 and 3.6.0-rc1.

With your set applied consumption increases 28 mA when the sidetone is
active (i.e. +11 mA higher than on top of 196973c). Plain threshold
based playback remains still the same.

> I'm doing the same test on BeagleBoard, but this thing does not want to hit
> retention despite all the effort :(
> 
Could it be display subsystem which keeps it active? At least on N900 I
need to get DSS drivers activated in order to be able to blank the
display after bootloader and thus be able to reach the retention idle.
Peter Ujfalusi Aug. 10, 2012, 3:39 p.m. UTC | #4
Hi Jarkko,

On 08/10/2012 04:00 PM, Jarkko Nikula wrote:
> I got some odd current consumption results from Beagle to really compare
> different kernel versions but I was able get sensible results from N900.

I still use my n900 as a main phone so I can not really hack on it :(

> I tested your set on top of 196973c of sound.git.
> 
> First consumption when using threshold based transfer has remained the
> same between 2.6.38-3.6.0-rc1 (wau!).

Cool!

> Active sidetone under "arecord -f
> dat >/dev/null |aplay -f dat /dev/zero" test increases consumption from
> plain playback about 17 mA in 3.4 and 3.6.0-rc1.
> 
> With your set applied consumption increases 28 mA when the sidetone is
> active (i.e. +11 mA higher than on top of 196973c). Plain threshold
> based playback remains still the same.

Oh. 11mA is huge... Really strange. I would expected smaller (actually zero)
difference since in essence both workaround should be doing the same.
Only if the OMAP pm core making different decisions runtime when it see that
the no-idle is selected in the SYSCONFIG register of McBSP2. But the
autogating of clocks should work without SW.

So it seams that we need to do this down at PRCM level?

>> I'm doing the same test on BeagleBoard, but this thing does not want to hit
>> retention despite all the effort :(
>>
> Could it be display subsystem which keeps it active? At least on N900 I
> need to get DSS drivers activated in order to be able to blank the
> display after bootloader and thus be able to reach the retention idle.

I tried that as well, but nothing helps. It must be something else which I
have overlooked...
diff mbox

Patch

diff --git a/arch/arm/mach-omap2/mcbsp.c b/arch/arm/mach-omap2/mcbsp.c
index d57a357..996a15f 100644
--- a/arch/arm/mach-omap2/mcbsp.c
+++ b/arch/arm/mach-omap2/mcbsp.c
@@ -26,31 +26,6 @@ 
 #include <plat/omap_device.h>
 #include <linux/pm_runtime.h>
 
-/*
- * FIXME: Find a mechanism to enable/disable runtime the McBSP ICLK autoidle.
- * Sidetone needs non-gated ICLK and sidetone autoidle is broken.
- */
-#include "cm2xxx_3xxx.h"
-#include "cm-regbits-34xx.h"
-
-static int omap3_enable_st_clock(unsigned int id, bool enable)
-{
-	unsigned int w;
-
-	/*
-	 * Sidetone uses McBSP ICLK - which must not idle when sidetones
-	 * are enabled or sidetones start sounding ugly.
-	 */
-	w = omap2_cm_read_mod_reg(OMAP3430_PER_MOD, CM_AUTOIDLE);
-	if (enable)
-		w &= ~(1 << (id - 2));
-	else
-		w |= 1 << (id - 2);
-	omap2_cm_write_mod_reg(w, OMAP3430_PER_MOD, CM_AUTOIDLE);
-
-	return 0;
-}
-
 static int __init omap_init_mcbsp(struct omap_hwmod *oh, void *unused)
 {
 	int id, count = 1;
@@ -98,7 +73,6 @@  static int __init omap_init_mcbsp(struct omap_hwmod *oh, void *unused)
 	if (oh->dev_attr) {
 		oh_device[1] = omap_hwmod_lookup((
 		(struct omap_mcbsp_dev_attr *)(oh->dev_attr))->sidetone);
-		pdata->enable_st_clock = omap3_enable_st_clock;
 		count++;
 	}
 	pdev = omap_device_build_ss(name, id, oh_device, count, pdata,
diff --git a/arch/arm/plat-omap/include/plat/mcbsp.h b/arch/arm/plat-omap/include/plat/mcbsp.h
index c78d90b..698ccd8 100644
--- a/arch/arm/plat-omap/include/plat/mcbsp.h
+++ b/arch/arm/plat-omap/include/plat/mcbsp.h
@@ -46,7 +46,6 @@  struct omap_mcbsp_platform_data {
 	/* McBSP platform and instance specific features */
 	bool has_wakeup; /* Wakeup capability */
 	bool has_ccr; /* Transceiver has configuration control registers */
-	int (*enable_st_clock)(unsigned int, bool);
 };
 
 /**