[PATCH] drm/amd/powerplay: Partially revert changes and fix smu7_notify_smc_display()

[PATCH] drm/amd/powerplay: Partially revert changes and fix smu7_notify_smc_display()

[Tom St Denis]

This partially reverts 0b6b4cbf77c995a34a4ec3d705a636434dadc51a and fixes
the noise issues on Tonga.

Signed-off-by: Tom St Denis <tom.stdenis@amd.com>
---
 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
index 8dbe9148aad3..4826b2991b7e 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
@@ -3825,14 +3825,11 @@ static int smu7_notify_link_speed_change_after_state_change(
 static int smu7_notify_smc_display(struct pp_hwmgr *hwmgr)
 {
 	struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend);
-	int ret = 0;
 
-	if (hwmgr->feature_mask & PP_VBI_TIME_SUPPORT_MASK) {
+	if (hwmgr->feature_mask & PP_VBI_TIME_SUPPORT_MASK)
 		smum_send_msg_to_smc_with_parameter(hwmgr,
 			(PPSMC_Msg)PPSMC_MSG_SetVBITimeout, data->frame_time_x2);
-		ret = (smum_send_msg_to_smc(hwmgr, (PPSMC_Msg)PPSMC_HasDisplay) == 0) ?  0 : -EINVAL;
-	}
-	return ret;
+	return (smum_send_msg_to_smc(hwmgr, (PPSMC_Msg)PPSMC_HasDisplay) == 0) ?  0 : -EINVAL;
 }
 
 static int smu7_set_power_state_tasks(struct pp_hwmgr *hwmgr, const void *input)
-- 
2.12.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] drm/amd/powerplay: Partially revert changes and fix smu7_notify_smc_display()

[Deucher, Alexander]

> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf
> Of Tom St Denis
> Sent: Wednesday, October 04, 2017 1:49 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: StDenis, Tom
> Subject: [PATCH] drm/amd/powerplay: Partially revert changes and fix
> smu7_notify_smc_display()
> 
> This partially reverts 0b6b4cbf77c995a34a4ec3d705a636434dadc51a and fixes
> the noise issues on Tonga.
> 
> Signed-off-by: Tom St Denis <tom.stdenis@amd.com>

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> index 8dbe9148aad3..4826b2991b7e 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> @@ -3825,14 +3825,11 @@ static int
> smu7_notify_link_speed_change_after_state_change(
>  static int smu7_notify_smc_display(struct pp_hwmgr *hwmgr)
>  {
>  	struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr-
> >backend);
> -	int ret = 0;
> 
> -	if (hwmgr->feature_mask & PP_VBI_TIME_SUPPORT_MASK) {
> +	if (hwmgr->feature_mask & PP_VBI_TIME_SUPPORT_MASK)
>  		smum_send_msg_to_smc_with_parameter(hwmgr,
>  			(PPSMC_Msg)PPSMC_MSG_SetVBITimeout, data-
> >frame_time_x2);
> -		ret = (smum_send_msg_to_smc(hwmgr,
> (PPSMC_Msg)PPSMC_HasDisplay) == 0) ?  0 : -EINVAL;
> -	}
> -	return ret;
> +	return (smum_send_msg_to_smc(hwmgr,
> (PPSMC_Msg)PPSMC_HasDisplay) == 0) ?  0 : -EINVAL;
>  }
> 
>  static int smu7_set_power_state_tasks(struct pp_hwmgr *hwmgr, const
> void *input)
> --
> 2.12.0
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amd/powerplay: Partially revert changes and fix smu7_notify_smc_display()

[Andy Furniss]

For me testing on 4.15-wip with tonga it does "fix", but memclk is 
stuck, which would have avoided the lines had I forced it without this.

Maybe you see different on a different kernel - is memclk stuck for you 
with this?

Tom St Denis wrote:
> This partially reverts 0b6b4cbf77c995a34a4ec3d705a636434dadc51a and fixes
> the noise issues on Tonga.
> 
> Signed-off-by: Tom St Denis <tom.stdenis@amd.com>
> ---
>   drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 7 ++-----
>   1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> index 8dbe9148aad3..4826b2991b7e 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> @@ -3825,14 +3825,11 @@ static int smu7_notify_link_speed_change_after_state_change(
>   static int smu7_notify_smc_display(struct pp_hwmgr *hwmgr)
>   {
>   	struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend);
> -	int ret = 0;
>   
> -	if (hwmgr->feature_mask & PP_VBI_TIME_SUPPORT_MASK) {
> +	if (hwmgr->feature_mask & PP_VBI_TIME_SUPPORT_MASK)
>   		smum_send_msg_to_smc_with_parameter(hwmgr,
>   			(PPSMC_Msg)PPSMC_MSG_SetVBITimeout, data->frame_time_x2);
> -		ret = (smum_send_msg_to_smc(hwmgr, (PPSMC_Msg)PPSMC_HasDisplay) == 0) ?  0 : -EINVAL;
> -	}
> -	return ret;
> +	return (smum_send_msg_to_smc(hwmgr, (PPSMC_Msg)PPSMC_HasDisplay) == 0) ?  0 : -EINVAL;
>   }
>   
>   static int smu7_set_power_state_tasks(struct pp_hwmgr *hwmgr, const void *input)
> 

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amd/powerplay: Partially revert changes and fix smu7_notify_smc_display()

[Tom St Denis]

Hi Andy,

I didn't check that but this part of the original patch:

@@ -689,7 +697,7 @@ static int smu7_setup_dpm_tables_v0(struct pp_hwmgr 
*hwmgr)
                                 allowed_vdd_sclk_table->entries[i].clk) {
 
data->dpm_table.sclk_table.dpm_levels[data->dpm_table.sclk_table.count].value 
=
                                 allowed_vdd_sclk_table->entries[i].clk;
- 
data->dpm_table.sclk_table.dpm_levels[data->dpm_table.sclk_table.count].enabled 
= 1; /*(i==0) ? 1 : 0; to do */
+ 
data->dpm_table.sclk_table.dpm_levels[data->dpm_table.sclk_table.count].enabled 
= (i == 0) ? 1 : 0;
                         data->dpm_table.sclk_table.count++;
                 }
         }
@@ -703,7 +711,7 @@ static int smu7_setup_dpm_tables_v0(struct pp_hwmgr 
*hwmgr)
                         allowed_vdd_mclk_table->entries[i].clk) {
 
data->dpm_table.mclk_table.dpm_levels[data->dpm_table.mclk_table.count].value 
=
                                 allowed_vdd_mclk_table->entries[i].clk;
- 
data->dpm_table.mclk_table.dpm_levels[data->dpm_table.mclk_table.count].enabled 
= 1; /*(i==0) ? 1 : 0; */
+ 
data->dpm_table.mclk_table.dpm_levels[data->dpm_table.mclk_table.count].enabled 
= (i == 0) ? 1 : 0;
                         data->dpm_table.mclk_table.count++;
                 }

Might be the culprit.  It seems to disable all but the first clock 
settings.  It's beyond EOD for me today but I'll check first thing in 
the morning.

Cheers,

Tom

On 04/10/17 04:21 PM, Andy Furniss wrote:
> For me testing on 4.15-wip with tonga it does "fix", but memclk is 
> stuck, which would have avoided the lines had I forced it without this.
> 
> Maybe you see different on a different kernel - is memclk stuck for you 
> with this?
> 
> Tom St Denis wrote:
>> This partially reverts 0b6b4cbf77c995a34a4ec3d705a636434dadc51a and fixes
>> the noise issues on Tonga.
>>
>> Signed-off-by: Tom St Denis <tom.stdenis@amd.com>
>> ---
>>   drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 7 ++-----
>>   1 file changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c 
>> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> index 8dbe9148aad3..4826b2991b7e 100644
>> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> @@ -3825,14 +3825,11 @@ static int 
>> smu7_notify_link_speed_change_after_state_change(
>>   static int smu7_notify_smc_display(struct pp_hwmgr *hwmgr)
>>   {
>>       struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend);
>> -    int ret = 0;
>> -    if (hwmgr->feature_mask & PP_VBI_TIME_SUPPORT_MASK) {
>> +    if (hwmgr->feature_mask & PP_VBI_TIME_SUPPORT_MASK)
>>           smum_send_msg_to_smc_with_parameter(hwmgr,
>>               (PPSMC_Msg)PPSMC_MSG_SetVBITimeout, data->frame_time_x2);
>> -        ret = (smum_send_msg_to_smc(hwmgr, 
>> (PPSMC_Msg)PPSMC_HasDisplay) == 0) ?  0 : -EINVAL;
>> -    }
>> -    return ret;
>> +    return (smum_send_msg_to_smc(hwmgr, (PPSMC_Msg)PPSMC_HasDisplay) 
>> == 0) ?  0 : -EINVAL;
>>   }
>>   static int smu7_set_power_state_tasks(struct pp_hwmgr *hwmgr, const 
>> void *input)
>>
> 

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amd/powerplay: Partially revert changes and fix smu7_notify_smc_display()

[Tom St Denis]

I reset to before offending commit and mclk is still pegged at low.  So 
this commit isn't responsible for this.

I'll bisect and try to hunt it down.

Tom

On 04/10/17 04:52 PM, Tom St Denis wrote:
> Hi Andy,
> 
> I didn't check that but this part of the original patch:
> 
> @@ -689,7 +697,7 @@ static int smu7_setup_dpm_tables_v0(struct pp_hwmgr 
> *hwmgr)
>                                  allowed_vdd_sclk_table->entries[i].clk) {
> 
> data->dpm_table.sclk_table.dpm_levels[data->dpm_table.sclk_table.count].value 
> =
>                                  allowed_vdd_sclk_table->entries[i].clk;
> - 
> data->dpm_table.sclk_table.dpm_levels[data->dpm_table.sclk_table.count].enabled 
> = 1; /*(i==0) ? 1 : 0; to do */
> + 
> data->dpm_table.sclk_table.dpm_levels[data->dpm_table.sclk_table.count].enabled 
> = (i == 0) ? 1 : 0;
>                          data->dpm_table.sclk_table.count++;
>                  }
>          }
> @@ -703,7 +711,7 @@ static int smu7_setup_dpm_tables_v0(struct pp_hwmgr 
> *hwmgr)
>                          allowed_vdd_mclk_table->entries[i].clk) {
> 
> data->dpm_table.mclk_table.dpm_levels[data->dpm_table.mclk_table.count].value 
> =
>                                  allowed_vdd_mclk_table->entries[i].clk;
> - 
> data->dpm_table.mclk_table.dpm_levels[data->dpm_table.mclk_table.count].enabled 
> = 1; /*(i==0) ? 1 : 0; */
> + 
> data->dpm_table.mclk_table.dpm_levels[data->dpm_table.mclk_table.count].enabled 
> = (i == 0) ? 1 : 0;
>                          data->dpm_table.mclk_table.count++;
>                  }
> 
> Might be the culprit.  It seems to disable all but the first clock 
> settings.  It's beyond EOD for me today but I'll check first thing in 
> the morning.
> 
> Cheers,
> 
> Tom
> 
> On 04/10/17 04:21 PM, Andy Furniss wrote:
>> For me testing on 4.15-wip with tonga it does "fix", but memclk is 
>> stuck, which would have avoided the lines had I forced it without this.
>>
>> Maybe you see different on a different kernel - is memclk stuck for 
>> you with this?
>>
>> Tom St Denis wrote:
>>> This partially reverts 0b6b4cbf77c995a34a4ec3d705a636434dadc51a and 
>>> fixes
>>> the noise issues on Tonga.
>>>
>>> Signed-off-by: Tom St Denis <tom.stdenis@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 7 ++-----
>>>   1 file changed, 2 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c 
>>> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>>> index 8dbe9148aad3..4826b2991b7e 100644
>>> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>>> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>>> @@ -3825,14 +3825,11 @@ static int 
>>> smu7_notify_link_speed_change_after_state_change(
>>>   static int smu7_notify_smc_display(struct pp_hwmgr *hwmgr)
>>>   {
>>>       struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend);
>>> -    int ret = 0;
>>> -    if (hwmgr->feature_mask & PP_VBI_TIME_SUPPORT_MASK) {
>>> +    if (hwmgr->feature_mask & PP_VBI_TIME_SUPPORT_MASK)
>>>           smum_send_msg_to_smc_with_parameter(hwmgr,
>>>               (PPSMC_Msg)PPSMC_MSG_SetVBITimeout, data->frame_time_x2);
>>> -        ret = (smum_send_msg_to_smc(hwmgr, 
>>> (PPSMC_Msg)PPSMC_HasDisplay) == 0) ?  0 : -EINVAL;
>>> -    }
>>> -    return ret;
>>> +    return (smum_send_msg_to_smc(hwmgr, (PPSMC_Msg)PPSMC_HasDisplay) 
>>> == 0) ?  0 : -EINVAL;
>>>   }
>>>   static int smu7_set_power_state_tasks(struct pp_hwmgr *hwmgr, const 
>>> void *input)
>>>
>>
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx