linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/6] fix bugs; enable iommu for ARM64
@ 2016-06-15 12:04 Shunqian Zheng
  2016-06-15 12:04 ` [PATCH v3 1/6] iommu/rockchip: fix devm_{request,free}_irq parameter Shunqian Zheng
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Shunqian Zheng @ 2016-06-15 12:04 UTC (permalink / raw)
  To: joro, heiko, robh+dt, mark.rutland, linux, mark.yao, airlied, tfiga, xxm
  Cc: linux-arm-kernel, iommu, devicetree, dri-devel, linux-kernel,
	linux-rockchip, Shunqian Zheng

This series patches mainly for ARM64 supporting.
To do this, it first add virtual iommu slave device which DRM can attach to,
convert DRM driver to use common iommu API instead of the ARM32
functions, and then use DMA API in iommu driver to map, to flush cache.

Mainly changes of V3:
    - Instead of registering virtual iommu in DTS, V3
      creates a iommu when attaching.
    - Fix some bugs according to Tomasz's comments,
      most of them are offline, locally.

Shunqian Zheng (3):
  iommu/rockchip: support virtual iommu slave device
  drm: rockchip: use common iommu api to attach iommu
  iommu/rockchip: use DMA API to map, to flush cache

Simon Xue (3):
  iommu/rockchip: fix devm_{request,free}_irq parameter
  iommu/rockchip: add map_sg callback for rk_iommu_ops
  iommu/rockchip: enable rockchip iommu on ARM64 platform

 drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 112 ++++++++-----
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   1 +
 drivers/iommu/Kconfig                       |   2 +-
 drivers/iommu/rockchip-iommu.c              | 251 ++++++++++++++++++++++------
 4 files changed, 273 insertions(+), 93 deletions(-)

-- 
1.9.1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 1/6] iommu/rockchip: fix devm_{request,free}_irq parameter
  2016-06-15 12:04 [PATCH v3 0/6] fix bugs; enable iommu for ARM64 Shunqian Zheng
@ 2016-06-15 12:04 ` Shunqian Zheng
  2016-06-15 12:04 ` [PATCH v3 2/6] iommu/rockchip: add map_sg callback for rk_iommu_ops Shunqian Zheng
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Shunqian Zheng @ 2016-06-15 12:04 UTC (permalink / raw)
  To: joro, heiko, robh+dt, mark.rutland, linux, mark.yao, airlied, tfiga, xxm
  Cc: linux-arm-kernel, iommu, devicetree, dri-devel, linux-kernel,
	linux-rockchip, Shunqian Zheng

From: Simon Xue <xxm@rock-chips.com>

Even though the iommu shares irq with its master, using the *dev of iommu
instead of master's *dev for devm_{request,free}_irq makes things clear.

Signed-off-by: Simon Xue <xxm@rock-chips.com>
Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com>
Reviewed-by: Tomasz Figa <tfiga@chromium.org>
---
 drivers/iommu/rockchip-iommu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index c7d6156..ec0ce62 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -807,7 +807,7 @@ static int rk_iommu_attach_device(struct iommu_domain *domain,
 
 	iommu->domain = domain;
 
-	ret = devm_request_irq(dev, iommu->irq, rk_iommu_irq,
+	ret = devm_request_irq(iommu->dev, iommu->irq, rk_iommu_irq,
 			       IRQF_SHARED, dev_name(dev), iommu);
 	if (ret)
 		return ret;
@@ -860,7 +860,7 @@ static void rk_iommu_detach_device(struct iommu_domain *domain,
 	}
 	rk_iommu_disable_stall(iommu);
 
-	devm_free_irq(dev, iommu->irq, iommu);
+	devm_free_irq(iommu->dev, iommu->irq, iommu);
 
 	iommu->domain = NULL;
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 2/6] iommu/rockchip: add map_sg callback for rk_iommu_ops
  2016-06-15 12:04 [PATCH v3 0/6] fix bugs; enable iommu for ARM64 Shunqian Zheng
  2016-06-15 12:04 ` [PATCH v3 1/6] iommu/rockchip: fix devm_{request,free}_irq parameter Shunqian Zheng
@ 2016-06-15 12:04 ` Shunqian Zheng
  2016-06-15 12:04 ` [PATCH v3 3/6] iommu/rockchip: support virtual iommu slave device Shunqian Zheng
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Shunqian Zheng @ 2016-06-15 12:04 UTC (permalink / raw)
  To: joro, heiko, robh+dt, mark.rutland, linux, mark.yao, airlied, tfiga, xxm
  Cc: linux-arm-kernel, iommu, devicetree, dri-devel, linux-kernel,
	linux-rockchip

From: Simon Xue <xxm@rock-chips.com>

The iommu_dma_alloc() in iommu/dma-iommu.c calls iommu_map_sg()
that requires the callback iommu_ops .map_sg(). Adding the
default_iommu_map_sg() to rockchip iommu accordingly.

Signed-off-by: Simon Xue <xxm@rock-chips.com>
Signed-off-by: Shunqian Zheng <xxm@rock-chips.com>
Reviewed-by: Tomasz Figa <tfiga@chromium.org>
---
 drivers/iommu/rockchip-iommu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index ec0ce62..3c16ec3 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -1022,6 +1022,7 @@ static const struct iommu_ops rk_iommu_ops = {
 	.detach_dev = rk_iommu_detach_device,
 	.map = rk_iommu_map,
 	.unmap = rk_iommu_unmap,
+	.map_sg = default_iommu_map_sg,
 	.add_device = rk_iommu_add_device,
 	.remove_device = rk_iommu_remove_device,
 	.iova_to_phys = rk_iommu_iova_to_phys,
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 3/6] iommu/rockchip: support virtual iommu slave device
  2016-06-15 12:04 [PATCH v3 0/6] fix bugs; enable iommu for ARM64 Shunqian Zheng
  2016-06-15 12:04 ` [PATCH v3 1/6] iommu/rockchip: fix devm_{request,free}_irq parameter Shunqian Zheng
  2016-06-15 12:04 ` [PATCH v3 2/6] iommu/rockchip: add map_sg callback for rk_iommu_ops Shunqian Zheng
@ 2016-06-15 12:04 ` Shunqian Zheng
  2016-06-15 14:21   ` Tomasz Figa
  2016-06-15 12:04 ` [PATCH v3 4/6] drm: rockchip: use common iommu api to attach iommu Shunqian Zheng
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 10+ messages in thread
From: Shunqian Zheng @ 2016-06-15 12:04 UTC (permalink / raw)
  To: joro, heiko, robh+dt, mark.rutland, linux, mark.yao, airlied, tfiga, xxm
  Cc: linux-arm-kernel, iommu, devicetree, dri-devel, linux-kernel,
	linux-rockchip, Shunqian Zheng

An virtual master device like DRM need to attach to iommu
domain to share the mapping with VOPs(the one with actual
iommu slaves).
DRM attaches to iommu and allocates buffers before VOPs enabled,
which means there may have not real iommu devices can be used
to do dma mapping.

This patch creates a iommu when virtual master(group is NULL)
attaching, so it can use this iommu even the real iommus disabled.

Changes of V3:
- Instead of registering virtual iommu in DTS, this patch
  creates a iommu when attaching.

Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com>
Suggested-by: Tomasz Figa <tfiga@chromium.org>
---
 drivers/iommu/rockchip-iommu.c | 133 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 122 insertions(+), 11 deletions(-)

diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index 3c16ec3..82ecc99 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -9,6 +9,7 @@
 #include <linux/compiler.h>
 #include <linux/delay.h>
 #include <linux/device.h>
+#include <linux/dma-iommu.h>
 #include <linux/errno.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
@@ -93,6 +94,14 @@ struct rk_iommu {
 	struct iommu_domain *domain; /* domain to which iommu is attached */
 };
 
+/* A virtual iommu registered without resource or
+ * interrupts when DRM attaching.
+ */
+static inline bool rk_iommu_is_virtual(struct rk_iommu *iommu)
+{
+	return iommu->num_mmu == 0;
+}
+
 static inline void rk_table_flush(u32 *va, unsigned int count)
 {
 	phys_addr_t pa_start = virt_to_phys(va);
@@ -780,6 +789,85 @@ static struct rk_iommu *rk_iommu_from_dev(struct device *dev)
 	return rk_iommu;
 }
 
+static struct rk_iommu *rk_iommu_add_virtual_iommu(struct device *dev,
+					struct rk_iommu_domain *rk_domain)
+{
+	struct rk_iommu *iommu;
+	struct iommu_group *group;
+	struct platform_device *pdev;
+	struct device *iommu_dev;
+	int ret;
+
+	pdev = platform_device_register_simple("rk_iommu", PLATFORM_DEVID_AUTO,
+					       NULL, 0);
+	if (IS_ERR(pdev)) {
+		dev_err(dev, "Failed to register simple rk_iommu device\n");
+		return NULL;
+	}
+
+	iommu_dev = &pdev->dev;
+	iommu_dev->dma_parms = devm_kzalloc(iommu_dev,
+				sizeof(*iommu_dev->dma_parms), GFP_KERNEL);
+	if (!iommu_dev->dma_parms)
+		goto err_unreg_pdev;
+
+	/* Set dma_ops for virtual iommu, otherwise it would be dummy_dma_ops */
+	arch_setup_dma_ops(iommu_dev, 0, DMA_BIT_MASK(32), NULL, false);
+
+	dma_set_max_seg_size(iommu_dev, DMA_BIT_MASK(32));
+	dma_coerce_mask_and_coherent(iommu_dev, DMA_BIT_MASK(32));
+
+	iommu = devm_kzalloc(iommu_dev, sizeof(*iommu), GFP_KERNEL);
+	if (!iommu)
+		goto err_unreg_pdev;
+
+	iommu->dev = iommu_dev;
+	iommu->num_mmu = 0;
+	platform_set_drvdata(pdev, iommu);
+
+	group = iommu_group_alloc();
+	if (IS_ERR(group)) {
+		dev_err(iommu_dev, "Failed to allocate IOMMU group\n");
+		goto err_unreg_pdev;
+	}
+
+	ret = iommu_group_add_device(group, dev);
+	if (ret) {
+		dev_err(iommu_dev, "Failed to add device to group\n");
+		goto err_put_group;
+	}
+
+	iommu_group_set_iommudata(group, iommu_dev, NULL);
+
+	ret = iommu_attach_group(&rk_domain->domain, group);
+	if (ret) {
+		dev_err(iommu_dev, "Failed to attach group\n");
+		goto err_remove_device;
+	}
+
+	iommu_group_put(group);
+
+	return iommu;
+
+err_remove_device:
+	iommu_group_remove_device(dev);
+err_put_group:
+	iommu_group_put(group);
+err_unreg_pdev:
+	platform_device_unregister(pdev);
+
+	return NULL;
+}
+
+static void rk_iommu_remove_virtual_iommu(struct device *dev)
+{
+	struct rk_iommu *iommu = rk_iommu_from_dev(dev);
+	struct platform_device *pdev = to_platform_device(iommu->dev);
+
+	iommu_group_remove_device(dev);
+	platform_device_unregister(pdev);
+}
+
 static int rk_iommu_attach_device(struct iommu_domain *domain,
 				  struct device *dev)
 {
@@ -789,13 +877,20 @@ static int rk_iommu_attach_device(struct iommu_domain *domain,
 	int ret, i;
 	phys_addr_t dte_addr;
 
-	/*
-	 * Allow 'virtual devices' (e.g., drm) to attach to domain.
-	 * Such a device does not belong to an iommu group.
-	 */
 	iommu = rk_iommu_from_dev(dev);
-	if (!iommu)
+
+	/* Register iommu for virtual master(like DRM) */
+	if (!iommu) {
+		iommu = rk_iommu_add_virtual_iommu(dev, rk_domain);
+		if (!iommu)
+			return -ENOMEM;
+	}
+
+	iommu->domain = domain;
+	if (rk_iommu_is_virtual(iommu)) {
+		dev_dbg(dev, "Attach virtual device to iommu domain\n");
 		return 0;
+	}
 
 	ret = rk_iommu_enable_stall(iommu);
 	if (ret)
@@ -805,7 +900,6 @@ static int rk_iommu_attach_device(struct iommu_domain *domain,
 	if (ret)
 		return ret;
 
-	iommu->domain = domain;
 
 	ret = devm_request_irq(iommu->dev, iommu->irq, rk_iommu_irq,
 			       IRQF_SHARED, dev_name(dev), iommu);
@@ -842,10 +936,14 @@ static void rk_iommu_detach_device(struct iommu_domain *domain,
 	unsigned long flags;
 	int i;
 
-	/* Allow 'virtual devices' (eg drm) to detach from domain */
 	iommu = rk_iommu_from_dev(dev);
-	if (!iommu)
+
+	if (rk_iommu_is_virtual(iommu)) {
+		rk_iommu_remove_virtual_iommu(dev);
+		iommu->domain = NULL;
+		dev_dbg(dev, "Master with virtual iommu detached\n");
 		return;
+	}
 
 	spin_lock_irqsave(&rk_domain->iommus_lock, flags);
 	list_del_init(&iommu->node);
@@ -861,7 +959,6 @@ static void rk_iommu_detach_device(struct iommu_domain *domain,
 	rk_iommu_disable_stall(iommu);
 
 	devm_free_irq(iommu->dev, iommu->irq, iommu);
-
 	iommu->domain = NULL;
 
 	dev_dbg(dev, "Detached from iommu domain\n");
@@ -878,6 +975,9 @@ static struct iommu_domain *rk_iommu_domain_alloc(unsigned type)
 	if (!rk_domain)
 		return NULL;
 
+	if (iommu_get_dma_cookie(&rk_domain->domain))
+		goto err_dt;
+
 	/*
 	 * rk32xx iommus use a 2 level pagetable.
 	 * Each level1 (dt) and level2 (pt) table has 1024 4-byte entries.
@@ -917,6 +1017,9 @@ static void rk_iommu_domain_free(struct iommu_domain *domain)
 	}
 
 	free_page((unsigned long)rk_domain->dt);
+
+	iommu_put_dma_cookie(&rk_domain->domain);
+
 	kfree(rk_domain);
 }
 
@@ -1034,8 +1137,15 @@ static int rk_iommu_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct rk_iommu *iommu;
 	struct resource *res;
+	int num_res = pdev->num_resources;
 	int i;
 
+	/* This is iommu register in rk_iommu_add_virtual_iommu()
+	 * when virtual device attaching.
+	 */
+	if (num_res == 0)
+		return 0;
+
 	iommu = devm_kzalloc(dev, sizeof(*iommu), GFP_KERNEL);
 	if (!iommu)
 		return -ENOMEM;
@@ -1043,12 +1153,13 @@ static int rk_iommu_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, iommu);
 	iommu->dev = dev;
 	iommu->num_mmu = 0;
-	iommu->bases = devm_kzalloc(dev, sizeof(*iommu->bases) * iommu->num_mmu,
+
+	iommu->bases = devm_kzalloc(dev, sizeof(*iommu->bases) * num_res,
 				    GFP_KERNEL);
 	if (!iommu->bases)
 		return -ENOMEM;
 
-	for (i = 0; i < pdev->num_resources; i++) {
+	for (i = 0; i < num_res; i++) {
 		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
 		if (!res)
 			continue;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 4/6] drm: rockchip: use common iommu api to attach iommu
  2016-06-15 12:04 [PATCH v3 0/6] fix bugs; enable iommu for ARM64 Shunqian Zheng
                   ` (2 preceding siblings ...)
  2016-06-15 12:04 ` [PATCH v3 3/6] iommu/rockchip: support virtual iommu slave device Shunqian Zheng
@ 2016-06-15 12:04 ` Shunqian Zheng
  2016-06-15 14:24   ` Tomasz Figa
  2016-06-15 12:04 ` [PATCH v3 5/6] iommu/rockchip: use DMA API to map, to flush cache Shunqian Zheng
  2016-06-15 12:04 ` [PATCH v3 6/6] iommu/rockchip: enable rockchip iommu on ARM64 platform Shunqian Zheng
  5 siblings, 1 reply; 10+ messages in thread
From: Shunqian Zheng @ 2016-06-15 12:04 UTC (permalink / raw)
  To: joro, heiko, robh+dt, mark.rutland, linux, mark.yao, airlied, tfiga, xxm
  Cc: linux-arm-kernel, iommu, devicetree, dri-devel, linux-kernel,
	linux-rockchip, Shunqian Zheng

Rockchip DRM used the arm special API, arm_iommu_*(), to attach
iommu for ARM32 SoCs. This patch convert to common iommu API
so it would support ARM64 like RK3399.

The general idea is domain_alloc(), attach_device() and
arch_setup_dma_ops() to set dma_ops manually for DRM at the last.

Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 112 +++++++++++++++++-----------
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   1 +
 2 files changed, 71 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index f5a68fc..b52c38d 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -14,16 +14,16 @@
  * GNU General Public License for more details.
  */
 
-#include <asm/dma-iommu.h>
-
 #include <drm/drmP.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_fb_helper.h>
+#include <linux/dma-iommu.h>
 #include <linux/dma-mapping.h>
 #include <linux/pm_runtime.h>
 #include <linux/module.h>
 #include <linux/of_graph.h>
 #include <linux/component.h>
+#include <linux/iommu.h>
 
 #include "rockchip_drm_drv.h"
 #include "rockchip_drm_fb.h"
@@ -46,7 +46,8 @@ static bool is_support_iommu = true;
 int rockchip_drm_dma_attach_device(struct drm_device *drm_dev,
 				   struct device *dev)
 {
-	struct dma_iommu_mapping *mapping = drm_dev->dev->archdata.mapping;
+	struct rockchip_drm_private *private = drm_dev->dev_private;
+	struct iommu_domain *domain = private->domain;
 	int ret;
 
 	if (!is_support_iommu)
@@ -58,16 +59,25 @@ int rockchip_drm_dma_attach_device(struct drm_device *drm_dev,
 
 	dma_set_max_seg_size(dev, DMA_BIT_MASK(32));
 
-	return arm_iommu_attach_device(dev, mapping);
+	ret = iommu_attach_device(domain, dev);
+	if (ret) {
+		dev_err(dev, "Failed to attach iommu device\n");
+		return ret;
+	}
+
+	/* TODO(djkurtz): fetch the mapping start/size from somewhere */
+	arch_setup_dma_ops(dev, 0x00000000, SZ_2G, dev->bus->iommu_ops, false);
+	return 0;
 }
 
 void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
 				    struct device *dev)
 {
-	if (!is_support_iommu)
-		return;
+	struct rockchip_drm_private *private = drm_dev->dev_private;
+	struct iommu_domain *domain = private->domain;
 
-	arm_iommu_detach_device(dev);
+	if (is_support_iommu)
+		iommu_detach_device(domain, dev);
 }
 
 int rockchip_register_crtc_funcs(struct drm_crtc *crtc,
@@ -132,10 +142,52 @@ static void rockchip_drm_crtc_disable_vblank(struct drm_device *dev,
 		priv->crtc_funcs[pipe]->disable_vblank(crtc);
 }
 
+static int rockchip_drm_init_iommu(struct drm_device *drm_dev)
+{
+	struct rockchip_drm_private *private = drm_dev->dev_private;
+	struct device *dev = drm_dev->dev;
+	int ret;
+
+	dev->dma_parms = devm_kzalloc(dev, sizeof(*dev->dma_parms),
+				      GFP_KERNEL);
+	if (!dev->dma_parms)
+		return -ENOMEM;
+
+	private->domain = iommu_domain_alloc(&platform_bus_type);
+	if (!private->domain)
+		return -ENOMEM;
+
+	/* TODO(djkurtz): fetch the mapping start/size from somewhere */
+	ret = iommu_dma_init_domain(private->domain, 0x00000000, SZ_2G);
+	if (ret) {
+		dev_err(dev, "Failed to init domain\n");
+		goto err_free_domain;
+	}
+
+	ret = rockchip_drm_dma_attach_device(drm_dev, dev);
+	if (ret) {
+		dev_err(dev, "Failed to attach device\n");
+		goto err_free_domain;
+	}
+
+	return 0;
+
+err_free_domain:
+	iommu_domain_free(private->domain);
+
+	return ret;
+}
+
+static void rockchip_iommu_cleanup(struct drm_device *drm_dev)
+{
+	struct rockchip_drm_private *private = drm_dev->dev_private;
+
+	iommu_domain_free(private->domain);
+}
+
 static int rockchip_drm_load(struct drm_device *drm_dev, unsigned long flags)
 {
 	struct rockchip_drm_private *private;
-	struct dma_iommu_mapping *mapping = NULL;
 	struct device *dev = drm_dev->dev;
 	struct drm_connector *connector;
 	int ret;
@@ -153,38 +205,18 @@ static int rockchip_drm_load(struct drm_device *drm_dev, unsigned long flags)
 
 	rockchip_drm_mode_config_init(drm_dev);
 
-	dev->dma_parms = devm_kzalloc(dev, sizeof(*dev->dma_parms),
-				      GFP_KERNEL);
-	if (!dev->dma_parms) {
-		ret = -ENOMEM;
-		goto err_config_cleanup;
-	}
-
 	if (is_support_iommu) {
-		/* TODO(djkurtz): fetch the mapping start/size from somewhere */
-		mapping = arm_iommu_create_mapping(&platform_bus_type,
-						   0x00000000,
-						   SZ_2G);
-		if (IS_ERR(mapping)) {
-			ret = PTR_ERR(mapping);
-			goto err_config_cleanup;
-		}
-
-		ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
-		if (ret)
-			goto err_release_mapping;
-
-		dma_set_max_seg_size(dev, DMA_BIT_MASK(32));
-
-		ret = arm_iommu_attach_device(dev, mapping);
+		ret = rockchip_drm_init_iommu(drm_dev);
 		if (ret)
-			goto err_release_mapping;
+			goto err_config_cleanup;
 	}
 
 	/* Try to bind all sub drivers. */
 	ret = component_bind_all(dev, drm_dev);
-	if (ret)
-		goto err_detach_device;
+	if (ret) {
+		dev_err(dev, "Failed to bind components\n");
+		goto err_iommu_cleanup;
+	}
 
 	/*
 	 * All components are now added, we can publish the connector sysfs
@@ -222,21 +254,17 @@ static int rockchip_drm_load(struct drm_device *drm_dev, unsigned long flags)
 	if (ret)
 		goto err_vblank_cleanup;
 
-	if (is_support_iommu)
-		arm_iommu_release_mapping(mapping);
 	return 0;
+
 err_vblank_cleanup:
 	drm_vblank_cleanup(drm_dev);
 err_kms_helper_poll_fini:
 	drm_kms_helper_poll_fini(drm_dev);
 err_unbind:
 	component_unbind_all(dev, drm_dev);
-err_detach_device:
-	if (is_support_iommu)
-		arm_iommu_detach_device(dev);
-err_release_mapping:
+err_iommu_cleanup:
 	if (is_support_iommu)
-		arm_iommu_release_mapping(mapping);
+		rockchip_iommu_cleanup(drm_dev);
 err_config_cleanup:
 	drm_mode_config_cleanup(drm_dev);
 	drm_dev->dev_private = NULL;
@@ -252,7 +280,7 @@ static int rockchip_drm_unload(struct drm_device *drm_dev)
 	drm_kms_helper_poll_fini(drm_dev);
 	component_unbind_all(dev, drm_dev);
 	if (is_support_iommu)
-		arm_iommu_detach_device(dev);
+		rockchip_iommu_cleanup(drm_dev);
 	drm_mode_config_cleanup(drm_dev);
 	drm_dev->dev_private = NULL;
 
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
index 56f43a3..1e2a666 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
@@ -70,6 +70,7 @@ struct rockchip_drm_private {
 	const struct rockchip_crtc_funcs *crtc_funcs[ROCKCHIP_MAX_CRTC];
 
 	struct rockchip_atomic_commit commit;
+	struct iommu_domain *domain;
 };
 
 void rockchip_drm_atomic_work(struct work_struct *work);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 5/6] iommu/rockchip: use DMA API to map, to flush cache
  2016-06-15 12:04 [PATCH v3 0/6] fix bugs; enable iommu for ARM64 Shunqian Zheng
                   ` (3 preceding siblings ...)
  2016-06-15 12:04 ` [PATCH v3 4/6] drm: rockchip: use common iommu api to attach iommu Shunqian Zheng
@ 2016-06-15 12:04 ` Shunqian Zheng
  2016-06-15 14:31   ` Tomasz Figa
  2016-06-15 12:04 ` [PATCH v3 6/6] iommu/rockchip: enable rockchip iommu on ARM64 platform Shunqian Zheng
  5 siblings, 1 reply; 10+ messages in thread
From: Shunqian Zheng @ 2016-06-15 12:04 UTC (permalink / raw)
  To: joro, heiko, robh+dt, mark.rutland, linux, mark.yao, airlied, tfiga, xxm
  Cc: linux-arm-kernel, iommu, devicetree, dri-devel, linux-kernel,
	linux-rockchip, Shunqian Zheng

Use DMA API instead of architecture internal functions like
__cpuc_flush_dcache_area() etc.

To support the virtual device like DRM the virtual slave iommu
added in the previous patch, attaching to which the DRM can use
it own domain->dev for dma_map_*(), dma_sync_*() even VOP is disabled.

With this patch, this driver is available for ARM64 like RK3399.

Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com>
---
 drivers/iommu/rockchip-iommu.c | 113 +++++++++++++++++++++++++++--------------
 1 file changed, 76 insertions(+), 37 deletions(-)

diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index 82ecc99..b60b29e 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -4,8 +4,6 @@
  * published by the Free Software Foundation.
  */
 
-#include <asm/cacheflush.h>
-#include <asm/pgtable.h>
 #include <linux/compiler.h>
 #include <linux/delay.h>
 #include <linux/device.h>
@@ -78,7 +76,9 @@
 
 struct rk_iommu_domain {
 	struct list_head iommus;
+	struct device *dev;
 	u32 *dt; /* page directory table */
+	dma_addr_t dt_dma;
 	spinlock_t iommus_lock; /* lock for iommus list */
 	spinlock_t dt_lock; /* lock for modifying page directory table */
 
@@ -102,14 +102,12 @@ static inline bool rk_iommu_is_virtual(struct rk_iommu *iommu)
 	return iommu->num_mmu == 0;
 }
 
-static inline void rk_table_flush(u32 *va, unsigned int count)
+static inline void rk_table_flush(struct device *dev, dma_addr_t dma,
+				  unsigned int count)
 {
-	phys_addr_t pa_start = virt_to_phys(va);
-	phys_addr_t pa_end = virt_to_phys(va + count);
-	size_t size = pa_end - pa_start;
+	size_t size = count * 4; /* count of entry, 4 bytes per entry */
 
-	__cpuc_flush_dcache_area(va, size);
-	outer_flush_range(pa_start, pa_end);
+	dma_sync_single_for_device(dev, dma, size, DMA_TO_DEVICE);
 }
 
 static struct rk_iommu_domain *to_rk_domain(struct iommu_domain *dom)
@@ -192,10 +190,9 @@ static inline bool rk_dte_is_pt_valid(u32 dte)
 	return dte & RK_DTE_PT_VALID;
 }
 
-static u32 rk_mk_dte(u32 *pt)
+static inline u32 rk_mk_dte(dma_addr_t pt_dma)
 {
-	phys_addr_t pt_phys = virt_to_phys(pt);
-	return (pt_phys & RK_DTE_PT_ADDRESS_MASK) | RK_DTE_PT_VALID;
+	return (pt_dma & RK_DTE_PT_ADDRESS_MASK) | RK_DTE_PT_VALID;
 }
 
 /*
@@ -613,12 +610,14 @@ static u32 *rk_dte_get_page_table(struct rk_iommu_domain *rk_domain,
 				  dma_addr_t iova)
 {
 	u32 *page_table, *dte_addr;
-	u32 dte;
+	u32 dte_index, dte;
 	phys_addr_t pt_phys;
+	dma_addr_t pt_dma;
 
 	assert_spin_locked(&rk_domain->dt_lock);
 
-	dte_addr = &rk_domain->dt[rk_iova_dte_index(iova)];
+	dte_index = rk_iova_dte_index(iova);
+	dte_addr = &rk_domain->dt[dte_index];
 	dte = *dte_addr;
 	if (rk_dte_is_pt_valid(dte))
 		goto done;
@@ -627,19 +626,28 @@ static u32 *rk_dte_get_page_table(struct rk_iommu_domain *rk_domain,
 	if (!page_table)
 		return ERR_PTR(-ENOMEM);
 
-	dte = rk_mk_dte(page_table);
-	*dte_addr = dte;
+	pt_dma = dma_map_single(rk_domain->dev, page_table,
+				SPAGE_SIZE, DMA_TO_DEVICE);
+	if (dma_mapping_error(rk_domain->dev, pt_dma)) {
+		dev_err(rk_domain->dev,
+			"DMA mapping error while allocating page table\n");
+		free_page((unsigned long)page_table);
+		return ERR_PTR(-ENOMEM);
+	}
 
-	rk_table_flush(page_table, NUM_PT_ENTRIES);
-	rk_table_flush(dte_addr, 1);
+	dte = rk_mk_dte(pt_dma);
+	*dte_addr = dte;
 
+	rk_table_flush(rk_domain->dev, pt_dma, NUM_PT_ENTRIES);
+	rk_table_flush(rk_domain->dev, rk_domain->dt_dma + dte_index * 4, 1);
 done:
 	pt_phys = rk_dte_pt_address(dte);
 	return (u32 *)phys_to_virt(pt_phys);
 }
 
 static size_t rk_iommu_unmap_iova(struct rk_iommu_domain *rk_domain,
-				  u32 *pte_addr, dma_addr_t iova, size_t size)
+				  u32 *pte_addr, dma_addr_t pte_dma,
+				  size_t size)
 {
 	unsigned int pte_count;
 	unsigned int pte_total = size / SPAGE_SIZE;
@@ -654,14 +662,14 @@ static size_t rk_iommu_unmap_iova(struct rk_iommu_domain *rk_domain,
 		pte_addr[pte_count] = rk_mk_pte_invalid(pte);
 	}
 
-	rk_table_flush(pte_addr, pte_count);
+	rk_table_flush(rk_domain->dev, pte_dma, pte_count);
 
 	return pte_count * SPAGE_SIZE;
 }
 
 static int rk_iommu_map_iova(struct rk_iommu_domain *rk_domain, u32 *pte_addr,
-			     dma_addr_t iova, phys_addr_t paddr, size_t size,
-			     int prot)
+			     dma_addr_t pte_dma, dma_addr_t iova,
+			     phys_addr_t paddr, size_t size, int prot)
 {
 	unsigned int pte_count;
 	unsigned int pte_total = size / SPAGE_SIZE;
@@ -680,7 +688,7 @@ static int rk_iommu_map_iova(struct rk_iommu_domain *rk_domain, u32 *pte_addr,
 		paddr += SPAGE_SIZE;
 	}
 
-	rk_table_flush(pte_addr, pte_count);
+	rk_table_flush(rk_domain->dev, pte_dma, pte_total);
 
 	/*
 	 * Zap the first and last iova to evict from iotlb any previously
@@ -693,7 +701,8 @@ static int rk_iommu_map_iova(struct rk_iommu_domain *rk_domain, u32 *pte_addr,
 	return 0;
 unwind:
 	/* Unmap the range of iovas that we just mapped */
-	rk_iommu_unmap_iova(rk_domain, pte_addr, iova, pte_count * SPAGE_SIZE);
+	rk_iommu_unmap_iova(rk_domain, pte_addr, pte_dma,
+			    pte_count * SPAGE_SIZE);
 
 	iova += pte_count * SPAGE_SIZE;
 	page_phys = rk_pte_page_address(pte_addr[pte_count]);
@@ -708,8 +717,9 @@ static int rk_iommu_map(struct iommu_domain *domain, unsigned long _iova,
 {
 	struct rk_iommu_domain *rk_domain = to_rk_domain(domain);
 	unsigned long flags;
-	dma_addr_t iova = (dma_addr_t)_iova;
+	dma_addr_t pte_dma, iova = (dma_addr_t)_iova;
 	u32 *page_table, *pte_addr;
+	u32 dte_index, pte_index;
 	int ret;
 
 	spin_lock_irqsave(&rk_domain->dt_lock, flags);
@@ -727,8 +737,13 @@ static int rk_iommu_map(struct iommu_domain *domain, unsigned long _iova,
 		return PTR_ERR(page_table);
 	}
 
-	pte_addr = &page_table[rk_iova_pte_index(iova)];
-	ret = rk_iommu_map_iova(rk_domain, pte_addr, iova, paddr, size, prot);
+	dte_index = rk_domain->dt[rk_iova_dte_index(iova)];
+	pte_index = rk_iova_pte_index(iova);
+	pte_addr = &page_table[pte_index];
+	pte_dma = rk_dte_pt_address(dte_index) + pte_index * 4;
+	ret = rk_iommu_map_iova(rk_domain, pte_addr, pte_dma, iova,
+				paddr, size, prot);
+
 	spin_unlock_irqrestore(&rk_domain->dt_lock, flags);
 
 	return ret;
@@ -739,7 +754,7 @@ static size_t rk_iommu_unmap(struct iommu_domain *domain, unsigned long _iova,
 {
 	struct rk_iommu_domain *rk_domain = to_rk_domain(domain);
 	unsigned long flags;
-	dma_addr_t iova = (dma_addr_t)_iova;
+	dma_addr_t pte_dma, iova = (dma_addr_t)_iova;
 	phys_addr_t pt_phys;
 	u32 dte;
 	u32 *pte_addr;
@@ -763,7 +778,8 @@ static size_t rk_iommu_unmap(struct iommu_domain *domain, unsigned long _iova,
 
 	pt_phys = rk_dte_pt_address(dte);
 	pte_addr = (u32 *)phys_to_virt(pt_phys) + rk_iova_pte_index(iova);
-	unmap_size = rk_iommu_unmap_iova(rk_domain, pte_addr, iova, size);
+	pte_dma = pt_phys + rk_iova_pte_index(iova) * 4;
+	unmap_size = rk_iommu_unmap_iova(rk_domain, pte_addr, pte_dma, size);
 
 	spin_unlock_irqrestore(&rk_domain->dt_lock, flags);
 
@@ -875,7 +891,6 @@ static int rk_iommu_attach_device(struct iommu_domain *domain,
 	struct rk_iommu_domain *rk_domain = to_rk_domain(domain);
 	unsigned long flags;
 	int ret, i;
-	phys_addr_t dte_addr;
 
 	iommu = rk_iommu_from_dev(dev);
 
@@ -886,6 +901,22 @@ static int rk_iommu_attach_device(struct iommu_domain *domain,
 			return -ENOMEM;
 	}
 
+	/* Set the domain dev to virtual one if exist */
+	if (rk_iommu_is_virtual(iommu) || !rk_domain->dev)
+		rk_domain->dev = iommu->dev;
+
+	if (!rk_domain->dt_dma) {
+		rk_domain->dt_dma = dma_map_single(rk_domain->dev,
+				rk_domain->dt, SPAGE_SIZE, DMA_TO_DEVICE);
+		if (dma_mapping_error(rk_domain->dev, rk_domain->dt_dma)) {
+			dev_err(rk_domain->dev, "dma map error for DT\n");
+			return -ENOMEM;
+		}
+
+		rk_table_flush(rk_domain->dev, rk_domain->dt_dma,
+			       NUM_DT_ENTRIES);
+	}
+
 	iommu->domain = domain;
 	if (rk_iommu_is_virtual(iommu)) {
 		dev_dbg(dev, "Attach virtual device to iommu domain\n");
@@ -894,28 +925,28 @@ static int rk_iommu_attach_device(struct iommu_domain *domain,
 
 	ret = rk_iommu_enable_stall(iommu);
 	if (ret)
-		return ret;
+		goto unmap;
 
 	ret = rk_iommu_force_reset(iommu);
 	if (ret)
-		return ret;
+		goto unmap;
 
 
 	ret = devm_request_irq(iommu->dev, iommu->irq, rk_iommu_irq,
 			       IRQF_SHARED, dev_name(dev), iommu);
 	if (ret)
-		return ret;
+		goto unmap;
 
-	dte_addr = virt_to_phys(rk_domain->dt);
 	for (i = 0; i < iommu->num_mmu; i++) {
-		rk_iommu_write(iommu->bases[i], RK_MMU_DTE_ADDR, dte_addr);
+		rk_iommu_write(iommu->bases[i], RK_MMU_DTE_ADDR,
+			       rk_domain->dt_dma);
 		rk_iommu_command(iommu->bases[i], RK_MMU_CMD_ZAP_CACHE);
 		rk_iommu_write(iommu->bases[i], RK_MMU_INT_MASK, RK_MMU_IRQ_MASK);
 	}
 
 	ret = rk_iommu_enable_paging(iommu);
 	if (ret)
-		return ret;
+		goto unmap;
 
 	spin_lock_irqsave(&rk_domain->iommus_lock, flags);
 	list_add_tail(&iommu->node, &rk_domain->iommus);
@@ -926,6 +957,11 @@ static int rk_iommu_attach_device(struct iommu_domain *domain,
 	rk_iommu_disable_stall(iommu);
 
 	return 0;
+
+unmap:
+	dma_unmap_single(rk_domain->dev, rk_domain->dt_dma, SPAGE_SIZE,
+			 DMA_TO_DEVICE);
+	return ret;
 }
 
 static void rk_iommu_detach_device(struct iommu_domain *domain,
@@ -987,8 +1023,6 @@ static struct iommu_domain *rk_iommu_domain_alloc(unsigned type)
 	if (!rk_domain->dt)
 		goto err_dt;
 
-	rk_table_flush(rk_domain->dt, NUM_DT_ENTRIES);
-
 	spin_lock_init(&rk_domain->iommus_lock);
 	spin_lock_init(&rk_domain->dt_lock);
 	INIT_LIST_HEAD(&rk_domain->iommus);
@@ -1012,10 +1046,15 @@ static void rk_iommu_domain_free(struct iommu_domain *domain)
 		if (rk_dte_is_pt_valid(dte)) {
 			phys_addr_t pt_phys = rk_dte_pt_address(dte);
 			u32 *page_table = phys_to_virt(pt_phys);
+			dma_unmap_single(rk_domain->dev, pt_phys,
+					 SPAGE_SIZE, DMA_TO_DEVICE);
 			free_page((unsigned long)page_table);
 		}
 	}
 
+	if (!rk_domain->dt_dma)
+		dma_unmap_single(rk_domain->dev, rk_domain->dt_dma,
+				 SPAGE_SIZE, DMA_TO_DEVICE);
 	free_page((unsigned long)rk_domain->dt);
 
 	iommu_put_dma_cookie(&rk_domain->domain);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 6/6] iommu/rockchip: enable rockchip iommu on ARM64 platform
  2016-06-15 12:04 [PATCH v3 0/6] fix bugs; enable iommu for ARM64 Shunqian Zheng
                   ` (4 preceding siblings ...)
  2016-06-15 12:04 ` [PATCH v3 5/6] iommu/rockchip: use DMA API to map, to flush cache Shunqian Zheng
@ 2016-06-15 12:04 ` Shunqian Zheng
  5 siblings, 0 replies; 10+ messages in thread
From: Shunqian Zheng @ 2016-06-15 12:04 UTC (permalink / raw)
  To: joro, heiko, robh+dt, mark.rutland, linux, mark.yao, airlied, tfiga, xxm
  Cc: linux-arm-kernel, iommu, devicetree, dri-devel, linux-kernel,
	linux-rockchip, Shunqian Zheng

From: Simon Xue <xxm@rock-chips.com>

This patch makes it possible to compile the rockchip-iommu driver on
ARM64 platform to be used with 64-bit SoCs equipped with this type
of IOMMU.

Signed-off-by: Simon Xue <xxm@rock-chips.com>
Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com>
Reviewed-by: Tomasz Figa <tfiga@chromium.org>
---
 drivers/iommu/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index ad08603..5572621 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -218,7 +218,7 @@ config OMAP_IOMMU_DEBUG
 
 config ROCKCHIP_IOMMU
 	bool "Rockchip IOMMU Support"
-	depends on ARM
+	depends on ARM || ARM64
 	depends on ARCH_ROCKCHIP || COMPILE_TEST
 	select IOMMU_API
 	select ARM_DMA_USE_IOMMU
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 3/6] iommu/rockchip: support virtual iommu slave device
  2016-06-15 12:04 ` [PATCH v3 3/6] iommu/rockchip: support virtual iommu slave device Shunqian Zheng
@ 2016-06-15 14:21   ` Tomasz Figa
  0 siblings, 0 replies; 10+ messages in thread
From: Tomasz Figa @ 2016-06-15 14:21 UTC (permalink / raw)
  To: Shunqian Zheng
  Cc: Joerg Roedel, Heiko Stübner, Rob Herring, Mark Rutland,
	linux, 姚智情,
	David Airlie, simon xue, linux-arm-kernel,
	open list:IOMMU DRIVERS, devicetree, dri-devel, linux-kernel,
	open list:ARM/Rockchip SoC...

Hi Shunqian,

On Wed, Jun 15, 2016 at 9:04 PM, Shunqian Zheng <zhengsq@rock-chips.com> wrote:
> An virtual master device like DRM need to attach to iommu
> domain to share the mapping with VOPs(the one with actual
> iommu slaves).
> DRM attaches to iommu and allocates buffers before VOPs enabled,
> which means there may have not real iommu devices can be used
> to do dma mapping.
>
> This patch creates a iommu when virtual master(group is NULL)
> attaching, so it can use this iommu even the real iommus disabled.
>
> Changes of V3:
> - Instead of registering virtual iommu in DTS, this patch
>   creates a iommu when attaching.
>
> Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com>
> Suggested-by: Tomasz Figa <tfiga@chromium.org>

To clarify, I don't really like the idea of virtual IOMMU, however it
is registered, dts or manually, but I don't think there is any other
reasonable way of dealing with allocations for subsystems such as DRM,
where there are multiple devices in one IOMMU domain. Having said
that, please see some minor comments inline.

> ---
>  drivers/iommu/rockchip-iommu.c | 133 +++++++++++++++++++++++++++++++++++++----
>  1 file changed, 122 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
> index 3c16ec3..82ecc99 100644
> --- a/drivers/iommu/rockchip-iommu.c
> +++ b/drivers/iommu/rockchip-iommu.c
[snip]
> @@ -878,6 +975,9 @@ static struct iommu_domain *rk_iommu_domain_alloc(unsigned type)
>         if (!rk_domain)
>                 return NULL;
>
> +       if (iommu_get_dma_cookie(&rk_domain->domain))
> +               goto err_dt;
> +

Shouldn't this belong to a separate patch? Actually, is this required?
If so, how this worked before?

>         /*
>          * rk32xx iommus use a 2 level pagetable.
>          * Each level1 (dt) and level2 (pt) table has 1024 4-byte entries.
> @@ -917,6 +1017,9 @@ static void rk_iommu_domain_free(struct iommu_domain *domain)
>         }
>
>         free_page((unsigned long)rk_domain->dt);
> +
> +       iommu_put_dma_cookie(&rk_domain->domain);
> +

See above.

Otherwise, with the above addressed, you can add my Reviewed-by.

Best regards,
Tomasz

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 4/6] drm: rockchip: use common iommu api to attach iommu
  2016-06-15 12:04 ` [PATCH v3 4/6] drm: rockchip: use common iommu api to attach iommu Shunqian Zheng
@ 2016-06-15 14:24   ` Tomasz Figa
  0 siblings, 0 replies; 10+ messages in thread
From: Tomasz Figa @ 2016-06-15 14:24 UTC (permalink / raw)
  To: Shunqian Zheng
  Cc: Joerg Roedel, Heiko Stübner, Rob Herring, Mark Rutland,
	linux, 姚智情,
	David Airlie, simon xue, linux-arm-kernel,
	open list:IOMMU DRIVERS, devicetree, dri-devel, linux-kernel,
	open list:ARM/Rockchip SoC...

Hi Shunqian,

On Wed, Jun 15, 2016 at 9:04 PM, Shunqian Zheng <zhengsq@rock-chips.com> wrote:
> Rockchip DRM used the arm special API, arm_iommu_*(), to attach
> iommu for ARM32 SoCs. This patch convert to common iommu API
> so it would support ARM64 like RK3399.
>
> The general idea is domain_alloc(), attach_device() and
> arch_setup_dma_ops() to set dma_ops manually for DRM at the last.
>
> Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com>
> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 112 +++++++++++++++++-----------
>  drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   1 +
>  2 files changed, 71 insertions(+), 42 deletions(-)

Please see my comment inline.

> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> index f5a68fc..b52c38d 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> @@ -14,16 +14,16 @@
>   * GNU General Public License for more details.
>   */
>
> -#include <asm/dma-iommu.h>
> -
>  #include <drm/drmP.h>
>  #include <drm/drm_crtc_helper.h>
>  #include <drm/drm_fb_helper.h>
> +#include <linux/dma-iommu.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/module.h>
>  #include <linux/of_graph.h>
>  #include <linux/component.h>
> +#include <linux/iommu.h>
>
>  #include "rockchip_drm_drv.h"
>  #include "rockchip_drm_fb.h"
> @@ -46,7 +46,8 @@ static bool is_support_iommu = true;
>  int rockchip_drm_dma_attach_device(struct drm_device *drm_dev,
>                                    struct device *dev)
>  {
> -       struct dma_iommu_mapping *mapping = drm_dev->dev->archdata.mapping;
> +       struct rockchip_drm_private *private = drm_dev->dev_private;
> +       struct iommu_domain *domain = private->domain;
>         int ret;
>
>         if (!is_support_iommu)
> @@ -58,16 +59,25 @@ int rockchip_drm_dma_attach_device(struct drm_device *drm_dev,
>
>         dma_set_max_seg_size(dev, DMA_BIT_MASK(32));
>
> -       return arm_iommu_attach_device(dev, mapping);
> +       ret = iommu_attach_device(domain, dev);
> +       if (ret) {
> +               dev_err(dev, "Failed to attach iommu device\n");
> +               return ret;
> +       }
> +
> +       /* TODO(djkurtz): fetch the mapping start/size from somewhere */
> +       arch_setup_dma_ops(dev, 0x00000000, SZ_2G, dev->bus->iommu_ops, false);
> +       return 0;
>  }
>
>  void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
>                                     struct device *dev)
>  {
> -       if (!is_support_iommu)
> -               return;
> +       struct rockchip_drm_private *private = drm_dev->dev_private;
> +       struct iommu_domain *domain = private->domain;
>
> -       arm_iommu_detach_device(dev);
> +       if (is_support_iommu)
> +               iommu_detach_device(domain, dev);
>  }
>
>  int rockchip_register_crtc_funcs(struct drm_crtc *crtc,
> @@ -132,10 +142,52 @@ static void rockchip_drm_crtc_disable_vblank(struct drm_device *dev,
>                 priv->crtc_funcs[pipe]->disable_vblank(crtc);
>  }
>
> +static int rockchip_drm_init_iommu(struct drm_device *drm_dev)
> +{
> +       struct rockchip_drm_private *private = drm_dev->dev_private;
> +       struct device *dev = drm_dev->dev;
> +       int ret;
> +
> +       dev->dma_parms = devm_kzalloc(dev, sizeof(*dev->dma_parms),
> +                                     GFP_KERNEL);
> +       if (!dev->dma_parms)
> +               return -ENOMEM;
> +
> +       private->domain = iommu_domain_alloc(&platform_bus_type);
> +       if (!private->domain)
> +               return -ENOMEM;
> +
> +       /* TODO(djkurtz): fetch the mapping start/size from somewhere */
> +       ret = iommu_dma_init_domain(private->domain, 0x00000000, SZ_2G);
> +       if (ret) {
> +               dev_err(dev, "Failed to init domain\n");
> +               goto err_free_domain;
> +       }
> +
> +       ret = rockchip_drm_dma_attach_device(drm_dev, dev);
> +       if (ret) {
> +               dev_err(dev, "Failed to attach device\n");
> +               goto err_free_domain;
> +       }
> +
> +       return 0;
> +
> +err_free_domain:
> +       iommu_domain_free(private->domain);
> +
> +       return ret;
> +}
> +
> +static void rockchip_iommu_cleanup(struct drm_device *drm_dev)
> +{
> +       struct rockchip_drm_private *private = drm_dev->dev_private;
> +

No need to call rockchip_drm_dma_detach_device() here?

> +       iommu_domain_free(private->domain);
> +}

Otherwise looks good, so after addressing the above, feel free to add
my Reviewed-by.

Best regards,
Tomasz

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 5/6] iommu/rockchip: use DMA API to map, to flush cache
  2016-06-15 12:04 ` [PATCH v3 5/6] iommu/rockchip: use DMA API to map, to flush cache Shunqian Zheng
@ 2016-06-15 14:31   ` Tomasz Figa
  0 siblings, 0 replies; 10+ messages in thread
From: Tomasz Figa @ 2016-06-15 14:31 UTC (permalink / raw)
  To: Shunqian Zheng
  Cc: Joerg Roedel, Heiko Stübner, Rob Herring, Mark Rutland,
	linux, 姚智情,
	David Airlie, simon xue, linux-arm-kernel,
	open list:IOMMU DRIVERS, devicetree, dri-devel, linux-kernel,
	open list:ARM/Rockchip SoC...

Hi Shunqian,

On Wed, Jun 15, 2016 at 9:04 PM, Shunqian Zheng <zhengsq@rock-chips.com> wrote:
> Use DMA API instead of architecture internal functions like
> __cpuc_flush_dcache_area() etc.
>
> To support the virtual device like DRM the virtual slave iommu
> added in the previous patch, attaching to which the DRM can use
> it own domain->dev for dma_map_*(), dma_sync_*() even VOP is disabled.
>
> With this patch, this driver is available for ARM64 like RK3399.
>
> Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com>
> ---
>  drivers/iommu/rockchip-iommu.c | 113 +++++++++++++++++++++++++++--------------
>  1 file changed, 76 insertions(+), 37 deletions(-)

In general looks good to me, but still have some concern about
attaching and detaching. Please see inline.

> diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
> index 82ecc99..b60b29e 100644
> --- a/drivers/iommu/rockchip-iommu.c
> +++ b/drivers/iommu/rockchip-iommu.c
[snip]
> @@ -886,6 +901,22 @@ static int rk_iommu_attach_device(struct iommu_domain *domain,
>                         return -ENOMEM;
>         }
>
> +       /* Set the domain dev to virtual one if exist */
> +       if (rk_iommu_is_virtual(iommu) || !rk_domain->dev)

Does it matter if the iommu is virtual or not? This condition will
always evaluate to true on first attach anyway, so there might be
times when rk_domain->dev points to a real device.

> +               rk_domain->dev = iommu->dev;
> +
[snip]
>  static void rk_iommu_detach_device(struct iommu_domain *domain,
> @@ -987,8 +1023,6 @@ static struct iommu_domain *rk_iommu_domain_alloc(unsigned type)
>         if (!rk_domain->dt)
>                 goto err_dt;
>
> -       rk_table_flush(rk_domain->dt, NUM_DT_ENTRIES);
> -
>         spin_lock_init(&rk_domain->iommus_lock);
>         spin_lock_init(&rk_domain->dt_lock);
>         INIT_LIST_HEAD(&rk_domain->iommus);
> @@ -1012,10 +1046,15 @@ static void rk_iommu_domain_free(struct iommu_domain *domain)
>                 if (rk_dte_is_pt_valid(dte)) {
>                         phys_addr_t pt_phys = rk_dte_pt_address(dte);
>                         u32 *page_table = phys_to_virt(pt_phys);
> +                       dma_unmap_single(rk_domain->dev, pt_phys,
> +                                        SPAGE_SIZE, DMA_TO_DEVICE);
>                         free_page((unsigned long)page_table);
>                 }
>         }
>
> +       if (!rk_domain->dt_dma)
> +               dma_unmap_single(rk_domain->dev, rk_domain->dt_dma,
> +                                SPAGE_SIZE, DMA_TO_DEVICE);
>         free_page((unsigned long)rk_domain->dt);
>
>         iommu_put_dma_cookie(&rk_domain->domain);

If we detach here a device whose IOMMU is currently pointed by
rk_domain->dev, then the pointer will not point to anything valid
anymore. To be honest, I don't see any good solution for this. Maybe
we should keep all the IOMMUs in a list and set the ->dev pointer to
any from the list here?

Best regards,
Tomasz

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2016-06-15 14:32 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-15 12:04 [PATCH v3 0/6] fix bugs; enable iommu for ARM64 Shunqian Zheng
2016-06-15 12:04 ` [PATCH v3 1/6] iommu/rockchip: fix devm_{request,free}_irq parameter Shunqian Zheng
2016-06-15 12:04 ` [PATCH v3 2/6] iommu/rockchip: add map_sg callback for rk_iommu_ops Shunqian Zheng
2016-06-15 12:04 ` [PATCH v3 3/6] iommu/rockchip: support virtual iommu slave device Shunqian Zheng
2016-06-15 14:21   ` Tomasz Figa
2016-06-15 12:04 ` [PATCH v3 4/6] drm: rockchip: use common iommu api to attach iommu Shunqian Zheng
2016-06-15 14:24   ` Tomasz Figa
2016-06-15 12:04 ` [PATCH v3 5/6] iommu/rockchip: use DMA API to map, to flush cache Shunqian Zheng
2016-06-15 14:31   ` Tomasz Figa
2016-06-15 12:04 ` [PATCH v3 6/6] iommu/rockchip: enable rockchip iommu on ARM64 platform Shunqian Zheng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).