Am 23.08.2018 um 05:07 schrieb Zhang, Jerry (Junwei): > On 08/22/2018 11:05 PM, Christian König wrote: >> Add a helper to get the root PD address and remove the workarounds from >> the GMC9 code for that. >> >> Signed-off-by: Christian König <christian.koenig at amd.com> >> --- >>  drivers/gpu/drm/amd/amdgpu/Makefile          | 3 +- >>  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 5 +- >>  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c       | 2 +- >>  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c      | 47 +++++++++++++++++++ >>  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h      | 2 + >>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c      | 2 +- >>  drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c     | 7 +-- >>  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c        | 4 -- >>  drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c      | 7 +-- >>  9 files changed, 56 insertions(+), 23 deletions(-) >>  create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile >> b/drivers/gpu/drm/amd/amdgpu/Makefile >> index 860cb8731c7c..d2bafabe585d 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/Makefile >> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile >> @@ -51,7 +51,8 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \ >>      amdgpu_prime.o amdgpu_vm.o amdgpu_ib.o amdgpu_pll.o \ >>      amdgpu_ucode.o amdgpu_bo_list.o amdgpu_ctx.o amdgpu_sync.o \ >>      amdgpu_gtt_mgr.o amdgpu_vram_mgr.o amdgpu_virt.o >> amdgpu_atomfirmware.o \ >> -   amdgpu_vf_error.o amdgpu_sched.o amdgpu_debugfs.o amdgpu_ids.o >> +   amdgpu_vf_error.o amdgpu_sched.o amdgpu_debugfs.o amdgpu_ids.o \ >> +   amdgpu_gmc.o >> >>  # add asic specific block >>  amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o kv_smc.o kv_dpm.o \ >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c >> index 7eadc58231f2..2e2393fe09b2 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c >> @@ -364,7 +364,6 @@ static int vm_validate_pt_pd_bos(struct amdgpu_vm >> *vm) >>      struct amdgpu_bo *pd = vm->root.base.bo; >>      struct amdgpu_device *adev = amdgpu_ttm_adev(pd->tbo.bdev); >>      struct amdgpu_vm_parser param; >> -   uint64_t addr, flags = AMDGPU_PTE_VALID; >>      int ret; >> >>      param.domain = AMDGPU_GEM_DOMAIN_VRAM; >> @@ -383,9 +382,7 @@ static int vm_validate_pt_pd_bos(struct amdgpu_vm >> *vm) >>          return ret; >>      } >> >> -   addr = amdgpu_bo_gpu_offset(vm->root.base.bo); >> -   amdgpu_gmc_get_vm_pde(adev, -1, &addr, &flags); >> -   vm->pd_phys_addr = addr; >> +   vm->pd_phys_addr = amdgpu_gmc_pd_addr(vm->root.base.bo); >> >>      if (vm->use_cpu_for_update) { >>          ret = amdgpu_bo_kmap(pd, NULL); >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >> index 17bf63f93c93..d268035cf2f3 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >> @@ -946,7 +946,7 @@ static int amdgpu_cs_vm_handling(struct >> amdgpu_cs_parser *p) >>      if (r) >>          return r; >> >> -   p->job->vm_pd_addr = amdgpu_bo_gpu_offset(vm->root.base.bo); >> +   p->job->vm_pd_addr = amdgpu_gmc_pd_addr(vm->root.base.bo); >> >>      if (amdgpu_vm_debug) { >>          /* Invalidate all BOs to test for userspace bugs */ >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c >> new file mode 100644 >> index 000000000000..36058feac64f >> --- /dev/null >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c >> @@ -0,0 +1,47 @@ >> +/* >> + * Copyright 2018 Advanced Micro Devices, Inc. >> + * All Rights Reserved. >> + * >> + * Permission is hereby granted, free of charge, to any person >> obtaining a >> + * copy of this software and associated documentation files (the >> + * "Software"), to deal in the Software without restriction, including >> + * without limitation the rights to use, copy, modify, merge, publish, >> + * distribute, sub license, and/or sell copies of the Software, and to >> + * permit persons to whom the Software is furnished to do so, >> subject to >> + * the following conditions: >> + * >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, >> EXPRESS OR >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF >> MERCHANTABILITY, >> + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO >> EVENT SHALL >> + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR >> ANY CLAIM, >> + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, >> TORT OR >> + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE >> SOFTWARE OR THE >> + * USE OR OTHER DEALINGS IN THE SOFTWARE. >> + * >> + * The above copyright notice and this permission notice (including the >> + * next paragraph) shall be included in all copies or substantial >> portions >> + * of the Software. >> + * >> + */ >> + >> +#include "amdgpu.h" >> + >> +/** >> + * amdgpu_gmc_pd_addr - return the address of the root directory >> + * >> + */ >> +uint64_t amdgpu_gmc_pd_addr(struct amdgpu_bo *bo) > > If the func is going to handle all pd address, it's better to be > called in gmc6,7,8 as well. > >> +{ >> +   struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev); >> +   uint64_t pd_addr; >> + >> +   pd_addr = amdgpu_bo_gpu_offset(bo); >> +   /* TODO: move that into ASIC specific code */ > > Sounds it could be a func(.gmc_pd_addr) in the group of amdgpu_gmc_funcs? GMC6,7 and 8 want an MC address, but GMC9 uses a PDE for the root PD address. So the idea here is to get all ASIC specific flush functions to accept a PDE, cause that includes all the information you need and not only the MC address. Regards, Christian. > > Regards, > Jerry > >> +   if (adev->asic_type >= CHIP_VEGA10) { >> +       uint64_t flags = AMDGPU_PTE_VALID; >> + >> +       amdgpu_gmc_get_vm_pde(adev, -1, &pd_addr, &flags); >> +       pd_addr |= flags; >> +   } >> +   return pd_addr; >> +} >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h >> index f3ea0a6d4660..7c469cce0498 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h >> @@ -131,4 +131,6 @@ static inline bool >> amdgpu_gmc_vram_full_visible(struct amdgpu_gmc *gmc) >>      return (gmc->real_vram_size == gmc->visible_vram_size); >>  } >> >> +uint64_t amdgpu_gmc_pd_addr(struct amdgpu_bo *bo); >> + >>  #endif >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c >> index e7f73deed975..eb08a03b82a0 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c >> @@ -2049,7 +2049,7 @@ int amdgpu_copy_buffer(struct amdgpu_ring >> *ring, uint64_t src_offset, >>          return r; >> >>      if (vm_needs_flush) { >> -       job->vm_pd_addr = amdgpu_bo_gpu_offset(adev->gart.bo); >> +       job->vm_pd_addr = amdgpu_gmc_pd_addr(adev->gart.bo); >>          job->vm_needs_flush = true; >>      } >>      if (resv) { >> diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c >> b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c >> index 2baab7e69ef5..3403ded39d13 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c >> +++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c >> @@ -37,12 +37,7 @@ u64 gfxhub_v1_0_get_mc_fb_offset(struct >> amdgpu_device *adev) >> >>  static void gfxhub_v1_0_init_gart_pt_regs(struct amdgpu_device *adev) >>  { >> -   uint64_t value = amdgpu_bo_gpu_offset(adev->gart.bo); >> - >> -   BUG_ON(value & (~0x0000FFFFFFFFF000ULL)); >> -   value -= adev->gmc.vram_start + adev->vm_manager.vram_base_offset; >> -   value &= 0x0000FFFFFFFFF000ULL; >> -   value |= 0x1; /*valid bit*/ >> +   uint64_t value = amdgpu_gmc_pd_addr(adev->gart.bo); >> >>      WREG32_SOC15(GC, 0, mmVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_LO32, >>               lower_32_bits(value)); >> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c >> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c >> index 3393a329fc9c..e12e007cf7d9 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c >> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c >> @@ -436,12 +436,8 @@ static uint64_t >> gmc_v9_0_emit_flush_gpu_tlb(struct amdgpu_ring *ring, >>      struct amdgpu_device *adev = ring->adev; >>      struct amdgpu_vmhub *hub = &adev->vmhub[ring->funcs->vmhub]; >>      uint32_t req = gmc_v9_0_get_invalidate_req(vmid); >> -   uint64_t flags = AMDGPU_PTE_VALID; >>      unsigned eng = ring->vm_inv_eng; >> >> -   amdgpu_gmc_get_vm_pde(adev, -1, &pd_addr, &flags); >> -   pd_addr |= flags; >> - >>      amdgpu_ring_emit_wreg(ring, hub->ctx0_ptb_addr_lo32 + (2 * vmid), >>                    lower_32_bits(pd_addr)); >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c >> b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c >> index 800ec4687f13..5f6a9c85488f 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c >> +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c >> @@ -47,12 +47,7 @@ u64 mmhub_v1_0_get_fb_location(struct >> amdgpu_device *adev) >> >>  static void mmhub_v1_0_init_gart_pt_regs(struct amdgpu_device *adev) >>  { >> -   uint64_t value = amdgpu_bo_gpu_offset(adev->gart.bo); >> - >> -   BUG_ON(value & (~0x0000FFFFFFFFF000ULL)); >> -   value -= adev->gmc.vram_start + adev->vm_manager.vram_base_offset; >> -   value &= 0x0000FFFFFFFFF000ULL; >> -   value |= 0x1; /* valid bit */ >> +   uint64_t value = amdgpu_gmc_pd_addr(adev->gart.bo); >> >>      WREG32_SOC15(MMHUB, 0, mmVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_LO32, >>               lower_32_bits(value)); >>