On 9/12/2024 5:29 PM, Asad Kamal wrote:
> Use same metric table for APU and Non APU systems
> for smu_v_13_0_6 to get metric data based on newer pmfw
> versions
> 
> v2: Use inline func to check for ubified metrics support
> 

Typo -ubified  => unified

        Reviewed-by: Lijo Lazar <lijo.la...@amd.com>

Thanks,
Lijo

> Signed-off-by: Asad Kamal <asad.ka...@amd.com>
> ---
>  .../drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c  | 102 ++++++++++--------
>  1 file changed, 55 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c 
> b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
> index 9974c9f8135e..ee178914ca53 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
> @@ -102,6 +102,12 @@ MODULE_FIRMWARE("amdgpu/smu_13_0_14.bin");
>  #define MCA_BANK_IPID(_ip, _hwid, _type) \
>       [AMDGPU_MCA_IP_##_ip] = { .hwid = _hwid, .mcatype = _type, }
>  
> +static inline bool smu_v13_0_6_is_unified_metrics(struct smu_context *smu)
> +{
> +     return (smu->adev->flags & AMD_IS_APU) &&
> +             smu->smc_fw_version <= 0x4556900;
> +}
> +
>  struct mca_bank_ipid {
>       enum amdgpu_mca_ip ip;
>       uint16_t hwid;
> @@ -253,7 +259,7 @@ struct PPTable_t {
>  #define SMUQ10_TO_UINT(x) ((x) >> 10)
>  #define SMUQ10_FRAC(x) ((x) & 0x3ff)
>  #define SMUQ10_ROUND(x) ((SMUQ10_TO_UINT(x)) + ((SMUQ10_FRAC(x)) >= 0x200))
> -#define GET_METRIC_FIELD(field) ((adev->flags & AMD_IS_APU) ?\
> +#define GET_METRIC_FIELD(field, flag) ((flag) ?\
>               (metrics_a->field) : (metrics_x->field))
>  
>  struct smu_v13_0_6_dpm_map {
> @@ -583,7 +589,7 @@ static int smu_v13_0_6_setup_driver_pptable(struct 
> smu_context *smu)
>       MetricsTableA_t *metrics_a = (MetricsTableA_t 
> *)smu_table->metrics_table;
>       struct PPTable_t *pptable =
>               (struct PPTable_t *)smu_table->driver_pptable;
> -     struct amdgpu_device *adev = smu->adev;
> +     bool flag = smu_v13_0_6_is_unified_metrics(smu);
>       int ret, i, retry = 100;
>       uint32_t table_version;
>  
> @@ -595,7 +601,7 @@ static int smu_v13_0_6_setup_driver_pptable(struct 
> smu_context *smu)
>                               return ret;
>  
>                       /* Ensure that metrics have been updated */
> -                     if (GET_METRIC_FIELD(AccumulationCounter))
> +                     if (GET_METRIC_FIELD(AccumulationCounter, flag))
>                               break;
>  
>                       usleep_range(1000, 1100);
> @@ -612,29 +618,29 @@ static int smu_v13_0_6_setup_driver_pptable(struct 
> smu_context *smu)
>                       table_version;
>  
>               pptable->MaxSocketPowerLimit =
> -                     SMUQ10_ROUND(GET_METRIC_FIELD(MaxSocketPowerLimit));
> +                     SMUQ10_ROUND(GET_METRIC_FIELD(MaxSocketPowerLimit, 
> flag));
>               pptable->MaxGfxclkFrequency =
> -                     SMUQ10_ROUND(GET_METRIC_FIELD(MaxGfxclkFrequency));
> +                     SMUQ10_ROUND(GET_METRIC_FIELD(MaxGfxclkFrequency, 
> flag));
>               pptable->MinGfxclkFrequency =
> -                     SMUQ10_ROUND(GET_METRIC_FIELD(MinGfxclkFrequency));
> +                     SMUQ10_ROUND(GET_METRIC_FIELD(MinGfxclkFrequency, 
> flag));
>  
>               for (i = 0; i < 4; ++i) {
>                       pptable->FclkFrequencyTable[i] =
> -                             
> SMUQ10_ROUND(GET_METRIC_FIELD(FclkFrequencyTable)[i]);
> +                             
> SMUQ10_ROUND(GET_METRIC_FIELD(FclkFrequencyTable, flag)[i]);
>                       pptable->UclkFrequencyTable[i] =
> -                             
> SMUQ10_ROUND(GET_METRIC_FIELD(UclkFrequencyTable)[i]);
> +                             
> SMUQ10_ROUND(GET_METRIC_FIELD(UclkFrequencyTable, flag)[i]);
>                       pptable->SocclkFrequencyTable[i] = SMUQ10_ROUND(
> -                             GET_METRIC_FIELD(SocclkFrequencyTable)[i]);
> +                             GET_METRIC_FIELD(SocclkFrequencyTable, 
> flag)[i]);
>                       pptable->VclkFrequencyTable[i] =
> -                             
> SMUQ10_ROUND(GET_METRIC_FIELD(VclkFrequencyTable)[i]);
> +                             
> SMUQ10_ROUND(GET_METRIC_FIELD(VclkFrequencyTable, flag)[i]);
>                       pptable->DclkFrequencyTable[i] =
> -                             
> SMUQ10_ROUND(GET_METRIC_FIELD(DclkFrequencyTable)[i]);
> +                             
> SMUQ10_ROUND(GET_METRIC_FIELD(DclkFrequencyTable, flag)[i]);
>                       pptable->LclkFrequencyTable[i] =
> -                             
> SMUQ10_ROUND(GET_METRIC_FIELD(LclkFrequencyTable)[i]);
> +                             
> SMUQ10_ROUND(GET_METRIC_FIELD(LclkFrequencyTable, flag)[i]);
>               }
>  
>               /* use AID0 serial number by default */
> -             pptable->PublicSerialNumber_AID = 
> GET_METRIC_FIELD(PublicSerialNumber_AID)[0];
> +             pptable->PublicSerialNumber_AID = 
> GET_METRIC_FIELD(PublicSerialNumber_AID, flag)[0];
>  
>               pptable->Init = true;
>       }
> @@ -957,6 +963,7 @@ static int smu_v13_0_6_get_smu_metrics_data(struct 
> smu_context *smu,
>       struct smu_table_context *smu_table = &smu->smu_table;
>       MetricsTableX_t *metrics_x = (MetricsTableX_t 
> *)smu_table->metrics_table;
>       MetricsTableA_t *metrics_a = (MetricsTableA_t 
> *)smu_table->metrics_table;
> +     bool flag = smu_v13_0_6_is_unified_metrics(smu);
>       struct amdgpu_device *adev = smu->adev;
>       int ret = 0;
>       int xcc_id;
> @@ -971,50 +978,50 @@ static int smu_v13_0_6_get_smu_metrics_data(struct 
> smu_context *smu,
>       case METRICS_AVERAGE_GFXCLK:
>               if (smu->smc_fw_version >= 0x552F00) {
>                       xcc_id = GET_INST(GC, 0);
> -                     *value = 
> SMUQ10_ROUND(GET_METRIC_FIELD(GfxclkFrequency)[xcc_id]);
> +                     *value = SMUQ10_ROUND(GET_METRIC_FIELD(GfxclkFrequency, 
> flag)[xcc_id]);
>               } else {
>                       *value = 0;
>               }
>               break;
>       case METRICS_CURR_SOCCLK:
>       case METRICS_AVERAGE_SOCCLK:
> -             *value = SMUQ10_ROUND(GET_METRIC_FIELD(SocclkFrequency)[0]);
> +             *value = SMUQ10_ROUND(GET_METRIC_FIELD(SocclkFrequency, 
> flag)[0]);
>               break;
>       case METRICS_CURR_UCLK:
>       case METRICS_AVERAGE_UCLK:
> -             *value = SMUQ10_ROUND(GET_METRIC_FIELD(UclkFrequency));
> +             *value = SMUQ10_ROUND(GET_METRIC_FIELD(UclkFrequency, flag));
>               break;
>       case METRICS_CURR_VCLK:
> -             *value = SMUQ10_ROUND(GET_METRIC_FIELD(VclkFrequency)[0]);
> +             *value = SMUQ10_ROUND(GET_METRIC_FIELD(VclkFrequency, flag)[0]);
>               break;
>       case METRICS_CURR_DCLK:
> -             *value = SMUQ10_ROUND(GET_METRIC_FIELD(DclkFrequency)[0]);
> +             *value = SMUQ10_ROUND(GET_METRIC_FIELD(DclkFrequency, flag)[0]);
>               break;
>       case METRICS_CURR_FCLK:
> -             *value = SMUQ10_ROUND(GET_METRIC_FIELD(FclkFrequency));
> +             *value = SMUQ10_ROUND(GET_METRIC_FIELD(FclkFrequency, flag));
>               break;
>       case METRICS_AVERAGE_GFXACTIVITY:
> -             *value = SMUQ10_ROUND(GET_METRIC_FIELD(SocketGfxBusy));
> +             *value = SMUQ10_ROUND(GET_METRIC_FIELD(SocketGfxBusy, flag));
>               break;
>       case METRICS_AVERAGE_MEMACTIVITY:
> -             *value = 
> SMUQ10_ROUND(GET_METRIC_FIELD(DramBandwidthUtilization));
> +             *value = 
> SMUQ10_ROUND(GET_METRIC_FIELD(DramBandwidthUtilization, flag));
>               break;
>       case METRICS_CURR_SOCKETPOWER:
> -             *value = SMUQ10_ROUND(GET_METRIC_FIELD(SocketPower)) << 8;
> +             *value = SMUQ10_ROUND(GET_METRIC_FIELD(SocketPower, flag)) << 8;
>               break;
>       case METRICS_TEMPERATURE_HOTSPOT:
> -             *value = SMUQ10_ROUND(GET_METRIC_FIELD(MaxSocketTemperature)) *
> +             *value = SMUQ10_ROUND(GET_METRIC_FIELD(MaxSocketTemperature, 
> flag)) *
>                        SMU_TEMPERATURE_UNITS_PER_CENTIGRADES;
>               break;
>       case METRICS_TEMPERATURE_MEM:
> -             *value = SMUQ10_ROUND(GET_METRIC_FIELD(MaxHbmTemperature)) *
> +             *value = SMUQ10_ROUND(GET_METRIC_FIELD(MaxHbmTemperature, 
> flag)) *
>                        SMU_TEMPERATURE_UNITS_PER_CENTIGRADES;
>               break;
>       /* This is the max of all VRs and not just SOC VR.
>        * No need to define another data type for the same.
>        */
>       case METRICS_TEMPERATURE_VRSOC:
> -             *value = SMUQ10_ROUND(GET_METRIC_FIELD(MaxVrTemperature)) *
> +             *value = SMUQ10_ROUND(GET_METRIC_FIELD(MaxVrTemperature, flag)) 
> *
>                        SMU_TEMPERATURE_UNITS_PER_CENTIGRADES;
>               break;
>       default:
> @@ -2298,6 +2305,7 @@ static ssize_t smu_v13_0_6_get_gpu_metrics(struct 
> smu_context *smu, void **table
>       struct smu_table_context *smu_table = &smu->smu_table;
>       struct gpu_metrics_v1_5 *gpu_metrics =
>               (struct gpu_metrics_v1_5 *)smu_table->gpu_metrics_table;
> +     bool flag = smu_v13_0_6_is_unified_metrics(smu);
>       struct amdgpu_device *adev = smu->adev;
>       int ret = 0, xcc_id, inst, i, j;
>       MetricsTableX_t *metrics_x;
> @@ -2316,50 +2324,50 @@ static ssize_t smu_v13_0_6_get_gpu_metrics(struct 
> smu_context *smu, void **table
>       smu_cmn_init_soft_gpu_metrics(gpu_metrics, 1, 5);
>  
>       gpu_metrics->temperature_hotspot =
> -             SMUQ10_ROUND(GET_METRIC_FIELD(MaxSocketTemperature));
> +             SMUQ10_ROUND(GET_METRIC_FIELD(MaxSocketTemperature, flag));
>       /* Individual HBM stack temperature is not reported */
>       gpu_metrics->temperature_mem =
> -             SMUQ10_ROUND(GET_METRIC_FIELD(MaxHbmTemperature));
> +             SMUQ10_ROUND(GET_METRIC_FIELD(MaxHbmTemperature, flag));
>       /* Reports max temperature of all voltage rails */
>       gpu_metrics->temperature_vrsoc =
> -             SMUQ10_ROUND(GET_METRIC_FIELD(MaxVrTemperature));
> +             SMUQ10_ROUND(GET_METRIC_FIELD(MaxVrTemperature, flag));
>  
>       gpu_metrics->average_gfx_activity =
> -             SMUQ10_ROUND(GET_METRIC_FIELD(SocketGfxBusy));
> +             SMUQ10_ROUND(GET_METRIC_FIELD(SocketGfxBusy, flag));
>       gpu_metrics->average_umc_activity =
> -             SMUQ10_ROUND(GET_METRIC_FIELD(DramBandwidthUtilization));
> +             SMUQ10_ROUND(GET_METRIC_FIELD(DramBandwidthUtilization, flag));
>  
>       gpu_metrics->curr_socket_power =
> -             SMUQ10_ROUND(GET_METRIC_FIELD(SocketPower));
> +             SMUQ10_ROUND(GET_METRIC_FIELD(SocketPower, flag));
>       /* Energy counter reported in 15.259uJ (2^-16) units */
> -     gpu_metrics->energy_accumulator = GET_METRIC_FIELD(SocketEnergyAcc);
> +     gpu_metrics->energy_accumulator = GET_METRIC_FIELD(SocketEnergyAcc, 
> flag);
>  
>       for (i = 0; i < MAX_GFX_CLKS; i++) {
>               xcc_id = GET_INST(GC, i);
>               if (xcc_id >= 0)
>                       gpu_metrics->current_gfxclk[i] =
> -                             
> SMUQ10_ROUND(GET_METRIC_FIELD(GfxclkFrequency)[xcc_id]);
> +                             SMUQ10_ROUND(GET_METRIC_FIELD(GfxclkFrequency, 
> flag)[xcc_id]);
>  
>               if (i < MAX_CLKS) {
>                       gpu_metrics->current_socclk[i] =
> -                             
> SMUQ10_ROUND(GET_METRIC_FIELD(SocclkFrequency)[i]);
> +                             SMUQ10_ROUND(GET_METRIC_FIELD(SocclkFrequency, 
> flag)[i]);
>                       inst = GET_INST(VCN, i);
>                       if (inst >= 0) {
>                               gpu_metrics->current_vclk0[i] =
> -                                     
> SMUQ10_ROUND(GET_METRIC_FIELD(VclkFrequency)[inst]);
> +                                     
> SMUQ10_ROUND(GET_METRIC_FIELD(VclkFrequency, flag)[inst]);
>                               gpu_metrics->current_dclk0[i] =
> -                                     
> SMUQ10_ROUND(GET_METRIC_FIELD(DclkFrequency)[inst]);
> +                                     
> SMUQ10_ROUND(GET_METRIC_FIELD(DclkFrequency, flag)[inst]);
>                       }
>               }
>       }
>  
> -     gpu_metrics->current_uclk = 
> SMUQ10_ROUND(GET_METRIC_FIELD(UclkFrequency));
> +     gpu_metrics->current_uclk = 
> SMUQ10_ROUND(GET_METRIC_FIELD(UclkFrequency, flag));
>  
>       /* Throttle status is not reported through metrics now */
>       gpu_metrics->throttle_status = 0;
>  
>       /* Clock Lock Status. Each bit corresponds to each GFXCLK instance */
> -     gpu_metrics->gfxclk_lock_status = GET_METRIC_FIELD(GfxLockXCDMak) >> 
> GET_INST(GC, 0);
> +     gpu_metrics->gfxclk_lock_status = GET_METRIC_FIELD(GfxLockXCDMak, flag) 
> >> GET_INST(GC, 0);
>  
>       if (!(adev->flags & AMD_IS_APU)) {
>               /*Check smu version, PCIE link speed and width will be reported 
> from pmfw metric
> @@ -2400,22 +2408,22 @@ static ssize_t smu_v13_0_6_get_gpu_metrics(struct 
> smu_context *smu, void **table
>       gpu_metrics->system_clock_counter = ktime_get_boottime_ns();
>  
>       gpu_metrics->gfx_activity_acc =
> -             SMUQ10_ROUND(GET_METRIC_FIELD(SocketGfxBusyAcc));
> +             SMUQ10_ROUND(GET_METRIC_FIELD(SocketGfxBusyAcc, flag));
>       gpu_metrics->mem_activity_acc =
> -             SMUQ10_ROUND(GET_METRIC_FIELD(DramBandwidthUtilizationAcc));
> +             SMUQ10_ROUND(GET_METRIC_FIELD(DramBandwidthUtilizationAcc, 
> flag));
>  
>       for (i = 0; i < NUM_XGMI_LINKS; i++) {
>               gpu_metrics->xgmi_read_data_acc[i] =
> -                     SMUQ10_ROUND(GET_METRIC_FIELD(XgmiReadDataSizeAcc)[i]);
> +                     SMUQ10_ROUND(GET_METRIC_FIELD(XgmiReadDataSizeAcc, 
> flag)[i]);
>               gpu_metrics->xgmi_write_data_acc[i] =
> -                     SMUQ10_ROUND(GET_METRIC_FIELD(XgmiWriteDataSizeAcc)[i]);
> +                     SMUQ10_ROUND(GET_METRIC_FIELD(XgmiWriteDataSizeAcc, 
> flag)[i]);
>       }
>  
>       for (i = 0; i < adev->jpeg.num_jpeg_inst; ++i) {
>               inst = GET_INST(JPEG, i);
>               for (j = 0; j < adev->jpeg.num_jpeg_rings; ++j) {
>                       gpu_metrics->jpeg_activity[(i * 
> adev->jpeg.num_jpeg_rings) + j] =
> -                             SMUQ10_ROUND(GET_METRIC_FIELD(JpegBusy)
> +                             SMUQ10_ROUND(GET_METRIC_FIELD(JpegBusy, flag)
>                               [(inst * adev->jpeg.num_jpeg_rings) + j]);
>               }
>       }
> @@ -2423,13 +2431,13 @@ static ssize_t smu_v13_0_6_get_gpu_metrics(struct 
> smu_context *smu, void **table
>       for (i = 0; i < adev->vcn.num_vcn_inst; ++i) {
>               inst = GET_INST(VCN, i);
>               gpu_metrics->vcn_activity[i] =
> -                     SMUQ10_ROUND(GET_METRIC_FIELD(VcnBusy)[inst]);
> +                     SMUQ10_ROUND(GET_METRIC_FIELD(VcnBusy, flag)[inst]);
>       }
>  
> -     gpu_metrics->xgmi_link_width = 
> SMUQ10_ROUND(GET_METRIC_FIELD(XgmiWidth));
> -     gpu_metrics->xgmi_link_speed = 
> SMUQ10_ROUND(GET_METRIC_FIELD(XgmiBitrate));
> +     gpu_metrics->xgmi_link_width = SMUQ10_ROUND(GET_METRIC_FIELD(XgmiWidth, 
> flag));
> +     gpu_metrics->xgmi_link_speed = 
> SMUQ10_ROUND(GET_METRIC_FIELD(XgmiBitrate, flag));
>  
> -     gpu_metrics->firmware_timestamp = GET_METRIC_FIELD(Timestamp);
> +     gpu_metrics->firmware_timestamp = GET_METRIC_FIELD(Timestamp, flag);
>  
>       *table = (void *)gpu_metrics;
>       kfree(metrics_x);

Reply via email to