Re: [PATCH] dmaengine: pl330: Fix unbalanced runtime PM

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Vinod,

在 2022/4/11 21:48, Vinod Koul 写道:
On 26-03-22, 20:16, Sugar Zhang wrote:
This driver use runtime PM autosuspend mechanism to manager clk.

   pm_runtime_use_autosuspend(&adev->dev);
   pm_runtime_set_autosuspend_delay(&adev->dev, PL330_AUTOSUSPEND_DELAY);

So, after ref count reached to zero, it will enter suspend
after the delay time elapsed.

The unbalanced PM:

* May cause dmac the next start failed.
* May cause dmac read unexpected state.
* May cause dmac stall if power down happen at the middle of the transfer.
   e.g. may lose ack from AXI bus and stall.

Considering the following situation:

       DMA TERMINATE               TASKLET ROUTINE
             |                            |
             |                       issue_pending
             |                            |
             |                     pch->active = true
             |                       pm_runtime_get
   pm_runtime_put(if active)              |
     pch->active = false                  |
             |                      work_list empty
             |                            |
             |                     pm_runtime_put(force)
maybe unconditional is a better word than force here?
okay, will do in v2.

             |                            |

At this point, it's unbalanced(1 get / 2 put).

After this patch:

       DMA TERMINATE               TASKLET ROUTINE
             |                            |
             |                       issue_pending
             |                            |
             |                     pch->active = true
             |                       pm_runtime_get
   pm_runtime_put(if active)              |
     pch->active = false                  |
             |                      work_list empty
             |                            |
             |                   pm_runtime_put(if active)
             |                            |

Now, it's balanced(1 get / 1 put).

Fixes:
commit 5c9e6c2b2ba3 ("dmaengine: pl330: Fix runtime PM support for terminated transfers")
commit ae43b3289186 ("ARM: 8202/1: dmaengine: pl330: Add runtime Power Management support v12")
That is not the right way for Fixes tag

like this?

Fixes: 5c9e6c2b2ba3 ("dmaengine: pl330: Fix runtime PM support for terminated transfers")
Fixes: ae43b3289186 ("ARM: 8202/1: dmaengine: pl330: Add runtime Power Management support v12")

Signed-off-by: Huibin Hong <huibin.hong@xxxxxxxxxxxxxx>
Signed-off-by: Sugar Zhang <sugar.zhang@xxxxxxxxxxxxxx>
---

  drivers/dma/pl330.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index 858400e..ccd430e 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -2084,7 +2084,7 @@ static void pl330_tasklet(struct tasklet_struct *t)
  		spin_lock(&pch->thread->dmac->lock);
  		_stop(pch->thread);
  		spin_unlock(&pch->thread->dmac->lock);
-		power_down = true;
+		power_down = pch->active;
  		pch->active = false;
  	} else {
  		/* Make sure the PL330 Channel thread is active */
--
2.7.4

--
Best Regards!
张学广/Sugar
瑞芯微电子股份有限公司
Rockchip Electronics Co., Ltd.




[Index of Archives]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux PCI]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux