lkml.org 
[lkml]   [2023]   [Aug]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
SubjectRe: [PATCH v2 2/7] drm/amdgpu: Add new function to set GPU power profile
From

On 8/21/2023 11:40 PM, Alex Deucher wrote:
> On Mon, Aug 21, 2023 at 1:54 PM Yadav, Arvind <arvyadav@amd.com> wrote:
>>
>> On 8/21/2023 9:52 PM, Alex Deucher wrote:
>>> On Mon, Aug 21, 2023 at 2:55 AM Arvind Yadav <Arvind.Yadav@amd.com> wrote:
>>>> This patch adds a function which will change the GPU
>>>> power profile based on a submitted job. This can optimize
>>>> the power performance when the workload is on.
>>>>
>>>> v2:
>>>> - Splitting workload_profile_set and workload_profile_put
>>>> into two separate patches.
>>>> - Addressed review comment.
>>>>
>>>> Cc: Shashank Sharma <shashank.sharma@amd.com>
>>>> Cc: Christian Koenig <christian.koenig@amd.com>
>>>> Cc: Alex Deucher <alexander.deucher@amd.com>
>>>> Signed-off-by: Arvind Yadav <Arvind.Yadav@amd.com>
>>>> ---
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c | 56 +++++++++++++++++++
>>>> drivers/gpu/drm/amd/include/amdgpu_workload.h | 3 +
>>>> 2 files changed, 59 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c
>>>> index 32166f482f77..e661cc5b3d92 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c
>>>> @@ -24,6 +24,62 @@
>>>>
>>>> #include "amdgpu.h"
>>>>
>>>> +static enum PP_SMC_POWER_PROFILE
>>>> +ring_to_power_profile(uint32_t ring_type)
>>>> +{
>>>> + switch (ring_type) {
>>>> + case AMDGPU_RING_TYPE_GFX:
>>>> + return PP_SMC_POWER_PROFILE_FULLSCREEN3D;
>>>> + case AMDGPU_RING_TYPE_COMPUTE:
>>>> + return PP_SMC_POWER_PROFILE_COMPUTE;
>>>> + case AMDGPU_RING_TYPE_UVD:
>>>> + case AMDGPU_RING_TYPE_VCE:
>>>> + case AMDGPU_RING_TYPE_UVD_ENC:
>>>> + case AMDGPU_RING_TYPE_VCN_DEC:
>>>> + case AMDGPU_RING_TYPE_VCN_ENC:
>>>> + case AMDGPU_RING_TYPE_VCN_JPEG:
>>>> + return PP_SMC_POWER_PROFILE_VIDEO;
>>>> + default:
>>>> + return PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT;
>>>> + }
>>>> +}
>>>> +
>>>> +static int
>>>> +amdgpu_power_profile_set(struct amdgpu_device *adev,
>>>> + enum PP_SMC_POWER_PROFILE profile)
>>>> +{
>>>> + int ret = amdgpu_dpm_switch_power_profile(adev, profile, true);
>>>> +
>>>> + if (!ret) {
>>>> + /* Set the bit for the submitted workload profile */
>>>> + adev->smu_workload.submit_workload_status |= (1 << profile);
>>>> + atomic_inc(&adev->smu_workload.power_profile_ref[profile]);
>>>> + }
>>>> +
>>>> + return ret;
>>>> +}
>>>> +
>>>> +void amdgpu_workload_profile_set(struct amdgpu_device *adev,
>>>> + uint32_t ring_type)
>>>> +{
>>>> + struct amdgpu_smu_workload *workload = &adev->smu_workload;
>>>> + enum PP_SMC_POWER_PROFILE profile = ring_to_power_profile(ring_type);
>>>> + int ret;
>>>> +
>>>> + if (profile == PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT)
>>>> + return;
>>> Why is this one skipped? How do we get back to the boot up profile?
>> Hi Alex,
>>
>> enum PP_SMC_POWER_PROFILE {
>> PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT = 0x0,
>> PP_SMC_POWER_PROFILE_FULLSCREEN3D = 0x1,
>> PP_SMC_POWER_PROFILE_POWERSAVING = 0x2,
>> PP_SMC_POWER_PROFILE_VIDEO = 0x3,
>> PP_SMC_POWER_PROFILE_VR = 0x4,
>> PP_SMC_POWER_PROFILE_COMPUTE = 0x5,
>> PP_SMC_POWER_PROFILE_CUSTOM = 0x6,
>> PP_SMC_POWER_PROFILE_WINDOW3D = 0x7,
>> PP_SMC_POWER_PROFILE_COUNT,
>> };
>>
>> These are all the profiles. We are using which is >
>> PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT.
>> Now suppose the profile was DEFAULT and we set it to VIDEO, SMU will
>> move the profile to a higher level.
>> When we reset the VIDEO profile then SMU will move back to the DEFAULT one.
>>
>> Our job is to set the profile and reset it after the job is done.
>> SMU will take care to move to a higher profile and after reset, it will
>> move back to DEFAULT.
> I guess that is the part I'm missing. How does the call to the SMU to
> set the profile back to DEFAULT actually happen? It seems that both
> the put and get functions return early in this case.
SMU is calculating a workload for given the profile and setting it when
we call the *get and *put function.
When we call *set function for VIDEO then SMU will calculate a workload
for VIDEO and set it. Now We call
*put function for the same profile then SMU calculates a workload which
will be lower or DEFAULT (0)
and then it will set it.

Suppose we have called *set function for VIDEO profile then SMU will
calculate the workload = 4 and set it.
Now we have called *put function for the same profile then SMU will
calculate the workload = 0 and set it.

please see the below smu code where index will be DEFAULT (0) or lower
for *put function.

if (!en) { // put function
        smu->workload_mask &= ~(1 << smu->workload_prority[type]);
        index = fls(smu->workload_mask);
        index = index > 0 && index <= WORKLOAD_POLICY_MAX ? index - 1 : 0;
        workload = smu->workload_setting[index];
} else { // set function.
        smu->workload_mask |= (1 << smu->workload_prority[type]);
        index = fls(smu->workload_mask);
        index = index <= WORKLOAD_POLICY_MAX ? index - 1 : 0;
        workload = smu->workload_setting[index];
}

In our case the *set function will set the GPU  power profile and the
*put function will schedule
the smu_delayed_work task after 100ms delay. This smu_delayed_work task
will clear a GPU power profile if any
new jobs are not scheduled within 100 ms. But if any new job comes
within 100ms then the *set function
will cancel this work and set the GPU power profile based on preferences.

Thank You
~Arvind

>
> Alex
>
>
> Alex
>
>
>> ThankYou,
>> ~Arvind
>>
>>> Alex
>>>
>>>> +
>>>> + mutex_lock(&workload->workload_lock);
>>>> +
>>>> + ret = amdgpu_power_profile_set(adev, profile);
>>>> + if (ret) {
>>>> + DRM_WARN("Failed to set workload profile to %s, error = %d\n",
>>>> + amdgpu_workload_mode_name[profile], ret);
>>>> + }
>>>> +
>>>> + mutex_unlock(&workload->workload_lock);
>>>> +}
>>>> +
>>>> void amdgpu_workload_profile_init(struct amdgpu_device *adev)
>>>> {
>>>> adev->smu_workload.adev = adev;
>>>> diff --git a/drivers/gpu/drm/amd/include/amdgpu_workload.h b/drivers/gpu/drm/amd/include/amdgpu_workload.h
>>>> index 5d0f068422d4..5022f28fc2f9 100644
>>>> --- a/drivers/gpu/drm/amd/include/amdgpu_workload.h
>>>> +++ b/drivers/gpu/drm/amd/include/amdgpu_workload.h
>>>> @@ -46,6 +46,9 @@ static const char * const amdgpu_workload_mode_name[] = {
>>>> "Window3D"
>>>> };
>>>>
>>>> +void amdgpu_workload_profile_set(struct amdgpu_device *adev,
>>>> + uint32_t ring_type);
>>>> +
>>>> void amdgpu_workload_profile_init(struct amdgpu_device *adev);
>>>>
>>>> void amdgpu_workload_profile_fini(struct amdgpu_device *adev);
>>>> --
>>>> 2.34.1
>>>>

\
 
 \ /
  Last update: 2023-08-22 08:14    [W:0.092 / U:0.124 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site