RE: [PATCH v4 4/9] cpufreq: amd_pstate: implement Pstate EPP support for the AMD processors

[Yuan, Perry]

[AMD Official Use Only - General]



> -----Original Message-----
> From: Karny, Wyes <Wyes.Karny@xxxxxxx>
> Sent: Thursday, November 17, 2022 10:03 PM
> To: Yuan, Perry <Perry.Yuan@xxxxxxx>; rafael.j.wysocki@xxxxxxxxx;
> Limonciello, Mario <Mario.Limonciello@xxxxxxx>; Huang, Ray
> <Ray.Huang@xxxxxxx>; viresh.kumar@xxxxxxxxxx
> Cc: Sharma, Deepak <Deepak.Sharma@xxxxxxx>; Fontenot, Nathan
> <Nathan.Fontenot@xxxxxxx>; Deucher, Alexander
> <Alexander.Deucher@xxxxxxx>; Huang, Shimmer
> <Shimmer.Huang@xxxxxxx>; Du, Xiaojian <Xiaojian.Du@xxxxxxx>; Meng, Li
> (Jassmine) <Li.Meng@xxxxxxx>; linux-pm@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v4 4/9] cpufreq: amd_pstate: implement Pstate EPP support
> for the AMD processors
> 
> Hi Perry,
> 
> On 11/10/2022 11:28 PM, Perry Yuan wrote:
> [snip]
> > +
> > +static ssize_t store_energy_performance_preference(
> > +		struct cpufreq_policy *policy, const char *buf, size_t count) {
> > +	struct amd_cpudata *cpudata = policy->driver_data;
> > +	char str_preference[21];
> > +	ssize_t ret;
> > +	u32 epp = 0;
> 
> Here `epp` is a unused variable.
>  [snip]
> > @@ -657,6 +1281,20 @@ static int __init amd_pstate_param(char *str)
> >  	if (!str)
> >  		return -EINVAL;
> >
> > +	/*
> > +	 * Disable amd-pstate driver by default for now
> > +	 * append amd-pstate=active or amd-pstate=active
> > +	 * to enable driver loaded
> > +	 */
> > +	if (!strcmp(str, "active")) {
> > +		cppc_load = 1;
> > +		default_pstate_driver = &amd_pstate_epp_driver;
> > +	} else if (!strcmp(str, "passive")) {
> > +		epp_off = 1;
> > +		cppc_load = 1;
> > +		default_pstate_driver = &amd_pstate_driver;
> > +	}
> > +
> >  	/*
> >  	 * support shared memory type CPPC which has no MSR function.
> >  	 * enable amd-pstate on processors with shared memory solution
> 
> In the shared memory support section also `cppc_load` has to be set otherwise
> amd_pstate=legacy_cppc will not work.
If the patches just got reviewed are accepted, the legacy_cppcc will not be needed any more.
I  will rebase the EPP patches based on the that. 


> 
> if (!strcmp(str, "legacy_cppc")) {
> 	shared_mem = true;
> 	cppc_load = 1;
> }

Previous plan was to add two parameters to kernel command line, 
passive or active  and legacy_cppc for shared memory platform.

> 
> 
> --
> Thanks & Regards,
> Wyes