Messages in this thread Patch in this message | | | Date | Tue, 22 Aug 2023 11:43:18 +0530 | Subject | Re: [PATCH v2 2/7] drm/amdgpu: Add new function to set GPU power profile | From | "Yadav, Arvind" <> |
| |
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 >>>>
| |