<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> <meta name="Generator" content="Microsoft Word 15 (filtered medium)"> <style></style> </head> <body lang="ZH-CN" link="blue" vlink="purple"> <div class="WordSection1"> Yep, will send v3.<o:p></o:p> <o:p> </o:p> Thanks<o:p></o:p> xinhui<o:p></o:p> <o:p> </o:p> <div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0cm 0cm 0cm"> 发件人: "Koenig, Christian" <Christian.Koenig@amd.com> 日期: 2020年3月13日 星期五 21:46 收件人: "Pan, Xinhui" <Xinhui.Pan@amd.com>, "amd-gfx@lists.freedesktop.org" <amd-gfx@lists.freedesktop.org> 抄送: "Deucher, Alexander" <Alexander.Deucher@amd.com>, "Kuehling, Felix" <Felix.Kuehling@amd.com> 主题: Re: [PATCH 1/2] drm//amdgpu: Add job fence to resv conditionally<o:p></o:p> </div> <div> <o:p> </o:p> </div> <div> 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:<o:p></o:p> </div> <blockquote style="margin-top:5.0pt;margin-bottom:5.0pt"> [AMD Official Use Only - Internal Distribution Only]<o:p></o:p> <o:p> </o:p> <div> <div> page table BOs share same resv.It should be ok using any of them, root bo resv or bo resv.<o:p></o:p> </div> <div> I forgot to unref bos which cause problems. not good at rebasing...<o:p></o:p> </div> <div> <o:p> </o:p> </div> <div> <o:p> </o:p> </div> <div class="MsoNormal" align="center" style="text-align:center"> <hr size="0" width="100%" align="center"> </div> <div id="divRplyFwdMsg"> From: Koenig, Christian <a href="mailto:Christian.Koenig@amd.com"><Christian.Koenig@amd.com></a> Sent: Friday, March 13, 2020 9:34:42 PM To: Pan, Xinhui <a href="mailto:Xinhui.Pan@amd.com"><Xinhui.Pan@amd.com></a>; <a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> <a href="mailto:amd-gfx@lists.freedesktop.org"> <amd-gfx@lists.freedesktop.org></a> Cc: Deucher, Alexander <a href="mailto:Alexander.Deucher@amd.com"><Alexander.Deucher@amd.com></a>; Kuehling, Felix <a href="mailto:Felix.Kuehling@amd.com"><Felix.Kuehling@amd.com></a> Subject: Re: [PATCH 1/2] drm//amdgpu: Add job fence to resv conditionally <o:p></o:p> <div> <o:p></o:p> </div> </div> <div> <div> 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 <a href="mailto:christian.koenig@amd.com"><christian.koenig@amd.com></a> > Cc: Alex Deucher <a href="mailto:alexander.deucher@amd.com"><alexander.deucher@amd.com></a> > Cc: Felix Kuehling <a href="mailto:Felix.Kuehling@amd.com"><Felix.Kuehling@amd.com></a> > Suggested-by: Christian König <a href="mailto:christian.koenig@amd.com"><christian.koenig@amd.com></a> > Signed-off-by: xinhui pan <a href="mailto:xinhui.pan@amd.com"><xinhui.pan@amd.com></a> > --- > 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);<o:p></o:p> </div> </div> </div> </blockquote> <o:p></o:p> </div> </body> </html>