On Tue, Aug 05, 2014 at 06:05:31PM +0200, Christian König wrote:
> From: Christian König <christian.koenig@xxxxxxx>
>
> 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önig <christian.koenig@xxxxxxx>
> ---
> 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/Makefile
> index 0013ad0..c7fa1ae 100644
> --- a/drivers/gpu/drm/radeon/Makefile
> +++ b/drivers/gpu/drm/radeon/Makefile
> @@ -80,7 +80,7 @@ radeon-y += 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 += \
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index 3c6999e..511191f 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -65,6 +65,7 @@
> #include <linux/list.h>
> #include <linux/kref.h>
> #include <linux/interval_tree.h>
> +#include <linux/hashtable.h>
>
> #include <ttm/ttm_bo_api.h>
> #include <ttm/ttm_bo_driver.h>
> @@ -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/radeon/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 = 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 = 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 = 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, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
> + * 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 portions
> + * of the Software.
> + *
> + */
> +/*
> + * Authors:
> + * Christian König <christian.koenig@xxxxxxx>
> + */
> +
> +#include <linux/firmware.h>
> +#include <linux/module.h>
> +#include <linux/mmu_notifier.h>
> +#include <drm/drmP.h>
> +#include <drm/drm.h>
> +
> +#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 = container_of(work, struct radeon_mn, work);
> + struct radeon_device *rdev = 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 = 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 = 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 = container_of(mn, struct radeon_mn, mn);
> + struct interval_tree_node *it;
> +
> + /* notification is exclusive, but interval is inclusive */
> + end -= 1;
> +
> + mutex_lock(&rmn->lock);
> +
> + it = interval_tree_iter_first(&rmn->objects, start, end);
> + while (it) {
> + struct radeon_bo *bo;
> + int r;
> +
> + bo = container_of(it, struct radeon_bo, mn_it);
> + it = interval_tree_iter_next(it, start, end);
> +
> + r = radeon_bo_reserve(bo, true);
> + if (r) {
> + DRM_ERROR("(%d) failed to reserve user bo\n", r);
> + continue;
> + }
> +
> + if (bo->tbo.sync_obj) {
> + r = 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 = 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 = {
> + .release = radeon_mn_release,
> + .invalidate_range_start = 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 = 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 == mm)
> + goto release_locks;
> +
> + rmn = kzalloc(sizeof(*rmn), GFP_KERNEL);
> + if (!rmn) {
> + rmn = ERR_PTR(-ENOMEM);
> + goto release_locks;
> + }
> +
> + rmn->rdev = rdev;
> + rmn->mm = mm;
> + rmn->mn.ops = &radeon_mn_ops;
> + mutex_init(&rmn->lock);
> + rmn->objects = RB_ROOT;
> +
> + r = __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 = addr + radeon_bo_size(bo) - 1;
> + struct radeon_device *rdev = bo->rdev;
> + struct radeon_mn *rmn;
> + struct interval_tree_node *it;
> +
> + rmn = radeon_mn_get(rdev);
> + if (IS_ERR(rmn))
> + return PTR_ERR(rmn);
> +
> + mutex_lock(&rmn->lock);
> +
> + it = interval_tree_iter_first(&rmn->objects, addr, end);
> + if (it) {
> + mutex_unlock(&rmn->lock);
> + return -EEXIST;
> + }
> +
> + bo->mn = rmn;
> + bo->mn_it.start = addr;
> + bo->mn_it.last = 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 object.
> + */
> +void radeon_mn_unregister(struct radeon_bo *bo)
> +{
> + struct radeon_device *rdev = bo->rdev;
> + struct radeon_mn *rmn;
> +
> + mutex_lock(&rdev->mn_lock);
> + rmn = bo->mn;
> + if (rmn == NULL) {
> + mutex_unlock(&rdev->mn_lock);
> + return;
> + }
> +
> + mutex_lock(&rmn->lock);
> + interval_tree_remove(&bo->mn_it, &rmn->objects);
> + bo->mn = NULL;
> + mutex_unlock(&rmn->lock);
> + mutex_unlock(&rdev->mn_lock);
> +}
> diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/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_object *tbo)
> bo = 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@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
