diff mbox series

[2/2] cpufreq/amd-pstate: Adding Zen4 support in introduction

Message ID 20240109175844.448006-1-e.velu@criteo.com (mailing list archive)
State Changes Requested, archived
Headers show
Series [1/2] admin-guide: Fixing typos | expand

Commit Message

Erwan Velu Jan. 9, 2024, 5:58 p.m. UTC
amd-pstate is implemented on Zen4, adding this architecture in the
introduction.

Signed-off-by: Erwan Velu <e.velu@criteo.com>
---
 drivers/cpufreq/amd-pstate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Rafael J. Wysocki Jan. 10, 2024, 2:21 p.m. UTC | #1
On Tue, Jan 9, 2024 at 6:58 PM Erwan Velu <erwanaliasr1@gmail.com> wrote:
>
> amd-pstate is implemented on Zen4, adding this architecture in the
> introduction.
>
> Signed-off-by: Erwan Velu <e.velu@criteo.com>
> ---
>  drivers/cpufreq/amd-pstate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 1f6186475715..9a8d083f6ba5 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -14,7 +14,7 @@
>   * communicate the performance hints to hardware.
>   *
>   * AMD P-State is supported on recent AMD Zen base CPU series include some of
> - * Zen2 and Zen3 processors. _CPC needs to be present in the ACPI tables of AMD
> + * Zen2, Zen3 and Zen4 processors. _CPC needs to be present in the ACPI tables of AMD

IMO, it would be really nice to avoid extending this line so much, but
this is up to the AMD people.

Also, while you are at it, "include" in the first sentence should be
"including" and a new paragraph after "processors." would make this
comment easier to follow from the logical standpoint, because the
first sentence is about the supported platforms and the second one is
about additional requirements (ie. something else).

>   * P-State supported system. And there are two types of hardware implementations
>   * for AMD P-State: 1) Full MSR Solution and 2) Shared Memory Solution.
>   * X86_FEATURE_CPPC CPU feature flag is used to distinguish the different types.
> --
Erwan Velu Jan. 10, 2024, 2:30 p.m. UTC | #2
So it's probably better to let AMD people fixing this part.
Thx for the feedback.

Le mer. 10 janv. 2024 à 15:21, Rafael J. Wysocki <rafael@kernel.org> a écrit :
>
> On Tue, Jan 9, 2024 at 6:58 PM Erwan Velu <erwanaliasr1@gmail.com> wrote:
> >
> > amd-pstate is implemented on Zen4, adding this architecture in the
> > introduction.
> >
> > Signed-off-by: Erwan Velu <e.velu@criteo.com>
> > ---
> >  drivers/cpufreq/amd-pstate.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> > index 1f6186475715..9a8d083f6ba5 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -14,7 +14,7 @@
> >   * communicate the performance hints to hardware.
> >   *
> >   * AMD P-State is supported on recent AMD Zen base CPU series include some of
> > - * Zen2 and Zen3 processors. _CPC needs to be present in the ACPI tables of AMD
> > + * Zen2, Zen3 and Zen4 processors. _CPC needs to be present in the ACPI tables of AMD
>
> IMO, it would be really nice to avoid extending this line so much, but
> this is up to the AMD people.
>
> Also, while you are at it, "include" in the first sentence should be
> "including" and a new paragraph after "processors." would make this
> comment easier to follow from the logical standpoint, because the
> first sentence is about the supported platforms and the second one is
> about additional requirements (ie. something else).
>
> >   * P-State supported system. And there are two types of hardware implementations
> >   * for AMD P-State: 1) Full MSR Solution and 2) Shared Memory Solution.
> >   * X86_FEATURE_CPPC CPU feature flag is used to distinguish the different types.
> > --
Huang Rui Jan. 10, 2024, 5:19 p.m. UTC | #3
On Wed, Jan 10, 2024 at 10:21:20PM +0800, Rafael J. Wysocki wrote:
> On Tue, Jan 9, 2024 at 6:58 PM Erwan Velu <erwanaliasr1@gmail.com> wrote:
> >
> > amd-pstate is implemented on Zen4, adding this architecture in the
> > introduction.
> >
> > Signed-off-by: Erwan Velu <e.velu@criteo.com>
> > ---
> >  drivers/cpufreq/amd-pstate.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> > index 1f6186475715..9a8d083f6ba5 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -14,7 +14,7 @@
> >   * communicate the performance hints to hardware.
> >   *
> >   * AMD P-State is supported on recent AMD Zen base CPU series include some of
> > - * Zen2 and Zen3 processors. _CPC needs to be present in the ACPI tables of AMD
> > + * Zen2, Zen3 and Zen4 processors. _CPC needs to be present in the ACPI tables of AMD
> 
> IMO, it would be really nice to avoid extending this line so much, but
> this is up to the AMD people.

Agree. Actually, we will use this driver for all future AMD CPU generations
that starts from paritial Zen2.

> 
> Also, while you are at it, "include" in the first sentence should be
> "including" and a new paragraph after "processors." would make this
> comment easier to follow from the logical standpoint, because the
> first sentence is about the supported platforms and the second one is
> about additional requirements (ie. something else).
> 

How about modifying here as "partial Zen2, Zen3, and future processors"?

Thanks,
Ray

> >   * P-State supported system. And there are two types of hardware implementations
> >   * for AMD P-State: 1) Full MSR Solution and 2) Shared Memory Solution.
> >   * X86_FEATURE_CPPC CPU feature flag is used to distinguish the different types.
> > --
Erwan Velu Jan. 10, 2024, 5:49 p.m. UTC | #4
> How about modifying here as "partial Zen2, Zen3, and future processors"?
And all processors starting Zen4 ?

Le mer. 10 janv. 2024 à 18:19, Huang Rui <ray.huang@amd.com> a écrit :
>
> On Wed, Jan 10, 2024 at 10:21:20PM +0800, Rafael J. Wysocki wrote:
> > On Tue, Jan 9, 2024 at 6:58 PM Erwan Velu <erwanaliasr1@gmail.com> wrote:
> > >
> > > amd-pstate is implemented on Zen4, adding this architecture in the
> > > introduction.
> > >
> > > Signed-off-by: Erwan Velu <e.velu@criteo.com>
> > > ---
> > >  drivers/cpufreq/amd-pstate.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> > > index 1f6186475715..9a8d083f6ba5 100644
> > > --- a/drivers/cpufreq/amd-pstate.c
> > > +++ b/drivers/cpufreq/amd-pstate.c
> > > @@ -14,7 +14,7 @@
> > >   * communicate the performance hints to hardware.
> > >   *
> > >   * AMD P-State is supported on recent AMD Zen base CPU series include some of
> > > - * Zen2 and Zen3 processors. _CPC needs to be present in the ACPI tables of AMD
> > > + * Zen2, Zen3 and Zen4 processors. _CPC needs to be present in the ACPI tables of AMD
> >
> > IMO, it would be really nice to avoid extending this line so much, but
> > this is up to the AMD people.
>
> Agree. Actually, we will use this driver for all future AMD CPU generations
> that starts from paritial Zen2.
>
> >
> > Also, while you are at it, "include" in the first sentence should be
> > "including" and a new paragraph after "processors." would make this
> > comment easier to follow from the logical standpoint, because the
> > first sentence is about the supported platforms and the second one is
> > about additional requirements (ie. something else).
> >
>
> How about modifying here as "partial Zen2, Zen3, and future processors"?
>
> Thanks,
> Ray
>
> > >   * P-State supported system. And there are two types of hardware implementations
> > >   * for AMD P-State: 1) Full MSR Solution and 2) Shared Memory Solution.
> > >   * X86_FEATURE_CPPC CPU feature flag is used to distinguish the different types.
> > > --
Huang Rui Jan. 11, 2024, 3:25 a.m. UTC | #5
On Thu, Jan 11, 2024 at 01:49:27AM +0800, Erwan Velu wrote:
> > How about modifying here as "partial Zen2, Zen3, and future processors"?
> And all processors starting Zen4 ?

"partial Zen2 and all processors starting Zen3"

Thanks,
Ray

> 
> Le mer. 10 janv. 2024 à 18:19, Huang Rui <ray.huang@amd.com> a écrit :
> >
> > On Wed, Jan 10, 2024 at 10:21:20PM +0800, Rafael J. Wysocki wrote:
> > > On Tue, Jan 9, 2024 at 6:58 PM Erwan Velu <erwanaliasr1@gmail.com> wrote:
> > > >
> > > > amd-pstate is implemented on Zen4, adding this architecture in the
> > > > introduction.
> > > >
> > > > Signed-off-by: Erwan Velu <e.velu@criteo.com>
> > > > ---
> > > >  drivers/cpufreq/amd-pstate.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> > > > index 1f6186475715..9a8d083f6ba5 100644
> > > > --- a/drivers/cpufreq/amd-pstate.c
> > > > +++ b/drivers/cpufreq/amd-pstate.c
> > > > @@ -14,7 +14,7 @@
> > > >   * communicate the performance hints to hardware.
> > > >   *
> > > >   * AMD P-State is supported on recent AMD Zen base CPU series include some of
> > > > - * Zen2 and Zen3 processors. _CPC needs to be present in the ACPI tables of AMD
> > > > + * Zen2, Zen3 and Zen4 processors. _CPC needs to be present in the ACPI tables of AMD
> > >
> > > IMO, it would be really nice to avoid extending this line so much, but
> > > this is up to the AMD people.
> >
> > Agree. Actually, we will use this driver for all future AMD CPU generations
> > that starts from paritial Zen2.
> >
> > >
> > > Also, while you are at it, "include" in the first sentence should be
> > > "including" and a new paragraph after "processors." would make this
> > > comment easier to follow from the logical standpoint, because the
> > > first sentence is about the supported platforms and the second one is
> > > about additional requirements (ie. something else).
> > >
> >
> > How about modifying here as "partial Zen2, Zen3, and future processors"?
> >
> > Thanks,
> > Ray
> >
> > > >   * P-State supported system. And there are two types of hardware implementations
> > > >   * for AMD P-State: 1) Full MSR Solution and 2) Shared Memory Solution.
> > > >   * X86_FEATURE_CPPC CPU feature flag is used to distinguish the different types.
> > > > --
diff mbox series

Patch

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 1f6186475715..9a8d083f6ba5 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -14,7 +14,7 @@ 
  * communicate the performance hints to hardware.
  *
  * AMD P-State is supported on recent AMD Zen base CPU series include some of
- * Zen2 and Zen3 processors. _CPC needs to be present in the ACPI tables of AMD
+ * Zen2, Zen3 and Zen4 processors. _CPC needs to be present in the ACPI tables of AMD
  * P-State supported system. And there are two types of hardware implementations
  * for AMD P-State: 1) Full MSR Solution and 2) Shared Memory Solution.
  * X86_FEATURE_CPPC CPU feature flag is used to distinguish the different types.