[RFC PATCH v1 09/15] drm/msm/gpu: Move address space setup to the GPU targets
Jonathan Marek
jonathan at marek.ca
Fri Mar 1 23:01:06 UTC 2019
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 <jcrouse at codeaurora.org>
> ---
>
> 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 <mach/ocmem.h>
> #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 = 0xffffffff;
> +
> + 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_address_space,
> },
> };
>
> @@ -520,19 +557,6 @@ struct msm_gpu *a3xx_gpu_init(struct drm_device *dev)
> #endif
> }
>
> - if (!gpu->aspace) {
> - /* TODO we think it is possible to configure the GPU to
> - * restrict access to VRAM carveout. But the required
> - * registers are unknown. For now just bail out and
> - * limp along with just modesetting. If it turns out
> - * to not be possible to restrict access, then we must
> - * implement a cmdstream validator.
> - */
> - DRM_DEV_ERROR(dev->dev, "No memory protection without IOMMU\n");
> - ret = -ENXIO;
> - goto fail;
> - }
> -
> return gpu;
>
> fail:
> diff --git a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
> index 18f9a8e..08a5729 100644
> --- a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
> @@ -15,6 +15,8 @@
> # include <soc/qcom/ocmem.h>
> #endif
>
> +#include "msm_gem.h"
> +
> #define A4XX_INT0_MASK \
> (A4XX_INT0_RBBM_AHB_ERROR | \
> A4XX_INT0_RBBM_ATB_BUS_OVERFLOW | \
> @@ -530,6 +532,41 @@ static int a4xx_get_timestamp(struct msm_gpu *gpu, uint64_t *value)
> return 0;
> }
>
> +static struct msm_gem_address_space *
> +a4xx_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 = 0xffffffff;
> +
> + 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;
> +}
> +
> +
> static const struct adreno_gpu_funcs funcs = {
> .base = {
> .get_param = adreno_get_param,
> @@ -547,6 +584,7 @@ static const struct adreno_gpu_funcs funcs = {
> #endif
> .gpu_state_get = a4xx_gpu_state_get,
> .gpu_state_put = adreno_gpu_state_put,
> + .create_address_space = a4xx_create_address_space,
> },
> .get_timestamp = a4xx_get_timestamp,
> };
> @@ -600,19 +638,6 @@ struct msm_gpu *a4xx_gpu_init(struct drm_device *dev)
> #endif
> }
>
> - if (!gpu->aspace) {
> - /* TODO we think it is possible to configure the GPU to
> - * restrict access to VRAM carveout. But the required
> - * registers are unknown. For now just bail out and
> - * limp along with just modesetting. If it turns out
> - * to not be possible to restrict access, then we must
> - * implement a cmdstream validator.
> - */
> - DRM_DEV_ERROR(dev->dev, "No memory protection without IOMMU\n");
> - ret = -ENXIO;
> - goto fail;
> - }
> -
> return gpu;
>
> fail:
> diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> index 45662d3..3d6f414 100644
> --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> @@ -1456,6 +1456,38 @@ static unsigned long a5xx_gpu_busy(struct msm_gpu *gpu)
> return (unsigned long)busy_time;
> }
>
> +static struct msm_gem_address_space *
> +a5xx_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)
> + return NULL;
> +
> + iommu->geometry.aperture_start = 0x100000000ULL;
> + iommu->geometry.aperture_end = 0x1ffffffffULL;
> +
> + 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);
> + }
> +
> + msm_mmu_set_fault_handler(aspace->mmu, gpu, a5xx_fault_handler);
> + return aspace;
> +}
> +
> static const struct adreno_gpu_funcs funcs = {
> .base = {
> .get_param = adreno_get_param,
> @@ -1477,6 +1509,7 @@ static const struct adreno_gpu_funcs funcs = {
> .gpu_busy = a5xx_gpu_busy,
> .gpu_state_get = a5xx_gpu_state_get,
> .gpu_state_put = a5xx_gpu_state_put,
> + .create_address_space = a5xx_create_address_space,
> },
> .get_timestamp = a5xx_get_timestamp,
> };
> @@ -1523,7 +1556,6 @@ struct msm_gpu *a5xx_gpu_init(struct drm_device *dev)
>
> adreno_gpu->registers = a5xx_registers;
> adreno_gpu->reg_offsets = a5xx_register_offsets;
> -
> a5xx_gpu->lm_leakage = 0x4E001A;
>
> check_speed_bin(&pdev->dev);
> @@ -1534,9 +1566,6 @@ struct msm_gpu *a5xx_gpu_init(struct drm_device *dev)
> return ERR_PTR(ret);
> }
>
> - if (gpu->aspace)
> - msm_mmu_set_fault_handler(gpu->aspace->mmu, gpu, a5xx_fault_handler);
> -
> /* Set up the preemption specific bits and pieces for each ringbuffer */
> a5xx_preempt_init(gpu);
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index 1c20d59..f2e0800 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -783,6 +783,38 @@ static unsigned long a6xx_gpu_busy(struct msm_gpu *gpu)
> return (unsigned long)busy_time;
> }
>
> +static struct msm_gem_address_space *
> +a6xx_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)
> + return NULL;
> +
> + iommu->geometry.aperture_start = 0x100000000ULL;
> + iommu->geometry.aperture_end = 0x1ffffffffULL;
> +
> + 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);
> + }
> +
> + msm_mmu_set_fault_handler(aspace->mmu, gpu, a6xx_fault_handler);
> + return aspace;
> +}
> +
> static const struct adreno_gpu_funcs funcs = {
> .base = {
> .get_param = adreno_get_param,
> @@ -803,6 +835,7 @@ static const struct adreno_gpu_funcs funcs = {
> .gpu_set_freq = a6xx_gmu_set_freq,
> .gpu_state_get = a6xx_gpu_state_get,
> .gpu_state_put = a6xx_gpu_state_put,
> + .create_address_space = a6xx_create_address_space,
> },
> .get_timestamp = a6xx_get_timestamp,
> };
> @@ -845,9 +878,5 @@ struct msm_gpu *a6xx_gpu_init(struct drm_device *dev)
> return ERR_PTR(ret);
> }
>
> - if (gpu->aspace)
> - msm_mmu_set_fault_handler(gpu->aspace->mmu, gpu,
> - a6xx_fault_handler);
> -
> return gpu;
> }
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index 2cfee1a..dc9ea82 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -765,13 +765,6 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
> adreno_gpu->rev = config->rev;
>
> adreno_gpu_config.ioname = "kgsl_3d0_reg_memory";
> -
> - adreno_gpu_config.va_start = SZ_16M;
> - adreno_gpu_config.va_end = 0xffffffff;
> - /* maximum range of a2xx mmu */
> - if (adreno_is_a2xx(adreno_gpu))
> - adreno_gpu_config.va_end = SZ_16M + 0xfff * SZ_64K;
> -
> adreno_gpu_config.nr_rings = nr_rings;
>
> adreno_get_pwrlevels(&pdev->dev, gpu);
> diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h
> index 5e21d01..777f5fb 100644
> --- a/drivers/gpu/drm/msm/msm_gem.h
> +++ b/drivers/gpu/drm/msm/msm_gem.h
> @@ -21,6 +21,7 @@
> #include <linux/kref.h>
> #include <linux/reservation.h>
> #include "msm_drv.h"
> +#include "msm_mmu.h"
>
> /* Additional internal-use only BO flags: */
> #define MSM_BO_STOLEN 0x10000000 /* try to use stolen/splash memory */
> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> index 79b71b1..ec48bb3 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.c
> +++ b/drivers/gpu/drm/msm/msm_gpu.c
> @@ -20,7 +20,6 @@
> #include "msm_mmu.h"
> #include "msm_fence.h"
> #include "msm_gpu_trace.h"
> -#include "adreno/adreno_gpu.h"
>
> #include <generated/utsrelease.h>
> #include <linux/string_helpers.h>
> @@ -821,51 +820,6 @@ static int get_clocks(struct platform_device *pdev, struct msm_gpu *gpu)
> return 0;
> }
>
> -static struct msm_gem_address_space *
> -msm_gpu_create_address_space(struct msm_gpu *gpu, struct platform_device *pdev,
> - uint64_t va_start, uint64_t va_end)
> -{
> - struct msm_gem_address_space *aspace;
> - int ret;
> -
> - /*
> - * Setup IOMMU.. eventually we will (I think) do this once per context
> - * and have separate page tables per context. For now, to keep things
> - * simple and to get something working, just use a single address space:
> - */
> - if (!adreno_is_a2xx(to_adreno_gpu(gpu))) {
> - struct iommu_domain *iommu = iommu_domain_alloc(&platform_bus_type);
> - if (!iommu)
> - return NULL;
> -
> - iommu->geometry.aperture_start = va_start;
> - iommu->geometry.aperture_end = va_end;
> -
> - DRM_DEV_INFO(gpu->dev->dev, "%s: using IOMMU\n", gpu->name);
> -
> - aspace = msm_gem_address_space_create(&pdev->dev, iommu, "gpu");
> - if (IS_ERR(aspace))
> - iommu_domain_free(iommu);
> - } else {
> - aspace = msm_gem_address_space_create_a2xx(&pdev->dev, gpu, "gpu",
> - va_start, va_end);
> - }
> -
> - if (IS_ERR(aspace)) {
> - DRM_DEV_ERROR(gpu->dev->dev, "failed to init mmu: %ld\n",
> - PTR_ERR(aspace));
> - return ERR_CAST(aspace);
> - }
> -
> - ret = aspace->mmu->funcs->attach(aspace->mmu, NULL, 0);
> - if (ret) {
> - msm_gem_address_space_put(aspace);
> - return ERR_PTR(ret);
> - }
> -
> - return aspace;
> -}
> -
> int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev,
> struct msm_gpu *gpu, const struct msm_gpu_funcs *funcs,
> const char *name, struct msm_gpu_config *config)
> @@ -938,12 +892,8 @@ int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev,
>
> msm_devfreq_init(gpu);
>
> - gpu->aspace = msm_gpu_create_address_space(gpu, pdev,
> - config->va_start, config->va_end);
> -
> - if (gpu->aspace == NULL)
> - DRM_DEV_INFO(drm->dev, "%s: no IOMMU, fallback to VRAM carveout!\n", name);
> - else if (IS_ERR(gpu->aspace)) {
> + gpu->aspace = gpu->funcs->create_address_space(gpu);
> + if (IS_ERR(gpu->aspace)) {
> ret = PTR_ERR(gpu->aspace);
> goto fail;
> }
> diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
> index ca17086..81b9861 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.h
> +++ b/drivers/gpu/drm/msm/msm_gpu.h
> @@ -74,6 +74,8 @@ struct msm_gpu_funcs {
> int (*gpu_state_put)(struct msm_gpu_state *state);
> unsigned long (*gpu_get_freq)(struct msm_gpu *gpu);
> void (*gpu_set_freq)(struct msm_gpu *gpu, unsigned long freq);
> + struct msm_gem_address_space *(*create_address_space)
> + (struct msm_gpu *gpu);
> };
>
> struct msm_gpu {
>
More information about the dri-devel
mailing list