[AMD Official Use Only - General] Good point. Let me update it. Regards, Guchun From: Lazar, Lijo <Lijo.Lazar@xxxxxxx> Sent: Monday, November 21, 2022 10:26 AM To: Chen, Guchun <Guchun.Chen@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx; Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Zhang, Hawking <Hawking.Zhang@xxxxxxx>; Quan, Evan <Evan.Quan@xxxxxxx> Cc: Chen, Guchun <Guchun.Chen@xxxxxxx> Subject: Re: [PATCH 1/2] drm/amdgpu: use rpm_mode as runtime pm check flag [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<mailto:Guchun.Chen@xxxxxxx>> Sent: Monday, November 21, 2022 6:08:58 AM To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx<mailto:amd-gfx@xxxxxxxxxxxxxxxxxxxxx> <amd-gfx@xxxxxxxxxxxxxxxxxxxxx<mailto:amd-gfx@xxxxxxxxxxxxxxxxxxxxx>>; Deucher, Alexander <Alexander.Deucher@xxxxxxx<mailto:Alexander.Deucher@xxxxxxx>>; Zhang, Hawking <Hawking.Zhang@xxxxxxx<mailto:Hawking.Zhang@xxxxxxx>>; Lazar, Lijo <Lijo.Lazar@xxxxxxx<mailto:Lijo.Lazar@xxxxxxx>>; Quan, Evan <Evan.Quan@xxxxxxx<mailto:Evan.Quan@xxxxxxx>> Cc: Chen, Guchun <Guchun.Chen@xxxxxxx<mailto: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<mailto: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
<<attachment: winmail.dat>>
