* [RFC 0/2] Let amdgpu react to platform profile changes @ 2021-11-10 6:23 Mario Limonciello 2021-11-10 6:23 ` [RFC 1/2] ACPI: platform_profile: Add support for notification chains Mario Limonciello 2021-11-10 6:23 ` [RFC 2/2] drm/amd/pm: Add support for reacting to platform profile notification Mario Limonciello 0 siblings, 2 replies; 9+ messages in thread From: Mario Limonciello @ 2021-11-10 6:23 UTC (permalink / raw) To: amd-gfx; +Cc: Mario Limonciello Many OEM platform provide a platform profile knob that can be used to make firmware tunings to the system to allow operating in a higher or lower performance mode trading off power consumption. Software like power-profiles-daemon to expose this knob to the UI. As we know the user's intent to go into power saving or performance mode from this, we can also let amdgpu react to the change. This patch series is sent as RFC right now only to amd-gfx, and if it's a good enough idea will re-send as PATCH to linux-acpi + amd-gfx. Mario Limonciello (2): ACPI: platform_profile: Add support for notification chains drm/amd/pm: Add support for reacting to platform profile notification drivers/acpi/platform_profile.c | 52 ++++++++++++-- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 3 + drivers/gpu/drm/amd/pm/amdgpu_pm.c | 105 +++++++++++++++++++++++----- include/linux/platform_profile.h | 10 +++ 4 files changed, 145 insertions(+), 25 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [RFC 1/2] ACPI: platform_profile: Add support for notification chains 2021-11-10 6:23 [RFC 0/2] Let amdgpu react to platform profile changes Mario Limonciello @ 2021-11-10 6:23 ` Mario Limonciello 2021-11-10 6:23 ` [RFC 2/2] drm/amd/pm: Add support for reacting to platform profile notification Mario Limonciello 1 sibling, 0 replies; 9+ messages in thread From: Mario Limonciello @ 2021-11-10 6:23 UTC (permalink / raw) To: amd-gfx; +Cc: Mario Limonciello Allow other drivers to initialize relative to current active profile and react to platform profile changes. Drivers wishing to utilize this should register for notification at module load and unregister when unloading. Notifications will come in the from a notifier call. Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> --- drivers/acpi/platform_profile.c | 52 +++++++++++++++++++++++++++----- include/linux/platform_profile.h | 10 ++++++ 2 files changed, 55 insertions(+), 7 deletions(-) diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c index d418462ab791..ca5d962020a2 100644 --- a/drivers/acpi/platform_profile.c +++ b/drivers/acpi/platform_profile.c @@ -21,6 +21,24 @@ static const char * const profile_names[] = { [PLATFORM_PROFILE_PERFORMANCE] = "performance", }; static_assert(ARRAY_SIZE(profile_names) == PLATFORM_PROFILE_LAST); +static BLOCKING_NOTIFIER_HEAD(platform_profile_chain_head); + +int platform_profile_register_notifier(struct notifier_block *nb) +{ + return blocking_notifier_chain_register(&platform_profile_chain_head, nb); +} +EXPORT_SYMBOL_GPL(platform_profile_register_notifier); + +int platform_profile_unregister_notifier(struct notifier_block *nb) +{ + return blocking_notifier_chain_unregister(&platform_profile_chain_head, nb); +} +EXPORT_SYMBOL_GPL(platform_profile_unregister_notifier); + +static void platform_profile_call_notifier(unsigned long action, void *data) +{ + blocking_notifier_call_chain(&platform_profile_chain_head, action, data); +} static ssize_t platform_profile_choices_show(struct device *dev, struct device_attribute *attr, @@ -49,11 +67,8 @@ static ssize_t platform_profile_choices_show(struct device *dev, return len; } -static ssize_t platform_profile_show(struct device *dev, - struct device_attribute *attr, - char *buf) +int platform_profile_get(enum platform_profile_option *profile) { - enum platform_profile_option profile = PLATFORM_PROFILE_BALANCED; int err; err = mutex_lock_interruptible(&profile_lock); @@ -65,15 +80,28 @@ static ssize_t platform_profile_show(struct device *dev, return -ENODEV; } - err = cur_profile->profile_get(cur_profile, &profile); + err = cur_profile->profile_get(cur_profile, profile); mutex_unlock(&profile_lock); if (err) return err; /* Check that profile is valid index */ - if (WARN_ON((profile < 0) || (profile >= ARRAY_SIZE(profile_names)))) + if (WARN_ON((*profile < 0) || (*profile >= ARRAY_SIZE(profile_names)))) return -EIO; + return 0; +} +EXPORT_SYMBOL_GPL(platform_profile_get); + +static ssize_t platform_profile_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + enum platform_profile_option profile = PLATFORM_PROFILE_BALANCED; + int ret = platform_profile_get(&profile); + + if (ret) + return ret; return sysfs_emit(buf, "%s\n", profile_names[profile]); } @@ -106,8 +134,10 @@ static ssize_t platform_profile_store(struct device *dev, } err = cur_profile->profile_set(cur_profile, i); - if (!err) + if (!err) { sysfs_notify(acpi_kobj, NULL, "platform_profile"); + platform_profile_call_notifier(PLATFORM_PROFILE_CHANGED, &i); + } mutex_unlock(&profile_lock); if (err) @@ -130,9 +160,17 @@ static const struct attribute_group platform_profile_group = { void platform_profile_notify(void) { + enum platform_profile_option profile; + int ret; + if (!cur_profile) return; sysfs_notify(acpi_kobj, NULL, "platform_profile"); + ret = platform_profile_get(&profile); + if (ret) + return; + platform_profile_call_notifier(PLATFORM_PROFILE_CHANGED, &profile); + } EXPORT_SYMBOL_GPL(platform_profile_notify); diff --git a/include/linux/platform_profile.h b/include/linux/platform_profile.h index a6329003aee7..dca9d47e18eb 100644 --- a/include/linux/platform_profile.h +++ b/include/linux/platform_profile.h @@ -11,6 +11,8 @@ #include <linux/bitops.h> +struct notifier_block; + /* * If more options are added please update profile_names array in * platform_profile.c and sysfs-platform_profile documentation. @@ -37,5 +39,13 @@ struct platform_profile_handler { int platform_profile_register(struct platform_profile_handler *pprof); int platform_profile_remove(void); void platform_profile_notify(void); +int platform_profile_get(enum platform_profile_option *profile); + +int platform_profile_register_notifier(struct notifier_block *nb); +int platform_profile_unregister_notifier(struct notifier_block *nb); + +enum platform_profile_notifier_actions { + PLATFORM_PROFILE_CHANGED, +}; #endif /*_PLATFORM_PROFILE_H_*/ -- 2.25.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [RFC 2/2] drm/amd/pm: Add support for reacting to platform profile notification 2021-11-10 6:23 [RFC 0/2] Let amdgpu react to platform profile changes Mario Limonciello 2021-11-10 6:23 ` [RFC 1/2] ACPI: platform_profile: Add support for notification chains Mario Limonciello @ 2021-11-10 6:23 ` Mario Limonciello 2021-11-10 15:13 ` Alex Deucher 1 sibling, 1 reply; 9+ messages in thread From: Mario Limonciello @ 2021-11-10 6:23 UTC (permalink / raw) To: amd-gfx; +Cc: Mario Limonciello 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. 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [RFC 2/2] drm/amd/pm: Add support for reacting to platform profile notification 2021-11-10 6:23 ` [RFC 2/2] drm/amd/pm: Add support for reacting to platform profile notification Mario Limonciello @ 2021-11-10 15:13 ` Alex Deucher 2021-11-10 16:05 ` Lazar, Lijo 0 siblings, 1 reply; 9+ messages in thread From: Alex Deucher @ 2021-11-10 15:13 UTC (permalink / raw) To: Mario Limonciello; +Cc: amd-gfx list 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 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC 2/2] drm/amd/pm: Add support for reacting to platform profile notification 2021-11-10 15:13 ` Alex Deucher @ 2021-11-10 16:05 ` Lazar, Lijo 2021-11-10 17:31 ` Limonciello, Mario 0 siblings, 1 reply; 9+ messages in thread From: Lazar, Lijo @ 2021-11-10 16:05 UTC (permalink / raw) To: Alex Deucher, Limonciello, Mario; +Cc: amd-gfx list [-- Attachment #1: Type: text/plain, Size: 8760 bytes --] [Public] 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. Also, not sure how to handle a case like, say a laptop with Intel CPU and AMD dgpu. Thanks, Lijo ________________________________ 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 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 > [-- Attachment #2: Type: text/html, Size: 17606 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [RFC 2/2] drm/amd/pm: Add support for reacting to platform profile notification 2021-11-10 16:05 ` Lazar, Lijo @ 2021-11-10 17:31 ` Limonciello, Mario 2021-11-10 17:51 ` Alex Deucher 2021-11-11 4:53 ` Lazar, Lijo 0 siblings, 2 replies; 9+ messages in thread From: Limonciello, Mario @ 2021-11-10 17:31 UTC (permalink / raw) To: Lazar, Lijo, Alex Deucher; +Cc: amd-gfx list [-- Attachment #1: Type: text/plain, Size: 10788 bytes --] [Public] > I don't think we want to force the performance level. This interface forces various fixed clock configurations for debugging and profiling. Ah got it. >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. Which profile mapping you think make sense? My guess would be: "BOOTUP_DEFAULT" for balanced "POWER_SAVING" for low-power "3D_FULL_SCREEN" for performance Since recently we removed that interface for YC, and some earlier APUs don't do as much with it. So I wonder if this is only really valuable to do this callback for !APU. > 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. Even if changing the heuristic for workload as Alex suggested? > Also, not sure how to handle a case like, say a laptop with Intel CPU and AMD dgpu. I was actually thinking this approach where there are platform profile callbacks is best because of that specifically. It would allow an Intel CPU system to have a platform profile driver implemented by the OEM, but then still notify amdgpu dGPU to tune for power saving or performance workload. From: Lazar, Lijo <Lijo.Lazar@amd.com> Sent: Wednesday, November 10, 2021 10:05 To: Alex Deucher <alexdeucher@gmail.com>; Limonciello, Mario <Mario.Limonciello@amd.com> Cc: amd-gfx list <amd-gfx@lists.freedesktop.org> Subject: Re: [RFC 2/2] drm/amd/pm: Add support for reacting to platform profile notification [Public] 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. Also, not sure how to handle a case like, say a laptop with Intel CPU and AMD dgpu. Thanks, Lijo ________________________________ From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org<mailto:amd-gfx-bounces@lists.freedesktop.org>> on behalf of Alex Deucher <alexdeucher@gmail.com<mailto: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 On Wed, Nov 10, 2021 at 1:24 AM Mario Limonciello <mario.limonciello@amd.com<mailto: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<mailto: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 > [-- Attachment #2: Type: text/html, Size: 24509 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC 2/2] drm/amd/pm: Add support for reacting to platform profile notification 2021-11-10 17:31 ` Limonciello, Mario @ 2021-11-10 17:51 ` Alex Deucher 2021-11-11 4:53 ` Lazar, Lijo 1 sibling, 0 replies; 9+ messages in thread From: Alex Deucher @ 2021-11-10 17:51 UTC (permalink / raw) To: Limonciello, Mario; +Cc: Lazar, Lijo, amd-gfx list On Wed, Nov 10, 2021 at 12:31 PM Limonciello, Mario <Mario.Limonciello@amd.com> wrote: > > [Public] > > > > > I don't think we want to force the performance level. This interface > forces various fixed clock configurations for debugging and profiling. > > Ah got it. > > > > >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. > > > > Which profile mapping you think make sense? > > My guess would be: > > “BOOTUP_DEFAULT” for balanced > > “POWER_SAVING” for low-power > > “3D_FULL_SCREEN” for performance Yeah, that is what I was thinking. > > > > Since recently we removed that interface for YC, and some earlier APUs don’t do as much with it. > > So I wonder if this is only really valuable to do this callback for !APU. > Yes, I think so. I think for APUs this will be handled under the covers in the platform firmware going forward. > > > > 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. > > > > Even if changing the heuristic for workload as Alex suggested? > > > > > Also, not sure how to handle a case like, say a laptop with Intel CPU and AMD dgpu. > > > > I was actually thinking this approach where there are platform profile callbacks is best because of that specifically. > > It would allow an Intel CPU system to have a platform profile driver implemented by the OEM, but then still notify amdgpu dGPU to tune for power saving or performance workload. > Right, a user could still adjust these via the existing sysfs interface today, this just makes it a little more automatic. I guess it depends whether we want to couple them or not. Alex > > > From: Lazar, Lijo <Lijo.Lazar@amd.com> > Sent: Wednesday, November 10, 2021 10:05 > To: Alex Deucher <alexdeucher@gmail.com>; Limonciello, Mario <Mario.Limonciello@amd.com> > Cc: amd-gfx list <amd-gfx@lists.freedesktop.org> > Subject: Re: [RFC 2/2] drm/amd/pm: Add support for reacting to platform profile notification > > > > [Public] > > > > 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. > > > > Also, not sure how to handle a case like, say a laptop with Intel CPU and AMD dgpu. > > > > Thanks, > Lijo > > > > ________________________________ > > 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 > > > > 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 > > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC 2/2] drm/amd/pm: Add support for reacting to platform profile notification 2021-11-10 17:31 ` Limonciello, Mario 2021-11-10 17:51 ` Alex Deucher @ 2021-11-11 4:53 ` Lazar, Lijo 2021-11-11 16:17 ` Limonciello, Mario 1 sibling, 1 reply; 9+ messages in thread From: Lazar, Lijo @ 2021-11-11 4:53 UTC (permalink / raw) To: Limonciello, Mario, Alex Deucher; +Cc: amd-gfx list On 11/10/2021 11:01 PM, Limonciello, Mario wrote: > [Public] > >> I don't think we want to force the performance level. This interface > forces various fixed clock configurations for debugging and profiling. > > Ah got it. > >>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. > > Which profile mapping you think make sense? > > My guess would be: > > “BOOTUP_DEFAULT” for balanced > > “POWER_SAVING” for low-power > > “3D_FULL_SCREEN” for performance > > Since recently we removed that interface for YC, and some earlier APUs > don’t do as much with it. > > So I wonder if this is only really valuable to do this callback for !APU. > >>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. > > Even if changing the heuristic for workload as Alex suggested? > Yes. I think this is meant to be BIOS driven for APU platforms and AMD APU + AMD dGPU with smartshift. >>Also, not sure how to handle a case like, say a laptop with Intel CPU > and AMD dgpu. > > I was actually thinking this approach where there are platform profile > callbacks is best because of that specifically. > > It would allow an Intel CPU system to have a platform profile driver > implemented by the OEM, but then still notify amdgpu dGPU to tune for > power saving or performance workload. > After seeing that this is coming under ACPI, I thought the intention is to have this driven by firmware primarily. The purpose of platform driver itself could be to optimize for those power profiles and while doing that it should have considered all the components in the whole platform (assuming platform driver testing covers the behavior of these modes on a particular platform). I am not sure if it's appropriate for another driver to plug-in to this automatically and tinker an 'expected-to-be-well-tuned' setting by the platform driver. The modes selected by another driver may or may not match with the conditions assumed by platform driver - for ex: some profile could mean fans running quieter with EC control and then the profile chosen by another driver could disturb that intended setting. Thanks, Lijo > *From:* Lazar, Lijo <Lijo.Lazar@amd.com> > *Sent:* Wednesday, November 10, 2021 10:05 > *To:* Alex Deucher <alexdeucher@gmail.com>; Limonciello, Mario > <Mario.Limonciello@amd.com> > *Cc:* amd-gfx list <amd-gfx@lists.freedesktop.org> > *Subject:* Re: [RFC 2/2] drm/amd/pm: Add support for reacting to > platform profile notification > > [Public] > > 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. > > Also, not sure how to handle a case like, say a laptop with Intel CPU > and AMD dgpu. > > Thanks, > Lijo > > ------------------------------------------------------------------------ > > *From:* amd-gfx <amd-gfx-bounces@lists.freedesktop.org > <mailto:amd-gfx-bounces@lists.freedesktop.org>> on behalf of Alex > Deucher <alexdeucher@gmail.com <mailto: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 > > On Wed, Nov 10, 2021 at 1:24 AM Mario Limonciello > <mario.limonciello@amd.com <mailto: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 > <mailto: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 > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [RFC 2/2] drm/amd/pm: Add support for reacting to platform profile notification 2021-11-11 4:53 ` Lazar, Lijo @ 2021-11-11 16:17 ` Limonciello, Mario 0 siblings, 0 replies; 9+ messages in thread From: Limonciello, Mario @ 2021-11-11 16:17 UTC (permalink / raw) To: Lazar, Lijo, Alex Deucher; +Cc: amd-gfx list [AMD Official Use Only] > > > > Even if changing the heuristic for workload as Alex suggested? > > > > Yes. I think this is meant to be BIOS driven for APU platforms and AMD > APU + AMD dGPU with smartshift. > So then it sounds like if any - it would make sense only on dGPU that isn't using smart shift. > > After seeing that this is coming under ACPI, I thought the intention is > to have this driven by firmware primarily. The purpose of platform > driver itself could be to optimize for those power profiles and while > doing that it should have considered all the components in the whole > platform (assuming platform driver testing covers the behavior of these > modes on a particular platform). > > I am not sure if it's appropriate for another driver to plug-in to this > automatically and tinker an 'expected-to-be-well-tuned' setting by the > platform driver. The modes selected by another driver may or may not > match with the conditions assumed by platform driver - for ex: some > profile could mean fans running quieter with EC control and then the > profile chosen by another driver could disturb that intended setting. > The key here is that any non-APU program won't have a path to influence dGPU behavior via FW or smartshift will it? So it could be useful to respond to a limited subset of hints that can be guaranteed to mean the same. Like maybe low power mode and balanced only, but don't make changes going to high power mode? > Thanks, > Lijo > > > *From:* Lazar, Lijo <Lijo.Lazar@amd.com> > > *Sent:* Wednesday, November 10, 2021 10:05 > > *To:* Alex Deucher <alexdeucher@gmail.com>; Limonciello, Mario > > <Mario.Limonciello@amd.com> > > *Cc:* amd-gfx list <amd-gfx@lists.freedesktop.org> > > *Subject:* Re: [RFC 2/2] drm/amd/pm: Add support for reacting to > > platform profile notification > > > > [Public] > > > > 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. > > > > Also, not sure how to handle a case like, say a laptop with Intel CPU > > and AMD dgpu. > > > > Thanks, > > Lijo > > > > ------------------------------------------------------------------------ > > > > *From:* amd-gfx <amd-gfx-bounces@lists.freedesktop.org > > <mailto:amd-gfx-bounces@lists.freedesktop.org>> on behalf of Alex > > Deucher <alexdeucher@gmail.com <mailto: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 > > > > On Wed, Nov 10, 2021 at 1:24 AM Mario Limonciello > > <mario.limonciello@amd.com <mailto: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 > > <mailto: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 > > > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-11-11 16:18 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-11-10 6:23 [RFC 0/2] Let amdgpu react to platform profile changes Mario Limonciello 2021-11-10 6:23 ` [RFC 1/2] ACPI: platform_profile: Add support for notification chains Mario Limonciello 2021-11-10 6:23 ` [RFC 2/2] drm/amd/pm: Add support for reacting to platform profile notification Mario Limonciello 2021-11-10 15:13 ` Alex Deucher 2021-11-10 16:05 ` Lazar, Lijo 2021-11-10 17:31 ` Limonciello, Mario 2021-11-10 17:51 ` Alex Deucher 2021-11-11 4:53 ` Lazar, Lijo 2021-11-11 16:17 ` Limonciello, Mario
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.