[PATCH 3/3] drm/amdgpu: move static CSA address to top of address space
Liu, Monk
Monk.Liu at amd.com
Tue Jan 23 13:44:59 UTC 2018
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>
________________________________
From: Koenig, Christian
Sent: Tuesday, January 23, 2018 8:51:21 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
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 at gmail.com><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 at lists.freedesktop.org] 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/20180123/3480a542/attachment-0001.html>
More information about the amd-gfx
mailing list