[PATCH 4/4] drm/amd/pp: Implement set_power_profile_mode on smu7

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 

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 ...
>
>
>
[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux