[PATCH 1/2] drm/msm: gpu: call qcom_mdt interfaces only for ARCH_QCOM

[PATCH 1/2] drm/msm: gpu: call qcom_mdt interfaces only for ARCH_QCOM

[Arnd Bergmann]

When compile-testing for something other than ARCH_QCOM,
we run into a link error:

drivers/gpu/drm/msm/adreno/a5xx_gpu.o: In function `a5xx_hw_init':
a5xx_gpu.c:(.text.a5xx_hw_init+0x600): undefined reference to `qcom_mdt_get_size'
a5xx_gpu.c:(.text.a5xx_hw_init+0x93c): undefined reference to `qcom_mdt_load'

There is already an #ifdef that tries to check for CONFIG_QCOM_MDT_LOADER,
but that symbol is only meaningful when building for ARCH_QCOM.

This adds a compile-time check for ARCH_QCOM, and clarifies the
Kconfig select statement so we don't even try it for other targets.

The check for CONFIG_QCOM_MDT_LOADER can then go away, which also
improves compile-time coverage and makes the code a little nicer
to read.

Fixes: 7c65817e6d38 ("drm/msm: gpu: Enable zap shader for A5XX")
Acked-by: Jordan Crouse <jcrouse@codeaurora.org>
Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/gpu/drm/msm/Kconfig           |  2 +-
 drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 11 +++--------
 2 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index b638d192ce5e..99d39b2aefa6 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -5,7 +5,7 @@ config DRM_MSM
 	depends on ARCH_QCOM || (ARM && COMPILE_TEST)
 	depends on OF && COMMON_CLK
 	depends on MMU
-	select QCOM_MDT_LOADER
+	select QCOM_MDT_LOADER if ARCH_QCOM
 	select REGULATOR
 	select DRM_KMS_HELPER
 	select DRM_PANEL
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index b4b54f1c24bc..1d54c76a7778 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -26,8 +26,6 @@ static void a5xx_dump(struct msm_gpu *gpu);
 
 #define GPU_PAS_ID 13
 
-#if IS_ENABLED(CONFIG_QCOM_MDT_LOADER)
-
 static int zap_shader_load_mdt(struct device *dev, const char *fwname)
 {
 	const struct firmware *fw;
@@ -36,6 +34,9 @@ static int zap_shader_load_mdt(struct device *dev, const char *fwname)
 	void *mem_region = NULL;
 	int ret;
 
+	if (!IS_ENABLED(CONFIG_ARCH_QCOM))
+		return -EINVAL;
+
 	/* Request the MDT file for the firmware */
 	ret = request_firmware(&fw, fwname, dev);
 	if (ret) {
@@ -73,12 +74,6 @@ static int zap_shader_load_mdt(struct device *dev, const char *fwname)
 
 	return ret;
 }
-#else
-static int zap_shader_load_mdt(struct device *dev, const char *fwname)
-{
-	return -ENODEV;
-}
-#endif
 
 static void a5xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit,
 	struct msm_file_private *ctx)
-- 
2.9.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/2] drm/msm: gpu: call qcom_mdt interfaces only for ARCH_QCOM

[Arnd Bergmann]

When compile-testing for something other than ARCH_QCOM,
we run into a link error:

drivers/gpu/drm/msm/adreno/a5xx_gpu.o: In function `a5xx_hw_init':
a5xx_gpu.c:(.text.a5xx_hw_init+0x600): undefined reference to `qcom_mdt_get_size'
a5xx_gpu.c:(.text.a5xx_hw_init+0x93c): undefined reference to `qcom_mdt_load'

There is already an #ifdef that tries to check for CONFIG_QCOM_MDT_LOADER,
but that symbol is only meaningful when building for ARCH_QCOM.

This adds a compile-time check for ARCH_QCOM, and clarifies the
Kconfig select statement so we don't even try it for other targets.

The check for CONFIG_QCOM_MDT_LOADER can then go away, which also
improves compile-time coverage and makes the code a little nicer
to read.

Fixes: 7c65817e6d38 ("drm/msm: gpu: Enable zap shader for A5XX")
Acked-by: Jordan Crouse <jcrouse@codeaurora.org>
Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/gpu/drm/msm/Kconfig           |  2 +-
 drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 11 +++--------
 2 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index b638d192ce5e..99d39b2aefa6 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -5,7 +5,7 @@ config DRM_MSM
 	depends on ARCH_QCOM || (ARM && COMPILE_TEST)
 	depends on OF && COMMON_CLK
 	depends on MMU
-	select QCOM_MDT_LOADER
+	select QCOM_MDT_LOADER if ARCH_QCOM
 	select REGULATOR
 	select DRM_KMS_HELPER
 	select DRM_PANEL
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index b4b54f1c24bc..1d54c76a7778 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -26,8 +26,6 @@ static void a5xx_dump(struct msm_gpu *gpu);
 
 #define GPU_PAS_ID 13
 
-#if IS_ENABLED(CONFIG_QCOM_MDT_LOADER)
-
 static int zap_shader_load_mdt(struct device *dev, const char *fwname)
 {
 	const struct firmware *fw;
@@ -36,6 +34,9 @@ static int zap_shader_load_mdt(struct device *dev, const char *fwname)
 	void *mem_region = NULL;
 	int ret;
 
+	if (!IS_ENABLED(CONFIG_ARCH_QCOM))
+		return -EINVAL;
+
 	/* Request the MDT file for the firmware */
 	ret = request_firmware(&fw, fwname, dev);
 	if (ret) {
@@ -73,12 +74,6 @@ static int zap_shader_load_mdt(struct device *dev, const char *fwname)
 
 	return ret;
 }
-#else
-static int zap_shader_load_mdt(struct device *dev, const char *fwname)
-{
-	return -ENODEV;
-}
-#endif
 
 static void a5xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit,
 	struct msm_file_private *ctx)
-- 
2.9.0

[PATCH 2/2] drm/msm: gpu: don't abuse dma_alloc for non-DMA allocations

[Arnd Bergmann]

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")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
I think we want this to be applied for 4.13, as the upstream
code that was added in the merge window is seriously broken without it
---
 drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 71 ++++++++++++-----------------------
 drivers/gpu/drm/msm/adreno/a5xx_gpu.h |  2 -
 2 files changed, 23 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index 1d54c76a7778..ce545b3a9d17 100644
--- 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(dev->of_node, "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;
@@ -373,44 +393,6 @@ static int a5xx_zap_shader_resume(struct msm_gpu *gpu)
 }
 
 /* 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;
@@ -439,11 +421,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;
 
@@ -686,9 +664,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 6638bc85645d..6b20f28c75a0 100644
--- 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)
-- 
2.9.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/2] drm/msm: gpu: don't abuse dma_alloc for non-DMA allocations

[Arnd Bergmann]

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")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
I think we want this to be applied for 4.13, as the upstream
code that was added in the merge window is seriously broken without it
---
 drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 71 ++++++++++++-----------------------
 drivers/gpu/drm/msm/adreno/a5xx_gpu.h |  2 -
 2 files changed, 23 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index 1d54c76a7778..ce545b3a9d17 100644
--- 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(dev->of_node, "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;
@@ -373,44 +393,6 @@ static int a5xx_zap_shader_resume(struct msm_gpu *gpu)
 }
 
 /* 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;
@@ -439,11 +421,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;
 
@@ -686,9 +664,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 6638bc85645d..6b20f28c75a0 100644
--- 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)
-- 
2.9.0

Re: [PATCH 2/2] drm/msm: gpu: don't abuse dma_alloc for non-DMA allocations

[Jordan Crouse]

On Wed, Jul 26, 2017 at 05:52:45PM +0200, Arnd Bergmann wrote:
> 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")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> I think we want this to be applied for 4.13, as the upstream
> code that was added in the merge window is seriously broken without it
> ---
>  drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 71 ++++++++++++-----------------------
>  drivers/gpu/drm/msm/adreno/a5xx_gpu.h |  2 -
>  2 files changed, 23 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> index 1d54c76a7778..ce545b3a9d17 100644
> --- 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(dev->of_node, "memory-region", 0);

I think this should be np = of_parse_phandle(np, "memory-region", 0);

With that change this patch works great.

> @@ -373,44 +393,6 @@ static int a5xx_zap_shader_resume(struct msm_gpu *gpu)
>  }
>  
>  /* Set up a child device to "own" the zap shader */

This now incorrect comment can be zapped (pun intended).

> -static int a5xx_zap_shader_dev_init(struct device *parent, struct device *dev)
> -{
> -	struct device_node *node;
> -	int ret;
> -
> -	if (dev->parent)
> -		return 0;
> -

With above changes,
Acked-and-Tested-By: Jordan Crouse <jcrouse@codeaurora.org>

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 2/2] drm/msm: gpu: don't abuse dma_alloc for non-DMA allocations

[Arnd Bergmann]

 On Jul 26, 2017 6:35 PM, "Jordan Crouse" <jcrouse@codeaurora.org> wrote:

> >       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(dev->of_node, "memory-region", 0);
>
> I think this should be np = of_parse_phandle(np, "memory-region", 0);
>

Ok, fixed.

> > @@ -373,44 +393,6 @@ static int a5xx_zap_shader_resume(struct msm_gpu *gpu)
> >  }
> >
> >  /* Set up a child device to "own" the zap shader */
>
> This now incorrect comment can be zapped (pun intended).

Done.

> > -static int a5xx_zap_shader_dev_init(struct device *parent, struct device *dev)
> > -{
> > -     struct device_node *node;
> > -     int ret;
> > -
> > -     if (dev->parent)
> > -             return 0;
> > -
>
> With above changes,
> Acked-and-Tested-By: Jordan Crouse <jcrouse@codeaurora.org>

Thanks for testing!

I've re-sent this patch as v3 now (this one was actually v2, depending on
how you count). I decided not to resend patch 1/2 of the series. We still
need that too, but they are independent. Hope that's ok.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/msm: gpu: don't abuse dma_alloc for non-DMA allocations

[Arnd Bergmann]

 On Jul 26, 2017 6:35 PM, "Jordan Crouse" <jcrouse@codeaurora.org> wrote:

> >       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(dev->of_node, "memory-region", 0);
>
> I think this should be np = of_parse_phandle(np, "memory-region", 0);
>

Ok, fixed.

> > @@ -373,44 +393,6 @@ static int a5xx_zap_shader_resume(struct msm_gpu *gpu)
> >  }
> >
> >  /* Set up a child device to "own" the zap shader */
>
> This now incorrect comment can be zapped (pun intended).

Done.

> > -static int a5xx_zap_shader_dev_init(struct device *parent, struct device *dev)
> > -{
> > -     struct device_node *node;
> > -     int ret;
> > -
> > -     if (dev->parent)
> > -             return 0;
> > -
>
> With above changes,
> Acked-and-Tested-By: Jordan Crouse <jcrouse@codeaurora.org>

Thanks for testing!

I've re-sent this patch as v3 now (this one was actually v2, depending on
how you count). I decided not to resend patch 1/2 of the series. We still
need that too, but they are independent. Hope that's ok.