Thanks Evan and Lijo. Keep in mind that the ASIC dependent DWORD with those bits is still being kept. That said, I have no problem with listing them out separately in the new field as well. I'll make the ASICs that don't support VR_MEM1/LIQUID1 map to VR_MEM0/LIQUID0 and not touch the *1 bits. If you have a problem with this approach let me know.
Best,
Graham
-----Original Message-----
From: Lazar, Lijo <Lijo.Lazar@xxxxxxx>
Sent: Friday, June 4, 2021 12:52 AM
To: Quan, Evan <Evan.Quan@xxxxxxx>; Sider, Graham <Graham.Sider@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Cc: Kasiviswanathan, Harish <Harish.Kasiviswanathan@xxxxxxx>; Sakhnovitch, Elena (Elen) <Elena.Sakhnovitch@xxxxxxx>
Subject: RE: [PATCH v3 4/8] drm/amd/pm: Add navi1x throttler translation
[AMD Official Use Only]
A modified version of 2 -
List all the possible ones and merge those which mean the same - ex: terminology changes like THM and TEMP.
In the mail earlier, I meant to list them out separately as the intention is to convey the throttle reason to the user- it's better to point out the exact regulator which is heating up.
Thanks,
Lijo
-----Original Message-----
From: Quan, Evan <Evan.Quan@xxxxxxx>
Sent: Friday, June 4, 2021 7:47 AM
To: Lazar, Lijo <Lijo.Lazar@xxxxxxx>; Sider, Graham <Graham.Sider@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Cc: Kasiviswanathan, Harish <Harish.Kasiviswanathan@xxxxxxx>; Sakhnovitch, Elena (Elen) <Elena.Sakhnovitch@xxxxxxx>
Subject: RE: [PATCH v3 4/8] drm/amd/pm: Add navi1x throttler translation
[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@xxxxxxx>
> Sent: Thursday, June 3, 2021 10:01 PM
> To: Sider, Graham <Graham.Sider@xxxxxxx>; Quan, Evan
> <Evan.Quan@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Kasiviswanathan, Harish <Harish.Kasiviswanathan@xxxxxxx>;
> Sakhnovitch, Elena (Elen) <Elena.Sakhnovitch@xxxxxxx>
> 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@xxxxxxx>
> Sent: Thursday, June 3, 2021 6:41 PM
> To: Quan, Evan <Evan.Quan@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Lazar, Lijo <Lijo.Lazar@xxxxxxx>; Kasiviswanathan, Harish
> <Harish.Kasiviswanathan@xxxxxxx>; Sakhnovitch, Elena (Elen)
> <Elena.Sakhnovitch@xxxxxxx>
> 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@xxxxxxx>
> Sent: Wednesday, June 2, 2021 12:37 AM
> To: Sider, Graham <Graham.Sider@xxxxxxx>; amd-
> gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Lazar, Lijo <Lijo.Lazar@xxxxxxx>; Kasiviswanathan, Harish
> <Harish.Kasiviswanathan@xxxxxxx>; Sider, Graham
> <Graham.Sider@xxxxxxx>; Sakhnovitch, Elena (Elen)
> <Elena.Sakhnovitch@xxxxxxx>
> 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@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of
> > Graham Sider
> > Sent: Wednesday, June 2, 2021 2:12 AM
> > To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> > Cc: Lazar, Lijo <Lijo.Lazar@xxxxxxx>; Kasiviswanathan, Harish
> > <Harish.Kasiviswanathan@xxxxxxx>; Sider, Graham
> > <Graham.Sider@xxxxxxx>; Sakhnovitch, Elena (Elen)
> > <Elena.Sakhnovitch@xxxxxxx>
> > 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@xxxxxxx>
> > ---
> > .../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@xxxxxxxxxxxxxxxxxxxxx
> >
> 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
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
