linux-kernel - Re: [PATCH v2 2/7] drm/amdgpu: Add new function to set GPU power profile

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <fff2391b-fc18-af9f-6fe0-7b4c6650dcdd@amd.com>
Date:   Mon, 21 Aug 2023 23:23:43 +0530
From:   "Yadav, Arvind" <arvyadav@....com>
To:     Alex Deucher <alexdeucher@...il.com>,
        Arvind Yadav <Arvind.Yadav@....com>
Cc:     Christian.Koenig@....com, alexander.deucher@....com,
        shashank.sharma@....com, Xinhui.Pan@....com, airlied@...il.com,
        daniel@...ll.ch, Felix.Kuehling@....com,
        amd-gfx@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
        dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH v2 2/7] drm/amdgpu: Add new function to set GPU power
 profile


On 8/21/2023 9:52 PM, Alex Deucher wrote:
> On Mon, Aug 21, 2023 at 2:55 AM Arvind Yadav <Arvind.Yadav@....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@....com>
>> Cc: Christian Koenig <christian.koenig@....com>
>> Cc: Alex Deucher <alexander.deucher@....com>
>> Signed-off-by: Arvind Yadav <Arvind.Yadav@....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.

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