[PATCH 1/5] drm/prime: fix potential race in drm_gem_map_detach

[PATCH 1/5] drm/prime: fix potential race in drm_gem_map_detach

[Christian König]

Unpin the GEM object only after freeing the sg table.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/drm_prime.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index e82a976f0fba..c38dacda6119 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -230,26 +230,26 @@ void drm_gem_map_detach(struct dma_buf *dma_buf,
 	struct drm_prime_attachment *prime_attach = attach->priv;
 	struct drm_gem_object *obj = dma_buf->priv;
 	struct drm_device *dev = obj->dev;
-	struct sg_table *sgt;
 
-	if (dev->driver->gem_prime_unpin)
-		dev->driver->gem_prime_unpin(obj);
+	if (prime_attach) {
+		struct sg_table *sgt = prime_attach->sgt;
 
-	if (!prime_attach)
-		return;
-
-	sgt = prime_attach->sgt;
-	if (sgt) {
-		if (prime_attach->dir != DMA_NONE)
-			dma_unmap_sg_attrs(attach->dev, sgt->sgl, sgt->nents,
-					   prime_attach->dir,
-					   DMA_ATTR_SKIP_CPU_SYNC);
-		sg_free_table(sgt);
+		if (sgt) {
+			if (prime_attach->dir != DMA_NONE)
+				dma_unmap_sg_attrs(attach->dev, sgt->sgl,
+						   sgt->nents,
+						   prime_attach->dir,
+						   DMA_ATTR_SKIP_CPU_SYNC);
+			sg_free_table(sgt);
+		}
+
+		kfree(sgt);
+		kfree(prime_attach);
+		attach->priv = NULL;
 	}
 
-	kfree(sgt);
-	kfree(prime_attach);
-	attach->priv = NULL;
+	if (dev->driver->gem_prime_unpin)
+		dev->driver->gem_prime_unpin(obj);
 }
 EXPORT_SYMBOL(drm_gem_map_detach);
 
-- 
2.14.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/5] drm/prime: make the pages array optional for drm_prime_sg_to_page_addr_arrays

[Christian König]

Most of the time we only need the dma addresses.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/drm_prime.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index c38dacda6119..7856a9b3f8a8 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -922,40 +922,40 @@ EXPORT_SYMBOL(drm_prime_pages_to_sg);
 /**
  * drm_prime_sg_to_page_addr_arrays - convert an sg table into a page array
  * @sgt: scatter-gather table to convert
- * @pages: array of page pointers to store the page array in
+ * @pages: optional array of page pointers to store the page array in
  * @addrs: optional array to store the dma bus address of each page
- * @max_pages: size of both the passed-in arrays
+ * @max_entries: size of both the passed-in arrays
  *
  * Exports an sg table into an array of pages and addresses. This is currently
  * required by the TTM driver in order to do correct fault handling.
  */
 int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt, struct page **pages,
-				     dma_addr_t *addrs, int max_pages)
+				     dma_addr_t *addrs, int max_entries)
 {
 	unsigned count;
 	struct scatterlist *sg;
 	struct page *page;
-	u32 len;
-	int pg_index;
+	u32 len, index;
 	dma_addr_t addr;
 
-	pg_index = 0;
+	index = 0;
 	for_each_sg(sgt->sgl, sg, sgt->nents, count) {
 		len = sg->length;
 		page = sg_page(sg);
 		addr = sg_dma_address(sg);
 
 		while (len > 0) {
-			if (WARN_ON(pg_index >= max_pages))
+			if (WARN_ON(index >= max_entries))
 				return -1;
-			pages[pg_index] = page;
+			if (pages)
+				pages[index] = page;
 			if (addrs)
-				addrs[pg_index] = addr;
+				addrs[index] = addr;
 
 			page++;
 			addr += PAGE_SIZE;
 			len -= PAGE_SIZE;
-			pg_index++;
+			index++;
 		}
 	}
 	return 0;
-- 
2.14.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/5] drm/prime: make the pages array optional for drm_prime_sg_to_page_addr_arrays

[Daniel Vetter]

On Tue, Feb 27, 2018 at 12:49:57PM +0100, Christian König wrote:
> Most of the time we only need the dma addresses.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/drm_prime.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index c38dacda6119..7856a9b3f8a8 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -922,40 +922,40 @@ EXPORT_SYMBOL(drm_prime_pages_to_sg);
>  /**
>   * drm_prime_sg_to_page_addr_arrays - convert an sg table into a page array
>   * @sgt: scatter-gather table to convert
> - * @pages: array of page pointers to store the page array in
> + * @pages: optional array of page pointers to store the page array in
>   * @addrs: optional array to store the dma bus address of each page
> - * @max_pages: size of both the passed-in arrays
> + * @max_entries: size of both the passed-in arrays
>   *
>   * Exports an sg table into an array of pages and addresses. This is currently
>   * required by the TTM driver in order to do correct fault handling.
>   */

Can't we just teach ttm to use sgts wherever needed, and deprecate
exporting dma-bufs to page arrays (which really breaks the abstraction
entirely and was just a quick hack to get things going that stuck around
for years). Last time I looked into ttm the only thing it did is convert
it back to sgts again (after calling dma_map once more, which the exporter
should have done already for you).
-Daniel

>  int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt, struct page **pages,
> -				     dma_addr_t *addrs, int max_pages)
> +				     dma_addr_t *addrs, int max_entries)
>  {
>  	unsigned count;
>  	struct scatterlist *sg;
>  	struct page *page;
> -	u32 len;
> -	int pg_index;
> +	u32 len, index;
>  	dma_addr_t addr;
>  
> -	pg_index = 0;
> +	index = 0;
>  	for_each_sg(sgt->sgl, sg, sgt->nents, count) {
>  		len = sg->length;
>  		page = sg_page(sg);
>  		addr = sg_dma_address(sg);
>  
>  		while (len > 0) {
> -			if (WARN_ON(pg_index >= max_pages))
> +			if (WARN_ON(index >= max_entries))
>  				return -1;
> -			pages[pg_index] = page;
> +			if (pages)
> +				pages[index] = page;
>  			if (addrs)
> -				addrs[pg_index] = addr;
> +				addrs[index] = addr;
>  
>  			page++;
>  			addr += PAGE_SIZE;
>  			len -= PAGE_SIZE;
> -			pg_index++;
> +			index++;
>  		}
>  	}
>  	return 0;
> -- 
> 2.14.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/5] drm/prime: make the pages array optional for drm_prime_sg_to_page_addr_arrays

[Christian König]

Am 06.03.2018 um 10:21 schrieb Daniel Vetter:
> On Tue, Feb 27, 2018 at 12:49:57PM +0100, Christian König wrote:
>> Most of the time we only need the dma addresses.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>   drivers/gpu/drm/drm_prime.c | 20 ++++++++++----------
>>   1 file changed, 10 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
>> index c38dacda6119..7856a9b3f8a8 100644
>> --- a/drivers/gpu/drm/drm_prime.c
>> +++ b/drivers/gpu/drm/drm_prime.c
>> @@ -922,40 +922,40 @@ EXPORT_SYMBOL(drm_prime_pages_to_sg);
>>   /**
>>    * drm_prime_sg_to_page_addr_arrays - convert an sg table into a page array
>>    * @sgt: scatter-gather table to convert
>> - * @pages: array of page pointers to store the page array in
>> + * @pages: optional array of page pointers to store the page array in
>>    * @addrs: optional array to store the dma bus address of each page
>> - * @max_pages: size of both the passed-in arrays
>> + * @max_entries: size of both the passed-in arrays
>>    *
>>    * Exports an sg table into an array of pages and addresses. This is currently
>>    * required by the TTM driver in order to do correct fault handling.
>>    */
> Can't we just teach ttm to use sgts wherever needed, and deprecate
> exporting dma-bufs to page arrays (which really breaks the abstraction
> entirely and was just a quick hack to get things going that stuck around
> for years). Last time I looked into ttm the only thing it did is convert
> it back to sgts again (after calling dma_map once more, which the exporter
> should have done already for you).

Thought about that as well, but the problem here isn't TTM.

We need to be able to access the SGT by an index in amdgpu to be able to 
build up the VM page tables and that is not possible because the SGT is 
potentially chained.

We could add a new sg_table access helper function to work around that 
thought.

BTW: TTM isn't mapping anything in that case, we just fill in the arrays 
from the SGT.

Christian.

> -Daniel
>
>>   int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt, struct page **pages,
>> -				     dma_addr_t *addrs, int max_pages)
>> +				     dma_addr_t *addrs, int max_entries)
>>   {
>>   	unsigned count;
>>   	struct scatterlist *sg;
>>   	struct page *page;
>> -	u32 len;
>> -	int pg_index;
>> +	u32 len, index;
>>   	dma_addr_t addr;
>>   
>> -	pg_index = 0;
>> +	index = 0;
>>   	for_each_sg(sgt->sgl, sg, sgt->nents, count) {
>>   		len = sg->length;
>>   		page = sg_page(sg);
>>   		addr = sg_dma_address(sg);
>>   
>>   		while (len > 0) {
>> -			if (WARN_ON(pg_index >= max_pages))
>> +			if (WARN_ON(index >= max_entries))
>>   				return -1;
>> -			pages[pg_index] = page;
>> +			if (pages)
>> +				pages[index] = page;
>>   			if (addrs)
>> -				addrs[pg_index] = addr;
>> +				addrs[index] = addr;
>>   
>>   			page++;
>>   			addr += PAGE_SIZE;
>>   			len -= PAGE_SIZE;
>> -			pg_index++;
>> +			index++;
>>   		}
>>   	}
>>   	return 0;
>> -- 
>> 2.14.1
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/5] drm/prime: make the pages array optional for drm_prime_sg_to_page_addr_arrays

[Daniel Vetter]

On Tue, Mar 06, 2018 at 10:25:03AM +0100, Christian König wrote:
> Am 06.03.2018 um 10:21 schrieb Daniel Vetter:
> > On Tue, Feb 27, 2018 at 12:49:57PM +0100, Christian König wrote:
> > > Most of the time we only need the dma addresses.
> > > 
> > > Signed-off-by: Christian König <christian.koenig@amd.com>
> > > ---
> > >   drivers/gpu/drm/drm_prime.c | 20 ++++++++++----------
> > >   1 file changed, 10 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> > > index c38dacda6119..7856a9b3f8a8 100644
> > > --- a/drivers/gpu/drm/drm_prime.c
> > > +++ b/drivers/gpu/drm/drm_prime.c
> > > @@ -922,40 +922,40 @@ EXPORT_SYMBOL(drm_prime_pages_to_sg);
> > >   /**
> > >    * drm_prime_sg_to_page_addr_arrays - convert an sg table into a page array
> > >    * @sgt: scatter-gather table to convert
> > > - * @pages: array of page pointers to store the page array in
> > > + * @pages: optional array of page pointers to store the page array in
> > >    * @addrs: optional array to store the dma bus address of each page
> > > - * @max_pages: size of both the passed-in arrays
> > > + * @max_entries: size of both the passed-in arrays
> > >    *
> > >    * Exports an sg table into an array of pages and addresses. This is currently
> > >    * required by the TTM driver in order to do correct fault handling.
> > >    */
> > Can't we just teach ttm to use sgts wherever needed, and deprecate
> > exporting dma-bufs to page arrays (which really breaks the abstraction
> > entirely and was just a quick hack to get things going that stuck around
> > for years). Last time I looked into ttm the only thing it did is convert
> > it back to sgts again (after calling dma_map once more, which the exporter
> > should have done already for you).
> 
> Thought about that as well, but the problem here isn't TTM.
> 
> We need to be able to access the SGT by an index in amdgpu to be able to
> build up the VM page tables and that is not possible because the SGT is
> potentially chained.
> 
> We could add a new sg_table access helper function to work around that
> thought.

There's some neat per-page sgt iter functions that we've build for i915.
See i915_gem_gtt.c. But yeah that's probably a pile more work, but imo
from the i915 code shuffling the end result looks fairly neat.
-Daniel
> 
> BTW: TTM isn't mapping anything in that case, we just fill in the arrays
> from the SGT.
> 
> Christian.
> 
> > -Daniel
> > 
> > >   int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt, struct page **pages,
> > > -				     dma_addr_t *addrs, int max_pages)
> > > +				     dma_addr_t *addrs, int max_entries)
> > >   {
> > >   	unsigned count;
> > >   	struct scatterlist *sg;
> > >   	struct page *page;
> > > -	u32 len;
> > > -	int pg_index;
> > > +	u32 len, index;
> > >   	dma_addr_t addr;
> > > -	pg_index = 0;
> > > +	index = 0;
> > >   	for_each_sg(sgt->sgl, sg, sgt->nents, count) {
> > >   		len = sg->length;
> > >   		page = sg_page(sg);
> > >   		addr = sg_dma_address(sg);
> > >   		while (len > 0) {
> > > -			if (WARN_ON(pg_index >= max_pages))
> > > +			if (WARN_ON(index >= max_entries))
> > >   				return -1;
> > > -			pages[pg_index] = page;
> > > +			if (pages)
> > > +				pages[index] = page;
> > >   			if (addrs)
> > > -				addrs[pg_index] = addr;
> > > +				addrs[index] = addr;
> > >   			page++;
> > >   			addr += PAGE_SIZE;
> > >   			len -= PAGE_SIZE;
> > > -			pg_index++;
> > > +			index++;
> > >   		}
> > >   	}
> > >   	return 0;
> > > -- 
> > > 2.14.1
> > > 
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/5] drm/ttm: move ttm_tt defines into ttm_tt.h

[Christian König]

Let's stop mangling everything in a single header and create one header
per object instead.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/ttm/ttm_tt.c    |   6 -
 include/drm/ttm/ttm_bo_driver.h | 237 +---------------------------------
 include/drm/ttm/ttm_tt.h        | 272 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 273 insertions(+), 242 deletions(-)
 create mode 100644 include/drm/ttm/ttm_tt.h

diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index 0ee3b8f11605..8e0b525cda00 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -31,17 +31,11 @@
 #define pr_fmt(fmt) "[TTM] " fmt
 
 #include <linux/sched.h>
-#include <linux/highmem.h>
 #include <linux/pagemap.h>
 #include <linux/shmem_fs.h>
 #include <linux/file.h>
-#include <linux/swap.h>
-#include <linux/slab.h>
-#include <linux/export.h>
 #include <drm/drm_cache.h>
-#include <drm/ttm/ttm_module.h>
 #include <drm/ttm/ttm_bo_driver.h>
-#include <drm/ttm/ttm_placement.h>
 #include <drm/ttm/ttm_page_alloc.h>
 #ifdef CONFIG_X86
 #include <asm/set_memory.h>
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 4312b5326f0b..f8e2515b401f 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -42,111 +42,10 @@
 #include "ttm_memory.h"
 #include "ttm_module.h"
 #include "ttm_placement.h"
+#include "ttm_tt.h"
 
 #define TTM_MAX_BO_PRIORITY	4U
 
-struct ttm_backend_func {
-	/**
-	 * struct ttm_backend_func member bind
-	 *
-	 * @ttm: Pointer to a struct ttm_tt.
-	 * @bo_mem: Pointer to a struct ttm_mem_reg describing the
-	 * memory type and location for binding.
-	 *
-	 * Bind the backend pages into the aperture in the location
-	 * indicated by @bo_mem. This function should be able to handle
-	 * differences between aperture and system page sizes.
-	 */
-	int (*bind) (struct ttm_tt *ttm, struct ttm_mem_reg *bo_mem);
-
-	/**
-	 * struct ttm_backend_func member unbind
-	 *
-	 * @ttm: Pointer to a struct ttm_tt.
-	 *
-	 * Unbind previously bound backend pages. This function should be
-	 * able to handle differences between aperture and system page sizes.
-	 */
-	int (*unbind) (struct ttm_tt *ttm);
-
-	/**
-	 * struct ttm_backend_func member destroy
-	 *
-	 * @ttm: Pointer to a struct ttm_tt.
-	 *
-	 * Destroy the backend. This will be call back from ttm_tt_destroy so
-	 * don't call ttm_tt_destroy from the callback or infinite loop.
-	 */
-	void (*destroy) (struct ttm_tt *ttm);
-};
-
-#define TTM_PAGE_FLAG_WRITE           (1 << 3)
-#define TTM_PAGE_FLAG_SWAPPED         (1 << 4)
-#define TTM_PAGE_FLAG_PERSISTENT_SWAP (1 << 5)
-#define TTM_PAGE_FLAG_ZERO_ALLOC      (1 << 6)
-#define TTM_PAGE_FLAG_DMA32           (1 << 7)
-#define TTM_PAGE_FLAG_SG              (1 << 8)
-#define TTM_PAGE_FLAG_NO_RETRY	      (1 << 9)
-
-enum ttm_caching_state {
-	tt_uncached,
-	tt_wc,
-	tt_cached
-};
-
-/**
- * struct ttm_tt
- *
- * @bdev: Pointer to a struct ttm_bo_device.
- * @func: Pointer to a struct ttm_backend_func that describes
- * the backend methods.
- * pointer.
- * @pages: Array of pages backing the data.
- * @num_pages: Number of pages in the page array.
- * @bdev: Pointer to the current struct ttm_bo_device.
- * @be: Pointer to the ttm backend.
- * @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.
- *
- * This is a structure holding the pages, caching- and aperture binding
- * status for a buffer object that isn't backed by fixed (VRAM / AGP)
- * memory.
- */
-
-struct ttm_tt {
-	struct ttm_bo_device *bdev;
-	struct ttm_backend_func *func;
-	struct page **pages;
-	uint32_t page_flags;
-	unsigned long num_pages;
-	struct sg_table *sg; /* for SG objects via dma-buf */
-	struct file *swap_storage;
-	enum ttm_caching_state caching_state;
-	enum {
-		tt_bound,
-		tt_unbound,
-		tt_unpopulated,
-	} state;
-};
-
-/**
- * struct ttm_dma_tt
- *
- * @ttm: Base ttm_tt struct.
- * @dma_address: The DMA (bus) addresses of the pages
- * @pages_list: used by some page allocation backend
- *
- * This is a structure holding the pages, caching- and aperture binding
- * status for a buffer object that isn't backed by fixed (VRAM / AGP)
- * memory.
- */
-struct ttm_dma_tt {
-	struct ttm_tt ttm;
-	dma_addr_t *dma_address;
-	struct list_head pages_list;
-};
-
 #define TTM_MEMTYPE_FLAG_FIXED         (1 << 0)	/* Fixed (on-card) PCI memory */
 #define TTM_MEMTYPE_FLAG_MAPPABLE      (1 << 1)	/* Memory mappable */
 #define TTM_MEMTYPE_FLAG_CMA           (1 << 3)	/* Can't map aperture */
@@ -610,117 +509,6 @@ ttm_flag_masked(uint32_t *old, uint32_t new, uint32_t mask)
 	return *old;
 }
 
-/**
- * ttm_tt_create
- *
- * @bo: pointer to a struct ttm_buffer_object
- * @zero_alloc: true if allocated pages needs to be zeroed
- *
- * Make sure we have a TTM structure allocated for the given BO.
- * No pages are actually allocated.
- */
-int ttm_tt_create(struct ttm_buffer_object *bo, bool zero_alloc);
-
-/**
- * ttm_tt_init
- *
- * @ttm: The struct ttm_tt.
- * @bdev: pointer to a struct ttm_bo_device:
- * @size: Size of the data needed backing.
- * @page_flags: Page flags as identified by TTM_PAGE_FLAG_XX flags.
- *
- * Create a struct ttm_tt to back data with system memory pages.
- * No pages are actually allocated.
- * Returns:
- * NULL: Out of memory.
- */
-int ttm_tt_init(struct ttm_tt *ttm, struct ttm_bo_device *bdev,
-		unsigned long size, uint32_t page_flags);
-int ttm_dma_tt_init(struct ttm_dma_tt *ttm_dma, struct ttm_bo_device *bdev,
-		    unsigned long size, uint32_t page_flags);
-
-/**
- * ttm_tt_fini
- *
- * @ttm: the ttm_tt structure.
- *
- * Free memory of ttm_tt structure
- */
-void ttm_tt_fini(struct ttm_tt *ttm);
-void ttm_dma_tt_fini(struct ttm_dma_tt *ttm_dma);
-
-/**
- * ttm_ttm_bind:
- *
- * @ttm: The struct ttm_tt containing backing pages.
- * @bo_mem: The struct ttm_mem_reg identifying the binding location.
- *
- * Bind the pages of @ttm to an aperture location identified by @bo_mem
- */
-int ttm_tt_bind(struct ttm_tt *ttm, struct ttm_mem_reg *bo_mem,
-		struct ttm_operation_ctx *ctx);
-
-/**
- * ttm_ttm_destroy:
- *
- * @ttm: The struct ttm_tt.
- *
- * Unbind, unpopulate and destroy common struct ttm_tt.
- */
-void ttm_tt_destroy(struct ttm_tt *ttm);
-
-/**
- * ttm_ttm_unbind:
- *
- * @ttm: The struct ttm_tt.
- *
- * Unbind a struct ttm_tt.
- */
-void ttm_tt_unbind(struct ttm_tt *ttm);
-
-/**
- * ttm_tt_swapin:
- *
- * @ttm: The struct ttm_tt.
- *
- * Swap in a previously swap out ttm_tt.
- */
-int ttm_tt_swapin(struct ttm_tt *ttm);
-
-/**
- * ttm_tt_set_placement_caching:
- *
- * @ttm A struct ttm_tt the backing pages of which will change caching policy.
- * @placement: Flag indicating the desired caching policy.
- *
- * This function will change caching policy of any default kernel mappings of
- * the pages backing @ttm. If changing from cached to uncached or
- * write-combined,
- * all CPU caches will first be flushed to make sure the data of the pages
- * hit RAM. This function may be very costly as it involves global TLB
- * and cache flushes and potential page splitting / combining.
- */
-int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t placement);
-int ttm_tt_swapout(struct ttm_tt *ttm, struct file *persistent_swap_storage);
-
-/**
- * ttm_tt_populate - allocate pages for a ttm
- *
- * @ttm: Pointer to the ttm_tt structure
- *
- * Calls the driver method to allocate pages for a ttm
- */
-int ttm_tt_populate(struct ttm_tt *ttm, struct ttm_operation_ctx *ctx);
-
-/**
- * ttm_tt_unpopulate - free pages from a ttm
- *
- * @ttm: Pointer to the ttm_tt structure
- *
- * Calls the driver method to free all pages from a ttm
- */
-void ttm_tt_unpopulate(struct ttm_tt *ttm);
-
 /*
  * ttm_bo.c
  */
@@ -1074,27 +862,4 @@ pgprot_t ttm_io_prot(uint32_t caching_flags, pgprot_t tmp);
 
 extern const struct ttm_mem_type_manager_func ttm_bo_manager_func;
 
-#if IS_ENABLED(CONFIG_AGP)
-#include <linux/agp_backend.h>
-
-/**
- * ttm_agp_tt_create
- *
- * @bdev: Pointer to a struct ttm_bo_device.
- * @bridge: The agp bridge this device is sitting on.
- * @size: Size of the data needed backing.
- * @page_flags: Page flags as identified by TTM_PAGE_FLAG_XX flags.
- *
- *
- * Create a TTM backend that uses the indicated AGP bridge as an aperture
- * for TT memory. This function uses the linux agpgart interface to
- * bind and unbind memory backing a ttm_tt.
- */
-struct ttm_tt *ttm_agp_tt_create(struct ttm_bo_device *bdev,
-				 struct agp_bridge_data *bridge,
-				 unsigned long size, uint32_t page_flags);
-int ttm_agp_tt_populate(struct ttm_tt *ttm, struct ttm_operation_ctx *ctx);
-void ttm_agp_tt_unpopulate(struct ttm_tt *ttm);
-#endif
-
 #endif
diff --git a/include/drm/ttm/ttm_tt.h b/include/drm/ttm/ttm_tt.h
new file mode 100644
index 000000000000..9c78556b488e
--- /dev/null
+++ b/include/drm/ttm/ttm_tt.h
@@ -0,0 +1,272 @@
+/**************************************************************************
+ *
+ * Copyright (c) 2006-2009 Vmware, Inc., Palo Alto, CA., USA
+ * 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 above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial portions
+ * of the Software.
+ *
+ * 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.
+ *
+ **************************************************************************/
+#ifndef _TTM_TT_H_
+#define _TTM_TT_H_
+
+#include <linux/types.h>
+
+struct ttm_tt;
+struct ttm_mem_reg;
+struct ttm_buffer_object;
+struct ttm_operation_ctx;
+
+#define TTM_PAGE_FLAG_WRITE           (1 << 3)
+#define TTM_PAGE_FLAG_SWAPPED         (1 << 4)
+#define TTM_PAGE_FLAG_PERSISTENT_SWAP (1 << 5)
+#define TTM_PAGE_FLAG_ZERO_ALLOC      (1 << 6)
+#define TTM_PAGE_FLAG_DMA32           (1 << 7)
+#define TTM_PAGE_FLAG_SG              (1 << 8)
+#define TTM_PAGE_FLAG_NO_RETRY	      (1 << 9)
+
+enum ttm_caching_state {
+	tt_uncached,
+	tt_wc,
+	tt_cached
+};
+
+struct ttm_backend_func {
+	/**
+	 * struct ttm_backend_func member bind
+	 *
+	 * @ttm: Pointer to a struct ttm_tt.
+	 * @bo_mem: Pointer to a struct ttm_mem_reg describing the
+	 * memory type and location for binding.
+	 *
+	 * Bind the backend pages into the aperture in the location
+	 * indicated by @bo_mem. This function should be able to handle
+	 * differences between aperture and system page sizes.
+	 */
+	int (*bind) (struct ttm_tt *ttm, struct ttm_mem_reg *bo_mem);
+
+	/**
+	 * struct ttm_backend_func member unbind
+	 *
+	 * @ttm: Pointer to a struct ttm_tt.
+	 *
+	 * Unbind previously bound backend pages. This function should be
+	 * able to handle differences between aperture and system page sizes.
+	 */
+	int (*unbind) (struct ttm_tt *ttm);
+
+	/**
+	 * struct ttm_backend_func member destroy
+	 *
+	 * @ttm: Pointer to a struct ttm_tt.
+	 *
+	 * Destroy the backend. This will be call back from ttm_tt_destroy so
+	 * don't call ttm_tt_destroy from the callback or infinite loop.
+	 */
+	void (*destroy) (struct ttm_tt *ttm);
+};
+
+/**
+ * struct ttm_tt
+ *
+ * @bdev: Pointer to a struct ttm_bo_device.
+ * @func: Pointer to a struct ttm_backend_func that describes
+ * the backend methods.
+ * pointer.
+ * @pages: Array of pages backing the data.
+ * @num_pages: Number of pages in the page array.
+ * @bdev: Pointer to the current struct ttm_bo_device.
+ * @be: Pointer to the ttm backend.
+ * @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.
+ *
+ * This is a structure holding the pages, caching- and aperture binding
+ * status for a buffer object that isn't backed by fixed (VRAM / AGP)
+ * memory.
+ */
+struct ttm_tt {
+	struct ttm_bo_device *bdev;
+	struct ttm_backend_func *func;
+	struct page **pages;
+	uint32_t page_flags;
+	unsigned long num_pages;
+	struct sg_table *sg; /* for SG objects via dma-buf */
+	struct file *swap_storage;
+	enum ttm_caching_state caching_state;
+	enum {
+		tt_bound,
+		tt_unbound,
+		tt_unpopulated,
+	} state;
+};
+
+/**
+ * struct ttm_dma_tt
+ *
+ * @ttm: Base ttm_tt struct.
+ * @dma_address: The DMA (bus) addresses of the pages
+ * @pages_list: used by some page allocation backend
+ *
+ * This is a structure holding the pages, caching- and aperture binding
+ * status for a buffer object that isn't backed by fixed (VRAM / AGP)
+ * memory.
+ */
+struct ttm_dma_tt {
+	struct ttm_tt ttm;
+	dma_addr_t *dma_address;
+	struct list_head pages_list;
+};
+
+/**
+ * ttm_tt_create
+ *
+ * @bo: pointer to a struct ttm_buffer_object
+ * @zero_alloc: true if allocated pages needs to be zeroed
+ *
+ * Make sure we have a TTM structure allocated for the given BO.
+ * No pages are actually allocated.
+ */
+int ttm_tt_create(struct ttm_buffer_object *bo, bool zero_alloc);
+
+/**
+ * ttm_tt_init
+ *
+ * @ttm: The struct ttm_tt.
+ * @bdev: pointer to a struct ttm_bo_device:
+ * @size: Size of the data needed backing.
+ * @page_flags: Page flags as identified by TTM_PAGE_FLAG_XX flags.
+ *
+ * Create a struct ttm_tt to back data with system memory pages.
+ * No pages are actually allocated.
+ * Returns:
+ * NULL: Out of memory.
+ */
+int ttm_tt_init(struct ttm_tt *ttm, struct ttm_bo_device *bdev,
+		unsigned long size, uint32_t page_flags);
+int ttm_dma_tt_init(struct ttm_dma_tt *ttm_dma, struct ttm_bo_device *bdev,
+		    unsigned long size, uint32_t page_flags);
+
+/**
+ * ttm_tt_fini
+ *
+ * @ttm: the ttm_tt structure.
+ *
+ * Free memory of ttm_tt structure
+ */
+void ttm_tt_fini(struct ttm_tt *ttm);
+void ttm_dma_tt_fini(struct ttm_dma_tt *ttm_dma);
+
+/**
+ * ttm_ttm_bind:
+ *
+ * @ttm: The struct ttm_tt containing backing pages.
+ * @bo_mem: The struct ttm_mem_reg identifying the binding location.
+ *
+ * Bind the pages of @ttm to an aperture location identified by @bo_mem
+ */
+int ttm_tt_bind(struct ttm_tt *ttm, struct ttm_mem_reg *bo_mem,
+		struct ttm_operation_ctx *ctx);
+
+/**
+ * ttm_ttm_destroy:
+ *
+ * @ttm: The struct ttm_tt.
+ *
+ * Unbind, unpopulate and destroy common struct ttm_tt.
+ */
+void ttm_tt_destroy(struct ttm_tt *ttm);
+
+/**
+ * ttm_ttm_unbind:
+ *
+ * @ttm: The struct ttm_tt.
+ *
+ * Unbind a struct ttm_tt.
+ */
+void ttm_tt_unbind(struct ttm_tt *ttm);
+
+/**
+ * ttm_tt_swapin:
+ *
+ * @ttm: The struct ttm_tt.
+ *
+ * Swap in a previously swap out ttm_tt.
+ */
+int ttm_tt_swapin(struct ttm_tt *ttm);
+
+/**
+ * ttm_tt_set_placement_caching:
+ *
+ * @ttm A struct ttm_tt the backing pages of which will change caching policy.
+ * @placement: Flag indicating the desired caching policy.
+ *
+ * This function will change caching policy of any default kernel mappings of
+ * the pages backing @ttm. If changing from cached to uncached or
+ * write-combined,
+ * all CPU caches will first be flushed to make sure the data of the pages
+ * hit RAM. This function may be very costly as it involves global TLB
+ * and cache flushes and potential page splitting / combining.
+ */
+int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t placement);
+int ttm_tt_swapout(struct ttm_tt *ttm, struct file *persistent_swap_storage);
+
+/**
+ * ttm_tt_populate - allocate pages for a ttm
+ *
+ * @ttm: Pointer to the ttm_tt structure
+ *
+ * Calls the driver method to allocate pages for a ttm
+ */
+int ttm_tt_populate(struct ttm_tt *ttm, struct ttm_operation_ctx *ctx);
+
+/**
+ * ttm_tt_unpopulate - free pages from a ttm
+ *
+ * @ttm: Pointer to the ttm_tt structure
+ *
+ * Calls the driver method to free all pages from a ttm
+ */
+void ttm_tt_unpopulate(struct ttm_tt *ttm);
+
+#if IS_ENABLED(CONFIG_AGP)
+#include <linux/agp_backend.h>
+
+/**
+ * ttm_agp_tt_create
+ *
+ * @bdev: Pointer to a struct ttm_bo_device.
+ * @bridge: The agp bridge this device is sitting on.
+ * @size: Size of the data needed backing.
+ * @page_flags: Page flags as identified by TTM_PAGE_FLAG_XX flags.
+ *
+ *
+ * Create a TTM backend that uses the indicated AGP bridge as an aperture
+ * for TT memory. This function uses the linux agpgart interface to
+ * bind and unbind memory backing a ttm_tt.
+ */
+struct ttm_tt *ttm_agp_tt_create(struct ttm_bo_device *bdev,
+				 struct agp_bridge_data *bridge,
+				 unsigned long size, uint32_t page_flags);
+int ttm_agp_tt_populate(struct ttm_tt *ttm, struct ttm_operation_ctx *ctx);
+void ttm_agp_tt_unpopulate(struct ttm_tt *ttm);
+#endif
+
+#endif
-- 
2.14.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/5] drm/ttm: move ttm_tt defines into ttm_tt.h

[Christian König]

Hi Michel & Thomas,

any more comments on this? Or can I commit it?

Thanks,
Christian.

Am 27.02.2018 um 12:49 schrieb Christian König:
> Let's stop mangling everything in a single header and create one header
> per object instead.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/ttm/ttm_tt.c    |   6 -
>   include/drm/ttm/ttm_bo_driver.h | 237 +---------------------------------
>   include/drm/ttm/ttm_tt.h        | 272 ++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 273 insertions(+), 242 deletions(-)
>   create mode 100644 include/drm/ttm/ttm_tt.h
>
> diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
> index 0ee3b8f11605..8e0b525cda00 100644
> --- a/drivers/gpu/drm/ttm/ttm_tt.c
> +++ b/drivers/gpu/drm/ttm/ttm_tt.c
> @@ -31,17 +31,11 @@
>   #define pr_fmt(fmt) "[TTM] " fmt
>   
>   #include <linux/sched.h>
> -#include <linux/highmem.h>
>   #include <linux/pagemap.h>
>   #include <linux/shmem_fs.h>
>   #include <linux/file.h>
> -#include <linux/swap.h>
> -#include <linux/slab.h>
> -#include <linux/export.h>
>   #include <drm/drm_cache.h>
> -#include <drm/ttm/ttm_module.h>
>   #include <drm/ttm/ttm_bo_driver.h>
> -#include <drm/ttm/ttm_placement.h>
>   #include <drm/ttm/ttm_page_alloc.h>
>   #ifdef CONFIG_X86
>   #include <asm/set_memory.h>
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 4312b5326f0b..f8e2515b401f 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -42,111 +42,10 @@
>   #include "ttm_memory.h"
>   #include "ttm_module.h"
>   #include "ttm_placement.h"
> +#include "ttm_tt.h"
>   
>   #define TTM_MAX_BO_PRIORITY	4U
>   
> -struct ttm_backend_func {
> -	/**
> -	 * struct ttm_backend_func member bind
> -	 *
> -	 * @ttm: Pointer to a struct ttm_tt.
> -	 * @bo_mem: Pointer to a struct ttm_mem_reg describing the
> -	 * memory type and location for binding.
> -	 *
> -	 * Bind the backend pages into the aperture in the location
> -	 * indicated by @bo_mem. This function should be able to handle
> -	 * differences between aperture and system page sizes.
> -	 */
> -	int (*bind) (struct ttm_tt *ttm, struct ttm_mem_reg *bo_mem);
> -
> -	/**
> -	 * struct ttm_backend_func member unbind
> -	 *
> -	 * @ttm: Pointer to a struct ttm_tt.
> -	 *
> -	 * Unbind previously bound backend pages. This function should be
> -	 * able to handle differences between aperture and system page sizes.
> -	 */
> -	int (*unbind) (struct ttm_tt *ttm);
> -
> -	/**
> -	 * struct ttm_backend_func member destroy
> -	 *
> -	 * @ttm: Pointer to a struct ttm_tt.
> -	 *
> -	 * Destroy the backend. This will be call back from ttm_tt_destroy so
> -	 * don't call ttm_tt_destroy from the callback or infinite loop.
> -	 */
> -	void (*destroy) (struct ttm_tt *ttm);
> -};
> -
> -#define TTM_PAGE_FLAG_WRITE           (1 << 3)
> -#define TTM_PAGE_FLAG_SWAPPED         (1 << 4)
> -#define TTM_PAGE_FLAG_PERSISTENT_SWAP (1 << 5)
> -#define TTM_PAGE_FLAG_ZERO_ALLOC      (1 << 6)
> -#define TTM_PAGE_FLAG_DMA32           (1 << 7)
> -#define TTM_PAGE_FLAG_SG              (1 << 8)
> -#define TTM_PAGE_FLAG_NO_RETRY	      (1 << 9)
> -
> -enum ttm_caching_state {
> -	tt_uncached,
> -	tt_wc,
> -	tt_cached
> -};
> -
> -/**
> - * struct ttm_tt
> - *
> - * @bdev: Pointer to a struct ttm_bo_device.
> - * @func: Pointer to a struct ttm_backend_func that describes
> - * the backend methods.
> - * pointer.
> - * @pages: Array of pages backing the data.
> - * @num_pages: Number of pages in the page array.
> - * @bdev: Pointer to the current struct ttm_bo_device.
> - * @be: Pointer to the ttm backend.
> - * @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.
> - *
> - * This is a structure holding the pages, caching- and aperture binding
> - * status for a buffer object that isn't backed by fixed (VRAM / AGP)
> - * memory.
> - */
> -
> -struct ttm_tt {
> -	struct ttm_bo_device *bdev;
> -	struct ttm_backend_func *func;
> -	struct page **pages;
> -	uint32_t page_flags;
> -	unsigned long num_pages;
> -	struct sg_table *sg; /* for SG objects via dma-buf */
> -	struct file *swap_storage;
> -	enum ttm_caching_state caching_state;
> -	enum {
> -		tt_bound,
> -		tt_unbound,
> -		tt_unpopulated,
> -	} state;
> -};
> -
> -/**
> - * struct ttm_dma_tt
> - *
> - * @ttm: Base ttm_tt struct.
> - * @dma_address: The DMA (bus) addresses of the pages
> - * @pages_list: used by some page allocation backend
> - *
> - * This is a structure holding the pages, caching- and aperture binding
> - * status for a buffer object that isn't backed by fixed (VRAM / AGP)
> - * memory.
> - */
> -struct ttm_dma_tt {
> -	struct ttm_tt ttm;
> -	dma_addr_t *dma_address;
> -	struct list_head pages_list;
> -};
> -
>   #define TTM_MEMTYPE_FLAG_FIXED         (1 << 0)	/* Fixed (on-card) PCI memory */
>   #define TTM_MEMTYPE_FLAG_MAPPABLE      (1 << 1)	/* Memory mappable */
>   #define TTM_MEMTYPE_FLAG_CMA           (1 << 3)	/* Can't map aperture */
> @@ -610,117 +509,6 @@ ttm_flag_masked(uint32_t *old, uint32_t new, uint32_t mask)
>   	return *old;
>   }
>   
> -/**
> - * ttm_tt_create
> - *
> - * @bo: pointer to a struct ttm_buffer_object
> - * @zero_alloc: true if allocated pages needs to be zeroed
> - *
> - * Make sure we have a TTM structure allocated for the given BO.
> - * No pages are actually allocated.
> - */
> -int ttm_tt_create(struct ttm_buffer_object *bo, bool zero_alloc);
> -
> -/**
> - * ttm_tt_init
> - *
> - * @ttm: The struct ttm_tt.
> - * @bdev: pointer to a struct ttm_bo_device:
> - * @size: Size of the data needed backing.
> - * @page_flags: Page flags as identified by TTM_PAGE_FLAG_XX flags.
> - *
> - * Create a struct ttm_tt to back data with system memory pages.
> - * No pages are actually allocated.
> - * Returns:
> - * NULL: Out of memory.
> - */
> -int ttm_tt_init(struct ttm_tt *ttm, struct ttm_bo_device *bdev,
> -		unsigned long size, uint32_t page_flags);
> -int ttm_dma_tt_init(struct ttm_dma_tt *ttm_dma, struct ttm_bo_device *bdev,
> -		    unsigned long size, uint32_t page_flags);
> -
> -/**
> - * ttm_tt_fini
> - *
> - * @ttm: the ttm_tt structure.
> - *
> - * Free memory of ttm_tt structure
> - */
> -void ttm_tt_fini(struct ttm_tt *ttm);
> -void ttm_dma_tt_fini(struct ttm_dma_tt *ttm_dma);
> -
> -/**
> - * ttm_ttm_bind:
> - *
> - * @ttm: The struct ttm_tt containing backing pages.
> - * @bo_mem: The struct ttm_mem_reg identifying the binding location.
> - *
> - * Bind the pages of @ttm to an aperture location identified by @bo_mem
> - */
> -int ttm_tt_bind(struct ttm_tt *ttm, struct ttm_mem_reg *bo_mem,
> -		struct ttm_operation_ctx *ctx);
> -
> -/**
> - * ttm_ttm_destroy:
> - *
> - * @ttm: The struct ttm_tt.
> - *
> - * Unbind, unpopulate and destroy common struct ttm_tt.
> - */
> -void ttm_tt_destroy(struct ttm_tt *ttm);
> -
> -/**
> - * ttm_ttm_unbind:
> - *
> - * @ttm: The struct ttm_tt.
> - *
> - * Unbind a struct ttm_tt.
> - */
> -void ttm_tt_unbind(struct ttm_tt *ttm);
> -
> -/**
> - * ttm_tt_swapin:
> - *
> - * @ttm: The struct ttm_tt.
> - *
> - * Swap in a previously swap out ttm_tt.
> - */
> -int ttm_tt_swapin(struct ttm_tt *ttm);
> -
> -/**
> - * ttm_tt_set_placement_caching:
> - *
> - * @ttm A struct ttm_tt the backing pages of which will change caching policy.
> - * @placement: Flag indicating the desired caching policy.
> - *
> - * This function will change caching policy of any default kernel mappings of
> - * the pages backing @ttm. If changing from cached to uncached or
> - * write-combined,
> - * all CPU caches will first be flushed to make sure the data of the pages
> - * hit RAM. This function may be very costly as it involves global TLB
> - * and cache flushes and potential page splitting / combining.
> - */
> -int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t placement);
> -int ttm_tt_swapout(struct ttm_tt *ttm, struct file *persistent_swap_storage);
> -
> -/**
> - * ttm_tt_populate - allocate pages for a ttm
> - *
> - * @ttm: Pointer to the ttm_tt structure
> - *
> - * Calls the driver method to allocate pages for a ttm
> - */
> -int ttm_tt_populate(struct ttm_tt *ttm, struct ttm_operation_ctx *ctx);
> -
> -/**
> - * ttm_tt_unpopulate - free pages from a ttm
> - *
> - * @ttm: Pointer to the ttm_tt structure
> - *
> - * Calls the driver method to free all pages from a ttm
> - */
> -void ttm_tt_unpopulate(struct ttm_tt *ttm);
> -
>   /*
>    * ttm_bo.c
>    */
> @@ -1074,27 +862,4 @@ pgprot_t ttm_io_prot(uint32_t caching_flags, pgprot_t tmp);
>   
>   extern const struct ttm_mem_type_manager_func ttm_bo_manager_func;
>   
> -#if IS_ENABLED(CONFIG_AGP)
> -#include <linux/agp_backend.h>
> -
> -/**
> - * ttm_agp_tt_create
> - *
> - * @bdev: Pointer to a struct ttm_bo_device.
> - * @bridge: The agp bridge this device is sitting on.
> - * @size: Size of the data needed backing.
> - * @page_flags: Page flags as identified by TTM_PAGE_FLAG_XX flags.
> - *
> - *
> - * Create a TTM backend that uses the indicated AGP bridge as an aperture
> - * for TT memory. This function uses the linux agpgart interface to
> - * bind and unbind memory backing a ttm_tt.
> - */
> -struct ttm_tt *ttm_agp_tt_create(struct ttm_bo_device *bdev,
> -				 struct agp_bridge_data *bridge,
> -				 unsigned long size, uint32_t page_flags);
> -int ttm_agp_tt_populate(struct ttm_tt *ttm, struct ttm_operation_ctx *ctx);
> -void ttm_agp_tt_unpopulate(struct ttm_tt *ttm);
> -#endif
> -
>   #endif
> diff --git a/include/drm/ttm/ttm_tt.h b/include/drm/ttm/ttm_tt.h
> new file mode 100644
> index 000000000000..9c78556b488e
> --- /dev/null
> +++ b/include/drm/ttm/ttm_tt.h
> @@ -0,0 +1,272 @@
> +/**************************************************************************
> + *
> + * Copyright (c) 2006-2009 Vmware, Inc., Palo Alto, CA., USA
> + * 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 above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial portions
> + * of the Software.
> + *
> + * 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.
> + *
> + **************************************************************************/
> +#ifndef _TTM_TT_H_
> +#define _TTM_TT_H_
> +
> +#include <linux/types.h>
> +
> +struct ttm_tt;
> +struct ttm_mem_reg;
> +struct ttm_buffer_object;
> +struct ttm_operation_ctx;
> +
> +#define TTM_PAGE_FLAG_WRITE           (1 << 3)
> +#define TTM_PAGE_FLAG_SWAPPED         (1 << 4)
> +#define TTM_PAGE_FLAG_PERSISTENT_SWAP (1 << 5)
> +#define TTM_PAGE_FLAG_ZERO_ALLOC      (1 << 6)
> +#define TTM_PAGE_FLAG_DMA32           (1 << 7)
> +#define TTM_PAGE_FLAG_SG              (1 << 8)
> +#define TTM_PAGE_FLAG_NO_RETRY	      (1 << 9)
> +
> +enum ttm_caching_state {
> +	tt_uncached,
> +	tt_wc,
> +	tt_cached
> +};
> +
> +struct ttm_backend_func {
> +	/**
> +	 * struct ttm_backend_func member bind
> +	 *
> +	 * @ttm: Pointer to a struct ttm_tt.
> +	 * @bo_mem: Pointer to a struct ttm_mem_reg describing the
> +	 * memory type and location for binding.
> +	 *
> +	 * Bind the backend pages into the aperture in the location
> +	 * indicated by @bo_mem. This function should be able to handle
> +	 * differences between aperture and system page sizes.
> +	 */
> +	int (*bind) (struct ttm_tt *ttm, struct ttm_mem_reg *bo_mem);
> +
> +	/**
> +	 * struct ttm_backend_func member unbind
> +	 *
> +	 * @ttm: Pointer to a struct ttm_tt.
> +	 *
> +	 * Unbind previously bound backend pages. This function should be
> +	 * able to handle differences between aperture and system page sizes.
> +	 */
> +	int (*unbind) (struct ttm_tt *ttm);
> +
> +	/**
> +	 * struct ttm_backend_func member destroy
> +	 *
> +	 * @ttm: Pointer to a struct ttm_tt.
> +	 *
> +	 * Destroy the backend. This will be call back from ttm_tt_destroy so
> +	 * don't call ttm_tt_destroy from the callback or infinite loop.
> +	 */
> +	void (*destroy) (struct ttm_tt *ttm);
> +};
> +
> +/**
> + * struct ttm_tt
> + *
> + * @bdev: Pointer to a struct ttm_bo_device.
> + * @func: Pointer to a struct ttm_backend_func that describes
> + * the backend methods.
> + * pointer.
> + * @pages: Array of pages backing the data.
> + * @num_pages: Number of pages in the page array.
> + * @bdev: Pointer to the current struct ttm_bo_device.
> + * @be: Pointer to the ttm backend.
> + * @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.
> + *
> + * This is a structure holding the pages, caching- and aperture binding
> + * status for a buffer object that isn't backed by fixed (VRAM / AGP)
> + * memory.
> + */
> +struct ttm_tt {
> +	struct ttm_bo_device *bdev;
> +	struct ttm_backend_func *func;
> +	struct page **pages;
> +	uint32_t page_flags;
> +	unsigned long num_pages;
> +	struct sg_table *sg; /* for SG objects via dma-buf */
> +	struct file *swap_storage;
> +	enum ttm_caching_state caching_state;
> +	enum {
> +		tt_bound,
> +		tt_unbound,
> +		tt_unpopulated,
> +	} state;
> +};
> +
> +/**
> + * struct ttm_dma_tt
> + *
> + * @ttm: Base ttm_tt struct.
> + * @dma_address: The DMA (bus) addresses of the pages
> + * @pages_list: used by some page allocation backend
> + *
> + * This is a structure holding the pages, caching- and aperture binding
> + * status for a buffer object that isn't backed by fixed (VRAM / AGP)
> + * memory.
> + */
> +struct ttm_dma_tt {
> +	struct ttm_tt ttm;
> +	dma_addr_t *dma_address;
> +	struct list_head pages_list;
> +};
> +
> +/**
> + * ttm_tt_create
> + *
> + * @bo: pointer to a struct ttm_buffer_object
> + * @zero_alloc: true if allocated pages needs to be zeroed
> + *
> + * Make sure we have a TTM structure allocated for the given BO.
> + * No pages are actually allocated.
> + */
> +int ttm_tt_create(struct ttm_buffer_object *bo, bool zero_alloc);
> +
> +/**
> + * ttm_tt_init
> + *
> + * @ttm: The struct ttm_tt.
> + * @bdev: pointer to a struct ttm_bo_device:
> + * @size: Size of the data needed backing.
> + * @page_flags: Page flags as identified by TTM_PAGE_FLAG_XX flags.
> + *
> + * Create a struct ttm_tt to back data with system memory pages.
> + * No pages are actually allocated.
> + * Returns:
> + * NULL: Out of memory.
> + */
> +int ttm_tt_init(struct ttm_tt *ttm, struct ttm_bo_device *bdev,
> +		unsigned long size, uint32_t page_flags);
> +int ttm_dma_tt_init(struct ttm_dma_tt *ttm_dma, struct ttm_bo_device *bdev,
> +		    unsigned long size, uint32_t page_flags);
> +
> +/**
> + * ttm_tt_fini
> + *
> + * @ttm: the ttm_tt structure.
> + *
> + * Free memory of ttm_tt structure
> + */
> +void ttm_tt_fini(struct ttm_tt *ttm);
> +void ttm_dma_tt_fini(struct ttm_dma_tt *ttm_dma);
> +
> +/**
> + * ttm_ttm_bind:
> + *
> + * @ttm: The struct ttm_tt containing backing pages.
> + * @bo_mem: The struct ttm_mem_reg identifying the binding location.
> + *
> + * Bind the pages of @ttm to an aperture location identified by @bo_mem
> + */
> +int ttm_tt_bind(struct ttm_tt *ttm, struct ttm_mem_reg *bo_mem,
> +		struct ttm_operation_ctx *ctx);
> +
> +/**
> + * ttm_ttm_destroy:
> + *
> + * @ttm: The struct ttm_tt.
> + *
> + * Unbind, unpopulate and destroy common struct ttm_tt.
> + */
> +void ttm_tt_destroy(struct ttm_tt *ttm);
> +
> +/**
> + * ttm_ttm_unbind:
> + *
> + * @ttm: The struct ttm_tt.
> + *
> + * Unbind a struct ttm_tt.
> + */
> +void ttm_tt_unbind(struct ttm_tt *ttm);
> +
> +/**
> + * ttm_tt_swapin:
> + *
> + * @ttm: The struct ttm_tt.
> + *
> + * Swap in a previously swap out ttm_tt.
> + */
> +int ttm_tt_swapin(struct ttm_tt *ttm);
> +
> +/**
> + * ttm_tt_set_placement_caching:
> + *
> + * @ttm A struct ttm_tt the backing pages of which will change caching policy.
> + * @placement: Flag indicating the desired caching policy.
> + *
> + * This function will change caching policy of any default kernel mappings of
> + * the pages backing @ttm. If changing from cached to uncached or
> + * write-combined,
> + * all CPU caches will first be flushed to make sure the data of the pages
> + * hit RAM. This function may be very costly as it involves global TLB
> + * and cache flushes and potential page splitting / combining.
> + */
> +int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t placement);
> +int ttm_tt_swapout(struct ttm_tt *ttm, struct file *persistent_swap_storage);
> +
> +/**
> + * ttm_tt_populate - allocate pages for a ttm
> + *
> + * @ttm: Pointer to the ttm_tt structure
> + *
> + * Calls the driver method to allocate pages for a ttm
> + */
> +int ttm_tt_populate(struct ttm_tt *ttm, struct ttm_operation_ctx *ctx);
> +
> +/**
> + * ttm_tt_unpopulate - free pages from a ttm
> + *
> + * @ttm: Pointer to the ttm_tt structure
> + *
> + * Calls the driver method to free all pages from a ttm
> + */
> +void ttm_tt_unpopulate(struct ttm_tt *ttm);
> +
> +#if IS_ENABLED(CONFIG_AGP)
> +#include <linux/agp_backend.h>
> +
> +/**
> + * ttm_agp_tt_create
> + *
> + * @bdev: Pointer to a struct ttm_bo_device.
> + * @bridge: The agp bridge this device is sitting on.
> + * @size: Size of the data needed backing.
> + * @page_flags: Page flags as identified by TTM_PAGE_FLAG_XX flags.
> + *
> + *
> + * Create a TTM backend that uses the indicated AGP bridge as an aperture
> + * for TT memory. This function uses the linux agpgart interface to
> + * bind and unbind memory backing a ttm_tt.
> + */
> +struct ttm_tt *ttm_agp_tt_create(struct ttm_bo_device *bdev,
> +				 struct agp_bridge_data *bridge,
> +				 unsigned long size, uint32_t page_flags);
> +int ttm_agp_tt_populate(struct ttm_tt *ttm, struct ttm_operation_ctx *ctx);
> +void ttm_agp_tt_unpopulate(struct ttm_tt *ttm);
> +#endif
> +
> +#endif

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/5] drm/ttm: move ttm_tt defines into ttm_tt.h

[Michel Dänzer]

On 2018-03-06 10:13 AM, Christian König wrote:
> Hi Michel & Thomas,
> 
> any more comments on this? Or can I commit it?

Acked-by: Michel Dänzer <michel.daenzer@amd.com>


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/5] drm/ttm: move ttm_tt defines into ttm_tt.h

[Thomas Hellstrom]

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

On 03/06/2018 10:13 AM, Christian König wrote:
> Hi Michel & Thomas,
>
> any more comments on this? Or can I commit it?
>
> Thanks,
> Christian.
>
> Am 27.02.2018 um 12:49 schrieb Christian König:
>> Let's stop mangling everything in a single header and create one header
>> per object instead.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>   drivers/gpu/drm/ttm/ttm_tt.c    |   6 -
>>   include/drm/ttm/ttm_bo_driver.h | 237 
>> +---------------------------------
>>   include/drm/ttm/ttm_tt.h        | 272 
>> ++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 273 insertions(+), 242 deletions(-)
>>   create mode 100644 include/drm/ttm/ttm_tt.h
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
>> index 0ee3b8f11605..8e0b525cda00 100644
>> --- a/drivers/gpu/drm/ttm/ttm_tt.c
>> +++ b/drivers/gpu/drm/ttm/ttm_tt.c
>> @@ -31,17 +31,11 @@
>>   #define pr_fmt(fmt) "[TTM] " fmt
>>     #include <linux/sched.h>
>> -#include <linux/highmem.h>
>>   #include <linux/pagemap.h>
>>   #include <linux/shmem_fs.h>
>>   #include <linux/file.h>
>> -#include <linux/swap.h>
>> -#include <linux/slab.h>
>> -#include <linux/export.h>
>>   #include <drm/drm_cache.h>
>> -#include <drm/ttm/ttm_module.h>
>>   #include <drm/ttm/ttm_bo_driver.h>
>> -#include <drm/ttm/ttm_placement.h>
>>   #include <drm/ttm/ttm_page_alloc.h>
>>   #ifdef CONFIG_X86
>>   #include <asm/set_memory.h>
>> diff --git a/include/drm/ttm/ttm_bo_driver.h 
>> b/include/drm/ttm/ttm_bo_driver.h
>> index 4312b5326f0b..f8e2515b401f 100644
>> --- a/include/drm/ttm/ttm_bo_driver.h
>> +++ b/include/drm/ttm/ttm_bo_driver.h
>> @@ -42,111 +42,10 @@
>>   #include "ttm_memory.h"
>>   #include "ttm_module.h"
>>   #include "ttm_placement.h"
>> +#include "ttm_tt.h"
>>     #define TTM_MAX_BO_PRIORITY    4U
>>   -struct ttm_backend_func {
>> -    /**
>> -     * struct ttm_backend_func member bind
>> -     *
>> -     * @ttm: Pointer to a struct ttm_tt.
>> -     * @bo_mem: Pointer to a struct ttm_mem_reg describing the
>> -     * memory type and location for binding.
>> -     *
>> -     * Bind the backend pages into the aperture in the location
>> -     * indicated by @bo_mem. This function should be able to handle
>> -     * differences between aperture and system page sizes.
>> -     */
>> -    int (*bind) (struct ttm_tt *ttm, struct ttm_mem_reg *bo_mem);
>> -
>> -    /**
>> -     * struct ttm_backend_func member unbind
>> -     *
>> -     * @ttm: Pointer to a struct ttm_tt.
>> -     *
>> -     * Unbind previously bound backend pages. This function should be
>> -     * able to handle differences between aperture and system page 
>> sizes.
>> -     */
>> -    int (*unbind) (struct ttm_tt *ttm);
>> -
>> -    /**
>> -     * struct ttm_backend_func member destroy
>> -     *
>> -     * @ttm: Pointer to a struct ttm_tt.
>> -     *
>> -     * Destroy the backend. This will be call back from 
>> ttm_tt_destroy so
>> -     * don't call ttm_tt_destroy from the callback or infinite loop.
>> -     */
>> -    void (*destroy) (struct ttm_tt *ttm);
>> -};
>> -
>> -#define TTM_PAGE_FLAG_WRITE           (1 << 3)
>> -#define TTM_PAGE_FLAG_SWAPPED         (1 << 4)
>> -#define TTM_PAGE_FLAG_PERSISTENT_SWAP (1 << 5)
>> -#define TTM_PAGE_FLAG_ZERO_ALLOC      (1 << 6)
>> -#define TTM_PAGE_FLAG_DMA32           (1 << 7)
>> -#define TTM_PAGE_FLAG_SG              (1 << 8)
>> -#define TTM_PAGE_FLAG_NO_RETRY          (1 << 9)
>> -
>> -enum ttm_caching_state {
>> -    tt_uncached,
>> -    tt_wc,
>> -    tt_cached
>> -};
>> -
>> -/**
>> - * struct ttm_tt
>> - *
>> - * @bdev: Pointer to a struct ttm_bo_device.
>> - * @func: Pointer to a struct ttm_backend_func that describes
>> - * the backend methods.
>> - * pointer.
>> - * @pages: Array of pages backing the data.
>> - * @num_pages: Number of pages in the page array.
>> - * @bdev: Pointer to the current struct ttm_bo_device.
>> - * @be: Pointer to the ttm backend.
>> - * @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.
>> - *
>> - * This is a structure holding the pages, caching- and aperture binding
>> - * status for a buffer object that isn't backed by fixed (VRAM / AGP)
>> - * memory.
>> - */
>> -
>> -struct ttm_tt {
>> -    struct ttm_bo_device *bdev;
>> -    struct ttm_backend_func *func;
>> -    struct page **pages;
>> -    uint32_t page_flags;
>> -    unsigned long num_pages;
>> -    struct sg_table *sg; /* for SG objects via dma-buf */
>> -    struct file *swap_storage;
>> -    enum ttm_caching_state caching_state;
>> -    enum {
>> -        tt_bound,
>> -        tt_unbound,
>> -        tt_unpopulated,
>> -    } state;
>> -};
>> -
>> -/**
>> - * struct ttm_dma_tt
>> - *
>> - * @ttm: Base ttm_tt struct.
>> - * @dma_address: The DMA (bus) addresses of the pages
>> - * @pages_list: used by some page allocation backend
>> - *
>> - * This is a structure holding the pages, caching- and aperture binding
>> - * status for a buffer object that isn't backed by fixed (VRAM / AGP)
>> - * memory.
>> - */
>> -struct ttm_dma_tt {
>> -    struct ttm_tt ttm;
>> -    dma_addr_t *dma_address;
>> -    struct list_head pages_list;
>> -};
>> -
>>   #define TTM_MEMTYPE_FLAG_FIXED         (1 << 0)    /* Fixed 
>> (on-card) PCI memory */
>>   #define TTM_MEMTYPE_FLAG_MAPPABLE      (1 << 1)    /* Memory 
>> mappable */
>>   #define TTM_MEMTYPE_FLAG_CMA           (1 << 3)    /* Can't map 
>> aperture */
>> @@ -610,117 +509,6 @@ ttm_flag_masked(uint32_t *old, uint32_t new, 
>> uint32_t mask)
>>       return *old;
>>   }
>>   -/**
>> - * ttm_tt_create
>> - *
>> - * @bo: pointer to a struct ttm_buffer_object
>> - * @zero_alloc: true if allocated pages needs to be zeroed
>> - *
>> - * Make sure we have a TTM structure allocated for the given BO.
>> - * No pages are actually allocated.
>> - */
>> -int ttm_tt_create(struct ttm_buffer_object *bo, bool zero_alloc);
>> -
>> -/**
>> - * ttm_tt_init
>> - *
>> - * @ttm: The struct ttm_tt.
>> - * @bdev: pointer to a struct ttm_bo_device:
>> - * @size: Size of the data needed backing.
>> - * @page_flags: Page flags as identified by TTM_PAGE_FLAG_XX flags.
>> - *
>> - * Create a struct ttm_tt to back data with system memory pages.
>> - * No pages are actually allocated.
>> - * Returns:
>> - * NULL: Out of memory.
>> - */
>> -int ttm_tt_init(struct ttm_tt *ttm, struct ttm_bo_device *bdev,
>> -        unsigned long size, uint32_t page_flags);
>> -int ttm_dma_tt_init(struct ttm_dma_tt *ttm_dma, struct ttm_bo_device 
>> *bdev,
>> -            unsigned long size, uint32_t page_flags);
>> -
>> -/**
>> - * ttm_tt_fini
>> - *
>> - * @ttm: the ttm_tt structure.
>> - *
>> - * Free memory of ttm_tt structure
>> - */
>> -void ttm_tt_fini(struct ttm_tt *ttm);
>> -void ttm_dma_tt_fini(struct ttm_dma_tt *ttm_dma);
>> -
>> -/**
>> - * ttm_ttm_bind:
>> - *
>> - * @ttm: The struct ttm_tt containing backing pages.
>> - * @bo_mem: The struct ttm_mem_reg identifying the binding location.
>> - *
>> - * Bind the pages of @ttm to an aperture location identified by @bo_mem
>> - */
>> -int ttm_tt_bind(struct ttm_tt *ttm, struct ttm_mem_reg *bo_mem,
>> -        struct ttm_operation_ctx *ctx);
>> -
>> -/**
>> - * ttm_ttm_destroy:
>> - *
>> - * @ttm: The struct ttm_tt.
>> - *
>> - * Unbind, unpopulate and destroy common struct ttm_tt.
>> - */
>> -void ttm_tt_destroy(struct ttm_tt *ttm);
>> -
>> -/**
>> - * ttm_ttm_unbind:
>> - *
>> - * @ttm: The struct ttm_tt.
>> - *
>> - * Unbind a struct ttm_tt.
>> - */
>> -void ttm_tt_unbind(struct ttm_tt *ttm);
>> -
>> -/**
>> - * ttm_tt_swapin:
>> - *
>> - * @ttm: The struct ttm_tt.
>> - *
>> - * Swap in a previously swap out ttm_tt.
>> - */
>> -int ttm_tt_swapin(struct ttm_tt *ttm);
>> -
>> -/**
>> - * ttm_tt_set_placement_caching:
>> - *
>> - * @ttm A struct ttm_tt the backing pages of which will change 
>> caching policy.
>> - * @placement: Flag indicating the desired caching policy.
>> - *
>> - * This function will change caching policy of any default kernel 
>> mappings of
>> - * the pages backing @ttm. If changing from cached to uncached or
>> - * write-combined,
>> - * all CPU caches will first be flushed to make sure the data of the 
>> pages
>> - * hit RAM. This function may be very costly as it involves global TLB
>> - * and cache flushes and potential page splitting / combining.
>> - */
>> -int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t 
>> placement);
>> -int ttm_tt_swapout(struct ttm_tt *ttm, struct file 
>> *persistent_swap_storage);
>> -
>> -/**
>> - * ttm_tt_populate - allocate pages for a ttm
>> - *
>> - * @ttm: Pointer to the ttm_tt structure
>> - *
>> - * Calls the driver method to allocate pages for a ttm
>> - */
>> -int ttm_tt_populate(struct ttm_tt *ttm, struct ttm_operation_ctx *ctx);
>> -
>> -/**
>> - * ttm_tt_unpopulate - free pages from a ttm
>> - *
>> - * @ttm: Pointer to the ttm_tt structure
>> - *
>> - * Calls the driver method to free all pages from a ttm
>> - */
>> -void ttm_tt_unpopulate(struct ttm_tt *ttm);
>> -
>>   /*
>>    * ttm_bo.c
>>    */
>> @@ -1074,27 +862,4 @@ pgprot_t ttm_io_prot(uint32_t caching_flags, 
>> pgprot_t tmp);
>>     extern const struct ttm_mem_type_manager_func ttm_bo_manager_func;
>>   -#if IS_ENABLED(CONFIG_AGP)
>> -#include <linux/agp_backend.h>
>> -
>> -/**
>> - * ttm_agp_tt_create
>> - *
>> - * @bdev: Pointer to a struct ttm_bo_device.
>> - * @bridge: The agp bridge this device is sitting on.
>> - * @size: Size of the data needed backing.
>> - * @page_flags: Page flags as identified by TTM_PAGE_FLAG_XX flags.
>> - *
>> - *
>> - * Create a TTM backend that uses the indicated AGP bridge as an 
>> aperture
>> - * for TT memory. This function uses the linux agpgart interface to
>> - * bind and unbind memory backing a ttm_tt.
>> - */
>> -struct ttm_tt *ttm_agp_tt_create(struct ttm_bo_device *bdev,
>> -                 struct agp_bridge_data *bridge,
>> -                 unsigned long size, uint32_t page_flags);
>> -int ttm_agp_tt_populate(struct ttm_tt *ttm, struct ttm_operation_ctx 
>> *ctx);
>> -void ttm_agp_tt_unpopulate(struct ttm_tt *ttm);
>> -#endif
>> -
>>   #endif
>> diff --git a/include/drm/ttm/ttm_tt.h b/include/drm/ttm/ttm_tt.h
>> new file mode 100644
>> index 000000000000..9c78556b488e
>> --- /dev/null
>> +++ b/include/drm/ttm/ttm_tt.h
>> @@ -0,0 +1,272 @@
>> +/************************************************************************** 
>>
>> + *
>> + * Copyright (c) 2006-2009 Vmware, Inc., Palo Alto, CA., USA
>> + * 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 above copyright notice and this permission notice (including the
>> + * next paragraph) shall be included in all copies or substantial 
>> portions
>> + * of the Software.
>> + *
>> + * 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.
>> + *
>> + 
>> **************************************************************************/
>> +#ifndef _TTM_TT_H_
>> +#define _TTM_TT_H_
>> +
>> +#include <linux/types.h>
>> +
>> +struct ttm_tt;
>> +struct ttm_mem_reg;
>> +struct ttm_buffer_object;
>> +struct ttm_operation_ctx;
>> +
>> +#define TTM_PAGE_FLAG_WRITE           (1 << 3)
>> +#define TTM_PAGE_FLAG_SWAPPED         (1 << 4)
>> +#define TTM_PAGE_FLAG_PERSISTENT_SWAP (1 << 5)
>> +#define TTM_PAGE_FLAG_ZERO_ALLOC      (1 << 6)
>> +#define TTM_PAGE_FLAG_DMA32           (1 << 7)
>> +#define TTM_PAGE_FLAG_SG              (1 << 8)
>> +#define TTM_PAGE_FLAG_NO_RETRY          (1 << 9)
>> +
>> +enum ttm_caching_state {
>> +    tt_uncached,
>> +    tt_wc,
>> +    tt_cached
>> +};
>> +
>> +struct ttm_backend_func {
>> +    /**
>> +     * struct ttm_backend_func member bind
>> +     *
>> +     * @ttm: Pointer to a struct ttm_tt.
>> +     * @bo_mem: Pointer to a struct ttm_mem_reg describing the
>> +     * memory type and location for binding.
>> +     *
>> +     * Bind the backend pages into the aperture in the location
>> +     * indicated by @bo_mem. This function should be able to handle
>> +     * differences between aperture and system page sizes.
>> +     */
>> +    int (*bind) (struct ttm_tt *ttm, struct ttm_mem_reg *bo_mem);
>> +
>> +    /**
>> +     * struct ttm_backend_func member unbind
>> +     *
>> +     * @ttm: Pointer to a struct ttm_tt.
>> +     *
>> +     * Unbind previously bound backend pages. This function should be
>> +     * able to handle differences between aperture and system page 
>> sizes.
>> +     */
>> +    int (*unbind) (struct ttm_tt *ttm);
>> +
>> +    /**
>> +     * struct ttm_backend_func member destroy
>> +     *
>> +     * @ttm: Pointer to a struct ttm_tt.
>> +     *
>> +     * Destroy the backend. This will be call back from 
>> ttm_tt_destroy so
>> +     * don't call ttm_tt_destroy from the callback or infinite loop.
>> +     */
>> +    void (*destroy) (struct ttm_tt *ttm);
>> +};
>> +
>> +/**
>> + * struct ttm_tt
>> + *
>> + * @bdev: Pointer to a struct ttm_bo_device.
>> + * @func: Pointer to a struct ttm_backend_func that describes
>> + * the backend methods.
>> + * pointer.
>> + * @pages: Array of pages backing the data.
>> + * @num_pages: Number of pages in the page array.
>> + * @bdev: Pointer to the current struct ttm_bo_device.
>> + * @be: Pointer to the ttm backend.
>> + * @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.
>> + *
>> + * This is a structure holding the pages, caching- and aperture binding
>> + * status for a buffer object that isn't backed by fixed (VRAM / AGP)
>> + * memory.
>> + */
>> +struct ttm_tt {
>> +    struct ttm_bo_device *bdev;
>> +    struct ttm_backend_func *func;
>> +    struct page **pages;
>> +    uint32_t page_flags;
>> +    unsigned long num_pages;
>> +    struct sg_table *sg; /* for SG objects via dma-buf */
>> +    struct file *swap_storage;
>> +    enum ttm_caching_state caching_state;
>> +    enum {
>> +        tt_bound,
>> +        tt_unbound,
>> +        tt_unpopulated,
>> +    } state;
>> +};
>> +
>> +/**
>> + * struct ttm_dma_tt
>> + *
>> + * @ttm: Base ttm_tt struct.
>> + * @dma_address: The DMA (bus) addresses of the pages
>> + * @pages_list: used by some page allocation backend
>> + *
>> + * This is a structure holding the pages, caching- and aperture binding
>> + * status for a buffer object that isn't backed by fixed (VRAM / AGP)
>> + * memory.
>> + */
>> +struct ttm_dma_tt {
>> +    struct ttm_tt ttm;
>> +    dma_addr_t *dma_address;
>> +    struct list_head pages_list;
>> +};
>> +
>> +/**
>> + * ttm_tt_create
>> + *
>> + * @bo: pointer to a struct ttm_buffer_object
>> + * @zero_alloc: true if allocated pages needs to be zeroed
>> + *
>> + * Make sure we have a TTM structure allocated for the given BO.
>> + * No pages are actually allocated.
>> + */
>> +int ttm_tt_create(struct ttm_buffer_object *bo, bool zero_alloc);
>> +
>> +/**
>> + * ttm_tt_init
>> + *
>> + * @ttm: The struct ttm_tt.
>> + * @bdev: pointer to a struct ttm_bo_device:
>> + * @size: Size of the data needed backing.
>> + * @page_flags: Page flags as identified by TTM_PAGE_FLAG_XX flags.
>> + *
>> + * Create a struct ttm_tt to back data with system memory pages.
>> + * No pages are actually allocated.
>> + * Returns:
>> + * NULL: Out of memory.
>> + */
>> +int ttm_tt_init(struct ttm_tt *ttm, struct ttm_bo_device *bdev,
>> +        unsigned long size, uint32_t page_flags);
>> +int ttm_dma_tt_init(struct ttm_dma_tt *ttm_dma, struct ttm_bo_device 
>> *bdev,
>> +            unsigned long size, uint32_t page_flags);
>> +
>> +/**
>> + * ttm_tt_fini
>> + *
>> + * @ttm: the ttm_tt structure.
>> + *
>> + * Free memory of ttm_tt structure
>> + */
>> +void ttm_tt_fini(struct ttm_tt *ttm);
>> +void ttm_dma_tt_fini(struct ttm_dma_tt *ttm_dma);
>> +
>> +/**
>> + * ttm_ttm_bind:
>> + *
>> + * @ttm: The struct ttm_tt containing backing pages.
>> + * @bo_mem: The struct ttm_mem_reg identifying the binding location.
>> + *
>> + * Bind the pages of @ttm to an aperture location identified by @bo_mem
>> + */
>> +int ttm_tt_bind(struct ttm_tt *ttm, struct ttm_mem_reg *bo_mem,
>> +        struct ttm_operation_ctx *ctx);
>> +
>> +/**
>> + * ttm_ttm_destroy:
>> + *
>> + * @ttm: The struct ttm_tt.
>> + *
>> + * Unbind, unpopulate and destroy common struct ttm_tt.
>> + */
>> +void ttm_tt_destroy(struct ttm_tt *ttm);
>> +
>> +/**
>> + * ttm_ttm_unbind:
>> + *
>> + * @ttm: The struct ttm_tt.
>> + *
>> + * Unbind a struct ttm_tt.
>> + */
>> +void ttm_tt_unbind(struct ttm_tt *ttm);
>> +
>> +/**
>> + * ttm_tt_swapin:
>> + *
>> + * @ttm: The struct ttm_tt.
>> + *
>> + * Swap in a previously swap out ttm_tt.
>> + */
>> +int ttm_tt_swapin(struct ttm_tt *ttm);
>> +
>> +/**
>> + * ttm_tt_set_placement_caching:
>> + *
>> + * @ttm A struct ttm_tt the backing pages of which will change 
>> caching policy.
>> + * @placement: Flag indicating the desired caching policy.
>> + *
>> + * This function will change caching policy of any default kernel 
>> mappings of
>> + * the pages backing @ttm. If changing from cached to uncached or
>> + * write-combined,
>> + * all CPU caches will first be flushed to make sure the data of the 
>> pages
>> + * hit RAM. This function may be very costly as it involves global TLB
>> + * and cache flushes and potential page splitting / combining.
>> + */
>> +int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t 
>> placement);
>> +int ttm_tt_swapout(struct ttm_tt *ttm, struct file 
>> *persistent_swap_storage);
>> +
>> +/**
>> + * ttm_tt_populate - allocate pages for a ttm
>> + *
>> + * @ttm: Pointer to the ttm_tt structure
>> + *
>> + * Calls the driver method to allocate pages for a ttm
>> + */
>> +int ttm_tt_populate(struct ttm_tt *ttm, struct ttm_operation_ctx *ctx);
>> +
>> +/**
>> + * ttm_tt_unpopulate - free pages from a ttm
>> + *
>> + * @ttm: Pointer to the ttm_tt structure
>> + *
>> + * Calls the driver method to free all pages from a ttm
>> + */
>> +void ttm_tt_unpopulate(struct ttm_tt *ttm);
>> +
>> +#if IS_ENABLED(CONFIG_AGP)
>> +#include <linux/agp_backend.h>
>> +
>> +/**
>> + * ttm_agp_tt_create
>> + *
>> + * @bdev: Pointer to a struct ttm_bo_device.
>> + * @bridge: The agp bridge this device is sitting on.
>> + * @size: Size of the data needed backing.
>> + * @page_flags: Page flags as identified by TTM_PAGE_FLAG_XX flags.
>> + *
>> + *
>> + * Create a TTM backend that uses the indicated AGP bridge as an 
>> aperture
>> + * for TT memory. This function uses the linux agpgart interface to
>> + * bind and unbind memory backing a ttm_tt.
>> + */
>> +struct ttm_tt *ttm_agp_tt_create(struct ttm_bo_device *bdev,
>> +                 struct agp_bridge_data *bridge,
>> +                 unsigned long size, uint32_t page_flags);
>> +int ttm_agp_tt_populate(struct ttm_tt *ttm, struct ttm_operation_ctx 
>> *ctx);
>> +void ttm_agp_tt_unpopulate(struct ttm_tt *ttm);
>> +#endif
>> +
>> +#endif


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/5] drm/ttm: add ttm_sg_tt_init

[Christian König]

This allows drivers to only allocate dma addresses, but not a page
array.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/ttm/ttm_tt.c | 54 ++++++++++++++++++++++++++++++++++++--------
 include/drm/ttm/ttm_tt.h     |  2 ++
 2 files changed, 47 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index 8e0b525cda00..971133106ec2 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -108,6 +108,16 @@ static int ttm_dma_tt_alloc_page_directory(struct ttm_dma_tt *ttm)
 	return 0;
 }
 
+static int ttm_sg_tt_alloc_page_directory(struct ttm_dma_tt *ttm)
+{
+	ttm->dma_address = kvmalloc_array(ttm->ttm.num_pages,
+					  sizeof(*ttm->dma_address),
+					  GFP_KERNEL | __GFP_ZERO);
+	if (!ttm->dma_address)
+		return -ENOMEM;
+	return 0;
+}
+
 #ifdef CONFIG_X86
 static inline int ttm_tt_set_page_caching(struct page *p,
 					  enum ttm_caching_state c_old,
@@ -227,8 +237,8 @@ void ttm_tt_destroy(struct ttm_tt *ttm)
 	ttm->func->destroy(ttm);
 }
 
-int ttm_tt_init(struct ttm_tt *ttm, struct ttm_bo_device *bdev,
-		unsigned long size, uint32_t page_flags)
+void ttm_tt_init_fields(struct ttm_tt *ttm, struct ttm_bo_device *bdev,
+			unsigned long size, uint32_t page_flags)
 {
 	ttm->bdev = bdev;
 	ttm->num_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
@@ -236,6 +246,12 @@ int ttm_tt_init(struct ttm_tt *ttm, struct ttm_bo_device *bdev,
 	ttm->page_flags = page_flags;
 	ttm->state = tt_unpopulated;
 	ttm->swap_storage = NULL;
+}
+
+int ttm_tt_init(struct ttm_tt *ttm, struct ttm_bo_device *bdev,
+		unsigned long size, uint32_t page_flags)
+{
+	ttm_tt_init_fields(ttm, bdev, size, page_flags);
 
 	if (ttm_tt_alloc_page_directory(ttm)) {
 		ttm_tt_destroy(ttm);
@@ -258,12 +274,7 @@ int ttm_dma_tt_init(struct ttm_dma_tt *ttm_dma, struct ttm_bo_device *bdev,
 {
 	struct ttm_tt *ttm = &ttm_dma->ttm;
 
-	ttm->bdev = bdev;
-	ttm->num_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
-	ttm->caching_state = tt_cached;
-	ttm->page_flags = page_flags;
-	ttm->state = tt_unpopulated;
-	ttm->swap_storage = NULL;
+	ttm_tt_init_fields(ttm, bdev, size, page_flags);
 
 	INIT_LIST_HEAD(&ttm_dma->pages_list);
 	if (ttm_dma_tt_alloc_page_directory(ttm_dma)) {
@@ -275,11 +286,36 @@ int ttm_dma_tt_init(struct ttm_dma_tt *ttm_dma, struct ttm_bo_device *bdev,
 }
 EXPORT_SYMBOL(ttm_dma_tt_init);
 
+int ttm_sg_tt_init(struct ttm_dma_tt *ttm_dma, struct ttm_bo_device *bdev,
+		   unsigned long size, uint32_t page_flags)
+{
+	struct ttm_tt *ttm = &ttm_dma->ttm;
+	int ret;
+
+	ttm_tt_init_fields(ttm, bdev, size, page_flags);
+
+	INIT_LIST_HEAD(&ttm_dma->pages_list);
+	if (page_flags & TTM_PAGE_FLAG_SG)
+		ret = ttm_sg_tt_alloc_page_directory(ttm_dma);
+	else
+		ret = ttm_dma_tt_alloc_page_directory(ttm_dma);
+	if (ret) {
+		ttm_tt_destroy(ttm);
+		pr_err("Failed allocating page table\n");
+		return -ENOMEM;
+	}
+	return 0;
+}
+EXPORT_SYMBOL(ttm_sg_tt_init);
+
 void ttm_dma_tt_fini(struct ttm_dma_tt *ttm_dma)
 {
 	struct ttm_tt *ttm = &ttm_dma->ttm;
 
-	kvfree(ttm->pages);
+	if (ttm->pages)
+		kvfree(ttm->pages);
+	else
+		kvfree(ttm_dma->dma_address);
 	ttm->pages = NULL;
 	ttm_dma->dma_address = NULL;
 }
diff --git a/include/drm/ttm/ttm_tt.h b/include/drm/ttm/ttm_tt.h
index 9c78556b488e..1cf316a4257c 100644
--- a/include/drm/ttm/ttm_tt.h
+++ b/include/drm/ttm/ttm_tt.h
@@ -163,6 +163,8 @@ int ttm_tt_init(struct ttm_tt *ttm, struct ttm_bo_device *bdev,
 		unsigned long size, uint32_t page_flags);
 int ttm_dma_tt_init(struct ttm_dma_tt *ttm_dma, struct ttm_bo_device *bdev,
 		    unsigned long size, uint32_t page_flags);
+int ttm_sg_tt_init(struct ttm_dma_tt *ttm_dma, struct ttm_bo_device *bdev,
+		   unsigned long size, uint32_t page_flags);
 
 /**
  * ttm_tt_fini
-- 
2.14.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 4/5] drm/ttm: add ttm_sg_tt_init

[Christian König]

Hi guys,

at least on amdgpu and radeon the page array allocated by 
ttm_dma_tt_init is completely unused in the case of DMA-buf sharing. So 
I'm trying to get rid of that by only allocating the DMA address array.

Now the only other user of DMA-buf together with ttm_dma_tt_init is 
Nouveau. So my question is are you guys using the page array anywhere in 
your kernel driver in case of a DMA-buf sharing?

If no then I could just make this the default behavior for all drivers 
and save quite a bit of memory for everybody.

Thanks,
Christian.

Am 27.02.2018 um 12:49 schrieb Christian König:
> This allows drivers to only allocate dma addresses, but not a page
> array.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/ttm/ttm_tt.c | 54 ++++++++++++++++++++++++++++++++++++--------
>   include/drm/ttm/ttm_tt.h     |  2 ++
>   2 files changed, 47 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
> index 8e0b525cda00..971133106ec2 100644
> --- a/drivers/gpu/drm/ttm/ttm_tt.c
> +++ b/drivers/gpu/drm/ttm/ttm_tt.c
> @@ -108,6 +108,16 @@ static int ttm_dma_tt_alloc_page_directory(struct ttm_dma_tt *ttm)
>   	return 0;
>   }
>   
> +static int ttm_sg_tt_alloc_page_directory(struct ttm_dma_tt *ttm)
> +{
> +	ttm->dma_address = kvmalloc_array(ttm->ttm.num_pages,
> +					  sizeof(*ttm->dma_address),
> +					  GFP_KERNEL | __GFP_ZERO);
> +	if (!ttm->dma_address)
> +		return -ENOMEM;
> +	return 0;
> +}
> +
>   #ifdef CONFIG_X86
>   static inline int ttm_tt_set_page_caching(struct page *p,
>   					  enum ttm_caching_state c_old,
> @@ -227,8 +237,8 @@ void ttm_tt_destroy(struct ttm_tt *ttm)
>   	ttm->func->destroy(ttm);
>   }
>   
> -int ttm_tt_init(struct ttm_tt *ttm, struct ttm_bo_device *bdev,
> -		unsigned long size, uint32_t page_flags)
> +void ttm_tt_init_fields(struct ttm_tt *ttm, struct ttm_bo_device *bdev,
> +			unsigned long size, uint32_t page_flags)
>   {
>   	ttm->bdev = bdev;
>   	ttm->num_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
> @@ -236,6 +246,12 @@ int ttm_tt_init(struct ttm_tt *ttm, struct ttm_bo_device *bdev,
>   	ttm->page_flags = page_flags;
>   	ttm->state = tt_unpopulated;
>   	ttm->swap_storage = NULL;
> +}
> +
> +int ttm_tt_init(struct ttm_tt *ttm, struct ttm_bo_device *bdev,
> +		unsigned long size, uint32_t page_flags)
> +{
> +	ttm_tt_init_fields(ttm, bdev, size, page_flags);
>   
>   	if (ttm_tt_alloc_page_directory(ttm)) {
>   		ttm_tt_destroy(ttm);
> @@ -258,12 +274,7 @@ int ttm_dma_tt_init(struct ttm_dma_tt *ttm_dma, struct ttm_bo_device *bdev,
>   {
>   	struct ttm_tt *ttm = &ttm_dma->ttm;
>   
> -	ttm->bdev = bdev;
> -	ttm->num_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
> -	ttm->caching_state = tt_cached;
> -	ttm->page_flags = page_flags;
> -	ttm->state = tt_unpopulated;
> -	ttm->swap_storage = NULL;
> +	ttm_tt_init_fields(ttm, bdev, size, page_flags);
>   
>   	INIT_LIST_HEAD(&ttm_dma->pages_list);
>   	if (ttm_dma_tt_alloc_page_directory(ttm_dma)) {
> @@ -275,11 +286,36 @@ int ttm_dma_tt_init(struct ttm_dma_tt *ttm_dma, struct ttm_bo_device *bdev,
>   }
>   EXPORT_SYMBOL(ttm_dma_tt_init);
>   
> +int ttm_sg_tt_init(struct ttm_dma_tt *ttm_dma, struct ttm_bo_device *bdev,
> +		   unsigned long size, uint32_t page_flags)
> +{
> +	struct ttm_tt *ttm = &ttm_dma->ttm;
> +	int ret;
> +
> +	ttm_tt_init_fields(ttm, bdev, size, page_flags);
> +
> +	INIT_LIST_HEAD(&ttm_dma->pages_list);
> +	if (page_flags & TTM_PAGE_FLAG_SG)
> +		ret = ttm_sg_tt_alloc_page_directory(ttm_dma);
> +	else
> +		ret = ttm_dma_tt_alloc_page_directory(ttm_dma);
> +	if (ret) {
> +		ttm_tt_destroy(ttm);
> +		pr_err("Failed allocating page table\n");
> +		return -ENOMEM;
> +	}
> +	return 0;
> +}
> +EXPORT_SYMBOL(ttm_sg_tt_init);
> +
>   void ttm_dma_tt_fini(struct ttm_dma_tt *ttm_dma)
>   {
>   	struct ttm_tt *ttm = &ttm_dma->ttm;
>   
> -	kvfree(ttm->pages);
> +	if (ttm->pages)
> +		kvfree(ttm->pages);
> +	else
> +		kvfree(ttm_dma->dma_address);
>   	ttm->pages = NULL;
>   	ttm_dma->dma_address = NULL;
>   }
> diff --git a/include/drm/ttm/ttm_tt.h b/include/drm/ttm/ttm_tt.h
> index 9c78556b488e..1cf316a4257c 100644
> --- a/include/drm/ttm/ttm_tt.h
> +++ b/include/drm/ttm/ttm_tt.h
> @@ -163,6 +163,8 @@ int ttm_tt_init(struct ttm_tt *ttm, struct ttm_bo_device *bdev,
>   		unsigned long size, uint32_t page_flags);
>   int ttm_dma_tt_init(struct ttm_dma_tt *ttm_dma, struct ttm_bo_device *bdev,
>   		    unsigned long size, uint32_t page_flags);
> +int ttm_sg_tt_init(struct ttm_dma_tt *ttm_dma, struct ttm_bo_device *bdev,
> +		   unsigned long size, uint32_t page_flags);
>   
>   /**
>    * ttm_tt_fini

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 4/5] drm/ttm: add ttm_sg_tt_init

[Christian König]

Ping?

Am 27.02.2018 um 13:07 schrieb Christian König:
> Hi guys,
>
> at least on amdgpu and radeon the page array allocated by 
> ttm_dma_tt_init is completely unused in the case of DMA-buf sharing. 
> So I'm trying to get rid of that by only allocating the DMA address 
> array.
>
> Now the only other user of DMA-buf together with ttm_dma_tt_init is 
> Nouveau. So my question is are you guys using the page array anywhere 
> in your kernel driver in case of a DMA-buf sharing?
>
> If no then I could just make this the default behavior for all drivers 
> and save quite a bit of memory for everybody.
>
> Thanks,
> Christian.
>
> Am 27.02.2018 um 12:49 schrieb Christian König:
>> This allows drivers to only allocate dma addresses, but not a page
>> array.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>   drivers/gpu/drm/ttm/ttm_tt.c | 54 
>> ++++++++++++++++++++++++++++++++++++--------
>>   include/drm/ttm/ttm_tt.h     |  2 ++
>>   2 files changed, 47 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
>> index 8e0b525cda00..971133106ec2 100644
>> --- a/drivers/gpu/drm/ttm/ttm_tt.c
>> +++ b/drivers/gpu/drm/ttm/ttm_tt.c
>> @@ -108,6 +108,16 @@ static int 
>> ttm_dma_tt_alloc_page_directory(struct ttm_dma_tt *ttm)
>>       return 0;
>>   }
>>   +static int ttm_sg_tt_alloc_page_directory(struct ttm_dma_tt *ttm)
>> +{
>> +    ttm->dma_address = kvmalloc_array(ttm->ttm.num_pages,
>> +                      sizeof(*ttm->dma_address),
>> +                      GFP_KERNEL | __GFP_ZERO);
>> +    if (!ttm->dma_address)
>> +        return -ENOMEM;
>> +    return 0;
>> +}
>> +
>>   #ifdef CONFIG_X86
>>   static inline int ttm_tt_set_page_caching(struct page *p,
>>                         enum ttm_caching_state c_old,
>> @@ -227,8 +237,8 @@ void ttm_tt_destroy(struct ttm_tt *ttm)
>>       ttm->func->destroy(ttm);
>>   }
>>   -int ttm_tt_init(struct ttm_tt *ttm, struct ttm_bo_device *bdev,
>> -        unsigned long size, uint32_t page_flags)
>> +void ttm_tt_init_fields(struct ttm_tt *ttm, struct ttm_bo_device *bdev,
>> +            unsigned long size, uint32_t page_flags)
>>   {
>>       ttm->bdev = bdev;
>>       ttm->num_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
>> @@ -236,6 +246,12 @@ int ttm_tt_init(struct ttm_tt *ttm, struct 
>> ttm_bo_device *bdev,
>>       ttm->page_flags = page_flags;
>>       ttm->state = tt_unpopulated;
>>       ttm->swap_storage = NULL;
>> +}
>> +
>> +int ttm_tt_init(struct ttm_tt *ttm, struct ttm_bo_device *bdev,
>> +        unsigned long size, uint32_t page_flags)
>> +{
>> +    ttm_tt_init_fields(ttm, bdev, size, page_flags);
>>         if (ttm_tt_alloc_page_directory(ttm)) {
>>           ttm_tt_destroy(ttm);
>> @@ -258,12 +274,7 @@ int ttm_dma_tt_init(struct ttm_dma_tt *ttm_dma, 
>> struct ttm_bo_device *bdev,
>>   {
>>       struct ttm_tt *ttm = &ttm_dma->ttm;
>>   -    ttm->bdev = bdev;
>> -    ttm->num_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
>> -    ttm->caching_state = tt_cached;
>> -    ttm->page_flags = page_flags;
>> -    ttm->state = tt_unpopulated;
>> -    ttm->swap_storage = NULL;
>> +    ttm_tt_init_fields(ttm, bdev, size, page_flags);
>>         INIT_LIST_HEAD(&ttm_dma->pages_list);
>>       if (ttm_dma_tt_alloc_page_directory(ttm_dma)) {
>> @@ -275,11 +286,36 @@ int ttm_dma_tt_init(struct ttm_dma_tt *ttm_dma, 
>> struct ttm_bo_device *bdev,
>>   }
>>   EXPORT_SYMBOL(ttm_dma_tt_init);
>>   +int ttm_sg_tt_init(struct ttm_dma_tt *ttm_dma, struct 
>> ttm_bo_device *bdev,
>> +           unsigned long size, uint32_t page_flags)
>> +{
>> +    struct ttm_tt *ttm = &ttm_dma->ttm;
>> +    int ret;
>> +
>> +    ttm_tt_init_fields(ttm, bdev, size, page_flags);
>> +
>> +    INIT_LIST_HEAD(&ttm_dma->pages_list);
>> +    if (page_flags & TTM_PAGE_FLAG_SG)
>> +        ret = ttm_sg_tt_alloc_page_directory(ttm_dma);
>> +    else
>> +        ret = ttm_dma_tt_alloc_page_directory(ttm_dma);
>> +    if (ret) {
>> +        ttm_tt_destroy(ttm);
>> +        pr_err("Failed allocating page table\n");
>> +        return -ENOMEM;
>> +    }
>> +    return 0;
>> +}
>> +EXPORT_SYMBOL(ttm_sg_tt_init);
>> +
>>   void ttm_dma_tt_fini(struct ttm_dma_tt *ttm_dma)
>>   {
>>       struct ttm_tt *ttm = &ttm_dma->ttm;
>>   -    kvfree(ttm->pages);
>> +    if (ttm->pages)
>> +        kvfree(ttm->pages);
>> +    else
>> +        kvfree(ttm_dma->dma_address);
>>       ttm->pages = NULL;
>>       ttm_dma->dma_address = NULL;
>>   }
>> diff --git a/include/drm/ttm/ttm_tt.h b/include/drm/ttm/ttm_tt.h
>> index 9c78556b488e..1cf316a4257c 100644
>> --- a/include/drm/ttm/ttm_tt.h
>> +++ b/include/drm/ttm/ttm_tt.h
>> @@ -163,6 +163,8 @@ int ttm_tt_init(struct ttm_tt *ttm, struct 
>> ttm_bo_device *bdev,
>>           unsigned long size, uint32_t page_flags);
>>   int ttm_dma_tt_init(struct ttm_dma_tt *ttm_dma, struct 
>> ttm_bo_device *bdev,
>>               unsigned long size, uint32_t page_flags);
>> +int ttm_sg_tt_init(struct ttm_dma_tt *ttm_dma, struct ttm_bo_device 
>> *bdev,
>> +           unsigned long size, uint32_t page_flags);
>>     /**
>>    * ttm_tt_fini
>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 4/5] drm/ttm: add ttm_sg_tt_init

[Ben Skeggs]

On Mon, Mar 5, 2018 at 10:06 PM, Christian König
<christian.koenig@amd.com> wrote:
> Ping?
On a quick look, it looks like both are used sometimes.  I believe
this had something to do with the addition of Tegra support, but I'll
need some time to look into the details of why/how these things are
used again.

Ben.

>
>
> Am 27.02.2018 um 13:07 schrieb Christian König:
>>
>> Hi guys,
>>
>> at least on amdgpu and radeon the page array allocated by ttm_dma_tt_init
>> is completely unused in the case of DMA-buf sharing. So I'm trying to get
>> rid of that by only allocating the DMA address array.
>>
>> Now the only other user of DMA-buf together with ttm_dma_tt_init is
>> Nouveau. So my question is are you guys using the page array anywhere in
>> your kernel driver in case of a DMA-buf sharing?
>>
>> If no then I could just make this the default behavior for all drivers and
>> save quite a bit of memory for everybody.
>>
>> Thanks,
>> Christian.
>>
>> Am 27.02.2018 um 12:49 schrieb Christian König:
>>>
>>> This allows drivers to only allocate dma addresses, but not a page
>>> array.
>>>
>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>> ---
>>>   drivers/gpu/drm/ttm/ttm_tt.c | 54
>>> ++++++++++++++++++++++++++++++++++++--------
>>>   include/drm/ttm/ttm_tt.h     |  2 ++
>>>   2 files changed, 47 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
>>> index 8e0b525cda00..971133106ec2 100644
>>> --- a/drivers/gpu/drm/ttm/ttm_tt.c
>>> +++ b/drivers/gpu/drm/ttm/ttm_tt.c
>>> @@ -108,6 +108,16 @@ static int ttm_dma_tt_alloc_page_directory(struct
>>> ttm_dma_tt *ttm)
>>>       return 0;
>>>   }
>>>   +static int ttm_sg_tt_alloc_page_directory(struct ttm_dma_tt *ttm)
>>> +{
>>> +    ttm->dma_address = kvmalloc_array(ttm->ttm.num_pages,
>>> +                      sizeof(*ttm->dma_address),
>>> +                      GFP_KERNEL | __GFP_ZERO);
>>> +    if (!ttm->dma_address)
>>> +        return -ENOMEM;
>>> +    return 0;
>>> +}
>>> +
>>>   #ifdef CONFIG_X86
>>>   static inline int ttm_tt_set_page_caching(struct page *p,
>>>                         enum ttm_caching_state c_old,
>>> @@ -227,8 +237,8 @@ void ttm_tt_destroy(struct ttm_tt *ttm)
>>>       ttm->func->destroy(ttm);
>>>   }
>>>   -int ttm_tt_init(struct ttm_tt *ttm, struct ttm_bo_device *bdev,
>>> -        unsigned long size, uint32_t page_flags)
>>> +void ttm_tt_init_fields(struct ttm_tt *ttm, struct ttm_bo_device *bdev,
>>> +            unsigned long size, uint32_t page_flags)
>>>   {
>>>       ttm->bdev = bdev;
>>>       ttm->num_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
>>> @@ -236,6 +246,12 @@ int ttm_tt_init(struct ttm_tt *ttm, struct
>>> ttm_bo_device *bdev,
>>>       ttm->page_flags = page_flags;
>>>       ttm->state = tt_unpopulated;
>>>       ttm->swap_storage = NULL;
>>> +}
>>> +
>>> +int ttm_tt_init(struct ttm_tt *ttm, struct ttm_bo_device *bdev,
>>> +        unsigned long size, uint32_t page_flags)
>>> +{
>>> +    ttm_tt_init_fields(ttm, bdev, size, page_flags);
>>>         if (ttm_tt_alloc_page_directory(ttm)) {
>>>           ttm_tt_destroy(ttm);
>>> @@ -258,12 +274,7 @@ int ttm_dma_tt_init(struct ttm_dma_tt *ttm_dma,
>>> struct ttm_bo_device *bdev,
>>>   {
>>>       struct ttm_tt *ttm = &ttm_dma->ttm;
>>>   -    ttm->bdev = bdev;
>>> -    ttm->num_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
>>> -    ttm->caching_state = tt_cached;
>>> -    ttm->page_flags = page_flags;
>>> -    ttm->state = tt_unpopulated;
>>> -    ttm->swap_storage = NULL;
>>> +    ttm_tt_init_fields(ttm, bdev, size, page_flags);
>>>         INIT_LIST_HEAD(&ttm_dma->pages_list);
>>>       if (ttm_dma_tt_alloc_page_directory(ttm_dma)) {
>>> @@ -275,11 +286,36 @@ int ttm_dma_tt_init(struct ttm_dma_tt *ttm_dma,
>>> struct ttm_bo_device *bdev,
>>>   }
>>>   EXPORT_SYMBOL(ttm_dma_tt_init);
>>>   +int ttm_sg_tt_init(struct ttm_dma_tt *ttm_dma, struct ttm_bo_device
>>> *bdev,
>>> +           unsigned long size, uint32_t page_flags)
>>> +{
>>> +    struct ttm_tt *ttm = &ttm_dma->ttm;
>>> +    int ret;
>>> +
>>> +    ttm_tt_init_fields(ttm, bdev, size, page_flags);
>>> +
>>> +    INIT_LIST_HEAD(&ttm_dma->pages_list);
>>> +    if (page_flags & TTM_PAGE_FLAG_SG)
>>> +        ret = ttm_sg_tt_alloc_page_directory(ttm_dma);
>>> +    else
>>> +        ret = ttm_dma_tt_alloc_page_directory(ttm_dma);
>>> +    if (ret) {
>>> +        ttm_tt_destroy(ttm);
>>> +        pr_err("Failed allocating page table\n");
>>> +        return -ENOMEM;
>>> +    }
>>> +    return 0;
>>> +}
>>> +EXPORT_SYMBOL(ttm_sg_tt_init);
>>> +
>>>   void ttm_dma_tt_fini(struct ttm_dma_tt *ttm_dma)
>>>   {
>>>       struct ttm_tt *ttm = &ttm_dma->ttm;
>>>   -    kvfree(ttm->pages);
>>> +    if (ttm->pages)
>>> +        kvfree(ttm->pages);
>>> +    else
>>> +        kvfree(ttm_dma->dma_address);
>>>       ttm->pages = NULL;
>>>       ttm_dma->dma_address = NULL;
>>>   }
>>> diff --git a/include/drm/ttm/ttm_tt.h b/include/drm/ttm/ttm_tt.h
>>> index 9c78556b488e..1cf316a4257c 100644
>>> --- a/include/drm/ttm/ttm_tt.h
>>> +++ b/include/drm/ttm/ttm_tt.h
>>> @@ -163,6 +163,8 @@ int ttm_tt_init(struct ttm_tt *ttm, struct
>>> ttm_bo_device *bdev,
>>>           unsigned long size, uint32_t page_flags);
>>>   int ttm_dma_tt_init(struct ttm_dma_tt *ttm_dma, struct ttm_bo_device
>>> *bdev,
>>>               unsigned long size, uint32_t page_flags);
>>> +int ttm_sg_tt_init(struct ttm_dma_tt *ttm_dma, struct ttm_bo_device
>>> *bdev,
>>> +           unsigned long size, uint32_t page_flags);
>>>     /**
>>>    * ttm_tt_fini
>>
>>
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

RE: [PATCH 4/5] drm/ttm: add ttm_sg_tt_init

[He, Roger]

Patch 1: Acked-by: Roger He <Hongbo.He@amd.com>

Patch 2~5: Reviewed-by: Roger He <Hongbo.He@amd.com>

-----Original Message-----
From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf Of Christian K?nig
Sent: Monday, March 05, 2018 8:07 PM
To: dri-devel@lists.freedesktop.org; amd-gfx@lists.freedesktop.org; Ben Skeggs <bskeggs@redhat.com>; Ilia Mirkin <imirkin@alum.mit.edu>; nouveau <nouveau@lists.freedesktop.org>
Subject: Re: [PATCH 4/5] drm/ttm: add ttm_sg_tt_init

Ping?

Am 27.02.2018 um 13:07 schrieb Christian König:
> Hi guys,
>
> at least on amdgpu and radeon the page array allocated by 
> ttm_dma_tt_init is completely unused in the case of DMA-buf sharing.
> So I'm trying to get rid of that by only allocating the DMA address 
> array.
>
> Now the only other user of DMA-buf together with ttm_dma_tt_init is 
> Nouveau. So my question is are you guys using the page array anywhere 
> in your kernel driver in case of a DMA-buf sharing?
>
> If no then I could just make this the default behavior for all drivers 
> and save quite a bit of memory for everybody.
>
> Thanks,
> Christian.
>
> Am 27.02.2018 um 12:49 schrieb Christian König:
>> This allows drivers to only allocate dma addresses, but not a page 
>> array.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>   drivers/gpu/drm/ttm/ttm_tt.c | 54
>> ++++++++++++++++++++++++++++++++++++--------
>>   include/drm/ttm/ttm_tt.h     |  2 ++
>>   2 files changed, 47 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_tt.c 
>> b/drivers/gpu/drm/ttm/ttm_tt.c index 8e0b525cda00..971133106ec2 
>> 100644
>> --- a/drivers/gpu/drm/ttm/ttm_tt.c
>> +++ b/drivers/gpu/drm/ttm/ttm_tt.c
>> @@ -108,6 +108,16 @@ static int
>> ttm_dma_tt_alloc_page_directory(struct ttm_dma_tt *ttm)
>>       return 0;
>>   }
>>   +static int ttm_sg_tt_alloc_page_directory(struct ttm_dma_tt *ttm)
>> +{
>> +    ttm->dma_address = kvmalloc_array(ttm->ttm.num_pages,
>> +                      sizeof(*ttm->dma_address),
>> +                      GFP_KERNEL | __GFP_ZERO);
>> +    if (!ttm->dma_address)
>> +        return -ENOMEM;
>> +    return 0;
>> +}
>> +
>>   #ifdef CONFIG_X86
>>   static inline int ttm_tt_set_page_caching(struct page *p,
>>                         enum ttm_caching_state c_old, @@ -227,8 
>> +237,8 @@ void ttm_tt_destroy(struct ttm_tt *ttm)
>>       ttm->func->destroy(ttm);
>>   }
>>   -int ttm_tt_init(struct ttm_tt *ttm, struct ttm_bo_device *bdev,
>> -        unsigned long size, uint32_t page_flags)
>> +void ttm_tt_init_fields(struct ttm_tt *ttm, struct ttm_bo_device 
>> +*bdev,
>> +            unsigned long size, uint32_t page_flags)
>>   {
>>       ttm->bdev = bdev;
>>       ttm->num_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT; @@ 
>> -236,6 +246,12 @@ int ttm_tt_init(struct ttm_tt *ttm, struct 
>> ttm_bo_device *bdev,
>>       ttm->page_flags = page_flags;
>>       ttm->state = tt_unpopulated;
>>       ttm->swap_storage = NULL;
>> +}
>> +
>> +int ttm_tt_init(struct ttm_tt *ttm, struct ttm_bo_device *bdev,
>> +        unsigned long size, uint32_t page_flags) {
>> +    ttm_tt_init_fields(ttm, bdev, size, page_flags);
>>         if (ttm_tt_alloc_page_directory(ttm)) {
>>           ttm_tt_destroy(ttm);
>> @@ -258,12 +274,7 @@ int ttm_dma_tt_init(struct ttm_dma_tt *ttm_dma, 
>> struct ttm_bo_device *bdev,
>>   {
>>       struct ttm_tt *ttm = &ttm_dma->ttm;
>>   -    ttm->bdev = bdev;
>> -    ttm->num_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
>> -    ttm->caching_state = tt_cached;
>> -    ttm->page_flags = page_flags;
>> -    ttm->state = tt_unpopulated;
>> -    ttm->swap_storage = NULL;
>> +    ttm_tt_init_fields(ttm, bdev, size, page_flags);
>>         INIT_LIST_HEAD(&ttm_dma->pages_list);
>>       if (ttm_dma_tt_alloc_page_directory(ttm_dma)) { @@ -275,11 
>> +286,36 @@ int ttm_dma_tt_init(struct ttm_dma_tt *ttm_dma, struct 
>> ttm_bo_device *bdev,
>>   }
>>   EXPORT_SYMBOL(ttm_dma_tt_init);
>>   +int ttm_sg_tt_init(struct ttm_dma_tt *ttm_dma, struct 
>> ttm_bo_device *bdev,
>> +           unsigned long size, uint32_t page_flags) {
>> +    struct ttm_tt *ttm = &ttm_dma->ttm;
>> +    int ret;
>> +
>> +    ttm_tt_init_fields(ttm, bdev, size, page_flags);
>> +
>> +    INIT_LIST_HEAD(&ttm_dma->pages_list);
>> +    if (page_flags & TTM_PAGE_FLAG_SG)
>> +        ret = ttm_sg_tt_alloc_page_directory(ttm_dma);
>> +    else
>> +        ret = ttm_dma_tt_alloc_page_directory(ttm_dma);
>> +    if (ret) {
>> +        ttm_tt_destroy(ttm);
>> +        pr_err("Failed allocating page table\n");
>> +        return -ENOMEM;
>> +    }
>> +    return 0;
>> +}
>> +EXPORT_SYMBOL(ttm_sg_tt_init);
>> +
>>   void ttm_dma_tt_fini(struct ttm_dma_tt *ttm_dma)
>>   {
>>       struct ttm_tt *ttm = &ttm_dma->ttm;
>>   -    kvfree(ttm->pages);
>> +    if (ttm->pages)
>> +        kvfree(ttm->pages);
>> +    else
>> +        kvfree(ttm_dma->dma_address);
>>       ttm->pages = NULL;
>>       ttm_dma->dma_address = NULL;
>>   }
>> diff --git a/include/drm/ttm/ttm_tt.h b/include/drm/ttm/ttm_tt.h 
>> index 9c78556b488e..1cf316a4257c 100644
>> --- a/include/drm/ttm/ttm_tt.h
>> +++ b/include/drm/ttm/ttm_tt.h
>> @@ -163,6 +163,8 @@ int ttm_tt_init(struct ttm_tt *ttm, struct 
>> ttm_bo_device *bdev,
>>           unsigned long size, uint32_t page_flags);
>>   int ttm_dma_tt_init(struct ttm_dma_tt *ttm_dma, struct 
>> ttm_bo_device *bdev,
>>               unsigned long size, uint32_t page_flags);
>> +int ttm_sg_tt_init(struct ttm_dma_tt *ttm_dma, struct ttm_bo_device
>> *bdev,
>> +           unsigned long size, uint32_t page_flags);
>>     /**
>>    * ttm_tt_fini
>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [Nouveau] [PATCH 4/5] drm/ttm: add ttm_sg_tt_init

[Daniel Vetter]

On Tue, Feb 27, 2018 at 01:07:06PM +0100, Christian König wrote:
> Hi guys,
> 
> at least on amdgpu and radeon the page array allocated by ttm_dma_tt_init is
> completely unused in the case of DMA-buf sharing. So I'm trying to get rid
> of that by only allocating the DMA address array.
> 
> Now the only other user of DMA-buf together with ttm_dma_tt_init is Nouveau.
> So my question is are you guys using the page array anywhere in your kernel
> driver in case of a DMA-buf sharing?
> 
> If no then I could just make this the default behavior for all drivers and
> save quite a bit of memory for everybody.

+1 on teaching ttm to no longer look at the struct page * in the dma-buf
sgt, but only the dma_buf address.

If there's still some need for in-kernel cpu or userspace mmap access then
imo ttm needs to be fixed to delegate all that to the right dma-buf
interfaces. The ttm abstraction is already there, it's just not passed
through.

I don't pretend to now enough of the details to review this stuff :-)
-Daniel

> 
> Thanks,
> Christian.
> 
> Am 27.02.2018 um 12:49 schrieb Christian König:
> > This allows drivers to only allocate dma addresses, but not a page
> > array.
> > 
> > Signed-off-by: Christian König <christian.koenig@amd.com>
> > ---
> >   drivers/gpu/drm/ttm/ttm_tt.c | 54 ++++++++++++++++++++++++++++++++++++--------
> >   include/drm/ttm/ttm_tt.h     |  2 ++
> >   2 files changed, 47 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
> > index 8e0b525cda00..971133106ec2 100644
> > --- a/drivers/gpu/drm/ttm/ttm_tt.c
> > +++ b/drivers/gpu/drm/ttm/ttm_tt.c
> > @@ -108,6 +108,16 @@ static int ttm_dma_tt_alloc_page_directory(struct ttm_dma_tt *ttm)
> >   	return 0;
> >   }
> > +static int ttm_sg_tt_alloc_page_directory(struct ttm_dma_tt *ttm)
> > +{
> > +	ttm->dma_address = kvmalloc_array(ttm->ttm.num_pages,
> > +					  sizeof(*ttm->dma_address),
> > +					  GFP_KERNEL | __GFP_ZERO);
> > +	if (!ttm->dma_address)
> > +		return -ENOMEM;
> > +	return 0;
> > +}
> > +
> >   #ifdef CONFIG_X86
> >   static inline int ttm_tt_set_page_caching(struct page *p,
> >   					  enum ttm_caching_state c_old,
> > @@ -227,8 +237,8 @@ void ttm_tt_destroy(struct ttm_tt *ttm)
> >   	ttm->func->destroy(ttm);
> >   }
> > -int ttm_tt_init(struct ttm_tt *ttm, struct ttm_bo_device *bdev,
> > -		unsigned long size, uint32_t page_flags)
> > +void ttm_tt_init_fields(struct ttm_tt *ttm, struct ttm_bo_device *bdev,
> > +			unsigned long size, uint32_t page_flags)
> >   {
> >   	ttm->bdev = bdev;
> >   	ttm->num_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
> > @@ -236,6 +246,12 @@ int ttm_tt_init(struct ttm_tt *ttm, struct ttm_bo_device *bdev,
> >   	ttm->page_flags = page_flags;
> >   	ttm->state = tt_unpopulated;
> >   	ttm->swap_storage = NULL;
> > +}
> > +
> > +int ttm_tt_init(struct ttm_tt *ttm, struct ttm_bo_device *bdev,
> > +		unsigned long size, uint32_t page_flags)
> > +{
> > +	ttm_tt_init_fields(ttm, bdev, size, page_flags);
> >   	if (ttm_tt_alloc_page_directory(ttm)) {
> >   		ttm_tt_destroy(ttm);
> > @@ -258,12 +274,7 @@ int ttm_dma_tt_init(struct ttm_dma_tt *ttm_dma, struct ttm_bo_device *bdev,
> >   {
> >   	struct ttm_tt *ttm = &ttm_dma->ttm;
> > -	ttm->bdev = bdev;
> > -	ttm->num_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
> > -	ttm->caching_state = tt_cached;
> > -	ttm->page_flags = page_flags;
> > -	ttm->state = tt_unpopulated;
> > -	ttm->swap_storage = NULL;
> > +	ttm_tt_init_fields(ttm, bdev, size, page_flags);
> >   	INIT_LIST_HEAD(&ttm_dma->pages_list);
> >   	if (ttm_dma_tt_alloc_page_directory(ttm_dma)) {
> > @@ -275,11 +286,36 @@ int ttm_dma_tt_init(struct ttm_dma_tt *ttm_dma, struct ttm_bo_device *bdev,
> >   }
> >   EXPORT_SYMBOL(ttm_dma_tt_init);
> > +int ttm_sg_tt_init(struct ttm_dma_tt *ttm_dma, struct ttm_bo_device *bdev,
> > +		   unsigned long size, uint32_t page_flags)
> > +{
> > +	struct ttm_tt *ttm = &ttm_dma->ttm;
> > +	int ret;
> > +
> > +	ttm_tt_init_fields(ttm, bdev, size, page_flags);
> > +
> > +	INIT_LIST_HEAD(&ttm_dma->pages_list);
> > +	if (page_flags & TTM_PAGE_FLAG_SG)
> > +		ret = ttm_sg_tt_alloc_page_directory(ttm_dma);
> > +	else
> > +		ret = ttm_dma_tt_alloc_page_directory(ttm_dma);
> > +	if (ret) {
> > +		ttm_tt_destroy(ttm);
> > +		pr_err("Failed allocating page table\n");
> > +		return -ENOMEM;
> > +	}
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(ttm_sg_tt_init);
> > +
> >   void ttm_dma_tt_fini(struct ttm_dma_tt *ttm_dma)
> >   {
> >   	struct ttm_tt *ttm = &ttm_dma->ttm;
> > -	kvfree(ttm->pages);
> > +	if (ttm->pages)
> > +		kvfree(ttm->pages);
> > +	else
> > +		kvfree(ttm_dma->dma_address);
> >   	ttm->pages = NULL;
> >   	ttm_dma->dma_address = NULL;
> >   }
> > diff --git a/include/drm/ttm/ttm_tt.h b/include/drm/ttm/ttm_tt.h
> > index 9c78556b488e..1cf316a4257c 100644
> > --- a/include/drm/ttm/ttm_tt.h
> > +++ b/include/drm/ttm/ttm_tt.h
> > @@ -163,6 +163,8 @@ int ttm_tt_init(struct ttm_tt *ttm, struct ttm_bo_device *bdev,
> >   		unsigned long size, uint32_t page_flags);
> >   int ttm_dma_tt_init(struct ttm_dma_tt *ttm_dma, struct ttm_bo_device *bdev,
> >   		    unsigned long size, uint32_t page_flags);
> > +int ttm_sg_tt_init(struct ttm_dma_tt *ttm_dma, struct ttm_bo_device *bdev,
> > +		   unsigned long size, uint32_t page_flags);
> >   /**
> >    * ttm_tt_fini
> 
> _______________________________________________
> Nouveau mailing list
> Nouveau@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 5/5] drm/amdgpu: stop allocating a page array for prime shared BOs

[Christian König]

We don't need the page array for prime shared BOs, stop allocating it.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c | 2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c  | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
index 137145dd14a9..dc8d9f3216fa 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
@@ -315,7 +315,7 @@ int amdgpu_gart_bind(struct amdgpu_device *adev, uint64_t offset,
 	t = offset / AMDGPU_GPU_PAGE_SIZE;
 	p = t / (PAGE_SIZE / AMDGPU_GPU_PAGE_SIZE);
 	for (i = 0; i < pages; i++, p++)
-		adev->gart.pages[p] = pagelist[i];
+		adev->gart.pages[p] = pagelist ? pagelist[i] : NULL;
 #endif
 
 	if (!adev->gart.ptr)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index e38e6db8f760..854421af1982 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -982,7 +982,7 @@ static struct ttm_tt *amdgpu_ttm_tt_create(struct ttm_bo_device *bdev,
 	}
 	gtt->ttm.ttm.func = &amdgpu_backend_func;
 	gtt->adev = adev;
-	if (ttm_dma_tt_init(&gtt->ttm, bdev, size, page_flags)) {
+	if (ttm_sg_tt_init(&gtt->ttm, bdev, size, page_flags)) {
 		kfree(gtt);
 		return NULL;
 	}
@@ -1008,7 +1008,8 @@ static int amdgpu_ttm_tt_populate(struct ttm_tt *ttm,
 
 	if (slave && ttm->sg) {
 		drm_prime_sg_to_page_addr_arrays(ttm->sg, ttm->pages,
-						 gtt->ttm.dma_address, ttm->num_pages);
+						 gtt->ttm.dma_address,
+						 ttm->num_pages);
 		ttm->state = tt_unbound;
 		return 0;
 	}
-- 
2.14.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/5] drm/prime: fix potential race in drm_gem_map_detach

[Christian König]

Ping? Anybody who could give me an review on those changes?

The first one is just nice to have, the rest is a nice memory usage 
reduction in some cases.

Christian.

Am 27.02.2018 um 12:49 schrieb Christian König:
> Unpin the GEM object only after freeing the sg table.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/drm_prime.c | 32 ++++++++++++++++----------------
>   1 file changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index e82a976f0fba..c38dacda6119 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -230,26 +230,26 @@ void drm_gem_map_detach(struct dma_buf *dma_buf,
>   	struct drm_prime_attachment *prime_attach = attach->priv;
>   	struct drm_gem_object *obj = dma_buf->priv;
>   	struct drm_device *dev = obj->dev;
> -	struct sg_table *sgt;
>   
> -	if (dev->driver->gem_prime_unpin)
> -		dev->driver->gem_prime_unpin(obj);
> +	if (prime_attach) {
> +		struct sg_table *sgt = prime_attach->sgt;
>   
> -	if (!prime_attach)
> -		return;
> -
> -	sgt = prime_attach->sgt;
> -	if (sgt) {
> -		if (prime_attach->dir != DMA_NONE)
> -			dma_unmap_sg_attrs(attach->dev, sgt->sgl, sgt->nents,
> -					   prime_attach->dir,
> -					   DMA_ATTR_SKIP_CPU_SYNC);
> -		sg_free_table(sgt);
> +		if (sgt) {
> +			if (prime_attach->dir != DMA_NONE)
> +				dma_unmap_sg_attrs(attach->dev, sgt->sgl,
> +						   sgt->nents,
> +						   prime_attach->dir,
> +						   DMA_ATTR_SKIP_CPU_SYNC);
> +			sg_free_table(sgt);
> +		}
> +
> +		kfree(sgt);
> +		kfree(prime_attach);
> +		attach->priv = NULL;
>   	}
>   
> -	kfree(sgt);
> -	kfree(prime_attach);
> -	attach->priv = NULL;
> +	if (dev->driver->gem_prime_unpin)
> +		dev->driver->gem_prime_unpin(obj);
>   }
>   EXPORT_SYMBOL(drm_gem_map_detach);
>   

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/5] drm/prime: fix potential race in drm_gem_map_detach

[Lucas Stach]

Hi Christian,

Am Dienstag, den 27.02.2018, 12:49 +0100 schrieb Christian König:
> Unpin the GEM object only after freeing the sg table.

What is the race that is being fixed here? The SG table is private to
the importer and the importer should hopefully only call map_detach if
it is done with all operations using the SG table. Thus it shouldn't
matter that the SG table might point to moved pages during execution of
this function.

Regards,
Lucas

> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/drm_prime.c | 32 ++++++++++++++++----------------
>  1 file changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_prime.c
> b/drivers/gpu/drm/drm_prime.c
> index e82a976f0fba..c38dacda6119 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -230,26 +230,26 @@ void drm_gem_map_detach(struct dma_buf
> *dma_buf,
>  	struct drm_prime_attachment *prime_attach = attach->priv;
>  	struct drm_gem_object *obj = dma_buf->priv;
>  	struct drm_device *dev = obj->dev;
> -	struct sg_table *sgt;
>  
> -	if (dev->driver->gem_prime_unpin)
> -		dev->driver->gem_prime_unpin(obj);
> +	if (prime_attach) {
> +		struct sg_table *sgt = prime_attach->sgt;
>  
> -	if (!prime_attach)
> -		return;
> -
> -	sgt = prime_attach->sgt;
> -	if (sgt) {
> -		if (prime_attach->dir != DMA_NONE)
> -			dma_unmap_sg_attrs(attach->dev, sgt->sgl,
> sgt->nents,
> -					   prime_attach->dir,
> -					   DMA_ATTR_SKIP_CPU_SYNC);
> -		sg_free_table(sgt);
> +		if (sgt) {
> +			if (prime_attach->dir != DMA_NONE)
> +				dma_unmap_sg_attrs(attach->dev, sgt-
> >sgl,
> +						   sgt->nents,
> +						   prime_attach-
> >dir,
> +						   DMA_ATTR_SKIP_CPU
> _SYNC);
> +			sg_free_table(sgt);
> +		}
> +
> +		kfree(sgt);
> +		kfree(prime_attach);
> +		attach->priv = NULL;
>  	}
>  
> -	kfree(sgt);
> -	kfree(prime_attach);
> -	attach->priv = NULL;
> +	if (dev->driver->gem_prime_unpin)
> +		dev->driver->gem_prime_unpin(obj);
>  }
>  EXPORT_SYMBOL(drm_gem_map_detach);
>  
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/5] drm/prime: fix potential race in drm_gem_map_detach

[Christian König]

Am 28.02.2018 um 10:48 schrieb Lucas Stach:
> Hi Christian,
>
> Am Dienstag, den 27.02.2018, 12:49 +0100 schrieb Christian König:
>> Unpin the GEM object only after freeing the sg table.
> What is the race that is being fixed here? The SG table is private to
> the importer and the importer should hopefully only call map_detach if
> it is done with all operations using the SG table. Thus it shouldn't
> matter that the SG table might point to moved pages during execution of
> this function.

Exactly, it shouldn't matter. This is just a precaution.

When the device driver is buggy I want proper error messages from IOMMU 
and not accessing pages which might already be reused for something else.

Regards,
Christian.

>
> Regards,
> Lucas
>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>   drivers/gpu/drm/drm_prime.c | 32 ++++++++++++++++----------------
>>   1 file changed, 16 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_prime.c
>> b/drivers/gpu/drm/drm_prime.c
>> index e82a976f0fba..c38dacda6119 100644
>> --- a/drivers/gpu/drm/drm_prime.c
>> +++ b/drivers/gpu/drm/drm_prime.c
>> @@ -230,26 +230,26 @@ void drm_gem_map_detach(struct dma_buf
>> *dma_buf,
>>   	struct drm_prime_attachment *prime_attach = attach->priv;
>>   	struct drm_gem_object *obj = dma_buf->priv;
>>   	struct drm_device *dev = obj->dev;
>> -	struct sg_table *sgt;
>>   
>> -	if (dev->driver->gem_prime_unpin)
>> -		dev->driver->gem_prime_unpin(obj);
>> +	if (prime_attach) {
>> +		struct sg_table *sgt = prime_attach->sgt;
>>   
>> -	if (!prime_attach)
>> -		return;
>> -
>> -	sgt = prime_attach->sgt;
>> -	if (sgt) {
>> -		if (prime_attach->dir != DMA_NONE)
>> -			dma_unmap_sg_attrs(attach->dev, sgt->sgl,
>> sgt->nents,
>> -					   prime_attach->dir,
>> -					   DMA_ATTR_SKIP_CPU_SYNC);
>> -		sg_free_table(sgt);
>> +		if (sgt) {
>> +			if (prime_attach->dir != DMA_NONE)
>> +				dma_unmap_sg_attrs(attach->dev, sgt-
>>> sgl,
>> +						   sgt->nents,
>> +						   prime_attach-
>>> dir,
>> +						   DMA_ATTR_SKIP_CPU
>> _SYNC);
>> +			sg_free_table(sgt);
>> +		}
>> +
>> +		kfree(sgt);
>> +		kfree(prime_attach);
>> +		attach->priv = NULL;
>>   	}
>>   
>> -	kfree(sgt);
>> -	kfree(prime_attach);
>> -	attach->priv = NULL;
>> +	if (dev->driver->gem_prime_unpin)
>> +		dev->driver->gem_prime_unpin(obj);
>>   }
>>   EXPORT_SYMBOL(drm_gem_map_detach);
>>   

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/5] drm/prime: fix potential race in drm_gem_map_detach

[Daniel Vetter]

On Wed, Feb 28, 2018 at 11:25:59AM +0100, Christian König wrote:
> Am 28.02.2018 um 10:48 schrieb Lucas Stach:
> > Hi Christian,
> > 
> > Am Dienstag, den 27.02.2018, 12:49 +0100 schrieb Christian König:
> > > Unpin the GEM object only after freeing the sg table.
> > What is the race that is being fixed here? The SG table is private to
> > the importer and the importer should hopefully only call map_detach if
> > it is done with all operations using the SG table. Thus it shouldn't
> > matter that the SG table might point to moved pages during execution of
> > this function.
> 
> Exactly, it shouldn't matter. This is just a precaution.
> 
> When the device driver is buggy I want proper error messages from IOMMU and
> not accessing pages which might already be reused for something else.

Please add this to the commit message, rather crucial to understand the
motivation. With that fixed you can have my

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

And pls push to drm-misc.
-Daniel

> 
> Regards,
> Christian.
> 
> > 
> > Regards,
> > Lucas
> > 
> > > Signed-off-by: Christian König <christian.koenig@amd.com>
> > > ---
> > >   drivers/gpu/drm/drm_prime.c | 32 ++++++++++++++++----------------
> > >   1 file changed, 16 insertions(+), 16 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_prime.c
> > > b/drivers/gpu/drm/drm_prime.c
> > > index e82a976f0fba..c38dacda6119 100644
> > > --- a/drivers/gpu/drm/drm_prime.c
> > > +++ b/drivers/gpu/drm/drm_prime.c
> > > @@ -230,26 +230,26 @@ void drm_gem_map_detach(struct dma_buf
> > > *dma_buf,
> > >   	struct drm_prime_attachment *prime_attach = attach->priv;
> > >   	struct drm_gem_object *obj = dma_buf->priv;
> > >   	struct drm_device *dev = obj->dev;
> > > -	struct sg_table *sgt;
> > > -	if (dev->driver->gem_prime_unpin)
> > > -		dev->driver->gem_prime_unpin(obj);
> > > +	if (prime_attach) {
> > > +		struct sg_table *sgt = prime_attach->sgt;
> > > -	if (!prime_attach)
> > > -		return;
> > > -
> > > -	sgt = prime_attach->sgt;
> > > -	if (sgt) {
> > > -		if (prime_attach->dir != DMA_NONE)
> > > -			dma_unmap_sg_attrs(attach->dev, sgt->sgl,
> > > sgt->nents,
> > > -					   prime_attach->dir,
> > > -					   DMA_ATTR_SKIP_CPU_SYNC);
> > > -		sg_free_table(sgt);
> > > +		if (sgt) {
> > > +			if (prime_attach->dir != DMA_NONE)
> > > +				dma_unmap_sg_attrs(attach->dev, sgt-
> > > > sgl,
> > > +						   sgt->nents,
> > > +						   prime_attach-
> > > > dir,
> > > +						   DMA_ATTR_SKIP_CPU
> > > _SYNC);
> > > +			sg_free_table(sgt);
> > > +		}
> > > +
> > > +		kfree(sgt);
> > > +		kfree(prime_attach);
> > > +		attach->priv = NULL;
> > >   	}
> > > -	kfree(sgt);
> > > -	kfree(prime_attach);
> > > -	attach->priv = NULL;
> > > +	if (dev->driver->gem_prime_unpin)
> > > +		dev->driver->gem_prime_unpin(obj);
> > >   }
> > >   EXPORT_SYMBOL(drm_gem_map_detach);
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/5] drm/prime: fix potential race in drm_gem_map_detach

[Christian König]

Am 06.03.2018 um 10:15 schrieb Daniel Vetter:
> On Wed, Feb 28, 2018 at 11:25:59AM +0100, Christian König wrote:
>> Am 28.02.2018 um 10:48 schrieb Lucas Stach:
>>> Hi Christian,
>>>
>>> Am Dienstag, den 27.02.2018, 12:49 +0100 schrieb Christian König:
>>>> Unpin the GEM object only after freeing the sg table.
>>> What is the race that is being fixed here? The SG table is private to
>>> the importer and the importer should hopefully only call map_detach if
>>> it is done with all operations using the SG table. Thus it shouldn't
>>> matter that the SG table might point to moved pages during execution of
>>> this function.
>> Exactly, it shouldn't matter. This is just a precaution.
>>
>> When the device driver is buggy I want proper error messages from IOMMU and
>> not accessing pages which might already be reused for something else.
> Please add this to the commit message, rather crucial to understand the
> motivation. With that fixed you can have my
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> And pls push to drm-misc.

Can I use standard git for that now? I really don't want to mess with 
dim in my environment.

Christian.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/5] drm/prime: fix potential race in drm_gem_map_detach

[Daniel Vetter]

On Tue, Mar 06, 2018 at 10:30:56AM +0100, Christian König wrote:
> Am 06.03.2018 um 10:15 schrieb Daniel Vetter:
> > On Wed, Feb 28, 2018 at 11:25:59AM +0100, Christian König wrote:
> > > Am 28.02.2018 um 10:48 schrieb Lucas Stach:
> > > > Hi Christian,
> > > > 
> > > > Am Dienstag, den 27.02.2018, 12:49 +0100 schrieb Christian König:
> > > > > Unpin the GEM object only after freeing the sg table.
> > > > What is the race that is being fixed here? The SG table is private to
> > > > the importer and the importer should hopefully only call map_detach if
> > > > it is done with all operations using the SG table. Thus it shouldn't
> > > > matter that the SG table might point to moved pages during execution of
> > > > this function.
> > > Exactly, it shouldn't matter. This is just a precaution.
> > > 
> > > When the device driver is buggy I want proper error messages from IOMMU and
> > > not accessing pages which might already be reused for something else.
> > Please add this to the commit message, rather crucial to understand the
> > motivation. With that fixed you can have my
> > 
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > 
> > And pls push to drm-misc.
> 
> Can I use standard git for that now? I really don't want to mess with dim in
> my environment.

Ping Alex to run it for you please. In an ideal world we'd run all that
stuff server-side, but that's not happening anytime soon.

Also if you have any specific issues about dim stomping over your setup,
we'll be happy to fix it. If you set the (relative) paths correctly you
can hide the various additional checkouts it needs rather well.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel