On 20.08.25 13:32, Srinivasan Shanmugam wrote:
> Create the single-page MMIO_REMAP BO and initialize a tiny on-chip
> range manager for the new placement:
>
> * add amdgpu_mmio_remap_bo_init()/fini()
> * in amdgpu_ttm_init(): initialize AMDGPU_PL_MMIO_REMAP heap and create
> the PAGE_SIZE BO
> * in amdgpu_ttm_fini(): drop BO and tear down the range manager
>
> This isolates lifetime management and error paths for the remap BO and
> ties them into the TTM bring-up/teardown flow.
>
> Cc: Christian König <christian.koe...@amd.com>
> Cc: Alex Deucher <alexander.deuc...@amd.com>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmu...@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 86 +++++++++++++++++++++++++
> 1 file changed, 86 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 157a5416a826..ab93fbec2a34 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1859,6 +1859,73 @@ static void amdgpu_ttm_pools_fini(struct amdgpu_device
> *adev)
> adev->mman.ttm_pools = NULL;
> }
>
> +/* ================= MMIO remap (HDP flush) singleton BO ================= */
Please drop those =, either use kerneldoc a just a comment.
> +static int amdgpu_mmio_remap_bo_init(struct amdgpu_device *adev)
> +{
> + resource_size_t bar_base, bar_len, bus, off;
> + int r;
> +
> + /* The ASIC code should have set this to the absolute bus address
> + * of the remap page (inside the register BAR).
> + */
> + bus = adev->rmmio_remap.bus_addr;
> + if (!bus) {
> + dev_dbg(adev->dev, "MMIO_REMAP: no remap bus addr; skipping
> BO\n");
> + return -ENODEV;
> + }
> +
> + /* The register BAR base/size were established in amdgpu_device_init()
> */
> + bar_base = adev->rmmio_base;
> + bar_len = adev->rmmio_size;
> +
> + /* Sanity: page must lie wholly inside the register BAR */
> + if (bus < bar_base || (bus + PAGE_SIZE) > (bar_base + bar_len)) {
> + dev_err(adev->dev,
> + "MMIO_REMAP: bus 0x%llx not in REG BAR
> [0x%llx..0x%llx)\n",
> + (unsigned long long)bus,
> + (unsigned long long)bar_base,
> + (unsigned long long)(bar_base + bar_len));
> + return -ERANGE;
> + }
> +
> + off = bus - bar_base;
> + if (!IS_ALIGNED(off, PAGE_SIZE)) {
> + dev_err(adev->dev, "MMIO_REMAP: offset 0x%llx not
> page-aligned\n",
> + (unsigned long long)off);
> + return -EINVAL;
> + }
> +
Please drop all that. The code here should not check anything but just creates
the BO at the specific location.
> + /* Create exactly ONE kernel-owned BO in the MMIO_REMAP domain */
> + r = amdgpu_bo_create_kernel(adev,
> + PAGE_SIZE, /* bo_size */
> + PAGE_SIZE, /* alignment*/
> + AMDGPU_GEM_DOMAIN_MMIO_REMAP,
> + &adev->mmio_remap_bo,
> + NULL, NULL);
That won't work like this. The difference between a kernel and an userspace BO
is that kernel BOs can't be mapped into userspace.
You need to use amdgpu_bo_create() here, lock it and kmap it manually.
> + if (r) {
> + dev_err(adev->dev, "MMIO_REMAP: BO create failed (%d)\n", r);
> + return r;
> + }
> +
> + dev_dbg(adev->dev,
> + "MMIO_REMAP: base=0x%llx off=0x%llx size=0x%lx (BO created)\n",
> + (unsigned long long)adev->mmio_remap_base,
> + (unsigned long long)adev->mmio_remap_offset,
> + (unsigned long)adev->mmio_remap_size);
> +
> + return 0;
> +}
> +
> +static void amdgpu_mmio_remap_bo_fini(struct amdgpu_device *adev)
> +{
> +
> + if (adev->mmio_remap_bo)
Drop that check.
Regards,
Christian.
> + amdgpu_bo_free_kernel(&adev->mmio_remap_bo, NULL, NULL);
> + adev->mmio_remap_bo = NULL;
> +
> + return;
> +}
> +
> /*
> * amdgpu_ttm_init - Init the memory management (ttm) as well as various
> * gtt/vram related fields.
> @@ -2026,6 +2093,20 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
> return r;
> }
>
> + /* Initialize MMIO-remap pool (single page) */
> + r = amdgpu_ttm_init_on_chip(adev, AMDGPU_PL_MMIO_REMAP, 1);
> + if (r) {
> + dev_err(adev->dev, "Failed initializing MMIO-remap heap.\n");
> + return r;
> + }
> +
> + /* Create the singleton MMIO-remap BO (one page) */
> + r = amdgpu_mmio_remap_bo_init(adev);
> + if (r) {
> + dev_err(adev->dev, "Failed to create MMIO-remap singleton
> BO.\n");
> + return r;
> + }
> +
> /* Initialize preemptible memory pool */
> r = amdgpu_preempt_mgr_init(adev);
> if (r) {
> @@ -2088,6 +2169,9 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev)
> }
> amdgpu_bo_free_kernel(&adev->mman.sdma_access_bo, NULL,
> &adev->mman.sdma_access_ptr);
> +
> + /* === Drop the singleton MMIO-remap BO === */
> + amdgpu_mmio_remap_bo_fini(adev);
> amdgpu_ttm_fw_reserve_vram_fini(adev);
> amdgpu_ttm_drv_reserve_vram_fini(adev);
>
> @@ -2109,6 +2193,8 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev)
> ttm_range_man_fini(&adev->mman.bdev, AMDGPU_PL_GWS);
> ttm_range_man_fini(&adev->mman.bdev, AMDGPU_PL_OA);
> ttm_range_man_fini(&adev->mman.bdev, AMDGPU_PL_DOORBELL);
> + /* Tear down the tiny range manager for MMIO-remap */
> + ttm_range_man_fini(&adev->mman.bdev, AMDGPU_PL_MMIO_REMAP);
> ttm_device_fini(&adev->mman.bdev);
> adev->mman.initialized = false;
> dev_info(adev->dev, "amdgpu: ttm finalized\n");
