>>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; [email protected]
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 <[email protected]> on behalf of
> Felix Kuehling <[email protected]>
> *Sent:* Thursday, January 25, 2018 7:12 AM
> *To:* Zhu, Rex; Huang, JinHuiEric; [email protected]
> *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; [email protected]
> > *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 <[email protected]> on behalf
> > > of Eric Huang <[email protected]>
> > > *Sent:* Thursday, January 25, 2018 12:04:55 AM
> > > *To:* [email protected]
> > > *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 <[email protected]>
> > > > ---
> > > > 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
> > > [email protected]
> > > 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
> > > [email protected]
> > > 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
> [email protected]
> 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 ...
>
>
>
--- Begin Message ---
Driver do not maintain manual mode for dpm_force_performance_level,
User can set sclk/mclk/pcie range through pp_dpm_sclk/pp_dpm_mclk/pp_dpm_pcie
directly.
In order to not break currently tools,
when set "manual" to power_dpm_force_performance_level
driver will do nothing and just return successful.
Change-Id: Iaf672b9abc7fa57b765ceb7fa2fba6ad3e80c50b
Signed-off-by: Rex Zhu <[email protected]>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 3 +--
drivers/gpu/drm/amd/amdgpu/ci_dpm.c | 5 -----
drivers/gpu/drm/amd/include/kgd_pp_interface.h | 15 +++++++--------
drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c | 4 ----
drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c | 1 -
drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 6 ------
drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 6 ------
7 files changed, 8 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index 1812009..66b4df0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -152,7 +152,6 @@ static ssize_t
amdgpu_get_dpm_forced_performance_level(struct device *dev,
(level == AMD_DPM_FORCED_LEVEL_AUTO) ? "auto" :
(level == AMD_DPM_FORCED_LEVEL_LOW) ? "low" :
(level == AMD_DPM_FORCED_LEVEL_HIGH) ? "high" :
- (level == AMD_DPM_FORCED_LEVEL_MANUAL) ? "manual" :
(level == AMD_DPM_FORCED_LEVEL_PROFILE_STANDARD) ?
"profile_standard" :
(level == AMD_DPM_FORCED_LEVEL_PROFILE_MIN_SCLK) ?
"profile_min_sclk" :
(level == AMD_DPM_FORCED_LEVEL_PROFILE_MIN_MCLK) ?
"profile_min_mclk" :
@@ -186,7 +185,7 @@ static ssize_t
amdgpu_set_dpm_forced_performance_level(struct device *dev,
} else if (strncmp("auto", buf, strlen("auto")) == 0) {
level = AMD_DPM_FORCED_LEVEL_AUTO;
} else if (strncmp("manual", buf, strlen("manual")) == 0) {
- level = AMD_DPM_FORCED_LEVEL_MANUAL;
+ pr_info("No need to set manual mode, Just go ahead\n");
} else if (strncmp("profile_exit", buf, strlen("profile_exit")) == 0) {
level = AMD_DPM_FORCED_LEVEL_PROFILE_EXIT;
} else if (strncmp("profile_standard", buf, strlen("profile_standard"))
== 0) {
diff --git a/drivers/gpu/drm/amd/amdgpu/ci_dpm.c
b/drivers/gpu/drm/amd/amdgpu/ci_dpm.c
index ab45232..8ddc978 100644
--- a/drivers/gpu/drm/amd/amdgpu/ci_dpm.c
+++ b/drivers/gpu/drm/amd/amdgpu/ci_dpm.c
@@ -6639,11 +6639,6 @@ static int ci_dpm_force_clock_level(void *handle,
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
struct ci_power_info *pi = ci_get_pi(adev);
- if (adev->pm.dpm.forced_level & (AMD_DPM_FORCED_LEVEL_AUTO |
- AMD_DPM_FORCED_LEVEL_LOW |
- AMD_DPM_FORCED_LEVEL_HIGH))
- return -EINVAL;
-
switch (type) {
case PP_SCLK:
if (!pi->sclk_dpm_key_disabled)
diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
index b9aa9f4..3fab686 100644
--- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
+++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
@@ -41,14 +41,13 @@ struct amd_vce_state {
enum amd_dpm_forced_level {
AMD_DPM_FORCED_LEVEL_AUTO = 0x1,
- AMD_DPM_FORCED_LEVEL_MANUAL = 0x2,
- AMD_DPM_FORCED_LEVEL_LOW = 0x4,
- AMD_DPM_FORCED_LEVEL_HIGH = 0x8,
- AMD_DPM_FORCED_LEVEL_PROFILE_STANDARD = 0x10,
- AMD_DPM_FORCED_LEVEL_PROFILE_MIN_SCLK = 0x20,
- AMD_DPM_FORCED_LEVEL_PROFILE_MIN_MCLK = 0x40,
- AMD_DPM_FORCED_LEVEL_PROFILE_PEAK = 0x80,
- AMD_DPM_FORCED_LEVEL_PROFILE_EXIT = 0x100,
+ AMD_DPM_FORCED_LEVEL_LOW = 0x2,
+ AMD_DPM_FORCED_LEVEL_HIGH = 0x4,
+ AMD_DPM_FORCED_LEVEL_PROFILE_STANDARD = 0x8,
+ AMD_DPM_FORCED_LEVEL_PROFILE_MIN_SCLK = 0x10,
+ AMD_DPM_FORCED_LEVEL_PROFILE_MIN_MCLK = 0x20,
+ AMD_DPM_FORCED_LEVEL_PROFILE_PEAK = 0x40,
+ AMD_DPM_FORCED_LEVEL_PROFILE_EXIT = 0x80,
};
enum amd_pm_state_type {
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
index dec8dd9..60d280c 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
@@ -1250,7 +1250,6 @@ static int cz_dpm_force_dpm_level(struct pp_hwmgr *hwmgr,
case AMD_DPM_FORCED_LEVEL_AUTO:
ret = cz_phm_unforce_dpm_levels(hwmgr);
break;
- case AMD_DPM_FORCED_LEVEL_MANUAL:
case AMD_DPM_FORCED_LEVEL_PROFILE_EXIT:
default:
break;
@@ -1558,9 +1557,6 @@ static int cz_get_dal_power_level(struct pp_hwmgr *hwmgr,
static int cz_force_clock_level(struct pp_hwmgr *hwmgr,
enum pp_clock_type type, uint32_t mask)
{
- if (hwmgr->dpm_level != AMD_DPM_FORCED_LEVEL_MANUAL)
- return -EINVAL;
-
switch (type) {
case PP_SCLK:
smum_send_msg_to_smc_with_parameter(hwmgr,
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c
b/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c
index 409a56b..eddcbcd 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c
@@ -605,7 +605,6 @@ static int rv_dpm_force_dpm_level(struct pp_hwmgr *hwmgr,
PPSMC_MSG_SetSoftMaxFclkByFreq,
RAVEN_UMD_PSTATE_MIN_FCLK);
break;
- case AMD_DPM_FORCED_LEVEL_MANUAL:
case AMD_DPM_FORCED_LEVEL_PROFILE_EXIT:
default:
break;
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
index 13db75c..e3a8374 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
@@ -2798,7 +2798,6 @@ static int smu7_force_dpm_level(struct pp_hwmgr *hwmgr,
smu7_force_clock_level(hwmgr, PP_MCLK, 1<<mclk_mask);
smu7_force_clock_level(hwmgr, PP_PCIE, 1<<pcie_mask);
break;
- case AMD_DPM_FORCED_LEVEL_MANUAL:
case AMD_DPM_FORCED_LEVEL_PROFILE_EXIT:
default:
break;
@@ -4311,11 +4310,6 @@ static int smu7_force_clock_level(struct pp_hwmgr *hwmgr,
{
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;
-
switch (type) {
case PP_SCLK:
if (!data->sclk_dpm_key_disabled)
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
index 6b28896..828677e 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
@@ -4241,7 +4241,6 @@ static int vega10_dpm_force_dpm_level(struct pp_hwmgr
*hwmgr,
vega10_force_clock_level(hwmgr, PP_SCLK, 1<<sclk_mask);
vega10_force_clock_level(hwmgr, PP_MCLK, 1<<mclk_mask);
break;
- case AMD_DPM_FORCED_LEVEL_MANUAL:
case AMD_DPM_FORCED_LEVEL_PROFILE_EXIT:
default:
break;
@@ -4500,11 +4499,6 @@ static int vega10_force_clock_level(struct pp_hwmgr
*hwmgr,
{
struct vega10_hwmgr *data = (struct vega10_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;
-
switch (type) {
case PP_SCLK:
data->smc_state_table.gfx_boot_level = mask ? (ffs(mask) - 1) :
0;
--
1.9.1
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
--- End Message ---
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
