On 02.09.25 15:25, Alex Deucher wrote:
> 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.
I thought you suggested to use the remapped HDP registers for the HDP flush in
the kernel as well?
If we don't want to do this we can just skip this.
Christian.
>
> 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);
>>>
>>
