Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1493193 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v4 0/2] Exynos IOMMU: proper runtime PM support (use device  dependencies) Marek Szyprowski <m.szyprowski@samsung.com> - 2016-09-29 10:20 +0200
    [PATCH v4 2/2] iommu/exynos: Add proper runtime pm support Marek Szyprowski <m.szyprowski@samsung.com> - 2016-09-29 10:20 +0200
      Re: [PATCH v4 2/2] iommu/exynos: Add proper runtime pm support "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-10-06 19:40 +0200
        Re: [PATCH v4 2/2] iommu/exynos: Add proper runtime pm support Marek Szyprowski <m.szyprowski@samsung.com> - 2016-10-10 15:40 +0200
    [PATCH v4 1/2] iommu/exynos: Remove excessive, useless debug Marek Szyprowski <m.szyprowski@samsung.com> - 2016-09-29 10:20 +0200
    Re: [PATCH v4 0/2] Exynos IOMMU: proper runtime PM support (use  device dependencies) Tobias Jakobi <tjakobi@math.uni-bielefeld.de> - 2016-10-06 00:50 +0200
      Re: [PATCH v4 0/2] Exynos IOMMU: proper runtime PM support (use  device dependencies) Tobias Jakobi <tjakobi@math.uni-bielefeld.de> - 2016-10-06 18:30 +0200

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

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@vger.kernel.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

[toc] | [next] | [standalone]

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

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@samsung.com>
---
 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

[toc] | [prev] | [next] | [standalone]

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

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@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 ?

  Luis

[toc] | [prev] | [next] | [standalone]

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

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@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.

>
>> 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@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.

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

[toc] | [prev] | [next] | [standalone]

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

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@samsung.com>
---
 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

[toc] | [prev] | [next] | [standalone]

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

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@vger.kernel.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(-)
> 

[toc] | [prev] | [next] | [standalone]

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

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@vger.kernel.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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web