Re: [PATCH v2 2/7] drm/amdgpu: Add new function to set GPU power profile
From: Yadav, Arvind
Date: Tue Aug 22 2023 - 02:13:39 EST
On 8/21/2023 11:40 PM, Alex Deucher wrote:
On Mon, Aug 21, 2023 at 1:54 PM Yadav, Arvind <arvyadav@xxxxxxx> wrote:
On 8/21/2023 9:52 PM, Alex Deucher wrote:
On Mon, Aug 21, 2023 at 2:55 AM Arvind Yadav <Arvind.Yadav@xxxxxxx> 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@xxxxxxx>
Cc: Christian Koenig <christian.koenig@xxxxxxx>
Cc: Alex Deucher <alexander.deucher@xxxxxxx>
Signed-off-by: Arvind Yadav <Arvind.Yadav@xxxxxxx>
---
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