Re: [PATCH] drm/amdgpu: always emit the job vm fence

["Timur Kristóf" <timur.kristof@gmail.com>]

On 2026. június 5., péntek 11:24:45 közép-európai nyári idő Christian König 
wrote:
> On 6/3/26 21:45, Alex Deucher wrote:
> > We need the fence to reemit the gds switch or spm update
> > after a queue reset.
> > 
> > Fixes: a17ef941212b ("drm/amdgpu: rework ring reset backup and reemit v9")
> > Cc: timur.kristof@gmail.com
> > Cc: christian.koenig@amd.com
> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> 
> That was avoided because it means another entry in the EOP ring buffer which
> can be bad for performance.
> 
> But correctness is obviously more important, just to keep in mind when we
> suddenly see 1% fps decrease and don't know where it's coming from.
> 
> Reviewed-by: Christian König <christian.koenig@amd.com>

Thanks Christian, that's a valid point.
Do you think we should worry about an actual perf impact here?

If we want to avoid adding an extra fence, then an alternative solution could 
be to include the emitted commands in the ib_wptr of the job's own fence when 
a VM fence was not emitted.

What do you guys think?

Thanks,
Timur

> 
> > ---
> > 
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 10 ++++------
> >  1 file changed, 4 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index
> > 2f3470208829e..7e0e2281719b1 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > @@ -853,12 +853,10 @@ void amdgpu_vm_flush(struct amdgpu_ring *ring,
> > struct amdgpu_job *job,> 
> >  					    job->oa_size);
> >  	
> >  	}
> > 
> > -	if (vm_flush_needed || pasid_mapping_needed || 
cleaner_shader_needed) {
> > -		amdgpu_fence_emit(ring, job->hw_vm_fence, 0);
> > -		fence = &job->hw_vm_fence->base;
> > -		/* get a ref for the job */
> > -		dma_fence_get(fence);
> > -	}
> > +	amdgpu_fence_emit(ring, job->hw_vm_fence, 0);
> > +	fence = &job->hw_vm_fence->base;
> > +	/* get a ref for the job */
> > +	dma_fence_get(fence);
> > 
> >  	if (vm_flush_needed) {
> >  	
> >  		mutex_lock(&id_mgr->lock);