[PATCH 0/2] Add PM runtime support for cppi41 DMA

[PATCH 0/2] Add PM runtime support for cppi41 DMA

[Tony Lindgren]

Hi all,

Here are two patches to add PM runtime support to cppi41 DMA.
I've tested these on am335x using the musb dsps glue layer.
If anybody has other test cases, please test if you can.

Regards,

Tony

Tony Lindgren (2):
  dmaengine: cppi41: Prepare to add PM runtime support
  dmaengine: cppi41: Add basic PM runtime support

 drivers/dma/cppi41.c | 134 +++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 114 insertions(+), 20 deletions(-)

-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] dmaengine: cppi41: Prepare to add PM runtime support

[Tony Lindgren]

Let's just move code from cppi41_dma_issue_pending() to
push_desc_queue() as that's the only call to push_desc_queue().

We want to do this for PM runtime as we need to call push_desc_queue()
also for pending queued transfers from PM runtime resume.

No functional changes, just moves code around.

Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
---
 drivers/dma/cppi41.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
index 3b4c842..66b84fe 100644
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -386,21 +386,6 @@ static void push_desc_queue(struct cppi41_channel *c)
 	u32 desc_phys;
 	u32 reg;
 
-	desc_phys = lower_32_bits(c->desc_phys);
-	desc_num = (desc_phys - cdd->descs_phys) / sizeof(struct cppi41_desc);
-	WARN_ON(cdd->chan_busy[desc_num]);
-	cdd->chan_busy[desc_num] = c;
-
-	reg = (sizeof(struct cppi41_desc) - 24) / 4;
-	reg |= desc_phys;
-	cppi_writel(reg, cdd->qmgr_mem + QMGR_QUEUE_D(c->q_num));
-}
-
-static void cppi41_dma_issue_pending(struct dma_chan *chan)
-{
-	struct cppi41_channel *c = to_cpp41_chan(chan);
-	u32 reg;
-
 	c->residue = 0;
 
 	reg = GCR_CHAN_ENABLE;
@@ -418,6 +403,21 @@ static void cppi41_dma_issue_pending(struct dma_chan *chan)
 	 * before starting the dma engine.
 	 */
 	__iowmb();
+
+	desc_phys = lower_32_bits(c->desc_phys);
+	desc_num = (desc_phys - cdd->descs_phys) / sizeof(struct cppi41_desc);
+	WARN_ON(cdd->chan_busy[desc_num]);
+	cdd->chan_busy[desc_num] = c;
+
+	reg = (sizeof(struct cppi41_desc) - 24) / 4;
+	reg |= desc_phys;
+	cppi_writel(reg, cdd->qmgr_mem + QMGR_QUEUE_D(c->q_num));
+}
+
+static void cppi41_dma_issue_pending(struct dma_chan *chan)
+{
+	struct cppi41_channel *c = to_cpp41_chan(chan);
+
 	push_desc_queue(c);
 }
 
-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] dmaengine: cppi41: Prepare to add PM runtime support

[Vinod Koul]

On Fri, Aug 19, 2016 at 03:59:39PM -0700, Tony Lindgren wrote:
> Let's just move code from cppi41_dma_issue_pending() to
> push_desc_queue() as that's the only call to push_desc_queue().
> 
> We want to do this for PM runtime as we need to call push_desc_queue()
> also for pending queued transfers from PM runtime resume.
> 
> No functional changes, just moves code around.

Applied, now

-- 
~Vinod
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] dmaengine: cppi41: Add basic PM runtime support

[Tony Lindgren]

Let's keep the device enabled between cppi41_dma_issue_pending()
and dmaengine_desc_get_callback_invoke() and rely on the PM runtime
autoidle timeout elsewhere.

As the PM runtime is for whole device, not for each channel,
we need to queue pending transfers if the device is PM runtime
suspended. Then we start the pending transfers in PM runtime
resume.

Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
---
 drivers/dma/cppi41.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 99 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
index 66b84fe..ce8739f 100644
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -108,6 +108,8 @@ struct cppi41_channel {
 	unsigned td_queued:1;
 	unsigned td_seen:1;
 	unsigned td_desc_seen:1;
+
+	struct list_head node;		/* Node for pending list */
 };
 
 struct cppi41_desc {
@@ -146,6 +148,9 @@ struct cppi41_dd {
 	const struct chan_queues *queues_tx;
 	struct chan_queues td_queue;
 
+	struct list_head pending;	/* Pending queued transfers */
+	spinlock_t lock;		/* Lock for pending list */
+
 	/* context for suspend/resume */
 	unsigned int dma_tdfdq;
 };
@@ -332,6 +337,10 @@ static irqreturn_t cppi41_irq(int irq, void *data)
 			c->residue = pd_trans_len(c->desc->pd6) - len;
 			dma_cookie_complete(&c->txd);
 			dmaengine_desc_get_callback_invoke(&c->txd, NULL);
+
+			/* Paired with cppi41_dma_issue_pending */
+			pm_runtime_mark_last_busy(cdd->ddev.dev);
+			pm_runtime_put_autosuspend(cdd->ddev.dev);
 		}
 	}
 	return IRQ_HANDLED;
@@ -349,6 +358,12 @@ static dma_cookie_t cppi41_tx_submit(struct dma_async_tx_descriptor *tx)
 static int cppi41_dma_alloc_chan_resources(struct dma_chan *chan)
 {
 	struct cppi41_channel *c = to_cpp41_chan(chan);
+	struct cppi41_dd *cdd = c->cdd;
+	int error;
+
+	error = pm_runtime_get_sync(cdd->ddev.dev);
+	if (error < 0)
+		return error;
 
 	dma_cookie_init(chan);
 	dma_async_tx_descriptor_init(&c->txd, chan);
@@ -357,11 +372,26 @@ static int cppi41_dma_alloc_chan_resources(struct dma_chan *chan)
 	if (!c->is_tx)
 		cppi_writel(c->q_num, c->gcr_reg + RXHPCRA0);
 
+	pm_runtime_mark_last_busy(cdd->ddev.dev);
+	pm_runtime_put_autosuspend(cdd->ddev.dev);
+
 	return 0;
 }
 
 static void cppi41_dma_free_chan_resources(struct dma_chan *chan)
 {
+	struct cppi41_channel *c = to_cpp41_chan(chan);
+	struct cppi41_dd *cdd = c->cdd;
+	int error;
+
+	error = pm_runtime_get_sync(cdd->ddev.dev);
+	if (error < 0)
+		return;
+
+	WARN_ON(!list_empty(&cdd->pending));
+
+	pm_runtime_mark_last_busy(cdd->ddev.dev);
+	pm_runtime_put_autosuspend(cdd->ddev.dev);
 }
 
 static enum dma_status cppi41_dma_tx_status(struct dma_chan *chan,
@@ -414,11 +444,35 @@ static void push_desc_queue(struct cppi41_channel *c)
 	cppi_writel(reg, cdd->qmgr_mem + QMGR_QUEUE_D(c->q_num));
 }
 
+static void pending_desc(struct cppi41_channel *c)
+{
+	struct cppi41_dd *cdd = c->cdd;
+	unsigned long flags;
+
+	spin_lock_irqsave(&cdd->lock, flags);
+	list_add_tail(&c->node, &cdd->pending);
+	spin_unlock_irqrestore(&cdd->lock, flags);
+}
+
 static void cppi41_dma_issue_pending(struct dma_chan *chan)
 {
 	struct cppi41_channel *c = to_cpp41_chan(chan);
+	struct cppi41_dd *cdd = c->cdd;
+	int error;
+
+	/* PM runtime paired with dmaengine_desc_get_callback_invoke */
+	error = pm_runtime_get(cdd->ddev.dev);
+	if (error < 0) {
+		dev_err(cdd->ddev.dev, "Failed to pm_runtime_get: %i\n",
+			error);
 
-	push_desc_queue(c);
+		return;
+	}
+
+	if (likely(pm_runtime_active(cdd->ddev.dev)))
+		push_desc_queue(c);
+	else
+		pending_desc(c);
 }
 
 static u32 get_host_pd0(u32 length)
@@ -940,12 +994,18 @@ static int cppi41_dma_probe(struct platform_device *pdev)
 	cdd->ctrl_mem = of_iomap(dev->of_node, 1);
 	cdd->sched_mem = of_iomap(dev->of_node, 2);
 	cdd->qmgr_mem = of_iomap(dev->of_node, 3);
+	spin_lock_init(&cdd->lock);
+	INIT_LIST_HEAD(&cdd->pending);
+
+	platform_set_drvdata(pdev, cdd);
 
 	if (!cdd->usbss_mem || !cdd->ctrl_mem || !cdd->sched_mem ||
 			!cdd->qmgr_mem)
 		return -ENXIO;
 
 	pm_runtime_enable(dev);
+	pm_runtime_set_autosuspend_delay(dev, 100);
+	pm_runtime_use_autosuspend(dev);
 	ret = pm_runtime_get_sync(dev);
 	if (ret < 0)
 		goto err_get_sync;
@@ -985,7 +1045,9 @@ static int cppi41_dma_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_of;
 
-	platform_set_drvdata(pdev, cdd);
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
+
 	return 0;
 err_of:
 	dma_async_device_unregister(&cdd->ddev);
@@ -996,7 +1058,8 @@ err_irq:
 err_chans:
 	deinit_cppi41(dev, cdd);
 err_init_cppi:
-	pm_runtime_put(dev);
+	pm_runtime_dont_use_autosuspend(dev);
+	pm_runtime_put_sync(dev);
 err_get_sync:
 	pm_runtime_disable(dev);
 	iounmap(cdd->usbss_mem);
@@ -1021,7 +1084,8 @@ static int cppi41_dma_remove(struct platform_device *pdev)
 	iounmap(cdd->ctrl_mem);
 	iounmap(cdd->sched_mem);
 	iounmap(cdd->qmgr_mem);
-	pm_runtime_put(&pdev->dev);
+	pm_runtime_dont_use_autosuspend(&pdev->dev);
+	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 	return 0;
 }
@@ -1062,9 +1126,39 @@ static int cppi41_resume(struct device *dev)
 
 	return 0;
 }
+
+static int cppi41_runtime_suspend(struct device *dev)
+{
+	struct cppi41_dd *cdd = dev_get_drvdata(dev);
+
+	WARN_ON(!list_empty(&cdd->pending));
+
+	return 0;
+}
+
+static int cppi41_runtime_resume(struct device *dev)
+{
+	struct cppi41_dd *cdd = dev_get_drvdata(dev);
+	struct cppi41_channel *c, *_c;
+	unsigned long flags;
+
+	spin_lock_irqsave(&cdd->lock, flags);
+	list_for_each_entry_safe(c, _c, &cdd->pending, node) {
+		push_desc_queue(c);
+		list_del(&c->node);
+	}
+	spin_unlock_irqrestore(&cdd->lock, flags);
+
+	return 0;
+}
 #endif
 
-static SIMPLE_DEV_PM_OPS(cppi41_pm_ops, cppi41_suspend, cppi41_resume);
+static const struct dev_pm_ops cppi41_pm_ops = {
+	SET_LATE_SYSTEM_SLEEP_PM_OPS(cppi41_suspend, cppi41_resume)
+	SET_RUNTIME_PM_OPS(cppi41_runtime_suspend,
+			   cppi41_runtime_resume,
+			   NULL)
+};
 
 static struct platform_driver cpp41_dma_driver = {
 	.probe  = cppi41_dma_probe,
-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] dmaengine: cppi41: Add basic PM runtime support

[Vinod Koul]

On Fri, Aug 19, 2016 at 03:59:40PM -0700, Tony Lindgren wrote:
> Let's keep the device enabled between cppi41_dma_issue_pending()
> and dmaengine_desc_get_callback_invoke() and rely on the PM runtime
> autoidle timeout elsewhere.
> 
> As the PM runtime is for whole device, not for each channel,
> we need to queue pending transfers if the device is PM runtime
> suspended. Then we start the pending transfers in PM runtime
> resume.
> 
> Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/dma/cppi41.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 99 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
> index 66b84fe..ce8739f 100644
> --- a/drivers/dma/cppi41.c
> +++ b/drivers/dma/cppi41.c
> @@ -108,6 +108,8 @@ struct cppi41_channel {
>  	unsigned td_queued:1;
>  	unsigned td_seen:1;
>  	unsigned td_desc_seen:1;
> +
> +	struct list_head node;		/* Node for pending list */
>  };
>  
>  struct cppi41_desc {
> @@ -146,6 +148,9 @@ struct cppi41_dd {
>  	const struct chan_queues *queues_tx;
>  	struct chan_queues td_queue;
>  
> +	struct list_head pending;	/* Pending queued transfers */
> +	spinlock_t lock;		/* Lock for pending list */
> +
>  	/* context for suspend/resume */
>  	unsigned int dma_tdfdq;
>  };
> @@ -332,6 +337,10 @@ static irqreturn_t cppi41_irq(int irq, void *data)
>  			c->residue = pd_trans_len(c->desc->pd6) - len;
>  			dma_cookie_complete(&c->txd);
>  			dmaengine_desc_get_callback_invoke(&c->txd, NULL);
> +
> +			/* Paired with cppi41_dma_issue_pending */
> +			pm_runtime_mark_last_busy(cdd->ddev.dev);
> +			pm_runtime_put_autosuspend(cdd->ddev.dev);
>  		}
>  	}
>  	return IRQ_HANDLED;
> @@ -349,6 +358,12 @@ static dma_cookie_t cppi41_tx_submit(struct dma_async_tx_descriptor *tx)
>  static int cppi41_dma_alloc_chan_resources(struct dma_chan *chan)
>  {
>  	struct cppi41_channel *c = to_cpp41_chan(chan);
> +	struct cppi41_dd *cdd = c->cdd;
> +	int error;
> +
> +	error = pm_runtime_get_sync(cdd->ddev.dev);

This will be problematic. The alloc callback are not supposed to sleep, so
we cannot use a _sync() call here :(

This is explicitly documented in Documentation/dmaengine/provider.txt

> +	if (error < 0)
> +		return error;
>  
>  	dma_cookie_init(chan);
>  	dma_async_tx_descriptor_init(&c->txd, chan);
> @@ -357,11 +372,26 @@ static int cppi41_dma_alloc_chan_resources(struct dma_chan *chan)
>  	if (!c->is_tx)
>  		cppi_writel(c->q_num, c->gcr_reg + RXHPCRA0);
>  
> +	pm_runtime_mark_last_busy(cdd->ddev.dev);
> +	pm_runtime_put_autosuspend(cdd->ddev.dev);
> +
>  	return 0;
>  }
>  
>  static void cppi41_dma_free_chan_resources(struct dma_chan *chan)
>  {
> +	struct cppi41_channel *c = to_cpp41_chan(chan);
> +	struct cppi41_dd *cdd = c->cdd;
> +	int error;
> +
> +	error = pm_runtime_get_sync(cdd->ddev.dev);

same applies here too


-- 
~Vinod
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] dmaengine: cppi41: Add basic PM runtime support

[Tony Lindgren]

Hi,

* Vinod Koul <vinod.koul-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> [160821 23:34]:
> On Fri, Aug 19, 2016 at 03:59:40PM -0700, Tony Lindgren wrote:
> @@ -349,6 +358,12 @@ static dma_cookie_t cppi41_tx_submit(struct dma_async_tx_descriptor *tx)
> >  static int cppi41_dma_alloc_chan_resources(struct dma_chan *chan)
> >  {
> >  	struct cppi41_channel *c = to_cpp41_chan(chan);
> > +	struct cppi41_dd *cdd = c->cdd;
> > +	int error;
> > +
> > +	error = pm_runtime_get_sync(cdd->ddev.dev);
> 
> This will be problematic. The alloc callback are not supposed to sleep, so
> we cannot use a _sync() call here :(
> 
> This is explicitly documented in Documentation/dmaengine/provider.txt

Hmm but for device_alloc_chan_resources and device_free_chan_resources
we have "These functions can sleep" in the documentation?

Maybe you got confused with the patch @@ line saying cppi41_tx_submit
although the pm_runtime_get is in cppi41_dma_alloc_chan_resources?
Or else I'm confused :)

Anyways, if necessary we could move the call to cppi_writel out of
cppi41_dma_alloc_chan_resources I think.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] dmaengine: cppi41: Add basic PM runtime support

[Vinod Koul]

On Mon, Aug 22, 2016 at 07:21:48AM -0700, Tony Lindgren wrote:
> Hi,
> 
> * Vinod Koul <vinod.koul-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> [160821 23:34]:
> > On Fri, Aug 19, 2016 at 03:59:40PM -0700, Tony Lindgren wrote:
> > @@ -349,6 +358,12 @@ static dma_cookie_t cppi41_tx_submit(struct dma_async_tx_descriptor *tx)
> > >  static int cppi41_dma_alloc_chan_resources(struct dma_chan *chan)
> > >  {
> > >  	struct cppi41_channel *c = to_cpp41_chan(chan);
> > > +	struct cppi41_dd *cdd = c->cdd;
> > > +	int error;
> > > +
> > > +	error = pm_runtime_get_sync(cdd->ddev.dev);
> > 
> > This will be problematic. The alloc callback are not supposed to sleep, so
> > we cannot use a _sync() call here :(
> > 
> > This is explicitly documented in Documentation/dmaengine/provider.txt
> 
> Hmm but for device_alloc_chan_resources and device_free_chan_resources
> we have "These functions can sleep" in the documentation?

which is correct :)

> 
> Maybe you got confused with the patch @@ line saying cppi41_tx_submit
> although the pm_runtime_get is in cppi41_dma_alloc_chan_resources?
> Or else I'm confused :)

Ah, I was trying to finish this before I took my lunch break and looks like
messed it up. Sorry..
Never to rush a review again!

> Anyways, if necessary we could move the call to cppi_writel out of
> cppi41_dma_alloc_chan_resources I think.

I think alloc is fine, I will take a detailed look tomorrow...

-- 
~Vinod
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] dmaengine: cppi41: Add basic PM runtime support

[Vinod Koul]

On Fri, Aug 19, 2016 at 03:59:40PM -0700, Tony Lindgren wrote:
> Let's keep the device enabled between cppi41_dma_issue_pending()
> and dmaengine_desc_get_callback_invoke() and rely on the PM runtime
> autoidle timeout elsewhere.
> 
> As the PM runtime is for whole device, not for each channel,
> we need to queue pending transfers if the device is PM runtime
> suspended. Then we start the pending transfers in PM runtime
> resume.

This fails to apply for me :(

Can you please rebase and resend this one.

-- 
~Vinod
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] dmaengine: cppi41: Add basic PM runtime support

[Tony Lindgren]

* Vinod Koul <vinod.koul-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> [160830 21:45]:
> On Fri, Aug 19, 2016 at 03:59:40PM -0700, Tony Lindgren wrote:
> > Let's keep the device enabled between cppi41_dma_issue_pending()
> > and dmaengine_desc_get_callback_invoke() and rely on the PM runtime
> > autoidle timeout elsewhere.
> > 
> > As the PM runtime is for whole device, not for each channel,
> > we need to queue pending transfers if the device is PM runtime
> > suspended. Then we start the pending transfers in PM runtime
> > resume.
> 
> This fails to apply for me :(
> 
> Can you please rebase and resend this one.

OK sorry about that, that was against Linux next. The conflict
is with c->txd.callback and dmaengine_desc_get_callback_invoke.

Below is a version against your cppi branch.

Regards,

Tony

8< ---------------------
From: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
Date: Wed, 31 Aug 2016 07:19:59 -0700
Subject: [PATCHv2] dmaengine: cppi41: Add basic PM runtime support

Let's keep the device enabled between cppi41_dma_issue_pending()
and dmaengine_desc_get_callback_invoke() and rely on the PM runtime
autoidle timeout elsewhere.

As the PM runtime is for whole device, not for each channel,
we need to queue pending transfers if the device is PM runtime
suspended. Then we start the pending transfers in PM runtime
resume.

Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
---
 drivers/dma/cppi41.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 99 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -108,6 +108,8 @@ struct cppi41_channel {
 	unsigned td_queued:1;
 	unsigned td_seen:1;
 	unsigned td_desc_seen:1;
+
+	struct list_head node;		/* Node for pending list */
 };
 
 struct cppi41_desc {
@@ -146,6 +148,9 @@ struct cppi41_dd {
 	const struct chan_queues *queues_tx;
 	struct chan_queues td_queue;
 
+	struct list_head pending;	/* Pending queued transfers */
+	spinlock_t lock;		/* Lock for pending list */
+
 	/* context for suspend/resume */
 	unsigned int dma_tdfdq;
 };
@@ -332,6 +337,10 @@ static irqreturn_t cppi41_irq(int irq, void *data)
 			c->residue = pd_trans_len(c->desc->pd6) - len;
 			dma_cookie_complete(&c->txd);
 			c->txd.callback(c->txd.callback_param);
+
+			/* Paired with cppi41_dma_issue_pending */
+			pm_runtime_mark_last_busy(cdd->ddev.dev);
+			pm_runtime_put_autosuspend(cdd->ddev.dev);
 		}
 	}
 	return IRQ_HANDLED;
@@ -349,6 +358,12 @@ static dma_cookie_t cppi41_tx_submit(struct dma_async_tx_descriptor *tx)
 static int cppi41_dma_alloc_chan_resources(struct dma_chan *chan)
 {
 	struct cppi41_channel *c = to_cpp41_chan(chan);
+	struct cppi41_dd *cdd = c->cdd;
+	int error;
+
+	error = pm_runtime_get_sync(cdd->ddev.dev);
+	if (error < 0)
+		return error;
 
 	dma_cookie_init(chan);
 	dma_async_tx_descriptor_init(&c->txd, chan);
@@ -357,11 +372,26 @@ static int cppi41_dma_alloc_chan_resources(struct dma_chan *chan)
 	if (!c->is_tx)
 		cppi_writel(c->q_num, c->gcr_reg + RXHPCRA0);
 
+	pm_runtime_mark_last_busy(cdd->ddev.dev);
+	pm_runtime_put_autosuspend(cdd->ddev.dev);
+
 	return 0;
 }
 
 static void cppi41_dma_free_chan_resources(struct dma_chan *chan)
 {
+	struct cppi41_channel *c = to_cpp41_chan(chan);
+	struct cppi41_dd *cdd = c->cdd;
+	int error;
+
+	error = pm_runtime_get_sync(cdd->ddev.dev);
+	if (error < 0)
+		return;
+
+	WARN_ON(!list_empty(&cdd->pending));
+
+	pm_runtime_mark_last_busy(cdd->ddev.dev);
+	pm_runtime_put_autosuspend(cdd->ddev.dev);
 }
 
 static enum dma_status cppi41_dma_tx_status(struct dma_chan *chan,
@@ -414,11 +444,35 @@ static void push_desc_queue(struct cppi41_channel *c)
 	cppi_writel(reg, cdd->qmgr_mem + QMGR_QUEUE_D(c->q_num));
 }
 
+static void pending_desc(struct cppi41_channel *c)
+{
+	struct cppi41_dd *cdd = c->cdd;
+	unsigned long flags;
+
+	spin_lock_irqsave(&cdd->lock, flags);
+	list_add_tail(&c->node, &cdd->pending);
+	spin_unlock_irqrestore(&cdd->lock, flags);
+}
+
 static void cppi41_dma_issue_pending(struct dma_chan *chan)
 {
 	struct cppi41_channel *c = to_cpp41_chan(chan);
+	struct cppi41_dd *cdd = c->cdd;
+	int error;
+
+	/* PM runtime paired with dmaengine_desc_get_callback_invoke */
+	error = pm_runtime_get(cdd->ddev.dev);
+	if (error < 0) {
+		dev_err(cdd->ddev.dev, "Failed to pm_runtime_get: %i\n",
+			error);
 
-	push_desc_queue(c);
+		return;
+	}
+
+	if (likely(pm_runtime_active(cdd->ddev.dev)))
+		push_desc_queue(c);
+	else
+		pending_desc(c);
 }
 
 static u32 get_host_pd0(u32 length)
@@ -940,12 +994,18 @@ static int cppi41_dma_probe(struct platform_device *pdev)
 	cdd->ctrl_mem = of_iomap(dev->of_node, 1);
 	cdd->sched_mem = of_iomap(dev->of_node, 2);
 	cdd->qmgr_mem = of_iomap(dev->of_node, 3);
+	spin_lock_init(&cdd->lock);
+	INIT_LIST_HEAD(&cdd->pending);
+
+	platform_set_drvdata(pdev, cdd);
 
 	if (!cdd->usbss_mem || !cdd->ctrl_mem || !cdd->sched_mem ||
 			!cdd->qmgr_mem)
 		return -ENXIO;
 
 	pm_runtime_enable(dev);
+	pm_runtime_set_autosuspend_delay(dev, 100);
+	pm_runtime_use_autosuspend(dev);
 	ret = pm_runtime_get_sync(dev);
 	if (ret < 0)
 		goto err_get_sync;
@@ -985,7 +1045,9 @@ static int cppi41_dma_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_of;
 
-	platform_set_drvdata(pdev, cdd);
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
+
 	return 0;
 err_of:
 	dma_async_device_unregister(&cdd->ddev);
@@ -996,7 +1058,8 @@ err_irq:
 err_chans:
 	deinit_cppi41(dev, cdd);
 err_init_cppi:
-	pm_runtime_put(dev);
+	pm_runtime_dont_use_autosuspend(dev);
+	pm_runtime_put_sync(dev);
 err_get_sync:
 	pm_runtime_disable(dev);
 	iounmap(cdd->usbss_mem);
@@ -1021,7 +1084,8 @@ static int cppi41_dma_remove(struct platform_device *pdev)
 	iounmap(cdd->ctrl_mem);
 	iounmap(cdd->sched_mem);
 	iounmap(cdd->qmgr_mem);
-	pm_runtime_put(&pdev->dev);
+	pm_runtime_dont_use_autosuspend(&pdev->dev);
+	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 	return 0;
 }
@@ -1062,9 +1126,39 @@ static int cppi41_resume(struct device *dev)
 
 	return 0;
 }
+
+static int cppi41_runtime_suspend(struct device *dev)
+{
+	struct cppi41_dd *cdd = dev_get_drvdata(dev);
+
+	WARN_ON(!list_empty(&cdd->pending));
+
+	return 0;
+}
+
+static int cppi41_runtime_resume(struct device *dev)
+{
+	struct cppi41_dd *cdd = dev_get_drvdata(dev);
+	struct cppi41_channel *c, *_c;
+	unsigned long flags;
+
+	spin_lock_irqsave(&cdd->lock, flags);
+	list_for_each_entry_safe(c, _c, &cdd->pending, node) {
+		push_desc_queue(c);
+		list_del(&c->node);
+	}
+	spin_unlock_irqrestore(&cdd->lock, flags);
+
+	return 0;
+}
 #endif
 
-static SIMPLE_DEV_PM_OPS(cppi41_pm_ops, cppi41_suspend, cppi41_resume);
+static const struct dev_pm_ops cppi41_pm_ops = {
+	SET_LATE_SYSTEM_SLEEP_PM_OPS(cppi41_suspend, cppi41_resume)
+	SET_RUNTIME_PM_OPS(cppi41_runtime_suspend,
+			   cppi41_runtime_resume,
+			   NULL)
+};
 
 static struct platform_driver cpp41_dma_driver = {
 	.probe  = cppi41_dma_probe,
-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] dmaengine: cppi41: Add basic PM runtime support

[Vinod Koul]

On Wed, Aug 31, 2016 at 07:30:55AM -0700, Tony Lindgren wrote:
> * Vinod Koul <vinod.koul-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> [160830 21:45]:
> > On Fri, Aug 19, 2016 at 03:59:40PM -0700, Tony Lindgren wrote:
> > > Let's keep the device enabled between cppi41_dma_issue_pending()
> > > and dmaengine_desc_get_callback_invoke() and rely on the PM runtime
> > > autoidle timeout elsewhere.
> > > 
> > > As the PM runtime is for whole device, not for each channel,
> > > we need to queue pending transfers if the device is PM runtime
> > > suspended. Then we start the pending transfers in PM runtime
> > > resume.
> > 
> > This fails to apply for me :(
> > 
> > Can you please rebase and resend this one.
> 
> OK sorry about that, that was against Linux next. The conflict
> is with c->txd.callback and dmaengine_desc_get_callback_invoke.
> 
> Below is a version against your cppi branch.

Thanks, applied now

-- 
~Vinod
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html