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

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

[Błażej Szczygieł]

Always setup overdrive tables after resume. Preserve only some
user-defined settings in user_overdrive_table if they're set.

Signed-off-by: Błażej Szczygieł <mumei6102@gmail.com>
---
 .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c   | 27 ++++++++++++-------
 1 file changed, 18 insertions(+), 9 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..e8780ea66067 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;
 }
-- 
2.39.2

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

[Błażej Szczygieł]

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

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

[Limonciello, Mario]

On 3/4/2023 17:44, Błażej Szczygieł wrote:
> 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,

Rather than introduce a new static function perhaps it would be better 
to just change 'smu_v11_0_restore_user_od_settings'.

That could help this issue if it also occurs on Navi10 as well.

>   	.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,

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

[Alex Deucher]

+ Evan, Kenneth

On Mon, Mar 6, 2023 at 2:49 PM Limonciello, Mario
<mario.limonciello@amd.com> wrote:
>
> On 3/4/2023 17:44, Błażej Szczygieł wrote:
> > 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>

Please add a link to the bug tracker as well:
Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1897

Alex



> > ---
> >   .../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,
>
> Rather than introduce a new static function perhaps it would be better
> to just change 'smu_v11_0_restore_user_od_settings'.
>
> That could help this issue if it also occurs on Navi10 as well.
>
> >       .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,
>

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

[Quan, Evan]

[AMD Official Use Only - General]

Can you share more background about this? I cannot see how this can address incorrect OD voltage issue.

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

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

[Alex Deucher]

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

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

[Quan, Evan]

[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

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

[Alex Deucher]

On Tue, Mar 7, 2023 at 9:34 PM Quan, Evan <Evan.Quan@amd.com> wrote:
>
> [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>

Presumably this should be done for navi10 as well?

Alex

>
> 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