Re: [PATCH v1 5/8] drm/panthor: Fix immediate ticking on a disabled tick

[Steven Price <steven.price@arm.com>]

On 06/11/2025 14:46, Boris Brezillon wrote:
> We have a few path where we schedule the tick work immediately without
NIT: s/path/paths/> changing the resched_target. If the tick was
stopped, this would lead
> to a remaining_jiffies that's always > 0, and it wouldn't force a full
> tick in that case. Add extra checks to cover that case properly.
> 
> Fixes: de8548813824 ("drm/panthor: Add the scheduler logical block")
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
>  drivers/gpu/drm/panthor/panthor_sched.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
> index b6489e9ba1f0..1eba56e7360d 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -2358,8 +2358,12 @@ static void tick_work(struct work_struct *work)
>  	if (drm_WARN_ON(&ptdev->base, ret))
>  		goto out_dev_exit;
>  
> -	if (time_before64(now, sched->resched_target))
> +	if (sched->resched_target != U64_MAX &&
> +	    time_before64(now, sched->resched_target))
>  		remaining_jiffies = sched->resched_target - now;
> +	else if (sched->resched_target == U64_MAX &&
> +		 time_before64(now, sched->last_tick + sched->tick_period))
> +		remaining_jiffies = sched->last_tick + sched->tick_period - now;

I'm wondering if this would be cleaner with an extra variable (and a
comment):

   u64 resched_target = sched->resched_target;

   /* If the tick is stopped, calculate when the next tick would be */
   if (resched_target == U64_MAX)
	   resched_target = sched->last_tick + sched->tick_period;

   if (time_before64(now, resched_target)
	   remaining_jiffies = resched_target - now;

It at least avoids some repetition.

Thanks,
Steve

>  
>  	full_tick = remaining_jiffies == 0;
>