Re: [PATCH 2/2] drm/amd/pm: fix incorrect default power limit in powerplay

[Eric Huang <jinhuieric.huang@xxxxxxx>]

Please ignore those two patches. I found a mistake and will send new 
ones for review.

Regards,
Eric

On 2021-03-12 4:03 p.m., Eric Huang wrote:
> There is no difference betweeen max and default to get
> power limit in powerplay, adding a new parameter in the
> funciton fixes it.
>
> Signed-off-by: Eric Huang <jinhuieric.huang@xxxxxxx>
> ---
>   drivers/gpu/drm/amd/include/kgd_pp_interface.h   |  3 ++-
>   drivers/gpu/drm/amd/pm/amdgpu_pm.c               | 12 ++++++++----
>   drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c | 11 ++++++-----
>   3 files changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
> index 79e309a789f3..a0bdb0ac25fa 100644
> --- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
> +++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
> @@ -288,7 +288,8 @@ struct amd_pm_funcs {
>   				uint32_t block_type, bool gate);
>   	int (*set_clockgating_by_smu)(void *handle, uint32_t msg_id);
>   	int (*set_power_limit)(void *handle, uint32_t n);
> -	int (*get_power_limit)(void *handle, uint32_t *limit, bool default_limit);
> +	int (*get_power_limit)(void *handle, uint32_t *limit, uint32_t *limit_max,
> +			bool default_limit);
>   	int (*get_power_profile_mode)(void *handle, char *buf);
>   	int (*set_power_profile_mode)(void *handle, long *input, uint32_t size);
>   	int (*set_fine_grain_clk_vol)(void *handle, uint32_t type, long *input, uint32_t size);
> diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> index 8ab6e7eaf6a1..23b6cc1746c1 100644
> --- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> @@ -2613,6 +2613,7 @@ static ssize_t amdgpu_hwmon_show_power_cap_max(struct device *dev,
>   	const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
>   	int limit_type = to_sensor_dev_attr(attr)->index;
>   	uint32_t limit = limit_type << 24;
> +	uint32_t limit_max = 0;
>   	ssize_t size;
>   	int r;
>   
> @@ -2629,8 +2630,9 @@ static ssize_t amdgpu_hwmon_show_power_cap_max(struct device *dev,
>   		smu_get_power_limit(&adev->smu, &limit, SMU_PPT_LIMIT_MAX);
>   		size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
>   	} else if (pp_funcs && pp_funcs->get_power_limit) {
> -		pp_funcs->get_power_limit(adev->powerplay.pp_handle, &limit, true);
> -		size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
> +		pp_funcs->get_power_limit(adev->powerplay.pp_handle,
> +				&limit, &limit_max, true);
> +		size = snprintf(buf, PAGE_SIZE, "%u\n", limit_max * 1000000);
>   	} else {
>   		size = snprintf(buf, PAGE_SIZE, "\n");
>   	}
> @@ -2665,7 +2667,8 @@ static ssize_t amdgpu_hwmon_show_power_cap(struct device *dev,
>   		smu_get_power_limit(&adev->smu, &limit, SMU_PPT_LIMIT_CURRENT);
>   		size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
>   	} else if (pp_funcs && pp_funcs->get_power_limit) {
> -		pp_funcs->get_power_limit(adev->powerplay.pp_handle, &limit, false);
> +		pp_funcs->get_power_limit(adev->powerplay.pp_handle,
> +				&limit, NULL, false);
>   		size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
>   	} else {
>   		size = snprintf(buf, PAGE_SIZE, "\n");
> @@ -2701,7 +2704,8 @@ static ssize_t amdgpu_hwmon_show_power_cap_default(struct device *dev,
>   		smu_get_power_limit(&adev->smu, &limit, SMU_PPT_LIMIT_DEFAULT);
>   		size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
>   	} else if (pp_funcs && pp_funcs->get_power_limit) {
> -		pp_funcs->get_power_limit(adev->powerplay.pp_handle, &limit, true);
> +		pp_funcs->get_power_limit(adev->powerplay.pp_handle,
> +				&limit, NULL, true);
>   		size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
>   	} else {
>   		size = snprintf(buf, PAGE_SIZE, "\n");
> diff --git a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
> index e0d288208220..b60f46b80ccd 100644
> --- a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
> +++ b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
> @@ -1034,7 +1034,8 @@ static int pp_set_power_limit(void *handle, uint32_t limit)
>   	return 0;
>   }
>   
> -static int pp_get_power_limit(void *handle, uint32_t *limit, bool default_limit)
> +static int pp_get_power_limit(void *handle, uint32_t *limit,
> +		uint32_t *limit_max, bool default_limit)
>   {
>   	struct pp_hwmgr *hwmgr = handle;
>   
> @@ -1045,10 +1046,10 @@ static int pp_get_power_limit(void *handle, uint32_t *limit, bool default_limit)
>   
>   	if (default_limit) {
>   		*limit = hwmgr->default_power_limit;
> -		if (hwmgr->od_enabled) {
> -			*limit *= (100 + hwmgr->platform_descriptor.TDPODLimit);
> -			*limit /= 100;
> -		}
> +		if (limit_max && hwmgr->od_enabled)
> +			*limit_max = *limit *
> +				(100 + hwmgr->platform_descriptor.TDPODLimit) /
> +				100;
>   	}
>   	else
>   		*limit = hwmgr->power_limit;

_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx