[PATCH] drm/amd/pm: bug fix for pcie dpm

Alex Deucher alexdeucher at gmail.com
Tue Mar 9 15:19:15 UTC 2021


On Tue, Mar 9, 2021 at 8:17 AM Kenneth Feng <kenneth.feng at amd.com> wrote:
>
> Currently the pcie dpm has two problems.
> 1. Only the high dpm level speed/width can be overrided
> if the requested values are out of the pcie capability.
> 2. The high dpm level is always overrided though sometimes
> it's not necesarry.
>
> Signed-off-by: Kenneth Feng <kenneth.feng at amd.com>

Reviewed-by: Alex Deucher <alexander.deucher at amd.com>

> ---
>  .../drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c | 48 ++++++++++++++
>  .../drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c | 66 +++++++++++++++++++
>  .../drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c | 48 ++++++++------
>  3 files changed, 141 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
> index 5e875ad8d633..408b35866704 100644
> --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
> +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
> @@ -1505,6 +1505,48 @@ static int vega10_populate_single_lclk_level(struct pp_hwmgr *hwmgr,
>         return 0;
>  }
>
> +static int vega10_override_pcie_parameters(struct pp_hwmgr *hwmgr)
> +{
> +       struct amdgpu_device *adev = (struct amdgpu_device *)(hwmgr->adev);
> +       struct vega10_hwmgr *data =
> +                       (struct vega10_hwmgr *)(hwmgr->backend);
> +       uint32_t pcie_gen = 0, pcie_width = 0;
> +       PPTable_t *pp_table = &(data->smc_state_table.pp_table);
> +       int i;
> +
> +       if (adev->pm.pcie_gen_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN4)
> +               pcie_gen = 3;
> +       else if (adev->pm.pcie_gen_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN3)
> +               pcie_gen = 2;
> +       else if (adev->pm.pcie_gen_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN2)
> +               pcie_gen = 1;
> +       else if (adev->pm.pcie_gen_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN1)
> +               pcie_gen = 0;
> +
> +       if (adev->pm.pcie_mlw_mask & CAIL_PCIE_LINK_WIDTH_SUPPORT_X16)
> +               pcie_width = 6;
> +       else if (adev->pm.pcie_mlw_mask & CAIL_PCIE_LINK_WIDTH_SUPPORT_X12)
> +               pcie_width = 5;
> +       else if (adev->pm.pcie_mlw_mask & CAIL_PCIE_LINK_WIDTH_SUPPORT_X8)
> +               pcie_width = 4;
> +       else if (adev->pm.pcie_mlw_mask & CAIL_PCIE_LINK_WIDTH_SUPPORT_X4)
> +               pcie_width = 3;
> +       else if (adev->pm.pcie_mlw_mask & CAIL_PCIE_LINK_WIDTH_SUPPORT_X2)
> +               pcie_width = 2;
> +       else if (adev->pm.pcie_mlw_mask & CAIL_PCIE_LINK_WIDTH_SUPPORT_X1)
> +               pcie_width = 1;
> +
> +       for (i = 0; i < NUM_LINK_LEVELS; i++) {
> +               if (pp_table->PcieGenSpeed[i] > pcie_gen)
> +                       pp_table->PcieGenSpeed[i] = pcie_gen;
> +
> +               if (pp_table->PcieLaneCount[i] > pcie_width)
> +                       pp_table->PcieLaneCount[i] = pcie_width;
> +       }
> +
> +       return 0;
> +}
> +
>  static int vega10_populate_smc_link_levels(struct pp_hwmgr *hwmgr)
>  {
>         int result = -1;
> @@ -2556,6 +2598,11 @@ static int vega10_init_smc_table(struct pp_hwmgr *hwmgr)
>                         "Failed to initialize Link Level!",
>                         return result);
>
> +       result = vega10_override_pcie_parameters(hwmgr);
> +       PP_ASSERT_WITH_CODE(!result,
> +                       "Failed to override pcie parameters!",
> +                       return result);
> +
>         result = vega10_populate_all_graphic_levels(hwmgr);
>         PP_ASSERT_WITH_CODE(!result,
>                         "Failed to initialize Graphics Level!",
> @@ -2922,6 +2969,7 @@ static int vega10_start_dpm(struct pp_hwmgr *hwmgr, uint32_t bitmap)
>         return 0;
>  }
>
> +
>  static int vega10_enable_disable_PCC_limit_feature(struct pp_hwmgr *hwmgr, bool enable)
>  {
>         struct vega10_hwmgr *data = hwmgr->backend;
> diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c
> index a827f2bc7904..196ac2a4d145 100644
> --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c
> +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c
> @@ -481,6 +481,67 @@ static void vega12_init_dpm_state(struct vega12_dpm_state *dpm_state)
>         dpm_state->hard_max_level = 0xffff;
>  }
>
> +static int vega12_override_pcie_parameters(struct pp_hwmgr *hwmgr)
> +{
> +       struct amdgpu_device *adev = (struct amdgpu_device *)(hwmgr->adev);
> +       struct vega12_hwmgr *data =
> +                       (struct vega12_hwmgr *)(hwmgr->backend);
> +       uint32_t pcie_gen = 0, pcie_width = 0, smu_pcie_arg, pcie_gen_arg, pcie_width_arg;
> +       PPTable_t *pp_table = &(data->smc_state_table.pp_table);
> +       int i;
> +       int ret;
> +
> +       if (adev->pm.pcie_gen_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN4)
> +               pcie_gen = 3;
> +       else if (adev->pm.pcie_gen_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN3)
> +               pcie_gen = 2;
> +       else if (adev->pm.pcie_gen_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN2)
> +               pcie_gen = 1;
> +       else if (adev->pm.pcie_gen_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN1)
> +               pcie_gen = 0;
> +
> +       if (adev->pm.pcie_mlw_mask & CAIL_PCIE_LINK_WIDTH_SUPPORT_X16)
> +               pcie_width = 6;
> +       else if (adev->pm.pcie_mlw_mask & CAIL_PCIE_LINK_WIDTH_SUPPORT_X12)
> +               pcie_width = 5;
> +       else if (adev->pm.pcie_mlw_mask & CAIL_PCIE_LINK_WIDTH_SUPPORT_X8)
> +               pcie_width = 4;
> +       else if (adev->pm.pcie_mlw_mask & CAIL_PCIE_LINK_WIDTH_SUPPORT_X4)
> +               pcie_width = 3;
> +       else if (adev->pm.pcie_mlw_mask & CAIL_PCIE_LINK_WIDTH_SUPPORT_X2)
> +               pcie_width = 2;
> +       else if (adev->pm.pcie_mlw_mask & CAIL_PCIE_LINK_WIDTH_SUPPORT_X1)
> +               pcie_width = 1;
> +
> +       /* Bit 31:16: LCLK DPM level. 0 is DPM0, and 1 is DPM1
> +        * Bit 15:8:  PCIE GEN, 0 to 3 corresponds to GEN1 to GEN4
> +        * Bit 7:0:   PCIE lane width, 1 to 7 corresponds is x1 to x32
> +        */
> +       for (i = 0; i < NUM_LINK_LEVELS; i++) {
> +               pcie_gen_arg = (pp_table->PcieGenSpeed[i] > pcie_gen) ? pcie_gen :
> +                       pp_table->PcieGenSpeed[i];
> +               pcie_width_arg = (pp_table->PcieLaneCount[i] > pcie_width) ? pcie_width :
> +                       pp_table->PcieLaneCount[i];
> +
> +               if (pcie_gen_arg != pp_table->PcieGenSpeed[i] || pcie_width_arg !=
> +                   pp_table->PcieLaneCount[i]) {
> +                       smu_pcie_arg = (i << 16) | (pcie_gen_arg << 8) | pcie_width_arg;
> +                       ret = smum_send_msg_to_smc_with_parameter(hwmgr,
> +                               PPSMC_MSG_OverridePcieParameters, smu_pcie_arg,
> +                               NULL);
> +                       PP_ASSERT_WITH_CODE(!ret,
> +                               "[OverridePcieParameters] Attempt to override pcie params failed!",
> +                               return ret);
> +               }
> +
> +               /* update the pptable */
> +               pp_table->PcieGenSpeed[i] = pcie_gen_arg;
> +               pp_table->PcieLaneCount[i] = pcie_width_arg;
> +       }
> +
> +       return 0;
> +}
> +
>  static int vega12_get_number_of_dpm_level(struct pp_hwmgr *hwmgr,
>                 PPCLK_e clk_id, uint32_t *num_of_levels)
>  {
> @@ -968,6 +1029,11 @@ static int vega12_enable_dpm_tasks(struct pp_hwmgr *hwmgr)
>                         "Failed to enable all smu features!",
>                         return result);
>
> +       result = vega12_override_pcie_parameters(hwmgr);
> +       PP_ASSERT_WITH_CODE(!result,
> +                       "[EnableDPMTasks] Failed to override pcie parameters!",
> +                       return result);
> +
>         tmp_result = vega12_power_control_set_level(hwmgr);
>         PP_ASSERT_WITH_CODE(!tmp_result,
>                         "Failed to power control set level!",
> diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c
> index e8eec2539c17..78bbd4d666f2 100644
> --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c
> +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c
> @@ -831,7 +831,9 @@ static int vega20_override_pcie_parameters(struct pp_hwmgr *hwmgr)
>         struct amdgpu_device *adev = (struct amdgpu_device *)(hwmgr->adev);
>         struct vega20_hwmgr *data =
>                         (struct vega20_hwmgr *)(hwmgr->backend);
> -       uint32_t pcie_gen = 0, pcie_width = 0, smu_pcie_arg;
> +       uint32_t pcie_gen = 0, pcie_width = 0, smu_pcie_arg, pcie_gen_arg, pcie_width_arg;
> +       PPTable_t *pp_table = &(data->smc_state_table.pp_table);
> +       int i;
>         int ret;
>
>         if (adev->pm.pcie_gen_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN4)
> @@ -860,17 +862,27 @@ static int vega20_override_pcie_parameters(struct pp_hwmgr *hwmgr)
>          * Bit 15:8:  PCIE GEN, 0 to 3 corresponds to GEN1 to GEN4
>          * Bit 7:0:   PCIE lane width, 1 to 7 corresponds is x1 to x32
>          */
> -       smu_pcie_arg = (1 << 16) | (pcie_gen << 8) | pcie_width;
> -       ret = smum_send_msg_to_smc_with_parameter(hwmgr,
> -                       PPSMC_MSG_OverridePcieParameters, smu_pcie_arg,
> -                       NULL);
> -       PP_ASSERT_WITH_CODE(!ret,
> -               "[OverridePcieParameters] Attempt to override pcie params failed!",
> -               return ret);
> +       for (i = 0; i < NUM_LINK_LEVELS; i++) {
> +               pcie_gen_arg = (pp_table->PcieGenSpeed[i] > pcie_gen) ? pcie_gen :
> +                       pp_table->PcieGenSpeed[i];
> +               pcie_width_arg = (pp_table->PcieLaneCount[i] > pcie_width) ? pcie_width :
> +                       pp_table->PcieLaneCount[i];
> +
> +               if (pcie_gen_arg != pp_table->PcieGenSpeed[i] || pcie_width_arg !=
> +                   pp_table->PcieLaneCount[i]) {
> +                       smu_pcie_arg = (i << 16) | (pcie_gen_arg << 8) | pcie_width_arg;
> +                       ret = smum_send_msg_to_smc_with_parameter(hwmgr,
> +                               PPSMC_MSG_OverridePcieParameters, smu_pcie_arg,
> +                               NULL);
> +                       PP_ASSERT_WITH_CODE(!ret,
> +                               "[OverridePcieParameters] Attempt to override pcie params failed!",
> +                               return ret);
> +               }
>
> -       data->pcie_parameters_override = true;
> -       data->pcie_gen_level1 = pcie_gen;
> -       data->pcie_width_level1 = pcie_width;
> +               /* update the pptable */
> +               pp_table->PcieGenSpeed[i] = pcie_gen_arg;
> +               pp_table->PcieLaneCount[i] = pcie_width_arg;
> +       }
>
>         return 0;
>  }
> @@ -3319,9 +3331,7 @@ static int vega20_print_clock_levels(struct pp_hwmgr *hwmgr,
>                         data->od8_settings.od8_settings_array;
>         OverDriveTable_t *od_table =
>                         &(data->smc_state_table.overdrive_table);
> -       struct phm_ppt_v3_information *pptable_information =
> -               (struct phm_ppt_v3_information *)hwmgr->pptable;
> -       PPTable_t *pptable = (PPTable_t *)pptable_information->smc_pptable;
> +       PPTable_t *pptable = &(data->smc_state_table.pp_table);
>         struct pp_clock_levels_with_latency clocks;
>         struct vega20_single_dpm_table *fclk_dpm_table =
>                         &(data->dpm_table.fclk_table);
> @@ -3420,13 +3430,9 @@ static int vega20_print_clock_levels(struct pp_hwmgr *hwmgr,
>                 current_lane_width =
>                         vega20_get_current_pcie_link_width_level(hwmgr);
>                 for (i = 0; i < NUM_LINK_LEVELS; i++) {
> -                       if (i == 1 && data->pcie_parameters_override) {
> -                               gen_speed = data->pcie_gen_level1;
> -                               lane_width = data->pcie_width_level1;
> -                       } else {
> -                               gen_speed = pptable->PcieGenSpeed[i];
> -                               lane_width = pptable->PcieLaneCount[i];
> -                       }
> +                       gen_speed = pptable->PcieGenSpeed[i];
> +                       lane_width = pptable->PcieLaneCount[i];
> +
>                         size += sprintf(buf + size, "%d: %s %s %dMhz %s\n", i,
>                                         (gen_speed == 0) ? "2.5GT/s," :
>                                         (gen_speed == 1) ? "5.0GT/s," :
> --
> 2.17.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


More information about the amd-gfx mailing list