Re: [PATCH v7 05/32] drm/xe: Don't change LRC ring head on job resubmission

[Matthew Brost <matthew.brost@intel.com>]

On Tue, Oct 07, 2025 at 01:29:58PM +0100, Matthew Auld wrote:
> On 07/10/2025 12:26, Matthew Brost wrote:
> > Now that we save the job's head during submission, it's no longer
> > necessary to adjust the LRC ring head during resubmission. Instead, a
> > software-based adjustment of the tail will overwrite the old jobs in
> > place. For some odd reason, adjusting the LRC ring head didn't work on
> > parallel queues, which was causing issues in our CI.
> > 
> > v5:
> >   - Add comment in guc_exec_queue_start explaning why the function works
> >     (Auld)
> > v7:
> >   - Only adjust first state on first unsignaled job (Auld)
> > 
> > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > Reviewed-by: Tomasz Lis <tomasz.lis@intel.com>
> > ---
> >   drivers/gpu/drm/xe/xe_gpu_scheduler.h | 21 +++++++++++++++++----
> >   drivers/gpu/drm/xe/xe_guc_submit.c    | 18 ++++++++++++++++--
> >   2 files changed, 33 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_gpu_scheduler.h b/drivers/gpu/drm/xe/xe_gpu_scheduler.h
> > index e548b2aed95a..3a9ff78d9346 100644
> > --- a/drivers/gpu/drm/xe/xe_gpu_scheduler.h
> > +++ b/drivers/gpu/drm/xe/xe_gpu_scheduler.h
> > @@ -77,17 +77,30 @@ static inline void xe_sched_add_pending_job(struct xe_gpu_scheduler *sched,
> >   	spin_unlock(&sched->base.job_list_lock);
> >   }
> > +/**
> > + * xe_sched_first_pending_job() - Find first pending job which is unsignaled
> > + * @sched: Xe GPU scheduler
> > + *
> > + * Return first unsignaled job in pending list or NULL
> > + */
> >   static inline
> >   struct xe_sched_job *xe_sched_first_pending_job(struct xe_gpu_scheduler *sched)
> >   {
> > -	struct xe_sched_job *job;
> > +	struct xe_sched_job *job, *r_job = NULL;
> >   	spin_lock(&sched->base.job_list_lock);
> > -	job = list_first_entry_or_null(&sched->base.pending_list,
> > -				       struct xe_sched_job, drm.list);
> > +	list_for_each_entry(job, &sched->base.pending_list, drm.list) {
> > +		struct drm_sched_fence *s_fence = job->drm.s_fence;
> > +		struct dma_fence *hw_fence = s_fence->parent;
> > +
> > +		if (hw_fence && !dma_fence_is_signaled(hw_fence)) {
> > +			r_job = job;
> > +			break;
> > +		}
> > +	}
> >   	spin_unlock(&sched->base.job_list_lock);
> 
> I think it might make sense to split this change out, since it also changes
> the behaviour of the other existing caller, and that might want to be
> treated as a potential fix. Or commit needs to be reworded to make this more
> clear.
> 

I'll break it out into its own patch as I need to respin for CI now.

I don't think I'll include a fixes tag as GT resets shouldn't ever be
getting triggered by end users unless our hardware or sw stack is
broken.

Matt

> Otherwise lgtm.
> 
> > -	return job;
> > +	return r_job;
> >   }
> >   static inline int
> > diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> > index 3a534d93505f..d123bdb63369 100644
> > --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> > +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> > @@ -2008,11 +2008,25 @@ static void guc_exec_queue_start(struct xe_exec_queue *q)
> >   	struct xe_gpu_scheduler *sched = &q->guc->sched;
> >   	if (!exec_queue_killed_or_banned_or_wedged(q)) {
> > +		struct xe_sched_job *job = xe_sched_first_pending_job(sched);
> >   		int i;
> >   		trace_xe_exec_queue_resubmit(q);
> > -		for (i = 0; i < q->width; ++i)
> > -			xe_lrc_set_ring_head(q->lrc[i], q->lrc[i]->ring.tail);
> > +		if (job) {
> > +			for (i = 0; i < q->width; ++i) {
> > +				/*
> > +				 * The GuC context is unregistered at this point
> > +				 * time, adjusting software ring tail ensures
> > +				 * jobs are rewritten in original placement,
> > +				 * adjusting LRC tail ensures the newly loaded
> > +				 * GuC / contexts only view the LRC tail
> > +				 * increasing as jobs are written out.
> > +				 */
> > +				q->lrc[i]->ring.tail = job->ptrs[i].head;
> > +				xe_lrc_set_ring_tail(q->lrc[i],
> > +						     xe_lrc_ring_head(q->lrc[i]));
> > +			}
> > +		}
> >   		xe_sched_resubmit_jobs(sched);
> >   	}
>