[Public] The change here in smu_restore_dpm_user_profile will need a revision of patch 3 also 😊
Thanks, Lijo -----Original Message----- From: amd-gfx <[email protected]> On Behalf Of Mario Limonciello Sent: Monday, October 6, 2025 10:11 PM To: open list:RADEON and AMDGPU DRM DRIVERS <[email protected]> Subject: Re: [PATCH v2 2/5] drm/amd: Stop overloading power limit with limit type On 10/6/2025 11:31 AM, Mario Limonciello wrote: > When passed around internally the upper 8 bits of power limit include the > limit type. > This is non-obvious without digging into the nuances of each function. > Instead pass the limit type as an argument to all applicable layers. > > Signed-off-by: Mario Limonciello <[email protected]> Just realized that I missed moving a hunk from patch 3/5 to this one with the split. ❯ git diff diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c index c4459dc553f0..1c5f37cd5b75 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c @@ -510,7 +510,7 @@ static void smu_restore_dpm_user_profile(struct smu_context *smu) /* set the user dpm power limit */ if (smu->user_dpm_profile.power_limit) { - ret = smu_set_power_limit(smu, smu->user_dpm_profile.power_limit); + ret = smu_set_power_limit(smu, SMU_DEFAULT_PPT_LIMIT, smu->current_power_limit); if (ret) dev_err(smu->adev->dev, "Failed to set power limit value\n"); } @@ -2258,7 +2258,7 @@ static int smu_resume(struct amdgpu_ip_block *ip_block) adev->pm.dpm_enabled = true; if (smu->current_power_limit) { - ret = smu_set_power_limit(smu, smu->current_power_limit); + ret = smu_set_power_limit(smu, SMU_DEFAULT_PPT_LIMIT, smu->current_power_limit); if (ret && ret != -EOPNOTSUPP) return ret; } > --- > v2: > * split into two patches > --- > drivers/gpu/drm/amd/include/kgd_pp_interface.h | 2 +- > drivers/gpu/drm/amd/pm/amdgpu_dpm.c | 3 ++- > drivers/gpu/drm/amd/pm/amdgpu_pm.c | 3 +-- > drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h | 2 +- > drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c | 2 +- > drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 6 ++---- > drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c | 1 - > 7 files changed, 8 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h > b/drivers/gpu/drm/amd/include/kgd_pp_interface.h > index 2b0cdb2a2775..87814c2b526e 100644 > --- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h > +++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h > @@ -454,7 +454,7 @@ struct amd_pm_funcs { > bool gate, > int inst); > int (*set_clockgating_by_smu)(void *handle, uint32_t msg_id); > - int (*set_power_limit)(void *handle, uint32_t n); > + int (*set_power_limit)(void *handle, uint32_t limit_type, uint32_t > +n); > int (*get_power_limit)(void *handle, uint32_t *limit, > enum pp_power_limit_level pp_limit_level, > enum pp_power_type power_type); > diff --git a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c > b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c > index 518d07afc7df..5d08dc3b7110 100644 > --- a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c > +++ b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c > @@ -1616,6 +1616,7 @@ int amdgpu_dpm_get_power_limit(struct amdgpu_device > *adev, > } > > int amdgpu_dpm_set_power_limit(struct amdgpu_device *adev, > + uint32_t limit_type, > uint32_t limit) > { > const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs; @@ > -1626,7 +1627,7 @@ int amdgpu_dpm_set_power_limit(struct amdgpu_device > *adev, > > mutex_lock(&adev->pm.mutex); > ret = pp_funcs->set_power_limit(adev->powerplay.pp_handle, > - limit); > + limit_type, limit); > mutex_unlock(&adev->pm.mutex); > > return ret; > diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c > b/drivers/gpu/drm/amd/pm/amdgpu_pm.c > index b5fbb0fd1dc0..6bdf185c6b60 100644 > --- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c > +++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c > @@ -3390,13 +3390,12 @@ static ssize_t amdgpu_hwmon_set_power_cap(struct > device *dev, > return err; > > value = value / 1000000; /* convert to Watt */ > - value |= limit_type << 24; > > err = amdgpu_pm_get_access(adev); > if (err < 0) > return err; > > - err = amdgpu_dpm_set_power_limit(adev, value); > + err = amdgpu_dpm_set_power_limit(adev, limit_type, value); > > amdgpu_pm_put_access(adev); > > diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h > b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h > index 65c1d98af26c..3bce74f8bb0a 100644 > --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h > +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h > @@ -553,7 +553,7 @@ int amdgpu_dpm_get_power_limit(struct amdgpu_device *adev, > enum pp_power_limit_level pp_limit_level, > enum pp_power_type power_type); > int amdgpu_dpm_set_power_limit(struct amdgpu_device *adev, > - uint32_t limit); > + uint32_t limit_type, uint32_t limit); > int amdgpu_dpm_is_cclk_dpm_supported(struct amdgpu_device *adev); > int amdgpu_dpm_debugfs_print_current_performance_level(struct amdgpu_device > *adev, > struct seq_file *m); > diff --git a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c > b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c > index fb595a70bbd1..76a5353d7f4a 100644 > --- a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c > +++ b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c > @@ -952,7 +952,7 @@ static int pp_dpm_switch_power_profile(void *handle, > return 0; > } > > -static int pp_set_power_limit(void *handle, uint32_t limit) > +static int pp_set_power_limit(void *handle, uint32_t limit_type, > +uint32_t limit) > { > struct pp_hwmgr *hwmgr = handle; > uint32_t max_power_limit; > diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > index fb8086859857..c4459dc553f0 100644 > --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > @@ -68,7 +68,7 @@ static int smu_handle_task(struct smu_context *smu, > static int smu_reset(struct smu_context *smu); > static int smu_set_fan_speed_pwm(void *handle, u32 speed); > static int smu_set_fan_control_mode(void *handle, u32 value); > -static int smu_set_power_limit(void *handle, uint32_t limit); > +static int smu_set_power_limit(void *handle, uint32_t limit_type, > +uint32_t limit); > static int smu_set_fan_speed_rpm(void *handle, uint32_t speed); > static int smu_set_gfx_cgpg(struct smu_context *smu, bool enabled); > static int smu_set_mp1_state(void *handle, enum pp_mp1_state > mp1_state); @@ -2958,16 +2958,14 @@ int smu_get_power_limit(void *handle, > return ret; > } > > -static int smu_set_power_limit(void *handle, uint32_t limit) > +static int smu_set_power_limit(void *handle, uint32_t limit_type, > +uint32_t limit) > { > struct smu_context *smu = handle; > - uint32_t limit_type = limit >> 24; > int ret = 0; > > if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled) > return -EOPNOTSUPP; > > - limit &= (1<<24)-1; > if (limit_type != SMU_DEFAULT_PPT_LIMIT) > if (smu->ppt_funcs->set_power_limit) > return smu->ppt_funcs->set_power_limit(smu, limit_type, > limit); > diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c > b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c > index 2c9869feba61..81077a3969e9 100644 > --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c > +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c > @@ -2399,7 +2399,6 @@ static int vangogh_set_power_limit(struct smu_context > *smu, > smu->current_power_limit = ppt_limit; > break; > case SMU_FAST_PPT_LIMIT: > - ppt_limit &= ~(SMU_FAST_PPT_LIMIT << 24); > if (ppt_limit > power_context->max_fast_ppt_limit) { > dev_err(smu->adev->dev, > "New power limit (%d) is over the max allowed > %d\n",
