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

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

[Kenneth Feng]

drm/amd/pm: disallow the fan setting if there is no fan on smu 13.0.0
V2: depend on pm.no_fan to check

Signed-off-by: Kenneth Feng <kenneth.feng@amd.com>
---
 drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
index fddcd834bcec..0fb6be11a0cc 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
@@ -331,6 +331,7 @@ static int smu_v13_0_0_check_powerplay_table(struct smu_context *smu)
 	struct smu_13_0_0_powerplay_table *powerplay_table =
 		table_context->power_play_table;
 	struct smu_baco_context *smu_baco = &smu->smu_baco;
+	PPTable_t *pptable = smu->smu_table.driver_pptable;
 #if 0
 	PPTable_t *pptable = smu->smu_table.driver_pptable;
 	const OverDriveLimits_t * const overdrive_upperlimits =
@@ -371,6 +372,9 @@ static int smu_v13_0_0_check_powerplay_table(struct smu_context *smu)
 	table_context->thermal_controller_type =
 		powerplay_table->thermal_controller_type;
 
+	smu->adev->pm.no_fan =
+		!(pptable->SkuTable.FeaturesToRun[0] & (1 << FEATURE_FAN_CONTROL_BIT));
+
 	return 0;
 }
 
-- 
2.34.1

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

[Alex Deucher]

On Wed, Aug 9, 2023 at 6:12 AM Kenneth Feng <kenneth.feng@amd.com> wrote:
>
> drm/amd/pm: disallow the fan setting if there is no fan on smu 13.0.0
> V2: depend on pm.no_fan to check
>
> Signed-off-by: Kenneth Feng <kenneth.feng@amd.com>

You can still read the fan speed though right?  Don't we want to just
not expose the ability to manually set the fan speed?

Alex

> ---
>  drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> index fddcd834bcec..0fb6be11a0cc 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> @@ -331,6 +331,7 @@ static int smu_v13_0_0_check_powerplay_table(struct smu_context *smu)
>         struct smu_13_0_0_powerplay_table *powerplay_table =
>                 table_context->power_play_table;
>         struct smu_baco_context *smu_baco = &smu->smu_baco;
> +       PPTable_t *pptable = smu->smu_table.driver_pptable;
>  #if 0
>         PPTable_t *pptable = smu->smu_table.driver_pptable;
>         const OverDriveLimits_t * const overdrive_upperlimits =
> @@ -371,6 +372,9 @@ static int smu_v13_0_0_check_powerplay_table(struct smu_context *smu)
>         table_context->thermal_controller_type =
>                 powerplay_table->thermal_controller_type;
>
> +       smu->adev->pm.no_fan =
> +               !(pptable->SkuTable.FeaturesToRun[0] & (1 << FEATURE_FAN_CONTROL_BIT));
> +
>         return 0;
>  }
>
> --
> 2.34.1
>

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

[Lazar, Lijo]

On 8/9/2023 5:50 PM, Alex Deucher wrote:
> On Wed, Aug 9, 2023 at 6:12 AM Kenneth Feng <kenneth.feng@amd.com> wrote:
>>
>> drm/amd/pm: disallow the fan setting if there is no fan on smu 13.0.0
>> V2: depend on pm.no_fan to check
>>
>> Signed-off-by: Kenneth Feng <kenneth.feng@amd.com>
> 
> You can still read the fan speed though right?  Don't we want to just
> not expose the ability to manually set the fan speed?

If PMFW is not controlling fan, there is no guarantee that the cooling 
solution (any other external one) is using GPU's fan controller itself. 
In that way, reading the speed from controller doesn't make sense.

Thanks,
Lijo

> 
> Alex
> 
>> ---
>>   drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
>> index fddcd834bcec..0fb6be11a0cc 100644
>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
>> @@ -331,6 +331,7 @@ static int smu_v13_0_0_check_powerplay_table(struct smu_context *smu)
>>          struct smu_13_0_0_powerplay_table *powerplay_table =
>>                  table_context->power_play_table;
>>          struct smu_baco_context *smu_baco = &smu->smu_baco;
>> +       PPTable_t *pptable = smu->smu_table.driver_pptable;
>>   #if 0
>>          PPTable_t *pptable = smu->smu_table.driver_pptable;
>>          const OverDriveLimits_t * const overdrive_upperlimits =
>> @@ -371,6 +372,9 @@ static int smu_v13_0_0_check_powerplay_table(struct smu_context *smu)
>>          table_context->thermal_controller_type =
>>                  powerplay_table->thermal_controller_type;
>>
>> +       smu->adev->pm.no_fan =
>> +               !(pptable->SkuTable.FeaturesToRun[0] & (1 << FEATURE_FAN_CONTROL_BIT));
>> +
>>          return 0;
>>   }
>>
>> --
>> 2.34.1
>>

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

[Alex Deucher]

On Wed, Aug 9, 2023 at 8:28 AM Lazar, Lijo <lijo.lazar@amd.com> wrote:
>
>
>
> On 8/9/2023 5:50 PM, Alex Deucher wrote:
> > On Wed, Aug 9, 2023 at 6:12 AM Kenneth Feng <kenneth.feng@amd.com> wrote:
> >>
> >> drm/amd/pm: disallow the fan setting if there is no fan on smu 13.0.0
> >> V2: depend on pm.no_fan to check
> >>
> >> Signed-off-by: Kenneth Feng <kenneth.feng@amd.com>
> >
> > You can still read the fan speed though right?  Don't we want to just
> > not expose the ability to manually set the fan speed?
>
> If PMFW is not controlling fan, there is no guarantee that the cooling
> solution (any other external one) is using GPU's fan controller itself.
> In that way, reading the speed from controller doesn't make sense.
>

Ah, ok, I misunderstood.  My brain isn't working yet.  This is for the
case of the PMFW not controlling the fan.

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

> Thanks,
> Lijo
>
> >
> > Alex
> >
> >> ---
> >>   drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c | 4 ++++
> >>   1 file changed, 4 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> >> index fddcd834bcec..0fb6be11a0cc 100644
> >> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> >> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> >> @@ -331,6 +331,7 @@ static int smu_v13_0_0_check_powerplay_table(struct smu_context *smu)
> >>          struct smu_13_0_0_powerplay_table *powerplay_table =
> >>                  table_context->power_play_table;
> >>          struct smu_baco_context *smu_baco = &smu->smu_baco;
> >> +       PPTable_t *pptable = smu->smu_table.driver_pptable;
> >>   #if 0
> >>          PPTable_t *pptable = smu->smu_table.driver_pptable;
> >>          const OverDriveLimits_t * const overdrive_upperlimits =
> >> @@ -371,6 +372,9 @@ static int smu_v13_0_0_check_powerplay_table(struct smu_context *smu)
> >>          table_context->thermal_controller_type =
> >>                  powerplay_table->thermal_controller_type;
> >>
> >> +       smu->adev->pm.no_fan =
> >> +               !(pptable->SkuTable.FeaturesToRun[0] & (1 << FEATURE_FAN_CONTROL_BIT));
> >> +
> >>          return 0;
> >>   }
> >>
> >> --
> >> 2.34.1
> >>

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

[Lazar, Lijo]

On 8/9/2023 6:07 PM, Alex Deucher wrote:
> On Wed, Aug 9, 2023 at 8:28 AM Lazar, Lijo <lijo.lazar@amd.com> wrote:
>>
>>
>>
>> On 8/9/2023 5:50 PM, Alex Deucher wrote:
>>> On Wed, Aug 9, 2023 at 6:12 AM Kenneth Feng <kenneth.feng@amd.com> wrote:
>>>>
>>>> drm/amd/pm: disallow the fan setting if there is no fan on smu 13.0.0
>>>> V2: depend on pm.no_fan to check
>>>>
>>>> Signed-off-by: Kenneth Feng <kenneth.feng@amd.com>
>>>
>>> You can still read the fan speed though right?  Don't we want to just
>>> not expose the ability to manually set the fan speed?
>>
>> If PMFW is not controlling fan, there is no guarantee that the cooling
>> solution (any other external one) is using GPU's fan controller itself.
>> In that way, reading the speed from controller doesn't make sense.
>>
> 
> Ah, ok, I misunderstood.  My brain isn't working yet.  This is for the
> case of the PMFW not controlling the fan.

Yes, not the overdrive related control :)

	Reviewed-by: Lijo Lazar <lijo.lazar@amd.com>

Thanks,
Lijo

> 
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> 
>> Thanks,
>> Lijo
>>
>>>
>>> Alex
>>>
>>>> ---
>>>>    drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c | 4 ++++
>>>>    1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
>>>> index fddcd834bcec..0fb6be11a0cc 100644
>>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
>>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
>>>> @@ -331,6 +331,7 @@ static int smu_v13_0_0_check_powerplay_table(struct smu_context *smu)
>>>>           struct smu_13_0_0_powerplay_table *powerplay_table =
>>>>                   table_context->power_play_table;
>>>>           struct smu_baco_context *smu_baco = &smu->smu_baco;
>>>> +       PPTable_t *pptable = smu->smu_table.driver_pptable;
>>>>    #if 0
>>>>           PPTable_t *pptable = smu->smu_table.driver_pptable;
>>>>           const OverDriveLimits_t * const overdrive_upperlimits =
>>>> @@ -371,6 +372,9 @@ static int smu_v13_0_0_check_powerplay_table(struct smu_context *smu)
>>>>           table_context->thermal_controller_type =
>>>>                   powerplay_table->thermal_controller_type;
>>>>
>>>> +       smu->adev->pm.no_fan =
>>>> +               !(pptable->SkuTable.FeaturesToRun[0] & (1 << FEATURE_FAN_CONTROL_BIT));
>>>> +
>>>>           return 0;
>>>>    }
>>>>
>>>> --
>>>> 2.34.1
>>>>