All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lijo Lazar <lijo.lazar@amd.com>
To: David M Nieto <david.nieto@amd.com>, amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/3] drm/amdgpu/pm: add new fields for Navi1x
Date: Wed, 19 May 2021 11:05:47 +0530	[thread overview]
Message-ID: <b03e3953-9a83-36b7-a7a3-3a2afa19c41a@amd.com> (raw)
In-Reply-To: <20210518040957.23266-2-david.nieto@amd.com>



On 5/18/2021 9:39 AM, David M Nieto wrote:
> Fill voltage fields in metrics table
> 
> Signed-off-by: David M Nieto <david.nieto@amd.com>
> ---
>   .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c   | 62 ++++++++++++++-----
>   1 file changed, 45 insertions(+), 17 deletions(-)
> 
> 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 ac13042672ea..9339fd24ae8c 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> @@ -505,7 +505,7 @@ static int navi10_tables_init(struct smu_context *smu)
>   		goto err0_out;
>   	smu_table->metrics_time = 0;
>   
> -	smu_table->gpu_metrics_table_size = sizeof(struct gpu_metrics_v1_1);
> +	smu_table->gpu_metrics_table_size = sizeof(struct gpu_metrics_v1_3);
>   	smu_table->gpu_metrics_table = kzalloc(smu_table->gpu_metrics_table_size, GFP_KERNEL);
>   	if (!smu_table->gpu_metrics_table)
>   		goto err1_out;
> @@ -2627,10 +2627,11 @@ static ssize_t navi10_get_legacy_gpu_metrics(struct smu_context *smu,
>   					     void **table)
>   {
>   	struct smu_table_context *smu_table = &smu->smu_table;
> -	struct gpu_metrics_v1_1 *gpu_metrics =
> -		(struct gpu_metrics_v1_1 *)smu_table->gpu_metrics_table;
> +	struct gpu_metrics_v1_3 *gpu_metrics =
> +		(struct gpu_metrics_v1_3 *)smu_table->gpu_metrics_table;
>   	SmuMetrics_legacy_t metrics;
>   	int ret = 0;
> +	int freq = 0, dpm = 0;

Variables added, seems unused in new code.

>   	mutex_lock(&smu->metrics_lock);
>   
> @@ -2646,7 +2647,7 @@ static ssize_t navi10_get_legacy_gpu_metrics(struct smu_context *smu,
>   
>   	mutex_unlock(&smu->metrics_lock);
>   
> -	smu_cmn_init_soft_gpu_metrics(gpu_metrics, 1, 1);
> +	smu_cmn_init_soft_gpu_metrics(gpu_metrics, 1, 3);
>   
>   	gpu_metrics->temperature_edge = metrics.TemperatureEdge;
>   	gpu_metrics->temperature_hotspot = metrics.TemperatureHotspot;
> @@ -2681,19 +2682,26 @@ static ssize_t navi10_get_legacy_gpu_metrics(struct smu_context *smu,
>   
>   	gpu_metrics->system_clock_counter = ktime_get_boottime_ns();
>   
> +	gpu_metrics->voltage_gfx = (155000 - 625 * metrics.CurrGfxVoltageOffset) / 100;
> +	gpu_metrics->voltage_mem = (155000 - 625 * metrics.CurrMemVidOffset) / 100;
> +	gpu_metrics->voltage_soc = (155000 - 625 * metrics.CurrSocVoltageOffset) / 100;
> +

It's better to add a non-zero check for offset values. Having 0 as 
offset value is unlikely, otherwise it could show the wrong voltage if 
FW is not passing the data.

Same comments for below functions also.

Thanks,
Lijo

>   	*table = (void *)gpu_metrics;
>   
> -	return sizeof(struct gpu_metrics_v1_1);
> +	return sizeof(struct gpu_metrics_v1_3);
> +out:
> +	return ret;
>   }
>   
>   static ssize_t navi10_get_gpu_metrics(struct smu_context *smu,
>   				      void **table)
>   {
>   	struct smu_table_context *smu_table = &smu->smu_table;
> -	struct gpu_metrics_v1_1 *gpu_metrics =
> -		(struct gpu_metrics_v1_1 *)smu_table->gpu_metrics_table;
> +	struct gpu_metrics_v1_3 *gpu_metrics =
> +		(struct gpu_metrics_v1_3 *)smu_table->gpu_metrics_table;
>   	SmuMetrics_t metrics;
>   	int ret = 0;
> +	int freq = 0, dpm = 0;
>   
>   	mutex_lock(&smu->metrics_lock);
>   
> @@ -2709,7 +2717,7 @@ static ssize_t navi10_get_gpu_metrics(struct smu_context *smu,
>   
>   	mutex_unlock(&smu->metrics_lock);
>   
> -	smu_cmn_init_soft_gpu_metrics(gpu_metrics, 1, 1);
> +	smu_cmn_init_soft_gpu_metrics(gpu_metrics, 1, 3);
>   
>   	gpu_metrics->temperature_edge = metrics.TemperatureEdge;
>   	gpu_metrics->temperature_hotspot = metrics.TemperatureHotspot;
> @@ -2746,19 +2754,26 @@ static ssize_t navi10_get_gpu_metrics(struct smu_context *smu,
>   
>   	gpu_metrics->system_clock_counter = ktime_get_boottime_ns();
>   
> +	gpu_metrics->voltage_gfx = (155000 - 625 * metrics.CurrGfxVoltageOffset) / 100;
> +	gpu_metrics->voltage_mem = (155000 - 625 * metrics.CurrMemVidOffset) / 100;
> +	gpu_metrics->voltage_soc = (155000 - 625 * metrics.CurrSocVoltageOffset) / 100;
> +
>   	*table = (void *)gpu_metrics;
>   
> -	return sizeof(struct gpu_metrics_v1_1);
> +	return sizeof(struct gpu_metrics_v1_3);
> +out:
> +	return ret;
>   }
>   
>   static ssize_t navi12_get_legacy_gpu_metrics(struct smu_context *smu,
>   					     void **table)
>   {
>   	struct smu_table_context *smu_table = &smu->smu_table;
> -	struct gpu_metrics_v1_1 *gpu_metrics =
> -		(struct gpu_metrics_v1_1 *)smu_table->gpu_metrics_table;
> +	struct gpu_metrics_v1_3 *gpu_metrics =
> +		(struct gpu_metrics_v1_3 *)smu_table->gpu_metrics_table;
>   	SmuMetrics_NV12_legacy_t metrics;
>   	int ret = 0;
> +	int freq = 0, dpm = 0;
>   
>   	mutex_lock(&smu->metrics_lock);
>   
> @@ -2774,7 +2789,7 @@ static ssize_t navi12_get_legacy_gpu_metrics(struct smu_context *smu,
>   
>   	mutex_unlock(&smu->metrics_lock);
>   
> -	smu_cmn_init_soft_gpu_metrics(gpu_metrics, 1, 1);
> +	smu_cmn_init_soft_gpu_metrics(gpu_metrics, 1, 3);
>   
>   	gpu_metrics->temperature_edge = metrics.TemperatureEdge;
>   	gpu_metrics->temperature_hotspot = metrics.TemperatureHotspot;
> @@ -2814,19 +2829,26 @@ static ssize_t navi12_get_legacy_gpu_metrics(struct smu_context *smu,
>   
>   	gpu_metrics->system_clock_counter = ktime_get_boottime_ns();
>   
> +	gpu_metrics->voltage_gfx = (155000 - 625 * metrics.CurrGfxVoltageOffset) / 100;
> +	gpu_metrics->voltage_mem = (155000 - 625 * metrics.CurrMemVidOffset) / 100;
> +	gpu_metrics->voltage_soc = (155000 - 625 * metrics.CurrSocVoltageOffset) / 100;
> +
>   	*table = (void *)gpu_metrics;
>   
> -	return sizeof(struct gpu_metrics_v1_1);
> +	return sizeof(struct gpu_metrics_v1_3);
> +out:
> +	return ret;
>   }
>   
>   static ssize_t navi12_get_gpu_metrics(struct smu_context *smu,
>   				      void **table)
>   {
>   	struct smu_table_context *smu_table = &smu->smu_table;
> -	struct gpu_metrics_v1_1 *gpu_metrics =
> -		(struct gpu_metrics_v1_1 *)smu_table->gpu_metrics_table;
> +	struct gpu_metrics_v1_3 *gpu_metrics =
> +		(struct gpu_metrics_v1_3 *)smu_table->gpu_metrics_table;
>   	SmuMetrics_NV12_t metrics;
>   	int ret = 0;
> +	int freq = 0, dpm = 0;
>   
>   	mutex_lock(&smu->metrics_lock);
>   
> @@ -2842,7 +2864,7 @@ static ssize_t navi12_get_gpu_metrics(struct smu_context *smu,
>   
>   	mutex_unlock(&smu->metrics_lock);
>   
> -	smu_cmn_init_soft_gpu_metrics(gpu_metrics, 1, 1);
> +	smu_cmn_init_soft_gpu_metrics(gpu_metrics, 1, 3);
>   
>   	gpu_metrics->temperature_edge = metrics.TemperatureEdge;
>   	gpu_metrics->temperature_hotspot = metrics.TemperatureHotspot;
> @@ -2884,9 +2906,15 @@ static ssize_t navi12_get_gpu_metrics(struct smu_context *smu,
>   
>   	gpu_metrics->system_clock_counter = ktime_get_boottime_ns();
>   
> +	gpu_metrics->voltage_gfx = (155000 - 625 * metrics.CurrGfxVoltageOffset) / 100;
> +	gpu_metrics->voltage_mem = (155000 - 625 * metrics.CurrMemVidOffset) / 100;
> +	gpu_metrics->voltage_soc = (155000 - 625 * metrics.CurrSocVoltageOffset) / 100;
> +
>   	*table = (void *)gpu_metrics;
>   
> -	return sizeof(struct gpu_metrics_v1_1);
> +	return sizeof(struct gpu_metrics_v1_3);
> +out:
> +	return ret;
>   }
>   
>   static ssize_t navi1x_get_gpu_metrics(struct smu_context *smu,
> 

-- 
Thanks,
Lijo
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

  reply	other threads:[~2021-05-19  5:36 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-14 21:01 [PATCH 1/2] drm/amdgpu/pm: Update metrics table David M Nieto
2021-05-14 21:01 ` [PATCH 2/2] drm/amdgpu/pm: add new fields for Navi1x David M Nieto
2021-05-17  6:28   ` Lazar, Lijo
2021-05-17 20:06     ` Nieto, David M
2021-05-18  4:09     ` [PATCH 1/3] drm/amdgpu/pm: Update metrics table David M Nieto
2021-05-18  4:09       ` [PATCH 2/3] drm/amdgpu/pm: add new fields for Navi1x David M Nieto
2021-05-19  5:35         ` Lijo Lazar [this message]
2021-05-18  4:09       ` [PATCH 3/3] drm/amdgpu/pm: display vcn pp dpm David M Nieto
2021-05-19  5:43         ` Lijo Lazar
2021-05-19  6:02           ` [PATCH 1/3] drm/amdgpu/pm: Update metrics table David M Nieto
2021-05-19  6:02             ` [PATCH 2/3] drm/amdgpu/pm: add new fields for Navi1x David M Nieto
2021-05-19 15:43               ` Lijo Lazar
2021-05-19  6:02             ` [PATCH 3/3] drm/amdgpu/pm: display vcn pp dpm David M Nieto
2021-05-19 15:44               ` Lijo Lazar
2021-05-19 15:42             ` [PATCH 1/3] drm/amdgpu/pm: Update metrics table Lijo Lazar
2021-05-19 17:39           ` [PATCH 1/3] drm/amdgpu/pm: Update metrics table (v2) David M Nieto
2021-05-19 17:39             ` [PATCH 2/3] drm/amdgpu/pm: add new fields for Navi1x (v3) David M Nieto
2021-05-19 17:39             ` [PATCH 3/3] drm/amdgpu/pm: display vcn pp dpm (v3) David M Nieto
2021-05-20  5:16           ` [PATCH] drm/amdgpu/pm: display vcn pp dpm (v4) David M Nieto

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=b03e3953-9a83-36b7-a7a3-3a2afa19c41a@amd.com \
    --to=lijo.lazar@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=david.nieto@amd.com \
    /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.