AMD General
------------------------------------------------------------------------
*From:* Lazar, Lijo <[email protected]>
*Sent:* Monday, July 27, 2026 5:20 PM
*To:* Milind Sardeshpande, Shubhankar
<[email protected]>; Deucher, Alexander
<[email protected]>; Vishwakarma, Pratik
<[email protected]>; Koenig, Christian
<[email protected]>; [email protected] <amd-
[email protected]>
*Cc:* [email protected]
<[email protected]>; Gopalakrishnan, Veerabadhran
(Veera) <[email protected]>; Guttula, Suresh
<[email protected]>; Ramalingeswara Reddy, Kanala
<[email protected]>
*Subject:* Re: [PATCH V4 2/2] drm/amd/pm/smu15: switch SMU v15.0.0 to
DRAM-based accumulator metrics
On 27-Jul-26 4:18 PM, Shubhankar Milind Sardeshpande wrote:
> Replace the legacy SMU table-copy metrics path with a DRAM-mapped,
> accumulator-based approach for SMU v15.0.0, using a ping-pong buffer
> to compute averaged metrics from deltas between consecutive samples.
>
> - Add GetMetricsTableVersion, GetMetricsTableLogSample and
> GetMetricsTableLogDramAddr messages and their MSG_MAP entries.
> - Introduce SMU_15_0_0_MetricsInfo_t holding two MetricsTable_t
> buffers, the mapped DRAM address, table size and pre-computed
> avg_metric[] values.
> - Resolve and ioremap_wc() the firmware log DRAM address once in
> init_smc_tables(); iounmap() it in fini_smc_tables().
> - Fetch samples into the inactive buffer, compute averaged clocks,
> activity, power and temperature via wrapping_sub() accumulator
> deltas, then swap the active index.
> - Rename the old get_metrics_table() to get_gpu_metrics_table() for
> the gpu_metrics v3.0 export path.
> - Move AMDGPU_PP_SENSOR_SS_APU_SHARE, AMDGPU_PP_SENSOR_SS_DGPU_SHARE
> to default case since they are not supported.
>
> Co-developed-by: Pratik Vishwakarma <[email protected]>
> Signed-off-by: Pratik Vishwakarma <[email protected]>
> Co-developed-by: Suresh Guttula <[email protected]>
> Signed-off-by: Suresh Guttula <[email protected]>
> Co-developed-by: Kanala Ramalingeswara Reddy
<[email protected]>
> Signed-off-by: Kanala Ramalingeswara Reddy
<[email protected]>
> Signed-off-by: Shubhankar Milind Sardeshpande
<[email protected]>
Nit comment below. With that changed -
Reviewed-by: Lijo Lazar <[email protected]>
> ---
> .../pm/swsmu/inc/pmfw_if/smu_v15_0_0_ppsmc.h | 5 +-
> drivers/gpu/drm/amd/pm/swsmu/inc/smu_types.h | 3 +
> .../drm/amd/pm/swsmu/smu15/smu_v15_0_0_ppt.c | 291 +++++++++++-------
> .../drm/amd/pm/swsmu/smu15/smu_v15_0_0_ppt.h | 13 +
> 4 files changed, 197 insertions(+), 115 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/
smu_v15_0_0_ppsmc.h b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/
smu_v15_0_0_ppsmc.h
> index bf216f83e689..9f627b5fef5b 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu_v15_0_0_ppsmc.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu_v15_0_0_ppsmc.h
> @@ -100,7 +100,10 @@
> #define PPSMC_MSG_DisableLSdma 0x1F ///< Disable LSDMA
> #define PPSMC_MSG_SetSoftMaxVpe 0x20 ///<
> #define PPSMC_MSG_SetSoftMinVpe 0x21 ///<
> -#define PPSMC_Message_Count 0x22 ///< Total
number of PPSMC messages
> +#define PPSMC_MSG_GetMetricsTableVersion 0x22
> +#define PPSMC_MSG_GetMetricsTableLogSample 0x23
> +#define PPSMC_MSG_GetMetricsTableLogDramAddr 0x24
> +#define PPSMC_Message_Count 0x25 ///< Total
number of PPSMC messages
> /** @}*/
>
> /**
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/smu_types.h b/drivers/
gpu/drm/amd/pm/swsmu/inc/smu_types.h
> index acf03838d49d..d6675b4ee9be 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/inc/smu_types.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/smu_types.h
> @@ -297,6 +297,9 @@
> __SMU_DUMMY_MAP(GetSmartShiftStatus), \
> __SMU_DUMMY_MAP(EnableLSdma), \
> __SMU_DUMMY_MAP(DisableLSdma), \
> + __SMU_DUMMY_MAP(GetMetricsTableVersion), \
> + __SMU_DUMMY_MAP(GetMetricsTableLogSample), \
> + __SMU_DUMMY_MAP(GetMetricsTableLogDramAddr), \
> __SMU_DUMMY_MAP(InitializeGfx), \
> __SMU_DUMMY_MAP(SetSoftMaxFclk), \
> __SMU_DUMMY_MAP(SetSoftMaxGl2clk), \
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu15/smu_v15_0_0_ppt.c b/
drivers/gpu/drm/amd/pm/swsmu/smu15/smu_v15_0_0_ppt.c
> index faa7d661f3bf..740bea549756 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu15/smu_v15_0_0_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu15/smu_v15_0_0_ppt.c
> @@ -33,6 +33,8 @@
> #include "smu_v15_0_0_pmfw.h"
> #include "smu_cmn.h"
>
> +#include <linux/overflow.h>
> +
> /*
> * DO NOT use these for err/warn/info/debug messages.
> * Use dev_err, dev_warn, dev_info and dev_dbg instead.
> @@ -119,6 +121,9 @@ static struct cmn2asic_msg_mapping
smu_v15_0_0_message_map[SMU_MSG_MAX_COUNT] =
> MSG_MAP(DisableLSdma,
PPSMC_MSG_DisableLSdma, 1),
> MSG_MAP(SetSoftMaxVpe,
PPSMC_MSG_SetSoftMaxVpe, 1),
> MSG_MAP(SetSoftMinVpe,
PPSMC_MSG_SetSoftMinVpe, 1),
> + MSG_MAP(GetMetricsTableVersion,
PPSMC_MSG_GetMetricsTableVersion, 1),
> + MSG_MAP(GetMetricsTableLogSample,
PPSMC_MSG_GetMetricsTableLogSample, 1),
> + MSG_MAP(GetMetricsTableLogDramAddr,
PPSMC_MSG_GetMetricsTableLogDramAddr, 1),
> };
>
> static struct cmn2asic_mapping
smu_v15_0_0_feature_mask_map[SMU_FEATURE_COUNT] = {
> @@ -160,10 +165,37 @@ static struct cmn2asic_mapping
smu_v15_0_0_table_map[SMU_TABLE_COUNT] = {
> TAB_MAP_VALID(DPMCLOCKS),
> };
>
> +static int smu_v15_0_0_get_metrics_table_dram_addr(struct
smu_context *smu,
> + SMU_15_0_0_MetricsInfo_t
*metrics_info)
> +{
> + struct smu_msg_ctl *ctl = &smu->msg_ctl;
> + struct smu_msg_args args = {
> + .msg = SMU_MSG_GetMetricsTableLogDramAddr,
> + .num_args = 0,
> + .num_out_args = 3,
> + };
> + int ret;
> +
> + ret = ctl->ops->send_msg(ctl, &args);
> + if (ret)
> + return ret;
> +
> + metrics_info->addr = ((uint64_t)args.out_args[1] << 32) |
args.out_args[0];
> + metrics_info->table_size = args.out_args[2];
> +
> + metrics_info->cpu_addr = ioremap_wc(metrics_info->addr,
> + metrics_info->table_size);
> + if (!metrics_info->cpu_addr)
> + return -ENOMEM;
> +
> + return 0;
> +}
> +
> static int smu_v15_0_0_init_smc_tables(struct smu_context *smu)
> {
> struct smu_table_context *smu_table = &smu->smu_table;
> struct smu_table *tables = smu_table->tables;
> + SMU_15_0_0_MetricsInfo_t *metrics_info;
> int ret;
>
> SMU_TABLE_INIT(tables, SMU_TABLE_WATERMARKS, sizeof(Watermarks_t),
> @@ -173,7 +205,7 @@ static int smu_v15_0_0_init_smc_tables(struct
smu_context *smu)
> SMU_TABLE_INIT(tables, SMU_TABLE_SMU_METRICS,
sizeof(SmuMetrics_t),
> PAGE_SIZE, AMDGPU_GEM_DOMAIN_VRAM);
>
> - smu_table->metrics_table = kzalloc_obj(SmuMetrics_t);
> + smu_table->metrics_table = kzalloc_obj(SMU_15_0_0_MetricsInfo_t);
> if (!smu_table->metrics_table)
> goto err0_out;
> smu_table->metrics_time = 0;
> @@ -192,8 +224,18 @@ static int smu_v15_0_0_init_smc_tables(struct
smu_context *smu)
> if (ret)
> goto err3_out;
>
> + metrics_info = (SMU_15_0_0_MetricsInfo_t *)smu_table-
>metrics_table;
> +
> + if (metrics_info->addr == 0) {
> + ret = smu_v15_0_0_get_metrics_table_dram_addr(smu,
metrics_info);
> + if (ret)
> + goto err4_out;
> + }
> +
> return 0;
>
> +err4_out:
> + smu_driver_table_fini(smu, SMU_DRIVER_TABLE_GPU_METRICS);
> err3_out:
> kfree(smu_table->watermarks_table);
> err2_out:
> @@ -207,6 +249,14 @@ static int smu_v15_0_0_init_smc_tables(struct
smu_context *smu)
> static int smu_v15_0_0_fini_smc_tables(struct smu_context *smu)
> {
> struct smu_table_context *smu_table = &smu->smu_table;
> + SMU_15_0_0_MetricsInfo_t *metrics_info = smu_table->metrics_table;
> +
> + if (metrics_info) {
> + if (metrics_info->cpu_addr) {
> + iounmap(metrics_info->cpu_addr);
> + metrics_info->cpu_addr = NULL;
> + }
> + }
>
> kfree(smu_table->clocks_table);
> smu_table->clocks_table = NULL;
> @@ -299,7 +349,7 @@ static int
smu_v15_0_0_set_default_dpm_tables(struct smu_context *smu)
> smu_table->clocks_table, false);
> }
>
> -static int smu_v15_0_0_get_metrics_table(struct smu_context *smu,
> +static int smu_v15_0_0_get_gpu_metrics_table(struct smu_context *smu,
> void
*metrics_table,
> bool bypass_cache)
> {
> @@ -329,115 +379,140 @@ static int
smu_v15_0_0_get_metrics_table(struct smu_context *smu,
> return 0;
> }
>
> +/*
> + * Fetch a fresh metrics sample into the inactive buffer.
> + * Returns 0 if a new sample was copied, 1 if the cached sample is still
> + * fresh (nothing copied), or a negative errno on failure.
> + */
> +static int smu_v15_0_0_get_metrics_table(struct smu_context *smu,
> + SMU_15_0_0_MetricsInfo_t
*metrics_info)
> +{
> + struct smu_table_context *smu_table = &smu->smu_table;
> + void __iomem *cpu_addr = metrics_info->cpu_addr;
> + struct smu_msg_ctl *ctl = &smu->msg_ctl;
> + struct smu_msg_args args = {0};
> + int table_size = metrics_info->table_size;
> + int ret;
> +
> + if (smu_table->metrics_time &&
> + !time_after(jiffies, smu_table->metrics_time +
msecs_to_jiffies(1)))
> + return 1;
> +
> + if (!cpu_addr)
> + return -ENOMEM;
> +
> + args.msg = SMU_MSG_GetMetricsTableLogSample;
> + args.num_args = 0;
> + args.num_out_args = 0;
> +
> + ret = ctl->ops->send_msg(ctl, &args);
> + if (ret)
> + return ret;
> +
> + amdgpu_hdp_invalidate(smu->adev, NULL);//best to flush before copy
// commenting style is not followed. Please change such occurences.
> + if (table_size <= sizeof(MetricsTable_t))
> + memcpy(&metrics_info->metrics[!metrics_info->active_idx],
> + cpu_addr, table_size);
> + else
> + memcpy(&metrics_info->metrics[!metrics_info->active_idx],
> + cpu_addr, sizeof(MetricsTable_t));
Ideally, from this point onwards active index is switched. The new index
has the latest data.
Thanks,
Lijo
> +
> + return 0;
> +}
> +
> +/*
> + * Accumulators monotonically increase and roll over at their type
width.
> + * Use the kernel wrapping_sub() API to compute the delta so the
subtraction
> + * wraps modulo 2^n (correct across a single rollover) without
tripping any
> + * wrap-around sanitizers.
> + */
> +static void smu_v15_0_0_compute_all_metrics(
> + uint32_t *avg_metric,
> + MetricsTable_t *prev,
> + MetricsTable_t *curr)
> +{
> + uint64_t counter, val;
> + MetricsTable_IOD_t *p = &prev->IOD;
> + MetricsTable_IOD_t *c = &curr->IOD;
> +
> + counter = wrapping_sub(u32, c->AccumulationCounter, p-
>AccumulationCounter);
> + if (!counter)
> + return;
> +
> + /* Accumulator-based clock frequencies (fixed-point /1024) */
> + val = wrapping_sub(u64, c->GfxclkFreqEffAcc, p->GfxclkFreqEffAcc);
> + avg_metric[METRICS_AVERAGE_GFXCLK] = (val / counter) / 1024;
> +
> + val = wrapping_sub(u64, c->SocclkFreqEffAcc, p->SocclkFreqEffAcc);
> + avg_metric[METRICS_AVERAGE_SOCCLK] = (val / counter) / 1024;
> +
> + val = wrapping_sub(u64, c->VclkFreqEffAcc, p->VclkFreqEffAcc);
> + avg_metric[METRICS_AVERAGE_VCLK] = (val / counter) / 1024;
> +
> + val = wrapping_sub(u64, c->MemclkFreqEffAcc, p->MemclkFreqEffAcc);
> + avg_metric[METRICS_AVERAGE_UCLK] = (val / counter) / 1024;
> +
> + val = wrapping_sub(u64, c->FclkFreqEffAcc, p->FclkFreqEffAcc);
> + avg_metric[METRICS_AVERAGE_FCLK] = (val / counter) / 1024;
> +
> + val = wrapping_sub(u64, c->NpuhclkFreqEffAcc, p-
>NpuhclkFreqEffAcc);
> + avg_metric[METRICS_AVERAGE_NPUCLK] = (val / counter) / 1024;
> +
> + /* Activity (fixed-point /1024) */
> + val = wrapping_sub(u64, c->GfxBusyAcc, p->GfxBusyAcc);
> + avg_metric[METRICS_AVERAGE_GFXACTIVITY] = (val / counter) / 1024;
> +
> + val = wrapping_sub(u64, c->VcnBusyAcc, p->VcnBusyAcc);
> + avg_metric[METRICS_AVERAGE_VCNACTIVITY] = (val / counter) / 1024;
> +
> + /* Power (fixed-point /1024) */
> + val = wrapping_sub(u64, c->ApuPowerAcc, p->ApuPowerAcc);
> + avg_metric[METRICS_AVERAGE_SOCKETPOWER] = (val / counter) / 1024;
> +
> + val = wrapping_sub(u64, c->SystemPowerAcc, p->SystemPowerAcc);
> + avg_metric[METRICS_CURR_SOCKETPOWER] = (val / counter) / 1024;
> +
> + /* Temperature */
> + val = wrapping_sub(u64, c->GFX_TempAcc, p->GFX_TempAcc);
> + avg_metric[METRICS_TEMPERATURE_EDGE] = val / counter;
> +
> + val = wrapping_sub(u64, c->STT_APU_HotSpotTempAcc, p-
>STT_APU_HotSpotTempAcc);
> + avg_metric[METRICS_TEMPERATURE_HOTSPOT] = val / counter;
> +
> + /* Voltage(mV) */
> + val = wrapping_sub(u64, c->VDDCR_GFX_TelemetryVoltage, p-
>VDDCR_GFX_TelemetryVoltage);
> + avg_metric[METRICS_VOLTAGE_VDDGFX] = (val / counter) /
1000;
> +
> + val = wrapping_sub(u64, c->VDDCR_SOC_TelemetryVoltage, p-
>VDDCR_SOC_TelemetryVoltage);
> + avg_metric[METRICS_VOLTAGE_VDDSOC] = (val / counter) /
1000;
> +}
> +
> static int smu_v15_0_0_get_smu_metrics_data(struct smu_context *smu,
> MetricsMember_t member,
> uint32_t *value)
> {
> struct smu_table_context *smu_table = &smu->smu_table;
> + SMU_15_0_0_MetricsInfo_t *metrics_info =
> + (SMU_15_0_0_MetricsInfo_t *)smu_table->metrics_table;
> + int ret;
>
> - SmuMetrics_t *metrics = (SmuMetrics_t *)smu_table->metrics_table;
> - int ret = 0;
> -
> - ret = smu_v15_0_0_get_metrics_table(smu, NULL, false);
> - if (ret)
> + ret = smu_v15_0_0_get_metrics_table(smu, metrics_info);
> + if (ret < 0)
> return ret;
>
> - switch (member) {
> - case METRICS_AVERAGE_GFXCLK:
> - *value = metrics->GfxclkFrequency;
> - break;
> - case METRICS_AVERAGE_SOCCLK:
> - *value = metrics->SocclkFrequency;
> - break;
> - case METRICS_AVERAGE_VCLK:
> - *value = metrics->VclkFrequency;
> - break;
> - case METRICS_AVERAGE_DCLK:
> - *value = 0;
> - break;
> - case METRICS_AVERAGE_UCLK:
> - *value = metrics->MemclkFrequency;
> - break;
> - case METRICS_AVERAGE_FCLK:
> - *value = metrics->FclkFrequency;
> - break;
> - case METRICS_AVERAGE_VPECLK:
> - *value = metrics->VpeclkFrequency;
> - break;
> - case METRICS_AVERAGE_NPUCLK:
> - *value = metrics->NpuclkFrequency;
> - break;
> - case METRICS_AVERAGE_GFXACTIVITY:
> - if ((smu->smc_fw_version > 0x5d4600))
> - *value = metrics->GfxActivity;
> - else
> - *value = metrics->GfxActivity / 100;
> - break;
> - case METRICS_AVERAGE_VCNACTIVITY:
> - *value = metrics->VcnActivity / 100;
> - break;
> - case METRICS_AVERAGE_SOCKETPOWER:
> - case METRICS_CURR_SOCKETPOWER:
> - *value = (metrics->SocketPower / 1000 << 8) +
> - (metrics->SocketPower % 1000 / 10);
> - break;
> - case METRICS_TEMPERATURE_EDGE:
> - *value = metrics->GfxTemperature / 100 *
> - SMU_TEMPERATURE_UNITS_PER_CENTIGRADES;
> - break;
> - case METRICS_TEMPERATURE_HOTSPOT:
> - *value = metrics->SocTemperature / 100 *
> - SMU_TEMPERATURE_UNITS_PER_CENTIGRADES;
> - break;
> - case METRICS_THROTTLER_RESIDENCY_PROCHOT:
> - *value = metrics->ThrottleResidency_PROCHOT;
> - break;
> - case METRICS_THROTTLER_RESIDENCY_SPL:
> - *value = metrics->ThrottleResidency_SPL;
> - break;
> - case METRICS_THROTTLER_RESIDENCY_FPPT:
> - *value = metrics->ThrottleResidency_FPPT;
> - break;
> - case METRICS_THROTTLER_RESIDENCY_SPPT:
> - *value = metrics->ThrottleResidency_SPPT;
> - break;
> - case METRICS_THROTTLER_RESIDENCY_THM_SOC:
> - *value = metrics->ThrottleResidency_THM_SOC;
> - break;
> - case METRICS_VOLTAGE_VDDGFX:
> - *value = 0;
> - break;
> - case METRICS_VOLTAGE_VDDSOC:
> - *value = 0;
> - break;
> - case METRICS_SS_APU_SHARE:
> - /* return the percentage of APU power with respect to
APU's power limit.
> - * percentage is reported, this isn't boost value.
Smartshift power
> - * boost/shift is only when the percentage is more than
100.
> - */
> - if (metrics->StapmOpnLimit > 0)
> - *value = (metrics->ApuPower * 100) / metrics-
>StapmOpnLimit;
> - else
> - *value = 0;
> - break;
> - case METRICS_SS_DGPU_SHARE:
> - /* return the percentage of dGPU power with respect to
dGPU's power limit.
> - * percentage is reported, this isn't boost value.
Smartshift power
> - * boost/shift is only when the percentage is more than
100.
> - */
> - if ((metrics->dGpuPower > 0) &&
> - (metrics->StapmCurrentLimit > metrics->StapmOpnLimit))
> - *value = (metrics->dGpuPower * 100) /
> - (metrics->StapmCurrentLimit - metrics-
>StapmOpnLimit);
> - else
> - *value = 0;
> - break;
> - default:
> - *value = UINT_MAX;
> - break;
> + if (ret == 0 &&
> + metrics_info->metrics[metrics_info-
>active_idx].IOD.AccumulationCounter !=
> + metrics_info->metrics[!metrics_info-
>active_idx].IOD.AccumulationCounter) {
> + /* New sample: compute all accumulator-based metrics
and swap */
> + smu_v15_0_0_compute_all_metrics(metrics_info->avg_metric,
> + &metrics_info-
>metrics[metrics_info->active_idx],
> + &metrics_info-
>metrics[!metrics_info->active_idx]);
> + metrics_info->active_idx = !metrics_info->active_idx;
> }
>
> - return ret;
> + *value = metrics_info->avg_metric[member];
> +
> + return 0;
> }
Indexes are switched in the above highlighted line.
smu_v15_0_0_get_metrics_table() takes active_idx as 0 initially(first
time). New metrics are fetched in index 1. Then in
smu_v15_0_0_get_smu_metrics_data() accumulation counter is compared
across both indexes and if it has changed, we compute all the metrics.
Then we change the active_idx to 0(highlighted line). So, next time when
smu_v15_0_0_get_smu_metrics_data() is called active_idx is 1 and new
metrics are fetched in index 0. That is how the metrics are switched
alternately.
