[PATCH] drm/amdgpu: Get atomicOps info from Host for sriov setup

[Felix Kuehling]

Am 2021-09-09 um 11:59 a.m. schrieb shaoyunl:
> The AtomicOp Requester Enable bit is reserved in VFs and the PF value applies to all
> associated VFs. so guest driver can not directly enable the atomicOps for VF, it
> depends on PF to enable it. In current design, amdgpu driver  will get the enabled
> atomicOps bits through private pf2vf data
>
> Signed-off-by: shaoyunl <shaoyun.liu at amd.com>
> Change-Id: Ifdbcb4396d64e3f3cbf6bcbf7ab9c7b2cb061052
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  | 20 ++++++++++++++++++--
>  drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h |  4 +++-
>  2 files changed, 21 insertions(+), 3 deletions(-)
>  mode change 100644 => 100755 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>  mode change 100644 => 100755 drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> old mode 100644
> new mode 100755
> index 653bd8fdaa33..a0d2b9eb84fc
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2167,8 +2167,6 @@ static int amdgpu_device_ip_early_init(struct amdgpu_device *adev)
>  		return -EINVAL;
>  	}
>  
> -	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)
>  		adev->pm.pp_feature &= ~PP_GFXOFF_MASK;
> @@ -3562,6 +3560,24 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>  	if (r)
>  		return r;
>  
> +	/* enable PCIE atomic ops */
> +	if (amdgpu_sriov_bios(adev))
> +		adev->have_atomics_support = (((struct amd_sriov_msg_pf2vf_info *)
> +			adev->virt.fw_reserve.p_pf2vf)->pcie_atomic_ops_enabled_flags ==
> +			(PCI_EXP_DEVCAP2_ATOMIC_COMP32 | PCI_EXP_DEVCAP2_ATOMIC_COMP64))
> +			? TRUE : FALSE;

Please don't use this "condition ? TRUE : FALSE" idiom. Just "condition"
is good enough.


> +	else
> +		adev->have_atomics_support =
> +			pci_enable_atomic_ops_to_root(adev->pdev,
> +					  PCI_EXP_DEVCAP2_ATOMIC_COMP32 |
> +					  PCI_EXP_DEVCAP2_ATOMIC_COMP64)
> +			? FALSE : TRUE;

Same as above, but in this case it's "!condition". Also, I would have
expected that you remove the other call to pci_enable_atomic_ops_to_root
from this function.


> +	if (adev->have_atomics_support = false )

This should be "==", but even better would be "if
(!adev->have_atomics_support) ...

That said, the message below may be redundant. The PCIe atomic check in
kgd2kfd_device_init already prints an error message if atomics are
required by the GPU but not supported. If you really want to print it
for information on GPUs where it's not required, use dev_info so the
message clearly shows which GPU in a multi-GPU system it refers to.


> +		DRM_INFO("PCIE atomic ops is not supported\n");
> +
> +	amdgpu_amdkfd_device_probe(adev);

This should not be necessary. I just sent another patch for review that
moves the PCIe atomic check in KFD into kgd2kfd_device_init:
"drm/amdkfd: make needs_pcie_atomics FW-version dependent". So
amdgpu_amdkfd_device_probe can stay where it is, if you can wait a few
days for my change to go in first.

Regards,
  Felix


> +
> +
>  	/* doorbell bar mapping and doorbell index init*/
>  	amdgpu_device_doorbell_init(adev);
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
> old mode 100644
> new mode 100755
> index a434c71fde8e..995899191288
> --- a/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
> @@ -204,8 +204,10 @@ struct amd_sriov_msg_pf2vf_info {
>  	} mm_bw_management[AMD_SRIOV_MSG_RESERVE_VCN_INST];
>  	/* UUID info */
>  	struct amd_sriov_msg_uuid_info uuid_info;
> +	/* pcie atomic Ops info */
> +	uint32_t pcie_atomic_ops_enabled_flags;
>  	/* reserved */
> -	uint32_t reserved[256 - 47];
> +	uint32_t reserved[256 - 48];
>  };
>  
>  struct amd_sriov_msg_vf2pf_info_header {