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 <[email protected]>
---
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;
--
2.34.1
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."
>
> Signed-off-by: Perry Yuan <[email protected]>
> ---
> 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;
[AMD Official Use Only - General]
Hi Mario.
> -----Original Message-----
> From: Limonciello, Mario <[email protected]>
> Sent: Saturday, September 10, 2022 2:53 AM
> To: Yuan, Perry <[email protected]>; [email protected]; Huang,
> Ray <[email protected]>; [email protected]
> Cc: Sharma, Deepak <[email protected]>; Fontenot, Nathan
> <[email protected]>; Deucher, Alexander
> <[email protected]>; Su, Jinzhou (Joe) <[email protected]>;
> Huang, Shimmer <[email protected]>; Du, Xiaojian
> <[email protected]>; Meng, Li (Jassmine) <[email protected]>; linux-
> [email protected]; [email protected]
> 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 <[email protected]>
> > ---
> > 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 <[email protected]>
> ---
> 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 <[email protected]>
> Sent: Friday, September 16, 2022 12:24 AM
> To: Yuan, Perry <[email protected]>; [email protected]; Huang,
> Ray <[email protected]>; [email protected]
> Cc: Sharma, Deepak <[email protected]>; Limonciello, Mario
> <[email protected]>; Deucher, Alexander
> <[email protected]>; Su, Jinzhou (Joe) <[email protected]>;
> Huang, Shimmer <[email protected]>; Du, Xiaojian
> <[email protected]>; Meng, Li (Jassmine) <[email protected]>; linux-
> [email protected]; [email protected]
> 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 <[email protected]>
> > ---
> > 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 <[email protected]>
> Sent: Saturday, September 10, 2022 2:53 AM
> To: Yuan, Perry <[email protected]>; [email protected]; Huang,
> Ray <[email protected]>; [email protected]
> Cc: Sharma, Deepak <[email protected]>; Fontenot, Nathan
> <[email protected]>; Deucher, Alexander
> <[email protected]>; Su, Jinzhou (Joe) <[email protected]>;
> Huang, Shimmer <[email protected]>; Du, Xiaojian
> <[email protected]>; Meng, Li (Jassmine) <[email protected]>; linux-
> [email protected]; [email protected]
> 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 <[email protected]>
> > ---
> > 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 <[email protected]>
>> Sent: Friday, September 16, 2022 12:24 AM
>> To: Yuan, Perry <[email protected]>; [email protected]; Huang,
>> Ray <[email protected]>; [email protected]
>> Cc: Sharma, Deepak <[email protected]>; Limonciello, Mario
>> <[email protected]>; Deucher, Alexander
>> <[email protected]>; Su, Jinzhou (Joe) <[email protected]>;
>> Huang, Shimmer <[email protected]>; Du, Xiaojian
>> <[email protected]>; Meng, Li (Jassmine) <[email protected]>; linux-
>> [email protected]; [email protected]
>> 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 <[email protected]>
>>> ---
>>> 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;