This patch series fixes usage of the etnaviv driver with GPUs behind a
IOMMU. It was tested on a NXP LS1028A SoC. Together with Lucas' MMU patches
[1] there are not more (GPU internal) MMU nor (system) IOMMU faults on the
LS1028A.
[1] https://lists.freedesktop.org/archives/etnaviv/2021-August/003682.html
Michael Walle (3):
drm/etnaviv: use PLATFORM_DEVID_NONE
drm/etnaviv: fix dma configuration of the virtual device
drm/etnaviv: use a 32 bit mask as coherent DMA mask
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 41 ++++++++++++++++++++-------
1 file changed, 31 insertions(+), 10 deletions(-)
--
2.30.2
There is already a macro for the magic value. Use it.
Signed-off-by: Michael Walle <[email protected]>
---
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 7dcc6392792d..2509b3e85709 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -653,7 +653,7 @@ static int __init etnaviv_init(void)
if (!of_device_is_available(np))
continue;
- pdev = platform_device_alloc("etnaviv", -1);
+ pdev = platform_device_alloc("etnaviv", PLATFORM_DEVID_NONE);
if (!pdev) {
ret = -ENOMEM;
of_node_put(np);
--
2.30.2
The STLB and the first command buffer (which is used to set up the TLBs)
has a 32 bit size restriction in hardware. There seems to be no way to
specify addresses larger than 32 bit. Keep it simple and restict the
addresses to the lower 4 GiB range for all coherent DMA memory
allocations.
Signed-off-by: Michael Walle <[email protected]>
---
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index ff6425f6ebad..0b756ecb1bc2 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -613,8 +613,23 @@ static int etnaviv_pdev_probe(struct platform_device *pdev)
component_match_add(dev, &match, compare_str, names[i]);
}
- pdev->dev.coherent_dma_mask = DMA_BIT_MASK(40);
- pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
+ /*
+ * PTA and MTLB can have 40 bit base addresses, but
+ * unfortunately, an entry in the MTLB can only point to a
+ * 32 bit base address of a STLB. Moreover, to initialize the
+ * MMU we need a command buffer with a 32 bit address because
+ * without an MMU there is only an indentity mapping between
+ * the internal 32 bit addresses and the bus addresses.
+ *
+ * To make things easy, we set the dma_coherent_mask to 32
+ * bit to make sure we are allocating the command buffers and
+ * TLBs in the lower 4 GiB address space.
+ */
+ if (dma_set_mask(&pdev->dev, DMA_BIT_MASK(40)) ||
+ dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32))) {
+ dev_dbg(&pdev->dev, "No suitable DMA available\n");
+ return -ENODEV;
+ }
/*
* Apply the same DMA configuration to the virtual etnaviv
--
2.30.2
On Thu, Aug 26, 2021 at 02:10:06PM +0200, Michael Walle wrote:
> - pdev->dev.coherent_dma_mask = DMA_BIT_MASK(40);
> - pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
> + /*
> + * PTA and MTLB can have 40 bit base addresses, but
> + * unfortunately, an entry in the MTLB can only point to a
> + * 32 bit base address of a STLB. Moreover, to initialize the
> + * MMU we need a command buffer with a 32 bit address because
> + * without an MMU there is only an indentity mapping between
> + * the internal 32 bit addresses and the bus addresses.
> + *
> + * To make things easy, we set the dma_coherent_mask to 32
> + * bit to make sure we are allocating the command buffers and
> + * TLBs in the lower 4 GiB address space.
> + */
> + if (dma_set_mask(&pdev->dev, DMA_BIT_MASK(40)) ||
> + dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32))) {
> + dev_dbg(&pdev->dev, "No suitable DMA available\n");
> + return -ENODEV;
> + }
This makes no sense. In the previous patch, you initialised
pdev->dev.dma_mask ot point at the coherent mask, implying that
it wasn't already set - for which dma_coerce_mask_and_coherent()
should be used. Now you're just calling dma_set_mask(), which will
fail if pdev->dev.dma_mask hasn't already been set to point at
something.
If it's already been initialised to point at something, then you
shouldn't be overwriting it in the driver, and you should've used
dma_set_mask_and_coherent() in your previous patch.
Confused.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Am 2021-08-26 14:19, schrieb Russell King (Oracle):
> On Thu, Aug 26, 2021 at 02:10:06PM +0200, Michael Walle wrote:
>> - pdev->dev.coherent_dma_mask = DMA_BIT_MASK(40);
>> - pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
>> + /*
>> + * PTA and MTLB can have 40 bit base addresses, but
>> + * unfortunately, an entry in the MTLB can only point to a
>> + * 32 bit base address of a STLB. Moreover, to initialize the
>> + * MMU we need a command buffer with a 32 bit address because
>> + * without an MMU there is only an indentity mapping between
>> + * the internal 32 bit addresses and the bus addresses.
>> + *
>> + * To make things easy, we set the dma_coherent_mask to 32
>> + * bit to make sure we are allocating the command buffers and
>> + * TLBs in the lower 4 GiB address space.
>> + */
>> + if (dma_set_mask(&pdev->dev, DMA_BIT_MASK(40)) ||
>> + dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32))) {
>> + dev_dbg(&pdev->dev, "No suitable DMA available\n");
>> + return -ENODEV;
>> + }
>
> This makes no sense. In the previous patch, you initialised
> pdev->dev.dma_mask ot point at the coherent mask, implying that
> it wasn't already set - for which dma_coerce_mask_and_coherent()
> should be used. Now you're just calling dma_set_mask(), which will
> fail if pdev->dev.dma_mask hasn't already been set to point at
> something.
>
> If it's already been initialised to point at something, then you
> shouldn't be overwriting it in the driver, and you should've used
> dma_set_mask_and_coherent() in your previous patch.
>
> Confused.
Mh, I see that moving these two lines was a bad idea. See commit
message in patch 2/3:
> Also move the dma mask assignemnts to probe() to keep all DMA related
> settings together.
The actual fix in patch 2/3 is the move of the of_dma_configure()
not the dma_mask assignments.
-michael
Am 2021-08-26 14:10, schrieb Michael Walle:
> The STLB and the first command buffer (which is used to set up the
> TLBs)
> has a 32 bit size restriction in hardware. There seems to be no way to
> specify addresses larger than 32 bit. Keep it simple and restict the
> addresses to the lower 4 GiB range for all coherent DMA memory
> allocations.
>
> Signed-off-by: Michael Walle <[email protected]>
Suggested-by: Lucas Stach <[email protected]>
is missing here. sorry, will add it in the next version.
-michael
Am Do., 26. Aug. 2021 um 14:10 Uhr schrieb Michael Walle <[email protected]>:
>
> There is already a macro for the magic value. Use it.
>
> Signed-off-by: Michael Walle <[email protected]>
Reviewed-by: Christian Gmeiner <[email protected]>
I will wait for v2 for the rest of the changes to review.
> ---
> drivers/gpu/drm/etnaviv/etnaviv_drv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index 7dcc6392792d..2509b3e85709 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -653,7 +653,7 @@ static int __init etnaviv_init(void)
> if (!of_device_is_available(np))
> continue;
>
> - pdev = platform_device_alloc("etnaviv", -1);
> + pdev = platform_device_alloc("etnaviv", PLATFORM_DEVID_NONE);
> if (!pdev) {
> ret = -ENOMEM;
> of_node_put(np);
> --
> 2.30.2
>
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy