Re: [E3-hacking] [RFC][PATCH] ARM: OMAP: McBSP: Use register cache
Friday 04 September 2009 07:39:36 Eero Nurkkala wrote:
Register values should not get corrupted. Could you please point out the registers that may get corrupted? Do they get corrupted during audio play? I haven't seen corrupted register values.
Hi Eero, Please have a look at some debug log lines below. Those were generated on my Amstrad Delta videophone, using an application (asterisk softpbx with alsa channel) that can restart a receiver of a prevoiusly requested McBSP by calling omap_mcbsp_start() on user request. As you can see, corrupted values can happen to get read from any register that is expected to return predictable results. Sep 4 08:21:17 amsdelta user.debug kernel: [309910.170000] omap-mcbsp omap-mcbsp.1: mcbsp: **** McBSP1 regs **** Sep 4 08:21:17 amsdelta user.debug kernel: [309910.170000] omap-mcbsp omap-mcbsp.1: mcbsp: DRR2: 0x1000 Sep 4 08:21:17 amsdelta user.debug kernel: [309910.170000] omap-mcbsp omap-mcbsp.1: mcbsp: DRR1: 0x009d Sep 4 08:21:17 amsdelta user.debug kernel: [309910.170000] omap-mcbsp omap-mcbsp.1: mcbsp: DXR2: 0x3000 Sep 4 08:21:17 amsdelta user.debug kernel: [309910.170000] omap-mcbsp omap-mcbsp.1: mcbsp: DXR1: 0x7da1 Sep 4 08:21:17 amsdelta user.debug kernel: [309910.170000] omap-mcbsp omap-mcbsp.1: mcbsp: SPCR2: 0x02f0 Sep 4 08:21:17 amsdelta user.debug kernel: [309910.170000] omap-mcbsp omap-mcbsp.1: mcbsp: SPCR1: 0x7031 Sep 4 08:21:17 amsdelta user.debug kernel: [309910.170000] omap-mcbsp omap-mcbsp.1: mcbsp: RCR2: 0x0045 Sep 4 08:21:17 amsdelta user.debug kernel: [309910.170000] omap-mcbsp omap-mcbsp.1: mcbsp: RCR1: 0x0040 Sep 4 08:21:17 amsdelta user.debug kernel: [309910.170000] omap-mcbsp omap-mcbsp.1: mcbsp: XCR2: 0x1045 Sep 4 08:21:17 amsdelta user.debug kernel: [309910.170000] omap-mcbsp omap-mcbsp.1: mcbsp: XCR1: 0x0040 Sep 4 08:21:17 amsdelta user.debug kernel: [309910.170000] omap-mcbsp omap-mcbsp.1: mcbsp: SRGR2: 0x310f Sep 4 08:21:17 amsdelta user.debug kernel: [309910.170000] omap-mcbsp omap-mcbsp.1: mcbsp: SRGR1: 0x0000 Sep 4 08:21:17 amsdelta user.debug kernel: [309910.170000] omap-mcbsp omap-mcbsp.1: mcbsp: PCR0: 0x0003 Sep 4 08:21:17 amsdelta user.debug kernel: [309910.170000] omap-mcbsp omap-mcbsp.1: mcbsp: *********************** Sep 4 08:21:19 amsdelta user.debug kernel: [309912.050000] omap-mcbsp omap-mcbsp.1: mcbsp: **** McBSP1 regs **** Sep 4 08:21:19 amsdelta user.debug kernel: [309912.050000] omap-mcbsp omap-mcbsp.1: mcbsp: DRR2: 0x0000 Sep 4 08:21:19 amsdelta user.debug kernel: [309912.050000] omap-mcbsp omap-mcbsp.1: mcbsp: DRR1: 0x1086 Sep 4 08:21:19 amsdelta user.debug kernel: [309912.050000] omap-mcbsp omap-mcbsp.1: mcbsp: DXR2: 0x0000 Sep 4 08:21:19 amsdelta user.debug kernel: [309912.050000] omap-mcbsp omap-mcbsp.1: mcbsp: DXR1: 0x7da1 Sep 4 08:21:19 amsdelta user.debug kernel: [309912.050000] omap-mcbsp omap-mcbsp.1: mcbsp: SPCR2: 0x02f0 Sep 4 08:21:19 amsdelta user.debug kernel: [309912.050000] omap-mcbsp omap-mcbsp.1: mcbsp: SPCR1: 0x0031 Sep 4 08:21:19 amsdelta user.debug kernel: [309912.050000] omap-mcbsp omap-mcbsp.1: mcbsp: RCR2: 0x1045 Sep 4 08:21:19 amsdelta user.debug kernel: [309912.050000] omap-mcbsp omap-mcbsp.1: mcbsp: RCR1: 0x1040 Sep 4 08:21:19 amsdelta user.debug kernel: [309912.050000] omap-mcbsp omap-mcbsp.1: mcbsp: XCR2: 0x3045 Sep 4 08:21:19 amsdelta user.debug kernel: [309912.050000] omap-mcbsp omap-mcbsp.1: mcbsp: XCR1: 0x1040 Sep 4 08:21:19 amsdelta user.debug kernel: [309912.050000] omap-mcbsp omap-mcbsp.1: mcbsp: SRGR2: 0x000f Sep 4 08:21:19 amsdelta user.debug kernel: [309912.050000] omap-mcbsp omap-mcbsp.1: mcbsp: SRGR1: 0x0000 Sep 4 08:21:19 amsdelta user.debug kernel: [309912.050000] omap-mcbsp omap-mcbsp.1: mcbsp: PCR0: 0x1003 Sep 4 08:21:19 amsdelta user.debug kernel: [309912.050000] omap-mcbsp omap-mcbsp.1: mcbsp: *********************** Sep 4 08:21:21 amsdelta user.debug kernel: [309913.950000] omap-mcbsp omap-mcbsp.1: mcbsp: **** McBSP1 regs **** Sep 4 08:21:21 amsdelta user.debug kernel: [309913.950000] omap-mcbsp omap-mcbsp.1: mcbsp: DRR2: 0x1000 Sep 4 08:21:21 amsdelta user.debug kernel: [309913.950000] omap-mcbsp omap-mcbsp.1: mcbsp: DRR1: 0x109e Sep 4 08:21:21 amsdelta user.debug kernel: [309913.950000] omap-mcbsp omap-mcbsp.1: mcbsp: DXR2: 0x1000 Sep 4 08:21:21 amsdelta user.debug kernel: [309913.950000] omap-mcbsp omap-mcbsp.1: mcbsp: DXR1: 0x7da1 Sep 4 08:21:21 amsdelta user.debug kernel: [309913.950000] omap-mcbsp omap-mcbsp.1: mcbsp: SPCR2: 0x02f0 Sep 4 08:21:21 amsdelta user.debug kernel: [309913.950000] omap-mcbsp omap-mcbsp.1: mcbsp: SPCR1: 0x1031 Sep 4 08:21:21 amsdelta user.debug kernel: [309913.950000] omap-mcbsp omap-mcbsp.1: mcbsp: RCR2: 0x1045 Sep 4 08:21:21 amsdelta user.debug kernel: [309913.950000] omap-mcbsp omap-mcbsp.1: mcbsp: RCR1: 0x1040 Sep 4 08:21:21 amsdelta user.debug kernel: [309913.950000] omap-mcbsp omap-mcbsp.1: mcbsp: XCR2: 0x2045 Sep 4 08:21:21 amsdelta user.debug kernel: [309913.950000] omap-mcbsp omap-mcbsp.1: mcbsp: XCR1: 0x1040 Sep 4 08:21:21 amsdelta user.debug kernel: [309913.950000] omap-mcbsp omap-mcbsp.1: mcbsp: SRGR2: 0x100f Sep 4 08:21:21 amsdelta user.debug kernel: [309913.950000] omap-mcbsp omap-mcbsp.1: mcbsp: SRGR1: 0x1000 Sep 4 08:21:21 amsdelta user.debug kernel: [309913.950000] omap-mcbsp omap-mcbsp.1: mcbsp: PCR0: 0x2003 Sep 4 08:21:21 amsdelta user.debug kernel: [309913.950000] omap-mcbsp omap-mcbsp.1: mcbsp: *********************** Those corruptions, even if should never happen, were actually caused by a wireless adapter connected to the machine USB port. Extra piece of hardware, very usefull in some cases, but obviously not taken into account while the machine was being designed. While playback seems unaffected, WiFi noise in capture stream prevents from using the videophone in speakerphone mode, but in normal (handset) mode, the noise level seems still acceptable. The only missing piece of a puzzle is my patch, that prevents the kernel driver from putting corrupted values into the McBSP control registers. That learned, please reconsider: 1. Can my patch break anything, related or not? 2. How does it affect performance of otherwise unaffected machines? 3. Is correcting a poorly designed machine behaviour worth of the change? Thanks, Janusz
On Fri, 2009-09-04 at 12:34 +0200, ext Janusz Krzysztofik wrote:
in normal (handset) mode, the noise level seems still acceptable. The only missing piece of a puzzle is my patch, that prevents the kernel driver from putting corrupted values into the McBSP control registers.
That learned, please reconsider: 1. Can my patch break anything, related or not?
That needs to be investigated in more detail.
2. How does it affect performance of otherwise unaffected machines?
Like you already know, shouldn't.
3. Is correcting a poorly designed machine behaviour worth of the change?
Thanks, Janusz
I'd say this is in the right track. All McBSP registers (not status etc) will need to be stored in memory. (Now, they're not). And all those register contents will need to be written back in certain situations. So this is the case when there is an external audio chip, that takes audio in in bursts. So once the burst is complete, all McBSP clocks will be disabled for a duration. Then (at least >= 3430) device will hit OFF mode, meaning all register contents are lost, also McBSP's. They will need to be written back at some point. My quick verdict is, that your patch is, going into right direction. But the thing is that my words don't count much ;) (Possibly worth taking the patch in, if guaranteed to not break any others). - Eero
On Fri, Sep 4, 2009 at 4:37 PM, Eero Nurkkala<ext-eero.nurkkala@nokia.com> wrote:
On Fri, 2009-09-04 at 12:34 +0200, ext Janusz Krzysztofik wrote:
in normal (handset) mode, the noise level seems still acceptable. The only missing piece of a puzzle is my patch, that prevents the kernel driver from putting corrupted values into the McBSP control registers.
That learned, please reconsider: 1. Can my patch break anything, related or not?
That needs to be investigated in more detail.
2. How does it affect performance of otherwise unaffected machines?
Like you already know, shouldn't.
3. Is correcting a poorly designed machine behaviour worth of the change?
Thanks, Janusz
I'd say this is in the right track. All McBSP registers (not status etc) will need to be stored in memory. (Now, they're not). And all those register contents will need to be written back in certain situations. So this is the case when there is an external audio chip, that takes audio in in bursts. So once the burst is complete, all McBSP clocks will be disabled for a duration.
Then (at least >= 3430) device will hit OFF mode, meaning all register contents are lost, also McBSP's. They will need to be written back at some point. Isn't the better way to achieve this thing, at least from the OFF mode perspective, is to provide similar implementation for McBSP in omap3_core_save_context () / omap3_core_restore_context(), in file arch/arm/mach-omap2/pm34xx.c? Code for other core peripherals is already there and we have to add support for rest of the core-domain peripherals.
My quick verdict is, that your patch is, going into right direction. But the thing is that my words don't count much ;)
(Possibly worth taking the patch in, if guaranteed to not break any others).
- Eero
-- 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
-- Best Regards, Anuj Aggarwal
Isn't the better way to achieve this thing, at least from the OFF mode perspective, is to provide similar implementation for McBSP in omap3_core_save_context () / omap3_core_restore_context(), in file arch/arm/mach-omap2/pm34xx.c? Code for other core peripherals is already there and we have to add support for rest of the core-domain peripherals.
Yes and no. I'm not sure McBSP context would be needed to be restored after every wake up from PER off mode.. isn't that how it's handled now for other peripherals? But maybe restore context for McBSP whenever there's need for it. Rewriting millions of registers just for the case doesn't sound good. IMO, with an irq from an external device, a rewrite of the McBSP registers occurs. You loose the point of the OFF mode if you always start rewriting all registers? (you're not saving power.. the opposite). Or did I not understand something? (most likely the case) =) - Eero
participants (4)
-
Anuj Aggarwal -
Eero Nurkkala -
ext-Eero.Nurkkala＠nokia.com
-
Janusz Krzysztofik