From fbe930ad0e3c40ffda73ab9c40ec497e28ac507d Mon Sep 17 00:00:00 2001
From: Tomasz Leman <tomasz.m.leman@intel.com>
Date: Fri, 17 Mar 2023 11:43:15 +0100
Subject: [PATCH] driver: gpdma: balance the pm usage

Because the DMA driver allows multiple start and stop calls for the same
instance and the same channel, we cannot rely on the error codes
returned by these functions to notify the device's power manager that a
device is still in use.

Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
---
 drivers/dma/dma_dw_common.c        |  4 +++-
 drivers/dma/dma_intel_adsp_gpdma.c | 31 ++++++++++++++++++++++++------
 2 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/dma_dw_common.c b/drivers/dma/dma_dw_common.c
index 87bc48d3c2fad8..d216dd00b0f4de 100644
--- a/drivers/dma/dma_dw_common.c
+++ b/drivers/dma/dma_dw_common.c
@@ -12,6 +12,7 @@
 #include <zephyr/device.h>
 #include <zephyr/init.h>
 #include <zephyr/drivers/dma.h>
+#include <zephyr/pm/device_runtime.h>
 #include <soc.h>
 #include "dma_dw_common.h"
 
@@ -508,6 +509,7 @@ int dw_dma_start(const struct device *dev, uint32_t channel)
 
 	/* enable the channel */
 	dw_write(dev_cfg->base, DW_DMA_CHAN_EN, DW_CHAN_UNMASK(channel));
+	ret = pm_device_runtime_get(dev);
 
 out:
 	return ret;
@@ -577,7 +579,7 @@ int dw_dma_stop(const struct device *dev, uint32_t channel)
 	}
 #endif
 	chan_data->state = DW_DMA_IDLE;
-
+	ret = pm_device_runtime_put(dev);
 out:
 	return ret;
 }
diff --git a/drivers/dma/dma_intel_adsp_gpdma.c b/drivers/dma/dma_intel_adsp_gpdma.c
index 5d673e36cee135..2435fdbf6883dd 100644
--- a/drivers/dma/dma_intel_adsp_gpdma.c
+++ b/drivers/dma/dma_intel_adsp_gpdma.c
@@ -161,7 +161,24 @@ static int intel_adsp_gpdma_config(const struct device *dev, uint32_t channel,
 
 static int intel_adsp_gpdma_start(const struct device *dev, uint32_t channel)
 {
-	int ret;
+	int ret = 0;
+	bool first_use = false;
+	enum pm_device_state state;
+
+	/* We need to power-up device before using it. So in case of a GPDMA, we need to check if
+	 * the current instance is already active, and if not, we let the power manager know that
+	 * we want to use it.
+	 */
+	if (pm_device_state_get(dev, &state) != -ENOSYS) {
+		first_use = state != PM_DEVICE_STATE_ACTIVE;
+		if (first_use) {
+			ret = pm_device_runtime_get(dev);
+		}
+	}
+
+	if (ret < 0) {
+		return ret;
+	}
 
 	intel_adsp_gpdma_llp_enable(dev, channel);
 	ret = dw_dma_start(dev, channel);
@@ -169,8 +186,12 @@ static int intel_adsp_gpdma_start(const struct device *dev, uint32_t channel)
 		intel_adsp_gpdma_llp_disable(dev, channel);
 	}
 
-	if (ret == 0) {
-		ret = pm_device_runtime_get(dev);
+	/* Device usage is counted by the calls of dw_dma_start and dw_dma_stop. For the first use,
+	 * we need to make sure that the pm_device_runtime_get and pm_device_runtime_put functions
+	 * calls are balanced.
+	 */
+	if (first_use) {
+		ret = pm_device_runtime_put(dev);
 	}
 
 	return ret;
@@ -178,12 +199,10 @@ static int intel_adsp_gpdma_start(const struct device *dev, uint32_t channel)
 
 static int intel_adsp_gpdma_stop(const struct device *dev, uint32_t channel)
 {
-	int ret;
+	int ret = dw_dma_stop(dev, channel);
 
-	ret = dw_dma_stop(dev, channel);
 	if (ret == 0) {
 		intel_adsp_gpdma_llp_disable(dev, channel);
-		ret = pm_device_runtime_put(dev);
 	}
 
 	return ret;