Received-SPF: pass (google.com: domain of linux-kernel+bounces-57492-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Thu, 8 Feb 2024 11:37:29 +0530
From: "Gautham R. Shenoy" <gautham.shenoy@amd.com>
To: Perry Yuan <perry.yuan@amd.com>
Cc: rafael.j.wysocki@intel.com, Mario.Limonciello@amd.com,
	viresh.kumar@linaro.org, Ray.Huang@amd.com, Borislav.Petkov@amd.com,
	Alexander.Deucher@amd.com, Xinmei.Huang@amd.com,
	Xiaojian.Du@amd.com, Li.Meng@amd.com, linux-pm@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v6 2/6] cpufreq:amd-pstate: fix the nominal freq value set
Message-ID: <ZcRvoYZKdUEjBUHp@BLR-5CG11610CF.amd.com>
References: <cover.1707363758.git.perry.yuan@amd.com>
 <ebbe3741501a31b1ffcea208393cdd8929df6455.1707363758.git.perry.yuan@amd.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ebbe3741501a31b1ffcea208393cdd8929df6455.1707363758.git.perry.yuan@amd.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?I27UkG6PXkpojh3Prdh1XtdycUl3bqPRQsVHHXlkgNhJBQ3rVokOENSb2Oo+?=
 =?us-ascii?Q?zoqz1Vtavj2DTaDDIT49Etxe0OcGMHxCiZ9rZ9KNjzB5me1gHvahvpQxypLm?=
 =?us-ascii?Q?6mW+dUUwq/yfatp5GPn6xNuioxQ0o6O+VTIHJpin1+kZebzkUVo0fPBctnrV?=
 =?us-ascii?Q?UkGclKeHEuEG+2Rlu05G8hSgxRlngUfimZHV0utbNgkvhvIeYmSZ5ZRXZxBj?=
 =?us-ascii?Q?FcAFqkxHKEwAy9t0X0fJWxkBDVYP6WiMQ8ixlvJODQaZHT3/mZE3pKJ3DK2c?=
 =?us-ascii?Q?wcG6yTqseWaqB7AcxzXvfjE9ULoRwBWpHlUQOh6fDoNepq0sjDerlnloIbGB?=
 =?us-ascii?Q?kJEI8bVPVXdyhFYzuhwto8Tzd82gB0za65D00sinN6mvHAlDqTg36OJVsKIx?=
 =?us-ascii?Q?VLpI9PiXQV8Zs5bmwqgDDtH5oCPtyEVT4qFQp8gY5aQRqtP4Vro7c+UWgtTg?=
 =?us-ascii?Q?Q9+4U0ZdQW0U8IDiKyzAUTrUZePA7hN8vz4gG+k1D+PwH8FES64mxDGF0GIm?=
 =?us-ascii?Q?JSiTAOJF/2UF7JG2Ca1jxLu/JWegd9MWPiKhnG1nxADEOop1THss00bNVaC6?=
 =?us-ascii?Q?mR031uKJly9mv7zz8dH45pS+ZnIKa4jchFgQIe5bcQ4U6gd0VBU9MFcRJ2ga?=
 =?us-ascii?Q?bpUXxEPDNaBOZLwTyEe8OFkqTEhc9mqDykz6D77XIkfOO0BQRxRfsWaVo8ci?=
 =?us-ascii?Q?gtBDYVwtmMCOk3dD6Rzh/QPFr4kvezy8CXe1z7vD0zj/Mdx+GQnGuWPeWUPN?=
 =?us-ascii?Q?Eo3DCwJ2glT9YSXb9WS2L4Wds01Ug5kHuqLzCIORuv7rlsbrF+unQ0GywkOg?=
 =?us-ascii?Q?b6R/EOBQDic9ItX0VDKESs3ZXknM1qod4qFycLZfLm8Pq4WLB3DjDBJiRLWF?=
 =?us-ascii?Q?9VeVOR/+kXFVoS7u3ZGsaVIDZsCDTHOc3Uu07au4DnLOWpI9R/waqPSIcrRj?=
 =?us-ascii?Q?rGVGWC6nXMochTQQCcxjU7zlSLU7vt/ZYscweoUXFnPa/AnQaUc2eoG0F1Rj?=
 =?us-ascii?Q?4n2KNNEPSE6FUIZ6noQPaukZNreL3umBuf57W2XjhKrAMAe2h5ms4BgKmhEC?=
 =?us-ascii?Q?g3BM1t6hR7SQYoZgmMCO7bPyVceJ1Qba5yOIbdPsdjPnSx3kjsRtPg51OD/3?=
 =?us-ascii?Q?jdtpIA86uPBT0WxtdTcaK6vXROC1Yqa6dngQEnyxSRr5oYhe5T2fx2hykGQb?=
 =?us-ascii?Q?fyGqZFBT7ZL182b/zNOt743yujfjdcrDIikIa+bKwC5SmCvffoDxsNGp9R+V?=
 =?us-ascii?Q?iu2sev0mnDDRsujSn+Sc0qZ9H3H5p37asRtJ8Wz1eAhFZsXXoVq5APdbTRrH?=
 =?us-ascii?Q?8zcWmVLscotVquNucI541i3/6VtqRTvbWWWH0uP9CjXS0R0UxdCmVr00P6j7?=
 =?us-ascii?Q?9z+emYusW9Ikm8sjryb5pPjhfoHkp9jPJP3QTqorJFDPFTzI8iihzW1q639e?=
 =?us-ascii?Q?ds5zvZJeRWwy3CGwyn1lYNwPAbxFZzm4UeV4oX0b3k+q6cF3JtZazUSJq41E?=
 =?us-ascii?Q?gNd6VFb/Vq9kM+oS5PbWFyDeSUKXotZF6uVGP75YHCouxtSe0ratQhoquijW?=
 =?us-ascii?Q?eOqPteso0KHF773YKaKybyud6SWAh4r7jlkUEL2/?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 3a879a62-af03-4071-4c63-08dc286c40c2
X-MS-Exchange-CrossTenant-AuthSource: DS7PR12MB8252.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Feb 2024 06:07:39.1274
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: CGjBVfe2WXUANuuYWCfXKaosaCRxg3iKs7aDPp23CeF9afGPwe1sEZaib+RYz+HU/oN1VJLkHaKllLCFZ8eHsg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ2PR12MB9113

On Thu, Feb 08, 2024 at 11:46:29AM +0800, Perry Yuan wrote:
> Address an untested error where the nominal_freq was returned in KHz
> instead of the correct MHz units, this oversight led to a wrong
> nominal_freq set and resued, it will cause the max frequency of core to
> be initialized with a wrong frequency value.

The _CPC object on my Zen2 EPYC server which has CPPC v3, I see :
            0x00000190,			//LowestFrequency 
            0x000007D0			//NominalFrequency
        })
    }
 
where 0x7D0 is 2000 Mhz. Thus the value returend by cppc_perf.nominal_freq is in Mhz.

In this patch, we are changing making amd_get_nominal_freq() return
2000 (Mhz) instead of 2000000 (Khz) as it was doing previously.

amd_get_min_freq(), amd_get_max_freq(),
amd_get_lowest_nonlinear_perf() -all these functions return the
frequency in Khz units.

These functions are used the initialize the value of
cpudata->max_freq, cpu_data->min_freq, .... all of which are in Khz
units.

In, amd_pstate_set_boost(), we have:
 
 
	if (state)
		policy->cpuinfo.max_freq = cpudata->max_freq;  <---- In Khz
	else
		policy->cpuinfo.max_freq = cpudata->nominal_freq; <--- Now in Mhz

Since policy->cpuinfo.max_freq is expected to be in Khz, isn't this
patch introducing an error ? Or perhaps this patch series is based on
another patchset ?

--
Thanks and Regards
gautham.

 
> 
> Cc: stable@vger.kernel.org
> Fixes: ec437d71db7 ("cpufreq: amd-pstate: Introduce a new AMD P-State driver to support future processors")
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> Signed-off-by: Perry Yuan <perry.yuan@amd.com>
> ---
>  drivers/cpufreq/amd-pstate.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 08e112444c27..ac7faa98a450 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -640,8 +640,7 @@ static int amd_get_nominal_freq(struct amd_cpudata *cpudata)
>  	if (ret)
>  		return ret;
>  
> -	/* Switch to khz */
> -	return cppc_perf.nominal_freq * 1000;
> +	return cppc_perf.nominal_freq;
>  }
>  
>  static int amd_get_lowest_nonlinear_freq(struct amd_cpudata *cpudata)
> -- 
> 2.34.1
>