Re: [PATCH v3 3/3] drm/xe: Move LRC seqno to system memory to avoid slow dGPU reads

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

On Thu, 2026-02-26 at 09:26 -0800, Matthew Brost wrote:
> On Thu, Feb 26, 2026 at 09:11:20AM -0800, Matthew Brost wrote:
> 
> Missed a comment.
> 
> > 

8<------------------------

> > > > struct xe_hw_engine *hwe,
> > > >  	if (IS_ERR(lrc->bo))
> > > >  		return PTR_ERR(lrc->bo);
> > > >  
> > > > +	seqno_bo = xe_bo_create_pin_map_novm(xe, tile,
> > > > PAGE_SIZE,
> > > > +					    
> > > > ttm_bo_type_kernel,
> > > > +					     XE_BO_FLAG_GGTT |
> > > > +					    
> > > > XE_BO_FLAG_GGTT_INVALIDATE |
> > > > +					    
> > > > XE_BO_FLAG_SYSTEM,
> > > > false);
> > > 
> > > XE_BO_FLAG_PINNED_NORESTORE?
> > > 
> 
> Maybe (?), but this seems dangerous… Can’t fences be pending during
> hibernate? We also check whether a job has started (by looking at the
> start seqno) in the TDR, and if the seqno is in VRAM, nonsensical
> reads
> could confuse those checks. Also consider the case where the fence
> seqno
> is clobbered—we could end up with values in memory that indicate the
> next job we run is already signaled.
> 
> So after typing this out, I actually think the answer is no to this
> flag.
> 
> Matt

OK, I wasn't sure exactly what happens on suspend / resume with LR
jobs. The !LR jobs are idled AFAIR.

We have an issure registered somewhere with LR Jobs WRT Suspend /
Resume, since if we change the spinner to preemptible in
xe_exec_compute_mode@lr-mode-workload and suspend while it's running it
doesn't complete on resume, even if the VM gets properly preempted
during the VRAM eviction.

But that's ofc beyond this patch.

With the IS_ERR() fix,
Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>


> 
> > > Thanks,
> > > Thomas
> > > 
> > > 
> > > > +	if (IS_ERR(seqno_bo)) {
> > > > +		err = PTR_ERR(lrc->bo);
> > > > +		goto err_lrc_finish;
> > > > +	}
> > > > +	lrc->seqno_bo = seqno_bo;
> > > > +
> > > >  	xe_hw_fence_ctx_init(&lrc->fence_ctx, hwe->gt,
> > > >  			     hwe->fence_irq, hwe->name);
> > > >  
> > > > diff --git a/drivers/gpu/drm/xe/xe_lrc_types.h
> > > > b/drivers/gpu/drm/xe/xe_lrc_types.h
> > > > index a4373d280c39..5a718f759ed6 100644
> > > > --- a/drivers/gpu/drm/xe/xe_lrc_types.h
> > > > +++ b/drivers/gpu/drm/xe/xe_lrc_types.h
> > > > @@ -22,6 +22,12 @@ struct xe_lrc {
> > > >  	 */
> > > >  	struct xe_bo *bo;
> > > >  
> > > > +	/**
> > > > +	 * @seqno_bo: Buffer object (memory) for seqno
> > > > numbers.
> > > > Always in system
> > > > +	 * memory as this a CPU read, GPU write path object.
> > > > +	 */
> > > > +	struct xe_bo *seqno_bo;
> > > > +
> > > >  	/** @size: size of the lrc and optional indirect ring
> > > > state
> > > > */
> > > >  	u32 size;
> > > >