From: "Quan, Evan" <Evan.Quan@amd.com>
To: "Sharma, Shashank" <Shashank.Sharma@amd.com>,
"amd-gfx@lists.freedesktop.org" <amd-gfx@lists.freedesktop.org>
Cc: "Deucher, Alexander" <Alexander.Deucher@amd.com>,
"Somalapuram, Amaranath" <Amaranath.Somalapuram@amd.com>,
"Koenig, Christian" <Christian.Koenig@amd.com>
Subject: RE: [PATCH v3 2/5] drm/amdgpu: add new functions to set GPU power profile
Date: Tue, 27 Sep 2022 09:29:46 +0000 [thread overview]
Message-ID: <DM6PR12MB26194BBD083A1FCF60FAB9E7E4559@DM6PR12MB2619.namprd12.prod.outlook.com> (raw)
In-Reply-To: <80efbb83-c6a5-7585-1c16-21832ded522e@amd.com>
[AMD Official Use Only - General]
> -----Original Message-----
> From: Sharma, Shashank <Shashank.Sharma@amd.com>
> Sent: Tuesday, September 27, 2022 3:30 PM
> To: Quan, Evan <Evan.Quan@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Somalapuram,
> Amaranath <Amaranath.Somalapuram@amd.com>; Koenig, Christian
> <Christian.Koenig@amd.com>
> Subject: Re: [PATCH v3 2/5] drm/amdgpu: add new functions to set GPU
> power profile
>
> Hello Evan,
>
> On 9/27/2022 4:14 AM, Quan, Evan wrote:
> > [AMD Official Use Only - General]
> >
> >
> >
> >> -----Original Message-----
> >> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
> >> Shashank Sharma
> >> Sent: Tuesday, September 27, 2022 5:40 AM
> >> To: amd-gfx@lists.freedesktop.org
> >> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Somalapuram,
> >> Amaranath <Amaranath.Somalapuram@amd.com>; Koenig, Christian
> >> <Christian.Koenig@amd.com>; Sharma, Shashank
> >> <Shashank.Sharma@amd.com>
> >> Subject: [PATCH v3 2/5] drm/amdgpu: add new functions to set GPU
> >> power profile
> >>
> >> This patch adds new functions which will allow a user to change the
> >> GPU power profile based a GPU workload hint flag.
> >>
> >> Cc: Alex Deucher <alexander.deucher@amd.com>
> >> Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
> >> ---
> >> drivers/gpu/drm/amd/amdgpu/Makefile | 2 +-
> >> .../gpu/drm/amd/amdgpu/amdgpu_ctx_workload.c | 97
> >> +++++++++++++++++++
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 +
> >> .../gpu/drm/amd/include/amdgpu_ctx_workload.h | 54 +++++++++++
> >> drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h | 5 +
> >> 5 files changed, 158 insertions(+), 1 deletion(-)
> >> create mode 100644
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx_workload.c
> >> create mode 100644
> >> drivers/gpu/drm/amd/include/amdgpu_ctx_workload.h
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile
> >> b/drivers/gpu/drm/amd/amdgpu/Makefile
> >> index 5a283d12f8e1..34679c657ecc 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
> >> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
> >> @@ -50,7 +50,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \
> >> atombios_dp.o amdgpu_afmt.o amdgpu_trace_points.o \
> >> atombios_encoders.o amdgpu_sa.o atombios_i2c.o \
> >> amdgpu_dma_buf.o amdgpu_vm.o amdgpu_vm_pt.o amdgpu_ib.o
> >> amdgpu_pll.o \
> >> - amdgpu_ucode.o amdgpu_bo_list.o amdgpu_ctx.o amdgpu_sync.o \
> >> + amdgpu_ucode.o amdgpu_bo_list.o amdgpu_ctx.o
> >> amdgpu_ctx_workload.o amdgpu_sync.o \
> >> amdgpu_gtt_mgr.o amdgpu_preempt_mgr.o amdgpu_vram_mgr.o
> >> amdgpu_virt.o \
> >> amdgpu_atomfirmware.o amdgpu_vf_error.o amdgpu_sched.o \
> >> amdgpu_debugfs.o amdgpu_ids.o amdgpu_gmc.o \ diff --git
> >> a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx_workload.c
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx_workload.c
> >> new file mode 100644
> >> index 000000000000..a11cf29bc388
> >> --- /dev/null
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx_workload.c
> >> @@ -0,0 +1,97 @@
> >> +/*
> >> + * Copyright 2022 Advanced Micro Devices, Inc.
> >> + *
> >> + * Permission is hereby granted, free of charge, to any person
> >> +obtaining a
> >> + * copy of this software and associated documentation files (the
> >> "Software"),
> >> + * to deal in the Software without restriction, including without
> >> + limitation
> >> + * the rights to use, copy, modify, merge, publish, distribute,
> >> + sublicense,
> >> + * and/or sell copies of the Software, and to permit persons to whom
> >> + the
> >> + * Software is furnished to do so, subject to the following conditions:
> >> + *
> >> + * The above copyright notice and this permission notice shall be
> >> + included in
> >> + * all copies or substantial portions of the Software.
> >> + *
> >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY
> KIND,
> >> EXPRESS OR
> >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> >> MERCHANTABILITY,
> >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
> NO
> >> EVENT SHALL
> >> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM,
> >> DAMAGES OR
> >> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> >> OTHERWISE,
> >> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR
> >> THE USE OR
> >> + * OTHER DEALINGS IN THE SOFTWARE.
> >> + *
> >> + */
> >> +#include <drm/drm.h>
> >> +#include "kgd_pp_interface.h"
> >> +#include "amdgpu_ctx_workload.h"
> >> +
> >> +static enum PP_SMC_POWER_PROFILE
> >> +amdgpu_workload_to_power_profile(uint32_t hint) {
> >> + switch (hint) {
> >> + case AMDGPU_CTX_WORKLOAD_HINT_NONE:
> >> + default:
> >> + return PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT;
> >> +
> >> + case AMDGPU_CTX_WORKLOAD_HINT_3D:
> >> + return PP_SMC_POWER_PROFILE_FULLSCREEN3D;
> >> + case AMDGPU_CTX_WORKLOAD_HINT_VIDEO:
> >> + return PP_SMC_POWER_PROFILE_VIDEO;
> >> + case AMDGPU_CTX_WORKLOAD_HINT_VR:
> >> + return PP_SMC_POWER_PROFILE_VR;
> >> + case AMDGPU_CTX_WORKLOAD_HINT_COMPUTE:
> >> + return PP_SMC_POWER_PROFILE_COMPUTE;
> >> + }
> >> +}
> >> +
> >> +int amdgpu_set_workload_profile(struct amdgpu_device *adev,
> >> + uint32_t hint)
> >> +{
> >> + int ret = 0;
> >> + enum PP_SMC_POWER_PROFILE profile =
> >> + amdgpu_workload_to_power_profile(hint);
> >> +
> >> + if (adev->pm.workload_mode == hint)
> >> + return 0;
> >> +
> >> + mutex_lock(&adev->pm.smu_workload_lock);
> >> +
> >> + if (adev->pm.workload_mode == hint)
> >> + goto unlock;
> > [Quan, Evan] This seems redundant with code above. I saw you dropped
> this in Patch4.
> > But I kind of feel this should be the one which needs to be kept.
>
> Yes, this shuffle happened during the rebase-testing of V3, will update this.
>
> >> +
> >> + ret = amdgpu_dpm_switch_power_profile(adev, profile, 1);
> >> + if (!ret)
> >> + adev->pm.workload_mode = hint;
> >> + atomic_inc(&adev->pm.workload_switch_ref);
> >> +
> >> +unlock:
> >> + mutex_unlock(&adev->pm.smu_workload_lock);
> >> + return ret;
> >> +}
> >> +
> >> +int amdgpu_clear_workload_profile(struct amdgpu_device *adev,
> >> + uint32_t hint)
> >> +{
> >> + int ret = 0;
> >> + enum PP_SMC_POWER_PROFILE profile =
> >> + amdgpu_workload_to_power_profile(hint);
> >> +
> >> + if (hint == AMDGPU_CTX_WORKLOAD_HINT_NONE)
> >> + return 0;
> >> +
> >> + /* Do not reset GPU power profile if another reset is coming */
> >> + if (atomic_dec_return(&adev->pm.workload_switch_ref) > 0)
> >> + return 0;
> >> +
> >> + mutex_lock(&adev->pm.smu_workload_lock);
> >> +
> >> + if (adev->pm.workload_mode != hint)
> >> + goto unlock;
> >> +
> >> + ret = amdgpu_dpm_switch_power_profile(adev, profile, 0);
> >> + if (!ret)
> >> + adev->pm.workload_mode =
> >> AMDGPU_CTX_WORKLOAD_HINT_NONE;
> >> +
> >> +unlock:
> >> + mutex_unlock(&adev->pm.smu_workload_lock);
> >> + return ret;
> >> +}
> > [Quan, Evan] Instead of setting to
> AMDGPU_CTX_WORKLOAD_HINT_NONE, better to reset it back to original
> workload profile mode.
> > That can make it compatible with existing sysfs interface which has similar
> functionality for setting workload profile mode.
>
> This API is specifically written to remove any workload profile applied, hense
> named as "clear_workload_profile" and the intention is reset. As you can see
> in the next patch, the work profile is being set from the job_run and reset
> again once the job execution is done.
>
> If there is another set() in progress, the reference counter takes care of that.
> So I would like to keep it this way.
[Quan, Evan] What I meant is some case like below:
1. User sets a workload profile mode via sysfs interface (e.g. setting compute mode via "echo 5 > /sys/class/drm/card0/device/pp_power_profile_mode")
2. Then a job was launched with a different workload profile mode requested(e.g. 3D_FULL_SCREEN mode).
3. Finally on the job ended, better to switch back to original compute mode, not just reset it back to NONE. Does that make sense?
BR
Evan
>
> - Shashank
>
> > /**
> > * DOC: pp_power_profile_mode
> > *
> > * The amdgpu driver provides a sysfs API for adjusting the heuristics
> > * related to switching between power levels in a power state. The file
> > * pp_power_profile_mode is used for this.
> > *
> > * Reading this file outputs a list of all of the predefined power profiles
> > * and the relevant heuristics settings for that profile.
> > *
> > * To select a profile or create a custom profile, first select manual using
> > * power_dpm_force_performance_level. Writing the number of a
> predefined
> > * profile to pp_power_profile_mode will enable those heuristics. To
> > * create a custom set of heuristics, write a string of numbers to the file
> > * starting with the number of the custom profile along with a setting
> > * for each heuristic parameter. Due to differences across asic families
> > * the heuristic parameters vary from family to family.
> > *
> > */
> >
> > BR
> > Evan
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> index be7aff2d4a57..1f0f64662c04 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> @@ -3554,6 +3554,7 @@ int amdgpu_device_init(struct amdgpu_device
> >> *adev,
> >> mutex_init(&adev->psp.mutex);
> >> mutex_init(&adev->notifier_lock);
> >> mutex_init(&adev->pm.stable_pstate_ctx_lock);
> >> + mutex_init(&adev->pm.smu_workload_lock);
> >> mutex_init(&adev->benchmark_mutex);
> >>
> >> amdgpu_device_init_apu_flags(adev);
> >> diff --git a/drivers/gpu/drm/amd/include/amdgpu_ctx_workload.h
> >> b/drivers/gpu/drm/amd/include/amdgpu_ctx_workload.h
> >> new file mode 100644
> >> index 000000000000..6060fc53c3b0
> >> --- /dev/null
> >> +++ b/drivers/gpu/drm/amd/include/amdgpu_ctx_workload.h
> >> @@ -0,0 +1,54 @@
> >> +/*
> >> + * Copyright 2022 Advanced Micro Devices, Inc.
> >> + *
> >> + * Permission is hereby granted, free of charge, to any person
> >> +obtaining a
> >> + * copy of this software and associated documentation files (the
> >> "Software"),
> >> + * to deal in the Software without restriction, including without
> >> + limitation
> >> + * the rights to use, copy, modify, merge, publish, distribute,
> >> + sublicense,
> >> + * and/or sell copies of the Software, and to permit persons to whom
> >> + the
> >> + * Software is furnished to do so, subject to the following conditions:
> >> + *
> >> + * The above copyright notice and this permission notice shall be
> >> + included in
> >> + * all copies or substantial portions of the Software.
> >> + *
> >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY
> KIND,
> >> EXPRESS OR
> >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> >> MERCHANTABILITY,
> >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
> NO
> >> EVENT SHALL
> >> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM,
> >> DAMAGES OR
> >> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> >> OTHERWISE,
> >> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR
> >> THE USE OR
> >> + * OTHER DEALINGS IN THE SOFTWARE.
> >> + *
> >> + */
> >> +#ifndef _AMDGPU_CTX_WL_H_
> >> +#define _AMDGPU_CTX_WL_H_
> >> +#include <drm/amdgpu_drm.h>
> >> +#include "amdgpu.h"
> >> +
> >> +/* Workload mode names */
> >> +static const char * const amdgpu_workload_mode_name[] = {
> >> + "None",
> >> + "3D",
> >> + "Video",
> >> + "VR",
> >> + "Compute",
> >> + "Unknown",
> >> +};
> >> +
> >> +static inline const
> >> +char *amdgpu_workload_profile_name(uint32_t profile) {
> >> + if (profile >= AMDGPU_CTX_WORKLOAD_HINT_NONE &&
> >> + profile < AMDGPU_CTX_WORKLOAD_HINT_MAX)
> >> + return
> >>
> amdgpu_workload_mode_name[AMDGPU_CTX_WORKLOAD_INDEX(profile
> >> )];
> >> +
> >> + return
> >>
> amdgpu_workload_mode_name[AMDGPU_CTX_WORKLOAD_HINT_MAX];
> >> +}
> >> +
> >> +int amdgpu_clear_workload_profile(struct amdgpu_device *adev,
> >> + uint32_t hint);
> >> +
> >> +int amdgpu_set_workload_profile(struct amdgpu_device *adev,
> >> + uint32_t hint);
> >> +
> >> +#endif
> >> diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> >> b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> >> index 65624d091ed2..565131f789d0 100644
> >> --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> >> +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> >> @@ -361,6 +361,11 @@ struct amdgpu_pm {
> >> struct mutex stable_pstate_ctx_lock;
> >> struct amdgpu_ctx *stable_pstate_ctx;
> >>
> >> + /* SMU workload mode */
> >> + struct mutex smu_workload_lock;
> >> + uint32_t workload_mode;
> >> + atomic_t workload_switch_ref;
> >> +
> >> struct config_table_setting config_table;
> >> /* runtime mode */
> >> enum amdgpu_runpm_mode rpm_mode;
> >> --
> >> 2.34.1
next prev parent reply other threads:[~2022-09-27 9:29 UTC|newest]
Thread overview: 76+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-26 21:40 [PATCH v3 0/5] GPU workload hints for better performance Shashank Sharma
2022-09-26 21:40 ` [PATCH v3 1/5] drm/amdgpu: add UAPI for workload hints to ctx ioctl Shashank Sharma
2022-09-27 6:07 ` Christian König
2022-09-27 14:28 ` Felix Kuehling
2023-03-21 3:05 ` Marek Olšák
2023-03-21 13:00 ` Sharma, Shashank
2023-03-21 13:54 ` Christian König
2023-03-22 14:05 ` Marek Olšák
2023-03-22 14:08 ` Christian König
2023-03-22 14:24 ` Marek Olšák
2023-03-22 14:29 ` Christian König
2023-03-22 14:36 ` Marek Olšák
2023-03-22 14:52 ` Alex Deucher
2023-03-22 15:11 ` Marek Olšák
2023-03-22 14:38 ` Sharma, Shashank
2022-09-26 21:40 ` [PATCH v3 2/5] drm/amdgpu: add new functions to set GPU power profile Shashank Sharma
2022-09-27 2:14 ` Quan, Evan
2022-09-27 7:29 ` Sharma, Shashank
2022-09-27 9:29 ` Quan, Evan [this message]
2022-09-27 10:00 ` Sharma, Shashank
2022-09-27 6:08 ` Christian König
2022-09-27 9:58 ` Lazar, Lijo
2022-09-27 11:41 ` Sharma, Shashank
2022-09-27 12:10 ` Lazar, Lijo
2022-09-27 12:23 ` Sharma, Shashank
2022-09-27 12:39 ` Lazar, Lijo
2022-09-27 12:53 ` Sharma, Shashank
2022-09-27 13:29 ` Lazar, Lijo
2022-09-27 13:47 ` Sharma, Shashank
2022-09-27 14:00 ` Lazar, Lijo
2022-09-27 14:20 ` Sharma, Shashank
2022-09-27 14:34 ` Lazar, Lijo
2022-09-27 14:50 ` Sharma, Shashank
2022-09-27 15:20 ` Felix Kuehling
2022-09-26 21:40 ` [PATCH v3 3/5] drm/amdgpu: set GPU workload via ctx IOCTL Shashank Sharma
2022-09-27 6:09 ` Christian König
2022-09-26 21:40 ` [PATCH v3 4/5] drm/amdgpu: switch GPU workload profile Shashank Sharma
2022-09-27 6:11 ` Christian König
2022-09-27 10:03 ` Lazar, Lijo
2022-09-27 11:47 ` Sharma, Shashank
2022-09-27 12:20 ` Lazar, Lijo
2022-09-27 12:25 ` Sharma, Shashank
2022-09-27 16:33 ` Michel Dänzer
2022-09-27 17:06 ` Sharma, Shashank
2022-09-27 17:29 ` Michel Dänzer
2022-09-26 21:40 ` [PATCH v3 5/5] drm/amdgpu: switch workload context to/from compute Shashank Sharma
2022-09-27 6:12 ` Christian König
2022-09-27 14:48 ` Felix Kuehling
2022-09-27 14:58 ` Sharma, Shashank
2022-09-27 15:23 ` Felix Kuehling
2022-09-27 15:38 ` Sharma, Shashank
2022-09-27 20:40 ` Alex Deucher
2022-09-28 7:05 ` Lazar, Lijo
2022-09-28 8:56 ` Sharma, Shashank
2022-09-28 9:00 ` Sharma, Shashank
2022-09-28 21:51 ` Alex Deucher
2022-09-29 8:48 ` Sharma, Shashank
2022-09-29 11:10 ` Lazar, Lijo
2022-09-29 13:20 ` Sharma, Shashank
2022-09-29 13:37 ` Lazar, Lijo
2022-09-29 14:00 ` Sharma, Shashank
2022-09-29 14:14 ` Lazar, Lijo
2022-09-29 14:40 ` Sharma, Shashank
2022-09-29 18:32 ` Alex Deucher
2022-09-30 5:08 ` Lazar, Lijo
2022-09-30 8:37 ` Sharma, Shashank
2022-09-30 9:13 ` Lazar, Lijo
2022-09-30 9:22 ` Sharma, Shashank
2022-09-30 9:54 ` Lazar, Lijo
2022-09-30 10:09 ` Sharma, Shashank
2022-09-29 18:07 ` Felix Kuehling
2022-09-30 4:46 ` Lazar, Lijo
2022-09-27 16:24 ` [PATCH v3 0/5] GPU workload hints for better performance Michel Dänzer
2022-09-27 16:59 ` Sharma, Shashank
2022-09-27 17:13 ` Michel Dänzer
2022-09-27 17:25 ` Sharma, Shashank
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DM6PR12MB26194BBD083A1FCF60FAB9E7E4559@DM6PR12MB2619.namprd12.prod.outlook.com \
--to=evan.quan@amd.com \
--cc=Alexander.Deucher@amd.com \
--cc=Amaranath.Somalapuram@amd.com \
--cc=Christian.Koenig@amd.com \
--cc=Shashank.Sharma@amd.com \
--cc=amd-gfx@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).