From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jerome Glisse Subject: Re: [PATCH 4/5] drm/radeon: add userptr flag to register MMU notifier v3 Date: Wed, 6 Aug 2014 11:16:30 -0400 Message-ID: <20140806151628.GA2960@gmail.com> References: <1407254732-8332-1-git-send-email-deathsimple@vodafone.de> <1407254732-8332-4-git-send-email-deathsimple@vodafone.de> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-qa0-f48.google.com (mail-qa0-f48.google.com [209.85.216.48]) by gabe.freedesktop.org (Postfix) with ESMTP id BDBD36E694 for ; Wed, 6 Aug 2014 08:16:28 -0700 (PDT) Received: by mail-qa0-f48.google.com with SMTP id m5so2563112qaj.21 for ; Wed, 06 Aug 2014 08:16:28 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1407254732-8332-4-git-send-email-deathsimple@vodafone.de> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Christian =?iso-8859-1?Q?K=F6nig?= Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On Tue, Aug 05, 2014 at 06:05:31PM +0200, Christian K=F6nig wrote: > From: Christian K=F6nig > = > Whenever userspace mapping related to our userptr change > we wait for it to become idle and unmap it from GTT. > = > v2: rebased, fix mutex unlock in error path > v3: improve commit message Why in hell do you not register the mmu_notifier on first userptr bo ? > = > Signed-off-by: Christian K=F6nig > --- > drivers/gpu/drm/Kconfig | 1 + > drivers/gpu/drm/radeon/Makefile | 2 +- > drivers/gpu/drm/radeon/radeon.h | 12 ++ > drivers/gpu/drm/radeon/radeon_device.c | 2 + > drivers/gpu/drm/radeon/radeon_gem.c | 9 +- > drivers/gpu/drm/radeon/radeon_mn.c | 272 +++++++++++++++++++++++++++= ++++++ > drivers/gpu/drm/radeon/radeon_object.c | 1 + > include/uapi/drm/radeon_drm.h | 1 + > 8 files changed, 298 insertions(+), 2 deletions(-) > create mode 100644 drivers/gpu/drm/radeon/radeon_mn.c > = > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index 9b2eedc..2745284 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -115,6 +115,7 @@ config DRM_RADEON > select HWMON > select BACKLIGHT_CLASS_DEVICE > select INTERVAL_TREE > + select MMU_NOTIFIER > help > Choose this option if you have an ATI Radeon graphics card. There > are both PCI and AGP versions. You don't need to choose this to > diff --git a/drivers/gpu/drm/radeon/Makefile b/drivers/gpu/drm/radeon/Mak= efile > index 0013ad0..c7fa1ae 100644 > --- a/drivers/gpu/drm/radeon/Makefile > +++ b/drivers/gpu/drm/radeon/Makefile > @@ -80,7 +80,7 @@ radeon-y +=3D radeon_device.o radeon_asic.o radeon_kms.= o \ > r600_dpm.o rs780_dpm.o rv6xx_dpm.o rv770_dpm.o rv730_dpm.o rv740_dpm.o \ > rv770_smc.o cypress_dpm.o btc_dpm.o sumo_dpm.o sumo_smc.o trinity_dpm.o= \ > trinity_smc.o ni_dpm.o si_smc.o si_dpm.o kv_smc.o kv_dpm.o ci_smc.o \ > - ci_dpm.o dce6_afmt.o radeon_vm.o radeon_ucode.o radeon_ib.o > + ci_dpm.o dce6_afmt.o radeon_vm.o radeon_ucode.o radeon_ib.o radeon_mn.o > = > # add async DMA block > radeon-y +=3D \ > diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/rad= eon.h > index 3c6999e..511191f 100644 > --- a/drivers/gpu/drm/radeon/radeon.h > +++ b/drivers/gpu/drm/radeon/radeon.h > @@ -65,6 +65,7 @@ > #include > #include > #include > +#include > = > #include > #include > @@ -487,6 +488,9 @@ struct radeon_bo { > = > struct ttm_bo_kmap_obj dma_buf_vmap; > pid_t pid; > + > + struct radeon_mn *mn; > + struct interval_tree_node mn_it; > }; > #define gem_to_radeon_bo(gobj) container_of((gobj), struct radeon_bo, ge= m_base) > = > @@ -1725,6 +1729,11 @@ void radeon_test_ring_sync(struct radeon_device *r= dev, > struct radeon_ring *cpB); > void radeon_test_syncing(struct radeon_device *rdev); > = > +/* > + * MMU Notifier > + */ > +int radeon_mn_register(struct radeon_bo *bo, unsigned long addr); > +void radeon_mn_unregister(struct radeon_bo *bo); > = > /* > * Debugfs > @@ -2372,6 +2381,9 @@ struct radeon_device { > /* tracking pinned memory */ > u64 vram_pin_size; > u64 gart_pin_size; > + > + struct mutex mn_lock; > + DECLARE_HASHTABLE(mn_hash, 7); > }; > = > bool radeon_is_px(struct drm_device *dev); > diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/rad= eon/radeon_device.c > index c8ea050..c58f84f 100644 > --- a/drivers/gpu/drm/radeon/radeon_device.c > +++ b/drivers/gpu/drm/radeon/radeon_device.c > @@ -1270,6 +1270,8 @@ int radeon_device_init(struct radeon_device *rdev, > init_rwsem(&rdev->pm.mclk_lock); > init_rwsem(&rdev->exclusive_lock); > init_waitqueue_head(&rdev->irq.vblank_queue); > + mutex_init(&rdev->mn_lock); > + hash_init(rdev->mn_hash); > r =3D radeon_gem_init(rdev); > if (r) > return r; > diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon= /radeon_gem.c > index 4506560..2a6fbf1 100644 > --- a/drivers/gpu/drm/radeon/radeon_gem.c > +++ b/drivers/gpu/drm/radeon/radeon_gem.c > @@ -291,7 +291,8 @@ int radeon_gem_userptr_ioctl(struct drm_device *dev, = void *data, > = > /* reject unknown flag values */ > if (args->flags & ~(RADEON_GEM_USERPTR_READONLY | > - RADEON_GEM_USERPTR_ANONONLY | RADEON_GEM_USERPTR_VALIDATE)) > + RADEON_GEM_USERPTR_ANONONLY | RADEON_GEM_USERPTR_VALIDATE | > + RADEON_GEM_USERPTR_REGISTER)) > return -EINVAL; > = > /* readonly pages not tested on older hardware */ > @@ -312,6 +313,12 @@ int radeon_gem_userptr_ioctl(struct drm_device *dev,= void *data, > if (r) > goto release_object; > = > + if (args->flags & RADEON_GEM_USERPTR_REGISTER) { > + r =3D radeon_mn_register(bo, args->addr); > + if (r) > + goto release_object; > + } > + > if (args->flags & RADEON_GEM_USERPTR_VALIDATE) { > down_read(¤t->mm->mmap_sem); > r =3D radeon_bo_reserve(bo, true); > diff --git a/drivers/gpu/drm/radeon/radeon_mn.c b/drivers/gpu/drm/radeon/= radeon_mn.c > new file mode 100644 > index 0000000..0157bc2 > --- /dev/null > +++ b/drivers/gpu/drm/radeon/radeon_mn.c > @@ -0,0 +1,272 @@ > +/* > + * Copyright 2014 Advanced Micro Devices, Inc. > + * All Rights Reserved. > + * > + * Permission is hereby granted, free of charge, to any person obtaining= a > + * copy of this software and associated documentation files (the > + * "Software"), to deal in the Software without restriction, including > + * without limitation the rights to use, copy, modify, merge, publish, > + * distribute, sub license, and/or sell copies of the Software, and to > + * permit persons to whom the Software is furnished to do so, subject to > + * the following conditions: > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRE= SS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILI= TY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SH= ALL > + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY= CLAIM, > + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR > + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR= THE > + * USE OR OTHER DEALINGS IN THE SOFTWARE. > + * > + * The above copyright notice and this permission notice (including the > + * next paragraph) shall be included in all copies or substantial portio= ns > + * of the Software. > + * > + */ > +/* > + * Authors: > + * Christian K=F6nig > + */ > + > +#include > +#include > +#include > +#include > +#include > + > +#include "radeon.h" > + > +struct radeon_mn { > + /* constant after initialisation */ > + struct radeon_device *rdev; > + struct mm_struct *mm; > + struct mmu_notifier mn; > + > + /* only used on destruction */ > + struct work_struct work; > + > + /* protected by rdev->mn_lock */ > + struct hlist_node node; > + > + /* objects protected by lock */ > + struct mutex lock; > + struct rb_root objects; > +}; > + > +/** > + * radeon_mn_destroy - destroy the rmn > + * > + * @work: previously sheduled work item > + * > + * Lazy destroys the notifier from a work item > + */ > +static void radeon_mn_destroy(struct work_struct *work) > +{ > + struct radeon_mn *rmn =3D container_of(work, struct radeon_mn, work); > + struct radeon_device *rdev =3D rmn->rdev; > + struct radeon_bo *bo, *next; > + > + mutex_lock(&rdev->mn_lock); > + mutex_lock(&rmn->lock); > + hash_del(&rmn->node); > + rbtree_postorder_for_each_entry_safe(bo, next, &rmn->objects, mn_it.rb)= { > + interval_tree_remove(&bo->mn_it, &rmn->objects); > + bo->mn =3D NULL; > + } > + mutex_unlock(&rmn->lock); > + mutex_unlock(&rdev->mn_lock); > + mmu_notifier_unregister(&rmn->mn, rmn->mm); > + kfree(rmn); > +} > + > +/** > + * radeon_mn_release - callback to notify about mm destruction > + * > + * @mn: our notifier > + * @mn: the mm this callback is about > + * > + * Shedule a work item to lazy destroy our notifier. > + */ > +static void radeon_mn_release(struct mmu_notifier *mn, > + struct mm_struct *mm) > +{ > + struct radeon_mn *rmn =3D container_of(mn, struct radeon_mn, mn); > + INIT_WORK(&rmn->work, radeon_mn_destroy); > + schedule_work(&rmn->work); > +} > + > +/** > + * radeon_mn_invalidate_range_start - callback to notify about mm change > + * > + * @mn: our notifier > + * @mn: the mm this callback is about > + * @start: start of updated range > + * @end: end of updated range > + * > + * We block for all BOs between start and end to be idle and > + * unmap them by move them into system domain again. > + */ > +static void radeon_mn_invalidate_range_start(struct mmu_notifier *mn, > + struct mm_struct *mm, > + unsigned long start, > + unsigned long end) > +{ > + struct radeon_mn *rmn =3D container_of(mn, struct radeon_mn, mn); > + struct interval_tree_node *it; > + > + /* notification is exclusive, but interval is inclusive */ > + end -=3D 1; > + > + mutex_lock(&rmn->lock); > + > + it =3D interval_tree_iter_first(&rmn->objects, start, end); > + while (it) { > + struct radeon_bo *bo; > + int r; > + > + bo =3D container_of(it, struct radeon_bo, mn_it); > + it =3D interval_tree_iter_next(it, start, end); > + > + r =3D radeon_bo_reserve(bo, true); > + if (r) { > + DRM_ERROR("(%d) failed to reserve user bo\n", r); > + continue; > + } > + > + if (bo->tbo.sync_obj) { > + r =3D radeon_fence_wait(bo->tbo.sync_obj, false); > + if (r) > + DRM_ERROR("(%d) failed to wait for user bo\n", r); > + } > + > + radeon_ttm_placement_from_domain(bo, RADEON_GEM_DOMAIN_CPU); > + r =3D ttm_bo_validate(&bo->tbo, &bo->placement, false, false); > + if (r) > + DRM_ERROR("(%d) failed to validate user bo\n", r); > + > + radeon_bo_unreserve(bo); > + } > + = > + mutex_unlock(&rmn->lock); > +} > + > +static const struct mmu_notifier_ops radeon_mn_ops =3D { > + .release =3D radeon_mn_release, > + .invalidate_range_start =3D radeon_mn_invalidate_range_start, > +}; > + > +/** > + * radeon_mn_get - create notifier context > + * > + * @rdev: radeon device pointer > + * > + * Creates a notifier context for current->mm. > + */ > +static struct radeon_mn *radeon_mn_get(struct radeon_device *rdev) > +{ > + struct mm_struct *mm =3D current->mm; > + struct radeon_mn *rmn; > + int r; > + > + down_write(&mm->mmap_sem); > + mutex_lock(&rdev->mn_lock); > + > + hash_for_each_possible(rdev->mn_hash, rmn, node, (unsigned long)mm) > + if (rmn->mm =3D=3D mm) > + goto release_locks; > + > + rmn =3D kzalloc(sizeof(*rmn), GFP_KERNEL); > + if (!rmn) { > + rmn =3D ERR_PTR(-ENOMEM); > + goto release_locks; > + } > + > + rmn->rdev =3D rdev; > + rmn->mm =3D mm; > + rmn->mn.ops =3D &radeon_mn_ops; > + mutex_init(&rmn->lock); > + rmn->objects =3D RB_ROOT; > + = > + r =3D __mmu_notifier_register(&rmn->mn, mm); > + if (r) > + goto free_rmn; > + > + hash_add(rdev->mn_hash, &rmn->node, (unsigned long)mm); > + > +release_locks: > + mutex_unlock(&rdev->mn_lock); > + up_write(&mm->mmap_sem); > + > + return rmn; > + > +free_rmn: > + mutex_unlock(&rdev->mn_lock); > + up_write(&mm->mmap_sem); > + kfree(rmn); > + > + return ERR_PTR(r); > +} > + > +/** > + * radeon_mn_register - register a BO for notifier updates > + * > + * @bo: radeon buffer object > + * @addr: userptr addr we should monitor > + * > + * Registers an MMU notifier for the given BO at the specified address. > + * Returns 0 on success, -ERRNO if anything goes wrong. > + */ > +int radeon_mn_register(struct radeon_bo *bo, unsigned long addr) > +{ > + unsigned long end =3D addr + radeon_bo_size(bo) - 1; > + struct radeon_device *rdev =3D bo->rdev; > + struct radeon_mn *rmn; > + struct interval_tree_node *it; > + > + rmn =3D radeon_mn_get(rdev); > + if (IS_ERR(rmn)) > + return PTR_ERR(rmn); > + > + mutex_lock(&rmn->lock); > + > + it =3D interval_tree_iter_first(&rmn->objects, addr, end); > + if (it) { > + mutex_unlock(&rmn->lock); > + return -EEXIST; > + } > + > + bo->mn =3D rmn; > + bo->mn_it.start =3D addr; > + bo->mn_it.last =3D end; > + interval_tree_insert(&bo->mn_it, &rmn->objects); > + > + mutex_unlock(&rmn->lock); > + > + return 0; > +} > + > +/** > + * radeon_mn_unregister - unregister a BO for notifier updates > + * > + * @bo: radeon buffer object > + * > + * Remove any registration of MMU notifier updates from the buffer objec= t. > + */ > +void radeon_mn_unregister(struct radeon_bo *bo) > +{ > + struct radeon_device *rdev =3D bo->rdev; > + struct radeon_mn *rmn; > + > + mutex_lock(&rdev->mn_lock); > + rmn =3D bo->mn; > + if (rmn =3D=3D NULL) { > + mutex_unlock(&rdev->mn_lock); > + return; > + } > + > + mutex_lock(&rmn->lock); > + interval_tree_remove(&bo->mn_it, &rmn->objects); > + bo->mn =3D NULL; > + mutex_unlock(&rmn->lock); > + mutex_unlock(&rdev->mn_lock); > +} > diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/rad= eon/radeon_object.c > index c73c1e3..2875238 100644 > --- a/drivers/gpu/drm/radeon/radeon_object.c > +++ b/drivers/gpu/drm/radeon/radeon_object.c > @@ -75,6 +75,7 @@ static void radeon_ttm_bo_destroy(struct ttm_buffer_obj= ect *tbo) > bo =3D container_of(tbo, struct radeon_bo, tbo); > = > radeon_update_memory_usage(bo, bo->tbo.mem.mem_type, -1); > + radeon_mn_unregister(bo); > = > mutex_lock(&bo->rdev->gem.mutex); > list_del_init(&bo->list); > diff --git a/include/uapi/drm/radeon_drm.h b/include/uapi/drm/radeon_drm.h > index 5dc61c2..c77495f 100644 > --- a/include/uapi/drm/radeon_drm.h > +++ b/include/uapi/drm/radeon_drm.h > @@ -818,6 +818,7 @@ struct drm_radeon_gem_create { > #define RADEON_GEM_USERPTR_READONLY (1 << 0) > #define RADEON_GEM_USERPTR_ANONONLY (1 << 1) > #define RADEON_GEM_USERPTR_VALIDATE (1 << 2) > +#define RADEON_GEM_USERPTR_REGISTER (1 << 3) > = > struct drm_radeon_gem_userptr { > uint64_t addr; > -- = > 1.9.1 > = > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel