Re: [RFC PATCH v1 09/15] drm/msm/gpu: Move address space setup to the GPU targets

2019-03-03 Thread Jonathan Marek
There is an error in the a2xx part of this patch: 0xfff in adreno_gpu.c 
became 0xff in a2xx_gpu.c


On 3/1/19 2:38 PM, Jordan Crouse wrote:

Move the address space steup code out of the generic msm GPU code to
to the individual GPU targets. This allows us to do target specific
setup such as gpummu for a2xx or split pagetables and per-instance
pagetables for newer a5xx and a6xx targets. All this is at the
expense of duplicated code in some of the target files but I think
it pays for itself in improved code flow and flexibility.

Signed-off-by: Jordan Crouse 
---

  drivers/gpu/drm/msm/adreno/a2xx_gpu.c   | 37 --
  drivers/gpu/drm/msm/adreno/a3xx_gpu.c   | 50 ++
  drivers/gpu/drm/msm/adreno/a4xx_gpu.c   | 51 +++
  drivers/gpu/drm/msm/adreno/a5xx_gpu.c   | 37 +++---
  drivers/gpu/drm/msm/adreno/a6xx_gpu.c   | 37 +++---
  drivers/gpu/drm/msm/adreno/adreno_gpu.c |  7 -
  drivers/gpu/drm/msm/msm_gem.h   |  1 +
  drivers/gpu/drm/msm/msm_gpu.c   | 54 ++---
  drivers/gpu/drm/msm/msm_gpu.h   |  2 ++
  9 files changed, 173 insertions(+), 103 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a2xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a2xx_gpu.c
index 1f83bc1..49241d0 100644
--- a/drivers/gpu/drm/msm/adreno/a2xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a2xx_gpu.c
@@ -401,6 +401,30 @@ static struct msm_gpu_state *a2xx_gpu_state_get(struct 
msm_gpu *gpu)
return state;
  }
  
+static struct msm_gem_address_space *

+a2xx_create_address_space(struct msm_gpu *gpu)
+{
+   struct msm_gem_address_space *aspace;
+   int ret;
+
+   aspace = msm_gem_address_space_create_a2xx(&gpu->pdev->dev, gpu,
+   "gpu", SZ_16M, SZ_16M + 0xff * SZ_64K);
+   if (IS_ERR(aspace)) {
+   DRM_DEV_ERROR(gpu->dev->dev,
+   "No memory protection without MMU\n");
+   return ERR_PTR(-ENXIO);
+   }
+
+   ret = aspace->mmu->funcs->attach(aspace->mmu, NULL, 0);
+   if (ret) {
+   msm_gem_address_space_put(aspace);
+   return ERR_PTR(ret);
+   }
+
+   return aspace;
+}
+
+
  /* Register offset defines for A2XX - copy of A3XX */
  static const unsigned int a2xx_register_offsets[REG_ADRENO_REGISTER_MAX] = {
REG_ADRENO_DEFINE(REG_ADRENO_CP_RB_BASE, REG_AXXX_CP_RB_BASE),
@@ -429,6 +453,7 @@ static const struct adreno_gpu_funcs funcs = {
  #endif
.gpu_state_get = a2xx_gpu_state_get,
.gpu_state_put = adreno_gpu_state_put,
+   .create_address_space = a2xx_create_address_space,
},
  };
  
@@ -473,16 +498,8 @@ struct msm_gpu *a2xx_gpu_init(struct drm_device *dev)

adreno_gpu->reg_offsets = a2xx_register_offsets;
  
  	ret = adreno_gpu_init(dev, pdev, adreno_gpu, &funcs, 1);

-   if (ret)
-   goto fail;
-
-   if (!gpu->aspace) {
-   dev_err(dev->dev, "No memory protection without MMU\n");
-   ret = -ENXIO;
-   goto fail;
-   }
-
-   return gpu;
+   if (!ret)
+   return gpu;
  
  fail:

if (a2xx_gpu)
diff --git a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
index c3b4bc6..33ab5e8 100644
--- a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
@@ -21,6 +21,7 @@
  #  include 
  #endif
  
+#include "msm_gem.h"

  #include "a3xx_gpu.h"
  
  #define A3XX_INT0_MASK \

@@ -433,6 +434,41 @@ static struct msm_gpu_state *a3xx_gpu_state_get(struct 
msm_gpu *gpu)
return state;
  }
  
+static struct msm_gem_address_space *

+a3xx_create_address_space(struct msm_gpu *gpu)
+{
+   struct msm_gem_address_space *aspace;
+   struct iommu_domain *iommu;
+   int ret;
+
+   iommu = iommu_domain_alloc(&platform_bus_type);
+   if (!iommu) {
+   DRM_DEV_ERROR(gpu->dev->dev,
+   "No memory protection without IOMMU\n");
+   return ERR_PTR(-ENXIO);
+   }
+
+   iommu->geometry.aperture_start = SZ_16M;
+   iommu->geometry.aperture_end = 0x;
+
+   aspace = msm_gem_address_space_create(&gpu->pdev->dev, iommu, "gpu");
+   if (IS_ERR(aspace)) {
+   iommu_domain_free(iommu);
+   DRM_DEV_ERROR(gpu->dev->dev, "failed to init mmu: %ld\n",
+   PTR_ERR(aspace));
+   return aspace;
+   }
+
+   ret = aspace->mmu->funcs->attach(aspace->mmu, NULL, 0);
+   if (ret) {
+   msm_gem_address_space_put(aspace);
+   return ERR_PTR(ret);
+   }
+
+   return aspace;
+}
+
+
  /* Register offset defines for A3XX */
  static const unsigned int a3xx_register_offsets[REG_ADRENO_REGISTER_MAX] = {
REG_ADRENO_DEFINE(REG_ADRENO_CP_RB_BASE, REG_AXXX_CP_RB_BASE),
@@ -461,6 +497,7 @@ static const struct adreno_gpu_funcs funcs = {

[RFC PATCH v1 09/15] drm/msm/gpu: Move address space setup to the GPU targets

2019-03-01 Thread Jordan Crouse
Move the address space steup code out of the generic msm GPU code to
to the individual GPU targets. This allows us to do target specific
setup such as gpummu for a2xx or split pagetables and per-instance
pagetables for newer a5xx and a6xx targets. All this is at the
expense of duplicated code in some of the target files but I think
it pays for itself in improved code flow and flexibility.

Signed-off-by: Jordan Crouse 
---

 drivers/gpu/drm/msm/adreno/a2xx_gpu.c   | 37 --
 drivers/gpu/drm/msm/adreno/a3xx_gpu.c   | 50 ++
 drivers/gpu/drm/msm/adreno/a4xx_gpu.c   | 51 +++
 drivers/gpu/drm/msm/adreno/a5xx_gpu.c   | 37 +++---
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c   | 37 +++---
 drivers/gpu/drm/msm/adreno/adreno_gpu.c |  7 -
 drivers/gpu/drm/msm/msm_gem.h   |  1 +
 drivers/gpu/drm/msm/msm_gpu.c   | 54 ++---
 drivers/gpu/drm/msm/msm_gpu.h   |  2 ++
 9 files changed, 173 insertions(+), 103 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a2xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a2xx_gpu.c
index 1f83bc1..49241d0 100644
--- a/drivers/gpu/drm/msm/adreno/a2xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a2xx_gpu.c
@@ -401,6 +401,30 @@ static struct msm_gpu_state *a2xx_gpu_state_get(struct 
msm_gpu *gpu)
return state;
 }
 
+static struct msm_gem_address_space *
+a2xx_create_address_space(struct msm_gpu *gpu)
+{
+   struct msm_gem_address_space *aspace;
+   int ret;
+
+   aspace = msm_gem_address_space_create_a2xx(&gpu->pdev->dev, gpu,
+   "gpu", SZ_16M, SZ_16M + 0xff * SZ_64K);
+   if (IS_ERR(aspace)) {
+   DRM_DEV_ERROR(gpu->dev->dev,
+   "No memory protection without MMU\n");
+   return ERR_PTR(-ENXIO);
+   }
+
+   ret = aspace->mmu->funcs->attach(aspace->mmu, NULL, 0);
+   if (ret) {
+   msm_gem_address_space_put(aspace);
+   return ERR_PTR(ret);
+   }
+
+   return aspace;
+}
+
+
 /* Register offset defines for A2XX - copy of A3XX */
 static const unsigned int a2xx_register_offsets[REG_ADRENO_REGISTER_MAX] = {
REG_ADRENO_DEFINE(REG_ADRENO_CP_RB_BASE, REG_AXXX_CP_RB_BASE),
@@ -429,6 +453,7 @@ static const struct adreno_gpu_funcs funcs = {
 #endif
.gpu_state_get = a2xx_gpu_state_get,
.gpu_state_put = adreno_gpu_state_put,
+   .create_address_space = a2xx_create_address_space,
},
 };
 
@@ -473,16 +498,8 @@ struct msm_gpu *a2xx_gpu_init(struct drm_device *dev)
adreno_gpu->reg_offsets = a2xx_register_offsets;
 
ret = adreno_gpu_init(dev, pdev, adreno_gpu, &funcs, 1);
-   if (ret)
-   goto fail;
-
-   if (!gpu->aspace) {
-   dev_err(dev->dev, "No memory protection without MMU\n");
-   ret = -ENXIO;
-   goto fail;
-   }
-
-   return gpu;
+   if (!ret)
+   return gpu;
 
 fail:
if (a2xx_gpu)
diff --git a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
index c3b4bc6..33ab5e8 100644
--- a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
@@ -21,6 +21,7 @@
 #  include 
 #endif
 
+#include "msm_gem.h"
 #include "a3xx_gpu.h"
 
 #define A3XX_INT0_MASK \
@@ -433,6 +434,41 @@ static struct msm_gpu_state *a3xx_gpu_state_get(struct 
msm_gpu *gpu)
return state;
 }
 
+static struct msm_gem_address_space *
+a3xx_create_address_space(struct msm_gpu *gpu)
+{
+   struct msm_gem_address_space *aspace;
+   struct iommu_domain *iommu;
+   int ret;
+
+   iommu = iommu_domain_alloc(&platform_bus_type);
+   if (!iommu) {
+   DRM_DEV_ERROR(gpu->dev->dev,
+   "No memory protection without IOMMU\n");
+   return ERR_PTR(-ENXIO);
+   }
+
+   iommu->geometry.aperture_start = SZ_16M;
+   iommu->geometry.aperture_end = 0x;
+
+   aspace = msm_gem_address_space_create(&gpu->pdev->dev, iommu, "gpu");
+   if (IS_ERR(aspace)) {
+   iommu_domain_free(iommu);
+   DRM_DEV_ERROR(gpu->dev->dev, "failed to init mmu: %ld\n",
+   PTR_ERR(aspace));
+   return aspace;
+   }
+
+   ret = aspace->mmu->funcs->attach(aspace->mmu, NULL, 0);
+   if (ret) {
+   msm_gem_address_space_put(aspace);
+   return ERR_PTR(ret);
+   }
+
+   return aspace;
+}
+
+
 /* Register offset defines for A3XX */
 static const unsigned int a3xx_register_offsets[REG_ADRENO_REGISTER_MAX] = {
REG_ADRENO_DEFINE(REG_ADRENO_CP_RB_BASE, REG_AXXX_CP_RB_BASE),
@@ -461,6 +497,7 @@ static const struct adreno_gpu_funcs funcs = {
 #endif
.gpu_state_get = a3xx_gpu_state_get,
.gpu_state_put = adreno_gpu_state_put,
+   .create_address_space = a3xx_create_addres