Re: [PATCH] drm/amd/pp: Revert gfx/compute profile switch sysfs

2018-02-28 Thread Alex Deucher 
On Wed, Feb 28, 2018 at 1:47 PM, Felix Kuehling <felix.kuehl...@amd.com> wrote:
> On 2018-02-28 12:37 PM, Zhu, Rex wrote:
>>
>> Hi Felix,
>>
>>
>> First, thanks very much for taking so much time to review this interface.
>>
>> In fact, we just hope and want to support power profiling feature better.
>>
>>
>> Sclk level 7  i mean the highest sclk level, we get used to start from
>> level 0. sorry for makeing you confuse.
>>
>
> Oh right. In the profile we set the minimum clock to the second highest
> level. So that's level 6.

I think we are still good to go with Rex's design.  If you want to
experiment with different heuristics, you can switch to manual mode
and create custom ones and test.  If we find ones that are useful for
various use cases, we can always add them to the set of profiles that
the driver can select between at runtime when it's in auto mode.  The
kfd can select the profile at runtime based on heuristics, or if
userspace has a better idea, you could add an ioctl where userspace
could pass hints to the kfd as to what workload it is seeing and the
kfd can select a profile based on those hints.

Alex


>
> Regards,
>   Felix
>
>>
>> Best Regards
>>
>> Rex
>>
>> 
>> *From:* Kuehling, Felix
>> *Sent:* Thursday, March 1, 2018 1:13:58 AM
>> *To:* Zhu, Rex; Alex Deucher; Huang, JinHuiEric
>> *Cc:* Russell, Kent; amd-gfx@lists.freedesktop.org
>> *Subject:* Re: [PATCH] drm/amd/pp: Revert gfx/compute profile switch
>> sysfs
>>
>> Hi Rex,
>>
>> The purpose of the compute power profile for us is to optimize
>> performance. Compute workloads tend to be "bursty". The default
>> heuristics of the SMU don't work well for such work loads. They aren't
>> able to raise clocks quickly enough to accelerate short bursts of
>> compute work.
>>
>> We need to quickly raise clocks when needed. That's why we change
>> hysteresis parameters, and set a minimum clock. This minimizes the
>> latency of reaching maximum clocks.
>>
>> The goal is not to set a fixed frequency, but to minimize the time
>> needed to reach the highest frequency. Setting fixed DPM level 7 will
>> not allow us to reach level 8. That will hurt our performance.
>>
>> Regards,
>>   Felix
>>
>>
>> On 2018-02-28 06:08 AM, Zhu, Rex wrote:
>> > I went through the power profiling feature in windows - Auto Wattman
>> supported on Polaris and Vega. Initially apply to Polaris.
>> >
>> > The Goal is to maximize performance/watt without impacting
>> performance and no visual impact per system/use case
>> >
>> > On smu7, 3 parameters control how SMU responds to sclk/mclk
>> activity, up/down hysteresis and activity threshold.
>> >
>> > My understanding is:
>> > When sclk_activity large than threshold, adjust
>> uphyst/downhyst/activity threshold to go higher dpm level more faster
>> or stay in higher dpm level longer for boost performance.
>> > Otherwise, for power saving.
>> >
>> > Those parameters were used to change the behavior of dpm switching
>> for performance in watt. So we wish to add an independent sysfs to
>> configure them.
>> >
>> > When this feature enabled, the performance in watt is improved in
>> most case(low activity and Multimedia) from pplib's test. I do not
>> find compute test result.
>> >
>> > So there is a question:
>> >
>> > When compute ring begin, if we just fix sclk in level7 directly, ,
>> > Is there any difference in performance or performance in watt,
>> compared to set compute profile mode (include apply min sclk to level6)?
>> >
>> >
>> > Best Regards
>> > Rex
>> >
>> >
>> >
>> >
>> >
>> > -Original Message-
>> > From: Kuehling, Felix
>> > Sent: Wednesday, February 28, 2018 2:55 AM
>> > To: Alex Deucher; Huang, JinHuiEric
>> > Cc: Zhu, Rex; Russell, Kent; amd-gfx list
>> > Subject: Re: [PATCH] drm/amd/pp: Revert gfx/compute profile switch sysfs
>> >
>> > On 2018-02-27 11:27 AM, Alex Deucher wrote:
>> >> On Tue, Feb 27, 2018 at 11:22 AM, Eric Huang
>> <jinhuieric.hu...@amd.com> wrote:
>> >>> As I mentioned in code review for new power profile, old
>> gfx/compute power
>> >>> profile have two scenarios for auto switching. One is
>> >>> gfx->compute(default)->gfx and other is gfx->comput

Re: [PATCH] drm/amd/pp: Revert gfx/compute profile switch sysfs

2018-02-28 Thread Felix Kuehling 
On 2018-02-28 12:37 PM, Zhu, Rex wrote:
>
> Hi Felix,
>
>
> First, thanks very much for taking so much time to review this interface.
>
> In fact, we just hope and want to support power profiling feature better.
>
>
> Sclk level 7  i mean the highest sclk level, we get used to start from
> level 0. sorry for makeing you confuse.
>

Oh right. In the profile we set the minimum clock to the second highest
level. So that's level 6.

Regards,
  Felix

>
> Best Regards
>
> Rex
>
> 
> *From:* Kuehling, Felix
> *Sent:* Thursday, March 1, 2018 1:13:58 AM
> *To:* Zhu, Rex; Alex Deucher; Huang, JinHuiEric
> *Cc:* Russell, Kent; amd-gfx@lists.freedesktop.org
> *Subject:* Re: [PATCH] drm/amd/pp: Revert gfx/compute profile switch
> sysfs
>  
> Hi Rex,
>
> The purpose of the compute power profile for us is to optimize
> performance. Compute workloads tend to be "bursty". The default
> heuristics of the SMU don't work well for such work loads. They aren't
> able to raise clocks quickly enough to accelerate short bursts of
> compute work.
>
> We need to quickly raise clocks when needed. That's why we change
> hysteresis parameters, and set a minimum clock. This minimizes the
> latency of reaching maximum clocks.
>
> The goal is not to set a fixed frequency, but to minimize the time
> needed to reach the highest frequency. Setting fixed DPM level 7 will
> not allow us to reach level 8. That will hurt our performance.
>
> Regards,
>   Felix
>
>
> On 2018-02-28 06:08 AM, Zhu, Rex wrote:
> > I went through the power profiling feature in windows - Auto Wattman
> supported on Polaris and Vega. Initially apply to Polaris.
> >
> > The Goal is to maximize performance/watt without impacting
> performance and no visual impact per system/use case
> >
> > On smu7, 3 parameters control how SMU responds to sclk/mclk
> activity, up/down hysteresis and activity threshold.
> >
> > My understanding is:
> > When sclk_activity large than threshold, adjust
> uphyst/downhyst/activity threshold to go higher dpm level more faster
> or stay in higher dpm level longer for boost performance.
> > Otherwise, for power saving.
> >
> > Those parameters were used to change the behavior of dpm switching
> for performance in watt. So we wish to add an independent sysfs to
> configure them.
> >
> > When this feature enabled, the performance in watt is improved in
> most case(low activity and Multimedia) from pplib's test. I do not
> find compute test result.
> >
> > So there is a question:
> >
> > When compute ring begin, if we just fix sclk in level7 directly, ,
> > Is there any difference in performance or performance in watt,
> compared to set compute profile mode (include apply min sclk to level6)?
> >
> >
> > Best Regards
> > Rex
> >
> >
> >
> >
> >
> > -Original Message-
> > From: Kuehling, Felix
> > Sent: Wednesday, February 28, 2018 2:55 AM
> > To: Alex Deucher; Huang, JinHuiEric
> > Cc: Zhu, Rex; Russell, Kent; amd-gfx list
> > Subject: Re: [PATCH] drm/amd/pp: Revert gfx/compute profile switch sysfs
> >
> > On 2018-02-27 11:27 AM, Alex Deucher wrote:
> >> On Tue, Feb 27, 2018 at 11:22 AM, Eric Huang
> <jinhuieric.hu...@amd.com> wrote:
> >>> As I mentioned in code review for new power profile, old
> gfx/compute power
> >>> profile have two scenarios for auto switching. One is
> >>> gfx->compute(default)->gfx and other is gfx->compute(custom)->gfx.
> New power
> >>> profile only satisfies first one, but in second one for user
> debugging, user
> >>> setting of power profile will be over-written when switching to
> compute
> >>> profile.
> >> For debugging, the idea would be to select manual mode via
> >> force_performance_level and then force a profile via sysfs.  If manual
> >> mode was selected, none of the automatic profile changing would
> >> happen.  Is that adequate or do you need the automatic switching even
> >> with a custom profile?
> > It would result in higher power consumption while no compute work is
> > running by applying minimum clocks (if that's still part of the
> > profile). If minimum clock is not part of the profile, then that's
> > another major regression for us.
> >
> > I would prefer to add the ability to tweak all the power profiles so we
> > can have workload-specific customization, not just for compute. Maybe
> > that's something we can add later.
> >
&

Re: [PATCH] drm/amd/pp: Revert gfx/compute profile switch sysfs

2018-02-28 Thread Felix Kuehling 
Hi Rex,

The purpose of the compute power profile for us is to optimize
performance. Compute workloads tend to be "bursty". The default
heuristics of the SMU don't work well for such work loads. They aren't
able to raise clocks quickly enough to accelerate short bursts of
compute work.

We need to quickly raise clocks when needed. That's why we change
hysteresis parameters, and set a minimum clock. This minimizes the
latency of reaching maximum clocks.

The goal is not to set a fixed frequency, but to minimize the time
needed to reach the highest frequency. Setting fixed DPM level 7 will
not allow us to reach level 8. That will hurt our performance.

Regards,
  Felix


On 2018-02-28 06:08 AM, Zhu, Rex wrote:
> I went through the power profiling feature in windows - Auto Wattman 
> supported on Polaris and Vega. Initially apply to Polaris.
>
> The Goal is to maximize performance/watt without impacting performance and no 
> visual impact per system/use case
>
> On smu7, 3 parameters control how SMU responds to sclk/mclk activity, up/down 
> hysteresis and activity threshold.
>
> My understanding is:
> When sclk_activity large than threshold, adjust uphyst/downhyst/activity 
> threshold to go higher dpm level more faster or stay in higher dpm level 
> longer for boost performance.
> Otherwise, for power saving.
>
> Those parameters were used to change the behavior of dpm switching for 
> performance in watt. So we wish to add an independent sysfs to configure them.
>
> When this feature enabled, the performance in watt is improved in most 
> case(low activity and Multimedia) from pplib's test. I do not find compute 
> test result.
>
> So there is a question: 
>
> When compute ring begin, if we just fix sclk in level7 directly, , 
> Is there any difference in performance or performance in watt, compared to 
> set compute profile mode (include apply min sclk to level6)?
>
>
> Best Regards
> Rex
>
>
>
>
>
> -Original Message-
> From: Kuehling, Felix 
> Sent: Wednesday, February 28, 2018 2:55 AM
> To: Alex Deucher; Huang, JinHuiEric
> Cc: Zhu, Rex; Russell, Kent; amd-gfx list
> Subject: Re: [PATCH] drm/amd/pp: Revert gfx/compute profile switch sysfs
>
> On 2018-02-27 11:27 AM, Alex Deucher wrote:
>> On Tue, Feb 27, 2018 at 11:22 AM, Eric Huang <jinhuieric.hu...@amd.com> 
>> wrote:
>>> As I mentioned in code review for new power profile, old gfx/compute power
>>> profile have two scenarios for auto switching. One is
>>> gfx->compute(default)->gfx and other is gfx->compute(custom)->gfx. New power
>>> profile only satisfies first one, but in second one for user debugging, user
>>> setting of power profile will be over-written when switching to compute
>>> profile.
>> For debugging, the idea would be to select manual mode via
>> force_performance_level and then force a profile via sysfs.  If manual
>> mode was selected, none of the automatic profile changing would
>> happen.  Is that adequate or do you need the automatic switching even
>> with a custom profile?
> It would result in higher power consumption while no compute work is
> running by applying minimum clocks (if that's still part of the
> profile). If minimum clock is not part of the profile, then that's
> another major regression for us.
>
> I would prefer to add the ability to tweak all the power profiles so we
> can have workload-specific customization, not just for compute. Maybe
> that's something we can add later.
>
> Regards,
>   Felix
>
>> Alex
>>
>>
>>> Regards,
>>> Eric
>>>
>>> On 2018-02-27 10:52 AM, Felix Kuehling wrote:
>>>> [+Eric]
>>>>
>>>> Compute profile switching code as well as KFD compute support for most
>>>> GPUs is not upstream yet. As such, there is probably no requirement
>>>> (yet) to keep the compute profile API stable, that we added specifically
>>>> for KFD. Once we are upstream that will change.
>>>>
>>>> If you change it now, we'll have to adapt, like we have often done.
>>>>
>>>> I'll let Eric comment on whether the new power profile code and API is
>>>> an adequate replacement from a functionality perspective. Eric, Kent,
>>>> have we done any testing with Rex's new profile code?
>>>>
>>>> Regards,
>>>>Felix
>>>>
>>>>
>>>> On 2018-02-27 10:41 AM, Alex Deucher wrote:
>>>>> + Kent and Felix for comment
>>>>>
>>>>> On Tue, Feb 27, 2018 at 6:21 AM, Rex Zhu <rex@amd.com> wrote:
>>>>>

RE: [PATCH] drm/amd/pp: Revert gfx/compute profile switch sysfs

2018-02-28 Thread Zhu, Rex 
I went through the power profiling feature in windows - Auto Wattman supported 
on Polaris and Vega. Initially apply to Polaris.

The Goal is to maximize performance/watt without impacting performance and no 
visual impact per system/use case

On smu7, 3 parameters control how SMU responds to sclk/mclk activity, up/down 
hysteresis and activity threshold.

My understanding is:
When sclk_activity large than threshold, adjust uphyst/downhyst/activity 
threshold to go higher dpm level more faster or stay in higher dpm level longer 
for boost performance.
Otherwise, for power saving.

Those parameters were used to change the behavior of dpm switching for 
performance in watt. So we wish to add an independent sysfs to configure them.

When this feature enabled, the performance in watt is improved in most case(low 
activity and Multimedia) from pplib's test. I do not find compute test result.

So there is a question: 

When compute ring begin, if we just fix sclk in level7 directly, , 
Is there any difference in performance or performance in watt, compared to set 
compute profile mode (include apply min sclk to level6)?


Best Regards
Rex





-Original Message-
From: Kuehling, Felix 
Sent: Wednesday, February 28, 2018 2:55 AM
To: Alex Deucher; Huang, JinHuiEric
Cc: Zhu, Rex; Russell, Kent; amd-gfx list
Subject: Re: [PATCH] drm/amd/pp: Revert gfx/compute profile switch sysfs

On 2018-02-27 11:27 AM, Alex Deucher wrote:
> On Tue, Feb 27, 2018 at 11:22 AM, Eric Huang <jinhuieric.hu...@amd.com> wrote:
>> As I mentioned in code review for new power profile, old gfx/compute power
>> profile have two scenarios for auto switching. One is
>> gfx->compute(default)->gfx and other is gfx->compute(custom)->gfx. New power
>> profile only satisfies first one, but in second one for user debugging, user
>> setting of power profile will be over-written when switching to compute
>> profile.
> For debugging, the idea would be to select manual mode via
> force_performance_level and then force a profile via sysfs.  If manual
> mode was selected, none of the automatic profile changing would
> happen.  Is that adequate or do you need the automatic switching even
> with a custom profile?

It would result in higher power consumption while no compute work is
running by applying minimum clocks (if that's still part of the
profile). If minimum clock is not part of the profile, then that's
another major regression for us.

I would prefer to add the ability to tweak all the power profiles so we
can have workload-specific customization, not just for compute. Maybe
that's something we can add later.

Regards,
  Felix

>
> Alex
>
>
>> Regards,
>> Eric
>>
>> On 2018-02-27 10:52 AM, Felix Kuehling wrote:
>>> [+Eric]
>>>
>>> Compute profile switching code as well as KFD compute support for most
>>> GPUs is not upstream yet. As such, there is probably no requirement
>>> (yet) to keep the compute profile API stable, that we added specifically
>>> for KFD. Once we are upstream that will change.
>>>
>>> If you change it now, we'll have to adapt, like we have often done.
>>>
>>> I'll let Eric comment on whether the new power profile code and API is
>>> an adequate replacement from a functionality perspective. Eric, Kent,
>>> have we done any testing with Rex's new profile code?
>>>
>>> Regards,
>>>Felix
>>>
>>>
>>> On 2018-02-27 10:41 AM, Alex Deucher wrote:
>>>> + Kent and Felix for comment
>>>>
>>>> On Tue, Feb 27, 2018 at 6:21 AM, Rex Zhu <rex@amd.com> wrote:
>>>>> The gfx/compute profiling mode switch is only for internally
>>>>> test. Not a complete solution and unexpectly upstream.
>>>>> so revert it.
>>>>>
>>>>> Change-Id: I1af1b64a63b6fc12c24cf73df03b083b3661ca02
>>>>> Signed-off-by: Rex Zhu <rex@amd.com>
>>>>> ---
>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h|   8 -
>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 180
>>>>> ---
>>>>>   drivers/gpu/drm/amd/amdgpu/ci_dpm.c| 256
>>>>> -
>>>>>   drivers/gpu/drm/amd/amdgpu/ci_dpm.h|   7 -
>>>>>   drivers/gpu/drm/amd/include/kgd_pp_interface.h |   7 -
>>>>>   drivers/gpu/drm/amd/powerplay/amd_powerplay.c  | 114 -
>>>>>   .../gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c  |  17 --
>>>>>   drivers/gpu/drm/amd/powerplay/hwmgr/pp_psm.c   |   2 -
>>>>>   drivers/gpu/drm/amd/powerpla

RE: [PATCH] drm/amd/pp: Revert gfx/compute profile switch sysfs

2018-02-27 Thread Russell, Kent 
To answer this question, we haven't tested anything intentionally on the farm. 
We don't really have a test to test profiles aside from ensuring that sysfs 
returns what we set, and even then that's not part of automation.

 Kent

-Original Message-
From: Kuehling, Felix 
Sent: Tuesday, February 27, 2018 10:52 AM
To: Alex Deucher; Zhu, Rex; Russell, Kent; Huang, JinHuiEric
Cc: amd-gfx list
Subject: Re: [PATCH] drm/amd/pp: Revert gfx/compute profile switch sysfs

[+Eric]

Compute profile switching code as well as KFD compute support for most
GPUs is not upstream yet. As such, there is probably no requirement
(yet) to keep the compute profile API stable, that we added specifically
for KFD. Once we are upstream that will change.

If you change it now, we'll have to adapt, like we have often done.

I'll let Eric comment on whether the new power profile code and API is
an adequate replacement from a functionality perspective. Eric, Kent,
have we done any testing with Rex's new profile code?

Regards,
  Felix


On 2018-02-27 10:41 AM, Alex Deucher wrote:
> + Kent and Felix for comment
>
> On Tue, Feb 27, 2018 at 6:21 AM, Rex Zhu <rex@amd.com> wrote:
>> The gfx/compute profiling mode switch is only for internally
>> test. Not a complete solution and unexpectly upstream.
>> so revert it.
>>
>> Change-Id: I1af1b64a63b6fc12c24cf73df03b083b3661ca02
>> Signed-off-by: Rex Zhu <rex@amd.com>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h|   8 -
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 180 ---
>>  drivers/gpu/drm/amd/amdgpu/ci_dpm.c| 256 
>> -
>>  drivers/gpu/drm/amd/amdgpu/ci_dpm.h|   7 -
>>  drivers/gpu/drm/amd/include/kgd_pp_interface.h |   7 -
>>  drivers/gpu/drm/amd/powerplay/amd_powerplay.c  | 114 -
>>  .../gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c  |  17 --
>>  drivers/gpu/drm/amd/powerplay/hwmgr/pp_psm.c   |   2 -
>>  drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c   |  84 ---
>>  drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c |  86 ---
>>  .../gpu/drm/amd/powerplay/inc/hardwaremanager.h|   1 -
>>  drivers/gpu/drm/amd/powerplay/inc/hwmgr.h  |   9 -
>>  drivers/gpu/drm/amd/powerplay/inc/smumgr.h |   3 -
>>  drivers/gpu/drm/amd/powerplay/smumgr/ci_smumgr.c   |  27 ---
>>  drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c |  66 --
>>  .../drm/amd/powerplay/smumgr/polaris10_smumgr.c|  64 --
>>  drivers/gpu/drm/amd/powerplay/smumgr/smumgr.c  |  10 -
>>  .../gpu/drm/amd/powerplay/smumgr/tonga_smumgr.c|  64 --
>>  18 files changed, 1005 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
>> index bd745a4..9c373f8f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
>> @@ -341,14 +341,6 @@ enum amdgpu_pcie_gen {
>> ((adev)->powerplay.pp_funcs->reset_power_profile_state(\
>> (adev)->powerplay.pp_handle, request))
>>
>> -#define amdgpu_dpm_get_power_profile_state(adev, query) \
>> -   ((adev)->powerplay.pp_funcs->get_power_profile_state(\
>> -   (adev)->powerplay.pp_handle, query))
>> -
>> -#define amdgpu_dpm_set_power_profile_state(adev, request) \
>> -   ((adev)->powerplay.pp_funcs->set_power_profile_state(\
>> -   (adev)->powerplay.pp_handle, request))
>> -
>>  #define amdgpu_dpm_switch_power_profile(adev, type) \
>> ((adev)->powerplay.pp_funcs->switch_power_profile(\
>> (adev)->powerplay.pp_handle, type))
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
>> index 9e73cbc..632b186 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
>> @@ -734,161 +734,6 @@ static ssize_t amdgpu_set_pp_power_profile_mode(struct 
>> device *dev,
>> return -EINVAL;
>>  }
>>
>> -static ssize_t amdgpu_get_pp_power_profile(struct device *dev,
>> -   char *buf, struct amd_pp_profile *query)
>> -{
>> -   struct drm_device *ddev = dev_get_drvdata(dev);
>> -   struct amdgpu_device *adev = ddev->dev_private;
>> -   int ret = 0xff;
>> -
>> -   if (adev->powerplay.pp_funcs->get_power_profile_state)
>> -   ret = amdgpu_dpm_get_power_profile_state(
>> -   adev, que

Re: [PATCH] drm/amd/pp: Revert gfx/compute profile switch sysfs

2018-02-27 Thread Felix Kuehling 
On 2018-02-27 11:27 AM, Alex Deucher wrote:
> On Tue, Feb 27, 2018 at 11:22 AM, Eric Huang  wrote:
>> As I mentioned in code review for new power profile, old gfx/compute power
>> profile have two scenarios for auto switching. One is
>> gfx->compute(default)->gfx and other is gfx->compute(custom)->gfx. New power
>> profile only satisfies first one, but in second one for user debugging, user
>> setting of power profile will be over-written when switching to compute
>> profile.
> For debugging, the idea would be to select manual mode via
> force_performance_level and then force a profile via sysfs.  If manual
> mode was selected, none of the automatic profile changing would
> happen.  Is that adequate or do you need the automatic switching even
> with a custom profile?

It would result in higher power consumption while no compute work is
running by applying minimum clocks (if that's still part of the
profile). If minimum clock is not part of the profile, then that's
another major regression for us.

I would prefer to add the ability to tweak all the power profiles so we
can have workload-specific customization, not just for compute. Maybe
that's something we can add later.

Regards,
  Felix

>
> Alex
>
>
>> Regards,
>> Eric
>>
>> On 2018-02-27 10:52 AM, Felix Kuehling wrote:
>>> [+Eric]
>>>
>>> Compute profile switching code as well as KFD compute support for most
>>> GPUs is not upstream yet. As such, there is probably no requirement
>>> (yet) to keep the compute profile API stable, that we added specifically
>>> for KFD. Once we are upstream that will change.
>>>
>>> If you change it now, we'll have to adapt, like we have often done.
>>>
>>> I'll let Eric comment on whether the new power profile code and API is
>>> an adequate replacement from a functionality perspective. Eric, Kent,
>>> have we done any testing with Rex's new profile code?
>>>
>>> Regards,
>>>Felix
>>>
>>>
>>> On 2018-02-27 10:41 AM, Alex Deucher wrote:
 + Kent and Felix for comment

 On Tue, Feb 27, 2018 at 6:21 AM, Rex Zhu  wrote:
> The gfx/compute profiling mode switch is only for internally
> test. Not a complete solution and unexpectly upstream.
> so revert it.
>
> Change-Id: I1af1b64a63b6fc12c24cf73df03b083b3661ca02
> Signed-off-by: Rex Zhu 
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h|   8 -
>   drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 180
> ---
>   drivers/gpu/drm/amd/amdgpu/ci_dpm.c| 256
> -
>   drivers/gpu/drm/amd/amdgpu/ci_dpm.h|   7 -
>   drivers/gpu/drm/amd/include/kgd_pp_interface.h |   7 -
>   drivers/gpu/drm/amd/powerplay/amd_powerplay.c  | 114 -
>   .../gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c  |  17 --
>   drivers/gpu/drm/amd/powerplay/hwmgr/pp_psm.c   |   2 -
>   drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c   |  84 ---
>   drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c |  86 ---
>   .../gpu/drm/amd/powerplay/inc/hardwaremanager.h|   1 -
>   drivers/gpu/drm/amd/powerplay/inc/hwmgr.h  |   9 -
>   drivers/gpu/drm/amd/powerplay/inc/smumgr.h |   3 -
>   drivers/gpu/drm/amd/powerplay/smumgr/ci_smumgr.c   |  27 ---
>   drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c |  66 --
>   .../drm/amd/powerplay/smumgr/polaris10_smumgr.c|  64 --
>   drivers/gpu/drm/amd/powerplay/smumgr/smumgr.c  |  10 -
>   .../gpu/drm/amd/powerplay/smumgr/tonga_smumgr.c|  64 --
>   18 files changed, 1005 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
> index bd745a4..9c373f8f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
> @@ -341,14 +341,6 @@ enum amdgpu_pcie_gen {
>
> ((adev)->powerplay.pp_funcs->reset_power_profile_state(\
>  (adev)->powerplay.pp_handle, request))
>
> -#define amdgpu_dpm_get_power_profile_state(adev, query) \
> -   ((adev)->powerplay.pp_funcs->get_power_profile_state(\
> -   (adev)->powerplay.pp_handle, query))
> -
> -#define amdgpu_dpm_set_power_profile_state(adev, request) \
> -   ((adev)->powerplay.pp_funcs->set_power_profile_state(\
> -   (adev)->powerplay.pp_handle, request))
> -
>   #define amdgpu_dpm_switch_power_profile(adev, type) \
>  ((adev)->powerplay.pp_funcs->switch_power_profile(\
>  (adev)->powerplay.pp_handle, type))
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> index 9e73cbc..632b186 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> +++

Re: [PATCH] drm/amd/pp: Revert gfx/compute profile switch sysfs

2018-02-27 Thread Eric Huang 

Yes. That is a solution.

Eric


On 2018-02-27 11:27 AM, Alex Deucher wrote:

On Tue, Feb 27, 2018 at 11:22 AM, Eric Huang  wrote:

As I mentioned in code review for new power profile, old gfx/compute power
profile have two scenarios for auto switching. One is
gfx->compute(default)->gfx and other is gfx->compute(custom)->gfx. New power
profile only satisfies first one, but in second one for user debugging, user
setting of power profile will be over-written when switching to compute
profile.

For debugging, the idea would be to select manual mode via
force_performance_level and then force a profile via sysfs.  If manual
mode was selected, none of the automatic profile changing would
happen.  Is that adequate or do you need the automatic switching even
with a custom profile?

Alex



Regards,
Eric

On 2018-02-27 10:52 AM, Felix Kuehling wrote:

[+Eric]

Compute profile switching code as well as KFD compute support for most
GPUs is not upstream yet. As such, there is probably no requirement
(yet) to keep the compute profile API stable, that we added specifically
for KFD. Once we are upstream that will change.

If you change it now, we'll have to adapt, like we have often done.

I'll let Eric comment on whether the new power profile code and API is
an adequate replacement from a functionality perspective. Eric, Kent,
have we done any testing with Rex's new profile code?

Regards,
Felix


On 2018-02-27 10:41 AM, Alex Deucher wrote:

+ Kent and Felix for comment

On Tue, Feb 27, 2018 at 6:21 AM, Rex Zhu  wrote:

The gfx/compute profiling mode switch is only for internally
test. Not a complete solution and unexpectly upstream.
so revert it.

Change-Id: I1af1b64a63b6fc12c24cf73df03b083b3661ca02
Signed-off-by: Rex Zhu 
---
   drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h|   8 -
   drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 180
---
   drivers/gpu/drm/amd/amdgpu/ci_dpm.c| 256
-
   drivers/gpu/drm/amd/amdgpu/ci_dpm.h|   7 -
   drivers/gpu/drm/amd/include/kgd_pp_interface.h |   7 -
   drivers/gpu/drm/amd/powerplay/amd_powerplay.c  | 114 -
   .../gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c  |  17 --
   drivers/gpu/drm/amd/powerplay/hwmgr/pp_psm.c   |   2 -
   drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c   |  84 ---
   drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c |  86 ---
   .../gpu/drm/amd/powerplay/inc/hardwaremanager.h|   1 -
   drivers/gpu/drm/amd/powerplay/inc/hwmgr.h  |   9 -
   drivers/gpu/drm/amd/powerplay/inc/smumgr.h |   3 -
   drivers/gpu/drm/amd/powerplay/smumgr/ci_smumgr.c   |  27 ---
   drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c |  66 --
   .../drm/amd/powerplay/smumgr/polaris10_smumgr.c|  64 --
   drivers/gpu/drm/amd/powerplay/smumgr/smumgr.c  |  10 -
   .../gpu/drm/amd/powerplay/smumgr/tonga_smumgr.c|  64 --
   18 files changed, 1005 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
index bd745a4..9c373f8f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
@@ -341,14 +341,6 @@ enum amdgpu_pcie_gen {

((adev)->powerplay.pp_funcs->reset_power_profile_state(\
  (adev)->powerplay.pp_handle, request))

-#define amdgpu_dpm_get_power_profile_state(adev, query) \
-   ((adev)->powerplay.pp_funcs->get_power_profile_state(\
-   (adev)->powerplay.pp_handle, query))
-
-#define amdgpu_dpm_set_power_profile_state(adev, request) \
-   ((adev)->powerplay.pp_funcs->set_power_profile_state(\
-   (adev)->powerplay.pp_handle, request))
-
   #define amdgpu_dpm_switch_power_profile(adev, type) \
  ((adev)->powerplay.pp_funcs->switch_power_profile(\
  (adev)->powerplay.pp_handle, type))
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index 9e73cbc..632b186 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -734,161 +734,6 @@ static ssize_t
amdgpu_set_pp_power_profile_mode(struct device *dev,
  return -EINVAL;
   }

-static ssize_t amdgpu_get_pp_power_profile(struct device *dev,
-   char *buf, struct amd_pp_profile *query)
-{
-   struct drm_device *ddev = dev_get_drvdata(dev);
-   struct amdgpu_device *adev = ddev->dev_private;
-   int ret = 0xff;
-
-   if (adev->powerplay.pp_funcs->get_power_profile_state)
-   ret = amdgpu_dpm_get_power_profile_state(
-   adev, query);
-
-   if (ret)
-   return ret;
-
-   return snprintf(buf, PAGE_SIZE,
-   "%d %d %d %d %d\n",
-   query->min_sclk / 100,
-   query->min_mclk / 100,
-

Re: [PATCH] drm/amd/pp: Revert gfx/compute profile switch sysfs

2018-02-27 Thread Alex Deucher 
On Tue, Feb 27, 2018 at 11:22 AM, Eric Huang  wrote:
> As I mentioned in code review for new power profile, old gfx/compute power
> profile have two scenarios for auto switching. One is
> gfx->compute(default)->gfx and other is gfx->compute(custom)->gfx. New power
> profile only satisfies first one, but in second one for user debugging, user
> setting of power profile will be over-written when switching to compute
> profile.

For debugging, the idea would be to select manual mode via
force_performance_level and then force a profile via sysfs.  If manual
mode was selected, none of the automatic profile changing would
happen.  Is that adequate or do you need the automatic switching even
with a custom profile?

Alex


>
> Regards,
> Eric
>
> On 2018-02-27 10:52 AM, Felix Kuehling wrote:
>>
>> [+Eric]
>>
>> Compute profile switching code as well as KFD compute support for most
>> GPUs is not upstream yet. As such, there is probably no requirement
>> (yet) to keep the compute profile API stable, that we added specifically
>> for KFD. Once we are upstream that will change.
>>
>> If you change it now, we'll have to adapt, like we have often done.
>>
>> I'll let Eric comment on whether the new power profile code and API is
>> an adequate replacement from a functionality perspective. Eric, Kent,
>> have we done any testing with Rex's new profile code?
>>
>> Regards,
>>Felix
>>
>>
>> On 2018-02-27 10:41 AM, Alex Deucher wrote:
>>>
>>> + Kent and Felix for comment
>>>
>>> On Tue, Feb 27, 2018 at 6:21 AM, Rex Zhu  wrote:

 The gfx/compute profiling mode switch is only for internally
 test. Not a complete solution and unexpectly upstream.
 so revert it.

 Change-Id: I1af1b64a63b6fc12c24cf73df03b083b3661ca02
 Signed-off-by: Rex Zhu 
 ---
   drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h|   8 -
   drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 180
 ---
   drivers/gpu/drm/amd/amdgpu/ci_dpm.c| 256
 -
   drivers/gpu/drm/amd/amdgpu/ci_dpm.h|   7 -
   drivers/gpu/drm/amd/include/kgd_pp_interface.h |   7 -
   drivers/gpu/drm/amd/powerplay/amd_powerplay.c  | 114 -
   .../gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c  |  17 --
   drivers/gpu/drm/amd/powerplay/hwmgr/pp_psm.c   |   2 -
   drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c   |  84 ---
   drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c |  86 ---
   .../gpu/drm/amd/powerplay/inc/hardwaremanager.h|   1 -
   drivers/gpu/drm/amd/powerplay/inc/hwmgr.h  |   9 -
   drivers/gpu/drm/amd/powerplay/inc/smumgr.h |   3 -
   drivers/gpu/drm/amd/powerplay/smumgr/ci_smumgr.c   |  27 ---
   drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c |  66 --
   .../drm/amd/powerplay/smumgr/polaris10_smumgr.c|  64 --
   drivers/gpu/drm/amd/powerplay/smumgr/smumgr.c  |  10 -
   .../gpu/drm/amd/powerplay/smumgr/tonga_smumgr.c|  64 --
   18 files changed, 1005 deletions(-)

 diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
 b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
 index bd745a4..9c373f8f 100644
 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
 +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
 @@ -341,14 +341,6 @@ enum amdgpu_pcie_gen {

 ((adev)->powerplay.pp_funcs->reset_power_profile_state(\
  (adev)->powerplay.pp_handle, request))

 -#define amdgpu_dpm_get_power_profile_state(adev, query) \
 -   ((adev)->powerplay.pp_funcs->get_power_profile_state(\
 -   (adev)->powerplay.pp_handle, query))
 -
 -#define amdgpu_dpm_set_power_profile_state(adev, request) \
 -   ((adev)->powerplay.pp_funcs->set_power_profile_state(\
 -   (adev)->powerplay.pp_handle, request))
 -
   #define amdgpu_dpm_switch_power_profile(adev, type) \
  ((adev)->powerplay.pp_funcs->switch_power_profile(\
  (adev)->powerplay.pp_handle, type))
 diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
 b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
 index 9e73cbc..632b186 100644
 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
 +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
 @@ -734,161 +734,6 @@ static ssize_t
 amdgpu_set_pp_power_profile_mode(struct device *dev,
  return -EINVAL;
   }

 -static ssize_t amdgpu_get_pp_power_profile(struct device *dev,
 -   char *buf, struct amd_pp_profile *query)
 -{
 -   struct drm_device *ddev = dev_get_drvdata(dev);
 -   struct amdgpu_device *adev = ddev->dev_private;
 -   int ret = 0xff;
 -
 -   if (adev->powerplay.pp_funcs->get_power_profile_state)
 -

Re: [PATCH] drm/amd/pp: Revert gfx/compute profile switch sysfs

2018-02-27 Thread Eric Huang 
As I mentioned in code review for new power profile, old gfx/compute 
power profile have two scenarios for auto switching. One is 
gfx->compute(default)->gfx and other is gfx->compute(custom)->gfx. New 
power profile only satisfies first one, but in second one for user 
debugging, user setting of power profile will be over-written when 
switching to compute profile.


Regards,
Eric

On 2018-02-27 10:52 AM, Felix Kuehling wrote:

[+Eric]

Compute profile switching code as well as KFD compute support for most
GPUs is not upstream yet. As such, there is probably no requirement
(yet) to keep the compute profile API stable, that we added specifically
for KFD. Once we are upstream that will change.

If you change it now, we'll have to adapt, like we have often done.

I'll let Eric comment on whether the new power profile code and API is
an adequate replacement from a functionality perspective. Eric, Kent,
have we done any testing with Rex's new profile code?

Regards,
   Felix


On 2018-02-27 10:41 AM, Alex Deucher wrote:

+ Kent and Felix for comment

On Tue, Feb 27, 2018 at 6:21 AM, Rex Zhu  wrote:

The gfx/compute profiling mode switch is only for internally
test. Not a complete solution and unexpectly upstream.
so revert it.

Change-Id: I1af1b64a63b6fc12c24cf73df03b083b3661ca02
Signed-off-by: Rex Zhu 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h|   8 -
  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 180 ---
  drivers/gpu/drm/amd/amdgpu/ci_dpm.c| 256 -
  drivers/gpu/drm/amd/amdgpu/ci_dpm.h|   7 -
  drivers/gpu/drm/amd/include/kgd_pp_interface.h |   7 -
  drivers/gpu/drm/amd/powerplay/amd_powerplay.c  | 114 -
  .../gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c  |  17 --
  drivers/gpu/drm/amd/powerplay/hwmgr/pp_psm.c   |   2 -
  drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c   |  84 ---
  drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c |  86 ---
  .../gpu/drm/amd/powerplay/inc/hardwaremanager.h|   1 -
  drivers/gpu/drm/amd/powerplay/inc/hwmgr.h  |   9 -
  drivers/gpu/drm/amd/powerplay/inc/smumgr.h |   3 -
  drivers/gpu/drm/amd/powerplay/smumgr/ci_smumgr.c   |  27 ---
  drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c |  66 --
  .../drm/amd/powerplay/smumgr/polaris10_smumgr.c|  64 --
  drivers/gpu/drm/amd/powerplay/smumgr/smumgr.c  |  10 -
  .../gpu/drm/amd/powerplay/smumgr/tonga_smumgr.c|  64 --
  18 files changed, 1005 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
index bd745a4..9c373f8f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
@@ -341,14 +341,6 @@ enum amdgpu_pcie_gen {
 ((adev)->powerplay.pp_funcs->reset_power_profile_state(\
 (adev)->powerplay.pp_handle, request))

-#define amdgpu_dpm_get_power_profile_state(adev, query) \
-   ((adev)->powerplay.pp_funcs->get_power_profile_state(\
-   (adev)->powerplay.pp_handle, query))
-
-#define amdgpu_dpm_set_power_profile_state(adev, request) \
-   ((adev)->powerplay.pp_funcs->set_power_profile_state(\
-   (adev)->powerplay.pp_handle, request))
-
  #define amdgpu_dpm_switch_power_profile(adev, type) \
 ((adev)->powerplay.pp_funcs->switch_power_profile(\
 (adev)->powerplay.pp_handle, type))
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index 9e73cbc..632b186 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -734,161 +734,6 @@ static ssize_t amdgpu_set_pp_power_profile_mode(struct 
device *dev,
 return -EINVAL;
  }

-static ssize_t amdgpu_get_pp_power_profile(struct device *dev,
-   char *buf, struct amd_pp_profile *query)
-{
-   struct drm_device *ddev = dev_get_drvdata(dev);
-   struct amdgpu_device *adev = ddev->dev_private;
-   int ret = 0xff;
-
-   if (adev->powerplay.pp_funcs->get_power_profile_state)
-   ret = amdgpu_dpm_get_power_profile_state(
-   adev, query);
-
-   if (ret)
-   return ret;
-
-   return snprintf(buf, PAGE_SIZE,
-   "%d %d %d %d %d\n",
-   query->min_sclk / 100,
-   query->min_mclk / 100,
-   query->activity_threshold,
-   query->up_hyst,
-   query->down_hyst);
-}
-
-static ssize_t amdgpu_get_pp_gfx_power_profile(struct device *dev,
-   struct device_attribute *attr,
-   char *buf)
-{
-   struct amd_pp_profile query = {0};
-
-   query.type = AMD_PP_GFX_PROFILE;
-
-   return amdgpu_get_pp_power_profile(dev, buf, );
-}
-
-static ssize_t

Re: [PATCH] drm/amd/pp: Revert gfx/compute profile switch sysfs

2018-02-27 Thread Felix Kuehling 
[+Eric]

Compute profile switching code as well as KFD compute support for most
GPUs is not upstream yet. As such, there is probably no requirement
(yet) to keep the compute profile API stable, that we added specifically
for KFD. Once we are upstream that will change.

If you change it now, we'll have to adapt, like we have often done.

I'll let Eric comment on whether the new power profile code and API is
an adequate replacement from a functionality perspective. Eric, Kent,
have we done any testing with Rex's new profile code?

Regards,
  Felix


On 2018-02-27 10:41 AM, Alex Deucher wrote:
> + Kent and Felix for comment
>
> On Tue, Feb 27, 2018 at 6:21 AM, Rex Zhu  wrote:
>> The gfx/compute profiling mode switch is only for internally
>> test. Not a complete solution and unexpectly upstream.
>> so revert it.
>>
>> Change-Id: I1af1b64a63b6fc12c24cf73df03b083b3661ca02
>> Signed-off-by: Rex Zhu 
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h|   8 -
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 180 ---
>>  drivers/gpu/drm/amd/amdgpu/ci_dpm.c| 256 
>> -
>>  drivers/gpu/drm/amd/amdgpu/ci_dpm.h|   7 -
>>  drivers/gpu/drm/amd/include/kgd_pp_interface.h |   7 -
>>  drivers/gpu/drm/amd/powerplay/amd_powerplay.c  | 114 -
>>  .../gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c  |  17 --
>>  drivers/gpu/drm/amd/powerplay/hwmgr/pp_psm.c   |   2 -
>>  drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c   |  84 ---
>>  drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c |  86 ---
>>  .../gpu/drm/amd/powerplay/inc/hardwaremanager.h|   1 -
>>  drivers/gpu/drm/amd/powerplay/inc/hwmgr.h  |   9 -
>>  drivers/gpu/drm/amd/powerplay/inc/smumgr.h |   3 -
>>  drivers/gpu/drm/amd/powerplay/smumgr/ci_smumgr.c   |  27 ---
>>  drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c |  66 --
>>  .../drm/amd/powerplay/smumgr/polaris10_smumgr.c|  64 --
>>  drivers/gpu/drm/amd/powerplay/smumgr/smumgr.c  |  10 -
>>  .../gpu/drm/amd/powerplay/smumgr/tonga_smumgr.c|  64 --
>>  18 files changed, 1005 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
>> index bd745a4..9c373f8f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
>> @@ -341,14 +341,6 @@ enum amdgpu_pcie_gen {
>> ((adev)->powerplay.pp_funcs->reset_power_profile_state(\
>> (adev)->powerplay.pp_handle, request))
>>
>> -#define amdgpu_dpm_get_power_profile_state(adev, query) \
>> -   ((adev)->powerplay.pp_funcs->get_power_profile_state(\
>> -   (adev)->powerplay.pp_handle, query))
>> -
>> -#define amdgpu_dpm_set_power_profile_state(adev, request) \
>> -   ((adev)->powerplay.pp_funcs->set_power_profile_state(\
>> -   (adev)->powerplay.pp_handle, request))
>> -
>>  #define amdgpu_dpm_switch_power_profile(adev, type) \
>> ((adev)->powerplay.pp_funcs->switch_power_profile(\
>> (adev)->powerplay.pp_handle, type))
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
>> index 9e73cbc..632b186 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
>> @@ -734,161 +734,6 @@ static ssize_t amdgpu_set_pp_power_profile_mode(struct 
>> device *dev,
>> return -EINVAL;
>>  }
>>
>> -static ssize_t amdgpu_get_pp_power_profile(struct device *dev,
>> -   char *buf, struct amd_pp_profile *query)
>> -{
>> -   struct drm_device *ddev = dev_get_drvdata(dev);
>> -   struct amdgpu_device *adev = ddev->dev_private;
>> -   int ret = 0xff;
>> -
>> -   if (adev->powerplay.pp_funcs->get_power_profile_state)
>> -   ret = amdgpu_dpm_get_power_profile_state(
>> -   adev, query);
>> -
>> -   if (ret)
>> -   return ret;
>> -
>> -   return snprintf(buf, PAGE_SIZE,
>> -   "%d %d %d %d %d\n",
>> -   query->min_sclk / 100,
>> -   query->min_mclk / 100,
>> -   query->activity_threshold,
>> -   query->up_hyst,
>> -   query->down_hyst);
>> -}
>> -
>> -static ssize_t amdgpu_get_pp_gfx_power_profile(struct device *dev,
>> -   struct device_attribute *attr,
>> -   char *buf)
>> -{
>> -   struct amd_pp_profile query = {0};
>> -
>> -   query.type = AMD_PP_GFX_PROFILE;
>> -
>> -   return amdgpu_get_pp_power_profile(dev, buf, );
>> -}
>> -
>> -static ssize_t amdgpu_get_pp_compute_power_profile(struct device *dev,
>> -   struct device_attribute *attr,
>> -   char *buf)
>> -{
>> -   struct amd_pp_profile query = {0};
>> -

Re: [PATCH] drm/amd/pp: Revert gfx/compute profile switch sysfs

2018-02-27 Thread Alex Deucher 
+ Kent and Felix for comment

On Tue, Feb 27, 2018 at 6:21 AM, Rex Zhu  wrote:
> The gfx/compute profiling mode switch is only for internally
> test. Not a complete solution and unexpectly upstream.
> so revert it.
>
> Change-Id: I1af1b64a63b6fc12c24cf73df03b083b3661ca02
> Signed-off-by: Rex Zhu 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h|   8 -
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 180 ---
>  drivers/gpu/drm/amd/amdgpu/ci_dpm.c| 256 
> -
>  drivers/gpu/drm/amd/amdgpu/ci_dpm.h|   7 -
>  drivers/gpu/drm/amd/include/kgd_pp_interface.h |   7 -
>  drivers/gpu/drm/amd/powerplay/amd_powerplay.c  | 114 -
>  .../gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c  |  17 --
>  drivers/gpu/drm/amd/powerplay/hwmgr/pp_psm.c   |   2 -
>  drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c   |  84 ---
>  drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c |  86 ---
>  .../gpu/drm/amd/powerplay/inc/hardwaremanager.h|   1 -
>  drivers/gpu/drm/amd/powerplay/inc/hwmgr.h  |   9 -
>  drivers/gpu/drm/amd/powerplay/inc/smumgr.h |   3 -
>  drivers/gpu/drm/amd/powerplay/smumgr/ci_smumgr.c   |  27 ---
>  drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c |  66 --
>  .../drm/amd/powerplay/smumgr/polaris10_smumgr.c|  64 --
>  drivers/gpu/drm/amd/powerplay/smumgr/smumgr.c  |  10 -
>  .../gpu/drm/amd/powerplay/smumgr/tonga_smumgr.c|  64 --
>  18 files changed, 1005 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
> index bd745a4..9c373f8f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
> @@ -341,14 +341,6 @@ enum amdgpu_pcie_gen {
> ((adev)->powerplay.pp_funcs->reset_power_profile_state(\
> (adev)->powerplay.pp_handle, request))
>
> -#define amdgpu_dpm_get_power_profile_state(adev, query) \
> -   ((adev)->powerplay.pp_funcs->get_power_profile_state(\
> -   (adev)->powerplay.pp_handle, query))
> -
> -#define amdgpu_dpm_set_power_profile_state(adev, request) \
> -   ((adev)->powerplay.pp_funcs->set_power_profile_state(\
> -   (adev)->powerplay.pp_handle, request))
> -
>  #define amdgpu_dpm_switch_power_profile(adev, type) \
> ((adev)->powerplay.pp_funcs->switch_power_profile(\
> (adev)->powerplay.pp_handle, type))
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> index 9e73cbc..632b186 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> @@ -734,161 +734,6 @@ static ssize_t amdgpu_set_pp_power_profile_mode(struct 
> device *dev,
> return -EINVAL;
>  }
>
> -static ssize_t amdgpu_get_pp_power_profile(struct device *dev,
> -   char *buf, struct amd_pp_profile *query)
> -{
> -   struct drm_device *ddev = dev_get_drvdata(dev);
> -   struct amdgpu_device *adev = ddev->dev_private;
> -   int ret = 0xff;
> -
> -   if (adev->powerplay.pp_funcs->get_power_profile_state)
> -   ret = amdgpu_dpm_get_power_profile_state(
> -   adev, query);
> -
> -   if (ret)
> -   return ret;
> -
> -   return snprintf(buf, PAGE_SIZE,
> -   "%d %d %d %d %d\n",
> -   query->min_sclk / 100,
> -   query->min_mclk / 100,
> -   query->activity_threshold,
> -   query->up_hyst,
> -   query->down_hyst);
> -}
> -
> -static ssize_t amdgpu_get_pp_gfx_power_profile(struct device *dev,
> -   struct device_attribute *attr,
> -   char *buf)
> -{
> -   struct amd_pp_profile query = {0};
> -
> -   query.type = AMD_PP_GFX_PROFILE;
> -
> -   return amdgpu_get_pp_power_profile(dev, buf, );
> -}
> -
> -static ssize_t amdgpu_get_pp_compute_power_profile(struct device *dev,
> -   struct device_attribute *attr,
> -   char *buf)
> -{
> -   struct amd_pp_profile query = {0};
> -
> -   query.type = AMD_PP_COMPUTE_PROFILE;
> -
> -   return amdgpu_get_pp_power_profile(dev, buf, );
> -}
> -
> -static ssize_t amdgpu_set_pp_power_profile(struct device *dev,
> -   const char *buf,
> -   size_t count,
> -   struct amd_pp_profile *request)
> -{
> -   struct drm_device *ddev = dev_get_drvdata(dev);
> -   struct amdgpu_device *adev = ddev->dev_private;
> -   uint32_t loop = 0;
> -   char *sub_str, buf_cpy[128], *tmp_str;
> -   const char delimiter[3] = {' ', '\n', '\0'};
> -   long int value;
> -   int ret = 0xff;
> -
> -   if (strncmp("reset", buf, strlen("reset")) == 0) {
> -   if

Re: [PATCH] drm/amd/pp: Revert gfx/compute profile switch sysfs

2018-02-27 Thread Christian König 

Am 27.02.2018 um 12:21 schrieb Rex Zhu:

The gfx/compute profiling mode switch is only for internally
test. Not a complete solution and unexpectly upstream.
so revert it.

Change-Id: I1af1b64a63b6fc12c24cf73df03b083b3661ca02
Signed-off-by: Rex Zhu 


Patch is Acked-by: Christian König .

Let's hope that really nobody used this upstream.

Regards,
Christian.


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h|   8 -
  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 180 ---
  drivers/gpu/drm/amd/amdgpu/ci_dpm.c| 256 -
  drivers/gpu/drm/amd/amdgpu/ci_dpm.h|   7 -
  drivers/gpu/drm/amd/include/kgd_pp_interface.h |   7 -
  drivers/gpu/drm/amd/powerplay/amd_powerplay.c  | 114 -
  .../gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c  |  17 --
  drivers/gpu/drm/amd/powerplay/hwmgr/pp_psm.c   |   2 -
  drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c   |  84 ---
  drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c |  86 ---
  .../gpu/drm/amd/powerplay/inc/hardwaremanager.h|   1 -
  drivers/gpu/drm/amd/powerplay/inc/hwmgr.h  |   9 -
  drivers/gpu/drm/amd/powerplay/inc/smumgr.h |   3 -
  drivers/gpu/drm/amd/powerplay/smumgr/ci_smumgr.c   |  27 ---
  drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c |  66 --
  .../drm/amd/powerplay/smumgr/polaris10_smumgr.c|  64 --
  drivers/gpu/drm/amd/powerplay/smumgr/smumgr.c  |  10 -
  .../gpu/drm/amd/powerplay/smumgr/tonga_smumgr.c|  64 --
  18 files changed, 1005 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
index bd745a4..9c373f8f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
@@ -341,14 +341,6 @@ enum amdgpu_pcie_gen {
((adev)->powerplay.pp_funcs->reset_power_profile_state(\
(adev)->powerplay.pp_handle, request))
  
-#define amdgpu_dpm_get_power_profile_state(adev, query) \

-   ((adev)->powerplay.pp_funcs->get_power_profile_state(\
-   (adev)->powerplay.pp_handle, query))
-
-#define amdgpu_dpm_set_power_profile_state(adev, request) \
-   ((adev)->powerplay.pp_funcs->set_power_profile_state(\
-   (adev)->powerplay.pp_handle, request))
-
  #define amdgpu_dpm_switch_power_profile(adev, type) \
((adev)->powerplay.pp_funcs->switch_power_profile(\
(adev)->powerplay.pp_handle, type))
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index 9e73cbc..632b186 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -734,161 +734,6 @@ static ssize_t amdgpu_set_pp_power_profile_mode(struct 
device *dev,
return -EINVAL;
  }
  
-static ssize_t amdgpu_get_pp_power_profile(struct device *dev,

-   char *buf, struct amd_pp_profile *query)
-{
-   struct drm_device *ddev = dev_get_drvdata(dev);
-   struct amdgpu_device *adev = ddev->dev_private;
-   int ret = 0xff;
-
-   if (adev->powerplay.pp_funcs->get_power_profile_state)
-   ret = amdgpu_dpm_get_power_profile_state(
-   adev, query);
-
-   if (ret)
-   return ret;
-
-   return snprintf(buf, PAGE_SIZE,
-   "%d %d %d %d %d\n",
-   query->min_sclk / 100,
-   query->min_mclk / 100,
-   query->activity_threshold,
-   query->up_hyst,
-   query->down_hyst);
-}
-
-static ssize_t amdgpu_get_pp_gfx_power_profile(struct device *dev,
-   struct device_attribute *attr,
-   char *buf)
-{
-   struct amd_pp_profile query = {0};
-
-   query.type = AMD_PP_GFX_PROFILE;
-
-   return amdgpu_get_pp_power_profile(dev, buf, );
-}
-
-static ssize_t amdgpu_get_pp_compute_power_profile(struct device *dev,
-   struct device_attribute *attr,
-   char *buf)
-{
-   struct amd_pp_profile query = {0};
-
-   query.type = AMD_PP_COMPUTE_PROFILE;
-
-   return amdgpu_get_pp_power_profile(dev, buf, );
-}
-
-static ssize_t amdgpu_set_pp_power_profile(struct device *dev,
-   const char *buf,
-   size_t count,
-   struct amd_pp_profile *request)
-{
-   struct drm_device *ddev = dev_get_drvdata(dev);
-   struct amdgpu_device *adev = ddev->dev_private;
-   uint32_t loop = 0;
-   char *sub_str, buf_cpy[128], *tmp_str;
-   const char delimiter[3] = {' ', '\n', '\0'};
-   long int value;
-   int ret = 0xff;
-
-   if (strncmp("reset", buf, strlen("reset")) == 0) {
-   if (adev->powerplay.pp_funcs->reset_power_profile_state)
-   ret = amdgpu_dpm_reset_power_profile_state(
-