Re: [PATCH 1/1] drm/amdgpu: Wait for newly allocated PTs to be idle

[Christian König <ckoenig.leichtzumerken@xxxxxxxxx>]

All commands updating the page tables are pushed through the same 
scheduler entity.

And that one makes sure that they are all executed on the same SDMA engine.

Switching of entities between engines happens only when the entities are 
idle.

Regards,
Christian.

Am 13.03.19 um 00:36 schrieb Kuehling, Felix:
> When we use SDMA, we don't wait on the CPU. The GPU scheduler waits for
> the fences on the root PD reservation before executing the SDMA IB.
> amdgpu_vm_bo_update_mapping gets those fences and builds the sync object
> for the scheduler after all the page tables have been allocated, so it
> should be no problem.
>
> Regards,
>     Felix
>
> On 2019-03-12 6:13 p.m., Liu, Shaoyun wrote:
> > Hi,
> >
> > I think even use SDMA to update PTE we may still need to wait the clear
> > job to be completed if we can not guarantee the clear and set PTE job
> > will use the exact same SDMA engine ( Did we use a dedicate SDMA engine
> > for PTE update including clear? ).  But if we didn't use the  same
> > engine , it may explain why the  test failed occasionally.
> >
> > Regards
> >
> > shaoyun.liu
> >
> >
> >
> > On 2019-03-12 5:20 p.m., Kuehling, Felix wrote:
> > > When page table are updated by the CPU, synchronize with the
> > > allocation and initialization of newly allocated page tables.
> > >
> > > Signed-off-by: Felix Kuehling <Felix.Kuehling@xxxxxxx>
> > > ---
> > >     drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 20 +++++++++++++-------
> > >     1 file changed, 13 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > > index 8603c85..4303436 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > > @@ -899,17 +899,17 @@ static void amdgpu_vm_bo_param(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> > >     }
> > >     
> > >     /**
> > > - * amdgpu_vm_alloc_pts - Allocate page tables.
> > > + * amdgpu_vm_alloc_pts - Allocate a specific page table
> > >      *
> > >      * @adev: amdgpu_device pointer
> > >      * @vm: VM to allocate page tables for
> > > - * @saddr: Start address which needs to be allocated
> > > - * @size: Size from start address we need.
> > > + * @cursor: Which page table to allocate
> > >      *
> > > - * Make sure the page directories and page tables are allocated
> > > + * Make sure a specific page table or directory is allocated.
> > >      *
> > >      * Returns:
> > > - * 0 on success, errno otherwise.
> > > + * 1 if page table needed to be allocated, 0 if page table was already
> > > + * allocated, negative errno if an error occurred.
> > >      */
> > >     static int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
> > >     			       struct amdgpu_vm *vm,
> > > @@ -956,7 +956,7 @@ static int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
> > >     	if (r)
> > >     		goto error_free_pt;
> > >     
> > > -	return 0;
> > > +	return 1;
> > >     
> > >     error_free_pt:
> > >     	amdgpu_bo_unref(&pt->shadow);
> > > @@ -1621,10 +1621,12 @@ static int amdgpu_vm_update_ptes(struct amdgpu_pte_update_params *params,
> > >     		unsigned shift, parent_shift, mask;
> > >     		uint64_t incr, entry_end, pe_start;
> > >     		struct amdgpu_bo *pt;
> > > +		bool need_to_sync;
> > >     
> > >     		r = amdgpu_vm_alloc_pts(params->adev, params->vm, &cursor);
> > > -		if (r)
> > > +		if (r < 0)
> > >     			return r;
> > > +		need_to_sync = (r && params->vm->use_cpu_for_update);
> > >     
> > >     		pt = cursor.entry->base.bo;
> > >     
> > > @@ -1672,6 +1674,10 @@ static int amdgpu_vm_update_ptes(struct amdgpu_pte_update_params *params,
> > >     		entry_end += cursor.pfn & ~(entry_end - 1);
> > >     		entry_end = min(entry_end, end);
> > >     
> > > +		if (need_to_sync)
> > > +			r = amdgpu_bo_sync_wait(params->vm->root.base.bo,
> > > +						AMDGPU_FENCE_OWNER_VM, true);
> > > +
> > >     		do {
> > >     			uint64_t upd_end = min(entry_end, frag_end);
> > >     			unsigned nptes = (upd_end - frag_start) >> shift;
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx