PMFW interprets OverDriveTable.FeatureCtrlMask as the complete set of overdrive features that are currently overridden, not as a delta describing the operation at hand. On import it resets every feature whose bit is clear back to its default value - for the VF curve that means zeroing VoltageOffsetPerZoneBoundary[] outright.
The driver assumed the opposite. smu_v14_0_2_od_edit_dpm_table() clears the mask after a successful commit, so a subsequent above-default power1_cap write goes through smu_v14_0_2_set_ppt_limit() and uploads a table carrying only PP_OD_FEATURE_PPT_BIT. PMFW then drops the user's voltage offset and reverts to the stock V/F curve, while the driver's cached user table still reports the offset through pp_od_clk_voltage. Closes: https://gitlab.freedesktop.org/drm/amd/-/work_items/5935 Signed-off-by: Kenneth Feng <[email protected]> --- drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 2 + .../drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c | 35 +++++++---------- .../drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c | 32 +++++++--------- .../drm/amd/pm/swsmu/smu14/smu_v14_0_2_ppt.c | 38 +++++++++++-------- 4 files changed, 52 insertions(+), 55 deletions(-) diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h index f516d0d3bb92..9d47d2ab1b49 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h @@ -433,6 +433,8 @@ struct smu_table_context { void *boot_overdrive_table; void *user_overdrive_table; + uint32_t user_od_feature_mask; + struct smu_driver_table driver_tables[SMU_DRIVER_TABLE_COUNT]; }; diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c index 6beafabf1306..468aaeb79ffb 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c @@ -1569,7 +1569,6 @@ static int smu_v13_0_0_od_edit_dpm_table(struct smu_context *smu, struct amdgpu_device *adev = smu->adev; uint32_t offset_of_voltageoffset; int32_t minimum, maximum; - uint32_t feature_ctrlmask; int i, ret = 0; switch (type) { @@ -1891,21 +1890,16 @@ static int smu_v13_0_0_od_edit_dpm_table(struct smu_context *smu, if (ret) return ret; } else { - feature_ctrlmask = od_table->OverDriveTable.FeatureCtrlMask; memcpy(od_table, table_context->boot_overdrive_table, sizeof(OverDriveTableExternal_t)); - od_table->OverDriveTable.FeatureCtrlMask = feature_ctrlmask; + od_table->OverDriveTable.FeatureCtrlMask = 0; + table_context->user_od_feature_mask = 0; } fallthrough; case PP_OD_COMMIT_DPM_TABLE: - /* - * The member below instructs PMFW the settings focused in - * this single operation. - * `uint32_t FeatureCtrlMask;` - * It does not contain actual informations about user's custom - * settings. Thus we do not cache it. - */ + table_context->user_od_feature_mask = + od_table->OverDriveTable.FeatureCtrlMask; offset_of_voltageoffset = offsetof(OverDriveTable_t, VoltageOffsetPerZoneBoundary); if (memcmp((u8 *)od_table + offset_of_voltageoffset, table_context->user_overdrive_table + offset_of_voltageoffset, @@ -1918,7 +1912,6 @@ static int smu_v13_0_0_od_edit_dpm_table(struct smu_context *smu, return ret; } - od_table->OverDriveTable.FeatureCtrlMask = 0; memcpy(table_context->user_overdrive_table + offset_of_voltageoffset, (u8 *)od_table + offset_of_voltageoffset, sizeof(OverDriveTableExternal_t) - offset_of_voltageoffset); @@ -2201,6 +2194,7 @@ static int smu_v13_0_0_set_default_od_settings(struct smu_context *smu) boot_od_table, sizeof(OverDriveTableExternal_t)); smu->user_dpm_profile.user_od = false; + smu->smu_table.user_od_feature_mask = 0; } else if (smu->user_dpm_profile.user_od) { memcpy(&user_od_table_bak, user_od_table, @@ -2250,12 +2244,8 @@ static int smu_v13_0_0_restore_user_od_settings(struct smu_context *smu) OverDriveTableExternal_t *od_table = table_context->overdrive_table; OverDriveTableExternal_t *user_od_table = table_context->user_overdrive_table; int res; - - user_od_table->OverDriveTable.FeatureCtrlMask = BIT(PP_OD_FEATURE_GFXCLK_BIT) | - BIT(PP_OD_FEATURE_UCLK_BIT) | - BIT(PP_OD_FEATURE_GFX_VF_CURVE_BIT) | - BIT(PP_OD_FEATURE_FAN_CURVE_BIT) | - BIT(PP_OD_FEATURE_ZERO_FAN_BIT); + user_od_table->OverDriveTable.FeatureCtrlMask = + table_context->user_od_feature_mask; res = smu_v13_0_0_upload_overdrive_table(smu, user_od_table); user_od_table->OverDriveTable.FeatureCtrlMask = 0; if (res == 0) @@ -3069,8 +3059,10 @@ static int smu_v13_0_0_set_ppt_limit(struct smu_context *smu, if (current_od_table.OverDriveTable.Ppt) { od_table->OverDriveTable.Ppt = 0; - od_table->OverDriveTable.FeatureCtrlMask |= 1U << PP_OD_FEATURE_PPT_BIT; - + table_context->user_od_feature_mask &= + ~(1U << PP_OD_FEATURE_PPT_BIT); + od_table->OverDriveTable.FeatureCtrlMask = + table_context->user_od_feature_mask; ret = smu_v13_0_0_upload_overdrive_table(smu, od_table); if (ret) { dev_err(smu->adev->dev, "Failed to upload overdrive table!\n"); @@ -3084,8 +3076,9 @@ static int smu_v13_0_0_set_ppt_limit(struct smu_context *smu, return ret; od_table->OverDriveTable.Ppt = (limit * 100) / msg_limit - 100; - od_table->OverDriveTable.FeatureCtrlMask |= 1U << PP_OD_FEATURE_PPT_BIT; - + table_context->user_od_feature_mask |= 1U << PP_OD_FEATURE_PPT_BIT; + od_table->OverDriveTable.FeatureCtrlMask = + table_context->user_od_feature_mask; ret = smu_v13_0_0_upload_overdrive_table(smu, od_table); if (ret) { dev_err(smu->adev->dev, "Failed to upload overdrive table!\n"); diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c index 414fb436d88f..b96152d3920f 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c @@ -1578,7 +1578,6 @@ static int smu_v13_0_7_od_edit_dpm_table(struct smu_context *smu, struct amdgpu_device *adev = smu->adev; uint32_t offset_of_voltageoffset; int32_t minimum, maximum; - uint32_t feature_ctrlmask; int i, ret = 0; switch (type) { @@ -1900,22 +1899,17 @@ static int smu_v13_0_7_od_edit_dpm_table(struct smu_context *smu, if (ret) return ret; } else { - feature_ctrlmask = od_table->OverDriveTable.FeatureCtrlMask; memcpy(od_table, table_context->boot_overdrive_table, sizeof(OverDriveTableExternal_t)); - od_table->OverDriveTable.FeatureCtrlMask = feature_ctrlmask; + od_table->OverDriveTable.FeatureCtrlMask = 0; + table_context->user_od_feature_mask = 0; } fallthrough; case PP_OD_COMMIT_DPM_TABLE: - /* - * The member below instructs PMFW the settings focused in - * this single operation. - * `uint32_t FeatureCtrlMask;` - * It does not contain actual informations about user's custom - * settings. Thus we do not cache it. - */ + table_context->user_od_feature_mask = + od_table->OverDriveTable.FeatureCtrlMask; offset_of_voltageoffset = offsetof(OverDriveTable_t, VoltageOffsetPerZoneBoundary); if (memcmp((u8 *)od_table + offset_of_voltageoffset, table_context->user_overdrive_table + offset_of_voltageoffset, @@ -1928,7 +1922,6 @@ static int smu_v13_0_7_od_edit_dpm_table(struct smu_context *smu, return ret; } - od_table->OverDriveTable.FeatureCtrlMask = 0; memcpy(table_context->user_overdrive_table + offset_of_voltageoffset, (u8 *)od_table + offset_of_voltageoffset, sizeof(OverDriveTableExternal_t) - offset_of_voltageoffset); @@ -2203,6 +2196,7 @@ static int smu_v13_0_7_set_default_od_settings(struct smu_context *smu) boot_od_table, sizeof(OverDriveTableExternal_t)); smu->user_dpm_profile.user_od = false; + smu->smu_table.user_od_feature_mask = 0; } else if (smu->user_dpm_profile.user_od) { memcpy(&user_od_table_bak, user_od_table, @@ -2253,11 +2247,8 @@ static int smu_v13_0_7_restore_user_od_settings(struct smu_context *smu) OverDriveTableExternal_t *user_od_table = table_context->user_overdrive_table; int res; - user_od_table->OverDriveTable.FeatureCtrlMask = BIT(PP_OD_FEATURE_GFXCLK_BIT) | - BIT(PP_OD_FEATURE_UCLK_BIT) | - BIT(PP_OD_FEATURE_GFX_VF_CURVE_BIT) | - BIT(PP_OD_FEATURE_FAN_CURVE_BIT) | - BIT(PP_OD_FEATURE_ZERO_FAN_BIT); + user_od_table->OverDriveTable.FeatureCtrlMask = + table_context->user_od_feature_mask; res = smu_v13_0_7_upload_overdrive_table(smu, user_od_table); user_od_table->OverDriveTable.FeatureCtrlMask = 0; if (res == 0) @@ -2719,7 +2710,10 @@ static int smu_v13_0_7_set_ppt_limit(struct smu_context *smu, if (current_od_table.OverDriveTable.Ppt) { od_table->OverDriveTable.Ppt = 0; - od_table->OverDriveTable.FeatureCtrlMask |= 1U << PP_OD_FEATURE_PPT_BIT; + table_context->user_od_feature_mask &= + ~(1U << PP_OD_FEATURE_PPT_BIT); + od_table->OverDriveTable.FeatureCtrlMask = + table_context->user_od_feature_mask; ret = smu_v13_0_7_upload_overdrive_table(smu, od_table); if (ret) { @@ -2734,7 +2728,9 @@ static int smu_v13_0_7_set_ppt_limit(struct smu_context *smu, return ret; od_table->OverDriveTable.Ppt = (limit * 100) / msg_limit - 100; - od_table->OverDriveTable.FeatureCtrlMask |= 1U << PP_OD_FEATURE_PPT_BIT; + table_context->user_od_feature_mask |= 1U << PP_OD_FEATURE_PPT_BIT; + od_table->OverDriveTable.FeatureCtrlMask = + table_context->user_od_feature_mask; ret = smu_v13_0_7_upload_overdrive_table(smu, od_table); if (ret) { diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0_2_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0_2_ppt.c index 33f93f6d90af..c4dfa66c723b 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0_2_ppt.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0_2_ppt.c @@ -2385,6 +2385,7 @@ static int smu_v14_0_2_set_default_od_settings(struct smu_context *smu) boot_od_table, sizeof(OverDriveTableExternal_t)); smu->user_dpm_profile.user_od = false; + smu->smu_table.user_od_feature_mask = 0; } else if (smu->user_dpm_profile.user_od) { memcpy(&user_od_table_bak, user_od_table, @@ -2433,11 +2434,8 @@ static int smu_v14_0_2_restore_user_od_settings(struct smu_context *smu) OverDriveTableExternal_t *user_od_table = table_context->user_overdrive_table; int res; - user_od_table->OverDriveTable.FeatureCtrlMask = BIT(PP_OD_FEATURE_GFXCLK_BIT) | - BIT(PP_OD_FEATURE_UCLK_BIT) | - BIT(PP_OD_FEATURE_GFX_VF_CURVE_BIT) | - BIT(PP_OD_FEATURE_FAN_CURVE_BIT) | - BIT(PP_OD_FEATURE_ZERO_FAN_BIT); + user_od_table->OverDriveTable.FeatureCtrlMask = + table_context->user_od_feature_mask; res = smu_v14_0_2_upload_overdrive_table(smu, user_od_table); user_od_table->OverDriveTable.FeatureCtrlMask = 0; if (res == 0) @@ -2522,7 +2520,6 @@ static int smu_v14_0_2_od_edit_dpm_table(struct smu_context *smu, struct amdgpu_device *adev = smu->adev; uint32_t offset_of_voltageoffset; int32_t minimum, maximum; - uint32_t feature_ctrlmask; int i, ret = 0; switch (type) { @@ -2811,21 +2808,26 @@ static int smu_v14_0_2_od_edit_dpm_table(struct smu_context *smu, if (ret) return ret; } else { - feature_ctrlmask = od_table->OverDriveTable.FeatureCtrlMask; memcpy(od_table, table_context->boot_overdrive_table, sizeof(OverDriveTableExternal_t)); - od_table->OverDriveTable.FeatureCtrlMask = feature_ctrlmask; + od_table->OverDriveTable.FeatureCtrlMask = 0; + table_context->user_od_feature_mask = 0; } fallthrough; case PP_OD_COMMIT_DPM_TABLE: /* - * The member below instructs PMFW the settings focused in - * this single operation. - * `uint32_t FeatureCtrlMask;` - * It does not contain actual informations about user's custom - * settings. Thus we do not cache it. + * PMFW interprets FeatureCtrlMask as the complete set of OD + * features that are currently overridden, not as a delta + * describing this single operation. Any feature whose bit is + * clear gets reset to its default on import. od_table's mask + * has accumulated every feature the user touched since the + * last full restore, so record it and keep restating it on + * subsequent uploads. */ + table_context->user_od_feature_mask = + od_table->OverDriveTable.FeatureCtrlMask; + offset_of_voltageoffset = offsetof(OverDriveTable_t, VoltageOffsetPerZoneBoundary); if (memcmp((u8 *)od_table + offset_of_voltageoffset, table_context->user_overdrive_table + offset_of_voltageoffset, @@ -2838,7 +2840,6 @@ static int smu_v14_0_2_od_edit_dpm_table(struct smu_context *smu, return ret; } - od_table->OverDriveTable.FeatureCtrlMask = 0; memcpy(table_context->user_overdrive_table + offset_of_voltageoffset, (u8 *)od_table + offset_of_voltageoffset, sizeof(OverDriveTableExternal_t) - offset_of_voltageoffset); @@ -2882,7 +2883,10 @@ static int smu_v14_0_2_set_ppt_limit(struct smu_context *smu, if (current_od_table.OverDriveTable.Ppt) { od_table->OverDriveTable.Ppt = 0; - od_table->OverDriveTable.FeatureCtrlMask |= 1U << PP_OD_FEATURE_PPT_BIT; + table_context->user_od_feature_mask &= + ~(1U << PP_OD_FEATURE_PPT_BIT); + od_table->OverDriveTable.FeatureCtrlMask = + table_context->user_od_feature_mask; ret = smu_v14_0_2_upload_overdrive_table(smu, od_table); if (ret) { @@ -2897,7 +2901,9 @@ static int smu_v14_0_2_set_ppt_limit(struct smu_context *smu, return ret; od_table->OverDriveTable.Ppt = (limit * 100) / msg_limit - 100; - od_table->OverDriveTable.FeatureCtrlMask |= 1U << PP_OD_FEATURE_PPT_BIT; + table_context->user_od_feature_mask |= 1U << PP_OD_FEATURE_PPT_BIT; + od_table->OverDriveTable.FeatureCtrlMask = + table_context->user_od_feature_mask; ret = smu_v14_0_2_upload_overdrive_table(smu, od_table); if (ret) { -- 2.43.0
