[PATCH 1/2] drm//amdgpu: Add job fence to resv conditionally

[Christian König]

Yeah, but this is still the wrong resv object :)

See the object passed to amdgpu_vm_sdma_prepare() is the one of the BO 
which is mapped into the page tables and NOT the one of the page tables.

You need to use p->vm->root.base.bo->tbo.base.resv here.

Regards,
Christian.

Am 13.03.20 um 14:43 schrieb Pan, Xinhui:
>
> [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 at amd.com>
> *Sent:* Friday, March 13, 2020 9:34:42 PM
> *To:* Pan, Xinhui <Xinhui.Pan at amd.com>; amd-gfx at lists.freedesktop.org 
> <amd-gfx at lists.freedesktop.org>
> *Cc:* Deucher, Alexander <Alexander.Deucher at amd.com>; Kuehling, Felix 
> <Felix.Kuehling at amd.com>
> *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 at amd.com>
> > Cc: Alex Deucher <alexander.deucher at amd.com>
> > Cc: Felix Kuehling <Felix.Kuehling at amd.com>
> > Suggested-by: Christian König <christian.koenig at amd.com>
> > Signed-off-by: xinhui pan <xinhui.pan at amd.com>
> > ---
> >   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);
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20200313/5f453795/attachment.htm>