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

[Cavitt, Jonathan]

-----Original Message-----
From: igt-dev <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; Gupta, Anshuman <anshuman.gupta at intel.com>
Cc: Purkait, Soham <soham.purkait at intel.com>; Tauro, Riana <riana.tauro at intel.com>; De Marchi, Lucas <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>


> out-of-bound values that could lead to undefined behavior during runtime.
> 
> Signed-off-by: Soham Purkait <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));
"""

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?

(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
> 
>