[PATCH v2] OMAP: PM: DMA: Enable runtime pm

[PATCH v2] OMAP: PM: DMA: Enable runtime pm

[G, Manjunath Kondaiah]

From: Manjunath G Kondaiah <manjugk@ti.com>

Enable runtime pm and use pm_runtime_get_sync and pm_runtime_put_autosuspend
for OMAP DMA driver.

The DMA driver uses auto suspend feature of runtime pm framework through
which the clock gets disabled automatically if there is no activity for
more than one second.

Testing:
Compile: omap1_defconfig and omap2plus_defconfig
Boot: OMAP1710(H3), OMAP2420(H4), OMAP3630(Zoom3), OMAP4(Blaze)

On zoom3 core retention is tested with following steps:
echo 1 > /debug/pm_debug/sleep_while_idle
echo 1 > /debug/pm_debug/enable_off_mode
echo 5 > /sys/devices/platform/omap/omap_uart.0/sleep_timeout
echo 5 > /sys/devices/platform/omap/omap_uart.1/sleep_timeout
echo 5 > /sys/devices/platform/omap/omap_uart.2/sleep_timeout
echo 5 > /sys/devices/platform/omap/omap_uart.3/sleep_timeout

It is observed that(on pm branch), core retention count gets increasing if the
board is left idle for more than 5 seconds. However, it doesnot enter off mode
(even without DMA runtime changes).

Signed-off-by: G, Manjunath Kondaiah <manjugk@ti.com>
---
 arch/arm/plat-omap/dma.c |   27 +++++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-omap/dma.c b/arch/arm/plat-omap/dma.c
index 2ec3b5d..6bfe25c 100644
--- a/arch/arm/plat-omap/dma.c
+++ b/arch/arm/plat-omap/dma.c
@@ -35,6 +35,7 @@
 #include <linux/io.h>
 #include <linux/slab.h>
 #include <linux/delay.h>
+#include <linux/pm_runtime.h>
 
 #include <asm/system.h>
 #include <mach/hardware.h>
@@ -59,6 +60,7 @@ enum { DMA_CHAIN_STARTED, DMA_CHAIN_NOTSTARTED };
 
 static struct omap_system_dma_plat_info *p;
 static struct omap_dma_dev_attr *d;
+static struct device *dev;
 
 static int enable_1510_mode;
 static u32 errata;
@@ -676,6 +678,7 @@ int omap_request_dma(int dev_id, const char *dev_name,
 	unsigned long flags;
 	struct omap_dma_lch *chan;
 
+	pm_runtime_get_sync(dev);
 	spin_lock_irqsave(&dma_chan_lock, flags);
 	for (ch = 0; ch < dma_chan_count; ch++) {
 		if (free_ch == -1 && dma_chan[ch].dev_id == -1) {
@@ -686,6 +689,7 @@ int omap_request_dma(int dev_id, const char *dev_name,
 	}
 	if (free_ch == -1) {
 		spin_unlock_irqrestore(&dma_chan_lock, flags);
+		pm_runtime_put_autosuspend(dev);
 		return -EBUSY;
 	}
 	chan = dma_chan + free_ch;
@@ -743,6 +747,7 @@ int omap_request_dma(int dev_id, const char *dev_name,
 	}
 
 	*dma_ch_out = free_ch;
+	pm_runtime_put_autosuspend(dev);
 
 	return 0;
 }
@@ -871,6 +876,8 @@ void omap_start_dma(int lch)
 {
 	u32 l;
 
+	pm_runtime_get_sync(dev);
+
 	/*
 	 * The CPC/CDAC register needs to be initialized to zero
 	 * before starting dma transfer.
@@ -1805,6 +1812,8 @@ static int omap1_dma_handle_ch(int ch)
 	if (likely(dma_chan[ch].callback != NULL))
 		dma_chan[ch].callback(ch, csr, dma_chan[ch].data);
 
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
 	return 1;
 }
 
@@ -1899,6 +1908,8 @@ static int omap2_dma_handle_ch(int ch)
 	if (likely(dma_chan[ch].callback != NULL))
 		dma_chan[ch].callback(ch, status, dma_chan[ch].data);
 
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
 	return 0;
 }
 
@@ -1978,6 +1989,7 @@ static int __devinit omap_system_dma_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
+	dev			= &pdev->dev;
 	d			= p->dma_attr;
 	errata			= p->errata;
 
@@ -1999,6 +2011,11 @@ static int __devinit omap_system_dma_probe(struct platform_device *pdev)
 		}
 	}
 
+	pm_runtime_use_autosuspend(dev);
+	pm_runtime_set_autosuspend_delay(dev, 1000);
+	pm_runtime_enable(dev);
+	pm_runtime_get_sync(dev);
+
 	spin_lock_init(&dma_chan_lock);
 	for (ch = 0; ch < dma_chan_count; ch++) {
 		omap_clear_dma(ch);
@@ -2064,6 +2081,16 @@ static int __devinit omap_system_dma_probe(struct platform_device *pdev)
 		dma_chan[1].dev_id = 1;
 	}
 	p->show_dma_caps();
+
+	/*
+	 * Note: If dma channels are reserved through boot paramters,
+	 * then dma device is always enabled.
+	 */
+	if (omap_dma_reserve_channels)
+		pm_runtime_get(dev);
+
+	pm_runtime_put_autosuspend(dev);
+
 	return 0;
 
 exit_dma_irq_fail:
-- 
1.7.1

[PATCH v2] OMAP: PM: DMA: Enable runtime pm

[G, Manjunath Kondaiah]

Kevin,
Do you have any comments on these changes. I am planning to have these changes
as part of new dma series(runtime support+mstandby mode api changes+removing
redundant code). I am in the process of testing all the above changes on all 
the omap boards.

-Manjunath

On Mon, Feb 07, 2011 at 06:08:06PM +0530, G, Manjunath Kondaiah wrote:
> From: Manjunath G Kondaiah <manjugk@ti.com>
> 
> Enable runtime pm and use pm_runtime_get_sync and pm_runtime_put_autosuspend
> for OMAP DMA driver.
> 
> The DMA driver uses auto suspend feature of runtime pm framework through
> which the clock gets disabled automatically if there is no activity for
> more than one second.
> 
> Testing:
> Compile: omap1_defconfig and omap2plus_defconfig
> Boot: OMAP1710(H3), OMAP2420(H4), OMAP3630(Zoom3), OMAP4(Blaze)
> 
> On zoom3 core retention is tested with following steps:
> echo 1 > /debug/pm_debug/sleep_while_idle
> echo 1 > /debug/pm_debug/enable_off_mode
> echo 5 > /sys/devices/platform/omap/omap_uart.0/sleep_timeout
> echo 5 > /sys/devices/platform/omap/omap_uart.1/sleep_timeout
> echo 5 > /sys/devices/platform/omap/omap_uart.2/sleep_timeout
> echo 5 > /sys/devices/platform/omap/omap_uart.3/sleep_timeout
> 
> It is observed that(on pm branch), core retention count gets increasing if the
> board is left idle for more than 5 seconds. However, it doesnot enter off mode
> (even without DMA runtime changes).
> 
> Signed-off-by: G, Manjunath Kondaiah <manjugk@ti.com>
> ---
>  arch/arm/plat-omap/dma.c |   27 +++++++++++++++++++++++++++
>  1 files changed, 27 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/dma.c b/arch/arm/plat-omap/dma.c
> index 2ec3b5d..6bfe25c 100644
> --- a/arch/arm/plat-omap/dma.c
> +++ b/arch/arm/plat-omap/dma.c
> @@ -35,6 +35,7 @@
>  #include <linux/io.h>
>  #include <linux/slab.h>
>  #include <linux/delay.h>
> +#include <linux/pm_runtime.h>
>  
>  #include <asm/system.h>
>  #include <mach/hardware.h>
> @@ -59,6 +60,7 @@ enum { DMA_CHAIN_STARTED, DMA_CHAIN_NOTSTARTED };
>  
>  static struct omap_system_dma_plat_info *p;
>  static struct omap_dma_dev_attr *d;
> +static struct device *dev;
>  
>  static int enable_1510_mode;
>  static u32 errata;
> @@ -676,6 +678,7 @@ int omap_request_dma(int dev_id, const char *dev_name,
>  	unsigned long flags;
>  	struct omap_dma_lch *chan;
>  
> +	pm_runtime_get_sync(dev);
>  	spin_lock_irqsave(&dma_chan_lock, flags);
>  	for (ch = 0; ch < dma_chan_count; ch++) {
>  		if (free_ch == -1 && dma_chan[ch].dev_id == -1) {
> @@ -686,6 +689,7 @@ int omap_request_dma(int dev_id, const char *dev_name,
>  	}
>  	if (free_ch == -1) {
>  		spin_unlock_irqrestore(&dma_chan_lock, flags);
> +		pm_runtime_put_autosuspend(dev);
>  		return -EBUSY;
>  	}
>  	chan = dma_chan + free_ch;
> @@ -743,6 +747,7 @@ int omap_request_dma(int dev_id, const char *dev_name,
>  	}
>  
>  	*dma_ch_out = free_ch;
> +	pm_runtime_put_autosuspend(dev);
>  
>  	return 0;
>  }
> @@ -871,6 +876,8 @@ void omap_start_dma(int lch)
>  {
>  	u32 l;
>  
> +	pm_runtime_get_sync(dev);
> +
>  	/*
>  	 * The CPC/CDAC register needs to be initialized to zero
>  	 * before starting dma transfer.
> @@ -1805,6 +1812,8 @@ static int omap1_dma_handle_ch(int ch)
>  	if (likely(dma_chan[ch].callback != NULL))
>  		dma_chan[ch].callback(ch, csr, dma_chan[ch].data);
>  
> +	pm_runtime_mark_last_busy(dev);
> +	pm_runtime_put_autosuspend(dev);
>  	return 1;
>  }
>  
> @@ -1899,6 +1908,8 @@ static int omap2_dma_handle_ch(int ch)
>  	if (likely(dma_chan[ch].callback != NULL))
>  		dma_chan[ch].callback(ch, status, dma_chan[ch].data);
>  
> +	pm_runtime_mark_last_busy(dev);
> +	pm_runtime_put_autosuspend(dev);
>  	return 0;
>  }
>  
> @@ -1978,6 +1989,7 @@ static int __devinit omap_system_dma_probe(struct platform_device *pdev)
>  		return -EINVAL;
>  	}
>  
> +	dev			= &pdev->dev;
>  	d			= p->dma_attr;
>  	errata			= p->errata;
>  
> @@ -1999,6 +2011,11 @@ static int __devinit omap_system_dma_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +	pm_runtime_use_autosuspend(dev);
> +	pm_runtime_set_autosuspend_delay(dev, 1000);
> +	pm_runtime_enable(dev);
> +	pm_runtime_get_sync(dev);
> +
>  	spin_lock_init(&dma_chan_lock);
>  	for (ch = 0; ch < dma_chan_count; ch++) {
>  		omap_clear_dma(ch);
> @@ -2064,6 +2081,16 @@ static int __devinit omap_system_dma_probe(struct platform_device *pdev)
>  		dma_chan[1].dev_id = 1;
>  	}
>  	p->show_dma_caps();
> +
> +	/*
> +	 * Note: If dma channels are reserved through boot paramters,
> +	 * then dma device is always enabled.
> +	 */
> +	if (omap_dma_reserve_channels)
> +		pm_runtime_get(dev);
> +
> +	pm_runtime_put_autosuspend(dev);
> +
>  	return 0;
>  
>  exit_dma_irq_fail:
> -- 
> 1.7.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] OMAP: PM: DMA: Enable runtime pm

[Kevin Hilman]

"G, Manjunath Kondaiah" <manjugk@ti.com> writes:

> From: Manjunath G Kondaiah <manjugk@ti.com>
>
> Enable runtime pm and use pm_runtime_get_sync and pm_runtime_put_autosuspend
> for OMAP DMA driver.
>
> The DMA driver uses auto suspend feature of runtime pm framework through
> which the clock gets disabled automatically if there is no activity for
> more than one second.
>
> Testing:
> Compile: omap1_defconfig and omap2plus_defconfig
> Boot: OMAP1710(H3), OMAP2420(H4), OMAP3630(Zoom3), OMAP4(Blaze)

The normal DMA tests should also be run on these platforms.  Based on
the above, I can't tell any DMA tests were run.   Based on my tests,
this isn't working for chained xfers.

Using the runtime PM sysfs interface, you can check the runtime status
of the device:

# cat /sys/devices/platform/omap/omap_dma_system.0/power/runtime_status          

It should show 'active' during transfer, and after timeout expires it
will show 'suspended'.

Doing some tests using my dmatest module:

  git://gitorious.org/omap-test/dmatest.git

I noticed that it gets stuck in 'active' and never gets suspended when I
used DMA channel linking (load module using 'linking=1' as load-time option)

I'm not sure exactly why, but I will guess that the reason is that there
is an imbalance in get/put calls when using chaining, since 'get' is
only called once upon omap_start_dma() but 'put' is called for every
channel in the callback.

> On zoom3 core retention is tested with following steps:
> echo 1 > /debug/pm_debug/sleep_while_idle
> echo 1 > /debug/pm_debug/enable_off_mode
> echo 5 > /sys/devices/platform/omap/omap_uart.0/sleep_timeout
> echo 5 > /sys/devices/platform/omap/omap_uart.1/sleep_timeout
> echo 5 > /sys/devices/platform/omap/omap_uart.2/sleep_timeout
> echo 5 > /sys/devices/platform/omap/omap_uart.3/sleep_timeout
>
> It is observed that(on pm branch), core retention count gets increasing if the
> board is left idle for more than 5 seconds. However, it doesnot enter off mode
> (even without DMA runtime changes).

What silicon rev is on your Zoom3?  Mainline kernels now disable core
off-mode for 3630 revs < ES2.1 due to erratum i583.

If this happens, you should see something like this on the console:

           Core OFF disabled due to errata i583

Kevin

> Signed-off-by: G, Manjunath Kondaiah <manjugk@ti.com>
> ---
>  arch/arm/plat-omap/dma.c |   27 +++++++++++++++++++++++++++
>  1 files changed, 27 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/plat-omap/dma.c b/arch/arm/plat-omap/dma.c
> index 2ec3b5d..6bfe25c 100644
> --- a/arch/arm/plat-omap/dma.c
> +++ b/arch/arm/plat-omap/dma.c
> @@ -35,6 +35,7 @@
>  #include <linux/io.h>
>  #include <linux/slab.h>
>  #include <linux/delay.h>
> +#include <linux/pm_runtime.h>
>  
>  #include <asm/system.h>
>  #include <mach/hardware.h>
> @@ -59,6 +60,7 @@ enum { DMA_CHAIN_STARTED, DMA_CHAIN_NOTSTARTED };
>  
>  static struct omap_system_dma_plat_info *p;
>  static struct omap_dma_dev_attr *d;
> +static struct device *dev;
>  
>  static int enable_1510_mode;
>  static u32 errata;
> @@ -676,6 +678,7 @@ int omap_request_dma(int dev_id, const char *dev_name,
>  	unsigned long flags;
>  	struct omap_dma_lch *chan;
>  
> +	pm_runtime_get_sync(dev);
>  	spin_lock_irqsave(&dma_chan_lock, flags);
>  	for (ch = 0; ch < dma_chan_count; ch++) {
>  		if (free_ch == -1 && dma_chan[ch].dev_id == -1) {
> @@ -686,6 +689,7 @@ int omap_request_dma(int dev_id, const char *dev_name,
>  	}
>  	if (free_ch == -1) {
>  		spin_unlock_irqrestore(&dma_chan_lock, flags);
> +		pm_runtime_put_autosuspend(dev);
>  		return -EBUSY;
>  	}
>  	chan = dma_chan + free_ch;
> @@ -743,6 +747,7 @@ int omap_request_dma(int dev_id, const char *dev_name,
>  	}
>  
>  	*dma_ch_out = free_ch;
> +	pm_runtime_put_autosuspend(dev);
>  
>  	return 0;
>  }
> @@ -871,6 +876,8 @@ void omap_start_dma(int lch)
>  {
>  	u32 l;
>  
> +	pm_runtime_get_sync(dev);
> +
>  	/*
>  	 * The CPC/CDAC register needs to be initialized to zero
>  	 * before starting dma transfer.
> @@ -1805,6 +1812,8 @@ static int omap1_dma_handle_ch(int ch)
>  	if (likely(dma_chan[ch].callback != NULL))
>  		dma_chan[ch].callback(ch, csr, dma_chan[ch].data);
>  
> +	pm_runtime_mark_last_busy(dev);
> +	pm_runtime_put_autosuspend(dev);
>  	return 1;
>  }
>  
> @@ -1899,6 +1908,8 @@ static int omap2_dma_handle_ch(int ch)
>  	if (likely(dma_chan[ch].callback != NULL))
>  		dma_chan[ch].callback(ch, status, dma_chan[ch].data);
>  
> +	pm_runtime_mark_last_busy(dev);
> +	pm_runtime_put_autosuspend(dev);
>  	return 0;
>  }
>  
> @@ -1978,6 +1989,7 @@ static int __devinit omap_system_dma_probe(struct platform_device *pdev)
>  		return -EINVAL;
>  	}
>  
> +	dev			= &pdev->dev;
>  	d			= p->dma_attr;
>  	errata			= p->errata;
>  
> @@ -1999,6 +2011,11 @@ static int __devinit omap_system_dma_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +	pm_runtime_use_autosuspend(dev);
> +	pm_runtime_set_autosuspend_delay(dev, 1000);
> +	pm_runtime_enable(dev);
> +	pm_runtime_get_sync(dev);
> +
>  	spin_lock_init(&dma_chan_lock);
>  	for (ch = 0; ch < dma_chan_count; ch++) {
>  		omap_clear_dma(ch);
> @@ -2064,6 +2081,16 @@ static int __devinit omap_system_dma_probe(struct platform_device *pdev)
>  		dma_chan[1].dev_id = 1;
>  	}
>  	p->show_dma_caps();
> +
> +	/*
> +	 * Note: If dma channels are reserved through boot paramters,
> +	 * then dma device is always enabled.
> +	 */
> +	if (omap_dma_reserve_channels)
> +		pm_runtime_get(dev);
> +
> +	pm_runtime_put_autosuspend(dev);
> +
>  	return 0;
>  
>  exit_dma_irq_fail:

[PATCH v2] OMAP: PM: DMA: Enable runtime pm

[G, Manjunath Kondaiah]

Hi Kevin,

On Mon, Feb 14, 2011 at 02:06:53PM -0800, Kevin Hilman wrote:
> "G, Manjunath Kondaiah" <manjugk@ti.com> writes:
> 
> > From: Manjunath G Kondaiah <manjugk@ti.com>
> >
> > Enable runtime pm and use pm_runtime_get_sync and pm_runtime_put_autosuspend
> > for OMAP DMA driver.
> >
> > The DMA driver uses auto suspend feature of runtime pm framework through
> > which the clock gets disabled automatically if there is no activity for
> > more than one second.
> >
> > Testing:
> > Compile: omap1_defconfig and omap2plus_defconfig
> > Boot: OMAP1710(H3), OMAP2420(H4), OMAP3630(Zoom3), OMAP4(Blaze)
> 
> The normal DMA tests should also be run on these platforms.  Based on
> the above, I can't tell any DMA tests were run.   Based on my tests,
> this isn't working for chained xfers.
> 
> Using the runtime PM sysfs interface, you can check the runtime status
> of the device:
> 
> # cat /sys/devices/platform/omap/omap_dma_system.0/power/runtime_status          
> 
> It should show 'active' during transfer, and after timeout expires it
> will show 'suspended'.
> 
> Doing some tests using my dmatest module:
> 
>   git://gitorious.org/omap-test/dmatest.git
> 
> I noticed that it gets stuck in 'active' and never gets suspended when I
> used DMA channel linking (load module using 'linking=1' as load-time option)
> 
> I'm not sure exactly why, but I will guess that the reason is that there
> is an imbalance in get/put calls when using chaining, since 'get' is
> only called once upon omap_start_dma() but 'put' is called for every
> channel in the callback.

Even I noticed this after running chaining test case and checking
runtime status. But, I am wondering even with 'active' runtime status, 
the core hits off and retention.

The complete log which has all the sequences of running chaining tests,
enabling off mode and checking runtime status is available at:
http://pastebin.com/YEHMEXUP

Though I agree on the point that, it is mismatch with get/put calls with
DMA chaining, I still need to analyze this in detail.

The other thing which is not considered here is, the get_sync is called
inside start_dma only(request_dma will call get_sync and put after the
getting requested channel). After request_dma and start_dma, there are
API's called by user(dma_set_params, priority etc) which also require
get_sync since those API's will access configuration registers. I am
wondering if have get_sync and put in all the API's, this might result
in over loading. 

> 
> > On zoom3 core retention is tested with following steps:
> > echo 1 > /debug/pm_debug/sleep_while_idle
> > echo 1 > /debug/pm_debug/enable_off_mode
> > echo 5 > /sys/devices/platform/omap/omap_uart.0/sleep_timeout
> > echo 5 > /sys/devices/platform/omap/omap_uart.1/sleep_timeout
> > echo 5 > /sys/devices/platform/omap/omap_uart.2/sleep_timeout
> > echo 5 > /sys/devices/platform/omap/omap_uart.3/sleep_timeout
> >
> > It is observed that(on pm branch), core retention count gets increasing if the
> > board is left idle for more than 5 seconds. However, it doesnot enter off mode
> > (even without DMA runtime changes).
> 
> What silicon rev is on your Zoom3?  
It's 3630 ES1.0. 
> Mainline kernels now disable core off-mode for 3630 revs < ES2.1 due to erratum 
>i583.
> 
> If this happens, you should see something like this on the console:
> 
>            Core OFF disabled due to errata i583
> 
We can observe above message in mainline after enabling cpu idle in
omap2plus_defconfig.

I switched to zoom2 and able to hit core retention and
off mode with mainline.

-Manjunath

[PATCH v2] OMAP: PM: DMA: Enable runtime pm

[Kevin Hilman]

"G, Manjunath Kondaiah" <manjugk@ti.com> writes:

> Hi Kevin,
>
> On Mon, Feb 14, 2011 at 02:06:53PM -0800, Kevin Hilman wrote:
>> "G, Manjunath Kondaiah" <manjugk@ti.com> writes:
>> 
>> > From: Manjunath G Kondaiah <manjugk@ti.com>
>> >
>> > Enable runtime pm and use pm_runtime_get_sync and pm_runtime_put_autosuspend
>> > for OMAP DMA driver.
>> >
>> > The DMA driver uses auto suspend feature of runtime pm framework through
>> > which the clock gets disabled automatically if there is no activity for
>> > more than one second.
>> >
>> > Testing:
>> > Compile: omap1_defconfig and omap2plus_defconfig
>> > Boot: OMAP1710(H3), OMAP2420(H4), OMAP3630(Zoom3), OMAP4(Blaze)
>> 
>> The normal DMA tests should also be run on these platforms.  Based on
>> the above, I can't tell any DMA tests were run.   Based on my tests,
>> this isn't working for chained xfers.
>> 
>> Using the runtime PM sysfs interface, you can check the runtime status
>> of the device:
>> 
>> # cat /sys/devices/platform/omap/omap_dma_system.0/power/runtime_status          
>> 
>> It should show 'active' during transfer, and after timeout expires it
>> will show 'suspended'.
>> 
>> Doing some tests using my dmatest module:
>> 
>>   git://gitorious.org/omap-test/dmatest.git
>> 
>> I noticed that it gets stuck in 'active' and never gets suspended when I
>> used DMA channel linking (load module using 'linking=1' as load-time option)
>> 
>> I'm not sure exactly why, but I will guess that the reason is that there
>> is an imbalance in get/put calls when using chaining, since 'get' is
>> only called once upon omap_start_dma() but 'put' is called for every
>> channel in the callback.
>
> Even I noticed this after running chaining test case and checking
> runtime status. But, I am wondering even with 'active' runtime status, 
> the core hits off and retention.

Probably because system DMA is auto-idle and clocked by the core_l3_iclk

> The complete log which has all the sequences of running chaining tests,
> enabling off mode and checking runtime status is available at:
> http://pastebin.com/YEHMEXUP
>
> Though I agree on the point that, it is mismatch with get/put calls with
> DMA chaining, I still need to analyze this in detail.

Yes.  The mismatch highlights an underlying problem.

> The other thing which is not considered here is, the get_sync is called
> inside start_dma only(request_dma will call get_sync and put after the
> getting requested channel). After request_dma and start_dma, there are
> API's called by user(dma_set_params, priority etc) which also require
> get_sync since those API's will access configuration registers. I am
> wondering if have get_sync and put in all the API's, this might result
> in over loading. 

I'm not sure what you mean by over loading.

You need to have all register accesses inside get/put calls.  As long as
they are balanced, this should not leed to problems.

>> 
>> > On zoom3 core retention is tested with following steps:
>> > echo 1 > /debug/pm_debug/sleep_while_idle
>> > echo 1 > /debug/pm_debug/enable_off_mode
>> > echo 5 > /sys/devices/platform/omap/omap_uart.0/sleep_timeout
>> > echo 5 > /sys/devices/platform/omap/omap_uart.1/sleep_timeout
>> > echo 5 > /sys/devices/platform/omap/omap_uart.2/sleep_timeout
>> > echo 5 > /sys/devices/platform/omap/omap_uart.3/sleep_timeout
>> >
>> > It is observed that(on pm branch), core retention count gets increasing if the
>> > board is left idle for more than 5 seconds. However, it doesnot enter off mode
>> > (even without DMA runtime changes).
>> 
>> What silicon rev is on your Zoom3?  
> It's 3630 ES1.0. 
>> Mainline kernels now disable core off-mode for 3630 revs < ES2.1 due to erratum 
>>i583.
>> 
>> If this happens, you should see something like this on the console:
>> 
>>            Core OFF disabled due to errata i583
>> 
> We can observe above message in mainline after enabling cpu idle in
> omap2plus_defconfig.
>
> I switched to zoom2 and able to hit core retention and
> off mode with mainline.

OK, good. 

Thanks for clarifying.


Kevin

[PATCH v2] OMAP: PM: DMA: Enable runtime pm

[G, Manjunath Kondaiah]

On Wed, Feb 16, 2011 at 11:47:33AM -0800, Kevin Hilman wrote:
> "G, Manjunath Kondaiah" <manjugk@ti.com> writes:
> 
> > Hi Kevin,
> >
> > On Mon, Feb 14, 2011 at 02:06:53PM -0800, Kevin Hilman wrote:
> >> "G, Manjunath Kondaiah" <manjugk@ti.com> writes:
> >> 
> >> > From: Manjunath G Kondaiah <manjugk@ti.com>
> >> >
> >> > Enable runtime pm and use pm_runtime_get_sync and pm_runtime_put_autosuspend
> >> > for OMAP DMA driver.
> >> >
> >> > The DMA driver uses auto suspend feature of runtime pm framework through
> >> > which the clock gets disabled automatically if there is no activity for
> >> > more than one second.
> >> >
> >> > Testing:
> >> > Compile: omap1_defconfig and omap2plus_defconfig
> >> > Boot: OMAP1710(H3), OMAP2420(H4), OMAP3630(Zoom3), OMAP4(Blaze)
> >> 
> >> The normal DMA tests should also be run on these platforms.  Based on
> >> the above, I can't tell any DMA tests were run.   Based on my tests,
> >> this isn't working for chained xfers.
> >> 
> >> Using the runtime PM sysfs interface, you can check the runtime status
> >> of the device:
> >> 
> >> # cat /sys/devices/platform/omap/omap_dma_system.0/power/runtime_status          
> >> 
> >> It should show 'active' during transfer, and after timeout expires it
> >> will show 'suspended'.
> >> 
> >> Doing some tests using my dmatest module:
> >> 
> >>   git://gitorious.org/omap-test/dmatest.git
> >> 
> >> I noticed that it gets stuck in 'active' and never gets suspended when I
> >> used DMA channel linking (load module using 'linking=1' as load-time option)
> >> 
> >> I'm not sure exactly why, but I will guess that the reason is that there
> >> is an imbalance in get/put calls when using chaining, since 'get' is
> >> only called once upon omap_start_dma() but 'put' is called for every
> >> channel in the callback.
> >
> > Even I noticed this after running chaining test case and checking
> > runtime status. But, I am wondering even with 'active' runtime status, 
> > the core hits off and retention.
> 
> Probably because system DMA is auto-idle and clocked by the core_l3_iclk
> 
> > The complete log which has all the sequences of running chaining tests,
> > enabling off mode and checking runtime status is available at:
> > http://pastebin.com/YEHMEXUP
> >
> > Though I agree on the point that, it is mismatch with get/put calls with
> > DMA chaining, I still need to analyze this in detail.
> 
> Yes.  The mismatch highlights an underlying problem.
> 
> > The other thing which is not considered here is, the get_sync is called
> > inside start_dma only(request_dma will call get_sync and put after the
> > getting requested channel). After request_dma and start_dma, there are
> > API's called by user(dma_set_params, priority etc) which also require
> > get_sync since those API's will access configuration registers. I am
> > wondering if have get_sync and put in all the API's, this might result
> > in over loading. 
> 
> I'm not sure what you mean by over loading.
> 
> You need to have all register accesses inside get/put calls.  As long as
> they are balanced, this should not leed to problems.

I mean, almost all the api's will have get_sync and put_autosuspend
calls. If it is acceptable, let me check the same with implementation.

I feel this is equivalent to placing get_sync and put_autosuspend with 
low level read/write api's.

-Manjunath

[PATCH v2] OMAP: PM: DMA: Enable runtime pm

[G, Manjunath Kondaiah]

On Mon, Feb 14, 2011 at 02:06:53PM -0800, Kevin Hilman wrote:
> "G, Manjunath Kondaiah" <manjugk@ti.com> writes:
> 
> > From: Manjunath G Kondaiah <manjugk@ti.com>
> >
> > Enable runtime pm and use pm_runtime_get_sync and pm_runtime_put_autosuspend
> > for OMAP DMA driver.
> >
> > The DMA driver uses auto suspend feature of runtime pm framework through
> > which the clock gets disabled automatically if there is no activity for
> > more than one second.
> >
> > Testing:
> > Compile: omap1_defconfig and omap2plus_defconfig
> > Boot: OMAP1710(H3), OMAP2420(H4), OMAP3630(Zoom3), OMAP4(Blaze)
> 
> The normal DMA tests should also be run on these platforms.  Based on
> the above, I can't tell any DMA tests were run.   Based on my tests,
> this isn't working for chained xfers.
> 
> Using the runtime PM sysfs interface, you can check the runtime status
> of the device:
> 
> # cat /sys/devices/platform/omap/omap_dma_system.0/power/runtime_status
> 
> It should show 'active' during transfer, and after timeout expires it
> will show 'suspended'.
> 
> Doing some tests using my dmatest module:
> 
>   git://gitorious.org/omap-test/dmatest.git
> 
> I noticed that it gets stuck in 'active' and never gets suspended when I
> used DMA channel linking (load module using 'linking=1' as load-time option)
> 
> I'm not sure exactly why, but I will guess that the reason is that there
> is an imbalance in get/put calls when using chaining, since 'get' is
> only called once upon omap_start_dma() but 'put' is called for every
> channel in the callback.

found this issue and fixed. It was due to chain handling in interrupt
handler. Since get_sync used only in omap_start_dma for 1st transfer and there
will be no omap_start_dma for second transfer onwards since it will be linked
through chaining in interrupt handler. Here is the fix.

@@ -1895,8 +2002,11 @@ static int omap2_dma_handle_ch(int ch)
                                                OMAP_DMA_DYNAMIC_CHAIN)
                        disable_lnk(ch);
 
-               if (!OMAP_DMA_CHAIN_QEMPTY(chain_id))
+               if (!OMAP_DMA_CHAIN_QEMPTY(chain_id)) {
                        OMAP_DMA_CHAIN_INCQHEAD(chain_id);
+                       pm_runtime_get_sync(dev);
+               }
+
 
                status = p->dma_read(CSR, ch);
                p->dma_write(status, CSR, ch);

-Manjunath

> 
> > On zoom3 core retention is tested with following steps:
> > echo 1 > /debug/pm_debug/sleep_while_idle
> > echo 1 > /debug/pm_debug/enable_off_mode
> > echo 5 > /sys/devices/platform/omap/omap_uart.0/sleep_timeout
> > echo 5 > /sys/devices/platform/omap/omap_uart.1/sleep_timeout
> > echo 5 > /sys/devices/platform/omap/omap_uart.2/sleep_timeout
> > echo 5 > /sys/devices/platform/omap/omap_uart.3/sleep_timeout
> >
> > It is observed that(on pm branch), core retention count gets increasing if the
> > board is left idle for more than 5 seconds. However, it doesnot enter off mode
> > (even without DMA runtime changes).
> 
> What silicon rev is on your Zoom3?  Mainline kernels now disable core
> off-mode for 3630 revs < ES2.1 due to erratum i583.
> 
> If this happens, you should see something like this on the console:
> 
>            Core OFF disabled due to errata i583
> 
> Kevin
> 
> > Signed-off-by: G, Manjunath Kondaiah <manjugk@ti.com>
> > ---
> >  arch/arm/plat-omap/dma.c |   27 +++++++++++++++++++++++++++
> >  1 files changed, 27 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/plat-omap/dma.c b/arch/arm/plat-omap/dma.c
> > index 2ec3b5d..6bfe25c 100644
> > --- a/arch/arm/plat-omap/dma.c
> > +++ b/arch/arm/plat-omap/dma.c
> > @@ -35,6 +35,7 @@
> >  #include <linux/io.h>
> >  #include <linux/slab.h>
> >  #include <linux/delay.h>
> > +#include <linux/pm_runtime.h>
> >  
> >  #include <asm/system.h>
> >  #include <mach/hardware.h>
> > @@ -59,6 +60,7 @@ enum { DMA_CHAIN_STARTED, DMA_CHAIN_NOTSTARTED };
> >  
> >  static struct omap_system_dma_plat_info *p;
> >  static struct omap_dma_dev_attr *d;
> > +static struct device *dev;
> >  
> >  static int enable_1510_mode;
> >  static u32 errata;
> > @@ -676,6 +678,7 @@ int omap_request_dma(int dev_id, const char *dev_name,
> >  	unsigned long flags;
> >  	struct omap_dma_lch *chan;
> >  
> > +	pm_runtime_get_sync(dev);
> >  	spin_lock_irqsave(&dma_chan_lock, flags);
> >  	for (ch = 0; ch < dma_chan_count; ch++) {
> >  		if (free_ch == -1 && dma_chan[ch].dev_id == -1) {
> > @@ -686,6 +689,7 @@ int omap_request_dma(int dev_id, const char *dev_name,
> >  	}
> >  	if (free_ch == -1) {
> >  		spin_unlock_irqrestore(&dma_chan_lock, flags);
> > +		pm_runtime_put_autosuspend(dev);
> >  		return -EBUSY;
> >  	}
> >  	chan = dma_chan + free_ch;
> > @@ -743,6 +747,7 @@ int omap_request_dma(int dev_id, const char *dev_name,
> >  	}
> >  
> >  	*dma_ch_out = free_ch;
> > +	pm_runtime_put_autosuspend(dev);
> >  
> >  	return 0;
> >  }
> > @@ -871,6 +876,8 @@ void omap_start_dma(int lch)
> >  {
> >  	u32 l;
> >  
> > +	pm_runtime_get_sync(dev);
> > +
> >  	/*
> >  	 * The CPC/CDAC register needs to be initialized to zero
> >  	 * before starting dma transfer.
> > @@ -1805,6 +1812,8 @@ static int omap1_dma_handle_ch(int ch)
> >  	if (likely(dma_chan[ch].callback != NULL))
> >  		dma_chan[ch].callback(ch, csr, dma_chan[ch].data);
> >  
> > +	pm_runtime_mark_last_busy(dev);
> > +	pm_runtime_put_autosuspend(dev);
> >  	return 1;
> >  }
> >  
> > @@ -1899,6 +1908,8 @@ static int omap2_dma_handle_ch(int ch)
> >  	if (likely(dma_chan[ch].callback != NULL))
> >  		dma_chan[ch].callback(ch, status, dma_chan[ch].data);
> >  
> > +	pm_runtime_mark_last_busy(dev);
> > +	pm_runtime_put_autosuspend(dev);
> >  	return 0;
> >  }
> >  
> > @@ -1978,6 +1989,7 @@ static int __devinit omap_system_dma_probe(struct platform_device *pdev)
> >  		return -EINVAL;
> >  	}
> >  
> > +	dev			= &pdev->dev;
> >  	d			= p->dma_attr;
> >  	errata			= p->errata;
> >  
> > @@ -1999,6 +2011,11 @@ static int __devinit omap_system_dma_probe(struct platform_device *pdev)
> >  		}
> >  	}
> >  
> > +	pm_runtime_use_autosuspend(dev);
> > +	pm_runtime_set_autosuspend_delay(dev, 1000);
> > +	pm_runtime_enable(dev);
> > +	pm_runtime_get_sync(dev);
> > +
> >  	spin_lock_init(&dma_chan_lock);
> >  	for (ch = 0; ch < dma_chan_count; ch++) {
> >  		omap_clear_dma(ch);
> > @@ -2064,6 +2081,16 @@ static int __devinit omap_system_dma_probe(struct platform_device *pdev)
> >  		dma_chan[1].dev_id = 1;
> >  	}
> >  	p->show_dma_caps();
> > +
> > +	/*
> > +	 * Note: If dma channels are reserved through boot paramters,
> > +	 * then dma device is always enabled.
> > +	 */
> > +	if (omap_dma_reserve_channels)
> > +		pm_runtime_get(dev);
> > +
> > +	pm_runtime_put_autosuspend(dev);
> > +
> >  	return 0;
> >  
> >  exit_dma_irq_fail: