On Thu, 6 Nov 2025 16:23:24 +0000
Steven Price <[email protected]> wrote:
> 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 <[email protected]>
> > ---
> > 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.
Yep that looks better expressed like that. I'll go for your suggestion
in the next version.
