Re: [PATCH] drm/amdgpu: Get atomicOps info from Host for sriov setup
On 2021-09-10 11:15 a.m., shaoyunl wrote:
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
Reviewed-by: Felix Kuehling
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 24 +++--
drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h | 4 +++-
2 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 653bd8fdaa33..3ae1721ca859 100644
--- 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,19 @@ int amdgpu_device_init(struct amdgpu_device *adev,
if (r)
return r;
+ /* enable PCIE atomic ops */
+ if (amdgpu_sriov_vf(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);
+ 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");
+
/* 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
index a434c71fde8e..995899191288 100644
--- 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 {
RE: [PATCH] drm/amdgpu: Get atomicOps info from Host for sriov setup
[Public]
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
Please don't modify the file mode.
Regards,
Guchun
-Original Message-
From: amd-gfx On Behalf Of shaoyunl
Sent: Friday, September 10, 2021 10:26 PM
To: amd-gfx@lists.freedesktop.org
Cc: Liu, Shaoyun
Subject: [PATCH] drm/amdgpu: Get atomicOps info from Host for sriov setup
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
Change-Id: Ifdbcb4396d64e3f3cbf6bcbf7ab9c7b2cb061052
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 24 +++--
drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h | 4 +++-
2 files changed, 16 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..3ae1721ca859
--- 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,19 @@ int amdgpu_device_init(struct amdgpu_device *adev,
if (r)
return r;
+ /* enable PCIE atomic ops */
+ if (amdgpu_sriov_vf(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);
+ 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");
+
/* 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 {
--
2.17.1
Re: [PATCH] drm/amdgpu: Get atomicOps info from Host for sriov setup
Am 2021-09-10 um 10:26 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
> Change-Id: Ifdbcb4396d64e3f3cbf6bcbf7ab9c7b2cb061052
You can disable the generation of ChangeIds for a repository with
git config gerrit.createChangeId false
Other than that, the patch is
Reviewed-by: Felix Kuehling
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 24 +++--
> drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h | 4 +++-
> 2 files changed, 16 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..3ae1721ca859
> --- 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,19 @@ int amdgpu_device_init(struct amdgpu_device *adev,
> if (r)
> return r;
>
> + /* enable PCIE atomic ops */
> + if (amdgpu_sriov_vf(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);
> + 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");
> +
> /* 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 {
RE: [PATCH] drm/amdgpu: Get atomicOps info from Host for sriov setup
[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
Sent: Friday, September 10, 2021 10:19 AM
To: amd-gfx@lists.freedesktop.org; Liu, Shaoyun
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
> 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 {
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
> 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 {
RE: [PATCH] drm/amdgpu: Get atomicOps info from Host for sriov setup
[AMD Official Use Only]
Thanks for the review . I accepted your comments and will sent another
change list for review once your change is in.
Regards
Shaoyun.liu
-Original Message-
From: Kuehling, Felix
Sent: Thursday, September 9, 2021 12:18 PM
To: Liu, Shaoyun ; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: Get atomicOps info from Host for sriov setup
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
> 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 {
Re: [PATCH] drm/amdgpu: Get atomicOps info from Host for sriov setup
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
> 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 {
