From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?windows-1252?Q?Christian_K=F6nig?= Subject: Re: [PATCH 4/5] drm/radeon: add userptr flag to register MMU notifier v3 Date: Wed, 06 Aug 2014 17:23:15 +0200 Message-ID: <53E24863.3080000@vodafone.de> References: <1407254732-8332-1-git-send-email-deathsimple@vodafone.de> <1407254732-8332-4-git-send-email-deathsimple@vodafone.de> <20140806151628.GA2960@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252"; Format="flowed" Content-Transfer-Encoding: quoted-printable Return-path: Received: from pegasos-out.vodafone.de (pegasos-out.vodafone.de [80.84.1.38]) by gabe.freedesktop.org (Postfix) with ESMTP id EA9B36E5CC for ; Wed, 6 Aug 2014 08:23:57 -0700 (PDT) Received: from localhost (localhost.localdomain [127.0.0.1]) by pegasos-out.vodafone.de (Rohrpostix1 Daemon) with ESMTP id 463F9260913 for ; Wed, 6 Aug 2014 17:23:56 +0200 (CEST) Received: from pegasos-out.vodafone.de ([127.0.0.1]) by localhost (rohrpostix1.prod.vfnet.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id RI1z8P5KdwOY for ; Wed, 6 Aug 2014 17:23:33 +0200 (CEST) In-Reply-To: <20140806151628.GA2960@gmail.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Jerome Glisse Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org Am 06.08.2014 um 17:16 schrieb Jerome Glisse: > 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 ? The MMU notifier is registered on the first userptr bo that uses it. Using the notifier is only optional for readonly BOs because it has = quite an overhead and most uses of userptr are snapshot like uses = anyway. E.g. buffer uploads that just needs the data from that user = address at a specific point in time and are not interested in further = updates are created without registering the notifier. Christian. > >> 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/Ma= kefile >> 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/ra= deon.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, = gem_base) >> = >> @@ -1725,6 +1729,11 @@ void radeon_test_ring_sync(struct radeon_device *= rdev, >> 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/ra= deon/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/radeo= n/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 obtainin= g 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, EXPR= ESS OR >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABIL= ITY, >> + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT S= HALL >> + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR AN= Y CLAIM, >> + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR >> + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE O= R 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 porti= ons >> + * 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 obje= ct. >> + */ >> +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/ra= deon/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_ob= ject *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