On Tue, Sep 2, 2025 at 3:38 AM Christian König <christian.koe...@amd.com> wrote:
>
> On 02.09.25 05:29, Srinivasan Shanmugam wrote:
> > Add mmio_remap bookkeeping to amdgpu_device and introduce
> > amdgpu_ttm_mmio_remap_bo_init()/fini() to manage a kernel-owned,
> > one-page (4K) BO in AMDGPU_GEM_DOMAIN_MMIO_REMAP.
> >
> > Bookkeeping:
> > - adev->rmmio_remap.bo : kernel-owned singleton BO
> >
> > The BO is allocated during TTM init when a remap bus address is available
> > (adev->rmmio_remap.bus_addr) and PAGE_SIZE <= AMDGPU_GPU_PAGE_SIZE (4K),
> > and freed during TTM fini.
> >
> > v2:
> > - Check mmio_remap bus address (adev->rmmio_remap.bus_addr) instead of
> > rmmio_base. (Alex)
> > - Skip quietly if PAGE_SIZE > AMDGPU_GPU_PAGE_SIZE or no bus address
> > (no warn). (Alex)
> > - Use `amdgpu_bo_create()` (not *_kernel) - Only with this The object
> > is stored in adev->mmio_remap.bo and will later be exposed to
> > userspace via a GEM handle. (Christian)
> >
> > v3:
> > - Remove obvious comment before amdgpu_ttm_mmio_remap_bo_fini() call.
> > (Alex)
> >
> > v4:
> > - Squash bookkeeping into this patch
> > - Place longer declaration first; clear bp via memset
> > - Reserve + pin + kmap(+kunmap) the BO at init; unpin in fini
> > (Christian)
> >
> > Suggested-by: Christian König <christian.koe...@amd.com>
> > Suggested-by: Alex Deucher <alexander.deuc...@amd.com>
> > Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmu...@amd.com>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
> > drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 87 +++++++++++++++++++++++++
> > 2 files changed, 88 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > index ddd472e56f69..24501d3fbefe 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > @@ -752,6 +752,7 @@ typedef void (*amdgpu_block_wreg_t)(struct
> > amdgpu_device*, uint32_t, uint32_t, u
> > struct amdgpu_mmio_remap {
> > u32 reg_offset;
> > resource_size_t bus_addr;
> > + struct amdgpu_bo *bo;
> > };
> >
> > /* Define the HW IP blocks will be used in driver , add more if necessary
> > */
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > index 1a68ba17a62d..0d03e3a6f92d 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > @@ -1854,6 +1854,87 @@ static void amdgpu_ttm_pools_fini(struct
> > amdgpu_device *adev)
> > adev->mman.ttm_pools = NULL;
> > }
> >
> > +/**
> > + * amdgpu_ttm_mmio_remap_bo_init - Allocate the singleton 4K MMIO_REMAP BO
> > + * @adev: amdgpu device
> > + *
> > + * Allocates a one-page (4K) GEM BO in AMDGPU_GEM_DOMAIN_MMIO_REMAP when
> > the
> > + * hardware exposes a remap base (adev->rmmio_remap.bus_addr) and the host
> > + * PAGE_SIZE is <= AMDGPU_GPU_PAGE_SIZE (4K). The BO is created as a
> > regular
> > + * GEM object (amdgpu_bo_create).
> > + *
> > + * Return:
> > + * * 0 on success or intentional skip (feature not present/unsupported)
> > + * * negative errno on allocation failure
> > + */
> > +static int amdgpu_ttm_mmio_remap_bo_init(struct amdgpu_device *adev)
> > +{
> > + struct amdgpu_bo_param bp;
> > + int r;
>
> > + void *kptr;
>
> kptr should potentially be saved in amdgpu_mmio_remap.
>
> > +
> > + /* Skip if HW doesn't expose remap, or if PAGE_SIZE >
> > AMDGPU_GPU_PAGE_SIZE (4K). */
> > + if (!adev->rmmio_remap.bus_addr || PAGE_SIZE > AMDGPU_GPU_PAGE_SIZE)
> > + return 0;
> > +
> > + memset(&bp, 0, sizeof(bp));
> > +
> > + /* Create exactly one GEM BO in the MMIO_REMAP domain. */
> > + bp.type = ttm_bo_type_device; /* userspace-mappable
> > GEM */
> > + bp.size = AMDGPU_GPU_PAGE_SIZE; /* 4K */
> > + bp.byte_align = AMDGPU_GPU_PAGE_SIZE;
> > + bp.domain = AMDGPU_GEM_DOMAIN_MMIO_REMAP;
> > + bp.flags = 0;
> > + bp.resv = NULL;
> > + bp.bo_ptr_size = sizeof(struct amdgpu_bo);
> > +
> > + r = amdgpu_bo_create(adev, &bp, &adev->rmmio_remap.bo);
> > + if (r)
> > + return r;
> > +
> > + r = amdgpu_bo_reserve(adev->rmmio_remap.bo, false);
>
> The last parameter should probably be true here.
>
> > + if (r)
> > + goto err_unref;
> > +
> > + r = amdgpu_bo_pin(adev->rmmio_remap.bo, AMDGPU_GEM_DOMAIN_MMIO_REMAP);
> > + if (r)
> > + goto err_unres;
> > +
> > + r = amdgpu_bo_kmap(adev->rmmio_remap.bo, &kptr);
Can't we just skip this? We don't need the CPU address in the kernel.
Alex
> > + if (r)
> > + goto err_unpin;
> > +
> > + amdgpu_bo_kunmap(adev->rmmio_remap.bo);
> > + amdgpu_bo_unreserve(adev->rmmio_remap.bo);
> > + return 0;
> > +
> > +err_unpin:
> > + amdgpu_bo_unpin(adev->rmmio_remap.bo);
> > +err_unres:
> > + amdgpu_bo_unreserve(adev->rmmio_remap.bo);
> > +err_unref:
> > + amdgpu_bo_unref(&adev->rmmio_remap.bo);
> > + adev->rmmio_remap.bo = NULL;
> > + return r;
> > +}
> > +
> > +/**
> > + * amdgpu_ttm_mmio_remap_bo_fini - Free the singleton MMIO_REMAP BO
> > + * @adev: amdgpu device
> > + *
> > + * Frees the kernel-owned MMIO_REMAP BO if it was allocated by
> > + * amdgpu_ttm_mmio_remap_bo_init().
> > + */
> > +static void amdgpu_ttm_mmio_remap_bo_fini(struct amdgpu_device *adev)
> > +{
> > + if (!amdgpu_bo_reserve(adev->rmmio_remap.bo, false)) {
>
> Same here.
>
> Apart from that looks good to me, feel free to add my rb.
>
> Regards,
> Christian.
>
> > + amdgpu_bo_unpin(adev->rmmio_remap.bo);
> > + amdgpu_bo_unreserve(adev->rmmio_remap.bo);
> > + }
> > + amdgpu_bo_unref(&adev->rmmio_remap.bo);
> > + adev->rmmio_remap.bo = NULL;
> > +}
> > +
> > /*
> > * amdgpu_ttm_init - Init the memory management (ttm) as well as various
> > * gtt/vram related fields.
> > @@ -2028,6 +2109,11 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
> > return r;
> > }
> >
> > + /* Allocate the singleton MMIO_REMAP BO (4K) if supported */
> > + r = amdgpu_ttm_mmio_remap_bo_init(adev);
> > + if (r)
> > + return r;
> > +
> > /* Initialize preemptible memory pool */
> > r = amdgpu_preempt_mgr_init(adev);
> > if (r) {
> > @@ -2091,6 +2177,7 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev)
> > amdgpu_bo_free_kernel(&adev->mman.sdma_access_bo, NULL,
> > &adev->mman.sdma_access_ptr);
> >
> > + amdgpu_ttm_mmio_remap_bo_fini(adev);
> > amdgpu_ttm_fw_reserve_vram_fini(adev);
> > amdgpu_ttm_drv_reserve_vram_fini(adev);
> >
>
