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

[Purkait, Soham]

Hi Jonathan,

On 25-07-2025 02:51, Cavitt, Jonathan wrote:
> -----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?
What could be the possible usages of the raw values ? Thanks, Soham
>
> (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/fd84f9aa/attachment.htm>