[Public]

Didn't realize 16-bits will max out so fast. For THM_GFX - 
"SMU_THROTTLER_TEMP_GPU_BIT" looks appropriate. SOC domain will need a new one. 
As temp throttling reasons are more, you may shift the "OTHERS" by 8-bits if 
required. 

Thanks,
Lijo

-----Original Message-----
From: Sider, Graham <graham.si...@amd.com> 
Sent: Monday, June 7, 2021 8:23 PM
To: Lazar, Lijo <lijo.la...@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Kasiviswanathan, Harish <harish.kasiviswanat...@amd.com>; Sakhnovitch, 
Elena (Elen) <elena.sakhnovi...@amd.com>
Subject: RE: [PATCH v5 7/9] drm/amd/pm: Add vangogh throttler translation

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.la...@amd.com> 
Sent: Monday, June 7, 2021 10:35 AM
To: Sider, Graham <graham.si...@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Kasiviswanathan, Harish <harish.kasiviswanat...@amd.com>; Sakhnovitch, 
Elena (Elen) <elena.sakhnovi...@amd.com>
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.si...@amd.com>
> ---
>   .../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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to