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

[Alex Deucher]

On Tue, Sep 19, 2023 at 11:00 PM Ma, Jun <majun at amd.com> wrote:
>
> 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.
>

I guess if that's what we do for seinna_cichlid, it's fine.  Too bad
we lost consistency here.
Acked-by: Alex Deucher <alexander.deucher at amd.com>

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