<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<!--[if !mso]><style>v\:* {behavior:url(#default#VML);}
o\:* {behavior:url(#default#VML);}
w\:* {behavior:url(#default#VML);}
.shape {behavior:url(#default#VML);}
</style><![endif]--><style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:DengXian;
panose-1:2 1 6 0 3 1 1 1 1 1;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:"\@DengXian";
panose-1:2 1 6 0 3 1 1 1 1 1;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:#0563C1;
text-decoration:underline;}
span.EmailStyle18
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:windowtext;}
p.msipheadera4477989, li.msipheadera4477989, div.msipheadera4477989
{mso-style-name:msipheadera4477989;
mso-margin-top-alt:auto;
margin-right:0in;
mso-margin-bottom-alt:auto;
margin-left:0in;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="#0563C1" vlink="#954F72" style="word-wrap:break-word">
<div class="WordSection1">
<p class="msipheadera4477989" style="margin:0in"><span style="font-size:10.0pt;font-family:"Arial",sans-serif;color:blue">[AMD Official Use Only]</span><o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Hi Alex & Lijo,<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">For now, those code for updating OD table are in smu_cmn.c. And they can only be called from _ppt.c.<o:p></o:p></p>
<p class="MsoNormal">So, unless we split some code from smu_cmn.c and move them to upper layer(suggested by Lijo before), otherwise this cannot be performed.<o:p></o:p></p>
<p class="MsoNormal">So, definitely we need some follow up patches if we want this done in a more generic way.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">BR<o:p></o:p></p>
<p class="MsoNormal">Evan<o:p></o:p></p>
<div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt">
<div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b>From:</b> Deucher, Alexander <Alexander.Deucher@amd.com> <br>
<b>Sent:</b> Friday, July 23, 2021 9:36 PM<br>
<b>To:</b> Lazar, Lijo <Lijo.Lazar@amd.com>; Quan, Evan <Evan.Quan@amd.com>; amd-gfx@lists.freedesktop.org<br>
<b>Subject:</b> Re: [PATCH V2 1/2] drm/amd/pm: restore user customized OD settings properly for NV1x<o:p></o:p></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p style="margin:5.0pt"><span style="font-size:10.0pt;font-family:"Arial",sans-serif;color:blue">[AMD Official Use Only]<o:p></o:p></span></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal"><span style="font-size:12.0pt;color:black">I haven't had a chance to look at the patches too closely, but if it could be done in a generic may, that makes sense to me. Maybe as a follow up patch?<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:12.0pt;color:black"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:12.0pt;color:black">Alex<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:12.0pt;color:black"><o:p> </o:p></span></p>
</div>
<div class="MsoNormal" align="center" style="text-align:center">
<hr size="2" width="98%" align="center">
</div>
<div id="divRplyFwdMsg">
<p class="MsoNormal"><b><span style="color:black">From:</span></b><span style="color:black"> Lazar, Lijo <<a href="mailto:Lijo.Lazar@amd.com">Lijo.Lazar@amd.com</a>><br>
<b>Sent:</b> Friday, July 23, 2021 6:09 AM<br>
<b>To:</b> Quan, Evan <<a href="mailto:Evan.Quan@amd.com">Evan.Quan@amd.com</a>>;
<a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> <<a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a>><br>
<b>Cc:</b> Deucher, Alexander <<a href="mailto:Alexander.Deucher@amd.com">Alexander.Deucher@amd.com</a>><br>
<b>Subject:</b> Re: [PATCH V2 1/2] drm/amd/pm: restore user customized OD settings properly for NV1x</span>
<o:p></o:p></p>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal">The series looks good to me, though I prefer to use a common logic to
<br>
restore od settings so that smuv12,smuv13 gets the restore feature by <br>
default once they add the user table logic. Don't have strong argument <br>
for it unless Alex, Kenneth or others have some comments.<br>
<br>
Anyway, the series is<br>
Reviewed-by: Lijo Lazar <<a href="mailto:lijo.lazar@amd.com">lijo.lazar@amd.com</a>><br>
<br>
On 7/23/2021 2:39 PM, Evan Quan wrote:<br>
> The customized OD settings can be divided into two parts: those<br>
> committed ones and non-committed ones.<br>
> - For those changes which had been fed to SMU before S3/S4/Runpm<br>
> suspend kicked, they are committed changes. They should be properly<br>
> restored and fed to SMU on S3/S4/Runpm resume.<br>
> - For those non-committed changes, they are restored only without feeding<br>
> to SMU.<br>
> <br>
> Change-Id: Iea7cf7908dfcd919a4d0205e10bff91b1149a440<br>
> Signed-off-by: Evan Quan <<a href="mailto:evan.quan@amd.com">evan.quan@amd.com</a>><br>
> --<br>
> v1->v2<br>
> - better naming and logic revised for checking OD setting update(Lijo)<br>
> ---<br>
> drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h | 8 +++<br>
> drivers/gpu/drm/amd/pm/inc/smu_v11_0.h | 2 +<br>
> drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 9 +++<br>
> .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 55 +++++++++++++------<br>
> .../gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c | 25 +++++++++<br>
> 5 files changed, 82 insertions(+), 17 deletions(-)<br>
> <br>
> diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h<br>
> index 3e89852e4820..c2c201b8e3cf 100644<br>
> --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h<br>
> +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h<br>
> @@ -231,6 +231,7 @@ struct smu_user_dpm_profile {<br>
> uint32_t power_limit;<br>
> uint32_t fan_speed_percent;<br>
> uint32_t flags;<br>
> + uint32_t user_od;<br>
> <br>
> /* user clock state information */<br>
> uint32_t clk_mask[SMU_CLK_COUNT];<br>
> @@ -352,6 +353,7 @@ struct smu_table_context<br>
> <br>
> void *overdrive_table;<br>
> void *boot_overdrive_table;<br>
> + void *user_overdrive_table;<br>
> <br>
> uint32_t gpu_metrics_table_size;<br>
> void *gpu_metrics_table;<br>
> @@ -623,6 +625,12 @@ struct pptable_funcs {<br>
> enum PP_OD_DPM_TABLE_COMMAND type,<br>
> long *input, uint32_t size);<br>
> <br>
> + /**<br>
> + * @restore_user_od_settings: Restore the user customized<br>
> + * OD settings on S3/S4/Runpm resume.<br>
> + */<br>
> + int (*restore_user_od_settings)(struct smu_context *smu);<br>
> +<br>
> /**<br>
> * @get_clock_by_type_with_latency: Get the speed and latency of a clock<br>
> * domain.<br>
> diff --git a/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h b/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h<br>
> index 385b2ea5379c..1e42aafbb9fd 100644<br>
> --- a/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h<br>
> +++ b/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h<br>
> @@ -302,5 +302,7 @@ void smu_v11_0_interrupt_work(struct smu_context *smu);<br>
> <br>
> int smu_v11_0_set_light_sbr(struct smu_context *smu, bool enable);<br>
> <br>
> +int smu_v11_0_restore_user_od_settings(struct smu_context *smu);<br>
> +<br>
> #endif<br>
> #endif<br>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c<br>
> index ebe672142808..8ca7337ea5fc 100644<br>
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c<br>
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c<br>
> @@ -416,6 +416,15 @@ static void smu_restore_dpm_user_profile(struct smu_context *smu)<br>
> }<br>
> }<br>
> <br>
> + /* Restore user customized OD settings */<br>
> + if (smu->user_dpm_profile.user_od) {<br>
> + if (smu->ppt_funcs->restore_user_od_settings) {<br>
> + ret = smu->ppt_funcs->restore_user_od_settings(smu);<br>
> + if (ret)<br>
> + dev_err(smu->adev->dev, "Failed to upload customized OD settings\n");<br>
> + }<br>
> + }<br>
> +<br>
> /* Disable restore flag */<br>
> smu->user_dpm_profile.flags &= ~SMU_DPM_USER_PROFILE_RESTORE;<br>
> }<br>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c<br>
> index 59ea59acfb00..d7722c229ddd 100644<br>
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c<br>
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c<br>
> @@ -2294,41 +2294,52 @@ static int navi10_set_default_od_settings(struct smu_context *smu)<br>
> (OverDriveTable_t *)smu->smu_table.overdrive_table;<br>
> OverDriveTable_t *boot_od_table =<br>
> (OverDriveTable_t *)smu->smu_table.boot_overdrive_table;<br>
> + OverDriveTable_t *user_od_table =<br>
> + (OverDriveTable_t *)smu->smu_table.user_overdrive_table;<br>
> int ret = 0;<br>
> <br>
> - ret = smu_cmn_update_table(smu, SMU_TABLE_OVERDRIVE, 0, (void *)od_table, false);<br>
> + /*<br>
> + * For S3/S4/Runpm resume, no need to setup those overdrive tables again as<br>
> + * - either they already have the default OD settings got during cold bootup<br>
> + * - or they have some user customized OD settings which cannot be overwritten<br>
> + */<br>
> + if (smu->adev->in_suspend)<br>
> + return 0;<br>
> +<br>
> + ret = smu_cmn_update_table(smu, SMU_TABLE_OVERDRIVE, 0, (void *)boot_od_table, false);<br>
> if (ret) {<br>
> dev_err(smu->adev->dev, "Failed to get overdrive table!\n");<br>
> return ret;<br>
> }<br>
> <br>
> - if (!od_table->GfxclkVolt1) {<br>
> + if (!boot_od_table->GfxclkVolt1) {<br>
> ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,<br>
> - &od_table->GfxclkVolt1,<br>
> - od_table->GfxclkFreq1);<br>
> + &boot_od_table->GfxclkVolt1,<br>
> + boot_od_table->GfxclkFreq1);<br>
> if (ret)<br>
> return ret;<br>
> }<br>
> <br>
> - if (!od_table->GfxclkVolt2) {<br>
> + if (!boot_od_table->GfxclkVolt2) {<br>
> ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,<br>
> - &od_table->GfxclkVolt2,<br>
> - od_table->GfxclkFreq2);<br>
> + &boot_od_table->GfxclkVolt2,<br>
> + boot_od_table->GfxclkFreq2);<br>
> if (ret)<br>
> return ret;<br>
> }<br>
> <br>
> - if (!od_table->GfxclkVolt3) {<br>
> + if (!boot_od_table->GfxclkVolt3) {<br>
> ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,<br>
> - &od_table->GfxclkVolt3,<br>
> - od_table->GfxclkFreq3);<br>
> + &boot_od_table->GfxclkVolt3,<br>
> + boot_od_table->GfxclkFreq3);<br>
> if (ret)<br>
> return ret;<br>
> }<br>
> <br>
> - memcpy(boot_od_table, od_table, sizeof(OverDriveTable_t));<br>
> + navi10_dump_od_table(smu, boot_od_table);<br>
> <br>
> - navi10_dump_od_table(smu, od_table);<br>
> + memcpy(od_table, boot_od_table, sizeof(OverDriveTable_t));<br>
> + memcpy(user_od_table, boot_od_table, sizeof(OverDriveTable_t));<br>
> <br>
> return 0;<br>
> }<br>
> @@ -2429,11 +2440,20 @@ static int navi10_od_edit_dpm_table(struct smu_context *smu, enum PP_OD_DPM_TABL<br>
> memcpy(table_context->overdrive_table, table_context->boot_overdrive_table, sizeof(OverDriveTable_t));<br>
> break;<br>
> case PP_OD_COMMIT_DPM_TABLE:<br>
> - navi10_dump_od_table(smu, od_table);<br>
> - ret = smu_cmn_update_table(smu, SMU_TABLE_OVERDRIVE, 0, (void *)od_table, true);<br>
> - if (ret) {<br>
> - dev_err(smu->adev->dev, "Failed to import overdrive table!\n");<br>
> - return ret;<br>
> + if (memcmp(od_table, table_context->user_overdrive_table, sizeof(OverDriveTable_t))) {<br>
> + navi10_dump_od_table(smu, od_table);<br>
> + ret = smu_cmn_update_table(smu, SMU_TABLE_OVERDRIVE, 0, (void *)od_table, true);<br>
> + if (ret) {<br>
> + dev_err(smu->adev->dev, "Failed to import overdrive table!\n");<br>
> + return ret;<br>
> + }<br>
> + memcpy(table_context->user_overdrive_table, od_table, sizeof(OverDriveTable_t));<br>
> + smu->user_dpm_profile.user_od = true;<br>
> +<br>
> + if (!memcmp(table_context->user_overdrive_table,<br>
> + table_context->boot_overdrive_table,<br>
> + sizeof(OverDriveTable_t)))<br>
> + smu->user_dpm_profile.user_od = false;<br>
> }<br>
> break;<br>
> case PP_OD_EDIT_VDDC_CURVE:<br>
> @@ -3262,6 +3282,7 @@ static const struct pptable_funcs navi10_ppt_funcs = {<br>
> .set_soft_freq_limited_range = smu_v11_0_set_soft_freq_limited_range,<br>
> .set_default_od_settings = navi10_set_default_od_settings,<br>
> .od_edit_dpm_table = navi10_od_edit_dpm_table,<br>
> + .restore_user_od_settings = smu_v11_0_restore_user_od_settings,<br>
> .run_btc = navi10_run_btc,<br>
> .set_power_source = smu_v11_0_set_power_source,<br>
> .get_pp_feature_mask = smu_cmn_get_pp_feature_mask,<br>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c<br>
> index 0a5d46ac9ccd..7aa47dbba8d8 100644<br>
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c<br>
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c<br>
> @@ -422,10 +422,20 @@ int smu_v11_0_init_smc_tables(struct smu_context *smu)<br>
> ret = -ENOMEM;<br>
> goto err3_out;<br>
> }<br>
> +<br>
> + smu_table->user_overdrive_table =<br>
> + kzalloc(tables[SMU_TABLE_OVERDRIVE].size, GFP_KERNEL);<br>
> + if (!smu_table->user_overdrive_table) {<br>
> + ret = -ENOMEM;<br>
> + goto err4_out;<br>
> + }<br>
> +<br>
> }<br>
> <br>
> return 0;<br>
> <br>
> +err4_out:<br>
> + kfree(smu_table->boot_overdrive_table);<br>
> err3_out:<br>
> kfree(smu_table->overdrive_table);<br>
> err2_out:<br>
> @@ -442,12 +452,14 @@ int smu_v11_0_fini_smc_tables(struct smu_context *smu)<br>
> struct smu_dpm_context *smu_dpm = &smu->smu_dpm;<br>
> <br>
> kfree(smu_table->gpu_metrics_table);<br>
> + kfree(smu_table->user_overdrive_table);<br>
> kfree(smu_table->boot_overdrive_table);<br>
> kfree(smu_table->overdrive_table);<br>
> kfree(smu_table->max_sustainable_clocks);<br>
> kfree(smu_table->driver_pptable);<br>
> kfree(smu_table->clocks_table);<br>
> smu_table->gpu_metrics_table = NULL;<br>
> + smu_table->user_overdrive_table = NULL;<br>
> smu_table->boot_overdrive_table = NULL;<br>
> smu_table->overdrive_table = NULL;<br>
> smu_table->max_sustainable_clocks = NULL;<br>
> @@ -2101,3 +2113,16 @@ int smu_v11_0_deep_sleep_control(struct smu_context *smu,<br>
> <br>
> return ret;<br>
> }<br>
> +<br>
> +int smu_v11_0_restore_user_od_settings(struct smu_context *smu)<br>
> +{<br>
> + struct smu_table_context *table_context = &smu->smu_table;<br>
> + void *user_od_table = table_context->user_overdrive_table;<br>
> + int ret = 0;<br>
> +<br>
> + ret = smu_cmn_update_table(smu, SMU_TABLE_OVERDRIVE, 0, (void *)user_od_table, true);<br>
> + if (ret)<br>
> + dev_err(smu->adev->dev, "Failed to import overdrive table!\n");<br>
> +<br>
> + return ret;<br>
> +}<br>
> <o:p></o:p></p>
</div>
</div>
</div>
</div>
</div>
</body>
</html>