diff mbox

[2/4] drm/ttm: Add optional support for two-ended allocation

Message ID 20140331152818.cfcea033.cand@gmx.com (mailing list archive)
State New, archived
Headers show

Commit Message

Lauri Kasanen March 31, 2014, 12:28 p.m. UTC
Allocating small bos from one end and large ones from the other helps
improve the quality of fragmentation.

This depends on "drm: Optionally create mm blocks from top-to-bottom" by
Chris Wilson.

Signed-off-by: Lauri Kasanen <cand@gmx.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c         |  4 +++-
 drivers/gpu/drm/ttm/ttm_bo_manager.c | 16 +++++++++++++---
 include/drm/ttm/ttm_bo_driver.h      |  7 ++++++-
 3 files changed, 22 insertions(+), 5 deletions(-)

Comments

Lucas Stach March 31, 2014, 12:41 p.m. UTC | #1
Am Montag, den 31.03.2014, 15:28 +0300 schrieb Lauri Kasanen:
> Allocating small bos from one end and large ones from the other helps
> improve the quality of fragmentation.
> 
> This depends on "drm: Optionally create mm blocks from top-to-bottom" by
> Chris Wilson.
> 
You are breaking bisectability here. This patch deliberately introduces
a build failure that you only fix up in the next two patches. This isn't
acceptable, you must squash the following patches in to avoid the
breakage.

Also if the first patch is from Chris Wilson and you only did some small
changes this should be reflected in the author name of the patch.

> Signed-off-by: Lauri Kasanen <cand@gmx.com>
> ---
>  drivers/gpu/drm/ttm/ttm_bo.c         |  4 +++-
>  drivers/gpu/drm/ttm/ttm_bo_manager.c | 16 +++++++++++++---
>  include/drm/ttm/ttm_bo_driver.h      |  7 ++++++-
>  3 files changed, 22 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 9df79ac..caf7cd3 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -1453,7 +1453,8 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
>  		       struct ttm_bo_driver *driver,
>  		       struct address_space *mapping,
>  		       uint64_t file_page_offset,
> -		       bool need_dma32)
> +		       bool need_dma32,
> +		       uint32_t alloc_threshold)
>  {
>  	int ret = -EINVAL;
>  
> @@ -1476,6 +1477,7 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
>  	bdev->dev_mapping = mapping;
>  	bdev->glob = glob;
>  	bdev->need_dma32 = need_dma32;
> +	bdev->alloc_threshold = alloc_threshold;
>  	bdev->val_seq = 0;
>  	spin_lock_init(&bdev->fence_lock);
>  	mutex_lock(&glob->device_list_mutex);
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_manager.c b/drivers/gpu/drm/ttm/ttm_bo_manager.c
> index c58eba33..db9fcb4 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_manager.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_manager.c
> @@ -55,6 +55,7 @@ static int ttm_bo_man_get_node(struct ttm_mem_type_manager *man,
>  	struct ttm_range_manager *rman = (struct ttm_range_manager *) man->priv;
>  	struct drm_mm *mm = &rman->mm;
>  	struct drm_mm_node *node = NULL;
> +	enum drm_mm_allocator_flags aflags = DRM_MM_CREATE_DEFAULT;
>  	unsigned long lpfn;
>  	int ret;
>  
> @@ -65,12 +66,21 @@ static int ttm_bo_man_get_node(struct ttm_mem_type_manager *man,
>  	node = kzalloc(sizeof(*node), GFP_KERNEL);
>  	if (!node)
>  		return -ENOMEM;
> +	/**
> +	 * If the driver requested a threshold, use two-ended allocation.
> +	 * Pinned buffers require bottom-up allocation.
> +	 */
> +	if (man->bdev->alloc_threshold &&
> +		!(bo->mem.placement & TTM_PL_FLAG_NO_EVICT) &&
> +		man->bdev->alloc_threshold < (mem->num_pages * PAGE_SIZE))
> +		aflags = DRM_MM_CREATE_TOP;
>  
>  	spin_lock(&rman->lock);
> -	ret = drm_mm_insert_node_in_range(mm, node, mem->num_pages,
> -					  mem->page_alignment,
> +	ret = drm_mm_insert_node_in_range_generic(mm, node, mem->num_pages,
> +					  mem->page_alignment, 0,
>  					  placement->fpfn, lpfn,
> -					  DRM_MM_SEARCH_BEST);
> +					  DRM_MM_SEARCH_BEST,
> +					  aflags);
>  	spin_unlock(&rman->lock);
>  
>  	if (unlikely(ret)) {
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 5d8aabe..f5fe6df 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -565,6 +565,7 @@ struct ttm_bo_device {
>  	struct delayed_work wq;
>  
>  	bool need_dma32;
> +	uint32_t alloc_threshold;
>  };
>  
>  /**
> @@ -751,6 +752,8 @@ extern int ttm_bo_device_release(struct ttm_bo_device *bdev);
>   * @file_page_offset: Offset into the device address space that is available
>   * for buffer data. This ensures compatibility with other users of the
>   * address space.
> + * @alloc_threshold: If non-zero, use this as the threshold for two-ended
> + * allocation.
>   *
>   * Initializes a struct ttm_bo_device:
>   * Returns:
> @@ -760,7 +763,9 @@ extern int ttm_bo_device_init(struct ttm_bo_device *bdev,
>  			      struct ttm_bo_global *glob,
>  			      struct ttm_bo_driver *driver,
>  			      struct address_space *mapping,
> -			      uint64_t file_page_offset, bool need_dma32);
> +			      uint64_t file_page_offset,
> +			      bool need_dma32,
> +			      uint32_t alloc_threshold);
>  
>  /**
>   * ttm_bo_unmap_virtual
Lauri Kasanen March 31, 2014, 2:51 p.m. UTC | #2
On Mon, 31 Mar 2014 14:41:05 +0200
Lucas Stach <l.stach@pengutronix.de> wrote:

> Am Montag, den 31.03.2014, 15:28 +0300 schrieb Lauri Kasanen:
> > Allocating small bos from one end and large ones from the other helps
> > improve the quality of fragmentation.
> > 
> > This depends on "drm: Optionally create mm blocks from top-to-bottom" by
> > Chris Wilson.
> > 
> You are breaking bisectability here. This patch deliberately introduces
> a build failure that you only fix up in the next two patches. This isn't
> acceptable, you must squash the following patches in to avoid the
> breakage.
> 
> Also if the first patch is from Chris Wilson and you only did some small
> changes this should be reflected in the author name of the patch.

OK, will squash and resend. Then the changes are mostly mine in the
singular patch.

BTW, how would I handle the case of a different author, when I cannot
use git-send-email due to network mail restrictions? I can't send email
from an address I don't own.

- Lauri
Lucas Stach March 31, 2014, 3:08 p.m. UTC | #3
Am Montag, den 31.03.2014, 17:51 +0300 schrieb Lauri Kasanen:
> On Mon, 31 Mar 2014 14:41:05 +0200
> Lucas Stach <l.stach@pengutronix.de> wrote:
> 
> > Am Montag, den 31.03.2014, 15:28 +0300 schrieb Lauri Kasanen:
> > > Allocating small bos from one end and large ones from the other helps
> > > improve the quality of fragmentation.
> > > 
> > > This depends on "drm: Optionally create mm blocks from top-to-bottom" by
> > > Chris Wilson.
> > > 
> > You are breaking bisectability here. This patch deliberately introduces
> > a build failure that you only fix up in the next two patches. This isn't
> > acceptable, you must squash the following patches in to avoid the
> > breakage.
> > 
> > Also if the first patch is from Chris Wilson and you only did some small
> > changes this should be reflected in the author name of the patch.
> 
> OK, will squash and resend. Then the changes are mostly mine in the
> singular patch.
> 
> BTW, how would I handle the case of a different author, when I cannot
> use git-send-email due to network mail restrictions? I can't send email
> from an address I don't own.
> 
You should send it from your address, but start the patch description
with a line "From: Original Author <original@author.bla>", so git picks
up the proper authorship. Git send-email does this the same way.

Regards,
Lucas
diff mbox

Patch

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 9df79ac..caf7cd3 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1453,7 +1453,8 @@  int ttm_bo_device_init(struct ttm_bo_device *bdev,
 		       struct ttm_bo_driver *driver,
 		       struct address_space *mapping,
 		       uint64_t file_page_offset,
-		       bool need_dma32)
+		       bool need_dma32,
+		       uint32_t alloc_threshold)
 {
 	int ret = -EINVAL;
 
@@ -1476,6 +1477,7 @@  int ttm_bo_device_init(struct ttm_bo_device *bdev,
 	bdev->dev_mapping = mapping;
 	bdev->glob = glob;
 	bdev->need_dma32 = need_dma32;
+	bdev->alloc_threshold = alloc_threshold;
 	bdev->val_seq = 0;
 	spin_lock_init(&bdev->fence_lock);
 	mutex_lock(&glob->device_list_mutex);
diff --git a/drivers/gpu/drm/ttm/ttm_bo_manager.c b/drivers/gpu/drm/ttm/ttm_bo_manager.c
index c58eba33..db9fcb4 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_manager.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_manager.c
@@ -55,6 +55,7 @@  static int ttm_bo_man_get_node(struct ttm_mem_type_manager *man,
 	struct ttm_range_manager *rman = (struct ttm_range_manager *) man->priv;
 	struct drm_mm *mm = &rman->mm;
 	struct drm_mm_node *node = NULL;
+	enum drm_mm_allocator_flags aflags = DRM_MM_CREATE_DEFAULT;
 	unsigned long lpfn;
 	int ret;
 
@@ -65,12 +66,21 @@  static int ttm_bo_man_get_node(struct ttm_mem_type_manager *man,
 	node = kzalloc(sizeof(*node), GFP_KERNEL);
 	if (!node)
 		return -ENOMEM;
+	/**
+	 * If the driver requested a threshold, use two-ended allocation.
+	 * Pinned buffers require bottom-up allocation.
+	 */
+	if (man->bdev->alloc_threshold &&
+		!(bo->mem.placement & TTM_PL_FLAG_NO_EVICT) &&
+		man->bdev->alloc_threshold < (mem->num_pages * PAGE_SIZE))
+		aflags = DRM_MM_CREATE_TOP;
 
 	spin_lock(&rman->lock);
-	ret = drm_mm_insert_node_in_range(mm, node, mem->num_pages,
-					  mem->page_alignment,
+	ret = drm_mm_insert_node_in_range_generic(mm, node, mem->num_pages,
+					  mem->page_alignment, 0,
 					  placement->fpfn, lpfn,
-					  DRM_MM_SEARCH_BEST);
+					  DRM_MM_SEARCH_BEST,
+					  aflags);
 	spin_unlock(&rman->lock);
 
 	if (unlikely(ret)) {
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 5d8aabe..f5fe6df 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -565,6 +565,7 @@  struct ttm_bo_device {
 	struct delayed_work wq;
 
 	bool need_dma32;
+	uint32_t alloc_threshold;
 };
 
 /**
@@ -751,6 +752,8 @@  extern int ttm_bo_device_release(struct ttm_bo_device *bdev);
  * @file_page_offset: Offset into the device address space that is available
  * for buffer data. This ensures compatibility with other users of the
  * address space.
+ * @alloc_threshold: If non-zero, use this as the threshold for two-ended
+ * allocation.
  *
  * Initializes a struct ttm_bo_device:
  * Returns:
@@ -760,7 +763,9 @@  extern int ttm_bo_device_init(struct ttm_bo_device *bdev,
 			      struct ttm_bo_global *glob,
 			      struct ttm_bo_driver *driver,
 			      struct address_space *mapping,
-			      uint64_t file_page_offset, bool need_dma32);
+			      uint64_t file_page_offset,
+			      bool need_dma32,
+			      uint32_t alloc_threshold);
 
 /**
  * ttm_bo_unmap_virtual