On Fri, Jun 5, 2026 at 6:11 AM Timur Kristóf <[email protected]> wrote:
>
> 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: [email protected]
> > > Cc: [email protected]
> > > Signed-off-by: Alex Deucher <[email protected]>
> >
> > 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 <[email protected]>
>
> 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?

@Christian Koenig Is there a reason we need to emit gds and spm before
the vm fence if the vm fence is required?  I've sent out a series to
retain that behavior, but if it doesn't matter, we could simplify
things more and move gds and spm into the ib fence.

Alex

>
> 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);
>
>
>
>

Reply via email to