[PATCH] drm/vc4: Set DRM DMA device directly from HVS and V3D

[PATCH] drm/vc4: Set DRM DMA device directly from HVS and V3D

[Daniel Drake]

vc4 uses of_dma_configure() during bind to set the DMA configuration of
the drm device by borrowing the configuration of one of its component
devices.

This violates driver model expectations that IOMMU probing and domain
attachment occur before driver binding - see commit bcb81ac6ae3c ("iommu:
Get DT/ACPI parsing into the proper probe path").

With the introduction of the bcm2712-iommu driver behind the vc4 hvs
device, a warning is triggered:

    vc4-drm gpu: late IOMMU probe at driver bind, something fishy here!

Instead of configuring the virtual aggregate device, adopt the pattern
used by sun4i/sun8i/exynos where candidate DMA-capable hardware components
register themselves as the device to use for DRM allocations via
drm_dev_set_dma_dev(), provided a DMA device has not already been
assigned.

HVS will typically bind first and claim the DMA device. The gen6 36-bit
DMA mask configuration was moved into vc4_hvs_bind() accordingly. For
older generations, vc4's v3d component continues to be available as a
fallback, and the default platform bus 32-bit DMA mask is retained.

Fixes: da8e393e23ef ("drm/vc4: drv: Adopt the dma configuration from the HVS or V3D component")
Signed-off-by: Daniel Drake <dan@reactivated.net>
---
 drivers/gpu/drm/vc4/vc4_drv.c | 27 ---------------------------
 drivers/gpu/drm/vc4/vc4_hvs.c | 14 ++++++++++++++
 drivers/gpu/drm/vc4/vc4_v3d.c |  8 ++++++++
 3 files changed, 22 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index 616caf9d9915..62f1a5731e33 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -273,16 +273,6 @@ static void vc4_component_unbind_all(void *ptr)
 	component_unbind_all(vc4->dev, &vc4->base);
 }
 
-static const struct of_device_id vc4_dma_range_matches[] = {
-	{ .compatible = "brcm,bcm2711-hvs" },
-	{ .compatible = "brcm,bcm2712-hvs" },
-	{ .compatible = "brcm,bcm2835-hvs" },
-	{ .compatible = "brcm,bcm2835-v3d" },
-	{ .compatible = "brcm,cygnus-v3d" },
-	{ .compatible = "brcm,vc4-v3d" },
-	{}
-};
-
 static int vc4_drm_bind(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
@@ -295,8 +285,6 @@ static int vc4_drm_bind(struct device *dev)
 	enum vc4_gen gen;
 	int ret = 0;
 
-	dev->coherent_dma_mask = DMA_BIT_MASK(32);
-
 	gen = (enum vc4_gen)of_device_get_match_data(dev);
 
 	if (gen > VC4_GEN_4)
@@ -304,21 +292,6 @@ static int vc4_drm_bind(struct device *dev)
 	else
 		driver = &vc4_drm_driver;
 
-	if (gen >= VC4_GEN_6_C)
-		dma_set_mask_and_coherent(dev, DMA_BIT_MASK(36));
-	else
-		dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
-
-	node = of_find_matching_node_and_match(NULL, vc4_dma_range_matches,
-					       NULL);
-	if (node) {
-		ret = of_dma_configure(dev, node, true);
-		of_node_put(node);
-
-		if (ret)
-			return ret;
-	}
-
 	vc4 = devm_drm_dev_alloc(dev, driver, struct vc4_dev, base);
 	if (IS_ERR(vc4))
 		return PTR_ERR(vc4);
diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
index e715147d091c..305770c87dcf 100644
--- a/drivers/gpu/drm/vc4/vc4_hvs.c
+++ b/drivers/gpu/drm/vc4/vc4_hvs.c
@@ -22,6 +22,7 @@
 #include <linux/bitfield.h>
 #include <linux/clk.h>
 #include <linux/component.h>
+#include <linux/dma-mapping.h>
 #include <linux/platform_device.h>
 
 #include <drm/drm_atomic_helper.h>
@@ -1662,6 +1663,19 @@ static int vc4_hvs_bind(struct device *dev, struct device *master, void *data)
 		hvs->regset.nregs = ARRAY_SIZE(vc4_hvs_regs);
 	}
 
+	if (vc4->gen >= VC4_GEN_6_C) {
+		ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(36));
+		if (ret)
+			return ret;
+	}
+
+	/*
+	 * Use the HVS as the DRM device's DMA controller for buffer
+	 * allocations if one has not already been configured.
+	 */
+	if (drm_dev_dma_dev(drm) == drm->dev)
+		drm_dev_set_dma_dev(drm, dev);
+
 	if (vc4->gen >= VC4_GEN_5) {
 		struct rpi_firmware *firmware;
 		struct device_node *node;
diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c
index f32410420d3e..1f76d4850122 100644
--- a/drivers/gpu/drm/vc4/vc4_v3d.c
+++ b/drivers/gpu/drm/vc4/vc4_v3d.c
@@ -10,6 +10,7 @@
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 
+#include <drm/drm_drv.h>
 #include <drm/drm_print.h>
 
 #include "vc4_drv.h"
@@ -443,6 +444,13 @@ static int vc4_v3d_bind(struct device *dev, struct device *master, void *data)
 	vc4->v3d = v3d;
 	v3d->vc4 = vc4;
 
+	/*
+	 * In headless setups where HVS is absent, use V3D as the fallback
+	 * DMA controller for buffer allocations.
+	 */
+	if (drm_dev_dma_dev(drm) == drm->dev)
+		drm_dev_set_dma_dev(drm, dev);
+
 	v3d->clk = devm_clk_get_optional(dev, NULL);
 	if (IS_ERR(v3d->clk))
 		return dev_err_probe(dev, PTR_ERR(v3d->clk), "Failed to get V3D clock\n");

---
base-commit: 16130ac7861fbbaef5564463a9f6be230e00fb24
change-id: 20260927-drm-dma-device-2-7cb00af72069

Best regards,
-- 
Daniel Drake <dan@reactivated.net>

Re: [PATCH] drm/vc4: Set DRM DMA device directly from HVS and V3D

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/vc4: dma_free_wc() used with wrong device in vc4_bo_purge()
--

commit abd61016c96bce404abb5e31405d8c24a7acfb12
Author: Daniel Drake <dan@reactivated.net>

    drm/vc4: Set DRM DMA device directly from HVS and V3D
    
    vc4 uses of_dma_configure() during bind to set the DMA configuration of
    the drm device by borrowing the configuration of one of its component
    devices.

> diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
> index e715147d091ca..305770c87dcf0 100644
> --- a/drivers/gpu/drm/vc4/vc4_hvs.c
> +++ b/drivers/gpu/drm/vc4/vc4_hvs.c
[ ... ]
> @@ -1662,6 +1663,19 @@ static int vc4_hvs_bind(struct device *dev, struct device *master, void *data)
>  		hvs->regset.nregs = ARRAY_SIZE(vc4_hvs_regs);
>  	}
>  
> +	if (vc4->gen >= VC4_GEN_6_C) {
> +		ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(36));
> +		if (ret)
> +			return ret;
> +	}
> +
> +	/*
> +	 * Use the HVS as the DRM device's DMA controller for buffer
> +	 * allocations if one has not already been configured.
> +	 */
> +	if (drm_dev_dma_dev(drm) == drm->dev)
> +		drm_dev_set_dma_dev(drm, dev);

[Severity: High]
Will this change lead to a DMA API regression in vc4_bo_purge()?

Looking at drivers/gpu/drm/vc4/vc4_bo.c, when a DMA allocation fails under
memory pressure, vc4_bo_userspace_cache_purge() calls vc4_bo_purge() to
reclaim memory from BOs marked as DONTNEED.

In vc4_bo_purge(), the memory is freed using the virtual aggregate device
instead of the newly configured DMA device:

static void vc4_bo_purge(struct drm_gem_object *obj)
{
	...
	dma_free_wc(dev->dev, obj->size, bo->base.vaddr, bo->base.dma_addr);
	...
}

Since buffer objects are allocated using drm_gem_dma_create(), which now
uses the properly configured drm_dev_dma_dev(drm) device, bypassing GEM
helpers and explicitly calling dma_free_wc(dev->dev, ...) breaks the DMA
API contract.

This contract requires memory to be freed with the exact same device pointer
used for allocation. Using a different device guarantees IOMMU unmap failures,
as the aggregate device no longer has a matching DMA/IOMMU configuration.

Should vc4_bo_purge() be updated to use drm_dev_dma_dev(dev) instead of
dev->dev?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260927-drm-dma-device-2-v1-1-3230afcdc851@reactivated.net?part=1