[PATCH v4 0/2] Exynos IOMMU: proper runtime PM support (use device dependencies)

[PATCH v4 0/2] Exynos IOMMU: proper runtime PM support (use device dependencies)

[Marek Szyprowski]

Hello,

This patch series finally implements proper runtime PM support in Exynos
IOMMU driver. This has been achieved by using recently introduce device
links, which lets SYSMMU controller's runtime PM to follow master's device
runtime PM state (the device which actually performs DMA transaction).
The main idea behind this solution is an observation that any DMA activity
from master device can be done only when master device is active, thus when
master device is suspended SYSMMU controller device can also be suspended.

This patchset solves the situation that power domains are always enabled,
because all SYSMMU controllers (which belongs to those domains) are
permanently active (because existing driver was simplified and kept
SYSMMU device active all the time after initialization).

Patch requires fourth version of Rafeal's "Functional dependencies
between devices" patchset, which is available here:
https://www.mail-archive.com/linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg1241473.html

If one wants to test this patchset, I've provided a branch with all needed
patches (some fixes for Exynos4 FIMC-IS driver are needed):
https://git.linaro.org/people/marek.szyprowski/linux-srpol.git v4.8-iommu-pm-v4

Patches are based on vanilla v4.8-rc8 kernel with Rafael's device
dependencies v4 patchset applied.

Best regards
Marek Szyprowski
Samsung R&D Institute Poland


Changelog:
v4:
- rebased on top of v4 of device dependencies/links patchset, what resolved
  system hang on reboot

v3: http://www.spinics.net/lists/linux-samsung-soc/msg55256.html
- rebased on top of latest device dependencies/links patchset
- added proper locking between runtime pm, iommu_attach/detach and sysmmu
  enable/disable(added per iommu owner device's rpm lock)

v2: http://www.spinics.net/lists/arm-kernel/msg512082.html
- replaced PM notifiers with generic device dependencies/links developped
  by Rafael J. Wysocki

v1: http://www.spinics.net/lists/arm-kernel/msg509600.html
- initial version


Patch summary:

Marek Szyprowski (2):
  iommu/exynos: Remove excessive, useless debug
  iommu/exynos: Add proper runtime pm support

 drivers/iommu/exynos-iommu.c | 228 ++++++++++++++++++-------------------------
 1 file changed, 94 insertions(+), 134 deletions(-)

-- 
1.9.1

[PATCH v4 1/2] iommu/exynos: Remove excessive, useless debug

[Marek Szyprowski]

Remove excessive, useless debug about skipping TLB invalidation, which
is a normal situation when more aggressive power management is enabled.

Signed-off-by: Marek Szyprowski <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 drivers/iommu/exynos-iommu.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 33dcc29ec200..dfb44034b4ee 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -578,9 +578,6 @@ static void sysmmu_tlb_invalidate_entry(struct sysmmu_drvdata *data,
 			sysmmu_unblock(data);
 		}
 		clk_disable(data->clk_master);
-	} else {
-		dev_dbg(data->master,
-			"disabled. Skipping TLB invalidation @ %#x\n", iova);
 	}
 	spin_unlock_irqrestore(&data->lock, flags);
 }
-- 
1.9.1

[PATCH v4 2/2] iommu/exynos: Add proper runtime pm support

[Marek Szyprowski]

This patch uses recently introduced device links to track the runtime pm
state of the master's device. This way each SYSMMU controller is runtime
activated when its master's device is active and can save/restore its state
instead of being enabled all the time. This way SYSMMU controllers no
longer prevents respective power domains to be turned off when master's
device is not used.

Signed-off-by: Marek Szyprowski <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 drivers/iommu/exynos-iommu.c | 225 ++++++++++++++++++-------------------------
 1 file changed, 94 insertions(+), 131 deletions(-)

diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index dfb44034b4ee..c8926e030713 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -206,6 +206,7 @@ static const struct sysmmu_fault_info sysmmu_v5_faults[] = {
 struct exynos_iommu_owner {
 	struct list_head controllers;	/* list of sysmmu_drvdata.owner_node */
 	struct iommu_domain *domain;	/* domain this device is attached */
+	struct mutex rpm_lock;		/* for runtime pm of all sysmmus */
 };
 
 /*
@@ -237,8 +238,8 @@ struct sysmmu_drvdata {
 	struct clk *aclk;		/* SYSMMU's aclk clock */
 	struct clk *pclk;		/* SYSMMU's pclk clock */
 	struct clk *clk_master;		/* master's device clock */
-	int activations;		/* number of calls to sysmmu_enable */
 	spinlock_t lock;		/* lock for modyfying state */
+	int active;			/* current status */
 	struct exynos_iommu_domain *domain; /* domain we belong to */
 	struct list_head domain_node;	/* node for domain clients list */
 	struct list_head owner_node;	/* node for owner controllers list */
@@ -251,25 +252,6 @@ static struct exynos_iommu_domain *to_exynos_domain(struct iommu_domain *dom)
 	return container_of(dom, struct exynos_iommu_domain, domain);
 }
 
-static bool set_sysmmu_active(struct sysmmu_drvdata *data)
-{
-	/* return true if the System MMU was not active previously
-	   and it needs to be initialized */
-	return ++data->activations == 1;
-}
-
-static bool set_sysmmu_inactive(struct sysmmu_drvdata *data)
-{
-	/* return true if the System MMU is needed to be disabled */
-	BUG_ON(data->activations < 1);
-	return --data->activations == 0;
-}
-
-static bool is_sysmmu_active(struct sysmmu_drvdata *data)
-{
-	return data->activations > 0;
-}
-
 static void sysmmu_unblock(struct sysmmu_drvdata *data)
 {
 	writel(CTRL_ENABLE, data->sfrbase + REG_MMU_CTRL);
@@ -388,7 +370,7 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id)
 	unsigned short reg_status, reg_clear;
 	int ret = -ENOSYS;
 
-	WARN_ON(!is_sysmmu_active(data));
+	WARN_ON(!data->active);
 
 	if (MMU_MAJ_VER(data->version) < 5) {
 		reg_status = REG_INT_STATUS;
@@ -434,40 +416,19 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static void __sysmmu_disable_nocount(struct sysmmu_drvdata *data)
+static void __sysmmu_disable(struct sysmmu_drvdata *data)
 {
+	unsigned long flags;
+
 	clk_enable(data->clk_master);
 
+	spin_lock_irqsave(&data->lock, flags);
 	writel(CTRL_DISABLE, data->sfrbase + REG_MMU_CTRL);
 	writel(0, data->sfrbase + REG_MMU_CFG);
-
-	__sysmmu_disable_clocks(data);
-}
-
-static bool __sysmmu_disable(struct sysmmu_drvdata *data)
-{
-	bool disabled;
-	unsigned long flags;
-
-	spin_lock_irqsave(&data->lock, flags);
-
-	disabled = set_sysmmu_inactive(data);
-
-	if (disabled) {
-		data->pgtable = 0;
-		data->domain = NULL;
-
-		__sysmmu_disable_nocount(data);
-
-		dev_dbg(data->sysmmu, "Disabled\n");
-	} else  {
-		dev_dbg(data->sysmmu, "%d times left to disable\n",
-					data->activations);
-	}
-
+	data->active = false;
 	spin_unlock_irqrestore(&data->lock, flags);
 
-	return disabled;
+	__sysmmu_disable_clocks(data);
 }
 
 static void __sysmmu_init_config(struct sysmmu_drvdata *data)
@@ -484,17 +445,19 @@ static void __sysmmu_init_config(struct sysmmu_drvdata *data)
 	writel(cfg, data->sfrbase + REG_MMU_CFG);
 }
 
-static void __sysmmu_enable_nocount(struct sysmmu_drvdata *data)
+static void __sysmmu_enable(struct sysmmu_drvdata *data)
 {
+	unsigned long flags;
+
 	__sysmmu_enable_clocks(data);
 
+	spin_lock_irqsave(&data->lock, flags);
 	writel(CTRL_BLOCK, data->sfrbase + REG_MMU_CTRL);
-
 	__sysmmu_init_config(data);
-
 	__sysmmu_set_ptbase(data, data->pgtable);
-
 	writel(CTRL_ENABLE, data->sfrbase + REG_MMU_CTRL);
+	data->active = true;
+	spin_unlock_irqrestore(&data->lock, flags);
 
 	/*
 	 * SYSMMU driver keeps master's clock enabled only for the short
@@ -505,48 +468,18 @@ static void __sysmmu_enable_nocount(struct sysmmu_drvdata *data)
 	clk_disable(data->clk_master);
 }
 
-static int __sysmmu_enable(struct sysmmu_drvdata *data, phys_addr_t pgtable,
-			   struct exynos_iommu_domain *domain)
-{
-	int ret = 0;
-	unsigned long flags;
-
-	spin_lock_irqsave(&data->lock, flags);
-	if (set_sysmmu_active(data)) {
-		data->pgtable = pgtable;
-		data->domain = domain;
-
-		__sysmmu_enable_nocount(data);
-
-		dev_dbg(data->sysmmu, "Enabled\n");
-	} else {
-		ret = (pgtable == data->pgtable) ? 1 : -EBUSY;
-
-		dev_dbg(data->sysmmu, "already enabled\n");
-	}
-
-	if (WARN_ON(ret < 0))
-		set_sysmmu_inactive(data); /* decrement count */
-
-	spin_unlock_irqrestore(&data->lock, flags);
-
-	return ret;
-}
-
 static void sysmmu_tlb_invalidate_flpdcache(struct sysmmu_drvdata *data,
 					    sysmmu_iova_t iova)
 {
 	unsigned long flags;
 
-
 	spin_lock_irqsave(&data->lock, flags);
-	if (is_sysmmu_active(data) && data->version >= MAKE_MMU_VER(3, 3)) {
+	if (data->active && data->version >= MAKE_MMU_VER(3, 3)) {
 		clk_enable(data->clk_master);
 		__sysmmu_tlb_invalidate_entry(data, iova, 1);
 		clk_disable(data->clk_master);
 	}
 	spin_unlock_irqrestore(&data->lock, flags);
-
 }
 
 static void sysmmu_tlb_invalidate_entry(struct sysmmu_drvdata *data,
@@ -555,7 +488,7 @@ static void sysmmu_tlb_invalidate_entry(struct sysmmu_drvdata *data,
 	unsigned long flags;
 
 	spin_lock_irqsave(&data->lock, flags);
-	if (is_sysmmu_active(data)) {
+	if (data->active) {
 		unsigned int num_inv = 1;
 
 		clk_enable(data->clk_master);
@@ -656,40 +589,55 @@ static int __init exynos_sysmmu_probe(struct platform_device *pdev)
 	}
 
 	pm_runtime_enable(dev);
-
 	of_iommu_set_ops(dev->of_node, &exynos_iommu_ops);
 
 	return 0;
 }
 
-#ifdef CONFIG_PM_SLEEP
 static int exynos_sysmmu_suspend(struct device *dev)
 {
 	struct sysmmu_drvdata *data = dev_get_drvdata(dev);
+	struct exynos_iommu_owner *owner;
 
-	dev_dbg(dev, "suspend\n");
-	if (is_sysmmu_active(data)) {
-		__sysmmu_disable_nocount(data);
-		pm_runtime_put(dev);
+	if (!data->master)
+		return 0;
+
+	owner = data->master->archdata.iommu;
+
+	mutex_lock(&owner->rpm_lock);
+	if (data->domain) {
+		dev_dbg(data->sysmmu, "saving state\n");
+		__sysmmu_disable(data);
 	}
+	mutex_unlock(&owner->rpm_lock);
+
 	return 0;
 }
 
 static int exynos_sysmmu_resume(struct device *dev)
 {
 	struct sysmmu_drvdata *data = dev_get_drvdata(dev);
+	struct exynos_iommu_owner *owner;
 
-	dev_dbg(dev, "resume\n");
-	if (is_sysmmu_active(data)) {
-		pm_runtime_get_sync(dev);
-		__sysmmu_enable_nocount(data);
+	if (!data->master)
+		return 0;
+
+	owner = data->master->archdata.iommu;
+
+	mutex_lock(&owner->rpm_lock);
+	if (data->domain) {
+		dev_dbg(data->sysmmu, "restoring state\n");
+		__sysmmu_enable(data);
 	}
+	mutex_unlock(&owner->rpm_lock);
+
 	return 0;
 }
-#endif
 
 static const struct dev_pm_ops sysmmu_pm_ops = {
-	SET_LATE_SYSTEM_SLEEP_PM_OPS(exynos_sysmmu_suspend, exynos_sysmmu_resume)
+	SET_RUNTIME_PM_OPS(exynos_sysmmu_suspend, exynos_sysmmu_resume, NULL)
+	SET_LATE_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+				     pm_runtime_force_resume)
 };
 
 static const struct of_device_id sysmmu_of_match[] __initconst = {
@@ -793,9 +741,12 @@ static void exynos_iommu_domain_free(struct iommu_domain *iommu_domain)
 	spin_lock_irqsave(&domain->lock, flags);
 
 	list_for_each_entry_safe(data, next, &domain->clients, domain_node) {
-		if (__sysmmu_disable(data))
-			data->master = NULL;
+		spin_lock(&data->lock);
+		__sysmmu_disable(data);
+		data->pgtable = 0;
+		data->domain = NULL;
 		list_del_init(&data->domain_node);
+		spin_unlock(&data->lock);
 	}
 
 	spin_unlock_irqrestore(&domain->lock, flags);
@@ -829,31 +780,32 @@ static void exynos_iommu_detach_device(struct iommu_domain *iommu_domain,
 	phys_addr_t pagetable = virt_to_phys(domain->pgtable);
 	struct sysmmu_drvdata *data, *next;
 	unsigned long flags;
-	bool found = false;
 
 	if (!has_sysmmu(dev) || owner->domain != iommu_domain)
 		return;
 
+	mutex_lock(&owner->rpm_lock);
+
+	list_for_each_entry(data, &owner->controllers, owner_node) {
+		if (pm_runtime_active(data->sysmmu))
+			__sysmmu_disable(data);
+	}
+
 	spin_lock_irqsave(&domain->lock, flags);
 	list_for_each_entry_safe(data, next, &domain->clients, domain_node) {
-		if (data->master == dev) {
-			if (__sysmmu_disable(data)) {
-				data->master = NULL;
-				list_del_init(&data->domain_node);
-			}
-			pm_runtime_put(data->sysmmu);
-			found = true;
-		}
+		spin_lock(&data->lock);
+		data->pgtable = 0;
+		data->domain = NULL;
+		list_del_init(&data->domain_node);
+		spin_unlock(&data->lock);
 	}
+	owner->domain = NULL;
 	spin_unlock_irqrestore(&domain->lock, flags);
 
-	owner->domain = NULL;
+	mutex_unlock(&owner->rpm_lock);
 
-	if (found)
-		dev_dbg(dev, "%s: Detached IOMMU with pgtable %pa\n",
-					__func__, &pagetable);
-	else
-		dev_err(dev, "%s: No IOMMU is attached\n", __func__);
+	dev_dbg(dev, "%s: Detached IOMMU with pgtable %pa\n", __func__,
+		&pagetable);
 }
 
 static int exynos_iommu_attach_device(struct iommu_domain *iommu_domain,
@@ -864,7 +816,6 @@ static int exynos_iommu_attach_device(struct iommu_domain *iommu_domain,
 	struct sysmmu_drvdata *data;
 	phys_addr_t pagetable = virt_to_phys(domain->pgtable);
 	unsigned long flags;
-	int ret = -ENODEV;
 
 	if (!has_sysmmu(dev))
 		return -ENODEV;
@@ -872,29 +823,30 @@ static int exynos_iommu_attach_device(struct iommu_domain *iommu_domain,
 	if (owner->domain)
 		exynos_iommu_detach_device(owner->domain, dev);
 
+	mutex_lock(&owner->rpm_lock);
+
+	spin_lock_irqsave(&domain->lock, flags);
 	list_for_each_entry(data, &owner->controllers, owner_node) {
-		pm_runtime_get_sync(data->sysmmu);
-		ret = __sysmmu_enable(data, pagetable, domain);
-		if (ret >= 0) {
-			data->master = dev;
-
-			spin_lock_irqsave(&domain->lock, flags);
-			list_add_tail(&data->domain_node, &domain->clients);
-			spin_unlock_irqrestore(&domain->lock, flags);
-		}
+		spin_lock(&data->lock);
+		data->pgtable = pagetable;
+		data->domain = domain;
+		list_add_tail(&data->domain_node, &domain->clients);
+		spin_unlock(&data->lock);
 	}
+	owner->domain = iommu_domain;
+	spin_unlock_irqrestore(&domain->lock, flags);
 
-	if (ret < 0) {
-		dev_err(dev, "%s: Failed to attach IOMMU with pgtable %pa\n",
-					__func__, &pagetable);
-		return ret;
+	list_for_each_entry(data, &owner->controllers, owner_node) {
+		if (pm_runtime_active(data->sysmmu))
+			__sysmmu_enable(data);
 	}
 
-	owner->domain = iommu_domain;
-	dev_dbg(dev, "%s: Attached IOMMU with pgtable %pa %s\n",
-		__func__, &pagetable, (ret == 0) ? "" : ", again");
+	mutex_unlock(&owner->rpm_lock);
 
-	return ret;
+	dev_dbg(dev, "%s: Attached IOMMU with pgtable %pa\n", __func__,
+		&pagetable);
+
+	return 0;
 }
 
 static sysmmu_pte_t *alloc_lv2entry(struct exynos_iommu_domain *domain,
@@ -1265,10 +1217,21 @@ static int exynos_iommu_of_xlate(struct device *dev,
 			return -ENOMEM;
 
 		INIT_LIST_HEAD(&owner->controllers);
+		mutex_init(&owner->rpm_lock);
 		dev->archdata.iommu = owner;
 	}
 
 	list_add_tail(&data->owner_node, &owner->controllers);
+	data->master = dev;
+
+	/*
+	 * SYSMMU will be runtime enabled via device link (dependency) to its
+	 * master device, so there are no direct calls to pm_runtime_get/put
+	 * in this driver.
+	 */
+	device_link_add(dev, data->sysmmu, DEVICE_LINK_AVAILABLE,
+			DEVICE_LINK_PM_RUNTIME);
+
 	return 0;
 }
 
-- 
1.9.1

Re: [PATCH v4 2/2] iommu/exynos: Add proper runtime pm support

[Luis R. Rodriguez]

On Thu, Sep 29, 2016 at 10:12:31AM +0200, Marek Szyprowski wrote:
> This patch uses recently introduced device links to track the runtime pm
> state of the master's device. This way each SYSMMU controller is runtime
> activated when its master's device is active 

instead of?

BTW what is the master device of a SYSMMU? I have no clue about these
IOMMU devices here.

> and can save/restore its state instead of being enabled all the time.

I take it this means currently even if the master device is disabled
(whatever that is) all SYSMMU controllers are kept enabled, is that right?
The issue here is this wastes power? Or what is the issue?

> This way SYSMMU controllers no
> longer prevents respective power domains to be turned off when master's
> device is not used.

So when the master device is idle we want to also remove power from the
controllers ? How much power does this save on a typical device in the
market BTW ?

> Signed-off-by: Marek Szyprowski <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/iommu/exynos-iommu.c | 225 ++++++++++++++++++-------------------------
>  1 file changed, 94 insertions(+), 131 deletions(-)

I'm reviewing the device link patches now but since this is a demo of
use of that I'll note the changes here are pretty large and it makes
it terribly difficult for review. Is there any way this patch can be split
up in to logical atomic pieces that only do one task upon change ?

  Luis

Re: [PATCH v4 2/2] iommu/exynos: Add proper runtime pm support

[Marek Szyprowski]

Hi Luis


On 2016-10-06 19:37, Luis R. Rodriguez wrote:
> On Thu, Sep 29, 2016 at 10:12:31AM +0200, Marek Szyprowski wrote:
>> This patch uses recently introduced device links to track the runtime pm
>> state of the master's device. This way each SYSMMU controller is runtime
>> activated when its master's device is active
> instead of?

instead of keeping SYSMMU controller runtime active all the time.

> BTW what is the master device of a SYSMMU? I have no clue about these
> IOMMU devices here.

Here is a more detailed description of IOMMU hardware I wrote a few days ago
for Ulf:
http://www.mail-archive.com/linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg1231006.html

In short: there is a SYSMMU controller and its master device - a device,
which performs DMA operations. That SYSMMU sits in between system memory
and the master device, so it performs mapping of DMA addresses to physical
memory addresses on each DMA operation.

>
>> and can save/restore its state instead of being enabled all the time.
> I take it this means currently even if the master device is disabled
> (whatever that is) all SYSMMU controllers are kept enabled, is that right?
> The issue here is this wastes power? Or what is the issue?

Yes, the issue here is the fact that SYSMMU is kept active all the time,
what in turn prevent the power domain for turning off even if master device
doesn't do anything and is already suspended. This directly (some clocks
enabled) and in-directly (leakage current) causes power looses.

>
>> This way SYSMMU controllers no
>> longer prevents respective power domains to be turned off when master's
>> device is not used.
> So when the master device is idle we want to also remove power from the
> controllers ? How much power does this save on a typical device in the
> market BTW ?

The main purpose of this patchset is to let power domains to be turned off,
because with the current code all domains are all the time turned on, 
because
SYSMMU controllers prevent them from turning them off.

If you want I can measure the power consumption of the idle board with all
domains enabled and disabled if you want to see the numbers. On the 
other board
disabling most power domains in idle state (the clocks were already 
disabled)
gave me about 20mA savings (at 3.7V), what is a significant value for the
battery powered device.

>
>> Signed-off-by: Marek Szyprowski <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> ---
>>   drivers/iommu/exynos-iommu.c | 225 ++++++++++++++++++-------------------------
>>   1 file changed, 94 insertions(+), 131 deletions(-)
> I'm reviewing the device link patches now but since this is a demo of
> use of that I'll note the changes here are pretty large and it makes
> it terribly difficult for review. Is there any way this patch can be split
> up in to logical atomic pieces that only do one task upon change ?

I will try to split it a bit, but I cannot promise that much can be done
to improve readability for someone not very familiar with the driver
internals.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH v4 2/2] iommu/exynos: Add proper runtime pm support

[Luis R. Rodriguez]

On Mon, Oct 10, 2016 at 03:32:06PM +0200, Marek Szyprowski wrote:
> Hi Luis
> 
> 
> On 2016-10-06 19:37, Luis R. Rodriguez wrote:
> > On Thu, Sep 29, 2016 at 10:12:31AM +0200, Marek Szyprowski wrote:
> > > This patch uses recently introduced device links to track the runtime pm
> > > state of the master's device. This way each SYSMMU controller is runtime
> > > activated when its master's device is active
> > instead of?
> 
> instead of keeping SYSMMU controller runtime active all the time.

I thought Rafael's work was for suspend/resume, not for runtime suspend.
Is it for both ? Because as far as I can tell this was painted to help
with suspend/resume ?

> > BTW what is the master device of a SYSMMU? I have no clue about these
> > IOMMU devices here.
> 
> Here is a more detailed description of IOMMU hardware I wrote a few days ago
> for Ulf:
> http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1231006.html
> 
> In short: there is a SYSMMU controller and its master device - a device,
> which performs DMA operations. That SYSMMU sits in between system memory
> and the master device, so it performs mapping of DMA addresses to physical
> memory addresses on each DMA operation.

So you seek a run time power optimization ? Or a fix on suspend? Or both?

> > > and can save/restore its state instead of being enabled all the time.
> > I take it this means currently even if the master device is disabled
> > (whatever that is) all SYSMMU controllers are kept enabled, is that right?
> > The issue here is this wastes power? Or what is the issue?
> 
> Yes, the issue here is the fact that SYSMMU is kept active all the time,
> what in turn prevent the power domain for turning off even if master device
> doesn't do anything and is already suspended. This directly (some clocks
> enabled) and in-directly (leakage current) causes power looses.

Thanks for the confirmation so really the biggest concern here was run time PM.

> > > This way SYSMMU controllers no
> > > longer prevents respective power domains to be turned off when master's
> > > device is not used.
> > So when the master device is idle we want to also remove power from the
> > controllers ? How much power does this save on a typical device in the
> > market BTW ?
> 
> The main purpose of this patchset is to let power domains to be turned off,
> because with the current code all domains are all the time turned on,
> because
> SYSMMU controllers prevent them from turning them off.

I see.. I think the audio folks already addressed this with DAPM, but granted
this was for audio. Then I was also referred to the DRM / Audio component
framework, has this been looked into? v4l folks have v4l async stuff but
its not clear if that help with run time PM. I'm mentioning these given it'd be
silly to re-invent the wheel, additionally if we now have a generic solution
everyone can jump on board with there is quite a bit of work we can do to
dump a lot of old legacy crap.

> If you want I can measure the power consumption of the idle board with all
> domains enabled and disabled if you want to see the numbers. On the other
> board
> disabling most power domains in idle state (the clocks were already
> disabled)
> gave me about 20mA savings (at 3.7V), what is a significant value for the
> battery powered device.

Thanks, this means nothing to me, however it would be value-add to the commit log
as anyone reviewing  this can understand what the goal / savings was for exactly.

> > 
> > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > > ---
> > >   drivers/iommu/exynos-iommu.c | 225 ++++++++++++++++++-------------------------
> > >   1 file changed, 94 insertions(+), 131 deletions(-)
> > I'm reviewing the device link patches now but since this is a demo of
> > use of that I'll note the changes here are pretty large and it makes
> > it terribly difficult for review. Is there any way this patch can be split
> > up in to logical atomic pieces that only do one task upon change ?
> 
> I will try to split it a bit, but I cannot promise that much can be done
> to improve readability for someone not very familiar with the driver
> internals.

I've heard this before, I don't buy it but lets see!

  Luis

Re: [PATCH v4 2/2] iommu/exynos: Add proper runtime pm support

[Marek Szyprowski]

Hi Luis,


On 2016-11-08 23:14, Luis R. Rodriguez wrote:
> On Mon, Oct 10, 2016 at 03:32:06PM +0200, Marek Szyprowski wrote:
>> Hi Luis
>>
>>
>> On 2016-10-06 19:37, Luis R. Rodriguez wrote:
>>> On Thu, Sep 29, 2016 at 10:12:31AM +0200, Marek Szyprowski wrote:
>>>> This patch uses recently introduced device links to track the runtime pm
>>>> state of the master's device. This way each SYSMMU controller is runtime
>>>> activated when its master's device is active
>>> instead of?
>> instead of keeping SYSMMU controller runtime active all the time.
> I thought Rafael's work was for suspend/resume, not for runtime suspend.
> Is it for both ?

Yes, it solves both problems, although the suspend/resume was easy to 
workaround
just by using LATE_SLEEP_OPS.

> Because as far as I can tell this was painted to help
> with suspend/resume ?

It also helped to remove the suspend/resume workaround.

>>> BTW what is the master device of a SYSMMU? I have no clue about these
>>> IOMMU devices here.
>> Here is a more detailed description of IOMMU hardware I wrote a few days ago
>> for Ulf:
>> http://www.mail-archive.com/linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg1231006.html
>>
>> In short: there is a SYSMMU controller and its master device - a device,
>> which performs DMA operations. That SYSMMU sits in between system memory
>> and the master device, so it performs mapping of DMA addresses to physical
>> memory addresses on each DMA operation.
> So you seek a run time power optimization ? Or a fix on suspend? Or both?

The main reason for using device links was to implement proper runtime power
optimization.

>>>> and can save/restore its state instead of being enabled all the time.
>>> I take it this means currently even if the master device is disabled
>>> (whatever that is) all SYSMMU controllers are kept enabled, is that right?
>>> The issue here is this wastes power? Or what is the issue?
>> Yes, the issue here is the fact that SYSMMU is kept active all the time,
>> what in turn prevent the power domain for turning off even if master device
>> doesn't do anything and is already suspended. This directly (some clocks
>> enabled) and in-directly (leakage current) causes power looses.
> Thanks for the confirmation so really the biggest concern here was run time PM.
>
>>>> This way SYSMMU controllers no
>>>> longer prevents respective power domains to be turned off when master's
>>>> device is not used.
>>> So when the master device is idle we want to also remove power from the
>>> controllers ? How much power does this save on a typical device in the
>>> market BTW ?
>> The main purpose of this patchset is to let power domains to be turned off,
>> because with the current code all domains are all the time turned on,
>> because
>> SYSMMU controllers prevent them from turning them off.
> I see.. I think the audio folks already addressed this with DAPM, but granted
> this was for audio. Then I was also referred to the DRM / Audio component
> framework, has this been looked into? v4l folks have v4l async stuff but
> its not clear if that help with run time PM. I'm mentioning these given it'd be
> silly to re-invent the wheel, additionally if we now have a generic solution
> everyone can jump on board with there is quite a bit of work we can do to
> dump a lot of old legacy crap.

Right, probably some workarounds here and there can be removed. However 
components
and v4l-async solutions are for resolving only probe and registration 
issues and they
are some kind of pool for grouping devices and triggering special 
callback once all
requested devices in the pool have probed.

>> If you want I can measure the power consumption of the idle board with all
>> domains enabled and disabled if you want to see the numbers. On the other
>> board
>> disabling most power domains in idle state (the clocks were already
>> disabled)
>> gave me about 20mA savings (at 3.7V), what is a significant value for the
>> battery powered device.
> Thanks, this means nothing to me, however it would be value-add to the commit log
> as anyone reviewing  this can understand what the goal / savings was for exactly.
>
>>>> Signed-off-by: Marek Szyprowski <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>>>> ---
>>>>    drivers/iommu/exynos-iommu.c | 225 ++++++++++++++++++-------------------------
>>>>    1 file changed, 94 insertions(+), 131 deletions(-)
>>> I'm reviewing the device link patches now but since this is a demo of
>>> use of that I'll note the changes here are pretty large and it makes
>>> it terribly difficult for review. Is there any way this patch can be split
>>> up in to logical atomic pieces that only do one task upon change ?
>> I will try to split it a bit, but I cannot promise that much can be done
>> to improve readability for someone not very familiar with the driver
>> internals.
> I've heard this before, I don't buy it but lets see!

Somehow I managed to split this all-in-one patch into several smaller 
changes
in v5 and v6 was posted yesterday ago.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH v4 2/2] iommu/exynos: Add proper runtime pm support

[Rafael J. Wysocki]

On Wed, Nov 9, 2016 at 4:07 PM, Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
> Hi Luis,
>
>
> On 2016-11-08 23:14, Luis R. Rodriguez wrote:
>>
>> On Mon, Oct 10, 2016 at 03:32:06PM +0200, Marek Szyprowski wrote:
>>>
>>> Hi Luis
>>>
>>>
>>> On 2016-10-06 19:37, Luis R. Rodriguez wrote:
>>>>
>>>> On Thu, Sep 29, 2016 at 10:12:31AM +0200, Marek Szyprowski wrote:
>>>>>
>>>>> This patch uses recently introduced device links to track the runtime
>>>>> pm
>>>>> state of the master's device. This way each SYSMMU controller is
>>>>> runtime
>>>>> activated when its master's device is active
>>>>
>>>> instead of?
>>>
>>> instead of keeping SYSMMU controller runtime active all the time.
>>
>> I thought Rafael's work was for suspend/resume, not for runtime suspend.
>> Is it for both ?
>
>
> Yes, it solves both problems, although the suspend/resume was easy to
> workaround just by using LATE_SLEEP_OPS.

Right, but that's just in this particular case, because the dependency
chain is of length 2. :-)

If you had a longer chain, you might in theory use  the _noirq() stage
somehow, but that has limitations.

>> Because as far as I can tell this was painted to help
>> with suspend/resume ?

It helps with three things, (async) suspend/resume, runtime PM and
shutdown (that last part is the hardest to figure out).  The ordering
in which all of these are carried out is analogous and cannot be
determined correctly by the device registration ordering itself in
general (which has been a known fact for years, but some localized
workarounds were put in some places to work around that).

Moreover, even if the list ordering (of dpm_list, for instance) is
correct, it still doesn't guarantee the right ordering of actions that
are carried out asynchronously.  They are all started in the list
order, but they may be running in parallel with each other and
complete at different times.  For this reason, there needs to be a way
to ensure that, say, the suspend operations for consumer devices
complete before their suppliers will become unavailable and so on.

Both runtime PM and system suspend/resume have this problem.  It is
not present in the system shutdown case, but it still helps to get a
correct list ordering (ie. such that won't cause supplier devices to
be shut down before their consumers) in this case too.

Thanks,
Rafael

Re: [PATCH v4 2/2] iommu/exynos: Add proper runtime pm support

[Luis R. Rodriguez]

On Thu, Nov 10, 2016 at 01:36:29AM +0100, Rafael J. Wysocki wrote:
> On Wed, Nov 9, 2016 at 4:07 PM, Marek Szyprowski
> <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
> > Hi Luis,
> >
> >
> > On 2016-11-08 23:14, Luis R. Rodriguez wrote:
> >>
> >> On Mon, Oct 10, 2016 at 03:32:06PM +0200, Marek Szyprowski wrote:
> >>>
> >>> Hi Luis
> >>>
> >>>
> >>> On 2016-10-06 19:37, Luis R. Rodriguez wrote:
> >>>>
> >>>> On Thu, Sep 29, 2016 at 10:12:31AM +0200, Marek Szyprowski wrote:
> >>>>>
> >>>>> This patch uses recently introduced device links to track the runtime
> >>>>> pm
> >>>>> state of the master's device. This way each SYSMMU controller is
> >>>>> runtime
> >>>>> activated when its master's device is active
> >>>>
> >>>> instead of?
> >>>
> >>> instead of keeping SYSMMU controller runtime active all the time.
> >>
> >> I thought Rafael's work was for suspend/resume, not for runtime suspend.
> >> Is it for both ?
> >
> >
> > Yes, it solves both problems, although the suspend/resume was easy to
> > workaround just by using LATE_SLEEP_OPS.
> 
> Right, but that's just in this particular case, because the dependency
> chain is of length 2. :-)
> 
> If you had a longer chain, you might in theory use  the _noirq() stage
> somehow, but that has limitations.
> 
> >> Because as far as I can tell this was painted to help
> >> with suspend/resume ?
> 
> It helps with three things, (async) suspend/resume, runtime PM and
> shutdown (that last part is the hardest to figure out).  The ordering
> in which all of these are carried out is analogous and cannot be
> determined correctly by the device registration ordering itself in
> general (which has been a known fact for years, but some localized
> workarounds were put in some places to work around that).

Thanks for the clarification, this is due to the implicit sort you
had explained (and I provided notes for on ksummit-discuss) right?

Can you itemize a few of the workarounds that are used today?
As you clarify below, getting this order of device registration
correct does not necessarily guarantee devices will wait for their
provider to be ready.

> Moreover, even if the list ordering (of dpm_list, for instance) is
> correct, it still doesn't guarantee the right ordering of actions that
> are carried out asynchronously.  They are all started in the list
> order, but they may be running in parallel with each other and
> complete at different times.  For this reason, there needs to be a way
> to ensure that, say, the suspend operations for consumer devices
> complete before their suppliers will become unavailable and so on.

Thanks this helps as well!

> Both runtime PM and system suspend/resume have this problem.  It is
> not present in the system shutdown case, but it still helps to get a
> correct list ordering (ie. such that won't cause supplier devices to
> be shut down before their consumers) in this case too.

Is the fact that its not on shutdown just because we don't care about
being sloppy about shutdown ? Shouldn't some devices care about that
order ?

  Luis

Re: [PATCH v4 0/2] Exynos IOMMU: proper runtime PM support (use device dependencies)

[Tobias Jakobi]

Hello Marek,

I have applied the new version onto 4.8.0 but I'm seeing this Oops on
shutdown/reboot. However it only shows up with my non-debug config.


> [  897.046373] Internal error: Oops: 80000005 [#1] PREEMPT SMP ARM
> [  897.046652] Modules linked in: bridge stp llc bnep btrfs xor xor_neon zlib_inflate zlib_deflate raid6_pq s5p_mfc extcon_odroid_usbotg btusb btbcm btintel s5p_jpeg videobuf2_dma_contig v4l2_mem2mem videobuf2_memops videobuf2_v4l2 videobuf2_core
> [  897.068174] CPU: 2 PID: 9437 Comm: reboot Not tainted 4.8.0-vanilla+ #2
> [  897.074772] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
> [  897.080848] task: eea02100 task.stack: cb488000
> [  897.085363] PC is at 0xb886c672
> [  897.088489] LR is at device_shutdown+0x134/0x1c0
> [  897.093087] pc : [<b886c672>]    lr : [<c0405b14>]    psr: 600a0073
> [  897.093087] sp : cb489e28  ip : 00000000  fp : cb489e4c
> [  897.104547] r10: 00000000  r9 : ee0cfe44  r8 : c0a39020
> [  897.109752] r7 : c0a6d774  r6 : ee0cfe10  r5 : ee08b810  r4 : ee0cfe1c
> [  897.116262] r3 : b886c673  r2 : 00000000  r1 : 00000002  r0 : ee0cfe10
> [  897.122773] Flags: nZCv  IRQs on  FIQs off  Mode SVC_32  ISA Thumb  Segment none
> [  897.130151] Control: 10c5387d  Table: 6a24404a  DAC: 00000051
> [  897.135879] Process reboot (pid: 9437, stack limit = 0xcb488218)
> [  897.141868] Stack: (0xcb489e28 to 0xcb48a000)
> [  897.146209] 9e20:                   00000000 00000000 01234567 c0a09d4c 00000010 c0a09a88
> [  897.154371] 9e40: cb489e5c cb489e50 c013eb5c c04059ec cb489e74 cb489e60 c013ec38 c013eb2c
> [  897.162530] 9e60: 01234567 c0a02448 cb489fa4 cb489e78 c013ef08 c013ec30 eb87ca80 00c0d000
> [  897.170698] 9e80: cb489ea4 cb489e90 c01c9660 c01c9434 ef026000 00000000 cb489eb4 cb489ea8
> [  897.178855] 9ea0: c01c977c c01c961c cb489ec4 cb489eb8 c01c9798 c01c9760 cb489f3c cb489ec8
> [  897.187014] 9ec0: c01eeaa4 c01c978c eb87ca80 00000000 ec421210 00c0d000 ed8af3c0 eefd5140
> [  897.195173] 9ee0: ee84a140 eea11e40 cb489f14 cb489ef8 c022bc58 c016cfa0 ed8afa80 ee665c38
> [  897.203333] 9f00: 00000000 eea11e50 cb489f24 cb489f18 c022be1c c022bc20 cb489f5c cb489f28
> [  897.211492] 9f20: c020ce88 c022bdfc 00000020 00000000 cb489f54 eea024f8 c0a39f7c 00000000
> [  897.219651] 9f40: eea02100 c0107ee4 cb488000 00000000 cb489f6c cb489f60 c020cf80 c020cd5c
> [  897.227810] 9f60: cb489f8c cb489f70 c013aa40 c06dd2dc cb488010 c0107ee4 cb489fb0 00040800
> [  897.235969] 9f80: 00000001 be966ce0 00bec008 00000058 c0107ee4 cb488000 00000000 cb489fa8
> [  897.244129] 9fa0: c0107d20 c013ee04 00000001 be966ce0 fee1dead 28121969 01234567 00000010
> [  897.252287] 9fc0: 00000001 be966ce0 00bec008 00000058 00000000 000230e4 00000000 00000000
> [  897.260447] 9fe0: 00023030 be966cc4 00010e9c b6f3f290 200a0050 fee1dead 00000000 00000000
> [  897.268603] Backtrace: 
> [  897.271033] [<c04059e0>] (device_shutdown) from [<c013eb5c>] (kernel_restart_prepare+0x3c/0x40)
> [  897.279715]  r9:c0a09a88 r8:00000010 r7:c0a09d4c r6:01234567 r5:00000000 r4:00000000
> [  897.287439] [<c013eb20>] (kernel_restart_prepare) from [<c013ec38>] (kernel_restart+0x14/0x58)
> [  897.296036] [<c013ec24>] (kernel_restart) from [<c013ef08>] (SyS_reboot+0x110/0x1f8)
> [  897.303757]  r4:c0a02448 r3:01234567
> [  897.307314] [<c013edf8>] (SyS_reboot) from [<c0107d20>] (ret_fast_syscall+0x0/0x3c)
> [  897.314955]  r9:cb488000 r8:c0107ee4 r7:00000058 r6:00bec008 r5:be966ce0 r4:00000001
> [  897.322680] Code: bad PC value
> [  897.325940] ---[ end trace 861fd282a7bdc01e ]---


The corresponding config:
https://github.com/tobiasjakobi/odroid-environment/blob/master/sourcecode/system/vanilla-4.8.conf


With best wishes,
Tobias




Marek Szyprowski wrote:
> Hello,
> 
> This patch series finally implements proper runtime PM support in Exynos
> IOMMU driver. This has been achieved by using recently introduce device
> links, which lets SYSMMU controller's runtime PM to follow master's device
> runtime PM state (the device which actually performs DMA transaction).
> The main idea behind this solution is an observation that any DMA activity
> from master device can be done only when master device is active, thus when
> master device is suspended SYSMMU controller device can also be suspended.
> 
> This patchset solves the situation that power domains are always enabled,
> because all SYSMMU controllers (which belongs to those domains) are
> permanently active (because existing driver was simplified and kept
> SYSMMU device active all the time after initialization).
> 
> Patch requires fourth version of Rafeal's "Functional dependencies
> between devices" patchset, which is available here:
> https://www.mail-archive.com/linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg1241473.html
> 
> If one wants to test this patchset, I've provided a branch with all needed
> patches (some fixes for Exynos4 FIMC-IS driver are needed):
> https://git.linaro.org/people/marek.szyprowski/linux-srpol.git v4.8-iommu-pm-v4
> 
> Patches are based on vanilla v4.8-rc8 kernel with Rafael's device
> dependencies v4 patchset applied.
> 
> Best regards
> Marek Szyprowski
> Samsung R&D Institute Poland
> 
> 
> Changelog:
> v4:
> - rebased on top of v4 of device dependencies/links patchset, what resolved
>   system hang on reboot
> 
> v3: http://www.spinics.net/lists/linux-samsung-soc/msg55256.html
> - rebased on top of latest device dependencies/links patchset
> - added proper locking between runtime pm, iommu_attach/detach and sysmmu
>   enable/disable(added per iommu owner device's rpm lock)
> 
> v2: http://www.spinics.net/lists/arm-kernel/msg512082.html
> - replaced PM notifiers with generic device dependencies/links developped
>   by Rafael J. Wysocki
> 
> v1: http://www.spinics.net/lists/arm-kernel/msg509600.html
> - initial version
> 
> 
> Patch summary:
> 
> Marek Szyprowski (2):
>   iommu/exynos: Remove excessive, useless debug
>   iommu/exynos: Add proper runtime pm support
> 
>  drivers/iommu/exynos-iommu.c | 228 ++++++++++++++++++-------------------------
>  1 file changed, 94 insertions(+), 134 deletions(-)
> 

Re: [PATCH v4 0/2] Exynos IOMMU: proper runtime PM support (use device dependencies)

[Tobias Jakobi]

Hello again,


Tobias Jakobi wrote:
> Hello Marek,
> 
> I have applied the new version onto 4.8.0 but I'm seeing this Oops on
> shutdown/reboot. However it only shows up with my non-debug config.
sorry for the false alarm. That Oops on reboot was due to some other
patch I had applied.


With best wishes,
Tobias


>> [  897.046373] Internal error: Oops: 80000005 [#1] PREEMPT SMP ARM
>> [  897.046652] Modules linked in: bridge stp llc bnep btrfs xor xor_neon zlib_inflate zlib_deflate raid6_pq s5p_mfc extcon_odroid_usbotg btusb btbcm btintel s5p_jpeg videobuf2_dma_contig v4l2_mem2mem videobuf2_memops videobuf2_v4l2 videobuf2_core
>> [  897.068174] CPU: 2 PID: 9437 Comm: reboot Not tainted 4.8.0-vanilla+ #2
>> [  897.074772] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
>> [  897.080848] task: eea02100 task.stack: cb488000
>> [  897.085363] PC is at 0xb886c672
>> [  897.088489] LR is at device_shutdown+0x134/0x1c0
>> [  897.093087] pc : [<b886c672>]    lr : [<c0405b14>]    psr: 600a0073
>> [  897.093087] sp : cb489e28  ip : 00000000  fp : cb489e4c
>> [  897.104547] r10: 00000000  r9 : ee0cfe44  r8 : c0a39020
>> [  897.109752] r7 : c0a6d774  r6 : ee0cfe10  r5 : ee08b810  r4 : ee0cfe1c
>> [  897.116262] r3 : b886c673  r2 : 00000000  r1 : 00000002  r0 : ee0cfe10
>> [  897.122773] Flags: nZCv  IRQs on  FIQs off  Mode SVC_32  ISA Thumb  Segment none
>> [  897.130151] Control: 10c5387d  Table: 6a24404a  DAC: 00000051
>> [  897.135879] Process reboot (pid: 9437, stack limit = 0xcb488218)
>> [  897.141868] Stack: (0xcb489e28 to 0xcb48a000)
>> [  897.146209] 9e20:                   00000000 00000000 01234567 c0a09d4c 00000010 c0a09a88
>> [  897.154371] 9e40: cb489e5c cb489e50 c013eb5c c04059ec cb489e74 cb489e60 c013ec38 c013eb2c
>> [  897.162530] 9e60: 01234567 c0a02448 cb489fa4 cb489e78 c013ef08 c013ec30 eb87ca80 00c0d000
>> [  897.170698] 9e80: cb489ea4 cb489e90 c01c9660 c01c9434 ef026000 00000000 cb489eb4 cb489ea8
>> [  897.178855] 9ea0: c01c977c c01c961c cb489ec4 cb489eb8 c01c9798 c01c9760 cb489f3c cb489ec8
>> [  897.187014] 9ec0: c01eeaa4 c01c978c eb87ca80 00000000 ec421210 00c0d000 ed8af3c0 eefd5140
>> [  897.195173] 9ee0: ee84a140 eea11e40 cb489f14 cb489ef8 c022bc58 c016cfa0 ed8afa80 ee665c38
>> [  897.203333] 9f00: 00000000 eea11e50 cb489f24 cb489f18 c022be1c c022bc20 cb489f5c cb489f28
>> [  897.211492] 9f20: c020ce88 c022bdfc 00000020 00000000 cb489f54 eea024f8 c0a39f7c 00000000
>> [  897.219651] 9f40: eea02100 c0107ee4 cb488000 00000000 cb489f6c cb489f60 c020cf80 c020cd5c
>> [  897.227810] 9f60: cb489f8c cb489f70 c013aa40 c06dd2dc cb488010 c0107ee4 cb489fb0 00040800
>> [  897.235969] 9f80: 00000001 be966ce0 00bec008 00000058 c0107ee4 cb488000 00000000 cb489fa8
>> [  897.244129] 9fa0: c0107d20 c013ee04 00000001 be966ce0 fee1dead 28121969 01234567 00000010
>> [  897.252287] 9fc0: 00000001 be966ce0 00bec008 00000058 00000000 000230e4 00000000 00000000
>> [  897.260447] 9fe0: 00023030 be966cc4 00010e9c b6f3f290 200a0050 fee1dead 00000000 00000000
>> [  897.268603] Backtrace: 
>> [  897.271033] [<c04059e0>] (device_shutdown) from [<c013eb5c>] (kernel_restart_prepare+0x3c/0x40)
>> [  897.279715]  r9:c0a09a88 r8:00000010 r7:c0a09d4c r6:01234567 r5:00000000 r4:00000000
>> [  897.287439] [<c013eb20>] (kernel_restart_prepare) from [<c013ec38>] (kernel_restart+0x14/0x58)
>> [  897.296036] [<c013ec24>] (kernel_restart) from [<c013ef08>] (SyS_reboot+0x110/0x1f8)
>> [  897.303757]  r4:c0a02448 r3:01234567
>> [  897.307314] [<c013edf8>] (SyS_reboot) from [<c0107d20>] (ret_fast_syscall+0x0/0x3c)
>> [  897.314955]  r9:cb488000 r8:c0107ee4 r7:00000058 r6:00bec008 r5:be966ce0 r4:00000001
>> [  897.322680] Code: bad PC value
>> [  897.325940] ---[ end trace 861fd282a7bdc01e ]---
> 
> 
> The corresponding config:
> https://github.com/tobiasjakobi/odroid-environment/blob/master/sourcecode/system/vanilla-4.8.conf
> 
> 
> With best wishes,
> Tobias
> 
> 
> 
> 
> Marek Szyprowski wrote:
>> Hello,
>>
>> This patch series finally implements proper runtime PM support in Exynos
>> IOMMU driver. This has been achieved by using recently introduce device
>> links, which lets SYSMMU controller's runtime PM to follow master's device
>> runtime PM state (the device which actually performs DMA transaction).
>> The main idea behind this solution is an observation that any DMA activity
>> from master device can be done only when master device is active, thus when
>> master device is suspended SYSMMU controller device can also be suspended.
>>
>> This patchset solves the situation that power domains are always enabled,
>> because all SYSMMU controllers (which belongs to those domains) are
>> permanently active (because existing driver was simplified and kept
>> SYSMMU device active all the time after initialization).
>>
>> Patch requires fourth version of Rafeal's "Functional dependencies
>> between devices" patchset, which is available here:
>> https://www.mail-archive.com/linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg1241473.html
>>
>> If one wants to test this patchset, I've provided a branch with all needed
>> patches (some fixes for Exynos4 FIMC-IS driver are needed):
>> https://git.linaro.org/people/marek.szyprowski/linux-srpol.git v4.8-iommu-pm-v4
>>
>> Patches are based on vanilla v4.8-rc8 kernel with Rafael's device
>> dependencies v4 patchset applied.
>>
>> Best regards
>> Marek Szyprowski
>> Samsung R&D Institute Poland
>>
>>
>> Changelog:
>> v4:
>> - rebased on top of v4 of device dependencies/links patchset, what resolved
>>   system hang on reboot
>>
>> v3: http://www.spinics.net/lists/linux-samsung-soc/msg55256.html
>> - rebased on top of latest device dependencies/links patchset
>> - added proper locking between runtime pm, iommu_attach/detach and sysmmu
>>   enable/disable(added per iommu owner device's rpm lock)
>>
>> v2: http://www.spinics.net/lists/arm-kernel/msg512082.html
>> - replaced PM notifiers with generic device dependencies/links developped
>>   by Rafael J. Wysocki
>>
>> v1: http://www.spinics.net/lists/arm-kernel/msg509600.html
>> - initial version
>>
>>
>> Patch summary:
>>
>> Marek Szyprowski (2):
>>   iommu/exynos: Remove excessive, useless debug
>>   iommu/exynos: Add proper runtime pm support
>>
>>  drivers/iommu/exynos-iommu.c | 228 ++++++++++++++++++-------------------------
>>  1 file changed, 94 insertions(+), 134 deletions(-)
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>