Re: [PATCH 5/9] drm/amd/powerplay: clean up the APIs for pptable setup

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jun 1, 2020 at 3:31 AM Evan Quan <evan.quan@xxxxxxx> wrote:
>
> Combine and simplify the logics for setup pptable.
>
> Change-Id: I062f15eab586050593afd960432c4c70fbdd5d41
> Signed-off-by: Evan Quan <evan.quan@xxxxxxx>

Acked-by: Alex Deucher <alexander.deucher@xxxxxxx>

> ---
>  drivers/gpu/drm/amd/powerplay/amdgpu_smu.c    | 17 ----
>  drivers/gpu/drm/amd/powerplay/arcturus_ppt.c  | 66 ++++++++-----
>  .../gpu/drm/amd/powerplay/inc/amdgpu_smu.h    |  5 -
>  drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h |  4 -
>  drivers/gpu/drm/amd/powerplay/navi10_ppt.c    | 92 ++++++++++---------
>  drivers/gpu/drm/amd/powerplay/smu_internal.h  | 10 --
>  drivers/gpu/drm/amd/powerplay/smu_v11_0.c     | 21 -----
>  7 files changed, 89 insertions(+), 126 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index 9bafa6b3e123..b079ac6325d0 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -1132,23 +1132,6 @@ static int smu_smc_table_hw_init(struct smu_context *smu,
>                 if (ret)
>                         return ret;
>
> -               /*
> -                * check if the format_revision in vbios is up to pptable header
> -                * version, and the structure size is not 0.
> -                */
> -               ret = smu_check_pptable(smu);
> -               if (ret)
> -                       return ret;
> -
> -               /*
> -                * Parse pptable format and fill PPTable_t smc_pptable to
> -                * smu_table_context structure. And read the smc_dpm_table from vbios,
> -                * then fill it into smc_pptable.
> -                */
> -               ret = smu_parse_pptable(smu);
> -               if (ret)
> -                       return ret;
> -
>                 /*
>                  * Send msg GetDriverIfVersion to check if the return value is equal
>                  * with DRIVER_IF_VERSION of smc header.
> diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> index 902c8cfa4a3b..c5c23126ec2d 100644
> --- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> @@ -487,33 +487,33 @@ static int arcturus_set_default_dpm_table(struct smu_context *smu)
>
>  static int arcturus_check_powerplay_table(struct smu_context *smu)
>  {
> +       struct smu_table_context *table_context = &smu->smu_table;
> +       struct smu_11_0_powerplay_table *powerplay_table =
> +               table_context->power_play_table;
> +       struct smu_baco_context *smu_baco = &smu->smu_baco;
> +
> +       mutex_lock(&smu_baco->mutex);
> +       if (powerplay_table->platform_caps & SMU_11_0_PP_PLATFORM_CAP_BACO ||
> +           powerplay_table->platform_caps & SMU_11_0_PP_PLATFORM_CAP_MACO)
> +               smu_baco->platform_support = true;
> +       mutex_unlock(&smu_baco->mutex);
> +
> +       table_context->thermal_controller_type =
> +               powerplay_table->thermal_controller_type;
> +
>         return 0;
>  }
>
>  static int arcturus_store_powerplay_table(struct smu_context *smu)
>  {
> -       struct smu_11_0_powerplay_table *powerplay_table = NULL;
>         struct smu_table_context *table_context = &smu->smu_table;
> -       struct smu_baco_context *smu_baco = &smu->smu_baco;
> -       int ret = 0;
> -
> -       if (!table_context->power_play_table)
> -               return -EINVAL;
> -
> -       powerplay_table = table_context->power_play_table;
> +       struct smu_11_0_powerplay_table *powerplay_table =
> +               table_context->power_play_table;
>
>         memcpy(table_context->driver_pptable, &powerplay_table->smc_pptable,
>                sizeof(PPTable_t));
>
> -       table_context->thermal_controller_type = powerplay_table->thermal_controller_type;
> -
> -       mutex_lock(&smu_baco->mutex);
> -       if (powerplay_table->platform_caps & SMU_11_0_PP_PLATFORM_CAP_BACO ||
> -           powerplay_table->platform_caps & SMU_11_0_PP_PLATFORM_CAP_MACO)
> -               smu_baco->platform_support = true;
> -       mutex_unlock(&smu_baco->mutex);
> -
> -       return ret;
> +       return 0;
>  }
>
>  static int arcturus_append_powerplay_table(struct smu_context *smu)
> @@ -544,6 +544,29 @@ static int arcturus_append_powerplay_table(struct smu_context *smu)
>         return 0;
>  }
>
> +static int arcturus_setup_pptable(struct smu_context *smu)
> +{
> +       int ret = 0;
> +
> +       ret = smu_v11_0_setup_pptable(smu);
> +       if (ret)
> +               return ret;
> +
> +       ret = arcturus_store_powerplay_table(smu);
> +       if (ret)
> +               return ret;
> +
> +       ret = arcturus_append_powerplay_table(smu);
> +       if (ret)
> +               return ret;
> +
> +       ret = arcturus_check_powerplay_table(smu);
> +       if (ret)
> +               return ret;
> +
> +       return ret;
> +}
> +
>  static int arcturus_run_btc(struct smu_context *smu)
>  {
>         int ret = 0;
> @@ -2383,10 +2406,6 @@ static const struct pptable_funcs arcturus_ppt_funcs = {
>         /* internal structurs allocations */
>         .tables_init = arcturus_tables_init,
>         .alloc_dpm_context = arcturus_allocate_dpm_context,
> -       /* pptable related */
> -       .check_powerplay_table = arcturus_check_powerplay_table,
> -       .store_powerplay_table = arcturus_store_powerplay_table,
> -       .append_powerplay_table = arcturus_append_powerplay_table,
>         /* init dpm */
>         .get_allowed_feature_mask = arcturus_get_allowed_feature_mask,
>         /* btc */
> @@ -2421,10 +2440,9 @@ static const struct pptable_funcs arcturus_ppt_funcs = {
>         .init_power = smu_v11_0_init_power,
>         .fini_power = smu_v11_0_fini_power,
>         .check_fw_status = smu_v11_0_check_fw_status,
> -       .setup_pptable = smu_v11_0_setup_pptable,
> +       /* pptable related */
> +       .setup_pptable = arcturus_setup_pptable,
>         .get_vbios_bootup_values = smu_v11_0_get_vbios_bootup_values,
> -       .check_pptable = smu_v11_0_check_pptable,
> -       .parse_pptable = smu_v11_0_parse_pptable,
>         .populate_smc_tables = smu_v11_0_populate_smc_pptable,
>         .check_fw_version = smu_v11_0_check_fw_version,
>         .write_pptable = smu_v11_0_write_pptable,
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> index 223678e329a5..14f4a850b553 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> @@ -417,9 +417,6 @@ struct i2c_adapter;
>
>  struct pptable_funcs {
>         int (*alloc_dpm_context)(struct smu_context *smu);
> -       int (*store_powerplay_table)(struct smu_context *smu);
> -       int (*check_powerplay_table)(struct smu_context *smu);
> -       int (*append_powerplay_table)(struct smu_context *smu);
>         int (*get_smu_msg_index)(struct smu_context *smu, uint32_t index);
>         int (*get_smu_clk_index)(struct smu_context *smu, uint32_t index);
>         int (*get_smu_feature_index)(struct smu_context *smu, uint32_t index);
> @@ -505,8 +502,6 @@ struct pptable_funcs {
>         int (*check_fw_status)(struct smu_context *smu);
>         int (*setup_pptable)(struct smu_context *smu);
>         int (*get_vbios_bootup_values)(struct smu_context *smu);
> -       int (*check_pptable)(struct smu_context *smu);
> -       int (*parse_pptable)(struct smu_context *smu);
>         int (*populate_smc_tables)(struct smu_context *smu);
>         int (*check_fw_version)(struct smu_context *smu);
>         int (*powergate_sdma)(struct smu_context *smu, bool gate);
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h b/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
> index 5b785816aa64..51868dc33238 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
> @@ -161,10 +161,6 @@ int smu_v11_0_setup_pptable(struct smu_context *smu);
>
>  int smu_v11_0_get_vbios_bootup_values(struct smu_context *smu);
>
> -int smu_v11_0_check_pptable(struct smu_context *smu);
> -
> -int smu_v11_0_parse_pptable(struct smu_context *smu);
> -
>  int smu_v11_0_populate_smc_pptable(struct smu_context *smu);
>
>  int smu_v11_0_check_fw_version(struct smu_context *smu);
> diff --git a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> index bea6a96b5afb..db38fb10524d 100644
> --- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> @@ -417,6 +417,29 @@ navi10_get_allowed_feature_mask(struct smu_context *smu,
>
>  static int navi10_check_powerplay_table(struct smu_context *smu)
>  {
> +       struct smu_table_context *table_context = &smu->smu_table;
> +       struct smu_11_0_powerplay_table *powerplay_table =
> +               table_context->power_play_table;
> +       struct smu_baco_context *smu_baco = &smu->smu_baco;
> +
> +       if (powerplay_table->platform_caps & SMU_11_0_PP_PLATFORM_CAP_HARDWAREDC)
> +               smu->dc_controlled_by_gpio = true;
> +
> +       mutex_lock(&smu_baco->mutex);
> +       if (powerplay_table->platform_caps & SMU_11_0_PP_PLATFORM_CAP_BACO ||
> +           powerplay_table->platform_caps & SMU_11_0_PP_PLATFORM_CAP_MACO)
> +               smu_baco->platform_support = true;
> +       mutex_unlock(&smu_baco->mutex);
> +
> +       table_context->thermal_controller_type =
> +               powerplay_table->thermal_controller_type;
> +
> +       /*
> +        * Instead of having its own buffer space and get overdrive_table copied,
> +        * smu->od_settings just points to the actual overdrive_table
> +        */
> +       smu->od_settings = &powerplay_table->overdrive_table;
> +
>         return 0;
>  }
>
> @@ -475,30 +498,37 @@ static int navi10_append_powerplay_table(struct smu_context *smu)
>
>  static int navi10_store_powerplay_table(struct smu_context *smu)
>  {
> -       struct smu_11_0_powerplay_table *powerplay_table = NULL;
>         struct smu_table_context *table_context = &smu->smu_table;
> -       struct smu_baco_context *smu_baco = &smu->smu_baco;
> -
> -       if (!table_context->power_play_table)
> -               return -EINVAL;
> -
> -       powerplay_table = table_context->power_play_table;
> +       struct smu_11_0_powerplay_table *powerplay_table =
> +               table_context->power_play_table;
>
>         memcpy(table_context->driver_pptable, &powerplay_table->smc_pptable,
>                sizeof(PPTable_t));
>
> -       table_context->thermal_controller_type = powerplay_table->thermal_controller_type;
> +       return 0;
> +}
>
> -       if (powerplay_table->platform_caps & SMU_11_0_PP_PLATFORM_CAP_HARDWAREDC)
> -               smu->dc_controlled_by_gpio = true;
> +static int navi10_setup_pptable(struct smu_context *smu)
> +{
> +       int ret = 0;
>
> -       mutex_lock(&smu_baco->mutex);
> -       if (powerplay_table->platform_caps & SMU_11_0_PP_PLATFORM_CAP_BACO ||
> -           powerplay_table->platform_caps & SMU_11_0_PP_PLATFORM_CAP_MACO)
> -               smu_baco->platform_support = true;
> -       mutex_unlock(&smu_baco->mutex);
> +       ret = smu_v11_0_setup_pptable(smu);
> +       if (ret)
> +               return ret;
>
> -       return 0;
> +       ret = navi10_store_powerplay_table(smu);
> +       if (ret)
> +               return ret;
> +
> +       ret = navi10_append_powerplay_table(smu);
> +       if (ret)
> +               return ret;
> +
> +       ret = navi10_check_powerplay_table(smu);
> +       if (ret)
> +               return ret;
> +
> +       return ret;
>  }
>
>  static int navi10_tables_init(struct smu_context *smu, struct smu_table *tables)
> @@ -1927,24 +1957,6 @@ static int navi10_overdrive_get_gfx_clk_base_voltage(struct smu_context *smu,
>         return 0;
>  }
>
> -static int navi10_setup_od_limits(struct smu_context *smu) {
> -       struct smu_11_0_overdrive_table *overdrive_table = NULL;
> -       struct smu_11_0_powerplay_table *powerplay_table = NULL;
> -
> -       if (!smu->smu_table.power_play_table) {
> -               pr_err("powerplay table uninitialized!\n");
> -               return -ENOENT;
> -       }
> -       powerplay_table = (struct smu_11_0_powerplay_table *)smu->smu_table.power_play_table;
> -       overdrive_table = &powerplay_table->overdrive_table;
> -       if (!smu->od_settings) {
> -               smu->od_settings = kmemdup(overdrive_table, sizeof(struct smu_11_0_overdrive_table), GFP_KERNEL);
> -       } else {
> -               memcpy(smu->od_settings, overdrive_table, sizeof(struct smu_11_0_overdrive_table));
> -       }
> -       return 0;
> -}
> -
>  static bool navi10_is_baco_supported(struct smu_context *smu)
>  {
>         struct amdgpu_device *adev = smu->adev;
> @@ -1968,11 +1980,6 @@ static int navi10_set_default_od_settings(struct smu_context *smu, bool initiali
>         od_table = (OverDriveTable_t *)smu->smu_table.overdrive_table;
>         boot_od_table = (OverDriveTable_t *)smu->smu_table.boot_overdrive_table;
>         if (initialize) {
> -               ret = navi10_setup_od_limits(smu);
> -               if (ret) {
> -                       pr_err("Failed to retrieve board OD limits\n");
> -                       return ret;
> -               }
>                 if (od_table) {
>                         if (!od_table->GfxclkVolt1) {
>                                 ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
> @@ -2274,9 +2281,6 @@ static int navi10_disable_umc_cdr_12gbps_workaround(struct smu_context *smu)
>  static const struct pptable_funcs navi10_ppt_funcs = {
>         .tables_init = navi10_tables_init,
>         .alloc_dpm_context = navi10_allocate_dpm_context,
> -       .store_powerplay_table = navi10_store_powerplay_table,
> -       .check_powerplay_table = navi10_check_powerplay_table,
> -       .append_powerplay_table = navi10_append_powerplay_table,
>         .get_smu_msg_index = navi10_get_smu_msg_index,
>         .get_smu_clk_index = navi10_get_smu_clk_index,
>         .get_smu_feature_index = navi10_get_smu_feature_index,
> @@ -2318,10 +2322,8 @@ static const struct pptable_funcs navi10_ppt_funcs = {
>         .init_power = smu_v11_0_init_power,
>         .fini_power = smu_v11_0_fini_power,
>         .check_fw_status = smu_v11_0_check_fw_status,
> -       .setup_pptable = smu_v11_0_setup_pptable,
> +       .setup_pptable = navi10_setup_pptable,
>         .get_vbios_bootup_values = smu_v11_0_get_vbios_bootup_values,
> -       .check_pptable = smu_v11_0_check_pptable,
> -       .parse_pptable = smu_v11_0_parse_pptable,
>         .populate_smc_tables = smu_v11_0_populate_smc_pptable,
>         .check_fw_version = smu_v11_0_check_fw_version,
>         .write_pptable = smu_v11_0_write_pptable,
> diff --git a/drivers/gpu/drm/amd/powerplay/smu_internal.h b/drivers/gpu/drm/amd/powerplay/smu_internal.h
> index a31df7f4e91a..33086f94267a 100644
> --- a/drivers/gpu/drm/amd/powerplay/smu_internal.h
> +++ b/drivers/gpu/drm/amd/powerplay/smu_internal.h
> @@ -47,10 +47,6 @@
>
>  #define smu_get_vbios_bootup_values(smu) \
>         ((smu)->ppt_funcs->get_vbios_bootup_values ? (smu)->ppt_funcs->get_vbios_bootup_values((smu)) : 0)
> -#define smu_check_pptable(smu) \
> -       ((smu)->ppt_funcs->check_pptable ? (smu)->ppt_funcs->check_pptable((smu)) : 0)
> -#define smu_parse_pptable(smu) \
> -       ((smu)->ppt_funcs->parse_pptable ? (smu)->ppt_funcs->parse_pptable((smu)) : 0)
>  #define smu_populate_smc_tables(smu) \
>         ((smu)->ppt_funcs->populate_smc_tables ? (smu)->ppt_funcs->populate_smc_tables((smu)) : 0)
>  #define smu_check_fw_version(smu) \
> @@ -96,12 +92,6 @@ static inline int smu_send_smc_msg(struct smu_context *smu, enum smu_message_typ
>         ((smu)->ppt_funcs->is_dpm_running ? (smu)->ppt_funcs->is_dpm_running((smu)) : 0)
>  #define smu_notify_display_change(smu) \
>         ((smu)->ppt_funcs->notify_display_change? (smu)->ppt_funcs->notify_display_change((smu)) : 0)
> -#define smu_store_powerplay_table(smu) \
> -       ((smu)->ppt_funcs->store_powerplay_table ? (smu)->ppt_funcs->store_powerplay_table((smu)) : 0)
> -#define smu_check_powerplay_table(smu) \
> -       ((smu)->ppt_funcs->check_powerplay_table ? (smu)->ppt_funcs->check_powerplay_table((smu)) : 0)
> -#define smu_append_powerplay_table(smu) \
> -       ((smu)->ppt_funcs->append_powerplay_table ? (smu)->ppt_funcs->append_powerplay_table((smu)) : 0)
>  #define smu_set_default_dpm_table(smu) \
>         ((smu)->ppt_funcs->set_default_dpm_table ? (smu)->ppt_funcs->set_default_dpm_table((smu)) : 0)
>  #define smu_populate_umd_state_clk(smu) \
> diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> index be6dca8c6014..7a97a4510c6d 100644
> --- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> +++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> @@ -725,27 +725,6 @@ int smu_v11_0_notify_memory_pool_location(struct smu_context *smu)
>         return ret;
>  }
>
> -int smu_v11_0_check_pptable(struct smu_context *smu)
> -{
> -       int ret;
> -
> -       ret = smu_check_powerplay_table(smu);
> -       return ret;
> -}
> -
> -int smu_v11_0_parse_pptable(struct smu_context *smu)
> -{
> -       int ret;
> -
> -       ret = smu_store_powerplay_table(smu);
> -       if (ret)
> -               return -EINVAL;
> -
> -       ret = smu_append_powerplay_table(smu);
> -
> -       return ret;
> -}
> -
>  int smu_v11_0_populate_smc_pptable(struct smu_context *smu)
>  {
>         int ret;
> --
> 2.26.2
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux