On Thu, Nov 03, 2011 at 07:39:12PM -0400, j.glisse@xxxxxxxxx wrote: > From: Jerome Glisse <jglisse@xxxxxxxxxx> > > Move the page allocation and freeing to driver callback and > provide ttm code helper function for those. > > Most intrusive change, is the fact that we now only fully > populate an object this simplify some of code designed around > the page fault design. Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> > > Signed-off-by: Jerome Glisse <jglisse@xxxxxxxxxx> > --- > drivers/gpu/drm/nouveau/nouveau_bo.c | 3 + > drivers/gpu/drm/radeon/radeon_ttm.c | 2 + > drivers/gpu/drm/ttm/ttm_bo_util.c | 31 ++++++----- > drivers/gpu/drm/ttm/ttm_bo_vm.c | 13 ++-- > drivers/gpu/drm/ttm/ttm_page_alloc.c | 42 ++++++++++++++ > drivers/gpu/drm/ttm/ttm_tt.c | 97 +++---------------------------- > drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c | 3 + > include/drm/ttm/ttm_bo_driver.h | 41 ++++++++------ > include/drm/ttm/ttm_page_alloc.h | 18 ++++++ > 9 files changed, 125 insertions(+), 125 deletions(-) > > diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c > index b060fa4..7e5ca3f 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_bo.c > +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c > @@ -28,6 +28,7 @@ > */ > > #include "drmP.h" > +#include "ttm/ttm_page_alloc.h" > > #include "nouveau_drm.h" > #include "nouveau_drv.h" > @@ -1050,6 +1051,8 @@ nouveau_bo_fence(struct nouveau_bo *nvbo, struct nouveau_fence *fence) > > struct ttm_bo_driver nouveau_bo_driver = { > .ttm_tt_create = &nouveau_ttm_tt_create, > + .ttm_tt_populate = &ttm_page_alloc_ttm_tt_populate, > + .ttm_tt_unpopulate = &ttm_page_alloc_ttm_tt_unpopulate, > .invalidate_caches = nouveau_bo_invalidate_caches, > .init_mem_type = nouveau_bo_init_mem_type, > .evict_flags = nouveau_bo_evict_flags, > diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c > index 53ff62b..490afce 100644 > --- a/drivers/gpu/drm/radeon/radeon_ttm.c > +++ b/drivers/gpu/drm/radeon/radeon_ttm.c > @@ -584,6 +584,8 @@ struct ttm_tt *radeon_ttm_tt_create(struct ttm_bo_device *bdev, > > static struct ttm_bo_driver radeon_bo_driver = { > .ttm_tt_create = &radeon_ttm_tt_create, > + .ttm_tt_populate = &ttm_page_alloc_ttm_tt_populate, > + .ttm_tt_unpopulate = &ttm_page_alloc_ttm_tt_unpopulate, > .invalidate_caches = &radeon_invalidate_caches, > .init_mem_type = &radeon_init_mem_type, > .evict_flags = &radeon_evict_flags, > diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c > index 082fcae..60f204d 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo_util.c > +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c > @@ -244,7 +244,7 @@ static int ttm_copy_io_ttm_page(struct ttm_tt *ttm, void *src, > unsigned long page, > pgprot_t prot) > { > - struct page *d = ttm_tt_get_page(ttm, page); > + struct page *d = ttm->pages[page]; > void *dst; > > if (!d) > @@ -281,7 +281,7 @@ static int ttm_copy_ttm_io_page(struct ttm_tt *ttm, void *dst, > unsigned long page, > pgprot_t prot) > { > - struct page *s = ttm_tt_get_page(ttm, page); > + struct page *s = ttm->pages[page]; > void *src; > > if (!s) > @@ -342,6 +342,12 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo, > if (old_iomap == NULL && ttm == NULL) > goto out2; > > + if (ttm->state == tt_unpopulated) { > + ret = ttm->bdev->driver->ttm_tt_populate(ttm); > + if (ret) > + goto out1; > + } > + > add = 0; > dir = 1; > > @@ -502,10 +508,16 @@ static int ttm_bo_kmap_ttm(struct ttm_buffer_object *bo, > { > struct ttm_mem_reg *mem = &bo->mem; pgprot_t prot; > struct ttm_tt *ttm = bo->ttm; > - struct page *d; > - int i; > + int ret; > > BUG_ON(!ttm); > + > + if (ttm->state == tt_unpopulated) { > + ret = ttm->bdev->driver->ttm_tt_populate(ttm); > + if (ret) > + return ret; > + } > + > if (num_pages == 1 && (mem->placement & TTM_PL_FLAG_CACHED)) { > /* > * We're mapping a single page, and the desired > @@ -513,18 +525,9 @@ static int ttm_bo_kmap_ttm(struct ttm_buffer_object *bo, > */ > > map->bo_kmap_type = ttm_bo_map_kmap; > - map->page = ttm_tt_get_page(ttm, start_page); > + map->page = ttm->pages[start_page]; > map->virtual = kmap(map->page); > } else { > - /* > - * Populate the part we're mapping; > - */ > - for (i = start_page; i < start_page + num_pages; ++i) { > - d = ttm_tt_get_page(ttm, i); > - if (!d) > - return -ENOMEM; > - } > - > /* > * We need to use vmap to get the desired page protection > * or to make the buffer object look contiguous. > diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c > index 221b924..bc1d751 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c > +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c > @@ -174,18 +174,19 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf) > vma->vm_page_prot = (bo->mem.placement & TTM_PL_FLAG_CACHED) ? > vm_get_page_prot(vma->vm_flags) : > ttm_io_prot(bo->mem.placement, vma->vm_page_prot); > - } > > - /* > - * Speculatively prefault a number of pages. Only error on > - * first page. > - */ > + /* Allocate all page at once, most common usage */ > + if (ttm->bdev->driver->ttm_tt_populate(ttm)) { > + retval = VM_FAULT_OOM; > + goto out_io_unlock; > + } > + } > > for (i = 0; i < TTM_BO_VM_NUM_PREFAULT; ++i) { > if (bo->mem.bus.is_iomem) > pfn = ((bo->mem.bus.base + bo->mem.bus.offset) >> PAGE_SHIFT) + page_offset; > else { > - page = ttm_tt_get_page(ttm, page_offset); > + page = ttm->pages[page_offset]; > if (unlikely(!page && i == 0)) { > retval = VM_FAULT_OOM; > goto out_io_unlock; > diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c b/drivers/gpu/drm/ttm/ttm_page_alloc.c > index c4f18b9..5b5b1e0 100644 > --- a/drivers/gpu/drm/ttm/ttm_page_alloc.c > +++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c > @@ -852,6 +852,48 @@ void ttm_page_alloc_fini(void) > _manager = NULL; > } > > +int ttm_page_alloc_ttm_tt_populate(struct ttm_tt *ttm) > +{ > + struct ttm_mem_global *mem_glob = ttm->glob->mem_glob; > + int ret; > + > + if (ttm->state != tt_unpopulated) > + return 0; > + > + if (unlikely(ttm->page_flags & TTM_PAGE_FLAG_SWAPPED)) { > + ret = ttm_tt_swapin(ttm); > + if (unlikely(ret != 0)) > + return ret; > + } > + > + ret = ttm_get_pages(ttm->pages, ttm->num_pages, ttm->page_flags, > + ttm->caching_state, ttm->dma_address); > + if (ret != 0) > + return -ENOMEM; > + ret = ttm_mem_global_alloc_pages(mem_glob, ttm->pages, > + ttm->num_pages, false, false); > + if (unlikely(ret != 0)) { > + ttm_put_pages(ttm->pages, ttm->num_pages, ttm->page_flags, > + ttm->caching_state, ttm->dma_address); > + return -ENOMEM; > + } > + > + ttm->state = tt_unbound; > + return 0; > +} > +EXPORT_SYMBOL(ttm_page_alloc_ttm_tt_populate); > + > +void ttm_page_alloc_ttm_tt_unpopulate(struct ttm_tt *ttm) > +{ > + struct ttm_mem_global *glob = ttm->glob->mem_glob; > + > + ttm_mem_global_free_pages(glob, ttm->pages, ttm->num_pages); > + ttm_put_pages(ttm->pages, ttm->num_pages, ttm->page_flags, > + ttm->caching_state, ttm->dma_address); > + ttm->state = tt_unpopulated; > +} > +EXPORT_SYMBOL(ttm_page_alloc_ttm_tt_unpopulate); > + > int ttm_page_alloc_debugfs(struct seq_file *m, void *data) > { > struct ttm_page_pool *p; > diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c > index b55e132..07e86c4 100644 > --- a/drivers/gpu/drm/ttm/ttm_tt.c > +++ b/drivers/gpu/drm/ttm/ttm_tt.c > @@ -42,8 +42,6 @@ > #include "ttm/ttm_placement.h" > #include "ttm/ttm_page_alloc.h" > > -static int ttm_tt_swapin(struct ttm_tt *ttm); > - > /** > * Allocates storage for pointers to the pages that back the ttm. > */ > @@ -62,75 +60,6 @@ static void ttm_tt_free_page_directory(struct ttm_tt *ttm) > ttm->dma_address = NULL; > } > > -static struct page *__ttm_tt_get_page(struct ttm_tt *ttm, int index) > -{ > - struct page *p; > - struct ttm_mem_global *mem_glob = ttm->glob->mem_glob; > - int ret; > - > - if (NULL == (p = ttm->pages[index])) { > - > - ret = ttm_get_pages(&ttm->pages[index], 1, ttm->page_flags, > - ttm->caching_state, > - &ttm->dma_address[index]); > - if (ret != 0) > - return NULL; > - > - ret = ttm_mem_global_alloc_pages(mem_glob, &ttm->pages[index], > - 1, false, false); > - if (unlikely(ret != 0)) > - goto out_err; > - } > - return p; > -out_err: > - ttm_put_pages(&ttm->pages[index], 1, ttm->page_flags, > - ttm->caching_state, &ttm->dma_address[index]); > - return NULL; > -} > - > -struct page *ttm_tt_get_page(struct ttm_tt *ttm, int index) > -{ > - int ret; > - > - if (unlikely(ttm->page_flags & TTM_PAGE_FLAG_SWAPPED)) { > - ret = ttm_tt_swapin(ttm); > - if (unlikely(ret != 0)) > - return NULL; > - } > - return __ttm_tt_get_page(ttm, index); > -} > - > -int ttm_tt_populate(struct ttm_tt *ttm) > -{ > - struct ttm_mem_global *mem_glob = ttm->glob->mem_glob; > - int ret; > - > - if (ttm->state != tt_unpopulated) > - return 0; > - > - if (unlikely(ttm->page_flags & TTM_PAGE_FLAG_SWAPPED)) { > - ret = ttm_tt_swapin(ttm); > - if (unlikely(ret != 0)) > - return ret; > - } > - > - ret = ttm_get_pages(ttm->pages, ttm->num_pages, ttm->page_flags, > - ttm->caching_state, ttm->dma_address); > - if (ret != 0) > - return -ENOMEM; > - ret = ttm_mem_global_alloc_pages(mem_glob, ttm->pages, > - ttm->num_pages, false, false); > - if (unlikely(ret != 0)) { > - ttm_put_pages(ttm->pages, ttm->num_pages, ttm->page_flags, > - ttm->caching_state, ttm->dma_address); > - return -ENOMEM; > - } > - > - ttm->state = tt_unbound; > - return 0; > -} > -EXPORT_SYMBOL(ttm_tt_populate); > - > #ifdef CONFIG_X86 > static inline int ttm_tt_set_page_caching(struct page *p, > enum ttm_caching_state c_old, > @@ -232,23 +161,13 @@ int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t placement) > } > EXPORT_SYMBOL(ttm_tt_set_placement_caching); > > -static void ttm_tt_free_alloced_pages(struct ttm_tt *ttm) > -{ > - struct ttm_mem_global *glob = ttm->glob->mem_glob; > - > - ttm_mem_global_free_pages(glob, ttm->pages, ttm->num_pages); > - ttm_put_pages(ttm->pages, ttm->num_pages, ttm->page_flags, > - ttm->caching_state, ttm->dma_address); > - ttm->state = tt_unpopulated; > -} > - > void ttm_tt_destroy(struct ttm_tt *ttm) > { > if (unlikely(ttm == NULL)) > return; > > if (likely(ttm->pages != NULL)) { > - ttm_tt_free_alloced_pages(ttm); > + ttm->bdev->driver->ttm_tt_unpopulate(ttm); > ttm_tt_free_page_directory(ttm); > } > > @@ -303,7 +222,7 @@ int ttm_tt_bind(struct ttm_tt *ttm, struct ttm_mem_reg *bo_mem) > if (ttm->state == tt_bound) > return 0; > > - ret = ttm_tt_populate(ttm); > + ret = ttm->bdev->driver->ttm_tt_populate(ttm); > if (ret) > return ret; > > @@ -317,7 +236,7 @@ int ttm_tt_bind(struct ttm_tt *ttm, struct ttm_mem_reg *bo_mem) > } > EXPORT_SYMBOL(ttm_tt_bind); > > -static int ttm_tt_swapin(struct ttm_tt *ttm) > +int ttm_tt_swapin(struct ttm_tt *ttm) > { > struct address_space *swap_space; > struct file *swap_storage; > @@ -331,6 +250,10 @@ static int ttm_tt_swapin(struct ttm_tt *ttm) > swap_storage = ttm->swap_storage; > BUG_ON(swap_storage == NULL); > > + ret = ttm->bdev->driver->ttm_tt_populate(ttm); > + if (ret) > + return ret; > + > swap_space = swap_storage->f_path.dentry->d_inode->i_mapping; > > for (i = 0; i < ttm->num_pages; ++i) { > @@ -339,7 +262,7 @@ static int ttm_tt_swapin(struct ttm_tt *ttm) > ret = PTR_ERR(from_page); > goto out_err; > } > - to_page = __ttm_tt_get_page(ttm, i); > + to_page = ttm->pages[i]; > if (unlikely(to_page == NULL)) > goto out_err; > > @@ -360,7 +283,7 @@ static int ttm_tt_swapin(struct ttm_tt *ttm) > > return 0; > out_err: > - ttm_tt_free_alloced_pages(ttm); > + ttm->bdev->driver->ttm_tt_unpopulate(ttm); > return ret; > } > > @@ -412,7 +335,7 @@ int ttm_tt_swapout(struct ttm_tt *ttm, struct file *persistent_swap_storage) > page_cache_release(to_page); > } > > - ttm_tt_free_alloced_pages(ttm); > + ttm->bdev->driver->ttm_tt_unpopulate(ttm); > ttm->swap_storage = swap_storage; > ttm->page_flags |= TTM_PAGE_FLAG_SWAPPED; > if (persistent_swap_storage) > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c > index cc72435..383993b 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c > @@ -28,6 +28,7 @@ > #include "vmwgfx_drv.h" > #include "ttm/ttm_bo_driver.h" > #include "ttm/ttm_placement.h" > +#include "ttm/ttm_page_alloc.h" > > static uint32_t vram_placement_flags = TTM_PL_FLAG_VRAM | > TTM_PL_FLAG_CACHED; > @@ -334,6 +335,8 @@ static int vmw_sync_obj_wait(void *sync_obj, void *sync_arg, > > struct ttm_bo_driver vmw_bo_driver = { > .ttm_tt_create = &vmw_ttm_tt_create, > + .ttm_tt_populate = &ttm_page_alloc_ttm_tt_populate, > + .ttm_tt_unpopulate = &ttm_page_alloc_ttm_tt_unpopulate, > .invalidate_caches = vmw_invalidate_caches, > .init_mem_type = vmw_init_mem_type, > .evict_flags = vmw_evict_flags, > diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h > index 6b8c5cd..ae06e42 100644 > --- a/include/drm/ttm/ttm_bo_driver.h > +++ b/include/drm/ttm/ttm_bo_driver.h > @@ -319,6 +319,26 @@ struct ttm_bo_driver { > struct page *dummy_read_page); > > /** > + * ttm_tt_populate > + * > + * @ttm: The struct ttm_tt to contain the backing pages. > + * > + * Allocate all backing pages > + * Returns: > + * -ENOMEM: Out of memory. > + */ > + int (*ttm_tt_populate)(struct ttm_tt *ttm); > + > + /** > + * ttm_tt_unpopulate > + * > + * @ttm: The struct ttm_tt to contain the backing pages. > + * > + * Free all backing page > + */ > + void (*ttm_tt_unpopulate)(struct ttm_tt *ttm); > + > + /** > * struct ttm_bo_driver member invalidate_caches > * > * @bdev: the buffer object device. > @@ -585,15 +605,6 @@ extern int ttm_tt_init(struct ttm_tt *ttm, struct ttm_bo_device *bdev, > extern int ttm_tt_bind(struct ttm_tt *ttm, struct ttm_mem_reg *bo_mem); > > /** > - * ttm_tt_populate: > - * > - * @ttm: The struct ttm_tt to contain the backing pages. > - * > - * Add backing pages to all of @ttm > - */ > -extern int ttm_tt_populate(struct ttm_tt *ttm); > - > -/** > * ttm_ttm_destroy: > * > * @ttm: The struct ttm_tt. > @@ -612,19 +623,13 @@ extern void ttm_tt_destroy(struct ttm_tt *ttm); > extern void ttm_tt_unbind(struct ttm_tt *ttm); > > /** > - * ttm_ttm_destroy: > + * ttm_tt_swapin: > * > * @ttm: The struct ttm_tt. > - * @index: Index of the desired page. > * > - * Return a pointer to the struct page backing @ttm at page > - * index @index. If the page is unpopulated, one will be allocated to > - * populate that index. > - * > - * Returns: > - * NULL on OOM. > + * Swap in a previously swap out ttm_tt. > */ > -extern struct page *ttm_tt_get_page(struct ttm_tt *ttm, int index); > +extern int ttm_tt_swapin(struct ttm_tt *ttm); > > /** > * ttm_tt_cache_flush: > diff --git a/include/drm/ttm/ttm_page_alloc.h b/include/drm/ttm/ttm_page_alloc.h > index fffb3bd..18a2d6b 100644 > --- a/include/drm/ttm/ttm_page_alloc.h > +++ b/include/drm/ttm/ttm_page_alloc.h > @@ -67,6 +67,24 @@ int ttm_page_alloc_init(struct ttm_mem_global *glob, unsigned max_pages); > void ttm_page_alloc_fini(void); > > /** > + * ttm_page_alloc_ttm_tt_populate: > + * > + * @ttm: The struct ttm_tt to contain the backing pages. > + * > + * Add backing pages to all of @ttm > + */ > +extern int ttm_page_alloc_ttm_tt_populate(struct ttm_tt *ttm); > + > +/** > + * ttm_page_alloc_ttm_tt_unpopulate: > + * > + * @ttm: The struct ttm_tt which to free backing pages. > + * > + * Free all pages of @ttm > + */ > +extern void ttm_page_alloc_ttm_tt_unpopulate(struct ttm_tt *ttm); > + > +/** > * Output the state of pools to debugfs file > */ > extern int ttm_page_alloc_debugfs(struct seq_file *m, void *data); > -- > 1.7.7.1 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel