* [PATCH] drm/amdgpu: Get atomicOps info from Host for sriov setup
@ 2021-09-10 14:04 shaoyunl
2021-09-10 14:19 ` Felix Kuehling
0 siblings, 1 reply; 11+ messages in thread
From: shaoyunl @ 2021-09-10 14:04 UTC (permalink / raw)
To: amd-gfx; +Cc: 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@amd.com>
Change-Id: Ifdbcb4396d64e3f3cbf6bcbf7ab9c7b2cb061052
---
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))
+ 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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH] drm/amdgpu: Get atomicOps info from Host for sriov setup
2021-09-10 14:04 [PATCH] drm/amdgpu: Get atomicOps info from Host for sriov setup shaoyunl
@ 2021-09-10 14:19 ` Felix Kuehling
2021-09-10 14:23 ` Liu, Shaoyun
0 siblings, 1 reply; 11+ messages in thread
From: Felix Kuehling @ 2021-09-10 14:19 UTC (permalink / raw)
To: amd-gfx, Liu, Shaoyun
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@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 {
^ permalink raw reply [flat|nested] 11+ messages in thread* RE: [PATCH] drm/amdgpu: Get atomicOps info from Host for sriov setup
2021-09-10 14:19 ` Felix Kuehling
@ 2021-09-10 14:23 ` Liu, Shaoyun
0 siblings, 0 replies; 11+ messages in thread
From: Liu, Shaoyun @ 2021-09-10 14:23 UTC (permalink / raw)
To: Kuehling, Felix, amd-gfx@lists.freedesktop.org
[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@amd.com>
Sent: Friday, September 10, 2021 10:19 AM
To: amd-gfx@lists.freedesktop.org; Liu, Shaoyun <Shaoyun.Liu@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@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 {
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] drm/amdgpu: Get atomicOps info from Host for sriov setup
@ 2021-09-10 15:15 shaoyunl
2021-09-10 19:28 ` Felix Kuehling
0 siblings, 1 reply; 11+ messages in thread
From: shaoyunl @ 2021-09-10 15:15 UTC (permalink / raw)
To: amd-gfx; +Cc: 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@amd.com>
---
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 {
--
2.17.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH] drm/amdgpu: Get atomicOps info from Host for sriov setup
2021-09-10 15:15 shaoyunl
@ 2021-09-10 19:28 ` Felix Kuehling
0 siblings, 0 replies; 11+ messages in thread
From: Felix Kuehling @ 2021-09-10 19:28 UTC (permalink / raw)
To: shaoyunl, amd-gfx
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 <shaoyun.liu@amd.com>
Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>
> ---
> 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 {
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] drm/amdgpu: Get atomicOps info from Host for sriov setup
@ 2021-09-10 14:26 shaoyunl
2021-09-10 14:33 ` Felix Kuehling
2021-09-10 14:38 ` Chen, Guchun
0 siblings, 2 replies; 11+ messages in thread
From: shaoyunl @ 2021-09-10 14:26 UTC (permalink / raw)
To: amd-gfx; +Cc: 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@amd.com>
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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH] drm/amdgpu: Get atomicOps info from Host for sriov setup
2021-09-10 14:26 shaoyunl
@ 2021-09-10 14:33 ` Felix Kuehling
2021-09-10 14:38 ` Chen, Guchun
1 sibling, 0 replies; 11+ messages in thread
From: Felix Kuehling @ 2021-09-10 14:33 UTC (permalink / raw)
To: amd-gfx, Liu, Shaoyun
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 <shaoyun.liu@amd.com>
> 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 <Felix.Kuehling@amd.com>
> ---
> 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 {
^ permalink raw reply [flat|nested] 11+ messages in thread* RE: [PATCH] drm/amdgpu: Get atomicOps info from Host for sriov setup
2021-09-10 14:26 shaoyunl
2021-09-10 14:33 ` Felix Kuehling
@ 2021-09-10 14:38 ` Chen, Guchun
1 sibling, 0 replies; 11+ messages in thread
From: Chen, Guchun @ 2021-09-10 14:38 UTC (permalink / raw)
To: Liu, Shaoyun, amd-gfx@lists.freedesktop.org; +Cc: Liu, Shaoyun
[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 <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of shaoyunl
Sent: Friday, September 10, 2021 10:26 PM
To: amd-gfx@lists.freedesktop.org
Cc: Liu, Shaoyun <Shaoyun.Liu@amd.com>
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 <shaoyun.liu@amd.com>
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH] drm/amdgpu: Get atomicOps info from Host for sriov setup
@ 2021-09-09 15:59 shaoyunl
2021-09-09 16:17 ` Felix Kuehling
0 siblings, 1 reply; 11+ messages in thread
From: shaoyunl @ 2021-09-09 15:59 UTC (permalink / raw)
To: amd-gfx; +Cc: 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@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;
+ 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;
+ if (adev->have_atomics_support = false )
+ DRM_INFO("PCIE atomic ops is not supported\n");
+
+ amdgpu_amdkfd_device_probe(adev);
+
+
/* 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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH] drm/amdgpu: Get atomicOps info from Host for sriov setup
2021-09-09 15:59 shaoyunl
@ 2021-09-09 16:17 ` Felix Kuehling
2021-09-09 17:19 ` Liu, Shaoyun
0 siblings, 1 reply; 11+ messages in thread
From: Felix Kuehling @ 2021-09-09 16:17 UTC (permalink / raw)
To: shaoyunl, amd-gfx
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@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 {
^ permalink raw reply [flat|nested] 11+ messages in thread* RE: [PATCH] drm/amdgpu: Get atomicOps info from Host for sriov setup
2021-09-09 16:17 ` Felix Kuehling
@ 2021-09-09 17:19 ` Liu, Shaoyun
0 siblings, 0 replies; 11+ messages in thread
From: Liu, Shaoyun @ 2021-09-09 17:19 UTC (permalink / raw)
To: Kuehling, Felix, amd-gfx@lists.freedesktop.org
[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 <Felix.Kuehling@amd.com>
Sent: Thursday, September 9, 2021 12:18 PM
To: Liu, Shaoyun <Shaoyun.Liu@amd.com>; 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 <shaoyun.liu@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 {
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-09-10 19:28 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-09-10 14:04 [PATCH] drm/amdgpu: Get atomicOps info from Host for sriov setup shaoyunl
2021-09-10 14:19 ` Felix Kuehling
2021-09-10 14:23 ` Liu, Shaoyun
-- strict thread matches above, loose matches on Subject: below --
2021-09-10 15:15 shaoyunl
2021-09-10 19:28 ` Felix Kuehling
2021-09-10 14:26 shaoyunl
2021-09-10 14:33 ` Felix Kuehling
2021-09-10 14:38 ` Chen, Guchun
2021-09-09 15:59 shaoyunl
2021-09-09 16:17 ` Felix Kuehling
2021-09-09 17:19 ` Liu, Shaoyun
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.