[PATCH] drm/i915/gt: Ensure sleep calculations in wait_for_freq() do not use negative value of timeout_ms

[Andi Shyti]

Hi Krzysztof,

...

> Change the type of "sleep" variable to unsigned and ensure it
> does not use "timeout_ms", when it is a negative value in the
> calculations.

ehm... if it does, though, it's a different, bigger problem :-)

> Signed-off-by: Krzysztof Karas <krzysztof.karas at intel.com>
> ---
>  drivers/gpu/drm/i915/gt/selftest_rps.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/selftest_rps.c b/drivers/gpu/drm/i915/gt/selftest_rps.c
> index 73bc91c6ea07..20ec7c0c94dc 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_rps.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_rps.c
> @@ -152,7 +152,7 @@ static u8 wait_for_freq(struct intel_rps *rps, u8 freq, int timeout_ms)
>  {
>  	u8 history[64], i;
>  	unsigned long end;
> -	int sleep;
> +	unsigned int sleep;

I'm not a big fan of this change.

>  	i = 0;
>  	memset(history, freq, sizeof(history));
> @@ -180,7 +180,7 @@ static u8 wait_for_freq(struct intel_rps *rps, u8 freq, int timeout_ms)

How about doing at this point:

	if (!timeout_ms)
		return act;

Maybe the caller doesn't want to wait (even though, as Jani said
this validation is a bit overengineered, but I would still accept
it).

If you really want, you could add it at the very beginning to
avoid extra operations.

Andi

>  		usleep_range(sleep, 2 * sleep);
>  		sleep *= 2;
> -		if (sleep > timeout_ms * 20)
> +		if (sleep > timeout_ms * 20 && timeout_ms > 0)
>  			sleep = timeout_ms * 20;
>  	} while (1);
>  }
> -- 
> 2.34.1
> 
> -- 
> Best Regards,
> Krzysztof