sorry, I don't have time running DRM-NEXT kernel currently, we are doing new release and bug fixing for custome, theoretically it's ok ay to move CSA address to another place. I'll try it in next SRIOV branch rebase BR Monk ________________________________ From: Koenig, Christian Sent: Tuesday, January 23, 2018 10:45:40 PM To: Liu, Monk; He, Roger; amd-gfx at lists.freedesktop.org Subject: Re: [PATCH 3/3] drm/amdgpu: move static CSA address to top of address space Thanks, can you give the patches a try as well? I don't have access to SRIOV hardware to test them. Christian. Am 23.01.2018 um 14:44 schrieb Liu, Monk: looks this is HSA feature to me that CPU and GPU share the same virtual address .... Ack-by: Monk Liu <monk.liu at amd.com><mailto:monk.liu at amd.com> ________________________________ From: Koenig, Christian Sent: Tuesday, January 23, 2018 8:51:21 PM To: Liu, Monk; He, Roger; amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org> Subject: Re: [PATCH 3/3] drm/amdgpu: move static CSA address to top of address space The ATC (or alternatively HMM) maps the CPU addresses into the GPU address space. Now on Linux the lower range (0x0-0x7fffffffffff) is used for the userspace address space on the CPU, so when we want to use that on the GPU it must be free of other mappings. GFX8 and GFX7 hardware had a separate flag which address space to use in the memory instructions, but on GFX9 it's all one big shared address space and we need to make sure that we don't kick each others feet. Had to move the addresses Mesa use as well because of this and older VCE firmware unfortunately was buggy regarding this as well. Regards, Christian. Am 23.01.2018 um 13:43 schrieb Liu, Monk: OK I see, but why HMM/ATC/SVN want low 8MB range ? why they cannot use high range address ? ________________________________ From: Christian König <ckoenig.leichtzumerken@xxxxxxxxx><mailto:ckoenig.leichtzumerken at gmail.com> Sent: Tuesday, January 23, 2018 4:26:21 PM To: Liu, Monk; He, Roger; amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org> Subject: Re: [PATCH 3/3] drm/amdgpu: move static CSA address to top of address space Any bug introduced by original design ? I don't see why 8MB- 8KB as the start vm address of CSA has any trouble compared with top range ? On gfx9 we need the lower address range (0x0-0x7fffffffffff) for HMM/ATC/SVM. If we map the 8k CSA in there and hit it by accident then the GPU will not do what it is supposed to do :) I could limit the change to only gfx9, but I think the code will be simpler if we change both gfx8 and gfx9. Besides, please note that you also need modify gfx8/9 source to align emit_cs/de_meta with your change That was the design flaw I was talking about. In other words I couldn't find the code where that address was actually used. We should use the macro define in those functions as well. Going to send fixed for this in a minute. Thanks, Christian. Am 23.01.2018 um 04:23 schrieb Liu, Monk: anyway I prefer no change on that part unless there issues or bug need to fix by the change. the CSA address is for use by CPG h/w not s/w, and since 8MB is a reserved range for each VM I don't see it's a design flaw with it ________________________________ From: Liu, Monk Sent: Tuesday, January 23, 2018 11:20:35 AM To: He, Roger; Christian König; amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org> Subject: Re: [PATCH 3/3] drm/amdgpu: move static CSA address to top of address space Any bug introduced by original design ? I don't see why 8MB- 8KB as the start vm address of CSA has any trouble compared with top range ? Besides, please note that you also need modify gfx8/9 source to align emit_cs/de_meta with your change ________________________________ From: He, Roger Sent: Tuesday, January 23, 2018 10:36:31 AM To: Christian König; Liu, Monk; amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org> Subject: RE: [PATCH 3/3] drm/amdgpu: move static CSA address to top of address space -----Original Message----- From: amd-gfx [mailto:amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of Christian K?nig Sent: Monday, January 22, 2018 6:44 PM To: Liu, Monk <Monk.Liu at amd.com><mailto:Monk.Liu at amd.com>; amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org> Subject: [PATCH 3/3] drm/amdgpu: move static CSA address to top of address space There seems to be a design flaw with this since the address of the static CSA is never exported anywhere. Signed-off-by: Christian König <christian.koenig at amd.com><mailto:christian.koenig at amd.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 15 +++++++-------- drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h | 6 ++++-- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c index e7dfb7b44b4b..c7d24af03e3e 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c @@ -55,11 +55,10 @@ void amdgpu_free_static_csa(struct amdgpu_device *adev) { /* * amdgpu_map_static_csa should be called during amdgpu_vm_init - * it maps virtual address "AMDGPU_VA_RESERVED_SIZE - AMDGPU_CSA_SIZE" - * to this VM, and each command submission of GFX should use this virtual - * address within META_DATA init package to support SRIOV gfx preemption. + * it maps virtual address "AMDGPU_CSA_VADDR" to this VM, and each + command + * submission of GFX should use this virtual address within META_DATA + init + * package to support SRIOV gfx preemption. */ - int amdgpu_map_static_csa(struct amdgpu_device *adev, struct amdgpu_vm *vm, struct amdgpu_bo_va **bo_va) { @@ -90,7 +89,7 @@ int amdgpu_map_static_csa(struct amdgpu_device *adev, struct amdgpu_vm *vm, return -ENOMEM; } - r = amdgpu_vm_alloc_pts(adev, (*bo_va)->base.vm, AMDGPU_CSA_VADDR, + r = amdgpu_vm_alloc_pts(adev, (*bo_va)->base.vm, +AMDGPU_CSA_VADDR(adev), AMDGPU_CSA_SIZE); if (r) { DRM_ERROR("failed to allocate pts for static CSA, err=%d\n", r); @@ -99,9 +98,9 @@ int amdgpu_map_static_csa(struct amdgpu_device *adev, struct amdgpu_vm *vm, return r; } - r = amdgpu_vm_bo_map(adev, *bo_va, AMDGPU_CSA_VADDR, 0, AMDGPU_CSA_SIZE, - AMDGPU_PTE_READABLE | AMDGPU_PTE_WRITEABLE | - AMDGPU_PTE_EXECUTABLE); + r = amdgpu_vm_bo_map(adev, *bo_va, AMDGPU_CSA_VADDR(adev), 0, + AMDGPU_CSA_SIZE, AMDGPU_PTE_READABLE | + AMDGPU_PTE_WRITEABLE | AMDGPU_PTE_EXECUTABLE); if (r) { DRM_ERROR("failed to do bo_map on static CSA, err=%d\n", r); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h index 6a83425aa9ed..499362b55e45 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h @@ -251,8 +251,10 @@ struct amdgpu_virt { uint32_t gim_feature; }; -#define AMDGPU_CSA_SIZE (8 * 1024) -#define AMDGPU_CSA_VADDR (AMDGPU_VA_RESERVED_SIZE - AMDGPU_CSA_SIZE) +#define AMDGPU_CSA_SIZE (8 * 1024) +#define AMDGPU_CSA_VADDR(adev) \ + (((adev)->vm_manager.max_pfn << AMDGPU_GPU_PAGE_SHIFT) + \ + AMDGPU_VA_RESERVED_SIZE) If I understand your intention correctly, it should be that: + (((adev)->vm_manager.max_pfn << AMDGPU_GPU_PAGE_SHIFT) - \ + AMDGPU_VA_RESERVED_SIZE) Thanks Roger(Hongbo.He) #define amdgpu_sriov_enabled(adev) \ ((adev)->virt.caps & AMDGPU_SRIOV_CAPS_ENABLE_IOV) -- 2.14.1 _______________________________________________ amd-gfx mailing list amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org> https://lists.freedesktop.org/mailman/listinfo/amd-gfx -------------- next part -------------- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20180124/a6163a58/attachment-0001.html>
