<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<style type="text/css" style="display:none;"><!-- P {margin-top:0;margin-bottom:0;} --></style>
</head>
<body dir="ltr">
<div id="divtagdefaultwrapper" style="font-size:12pt;color:#000000;font-family:Calibri,Helvetica,sans-serif;" dir="ltr">
<p style="margin-top:0;margin-bottom:0">Looks good.</p>
<p style="margin-top:0;margin-bottom:0">Reviewed-by: Alex Deucher <alexander.deucher@amd.com></p>
<p style="margin-top:0;margin-bottom:0">Please also send a patch to bump the kernel driver version so the UMDs know when the query is available.</p>
<p style="margin-top:0;margin-bottom:0"><br>
</p>
<p style="margin-top:0;margin-bottom:0">Thanks!</p>
<p style="margin-top:0;margin-bottom:0"><br>
</p>
<p style="margin-top:0;margin-bottom:0">Alex<br>
</p>
</div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Zhu, Rex <Rex.Zhu@amd.com><br>
<b>Sent:</b> Wednesday, January 17, 2018 12:48:48 AM<br>
<b>To:</b> 'Alex Deucher'<br>
<b>Cc:</b> amd-gfx list<br>
<b>Subject:</b> RE: [PATCH 2/3] drm/amd/pp: Add stable Pstate clk display when print_clock_levels</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt;">
<div class="PlainText">Ok, I will drop this patch. and please review the new attached patch expose stable pstate clock by ioctl.<br>
<br>
Best Regards<br>
Rex<br>
<br>
-----Original Message-----<br>
From: Alex Deucher [<a href="mailto:alexdeucher@gmail.com">mailto:alexdeucher@gmail.com</a>]
<br>
Sent: Wednesday, January 17, 2018 12:03 AM<br>
To: Zhu, Rex<br>
Cc: amd-gfx list<br>
Subject: Re: [PATCH 2/3] drm/amd/pp: Add stable Pstate clk display when print_clock_levels<br>
<br>
On Tue, Jan 16, 2018 at 6:59 AM, Rex Zhu <Rex.Zhu@amd.com> wrote:<br>
> The additional output are at the end of sclk/mclk info as cat <br>
> pp_dpm_mclk<br>
> 0: 300Mhz *<br>
> 1: 1650Mhz<br>
> P: 300Mhz<br>
><br>
> Signed-off-by: Rex Zhu <Rex.Zhu@amd.com><br>
<br>
I'm not crazy about this patch.  I think it conflates things and breaks older versions of the rocm smi tool.  I'd prefer to add a new file for the pstate clocks or add a query to the amdgpu INFO ioctl to fetch them.<br>
<br>
Alex<br>
<br>
<br>
><br>
> Change-Id: Idf8eeedb5d399d9ffb7de7a2fb2976c7fa7c01a8<br>
> ---<br>
>  drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c     | 2 ++<br>
>  drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c     | 2 ++<br>
>  drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c   | 2 ++<br>
>  drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 2 ++<br>
>  4 files changed, 8 insertions(+)<br>
><br>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c <br>
> b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c<br>
> index f68dd08..03dfba0 100644<br>
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c<br>
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c<br>
> @@ -1601,6 +1601,7 @@ static int cz_print_clock_levels(struct pp_hwmgr *hwmgr,<br>
>                         size += sprintf(buf + size, "%d: %uMhz %s\n",<br>
>                                         i, sclk_table->entries[i].clk / 100,<br>
>                                         (i == now) ? "*" : "");<br>
> +               size += sprintf(buf + size, "P: %uMhz\n", <br>
> + hwmgr->pstate_sclk/100);<br>
>                 break;<br>
>         case PP_MCLK:<br>
>                 now = <br>
> PHM_GET_FIELD(cgs_read_ind_register(hwmgr->device,<br>
> @@ -1613,6 +1614,7 @@ static int cz_print_clock_levels(struct pp_hwmgr *hwmgr,<br>
>                         size += sprintf(buf + size, "%d: %uMhz %s\n",<br>
>                                         CZ_NUM_NBPMEMORYCLOCK-i, data->sys_info.nbp_memory_clock[i-1] / 100,<br>
>                                         (CZ_NUM_NBPMEMORYCLOCK-i == <br>
> now) ? "*" : "");<br>
> +               size += sprintf(buf + size, "P: %uMhz\n", <br>
> + hwmgr->pstate_mclk/100);<br>
>                 break;<br>
>         default:<br>
>                 break;<br>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c <br>
> b/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c<br>
> index 409a56b..88c6ad8 100644<br>
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c<br>
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c<br>
> @@ -756,6 +756,7 @@ static int rv_print_clock_levels(struct pp_hwmgr *hwmgr,<br>
>                                 data->gfx_max_freq_limit / 100,<br>
>                                 ((data->gfx_max_freq_limit / 100)<br>
>                                  == now) ? "*" : "");<br>
> +               size += sprintf(buf + size, "P: %uMhz\n", <br>
> + hwmgr->pstate_sclk/100);<br>
>                 break;<br>
>         case PP_MCLK:<br>
>                 PP_ASSERT_WITH_CODE(!smum_send_msg_to_smc(hwmgr,<br>
> @@ -773,6 +774,7 @@ static int rv_print_clock_levels(struct pp_hwmgr *hwmgr,<br>
>                                         mclk_table->entries[i].clk / 100,<br>
>                                         ((mclk_table->entries[i].clk / 100)<br>
>                                          == now) ? "*" : "");<br>
> +               size += sprintf(buf + size, "P: %uMhz\n", <br>
> + hwmgr->pstate_mclk/100);<br>
>                 break;<br>
>         default:<br>
>                 break;<br>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c <br>
> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c<br>
> index 11a900b..6f053fa 100644<br>
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c<br>
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c<br>
> @@ -4301,6 +4301,7 @@ static int smu7_print_clock_levels(struct pp_hwmgr *hwmgr,<br>
>                         size += sprintf(buf + size, "%d: %uMhz %s\n",<br>
>                                         i, sclk_table->dpm_levels[i].value / 100,<br>
>                                         (i == now) ? "*" : "");<br>
> +               size += sprintf(buf + size, "P: %uMhz\n", <br>
> + hwmgr->pstate_sclk/100);<br>
>                 break;<br>
>         case PP_MCLK:<br>
>                 smum_send_msg_to_smc(hwmgr, <br>
> PPSMC_MSG_API_GetMclkFrequency); @@ -4317,6 +4318,7 @@ static int smu7_print_clock_levels(struct pp_hwmgr *hwmgr,<br>
>                         size += sprintf(buf + size, "%d: %uMhz %s\n",<br>
>                                         i, mclk_table->dpm_levels[i].value / 100,<br>
>                                         (i == now) ? "*" : "");<br>
> +               size += sprintf(buf + size, "P: %uMhz\n", <br>
> + hwmgr->pstate_mclk/100);<br>
>                 break;<br>
>         case PP_PCIE:<br>
>                 pcie_speed = smu7_get_current_pcie_speed(hwmgr);<br>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c <br>
> b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c<br>
> index adfbbc1..d646b27b 100644<br>
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c<br>
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c<br>
> @@ -4571,6 +4571,7 @@ static int vega10_print_clock_levels(struct pp_hwmgr *hwmgr,<br>
>                         size += sprintf(buf + size, "%d: %uMhz %s\n",<br>
>                                         i, sclk_table->dpm_levels[i].value / 100,<br>
>                                         (i == now) ? "*" : "");<br>
> +               size += sprintf(buf + size, "P: %uMhz\n", <br>
> + hwmgr->pstate_sclk/100);<br>
>                 break;<br>
>         case PP_MCLK:<br>
>                 if (data->registry_data.mclk_dpm_key_disabled)<br>
> @@ -4589,6 +4590,7 @@ static int vega10_print_clock_levels(struct pp_hwmgr *hwmgr,<br>
>                         size += sprintf(buf + size, "%d: %uMhz %s\n",<br>
>                                         i, mclk_table->dpm_levels[i].value / 100,<br>
>                                         (i == now) ? "*" : "");<br>
> +               size += sprintf(buf + size, "P: %uMhz\n", <br>
> + hwmgr->pstate_mclk/100);<br>
>                 break;<br>
>         case PP_PCIE:<br>
>                 PP_ASSERT_WITH_CODE(!smum_send_msg_to_smc(hwmgr,<br>
> --<br>
> 1.9.1<br>
><br>
> _______________________________________________<br>
> amd-gfx mailing list<br>
> amd-gfx@lists.freedesktop.org<br>
> <a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a><br>
</div>
</span></font></div>
</body>
</html>