[PATCH] drm/amd/pm: Add reset option for fan_curve on smu13_0_0

[Ma, Jun]

Hi Alex,

On 9/18/2023 10:05 PM, Alex Deucher wrote:
> On Mon, Sep 11, 2023 at 2:00 AM Ma Jun <Jun.Ma2 at amd.com> wrote:
>>
>> Add reset option for fan_curve.
>> User can use command "echo r > fan_cure" to reset the fan_curve
>> to boot value
>>
>> Signed-off-by: Ma Jun <Jun.Ma2 at amd.com>
>> ---
>>  drivers/gpu/drm/amd/pm/amdgpu_pm.c            |  8 ++++
>>  .../drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c  | 43 +++++++++++++++++--
>>  2 files changed, 47 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
>> index d05d9cd61331..2acac21387bc 100644
>> --- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
>> +++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
>> @@ -3437,6 +3437,11 @@ static int parse_input_od_command_lines(const char *buf,
>>         case 'c':
>>                 *type = PP_OD_COMMIT_DPM_TABLE;
>>                 return 0;
>> +       case 'r':
>> +               params[parameter_size] = *type;
>> +               *num_of_params = 1;
>> +               *type = PP_OD_RESTORE_DEFAULT_TABLE;
>> +               return 0;
>>         default:
>>                 break;
>>         }
>> @@ -3531,6 +3536,9 @@ amdgpu_distribute_custom_od_settings(struct amdgpu_device *adev,
>>   * When you have finished the editing, write "c" (commit) to the file to commit
>>   * your changes.
>>   *
>> + * If you want to reset to the default value, write "r" (reset) to the file to
>> + * reset them
>> + *
>>   * There are two fan control modes supported: auto and manual. With auto mode,
>>   * PMFW handles the fan speed control(how fan speed reacts to ASIC temperature).
>>   * While with manual mode, users can set their own fan curve line as what
>> 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 a719bae54e2c..644773c4bccb 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
>> @@ -1484,6 +1484,36 @@ static int smu_v13_0_0_print_clk_levels(struct smu_context *smu,
>>         return size;
>>  }
>>
>> +
>> +static int smu_v13_0_0_od_restore_table_single(struct smu_context *smu, long input)
>> +{
>> +       struct smu_table_context *table_context = &smu->smu_table;
>> +       OverDriveTableExternal_t *boot_overdrive_table =
>> +               (OverDriveTableExternal_t *)table_context->boot_overdrive_table;
>> +       OverDriveTableExternal_t *od_table =
>> +               (OverDriveTableExternal_t *)table_context->overdrive_table;
>> +       struct amdgpu_device *adev = smu->adev;
>> +       int i;
>> +
>> +       switch (input) {
>> +       case PP_OD_EDIT_FAN_CURVE:
>> +               for (i = 0; i < NUM_OD_FAN_MAX_POINTS; i++) {
>> +                       od_table->OverDriveTable.FanLinearTempPoints[i] =
>> +                                       boot_overdrive_table->OverDriveTable.FanLinearTempPoints[i];
>> +                       od_table->OverDriveTable.FanLinearPwmPoints[i] =
>> +                                       boot_overdrive_table->OverDriveTable.FanLinearPwmPoints[i];
>> +               }
>> +               od_table->OverDriveTable.FanMode = FAN_MODE_AUTO;
>> +               od_table->OverDriveTable.FeatureCtrlMask |= BIT(PP_OD_FEATURE_FAN_CURVE_BIT);
>> +               break;
>> +       default:
>> +               dev_info(adev->dev, "Invalid table index: %ld\n", input);
>> +               return -EINVAL;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>>  static int smu_v13_0_0_od_edit_dpm_table(struct smu_context *smu,
>>                                          enum PP_OD_DPM_TABLE_COMMAND type,
>>                                          long input[],
>> @@ -1770,13 +1800,18 @@ static int smu_v13_0_0_od_edit_dpm_table(struct smu_context *smu,
>>                 break;
>>
>>         case PP_OD_RESTORE_DEFAULT_TABLE:
>> -               feature_ctrlmask = od_table->OverDriveTable.FeatureCtrlMask;
>> -               memcpy(od_table,
>> +               if (size == 1) {
>> +                       ret = smu_v13_0_0_od_restore_table_single(smu, input[0]);
>> +                       if (ret)
>> +                               return ret;
>> +               } else {
>> +                       feature_ctrlmask = od_table->OverDriveTable.FeatureCtrlMask;
>> +                       memcpy(od_table,
>>                        table_context->boot_overdrive_table,
>>                        sizeof(OverDriveTableExternal_t));
>> -               od_table->OverDriveTable.FeatureCtrlMask = feature_ctrlmask;
>> +                       od_table->OverDriveTable.FeatureCtrlMask = feature_ctrlmask;
>> +               }
>>                 fallthrough;
> 
> I'm not sure the fallthrough here is correct.  I think setting the
> reset should just reset the values, but you'd still need to commit to
> actually set them to the firmware.  Double check that the behavior is
> consistent with older chips.
> 

We also use the "fallthrough" in sienna_cichlid_od_edit_dpm_table() and
smu_v13_0_7_od_edit_dpm_table().

But other chips used "break" here for reset. I think user maybe need
to use command " echo r > xx" and "echo c > xx" to reset.

So, It seems that it makes more sense to use  "fallthrough" here.

Regards,
Ma Jun

> Alex
> 
> 
>> -
>>         case PP_OD_COMMIT_DPM_TABLE:
>>                 /*
>>                  * The member below instructs PMFW the settings focused in
>> --
>> 2.34.1
>>