<html> <head> <meta content="text/html; charset=utf-8" http-equiv="Content-Type"> </head> <body bgcolor="#FFFFFF" text="#000000"> <div class="moz-cite-prefix"> <blockquote type="cite"> NAH, for AI/VI the gds_addr is the bottom half of CSA area (8kb), but we don't know for later chips ... </blockquote> 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. <blockquote type="cite"> I prefer keep a member field in the struct for flexibility ... </blockquote> 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: </div> <blockquote cite="mid:BY2PR1201MB1110F2B6E7FB093F369ACA2884670@BY2PR1201MB1110.namprd12.prod.outlook.com" type="cite"> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> <style type="text/css" style="display:none;"></style> <div id="divtagdefaultwrapper" style="font-size:12pt;color:#000000;font-family:Calibri,Arial,Helvetica,sans-serif;" dir="ltr"> 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 </div> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr">发件人: amd-gfx <a class="moz-txt-link-rfc2396E" href="mailto:amd-gfx-bounces@lists.freedesktop.org"><amd-gfx-bounces@lists.freedesktop.org></a> 代表 Christian König <a class="moz-txt-link-rfc2396E" href="mailto:deathsimple@vodafone.de"><deathsimple@vodafone.de></a> 发送时间: 2017年1月10日 17:51:46 收件人: Liu, Monk; <a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> 主题: Re: 答复: [PATCH 2/7] drm/amdgpu:new field members for SRIOV <div> </div> </div> <div> <div class="moz-cite-prefix">Am 10.01.2017 um 04:03 schrieb Liu, Monk: </div> <blockquote cite="mid:BY2PR1201MB11106E7ABC5FF87AF1F6F67A84670@BY2PR1201MB1110.namprd12.prod.outlook.com" type="cite"> <meta name="Generator" content="Microsoft Exchange Server">  <style></style> <meta content="text/html; charset=UTF-8"> <style type="text/css" style="">  </style> <div dir="ltr"> <div id="x_divtagdefaultwrapper" dir="ltr" style="font-size:12pt; color:#000000; font-family:Calibri,Arial,Helvetica,sans-serif"> > + /* 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 ... </div> </div> </blockquote> 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. <blockquote cite="mid:BY2PR1201MB11106E7ABC5FF87AF1F6F67A84670@BY2PR1201MB1110.namprd12.prod.outlook.com" type="cite"> <div dir="ltr"> <div id="x_divtagdefaultwrapper" dir="ltr" style="font-size:12pt; color:#000000; font-family:Calibri,Arial,Helvetica,sans-serif"> </div> <hr tabindex="-1" style="display:inline-block; width:98%"> <div id="x_divRplyFwdMsg" dir="ltr">发件人: Christian König <a moz-do-not-send="true" class="moz-txt-link-rfc2396E" href="mailto:deathsimple@vodafone.de"><deathsimple@vodafone.de></a> 发送时间: 2017年1月9日 18:57:06 收件人: Liu, Monk; <a moz-do-not-send="true" class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org"> amd-gfx@lists.freedesktop.org</a> 主题: Re: [PATCH 2/7] drm/amdgpu:new field members for SRIOV <div> </div> </div> </div> <div class="PlainText">Am 09.01.2017 um 09:02 schrieb Monk Liu: > Change-Id: Ife0eff7b13b8b5946f005a39f6ecb8db1cb72c38 > Signed-off-by: Monk Liu <a moz-do-not-send="true" class="moz-txt-link-rfc2396E" href="mailto:Monk.Liu@amd.com"> <Monk.Liu@amd.com></a> > --- > 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 { </div> <fieldset class="mimeAttachmentHeader"></fieldset> <pre wrap="">_______________________________________________ amd-gfx mailing list <a moz-do-not-send="true" class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> <a moz-do-not-send="true" class="moz-txt-link-freetext" href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a> </pre> </blockquote> </div> </blockquote> </body> </html>