[PATCH] drm/amdgpu/pm: fix the Stable pstate Test in amdgpu_test

[PATCH] drm/amdgpu/pm: fix the Stable pstate Test in amdgpu_test

[Yifan Zhang]

If GFX DPM is disbaled, Stable pstate Test in amdgpu_test fails.
Check GFX DPM statue before change clock level

Log:
[   46.595274] [drm] Initialized amdgpu 3.46.0 20150101 for 0000:02:00.0 on minor 0
[   46.599929] fbcon: amdgpudrmfb (fb0) is primary device
[   46.785753] Console: switching to colour frame buffer device 240x67
[   46.811765] amdgpu 0000:02:00.0: [drm] fb0: amdgpudrmfb frame buffer device
[  131.398407] amdgpu 0000:02:00.0: amdgpu: Failed to set performance level!

Signed-off-by: Yifan Zhang <yifan1.zhang@amd.com>
---
 drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_5_ppt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_5_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_5_ppt.c
index 7bfac029e513..b81711c4ff33 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_5_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_5_ppt.c
@@ -991,7 +991,7 @@ static int smu_v13_0_5_set_performance_level(struct smu_context *smu,
 		return -EINVAL;
 	}
 
-	if (sclk_min && sclk_max) {
+	if (sclk_min && sclk_max && smu_v13_0_5_clk_dpm_is_enabled(smu, SMU_SCLK)) {
 		ret = smu_v13_0_5_set_soft_freq_limited_range(smu,
 							    SMU_SCLK,
 							    sclk_min,
-- 
2.35.1

Re: [PATCH] drm/amdgpu/pm: fix the Stable pstate Test in amdgpu_test

[Huang Rui]

On Tue, Mar 15, 2022 at 06:25:51PM +0800, Zhang, Yifan wrote:
> If GFX DPM is disbaled, Stable pstate Test in amdgpu_test fails.
> Check GFX DPM statue before change clock level
> 
> Log:
> [   46.595274] [drm] Initialized amdgpu 3.46.0 20150101 for 0000:02:00.0 on minor 0
> [   46.599929] fbcon: amdgpudrmfb (fb0) is primary device
> [   46.785753] Console: switching to colour frame buffer device 240x67
> [   46.811765] amdgpu 0000:02:00.0: [drm] fb0: amdgpudrmfb frame buffer device
> [  131.398407] amdgpu 0000:02:00.0: amdgpu: Failed to set performance level!
> 
> Signed-off-by: Yifan Zhang <yifan1.zhang@amd.com>
> ---
>  drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_5_ppt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_5_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_5_ppt.c
> index 7bfac029e513..b81711c4ff33 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_5_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_5_ppt.c
> @@ -991,7 +991,7 @@ static int smu_v13_0_5_set_performance_level(struct smu_context *smu,
>  		return -EINVAL;
>  	}
>  
> -	if (sclk_min && sclk_max) {
> +	if (sclk_min && sclk_max && smu_v13_0_5_clk_dpm_is_enabled(smu, SMU_SCLK)) {

SMU driver actually checked smu->pm_enabled and smu->adev->pm.dpm_enabled
in smu_force_performance_level. I am confused why these two flags are true
while the smu v13.0.5's dpm is disabled.

Thanks,
Ray

RE: [PATCH] drm/amdgpu/pm: fix the Stable pstate Test in amdgpu_test

[Zhang, Yifan]

[AMD Official Use Only]

I think It is a swsmu issue rather than a smu 13.0.5 specific one, in current swsmu implementation, smu->adev->pm.dpm_enabled is always true after smu_hw_init, doesn't reflect the real DPM status; smu->pm_enabled indicates SMU functionality, also always true except in sriov.

-----Original Message-----
From: Huang, Ray <Ray.Huang@amd.com> 
Sent: Tuesday, March 15, 2022 8:12 PM
To: Zhang, Yifan <Yifan1.Zhang@amd.com>
Cc: amd-gfx@lists.freedesktop.org; Deucher, Alexander <Alexander.Deucher@amd.com>
Subject: Re: [PATCH] drm/amdgpu/pm: fix the Stable pstate Test in amdgpu_test

On Tue, Mar 15, 2022 at 06:25:51PM +0800, Zhang, Yifan wrote:
> If GFX DPM is disbaled, Stable pstate Test in amdgpu_test fails.
> Check GFX DPM statue before change clock level
> 
> Log:
> [   46.595274] [drm] Initialized amdgpu 3.46.0 20150101 for 0000:02:00.0 on minor 0
> [   46.599929] fbcon: amdgpudrmfb (fb0) is primary device
> [   46.785753] Console: switching to colour frame buffer device 240x67
> [   46.811765] amdgpu 0000:02:00.0: [drm] fb0: amdgpudrmfb frame buffer device
> [  131.398407] amdgpu 0000:02:00.0: amdgpu: Failed to set performance level!
> 
> Signed-off-by: Yifan Zhang <yifan1.zhang@amd.com>
> ---
>  drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_5_ppt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_5_ppt.c 
> b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_5_ppt.c
> index 7bfac029e513..b81711c4ff33 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_5_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_5_ppt.c
> @@ -991,7 +991,7 @@ static int smu_v13_0_5_set_performance_level(struct smu_context *smu,
>  		return -EINVAL;
>  	}
>  
> -	if (sclk_min && sclk_max) {
> +	if (sclk_min && sclk_max && smu_v13_0_5_clk_dpm_is_enabled(smu, 
> +SMU_SCLK)) {

SMU driver actually checked smu->pm_enabled and smu->adev->pm.dpm_enabled in smu_force_performance_level. I am confused why these two flags are true while the smu v13.0.5's dpm is disabled.

Thanks,
Ray

RE: [PATCH] drm/amdgpu/pm: fix the Stable pstate Test in amdgpu_test

[Zhang, Yifan]

[AMD Official Use Only]

Since it is a generic swsmu issue, shall we merge it first as temporary fix to unblock amdgpu_test ? As most of swsmu_pm_funcs have an assumption that pm.dpm_enabled is true, if allow this value is be false, Lots of swsmu pm logic needs to be changed.

-----Original Message-----
From: Zhang, Yifan 
Sent: Tuesday, March 15, 2022 8:42 PM
To: Huang, Ray <Ray.Huang@amd.com>
Cc: amd-gfx@lists.freedesktop.org; Deucher, Alexander <Alexander.Deucher@amd.com>
Subject: RE: [PATCH] drm/amdgpu/pm: fix the Stable pstate Test in amdgpu_test

[AMD Official Use Only]

I think It is a swsmu issue rather than a smu 13.0.5 specific one, in current swsmu implementation, smu->adev->pm.dpm_enabled is always true after smu_hw_init, doesn't reflect the real DPM status; smu->pm_enabled indicates SMU functionality, also always true except in sriov.

-----Original Message-----
From: Huang, Ray <Ray.Huang@amd.com> 
Sent: Tuesday, March 15, 2022 8:12 PM
To: Zhang, Yifan <Yifan1.Zhang@amd.com>
Cc: amd-gfx@lists.freedesktop.org; Deucher, Alexander <Alexander.Deucher@amd.com>
Subject: Re: [PATCH] drm/amdgpu/pm: fix the Stable pstate Test in amdgpu_test

On Tue, Mar 15, 2022 at 06:25:51PM +0800, Zhang, Yifan wrote:
> If GFX DPM is disbaled, Stable pstate Test in amdgpu_test fails.
> Check GFX DPM statue before change clock level
> 
> Log:
> [   46.595274] [drm] Initialized amdgpu 3.46.0 20150101 for 0000:02:00.0 on minor 0
> [   46.599929] fbcon: amdgpudrmfb (fb0) is primary device
> [   46.785753] Console: switching to colour frame buffer device 240x67
> [   46.811765] amdgpu 0000:02:00.0: [drm] fb0: amdgpudrmfb frame buffer device
> [  131.398407] amdgpu 0000:02:00.0: amdgpu: Failed to set performance level!
> 
> Signed-off-by: Yifan Zhang <yifan1.zhang@amd.com>
> ---
>  drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_5_ppt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_5_ppt.c 
> b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_5_ppt.c
> index 7bfac029e513..b81711c4ff33 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_5_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_5_ppt.c
> @@ -991,7 +991,7 @@ static int smu_v13_0_5_set_performance_level(struct smu_context *smu,
>  		return -EINVAL;
>  	}
>  
> -	if (sclk_min && sclk_max) {
> +	if (sclk_min && sclk_max && smu_v13_0_5_clk_dpm_is_enabled(smu, 
> +SMU_SCLK)) {

SMU driver actually checked smu->pm_enabled and smu->adev->pm.dpm_enabled in smu_force_performance_level. I am confused why these two flags are true while the smu v13.0.5's dpm is disabled.

Thanks,
Ray

Re: [PATCH] drm/amdgpu/pm: fix the Stable pstate Test in amdgpu_test

[Huang Rui]

On Thu, Mar 17, 2022 at 11:14:53AM +0800, Zhang, Yifan wrote:
> [AMD Official Use Only]
> 
> Since it is a generic swsmu issue, shall we merge it first as temporary fix to unblock amdgpu_test ? As most of swsmu_pm_funcs have an assumption that pm.dpm_enabled is true, if allow this value is be false, Lots of swsmu pm logic needs to be changed.

(+ Evan)

This is actually a common issue for APU series. The best solution is to
modify the swSMU driver to revise pm.dpm_enabled to indicate the real DPM
status of current platform.

For now, I am fine to give the quick fix to unblock the amdgpu_test. You
may add the comment (FIXME) around the code to describe this issue and what
you plan for next step.

With that fixed, patch is Acked-by: Huang Rui <ray.huang@amd.com>

> 
> -----Original Message-----
> From: Zhang, Yifan 
> Sent: Tuesday, March 15, 2022 8:42 PM
> To: Huang, Ray <Ray.Huang@amd.com>
> Cc: amd-gfx@lists.freedesktop.org; Deucher, Alexander <Alexander.Deucher@amd.com>
> Subject: RE: [PATCH] drm/amdgpu/pm: fix the Stable pstate Test in amdgpu_test
> 
> [AMD Official Use Only]
> 
> I think It is a swsmu issue rather than a smu 13.0.5 specific one, in current swsmu implementation, smu->adev->pm.dpm_enabled is always true after smu_hw_init, doesn't reflect the real DPM status; smu->pm_enabled indicates SMU functionality, also always true except in sriov.
> 
> -----Original Message-----
> From: Huang, Ray <Ray.Huang@amd.com> 
> Sent: Tuesday, March 15, 2022 8:12 PM
> To: Zhang, Yifan <Yifan1.Zhang@amd.com>
> Cc: amd-gfx@lists.freedesktop.org; Deucher, Alexander <Alexander.Deucher@amd.com>
> Subject: Re: [PATCH] drm/amdgpu/pm: fix the Stable pstate Test in amdgpu_test
> 
> On Tue, Mar 15, 2022 at 06:25:51PM +0800, Zhang, Yifan wrote:
> > If GFX DPM is disbaled, Stable pstate Test in amdgpu_test fails.
> > Check GFX DPM statue before change clock level
> > 
> > Log:
> > [   46.595274] [drm] Initialized amdgpu 3.46.0 20150101 for 0000:02:00.0 on minor 0
> > [   46.599929] fbcon: amdgpudrmfb (fb0) is primary device
> > [   46.785753] Console: switching to colour frame buffer device 240x67
> > [   46.811765] amdgpu 0000:02:00.0: [drm] fb0: amdgpudrmfb frame buffer device
> > [  131.398407] amdgpu 0000:02:00.0: amdgpu: Failed to set performance level!
> > 
> > Signed-off-by: Yifan Zhang <yifan1.zhang@amd.com>
> > ---
> >  drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_5_ppt.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_5_ppt.c 
> > b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_5_ppt.c
> > index 7bfac029e513..b81711c4ff33 100644
> > --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_5_ppt.c
> > +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_5_ppt.c
> > @@ -991,7 +991,7 @@ static int smu_v13_0_5_set_performance_level(struct smu_context *smu,
> >  		return -EINVAL;
> >  	}
> >  
> > -	if (sclk_min && sclk_max) {
> > +	if (sclk_min && sclk_max && smu_v13_0_5_clk_dpm_is_enabled(smu, 
> > +SMU_SCLK)) {
> 
> SMU driver actually checked smu->pm_enabled and smu->adev->pm.dpm_enabled in smu_force_performance_level. I am confused why these two flags are true while the smu v13.0.5's dpm is disabled.
> 
> Thanks,
> Ray