From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?ISO-8859-1?Q?Christian_K=F6nig?= Subject: Re: [PATCH] drm/radeon: fence virtual address and free it once idle v3 Date: Sat, 04 Aug 2012 11:07:30 +0200 Message-ID: <501CE652.7080509@vodafone.de> References: <1344029208-10069-1-git-send-email-j.glisse@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1"; Format="flowed" Content-Transfer-Encoding: quoted-printable Return-path: Received: from outgoing.email.vodafone.de (outgoing.email.vodafone.de [139.7.28.128]) by gabe.freedesktop.org (Postfix) with ESMTP id DA4659F049 for ; Sat, 4 Aug 2012 02:07:34 -0700 (PDT) In-Reply-To: <1344029208-10069-1-git-send-email-j.glisse@gmail.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: j.glisse@gmail.com Cc: Jerome Glisse , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On 03.08.2012 23:26, j.glisse@gmail.com wrote: > From: Jerome Glisse > > Virtual address need to be fenced to know when we can safely remove it. > This patch also properly clear the pagetable. Previously it was > serouisly broken. > > Kernel 3.5/3.4 need a similar patch but adapted for difference in mutex l= ocking. > > v2: For to update pagetable when unbinding bo (don't bailout if > bo_va->valid is true). > v3: Add kernel 3.5/3.4 comment. > > Signed-off-by: Jerome Glisse It should fix the problem, going to test it as soon as I find some 5min = spare in my vacation. Till then it is: Reviewed-by: Christian K=F6nig In the future I would really like to make the updating of the PTE also = async, that should save us allot of problems here. Christian. > --- > drivers/gpu/drm/radeon/radeon.h | 1 + > drivers/gpu/drm/radeon/radeon_cs.c | 32 +++++++++++++++++++++++++= ++++--- > drivers/gpu/drm/radeon/radeon_gart.c | 24 ++++++++++++++++++++++-- > drivers/gpu/drm/radeon/radeon_gem.c | 13 ++----------- > drivers/gpu/drm/radeon/radeon_object.c | 6 +----- > 5 files changed, 55 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/rad= eon.h > index 5431af2..8d75c65 100644 > --- a/drivers/gpu/drm/radeon/radeon.h > +++ b/drivers/gpu/drm/radeon/radeon.h > @@ -300,6 +300,7 @@ struct radeon_bo_va { > uint64_t soffset; > uint64_t eoffset; > uint32_t flags; > + struct radeon_fence *fence; > bool valid; > }; > = > diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/= radeon_cs.c > index 8a4c49e..995f3ab 100644 > --- a/drivers/gpu/drm/radeon/radeon_cs.c > +++ b/drivers/gpu/drm/radeon/radeon_cs.c > @@ -278,6 +278,30 @@ int radeon_cs_parser_init(struct radeon_cs_parser *p= , void *data) > return 0; > } > = > +static void radeon_bo_vm_fence_va(struct radeon_cs_parser *parser, > + struct radeon_fence *fence) > +{ > + struct radeon_fpriv *fpriv =3D parser->filp->driver_priv; > + struct radeon_vm *vm =3D &fpriv->vm; > + struct radeon_bo_list *lobj; > + int r; > + > + if (parser->chunk_ib_idx =3D=3D -1) > + return; > + if ((parser->cs_flags & RADEON_CS_USE_VM) =3D=3D 0) > + return; > + > + list_for_each_entry(lobj, &parser->validated, tv.head) { > + struct radeon_bo_va *bo_va; > + struct radeon_bo *rbo =3D lobj->bo; > + > + bo_va =3D radeon_bo_va(rbo, vm); > + radeon_fence_unref(&bo_va->fence); > + bo_va->fence =3D radeon_fence_ref(fence); > + } > + return 0; > +} > + > /** > * cs_parser_fini() - clean parser states > * @parser: parser structure holding parsing context. > @@ -290,11 +314,14 @@ static void radeon_cs_parser_fini(struct radeon_cs_= parser *parser, int error) > { > unsigned i; > = > - if (!error) > + if (!error) { > + /* fence all bo va before ttm_eu_fence_buffer_objects so bo are still = reserved */ > + radeon_bo_vm_fence_va(parser, parser->ib.fence); > ttm_eu_fence_buffer_objects(&parser->validated, > parser->ib.fence); > - else > + } else { > ttm_eu_backoff_reservation(&parser->validated); > + } > = > if (parser->relocs !=3D NULL) { > for (i =3D 0; i < parser->nrelocs; i++) { > @@ -388,7 +415,6 @@ static int radeon_cs_ib_vm_chunk(struct radeon_device= *rdev, > = > if (parser->chunk_ib_idx =3D=3D -1) > return 0; > - > if ((parser->cs_flags & RADEON_CS_USE_VM) =3D=3D 0) > return 0; > = > diff --git a/drivers/gpu/drm/radeon/radeon_gart.c b/drivers/gpu/drm/radeo= n/radeon_gart.c > index b372005..9912182 100644 > --- a/drivers/gpu/drm/radeon/radeon_gart.c > +++ b/drivers/gpu/drm/radeon/radeon_gart.c > @@ -814,7 +814,7 @@ int radeon_vm_bo_update_pte(struct radeon_device *rde= v, > return -EINVAL; > } > = > - if (bo_va->valid) > + if (bo_va->valid && mem) > return 0; > = > ngpu_pages =3D radeon_bo_ngpu_pages(bo); > @@ -859,11 +859,27 @@ int radeon_vm_bo_rmv(struct radeon_device *rdev, > struct radeon_bo *bo) > { > struct radeon_bo_va *bo_va; > + int r; > = > bo_va =3D radeon_bo_va(bo, vm); > if (bo_va =3D=3D NULL) > return 0; > = > + /* wait for va use to end */ > + while (bo_va->fence) { > + r =3D radeon_fence_wait(bo_va->fence, false); > + if (r) { > + DRM_ERROR("error while waiting for fence: %d\n", r); > + } > + if (r =3D=3D -EDEADLK) { > + r =3D radeon_gpu_reset(rdev); > + if (!r) > + continue; > + } > + break; > + } > + radeon_fence_unref(&bo_va->fence); > + > mutex_lock(&rdev->vm_manager.lock); > mutex_lock(&vm->mutex); > radeon_vm_bo_update_pte(rdev, vm, bo, NULL); > @@ -952,12 +968,15 @@ void radeon_vm_fini(struct radeon_device *rdev, str= uct radeon_vm *vm) > radeon_vm_unbind_locked(rdev, vm); > mutex_unlock(&rdev->vm_manager.lock); > = > - /* remove all bo */ > + /* remove all bo at this point non are busy any more because unbind > + * waited for the last vm fence to signal > + */ > r =3D radeon_bo_reserve(rdev->ring_tmp_bo.bo, false); > if (!r) { > bo_va =3D radeon_bo_va(rdev->ring_tmp_bo.bo, vm); > list_del_init(&bo_va->bo_list); > list_del_init(&bo_va->vm_list); > + radeon_fence_unref(&bo_va->fence); > radeon_bo_unreserve(rdev->ring_tmp_bo.bo); > kfree(bo_va); > } > @@ -969,6 +988,7 @@ void radeon_vm_fini(struct radeon_device *rdev, struc= t radeon_vm *vm) > r =3D radeon_bo_reserve(bo_va->bo, false); > if (!r) { > list_del_init(&bo_va->bo_list); > + radeon_fence_unref(&bo_va->fence); > radeon_bo_unreserve(bo_va->bo); > kfree(bo_va); > } > diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon= /radeon_gem.c > index 84d0452..1b57b00 100644 > --- a/drivers/gpu/drm/radeon/radeon_gem.c > +++ b/drivers/gpu/drm/radeon/radeon_gem.c > @@ -134,25 +134,16 @@ void radeon_gem_object_close(struct drm_gem_object = *obj, > struct radeon_device *rdev =3D rbo->rdev; > struct radeon_fpriv *fpriv =3D file_priv->driver_priv; > struct radeon_vm *vm =3D &fpriv->vm; > - struct radeon_bo_va *bo_va, *tmp; > = > if (rdev->family < CHIP_CAYMAN) { > return; > } > = > if (radeon_bo_reserve(rbo, false)) { > + dev_err(rdev->dev, "leaking bo va because we fail to reserve bo\n"); > return; > } > - list_for_each_entry_safe(bo_va, tmp, &rbo->va, bo_list) { > - if (bo_va->vm =3D=3D vm) { > - /* remove from this vm address space */ > - mutex_lock(&vm->mutex); > - list_del(&bo_va->vm_list); > - mutex_unlock(&vm->mutex); > - list_del(&bo_va->bo_list); > - kfree(bo_va); > - } > - } > + radeon_vm_bo_rmv(rdev, vm, rbo); > radeon_bo_unreserve(rbo); > } > = > diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/rad= eon/radeon_object.c > index 1f1a4c8..1cb014b 100644 > --- a/drivers/gpu/drm/radeon/radeon_object.c > +++ b/drivers/gpu/drm/radeon/radeon_object.c > @@ -52,11 +52,7 @@ void radeon_bo_clear_va(struct radeon_bo *bo) > = > list_for_each_entry_safe(bo_va, tmp, &bo->va, bo_list) { > /* remove from all vm address space */ > - mutex_lock(&bo_va->vm->mutex); > - list_del(&bo_va->vm_list); > - mutex_unlock(&bo_va->vm->mutex); > - list_del(&bo_va->bo_list); > - kfree(bo_va); > + radeon_vm_bo_rmv(bo->rdev, bo_va->vm, bo); > } > } > =