RE: [PATCH v2] drm/amd/pm: Fix sienna cichlid incorrect OD volage after resume

["Quan, Evan" <Evan.Quan@amd.com>]

[AMD Official Use Only - General]

Thanks Alex. I probably get the root cause of the issue. It should be related with the BTC feature.
- On cold boot, BTC was triggered and GfxVfCurve was calibrated.
   We got VfCurve settings (a).
- On resuming back, BTC will be triggered again and GfxVfCurve will be recalibrated.
   VfCurve settings (b) got may be different from those of cold boot. 
   So if to reuse those VfCurve settings (a) got on cold boot, we might got some V/f issues.

These can be confirmed by comparing the CustomGfxVfCurve settings got on cold boot and resuming.
+       dev_err(smu->adev->dev, "OD: GfxVfCurve: (%d, %d, %d)\n",
+                                                       od_table->CustomGfxVfCurve.a,
+                                                       od_table->CustomGfxVfCurve.b,
+                                                       od_table->CustomGfxVfCurve.c);
Below are some data collected on my nv21 platform and we can see the GfxVfCurve settings are different.
- On cold boot: OD: GfxVfCurve: (1046469987, -1089068751, 1066221898)
- On resuming back: OD: GfxVfCurve: (1046393849, -1089130480, 1066199153)

Hi @Błażej Szczygieł,
If you can add some descriptions about the BTC and GfxVfCurve related in the patch header/description part, it will be better.
Anyway, the patch is Reviewed-by: Evan Quan <evan.quan@amd.com>

BR
Evan
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex
> Deucher
> Sent: Tuesday, March 7, 2023 11:23 PM
> To: Quan, Evan <Evan.Quan@amd.com>
> Cc: Błażej Szczygieł <mumei6102@gmail.com>; amd-
> gfx@lists.freedesktop.org
> Subject: Re: [PATCH v2] drm/amd/pm: Fix sienna cichlid incorrect OD volage
> after resume
> 
> On Tue, Mar 7, 2023 at 3:23 AM Quan, Evan <Evan.Quan@amd.com> wrote:
> >
> > [AMD Official Use Only - General]
> >
> > Can you share more background about this? I cannot see how this can
> address incorrect OD voltage issue.
> 
> See https://gitlab.freedesktop.org/drm/amd/-/issues/1897
> The OD settings don't seem to be restored properly on resume.
> 
> Alex
> 
> >
> > BR
> > Evan
> > > -----Original Message-----
> > > From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
> > > Blazej Szczygiel
> > > Sent: Sunday, March 5, 2023 7:45 AM
> > > To: amd-gfx@lists.freedesktop.org
> > > Cc: Błażej Szczygieł <mumei6102@gmail.com>
> > > Subject: [PATCH v2] drm/amd/pm: Fix sienna cichlid incorrect OD
> > > volage after resume
> > >
> > > Always setup overdrive tables after resume. Preserve only some
> > > user-defined settings in user_overdrive_table if they're set.
> > >
> > > Copy restored user_overdrive_table into od_table to get correct
> > > values.
> > >
> > > Signed-off-by: Błażej Szczygieł <mumei6102@gmail.com>
> > > ---
> > >  .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c   | 43
> ++++++++++++++----
> > > -
> > >  1 file changed, 33 insertions(+), 10 deletions(-)
> > >
> > > diff --git
> a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> > > b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> > > index 697e98a0a20a..75f18681e984 100644
> > > --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> > > +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> > > @@ -2143,16 +2143,9 @@ static int
> > > sienna_cichlid_set_default_od_settings(struct smu_context *smu)
> > >               (OverDriveTable_t *)smu->smu_table.boot_overdrive_table;
> > >       OverDriveTable_t *user_od_table =
> > >               (OverDriveTable_t
> > > *)smu->smu_table.user_overdrive_table;
> > > +     OverDriveTable_t user_od_table_bak;
> > >       int ret = 0;
> > >
> > > -     /*
> > > -      * For S3/S4/Runpm resume, no need to setup those overdrive
> > > tables again as
> > > -      *   - either they already have the default OD settings got during cold
> > > bootup
> > > -      *   - or they have some user customized OD settings which cannot be
> > > overwritten
> > > -      */
> > > -     if (smu->adev->in_suspend)
> > > -             return 0;
> > > -
> > >       ret = smu_cmn_update_table(smu, SMU_TABLE_OVERDRIVE,
> > >                                  0, (void *)boot_od_table, false);
> > >       if (ret) {
> > > @@ -2163,7 +2156,23 @@ static int
> > > sienna_cichlid_set_default_od_settings(struct smu_context *smu)
> > >       sienna_cichlid_dump_od_table(smu, boot_od_table);
> > >
> > >       memcpy(od_table, boot_od_table, sizeof(OverDriveTable_t));
> > > -     memcpy(user_od_table, boot_od_table, sizeof(OverDriveTable_t));
> > > +
> > > +     /*
> > > +      * For S3/S4/Runpm resume, we need to setup those overdrive
> > > tables again,
> > > +      * but we have to preserve user defined values in "user_od_table".
> > > +      */
> > > +     if (!smu->adev->in_suspend) {
> > > +             memcpy(user_od_table, boot_od_table,
> > > sizeof(OverDriveTable_t));
> > > +             smu->user_dpm_profile.user_od = false;
> > > +     } else if (smu->user_dpm_profile.user_od) {
> > > +             memcpy(&user_od_table_bak, user_od_table,
> > > sizeof(OverDriveTable_t));
> > > +             memcpy(user_od_table, boot_od_table,
> > > sizeof(OverDriveTable_t));
> > > +             user_od_table->GfxclkFmin =
> > > user_od_table_bak.GfxclkFmin;
> > > +             user_od_table->GfxclkFmax =
> > > user_od_table_bak.GfxclkFmax;
> > > +             user_od_table->UclkFmin = user_od_table_bak.UclkFmin;
> > > +             user_od_table->UclkFmax = user_od_table_bak.UclkFmax;
> > > +             user_od_table->VddGfxOffset =
> > > user_od_table_bak.VddGfxOffset;
> > > +     }
> > >
> > >       return 0;
> > >  }
> > > @@ -2373,6 +2382,20 @@ static int
> > > sienna_cichlid_od_edit_dpm_table(struct smu_context *smu,
> > >       return ret;
> > >  }
> > >
> > > +static int sienna_cichlid_restore_user_od_settings(struct
> > > +smu_context
> > > *smu)
> > > +{
> > > +     struct smu_table_context *table_context = &smu->smu_table;
> > > +     OverDriveTable_t *od_table = table_context->overdrive_table;
> > > +     OverDriveTable_t *user_od_table = table_context-
> > > >user_overdrive_table;
> > > +     int res;
> > > +
> > > +     res = smu_v11_0_restore_user_od_settings(smu);
> > > +     if (res == 0)
> > > +             memcpy(od_table, user_od_table,
> > > sizeof(OverDriveTable_t));
> > > +
> > > +     return res;
> > > +}
> > > +
> > >  static int sienna_cichlid_run_btc(struct smu_context *smu)  {
> > >       int res;
> > > @@ -4400,7 +4423,7 @@ static const struct pptable_funcs
> > > sienna_cichlid_ppt_funcs = {
> > >       .set_soft_freq_limited_range =
> > > smu_v11_0_set_soft_freq_limited_range,
> > >       .set_default_od_settings = sienna_cichlid_set_default_od_settings,
> > >       .od_edit_dpm_table = sienna_cichlid_od_edit_dpm_table,
> > > -     .restore_user_od_settings = smu_v11_0_restore_user_od_settings,
> > > +     .restore_user_od_settings =
> > > sienna_cichlid_restore_user_od_settings,
> > >       .run_btc = sienna_cichlid_run_btc,
> > >       .set_power_source = smu_v11_0_set_power_source,
> > >       .get_pp_feature_mask = smu_cmn_get_pp_feature_mask,
> > > --
> > > 2.39.2