[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-25 06:48 AM, Zhu, Rex wrote:
>>> 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.

How do you unforce the clock levels with your patch? Currently switching
from "manual" back to "auto" will call smu7_unforce_dpm_levels. However,
if you are in "auto" mode all the time, that won't happen. So you're
still breaking the interface.

Regards,
  Felix

>
>
> 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 ...
>>
>>
>>



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux