[PATCH 1/2] drm/amdgpu: Enable seq64 manager and fix bugs
Christian König
christian.koenig at amd.com
Thu Nov 2 12:44:55 UTC 2023
Am 01.11.23 um 17:26 schrieb Arunpravin Paneer Selvam:
> - Enable the seq64 mapping sequence.
> - Fix wflinfo va conflict and other bugs.
>
> v1:
> - The seq64 area needs to be included in the AMDGPU_VA_RESERVED_SIZE
> otherwise the areas will conflict with user space allocations (Alex)
>
> - It needs to be mapped read only in the user VM (Alex)
>
> v2:
> - Instead of just one define for TOP/BOTTOM
> reserved space separate them into two (Christian)
>
> - Fix the CPU and VA calculations and while at it
> also cleanup error handling and kerneldoc (Christian)
>
> Signed-off-by: Christian König <christian.koenig at amd.com>
> Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
> Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam at amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 8 ++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.c | 69 +++++++++++---------
> drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.h | 9 ++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_umsch_mm.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 5 +-
> drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c | 5 +-
> drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 5 +-
> drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 5 +-
> 11 files changed, 68 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
> index 23d054526e7c..c7622efdafee 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
> @@ -28,7 +28,7 @@ uint64_t amdgpu_csa_vaddr(struct amdgpu_device *adev)
> {
> uint64_t addr = adev->vm_manager.max_pfn << AMDGPU_GPU_PAGE_SHIFT;
>
> - addr -= AMDGPU_VA_RESERVED_SIZE;
> + addr -= AMDGPU_VA_RESERVED_CSA_SIZE;
> addr = amdgpu_gmc_sign_extend(addr);
>
> return addr;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 849fffbb367d..f4455ed78e72 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -687,10 +687,10 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
> uint64_t vm_size;
> int r = 0;
>
> - if (args->va_address < AMDGPU_VA_RESERVED_SIZE) {
> + if (args->va_address < AMDGPU_VA_RESERVED_BOTTOM) {
> dev_dbg(dev->dev,
> "va_address 0x%llx is in reserved area 0x%llx\n",
> - args->va_address, AMDGPU_VA_RESERVED_SIZE);
> + args->va_address, AMDGPU_VA_RESERVED_BOTTOM);
> return -EINVAL;
> }
>
> @@ -706,7 +706,7 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
> args->va_address &= AMDGPU_GMC_HOLE_MASK;
>
> vm_size = adev->vm_manager.max_pfn * AMDGPU_GPU_PAGE_SIZE;
> - vm_size -= AMDGPU_VA_RESERVED_SIZE;
> + vm_size -= AMDGPU_VA_RESERVED_TOP;
> if (args->va_address + args->map_size > vm_size) {
> dev_dbg(dev->dev,
> "va_address 0x%llx is in top reserved area 0x%llx\n",
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index b5ebafd4a3ad..bb4aa14b868c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -894,14 +894,14 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
> dev_info->ids_flags |= AMDGPU_IDS_FLAGS_CONFORMANT_TRUNC_COORD;
>
> vm_size = adev->vm_manager.max_pfn * AMDGPU_GPU_PAGE_SIZE;
> - vm_size -= AMDGPU_VA_RESERVED_SIZE;
> + vm_size -= AMDGPU_VA_RESERVED_TOP;
>
> /* Older VCE FW versions are buggy and can handle only 40bits */
> if (adev->vce.fw_version &&
> adev->vce.fw_version < AMDGPU_VCE_FW_53_45)
> vm_size = min(vm_size, 1ULL << 40);
>
> - dev_info->virtual_address_offset = AMDGPU_VA_RESERVED_SIZE;
> + dev_info->virtual_address_offset = AMDGPU_VA_RESERVED_BOTTOM;
> dev_info->virtual_address_max =
> min(vm_size, AMDGPU_GMC_HOLE_START);
>
> @@ -1365,6 +1365,10 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv)
> goto error_vm;
> }
>
> + r = amdgpu_seq64_map(adev, &fpriv->vm, &fpriv->seq64_va);
> + if (r)
> + goto error_vm;
> +
> mutex_init(&fpriv->bo_list_lock);
> idr_init_base(&fpriv->bo_list_handles, 1);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> index 70fe3b39c004..108908a10b92 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> @@ -1325,7 +1325,7 @@ int amdgpu_mes_self_test(struct amdgpu_device *adev)
> goto error_fini;
> }
>
> - ctx_data.meta_data_gpu_addr = AMDGPU_VA_RESERVED_SIZE;
> + ctx_data.meta_data_gpu_addr = AMDGPU_VA_RESERVED_BOTTOM;
> r = amdgpu_mes_ctx_map_meta_data(adev, vm, &ctx_data);
> if (r) {
> DRM_ERROR("failed to map ctx meta data\n");
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.c
> index 64d4914001e0..63d8b68023be 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.c
> @@ -47,15 +47,33 @@
> * Returns:
> * 0 on success or a negative error code on failure
> */
> +
> +/**
> + * amdgpu_seq64_get_va_base - Get the seq64 va base address
> + *
> + * @adev: amdgpu_device pointer
> + *
> + * Returns:
> + * va base address on success
> + */
> +static inline u64 amdgpu_seq64_get_va_base(struct amdgpu_device *adev)
> +{
> + u64 addr = adev->vm_manager.max_pfn << AMDGPU_GPU_PAGE_SHIFT;
> +
> + addr -= AMDGPU_VA_RESERVED_TOP;
> +
> + return addr;
> +}
> +
> int amdgpu_seq64_map(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> - struct amdgpu_bo_va **bo_va, u64 seq64_addr,
> - uint32_t size)
> + struct amdgpu_bo_va **bo_va)
> {
> struct ttm_validate_buffer seq64_tv;
> struct amdgpu_bo_list_entry pd;
> struct ww_acquire_ctx ticket;
> struct list_head list;
> struct amdgpu_bo *bo;
> + u64 seq64_addr;
> int r;
>
> bo = adev->seq64.sbo;
> @@ -81,9 +99,9 @@ int amdgpu_seq64_map(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> goto error_vm;
> }
>
> - r = amdgpu_vm_bo_map(adev, *bo_va, seq64_addr, 0, size,
> - AMDGPU_PTE_READABLE | AMDGPU_PTE_WRITEABLE |
> - AMDGPU_PTE_EXECUTABLE);
> + seq64_addr = amdgpu_seq64_get_va_base(adev);
> + r = amdgpu_vm_bo_map(adev, *bo_va, seq64_addr, 0, AMDGPU_VA_RESERVED_SEQ64_SIZE,
> + AMDGPU_PTE_READABLE);
> if (r) {
> DRM_ERROR("failed to do bo_map on userq sem, err=%d\n", r);
> goto error_map;
> @@ -151,31 +169,25 @@ void amdgpu_seq64_unmap(struct amdgpu_device *adev, struct amdgpu_fpriv *fpriv)
> * amdgpu_seq64_alloc - Allocate a 64 bit memory
> *
> * @adev: amdgpu_device pointer
> - * @gpu_addr: allocated gpu VA start address
> - * @cpu_addr: allocated cpu VA start address
> + * @va: VA to access the seq in process address space
> + * @cpu_addr: CPU address to access the seq
> *
> * Alloc a 64 bit memory from seq64 pool.
> *
> * Returns:
> * 0 on success or a negative error code on failure
> */
> -int amdgpu_seq64_alloc(struct amdgpu_device *adev, u64 *gpu_addr,
> - u64 **cpu_addr)
> +int amdgpu_seq64_alloc(struct amdgpu_device *adev, u64 *va, u64 **cpu_addr)
> {
> unsigned long bit_pos;
> - u32 offset;
>
> bit_pos = find_first_zero_bit(adev->seq64.used, adev->seq64.num_sem);
> + if (bit_pos >= adev->seq64.num_sem)
> + return -ENOSPC;
>
> - if (bit_pos < adev->seq64.num_sem) {
> - __set_bit(bit_pos, adev->seq64.used);
> - offset = bit_pos << 6; /* convert to qw offset */
> - } else {
> - return -EINVAL;
> - }
> -
> - *gpu_addr = offset + AMDGPU_SEQ64_VADDR_START;
> - *cpu_addr = offset + adev->seq64.cpu_base_addr;
> + __set_bit(bit_pos, adev->seq64.used);
> + *va = bit_pos * sizeof(u64) + amdgpu_seq64_get_va_base(adev);
> + *cpu_addr = bit_pos + adev->seq64.cpu_base_addr;
>
> return 0;
> }
> @@ -184,20 +196,17 @@ int amdgpu_seq64_alloc(struct amdgpu_device *adev, u64 *gpu_addr,
> * amdgpu_seq64_free - Free the given 64 bit memory
> *
> * @adev: amdgpu_device pointer
> - * @gpu_addr: gpu start address to be freed
> + * @va: gpu start address to be freed
> *
> * Free the given 64 bit memory from seq64 pool.
> - *
> */
> -void amdgpu_seq64_free(struct amdgpu_device *adev, u64 gpu_addr)
> +void amdgpu_seq64_free(struct amdgpu_device *adev, u64 va)
> {
> - u32 offset;
> -
> - offset = gpu_addr - AMDGPU_SEQ64_VADDR_START;
> + unsigned long bit_pos;
>
> - offset >>= 6;
> - if (offset < adev->seq64.num_sem)
> - __clear_bit(offset, adev->seq64.used);
> + bit_pos = (va - amdgpu_seq64_get_va_base(adev)) / sizeof(u64);
> + if (bit_pos < adev->seq64.num_sem)
> + __clear_bit(bit_pos, adev->seq64.used);
> }
>
> /**
> @@ -236,7 +245,7 @@ int amdgpu_seq64_init(struct amdgpu_device *adev)
> * AMDGPU_MAX_SEQ64_SLOTS * sizeof(u64) * 8 = AMDGPU_MAX_SEQ64_SLOTS
> * 64bit slots
> */
> - r = amdgpu_bo_create_kernel(adev, AMDGPU_SEQ64_SIZE,
> + r = amdgpu_bo_create_kernel(adev, AMDGPU_VA_RESERVED_SEQ64_SIZE,
> PAGE_SIZE, AMDGPU_GEM_DOMAIN_GTT,
> &adev->seq64.sbo, NULL,
> (void **)&adev->seq64.cpu_base_addr);
> @@ -245,7 +254,7 @@ int amdgpu_seq64_init(struct amdgpu_device *adev)
> return r;
> }
>
> - memset(adev->seq64.cpu_base_addr, 0, AMDGPU_SEQ64_SIZE);
> + memset(adev->seq64.cpu_base_addr, 0, AMDGPU_VA_RESERVED_SEQ64_SIZE);
>
> adev->seq64.num_sem = AMDGPU_MAX_SEQ64_SLOTS;
> memset(&adev->seq64.used, 0, sizeof(adev->seq64.used));
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.h
> index 2196e72be508..4203b2ab318d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.h
> @@ -25,10 +25,9 @@
> #ifndef __AMDGPU_SEQ64_H__
> #define __AMDGPU_SEQ64_H__
>
> -#define AMDGPU_SEQ64_SIZE (2ULL << 20)
> -#define AMDGPU_MAX_SEQ64_SLOTS (AMDGPU_SEQ64_SIZE / (sizeof(u64) * 8))
> -#define AMDGPU_SEQ64_VADDR_OFFSET 0x50000
> -#define AMDGPU_SEQ64_VADDR_START (AMDGPU_VA_RESERVED_SIZE + AMDGPU_SEQ64_VADDR_OFFSET)
> +#include "amdgpu_vm.h"
> +
> +#define AMDGPU_MAX_SEQ64_SLOTS (AMDGPU_VA_RESERVED_SEQ64_SIZE / sizeof(u64))
>
> struct amdgpu_seq64 {
> struct amdgpu_bo *sbo;
> @@ -42,7 +41,7 @@ int amdgpu_seq64_init(struct amdgpu_device *adev);
> int amdgpu_seq64_alloc(struct amdgpu_device *adev, u64 *gpu_addr, u64 **cpu_addr);
> void amdgpu_seq64_free(struct amdgpu_device *adev, u64 gpu_addr);
> int amdgpu_seq64_map(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> - struct amdgpu_bo_va **bo_va, u64 seq64_addr, uint32_t size);
> + struct amdgpu_bo_va **bo_va);
> void amdgpu_seq64_unmap(struct amdgpu_device *adev, struct amdgpu_fpriv *fpriv);
>
> #endif
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_umsch_mm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_umsch_mm.c
> index f5fdde5181f7..a6ddced833ec 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_umsch_mm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_umsch_mm.c
> @@ -359,7 +359,7 @@ static int setup_umsch_mm_test(struct amdgpu_device *adev,
>
> memset(test->ring_data_cpu_addr, 0, sizeof(struct umsch_mm_test_ring_data));
>
> - test->ring_data_gpu_addr = AMDGPU_VA_RESERVED_SIZE;
> + test->ring_data_gpu_addr = AMDGPU_VA_RESERVED_BOTTOM;
> r = map_ring_data(adev, test->vm, test->ring_data_obj, &test->bo_va,
> test->ring_data_gpu_addr, sizeof(struct umsch_mm_test_ring_data));
> if (r)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index 9ea8b5b644e3..ed95f01a62e1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -134,7 +134,10 @@ struct amdgpu_mem_stats;
> #define AMDGPU_IS_MMHUB1(x) ((x) >= AMDGPU_MMHUB1_START && (x) < AMDGPU_MAX_VMHUBS)
>
> /* Reserve 2MB at top/bottom of address space for kernel use */
> -#define AMDGPU_VA_RESERVED_SIZE (2ULL << 20)
> +#define AMDGPU_VA_RESERVED_BOTTOM (2ULL << 20)
> +#define AMDGPU_VA_RESERVED_TOP (4ULL << 20)
Please adjust this define to
#define AMDGPU_VA_RESERVED_TOP (AMDGPU_VA_RESERVED_SEQ64_SIZE + \
AMDGPU_VA_RESERVED_CSA_SIZE)
With that done the patch is Reviewed-by: Christian König
<christian.koenig at amd.com>
Regards,
Christian.
> +#define AMDGPU_VA_RESERVED_CSA_SIZE (2ULL << 20)
> +#define AMDGPU_VA_RESERVED_SEQ64_SIZE (2ULL << 20)
>
> /* See vm_update_mode */
> #define AMDGPU_VM_USE_CPU_FOR_GFX (1 << 0)
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> index 7f66954fd302..f8711998e762 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> @@ -434,9 +434,10 @@ static void gmc_v6_0_set_prt(struct amdgpu_device *adev, bool enable)
> WREG32(mmVM_PRT_CNTL, tmp);
>
> if (enable) {
> - uint32_t low = AMDGPU_VA_RESERVED_SIZE >> AMDGPU_GPU_PAGE_SHIFT;
> + uint32_t low = AMDGPU_VA_RESERVED_BOTTOM >>
> + AMDGPU_GPU_PAGE_SHIFT;
> uint32_t high = adev->vm_manager.max_pfn -
> - (AMDGPU_VA_RESERVED_SIZE >> AMDGPU_GPU_PAGE_SHIFT);
> + (AMDGPU_VA_RESERVED_TOP >> AMDGPU_GPU_PAGE_SHIFT);
>
> WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, low);
> WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, low);
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> index 61ca1a82b651..f86ed37cad6e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> @@ -562,9 +562,10 @@ static void gmc_v7_0_set_prt(struct amdgpu_device *adev, bool enable)
> WREG32(mmVM_PRT_CNTL, tmp);
>
> if (enable) {
> - uint32_t low = AMDGPU_VA_RESERVED_SIZE >> AMDGPU_GPU_PAGE_SHIFT;
> + uint32_t low = AMDGPU_VA_RESERVED_BOTTOM >>
> + AMDGPU_GPU_PAGE_SHIFT;
> uint32_t high = adev->vm_manager.max_pfn -
> - (AMDGPU_VA_RESERVED_SIZE >> AMDGPU_GPU_PAGE_SHIFT);
> + (AMDGPU_VA_RESERVED_TOP >> AMDGPU_GPU_PAGE_SHIFT);
>
> WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, low);
> WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, low);
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> index fa59749c2aef..eab26db0f398 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> @@ -776,9 +776,10 @@ static void gmc_v8_0_set_prt(struct amdgpu_device *adev, bool enable)
> WREG32(mmVM_PRT_CNTL, tmp);
>
> if (enable) {
> - uint32_t low = AMDGPU_VA_RESERVED_SIZE >> AMDGPU_GPU_PAGE_SHIFT;
> + uint32_t low = AMDGPU_VA_RESERVED_BOTTOM >>
> + AMDGPU_GPU_PAGE_SHIFT;
> uint32_t high = adev->vm_manager.max_pfn -
> - (AMDGPU_VA_RESERVED_SIZE >> AMDGPU_GPU_PAGE_SHIFT);
> + (AMDGPU_VA_RESERVED_TOP >> AMDGPU_GPU_PAGE_SHIFT);
>
> WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, low);
> WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, low);
More information about the amd-gfx
mailing list