[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. 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