| Message ID | 20220909164534.71864-5-Perry.Yuan@amd.com |
|---|---|
| State | New |
| Headers | show |
| Series | Implement AMD Pstate EPP Driver | expand |
[AMD Official Use Only - General] Hi Mario. > -----Original Message----- > From: Limonciello, Mario <Mario.Limonciello@amd.com> > Sent: Saturday, September 10, 2022 2:53 AM > To: Yuan, Perry <Perry.Yuan@amd.com>; rafael.j.wysocki@intel.com; Huang, > Ray <Ray.Huang@amd.com>; viresh.kumar@linaro.org > Cc: Sharma, Deepak <Deepak.Sharma@amd.com>; Fontenot, Nathan > <Nathan.Fontenot@amd.com>; Deucher, Alexander > <Alexander.Deucher@amd.com>; Su, Jinzhou (Joe) <Jinzhou.Su@amd.com>; > Huang, Shimmer <Shimmer.Huang@amd.com>; Du, Xiaojian > <Xiaojian.Du@amd.com>; Meng, Li (Jassmine) <Li.Meng@amd.com>; linux- > pm@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH 4/7] cpufreq: amd_pstate: add AMD pstate EPP support > for shared memory type processor > > On 9/9/2022 11:45, Perry Yuan wrote: > > Add Energy Performance Preference support for AMD SOCs which only > > support the shared memory interface that implemented on Zen2 and Zen3 > > processors, because this type CPU has no MSR supported, it will use > > ACPI PCC channel to enable EPP and reset desired perf to be zero. > > This reads like all Zen2 and Zen3 processors don't have the MSR, but that's > not true. How about: > > "Add Energy Performance Preference support for AMD SOCs which do not > contain a designated MSR for CPPC support. A shared memory interface is > used for CPPC on these SOCs and the ACPI PCC channel is used to enable EPP > and reset the desired performance." > Yes, those new interfaces are added to support the none MSR processors on EPP mode. Will update the commit info like you suggested. Thanks Perry. > > > > Signed-off-by: Perry Yuan <Perry.Yuan@amd.com> > > --- > > drivers/cpufreq/amd-pstate.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/drivers/cpufreq/amd-pstate.c > > b/drivers/cpufreq/amd-pstate.c index 451295284a26..fff298744a8e 100644 > > --- a/drivers/cpufreq/amd-pstate.c > > +++ b/drivers/cpufreq/amd-pstate.c > > @@ -133,12 +133,24 @@ static inline int pstate_enable(bool enable) > > > > static int cppc_enable(bool enable) > > { > > + struct cppc_perf_ctrls perf_ctrls; > > int cpu, ret = 0; > > > > for_each_present_cpu(cpu) { > > ret = cppc_set_enable(cpu, enable); > > if (ret) > > return ret; > > + > > + /* Enable active mode for EPP */ > > + ret = cppc_set_auto_epp(cpu, enable); > > + if (ret) > > + return ret; > > + > > + /* Set zero to desired perf to enable EPP control*/ > > + perf_ctrls.desired_perf = 0; > > + ret = cppc_set_perf(cpu, &perf_ctrls); > > + if (ret) > > + return ret; > > } > > > > return ret;
On 9/9/22 11:45, Perry Yuan wrote: > Add Energy Performance Preference support for AMD SOCs which only > support the shared memory interface that implemented on Zen2 and Zen3 > processors, because this type CPU has no MSR supported, it will use > ACPI PCC channel to enable EPP and reset desired perf to be zero. > > Signed-off-by: Perry Yuan <Perry.Yuan@amd.com> > --- > drivers/cpufreq/amd-pstate.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c > index 451295284a26..fff298744a8e 100644 > --- a/drivers/cpufreq/amd-pstate.c > +++ b/drivers/cpufreq/amd-pstate.c > @@ -133,12 +133,24 @@ static inline int pstate_enable(bool enable) > > static int cppc_enable(bool enable) > { > + struct cppc_perf_ctrls perf_ctrls; > int cpu, ret = 0; > > for_each_present_cpu(cpu) { > ret = cppc_set_enable(cpu, enable); > if (ret) > return ret; > + > + /* Enable active mode for EPP */ > + ret = cppc_set_auto_epp(cpu, enable); > + if (ret) > + return ret; > + > + /* Set zero to desired perf to enable EPP control*/ > + perf_ctrls.desired_perf = 0; > + ret = cppc_set_perf(cpu, &perf_ctrls); > + if (ret) > + return ret; Shouldn't this entire block be indented one additional tab over since its part of the for_each_present_cpu() loop? -Nathan > } > > return ret;
[AMD Official Use Only - General] Hi Nathan. > -----Original Message----- > From: Fontenot, Nathan <Nathan.Fontenot@amd.com> > Sent: Friday, September 16, 2022 12:24 AM > To: Yuan, Perry <Perry.Yuan@amd.com>; rafael.j.wysocki@intel.com; Huang, > Ray <Ray.Huang@amd.com>; viresh.kumar@linaro.org > Cc: Sharma, Deepak <Deepak.Sharma@amd.com>; Limonciello, Mario > <Mario.Limonciello@amd.com>; Deucher, Alexander > <Alexander.Deucher@amd.com>; Su, Jinzhou (Joe) <Jinzhou.Su@amd.com>; > Huang, Shimmer <Shimmer.Huang@amd.com>; Du, Xiaojian > <Xiaojian.Du@amd.com>; Meng, Li (Jassmine) <Li.Meng@amd.com>; linux- > pm@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH 4/7] cpufreq: amd_pstate: add AMD pstate EPP support > for shared memory type processor > > On 9/9/22 11:45, Perry Yuan wrote: > > Add Energy Performance Preference support for AMD SOCs which only > > support the shared memory interface that implemented on Zen2 and Zen3 > > processors, because this type CPU has no MSR supported, it will use > > ACPI PCC channel to enable EPP and reset desired perf to be zero. > > > > Signed-off-by: Perry Yuan <Perry.Yuan@amd.com> > > --- > > drivers/cpufreq/amd-pstate.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/drivers/cpufreq/amd-pstate.c > > b/drivers/cpufreq/amd-pstate.c index 451295284a26..fff298744a8e 100644 > > --- a/drivers/cpufreq/amd-pstate.c > > +++ b/drivers/cpufreq/amd-pstate.c > > @@ -133,12 +133,24 @@ static inline int pstate_enable(bool enable) > > > > static int cppc_enable(bool enable) > > { > > + struct cppc_perf_ctrls perf_ctrls; > > int cpu, ret = 0; > > > > for_each_present_cpu(cpu) { > > ret = cppc_set_enable(cpu, enable); > > if (ret) > > return ret; > > + > > + /* Enable active mode for EPP */ > > + ret = cppc_set_auto_epp(cpu, enable); > > + if (ret) > > + return ret; > > + > > + /* Set zero to desired perf to enable EPP control*/ > > + perf_ctrls.desired_perf = 0; > > + ret = cppc_set_perf(cpu, &perf_ctrls); > > + if (ret) > > + return ret; > > Shouldn't this entire block be indented one additional tab over since its part > of the for_each_present_cpu() loop? > > -Nathan Yes, I can see the indent in my local vscode editor, I do not know why the patch didn`t show that. Is it correct as below ? for_each_present_cpu(cpu) { ret = cppc_set_enable(cpu, enable); if (ret) return ret; if (epp_enabled) { /* Enable autonomous mode for EPP */ ret = cppc_set_auto_epp(cpu, enable); if (ret) return ret; /* Set zero to desired perf to allow EPP firmware control*/ perf_ctrls.desired_perf = 0; ret = cppc_set_perf(cpu, &perf_ctrls); if (ret) return ret; } } Perry. > > > } > > > > return ret;
[AMD Official Use Only - General] HI Mario. > -----Original Message----- > From: Limonciello, Mario <Mario.Limonciello@amd.com> > Sent: Saturday, September 10, 2022 2:53 AM > To: Yuan, Perry <Perry.Yuan@amd.com>; rafael.j.wysocki@intel.com; Huang, > Ray <Ray.Huang@amd.com>; viresh.kumar@linaro.org > Cc: Sharma, Deepak <Deepak.Sharma@amd.com>; Fontenot, Nathan > <Nathan.Fontenot@amd.com>; Deucher, Alexander > <Alexander.Deucher@amd.com>; Su, Jinzhou (Joe) <Jinzhou.Su@amd.com>; > Huang, Shimmer <Shimmer.Huang@amd.com>; Du, Xiaojian > <Xiaojian.Du@amd.com>; Meng, Li (Jassmine) <Li.Meng@amd.com>; linux- > pm@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH 4/7] cpufreq: amd_pstate: add AMD pstate EPP support > for shared memory type processor > > On 9/9/2022 11:45, Perry Yuan wrote: > > Add Energy Performance Preference support for AMD SOCs which only > > support the shared memory interface that implemented on Zen2 and Zen3 > > processors, because this type CPU has no MSR supported, it will use > > ACPI PCC channel to enable EPP and reset desired perf to be zero. > > This reads like all Zen2 and Zen3 processors don't have the MSR, but that's > not true. How about: > > "Add Energy Performance Preference support for AMD SOCs which do not > contain a designated MSR for CPPC support. A shared memory interface is > used for CPPC on these SOCs and the ACPI PCC channel is used to enable EPP > and reset the desired performance." > This is more correctly to describe the detail, take this into V2. Thank you ! Perry. > > > > Signed-off-by: Perry Yuan <Perry.Yuan@amd.com> > > --- > > drivers/cpufreq/amd-pstate.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/drivers/cpufreq/amd-pstate.c > > b/drivers/cpufreq/amd-pstate.c index 451295284a26..fff298744a8e 100644 > > --- a/drivers/cpufreq/amd-pstate.c > > +++ b/drivers/cpufreq/amd-pstate.c > > @@ -133,12 +133,24 @@ static inline int pstate_enable(bool enable) > > > > static int cppc_enable(bool enable) > > { > > + struct cppc_perf_ctrls perf_ctrls; > > int cpu, ret = 0; > > > > for_each_present_cpu(cpu) { > > ret = cppc_set_enable(cpu, enable); > > if (ret) > > return ret; > > + > > + /* Enable active mode for EPP */ > > + ret = cppc_set_auto_epp(cpu, enable); > > + if (ret) > > + return ret; > > + > > + /* Set zero to desired perf to enable EPP control*/ > > + perf_ctrls.desired_perf = 0; > > + ret = cppc_set_perf(cpu, &perf_ctrls); > > + if (ret) > > + return ret; > > } > > > > return ret;
On 9/25/22 07:23, Yuan, Perry wrote: > [AMD Official Use Only - General] > > Hi Nathan. > > >> -----Original Message----- >> From: Fontenot, Nathan <Nathan.Fontenot@amd.com> >> Sent: Friday, September 16, 2022 12:24 AM >> To: Yuan, Perry <Perry.Yuan@amd.com>; rafael.j.wysocki@intel.com; Huang, >> Ray <Ray.Huang@amd.com>; viresh.kumar@linaro.org >> Cc: Sharma, Deepak <Deepak.Sharma@amd.com>; Limonciello, Mario >> <Mario.Limonciello@amd.com>; Deucher, Alexander >> <Alexander.Deucher@amd.com>; Su, Jinzhou (Joe) <Jinzhou.Su@amd.com>; >> Huang, Shimmer <Shimmer.Huang@amd.com>; Du, Xiaojian >> <Xiaojian.Du@amd.com>; Meng, Li (Jassmine) <Li.Meng@amd.com>; linux- >> pm@vger.kernel.org; linux-kernel@vger.kernel.org >> Subject: Re: [PATCH 4/7] cpufreq: amd_pstate: add AMD pstate EPP support >> for shared memory type processor >> >> On 9/9/22 11:45, Perry Yuan wrote: >>> Add Energy Performance Preference support for AMD SOCs which only >>> support the shared memory interface that implemented on Zen2 and Zen3 >>> processors, because this type CPU has no MSR supported, it will use >>> ACPI PCC channel to enable EPP and reset desired perf to be zero. >>> >>> Signed-off-by: Perry Yuan <Perry.Yuan@amd.com> >>> --- >>> drivers/cpufreq/amd-pstate.c | 12 ++++++++++++ >>> 1 file changed, 12 insertions(+) >>> >>> diff --git a/drivers/cpufreq/amd-pstate.c >>> b/drivers/cpufreq/amd-pstate.c index 451295284a26..fff298744a8e 100644 >>> --- a/drivers/cpufreq/amd-pstate.c >>> +++ b/drivers/cpufreq/amd-pstate.c >>> @@ -133,12 +133,24 @@ static inline int pstate_enable(bool enable) >>> >>> static int cppc_enable(bool enable) >>> { >>> + struct cppc_perf_ctrls perf_ctrls; >>> int cpu, ret = 0; >>> >>> for_each_present_cpu(cpu) { >>> ret = cppc_set_enable(cpu, enable); >>> if (ret) >>> return ret; >>> + >>> + /* Enable active mode for EPP */ >>> + ret = cppc_set_auto_epp(cpu, enable); >>> + if (ret) >>> + return ret; >>> + >>> + /* Set zero to desired perf to enable EPP control*/ >>> + perf_ctrls.desired_perf = 0; >>> + ret = cppc_set_perf(cpu, &perf_ctrls); >>> + if (ret) >>> + return ret; >> >> Shouldn't this entire block be indented one additional tab over since its part >> of the for_each_present_cpu() loop? >> >> -Nathan > > Yes, I can see the indent in my local vscode editor, I do not know why the patch didn`t show that. > Is it correct as below ? > > for_each_present_cpu(cpu) { > ret = cppc_set_enable(cpu, enable); > if (ret) > return ret; > > if (epp_enabled) { > /* Enable autonomous mode for EPP */ > ret = cppc_set_auto_epp(cpu, enable); > if (ret) > return ret; > > /* Set zero to desired perf to allow EPP firmware control*/ > perf_ctrls.desired_perf = 0; > ret = cppc_set_perf(cpu, &perf_ctrls); > if (ret) > return ret; > } > } > > Perry. That looks much better. -Nathan > >> >>> } >>> >>> return ret;
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c index 451295284a26..fff298744a8e 100644 --- a/drivers/cpufreq/amd-pstate.c +++ b/drivers/cpufreq/amd-pstate.c @@ -133,12 +133,24 @@ static inline int pstate_enable(bool enable) static int cppc_enable(bool enable) { + struct cppc_perf_ctrls perf_ctrls; int cpu, ret = 0; for_each_present_cpu(cpu) { ret = cppc_set_enable(cpu, enable); if (ret) return ret; + + /* Enable active mode for EPP */ + ret = cppc_set_auto_epp(cpu, enable); + if (ret) + return ret; + + /* Set zero to desired perf to enable EPP control*/ + perf_ctrls.desired_perf = 0; + ret = cppc_set_perf(cpu, &perf_ctrls); + if (ret) + return ret; } return ret;
Add Energy Performance Preference support for AMD SOCs which only support the shared memory interface that implemented on Zen2 and Zen3 processors, because this type CPU has no MSR supported, it will use ACPI PCC channel to enable EPP and reset desired perf to be zero. Signed-off-by: Perry Yuan <Perry.Yuan@amd.com> --- drivers/cpufreq/amd-pstate.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)