Re: [PATCH v2 7/8] drm/xe: Use DRM_GPU_SCHED_STAT_NO_HANG to skip the reset

[Philipp Stanner <phasta@mailbox.org>]

On Fri, 2025-05-30 at 11:01 -0300, Maíra Canal wrote:
> Xe can skip the reset if TDR has fired before the free job worker and
> can
> also re-arm the timeout timer in some scenarios. Instead of using the
> scheduler internals to add the job to the pending list, use the
> DRM_GPU_SCHED_STAT_NO_HANG status to skip the reset and re-arm the
> timer.
> 
> Note that, in the first case, there is no need to restart submission
> if it
> hasn't been stopped.
> 
> Signed-off-by: Maíra Canal <mcanal@igalia.com>
> ---
>  drivers/gpu/drm/xe/xe_guc_submit.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c
> b/drivers/gpu/drm/xe/xe_guc_submit.c
> index
> 98363d688cbbf884e17e6610366202a3372f5fe0..0149c85aa1a85b2b2e739774310
> d7b3265e33228 100644
> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> @@ -1067,12 +1067,8 @@ guc_exec_queue_timedout_job(struct
> drm_sched_job *drm_job)
>  	 * list so job can be freed and kick scheduler ensuring free
> job is not
>  	 * lost.
>  	 */
> -	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &job->fence-
> >flags)) {
> -		xe_sched_add_pending_job(sched, job);
> -		xe_sched_submission_start(sched);
> -
> -		return DRM_GPU_SCHED_STAT_RESET;
> -	}
> +	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &job->fence-
> >flags))
> +		return DRM_GPU_SCHED_STAT_NO_HANG;
>  
>  	/* Kill the run_job entry point */
>  	xe_sched_submission_stop(sched);
> @@ -1247,10 +1243,8 @@ guc_exec_queue_timedout_job(struct
> drm_sched_job *drm_job)
>  	 * but there is not currently an easy way to do in DRM
> scheduler. With
>  	 * some thought, do this in a follow up.
>  	 */
> -	xe_sched_add_pending_job(sched, job);
>  	xe_sched_submission_start(sched);
> -
> -	return DRM_GPU_SCHED_STAT_RESET;
> +	return DRM_GPU_SCHED_STAT_NO_HANG;
>  }

This patch removes two of three uses of xe_sched_add_pending_job().
There's now only one left, in the same function, slightly above.

@Matthew, can that call be removed, too? Should that be done in this
patch or seperately?

P.

>  
>  static void __guc_exec_queue_fini_async(struct work_struct *w)
>