[AMD Official Use Only - Internal Distribution Only]
page table BOs share same resv.It should be ok using any of them, root bo resv or bo resv.
I forgot to unref bos which cause problems. not good at rebasing...
From: Koenig, Christian <Christian.Koenig@xxxxxxx>
Sent: Friday, March 13, 2020 9:34:42 PM
To: Pan, Xinhui <Xinhui.Pan@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>
Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Kuehling, Felix <Felix.Kuehling@xxxxxxx>
Subject: Re: [PATCH 1/2] drm//amdgpu: Add job fence to resv conditionally
Sent: Friday, March 13, 2020 9:34:42 PM
To: Pan, Xinhui <Xinhui.Pan@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>
Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Kuehling, Felix <Felix.Kuehling@xxxxxxx>
Subject: Re: [PATCH 1/2] drm//amdgpu: Add job fence to resv conditionally
Am 13.03.20 um 12:53 schrieb xinhui pan:
> If a job need sync the bo resv, it is likely that bo need the job fence
> to sync with others.
That won't work because this is the wrong resv object :)
You added the fence to the mapped BO and not the page table.
No wonder that this doesn't work,
Christian.
>
> Cc: Christian König <christian.koenig@xxxxxxx>
> Cc: Alex Deucher <alexander.deucher@xxxxxxx>
> Cc: Felix Kuehling <Felix.Kuehling@xxxxxxx>
> Suggested-by: Christian König <christian.koenig@xxxxxxx>
> Signed-off-by: xinhui pan <xinhui.pan@xxxxxxx>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 5 +++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 9 +++++++++
> 2 files changed, 14 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index b5705fcfc935..ca6021b4200b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -226,6 +226,11 @@ struct amdgpu_vm_update_params {
> * @num_dw_left: number of dw left for the IB
> */
> unsigned int num_dw_left;
> +
> + /**
> + * @resv: sync the resv and add job fence to it conditionally.
> + */
> + struct dma_resv *resv;
> };
>
> struct amdgpu_vm_update_funcs {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
> index 4cc7881f438c..0cfac59bff36 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
> @@ -70,6 +70,8 @@ static int amdgpu_vm_sdma_prepare(struct amdgpu_vm_update_params *p,
>
> p->num_dw_left = ndw;
>
> + p->resv = resv;
> +
> if (!resv)
> return 0;
>
> @@ -111,6 +113,13 @@ static int amdgpu_vm_sdma_commit(struct amdgpu_vm_update_params *p,
> swap(p->vm->last_delayed, tmp);
> dma_fence_put(tmp);
>
> + /* add job fence to resv.
> + * MM notifier path is an exception as we can not grab the
> + * resv lock.
> + */
> + if (!p->direct && p->resv)
> + dma_resv_add_shared_fence(p->resv, f);
> +
> if (fence && !p->direct)
> swap(*fence, f);
> dma_fence_put(f);
> If a job need sync the bo resv, it is likely that bo need the job fence
> to sync with others.
That won't work because this is the wrong resv object :)
You added the fence to the mapped BO and not the page table.
No wonder that this doesn't work,
Christian.
>
> Cc: Christian König <christian.koenig@xxxxxxx>
> Cc: Alex Deucher <alexander.deucher@xxxxxxx>
> Cc: Felix Kuehling <Felix.Kuehling@xxxxxxx>
> Suggested-by: Christian König <christian.koenig@xxxxxxx>
> Signed-off-by: xinhui pan <xinhui.pan@xxxxxxx>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 5 +++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 9 +++++++++
> 2 files changed, 14 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index b5705fcfc935..ca6021b4200b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -226,6 +226,11 @@ struct amdgpu_vm_update_params {
> * @num_dw_left: number of dw left for the IB
> */
> unsigned int num_dw_left;
> +
> + /**
> + * @resv: sync the resv and add job fence to it conditionally.
> + */
> + struct dma_resv *resv;
> };
>
> struct amdgpu_vm_update_funcs {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
> index 4cc7881f438c..0cfac59bff36 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
> @@ -70,6 +70,8 @@ static int amdgpu_vm_sdma_prepare(struct amdgpu_vm_update_params *p,
>
> p->num_dw_left = ndw;
>
> + p->resv = resv;
> +
> if (!resv)
> return 0;
>
> @@ -111,6 +113,13 @@ static int amdgpu_vm_sdma_commit(struct amdgpu_vm_update_params *p,
> swap(p->vm->last_delayed, tmp);
> dma_fence_put(tmp);
>
> + /* add job fence to resv.
> + * MM notifier path is an exception as we can not grab the
> + * resv lock.
> + */
> + if (!p->direct && p->resv)
> + dma_resv_add_shared_fence(p->resv, f);
> +
> if (fence && !p->direct)
> swap(*fence, f);
> dma_fence_put(f);
_______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx