Re: [PATCH v2 3/3] drm/amdgpu/dma-buf: Map/Unmap MMIO_REMAP as BAR register window (dma_map_resource)

["Christian König" <christian.koenig@amd.com>]

On 12.09.25 13:11, Srinivasan Shanmugam wrote:
> MMIO_REMAP (HDP flush page) exposes a hardware MMIO register window via a PCI BAR.
> 
> Handle it as fixed I/O:
> - map(): if MMIO_REMAP, require P2P, compute the BAR address (bus_addr + page
>   offset), and build a 1-entry sg_table with dma_map_resource().
> - unmap(): if MMIO_REMAP, call dma_unmap_resource() and return.
> 
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 39 +++++++++++++++++++--
>  1 file changed, 37 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> index 33fa17a927ce..f85e16be438f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> @@ -151,7 +151,7 @@ static void amdgpu_dma_buf_unpin(struct dma_buf_attachment *attach)
>  }
>  
>  /* Map a BAR-backed I/O span as a 1-entry sg_table via dma_map_resource(). */
> -static __maybe_unused struct sg_table *
> +static struct sg_table *
>  amdgpu_dmabuf_map_iomem(struct device *dev, resource_size_t phys,
>  			size_t size, enum dma_data_direction dir)
>  {
> @@ -183,7 +183,7 @@ amdgpu_dmabuf_map_iomem(struct device *dev, resource_size_t phys,
>  	return sgt;
>  }
>  
> -static __maybe_unused void
> +static void
>  amdgpu_dmabuf_unmap_iomem(struct device *dev, struct sg_table *sgt,
>  			  enum dma_data_direction dir)
>  {
> @@ -218,6 +218,9 @@ static struct sg_table *amdgpu_dma_buf_map(struct dma_buf_attachment *attach,
>  	struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
>  	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
>  	struct sg_table *sgt;
> +	resource_size_t phys;
> +	u64 off;
> +	size_t len;
>  	long r;
>  
>  	if (!bo->tbo.pin_count) {
> @@ -261,6 +264,29 @@ static struct sg_table *amdgpu_dma_buf_map(struct dma_buf_attachment *attach,
>  		if (r)
>  			return ERR_PTR(r);
>  		break;
> +
> +	case AMDGPU_PL_MMIO_REMAP:
> +		/* Only allow when importer can reach exporter via P2P. */
> +		if (!attach->peer2peer ||
> +		    pci_p2pdma_distance(adev->pdev, attach->dev, false) < 0)
> +			return ERR_PTR(-EOPNOTSUPP);
> +

> +		if (!adev->rmmio_remap.bus_addr)
> +			return ERR_PTR(-ENODEV);
> +
> +		if (bo->tbo.base.size != AMDGPU_GPU_PAGE_SIZE)
> +			return ERR_PTR(-EINVAL);

I think you can drop those checks.

> +
> +		/* TTM start is in pages → convert to byte offset. */
> +		off  = (u64)bo->tbo.resource->start << PAGE_SHIFT;

Please don't use resource->start for that. Instead use the functions in amdgpu_res_cursor.h.

> +		len  = AMDGPU_GPU_PAGE_SIZE;
> +		phys = adev->rmmio_remap.bus_addr + off;
> +
> +		sgt = amdgpu_dmabuf_map_iomem(attach->dev, phys, len, dir);

Move the amdgpu_dmabuf_map_iomem() function as well as the off and length calculation into amdgpu_ttm.c.

That is still not an ideal placement, but better than here.

Apart from that looks good to me.

Regards,
Christian.

> +		if (IS_ERR(sgt))
> +			return sgt;
> +		break;
> +
>  	default:
>  		return ERR_PTR(-EINVAL);
>  	}
> @@ -286,6 +312,15 @@ static void amdgpu_dma_buf_unmap(struct dma_buf_attachment *attach,
>  				 struct sg_table *sgt,
>  				 enum dma_data_direction dir)
>  {
> +	struct drm_gem_object *obj = attach->dmabuf->priv;
> +	struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
> +
> +	if (bo->tbo.resource &&
> +	    bo->tbo.resource->mem_type == AMDGPU_PL_MMIO_REMAP) {
> +		amdgpu_dmabuf_unmap_iomem(attach->dev, sgt, dir);
> +		return;
> +	}
> +
>  	if (sg_page(sgt->sgl)) {
>  		dma_unmap_sgtable(attach->dev, sgt, dir, 0);
>  		sg_free_table(sgt);