| 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
| ||
|
Message-ID: <cccd1cd1-28ec-5fb0-e3a3-07caa4bf1b07@amd.com>
Date: Tue, 22 Aug 2023 11:43:18 +0530
From: "Yadav, Arvind" <arvyadav@....com>
To: Alex Deucher <alexdeucher@...il.com>
Cc: Arvind Yadav <Arvind.Yadav@....com>, 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 11:40 PM, Alex Deucher wrote:
> On Mon, Aug 21, 2023 at 1:54 PM Yadav, Arvind <arvyadav@....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@....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.
> 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
>>>>
Powered by blists - more mailing lists