comments inline.
On 2019-04-23 2:09 p.m., Kuehling, Felix wrote:
> See inline.
>
> On 2019-04-17 2:58 p.m., Liu, Shaoyun wrote:
>> KFD need to provide the info for upper level to determine the data path
>>
>> Change-Id: Idc809e8f3381b9222dd7be96539522d440f3ee7d
>> Signed-off-by: shaoyunl <shaoyun.liu@xxxxxxx>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 15 +++++++++++++++
>> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 1 +
>> drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c | 21 +++++++++++++++++++++
>> drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h | 2 ++
>> 4 files changed, 39 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>> index acf8ae0..3fe9a38 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>> @@ -27,6 +27,7 @@
>> #include "amdgpu_gfx.h"
>> #include <linux/module.h>
>> #include <linux/dma-buf.h>
>> +#include "amdgpu_xgmi.h"
>>
>> static const unsigned int compute_vmid_bitmap = 0xFF00;
>>
>> @@ -481,6 +482,20 @@ uint64_t amdgpu_amdkfd_get_hive_id(struct kgd_dev *kgd)
>>
>> return adev->gmc.xgmi.hive_id;
>> }
>> +uint32_t amdgpu_amdkfd_get_xgmi_hops_count(struct kgd_dev *dst, struct kgd_dev *src)
>> +{
>> + struct amdgpu_device *peer_adev = (struct amdgpu_device *)src;
>> + struct amdgpu_device *adev = (struct amdgpu_device *)dst;
>> + int ret = amdgpu_xgmi_get_hops_count(adev, peer_adev);
>> +
>> + if (ret < 0) {
>> + DRM_ERROR("amdgpu: failed to get xgmi hops count between node %d and %d. ret = %d\n",
>> + adev->gmc.xgmi.physical_node_id,
>> + peer_adev->gmc.xgmi.physical_node_id, ret);
>> + ret = 0;
>> + }
>> + return (uint32_t)ret;
>> +}
>>
>> int amdgpu_amdkfd_submit_ib(struct kgd_dev *kgd, enum kgd_engine_type engine,
>> uint32_t vmid, uint64_t gpu_addr,
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
>> index e6a5037..8cc8a5a1 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
>> @@ -154,6 +154,7 @@ int amdgpu_amdkfd_get_dmabuf_info(struct kgd_dev *kgd, int dma_buf_fd,
>> uint32_t *flags);
>> uint64_t amdgpu_amdkfd_get_vram_usage(struct kgd_dev *kgd);
>> uint64_t amdgpu_amdkfd_get_hive_id(struct kgd_dev *kgd);
>> +uint32_t amdgpu_amdkfd_get_xgmi_hops_count(struct kgd_dev *dst, struct kgd_dev *src);
>>
>> #define read_user_wptr(mmptr, wptr, dst) \
>> ({ \
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
>> index a48c84c..eaebd47 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
>> @@ -248,6 +248,27 @@ int amdgpu_xgmi_update_topology(struct amdgpu_hive_info *hive, struct amdgpu_dev
>> return ret;
>> }
>>
>> +
>> +int amdgpu_xgmi_get_hops_count(struct amdgpu_device *adev,
>> + struct amdgpu_device *peer_adev)
>> +{
>> + struct psp_xgmi_topology_info *top;
>> + int ret = 0;
>> +
>> + top = kzalloc(sizeof(struct psp_xgmi_topology_info), GFP_KERNEL);
> Where does this get freed? Looks like you have a memory leak.
>
> Also, instead of allocating a new copy and querying the info from the
> PSP many times, could the psp_xgmi_topology_info structure be persistent
> somewhere in adev. I think after the driver initialization it won't
> change any more.
>
> Regards,
> Felix
>
Good catch for the memory leak. I will add free before this exit
function. I think of move psp_xgmi_topology_info into adev , but seems
involves too much change . Let me re-think it .
Regards
shaoyun.liu
>
>> + if (!top)
>> + return -ENOMEM;
>> + top->num_nodes = 1;
>> + top->nodes[0].node_id = peer_adev->gmc.xgmi.node_id;
>> + ret = psp_xgmi_get_topology_info(&adev->psp, 1, top);
>> + if (ret) {
>> + dev_err(adev->dev,
>> + "XGMI: Failed to get topology info\n");
>> + return ret;
>> + }
>> + return top->nodes[0].num_hops;
>> +}
>> +
>> int amdgpu_xgmi_add_device(struct amdgpu_device *adev)
>> {
>> struct psp_xgmi_topology_info *hive_topology;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
>> index 3e9c91e..8a945bf 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
>> @@ -41,6 +41,8 @@ int amdgpu_xgmi_update_topology(struct amdgpu_hive_info *hive, struct amdgpu_dev
>> int amdgpu_xgmi_add_device(struct amdgpu_device *adev);
>> void amdgpu_xgmi_remove_device(struct amdgpu_device *adev);
>> int amdgpu_xgmi_set_pstate(struct amdgpu_device *adev, int pstate);
>> +int amdgpu_xgmi_get_hops_count(struct amdgpu_device *adev,
>> + struct amdgpu_device *peer_adev);
>>
>> static inline bool amdgpu_xgmi_same_hive(struct amdgpu_device *adev,
>> struct amdgpu_device *bo_adev)
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
