Re: [PATCH] admin guide/pm: Admin guide for intel-speed-select
From: Jonathan Corbet
Date: Mon Nov 18 2019 - 12:41:09 EST
On Fri, 15 Nov 2019 12:49:25 -0800
Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx> wrote:
> Added documentation to configure servers to use Intel(R) Speed
> Select Technology using intel-speed-select tool.
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx>
Thanks for improving the docs! I do have a few overall comments,
> .../admin-guide/pm/intel-speed-select.rst | 934 ++++++++++++++++++
> .../admin-guide/pm/working-state.rst | 1 +
> 2 files changed, 935 insertions(+)
> create mode 100644 Documentation/admin-guide/pm/intel-speed-select.rst
> diff --git a/Documentation/admin-guide/pm/intel-speed-select.rst b/Documentation/admin-guide/pm/intel-speed-select.rst
> new file mode 100644
> index 000000000000..c2ce57ebc268
> --- /dev/null
> +++ b/Documentation/admin-guide/pm/intel-speed-select.rst
> @@ -0,0 +1,934 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +IntelÂ Speed Select Technology (IntelÂ SST) : User Guide
People give me grief when I take docs patches adding non-ascii characters.
Adding nearly 100 useless Â symbols seems likely to trigger that sort of
unicode aversion. Can I ask you, please, to take those out? There are
many occurrences of unadorned "Intel" in the kernel, and the world hasn't
> +The IntelÂ Speed Select Technology (IntelÂ SST) provides a powerful new
> +collection of features that give more granular control over CPU performance.
> +With IntelÂ SST, one server can be configured for power and performance for a
> +variety of diverse workload requirements.
> +Refer to the links below for an overview of the technology:
> +- https://www.intel.com/content/www/us/en/architecture-and-technology/speed-select-technology-article.html
> +- https://builders.intel.com/docs/networkbuilders/intel-speed-select-technology-base-frequency-enhancing-performance.pdf
> +These capabilities are further enhanced in some of the newer generations of
> +server platforms where these features can be enumerated and controlled
> +dynamically without pre-configuring via BIOS setup options. This dynamic
> +configuration is done via mailbox commands to the hardware. One way to enumerate
> +and configure these features is by using the IntelÂ Speed Select utility.
> +This document explains how to use the IntelÂ Speed Select tool to enumerate and
> +control IntelÂ SST features. This document gives example commands and explains
> +how these commands change the power and performance profile of the system under
> +test. Using this tool as an example, customers can replicate the messaging
> +implemented in the tool in their production software.
> +intel-speed-select configuration tool
The conventions for subsection markers are documented in
Documentation/doc-guide/sphinx.rst; this should be "========" rather than
> +Most Linux distribution packages include the "intel-speed-select" tool. If not,
> +it can be built by downloading the Linux kernel tree from kernel.org. Once
> +downloaded, the tool can be built without building the full kernel.
> +From the kernel tree, run the following commands:
> +# cd tools/power/x86/intel-speed-select/
> +# make
> +# make install
This kind of stuff isn't going to render well in the built docs. Can you
please change the literal text to literal blocks? Something like:
From the kernel tree, run the following commands::
# cd tools/power/x86/intel-speed-select/
# make install
Note the "::" on the first line; that introduces a literal block. It would
be good to build the docs once you're done and be sure that you're happy
with the results.
There's a lot of these in this document.
> +**Getting Help**
Since this is meant to be a section header, please mark it as such;
"------" would be the appropriate marker for a header at this level.
That's enough for a first pass.