Re: [PATCH v2] admin guide/pm: Admin guide for Intel Uncore Frequency limits

[Srinivas Pandruvada]

On Fri, 2020-01-24 at 10:00 -0700, Jonathan Corbet wrote:
> On Sun, 12 Jan 2020 20:01:43 -0800
> Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx> wrote:
> 
> > Added documentation for the attributes to control uncore frequency
> > selection.
> > 
> > Signed-off-by: Srinivas Pandruvada <
> > srinivas.pandruvada@xxxxxxxxxxxxxxx>
> > ---
> > v2:
> >  - Split the documentation patch to another patch to merge via
> > different
> >     tree
> 
> Which tree did you have in mind?  PM stuff tends to go through
> Rafael's
> tree, normally, which is fine.
Linux PM tree is fine. I will add necessary --to and --cc to my next
version of the patch with the suggested changes from you.

Thanks,
Srinivas

> 
> >  Documentation/admin-guide/pm/intel_uncore.rst | 23
> > +++++++++++++++++++
> >  .../admin-guide/pm/working-state.rst          |  1 +
> >  2 files changed, 24 insertions(+)
> >  create mode 100644 Documentation/admin-guide/pm/intel_uncore.rst
> > 
> > diff --git a/Documentation/admin-guide/pm/intel_uncore.rst
> > b/Documentation/admin-guide/pm/intel_uncore.rst
> > new file mode 100644
> > index 000000000000..d75be65fb16a
> > --- /dev/null
> > +++ b/Documentation/admin-guide/pm/intel_uncore.rst
> > @@ -0,0 +1,23 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +
> > +=========================================================
> > +Intel Uncore Frequency Selection
> 
> I would really like to avoid adding  symbols throughout the docs.  I
> get
> grief for non-ASCII symbols that actually have a need to be there;
> this
> isn't one of those.  
> 
> > +=========================================================
> > +
> > +The uncore frequency in the Intel(R) hardware is selected based on
> > internal heuristics, which uses the current selected performance
> > state and various system power constraints. In majority of the
> > cases this selection is the most optimal, so there is no need for
> > placing external constraints from the Operating System.
> 
> I would say that this violates the 80-character limit by a character
> or
> two...  The entire patch has this problem.
> 
> > +
> > +But there are some customers who wants less jitters from dynamic
> > uncore frequency selection. For them, power saving is much lower
> > priority than consistent performance. Currently these customers
> > uses MSR 0x620, to place hard limits on the maximum and the minimum
> > uncore frequency. They can now use Linux sysfs to place these
> > limits and also have additional capability to place hard limits
> > under power constraint scenario.
> 
> less jitter (singular)
> 
> > +
> > +The Uncore frequency section attributes are present under
> > "/sys/devices/system/cpu/intel_uncore_frequency".
> > +The scope of these attributes is per die in multi-die systems or
> > package wide in non multi-die systems. There is a unique folder for
> > each die or package. For example:
> > +"package_00_die_00" for package 0 and die 0.
> 
> This may not render as you would like; use an RST literal block here.
> 
> Thanks,
> 
> jon
>