[PATCH 4/5] drm/radeon: add userptr flag to register MMU notifier v3

Christian König deathsimple at vodafone.de
Wed Aug 6 08:23:15 PDT 2014


Am 06.08.2014 um 17:16 schrieb Jerome Glisse:
> On Tue, Aug 05, 2014 at 06:05:31PM +0200, Christian König wrote:
>> From: Christian König <christian.koenig at amd.com>
>>
>> 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önig <christian.koenig at amd.com>
>> ---
>>   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(&current->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 at amd.com>
>> + */
>> +
>> +#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 at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel



More information about the dri-devel mailing list