<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=us-ascii"> <style type="text/css" style="display:none;"></style> </head> <body dir="ltr"> <div id="divtagdefaultwrapper" style="font-size:12pt;color:#000000;font-family:Calibri,Helvetica,sans-serif;" dir="ltr"> With your proposed changes patch is: Acked-by: Alex Deucher <alexander.deucher@amd.com> </div> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr">From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Quan, Evan <Evan.Quan@amd.com> Sent: Wednesday, June 20, 2018 2:17:57 AM To: Alex Deucher Cc: amd-gfx list Subject: RE: [PATCH 10/13] drm/amd/powerplay: apply clocks adjust rules on power state change <div> </div> </div> <div class="BodyFragment"> <div class="PlainText">Hi Alex, Comment inline. > -----Original Message----- > From: Alex Deucher [<a href="mailto:alexdeucher@gmail.com">mailto:alexdeucher@gmail.com</a>] > Sent: Tuesday, June 19, 2018 11:17 PM > To: Quan, Evan <Evan.Quan@amd.com> > Cc: amd-gfx list <amd-gfx@lists.freedesktop.org> > Subject: Re: [PATCH 10/13] drm/amd/powerplay: apply clocks adjust rules on > power state change > > On Tue, Jun 19, 2018 at 3:39 AM, Evan Quan <evan.quan@amd.com> wrote: > > The clocks hard/soft min/max clock levels will be adjusted > > correspondingly. > > > Also note that this add the apply_clocks_adjust_rules callback which is used > to validate the clock settings on a power state change. One other comment > below. Yes, this is for the apply_clocks_adjust_rules callback. I updated the patch description as below drm/amd/powerplay: apply clocks adjust rules on power state change This add the apply_clocks_adjust_rules callback which is used to validate the clock settings on a power state change. > > > > Change-Id: I2c4b6cd6756d40a28933f0c26b9e1a3d5078bab8 > > Signed-off-by: Evan Quan <evan.quan@amd.com> > > --- > > drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c | 162 > +++++++++++++++++++++ > > drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.h | 2 + > > 2 files changed, 164 insertions(+) > > > > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c > > b/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c > > index a227ace..26bdfff 100644 > > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c > > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c > > @@ -1950,6 +1950,166 @@ static int vega12_print_clock_levels(struct > pp_hwmgr *hwmgr, > > return size; > > } > > > > +static int vega12_apply_clocks_adjust_rules(struct pp_hwmgr *hwmgr) { > > + struct vega12_hwmgr *data = (struct vega12_hwmgr *)(hwmgr- > >backend); > > + struct vega12_single_dpm_table *dpm_table; > > + bool vblank_too_short = false; > > + bool disable_mclk_switching; > > + uint32_t i, latency; > > + > > + disable_mclk_switching = ((1 < hwmgr->display_config->num_display) > && > > + !hwmgr->display_config->multi_monitor_in_sync) || > > + vblank_too_short; > > + latency = > > + hwmgr->display_config->dce_tolerable_mclk_in_active_latency; > > + > > + /* gfxclk */ > > + dpm_table = &(data->dpm_table.gfx_table); > > + dpm_table->dpm_state.soft_min_level = dpm_table- > >dpm_levels[0].value; > > + dpm_table->dpm_state.soft_max_level = dpm_table- > >dpm_levels[dpm_table->count - 1].value; > > + dpm_table->dpm_state.hard_min_level = dpm_table- > >dpm_levels[0].value; > > + dpm_table->dpm_state.hard_max_level = > > + dpm_table->dpm_levels[dpm_table->count - 1].value; > > + > > + if (PP_CAP(PHM_PlatformCaps_UMDPState)) { > > + if (VEGA12_UMD_PSTATE_GFXCLK_LEVEL < dpm_table->count) { > > + dpm_table->dpm_state.soft_min_level = dpm_table- > >dpm_levels[VEGA12_UMD_PSTATE_GFXCLK_LEVEL].value; > > + dpm_table->dpm_state.soft_max_level = dpm_table- > >dpm_levels[VEGA12_UMD_PSTATE_GFXCLK_LEVEL].value; > > + } > > + > > + if (hwmgr->dpm_level == > AMD_DPM_FORCED_LEVEL_PROFILE_MIN_SCLK) { > > + dpm_table->dpm_state.soft_min_level = dpm_table- > >dpm_levels[0].value; > > + dpm_table->dpm_state.soft_max_level = dpm_table- > >dpm_levels[0].value; > > + } > > + > > + if (hwmgr->dpm_level == > AMD_DPM_FORCED_LEVEL_PROFILE_PEAK) { > > + dpm_table->dpm_state.soft_min_level = dpm_table- > >dpm_levels[dpm_table->count - 1].value; > > + dpm_table->dpm_state.soft_max_level = dpm_table- > >dpm_levels[dpm_table->count - 1].value; > > + } > > + } > > + > > + /* memclk */ > > + dpm_table = &(data->dpm_table.mem_table); > > + dpm_table->dpm_state.soft_min_level = dpm_table- > >dpm_levels[0].value; > > + dpm_table->dpm_state.soft_max_level = dpm_table- > >dpm_levels[dpm_table->count - 1].value; > > + dpm_table->dpm_state.hard_min_level = dpm_table- > >dpm_levels[0].value; > > + dpm_table->dpm_state.hard_max_level = > > + dpm_table->dpm_levels[dpm_table->count - 1].value; > > + > > + if (PP_CAP(PHM_PlatformCaps_UMDPState)) { > > + if (VEGA12_UMD_PSTATE_MCLK_LEVEL < dpm_table->count) { > > + dpm_table->dpm_state.soft_min_level = dpm_table- > >dpm_levels[VEGA12_UMD_PSTATE_MCLK_LEVEL].value; > > + dpm_table->dpm_state.soft_max_level = dpm_table- > >dpm_levels[VEGA12_UMD_PSTATE_MCLK_LEVEL].value; > > + } > > + > > + if (hwmgr->dpm_level == > AMD_DPM_FORCED_LEVEL_PROFILE_MIN_MCLK) { > > + dpm_table->dpm_state.soft_min_level = dpm_table- > >dpm_levels[0].value; > > + dpm_table->dpm_state.soft_max_level = dpm_table- > >dpm_levels[0].value; > > + } > > + > > + if (hwmgr->dpm_level == > AMD_DPM_FORCED_LEVEL_PROFILE_PEAK) { > > + dpm_table->dpm_state.soft_min_level = dpm_table- > >dpm_levels[dpm_table->count - 1].value; > > + dpm_table->dpm_state.soft_max_level = dpm_table- > >dpm_levels[dpm_table->count - 1].value; > > + } > > + } > > + > > + /* honour DAL's UCLK Hardmin */ > > + if (dpm_table->dpm_state.hard_min_level < (hwmgr->display_config- > >min_mem_set_clock / 100)) > > + dpm_table->dpm_state.hard_min_level = > > + hwmgr->display_config->min_mem_set_clock / 100; > > + > > Didn't you just remove the uclk hard min setting in a previous patch? > Yes, it was moved here together with other clocks' min settings. > > > + /* Hardmin is dependent on displayconfig */ > > + if (disable_mclk_switching) { > > + dpm_table->dpm_state.hard_min_level = dpm_table- > >dpm_levels[dpm_table->count - 1].value; > > + for (i = 0; i < data->mclk_latency_table.count - 1; i++) { > > + if (data->mclk_latency_table.entries[i].latency <= latency) { > > + if (dpm_table->dpm_levels[i].value >= (hwmgr- > >display_config->min_mem_set_clock / 100)) { > > + dpm_table->dpm_state.hard_min_level = dpm_table- > >dpm_levels[i].value; > > + break; > > + } > > + } > > + } > > + } > > + > > + if (hwmgr->display_config->nb_pstate_switch_disable) > > + dpm_table->dpm_state.hard_min_level = > > + dpm_table->dpm_levels[dpm_table->count - 1].value; > > + > > + /* vclk */ > > + dpm_table = &(data->dpm_table.vclk_table); > > + dpm_table->dpm_state.soft_min_level = dpm_table- > >dpm_levels[0].value; > > + dpm_table->dpm_state.soft_max_level = dpm_table- > >dpm_levels[dpm_table->count - 1].value; > > + dpm_table->dpm_state.hard_min_level = dpm_table- > >dpm_levels[0].value; > > + dpm_table->dpm_state.hard_max_level = > > + dpm_table->dpm_levels[dpm_table->count - 1].value; > > + > > + if (PP_CAP(PHM_PlatformCaps_UMDPState)) { > > + if (VEGA12_UMD_PSTATE_UVDCLK_LEVEL < dpm_table->count) { > > + dpm_table->dpm_state.soft_min_level = dpm_table- > >dpm_levels[VEGA12_UMD_PSTATE_UVDCLK_LEVEL].value; > > + dpm_table->dpm_state.soft_max_level = dpm_table- > >dpm_levels[VEGA12_UMD_PSTATE_UVDCLK_LEVEL].value; > > + } > > + > > + if (hwmgr->dpm_level == > AMD_DPM_FORCED_LEVEL_PROFILE_PEAK) { > > + dpm_table->dpm_state.soft_min_level = dpm_table- > >dpm_levels[dpm_table->count - 1].value; > > + dpm_table->dpm_state.soft_max_level = dpm_table- > >dpm_levels[dpm_table->count - 1].value; > > + } > > + } > > + > > + /* dclk */ > > + dpm_table = &(data->dpm_table.dclk_table); > > + dpm_table->dpm_state.soft_min_level = dpm_table- > >dpm_levels[0].value; > > + dpm_table->dpm_state.soft_max_level = dpm_table- > >dpm_levels[dpm_table->count - 1].value; > > + dpm_table->dpm_state.hard_min_level = dpm_table- > >dpm_levels[0].value; > > + dpm_table->dpm_state.hard_max_level = > > + dpm_table->dpm_levels[dpm_table->count - 1].value; > > + > > + if (PP_CAP(PHM_PlatformCaps_UMDPState)) { > > + if (VEGA12_UMD_PSTATE_UVDCLK_LEVEL < dpm_table->count) { > > + dpm_table->dpm_state.soft_min_level = dpm_table- > >dpm_levels[VEGA12_UMD_PSTATE_UVDCLK_LEVEL].value; > > + dpm_table->dpm_state.soft_max_level = dpm_table- > >dpm_levels[VEGA12_UMD_PSTATE_UVDCLK_LEVEL].value; > > + } > > + > > + if (hwmgr->dpm_level == > AMD_DPM_FORCED_LEVEL_PROFILE_PEAK) { > > + dpm_table->dpm_state.soft_min_level = dpm_table- > >dpm_levels[dpm_table->count - 1].value; > > + dpm_table->dpm_state.soft_max_level = dpm_table- > >dpm_levels[dpm_table->count - 1].value; > > + } > > + } > > + > > + /* socclk */ > > + dpm_table = &(data->dpm_table.soc_table); > > + dpm_table->dpm_state.soft_min_level = dpm_table- > >dpm_levels[0].value; > > + dpm_table->dpm_state.soft_max_level = dpm_table- > >dpm_levels[dpm_table->count - 1].value; > > + dpm_table->dpm_state.hard_min_level = dpm_table- > >dpm_levels[0].value; > > + dpm_table->dpm_state.hard_max_level = > > + dpm_table->dpm_levels[dpm_table->count - 1].value; > > + > > + if (PP_CAP(PHM_PlatformCaps_UMDPState)) { > > + if (VEGA12_UMD_PSTATE_SOCCLK_LEVEL < dpm_table->count) { > > + dpm_table->dpm_state.soft_min_level = dpm_table- > >dpm_levels[VEGA12_UMD_PSTATE_SOCCLK_LEVEL].value; > > + dpm_table->dpm_state.soft_max_level = dpm_table- > >dpm_levels[VEGA12_UMD_PSTATE_SOCCLK_LEVEL].value; > > + } > > + > > + if (hwmgr->dpm_level == > AMD_DPM_FORCED_LEVEL_PROFILE_PEAK) { > > + dpm_table->dpm_state.soft_min_level = dpm_table- > >dpm_levels[dpm_table->count - 1].value; > > + dpm_table->dpm_state.soft_max_level = dpm_table- > >dpm_levels[dpm_table->count - 1].value; > > + } > > + } > > + > > + /* eclk */ > > + dpm_table = &(data->dpm_table.eclk_table); > > + dpm_table->dpm_state.soft_min_level = dpm_table- > >dpm_levels[0].value; > > + dpm_table->dpm_state.soft_max_level = dpm_table- > >dpm_levels[dpm_table->count - 1].value; > > + dpm_table->dpm_state.hard_min_level = dpm_table- > >dpm_levels[0].value; > > + dpm_table->dpm_state.hard_max_level = > > + dpm_table->dpm_levels[dpm_table->count - 1].value; > > + > > + if (PP_CAP(PHM_PlatformCaps_UMDPState)) { > > + if (VEGA12_UMD_PSTATE_VCEMCLK_LEVEL < dpm_table->count) > { > > + dpm_table->dpm_state.soft_min_level = dpm_table- > >dpm_levels[VEGA12_UMD_PSTATE_VCEMCLK_LEVEL].value; > > + dpm_table->dpm_state.soft_max_level = dpm_table- > >dpm_levels[VEGA12_UMD_PSTATE_VCEMCLK_LEVEL].value; > > + } > > + > > + if (hwmgr->dpm_level == > AMD_DPM_FORCED_LEVEL_PROFILE_PEAK) { > > + dpm_table->dpm_state.soft_min_level = dpm_table- > >dpm_levels[dpm_table->count - 1].value; > > + dpm_table->dpm_state.soft_max_level = dpm_table- > >dpm_levels[dpm_table->count - 1].value; > > + } > > + } > > + > > + return 0; > > +} > > + > > static int vega12_display_configuration_changed_task(struct pp_hwmgr > > *hwmgr) { > > struct vega12_hwmgr *data = (struct vega12_hwmgr > > *)(hwmgr->backend); @@ -2196,6 +2356,8 @@ static const struct > pp_hwmgr_func vega12_hwmgr_funcs = { > > .display_clock_voltage_request = > vega12_display_clock_voltage_request, > > .force_clock_level = vega12_force_clock_level, > > .print_clock_levels = vega12_print_clock_levels, > > + .apply_clocks_adjust_rules = > > + vega12_apply_clocks_adjust_rules, > > .display_config_changed = > vega12_display_configuration_changed_task, > > .powergate_uvd = vega12_power_gate_uvd, > > .powergate_vce = vega12_power_gate_vce, diff --git > > a/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.h > > b/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.h > > index e18c083..e17237c 100644 > > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.h > > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.h > > @@ -443,6 +443,8 @@ struct vega12_hwmgr { > > #define VEGA12_UMD_PSTATE_GFXCLK_LEVEL 0x3 > > #define VEGA12_UMD_PSTATE_SOCCLK_LEVEL 0x3 > > #define VEGA12_UMD_PSTATE_MCLK_LEVEL 0x2 > > +#define VEGA12_UMD_PSTATE_UVDCLK_LEVEL 0x3 > > +#define VEGA12_UMD_PSTATE_VCEMCLK_LEVEL 0x3 > > > > int vega12_enable_disable_vce_dpm(struct pp_hwmgr *hwmgr, bool > > enable); > > > > -- > > 2.7.4 > > > > _______________________________________________ > > amd-gfx mailing list > > amd-gfx@lists.freedesktop.org > > <a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a> _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org <a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a> </div> </div> </body> </html>