Re: [PATCH 1/2] drm/amdgpu: use rpm_mode as runtime pm check flag

["Lazar, Lijo" <Lijo.Lazar@xxxxxxx>]

[AMD Official Use Only - General]

Runtime PM can be disabled with a module param. BACO state is supported for non-RPM use cases also like regular suspend or a reset. Relying on RPM mode for BACO state is not the right thing to do.

Thanks,
Lijo

---

From: Chen, Guchun <Guchun.Chen@xxxxxxx>
Sent: Monday, November 21, 2022 6:08:58 AM
To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>; Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Zhang, Hawking <Hawking.Zhang@xxxxxxx>; Lazar, Lijo <Lijo.Lazar@xxxxxxx>; Quan, Evan <Evan.Quan@xxxxxxx>
Cc: Chen, Guchun <Guchun.Chen@xxxxxxx>
Subject: [PATCH 1/2] drm/amdgpu: use rpm_mode as runtime pm check flag

 

Driver was not calling BACO exit at all in runtime pm
resume, and it caused the timing issue leading to a PCI
AER error, as once system enters BACO, it's not reliable
to check runtime pm mode by talking to SMU. So use rpm_mode
instead as a general pm mode check to ensure driver executes
BACO exit in runtime pm resume.

Fixes: 8795e182b02d ("PCI/portdrv: Don't disable AER reporting in get_port_device_capability()")

Signed-off-by: Guchun Chen <guchun.chen@xxxxxxx>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    | 25 +++++++++++-----------
 2 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index ab8f970b2849..40af21040b47 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -5619,7 +5619,7 @@ int amdgpu_device_baco_enter(struct drm_device *dev)
         struct amdgpu_device *adev = drm_to_adev(dev);
         struct amdgpu_ras *ras = amdgpu_ras_get_context(adev);
 
-       if (!amdgpu_device_supports_baco(adev_to_drm(adev)))
+       if (adev->pm.rpm_mode != AMDGPU_RUNPM_BACO)
                 return -ENOTSUPP;
 
         if (ras && adev->ras_enabled &&
@@ -5635,7 +5635,7 @@ int amdgpu_device_baco_exit(struct drm_device *dev)
         struct amdgpu_ras *ras = amdgpu_ras_get_context(adev);
         int ret = 0;
 
-       if (!amdgpu_device_supports_baco(adev_to_drm(adev)))
+       if (adev->pm.rpm_mode != AMDGPU_RUNPM_BACO)
                 return -ENOTSUPP;
 
         ret = amdgpu_dpm_baco_exit(adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 3c9fecdd6b2f..be03f7b1cee1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2532,7 +2532,7 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
         }
 
         adev->in_runpm = true;
-       if (amdgpu_device_supports_px(drm_dev))
+       if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX)
                 drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
 
         /*
@@ -2542,21 +2542,21 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
          * platforms.
          * TODO: this may be also needed for PX capable platform.
          */
-       if (amdgpu_device_supports_boco(drm_dev))
+       if (adev->pm.rpm_mode == AMDGPU_RUNPM_BOCO)
                 adev->mp1_state = PP_MP1_STATE_UNLOAD;
 
         ret = amdgpu_device_suspend(drm_dev, false);
         if (ret) {
                 adev->in_runpm = false;
-               if (amdgpu_device_supports_boco(drm_dev))
+               if (adev->pm.rpm_mode == AMDGPU_RUNPM_BOCO)
                         adev->mp1_state = PP_MP1_STATE_NONE;
                 return ret;
         }
 
-       if (amdgpu_device_supports_boco(drm_dev))
+       if (adev->pm.rpm_mode == AMDGPU_RUNPM_BOCO)
                 adev->mp1_state = PP_MP1_STATE_NONE;
 
-       if (amdgpu_device_supports_px(drm_dev)) {
+       if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX) {
                 /* Only need to handle PCI state in the driver for ATPX
                  * PCI core handles it for _PR3.
                  */
@@ -2565,9 +2565,9 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
                 pci_ignore_hotplug(pdev);
                 pci_set_power_state(pdev, PCI_D3cold);
                 drm_dev->switch_power_state = DRM_SWITCH_POWER_DYNAMIC_OFF;
-       } else if (amdgpu_device_supports_boco(drm_dev)) {
+       } else if (adev->pm.rpm_mode == AMDGPU_RUNPM_BOCO) {
                 /* nothing to do */
-       } else if (amdgpu_device_supports_baco(drm_dev)) {
+       } else {
                 amdgpu_device_baco_enter(drm_dev);
         }
 
@@ -2588,7 +2588,7 @@ static int amdgpu_pmops_runtime_resume(struct device *dev)
         if (!pci_device_is_present(adev->pdev))
                 adev->no_hw_access = true;
 
-       if (amdgpu_device_supports_px(drm_dev)) {
+       if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX) {
                 drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
 
                 /* Only need to handle PCI state in the driver for ATPX
@@ -2600,22 +2600,23 @@ static int amdgpu_pmops_runtime_resume(struct device *dev)
                 if (ret)
                         return ret;
                 pci_set_master(pdev);
-       } else if (amdgpu_device_supports_boco(drm_dev)) {
+       } else if (adev->pm.rpm_mode == AMDGPU_RUNPM_BOCO) {
                 /* Only need to handle PCI state in the driver for ATPX
                  * PCI core handles it for _PR3.
                  */
                 pci_set_master(pdev);
-       } else if (amdgpu_device_supports_baco(drm_dev)) {
+       } else {
                 amdgpu_device_baco_exit(drm_dev);
         }
+
         ret = amdgpu_device_resume(drm_dev, false);
         if (ret) {
-               if (amdgpu_device_supports_px(drm_dev))
+               if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX)
                         pci_disable_device(pdev);
                 return ret;
         }
 
-       if (amdgpu_device_supports_px(drm_dev))
+       if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX)
                 drm_dev->switch_power_state = DRM_SWITCH_POWER_ON;
         adev->in_runpm = false;
         return 0;
--
2.25.1