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

[Liu, Shaoyun]

[AMD Official Use Only]

Good catch  . my editor seems has auto complete feature and  I just select the first one .  ☹

Thanks 
Shaoyun.liu

-----Original Message-----
From: Kuehling, Felix <Felix.Kuehling at amd.com> 
Sent: Friday, September 10, 2021 10:19 AM
To: amd-gfx at lists.freedesktop.org; Liu, Shaoyun <Shaoyun.Liu at amd.com>
Subject: Re: [PATCH] drm/amdgpu: Get atomicOps info from Host for sriov setup

Am 2021-09-10 um 10:04 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

Please remove the Change-Id.

In general, the change looks good to me. One question and one more nit-pick inline ...


> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  | 25 
> ++++++++++++---------  drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h |  
> 4 +++-
>  2 files changed, 17 insertions(+), 12 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..fc6a6491c1b6
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -3529,17 +3529,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>  	DRM_INFO("register mmio base: 0x%08X\n", (uint32_t)adev->rmmio_base);
>  	DRM_INFO("register mmio size: %u\n", (unsigned)adev->rmmio_size);
>  
> -	/* enable PCIE atomic ops */
> -	r = pci_enable_atomic_ops_to_root(adev->pdev,
> -					  PCI_EXP_DEVCAP2_ATOMIC_COMP32 |
> -					  PCI_EXP_DEVCAP2_ATOMIC_COMP64);
> -	if (r) {
> -		adev->have_atomics_support = false;
> -		DRM_INFO("PCIE atomic ops is not supported\n");
> -	} else {
> -		adev->have_atomics_support = true;
> -	}
> -
>  	amdgpu_device_get_pcie_info(adev);
>  
>  	if (amdgpu_mcbp)
> @@ -3562,6 +3551,20 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>  	if (r)
>  		return r;
>  
> +	/* enable PCIE atomic ops */
> +	if (amdgpu_sriov_bios(adev))

Is this the correct condition? I think this would be true for the PF as well. But on the PF we still need to call pci_enable_atomic_ops_to_root.
I would expect a condition that only applies to VFs.


> +		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);
> +	else
> +		adev->have_atomics_support =
> +			!pci_enable_atomic_ops_to_root(adev->pdev,
> +					  PCI_EXP_DEVCAP2_ATOMIC_COMP32 |
> +					  PCI_EXP_DEVCAP2_ATOMIC_COMP64);
> +	if (!adev->have_atomics_support)
> +		dev_info(adev->dev, "PCIE atomic ops is not supported\n");
> +
> +

Double blank lines. One is enough.

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 {