[PATCH v3 4/8] drm/amd/pm: Add navi1x throttler translation

Quan, Evan Evan.Quan at amd.com
Fri Jun 4 02:17:11 UTC 2021


[AMD Official Use Only]

Thanks Lijo and Graham. Yes, I know that only some specific ASICs support VR_MEM1 and LIQUID1.
However, the problem is about the design:
1. should we just list those which are commonly supported by all ASICs.
2. Or we list all the possible throttlers and mask out those unsupported for some specific ASICs

BR
Evan
> -----Original Message-----
> From: Lazar, Lijo <Lijo.Lazar at amd.com>
> Sent: Thursday, June 3, 2021 10:01 PM
> To: Sider, Graham <Graham.Sider at amd.com>; Quan, Evan
> <Evan.Quan at amd.com>; amd-gfx at lists.freedesktop.org
> Cc: Kasiviswanathan, Harish <Harish.Kasiviswanathan at amd.com>;
> Sakhnovitch, Elena (Elen) <Elena.Sakhnovitch at amd.com>
> Subject: RE: [PATCH v3 4/8] drm/amd/pm: Add navi1x throttler translation
> 
> [AMD Official Use Only]
> 
> VR_*0/1 reflect the throttle status of separate voltage rails - availability of
> both depends on board and FW capability to query their temperature.
> 
> Thanks,
> Lijo
> 
> -----Original Message-----
> From: Sider, Graham <Graham.Sider at amd.com>
> Sent: Thursday, June 3, 2021 6:41 PM
> To: Quan, Evan <Evan.Quan at amd.com>; amd-gfx at lists.freedesktop.org
> Cc: Lazar, Lijo <Lijo.Lazar at amd.com>; Kasiviswanathan, Harish
> <Harish.Kasiviswanathan at amd.com>; Sakhnovitch, Elena (Elen)
> <Elena.Sakhnovitch at amd.com>
> Subject: RE: [PATCH v3 4/8] drm/amd/pm: Add navi1x throttler translation
> 
> Some ASICs use a single VR_MEM bit, whereas others split it into VR_MEM0
> and VR_MEM1. To avoid confusion, we've combined the VR_MEM0 and
> VR_MEM1 bits on those ASICs. For consistency we did the same with
> LIQUID0 and LIQUID1.
> 
> -----Original Message-----
> From: Quan, Evan <Evan.Quan at amd.com>
> Sent: Wednesday, June 2, 2021 12:37 AM
> To: Sider, Graham <Graham.Sider at amd.com>; amd-
> gfx at lists.freedesktop.org
> Cc: Lazar, Lijo <Lijo.Lazar at amd.com>; Kasiviswanathan, Harish
> <Harish.Kasiviswanathan at amd.com>; Sider, Graham
> <Graham.Sider at amd.com>; Sakhnovitch, Elena (Elen)
> <Elena.Sakhnovitch at amd.com>
> Subject: RE: [PATCH v3 4/8] drm/amd/pm: Add navi1x throttler translation
> 
> [AMD Official Use Only]
> 
> 
> 
> > -----Original Message-----
> > From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of
> > Graham Sider
> > Sent: Wednesday, June 2, 2021 2:12 AM
> > To: amd-gfx at lists.freedesktop.org
> > Cc: Lazar, Lijo <Lijo.Lazar at amd.com>; Kasiviswanathan, Harish
> > <Harish.Kasiviswanathan at amd.com>; Sider, Graham
> > <Graham.Sider at amd.com>; Sakhnovitch, Elena (Elen)
> > <Elena.Sakhnovitch at amd.com>
> > Subject: [PATCH v3 4/8] drm/amd/pm: Add navi1x throttler translation
> >
> > Perform dependent to independent throttle status translation for
> > navi1x. Makes use of lookup table navi1x_throttler_map.
> >
> > Signed-off-by: Graham Sider <Graham.Sider at amd.com>
> > ---
> >  .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c   | 43
> > +++++++++++++++++++
> >  1 file changed, 43 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> > b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> > index 78fe13183e8b..bf376b1be08d 100644
> > --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> > +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> > @@ -238,6 +238,28 @@ static struct cmn2asic_mapping
> > navi10_workload_map[PP_SMC_POWER_PROFILE_COUNT] =
> >  	WORKLOAD_MAP(PP_SMC_POWER_PROFILE_CUSTOM,
> > 	WORKLOAD_PPLIB_CUSTOM_BIT),
> >  };
> >
> > +static const uint8_t navi1x_throttler_map[] = {
> > +	[THROTTLER_TEMP_EDGE_BIT]	=
> > (SMU_THROTTLER_TEMP_EDGE_BIT),
> > +	[THROTTLER_TEMP_HOTSPOT_BIT]	=
> > (SMU_THROTTLER_TEMP_HOTSPOT_BIT),
> > +	[THROTTLER_TEMP_MEM_BIT]	=
> > (SMU_THROTTLER_TEMP_MEM_BIT),
> > +	[THROTTLER_TEMP_VR_GFX_BIT]	=
> > (SMU_THROTTLER_TEMP_VR_GFX_BIT),
> > +	[THROTTLER_TEMP_VR_MEM0_BIT]	=
> > (SMU_THROTTLER_TEMP_VR_MEM_BIT),
> > +	[THROTTLER_TEMP_VR_MEM1_BIT]	=
> > (SMU_THROTTLER_TEMP_VR_MEM_BIT),
> [Quan, Evan] I'm wondering why you map the two ASIC dependent bits to
> the same non ASIC independent bit. Instead of defining two non ASIC
> independent bits.
> > +	[THROTTLER_TEMP_VR_SOC_BIT]	=
> > (SMU_THROTTLER_TEMP_VR_SOC_BIT),
> > +	[THROTTLER_TEMP_LIQUID0_BIT]	=
> > (SMU_THROTTLER_TEMP_LIQUID_BIT),
> > +	[THROTTLER_TEMP_LIQUID1_BIT]	=
> > (SMU_THROTTLER_TEMP_LIQUID_BIT),
> [Quan, Evan] Same question here and for Patch4.
> 
> BR
> Evan
> > +	[THROTTLER_TDC_GFX_BIT]		=
> > (SMU_THROTTLER_TDC_GFX_BIT),
> > +	[THROTTLER_TDC_SOC_BIT]		=
> > (SMU_THROTTLER_TDC_SOC_BIT),
> > +	[THROTTLER_PPT0_BIT]		=
> > (SMU_THROTTLER_PPT0_BIT),
> > +	[THROTTLER_PPT1_BIT]		=
> > (SMU_THROTTLER_PPT1_BIT),
> > +	[THROTTLER_PPT2_BIT]		=
> > (SMU_THROTTLER_PPT2_BIT),
> > +	[THROTTLER_PPT3_BIT]		=
> > (SMU_THROTTLER_PPT3_BIT),
> > +	[THROTTLER_FIT_BIT]		= (SMU_THROTTLER_FIT_BIT),
> > +	[THROTTLER_PPM_BIT]		=
> > (SMU_THROTTLER_PPM_BIT),
> > +	[THROTTLER_APCC_BIT]		=
> > (SMU_THROTTLER_APCC_BIT),
> > +};
> > +
> > +
> >  static bool is_asic_secure(struct smu_context *smu)  {
> >  	struct amdgpu_device *adev = smu->adev; @@ -524,6 +546,19 @@
> static
> > int navi10_tables_init(struct smu_context
> > *smu)
> >  	return -ENOMEM;
> >  }
> >
> > +static uint64_t navi1x_get_indep_throttler_status(
> > +					const unsigned long dep_status)
> > +{
> > +	uint64_t indep_status = 0;
> > +	uint8_t dep_bit = 0;
> > +
> > +	for_each_set_bit(dep_bit, &dep_status, 32)
> > +		indep_status |= smu_u64_throttler_bit(dep_status,
> > +			navi1x_throttler_map[dep_bit], dep_bit);
> > +
> > +	return indep_status;
> > +}
> > +
> >  static int navi10_get_legacy_smu_metrics_data(struct smu_context *smu,
> >  					      MetricsMember_t member,
> >  					      uint32_t *value)
> > @@ -2673,6 +2708,8 @@ static ssize_t
> > navi10_get_legacy_gpu_metrics(struct smu_context *smu,
> >  	gpu_metrics->current_dclk0 = metrics.CurrClock[PPCLK_DCLK];
> >
> >  	gpu_metrics->throttle_status = metrics.ThrottlerStatus;
> > +	gpu_metrics->indep_throttle_status =
> > +
> > 	navi1x_get_indep_throttler_status(metrics.ThrottlerStatus);
> >
> >  	gpu_metrics->current_fan_speed = metrics.CurrFanSpeed;
> >
> > @@ -2750,6 +2787,8 @@ static ssize_t navi10_get_gpu_metrics(struct
> > smu_context *smu,
> >  	gpu_metrics->current_dclk0 = metrics.CurrClock[PPCLK_DCLK];
> >
> >  	gpu_metrics->throttle_status = metrics.ThrottlerStatus;
> > +	gpu_metrics->indep_throttle_status =
> > +
> > 	navi1x_get_indep_throttler_status(metrics.ThrottlerStatus);
> >
> >  	gpu_metrics->current_fan_speed = metrics.CurrFanSpeed;
> >
> > @@ -2826,6 +2865,8 @@ static ssize_t
> > navi12_get_legacy_gpu_metrics(struct smu_context *smu,
> >  	gpu_metrics->current_dclk0 = metrics.CurrClock[PPCLK_DCLK];
> >
> >  	gpu_metrics->throttle_status = metrics.ThrottlerStatus;
> > +	gpu_metrics->indep_throttle_status =
> > +
> > 	navi1x_get_indep_throttler_status(metrics.ThrottlerStatus);
> >
> >  	gpu_metrics->current_fan_speed = metrics.CurrFanSpeed;
> >
> > @@ -2908,6 +2949,8 @@ static ssize_t navi12_get_gpu_metrics(struct
> > smu_context *smu,
> >  	gpu_metrics->current_dclk0 = metrics.CurrClock[PPCLK_DCLK];
> >
> >  	gpu_metrics->throttle_status = metrics.ThrottlerStatus;
> > +	gpu_metrics->indep_throttle_status =
> > +
> > 	navi1x_get_indep_throttler_status(metrics.ThrottlerStatus);
> >
> >  	gpu_metrics->current_fan_speed = metrics.CurrFanSpeed;
> >
> > --
> > 2.17.1
> >
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx at lists.freedesktop.org
> >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.
> > freedesktop.org%2Fmailman%2Flistinfo%2Famd-
> >
> gfx&data=04%7C01%7Cevan.quan%40amd.com%7Cf05ba28afbe0417ac
> >
> 54008d925290dc0%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63
> >
> 7581680520671680%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMD
> >
> AiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=
> >
> PzZzTHlRh0ygXIJdQeN8%2Ff4ojC9KcCy4Ia5POPGw1nI%3D&reserved=0


More information about the amd-gfx mailing list