Great, thanks for all the feedback Lijo. Out of the new bit definitions in amdgpu_smu.h are there any that currently exist that are more applicable for these mappings? *_THM_GFX and *_THM_SOC only exist in VanGogh and Renoir. With the expansion of the MEM and LIQUID bits there is not enough room in the temperature field to add two new definitions.
Best,
Graham
-----Original Message-----
From: Lazar, Lijo <Lijo.Lazar@xxxxxxx>
Sent: Monday, June 7, 2021 10:35 AM
To: Sider, Graham <Graham.Sider@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Cc: Kasiviswanathan, Harish <Harish.Kasiviswanathan@xxxxxxx>; Sakhnovitch, Elena (Elen) <Elena.Sakhnovitch@xxxxxxx>
Subject: Re: [PATCH v5 7/9] drm/amd/pm: Add vangogh throttler translation
On 6/7/2021 7:14 PM, Graham Sider wrote:
> Perform dependent to independent throttle status translation for
> vangogh.
>
> Signed-off-by: Graham Sider <Graham.Sider@xxxxxxx>
> ---
> .../gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c | 38 ++++++++++++++-----
> 1 file changed, 29 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
> b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
> index 77f532a49e37..589304367929 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
> @@ -190,6 +190,20 @@ static struct cmn2asic_mapping vangogh_workload_map[PP_SMC_POWER_PROFILE_COUNT]
> WORKLOAD_MAP(PP_SMC_POWER_PROFILE_CUSTOM, WORKLOAD_PPLIB_CUSTOM_BIT),
> };
>
> +static const uint8_t vangogh_throttler_map[] = {
> + [THROTTLER_STATUS_BIT_SPL] = (SMU_THROTTLER_SPL_BIT),
> + [THROTTLER_STATUS_BIT_FPPT] = (SMU_THROTTLER_FPPT_BIT),
> + [THROTTLER_STATUS_BIT_SPPT] = (SMU_THROTTLER_SPPT_BIT),
> + [THROTTLER_STATUS_BIT_SPPT_APU] = (SMU_THROTTLER_SPPT_APU_BIT),
> + [THROTTLER_STATUS_BIT_THM_CORE] = (SMU_THROTTLER_TEMP_CORE_BIT),
> + [THROTTLER_STATUS_BIT_THM_GFX] = (SMU_THROTTLER_TEMP_VR_GFX_BIT),
> + [THROTTLER_STATUS_BIT_THM_SOC] = (SMU_THROTTLER_TEMP_VR_SOC_BIT),
Above two mappings don't look correct. They essentially mean throttling due to GFX/SOC domain temperatures in APU exceeding their limits, not the VR temperatures. Except those mappings, rest of the patch series looks good to me.
Thanks,
Lijo
> + [THROTTLER_STATUS_BIT_TDC_VDD] = (SMU_THROTTLER_TDC_VDD_BIT),
> + [THROTTLER_STATUS_BIT_TDC_SOC] = (SMU_THROTTLER_TDC_SOC_BIT),
> + [THROTTLER_STATUS_BIT_TDC_GFX] = (SMU_THROTTLER_TDC_GFX_BIT),
> + [THROTTLER_STATUS_BIT_TDC_CVIP] = (SMU_THROTTLER_TDC_CVIP_BIT),
> +};
> +
> static int vangogh_tables_init(struct smu_context *smu)
> {
> struct smu_table_context *smu_table = &smu->smu_table; @@ -226,7
> +240,7 @@ static int vangogh_tables_init(struct smu_context *smu)
> goto err0_out;
> smu_table->metrics_time = 0;
>
> - smu_table->gpu_metrics_table_size = sizeof(struct gpu_metrics_v2_1);
> + smu_table->gpu_metrics_table_size = sizeof(struct gpu_metrics_v2_2);
> smu_table->gpu_metrics_table = kzalloc(smu_table->gpu_metrics_table_size, GFP_KERNEL);
> if (!smu_table->gpu_metrics_table)
> goto err1_out;
> @@ -1632,8 +1646,8 @@ static ssize_t vangogh_get_legacy_gpu_metrics(struct smu_context *smu,
> void **table)
> {
> struct smu_table_context *smu_table = &smu->smu_table;
> - struct gpu_metrics_v2_1 *gpu_metrics =
> - (struct gpu_metrics_v2_1 *)smu_table->gpu_metrics_table;
> + struct gpu_metrics_v2_2 *gpu_metrics =
> + (struct gpu_metrics_v2_2 *)smu_table->gpu_metrics_table;
> SmuMetrics_legacy_t metrics;
> int ret = 0;
>
> @@ -1641,7 +1655,7 @@ static ssize_t vangogh_get_legacy_gpu_metrics(struct smu_context *smu,
> if (ret)
> return ret;
>
> - smu_cmn_init_soft_gpu_metrics(gpu_metrics, 2, 1);
> + smu_cmn_init_soft_gpu_metrics(gpu_metrics, 2, 2);
>
> gpu_metrics->temperature_gfx = metrics.GfxTemperature;
> gpu_metrics->temperature_soc = metrics.SocTemperature; @@ -1674,20
> +1688,23 @@ static ssize_t vangogh_get_legacy_gpu_metrics(struct smu_context *smu,
> gpu_metrics->current_l3clk[0] = metrics.L3Frequency[0];
>
> gpu_metrics->throttle_status = metrics.ThrottlerStatus;
> + gpu_metrics->indep_throttle_status =
> + smu_cmn_get_indep_throttler_status(metrics.ThrottlerStatus,
> + vangogh_throttler_map);
>
> gpu_metrics->system_clock_counter = ktime_get_boottime_ns();
>
> *table = (void *)gpu_metrics;
>
> - return sizeof(struct gpu_metrics_v2_1);
> + return sizeof(struct gpu_metrics_v2_2);
> }
>
> static ssize_t vangogh_get_gpu_metrics(struct smu_context *smu,
> void **table)
> {
> struct smu_table_context *smu_table = &smu->smu_table;
> - struct gpu_metrics_v2_1 *gpu_metrics =
> - (struct gpu_metrics_v2_1 *)smu_table->gpu_metrics_table;
> + struct gpu_metrics_v2_2 *gpu_metrics =
> + (struct gpu_metrics_v2_2 *)smu_table->gpu_metrics_table;
> SmuMetrics_t metrics;
> int ret = 0;
>
> @@ -1695,7 +1712,7 @@ static ssize_t vangogh_get_gpu_metrics(struct smu_context *smu,
> if (ret)
> return ret;
>
> - smu_cmn_init_soft_gpu_metrics(gpu_metrics, 2, 1);
> + smu_cmn_init_soft_gpu_metrics(gpu_metrics, 2, 2);
>
> gpu_metrics->temperature_gfx = metrics.Current.GfxTemperature;
> gpu_metrics->temperature_soc = metrics.Current.SocTemperature; @@
> -1735,12 +1752,15 @@ static ssize_t vangogh_get_gpu_metrics(struct smu_context *smu,
> gpu_metrics->current_l3clk[0] = metrics.Current.L3Frequency[0];
>
> gpu_metrics->throttle_status = metrics.Current.ThrottlerStatus;
> + gpu_metrics->indep_throttle_status =
> + smu_cmn_get_indep_throttler_status(metrics.Current.ThrottlerStatus,
> + vangogh_throttler_map);
>
> gpu_metrics->system_clock_counter = ktime_get_boottime_ns();
>
> *table = (void *)gpu_metrics;
>
> - return sizeof(struct gpu_metrics_v2_1);
> + return sizeof(struct gpu_metrics_v2_2);
> }
>
> static ssize_t vangogh_common_get_gpu_metrics(struct smu_context
> *smu,
>
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
