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 <[email protected]> > > --- > > 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
