From: "Zhang, Jerry (Junwei)" <Jerry.Zhang-5C7GfCeVMHo@public.gmane.org>
To: "Christian König"
<ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: Re: [PATCH 1/6] drm/amdgpu: correctly sign extend 48bit addresses v3
Date: Fri, 31 Aug 2018 09:51:36 +0800 [thread overview]
Message-ID: <5B889F28.8020202@amd.com> (raw)
In-Reply-To: <20180830121414.3397-1-christian.koenig-5C7GfCeVMHo@public.gmane.org>
Patch 1~5 are
Reviewed-by: Junwei Zhang <Jerry.Zhang@amd.com>
Patch 6 is
Acked-by: Junwei Zhang <Jerry.Zhang@amd.com>
BTW, [PATCH 4/6] drm/amdgpu: manually map the shadow BOs again
with this patch, the user cannot create a shadow bo with gart address.
anyway, I cannot image that use case either.
Regards,
Jerry
On 08/30/2018 08:14 PM, Christian König wrote:
> Correct sign extend the GMC addresses to 48bit.
>
> v2: sign extending turned out easier than thought.
> v3: clean up the defines and move them into amdgpu_gmc.h as well
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 10 ++++-----
> drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h | 26 ++++++++++++++++++++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 8 +++----
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 6 ++---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 7 +++---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 13 -----------
> 9 files changed, 44 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> index 8c652ecc4f9a..bc5ccfca68c5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> @@ -135,7 +135,7 @@ void amdgpu_amdkfd_device_init(struct amdgpu_device *adev)
> .num_queue_per_pipe = adev->gfx.mec.num_queue_per_pipe,
> .gpuvm_size = min(adev->vm_manager.max_pfn
> << AMDGPU_GPU_PAGE_SHIFT,
> - AMDGPU_VA_HOLE_START),
> + AMDGPU_GMC_HOLE_START),
> .drm_render_minor = adev->ddev->render->index
> };
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index dd734970e167..ef2bfc04b41c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -835,7 +835,7 @@ static int amdgpu_cs_vm_handling(struct amdgpu_cs_parser *p)
> if (chunk->chunk_id != AMDGPU_CHUNK_ID_IB)
> continue;
>
> - va_start = chunk_ib->va_start & AMDGPU_VA_HOLE_MASK;
> + va_start = chunk_ib->va_start & AMDGPU_GMC_HOLE_MASK;
> r = amdgpu_cs_find_mapping(p, va_start, &aobj, &m);
> if (r) {
> DRM_ERROR("IB va_start is invalid\n");
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 71792d820ae0..d30a0838851b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -572,16 +572,16 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
> return -EINVAL;
> }
>
> - if (args->va_address >= AMDGPU_VA_HOLE_START &&
> - args->va_address < AMDGPU_VA_HOLE_END) {
> + if (args->va_address >= AMDGPU_GMC_HOLE_START &&
> + args->va_address < AMDGPU_GMC_HOLE_END) {
> dev_dbg(&dev->pdev->dev,
> "va_address 0x%LX is in VA hole 0x%LX-0x%LX\n",
> - args->va_address, AMDGPU_VA_HOLE_START,
> - AMDGPU_VA_HOLE_END);
> + args->va_address, AMDGPU_GMC_HOLE_START,
> + AMDGPU_GMC_HOLE_END);
> return -EINVAL;
> }
>
> - args->va_address &= AMDGPU_VA_HOLE_MASK;
> + args->va_address &= AMDGPU_GMC_HOLE_MASK;
>
> if ((args->flags & ~valid_flags) && (args->flags & ~prt_flags)) {
> dev_dbg(&dev->pdev->dev, "invalid flags combination 0x%08X\n",
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> index 72fcc9338f5e..48715dd5808a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> @@ -30,6 +30,19 @@
>
> #include "amdgpu_irq.h"
>
> +/* VA hole for 48bit addresses on Vega10 */
> +#define AMDGPU_GMC_HOLE_START 0x0000800000000000ULL
> +#define AMDGPU_GMC_HOLE_END 0xffff800000000000ULL
> +
> +/*
> + * Hardware is programmed as if the hole doesn't exists with start and end
> + * address values.
> + *
> + * This mask is used to remove the upper 16bits of the VA and so come up with
> + * the linear addr value.
> + */
> +#define AMDGPU_GMC_HOLE_MASK 0x0000ffffffffffffULL
> +
> struct firmware;
>
> /*
> @@ -131,6 +144,19 @@ static inline bool amdgpu_gmc_vram_full_visible(struct amdgpu_gmc *gmc)
> return (gmc->real_vram_size == gmc->visible_vram_size);
> }
>
> +/**
> + * amdgpu_gmc_sign_extend - sign extend the given gmc address
> + *
> + * @addr: address to extend
> + */
> +static inline uint64_t amdgpu_gmc_sign_extend(uint64_t addr)
> +{
> + if (addr >= AMDGPU_GMC_HOLE_START)
> + addr |= AMDGPU_GMC_HOLE_END;
> +
> + return addr;
> +}
> +
> void amdgpu_gmc_get_pde_for_bo(struct amdgpu_bo *bo, int level,
> uint64_t *addr, uint64_t *flags);
> uint64_t amdgpu_gmc_pd_addr(struct amdgpu_bo *bo);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index 9c4e45936ade..29ac3873eeb0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -655,11 +655,11 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
>
> dev_info.virtual_address_offset = AMDGPU_VA_RESERVED_SIZE;
> dev_info.virtual_address_max =
> - min(vm_size, AMDGPU_VA_HOLE_START);
> + min(vm_size, AMDGPU_GMC_HOLE_START);
>
> - if (vm_size > AMDGPU_VA_HOLE_START) {
> - dev_info.high_va_offset = AMDGPU_VA_HOLE_END;
> - dev_info.high_va_max = AMDGPU_VA_HOLE_END | vm_size;
> + if (vm_size > AMDGPU_GMC_HOLE_START) {
> + dev_info.high_va_offset = AMDGPU_GMC_HOLE_END;
> + dev_info.high_va_max = AMDGPU_GMC_HOLE_END | vm_size;
> }
> dev_info.virtual_address_alignment = max((int)PAGE_SIZE, AMDGPU_GPU_PAGE_SIZE);
> dev_info.pte_fragment_size = (1 << adev->vm_manager.fragment_size) * AMDGPU_GPU_PAGE_SIZE;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index b5f20b42439e..0cbf651a88a6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -1368,7 +1368,7 @@ u64 amdgpu_bo_gpu_offset(struct amdgpu_bo *bo)
> WARN_ON_ONCE(bo->tbo.mem.mem_type == TTM_PL_VRAM &&
> !(bo->flags & AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS));
>
> - return bo->tbo.offset;
> + return amdgpu_gmc_sign_extend(bo->tbo.offset);
> }
>
> /**
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> index 38856365580d..f2f358aa0597 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> @@ -28,9 +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;
> -
> - if (addr >= AMDGPU_VA_HOLE_START)
> - addr |= AMDGPU_VA_HOLE_END;
> + addr = amdgpu_gmc_sign_extend(addr);
>
> return addr;
> }
> @@ -73,7 +71,7 @@ void amdgpu_free_static_csa(struct amdgpu_device *adev) {
> int amdgpu_map_static_csa(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> struct amdgpu_bo_va **bo_va)
> {
> - uint64_t csa_addr = amdgpu_csa_vaddr(adev) & AMDGPU_VA_HOLE_MASK;
> + uint64_t csa_addr = amdgpu_csa_vaddr(adev) & AMDGPU_GMC_HOLE_MASK;
> struct ww_acquire_ctx ticket;
> struct list_head list;
> struct amdgpu_bo_list_entry pd;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 72f8c750e128..379903c7fddf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -403,7 +403,7 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
> if (level == adev->vm_manager.root_level) {
> ats_entries = amdgpu_vm_level_shift(adev, level);
> ats_entries += AMDGPU_GPU_PAGE_SHIFT;
> - ats_entries = AMDGPU_VA_HOLE_START >> ats_entries;
> + ats_entries = AMDGPU_GMC_HOLE_START >> ats_entries;
> ats_entries = min(ats_entries, entries);
> entries -= ats_entries;
> } else {
> @@ -632,7 +632,7 @@ int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
> eaddr = saddr + size - 1;
>
> if (vm->pte_support_ats)
> - ats = saddr < AMDGPU_VA_HOLE_START;
> + ats = saddr < AMDGPU_GMC_HOLE_START;
>
> saddr /= AMDGPU_GPU_PAGE_SIZE;
> eaddr /= AMDGPU_GPU_PAGE_SIZE;
> @@ -1929,7 +1929,8 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
> struct amdgpu_bo_va_mapping, list);
> list_del(&mapping->list);
>
> - if (vm->pte_support_ats && mapping->start < AMDGPU_VA_HOLE_START)
> + if (vm->pte_support_ats &&
> + mapping->start < AMDGPU_GMC_HOLE_START)
> init_pte_value = AMDGPU_PTE_DEFAULT_ATC;
>
> r = amdgpu_vm_bo_update_mapping(adev, NULL, NULL, vm,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index 62116fa44718..ae61fa587180 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -101,19 +101,6 @@ struct amdgpu_bo_list_entry;
> /* hardcode that limit for now */
> #define AMDGPU_VA_RESERVED_SIZE (1ULL << 20)
>
> -/* VA hole for 48bit addresses on Vega10 */
> -#define AMDGPU_VA_HOLE_START 0x0000800000000000ULL
> -#define AMDGPU_VA_HOLE_END 0xffff800000000000ULL
> -
> -/*
> - * Hardware is programmed as if the hole doesn't exists with start and end
> - * address values.
> - *
> - * This mask is used to remove the upper 16bits of the VA and so come up with
> - * the linear addr value.
> - */
> -#define AMDGPU_VA_HOLE_MASK 0x0000ffffffffffffULL
> -
> /* max vmids dedicated for process */
> #define AMDGPU_VM_MAX_RESERVED_VMID 1
>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
prev parent reply other threads:[~2018-08-31 1:51 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-08-30 12:14 [PATCH 1/6] drm/amdgpu: correctly sign extend 48bit addresses v3 Christian König
[not found] ` <20180830121414.3397-1-christian.koenig-5C7GfCeVMHo@public.gmane.org>
2018-08-30 12:14 ` [PATCH 2/6] drm/amdgpu: add amdgpu_gmc_agp_location v3 Christian König
[not found] ` <20180830121414.3397-2-christian.koenig-5C7GfCeVMHo@public.gmane.org>
2018-08-30 18:09 ` Felix Kuehling
2018-08-30 12:14 ` [PATCH 3/6] drm/amdgpu: use the AGP aperture for system memory access v2 Christian König
2018-08-30 12:14 ` [PATCH 4/6] drm/amdgpu: manually map the shadow BOs again Christian König
2018-08-30 12:14 ` [PATCH 5/6] drm/amdgpu: enable AGP aperture for GMC9 Christian König
2018-08-30 12:14 ` [PATCH 6/6] drm/amdgpu: try to make kernel allocations USWC Christian König
2018-08-31 1:51 ` Zhang, Jerry (Junwei) [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5B889F28.8020202@amd.com \
--to=jerry.zhang-5c7gfcevmho@public.gmane.org \
--cc=amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org \
--cc=ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.