Am 2021-04-29 um 11:04 a.m. schrieb Kim, Jonathan: > [AMD Official Use Only - Internal Distribution Only] > >> -----Original Message----- >> From: Kuehling, Felix <Felix.Kuehling@xxxxxxx> >> Sent: Thursday, April 29, 2021 10:49 AM >> To: Kim, Jonathan <Jonathan.Kim@xxxxxxx>; amd- >> gfx@xxxxxxxxxxxxxxxxxxxxx >> Cc: Zeng, Oak <Oak.Zeng@xxxxxxx>; Errabolu, Ramesh >> <Ramesh.Errabolu@xxxxxxx> >> Subject: Re: [PATCH] drm/amdkfd: report atomics support in io_links over >> xgmi >> >> Am 2021-04-29 um 5:36 a.m. schrieb Jonathan Kim: >>> Link atomics support over xGMI should be reported independently of PCIe. >> I don't understand this change. I don't see any code that gets executed if >> (adev->gmc.xgmi.connected_to_cpu). Where is the code that reports >> atomics support for this case? > The atomics aren't set but rather NO_ATOMICS are set if non-xgmi and non PCIe supported: > cpu_flag |= CRAT_IOLINK_FLAGS_NO_ATOMICS_32_BIT | CRAT_IOLINK_FLAGS_NO_ATOMICS_64_BIT; > adev->gmc.xgmi.connected_to_cpu == true would bypass this flag NO_ATOMICS setting. > >> Also, the PCIe code doesn't clear any atomic flags. It only sets flags that >> would be set for XGMI anyway. So I don't see why you need to make that >> code conditional. > As mentioned above they set NO_ATOMICS if not PCIe supported. > This has been observed on Aldebaran with AMD systems. OK. I missed that these flags are negative logic. Thanks, the change makes sense now. But the patch description is a bit misleading compared to the code. A different wording would make it clearer, e.g.: "Don't set NO_ATOMICS flags on XGMI links between CPU and GPU." With that fixed, the patch is Reviewed-by: Felix Kuehling <Felix.Kuehling@xxxxxxx> Regards, Felix > > Thanks, > > Jon > >> Regards, >> Felix >> >> >>> Signed-off-by: Jonathan Kim <jonathan.kim@xxxxxxx> >>> Tested-by: Ramesh Errabolu <ramesh.errabolu@xxxxxxx> >>> --- >>> drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 29 >>> ++++++++++++++--------- >>> 1 file changed, 18 insertions(+), 11 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c >>> b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c >>> index 083ac9babfa8..30430aefcfc7 100644 >>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c >>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c >>> @@ -1196,6 +1196,7 @@ static void kfd_fill_iolink_non_crat_info(struct >>> kfd_topology_device *dev) { >>> struct kfd_iolink_properties *link, *cpu_link; >>> struct kfd_topology_device *cpu_dev; >>> +struct amdgpu_device *adev; >>> uint32_t cap; >>> uint32_t cpu_flag = CRAT_IOLINK_FLAGS_ENABLED; >>> uint32_t flag = CRAT_IOLINK_FLAGS_ENABLED; @@ -1203,18 >> +1204,24 @@ >>> static void kfd_fill_iolink_non_crat_info(struct kfd_topology_device *dev) >>> if (!dev || !dev->gpu) >>> return; >>> >>> -pcie_capability_read_dword(dev->gpu->pdev, >>> -PCI_EXP_DEVCAP2, &cap); >>> - >>> -if (!(cap & (PCI_EXP_DEVCAP2_ATOMIC_COMP32 | >>> - PCI_EXP_DEVCAP2_ATOMIC_COMP64))) >>> -cpu_flag |= CRAT_IOLINK_FLAGS_NO_ATOMICS_32_BIT | >>> -CRAT_IOLINK_FLAGS_NO_ATOMICS_64_BIT; >>> +adev = (struct amdgpu_device *)(dev->gpu->kgd); >>> +if (!adev->gmc.xgmi.connected_to_cpu) { >>> +pcie_capability_read_dword(dev->gpu->pdev, >>> +PCI_EXP_DEVCAP2, &cap); >>> + >>> +if (!(cap & (PCI_EXP_DEVCAP2_ATOMIC_COMP32 | >>> + PCI_EXP_DEVCAP2_ATOMIC_COMP64))) >>> +cpu_flag |= >> CRAT_IOLINK_FLAGS_NO_ATOMICS_32_BIT | >>> +CRAT_IOLINK_FLAGS_NO_ATOMICS_64_BIT; >>> +} >>> >>> -if (!dev->gpu->pci_atomic_requested || >>> - dev->gpu->device_info->asic_family == CHIP_HAWAII) >>> -flag |= CRAT_IOLINK_FLAGS_NO_ATOMICS_32_BIT | >>> -CRAT_IOLINK_FLAGS_NO_ATOMICS_64_BIT; >>> +if (!adev->gmc.xgmi.num_physical_nodes) { >>> +if (!dev->gpu->pci_atomic_requested || >>> +dev->gpu->device_info->asic_family == >>> +CHIP_HAWAII) >>> +flag |= CRAT_IOLINK_FLAGS_NO_ATOMICS_32_BIT | >>> +CRAT_IOLINK_FLAGS_NO_ATOMICS_64_BIT; >>> +} >>> >>> /* GPU only creates direct links so apply flags setting to all */ >>> list_for_each_entry(link, &dev->io_link_props, list) { _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx