Re: [PATCH v2 2/4] Documentation/powercap/dtpm: Add documentation for dtpm

Jonathan Corbet
2020-11-17T16:09:07Z

On Mon, 16 Nov 2020 16:26:47 +0100
Daniel Lezcano <daniel.lezcano@linaro.org> wrote:

> The dynamic thermal and power management is a technique to dynamically
> adjust the power consumption of different devices in order to ensure a
> global thermal constraint.
>
> An userspace daemon is usually monitoring the temperature and the
> power to take immediate action on the device.
>
> The DTPM framework provides an unified API to userspace to act on the
> power.
>
> Document this framework.

It's always refreshing to see documentation show up with a new feature! :)

That said, it's clear that you haven't built the docs with this new
material. There's a couple of little things I would ask you to do...

> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
> Documentation/power/powercap/dtpm.rst | 222 ++++++++++++++++++++++++++
> 1 file changed, 222 insertions(+)
> create mode 100644 Documentation/power/powercap/dtpm.rst

You also need to add this new file to index.rst

> diff --git a/Documentation/power/powercap/dtpm.rst b/Documentation/power/powercap/dtpm.rst
> new file mode 100644
> index 000000000000..ce11cf183994
> --- /dev/null
> +++ b/Documentation/power/powercap/dtpm.rst
> @@ -0,0 +1,222 @@
> +==========================================
> +Dynamic Thermal Power Management framework
> +==========================================
> +
> +On the embedded world, the complexity of the SoC leads to an
> +increasing number of hotspots which need to be monitored and mitigated
> +as a whole in order to prevent the temperature to go above the
> +normative and legally stated 'skin temperature'.
> +
> +Another aspect is to sustain the performance for a given power budget,
> +for example virtual reality where the user can feel dizziness if the
> +performance is capped while a big CPU is processing something else. Or
> +reduce the battery charging because the dissipated power is too high
> +compared with the power consumed by other devices.
> +
> +The userspace is the most adequate place to dynamically act on the
> +different devices by limiting their power given an application
> +profile: it has the knowledge of the platform.
> +
> +The Dynamic Thermal Power Management (DTPM) is a technique acting on
> +the device power by limiting and/or balancing a power budget among
> +different devices.
> +
> +The DTPM framework provides an unified interface to act on the
> +device power.
> +
> +===========
> +1. Overview
> +===========

Please follow the sequence of subheading markers described in
Documentation/doc-guide/sphinx.rst. This one should be:

Overview
========

(I took out the section number because that's not really needed in the RST
world, but that's up to you).

> +The DTPM framework relies on the powercap framework to create the
> +powercap entries in the sysfs directory and implement the backend
> +driver to do the connection with the power manageable device.
> +
> +The DTPM is a tree representation describing the power constraints
> +shared between devices, not their physical positions.
> +
> +The nodes of the tree are a virtual description aggregating the power
> +characteristics of the children nodes and their power limitations.
> +
> +The leaves of the tree are the real power manageable devices.
> +
> +For instance:
> +
> + SoC
> + |
> + `-- pkg
> + |
> + |-- pd0 (cpu0-3)
> + |
> + `-- pd1 (cpu4-5)

This formatting will throw errors when you try to do the docs build. The
easiest solution is to put literal blocks like this into ... a literal
block. And that is most easily done by putting a double colon at the end
of the leading line:

For instance::

The same will need to happen for all the rest of the ascii art.

> +* The pkg power will be the sum of pd0 and pd1 power numbers.
> +
> + SoC (400mW - 3100mW)
> + |
> + `-- pkg (400mW - 3100mW)
> + |
> + |-- pd0 (100mW - 700mW)
> + |
> + `-- pd1 (300mW - 2400mW)
> +
> +* When the nodes are inserted in the tree, their power characteristics
> + are propagated to the parents.

I suspect you'll also need to indent the literal block another space or two
to separate it from the bulleted list.

You don't have any kernel-doc directives to bring in the kerneldoc comments
from the code. The code itself *does* have those comments, so you might
want to add those.

Thanks,

jon