[AMD Official Use Only]
> -----Original Message-----
> From: Lazar, Lijo <Lijo.Lazar@xxxxxxx>
> Sent: Tuesday, November 30, 2021 8:22 PM
> To: Quan, Evan <Evan.Quan@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Koenig, Christian
> <Christian.Koenig@xxxxxxx>; Feng, Kenneth <Kenneth.Feng@xxxxxxx>
> Subject: Re: [PATCH V2 05/17] drm/amd/pm: do not expose those APIs used
> internally only in si_dpm.c
>
>
>
> On 11/30/2021 1:12 PM, Evan Quan wrote:
> > Move them to si_dpm.c instead.
> >
> > Signed-off-by: Evan Quan <evan.quan@xxxxxxx>
> > Change-Id: I288205cfd7c6ba09cfb22626ff70360d61ff0c67
> > --
> > v1->v2:
> > - rename the API with "si_" prefix(Alex)
> > ---
> > drivers/gpu/drm/amd/pm/amdgpu_dpm.c | 25 -----------
> > drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h | 25 -----------
> > drivers/gpu/drm/amd/pm/powerplay/si_dpm.c | 54
> +++++++++++++++++++----
> > drivers/gpu/drm/amd/pm/powerplay/si_dpm.h | 7 +++
> > 4 files changed, 53 insertions(+), 58 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
> > b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
> > index 52ac3c883a6e..fbfc07a83122 100644
> > --- a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
> > +++ b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
> > @@ -894,31 +894,6 @@ void amdgpu_add_thermal_controller(struct
> amdgpu_device *adev)
> > }
> > }
> >
> > -enum amdgpu_pcie_gen amdgpu_get_pcie_gen_support(struct
> amdgpu_device *adev,
> > - u32 sys_mask,
> > - enum amdgpu_pcie_gen
> asic_gen,
> > - enum amdgpu_pcie_gen
> default_gen)
> > -{
> > - switch (asic_gen) {
> > - case AMDGPU_PCIE_GEN1:
> > - return AMDGPU_PCIE_GEN1;
> > - case AMDGPU_PCIE_GEN2:
> > - return AMDGPU_PCIE_GEN2;
> > - case AMDGPU_PCIE_GEN3:
> > - return AMDGPU_PCIE_GEN3;
> > - default:
> > - if ((sys_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN3)
> &&
> > - (default_gen == AMDGPU_PCIE_GEN3))
> > - return AMDGPU_PCIE_GEN3;
> > - else if ((sys_mask &
> CAIL_PCIE_LINK_SPEED_SUPPORT_GEN2) &&
> > - (default_gen == AMDGPU_PCIE_GEN2))
> > - return AMDGPU_PCIE_GEN2;
> > - else
> > - return AMDGPU_PCIE_GEN1;
> > - }
> > - return AMDGPU_PCIE_GEN1;
> > -}
> > -
> > struct amd_vce_state*
> > amdgpu_get_vce_clock_state(void *handle, u32 idx)
> > {
> > diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> > b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> > index 6681b878e75f..f43b96dfe9d8 100644
> > --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> > +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> > @@ -45,19 +45,6 @@ enum amdgpu_int_thermal_type {
> > THERMAL_TYPE_KV,
> > };
> >
> > -enum amdgpu_dpm_auto_throttle_src {
> > - AMDGPU_DPM_AUTO_THROTTLE_SRC_THERMAL,
> > - AMDGPU_DPM_AUTO_THROTTLE_SRC_EXTERNAL
> > -};
> > -
> > -enum amdgpu_dpm_event_src {
> > - AMDGPU_DPM_EVENT_SRC_ANALOG = 0,
> > - AMDGPU_DPM_EVENT_SRC_EXTERNAL = 1,
> > - AMDGPU_DPM_EVENT_SRC_DIGITAL = 2,
> > - AMDGPU_DPM_EVENT_SRC_ANALOG_OR_EXTERNAL = 3,
> > - AMDGPU_DPM_EVENT_SRC_DIGIAL_OR_EXTERNAL = 4
> > -};
> > -
> > struct amdgpu_ps {
> > u32 caps; /* vbios flags */
> > u32 class; /* vbios flags */
> > @@ -252,13 +239,6 @@ struct amdgpu_dpm_fan {
> > bool ucode_fan_control;
> > };
> >
> > -enum amdgpu_pcie_gen {
> > - AMDGPU_PCIE_GEN1 = 0,
> > - AMDGPU_PCIE_GEN2 = 1,
> > - AMDGPU_PCIE_GEN3 = 2,
> > - AMDGPU_PCIE_GEN_INVALID = 0xffff
> > -};
> > -
> > #define amdgpu_dpm_reset_power_profile_state(adev, request) \
> > ((adev)->powerplay.pp_funcs-
> >reset_power_profile_state(\
> > (adev)->powerplay.pp_handle, request)) @@ -
> 403,11 +383,6 @@ void
> > amdgpu_free_extended_power_table(struct amdgpu_device *adev);
> >
> > void amdgpu_add_thermal_controller(struct amdgpu_device *adev);
> >
> > -enum amdgpu_pcie_gen amdgpu_get_pcie_gen_support(struct
> amdgpu_device *adev,
> > - u32 sys_mask,
> > - enum amdgpu_pcie_gen
> asic_gen,
> > - enum amdgpu_pcie_gen
> default_gen);
> > -
> > struct amd_vce_state*
> > amdgpu_get_vce_clock_state(void *handle, u32 idx);
> >
> > diff --git a/drivers/gpu/drm/amd/pm/powerplay/si_dpm.c
> > b/drivers/gpu/drm/amd/pm/powerplay/si_dpm.c
> > index 81f82aa05ec2..4f84d8b893f1 100644
> > --- a/drivers/gpu/drm/amd/pm/powerplay/si_dpm.c
> > +++ b/drivers/gpu/drm/amd/pm/powerplay/si_dpm.c
> > @@ -96,6 +96,19 @@ union pplib_clock_info {
> > struct _ATOM_PPLIB_SI_CLOCK_INFO si;
> > };
> >
> > +enum amdgpu_dpm_auto_throttle_src {
> > + AMDGPU_DPM_AUTO_THROTTLE_SRC_THERMAL,
> > + AMDGPU_DPM_AUTO_THROTTLE_SRC_EXTERNAL
> > +};
> > +
> > +enum amdgpu_dpm_event_src {
> > + AMDGPU_DPM_EVENT_SRC_ANALOG = 0,
> > + AMDGPU_DPM_EVENT_SRC_EXTERNAL = 1,
> > + AMDGPU_DPM_EVENT_SRC_DIGITAL = 2,
> > + AMDGPU_DPM_EVENT_SRC_ANALOG_OR_EXTERNAL = 3,
> > + AMDGPU_DPM_EVENT_SRC_DIGIAL_OR_EXTERNAL = 4 };
> > +
>
> Better to rename the enums also including amdgpu_pcie_gen if they are
> used only within si_dpm.
[Quan, Evan] Sure, will do that.
Thanks
Evan
>
> Thanks,
> Lijo
>
> > static const u32 r600_utc[R600_PM_NUMBER_OF_TC] =
> > {
> > R600_UTC_DFLT_00,
> > @@ -4927,6 +4940,31 @@ static int si_populate_smc_initial_state(struct
> amdgpu_device *adev,
> > return 0;
> > }
> >
> > +static enum amdgpu_pcie_gen si_gen_pcie_gen_support(struct
> amdgpu_device *adev,
> > + u32 sys_mask,
> > + enum amdgpu_pcie_gen
> asic_gen,
> > + enum amdgpu_pcie_gen
> default_gen) {
> > + switch (asic_gen) {
> > + case AMDGPU_PCIE_GEN1:
> > + return AMDGPU_PCIE_GEN1;
> > + case AMDGPU_PCIE_GEN2:
> > + return AMDGPU_PCIE_GEN2;
> > + case AMDGPU_PCIE_GEN3:
> > + return AMDGPU_PCIE_GEN3;
> > + default:
> > + if ((sys_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN3)
> &&
> > + (default_gen == AMDGPU_PCIE_GEN3))
> > + return AMDGPU_PCIE_GEN3;
> > + else if ((sys_mask &
> CAIL_PCIE_LINK_SPEED_SUPPORT_GEN2) &&
> > + (default_gen == AMDGPU_PCIE_GEN2))
> > + return AMDGPU_PCIE_GEN2;
> > + else
> > + return AMDGPU_PCIE_GEN1;
> > + }
> > + return AMDGPU_PCIE_GEN1;
> > +}
> > +
> > static int si_populate_smc_acpi_state(struct amdgpu_device *adev,
> > SISLANDS_SMC_STATETABLE *table)
> > {
> > @@ -4989,10 +5027,10 @@ static int si_populate_smc_acpi_state(struct
> amdgpu_device *adev,
> > &table-
> >ACPIState.level.std_vddc);
> > }
> > table->ACPIState.level.gen2PCIE =
> > - (u8)amdgpu_get_pcie_gen_support(adev,
> > - si_pi->sys_pcie_mask,
> > - si_pi->boot_pcie_gen,
> > -
> AMDGPU_PCIE_GEN1);
> > + (u8)si_gen_pcie_gen_support(adev,
> > + si_pi->sys_pcie_mask,
> > + si_pi->boot_pcie_gen,
> > + AMDGPU_PCIE_GEN1);
> >
> > if (si_pi->vddc_phase_shed_control)
> > si_populate_phase_shedding_value(adev,
> > @@ -7148,10 +7186,10 @@ static void si_parse_pplib_clock_info(struct
> amdgpu_device *adev,
> > pl->vddc = le16_to_cpu(clock_info->si.usVDDC);
> > pl->vddci = le16_to_cpu(clock_info->si.usVDDCI);
> > pl->flags = le32_to_cpu(clock_info->si.ulFlags);
> > - pl->pcie_gen = amdgpu_get_pcie_gen_support(adev,
> > - si_pi->sys_pcie_mask,
> > - si_pi->boot_pcie_gen,
> > - clock_info->si.ucPCIEGen);
> > + pl->pcie_gen = si_gen_pcie_gen_support(adev,
> > + si_pi->sys_pcie_mask,
> > + si_pi->boot_pcie_gen,
> > + clock_info->si.ucPCIEGen);
> >
> > /* patch up vddc if necessary */
> > ret = si_get_leakage_voltage_from_leakage_index(adev, pl->vddc,
> > diff --git a/drivers/gpu/drm/amd/pm/powerplay/si_dpm.h
> > b/drivers/gpu/drm/amd/pm/powerplay/si_dpm.h
> > index bc0be6818e21..8c267682eeef 100644
> > --- a/drivers/gpu/drm/amd/pm/powerplay/si_dpm.h
> > +++ b/drivers/gpu/drm/amd/pm/powerplay/si_dpm.h
> > @@ -595,6 +595,13 @@ struct rv7xx_power_info {
> > RV770_SMC_STATETABLE smc_statetable;
> > };
> >
> > +enum amdgpu_pcie_gen {
> > + AMDGPU_PCIE_GEN1 = 0,
> > + AMDGPU_PCIE_GEN2 = 1,
> > + AMDGPU_PCIE_GEN3 = 2,
> > + AMDGPU_PCIE_GEN_INVALID = 0xffff
> > +};
> > +
> > struct rv7xx_pl {
> > u32 sclk;
> > u32 mclk;
> >
