From mboxrd@z Thu Jan 1 00:00:00 1970 From: Krzysztof Kozlowski Subject: Re: [PATCH v7 4/5] dma: pl330: add Power Management support Date: Wed, 15 Oct 2014 10:09:10 +0200 Message-ID: <1413360550.25079.9.camel@AMDC1943> References: <1411566612-32277-1-git-send-email-k.kozlowski@samsung.com> <1411566612-32277-5-git-send-email-k.kozlowski@samsung.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: Received: from mailout3.w1.samsung.com ([210.118.77.13]:44915 "EHLO mailout3.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751281AbaJOIJP (ORCPT ); Wed, 15 Oct 2014 04:09:15 -0400 In-reply-to: Sender: linux-pm-owner@vger.kernel.org List-Id: linux-pm@vger.kernel.org To: Ulf Hansson Cc: Russell King , Dan Williams , Vinod Koul , "linux-kernel@vger.kernel.org" , dmaengine@vger.kernel.org, Grant Likely , Lars-Peter Clausen , Michal Simek , "Rafael J. Wysocki" , Len Brown , Pavel Machek , Randy Dunlap , Alan Stern , "linux-doc@vger.kernel.org" , "linux-pm@vger.kernel.org" , Kyungmin Park , Marek Szyprowski , Bartlomiej Zolnierkiewicz On =C5=9Bro, 2014-10-15 at 09:19 +0200, Ulf Hansson wrote: > On 24 September 2014 15:50, Krzysztof Kozlowski wrote: > > This patch adds both normal PM suspend/resume support and runtime P= M > > support to pl330 DMA engine driver. > > > > The runtime power management for pl330 DMA driver allows gating of = AMBA > > clock (PDMA) in FSYS clock domain, when the device is not processin= g any > > requests. This is necessary to enter low power modes on Exynos SoCs > > (e.g. LPA on Exynos4x12 or W-AFTR on Exynos3250). > > > > Runtime PM resuming of the device may happen in atomic context (dur= ing > > call device_issue_pending()) so pm_runtime_irq_safe() is used. This= will > > lead only to disabling/enabling of the clock but this is sufficient= for > > gating the clock and for reducing energy usage. > > > > During system sleep the AMBA bus clock is also unprepared. > > > > Suggested-by: Bartlomiej Zolnierkiewicz > > Signed-off-by: Krzysztof Kozlowski >=20 > Reviewed-by: Ulf Hansson Thanks for review! You pointed one issue with patch 2/5 off-the-list. I'll wait for merge window to end, fix it and re-send. Best regards, Krzysztof >=20 > Kind regards > Uffe >=20 > > --- > > drivers/dma/pl330.c | 94 +++++++++++++++++++++++++++++++++++++++++= +++++++++--- > > 1 file changed, 90 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c > > index 4839bfa74a10..b123431e62ed 100644 > > --- a/drivers/dma/pl330.c > > +++ b/drivers/dma/pl330.c > > @@ -27,6 +27,7 @@ > > #include > > #include > > #include > > +#include > > > > #include "dmaengine.h" > > #define PL330_MAX_CHAN 8 > > @@ -265,6 +266,9 @@ static unsigned cmd_line; > > > > #define NR_DEFAULT_DESC 16 > > > > +/* Delay for runtime PM autosuspend, ms */ > > +#define PL330_AUTOSUSPEND_DELAY 20 > > + > > /* Populated by the PL330 core driver for DMA API driver's info */ > > struct pl330_config { > > u32 periph_id; > > @@ -1958,6 +1962,7 @@ static void pl330_tasklet(unsigned long data) > > struct dma_pl330_chan *pch =3D (struct dma_pl330_chan *)dat= a; > > struct dma_pl330_desc *desc, *_dt; > > unsigned long flags; > > + bool power_down =3D false; > > > > spin_lock_irqsave(&pch->lock, flags); > > > > @@ -1972,10 +1977,17 @@ static void pl330_tasklet(unsigned long dat= a) > > /* Try to submit a req imm. next to the last completed cook= ie */ > > fill_queue(pch); > > > > - /* Make sure the PL330 Channel thread is active */ > > - spin_lock(&pch->thread->dmac->lock); > > - _start(pch->thread); > > - spin_unlock(&pch->thread->dmac->lock); > > + if (list_empty(&pch->work_list)) { > > + spin_lock(&pch->thread->dmac->lock); > > + _stop(pch->thread); > > + spin_unlock(&pch->thread->dmac->lock); > > + power_down =3D true; > > + } else { > > + /* Make sure the PL330 Channel thread is active */ > > + spin_lock(&pch->thread->dmac->lock); > > + _start(pch->thread); > > + spin_unlock(&pch->thread->dmac->lock); > > + } > > > > while (!list_empty(&pch->completed_list)) { > > dma_async_tx_callback callback; > > @@ -1990,6 +2002,12 @@ static void pl330_tasklet(unsigned long data= ) > > if (pch->cyclic) { > > desc->status =3D PREP; > > list_move_tail(&desc->node, &pch->work_list= ); > > + if (power_down) { > > + spin_lock(&pch->thread->dmac->lock)= ; > > + _start(pch->thread); > > + spin_unlock(&pch->thread->dmac->loc= k); > > + power_down =3D false; > > + } > > } else { > > desc->status =3D FREE; > > list_move_tail(&desc->node, &pch->dmac->des= c_pool); > > @@ -2004,6 +2022,12 @@ static void pl330_tasklet(unsigned long data= ) > > } > > } > > spin_unlock_irqrestore(&pch->lock, flags); > > + > > + /* If work list empty, power down */ > > + if (power_down) { > > + pm_runtime_mark_last_busy(pch->dmac->ddma.dev); > > + pm_runtime_put_autosuspend(pch->dmac->ddma.dev); > > + } > > } > > > > bool pl330_filter(struct dma_chan *chan, void *param) > > @@ -2073,6 +2097,7 @@ static int pl330_control(struct dma_chan *cha= n, enum dma_ctrl_cmd cmd, unsigned > > > > switch (cmd) { > > case DMA_TERMINATE_ALL: > > + pm_runtime_get_sync(pl330->ddma.dev); > > spin_lock_irqsave(&pch->lock, flags); > > > > spin_lock(&pl330->lock); > > @@ -2099,10 +2124,15 @@ static int pl330_control(struct dma_chan *c= han, enum dma_ctrl_cmd cmd, unsigned > > dma_cookie_complete(&desc->txd); > > } > > > > + if (!list_empty(&pch->work_list)) > > + pm_runtime_put(pl330->ddma.dev); > > + > > list_splice_tail_init(&pch->submitted_list, &pl330-= >desc_pool); > > list_splice_tail_init(&pch->work_list, &pl330->desc= _pool); > > list_splice_tail_init(&pch->completed_list, &pl330-= >desc_pool); > > spin_unlock_irqrestore(&pch->lock, flags); > > + pm_runtime_mark_last_busy(pl330->ddma.dev); > > + pm_runtime_put_autosuspend(pl330->ddma.dev); > > break; > > case DMA_SLAVE_CONFIG: > > slave_config =3D (struct dma_slave_config *)arg; > > @@ -2138,6 +2168,7 @@ static void pl330_free_chan_resources(struct = dma_chan *chan) > > > > tasklet_kill(&pch->task); > > > > + pm_runtime_get_sync(pch->dmac->ddma.dev); > > spin_lock_irqsave(&pch->lock, flags); > > > > pl330_release_channel(pch->thread); > > @@ -2147,6 +2178,8 @@ static void pl330_free_chan_resources(struct = dma_chan *chan) > > list_splice_tail_init(&pch->work_list, &pch->dmac->= desc_pool); > > > > spin_unlock_irqrestore(&pch->lock, flags); > > + pm_runtime_mark_last_busy(pch->dmac->ddma.dev); > > + pm_runtime_put_autosuspend(pch->dmac->ddma.dev); > > } > > > > static enum dma_status > > @@ -2162,6 +2195,15 @@ static void pl330_issue_pending(struct dma_c= han *chan) > > unsigned long flags; > > > > spin_lock_irqsave(&pch->lock, flags); > > + if (list_empty(&pch->work_list)) { > > + /* > > + * Warn on nothing pending. Empty submitted_list ma= y > > + * break our pm_runtime usage counter as it is > > + * updated on work_list emptiness status. > > + */ > > + WARN_ON(list_empty(&pch->submitted_list)); > > + pm_runtime_get_sync(pch->dmac->ddma.dev); > > + } > > list_splice_tail_init(&pch->submitted_list, &pch->work_list= ); > > spin_unlock_irqrestore(&pch->lock, flags); > > > > @@ -2585,6 +2627,41 @@ static int pl330_dma_device_slave_caps(struc= t dma_chan *dchan, > > return 0; > > } > > > > +/* > > + * Assume that IRQ safe runtime PM is chosen in probe and amba bus= driver > > + * will only disable/enable the clock in runtime PM suspend/resume= =2E > > + */ > > +static int __maybe_unused pl330_suspend(struct device *dev) > > +{ > > + struct amba_device *pcdev =3D to_amba_device(dev); > > + int ret; > > + > > + ret =3D pm_runtime_force_suspend(dev); > > + if (ret) > > + return ret; > > + > > + amba_pclk_unprepare(pcdev); > > + > > + return 0; > > +} > > + > > +static int __maybe_unused pl330_resume(struct device *dev) > > +{ > > + struct amba_device *pcdev =3D to_amba_device(dev); > > + > > + amba_pclk_prepare(pcdev); > > + > > + /* > > + * TODO: Idea for future. The device should not be woken up= after > > + * system resume if it is not needed. It could stay runtime= suspended > > + * waiting for DMA requests. However for safe suspend and r= esume we > > + * forcibly resume the device here. > > + */ > > + return pm_runtime_force_resume(dev); > > +} > > + > > +static SIMPLE_DEV_PM_OPS(pl330_pm, pl330_suspend, pl330_resume); > > + > > static int > > pl330_probe(struct amba_device *adev, const struct amba_id *id) > > { > > @@ -2738,6 +2815,12 @@ pl330_probe(struct amba_device *adev, const = struct amba_id *id) > > pcfg->data_buf_dep, pcfg->data_bus_width / 8, pcfg-= >num_chan, > > pcfg->num_peri, pcfg->num_events); > > > > + pm_runtime_irq_safe(&adev->dev); > > + pm_runtime_use_autosuspend(&adev->dev); > > + pm_runtime_set_autosuspend_delay(&adev->dev, PL330_AUTOSUSP= END_DELAY); > > + pm_runtime_mark_last_busy(&adev->dev); > > + pm_runtime_put_autosuspend(&adev->dev); > > + > > return 0; > > probe_err3: > > /* Idle the DMAC */ > > @@ -2764,6 +2847,8 @@ static int pl330_remove(struct amba_device *a= dev) > > struct pl330_dmac *pl330 =3D amba_get_drvdata(adev); > > struct dma_pl330_chan *pch, *_p; > > > > + pm_runtime_get_noresume(pl330->ddma.dev); > > + > > if (adev->dev.of_node) > > of_dma_controller_free(adev->dev.of_node); > > > > @@ -2802,6 +2887,7 @@ static struct amba_driver pl330_driver =3D { > > .drv =3D { > > .owner =3D THIS_MODULE, > > .name =3D "dma-pl330", > > + .pm =3D &pl330_pm, > > }, > > .id_table =3D pl330_ids, > > .probe =3D pl330_probe, > > -- > > 1.9.1 > >