[RFC][PATCH v6 3/7] drm: ttm_pool: Rework ttm_pool_free_page to allow us to use it as a function pointer

From: John Stultz
Date: Fri Feb 05 2021 - 03:09:35 EST


This refactors ttm_pool_free_page(), and by adding extra entries
to ttm_pool_page_dat, we then use it for all allocations, which
allows us to simplify the arguments needed to be passed to
ttm_pool_free_page().

This is critical for allowing the free function to be called
by the sharable drm_page_pool logic.

Cc: Daniel Vetter <daniel@xxxxxxxx>
Cc: Christian Koenig <christian.koenig@xxxxxxx>
Cc: Sumit Semwal <sumit.semwal@xxxxxxxxxx>
Cc: Liam Mark <lmark@xxxxxxxxxxxxxx>
Cc: Chris Goldsworthy <cgoldswo@xxxxxxxxxxxxxx>
Cc: Laura Abbott <labbott@xxxxxxxxxx>
Cc: Brian Starkey <Brian.Starkey@xxxxxxx>
Cc: Hridya Valsaraju <hridya@xxxxxxxxxx>
Cc: Suren Baghdasaryan <surenb@xxxxxxxxxx>
Cc: Sandeep Patil <sspatil@xxxxxxxxxx>
Cc: Daniel Mentz <danielmentz@xxxxxxxxxx>
Cc: Ørjan Eide <orjan.eide@xxxxxxx>
Cc: Robin Murphy <robin.murphy@xxxxxxx>
Cc: Ezequiel Garcia <ezequiel@xxxxxxxxxxxxx>
Cc: Simon Ser <contact@xxxxxxxxxxx>
Cc: James Jones <jajones@xxxxxxxxxx>
Cc: linux-media@xxxxxxxxxxxxxxx
Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
Signed-off-by: John Stultz <john.stultz@xxxxxxxxxx>
---
drivers/gpu/drm/ttm/ttm_pool.c | 60 ++++++++++++++++++----------------
1 file changed, 32 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c
index c0274e256be3..eca36678f967 100644
--- a/drivers/gpu/drm/ttm/ttm_pool.c
+++ b/drivers/gpu/drm/ttm/ttm_pool.c
@@ -44,10 +44,14 @@
/**
* struct ttm_pool_page_dat - Helper object for coherent DMA mappings
*
+ * @pool: ttm_pool pointer the page was allocated by
+ * @caching: the caching value the allocated page was configured for
* @addr: original DMA address returned for the mapping
* @vaddr: original vaddr return for the mapping and order in the lower bits
*/
struct ttm_pool_page_dat {
+ struct ttm_pool *pool;
+ enum ttm_caching caching;
dma_addr_t addr;
unsigned long vaddr;
};
@@ -71,13 +75,20 @@ static struct shrinker mm_shrinker;

/* Allocate pages of size 1 << order with the given gfp_flags */
static struct page *ttm_pool_alloc_page(struct ttm_pool *pool, gfp_t gfp_flags,
- unsigned int order)
+ unsigned int order, enum ttm_caching caching)
{
unsigned long attr = DMA_ATTR_FORCE_CONTIGUOUS;
struct ttm_pool_page_dat *dat;
struct page *p;
void *vaddr;

+ dat = kmalloc(sizeof(*dat), GFP_KERNEL);
+ if (!dat)
+ return NULL;
+
+ dat->pool = pool;
+ dat->caching = caching;
+
/* Don't set the __GFP_COMP flag for higher order allocations.
* Mapping pages directly into an userspace process and calling
* put_page() on a TTM allocated page is illegal.
@@ -88,15 +99,13 @@ static struct page *ttm_pool_alloc_page(struct ttm_pool *pool, gfp_t gfp_flags,

if (!pool->use_dma_alloc) {
p = alloc_pages(gfp_flags, order);
- if (p)
- p->private = order;
+ if (!p)
+ goto error_free;
+ dat->vaddr = order;
+ p->private = (unsigned long)dat;
return p;
}

- dat = kmalloc(sizeof(*dat), GFP_KERNEL);
- if (!dat)
- return NULL;
-
if (order)
attr |= DMA_ATTR_NO_WARN;

@@ -123,34 +132,34 @@ static struct page *ttm_pool_alloc_page(struct ttm_pool *pool, gfp_t gfp_flags,
}

/* Reset the caching and pages of size 1 << order */
-static void ttm_pool_free_page(struct ttm_pool *pool, enum ttm_caching caching,
- unsigned int order, struct page *p)
+static int ttm_pool_free_page(struct page *p, unsigned int order)
{
unsigned long attr = DMA_ATTR_FORCE_CONTIGUOUS;
- struct ttm_pool_page_dat *dat;
+ struct ttm_pool_page_dat *dat = (void *)p->private;
void *vaddr;

#ifdef CONFIG_X86
/* We don't care that set_pages_wb is inefficient here. This is only
* used when we have to shrink and CPU overhead is irrelevant then.
*/
- if (caching != ttm_cached && !PageHighMem(p))
+ if (dat->caching != ttm_cached && !PageHighMem(p))
set_pages_wb(p, 1 << order);
#endif

- if (!pool || !pool->use_dma_alloc) {
+ if (!dat->pool || !dat->pool->use_dma_alloc) {
__free_pages(p, order);
- return;
+ goto out;
}

if (order)
attr |= DMA_ATTR_NO_WARN;

- dat = (void *)p->private;
vaddr = (void *)(dat->vaddr & PAGE_MASK);
- dma_free_attrs(pool->dev, (1UL << order) * PAGE_SIZE, vaddr, dat->addr,
+ dma_free_attrs(dat->pool->dev, (1UL << order) * PAGE_SIZE, vaddr, dat->addr,
attr);
+out:
kfree(dat);
+ return 1 << order;
}

/* Apply a new caching to an array of pages */
@@ -264,7 +273,7 @@ static void ttm_pool_type_fini(struct ttm_pool_type *pt)
mutex_unlock(&shrinker_lock);

list_for_each_entry_safe(p, tmp, &pt->pages, lru)
- ttm_pool_free_page(pt->pool, pt->caching, pt->order, p);
+ ttm_pool_free_page(p, pt->order);
}

/* Return the pool_type to use for the given caching and order */
@@ -307,7 +316,7 @@ static unsigned int ttm_pool_shrink(void)

p = ttm_pool_type_take(pt);
if (p) {
- ttm_pool_free_page(pt->pool, pt->caching, pt->order, p);
+ ttm_pool_free_page(p, pt->order);
num_freed = 1 << pt->order;
} else {
num_freed = 0;
@@ -322,13 +331,9 @@ static unsigned int ttm_pool_shrink(void)
/* Return the allocation order based for a page */
static unsigned int ttm_pool_page_order(struct ttm_pool *pool, struct page *p)
{
- if (pool->use_dma_alloc) {
- struct ttm_pool_page_dat *dat = (void *)p->private;
-
- return dat->vaddr & ~PAGE_MASK;
- }
+ struct ttm_pool_page_dat *dat = (void *)p->private;

- return p->private;
+ return dat->vaddr & ~PAGE_MASK;
}

/**
@@ -379,7 +384,7 @@ int ttm_pool_alloc(struct ttm_pool *pool, struct ttm_tt *tt,
if (p) {
apply_caching = true;
} else {
- p = ttm_pool_alloc_page(pool, gfp_flags, order);
+ p = ttm_pool_alloc_page(pool, gfp_flags, order, tt->caching);
if (p && PageHighMem(p))
apply_caching = true;
}
@@ -428,13 +433,13 @@ int ttm_pool_alloc(struct ttm_pool *pool, struct ttm_tt *tt,
ttm_mem_global_free_page(&ttm_mem_glob, p, (1 << order) * PAGE_SIZE);

error_free_page:
- ttm_pool_free_page(pool, tt->caching, order, p);
+ ttm_pool_free_page(p, order);

error_free_all:
num_pages = tt->num_pages - num_pages;
for (i = 0; i < num_pages; ) {
order = ttm_pool_page_order(pool, tt->pages[i]);
- ttm_pool_free_page(pool, tt->caching, order, tt->pages[i]);
+ ttm_pool_free_page(tt->pages[i], order);
i += 1 << order;
}

@@ -470,8 +475,7 @@ void ttm_pool_free(struct ttm_pool *pool, struct ttm_tt *tt)
if (pt)
ttm_pool_type_give(pt, tt->pages[i]);
else
- ttm_pool_free_page(pool, tt->caching, order,
- tt->pages[i]);
+ ttm_pool_free_page(tt->pages[i], order);

i += num_pages;
}
--
2.25.1