All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2 1/2] drm/amd/pm: restore user customized OD settings properly for NV1x
@ 2021-07-23  9:09 Evan Quan
  2021-07-23 10:09 ` Lazar, Lijo
  0 siblings, 1 reply; 7+ messages in thread
From: Evan Quan @ 2021-07-23  9:09 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alexander.Deucher, Lijo.Lazar, Evan Quan

The customized OD settings can be divided into two parts: those
committed ones and non-committed ones.
  - For those changes which had been fed to SMU before S3/S4/Runpm
    suspend kicked, they are committed changes. They should be properly
    restored and fed to SMU on S3/S4/Runpm resume.
  - For those non-committed changes, they are restored only without feeding
    to SMU.

Change-Id: Iea7cf7908dfcd919a4d0205e10bff91b1149a440
Signed-off-by: Evan Quan <evan.quan@amd.com>
--
v1->v2
  - better naming and logic revised for checking OD setting update(Lijo)
---
 drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h       |  8 +++
 drivers/gpu/drm/amd/pm/inc/smu_v11_0.h        |  2 +
 drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c     |  9 +++
 .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c   | 55 +++++++++++++------
 .../gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c    | 25 +++++++++
 5 files changed, 82 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
index 3e89852e4820..c2c201b8e3cf 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
@@ -231,6 +231,7 @@ struct smu_user_dpm_profile {
 	uint32_t power_limit;
 	uint32_t fan_speed_percent;
 	uint32_t flags;
+	uint32_t user_od;
 
 	/* user clock state information */
 	uint32_t clk_mask[SMU_CLK_COUNT];
@@ -352,6 +353,7 @@ struct smu_table_context
 
 	void				*overdrive_table;
 	void                            *boot_overdrive_table;
+	void				*user_overdrive_table;
 
 	uint32_t			gpu_metrics_table_size;
 	void				*gpu_metrics_table;
@@ -623,6 +625,12 @@ struct pptable_funcs {
 				 enum PP_OD_DPM_TABLE_COMMAND type,
 				 long *input, uint32_t size);
 
+	/**
+	 * @restore_user_od_settings: Restore the user customized
+	 *                            OD settings on S3/S4/Runpm resume.
+	 */
+	int (*restore_user_od_settings)(struct smu_context *smu);
+
 	/**
 	 * @get_clock_by_type_with_latency: Get the speed and latency of a clock
 	 *                                  domain.
diff --git a/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h b/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h
index 385b2ea5379c..1e42aafbb9fd 100644
--- a/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h
+++ b/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h
@@ -302,5 +302,7 @@ void smu_v11_0_interrupt_work(struct smu_context *smu);
 
 int smu_v11_0_set_light_sbr(struct smu_context *smu, bool enable);
 
+int smu_v11_0_restore_user_od_settings(struct smu_context *smu);
+
 #endif
 #endif
diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index ebe672142808..8ca7337ea5fc 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -416,6 +416,15 @@ static void smu_restore_dpm_user_profile(struct smu_context *smu)
 		}
 	}
 
+	/* Restore user customized OD settings */
+	if (smu->user_dpm_profile.user_od) {
+		if (smu->ppt_funcs->restore_user_od_settings) {
+			ret = smu->ppt_funcs->restore_user_od_settings(smu);
+			if (ret)
+				dev_err(smu->adev->dev, "Failed to upload customized OD settings\n");
+		}
+	}
+
 	/* Disable restore flag */
 	smu->user_dpm_profile.flags &= ~SMU_DPM_USER_PROFILE_RESTORE;
 }
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
index 59ea59acfb00..d7722c229ddd 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
@@ -2294,41 +2294,52 @@ static int navi10_set_default_od_settings(struct smu_context *smu)
 		(OverDriveTable_t *)smu->smu_table.overdrive_table;
 	OverDriveTable_t *boot_od_table =
 		(OverDriveTable_t *)smu->smu_table.boot_overdrive_table;
+	OverDriveTable_t *user_od_table =
+		(OverDriveTable_t *)smu->smu_table.user_overdrive_table;
 	int ret = 0;
 
-	ret = smu_cmn_update_table(smu, SMU_TABLE_OVERDRIVE, 0, (void *)od_table, false);
+	/*
+	 * 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) {
 		dev_err(smu->adev->dev, "Failed to get overdrive table!\n");
 		return ret;
 	}
 
-	if (!od_table->GfxclkVolt1) {
+	if (!boot_od_table->GfxclkVolt1) {
 		ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
-								&od_table->GfxclkVolt1,
-								od_table->GfxclkFreq1);
+								&boot_od_table->GfxclkVolt1,
+								boot_od_table->GfxclkFreq1);
 		if (ret)
 			return ret;
 	}
 
-	if (!od_table->GfxclkVolt2) {
+	if (!boot_od_table->GfxclkVolt2) {
 		ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
-								&od_table->GfxclkVolt2,
-								od_table->GfxclkFreq2);
+								&boot_od_table->GfxclkVolt2,
+								boot_od_table->GfxclkFreq2);
 		if (ret)
 			return ret;
 	}
 
-	if (!od_table->GfxclkVolt3) {
+	if (!boot_od_table->GfxclkVolt3) {
 		ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
-								&od_table->GfxclkVolt3,
-								od_table->GfxclkFreq3);
+								&boot_od_table->GfxclkVolt3,
+								boot_od_table->GfxclkFreq3);
 		if (ret)
 			return ret;
 	}
 
-	memcpy(boot_od_table, od_table, sizeof(OverDriveTable_t));
+	navi10_dump_od_table(smu, boot_od_table);
 
-	navi10_dump_od_table(smu, od_table);
+	memcpy(od_table, boot_od_table, sizeof(OverDriveTable_t));
+	memcpy(user_od_table, boot_od_table, sizeof(OverDriveTable_t));
 
 	return 0;
 }
@@ -2429,11 +2440,20 @@ static int navi10_od_edit_dpm_table(struct smu_context *smu, enum PP_OD_DPM_TABL
 		memcpy(table_context->overdrive_table, table_context->boot_overdrive_table, sizeof(OverDriveTable_t));
 		break;
 	case PP_OD_COMMIT_DPM_TABLE:
-		navi10_dump_od_table(smu, od_table);
-		ret = smu_cmn_update_table(smu, SMU_TABLE_OVERDRIVE, 0, (void *)od_table, true);
-		if (ret) {
-			dev_err(smu->adev->dev, "Failed to import overdrive table!\n");
-			return ret;
+		if (memcmp(od_table, table_context->user_overdrive_table, sizeof(OverDriveTable_t))) {
+			navi10_dump_od_table(smu, od_table);
+			ret = smu_cmn_update_table(smu, SMU_TABLE_OVERDRIVE, 0, (void *)od_table, true);
+			if (ret) {
+				dev_err(smu->adev->dev, "Failed to import overdrive table!\n");
+				return ret;
+			}
+			memcpy(table_context->user_overdrive_table, od_table, sizeof(OverDriveTable_t));
+			smu->user_dpm_profile.user_od = true;
+
+			if (!memcmp(table_context->user_overdrive_table,
+				    table_context->boot_overdrive_table,
+				    sizeof(OverDriveTable_t)))
+				smu->user_dpm_profile.user_od = false;
 		}
 		break;
 	case PP_OD_EDIT_VDDC_CURVE:
@@ -3262,6 +3282,7 @@ static const struct pptable_funcs navi10_ppt_funcs = {
 	.set_soft_freq_limited_range = smu_v11_0_set_soft_freq_limited_range,
 	.set_default_od_settings = navi10_set_default_od_settings,
 	.od_edit_dpm_table = navi10_od_edit_dpm_table,
+	.restore_user_od_settings = smu_v11_0_restore_user_od_settings,
 	.run_btc = navi10_run_btc,
 	.set_power_source = smu_v11_0_set_power_source,
 	.get_pp_feature_mask = smu_cmn_get_pp_feature_mask,
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
index 0a5d46ac9ccd..7aa47dbba8d8 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
@@ -422,10 +422,20 @@ int smu_v11_0_init_smc_tables(struct smu_context *smu)
 			ret = -ENOMEM;
 			goto err3_out;
 		}
+
+		smu_table->user_overdrive_table =
+			kzalloc(tables[SMU_TABLE_OVERDRIVE].size, GFP_KERNEL);
+		if (!smu_table->user_overdrive_table) {
+			ret = -ENOMEM;
+			goto err4_out;
+		}
+
 	}
 
 	return 0;
 
+err4_out:
+	kfree(smu_table->boot_overdrive_table);
 err3_out:
 	kfree(smu_table->overdrive_table);
 err2_out:
@@ -442,12 +452,14 @@ int smu_v11_0_fini_smc_tables(struct smu_context *smu)
 	struct smu_dpm_context *smu_dpm = &smu->smu_dpm;
 
 	kfree(smu_table->gpu_metrics_table);
+	kfree(smu_table->user_overdrive_table);
 	kfree(smu_table->boot_overdrive_table);
 	kfree(smu_table->overdrive_table);
 	kfree(smu_table->max_sustainable_clocks);
 	kfree(smu_table->driver_pptable);
 	kfree(smu_table->clocks_table);
 	smu_table->gpu_metrics_table = NULL;
+	smu_table->user_overdrive_table = NULL;
 	smu_table->boot_overdrive_table = NULL;
 	smu_table->overdrive_table = NULL;
 	smu_table->max_sustainable_clocks = NULL;
@@ -2101,3 +2113,16 @@ int smu_v11_0_deep_sleep_control(struct smu_context *smu,
 
 	return ret;
 }
+
+int smu_v11_0_restore_user_od_settings(struct smu_context *smu)
+{
+	struct smu_table_context *table_context = &smu->smu_table;
+	void *user_od_table = table_context->user_overdrive_table;
+	int ret = 0;
+
+	ret = smu_cmn_update_table(smu, SMU_TABLE_OVERDRIVE, 0, (void *)user_od_table, true);
+	if (ret)
+		dev_err(smu->adev->dev, "Failed to import overdrive table!\n");
+
+	return ret;
+}
-- 
2.29.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH V2 1/2] drm/amd/pm: restore user customized OD settings properly for NV1x
  2021-07-23  9:09 [PATCH V2 1/2] drm/amd/pm: restore user customized OD settings properly for NV1x Evan Quan
@ 2021-07-23 10:09 ` Lazar, Lijo
  2021-07-23 13:35   ` Deucher, Alexander
  2021-07-23 13:44   ` Chen, Guchun
  0 siblings, 2 replies; 7+ messages in thread
From: Lazar, Lijo @ 2021-07-23 10:09 UTC (permalink / raw)
  To: Evan Quan, amd-gfx; +Cc: Alexander.Deucher

The series looks good to me, though I prefer to use a common logic to 
restore od settings so that smuv12,smuv13 gets the restore feature by 
default once they add the user table logic. Don't have strong argument 
for it unless Alex, Kenneth or others have some comments.

Anyway, the series is
	Reviewed-by: Lijo Lazar <lijo.lazar@amd.com>

On 7/23/2021 2:39 PM, Evan Quan wrote:
> The customized OD settings can be divided into two parts: those
> committed ones and non-committed ones.
>    - For those changes which had been fed to SMU before S3/S4/Runpm
>      suspend kicked, they are committed changes. They should be properly
>      restored and fed to SMU on S3/S4/Runpm resume.
>    - For those non-committed changes, they are restored only without feeding
>      to SMU.
> 
> Change-Id: Iea7cf7908dfcd919a4d0205e10bff91b1149a440
> Signed-off-by: Evan Quan <evan.quan@amd.com>
> --
> v1->v2
>    - better naming and logic revised for checking OD setting update(Lijo)
> ---
>   drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h       |  8 +++
>   drivers/gpu/drm/amd/pm/inc/smu_v11_0.h        |  2 +
>   drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c     |  9 +++
>   .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c   | 55 +++++++++++++------
>   .../gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c    | 25 +++++++++
>   5 files changed, 82 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> index 3e89852e4820..c2c201b8e3cf 100644
> --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> @@ -231,6 +231,7 @@ struct smu_user_dpm_profile {
>   	uint32_t power_limit;
>   	uint32_t fan_speed_percent;
>   	uint32_t flags;
> +	uint32_t user_od;
>   
>   	/* user clock state information */
>   	uint32_t clk_mask[SMU_CLK_COUNT];
> @@ -352,6 +353,7 @@ struct smu_table_context
>   
>   	void				*overdrive_table;
>   	void                            *boot_overdrive_table;
> +	void				*user_overdrive_table;
>   
>   	uint32_t			gpu_metrics_table_size;
>   	void				*gpu_metrics_table;
> @@ -623,6 +625,12 @@ struct pptable_funcs {
>   				 enum PP_OD_DPM_TABLE_COMMAND type,
>   				 long *input, uint32_t size);
>   
> +	/**
> +	 * @restore_user_od_settings: Restore the user customized
> +	 *                            OD settings on S3/S4/Runpm resume.
> +	 */
> +	int (*restore_user_od_settings)(struct smu_context *smu);
> +
>   	/**
>   	 * @get_clock_by_type_with_latency: Get the speed and latency of a clock
>   	 *                                  domain.
> diff --git a/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h b/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h
> index 385b2ea5379c..1e42aafbb9fd 100644
> --- a/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h
> +++ b/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h
> @@ -302,5 +302,7 @@ void smu_v11_0_interrupt_work(struct smu_context *smu);
>   
>   int smu_v11_0_set_light_sbr(struct smu_context *smu, bool enable);
>   
> +int smu_v11_0_restore_user_od_settings(struct smu_context *smu);
> +
>   #endif
>   #endif
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> index ebe672142808..8ca7337ea5fc 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> @@ -416,6 +416,15 @@ static void smu_restore_dpm_user_profile(struct smu_context *smu)
>   		}
>   	}
>   
> +	/* Restore user customized OD settings */
> +	if (smu->user_dpm_profile.user_od) {
> +		if (smu->ppt_funcs->restore_user_od_settings) {
> +			ret = smu->ppt_funcs->restore_user_od_settings(smu);
> +			if (ret)
> +				dev_err(smu->adev->dev, "Failed to upload customized OD settings\n");
> +		}
> +	}
> +
>   	/* Disable restore flag */
>   	smu->user_dpm_profile.flags &= ~SMU_DPM_USER_PROFILE_RESTORE;
>   }
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> index 59ea59acfb00..d7722c229ddd 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> @@ -2294,41 +2294,52 @@ static int navi10_set_default_od_settings(struct smu_context *smu)
>   		(OverDriveTable_t *)smu->smu_table.overdrive_table;
>   	OverDriveTable_t *boot_od_table =
>   		(OverDriveTable_t *)smu->smu_table.boot_overdrive_table;
> +	OverDriveTable_t *user_od_table =
> +		(OverDriveTable_t *)smu->smu_table.user_overdrive_table;
>   	int ret = 0;
>   
> -	ret = smu_cmn_update_table(smu, SMU_TABLE_OVERDRIVE, 0, (void *)od_table, false);
> +	/*
> +	 * 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) {
>   		dev_err(smu->adev->dev, "Failed to get overdrive table!\n");
>   		return ret;
>   	}
>   
> -	if (!od_table->GfxclkVolt1) {
> +	if (!boot_od_table->GfxclkVolt1) {
>   		ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
> -								&od_table->GfxclkVolt1,
> -								od_table->GfxclkFreq1);
> +								&boot_od_table->GfxclkVolt1,
> +								boot_od_table->GfxclkFreq1);
>   		if (ret)
>   			return ret;
>   	}
>   
> -	if (!od_table->GfxclkVolt2) {
> +	if (!boot_od_table->GfxclkVolt2) {
>   		ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
> -								&od_table->GfxclkVolt2,
> -								od_table->GfxclkFreq2);
> +								&boot_od_table->GfxclkVolt2,
> +								boot_od_table->GfxclkFreq2);
>   		if (ret)
>   			return ret;
>   	}
>   
> -	if (!od_table->GfxclkVolt3) {
> +	if (!boot_od_table->GfxclkVolt3) {
>   		ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
> -								&od_table->GfxclkVolt3,
> -								od_table->GfxclkFreq3);
> +								&boot_od_table->GfxclkVolt3,
> +								boot_od_table->GfxclkFreq3);
>   		if (ret)
>   			return ret;
>   	}
>   
> -	memcpy(boot_od_table, od_table, sizeof(OverDriveTable_t));
> +	navi10_dump_od_table(smu, boot_od_table);
>   
> -	navi10_dump_od_table(smu, od_table);
> +	memcpy(od_table, boot_od_table, sizeof(OverDriveTable_t));
> +	memcpy(user_od_table, boot_od_table, sizeof(OverDriveTable_t));
>   
>   	return 0;
>   }
> @@ -2429,11 +2440,20 @@ static int navi10_od_edit_dpm_table(struct smu_context *smu, enum PP_OD_DPM_TABL
>   		memcpy(table_context->overdrive_table, table_context->boot_overdrive_table, sizeof(OverDriveTable_t));
>   		break;
>   	case PP_OD_COMMIT_DPM_TABLE:
> -		navi10_dump_od_table(smu, od_table);
> -		ret = smu_cmn_update_table(smu, SMU_TABLE_OVERDRIVE, 0, (void *)od_table, true);
> -		if (ret) {
> -			dev_err(smu->adev->dev, "Failed to import overdrive table!\n");
> -			return ret;
> +		if (memcmp(od_table, table_context->user_overdrive_table, sizeof(OverDriveTable_t))) {
> +			navi10_dump_od_table(smu, od_table);
> +			ret = smu_cmn_update_table(smu, SMU_TABLE_OVERDRIVE, 0, (void *)od_table, true);
> +			if (ret) {
> +				dev_err(smu->adev->dev, "Failed to import overdrive table!\n");
> +				return ret;
> +			}
> +			memcpy(table_context->user_overdrive_table, od_table, sizeof(OverDriveTable_t));
> +			smu->user_dpm_profile.user_od = true;
> +
> +			if (!memcmp(table_context->user_overdrive_table,
> +				    table_context->boot_overdrive_table,
> +				    sizeof(OverDriveTable_t)))
> +				smu->user_dpm_profile.user_od = false;
>   		}
>   		break;
>   	case PP_OD_EDIT_VDDC_CURVE:
> @@ -3262,6 +3282,7 @@ static const struct pptable_funcs navi10_ppt_funcs = {
>   	.set_soft_freq_limited_range = smu_v11_0_set_soft_freq_limited_range,
>   	.set_default_od_settings = navi10_set_default_od_settings,
>   	.od_edit_dpm_table = navi10_od_edit_dpm_table,
> +	.restore_user_od_settings = smu_v11_0_restore_user_od_settings,
>   	.run_btc = navi10_run_btc,
>   	.set_power_source = smu_v11_0_set_power_source,
>   	.get_pp_feature_mask = smu_cmn_get_pp_feature_mask,
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> index 0a5d46ac9ccd..7aa47dbba8d8 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> @@ -422,10 +422,20 @@ int smu_v11_0_init_smc_tables(struct smu_context *smu)
>   			ret = -ENOMEM;
>   			goto err3_out;
>   		}
> +
> +		smu_table->user_overdrive_table =
> +			kzalloc(tables[SMU_TABLE_OVERDRIVE].size, GFP_KERNEL);
> +		if (!smu_table->user_overdrive_table) {
> +			ret = -ENOMEM;
> +			goto err4_out;
> +		}
> +
>   	}
>   
>   	return 0;
>   
> +err4_out:
> +	kfree(smu_table->boot_overdrive_table);
>   err3_out:
>   	kfree(smu_table->overdrive_table);
>   err2_out:
> @@ -442,12 +452,14 @@ int smu_v11_0_fini_smc_tables(struct smu_context *smu)
>   	struct smu_dpm_context *smu_dpm = &smu->smu_dpm;
>   
>   	kfree(smu_table->gpu_metrics_table);
> +	kfree(smu_table->user_overdrive_table);
>   	kfree(smu_table->boot_overdrive_table);
>   	kfree(smu_table->overdrive_table);
>   	kfree(smu_table->max_sustainable_clocks);
>   	kfree(smu_table->driver_pptable);
>   	kfree(smu_table->clocks_table);
>   	smu_table->gpu_metrics_table = NULL;
> +	smu_table->user_overdrive_table = NULL;
>   	smu_table->boot_overdrive_table = NULL;
>   	smu_table->overdrive_table = NULL;
>   	smu_table->max_sustainable_clocks = NULL;
> @@ -2101,3 +2113,16 @@ int smu_v11_0_deep_sleep_control(struct smu_context *smu,
>   
>   	return ret;
>   }
> +
> +int smu_v11_0_restore_user_od_settings(struct smu_context *smu)
> +{
> +	struct smu_table_context *table_context = &smu->smu_table;
> +	void *user_od_table = table_context->user_overdrive_table;
> +	int ret = 0;
> +
> +	ret = smu_cmn_update_table(smu, SMU_TABLE_OVERDRIVE, 0, (void *)user_od_table, true);
> +	if (ret)
> +		dev_err(smu->adev->dev, "Failed to import overdrive table!\n");
> +
> +	return ret;
> +}
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V2 1/2] drm/amd/pm: restore user customized OD settings properly for NV1x
  2021-07-23 10:09 ` Lazar, Lijo
@ 2021-07-23 13:35   ` Deucher, Alexander
  2021-07-26  7:30     ` Quan, Evan
  2021-07-23 13:44   ` Chen, Guchun
  1 sibling, 1 reply; 7+ messages in thread
From: Deucher, Alexander @ 2021-07-23 13:35 UTC (permalink / raw)
  To: Lazar, Lijo, Quan, Evan, amd-gfx


[-- Attachment #1.1: Type: text/plain, Size: 12660 bytes --]

[AMD Official Use Only]

I haven't had a chance to look at the patches too closely, but if it could be done in a generic may, that makes sense to me.  Maybe as a follow up patch?

Alex

________________________________
From: Lazar, Lijo <Lijo.Lazar@amd.com>
Sent: Friday, July 23, 2021 6:09 AM
To: Quan, Evan <Evan.Quan@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
Subject: Re: [PATCH V2 1/2] drm/amd/pm: restore user customized OD settings properly for NV1x

The series looks good to me, though I prefer to use a common logic to
restore od settings so that smuv12,smuv13 gets the restore feature by
default once they add the user table logic. Don't have strong argument
for it unless Alex, Kenneth or others have some comments.

Anyway, the series is
        Reviewed-by: Lijo Lazar <lijo.lazar@amd.com>

On 7/23/2021 2:39 PM, Evan Quan wrote:
> The customized OD settings can be divided into two parts: those
> committed ones and non-committed ones.
>    - For those changes which had been fed to SMU before S3/S4/Runpm
>      suspend kicked, they are committed changes. They should be properly
>      restored and fed to SMU on S3/S4/Runpm resume.
>    - For those non-committed changes, they are restored only without feeding
>      to SMU.
>
> Change-Id: Iea7cf7908dfcd919a4d0205e10bff91b1149a440
> Signed-off-by: Evan Quan <evan.quan@amd.com>
> --
> v1->v2
>    - better naming and logic revised for checking OD setting update(Lijo)
> ---
>   drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h       |  8 +++
>   drivers/gpu/drm/amd/pm/inc/smu_v11_0.h        |  2 +
>   drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c     |  9 +++
>   .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c   | 55 +++++++++++++------
>   .../gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c    | 25 +++++++++
>   5 files changed, 82 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> index 3e89852e4820..c2c201b8e3cf 100644
> --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> @@ -231,6 +231,7 @@ struct smu_user_dpm_profile {
>        uint32_t power_limit;
>        uint32_t fan_speed_percent;
>        uint32_t flags;
> +     uint32_t user_od;
>
>        /* user clock state information */
>        uint32_t clk_mask[SMU_CLK_COUNT];
> @@ -352,6 +353,7 @@ struct smu_table_context
>
>        void                            *overdrive_table;
>        void                            *boot_overdrive_table;
> +     void                            *user_overdrive_table;
>
>        uint32_t                        gpu_metrics_table_size;
>        void                            *gpu_metrics_table;
> @@ -623,6 +625,12 @@ struct pptable_funcs {
>                                 enum PP_OD_DPM_TABLE_COMMAND type,
>                                 long *input, uint32_t size);
>
> +     /**
> +      * @restore_user_od_settings: Restore the user customized
> +      *                            OD settings on S3/S4/Runpm resume.
> +      */
> +     int (*restore_user_od_settings)(struct smu_context *smu);
> +
>        /**
>         * @get_clock_by_type_with_latency: Get the speed and latency of a clock
>         *                                  domain.
> diff --git a/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h b/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h
> index 385b2ea5379c..1e42aafbb9fd 100644
> --- a/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h
> +++ b/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h
> @@ -302,5 +302,7 @@ void smu_v11_0_interrupt_work(struct smu_context *smu);
>
>   int smu_v11_0_set_light_sbr(struct smu_context *smu, bool enable);
>
> +int smu_v11_0_restore_user_od_settings(struct smu_context *smu);
> +
>   #endif
>   #endif
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> index ebe672142808..8ca7337ea5fc 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> @@ -416,6 +416,15 @@ static void smu_restore_dpm_user_profile(struct smu_context *smu)
>                }
>        }
>
> +     /* Restore user customized OD settings */
> +     if (smu->user_dpm_profile.user_od) {
> +             if (smu->ppt_funcs->restore_user_od_settings) {
> +                     ret = smu->ppt_funcs->restore_user_od_settings(smu);
> +                     if (ret)
> +                             dev_err(smu->adev->dev, "Failed to upload customized OD settings\n");
> +             }
> +     }
> +
>        /* Disable restore flag */
>        smu->user_dpm_profile.flags &= ~SMU_DPM_USER_PROFILE_RESTORE;
>   }
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> index 59ea59acfb00..d7722c229ddd 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> @@ -2294,41 +2294,52 @@ static int navi10_set_default_od_settings(struct smu_context *smu)
>                (OverDriveTable_t *)smu->smu_table.overdrive_table;
>        OverDriveTable_t *boot_od_table =
>                (OverDriveTable_t *)smu->smu_table.boot_overdrive_table;
> +     OverDriveTable_t *user_od_table =
> +             (OverDriveTable_t *)smu->smu_table.user_overdrive_table;
>        int ret = 0;
>
> -     ret = smu_cmn_update_table(smu, SMU_TABLE_OVERDRIVE, 0, (void *)od_table, false);
> +     /*
> +      * 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) {
>                dev_err(smu->adev->dev, "Failed to get overdrive table!\n");
>                return ret;
>        }
>
> -     if (!od_table->GfxclkVolt1) {
> +     if (!boot_od_table->GfxclkVolt1) {
>                ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
> -                                                             &od_table->GfxclkVolt1,
> -                                                             od_table->GfxclkFreq1);
> +                                                             &boot_od_table->GfxclkVolt1,
> +                                                             boot_od_table->GfxclkFreq1);
>                if (ret)
>                        return ret;
>        }
>
> -     if (!od_table->GfxclkVolt2) {
> +     if (!boot_od_table->GfxclkVolt2) {
>                ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
> -                                                             &od_table->GfxclkVolt2,
> -                                                             od_table->GfxclkFreq2);
> +                                                             &boot_od_table->GfxclkVolt2,
> +                                                             boot_od_table->GfxclkFreq2);
>                if (ret)
>                        return ret;
>        }
>
> -     if (!od_table->GfxclkVolt3) {
> +     if (!boot_od_table->GfxclkVolt3) {
>                ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
> -                                                             &od_table->GfxclkVolt3,
> -                                                             od_table->GfxclkFreq3);
> +                                                             &boot_od_table->GfxclkVolt3,
> +                                                             boot_od_table->GfxclkFreq3);
>                if (ret)
>                        return ret;
>        }
>
> -     memcpy(boot_od_table, od_table, sizeof(OverDriveTable_t));
> +     navi10_dump_od_table(smu, boot_od_table);
>
> -     navi10_dump_od_table(smu, od_table);
> +     memcpy(od_table, boot_od_table, sizeof(OverDriveTable_t));
> +     memcpy(user_od_table, boot_od_table, sizeof(OverDriveTable_t));
>
>        return 0;
>   }
> @@ -2429,11 +2440,20 @@ static int navi10_od_edit_dpm_table(struct smu_context *smu, enum PP_OD_DPM_TABL
>                memcpy(table_context->overdrive_table, table_context->boot_overdrive_table, sizeof(OverDriveTable_t));
>                break;
>        case PP_OD_COMMIT_DPM_TABLE:
> -             navi10_dump_od_table(smu, od_table);
> -             ret = smu_cmn_update_table(smu, SMU_TABLE_OVERDRIVE, 0, (void *)od_table, true);
> -             if (ret) {
> -                     dev_err(smu->adev->dev, "Failed to import overdrive table!\n");
> -                     return ret;
> +             if (memcmp(od_table, table_context->user_overdrive_table, sizeof(OverDriveTable_t))) {
> +                     navi10_dump_od_table(smu, od_table);
> +                     ret = smu_cmn_update_table(smu, SMU_TABLE_OVERDRIVE, 0, (void *)od_table, true);
> +                     if (ret) {
> +                             dev_err(smu->adev->dev, "Failed to import overdrive table!\n");
> +                             return ret;
> +                     }
> +                     memcpy(table_context->user_overdrive_table, od_table, sizeof(OverDriveTable_t));
> +                     smu->user_dpm_profile.user_od = true;
> +
> +                     if (!memcmp(table_context->user_overdrive_table,
> +                                 table_context->boot_overdrive_table,
> +                                 sizeof(OverDriveTable_t)))
> +                             smu->user_dpm_profile.user_od = false;
>                }
>                break;
>        case PP_OD_EDIT_VDDC_CURVE:
> @@ -3262,6 +3282,7 @@ static const struct pptable_funcs navi10_ppt_funcs = {
>        .set_soft_freq_limited_range = smu_v11_0_set_soft_freq_limited_range,
>        .set_default_od_settings = navi10_set_default_od_settings,
>        .od_edit_dpm_table = navi10_od_edit_dpm_table,
> +     .restore_user_od_settings = smu_v11_0_restore_user_od_settings,
>        .run_btc = navi10_run_btc,
>        .set_power_source = smu_v11_0_set_power_source,
>        .get_pp_feature_mask = smu_cmn_get_pp_feature_mask,
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> index 0a5d46ac9ccd..7aa47dbba8d8 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> @@ -422,10 +422,20 @@ int smu_v11_0_init_smc_tables(struct smu_context *smu)
>                        ret = -ENOMEM;
>                        goto err3_out;
>                }
> +
> +             smu_table->user_overdrive_table =
> +                     kzalloc(tables[SMU_TABLE_OVERDRIVE].size, GFP_KERNEL);
> +             if (!smu_table->user_overdrive_table) {
> +                     ret = -ENOMEM;
> +                     goto err4_out;
> +             }
> +
>        }
>
>        return 0;
>
> +err4_out:
> +     kfree(smu_table->boot_overdrive_table);
>   err3_out:
>        kfree(smu_table->overdrive_table);
>   err2_out:
> @@ -442,12 +452,14 @@ int smu_v11_0_fini_smc_tables(struct smu_context *smu)
>        struct smu_dpm_context *smu_dpm = &smu->smu_dpm;
>
>        kfree(smu_table->gpu_metrics_table);
> +     kfree(smu_table->user_overdrive_table);
>        kfree(smu_table->boot_overdrive_table);
>        kfree(smu_table->overdrive_table);
>        kfree(smu_table->max_sustainable_clocks);
>        kfree(smu_table->driver_pptable);
>        kfree(smu_table->clocks_table);
>        smu_table->gpu_metrics_table = NULL;
> +     smu_table->user_overdrive_table = NULL;
>        smu_table->boot_overdrive_table = NULL;
>        smu_table->overdrive_table = NULL;
>        smu_table->max_sustainable_clocks = NULL;
> @@ -2101,3 +2113,16 @@ int smu_v11_0_deep_sleep_control(struct smu_context *smu,
>
>        return ret;
>   }
> +
> +int smu_v11_0_restore_user_od_settings(struct smu_context *smu)
> +{
> +     struct smu_table_context *table_context = &smu->smu_table;
> +     void *user_od_table = table_context->user_overdrive_table;
> +     int ret = 0;
> +
> +     ret = smu_cmn_update_table(smu, SMU_TABLE_OVERDRIVE, 0, (void *)user_od_table, true);
> +     if (ret)
> +             dev_err(smu->adev->dev, "Failed to import overdrive table!\n");
> +
> +     return ret;
> +}
>

[-- Attachment #1.2: Type: text/html, Size: 28709 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH V2 1/2] drm/amd/pm: restore user customized OD settings properly for NV1x
  2021-07-23 10:09 ` Lazar, Lijo
  2021-07-23 13:35   ` Deucher, Alexander
@ 2021-07-23 13:44   ` Chen, Guchun
  2021-07-23 14:28     ` Lazar, Lijo
  2021-07-26  7:20     ` Quan, Evan
  1 sibling, 2 replies; 7+ messages in thread
From: Chen, Guchun @ 2021-07-23 13:44 UTC (permalink / raw)
  To: Lazar, Lijo, Quan, Evan, amd-gfx; +Cc: Deucher, Alexander

[Public]

Just curious that in the patch, it adds a variable *user_od* serving the check of applying user customized OD setting. Instead of this, does it make sense to use the *flag* in struct smu_user_dpm_profile for this? As we have below bit in pm/inc/amdgpu_smu.h, can we add another bit for OD restore? This will help unify the usage of *flag* in SMU.

#define SMU_DPM_USER_PROFILE_RESTORE (1 << 0)

Regards,
Guchun

-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Lazar, Lijo
Sent: Friday, July 23, 2021 6:09 PM
To: Quan, Evan <Evan.Quan@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
Subject: Re: [PATCH V2 1/2] drm/amd/pm: restore user customized OD settings properly for NV1x

The series looks good to me, though I prefer to use a common logic to restore od settings so that smuv12,smuv13 gets the restore feature by default once they add the user table logic. Don't have strong argument for it unless Alex, Kenneth or others have some comments.

Anyway, the series is
	Reviewed-by: Lijo Lazar <lijo.lazar@amd.com>

On 7/23/2021 2:39 PM, Evan Quan wrote:
> The customized OD settings can be divided into two parts: those 
> committed ones and non-committed ones.
>    - For those changes which had been fed to SMU before S3/S4/Runpm
>      suspend kicked, they are committed changes. They should be properly
>      restored and fed to SMU on S3/S4/Runpm resume.
>    - For those non-committed changes, they are restored only without feeding
>      to SMU.
> 
> Change-Id: Iea7cf7908dfcd919a4d0205e10bff91b1149a440
> Signed-off-by: Evan Quan <evan.quan@amd.com>
> --
> v1->v2
>    - better naming and logic revised for checking OD setting 
> update(Lijo)
> ---
>   drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h       |  8 +++
>   drivers/gpu/drm/amd/pm/inc/smu_v11_0.h        |  2 +
>   drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c     |  9 +++
>   .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c   | 55 +++++++++++++------
>   .../gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c    | 25 +++++++++
>   5 files changed, 82 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h 
> b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> index 3e89852e4820..c2c201b8e3cf 100644
> --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> @@ -231,6 +231,7 @@ struct smu_user_dpm_profile {
>   	uint32_t power_limit;
>   	uint32_t fan_speed_percent;
>   	uint32_t flags;
> +	uint32_t user_od;
>   
>   	/* user clock state information */
>   	uint32_t clk_mask[SMU_CLK_COUNT];
> @@ -352,6 +353,7 @@ struct smu_table_context
>   
>   	void				*overdrive_table;
>   	void                            *boot_overdrive_table;
> +	void				*user_overdrive_table;
>   
>   	uint32_t			gpu_metrics_table_size;
>   	void				*gpu_metrics_table;
> @@ -623,6 +625,12 @@ struct pptable_funcs {
>   				 enum PP_OD_DPM_TABLE_COMMAND type,
>   				 long *input, uint32_t size);
>   
> +	/**
> +	 * @restore_user_od_settings: Restore the user customized
> +	 *                            OD settings on S3/S4/Runpm resume.
> +	 */
> +	int (*restore_user_od_settings)(struct smu_context *smu);
> +
>   	/**
>   	 * @get_clock_by_type_with_latency: Get the speed and latency of a clock
>   	 *                                  domain.
> diff --git a/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h 
> b/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h
> index 385b2ea5379c..1e42aafbb9fd 100644
> --- a/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h
> +++ b/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h
> @@ -302,5 +302,7 @@ void smu_v11_0_interrupt_work(struct smu_context 
> *smu);
>   
>   int smu_v11_0_set_light_sbr(struct smu_context *smu, bool enable);
>   
> +int smu_v11_0_restore_user_od_settings(struct smu_context *smu);
> +
>   #endif
>   #endif
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c 
> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> index ebe672142808..8ca7337ea5fc 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> @@ -416,6 +416,15 @@ static void smu_restore_dpm_user_profile(struct smu_context *smu)
>   		}
>   	}
>   
> +	/* Restore user customized OD settings */
> +	if (smu->user_dpm_profile.user_od) {
> +		if (smu->ppt_funcs->restore_user_od_settings) {
> +			ret = smu->ppt_funcs->restore_user_od_settings(smu);
> +			if (ret)
> +				dev_err(smu->adev->dev, "Failed to upload customized OD settings\n");
> +		}
> +	}
> +
>   	/* Disable restore flag */
>   	smu->user_dpm_profile.flags &= ~SMU_DPM_USER_PROFILE_RESTORE;
>   }
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c 
> b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> index 59ea59acfb00..d7722c229ddd 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> @@ -2294,41 +2294,52 @@ static int navi10_set_default_od_settings(struct smu_context *smu)
>   		(OverDriveTable_t *)smu->smu_table.overdrive_table;
>   	OverDriveTable_t *boot_od_table =
>   		(OverDriveTable_t *)smu->smu_table.boot_overdrive_table;
> +	OverDriveTable_t *user_od_table =
> +		(OverDriveTable_t *)smu->smu_table.user_overdrive_table;
>   	int ret = 0;
>   
> -	ret = smu_cmn_update_table(smu, SMU_TABLE_OVERDRIVE, 0, (void *)od_table, false);
> +	/*
> +	 * 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) {
>   		dev_err(smu->adev->dev, "Failed to get overdrive table!\n");
>   		return ret;
>   	}
>   
> -	if (!od_table->GfxclkVolt1) {
> +	if (!boot_od_table->GfxclkVolt1) {
>   		ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
> -								&od_table->GfxclkVolt1,
> -								od_table->GfxclkFreq1);
> +								&boot_od_table->GfxclkVolt1,
> +								boot_od_table->GfxclkFreq1);
>   		if (ret)
>   			return ret;
>   	}
>   
> -	if (!od_table->GfxclkVolt2) {
> +	if (!boot_od_table->GfxclkVolt2) {
>   		ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
> -								&od_table->GfxclkVolt2,
> -								od_table->GfxclkFreq2);
> +								&boot_od_table->GfxclkVolt2,
> +								boot_od_table->GfxclkFreq2);
>   		if (ret)
>   			return ret;
>   	}
>   
> -	if (!od_table->GfxclkVolt3) {
> +	if (!boot_od_table->GfxclkVolt3) {
>   		ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
> -								&od_table->GfxclkVolt3,
> -								od_table->GfxclkFreq3);
> +								&boot_od_table->GfxclkVolt3,
> +								boot_od_table->GfxclkFreq3);
>   		if (ret)
>   			return ret;
>   	}
>   
> -	memcpy(boot_od_table, od_table, sizeof(OverDriveTable_t));
> +	navi10_dump_od_table(smu, boot_od_table);
>   
> -	navi10_dump_od_table(smu, od_table);
> +	memcpy(od_table, boot_od_table, sizeof(OverDriveTable_t));
> +	memcpy(user_od_table, boot_od_table, sizeof(OverDriveTable_t));
>   
>   	return 0;
>   }
> @@ -2429,11 +2440,20 @@ static int navi10_od_edit_dpm_table(struct smu_context *smu, enum PP_OD_DPM_TABL
>   		memcpy(table_context->overdrive_table, table_context->boot_overdrive_table, sizeof(OverDriveTable_t));
>   		break;
>   	case PP_OD_COMMIT_DPM_TABLE:
> -		navi10_dump_od_table(smu, od_table);
> -		ret = smu_cmn_update_table(smu, SMU_TABLE_OVERDRIVE, 0, (void *)od_table, true);
> -		if (ret) {
> -			dev_err(smu->adev->dev, "Failed to import overdrive table!\n");
> -			return ret;
> +		if (memcmp(od_table, table_context->user_overdrive_table, sizeof(OverDriveTable_t))) {
> +			navi10_dump_od_table(smu, od_table);
> +			ret = smu_cmn_update_table(smu, SMU_TABLE_OVERDRIVE, 0, (void *)od_table, true);
> +			if (ret) {
> +				dev_err(smu->adev->dev, "Failed to import overdrive table!\n");
> +				return ret;
> +			}
> +			memcpy(table_context->user_overdrive_table, od_table, sizeof(OverDriveTable_t));
> +			smu->user_dpm_profile.user_od = true;
> +
> +			if (!memcmp(table_context->user_overdrive_table,
> +				    table_context->boot_overdrive_table,
> +				    sizeof(OverDriveTable_t)))
> +				smu->user_dpm_profile.user_od = false;
>   		}
>   		break;
>   	case PP_OD_EDIT_VDDC_CURVE:
> @@ -3262,6 +3282,7 @@ static const struct pptable_funcs navi10_ppt_funcs = {
>   	.set_soft_freq_limited_range = smu_v11_0_set_soft_freq_limited_range,
>   	.set_default_od_settings = navi10_set_default_od_settings,
>   	.od_edit_dpm_table = navi10_od_edit_dpm_table,
> +	.restore_user_od_settings = smu_v11_0_restore_user_od_settings,
>   	.run_btc = navi10_run_btc,
>   	.set_power_source = smu_v11_0_set_power_source,
>   	.get_pp_feature_mask = smu_cmn_get_pp_feature_mask, diff --git 
> a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c 
> b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> index 0a5d46ac9ccd..7aa47dbba8d8 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> @@ -422,10 +422,20 @@ int smu_v11_0_init_smc_tables(struct smu_context *smu)
>   			ret = -ENOMEM;
>   			goto err3_out;
>   		}
> +
> +		smu_table->user_overdrive_table =
> +			kzalloc(tables[SMU_TABLE_OVERDRIVE].size, GFP_KERNEL);
> +		if (!smu_table->user_overdrive_table) {
> +			ret = -ENOMEM;
> +			goto err4_out;
> +		}
> +
>   	}
>   
>   	return 0;
>   
> +err4_out:
> +	kfree(smu_table->boot_overdrive_table);
>   err3_out:
>   	kfree(smu_table->overdrive_table);
>   err2_out:
> @@ -442,12 +452,14 @@ int smu_v11_0_fini_smc_tables(struct smu_context *smu)
>   	struct smu_dpm_context *smu_dpm = &smu->smu_dpm;
>   
>   	kfree(smu_table->gpu_metrics_table);
> +	kfree(smu_table->user_overdrive_table);
>   	kfree(smu_table->boot_overdrive_table);
>   	kfree(smu_table->overdrive_table);
>   	kfree(smu_table->max_sustainable_clocks);
>   	kfree(smu_table->driver_pptable);
>   	kfree(smu_table->clocks_table);
>   	smu_table->gpu_metrics_table = NULL;
> +	smu_table->user_overdrive_table = NULL;
>   	smu_table->boot_overdrive_table = NULL;
>   	smu_table->overdrive_table = NULL;
>   	smu_table->max_sustainable_clocks = NULL; @@ -2101,3 +2113,16 @@ 
> int smu_v11_0_deep_sleep_control(struct smu_context *smu,
>   
>   	return ret;
>   }
> +
> +int smu_v11_0_restore_user_od_settings(struct smu_context *smu) {
> +	struct smu_table_context *table_context = &smu->smu_table;
> +	void *user_od_table = table_context->user_overdrive_table;
> +	int ret = 0;
> +
> +	ret = smu_cmn_update_table(smu, SMU_TABLE_OVERDRIVE, 0, (void *)user_od_table, true);
> +	if (ret)
> +		dev_err(smu->adev->dev, "Failed to import overdrive table!\n");
> +
> +	return ret;
> +}
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=04%7C01%7Cguchun.chen%40amd.com%7Cad0c1951c4f54d7a88c508d94dc1f9d6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637626317788023084%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=tq9QDYaQmRxqBNRpFZgRQ0DPBSO6AI3YFN033RQUgOI%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V2 1/2] drm/amd/pm: restore user customized OD settings properly for NV1x
  2021-07-23 13:44   ` Chen, Guchun
@ 2021-07-23 14:28     ` Lazar, Lijo
  2021-07-26  7:20     ` Quan, Evan
  1 sibling, 0 replies; 7+ messages in thread
From: Lazar, Lijo @ 2021-07-23 14:28 UTC (permalink / raw)
  To: Chen, Guchun, Quan, Evan, amd-gfx; +Cc: Deucher, Alexander


[-- Attachment #1.1: Type: text/plain, Size: 13909 bytes --]

Good one, that makes better use of flags.

Thanks,
Lijo
________________________________
From: Chen, Guchun <Guchun.Chen@amd.com>
Sent: Friday, July 23, 2021 7:14:58 PM
To: Lazar, Lijo <Lijo.Lazar@amd.com>; Quan, Evan <Evan.Quan@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
Subject: RE: [PATCH V2 1/2] drm/amd/pm: restore user customized OD settings properly for NV1x

[Public]

Just curious that in the patch, it adds a variable *user_od* serving the check of applying user customized OD setting. Instead of this, does it make sense to use the *flag* in struct smu_user_dpm_profile for this? As we have below bit in pm/inc/amdgpu_smu.h, can we add another bit for OD restore? This will help unify the usage of *flag* in SMU.

#define SMU_DPM_USER_PROFILE_RESTORE (1 << 0)

Regards,
Guchun

-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Lazar, Lijo
Sent: Friday, July 23, 2021 6:09 PM
To: Quan, Evan <Evan.Quan@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
Subject: Re: [PATCH V2 1/2] drm/amd/pm: restore user customized OD settings properly for NV1x

The series looks good to me, though I prefer to use a common logic to restore od settings so that smuv12,smuv13 gets the restore feature by default once they add the user table logic. Don't have strong argument for it unless Alex, Kenneth or others have some comments.

Anyway, the series is
        Reviewed-by: Lijo Lazar <lijo.lazar@amd.com>

On 7/23/2021 2:39 PM, Evan Quan wrote:
> The customized OD settings can be divided into two parts: those
> committed ones and non-committed ones.
>    - For those changes which had been fed to SMU before S3/S4/Runpm
>      suspend kicked, they are committed changes. They should be properly
>      restored and fed to SMU on S3/S4/Runpm resume.
>    - For those non-committed changes, they are restored only without feeding
>      to SMU.
>
> Change-Id: Iea7cf7908dfcd919a4d0205e10bff91b1149a440
> Signed-off-by: Evan Quan <evan.quan@amd.com>
> --
> v1->v2
>    - better naming and logic revised for checking OD setting
> update(Lijo)
> ---
>   drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h       |  8 +++
>   drivers/gpu/drm/amd/pm/inc/smu_v11_0.h        |  2 +
>   drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c     |  9 +++
>   .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c   | 55 +++++++++++++------
>   .../gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c    | 25 +++++++++
>   5 files changed, 82 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> index 3e89852e4820..c2c201b8e3cf 100644
> --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> @@ -231,6 +231,7 @@ struct smu_user_dpm_profile {
>        uint32_t power_limit;
>        uint32_t fan_speed_percent;
>        uint32_t flags;
> +     uint32_t user_od;
>
>        /* user clock state information */
>        uint32_t clk_mask[SMU_CLK_COUNT];
> @@ -352,6 +353,7 @@ struct smu_table_context
>
>        void                            *overdrive_table;
>        void                            *boot_overdrive_table;
> +     void                            *user_overdrive_table;
>
>        uint32_t                        gpu_metrics_table_size;
>        void                            *gpu_metrics_table;
> @@ -623,6 +625,12 @@ struct pptable_funcs {
>                                 enum PP_OD_DPM_TABLE_COMMAND type,
>                                 long *input, uint32_t size);
>
> +     /**
> +      * @restore_user_od_settings: Restore the user customized
> +      *                            OD settings on S3/S4/Runpm resume.
> +      */
> +     int (*restore_user_od_settings)(struct smu_context *smu);
> +
>        /**
>         * @get_clock_by_type_with_latency: Get the speed and latency of a clock
>         *                                  domain.
> diff --git a/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h
> b/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h
> index 385b2ea5379c..1e42aafbb9fd 100644
> --- a/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h
> +++ b/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h
> @@ -302,5 +302,7 @@ void smu_v11_0_interrupt_work(struct smu_context
> *smu);
>
>   int smu_v11_0_set_light_sbr(struct smu_context *smu, bool enable);
>
> +int smu_v11_0_restore_user_od_settings(struct smu_context *smu);
> +
>   #endif
>   #endif
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> index ebe672142808..8ca7337ea5fc 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> @@ -416,6 +416,15 @@ static void smu_restore_dpm_user_profile(struct smu_context *smu)
>                }
>        }
>
> +     /* Restore user customized OD settings */
> +     if (smu->user_dpm_profile.user_od) {
> +             if (smu->ppt_funcs->restore_user_od_settings) {
> +                     ret = smu->ppt_funcs->restore_user_od_settings(smu);
> +                     if (ret)
> +                             dev_err(smu->adev->dev, "Failed to upload customized OD settings\n");
> +             }
> +     }
> +
>        /* Disable restore flag */
>        smu->user_dpm_profile.flags &= ~SMU_DPM_USER_PROFILE_RESTORE;
>   }
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> index 59ea59acfb00..d7722c229ddd 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> @@ -2294,41 +2294,52 @@ static int navi10_set_default_od_settings(struct smu_context *smu)
>                (OverDriveTable_t *)smu->smu_table.overdrive_table;
>        OverDriveTable_t *boot_od_table =
>                (OverDriveTable_t *)smu->smu_table.boot_overdrive_table;
> +     OverDriveTable_t *user_od_table =
> +             (OverDriveTable_t *)smu->smu_table.user_overdrive_table;
>        int ret = 0;
>
> -     ret = smu_cmn_update_table(smu, SMU_TABLE_OVERDRIVE, 0, (void *)od_table, false);
> +     /*
> +      * 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) {
>                dev_err(smu->adev->dev, "Failed to get overdrive table!\n");
>                return ret;
>        }
>
> -     if (!od_table->GfxclkVolt1) {
> +     if (!boot_od_table->GfxclkVolt1) {
>                ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
> -                                                             &od_table->GfxclkVolt1,
> -                                                             od_table->GfxclkFreq1);
> +                                                             &boot_od_table->GfxclkVolt1,
> +                                                             boot_od_table->GfxclkFreq1);
>                if (ret)
>                        return ret;
>        }
>
> -     if (!od_table->GfxclkVolt2) {
> +     if (!boot_od_table->GfxclkVolt2) {
>                ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
> -                                                             &od_table->GfxclkVolt2,
> -                                                             od_table->GfxclkFreq2);
> +                                                             &boot_od_table->GfxclkVolt2,
> +                                                             boot_od_table->GfxclkFreq2);
>                if (ret)
>                        return ret;
>        }
>
> -     if (!od_table->GfxclkVolt3) {
> +     if (!boot_od_table->GfxclkVolt3) {
>                ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
> -                                                             &od_table->GfxclkVolt3,
> -                                                             od_table->GfxclkFreq3);
> +                                                             &boot_od_table->GfxclkVolt3,
> +                                                             boot_od_table->GfxclkFreq3);
>                if (ret)
>                        return ret;
>        }
>
> -     memcpy(boot_od_table, od_table, sizeof(OverDriveTable_t));
> +     navi10_dump_od_table(smu, boot_od_table);
>
> -     navi10_dump_od_table(smu, od_table);
> +     memcpy(od_table, boot_od_table, sizeof(OverDriveTable_t));
> +     memcpy(user_od_table, boot_od_table, sizeof(OverDriveTable_t));
>
>        return 0;
>   }
> @@ -2429,11 +2440,20 @@ static int navi10_od_edit_dpm_table(struct smu_context *smu, enum PP_OD_DPM_TABL
>                memcpy(table_context->overdrive_table, table_context->boot_overdrive_table, sizeof(OverDriveTable_t));
>                break;
>        case PP_OD_COMMIT_DPM_TABLE:
> -             navi10_dump_od_table(smu, od_table);
> -             ret = smu_cmn_update_table(smu, SMU_TABLE_OVERDRIVE, 0, (void *)od_table, true);
> -             if (ret) {
> -                     dev_err(smu->adev->dev, "Failed to import overdrive table!\n");
> -                     return ret;
> +             if (memcmp(od_table, table_context->user_overdrive_table, sizeof(OverDriveTable_t))) {
> +                     navi10_dump_od_table(smu, od_table);
> +                     ret = smu_cmn_update_table(smu, SMU_TABLE_OVERDRIVE, 0, (void *)od_table, true);
> +                     if (ret) {
> +                             dev_err(smu->adev->dev, "Failed to import overdrive table!\n");
> +                             return ret;
> +                     }
> +                     memcpy(table_context->user_overdrive_table, od_table, sizeof(OverDriveTable_t));
> +                     smu->user_dpm_profile.user_od = true;
> +
> +                     if (!memcmp(table_context->user_overdrive_table,
> +                                 table_context->boot_overdrive_table,
> +                                 sizeof(OverDriveTable_t)))
> +                             smu->user_dpm_profile.user_od = false;
>                }
>                break;
>        case PP_OD_EDIT_VDDC_CURVE:
> @@ -3262,6 +3282,7 @@ static const struct pptable_funcs navi10_ppt_funcs = {
>        .set_soft_freq_limited_range = smu_v11_0_set_soft_freq_limited_range,
>        .set_default_od_settings = navi10_set_default_od_settings,
>        .od_edit_dpm_table = navi10_od_edit_dpm_table,
> +     .restore_user_od_settings = smu_v11_0_restore_user_od_settings,
>        .run_btc = navi10_run_btc,
>        .set_power_source = smu_v11_0_set_power_source,
>        .get_pp_feature_mask = smu_cmn_get_pp_feature_mask, diff --git
> a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> index 0a5d46ac9ccd..7aa47dbba8d8 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> @@ -422,10 +422,20 @@ int smu_v11_0_init_smc_tables(struct smu_context *smu)
>                        ret = -ENOMEM;
>                        goto err3_out;
>                }
> +
> +             smu_table->user_overdrive_table =
> +                     kzalloc(tables[SMU_TABLE_OVERDRIVE].size, GFP_KERNEL);
> +             if (!smu_table->user_overdrive_table) {
> +                     ret = -ENOMEM;
> +                     goto err4_out;
> +             }
> +
>        }
>
>        return 0;
>
> +err4_out:
> +     kfree(smu_table->boot_overdrive_table);
>   err3_out:
>        kfree(smu_table->overdrive_table);
>   err2_out:
> @@ -442,12 +452,14 @@ int smu_v11_0_fini_smc_tables(struct smu_context *smu)
>        struct smu_dpm_context *smu_dpm = &smu->smu_dpm;
>
>        kfree(smu_table->gpu_metrics_table);
> +     kfree(smu_table->user_overdrive_table);
>        kfree(smu_table->boot_overdrive_table);
>        kfree(smu_table->overdrive_table);
>        kfree(smu_table->max_sustainable_clocks);
>        kfree(smu_table->driver_pptable);
>        kfree(smu_table->clocks_table);
>        smu_table->gpu_metrics_table = NULL;
> +     smu_table->user_overdrive_table = NULL;
>        smu_table->boot_overdrive_table = NULL;
>        smu_table->overdrive_table = NULL;
>        smu_table->max_sustainable_clocks = NULL; @@ -2101,3 +2113,16 @@
> int smu_v11_0_deep_sleep_control(struct smu_context *smu,
>
>        return ret;
>   }
> +
> +int smu_v11_0_restore_user_od_settings(struct smu_context *smu) {
> +     struct smu_table_context *table_context = &smu->smu_table;
> +     void *user_od_table = table_context->user_overdrive_table;
> +     int ret = 0;
> +
> +     ret = smu_cmn_update_table(smu, SMU_TABLE_OVERDRIVE, 0, (void *)user_od_table, true);
> +     if (ret)
> +             dev_err(smu->adev->dev, "Failed to import overdrive table!\n");
> +
> +     return ret;
> +}
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=04%7C01%7Cguchun.chen%40amd.com%7Cad0c1951c4f54d7a88c508d94dc1f9d6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637626317788023084%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=tq9QDYaQmRxqBNRpFZgRQ0DPBSO6AI3YFN033RQUgOI%3D&amp;reserved=0

[-- Attachment #1.2: Type: text/html, Size: 30032 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH V2 1/2] drm/amd/pm: restore user customized OD settings properly for NV1x
  2021-07-23 13:44   ` Chen, Guchun
  2021-07-23 14:28     ` Lazar, Lijo
@ 2021-07-26  7:20     ` Quan, Evan
  1 sibling, 0 replies; 7+ messages in thread
From: Quan, Evan @ 2021-07-26  7:20 UTC (permalink / raw)
  To: Chen, Guchun, Lazar, Lijo, amd-gfx; +Cc: Deucher, Alexander

[Public]

Hi Guchun,

Currently, the smu->user_dpm_profile.flags seems only used to flag whether we are in a process restoring the user customized settings(power limit/fan settings/od settings etc). We could either drop it or expand its usage. Let me consider more and make a follow-up patch for this.

BR
Evan
> -----Original Message-----
> From: Chen, Guchun <Guchun.Chen@amd.com>
> Sent: Friday, July 23, 2021 9:45 PM
> To: Lazar, Lijo <Lijo.Lazar@amd.com>; Quan, Evan <Evan.Quan@amd.com>;
> amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
> Subject: RE: [PATCH V2 1/2] drm/amd/pm: restore user customized OD
> settings properly for NV1x
> 
> [Public]
> 
> Just curious that in the patch, it adds a variable *user_od* serving the check
> of applying user customized OD setting. Instead of this, does it make sense
> to use the *flag* in struct smu_user_dpm_profile for this? As we have below
> bit in pm/inc/amdgpu_smu.h, can we add another bit for OD restore? This
> will help unify the usage of *flag* in SMU.
> 
> #define SMU_DPM_USER_PROFILE_RESTORE (1 << 0)
> 
> Regards,
> Guchun
> 
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
> Lazar, Lijo
> Sent: Friday, July 23, 2021 6:09 PM
> To: Quan, Evan <Evan.Quan@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
> Subject: Re: [PATCH V2 1/2] drm/amd/pm: restore user customized OD
> settings properly for NV1x
> 
> The series looks good to me, though I prefer to use a common logic to
> restore od settings so that smuv12,smuv13 gets the restore feature by
> default once they add the user table logic. Don't have strong argument for it
> unless Alex, Kenneth or others have some comments.
> 
> Anyway, the series is
> 	Reviewed-by: Lijo Lazar <lijo.lazar@amd.com>
> 
> On 7/23/2021 2:39 PM, Evan Quan wrote:
> > The customized OD settings can be divided into two parts: those
> > committed ones and non-committed ones.
> >    - For those changes which had been fed to SMU before S3/S4/Runpm
> >      suspend kicked, they are committed changes. They should be properly
> >      restored and fed to SMU on S3/S4/Runpm resume.
> >    - For those non-committed changes, they are restored only without
> feeding
> >      to SMU.
> >
> > Change-Id: Iea7cf7908dfcd919a4d0205e10bff91b1149a440
> > Signed-off-by: Evan Quan <evan.quan@amd.com>
> > --
> > v1->v2
> >    - better naming and logic revised for checking OD setting
> > update(Lijo)
> > ---
> >   drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h       |  8 +++
> >   drivers/gpu/drm/amd/pm/inc/smu_v11_0.h        |  2 +
> >   drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c     |  9 +++
> >   .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c   | 55
> +++++++++++++------
> >   .../gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c    | 25 +++++++++
> >   5 files changed, 82 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> > b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> > index 3e89852e4820..c2c201b8e3cf 100644
> > --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> > +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> > @@ -231,6 +231,7 @@ struct smu_user_dpm_profile {
> >   	uint32_t power_limit;
> >   	uint32_t fan_speed_percent;
> >   	uint32_t flags;
> > +	uint32_t user_od;
> >
> >   	/* user clock state information */
> >   	uint32_t clk_mask[SMU_CLK_COUNT];
> > @@ -352,6 +353,7 @@ struct smu_table_context
> >
> >   	void				*overdrive_table;
> >   	void                            *boot_overdrive_table;
> > +	void				*user_overdrive_table;
> >
> >   	uint32_t			gpu_metrics_table_size;
> >   	void				*gpu_metrics_table;
> > @@ -623,6 +625,12 @@ struct pptable_funcs {
> >   				 enum PP_OD_DPM_TABLE_COMMAND
> type,
> >   				 long *input, uint32_t size);
> >
> > +	/**
> > +	 * @restore_user_od_settings: Restore the user customized
> > +	 *                            OD settings on S3/S4/Runpm resume.
> > +	 */
> > +	int (*restore_user_od_settings)(struct smu_context *smu);
> > +
> >   	/**
> >   	 * @get_clock_by_type_with_latency: Get the speed and latency of
> a clock
> >   	 *                                  domain.
> > diff --git a/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h
> > b/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h
> > index 385b2ea5379c..1e42aafbb9fd 100644
> > --- a/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h
> > +++ b/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h
> > @@ -302,5 +302,7 @@ void smu_v11_0_interrupt_work(struct
> smu_context
> > *smu);
> >
> >   int smu_v11_0_set_light_sbr(struct smu_context *smu, bool enable);
> >
> > +int smu_v11_0_restore_user_od_settings(struct smu_context *smu);
> > +
> >   #endif
> >   #endif
> > diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> > b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> > index ebe672142808..8ca7337ea5fc 100644
> > --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> > +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> > @@ -416,6 +416,15 @@ static void smu_restore_dpm_user_profile(struct
> smu_context *smu)
> >   		}
> >   	}
> >
> > +	/* Restore user customized OD settings */
> > +	if (smu->user_dpm_profile.user_od) {
> > +		if (smu->ppt_funcs->restore_user_od_settings) {
> > +			ret = smu->ppt_funcs-
> >restore_user_od_settings(smu);
> > +			if (ret)
> > +				dev_err(smu->adev->dev, "Failed to upload
> customized OD settings\n");
> > +		}
> > +	}
> > +
> >   	/* Disable restore flag */
> >   	smu->user_dpm_profile.flags &=
> ~SMU_DPM_USER_PROFILE_RESTORE;
> >   }
> > diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> > b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> > index 59ea59acfb00..d7722c229ddd 100644
> > --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> > +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> > @@ -2294,41 +2294,52 @@ static int
> navi10_set_default_od_settings(struct smu_context *smu)
> >   		(OverDriveTable_t *)smu->smu_table.overdrive_table;
> >   	OverDriveTable_t *boot_od_table =
> >   		(OverDriveTable_t *)smu->smu_table.boot_overdrive_table;
> > +	OverDriveTable_t *user_od_table =
> > +		(OverDriveTable_t *)smu->smu_table.user_overdrive_table;
> >   	int ret = 0;
> >
> > -	ret = smu_cmn_update_table(smu, SMU_TABLE_OVERDRIVE, 0,
> (void *)od_table, false);
> > +	/*
> > +	 * 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) {
> >   		dev_err(smu->adev->dev, "Failed to get overdrive table!\n");
> >   		return ret;
> >   	}
> >
> > -	if (!od_table->GfxclkVolt1) {
> > +	if (!boot_od_table->GfxclkVolt1) {
> >   		ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
> > -								&od_table-
> >GfxclkVolt1,
> > -								od_table-
> >GfxclkFreq1);
> > +
> 	&boot_od_table->GfxclkVolt1,
> > +
> 	boot_od_table->GfxclkFreq1);
> >   		if (ret)
> >   			return ret;
> >   	}
> >
> > -	if (!od_table->GfxclkVolt2) {
> > +	if (!boot_od_table->GfxclkVolt2) {
> >   		ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
> > -								&od_table-
> >GfxclkVolt2,
> > -								od_table-
> >GfxclkFreq2);
> > +
> 	&boot_od_table->GfxclkVolt2,
> > +
> 	boot_od_table->GfxclkFreq2);
> >   		if (ret)
> >   			return ret;
> >   	}
> >
> > -	if (!od_table->GfxclkVolt3) {
> > +	if (!boot_od_table->GfxclkVolt3) {
> >   		ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
> > -								&od_table-
> >GfxclkVolt3,
> > -								od_table-
> >GfxclkFreq3);
> > +
> 	&boot_od_table->GfxclkVolt3,
> > +
> 	boot_od_table->GfxclkFreq3);
> >   		if (ret)
> >   			return ret;
> >   	}
> >
> > -	memcpy(boot_od_table, od_table, sizeof(OverDriveTable_t));
> > +	navi10_dump_od_table(smu, boot_od_table);
> >
> > -	navi10_dump_od_table(smu, od_table);
> > +	memcpy(od_table, boot_od_table, sizeof(OverDriveTable_t));
> > +	memcpy(user_od_table, boot_od_table, sizeof(OverDriveTable_t));
> >
> >   	return 0;
> >   }
> > @@ -2429,11 +2440,20 @@ static int navi10_od_edit_dpm_table(struct
> smu_context *smu, enum PP_OD_DPM_TABL
> >   		memcpy(table_context->overdrive_table, table_context-
> >boot_overdrive_table, sizeof(OverDriveTable_t));
> >   		break;
> >   	case PP_OD_COMMIT_DPM_TABLE:
> > -		navi10_dump_od_table(smu, od_table);
> > -		ret = smu_cmn_update_table(smu,
> SMU_TABLE_OVERDRIVE, 0, (void *)od_table, true);
> > -		if (ret) {
> > -			dev_err(smu->adev->dev, "Failed to import
> overdrive table!\n");
> > -			return ret;
> > +		if (memcmp(od_table, table_context->user_overdrive_table,
> sizeof(OverDriveTable_t))) {
> > +			navi10_dump_od_table(smu, od_table);
> > +			ret = smu_cmn_update_table(smu,
> SMU_TABLE_OVERDRIVE, 0, (void *)od_table, true);
> > +			if (ret) {
> > +				dev_err(smu->adev->dev, "Failed to import
> overdrive table!\n");
> > +				return ret;
> > +			}
> > +			memcpy(table_context->user_overdrive_table,
> od_table, sizeof(OverDriveTable_t));
> > +			smu->user_dpm_profile.user_od = true;
> > +
> > +			if (!memcmp(table_context->user_overdrive_table,
> > +				    table_context->boot_overdrive_table,
> > +				    sizeof(OverDriveTable_t)))
> > +				smu->user_dpm_profile.user_od = false;
> >   		}
> >   		break;
> >   	case PP_OD_EDIT_VDDC_CURVE:
> > @@ -3262,6 +3282,7 @@ static const struct pptable_funcs
> navi10_ppt_funcs = {
> >   	.set_soft_freq_limited_range =
> smu_v11_0_set_soft_freq_limited_range,
> >   	.set_default_od_settings = navi10_set_default_od_settings,
> >   	.od_edit_dpm_table = navi10_od_edit_dpm_table,
> > +	.restore_user_od_settings = smu_v11_0_restore_user_od_settings,
> >   	.run_btc = navi10_run_btc,
> >   	.set_power_source = smu_v11_0_set_power_source,
> >   	.get_pp_feature_mask = smu_cmn_get_pp_feature_mask, diff --git
> > a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> > b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> > index 0a5d46ac9ccd..7aa47dbba8d8 100644
> > --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> > +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> > @@ -422,10 +422,20 @@ int smu_v11_0_init_smc_tables(struct
> smu_context *smu)
> >   			ret = -ENOMEM;
> >   			goto err3_out;
> >   		}
> > +
> > +		smu_table->user_overdrive_table =
> > +			kzalloc(tables[SMU_TABLE_OVERDRIVE].size,
> GFP_KERNEL);
> > +		if (!smu_table->user_overdrive_table) {
> > +			ret = -ENOMEM;
> > +			goto err4_out;
> > +		}
> > +
> >   	}
> >
> >   	return 0;
> >
> > +err4_out:
> > +	kfree(smu_table->boot_overdrive_table);
> >   err3_out:
> >   	kfree(smu_table->overdrive_table);
> >   err2_out:
> > @@ -442,12 +452,14 @@ int smu_v11_0_fini_smc_tables(struct
> smu_context *smu)
> >   	struct smu_dpm_context *smu_dpm = &smu->smu_dpm;
> >
> >   	kfree(smu_table->gpu_metrics_table);
> > +	kfree(smu_table->user_overdrive_table);
> >   	kfree(smu_table->boot_overdrive_table);
> >   	kfree(smu_table->overdrive_table);
> >   	kfree(smu_table->max_sustainable_clocks);
> >   	kfree(smu_table->driver_pptable);
> >   	kfree(smu_table->clocks_table);
> >   	smu_table->gpu_metrics_table = NULL;
> > +	smu_table->user_overdrive_table = NULL;
> >   	smu_table->boot_overdrive_table = NULL;
> >   	smu_table->overdrive_table = NULL;
> >   	smu_table->max_sustainable_clocks = NULL; @@ -2101,3 +2113,16
> @@
> > int smu_v11_0_deep_sleep_control(struct smu_context *smu,
> >
> >   	return ret;
> >   }
> > +
> > +int smu_v11_0_restore_user_od_settings(struct smu_context *smu) {
> > +	struct smu_table_context *table_context = &smu->smu_table;
> > +	void *user_od_table = table_context->user_overdrive_table;
> > +	int ret = 0;
> > +
> > +	ret = smu_cmn_update_table(smu, SMU_TABLE_OVERDRIVE, 0,
> (void *)user_od_table, true);
> > +	if (ret)
> > +		dev_err(smu->adev->dev, "Failed to import overdrive
> table!\n");
> > +
> > +	return ret;
> > +}
> >
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.
> freedesktop.org%2Fmailman%2Flistinfo%2Famd-
> gfx&amp;data=04%7C01%7Cguchun.chen%40amd.com%7Cad0c1951c4f54d7
> a88c508d94dc1f9d6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C
> 637626317788023084%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwM
> DAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata
> =tq9QDYaQmRxqBNRpFZgRQ0DPBSO6AI3YFN033RQUgOI%3D&amp;reserve
> d=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH V2 1/2] drm/amd/pm: restore user customized OD settings properly for NV1x
  2021-07-23 13:35   ` Deucher, Alexander
@ 2021-07-26  7:30     ` Quan, Evan
  0 siblings, 0 replies; 7+ messages in thread
From: Quan, Evan @ 2021-07-26  7:30 UTC (permalink / raw)
  To: Deucher, Alexander, Lazar, Lijo, amd-gfx


[-- Attachment #1.1: Type: text/plain, Size: 13556 bytes --]

[AMD Official Use Only]

Hi Alex & Lijo,

For now, those code for updating OD table are in smu_cmn.c. And they can only be called from _ppt.c.
So, unless we split some code from smu_cmn.c and move them to upper layer(suggested by Lijo before), otherwise this cannot be performed.
So, definitely we need some follow up patches if we want this done in a more generic way.

BR
Evan
From: Deucher, Alexander <Alexander.Deucher@amd.com>
Sent: Friday, July 23, 2021 9:36 PM
To: Lazar, Lijo <Lijo.Lazar@amd.com>; Quan, Evan <Evan.Quan@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH V2 1/2] drm/amd/pm: restore user customized OD settings properly for NV1x


[AMD Official Use Only]

I haven't had a chance to look at the patches too closely, but if it could be done in a generic may, that makes sense to me.  Maybe as a follow up patch?

Alex

________________________________
From: Lazar, Lijo <Lijo.Lazar@amd.com<mailto:Lijo.Lazar@amd.com>>
Sent: Friday, July 23, 2021 6:09 AM
To: Quan, Evan <Evan.Quan@amd.com<mailto:Evan.Quan@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Cc: Deucher, Alexander <Alexander.Deucher@amd.com<mailto:Alexander.Deucher@amd.com>>
Subject: Re: [PATCH V2 1/2] drm/amd/pm: restore user customized OD settings properly for NV1x

The series looks good to me, though I prefer to use a common logic to
restore od settings so that smuv12,smuv13 gets the restore feature by
default once they add the user table logic. Don't have strong argument
for it unless Alex, Kenneth or others have some comments.

Anyway, the series is
        Reviewed-by: Lijo Lazar <lijo.lazar@amd.com<mailto:lijo.lazar@amd.com>>

On 7/23/2021 2:39 PM, Evan Quan wrote:
> The customized OD settings can be divided into two parts: those
> committed ones and non-committed ones.
>    - For those changes which had been fed to SMU before S3/S4/Runpm
>      suspend kicked, they are committed changes. They should be properly
>      restored and fed to SMU on S3/S4/Runpm resume.
>    - For those non-committed changes, they are restored only without feeding
>      to SMU.
>
> Change-Id: Iea7cf7908dfcd919a4d0205e10bff91b1149a440
> Signed-off-by: Evan Quan <evan.quan@amd.com<mailto:evan.quan@amd.com>>
> --
> v1->v2
>    - better naming and logic revised for checking OD setting update(Lijo)
> ---
>   drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h       |  8 +++
>   drivers/gpu/drm/amd/pm/inc/smu_v11_0.h        |  2 +
>   drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c     |  9 +++
>   .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c   | 55 +++++++++++++------
>   .../gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c    | 25 +++++++++
>   5 files changed, 82 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> index 3e89852e4820..c2c201b8e3cf 100644
> --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> @@ -231,6 +231,7 @@ struct smu_user_dpm_profile {
>        uint32_t power_limit;
>        uint32_t fan_speed_percent;
>        uint32_t flags;
> +     uint32_t user_od;
>
>        /* user clock state information */
>        uint32_t clk_mask[SMU_CLK_COUNT];
> @@ -352,6 +353,7 @@ struct smu_table_context
>
>        void                            *overdrive_table;
>        void                            *boot_overdrive_table;
> +     void                            *user_overdrive_table;
>
>        uint32_t                        gpu_metrics_table_size;
>        void                            *gpu_metrics_table;
> @@ -623,6 +625,12 @@ struct pptable_funcs {
>                                 enum PP_OD_DPM_TABLE_COMMAND type,
>                                 long *input, uint32_t size);
>
> +     /**
> +      * @restore_user_od_settings: Restore the user customized
> +      *                            OD settings on S3/S4/Runpm resume.
> +      */
> +     int (*restore_user_od_settings)(struct smu_context *smu);
> +
>        /**
>         * @get_clock_by_type_with_latency: Get the speed and latency of a clock
>         *                                  domain.
> diff --git a/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h b/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h
> index 385b2ea5379c..1e42aafbb9fd 100644
> --- a/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h
> +++ b/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h
> @@ -302,5 +302,7 @@ void smu_v11_0_interrupt_work(struct smu_context *smu);
>
>   int smu_v11_0_set_light_sbr(struct smu_context *smu, bool enable);
>
> +int smu_v11_0_restore_user_od_settings(struct smu_context *smu);
> +
>   #endif
>   #endif
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> index ebe672142808..8ca7337ea5fc 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> @@ -416,6 +416,15 @@ static void smu_restore_dpm_user_profile(struct smu_context *smu)
>                }
>        }
>
> +     /* Restore user customized OD settings */
> +     if (smu->user_dpm_profile.user_od) {
> +             if (smu->ppt_funcs->restore_user_od_settings) {
> +                     ret = smu->ppt_funcs->restore_user_od_settings(smu);
> +                     if (ret)
> +                             dev_err(smu->adev->dev, "Failed to upload customized OD settings\n");
> +             }
> +     }
> +
>        /* Disable restore flag */
>        smu->user_dpm_profile.flags &= ~SMU_DPM_USER_PROFILE_RESTORE;
>   }
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> index 59ea59acfb00..d7722c229ddd 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> @@ -2294,41 +2294,52 @@ static int navi10_set_default_od_settings(struct smu_context *smu)
>                (OverDriveTable_t *)smu->smu_table.overdrive_table;
>        OverDriveTable_t *boot_od_table =
>                (OverDriveTable_t *)smu->smu_table.boot_overdrive_table;
> +     OverDriveTable_t *user_od_table =
> +             (OverDriveTable_t *)smu->smu_table.user_overdrive_table;
>        int ret = 0;
>
> -     ret = smu_cmn_update_table(smu, SMU_TABLE_OVERDRIVE, 0, (void *)od_table, false);
> +     /*
> +      * 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) {
>                dev_err(smu->adev->dev, "Failed to get overdrive table!\n");
>                return ret;
>        }
>
> -     if (!od_table->GfxclkVolt1) {
> +     if (!boot_od_table->GfxclkVolt1) {
>                ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
> -                                                             &od_table->GfxclkVolt1,
> -                                                             od_table->GfxclkFreq1);
> +                                                             &boot_od_table->GfxclkVolt1,
> +                                                             boot_od_table->GfxclkFreq1);
>                if (ret)
>                        return ret;
>        }
>
> -     if (!od_table->GfxclkVolt2) {
> +     if (!boot_od_table->GfxclkVolt2) {
>                ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
> -                                                             &od_table->GfxclkVolt2,
> -                                                             od_table->GfxclkFreq2);
> +                                                             &boot_od_table->GfxclkVolt2,
> +                                                             boot_od_table->GfxclkFreq2);
>                if (ret)
>                        return ret;
>        }
>
> -     if (!od_table->GfxclkVolt3) {
> +     if (!boot_od_table->GfxclkVolt3) {
>                ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
> -                                                             &od_table->GfxclkVolt3,
> -                                                             od_table->GfxclkFreq3);
> +                                                             &boot_od_table->GfxclkVolt3,
> +                                                             boot_od_table->GfxclkFreq3);
>                if (ret)
>                        return ret;
>        }
>
> -     memcpy(boot_od_table, od_table, sizeof(OverDriveTable_t));
> +     navi10_dump_od_table(smu, boot_od_table);
>
> -     navi10_dump_od_table(smu, od_table);
> +     memcpy(od_table, boot_od_table, sizeof(OverDriveTable_t));
> +     memcpy(user_od_table, boot_od_table, sizeof(OverDriveTable_t));
>
>        return 0;
>   }
> @@ -2429,11 +2440,20 @@ static int navi10_od_edit_dpm_table(struct smu_context *smu, enum PP_OD_DPM_TABL
>                memcpy(table_context->overdrive_table, table_context->boot_overdrive_table, sizeof(OverDriveTable_t));
>                break;
>        case PP_OD_COMMIT_DPM_TABLE:
> -             navi10_dump_od_table(smu, od_table);
> -             ret = smu_cmn_update_table(smu, SMU_TABLE_OVERDRIVE, 0, (void *)od_table, true);
> -             if (ret) {
> -                     dev_err(smu->adev->dev, "Failed to import overdrive table!\n");
> -                     return ret;
> +             if (memcmp(od_table, table_context->user_overdrive_table, sizeof(OverDriveTable_t))) {
> +                     navi10_dump_od_table(smu, od_table);
> +                     ret = smu_cmn_update_table(smu, SMU_TABLE_OVERDRIVE, 0, (void *)od_table, true);
> +                     if (ret) {
> +                             dev_err(smu->adev->dev, "Failed to import overdrive table!\n");
> +                             return ret;
> +                     }
> +                     memcpy(table_context->user_overdrive_table, od_table, sizeof(OverDriveTable_t));
> +                     smu->user_dpm_profile.user_od = true;
> +
> +                     if (!memcmp(table_context->user_overdrive_table,
> +                                 table_context->boot_overdrive_table,
> +                                 sizeof(OverDriveTable_t)))
> +                             smu->user_dpm_profile.user_od = false;
>                }
>                break;
>        case PP_OD_EDIT_VDDC_CURVE:
> @@ -3262,6 +3282,7 @@ static const struct pptable_funcs navi10_ppt_funcs = {
>        .set_soft_freq_limited_range = smu_v11_0_set_soft_freq_limited_range,
>        .set_default_od_settings = navi10_set_default_od_settings,
>        .od_edit_dpm_table = navi10_od_edit_dpm_table,
> +     .restore_user_od_settings = smu_v11_0_restore_user_od_settings,
>        .run_btc = navi10_run_btc,
>        .set_power_source = smu_v11_0_set_power_source,
>        .get_pp_feature_mask = smu_cmn_get_pp_feature_mask,
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> index 0a5d46ac9ccd..7aa47dbba8d8 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> @@ -422,10 +422,20 @@ int smu_v11_0_init_smc_tables(struct smu_context *smu)
>                        ret = -ENOMEM;
>                        goto err3_out;
>                }
> +
> +             smu_table->user_overdrive_table =
> +                     kzalloc(tables[SMU_TABLE_OVERDRIVE].size, GFP_KERNEL);
> +             if (!smu_table->user_overdrive_table) {
> +                     ret = -ENOMEM;
> +                     goto err4_out;
> +             }
> +
>        }
>
>        return 0;
>
> +err4_out:
> +     kfree(smu_table->boot_overdrive_table);
>   err3_out:
>        kfree(smu_table->overdrive_table);
>   err2_out:
> @@ -442,12 +452,14 @@ int smu_v11_0_fini_smc_tables(struct smu_context *smu)
>        struct smu_dpm_context *smu_dpm = &smu->smu_dpm;
>
>        kfree(smu_table->gpu_metrics_table);
> +     kfree(smu_table->user_overdrive_table);
>        kfree(smu_table->boot_overdrive_table);
>        kfree(smu_table->overdrive_table);
>        kfree(smu_table->max_sustainable_clocks);
>        kfree(smu_table->driver_pptable);
>        kfree(smu_table->clocks_table);
>        smu_table->gpu_metrics_table = NULL;
> +     smu_table->user_overdrive_table = NULL;
>        smu_table->boot_overdrive_table = NULL;
>        smu_table->overdrive_table = NULL;
>        smu_table->max_sustainable_clocks = NULL;
> @@ -2101,3 +2113,16 @@ int smu_v11_0_deep_sleep_control(struct smu_context *smu,
>
>        return ret;
>   }
> +
> +int smu_v11_0_restore_user_od_settings(struct smu_context *smu)
> +{
> +     struct smu_table_context *table_context = &smu->smu_table;
> +     void *user_od_table = table_context->user_overdrive_table;
> +     int ret = 0;
> +
> +     ret = smu_cmn_update_table(smu, SMU_TABLE_OVERDRIVE, 0, (void *)user_od_table, true);
> +     if (ret)
> +             dev_err(smu->adev->dev, "Failed to import overdrive table!\n");
> +
> +     return ret;
> +}
>

[-- Attachment #1.2: Type: text/html, Size: 32431 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-07-26  7:30 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-23  9:09 [PATCH V2 1/2] drm/amd/pm: restore user customized OD settings properly for NV1x Evan Quan
2021-07-23 10:09 ` Lazar, Lijo
2021-07-23 13:35   ` Deucher, Alexander
2021-07-26  7:30     ` Quan, Evan
2021-07-23 13:44   ` Chen, Guchun
2021-07-23 14:28     ` Lazar, Lijo
2021-07-26  7:20     ` Quan, Evan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.