<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=us-ascii"> </head> <body> [Public] <div> <div style="color: rgb(33, 33, 33); background-color: rgb(255, 255, 255);" dir="auto"> I feel it's better to leave to platform vendors. For ex: for APU cases they may have implementations in which their BIOSes talk to PMFW and this might be driving something else here.</div> <div style="color: rgb(33, 33, 33); background-color: rgb(255, 255, 255);" dir="auto"> </div> <div style="color: rgb(33, 33, 33); background-color: rgb(255, 255, 255);" dir="auto"> Also, not sure how to handle a case like, say a laptop with Intel CPU and AMD dgpu.</div> <div id="ms-outlook-mobile-signature"> <div> </div> Thanks, Lijo</div> <div id="id-f61421a9-d54c-4f81-aa61-517d64763a14" class="ms-outlook-mobile-reference-message"> <div style="font-family: sans-serif; font-size: 13.2pt; color: rgb(0, 0, 0);"> </div> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg">From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Alex Deucher <alexdeucher@gmail.com> Sent: Wednesday, 10 November 2021, 8:44 pm To: Limonciello, Mario Cc: amd-gfx list Subject: Re: [RFC 2/2] drm/amd/pm: Add support for reacting to platform profile notification </div> <meta name="Generator" content="Microsoft Exchange Server"> <style></style> <div class="PlainText">On Wed, Nov 10, 2021 at 1:24 AM Mario Limonciello <mario.limonciello@amd.com> wrote: > > Various drivers provide platform profile support to let users set a hint > in their GUI whether they want to run in a high performance, low battery > life or balanced configuration. > > Drivers that provide this typically work with the firmware on their system > to configure hardware. In the case of AMDGPU however, the notification > path doesn't come through firmware and can instead be provided directly > to the driver from a notification chain. > > Use the information of the newly selected profile to tweak > `dpm_force_performance_level` to that profile IFF the user hasn't manually > selected `manual` or any other `profile_*` options. I don't think we want to force the performance level. This interface forces various fixed clock configurations for debugging and profiling. I think what we'd want to select here is the power profile (see amdgpu_set_pp_power_profile_mode()). For this interface you can select various profiles (BOOTUP_DEFAULT, 3D_FULL_SCREEN, POWER_SAVING, VIDEO, VR, COMPUTE, etc.). These still use dynamic power management, but they adjust the heuristics used by the GPU to select power states so the GPU performance ramps up/down more or less aggressively. Alex > > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 3 + > drivers/gpu/drm/amd/pm/amdgpu_pm.c | 105 +++++++++++++++++++++++----- > 2 files changed, 90 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > index b85b67a88a3d..27b0be23b6ac 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > @@ -1097,6 +1097,9 @@ struct amdgpu_device { > > struct amdgpu_reset_control *reset_cntl; > uint32_t ip_versions[HW_ID_MAX][HWIP_MAX_INSTANCE]; > + > + /* platform profile notifications */ > + struct notifier_block platform_profile_notifier; > }; > > static inline struct amdgpu_device *drm_to_adev(struct drm_device *ddev) > diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c > index 41472ed99253..33fc52b90d4c 100644 > --- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c > +++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c > @@ -32,6 +32,7 @@ > #include <linux/hwmon.h> > #include <linux/hwmon-sysfs.h> > #include <linux/nospec.h> > +#include <linux/platform_profile.h> > #include <linux/pm_runtime.h> > #include <asm/processor.h> > #include "hwmgr.h" > @@ -200,6 +201,33 @@ static ssize_t amdgpu_set_power_dpm_state(struct device *dev, > return count; > } > > +static int amdgpu_get_forced_level(struct device *dev, enum amd_dpm_forced_level *level) > +{ > + struct drm_device *ddev = dev_get_drvdata(dev); > + struct amdgpu_device *adev = drm_to_adev(ddev); > + int ret; > + > + if (amdgpu_in_reset(adev)) > + return -EPERM; > + if (adev->in_suspend && !adev->in_runpm) > + return -EPERM; > + > + ret = pm_runtime_get_sync(ddev->dev); > + if (ret < 0) { > + pm_runtime_put_autosuspend(ddev->dev); > + return ret; > + } > + > + if (adev->powerplay.pp_funcs->get_performance_level) > + *level = amdgpu_dpm_get_performance_level(adev); > + else > + *level = adev->pm.dpm.forced_level; > + > + pm_runtime_mark_last_busy(ddev->dev); > + pm_runtime_put_autosuspend(ddev->dev); > + > + return 0; > +} > > /** > * DOC: power_dpm_force_performance_level > @@ -264,29 +292,13 @@ static ssize_t amdgpu_get_power_dpm_force_performance_level(struct device *dev, > struct device_attribute *attr, > char *buf) > { > - struct drm_device *ddev = dev_get_drvdata(dev); > - struct amdgpu_device *adev = drm_to_adev(ddev); > enum amd_dpm_forced_level level = 0xff; > int ret; > > - if (amdgpu_in_reset(adev)) > - return -EPERM; > - if (adev->in_suspend && !adev->in_runpm) > - return -EPERM; > + ret = amdgpu_get_forced_level(dev, &level); > > - ret = pm_runtime_get_sync(ddev->dev); > - if (ret < 0) { > - pm_runtime_put_autosuspend(ddev->dev); > + if (ret < 0) > return ret; > - } > - > - if (adev->powerplay.pp_funcs->get_performance_level) > - level = amdgpu_dpm_get_performance_level(adev); > - else > - level = adev->pm.dpm.forced_level; > - > - pm_runtime_mark_last_busy(ddev->dev); > - pm_runtime_put_autosuspend(ddev->dev); > > return sysfs_emit(buf, "%s\n", > (level == AMD_DPM_FORCED_LEVEL_AUTO) ? "auto" : > @@ -405,6 +417,59 @@ static ssize_t amdgpu_set_power_dpm_force_performance_level(struct device *dev, > return count; > } > > +static void amdgpu_update_profile(struct device *dev, enum platform_profile_option *profile) > +{ > + enum amd_dpm_forced_level level; > + const char *str; > + int ret; > + > + ret = amdgpu_get_forced_level(dev, &level); > + if (ret < 0) > + return; > + > + /* only update profile if we're in fixed modes right now that need updating */ > + switch (level) { > + case AMD_DPM_FORCED_LEVEL_LOW: > + if (*profile < PLATFORM_PROFILE_BALANCED) > + return; > + break; > + case AMD_DPM_FORCED_LEVEL_HIGH: > + if (*profile > PLATFORM_PROFILE_BALANCED) > + return; > + break; > + case AMD_DPM_FORCED_LEVEL_AUTO: > + if (*profile == PLATFORM_PROFILE_BALANCED) > + return; > + break; > + default: > + dev_dbg(dev, "refusing to update amdgpu profile from %d\n", level); > + return; > + } > + if (*profile > PLATFORM_PROFILE_BALANCED) > + str = "high"; > + else if (*profile < PLATFORM_PROFILE_BALANCED) > + str = "low"; > + else > + str = "auto"; > + > + dev_dbg(dev, "updating platform profile to %s\n", str); > + amdgpu_set_power_dpm_force_performance_level(dev, NULL, str, 0); > +} > + > +static int amdgpu_platform_profile_notifier_call(struct notifier_block *nb, > + unsigned long action, void *data) > +{ > + if (action == PLATFORM_PROFILE_CHANGED) { > + enum platform_profile_option *profile = data; > + struct amdgpu_device *adev; > + > + adev = container_of(nb, struct amdgpu_device, platform_profile_notifier); > + amdgpu_update_profile(adev->dev, profile); > + } > + > + return NOTIFY_OK; > +} > + > static ssize_t amdgpu_get_pp_num_states(struct device *dev, > struct device_attribute *attr, > char *buf) > @@ -3506,6 +3571,9 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev) > if (ret) > return ret; > > + adev->platform_profile_notifier.notifier_call = amdgpu_platform_profile_notifier_call; > + platform_profile_register_notifier(&adev->platform_profile_notifier); > + > adev->pm.sysfs_initialized = true; > > return 0; > @@ -3519,6 +3587,7 @@ void amdgpu_pm_sysfs_fini(struct amdgpu_device *adev) > if (adev->pm.int_hwmon_dev) > hwmon_device_unregister(adev->pm.int_hwmon_dev); > > + platform_profile_unregister_notifier(&adev->platform_profile_notifier); > amdgpu_device_attr_remove_groups(adev, &adev->pm.pm_attr_list); > } > > -- > 2.25.1 > </div> </div> </div> </body> </html>