diff mbox series

[v2] of/platform: initialise AMBA default DMA masks

Message ID 20180831082614.27455-1-linus.walleij@linaro.org (mailing list archive)
State New, archived
Headers show
Series [v2] of/platform: initialise AMBA default DMA masks | expand

Commit Message

Linus Walleij Aug. 31, 2018, 8:26 a.m. UTC
This addresses a v4.19-rc1 regression in the PL111 DRM driver
in drivers/gpu/pl111/*

The driver uses the CMA KMS helpers and will thus at some
point call down to dma_alloc_attrs() to allocate a chunk
of contigous DMA memory for the framebuffer.

It appears that in v4.18, it was OK that this (and other
DMA mastering AMBA devices) left dev->coherent_dma_mask
blank (zero).

In v4.19-rc1 the WARN_ON_ONCE(dev && !dev->coherent_dma_mask)
in dma_alloc_attrs() in include/linux/dma-mapping.h is
triggered. The allocation later fails when get_coherent_dma_mask()
is called from __dma_alloc() and __dma_alloc() returns
NULL:

drm-clcd-pl111 dev:20: coherent DMA mask is unset
drm-clcd-pl111 dev:20: [drm:drm_fb_helper_fbdev_setup] *ERROR*
 	       	        Failed to set fbdev configuration

I have not been able to properly bisect down to the actual
committ causing it beacuse the kernel simply fails to build
at to many bisection points, pushing the bisect back to places
like the merge of entire subsystem trees, where things have
likely been resolved while merging them.

I found that Robin Murphy solved a similar problem in
a5516219b102 ("of/platform: Initialise default DMA masks")
by simply assigning dev.coherent_dma_mask and the
dev.dma_mask to point to the same when creating devices
from the device tree, and introducing the same code into
the code path creating AMBA/PrimeCell devices solved my
problem, graphics now come up.

The code simply assumes that the device can access all
of the system memory by setting the coherent DMA mask
to 0xffffffff when creating a device from the device
tree, which is crude, but seems to be what kernel v4.18
assumed.

Cc: Russell King <linux@armlinux.org.uk>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
Russell, Christoph: this is probably not the last patch.
But it has a more accurate description of the problem.
I still do not know what change to the kernel made
it start triggering this, whether in the DMA API or
in DRM :(

I am looking into Russell's suggestion to use the DMA
ranges, and thusly patch all device trees with DMA
mastering devices adding DMA ranges to them and parsing
that in the device tree AMBA/PrimeCell population code.
Possibly that is the right path forward.
---
 drivers/of/platform.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Russell King (Oracle) Aug. 31, 2018, 10:09 a.m. UTC | #1
On Fri, Aug 31, 2018 at 10:26:14AM +0200, Linus Walleij wrote:
> This addresses a v4.19-rc1 regression in the PL111 DRM driver
> in drivers/gpu/pl111/*
> 
> The driver uses the CMA KMS helpers and will thus at some
> point call down to dma_alloc_attrs() to allocate a chunk
> of contigous DMA memory for the framebuffer.
> 
> It appears that in v4.18, it was OK that this (and other
> DMA mastering AMBA devices) left dev->coherent_dma_mask
> blank (zero).
> 
> In v4.19-rc1 the WARN_ON_ONCE(dev && !dev->coherent_dma_mask)
> in dma_alloc_attrs() in include/linux/dma-mapping.h is
> triggered. The allocation later fails when get_coherent_dma_mask()
> is called from __dma_alloc() and __dma_alloc() returns
> NULL:
> 
> drm-clcd-pl111 dev:20: coherent DMA mask is unset
> drm-clcd-pl111 dev:20: [drm:drm_fb_helper_fbdev_setup] *ERROR*
>  	       	        Failed to set fbdev configuration
> 
> I have not been able to properly bisect down to the actual
> committ causing it beacuse the kernel simply fails to build
> at to many bisection points, pushing the bisect back to places
> like the merge of entire subsystem trees, where things have
> likely been resolved while merging them.
> 
> I found that Robin Murphy solved a similar problem in
> a5516219b102 ("of/platform: Initialise default DMA masks")
> by simply assigning dev.coherent_dma_mask and the
> dev.dma_mask to point to the same when creating devices
> from the device tree, and introducing the same code into
> the code path creating AMBA/PrimeCell devices solved my
> problem, graphics now come up.
> 
> The code simply assumes that the device can access all
> of the system memory by setting the coherent DMA mask
> to 0xffffffff when creating a device from the device
> tree, which is crude, but seems to be what kernel v4.18
> assumed.
> 
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> Russell, Christoph: this is probably not the last patch.
> But it has a more accurate description of the problem.
> I still do not know what change to the kernel made
> it start triggering this, whether in the DMA API or
> in DRM :(

Doing some research, it seems the warning was added in v4.16-rc1, and
it is only a warning - it doesn't otherwise change the behaviour, so
it's not the actual warning that's the problem.

I can't see anything in the DMA API nor DRM which would cause this
either, but there are changes in the DT code.

However, there are changes to of_dma_configure() which will be called
just before the driver's probe function is called - and I guess the
culpret is 4d8bde883bfb ("OF: Don't set default coherent DMA mask").
So I guess that's the root cause of the problem, and indeed this
change was made in 4.19-rc1.
diff mbox series

Patch

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 7ba90c290a42..6c59673933e9 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -241,6 +241,10 @@  static struct amba_device *of_amba_device_create(struct device_node *node,
 	if (!dev)
 		goto err_clear_flag;
 
+	/* AMBA devices only support a single DMA mask */
+	dev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
+	dev->dev.dma_mask = &dev->dev.coherent_dma_mask;
+
 	/* setup generic device info */
 	dev->dev.of_node = of_node_get(node);
 	dev->dev.fwnode = &node->fwnode;