From: Thomas Monjalon <thomas@monjalon.net>
To: Tadhg Kearney <tadhg.kearney@intel.com>
Cc: dev@dpdk.org, david.hunt@intel.com, anatoly.burakov@intel.com,
reshma.pattan@intel.com, david.marchand@redhat.com
Subject: Re: [PATCH v9 1/3] power: add Intel uncore frequency control API to power library
Date: Mon, 10 Oct 2022 14:46:17 +0200 [thread overview]
Message-ID: <10475720.aFP6jjVeTY@thomas> (raw)
In-Reply-To: <20221006093803.2076768-2-tadhg.kearney@intel.com>
06/10/2022 11:38, Tadhg Kearney:
> +API Overview for Intel Uncore
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +Overview of each function in the Intel Uncore API, with explanation of what
> +they do. Each function should not be called in the fast path.
> +
> +* **Uncore Power Init**
> + Initialize uncore power, populate frequency array and record
> + original min & max for die on pkg.
> +
> +* **Uncore Power Exit**
> + Exit uncore power, restoring original min & max for die on pkg.
> +
> +* **Get Uncore Power Freq**
> + Get current uncore freq index for die on pkg.
> +
> +* **Set Uncore Power Freq**
> + Set min & max uncore freq index for die on pkg to specified index value
> + (min and max will be the same).
> +
> +* **Uncore Power Max**
> + Set min & max uncore freq to maximum frequency index for die on pkg
> + (min and max will be the same).
> +
> +* **Uncore Power Min**
> + Set min & max uncore freq to minimum frequency index for die on pkg
> + (min and max will be the same).
> +
> +* **Get Num Freqs**
> + Get the number of frequencies in the index array.
> +
> +* **Get Num Pkgs**
> + Get the number of packages (CPU's) on the system.
> +
> +* **Get Num Dies**
> + Get the number of die's on a given package.
This is the role of doxygen documentation to explain API.
I don't understand why it is there.
Anyway I've converted it into a definition list,
which the proper RST syntax for what you do.
> 'rte_power.c',
> 'rte_power_empty_poll.c',
> 'rte_power_pmd_mgmt.c',
> + 'rte_power_intel_uncore.c',
In general, we keep such list in alphabetical order.
[...]
> +struct 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 */
> + FILE *f_cur_min; /** FD of scaling_min */
> + FILE *f_cur_max; /** FD of scaling_max */
> + uint32_t curr_idx; /** Freq index in freqs array */
> + uint32_t org_min_freq; /** Original min freq of uncore */
> + uint32_t org_max_freq; /** Original max freq of uncore */
> + uint32_t init_max_freq; /** System max uncore freq */
> + uint32_t init_min_freq; /** System min uncore freq */
> +} __rte_cache_aligned;
No need of doxygen syntax in a .c file.
And an alignment is wrong.
[...]
> + RTE_LOG(DEBUG, POWER, "Invalid uncore frequency index %u, which "
> + "should be less than %u\n", idx, ui->nb_freqs);
When you have time, it would be good to switch to dynamic logging.
See RTE_LOG_REGISTER_DEFAULT
[...]
> +#ifndef _RTE_POWER_INTEL_UNCORE_H
> +#define _RTE_POWER_INTEL_UNCORE_H
underscore prefix is reserved for reserved keywords
[...]
> +/**
> + * Exit uncore frequency management on a specific die on a package. It will restore uncore min and
Screen width is limited, while scrolling bar is infinite,
so don't hesitate to break your lines shorter when possible,
like at the end of a sentence.
next prev parent reply other threads:[~2022-10-10 12:46 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-20 9:48 [PATCH v4 0/3] add uncore api to be called through l3fwd-power Tadhg Kearney
2022-09-20 9:48 ` [PATCH v4 1/3] power: add uncore frequency control API to the power library Tadhg Kearney
2022-09-23 13:15 ` Hunt, David
2022-09-20 9:48 ` [PATCH v4 2/3] l3fwd-power: add option to call uncore API Tadhg Kearney
2022-09-23 13:13 ` Hunt, David
2022-09-20 9:48 ` [PATCH v4 3/3] test/power: add unit tests for " Tadhg Kearney
2022-09-23 13:15 ` Hunt, David
2022-09-27 10:09 ` [PATCH v5 0/3] add uncore api to be called through l3fwd-power Tadhg Kearney
2022-09-27 10:09 ` [PATCH v5 1/3] power: add uncore frequency control API to the power library Tadhg Kearney
2022-09-27 10:09 ` [PATCH v5 2/3] l3fwd-power: add option to call uncore API Tadhg Kearney
2022-09-27 10:09 ` [PATCH v5 3/3] test/power: add unit tests for " Tadhg Kearney
2022-09-28 9:06 ` [PATCH v6 0/3] add uncore api to be called through l3fwd-power Tadhg Kearney
2022-09-28 9:06 ` [PATCH v6 1/3] power: add uncore frequency control API to the power library Tadhg Kearney
2022-09-28 9:06 ` [PATCH v6 2/3] l3fwd-power: add option to call uncore API Tadhg Kearney
2022-09-28 12:18 ` Hunt, David
2022-09-28 9:06 ` [PATCH v6 3/3] test/power: add unit tests for " Tadhg Kearney
2022-09-28 13:30 ` [PATCH v7 0/3] add uncore api to be called through l3fwd-power Tadhg Kearney
2022-09-28 13:30 ` [PATCH v7 1/3] power: add uncore frequency control API to the power library Tadhg Kearney
2022-10-04 17:09 ` Thomas Monjalon
2022-10-05 10:50 ` Kearney, Tadhg
2022-10-05 12:11 ` Thomas Monjalon
2022-09-28 13:30 ` [PATCH v7 2/3] l3fwd-power: add option to call uncore API Tadhg Kearney
2022-09-28 13:30 ` [PATCH v7 3/3] test/power: add unit tests for " Tadhg Kearney
2022-09-29 13:27 ` [PATCH v7 0/3] add uncore api to be called through l3fwd-power Hunt, David
2022-10-05 16:20 ` [PATCH v8 0/3] add Intel " Tadhg Kearney
2022-10-05 16:20 ` [PATCH v8 1/3] power: add Intel uncore frequency control API to power library Tadhg Kearney
2022-10-05 16:20 ` [PATCH v8 2/3] l3fwd-power: add option to call uncore API Tadhg Kearney
2022-10-05 16:20 ` [PATCH v8 3/3] test/power: add unit tests for " Tadhg Kearney
2022-10-06 9:38 ` [PATCH v9 0/3] add Intel uncore api to be called through l3fwd-power Tadhg Kearney
2022-10-06 9:38 ` [PATCH v9 1/3] power: add Intel uncore frequency control API to power library Tadhg Kearney
2022-10-06 17:32 ` Stephen Hemminger
2022-10-07 10:30 ` Hunt, David
2022-10-10 12:46 ` Thomas Monjalon [this message]
2022-10-06 9:38 ` [PATCH v9 2/3] l3fwd-power: add option to call uncore API Tadhg Kearney
2022-10-06 9:38 ` [PATCH v9 3/3] test/power: add unit tests for " Tadhg Kearney
2022-10-10 12:52 ` [PATCH v9 0/3] add Intel uncore api to be called through l3fwd-power Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=10475720.aFP6jjVeTY@thomas \
--to=thomas@monjalon.net \
--cc=anatoly.burakov@intel.com \
--cc=david.hunt@intel.com \
--cc=david.marchand@redhat.com \
--cc=dev@dpdk.org \
--cc=reshma.pattan@intel.com \
--cc=tadhg.kearney@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).