Re: [PATCH] drm/amd/powerplay: bypass fan table setup if no fan connected

[Bernhard Froemel]

On Thu, Dec 1, 2016 at 3:59 PM, Deucher, Alexander
 wrote:
>> -Original Message-
>> From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf
>> Of Hawking Zhang
>> Sent: Thursday, December 01, 2016 4:16 AM
>> To: amd-gfx@lists.freedesktop.org
>> Cc: Zhang, Hawking
>> Subject: [PATCH] drm/amd/powerplay: bypass fan table setup if no fan
>> connected
>>
>> If vBIOS noFan bit is set, the fan table parameters in thermal controller
>> will not get initialized. The driver should avoid to use these uninitialized
>> parameter to do calculation. Otherwise, it may trigger divide 0 error.
>>
>> Change-Id: I76680a5ec5411f59742b65bb70eb7b4a08bda3ef
>> Signed-off-by: Hawking Zhang 
>
> Reviewed-by: Alex Deucher 
Tested-by: Bernhard Froemel 

Thanks, this exactly fixes one of the issues with the MacBookPro13,3:
https://lists.freedesktop.org/archives/amd-gfx/2016-November/003673.html

>
>> ---
>>  drivers/gpu/drm/amd/powerplay/smumgr/fiji_smc.c  | 6 ++
>>  drivers/gpu/drm/amd/powerplay/smumgr/iceland_smc.c   | 6 ++
>>  drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smc.c | 6 ++
>>  drivers/gpu/drm/amd/powerplay/smumgr/tonga_smc.c | 6 ++
>>  4 files changed, 24 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smc.c
>> b/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smc.c
>> index 34523fe..6aeb1d2 100644
>> --- a/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smc.c
>> +++ b/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smc.c
>> @@ -1958,6 +1958,12 @@ int fiji_thermal_setup_fan_table(struct
>> pp_hwmgr *hwmgr)
>>   int res;
>>   uint64_t tmp64;
>>
>> + if (hwmgr->thermal_controller.fanInfo.bNoFan) {
>> + phm_cap_unset(hwmgr->platform_descriptor.platformCaps,
>> + PHM_PlatformCaps_MicrocodeFanControl);
>> + return 0;
>> + }
>> +
>>   if (smu_data->smu7_data.fan_table_start == 0) {
>>   phm_cap_unset(hwmgr->platform_descriptor.platformCaps,
>>   PHM_PlatformCaps_MicrocodeFanControl);
>> diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/iceland_smc.c
>> b/drivers/gpu/drm/amd/powerplay/smumgr/iceland_smc.c
>> index b579f0c..a24971a 100644
>> --- a/drivers/gpu/drm/amd/powerplay/smumgr/iceland_smc.c
>> +++ b/drivers/gpu/drm/amd/powerplay/smumgr/iceland_smc.c
>> @@ -2006,6 +2006,12 @@ int iceland_thermal_setup_fan_table(struct
>> pp_hwmgr *hwmgr)
>>   if (!phm_cap_enabled(hwmgr->platform_descriptor.platformCaps,
>> PHM_PlatformCaps_MicrocodeFanControl))
>>   return 0;
>>
>> + if (hwmgr->thermal_controller.fanInfo.bNoFan) {
>> + phm_cap_unset(hwmgr->platform_descriptor.platformCaps,
>> + PHM_PlatformCaps_MicrocodeFanControl);
>> + return 0;
>> + }
>> +
>>   if (0 == smu7_data->fan_table_start) {
>>   phm_cap_unset(hwmgr->platform_descriptor.platformCaps,
>> PHM_PlatformCaps_MicrocodeFanControl);
>>   return 0;
>> diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smc.c
>> b/drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smc.c
>> index 8db8e20..5190e82 100644
>> --- a/drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smc.c
>> +++ b/drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smc.c
>> @@ -1885,6 +1885,12 @@ int polaris10_thermal_setup_fan_table(struct
>> pp_hwmgr *hwmgr)
>>   int res;
>>   uint64_t tmp64;
>>
>> + if (hwmgr->thermal_controller.fanInfo.bNoFan) {
>> + phm_cap_unset(hwmgr->platform_descriptor.platformCaps,
>> + PHM_PlatformCaps_MicrocodeFanControl);
>> + return 0;
>> + }
>> +
>>   if (smu_data->smu7_data.fan_table_start == 0) {
>>   phm_cap_unset(hwmgr->platform_descriptor.platformCaps,
>>   PHM_PlatformCaps_MicrocodeFanControl);
>> diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/tonga_smc.c
>> b/drivers/gpu/drm/amd/powerplay/smumgr/tonga_smc.c
>> index d08f6f1..2e1493c 100644
>> --- a/drivers/gpu/drm/amd/powerplay/smumgr/tonga_smc.c
>> +++ b/drivers/gpu/drm/amd/powerplay/smumgr/tonga_smc.c
>> @@ -2496,6 +2496,12 @@ int tonga_thermal_setup_fan_table(struct
>> pp_hwmgr *hwmgr)
>>
>>   PHM_PlatformCaps_MicrocodeFanControl))
>>   return 0;
>>
>> + if (hwmgr->thermal_controller.fanInfo.bNoFan) {
>> + phm_cap_unset(hwmgr->platform_descriptor.platformCaps,
>> + PHM_PlatformCaps_MicrocodeFanControl);
>> + return 0;
>> + }
>> +
>>   if (0 == smu_data->smu7_data.fan_table_start) {
>>   phm_cap_unset(hwmgr->platform_descriptor.platformCaps,
>>
>>   PHM_PlatformCaps_MicrocodeFanControl);
>> --
>> 2.7.4
>>
>> ___
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> 

RE: [PATCH] drm/amd/powerplay: bypass fan table setup if no fan connected

[Deucher, Alexander]

> -Original Message-
> From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf
> Of Hawking Zhang
> Sent: Thursday, December 01, 2016 4:16 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Zhang, Hawking
> Subject: [PATCH] drm/amd/powerplay: bypass fan table setup if no fan
> connected
> 
> If vBIOS noFan bit is set, the fan table parameters in thermal controller
> will not get initialized. The driver should avoid to use these uninitialized
> parameter to do calculation. Otherwise, it may trigger divide 0 error.
> 
> Change-Id: I76680a5ec5411f59742b65bb70eb7b4a08bda3ef
> Signed-off-by: Hawking Zhang 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/powerplay/smumgr/fiji_smc.c  | 6 ++
>  drivers/gpu/drm/amd/powerplay/smumgr/iceland_smc.c   | 6 ++
>  drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smc.c | 6 ++
>  drivers/gpu/drm/amd/powerplay/smumgr/tonga_smc.c | 6 ++
>  4 files changed, 24 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smc.c
> b/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smc.c
> index 34523fe..6aeb1d2 100644
> --- a/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smc.c
> +++ b/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smc.c
> @@ -1958,6 +1958,12 @@ int fiji_thermal_setup_fan_table(struct
> pp_hwmgr *hwmgr)
>   int res;
>   uint64_t tmp64;
> 
> + if (hwmgr->thermal_controller.fanInfo.bNoFan) {
> + phm_cap_unset(hwmgr->platform_descriptor.platformCaps,
> + PHM_PlatformCaps_MicrocodeFanControl);
> + return 0;
> + }
> +
>   if (smu_data->smu7_data.fan_table_start == 0) {
>   phm_cap_unset(hwmgr->platform_descriptor.platformCaps,
>   PHM_PlatformCaps_MicrocodeFanControl);
> diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/iceland_smc.c
> b/drivers/gpu/drm/amd/powerplay/smumgr/iceland_smc.c
> index b579f0c..a24971a 100644
> --- a/drivers/gpu/drm/amd/powerplay/smumgr/iceland_smc.c
> +++ b/drivers/gpu/drm/amd/powerplay/smumgr/iceland_smc.c
> @@ -2006,6 +2006,12 @@ int iceland_thermal_setup_fan_table(struct
> pp_hwmgr *hwmgr)
>   if (!phm_cap_enabled(hwmgr->platform_descriptor.platformCaps,
> PHM_PlatformCaps_MicrocodeFanControl))
>   return 0;
> 
> + if (hwmgr->thermal_controller.fanInfo.bNoFan) {
> + phm_cap_unset(hwmgr->platform_descriptor.platformCaps,
> + PHM_PlatformCaps_MicrocodeFanControl);
> + return 0;
> + }
> +
>   if (0 == smu7_data->fan_table_start) {
>   phm_cap_unset(hwmgr->platform_descriptor.platformCaps,
> PHM_PlatformCaps_MicrocodeFanControl);
>   return 0;
> diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smc.c
> b/drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smc.c
> index 8db8e20..5190e82 100644
> --- a/drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smc.c
> +++ b/drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smc.c
> @@ -1885,6 +1885,12 @@ int polaris10_thermal_setup_fan_table(struct
> pp_hwmgr *hwmgr)
>   int res;
>   uint64_t tmp64;
> 
> + if (hwmgr->thermal_controller.fanInfo.bNoFan) {
> + phm_cap_unset(hwmgr->platform_descriptor.platformCaps,
> + PHM_PlatformCaps_MicrocodeFanControl);
> + return 0;
> + }
> +
>   if (smu_data->smu7_data.fan_table_start == 0) {
>   phm_cap_unset(hwmgr->platform_descriptor.platformCaps,
>   PHM_PlatformCaps_MicrocodeFanControl);
> diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/tonga_smc.c
> b/drivers/gpu/drm/amd/powerplay/smumgr/tonga_smc.c
> index d08f6f1..2e1493c 100644
> --- a/drivers/gpu/drm/amd/powerplay/smumgr/tonga_smc.c
> +++ b/drivers/gpu/drm/amd/powerplay/smumgr/tonga_smc.c
> @@ -2496,6 +2496,12 @@ int tonga_thermal_setup_fan_table(struct
> pp_hwmgr *hwmgr)
> 
>   PHM_PlatformCaps_MicrocodeFanControl))
>   return 0;
> 
> + if (hwmgr->thermal_controller.fanInfo.bNoFan) {
> + phm_cap_unset(hwmgr->platform_descriptor.platformCaps,
> + PHM_PlatformCaps_MicrocodeFanControl);
> + return 0;
> + }
> +
>   if (0 == smu_data->smu7_data.fan_table_start) {
>   phm_cap_unset(hwmgr->platform_descriptor.platformCaps,
> 
>   PHM_PlatformCaps_MicrocodeFanControl);
> --
> 2.7.4
> 
> ___
> 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