RE: [PATCH v1 4/4] power/amd_uncore: uncore power management support for AMD EPYC processors

["Tummala, Sivaprasad" <Sivaprasad.Tummala@amd.com>]

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Dave,

> -----Original Message-----
> From: Hunt, David <david.hunt@intel.com>
> Sent: Tuesday, July 23, 2024 4:03 PM
> To: Tummala, Sivaprasad <Sivaprasad.Tummala@amd.com>;
> anatoly.burakov@intel.com; jerinj@marvell.com; lihuisong@huawei.com;
> david.marchand@redhat.com; Yigit, Ferruh <Ferruh.Yigit@amd.com>;
> konstantin.ananyev@huawei.com
> Cc: dev@dpdk.org
> Subject: Re: [PATCH v1 4/4] power/amd_uncore: uncore power management
> support for AMD EPYC processors
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On 20/07/2024 17:50, Sivaprasad Tummala wrote:
> > This patch introduces driver support for power management of uncore
> > components in AMD EPYC processors.
> >
> > Signed-off-by: Sivaprasad Tummala <sivaprasad.tummala@amd.com>
> > ---
> >   drivers/power/amd_uncore/amd_uncore.c | 321
> ++++++++++++++++++++++++++
> >   drivers/power/amd_uncore/amd_uncore.h | 226 ++++++++++++++++++
> >   drivers/power/amd_uncore/meson.build  |  20 ++
> >   drivers/power/meson.build             |   1 +
> >   4 files changed, 568 insertions(+)
> >   create mode 100644 drivers/power/amd_uncore/amd_uncore.c
> >   create mode 100644 drivers/power/amd_uncore/amd_uncore.h
> >   create mode 100644 drivers/power/amd_uncore/meson.build
> >
> > diff --git a/drivers/power/amd_uncore/amd_uncore.c
> > b/drivers/power/amd_uncore/amd_uncore.c
> > new file mode 100644
> > index 0000000000..f15eaaa307
> > --- /dev/null
> > +++ b/drivers/power/amd_uncore/amd_uncore.c
> > @@ -0,0 +1,321 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright(c) 2024 Advanced Micro Devices, Inc.
> > + */
> > +
> > +#include <errno.h>
> > +#include <dirent.h>
> > +#include <fnmatch.h>
> > +
> > +#include <rte_memcpy.h>
> > +
> > +#include "amd_uncore.h"
> > +#include "power_common.h"
> > +#include "e_smi/e_smi.h"
> > +
> > +#define MAX_UNCORE_FREQS 8
> > +#define MAX_NUMA_DIE 8
> > +
> > +#define BUS_FREQ 1000
> > +
> > +struct  __rte_cache_aligned uncore_power_info {
> > +     unsigned int die;                  /* Core die id */
> > +     unsigned int pkg;                  /* Package id */
> > +     uint32_t freqs[MAX_UNCORE_FREQS];  /* Frequency array */
> > +     uint32_t nb_freqs;                 /* Number of available freqs */
> > +     uint32_t curr_idx;                 /* Freq index in freqs array */
> > +     uint32_t max_freq;            /* System max uncore freq */
> > +     uint32_t min_freq;            /* System min uncore freq */
> > +};
> > +
> > +static struct uncore_power_info
> > +uncore_info[RTE_MAX_NUMA_NODES][MAX_NUMA_DIE];
> > +static int esmi_initialized;
> > +
> > +static int
> > +set_uncore_freq_internal(struct uncore_power_info *ui, uint32_t idx)
> > +{
> > +     int ret;
> > +
> > +     if (idx >= MAX_UNCORE_FREQS || idx >= ui->nb_freqs) {
> > +             POWER_LOG(DEBUG, "Invalid uncore frequency index %u, which "
> > +                             "should be less than %u", idx, ui->nb_freqs);
> > +             return -1;
> > +     }
> > +
> > +     ret = esmi_apb_disable(ui->pkg, idx);
> > +     if (ret != ESMI_SUCCESS) {
> > +             POWER_LOG(ERR, "DF P-state '%u' set failed for pkg %02u",
> > +                             idx, ui->pkg);
> > +             return -1;
> > +     }
> > +
> > +     POWER_DEBUG_LOG("DF P-state '%u' to be set for pkg %02u die %02u",
> > +                             idx, ui->pkg, ui->die);
> > +
> > +     /* write the minimum value first if the target freq is less than current max
> */
> > +     ui->curr_idx = idx;
> > +
> > +     return 0;
> > +}
> > +
> > +/*
> > + * Fopen the sys file for the future setting of the uncore die frequency.
> > + */
>
>
> Comment may need updating, as function is not reading any sysfs files (for the
> moment, at least).
ACK! Will address this in next version.

>
>
> > +static int
> > +power_init_for_setting_uncore_freq(struct uncore_power_info *ui) {
> > +     /* open and read all uncore sys files */
>
>
> Comment may need updating, as function is not reading any sysfs files (for the
> moment, at least).
ACK! Will address this in next version.
>
>
>
> > +     /* Base max */
> > +     ui->max_freq = 1800000;
> > +     ui->min_freq = 1200000;
> > +
> > +     return 0;
> > +}
> > +
> > +/*
> > + * Get the available uncore frequencies of the specific die by
> > +reading the
> > + * sys file.
> > + */
>
>
> Comment may need updating, as function is not reading any sysfs files. 3
> uncore frequencies hard-coded for the moment, may get via esmi or sysfs in
> the future.
ACK! Will address this in next version.
>
>
> > +static int
> > +power_get_available_uncore_freqs(struct uncore_power_info *ui) {
> > +     int ret = -1;
> > +     uint32_t i, num_uncore_freqs = 3;
> > +     uint32_t fabric_freqs[] = {
> > +     /* to be extended for probing support in future */
> > +             1800,
> > +             1444,
> > +             1200
> > +     };
> > +
> > +     if (num_uncore_freqs >= MAX_UNCORE_FREQS) {
> > +             POWER_LOG(ERR, "Too many available uncore frequencies: %d",
> > +                             num_uncore_freqs);
> > +             goto out;
> > +     }
> > +
> > +     /* Generate the uncore freq bucket array. */
> > +     for (i = 0; i < num_uncore_freqs; i++)
> > +             ui->freqs[i] = fabric_freqs[i] * BUS_FREQ;
> > +
> > +     ui->nb_freqs = num_uncore_freqs;
> > +
> > +     ret = 0;
> > +
> > +     POWER_DEBUG_LOG("%d frequency(s) of pkg %02u die %02u are
> available",
> > +                     num_uncore_freqs, ui->pkg, ui->die);
> > +
> > +out:
> > +     return ret;
> > +}
> > +
>
>
> --snip--
>
> >