All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Lazar, Lijo" <Lijo.Lazar@amd.com>
To: "Quan, Evan" <Evan.Quan@amd.com>,
	"Sider, Graham" <Graham.Sider@amd.com>,
	 "amd-gfx@lists.freedesktop.org" <amd-gfx@lists.freedesktop.org>
Cc: "Kasiviswanathan, Harish" <Harish.Kasiviswanathan@amd.com>,
	"Sakhnovitch,  Elena \(Elen\)" <Elena.Sakhnovitch@amd.com>
Subject: RE: [PATCH v3 4/8] drm/amd/pm: Add navi1x throttler translation
Date: Fri, 4 Jun 2021 04:51:52 +0000	[thread overview]
Message-ID: <CH0PR12MB5348CA39DC370EEDE3817C99973B9@CH0PR12MB5348.namprd12.prod.outlook.com> (raw)
In-Reply-To: <DM6PR12MB26197ADDF59681FD0902780DE43B9@DM6PR12MB2619.namprd12.prod.outlook.com>

[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@amd.com> 
Sent: Friday, June 4, 2021 7:47 AM
To: Lazar, Lijo <Lijo.Lazar@amd.com>; Sider, Graham <Graham.Sider@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Kasiviswanathan, Harish <Harish.Kasiviswanathan@amd.com>; Sakhnovitch, Elena (Elen) <Elena.Sakhnovitch@amd.com>
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@amd.com>
> Sent: Thursday, June 3, 2021 10:01 PM
> To: Sider, Graham <Graham.Sider@amd.com>; Quan, Evan 
> <Evan.Quan@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Kasiviswanathan, Harish <Harish.Kasiviswanathan@amd.com>; 
> Sakhnovitch, Elena (Elen) <Elena.Sakhnovitch@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@amd.com>
> Sent: Thursday, June 3, 2021 6:41 PM
> To: Quan, Evan <Evan.Quan@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Lazar, Lijo <Lijo.Lazar@amd.com>; Kasiviswanathan, Harish 
> <Harish.Kasiviswanathan@amd.com>; Sakhnovitch, Elena (Elen) 
> <Elena.Sakhnovitch@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@amd.com>
> Sent: Wednesday, June 2, 2021 12:37 AM
> To: Sider, Graham <Graham.Sider@amd.com>; amd- 
> gfx@lists.freedesktop.org
> Cc: Lazar, Lijo <Lijo.Lazar@amd.com>; Kasiviswanathan, Harish 
> <Harish.Kasiviswanathan@amd.com>; Sider, Graham 
> <Graham.Sider@amd.com>; Sakhnovitch, Elena (Elen) 
> <Elena.Sakhnovitch@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@lists.freedesktop.org> On Behalf Of 
> > Graham Sider
> > Sent: Wednesday, June 2, 2021 2:12 AM
> > To: amd-gfx@lists.freedesktop.org
> > Cc: Lazar, Lijo <Lijo.Lazar@amd.com>; Kasiviswanathan, Harish 
> > <Harish.Kasiviswanathan@amd.com>; Sider, Graham 
> > <Graham.Sider@amd.com>; Sakhnovitch, Elena (Elen) 
> > <Elena.Sakhnovitch@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@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@lists.freedesktop.org
> >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.
> > freedesktop.org%2Fmailman%2Flistinfo%2Famd-
> >
> gfx&amp;data=04%7C01%7Cevan.quan%40amd.com%7Cf05ba28afbe0417ac
> >
> 54008d925290dc0%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63
> >
> 7581680520671680%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMD
> >
> AiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=
> >
> PzZzTHlRh0ygXIJdQeN8%2Ff4ojC9KcCy4Ia5POPGw1nI%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

  reply	other threads:[~2021-06-04  4:51 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-01 18:12 [PATCH v3 1/8] drm/amd/pm: Add u64 throttler status field to gpu_metrics Graham Sider
2021-06-01 18:12 ` [PATCH v3 2/8] drm/amd/pm: Add ASIC independent throttle bits Graham Sider
2021-06-01 18:12 ` [PATCH v3 3/8] drm/amd/pm: Add arcturus throttler translation Graham Sider
2021-06-02  4:55   ` Lazar, Lijo
2021-06-01 18:12 ` [PATCH v3 4/8] drm/amd/pm: Add navi1x " Graham Sider
2021-06-02  4:37   ` Quan, Evan
2021-06-03 13:10     ` Sider, Graham
2021-06-03 14:00       ` Lazar, Lijo
2021-06-04  2:17         ` Quan, Evan
2021-06-04  4:51           ` Lazar, Lijo [this message]
2021-06-04 14:21             ` Sider, Graham
2021-06-01 18:12 ` [PATCH v3 5/8] drm/amd/pm: Add sienna cichlid " Graham Sider
2021-06-01 18:12 ` [PATCH v3 6/8] drm/amd/pm: Add vangogh " Graham Sider
2021-06-01 18:12 ` [PATCH v3 7/8] drm/amd/pm: Add renoir " Graham Sider
2021-06-01 18:12 ` [PATCH v3 8/8] drm/amd/pm: Add aldebaran " Graham Sider

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CH0PR12MB5348CA39DC370EEDE3817C99973B9@CH0PR12MB5348.namprd12.prod.outlook.com \
    --to=lijo.lazar@amd.com \
    --cc=Elena.Sakhnovitch@amd.com \
    --cc=Evan.Quan@amd.com \
    --cc=Graham.Sider@amd.com \
    --cc=Harish.Kasiviswanathan@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.