Re: [PATCH 01/12] drm/ttm: remove userspace backed ttm object support

[Thomas Hellstrom <thellstrom@vmware.com>]

On 11/08/2011 12:40 AM, j.glisse@gmail.com wrote:
> From: Jerome Glisse<jglisse@redhat.com>
>
> This was never use in none of the driver, properly using userspace
> page for bo would need more code (vma interaction mostly). Removing
> this dead code in preparation of ttm_tt&  backend merge.
>
>    

This is not exactly true. It was once used by psb, and we might be 
implementing it for vmwgfx as well,
if it turns out to give us a performance boost.
There are some usage restrictions, though: The bo can only be accessed 
through the original VMA, and the backend must support cached pages.

But I'll figure out a way to implement this in the context of the 
unified backend / tt.

Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>

> Signed-off-by: Jerome Glisse<jglisse@redhat.com>
> Reviewed-by: Konrad Rzeszutek Wilk<konrad.wilk@oracle.com>
> ---
>   drivers/gpu/drm/ttm/ttm_bo.c    |   22 --------
>   drivers/gpu/drm/ttm/ttm_tt.c    |  105 +--------------------------------------
>   include/drm/ttm/ttm_bo_api.h    |    5 --
>   include/drm/ttm/ttm_bo_driver.h |   24 ---------
>   4 files changed, 1 insertions(+), 155 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 617b646..4bde335 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -342,22 +342,6 @@ static int ttm_bo_add_ttm(struct ttm_buffer_object *bo, bool zero_alloc)
>   		if (unlikely(bo->ttm == NULL))
>   			ret = -ENOMEM;
>   		break;
> -	case ttm_bo_type_user:
> -		bo->ttm = ttm_tt_create(bdev, bo->num_pages<<  PAGE_SHIFT,
> -					page_flags | TTM_PAGE_FLAG_USER,
> -					glob->dummy_read_page);
> -		if (unlikely(bo->ttm == NULL)) {
> -			ret = -ENOMEM;
> -			break;
> -		}
> -
> -		ret = ttm_tt_set_user(bo->ttm, current,
> -				      bo->buffer_start, bo->num_pages);
> -		if (unlikely(ret != 0)) {
> -			ttm_tt_destroy(bo->ttm);
> -			bo->ttm = NULL;
> -		}
> -		break;
>   	default:
>   		printk(KERN_ERR TTM_PFX "Illegal buffer object type\n");
>   		ret = -EINVAL;
> @@ -907,16 +891,12 @@ static uint32_t ttm_bo_select_caching(struct ttm_mem_type_manager *man,
>   }
>
>   static bool ttm_bo_mt_compatible(struct ttm_mem_type_manager *man,
> -				 bool disallow_fixed,
>   				 uint32_t mem_type,
>   				 uint32_t proposed_placement,
>   				 uint32_t *masked_placement)
>   {
>   	uint32_t cur_flags = ttm_bo_type_flags(mem_type);
>
> -	if ((man->flags&  TTM_MEMTYPE_FLAG_FIXED)&&  disallow_fixed)
> -		return false;
> -
>   	if ((cur_flags&  proposed_placement&  TTM_PL_MASK_MEM) == 0)
>   		return false;
>
> @@ -961,7 +941,6 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
>   		man =&bdev->man[mem_type];
>
>   		type_ok = ttm_bo_mt_compatible(man,
> -						bo->type == ttm_bo_type_user,
>   						mem_type,
>   						placement->placement[i],
>   						&cur_flags);
> @@ -1009,7 +988,6 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
>   		if (!man->has_type)
>   			continue;
>   		if (!ttm_bo_mt_compatible(man,
> -						bo->type == ttm_bo_type_user,
>   						mem_type,
>   						placement->busy_placement[i],
>   						&cur_flags))
> diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
> index 58c271e..82a1161 100644
> --- a/drivers/gpu/drm/ttm/ttm_tt.c
> +++ b/drivers/gpu/drm/ttm/ttm_tt.c
> @@ -62,43 +62,6 @@ static void ttm_tt_free_page_directory(struct ttm_tt *ttm)
>   	ttm->dma_address = NULL;
>   }
>
> -static void ttm_tt_free_user_pages(struct ttm_tt *ttm)
> -{
> -	int write;
> -	int dirty;
> -	struct page *page;
> -	int i;
> -	struct ttm_backend *be = ttm->be;
> -
> -	BUG_ON(!(ttm->page_flags&  TTM_PAGE_FLAG_USER));
> -	write = ((ttm->page_flags&  TTM_PAGE_FLAG_WRITE) != 0);
> -	dirty = ((ttm->page_flags&  TTM_PAGE_FLAG_USER_DIRTY) != 0);
> -
> -	if (be)
> -		be->func->clear(be);
> -
> -	for (i = 0; i<  ttm->num_pages; ++i) {
> -		page = ttm->pages[i];
> -		if (page == NULL)
> -			continue;
> -
> -		if (page == ttm->dummy_read_page) {
> -			BUG_ON(write);
> -			continue;
> -		}
> -
> -		if (write&&  dirty&&  !PageReserved(page))
> -			set_page_dirty_lock(page);
> -
> -		ttm->pages[i] = NULL;
> -		ttm_mem_global_free(ttm->glob->mem_glob, PAGE_SIZE);
> -		put_page(page);
> -	}
> -	ttm->state = tt_unpopulated;
> -	ttm->first_himem_page = ttm->num_pages;
> -	ttm->last_lomem_page = -1;
> -}
> -
>   static struct page *__ttm_tt_get_page(struct ttm_tt *ttm, int index)
>   {
>   	struct page *p;
> @@ -325,10 +288,7 @@ void ttm_tt_destroy(struct ttm_tt *ttm)
>   	}
>
>   	if (likely(ttm->pages != NULL)) {
> -		if (ttm->page_flags&  TTM_PAGE_FLAG_USER)
> -			ttm_tt_free_user_pages(ttm);
> -		else
> -			ttm_tt_free_alloced_pages(ttm);
> +		ttm_tt_free_alloced_pages(ttm);
>
>   		ttm_tt_free_page_directory(ttm);
>   	}
> @@ -340,45 +300,6 @@ void ttm_tt_destroy(struct ttm_tt *ttm)
>   	kfree(ttm);
>   }
>
> -int ttm_tt_set_user(struct ttm_tt *ttm,
> -		    struct task_struct *tsk,
> -		    unsigned long start, unsigned long num_pages)
> -{
> -	struct mm_struct *mm = tsk->mm;
> -	int ret;
> -	int write = (ttm->page_flags&  TTM_PAGE_FLAG_WRITE) != 0;
> -	struct ttm_mem_global *mem_glob = ttm->glob->mem_glob;
> -
> -	BUG_ON(num_pages != ttm->num_pages);
> -	BUG_ON((ttm->page_flags&  TTM_PAGE_FLAG_USER) == 0);
> -
> -	/**
> -	 * Account user pages as lowmem pages for now.
> -	 */
> -
> -	ret = ttm_mem_global_alloc(mem_glob, num_pages * PAGE_SIZE,
> -				   false, false);
> -	if (unlikely(ret != 0))
> -		return ret;
> -
> -	down_read(&mm->mmap_sem);
> -	ret = get_user_pages(tsk, mm, start, num_pages,
> -			     write, 0, ttm->pages, NULL);
> -	up_read(&mm->mmap_sem);
> -
> -	if (ret != num_pages&&  write) {
> -		ttm_tt_free_user_pages(ttm);
> -		ttm_mem_global_free(mem_glob, num_pages * PAGE_SIZE);
> -		return -ENOMEM;
> -	}
> -
> -	ttm->tsk = tsk;
> -	ttm->start = start;
> -	ttm->state = tt_unbound;
> -
> -	return 0;
> -}
> -
>   struct ttm_tt *ttm_tt_create(struct ttm_bo_device *bdev, unsigned long size,
>   			     uint32_t page_flags, struct page *dummy_read_page)
>   {
> @@ -452,8 +373,6 @@ int ttm_tt_bind(struct ttm_tt *ttm, struct ttm_mem_reg *bo_mem)
>
>   	ttm->state = tt_bound;
>
> -	if (ttm->page_flags&  TTM_PAGE_FLAG_USER)
> -		ttm->page_flags |= TTM_PAGE_FLAG_USER_DIRTY;
>   	return 0;
>   }
>   EXPORT_SYMBOL(ttm_tt_bind);
> @@ -469,16 +388,6 @@ static int ttm_tt_swapin(struct ttm_tt *ttm)
>   	int i;
>   	int ret = -ENOMEM;
>
> -	if (ttm->page_flags&  TTM_PAGE_FLAG_USER) {
> -		ret = ttm_tt_set_user(ttm, ttm->tsk, ttm->start,
> -				      ttm->num_pages);
> -		if (unlikely(ret != 0))
> -			return ret;
> -
> -		ttm->page_flags&= ~TTM_PAGE_FLAG_SWAPPED;
> -		return 0;
> -	}
> -
>   	swap_storage = ttm->swap_storage;
>   	BUG_ON(swap_storage == NULL);
>
> @@ -529,18 +438,6 @@ int ttm_tt_swapout(struct ttm_tt *ttm, struct file *persistent_swap_storage)
>   	BUG_ON(ttm->state != tt_unbound&&  ttm->state != tt_unpopulated);
>   	BUG_ON(ttm->caching_state != tt_cached);
>
> -	/*
> -	 * For user buffers, just unpin the pages, as there should be
> -	 * vma references.
> -	 */
> -
> -	if (ttm->page_flags&  TTM_PAGE_FLAG_USER) {
> -		ttm_tt_free_user_pages(ttm);
> -		ttm->page_flags |= TTM_PAGE_FLAG_SWAPPED;
> -		ttm->swap_storage = NULL;
> -		return 0;
> -	}
> -
>   	if (!persistent_swap_storage) {
>   		swap_storage = shmem_file_setup("ttm swap",
>   						ttm->num_pages<<  PAGE_SHIFT,
> diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
> index 42e3469..8d95a42 100644
> --- a/include/drm/ttm/ttm_bo_api.h
> +++ b/include/drm/ttm/ttm_bo_api.h
> @@ -122,17 +122,12 @@ struct ttm_mem_reg {
>    * be mmapped by user space. Each of these bos occupy a slot in the
>    * device address space, that can be used for normal vm operations.
>    *
> - * @ttm_bo_type_user: These are user-space memory areas that are made
> - * available to the GPU by mapping the buffer pages into the GPU aperture
> - * space. These buffers cannot be mmaped from the device address space.
> - *
>    * @ttm_bo_type_kernel: These buffers are like ttm_bo_type_device buffers,
>    * but they cannot be accessed from user-space. For kernel-only use.
>    */
>
>   enum ttm_bo_type {
>   	ttm_bo_type_device,
> -	ttm_bo_type_user,
>   	ttm_bo_type_kernel
>   };
>
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 94eb143..37527d6 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -118,8 +118,6 @@ struct ttm_backend {
>   	struct ttm_backend_func *func;
>   };
>
> -#define TTM_PAGE_FLAG_USER            (1<<  1)
> -#define TTM_PAGE_FLAG_USER_DIRTY      (1<<  2)
>   #define TTM_PAGE_FLAG_WRITE           (1<<  3)
>   #define TTM_PAGE_FLAG_SWAPPED         (1<<  4)
>   #define TTM_PAGE_FLAG_PERSISTENT_SWAP (1<<  5)
> @@ -146,8 +144,6 @@ enum ttm_caching_state {
>    * @num_pages: Number of pages in the page array.
>    * @bdev: Pointer to the current struct ttm_bo_device.
>    * @be: Pointer to the ttm backend.
> - * @tsk: The task for user ttm.
> - * @start: virtual address for user ttm.
>    * @swap_storage: Pointer to shmem struct file for swap storage.
>    * @caching_state: The current caching state of the pages.
>    * @state: The current binding state of the pages.
> @@ -167,8 +163,6 @@ struct ttm_tt {
>   	unsigned long num_pages;
>   	struct ttm_bo_global *glob;
>   	struct ttm_backend *be;
> -	struct task_struct *tsk;
> -	unsigned long start;
>   	struct file *swap_storage;
>   	enum ttm_caching_state caching_state;
>   	enum {
> @@ -618,24 +612,6 @@ extern struct ttm_tt *ttm_tt_create(struct ttm_bo_device *bdev,
>   				    struct page *dummy_read_page);
>
>   /**
> - * ttm_tt_set_user:
> - *
> - * @ttm: The struct ttm_tt to populate.
> - * @tsk: A struct task_struct for which @start is a valid user-space address.
> - * @start: A valid user-space address.
> - * @num_pages: Size in pages of the user memory area.
> - *
> - * Populate a struct ttm_tt with a user-space memory area after first pinning
> - * the pages backing it.
> - * Returns:
> - * !0: Error.
> - */
> -
> -extern int ttm_tt_set_user(struct ttm_tt *ttm,
> -			   struct task_struct *tsk,
> -			   unsigned long start, unsigned long num_pages);
> -
> -/**
>    * ttm_ttm_bind:
>    *
>    * @ttm: The struct ttm_tt containing backing pages.
>