From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: drm/sun4i: Use backend/mixer as dedicated DMA device
Date: Wed, 11 Mar 2026 13:48:36 +1000
Message-ID: <review-patch4-20260310032012.2542334-5-wenst@chromium.org>
In-Reply-To: <20260310032012.2542334-5-wenst@chromium.org>
References: <20260310032012.2542334-1-wenst@chromium.org> <20260310032012.2542334-5-wenst@chromium.org>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0

Patch Review

This replaces the `of_dma_configure()` hack with proper `drm_dev_set_dma_dev()` calls. The approach is:

```c
-	if (of_property_present(dev->of_node, "interconnects")) {
-		ret = of_dma_configure(drm->dev, dev->of_node, true);
-		if (ret)
-			return ret;
-	}
+	if (drm_dev_dma_dev(drm) == drm->dev)
+		drm_dev_set_dma_dev(drm, dev);
```

The guard `drm_dev_dma_dev(drm) == drm->dev` ensures only the first backend/mixer to bind sets itself as the DMA device, which matches the comment explaining the rationale.

**Behavioral change to note**: The old backend code was gated on `of_property_present(dev->of_node, "interconnects")`, and the old mixer code was gated on `of_property_present(dev->of_node, "iommus")`. The new code unconditionally sets the DMA device. This changes behavior for devices *without* those DT properties -- previously the DRM device's own (parent) device would be used for DMA, now it will use the backend/mixer device. This should generally be fine because the backend/mixer *is* the actual DMA device regardless of whether an IOMMU or interconnect is involved, but it's a semantic change worth calling out. If any board has a backend/mixer that truly can't do DMA and relies on the parent device, this could be a problem, though that scenario seems unlikely.

**Comment nit**: The comment in `sun8i_mixer.c` says "backends" and "frontends" (copy-pasted from `sun4i_backend.c`) but should probably say "mixers" to be accurate for the mixer context. This existed partially before but was made worse by the patch expanding the comment. Line 1175-1176:

```c
+	 * This assume we have the same DMA constraints for all our the
+	 * devices in our pipeline (all the backends, but also the
+	 * frontends).
```

This text makes sense for `sun4i_backend.c` but is inaccurate in `sun8i_mixer.c` where the original comment correctly said "mixers" instead of "backends"/"frontends". The old mixer comment was already more appropriate.

**Unbind path**: The old code modified `drm->dev`'s DMA configuration directly, so there was no cleanup needed on unbind. With the new approach, `drm_dev_set_dma_dev()` takes a reference on the device via `get_device()`. Is this reference properly released when the DRM device is torn down? Looking at `drm_dev_set_dma_dev()`, it does `get_device(dma_dev)` and `put_device(dev->dma_dev)`. The DRM device cleanup in `drm_dev_release()` should handle this, but it's worth verifying that `dma_dev` is properly cleared. This applies to all drivers calling `drm_dev_set_dma_dev()` from a component bind function -- if the component unbinds before the DRM device is destroyed, the reference on the component's device will keep it alive, which should be fine since the DRM device will outlive individual components.

**Reviewed-by worthy**: Yes, with the comment nit about "backends"/"frontends" in the mixer code.

---
Generated by Claude Code Patch Reviewer