[PATCH 4/4] drm/amd/pp: Implement set_power_profile_mode on smu7
Zhu, Rex
Rex.Zhu at amd.com
Thu Jan 25 11:48:31 UTC 2018
>>That's a change in the pp_dpm_sclk/mclk interface. Currenly these only work in manual mode and fail or are ignored in auto mode. E.g. see this code in smu7_hwmgr.c:
>>static int smu7_force_clock_level(struct pp_hwmgr *hwmgr,
The attached patch can fix this issue.
Thanks.
Best Regards
Rex
-----Original Message-----
From: Kuehling, Felix
Sent: Thursday, January 25, 2018 12:16 PM
To: Zhu, Rex; Huang, JinHuiEric; amd-gfx at lists.freedesktop.org
Subject: Re: [PATCH 4/4] drm/amd/pp: Implement set_power_profile_mode on smu7
On 2018-01-24 09:28 PM, Zhu, Rex wrote:
>
> >But then you're changing the semantics of how pp_dpm_sclk/mclk wok
> >together with pp_dpm_force_performance_level.
>
> Rex:The two sysfs are all for clock range setting.
>
>
> pp_dpm_sclk/mclk/pcie can set sclk/mclk/pcie range separately.
>
Only in manual mode. If you change that, you're changing the interface and breaking existing tools.
>
> and pp_dpm_force_performance_level, we can support low/high/umd-pstate
> that set all the clocks range.
>
> (manual state, driver do nothing except set the flag)
>
>
> No matter what state user enter in, driver can support to continue to
> change the clock range through pp_dpm_sclk/mclk/pcie.
>
>
> so in fact, don't need manual state for sepalately set the clock range.
>
That's a change in the pp_dpm_sclk/mclk interface. Currenly these only work in manual mode and fail or are ignored in auto mode. E.g. see this code in smu7_hwmgr.c:
static int smu7_force_clock_level(struct pp_hwmgr *hwmgr,
enum pp_clock_type type, uint32_t mask) {
struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend);
if (hwmgr->request_dpm_level & (AMD_DPM_FORCED_LEVEL_AUTO |
AMD_DPM_FORCED_LEVEL_LOW |
AMD_DPM_FORCED_LEVEL_HIGH))
return -EINVAL;
Regards,
Felix
>
> >Sysfs interfaces should be maintained stable. I'm OK with breaking
> >the old power profile stuff, because it only affects KFD which isn't
> >upstream yet.
>
>
> Rex: As the old power profile interface can't support Vega.
>
> so add new interface and we are try to support old asics with new sysfs.
>
>
>
> >But the old pp_dpm_sclk/mclk and
> >pp_dpm_force_performance_level affect code that's currently upstream
> >and used by existing tools.
>
>
> Rex: Yes, we are trying to maintain the sysfs stable.
> if we change the old sysfs code, we will try to not affect existing
> tools.
>
> Best Regards
> Rex
>
>
> ----------------------------------------------------------------------
> --
> *From:* amd-gfx <amd-gfx-bounces at lists.freedesktop.org> on behalf of
> Felix Kuehling <felix.kuehling at amd.com>
> *Sent:* Thursday, January 25, 2018 7:12 AM
> *To:* Zhu, Rex; Huang, JinHuiEric; amd-gfx at lists.freedesktop.org
> *Subject:* Re: [PATCH 4/4] drm/amd/pp: Implement
> set_power_profile_mode on smu7
>
> On 2018-01-24 06:05 PM, Zhu, Rex wrote:
> > Hi Felix,
> >
> > The logic of gfx/computer profile setting works in such way as you
> > said,need under “auto” state.
> > But in new sysfs of power profile setting, we do not check the
> performance
> > Level.
> >
> > And The “manual” state is a confusing flag，when user set manual
> > state，and then change the clk range through pp-dpm-sclk/mclk，the dpm
> > still works automatically in new clock range, I think we can remove
> > this flag.
>
> But then you're changing the semantics of how pp_dpm_sclk/mclk wok
> together with pp_dpm_force_performance_level.
>
> Sysfs interfaces should be maintained stable. I'm OK with breaking the
> old power profile stuff, because it only affects KFD which isn't
> upstream yet. But the old pp_dpm_sclk/mclk and
> pp_dpm_force_performance_level affect code that's currently upstream
> and used by existing tools.
>
> >
> > So My idea is
> > in the sysfs of pp dpm sclk/mclk, user can set clock range.
> > In the sysfs of power profile state, user can configure the
> > parameters smu/driver exposed. They are independent.
>
> I disagree. the clock range is not independent of the profile.
> Profiles correspond to use cases. Different use cases have different
> clock requirements. For compute we want the clocks to be high. For
> video you may have requirements for minimum mclks to ensure smooth
> playback. For power saving you may want to limit maximum clocks, etc.
> So setting the clock range independent of the profile in "auto" mode
> does not make sense to me.
>
> Regards,
> Felix
>
> >
> >
> >
> >
> >
> >
> > Best Regards
> > Rex
> > --------------------------------------------------------------------
> > ----
> > *From:* Kuehling, Felix
> > *Sent:* Thursday, January 25, 2018 5:41:43 AM
> > *To:* Zhu, Rex; Huang, JinHuiEric; amd-gfx at lists.freedesktop.org
> > *Subject:* Re: [PATCH 4/4] drm/amd/pp: Implement
> > set_power_profile_mode on smu7
> >
> > Hi Rex,
> >
> > As I understand it (the way power profiles currently work),
> > pp_dpm_sclk/mclk only apply if pp_dpm_force_performance_level is set
> > to "manual". Power profiles and automatic switching between profiles
> > only happens when pp_dpm_force_performance_level is set to "auto".
> >
> > This means pp_dpm_sclk/mclk don't apply when profiles are in effect.
> > Also, there would be no way to set different minimum clocks for
> > different profiles.
> >
> > I think minimum clocks should be part of the profiles.
> >
> > Regards,
> > Felix
> >
> >
> > On 2018-01-24 03:13 PM, Zhu, Rex wrote:
> > > Hi Eric,
> > >
> > > We have sysfs pp-dpm-sclk/mclk to set min dpm level
> > >
> > > Best Regards
> > > Rex
> > >
> ----------------------------------------------------------------------
> --
> > > *From:* amd-gfx <amd-gfx-bounces at lists.freedesktop.org> on behalf
> > > of Eric Huang <jinhuieric.huang at amd.com>
> > > *Sent:* Thursday, January 25, 2018 12:04:55 AM
> > > *To:* amd-gfx at lists.freedesktop.org
> > > *Subject:* Re: [PATCH 4/4] drm/amd/pp: Implement
> > > set_power_profile_mode on smu7
> > >
> > > We have min_sclk and min_mclk in previous power profile parameters
> > > for VI, which are similar with min_active_level for Vega10. How to
> implement
> > > these parameters?
> > >
> > > Regards,
> > > Eric
> > >
> > > On 2018-01-24 04:37 AM, Rex Zhu wrote:
> > > > User can set smu7 profile pamameters through sysfs
> > > >
> > > > echo "0/1/2/3/4">pp_power_profile_mode to select
> > > > 3D_FULL_SCREEN/POWER_SAVING/VIDEO/VR/COMPUTE
> > > > mode.
> > > > echo "5 * * * * * * * *">pp_power_profile_mode to config custom
> > > > mode.
> > > > "5 * * * * * * * *" mean "CUSTOM enable_sclk SCLK_UP_HYST
> > > > SCLK_DOWN_HYST SCLK_ACTIVE_LEVEL enable_mclk MCLK_UP_HYST
> > > > MCLK_DOWN_HYST MCLK_ACTIVE_LEVEL"
> > > >
> > > > Change-Id: Ic6d6f37363bc81ab17051285f6ace847edf725de
> > > > Signed-off-by: Rex Zhu <Rex.Zhu at amd.com>
> > > > ---
> > > > drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 49
> > > +++++++++++++++++++++++-
> > > > 1 file changed, 48 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> > > b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> > > > index 9f6afd5..13db75c 100644
> > > > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> > > > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> > > > @@ -5036,7 +5036,54 @@ static int
> > > > smu7_get_power_profile_mode(struct
> > > pp_hwmgr *hwmgr, char *buf)
> > > >
> > > > static int smu7_set_power_profile_mode(struct pp_hwmgr *hwmgr,
> > > long *input, uint32_t size)
> > > > {
> > > > - /* To Do */
> > > > + struct smu7_hwmgr *data = (struct smu7_hwmgr
> *)(hwmgr->backend);
> > > > + struct profile_mode_setting tmp;
> > > > +
> > > > + hwmgr->power_profile_mode = input[size];
> > > > +
> > > > + switch (hwmgr->power_profile_mode) {
> > > > + case PP_SMC_POWER_PROFILE_CUSTOM:
> > > > + if (size < 8)
> > > > + return -EINVAL;
> > > > +
> > > > + data->custom_profile_setting.bupdate_sclk =
> > > > +input[0];
> > > > + data->custom_profile_setting.sclk_up_hyst =
> > > > +input[1];
> > > > + data->custom_profile_setting.sclk_down_hyst =
> input[2];
> > > > + data->custom_profile_setting.sclk_activity =
> input[3];
> > > > + data->custom_profile_setting.bupdate_mclk =
> > > > +input[4];
> > > > + data->custom_profile_setting.mclk_up_hyst =
> > > > +input[5];
> > > > + data->custom_profile_setting.mclk_down_hyst =
> input[6];
> > > > + data->custom_profile_setting.mclk_activity =
> input[7];
> > > > + if (!smum_update_dpm_settings(hwmgr,
> > > &data->custom_profile_setting))
> > > > + memcpy(&data->current_profile_setting,
> > > &data->custom_profile_setting, sizeof(struct
> > > profile_mode_setting));
> > > > + break;
> > > > + case PP_SMC_POWER_PROFILE_FULLSCREEN3D:
> > > > + case PP_SMC_POWER_PROFILE_POWERSAVING:
> > > > + case PP_SMC_POWER_PROFILE_VIDEO:
> > > > + case PP_SMC_POWER_PROFILE_VR:
> > > > + case PP_SMC_POWER_PROFILE_COMPUTE:
> > > > + memcpy(&tmp,
> > > &smu7_profiling[hwmgr->power_profile_mode], sizeof(struct
> > > profile_mode_setting));
> > > > + if (!smum_update_dpm_settings(hwmgr, &tmp)) {
> > > > + if (tmp.bupdate_sclk) {
> > > > +
> > > data->current_profile_setting.bupdate_sclk = tmp.bupdate_sclk;
> > > > +
> > > data->current_profile_setting.sclk_up_hyst = tmp.sclk_up_hyst;
> > > > +
> > > data->current_profile_setting.sclk_down_hyst = tmp.sclk_down_hyst;
> > > > +
> > > data->current_profile_setting.sclk_activity = tmp.sclk_activity;
> > > > + }
> > > > + if (tmp.bupdate_mclk) {
> > > > +
> > > data->current_profile_setting.bupdate_mclk = tmp.bupdate_mclk;
> > > > +
> > > data->current_profile_setting.mclk_up_hyst = tmp.mclk_up_hyst;
> > > > +
> > > data->current_profile_setting.mclk_down_hyst = tmp.mclk_down_hyst;
> > > > +
> > > data->current_profile_setting.mclk_activity = tmp.mclk_activity;
> > > > + }
> > > > + }
> > > > + break;
> > > > + case PP_SMC_POWER_PROFILE_AUTO: /* TO DO auto wattman
> > > > +feature
> > > not implement */
> > > > + return 0;
> > > > + default:
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > return 0;
> > > > }
> > > >
> > >
> > > _______________________________________________
> > > amd-gfx mailing list
> > > amd-gfx at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> amd-gfx Info Page - freedesktop.org
> <https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
> lists.freedesktop.org
> Subscribing to amd-gfx: Subscribe to amd-gfx by filling out the
> following form. Use of all freedesktop.org lists is subject to our
> Code of ...
>
>
>
> > >
> > >
> > > _______________________________________________
> > > amd-gfx mailing list
> > > amd-gfx at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> amd-gfx Info Page - freedesktop.org
> <https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
> lists.freedesktop.org
> Subscribing to amd-gfx: Subscribe to amd-gfx by filling out the
> following form. Use of all freedesktop.org lists is subject to our
> Code of ...
>
>
>
> >
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> amd-gfx Info Page - freedesktop.org
> <https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
> lists.freedesktop.org
> Subscribing to amd-gfx: Subscribe to amd-gfx by filling out the
> following form. Use of all freedesktop.org lists is subject to our
> Code of ...
>
>
>
-------------- next part --------------
An embedded message was scrubbed...
From: Rex Zhu <Rex.Zhu at amd.com>
Subject: [PATCH 1/2] drm/amd/pp: Remove manual mode for power_dpm_force_performance_level
Date: Thu, 25 Jan 2018 19:26:53 +0800
Size: 28794
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20180125/b99e665e/attachment-0001.mht>
More information about the amd-gfx
mailing list