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

Groups > linux.kernel > #1427280 > unrolled thread

[PATCH v4 0/8] iommu/rockchip: Fix bugs and enable on ARM64

Back to article view | Back to linux.kernel

Contents

  [PATCH v4 0/8] iommu/rockchip: Fix bugs and enable on ARM64 Tomasz Figa <tfiga@chromium.org> - 2016-06-21 06:40 +0200
    [PATCH v4 7/8] drm/rockchip: Use common IOMMU API to attach devices Tomasz Figa <tfiga@chromium.org> - 2016-06-21 06:40 +0200
    [PATCH v4 5/8] iommu/rockchip: Prepare to support generic DMA mapping Tomasz Figa <tfiga@chromium.org> - 2016-06-21 06:40 +0200
    Re: [PATCH v4 0/8] iommu/rockchip: Fix bugs and enable on ARM64 Joerg Roedel <joro@8bytes.org> - 2016-06-21 11:30 +0200
      Re: [PATCH v4 0/8] iommu/rockchip: Fix bugs and enable on ARM64 Tomasz Figa <tfiga@chromium.org> - 2016-06-21 14:50 +0200
        Re: [PATCH v4 0/8] iommu/rockchip: Fix bugs and enable on ARM64 Joerg Roedel <joro@8bytes.org> - 2016-06-21 15:10 +0200
          Re: [PATCH v4 0/8] iommu/rockchip: Fix bugs and enable on ARM64 Heiko Stübner <heiko@sntech.de> - 2016-06-21 16:00 +0200
            Re: [PATCH v4 0/8] iommu/rockchip: Fix bugs and enable on ARM64 Joerg Roedel <joro@8bytes.org> - 2016-06-21 16:10 +0200

#1427280 — [PATCH v4 0/8] iommu/rockchip: Fix bugs and enable on ARM64

This series intends mostly to enable support for ARM64 architecture
in the rockchip-iommu driver. On the way to do so, some bugs are also
fixed.

The most important changes here are:
 - making the Rockchip IOMMU driver use DMA API for managing cache
   coherency of page tables,
 - making the Rockchip DRM driver not use DMA API on behalf of a virtual
   device (behind a virtual IOMMU) to allocate and map buffers, but
   instead proper DRM helpers and IOMMU API directly.

Changes since v3:
 - Drop the idea of virtual IOMMU. Instead replace hacky allocation code
   in DRM driver, with proper management of IOMMU domain.
 - Add one more fix for allocation of IOMMU register base addresses.
Changes since v2:
 - Instead of registering virtual IOMMU from DTS, create it when
   attaching.
 - Fix some bugs found in internal review.

Shunqian Zheng (4):
  iommu/rockchip: Fix allocation of bases array in driver probe
  iommu/rockchip: Use DMA API to manage coherency
  iommu/rockchip: Prepare to support generic DMA mapping
  drm/rockchip: Use common IOMMU API to attach devices

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

Tomasz Figa (1):
  drm/rockchip: Do not use DMA mapping API if attached to IOMMU domain

 drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 100 ++++++------
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   3 +
 drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 236 ++++++++++++++++++++++++++--
 drivers/gpu/drm/rockchip/rockchip_drm_gem.h |   9 ++
 drivers/iommu/Kconfig                       |   2 +-
 drivers/iommu/rockchip-iommu.c              | 180 +++++++++++++++------
 6 files changed, 427 insertions(+), 103 deletions(-)

-- 
2.8.0.rc3.226.g39d4020

[toc] | [next] | [standalone]

#1427282 — [PATCH v4 7/8] drm/rockchip: Use common IOMMU API to attach devices

From: Shunqian Zheng <zhengsq@rock-chips.com>

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.

Since previous patch added support for direct IOMMU address space
management, there is no need to use DMA API anymore and this patch wires
things to use the new method.

Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com>
Signed-off-by: Tomasz Figa <tfiga@chromium.org>
---
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 100 +++++++++++++++-------------
 1 file changed, 53 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index e2c31d3..2793ac9 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -14,18 +14,18 @@
  * 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 <drm/drm_gem_cma_helper.h>
 #include <linux/dma-mapping.h>
+#include <linux/dma-iommu.h>
 #include <linux/pm_runtime.h>
 #include <linux/module.h>
 #include <linux/of_graph.h>
 #include <linux/component.h>
 #include <linux/console.h>
+#include <linux/iommu.h>
 
 #include "rockchip_drm_drv.h"
 #include "rockchip_drm_fb.h"
@@ -49,28 +49,31 @@ static struct drm_driver rockchip_drm_driver;
 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;
 	int ret;
 
 	if (!is_support_iommu)
 		return 0;
 
-	ret = dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
-	if (ret)
+	ret = iommu_attach_device(private->domain, dev);
+	if (ret) {
+		dev_err(dev, "Failed to attach iommu device\n");
 		return ret;
+	}
 
-	dma_set_max_seg_size(dev, DMA_BIT_MASK(32));
-
-	return arm_iommu_attach_device(dev, mapping);
+	return 0;
 }
 
 void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
 				    struct device *dev)
 {
+	struct rockchip_drm_private *private = drm_dev->dev_private;
+	struct iommu_domain *domain = private->domain;
+
 	if (!is_support_iommu)
 		return;
 
-	arm_iommu_detach_device(dev);
+	iommu_detach_device(domain, dev);
 }
 
 int rockchip_register_crtc_funcs(struct drm_crtc *crtc,
@@ -135,11 +138,45 @@ 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 iommu_domain_geometry *geometry;
+	u64 start, end;
+
+	if (!is_support_iommu)
+		return 0;
+
+	private->domain = iommu_domain_alloc(&platform_bus_type);
+	if (!private->domain)
+		return -ENOMEM;
+
+	geometry = &private->domain->geometry;
+	start = geometry->aperture_start;
+	end = geometry->aperture_end;
+
+	DRM_DEBUG("IOMMU context initialized (aperture: %#llx-%#llx)\n",
+		  start, end);
+	drm_mm_init(&private->mm, start, end - start + 1);
+
+	return 0;
+}
+
+static void rockchip_iommu_cleanup(struct drm_device *drm_dev)
+{
+	struct rockchip_drm_private *private = drm_dev->dev_private;
+
+	if (!is_support_iommu)
+		return;
+
+	drm_mm_takedown(&private->mm);
+	iommu_domain_free(private->domain);
+}
+
 static int rockchip_drm_bind(struct device *dev)
 {
 	struct drm_device *drm_dev;
 	struct rockchip_drm_private *private;
-	struct dma_iommu_mapping *mapping = NULL;
 	int ret;
 
 	drm_dev = drm_dev_alloc(&rockchip_drm_driver, dev);
@@ -160,38 +197,14 @@ static int rockchip_drm_bind(struct device *dev)
 
 	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;
+	ret = rockchip_drm_init_iommu(drm_dev);
+	if (ret)
 		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);
-		if (ret)
-			goto err_release_mapping;
-	}
 
 	/* Try to bind all sub drivers. */
 	ret = component_bind_all(dev, drm_dev);
 	if (ret)
-		goto err_detach_device;
+		goto err_iommu_cleanup;
 
 	/* init kms poll for handling hpd */
 	drm_kms_helper_poll_init(drm_dev);
@@ -216,8 +229,6 @@ static int rockchip_drm_bind(struct device *dev)
 	if (ret)
 		goto err_fbdev_fini;
 
-	if (is_support_iommu)
-		arm_iommu_release_mapping(mapping);
 	return 0;
 err_fbdev_fini:
 	rockchip_drm_fbdev_fini(drm_dev);
@@ -227,12 +238,8 @@ 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:
-	if (is_support_iommu)
-		arm_iommu_release_mapping(mapping);
+err_iommu_cleanup:
+	rockchip_iommu_cleanup(drm_dev);
 err_config_cleanup:
 	drm_mode_config_cleanup(drm_dev);
 	drm_dev->dev_private = NULL;
@@ -251,8 +258,7 @@ static void rockchip_drm_unbind(struct device *dev)
 	drm_vblank_cleanup(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;
 	drm_dev_unregister(drm_dev);
-- 
2.8.0.rc3.226.g39d4020

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

#1427283 — [PATCH v4 5/8] iommu/rockchip: Prepare to support generic DMA mapping

From: Shunqian Zheng <zhengsq@rock-chips.com>

Set geometry for allocated domains and fix .domain_alloc() callback to
work with IOMMU_DOMAIN_DMA domain type, which is used for implicit
domains on ARM64.

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

diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index 0551146..d5fd074 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -888,7 +888,7 @@ static struct iommu_domain *rk_iommu_domain_alloc(unsigned type)
 	struct platform_device *pdev;
 	struct device *iommu_dev;
 
-	if (type != IOMMU_DOMAIN_UNMANAGED)
+	if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
 		return NULL;
 
 	/* Register a pdev per domain, so DMA API can base on this *dev
@@ -905,8 +905,8 @@ static struct iommu_domain *rk_iommu_domain_alloc(unsigned type)
 
 	rk_domain->pdev = pdev;
 
-	/* To init the iovad which is required by iommu_dma_init_domain() */
-	if (iommu_get_dma_cookie(&rk_domain->domain))
+	if (type == IOMMU_DOMAIN_DMA &&
+	    iommu_get_dma_cookie(&rk_domain->domain))
 		goto err_unreg_pdev;
 
 	/*
@@ -932,12 +932,17 @@ static struct iommu_domain *rk_iommu_domain_alloc(unsigned type)
 	spin_lock_init(&rk_domain->dt_lock);
 	INIT_LIST_HEAD(&rk_domain->iommus);
 
+	rk_domain->domain.geometry.aperture_start = 0;
+	rk_domain->domain.geometry.aperture_end   = DMA_BIT_MASK(32);
+	rk_domain->domain.geometry.force_aperture = true;
+
 	return &rk_domain->domain;
 
 err_free_dt:
 	free_page((unsigned long)rk_domain->dt);
 err_put_cookie:
-	iommu_put_dma_cookie(&rk_domain->domain);
+	if (type == IOMMU_DOMAIN_DMA)
+		iommu_put_dma_cookie(&rk_domain->domain);
 err_unreg_pdev:
 	platform_device_unregister(pdev);
 
@@ -966,7 +971,8 @@ static void rk_iommu_domain_free(struct iommu_domain *domain)
 			 SPAGE_SIZE, DMA_TO_DEVICE);
 	free_page((unsigned long)rk_domain->dt);
 
-	iommu_put_dma_cookie(&rk_domain->domain);
+	if (domain->type == IOMMU_DOMAIN_DMA)
+		iommu_put_dma_cookie(&rk_domain->domain);
 
 	platform_device_unregister(rk_domain->pdev);
 }
-- 
2.8.0.rc3.226.g39d4020

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

#1427510

On Tue, Jun 21, 2016 at 01:34:33PM +0900, Tomasz Figa wrote:
> This series intends mostly to enable support for ARM64 architecture
> in the rockchip-iommu driver. On the way to do so, some bugs are also
> fixed.
> 
> The most important changes here are:
>  - making the Rockchip IOMMU driver use DMA API for managing cache
>    coherency of page tables,
>  - making the Rockchip DRM driver not use DMA API on behalf of a virtual
>    device (behind a virtual IOMMU) to allocate and map buffers, but
>    instead proper DRM helpers and IOMMU API directly.

Are these two parts dependent on each other or can the IOMMU and the DRM
part merged independently?


	Joerg

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

#1427720

Hi Joerg,

On Tue, Jun 21, 2016 at 6:17 PM, Joerg Roedel <joro@8bytes.org> wrote:
>
> On Tue, Jun 21, 2016 at 01:34:33PM +0900, Tomasz Figa wrote:
> > This series intends mostly to enable support for ARM64 architecture
> > in the rockchip-iommu driver. On the way to do so, some bugs are also
> > fixed.
> >
> > The most important changes here are:
> >  - making the Rockchip IOMMU driver use DMA API for managing cache
> >    coherency of page tables,
> >  - making the Rockchip DRM driver not use DMA API on behalf of a virtual
> >    device (behind a virtual IOMMU) to allocate and map buffers, but
> >    instead proper DRM helpers and IOMMU API directly.
>
> Are these two parts dependent on each other or can the IOMMU and the DRM
> part merged independently?

In simple words, DRM patches depend on IOMMU patches.

More precisely: The IOMMU patches alone are supposed to not break
anything. Same goes for the first DRM patch (7/8). Only second DRM
patch (8/8) depends on changes introduced by its predecessors.

Best regards,
Tomasz

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

#1427731

Hi Tomasz,

On Tue, Jun 21, 2016 at 09:42:16PM +0900, Tomasz Figa wrote:
> In simple words, DRM patches depend on IOMMU patches.
> 
> More precisely: The IOMMU patches alone are supposed to not break
> anything. Same goes for the first DRM patch (7/8). Only second DRM
> patch (8/8) depends on changes introduced by its predecessors.

The first DRM patch is 6/7, so it is 7/8 with the iommu dependency,
right? Anyway, I think the best is I take the iommu patches when Heiko
is ok with them and then the DRM tree can merge that branch in to apply
the DRM patches.

But first Heiko should have a look at the patches.


Thanks,

	Joerg

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

#1427779

Am Dienstag, 21. Juni 2016, 14:54:35 schrieb Joerg Roedel:
> Hi Tomasz,
> 
> On Tue, Jun 21, 2016 at 09:42:16PM +0900, Tomasz Figa wrote:
> > In simple words, DRM patches depend on IOMMU patches.
> > 
> > More precisely: The IOMMU patches alone are supposed to not break
> > anything. Same goes for the first DRM patch (7/8). Only second DRM
> > patch (8/8) depends on changes introduced by its predecessors.
> 
> The first DRM patch is 6/7, so it is 7/8 with the iommu dependency,
> right? Anyway, I think the best is I take the iommu patches when Heiko
> is ok with them and then the DRM tree can merge that branch in to apply
> the DRM patches.
> 
> But first Heiko should have a look at the patches.

I think from all his previous work on the rockchip iommus Tomasz is a lot more 
qualified to judge them - which I guess he did when picking up the ones from 
Rockchip devs :-) .

From a style-side, please don't carry the Reviewed-on gerrit tags over to 
mainline patches (patches 1 and 2).

Other than that, I didn't see anything jump out and it looks all pretty nice.


Heiko

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

#1427789

On Tue, Jun 21, 2016 at 03:18:49PM +0200, Heiko Stübner wrote:
> Am Dienstag, 21. Juni 2016, 14:54:35 schrieb Joerg Roedel:
> > Hi Tomasz,
> > 
> > On Tue, Jun 21, 2016 at 09:42:16PM +0900, Tomasz Figa wrote:
> > > In simple words, DRM patches depend on IOMMU patches.
> > > 
> > > More precisely: The IOMMU patches alone are supposed to not break
> > > anything. Same goes for the first DRM patch (7/8). Only second DRM
> > > patch (8/8) depends on changes introduced by its predecessors.
> > 
> > The first DRM patch is 6/7, so it is 7/8 with the iommu dependency,
> > right? Anyway, I think the best is I take the iommu patches when Heiko
> > is ok with them and then the DRM tree can merge that branch in to apply
> > the DRM patches.
> > 
> > But first Heiko should have a look at the patches.
> 
> I think from all his previous work on the rockchip iommus Tomasz is a lot more 
> qualified to judge them - which I guess he did when picking up the ones from 
> Rockchip devs :-) .

In that case you guys should probably co-maintain that driver?
> 
> >From a style-side, please don't carry the Reviewed-on gerrit tags over to 
> mainline patches (patches 1 and 2).

Okay.



	Joerg

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web