Re: [PATCH] drm/amd/pm: disallow the fan setting if there is no fan on smu13

2023-08-08 Thread Lazar, Lijo




On 8/8/2023 3:56 PM, Feng, Kenneth wrote:

[AMD Official Use Only - General]

Currently no_fan is determined in sw init.
 if (!smu->ppt_funcs->get_fan_control_mode)
 smu->adev->pm.no_fan = true;

This is the case that some boards have fans and some don't have.
smu->ppt_funcs->get_fan_control_mode still need to be defined.
!smu_cmn_feature_is_supported(smu, SMU_FEATURE_FAN_CONTROL_BIT) is enough to 
get the fan capability.
Not sure if it's better to depend on pm.no_fan.


What I meant is, based on fan control feature bit you could set 
pm.no_fan flag.


When pm.no_fan is set, we won't create hwmon fan attributes for 
get/set[1]. That way you could avoid the other checks also. Also when 
PMFW is not controlling, it's not guaranteed that the fan controller is 
initialized correctly for get to return correct speed/pwm.


[1] 
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/amd/pm/amdgpu_pm.c#L3338


Thanks,
Lijo


Thanks.



-Original Message-
From: Lazar, Lijo 
Sent: Tuesday, August 8, 2023 6:12 PM
To: Feng, Kenneth ; amd-gfx@lists.freedesktop.org
Cc: Arif, Maisam 
Subject: Re: [PATCH] drm/amd/pm: disallow the fan setting if there is no fan on 
smu13



On 8/8/2023 1:21 PM, Kenneth Feng wrote:

disallow the fan setting if there is no fan on smu13

Signed-off-by: Kenneth Feng 
---
   drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c | 9 ++---
   1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
index 9b62b45ebb7f..09ef0a7e7679 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
@@ -1131,7 +1131,9 @@ smu_v13_0_display_clock_voltage_request(struct 
smu_context *smu,

   uint32_t smu_v13_0_get_fan_control_mode(struct smu_context *smu)
   {
- if (!smu_cmn_feature_is_enabled(smu, SMU_FEATURE_FAN_CONTROL_BIT))
+ if (!smu_cmn_feature_is_supported(smu, SMU_FEATURE_FAN_CONTROL_BIT))
+ return AMD_FAN_CTRL_NONE;


If there is no PMFW fan control, isn't it better to set pm.no_fan?

Thanks,
Lijo


+ else if (!smu_cmn_feature_is_enabled(smu, SMU_FEATURE_FAN_CONTROL_BIT))
   return AMD_FAN_CTRL_MANUAL;
   else
   return AMD_FAN_CTRL_AUTO;
@@ -1143,7 +1145,7 @@ smu_v13_0_auto_fan_control(struct smu_context *smu, bool 
auto_fan_control)
   int ret = 0;

   if (!smu_cmn_feature_is_supported(smu, SMU_FEATURE_FAN_CONTROL_BIT))
- return 0;
+ return -EINVAL;

   ret = smu_cmn_feature_set_enabled(smu, SMU_FEATURE_FAN_CONTROL_BIT, 
auto_fan_control);
   if (ret)
@@ -1204,7 +1206,8 @@ smu_v13_0_set_fan_control_mode(struct smu_context *smu,

   switch (mode) {
   case AMD_FAN_CTRL_NONE:
- ret = smu_v13_0_set_fan_speed_pwm(smu, 255);
+ if (smu_cmn_feature_is_supported(smu, 
SMU_FEATURE_FAN_CONTROL_BIT))
+ ret = -EINVAL;
   break;
   case AMD_FAN_CTRL_MANUAL:
   ret = smu_v13_0_auto_fan_control(smu, 0);


RE: [PATCH] drm/amd/pm: disallow the fan setting if there is no fan on smu13

2023-08-08 Thread Feng, Kenneth
[AMD Official Use Only - General]

Currently no_fan is determined in sw init.
if (!smu->ppt_funcs->get_fan_control_mode)
smu->adev->pm.no_fan = true;

This is the case that some boards have fans and some don't have.
smu->ppt_funcs->get_fan_control_mode still need to be defined.
!smu_cmn_feature_is_supported(smu, SMU_FEATURE_FAN_CONTROL_BIT) is enough to 
get the fan capability.
Not sure if it's better to depend on pm.no_fan.
Thanks.



-Original Message-
From: Lazar, Lijo 
Sent: Tuesday, August 8, 2023 6:12 PM
To: Feng, Kenneth ; amd-gfx@lists.freedesktop.org
Cc: Arif, Maisam 
Subject: Re: [PATCH] drm/amd/pm: disallow the fan setting if there is no fan on 
smu13



On 8/8/2023 1:21 PM, Kenneth Feng wrote:
> disallow the fan setting if there is no fan on smu13
>
> Signed-off-by: Kenneth Feng 
> ---
>   drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c | 9 ++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c 
> b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
> index 9b62b45ebb7f..09ef0a7e7679 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
> @@ -1131,7 +1131,9 @@ smu_v13_0_display_clock_voltage_request(struct 
> smu_context *smu,
>
>   uint32_t smu_v13_0_get_fan_control_mode(struct smu_context *smu)
>   {
> - if (!smu_cmn_feature_is_enabled(smu, SMU_FEATURE_FAN_CONTROL_BIT))
> + if (!smu_cmn_feature_is_supported(smu, SMU_FEATURE_FAN_CONTROL_BIT))
> + return AMD_FAN_CTRL_NONE;

If there is no PMFW fan control, isn't it better to set pm.no_fan?

Thanks,
Lijo

> + else if (!smu_cmn_feature_is_enabled(smu, SMU_FEATURE_FAN_CONTROL_BIT))
>   return AMD_FAN_CTRL_MANUAL;
>   else
>   return AMD_FAN_CTRL_AUTO;
> @@ -1143,7 +1145,7 @@ smu_v13_0_auto_fan_control(struct smu_context *smu, 
> bool auto_fan_control)
>   int ret = 0;
>
>   if (!smu_cmn_feature_is_supported(smu, SMU_FEATURE_FAN_CONTROL_BIT))
> - return 0;
> + return -EINVAL;
>
>   ret = smu_cmn_feature_set_enabled(smu, SMU_FEATURE_FAN_CONTROL_BIT, 
> auto_fan_control);
>   if (ret)
> @@ -1204,7 +1206,8 @@ smu_v13_0_set_fan_control_mode(struct smu_context *smu,
>
>   switch (mode) {
>   case AMD_FAN_CTRL_NONE:
> - ret = smu_v13_0_set_fan_speed_pwm(smu, 255);
> + if (smu_cmn_feature_is_supported(smu, 
> SMU_FEATURE_FAN_CONTROL_BIT))
> + ret = -EINVAL;
>   break;
>   case AMD_FAN_CTRL_MANUAL:
>   ret = smu_v13_0_auto_fan_control(smu, 0);


Re: [PATCH] drm/amd/pm: disallow the fan setting if there is no fan on smu13

2023-08-08 Thread Lazar, Lijo




On 8/8/2023 1:21 PM, Kenneth Feng wrote:

disallow the fan setting if there is no fan on smu13

Signed-off-by: Kenneth Feng 
---
  drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c | 9 ++---
  1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
index 9b62b45ebb7f..09ef0a7e7679 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
@@ -1131,7 +1131,9 @@ smu_v13_0_display_clock_voltage_request(struct 
smu_context *smu,
  
  uint32_t smu_v13_0_get_fan_control_mode(struct smu_context *smu)

  {
-   if (!smu_cmn_feature_is_enabled(smu, SMU_FEATURE_FAN_CONTROL_BIT))
+   if (!smu_cmn_feature_is_supported(smu, SMU_FEATURE_FAN_CONTROL_BIT))
+   return AMD_FAN_CTRL_NONE;


If there is no PMFW fan control, isn't it better to set pm.no_fan?

Thanks,
Lijo


+   else if (!smu_cmn_feature_is_enabled(smu, SMU_FEATURE_FAN_CONTROL_BIT))
return AMD_FAN_CTRL_MANUAL;
else
return AMD_FAN_CTRL_AUTO;
@@ -1143,7 +1145,7 @@ smu_v13_0_auto_fan_control(struct smu_context *smu, bool 
auto_fan_control)
int ret = 0;
  
  	if (!smu_cmn_feature_is_supported(smu, SMU_FEATURE_FAN_CONTROL_BIT))

-   return 0;
+   return -EINVAL;
  
  	ret = smu_cmn_feature_set_enabled(smu, SMU_FEATURE_FAN_CONTROL_BIT, auto_fan_control);

if (ret)
@@ -1204,7 +1206,8 @@ smu_v13_0_set_fan_control_mode(struct smu_context *smu,
  
  	switch (mode) {

case AMD_FAN_CTRL_NONE:
-   ret = smu_v13_0_set_fan_speed_pwm(smu, 255);
+   if (smu_cmn_feature_is_supported(smu, 
SMU_FEATURE_FAN_CONTROL_BIT))
+   ret = -EINVAL;
break;
case AMD_FAN_CTRL_MANUAL:
ret = smu_v13_0_auto_fan_control(smu, 0);


[PATCH] drm/amd/pm: disallow the fan setting if there is no fan on smu13

2023-08-08 Thread Kenneth Feng
disallow the fan setting if there is no fan on smu13

Signed-off-by: Kenneth Feng 
---
 drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
index 9b62b45ebb7f..09ef0a7e7679 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
@@ -1131,7 +1131,9 @@ smu_v13_0_display_clock_voltage_request(struct 
smu_context *smu,
 
 uint32_t smu_v13_0_get_fan_control_mode(struct smu_context *smu)
 {
-   if (!smu_cmn_feature_is_enabled(smu, SMU_FEATURE_FAN_CONTROL_BIT))
+   if (!smu_cmn_feature_is_supported(smu, SMU_FEATURE_FAN_CONTROL_BIT))
+   return AMD_FAN_CTRL_NONE;
+   else if (!smu_cmn_feature_is_enabled(smu, SMU_FEATURE_FAN_CONTROL_BIT))
return AMD_FAN_CTRL_MANUAL;
else
return AMD_FAN_CTRL_AUTO;
@@ -1143,7 +1145,7 @@ smu_v13_0_auto_fan_control(struct smu_context *smu, bool 
auto_fan_control)
int ret = 0;
 
if (!smu_cmn_feature_is_supported(smu, SMU_FEATURE_FAN_CONTROL_BIT))
-   return 0;
+   return -EINVAL;
 
ret = smu_cmn_feature_set_enabled(smu, SMU_FEATURE_FAN_CONTROL_BIT, 
auto_fan_control);
if (ret)
@@ -1204,7 +1206,8 @@ smu_v13_0_set_fan_control_mode(struct smu_context *smu,
 
switch (mode) {
case AMD_FAN_CTRL_NONE:
-   ret = smu_v13_0_set_fan_speed_pwm(smu, 255);
+   if (smu_cmn_feature_is_supported(smu, 
SMU_FEATURE_FAN_CONTROL_BIT))
+   ret = -EINVAL;
break;
case AMD_FAN_CTRL_MANUAL:
ret = smu_v13_0_auto_fan_control(smu, 0);
-- 
2.34.1