[PATCH i-g-t] tools/gputop/xe_gputop: Keep engine activity percentage between 0 and 100

[Cavitt, Jonathan]

From: Purkait, Soham <soham.purkait at intel.com>
Sent: Friday, July 25, 2025 3:29 AM
To: Cavitt, Jonathan <jonathan.cavitt at intel.com>; igt-dev at lists.freedesktop.org; Gupta, Anshuman <anshuman.gupta at intel.com>
Cc: Tauro, Riana <riana.tauro at intel.com>; De Marchi, Lucas <lucas.demarchi at intel.com>
Subject: Re: [PATCH i-g-t] tools/gputop/xe_gputop: Keep engine activity percentage between 0 and 100

> Hi Jonathan,
> > On 25-07-2025 02:51, Cavitt, Jonathan wrote:

> > -----Original Message-----

> > From: igt-dev <igt-dev-bounces at lists.freedesktop.org><mailto:igt-dev-bounces at lists.freedesktop.org> On Behalf Of Soham Purkait

> > Sent: Thursday, July 24, 2025 11:51 AM

> > To: igt-dev at lists.freedesktop.org<mailto:igt-dev at lists.freedesktop.org>; Gupta, Anshuman <anshuman.gupta at intel.com><mailto:anshuman.gupta at intel.com>

> > Cc: Purkait, Soham <soham.purkait at intel.com><mailto:soham.purkait at intel.com>; Tauro, Riana <riana.tauro at intel.com><mailto:riana.tauro at intel.com>; De Marchi, Lucas <lucas.demarchi at intel.com><mailto:lucas.demarchi at intel.com>

> > Subject: [PATCH i-g-t] tools/gputop/xe_gputop: Keep engine activity percentage between 0 and 100

> >

> > Engine activity percentage within the valid range of 0 to 100 prevents

> >

> >

> > s/Engine activity percentage/Clamping engine activity percentage to

> >

> > There's also a nit below, but it's non-blocking, so:

> > Reviewed-by: Jonathan Cavitt <jonathan.cavitt at intel.com><mailto:jonathan.cavitt at intel.com>

> >

> >

> > out-of-bound values that could lead to undefined behavior during runtime.

> >

> > Signed-off-by: Soham Purkait <soham.purkait at intel.com><mailto:soham.purkait at intel.com>

> > ---

> >  tools/gputop/xe_gputop.c | 6 ++++++

> >  1 file changed, 6 insertions(+)

> >

> > diff --git a/tools/gputop/xe_gputop.c b/tools/gputop/xe_gputop.c

> > index 9757369a8..ced427c0c 100644

> > --- a/tools/gputop/xe_gputop.c

> > +++ b/tools/gputop/xe_gputop.c

> > @@ -281,6 +281,12 @@ static double pmu_active_percentage(struct xe_engine *engine)

> >   double percentage;

> >

> >   percentage = (pmu_active_ticks * 100) / pmu_total_ticks;

> > +

> > + if (percentage > 100)

> > +  percentage = 100;

> > + else if (percentage < 0)

> > +  percentage = 0;

> > +

> >

> > NIT:

> > This clamping operation can be compressed using the fmin and fmax functions:

> >

> > """

> >  return fmax(0, fmin(percentage, 100));

> > """



I see Lucas pointed out a proper ‘clamp’ function for this purpose, as defined in igt_aux.h.

I had tried searching for one prior but failed to do a grep for one in the repo, and eventually

discovered that C didn’t support that operation natively, so I gave up searching for it.



I agree with Lucas that we should be using the clamp() function here.



> >

> > However, it's not particularly important that this be compressed, and doing so might

> > obfuscate what the code does (on top of possibly being slower due to the function

> > calls), so I'm fine with either way of performing the clamp.

> >

> > I should also note that we normally shouldn't clamp the percentage like this generally,

> > but since this is just a helper function used to pass a value into a pretty-print operation

> > for data visualization purposes, I suppose it's okay.  On the other hand, maybe we should

> > be printing the true, raw value separately?  Maybe as an extension of

> > print_percentage_bar?
> What could be the possible usages of the raw values ? Thanks, Soham

We return the raw percentage value for tests such as xe_drm_fdinfo, even if the raw percentage
value is some greater-than-100 amount, so I figured it would be reasonable to print the raw
percentage value in this case as well.
-Jonathan Cavitt


> >

> >

> > (Though, I guess at that point, we'd want to do the clamping there, rather than in

> > pmu_active_percentage...).

> >

> > Sorry.  I'm just thinking aloud.  You can take this advice or leave it.

> >

> > -Jonathan Cavitt

> >

> >   return percentage;

> >  }

> >

> > --

> > 2.43.0

> >

> >

> >
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/igt-dev/attachments/20250725/8dcf23d2/attachment-0001.htm>