[PATCH] Add support for Zen 2 CPUs to amd_energy driver

[PATCH] Add support for Zen 2 CPUs to amd_energy driver

[Sandeep Raghuraman]

This patch adds support for Zen 2 desktop CPUs to the amd_energy driver.

Signed-off-by: Sandeep Raghuraman <sandy.8925@gmail.com>
---
 drivers/hwmon/amd_energy.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/hwmon/amd_energy.c b/drivers/hwmon/amd_energy.c
index 0dee535e6c85..21341c8e1d6e 100644
--- a/drivers/hwmon/amd_energy.c
+++ b/drivers/hwmon/amd_energy.c
@@ -363,6 +363,7 @@ static struct platform_device *amd_energy_platdev;
 
 static const struct x86_cpu_id cpu_ids[] __initconst = {
        X86_MATCH_VENDOR_FAM_MODEL(AMD, 0x17, 0x31, NULL),
+       X86_MATCH_VENDOR_FAM_MODEL(AMD, 0x17, 0x71, NULL),
        {}
 };
 MODULE_DEVICE_TABLE(x86cpu, cpu_ids);
--

Re: [PATCH] Add support for Zen 2 CPUs to amd_energy driver

[Guenter Roeck]

On Wed, Dec 09, 2020 at 01:51:48AM +0530, Sandeep Raghuraman wrote:
> This patch adds support for Zen 2 desktop CPUs to the amd_energy driver.
> 
> Signed-off-by: Sandeep Raghuraman <sandy.8925@gmail.com>

This patch will require an Ack from Naveen; support for this model was
previously removed due to inconsistent results; see commit f28e360f2903
("hwmon: (amd_energy) match for supported models"),

Guenter

> ---
>  drivers/hwmon/amd_energy.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> --
> 
> diff --git a/drivers/hwmon/amd_energy.c b/drivers/hwmon/amd_energy.c
> index 0dee535e6c85..21341c8e1d6e 100644
> --- a/drivers/hwmon/amd_energy.c
> +++ b/drivers/hwmon/amd_energy.c
> @@ -363,6 +363,7 @@ static struct platform_device *amd_energy_platdev;
>  
>  static const struct x86_cpu_id cpu_ids[] __initconst = {
>         X86_MATCH_VENDOR_FAM_MODEL(AMD, 0x17, 0x31, NULL),
> +       X86_MATCH_VENDOR_FAM_MODEL(AMD, 0x17, 0x71, NULL),
>         {}
>  };
>  MODULE_DEVICE_TABLE(x86cpu, cpu_ids);

RE: [PATCH] Add support for Zen 2 CPUs to amd_energy driver

[Chatradhi, Naveen Krishna]

[AMD Official Use Only - Approved for External Use]

Hi Guenter,

>> This patch will require an Ack from Naveen; support for this model was previously removed due to inconsistent results; see commit f28e360f2903
("hwmon: (amd_energy) match for supported models"),
I could not get hold of a board with the mentioned processor. 

Sandeep,

Earlier, we noticed on some of the family 17h processors the core and socket energy counters are not well calibrated.

While running avx-turbo work load https://github.com/travisdowns/avx-turbo as below, we noticed that for a given time,
the sum of the energy consumed by all the cores in a socket is greater than the energy consumed by the socket itself.

Can you run the avx-turbo test, with following options and confirm the results ?

  ./avx-turbo --iters 10000000 --spec avx256_fma_t/8

This will run AVX code on all 8 cores for about 1 second. To run it for longer, increase the --iters argument.

Regards,
Naveenk

-----Original Message-----
From: Guenter Roeck <linux@roeck-us.net> 
Sent: Wednesday, December 9, 2020 2:11 AM
To: Sandeep Raghuraman <sandy.8925@gmail.com>
Cc: linux-hwmon@vger.kernel.org; Chatradhi, Naveen Krishna <NaveenKrishna.Chatradhi@amd.com>
Subject: Re: [PATCH] Add support for Zen 2 CPUs to amd_energy driver

[CAUTION: External Email]

On Wed, Dec 09, 2020 at 01:51:48AM +0530, Sandeep Raghuraman wrote:
> This patch adds support for Zen 2 desktop CPUs to the amd_energy driver.
>
> Signed-off-by: Sandeep Raghuraman <sandy.8925@gmail.com>

This patch will require an Ack from Naveen; support for this model was previously removed due to inconsistent results; see commit f28e360f2903
("hwmon: (amd_energy) match for supported models"),

Guenter

> ---
>  drivers/hwmon/amd_energy.c | 1 +
>  1 file changed, 1 insertion(+)
>
> --
>
> diff --git a/drivers/hwmon/amd_energy.c b/drivers/hwmon/amd_energy.c 
> index 0dee535e6c85..21341c8e1d6e 100644
> --- a/drivers/hwmon/amd_energy.c
> +++ b/drivers/hwmon/amd_energy.c
> @@ -363,6 +363,7 @@ static struct platform_device *amd_energy_platdev;
>
>  static const struct x86_cpu_id cpu_ids[] __initconst = {
>         X86_MATCH_VENDOR_FAM_MODEL(AMD, 0x17, 0x31, NULL),
> +       X86_MATCH_VENDOR_FAM_MODEL(AMD, 0x17, 0x71, NULL),
>         {}
>  };
>  MODULE_DEVICE_TABLE(x86cpu, cpu_ids);

Re: [PATCH] Add support for Zen 2 CPUs to amd_energy driver

[Sandeep]

On Wed, 9 Dec 2020 at 16:33, Chatradhi, Naveen Krishna
<NaveenKrishna.Chatradhi@amd.com> wrote:
>
> [AMD Official Use Only - Approved for External Use]
>
> Hi Guenter,
>
> >> This patch will require an Ack from Naveen; support for this model was previously removed due to inconsistent results; see commit f28e360f2903
> ("hwmon: (amd_energy) match for supported models"),
> I could not get hold of a board with the mentioned processor.
>
> Sandeep,
>
> Earlier, we noticed on some of the family 17h processors the core and socket energy counters are not well calibrated.
>
> While running avx-turbo work load https://github.com/travisdowns/avx-turbo as below, we noticed that for a given time,
> the sum of the energy consumed by all the cores in a socket is greater than the energy consumed by the socket itself.
>
> Can you run the avx-turbo test, with following options and confirm the results ?
>
>   ./avx-turbo --iters 10000000 --spec avx256_fma_t/8
>
> This will run AVX code on all 8 cores for about 1 second. To run it for longer, increase the --iters argument.
>
> Regards,
> Naveenk

Sure, I'll run that test. I have a 3900X though, which is 12 core, so
do I have to adjust anything for that CPU, or just run with the
parameters you've specified?

- Sandeep

Re: [PATCH] Add support for Zen 2 CPUs to amd_energy driver

[Sandeep]

On Wed, 9 Dec 2020 at 20:23, Sandeep <sandy.8925@gmail.com> wrote:
>
> On Wed, 9 Dec 2020 at 16:33, Chatradhi, Naveen Krishna
> <NaveenKrishna.Chatradhi@amd.com> wrote:
> >
> > [AMD Official Use Only - Approved for External Use]
> >
> > Hi Guenter,
> >
> > >> This patch will require an Ack from Naveen; support for this model was previously removed due to inconsistent results; see commit f28e360f2903
> > ("hwmon: (amd_energy) match for supported models"),
> > I could not get hold of a board with the mentioned processor.
> >
> > Sandeep,
> >
> > Earlier, we noticed on some of the family 17h processors the core and socket energy counters are not well calibrated.
> >
> > While running avx-turbo work load https://github.com/travisdowns/avx-turbo as below, we noticed that for a given time,
> > the sum of the energy consumed by all the cores in a socket is greater than the energy consumed by the socket itself.
> >
> > Can you run the avx-turbo test, with following options and confirm the results ?
> >
> >   ./avx-turbo --iters 10000000 --spec avx256_fma_t/8
> >
> > This will run AVX code on all 8 cores for about 1 second. To run it for longer, increase the --iters argument.
> >
> > Regards,
> > Naveenk
>
> Sure, I'll run that test. I have a 3900X though, which is 12 core, so
> do I have to adjust anything for that CPU, or just run with the
> parameters you've specified?
>
> - Sandeep

Ok, I ran the test for both 8 cores and 12 cores - in both cases, I
can confirm that the sum of the energy consumed by all the cores in a
socket, is NOT greater than the energy consumed by the socket itself.

- Sandeep

RE: [PATCH] Add support for Zen 2 CPUs to amd_energy driver

[Chatradhi, Naveen Krishna]

[AMD Official Use Only - Approved for External Use]

Hi Sandeep,

>> Sure, I'll run that test. I have a 3900X though, which is 12 core, so do I have to adjust anything for that CPU, or just run with the parameters you've specified?

I'm trying to get hold of a system to test it myself. In the meanwhile, you may run the following command
in the back ground and read the counters (cores and socket) at 5 to 10 secs apart and
sum of the diff of core counters should be not be > diff of socket counters

sudo ./avx-turbo --iters 1000000000 --spec avx256_fma_t/12

You may refer https://www.spinics.net/lists/linux-hwmon/msg08255.html link for more background of the testing.

Regards,
Naveenk

-----Original Message-----
From: Sandeep <sandy.8925@gmail.com> 
Sent: Wednesday, December 9, 2020 8:24 PM
To: Chatradhi, Naveen Krishna <NaveenKrishna.Chatradhi@amd.com>
Cc: Guenter Roeck <linux@roeck-us.net>; linux-hwmon@vger.kernel.org
Subject: Re: [PATCH] Add support for Zen 2 CPUs to amd_energy driver

[CAUTION: External Email]

On Wed, 9 Dec 2020 at 16:33, Chatradhi, Naveen Krishna <NaveenKrishna.Chatradhi@amd.com> wrote:
>
> [AMD Official Use Only - Approved for External Use]
>
> Hi Guenter,
>
> >> This patch will require an Ack from Naveen; support for this model 
> >> was previously removed due to inconsistent results; see commit 
> >> f28e360f2903
> ("hwmon: (amd_energy) match for supported models"), I could not get 
> hold of a board with the mentioned processor.
>
> Sandeep,
>
> Earlier, we noticed on some of the family 17h processors the core and socket energy counters are not well calibrated.
>
> While running avx-turbo work load 
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftravisdowns%2Favx-turbo&amp;data=04%7C01%7CNaveenKrishna.Chatradhi%40amd.com%7C8cd70f66176547ea9d8208d89c525af5%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637431224816944682%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=mJmMnL3Y4Wwi%2BLCqZmn9AjxGeIKdxgoLrjz%2FqASnl68%3D&amp;reserved=0 as below, we noticed that for a given time, the sum of the energy consumed by all the cores in a socket is greater than the energy consumed by the socket itself.
>
> Can you run the avx-turbo test, with following options and confirm the results ?
>
>   ./avx-turbo --iters 10000000 --spec avx256_fma_t/8
>
> This will run AVX code on all 8 cores for about 1 second. To run it for longer, increase the --iters argument.
>
> Regards,
> Naveenk

Sure, I'll run that test. I have a 3900X though, which is 12 core, so do I have to adjust anything for that CPU, or just run with the parameters you've specified?

- Sandeep

Re: [PATCH] Add support for Zen 2 CPUs to amd_energy driver

[Sandeep]

On Wed, 9 Dec 2020 at 20:58, Chatradhi, Naveen Krishna
<NaveenKrishna.Chatradhi@amd.com> wrote:
>
> [AMD Official Use Only - Approved for External Use]
>
> Hi Sandeep,
>
> >> Sure, I'll run that test. I have a 3900X though, which is 12 core, so do I have to adjust anything for that CPU, or just run with the parameters you've specified?
>
> I'm trying to get hold of a system to test it myself. In the meanwhile, you may run the following command
> in the back ground and read the counters (cores and socket) at 5 to 10 secs apart and
> sum of the diff of core counters should be not be > diff of socket counters
>
> sudo ./avx-turbo --iters 1000000000 --spec avx256_fma_t/12
>
> You may refer https://www.spinics.net/lists/linux-hwmon/msg08255.html link for more background of the testing.
>
> Regards,
> Naveenk
>

Do you already have a script or something that I can run to
automatically gather this information (i.e check values every 5
seconds, calculate differences and check if the results line up)?
Doing this manually, sounds like a lot of work that I really don't
want to get into.

- Sandeep

Re: [PATCH] Add support for Zen 2 CPUs to amd_energy driver

[Sandeep]

On Thu, 10 Dec 2020 at 01:35, Sandeep <sandy.8925@gmail.com> wrote:
>
> On Wed, 9 Dec 2020 at 20:58, Chatradhi, Naveen Krishna
> <NaveenKrishna.Chatradhi@amd.com> wrote:
> >
> > [AMD Official Use Only - Approved for External Use]
> >
> > Hi Sandeep,
> >
> > >> Sure, I'll run that test. I have a 3900X though, which is 12 core, so do I have to adjust anything for that CPU, or just run with the parameters you've specified?
> >
> > I'm trying to get hold of a system to test it myself. In the meanwhile, you may run the following command
> > in the back ground and read the counters (cores and socket) at 5 to 10 secs apart and
> > sum of the diff of core counters should be not be > diff of socket counters
> >
> > sudo ./avx-turbo --iters 1000000000 --spec avx256_fma_t/12
> >
> > You may refer https://www.spinics.net/lists/linux-hwmon/msg08255.html link for more background of the testing.
> >
> > Regards,
> > Naveenk
> >
>
> Do you already have a script or something that I can run to
> automatically gather this information (i.e check values every 5
> seconds, calculate differences and check if the results line up)?
> Doing this manually, sounds like a lot of work that I really don't
> want to get into.
>
> - Sandeep

Ok, I was able to write a small program to automate the test, and it
looks like the following condition "sum of the diff of core counters
> diff of socket counters" is indeed true. Looks like the values are
indeed incorrect sometimes.
- Sandeep

Re: [PATCH] Add support for Zen 2 CPUs to amd_energy driver

[Sandeep]

On Thu, 10 Dec 2020 at 04:43, Sandeep <sandy.8925@gmail.com> wrote:
>
> On Thu, 10 Dec 2020 at 01:35, Sandeep <sandy.8925@gmail.com> wrote:
> >
> > On Wed, 9 Dec 2020 at 20:58, Chatradhi, Naveen Krishna
> > <NaveenKrishna.Chatradhi@amd.com> wrote:
> > >
> > > [AMD Official Use Only - Approved for External Use]
> > >
> > > Hi Sandeep,
> > >
> > > >> Sure, I'll run that test. I have a 3900X though, which is 12 core, so do I have to adjust anything for that CPU, or just run with the parameters you've specified?
> > >
> > > I'm trying to get hold of a system to test it myself. In the meanwhile, you may run the following command
> > > in the back ground and read the counters (cores and socket) at 5 to 10 secs apart and
> > > sum of the diff of core counters should be not be > diff of socket counters
> > >
> > > sudo ./avx-turbo --iters 1000000000 --spec avx256_fma_t/12
> > >
> > > You may refer https://www.spinics.net/lists/linux-hwmon/msg08255.html link for more background of the testing.
> > >
> > > Regards,
> > > Naveenk
> > >
> >
> > Do you already have a script or something that I can run to
> > automatically gather this information (i.e check values every 5
> > seconds, calculate differences and check if the results line up)?
> > Doing this manually, sounds like a lot of work that I really don't
> > want to get into.
> >
> > - Sandeep
>
> Ok, I was able to write a small program to automate the test, and it
> looks like the following condition "sum of the diff of core counters
> > diff of socket counters" is indeed true. Looks like the values are
> indeed incorrect sometimes.
> - Sandeep

I've uploaded the test program I wrote here -
https://github.com/sandy-8925/amd-energy-tester - in case it's useful
for anyone else. Let me know if you spot any bugs in that code.

- Sandeep