[PATCH] drm/amdgpu: add vram reservation logic based on vram_usagebyfirmware_v2_2
Christian König
christian.koenig at amd.com
Fri Nov 4 09:39:45 UTC 2022
Hi Esther,
ok, that works as well.
I would just move the handling of version of the firmware table into
separate functions. That should make it easier to understand what's
going on here.
Thanks,
Christian.
Am 04.11.22 um 10:34 schrieb Liu01, Tong (Esther):
> [AMD Official Use Only - General]
>
> Hi Christian,
>
> Based on VBIOS's comment " driver can allocate their own reservation region as long as it does not overlap firwmare's reservation region". Our understanding is that they are not mutual exclusive. So we create extra parameters to record drv reservation request.
>
> Kind regards,
> Esther
>
> -----Original Message-----
> From: Koenig, Christian <Christian.Koenig at amd.com>
> Sent: 2022年11月4日星期五 下午5:09
> To: Liu01, Tong (Esther) <Tong.Liu01 at amd.com>; amd-gfx at lists.freedesktop.org
> Cc: Andrey Grodzovsky <Andrey.Grodzovsky at amd.com>; Quan, Evan <Evan.Quan at amd.com>; Chen, Horace <Horace.Chen at amd.com>; Tuikov, Luben <Luben.Tuikov at amd.com>; Deucher, Alexander <Alexander.Deucher at amd.com>; Xiao, Jack <Jack.Xiao at amd.com>; Zhang, Hawking <Hawking.Zhang at amd.com>; Liu, Monk <Monk.Liu at amd.com>; Xu, Feifei <Feifei.Xu at amd.com>; Wang, Yang(Kevin) <KevinYang.Wang at amd.com>
> Subject: Re: [PATCH] drm/amdgpu: add vram reservation logic based on vram_usagebyfirmware_v2_2
>
> Am 04.11.22 um 09:52 schrieb Tong Liu01:
>> Move TMR region from top of FB to 2MB for FFBM, so we need to reserve
>> TMR region firstly to make sure TMR can be allocated at 2MB
> If I understand it correctly the two methods are mutual exclusive. So I think you can just extend the existing function in the amdgpu TTM code instead of adding a new one.
>
>> Signed-off-by: Tong Liu01 <Tong.Liu01 at amd.com>
>> ---
>> .../gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c | 84 ++++++++++++++-----
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 52 ++++++++++++
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 6 ++
>> drivers/gpu/drm/amd/include/atomfirmware.h | 56 +++++++++++--
>> 4 files changed, 171 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
>> index b81b77a9efa6..f577b1d151d9 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
>> @@ -106,34 +106,74 @@ int amdgpu_atomfirmware_allocate_fb_scratch(struct amdgpu_device *adev)
>> struct atom_context *ctx = adev->mode_info.atom_context;
>> int index = get_index_into_master_table(atom_master_list_of_data_tables_v2_1,
>> vram_usagebyfirmware);
>> - struct vram_usagebyfirmware_v2_1 *firmware_usage;
>> - uint32_t start_addr, size;
>> + struct vram_usagebyfirmware_v2_1 *firmware_usage_v2_1;
>> + struct vram_usagebyfirmware_v2_2 *firmware_usage_v2_2;
>> + uint32_t start_addr, size, fw_start_addr, fw_size, drv_addr, drv_size;
>> uint16_t data_offset;
>> + uint8_t frev, crev;
>> int usage_bytes = 0;
>>
>> - if (amdgpu_atom_parse_data_header(ctx, index, NULL, NULL, NULL, &data_offset)) {
>> - firmware_usage = (struct vram_usagebyfirmware_v2_1 *)(ctx->bios + data_offset);
>> - DRM_DEBUG("atom firmware requested %08x %dkb fw %dkb drv\n",
>> - le32_to_cpu(firmware_usage->start_address_in_kb),
>> - le16_to_cpu(firmware_usage->used_by_firmware_in_kb),
>> - le16_to_cpu(firmware_usage->used_by_driver_in_kb));
>> -
>> - start_addr = le32_to_cpu(firmware_usage->start_address_in_kb);
>> - size = le16_to_cpu(firmware_usage->used_by_firmware_in_kb);
>> -
>> - if ((uint32_t)(start_addr & ATOM_VRAM_OPERATION_FLAGS_MASK) ==
>> - (uint32_t)(ATOM_VRAM_BLOCK_SRIOV_MSG_SHARE_RESERVATION <<
>> - ATOM_VRAM_OPERATION_FLAGS_SHIFT)) {
>> - /* Firmware request VRAM reservation for SR-IOV */
>> - adev->mman.fw_vram_usage_start_offset = (start_addr &
>> - (~ATOM_VRAM_OPERATION_FLAGS_MASK)) << 10;
>> - adev->mman.fw_vram_usage_size = size << 10;
>> - /* Use the default scratch size */
>> + if (amdgpu_atom_parse_data_header(ctx, index, NULL, &frev, &crev, &data_offset)) {
>> + if (frev == 2 && crev == 1) {
>> + firmware_usage_v2_1 =
>> + (struct vram_usagebyfirmware_v2_1 *)(ctx->bios + data_offset);
>> + DRM_DEBUG("atom firmware v2_1 requested %08x %dkb fw %dkb drv\n",
>> + le32_to_cpu(firmware_usage_v2_1->start_address_in_kb),
>> + le16_to_cpu(firmware_usage_v2_1->used_by_firmware_in_kb),
>> + le16_to_cpu(firmware_usage_v2_1->used_by_driver_in_kb));
>> +
>> + start_addr = le32_to_cpu(firmware_usage_v2_1->start_address_in_kb);
>> + size = le16_to_cpu(firmware_usage_v2_1->used_by_firmware_in_kb);
>> +
>> + if ((uint32_t)(start_addr & ATOM_VRAM_OPERATION_FLAGS_MASK) ==
>> + (uint32_t)(ATOM_VRAM_BLOCK_SRIOV_MSG_SHARE_RESERVATION <<
>> + ATOM_VRAM_OPERATION_FLAGS_SHIFT)) {
>> + /* Firmware request VRAM reservation for SR-IOV */
>> + adev->mman.fw_vram_usage_start_offset = (start_addr &
>> + (~ATOM_VRAM_OPERATION_FLAGS_MASK)) << 10;
>> + adev->mman.fw_vram_usage_size = size << 10;
>> + /* Use the default scratch size */
>> + usage_bytes = 0;
>> + } else {
>> + usage_bytes =
>> + le16_to_cpu(firmware_usage_v2_1->used_by_driver_in_kb) << 10;
>> + }
> That should probably be put into separate functions.
>
> Regards,
> Christian.
>
>> + } else if (frev >= 2 && crev >= 2) {
>> + firmware_usage_v2_2 =
>> + (struct vram_usagebyfirmware_v2_2 *)(ctx->bios + data_offset);
>> + DRM_DEBUG("atom requested fw start at %08x %dkb and drv start at %08x %dkb\n",
>> + le32_to_cpu(firmware_usage_v2_2->fw_region_start_address_in_kb),
>> + le16_to_cpu(firmware_usage_v2_2->used_by_firmware_in_kb),
>> + le32_to_cpu(firmware_usage_v2_2->driver_region0_start_address_in_kb),
>> + le32_to_cpu(firmware_usage_v2_2->used_by_driver_region0_in_kb));
>> +
>> + if ((uint32_t)((le32_to_cpu(firmware_usage_v2_2->fw_region_start_address_in_kb))
>> + & (ATOM_VRAM_BLOCK_NEEDS_NO_RESERVATION << 30)) == 0) {
>> + fw_start_addr = (le32_to_cpu(firmware_usage_v2_2->fw_region_start_address_in_kb))
>> + & (~ATOM_VRAM_OPERATION_FLAGS_MASK);
>> + fw_size = le16_to_cpu(firmware_usage_v2_2->used_by_firmware_in_kb);
>> +
>> + /* Firmware request VRAM reservation for SR-IOV */
>> + adev->mman.fw_vram_usage_start_offset = fw_start_addr << 10;
>> + adev->mman.fw_vram_usage_size = fw_size << 10;
>> + }
>> +
>> + if ((uint32_t)((ATOM_VRAM_BLOCK_NEEDS_NO_RESERVATION << 30)
>> + & (le32_to_cpu(firmware_usage_v2_2->driver_region0_start_address_in_kb))) == 0) {
>> + drv_addr = (le32_to_cpu(firmware_usage_v2_2->driver_region0_start_address_in_kb))
>> + & (~ATOM_VRAM_OPERATION_FLAGS_MASK);
>> + drv_size = le32_to_cpu(firmware_usage_v2_2->used_by_driver_region0_in_kb);
>> +
>> + /* driver request VRAM reservation for SR-IOV */
>> + adev->mman.drv_vram_usage_start_offset = drv_addr << 10;
>> + adev->mman.drv_vram_usage_size = drv_size << 10;
>> + }
>> +
>> usage_bytes = 0;
>> - } else {
>> - usage_bytes = le16_to_cpu(firmware_usage->used_by_driver_in_kb) << 10;
>> +
>> }
>> }
>> +
>> ctx->scratch_size_bytes = 0;
>> if (usage_bytes == 0)
>> usage_bytes = 20 * 1024;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> index 585460ab8dfd..099605380b06 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> @@ -1578,6 +1578,22 @@ static void amdgpu_ttm_fw_reserve_vram_fini(struct amdgpu_device *adev)
>> NULL, &adev->mman.fw_vram_usage_va);
>> }
>>
>> +/*
>> + * Driver Reservation functions
>> + */
>> +/**
>> + * amdgpu_ttm_drv_reserve_vram_fini - free drv reserved vram
>> + *
>> + * @adev: amdgpu_device pointer
>> + *
>> + * free drv reserved vram if it has been reserved.
>> + */
>> +static void amdgpu_ttm_drv_reserve_vram_fini(struct amdgpu_device *adev)
>> +{
>> + amdgpu_bo_free_kernel(&adev->mman.drv_vram_usage_reserved_bo,
>> + NULL, &adev->mman.drv_vram_usage_va);
>> +}
>> +
>> /**
>> * amdgpu_ttm_fw_reserve_vram_init - create bo vram reservation from fw
>> *
>> @@ -1604,6 +1620,32 @@ static int amdgpu_ttm_fw_reserve_vram_init(struct amdgpu_device *adev)
>> &adev->mman.fw_vram_usage_va);
>> }
>>
>> +/**
>> + * amdgpu_ttm_drv_reserve_vram_init - create bo vram reservation from driver
>> + *
>> + * @adev: amdgpu_device pointer
>> + *
>> + * create bo vram reservation from drv.
>> + */
>> +static int amdgpu_ttm_drv_reserve_vram_init(struct amdgpu_device *adev)
>> +{
>> + uint64_t vram_size = adev->gmc.visible_vram_size;
>> +
>> + adev->mman.drv_vram_usage_va = NULL;
>> + adev->mman.drv_vram_usage_reserved_bo = NULL;
>> +
>> + if (adev->mman.drv_vram_usage_size == 0 ||
>> + adev->mman.drv_vram_usage_size > vram_size)
>> + return 0;
>> +
>> + return amdgpu_bo_create_kernel_at(adev,
>> + adev->mman.drv_vram_usage_start_offset,
>> + adev->mman.drv_vram_usage_size,
>> + AMDGPU_GEM_DOMAIN_VRAM,
>> + &adev->mman.drv_vram_usage_reserved_bo,
>> + &adev->mman.drv_vram_usage_va);
>> +}
>> +
>> /*
>> * Memoy training reservation functions
>> */
>> @@ -1771,6 +1813,15 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
>> return r;
>> }
>>
>> + /*
>> + *The reserved vram for driver must be pinned to the specified
>> + *place on the VRAM, so reserve it early.
>> + */
>> + r = amdgpu_ttm_drv_reserve_vram_init(adev);
>> + if (r) {
>> + return r;
>> + }
>> +
>> /*
>> * only NAVI10 and onwards ASIC support for IP discovery.
>> * If IP discovery enabled, a block of memory should be
>> @@ -1896,6 +1947,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_fw_reserve_vram_fini(adev);
>> + amdgpu_ttm_drv_reserve_vram_fini(adev);
>>
>> if (drm_dev_enter(adev_to_drm(adev), &idx)) {
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> index 9120ae80ef52..c60246f32f98 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> @@ -92,6 +92,12 @@ struct amdgpu_mman {
>> struct amdgpu_bo *fw_vram_usage_reserved_bo;
>> void *fw_vram_usage_va;
>>
>> + /* driver VRAM reservation */
>> + u64 drv_vram_usage_start_offset;
>> + u64 drv_vram_usage_size;
>> + struct amdgpu_bo *drv_vram_usage_reserved_bo;
>> + void *drv_vram_usage_va;
>> +
>> /* PAGE_SIZE'd BO for process memory r/w over SDMA. */
>> struct amdgpu_bo *sdma_access_bo;
>> void *sdma_access_ptr;
>> diff --git a/drivers/gpu/drm/amd/include/atomfirmware.h b/drivers/gpu/drm/amd/include/atomfirmware.h
>> index ff855cb21d3f..9f8761407099 100644
>> --- a/drivers/gpu/drm/amd/include/atomfirmware.h
>> +++ b/drivers/gpu/drm/amd/include/atomfirmware.h
>> @@ -705,10 +705,47 @@ struct atom_gpio_pin_lut_v2_1
>> };
>>
>>
>> -/*
>> - ***************************************************************************
>> - Data Table vram_usagebyfirmware structure
>> - ***************************************************************************
>> +/*
>> + VBIOS/PRE-OS always reserve a FB region at the top of frame buffer. driver should not write access that region.
>> + driver can allocate their own reservation region as long as it does not overlap firwmare's reservation region.
>> + if( atom data table firmwareInfoTable version < 3.3) { //( pre-NV1X )
>> + in this case, atom data table vram_usagebyfirmwareTable version always <= 2.1
>> + if( VBIOS/UEFI GOP is posted ) {
>> + VBIOS/UEFIGOP update used_by_firmware_in_kb = total reserved size by VBIOS
>> + update start_address_in_kb = total_mem_size_in_kb - used_by_firmware_in_kb; ( total_mem_size_in_kb = reg(CONFIG_MEMSIZE)<<10)
>> + driver can allocate driver reservation region under firmware reservation,
>> + used_by_driver_in_kb = driver reservation size
>> + driver reservation start address = (start_address_in_kb - used_by_driver_in_kb)
>> + Comment1[hchan]: There is only one reservation at the beginning of the FB reserved by Host driver.
>> + Host driver would overwrite the table with the following
>> + used_by_firmware_in_kb = total reserved size for pf-vf info exchange and
>> + set SRIOV_MSG_SHARE_RESERVATION mask start_address_in_kb = 0
>> + } else {
>> + there is no VBIOS reservation region
>> + driver must allocate driver reservation region at top of FB.
>> + driver set used_by_driver_in_kb = driver reservation size
>> + driver reservation start address = (total_mem_size_in_kb - used_by_driver_in_kb)
>> + same as Comment1
>> + }
>> + } else { //( NV1X and after)
>> + if( VBIOS/UEFI GOP is posted ) {
>> + VBIOS/UEFIGOP update used_by_firmware_in_kb = atom_firmware_Info_v3_3.fw_reserved_size_in_kb;
>> + update start_address_in_kb = total_mem_size_in_kb - used_by_firmware_in_kb; ( total_mem_size_in_kb = reg(CONFIG_MEMSIZE)<<10 )
>> + }
>> + if( vram_usagebyfirmwareTable version <= 2.1 ) {
>> + driver can allocate driver reservation region under firmware reservation,
>> + driver set used_by_driver_in_kb = driver reservation size
>> + driver reservation start address = (start_address_in_kb - used_by_driver_in_kb)
>> + same as Comment1
>> + } else {
>> + dirver can allocate it reservation any place as long as it does overlap pre-OS FW reservation area
>> + driver set used_by_driver_region0_in_kb = driver reservation size
>> + driver set driver_region0_start_address_in_kb = driver reservation region start address
>> + Comment2[hchan]: Host driver can set used_by_firmware_in_kb and start_address_in_kb to zero
>> + as the reservation for VF as it doesn’t exist. And Host driver should also
>> + update atom_firmware_Info table to remove the same VBIOS reservation as well.
>> + }
>> + }
>> */
>>
>> struct vram_usagebyfirmware_v2_1
>> @@ -716,9 +753,18 @@ struct vram_usagebyfirmware_v2_1
>> struct atom_common_table_header table_header;
>> uint32_t start_address_in_kb;
>> uint16_t used_by_firmware_in_kb;
>> - uint16_t used_by_driver_in_kb;
>> + uint16_t used_by_driver_in_kb;
>> };
>>
>> +struct vram_usagebyfirmware_v2_2{
>> + struct atom_common_table_header table_header;
>> + uint32_t fw_region_start_address_in_kb;
>> + uint16_t used_by_firmware_in_kb;
>> + uint16_t reserved;
>> + uint32_t driver_region0_start_address_in_kb;
>> + uint32_t used_by_driver_region0_in_kb;
>> + uint32_t reserved32[7];
>> +};
>>
>> /*
>> ***************************************************************************
More information about the amd-gfx
mailing list