DPDK patches and discussions
 help / color / mirror / Atom feed
* [RFC PATCH] power: refactor uncore power management interfaces
@ 2023-05-03 17:03 Sivaprasad Tummala
  2023-05-17 11:42 ` Tummala, Sivaprasad
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Sivaprasad Tummala @ 2023-05-03 17:03 UTC (permalink / raw)
  To: david.hunt; +Cc: dev, anatoly.burakov, ferruh.yigit, Sivaprasad Tummala

From: Sivaprasad Tummala <Sivaprasad.Tummala@amd.com>

currently the uncore power management implementation is vendor specific.
Added new vendor agnostic uncore power interface similar to rte_power
and subsequently will rename specific implementations
("rte_power_intel_uncore") to "power_intel_uncore" along with functions.

Signed-off-by: Sivaprasad Tummala <Sivaprasad.Tummala@amd.com>
---
 lib/power/rte_power_uncore.h | 234 +++++++++++++++++++++++++++++++++++
 1 file changed, 234 insertions(+)
 create mode 100644 lib/power/rte_power_uncore.h

diff --git a/lib/power/rte_power_uncore.h b/lib/power/rte_power_uncore.h
new file mode 100644
index 0000000000..196fb9ec01
--- /dev/null
+++ b/lib/power/rte_power_uncore.h
@@ -0,0 +1,234 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2022 Intel Corporation
+ * Copyright(c) 2023 AMD Corporation
+ */
+
+#ifndef RTE_POWER_UNCORE_H
+#define RTE_POWER_UNCORE_H
+
+/**
+ * @file
+ * RTE Uncore Frequency Management
+ */
+
+#include <rte_compat.h>
+#include "rte_power.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/* Uncore Power Management Environment */
+enum uncore_power_mgmt_env { UNCORE_PM_ENV_NOT_SET,
+		UNCORE_PM_ENV_INTEL_UNCORE, UNCORE_PM_ENV_AMD_HSMP};
+
+/**
+ * Initialize uncore frequency management for specific die on a package.
+ * It will get the available frequencies and prepare to set new die frequencies.
+ *
+ * This function should NOT be called in the fast path.
+ *
+ * @param pkg
+ *  Package number.
+ *  Each physical CPU in a system is referred to as a package.
+ * @param die
+ *  Die number.
+ *  Each package can have several dies connected together via the uncore mesh.
+ *
+ * @return
+ *  - 0 on success.
+ *  - Negative on error.
+ */
+__rte_experimental
+int
+rte_power_uncore_init(unsigned int pkg, unsigned int die);
+
+/**
+ * Exit uncore frequency management on a specific die on a package.
+ * It will restore uncore min and* max values to previous values
+ * before initialization of API.
+ *
+ * This function should NOT be called in the fast path.
+ *
+ * @param pkg
+ *  Package number.
+ *  Each physical CPU in a system is referred to as a package.
+ * @param die
+ *  Die number.
+ *  Each package can have several dies connected together via the uncore mesh.
+ *
+ * @return
+ *  - 0 on success.
+ *  - Negative on error.
+ */
+__rte_experimental
+int
+rte_power_uncore_exit(unsigned int pkg, unsigned int die);
+
+/**
+ * Return the current index of available frequencies of a specific die on a package.
+ * It should be protected outside of this function for threadsafe.
+ *
+ * This function should NOT be called in the fast path.
+ *
+ * @param pkg
+ *  Package number.
+ *  Each physical CPU in a system is referred to as a package.
+ * @param die
+ *  Die number.
+ *  Each package can have several dies connected together via the uncore mesh.
+ *
+ * @return
+ *  The current index of available frequencies.
+ *  If error, it will return 'RTE_POWER_INVALID_FREQ_INDEX = (~0)'.
+ */
+typedef uint32_t (*rte_power_get_uncore_freq_t)(unsigned int pkg, unsigned int die);
+
+extern rte_power_get_uncore_freq_t rte_power_get_uncore_freq;
+
+/**
+ * Set minimum and maximum uncore frequency for specified die on a package
+ * to specified index value.
+ * It should be protected outside of this function for threadsafe.
+ *
+ * This function should NOT be called in the fast path.
+ *
+ * @param pkg
+ *  Package number.
+ *  Each physical CPU in a system is referred to as a package.
+ * @param die
+ *  Die number.
+ *  Each package can have several dies connected together via the uncore mesh.
+ * @param index
+ *  The index of available frequencies.
+ *
+ * @return
+ *  - 1 on success with frequency changed.
+ *  - 0 on success without frequency changed.
+ *  - Negative on error.
+ */
+typedef int (*rte_power_set_uncore_freq_t)(unsigned int pkg, unsigned int die, uint32_t index);
+
+extern rte_power_set_uncore_freq_t rte_power_set_uncore_freq;
+
+/**
+ * Function pointer definition for generic frequency change functions.
+ *
+ * @param pkg
+ *  Package number.
+ *  Each physical CPU in a system is referred to as a package.
+ * @param die
+ *  Die number.
+ *  Each package can have several dies connected together via the uncore mesh.
+ *
+ * @return
+ *  - 1 on success with frequency changed.
+ *  - 0 on success without frequency changed.
+ *  - Negative on error.
+ */
+typedef int (*rte_power_uncore_freq_change_t)(unsigned int pkg, unsigned int die);
+
+/**
+ * Set minimum and maximum uncore frequency for specified die on a package
+ * to maximum value according to the available frequencies.
+ * It should be protected outside of this function for threadsafe.
+ *
+ * This function should NOT be called in the fast path.
+ *
+ * @param pkg
+ *  Package number.
+ *  Each physical CPU in a system is referred to as a package.
+ * @param die
+ *  Die number.
+ *  Each package can have several dies connected together via the uncore mesh.
+ *
+ * @return
+ *  - 1 on success with frequency changed.
+ *  - 0 on success without frequency changed.
+ *  - Negative on error.
+ */
+extern rte_power_uncore_freq_change_t rte_power_uncore_freq_max;
+
+/**
+ * Set minimum and maximum uncore frequency for specified die on a package
+ * to minimum value according to the available frequencies.
+ * It should be protected outside of this function for threadsafe.
+ *
+ * This function should NOT be called in the fast path.
+ *
+ * @param pkg
+ *  Package number.
+ *  Each physical CPU in a system is referred to as a package.
+ * @param die
+ *  Die number.
+ *  Each package can have several dies connected together via the uncore mesh.
+ *
+ * @return
+ *  - 1 on success with frequency changed.
+ *  - 0 on success without frequency changed.
+ *  - Negative on error.
+ */
+extern rte_power_uncore_freq_change_t rte_power_uncore_freq_min;
+
+/**
+ * Return the list length of available frequencies in the index array.
+ *
+ * This function should NOT be called in the fast path.
+ *
+ * @param pkg
+ *  Package number.
+ *  Each physical CPU in a system is referred to as a package.
+ * @param die
+ *  Die number.
+ *  Each package can have several dies connected together via the uncore mesh.
+ * @param freqs
+ *  The buffer array to save the frequencies.
+ * @param num
+ *  The number of frequencies to get.
+ *
+ * @return
+ *  - The number of available index's in frequency array.
+ *  - Negative on error.
+ */
+typedef int (*rte_power_uncore_freqs_t)(unsigned int pkg, unsigned int die,
+		uint32_t *freqs, uint32_t num);
+
+extern rte_power_uncore_freqs_t rte_power_uncore_freqs;
+
+/**
+ * Return the number of packages (CPUs) on a system
+ * by parsing the uncore sysfs directory.
+ *
+ * This function should NOT be called in the fast path.
+ *
+ * @return
+ *  - Zero on error.
+ *  - Number of package on system on success.
+ */
+typedef unsigned int (*rte_power_uncore_get_num_pkgs_t)(void);
+
+extern rte_power_uncore_get_num_pkgs_t rte_power_uncore_get_num_pkgs;
+
+/**
+ * Return the number of dies for pakckages (CPUs) specified
+ * from parsing the uncore sysfs directory.
+ *
+ * This function should NOT be called in the fast path.
+ *
+ * @param pkg
+ *  Package number.
+ *  Each physical CPU in a system is referred to as a package.
+ *
+ * @return
+ *  - Zero on error.
+ *  - Number of dies for package on sucecss.
+ */
+typedef unsigned int (*rte_power_uncore_get_num_pkgs_t)(unsigned int pkg);
+
+extern rte_power_uncore_get_num_dies_t rte_power_uncore_get_num_dies;
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* RTE_POWER_UNCORE_H */
-- 
2.34.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [RFC PATCH] power: refactor uncore power management interfaces
  2023-05-03 17:03 [RFC PATCH] power: refactor uncore power management interfaces Sivaprasad Tummala
@ 2023-05-17 11:42 ` Tummala, Sivaprasad
  2023-05-19  9:42 ` Burakov, Anatoly
  2023-08-11 10:25 ` [PATCH v1 1/2] " Sivaprasad Tummala
  2 siblings, 0 replies; 10+ messages in thread
From: Tummala, Sivaprasad @ 2023-05-17 11:42 UTC (permalink / raw)
  To: david.hunt, anatoly.burakov; +Cc: dev, Yigit, Ferruh

[AMD Official Use Only - General]

Hi Dave, Anatoly,

We are planning to implement these changes in 23.07 release. 
Would like to hear your comments and any objections.
Can you please review this and share your thoughts. 

Thanks & Regards,
Sivaprasad

> -----Original Message-----
> From: Tummala, Sivaprasad <Sivaprasad.Tummala@amd.com>
> Sent: Wednesday, May 3, 2023 10:34 PM
> To: david.hunt@intel.com
> Cc: dev@dpdk.org; anatoly.burakov@intel.com; Yigit, Ferruh
> <Ferruh.Yigit@amd.com>; Tummala, Sivaprasad
> <Sivaprasad.Tummala@amd.com>
> Subject: [RFC PATCH] power: refactor uncore power management interfaces
> 
> From: Sivaprasad Tummala <Sivaprasad.Tummala@amd.com>
> 
> currently the uncore power management implementation is vendor specific.
> Added new vendor agnostic uncore power interface similar to rte_power and
> subsequently will rename specific implementations
> ("rte_power_intel_uncore") to "power_intel_uncore" along with functions.
> 
> Signed-off-by: Sivaprasad Tummala <Sivaprasad.Tummala@amd.com>
> ---
>  lib/power/rte_power_uncore.h | 234
> +++++++++++++++++++++++++++++++++++
>  1 file changed, 234 insertions(+)
>  create mode 100644 lib/power/rte_power_uncore.h
> 
> diff --git a/lib/power/rte_power_uncore.h b/lib/power/rte_power_uncore.h new file
> mode 100644 index 0000000000..196fb9ec01
> --- /dev/null
> +++ b/lib/power/rte_power_uncore.h
> @@ -0,0 +1,234 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2022 Intel Corporation
> + * Copyright(c) 2023 AMD Corporation
> + */
> +
> +#ifndef RTE_POWER_UNCORE_H
> +#define RTE_POWER_UNCORE_H
> +
> +/**
> + * @file
> + * RTE Uncore Frequency Management
> + */
> +
> +#include <rte_compat.h>
> +#include "rte_power.h"
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +/* Uncore Power Management Environment */ enum uncore_power_mgmt_env
> {
> +UNCORE_PM_ENV_NOT_SET,
> +		UNCORE_PM_ENV_INTEL_UNCORE, UNCORE_PM_ENV_AMD_HSMP};
> +
> +/**
> + * Initialize uncore frequency management for specific die on a package.
> + * It will get the available frequencies and prepare to set new die frequencies.
> + *
> + * This function should NOT be called in the fast path.
> + *
> + * @param pkg
> + *  Package number.
> + *  Each physical CPU in a system is referred to as a package.
> + * @param die
> + *  Die number.
> + *  Each package can have several dies connected together via the uncore mesh.
> + *
> + * @return
> + *  - 0 on success.
> + *  - Negative on error.
> + */
> +__rte_experimental
> +int
> +rte_power_uncore_init(unsigned int pkg, unsigned int die);
> +
> +/**
> + * Exit uncore frequency management on a specific die on a package.
> + * It will restore uncore min and* max values to previous values
> + * before initialization of API.
> + *
> + * This function should NOT be called in the fast path.
> + *
> + * @param pkg
> + *  Package number.
> + *  Each physical CPU in a system is referred to as a package.
> + * @param die
> + *  Die number.
> + *  Each package can have several dies connected together via the uncore mesh.
> + *
> + * @return
> + *  - 0 on success.
> + *  - Negative on error.
> + */
> +__rte_experimental
> +int
> +rte_power_uncore_exit(unsigned int pkg, unsigned int die);
> +
> +/**
> + * Return the current index of available frequencies of a specific die on a package.
> + * It should be protected outside of this function for threadsafe.
> + *
> + * This function should NOT be called in the fast path.
> + *
> + * @param pkg
> + *  Package number.
> + *  Each physical CPU in a system is referred to as a package.
> + * @param die
> + *  Die number.
> + *  Each package can have several dies connected together via the uncore mesh.
> + *
> + * @return
> + *  The current index of available frequencies.
> + *  If error, it will return 'RTE_POWER_INVALID_FREQ_INDEX = (~0)'.
> + */
> +typedef uint32_t (*rte_power_get_uncore_freq_t)(unsigned int pkg,
> +unsigned int die);
> +
> +extern rte_power_get_uncore_freq_t rte_power_get_uncore_freq;
> +
> +/**
> + * Set minimum and maximum uncore frequency for specified die on a
> +package
> + * to specified index value.
> + * It should be protected outside of this function for threadsafe.
> + *
> + * This function should NOT be called in the fast path.
> + *
> + * @param pkg
> + *  Package number.
> + *  Each physical CPU in a system is referred to as a package.
> + * @param die
> + *  Die number.
> + *  Each package can have several dies connected together via the uncore mesh.
> + * @param index
> + *  The index of available frequencies.
> + *
> + * @return
> + *  - 1 on success with frequency changed.
> + *  - 0 on success without frequency changed.
> + *  - Negative on error.
> + */
> +typedef int (*rte_power_set_uncore_freq_t)(unsigned int pkg, unsigned
> +int die, uint32_t index);
> +
> +extern rte_power_set_uncore_freq_t rte_power_set_uncore_freq;
> +
> +/**
> + * Function pointer definition for generic frequency change functions.
> + *
> + * @param pkg
> + *  Package number.
> + *  Each physical CPU in a system is referred to as a package.
> + * @param die
> + *  Die number.
> + *  Each package can have several dies connected together via the uncore mesh.
> + *
> + * @return
> + *  - 1 on success with frequency changed.
> + *  - 0 on success without frequency changed.
> + *  - Negative on error.
> + */
> +typedef int (*rte_power_uncore_freq_change_t)(unsigned int pkg,
> +unsigned int die);
> +
> +/**
> + * Set minimum and maximum uncore frequency for specified die on a
> +package
> + * to maximum value according to the available frequencies.
> + * It should be protected outside of this function for threadsafe.
> + *
> + * This function should NOT be called in the fast path.
> + *
> + * @param pkg
> + *  Package number.
> + *  Each physical CPU in a system is referred to as a package.
> + * @param die
> + *  Die number.
> + *  Each package can have several dies connected together via the uncore mesh.
> + *
> + * @return
> + *  - 1 on success with frequency changed.
> + *  - 0 on success without frequency changed.
> + *  - Negative on error.
> + */
> +extern rte_power_uncore_freq_change_t rte_power_uncore_freq_max;
> +
> +/**
> + * Set minimum and maximum uncore frequency for specified die on a
> +package
> + * to minimum value according to the available frequencies.
> + * It should be protected outside of this function for threadsafe.
> + *
> + * This function should NOT be called in the fast path.
> + *
> + * @param pkg
> + *  Package number.
> + *  Each physical CPU in a system is referred to as a package.
> + * @param die
> + *  Die number.
> + *  Each package can have several dies connected together via the uncore mesh.
> + *
> + * @return
> + *  - 1 on success with frequency changed.
> + *  - 0 on success without frequency changed.
> + *  - Negative on error.
> + */
> +extern rte_power_uncore_freq_change_t rte_power_uncore_freq_min;
> +
> +/**
> + * Return the list length of available frequencies in the index array.
> + *
> + * This function should NOT be called in the fast path.
> + *
> + * @param pkg
> + *  Package number.
> + *  Each physical CPU in a system is referred to as a package.
> + * @param die
> + *  Die number.
> + *  Each package can have several dies connected together via the uncore mesh.
> + * @param freqs
> + *  The buffer array to save the frequencies.
> + * @param num
> + *  The number of frequencies to get.
> + *
> + * @return
> + *  - The number of available index's in frequency array.
> + *  - Negative on error.
> + */
> +typedef int (*rte_power_uncore_freqs_t)(unsigned int pkg, unsigned int die,
> +		uint32_t *freqs, uint32_t num);
> +
> +extern rte_power_uncore_freqs_t rte_power_uncore_freqs;
> +
> +/**
> + * Return the number of packages (CPUs) on a system
> + * by parsing the uncore sysfs directory.
> + *
> + * This function should NOT be called in the fast path.
> + *
> + * @return
> + *  - Zero on error.
> + *  - Number of package on system on success.
> + */
> +typedef unsigned int (*rte_power_uncore_get_num_pkgs_t)(void);
> +
> +extern rte_power_uncore_get_num_pkgs_t rte_power_uncore_get_num_pkgs;
> +
> +/**
> + * Return the number of dies for pakckages (CPUs) specified
> + * from parsing the uncore sysfs directory.
> + *
> + * This function should NOT be called in the fast path.
> + *
> + * @param pkg
> + *  Package number.
> + *  Each physical CPU in a system is referred to as a package.
> + *
> + * @return
> + *  - Zero on error.
> + *  - Number of dies for package on sucecss.
> + */
> +typedef unsigned int (*rte_power_uncore_get_num_pkgs_t)(unsigned int
> +pkg);
> +
> +extern rte_power_uncore_get_num_dies_t rte_power_uncore_get_num_dies;
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* RTE_POWER_UNCORE_H */
> --
> 2.34.1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH] power: refactor uncore power management interfaces
  2023-05-03 17:03 [RFC PATCH] power: refactor uncore power management interfaces Sivaprasad Tummala
  2023-05-17 11:42 ` Tummala, Sivaprasad
@ 2023-05-19  9:42 ` Burakov, Anatoly
  2023-08-11 10:25 ` [PATCH v1 1/2] " Sivaprasad Tummala
  2 siblings, 0 replies; 10+ messages in thread
From: Burakov, Anatoly @ 2023-05-19  9:42 UTC (permalink / raw)
  To: Sivaprasad Tummala, david.hunt; +Cc: dev, ferruh.yigit

On 5/3/2023 6:03 PM, Sivaprasad Tummala wrote:
> From: Sivaprasad Tummala <Sivaprasad.Tummala@amd.com>
> 
> currently the uncore power management implementation is vendor specific.
> Added new vendor agnostic uncore power interface similar to rte_power
> and subsequently will rename specific implementations
> ("rte_power_intel_uncore") to "power_intel_uncore" along with functions.
> 
> Signed-off-by: Sivaprasad Tummala <Sivaprasad.Tummala@amd.com>
> ---

Hi,

This looks a perfectly fine refactor, so please go ahead with v1 :)

-- 
Thanks,
Anatoly


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v1 1/2] power: refactor uncore power management interfaces
  2023-05-03 17:03 [RFC PATCH] power: refactor uncore power management interfaces Sivaprasad Tummala
  2023-05-17 11:42 ` Tummala, Sivaprasad
  2023-05-19  9:42 ` Burakov, Anatoly
@ 2023-08-11 10:25 ` Sivaprasad Tummala
  2023-08-11 10:25   ` [PATCH v1 2/2] power: refactor uncore power management implementation Sivaprasad Tummala
  2023-08-16 10:09   ` [PATCH v2 1/2] power: refactor uncore power management interfaces Sivaprasad Tummala
  2 siblings, 2 replies; 10+ messages in thread
From: Sivaprasad Tummala @ 2023-08-11 10:25 UTC (permalink / raw)
  To: david.hunt, anatoly.burakov, ferruh.yigit; +Cc: dev

currently the uncore power management implementation is vendor specific.
Added new vendor agnostic uncore power interface similar to rte_power
and rename specific implementations ("rte_power_intel_uncore") to
"power_intel_uncore" along with functions.

Signed-off-by: Sivaprasad Tummala <sivaprasad.tummala@amd.com>
---
 lib/power/rte_power_uncore.h | 254 +++++++++++++++++++++++++++++++++++
 1 file changed, 254 insertions(+)
 create mode 100644 lib/power/rte_power_uncore.h

diff --git a/lib/power/rte_power_uncore.h b/lib/power/rte_power_uncore.h
new file mode 100644
index 0000000000..e27f483eae
--- /dev/null
+++ b/lib/power/rte_power_uncore.h
@@ -0,0 +1,254 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2022 Intel Corporation
+ * Copyright(c) 2023 AMD Corporation
+ */
+
+#ifndef RTE_POWER_UNCORE_H
+#define RTE_POWER_UNCORE_H
+
+/**
+ * @file
+ * RTE Uncore Frequency Management
+ */
+
+#include <rte_compat.h>
+#include "rte_power.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/* Uncore Power Management Environment */
+enum uncore_power_mgmt_env { UNCORE_PM_ENV_NOT_SET,
+		UNCORE_PM_ENV_INTEL_UNCORE, UNCORE_PM_ENV_AMD_HSMP};
+
+/**
+ * Initialize uncore frequency management for specific die on a package.
+ * It will get the available frequencies and prepare to set new die frequencies.
+ *
+ * This function should NOT be called in the fast path.
+ *
+ * @param pkg
+ *  Package number.
+ *  Each physical CPU in a system is referred to as a package.
+ * @param die
+ *  Die number.
+ *  Each package can have several dies connected together via the uncore mesh.
+ *
+ * @return
+ *  - 0 on success.
+ *  - Negative on error.
+ */
+__rte_experimental
+int
+rte_power_uncore_init(unsigned int pkg, unsigned int die);
+
+/**
+ * Exit uncore frequency management on a specific die on a package.
+ * It will restore uncore min and* max values to previous values
+ * before initialization of API.
+ *
+ * This function should NOT be called in the fast path.
+ *
+ * @param pkg
+ *  Package number.
+ *  Each physical CPU in a system is referred to as a package.
+ * @param die
+ *  Die number.
+ *  Each package can have several dies connected together via the uncore mesh.
+ *
+ * @return
+ *  - 0 on success.
+ *  - Negative on error.
+ */
+__rte_experimental
+int
+rte_power_uncore_exit(unsigned int pkg, unsigned int die);
+
+/**
+ * Return the current index of available frequencies of a specific die on a package.
+ * It should be protected outside of this function for threadsafe.
+ *
+ * This function should NOT be called in the fast path.
+ *
+ * @param pkg
+ *  Package number.
+ *  Each physical CPU in a system is referred to as a package.
+ * @param die
+ *  Die number.
+ *  Each package can have several dies connected together via the uncore mesh.
+ *
+ * @return
+ *  The current index of available frequencies.
+ *  If error, it will return 'RTE_POWER_INVALID_FREQ_INDEX = (~0)'.
+ */
+typedef uint32_t (*rte_power_get_uncore_freq_t)(unsigned int pkg, unsigned int die);
+
+extern rte_power_get_uncore_freq_t rte_power_get_uncore_freq;
+
+/**
+ * Set minimum and maximum uncore frequency for specified die on a package
+ * to specified index value.
+ * It should be protected outside of this function for threadsafe.
+ *
+ * This function should NOT be called in the fast path.
+ *
+ * @param pkg
+ *  Package number.
+ *  Each physical CPU in a system is referred to as a package.
+ * @param die
+ *  Die number.
+ *  Each package can have several dies connected together via the uncore mesh.
+ * @param index
+ *  The index of available frequencies.
+ *
+ * @return
+ *  - 1 on success with frequency changed.
+ *  - 0 on success without frequency changed.
+ *  - Negative on error.
+ */
+typedef int (*rte_power_set_uncore_freq_t)(unsigned int pkg, unsigned int die, uint32_t index);
+
+extern rte_power_set_uncore_freq_t rte_power_set_uncore_freq;
+
+/**
+ * Function pointer definition for generic frequency change functions.
+ *
+ * @param pkg
+ *  Package number.
+ *  Each physical CPU in a system is referred to as a package.
+ * @param die
+ *  Die number.
+ *  Each package can have several dies connected together via the uncore mesh.
+ *
+ * @return
+ *  - 1 on success with frequency changed.
+ *  - 0 on success without frequency changed.
+ *  - Negative on error.
+ */
+typedef int (*rte_power_uncore_freq_change_t)(unsigned int pkg, unsigned int die);
+
+/**
+ * Set minimum and maximum uncore frequency for specified die on a package
+ * to maximum value according to the available frequencies.
+ * It should be protected outside of this function for threadsafe.
+ *
+ * This function should NOT be called in the fast path.
+ *
+ * @param pkg
+ *  Package number.
+ *  Each physical CPU in a system is referred to as a package.
+ * @param die
+ *  Die number.
+ *  Each package can have several dies connected together via the uncore mesh.
+ *
+ * @return
+ *  - 1 on success with frequency changed.
+ *  - 0 on success without frequency changed.
+ *  - Negative on error.
+ */
+extern rte_power_uncore_freq_change_t rte_power_uncore_freq_max;
+
+/**
+ * Set minimum and maximum uncore frequency for specified die on a package
+ * to minimum value according to the available frequencies.
+ * It should be protected outside of this function for threadsafe.
+ *
+ * This function should NOT be called in the fast path.
+ *
+ * @param pkg
+ *  Package number.
+ *  Each physical CPU in a system is referred to as a package.
+ * @param die
+ *  Die number.
+ *  Each package can have several dies connected together via the uncore mesh.
+ *
+ * @return
+ *  - 1 on success with frequency changed.
+ *  - 0 on success without frequency changed.
+ *  - Negative on error.
+ */
+extern rte_power_uncore_freq_change_t rte_power_uncore_freq_min;
+
+/**
+ * Return the list of available frequencies in the index array.
+ *
+ * This function should NOT be called in the fast path.
+ *
+ * @param pkg
+ *  Package number.
+ *  Each physical CPU in a system is referred to as a package.
+ * @param die
+ *  Die number.
+ *  Each package can have several dies connected together via the uncore mesh.
+ * @param freqs
+ *  The buffer array to save the frequencies.
+ * @param num
+ *  The number of frequencies to get.
+ *
+ * @return
+ *  - The number of available index's in frequency array.
+ *  - Negative on error.
+ */
+typedef int (*rte_power_uncore_freqs_t)(unsigned int pkg, unsigned int die,
+		uint32_t *freqs, uint32_t num);
+
+extern rte_power_uncore_freqs_t rte_power_uncore_freqs;
+
+/**
+ * Return the list length of available frequencies in the index array.
+ *
+ * This function should NOT be called in the fast path.
+ *
+ * @param pkg
+ *  Package number.
+ *  Each physical CPU in a system is referred to as a package.
+ * @param die
+ *  Die number.
+ *  Each package can have several dies connected together via the uncore mesh.
+ *
+ * @return
+ *  - The number of available index's in frequency array.
+ *  - Negative on error.
+ */
+typedef int (*rte_power_uncore_get_num_freqs_t)(unsigned int pkg, unsigned int die);
+
+extern rte_power_uncore_get_num_freqs_t rte_power_uncore_get_num_freqs;
+
+/**
+ * Return the number of packages (CPUs) on a system
+ * by parsing the uncore sysfs directory.
+ *
+ * This function should NOT be called in the fast path.
+ *
+ * @return
+ *  - Zero on error.
+ *  - Number of package on system on success.
+ */
+typedef unsigned int (*rte_power_uncore_get_num_pkgs_t)(void);
+
+extern rte_power_uncore_get_num_pkgs_t rte_power_uncore_get_num_pkgs;
+
+/**
+ * Return the number of dies for pakckages (CPUs) specified
+ * from parsing the uncore sysfs directory.
+ *
+ * This function should NOT be called in the fast path.
+ *
+ * @param pkg
+ *  Package number.
+ *  Each physical CPU in a system is referred to as a package.
+ *
+ * @return
+ *  - Zero on error.
+ *  - Number of dies for package on sucecss.
+ */
+typedef unsigned int (*rte_power_uncore_get_num_dies_t)(unsigned int pkg);
+
+extern rte_power_uncore_get_num_dies_t rte_power_uncore_get_num_dies;
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* RTE_POWER_UNCORE_H */
-- 
2.34.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v1 2/2] power: refactor uncore power management implementation
  2023-08-11 10:25 ` [PATCH v1 1/2] " Sivaprasad Tummala
@ 2023-08-11 10:25   ` Sivaprasad Tummala
  2023-08-16 10:09   ` [PATCH v2 1/2] power: refactor uncore power management interfaces Sivaprasad Tummala
  1 sibling, 0 replies; 10+ messages in thread
From: Sivaprasad Tummala @ 2023-08-11 10:25 UTC (permalink / raw)
  To: david.hunt, anatoly.burakov, ferruh.yigit; +Cc: dev

currently the uncore power management implementation is vendor specific.
Added new vendor agnostic uncore power management implementation similar
to rte_power and rename specific implementations ("rte_power_intel_uncore")
to "power_intel_uncore" along with functions.

Signed-off-by: Sivaprasad Tummala <sivaprasad.tummala@amd.com>
---
 app/test/meson.build                          |   2 +-
 ...wer_intel_uncore.c => test_power_uncore.c} |  43 ++++++-
 config/rte_config.h                           |   1 +
 examples/l3fwd-power/main.c                   |   2 +-
 lib/power/meson.build                         |   5 +-
 ...er_intel_uncore.c => power_intel_uncore.c} |  53 +++++---
 ...er_intel_uncore.h => power_intel_uncore.h} |  66 +++++-----
 lib/power/rte_power_uncore.c                  | 118 ++++++++++++++++++
 lib/power/version.map                         |   3 +
 9 files changed, 239 insertions(+), 54 deletions(-)
 rename app/test/{test_power_intel_uncore.c => test_power_uncore.c} (87%)
 rename lib/power/{rte_power_intel_uncore.c => power_intel_uncore.c} (90%)
 rename lib/power/{rte_power_intel_uncore.h => power_intel_uncore.h} (77%)
 create mode 100644 lib/power/rte_power_uncore.c

diff --git a/app/test/meson.build b/app/test/meson.build
index 66897c14a3..23ef7a8094 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -99,7 +99,7 @@ test_sources = files(
         'test_power.c',
         'test_power_cpufreq.c',
         'test_power_kvm_vm.c',
-        'test_power_intel_uncore.c',
+        'test_power_uncore.c',
         'test_prefetch.c',
         'test_rand_perf.c',
         'test_rawdev.c',
diff --git a/app/test/test_power_intel_uncore.c b/app/test/test_power_uncore.c
similarity index 87%
rename from app/test/test_power_intel_uncore.c
rename to app/test/test_power_uncore.c
index 31163af84e..5fe7434a4c 100644
--- a/app/test/test_power_intel_uncore.c
+++ b/app/test/test_power_uncore.c
@@ -1,5 +1,6 @@
 /* SPDX-License-Identifier: BSD-3-Clause
  * Copyright(c) 2010-2022 Intel Corporation
+ * Copyright(c) 2023 AMD Corporation
  */
 
 #include "test.h"
@@ -7,14 +8,14 @@
 #ifndef RTE_LIB_POWER
 
 static int
-test_power_intel_uncore(void)
+test_power_uncore(void)
 {
 	printf("Power management library not supported, skipping test\n");
 	return TEST_SKIPPED;
 }
 
 #else
-#include <rte_power_intel_uncore.h>
+#include <rte_power_uncore.h>
 #include <power_common.h>
 
 #define MAX_UNCORE_FREQS 32
@@ -155,6 +156,32 @@ check_power_uncore_freq_min(void)
 	return 0;
 }
 
+static int
+check_power_uncore_get_freqs(void)
+{
+	int ret;
+	uint32_t freqs[RTE_MAX_UNCORE_FREQS];
+
+	/* Successfully get uncore freq */
+	ret = rte_power_uncore_freqs(VALID_PKG, VALID_DIE, freqs, RTE_MAX_UNCORE_FREQS);
+	if (ret < 0) {
+		printf("Failed to get uncore frequencies for pkg %u die %u\n",
+							VALID_PKG, VALID_DIE);
+		return -1;
+	}
+
+	/* Unsuccessful Test */
+	ret = rte_power_uncore_freqs(INVALID_PKG, INVALID_DIE, freqs,
+							RTE_MAX_UNCORE_FREQS);
+	if (ret >= 0) {
+		printf("Unexpectedly got invalid frequencies for pkg %u die %u\n",
+							INVALID_PKG, INVALID_DIE);
+		return -1;
+	}
+
+	return 0;
+}
+
 static int
 check_power_uncore_get_num_freqs(void)
 {
@@ -172,7 +199,7 @@ check_power_uncore_get_num_freqs(void)
 	ret = rte_power_uncore_get_num_freqs(INVALID_PKG, INVALID_DIE);
 	if (ret >= 0) {
 		printf("Unexpectedly got number of invalid frequencies for pkg %u die %u\n",
-							INVALID_PKG, INVALID_DIE);
+								INVALID_PKG, INVALID_DIE);
 		return -1;
 	}
 
@@ -242,7 +269,7 @@ check_power_uncore_exit(void)
 }
 
 static int
-test_power_intel_uncore(void)
+test_power_uncore(void)
 {
 	int ret;
 
@@ -274,7 +301,11 @@ test_power_intel_uncore(void)
 	if (ret < 0)
 		goto fail_all;
 
-	ret = check_power_uncore_get_num_freqs();
+	ret = check_power_uncore_get_freqs();
+	if (ret < 0)
+		goto fail_all;
+
+	ret = check_power_uncore_get_freqs();
 	if (ret < 0)
 		goto fail_all;
 
@@ -298,4 +329,4 @@ test_power_intel_uncore(void)
 }
 #endif
 
-REGISTER_TEST_COMMAND(power_intel_uncore_autotest, test_power_intel_uncore);
+REGISTER_TEST_COMMAND(power_uncore_autotest, test_power_uncore);
diff --git a/config/rte_config.h b/config/rte_config.h
index 400e44e3cf..ccca68b4f3 100644
--- a/config/rte_config.h
+++ b/config/rte_config.h
@@ -87,6 +87,7 @@
 
 /* rte_power defines */
 #define RTE_MAX_LCORE_FREQS 64
+#define RTE_MAX_UNCORE_FREQS 64
 
 /* rte_graph defines */
 #define RTE_GRAPH_BURST_SIZE 256
diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index 3f01cbd9e2..a34a7958f0 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -46,7 +46,7 @@
 #include <rte_metrics.h>
 #include <rte_telemetry.h>
 #include <rte_power_pmd_mgmt.h>
-#include <rte_power_intel_uncore.h>
+#include <rte_power_uncore.h>
 
 #include "perf_core.h"
 #include "main.h"
diff --git a/lib/power/meson.build b/lib/power/meson.build
index 1ce8b7c07d..9693c3ba7d 100644
--- a/lib/power/meson.build
+++ b/lib/power/meson.build
@@ -18,13 +18,14 @@ sources = files(
         'power_cppc_cpufreq.c',
         'power_kvm_vm.c',
         'power_pstate_cpufreq.c',
+        'power_intel_uncore.c',
         'rte_power.c',
-        'rte_power_intel_uncore.c',
+        'rte_power_uncore.c',
         'rte_power_pmd_mgmt.c',
 )
 headers = files(
         'rte_power.h',
-        'rte_power_intel_uncore.h',
+        'rte_power_uncore.h',
         'rte_power_pmd_mgmt.h',
         'rte_power_guest_channel.h',
 )
diff --git a/lib/power/rte_power_intel_uncore.c b/lib/power/power_intel_uncore.c
similarity index 90%
rename from lib/power/rte_power_intel_uncore.c
rename to lib/power/power_intel_uncore.c
index 3b8724385f..7c8fd7a21c 100644
--- a/lib/power/rte_power_intel_uncore.c
+++ b/lib/power/power_intel_uncore.c
@@ -8,7 +8,7 @@
 
 #include <rte_memcpy.h>
 
-#include "rte_power_intel_uncore.h"
+#include "power_intel_uncore.h"
 #include "power_common.h"
 
 #define MAX_UNCORE_FREQS 32
@@ -196,7 +196,7 @@ power_init_for_setting_uncore_freq(struct uncore_power_info *ui)
 	fclose(f_base_max);
 	/* f_min and f_max are stored, no need to close */
 
-	return 0;
+return 0;
 
 err:
 	if (f_base_min != NULL)
@@ -246,7 +246,7 @@ static int
 check_pkg_die_values(unsigned int pkg, unsigned int die)
 {
 	unsigned int max_pkgs, max_dies;
-	max_pkgs = rte_power_uncore_get_num_pkgs();
+	max_pkgs = power_intel_uncore_get_num_pkgs();
 	if (max_pkgs == 0)
 		return -1;
 	if (pkg >= max_pkgs) {
@@ -255,7 +255,7 @@ check_pkg_die_values(unsigned int pkg, unsigned int die)
 		return -1;
 	}
 
-	max_dies = rte_power_uncore_get_num_dies(pkg);
+	max_dies = power_intel_uncore_get_num_dies(pkg);
 	if (max_dies == 0)
 		return -1;
 	if (die >= max_dies) {
@@ -268,7 +268,7 @@ check_pkg_die_values(unsigned int pkg, unsigned int die)
 }
 
 int
-rte_power_uncore_init(unsigned int pkg, unsigned int die)
+power_intel_uncore_init(unsigned int pkg, unsigned int die)
 {
 	struct uncore_power_info *ui;
 
@@ -298,7 +298,7 @@ rte_power_uncore_init(unsigned int pkg, unsigned int die)
 }
 
 int
-rte_power_uncore_exit(unsigned int pkg, unsigned int die)
+power_intel_uncore_exit(unsigned int pkg, unsigned int die)
 {
 	struct uncore_power_info *ui;
 
@@ -333,7 +333,7 @@ rte_power_uncore_exit(unsigned int pkg, unsigned int die)
 }
 
 uint32_t
-rte_power_get_uncore_freq(unsigned int pkg, unsigned int die)
+power_get_intel_uncore_freq(unsigned int pkg, unsigned int die)
 {
 	int ret = check_pkg_die_values(pkg, die);
 	if (ret < 0)
@@ -343,7 +343,7 @@ rte_power_get_uncore_freq(unsigned int pkg, unsigned int die)
 }
 
 int
-rte_power_set_uncore_freq(unsigned int pkg, unsigned int die, uint32_t index)
+power_set_intel_uncore_freq(unsigned int pkg, unsigned int die, uint32_t index)
 {
 	int ret = check_pkg_die_values(pkg, die);
 	if (ret < 0)
@@ -353,7 +353,7 @@ rte_power_set_uncore_freq(unsigned int pkg, unsigned int die, uint32_t index)
 }
 
 int
-rte_power_uncore_freq_max(unsigned int pkg, unsigned int die)
+power_intel_uncore_freq_max(unsigned int pkg, unsigned int die)
 {
 	int ret = check_pkg_die_values(pkg, die);
 	if (ret < 0)
@@ -364,7 +364,7 @@ rte_power_uncore_freq_max(unsigned int pkg, unsigned int die)
 
 
 int
-rte_power_uncore_freq_min(unsigned int pkg, unsigned int die)
+power_intel_uncore_freq_min(unsigned int pkg, unsigned int die)
 {
 	int ret = check_pkg_die_values(pkg, die);
 	if (ret < 0)
@@ -376,7 +376,32 @@ rte_power_uncore_freq_min(unsigned int pkg, unsigned int die)
 }
 
 int
-rte_power_uncore_get_num_freqs(unsigned int pkg, unsigned int die)
+power_intel_uncore_freqs(unsigned int pkg, unsigned int die,
+				 uint32_t *freqs, uint32_t num)
+{
+	struct uncore_power_info *ui;
+
+	int ret = check_pkg_die_values(pkg, die);
+	if (ret < 0)
+		return -1;
+
+	if (freqs == NULL) {
+		RTE_LOG(ERR, POWER, "NULL buffer supplied\n");
+		return 0;
+	}
+
+	ui = &uncore_info[pkg][die];
+	if (num < ui->nb_freqs) {
+		RTE_LOG(ERR, POWER, "Buffer size is not enough\n");
+		return 0;
+	}
+	rte_memcpy(freqs, ui->freqs, ui->nb_freqs * sizeof(uint32_t));
+
+	return ui->nb_freqs;
+}
+
+int
+power_intel_uncore_get_num_freqs(unsigned int pkg, unsigned int die)
 {
 	int ret = check_pkg_die_values(pkg, die);
 	if (ret < 0)
@@ -386,7 +411,7 @@ rte_power_uncore_get_num_freqs(unsigned int pkg, unsigned int die)
 }
 
 unsigned int
-rte_power_uncore_get_num_pkgs(void)
+power_intel_uncore_get_num_pkgs(void)
 {
 	DIR *d;
 	struct dirent *dir;
@@ -416,14 +441,14 @@ rte_power_uncore_get_num_pkgs(void)
 }
 
 unsigned int
-rte_power_uncore_get_num_dies(unsigned int pkg)
+power_intel_uncore_get_num_dies(unsigned int pkg)
 {
 	DIR *d;
 	struct dirent *dir;
 	unsigned int count = 0, max_pkgs;
 	char filter[FILTER_LENGTH];
 
-	max_pkgs = rte_power_uncore_get_num_pkgs();
+	max_pkgs = power_intel_uncore_get_num_pkgs();
 	if (max_pkgs == 0)
 		return 0;
 	if (pkg >= max_pkgs) {
diff --git a/lib/power/rte_power_intel_uncore.h b/lib/power/power_intel_uncore.h
similarity index 77%
rename from lib/power/rte_power_intel_uncore.h
rename to lib/power/power_intel_uncore.h
index 0bd9f193a1..32d6b1f7a4 100644
--- a/lib/power/rte_power_intel_uncore.h
+++ b/lib/power/power_intel_uncore.h
@@ -2,8 +2,8 @@
  * Copyright(c) 2022 Intel Corporation
  */
 
-#ifndef RTE_POWER_INTEL_UNCORE_H
-#define RTE_POWER_INTEL_UNCORE_H
+#ifndef POWER_INTEL_UNCORE_H
+#define POWER_INTEL_UNCORE_H
 
 /**
  * @file
@@ -12,6 +12,7 @@
 
 #include <rte_compat.h>
 #include "rte_power.h"
+#include "rte_power_uncore.h"
 
 #ifdef __cplusplus
 extern "C" {
@@ -34,9 +35,7 @@ extern "C" {
  *  - 0 on success.
  *  - Negative on error.
  */
-__rte_experimental
-int
-rte_power_uncore_init(unsigned int pkg, unsigned int die);
+int power_intel_uncore_init(unsigned int pkg, unsigned int die);
 
 /**
  * Exit uncore frequency management on a specific die on a package.
@@ -56,9 +55,7 @@ rte_power_uncore_init(unsigned int pkg, unsigned int die);
  *  - 0 on success.
  *  - Negative on error.
  */
-__rte_experimental
-int
-rte_power_uncore_exit(unsigned int pkg, unsigned int die);
+int power_intel_uncore_exit(unsigned int pkg, unsigned int die);
 
 /**
  * Return the current index of available frequencies of a specific die on a package.
@@ -77,9 +74,7 @@ rte_power_uncore_exit(unsigned int pkg, unsigned int die);
  *  The current index of available frequencies.
  *  If error, it will return 'RTE_POWER_INVALID_FREQ_INDEX = (~0)'.
  */
-__rte_experimental
-uint32_t
-rte_power_get_uncore_freq(unsigned int pkg, unsigned int die);
+uint32_t power_get_intel_uncore_freq(unsigned int pkg, unsigned int die);
 
 /**
  * Set minimum and maximum uncore frequency for specified die on a package
@@ -102,9 +97,7 @@ rte_power_get_uncore_freq(unsigned int pkg, unsigned int die);
  *  - 0 on success without frequency changed.
  *  - Negative on error.
  */
-__rte_experimental
-int
-rte_power_set_uncore_freq(unsigned int pkg, unsigned int die, uint32_t index);
+int power_set_intel_uncore_freq(unsigned int pkg, unsigned int die, uint32_t index);
 
 /**
  * Set minimum and maximum uncore frequency for specified die on a package
@@ -125,9 +118,7 @@ rte_power_set_uncore_freq(unsigned int pkg, unsigned int die, uint32_t index);
  *  - 0 on success without frequency changed.
  *  - Negative on error.
  */
-__rte_experimental
-int
-rte_power_uncore_freq_max(unsigned int pkg, unsigned int die);
+int power_intel_uncore_freq_max(unsigned int pkg, unsigned int die);
 
 /**
  * Set minimum and maximum uncore frequency for specified die on a package
@@ -148,9 +139,30 @@ rte_power_uncore_freq_max(unsigned int pkg, unsigned int die);
  *  - 0 on success without frequency changed.
  *  - Negative on error.
  */
-__rte_experimental
-int
-rte_power_uncore_freq_min(unsigned int pkg, unsigned int die);
+int power_intel_uncore_freq_min(unsigned int pkg, unsigned int die);
+
+/**
+ * Return the list of available frequencies in the index array.
+ *
+ * This function should NOT be called in the fast path.
+ *
+ * @param pkg
+ *  Package number.
+ *  Each physical CPU in a system is referred to as a package.
+ * @param die
+ *  Die number.
+ *  Each package can have several dies connected together via the uncore mesh.
+ * @param freqs
+ *  The buffer array to save the frequencies.
+ * @param num
+ *  The number of frequencies to get.
+ *
+ * @return
+ *  - The number of available index's in frequency array.
+ *  - Negative on error.
+ */
+int power_intel_uncore_freqs(unsigned int pkg, unsigned int die,
+				unsigned int *freqs, unsigned int num);
 
 /**
  * Return the list length of available frequencies in the index array.
@@ -168,9 +180,7 @@ rte_power_uncore_freq_min(unsigned int pkg, unsigned int die);
  *  - The number of available index's in frequency array.
  *  - Negative on error.
  */
-__rte_experimental
-int
-rte_power_uncore_get_num_freqs(unsigned int pkg, unsigned int die);
+int power_intel_uncore_get_num_freqs(unsigned int pkg, unsigned int die);
 
 /**
  * Return the number of packages (CPUs) on a system
@@ -182,9 +192,7 @@ rte_power_uncore_get_num_freqs(unsigned int pkg, unsigned int die);
  *  - Zero on error.
  *  - Number of package on system on success.
  */
-__rte_experimental
-unsigned int
-rte_power_uncore_get_num_pkgs(void);
+unsigned int power_intel_uncore_get_num_pkgs(void);
 
 /**
  * Return the number of dies for pakckages (CPUs) specified
@@ -200,12 +208,10 @@ rte_power_uncore_get_num_pkgs(void);
  *  - Zero on error.
  *  - Number of dies for package on sucecss.
  */
-__rte_experimental
-unsigned int
-rte_power_uncore_get_num_dies(unsigned int pkg);
+unsigned int power_intel_uncore_get_num_dies(unsigned int pkg);
 
 #ifdef __cplusplus
 }
 #endif
 
-#endif /* RTE_POWER_INTEL_UNCORE_H */
+#endif /* POWER_INTEL_UNCORE_H */
diff --git a/lib/power/rte_power_uncore.c b/lib/power/rte_power_uncore.c
new file mode 100644
index 0000000000..e88477fdd2
--- /dev/null
+++ b/lib/power/rte_power_uncore.c
@@ -0,0 +1,118 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2010-2014 Intel Corporation
+ * Copyright(c) 2023 AMD Corporation
+ */
+
+#include <errno.h>
+
+#include <rte_errno.h>
+#include <rte_spinlock.h>
+
+#include "rte_power_uncore.h"
+#include "power_intel_uncore.h"
+
+enum uncore_power_mgmt_env default_uncore_env = UNCORE_PM_ENV_NOT_SET;
+
+static rte_spinlock_t global_env_cfg_lock = RTE_SPINLOCK_INITIALIZER;
+
+/* function pointers */
+rte_power_get_uncore_freq_t rte_power_get_uncore_freq;
+rte_power_set_uncore_freq_t rte_power_set_uncore_freq;
+rte_power_uncore_freq_change_t rte_power_uncore_freq_max;
+rte_power_uncore_freq_change_t rte_power_uncore_freq_min;
+rte_power_uncore_freqs_t rte_power_uncore_freqs;
+rte_power_uncore_get_num_freqs_t rte_power_uncore_get_num_freqs;
+rte_power_uncore_get_num_pkgs_t rte_power_uncore_get_num_pkgs;
+rte_power_uncore_get_num_dies_t rte_power_uncore_get_num_dies;
+
+static void
+reset_power_uncore_function_ptrs(void)
+{
+	rte_power_get_uncore_freq = NULL;
+	rte_power_set_uncore_freq = NULL;
+	rte_power_uncore_freq_max = NULL;
+	rte_power_uncore_freq_min = NULL;
+	rte_power_uncore_freqs  = NULL;
+	rte_power_uncore_get_num_freqs = NULL;
+	rte_power_uncore_get_num_pkgs = NULL;
+	rte_power_uncore_get_num_dies = NULL;
+}
+
+static int
+power_set_uncore_env(enum uncore_power_mgmt_env env)
+{
+	rte_spinlock_lock(&global_env_cfg_lock);
+
+	if (default_uncore_env != UNCORE_PM_ENV_NOT_SET) {
+		RTE_LOG(ERR, POWER, "Uncore Power Management Env already set.\n");
+		rte_spinlock_unlock(&global_env_cfg_lock);
+		return -1;
+	}
+
+	int ret = 0;
+
+	if (env == UNCORE_PM_ENV_INTEL_UNCORE) {
+		rte_power_get_uncore_freq = power_get_intel_uncore_freq;
+		rte_power_set_uncore_freq = power_set_intel_uncore_freq;
+		rte_power_uncore_freq_min  = power_intel_uncore_freq_min;
+		rte_power_uncore_freq_max  = power_intel_uncore_freq_max;
+		rte_power_uncore_freqs = power_intel_uncore_freqs;
+		rte_power_uncore_get_num_freqs = power_intel_uncore_get_num_freqs;
+		rte_power_uncore_get_num_pkgs = power_intel_uncore_get_num_pkgs;
+		rte_power_uncore_get_num_dies = power_intel_uncore_get_num_dies;
+	} else {
+		RTE_LOG(ERR, POWER, "Invalid Power Management Environment(%d) set\n",
+				env);
+		ret = -1;
+	}
+
+	if (ret == 0)
+		default_uncore_env = env;
+	else {
+		default_uncore_env = UNCORE_PM_ENV_NOT_SET;
+		reset_power_uncore_function_ptrs();
+	}
+
+	rte_spinlock_unlock(&global_env_cfg_lock);
+	return ret;
+}
+
+int
+rte_power_uncore_init(unsigned int pkg, unsigned int die)
+{
+	int ret = -1;
+
+	switch (default_uncore_env) {
+	case UNCORE_PM_ENV_INTEL_UNCORE:
+		return power_intel_uncore_init(pkg, die);
+	default:
+		RTE_LOG(INFO, POWER, "Uncore Env isn't set yet!\n");
+	}
+
+	/* Auto detect Environment */
+	RTE_LOG(INFO, POWER, "Attempting to initialise Intel Uncore power mgmt...\n");
+	ret = power_intel_uncore_init(pkg, die);
+	if (ret == 0) {
+		power_set_uncore_env(UNCORE_PM_ENV_INTEL_UNCORE);
+		goto out;
+	}
+
+	RTE_LOG(ERR, POWER, "Unable to set Power Management Environment for package "
+			"%u Die %u\n", pkg, die);
+out:
+	return ret;
+}
+
+int
+rte_power_uncore_exit(unsigned int pkg, unsigned int die)
+{
+	switch (default_uncore_env) {
+	case UNCORE_PM_ENV_INTEL_UNCORE:
+		return power_intel_uncore_exit(pkg, die);
+	default:
+		RTE_LOG(ERR, POWER, "Uncore Env has not been set,"
+			       "unable to exit gracefully\n");
+	}
+	return -1;
+
+}
diff --git a/lib/power/version.map b/lib/power/version.map
index b8b54f768e..ca002ec5b4 100644
--- a/lib/power/version.map
+++ b/lib/power/version.map
@@ -52,4 +52,7 @@ EXPERIMENTAL {
 	rte_power_uncore_get_num_freqs;
 	rte_power_uncore_get_num_pkgs;
 	rte_power_uncore_init;
+
+	# added in 23.11
+	rte_power_uncore_freqs;
 };
-- 
2.34.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/2] power: refactor uncore power management interfaces
  2023-08-11 10:25 ` [PATCH v1 1/2] " Sivaprasad Tummala
  2023-08-11 10:25   ` [PATCH v1 2/2] power: refactor uncore power management implementation Sivaprasad Tummala
@ 2023-08-16 10:09   ` Sivaprasad Tummala
  2023-08-16 10:09     ` [PATCH v2 2/2] power: refactor uncore power management implementation Sivaprasad Tummala
                       ` (2 more replies)
  1 sibling, 3 replies; 10+ messages in thread
From: Sivaprasad Tummala @ 2023-08-16 10:09 UTC (permalink / raw)
  To: david.hunt, anatoly.burakov, ferruh.yigit; +Cc: dev

currently the uncore power management implementation is vendor specific.
Added new vendor agnostic uncore power interface similar to rte_power
and rename specific implementations ("rte_power_intel_uncore") to
"power_intel_uncore" along with functions.

Signed-off-by: Sivaprasad Tummala <sivaprasad.tummala@amd.com>
---
 lib/power/rte_power_uncore.h | 254 +++++++++++++++++++++++++++++++++++
 1 file changed, 254 insertions(+)
 create mode 100644 lib/power/rte_power_uncore.h

diff --git a/lib/power/rte_power_uncore.h b/lib/power/rte_power_uncore.h
new file mode 100644
index 0000000000..e27f483eae
--- /dev/null
+++ b/lib/power/rte_power_uncore.h
@@ -0,0 +1,254 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2022 Intel Corporation
+ * Copyright(c) 2023 AMD Corporation
+ */
+
+#ifndef RTE_POWER_UNCORE_H
+#define RTE_POWER_UNCORE_H
+
+/**
+ * @file
+ * RTE Uncore Frequency Management
+ */
+
+#include <rte_compat.h>
+#include "rte_power.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/* Uncore Power Management Environment */
+enum uncore_power_mgmt_env { UNCORE_PM_ENV_NOT_SET,
+		UNCORE_PM_ENV_INTEL_UNCORE, UNCORE_PM_ENV_AMD_HSMP};
+
+/**
+ * Initialize uncore frequency management for specific die on a package.
+ * It will get the available frequencies and prepare to set new die frequencies.
+ *
+ * This function should NOT be called in the fast path.
+ *
+ * @param pkg
+ *  Package number.
+ *  Each physical CPU in a system is referred to as a package.
+ * @param die
+ *  Die number.
+ *  Each package can have several dies connected together via the uncore mesh.
+ *
+ * @return
+ *  - 0 on success.
+ *  - Negative on error.
+ */
+__rte_experimental
+int
+rte_power_uncore_init(unsigned int pkg, unsigned int die);
+
+/**
+ * Exit uncore frequency management on a specific die on a package.
+ * It will restore uncore min and* max values to previous values
+ * before initialization of API.
+ *
+ * This function should NOT be called in the fast path.
+ *
+ * @param pkg
+ *  Package number.
+ *  Each physical CPU in a system is referred to as a package.
+ * @param die
+ *  Die number.
+ *  Each package can have several dies connected together via the uncore mesh.
+ *
+ * @return
+ *  - 0 on success.
+ *  - Negative on error.
+ */
+__rte_experimental
+int
+rte_power_uncore_exit(unsigned int pkg, unsigned int die);
+
+/**
+ * Return the current index of available frequencies of a specific die on a package.
+ * It should be protected outside of this function for threadsafe.
+ *
+ * This function should NOT be called in the fast path.
+ *
+ * @param pkg
+ *  Package number.
+ *  Each physical CPU in a system is referred to as a package.
+ * @param die
+ *  Die number.
+ *  Each package can have several dies connected together via the uncore mesh.
+ *
+ * @return
+ *  The current index of available frequencies.
+ *  If error, it will return 'RTE_POWER_INVALID_FREQ_INDEX = (~0)'.
+ */
+typedef uint32_t (*rte_power_get_uncore_freq_t)(unsigned int pkg, unsigned int die);
+
+extern rte_power_get_uncore_freq_t rte_power_get_uncore_freq;
+
+/**
+ * Set minimum and maximum uncore frequency for specified die on a package
+ * to specified index value.
+ * It should be protected outside of this function for threadsafe.
+ *
+ * This function should NOT be called in the fast path.
+ *
+ * @param pkg
+ *  Package number.
+ *  Each physical CPU in a system is referred to as a package.
+ * @param die
+ *  Die number.
+ *  Each package can have several dies connected together via the uncore mesh.
+ * @param index
+ *  The index of available frequencies.
+ *
+ * @return
+ *  - 1 on success with frequency changed.
+ *  - 0 on success without frequency changed.
+ *  - Negative on error.
+ */
+typedef int (*rte_power_set_uncore_freq_t)(unsigned int pkg, unsigned int die, uint32_t index);
+
+extern rte_power_set_uncore_freq_t rte_power_set_uncore_freq;
+
+/**
+ * Function pointer definition for generic frequency change functions.
+ *
+ * @param pkg
+ *  Package number.
+ *  Each physical CPU in a system is referred to as a package.
+ * @param die
+ *  Die number.
+ *  Each package can have several dies connected together via the uncore mesh.
+ *
+ * @return
+ *  - 1 on success with frequency changed.
+ *  - 0 on success without frequency changed.
+ *  - Negative on error.
+ */
+typedef int (*rte_power_uncore_freq_change_t)(unsigned int pkg, unsigned int die);
+
+/**
+ * Set minimum and maximum uncore frequency for specified die on a package
+ * to maximum value according to the available frequencies.
+ * It should be protected outside of this function for threadsafe.
+ *
+ * This function should NOT be called in the fast path.
+ *
+ * @param pkg
+ *  Package number.
+ *  Each physical CPU in a system is referred to as a package.
+ * @param die
+ *  Die number.
+ *  Each package can have several dies connected together via the uncore mesh.
+ *
+ * @return
+ *  - 1 on success with frequency changed.
+ *  - 0 on success without frequency changed.
+ *  - Negative on error.
+ */
+extern rte_power_uncore_freq_change_t rte_power_uncore_freq_max;
+
+/**
+ * Set minimum and maximum uncore frequency for specified die on a package
+ * to minimum value according to the available frequencies.
+ * It should be protected outside of this function for threadsafe.
+ *
+ * This function should NOT be called in the fast path.
+ *
+ * @param pkg
+ *  Package number.
+ *  Each physical CPU in a system is referred to as a package.
+ * @param die
+ *  Die number.
+ *  Each package can have several dies connected together via the uncore mesh.
+ *
+ * @return
+ *  - 1 on success with frequency changed.
+ *  - 0 on success without frequency changed.
+ *  - Negative on error.
+ */
+extern rte_power_uncore_freq_change_t rte_power_uncore_freq_min;
+
+/**
+ * Return the list of available frequencies in the index array.
+ *
+ * This function should NOT be called in the fast path.
+ *
+ * @param pkg
+ *  Package number.
+ *  Each physical CPU in a system is referred to as a package.
+ * @param die
+ *  Die number.
+ *  Each package can have several dies connected together via the uncore mesh.
+ * @param freqs
+ *  The buffer array to save the frequencies.
+ * @param num
+ *  The number of frequencies to get.
+ *
+ * @return
+ *  - The number of available index's in frequency array.
+ *  - Negative on error.
+ */
+typedef int (*rte_power_uncore_freqs_t)(unsigned int pkg, unsigned int die,
+		uint32_t *freqs, uint32_t num);
+
+extern rte_power_uncore_freqs_t rte_power_uncore_freqs;
+
+/**
+ * Return the list length of available frequencies in the index array.
+ *
+ * This function should NOT be called in the fast path.
+ *
+ * @param pkg
+ *  Package number.
+ *  Each physical CPU in a system is referred to as a package.
+ * @param die
+ *  Die number.
+ *  Each package can have several dies connected together via the uncore mesh.
+ *
+ * @return
+ *  - The number of available index's in frequency array.
+ *  - Negative on error.
+ */
+typedef int (*rte_power_uncore_get_num_freqs_t)(unsigned int pkg, unsigned int die);
+
+extern rte_power_uncore_get_num_freqs_t rte_power_uncore_get_num_freqs;
+
+/**
+ * Return the number of packages (CPUs) on a system
+ * by parsing the uncore sysfs directory.
+ *
+ * This function should NOT be called in the fast path.
+ *
+ * @return
+ *  - Zero on error.
+ *  - Number of package on system on success.
+ */
+typedef unsigned int (*rte_power_uncore_get_num_pkgs_t)(void);
+
+extern rte_power_uncore_get_num_pkgs_t rte_power_uncore_get_num_pkgs;
+
+/**
+ * Return the number of dies for pakckages (CPUs) specified
+ * from parsing the uncore sysfs directory.
+ *
+ * This function should NOT be called in the fast path.
+ *
+ * @param pkg
+ *  Package number.
+ *  Each physical CPU in a system is referred to as a package.
+ *
+ * @return
+ *  - Zero on error.
+ *  - Number of dies for package on sucecss.
+ */
+typedef unsigned int (*rte_power_uncore_get_num_dies_t)(unsigned int pkg);
+
+extern rte_power_uncore_get_num_dies_t rte_power_uncore_get_num_dies;
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* RTE_POWER_UNCORE_H */
-- 
2.34.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 2/2] power: refactor uncore power management implementation
  2023-08-16 10:09   ` [PATCH v2 1/2] power: refactor uncore power management interfaces Sivaprasad Tummala
@ 2023-08-16 10:09     ` Sivaprasad Tummala
  2023-10-06  9:03     ` [PATCH v2 1/2] power: refactor uncore power management interfaces David Marchand
  2023-10-17  8:19     ` David Marchand
  2 siblings, 0 replies; 10+ messages in thread
From: Sivaprasad Tummala @ 2023-08-16 10:09 UTC (permalink / raw)
  To: david.hunt, anatoly.burakov, ferruh.yigit; +Cc: dev

currently the uncore power management implementation is vendor specific.
Added new vendor agnostic uncore power management implementation similar
to rte_power and rename specific implementations ("rte_power_intel_uncore")
to "power_intel_uncore" along with functions.

Signed-off-by: Sivaprasad Tummala <sivaprasad.tummala@amd.com>
---
 app/test/test_power_intel_uncore.c            |  10 +-
 config/rte_config.h                           |   1 +
 examples/l3fwd-power/main.c                   |   2 +-
 lib/power/meson.build                         |   5 +-
 ...er_intel_uncore.c => power_intel_uncore.c} |  51 +++--
 ...er_intel_uncore.h => power_intel_uncore.h} |  66 +++---
 lib/power/rte_power_uncore.c                  | 189 ++++++++++++++++++
 lib/power/rte_power_uncore.h                  |  32 +++
 lib/power/version.map                         |   6 +
 9 files changed, 313 insertions(+), 49 deletions(-)
 rename lib/power/{rte_power_intel_uncore.c => power_intel_uncore.c} (90%)
 rename lib/power/{rte_power_intel_uncore.h => power_intel_uncore.h} (77%)
 create mode 100644 lib/power/rte_power_uncore.c

diff --git a/app/test/test_power_intel_uncore.c b/app/test/test_power_intel_uncore.c
index 31163af84e..27f4684f76 100644
--- a/app/test/test_power_intel_uncore.c
+++ b/app/test/test_power_intel_uncore.c
@@ -14,7 +14,7 @@ test_power_intel_uncore(void)
 }
 
 #else
-#include <rte_power_intel_uncore.h>
+#include <rte_power_uncore.h>
 #include <power_common.h>
 
 #define MAX_UNCORE_FREQS 32
@@ -246,11 +246,15 @@ test_power_intel_uncore(void)
 {
 	int ret;
 
+	ret = rte_power_set_uncore_env(UNCORE_PM_ENV_INTEL_UNCORE);
+	if (ret < 0)
+		goto fail_all;
+
 	ret = rte_power_uncore_get_num_pkgs();
 	if (ret == 0) {
 		printf("Uncore frequency management not supported/enabled on this kernel. "
-		"Please enable CONFIG_INTEL_UNCORE_FREQ_CONTROL if on x86 with linux kernel"
-		" >= 5.6\n");
+		"Please enable CONFIG_INTEL_UNCORE_FREQ_CONTROL if on intel x86 with "
+		"linux kernel >= 5.6\n");
 		return TEST_SKIPPED;
 	}
 
diff --git a/config/rte_config.h b/config/rte_config.h
index 400e44e3cf..ccca68b4f3 100644
--- a/config/rte_config.h
+++ b/config/rte_config.h
@@ -87,6 +87,7 @@
 
 /* rte_power defines */
 #define RTE_MAX_LCORE_FREQS 64
+#define RTE_MAX_UNCORE_FREQS 64
 
 /* rte_graph defines */
 #define RTE_GRAPH_BURST_SIZE 256
diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index 3f01cbd9e2..a34a7958f0 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -46,7 +46,7 @@
 #include <rte_metrics.h>
 #include <rte_telemetry.h>
 #include <rte_power_pmd_mgmt.h>
-#include <rte_power_intel_uncore.h>
+#include <rte_power_uncore.h>
 
 #include "perf_core.h"
 #include "main.h"
diff --git a/lib/power/meson.build b/lib/power/meson.build
index 1ce8b7c07d..9693c3ba7d 100644
--- a/lib/power/meson.build
+++ b/lib/power/meson.build
@@ -18,13 +18,14 @@ sources = files(
         'power_cppc_cpufreq.c',
         'power_kvm_vm.c',
         'power_pstate_cpufreq.c',
+        'power_intel_uncore.c',
         'rte_power.c',
-        'rte_power_intel_uncore.c',
+        'rte_power_uncore.c',
         'rte_power_pmd_mgmt.c',
 )
 headers = files(
         'rte_power.h',
-        'rte_power_intel_uncore.h',
+        'rte_power_uncore.h',
         'rte_power_pmd_mgmt.h',
         'rte_power_guest_channel.h',
 )
diff --git a/lib/power/rte_power_intel_uncore.c b/lib/power/power_intel_uncore.c
similarity index 90%
rename from lib/power/rte_power_intel_uncore.c
rename to lib/power/power_intel_uncore.c
index 3b8724385f..258cf78dff 100644
--- a/lib/power/rte_power_intel_uncore.c
+++ b/lib/power/power_intel_uncore.c
@@ -8,7 +8,7 @@
 
 #include <rte_memcpy.h>
 
-#include "rte_power_intel_uncore.h"
+#include "power_intel_uncore.h"
 #include "power_common.h"
 
 #define MAX_UNCORE_FREQS 32
@@ -246,7 +246,7 @@ static int
 check_pkg_die_values(unsigned int pkg, unsigned int die)
 {
 	unsigned int max_pkgs, max_dies;
-	max_pkgs = rte_power_uncore_get_num_pkgs();
+	max_pkgs = power_intel_uncore_get_num_pkgs();
 	if (max_pkgs == 0)
 		return -1;
 	if (pkg >= max_pkgs) {
@@ -255,7 +255,7 @@ check_pkg_die_values(unsigned int pkg, unsigned int die)
 		return -1;
 	}
 
-	max_dies = rte_power_uncore_get_num_dies(pkg);
+	max_dies = power_intel_uncore_get_num_dies(pkg);
 	if (max_dies == 0)
 		return -1;
 	if (die >= max_dies) {
@@ -268,7 +268,7 @@ check_pkg_die_values(unsigned int pkg, unsigned int die)
 }
 
 int
-rte_power_uncore_init(unsigned int pkg, unsigned int die)
+power_intel_uncore_init(unsigned int pkg, unsigned int die)
 {
 	struct uncore_power_info *ui;
 
@@ -298,7 +298,7 @@ rte_power_uncore_init(unsigned int pkg, unsigned int die)
 }
 
 int
-rte_power_uncore_exit(unsigned int pkg, unsigned int die)
+power_intel_uncore_exit(unsigned int pkg, unsigned int die)
 {
 	struct uncore_power_info *ui;
 
@@ -333,7 +333,7 @@ rte_power_uncore_exit(unsigned int pkg, unsigned int die)
 }
 
 uint32_t
-rte_power_get_uncore_freq(unsigned int pkg, unsigned int die)
+power_get_intel_uncore_freq(unsigned int pkg, unsigned int die)
 {
 	int ret = check_pkg_die_values(pkg, die);
 	if (ret < 0)
@@ -343,7 +343,7 @@ rte_power_get_uncore_freq(unsigned int pkg, unsigned int die)
 }
 
 int
-rte_power_set_uncore_freq(unsigned int pkg, unsigned int die, uint32_t index)
+power_set_intel_uncore_freq(unsigned int pkg, unsigned int die, uint32_t index)
 {
 	int ret = check_pkg_die_values(pkg, die);
 	if (ret < 0)
@@ -353,7 +353,7 @@ rte_power_set_uncore_freq(unsigned int pkg, unsigned int die, uint32_t index)
 }
 
 int
-rte_power_uncore_freq_max(unsigned int pkg, unsigned int die)
+power_intel_uncore_freq_max(unsigned int pkg, unsigned int die)
 {
 	int ret = check_pkg_die_values(pkg, die);
 	if (ret < 0)
@@ -364,7 +364,7 @@ rte_power_uncore_freq_max(unsigned int pkg, unsigned int die)
 
 
 int
-rte_power_uncore_freq_min(unsigned int pkg, unsigned int die)
+power_intel_uncore_freq_min(unsigned int pkg, unsigned int die)
 {
 	int ret = check_pkg_die_values(pkg, die);
 	if (ret < 0)
@@ -376,7 +376,32 @@ rte_power_uncore_freq_min(unsigned int pkg, unsigned int die)
 }
 
 int
-rte_power_uncore_get_num_freqs(unsigned int pkg, unsigned int die)
+power_intel_uncore_freqs(unsigned int pkg, unsigned int die,
+				 uint32_t *freqs, uint32_t num)
+{
+	struct uncore_power_info *ui;
+
+	int ret = check_pkg_die_values(pkg, die);
+	if (ret < 0)
+		return -1;
+
+	if (freqs == NULL) {
+		RTE_LOG(ERR, POWER, "NULL buffer supplied\n");
+		return 0;
+	}
+
+	ui = &uncore_info[pkg][die];
+	if (num < ui->nb_freqs) {
+		RTE_LOG(ERR, POWER, "Buffer size is not enough\n");
+		return 0;
+	}
+	rte_memcpy(freqs, ui->freqs, ui->nb_freqs * sizeof(uint32_t));
+
+	return ui->nb_freqs;
+}
+
+int
+power_intel_uncore_get_num_freqs(unsigned int pkg, unsigned int die)
 {
 	int ret = check_pkg_die_values(pkg, die);
 	if (ret < 0)
@@ -386,7 +411,7 @@ rte_power_uncore_get_num_freqs(unsigned int pkg, unsigned int die)
 }
 
 unsigned int
-rte_power_uncore_get_num_pkgs(void)
+power_intel_uncore_get_num_pkgs(void)
 {
 	DIR *d;
 	struct dirent *dir;
@@ -416,14 +441,14 @@ rte_power_uncore_get_num_pkgs(void)
 }
 
 unsigned int
-rte_power_uncore_get_num_dies(unsigned int pkg)
+power_intel_uncore_get_num_dies(unsigned int pkg)
 {
 	DIR *d;
 	struct dirent *dir;
 	unsigned int count = 0, max_pkgs;
 	char filter[FILTER_LENGTH];
 
-	max_pkgs = rte_power_uncore_get_num_pkgs();
+	max_pkgs = power_intel_uncore_get_num_pkgs();
 	if (max_pkgs == 0)
 		return 0;
 	if (pkg >= max_pkgs) {
diff --git a/lib/power/rte_power_intel_uncore.h b/lib/power/power_intel_uncore.h
similarity index 77%
rename from lib/power/rte_power_intel_uncore.h
rename to lib/power/power_intel_uncore.h
index 0bd9f193a1..32d6b1f7a4 100644
--- a/lib/power/rte_power_intel_uncore.h
+++ b/lib/power/power_intel_uncore.h
@@ -2,8 +2,8 @@
  * Copyright(c) 2022 Intel Corporation
  */
 
-#ifndef RTE_POWER_INTEL_UNCORE_H
-#define RTE_POWER_INTEL_UNCORE_H
+#ifndef POWER_INTEL_UNCORE_H
+#define POWER_INTEL_UNCORE_H
 
 /**
  * @file
@@ -12,6 +12,7 @@
 
 #include <rte_compat.h>
 #include "rte_power.h"
+#include "rte_power_uncore.h"
 
 #ifdef __cplusplus
 extern "C" {
@@ -34,9 +35,7 @@ extern "C" {
  *  - 0 on success.
  *  - Negative on error.
  */
-__rte_experimental
-int
-rte_power_uncore_init(unsigned int pkg, unsigned int die);
+int power_intel_uncore_init(unsigned int pkg, unsigned int die);
 
 /**
  * Exit uncore frequency management on a specific die on a package.
@@ -56,9 +55,7 @@ rte_power_uncore_init(unsigned int pkg, unsigned int die);
  *  - 0 on success.
  *  - Negative on error.
  */
-__rte_experimental
-int
-rte_power_uncore_exit(unsigned int pkg, unsigned int die);
+int power_intel_uncore_exit(unsigned int pkg, unsigned int die);
 
 /**
  * Return the current index of available frequencies of a specific die on a package.
@@ -77,9 +74,7 @@ rte_power_uncore_exit(unsigned int pkg, unsigned int die);
  *  The current index of available frequencies.
  *  If error, it will return 'RTE_POWER_INVALID_FREQ_INDEX = (~0)'.
  */
-__rte_experimental
-uint32_t
-rte_power_get_uncore_freq(unsigned int pkg, unsigned int die);
+uint32_t power_get_intel_uncore_freq(unsigned int pkg, unsigned int die);
 
 /**
  * Set minimum and maximum uncore frequency for specified die on a package
@@ -102,9 +97,7 @@ rte_power_get_uncore_freq(unsigned int pkg, unsigned int die);
  *  - 0 on success without frequency changed.
  *  - Negative on error.
  */
-__rte_experimental
-int
-rte_power_set_uncore_freq(unsigned int pkg, unsigned int die, uint32_t index);
+int power_set_intel_uncore_freq(unsigned int pkg, unsigned int die, uint32_t index);
 
 /**
  * Set minimum and maximum uncore frequency for specified die on a package
@@ -125,9 +118,7 @@ rte_power_set_uncore_freq(unsigned int pkg, unsigned int die, uint32_t index);
  *  - 0 on success without frequency changed.
  *  - Negative on error.
  */
-__rte_experimental
-int
-rte_power_uncore_freq_max(unsigned int pkg, unsigned int die);
+int power_intel_uncore_freq_max(unsigned int pkg, unsigned int die);
 
 /**
  * Set minimum and maximum uncore frequency for specified die on a package
@@ -148,9 +139,30 @@ rte_power_uncore_freq_max(unsigned int pkg, unsigned int die);
  *  - 0 on success without frequency changed.
  *  - Negative on error.
  */
-__rte_experimental
-int
-rte_power_uncore_freq_min(unsigned int pkg, unsigned int die);
+int power_intel_uncore_freq_min(unsigned int pkg, unsigned int die);
+
+/**
+ * Return the list of available frequencies in the index array.
+ *
+ * This function should NOT be called in the fast path.
+ *
+ * @param pkg
+ *  Package number.
+ *  Each physical CPU in a system is referred to as a package.
+ * @param die
+ *  Die number.
+ *  Each package can have several dies connected together via the uncore mesh.
+ * @param freqs
+ *  The buffer array to save the frequencies.
+ * @param num
+ *  The number of frequencies to get.
+ *
+ * @return
+ *  - The number of available index's in frequency array.
+ *  - Negative on error.
+ */
+int power_intel_uncore_freqs(unsigned int pkg, unsigned int die,
+				unsigned int *freqs, unsigned int num);
 
 /**
  * Return the list length of available frequencies in the index array.
@@ -168,9 +180,7 @@ rte_power_uncore_freq_min(unsigned int pkg, unsigned int die);
  *  - The number of available index's in frequency array.
  *  - Negative on error.
  */
-__rte_experimental
-int
-rte_power_uncore_get_num_freqs(unsigned int pkg, unsigned int die);
+int power_intel_uncore_get_num_freqs(unsigned int pkg, unsigned int die);
 
 /**
  * Return the number of packages (CPUs) on a system
@@ -182,9 +192,7 @@ rte_power_uncore_get_num_freqs(unsigned int pkg, unsigned int die);
  *  - Zero on error.
  *  - Number of package on system on success.
  */
-__rte_experimental
-unsigned int
-rte_power_uncore_get_num_pkgs(void);
+unsigned int power_intel_uncore_get_num_pkgs(void);
 
 /**
  * Return the number of dies for pakckages (CPUs) specified
@@ -200,12 +208,10 @@ rte_power_uncore_get_num_pkgs(void);
  *  - Zero on error.
  *  - Number of dies for package on sucecss.
  */
-__rte_experimental
-unsigned int
-rte_power_uncore_get_num_dies(unsigned int pkg);
+unsigned int power_intel_uncore_get_num_dies(unsigned int pkg);
 
 #ifdef __cplusplus
 }
 #endif
 
-#endif /* RTE_POWER_INTEL_UNCORE_H */
+#endif /* POWER_INTEL_UNCORE_H */
diff --git a/lib/power/rte_power_uncore.c b/lib/power/rte_power_uncore.c
new file mode 100644
index 0000000000..79303ebb3a
--- /dev/null
+++ b/lib/power/rte_power_uncore.c
@@ -0,0 +1,189 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2010-2014 Intel Corporation
+ * Copyright(c) 2023 AMD Corporation
+ */
+
+#include <errno.h>
+
+#include <rte_errno.h>
+#include <rte_spinlock.h>
+
+#include "rte_power_uncore.h"
+#include "power_intel_uncore.h"
+
+enum uncore_power_mgmt_env default_uncore_env = UNCORE_PM_ENV_NOT_SET;
+
+static rte_spinlock_t global_env_cfg_lock = RTE_SPINLOCK_INITIALIZER;
+
+static uint32_t
+power_get_dummy_uncore_freq(unsigned int pkg __rte_unused,
+	       unsigned int die __rte_unused)
+{
+	return 0;
+}
+
+static int
+power_set_dummy_uncore_freq(unsigned int pkg __rte_unused,
+	       unsigned int die __rte_unused, uint32_t index __rte_unused)
+{
+	return 0;
+}
+
+static int
+power_dummy_uncore_freq_max(unsigned int pkg __rte_unused,
+	       unsigned int die __rte_unused)
+{
+	return 0;
+}
+
+static int
+power_dummy_uncore_freq_min(unsigned int pkg __rte_unused,
+	       unsigned int die __rte_unused)
+{
+	return 0;
+}
+
+static int
+power_dummy_uncore_freqs(unsigned int pkg __rte_unused, unsigned int die __rte_unused,
+			uint32_t *freqs __rte_unused, uint32_t num __rte_unused)
+{
+	return 0;
+}
+
+static int
+power_dummy_uncore_get_num_freqs(unsigned int pkg __rte_unused,
+	       unsigned int die __rte_unused)
+{
+	return 0;
+}
+
+static unsigned int
+power_dummy_uncore_get_num_pkgs(void)
+{
+	return 0;
+}
+
+static unsigned int
+power_dummy_uncore_get_num_dies(unsigned int pkg __rte_unused)
+{
+	return 0;
+}
+
+/* function pointers */
+rte_power_get_uncore_freq_t rte_power_get_uncore_freq = power_get_dummy_uncore_freq;
+rte_power_set_uncore_freq_t rte_power_set_uncore_freq = power_set_dummy_uncore_freq;
+rte_power_uncore_freq_change_t rte_power_uncore_freq_max = power_dummy_uncore_freq_max;
+rte_power_uncore_freq_change_t rte_power_uncore_freq_min = power_dummy_uncore_freq_min;
+rte_power_uncore_freqs_t rte_power_uncore_freqs = power_dummy_uncore_freqs;
+rte_power_uncore_get_num_freqs_t rte_power_uncore_get_num_freqs =
+					power_dummy_uncore_get_num_freqs;
+rte_power_uncore_get_num_pkgs_t rte_power_uncore_get_num_pkgs =
+					power_dummy_uncore_get_num_pkgs;
+rte_power_uncore_get_num_dies_t rte_power_uncore_get_num_dies =
+					power_dummy_uncore_get_num_dies;
+
+static void
+reset_power_uncore_function_ptrs(void)
+{
+	rte_power_get_uncore_freq = power_get_dummy_uncore_freq;
+	rte_power_set_uncore_freq = power_set_dummy_uncore_freq;
+	rte_power_uncore_freq_max = power_dummy_uncore_freq_max;
+	rte_power_uncore_freq_min = power_dummy_uncore_freq_min;
+	rte_power_uncore_freqs  = power_dummy_uncore_freqs;
+	rte_power_uncore_get_num_freqs = power_dummy_uncore_get_num_freqs;
+	rte_power_uncore_get_num_pkgs = power_dummy_uncore_get_num_pkgs;
+	rte_power_uncore_get_num_dies = power_dummy_uncore_get_num_dies;
+}
+
+int
+rte_power_set_uncore_env(enum uncore_power_mgmt_env env)
+{
+	rte_spinlock_lock(&global_env_cfg_lock);
+
+	if (default_uncore_env != UNCORE_PM_ENV_NOT_SET) {
+		RTE_LOG(ERR, POWER, "Uncore Power Management Env already set.\n");
+		rte_spinlock_unlock(&global_env_cfg_lock);
+		return -1;
+	}
+
+	int ret = 0;
+
+	if (env == UNCORE_PM_ENV_INTEL_UNCORE) {
+		rte_power_get_uncore_freq = power_get_intel_uncore_freq;
+		rte_power_set_uncore_freq = power_set_intel_uncore_freq;
+		rte_power_uncore_freq_min  = power_intel_uncore_freq_min;
+		rte_power_uncore_freq_max  = power_intel_uncore_freq_max;
+		rte_power_uncore_freqs = power_intel_uncore_freqs;
+		rte_power_uncore_get_num_freqs = power_intel_uncore_get_num_freqs;
+		rte_power_uncore_get_num_pkgs = power_intel_uncore_get_num_pkgs;
+		rte_power_uncore_get_num_dies = power_intel_uncore_get_num_dies;
+	} else {
+		RTE_LOG(ERR, POWER, "Invalid Power Management Environment(%d) set\n",
+				env);
+		ret = -1;
+		goto out;
+	}
+
+	default_uncore_env = env;
+out:
+	rte_spinlock_unlock(&global_env_cfg_lock);
+	return ret;
+}
+
+void
+rte_power_unset_uncore_env(void)
+{
+	rte_spinlock_lock(&global_env_cfg_lock);
+	default_uncore_env = UNCORE_PM_ENV_NOT_SET;
+	reset_power_uncore_function_ptrs();
+	rte_spinlock_unlock(&global_env_cfg_lock);
+}
+
+enum uncore_power_mgmt_env
+rte_power_get_uncore_env(void)
+{
+	return default_uncore_env;
+}
+
+int
+rte_power_uncore_init(unsigned int pkg, unsigned int die)
+{
+	int ret = -1;
+
+	switch (default_uncore_env) {
+	case UNCORE_PM_ENV_INTEL_UNCORE:
+		return power_intel_uncore_init(pkg, die);
+	default:
+		RTE_LOG(INFO, POWER, "Uncore Env isn't set yet!\n");
+	}
+
+	/* Auto detect Environment */
+	RTE_LOG(INFO, POWER, "Attempting to initialise Intel Uncore power mgmt...\n");
+	ret = power_intel_uncore_init(pkg, die);
+	if (ret == 0) {
+		rte_power_set_uncore_env(UNCORE_PM_ENV_INTEL_UNCORE);
+		goto out;
+	}
+
+	if (default_uncore_env == UNCORE_PM_ENV_NOT_SET) {
+		RTE_LOG(ERR, POWER, "Unable to set Power Management Environment "
+				"for package %u Die %u\n", pkg, die);
+		ret = 0;
+	}
+out:
+	return ret;
+}
+
+int
+rte_power_uncore_exit(unsigned int pkg, unsigned int die)
+{
+	switch (default_uncore_env) {
+	case UNCORE_PM_ENV_INTEL_UNCORE:
+		return power_intel_uncore_exit(pkg, die);
+	default:
+		RTE_LOG(ERR, POWER, "Uncore Env has not been set,"
+			       "unable to exit gracefully\n");
+	}
+	return -1;
+
+}
diff --git a/lib/power/rte_power_uncore.h b/lib/power/rte_power_uncore.h
index e27f483eae..a43346b186 100644
--- a/lib/power/rte_power_uncore.h
+++ b/lib/power/rte_power_uncore.h
@@ -22,6 +22,38 @@ extern "C" {
 enum uncore_power_mgmt_env { UNCORE_PM_ENV_NOT_SET,
 		UNCORE_PM_ENV_INTEL_UNCORE, UNCORE_PM_ENV_AMD_HSMP};
 
+/**
+ * Set the default uncore power management implementation. If this is not called prior
+ * to rte_power_uncore_init(), then auto-detect of the environment will take place.
+ * It is thread safe. New env can be set only in uninitialized state
+ * (thus rte_power_unset_uncore_env must be called if different env was already set).
+ *
+ * @param env
+ *  env. The environment in which to initialise Uncore Power Management for.
+ *
+ * @return
+ *  - 0 on success.
+ *  - Negative on error.
+ */
+__rte_experimental
+int rte_power_set_uncore_env(enum uncore_power_mgmt_env env);
+
+/**
+ * Unset the global uncore environment configuration.
+ * This can only be called after all threads have completed.
+ */
+__rte_experimental
+void rte_power_unset_uncore_env(void);
+
+/**
+ * Get the default uncore power management implementation.
+ *
+ * @return
+ *  power_management_env The configured environment.
+ */
+__rte_experimental
+enum uncore_power_mgmt_env rte_power_get_uncore_env(void);
+
 /**
  * Initialize uncore frequency management for specific die on a package.
  * It will get the available frequencies and prepare to set new die frequencies.
diff --git a/lib/power/version.map b/lib/power/version.map
index b8b54f768e..c22813e89a 100644
--- a/lib/power/version.map
+++ b/lib/power/version.map
@@ -52,4 +52,10 @@ EXPERIMENTAL {
 	rte_power_uncore_get_num_freqs;
 	rte_power_uncore_get_num_pkgs;
 	rte_power_uncore_init;
+
+	# added in 23.11
+	rte_power_uncore_freqs;
+	rte_power_set_uncore_env;
+	rte_power_unset_uncore_env;
+	rte_power_get_uncore_env;
 };
-- 
2.34.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] power: refactor uncore power management interfaces
  2023-08-16 10:09   ` [PATCH v2 1/2] power: refactor uncore power management interfaces Sivaprasad Tummala
  2023-08-16 10:09     ` [PATCH v2 2/2] power: refactor uncore power management implementation Sivaprasad Tummala
@ 2023-10-06  9:03     ` David Marchand
  2023-10-09 13:56       ` Tummala, Sivaprasad
  2023-10-17  8:19     ` David Marchand
  2 siblings, 1 reply; 10+ messages in thread
From: David Marchand @ 2023-10-06  9:03 UTC (permalink / raw)
  To: david.hunt, anatoly.burakov; +Cc: ferruh.yigit, dev, Sivaprasad Tummala

On Wed, Aug 16, 2023 at 12:10 PM Sivaprasad Tummala
<sivaprasad.tummala@amd.com> wrote:
>
> currently the uncore power management implementation is vendor specific.
> Added new vendor agnostic uncore power interface similar to rte_power
> and rename specific implementations ("rte_power_intel_uncore") to
> "power_intel_uncore" along with functions.
>
> Signed-off-by: Sivaprasad Tummala <sivaprasad.tummala@amd.com>

Review please.

-- 
David Marchand


^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH v2 1/2] power: refactor uncore power management interfaces
  2023-10-06  9:03     ` [PATCH v2 1/2] power: refactor uncore power management interfaces David Marchand
@ 2023-10-09 13:56       ` Tummala, Sivaprasad
  0 siblings, 0 replies; 10+ messages in thread
From: Tummala, Sivaprasad @ 2023-10-09 13:56 UTC (permalink / raw)
  To: David Marchand, david.hunt, anatoly.burakov; +Cc: Yigit, Ferruh, dev

[AMD Official Use Only - General]

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Friday, October 6, 2023 2:33 PM
> To: david.hunt@intel.com; anatoly.burakov@intel.com
> Cc: Yigit, Ferruh <Ferruh.Yigit@amd.com>; dev@dpdk.org; Tummala, Sivaprasad
> <Sivaprasad.Tummala@amd.com>
> Subject: Re: [PATCH v2 1/2] power: refactor uncore power management interfaces
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On Wed, Aug 16, 2023 at 12:10 PM Sivaprasad Tummala
> <sivaprasad.tummala@amd.com> wrote:
> >
> > currently the uncore power management implementation is vendor specific.
> > Added new vendor agnostic uncore power interface similar to rte_power
> > and rename specific implementations ("rte_power_intel_uncore") to
> > "power_intel_uncore" along with functions.
> >
> > Signed-off-by: Sivaprasad Tummala <sivaprasad.tummala@amd.com>
>
> Review please.
>
Hi Anatoly,
Based on your earlier review comments on RFC patch, it appears you are fine with the refactor.
Can I use that as an ACK to this patch. Please let me know otherwise.

> --
> David Marchand


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] power: refactor uncore power management interfaces
  2023-08-16 10:09   ` [PATCH v2 1/2] power: refactor uncore power management interfaces Sivaprasad Tummala
  2023-08-16 10:09     ` [PATCH v2 2/2] power: refactor uncore power management implementation Sivaprasad Tummala
  2023-10-06  9:03     ` [PATCH v2 1/2] power: refactor uncore power management interfaces David Marchand
@ 2023-10-17  8:19     ` David Marchand
  2 siblings, 0 replies; 10+ messages in thread
From: David Marchand @ 2023-10-17  8:19 UTC (permalink / raw)
  To: Sivaprasad Tummala
  Cc: david.hunt, anatoly.burakov, ferruh.yigit, dev, Thomas Monjalon,
	Mcnamara, John

On Wed, Aug 16, 2023 at 12:10 PM Sivaprasad Tummala
<sivaprasad.tummala@amd.com> wrote:
>
> currently the uncore power management implementation is vendor specific.
> Added new vendor agnostic uncore power interface similar to rte_power
> and rename specific implementations ("rte_power_intel_uncore") to
> "power_intel_uncore" along with functions.
>
> Signed-off-by: Sivaprasad Tummala <sivaprasad.tummala@amd.com>

No news from the maintainers is not a good news...
But, given Anatoly was not against it, I took this series for rc1.


Applied with some fixes on doc and style, and added an entry in the RN, thanks.

-- 
David Marchand


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-10-17  8:19 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-03 17:03 [RFC PATCH] power: refactor uncore power management interfaces Sivaprasad Tummala
2023-05-17 11:42 ` Tummala, Sivaprasad
2023-05-19  9:42 ` Burakov, Anatoly
2023-08-11 10:25 ` [PATCH v1 1/2] " Sivaprasad Tummala
2023-08-11 10:25   ` [PATCH v1 2/2] power: refactor uncore power management implementation Sivaprasad Tummala
2023-08-16 10:09   ` [PATCH v2 1/2] power: refactor uncore power management interfaces Sivaprasad Tummala
2023-08-16 10:09     ` [PATCH v2 2/2] power: refactor uncore power management implementation Sivaprasad Tummala
2023-10-06  9:03     ` [PATCH v2 1/2] power: refactor uncore power management interfaces David Marchand
2023-10-09 13:56       ` Tummala, Sivaprasad
2023-10-17  8:19     ` David Marchand

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).