> -----Original Message----- > From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf > Of Horace Chen > Sent: Tuesday, September 26, 2017 6:12 AM > To: amd-gfx at lists.freedesktop.org > Cc: Chen, Horace > Subject: [PATCH] drm/amdgpu: Add firmware VRAM reservation from vbios > > VBIOS has a structure VRAM_UsageByFirmware to tell the driver that > VBIOS need reserve some memory at the specified place on the VRAM. > SR-IOV need to enable this feature to exchange data between PF > and VF. > So add two new method to create and free bo at the exact place. > And add a new AMDGPU_GEM_CREATE_VRAM_SPECIFIED flag to tell > function to set the ttm_placement to the exact place to avoid > eviction whenpinnning. First, this patch needs to be split up into 3 patches: 1. update atombios.h 2. Updates to support fixed memory placements 3. add the vbios reserved area Some additional comments below. > > Signed-off-by: Horace Chen <horace.chen at amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 17 +++++ > drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c | 18 ++++- > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 58 ++++++++++++++ > drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 109 > ++++++++++++++++++++++++++- > drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 8 ++ > drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 9 +++ > drivers/gpu/drm/amd/include/atombios.h | 1 + > include/uapi/drm/amdgpu_drm.h | 2 + > 8 files changed, 219 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > index a5b0b67..ac53dba 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > @@ -1380,6 +1380,21 @@ struct amdgpu_atcs { > }; > > /* > + * Firmware VRAM reservation > + */ > +#define KB_TO_BYTE(x) ((x) << 10) > +#define BYTE_TO_KB(x) ((x) >> 10) > + > +struct amdgpu_fw_vram_usage { > + uint32_t start_address_in_kb; > + uint16_t size_in_kb; > + struct amdgpu_bo *reserved_bo; > + volatile uint32_t *va; > +}; > + > +int amdgpu_fw_reserve_vram_init(struct amdgpu_device *adev); > + > +/* > * CGS > */ > struct cgs_device *amdgpu_cgs_create_device(struct amdgpu_device > *adev); > @@ -1588,6 +1603,8 @@ struct amdgpu_device { > struct delayed_work late_init_work; > > struct amdgpu_virt virt; > + /* firmware VRAM reservation */ > + struct amdgpu_fw_vram_usage fw_vram_usage; > > /* link all shadow bo */ > struct list_head shadow_list; > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c > index ce44358..56bfddf 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c > @@ -1807,6 +1807,8 @@ int amdgpu_atombios_allocate_fb_scratch(struct > amdgpu_device *adev) > uint16_t data_offset; > int usage_bytes = 0; > struct _ATOM_VRAM_USAGE_BY_FIRMWARE *firmware_usage; > + uint32_t start_addr; > + uint16_t size; > > if (amdgpu_atom_parse_data_header(ctx, index, NULL, NULL, NULL, > &data_offset)) { > firmware_usage = (struct > _ATOM_VRAM_USAGE_BY_FIRMWARE *)(ctx->bios + data_offset); > @@ -1815,7 +1817,21 @@ int amdgpu_atombios_allocate_fb_scratch(struct > amdgpu_device *adev) > le32_to_cpu(firmware_usage- > >asFirmwareVramReserveInfo[0].ulStartAddrUsedByFirmware), > le16_to_cpu(firmware_usage- > >asFirmwareVramReserveInfo[0].usFirmwareUseInKb)); > > - usage_bytes = le16_to_cpu(firmware_usage- > >asFirmwareVramReserveInfo[0].usFirmwareUseInKb) * 1024; > + start_addr = firmware_usage- > >asFirmwareVramReserveInfo[0].ulStartAddrUsedByFirmware; > + size = firmware_usage- > >asFirmwareVramReserveInfo[0].usFirmwareUseInKb; > + > + if ((uint32_t)(start_addr & > ATOM_VRAM_OPERATION_FLAGS_MASK) == > + > (uint32_t)(ATOM_VRAM_BLOCK_SRIOV_MSG_SHARE_RESERVATIO > N << > + ATOM_VRAM_OPERATION_FLAGS_SHIFT)) { > + /* Firmware request VRAM reservation for SR-IOV */ > + adev->fw_vram_usage.start_address_in_kb = > + start_addr & > (~ATOM_VRAM_OPERATION_FLAGS_MASK); > + adev->fw_vram_usage.size_in_kb = size; > + /* Use the default scratch size */ > + usage_bytes = 0; > + } else { > + usage_bytes = le16_to_cpu(firmware_usage- > >asFirmwareVramReserveInfo[0].usFirmwareUseInKb) * 1024; > + } > } > ctx->scratch_size_bytes = 0; > if (usage_bytes == 0) > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index a86d856..51bee68 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -658,6 +658,62 @@ void amdgpu_gart_location(struct amdgpu_device > *adev, struct amdgpu_mc *mc) > } > > /* > + * Firmware Reservation function > + */ > +/** > + * amdgpu_fw_reserve_vram_fini - free fw reserved vram > + * > + * @adev: amdgpu_device pointer > + * > + * free fw reserved vram if it is reserved. > + */ > +void amdgpu_fw_reserve_vram_fini(struct amdgpu_device *adev) > +{ > + > + if (adev->fw_vram_usage.reserved_bo == NULL) > + return; > + > + amdgpu_bo_free_vram_specified(&adev- > >fw_vram_usage.reserved_bo, NULL, > + (void **)&adev->fw_vram_usage.va); > + adev->fw_vram_usage.reserved_bo = NULL; > +} > + > +/** > + * amdgpu_fw_reserve_vram_fini - create bo vram reservation from fw > + * > + * @adev: amdgpu_device pointer > + * > + * create bo vram reservation from fw. > + */ > +int amdgpu_fw_reserve_vram_init(struct amdgpu_device *adev) > +{ > + int r = 0; > + u64 gpu_addr; > + u64 vram_size = adev->mc.visible_vram_size; > + > + adev->fw_vram_usage.va = NULL; > + adev->fw_vram_usage.reserved_bo = NULL; > + > + if (adev->fw_vram_usage.size_in_kb > 0 && > + KB_TO_BYTE((u64)adev->fw_vram_usage.size_in_kb) <= > vram_size) { > + > + r = amdgpu_bo_create_vram_specified(adev, > + KB_TO_BYTE((u64)adev- > >fw_vram_usage.start_address_in_kb), > + KB_TO_BYTE((u64)adev- > >fw_vram_usage.size_in_kb), > + PAGE_SIZE, > + AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED > | > + AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS, > + &adev->fw_vram_usage.reserved_bo, > + &gpu_addr, (void **)&adev->fw_vram_usage.va); > + if (r) > + dev_info(adev->dev, "Reserve VRAM for firmware > failed"); > + } > + > + return r; > +} > + > + > +/* > * GPU helpers function. > */ > /** > @@ -2055,6 +2111,7 @@ int amdgpu_device_init(struct amdgpu_device > *adev, > adev->vm_manager.vm_pte_num_rings = 0; > adev->gart.gart_funcs = NULL; > adev->fence_context = > dma_fence_context_alloc(AMDGPU_MAX_RINGS); > + adev->fw_vram_usage.size_in_kb = 0; > > adev->smc_rreg = &amdgpu_invalid_rreg; > adev->smc_wreg = &amdgpu_invalid_wreg; > @@ -2331,6 +2388,7 @@ void amdgpu_device_fini(struct amdgpu_device > *adev) > /* evict vram memory */ > amdgpu_bo_evict_vram(adev); > amdgpu_ib_pool_fini(adev); > + amdgpu_fw_reserve_vram_fini(adev); > amdgpu_fence_driver_fini(adev); > amdgpu_fbdev_fini(adev); > r = amdgpu_fini(adev); > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > index 6982bae..8602784 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > @@ -87,6 +87,14 @@ void amdgpu_ttm_placement_from_domain(struct > amdgpu_bo *abo, u32 domain) > > if (flags & AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS) > places[c].flags |= TTM_PL_FLAG_CONTIGUOUS; > + > + if (flags & AMDGPU_GEM_CREATE_VRAM_SPECIFIED) { > + /* specified vram must be contiguous */ > + places[c].flags |= TTM_PL_FLAG_CONTIGUOUS; > + places[c].fpfn = abo->start_address >> PAGE_SHIFT; > + places[c].lpfn = > + (abo->start_address + abo->gem_base.size) >> > PAGE_SHIFT; > + } Do we actually need this? Won't amdgpu_bo_pin_restructed take care of this without the need for the extra flag and code here? Alex > c++; > } > > @@ -284,6 +292,7 @@ void amdgpu_bo_free_kernel(struct amdgpu_bo > **bo, u64 *gpu_addr, > } > > static int amdgpu_bo_do_create(struct amdgpu_device *adev, > + u64 start_addr, > unsigned long size, int byte_align, > bool kernel, u32 domain, u64 flags, > struct sg_table *sg, > @@ -300,6 +309,7 @@ static int amdgpu_bo_do_create(struct > amdgpu_device *adev, > > page_align = roundup(byte_align, PAGE_SIZE) >> PAGE_SHIFT; > size = ALIGN(size, PAGE_SIZE); > + start_addr = ALIGN(start_addr, PAGE_SIZE); > > if (kernel) { > type = ttm_bo_type_kernel; > @@ -333,6 +343,7 @@ static int amdgpu_bo_do_create(struct > amdgpu_device *adev, > if (!kernel && bo->allowed_domains == > AMDGPU_GEM_DOMAIN_VRAM) > bo->allowed_domains |= AMDGPU_GEM_DOMAIN_GTT; > > + bo->start_address = start_addr; > bo->flags = flags; > > #ifdef CONFIG_X86_32 > @@ -418,6 +429,100 @@ static int amdgpu_bo_do_create(struct > amdgpu_device *adev, > return r; > } > > +/** > + * amdgpu_bo_create_vram_specified - > + * create BO at the specified place on the VRAM. > + * > + * @adev: amdgpu device object > + * @start_addr: start offset of the new BO > + * @size: size for the new BO > + * @byte_align: alignment for the new BO > + * @flags: addition flags for the BO > + * @bo_ptr: resulting BO > + * @gpu_addr: GPU addr of the pinned BO > + * @cpu_addr: optional CPU address mapping > + * > + * Allocates and pins a BO at the specified place on the VRAM. > + * > + * Returns 0 on success, negative error code otherwise. > + */ > +int amdgpu_bo_create_vram_specified(struct amdgpu_device *adev, > + u64 start_addr, unsigned long size, int byte_align, > + u64 flags, struct amdgpu_bo **bo_ptr, > + u64 *gpu_addr, void **cpu_addr) > +{ > + int r; > + /* specified memory must be in contiguous*/ > + flags |= AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS; > + flags |= AMDGPU_GEM_CREATE_VRAM_SPECIFIED; > + > + r = amdgpu_bo_do_create(adev, start_addr, size, byte_align, true, > + AMDGPU_GEM_DOMAIN_VRAM, flags, NULL, NULL, 0, > bo_ptr); > + if (r) > + return r; > + > + r = amdgpu_bo_reserve(*bo_ptr, false); > + if (r) > + goto error_reserve; > + r = amdgpu_bo_pin_restricted(*bo_ptr, > + AMDGPU_GEM_DOMAIN_VRAM, start_addr, (start_addr + > size), > + gpu_addr); > + if (r) > + goto error_pin; > + if (cpu_addr) { > + r = amdgpu_bo_kmap(*bo_ptr, > + cpu_addr); > + if (r) > + goto error_kmap; > + } > + > + amdgpu_bo_unreserve(*bo_ptr); > + > + return r; > +error_kmap: > + amdgpu_bo_unpin(*bo_ptr); > +error_pin: > + amdgpu_bo_unreserve(*bo_ptr); > +error_reserve: > + amdgpu_bo_unref(bo_ptr); > + if (cpu_addr) > + *cpu_addr = NULL; > + if (gpu_addr) > + *gpu_addr = 0; > + return r; > +} > + > +/** > + * amdgpu_bo_free_vram_specified - free BO for specified vram > + * > + * @bo: amdgpu BO to free > + * @gpu_addr : gpu address > + * @cpu_addr : cpu address if mapped > + * > + * unmaps and unpin a BO for specified vram. > + */ > +void amdgpu_bo_free_vram_specified(struct amdgpu_bo **bo, u64 > *gpu_addr, > + void **cpu_addr) > +{ > + if (*bo == NULL) > + return; > + > + if (likely(amdgpu_bo_reserve(*bo, true) == 0)) { > + if (cpu_addr) > + amdgpu_bo_kunmap(*bo); > + amdgpu_bo_unpin(*bo); > + amdgpu_bo_unreserve(*bo); > + } > + amdgpu_bo_unref(bo); > + > + if (gpu_addr) > + *gpu_addr = 0; > + > + if (cpu_addr) > + *cpu_addr = NULL; > +} > + > + > static int amdgpu_bo_create_shadow(struct amdgpu_device *adev, > unsigned long size, int byte_align, > struct amdgpu_bo *bo) > @@ -427,7 +532,7 @@ static int amdgpu_bo_create_shadow(struct > amdgpu_device *adev, > if (bo->shadow) > return 0; > > - r = amdgpu_bo_do_create(adev, size, byte_align, true, > + r = amdgpu_bo_do_create(adev, 0, size, byte_align, true, > AMDGPU_GEM_DOMAIN_GTT, > AMDGPU_GEM_CREATE_CPU_GTT_USWC | > AMDGPU_GEM_CREATE_SHADOW, > @@ -457,7 +562,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev, > uint64_t parent_flags = flags & ~AMDGPU_GEM_CREATE_SHADOW; > int r; > > - r = amdgpu_bo_do_create(adev, size, byte_align, kernel, domain, > + r = amdgpu_bo_do_create(adev, 0, size, byte_align, kernel, domain, > parent_flags, sg, resv, init_value, bo_ptr); > if (r) > return r; > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h > b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h > index 39b6bf6..88f2c5a 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h > @@ -78,6 +78,8 @@ struct amdgpu_bo { > void *metadata; > u32 metadata_size; > unsigned prime_shared_count; > + /* specified VRAM place */ > + u64 start_address; > /* list of all virtual address to which this bo is associated to */ > struct list_head va; > /* Constant after initialization */ > @@ -205,6 +207,12 @@ int amdgpu_bo_create_kernel(struct > amdgpu_device *adev, > u64 *gpu_addr, void **cpu_addr); > void amdgpu_bo_free_kernel(struct amdgpu_bo **bo, u64 *gpu_addr, > void **cpu_addr); > +int amdgpu_bo_create_vram_specified(struct amdgpu_device *adev, > + u64 start_addr, unsigned long size, int byte_align, > + u64 flags, struct amdgpu_bo **bo_ptr, > + u64 *gpu_addr, void **cpu_addr); > +void amdgpu_bo_free_vram_specified(struct amdgpu_bo **bo, u64 > *gpu_addr, > + void **cpu_addr); > int amdgpu_bo_kmap(struct amdgpu_bo *bo, void **ptr); > void *amdgpu_bo_kptr(struct amdgpu_bo *bo); > void amdgpu_bo_kunmap(struct amdgpu_bo *bo); > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > index 1086f03..a5253cf 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > @@ -1256,6 +1256,15 @@ int amdgpu_ttm_init(struct amdgpu_device > *adev) > /* Change the size here instead of the init above so only lpfn is > affected */ > amdgpu_ttm_set_active_vram_size(adev, adev- > >mc.visible_vram_size); > > + /* > + *The reserved vram for firmware must be pinned to the specified > + *place on the VRAM, so reserve it early. > + */ > + r = amdgpu_fw_reserve_vram_init(adev); > + if (r) { > + return r; > + } > + > r = amdgpu_bo_create_kernel(adev, adev->mc.stolen_size, > PAGE_SIZE, > AMDGPU_GEM_DOMAIN_VRAM, > &adev->stolen_vga_memory, > diff --git a/drivers/gpu/drm/amd/include/atombios.h > b/drivers/gpu/drm/amd/include/atombios.h > index 181a2c3..f696bbb 100644 > --- a/drivers/gpu/drm/amd/include/atombios.h > +++ b/drivers/gpu/drm/amd/include/atombios.h > @@ -4292,6 +4292,7 @@ typedef struct _ATOM_DPCD_INFO > #define ATOM_VRAM_OPERATION_FLAGS_SHIFT 30 > #define ATOM_VRAM_BLOCK_NEEDS_NO_RESERVATION 0x1 > #define ATOM_VRAM_BLOCK_NEEDS_RESERVATION 0x0 > +#define ATOM_VRAM_BLOCK_SRIOV_MSG_SHARE_RESERVATION 0x2 > > > /********************************************************** > *************************/ > // Structure used in VRAM_UsageByFirmwareTable > diff --git a/include/uapi/drm/amdgpu_drm.h > b/include/uapi/drm/amdgpu_drm.h > index 9f5bd97..8c213f0 100644 > --- a/include/uapi/drm/amdgpu_drm.h > +++ b/include/uapi/drm/amdgpu_drm.h > @@ -91,6 +91,8 @@ extern "C" { > #define AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS (1 << 5) > /* Flag that BO is always valid in this VM */ > #define AMDGPU_GEM_CREATE_VM_ALWAYS_VALID (1 << 6) > +/* Flag that allocating the BO at the specified place on VRAM*/ > +#define AMDGPU_GEM_CREATE_VRAM_SPECIFIED (1 << 7) We don't want to expose this to userspace. > > struct drm_amdgpu_gem_create_in { > /** the requested memory size */ > -- > 2.7.4 > > _______________________________________________ > amd-gfx mailing list > amd-gfx at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
