[AMD Official Use Only]
Looks good to me .
Reviewed by Shaoyun.liu < shaoyun....@amd.com>
-----Original Message-----
From: Kuehling, Felix <felix.kuehl...@amd.com>
Sent: Friday, September 10, 2021 1:10 AM
To: amd-gfx@lists.freedesktop.org
Cc: Liu, Shaoyun <shaoyun....@amd.com>
Subject: Re: [PATCH v3 1/1] drm/amdkfd: make needs_pcie_atomics FW-version
dependent
Am 2021-09-08 um 6:48 p.m. schrieb Felix Kuehling:
> On some GPUs the PCIe atomic requirement for KFD depends on the MEC
> firmware version. Add a firmware version check for this. The minimum
> firmware version that works without atomics can be updated in the
> device_info structure for each GPU type.
>
> Move PCIe atomic detection from kgf2kfd_probe into kgf2kfd_device_init
> because the MEC firmware is not loaded yet at the probe stage.
>
> Signed-off-by: Felix Kuehling <felix.kuehl...@amd.com>
I tested this change on a Sienna Cichlid on a system without PCIe atomics, both
with the old and the new firmware. This version of the change should be good to
go if I can get an R-b.
Thanks,
Felix
> ---
> drivers/gpu/drm/amd/amdkfd/kfd_device.c | 44 ++++++++++++++++---------
> drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 1 +
> 2 files changed, 29 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> index 16a57b70cc1a..30fde852af19 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> @@ -468,6 +468,7 @@ static const struct kfd_device_info navi10_device_info = {
> .needs_iommu_device = false,
> .supports_cwsr = true,
> .needs_pci_atomics = true,
> + .no_atomic_fw_version = 145,
> .num_sdma_engines = 2,
> .num_xgmi_sdma_engines = 0,
> .num_sdma_queues_per_engine = 8,
> @@ -487,6 +488,7 @@ static const struct kfd_device_info navi12_device_info = {
> .needs_iommu_device = false,
> .supports_cwsr = true,
> .needs_pci_atomics = true,
> + .no_atomic_fw_version = 145,
> .num_sdma_engines = 2,
> .num_xgmi_sdma_engines = 0,
> .num_sdma_queues_per_engine = 8,
> @@ -506,6 +508,7 @@ static const struct kfd_device_info navi14_device_info = {
> .needs_iommu_device = false,
> .supports_cwsr = true,
> .needs_pci_atomics = true,
> + .no_atomic_fw_version = 145,
> .num_sdma_engines = 2,
> .num_xgmi_sdma_engines = 0,
> .num_sdma_queues_per_engine = 8,
> @@ -525,6 +528,7 @@ static const struct kfd_device_info
> sienna_cichlid_device_info = {
> .needs_iommu_device = false,
> .supports_cwsr = true,
> .needs_pci_atomics = true,
> + .no_atomic_fw_version = 92,
> .num_sdma_engines = 4,
> .num_xgmi_sdma_engines = 0,
> .num_sdma_queues_per_engine = 8,
> @@ -544,6 +548,7 @@ static const struct kfd_device_info
> navy_flounder_device_info = {
> .needs_iommu_device = false,
> .supports_cwsr = true,
> .needs_pci_atomics = true,
> + .no_atomic_fw_version = 92,
> .num_sdma_engines = 2,
> .num_xgmi_sdma_engines = 0,
> .num_sdma_queues_per_engine = 8,
> @@ -562,7 +567,8 @@ static const struct kfd_device_info vangogh_device_info =
> {
> .mqd_size_aligned = MQD_SIZE_ALIGNED,
> .needs_iommu_device = false,
> .supports_cwsr = true,
> - .needs_pci_atomics = false,
> + .needs_pci_atomics = true,
> + .no_atomic_fw_version = 92,
> .num_sdma_engines = 1,
> .num_xgmi_sdma_engines = 0,
> .num_sdma_queues_per_engine = 2,
> @@ -582,6 +588,7 @@ static const struct kfd_device_info
> dimgrey_cavefish_device_info = {
> .needs_iommu_device = false,
> .supports_cwsr = true,
> .needs_pci_atomics = true,
> + .no_atomic_fw_version = 92,
> .num_sdma_engines = 2,
> .num_xgmi_sdma_engines = 0,
> .num_sdma_queues_per_engine = 8,
> @@ -601,6 +608,7 @@ static const struct kfd_device_info
> beige_goby_device_info = {
> .needs_iommu_device = false,
> .supports_cwsr = true,
> .needs_pci_atomics = true,
> + .no_atomic_fw_version = 92,
> .num_sdma_engines = 1,
> .num_xgmi_sdma_engines = 0,
> .num_sdma_queues_per_engine = 8,
> @@ -619,7 +627,8 @@ static const struct kfd_device_info
> yellow_carp_device_info = {
> .mqd_size_aligned = MQD_SIZE_ALIGNED,
> .needs_iommu_device = false,
> .supports_cwsr = true,
> - .needs_pci_atomics = false,
> + .needs_pci_atomics = true,
> + .no_atomic_fw_version = 92,
> .num_sdma_engines = 1,
> .num_xgmi_sdma_engines = 0,
> .num_sdma_queues_per_engine = 2,
> @@ -708,20 +717,6 @@ struct kfd_dev *kgd2kfd_probe(struct kgd_dev *kgd,
> if (!kfd)
> return NULL;
>
> - /* Allow BIF to recode atomics to PCIe 3.0 AtomicOps.
> - * 32 and 64-bit requests are possible and must be
> - * supported.
> - */
> - kfd->pci_atomic_requested = amdgpu_amdkfd_have_atomics_support(kgd);
> - if (device_info->needs_pci_atomics &&
> - !kfd->pci_atomic_requested) {
> - dev_info(kfd_device,
> - "skipped device %x:%x, PCI rejects atomics\n",
> - pdev->vendor, pdev->device);
> - kfree(kfd);
> - return NULL;
> - }
> -
> kfd->kgd = kgd;
> kfd->device_info = device_info;
> kfd->pdev = pdev;
> @@ -821,6 +816,23 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd,
> kfd->vm_info.vmid_num_kfd = kfd->vm_info.last_vmid_kfd
> - kfd->vm_info.first_vmid_kfd + 1;
>
> + /* Allow BIF to recode atomics to PCIe 3.0 AtomicOps.
> + * 32 and 64-bit requests are possible and must be
> + * supported.
> + */
> + kfd->pci_atomic_requested =
> amdgpu_amdkfd_have_atomics_support(kfd->kgd);
> + if (!kfd->pci_atomic_requested &&
> + kfd->device_info->needs_pci_atomics &&
> + (!kfd->device_info->no_atomic_fw_version ||
> + kfd->mec_fw_version < kfd->device_info->no_atomic_fw_version)) {
> + dev_info(kfd_device,
> + "skipped device %x:%x, PCI rejects atomics %d<%d\n",
> + kfd->pdev->vendor, kfd->pdev->device,
> + kfd->mec_fw_version,
> + kfd->device_info->no_atomic_fw_version);
> + return false;
> + }
> +
> /* Verify module parameters regarding mapped process number*/
> if ((hws_max_conc_proc < 0)
> || (hws_max_conc_proc > kfd->vm_info.vmid_num_kfd)) {
> diff --git
> a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index ab83b0de6b22..6d8f9bb2d905 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -207,6 +207,7 @@ struct kfd_device_info {
> bool supports_cwsr;
> bool needs_iommu_device;
> bool needs_pci_atomics;
> + uint32_t no_atomic_fw_version;
> unsigned int num_sdma_engines;
> unsigned int num_xgmi_sdma_engines;
> unsigned int num_sdma_queues_per_engine;
