[PATCH] drm/amdkfd: On GFX11 check PCIe atomics support and set CP_HQD_HQ_STATUS0[29]

[Sider, Graham]

[Public]

A minor nitpick/suggestions below to make the comments a bit more concise. With that change the patch is

Reviewed-by: Graham Sider <Graham.Sider at amd.com>

Best,
Graham

> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of
> Sreekant Somasekharan
> Sent: Monday, April 3, 2023 3:59 PM
> To: amd-gfx at lists.freedesktop.org
> Cc: Somasekharan, Sreekant <Sreekant.Somasekharan at amd.com>
> Subject: [PATCH] drm/amdkfd: On GFX11 check PCIe atomics support and set
> CP_HQD_HQ_STATUS0[29]
> 
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
> 
> 
> On GFX11, CP_HQD_HQ_STATUS0[29] bit will be used by CPFW to
> acknowledge whether PCIe atomics are supported. The default value of this
> bit is set to 0. Driver will check whether PCIe atomics are supported and set
> the bit to 1 if supported. This will force CPFW to use real atomic ops.
> If the bit is not set, CPFW will default to read/modify/write using the
> firmware itself.
> 
> This is applicable only to RS64 based GFX11 with MEC FW greater than or
> equal to 509. If MEC FW is less than 509, PCIe atomics needs to be supported,
> else it will skip the device.
> 
> This commit also involves moving amdgpu_amdkfd_device_probe() function
> call after per-IP early_init loop in amdgpu_device_ip_early_init() function so
> as to check for RS64 enabled device.
> 
> Signed-off-by: Sreekant Somasekharan
> <sreekant.somasekharan at amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c       |  2 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_device.c          | 11 +++++++++++
>  drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c |  9 +++++++++
>  3 files changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 7116119ed038..b3a754ca0923 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2150,7 +2150,6 @@ static int amdgpu_device_ip_early_init(struct
> amdgpu_device *adev)
>                 adev->has_pr3 = parent ? pci_pr3_present(parent) : false;
>         }
> 
> -       amdgpu_amdkfd_device_probe(adev);
> 
>         adev->pm.pp_feature = amdgpu_pp_feature_mask;
>         if (amdgpu_sriov_vf(adev) || sched_policy ==
> KFD_SCHED_POLICY_NO_HWS) @@ -2206,6 +2205,7 @@ static int
> amdgpu_device_ip_early_init(struct amdgpu_device *adev)
>         if (!total)
>                 return -ENODEV;
> 
> +       amdgpu_amdkfd_device_probe(adev);
>         adev->cg_flags &= amdgpu_cg_mask;
>         adev->pg_flags &= amdgpu_pg_mask;
> 
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> index 521dfa88aad8..64a295a35d37 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> @@ -204,6 +204,17 @@ static void kfd_device_info_init(struct kfd_dev *kfd,
>                         /* Navi1x+ */
>                         if (gc_version >= IP_VERSION(10, 1, 1))
>                                 kfd->device_info.needs_pci_atomics = true;
> +               } else if (gc_version < IP_VERSION(12, 0, 0)) {
> +                       /* On GFX11 running on RS64, MEC FW version must be greater
> than
> +                        * or equal to version 509 to support acknowledging whether
> +                        * PCIe atomics are supported. Before MEC version 509, PCIe
> +                        * atomics are required. After that, the FW's use of atomics
> +                        * is controlled by CP_HQD_HQ_STATUS0[29].
> +                        * This will fail on GFX11 when PCIe atomics are not supported
> +                        * and MEC FW version < 509 for RS64 based CPFW.
> +                        */

Could probably make this comment and the one below a bit more concise. Suggestion:

/*
 * PCIe atomics support acknowledgement in GFX11 RS64 CPFW requires version >= 509.
 * Prior RS64 CPFW versions (and all F32) require PCIe atomics support.
 */

> +                       kfd->device_info.needs_pci_atomics = true;
> +                       kfd->device_info.no_atomic_fw_version =
> + kfd->adev->gfx.rs64_enable ? 509 : 0;
>                 }
>         } else {
>                 kfd->device_info.doorbell_size = 4; diff --git
> a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c
> index 4a9af800b1f1..c5ea594abbf6 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c
> @@ -143,6 +143,15 @@ static void init_mqd(struct mqd_manager *mm, void
> **mqd,
>                         1 << CP_HQD_QUANTUM__QUANTUM_SCALE__SHIFT |
>                         1 << CP_HQD_QUANTUM__QUANTUM_DURATION__SHIFT;
> 
> +       /*
> +        * If PCIe atomics are supported, set CP_HQD_HQ_STATUS0[29] == 1
> +        * to force CPFW to use atomics. This is supported only on MEC FW
> +        * version >= 509 and on RS64 based CPFW only. On previous versions,
> +        * platforms running on GFX11 must support atomics else will skip the
> device.
> +        */

Suggestion:

/* GFX11 RS64 CPFW version >= 509 supports PCIe atomics support acknowledgement */

> +       if (amdgpu_amdkfd_have_atomics_support((mm->dev->adev)))
> +               m->cp_hqd_hq_status0 |= 1 << 29;
> +
>         if (q->format == KFD_QUEUE_FORMAT_AQL) {
>                 m->cp_hqd_aql_control =
>                         1 << CP_HQD_AQL_CONTROL__CONTROL0__SHIFT;
> --
> 2.25.1