Re: [PATCH 2/4] drm/imagination: Don't timeout job if its fence has been signaled

[Brajesh Gupta <Brajesh.Gupta@imgtec.com>]

On Mon, 2026-05-18 at 15:01 +0000, Matt Coster wrote:
> Hi Brajesh,
> 
> On 12/05/2026 07:47, Brajesh Gupta wrote:
> > Check for job's fence in timedout handler and report NO HANG if its
> 
> Nit: s/its/it/
> 
Updated

> > is signaled
> 
> This could use more explanation: _why_ is returning NO_HANG in this
> instance the correct behaviour? See the kernel docs section "Describe
> your changes"[1] for inspiration.
> 
Updated

> > 
> > Signed-off-by: Brajesh Gupta <brajesh.gupta@imgtec.com>
> > ---
> >  drivers/gpu/drm/imagination/pvr_queue.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/imagination/pvr_queue.c b/drivers/gpu/drm/imagination/pvr_queue.c
> > index d10a13173f0f..073478919b22 100644
> > --- a/drivers/gpu/drm/imagination/pvr_queue.c
> > +++ b/drivers/gpu/drm/imagination/pvr_queue.c
> > @@ -851,7 +851,8 @@ static void pvr_queue_start(struct pvr_queue *queue)
> >   * the scheduler, and re-assign parent fences in the middle.
> >   *
> >   * Return:
> > - *  * DRM_GPU_SCHED_STAT_RESET.
> > + *  * Fence signaled : DRM_GPU_SCHED_STAT_NO_HANG
> > + *  * otherwise      : DRM_GPU_SCHED_STAT_RESET
> 
> The formatting here is off (and won't render nicely in the docs). I
> believe it should be something like:
> 
>    * Return:
>    *  * %DRM_GPU_SCHED_STAT_NO_HANG if the job fence has already been
>    *    signaled, or
>    *  * %DRM_gPU_SCHED_STAT_RESET otherwise.
> 
Updated

> >   */
> >  static enum drm_gpu_sched_stat
> >  pvr_queue_timedout_job(struct drm_sched_job *s_job)
> > @@ -862,6 +863,10 @@ pvr_queue_timedout_job(struct drm_sched_job *s_job)
> >         struct pvr_job *job;
> >         u32 job_count = 0;
> > 
> > +       /* If fence is already signaled then return no hang */
> 
> I don't think this comment really adds anything, the function name and
> return value are fairly descriptive already.
> 
Dropped

> Cheers,
> Matt
> 
> [1]: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
> 
> > +       if (dma_fence_is_signaled(s_job->s_fence->parent))
> > +               return DRM_GPU_SCHED_STAT_NO_HANG;
> > +
> >         dev_err(sched->dev, "Job timeout\n");
> > 
> >         /* Before we stop the scheduler, make sure the queue is out of any list, so
> > 
> > --
> > 2.43.0
> > 
> 
> 
Thanks,
Brajesh