drm/msm: gpu: don't abuse dma_alloc for non-DMA allocations

In zap_shader_load_mdt(), we pass a pointer to a phys_addr_t
into dmam_alloc_coherent, which the compiler warns about:

drivers/gpu/drm/msm/adreno/a5xx_gpu.c: In function 'zap_shader_load_mdt':
drivers/gpu/drm/msm/adreno/a5xx_gpu.c:54:50: error: passing argument 3 of 'dmam_alloc_coherent' from incompatible pointer type [-Werror=incompatible-pointer-types]

The returned DMA address is later passed on to a function that
takes a phys_addr_t, so it's clearly wrong to use the DMA
mapping interface here: the memory may be uncached, or the
address may be completely wrong if there is an IOMMU connected
to the device. What the code actually wants to do is to get
the physical address from the reserved-mem node. It goes through
the dma-mapping interfaces for obscure reasons, and this
apparently only works by chance, relying on specific bugs
in the error handling of the arm64 dma-mapping implementation.

The same problem existed in the "venus" media driver, which was
now fixed by Stanimir Varbanov after long discussions.

In order to make some progress here, I have now ported his
approach over to the adreno driver. The patch is currently
untested, and should get a good review, but it is now much
simpler than the original, and it should be obvious what
goes wrong if I made a mistake in the port.

See also: a6e2d36bf6b7 ("media: venus: don't abuse dma_alloc for non-DMA allocations")
Cc: Stanimir Varbanov <stanimir.varbanov@linaro.org>
Fixes: 7c65817e6d38 ("drm/msm: gpu: Enable zap shader for A5XX")
Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Acked-and-Tested-by: Jordan Crouse <jcrouse@codeaurora.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Rob Clark <robdclark@gmail.com>

diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index a3393e1dc4976433de7000dfdf029c22393e5199..f9eae03aa1dcaef072974d60216fb6b09ef81e66 100644 (file)
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -15,7 +15,7 @@
 #include <linux/cpumask.h>
 #include <linux/qcom_scm.h>
 #include <linux/dma-mapping.h>
-#include <linux/of_reserved_mem.h>
+#include <linux/of_address.h>
 #include <linux/soc/qcom/mdt_loader.h>
 #include "msm_gem.h"
 #include "msm_mmu.h"
@@ -29,6 +29,8 @@ static void a5xx_dump(struct msm_gpu *gpu);
 static int zap_shader_load_mdt(struct device *dev, const char *fwname)
 {
        const struct firmware *fw;
+       struct device_node *np;
+       struct resource r;
        phys_addr_t mem_phys;
        ssize_t mem_size;
        void *mem_region = NULL;
@@ -37,6 +39,21 @@ static int zap_shader_load_mdt(struct device *dev, const char *fwname)
        if (!IS_ENABLED(CONFIG_ARCH_QCOM))
                return -EINVAL;
 
+       np = of_get_child_by_name(dev->of_node, "zap-shader");
+       if (!np)
+               return -ENODEV;
+
+       np = of_parse_phandle(np, "memory-region", 0);
+       if (!np)
+               return -EINVAL;
+
+       ret = of_address_to_resource(np, 0, &r);
+       if (ret)
+               return ret;
+
+       mem_phys = r.start;
+       mem_size = resource_size(&r);
+
        /* Request the MDT file for the firmware */
        ret = request_firmware(&fw, fwname, dev);
        if (ret) {
@@ -52,7 +69,7 @@ static int zap_shader_load_mdt(struct device *dev, const char *fwname)
        }
 
        /* Allocate memory for the firmware image */
-       mem_region = dmam_alloc_coherent(dev, mem_size, &mem_phys, GFP_KERNEL);
+       mem_region = memremap(mem_phys, mem_size,  MEMREMAP_WC);
        if (!mem_region) {
                ret = -ENOMEM;
                goto out;
@@ -70,6 +87,9 @@ static int zap_shader_load_mdt(struct device *dev, const char *fwname)
                DRM_DEV_ERROR(dev, "Unable to authorize the image\n");
 
 out:
+       if (mem_region)
+               memunmap(mem_region);
+
        release_firmware(fw);
 
        return ret;
@@ -348,45 +368,6 @@ static int a5xx_zap_shader_resume(struct msm_gpu *gpu)
        return ret;
 }
 
-/* Set up a child device to "own" the zap shader */
-static int a5xx_zap_shader_dev_init(struct device *parent, struct device *dev)
-{
-       struct device_node *node;
-       int ret;
-
-       if (dev->parent)
-               return 0;
-
-       /* Find the sub-node for the zap shader */
-       node = of_get_child_by_name(parent->of_node, "zap-shader");
-       if (!node) {
-               DRM_DEV_ERROR(parent, "zap-shader not found in device tree\n");
-               return -ENODEV;
-       }
-
-       dev->parent = parent;
-       dev->of_node = node;
-       dev_set_name(dev, "adreno_zap_shader");
-
-       ret = device_register(dev);
-       if (ret) {
-               DRM_DEV_ERROR(parent, "Couldn't register zap shader device\n");
-               goto out;
-       }
-
-       ret = of_reserved_mem_device_init(dev);
-       if (ret) {
-               DRM_DEV_ERROR(parent, "Unable to set up the reserved memory\n");
-               device_unregister(dev);
-       }
-
-out:
-       if (ret)
-               dev->parent = NULL;
-
-       return ret;
-}
-
 static int a5xx_zap_shader_init(struct msm_gpu *gpu)
 {
        static bool loaded;
@@ -415,11 +396,7 @@ static int a5xx_zap_shader_init(struct msm_gpu *gpu)
                return -ENODEV;
        }
 
-       ret = a5xx_zap_shader_dev_init(&pdev->dev, &a5xx_gpu->zap_dev);
-
-       if (!ret)
-               ret = zap_shader_load_mdt(&a5xx_gpu->zap_dev,
-                       adreno_gpu->info->zapfw);
+       ret = zap_shader_load_mdt(&pdev->dev, adreno_gpu->info->zapfw);
 
        loaded = !ret;
 
@@ -662,9 +639,6 @@ static void a5xx_destroy(struct msm_gpu *gpu)
 
        DBG("%s", gpu->name);
 
-       if (a5xx_gpu->zap_dev.parent)
-               device_unregister(&a5xx_gpu->zap_dev);
-
        if (a5xx_gpu->pm4_bo) {
                if (a5xx_gpu->pm4_iova)
                        msm_gem_put_iova(a5xx_gpu->pm4_bo, gpu->aspace);

diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.h b/drivers/gpu/drm/msm/adreno/a5xx_gpu.h
index d24796f3a706417aaa433acd19fba72bebbab736..1137092241d593c34e4607e3c723acfb74861972 100644 (file)
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.h
@@ -36,8 +36,6 @@ struct a5xx_gpu {
        uint32_t gpmu_dwords;
 
        uint32_t lm_leakage;
-
-       struct device zap_dev;
 };
 
 #define to_a5xx_gpu(x) container_of(x, struct a5xx_gpu, base)