[PATCH 0/5] drm/amd/pm: overdrive support for smu8 (Carrizo/Stoney)

[PATCH 0/5] drm/amd/pm: overdrive support for smu8 (Carrizo/Stoney)

[Joshua Horne]

This series implements pp_od_clk_voltage for the smu8 backend, so the
graphics DPM levels of Carrizo and Stoney can be underclocked and
undervolted, or overclocked where the VBIOS declares an overdrive
limit. It uses the smu7 format and stays behind PP_OVERDRIVE_MASK.

Patch 1 fixes a stock bug I ran into along the way and does not depend
on the rest: the boot clock is below the lowest graphics level on this
VBIOS, so SetSclkSoftMax is sent 0xffffffff each time DPM is started,
which includes every driver load. It can be applied on its own.

A few things the diff does not show:

- The voltage is GfxVid, which drives the separate VDDGFX rail (SVI1 in
  the BKDGs for family 15h models 60h-6Fh and 70h-7Fh). GnbVid is not
  touched, so the NB rail and the memory controller are unaffected. The
  allowed range is the GfxVid range the VBIOS uses for the enabled
  levels, so nothing can go above the highest stock graphics voltage,
  and the hardware MinVid/MaxVid filter still applies.

- The new table is sent with ClkTableXferToSmu while DPM is running.
  The SMU firmware accepts it, but the level it is in keeps its old
  values until the SMU next enters that level. With the level forced,
  the VDDGFX sensor still read the old voltage a second after the
  commit, and the new one after a switch to another level and back
  (the clock itself cannot be read back). In auto mode the switch
  comes with the next level change; the driver does not force one.
  smu7 sends SCLKDPM_FreezeLevel/UnfreezeLevel around its upload, but
  those only keep the SMC from changing level while the driver
  rewrites the table in SMC RAM. smu8 has no such messages, and its
  table is transferred from a DRAM copy by a single message. Patch 5
  documents this in the pp_od_clk_voltage kernel-doc section in
  amdgpu_pm.c.

Known limitations:

- DCE 11 reads the sclk levels only once, in
  bw_calcs_data_update_from_pplib() when the resource pool is created,
  so after a large underclock its bandwidth calculations still assume
  the VBIOS clocks. smu7 and vega10 overdrive behave the same way on
  DCE 11.2 and DCE 12. A fix belongs in DC rather than in the
  powerplay backends, so it is not part of this series.

- Stoney is untested. It shares the code path and, per its BKDG, the
  rail layout, but whether the clock table is accepted at runtime is up
  to the SMU firmware, which ships with the system BIOS. Tested-by from
  Stoney owners would be welcome.

Testing was done on an A10-8700P (ASUS X555DA, VBIOS 113-C75100-028,
SMU firmware 18.60.00), on 7.3.1 with amdgpu built from that tree plus
these patches and "drm/amd/pm: send smu8 sclk level indices, not the
level mask" (already applied). The series was then rebased onto
amd-staging-drm-next without changes to the code and was not run again
there. This VBIOS has no overdrive limit, so only underclocking and
undervolting could be tested.

- Voltage changes were checked with the VDDGFX current-VID sensor:
  level 0 raised from 787 to 800 mV and level 7 lowered from 1187 mV
  (1163 mV requested, rounded up to the next VID, 1168 mV), each
  committed while the SMU sat on that level. The new value appeared on
  the next entry into the level and not before.

- A clock above the range, a voltage above the range and a table with
  non-increasing clocks were rejected with -EINVAL, and "r" restored
  the VBIOS levels.

- S3 with level 0 committed at 300 MHz / 800 mV: right after resume,
  before any level change, OD_SCLK still showed 800 mV and the VDDGFX
  sensor read 800 mV.

- On amd-staging-drm-next, each patch builds with clang W=1 without
  new warnings, and the series adds no sparse or smatch warnings.
  checkpatch --strict only reports the uint32_t and CamelCase
  checks that follow the existing powerplay code.

Two choices are left to whoever applies this:

- Earlier APU overdrive support (Raven, Renoir, VanGogh) is
  clock-only. The voltage control here stays within the GfxVid range
  of the VBIOS levels; a clock-only version can be sent instead if
  that is preferred.

- Stoney is enabled without having been tested. Limiting the series to
  CHIP_CARRIZO until someone tests it is a one-line change.

I used an AI coding assistant (Claude Code) for this series. It wrote
the code, the changelogs and a draft of this letter, and wrote and ran
the build checks and test scripts. I reviewed the series, ran the tests
on my hardware, and I am responsible for it.

---
Joshua Horne (5):
      drm/amd/pm: keep smu8 sclk level lookups within the table
      drm/amd/pm: move smu8_populate_umdpstate_clocks() up
      drm/amd/pm: show the smu8 overdrive sclk table
      drm/amd/pm: allow editing the smu8 overdrive sclk table
      drm/amd/pm: stop disabling overdrive on Carrizo and Stoney

 drivers/gpu/drm/amd/pm/amdgpu_pm.c                 |   6 +
 drivers/gpu/drm/amd/pm/powerplay/hwmgr/hwmgr.c     |   1 -
 .../gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c    | 319 ++++++++++++++++++++-
 .../gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.h    |   8 +
 4 files changed, 319 insertions(+), 15 deletions(-)
---
base-commit: 47df73aa49d1988c8f684b7caf8be6570e966844
change-id: 20261113-smu8-overdrive-4208a3756d57

[PATCH 0/5] drm/amd/pm: overdrive support for smu8 (Carrizo/Stoney)

[Joshua Horne via B4 Relay]

This series implements pp_od_clk_voltage for the smu8 backend, so the
graphics DPM levels of Carrizo and Stoney can be underclocked and
undervolted, or overclocked where the VBIOS declares an overdrive
limit. It uses the smu7 format and stays behind PP_OVERDRIVE_MASK.

Patch 1 fixes a stock bug I ran into along the way and does not depend
on the rest: the boot clock is below the lowest graphics level on this
VBIOS, so SetSclkSoftMax is sent 0xffffffff each time DPM is started,
which includes every driver load. It can be applied on its own.

A few things the diff does not show:

- The voltage is GfxVid, which drives the separate VDDGFX rail (SVI1 in
  the BKDGs for family 15h models 60h-6Fh and 70h-7Fh). GnbVid is not
  touched, so the NB rail and the memory controller are unaffected. The
  allowed range is the GfxVid range the VBIOS uses for the enabled
  levels, so nothing can go above the highest stock graphics voltage,
  and the hardware MinVid/MaxVid filter still applies.

- The new table is sent with ClkTableXferToSmu while DPM is running.
  The SMU firmware accepts it, but the level it is in keeps its old
  values until the SMU next enters that level. With the level forced,
  the VDDGFX sensor still read the old voltage a second after the
  commit, and the new one after a switch to another level and back
  (the clock itself cannot be read back). In auto mode the switch
  comes with the next level change; the driver does not force one.
  smu7 sends SCLKDPM_FreezeLevel/UnfreezeLevel around its upload, but
  those only keep the SMC from changing level while the driver
  rewrites the table in SMC RAM. smu8 has no such messages, and its
  table is transferred from a DRAM copy by a single message. Patch 5
  documents this in the pp_od_clk_voltage kernel-doc section in
  amdgpu_pm.c.

Known limitations:

- DCE 11 reads the sclk levels only once, in
  bw_calcs_data_update_from_pplib() when the resource pool is created,
  so after a large underclock its bandwidth calculations still assume
  the VBIOS clocks. smu7 and vega10 overdrive behave the same way on
  DCE 11.2 and DCE 12. A fix belongs in DC rather than in the
  powerplay backends, so it is not part of this series.

- Stoney is untested. It shares the code path and, per its BKDG, the
  rail layout, but whether the clock table is accepted at runtime is up
  to the SMU firmware, which ships with the system BIOS. Tested-by from
  Stoney owners would be welcome.

Testing was done on an A10-8700P (ASUS X555DA, VBIOS 113-C75100-028,
SMU firmware 18.60.00), on 7.3.1 with amdgpu built from that tree plus
these patches and "drm/amd/pm: send smu8 sclk level indices, not the
level mask" (already applied). The series was then rebased onto
amd-staging-drm-next without changes to the code and was not run again
there. This VBIOS has no overdrive limit, so only underclocking and
undervolting could be tested.

- Voltage changes were checked with the VDDGFX current-VID sensor:
  level 0 raised from 787 to 800 mV and level 7 lowered from 1187 mV
  (1163 mV requested, rounded up to the next VID, 1168 mV), each
  committed while the SMU sat on that level. The new value appeared on
  the next entry into the level and not before.

- A clock above the range, a voltage above the range and a table with
  non-increasing clocks were rejected with -EINVAL, and "r" restored
  the VBIOS levels.

- S3 with level 0 committed at 300 MHz / 800 mV: right after resume,
  before any level change, OD_SCLK still showed 800 mV and the VDDGFX
  sensor read 800 mV.

- On amd-staging-drm-next, each patch builds with clang W=1 without
  new warnings, and the series adds no sparse or smatch warnings.
  checkpatch --strict only reports the uint32_t and CamelCase
  checks that follow the existing powerplay code.

Two choices are left to whoever applies this:

- Earlier APU overdrive support (Raven, Renoir, VanGogh) is
  clock-only. The voltage control here stays within the GfxVid range
  of the VBIOS levels; a clock-only version can be sent instead if
  that is preferred.

- Stoney is enabled without having been tested. Limiting the series to
  CHIP_CARRIZO until someone tests it is a one-line change.

I used an AI coding assistant (Claude Code) for this series. It wrote
the code, the changelogs and a draft of this letter, and wrote and ran
the build checks and test scripts. I reviewed the series, ran the tests
on my hardware, and I am responsible for it.

---
Joshua Horne (5):
      drm/amd/pm: keep smu8 sclk level lookups within the table
      drm/amd/pm: move smu8_populate_umdpstate_clocks() up
      drm/amd/pm: show the smu8 overdrive sclk table
      drm/amd/pm: allow editing the smu8 overdrive sclk table
      drm/amd/pm: stop disabling overdrive on Carrizo and Stoney

 drivers/gpu/drm/amd/pm/amdgpu_pm.c                 |   6 +
 drivers/gpu/drm/amd/pm/powerplay/hwmgr/hwmgr.c     |   1 -
 .../gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c    | 319 ++++++++++++++++++++-
 .../gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.h    |   8 +
 4 files changed, 319 insertions(+), 15 deletions(-)
---
base-commit: 47df73aa49d1988c8f684b7caf8be6570e966844
change-id: 20261113-smu8-overdrive-4208a3756d57

[PATCH 1/5] drm/amd/pm: keep smu8 sclk level lookups within the table

[Joshua Horne]

smu8_get_sclk_level() returns the table size for a minimum clock above
the highest level and -1 for a maximum clock below the lowest level,
and the callers send that to the SMU as the level index.

This happens each time DPM is started, so at every driver load:
smu8_program_bootup_state() looks up the boot engine clock from the
IntegratedSystemInfo table, which is below the lowest graphics level on
at least some Carrizo VBIOS images (200 MHz vs. 300 MHz on an
A10-8700P), so PPSMC_MSG_SetSclkSoftMax is sent with 0xffffffff. A valid
limit is sent shortly after when the performance level is applied, so it
went unnoticed. The display's minimum engine clock is not taken from the
table either.

Select the nearest level instead.

Fixes: bdecc20a986b ("drm/amd/powerplay: add Carrizo dpm support")
Signed-off-by: Joshua Horne <hornejd@etsu.edu>
---
 drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
index 8f82d7c07e8c..f17894b5301f 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
@@ -123,7 +123,9 @@ static uint32_t smu8_get_sclk_level(struct pp_hwmgr *hwmgr,
 	default:
 		break;
 	}
-	return i;
+
+	/* a clock outside of the table selects the nearest level */
+	return clamp(i, 0, (int)table->count - 1);
 }
 
 static uint32_t smu8_get_uvd_level(struct pp_hwmgr *hwmgr,

-- 
2.56.0

[PATCH 1/5] drm/amd/pm: keep smu8 sclk level lookups within the table

[Joshua Horne via B4 Relay]

From: Joshua Horne <hornejd@etsu.edu>

smu8_get_sclk_level() returns the table size for a minimum clock above
the highest level and -1 for a maximum clock below the lowest level,
and the callers send that to the SMU as the level index.

This happens each time DPM is started, so at every driver load:
smu8_program_bootup_state() looks up the boot engine clock from the
IntegratedSystemInfo table, which is below the lowest graphics level on
at least some Carrizo VBIOS images (200 MHz vs. 300 MHz on an
A10-8700P), so PPSMC_MSG_SetSclkSoftMax is sent with 0xffffffff. A valid
limit is sent shortly after when the performance level is applied, so it
went unnoticed. The display's minimum engine clock is not taken from the
table either.

Select the nearest level instead.

Fixes: bdecc20a986b ("drm/amd/powerplay: add Carrizo dpm support")
Signed-off-by: Joshua Horne <hornejd@etsu.edu>
---
 drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
index 8f82d7c07e8c..f17894b5301f 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
@@ -123,7 +123,9 @@ static uint32_t smu8_get_sclk_level(struct pp_hwmgr *hwmgr,
 	default:
 		break;
 	}
-	return i;
+
+	/* a clock outside of the table selects the nearest level */
+	return clamp(i, 0, (int)table->count - 1);
 }
 
 static uint32_t smu8_get_uvd_level(struct pp_hwmgr *hwmgr,

-- 
2.56.0

[PATCH 2/5] drm/amd/pm: move smu8_populate_umdpstate_clocks() up

[Joshua Horne]

The overdrive support added next updates the UMD pstate clocks from a
new helper placed before smu8_set_power_state_tasks(). Move the function
above it, so that no forward declaration is needed.

No functional change.

Signed-off-by: Joshua Horne <hornejd@etsu.edu>
---
 .../gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c    | 24 +++++++++++-----------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
index f17894b5301f..77ab96bae4b0 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
@@ -848,6 +848,18 @@ static int smu8_update_low_mem_pstate(struct pp_hwmgr *hwmgr, const void *input)
 	return 0;
 }
 
+static void smu8_populate_umdpstate_clocks(struct pp_hwmgr *hwmgr)
+{
+	struct phm_clock_voltage_dependency_table *table =
+				hwmgr->dyn_state.vddc_dependency_on_sclk;
+
+	hwmgr->pstate_sclk = table->entries[0].clk / 100;
+	hwmgr->pstate_mclk = 0;
+
+	hwmgr->pstate_sclk_peak = table->entries[table->count - 1].clk / 100;
+	hwmgr->pstate_mclk_peak = 0;
+}
+
 static int smu8_set_power_state_tasks(struct pp_hwmgr *hwmgr, const void *input)
 {
 	int ret = 0;
@@ -988,18 +1000,6 @@ static void smu8_reset_acp_boot_level(struct pp_hwmgr *hwmgr)
 	data->acp_boot_level = 0xff;
 }
 
-static void smu8_populate_umdpstate_clocks(struct pp_hwmgr *hwmgr)
-{
-	struct phm_clock_voltage_dependency_table *table =
-				hwmgr->dyn_state.vddc_dependency_on_sclk;
-
-	hwmgr->pstate_sclk = table->entries[0].clk / 100;
-	hwmgr->pstate_mclk = 0;
-
-	hwmgr->pstate_sclk_peak = table->entries[table->count - 1].clk / 100;
-	hwmgr->pstate_mclk_peak = 0;
-}
-
 static int smu8_enable_dpm_tasks(struct pp_hwmgr *hwmgr)
 {
 	smu8_program_voting_clients(hwmgr);

-- 
2.56.0

[PATCH 2/5] drm/amd/pm: move smu8_populate_umdpstate_clocks() up

[Joshua Horne via B4 Relay]

From: Joshua Horne <hornejd@etsu.edu>

The overdrive support added next updates the UMD pstate clocks from a
new helper placed before smu8_set_power_state_tasks(). Move the function
above it, so that no forward declaration is needed.

No functional change.

Signed-off-by: Joshua Horne <hornejd@etsu.edu>
---
 .../gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c    | 24 +++++++++++-----------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
index f17894b5301f..77ab96bae4b0 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
@@ -848,6 +848,18 @@ static int smu8_update_low_mem_pstate(struct pp_hwmgr *hwmgr, const void *input)
 	return 0;
 }
 
+static void smu8_populate_umdpstate_clocks(struct pp_hwmgr *hwmgr)
+{
+	struct phm_clock_voltage_dependency_table *table =
+				hwmgr->dyn_state.vddc_dependency_on_sclk;
+
+	hwmgr->pstate_sclk = table->entries[0].clk / 100;
+	hwmgr->pstate_mclk = 0;
+
+	hwmgr->pstate_sclk_peak = table->entries[table->count - 1].clk / 100;
+	hwmgr->pstate_mclk_peak = 0;
+}
+
 static int smu8_set_power_state_tasks(struct pp_hwmgr *hwmgr, const void *input)
 {
 	int ret = 0;
@@ -988,18 +1000,6 @@ static void smu8_reset_acp_boot_level(struct pp_hwmgr *hwmgr)
 	data->acp_boot_level = 0xff;
 }
 
-static void smu8_populate_umdpstate_clocks(struct pp_hwmgr *hwmgr)
-{
-	struct phm_clock_voltage_dependency_table *table =
-				hwmgr->dyn_state.vddc_dependency_on_sclk;
-
-	hwmgr->pstate_sclk = table->entries[0].clk / 100;
-	hwmgr->pstate_mclk = 0;
-
-	hwmgr->pstate_sclk_peak = table->entries[table->count - 1].clk / 100;
-	hwmgr->pstate_mclk_peak = 0;
-}
-
 static int smu8_enable_dpm_tasks(struct pp_hwmgr *hwmgr)
 {
 	smu8_program_voting_clients(hwmgr);

-- 
2.56.0

[PATCH 3/5] drm/amd/pm: show the smu8 overdrive sclk table

[Joshua Horne]

Carrizo and Stoney upload up to eight graphics DPM levels to the SMU,
each with an engine clock, a VDDGFX voltage (GfxVid) and an NB voltage
(GnbVid) from the VBIOS dependency tables.

Prepare overdrive support for them in the pp_od_clk_voltage format of
smu7: keep the VBIOS levels and the user's edits in struct
phm_odn_clock_levels as smu7 does, and print the edits as OD_SCLK and
the allowed ranges as OD_RANGE. Only the levels that the SMU enables
(PPSMC_MSG_GetMaxSclkLevel) are used, as the VBIOS table can list
levels above the fused maximum of the part.

The engine clock may range from the lowest level up to the overdrive
limit of the VBIOS. Without a limit, the highest enabled level is used,
so only underclocking is possible, as smu7 and vega10 do since
commit 46defdd6fff7 ("drm/amd/pp: Allow underclocking when od table is
empty in vbios"). The voltage is limited to the range the VBIOS uses
for these levels and kept in mV as on smu7 and vega10, converted from
the VID with convert_to_vddc().

Overdrive stays disabled on these APUs until a later patch.

Signed-off-by: Joshua Horne <hornejd@etsu.edu>
---
 .../gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c    | 75 +++++++++++++++++++++-
 .../gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.h    |  7 ++
 2 files changed, 81 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
index 77ab96bae4b0..93a7e7e176f3 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
@@ -1080,6 +1080,54 @@ static int smu8_apply_state_adjust_rules(struct pp_hwmgr *hwmgr,
 	return 0;
 }
 
+static void smu8_init_od_settings(struct pp_hwmgr *hwmgr)
+{
+	struct smu8_hwmgr *data = hwmgr->backend;
+	struct phm_clock_voltage_dependency_table *vddc_table =
+				hwmgr->dyn_state.vddc_dependency_on_sclk;
+	struct phm_clock_voltage_dependency_table *vdd_gfx_table =
+				hwmgr->dyn_state.vdd_gfx_dependency_on_sclk;
+	struct phm_odn_clock_levels *od = &data->od_default_sclk;
+	struct amdgpu_device *adev = hwmgr->adev;
+	uint32_t i, mv;
+
+	if (!vddc_table || !vdd_gfx_table || !vddc_table->count ||
+	    vddc_table->count > ARRAY_SIZE(od->entries) ||
+	    vdd_gfx_table->count != vddc_table->count) {
+		dev_info(adev->dev,
+			 "OverDrive not supported by the sclk tables\n");
+		hwmgr->od_enabled = false;
+		return;
+	}
+
+	/*
+	 * Only expose the levels that the SMU enables: the VBIOS table can
+	 * list levels above the fused maximum of the part.
+	 */
+	od->num_of_pl = min(vddc_table->count, smu8_get_max_sclk_level(hwmgr));
+	data->od_min_vddc = U32_MAX;
+	data->od_max_vddc = 0;
+	for (i = 0; i < od->num_of_pl; i++) {
+		mv = convert_to_vddc((uint8_t)vdd_gfx_table->entries[i].v);
+		od->entries[i].clock = vddc_table->entries[i].clk;
+		od->entries[i].vddc = mv;
+		od->entries[i].enabled = true;
+		data->od_min_vddc = min(data->od_min_vddc, mv);
+		data->od_max_vddc = max(data->od_max_vddc, mv);
+	}
+
+	/*
+	 * Without an overdrive limit in the VBIOS only allow underclocking,
+	 * as smu7 and vega10 do. The voltages are limited to the range that
+	 * the VBIOS uses for these levels.
+	 */
+	if (!hwmgr->platform_descriptor.overdriveLimit.engineClock)
+		hwmgr->platform_descriptor.overdriveLimit.engineClock =
+			od->entries[od->num_of_pl - 1].clock;
+
+	data->od_edit_sclk = *od;
+}
+
 static int smu8_hwmgr_backend_init(struct pp_hwmgr *hwmgr)
 {
 	int result = 0;
@@ -1107,6 +1155,9 @@ static int smu8_hwmgr_backend_init(struct pp_hwmgr *hwmgr)
 
 	hwmgr->platform_descriptor.hardwareActivityPerformanceLevels =  SMU8_MAX_HARDWARE_POWERLEVELS;
 
+	if (hwmgr->od_enabled)
+		smu8_init_od_settings(hwmgr);
+
 	return result;
 }
 
@@ -1510,7 +1561,8 @@ static int smu8_emit_clock_levels(struct pp_hwmgr *hwmgr,
 	struct smu8_hwmgr *data = hwmgr->backend;
 	struct phm_clock_voltage_dependency_table *sclk_table =
 			hwmgr->dyn_state.vddc_dependency_on_sclk;
-	uint32_t i, now;
+	const struct phm_odn_clock_levels *od = &data->od_edit_sclk;
+	uint32_t i, now, clk;
 	int size = *offset;
 
 	switch (type) {
@@ -1541,6 +1593,27 @@ static int smu8_emit_clock_levels(struct pp_hwmgr *hwmgr,
 				(SMU8_NUM_NBPMEMORYCLOCK - i == now) ? "*" :
 								       "");
 		break;
+	case OD_SCLK:
+		if (!hwmgr->od_enabled)
+			break;
+		size += sysfs_emit_at(buf, size, "%s:\n", "OD_SCLK");
+		for (i = 0; i < od->num_of_pl; i++)
+			size += sysfs_emit_at(buf, size, "%u: %10uMHz %10umV\n",
+					      i, od->entries[i].clock / 100,
+					      od->entries[i].vddc);
+		break;
+	case OD_RANGE:
+		if (!hwmgr->od_enabled)
+			break;
+		size += sysfs_emit_at(buf, size, "%s:\n", "OD_RANGE");
+		clk = data->od_default_sclk.entries[0].clock;
+		size += sysfs_emit_at(buf, size, "SCLK: %7uMHz", clk / 100);
+		clk = hwmgr->platform_descriptor.overdriveLimit.engineClock;
+		size += sysfs_emit_at(buf, size, " %10uMHz\n", clk / 100);
+		/* VDDC is the graphics (VDDGFX) rail of these APUs */
+		size += sysfs_emit_at(buf, size, "VDDC: %7umV %11umV\n",
+				      data->od_min_vddc, data->od_max_vddc);
+		break;
 	default:
 		break;
 	}
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.h b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.h
index 05a06083e1b8..490fcc352a8f 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.h
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.h
@@ -25,6 +25,7 @@
 #define _SMU8_HWMGR_H_
 
 #include "cgs_common.h"
+#include "hardwaremanager.h"
 #include "ppatomctrl.h"
 
 #define SMU8_NUM_NBPSTATES               4
@@ -306,6 +307,12 @@ struct smu8_hwmgr {
 
 	uint32_t max_sclk_level;
 	uint32_t num_of_clk_entries;
+
+	/* overdrive: the VBIOS levels, the user's edits, the VDDGFX range */
+	struct phm_odn_clock_levels od_default_sclk;
+	struct phm_odn_clock_levels od_edit_sclk;
+	uint32_t od_min_vddc;		/* in mV */
+	uint32_t od_max_vddc;		/* in mV */
 };
 
 #endif /* _SMU8_HWMGR_H_ */

-- 
2.56.0

[PATCH 3/5] drm/amd/pm: show the smu8 overdrive sclk table

[Joshua Horne via B4 Relay]

From: Joshua Horne <hornejd@etsu.edu>

Carrizo and Stoney upload up to eight graphics DPM levels to the SMU,
each with an engine clock, a VDDGFX voltage (GfxVid) and an NB voltage
(GnbVid) from the VBIOS dependency tables.

Prepare overdrive support for them in the pp_od_clk_voltage format of
smu7: keep the VBIOS levels and the user's edits in struct
phm_odn_clock_levels as smu7 does, and print the edits as OD_SCLK and
the allowed ranges as OD_RANGE. Only the levels that the SMU enables
(PPSMC_MSG_GetMaxSclkLevel) are used, as the VBIOS table can list
levels above the fused maximum of the part.

The engine clock may range from the lowest level up to the overdrive
limit of the VBIOS. Without a limit, the highest enabled level is used,
so only underclocking is possible, as smu7 and vega10 do since
commit 46defdd6fff7 ("drm/amd/pp: Allow underclocking when od table is
empty in vbios"). The voltage is limited to the range the VBIOS uses
for these levels and kept in mV as on smu7 and vega10, converted from
the VID with convert_to_vddc().

Overdrive stays disabled on these APUs until a later patch.

Signed-off-by: Joshua Horne <hornejd@etsu.edu>
---
 .../gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c    | 75 +++++++++++++++++++++-
 .../gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.h    |  7 ++
 2 files changed, 81 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
index 77ab96bae4b0..93a7e7e176f3 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
@@ -1080,6 +1080,54 @@ static int smu8_apply_state_adjust_rules(struct pp_hwmgr *hwmgr,
 	return 0;
 }
 
+static void smu8_init_od_settings(struct pp_hwmgr *hwmgr)
+{
+	struct smu8_hwmgr *data = hwmgr->backend;
+	struct phm_clock_voltage_dependency_table *vddc_table =
+				hwmgr->dyn_state.vddc_dependency_on_sclk;
+	struct phm_clock_voltage_dependency_table *vdd_gfx_table =
+				hwmgr->dyn_state.vdd_gfx_dependency_on_sclk;
+	struct phm_odn_clock_levels *od = &data->od_default_sclk;
+	struct amdgpu_device *adev = hwmgr->adev;
+	uint32_t i, mv;
+
+	if (!vddc_table || !vdd_gfx_table || !vddc_table->count ||
+	    vddc_table->count > ARRAY_SIZE(od->entries) ||
+	    vdd_gfx_table->count != vddc_table->count) {
+		dev_info(adev->dev,
+			 "OverDrive not supported by the sclk tables\n");
+		hwmgr->od_enabled = false;
+		return;
+	}
+
+	/*
+	 * Only expose the levels that the SMU enables: the VBIOS table can
+	 * list levels above the fused maximum of the part.
+	 */
+	od->num_of_pl = min(vddc_table->count, smu8_get_max_sclk_level(hwmgr));
+	data->od_min_vddc = U32_MAX;
+	data->od_max_vddc = 0;
+	for (i = 0; i < od->num_of_pl; i++) {
+		mv = convert_to_vddc((uint8_t)vdd_gfx_table->entries[i].v);
+		od->entries[i].clock = vddc_table->entries[i].clk;
+		od->entries[i].vddc = mv;
+		od->entries[i].enabled = true;
+		data->od_min_vddc = min(data->od_min_vddc, mv);
+		data->od_max_vddc = max(data->od_max_vddc, mv);
+	}
+
+	/*
+	 * Without an overdrive limit in the VBIOS only allow underclocking,
+	 * as smu7 and vega10 do. The voltages are limited to the range that
+	 * the VBIOS uses for these levels.
+	 */
+	if (!hwmgr->platform_descriptor.overdriveLimit.engineClock)
+		hwmgr->platform_descriptor.overdriveLimit.engineClock =
+			od->entries[od->num_of_pl - 1].clock;
+
+	data->od_edit_sclk = *od;
+}
+
 static int smu8_hwmgr_backend_init(struct pp_hwmgr *hwmgr)
 {
 	int result = 0;
@@ -1107,6 +1155,9 @@ static int smu8_hwmgr_backend_init(struct pp_hwmgr *hwmgr)
 
 	hwmgr->platform_descriptor.hardwareActivityPerformanceLevels =  SMU8_MAX_HARDWARE_POWERLEVELS;
 
+	if (hwmgr->od_enabled)
+		smu8_init_od_settings(hwmgr);
+
 	return result;
 }
 
@@ -1510,7 +1561,8 @@ static int smu8_emit_clock_levels(struct pp_hwmgr *hwmgr,
 	struct smu8_hwmgr *data = hwmgr->backend;
 	struct phm_clock_voltage_dependency_table *sclk_table =
 			hwmgr->dyn_state.vddc_dependency_on_sclk;
-	uint32_t i, now;
+	const struct phm_odn_clock_levels *od = &data->od_edit_sclk;
+	uint32_t i, now, clk;
 	int size = *offset;
 
 	switch (type) {
@@ -1541,6 +1593,27 @@ static int smu8_emit_clock_levels(struct pp_hwmgr *hwmgr,
 				(SMU8_NUM_NBPMEMORYCLOCK - i == now) ? "*" :
 								       "");
 		break;
+	case OD_SCLK:
+		if (!hwmgr->od_enabled)
+			break;
+		size += sysfs_emit_at(buf, size, "%s:\n", "OD_SCLK");
+		for (i = 0; i < od->num_of_pl; i++)
+			size += sysfs_emit_at(buf, size, "%u: %10uMHz %10umV\n",
+					      i, od->entries[i].clock / 100,
+					      od->entries[i].vddc);
+		break;
+	case OD_RANGE:
+		if (!hwmgr->od_enabled)
+			break;
+		size += sysfs_emit_at(buf, size, "%s:\n", "OD_RANGE");
+		clk = data->od_default_sclk.entries[0].clock;
+		size += sysfs_emit_at(buf, size, "SCLK: %7uMHz", clk / 100);
+		clk = hwmgr->platform_descriptor.overdriveLimit.engineClock;
+		size += sysfs_emit_at(buf, size, " %10uMHz\n", clk / 100);
+		/* VDDC is the graphics (VDDGFX) rail of these APUs */
+		size += sysfs_emit_at(buf, size, "VDDC: %7umV %11umV\n",
+				      data->od_min_vddc, data->od_max_vddc);
+		break;
 	default:
 		break;
 	}
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.h b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.h
index 05a06083e1b8..490fcc352a8f 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.h
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.h
@@ -25,6 +25,7 @@
 #define _SMU8_HWMGR_H_
 
 #include "cgs_common.h"
+#include "hardwaremanager.h"
 #include "ppatomctrl.h"
 
 #define SMU8_NUM_NBPSTATES               4
@@ -306,6 +307,12 @@ struct smu8_hwmgr {
 
 	uint32_t max_sclk_level;
 	uint32_t num_of_clk_entries;
+
+	/* overdrive: the VBIOS levels, the user's edits, the VDDGFX range */
+	struct phm_odn_clock_levels od_default_sclk;
+	struct phm_odn_clock_levels od_edit_sclk;
+	uint32_t od_min_vddc;		/* in mV */
+	uint32_t od_max_vddc;		/* in mV */
 };
 
 #endif /* _SMU8_HWMGR_H_ */

-- 
2.56.0

[PATCH 4/5] drm/amd/pm: allow editing the smu8 overdrive sclk table

[Joshua Horne]

Implement the "s <level> <MHz> <mV>", "r" and "c" commands of
pp_od_clk_voltage for Carrizo and Stoney.

The levels given in one write are checked against OD_RANGE and applied
all or none. A new clock is rounded to the one the engine PLL produces,
as reported by the ComputeMemoryEnginePLL table; a level written back
with the clock OD_SCLK shows keeps its exact clock. A voltage between two
VID steps is rounded up by convert_to_vid(). A commit is refused unless
the clocks increase from level to level over the whole table, in which
the levels that the SMU does not enable keep their VBIOS clocks.

As on smu7 and vega10, a commit only marks the table as pending. The
power state readjustment that follows it writes the clocks and VDDGFX
voltages into the dependency tables and uploads them through
smu8_upload_pptable_to_smu(), the path used at init and resume, so the
settings also survive suspend. The NB voltages (GnbVid) are not
changed. The power states refer to the levels by clock and are read by
amdgpu_dpm_get_sclk(), so update them as well, like
vega10_odn_update_power_state() does.

The SMU accepts the new table at runtime and switches to the new clock
and voltage of a level the next time it enters that level. No
powerplay backend forces a level change after a commit, so this one
does not either.

Signed-off-by: Joshua Horne <hornejd@etsu.edu>
---
 .../gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c    | 216 +++++++++++++++++++++
 .../gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.h    |   1 +
 2 files changed, 217 insertions(+)

diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
index 93a7e7e176f3..c6045234965c 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
@@ -860,10 +860,86 @@ static void smu8_populate_umdpstate_clocks(struct pp_hwmgr *hwmgr)
 	hwmgr->pstate_mclk_peak = 0;
 }
 
+static void smu8_od_update_power_state(struct pp_hwmgr *hwmgr,
+				       struct pp_power_state *ps)
+{
+	struct smu8_hwmgr *data = hwmgr->backend;
+	const struct phm_odn_clock_levels *od = &data->od_edit_sclk;
+	const struct phm_clock_voltage_dependency_table *old =
+				hwmgr->dyn_state.vddc_dependency_on_sclk;
+	struct smu8_power_state *smu8_ps;
+	struct smu8_power_level *level;
+	uint32_t i, j;
+
+	if (!ps)
+		return;
+
+	smu8_ps = cast_smu8_power_state(&ps->hardware);
+	if (!smu8_ps)
+		return;
+
+	for (i = 0; i < smu8_ps->level; i++) {
+		level = &smu8_ps->levels[i];
+		for (j = 0; j < od->num_of_pl; j++) {
+			if (level->engineClock == old->entries[j].clk) {
+				level->engineClock = od->entries[j].clock;
+				break;
+			}
+		}
+	}
+}
+
+static int smu8_od_upload_sclk_table(struct pp_hwmgr *hwmgr)
+{
+	struct smu8_hwmgr *data = hwmgr->backend;
+	const struct phm_odn_clock_levels *od = &data->od_edit_sclk;
+	struct phm_clock_voltage_dependency_table *vddc_table =
+				hwmgr->dyn_state.vddc_dependency_on_sclk;
+	struct phm_clock_voltage_dependency_table *vdd_gfx_table =
+				hwmgr->dyn_state.vdd_gfx_dependency_on_sclk;
+	struct pp_power_state *ps;
+	uint32_t i;
+
+	data->od_update_pending = false;
+
+	/* the power states refer to the levels by clock, update them first */
+	ps = hwmgr->ps;
+	for (i = 0; i < hwmgr->num_ps; i++) {
+		smu8_od_update_power_state(hwmgr, ps);
+		ps = (struct pp_power_state *)
+			((unsigned long)ps + hwmgr->ps_size);
+	}
+	smu8_od_update_power_state(hwmgr, hwmgr->current_ps);
+	smu8_od_update_power_state(hwmgr, hwmgr->request_ps);
+
+	/* the NB voltages (GnbVid) of the levels are left untouched */
+	for (i = 0; i < od->num_of_pl; i++) {
+		vddc_table->entries[i].clk = od->entries[i].clock;
+		vdd_gfx_table->entries[i].clk = od->entries[i].clock;
+		vdd_gfx_table->entries[i].v =
+			convert_to_vid(od->entries[i].vddc);
+	}
+
+	smu8_populate_umdpstate_clocks(hwmgr);
+
+	/*
+	 * The SMU switches to the new clock and voltage of a level the next
+	 * time it enters that level.
+	 */
+	return smu8_upload_pptable_to_smu(hwmgr);
+}
+
 static int smu8_set_power_state_tasks(struct pp_hwmgr *hwmgr, const void *input)
 {
+	struct smu8_hwmgr *data = hwmgr->backend;
 	int ret = 0;
 
+	if (data->od_update_pending) {
+		ret = smu8_od_upload_sclk_table(hwmgr);
+		if (ret)
+			return ret;
+	}
+
 	smu8_update_sclk_limit(hwmgr);
 	smu8_set_deep_sleep_sclk_threshold(hwmgr);
 	smu8_set_watermark_threshold(hwmgr);
@@ -2057,6 +2133,145 @@ static void smu8_dpm_powergate_vce(struct pp_hwmgr *hwmgr, bool bgate)
 	}
 }
 
+/* the engine clock that the PLL produces for @clk */
+static uint32_t smu8_od_real_sclk(struct pp_hwmgr *hwmgr, uint32_t clk)
+{
+	pp_atomctrl_clock_dividers_kong dividers;
+
+	if (atomctrl_get_engine_pll_dividers_kong(hwmgr, clk, &dividers) ||
+	    !dividers.real_clock)
+		return clk;
+
+	return dividers.real_clock;
+}
+
+static int smu8_od_edit_sclk_level(struct pp_hwmgr *hwmgr,
+				   struct phm_odn_clock_levels *od,
+				   long level, long mhz, long mv)
+{
+	struct smu8_hwmgr *data = hwmgr->backend;
+	struct amdgpu_device *adev = hwmgr->adev;
+	uint32_t min_clk, max_clk, clk;
+
+	min_clk = data->od_default_sclk.entries[0].clock;
+	max_clk = hwmgr->platform_descriptor.overdriveLimit.engineClock;
+
+	if (level < 0 || level >= od->num_of_pl) {
+		dev_info(adev->dev,
+			 "OD sclk level %ld is out of range [0 - %u]\n",
+			 level, od->num_of_pl - 1);
+		return -EINVAL;
+	}
+
+	if (mhz < min_clk / 100 || mhz > max_clk / 100) {
+		dev_info(adev->dev,
+			 "OD engine clock is out of range [%u - %u] MHz\n",
+			 min_clk / 100, max_clk / 100);
+		return -EINVAL;
+	}
+
+	if (mv < data->od_min_vddc || mv > data->od_max_vddc) {
+		dev_info(adev->dev, "OD voltage is out of range [%u - %u] mV\n",
+			 data->od_min_vddc, data->od_max_vddc);
+		return -EINVAL;
+	}
+
+	/*
+	 * OD_SCLK shows whole MHz, so keep the exact clock of a level that is
+	 * written back unchanged. Otherwise use the clock the PLL produces.
+	 */
+	clk = mhz * 100;
+	if (clk == od->entries[level].clock / 100 * 100)
+		clk = od->entries[level].clock;
+	else
+		clk = smu8_od_real_sclk(hwmgr, clk);
+
+	if (clk < min_clk || clk > max_clk) {
+		dev_info(adev->dev,
+			 "OD engine clock %u.%02u MHz is out of range\n",
+			 clk / 100, clk % 100);
+		return -EINVAL;
+	}
+
+	od->entries[level].clock = clk;
+	/* a voltage between two VID steps is rounded up */
+	od->entries[level].vddc = convert_to_vddc(convert_to_vid(mv));
+
+	return 0;
+}
+
+static int smu8_od_commit_sclk_table(struct pp_hwmgr *hwmgr)
+{
+	struct smu8_hwmgr *data = hwmgr->backend;
+	struct phm_odn_clock_levels *od = &data->od_edit_sclk;
+	struct phm_clock_voltage_dependency_table *table =
+				hwmgr->dyn_state.vddc_dependency_on_sclk;
+	struct amdgpu_device *adev = hwmgr->adev;
+	uint32_t i, clk, prev = 0;
+
+	/* the levels that the SMU does not enable keep their clocks */
+	for (i = 0; i < table->count; i++) {
+		clk = i < od->num_of_pl ? od->entries[i].clock :
+					  table->entries[i].clk;
+		if (clk <= prev) {
+			dev_info(adev->dev, "OD sclk levels must increase\n");
+			return -EINVAL;
+		}
+		prev = clk;
+	}
+
+	/* uploaded by the power state readjustment that follows a commit */
+	data->od_update_pending = true;
+
+	return 0;
+}
+
+static int smu8_odn_edit_dpm_table(struct pp_hwmgr *hwmgr,
+				   enum PP_OD_DPM_TABLE_COMMAND type,
+				   long *input, uint32_t size)
+{
+	struct smu8_hwmgr *data = hwmgr->backend;
+	struct amdgpu_device *adev = hwmgr->adev;
+	struct phm_odn_clock_levels od;
+	uint32_t i;
+	int ret;
+
+	if (!hwmgr->od_enabled) {
+		dev_info(adev->dev, "OverDrive feature not enabled\n");
+		return -EINVAL;
+	}
+
+	switch (type) {
+	case PP_OD_EDIT_SCLK_VDDC_TABLE:
+		break;
+	case PP_OD_RESTORE_DEFAULT_TABLE:
+		data->od_edit_sclk = data->od_default_sclk;
+		return 0;
+	case PP_OD_COMMIT_DPM_TABLE:
+		return smu8_od_commit_sclk_table(hwmgr);
+	default:
+		return -EINVAL;
+	}
+
+	if (!input || !size || size % 3) {
+		dev_info(adev->dev,
+			 "OD sclk input must be <level> <MHz> <mV>\n");
+		return -EINVAL;
+	}
+
+	/* apply all of the given levels or none of them */
+	od = data->od_edit_sclk;
+	for (i = 0; i < size; i += 3) {
+		ret = smu8_od_edit_sclk_level(hwmgr, &od, input[i],
+					      input[i + 1], input[i + 2]);
+		if (ret)
+			return ret;
+	}
+	data->od_edit_sclk = od;
+
+	return 0;
+}
+
 static const struct pp_hwmgr_func smu8_hwmgr_funcs = {
 	.backend_init = smu8_hwmgr_backend_init,
 	.backend_fini = smu8_hwmgr_backend_fini,
@@ -2087,6 +2302,7 @@ static const struct pp_hwmgr_func smu8_hwmgr_funcs = {
 	.dynamic_state_management_disable = smu8_disable_dpm_tasks,
 	.notify_cac_buffer_info = smu8_notify_cac_buffer_info,
 	.get_thermal_temperature_range = smu8_get_thermal_temperature_range,
+	.odn_edit_dpm_table = smu8_odn_edit_dpm_table,
 };
 
 int smu8_init_function_pointers(struct pp_hwmgr *hwmgr)
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.h b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.h
index 490fcc352a8f..63ae7826a2da 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.h
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.h
@@ -313,6 +313,7 @@ struct smu8_hwmgr {
 	struct phm_odn_clock_levels od_edit_sclk;
 	uint32_t od_min_vddc;		/* in mV */
 	uint32_t od_max_vddc;		/* in mV */
+	bool od_update_pending;		/* committed, not uploaded yet */
 };
 
 #endif /* _SMU8_HWMGR_H_ */

-- 
2.56.0

[PATCH 4/5] drm/amd/pm: allow editing the smu8 overdrive sclk table

[Joshua Horne via B4 Relay]

From: Joshua Horne <hornejd@etsu.edu>

Implement the "s <level> <MHz> <mV>", "r" and "c" commands of
pp_od_clk_voltage for Carrizo and Stoney.

The levels given in one write are checked against OD_RANGE and applied
all or none. A new clock is rounded to the one the engine PLL produces,
as reported by the ComputeMemoryEnginePLL table; a level written back
with the clock OD_SCLK shows keeps its exact clock. A voltage between two
VID steps is rounded up by convert_to_vid(). A commit is refused unless
the clocks increase from level to level over the whole table, in which
the levels that the SMU does not enable keep their VBIOS clocks.

As on smu7 and vega10, a commit only marks the table as pending. The
power state readjustment that follows it writes the clocks and VDDGFX
voltages into the dependency tables and uploads them through
smu8_upload_pptable_to_smu(), the path used at init and resume, so the
settings also survive suspend. The NB voltages (GnbVid) are not
changed. The power states refer to the levels by clock and are read by
amdgpu_dpm_get_sclk(), so update them as well, like
vega10_odn_update_power_state() does.

The SMU accepts the new table at runtime and switches to the new clock
and voltage of a level the next time it enters that level. No
powerplay backend forces a level change after a commit, so this one
does not either.

Signed-off-by: Joshua Horne <hornejd@etsu.edu>
---
 .../gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c    | 216 +++++++++++++++++++++
 .../gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.h    |   1 +
 2 files changed, 217 insertions(+)

diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
index 93a7e7e176f3..c6045234965c 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
@@ -860,10 +860,86 @@ static void smu8_populate_umdpstate_clocks(struct pp_hwmgr *hwmgr)
 	hwmgr->pstate_mclk_peak = 0;
 }
 
+static void smu8_od_update_power_state(struct pp_hwmgr *hwmgr,
+				       struct pp_power_state *ps)
+{
+	struct smu8_hwmgr *data = hwmgr->backend;
+	const struct phm_odn_clock_levels *od = &data->od_edit_sclk;
+	const struct phm_clock_voltage_dependency_table *old =
+				hwmgr->dyn_state.vddc_dependency_on_sclk;
+	struct smu8_power_state *smu8_ps;
+	struct smu8_power_level *level;
+	uint32_t i, j;
+
+	if (!ps)
+		return;
+
+	smu8_ps = cast_smu8_power_state(&ps->hardware);
+	if (!smu8_ps)
+		return;
+
+	for (i = 0; i < smu8_ps->level; i++) {
+		level = &smu8_ps->levels[i];
+		for (j = 0; j < od->num_of_pl; j++) {
+			if (level->engineClock == old->entries[j].clk) {
+				level->engineClock = od->entries[j].clock;
+				break;
+			}
+		}
+	}
+}
+
+static int smu8_od_upload_sclk_table(struct pp_hwmgr *hwmgr)
+{
+	struct smu8_hwmgr *data = hwmgr->backend;
+	const struct phm_odn_clock_levels *od = &data->od_edit_sclk;
+	struct phm_clock_voltage_dependency_table *vddc_table =
+				hwmgr->dyn_state.vddc_dependency_on_sclk;
+	struct phm_clock_voltage_dependency_table *vdd_gfx_table =
+				hwmgr->dyn_state.vdd_gfx_dependency_on_sclk;
+	struct pp_power_state *ps;
+	uint32_t i;
+
+	data->od_update_pending = false;
+
+	/* the power states refer to the levels by clock, update them first */
+	ps = hwmgr->ps;
+	for (i = 0; i < hwmgr->num_ps; i++) {
+		smu8_od_update_power_state(hwmgr, ps);
+		ps = (struct pp_power_state *)
+			((unsigned long)ps + hwmgr->ps_size);
+	}
+	smu8_od_update_power_state(hwmgr, hwmgr->current_ps);
+	smu8_od_update_power_state(hwmgr, hwmgr->request_ps);
+
+	/* the NB voltages (GnbVid) of the levels are left untouched */
+	for (i = 0; i < od->num_of_pl; i++) {
+		vddc_table->entries[i].clk = od->entries[i].clock;
+		vdd_gfx_table->entries[i].clk = od->entries[i].clock;
+		vdd_gfx_table->entries[i].v =
+			convert_to_vid(od->entries[i].vddc);
+	}
+
+	smu8_populate_umdpstate_clocks(hwmgr);
+
+	/*
+	 * The SMU switches to the new clock and voltage of a level the next
+	 * time it enters that level.
+	 */
+	return smu8_upload_pptable_to_smu(hwmgr);
+}
+
 static int smu8_set_power_state_tasks(struct pp_hwmgr *hwmgr, const void *input)
 {
+	struct smu8_hwmgr *data = hwmgr->backend;
 	int ret = 0;
 
+	if (data->od_update_pending) {
+		ret = smu8_od_upload_sclk_table(hwmgr);
+		if (ret)
+			return ret;
+	}
+
 	smu8_update_sclk_limit(hwmgr);
 	smu8_set_deep_sleep_sclk_threshold(hwmgr);
 	smu8_set_watermark_threshold(hwmgr);
@@ -2057,6 +2133,145 @@ static void smu8_dpm_powergate_vce(struct pp_hwmgr *hwmgr, bool bgate)
 	}
 }
 
+/* the engine clock that the PLL produces for @clk */
+static uint32_t smu8_od_real_sclk(struct pp_hwmgr *hwmgr, uint32_t clk)
+{
+	pp_atomctrl_clock_dividers_kong dividers;
+
+	if (atomctrl_get_engine_pll_dividers_kong(hwmgr, clk, &dividers) ||
+	    !dividers.real_clock)
+		return clk;
+
+	return dividers.real_clock;
+}
+
+static int smu8_od_edit_sclk_level(struct pp_hwmgr *hwmgr,
+				   struct phm_odn_clock_levels *od,
+				   long level, long mhz, long mv)
+{
+	struct smu8_hwmgr *data = hwmgr->backend;
+	struct amdgpu_device *adev = hwmgr->adev;
+	uint32_t min_clk, max_clk, clk;
+
+	min_clk = data->od_default_sclk.entries[0].clock;
+	max_clk = hwmgr->platform_descriptor.overdriveLimit.engineClock;
+
+	if (level < 0 || level >= od->num_of_pl) {
+		dev_info(adev->dev,
+			 "OD sclk level %ld is out of range [0 - %u]\n",
+			 level, od->num_of_pl - 1);
+		return -EINVAL;
+	}
+
+	if (mhz < min_clk / 100 || mhz > max_clk / 100) {
+		dev_info(adev->dev,
+			 "OD engine clock is out of range [%u - %u] MHz\n",
+			 min_clk / 100, max_clk / 100);
+		return -EINVAL;
+	}
+
+	if (mv < data->od_min_vddc || mv > data->od_max_vddc) {
+		dev_info(adev->dev, "OD voltage is out of range [%u - %u] mV\n",
+			 data->od_min_vddc, data->od_max_vddc);
+		return -EINVAL;
+	}
+
+	/*
+	 * OD_SCLK shows whole MHz, so keep the exact clock of a level that is
+	 * written back unchanged. Otherwise use the clock the PLL produces.
+	 */
+	clk = mhz * 100;
+	if (clk == od->entries[level].clock / 100 * 100)
+		clk = od->entries[level].clock;
+	else
+		clk = smu8_od_real_sclk(hwmgr, clk);
+
+	if (clk < min_clk || clk > max_clk) {
+		dev_info(adev->dev,
+			 "OD engine clock %u.%02u MHz is out of range\n",
+			 clk / 100, clk % 100);
+		return -EINVAL;
+	}
+
+	od->entries[level].clock = clk;
+	/* a voltage between two VID steps is rounded up */
+	od->entries[level].vddc = convert_to_vddc(convert_to_vid(mv));
+
+	return 0;
+}
+
+static int smu8_od_commit_sclk_table(struct pp_hwmgr *hwmgr)
+{
+	struct smu8_hwmgr *data = hwmgr->backend;
+	struct phm_odn_clock_levels *od = &data->od_edit_sclk;
+	struct phm_clock_voltage_dependency_table *table =
+				hwmgr->dyn_state.vddc_dependency_on_sclk;
+	struct amdgpu_device *adev = hwmgr->adev;
+	uint32_t i, clk, prev = 0;
+
+	/* the levels that the SMU does not enable keep their clocks */
+	for (i = 0; i < table->count; i++) {
+		clk = i < od->num_of_pl ? od->entries[i].clock :
+					  table->entries[i].clk;
+		if (clk <= prev) {
+			dev_info(adev->dev, "OD sclk levels must increase\n");
+			return -EINVAL;
+		}
+		prev = clk;
+	}
+
+	/* uploaded by the power state readjustment that follows a commit */
+	data->od_update_pending = true;
+
+	return 0;
+}
+
+static int smu8_odn_edit_dpm_table(struct pp_hwmgr *hwmgr,
+				   enum PP_OD_DPM_TABLE_COMMAND type,
+				   long *input, uint32_t size)
+{
+	struct smu8_hwmgr *data = hwmgr->backend;
+	struct amdgpu_device *adev = hwmgr->adev;
+	struct phm_odn_clock_levels od;
+	uint32_t i;
+	int ret;
+
+	if (!hwmgr->od_enabled) {
+		dev_info(adev->dev, "OverDrive feature not enabled\n");
+		return -EINVAL;
+	}
+
+	switch (type) {
+	case PP_OD_EDIT_SCLK_VDDC_TABLE:
+		break;
+	case PP_OD_RESTORE_DEFAULT_TABLE:
+		data->od_edit_sclk = data->od_default_sclk;
+		return 0;
+	case PP_OD_COMMIT_DPM_TABLE:
+		return smu8_od_commit_sclk_table(hwmgr);
+	default:
+		return -EINVAL;
+	}
+
+	if (!input || !size || size % 3) {
+		dev_info(adev->dev,
+			 "OD sclk input must be <level> <MHz> <mV>\n");
+		return -EINVAL;
+	}
+
+	/* apply all of the given levels or none of them */
+	od = data->od_edit_sclk;
+	for (i = 0; i < size; i += 3) {
+		ret = smu8_od_edit_sclk_level(hwmgr, &od, input[i],
+					      input[i + 1], input[i + 2]);
+		if (ret)
+			return ret;
+	}
+	data->od_edit_sclk = od;
+
+	return 0;
+}
+
 static const struct pp_hwmgr_func smu8_hwmgr_funcs = {
 	.backend_init = smu8_hwmgr_backend_init,
 	.backend_fini = smu8_hwmgr_backend_fini,
@@ -2087,6 +2302,7 @@ static const struct pp_hwmgr_func smu8_hwmgr_funcs = {
 	.dynamic_state_management_disable = smu8_disable_dpm_tasks,
 	.notify_cac_buffer_info = smu8_notify_cac_buffer_info,
 	.get_thermal_temperature_range = smu8_get_thermal_temperature_range,
+	.odn_edit_dpm_table = smu8_odn_edit_dpm_table,
 };
 
 int smu8_init_function_pointers(struct pp_hwmgr *hwmgr)
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.h b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.h
index 490fcc352a8f..63ae7826a2da 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.h
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.h
@@ -313,6 +313,7 @@ struct smu8_hwmgr {
 	struct phm_odn_clock_levels od_edit_sclk;
 	uint32_t od_min_vddc;		/* in mV */
 	uint32_t od_max_vddc;		/* in mV */
+	bool od_update_pending;		/* committed, not uploaded yet */
 };
 
 #endif /* _SMU8_HWMGR_H_ */

-- 
2.56.0

[PATCH 5/5] drm/amd/pm: stop disabling overdrive on Carrizo and Stoney

[Joshua Horne]

Commit b7e919b9403f ("drm/amd/pp: Disable OD feature on APU/Iceland")
disabled overdrive on these APUs, which the smu8 backend did not
implement then. Now that it does, let the PP_OVERDRIVE_MASK bit of
amdgpu.ppfeaturemask decide, as on Tonga and newer dGPUs. The bit is
clear by default and taints the kernel when set.

Raven, disabled by the same commit, gained overdrive support with
commit 12a6727dee5d ("drm/amd/powerplay: add one sysfs file to support
the feature to modify gfx clock on Raven/Raven2/Picasso APU.").

Document the pp_od_clk_voltage format for Carrizo and Stoney.

Signed-off-by: Joshua Horne <hornejd@etsu.edu>
---
 drivers/gpu/drm/amd/pm/amdgpu_pm.c             | 6 ++++++
 drivers/gpu/drm/amd/pm/powerplay/hwmgr/hwmgr.c | 1 -
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index 48c52ac0fc77..aeee31cc08c9 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -705,6 +705,12 @@ static ssize_t amdgpu_set_pp_table(struct device *dev,
  * "c" (commit) to the file to commit your changes.  If you want to reset to the
  * default power levels, write "r" (reset) to the file to reset them.
  *
+ * < For Carrizo and Stoney APUs >
+ *
+ * As for Vega10 and previous ASICs, without OD_MCLK. The voltages are
+ * those of the graphics (VDDGFX) rail. A level switches to its new clock
+ * and voltage the next time the GPU enters that level.
+ *
  *
  * < For Vega20 and newer ASICs >
  *
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/hwmgr.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/hwmgr.c
index 4d553be56396..1567b8fa2a6b 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/hwmgr.c
@@ -120,7 +120,6 @@ int hwmgr_early_init(struct pp_hwmgr *hwmgr)
 		break;
 	case AMDGPU_FAMILY_CZ:
 		adev->pm.pp_feature &= ~PP_GFXOFF_MASK;
-		hwmgr->od_enabled = false;
 		hwmgr->smumgr_funcs = &smu8_smu_funcs;
 		hwmgr->feature_mask &= ~PP_GFXOFF_MASK;
 		smu8_init_function_pointers(hwmgr);

-- 
2.56.0

[PATCH 5/5] drm/amd/pm: stop disabling overdrive on Carrizo and Stoney

[Joshua Horne via B4 Relay]

From: Joshua Horne <hornejd@etsu.edu>

Commit b7e919b9403f ("drm/amd/pp: Disable OD feature on APU/Iceland")
disabled overdrive on these APUs, which the smu8 backend did not
implement then. Now that it does, let the PP_OVERDRIVE_MASK bit of
amdgpu.ppfeaturemask decide, as on Tonga and newer dGPUs. The bit is
clear by default and taints the kernel when set.

Raven, disabled by the same commit, gained overdrive support with
commit 12a6727dee5d ("drm/amd/powerplay: add one sysfs file to support
the feature to modify gfx clock on Raven/Raven2/Picasso APU.").

Document the pp_od_clk_voltage format for Carrizo and Stoney.

Signed-off-by: Joshua Horne <hornejd@etsu.edu>
---
 drivers/gpu/drm/amd/pm/amdgpu_pm.c             | 6 ++++++
 drivers/gpu/drm/amd/pm/powerplay/hwmgr/hwmgr.c | 1 -
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index 48c52ac0fc77..aeee31cc08c9 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -705,6 +705,12 @@ static ssize_t amdgpu_set_pp_table(struct device *dev,
  * "c" (commit) to the file to commit your changes.  If you want to reset to the
  * default power levels, write "r" (reset) to the file to reset them.
  *
+ * < For Carrizo and Stoney APUs >
+ *
+ * As for Vega10 and previous ASICs, without OD_MCLK. The voltages are
+ * those of the graphics (VDDGFX) rail. A level switches to its new clock
+ * and voltage the next time the GPU enters that level.
+ *
  *
  * < For Vega20 and newer ASICs >
  *
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/hwmgr.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/hwmgr.c
index 4d553be56396..1567b8fa2a6b 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/hwmgr.c
@@ -120,7 +120,6 @@ int hwmgr_early_init(struct pp_hwmgr *hwmgr)
 		break;
 	case AMDGPU_FAMILY_CZ:
 		adev->pm.pp_feature &= ~PP_GFXOFF_MASK;
-		hwmgr->od_enabled = false;
 		hwmgr->smumgr_funcs = &smu8_smu_funcs;
 		hwmgr->feature_mask &= ~PP_GFXOFF_MASK;
 		smu8_init_function_pointers(hwmgr);

-- 
2.56.0

Re: [PATCH 4/5] drm/amd/pm: allow editing the smu8 overdrive sclk table

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] drm/amd/pm: stale SclkHardMin level after
  smu8_od_upload_sclk_table()
- [Medium] drm/amd/pm: fractional boundary clocks rejected in
  smu8_od_edit_sclk_level()
--

commit f271cff6b522292ee10f53b957a90f73690433ee
Author: Joshua Horne <hornejd@etsu.edu>

drm/amd/pm: allow editing the smu8 overdrive sclk table

This commit implements pp_od_clk_voltage editing, reset, and commit
commands for the SMU8 SCLK OverDrive table on Carrizo and Stoney.

> diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
> index 93a7e7e176f37..c6045234965c0 100644
> --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
> +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
> @@ -860,10 +860,86 @@ static void smu8_populate_umdpstate_clocks(struct pp_hwmgr *hwmgr)

[ ... ]

> +static int smu8_od_upload_sclk_table(struct pp_hwmgr *hwmgr)
> +{
> +	struct smu8_hwmgr *data = hwmgr->backend;
> +	const struct phm_odn_clock_levels *od = &data->od_edit_sclk;
> +	struct phm_clock_voltage_dependency_table *vddc_table =
> +				hwmgr->dyn_state.vddc_dependency_on_sclk;
> +	struct phm_clock_voltage_dependency_table *vdd_gfx_table =
> +				hwmgr->dyn_state.vdd_gfx_dependency_on_sclk;
> +	struct pp_power_state *ps;
> +	uint32_t i;
> +
> +	data->od_update_pending = false;
> +
> +	/* the power states refer to the levels by clock, update them first */
> +	ps = hwmgr->ps;
> +	for (i = 0; i < hwmgr->num_ps; i++) {
> +		smu8_od_update_power_state(hwmgr, ps);
> +		ps = (struct pp_power_state *)
> +			((unsigned long)ps + hwmgr->ps_size);
> +	}
> +	smu8_od_update_power_state(hwmgr, hwmgr->current_ps);
> +	smu8_od_update_power_state(hwmgr, hwmgr->request_ps);
> +
> +	/* the NB voltages (GnbVid) of the levels are left untouched */
> +	for (i = 0; i < od->num_of_pl; i++) {
> +		vddc_table->entries[i].clk = od->entries[i].clock;
> +		vdd_gfx_table->entries[i].clk = od->entries[i].clock;
> +		vdd_gfx_table->entries[i].v =
> +			convert_to_vid(od->entries[i].vddc);
> +	}
> +
> +	smu8_populate_umdpstate_clocks(hwmgr);
> +
> +	/*
> +	 * The SMU switches to the new clock and voltage of a level the next
> +	 * time it enters that level.
> +	 */
> +	return smu8_upload_pptable_to_smu(hwmgr);
> +}
> +
>  static int smu8_set_power_state_tasks(struct pp_hwmgr *hwmgr, const void *input)
>  {
> +	struct smu8_hwmgr *data = hwmgr->backend;
>  	int ret = 0;
>  
> +	if (data->od_update_pending) {
> +		ret = smu8_od_upload_sclk_table(hwmgr);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	smu8_update_sclk_limit(hwmgr);

[Severity: Medium]
Does smu8_update_sclk_limit() skip updating PPSMC_MSG_SetSclkHardMin on
the SMU after smu8_od_upload_sclk_table() modifies
vddc_dependency_on_sclk?

smu8_update_sclk_limit() resolves hwmgr->display_config->min_core_set_clock
against vddc_dependency_on_sclk via smu8_get_sclk_level(), but only sends
PPSMC_MSG_SetSclkHardMin when data->sclk_dpm.hard_min_clk differs from
min_core_set_clock:

drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c:smu8_update_sclk_limit() {
	...
	clock = hwmgr->display_config->min_core_set_clock;
	if (clock == 0)
		pr_debug("min_core_set_clock not set\n");

	if (data->sclk_dpm.hard_min_clk != clock) {
		data->sclk_dpm.hard_min_clk = clock;

		smum_send_msg_to_smc_with_parameter(hwmgr,
						PPSMC_MSG_SetSclkHardMin,
						 smu8_get_sclk_level(hwmgr,
					data->sclk_dpm.hard_min_clk,
					     PPSMC_MSG_SetSclkHardMin),
						 NULL);
	}
	...
}

When committing an OverDrive edit while min_core_set_clock is unchanged,
data->sclk_dpm.hard_min_clk already equals clock, so the SMU keeps the old
DPM level index even if the edited frequencies shift which level satisfies
min_core_set_clock, allowing SCLK to drop below the display minimum or
staying pinned higher than needed.  Invalidating
data->sclk_dpm.hard_min_clk in smu8_od_upload_sclk_table() would force
smu8_update_sclk_limit() to resend the updated level.

[ ... ]

> @@ -2057,6 +2133,145 @@ static void smu8_dpm_powergate_vce(struct pp_hwmgr *hwmgr, bool bgate)
>  	}
>  }
>  
> +/* the engine clock that the PLL produces for @clk */
> +static uint32_t smu8_od_real_sclk(struct pp_hwmgr *hwmgr, uint32_t clk)
> +{
> +	pp_atomctrl_clock_dividers_kong dividers;
> +
> +	if (atomctrl_get_engine_pll_dividers_kong(hwmgr, clk, &dividers) ||
> +	    !dividers.real_clock)
> +		return clk;
> +
> +	return dividers.real_clock;
> +}
> +
> +static int smu8_od_edit_sclk_level(struct pp_hwmgr *hwmgr,
> +				   struct phm_odn_clock_levels *od,
> +				   long level, long mhz, long mv)
> +{
> +	struct smu8_hwmgr *data = hwmgr->backend;
> +	struct amdgpu_device *adev = hwmgr->adev;
> +	uint32_t min_clk, max_clk, clk;
> +
> +	min_clk = data->od_default_sclk.entries[0].clock;
> +	max_clk = hwmgr->platform_descriptor.overdriveLimit.engineClock;
> +
> +	if (level < 0 || level >= od->num_of_pl) {
> +		dev_info(adev->dev,
> +			 "OD sclk level %ld is out of range [0 - %u]\n",
> +			 level, od->num_of_pl - 1);
> +		return -EINVAL;
> +	}
> +
> +	if (mhz < min_clk / 100 || mhz > max_clk / 100) {
> +		dev_info(adev->dev,
> +			 "OD engine clock is out of range [%u - %u] MHz\n",
> +			 min_clk / 100, max_clk / 100);
> +		return -EINVAL;
> +	}
> +
> +	if (mv < data->od_min_vddc || mv > data->od_max_vddc) {
> +		dev_info(adev->dev, "OD voltage is out of range [%u - %u] mV\n",
> +			 data->od_min_vddc, data->od_max_vddc);
> +		return -EINVAL;
> +	}
> +
> +	/*
> +	 * OD_SCLK shows whole MHz, so keep the exact clock of a level that is
> +	 * written back unchanged. Otherwise use the clock the PLL produces.
> +	 */
> +	clk = mhz * 100;
> +	if (clk == od->entries[level].clock / 100 * 100)
> +		clk = od->entries[level].clock;
> +	else
> +		clk = smu8_od_real_sclk(hwmgr, clk);
> +
> +	if (clk < min_clk || clk > max_clk) {
> +		dev_info(adev->dev,
> +			 "OD engine clock %u.%02u MHz is out of range\n",
> +			 clk / 100, clk % 100);
> +		return -EINVAL;
> +	}

[Severity: Medium]
Can smu8_od_edit_sclk_level() reject the minimum MHz shown in OD_RANGE when
min_clk has a non-zero fractional MHz part (min_clk % 100 != 0)?

On Carrizo and Stoney, VBIOS clocks are stored in 10 kHz units and often
have fractional MHz values (such as 30476 for 304.76 MHz), while OD_RANGE
and OD_SCLK print truncated whole MHz (min_clk / 100).  Once level 0 is
edited to another frequency, writing back mhz = min_clk / 100 no longer
matches od->entries[level].clock / 100 * 100, so clk = (min_clk / 100) * 100
is passed to smu8_od_real_sclk().  Because that input is already strictly
less than min_clk and ComputeMemoryEnginePLL rounds down to the nearest PLL
step, the resulting clk is below min_clk and fails the clk < min_clk check
with -EINVAL.  Similarly, when max_clk % 100 != 0, writing max_clk / 100
after an edit passes (max_clk / 100) * 100 < max_clk to smu8_od_real_sclk()
and cannot reach the max_clk PLL step.

Checking data->od_default_sclk.entries[level].clock (as well as min_clk and
max_clk) for a whole-MHz match before calling smu8_od_real_sclk() would
allow restoring a level's default or boundary clock.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-smu8-overdrive-v1-0-763f07bdfca9@etsu.edu?part=4