> NAH, for AI/VI the gds_addr is the bottom half of CSA area (8kb), but > we don't know for later chips ... > In this case we shouldn't store the offset anywhere, but rather just calculate it in the chip specific code from the CSA address when it is needed. > I prefer keep a member field in the struct for flexibility ... > We can still change it if we find that we need more flexibility here. Usually we try to avoid flexibility for the cost of complexity if we don't have a good reason for it. It just complicates the code and over the long term creates maintenance problems because you end of with a lot of only halve way implemented features. Regards, Christian. Am 10.01.2017 um 10:57 schrieb Liu, Monk: > > NAH, for AI/VI the gds_addr is the bottom half of CSA area (8kb), but > we don't know for later chips ... > > I prefer keep a member field in the struct for flexibility ... > > > regards with put those member into amdgpu_vm structure, if you really > prefer that way I'm fine by moving them, not too important anyway. > > > BR Monk > > > ------------------------------------------------------------------------ > *发件人:* amd-gfx 代表 Christian > König > *发送时间:* 2017年1月10日 17:51:46 > *收件人:* Liu, Monk; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org > *主题:* Re: 答复: [PATCH 2/7] drm/amdgpu:new field members for SRIOV > Am 10.01.2017 um 04:03 schrieb Liu, Monk: >> >> > +/* virtual MC address of CSA & GDS for each VM */ >> > + uint64_t vm_csa_addr; >> > + uint64_t vm_gds_addr; >> >> That should be constant, shouldn't it? >> >> >> >> [ML] do you mean "const uint64_t vm_csa_addr" ? of cause not if >> that's your point ... compiling error on that >> >> and strictly speaking that's not constant, the address is calculated ... >> > > No that wasn't what I meant. The address is constant for all VM's in > the system, isn't it? > > Just add a define similar to AMDGPU_VA_RESERVED_SIZE and stop > calculating it all together (BTW: AMDGPU_VA_RESERVED_SIZE should > probably be moved to amdgpu_vm.h). > > Regards, > Christian. > >> ------------------------------------------------------------------------ >> *发件人:* Christian König >> *发送时间:* 2017年1月9日 18:57:06 >> *收件人:* Liu, Monk; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org >> *主题:* Re: [PATCH 2/7] drm/amdgpu:new field members for SRIOV >> Am 09.01.2017 um 09:02 schrieb Monk Liu: >> > Change-Id: Ife0eff7b13b8b5946f005a39f6ecb8db1cb72c38 >> > Signed-off-by: Monk Liu >> > --- >> > drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h | 19 +++++++++++++++++++ >> > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 2 ++ >> > 2 files changed, 21 insertions(+) >> > >> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h >> > index 0d821d9..5aa7f0c 100644 >> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h >> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h >> > @@ -28,9 +28,28 @@ >> > #define AMDGPU_SRIOV_CAPS_ENABLE_IOV (1 << 1) /* sr-iov is >> enabled on this GPU */ >> > #define AMDGPU_SRIOV_CAPS_IS_VF (1 << 2) /* this GPU is a >> virtual function */ >> > #define AMDGPU_PASSTHROUGH_MODE (1 << 3) /* thw whole GPU >> is pass through for VM */ >> > + >> > +struct amdgpu_vm; >> > /* GPU virtualization */ >> > struct amdgpu_virt { >> > uint32_t caps; >> > + uint32_t csa_size; >> > + struct amdgpu_bo *csa_obj; >> > + uint64_t csa_vmid0_addr; >> > + uint64_t gds_vmid0_addr; >> > + int (*allocate_csa)(struct amdgpu_device *adev); >> > + void (*deallocate_csa)(struct amdgpu_device *adev); >> > + int (*map_csa)(struct amdgpu_device *adev, struct amdgpu_vm *vm); >> > + void (*unmap_csa)(struct amdgpu_device *adev, struct >> amdgpu_vm *vm); >> >> Why callbacks for this? >> >> > +}; >> > + >> > +struct amdgpu_vm_virt { >> > + /* each VM will map on CSA */ >> > + struct ttm_validate_buffer csa_tv; >> > + struct amdgpu_bo_va *csa_bo_va; >> >> Please put that directly into the amdgpu_vm structure. >> >> > + /* virtual MC address of CSA & GDS for each VM */ >> > + uint64_t vm_csa_addr; >> > + uint64_t vm_gds_addr; >> >> That should be constant, shouldn't it? >> >> Regards, >> Christian. >> >> > }; >> > >> > #define amdgpu_sriov_enabled(adev) \ >> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h >> > index 42a629b..27cbcbc 100644 >> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h >> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h >> > @@ -29,6 +29,7 @@ >> > #include "gpu_scheduler.h" >> > #include "amdgpu_sync.h" >> > #include "amdgpu_ring.h" >> > +#include "amdgpu_virt.h" >> > >> > struct amdgpu_bo_va; >> > struct amdgpu_job; >> > @@ -111,6 +112,7 @@ struct amdgpu_vm { >> > >> > /* client id */ >> > u64 client_id; >> > + struct amdgpu_vm_virt vm_virt; >> > }; >> > >> > struct amdgpu_vm_id { >> >> >> >> >> _______________________________________________ >> amd-gfx mailing list >> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx > >