Hi Chen-Yu, Thank you for the patch.
On Tue, Mar 17, 2026 at 02:40:43PM +0800, Chen-Yu Tsai wrote: > The rcar-du driver is directly calling dma_get_sgtable() on a > drm_gem_dma_object. Not passing the dma_attrs field in may cause > problems when DMA_ATTR_NO_KERNEL_MAPPING is added to the GEM DMA > helpers gain support later. > > Instead, use the drm_gem_dma_get_sg_table() helper to get the scatter > gather table. > > Signed-off-by: Chen-Yu Tsai <[email protected]> > --- > Changes since v1: > - new patch > > Not sure if we should add a helper like drm_fb_dma_get_gem_addr(). This > seems to be the only driver that is using a scatter gather table to pass > DMA addresses. The DU can't access memory directly (at least on Gen3 and newer), and goes through a separate device called VSP that acts as an external DMA engine (*) and compositor. This is why buffers need to be mapped manually to the VSP, the GEM helpers would otherwise use the DU struct device, which isn't correct. I can't use drm_device.dma_dev as the DMA initiator can be different per CRTC. I don't think a separate helper with a single user would be very useful here, especially given that part of the logic in drm_fb_dma_get_gem_addr() is in the separate vsp1 driver. Refactoring may get messy, for little benefit. * And in some cases the VSP further delegates memory access to the FCP, which is yet another DMA initiator from an IOMMU point of view. > --- > drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c > b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c > index 94c22d2db197..6a62608ee3a9 100644 > --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c > +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c > @@ -291,10 +291,16 @@ int rcar_du_vsp_map_fb(struct rcar_du_vsp *vsp, struct > drm_framebuffer *fb, > dst = sg_next(dst); > } > } else { > - ret = dma_get_sgtable(rcdu->dev, sgt, gem->vaddr, > - gem->dma_addr, gem->base.size); Did you compile the patch ? The rcdu variable is now unused. Apart from that, it compiles fine and seems to operate as expected. > - if (ret) > + struct sg_table *ret_sgt; > + > + ret_sgt = drm_gem_dma_get_sg_table(gem); > + if (IS_ERR(ret_sgt)) { > + ret = PTR_ERR(ret_sgt); > goto fail; > + } > + > + memcpy(sgt, ret_sgt, sizeof(*sgt)); > + kfree(ret_sgt); It's a bit of a shame to kmalloc() a new sg_table and free it right after :-/ Would it be that bad to switch to dma_get_sgtable_attrs() here, and pass gem->dma_attrs to the function ? > } > > ret = vsp1_du_map_sg(vsp->vsp, sgt); -- Regards, Laurent Pinchart
