Re: [PATCH] tools/power: amd_pstate_trace: fix help path dependencies

[Mario Limonciello]

On 7/22/26 10:21, K Prateek Nayak wrote:
> Hello Rafael,
>
> On 7/22/2026 6:18 PM, Rafael J. Wysocki (Intel) wrote:
> > On Wed, Jun 24, 2026 at 2:31 PM Yousef Alhouseen
> > <alhouseenyousef@xxxxxxxxx> wrote:
> > > amd_pstate_trace imports Gnuplot and numpy before parsing command-line
> > > options. As a result, even "-h" fails if those optional runtime modules are
> > > not installed.
> > >
> > > It also handles "-h" by printing a blank line and exiting. Use the existing
> > > help printer for "-h", and move the numpy import to the path that needs it.
> > > The direct Gnuplot import is unused, so remove it.
> > >
> > > Signed-off-by: Yousef Alhouseen <alhouseenyousef@xxxxxxxxx>
> >
> > AMD folks, if this change is fine with you, I can take it into
> > pm-tools, but an ACK (or equivalent) is needed.
>
> Sorry for the delay in reviewing! I have some comments below for Yousef.

Yes; sorry this got buried in my inbox when I was out and I still 
haven't all the way dug out.

> > Thanks!
> >
> > > ---
> > >   tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py | 11 +++++------
> > >   1 file changed, 5 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py b/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py
> > > index 875b08655..b1b201b2e 100755
> > > --- a/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py
> > > +++ b/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py
> > > @@ -27,8 +27,6 @@ import re
> > >   import signal
> > >   import sys
> > >   import getopt
> > > -import Gnuplot
>
> We don't use Gnuplot so removing that is fine ...
>
> > > -from numpy import *
>
> ... but we want to keep all the imports together at top to easily audit
> dependencies. Afacit, most python scripts in tools/power/x86/ follow
> that convention.
>
> There exists a big fat comment on top of the file reading:
>
>    Prerequisites:
>        Python version 2.7.x or higher
>        gnuplot 5.0 or higher
>        gnuplot-py 1.8 or higher
>
> which is a good indication of whatis actually required to run the
> script. If users prefer, we can add a requirements.txt that pip can
> consume to install the necessary dependencies and make it clear.

I think a requirements.txt and a quick few steps of how to start a venv 
and use it is a great idea.

> > >   from decimal import *
> > >   sys.path.append(os.path.join(os.path.dirname(__file__), "..", "intel_pstate_tracer"))
> > >   import intel_pstate_tracer as ipt
> > > @@ -260,9 +258,6 @@ graph_data_present = False;
> > >   valid1 = False
> > >   valid2 = False
> > >
> > > -cpu_mask = zeros((MAX_CPUS,), dtype=int)
> > > -
> > > -
> > >   try:
> > >       opts, args = getopt.getopt(sys.argv[1:],"ht:i:c:n:m:",["help","trace_file=","interval=","cpu=","name=","memory="])
> > >   except getopt.GetoptError:
> > > @@ -270,7 +265,7 @@ except getopt.GetoptError:
> > >       sys.exit(2)
> > >   for opt, arg in opts:
> > >       if opt == '-h':
> > > -        print()
> > > +        ipt.print_help('amd_pstate')
> > >           sys.exit()
> > >       elif opt in ("-t", "--trace_file"):
> > >           valid1 = True
> > > @@ -291,6 +286,10 @@ if not (valid1 and valid2):
> > >       ipt.print_help('amd_pstate')
> > >       sys.exit()
> > >
> > > +from numpy import zeros
>
> Having it here needs users to skim through the file to know what
> external packages the script needs and we don't want that.
>
> > > +
> > > +cpu_mask = zeros((MAX_CPUS,), dtype=int)
> > > +
> > >   if cpu_list:
> > >       for p in re.split("[,]", cpu_list):
> > >           if int(p) < MAX_CPUS :
> > > --
> > > 2.54.0