diff mbox series

[1/1] AMDGPU: Fix bug where DPM is not enabled after hibernate and resume

Message ID 0d9e084d-0bb1-6dbe-f8c5-22d4fdb9c989@gmail.com (mailing list archive)
State New, archived
Headers show
Series [1/1] AMDGPU: Fix bug where DPM is not enabled after hibernate and resume | expand

Commit Message

Sandeep Aug. 6, 2020, 5:22 p.m. UTC
This fixes the bug I reported here: https://bugzilla.kernel.org/show_bug.cgi?id=208839

Reproducing bug report here:
After hibernating and resuming, DPM is not enabled. This remains the case even if you test hibernate using the steps here: https://www.kernel.org/doc/html/latest/power/basic-pm-debugging.html

I debugged the problem, and figured out that in the file hardwaremanager.c, in the function, phm_enable_dynamic_state_management(), the check 'if (!hwmgr->pp_one_vf && smum_is_dpm_running(hwmgr) && !amdgpu_passthrough(adev) && adev->in_suspend)' returns true for the hibernate case, and false for the suspend case.

This means that for the hibernate case, the AMDGPU driver doesn't enable DPM (even though it should) and simply returns from that function. In the suspend case, it goes ahead and enables DPM, even though it doesn't need to.

I debugged further, and found out that in the case of suspend, for the CIK/Hawaii GPUs, smum_is_dpm_running(hwmgr) returns false, while in the case of hibernate, smum_is_dpm_running(hwmgr) returns true.

For CIK, the ci_is_dpm_running() function calls the ci_is_smc_ram_running() function, which is ultimately used to determine if DPM is currently enabled or not, and this seems to provide the wrong answer.

I've changed the ci_is_dpm_running() function to instead use the same method that some other AMD GPU chips do (e.g Fiji), which seems to read the voltage controller. I've tested on my R9 390 and it seems to work correctly for both suspend and hibernate use cases, and has been stable so far.

Signed-off-by: Sandeep Raghuraman <sandy.8925@gmail.com>

---
 drivers/gpu/drm/amd/powerplay/smumgr/ci_smumgr.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Comments

Alex Deucher Aug. 7, 2020, 7:54 p.m. UTC | #1
Applied.  Thanks!

Alex

On Thu, Aug 6, 2020 at 1:22 PM Sandeep Raghuraman <sandy.8925@gmail.com> wrote:
>
> This fixes the bug I reported here: https://bugzilla.kernel.org/show_bug.cgi?id=208839
>
> Reproducing bug report here:
> After hibernating and resuming, DPM is not enabled. This remains the case even if you test hibernate using the steps here: https://www.kernel.org/doc/html/latest/power/basic-pm-debugging.html
>
> I debugged the problem, and figured out that in the file hardwaremanager.c, in the function, phm_enable_dynamic_state_management(), the check 'if (!hwmgr->pp_one_vf && smum_is_dpm_running(hwmgr) && !amdgpu_passthrough(adev) && adev->in_suspend)' returns true for the hibernate case, and false for the suspend case.
>
> This means that for the hibernate case, the AMDGPU driver doesn't enable DPM (even though it should) and simply returns from that function. In the suspend case, it goes ahead and enables DPM, even though it doesn't need to.
>
> I debugged further, and found out that in the case of suspend, for the CIK/Hawaii GPUs, smum_is_dpm_running(hwmgr) returns false, while in the case of hibernate, smum_is_dpm_running(hwmgr) returns true.
>
> For CIK, the ci_is_dpm_running() function calls the ci_is_smc_ram_running() function, which is ultimately used to determine if DPM is currently enabled or not, and this seems to provide the wrong answer.
>
> I've changed the ci_is_dpm_running() function to instead use the same method that some other AMD GPU chips do (e.g Fiji), which seems to read the voltage controller. I've tested on my R9 390 and it seems to work correctly for both suspend and hibernate use cases, and has been stable so far.
>
> Signed-off-by: Sandeep Raghuraman <sandy.8925@gmail.com>
>
> ---
>  drivers/gpu/drm/amd/powerplay/smumgr/ci_smumgr.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/ci_smumgr.c b/drivers/gpu/drm/amd/powerplay/smumgr/ci_smumgr.c
> index 56923a96b450..056b3ad1565f 100644
> --- a/drivers/gpu/drm/amd/powerplay/smumgr/ci_smumgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/smumgr/ci_smumgr.c
> @@ -2725,7 +2725,10 @@ static int ci_initialize_mc_reg_table(struct pp_hwmgr *hwmgr)
>
>  static bool ci_is_dpm_running(struct pp_hwmgr *hwmgr)
>  {
> -       return ci_is_smc_ram_running(hwmgr);
> +       return (1 == PHM_READ_INDIRECT_FIELD(hwmgr->device,
> +                                            CGS_IND_REG__SMC, FEATURE_STATUS,
> +                                     VOLTAGE_CONTROLLER_ON))
> +                       ? true : false;
>  }
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/ci_smumgr.c b/drivers/gpu/drm/amd/powerplay/smumgr/ci_smumgr.c
index 56923a96b450..056b3ad1565f 100644
--- a/drivers/gpu/drm/amd/powerplay/smumgr/ci_smumgr.c
+++ b/drivers/gpu/drm/amd/powerplay/smumgr/ci_smumgr.c
@@ -2725,7 +2725,10 @@  static int ci_initialize_mc_reg_table(struct pp_hwmgr *hwmgr)
 
 static bool ci_is_dpm_running(struct pp_hwmgr *hwmgr)
 {
-       return ci_is_smc_ram_running(hwmgr);
+       return (1 == PHM_READ_INDIRECT_FIELD(hwmgr->device,
+                                            CGS_IND_REG__SMC, FEATURE_STATUS, 
+                                     VOLTAGE_CONTROLLER_ON))
+                       ? true : false;
 }
_______________________________________________
dri-devel mailing list